-
Notifications
You must be signed in to change notification settings - Fork 1k
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
miklcct
wants to merge
25
commits into
opentripplanner:dev-2.x
Choose a base branch
from
Jnction:fix-departureArrivalStopTime-npe
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.
Open
Changes from 23 commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
5fcdf4a
add test for #6242
miklcct 782730d
add Nullable annotation
miklcct 98c5d13
refactor arrival and departure stop time
miklcct 38c3a5a
fix #6242
miklcct 8b4b0fe
delete TripTimeOnDateHelper and move the logic to DefaultTransitService
miklcct 816e9c0
move scheduled trip time fetching logic to TransitService
miklcct 9df81a3
use trip time logic in TransitService from GTFS GraphQL TripImpl
miklcct 8cf5e9b
reformat code
miklcct d6da964
do not silently return null in case of invalid date
miklcct cd74120
refactor date parsing
miklcct 29db2ef
reformat code
miklcct 38dd2aa
add test for new transit service methods
miklcct d3d283f
use optional for trip time on dates
miklcct a7dc78e
Merge branch 'dev-2.x' into fix-departureArrivalStopTime-npe
miklcct 5ab09f4
Merge branch 'dev-2.x' into fix-departureArrivalStopTime-npe
miklcct 7ad5e63
update test for latest dev
miklcct bdc6c0a
stub the real time information in GraphQL intergration test
miklcct f6c7cb7
Merge branch 'dev-2.x' into fix-departureArrivalStopTime-npe
miklcct 4b50129
format JSON files
miklcct 512f30b
Merge branch 'dev-2.x' into fix-departureArrivalStopTime-npe
miklcct 15ea30a
Merge branch 'dev-2.x' into fix-departureArrivalStopTime-npe
miklcct f72b9a4
fix test conflict
miklcct 236dc6e
apply review feedback
miklcct 4e542a7
TripUpdateBuilder is no longer needed
miklcct 7a85d1f
Merge branch 'dev-2.x' into fix-departureArrivalStopTime-npe
miklcct File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,8 @@ | |
import java.util.ArrayList; | ||
import java.util.Comparator; | ||
import java.util.List; | ||
import java.util.Objects; | ||
import javax.annotation.Nullable; | ||
import org.opentripplanner.framework.i18n.I18NString; | ||
import org.opentripplanner.transit.model.network.TripPattern; | ||
import org.opentripplanner.transit.model.site.StopLocation; | ||
|
@@ -31,7 +33,10 @@ public class TripTimeOnDate { | |
private final int stopIndex; | ||
// This is only needed because TripTimes has no reference to TripPattern | ||
private final TripPattern tripPattern; | ||
|
||
@Nullable | ||
private final LocalDate serviceDate; | ||
|
||
private final long midnight; | ||
|
||
public TripTimeOnDate(TripTimes tripTimes, int stopIndex, TripPattern tripPattern) { | ||
|
@@ -46,8 +51,8 @@ public TripTimeOnDate( | |
TripTimes tripTimes, | ||
int stopIndex, | ||
TripPattern tripPattern, | ||
LocalDate serviceDate, | ||
Instant midnight | ||
@Nullable LocalDate serviceDate, | ||
@Nullable Instant midnight | ||
) { | ||
this.tripTimes = tripTimes; | ||
this.stopIndex = stopIndex; | ||
|
@@ -59,9 +64,15 @@ public TripTimeOnDate( | |
/** | ||
* Must pass in both Timetable and Trip, because TripTimes do not have a reference to | ||
* StopPatterns. | ||
* | ||
* @return null if the trip does not exist in the timetable | ||
*/ | ||
@Nullable | ||
public static List<TripTimeOnDate> fromTripTimes(Timetable table, Trip trip) { | ||
TripTimes times = table.getTripTimes(trip); | ||
if (times == null) { | ||
return null; | ||
} | ||
miklcct marked this conversation as resolved.
Show resolved
Hide resolved
|
||
List<TripTimeOnDate> out = new ArrayList<>(); | ||
for (int i = 0; i < times.getNumStops(); ++i) { | ||
out.add(new TripTimeOnDate(times, i, table.getPattern())); | ||
|
@@ -74,14 +85,20 @@ public static List<TripTimeOnDate> fromTripTimes(Timetable table, Trip trip) { | |
* StopPatterns. | ||
* | ||
* @param serviceDate service day to set, if null none is set | ||
* @return null if the trip does not exist in the timetable | ||
*/ | ||
|
||
@Nullable | ||
public static List<TripTimeOnDate> fromTripTimes( | ||
Timetable table, | ||
Trip trip, | ||
LocalDate serviceDate, | ||
Instant midnight | ||
) { | ||
TripTimes times = table.getTripTimes(trip); | ||
if (times == null) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any code path that leads to this condition after your fix is applied? |
||
return null; | ||
} | ||
List<TripTimeOnDate> out = new ArrayList<>(); | ||
for (int i = 0; i < times.getNumStops(); ++i) { | ||
out.add(new TripTimeOnDate(times, i, table.getPattern(), serviceDate, midnight)); | ||
|
@@ -310,4 +327,22 @@ public BookingInfo getPickupBookingInfo() { | |
public BookingInfo getDropOffBookingInfo() { | ||
return tripTimes.getDropOffBookingInfo(stopIndex); | ||
} | ||
|
||
@Override | ||
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) | ||
); | ||
} | ||
miklcct marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hash(tripTimes, stopIndex, tripPattern, serviceDate, midnight); | ||
} | ||
} |
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any code path that leads to this condition after your fix is applied?
(I do not see any unit test that touches this line)