From 08883d0acb93a532619fbd5eb777722ab78e0d76 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 11 Sep 2020 17:12:53 -0400 Subject: [PATCH 1/2] fix(OtpDispatcherResponse): Prevent conversion of non-plan OTP responses to POJO. --- .../middleware/otp/OtpDispatcherResponse.java | 10 ++++++- .../opentripplanner/middleware/TestUtils.java | 6 ++-- .../otp/OtpDispatcherResponseTest.java | 30 +++++++++++++++++++ .../jobs/CheckMonitoredTripTest.java | 3 +- .../middleware/otp/parkRideResponse.json | 1 + 5 files changed, 45 insertions(+), 5 deletions(-) create mode 100644 src/test/java/org/opentripplanner/middleware/otp/OtpDispatcherResponseTest.java create mode 100644 src/test/resources/org/opentripplanner/middleware/otp/parkRideResponse.json diff --git a/src/main/java/org/opentripplanner/middleware/otp/OtpDispatcherResponse.java b/src/main/java/org/opentripplanner/middleware/otp/OtpDispatcherResponse.java index c47b8e5fe..c0a20f919 100644 --- a/src/main/java/org/opentripplanner/middleware/otp/OtpDispatcherResponse.java +++ b/src/main/java/org/opentripplanner/middleware/otp/OtpDispatcherResponse.java @@ -11,6 +11,8 @@ import java.net.URI; import java.net.http.HttpResponse; +import static org.opentripplanner.middleware.otp.OtpDispatcher.OTP_PLAN_ENDPOINT; + /** * An OTP dispatcher response represents the status code and body return from a call to an OTP end point e.g. plan */ @@ -66,10 +68,16 @@ public void setResponse(Response response) { @Override public String toString() { + // Only include the plan response if requestUri.path ends with OTP_PLAN_ENDPOINT. + // Without this check, we are sending valid responses from non-plan OTP endpoints to Bugsnag as errors. + String planResponse = requestUri.getPath().endsWith(OTP_PLAN_ENDPOINT) + ? ", response=" + getResponse() + : ""; + return "OtpDispatcherResponse{" + "statusCode=" + statusCode + ", responseBody='" + responseBody + '\'' + - ", response=" + getResponse() + + planResponse + '}'; } diff --git a/src/test/java/org/opentripplanner/middleware/TestUtils.java b/src/test/java/org/opentripplanner/middleware/TestUtils.java index 4627bc267..434098c29 100644 --- a/src/test/java/org/opentripplanner/middleware/TestUtils.java +++ b/src/test/java/org/opentripplanner/middleware/TestUtils.java @@ -30,6 +30,7 @@ public class TestUtils { private static final Logger LOG = LoggerFactory.getLogger(TestUtils.class); + public static final String TEST_RESOURCE_PATH = "src/test/resources/org/opentripplanner/middleware/"; /** * Prevents the mock OTP server being initialized more than once @@ -51,7 +52,7 @@ public static boolean getBooleanEnvVar(String var) { public static <T> T getResourceFileContentsAsJSON (String resourcePathName, Class<T> clazz) throws IOException { return FileUtils.getFileContentsAsJSON( - String.format("src/test/resources/org/opentripplanner/middleware/%s", resourcePathName), + TEST_RESOURCE_PATH + resourcePathName, clazz ); } @@ -148,10 +149,9 @@ static void mockOtpServer() { * Mock an OTP server plan response by provide a static response from file. */ private static String mockOtpPlanResponse(Request request, Response response) throws IOException { - final String filePath = "src/test/resources/org/opentripplanner/middleware/"; OtpDispatcherResponse otpDispatcherResponse = new OtpDispatcherResponse(); otpDispatcherResponse.statusCode = HttpStatus.OK_200; - otpDispatcherResponse.responseBody = FileUtils.getFileContents(filePath + "planResponse.json"); + otpDispatcherResponse.responseBody = FileUtils.getFileContents(TEST_RESOURCE_PATH + "planResponse.json"); response.type(APPLICATION_JSON); response.status(otpDispatcherResponse.statusCode); diff --git a/src/test/java/org/opentripplanner/middleware/otp/OtpDispatcherResponseTest.java b/src/test/java/org/opentripplanner/middleware/otp/OtpDispatcherResponseTest.java new file mode 100644 index 000000000..e2fd3b965 --- /dev/null +++ b/src/test/java/org/opentripplanner/middleware/otp/OtpDispatcherResponseTest.java @@ -0,0 +1,30 @@ +package org.opentripplanner.middleware.otp; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.opentripplanner.middleware.utils.FileUtils; + +import java.io.IOException; + +import static org.opentripplanner.middleware.TestUtils.TEST_RESOURCE_PATH; +import static org.opentripplanner.middleware.otp.OtpDispatcher.OTP_PLAN_ENDPOINT; + +public class OtpDispatcherResponseTest { + @Test + public void toStringShouldExludeResponseFieldIfNotCallingPlan() throws IOException { + String mockParkRideResponse = FileUtils.getFileContents( + TEST_RESOURCE_PATH + "otp/parkRideResponse.json" + ); + + + // The call below creates a request URI that is "http://test.com", + // which is not the OTP /plan endpoint, but we still assert that too. + OtpDispatcherResponse dispatcherResponse = new OtpDispatcherResponse(mockParkRideResponse); + Assertions.assertFalse(dispatcherResponse.requestUri.getPath().endsWith(OTP_PLAN_ENDPOINT)); + + // Expected string does not include the response={} field. + String expected = "OtpDispatcherResponse{statusCode=200, responseBody='" + mockParkRideResponse + "'}"; + + Assertions.assertEquals(expected, dispatcherResponse.toString()); + } +} diff --git a/src/test/java/org/opentripplanner/middleware/tripMonitor/jobs/CheckMonitoredTripTest.java b/src/test/java/org/opentripplanner/middleware/tripMonitor/jobs/CheckMonitoredTripTest.java index 3d9cc19af..4295b725e 100644 --- a/src/test/java/org/opentripplanner/middleware/tripMonitor/jobs/CheckMonitoredTripTest.java +++ b/src/test/java/org/opentripplanner/middleware/tripMonitor/jobs/CheckMonitoredTripTest.java @@ -33,6 +33,7 @@ import static com.mongodb.client.model.Filters.eq; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assumptions.assumeTrue; +import static org.opentripplanner.middleware.TestUtils.TEST_RESOURCE_PATH; import static org.opentripplanner.middleware.TestUtils.getBooleanEnvVar; import static org.opentripplanner.middleware.persistence.PersistenceUtil.createMonitoredTrip; import static org.opentripplanner.middleware.persistence.PersistenceUtil.createUser; @@ -64,7 +65,7 @@ public class CheckMonitoredTripTest extends OtpMiddlewareTest { public static void setup() throws IOException { user = createUser("user@example.com"); mockResponse = FileUtils.getFileContents( - "src/test/resources/org/opentripplanner/middleware/persistence/planResponse.json" + TEST_RESOURCE_PATH + "persistence/planResponse.json" ); otpDispatcherResponse = new OtpDispatcherResponse(mockResponse); } diff --git a/src/test/resources/org/opentripplanner/middleware/otp/parkRideResponse.json b/src/test/resources/org/opentripplanner/middleware/otp/parkRideResponse.json new file mode 100644 index 000000000..c756308ca --- /dev/null +++ b/src/test/resources/org/opentripplanner/middleware/otp/parkRideResponse.json @@ -0,0 +1 @@ +[{"name":"P+R","x":-81.36548775,"y":28.4528734},{"name":"P+R","x":-81.274944,"y":28.911900600000003},{"name":"P+R","x":-81.7390652,"y":28.540740550000002},{"name":"P+R","x":-81.13372815,"y":28.561692100000002}] \ No newline at end of file From a2a319a156fa0a63983eb827143ca7e5477d8393 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 15 Sep 2020 09:26:29 -0400 Subject: [PATCH 2/2] test(OtpDispatcherResponseTest): Fix typo in test name. --- .../middleware/otp/OtpDispatcherResponseTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/opentripplanner/middleware/otp/OtpDispatcherResponseTest.java b/src/test/java/org/opentripplanner/middleware/otp/OtpDispatcherResponseTest.java index e2fd3b965..ebcf214c5 100644 --- a/src/test/java/org/opentripplanner/middleware/otp/OtpDispatcherResponseTest.java +++ b/src/test/java/org/opentripplanner/middleware/otp/OtpDispatcherResponseTest.java @@ -11,7 +11,7 @@ public class OtpDispatcherResponseTest { @Test - public void toStringShouldExludeResponseFieldIfNotCallingPlan() throws IOException { + public void toStringShouldExcludeResponseFieldIfNotCallingPlan() throws IOException { String mockParkRideResponse = FileUtils.getFileContents( TEST_RESOURCE_PATH + "otp/parkRideResponse.json" );