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 24 commits into
base: dev-2.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
5fcdf4a
add test for #6242
miklcct Nov 8, 2024
782730d
add Nullable annotation
miklcct Nov 8, 2024
98c5d13
refactor arrival and departure stop time
miklcct Nov 8, 2024
38c3a5a
fix #6242
miklcct Nov 8, 2024
8b4b0fe
delete TripTimeOnDateHelper and move the logic to DefaultTransitService
miklcct Nov 14, 2024
816e9c0
move scheduled trip time fetching logic to TransitService
miklcct Nov 14, 2024
9df81a3
use trip time logic in TransitService from GTFS GraphQL TripImpl
miklcct Nov 14, 2024
8cf5e9b
reformat code
miklcct Nov 14, 2024
d6da964
do not silently return null in case of invalid date
miklcct Nov 14, 2024
cd74120
refactor date parsing
miklcct Nov 14, 2024
29db2ef
reformat code
miklcct Nov 14, 2024
38dd2aa
add test for new transit service methods
miklcct Nov 14, 2024
d3d283f
use optional for trip time on dates
miklcct Nov 14, 2024
a7dc78e
Merge branch 'dev-2.x' into fix-departureArrivalStopTime-npe
miklcct Nov 14, 2024
5ab09f4
Merge branch 'dev-2.x' into fix-departureArrivalStopTime-npe
miklcct Nov 26, 2024
7ad5e63
update test for latest dev
miklcct Nov 26, 2024
bdc6c0a
stub the real time information in GraphQL intergration test
miklcct Nov 27, 2024
f6c7cb7
Merge branch 'dev-2.x' into fix-departureArrivalStopTime-npe
miklcct Nov 27, 2024
4b50129
format JSON files
miklcct Nov 27, 2024
512f30b
Merge branch 'dev-2.x' into fix-departureArrivalStopTime-npe
miklcct Dec 4, 2024
15ea30a
Merge branch 'dev-2.x' into fix-departureArrivalStopTime-npe
miklcct Dec 17, 2024
f72b9a4
fix test conflict
miklcct Dec 17, 2024
236dc6e
apply review feedback
miklcct Dec 19, 2024
4e542a7
TripUpdateBuilder is no longer needed
miklcct Jan 3, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import graphql.schema.DataFetcher;
import graphql.schema.DataFetchingEnvironment;
import java.text.ParseException;
import java.time.Instant;
import java.time.LocalDate;
import java.time.ZoneId;
import java.util.ArrayList;
Expand All @@ -13,6 +12,7 @@
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;
import javax.annotation.Nullable;
import org.locationtech.jts.geom.Geometry;
import org.locationtech.jts.geom.LineString;
import org.opentripplanner.apis.gtfs.GraphQLRequestContext;
Expand All @@ -23,7 +23,6 @@
import org.opentripplanner.apis.gtfs.mapping.BikesAllowedMapper;
import org.opentripplanner.apis.gtfs.model.TripOccupancy;
import org.opentripplanner.apis.support.SemanticHash;
import org.opentripplanner.model.Timetable;
import org.opentripplanner.model.TripTimeOnDate;
import org.opentripplanner.routing.alertpatch.EntitySelector;
import org.opentripplanner.routing.alertpatch.TransitAlert;
Expand All @@ -36,7 +35,6 @@
import org.opentripplanner.transit.model.site.StopLocation;
import org.opentripplanner.transit.model.timetable.Direction;
import org.opentripplanner.transit.model.timetable.Trip;
import org.opentripplanner.transit.model.timetable.TripTimes;
import org.opentripplanner.transit.service.TransitService;
import org.opentripplanner.utils.time.ServiceDateUtils;

