Skip to content

Conversation

@vpaturet
Copy link
Contributor

@vpaturet vpaturet commented Nov 14, 2025

Summary

This is a fix for #7055: it restores the fallback mechanism in case the provided place id is not found in the stop repository.

Issue

Closes #7055

Unit tests

Added unit test

Documentation

No

Changelog

skip

When a GenericLocation contains both a stopId that doesn't exist in the
graph and valid coordinates, the system should fall back to using the
coordinates to create a temporary vertex. This was the behavior before
PR opentripplanner#6972 (temporary-vertex-refactor).

The regression was caused by mutually exclusive if/else if logic in
LinkingContextFactory.getStreetVerticesForLocation(). When location.stopId
was present but not found in the graph, the method would return an empty
set without checking if coordinates were available.

This commit restores the fallback behavior by checking if results are
empty after the stop ID lookup and, if coordinates are available, using
them to create a vertex. This allows robust queries that provide both
place IDs and coordinates for redundancy.

Fixes the issue where GraphQL trip queries with non-existing place IDs
and valid coordinates would return empty results instead of using the
coordinate fallback.
Add test case that verifies when a GenericLocation contains both a
stopId that doesn't exist in the graph and valid coordinates, the
system falls back to using the coordinates to create a temporary
vertex rather than throwing a RoutingValidationException.

This test documents the expected behavior that was broken by PR opentripplanner#6972
and is now fixed.
@vpaturet vpaturet force-pushed the fix-coordinate-fallback branch from b8469ee to 7002ae6 Compare November 14, 2025 10:41
@vpaturet vpaturet added the Entur Test This is currently being tested at Entur label Nov 14, 2025
@codecov
Copy link

codecov bot commented Nov 14, 2025

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 72.49%. Comparing base (772ae72) to head (e193e24).
⚠️ Report is 154 commits behind head on dev-2.x.

Files with missing lines Patch % Lines
...planner/routing/linking/LinkingContextFactory.java 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #7056      +/-   ##
=============================================
+ Coverage      72.23%   72.49%   +0.25%     
- Complexity     20169    20456     +287     
=============================================
  Files           2188     2206      +18     
  Lines          81186    82240    +1054     
  Branches        8141     8229      +88     
=============================================
+ Hits           58645    59619     +974     
- Misses         19667    19699      +32     
- Partials        2874     2922      +48     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vpaturet vpaturet changed the title Fix coordinate fallback Fix coordinate fallback when linking locations in the street graph Nov 14, 2025
@vpaturet vpaturet marked this pull request as ready for review November 14, 2025 12:13
@vpaturet vpaturet requested a review from a team as a code owner November 14, 2025 12:13
@vpaturet
Copy link
Contributor Author

Tested this in our staging environment, this fixes the problem that users reported.

Comment on lines 315 to 321
var fromVertices = linkingContext.findVertices(from);
assertThat(fromVertices).hasSize(1);
assertThat(fromVertices).isNotEmpty();

var toVertices = linkingContext.findVertices(to);
assertThat(toVertices).hasSize(1);
assertThat(toVertices).isNotEmpty();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this you might want to check that these vertices are also of type TemporaryStreetLocation as stop vertices might also end up in these collection.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test updated.

@optionsome optionsome added the !Bug Apply to issues describing a bug and PRs witch fixes it. label Nov 18, 2025
@t2gran t2gran added this to the 2.9 (next release) milestone Nov 19, 2025
@vpaturet vpaturet requested a review from optionsome November 19, 2025 08:58
@vpaturet vpaturet merged commit a759fd9 into opentripplanner:dev-2.x Nov 19, 2025
8 checks passed
t2gran pushed a commit that referenced this pull request Nov 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

!Bug Apply to issues describing a bug and PRs witch fixes it. Entur Test This is currently being tested at Entur

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Empty trip results with non-existing place IDs

4 participants