refactor(trip monitor): refactor trip monitor timing

Refs #70
ibi-group · Sep 16, 2020 · 38e9557 · 38e9557
1 parent c293bdc
commit 38e9557
Show file tree

Hide file tree

Showing 17 changed files with 436 additions and 248 deletions.
diff --git a/configurations/test/env.yml b/configurations/test/env.yml
@@ -22,9 +22,9 @@ MONGO_DB_NAME: otp_middleware_test
 #MONGO_PROTOCOL: mongodb+srv
 #MONGO_USER: user
 
-OTP_API_ROOT: http://otp-server.example.com/otp
-OTP_SERVER: http://otp-server.example.com/otp
-OTP_SERVER_PLAN_END_POINT: /routers/default/plan
+OTP_API_ROOT: http://localhost:8080/otp
+OTP_SERVER: http://localhost:8080/otp
+OTP_PLAN_ENDPOINT: /routers/default/plan
 MAXIMUM_PERMITTED_MONITORED_TRIPS: 5
 
 # Uncomment and provide info for running disabled notification tests.

diff --git a/src/main/java/org/opentripplanner/middleware/controllers/api/ApiController.java b/src/main/java/org/opentripplanner/middleware/controllers/api/ApiController.java
@@ -329,7 +329,7 @@ private T createOrUpdate(Request req, Response res) {
                     logMessageAndHalt(req, HttpStatus.FORBIDDEN_403, String.format("Requesting user not authorized to update %s.", className));
                 }
                 // Update last updated value.
-                object.lastUpdated = new Date();
+                object.lastUpdated = DateTimeUtils.nowAsDate();
                 // Pin the date created to pre-existing value.
                 object.dateCreated = preExistingObject.dateCreated;
                 // Validate that ID in JSON body matches ID param. TODO add test

diff --git a/src/main/java/org/opentripplanner/middleware/controllers/api/OtpRequestProcessor.java b/src/main/java/org/opentripplanner/middleware/controllers/api/OtpRequestProcessor.java
@@ -9,7 +9,7 @@
 import org.opentripplanner.middleware.models.TripSummary;
 import org.opentripplanner.middleware.otp.OtpDispatcher;
 import org.opentripplanner.middleware.otp.OtpDispatcherResponse;
-import org.opentripplanner.middleware.otp.response.Response;
+import org.opentripplanner.middleware.otp.response.OtpResponse;
 import org.opentripplanner.middleware.persistence.Persistence;
 import org.opentripplanner.middleware.utils.DateTimeUtils;
 import org.opentripplanner.middleware.utils.HttpUtils;
@@ -25,7 +25,6 @@
 import static javax.ws.rs.core.MediaType.APPLICATION_XML;
 import static org.opentripplanner.middleware.auth.Auth0Connection.isAuthHeaderPresent;
 import static org.opentripplanner.middleware.utils.ConfigUtils.getConfigPropertyAsText;
-import static org.opentripplanner.middleware.otp.OtpDispatcher.OTP_API_ROOT;
 import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt;
 
 /**
@@ -139,7 +138,7 @@ private static void handlePlanTripResponse(Request request, OtpDispatcherRespons
         if (!storeTripHistory) {
             LOG.debug("User does not want trip history stored");
         } else {
-            Response otpResponse = otpDispatcherResponse.getResponse();
+            OtpResponse otpResponse = otpDispatcherResponse.getResponse();
             if (otpResponse == null) {
                 LOG.warn("OTP response is null, cannot save trip history for user!");
             } else {

diff --git a/src/main/java/org/opentripplanner/middleware/models/JourneyState.java b/src/main/java/org/opentripplanner/middleware/models/JourneyState.java
@@ -35,11 +35,11 @@ public JourneyState(MonitoredTrip monitoredTrip) {
     /**
      * Timestamp checking the last time a journey was checked.
      */
-    public long lastChecked;
+    public long lastCheckedMillis;
 
     public Set<TripMonitorNotification> lastNotifications = new HashSet<>();
 
