GTFS Flex spec update: separate columns for location_id, location_group_id

pom.xml

@@ -853,7 +853,7 @@
         <dependency>
             <groupId>org.onebusaway</groupId>
             <artifactId>onebusaway-gtfs</artifactId>
-            <version>1.4.9</version>
+            <version>1.4.10</version>
             <exclusions>
                 <exclusion>
                     <groupId>org.slf4j</groupId>

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

@@ -5,7 +5,9 @@
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Objects;
 import org.onebusaway.gtfs.model.Location;
+import org.onebusaway.gtfs.model.LocationGroup;
 import org.onebusaway.gtfs.model.Stop;
 import org.opentripplanner.framework.collection.MapUtils;
 import org.opentripplanner.framework.i18n.NonLocalizedString;
@@ -19,7 +21,7 @@ public class LocationGroupMapper {
   private final LocationMapper locationMapper;
   private final StopModelBuilder stopModelBuilder;
 
-  private final Map<org.onebusaway.gtfs.model.LocationGroup, GroupStop> mappedLocationGroups = new HashMap<>();
+  private final Map<LocationGroup, GroupStop> mappedLocationGroups = new HashMap<>();
 
   public LocationGroupMapper(
     StopMapper stopMapper,
@@ -31,29 +33,34 @@ public LocationGroupMapper(
     this.stopModelBuilder = stopModelBuilder;
   }
 
-  Collection<GroupStop> map(Collection<org.onebusaway.gtfs.model.LocationGroup> allLocationGroups) {
+  Collection<GroupStop> map(Collection<LocationGroup> allLocationGroups) {
     return MapUtils.mapToList(allLocationGroups, this::map);
   }
 
   /** Map from GTFS to OTP model, {@code null} safe. */
-  GroupStop map(org.onebusaway.gtfs.model.LocationGroup original) {
+  GroupStop map(LocationGroup original) {
     return original == null ? null : mappedLocationGroups.computeIfAbsent(original, this::doMap);
   }
 
-  private GroupStop doMap(org.onebusaway.gtfs.model.LocationGroup element) {
+  private GroupStop doMap(LocationGroup element) {
     GroupStopBuilder groupStopBuilder = stopModelBuilder
       .groupStop(mapAgencyAndId(element.getId()))
       .withName(new NonLocalizedString(element.getName()));
 
-    for (org.onebusaway.gtfs.model.StopLocation location : element.getLocations()) {
-      if (location instanceof Stop) {
-        groupStopBuilder.addLocation(stopMapper.map((Stop) location));
-      } else if (location instanceof Location) {
-        groupStopBuilder.addLocation(locationMapper.map((Location) location));
-      } else if (location instanceof org.onebusaway.gtfs.model.LocationGroup) {
-        throw new RuntimeException("Nested GroupStops are not allowed");
-      } else {
-        throw new RuntimeException("Unknown location type: " + location.getClass().getSimpleName());
+    for (var location : element.getLocations()) {
+      Objects.requireNonNull(
+        location,
+        "Location group '%s' contains a null element.".formatted(element.getId())
+      );
+      switch (location) {
+        case Stop stop -> groupStopBuilder.addLocation(stopMapper.map(stop));
+        case Location loc -> groupStopBuilder.addLocation(locationMapper.map(loc));
+        case LocationGroup ignored -> throw new RuntimeException(
+          "Nested GroupStops are not allowed"
+        );
+        default -> throw new RuntimeException(
+          "Unknown location type: " + location.getClass().getSimpleName()
+        );
       }
     }
 

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

@@ -3,6 +3,7 @@
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Objects;
 import org.onebusaway.gtfs.model.Location;
 import org.onebusaway.gtfs.model.LocationGroup;
 import org.onebusaway.gtfs.model.Stop;
@@ -57,12 +58,18 @@ private StopTime doMap(org.onebusaway.gtfs.model.StopTime rhs) {
     StopTime lhs = new StopTime();
 
     lhs.setTrip(tripMapper.map(rhs.getTrip()));
-    if (rhs.getStop() instanceof Stop) {
-      lhs.setStop(stopMapper.map((Stop) rhs.getStop()));
-    } else if (rhs.getStop() instanceof Location) {
-      lhs.setStop(locationMapper.map((Location) rhs.getStop()));
-    } else if (rhs.getStop() instanceof LocationGroup) {
-      lhs.setStop(locationGroupMapper.map((LocationGroup) rhs.getStop()));
+    var stopLocation = rhs.getStopLocation();
+    Objects.requireNonNull(
+      stopLocation,
+      "Trip %s contains stop_time with no stop, location or group.".formatted(rhs.getTrip())
+    );
+    switch (stopLocation) {
+      case Stop stop -> lhs.setStop(stopMapper.map(stop));
+      case Location location -> lhs.setStop(locationMapper.map(location));
+      case LocationGroup locGroup -> lhs.setStop(locationGroupMapper.map(locGroup));
+      default -> throw new IllegalArgumentException(
+        "Unknown location type: %s".formatted(stopLocation)
+      );
     }
 
     I18NString stopHeadsign = null;

src/test/java/org/opentripplanner/gtfs/mapping/StopTimesMapperTest.java → src/test/java/org/opentripplanner/gtfs/mapping/StopTimeMapperTest.java

@@ -2,24 +2,35 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.List;
+import org.geojson.LngLatAlt;
+import org.geojson.Polygon;
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.onebusaway.gtfs.model.AgencyAndId;
+import org.onebusaway.gtfs.model.Location;
+import org.onebusaway.gtfs.model.LocationGroup;
 import org.onebusaway.gtfs.model.Stop;
 import org.onebusaway.gtfs.model.StopTime;
 import org.onebusaway.gtfs.model.Trip;
+import org.opentripplanner._support.geometry.Polygons;
 import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore;
 import org.opentripplanner.model.PickDrop;
+import org.opentripplanner.transit.model.site.AreaStop;
+import org.opentripplanner.transit.model.site.GroupStop;
 import org.opentripplanner.transit.service.StopModel;
 import org.opentripplanner.transit.service.StopModelBuilder;
 
-public class StopTimesMapperTest {
+public class StopTimeMapperTest {
 
   private static final String FEED_ID = "FEED";
 
@@ -41,8 +52,6 @@ public class StopTimesMapperTest {
 
   private static final double SHAPE_DIST_TRAVELED = 2.5d;
 
-  private static final Stop STOP = new Stop();
-
   private static final String STOP_NAME = "Stop";
 
   private static final String HEAD_SIGN = "Head Sign";
@@ -53,9 +62,11 @@ public class StopTimesMapperTest {
 
   private static final Trip TRIP = new GtfsTestData().trip;
 
-  private static final StopTime STOP_TIME = new StopTime();
-
   public static final DataImportIssueStore ISSUE_STORE = DataImportIssueStore.NOOP;
+  private static final List<LngLatAlt> ZONE_COORDINATES = Arrays
+    .stream(Polygons.BERLIN.getCoordinates())
+    .map(c -> new LngLatAlt(c.x, c.y))
+    .toList();
 
   private final StopModelBuilder stopModelBuilder = StopModel.of();
 
@@ -85,36 +96,60 @@ public class StopTimesMapperTest {
     new TranslationHelper()
   );
 
-  static {
+  /**
+   * Build a static ("regular") stop.
+   */
+  private static Stop buildStop() {
+    var stop = new Stop();
+    stop.setId(AGENCY_AND_ID);
+    stop.setName(STOP_NAME);
+    stop.setLat(53.12);
+    stop.setLon(12.34);
+    return stop;
+  }
+
+  /**
+   * Builds a stop time with a fixed stop.
+   */
+  static StopTime buildDefaultStopTime() {
+    var stopTime = buildStopTime();
+    var stop = buildStop();
+    stopTime.setStop(stop);
+    return stopTime;
+  }
+
+  /**
+   * Builds a stop time without a stop. Useful for testing flex fields.
+   */
+  private static StopTime buildStopTime() {
     TRIP.setId(AGENCY_AND_ID);
-    STOP.setId(AGENCY_AND_ID);
-    STOP.setName(STOP_NAME);
-
-    STOP_TIME.setId(ID);
-    STOP_TIME.setArrivalTime(ARRIVAL_TIME);
-    STOP_TIME.setDepartureTime(DEPARTURE_TIME);
-    STOP_TIME.setDropOffType(DROP_OFF_TYPE);
-    STOP_TIME.setFarePeriodId(FARE_PERIOD_ID);
-    STOP_TIME.setPickupType(PICKUP_TYPE);
-    STOP_TIME.setRouteShortName(ROUTE_SHORT_NAME);
-    STOP_TIME.setShapeDistTraveled(SHAPE_DIST_TRAVELED);
-    STOP_TIME.setStop(STOP);
-    STOP_TIME.setStopHeadsign(HEAD_SIGN);
-    STOP_TIME.setStopSequence(STOP_SEQUENCE);
-    STOP_TIME.setTimepoint(TIMEPOINT);
-    STOP_TIME.setTrip(TRIP);
+
+    var stopTime = new StopTime();
+    stopTime.setId(ID);
+    stopTime.setArrivalTime(ARRIVAL_TIME);
+    stopTime.setDepartureTime(DEPARTURE_TIME);
+    stopTime.setDropOffType(DROP_OFF_TYPE);
+    stopTime.setFarePeriodId(FARE_PERIOD_ID);
+    stopTime.setPickupType(PICKUP_TYPE);
+    stopTime.setRouteShortName(ROUTE_SHORT_NAME);
+    stopTime.setShapeDistTraveled(SHAPE_DIST_TRAVELED);
+    stopTime.setStopHeadsign(HEAD_SIGN);
+    stopTime.setStopSequence(STOP_SEQUENCE);
+    stopTime.setTimepoint(TIMEPOINT);
+    stopTime.setTrip(TRIP);
+    return stopTime;
   }
 
   @Test
   public void testMapCollection() {
     assertNull(subject.map((Collection<StopTime>) null));
     assertTrue(subject.map(Collections.emptyList()).isEmpty());
-    assertEquals(1, subject.map(Collections.singleton(STOP_TIME)).size());
+    assertEquals(1, subject.map(Collections.singleton(buildDefaultStopTime())).size());
   }
 
   @Test
   public void testMap() {
-    org.opentripplanner.model.StopTime result = subject.map(STOP_TIME);
+    var result = subject.map(buildDefaultStopTime());
 
     assertEquals(ARRIVAL_TIME, result.getArrivalTime());
     assertEquals(DEPARTURE_TIME, result.getDepartureTime());
@@ -132,7 +167,9 @@ public void testMap() {
 
   @Test
   public void testMapWithNulls() {
-    org.opentripplanner.model.StopTime result = subject.map(new StopTime());
+    var st = new StopTime();
+    st.setStop(buildStop());
+    var result = subject.map(st);
 
     assertFalse(result.isArrivalTimeSet());
     assertFalse(result.isDepartureTimeSet());
@@ -141,18 +178,56 @@ public void testMapWithNulls() {
     assertEquals(PickDrop.SCHEDULED, result.getPickupType());
     assertNull(result.getRouteShortName());
     assertFalse(result.isShapeDistTraveledSet());
-    assertNull(result.getStop());
+    assertNotNull(result.getStop());
     assertNull(result.getStopHeadsign());
     assertEquals(0, result.getStopSequence());
     assertFalse(result.isTimepointSet());
   }
 
-  /** 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() {
-    org.opentripplanner.model.StopTime result1 = subject.map(STOP_TIME);
-    org.opentripplanner.model.StopTime result2 = subject.map(STOP_TIME);
-
+    var st = buildDefaultStopTime();
+    var result1 = subject.map(st);
+    var result2 = subject.map(st);
     assertSame(result1, result2);
   }
+
+  @Test
+  public void testNull() {
+    var st = buildStopTime();
+    Assertions.assertThrows(NullPointerException.class, () -> subject.map(st));
+  }
+
+  @Test
+  public void testFlexLocation() {
+    var st = buildStopTime();
+    var flexLocation = new Location();
+    flexLocation.setId(AGENCY_AND_ID);
+    var polygon = new Polygon();
+    polygon.setExteriorRing(ZONE_COORDINATES);
+    flexLocation.setGeometry(polygon);
+    st.setStop(flexLocation);
+    var mapped = subject.map(st);
+
+    assertInstanceOf(AreaStop.class, mapped.getStop());
+    var areaStop = (AreaStop) mapped.getStop();
+    assertEquals(Polygons.BERLIN, areaStop.getGeometry());
+    assertEquals("A:1", areaStop.getId().toString());
+  }
+
+  @Test
+  public void testFlexLocationGroup() {
+    var st = buildStopTime();
+    var locGroup = new LocationGroup();
+    locGroup.setName("A location group");
+    locGroup.setId(AGENCY_AND_ID);
+    locGroup.addLocation(buildStop());
+    st.setStop(locGroup);
+    var mapped = subject.map(st);
+    assertInstanceOf(GroupStop.class, mapped.getStop());
+
+    var groupStop = (GroupStop) mapped.getStop();
+    assertEquals("[RegularStop{A:1 Stop}]", groupStop.getLocations().toString());
+  }
 }