Pass empty timetable to TransitLayerUpdater in order to clear added trip patterns

[miklcct]

PR Instructions

Summary

Currently, after clearing TimetableSnapshot, if it is committed directly empty, nothing is passed to TransitLayerUpdater. As a result, previously added trip patterns remain in the TransitLayer for routing purpose, resulting in stale timetables returned in itinerary results which no longer exist in the actual, published timetable snapshot.

Issue

Fixes #6197

Unit tests

Added

Documentation

Bug fix only, not needed

Changelog

[codecov]

Codecov Report

❌ Patch coverage is 88.46154% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.14%. Comparing base (87cf700) to head (85f356b).
⚠️ Report is 1 commits behind head on dev-2.x.

Files with missing lines	Patch %	Lines
...main/java/org/opentripplanner/model/Timetable.java	33.33%	1 Missing and 1 partial ⚠️
...a/org/opentripplanner/model/TimetableSnapshot.java	95.65%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             dev-2.x    #6280   +/-   ##
==========================================
  Coverage      72.14%   72.14%           
- Complexity     19673    19678    +5     
==========================================
  Files           2127     2127           
  Lines          79562    79587   +25     
  Branches        8041     8043    +2     
==========================================
+ Hits           57397    57419   +22     
- Misses         19331    19333    +2     
- Partials        2834     2835    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[miklcct]

Sorry it seems that the bug still appears on the live server, so I need to mark this as draft for now.

[miklcct]

The solution will not work if a timetable is cleared multiple times between commits. Also on my live server log there are a lot of "WARN could not fetch timetable for pattern" which may be related to this problem as well. I'll need to further fix it.

[miklcct]

What does this line actually mean:

OpenTripPlanner/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransitLayerUpdater.java

Line 198 in d931077

LOG.warn("Could not fetch timetable for {}", pattern);

My server log is flooded with this.

[vpaturet]

Would it work and would it be simpler if:

1. We set a flag in the TransitLayerUpdater each time we call TimetableSnapshot.clear():

public clear(String feedId) {
...
if(transitLayerUpdater != null) {
transitLayerUpdater.setCleared(feedId);
}
}

2. The TransitLayerUpdater checks the flag and clears all data belonging to this feed before applying the updates:

public void update(
    Collection<Timetable> updatedTimetables,
    Map<TripPattern, SortedSet<Timetable>> timetables
  ) {
if(isCleared(feedId)) {
realtimeTransitLayer.clear(feedId);
setCleared(feedId, false);
}
// apply updates
}

[miklcct]

Would it work and would it be simpler if:

1. We set a flag in the TransitLayerUpdater each time we call TimetableSnapshot.clear():

public clear(String feedId) {
...
if(transitLayerUpdater != null) {
transitLayerUpdater.setCleared(feedId);
}
}

2. The TransitLayerUpdater checks the flag and clears all data belonging to this feed before applying the updates:

public void update(
    Collection<Timetable> updatedTimetables,
    Map<TripPattern, SortedSet<Timetable>> timetables
  ) {
if(isCleared(feedId)) {
realtimeTransitLayer.clear(feedId);
setCleared(feedId, false);
}
// apply updates
}

I think it will be better and I will work on this.

[miklcct]

Unfortunately TransitLayerUpdater is not feed aware and it doesn't really have a state, so it is not possible for me to do that without adding complexity into the system. There is no concept of a "feed" and the "original" in the TransitLayer.

[miklcct]

I have updated my approach as the original contained a mistake which broke the cancelled trip fetching.

I have now deferred the actual restoration of the timetable to the time of commit. When clear() is called, the pattern / service date pairs cleared are stored as a separate field, and during commit, the scheduled timetables of these pairs are added to the dirty set such that they are passed to the updater.