Reject SIRI-ET updates with empty StopPointRefs - #6266
Conversation
leonardehrenfried
changed the title
leonardehrenfried
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #6266 +/- ##
=============================================
- Coverage 69.71% 69.71% -0.01%
- Complexity 17696 17698 +2
=============================================
Files 2008 2008
Lines 75834 75840 +6
Branches 7765 7768 +3
=============================================
+ Hits 52866 52869 +3
- Misses 20256 20258 +2
- Partials 2712 2713 +1 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
leonardehrenfried
added
the
+Real-Time
|
@lassetyr Can you confirm that we never want to process calls where the StopPointRef is empty? |
lassetyr
left a comment
lassetyr
left a comment
There was a problem hiding this comment.
StopPointRef is a required field in SIRI ET, so rejecting these will be fine
| return UpdateError.result(trip != null ? trip.getId() : null, NOT_MONITORED, dataSource); | ||
| } | ||
|
|
||
| for (var call : CallWrapper.of(estimatedVehicleJourney)) { |
There was a problem hiding this comment.
This will only be validated for modified trips, but I think we should probably enforce it for added trips as well? What do you think about moving this check to it's own method and calling it as the first thing in the SiriTimetableSnapshotSource.apply() method?
There was a problem hiding this comment.
That's a good point, since there it is even more important. I changed it here: 5315ec1
There was a problem hiding this comment.
I also added a parameterized test so both options are tested.
5315ec1 to
f1f9d8a
Compare
4 participants
Summary
A SIRI feed from South Tyrol that I'm working with contains empty
<StopPointRef>elements that cause the fuzzy trip matcher to crash.For this reason I added a check that rejects any
VehicleJourneywhich contains these empty elements.Issue
Relates to noi-techpark/opendatahub-mentor-otp#213
Unit tests
GTFS-RT has module tests but SIRI didn't so far but this PR adds them there, too.
Changelog
✔️
Bumping the serialization version id
⛔
cc @rcavaliere