Transit model refactor for easier integration with RT-updaters and transit model maintenance

[t2gran]

The transit model needs some love and care. RT-updates is error prone and inefficient, and almost impossible to maintain. The Raptor integration is unnecessarily complex.

Goals

• Make a better model which serves RT updates and Raptor routing well, over serving the index API.
• Improve readability and maintainability
• Make an encapsulated model with immutable data structures supporting concurrent updates
• We will keep the original concurrency strategy
• Avoid "per request" data preparation - takes time
• Keep only the latest state from realtime updates
• Provide a general interface for real time updates, so updaters/producers (gtfs-rt, siri, etc..) only do mapping. The domain logic should be encapsulated into the transit model.
• Consistent way of dealing with illegal input data. We should fail-early not late, and illegal data should prevent other data from being used/imported. Data errors should be reported (issue report or log). Entities dropped cascade to other entities referencing them.

• 5070 Strategy for error handling and error codes. On API-calls/RT-updates we should be be able to list all possible ERRORS that could occur, but we should not pollute the domain models with a global ERROR enum (like this code).

Milestones

This is a very big task, so we decided on adding a few milestones to track progress.

• [x] M1 - Initial design
• [x] M2 - Basic model infrastructure refactor. Migrate the most important transit entities to new immutable model and at the same time develop the infrastructure for it. Entities/services: TransitService with Agency, Operator, Route, Trip and all types these entities reference.

  4150, #4151, #4172, #4176 and #4222

• [x] M3 - Migrate StopLocationService to new model.
• [x] M4 - Migrate remaining transit entities to new model.
• [ ] M5 - Create new model needed for Raptor routing. Includes: TripPattern, Timetable, TransitCalendar, RaptorDataProviders. This is done when existing Raptor module tests can be run successfully using the new model. OTP will still not work on the new model.
• [ ] M6 - Make OTP work for the simplest possible routing request. This include extracting interfaces for the transit model and let the new and the old model implement this, so that itinerary mapping, filtering and APIs can work on both the old and the new model. Another approach is to duplicate some of the code, but this might become a maintenance issue since we will live with this for a while.
• [ ] M7 - Siri RT updater - views/poc. Copy one RT-updater and make it work with the new model, designing the needed views to safely update the transit model. Focus on thread-safety and designing the view as "developer friendly" as possible. Use-case: update departure and/or arrival-times.
• [ ] M8 - Siri RT updater - remaining use-cases.
• [ ] M9 - GTFS RT updater Copy existing GTFS updater and port it to new model.
• [ ] M10 - Migrate constrained transfers to new model
• [ ] M11 - Migrate frequency-based trips to new model
• [ ] M12 - Migrate other updaters if needed
• [ ] M13 - Final cleanup for new model - go through checklist below and fix reminding model issues.
• [ ] M14 - Acceptance test
• [ ] M15 - Remove old code

Strategy: We will try to refactor/migrate as much as possible, but in some cases it will not be cost effective - very difficult. So, at some point we will create a OTPFeature flag to switch between the new and the old model. Note! The graph will be different, so the feature flag must be set to the same value for transit graph build and serve. The Street graph should not be affected.

Checklist - features we should remember to support

This is the list of features we should use to "functionally" verify our model. We do not need to implement these, but the model should take them into account so we CAN implement them in the future.

• [ ] Allow for multiple new shapes for a changed trip/pattern (from @optionsome). Something like shape on Trip(Pattern)ForDate override shape on scheduled pattern.
• [ ] All times should be stored in a Server TimeZone (not agency of feed). We will use a time zero, then for a timetable we can use an offset + array of relative times. The smallest time in the array will be zero (can be both first arrival or departure time).
• [ ] Remove constraint that first departure time and last arrival time is known.
• [ ] The scheduled data structure should be immutable
• [x] #4070
• [ ] Prepare for a better heuristic search in Raptor
• [ ] Use transitServiceStart(build-cinfig) as time zero - we can start with this, and see how it looks.
• [ ] Configure TimeZone offset in router-config.json. Make all times in the transit model relative to this offset and time transitServiceStart. Use 32 bit operations for calculating time. Store as int, or less (arrays of short or byte). We must use absolute TimeZone (+/-HH:MM) without DST.
• [x] Refactor TripSearch out of Raptor #4069
• [ ] Introduce and use Dagger dependency-injection
• [ ] Use the the new transit model in module tests instead of a custom test model, possible with a builder/factory to simplify the creation of test data.
• [ ] Migrate all usage of FeedScopedId to ID, and use a configurable Factory/mapper in the import/APIs. This will allow deployments witch do not want to use the FeedScopedId to use another strategy.
• [ ] Remove model class headers: /* This file is based on code copied from project OneBusAway, ... */

Model refactoring

• The real time information will be part of the TransitModel, not a separate Snapshot. So a trip will have both scheduled and realtime data. The RealTime updaters will copy parts of the model, change it and post it back tho the service, which apply the changes in a thread-safe way.
• The data needed by for the routing will be put in a data structure optimized for routing. The entities will not hold this data as fields, but be views of the data in the optimized model. Fields/attributes not needed for trip routing will be part of the TransitModel objects.

Package structure

This is just a starting point/suggestion, while working with this we will probably refactor several times.

