updaters = new ArrayList<>();
+ for (var it : dataSources) {
+ updaters.add(fetchAndCreateUpdater(it));
+ }
LOG.info("{} updaters fetched", updaters.size());
-
return updaters;
}
+
+ private VehicleRentalUpdater fetchAndCreateUpdater(
+ GbfsVehicleRentalDataSourceParameters parameters
+ ) {
+ LOG.info("Fetched updater info for {} at url {}", parameters.network(), parameters.url());
+
+ VehicleRentalParameters vehicleRentalParameters = new VehicleRentalParameters(
+ "vehicle-rental-service-directory:" + parameters.network(),
+ DEFAULT_FREQUENCY,
+ parameters
+ );
+
+ var dataSource = VehicleRentalDataSourceFactory.create(
+ vehicleRentalParameters.sourceParameters(),
+ otpHttpClient
+ );
+ return new VehicleRentalUpdater(vehicleRentalParameters, dataSource, vertexLinker, repository);
+ }
+
+ private static JsonNode listSources(VehicleRentalServiceDirectoryFetcherParameters parameters) {
+ JsonNode node;
+ URI url = parameters.getUrl();
+ try (OtpHttpClient otpHttpClient = new OtpHttpClient()) {
+ node = otpHttpClient.getAndMapAsJsonNode(url, Map.of(), new ObjectMapper());
+ } catch (OtpHttpClientException e) {
+ LOG.warn("Error fetching list of vehicle rental endpoints from {}", url, e);
+ return MissingNode.getInstance();
+ }
+ if (node == null) {
+ LOG.warn("Error reading json from {}. Node is null!", url);
+ return MissingNode.getInstance();
+ }
+
+ String sourcesName = parameters.getSourcesName();
+ JsonNode sources = node.get(sourcesName);
+ if (sources == null) {
+ LOG.warn(
+ "Error reading json from {}. No JSON node for sources name '{}' found.",
+ url,
+ sourcesName
+ );
+ return MissingNode.getInstance();
+ }
+ return sources;
+ }
}
diff --git a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/NetworkParameters.java b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/NetworkParameters.java
new file mode 100644
index 00000000000..25d394f72ec
--- /dev/null
+++ b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/NetworkParameters.java
@@ -0,0 +1,19 @@
+package org.opentripplanner.ext.vehiclerentalservicedirectory.api;
+
+import org.opentripplanner.updater.vehicle_rental.datasources.params.GbfsVehicleRentalDataSourceParameters;
+
+/**
+ * Parameters for a specific network.
+ *
+ * The {@link GbfsVehicleRentalDataSourceParameters} support {@code overloadingAllowed} and
+ * {@code allowKeepingRentedVehicleAtDestination} is not included here since they are not part of
+ * the GBFS specification. If there is a demand for these, they can be added.
+ *
+ * @param network The network name
+ * @param geofencingZones enable geofencingZones for the given network
+ */
+public record NetworkParameters(String network, boolean geofencingZones) {
+ public NetworkParameters withName(String network) {
+ return new NetworkParameters(network, geofencingZones);
+ }
+}
diff --git a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/VehicleRentalServiceDirectoryFetcherParameters.java b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/VehicleRentalServiceDirectoryFetcherParameters.java
index 2be6ea5d1d2..9b67114b499 100644
--- a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/VehicleRentalServiceDirectoryFetcherParameters.java
+++ b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/VehicleRentalServiceDirectoryFetcherParameters.java
@@ -1,10 +1,16 @@
package org.opentripplanner.ext.vehiclerentalservicedirectory.api;
import java.net.URI;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Optional;
+import java.util.stream.Collectors;
+import javax.annotation.Nullable;
import org.opentripplanner.updater.spi.HttpHeaders;
public class VehicleRentalServiceDirectoryFetcherParameters {
+ public static final String DEFAULT_NETWORK_NAME = "default-network";
private final URI url;
private final String sourcesName;
@@ -17,13 +23,19 @@ public class VehicleRentalServiceDirectoryFetcherParameters {
private final String language;
+ private final Map parametersForNetwork;
+
+ @Nullable
+ private final NetworkParameters defaultNetwork;
+
public VehicleRentalServiceDirectoryFetcherParameters(
URI url,
String sourcesName,
String updaterUrlName,
String networkName,
String language,
- HttpHeaders headers
+ HttpHeaders headers,
+ Collection networkParameters
) {
this.url = url;
this.sourcesName = sourcesName;
@@ -31,6 +43,9 @@ public VehicleRentalServiceDirectoryFetcherParameters(
this.sourceNetworkName = networkName;
this.language = language;
this.headers = headers;
+ this.parametersForNetwork =
+ networkParameters.stream().collect(Collectors.toMap(NetworkParameters::network, it -> it));
+ this.defaultNetwork = parametersForNetwork.get(DEFAULT_NETWORK_NAME);
}
/**
@@ -81,4 +96,8 @@ public HttpHeaders getHeaders() {
public String getLanguage() {
return language;
}
+
+ public Optional networkParameters(String network) {
+ return Optional.ofNullable(parametersForNetwork.getOrDefault(network, defaultNetwork));
+ }
}
diff --git a/src/main/java/org/opentripplanner/framework/json/JsonUtils.java b/src/main/java/org/opentripplanner/framework/json/JsonUtils.java
new file mode 100644
index 00000000000..1f235eb6483
--- /dev/null
+++ b/src/main/java/org/opentripplanner/framework/json/JsonUtils.java
@@ -0,0 +1,17 @@
+package org.opentripplanner.framework.json;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import java.util.Optional;
+import javax.annotation.Nonnull;
+
+public class JsonUtils {
+
+ public static Optional asText(@Nonnull JsonNode node, @Nonnull String field) {
+ JsonNode valueNode = node.get(field);
+ if (valueNode == null) {
+ return Optional.empty();
+ }
+ String value = valueNode.asText();
+ return value.isEmpty() ? Optional.empty() : Optional.of(value);
+ }
+}
diff --git a/src/main/java/org/opentripplanner/framework/logging/AbstractFilterLogger.java b/src/main/java/org/opentripplanner/framework/logging/AbstractFilterLogger.java
index e753f0bbd0c..5fe1ef4c8e7 100644
--- a/src/main/java/org/opentripplanner/framework/logging/AbstractFilterLogger.java
+++ b/src/main/java/org/opentripplanner/framework/logging/AbstractFilterLogger.java
@@ -15,15 +15,17 @@
* The primary use-case for this class is to prevent a spamming the log with the same kind
* of events. There are two concrete implementations:
*
- * - {@link ThrottleLogger} - Prevent more than one log event within the same second. This is
- * nice for the journey planner operation.
* - {@link MaxCountLogger} - Log N events, then mute. This is suitable for data import.
*
*
*
* This class wrap the original logger and forward some log events, based on the
* implementation of the {@link #mute()} method.
+ *
+ * @deprecated This hide the actual logger in the log, the AbstractFilterLogger becomes thelogger -
+ this make it difficult to find the log statement in the code when investigating.
*/
+@Deprecated
public abstract class AbstractFilterLogger implements Logger {
private final Logger delegate;
diff --git a/src/main/java/org/opentripplanner/framework/logging/MaxCountLogger.java b/src/main/java/org/opentripplanner/framework/logging/MaxCountLogger.java
index 0cfa7004f3c..05df3f3e45c 100644
--- a/src/main/java/org/opentripplanner/framework/logging/MaxCountLogger.java
+++ b/src/main/java/org/opentripplanner/framework/logging/MaxCountLogger.java
@@ -15,20 +15,25 @@
* message. After a given limit this logger will be muted and no more log events are logged.
*
* THREAD SAFETY - The implementation is not thread safe.
+ *
+ * @deprecated TODO: Rewrite the same way as the {@link Throttle} is done. See
+ * {@link AbstractFilterLogger} for deprecation details.
*/
+@Deprecated
public class MaxCountLogger extends AbstractFilterLogger {
private static final int MAX_COUNT = 10;
private int count = 0;
- public MaxCountLogger(Logger delegate) {
+ private MaxCountLogger(Logger delegate) {
super(delegate);
}
/**
- * Wrap given logger, and throttle INFO, WARN and ERROR messages.
+ * Wrap given logger, and throttle INFO, WARN and ERROR messages. Maximum 10 messages is
+ * written to the log.
*/
- public static MaxCountLogger maxCount(Logger log) {
+ public static MaxCountLogger of(Logger log) {
return new MaxCountLogger(log);
}
diff --git a/src/main/java/org/opentripplanner/framework/logging/ThrottleLogger.java b/src/main/java/org/opentripplanner/framework/logging/Throttle.java
similarity index 55%
rename from src/main/java/org/opentripplanner/framework/logging/ThrottleLogger.java
rename to src/main/java/org/opentripplanner/framework/logging/Throttle.java
index 6958e003357..47417aa974a 100644
--- a/src/main/java/org/opentripplanner/framework/logging/ThrottleLogger.java
+++ b/src/main/java/org/opentripplanner/framework/logging/Throttle.java
@@ -1,52 +1,48 @@
package org.opentripplanner.framework.logging;
-import org.slf4j.Logger;
+import org.opentripplanner.framework.time.TimeUtils;
/**
- * This class can be used to throttle logging events with level:
- *
- * - INFO
- * - WARNING
- * - ERROR
- *
- * DEBUG and TRACE events are not throttled.
+ * This class can be used to throttle (logging) events.
*
* The primary use-case for this class is to prevent a logger for degrading the performance,
* because too many events are logged during a short period of time. This could happen if you are
* parsing thousands or millions of records and each of them will cause a log event to happen.
*
- * This class is used to wrap the original logger and it will forward only one log event for pr
- * second.
+ * To use it, wrap the log statement:
+ *
+ * THROTTLE.throttle(() -> LOG.warn("Cost mismatch ...", ...));
+ *
+ * By wrapping the log statement only one log event will occur per second.
*
* THREAD SAFETY - The implementation is very simple and do not do any synchronization, so it is
* possible that more than 1 log event is logged for each second, but that is the only thread
- * safety issue. It is safe to use in multi-threaded cases. See the JavaDoc on the private
+ * safety issue. It is safe to use in a multithreaded cases. See the JavaDoc on the private
* {@code throttle()} method for implementation details.
*/
-public class ThrottleLogger extends AbstractFilterLogger {
+public class Throttle {
- private static final int STALL_PERIOD_MILLISECONDS = 1000;
+ private final int quietPeriodMilliseconds;
private long timeout = Long.MIN_VALUE;
+ private final String setupInfo;
- private ThrottleLogger(Logger delegate) {
- super(delegate);
- delegate.info(
- "Logger {} is throttled, only one messages is logged for every {} second interval.",
- delegate.getName(),
- STALL_PERIOD_MILLISECONDS / 1000
- );
+ Throttle(int quietPeriodMilliseconds) {
+ this.quietPeriodMilliseconds = quietPeriodMilliseconds;
+ this.setupInfo = "(throttle " + TimeUtils.msToString(quietPeriodMilliseconds) + " interval)";
}
- /**
- * Wrap given logger, and throttle INFO, WARN and ERROR messages.
- */
- public static Logger throttle(Logger log) {
- return new ThrottleLogger(log);
+ public static Throttle ofOneSecond() {
+ return new Throttle(1000);
}
- @Override
- boolean mute() {
- return throttle();
+ public String setupInfo() {
+ return setupInfo;
+ }
+
+ public void throttle(Runnable body) {
+ if (!throttle()) {
+ body.run();
+ }
}
/**
@@ -57,17 +53,17 @@ boolean mute() {
* least one event is logged for each throttle time period. This is guaranteed based on the
* assumption that writing to the {@code timeout} (primitive long) is an atomic operation.
*
- * In a worst case scenario, each thread keep their local version of the {@code timeout} and one
+ * In the worst case scenario, each thread keep their local version of the {@code timeout} and one
* log message from each thread is printed every second. This can behave differently from one JVM
- * to anther.
+ * to another.
*/
- private boolean throttle() {
+ public boolean throttle() {
long time = System.currentTimeMillis();
if (time < timeout) {
return true;
}
- timeout = time + STALL_PERIOD_MILLISECONDS;
+ timeout = time + quietPeriodMilliseconds;
return false;
}
}
diff --git a/src/main/java/org/opentripplanner/framework/time/TimeUtils.java b/src/main/java/org/opentripplanner/framework/time/TimeUtils.java
index b8c51ac8f62..afeeb77ff5d 100644
--- a/src/main/java/org/opentripplanner/framework/time/TimeUtils.java
+++ b/src/main/java/org/opentripplanner/framework/time/TimeUtils.java
@@ -168,6 +168,35 @@ public static ZonedDateTime zonedDateTime(LocalDate date, int seconds, ZoneId zo
return RelativeTime.ofSeconds(seconds).toZonedDateTime(date, zoneId);
}
+ /**
+ * Convert system time in milliseconds to a sting:
+ *
+ * -1100 -> -1.1s
+ * 0 -> 0s
+ * 1000 -> 1s
+ * 1001 -> 1.001s
+ * 1010 -> 1.01s
+ * 1100 -> 1.1s
+ * 23456 -> 23.456s
+ *
+ */
+ public static String msToString(long milliseconds) {
+ long seconds = milliseconds / 1000L;
+ int decimals = Math.abs((int) (milliseconds % 1000));
+ if (decimals == 0) {
+ return seconds + "s";
+ }
+ if (decimals % 10 == 0) {
+ decimals = decimals / 10;
+ if (decimals % 10 == 0) {
+ decimals = decimals / 10;
+ return seconds + "." + decimals + "s";
+ }
+ return seconds + "." + String.format("%02d", decimals) + "s";
+ }
+ return seconds + "." + String.format("%03d", decimals) + "s";
+ }
+
/**
* Wait (compute) until the given {@code waitMs} is past. The returned long is a very random
* number. If this method is called twice a grace period of 5 times the wait-time is set. All
diff --git a/src/main/java/org/opentripplanner/netex/loader/parser/ServiceFrameParser.java b/src/main/java/org/opentripplanner/netex/loader/parser/ServiceFrameParser.java
index 280b9c64669..859ec92427e 100644
--- a/src/main/java/org/opentripplanner/netex/loader/parser/ServiceFrameParser.java
+++ b/src/main/java/org/opentripplanner/netex/loader/parser/ServiceFrameParser.java
@@ -1,7 +1,5 @@
package org.opentripplanner.netex.loader.parser;
-import static org.opentripplanner.framework.logging.MaxCountLogger.maxCount;
-
import jakarta.xml.bind.JAXBElement;
import java.util.ArrayList;
import java.util.Collection;
@@ -37,7 +35,7 @@
class ServiceFrameParser extends NetexParser {
private static final Logger LOG = LoggerFactory.getLogger(ServiceFrameParser.class);
- private static final MaxCountLogger PASSENGER_STOP_ASSIGNMENT_LOGGER = maxCount(LOG);
+ private static final MaxCountLogger PASSENGER_STOP_ASSIGNMENT_LOGGER = MaxCountLogger.of(LOG);
private final ReadOnlyHierarchicalMapById flexibleStopPlaceById;
diff --git a/src/main/java/org/opentripplanner/raptor/rangeraptor/path/DestinationArrivalPaths.java b/src/main/java/org/opentripplanner/raptor/rangeraptor/path/DestinationArrivalPaths.java
index 13ce34038ba..5f0fa376f74 100644
--- a/src/main/java/org/opentripplanner/raptor/rangeraptor/path/DestinationArrivalPaths.java
+++ b/src/main/java/org/opentripplanner/raptor/rangeraptor/path/DestinationArrivalPaths.java
@@ -4,7 +4,7 @@
import java.util.Collection;
import javax.annotation.Nullable;
import org.opentripplanner.framework.lang.OtpNumberFormat;
-import org.opentripplanner.framework.logging.ThrottleLogger;
+import org.opentripplanner.framework.logging.Throttle;
import org.opentripplanner.raptor.api.model.RaptorAccessEgress;
import org.opentripplanner.raptor.api.model.RaptorConstants;
import org.opentripplanner.raptor.api.model.RaptorTripSchedule;
@@ -40,7 +40,7 @@
public class DestinationArrivalPaths {
private static final Logger LOG = LoggerFactory.getLogger(DestinationArrivalPaths.class);
- private static final Logger LOG_MISS_MATCH = ThrottleLogger.throttle(LOG);
+ private static final Throttle THROTTLE_MISS_MATCH = Throttle.ofOneSecond();
private final ParetoSet> paths;
private final RaptorTransitCalculator transitCalculator;
@@ -207,11 +207,14 @@ private void assertGeneralizedCostIsCalculatedCorrectByMapper(
) {
if (path.c1() != destArrival.c1()) {
// TODO - Bug: Cost mismatch stop-arrivals and paths #3623
- LOG_MISS_MATCH.warn(
- "Cost mismatch - Mapper: {}, stop-arrivals: {}, path: {}",
- OtpNumberFormat.formatCostCenti(path.c1()),
- raptorCostsAsString(destArrival),
- path.toStringDetailed(stopNameResolver)
+ THROTTLE_MISS_MATCH.throttle(() ->
+ LOG.warn(
+ "Cost mismatch - Mapper: {}, stop-arrivals: {}, path: {} {}",
+ OtpNumberFormat.formatCostCenti(path.c1()),
+ raptorCostsAsString(destArrival),
+ path.toStringDetailed(stopNameResolver),
+ THROTTLE_MISS_MATCH.setupInfo()
+ )
);
}
}
diff --git a/src/main/java/org/opentripplanner/routing/algorithm/transferoptimization/OptimizeTransferService.java b/src/main/java/org/opentripplanner/routing/algorithm/transferoptimization/OptimizeTransferService.java
index a45288eaa07..8a4f25d8910 100644
--- a/src/main/java/org/opentripplanner/routing/algorithm/transferoptimization/OptimizeTransferService.java
+++ b/src/main/java/org/opentripplanner/routing/algorithm/transferoptimization/OptimizeTransferService.java
@@ -3,7 +3,7 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
-import org.opentripplanner.framework.logging.ThrottleLogger;
+import org.opentripplanner.framework.logging.Throttle;
import org.opentripplanner.raptor.api.model.RaptorTripSchedule;
import org.opentripplanner.raptor.api.path.RaptorPath;
import org.opentripplanner.routing.algorithm.raptoradapter.path.PathDiff;
@@ -20,7 +20,7 @@
public class OptimizeTransferService {
private static final Logger LOG = LoggerFactory.getLogger(OptimizeTransferService.class);
- private static final Logger OPTIMIZATION_FAILED_LOG = ThrottleLogger.throttle(LOG);
+ private static final Throttle THROTTLE_OPTIMIZATION_FAILED = Throttle.ofOneSecond();
private final OptimizePathDomainService optimizePathDomainService;
private final MinSafeTransferTimeCalculator minSafeTransferTimeCalculator;
@@ -84,11 +84,14 @@ private Collection> optimize(RaptorPath path) {
try {
return optimizePathDomainService.findBestTransitPath(path);
} catch (RuntimeException e) {
- OPTIMIZATION_FAILED_LOG.warn(
- "Unable to optimize transfers in path. Details: {}, path: {}",
- e.getMessage(),
- path,
- e
+ THROTTLE_OPTIMIZATION_FAILED.throttle(() ->
+ LOG.warn(
+ "Unable to optimize transfers in path. Details: {}, path: {} {}",
+ e.getMessage(),
+ path,
+ THROTTLE_OPTIMIZATION_FAILED.setupInfo(),
+ e
+ )
);
return List.of(new OptimizedPath<>(path));
}
diff --git a/src/main/java/org/opentripplanner/standalone/config/sandbox/VehicleRentalServiceDirectoryFetcherConfig.java b/src/main/java/org/opentripplanner/standalone/config/sandbox/VehicleRentalServiceDirectoryFetcherConfig.java
index 79562b28e6b..b30fc3d5b3f 100644
--- a/src/main/java/org/opentripplanner/standalone/config/sandbox/VehicleRentalServiceDirectoryFetcherConfig.java
+++ b/src/main/java/org/opentripplanner/standalone/config/sandbox/VehicleRentalServiceDirectoryFetcherConfig.java
@@ -1,8 +1,11 @@
package org.opentripplanner.standalone.config.sandbox;
-import static org.opentripplanner.standalone.config.framework.json.OtpVersion.NA;
import static org.opentripplanner.standalone.config.framework.json.OtpVersion.V2_0;
+import static org.opentripplanner.standalone.config.framework.json.OtpVersion.V2_1;
+import static org.opentripplanner.standalone.config.framework.json.OtpVersion.V2_4;
+import java.util.List;
+import org.opentripplanner.ext.vehiclerentalservicedirectory.api.NetworkParameters;
import org.opentripplanner.ext.vehiclerentalservicedirectory.api.VehicleRentalServiceDirectoryFetcherParameters;
import org.opentripplanner.standalone.config.framework.json.NodeAdapter;
import org.opentripplanner.standalone.config.routerconfig.updaters.HttpHeadersConfig;
@@ -24,24 +27,60 @@ public static VehicleRentalServiceDirectoryFetcherParameters create(
}
return new VehicleRentalServiceDirectoryFetcherParameters(
- c.of("url").since(NA).summary("Endpoint for the VehicleRentalServiceDirectory").asUri(),
+ c.of("url").since(V2_1).summary("Endpoint for the VehicleRentalServiceDirectory").asUri(),
c
.of("sourcesName")
- .since(NA)
+ .since(V2_1)
.summary("Json tag name for updater sources.")
.asString("systems"),
c
.of("updaterUrlName")
- .since(NA)
+ .since(V2_1)
.summary("Json tag name for endpoint urls for each source.")
.asString("url"),
c
.of("updaterNetworkName")
- .since(NA)
+ .since(V2_1)
.summary("Json tag name for the network name for each source.")
.asString("id"),
- c.of("language").since(NA).summary("Language code.").asString(null),
- HttpHeadersConfig.headers(c, NA)
+ c.of("language").since(V2_1).summary("Language code.").asString(null),
+ HttpHeadersConfig.headers(c, V2_1),
+ mapNetworkParameters("networks", c)
);
}
+
+ private static List mapNetworkParameters(
+ String parameterName,
+ NodeAdapter root
+ ) {
+ return root
+ .of(parameterName)
+ .since(V2_4)
+ .summary(
+ "List all networks to include. Use \"network\": \"" +
+ VehicleRentalServiceDirectoryFetcherParameters.DEFAULT_NETWORK_NAME +
+ "\" to set defaults."
+ )
+ .description(
+ """
+ If no default network exists only the listed networks are used. Configure a network with
+ name "{{default-network}}" to include all unlisted networks. If not present, all unlisted
+ networks are dropped. Note! The values in the "{{default-network}}" are not used to set
+ missing field values in networks listed.
+ """.replace(
+ "{{default-network}}",
+ VehicleRentalServiceDirectoryFetcherParameters.DEFAULT_NETWORK_NAME
+ )
+ )
+ .asObjects(c ->
+ new NetworkParameters(
+ c.of("network").since(V2_4).summary("The network name").asString(),
+ c
+ .of("geofencingZones")
+ .since(V2_4)
+ .summary("Enables geofencingZones for the given network")
+ .asBoolean(false)
+ )
+ );
+ }
}
diff --git a/src/main/java/org/opentripplanner/updater/vehicle_rental/VehicleRentalUpdater.java b/src/main/java/org/opentripplanner/updater/vehicle_rental/VehicleRentalUpdater.java
index efecff2eddc..1e037a7c1c2 100644
--- a/src/main/java/org/opentripplanner/updater/vehicle_rental/VehicleRentalUpdater.java
+++ b/src/main/java/org/opentripplanner/updater/vehicle_rental/VehicleRentalUpdater.java
@@ -10,6 +10,9 @@
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
+import org.opentripplanner.framework.lang.ObjectUtils;
+import org.opentripplanner.framework.logging.Throttle;
+import org.opentripplanner.framework.time.DurationUtils;
import org.opentripplanner.framework.time.TimeUtils;
import org.opentripplanner.framework.tostring.ToStringBuilder;
import org.opentripplanner.routing.graph.Graph;
@@ -45,13 +48,18 @@
public class VehicleRentalUpdater extends PollingGraphUpdater {
private static final Logger LOG = LoggerFactory.getLogger(VehicleRentalUpdater.class);
+
+ private final Throttle unlinkedPlaceThrottle;
+
private final VehicleRentalDatasource source;
+ private final String nameForLogging;
+
private WriteToGraphCallback saveResultOnGraph;
private Map latestModifiedEdges = Map.of();
private Set latestAppliedGeofencingZones = Set.of();
- Map verticesByStation = new HashMap<>();
- Map tempEdgesByStation = new HashMap<>();
+ private final Map verticesByStation = new HashMap<>();
+ private final Map tempEdgesByStation = new HashMap<>();
private final VertexLinker linker;
private final VehicleRentalRepository service;
@@ -64,9 +72,15 @@ public VehicleRentalUpdater(
) throws IllegalArgumentException {
super(parameters);
// Configure updater
- LOG.info("Setting up vehicle rental updater.");
+ LOG.info("Setting up vehicle rental updater for {}.", source);
this.source = source;
+ this.nameForLogging =
+ ObjectUtils.ifNotNull(
+ parameters.sourceParameters().network(),
+ parameters.sourceParameters().url()
+ );
+ this.unlinkedPlaceThrottle = Throttle.ofOneSecond();
// Creation of network linker library will not modify the graph
this.linker = vertexLinker;
@@ -78,16 +92,19 @@ public VehicleRentalUpdater(
// Do any setup if needed
source.setup();
} catch (UpdaterConstructionException e) {
- LOG.warn("Unable to setup updater: {}", this, e);
+ LOG.warn("Unable to setup updater: {}", nameForLogging, e);
}
if (runOnlyOnce()) {
- LOG.info("Creating vehicle-rental updater running once only (non-polling): {}", source);
+ LOG.info(
+ "Creating vehicle-rental updater running once only (non-polling): {}",
+ nameForLogging
+ );
} else {
LOG.info(
- "Creating vehicle-rental updater running every {} seconds: {}",
- pollingPeriod(),
- source
+ "Creating vehicle-rental updater running every {}: {}",
+ DurationUtils.durationToStr(pollingPeriod()),
+ nameForLogging
);
}
}
@@ -109,9 +126,9 @@ public String getConfigRef() {
@Override
protected void runPolling() {
- LOG.debug("Updating vehicle rental stations from {}", source);
+ LOG.debug("Updating vehicle rental stations from {}", nameForLogging);
if (!source.update()) {
- LOG.debug("No updates");
+ LOG.debug("No updates from {}", nameForLogging);
return;
}
List stations = source.getUpdates();
@@ -149,6 +166,7 @@ public void run(Graph graph, TransitModel transitModel) {
service.addVehicleRentalStation(station);
stationSet.add(station.getId());
VehicleRentalPlaceVertex vehicleRentalVertex = verticesByStation.get(station.getId());
+
if (vehicleRentalVertex == null) {
vehicleRentalVertex = vertexFactory.vehicleRentalPlace(station);
DisposableEdgeCollection tempEdges = linker.linkVertexForRealTime(
@@ -168,8 +186,17 @@ public void run(Graph graph, TransitModel transitModel) {
)
);
if (vehicleRentalVertex.getOutgoing().isEmpty()) {
- // the toString includes the text "Bike rental station"
- LOG.info("VehicleRentalPlace {} is unlinked", vehicleRentalVertex);
+ // Copy reference to pass into lambda
+ var vrv = vehicleRentalVertex;
+ unlinkedPlaceThrottle.throttle(() ->
+ // the toString includes the text "Bike rental station"
+ LOG.warn(
+ "VehicleRentalPlace is unlinked for {}: {} {}",
+ nameForLogging,
+ vrv,
+ unlinkedPlaceThrottle.setupInfo()
+ )
+ );
}
Set formFactors = Stream
.concat(
@@ -188,6 +215,7 @@ public void run(Graph graph, TransitModel transitModel) {
vehicleRentalVertex.setStation(station);
}
}
+
/* remove existing stations that were not present in the update */
List toRemove = new ArrayList<>();
for (Entry entry : verticesByStation.entrySet()) {
@@ -206,7 +234,7 @@ public void run(Graph graph, TransitModel transitModel) {
// this check relies on the generated equals for the record which also recursively checks that
// the JTS geometries are equal
if (!geofencingZones.isEmpty() && !geofencingZones.equals(latestAppliedGeofencingZones)) {
- LOG.info("Computing geofencing zones");
+ LOG.info("Computing geofencing zones for {}", nameForLogging);
var start = System.currentTimeMillis();
latestModifiedEdges.forEach(StreetEdge::removeRentalExtension);
@@ -218,9 +246,10 @@ public void run(Graph graph, TransitModel transitModel) {
var end = System.currentTimeMillis();
var millis = Duration.ofMillis(end - start);
LOG.info(
- "Geofencing zones computation took {}. Added extension to {} edges.",
+ "Geofencing zones computation took {}. Added extension to {} edges. For {}",
TimeUtils.durationToStrCompact(millis),
- latestModifiedEdges.size()
+ latestModifiedEdges.size(),
+ nameForLogging
);
}
}
diff --git a/src/main/java/org/opentripplanner/updater/vehicle_rental/datasources/GbfsVehicleRentalDataSource.java b/src/main/java/org/opentripplanner/updater/vehicle_rental/datasources/GbfsVehicleRentalDataSource.java
index 40343ec6a91..8cc8897f0de 100644
--- a/src/main/java/org/opentripplanner/updater/vehicle_rental/datasources/GbfsVehicleRentalDataSource.java
+++ b/src/main/java/org/opentripplanner/updater/vehicle_rental/datasources/GbfsVehicleRentalDataSource.java
@@ -22,6 +22,8 @@
import org.opentripplanner.service.vehiclerental.model.VehicleRentalPlace;
import org.opentripplanner.service.vehiclerental.model.VehicleRentalSystem;
import org.opentripplanner.updater.vehicle_rental.datasources.params.GbfsVehicleRentalDataSourceParameters;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* Created by demory on 2017-03-14.
@@ -33,11 +35,14 @@
*/
class GbfsVehicleRentalDataSource implements VehicleRentalDatasource {
+ private static final Logger LOG = LoggerFactory.getLogger(GbfsVehicleRentalDataSource.class);
+
private final GbfsVehicleRentalDataSourceParameters params;
+ private final OtpHttpClient otpHttpClient;
private GbfsFeedLoader loader;
private List geofencingZones = List.of();
- private final OtpHttpClient otpHttpClient;
+ private boolean logGeofencingZonesDoesNotExistWarning = true;
public GbfsVehicleRentalDataSource(
GbfsVehicleRentalDataSourceParameters parameters,
@@ -126,9 +131,18 @@ public List getUpdates() {
if (params.geofencingZones()) {
var zones = loader.getFeed(GBFSGeofencingZones.class);
-
- var mapper = new GbfsGeofencingZoneMapper(system.systemId);
- this.geofencingZones = mapper.mapGeofencingZone(zones);
+ if (zones != null) {
+ var mapper = new GbfsGeofencingZoneMapper(system.systemId);
+ this.geofencingZones = mapper.mapGeofencingZone(zones);
+ } else {
+ if (logGeofencingZonesDoesNotExistWarning) {
+ LOG.warn(
+ "GeofencingZones is enabled in OTP, but no zones exist for network: {}",
+ params.network()
+ );
+ }
+ logGeofencingZonesDoesNotExistWarning = false;
+ }
}
return stations;
}
diff --git a/src/main/java/org/opentripplanner/updater/vehicle_rental/datasources/params/VehicleRentalDataSourceParameters.java b/src/main/java/org/opentripplanner/updater/vehicle_rental/datasources/params/VehicleRentalDataSourceParameters.java
index 6f64bbaf64d..abf7aa805cd 100644
--- a/src/main/java/org/opentripplanner/updater/vehicle_rental/datasources/params/VehicleRentalDataSourceParameters.java
+++ b/src/main/java/org/opentripplanner/updater/vehicle_rental/datasources/params/VehicleRentalDataSourceParameters.java
@@ -1,6 +1,7 @@
package org.opentripplanner.updater.vehicle_rental.datasources.params;
import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
import org.opentripplanner.updater.spi.HttpHeaders;
import org.opentripplanner.updater.vehicle_rental.VehicleRentalSourceType;
@@ -8,6 +9,9 @@ public interface VehicleRentalDataSourceParameters {
@Nonnull
String url();
+ @Nullable
+ String network();
+
@Nonnull
VehicleRentalSourceType sourceType();
diff --git a/src/test/java/org/opentripplanner/framework/json/JsonUtilsTest.java b/src/test/java/org/opentripplanner/framework/json/JsonUtilsTest.java
new file mode 100644
index 00000000000..4f50e9ad1a5
--- /dev/null
+++ b/src/test/java/org/opentripplanner/framework/json/JsonUtilsTest.java
@@ -0,0 +1,35 @@
+package org.opentripplanner.framework.json;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.node.MissingNode;
+import com.fasterxml.jackson.databind.node.NullNode;
+import com.fasterxml.jackson.databind.node.TextNode;
+import java.util.Optional;
+import org.junit.jupiter.api.Test;
+
+class JsonUtilsTest {
+
+ private static final ObjectMapper MAPPER = new ObjectMapper();
+
+ @Test
+ void testAsText() throws JsonProcessingException {
+ assertTrue(JsonUtils.asText(MissingNode.getInstance(), "any").isEmpty());
+ assertTrue(JsonUtils.asText(NullNode.getInstance(), "any").isEmpty());
+ assertTrue(JsonUtils.asText(new TextNode("foo"), "bar").isEmpty());
+
+ JsonNode node = MAPPER.readTree("""
+ { "foo" : "bar", "array" : [] }
+ """);
+
+ Optional result = JsonUtils.asText(node, "foo");
+ assertTrue(result.isPresent());
+ assertEquals("bar", result.get());
+
+ assertTrue(JsonUtils.asText(node, "array").isEmpty());
+ }
+}
diff --git a/src/test/java/org/opentripplanner/framework/logging/ThrottleLoggerTest.java b/src/test/java/org/opentripplanner/framework/logging/ThrottleLoggerTest.java
deleted file mode 100644
index 2c0a9f2b19d..00000000000
--- a/src/test/java/org/opentripplanner/framework/logging/ThrottleLoggerTest.java
+++ /dev/null
@@ -1,48 +0,0 @@
-package org.opentripplanner.framework.logging;
-
-import java.util.ArrayList;
-import java.util.List;
-import org.junit.jupiter.api.Disabled;
-import org.junit.jupiter.api.Test;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-class ThrottleLoggerTest {
-
- private static final Logger LOG = LoggerFactory.getLogger(ThrottleLoggerTest.class);
- private static final Logger THROTTLED_LOG = ThrottleLogger.throttle(LOG);
-
- @Test
- @Disabled("Run this test manually")
- void warn() {
- List events = new ArrayList<>();
- for (int i = 0; i < 50_000_000; i++) {
- events.add(i);
- }
- long start = System.currentTimeMillis();
-
- events
- .parallelStream()
- .forEach(i ->
- THROTTLED_LOG.warn(String.format("%3.0f p", (System.currentTimeMillis() - start) / 1000.0))
- );
- /*
- EXPECTED OUTPUT
-
- 21:28:36.618 INFO (LogThrottle.java:30) Logger org.opentripplanner.util.logging.LogThrottleTest is throttled, only one messages is logged for every 1 second interval.
- 21:28:38.812 WARN (LogThrottle.java:264) 0 p
- 21:28:39.812 WARN (LogThrottle.java:264) 1 p
- 21:28:40.812 WARN (LogThrottle.java:264) 2 p
- 21:28:41.812 WARN (LogThrottle.java:264) 3 p
- 21:28:42.812 WARN (LogThrottle.java:264) 4 p
- 21:28:42.812 WARN (LogThrottle.java:264) 4 p
- 21:28:43.812 WARN (LogThrottle.java:264) 5 p
- 21:28:44.812 WARN (LogThrottle.java:264) 6 p
- 21:28:45.812 WARN (LogThrottle.java:264) 7 p
- 21:28:46.812 WARN (LogThrottle.java:264) 8 p <--- Duplicate for the 8. period
- 21:28:46.812 WARN (LogThrottle.java:264) 8 p <--- Duplicate for the 8. period
- 21:28:47.812 WARN (LogThrottle.java:264) 9 p
- 21:28:48.812 WARN (LogThrottle.java:264) 10 p
- */
- }
-}
diff --git a/src/test/java/org/opentripplanner/framework/logging/ThrottleTest.java b/src/test/java/org/opentripplanner/framework/logging/ThrottleTest.java
new file mode 100644
index 00000000000..91f1667486d
--- /dev/null
+++ b/src/test/java/org/opentripplanner/framework/logging/ThrottleTest.java
@@ -0,0 +1,80 @@
+package org.opentripplanner.framework.logging;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Locale;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+
+class ThrottleTest {
+
+ @Test
+ void testSetUp() {
+ Assertions.assertEquals("(throttle 1s interval)", Throttle.ofOneSecond().setupInfo());
+ }
+
+ @Test
+ void smokeTest() {
+ int SIZE = 2_000;
+ var counter = new AtomicInteger(0);
+ var events = createIntegerSequence(SIZE);
+ var subject = Throttle.ofOneSecond();
+
+ events.parallelStream().forEach(i -> subject.throttle(counter::incrementAndGet));
+ Assertions.assertTrue(
+ counter.get() > 0,
+ "The counter should be greater than 0: " + counter.get()
+ );
+ Assertions.assertTrue(
+ counter.get() < 100,
+ "The counter should be less than 10: " + counter.get()
+ );
+ }
+
+ @Test
+ @Disabled("Run this test manually")
+ void manualTest() {
+ double quietPeriodMs = 50.0;
+ var subject = new Throttle((int) quietPeriodMs);
+
+ List events = createIntegerSequence(20_000_000);
+ long start = System.currentTimeMillis();
+
+ events
+ .parallelStream()
+ .forEach(i ->
+ subject.throttle(() ->
+ System.err.printf(Locale.ROOT, "%d ms%n", (System.currentTimeMillis() - start))
+ )
+ );
+ /*
+ We get a lot of duplicates here because of "optimistic read/write" on shared memory - this is ok, as long as
+ it does not fail.
+
+ EXPECTED OUTPUT
+ 4 ms
+ 54 ms
+ 54 ms // Duplicate
+ :
+ 104 ms
+ 104 ms // Duplicate
+ :
+ 155 ms
+ 206 ms
+ 256 ms // Duplicate
+ 306 ms
+ 306 ms // Duplicate
+ :
+ */
+ }
+
+ private List createIntegerSequence(int size) {
+ List events = new ArrayList<>();
+ for (int i = 0; i < size; i++) {
+ events.add(i);
+ }
+ return events;
+ }
+}
diff --git a/src/test/java/org/opentripplanner/framework/time/TimeUtilsTest.java b/src/test/java/org/opentripplanner/framework/time/TimeUtilsTest.java
index 4d131a5fc1f..53a6e5d4be4 100644
--- a/src/test/java/org/opentripplanner/framework/time/TimeUtilsTest.java
+++ b/src/test/java/org/opentripplanner/framework/time/TimeUtilsTest.java
@@ -166,6 +166,22 @@ public void toZonedDateTimeDST() {
);
}
+ @Test
+ void testMsToString() {
+ assertEquals("0s", TimeUtils.msToString(0));
+ assertEquals("0.001s", TimeUtils.msToString(1));
+ assertEquals("0.012s", TimeUtils.msToString(12));
+ assertEquals("1s", TimeUtils.msToString(1000));
+ assertEquals("1.1s", TimeUtils.msToString(1100));
+ assertEquals("1.02s", TimeUtils.msToString(1020));
+ assertEquals("1.003s", TimeUtils.msToString(1003));
+ assertEquals("1.234s", TimeUtils.msToString(1234));
+
+ // Negative numbers
+ assertEquals("-1s", TimeUtils.msToString(-1000));
+ assertEquals("-1.234s", TimeUtils.msToString(-1234));
+ }
+
private static int time(int hour, int min, int sec) {
return 60 * (60 * hour + min) + sec;
}
diff --git a/src/test/java/org/opentripplanner/generate/doc/RouterConfigurationDocTest.java b/src/test/java/org/opentripplanner/generate/doc/RouterConfigurationDocTest.java
index ac6f02d9f83..d13f423ffc4 100644
--- a/src/test/java/org/opentripplanner/generate/doc/RouterConfigurationDocTest.java
+++ b/src/test/java/org/opentripplanner/generate/doc/RouterConfigurationDocTest.java
@@ -36,6 +36,7 @@ public class RouterConfigurationDocTest {
.skip("vectorTileLayers", "sandbox/MapboxVectorTilesApi.md")
.skipNestedElements("transferCacheRequests", "RouteRequest.md")
.skip("rideHailingServices", "sandbox/RideHailing.md")
+ .skip("vehicleRentalServiceDirectory", "sandbox/VehicleRentalServiceDirectory.md")
.build();
/**
diff --git a/src/test/java/org/opentripplanner/updater/vehicle_rental/VehicleRentalUpdaterTest.java b/src/test/java/org/opentripplanner/updater/vehicle_rental/VehicleRentalUpdaterTest.java
index ff8ecd4ea85..60f0389abe7 100644
--- a/src/test/java/org/opentripplanner/updater/vehicle_rental/VehicleRentalUpdaterTest.java
+++ b/src/test/java/org/opentripplanner/updater/vehicle_rental/VehicleRentalUpdaterTest.java
@@ -10,6 +10,7 @@
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
import org.junit.jupiter.api.Test;
import org.opentripplanner.routing.graph.Graph;
import org.opentripplanner.service.vehiclerental.internal.DefaultVehicleRentalService;
@@ -85,6 +86,12 @@ public String url() {
return "https://example.com";
}
+ @Nullable
+ @Override
+ public String network() {
+ return "Test";
+ }
+
@Nonnull
@Override
public VehicleRentalSourceType sourceType() {