Skip to content

Fix removal of old vehicle positions - #6523

Merged
leonardehrenfried merged 10 commits into
opentripplanner:dev-2.xfrom
ibi-group:clear-positions
Mar 14, 2025
Merged

leonardehrenfried merged 10 commits into
opentripplanner:dev-2.xfrom
ibi-group:clear-positions

Conversation

@leonardehrenfried

@leonardehrenfried leonardehrenfried commented Mar 10, 2025 •

Copy link
Copy Markdown
Member

Summary

The real time vehicle service does not correctly remove the data from the previous download which this PR fixes.

It introduces copy-on-write semantics for the service rather than explicit removal by the calling code, reducing the number of places where mutable state is handled.

It also adds a new GraphQL property so that the last update is now expressed as an ISO date time rather than number of seconds since the epoch.

Issue

n/a

Unit tests

Added.

Documentation

Javadoc

cc @whitneys-pm

@leonardehrenfried leonardehrenfried added the !Bug Apply to issues describing a bug and PRs witch fixes it. label Mar 10, 2025
@leonardehrenfried
leonardehrenfried requested a review from a team as a code owner March 10, 2025 09:56
@codecov

codecov Bot commented Mar 10, 2025 •

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 90.90909% with 2 lines in your changes missing coverage. Please review.

Project coverage is 70.20%. Comparing base (94950f1) to head (9892fe6).
Report is 54 commits behind head on dev-2.x.

Files with missing lines Patch % Lines
...ehicle_position/PollingVehiclePositionUpdater.java 0.00% 2 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #6523      +/-   ##
=============================================
+ Coverage      70.17%   70.20%   +0.03%     
- Complexity     18312    18324      +12     
=============================================
  Files           2082     2085       +3     
  Lines          77214    77291      +77     
  Branches        7831     7834       +3     
=============================================
+ Hits           54188    54266      +78     
+ Misses         20256    20251       -5     
- Partials        2770     2774       +4     

☔ 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.

* overwrite each other.
*/
void setRealtimeVehicles(TripPattern pattern, List<RealtimeVehicle> updates);
void setRealtimeVehicles(String feedId, Multimap<TripPattern, RealtimeVehicle> updates);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Update javadoc, this no longer updates vehicles just for one pattern but rather for the whole feed? Perhaps also rename the method so it's clear what this updates.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I've updated the Javadoc but I'm unsure what a better name is. Maybe setRealtimeVehiclesForFeed?

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The fix seems to work well, thank you, and code is clean.

@leonardehrenfried
leonardehrenfried merged commit 273f879 into opentripplanner:dev-2.x Mar 14, 2025
t2gran pushed a commit that referenced this pull request Mar 14, 2025
@leonardehrenfried
leonardehrenfried deleted the clear-positions branch March 14, 2025 14:07
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants