Refactor to only allow one coordinate outside of raptor

Author: optionsome

application/src/main/java/org/opentripplanner/apis/gtfs/mapping/routerequest/ViaLocationMapper.java

@@ -42,7 +42,7 @@ private static ViaLocation mapViaLocation(Map<String, Map<String, Object>> via)
         (String) visit.get(FIELD_LABEL),
         (Duration) visit.get(FIELD_MINIMUM_WAIT_TIME),
         mapStopLocationIds((List<String>) visit.get(FIELD_STOP_LOCATION_IDS)),
-        List.of()
+        null
       );
     } else {
       throw new IllegalArgumentException("ViaLocation must define either pass-through or visit.");

application/src/main/java/org/opentripplanner/apis/transmodel/mapping/TripViaLocationMapper.java

@@ -75,10 +75,12 @@ private static List<FeedScopedId> mapStopLocationIds(Map<String, Object> map) {
     return c == null ? List.of() : c.stream().map(TransitIdMapper::mapIDToDomain).toList();
   }
 
-  private static List<WgsCoordinate> mapCoordinate(Map<String, Object> map) {
-    return CoordinateInputType.mapToWgsCoordinate(ViaLocationInputType.FIELD_COORDINATE, map)
-      .map(List::of)
-      .orElseGet(List::of);
+  @Nullable
+  private static WgsCoordinate mapCoordinate(Map<String, Object> map) {
+    return CoordinateInputType.mapToWgsCoordinate(
+      ViaLocationInputType.FIELD_COORDINATE,
+      map
+    ).orElse(null);
   }
 
   /**

application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/RaptorRequestMapper.java

@@ -245,11 +245,11 @@ private RaptorViaLocation mapViaLocation(ViaLocation input) {
         builder.addViaStop(stopIndex);
         viaStops.add(stopIndex);
       }
-      for (var coordinate : input.coordinates()) {
+      if (input.coordinate().isPresent()) {
         var viaTransfers = viaTransferResolver.createViaTransfers(
           request,
           input.label(),
-          coordinate
+          input.coordinate().get()
         );
         for (var it : viaTransfers) {
           // If via-stop and via-transfers are used together then walking from a stop

application/src/main/java/org/opentripplanner/routing/api/request/via/ViaLocation.java

@@ -2,6 +2,7 @@
 
 import java.time.Duration;
 import java.util.List;
+import java.util.Optional;
 import javax.annotation.Nullable;
 import org.opentripplanner.framework.geometry.WgsCoordinate;
 import org.opentripplanner.transit.model.framework.FeedScopedId;
@@ -47,10 +48,10 @@ default Duration minimumWaitTime() {
   List<FeedScopedId> stopLocationIds();
 
   /**
-   * A list of coordinates used together with the {@code stopLocationIds} as the via location.
-   * This is optional, an empty list is returned if no coordinates are available.
+   * A coordinate used together with the {@code stopLocationIds} as the via location.
+   * This is optional.
    */
-  default List<WgsCoordinate> coordinates() {
-    return List.of();
+  default Optional<WgsCoordinate> coordinate() {
+    return Optional.empty();
   }
 }

application/src/main/java/org/opentripplanner/routing/api/request/via/VisitViaLocation.java

@@ -3,6 +3,7 @@
 import java.time.Duration;
 import java.util.List;
 import java.util.Objects;
+import java.util.Optional;
 import javax.annotation.Nullable;
 import org.opentripplanner.framework.geometry.WgsCoordinate;
 import org.opentripplanner.transit.model.framework.FeedScopedId;
@@ -19,24 +20,27 @@ public class VisitViaLocation extends AbstractViaLocation {
 
   private static final Duration MINIMUM_WAIT_TIME_MAX_LIMIT = Duration.ofHours(24);
 
+  @Nullable
   private final Duration minimumWaitTime;
-  private final List<WgsCoordinate> coordinates;
+
+  @Nullable
+  private final WgsCoordinate coordinate;
 
   public VisitViaLocation(
     @Nullable String label,
     @Nullable Duration minimumWaitTime,
     List<FeedScopedId> stopLocationIds,
-    List<WgsCoordinate> coordinates
+    @Nullable WgsCoordinate coordinate
   ) {
     super(label, stopLocationIds);
     this.minimumWaitTime = DurationUtils.requireNonNegative(
       minimumWaitTime == null ? Duration.ZERO : minimumWaitTime,
       MINIMUM_WAIT_TIME_MAX_LIMIT,
       "minimumWaitTime"
     );
-    this.coordinates = List.copyOf(coordinates);
+    this.coordinate = coordinate;
 
-    if (stopLocationIds().isEmpty() && coordinates().isEmpty()) {
+    if (stopLocationIds().isEmpty() && coordinate().isEmpty()) {
       throw new IllegalArgumentException(
         "A via location must have at least one stop location or a coordinate." +
         (label == null ? "" : " Label: " + label)
@@ -60,8 +64,8 @@ public boolean isPassThroughLocation() {
   }
 
   @Override
-  public List<WgsCoordinate> coordinates() {
-    return coordinates;
+  public Optional<WgsCoordinate> coordinate() {
+    return Optional.ofNullable(coordinate);
   }
 
   @Override
@@ -70,7 +74,7 @@ public String toString() {
       .addObj("label", label())
       .addDuration("minimumWaitTime", minimumWaitTime, Duration.ZERO)
       .addCol("stopLocationIds", stopLocationIds())
-      .addObj("coordinates", coordinates)
+      .addObj("coordinate", coordinate)
       .toString();
   }
 
@@ -88,12 +92,12 @@ public boolean equals(Object o) {
     VisitViaLocation that = (VisitViaLocation) o;
     return (
       Objects.equals(minimumWaitTime, that.minimumWaitTime) &&
-      Objects.equals(coordinates, that.coordinates)
+      Objects.equals(coordinate, that.coordinate)
     );
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(super.hashCode(), minimumWaitTime, coordinates);
+    return Objects.hash(super.hashCode(), minimumWaitTime, coordinate);
   }
 }

application/src/test/java/org/opentripplanner/apis/gtfs/mapping/routerequest/ViaLocationMapperTest.java

@@ -45,7 +45,7 @@ void mapToVisitViaLocations() {
     assertEquals(EXPECTED_IDS_AS_STRING, via.stopLocationIds().toString());
     assertFalse(via.isPassThroughLocation());
     assertEquals(
-      "[VisitViaLocation{label: TestLabel, minimumWaitTime: 5m, stopLocationIds: [F:ID1, F:ID2], coordinates: []}]",
+      "[VisitViaLocation{label: TestLabel, minimumWaitTime: 5m, stopLocationIds: [F:ID1, F:ID2]}]",
       result.toString()
     );
   }

application/src/test/java/org/opentripplanner/apis/transmodel/mapping/TripViaLocationMapperTest.java

@@ -54,7 +54,7 @@ void testMapToVisitViaLocations() {
     assertEquals(EXPECTED_IDS_AS_STRING, via.stopLocationIds().toString());
     assertFalse(via.isPassThroughLocation());
     assertEquals(
-      "[VisitViaLocation{label: TestLabel, minimumWaitTime: 5m, stopLocationIds: [F:ID1, F:ID2], coordinates: []}]",
+      "[VisitViaLocation{label: TestLabel, minimumWaitTime: 5m, stopLocationIds: [F:ID1, F:ID2]}]",
       result.toString()
     );
   }

application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/RaptorRequestMapperTest.java

@@ -47,7 +47,7 @@ class RaptorRequestMapperTest {
     "Via A",
     null,
     List.of(STOP_A.getId()),
-    List.of()
+    null
   );
   private static final int VIA_FROM_STOP_INDEX = 47;
   private static final int VIA_TO_STOP_INDEX = 123;
@@ -100,7 +100,7 @@ void testViaLocation() {
     var minWaitTime = Duration.ofMinutes(13);
 
     req.setViaLocations(
-      List.of(new VisitViaLocation("Via A", minWaitTime, List.of(STOP_A.getId()), List.of()))
+      List.of(new VisitViaLocation("Via A", minWaitTime, List.of(STOP_A.getId()), null))
     );
 
     var result = map(req);
@@ -133,9 +133,7 @@ void testViaCoordinate() {
     Duration minimumWaitTime = Duration.ofMinutes(10);
 
     req.setViaLocations(
-      List.of(
-        new VisitViaLocation("Via coordinate", minimumWaitTime, List.of(), List.of(VIA_COORDINATE))
-      )
+      List.of(new VisitViaLocation("Via coordinate", minimumWaitTime, List.of(), VIA_COORDINATE))
     );
 
     var result = map(req);

application/src/test/java/org/opentripplanner/routing/api/request/via/PassThroughViaLocationTest.java

@@ -40,7 +40,7 @@ void stopLocationIds() {
 
   @Test
   void coordinates() {
-    assertEquals("[]", subject.coordinates().toString());
+    assertTrue(subject.coordinate().isEmpty());
   }
 
   @Test

application/src/test/java/org/opentripplanner/routing/api/request/via/VisitViaLocationTest.java

@@ -21,7 +21,7 @@ class VisitViaLocationTest {
     LABEL,
     MINIMUM_WAIT_TIME,
     List.of(ID),
-    List.of(WgsCoordinate.GREENWICH)
+    WgsCoordinate.GREENWICH
   );
 
   @Test
@@ -46,13 +46,13 @@ void stopLocationIds() {
 
   @Test
   void coordinates() {
-    assertEquals("[" + WgsCoordinate.GREENWICH + "]", subject.coordinates().toString());
+    assertEquals(WgsCoordinate.GREENWICH, subject.coordinate().get());
   }
 
   @Test
   void testToString() {
     assertEquals(
-      "VisitViaLocation{label: AName, minimumWaitTime: 5m, stopLocationIds: [F:1], coordinates: [(51.48, 0.0)]}",
+      "VisitViaLocation{label: AName, minimumWaitTime: 5m, stopLocationIds: [F:1], coordinate: (51.48, 0.0)}",
       subject.toString()
     );
   }
@@ -62,15 +62,15 @@ void testEqAndHashCode() {
     var l = subject.label();
     var mwt = subject.minimumWaitTime();
     var ids = subject.stopLocationIds();
-    var cs = subject.coordinates();
+    var cs = subject.coordinate();
 
     AssertEqualsAndHashCode.verify(subject)
-      .sameAs(new VisitViaLocation(l, mwt, ids, cs))
+      .sameAs(new VisitViaLocation(l, mwt, ids, cs.orElse(null)))
       .differentFrom(
-        new VisitViaLocation("other", mwt, ids, cs),
-        new VisitViaLocation(l, Duration.ZERO, ids, cs),
-        new VisitViaLocation(l, mwt, List.of(), cs),
-        new VisitViaLocation(l, mwt, ids, List.of())
+        new VisitViaLocation("other", mwt, ids, cs.orElse(null)),
+        new VisitViaLocation(l, Duration.ZERO, ids, cs.orElse(null)),
+        new VisitViaLocation(l, mwt, List.of(), cs.orElse(null)),
+        new VisitViaLocation(l, mwt, ids, null)
       );
   }
 }