refactor(TrackedJourney): Remove cumulative deviation field.

ibi-group · Oct 21, 2024 · be37335 · be37335
1 parent 25d19f9
commit be37335
Show file tree

Hide file tree

Showing 6 changed files with 27 additions and 59 deletions.
diff --git a/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java b/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java
@@ -27,9 +27,7 @@ public class TrackedJourney extends Model {
 
     public Map<String, String> busNotificationMessages = new HashMap<>();
 
-    public Double totalDeviation;
-
-    public int longestConsecutiveDeviatedPoints;
+    public int longestConsecutiveDeviatedPoints = -1;
 
     public transient MonitoredTrip trip;
 
@@ -42,7 +40,7 @@ public class TrackedJourney extends Model {
 
     public static final String END_CONDITION_FIELD_NAME = "endCondition";
 
-    public static final String TOTAL_DEVIATION_FIELD_NAME = "totalDeviation";
+    public static final String LONGEST_CONSECUTIVE_DEVIATED_POINTS_FIELD_NAME = "longestConsecutiveDeviatedPoints";
 
     public static final String TERMINATED_BY_USER = "Tracking terminated by user.";
 
@@ -118,15 +116,20 @@ public int computeLargestConsecutiveDeviations() {
         int count = 0;
         int maxCount = 0;
         for (TrackingLocation location : locations) {
-            // Traveler must be moving (speed != 0) for a deviated location to be counted.
-            if (location.tripStatus == TripStatus.DEVIATED) {
-                if (location.speed != 0) {
-                    count++;
-                    if (maxCount < count) maxCount = count;
+            // A trip status must have been computed for a location to count.
+            // (The mobile app will send many other more for reference, but only those for which we compute a status
+            // (i.e. the last coordinate in every batch) will potentially count.
+            if (location.tripStatus != null) {
+                // Traveler must be moving (speed != 0) for a deviated location to be counted.
+                if (location.tripStatus == TripStatus.DEVIATED) {
+                    if (location.speed != 0) {
+                        count++;
+                        if (maxCount < count) maxCount = count;
+                    }
+                } else {
+                    // If a location has a status computed and is not deviated, reset the streak.
+                    count = 0;
                 }
-            } else {
-                // If a location is not deviated, reset the streak.
-                count = 0;
             }
         }
         return maxCount;

diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java b/src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java
@@ -152,8 +152,8 @@ private static EndTrackingResponse completeJourney(TripTrackingData tripData, bo
         trackedJourney.end(isForciblyEnded);
         Persistence.trackedJourneys.updateField(trackedJourney.id, TrackedJourney.END_TIME_FIELD_NAME, trackedJourney.endTime);
         Persistence.trackedJourneys.updateField(trackedJourney.id, TrackedJourney.END_CONDITION_FIELD_NAME, trackedJourney.endCondition);
-        trackedJourney.totalDeviation = trackedJourney.computeTotalDeviation();
-        Persistence.trackedJourneys.updateField(trackedJourney.id, TrackedJourney.TOTAL_DEVIATION_FIELD_NAME, trackedJourney.totalDeviation);
+        trackedJourney.longestConsecutiveDeviatedPoints = trackedJourney.computeLargestConsecutiveDeviations();
+        Persistence.trackedJourneys.updateField(trackedJourney.id, TrackedJourney.LONGEST_CONSECUTIVE_DEVIATED_POINTS_FIELD_NAME, trackedJourney.longestConsecutiveDeviatedPoints);
 
         // Provide response.
         return new EndTrackingResponse(

diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java
@@ -140,11 +140,6 @@ public static Map<OtpUser, List<TrackedJourney>> mapJourneysToUsers(List<Tracked
         return map;
     }
 
-    public static Optional<TrackedJourney> selectMostDeviatedJourney(List<TrackedJourney> journeys) {
-        if (journeys == null) return Optional.empty();
-        return journeys.stream().max(Comparator.comparingDouble(j -> j.totalDeviation));
-    }
-
     public static Optional<TrackedJourney> selectMostDeviatedJourneyUsingDeviatedPoints(List<TrackedJourney> journeys) {
         if (journeys == null) return Optional.empty();
         final double INTERVALS_IN_ONE_MINUTE = Math.ceil(60.0 / TRIP_TRACKING_UPDATE_FREQUENCY_SECONDS);

diff --git a/src/test/java/org/opentripplanner/middleware/controllers/api/TrackedTripControllerTest.java b/src/test/java/org/opentripplanner/middleware/controllers/api/TrackedTripControllerTest.java
@@ -190,8 +190,7 @@ void canCompleteJourneyLifeCycle() throws Exception {
         // Check that the TrackedJourney Mongo record has been updated.
         TrackedJourney mongoTrackedJourney = Persistence.trackedJourneys.getById(startTrackingResponse.journeyId);
         assertEquals(TERMINATED_BY_USER, mongoTrackedJourney.endCondition);
-        assertNotNull(mongoTrackedJourney.totalDeviation);
-        assertNotEquals(0.0, mongoTrackedJourney.totalDeviation);
+        assertNotEquals(-1, mongoTrackedJourney.longestConsecutiveDeviatedPoints);
         DateTimeUtils.useSystemDefaultClockAndTimezone();
     }
 
@@ -409,8 +408,7 @@ void canForciblyEndJourney() throws Exception {
         // Check that the TrackedJourney Mongo record has been updated.
         TrackedJourney mongoTrackedJourney = Persistence.trackedJourneys.getById(startTrackingResponse.journeyId);
         assertEquals(FORCIBLY_TERMINATED, mongoTrackedJourney.endCondition);
-        assertNotNull(mongoTrackedJourney.totalDeviation);
-        assertNotEquals(0.0, mongoTrackedJourney.totalDeviation);
+        assertNotEquals(-1, mongoTrackedJourney.longestConsecutiveDeviatedPoints);
     }
 
     @Test

diff --git a/src/test/java/org/opentripplanner/middleware/models/TrackedJourneyTest.java b/src/test/java/org/opentripplanner/middleware/models/TrackedJourneyTest.java
@@ -10,42 +10,30 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
 class TrackedJourneyTest {
-    @Test
-    void canComputeTotalDeviation() {
-        TrackedJourney journey = new TrackedJourney();
-        journey.locations = null;
-        assertEquals(-1.0, journey.computeTotalDeviation());
-
-        journey.locations = Stream
-            .of(11.0, 23.0, 6.4)
-            .map(d -> {
-                TrackingLocation location = new TrackingLocation();
-                location.deviationMeters = d;
-                return location;
-            })
-            .collect(Collectors.toList());
-        assertEquals(40.4, journey.computeTotalDeviation());
-    }
-
     @Test
     void canComputeLargestConsecutiveDeviations() {
         TrackedJourney journey = new TrackedJourney();
         journey.locations = null;
         assertEquals(-1, journey.computeLargestConsecutiveDeviations());
 
         journey.locations = Stream
-            .of(0, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 0)
+            .of(0, 1, 1, 1, null, 1, 1, 0, 0, null, 1, 1, 1, null, 0)
             .map(d -> {
                 TrackingLocation location = new TrackingLocation();
-                location.tripStatus = d == 1 ? TripStatus.DEVIATED : TripStatus.ON_SCHEDULE;
+                location.tripStatus = d == null
+                    ? null
+                    : d == 1
+                    ? TripStatus.DEVIATED
+                    : TripStatus.ON_SCHEDULE;
                 location.speed = 1;
                 return location;
             })
             .collect(Collectors.toList());
 
         // Insert a location where the traveler is not moving.
-        journey.locations.get(4).speed = 0;
+        journey.locations.get(5).speed = 0;
 
+        // After excluding the nulls, count the first group of consecutive ones.
         assertEquals(4, journey.computeLargestConsecutiveDeviations());
     }
 }
diff --git a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java
@@ -123,7 +123,6 @@ private static TrackedJourney createJourney(
         journey.endCondition = endCondition;
         journey.endTime = endTime != null ? Date.from(endTime) : null;
         journey.startTime = journey.endTime;
-        journey.totalDeviation = (double)points;
         journey.longestConsecutiveDeviatedPoints = points;
         Persistence.trackedJourneys.create(journey);
         return journey;
@@ -148,21 +147,6 @@ void canMapJourneysToUsers() {
         }
     }
 
-    @Test
-    void canSelectMostDeviatedJourney() {
-        TrackedJourney journey1 = new TrackedJourney();
-        journey1.totalDeviation = 250.0;
-        journey1.endTime = Date.from(Instant.now().minus(3, ChronoUnit.HOURS));
-
-        TrackedJourney journey2 = new TrackedJourney();
-        journey2.totalDeviation = 400.0;
-        journey2.endTime = Date.from(Instant.now().minus(5, ChronoUnit.HOURS));
-
-        Optional<TrackedJourney> optJourney = TripSurveySenderJob.selectMostDeviatedJourney(List.of(journey1, journey2));
-        assertTrue(optJourney.isPresent());
-        assertEquals(journey2, optJourney.get());
-    }
-
     @Test
     void canSelectMostDeviatedJourneyUsingDeviatedPoints() {
         TrackedJourney journey1 = new TrackedJourney();