Merge pull request #262 from ibi-group/feature/OTP-1330-trip-monitori…

…ng-tolerance

Tolerant monitored trip checking

README.md

@@ -266,6 +266,7 @@ The special E2E client settings should be defined in `env.yml`:
 | CONNECTED_DATA_PLATFORM_TRIP_HISTORY_UPLOAD_JOB_FREQUENCY_IN_MINUTES | integer | Optional | 5 | CDP trip history upload frequency. |
 | CONNECTED_DATA_PLATFORM_UPLOAD_BLANK_FILES | boolean | Optional | true | Whether to upload files where no records have been written. Defaults to true. |
 | DEFAULT_USAGE_PLAN_ID | string | Required | 123e45 | AWS API gateway default usage plan used when creating API keys for API users. |
+| MAXIMUM_MONITORED_TRIP_ITINERARY_CHECKS | integer | Optional | 3 | The maximum number of attempts to obtain a monitored trip itinerary. |
 | MAXIMUM_PERMITTED_MONITORED_TRIPS | integer | Optional | 5 | The maximum number of saved monitored trips. |
 | MONGO_DB_NAME | string | Required | otp_middleware | The name of the OTP Middleware Mongo DB. |
 | MONGO_HOST | string | Optional | localhost:27017 | Mongo host address. |

configurations/default/env.yml.tmp

@@ -104,5 +104,8 @@ US_RIDE_GWINNETT_BUS_OPERATOR_NOTIFIER_API_URL: https://bus.notifier.example.com
 US_RIDE_GWINNETT_BUS_OPERATOR_NOTIFIER_API_KEY: your-key
 US_RIDE_GWINNETT_BUS_OPERATOR_NOTIFIER_QUALIFYING_ROUTES: agency_id:route_id
 
+MAXIMUM_MONITORED_TRIP_ITINERARY_CHECKS: 3
+
 # The location for an OTP plan query request.
-PLAN_QUERY_RESOURCE_URI: https://plan.resource.com
+PLAN_QUERY_RESOURCE_URI: https://plan.resource.com
+

src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java

