Skip to content

Commit 2c8cfdb

Browse files
optionsomeoptionsome
authored
Merge pull request opentripplanner#6494 from HSLdevcom/maxStopCountForMode-default-fix
Use default max stop count for mode from routing defaults in transfer requests
2 parents d4bdcc5 + 9ed9728 commit 2c8cfdb

File tree

9 files changed

+319
-65
lines changed

9 files changed

+319
-65
lines changed

‎application/src/ext/java/org/opentripplanner/ext/dataoverlay/api/DataOverlayParameters.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public class DataOverlayParameters implements Serializable {
3535
private final Map<Parameter, Double> values;
3636

3737
public DataOverlayParameters(Map<Parameter, Double> values) {
38-
// Make a defencive copy to protect the map entries, this make this class immutable
38+
// Make a defensive copy to protect the map entries, this makes the class immutable
3939
// and thread safe
4040
this.values = Map.copyOf(values);
4141
}

‎application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -272,9 +272,7 @@ private Collection<? extends RoutingAccessEgress> fetchAccessEgresses(AccessEgre
272272
.accessEgress();
273273

274274
Duration durationLimit = accessEgressPreferences.maxDuration().valueOf(mode);
275-
int stopCountLimit = accessEgressPreferences
276-
.maxStopCountForMode()
277-
.getOrDefault(mode, accessEgressPreferences.defaultMaxStopCount());
275+
int stopCountLimit = accessEgressPreferences.maxStopCountLimit().limitForMode(mode);
278276

279277
var nearbyStops = AccessEgressRouter.findAccessEgresses(
280278
accessRequest,

‎application/src/main/java/org/opentripplanner/routing/api/request/framework/DurationForEnum.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ public Builder<E> with(E key, Duration value) {
152152

153153
/**
154154
* Build a copy of the current values, excluding the defaultValue from the map. This
155-
* ensures equality and make a defencive copy of the builder values. Hence, the builder
155+
* ensures equality and makes a defensive copy of the builder values. Hence, the builder
156156
* can be used to generate new values if desired.
157157
* */
158158
Map<E, Duration> copyValueForEnum() {
@@ -185,7 +185,7 @@ public Builder<E> apply(Consumer<Builder<E>> body) {
185185
public DurationForEnum<E> build() {
186186
var it = new DurationForEnum<>(this);
187187

188-
// Return original if not change, subscriber is not notified
188+
// Return the original if there are no changes, the subscriber is not notified.
189189
if (original != null && original.equals(it)) {
190190
return original;
191191
}

‎application/src/main/java/org/opentripplanner/routing/api/request/preference/AccessEgressPreferences.java

Lines changed: 18 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
import java.io.Serializable;
66
import java.time.Duration;
7-
import java.util.Collections;
87
import java.util.Map;
98
import java.util.Objects;
109
import java.util.function.Consumer;
@@ -28,21 +27,18 @@ public final class AccessEgressPreferences implements Serializable {
2827

2928
private final TimeAndCostPenaltyForEnum<StreetMode> penalty;
3029
private final DurationForEnum<StreetMode> maxDuration;
31-
private final int defaultMaxStopCount;
32-
private final Map<StreetMode, Integer> maxStopCountForMode;
30+
private final MaxStopCountLimit maxStopCountLimit;
3331

3432
private AccessEgressPreferences() {
3533
this.maxDuration = durationForStreetModeOf(ofMinutes(45));
3634
this.penalty = DEFAULT_TIME_AND_COST;
37-
this.defaultMaxStopCount = 500;
38-
this.maxStopCountForMode = Map.of();
35+
this.maxStopCountLimit = new MaxStopCountLimit();
3936
}
4037

4138
private AccessEgressPreferences(Builder builder) {
4239
this.maxDuration = builder.maxDuration;
4340
this.penalty = builder.penalty;
44-
this.defaultMaxStopCount = builder.defaultMaxStopCount;
45-
this.maxStopCountForMode = Collections.unmodifiableMap(builder.maxStopCountForMode);
41+
this.maxStopCountLimit = builder.maxStopCountLimit;
4642
}
4743

4844
public static Builder of() {
@@ -61,12 +57,8 @@ public DurationForEnum<StreetMode> maxDuration() {
6157
return maxDuration;
6258
}
6359

64-
public int defaultMaxStopCount() {
65-
return defaultMaxStopCount;
66-
}
67-
68-
public Map<StreetMode, Integer> maxStopCountForMode() {
69-
return maxStopCountForMode;
60+
public MaxStopCountLimit maxStopCountLimit() {
61+
return maxStopCountLimit;
7062
}
7163

7264
@Override
@@ -77,14 +69,13 @@ public boolean equals(Object o) {
7769
return (
7870
penalty.equals(that.penalty) &&
7971
maxDuration.equals(that.maxDuration) &&
80-
defaultMaxStopCount == that.defaultMaxStopCount &&
81-
maxStopCountForMode.equals(that.maxStopCountForMode)
72+
maxStopCountLimit.equals(that.maxStopCountLimit)
8273
);
8374
}
8475

8576
@Override
8677
public int hashCode() {
87-
return Objects.hash(penalty, maxDuration, defaultMaxStopCount, maxStopCountForMode);
78+
return Objects.hash(penalty, maxDuration, maxStopCountLimit);
8879
}
8980

9081
@Override
@@ -93,8 +84,7 @@ public String toString() {
9384
.of(AccessEgressPreferences.class)
9485
.addObj("penalty", penalty, DEFAULT.penalty)
9586
.addObj("maxDuration", maxDuration, DEFAULT.maxDuration)
96-
.addObj("defaultMaxStopCount", defaultMaxStopCount, DEFAULT.defaultMaxStopCount)
97-
.addObj("maxStopCountForMode", maxStopCountForMode, DEFAULT.maxStopCountForMode)
87+
.addObj("maxStopCount", maxStopCountLimit, DEFAULT.maxStopCountLimit)
9888
.toString();
9989
}
10090

@@ -103,15 +93,13 @@ public static class Builder {
10393
private final AccessEgressPreferences original;
10494
private TimeAndCostPenaltyForEnum<StreetMode> penalty;
10595
private DurationForEnum<StreetMode> maxDuration;
106-
private Map<StreetMode, Integer> maxStopCountForMode;
107-
private int defaultMaxStopCount;
96+
private MaxStopCountLimit maxStopCountLimit;
10897

10998
public Builder(AccessEgressPreferences original) {
11099
this.original = original;
111100
this.maxDuration = original.maxDuration;
112101
this.penalty = original.penalty;
113-
this.defaultMaxStopCount = original.defaultMaxStopCount;
114-
this.maxStopCountForMode = original.maxStopCountForMode;
102+
this.maxStopCountLimit = original.maxStopCountLimit;
115103
}
116104

117105
public Builder withMaxDuration(Consumer<DurationForEnum.Builder<StreetMode>> body) {
@@ -124,13 +112,18 @@ public Builder withMaxDuration(Duration defaultValue, Map<StreetMode, Duration>
124112
return withMaxDuration(b -> b.withDefault(defaultValue).withValues(values));
125113
}
126114

115+
public Builder withMaxStopCount(Consumer<MaxStopCountLimit.Builder> body) {
116+
this.maxStopCountLimit = this.maxStopCountLimit.copyOf().apply(body).build();
117+
return this;
118+
}
119+
127120
public Builder withMaxStopCount(
128121
int defaultMaxStopCount,
129122
Map<StreetMode, Integer> maxStopCountForMode
130123
) {
131-
this.defaultMaxStopCount = defaultMaxStopCount;
132-
this.maxStopCountForMode = maxStopCountForMode;
133-
return this;
124+
return withMaxStopCount(b ->
125+
b.withDefaultLimit(defaultMaxStopCount).withLimitsForModes(maxStopCountForMode)
126+
);
134127
}
135128

136129
public Builder withPenalty(Consumer<TimeAndCostPenaltyForEnum.Builder<StreetMode>> body) {

‎application/src/main/java/org/opentripplanner/routing/api/request/preference/MaxStopCountLimit.java

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
package org.opentripplanner.routing.api.request.preference;
2+
3+
import java.util.EnumMap;
4+
import java.util.Map;
5+
import java.util.Objects;
6+
import java.util.function.Consumer;
7+
import org.opentripplanner.routing.api.request.StreetMode;
8+
import org.opentripplanner.utils.lang.IntUtils;
9+
import org.opentripplanner.utils.tostring.ToStringBuilder;
10+
11+
/**
12+
* This class is used for storing default and mode specific stop count limits for access and egress
13+
* routing.
14+
*/
15+
public class MaxStopCountLimit {
16+
17+
private static final int DEFAULT_LIMIT = 500;
18+
private static final Map<StreetMode, Integer> DEFAULT_FOR_MODES = Map.of();
19+
20+
public static final MaxStopCountLimit DEFAULT = new MaxStopCountLimit();
21+
22+
private final int defaultLimit;
23+
private final Map<StreetMode, Integer> limitsForModes;
24+
25+
public MaxStopCountLimit() {
26+
this.defaultLimit = DEFAULT_LIMIT;
27+
this.limitsForModes = DEFAULT_FOR_MODES;
28+
}
29+
30+
MaxStopCountLimit(Builder builder) {
31+
this.defaultLimit = IntUtils.requireNotNegative(builder.defaultLimit());
32+
this.limitsForModes = builder.copyCustomLimits();
33+
}
34+
35+
public static Builder of() {
36+
return DEFAULT.copyOf();
37+
}
38+
39+
public Builder copyOf() {
40+
return new Builder(this);
41+
}
42+
43+
/**
44+
* Get the max stop count limit for mode. If no custom value is defined, default is used.
45+
*/
46+
public int limitForMode(StreetMode mode) {
47+
return limitsForModes.getOrDefault(mode, defaultLimit);
48+
}
49+
50+
public int defaultLimit() {
51+
return defaultLimit;
52+
}
53+
54+
@Override
55+
public String toString() {
56+
return ToStringBuilder
57+
.of(MaxStopCountLimit.class)
58+
.addNum("defaultLimit", defaultLimit, DEFAULT_LIMIT)
59+
.addObj("limitsForModes", limitsForModes, DEFAULT_FOR_MODES)
60+
.toString();
61+
}
62+
63+
@Override
64+
public boolean equals(Object o) {
65+
if (this == o) return true;
66+
if (o == null || getClass() != o.getClass()) return false;
67+
68+
var that = (MaxStopCountLimit) o;
69+
70+
return (defaultLimit == that.defaultLimit && limitsForModes.equals(that.limitsForModes));
71+
}
72+
73+
@Override
74+
public int hashCode() {
75+
return Objects.hash(defaultLimit, limitsForModes);
76+
}
77+
78+
private Map<StreetMode, Integer> limitsForModes() {
79+
return limitsForModes;
80+
}
81+
82+
private boolean hasCustomLimit(StreetMode mode) {
83+
return limitsForModes.containsKey(mode);
84+
}
85+
86+
public static class Builder {
87+
88+
private final MaxStopCountLimit original;
89+
private int defaultLimit;
90+
private Map<StreetMode, Integer> limitsForModes = null;
91+
92+
Builder(MaxStopCountLimit original) {
93+
this.original = original;
94+
this.defaultLimit = original.defaultLimit();
95+
}
96+
97+
int defaultLimit() {
98+
return defaultLimit;
99+
}
100+
101+
public Builder withDefaultLimit(int defaultLimit) {
102+
this.defaultLimit = defaultLimit;
103+
return this;
104+
}
105+
106+
public Builder with(StreetMode mode, Integer limit) {
107+
// Lazy initialize the valueForEnum map
108+
if (limitsForModes == null) {
109+
limitsForModes = new EnumMap<>(StreetMode.class);
110+
for (StreetMode it : StreetMode.values()) {
111+
if (original.hasCustomLimit(it)) {
112+
limitsForModes.put(it, original.limitForMode(it));
113+
}
114+
}
115+
}
116+
limitsForModes.put(mode, limit);
117+
return this;
118+
}
119+
120+
public Builder withLimitsForModes(Map<StreetMode, Integer> limitsForModes) {
121+
for (Map.Entry<StreetMode, Integer> e : limitsForModes.entrySet()) {
122+
with(e.getKey(), e.getValue());
123+
}
124+
return this;
125+
}
126+
127+
/**
128+
* Build a copy of the current values, excluding the defaultLimit from the map. This
129+
* ensures equality and makes a defensive copy of the builder values. Hence, the builder
130+
* can be used to generate new values if desired.
131+
* */
132+
Map<StreetMode, Integer> copyCustomLimits() {
133+
if (limitsForModes == null) {
134+
// The limitForMode is protected and should never be mutated, so we can reuse it
135+
return original.limitsForModes();
136+
}
137+
138+
var copy = new EnumMap<StreetMode, Integer>(StreetMode.class);
139+
for (Map.Entry<StreetMode, Integer> it : limitsForModes.entrySet()) {
140+
if (defaultLimit != it.getValue()) {
141+
copy.put(it.getKey(), it.getValue());
142+
}
143+
}
144+
return copy;
145+
}
146+
147+
public Builder apply(Consumer<Builder> body) {
148+
body.accept(this);
149+
return this;
150+
}
151+
152+
public MaxStopCountLimit build() {
153+
var it = new MaxStopCountLimit(this);
154+
155+
// Return the original if there are no changes, the subscriber is not notified.
156+
if (original != null && original.equals(it)) {
157+
return original;
158+
}
159+
160+
return it;
161+
}
162+
}
163+
}

‎application/src/main/java/org/opentripplanner/standalone/config/routerequest/RouteRequestConfig.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.opentripplanner.routing.api.request.preference.BikePreferences;
2828
import org.opentripplanner.routing.api.request.preference.CarPreferences;
2929
import org.opentripplanner.routing.api.request.preference.EscalatorPreferences;
30+
import org.opentripplanner.routing.api.request.preference.MaxStopCountLimit;
3031
import org.opentripplanner.routing.api.request.preference.RoutingPreferences;
3132
import org.opentripplanner.routing.api.request.preference.ScooterPreferences;
3233
import org.opentripplanner.routing.api.request.preference.StreetPreferences;
@@ -544,7 +545,7 @@ duration can be set per mode(`maxDurationForMode`), because some street modes se
544545
Safety limit to prevent access to and egress from too many stops.
545546
"""
546547
)
547-
.asInt(dftAccessEgress.defaultMaxStopCount()),
548+
.asInt(dftAccessEgress.maxStopCountLimit().defaultLimit()),
548549
cae
549550
.of("maxStopCountForMode")
550551
.since(V2_7)

0 commit comments

Comments
 (0)