-    public long lastNotificationTime;
+    public long lastNotificationTimeMillis;
 
     public Set<LocalizedAlert> lastSeenAlerts = new HashSet<>();
 
@@ -67,13 +67,13 @@ public JourneyState(MonitoredTrip monitoredTrip) {
      */
     public void update(CheckMonitoredTrip checkMonitoredTripJob) {
         targetDate = checkMonitoredTripJob.targetDate;
-        lastChecked = DateTimeUtils.currentTimeMillis();
+        lastCheckedMillis = DateTimeUtils.currentTimeMillis();
         matchingItinerary = checkMonitoredTripJob.matchingItinerary;
         lastDepartureDelay = checkMonitoredTripJob.departureDelay;
         lastArrivalDelay = checkMonitoredTripJob.arrivalDelay;
         // Update notification time if notification successfully sent.
-        if (checkMonitoredTripJob.notificationTimestamp != -1) {
-            lastNotificationTime = checkMonitoredTripJob.notificationTimestamp;
+        if (checkMonitoredTripJob.notificationTimestampMillis != -1) {
+            lastNotificationTimeMillis = checkMonitoredTripJob.notificationTimestampMillis;
         }
         Persistence.journeyStates.replace(this.id, this);
     }

diff --git a/src/main/java/org/opentripplanner/middleware/models/Model.java b/src/main/java/org/opentripplanner/middleware/models/Model.java
@@ -1,6 +1,7 @@
 package org.opentripplanner.middleware.models;
 
 import org.opentripplanner.middleware.auth.Auth0UserProfile;
+import org.opentripplanner.middleware.utils.DateTimeUtils;
 
 import java.io.Serializable;
 import java.util.Date;
@@ -16,8 +17,8 @@ public Model () {
         // This autogenerates an ID
         // this is OK for dump/restore, because the ID will simply be overridden
         this.id = UUID.randomUUID().toString();
-        this.lastUpdated = new Date();
-        this.dateCreated = new Date();
+        this.lastUpdated = DateTimeUtils.nowAsDate();
+        this.dateCreated = DateTimeUtils.nowAsDate();
     }
     public String id;
     public Date lastUpdated;

diff --git a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java
@@ -2,6 +2,7 @@
 
 import org.apache.http.NameValuePair;
 import org.apache.http.client.utils.URLEncodedUtils;
+import org.bson.codecs.pojo.annotations.BsonIgnore;
 import org.bson.conversions.Bson;
 import org.opentripplanner.middleware.auth.Auth0UserProfile;
 import org.opentripplanner.middleware.auth.Permission;
@@ -136,17 +137,22 @@ public class MonitoredTrip extends Model {
      */
     public boolean notifyOnItineraryChange = true;
 
-    private transient JourneyState journeyState;
-
-    private transient List<NameValuePair> parsedParams;
-
     public MonitoredTrip() {
     }
 
-    public MonitoredTrip(OtpDispatcherResponse otpDispatcherResponse) {
+    public MonitoredTrip(OtpDispatcherResponse otpDispatcherResponse) throws URISyntaxException {
         queryParams = otpDispatcherResponse.requestUri.getQuery();
         TripPlan plan = otpDispatcherResponse.getResponse().plan;
         itinerary = plan.itineraries.get(0);
+
+        // extract trip time from parsed params
+        List<NameValuePair> parsedParams = getParsedParams();
+        for (NameValuePair parsedParam : parsedParams) {
+            if (parsedParam.getName().equals("time")) {
+                tripTime = parsedParam.getValue();
+                break;
+            }
+        }
         initializeFromItinerary();
     }
 
@@ -250,10 +256,8 @@ private Bson tripIdFilter() {
      * Get the journey state for this trip.
      */
     public JourneyState retrieveJourneyState() {
-        // first return the journeyState for this trip if it has already been fetched
-        if (journeyState != null) return journeyState;
-        // hasn't been fetched, attempt to retrieve from the db
-        journeyState = Persistence.journeyStates.getOneFiltered(tripIdFilter());
+        // attempt to retrieve from the db
+        JourneyState journeyState = Persistence.journeyStates.getOneFiltered(tripIdFilter());
         // If journey state does not exist, create and persist.
         if (journeyState == null) {
             journeyState = new JourneyState(this);
@@ -300,19 +304,14 @@ public boolean delete() {
     }
 
     public List<NameValuePair> getParsedParams() throws URISyntaxException {
-        // use the transient value of parsedParams if it is available
-        if (parsedParams != null) return parsedParams;
-
-        // need to parse the params
-        parsedParams = URLEncodedUtils.parse(
+        return URLEncodedUtils.parse(
             new URI(String.format("http://example.com/%s", queryParams)),
             UTF_8
         );
-        return parsedParams;
     }
 
     public boolean isArriveBy() throws URISyntaxException {
-        for (NameValuePair param : parsedParams) {
+        for (NameValuePair param : getParsedParams()) {
             if (param.getName().equals("arriveBy")) {
                 return param.getValue().equals("true");
             }
@@ -328,6 +327,7 @@ public boolean isArriveBy() throws URISyntaxException {
      * use the local time at the destination. Therefore, this method will return the timezone at the destination if this
      * trip is an arriveBy trip, or the timezone at the origin if the trip is a depart at trip.
      */
+    @BsonIgnore
     public ZoneId getTimezoneForTargetLocation() throws URISyntaxException {
         double lat, lon;
         if (isArriveBy()) {
@@ -353,13 +353,15 @@ public ZoneId getTimezoneForTargetLocation() throws URISyntaxException {
     /**
      * Returns the target hour of the day that the trip is either departing at or arriving by
      */
+    @BsonIgnore
     public int getHour() {
         return Integer.valueOf(tripTime.split(":")[0]);
     }
 
     /**
      * Returns the target minute of the hour that the trip is either departing at or arriving by
      */
+    @BsonIgnore
     public int getMinute() {
         return Integer.valueOf(tripTime.split(":")[1]);
     }

diff --git a/src/main/java/org/opentripplanner/middleware/otp/OtpDispatcher.java b/src/main/java/org/opentripplanner/middleware/otp/OtpDispatcher.java
@@ -7,6 +7,7 @@
 import javax.ws.rs.core.UriBuilder;
 import java.io.IOException;
 import java.net.URI;
+import java.net.URISyntaxException;
 import java.net.http.HttpClient;
 import java.net.http.HttpRequest;
 import java.net.http.HttpResponse;
@@ -87,10 +88,11 @@ private static OtpDispatcherResponse sendOtpRequest(URI uri) {
         // Get response from OTP
         OtpDispatcherResponse otpDispatcherResponse = null;
         try {
+            LOG.info("Sending request to OTP: {}", uri.toString());
             HttpResponse<String> otpResponse = client.send(request, HttpResponse.BodyHandlers.ofString());
             otpDispatcherResponse = new OtpDispatcherResponse(otpResponse);
         } catch (InterruptedException | IOException e) {
-            LOG.error("Error requesting OTP data", e);
+            LOG.error("Error requesting OTP data from {}", uri, e);
         }
         return otpDispatcherResponse;
     }

diff --git a/src/main/java/org/opentripplanner/middleware/otp/OtpDispatcherResponse.java b/src/main/java/org/opentripplanner/middleware/otp/OtpDispatcherResponse.java
@@ -2,7 +2,7 @@
 
 
 import org.apache.commons.lang3.SerializationUtils;
-import org.opentripplanner.middleware.otp.response.Response;
+import org.opentripplanner.middleware.otp.response.OtpResponse;
 import org.opentripplanner.middleware.utils.JsonUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -55,11 +55,11 @@ public OtpDispatcherResponse(String otpResponse) {
      * Response. POJO version of response from an OTP server.
      * Do not persist in case these classes change. This should always be re-instantiated from responseBody if needed.
      */
-    public Response getResponse() {
-        return JsonUtils.getPOJOFromJSON(responseBody, Response.class);
+    public OtpResponse getResponse() {
+        return JsonUtils.getPOJOFromJSON(responseBody, OtpResponse.class);
     }
 
-    public void setResponse(Response response) {
+    public void setResponse(OtpResponse response) {
         responseBody = JsonUtils.toJson(response);
     }
 

diff --git a/src/main/java/org/opentripplanner/middleware/otp/response/Itinerary.java b/src/main/java/org/opentripplanner/middleware/otp/response/Itinerary.java
@@ -98,20 +98,31 @@ public void clearAlerts() {
         }
     }
 
+    /**
+     * This method calculates equality in the context of trip monitoring in order to analyzing equality when
+     * checking if itineraries match.
+     *
+     * FIXME: maybe don't check duration exactly as it might vary slightly in certain trips
+     */
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
         Itinerary itinerary = (Itinerary) o;
-        return startTime.equals(itinerary.startTime) &&
-            endTime.equals(itinerary.endTime) &&
+        return duration.equals(itinerary.duration) &&
             Objects.equals(transfers, itinerary.transfers) &&
             legs.equals(itinerary.legs);
     }
 
+    /**
+     * This method calculates the hash code in the context of trip monitoring in order to analyzing equality when
+     * checking if itineraries match.
+     *
+     * FIXME: maybe don't check duration exactly as it might vary slightly in certain trips
+     */
     @Override
     public int hashCode() {
-        return Objects.hash(startTime, endTime, transfers, legs);
+        return Objects.hash(duration, transfers, legs);
     }
 
     @Override

diff --git a/src/main/java/org/opentripplanner/middleware/otp/response/Leg.java b/src/main/java/org/opentripplanner/middleware/otp/response/Leg.java
@@ -51,14 +51,15 @@ public class Leg implements Cloneable {
     /**
      * This method calculates equality in the context of trip monitoring in order to analyzing equality when
      * checking if itineraries are the same.
+     *
+     * FIXME: maybe don't check duration exactly as it might vary slightly in certain trips
      */
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
         Leg leg = (Leg) o;
-        return startTime.equals(leg.startTime) &&
-            endTime.equals(leg.endTime) &&
+        return duration.equals(leg.duration) &&
             mode.equals(leg.mode) &&
             from.equals(leg.from) &&
             to.equals(leg.to) &&
@@ -73,14 +74,15 @@ public boolean equals(Object o) {
     }
 
     /**
-     * This method calculates equality in the context of trip monitoring in order to analyzing equality when
-     * checking if itineraries are the same.
+     * This method calculates the hash code in the context of trip monitoring in order to analyzing equality when
+     * checking if itineraries match.
+     *
+     * FIXME: maybe don't check duration exactly as it might vary slightly in certain trips
      */
     @Override
     public int hashCode() {
         return Objects.hash(
-            startTime,
-            endTime,
+            duration,
             mode,
             from,
             to,

diff --git a/...ner/middleware/otp/response/Response.java → .../middleware/otp/response/OtpResponse.java b/...ner/middleware/otp/response/Response.java → .../middleware/otp/response/OtpResponse.java
@@ -11,7 +11,7 @@
  * Pare down version of class original produced for OpenTripPlanner.
  */
 @JsonIgnoreProperties(ignoreUnknown = true)
-public class Response {
+public class OtpResponse {
 
     /** A dictionary of the parameters provided in the request that triggered this response. */
     public HashMap<String, String> requestParameters;

diff --git a/src/main/java/org/opentripplanner/middleware/persistence/TypedPersistence.java b/src/main/java/org/opentripplanner/middleware/persistence/TypedPersistence.java
@@ -11,6 +11,7 @@
 import org.bson.conversions.Bson;
 import org.opentripplanner.middleware.bugsnag.BugsnagReporter;
 import org.opentripplanner.middleware.models.Model;
+import org.opentripplanner.middleware.utils.DateTimeUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -113,7 +114,7 @@ public void replace(String id, T replaceObject) {
      */
     public T update(String id, Document updateDocument) {
         // Set last updated.
-        updateDocument.put("lastUpdated", new Date());
+        updateDocument.put("lastUpdated", DateTimeUtils.nowAsDate());
         return mongoCollection.findOneAndUpdate(eq(id), new Document("$set", updateDocument), findOneAndUpdateOptions);
     }