Implement SIRI Lite#6284
Conversation
6afe592 to
bf2fcf2
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #6284 +/- ##
=============================================
- Coverage 69.81% 69.80% -0.01%
- Complexity 17825 17933 +108
=============================================
Files 2020 2042 +22
Lines 76271 76631 +360
Branches 7806 7829 +23
=============================================
+ Hits 53250 53494 +244
- Misses 20304 20399 +95
- Partials 2717 2738 +21 ☔ View full report in Codecov by Sentry. |
9e15f52 to
538a7dd
Compare
|
@habrahamsson-skanetrafiken Can I ask you to review this one? If not let me know and I will reassign. |
|
It never occurred to me until yesterday's meeting that I also could have made the current updater/loader configurable. Let me know if you feel that's the better option, @t2gran. |
538a7dd to
6703d62
Compare
63687ed to
86a447f
Compare
t2gran
left a comment
There was a problem hiding this comment.
I read through most of the main code - not all. The wiring can be improved, so I will wait with the rest of the review to that is done.
| /** | ||
| * Feed id that is used to match trip ids in the TripUpdates | ||
| */ | ||
| private final String feedId; |
There was a problem hiding this comment.
Is this unused, or is it the SIRI namespace?
There was a problem hiding this comment.
It was a reference to the actual id of the feed and unused.
| .toString(); | ||
| } | ||
|
|
||
| private static SiriLoader createLoader(Parameters parameters) { |
There was a problem hiding this comment.
DRY - This can be refactored out. Either create a SiriLoaderFactory or create it in the UpdatorConfigurator and pass it in.
…ater/SiriETUpdater.java Co-authored-by: Thomas Gran <t2gran@gmail.com>
…ater/SiriETHttpTripUpdateSource.java Co-authored-by: Thomas Gran <t2gran@gmail.com>
1ab52da to
c7bf5ed
Compare
c7bf5ed to
790bdc6
Compare
18dfc6d to
ec9e724
Compare
|
Here is what I had in mind with respect to manual Dependency Injection: package org.opentripplanner.updater.siri.configure;
/** This is doing manually dependency injection and work as a factory to create the updater components. */
class SiriUpdaterModule {
public static SiriETUpdater createSiriETUpdater(...) {
var loader = createLoader(...);
var source = createSource(loader, ...);
return createETUpdater(source, ...);
}
private static createLoader(...);
private static createSource(...);
private static createUpdater(...);
}Each component define the parameters as an interface inside the class. The same parameters can be repeated, but not inherited. For example if The factory for creating the Loader can be inlined here. The |
|
Just a question, |
|
Yes, you cannot use it at the same time as GTFS-RT. I cannot promise anything but it looks like this might change in 2025. |
| } | ||
|
|
||
| private static SiriLoader createLoader(SiriSXUpdater.Parameters params) { | ||
| return switch (params) { |
There was a problem hiding this comment.
Maybe this should have support for file loading like the ET case has?
Summary
Implements the SIRI Light flavour (as opposed to the request/response one that Entur and Skanetrafiken uses). This means that the entire SIRI feed is downloaded in a single GET request.
Issue
Closes #6263
Documentation
The new updater is fully documented and I took the liberty to make all references to SIRI consistent in the docs.
Unit tests
Some added, but it's mostly wiring. I'm happy to add more if requested but generally we don't test the HTTP layer a lot.
cc @rcavaliere @Jouca