Skip to content

Commit cc481cc

Browse files
optionsomeoptionsome
committed
Refactor to only allow one coordinate outside of raptor
1 parent 8987881 commit cc481cc

File tree

11 files changed

+66
-52
lines changed

11 files changed

+66
-52
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ private static ViaLocation mapViaLocation(Map<String, Object> via) {
3939
visit.getGraphQLLabel(),
4040
visit.getGraphQLMinimumWaitTime(),
4141
mapStopLocationIds(visit.getGraphQLStopLocationIds()),
42-
mapCoordinate(visit.getGraphQLCoordinate()).map(List::of).orElse(List.of())
42+
mapCoordinate(visit.getGraphQLCoordinate()).orElse(null)
4343
);
4444
} else {
4545
throw new IllegalArgumentException("ViaLocation must define either pass-through or visit.");

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,12 @@ private List<FeedScopedId> mapStopLocationIds(Map<String, Object> map) {
8282
return c == null ? List.of() : idMapper.parseList(c);
8383
}
8484

85-
private static List<WgsCoordinate> mapCoordinate(Map<String, Object> map) {
86-
return CoordinateInputType.mapToWgsCoordinate(ViaLocationInputType.FIELD_COORDINATE, map)
87-
.map(List::of)
88-
.orElseGet(List::of);
85+
@Nullable
86+
private static WgsCoordinate mapCoordinate(Map<String, Object> map) {
87+
return CoordinateInputType.mapToWgsCoordinate(
88+
ViaLocationInputType.FIELD_COORDINATE,
89+
map
90+
).orElse(null);
8991
}
9092

9193
/**

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,17 +256,20 @@ private RaptorViaLocation mapViaLocation(ViaLocation input) {
256256
builder.addViaStop(stopIndex);
257257
viaStops.add(stopIndex);
258258
}
259-
for (var coordinate : input.coordinates()) {
259+
if (input.coordinate().isPresent()) {
260260
var vertices = linkingContext.findVertices(((VisitViaLocation) input).coordinateLocation());
261261
if (vertices.isEmpty()) {
262262
LOG.warn(
263263
"Found no vertices for the visit via location {} which indicates a problem.",
264264
input
265265
);
266-
continue;
267266
}
268267
for (var vertex : vertices) {
269-
var viaTransfers = viaTransferResolver.createViaTransfers(request, vertex, coordinate);
268+
var viaTransfers = viaTransferResolver.createViaTransfers(
269+
request,
270+
vertex,
271+
input.coordinate().get()
272+
);
270273
for (var it : viaTransfers) {
271274
// If via-stop and via-transfers are used together then walking from a stop
272275
// to the coordinate and back is not pareto optimal, using just the stop

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import java.time.Duration;
44
import java.util.List;
5+
import java.util.Optional;
56
import javax.annotation.Nullable;
67
import org.opentripplanner.framework.geometry.WgsCoordinate;
78
import org.opentripplanner.transit.model.framework.FeedScopedId;
@@ -47,10 +48,10 @@ default Duration minimumWaitTime() {
4748
List<FeedScopedId> stopLocationIds();
4849

4950
/**
50-
* A list of coordinates used together with the {@code stopLocationIds} as the via location.
51-
* This is optional, an empty list is returned if no coordinates are available.
51+
* A coordinate used together with the {@code stopLocationIds} as the via location.
52+
* This is optional.
5253
*/
53-
default List<WgsCoordinate> coordinates() {
54-
return List.of();
54+
default Optional<WgsCoordinate> coordinate() {
55+
return Optional.empty();
5556
}
5657
}

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

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import java.time.Duration;
44
import java.util.List;
55
import java.util.Objects;
6+
import java.util.Optional;
67
import javax.annotation.Nullable;
78
import org.opentripplanner.framework.geometry.WgsCoordinate;
89
import org.opentripplanner.model.GenericLocation;
@@ -20,24 +21,27 @@ public class VisitViaLocation extends AbstractViaLocation {
2021

2122
private static final Duration MINIMUM_WAIT_TIME_MAX_LIMIT = Duration.ofHours(24);
2223

24+
@Nullable
2325
private final Duration minimumWaitTime;
24-
private final List<WgsCoordinate> coordinates;
26+
27+
@Nullable
28+
private final WgsCoordinate coordinate;
2529

2630
public VisitViaLocation(
2731
@Nullable String label,
2832
@Nullable Duration minimumWaitTime,
2933
List<FeedScopedId> stopLocationIds,
30-
List<WgsCoordinate> coordinates
34+
@Nullable WgsCoordinate coordinate
3135
) {
3236
super(label, stopLocationIds);
3337
this.minimumWaitTime = DurationUtils.requireNonNegative(
3438
minimumWaitTime == null ? Duration.ZERO : minimumWaitTime,
3539
MINIMUM_WAIT_TIME_MAX_LIMIT,
3640
"minimumWaitTime"
3741
);
38-
this.coordinates = List.copyOf(coordinates);
42+
this.coordinate = coordinate;
3943

40-
if (stopLocationIds().isEmpty() && coordinates().isEmpty()) {
44+
if (stopLocationIds().isEmpty() && coordinate().isEmpty()) {
4145
throw new IllegalArgumentException(
4246
"A via location must have at least one stop location or a coordinate." +
4347
(label == null ? "" : " Label: " + label)
@@ -51,11 +55,9 @@ public VisitViaLocation(
5155
*/
5256
@Nullable
5357
public GenericLocation coordinateLocation() {
54-
if (coordinates.isEmpty()) {
58+
if (coordinate == null) {
5559
return null;
5660
}
57-
// TODO we will remove support for multiple coordinates
58-
var coordinate = coordinates.getFirst();
5961
return new GenericLocation(label(), null, coordinate.latitude(), coordinate.longitude());
6062
}
6163

@@ -75,8 +77,8 @@ public boolean isPassThroughLocation() {
7577
}
7678

7779
@Override
78-
public List<WgsCoordinate> coordinates() {
79-
return coordinates;
80+
public Optional<WgsCoordinate> coordinate() {
81+
return Optional.ofNullable(coordinate);
8082
}
8183

8284
@Override
@@ -85,7 +87,7 @@ public String toString() {
8587
.addObj("label", label())
8688
.addDuration("minimumWaitTime", minimumWaitTime, Duration.ZERO)
8789
.addCol("stopLocationIds", stopLocationIds())
88-
.addObj("coordinates", coordinates)
90+
.addObj("coordinate", coordinate)
8991
.toString();
9092
}
9193

@@ -103,12 +105,12 @@ public boolean equals(Object o) {
103105
VisitViaLocation that = (VisitViaLocation) o;
104106
return (
105107
Objects.equals(minimumWaitTime, that.minimumWaitTime) &&
106-
Objects.equals(coordinates, that.coordinates)
108+
Objects.equals(coordinate, that.coordinate)
107109
);
108110
}
109111

110112
@Override
111113
public int hashCode() {
112-
return Objects.hash(super.hashCode(), minimumWaitTime, coordinates);
114+
return Objects.hash(super.hashCode(), minimumWaitTime, coordinate);
113115
}
114116
}

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -77,26 +77,26 @@ void mapToVisitViaLocations() {
7777

7878
var secondVia = result.get(1);
7979

80-
assertThat(secondVia.coordinates()).hasSize(1);
81-
assertEquals(SECOND_LAT, secondVia.coordinates().get(0).latitude());
82-
assertEquals(SECOND_LON, secondVia.coordinates().get(0).longitude());
80+
assertThat(secondVia.coordinate()).isPresent();
81+
assertEquals(SECOND_LAT, secondVia.coordinate().get().latitude());
82+
assertEquals(SECOND_LON, secondVia.coordinate().get().longitude());
8383
assertFalse(secondVia.isPassThroughLocation());
8484

8585
var thirdVia = result.get(2);
8686

8787
assertEquals(LABEL_THIRD, thirdVia.label());
8888
assertEquals(MIN_WAIT_TIME_THIRD, thirdVia.minimumWaitTime());
8989
assertEquals(EXPECTED_IDS_AS_STRING_THIRD, thirdVia.stopLocationIds().toString());
90-
assertThat(thirdVia.coordinates()).hasSize(1);
91-
assertEquals(THIRD_LAT, thirdVia.coordinates().get(0).latitude());
92-
assertEquals(THIRD_LON, thirdVia.coordinates().get(0).longitude());
90+
assertThat(thirdVia.coordinate()).isPresent();
91+
assertEquals(THIRD_LAT, thirdVia.coordinate().get().latitude());
92+
assertEquals(THIRD_LON, thirdVia.coordinate().get().longitude());
9393
assertFalse(thirdVia.isPassThroughLocation());
9494

9595
assertEquals(
9696
"[" +
97-
"VisitViaLocation{label: TestLabel1, minimumWaitTime: 5m, stopLocationIds: [F:ID1, F:ID2], coordinates: []}, " +
98-
"VisitViaLocation{coordinates: [(30.5, 40.2)]}, " +
99-
"VisitViaLocation{label: TestLabel3, minimumWaitTime: 10m, stopLocationIds: [F:ID3, F:ID4], coordinates: [(35.5, 45.5)]}" +
97+
"VisitViaLocation{label: TestLabel1, minimumWaitTime: 5m, stopLocationIds: [F:ID1, F:ID2]}, " +
98+
"VisitViaLocation{coordinate: (30.5, 40.2)}, " +
99+
"VisitViaLocation{label: TestLabel3, minimumWaitTime: 10m, stopLocationIds: [F:ID3, F:ID4], coordinate: (35.5, 45.5)}" +
100100
"]",
101101
result.toString()
102102
);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ void testMapToVisitViaLocations() {
5353
assertEquals(EXPECTED_IDS_AS_STRING, via.stopLocationIds().toString());
5454
assertFalse(via.isPassThroughLocation());
5555
assertEquals(
56-
"[VisitViaLocation{label: TestLabel, minimumWaitTime: 5m, stopLocationIds: [F:ID1, F:ID2], coordinates: []}]",
56+
"[VisitViaLocation{label: TestLabel, minimumWaitTime: 5m, stopLocationIds: [F:ID1, F:ID2]}]",
5757
result.toString()
5858
);
5959
}

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ class RaptorRequestMapperTest {
5353
"Via A",
5454
null,
5555
List.of(STOP_A.getId()),
56-
List.of()
56+
null
5757
);
5858
private static final int VIA_FROM_STOP_INDEX = 47;
5959
private static final int VIA_TO_STOP_INDEX = 123;
@@ -73,7 +73,7 @@ class RaptorRequestMapperTest {
7373
"Via coordinate",
7474
Duration.ofMinutes(10),
7575
List.of(),
76-
List.of(VIA_COORDINATE)
76+
VIA_COORDINATE
7777
);
7878

7979
private static final CostLinearFunction R1 = CostLinearFunction.of("50 + 1.0x");
@@ -112,7 +112,7 @@ void testViaLocation() {
112112
var minWaitTime = Duration.ofMinutes(13);
113113

114114
req.withViaLocations(
115-
List.of(new VisitViaLocation("Via A", minWaitTime, List.of(STOP_A.getId()), List.of()))
115+
List.of(new VisitViaLocation("Via A", minWaitTime, List.of(STOP_A.getId()), null))
116116
);
117117

118118
var result = map(req.buildRequest());
@@ -144,6 +144,12 @@ void testViaCoordinate() {
144144
var req = requestBuilder();
145145
req.withViaLocations(List.of(VISIT_VIA_LOCATION_COORDINATE));
146146

147+
req.withViaLocations(
148+
List.of(
149+
new VisitViaLocation("Via coordinate", Duration.ofMinutes(10), List.of(), VIA_COORDINATE)
150+
)
151+
);
152+
147153
var result = map(req.buildRequest());
148154

149155
assertFalse(result.searchParams().viaLocations().isEmpty());

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ void stopLocationIds() {
4040

4141
@Test
4242
void coordinates() {
43-
assertEquals("[]", subject.coordinates().toString());
43+
assertTrue(subject.coordinate().isEmpty());
4444
}
4545

4646
@Test

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ class VisitViaLocationTest {
2121
LABEL,
2222
MINIMUM_WAIT_TIME,
2323
List.of(ID),
24-
List.of(WgsCoordinate.GREENWICH)
24+
WgsCoordinate.GREENWICH
2525
);
2626

2727
@Test
@@ -46,13 +46,13 @@ void stopLocationIds() {
4646

4747
@Test
4848
void coordinates() {
49-
assertEquals("[" + WgsCoordinate.GREENWICH + "]", subject.coordinates().toString());
49+
assertEquals(WgsCoordinate.GREENWICH, subject.coordinate().get());
5050
}
5151

5252
@Test
5353
void testToString() {
5454
assertEquals(
55-
"VisitViaLocation{label: AName, minimumWaitTime: 5m, stopLocationIds: [F:1], coordinates: [(51.48, 0.0)]}",
55+
"VisitViaLocation{label: AName, minimumWaitTime: 5m, stopLocationIds: [F:1], coordinate: (51.48, 0.0)}",
5656
subject.toString()
5757
);
5858
}
@@ -62,15 +62,15 @@ void testEqAndHashCode() {
6262
var l = subject.label();
6363
var mwt = subject.minimumWaitTime();
6464
var ids = subject.stopLocationIds();
65-
var cs = subject.coordinates();
65+
var cs = subject.coordinate();
6666

6767
AssertEqualsAndHashCode.verify(subject)
68-
.sameAs(new VisitViaLocation(l, mwt, ids, cs))
68+
.sameAs(new VisitViaLocation(l, mwt, ids, cs.orElse(null)))
6969
.differentFrom(
70-
new VisitViaLocation("other", mwt, ids, cs),
71-
new VisitViaLocation(l, Duration.ZERO, ids, cs),
72-
new VisitViaLocation(l, mwt, List.of(), cs),
73-
new VisitViaLocation(l, mwt, ids, List.of())
70+
new VisitViaLocation("other", mwt, ids, cs.orElse(null)),
71+
new VisitViaLocation(l, Duration.ZERO, ids, cs.orElse(null)),
72+
new VisitViaLocation(l, mwt, List.of(), cs.orElse(null)),
73+
new VisitViaLocation(l, mwt, ids, null)
7474
);
7575
}
7676
}

0 commit comments

Comments
 (0)