Repository navigation
Upstream merge 2026-09-08 - #321
Merged
Merged
Conversation
getOrCreateServiceIdForDate mutates serviceIdsByDate's Set values in place. Any copy of a CalendarServiceData that only shallow-copies that map still shares those Set instances with the original, so a later mutation on one side is visible through the other. Add a proper copyOf() that copies the sets too.
Collapse the TripCalendars interface and its sole DefaultTripCalendars implementation into a single immutable TripCalendars class. There is no builder: every modification (merge, withServiceCode, initializeServiceCodes, getOrCreateServiceIdForDate) returns a new instance rather than mutating this one, so a caller holding a reference keeps seeing a stable value even while another reference evolves. A caller that needs a long-lived, continuously updated view can just hold a plain mutable field and reassign it as each method returns its result. getOrCreateServiceIdForDate takes an onUpdate callback rather than returning the new instance directly, since the common case (a date that's already been seen) is a pure read that produces no new instance at all — the callback is only invoked when something actually changed. The calendar data (service ids and the dates they run on) is stored directly as plain maps instead of wrapping a CalendarServiceData instance, since that was only ever used as an internal implementation detail here.
Hold the trip calendar directly as a field on TimetableRepository/ TimetableRepositorySnapshot instead of giving it its own repository handle and lifecycle. Every consumer that needs realtime timetable data already needs the calendar too, since new service ids for real-time-added trips must be visible in the same transaction as the trips that use them, so a separate handle only added indirection without decoupling anything. TransitRepository keeps the scheduled-time TripCalendars; DefaultTimetableRepository owns the realtime write-buffer/snapshot pair, and DefaultTransitService's getOrCreateServiceIdForDate creates new service ids directly on the write buffer when write access is available. Also switch AddTransitEntitiesToTimetable and various test fixtures from mutating the map returned by TransitRepository.getServiceCodes() directly to the new TransitRepository.putServiceCode(id, code), since the trip calendar's service-code map is no longer a bare mutable map exposed for arbitrary external mutation. Related to opentripplanner#7893.
…Date to the buffer getOrCreateServiceIdForDate is a mutation, and TransitService is documented as a read-only interface used by every read-only call site in the codebase, not just the 4 real-time updater handlers that actually need to create service ids. Remove it from TransitService/DefaultTransitService and call it directly on the mutable TimetableRepository write buffer instead, which all 4 call sites (NewTripHandler, DuplicatedTripHandler, AddedTripBuilder, ExtraCallTripBuilder) already have access to, mirroring how they already apply trip updates directly through the buffer. The "is this date within the transit service period" check that used to live in DefaultTransitService (via TransitRepository.transitFeedCovers) moves into TripCalendars.getOrCreateServiceIdForDate itself, using its own startDate()/endDate(). Since only in-range dates are ever added and purging never touches the calendar, that range is stable and self-contained, so the buffer needs no reference to TransitRepository to enforce it.
UnmodifiableCollectionsSerializer's write/copy path always converted the wrapped value to an ArrayList via a blind (Collection) cast, and its read/copy path assumed that deserialized ArrayList was already the concrete type create() needed. This works for COLLECTION/RANDOM_ACCESS_LIST (a List and a Collection are compatible enough), but breaks for MAP/SORTED_MAP (a Map is not a Collection, so the write-side cast throws ClassCastException) and would also break for SET/SORTED_SET/LIST on the read side (an ArrayList is not a Set, so that cast would throw too). Give each variant its own toIntermediate()/create() pair: toIntermediate() converts to a plain, Kryo-friendly representation appropriate to the type (ArrayList for collection-based variants, HashMap for map-based ones), and create() reconstructs the concrete collection/map type it needs from that representation before wrapping it back up as unmodifiable. Nothing in the codebase had ever put a plain Collections.unmodifiableMap/Set into the Kryo-serialized graph before, so this bug was latent until now.
Both are invariant once an instance is built, so compute them once in the constructor instead of rescanning serviceIdsByDate's whole key set on every call to startDate()/endDate() — including every call to getOrCreateServiceIdForDate, which checks both on every invocation.
The method never assigns service codes — it computes getServiceCodesRunningForDate() from service ids/dates and codes that must already be registered (it skips any service id without one). The old name suggested the opposite of what it does.
…e sets" This reverts commit 041b7ab.
…ds from OSM tags
…KryoSerializedObject Clarify that the intermediate representation is specifically for Kryo serialization, and that create() is really the reverse conversion.
…lendar/TripCalendars.java Co-authored-by: Sigurd Totland <sigurdtotland@gmail.com>
…zation-performance I will merge this since @vpaturet is not available today - and the only change I made is the same change that Vincent purposed.
…xb-logging Fix logging of JAXB type names
…update Update prettier and prettier-java versions
Split graph_builder.module.osm into subpackages
…/logging-dependencies Update logging dependencies to v2.0.19
…onnectivity Only add stop connectivity graph build module, when both street and transit exists
…-transport-modes Add missing carpool transit mode to the Transmodel API mode enum
# Conflicts: # application/src/main/java/org/opentripplanner/graph_builder/module/osm/ElevatorProcessor.java # application/src/main/java/org/opentripplanner/graph_builder/module/osm/parameters/OsmProcessingParameters.java
…r-ids Add `elevatorRefTags` build config option to extract elevator ids from OSM
…/guava-monorepo Update dependency com.google.guava:guava to v33.7.1-jre
…style Remove UnusedImports checkstyle rule
…/com.google.cloud-libraries-bom-26.x Update dependency com.google.cloud:libraries-bom to v26.88.0
miles-grant-ibigroup
approved these changes
Sep 10, 2026
miles-grant-ibigroup
left a comment
miles-grant-ibigroup
left a comment
There was a problem hiding this comment.
Everything is still functioning!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There is nothing immediately needed for you guys, but the diff is already enormous, so lets run the smoke test and merge sooner rather than later.