opentripplanner / OpenTripPlanner

An open source multi-modal trip planner
http://www.opentripplanner.org
Other
2.18k stars 1.02k forks source link

Refactor SIRI-ET updaters #5904

Closed vpaturet closed 3 months ago

vpaturet commented 3 months ago

Summary

This refactoring aims at separating and encapsulating the following responsibilities:

  1. configuring and setting up network connections to read SIRI-ET feeds.
  2. consuming the SIRI-ET messages and applying the changes to the transit model.

While 1. is expected to be technology-specific (HTTP feed, Google PubSub subscription, Azure subscription, ...), 2. should be common for all implementations.

Issue

No

Unit tests

Updated unit tests.

Documentation

No

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 6.86275% with 95 lines in your changes missing coverage. Please review.

Project coverage is 69.44%. Comparing base (b1a7c53) to head (3727a8f). Report is 33 commits behind head on dev-2.x.

:exclamation: Current head 3727a8f differs from pull request most recent head ed7d9da

Please upload reports for the commit ed7d9da to get more accurate results.

Files Patch % Lines
...r/google/GooglePubsubEstimatedTimetableSource.java 0.00% 59 Missing :warning:
...siri/updater/google/SiriETGooglePubsubUpdater.java 0.00% 21 Missing :warning:
...siri/updater/AsyncEstimatedTimetableProcessor.java 0.00% 9 Missing :warning:
...er/ext/siri/updater/EstimatedTimetableHandler.java 70.00% 3 Missing :warning:
...pentripplanner/ext/siri/updater/SiriETUpdater.java 0.00% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev-2.x #5904 +/- ## ============================================= - Coverage 69.45% 69.44% -0.02% - Complexity 17066 17068 +2 ============================================= Files 1928 1931 +3 Lines 73580 73603 +23 Branches 7550 7547 -3 ============================================= + Hits 51108 51112 +4 - Misses 19847 19865 +18 - Partials 2625 2626 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

habrahamsson-skanetrafiken commented 3 months ago

I think the overall design of this looks good. How do you want to proceed with this PR? Do you want to merge this first and then we make a follow-up PR where we convert the Azure updators? I think that would probably be the easiest way forward.

vpaturet commented 3 months ago

I agree, and we will probably need a bit of back and forth to adapt the design to both the Google Pubsub and Azure updaters. So let's try first to have this PR reviewed and merged.

There is still a bit of refactoring that could be done to improve testability, notably SiriTimetableSnapshotSource cannot be stubbed easily since its applyEstimatedTimetable() method is not exposed in any interface. This can be done in a follow-up PR.

leonardehrenfried commented 3 months ago

I believe you can merge this in without having to wait for Henrik's approval again.

vpaturet commented 3 months ago

Yes, this was only cosmetic changes. Could you please use your admin rights to merge it?