Merge pull request #74 from ibi-group/prevent-nonplan-pojo-errors

Prevent conversion of non-plan OTP responses to POJO

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 +
                 '}';
     }
 

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

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 toStringShouldExcludeResponseFieldIfNotCallingPlan() 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());
+    }
+}

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

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}]