Dynamic Rerouting #286

br648 · 2025-01-09T11:59:34Z

Checklist

Appropriate branch selected (all PRs must first be merged to dev before they can be merged to master)
Any modified or new methods or classes have helpful JavaDoc and code is thoroughly commented
The description lists all applicable issues this PR seeks to resolve
The description lists any configuration setting(s) that differ from the default settings
All tests and CI builds passing

Description

Adds the ability for a traveler to reroute from their current location if deviated from the original trip. A new itinerary is retrieved from OTP (replacing the original trip) and if available tracking and instructions will be based on this instead. Subsequent rerouting will replace previous rerouted trips.

…m OTP from the traveler's curre Tracking is then based on the new itinerary

…response

binh-dam-ibigroup

First round of comments after a first read.

src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java

binh-dam-ibigroup · 2025-01-09T16:38:36Z

src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java

+            OtpGraphQLVariables query = trackedJourney.trip.otp2QueryParams;
+            query.fromPlace = new Coordinates(trackedJourney.lastLocation()).getCoordinates();
+            query.time = getTimeNowAsString();


Extract a method and write unit tests for it.

src/test/java/org/opentripplanner/middleware/controllers/api/TrackedTripControllerTest.java

…e PR feedback and updated unit

binh-dam-ibigroup

Here are the next changes I think are needed to get an instruction that matches the rerouted itinerary:

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

binh-dam-ibigroup · 2025-01-13T15:52:32Z

src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java