Expand Down Expand Up @@ -132,38 +130,13 @@ public DataFetcher<Iterable<TransitAlert>> alerts() {
@Override
public DataFetcher<TripTimeOnDate> arrivalStoptime() {
return environment -> {
try {
TransitService transitService = getTransitService(environment);
TripPattern tripPattern = getTripPattern(environment);
if (tripPattern == null) {
return null;
}
Timetable timetable = tripPattern.getScheduledTimetable();

TripTimes triptimes = timetable.getTripTimes(getSource(environment));
LocalDate serviceDate = null;
Instant midnight = null;

var args = new GraphQLTypes.GraphQLTripArrivalStoptimeArgs(environment.getArguments());
if (args.getGraphQLServiceDate() != null) {
serviceDate = ServiceDateUtils.parseString(args.getGraphQLServiceDate());
midnight =
ServiceDateUtils
.asStartOfService(serviceDate, transitService.getTimeZone())
.toInstant();
}

return new TripTimeOnDate(
triptimes,
triptimes.getNumStops() - 1,
tripPattern,
serviceDate,
midnight
);
} catch (ParseException e) {
//Invalid date format
return null;
}
var serviceDate = getOptionalServiceDateArgument(environment);
var trip = getSource(environment);
var transitService = getTransitService(environment);
var stopTimes = serviceDate
.map(date -> transitService.getTripTimeOnDates(trip, date))
.orElseGet(() -> transitService.getScheduledTripTimes(trip));
return stopTimes.map(List::getLast).orElse(null);
};
}

Expand All @@ -180,32 +153,13 @@ public DataFetcher<String> blockId() {
@Override
public DataFetcher<TripTimeOnDate> departureStoptime() {
return environment -> {
try {
TransitService transitService = getTransitService(environment);
TripPattern tripPattern = getTripPattern(environment);
if (tripPattern == null) {
return null;
}
Timetable timetable = tripPattern.getScheduledTimetable();

TripTimes triptimes = timetable.getTripTimes(getSource(environment));
LocalDate serviceDate = null;
Instant midnight = null;

var args = new GraphQLTypes.GraphQLTripDepartureStoptimeArgs(environment.getArguments());
if (args.getGraphQLServiceDate() != null) {
serviceDate = ServiceDateUtils.parseString(args.getGraphQLServiceDate());
midnight =
ServiceDateUtils
.asStartOfService(serviceDate, transitService.getTimeZone())
.toInstant();
}

return new TripTimeOnDate(triptimes, 0, tripPattern, serviceDate, midnight);
} catch (ParseException e) {
//Invalid date format
return null;
}
var serviceDate = getOptionalServiceDateArgument(environment);
var trip = getSource(environment);
var transitService = getTransitService(environment);
var stopTimes = serviceDate
.map(date -> transitService.getTripTimeOnDates(trip, date))
.orElseGet(() -> transitService.getScheduledTripTimes(trip));
return stopTimes.map(List::getFirst).orElse(null);
};
}

Expand Down Expand Up @@ -300,43 +254,23 @@ public DataFetcher<Iterable<Object>> stops() {

@Override
public DataFetcher<Iterable<TripTimeOnDate>> stoptimes() {
return environment -> {
TripPattern tripPattern = getTripPattern(environment);
if (tripPattern == null) {
return List.of();
}
return TripTimeOnDate.fromTripTimes(
tripPattern.getScheduledTimetable(),
getSource(environment)
);
};
return environment ->
getTransitService(environment).getScheduledTripTimes(getSource(environment)).orElse(null);
}

@Override
public DataFetcher<Iterable<TripTimeOnDate>> stoptimesForDate() {
return environment -> {
try {
TransitService transitService = getTransitService(environment);
Trip trip = getSource(environment);
var args = new GraphQLTypes.GraphQLTripStoptimesForDateArgs(environment.getArguments());

ZoneId timeZone = transitService.getTimeZone();
LocalDate serviceDate = args.getGraphQLServiceDate() != null
? ServiceDateUtils.parseString(args.getGraphQLServiceDate())
: LocalDate.now(timeZone);

TripPattern tripPattern = transitService.getPatternForTrip(trip, serviceDate);
// no matching pattern found
if (tripPattern == null) {
return List.of();
}

Instant midnight = ServiceDateUtils.asStartOfService(serviceDate, timeZone).toInstant();
Timetable timetable = transitService.getTimetableForTripPattern(tripPattern, serviceDate);
return TripTimeOnDate.fromTripTimes(timetable, trip, serviceDate, midnight);
} catch (ParseException e) {
return null; // Invalid date format
}
TransitService transitService = getTransitService(environment);
Trip trip = getSource(environment);
var args = new GraphQLTypes.GraphQLTripStoptimesForDateArgs(environment.getArguments());

ZoneId timeZone = transitService.getTimeZone();
LocalDate serviceDate = args.getGraphQLServiceDate() != null
? ServiceDateUtils.parseString(args.getGraphQLServiceDate())
: LocalDate.now(timeZone);

return transitService.getTripTimeOnDates(trip, serviceDate).orElse(null);
};
}

Expand Down Expand Up @@ -400,6 +334,15 @@ private TripPattern getTripPattern(DataFetchingEnvironment environment) {
return getTransitService(environment).getPatternForTrip(environment.getSource());
}

private TripPattern getTripPattern(
DataFetchingEnvironment environment,
@Nullable LocalDate date
) {
return date == null
? getTripPattern(environment)
: getTransitService(environment).getPatternForTrip(environment.getSource(), date);
}

private TransitService getTransitService(DataFetchingEnvironment environment) {
return environment.<GraphQLRequestContext>getContext().transitService();
}
Expand All @@ -408,6 +351,16 @@ private RealtimeVehicleService getRealtimeVehiclesService(DataFetchingEnvironmen
return environment.<GraphQLRequestContext>getContext().realTimeVehicleService();
}

private static Optional<LocalDate> getOptionalServiceDateArgument(
DataFetchingEnvironment environment
) throws ParseException {
var args = new GraphQLTypes.GraphQLTripArrivalStoptimeArgs(environment.getArguments());
if (args.getGraphQLServiceDate() != null) {
return Optional.of(ServiceDateUtils.parseString(args.getGraphQLServiceDate()));
}
return Optional.empty();
}

private Trip getSource(DataFetchingEnvironment environment) {
return environment.getSource();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import org.opentripplanner.apis.transmodel.model.framework.TransmodelDirectives;
import org.opentripplanner.apis.transmodel.model.framework.TransmodelScalars;
import org.opentripplanner.apis.transmodel.support.GqlUtil;
import org.opentripplanner.routing.TripTimeOnDateHelper;
import org.opentripplanner.transit.model.network.TripPattern;
import org.opentripplanner.transit.model.site.StopLocation;
import org.opentripplanner.transit.model.timetable.TripOnServiceDate;
Expand Down Expand Up @@ -150,11 +149,10 @@ public static GraphQLObjectType create(
)
.dataFetcher(environment -> {
TripOnServiceDate tripOnServiceDate = tripOnServiceDate(environment);
return TripTimeOnDateHelper.getTripTimeOnDates(
GqlUtil.getTransitService(environment),
tripOnServiceDate.getTrip(),
tripOnServiceDate.getServiceDate()
);
return GqlUtil
.getTransitService(environment)
.getTripTimeOnDates(tripOnServiceDate.getTrip(), tripOnServiceDate.getServiceDate())
.orElse(null);
})
.build()
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@
import org.opentripplanner.apis.transmodel.model.framework.TransmodelScalars;
import org.opentripplanner.apis.transmodel.support.GqlUtil;
import org.opentripplanner.framework.geometry.EncodedPolyline;
import org.opentripplanner.model.TripTimeOnDate;
import org.opentripplanner.routing.TripTimeOnDateHelper;
import org.opentripplanner.transit.model.network.TripPattern;
import org.opentripplanner.transit.model.site.StopLocation;
import org.opentripplanner.transit.model.timetable.Trip;
Expand Down Expand Up @@ -240,14 +238,7 @@ public static GraphQLObjectType create(
.description(
"Returns scheduled passing times only - without real-time-updates, for realtime-data use 'estimatedCalls'"
)
.dataFetcher(env -> {
Trip trip = trip(env);
TripPattern tripPattern = GqlUtil.getTransitService(env).getPatternForTrip(trip);
if (tripPattern == null) {
return List.of();
}
return TripTimeOnDate.fromTripTimes(tripPattern.getScheduledTimetable(), trip);
})
.dataFetcher(env -> GqlUtil.getTransitService(env).getScheduledTripTimes(trip(env)))
.build()
)
.field(
Expand All @@ -274,11 +265,10 @@ public static GraphQLObjectType create(
.ofNullable(environment.getArgument("date"))
.map(LocalDate.class::cast)
.orElse(LocalDate.now(GqlUtil.getTransitService(environment).getTimeZone()));
return TripTimeOnDateHelper.getTripTimeOnDates(
GqlUtil.getTransitService(environment),
trip(environment),
serviceDate
);
return GqlUtil
.getTransitService(environment)
.getTripTimeOnDates(trip(environment), serviceDate)
.orElse(null);
})
.build()
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand All @@ -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;
Expand All @@ -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()));
Expand All @@ -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) {
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));
Expand Down Expand Up @@ -270,4 +287,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)
);
}
Comment on lines +332 to +342
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


@Override
public int hashCode() {
return Objects.hash(tripTimes, stopIndex, tripPattern, serviceDate, midnight);
}
}
Loading
Loading