Pass empty timetable to TransitLayerUpdater in order to clear added trip patterns #6280
Conversation
miklcct
force-pushed
the
fix-update-stale-service
branch
from
5a4a351
to
d931077
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #6280 +/- ##
=============================================
+ Coverage 69.76% 69.78% +0.02%
- Complexity 17751 17792 +41
=============================================
Files 2010 2017 +7
Lines 75948 76079 +131
Branches 7784 7789 +5
=============================================
+ Hits 52984 53095 +111
- Misses 20257 20274 +17
- Partials 2707 2710 +3 ☔ View full report in Codecov by Sentry. |
leonardehrenfried
changed the title
| @@ -552,7 +552,39 @@ public boolean isEmpty() { | |||
| * @return true if the timetable changed as a result of the call | |||
| */ | |||
| private boolean clearTimetables(String feedId) { | |||
| return timetables.keySet().removeIf(tripPattern -> feedId.equals(tripPattern.getFeedId())); | |||
| var dirty = false; | |||
| dirtyTimetables.clear(); | |||
There was a problem hiding this comment.
I not sure if it's less confusing to just tell the TransitLayerUpdater to drop all previously added timetables. I think @vpaturet will be the best person to say.
|
Sorry it seems that the bug still appears on the live server, so I need to mark this as draft for now. |
|
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. |
|
What does this line actually mean: My server log is flooded with this. |
…Layer during commit
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