Repository navigation
Upstream merge 2026-09- 16 - #322
Merged
Merged
Conversation
Loads GBFS geofencing zones during the graph build rather than through the runtime updater, avoiding the cost of re-fetching and re-applying them on every restart. It takes a GBFS v3 manifest and discovers which systems to load, skipping any whose gbfs.json does not publish a geofencing_zones feed, so a blanket "permanent" default picks up only the systems that have zones. There is no feature flag: the sandbox is activated by the presence of its url, matching vehicleRentalServiceDirectory. Adds the permanent phase to geofencingZones, which until now had nothing to implement it, and persists the rental repository in the graph so the zones survive to the serve phase. The spatial indices are transient - JTS STRtree and PreparedGeometry caches do not survive Kryo - so the raw zones are stored and the indices rebuilt lazily on first access. Vehicles and stations remain realtime data, so a vehicle rental updater is still required. Rental vertices only exist at runtime, so the updater seeds their initial zones by reading through the repository rather than from an index of its own, filtered to each vertex's network. Note that a GBFS updater configured directly under "updaters" does not read the shared gbfs section, so enabling geofencingZones on one for a network also built here applies the zones twice.
The sandbox applies zones during the graph build, so both the zones and the boundary markers they leave on vertices have to survive serialization. Neither did. GeofencingZone was not Serializable. Nothing had noticed because until now the only zones in memory were created by updaters after the graph was loaded, so no zone was ever written. Registering them from the graph build made the repository fail to serialize. Vertex.geofencingBoundaries was transient for the same reason, and would have failed silently rather than loudly: the build would apply the boundaries, saving the graph would drop them, and the router would then behave as though no zone had a boundary. The spatial indices stay transient - JTS STRtree and PreparedGeometry caches do not survive Kryo - and are rebuilt lazily from the stored zones.
…ation The repository is part of the serialized graph, so the requirement belongs on the interface stored in SerializedGraphObject - matching WorldEnvelopeRepository, StreetRepository and StreetDetailsRepository - rather than on the one class that happens to implement it today.
# Conflicts: # application/src/main/java/org/opentripplanner/standalone/config/BuildConfig.java
- Take the geofencing_zones feed name from the GBFS model instead of a string literal. - Drop the debug logging in network selection; the INFO line per applied network already shows what was selected. - Derive zonesChanged from its precondition rather than assigning it inside the block it guards. - Move the sandbox doc's changelog to the bottom.
Reading a non-HTTP scheme as a local file is behaviour of the shared OtpHttpClient, useful for testing but not something to advertise as a feature of this sandbox. Support is unchanged.
The graph builder held DefaultVehicleRentalRepository and the module downcast to it, because only the implementation had the setter that also kept the raw zones for serialization. Give the interface a single registration method taking the zones, so both callers pass them and the sandbox no longer depends on the implementation. Whether a registration reaches the graph now depends on when it happens rather than which overload is called: only the graph build runs before the graph is written. Renames serializedZones to zonesByNetwork, which is what it now holds.
Replace the Externalizable fallback for Trove's TIntObjectHashMap with a dedicated Kryo serializer, following the pattern of conveyal kryo-tools' TIntIntHashMapSerializer. The Externalizable path routes every entry through the KryoObjectInput/KryoObjectOutput adapter and serializes Trove-internal bookkeeping; writing the entries directly is faster and smaller (measured -52 MB / -2.5% on the Norway graph with empirical delay data, ~290k maps). This changes the serialized wire format.
Both tests built the URI from a working-directory-relative path. The fixture sits at the classpath root rather than beside either test, so the loader is given the rooted path. The missing-file and relative-file-URI cases keep their literal paths: the loader throws when a resource does not exist, and the relative URI is what that test exercises.
Same wording as the geofencing sandbox, which copied it from here. The behaviour is unchanged; only the documentation stops presenting a testing affordance as a feature.
Renames GeofencingZoneScope to GeofencingZoneOtpPhase, PERMANENT to GRAPH_BUILD and REALTIME to SERVE, so the values name the phase a deployment chooses rather than the kind of graph change that results. Scope stays the vocabulary for the latter, and the enum documents how the two correspond. The config key applyGeofencingZones is unchanged; its values become graph-build and serve. Nothing released used the old spellings, so the migration guide maps the old boolean straight to the new names.
The index the applier returned was never read during the graph build: the build wires GeofencingZoneService.EMPTY, the applier computes boundary markers from each zone's geometry rather than the index, and both callers passed it straight to the repository. In the build-and-save path it was then discarded at serialization and rebuilt on load. Registration now takes the zones alone and the repository indexes them, so the serialized state is just the zones and index construction happens in one place. Drops GeofencingZoneApplierResult, which had one field left, and the write-only latestZoneIndex field in the updater.
The repository was serialized purely to carry geofencing zones out of the build: nothing in the build read it, and at serve time its stored zones were read exactly once, to rebuild the indices Kryo could not carry. Being serialized is also what forced the transient index, the volatile field and the double-checked locking around it. The zones now travel on the Graph, which was already serialized, so SerializedGraphObject loses its repository field rather than gaining a new one. The repository is created in the serve phase from those zones and holds indices alone. It is created on first injection, late enough that a combined build-and-serve run sees what the build just wrote. Bumps the serialization version id: the graph no longer contains a vehicle rental repository.
The manifest is named by build-config.json, so failing to read it is a misconfigured or broken build rather than a data problem, and continuing would produce a graph silently missing every network's zones. An individual network is third-party data the manifest references, so a network that cannot be read is added to the build issue report and skipped. Nothing recovers it later - a network in the graph-build phase is skipped by the updater - so the report is the only signal that its restrictions are absent, and the sandbox doc now says so. Networks that are simply unconfigured or in another phase stay ordinary log lines; with includeUnlistedNetworks off they are the common case and would bury the real failures.
…geofencing # Conflicts: # pom.xml
# Conflicts: # application/src/test/java/org/opentripplanner/netex/mapping/NetexMapperTest.java
…interface Building the record used to force every dependency (RoutingService, RealtimeVehicleService, NearbyPlaceFinder, ...) to be constructed as soon as any GraphQL data fetcher touched the context, even if the query never used most of them. GtfsGraphQLRequestContext is now an interface; the production implementation delegates each accessor to the enclosing RequestScopedFactory, so a dependency is only resolved on first actual use. Tests that need a plain, hand-built context use the new TestGtfsGraphQLRequestContext record.
…nterface Same motivation as the GTFS context: the class's constructor forced every dependency to be resolved as soon as any Transmodel data fetcher touched the context. TransmodelRequestContext is now an interface; the production implementation delegates each getter to the enclosing RequestScopedFactory, so a dependency is only resolved on first actual use. Tests that need a plain, hand-built context use the new TestTransmodelRequestContext class. # Conflicts: # application/src/test/java/org/opentripplanner/apis/transmodel/mapping/TripRequestMapperTest.java
Matches the naming already used for GtfsGraphQLRequestContext and the OTPFeature.TransmodelGraphQlApi flag / TransmodelGraphQLSchema — the Transmodel API is a GraphQL API just like GTFS, so its request context should say so too. # Conflicts: # application/src/test/java/org/opentripplanner/apis/transmodel/mapping/TripRequestMapperTest.java fix: correct stale @link in DaggerTransmodelGraphQLRequestContext javadoc Leftover reference to the old TransmodelRequestContext name.
… follow Java naming conventions
…port-mode Add snowAndIce to the Transmodel TransportMode enum
…lease-age Add minimum release age to renovate automerges
…memory Limit growth of NeTEx stop time index
…ld-request-services-on-demand Turn GraphQL request contexts into lazy, request-scoped interfaces
…m-svc Remove .idea files from version control
…/maven-plugins Update Maven plugins
…l-template
Expand {from}/{to} placeholders in the carpool booking URL
…/gtfs-api-code-generation Update dependency @graphql-codegen/cli to v7.4.0
…-comparison Remove 'Version Comparison' documentation page
…/highly-trusted-dependencies-(patch) Update highly trusted dependencies (patch)
Vitest 5 resolves the -c/config path relative to --root instead of the cwd, so `vitest --root src/` no longer finds client/vite.config.ts. Without it, the jsdom test environment configured there is never applied, so tests run under node and fail with "document is not defined". Point the test/coverage scripts at the config explicitly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018sSwbgmVFJz2sao3UFPX65
…/major-vitest-monorepo Update vitest monorepo to v5
…-api Deprecate serviceId in GTFS API and stop populating it
miles-grant-ibigroup
approved these changes
Sep 16, 2026
miles-grant-ibigroup
left a comment
miles-grant-ibigroup
left a comment
There was a problem hiding this comment.
Seems like we do use it, but this somehow doesn't break anything
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
7 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.
☠️ Attention: this sets the GraphQL field
Trip.serviceIdtonullas it is blocking a calendar refactoring. ☠️@binh-dam-ibigroup and @alec-georgoff said that this is not used by Arcadis, so lets see if this holds up in practise.