Skip to content

Commit

Permalink
Merge pull request #6494 from HSLdevcom/maxStopCountForMode-default-fix
Browse files Browse the repository at this point in the history
Use default max stop count for mode from routing defaults in transfer requests
  • Loading branch information
optionsome authored Mar 4, 2025
2 parents d4bdcc5 + 9ed9728 commit 2c8cfdb
Show file tree
Hide file tree
Showing 9 changed files with 319 additions and 65 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public class DataOverlayParameters implements Serializable {
private final Map<Parameter, Double> values;

public DataOverlayParameters(Map<Parameter, Double> values) {
// Make a defencive copy to protect the map entries, this make this class immutable
// Make a defensive copy to protect the map entries, this makes the class immutable
// and thread safe
this.values = Map.copyOf(values);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,9 +272,7 @@ private Collection<? extends RoutingAccessEgress> fetchAccessEgresses(AccessEgre
.accessEgress();

Duration durationLimit = accessEgressPreferences.maxDuration().valueOf(mode);
int stopCountLimit = accessEgressPreferences
.maxStopCountForMode()
.getOrDefault(mode, accessEgressPreferences.defaultMaxStopCount());
int stopCountLimit = accessEgressPreferences.maxStopCountLimit().limitForMode(mode);

var nearbyStops = AccessEgressRouter.findAccessEgresses(
accessRequest,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ public Builder<E> with(E key, Duration value) {

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

// Return original if not change, subscriber is not notified
// Return the original if there are no changes, the subscriber is not notified.
if (original != null && original.equals(it)) {
return original;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

import java.io.Serializable;
import java.time.Duration;
import java.util.Collections;
import java.util.Map;
import java.util.Objects;
import java.util.function.Consumer;
Expand All @@ -28,21 +27,18 @@ public final class AccessEgressPreferences implements Serializable {

private final TimeAndCostPenaltyForEnum<StreetMode> penalty;
private final DurationForEnum<StreetMode> maxDuration;
private final int defaultMaxStopCount;
private final Map<StreetMode, Integer> maxStopCountForMode;
private final MaxStopCountLimit maxStopCountLimit;

private AccessEgressPreferences() {
this.maxDuration = durationForStreetModeOf(ofMinutes(45));
this.penalty = DEFAULT_TIME_AND_COST;
this.defaultMaxStopCount = 500;
this.maxStopCountForMode = Map.of();
this.maxStopCountLimit = new MaxStopCountLimit();
}

private AccessEgressPreferences(Builder builder) {
this.maxDuration = builder.maxDuration;
this.penalty = builder.penalty;
this.defaultMaxStopCount = builder.defaultMaxStopCount;
this.maxStopCountForMode = Collections.unmodifiableMap(builder.maxStopCountForMode);
this.maxStopCountLimit = builder.maxStopCountLimit;
}

public static Builder of() {
Expand All @@ -61,12 +57,8 @@ public DurationForEnum<StreetMode> maxDuration() {
return maxDuration;
}

public int defaultMaxStopCount() {
return defaultMaxStopCount;
}

public Map<StreetMode, Integer> maxStopCountForMode() {
return maxStopCountForMode;
public MaxStopCountLimit maxStopCountLimit() {
return maxStopCountLimit;
}

@Override
Expand All @@ -77,14 +69,13 @@ public boolean equals(Object o) {
return (
penalty.equals(that.penalty) &&
maxDuration.equals(that.maxDuration) &&
defaultMaxStopCount == that.defaultMaxStopCount &&
maxStopCountForMode.equals(that.maxStopCountForMode)
maxStopCountLimit.equals(that.maxStopCountLimit)
);
}

@Override
public int hashCode() {
return Objects.hash(penalty, maxDuration, defaultMaxStopCount, maxStopCountForMode);
return Objects.hash(penalty, maxDuration, maxStopCountLimit);
}

@Override
Expand All @@ -93,8 +84,7 @@ public String toString() {
.of(AccessEgressPreferences.class)
.addObj("penalty", penalty, DEFAULT.penalty)
.addObj("maxDuration", maxDuration, DEFAULT.maxDuration)
.addObj("defaultMaxStopCount", defaultMaxStopCount, DEFAULT.defaultMaxStopCount)
.addObj("maxStopCountForMode", maxStopCountForMode, DEFAULT.maxStopCountForMode)
.addObj("maxStopCount", maxStopCountLimit, DEFAULT.maxStopCountLimit)
.toString();
}

Expand All @@ -103,15 +93,13 @@ public static class Builder {
private final AccessEgressPreferences original;
private TimeAndCostPenaltyForEnum<StreetMode> penalty;
private DurationForEnum<StreetMode> maxDuration;
private Map<StreetMode, Integer> maxStopCountForMode;
private int defaultMaxStopCount;
private MaxStopCountLimit maxStopCountLimit;

public Builder(AccessEgressPreferences original) {
this.original = original;
this.maxDuration = original.maxDuration;
this.penalty = original.penalty;
this.defaultMaxStopCount = original.defaultMaxStopCount;
this.maxStopCountForMode = original.maxStopCountForMode;
this.maxStopCountLimit = original.maxStopCountLimit;
}

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

public Builder withMaxStopCount(Consumer<MaxStopCountLimit.Builder> body) {
this.maxStopCountLimit = this.maxStopCountLimit.copyOf().apply(body).build();
return this;
}

public Builder withMaxStopCount(
int defaultMaxStopCount,
Map<StreetMode, Integer> maxStopCountForMode
) {
this.defaultMaxStopCount = defaultMaxStopCount;
this.maxStopCountForMode = maxStopCountForMode;
return this;
return withMaxStopCount(b ->
b.withDefaultLimit(defaultMaxStopCount).withLimitsForModes(maxStopCountForMode)
);
}

public Builder withPenalty(Consumer<TimeAndCostPenaltyForEnum.Builder<StreetMode>> body) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
package org.opentripplanner.routing.api.request.preference;

import java.util.EnumMap;
import java.util.Map;
import java.util.Objects;
import java.util.function.Consumer;
import org.opentripplanner.routing.api.request.StreetMode;
import org.opentripplanner.utils.lang.IntUtils;
import org.opentripplanner.utils.tostring.ToStringBuilder;

/**
* This class is used for storing default and mode specific stop count limits for access and egress
* routing.
*/
public class MaxStopCountLimit {

private static final int DEFAULT_LIMIT = 500;
private static final Map<StreetMode, Integer> DEFAULT_FOR_MODES = Map.of();

public static final MaxStopCountLimit DEFAULT = new MaxStopCountLimit();

private final int defaultLimit;
private final Map<StreetMode, Integer> limitsForModes;

public MaxStopCountLimit() {
this.defaultLimit = DEFAULT_LIMIT;
this.limitsForModes = DEFAULT_FOR_MODES;
}

MaxStopCountLimit(Builder builder) {
this.defaultLimit = IntUtils.requireNotNegative(builder.defaultLimit());
this.limitsForModes = builder.copyCustomLimits();
}

public static Builder of() {
return DEFAULT.copyOf();
}

public Builder copyOf() {
return new Builder(this);
}

/**
* Get the max stop count limit for mode. If no custom value is defined, default is used.
*/
public int limitForMode(StreetMode mode) {
return limitsForModes.getOrDefault(mode, defaultLimit);
}

public int defaultLimit() {
return defaultLimit;
}

@Override
public String toString() {
return ToStringBuilder
.of(MaxStopCountLimit.class)
.addNum("defaultLimit", defaultLimit, DEFAULT_LIMIT)
.addObj("limitsForModes", limitsForModes, DEFAULT_FOR_MODES)
.toString();
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;

var that = (MaxStopCountLimit) o;

return (defaultLimit == that.defaultLimit && limitsForModes.equals(that.limitsForModes));
}

@Override
public int hashCode() {
return Objects.hash(defaultLimit, limitsForModes);
}

private Map<StreetMode, Integer> limitsForModes() {
return limitsForModes;
}

private boolean hasCustomLimit(StreetMode mode) {
return limitsForModes.containsKey(mode);
}

public static class Builder {

private final MaxStopCountLimit original;
private int defaultLimit;
private Map<StreetMode, Integer> limitsForModes = null;

Builder(MaxStopCountLimit original) {
this.original = original;
this.defaultLimit = original.defaultLimit();
}

int defaultLimit() {
return defaultLimit;
}

public Builder withDefaultLimit(int defaultLimit) {
this.defaultLimit = defaultLimit;
return this;
}

public Builder with(StreetMode mode, Integer limit) {
// Lazy initialize the valueForEnum map
if (limitsForModes == null) {
limitsForModes = new EnumMap<>(StreetMode.class);
for (StreetMode it : StreetMode.values()) {
if (original.hasCustomLimit(it)) {
limitsForModes.put(it, original.limitForMode(it));
}
}
}
limitsForModes.put(mode, limit);
return this;
}

public Builder withLimitsForModes(Map<StreetMode, Integer> limitsForModes) {
for (Map.Entry<StreetMode, Integer> e : limitsForModes.entrySet()) {
with(e.getKey(), e.getValue());
}
return this;
}

/**
* Build a copy of the current values, excluding the defaultLimit from the map. This
* ensures equality and makes a defensive copy of the builder values. Hence, the builder
* can be used to generate new values if desired.
* */
Map<StreetMode, Integer> copyCustomLimits() {
if (limitsForModes == null) {
// The limitForMode is protected and should never be mutated, so we can reuse it
return original.limitsForModes();
}

var copy = new EnumMap<StreetMode, Integer>(StreetMode.class);
for (Map.Entry<StreetMode, Integer> it : limitsForModes.entrySet()) {
if (defaultLimit != it.getValue()) {
copy.put(it.getKey(), it.getValue());
}
}
return copy;
}

public Builder apply(Consumer<Builder> body) {
body.accept(this);
return this;
}

public MaxStopCountLimit build() {
var it = new MaxStopCountLimit(this);

// Return the original if there are no changes, the subscriber is not notified.
if (original != null && original.equals(it)) {
return original;
}

return it;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.opentripplanner.routing.api.request.preference.BikePreferences;
import org.opentripplanner.routing.api.request.preference.CarPreferences;
import org.opentripplanner.routing.api.request.preference.EscalatorPreferences;
import org.opentripplanner.routing.api.request.preference.MaxStopCountLimit;
import org.opentripplanner.routing.api.request.preference.RoutingPreferences;
import org.opentripplanner.routing.api.request.preference.ScooterPreferences;
import org.opentripplanner.routing.api.request.preference.StreetPreferences;
Expand Down Expand Up @@ -544,7 +545,7 @@ duration can be set per mode(`maxDurationForMode`), because some street modes se
Safety limit to prevent access to and egress from too many stops.
"""
)
.asInt(dftAccessEgress.defaultMaxStopCount()),
.asInt(dftAccessEgress.maxStopCountLimit().defaultLimit()),
cae
.of("maxStopCountForMode")
.since(V2_7)
Expand Down
Loading

0 comments on commit 2c8cfdb

Please sign in to comment.