From 752898b6eeab5ea5f2ed843c7e4ab17559e6ee82 Mon Sep 17 00:00:00 2001 From: nwithan8 Date: Thu, 23 Mar 2023 15:35:19 -0600 Subject: [PATCH 1/3] - Improve error handling (no more ambiguous NullPointerException when there's no matching recording in a cassette) --- .../RecordableHttpURLConnection.java | 9 ++++++ .../RecordableHttpsURLConnection.java | 9 ++++++ src/test/java/HttpUrlConnectionTest.java | 29 +++++++++++++++++++ 3 files changed, 47 insertions(+) diff --git a/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpURLConnection.java b/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpURLConnection.java index d8c5dd7..f722383 100644 --- a/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpURLConnection.java +++ b/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpURLConnection.java @@ -144,6 +144,12 @@ public RecordableHttpURLConnection(URL url, Cassette cassette, Mode mode) this(url, cassette, mode, new AdvancedSettings()); } + private void cachedInteractionExistsOtherwiseError() throws VCRException { + if (this.cachedInteraction != null) { + throw new VCRException("No matching interaction found."); + } + } + /** * Get an object from the cache. * @@ -725,6 +731,8 @@ Based on this Sun source code for HttpURLConnection (seen below): try { buildCache(); + cachedInteractionExistsOtherwiseError(); + if (this.cachedInteraction.getResponse().getStatus().getCode() >= 400) { // Client Error 4xx and Server Error 5xx return createInputStream(this.cachedInteraction.getResponse().getBody()); @@ -1297,6 +1305,7 @@ public InputStream getInputStream() throws IOException { } try { buildCache(); + cachedInteractionExistsOtherwiseError(); return createInputStream(this.cachedInteraction.getResponse().getBody()); } catch (VCRException | RecordingExpirationException e) { throw new RuntimeException(e); diff --git a/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpsURLConnection.java b/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpsURLConnection.java index 175f550..462da45 100644 --- a/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpsURLConnection.java +++ b/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpsURLConnection.java @@ -149,6 +149,12 @@ public RecordableHttpsURLConnection(URL url, Cassette cassette, Mode mode) this(url, cassette, mode, new AdvancedSettings()); } + private void cachedInteractionExistsOtherwiseError() throws VCRException { + if (this.cachedInteraction != null) { + throw new VCRException("No matching interaction found."); + } + } + /** * Get an object from the cache. * @@ -728,6 +734,8 @@ Based on this Sun source code for HttpURLConnection (seen below): try { buildCache(); + cachedInteractionExistsOtherwiseError(); + if (this.cachedInteraction.getResponse().getStatus().getCode() >= 400) { // Client Error 4xx and Server Error 5xx return createInputStream(this.cachedInteraction.getResponse().getBody()); @@ -1300,6 +1308,7 @@ public InputStream getInputStream() throws IOException { } try { buildCache(); + cachedInteractionExistsOtherwiseError(); return createInputStream(this.cachedInteraction.getResponse().getBody()); } catch (VCRException | RecordingExpirationException e) { throw new RuntimeException(e); diff --git a/src/test/java/HttpUrlConnectionTest.java b/src/test/java/HttpUrlConnectionTest.java index b012cdf..1d1b282 100644 --- a/src/test/java/HttpUrlConnectionTest.java +++ b/src/test/java/HttpUrlConnectionTest.java @@ -9,6 +9,7 @@ import com.easypost.easyvcr.Mode; import com.easypost.easyvcr.RecordingExpirationException; import com.easypost.easyvcr.TimeFrame; +import com.easypost.easyvcr.VCRException; import com.easypost.easyvcr.clients.httpurlconnection.RecordableHttpsURLConnection; import com.google.gson.JsonParseException; import org.junit.Assert; @@ -534,4 +535,32 @@ public void testReplayHttpError() throws Exception { Assert.assertNotNull(clientAfterRequest); Assert.assertNotNull(clientAfterRequest.getErrorStream()); } + + @Test + public void testCachedInteractionDoesNotExist() throws Exception { + Cassette cassette = TestUtils.getCassette("test_cached_interaction_does_not_exist"); + cassette.erase(); // Erase cassette before recording + + // make connection using Mode.Record + RecordableHttpsURLConnection connection = + (RecordableHttpsURLConnection) HttpClients.newClient(HttpClientType.HttpsUrlConnection, + FakeDataService.URL, cassette, Mode.Record); + // make data service using connection + FakeDataService.HttpsUrlConnection fakeDataService = new FakeDataService.HttpsUrlConnection(connection); + // make HTTP call with data service (record to cassette) + fakeDataService.getIPAddressDataRawResponse(); + Assert.assertTrue(cassette.numInteractions() > 0); // make sure we recorded something + + // Attempt to replay a cached interaction that does not exist + connection = (RecordableHttpsURLConnection) HttpClients.newClient(HttpClientType.HttpsUrlConnection, + FakeDataService.URL + "1", cassette, Mode.Replay); + // make data service using connection + fakeDataService = new FakeDataService.HttpsUrlConnection(connection); + // make HTTP call with data service (replay from cassette) + + // this throws a RuntimeException because of the way the exceptions are coalesced internally + FakeDataService.HttpsUrlConnection finalFakeDataService = fakeDataService; + fakeDataService.getIPAddressData(); + // Assert.assertThrows(RuntimeException.class, finalFakeDataService::getIPAddressData); + } } From afa8f6d3e9e7fd70c20c67e33bfb494a34c77b7f Mon Sep 17 00:00:00 2001 From: nwithan8 Date: Thu, 23 Mar 2023 16:14:53 -0600 Subject: [PATCH 2/3] - Improve cached interaction check logic - Fix bug with comparing base URLs --- .../java/com/easypost/easyvcr/MatchRules.java | 17 ++++++++++-- .../RecordableHttpURLConnection.java | 2 +- .../RecordableHttpsURLConnection.java | 2 +- src/test/java/HttpUrlConnectionTest.java | 27 ++++++++++--------- 4 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/main/java/com/easypost/easyvcr/MatchRules.java b/src/main/java/com/easypost/easyvcr/MatchRules.java index caa00f5..504341c 100644 --- a/src/main/java/com/easypost/easyvcr/MatchRules.java +++ b/src/main/java/com/easypost/easyvcr/MatchRules.java @@ -3,6 +3,7 @@ import com.easypost.easyvcr.internal.Utilities; import com.easypost.easyvcr.requestelements.Request; +import java.net.URI; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -50,13 +51,25 @@ private void by(BiFunction rule) { */ public MatchRules byBaseUrl() { by((received, recorded) -> { - String receivedUri = received.getUri().getPath(); - String recordedUri = recorded.getUri().getPath(); + String receivedUri = getBaseUrl(received.getUri()); + String recordedUri = getBaseUrl(recorded.getUri()); return receivedUri.equalsIgnoreCase(recordedUri); }); return this; } + private static String getBaseUrl(URI url) { + + String baseUrl = url.getScheme() + "://" + url.getHost(); + if (url.getPort() != -1) { + baseUrl += ":" + url.getPort(); + } + if (url.getPath() != null) { + baseUrl += url.getPath(); + } + return baseUrl; + } + /** * Add a rule to compare the bodies of the requests. * diff --git a/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpURLConnection.java b/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpURLConnection.java index f722383..6b6fcc8 100644 --- a/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpURLConnection.java +++ b/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpURLConnection.java @@ -145,7 +145,7 @@ public RecordableHttpURLConnection(URL url, Cassette cassette, Mode mode) } private void cachedInteractionExistsOtherwiseError() throws VCRException { - if (this.cachedInteraction != null) { + if (this.cachedInteraction == null) { throw new VCRException("No matching interaction found."); } } diff --git a/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpsURLConnection.java b/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpsURLConnection.java index 462da45..28b61f2 100644 --- a/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpsURLConnection.java +++ b/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpsURLConnection.java @@ -150,7 +150,7 @@ public RecordableHttpsURLConnection(URL url, Cassette cassette, Mode mode) } private void cachedInteractionExistsOtherwiseError() throws VCRException { - if (this.cachedInteraction != null) { + if (this.cachedInteraction == null) { throw new VCRException("No matching interaction found."); } } diff --git a/src/test/java/HttpUrlConnectionTest.java b/src/test/java/HttpUrlConnectionTest.java index 1d1b282..3536492 100644 --- a/src/test/java/HttpUrlConnectionTest.java +++ b/src/test/java/HttpUrlConnectionTest.java @@ -541,26 +541,27 @@ public void testCachedInteractionDoesNotExist() throws Exception { Cassette cassette = TestUtils.getCassette("test_cached_interaction_does_not_exist"); cassette.erase(); // Erase cassette before recording + final String url = "https://google.com/path/to/endpoint"; + // make connection using Mode.Record RecordableHttpsURLConnection connection = (RecordableHttpsURLConnection) HttpClients.newClient(HttpClientType.HttpsUrlConnection, - FakeDataService.URL, cassette, Mode.Record); - // make data service using connection - FakeDataService.HttpsUrlConnection fakeDataService = new FakeDataService.HttpsUrlConnection(connection); - // make HTTP call with data service (record to cassette) - fakeDataService.getIPAddressDataRawResponse(); + url, cassette, Mode.Record); + // make HTTP call (record to cassette) + connection.connect(); Assert.assertTrue(cassette.numInteractions() > 0); // make sure we recorded something // Attempt to replay a cached interaction that does not exist - connection = (RecordableHttpsURLConnection) HttpClients.newClient(HttpClientType.HttpsUrlConnection, - FakeDataService.URL + "1", cassette, Mode.Replay); - // make data service using connection - fakeDataService = new FakeDataService.HttpsUrlConnection(connection); - // make HTTP call with data service (replay from cassette) + // need to use strict matching to ensure we don't match a different interaction + AdvancedSettings advancedSettings = new AdvancedSettings(); + advancedSettings.matchRules = MatchRules.strict(); + connection = (RecordableHttpsURLConnection) HttpClients.newClient(HttpClientType.HttpsUrlConnection, + url + "1", cassette, Mode.Replay, advancedSettings); + // make HTTP call (attempt replay from cassette) + connection.connect(); + // Attempt to pull data (e.g. body) from the response (via the input stream) // this throws a RuntimeException because of the way the exceptions are coalesced internally - FakeDataService.HttpsUrlConnection finalFakeDataService = fakeDataService; - fakeDataService.getIPAddressData(); - // Assert.assertThrows(RuntimeException.class, finalFakeDataService::getIPAddressData); + Assert.assertThrows(RuntimeException.class, connection::getInputStream); } } From aebf596c5ac584f0bc5746d0413cc9e34b0bc210 Mon Sep 17 00:00:00 2001 From: nwithan8 Date: Thu, 23 Mar 2023 16:30:42 -0600 Subject: [PATCH 3/3] - Address feedback --- src/main/java/com/easypost/easyvcr/MatchRules.java | 7 ++++++- .../RecordableHttpURLConnection.java | 5 +++++ .../RecordableHttpsURLConnection.java | 5 +++++ src/test/java/HttpUrlConnectionTest.java | 11 ++++++++++- 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/easypost/easyvcr/MatchRules.java b/src/main/java/com/easypost/easyvcr/MatchRules.java index 504341c..0c9c0f0 100644 --- a/src/main/java/com/easypost/easyvcr/MatchRules.java +++ b/src/main/java/com/easypost/easyvcr/MatchRules.java @@ -58,8 +58,13 @@ public MatchRules byBaseUrl() { return this; } + /** + * Extract the base URL from a URI. + * + * @param url The URI to extract the base URL from. + * @return The base URL. + */ private static String getBaseUrl(URI url) { - String baseUrl = url.getScheme() + "://" + url.getHost(); if (url.getPort() != -1) { baseUrl += ":" + url.getPort(); diff --git a/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpURLConnection.java b/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpURLConnection.java index 6b6fcc8..56c7ec2 100644 --- a/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpURLConnection.java +++ b/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpURLConnection.java @@ -144,6 +144,11 @@ public RecordableHttpURLConnection(URL url, Cassette cassette, Mode mode) this(url, cassette, mode, new AdvancedSettings()); } + /** + * Throws an exception if a matching interaction has not been cached. + * + * @throws VCRException If the interaction has not been cached. + */ private void cachedInteractionExistsOtherwiseError() throws VCRException { if (this.cachedInteraction == null) { throw new VCRException("No matching interaction found."); diff --git a/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpsURLConnection.java b/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpsURLConnection.java index 28b61f2..4db354f 100644 --- a/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpsURLConnection.java +++ b/src/main/java/com/easypost/easyvcr/clients/httpurlconnection/RecordableHttpsURLConnection.java @@ -149,6 +149,11 @@ public RecordableHttpsURLConnection(URL url, Cassette cassette, Mode mode) this(url, cassette, mode, new AdvancedSettings()); } + /** + * Throws an exception if a matching interaction has not been cached. + * + * @throws VCRException If the interaction has not been cached. + */ private void cachedInteractionExistsOtherwiseError() throws VCRException { if (this.cachedInteraction == null) { throw new VCRException("No matching interaction found."); diff --git a/src/test/java/HttpUrlConnectionTest.java b/src/test/java/HttpUrlConnectionTest.java index 3536492..d519df6 100644 --- a/src/test/java/HttpUrlConnectionTest.java +++ b/src/test/java/HttpUrlConnectionTest.java @@ -560,8 +560,17 @@ public void testCachedInteractionDoesNotExist() throws Exception { url + "1", cassette, Mode.Replay, advancedSettings); // make HTTP call (attempt replay from cassette) connection.connect(); + // Attempt to pull data (e.g. body) from the response (via the input stream) // this throws a RuntimeException because of the way the exceptions are coalesced internally - Assert.assertThrows(RuntimeException.class, connection::getInputStream); + try { + connection.getInputStream(); + // if we get here, the exception was not thrown as expected + Assert.fail(); + } catch (Exception e) { + Assert.assertTrue(e instanceof RuntimeException); + Assert.assertTrue(e.getCause() instanceof VCRException); + Assert.assertEquals(e.getCause().getMessage(), "No matching interaction found."); + } } }