+            var trackedJourney = tripData.journey;
+            if (trackedJourney != null) {
+                if (rerouteTrip(tripData)) {
+                    return doUpdateTracking(request, tripData, false);


Interesting, POST-ing a rerouting request returned a "no-instruction" response instead of a new instruction. Let me dig more.

Also, POST-ing a track update with the same, originally deviated position should have updated to an on-track position with an updated instruction, but it stayed on the "deviated" status instead.

This line calls the linked piece of code, where the matching itinerary should be a clone of the journey state's matching itinerary, not of the saved itineary.

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

Line 257 in 8f981d7

matchingItinerary = trip.itinerary.clone();

We probably need to add or update the flag for updateTracking so that no new tracked journey is created, but the rerouting gives directions as if it were the beginning of the trip.

src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java

.github/workflows/maven-ci.yml

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

…ed used instead of localhost

…se and updates around this

src/main/java/org/opentripplanner/middleware/triptracker/response/RerouteResponse.java

src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java

binh-dam-ibigroup · 2025-01-15T16:47:37Z

src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java

        if (tripData != null) {
            var trackedJourney = tripData.journey;
-            return trackedJourney != null && updateTripWithNewItinerary(trackedJourney);
+            if (trackedJourney != null) {
+                return updateTripWithNewItinerary(trackedJourney);


We should probably notify companions/observers of that change.

binh-dam-ibigroup · 2025-01-15T17:46:22Z

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

If the traveler takes the trip, it will likely still within the active trip monitoring window, and trip monitoring will attempt to replace journeyState.matchingItinerary with an itinerary matching the original one. To prevent that, if rerouting has occured, getQueryParamsForTargetZonedDateTime should also plug the most recent rerouting location that is saved in TrackedJourney.

binh-dam-ibigroup · 2025-01-15T17:50:45Z

src/test/java/org/opentripplanner/middleware/controllers/api/TrackedTripControllerTest.java

+        // Confirm traveler is no longer 'deviated'.
+        updateTrackingResponse = JsonUtils.getPOJOFromJSON(response.responseBody, TrackingResponse.class);
+        assertNotEquals(TripStatus.DEVIATED.name(), updateTrackingResponse.tripStatus);
+


Try to insert a call to CheckMonitoredTrip and assert that it doesn't revert the matching itinerary to what was before.

binh-dam-ibigroup · 2025-01-15T17:51:33Z

src/test/java/org/opentripplanner/middleware/controllers/api/TrackedTripControllerTest.java

+    }
+
+    /** Provides a mock OTP 'plan' rerouted response. */
+    public Supplier<OtpResponse> mockOtpPlanResponse() {


Rename to mockOtpReroutedPlanResponse or something similar.

binh-dam-ibigroup

Make field itinerary in RerouteResponse public, and make the itinerary monitor process also apply the updated origin location so it can update the rerouted itinerary with realtime updates, if any.

br648 added 2 commits January 9, 2025 11:54

feat(Adds the ability to reroute trips): Requests a new itinerary fro…

d49dca4

…m OTP from the traveler's curre Tracking is then based on the new itinerary

refactor(Removed test to call reroute end point): Unable to mock OTP …

d8e002d

…response

br648 assigned binh-dam-ibigroup Jan 9, 2025

br648 added 2 commits January 9, 2025 15:20

refactor(Removed redundant last leg check):

e45a6f5

refactor(Removed missing import):

8f6b927

binh-dam-ibigroup requested changes Jan 9, 2025

View reviewed changes

binh-dam-ibigroup assigned br648 Jan 9, 2025

refactor(Updated to make use of otp response provider): Addressed som…

ec510ae

…e PR feedback and updated unit

binh-dam-ibigroup requested changes Jan 13, 2025

View reviewed changes

binh-dam-ibigroup removed their assignment Jan 13, 2025

Fixed merge conflicts

bdaa2a1

binh-dam-ibigroup reviewed Jan 14, 2025

View reviewed changes

.github/workflows/maven-ci.yml Outdated Show resolved Hide resolved

binh-dam-ibigroup reviewed Jan 14, 2025

View reviewed changes

src/main/java/org/opentripplanner/middleware/controllers/api/TrackedTripController.java Outdated Show resolved Hide resolved

br648 added 4 commits January 15, 2025 08:03

revert(Reinstated the github dir): Removed by mistake

a2784df

fix(Issue with localhost reference in swagger output): Weird referenc…

7eac962

…ed used instead of localhost

refactor(Updates to address PR feedback): New reroute tracking respon…

cf7113f

…se and updates around this

refactor(Updated swagger docs):

2836ef0

binh-dam-ibigroup reviewed Jan 15, 2025

View reviewed changes

src/main/java/org/opentripplanner/middleware/triptracker/response/RerouteResponse.java Outdated Show resolved Hide resolved

binh-dam-ibigroup reviewed Jan 15, 2025

View reviewed changes

src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java Outdated Show resolved Hide resolved

refactor(Updated unit tests to use besoke trip and rerouted trip):

1ae6f1e

br648 assigned binh-dam-ibigroup and unassigned br648 Jan 15, 2025

binh-dam-ibigroup reviewed Jan 15, 2025

View reviewed changes

binh-dam-ibigroup requested changes Jan 15, 2025

View reviewed changes

binh-dam-ibigroup assigned br648 and unassigned binh-dam-ibigroup Jan 15, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dynamic Rerouting #286

Dynamic Rerouting #286

br648 commented Jan 9, 2025

binh-dam-ibigroup left a comment

binh-dam-ibigroup Jan 9, 2025

binh-dam-ibigroup left a comment

binh-dam-ibigroup Jan 13, 2025

binh-dam-ibigroup Jan 13, 2025

binh-dam-ibigroup Jan 13, 2025

binh-dam-ibigroup Jan 13, 2025

binh-dam-ibigroup Jan 15, 2025

binh-dam-ibigroup Jan 15, 2025

binh-dam-ibigroup Jan 15, 2025

binh-dam-ibigroup Jan 15, 2025

binh-dam-ibigroup left a comment

Dynamic Rerouting #286

Are you sure you want to change the base?

Dynamic Rerouting #286

Conversation

br648 commented Jan 9, 2025

Checklist

Description

binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

binh-dam-ibigroup left a comment

Choose a reason for hiding this comment