• transit
  • model
    • lang // Shared/base types used across all packages in the transit/model like FeedScopedId and TransitEntity
    • stop // Stop and station related
    • network // Rote, shapes, stop patterns
    • timetable or schedule // Trip, StopTimes
    • realtime^[1] // RealTimeSnapshot
    • support // Supporting functionality that does not fit in one of the above packages (depend on more than one of the packages)

[1] Might be a sub package of timetable

Package dependencies

This is WIP, we will see if the works out well. It might be that we also want to add a dependency from [network] to [schedule] (Pattern ->* Trip).

[optionsome]

"Allow for multiple new shapes for a changed trip/pattern" I think this issue is not only related to realtime updated patterns but for even scheduled ones.

[slvlirnoff]

For RT updates, I feel that there's a lot of duplication between GTFS and Siri relatives TimeTableSnapshotSource and associated classes (TripPatternCache, etc..) and this is not helping integrating other RT sources (VDV, custom profiles, etc.)

Would it be a goal also to generalize the interface to update the real time information? The various producers (gtfs-rt, siri, etc..) only mapping information for it?

[t2gran]

@slvlirnoff I agree, this is the intention. I added another goals to the list above - hope that makes it a bit more clear.

[t2gran]

Draft v31

I AM NOT MAINTAINING THIS MODEL ANY MORE, GO TO THE TRANSIT MODEL PACKAGE FOR LATEST VERSION. THE DOC IS INCLUDED IN #4718.

Note! This is the static Transit model used to serve Raptor, the mutable model seen by RT-Updaters will be different, and so will the model seen by the API. The idea is to decorate with view classes to simplify read access and use builders for mutation.

[slvlirnoff]

@t2gran I have a few questions but also unable to see all the texts on my device (some are truncated), could you include it in another format?

[t2gran]

@t2gran Could you include it in another format?

Yes, I posted a png instead now - hope that works better. I used gliffy for this diagram, I can provide the gliffy file as well if someone wants it.

[t2gran]

At yesterdays developer meeting we discussed namin conventions and after the meeting I did little bit of design - I just document it here, waiting to get to the point where we can do this part.

---

Updaters run with a context(editor) This is not implemented, but is our vision :-)

TransitModelEditor ctx = transitService().editTransitModel();

// To modify a trip we get the builder, change it, and call save() 
ctx.trip(id).withPrivateCode("CBP").save();

// A more common case would be to perform a change to a trip schedule on a given date
// We would like something like this. Here a combination of command and entity builders are used        
ctx.on(date).changePattern(patternMatcher).removeStop(stopId).save();
ctx.on(date).cancelTrip(tripMatcher).save();
ctx.on(date).updateTrip(matchId(tripId)).arrivalTime(stopId, arrTime).depatureTime(stopId, depTime).save();

// To update the model and make the changes visible to other threads
ctx.commit();

The editor can keep track of what is changed and perform cascading updates and reindexing for the part of the model which is changed.

[slvlirnoff]

Hey @t2gran , some updaters have incremental updates and some have full dataset that should replace previous updates. How would that work with this? Would it be possible to "rollback" only the changes made by a specific ctx?

Maybe a "reset" call on the editor could help resetting changes saved and committed previously. A full dataset updater would first call reset() on the existing ctx before adding new changes and committing these and an incremental one would not and create new changes, committing these on top of existing ones.

Side question, how would the editors interact with each others? For instance ctx1 edit and commit trip1. Will ctx2 changes to trip1 be applied to the updated trip1 or to the "original" version of trip1. If these are applied on top of the updated trip1, what happens if you want to "reset" the changes made by ctx1?

[t2gran]

Maybe a "reset" call on the editor could help resetting changes saved and committed previously. A full dataset updater would first call reset() on the existing ctx before adding new changes and committing these and an incremental one would not and create new changes, committing these on top of existing ones.

Something like this:

// Revert an earlier update by going back to the planed version and apply new changes
ctx.on(date).trip(id).revertToPlanned().withDelay(5m).save();  

Side question, how would the editors interact with each others? For instance ctx1 edit and commit trip1. Will ctx2 changes to trip1 be applied to the updated trip1 or to the "original" version of trip1. If these are applied on top of the updated trip1, what happens if you want to "reset" the changes made by ctx1?

Not decided jet. But, I my first thought is to do "optimistic locking", if an updater have applied a change to an object that is not present in the main model any more, then the change is dropped. Each updater only see its own context; Hence it can query the model and get back the its own changes, but not changes in other contexts or changes made to the model after the context was created.

[2martens]

I have a question: would this model refactor allow combining, for example, GTFS schedule data and SIRI SX and SIRI ET?

[leonardehrenfried]

That's already possible.

Once the static data is imported we don't know anymore if it came from Netex or GTFS (well, almost) and you can apply any realtime updater you want.

[2martens]

Somewhere I took the information that NeTEx/SIRI and GTFS/GTFS-RT are linked.

[leonardehrenfried]

You have to do a bit of conceptual mapping (trip/journey, stop/quay, ...) in your head but OTP is already full of these.

[github-actions[bot]]

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 30 days

[github-actions[bot]]

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 30 days

[github-actions[bot]]

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 30 days

[github-actions[bot]]

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 30 days

[github-actions[bot]]

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 30 days