-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Improve handling of realtime updated StopPatterns #6909
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev-2.x
Are you sure you want to change the base?
Changes from all commits
aeaa862
0208cbf
a7ff17b
dcc9067
fa0f0b2
d7df30d
b022e57
cfa755c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -66,6 +66,31 @@ public RealTimeRaptorTransitDataUpdater(TimetableRepository timetableRepository) | |
| this.timetableRepository = timetableRepository; | ||
| } | ||
|
|
||
| /// Updates the real-time [RaptorTransitData] to use the modified timetables. | ||
| /// | ||
| /// This method bridges the different update approaches: | ||
| /// 1. `updatedTimetables` and `timetables` only contains [Timetable]s with real-time | ||
| /// updates. This means that removed items are not present. | ||
| /// 2. [RaptorTransitData] requires applying the changes to a previous snapshot: adding, | ||
| /// updating and removing timetables. | ||
| /// | ||
| /// To support this the method has three tasks: | ||
| /// 1. Collect [TripPatternForDate]s which have invalidated data (`oldTripPatternsForDate`). | ||
| /// Trips may change in multiple ways and because of that may move between [TripPattern]s. To | ||
| /// track a [TripIdAndServiceDate] it's previous state needs to be stored so that all relevant | ||
| /// places may be updated. | ||
| /// * a trip may have a new (real-time) Timetable, which results in two updated [Timetable]s | ||
| /// * a trip may move between scheduled [StopPattern]s and/or real-time [StopPattern]s | ||
| /// 2. Collect [TripPatternForDate]s which have valid data (`newTripPatternsForDate`). | ||
| /// There are two options: | ||
| /// 1. an update was received | ||
| /// 2. no update was received, and so the previous updated should be removed. If the update | ||
| /// was for a scheduled trip, then the schedule should be restored. | ||
| /// 3. Remove the `oldTripPatternsForDate` and add the `newTripPatternsForDate` to the | ||
| /// [RaptorTransitData]. | ||
| /// | ||
| /// @param updatedTimetables that changed with the current snapshot | ||
| /// @param timetables which are affected by real-time updates | ||
| public void update( | ||
| Collection<Timetable> updatedTimetables, | ||
| Map<TripPattern, SortedSet<Timetable>> timetables | ||
|
|
@@ -204,7 +229,8 @@ public void update( | |
| patternsForDate.remove(tripPatternForDate); | ||
| } | ||
| } else { | ||
| LOG.warn("Could not fetch timetable for {}", pattern); | ||
| LOG.warn("Could not fetch timetable for {}, removing.", pattern); | ||
| patternsForDate.remove(tripPatternForDate); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the most important change in the PR. I absolutely support changing this class but I'm having a hard time understanding the consequences of this change: the code is very complex with several collections updating each other. If you have a better understanding, do you have time to walk me through the code? Maybe @vpaturet is interested as well. |
||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| package org.opentripplanner.transit.model._data; | ||
|
|
||
| import java.time.Duration; | ||
| import java.util.List; | ||
| import org.opentripplanner.routing.algorithm.raptoradapter.transit.TransitTuningParameters; | ||
| import org.opentripplanner.routing.api.request.RouteRequest; | ||
| import org.opentripplanner.transit.model.site.StopTransferPriority; | ||
|
|
||
| class TestTransitTuningParameters implements TransitTuningParameters { | ||
|
|
||
| @Override | ||
| public boolean enableStopTransferPriority() { | ||
| return false; | ||
| } | ||
|
|
||
| @Override | ||
| public Integer stopBoardAlightDuringTransferCost(StopTransferPriority key) { | ||
| return 0; | ||
| } | ||
|
|
||
| @Override | ||
| public int transferCacheMaxSize() { | ||
| return 0; | ||
| } | ||
|
|
||
| @Override | ||
| public Duration maxSearchWindow() { | ||
| return null; | ||
| } | ||
|
|
||
| @Override | ||
| public List<Duration> pagingSearchWindowAdjustments() { | ||
| return List.of(); | ||
| } | ||
|
|
||
| @Override | ||
| public List<RouteRequest> transferCacheRequests() { | ||
| return List.of(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make this proper Javadoc, please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or is this now valid Markdown-ish Javadoc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use the correct Javadoc syntax here please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We discussed this while you were gone. Apparently IntelliJ IDEA already supports this new markdown format and then no one was really against using it. However, we didn't discuss it too much and not sure what is the way forward with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it's fine. i was unaware that it will be officially supported. Intellij is the only place I would read Javadoc so I don't care if other tools don't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is Markdown-ish javadoc