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"
         );