Cleanup, tests and documentation

doc-templates/Flex.md

@@ -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
 

docs/sandbox/Flex.md

@@ -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
 

src/ext-test/java/org/opentripplanner/ext/flex/flexpathcalculator/FlexPathTest.java

@@ -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());
   }
 }

src/ext-test/java/org/opentripplanner/ext/flex/trip/DurationModifierTest.java

@@ -17,4 +17,4 @@ void doesNotModify() {
   void modifies() {
     assertTrue(new DurationModifier(Duration.ofMinutes(1), 1.5f).modifies());
   }
-}
+}

src/ext-test/java/org/opentripplanner/ext/flex/trip/UnscheduledDrivingDurationTest.java

@@ -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);
+  }
+}

src/ext-test/java/org/opentripplanner/ext/flex/trip/UnscheduledTripTest.java

@@ -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;
@@ -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;
@@ -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")
@@ -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(
@@ -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")
@@ -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")

src/ext/java/org/opentripplanner/ext/flex/FlexTripsMapper.java

@@ -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(

src/ext/java/org/opentripplanner/ext/flex/flexpathcalculator/DurationModifierCalculator.java

@@ -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;

src/ext/java/org/opentripplanner/ext/flex/trip/ScheduledDeviatedTrip.java

@@ -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;
 
@@ -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
@@ -320,9 +315,4 @@ private ScheduledDeviatedStopTime(StopTime st) {
       this.dropOffType = arrivalTime == MISSING_VALUE ? PickDrop.NONE : st.getDropOffType();
     }
   }
-
-  enum TimeType {
-    FIXED_TIME,
-    FLEXIBLE_TIME,
-  }
 }

src/ext/java/org/opentripplanner/ext/flex/trip/UnscheduledTrip.java

@@ -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 {

src/main/java/org/opentripplanner/gtfs/mapping/GTFSToOtpTransitServiceMapper.java

@@ -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()));

src/main/java/org/opentripplanner/gtfs/mapping/TripMapper.java

@@ -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,
@@ -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) {
@@ -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 {

src/test/java/org/opentripplanner/gtfs/mapping/TripMapperTest.java

@@ -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();
@@ -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());
@@ -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);
@@ -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());
+  }
 }