From 4264ab8ed8933574a2272ccb34d39756b43a3f2b Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Thu, 21 Dec 2023 12:08:51 +0100 Subject: [PATCH 1/5] Introduce fallback zone id if none could be derived from the system --- .../api/common/RoutingResource.java | 3 +- .../apis/gtfs/mapping/RouteRequestMapper.java | 3 +- .../framework/application/ZoneIdFallback.java | 39 +++++++++++++++++++ .../framework/logging/Throttle.java | 4 ++ .../mapping/GraphPathToItineraryMapper.java | 3 +- .../service/DefaultRoutingService.java | 3 +- 6 files changed, 51 insertions(+), 4 deletions(-) create mode 100644 src/main/java/org/opentripplanner/framework/application/ZoneIdFallback.java diff --git a/src/main/java/org/opentripplanner/api/common/RoutingResource.java b/src/main/java/org/opentripplanner/api/common/RoutingResource.java index 9cb4f139bc4..a97a8368187 100644 --- a/src/main/java/org/opentripplanner/api/common/RoutingResource.java +++ b/src/main/java/org/opentripplanner/api/common/RoutingResource.java @@ -21,6 +21,7 @@ import org.opentripplanner.api.parameter.QualifiedModeSet; import org.opentripplanner.ext.dataoverlay.api.DataOverlayParameters; import org.opentripplanner.framework.application.OTPFeature; +import org.opentripplanner.framework.application.ZoneIdFallback; import org.opentripplanner.framework.lang.StringUtils; import org.opentripplanner.framework.time.DurationUtils; import org.opentripplanner.routing.api.request.RouteRequest; @@ -711,7 +712,7 @@ protected RouteRequest buildRequest(MultivaluedMap queryParamete { //FIXME: move into setter method on routing request - ZoneId tz = serverContext.transitService().getTimeZone(); + ZoneId tz = ZoneIdFallback.zoneId(serverContext.transitService().getTimeZone()); if (date == null && time != null) { // Time was provided but not date LOG.debug("parsing ISO datetime {}", time); try { diff --git a/src/main/java/org/opentripplanner/apis/gtfs/mapping/RouteRequestMapper.java b/src/main/java/org/opentripplanner/apis/gtfs/mapping/RouteRequestMapper.java index ab9e8bce823..7b47f3231c9 100644 --- a/src/main/java/org/opentripplanner/apis/gtfs/mapping/RouteRequestMapper.java +++ b/src/main/java/org/opentripplanner/apis/gtfs/mapping/RouteRequestMapper.java @@ -16,6 +16,7 @@ import org.opentripplanner.api.parameter.QualifiedMode; import org.opentripplanner.api.parameter.QualifiedModeSet; import org.opentripplanner.apis.gtfs.GraphQLRequestContext; +import org.opentripplanner.framework.application.ZoneIdFallback; import org.opentripplanner.framework.graphql.GraphQLUtils; import org.opentripplanner.model.GenericLocation; import org.opentripplanner.routing.api.request.RouteRequest; @@ -54,7 +55,7 @@ public static RouteRequest toRouteRequest( request.setDateTime( environment.getArgument("date"), environment.getArgument("time"), - context.transitService().getTimeZone() + ZoneIdFallback.zoneId(context.transitService().getTimeZone()) ); callWith.argument("wheelchair", request::setWheelchair); diff --git a/src/main/java/org/opentripplanner/framework/application/ZoneIdFallback.java b/src/main/java/org/opentripplanner/framework/application/ZoneIdFallback.java new file mode 100644 index 00000000000..260fb40f0bf --- /dev/null +++ b/src/main/java/org/opentripplanner/framework/application/ZoneIdFallback.java @@ -0,0 +1,39 @@ +package org.opentripplanner.framework.application; + +import java.time.ZoneId; +import javax.annotation.Nullable; +import org.opentripplanner.framework.logging.Throttle; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * This class provides a fallback mechanism for retrieving a zone id (=time zone). + * If a ZoneId is not provided, it returns a default fallback ZoneId (UTC). + *

+ * This situation happens when you don't load any transit data into the graph but want to route + * anyway, perhaps only on the street network. + */ +public class ZoneIdFallback { + + private static final Logger LOG = LoggerFactory.getLogger(ZoneIdFallback.class); + private static final ZoneId FALLBACK = ZoneId.of("UTC"); + private static final Throttle THROTTLE = Throttle.ofOneMinute(); + + /** + * Accepts a nullable zone id (time zone) and returns UTC as the fallback. + */ + public static ZoneId zoneId(@Nullable ZoneId id) { + if (id == null) { + THROTTLE.throttle(() -> { + LOG.warn( + "Your instance doesn't contain a time zone (which is usually derived from transit data). Assuming {}.", + FALLBACK + ); + LOG.warn("Please double-check that transit data was correctly loaded."); + }); + return FALLBACK; + } else { + return id; + } + } +} diff --git a/src/main/java/org/opentripplanner/framework/logging/Throttle.java b/src/main/java/org/opentripplanner/framework/logging/Throttle.java index 47417aa974a..631d59a2697 100644 --- a/src/main/java/org/opentripplanner/framework/logging/Throttle.java +++ b/src/main/java/org/opentripplanner/framework/logging/Throttle.java @@ -35,6 +35,10 @@ public static Throttle ofOneSecond() { return new Throttle(1000); } + public static Throttle ofOneMinute() { + return new Throttle(1000 * 60); + } + public String setupInfo() { return setupInfo; } diff --git a/src/main/java/org/opentripplanner/routing/algorithm/mapping/GraphPathToItineraryMapper.java b/src/main/java/org/opentripplanner/routing/algorithm/mapping/GraphPathToItineraryMapper.java index 9858ccfee50..c363689d03d 100644 --- a/src/main/java/org/opentripplanner/routing/algorithm/mapping/GraphPathToItineraryMapper.java +++ b/src/main/java/org/opentripplanner/routing/algorithm/mapping/GraphPathToItineraryMapper.java @@ -16,6 +16,7 @@ import org.opentripplanner.ext.flex.FlexibleTransitLeg; import org.opentripplanner.ext.flex.edgetype.FlexTripEdge; import org.opentripplanner.framework.application.OTPFeature; +import org.opentripplanner.framework.application.ZoneIdFallback; import org.opentripplanner.framework.geometry.GeometryUtils; import org.opentripplanner.framework.i18n.I18NString; import org.opentripplanner.model.plan.ElevationProfile; @@ -58,7 +59,7 @@ public GraphPathToItineraryMapper( StreetNotesService streetNotesService, double ellipsoidToGeoidDifference ) { - this.timeZone = timeZone; + this.timeZone = ZoneIdFallback.zoneId(timeZone); this.streetNotesService = streetNotesService; this.ellipsoidToGeoidDifference = ellipsoidToGeoidDifference; } diff --git a/src/main/java/org/opentripplanner/routing/service/DefaultRoutingService.java b/src/main/java/org/opentripplanner/routing/service/DefaultRoutingService.java index 320ffb2117d..a9a887588dd 100644 --- a/src/main/java/org/opentripplanner/routing/service/DefaultRoutingService.java +++ b/src/main/java/org/opentripplanner/routing/service/DefaultRoutingService.java @@ -2,6 +2,7 @@ import java.time.ZoneId; import org.opentripplanner.framework.application.OTPRequestTimeoutException; +import org.opentripplanner.framework.application.ZoneIdFallback; import org.opentripplanner.framework.tostring.MultiLineToStringBuilder; import org.opentripplanner.model.plan.Itinerary; import org.opentripplanner.routing.algorithm.RoutingWorker; @@ -30,7 +31,7 @@ public class DefaultRoutingService implements RoutingService { public DefaultRoutingService(OtpServerRequestContext serverContext) { this.serverContext = serverContext; - this.timeZone = serverContext.transitService().getTimeZone(); + this.timeZone = ZoneIdFallback.zoneId(serverContext.transitService().getTimeZone()); } @Override From bf2bef8b6795720a1aa65911f40d17c6174fdd0d Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Thu, 21 Dec 2023 12:18:40 +0100 Subject: [PATCH 2/5] Move fallback class --- .../java/org/opentripplanner/api/common/RoutingResource.java | 2 +- .../{framework/application => api/support}/ZoneIdFallback.java | 2 +- .../opentripplanner/apis/gtfs/mapping/RouteRequestMapper.java | 2 +- .../routing/algorithm/mapping/GraphPathToItineraryMapper.java | 2 +- .../opentripplanner/routing/service/DefaultRoutingService.java | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) rename src/main/java/org/opentripplanner/{framework/application => api/support}/ZoneIdFallback.java (96%) diff --git a/src/main/java/org/opentripplanner/api/common/RoutingResource.java b/src/main/java/org/opentripplanner/api/common/RoutingResource.java index a97a8368187..dab06e970ce 100644 --- a/src/main/java/org/opentripplanner/api/common/RoutingResource.java +++ b/src/main/java/org/opentripplanner/api/common/RoutingResource.java @@ -19,9 +19,9 @@ import javax.xml.datatype.DatatypeFactory; import javax.xml.datatype.XMLGregorianCalendar; import org.opentripplanner.api.parameter.QualifiedModeSet; +import org.opentripplanner.api.support.ZoneIdFallback; import org.opentripplanner.ext.dataoverlay.api.DataOverlayParameters; import org.opentripplanner.framework.application.OTPFeature; -import org.opentripplanner.framework.application.ZoneIdFallback; import org.opentripplanner.framework.lang.StringUtils; import org.opentripplanner.framework.time.DurationUtils; import org.opentripplanner.routing.api.request.RouteRequest; diff --git a/src/main/java/org/opentripplanner/framework/application/ZoneIdFallback.java b/src/main/java/org/opentripplanner/api/support/ZoneIdFallback.java similarity index 96% rename from src/main/java/org/opentripplanner/framework/application/ZoneIdFallback.java rename to src/main/java/org/opentripplanner/api/support/ZoneIdFallback.java index 260fb40f0bf..0175337e2d4 100644 --- a/src/main/java/org/opentripplanner/framework/application/ZoneIdFallback.java +++ b/src/main/java/org/opentripplanner/api/support/ZoneIdFallback.java @@ -1,4 +1,4 @@ -package org.opentripplanner.framework.application; +package org.opentripplanner.api.support; import java.time.ZoneId; import javax.annotation.Nullable; diff --git a/src/main/java/org/opentripplanner/apis/gtfs/mapping/RouteRequestMapper.java b/src/main/java/org/opentripplanner/apis/gtfs/mapping/RouteRequestMapper.java index 7b47f3231c9..306abf8945f 100644 --- a/src/main/java/org/opentripplanner/apis/gtfs/mapping/RouteRequestMapper.java +++ b/src/main/java/org/opentripplanner/apis/gtfs/mapping/RouteRequestMapper.java @@ -15,8 +15,8 @@ import org.opentripplanner.api.common.LocationStringParser; import org.opentripplanner.api.parameter.QualifiedMode; import org.opentripplanner.api.parameter.QualifiedModeSet; +import org.opentripplanner.api.support.ZoneIdFallback; import org.opentripplanner.apis.gtfs.GraphQLRequestContext; -import org.opentripplanner.framework.application.ZoneIdFallback; import org.opentripplanner.framework.graphql.GraphQLUtils; import org.opentripplanner.model.GenericLocation; import org.opentripplanner.routing.api.request.RouteRequest; diff --git a/src/main/java/org/opentripplanner/routing/algorithm/mapping/GraphPathToItineraryMapper.java b/src/main/java/org/opentripplanner/routing/algorithm/mapping/GraphPathToItineraryMapper.java index c363689d03d..ee86f8de720 100644 --- a/src/main/java/org/opentripplanner/routing/algorithm/mapping/GraphPathToItineraryMapper.java +++ b/src/main/java/org/opentripplanner/routing/algorithm/mapping/GraphPathToItineraryMapper.java @@ -12,11 +12,11 @@ import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.LineString; import org.locationtech.jts.geom.impl.PackedCoordinateSequence; +import org.opentripplanner.api.support.ZoneIdFallback; import org.opentripplanner.astar.model.GraphPath; import org.opentripplanner.ext.flex.FlexibleTransitLeg; import org.opentripplanner.ext.flex.edgetype.FlexTripEdge; import org.opentripplanner.framework.application.OTPFeature; -import org.opentripplanner.framework.application.ZoneIdFallback; import org.opentripplanner.framework.geometry.GeometryUtils; import org.opentripplanner.framework.i18n.I18NString; import org.opentripplanner.model.plan.ElevationProfile; diff --git a/src/main/java/org/opentripplanner/routing/service/DefaultRoutingService.java b/src/main/java/org/opentripplanner/routing/service/DefaultRoutingService.java index a9a887588dd..92ccd2cc786 100644 --- a/src/main/java/org/opentripplanner/routing/service/DefaultRoutingService.java +++ b/src/main/java/org/opentripplanner/routing/service/DefaultRoutingService.java @@ -1,8 +1,8 @@ package org.opentripplanner.routing.service; import java.time.ZoneId; +import org.opentripplanner.api.support.ZoneIdFallback; import org.opentripplanner.framework.application.OTPRequestTimeoutException; -import org.opentripplanner.framework.application.ZoneIdFallback; import org.opentripplanner.framework.tostring.MultiLineToStringBuilder; import org.opentripplanner.model.plan.Itinerary; import org.opentripplanner.routing.algorithm.RoutingWorker; From 8c8967a5a1ac66aa57123cfc92fa513a2a8b6fee Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Thu, 21 Dec 2023 12:24:28 +0100 Subject: [PATCH 3/5] Add test --- .../api/support/ZoneIdFallbackTest.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 src/test/java/org/opentripplanner/api/support/ZoneIdFallbackTest.java diff --git a/src/test/java/org/opentripplanner/api/support/ZoneIdFallbackTest.java b/src/test/java/org/opentripplanner/api/support/ZoneIdFallbackTest.java new file mode 100644 index 00000000000..02013f0856b --- /dev/null +++ b/src/test/java/org/opentripplanner/api/support/ZoneIdFallbackTest.java @@ -0,0 +1,19 @@ +package org.opentripplanner.api.support; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.junit.jupiter.api.Test; +import org.opentripplanner._support.time.ZoneIds; + +class ZoneIdFallbackTest { + + @Test + void fallback() { + assertEquals(ZoneIds.UTC, ZoneIdFallback.zoneId(null)); + } + + @Test + void keepOriginal() { + assertEquals(ZoneIds.BERLIN, ZoneIdFallback.zoneId(ZoneIds.BERLIN)); + } +} From 9e806f7957f7c487d6647b80fd11166cbdbff80f Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Tue, 16 Jan 2024 14:41:09 +0100 Subject: [PATCH 4/5] Move fallback class into framework.time package --- .../opentripplanner/ext/restapi/resources/RoutingResource.java | 2 +- .../opentripplanner/apis/gtfs/mapping/RouteRequestMapper.java | 2 +- .../{api/support => framework/time}/ZoneIdFallback.java | 2 +- .../routing/algorithm/mapping/GraphPathToItineraryMapper.java | 2 +- .../opentripplanner/routing/service/DefaultRoutingService.java | 2 +- .../{api/support => framework/time}/ZoneIdFallbackTest.java | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) rename src/main/java/org/opentripplanner/{api/support => framework/time}/ZoneIdFallback.java (96%) rename src/test/java/org/opentripplanner/{api/support => framework/time}/ZoneIdFallbackTest.java (89%) diff --git a/src/ext/java/org/opentripplanner/ext/restapi/resources/RoutingResource.java b/src/ext/java/org/opentripplanner/ext/restapi/resources/RoutingResource.java index ae63a95feaa..bfb3d6ed15a 100644 --- a/src/ext/java/org/opentripplanner/ext/restapi/resources/RoutingResource.java +++ b/src/ext/java/org/opentripplanner/ext/restapi/resources/RoutingResource.java @@ -19,11 +19,11 @@ import javax.xml.datatype.DatatypeFactory; import javax.xml.datatype.XMLGregorianCalendar; import org.opentripplanner.api.parameter.QualifiedModeSet; -import org.opentripplanner.api.support.ZoneIdFallback; import org.opentripplanner.ext.dataoverlay.api.DataOverlayParameters; import org.opentripplanner.framework.application.OTPFeature; import org.opentripplanner.framework.lang.StringUtils; import org.opentripplanner.framework.time.DurationUtils; +import org.opentripplanner.framework.time.ZoneIdFallback; import org.opentripplanner.routing.api.request.RouteRequest; import org.opentripplanner.routing.api.request.preference.ItineraryFilterDebugProfile; import org.opentripplanner.routing.api.request.request.filter.SelectRequest; diff --git a/src/main/java/org/opentripplanner/apis/gtfs/mapping/RouteRequestMapper.java b/src/main/java/org/opentripplanner/apis/gtfs/mapping/RouteRequestMapper.java index e4094aaa888..2eeaf229b11 100644 --- a/src/main/java/org/opentripplanner/apis/gtfs/mapping/RouteRequestMapper.java +++ b/src/main/java/org/opentripplanner/apis/gtfs/mapping/RouteRequestMapper.java @@ -15,9 +15,9 @@ import org.opentripplanner.api.common.LocationStringParser; import org.opentripplanner.api.parameter.QualifiedMode; import org.opentripplanner.api.parameter.QualifiedModeSet; -import org.opentripplanner.api.support.ZoneIdFallback; import org.opentripplanner.apis.gtfs.GraphQLRequestContext; import org.opentripplanner.framework.graphql.GraphQLUtils; +import org.opentripplanner.framework.time.ZoneIdFallback; import org.opentripplanner.model.GenericLocation; import org.opentripplanner.routing.api.request.RouteRequest; import org.opentripplanner.routing.api.request.framework.CostLinearFunction; diff --git a/src/main/java/org/opentripplanner/api/support/ZoneIdFallback.java b/src/main/java/org/opentripplanner/framework/time/ZoneIdFallback.java similarity index 96% rename from src/main/java/org/opentripplanner/api/support/ZoneIdFallback.java rename to src/main/java/org/opentripplanner/framework/time/ZoneIdFallback.java index 0175337e2d4..2d43216e4b2 100644 --- a/src/main/java/org/opentripplanner/api/support/ZoneIdFallback.java +++ b/src/main/java/org/opentripplanner/framework/time/ZoneIdFallback.java @@ -1,4 +1,4 @@ -package org.opentripplanner.api.support; +package org.opentripplanner.framework.time; import java.time.ZoneId; import javax.annotation.Nullable; diff --git a/src/main/java/org/opentripplanner/routing/algorithm/mapping/GraphPathToItineraryMapper.java b/src/main/java/org/opentripplanner/routing/algorithm/mapping/GraphPathToItineraryMapper.java index ee86f8de720..11302098f25 100644 --- a/src/main/java/org/opentripplanner/routing/algorithm/mapping/GraphPathToItineraryMapper.java +++ b/src/main/java/org/opentripplanner/routing/algorithm/mapping/GraphPathToItineraryMapper.java @@ -12,13 +12,13 @@ import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.LineString; import org.locationtech.jts.geom.impl.PackedCoordinateSequence; -import org.opentripplanner.api.support.ZoneIdFallback; import org.opentripplanner.astar.model.GraphPath; import org.opentripplanner.ext.flex.FlexibleTransitLeg; import org.opentripplanner.ext.flex.edgetype.FlexTripEdge; import org.opentripplanner.framework.application.OTPFeature; import org.opentripplanner.framework.geometry.GeometryUtils; import org.opentripplanner.framework.i18n.I18NString; +import org.opentripplanner.framework.time.ZoneIdFallback; import org.opentripplanner.model.plan.ElevationProfile; import org.opentripplanner.model.plan.Itinerary; import org.opentripplanner.model.plan.Leg; diff --git a/src/main/java/org/opentripplanner/routing/service/DefaultRoutingService.java b/src/main/java/org/opentripplanner/routing/service/DefaultRoutingService.java index dd5d5615be8..01736aac80e 100644 --- a/src/main/java/org/opentripplanner/routing/service/DefaultRoutingService.java +++ b/src/main/java/org/opentripplanner/routing/service/DefaultRoutingService.java @@ -1,8 +1,8 @@ package org.opentripplanner.routing.service; import java.time.ZoneId; -import org.opentripplanner.api.support.ZoneIdFallback; import org.opentripplanner.framework.application.OTPRequestTimeoutException; +import org.opentripplanner.framework.time.ZoneIdFallback; import org.opentripplanner.framework.tostring.MultiLineToStringBuilder; import org.opentripplanner.model.plan.Itinerary; import org.opentripplanner.routing.algorithm.RoutingWorker; diff --git a/src/test/java/org/opentripplanner/api/support/ZoneIdFallbackTest.java b/src/test/java/org/opentripplanner/framework/time/ZoneIdFallbackTest.java similarity index 89% rename from src/test/java/org/opentripplanner/api/support/ZoneIdFallbackTest.java rename to src/test/java/org/opentripplanner/framework/time/ZoneIdFallbackTest.java index 02013f0856b..b69e3ee2ff1 100644 --- a/src/test/java/org/opentripplanner/api/support/ZoneIdFallbackTest.java +++ b/src/test/java/org/opentripplanner/framework/time/ZoneIdFallbackTest.java @@ -1,4 +1,4 @@ -package org.opentripplanner.api.support; +package org.opentripplanner.framework.time; import static org.junit.jupiter.api.Assertions.assertEquals; From 83dd2282948c083ddd64653b617ed3fd0d8cb14b Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Tue, 16 Jan 2024 14:46:55 +0100 Subject: [PATCH 5/5] Allow dependency on logging package --- .../opentripplanner/framework/FrameworkArchitectureTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/opentripplanner/framework/FrameworkArchitectureTest.java b/src/test/java/org/opentripplanner/framework/FrameworkArchitectureTest.java index e1e00076a73..9e44dd05dda 100644 --- a/src/test/java/org/opentripplanner/framework/FrameworkArchitectureTest.java +++ b/src/test/java/org/opentripplanner/framework/FrameworkArchitectureTest.java @@ -97,7 +97,7 @@ void enforceTextPackageDependencies() { @Test void enforceTimePackageDependencies() { - TIME.verify(); + TIME.dependsOn(LOGGING).verify(); } @Test