Skip to content
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

Fix stop time fetching in added or replacement trips #6245

Open
wants to merge 19 commits into
base: dev-2.x
Choose a base branch
from

Conversation

miklcct
Copy link
Contributor

@miklcct miklcct commented Nov 8, 2024

Summary

This fixes problems when reading stop times for added or replacement trips.

The logic of fetching trip times is moved into TransitService. The list is optional because it can be empty if the trip doesn't run at all on a specified date (in contrast, an empty list would theoretically mean that the trip does run on the date, but it doesn't make any call for the whole journey).

Also, the behaviour of invalid date is changed to produce an error instead of returning null silently in the GTFS GraphQL API, since null is a valid return value meaning the trip doesn't run.

Issue

Fixes #6088
Fixes #6242

Unit tests

Unit tests are added into TransitService for the new methods, for both existing and added trips.

GraphQL Integration tests are added for one added trip and one replacement trip. This is the first time real-time updates can be integration tested.

Documentation

None needed

@miklcct miklcct requested a review from a team as a code owner November 8, 2024 15:52
@miklcct miklcct force-pushed the fix-departureArrivalStopTime-npe branch from 36ef65d to ed3bb9b Compare November 8, 2024 15:53
Copy link

codecov bot commented Nov 8, 2024

Codecov Report

Attention: Patch coverage is 63.38028% with 26 lines in your changes missing coverage. Please review.

Project coverage is 69.86%. Comparing base (211aafb) to head (4b50129).

Files with missing lines Patch % Lines
...java/org/opentripplanner/model/TripTimeOnDate.java 9.09% 3 Missing and 7 partials ⚠️
...entripplanner/apis/gtfs/datafetchers/TripImpl.java 87.09% 3 Missing and 1 partial ⚠️
...model/model/timetable/DatedServiceJourneyType.java 0.00% 4 Missing ⚠️
...transmodel/model/timetable/ServiceJourneyType.java 20.00% 4 Missing ⚠️
...planner/transit/service/DefaultTransitService.java 80.00% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #6245      +/-   ##
=============================================
+ Coverage      69.75%   69.86%   +0.10%     
- Complexity     17747    17775      +28     
=============================================
  Files           2010     2009       -1     
  Lines          75948    75933      -15     
  Branches        7784     7785       +1     
=============================================
+ Hits           52979    53048      +69     
+ Misses         20260    20166      -94     
- Partials        2709     2719      +10     

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

@miklcct miklcct force-pushed the fix-departureArrivalStopTime-npe branch 3 times, most recently from 428a549 to df345c2 Compare November 8, 2024 16:08
@miklcct miklcct force-pushed the fix-departureArrivalStopTime-npe branch from 39ca3c3 to d3d283f Compare November 14, 2024 18:32
Comment on lines +292 to +302
public boolean equals(Object o) {
if (o == null || getClass() != o.getClass()) return false;
TripTimeOnDate that = (TripTimeOnDate) o;
return (
stopIndex == that.stopIndex &&
midnight == that.midnight &&
Objects.equals(tripTimes, that.tripTimes) &&
Objects.equals(tripPattern, that.tripPattern) &&
Objects.equals(serviceDate, that.serviceDate)
);
}
Copy link
Member

Choose a reason for hiding this comment

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

What is this used for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For testing if they are equal

Copy link
Member

Choose a reason for hiding this comment

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

In a test or in main code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For a unit test

@miklcct
Copy link
Contributor Author

miklcct commented Nov 23, 2024

I am going to wait for #5393 first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants