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

Attention: Patch coverage is 88.46154% with 3 lines in your changes missing coverage. Please review.

Project coverage is 69.85%. Comparing base (5f9b448) to head (00f86bc).

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      69.85%   69.85%           
- Complexity     17921    17925    +4     
==========================================
  Files           2035     2035           
  Lines          76495    76520   +25     
  Branches        7824     7826    +2     
==========================================
+ Hits           53434    53454   +20     
- Misses         20324    20327    +3     
- Partials        2737     2739    +2     

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

[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.