From 04ea3d73484a0ee25c4fefb354b01bd6380aabc8 Mon Sep 17 00:00:00 2001 From: Nate Harris Date: Thu, 20 Oct 2022 10:53:58 -0600 Subject: [PATCH] Bug fixes (#16) - Fix bug where HTTP error data is not being stored as expected in cassette, causing empty error stream on replay - Fix bug with expiration time frames not handling non-never/forever enums correctly (throwing NullPointerException) --- CHANGELOG.md | 10 +++ VERSION | 2 +- pom.xml | 2 +- .../java/com/easypost/easyvcr/TimeFrame.java | 7 ++ ...HttpUrlConnectionInteractionConverter.java | 7 +- .../ExpirationActionExtensions.java | 2 +- .../easyvcr/requestelements/Response.java | 23 ------ src/test/java/HttpUrlConnectionTest.java | 76 +++++++++++++++---- 8 files changed, 83 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a47f09b..9685472 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # CHANGELOG +## v0.4.2 (2022-10-20) + +- Fix a bug where the error data of a bad HTTP request (4xx or 5xx) was not stored as expected in cassettes, causing + empty error streams on replay. + - Error data for a bad HTTP request is now stored as the "body" in the cassette just like a good HTTP request + would, rather than needlessly stored in a separate "error" key. This more closely matches the behavior of EasyVCR C#. + - This is a breaking change for previously-recorded "error" cassettes, which will no longer replay as expected and + will need to be re-recorded (although likely never worked as expected in the first place). +- Fix a bug where using any expiration time frame other than "forever" and "never" would throw a NullPointerException. + ## v0.4.1 (2022-10-19) - Fix a bug where the error stream of a bad HTTP request (4xx or 5xx) was not properly recreated on replay. diff --git a/VERSION b/VERSION index 267577d..2b7c5ae 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.4.1 +0.4.2 diff --git a/pom.xml b/pom.xml index a9801db..c564710 100644 --- a/pom.xml +++ b/pom.xml @@ -7,7 +7,7 @@ com.easypost easyvcr - 0.4.1 + 0.4.2 jar com.easypost:easyvcr diff --git a/src/main/java/com/easypost/easyvcr/TimeFrame.java b/src/main/java/com/easypost/easyvcr/TimeFrame.java index ba6bd1d..7b9d947 100644 --- a/src/main/java/com/easypost/easyvcr/TimeFrame.java +++ b/src/main/java/com/easypost/easyvcr/TimeFrame.java @@ -89,12 +89,19 @@ public boolean hasLapsed(long fromTimeEpochTimestamp) { * @return Starting time plus this time frame. */ private Instant timePlusFrame(Instant fromTime) { + // We need to do a null check here. The "default" case in the switch statement below doesn't handle null enums. + if (commonTimeFrame == null) { // No common time frame was used + return fromTime.plus(days, ChronoUnit.DAYS).plus(hours, ChronoUnit.HOURS).plus(minutes, ChronoUnit.MINUTES) + .plus(seconds, ChronoUnit.SECONDS); + } switch (commonTimeFrame) { case Forever: return Instant.MAX; // will always been in the future case Never: return Instant.MIN; // will always been in the past default: + // We should never get here, since there should always either be an accounted-for enum, + // or a null value (handled above). return fromTime.plus(days, ChronoUnit.DAYS).plus(hours, ChronoUnit.HOURS) .plus(minutes, ChronoUnit.MINUTES).plus(seconds, ChronoUnit.SECONDS); } diff --git a/src/main/java/com/easypost/easyvcr/interactionconverters/HttpUrlConnectionInteractionConverter.java b/src/main/java/com/easypost/easyvcr/interactionconverters/HttpUrlConnectionInteractionConverter.java index 1bd1131..c7b164e 100644 --- a/src/main/java/com/easypost/easyvcr/interactionconverters/HttpUrlConnectionInteractionConverter.java +++ b/src/main/java/com/easypost/easyvcr/interactionconverters/HttpUrlConnectionInteractionConverter.java @@ -80,11 +80,10 @@ public ResponseAndTime createRecordedResponse(HttpURLConnection connection, Cens String uriString = connection.getURL().toString(); Map> headers = connection.getHeaderFields(); String body = null; - String errors = null; try { body = readFromInputStream(connection.getInputStream()); } catch (NullPointerException | IOException ignored) { // nothing in body if bad status code from server - errors = readFromInputStream(connection.getErrorStream()); + body = readFromInputStream(connection.getErrorStream()); } // apply censors @@ -101,10 +100,6 @@ public ResponseAndTime createRecordedResponse(HttpURLConnection connection, Cens body = censors.applyBodyParameterCensors(body); response.setBody(body); } - if (errors != null) { - errors = censors.applyBodyParameterCensors(errors); - response.setErrors(errors); - } return new ResponseAndTime(response, milliseconds); } catch (URISyntaxException | IOException ignored) { diff --git a/src/main/java/com/easypost/easyvcr/internalutilities/ExpirationActionExtensions.java b/src/main/java/com/easypost/easyvcr/internalutilities/ExpirationActionExtensions.java index 709056d..141c3d0 100644 --- a/src/main/java/com/easypost/easyvcr/internalutilities/ExpirationActionExtensions.java +++ b/src/main/java/com/easypost/easyvcr/internalutilities/ExpirationActionExtensions.java @@ -16,7 +16,7 @@ public static void checkCompatibleSettings(ExpirationActions action, Mode mode) throws RecordingExpirationException { if (action == ExpirationActions.RecordAgain && mode == Mode.Replay) { throw new RecordingExpirationException( - "Cannot use the Record_Again expiration action in combination with Replay mode."); + "Cannot use the RecordAgain expiration action in combination with Replay mode."); } } } diff --git a/src/main/java/com/easypost/easyvcr/requestelements/Response.java b/src/main/java/com/easypost/easyvcr/requestelements/Response.java index 00fcd01..cd13c49 100644 --- a/src/main/java/com/easypost/easyvcr/requestelements/Response.java +++ b/src/main/java/com/easypost/easyvcr/requestelements/Response.java @@ -47,11 +47,6 @@ public final class Response extends HttpElement { */ private Status status; - /** - * The errors of the response. - */ - private String errors; - /** * The URI of the response. */ @@ -399,24 +394,6 @@ public void setStatus(Status status) { this.status = status; } - /** - * Returns the errors of the response. - * - * @return the errors of the response - */ - public String getErrors() { - return this.errors; - } - - /** - * Sets the errors of the response. - * - * @param errors the errors of the response - */ - public void setErrors(String errors) { - this.errors = errors; - } - /** * Returns the URI of the response. * diff --git a/src/test/java/HttpUrlConnectionTest.java b/src/test/java/HttpUrlConnectionTest.java index 003e8fd..c6e12a3 100644 --- a/src/test/java/HttpUrlConnectionTest.java +++ b/src/test/java/HttpUrlConnectionTest.java @@ -27,9 +27,10 @@ import static com.easypost.easyvcr.internalutilities.Tools.readFromInputStream; -public class HttpUrlConnectionTest { +public class HttpUrlConnectionTest { - private static FakeDataService.IPAddressData getIPAddressDataRequest(Cassette cassette, Mode mode) throws Exception { + private static FakeDataService.IPAddressData getIPAddressDataRequest(Cassette cassette, Mode mode) + throws Exception { RecordableHttpsURLConnection connection = TestUtils.getSimpleHttpsURLConnection(cassette.name, mode, null); FakeDataService.HttpsUrlConnection fakeDataService = new FakeDataService.HttpsUrlConnection(connection); @@ -42,7 +43,8 @@ public void testPOSTRequest() throws Exception { AdvancedSettings advancedSettings = new AdvancedSettings(); advancedSettings.matchRules = new MatchRules().byMethod().byBody().byFullUrl(); RecordableHttpsURLConnection connection = - TestUtils.getSimpleHttpsURLConnection("https://www.google.com", "test_post_request", Mode.Record, advancedSettings); + TestUtils.getSimpleHttpsURLConnection("https://www.google.com", "test_post_request", Mode.Record, + advancedSettings); String jsonInputString = "{'name': 'Upendra', 'job': 'Programmer'}"; connection.setDoOutput(true); connection.setRequestMethod("POST"); @@ -70,7 +72,8 @@ public void testNonJsonDataWithCensors() throws Exception { advancedSettings.matchRules = new MatchRules().byMethod().byBody().byFullUrl(); RecordableHttpsURLConnection connection = - TestUtils.getSimpleHttpsURLConnection("https://www.google.com", "test_non_json", Mode.Record, advancedSettings); + TestUtils.getSimpleHttpsURLConnection("https://www.google.com", "test_non_json", Mode.Record, + advancedSettings); String jsonInputString = "{'name': 'Upendra', 'job': 'Programmer'}"; connection.setDoOutput(true); connection.setRequestMethod("POST"); @@ -164,7 +167,8 @@ public void testInteractionElements() throws Exception { // Most elements of a VCR request are black-boxed, so we can't test them here. // Instead, we can get the recreated HttpResponseMessage and check the details. - RecordableHttpsURLConnection response = (RecordableHttpsURLConnection) fakeDataService.getIPAddressDataRawResponse(); + RecordableHttpsURLConnection response = + (RecordableHttpsURLConnection) fakeDataService.getIPAddressDataRawResponse(); Assert.assertNotNull(response); } @@ -193,7 +197,8 @@ public void testCensors() throws Exception { connection = (RecordableHttpsURLConnection) HttpClients.newClient(HttpClientType.HttpsUrlConnection, FakeDataService.URL, cassette, Mode.Replay, advancedSettings); fakeDataService = new FakeDataService.HttpsUrlConnection(connection); - RecordableHttpsURLConnection response = (RecordableHttpsURLConnection) fakeDataService.getIPAddressDataRawResponse(); + RecordableHttpsURLConnection response = + (RecordableHttpsURLConnection) fakeDataService.getIPAddressDataRawResponse(); // check that the replayed response contains the censored header Assert.assertNotNull(response); @@ -221,7 +226,8 @@ public void testMatchSettings() throws Exception { connection.setRequestProperty("X-Custom-Header", "custom-value"); // add custom header to request, shouldn't matter when matching by default rules fakeDataService = new FakeDataService.HttpsUrlConnection(connection); - RecordableHttpsURLConnection response = (RecordableHttpsURLConnection) fakeDataService.getIPAddressDataRawResponse(); + RecordableHttpsURLConnection response = + (RecordableHttpsURLConnection) fakeDataService.getIPAddressDataRawResponse(); Assert.assertNotNull(response); // replay cassette with custom match rules, should not find a match because request is different (throw exception) @@ -290,7 +296,8 @@ public void testIgnoreElementsFailMatch() throws URISyntaxException, IOException String bodyData2 = "{'name': 'NewName', 'job': 'Programmer'}"; // record baseline request first - RecordableHttpsURLConnection connection = HttpClients.newHttpsURLConnection(FakeDataService.URL, cassette, Mode.Record); + RecordableHttpsURLConnection connection = + HttpClients.newHttpsURLConnection(FakeDataService.URL, cassette, Mode.Record); connection.setDoOutput(true); connection.setRequestMethod("POST"); // use bodyData1 to make request @@ -339,7 +346,8 @@ public void testIgnoreElementsPassMatch() throws URISyntaxException, IOException String bodyData2 = "{'name': 'NewName', 'job': 'Programmer'}"; // record baseline request first - RecordableHttpsURLConnection connection = HttpClients.newHttpsURLConnection(FakeDataService.URL, cassette, Mode.Record); + RecordableHttpsURLConnection connection = + HttpClients.newHttpsURLConnection(FakeDataService.URL, cassette, Mode.Record); connection.setDoOutput(true); connection.setRequestMethod("POST"); // use bodyData1 to make request @@ -383,7 +391,7 @@ public void testIgnoreElementsPassMatch() throws URISyntaxException, IOException } @Test - public void testExpirationSettings() throws Exception { + public void testExpirationSettingsCommonTimeFrame() throws Exception { Cassette cassette = TestUtils.getCassette("test_expiration_settings"); cassette.erase(); // Erase cassette before recording @@ -398,7 +406,8 @@ public void testExpirationSettings() throws Exception { connection = (RecordableHttpsURLConnection) HttpClients.newClient(HttpClientType.HttpsUrlConnection, FakeDataService.URL, cassette, Mode.Replay); fakeDataService = new FakeDataService.HttpsUrlConnection(connection); - RecordableHttpsURLConnection response = (RecordableHttpsURLConnection) fakeDataService.getIPAddressDataRawResponse(); + RecordableHttpsURLConnection response = + (RecordableHttpsURLConnection) fakeDataService.getIPAddressDataRawResponse(); Assert.assertNotNull(response); // replay cassette with custom expiration rules, should not find a match because recording is expired (throw exception) @@ -416,10 +425,49 @@ public void testExpirationSettings() throws Exception { // replay cassette with bad expiration rules, should throw an exception because settings are bad advancedSettings = new AdvancedSettings(); advancedSettings.timeFrame = TimeFrame.never(); - advancedSettings.whenExpired = ExpirationActions.RecordAgain; // invalid settings for replay mode, should throw exception + advancedSettings.whenExpired = + ExpirationActions.RecordAgain; // invalid settings for replay mode, should throw exception AdvancedSettings finalAdvancedSettings = advancedSettings; - Assert.assertThrows(RecordingExpirationException.class, () -> HttpClients.newClient(HttpClientType.HttpsUrlConnection, - FakeDataService.URL, cassette, Mode.Replay, finalAdvancedSettings)); + Assert.assertThrows(RecordingExpirationException.class, + () -> HttpClients.newClient(HttpClientType.HttpsUrlConnection, FakeDataService.URL, cassette, + Mode.Replay, finalAdvancedSettings)); + } + + @Test + public void testExpirationSettingsCustomTimeFrame() throws Exception { + Cassette cassette = TestUtils.getCassette("test_expiration_settings"); + cassette.erase(); // Erase cassette before recording + + // record cassette first + RecordableHttpsURLConnection connection = + (RecordableHttpsURLConnection) HttpClients.newClient(HttpClientType.HttpsUrlConnection, + FakeDataService.URL, cassette, Mode.Record); + FakeDataService.HttpsUrlConnection fakeDataService = new FakeDataService.HttpsUrlConnection(connection); + fakeDataService.getIPAddressDataRawResponse(); + + // Custom expiration rules + AdvancedSettings advancedSettings = new AdvancedSettings(); + advancedSettings.timeFrame = new TimeFrame(1, 0, 0, 0); // 1 day from now + advancedSettings.whenExpired = ExpirationActions.ThrowException; // throw exception when recording is expired + + // replay cassette with custom expiration rules, should find a match + connection = (RecordableHttpsURLConnection) HttpClients.newClient(HttpClientType.HttpsUrlConnection, + FakeDataService.URL, cassette, Mode.Replay, advancedSettings); + fakeDataService = new FakeDataService.HttpsUrlConnection(connection); + RecordableHttpsURLConnection response = + (RecordableHttpsURLConnection) fakeDataService.getIPAddressDataRawResponse(); + Assert.assertNotNull(response); + + // Change expiration rules + advancedSettings.timeFrame = new TimeFrame(-1, 0, 0, 0); // 1 day ago + + // replay cassette with custom expiration rules, should not find a match because recording is expired (throw exception) + connection = (RecordableHttpsURLConnection) HttpClients.newClient(HttpClientType.HttpsUrlConnection, + FakeDataService.URL, cassette, Mode.Replay, advancedSettings); + fakeDataService = new FakeDataService.HttpsUrlConnection(connection); + FakeDataService.HttpsUrlConnection finalFakeDataService = fakeDataService; + // this throws a RuntimeException rather than a RecordingExpirationException because the exceptions are coalesced internally + Assert.assertThrows(Exception.class, () -> finalFakeDataService.getIPAddressData()); } @Test