-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add GBFS geofencing zones sandbox for graph build time #7155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
testower
wants to merge
11
commits into
opentripplanner:dev-2.x
Choose a base branch
from
entur:poc/sandbox-geofencing-zones-in-graph-builder
base: dev-2.x
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Add GBFS geofencing zones sandbox for graph build time #7155
testower
wants to merge
11
commits into
opentripplanner:dev-2.x
from
entur:poc/sandbox-geofencing-zones-in-graph-builder
Conversation
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
312368c to
d56b9ba
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #7155 +/- ##
=============================================
- Coverage 72.12% 71.97% -0.15%
- Complexity 21029 21049 +20
=============================================
Files 2289 2300 +11
Lines 84917 85250 +333
Branches 8466 8497 +31
=============================================
+ Hits 61247 61361 +114
- Misses 20699 20915 +216
- Partials 2971 2974 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
14fadbc to
550c1a9
Compare
05cba00 to
0788d82
Compare
Implement a new sandbox extension that fetches GBFS geofencing zones during graph build and applies travel restrictions to street edges. Key components: - GbfsGeofencingRepository: stores geofencing zones by system ID - GbfsClient: fetches GBFS v3.0 discovery and geofencing_zones.json - GeofencingZoneMapper: converts GBFS GeoJSON to OTP GeofencingZone - GeofencingZoneApplier: applies rental restrictions to street edges - GbfsGeofencingGraphBuilder: orchestrates the build process Configuration via build-config.json under gbfsGeofencing with list of feeds (url, headers, and optional network override). The zones are serialized with the graph and used at runtime to restrict vehicle rentals in no-go and slow zones.
Update method-level JavaDoc to explain "why" rather than "what", following OTP Codestyle.md guidelines. Each method now documents its purpose, caller context, and role in the system rather than simply restating the method name.
Handle the case where fetching the GBFS discovery file returns null, which can happen with network errors or invalid responses. Without this check, the code would throw an NPE when accessing getData(). Logs a warning to help operators identify misconfigured feed URLs.
The language parameter was declared and documented but never used. The GBFS geofencing module only supports GBFS v3.0, which has a flat feed structure with no language dimension (unlike v2.x). Removing this prevents users from configuring a property that has no effect.
Phase 1 of thread-safety improvements: Make the repository immutable after construction instead of using synchronization. Changes: - Remove mutable methods from GbfsGeofencingRepository interface - Make DefaultGbfsGeofencingRepository immutable with constructor-based initialization using List.copyOf() for defensive copying - Add new GbfsGeofencingRepositoryBuilder for use during graph build This approach ensures thread-safety through immutability rather than synchronization overhead, matching OTP's pattern for build-time data.
Add null-safety checks for features with missing or empty rules in the GBFS geofencing zone mapper. The code previously accessed `getRules().get(0)` directly, which would throw NPE if properties or rules were null/empty. Changes: - Add hasValidRules() predicate method to validate features - Filter out features with null properties or null/empty rules - Log warnings for skipped features to aid debugging This follows the existing defensive coding pattern used in the featureName() method and the null geometry filter.
Use Map.copyOf() to create an immutable copy of httpHeaders in the compact constructor, preventing mutation after construction. This follows the project's immutability guidelines for record types.
Phase 2 of thread-safety implementation for GbfsGeofencingRepository. Updates the Dagger dependency injection wiring to use the new GbfsGeofencingRepositoryBuilder instead of the mutable repository: - GbfsGeofencingRepositoryModule now provides the builder - GbfsGeofencingGraphBuilderModule injects the builder - GbfsGeofencingGraphBuilder uses builder to create immutable repository - GraphBuilderFactory takes builder via @BindsInstance - LoadApplicationFactory exposes gbfsGeofencingRepositoryBuilder() - ConstructApplication passes builder to GraphBuilder - LoadApplication handles both repository (from serialized graph) and builder (for fresh builds) The repository is now built immutably after graph construction completes, with a getRepository() method to retrieve it.
Add repository caching to GbfsGeofencingRepositoryBuilder and update ConstructApplication to properly retrieve the repository for both fresh builds and deserialized graphs. Key changes: - GbfsGeofencingRepositoryBuilder now caches the built repository - Added getBuiltRepository() method for post-build retrieval - ConstructApplication.gbfsGeofencingRepository() now correctly falls back to builder when repository is not available from Dagger This completes the serialization flow: - Fresh build: builder creates repository, serialized to graph file - Graph load: repository deserialized and injected via Dagger
When multiple GBFS providers have business areas, the code incorrectly used only the first provider's network ID for all business area boundaries. This caused vehicles from non-first providers to not be blocked at their own business area boundaries. The fix groups business areas by network (feed ID) before processing, then creates a separate BusinessAreaBorder for each network. This ensures each provider's vehicles are correctly restricted at their own business area boundaries.
0788d82 to
3ef56cb
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
+Bump Serialization Id
Add this label if you want the serialization id automatically bumped after merging the PR
Entur Test
This is currently being tested at Entur
+Sandbox
This will be implemented as a Sandbox feature
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.
Summary
Creates a sandbox that processes GBFS geofencing zones at graph build time
Issue
Closes #7120
Unit tests
TBD
Documentation
TBD
Bumping the serialization version id
Yes this needs a new serialization version because we added RentalRestrictionExtension and GeofencingZone to the graph.