@@ -104,7 +104,7 @@ MonitoredTrip postCreateHook(MonitoredTrip monitoredTrip, Request req) {
      * monitored trip job, so return the trip as found in the database after the job completes.
      */
     private MonitoredTrip runCheckMonitoredTrip(MonitoredTrip monitoredTrip) throws Exception {
-        new CheckMonitoredTrip(monitoredTrip).run();
+        new CheckMonitoredTrip(monitoredTrip, false).run();
         return Persistence.monitoredTrips.getById(monitoredTrip.id);
     }
 

src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java

@@ -173,6 +173,11 @@ public class MonitoredTrip extends Model {
      */
     public boolean notifyAtLeadingInterval = true;
 
+    /**
+     * The number of attempts made to obtain a trip's itinerary from OTP which matches this trip.
+     */
+    public int attemptsToGetMatchingItinerary;
+
     public MonitoredTrip() {
     }
 

src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java

@@ -54,6 +54,9 @@ public class CheckMonitoredTrip implements Runnable {
 
     private final String OTP_UI_NAME = ConfigUtils.getConfigPropertyAsText("OTP_UI_NAME");
 
+    public static final int MAXIMUM_MONITORED_TRIP_ITINERARY_CHECKS =
+        ConfigUtils.getConfigPropertyAsInt("MAXIMUM_MONITORED_TRIP_ITINERARY_CHECKS", 3);
+
     private final String ACCOUNT_PATH = "/#/account";
 
     private final String TRIPS_PATH = ACCOUNT_PATH + "/trips";
@@ -113,16 +116,32 @@ public class CheckMonitoredTrip implements Runnable {
     /** The OTP Response provider */
     private Supplier<OtpResponse> otpResponseProvider;
 
+    private final boolean hasTolerantItineraryCheck;
+
     public CheckMonitoredTrip(MonitoredTrip trip) throws CloneNotSupportedException {
+        this(trip, true);
+    }
+
+    public CheckMonitoredTrip(MonitoredTrip trip, boolean hasTolerantItineraryCheck) throws CloneNotSupportedException {
         this.trip = trip;
+        this.hasTolerantItineraryCheck = hasTolerantItineraryCheck;
         previousJourneyState = trip.journeyState;
         journeyState = previousJourneyState.clone();
         previousMatchingItinerary = trip.journeyState.matchingItinerary;
         otpResponseProvider = this::getOtpResponse;
     }
 
     public CheckMonitoredTrip(MonitoredTrip trip, Supplier<OtpResponse> otpResponseProvider) throws CloneNotSupportedException {
-        this(trip);
+        this(trip, false);
+        this.otpResponseProvider = otpResponseProvider;
+    }
+
+    public CheckMonitoredTrip(
+        MonitoredTrip trip,
+        Supplier<OtpResponse> otpResponseProvider,
+        boolean hasTolerantItineraryCheck
+    ) throws CloneNotSupportedException {
+        this(trip, hasTolerantItineraryCheck);
         this.otpResponseProvider = otpResponseProvider;
     }
 
@@ -246,6 +265,7 @@ private boolean makeOTPRequestAndUpdateMatchingItineraryInternal() {
             if (ItineraryUtils.itinerariesMatch(trip.itinerary, candidateItinerary)) {
                 // matching itinerary found!
                 LOG.info("Found matching itinerary!");
+                trip.attemptsToGetMatchingItinerary = 0;
 
                 // Set the matching itinerary.
                 matchingItinerary = candidateItinerary;
@@ -283,40 +303,68 @@ private boolean makeOTPRequestAndUpdateMatchingItineraryInternal() {
         // If this point is reached, a matching itinerary was not found
         LOG.warn("No comparison itinerary found in otp response for trip");
 
-        // Check whether this trip should no longer ever be checked due to not having matching itineraries on any
-        // monitored day of the week. For trips that are only monitored on one day of the week, they could have been not
-        // possible for just that day, but could again be possible the next week. Therefore, this checks if the trip
-        // was not possible on all monitored days of the previous week and if so, it updates the journeyState to say
-        // that the trip is no longer possible.
-        boolean noMatchingItineraryFoundOnPreviousChecks =
-            !trip.itineraryExistence.isPossibleOnAtLeastOneMonitoredDayOfTheWeek(trip);
-        journeyState.tripStatus = noMatchingItineraryFoundOnPreviousChecks
-            ? TripStatus.NO_LONGER_POSSIBLE
-            : TripStatus.NEXT_TRIP_NOT_POSSIBLE;
+        if (hasReachedMaxItineraryChecks()) {
+            // Check whether this trip should no longer ever be checked due to not having matching itineraries on any
+            // monitored day of the week. For trips that are only monitored on one day of the week, they could have been not
+            // possible for just that day, but could again be possible the next week. Therefore, this checks if the trip
+            // was not possible on all monitored days of the previous week and if so, it updates the journeyState to say
+            // that the trip is no longer possible.
+            boolean noMatchingItineraryFoundOnPreviousChecks =
+                !trip.itineraryExistence.isPossibleOnAtLeastOneMonitoredDayOfTheWeek(trip);
+            journeyState.tripStatus = noMatchingItineraryFoundOnPreviousChecks
+                ? TripStatus.NO_LONGER_POSSIBLE
+                : TripStatus.NEXT_TRIP_NOT_POSSIBLE;
 
-        LOG.info(
-            noMatchingItineraryFoundOnPreviousChecks
-                ? "Trip checking has no more possible days to check, TRIP NO LONGER POSSIBLE!"
-                : "Trip is not possible today, will check again next week."
-        );
+            LOG.info(
+                noMatchingItineraryFoundOnPreviousChecks
+                    ? "Trip checking has no more possible days to check, TRIP NO LONGER POSSIBLE!"
+                    : "Trip is not possible today, will check again next week."
+            );
 
-        // update trip itinerary existence to reflect that trip was not possible on this day of the week
-        trip.itineraryExistence
-            .getResultForDayOfWeek(targetZonedDateTime.getDayOfWeek())
-            .handleInvalidDate(targetZonedDateTime);
-        updateMonitoredTrip();
+            // update trip itinerary existence to reflect that trip was not possible on this day of the week
+            trip.itineraryExistence
+                .getResultForDayOfWeek(targetZonedDateTime.getDayOfWeek())
+                .handleInvalidDate(targetZonedDateTime);
+            updateMonitoredTrip();
 
-        // send an appropriate notification if the trip is still possible on another day of the week, or if it is now
-        // not possible on any day of the week that the trip should be monitored
-        enqueueNotification(
-            TripMonitorNotification.createItineraryNotFoundNotification(
-                !noMatchingItineraryFoundOnPreviousChecks,
-                getOtpUserLocale()
-            )
-        );
+            // send an appropriate notification if the trip is still possible on another day of the week, or if it is now
+            // not possible on any day of the week that the trip should be monitored
+            enqueueNotification(
+                TripMonitorNotification.createItineraryNotFoundNotification(
+                    !noMatchingItineraryFoundOnPreviousChecks,
+                    getOtpUserLocale()
+                )
+            );
+        }
         return false;
     }
 
+    /**
+     * If the OTP response does not contain the expected itinerary, check the number of attempts made and decide whether
+     * to try again or stop checking.
+     */
+    private boolean hasReachedMaxItineraryChecks() {
+        if (!hasTolerantItineraryCheck) {
+            LOG.info("Tolerant itinerary check disabled.");
+            return true;
+        }
+        trip.attemptsToGetMatchingItinerary++;
+        LOG.info(
+            "Attempt {} of {} to get matching itinerary.",
+            trip.attemptsToGetMatchingItinerary,
+            MAXIMUM_MONITORED_TRIP_ITINERARY_CHECKS
+        );
+        if (trip.attemptsToGetMatchingItinerary < MAXIMUM_MONITORED_TRIP_ITINERARY_CHECKS) {
+            if (Persistence.monitoredTrips.getById(trip.id) == null) {
+                // Trip has been deleted. Continue as if maximum itinerary checks have been reached.
+                return true;
+            }
+            Persistence.monitoredTrips.replace(trip.id, trip);
+            return false;
+        }
+        return true;
+    }
+
     /** Default implementation for OtpResponse provider that actually invokes the OTP server. */
     private OtpResponse getOtpResponse() {
         return OtpDispatcher.sendOtpRequestWithErrorHandling(getQueryParamsForTargetZonedDateTime());

src/main/resources/env.schema.json

@@ -128,6 +128,11 @@
       "examples": ["123e45"],
       "description": "AWS API gateway default usage plan used when creating API keys for API users."
     },
+    "MAXIMUM_MONITORED_TRIP_ITINERARY_CHECKS": {
+      "type": "integer",
+      "examples": ["3"],
+      "description": "The maximum number of attempts to obtain a monitored trip itinerary."
+    },
     "MAXIMUM_PERMITTED_MONITORED_TRIPS": {
       "type": "integer",
       "examples": ["5"],