Skip to content

Commit

Permalink
Cleanup, tests and documentation
Browse files Browse the repository at this point in the history
  • Loading branch information
leonardehrenfried committed Apr 11, 2024
1 parent 55661fa commit 0c1a651
Show file tree
Hide file tree
Showing 13 changed files with 107 additions and 42 deletions.
4 changes: 2 additions & 2 deletions doc-templates/Flex.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@

To enable this turn on `FlexRouting` as a feature in `otp-config.json`.

The GTFS feeds should conform to the
[GTFS-Flex v2 draft PR](https://github.com/google/transit/pull/388)
The GTFS feeds must conform to the final, approved version of the draft which has been
merged into the [mainline specification](https://gtfs.org/schedule/reference/) in March 2024.

## Configuration

Expand Down
4 changes: 2 additions & 2 deletions docs/sandbox/Flex.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@

To enable this turn on `FlexRouting` as a feature in `otp-config.json`.

The GTFS feeds should conform to the
[GTFS-Flex v2 draft PR](https://github.com/google/transit/pull/388)
The GTFS feeds must conform to the final, approved version of the draft which has been
merged into the [mainline specification](https://gtfs.org/schedule/reference/) in March 2024.

## Configuration

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,6 @@ static List<Arguments> cases() {
void calculate(DurationModifier mod, int expectedSeconds) {
var modified = PATH.withDurationModifier(mod);
assertEquals(expectedSeconds, modified.durationSeconds);
assertEquals(LineStrings.SIMPLE, modified.getGeometry());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ void doesNotModify() {
void modifies() {
assertTrue(new DurationModifier(Duration.ofMinutes(1), 1.5f).modifies());
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package org.opentripplanner.ext.flex.trip;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.opentripplanner.street.model._data.StreetModelForTest.V1;
import static org.opentripplanner.street.model._data.StreetModelForTest.V2;
import static org.opentripplanner.transit.model._data.TransitModelForTest.id;

import java.time.Duration;
import java.util.List;
import org.junit.jupiter.api.Test;
import org.opentripplanner._support.geometry.LineStrings;
import org.opentripplanner.ext.flex.FlexStopTimesForTest;
import org.opentripplanner.ext.flex.flexpathcalculator.FlexPath;
import org.opentripplanner.ext.flex.flexpathcalculator.FlexPathCalculator;
import org.opentripplanner.model.StopTime;

class UnscheduledDrivingDurationTest {

static final FlexPathCalculator STATIC_CALCULATOR = (fromv, tov, fromStopIndex, toStopIndex) ->
new FlexPath(10_000, (int) Duration.ofMinutes(10).toSeconds(), () -> LineStrings.SIMPLE);
private static final StopTime STOP_TIME = FlexStopTimesForTest.area("10:00", "18:00");

@Test
void noModifier() {
var trip = UnscheduledTrip.of(id("1")).withStopTimes(List.of(STOP_TIME)).build();

var calculator = trip.flexPathCalculator(STATIC_CALCULATOR);
var path = calculator.calculateFlexPath(V1, V2, 0, 0);
assertEquals(600, path.durationSeconds);
}

@Test
void withModifier() {
var trip = UnscheduledTrip
.of(id("1"))
.withStopTimes(List.of(STOP_TIME))
.withDurationModifier(new DurationModifier(Duration.ofMinutes(2), 1.5f))
.build();

var calculator = trip.flexPathCalculator(STATIC_CALCULATOR);
var path = calculator.calculateFlexPath(V1, V2, 0, 0);
assertEquals(1020, path.durationSeconds);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.opentripplanner.ext.flex.FlexStopTimesForTest.area;
import static org.opentripplanner.ext.flex.FlexStopTimesForTest.regularArrival;
import static org.opentripplanner.ext.flex.FlexStopTimesForTest.regularDeparture;
import static org.opentripplanner.ext.flex.trip.UnscheduledTrip.isUnscheduledTrip;
import static org.opentripplanner.ext.flex.trip.UnscheduledTripTest.TestCase.tc;
import static org.opentripplanner.model.PickDrop.NONE;
Expand All @@ -23,7 +25,6 @@
import org.junit.jupiter.params.provider.MethodSource;
import org.opentripplanner._support.geometry.Polygons;
import org.opentripplanner.ext.flex.FlexServiceDate;
import org.opentripplanner.ext.flex.FlexStopTimesForTest;
import org.opentripplanner.ext.flex.flexpathcalculator.DirectFlexPathCalculator;
import org.opentripplanner.ext.flex.template.FlexAccessTemplate;
import org.opentripplanner.ext.flex.template.FlexEgressTemplate;
Expand Down Expand Up @@ -197,7 +198,7 @@ void testUnscheduledFeederTripToScheduledStop() {

static Stream<TestCase> testRegularStopToAreaEarliestDepartureTimeTestCases() {
// REGULAR-STOP to AREA - (10:00-14:00) => (14:00)
var tc = tc(FlexStopTimesForTest.regularDeparture("10:00"), area("10:00", "14:00"));
var tc = tc(regularDeparture("10:00"), area("10:00", "14:00"));
return Stream.of(
tc
.expected("Requested departure time is before flex service departure time", "10:00")
Expand Down Expand Up @@ -246,7 +247,7 @@ void testRegularStopToAreaEarliestDepartureTime(TestCase tc) {

static Stream<TestCase> testAreaToRegularStopEarliestDepartureTestCases() {
// AREA TO REGULAR-STOP - (10:00-14:00) => (14:00)
var tc = tc(area("10:00", "14:00"), FlexStopTimesForTest.regularArrival("14:00"));
var tc = tc(area("10:00", "14:00"), regularArrival("14:00"));
return Stream.of(
tc
.expected(
Expand Down Expand Up @@ -367,7 +368,7 @@ void testAreaToAreaEarliestDepartureTime(TestCase tc) {

static Stream<TestCase> testRegularStopToAreaLatestArrivalTimeTestCases() {
// REGULAR-STOP to AREA - (10:00-14:00) => (14:00)
var tc = tc(FlexStopTimesForTest.regularDeparture("10:00"), area("10:00", "14:00"));
var tc = tc(regularDeparture("10:00"), area("10:00", "14:00"));
return Stream.of(
tc
.expectedNotFound("Requested arrival time is before flex service arrival window start")
Expand Down Expand Up @@ -422,7 +423,7 @@ void testRegularStopToAreaLatestArrivalTime(TestCase tc) {

static Stream<TestCase> testAreaToRegularStopLatestArrivalTimeTestCases() {
// AREA TO REGULAR-STOP - (10:00-14:00) => (14:00)
var tc = tc(area("10:00", "14:00"), FlexStopTimesForTest.regularArrival("14:00"));
var tc = tc(area("10:00", "14:00"), regularArrival("14:00"));
return Stream.of(
tc
.expectedNotFound("Requested arrival time is before flex service arrival window start")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,7 @@ public class FlexTripsMapper {
);
} else if (ScheduledDeviatedTrip.isScheduledFlexTrip(stopTimes)) {
result.add(
ScheduledDeviatedTrip
.of(trip.getId())
.withTrip(trip)
.withStopTimes(stopTimes)
.build()
ScheduledDeviatedTrip.of(trip.getId()).withTrip(trip).withStopTimes(stopTimes).build()
);
} else if (hasContinuousStops(stopTimes) && FlexTrip.containsFlexStops(stopTimes)) {
store.add(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
import org.opentripplanner.ext.flex.trip.DurationModifier;
import org.opentripplanner.street.model.vertex.Vertex;

/**
* A calculator to delegates the main computation to another instance and applies a duration
* modifier afterward.
*/
public class DurationModifierCalculator implements FlexPathCalculator {

private final FlexPathCalculator delegate;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package org.opentripplanner.ext.flex.trip;

import static org.opentripplanner.ext.flex.trip.ScheduledDeviatedTrip.TimeType.FIXED_TIME;
import static org.opentripplanner.ext.flex.trip.ScheduledDeviatedTrip.TimeType.FLEXIBLE_TIME;
import static org.opentripplanner.model.PickDrop.NONE;
import static org.opentripplanner.model.StopTime.MISSING_VALUE;

Expand Down Expand Up @@ -299,13 +297,10 @@ private static class ScheduledDeviatedStopTime implements Serializable {
private final int arrivalTime;
private final PickDrop pickupType;
private final PickDrop dropOffType;
private final TimeType timeType;

private ScheduledDeviatedStopTime(StopTime st) {
this.stop = st.getStop();

this.timeType = st.hasFlexWindow() ? FLEXIBLE_TIME : FIXED_TIME;

// Store the time the user is guaranteed to arrive at latest
this.arrivalTime = st.getLatestPossibleArrivalTime();
// Store the time the user needs to be ready for pickup
Expand All @@ -320,9 +315,4 @@ private ScheduledDeviatedStopTime(StopTime st) {
this.dropOffType = arrivalTime == MISSING_VALUE ? PickDrop.NONE : st.getDropOffType();
}
}

enum TimeType {
FIXED_TIME,
FLEXIBLE_TIME,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,11 @@ public Stream<FlexAccessTemplate> getFlexAccessTemplates(
);
}

private FlexPathCalculator flexPathCalculator(FlexPathCalculator calculator) {
/**
* Get the correct {@link FlexPathCalculator} depending on the {@code durationModified}.
* If the modifier doesn't actually modify, we don't
*/
protected FlexPathCalculator flexPathCalculator(FlexPathCalculator calculator) {
if (durationModifier.modifies()) {
return new DurationModifierCalculator(calculator, durationModifier);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ public void mapStopTripAndRouteDataIntoBuilder() {

builder.getPathways().addAll(pathwayMapper.map(data.getAllPathways()));
builder.getStopTimesSortedByTrip().addAll(stopTimeMapper.map(data.getAllStopTimes()));
builder.getFlexDurationFactors().putAll(tripMapper.flexSafeDurationFactors());
builder.getFlexDurationFactors().putAll(tripMapper.flexSafeDurationModifiers());
builder.getTripsById().addAll(tripMapper.map(data.getAllTrips()));

fareRulesBuilder.fareAttributes().addAll(fareAttributeMapper.map(data.getAllFareAttributes()));
Expand Down
10 changes: 5 additions & 5 deletions src/main/java/org/opentripplanner/gtfs/mapping/TripMapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class TripMapper {
private final TranslationHelper translationHelper;

private final Map<org.onebusaway.gtfs.model.Trip, Trip> mappedTrips = new HashMap<>();
private final Map<Trip, DurationModifier> flexSafeDurationFactors = new HashMap<>();
private final Map<Trip, DurationModifier> flexSafeDurationModifiers = new HashMap<>();

TripMapper(
RouteMapper routeMapper,
Expand All @@ -45,8 +45,8 @@ Collection<Trip> getMappedTrips() {
/**
* The map of flex duration factors per flex trip.
*/
Map<Trip, DurationModifier> flexSafeDurationFactors() {
return flexSafeDurationFactors;
Map<Trip, DurationModifier> flexSafeDurationModifiers() {
return flexSafeDurationModifiers;
}

private Trip doMap(org.onebusaway.gtfs.model.Trip rhs) {
Expand Down Expand Up @@ -74,11 +74,11 @@ private Trip doMap(org.onebusaway.gtfs.model.Trip rhs) {
lhs.withGtfsFareId(rhs.getFareId());

var trip = lhs.build();
mapSafeDurationFactors(rhs).ifPresent(f -> flexSafeDurationFactors.put(trip, f));
mapSafeDurationModifier(rhs).ifPresent(f -> flexSafeDurationModifiers.put(trip, f));
return trip;
}

private Optional<DurationModifier> mapSafeDurationFactors(org.onebusaway.gtfs.model.Trip rhs) {
private Optional<DurationModifier> mapSafeDurationModifier(org.onebusaway.gtfs.model.Trip rhs) {
if (rhs.getSafeDurationFactor() == null && rhs.getSafeDurationOffset() == null) {
return Optional.empty();
} else {
Expand Down
45 changes: 35 additions & 10 deletions src/test/java/org/opentripplanner/gtfs/mapping/TripMapperTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,15 @@ public class TripMapperTest {

public static final DataImportIssueStore ISSUE_STORE = DataImportIssueStore.NOOP;

private final TripMapper subject = new TripMapper(
new RouteMapper(new AgencyMapper(FEED_ID), ISSUE_STORE, new TranslationHelper()),
new DirectionMapper(ISSUE_STORE),
new TranslationHelper()
);
private final TripMapper subject = defaultTripMapper();

private static TripMapper defaultTripMapper() {
return new TripMapper(
new RouteMapper(new AgencyMapper(FEED_ID), ISSUE_STORE, new TranslationHelper()),
new DirectionMapper(ISSUE_STORE),
new TranslationHelper()
);
}

static {
GtfsTestData data = new GtfsTestData();
Expand All @@ -56,14 +60,14 @@ public class TripMapperTest {
}

@Test
public void testMapCollection() throws Exception {
void testMapCollection() throws Exception {
assertNull(subject.map((Collection<Trip>) null));
assertTrue(subject.map(Collections.emptyList()).isEmpty());
assertEquals(1, subject.map(Collections.singleton(TRIP)).size());
}

@Test
public void testMap() throws Exception {
void testMap() throws Exception {
org.opentripplanner.transit.model.timetable.Trip result = subject.map(TRIP);

assertEquals("A:1", result.getId().toString());
Expand All @@ -80,7 +84,7 @@ public void testMap() throws Exception {
}

@Test
public void testMapWithNulls() throws Exception {
void testMapWithNulls() throws Exception {
Trip input = new Trip();
input.setId(AGENCY_AND_ID);
input.setRoute(new GtfsTestData().route);
Expand All @@ -101,12 +105,33 @@ public void testMapWithNulls() throws Exception {
assertEquals(BikeAccess.UNKNOWN, result.getBikesAllowed());
}

/** Mapping the same object twice, should return the the same instance. */
/** Mapping the same object twice, should return the same instance. */
@Test
public void testMapCache() throws Exception {
void testMapCache() throws Exception {
org.opentripplanner.transit.model.timetable.Trip result1 = subject.map(TRIP);
org.opentripplanner.transit.model.timetable.Trip result2 = subject.map(TRIP);

assertSame(result1, result2);
}

@Test
void noFlexDurationModifier() {
var mapper = defaultTripMapper();
mapper.map(TRIP);
assertTrue(mapper.flexSafeDurationModifiers().isEmpty());
}

@Test
void flexDurationModifier() {
var flexTrip = new Trip();
flexTrip.setId(new AgencyAndId("1", "1"));
flexTrip.setSafeDurationFactor(1.5);
flexTrip.setSafeDurationOffset(600d);
flexTrip.setRoute(new GtfsTestData().route);
var mapper = defaultTripMapper();
var mapped = mapper.map(flexTrip);
var mod = mapper.flexSafeDurationModifiers().get(mapped);
assertEquals(1.5f, mod.factor());
assertEquals(600, mod.offsetInSeconds());
}
}

0 comments on commit 0c1a651

Please sign in to comment.