From 7ad81394c0d7e76ca4789fc83ba66b3da34fd2aa Mon Sep 17 00:00:00 2001 From: Vincent Paturet Date: Tue, 15 Aug 2023 18:09:00 +0200 Subject: [PATCH 01/53] Limit the search window to 24 hours --- src/ext/graphql/transmodelapi/schema.graphql | 2 +- .../ext/transmodelapi/model/plan/TripQuery.java | 3 ++- .../org/opentripplanner/routing/api/request/RouteRequest.java | 4 ++++ 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/ext/graphql/transmodelapi/schema.graphql b/src/ext/graphql/transmodelapi/schema.graphql index 694f49b2cd0..f1d53ff64b7 100644 --- a/src/ext/graphql/transmodelapi/schema.graphql +++ b/src/ext/graphql/transmodelapi/schema.graphql @@ -830,7 +830,7 @@ type QueryType { """ relaxTransitSearchGeneralizedCostAtDestination: Float = null, """ - The length of the search-window in minutes. This parameter is optional. + The length of the search-window in minutes. This parameter is optional. The maximum value is 1440 minutes (24 hours). The search-window is defined as the duration between the earliest-departure-time(EDT) and the latest-departure-time(LDT). OTP will search for all itineraries in this departure window. If `arriveBy=true` the `dateTime` parameter is the latest-arrival-time, so OTP will dynamically calculate the EDT. Using a short search-window is faster than using a longer one, but the search duration is not linear. Using a "too" short search-window will waste resources server side, while using a search-window that is too long will be slow. diff --git a/src/ext/java/org/opentripplanner/ext/transmodelapi/model/plan/TripQuery.java b/src/ext/java/org/opentripplanner/ext/transmodelapi/model/plan/TripQuery.java index 0f67b8ce406..c695795ab7c 100644 --- a/src/ext/java/org/opentripplanner/ext/transmodelapi/model/plan/TripQuery.java +++ b/src/ext/java/org/opentripplanner/ext/transmodelapi/model/plan/TripQuery.java @@ -58,7 +58,8 @@ public static GraphQLFieldDefinition create( .newArgument() .name("searchWindow") .description( - "The length of the search-window in minutes. This parameter is optional." + + "The length of the search-window in minutes. This parameter is optional. " + + "The maximum value is 1440 minutes (24 hours)." + "\n\n" + "The search-window is defined as the duration between the earliest-departure-time(EDT) and " + "the latest-departure-time(LDT). OTP will search for all itineraries in this departure " + diff --git a/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java b/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java index 357afbe9ff8..ec858a36209 100644 --- a/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java +++ b/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java @@ -12,6 +12,7 @@ import java.util.Locale; import java.util.function.Consumer; import org.opentripplanner.framework.time.DateUtils; +import org.opentripplanner.framework.time.TimeUtils; import org.opentripplanner.model.GenericLocation; import org.opentripplanner.model.plan.SortOrder; import org.opentripplanner.model.plan.pagecursor.PageCursor; @@ -319,6 +320,9 @@ public Duration searchWindow() { } public void setSearchWindow(Duration searchWindow) { + if (searchWindow != null && searchWindow.toSeconds() > TimeUtils.ONE_DAY_SECONDS) { + throw new IllegalArgumentException("The search window cannot exceed 24 hours"); + } this.searchWindow = searchWindow; } From 11e952f708dc7b801925f580a9ebfcbd98ca3a91 Mon Sep 17 00:00:00 2001 From: Vincent Paturet Date: Mon, 21 Aug 2023 12:33:54 +0200 Subject: [PATCH 02/53] Validate negative search window --- .../routing/api/request/RouteRequest.java | 3 ++ .../routing/core/RouteRequestTest.java | 39 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java b/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java index ec858a36209..03b08ad36c8 100644 --- a/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java +++ b/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java @@ -323,6 +323,9 @@ public void setSearchWindow(Duration searchWindow) { if (searchWindow != null && searchWindow.toSeconds() > TimeUtils.ONE_DAY_SECONDS) { throw new IllegalArgumentException("The search window cannot exceed 24 hours"); } + if (searchWindow != null && searchWindow.isNegative()) { + throw new IllegalArgumentException("The search window must be a positive duration"); + } this.searchWindow = searchWindow; } diff --git a/src/test/java/org/opentripplanner/routing/core/RouteRequestTest.java b/src/test/java/org/opentripplanner/routing/core/RouteRequestTest.java index a5c532cd323..1f3f7bc06d5 100644 --- a/src/test/java/org/opentripplanner/routing/core/RouteRequestTest.java +++ b/src/test/java/org/opentripplanner/routing/core/RouteRequestTest.java @@ -2,9 +2,11 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; +import java.time.Duration; import java.util.List; import org.junit.jupiter.api.Test; import org.opentripplanner.model.GenericLocation; @@ -16,6 +18,13 @@ class RouteRequestTest { + private static final Duration DURATION_24_HOURS_AND_ONE_MINUTE = Duration + .ofHours(24) + .plusMinutes(1); + private static final Duration DURATION_ZERO = Duration.ofMinutes(0); + private static final Duration DURATION_ONE_MINUTE = Duration.ofMinutes(1); + private static final Duration DURATION_MINUS_ONE_MINUTE = DURATION_ONE_MINUTE.negated(); + @Test public void testRequest() { // TODO VIA: looks like some parts of this test are obsolete since method no longer exist @@ -110,6 +119,36 @@ void testValidateFromAndTo() { request.validateOriginAndDestination(); } + @Test + void testValidSearchWindow() { + RouteRequest request = new RouteRequest(); + request.setSearchWindow(DURATION_ONE_MINUTE); + } + + @Test + void testZeroSearchWindow() { + RouteRequest request = new RouteRequest(); + request.setSearchWindow(DURATION_ZERO); + } + + @Test + void testTooLongSearchWindow() { + RouteRequest request = new RouteRequest(); + assertThrows( + IllegalArgumentException.class, + () -> request.setSearchWindow(DURATION_24_HOURS_AND_ONE_MINUTE) + ); + } + + @Test + void testNegativeSearchWindow() { + RouteRequest request = new RouteRequest(); + assertThrows( + IllegalArgumentException.class, + () -> request.setSearchWindow(DURATION_MINUS_ONE_MINUTE) + ); + } + private GenericLocation randomLocation() { return new GenericLocation(Math.random(), Math.random()); } From fced98decfa065a1a282d642d7b0fa534db54113 Mon Sep 17 00:00:00 2001 From: Vincent Paturet Date: Mon, 21 Aug 2023 16:18:30 +0200 Subject: [PATCH 03/53] Applied review suggestions --- .../routing/api/request/RouteRequest.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java b/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java index 03b08ad36c8..f7beda526ac 100644 --- a/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java +++ b/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java @@ -320,11 +320,13 @@ public Duration searchWindow() { } public void setSearchWindow(Duration searchWindow) { - if (searchWindow != null && searchWindow.toSeconds() > TimeUtils.ONE_DAY_SECONDS) { - throw new IllegalArgumentException("The search window cannot exceed 24 hours"); - } - if (searchWindow != null && searchWindow.isNegative()) { - throw new IllegalArgumentException("The search window must be a positive duration"); + if (searchWindow != null) { + if (searchWindow.toSeconds() > TimeUtils.ONE_DAY_SECONDS) { + throw new IllegalArgumentException("The search window cannot exceed 24 hours"); + } + if (searchWindow.isNegative()) { + throw new IllegalArgumentException("The search window must be a positive duration"); + } } this.searchWindow = searchWindow; } From 27759b036d95a8f44fcca286160bc9b4f602e211 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Fri, 25 Aug 2023 00:22:00 +0200 Subject: [PATCH 04/53] doc: Add configuration doc to the Vehicle Rental Service Directory --- .../sandbox/VehicleRentalServiceDirectory.md | 38 +++++++++ docs/RouterConfiguration.md | 14 ++-- docs/sandbox/VehicleRentalServiceDirectory.md | 77 ++++++++++++++++--- ...leRentalServiceDirectoryFetcherConfig.java | 14 ++-- ...leRentalServiceDirectoryConfigDocTest.java | 68 ++++++++++++++++ ...nfig-vehicle-rental-service-directory.json | 11 +++ 6 files changed, 197 insertions(+), 25 deletions(-) create mode 100644 doc-templates/sandbox/VehicleRentalServiceDirectory.md create mode 100644 src/test/java/org/opentripplanner/generate/doc/sandbox/VehicleRentalServiceDirectoryConfigDocTest.java create mode 100644 src/test/resources/standalone/config/sandbox/build-config-vehicle-rental-service-directory.json diff --git a/doc-templates/sandbox/VehicleRentalServiceDirectory.md b/doc-templates/sandbox/VehicleRentalServiceDirectory.md new file mode 100644 index 00000000000..646670150bd --- /dev/null +++ b/doc-templates/sandbox/VehicleRentalServiceDirectory.md @@ -0,0 +1,38 @@ +# Vehicle Rental Service Directory API support. + +This adds support for the GBFS service directory endpoint component located +at https://github.com/entur/lahmu. OTP use the service directory to lookup and connect to all GBFS +endpoints registered in the directory. This simplify the management of the GBFS endpoints, since +multiple services/components like OTP can connect to the directory and get the necessary +configuration from it. + + +## Contact Info + +- Entur, Norway + + +## Changelog + +- Initial implementation of bike share updater API support +- Make json tag names configurable [#3447](https://github.com/opentripplanner/OpenTripPlanner/pull/3447) + + +## Configuration + +To enable this you need to specify a url for the `vehicleRentalServiceDirectory` in +the `router-config.json` + +### Parameter Summary + + + + +### Parameter Details + + + + +### Example + + diff --git a/docs/RouterConfiguration.md b/docs/RouterConfiguration.md index 1d7f5443fd4..fc3d196b46d 100644 --- a/docs/RouterConfiguration.md +++ b/docs/RouterConfiguration.md @@ -68,12 +68,12 @@ A full list of them can be found in the [RouteRequest](RouteRequest.md). | [updaters](UpdaterConfig.md) | `object[]` | Configuration for the updaters that import various types of data into OTP. | *Optional* | | 1.5 | | [vectorTileLayers](sandbox/MapboxVectorTilesApi.md) | `object[]` | Configuration of the individual layers for the Mapbox vector tiles. | *Optional* | | 2.0 | | vehicleRentalServiceDirectory | `object` | Configuration for the vehicle rental service directory. | *Optional* | | 2.0 | -|    language | `string` | Language code. | *Optional* | | na | -|    sourcesName | `string` | Json tag name for updater sources. | *Optional* | `"systems"` | na | -|    updaterNetworkName | `string` | Json tag name for the network name for each source. | *Optional* | `"id"` | na | -|    updaterUrlName | `string` | Json tag name for endpoint urls for each source. | *Optional* | `"url"` | na | -|    url | `uri` | Endpoint for the VehicleRentalServiceDirectory | *Required* | | na | -|    [headers](#vehicleRentalServiceDirectory_headers) | `map of string` | HTTP headers to add to the request. Any header key, value can be inserted. | *Optional* | | na | +|    language | `string` | Language code. | *Optional* | | 2.1 | +|    sourcesName | `string` | Json tag name for updater sources. | *Optional* | `"systems"` | 2.1 | +|    updaterNetworkName | `string` | Json tag name for the network name for each source. | *Optional* | `"id"` | 2.1 | +|    updaterUrlName | `string` | Json tag name for endpoint urls for each source. | *Optional* | `"url"` | 2.1 | +|    url | `uri` | Endpoint for the VehicleRentalServiceDirectory | *Required* | | 2.1 | +|    [headers](#vehicleRentalServiceDirectory_headers) | `map of string` | HTTP headers to add to the request. Any header key, value can be inserted. | *Optional* | | 2.1 | @@ -417,7 +417,7 @@ Used to group requests when monitoring OTP.

headers

-**Since version:** `na` ∙ **Type:** `map of string` ∙ **Cardinality:** `Optional` +**Since version:** `2.1` ∙ **Type:** `map of string` ∙ **Cardinality:** `Optional` **Path:** /vehicleRentalServiceDirectory HTTP headers to add to the request. Any header key, value can be inserted. diff --git a/docs/sandbox/VehicleRentalServiceDirectory.md b/docs/sandbox/VehicleRentalServiceDirectory.md index 43a9727d251..f468a17f655 100644 --- a/docs/sandbox/VehicleRentalServiceDirectory.md +++ b/docs/sandbox/VehicleRentalServiceDirectory.md @@ -1,24 +1,79 @@ # Vehicle Rental Service Directory API support. +This adds support for the GBFS service directory endpoint component located +at https://github.com/entur/lahmu. OTP use the service directory to lookup and connect to all GBFS +endpoints registered in the directory. This simplify the management of the GBFS endpoints, since +multiple services/components like OTP can connect to the directory and get the necessary +configuration from it. + + ## Contact Info -- Gard Mellemstrand, Entur, Norway +- Entur, Norway + ## Changelog - Initial implementation of bike share updater API support -- Make json tag names - configurable [#3447](https://github.com/opentripplanner/OpenTripPlanner/pull/3447) +- Make json tag names configurable [#3447](https://github.com/opentripplanner/OpenTripPlanner/pull/3447) -## Documentation -This adds support for the GBFS service directory endpoint component located -at https://github.com/entur/lahmu. OTP use the service directory to lookup and connect to all GBFS -endpoints registered in the directory. This simplify the management of the GBFS endpoints, since -multiple services/components like OTP can connect to the directory and get the necessary -configuration from it. - -### Configuration +## Configuration To enable this you need to specify a url for the `vehicleRentalServiceDirectory` in the `router-config.json` + +### Parameter Summary + + + + +| Config Parameter | Type | Summary | Req./Opt. | Default Value | Since | +|---------------------------------------------------|:---------------:|----------------------------------------------------------------------------|:----------:|---------------|:-----:| +| language | `string` | Language code. | *Optional* | | 2.1 | +| sourcesName | `string` | Json tag name for updater sources. | *Optional* | `"systems"` | 2.1 | +| updaterNetworkName | `string` | Json tag name for the network name for each source. | *Optional* | `"id"` | 2.1 | +| updaterUrlName | `string` | Json tag name for endpoint urls for each source. | *Optional* | `"url"` | 2.1 | +| url | `uri` | Endpoint for the VehicleRentalServiceDirectory | *Required* | | 2.1 | +| [headers](#vehicleRentalServiceDirectory_headers) | `map of string` | HTTP headers to add to the request. Any header key, value can be inserted. | *Optional* | | 2.1 | + + + + +### Parameter Details + + + + +

headers

+ +**Since version:** `2.1` ∙ **Type:** `map of string` ∙ **Cardinality:** `Optional` +**Path:** /vehicleRentalServiceDirectory + +HTTP headers to add to the request. Any header key, value can be inserted. + + + + + +### Example + + + + +```JSON +// router-config.json +{ + "vehicleRentalServiceDirectory" : { + "url" : "https://example.com", + "sourcesName" : "systems", + "updaterUrlName" : "url", + "updaterNetworkName" : "id", + "headers" : { + "ET-Client-Name" : "otp" + } + } +} +``` + + 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..a96442fc869 100644 --- a/src/main/java/org/opentripplanner/standalone/config/sandbox/VehicleRentalServiceDirectoryFetcherConfig.java +++ b/src/main/java/org/opentripplanner/standalone/config/sandbox/VehicleRentalServiceDirectoryFetcherConfig.java @@ -1,7 +1,7 @@ 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 org.opentripplanner.ext.vehiclerentalservicedirectory.api.VehicleRentalServiceDirectoryFetcherParameters; import org.opentripplanner.standalone.config.framework.json.NodeAdapter; @@ -24,24 +24,24 @@ 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) ); } } diff --git a/src/test/java/org/opentripplanner/generate/doc/sandbox/VehicleRentalServiceDirectoryConfigDocTest.java b/src/test/java/org/opentripplanner/generate/doc/sandbox/VehicleRentalServiceDirectoryConfigDocTest.java new file mode 100644 index 00000000000..a64ef03dcd6 --- /dev/null +++ b/src/test/java/org/opentripplanner/generate/doc/sandbox/VehicleRentalServiceDirectoryConfigDocTest.java @@ -0,0 +1,68 @@ +package org.opentripplanner.generate.doc.sandbox; + +import static org.opentripplanner.framework.application.OtpFileNames.ROUTER_CONFIG_FILENAME; +import static org.opentripplanner.framework.io.FileUtils.assertFileEquals; +import static org.opentripplanner.framework.io.FileUtils.readFile; +import static org.opentripplanner.framework.io.FileUtils.writeFile; +import static org.opentripplanner.framework.text.MarkdownFormatter.HEADER_4; +import static org.opentripplanner.generate.doc.framework.DocsTestConstants.DOCS_ROOT; +import static org.opentripplanner.generate.doc.framework.DocsTestConstants.TEMPLATE_ROOT; +import static org.opentripplanner.generate.doc.framework.TemplateUtil.replaceJsonExample; +import static org.opentripplanner.generate.doc.framework.TemplateUtil.replaceParametersDetails; +import static org.opentripplanner.generate.doc.framework.TemplateUtil.replaceParametersTable; +import static org.opentripplanner.standalone.config.framework.json.JsonSupport.jsonNodeFromResource; + +import java.io.File; +import org.junit.jupiter.api.Test; +import org.opentripplanner.generate.doc.framework.GeneratesDocumentation; +import org.opentripplanner.generate.doc.framework.ParameterDetailsList; +import org.opentripplanner.generate.doc.framework.ParameterSummaryTable; +import org.opentripplanner.generate.doc.framework.SkipNodes; +import org.opentripplanner.generate.doc.framework.TemplateUtil; +import org.opentripplanner.standalone.config.RouterConfig; +import org.opentripplanner.standalone.config.framework.json.NodeAdapter; + +@GeneratesDocumentation +public class VehicleRentalServiceDirectoryConfigDocTest { + + private static final String DOCUMENT = "sandbox/VehicleRentalServiceDirectory.md"; + private static final File TEMPLATE = new File(TEMPLATE_ROOT, DOCUMENT); + private static final File OUT_FILE = new File(DOCS_ROOT, DOCUMENT); + private static final String CONFIG_PATH = + "standalone/config/sandbox/build-config-vehicle-rental-service-directory.json"; + private static final String CONFIG_TAG = "vehicleRentalServiceDirectory"; + private static final SkipNodes SKIP_NODES = SkipNodes.of().build(); + + @Test + public void updateConfigurationDoc() { + NodeAdapter node = readConfigDefaults(); + + // Read and close inout file (same as output file) + String doc = readFile(TEMPLATE); + String original = readFile(OUT_FILE); + + doc = replaceParametersTable(doc, getParameterSummaryTable(node)); + doc = replaceParametersDetails(doc, getParameterDetailsList(node)); + + var example = TemplateUtil.jsonExampleBuilder(node.rawNode()).wrapInObject(CONFIG_TAG).build(); + doc = replaceJsonExample(doc, example, ROUTER_CONFIG_FILENAME); + + writeFile(OUT_FILE, doc); + + assertFileEquals(original, OUT_FILE); + } + + private NodeAdapter readConfigDefaults() { + var json = jsonNodeFromResource(CONFIG_PATH); + var conf = new RouterConfig(json, CONFIG_PATH, false); + return conf.asNodeAdapter().child(CONFIG_TAG); + } + + private String getParameterSummaryTable(NodeAdapter node) { + return new ParameterSummaryTable(SKIP_NODES).createTable(node).toMarkdownTable(); + } + + private String getParameterDetailsList(NodeAdapter node) { + return ParameterDetailsList.listParametersWithDetails(node, SKIP_NODES, HEADER_4); + } +} diff --git a/src/test/resources/standalone/config/sandbox/build-config-vehicle-rental-service-directory.json b/src/test/resources/standalone/config/sandbox/build-config-vehicle-rental-service-directory.json new file mode 100644 index 00000000000..b4896072950 --- /dev/null +++ b/src/test/resources/standalone/config/sandbox/build-config-vehicle-rental-service-directory.json @@ -0,0 +1,11 @@ +{ + "vehicleRentalServiceDirectory": { + "url": "https://example.com", + "sourcesName": "systems", + "updaterUrlName": "url", + "updaterNetworkName": "id", + "headers": { + "ET-Client-Name": "otp" + } + } +} From 3a26539095f0df2c494a91e1ea79adfeec6ff273 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Fri, 25 Aug 2023 01:06:54 +0200 Subject: [PATCH 05/53] feature: Add geofencingZones to VehicleRentalServiceDirectory --- docs/RouterConfiguration.md | 15 +- docs/sandbox/VehicleRentalServiceDirectory.md | 34 +++-- ...leRentalServiceDirectoryConfigDocTest.java | 4 +- .../generatedoc/router-config.json} | 8 +- .../VehicleRentalServiceDirectoryFetcher.java | 129 +++++++++++------- .../api/NetworkParameters.java | 15 ++ ...ntalServiceDirectoryFetcherParameters.java | 14 +- .../framework/json/JsonUtils.java | 17 +++ ...leRentalServiceDirectoryFetcherConfig.java | 26 +++- .../framework/json/JsonUtilsTest.java | 35 +++++ .../doc/RouterConfigurationDocTest.java | 1 + 11 files changed, 218 insertions(+), 80 deletions(-) rename src/{test/java/org/opentripplanner/generate/doc/sandbox => ext-test/java/org/opentripplanner/ext/vehiclerentalservicedirectory/generatedoc}/VehicleRentalServiceDirectoryConfigDocTest.java (94%) rename src/{test/resources/standalone/config/sandbox/build-config-vehicle-rental-service-directory.json => ext-test/resources/org/opentripplanner/ext/vehiclerentalservicedirectory/generatedoc/router-config.json} (64%) create mode 100644 src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/NetworkParameters.java create mode 100644 src/main/java/org/opentripplanner/framework/json/JsonUtils.java create mode 100644 src/test/java/org/opentripplanner/framework/json/JsonUtilsTest.java diff --git a/docs/RouterConfiguration.md b/docs/RouterConfiguration.md index fc3d196b46d..af0e5daa817 100644 --- a/docs/RouterConfiguration.md +++ b/docs/RouterConfiguration.md @@ -67,13 +67,7 @@ A full list of them can be found in the [RouteRequest](RouteRequest.md). |    [tracingHeaderTags](#transmodelApi_tracingHeaderTags) | `string[]` | Used to group requests when monitoring OTP. | *Optional* | | na | | [updaters](UpdaterConfig.md) | `object[]` | Configuration for the updaters that import various types of data into OTP. | *Optional* | | 1.5 | | [vectorTileLayers](sandbox/MapboxVectorTilesApi.md) | `object[]` | Configuration of the individual layers for the Mapbox vector tiles. | *Optional* | | 2.0 | -| vehicleRentalServiceDirectory | `object` | Configuration for the vehicle rental service directory. | *Optional* | | 2.0 | -|    language | `string` | Language code. | *Optional* | | 2.1 | -|    sourcesName | `string` | Json tag name for updater sources. | *Optional* | `"systems"` | 2.1 | -|    updaterNetworkName | `string` | Json tag name for the network name for each source. | *Optional* | `"id"` | 2.1 | -|    updaterUrlName | `string` | Json tag name for endpoint urls for each source. | *Optional* | `"url"` | 2.1 | -|    url | `uri` | Endpoint for the VehicleRentalServiceDirectory | *Required* | | 2.1 | -|    [headers](#vehicleRentalServiceDirectory_headers) | `map of string` | HTTP headers to add to the request. Any header key, value can be inserted. | *Optional* | | 2.1 | +| [vehicleRentalServiceDirectory](sandbox/VehicleRentalServiceDirectory.md) | `object` | Configuration for the vehicle rental service directory. | *Optional* | | 2.0 | @@ -415,13 +409,6 @@ Only turn this feature on if you have unique ids across all feeds, without the f Used to group requests when monitoring OTP. -

headers

- -**Since version:** `2.1` ∙ **Type:** `map of string` ∙ **Cardinality:** `Optional` -**Path:** /vehicleRentalServiceDirectory - -HTTP headers to add to the request. Any header key, value can be inserted. - diff --git a/docs/sandbox/VehicleRentalServiceDirectory.md b/docs/sandbox/VehicleRentalServiceDirectory.md index f468a17f655..2952a743ea3 100644 --- a/docs/sandbox/VehicleRentalServiceDirectory.md +++ b/docs/sandbox/VehicleRentalServiceDirectory.md @@ -28,14 +28,17 @@ the `router-config.json` -| Config Parameter | Type | Summary | Req./Opt. | Default Value | Since | -|---------------------------------------------------|:---------------:|----------------------------------------------------------------------------|:----------:|---------------|:-----:| -| language | `string` | Language code. | *Optional* | | 2.1 | -| sourcesName | `string` | Json tag name for updater sources. | *Optional* | `"systems"` | 2.1 | -| updaterNetworkName | `string` | Json tag name for the network name for each source. | *Optional* | `"id"` | 2.1 | -| updaterUrlName | `string` | Json tag name for endpoint urls for each source. | *Optional* | `"url"` | 2.1 | -| url | `uri` | Endpoint for the VehicleRentalServiceDirectory | *Required* | | 2.1 | -| [headers](#vehicleRentalServiceDirectory_headers) | `map of string` | HTTP headers to add to the request. Any header key, value can be inserted. | *Optional* | | 2.1 | +| Config Parameter | Type | Summary | Req./Opt. | Default Value | Since | +|-----------------------------------------------------|:---------------:|----------------------------------------------------------------------------|:----------:|---------------|:-----:| +| language | `string` | Language code. | *Optional* | | 2.1 | +| sourcesName | `string` | Json tag name for updater sources. | *Optional* | `"systems"` | 2.1 | +| updaterNetworkName | `string` | Json tag name for the network name for each source. | *Optional* | `"id"` | 2.1 | +| updaterUrlName | `string` | Json tag name for endpoint urls for each source. | *Optional* | `"url"` | 2.1 | +| url | `uri` | Endpoint for the VehicleRentalServiceDirectory | *Required* | | 2.1 | +| [headers](#vehicleRentalServiceDirectory_headers) | `map of string` | HTTP headers to add to the request. Any header key, value can be inserted. | *Optional* | | 2.1 | +| [networks](#vehicleRentalServiceDirectory_networks) | `object[]` | List all networks to include. | *Optional* | | 2.4 | +|       geofencingZones | `boolean` | Enables geofencingZones for the given network | *Optional* | `false` | 2.4 | +|       network | `string` | The network name | *Required* | | 2.4 | @@ -52,6 +55,13 @@ the `router-config.json` HTTP headers to add to the request. Any header key, value can be inserted. +

networks

+ +**Since version:** `2.4` ∙ **Type:** `object[]` ∙ **Cardinality:** `Optional` +**Path:** /vehicleRentalServiceDirectory + +List all networks to include. + @@ -71,7 +81,13 @@ HTTP headers to add to the request. Any header key, value can be inserted. "updaterNetworkName" : "id", "headers" : { "ET-Client-Name" : "otp" - } + }, + "networks" : [ + { + "network" : "oslo-by-sykkel", + "geofencingZones" : true + } + ] } } ``` diff --git a/src/test/java/org/opentripplanner/generate/doc/sandbox/VehicleRentalServiceDirectoryConfigDocTest.java b/src/ext-test/java/org/opentripplanner/ext/vehiclerentalservicedirectory/generatedoc/VehicleRentalServiceDirectoryConfigDocTest.java similarity index 94% rename from src/test/java/org/opentripplanner/generate/doc/sandbox/VehicleRentalServiceDirectoryConfigDocTest.java rename to src/ext-test/java/org/opentripplanner/ext/vehiclerentalservicedirectory/generatedoc/VehicleRentalServiceDirectoryConfigDocTest.java index a64ef03dcd6..51f72c05e3e 100644 --- a/src/test/java/org/opentripplanner/generate/doc/sandbox/VehicleRentalServiceDirectoryConfigDocTest.java +++ b/src/ext-test/java/org/opentripplanner/ext/vehiclerentalservicedirectory/generatedoc/VehicleRentalServiceDirectoryConfigDocTest.java @@ -1,4 +1,4 @@ -package org.opentripplanner.generate.doc.sandbox; +package org.opentripplanner.ext.vehiclerentalservicedirectory.generatedoc; import static org.opentripplanner.framework.application.OtpFileNames.ROUTER_CONFIG_FILENAME; import static org.opentripplanner.framework.io.FileUtils.assertFileEquals; @@ -29,7 +29,7 @@ public class VehicleRentalServiceDirectoryConfigDocTest { private static final File TEMPLATE = new File(TEMPLATE_ROOT, DOCUMENT); private static final File OUT_FILE = new File(DOCS_ROOT, DOCUMENT); private static final String CONFIG_PATH = - "standalone/config/sandbox/build-config-vehicle-rental-service-directory.json"; + "org/opentripplanner/ext/vehiclerentalservicedirectory/generatedoc/" + ROUTER_CONFIG_FILENAME; private static final String CONFIG_TAG = "vehicleRentalServiceDirectory"; private static final SkipNodes SKIP_NODES = SkipNodes.of().build(); diff --git a/src/test/resources/standalone/config/sandbox/build-config-vehicle-rental-service-directory.json b/src/ext-test/resources/org/opentripplanner/ext/vehiclerentalservicedirectory/generatedoc/router-config.json similarity index 64% rename from src/test/resources/standalone/config/sandbox/build-config-vehicle-rental-service-directory.json rename to src/ext-test/resources/org/opentripplanner/ext/vehiclerentalservicedirectory/generatedoc/router-config.json index b4896072950..0c772551549 100644 --- a/src/test/resources/standalone/config/sandbox/build-config-vehicle-rental-service-directory.json +++ b/src/ext-test/resources/org/opentripplanner/ext/vehiclerentalservicedirectory/generatedoc/router-config.json @@ -6,6 +6,12 @@ "updaterNetworkName": "id", "headers": { "ET-Client-Name": "otp" - } + }, + "networks": [ + { + "network" : "oslo-by-sykkel", + "geofencingZones" : true + } + ] } } diff --git a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/VehicleRentalServiceDirectoryFetcher.java b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/VehicleRentalServiceDirectoryFetcher.java index 551dfd5c035..75765d49d7a 100644 --- a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/VehicleRentalServiceDirectoryFetcher.java +++ b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/VehicleRentalServiceDirectoryFetcher.java @@ -2,13 +2,17 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.MissingNode; +import java.net.URI; import java.time.Duration; import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Optional; import org.opentripplanner.ext.vehiclerentalservicedirectory.api.VehicleRentalServiceDirectoryFetcherParameters; import org.opentripplanner.framework.io.OtpHttpClient; import org.opentripplanner.framework.io.OtpHttpClientException; +import org.opentripplanner.framework.json.JsonUtils; import org.opentripplanner.routing.linking.VertexLinker; import org.opentripplanner.service.vehiclerental.VehicleRentalRepository; import org.opentripplanner.updater.spi.GraphUpdater; @@ -38,67 +42,88 @@ public static List createUpdatersFromEndpoint( LOG.info("Fetching list of updaters from {}", parameters.getUrl()); List updaters = new ArrayList<>(); - JsonNode node = null; - try (OtpHttpClient otpHttpClient = new OtpHttpClient()) { - node = otpHttpClient.getAndMapAsJsonNode(parameters.getUrl(), Map.of(), new ObjectMapper()); - } catch (OtpHttpClientException e) { - LOG.warn("Error fetching list of vehicle rental endpoints from {}", parameters.getUrl(), e); - } - if (node == null || node.get(parameters.getSourcesName()) == null) { - LOG.warn( - "Error reading json from {}. Are json tag names configured properly?", - parameters.getUrl() - ); - return updaters; + var sources = listSources(parameters); + + if (sources.isEmpty()) { + return List.of(); } - JsonNode sources = node.get(parameters.getSourcesName()); - if (!sources.isEmpty()) { - int maxHttpConnections = sources.size(); - OtpHttpClient otpHttpClient = new OtpHttpClient(maxHttpConnections); - for (JsonNode source : sources) { - JsonNode network = source.get(parameters.getSourceNetworkName()); - JsonNode updaterUrl = source.get(parameters.getSourceUrlName()); - - if (network == null || updaterUrl == null) { - LOG.warn( - "Error reading json from {}. Are json tag names configured properly?", - parameters.getUrl() - ); - return updaters; - } - - VehicleRentalParameters vehicleRentalParameters = new VehicleRentalParameters( - "vehicle-rental-service-directory:" + network, - DEFAULT_FREQUENCY, - new GbfsVehicleRentalDataSourceParameters( - updaterUrl.asText(), - parameters.getLanguage(), - false, - parameters.getHeaders(), - null, - false, - false - ) - ); - LOG.info("Fetched updater info for {} at url {}", network, updaterUrl); + int maxHttpConnections = sources.size(); + var otpHttpClient = new OtpHttpClient(maxHttpConnections); - var dataSource = VehicleRentalDataSourceFactory.create( - vehicleRentalParameters.sourceParameters(), - otpHttpClient - ); - GraphUpdater updater = new VehicleRentalUpdater( - vehicleRentalParameters, - dataSource, - vertexLinker, - repository + for (JsonNode source : sources) { + Optional network = JsonUtils.asText(source, parameters.getSourceNetworkName()); + Optional updaterUrl = JsonUtils.asText(source, parameters.getSourceUrlName()); + + if (network.isEmpty() || updaterUrl.isEmpty()) { + LOG.warn( + "Error reading json from {}. Are json tag names configured properly?", + parameters.getUrl() ); - updaters.add(updater); + continue; } + + var c = parameters.networkParameters(network.get()); + + VehicleRentalParameters vehicleRentalParameters = new VehicleRentalParameters( + "vehicle-rental-service-directory:" + network, + DEFAULT_FREQUENCY, + new GbfsVehicleRentalDataSourceParameters( + updaterUrl.get(), + parameters.getLanguage(), + // allowKeepingRentedVehicleAtDestination - not part of GBFS, not supported here + false, + parameters.getHeaders(), + network.get(), + c.geofencingZones(), + // overloadingAllowed - not part of GBFS, not supported here + false + ) + ); + LOG.info("Fetched updater info for {} at url {}", network.get(), updaterUrl.get()); + + var dataSource = VehicleRentalDataSourceFactory.create( + vehicleRentalParameters.sourceParameters(), + otpHttpClient + ); + var updater = new VehicleRentalUpdater( + vehicleRentalParameters, + dataSource, + vertexLinker, + repository + ); + updaters.add(updater); } LOG.info("{} updaters fetched", updaters.size()); return updaters; } + + 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..8fb1142e4d0 --- /dev/null +++ b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/NetworkParameters.java @@ -0,0 +1,15 @@ +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. I 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) {} 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..af84cef5c14 100644 --- a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/VehicleRentalServiceDirectoryFetcherParameters.java +++ b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/VehicleRentalServiceDirectoryFetcherParameters.java @@ -1,6 +1,9 @@ package org.opentripplanner.ext.vehiclerentalservicedirectory.api; import java.net.URI; +import java.util.Collection; +import java.util.Map; +import java.util.stream.Collectors; import org.opentripplanner.updater.spi.HttpHeaders; public class VehicleRentalServiceDirectoryFetcherParameters { @@ -17,13 +20,16 @@ public class VehicleRentalServiceDirectoryFetcherParameters { private final String language; + private final Map parametersForNetwork; + 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 +37,8 @@ public VehicleRentalServiceDirectoryFetcherParameters( this.sourceNetworkName = networkName; this.language = language; this.headers = headers; + this.parametersForNetwork = + networkParameters.stream().collect(Collectors.toMap(NetworkParameters::network, it -> it)); } /** @@ -81,4 +89,8 @@ public HttpHeaders getHeaders() { public String getLanguage() { return language; } + + public NetworkParameters networkParameters(String network) { + return parametersForNetwork.get(network); + } } 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/standalone/config/sandbox/VehicleRentalServiceDirectoryFetcherConfig.java b/src/main/java/org/opentripplanner/standalone/config/sandbox/VehicleRentalServiceDirectoryFetcherConfig.java index a96442fc869..6d4edb1bc84 100644 --- a/src/main/java/org/opentripplanner/standalone/config/sandbox/VehicleRentalServiceDirectoryFetcherConfig.java +++ b/src/main/java/org/opentripplanner/standalone/config/sandbox/VehicleRentalServiceDirectoryFetcherConfig.java @@ -2,7 +2,10 @@ 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; @@ -41,7 +44,28 @@ public static VehicleRentalServiceDirectoryFetcherParameters create( .summary("Json tag name for the network name for each source.") .asString("id"), c.of("language").since(V2_1).summary("Language code.").asString(null), - HttpHeadersConfig.headers(c, V2_1) + 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.") + .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/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/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(); /** From 896b7cae99f936f961c0063e9788dfd0e212137b Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Sun, 27 Aug 2023 20:12:58 +0200 Subject: [PATCH 06/53] refactor: Cleanup VehicleRentalServiceDirectoryFetcher --- .../VehicleRentalServiceDirectoryFetcher.java | 101 ++++++++++++------ 1 file changed, 67 insertions(+), 34 deletions(-) diff --git a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/VehicleRentalServiceDirectoryFetcher.java b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/VehicleRentalServiceDirectoryFetcher.java index 75765d49d7a..300761698ce 100644 --- a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/VehicleRentalServiceDirectoryFetcher.java +++ b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/VehicleRentalServiceDirectoryFetcher.java @@ -31,9 +31,22 @@ public class VehicleRentalServiceDirectoryFetcher { private static final Logger LOG = LoggerFactory.getLogger( VehicleRentalServiceDirectoryFetcher.class ); - private static final Duration DEFAULT_FREQUENCY = Duration.ofSeconds(15); + private final VertexLinker vertexLinker; + private final VehicleRentalRepository repository; + private final OtpHttpClient otpHttpClient; + + public VehicleRentalServiceDirectoryFetcher( + VertexLinker vertexLinker, + VehicleRentalRepository repository, + OtpHttpClient otpHttpClient + ) { + this.vertexLinker = vertexLinker; + this.repository = repository; + this.otpHttpClient = otpHttpClient; + } + public static List createUpdatersFromEndpoint( VehicleRentalServiceDirectoryFetcherParameters parameters, VertexLinker vertexLinker, @@ -41,7 +54,6 @@ public static List createUpdatersFromEndpoint( ) { LOG.info("Fetching list of updaters from {}", parameters.getUrl()); - List updaters = new ArrayList<>(); var sources = listSources(parameters); if (sources.isEmpty()) { @@ -51,6 +63,16 @@ public static List createUpdatersFromEndpoint( int maxHttpConnections = sources.size(); var otpHttpClient = new OtpHttpClient(maxHttpConnections); + return new VehicleRentalServiceDirectoryFetcher(vertexLinker, repository, otpHttpClient) + .createUpdatersFromEndpoint(parameters, sources); + } + + public List createUpdatersFromEndpoint( + VehicleRentalServiceDirectoryFetcherParameters parameters, + JsonNode sources + ) { + List updaters = new ArrayList<>(); + for (JsonNode source : sources) { Optional network = JsonUtils.asText(source, parameters.getSourceNetworkName()); Optional updaterUrl = JsonUtils.asText(source, parameters.getSourceUrlName()); @@ -60,39 +82,9 @@ public static List createUpdatersFromEndpoint( "Error reading json from {}. Are json tag names configured properly?", parameters.getUrl() ); - continue; + } else { + createUpdater(parameters, network.get(), updaterUrl.get()).ifPresent(updaters::add); } - - var c = parameters.networkParameters(network.get()); - - VehicleRentalParameters vehicleRentalParameters = new VehicleRentalParameters( - "vehicle-rental-service-directory:" + network, - DEFAULT_FREQUENCY, - new GbfsVehicleRentalDataSourceParameters( - updaterUrl.get(), - parameters.getLanguage(), - // allowKeepingRentedVehicleAtDestination - not part of GBFS, not supported here - false, - parameters.getHeaders(), - network.get(), - c.geofencingZones(), - // overloadingAllowed - not part of GBFS, not supported here - false - ) - ); - LOG.info("Fetched updater info for {} at url {}", network.get(), updaterUrl.get()); - - var dataSource = VehicleRentalDataSourceFactory.create( - vehicleRentalParameters.sourceParameters(), - otpHttpClient - ); - var updater = new VehicleRentalUpdater( - vehicleRentalParameters, - dataSource, - vertexLinker, - repository - ); - updaters.add(updater); } LOG.info("{} updaters fetched", updaters.size()); @@ -100,6 +92,47 @@ public static List createUpdatersFromEndpoint( return updaters; } + private Optional createUpdater( + VehicleRentalServiceDirectoryFetcherParameters parameters, + String networkName, + String updaterUrl + ) { + var config = parameters.networkParameters(networkName); + + if (config == null) { + LOG.info("Network not configured in OTP: {}", networkName); + return Optional.empty(); + } + + VehicleRentalParameters vehicleRentalParameters = new VehicleRentalParameters( + "vehicle-rental-service-directory:" + networkName, + DEFAULT_FREQUENCY, + new GbfsVehicleRentalDataSourceParameters( + updaterUrl, + parameters.getLanguage(), + // allowKeepingRentedVehicleAtDestination - not part of GBFS, not supported here + false, + parameters.getHeaders(), + networkName, + config.geofencingZones(), + // overloadingAllowed - not part of GBFS, not supported here + false + ) + ); + LOG.info("Fetched updater info for {} at url {}", networkName, updaterUrl); + + var dataSource = VehicleRentalDataSourceFactory.create( + vehicleRentalParameters.sourceParameters(), + otpHttpClient + ); + return Optional.of( new VehicleRentalUpdater( + vehicleRentalParameters, + dataSource, + vertexLinker, + repository + )); + } + private static JsonNode listSources(VehicleRentalServiceDirectoryFetcherParameters parameters) { JsonNode node; URI url = parameters.getUrl(); From 03424ce0ecf33efcbc271fb67962354ec8705f7d Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Mon, 28 Aug 2023 15:05:54 +0200 Subject: [PATCH 07/53] refactor: Improve logging for VehicleRental updaters --- .../VehicleRentalServiceDirectoryFetcher.java | 85 +++++++++++-------- .../GbfsVehicleRentalDataSource.java | 22 ++++- 2 files changed, 69 insertions(+), 38 deletions(-) diff --git a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/VehicleRentalServiceDirectoryFetcher.java b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/VehicleRentalServiceDirectoryFetcher.java index 300761698ce..508b7f2809b 100644 --- a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/VehicleRentalServiceDirectoryFetcher.java +++ b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/VehicleRentalServiceDirectoryFetcher.java @@ -63,15 +63,28 @@ public static List createUpdatersFromEndpoint( int maxHttpConnections = sources.size(); var otpHttpClient = new OtpHttpClient(maxHttpConnections); - return new VehicleRentalServiceDirectoryFetcher(vertexLinker, repository, otpHttpClient) - .createUpdatersFromEndpoint(parameters, sources); + var serviceDirectory = new VehicleRentalServiceDirectoryFetcher( + vertexLinker, + repository, + otpHttpClient + ); + return serviceDirectory.createUpdatersFromEndpoint(parameters, sources); } public List createUpdatersFromEndpoint( VehicleRentalServiceDirectoryFetcherParameters parameters, JsonNode sources ) { - List updaters = new ArrayList<>(); + return fetchUpdaterInfoFromDirectoryAndCreateUpdaters( + buildListOfNetworksFromConfig(parameters, sources) + ); + } + + private static List buildListOfNetworksFromConfig( + VehicleRentalServiceDirectoryFetcherParameters parameters, + JsonNode sources + ) { + List dataSources = new ArrayList<>(); for (JsonNode source : sources) { Optional network = JsonUtils.asText(source, parameters.getSourceNetworkName()); @@ -83,54 +96,58 @@ public List createUpdatersFromEndpoint( parameters.getUrl() ); } else { - createUpdater(parameters, network.get(), updaterUrl.get()).ifPresent(updaters::add); + var networkName = network.get(); + var config = parameters.networkParameters(networkName); + + if (config == null) { + LOG.warn("Network not configured in OTP: {}", networkName); + } else { + dataSources.add( + new GbfsVehicleRentalDataSourceParameters( + updaterUrl.get(), + parameters.getLanguage(), + // allowKeepingRentedVehicleAtDestination - not part of GBFS, not supported here + false, + parameters.getHeaders(), + networkName, + config.geofencingZones(), + // overloadingAllowed - not part of GBFS, not supported here + false + ) + ); + } } } + return dataSources; + } + private List fetchUpdaterInfoFromDirectoryAndCreateUpdaters( + List dataSources + ) { + List updaters = new ArrayList<>(); + for (var it : dataSources) { + updaters.add(fetchAndCreateUpdater(it)); + } LOG.info("{} updaters fetched", updaters.size()); - return updaters; } - private Optional createUpdater( - VehicleRentalServiceDirectoryFetcherParameters parameters, - String networkName, - String updaterUrl + private VehicleRentalUpdater fetchAndCreateUpdater( + GbfsVehicleRentalDataSourceParameters parameters ) { - var config = parameters.networkParameters(networkName); - - if (config == null) { - LOG.info("Network not configured in OTP: {}", networkName); - return Optional.empty(); - } + LOG.info("Fetched updater info for {} at url {}", parameters.network(), parameters.url()); VehicleRentalParameters vehicleRentalParameters = new VehicleRentalParameters( - "vehicle-rental-service-directory:" + networkName, + "vehicle-rental-service-directory:" + parameters.network(), DEFAULT_FREQUENCY, - new GbfsVehicleRentalDataSourceParameters( - updaterUrl, - parameters.getLanguage(), - // allowKeepingRentedVehicleAtDestination - not part of GBFS, not supported here - false, - parameters.getHeaders(), - networkName, - config.geofencingZones(), - // overloadingAllowed - not part of GBFS, not supported here - false - ) + parameters ); - LOG.info("Fetched updater info for {} at url {}", networkName, updaterUrl); var dataSource = VehicleRentalDataSourceFactory.create( vehicleRentalParameters.sourceParameters(), otpHttpClient ); - return Optional.of( new VehicleRentalUpdater( - vehicleRentalParameters, - dataSource, - vertexLinker, - repository - )); + return new VehicleRentalUpdater(vehicleRentalParameters, dataSource, vertexLinker, repository); } private static JsonNode listSources(VehicleRentalServiceDirectoryFetcherParameters parameters) { 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; } From e01906ba478fec367de0631f992a72f6af6ad1ce Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Wed, 30 Aug 2023 20:58:35 +0200 Subject: [PATCH 08/53] refactor: Small improvements to ThrottleLogger and MaxCountLogger --- .../opentripplanner/framework/logging/MaxCountLogger.java | 7 ++++--- .../opentripplanner/framework/logging/ThrottleLogger.java | 6 +++--- .../netex/loader/parser/ServiceFrameParser.java | 4 +--- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/opentripplanner/framework/logging/MaxCountLogger.java b/src/main/java/org/opentripplanner/framework/logging/MaxCountLogger.java index 0cfa7004f3c..721efd7d90f 100644 --- a/src/main/java/org/opentripplanner/framework/logging/MaxCountLogger.java +++ b/src/main/java/org/opentripplanner/framework/logging/MaxCountLogger.java @@ -21,14 +21,15 @@ 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/ThrottleLogger.java index 6958e003357..eaddf1fad6e 100644 --- a/src/main/java/org/opentripplanner/framework/logging/ThrottleLogger.java +++ b/src/main/java/org/opentripplanner/framework/logging/ThrottleLogger.java @@ -15,7 +15,7 @@ * 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 + * This class is used to wrap the original logger, and it will forward only one log event per * second. *

* THREAD SAFETY - The implementation is very simple and do not do any synchronization, so it is @@ -26,7 +26,7 @@ public class ThrottleLogger extends AbstractFilterLogger { private static final int STALL_PERIOD_MILLISECONDS = 1000; - private long timeout = Long.MIN_VALUE; + private volatile long timeout = Long.MIN_VALUE; private ThrottleLogger(Logger delegate) { super(delegate); @@ -59,7 +59,7 @@ boolean mute() { *

* In a 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() { long time = System.currentTimeMillis(); 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; From 01e1bb241fd49c81ca0dc8ed3c76730299ddda2f Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Wed, 30 Aug 2023 21:00:13 +0200 Subject: [PATCH 09/53] refactor: Improve logging in VehicleRentalUpdater --- .../vehicle_rental/VehicleRentalUpdater.java | 48 ++++++++++++++----- .../VehicleRentalDataSourceParameters.java | 4 ++ .../VehicleRentalUpdaterTest.java | 7 +++ 3 files changed, 47 insertions(+), 12 deletions(-) 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..9c0d4cd8018 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.ThrottleLogger; +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,7 +48,12 @@ public class VehicleRentalUpdater extends PollingGraphUpdater { private static final Logger LOG = LoggerFactory.getLogger(VehicleRentalUpdater.class); + + private final Logger unlinkedPlaceLogger; + private final VehicleRentalDatasource source; + private final String nameForLogging; + private WriteToGraphCallback saveResultOnGraph; private Map latestModifiedEdges = Map.of(); @@ -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.unlinkedPlaceLogger = ThrottleLogger.throttle(LOG); // 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( @@ -169,7 +187,11 @@ 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); + unlinkedPlaceLogger.info( + "VehicleRentalPlace is unlinked for {}: {}", + nameForLogging, + vehicleRentalVertex + ); } Set formFactors = Stream .concat( @@ -188,6 +210,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 +229,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 +241,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/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/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() { From 57c016e66feb1bc5ef7715c4db0719565d66d4a8 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 31 Aug 2023 13:32:42 +0200 Subject: [PATCH 10/53] refactor: Add method to format system-times in milliseconds for logging. --- .../framework/time/TimeUtils.java | 29 +++++++++++++++++++ .../framework/time/TimeUtilsTest.java | 16 ++++++++++ 2 files changed, 45 insertions(+) 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/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; } From 2f305512f740f09eab380a8d87cabff66c781663 Mon Sep 17 00:00:00 2001 From: Vincent Paturet Date: Thu, 31 Aug 2023 13:45:10 +0200 Subject: [PATCH 11/53] Add maxSearchWindow parameter --- docs/RouterConfiguration.md | 10 ++++++++++ .../transit/TransitTuningParameters.java | 7 +++++++ .../routerconfig/TransitRoutingConfig.java | 17 +++++++++++++++++ 3 files changed, 34 insertions(+) diff --git a/docs/RouterConfiguration.md b/docs/RouterConfiguration.md index bdb39df2bf6..44769ecb69f 100644 --- a/docs/RouterConfiguration.md +++ b/docs/RouterConfiguration.md @@ -50,6 +50,7 @@ A full list of them can be found in the [RouteRequest](RouteRequest.md). | [transit](#transit) | `object` | Configuration for transit searches with RAPTOR. | *Optional* | | na | |    [iterationDepartureStepInSeconds](#transit_iterationDepartureStepInSeconds) | `integer` | Step for departure times between each RangeRaptor iterations. | *Optional* | `60` | na | |    [maxNumberOfTransfers](#transit_maxNumberOfTransfers) | `integer` | This parameter is used to allocate enough memory space for Raptor. | *Optional* | `12` | na | +|    [maxSearchWindow](#transit_maxSearchWindow) | `duration` | Upper limit of the request parameter searchWindow. | *Optional* | `"PT24H"` | 2.4 | |    [scheduledTripBinarySearchThreshold](#transit_scheduledTripBinarySearchThreshold) | `integer` | This threshold is used to determine when to perform a binary trip schedule search. | *Optional* | `50` | na | |    [searchThreadPoolSize](#transit_searchThreadPoolSize) | `integer` | Split a travel search in smaller jobs and run them in parallel to improve performance. | *Optional* | `0` | na | |    [transferCacheMaxSize](#transit_transferCacheMaxSize) | `integer` | The maximum number of distinct transfers parameters to cache pre-calculated transfers for. | *Optional* | `25` | na | @@ -204,6 +205,15 @@ entire transit network. The memory overhead of setting this higher than the maxi transfers is very little so it is better to set it too high than to low. +

maxSearchWindow

+ +**Since version:** `2.4` ∙ **Type:** `duration` ∙ **Cardinality:** `Optional` ∙ **Default value:** `"PT24H"` +**Path:** /transit + +Upper limit of the request parameter searchWindow. + +Maximum search window that can be set through the searchWindow API parameter. +

scheduledTripBinarySearchThreshold

**Since version:** `na` ∙ **Type:** `integer` ∙ **Cardinality:** `Optional` ∙ **Default value:** `50` diff --git a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransitTuningParameters.java b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransitTuningParameters.java index 59e4436b09f..0f92bb449d3 100644 --- a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransitTuningParameters.java +++ b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransitTuningParameters.java @@ -47,6 +47,11 @@ public int transferCacheMaxSize() { return 5; } + @Override + public Duration maxSearchWindow() { + return Duration.ofHours(24); + } + @Override public List pagingSearchWindowAdjustments() { return PAGING_SEARCH_WINDOW_ADJUSTMENTS; @@ -77,6 +82,8 @@ public List transferCacheRequests() { */ int transferCacheMaxSize(); + Duration maxSearchWindow(); + /** * This parameter is used to reduce the number of pages a client have to step through for a * journey where there are few alternatives/low frequency. This also work well to adjust for diff --git a/src/main/java/org/opentripplanner/standalone/config/routerconfig/TransitRoutingConfig.java b/src/main/java/org/opentripplanner/standalone/config/routerconfig/TransitRoutingConfig.java index 79513866e43..106c7eaba98 100644 --- a/src/main/java/org/opentripplanner/standalone/config/routerconfig/TransitRoutingConfig.java +++ b/src/main/java/org/opentripplanner/standalone/config/routerconfig/TransitRoutingConfig.java @@ -5,6 +5,7 @@ import static org.opentripplanner.standalone.config.framework.json.OtpVersion.V2_1; import static org.opentripplanner.standalone.config.framework.json.OtpVersion.V2_2; import static org.opentripplanner.standalone.config.framework.json.OtpVersion.V2_3; +import static org.opentripplanner.standalone.config.framework.json.OtpVersion.V2_4; import java.time.Duration; import java.util.List; @@ -31,6 +32,7 @@ public final class TransitRoutingConfig implements RaptorTuningParameters, Trans private final List pagingSearchWindowAdjustments; private final Map stopTransferCost; + private final Duration maxSearchWindow; private final DynamicSearchWindowCoefficients dynamicSearchWindowCoefficients; public TransitRoutingConfig( @@ -200,6 +202,16 @@ public TransitRoutingConfig( ) .asDurations(PAGING_SEARCH_WINDOW_ADJUSTMENTS); + this.maxSearchWindow = + c + .of("maxSearchWindow") + .since(V2_4) + .summary("Upper limit of the request parameter searchWindow.") + .description( + "Maximum search window that can be set through the searchWindow API parameter." + ) + .asDuration(Duration.ofHours(24)); + this.dynamicSearchWindowCoefficients = new DynamicSearchWindowConfig("dynamicSearchWindow", c); } @@ -248,6 +260,11 @@ public List transferCacheRequests() { return transferCacheRequests; } + @Override + public Duration maxSearchWindow() { + return maxSearchWindow; + } + @Override public List pagingSearchWindowAdjustments() { return pagingSearchWindowAdjustments; From d014d0fe84d81e1b4eb625b746ea434f08ef06b6 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 31 Aug 2023 13:45:02 +0200 Subject: [PATCH 12/53] refactor: ThrottleLogger, and replace with Throttle --- .../logging/AbstractFilterLogger.java | 6 +- .../framework/logging/MaxCountLogger.java | 4 + .../{ThrottleLogger.java => Throttle.java} | 60 +++++++------- .../path/DestinationArrivalPaths.java | 17 ++-- .../OptimizeTransferService.java | 17 ++-- .../vehicle_rental/VehicleRentalUpdater.java | 25 +++--- .../framework/logging/ThrottleLoggerTest.java | 48 ----------- .../framework/logging/ThrottleTest.java | 80 +++++++++++++++++++ 8 files changed, 151 insertions(+), 106 deletions(-) rename src/main/java/org/opentripplanner/framework/logging/{ThrottleLogger.java => Throttle.java} (54%) delete mode 100644 src/test/java/org/opentripplanner/framework/logging/ThrottleLoggerTest.java create mode 100644 src/test/java/org/opentripplanner/framework/logging/ThrottleTest.java 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 721efd7d90f..05df3f3e45c 100644 --- a/src/main/java/org/opentripplanner/framework/logging/MaxCountLogger.java +++ b/src/main/java/org/opentripplanner/framework/logging/MaxCountLogger.java @@ -15,7 +15,11 @@ * 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; diff --git a/src/main/java/org/opentripplanner/framework/logging/ThrottleLogger.java b/src/main/java/org/opentripplanner/framework/logging/Throttle.java similarity index 54% rename from src/main/java/org/opentripplanner/framework/logging/ThrottleLogger.java rename to src/main/java/org/opentripplanner/framework/logging/Throttle.java index eaddf1fad6e..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 per - * 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 volatile long timeout = Long.MIN_VALUE; + 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 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/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/updater/vehicle_rental/VehicleRentalUpdater.java b/src/main/java/org/opentripplanner/updater/vehicle_rental/VehicleRentalUpdater.java index 9c0d4cd8018..1e037a7c1c2 100644 --- a/src/main/java/org/opentripplanner/updater/vehicle_rental/VehicleRentalUpdater.java +++ b/src/main/java/org/opentripplanner/updater/vehicle_rental/VehicleRentalUpdater.java @@ -11,7 +11,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import org.opentripplanner.framework.lang.ObjectUtils; -import org.opentripplanner.framework.logging.ThrottleLogger; +import org.opentripplanner.framework.logging.Throttle; import org.opentripplanner.framework.time.DurationUtils; import org.opentripplanner.framework.time.TimeUtils; import org.opentripplanner.framework.tostring.ToStringBuilder; @@ -49,7 +49,7 @@ public class VehicleRentalUpdater extends PollingGraphUpdater { private static final Logger LOG = LoggerFactory.getLogger(VehicleRentalUpdater.class); - private final Logger unlinkedPlaceLogger; + private final Throttle unlinkedPlaceThrottle; private final VehicleRentalDatasource source; private final String nameForLogging; @@ -58,8 +58,8 @@ public class VehicleRentalUpdater extends PollingGraphUpdater { 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; @@ -80,7 +80,7 @@ public VehicleRentalUpdater( parameters.sourceParameters().network(), parameters.sourceParameters().url() ); - this.unlinkedPlaceLogger = ThrottleLogger.throttle(LOG); + this.unlinkedPlaceThrottle = Throttle.ofOneSecond(); // Creation of network linker library will not modify the graph this.linker = vertexLinker; @@ -186,11 +186,16 @@ public void run(Graph graph, TransitModel transitModel) { ) ); if (vehicleRentalVertex.getOutgoing().isEmpty()) { - // the toString includes the text "Bike rental station" - unlinkedPlaceLogger.info( - "VehicleRentalPlace is unlinked for {}: {}", - nameForLogging, - 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 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; + } +} From 933a0fae07b624f243c3f18983414060b4eb0739 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 31 Aug 2023 13:30:25 +0200 Subject: [PATCH 13/53] feature: Add "default-network" to allow all networks to be merged, even not listed. --- docs/sandbox/VehicleRentalServiceDirectory.md | 30 +++++++++++-------- .../api/NetworkParameters.java | 6 +++- ...ntalServiceDirectoryFetcherParameters.java | 9 +++++- ...leRentalServiceDirectoryFetcherConfig.java | 17 ++++++++++- 4 files changed, 47 insertions(+), 15 deletions(-) diff --git a/docs/sandbox/VehicleRentalServiceDirectory.md b/docs/sandbox/VehicleRentalServiceDirectory.md index 2952a743ea3..29a45ebb062 100644 --- a/docs/sandbox/VehicleRentalServiceDirectory.md +++ b/docs/sandbox/VehicleRentalServiceDirectory.md @@ -28,17 +28,17 @@ the `router-config.json` -| Config Parameter | Type | Summary | Req./Opt. | Default Value | Since | -|-----------------------------------------------------|:---------------:|----------------------------------------------------------------------------|:----------:|---------------|:-----:| -| language | `string` | Language code. | *Optional* | | 2.1 | -| sourcesName | `string` | Json tag name for updater sources. | *Optional* | `"systems"` | 2.1 | -| updaterNetworkName | `string` | Json tag name for the network name for each source. | *Optional* | `"id"` | 2.1 | -| updaterUrlName | `string` | Json tag name for endpoint urls for each source. | *Optional* | `"url"` | 2.1 | -| url | `uri` | Endpoint for the VehicleRentalServiceDirectory | *Required* | | 2.1 | -| [headers](#vehicleRentalServiceDirectory_headers) | `map of string` | HTTP headers to add to the request. Any header key, value can be inserted. | *Optional* | | 2.1 | -| [networks](#vehicleRentalServiceDirectory_networks) | `object[]` | List all networks to include. | *Optional* | | 2.4 | -|       geofencingZones | `boolean` | Enables geofencingZones for the given network | *Optional* | `false` | 2.4 | -|       network | `string` | The network name | *Required* | | 2.4 | +| Config Parameter | Type | Summary | Req./Opt. | Default Value | Since | +|-----------------------------------------------------|:---------------:|---------------------------------------------------------------------------------|:----------:|---------------|:-----:| +| language | `string` | Language code. | *Optional* | | 2.1 | +| sourcesName | `string` | Json tag name for updater sources. | *Optional* | `"systems"` | 2.1 | +| updaterNetworkName | `string` | Json tag name for the network name for each source. | *Optional* | `"id"` | 2.1 | +| updaterUrlName | `string` | Json tag name for endpoint urls for each source. | *Optional* | `"url"` | 2.1 | +| url | `uri` | Endpoint for the VehicleRentalServiceDirectory | *Required* | | 2.1 | +| [headers](#vehicleRentalServiceDirectory_headers) | `map of string` | HTTP headers to add to the request. Any header key, value can be inserted. | *Optional* | | 2.1 | +| [networks](#vehicleRentalServiceDirectory_networks) | `object[]` | List all networks to include. Use "network": "default-network" to set defaults. | *Optional* | | 2.4 | +|       geofencingZones | `boolean` | Enables geofencingZones for the given network | *Optional* | `false` | 2.4 | +|       network | `string` | The network name | *Required* | | 2.4 | @@ -60,7 +60,13 @@ HTTP headers to add to the request. Any header key, value can be inserted. **Since version:** `2.4` ∙ **Type:** `object[]` ∙ **Cardinality:** `Optional` **Path:** /vehicleRentalServiceDirectory -List all networks to include. +List all networks to include. Use "network": "default-network" to set defaults. + +If no default network exist only the listed networks are used. Configure the a network +with name "default-network" to include all unlisted networks. If not present, all +unlisted networks is dropped. Note! The values int the "default-network" is not used to +set missing field values in networks listed. + diff --git a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/NetworkParameters.java b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/NetworkParameters.java index 8fb1142e4d0..2f4ae49de62 100644 --- a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/NetworkParameters.java +++ b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/NetworkParameters.java @@ -12,4 +12,8 @@ * @param network The network name * @param geofencingZones enable geofencingZones for the given network */ -public record NetworkParameters(String network, boolean geofencingZones) {} +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 af84cef5c14..abd2b7d8afc 100644 --- a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/VehicleRentalServiceDirectoryFetcherParameters.java +++ b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/VehicleRentalServiceDirectoryFetcherParameters.java @@ -4,10 +4,12 @@ import java.util.Collection; import java.util.Map; 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; @@ -22,6 +24,9 @@ public class VehicleRentalServiceDirectoryFetcherParameters { private final Map parametersForNetwork; + @Nullable + private final NetworkParameters defaultNetwork; + public VehicleRentalServiceDirectoryFetcherParameters( URI url, String sourcesName, @@ -39,6 +44,7 @@ public VehicleRentalServiceDirectoryFetcherParameters( this.headers = headers; this.parametersForNetwork = networkParameters.stream().collect(Collectors.toMap(NetworkParameters::network, it -> it)); + this.defaultNetwork = parametersForNetwork.get(DEFAULT_NETWORK_NAME); } /** @@ -90,7 +96,8 @@ public String getLanguage() { return language; } + @Nullable public NetworkParameters networkParameters(String network) { - return parametersForNetwork.get(network); + return parametersForNetwork.getOrDefault(network, defaultNetwork); } } 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 6d4edb1bc84..1576c0df560 100644 --- a/src/main/java/org/opentripplanner/standalone/config/sandbox/VehicleRentalServiceDirectoryFetcherConfig.java +++ b/src/main/java/org/opentripplanner/standalone/config/sandbox/VehicleRentalServiceDirectoryFetcherConfig.java @@ -56,7 +56,22 @@ private static List mapNetworkParameters( return root .of(parameterName) .since(V2_4) - .summary("List all networks to include.") + .summary( + "List all networks to include. Use \"network\": \"" + + VehicleRentalServiceDirectoryFetcherParameters.DEFAULT_NETWORK_NAME + + "\" to set defaults." + ) + .description( + """ + If no default network exist only the listed networks are used. Configure the a network + with name "{{default-network}}" to include all unlisted networks. If not present, all + unlisted networks is dropped. Note! The values int the "{{default-network}}" is 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(), From b3360d5590550134d7103ed74fac5996ce6ad933 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Wed, 27 Sep 2023 14:48:33 +0200 Subject: [PATCH 14/53] Fix check for invalid relation references --- .../graph_builder/module/osm/OsmDatabase.java | 10 +++- .../model/OSMRelationMember.java | 4 ++ .../module/osm/OsmDatabaseTest.java | 54 ++++++++++++++++++ .../model/BicycleNetworkRelationsTest.java | 35 ------------ ...brenner-invalid-relation-reference.osm.pbf | Bin 0 -> 15024 bytes 5 files changed, 67 insertions(+), 36 deletions(-) create mode 100644 src/test/java/org/opentripplanner/graph_builder/module/osm/OsmDatabaseTest.java delete mode 100644 src/test/java/org/opentripplanner/openstreetmap/model/BicycleNetworkRelationsTest.java create mode 100644 src/test/resources/org/opentripplanner/graph_builder/module/osm/brenner-invalid-relation-reference.osm.pbf diff --git a/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java b/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java index 599c66ef585..6c52e780211 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java @@ -1062,7 +1062,7 @@ private void processPublicTransportStopArea(OSMRelation relation) { } } case WAY -> { - if (member.hasRolePlatform() && areaWayIds.contains(member.getRef())) { + if (member.hasRolePlatform() && areaWaysById.containsKey(member.getRef())) { platformAreas.add(areaWaysById.get(member.getRef())); } } @@ -1075,6 +1075,14 @@ private void processPublicTransportStopArea(OSMRelation relation) { } for (OSMWithTags area : platformAreas) { + if (area == null) { + throw new RuntimeException( + "Could not process public transport relation '%s' (%s)".formatted( + relation, + relation.url() + ) + ); + } // single platform area presumably contains only one level in most cases // a node inside it may specify several levels if it is an elevator // make sure each node has access to the current platform level diff --git a/src/main/java/org/opentripplanner/openstreetmap/model/OSMRelationMember.java b/src/main/java/org/opentripplanner/openstreetmap/model/OSMRelationMember.java index e2ad2ff9691..f8cf363ad65 100644 --- a/src/main/java/org/opentripplanner/openstreetmap/model/OSMRelationMember.java +++ b/src/main/java/org/opentripplanner/openstreetmap/model/OSMRelationMember.java @@ -50,6 +50,10 @@ public boolean hasTypeWay() { return type == WAY; } + public String url() { + return "https://www.openstreetmap.org/%s/%s".formatted(type.toString().toLowerCase(), ref); + } + @Override public String toString() { return "osm rel " + type + ":" + role + ":" + ref; diff --git a/src/test/java/org/opentripplanner/graph_builder/module/osm/OsmDatabaseTest.java b/src/test/java/org/opentripplanner/graph_builder/module/osm/OsmDatabaseTest.java new file mode 100644 index 00000000000..4dcb91753a9 --- /dev/null +++ b/src/test/java/org/opentripplanner/graph_builder/module/osm/OsmDatabaseTest.java @@ -0,0 +1,54 @@ +package org.opentripplanner.graph_builder.module.osm; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +import java.io.File; +import org.junit.jupiter.api.Test; +import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore; +import org.opentripplanner.openstreetmap.OsmProvider; + +public class OsmDatabaseTest { + + /** + * The way https://www.openstreetmap.org/way/13876983 does not contain the tag lcn (local cycling network) + * but because it is part of a relation that _does_, the tag is copied from the relation to the way. + * This test assert that this is really happening. + */ + @Test + void bicycleRouteRelations() { + var osmdb = new OsmDatabase(DataImportIssueStore.NOOP); + var provider = new OsmProvider( + new File("src/test/resources/germany/ehningen-minimal.osm.pbf"), + true + ); + provider.readOSM(osmdb); + osmdb.postLoad(); + + var way = osmdb.getWay(13876983L); + assertNotNull(way); + + assertEquals(way.getTag("lcn"), "yes"); + assertEquals(way.getTag("name"), "Gärtringer Weg"); + } + + /** + * When extracting Austria, Geofabrik produces data where a public transport relation that crosses + * a border (https://www.openstreetmap.org/relation/4027804) references ways that are not in the + * extract. This needs to be dealt with gracefully. + */ + @Test + void invalidPublicTransportRelation() { + var osmdb = new OsmDatabase(DataImportIssueStore.NOOP); + var url = getClass().getResource("brenner-invalid-relation-reference.osm.pbf"); + assertNotNull(url); + var file = new File(url.getFile()); + var provider = new OsmProvider(file, true); + provider.readOSM(osmdb); + osmdb.postLoad(); + + var way = osmdb.getWay(302732658L); + assertNotNull(way); + assertEquals(way.getTag("public_transport"), "platform"); + } +} diff --git a/src/test/java/org/opentripplanner/openstreetmap/model/BicycleNetworkRelationsTest.java b/src/test/java/org/opentripplanner/openstreetmap/model/BicycleNetworkRelationsTest.java deleted file mode 100644 index be8fca3864d..00000000000 --- a/src/test/java/org/opentripplanner/openstreetmap/model/BicycleNetworkRelationsTest.java +++ /dev/null @@ -1,35 +0,0 @@ -package org.opentripplanner.openstreetmap.model; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; - -import java.io.File; -import org.junit.jupiter.api.Test; -import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore; -import org.opentripplanner.graph_builder.module.osm.OsmDatabase; -import org.opentripplanner.openstreetmap.OsmProvider; - -public class BicycleNetworkRelationsTest { - - /* The way https://www.openstreetmap.org/way/13876983 does not contain the tag lcn (local cycling network) - * but because it is part of a relation that _does_, the tag is copied from the relation to the way. - * This test assert that this is really happening. - */ - @Test - public void testBicycleRouteRelations() { - var issueStore = DataImportIssueStore.NOOP; - var osmdb = new OsmDatabase(issueStore); - var provider = new OsmProvider( - new File("src/test/resources/germany/ehningen-minimal.osm.pbf"), - true - ); - provider.readOSM(osmdb); - osmdb.postLoad(); - - var way = osmdb.getWay(13876983L); - assertNotNull(way); - - assertEquals(way.getTag("lcn"), "yes"); - assertEquals(way.getTag("name"), "Gärtringer Weg"); - } -} diff --git a/src/test/resources/org/opentripplanner/graph_builder/module/osm/brenner-invalid-relation-reference.osm.pbf b/src/test/resources/org/opentripplanner/graph_builder/module/osm/brenner-invalid-relation-reference.osm.pbf new file mode 100644 index 0000000000000000000000000000000000000000..250e4da1a32002ac6c6960622cf96729809eba82 GIT binary patch literal 15024 zcmZ8`Q;;T1u;tgbZQHh{ZQHi(X}f#cwr$(CZQJhI`ET5P*o}IrIGIruQJIxE2><}# zprPcIWhG1vO-!9Ir4a?N#XYi=4;~q2;N)f`re&AlSms!i51b^YY2=kYQ&S zsOMqIX(eJ9#wDhsUR^AWj;0xArfH{_IhI&hmstRSpkJt90000M8sgvCB8D!8m_0Cv z83@>IFdkX13Z62k>bN_vJ5x7ThYp%t=pq^l_n?Z}QJ>=0RYv6zw#ymhwq%W+ATl~; z<5z_J{?M2v`FXg|QiwEA>YC-m=s}Pg@fElQVuDFhG)khAWLg9BE*gI-LSD&x-@9)= z{V3?k>p@X)@n#H;$#eEOL)AYkO8H1qWSOz~f=n0ZSO=ifyo84!6~+7*+_w86qq+a= zdmi$qoM>v{ks2aYCNE%j7oDhQ1{)W!t&iRk28EEQ#alxW`JXBA^|xFq20K45uGJ? z8EgPD$z(Kak=PduaXDZ?|I{msvs|fPiyg$KA$0lNMLvaTY1B66Pz_n|y1KYF0-Xt( zV+h*igENie^NPi_OE=&*ve&76^)MkR)~$`}nV=Mvv>{~}xleAs!juM?n+0k!N(cyo z!}KA99}0D90;)6vZ|Gt3N-=j?saDX^YUPMLd0solo1Qw5$bo}H`5Da|Am$1<{>22l z5wd+s$VMc_kDCZ?4IbAnVi=uF@oYpwG-2*^jKb9e=L9|T3=A33PyoZ0*V0sn*WQT+ zSZ>h8BQ)qNtX|;nvuz|$^R|C^TrX|dtxG{ zP>qV3$acD|oB<$Aq!Vs+EhG9tu6m&aOGW0>kq*&>prXPhAqQ(D|3)<&wxy!RIYP&! zO-bhRW$r?whE~cA*pN0due+Ct=9@alt(UI8u?IQnQHLUykQtHFy(bD;TS(>DXTQ4~ zJNuH&t~>Scqm$b5AcpU01ChQQ3l=x|$np3@(H|y5L_wHeTLD)U2lDuOf1_Da3p7eF zTp{Az3ckrEOF(aB?Th!hTlmFm3D$}uC<*sb2zWN{+oJYp1;+pOdg3fwU-1Fx5(pM1 zm(c;C*)_2S9?>Jdn)=XU*IiBs(QWLmFNx0bbuK;Y9bfRi*Q2GxA-D(h3zfG6l0wu6 zFG9A@xa3+S_2(+AgTdbVJ4l~qW2b&w*QsHPzl?u4pj9-CCvll2rXRsE%88uz@l6l? zmgS=kk~hI~z|}lbWkhos!#NHUR6w(gUzPOkk7HbY4JF>}>BQ5SiRah!&8`4sMwa*7 z%5m$-0FA;2Rk1o)FI!f4zg9vkIN>z$ijL#Imd`*NnW$g8F0D(uydnju`0!6G%OT2c zEac7=lP875Qg2E=QcjEkNb1iP;nx=L7{Blxncz+M!0?3cu5!m3O8v*z#0AvzN_dl6 zcsFnMZU%8^8TNKV7|$WQRDll@6h$E_9>cLhQgvC7=jMdgvY>0w3%Y(ity-qJ;P;dT z#kHsw2afV+FV3ymQ93>ZFQ7EDR z2~<|WdKU2=x&w0X>RpG8L@g~4PY@KdaHq1!Q|h8Kj9~8$a9rRml{)Au*gW+g}23Rr?mV)klwQSwo z06fa5q<@iVQf^02bj1<`R8J_0W-_}YZjA@0zm?1PxW5qA(S8%zjN(zoO8`Vbsgf0) zY>chV1?2bhvTo0gXdN9C+ojBS;O*?+=vn?!xsRmBwr*G&hhVhHYt`NPv+)gO5OY0G zD=nMCk6D>#JgaznGHB5v^{$#*b$LdyqboyB;0R@hAMjl2KA&W zsP2c(w#<&bmhpE@BU}H2H)z+fUiKzkE;>q;Vr4E;*96He<+?GyptJ$pL)}xRYFWvG zR;i3;R*|;{0_t^RgzNKlCqA)r#kQ#o3@hgnMcfyT9`0xCHb&(+@C3w{Ppm#*96E%n?y#RizyJzH zGCn$)MLfQCb@=6{M|*PotCsM=Z;ICsISh37g-06I2JQwm-MNO+#Va2+jL(pm%FEKMUcW5>%7d!8aS)6@7R{_m6gKdUgo>6?c+_VIB^=WK#pIOL$Bs|1=UiE3oN&emB}5 z?@&-T@C(BB4lE)T^Jz0z$#E+7%)t(O)CU(B1oUj;F`L3V#+zHxyWlb1`U zWoO5xoH)bSZRw0JSUaOJT&kZGy&hX}KBJGpj0v7NQf^I+CO0f3;__dp8PegvTT!d zKqPnhpAgVEDkH@t?m|xO3F|0ak5fh$1`4t(nI{M;4?pbu3)TUgu|2u+AR0k!ct1LA zTKP}G=73{MoHF|G_8leFao8&ezZ8)nF?Ic|d()Yi!B#8hHP9u!MYOs(K~r{V3bI~1 z=QM)&(-owO-DLM~bF;=d!vtfEuD*PNQk%MU8&63|&pvLm@AeoF^p4hp{i4mg!qz4i z1nbs>zMdd4JGf*;2Gdm?G>a{A=E9sBe|jA5pWwM&1j3hUFvL|A6tg_V=EKHEU0`Za z|G!LcmoCf4{<3__h_MTWOX?;gHq;l^Tl$i+cQ-hIEL*}IHjC-#Wz^){9q}peR^(qc zyqRGi%aM6d3ml2_CuaWjKzY1H++q3EDcr(y&uJCZdyK?P1(||fk^{PEHrySLQ;qj` zHZ}x`s}|dQ%o@3GAW$u#w(i*Jv7hRp&HGnQE?pNF@E676nR8MaE(kygDHl2V=ZoqE zao=DQigex?n`IXn`&vUik+I3m+l9DZ%XzF9MCQdAaDZqcm92^||5Fy`dsnQZS(6+Xu@GUBE|MsrEEE1g=Du@?*?E+MVc5jK& zU9Cz$@ZQUM0{zZcoq}rFuK3rabt%l=2|lj7@&DgQUZsmTRt1V(tm~0=oeg22GMfFD z?{ULwIUcbDCOQVnAgB*Bk(1piN^hEX7`&H(;r@emqtJw+1uBU@7TYH?t_$#fQ{}|b{ zEI;cp^GS)jzxU<=2)&`<2YzZpo9}H66~<2YG}-OX4T*Bi2o8CxpZ|5vu93R4Rs8wW@8o>iCCL(i~ytISkw`gTV@*-SRqpee1)l->8r zk-JVdM7o{8x1`;}%=RWhKHtIC&)Cwy(Ty!k%?gEd>P?sb<^jF>tKnCx89~}MW4rah zF|Ur&pfo}H4kOF{MX#Nm-)Y8h_a(s|ADa*I^~xnh+fzG>Sg3h)TPGT@@!?`^X0N%S z(`n6cH^R+MBV*SWc4|~2oO?pMm4NTbfM=ok436&Jf;*i|jlV23z9mbo}`flr>WPuaiX>v$4gVRGtgOAR z|Lzd%^`!CYj6RE~C!2u#$Xqj@`J1IwZguH?u1>Z1UnTULo!&NP_t(u5bb_=Vdq;@t zt7h*L4*V>ZgX`AaBPTzvmw}L|*&LPw{>YiF+}Sqs(}izPymMP;<;S?&(|!yDOvink z(>7FYJhKy>ndeOczwn6~uvBZ8ldIuH^BB2Y&sgcNnJ4%TUJUviChk{Qb)4@poYI*2 zWY0QVc#4{h4|Hdk^>6!+phT-0$4KX1C^QKLhGh#vj;CT!2B>eAWz$s zCFr?aFt3Z{+0%|}_S@{e?>BIbJnphn^I$u!{!yU7m{hs!r(Xu20e-wTB!AA2L9WMu| zJNo?Y&+dr#>+E3&ZBm7luwF}#k45CtZ%MNxx6juzM*hxm`3HZ$%-k$*pDv8Q6YXst z0+Y_Uy-o-XyR#3t>aRjyOQjT^r(XUyzxjK&SzcScUutb$Y1MCSm7N!NGt)7942@#x z&k$~OIlSv8G44{P=Yg%pedUKC8RWP>1;(R|U*2BzB%wAt#zQ_hY z=Vxsn<|-q@U+HeJGYTOnxr;gG(tSX-JuM4| zoPFuZ<>wC5g9`>0S(p(FDjrr?TPTgl7Om*B+pcJQW?o5E$%RBoLvEEx=$Kg9m;tj{ zwYqBg82i9wXuH$+L~mVRb;DUzOCwvB{uvXx9qPu=V<&IGQrWtH3MMq%PKqJP=rcwWfmk-tv94 z=bAoLM@#hyR;`z52Ze=q%?O`e0U@1Ncw0hZ-c8mRP&FQLP4wmpW!%IUsy%E03;Nh1 z`d-8m+nV%IJa>@1PcUJf2wn!8zDbxJ*FTPVBS$RpSFw_Kt>Q{MZvj{Aq>MS*5Lb7A zlCDKL!PA&8icy5@671M5f|;=)9#1|ccEUGe$}88UqlJ_|DYl%q{+0Hz&2a()v07vBC#kqE)#Cs41yRcZGI? zHVN3bq6ODiCr7eU4Vbu6@fga`w=VVGUCcNmmjoM$71=R#V+WUN&&=4&;H=}7h`-w( zQ#^;e%v=cpQN{*cKj>#e zJWQkm<2LRpF8%FZ%9)E#r)sL`u!U3143ai3jGqZOg2~VqSWGJQiHvr0z5$7udFKPI zx>9#ls@p0Tj2gSTbPbd-kXuQgvkHS3gfKQsqT%i@S;URLt-S|4_M^P6DEwKI;|V%p zMWDc>hx#Omk1@zLd(FiHVY)OR9WRdtW>W1|4F(>qHdc>VzK4=VWV&pG5*uAC7OcEG zKoPgU(3OUcm^4|SIJG^$n4vRIi$NlDbPnkEl->Mov59{HPJ&67uNgH zSOP3YEF7c9!9fP^<3YnGk>7yg8j@H(usgP^=>tg1oNqBn93rKconX@%u^S;6yPA1W zU}ZOcbo_T4LqEGpZ2b8t_O=^#RH!4KlM&H^t)&mp1%E24#ZjR%m_#0WlrTf*70{P~ zox(DWBXX?#NBo+SHiu|lF{?Y>a0Y=T^*o67PVIl4F*Oc5xRoH-e6@DS+@z@stO<&sU5L3 zZkDF6iQ&&6G)hE-8qIFQMh4;rq#Ycxz6BXy?4pt8BI~X&VmY`{Ax;U+0CTMsr zHLHn&C3t$+RFQF~n)ZuG``s9(b%(b499xGR`4hwIBMZgjtgH6r&-6Q$M$*(}9-Z>1 zVF#k@eM-x~$z+YYGQ!)V9h+!JkJ-X>Qde2w2;}=O!x?AJc%SV@QamH-`zqO&JovY7 zzC*@hQ|b5)38e33Rp&jKsA)>TgOI#k7E9*LO;{;s>CT26)I9dBxm`A4#t*Dxb{Gx5rL;w`imIIf_MNvl> z&6h=|xF*jiTL#@ftC*oqTpULCXhfd-Pd9T+Mq?9tV$};YvD3a4e*`Zo0%AnDt_c3kT|ei*b(NEnC8G{43Zs#Z&PS9aa z2e8)!reO-)j1GZAcSe^z7fkZAjz?It;feWF^SHEOYQ%0Us}1~((JWn6@EP&!*^Yl~P2)_& zvAx13)sa})FEM( zd(jE^!rQTGugyq3&KJ!IJtd9e+POQeD+x1v@o5+kYr?IlGf;G-8~ z$5z8!Gv2?Mgc3>;Djhr(5W9`M9%%NJ_hFpsAt>gK8VV(N&|(sm)q+x_Xx)gSmL&_j zj60CY=NrS0`_YYUIYqz`!sUO4-tOw$a<43NKdvm3w&fZfFZq)VDjl`Ks80sK-NR4u zUcv}5i`})FQS?V;Rp<4I`)vst(1oLsUx$+*EXyU1v#~$+lOH`tqQ{jlt^^5*W5gG0 z8^ISFQz)pwjq^NS>tP{>PSIO1zk28@6M(Nty7!hZu4Y@f$`rXF{^E766bwFEOLMR` z;~e=i8vRQ#@@VPFRqK|wL~z7S1Wsnn#fw5Ue*&bSddNDy08t>Xp`wg*U21Q#9JGer zcUeA?VKf$ADU0e8zfSru;_ooH2oiCEMJ_R=H~}G+(fJ^WzvC#o+@2Rt%JR8f)-E?+ zEEufi<_!GiUl7&Oxqm;y_&FBB^JZAhzgr&qWwJ>z>IFH-TO=!%JCGIf)DUlvi;i1W z>)UCVH-^=`N!CZyc+8!3^Yz&}%jfLJEUH(vA8)HxogM3`S6xT<5K?Wfkm=A5S(*_zuX2rypqQjrh)|g zN5-QtvK~2E)=!ahc?}*yu~-rSVyzPLk;fQRp|X57$GIF*A8n)sRMzt>njdi(c45GQ zN|+dSVOkeXZ9y#iaGPbIV*Xm_T?V-;!^_oC-Qh6Y6JqsdrLvf*M6Gk(J0Fw@&~9U@ zi)0GIfrM%+pNqhO+irWm%awhriRq7lR^RG!=!1hh&0YZ?!L$o4+7}mu79+GPiiX~P z`%@|+9|8Q;RrXz~=5gZTyUG=B@J40~TZm zZoC*0;qsPM8Jt1}Q9Ku~*`U=?)~Px3j1}cVE*iw7TvYs=L1U_(tSxAUVIIl|ww}zbIuLPq&S-h#`zt zUGg?C3@(6w|HUBQ&k5O-dX>^yo$V-LvW1d9q_wL&ivOBH^RIF+-VOzPrBoo6r4yy3 ziB0wP@Fnz8v{?S2GF_4K=T}+BEx z?~|>)R~Dw^R8z~2@8>wTCCs5((UF{<&vcpcDP3+q)#q*97>B;>;jQSc=7@7bh_(m1 zr?N~@xnEkt=0l?IC7Hzi{6_|5IpuYHe5+g9ev~)v#-O8ar9WVKi<26do(k_0W@~+)KJ@6a?6&2vaj^Xs^@f4X7irifc2A^>MA1 zU~<8Z)?JCDm7Jcnz&9zN9kI$T>KPlB3YN`U5a9uNx{>@in3>Tl9m6AY z7uEcN{Ek{U$U76CiIjDl+xKTbVTeUbC)Yr7i5+YRrsEmtV0chc z^;Hq8e6NdFd|ay3s`T*PcqBjQbn zQ|$CCNmL&t31edIR#ipDMz;mMy}ZU{D2>Y|bak_y?U7kCbR-G5^ZNEXXFcpt()=!XKO%w*Yo2fHeNAJ-j4}#r z)TOon3-u<+ZmTM+&m3GX%>TSH;<60_a$zwyG4DP&+|i35$SlBMB4kFc5M$o;N)X6+ zSCgtpOiQYEN$nTsOogRNq-{Hka&q7MkL)0Rq*qc^aIrjoS;PhYdRtrA8-sI z?YVI)De;538=oBBQ7S3DMSMMas&ar=~iAnD?SrI!=^}y zb8MlXOBu?5N!XP*Bj*09oz!~VuZ<*urIV)F;gGpU%$ms(nEp=G0$m`%d>JqI3ik8u zQCmE*ZKCjvn)}~g$^pvLM{6#=7+25df0$E$T_)^+HTOK5?1mr5XsFDK*`r2T40Hb) zEIW?T+dKu7IW@_epMg&?hn-xS3YVxCIEKQ*8Y@b!#B8|*5R^7Z&-bYqNZgh1O^Cst z7AiyyfI=4{@NfQ0m%pT(A>&%?i0)s!i4}tV8)59PgX@Lg$Mz-sTf5D(KT{Zb{x0kY zuf)Gho9KcL{1Gvk{vAs?6!q(=I^Tll#sdHyOR3bRL0=Gp*}(x4lr8hUTDbQs&zhSM zWB|ox+{aXi>r7uz|JBsYZV|ULS7z-M5g|HqtgQa%Un*Adf&@^p)U`Iz%4yxTF-i)AFD7atnY zG`~+Sx_$*~5n=rNzMBNQa-IH);kN819fj+HF<9d6N~~J~f?{6BX` zhPbY|)339B%V3hQ{P>%46o`q+vq4oNwr*C5SryKHaQ{oQ2koT*a|Hdp{VqDr+t=Pq zJ<&^Aiyk*B{(8#b*dYrWM;r1(Eu^NhnScr}>=I*qPWL;-Zr*8S!;D&T_PQMI7ex>Yn7Jb@9a@p&+qyK7)Do z{rgIgDUv;;x4qj&ew3^)gxAcv5o~LJf1V5P4CR>b_^sJClJ?pjxAnC%?^Auh0Op$$ zpOMeEQh)tsoDjBuR1WrLa?>{qO}v`cX7PbqjSS`=5%VWGwesZWsA@J48femsuHiIt zCl#(asE@1ERv~*Ac}zu3%~L@eDsa|?wjvED6zC^~pQ>{{PSjW=(!{!_U++w2PnYH2e zNS8lM=(zyH7p2c}VALfj`01uXuu;}mHtLd*e(=5n;#YpoOhqd2g7`IUobt1%^J&ke z+BufFWwtyCxH=36!u(!uZMQy_@82nm--1cB`=mumNH7X(rIoeQs;jR%xb}uuQqG6z zJ4wig_NK&;#cU%chN4gSu^Dw{AC`LNS`4J73m5JUS`?poR$Cgk zO~t(XMKI8y#iCw?vlH=SiB>VcG)Z+M=P9Xe47ZZV9H;jC^IMTi_3Y@%b&4|>lQ0`S zW|!}zxJ0K%R~J@x2(u3P)Ttm~ozR9;a~e{))fYFZAb$kPBqJt35{qFJ`xvD18&7Bv z=Zgwld4i`#(nMrO#Y+$$yaAX((~^6~db;uu9g%*rN5Lxd$Jd=oUC330HuCrNB{tB; zE9X0NoG~8p-DniZ9I8Byh0R#adi_U?R%1CeeKA^pqCIbvQsftEId>pu3EG}q(Ck2! zztcu$MGsFLVeBZQ?!k*N<8c1M_!~*68J9TXD&T&Q~FoRXZbxYJccb{c1sI$zOs+B!+hp&aoS_~$FH0WX%Fl3UKlHTel#6h zou5m8Qw@dirRvM{X9L%T@<6J+wT?nNafu*Ya8*?o@{2IvNMzks3G%^cM6I){JaxGn zP;+qVuM3JB&V_m}HN9+T#qdp#p1_(e;J2L`?WN0iJuCs?$!QgTay9pe2`u5@(Yb%5 z?NBi(FNBbh;E5D=)xk%^2f^p|41HwW{H zZJn#pJbuG%FmH4xd=!L$|OagyeL1d}3&$s`Fr@g&8KJr#{TF`Y`FONWY_-9&iO?iX7zb zwhCgUj}8tWjDb(Nm;h8Hx>B6CWU?gvYfc=BO<7L-5FzDjq$qMyLc^3}qwwHnoaLi& z_`|8l* z(6mCpVw#H*1b^5`Oj?K1sWT2`oY9c|xv{)uCuowTew|u&v+VMHHvL%njAMCUc{mRK zsj0zNjv?>fjssR?W#<#cFy4FFb+QP>&iVz2uYMBSUqJ1S01|YOzh&~jTNo8ZngHfZ zBee^PWnxKjxk);3taTB^uC_|!1ph?(ZF@Mxq}`ef7#q#gQ9yhtzpW9@ScMmItgq?Ui~^83%8OpeOKh?5Rod zFh+N^TbK!~)hPJ&M;?YNOU+U~qJH3(pmqgw3X+ex>n~2(@|im!YMHO7-=-?6^DF(+UPt~bQ$dP zjLfs_mDsLwon9Ioa{ZyMN%^07BiMwc`6DH!HNM_0LkyUp!gpy0XzzP>o%e^UbH9Wa zDd}r7u>tZ_LFkvKMBD+0&mUn%U|e}5I?oEb5&qt1jDFc22a04G)Y8ZFq#{3lVK2^{ zi9QmTc1#nh5~}l50-)}DN>EwuPBKf{gLmKem5%ocvMB;sG=Cb7%^oz@&X~ywi{b)CV?$ET|tPF zz^3o*wa%czuiyP{td=HViGnD#xiUvJsBjn~gk0efQFm%P?D6!?0*k)KUgeJ!e#iRU zGxE>-K7w)$ivP!`qEf2t=U3>#{zx-oBV@Yn0X0#>Rtxf0sRco-Q4_>Al)IXexgst2 z8RDH&R!APr`N){M9eUS$B-LOW!^Q0eh>o_|89^7q#9cHSfkvh_auxBAivmUFl}Yfv zoEBCn1Fffwf*+x2g(9040F+b{{K=h&EmB|y%eI}eh-g;7Xai0{i|Gj34vEHW|Ba$5 zWP0AV>=L9$4w0mhLdCLIZtTooSAm)Tk<+-m1LHVuU2~865#0j{bLdOl2<*T}fN6d7 zRzO>TxKuUo;zxKI8C6}4Jx`V6!drV|m>&PV?jo3F4aOagCLT$ie4bAfD_2_;k9zci z8Smm3fN!0f074Q@46R!SZuIjg9bjgs`V*OkO7T^pvUUvC)a2eQE}{zo2dUf1g(~D+ z1?zrJCU)W(uo5i1_)IzLCnhZI4If8Avw~n&Ly$ETmH$zVMJW#w+7hsX_+v95F$XTF z7wm+21UJb><)v3HKimY#59 zhx5$0D^sG(O@Mvm`2o{OWRX?^Ziu*5v_T;iQHK*4(c^wzPwC6cuDlF(ji|s3CY&xN zoHnP(&u|HPFr$+}_%Zu6Wb^nhSzhE4^Y-9R$MS_|=?tcYVATj57Fb4q3gvJU03MzL zs2bYcmg;r+df6?pq0_^KLA`&VrmU)#47nq~Ffi`GSX=mxEe>vXsb zYaD*&*3;}_NypTK5KW8-ohdB#h23B}CfrdY#t;ob_ODH8U?8oeLn`6Q_P15sx92_p zWf6X9v+~-MJ#sh3{q!UFKON?F*>y>Ovdq{&-Ii*l)^uf$KQ@>gtiBhU*w1UEYQ=rp zvzX!4aI_r7UhUiqfu%x8xkr?yT0lmDYj3=c7Vp{W9&h6dCp)qNw4|*tAo`3wus4={v($GOy&k``mo`VB4}K=u>3zw! zM%AwujIaG|)UDd<*?#b>5(D_1%!D;qrr~TJY)iM9kgba69k{)Pgvc=NlIxne$um1| zmXblE$k;TKq$1}a-t&pIy+)ZTq*~mFgu{1g{GbvW4qUxPSvram4=%i@MgGf|6)prtZrHvEqq z)X(vM$D3%&emvD((5f=e_^%NXSWABY&VF+(_XO6rjtQZv5ByZ^re9w6v=v$`m)V*) z=9!8P4E(qT)FHR!Ko0RH!|W%k=g4jxs%iBmN3rN$Bz*#XE~vZ|mtRRMucwwYFvyzf zXH4|4Ci_}a({!2q+gmaA#$!*3ry!=`R&`Pitf{T9B}!usT~xVJ@tL3`Km=xa{Y)0; z6G2nIMAy>w@{!iKi^bpvmwvB2b^+=}xZ|!lK`X4;zmh_~EsiAR(9lCv1eU^=4BKS7 zb=aR)0$Mhn6R$Nin4{};#;1zNv!atIvQ&bKLNR?mNMM(=V2EMixTFr$t;6!vcqjnrY@qH8y zgr88hvt@^)@gPUj_h#($#!AB_1=SAS!Eu9QoSUjNBFQ>W9$#>H2hy|e$FMbAc!|K9~w zzUOYk7kQms+78@iuGYu*d($s!bd@o{`#06D!MMF|Roi4c6S2w#^={*dC@Ia3S$4I z%4!g*h3DsU9d$u^$##^K_MeJ40~jP66+g)F-R%iNf&0I!F)xM+I}cld8zB**`AmJs zD^<>Z%hStHHck?&+z+T)i6wmaWVsJ>C$SGDA+`HG5=@Gy+NpGCSV_yHK~6%pMMLq@ zN$Wzna>h6301H8sz{n99c#6Osb7)G3R)eRYD8aJPY3bs}8;NfYK{@4p-A&wI8cZGo8a(&8y=P~K0D2rz54A&F7h z99;55RT~5s*9Y2bUWrcJzX$mo&V1ym%frZr6h7iZlKwn?ondUIFH|$(keigSOP#m2 zRKA$lK$9-3sMM!8qYUfP+rTUA;2w`hFM5%%P}`lp=+yM(sJu0M-mdD&Ty}5#u=`)| zl|y8=yZ=3_DRbHLKd=5v$g|-aYWz*X|AAqr!_Lmk_qPGc^s3-Ro$a(LF6Te@5?@c= z6L!`p$%*DWnD~&={-{hScdwDd1c!?&PGo~i!E@O1zu>L}Ugt)O^9Z&orvPmMnZ~}Nc9Pvn`i8FuHDAi8MCeCQy4@uf+WMac zW+70+`EdEGH&~SVW=v^iOAYRX;CjsM>jBo0nB@ede{UM5Z8b7(Q6*$KYK7XY48hF4 zAW<{Ino?0^=rDhra~+vOG*q}qyd076Tu(q(p@=@=HH! zS?zOJ9u5p?aZkfZ?KZx@J^|(W-BsHKMJ!!jjeq@i^pdO;6&UGo4jDr(C^j1YJDv_F zV+|WSlLEV?MLoiv?DNUV^Y`kWhGf%?M9>G*@)Ha2;%d04<1W>$&TtM|W{UHNGsz*` z)4%kC@ep+|IZm|8$Z~8GL}M}d2Nrb~Mg#ZBs6~~JrO8=l%>GCtHYq$rQZVC8OSTgn zbs5R1N_V1m6q*;y-;WB3|u?=>!_d2|-<<#^9(p~D{o7tK>;V7*MGJi zT=Z2>4#pY(*Hfw_%g1Ah$4JE_?Z!XZxBz2HzIax7F7jdi1czejOs-#nUNGD8^jsrS zHlSAgK0cFypsmFoBNxgwmc6JVE!l((O>l&JH1(KDjLn8NEKyc+g=Y2Yaw z;<6en->kJP)ffrPmd`wz)rr0eY<5+zM?bN3EBY6>E<5;s8%jA23t(V@3&h>?=Eo98 zlFeeg0_~`UiYjXZZB)_+X@fz+_GEJ1Tal+M9DG?jyiu<@aJ5OwOFlkyw%2e2ShJCC zeT3?YKIc^4<<4x~*@1h1gEErJco{W25W&6*P`RyVfRSXW??A6?q9u7*GJUc5cr*jD ztNP^B3@B*o64BPip|4E9+!}!Y2fl;Ltjbd`E~WmD#nw0Gxv&^Q`a1tJ1VgY#6rp{p z-y?~I@$=e<-(wX<@^7F!vZ3EZvmN$xzH??zr(f=sZ%3_QrW2}|d9wL)%~{$Z7wJ1G zz{3-mjn_XDYhW&3*GRIqj&yYe>E;IdKk)tE^DD>~y1)LrtzVt(tD}Ql9Kj~A^zuC} zdD~QD2|4h4{!^EkZNhI9?$GCFD++7|XbGd1Ll@=f(dh@A8GH5}8Lr~4nu~c5jpY5U z*dJ>MOM_Q|???a=j4S}qc+J6doDgG*g{1nytE&-^`XGFXu3qpD$^eK$h^Ssy3px~5 z;w75IQxa+IAYv~Z1vx4SurcE0z)^s3&r#XjOk#v>ghr4fW)3Dly#?G@Z8-#Ei4DOT z!e?l8l7llS5bDR~&WX+G#uj`7g9$W?)L%%9RNYrsimH=%A6l(T>IMQpf8@F%V!5oV z=aY)C#`4-H(w%CE|5X>Ii;)haNhS!QO0qeXp;OTpJOuJ)4g5F2WcDOMMaCS4k93I+ gOL1Aax9UaZ!D`=MU^(z#iZZ^=UHKshJ#O{+KM(Cy0ssI2 literal 0 HcmV?d00001 From 7260353f05dff62b91511016db2c2e55a991104e Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Wed, 27 Sep 2023 15:51:11 +0200 Subject: [PATCH 15/53] review: Fix https://github.com/entur/lamassu link --- doc-templates/sandbox/VehicleRentalServiceDirectory.md | 2 +- docs/sandbox/VehicleRentalServiceDirectory.md | 2 +- .../VehicleRentalServiceDirectoryFetcher.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc-templates/sandbox/VehicleRentalServiceDirectory.md b/doc-templates/sandbox/VehicleRentalServiceDirectory.md index 646670150bd..4d1de22b687 100644 --- a/doc-templates/sandbox/VehicleRentalServiceDirectory.md +++ b/doc-templates/sandbox/VehicleRentalServiceDirectory.md @@ -1,7 +1,7 @@ # Vehicle Rental Service Directory API support. This adds support for the GBFS service directory endpoint component located -at https://github.com/entur/lahmu. OTP use the service directory to lookup and connect to all GBFS +at https://github.com/entur/lamassu. OTP use the service directory to lookup and connect to all GBFS endpoints registered in the directory. This simplify the management of the GBFS endpoints, since multiple services/components like OTP can connect to the directory and get the necessary configuration from it. diff --git a/docs/sandbox/VehicleRentalServiceDirectory.md b/docs/sandbox/VehicleRentalServiceDirectory.md index 29a45ebb062..5ada1522d7b 100644 --- a/docs/sandbox/VehicleRentalServiceDirectory.md +++ b/docs/sandbox/VehicleRentalServiceDirectory.md @@ -1,7 +1,7 @@ # Vehicle Rental Service Directory API support. This adds support for the GBFS service directory endpoint component located -at https://github.com/entur/lahmu. OTP use the service directory to lookup and connect to all GBFS +at https://github.com/entur/lamassu. OTP use the service directory to lookup and connect to all GBFS endpoints registered in the directory. This simplify the management of the GBFS endpoints, since multiple services/components like OTP can connect to the directory and get the necessary configuration from it. diff --git a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/VehicleRentalServiceDirectoryFetcher.java b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/VehicleRentalServiceDirectoryFetcher.java index 508b7f2809b..36fff9af0a6 100644 --- a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/VehicleRentalServiceDirectoryFetcher.java +++ b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/VehicleRentalServiceDirectoryFetcher.java @@ -24,7 +24,7 @@ /** * Fetches GBFS endpoints from the micromobility aggregation service located at - * https://github.com/entur/lahmu, which is an API for aggregating GBFS endpoints. + * https://github.com/entur/lamassu, which is an API for aggregating GBFS endpoints. */ public class VehicleRentalServiceDirectoryFetcher { From 38886d303d4f6e43727193900038ca267bfe52e6 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Wed, 27 Sep 2023 16:08:07 +0200 Subject: [PATCH 16/53] review: Make return type optional --- .../VehicleRentalServiceDirectoryFetcher.java | 9 +++++---- .../VehicleRentalServiceDirectoryFetcherParameters.java | 6 +++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/VehicleRentalServiceDirectoryFetcher.java b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/VehicleRentalServiceDirectoryFetcher.java index 36fff9af0a6..d51d0d70b6c 100644 --- a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/VehicleRentalServiceDirectoryFetcher.java +++ b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/VehicleRentalServiceDirectoryFetcher.java @@ -99,9 +99,8 @@ private static List buildListOfNetworksFr var networkName = network.get(); var config = parameters.networkParameters(networkName); - if (config == null) { - LOG.warn("Network not configured in OTP: {}", networkName); - } else { + if (config.isPresent()) { + var networkParams = config.get(); dataSources.add( new GbfsVehicleRentalDataSourceParameters( updaterUrl.get(), @@ -110,11 +109,13 @@ private static List buildListOfNetworksFr false, parameters.getHeaders(), networkName, - config.geofencingZones(), + networkParams.geofencingZones(), // overloadingAllowed - not part of GBFS, not supported here false ) ); + } else { + LOG.warn("Network not configured in OTP: {}", networkName); } } } 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 abd2b7d8afc..9b67114b499 100644 --- a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/VehicleRentalServiceDirectoryFetcherParameters.java +++ b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/VehicleRentalServiceDirectoryFetcherParameters.java @@ -3,6 +3,7 @@ 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; @@ -96,8 +97,7 @@ public String getLanguage() { return language; } - @Nullable - public NetworkParameters networkParameters(String network) { - return parametersForNetwork.getOrDefault(network, defaultNetwork); + public Optional networkParameters(String network) { + return Optional.ofNullable(parametersForNetwork.getOrDefault(network, defaultNetwork)); } } From dc1d50f709848c2261fe2772570d4038c2399703 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Wed, 27 Sep 2023 16:10:34 +0200 Subject: [PATCH 17/53] Apply suggestions from code review Co-authored-by: Leonard Ehrenfried --- doc-templates/sandbox/VehicleRentalServiceDirectory.md | 2 +- .../vehiclerentalservicedirectory/api/NetworkParameters.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc-templates/sandbox/VehicleRentalServiceDirectory.md b/doc-templates/sandbox/VehicleRentalServiceDirectory.md index 4d1de22b687..8cd82ed0ed3 100644 --- a/doc-templates/sandbox/VehicleRentalServiceDirectory.md +++ b/doc-templates/sandbox/VehicleRentalServiceDirectory.md @@ -2,7 +2,7 @@ This adds support for the GBFS service directory endpoint component located at https://github.com/entur/lamassu. OTP use the service directory to lookup and connect to all GBFS -endpoints registered in the directory. This simplify the management of the GBFS endpoints, since +endpoints registered in the directory. This simplifies the management of the GBFS endpoints, since multiple services/components like OTP can connect to the directory and get the necessary configuration from it. diff --git a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/NetworkParameters.java b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/NetworkParameters.java index 2f4ae49de62..25d394f72ec 100644 --- a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/NetworkParameters.java +++ b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/NetworkParameters.java @@ -7,7 +7,7 @@ *

* The {@link GbfsVehicleRentalDataSourceParameters} support {@code overloadingAllowed} and * {@code allowKeepingRentedVehicleAtDestination} is not included here since they are not part of - * the GBFS specification. I there is a demand for these, they can be added. + * 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 From 97251d52195a8a6576b552eb26e99d28f665ff5a Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Wed, 27 Sep 2023 18:06:19 +0200 Subject: [PATCH 18/53] Remove unused method --- .../openstreetmap/model/OSMRelationMember.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/main/java/org/opentripplanner/openstreetmap/model/OSMRelationMember.java b/src/main/java/org/opentripplanner/openstreetmap/model/OSMRelationMember.java index f8cf363ad65..e2ad2ff9691 100644 --- a/src/main/java/org/opentripplanner/openstreetmap/model/OSMRelationMember.java +++ b/src/main/java/org/opentripplanner/openstreetmap/model/OSMRelationMember.java @@ -50,10 +50,6 @@ public boolean hasTypeWay() { return type == WAY; } - public String url() { - return "https://www.openstreetmap.org/%s/%s".formatted(type.toString().toLowerCase(), ref); - } - @Override public String toString() { return "osm rel " + type + ":" + role + ":" + ref; From 0fa491fc16440fd91a4b41c18cb3be28bbd2865e Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 28 Sep 2023 19:26:16 +0200 Subject: [PATCH 19/53] doc: Fix spelling and add changelog item. --- .../sandbox/VehicleRentalServiceDirectory.md | 5 +++-- docs/sandbox/VehicleRentalServiceDirectory.md | 15 ++++++++------- ...ehicleRentalServiceDirectoryFetcherConfig.java | 8 ++++---- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/doc-templates/sandbox/VehicleRentalServiceDirectory.md b/doc-templates/sandbox/VehicleRentalServiceDirectory.md index 8cd82ed0ed3..d9908de1a90 100644 --- a/doc-templates/sandbox/VehicleRentalServiceDirectory.md +++ b/doc-templates/sandbox/VehicleRentalServiceDirectory.md @@ -1,7 +1,7 @@ # Vehicle Rental Service Directory API support. -This adds support for the GBFS service directory endpoint component located -at https://github.com/entur/lamassu. OTP use the service directory to lookup and connect to all GBFS +This adds support for the GBFS service directory endpoint component located at +https://github.com/entur/lamassu. OTP uses the service directory to lookup and connect to all GBFS endpoints registered in the directory. This simplifies the management of the GBFS endpoints, since multiple services/components like OTP can connect to the directory and get the necessary configuration from it. @@ -16,6 +16,7 @@ configuration from it. - Initial implementation of bike share updater API support - Make json tag names configurable [#3447](https://github.com/opentripplanner/OpenTripPlanner/pull/3447) +- Enable GBFS geofencing with VehicleRentalServiceDirectory [#5324](https://github.com/opentripplanner/OpenTripPlanner/pull/5324) ## Configuration diff --git a/docs/sandbox/VehicleRentalServiceDirectory.md b/docs/sandbox/VehicleRentalServiceDirectory.md index 5ada1522d7b..340a6715a48 100644 --- a/docs/sandbox/VehicleRentalServiceDirectory.md +++ b/docs/sandbox/VehicleRentalServiceDirectory.md @@ -1,8 +1,8 @@ # Vehicle Rental Service Directory API support. -This adds support for the GBFS service directory endpoint component located -at https://github.com/entur/lamassu. OTP use the service directory to lookup and connect to all GBFS -endpoints registered in the directory. This simplify the management of the GBFS endpoints, since +This adds support for the GBFS service directory endpoint component located at +https://github.com/entur/lamassu. OTP uses the service directory to lookup and connect to all GBFS +endpoints registered in the directory. This simplifies the management of the GBFS endpoints, since multiple services/components like OTP can connect to the directory and get the necessary configuration from it. @@ -16,6 +16,7 @@ configuration from it. - Initial implementation of bike share updater API support - Make json tag names configurable [#3447](https://github.com/opentripplanner/OpenTripPlanner/pull/3447) +- Enable GBFS geofencing with VehicleRentalServiceDirectory [#5324](https://github.com/opentripplanner/OpenTripPlanner/pull/5324) ## Configuration @@ -62,10 +63,10 @@ HTTP headers to add to the request. Any header key, value can be inserted. List all networks to include. Use "network": "default-network" to set defaults. -If no default network exist only the listed networks are used. Configure the a network -with name "default-network" to include all unlisted networks. If not present, all -unlisted networks is dropped. Note! The values int the "default-network" is not used to -set missing field values in networks listed. +If no default network exist only the listed networks are used. Configure the a network with +name "default-network" to include all unlisted networks. If not present, all unlisted +networks is dropped. Note! The values in the "default-network" are not used to set +missing field values in networks listed. 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 1576c0df560..48693e50b6d 100644 --- a/src/main/java/org/opentripplanner/standalone/config/sandbox/VehicleRentalServiceDirectoryFetcherConfig.java +++ b/src/main/java/org/opentripplanner/standalone/config/sandbox/VehicleRentalServiceDirectoryFetcherConfig.java @@ -63,10 +63,10 @@ private static List mapNetworkParameters( ) .description( """ - If no default network exist only the listed networks are used. Configure the a network - with name "{{default-network}}" to include all unlisted networks. If not present, all - unlisted networks is dropped. Note! The values int the "{{default-network}}" is not used to - set missing field values in networks listed. + If no default network exist only the listed networks are used. Configure the a network with + name "{{default-network}}" to include all unlisted networks. If not present, all unlisted + networks is 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 From 37a0d804ed35ea6e4a17327021a7adfd15038381 Mon Sep 17 00:00:00 2001 From: Vincent Paturet Date: Fri, 29 Sep 2023 15:07:15 +0200 Subject: [PATCH 20/53] Set maxSearchWindow in RouteRequest --- .../routing/api/request/RouteRequest.java | 14 ++++++++++++-- .../standalone/config/RouterConfig.java | 1 + .../routing/core/RouteRequestTest.java | 6 +++--- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java b/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java index f7beda526ac..22cbdafe2cc 100644 --- a/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java +++ b/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java @@ -56,6 +56,8 @@ public class RouteRequest implements Cloneable, Serializable { private Instant dateTime = Instant.now(); + private Duration maxSearchWindow = Duration.ZERO; + private Duration searchWindow; private PageCursor pageCursor; @@ -321,8 +323,8 @@ public Duration searchWindow() { public void setSearchWindow(Duration searchWindow) { if (searchWindow != null) { - if (searchWindow.toSeconds() > TimeUtils.ONE_DAY_SECONDS) { - throw new IllegalArgumentException("The search window cannot exceed 24 hours"); + if (hasMaxSearchWindow() && searchWindow.toSeconds() > maxSearchWindow.toSeconds()) { + throw new IllegalArgumentException("The search window cannot exceed " + maxSearchWindow); } if (searchWindow.isNegative()) { throw new IllegalArgumentException("The search window must be a positive duration"); @@ -331,6 +333,14 @@ public void setSearchWindow(Duration searchWindow) { this.searchWindow = searchWindow; } + private boolean hasMaxSearchWindow() { + return !Duration.ZERO.equals(maxSearchWindow); + } + + public void setMaxSearchWindow(Duration maxSearchWindow) { + this.maxSearchWindow = maxSearchWindow; + } + public Locale locale() { return locale; } diff --git a/src/main/java/org/opentripplanner/standalone/config/RouterConfig.java b/src/main/java/org/opentripplanner/standalone/config/RouterConfig.java index d70fb851e32..ae92486037d 100644 --- a/src/main/java/org/opentripplanner/standalone/config/RouterConfig.java +++ b/src/main/java/org/opentripplanner/standalone/config/RouterConfig.java @@ -69,6 +69,7 @@ public RouterConfig(JsonNode node, String source, boolean logUnusedParams) { this.transmodelApi = new TransmodelAPIConfig("transmodelApi", root); this.routingRequestDefaults = mapDefaultRouteRequest("routingDefaults", root); this.transitConfig = new TransitRoutingConfig("transit", root, routingRequestDefaults); + this.routingRequestDefaults.setMaxSearchWindow(transitConfig.maxSearchWindow()); this.updatersParameters = new UpdatersConfig(root); this.rideHailingConfig = new RideHailingServicesConfig(root); this.vectorTileLayers = VectorTileConfig.mapVectorTilesParameters(root, "vectorTileLayers"); diff --git a/src/test/java/org/opentripplanner/routing/core/RouteRequestTest.java b/src/test/java/org/opentripplanner/routing/core/RouteRequestTest.java index 1f3f7bc06d5..b18c12e0485 100644 --- a/src/test/java/org/opentripplanner/routing/core/RouteRequestTest.java +++ b/src/test/java/org/opentripplanner/routing/core/RouteRequestTest.java @@ -18,9 +18,8 @@ class RouteRequestTest { - private static final Duration DURATION_24_HOURS_AND_ONE_MINUTE = Duration - .ofHours(24) - .plusMinutes(1); + private static final Duration DURATION_24_HOURS = Duration.ofHours(24); + private static final Duration DURATION_24_HOURS_AND_ONE_MINUTE = DURATION_24_HOURS.plusMinutes(1); private static final Duration DURATION_ZERO = Duration.ofMinutes(0); private static final Duration DURATION_ONE_MINUTE = Duration.ofMinutes(1); private static final Duration DURATION_MINUS_ONE_MINUTE = DURATION_ONE_MINUTE.negated(); @@ -134,6 +133,7 @@ void testZeroSearchWindow() { @Test void testTooLongSearchWindow() { RouteRequest request = new RouteRequest(); + request.setMaxSearchWindow(DURATION_24_HOURS); assertThrows( IllegalArgumentException.class, () -> request.setSearchWindow(DURATION_24_HOURS_AND_ONE_MINUTE) From 5290a17c7d6f7d1f420887e099b7c05a74c9e12e Mon Sep 17 00:00:00 2001 From: Vincent Paturet Date: Mon, 2 Oct 2023 10:27:42 +0200 Subject: [PATCH 21/53] Add unit tests --- .../routing/api/request/RouteRequest.java | 4 ++ .../config/RouterConfigDocTest.java | 53 +++++++++++++++++++ .../standalone/config/RouterConfigTest.java | 49 ++++++----------- 3 files changed, 72 insertions(+), 34 deletions(-) create mode 100644 src/test/java/org/opentripplanner/standalone/config/RouterConfigDocTest.java diff --git a/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java b/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java index 22cbdafe2cc..35a14d32001 100644 --- a/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java +++ b/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java @@ -337,6 +337,10 @@ private boolean hasMaxSearchWindow() { return !Duration.ZERO.equals(maxSearchWindow); } + public Duration maxSearchWindow() { + return maxSearchWindow; + } + public void setMaxSearchWindow(Duration maxSearchWindow) { this.maxSearchWindow = maxSearchWindow; } diff --git a/src/test/java/org/opentripplanner/standalone/config/RouterConfigDocTest.java b/src/test/java/org/opentripplanner/standalone/config/RouterConfigDocTest.java new file mode 100644 index 00000000000..37b2e56f325 --- /dev/null +++ b/src/test/java/org/opentripplanner/standalone/config/RouterConfigDocTest.java @@ -0,0 +1,53 @@ +package org.opentripplanner.standalone.config; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.fail; +import static org.opentripplanner.standalone.config.framework.json.JsonSupport.jsonNodeForTest; +import static org.opentripplanner.standalone.config.framework.json.JsonSupport.jsonNodeFromResource; + +import org.junit.jupiter.api.Test; +import org.opentripplanner.framework.application.OtpAppException; +import org.opentripplanner.standalone.config.framework.json.NodeAdapter; + +class RouterConfigDocTest { + + private static final String SOURCE = "RouterConfigTest"; + + /** + * Test that the router-config.json example used in documentation is valid. + */ + @Test + void validateExample() { + var node = jsonNodeFromResource("standalone/config/router-config.json"); + + // Setup so we get access to the NodeAdapter + var a = new NodeAdapter(node, SOURCE); + var c = new RouterConfig(a, false); + + // Test for unused parameters + var buf = new StringBuilder(); + a.logAllWarnings(m -> buf.append("\n").append(m)); + if (!buf.isEmpty()) { + fail(buf.toString()); + } + } + + @Test + void testSemanticValidation() { + // apiProcessingTimeout must be greater then streetRoutingTimeout + var root = createNodeAdaptor( + """ + { + server: { apiProcessingTimeout : "1s" }, + routingDefaults: { streetRoutingTimeout: "17s" } + } + """ + ); + // + assertThrows(OtpAppException.class, () -> new RouterConfig(root, false)); + } + + private static NodeAdapter createNodeAdaptor(String jsonText) { + return new NodeAdapter(jsonNodeForTest(jsonText), "Test"); + } +} diff --git a/src/test/java/org/opentripplanner/standalone/config/RouterConfigTest.java b/src/test/java/org/opentripplanner/standalone/config/RouterConfigTest.java index 8cf5ebec62d..1c8f366f51e 100644 --- a/src/test/java/org/opentripplanner/standalone/config/RouterConfigTest.java +++ b/src/test/java/org/opentripplanner/standalone/config/RouterConfigTest.java @@ -1,53 +1,34 @@ package org.opentripplanner.standalone.config; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.fail; -import static org.opentripplanner.standalone.config.framework.json.JsonSupport.jsonNodeForTest; -import static org.opentripplanner.standalone.config.framework.json.JsonSupport.jsonNodeFromResource; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.opentripplanner.standalone.config.framework.json.JsonSupport.newNodeAdapterForTest; +import java.time.Duration; import org.junit.jupiter.api.Test; -import org.opentripplanner.framework.application.OtpAppException; -import org.opentripplanner.standalone.config.framework.json.NodeAdapter; class RouterConfigTest { - private static final String SOURCE = "RouterConfigTest"; - - /** - * Test that the router-config.json example used in documentation is valid. - */ @Test - void validateExample() { - var node = jsonNodeFromResource("standalone/config/router-config.json"); - - // Setup so we get access to the NodeAdapter - var a = new NodeAdapter(node, SOURCE); - var c = new RouterConfig(a, false); - - // Test for unused parameters - var buf = new StringBuilder(); - a.logAllWarnings(m -> buf.append("\n").append(m)); - if (!buf.isEmpty()) { - fail(buf.toString()); - } + void defaultMaxSearchWindowIs24Hours() { + validateMaxSearchWindow("", Duration.ofHours(24)); } @Test - void testSemanticValidation() { - // apiProcessingTimeout must be greater then streetRoutingTimeout - var root = createNodeAdaptor( + void maxSearchWindowIsOverriddenInRouterConfig() { + validateMaxSearchWindow( """ { - server: { apiProcessingTimeout : "1s" }, - routingDefaults: { streetRoutingTimeout: "17s" } + "transit": { + "maxSearchWindow": "48h" + } } - """ + """, + Duration.ofHours(48) ); - // - assertThrows(OtpAppException.class, () -> new RouterConfig(root, false)); } - private static NodeAdapter createNodeAdaptor(String jsonText) { - return new NodeAdapter(jsonNodeForTest(jsonText), "Test"); + private void validateMaxSearchWindow(String configuration, Duration expectedDuration) { + RouterConfig routerConfig = new RouterConfig(newNodeAdapterForTest(configuration), false); + assertEquals(expectedDuration, routerConfig.routingRequestDefaults().maxSearchWindow()); } } From 724ee9a360c3c4eb9461dd55a9f8f4777c885885 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Tue, 3 Oct 2023 20:42:10 +0200 Subject: [PATCH 22/53] Use resource loader --- .../graph_builder/module/osm/OsmDatabaseTest.java | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/test/java/org/opentripplanner/graph_builder/module/osm/OsmDatabaseTest.java b/src/test/java/org/opentripplanner/graph_builder/module/osm/OsmDatabaseTest.java index 4dcb91753a9..6355365df90 100644 --- a/src/test/java/org/opentripplanner/graph_builder/module/osm/OsmDatabaseTest.java +++ b/src/test/java/org/opentripplanner/graph_builder/module/osm/OsmDatabaseTest.java @@ -3,13 +3,15 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; -import java.io.File; import org.junit.jupiter.api.Test; import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore; import org.opentripplanner.openstreetmap.OsmProvider; +import org.opentripplanner.test.support.ResourceLoader; public class OsmDatabaseTest { + private static final ResourceLoader RESOURCE_LOADER = ResourceLoader.of(OsmDatabaseTest.class); + /** * The way https://www.openstreetmap.org/way/13876983 does not contain the tag lcn (local cycling network) * but because it is part of a relation that _does_, the tag is copied from the relation to the way. @@ -18,10 +20,7 @@ public class OsmDatabaseTest { @Test void bicycleRouteRelations() { var osmdb = new OsmDatabase(DataImportIssueStore.NOOP); - var provider = new OsmProvider( - new File("src/test/resources/germany/ehningen-minimal.osm.pbf"), - true - ); + var provider = new OsmProvider(RESOURCE_LOADER.file("ehningen-minimal.osm.pbf"), true); provider.readOSM(osmdb); osmdb.postLoad(); @@ -40,9 +39,7 @@ void bicycleRouteRelations() { @Test void invalidPublicTransportRelation() { var osmdb = new OsmDatabase(DataImportIssueStore.NOOP); - var url = getClass().getResource("brenner-invalid-relation-reference.osm.pbf"); - assertNotNull(url); - var file = new File(url.getFile()); + var file = RESOURCE_LOADER.file("brenner-invalid-relation-reference.osm.pbf"); var provider = new OsmProvider(file, true); provider.readOSM(osmdb); osmdb.postLoad(); From d6c9bf9b866c543390348721d30ac23b24d44b3f Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Tue, 3 Oct 2023 20:48:13 +0200 Subject: [PATCH 23/53] Swap expected/actual and move resource --- .../graph_builder/module/osm/OsmDatabaseTest.java | 6 +++--- .../module/osm}/ehningen-minimal.osm.pbf | Bin 2 files changed, 3 insertions(+), 3 deletions(-) rename src/test/resources/org/opentripplanner/{openstreetmap/model => graph_builder/module/osm}/ehningen-minimal.osm.pbf (100%) diff --git a/src/test/java/org/opentripplanner/graph_builder/module/osm/OsmDatabaseTest.java b/src/test/java/org/opentripplanner/graph_builder/module/osm/OsmDatabaseTest.java index 6355365df90..8c5671d3bf0 100644 --- a/src/test/java/org/opentripplanner/graph_builder/module/osm/OsmDatabaseTest.java +++ b/src/test/java/org/opentripplanner/graph_builder/module/osm/OsmDatabaseTest.java @@ -27,8 +27,8 @@ void bicycleRouteRelations() { var way = osmdb.getWay(13876983L); assertNotNull(way); - assertEquals(way.getTag("lcn"), "yes"); - assertEquals(way.getTag("name"), "Gärtringer Weg"); + assertEquals("yes", way.getTag("lcn")); + assertEquals("Gärtringer Weg", way.getTag("name")); } /** @@ -46,6 +46,6 @@ void invalidPublicTransportRelation() { var way = osmdb.getWay(302732658L); assertNotNull(way); - assertEquals(way.getTag("public_transport"), "platform"); + assertEquals("platform", way.getTag("public_transport")); } } diff --git a/src/test/resources/org/opentripplanner/openstreetmap/model/ehningen-minimal.osm.pbf b/src/test/resources/org/opentripplanner/graph_builder/module/osm/ehningen-minimal.osm.pbf similarity index 100% rename from src/test/resources/org/opentripplanner/openstreetmap/model/ehningen-minimal.osm.pbf rename to src/test/resources/org/opentripplanner/graph_builder/module/osm/ehningen-minimal.osm.pbf From 5b4686a4681d64c8715b6f4625b165ddbc239e3d Mon Sep 17 00:00:00 2001 From: Vincent Paturet Date: Tue, 3 Oct 2023 09:10:51 +0200 Subject: [PATCH 24/53] Update documentation --- src/ext/graphql/transmodelapi/schema.graphql | 2 +- .../ext/transmodelapi/model/plan/TripQuery.java | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ext/graphql/transmodelapi/schema.graphql b/src/ext/graphql/transmodelapi/schema.graphql index f1d53ff64b7..694f49b2cd0 100644 --- a/src/ext/graphql/transmodelapi/schema.graphql +++ b/src/ext/graphql/transmodelapi/schema.graphql @@ -830,7 +830,7 @@ type QueryType { """ relaxTransitSearchGeneralizedCostAtDestination: Float = null, """ - The length of the search-window in minutes. This parameter is optional. The maximum value is 1440 minutes (24 hours). + The length of the search-window in minutes. This parameter is optional. The search-window is defined as the duration between the earliest-departure-time(EDT) and the latest-departure-time(LDT). OTP will search for all itineraries in this departure window. If `arriveBy=true` the `dateTime` parameter is the latest-arrival-time, so OTP will dynamically calculate the EDT. Using a short search-window is faster than using a longer one, but the search duration is not linear. Using a "too" short search-window will waste resources server side, while using a search-window that is too long will be slow. diff --git a/src/ext/java/org/opentripplanner/ext/transmodelapi/model/plan/TripQuery.java b/src/ext/java/org/opentripplanner/ext/transmodelapi/model/plan/TripQuery.java index c695795ab7c..0f67b8ce406 100644 --- a/src/ext/java/org/opentripplanner/ext/transmodelapi/model/plan/TripQuery.java +++ b/src/ext/java/org/opentripplanner/ext/transmodelapi/model/plan/TripQuery.java @@ -58,8 +58,7 @@ public static GraphQLFieldDefinition create( .newArgument() .name("searchWindow") .description( - "The length of the search-window in minutes. This parameter is optional. " + - "The maximum value is 1440 minutes (24 hours)." + + "The length of the search-window in minutes. This parameter is optional." + "\n\n" + "The search-window is defined as the duration between the earliest-departure-time(EDT) and " + "the latest-departure-time(LDT). OTP will search for all itineraries in this departure " + From eec2025e953644006d61225c4ce5f913ff4203da Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Mon, 2 Oct 2023 23:21:44 +0200 Subject: [PATCH 25/53] Apply suggestions from code review Co-authored-by: Leonard Ehrenfried --- docs/sandbox/VehicleRentalServiceDirectory.md | 4 ++-- .../sandbox/VehicleRentalServiceDirectoryFetcherConfig.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/sandbox/VehicleRentalServiceDirectory.md b/docs/sandbox/VehicleRentalServiceDirectory.md index 340a6715a48..8009a62f912 100644 --- a/docs/sandbox/VehicleRentalServiceDirectory.md +++ b/docs/sandbox/VehicleRentalServiceDirectory.md @@ -63,9 +63,9 @@ HTTP headers to add to the request. Any header key, value can be inserted. List all networks to include. Use "network": "default-network" to set defaults. -If no default network exist only the listed networks are used. Configure the a network with +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 is dropped. Note! The values in the "default-network" are not used to set +networks are dropped. Note! The values in the "default-network" are not used to set missing field values in networks listed. 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 48693e50b6d..b30fc3d5b3f 100644 --- a/src/main/java/org/opentripplanner/standalone/config/sandbox/VehicleRentalServiceDirectoryFetcherConfig.java +++ b/src/main/java/org/opentripplanner/standalone/config/sandbox/VehicleRentalServiceDirectoryFetcherConfig.java @@ -63,9 +63,9 @@ private static List mapNetworkParameters( ) .description( """ - If no default network exist only the listed networks are used. Configure the a network with + 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 is dropped. Note! The values in the "{{default-network}}" are not used to set + networks are dropped. Note! The values in the "{{default-network}}" are not used to set missing field values in networks listed. """.replace( "{{default-network}}", From ab61358a4d9b5ae5ca2172a9f0bc650fd13ee5bc Mon Sep 17 00:00:00 2001 From: Vincent Paturet Date: Fri, 6 Oct 2023 10:39:58 +0200 Subject: [PATCH 26/53] Apply review suggestions --- docs/RouterConfiguration.md | 5 +++++ .../raptoradapter/transit/TransitTuningParameters.java | 6 ++++++ .../routing/api/request/RouteRequest.java | 9 +++++---- .../config/routerconfig/TransitRoutingConfig.java | 8 +++++++- 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/docs/RouterConfiguration.md b/docs/RouterConfiguration.md index 44769ecb69f..71d4e1e56b3 100644 --- a/docs/RouterConfiguration.md +++ b/docs/RouterConfiguration.md @@ -213,6 +213,11 @@ transfers is very little so it is better to set it too high than to low. Upper limit of the request parameter searchWindow. Maximum search window that can be set through the searchWindow API parameter. +Due to the way timetable data are collected before a Raptor trip search, +using a search window larger than 24 hours may lead to inconsistent search results. +Limiting the search window prevents also potential performance issues. +The recommended maximum value is 24 hours. +

scheduledTripBinarySearchThreshold

diff --git a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransitTuningParameters.java b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransitTuningParameters.java index 0f92bb449d3..caa5390633c 100644 --- a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransitTuningParameters.java +++ b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransitTuningParameters.java @@ -82,6 +82,12 @@ public List transferCacheRequests() { */ int transferCacheMaxSize(); + /** + * The maximum search window that can be set through the searchWindow API parameter. Due to the + * way timetable data are collected before a Raptor trip search, using a search window larger than + * 24 hours may lead to inconsistent search results. Limiting the search window prevents also + * potential performance issues. The recommended maximum value is 24 hours. + */ Duration maxSearchWindow(); /** diff --git a/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java b/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java index 35a14d32001..e9217b48a40 100644 --- a/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java +++ b/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java @@ -11,8 +11,8 @@ import java.util.List; import java.util.Locale; import java.util.function.Consumer; +import javax.annotation.Nullable; import org.opentripplanner.framework.time.DateUtils; -import org.opentripplanner.framework.time.TimeUtils; import org.opentripplanner.model.GenericLocation; import org.opentripplanner.model.plan.SortOrder; import org.opentripplanner.model.plan.pagecursor.PageCursor; @@ -56,7 +56,8 @@ public class RouteRequest implements Cloneable, Serializable { private Instant dateTime = Instant.now(); - private Duration maxSearchWindow = Duration.ZERO; + @Nullable + private Duration maxSearchWindow; private Duration searchWindow; @@ -334,14 +335,14 @@ public void setSearchWindow(Duration searchWindow) { } private boolean hasMaxSearchWindow() { - return !Duration.ZERO.equals(maxSearchWindow); + return maxSearchWindow != null; } public Duration maxSearchWindow() { return maxSearchWindow; } - public void setMaxSearchWindow(Duration maxSearchWindow) { + public void setMaxSearchWindow(@Nullable Duration maxSearchWindow) { this.maxSearchWindow = maxSearchWindow; } diff --git a/src/main/java/org/opentripplanner/standalone/config/routerconfig/TransitRoutingConfig.java b/src/main/java/org/opentripplanner/standalone/config/routerconfig/TransitRoutingConfig.java index 106c7eaba98..e1e67a1a781 100644 --- a/src/main/java/org/opentripplanner/standalone/config/routerconfig/TransitRoutingConfig.java +++ b/src/main/java/org/opentripplanner/standalone/config/routerconfig/TransitRoutingConfig.java @@ -208,7 +208,13 @@ public TransitRoutingConfig( .since(V2_4) .summary("Upper limit of the request parameter searchWindow.") .description( - "Maximum search window that can be set through the searchWindow API parameter." + """ + Maximum search window that can be set through the searchWindow API parameter. + Due to the way timetable data are collected before a Raptor trip search, + using a search window larger than 24 hours may lead to inconsistent search results. + Limiting the search window prevents also potential performance issues. + The recommended maximum value is 24 hours. + """ ) .asDuration(Duration.ofHours(24)); From 7295b6ecfee44fe8265718f3590fd599fec1ff3d Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Fri, 6 Oct 2023 22:24:00 +0000 Subject: [PATCH 27/53] Update dependency com.google.cloud:libraries-bom to v26.24.0 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 628dd75e37c..747785bceb2 100644 --- a/pom.xml +++ b/pom.xml @@ -572,7 +572,7 @@ com.google.cloud libraries-bom - 26.22.0 + 26.24.0 pom import From f89631b1dabeceb3b7cfa6e3bbe1a7f8ef603552 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Fri, 6 Oct 2023 22:24:18 +0000 Subject: [PATCH 28/53] Update graphqlcodegenerator monorepo --- .../apis/gtfs/generated/package.json | 6 +- .../apis/gtfs/generated/yarn.lock | 120 ++++++++++-------- 2 files changed, 71 insertions(+), 55 deletions(-) diff --git a/src/main/java/org/opentripplanner/apis/gtfs/generated/package.json b/src/main/java/org/opentripplanner/apis/gtfs/generated/package.json index c37b2e5a83b..6a2eb3343b9 100644 --- a/src/main/java/org/opentripplanner/apis/gtfs/generated/package.json +++ b/src/main/java/org/opentripplanner/apis/gtfs/generated/package.json @@ -11,9 +11,9 @@ "license": "LGPL-3.0", "dependencies": { "@graphql-codegen/add": "5.0.0", - "@graphql-codegen/cli": "4.0.1", - "@graphql-codegen/java": "3.3.6", - "@graphql-codegen/java-resolvers": "2.3.6", + "@graphql-codegen/cli": "5.0.0", + "@graphql-codegen/java": "4.0.0", + "@graphql-codegen/java-resolvers": "3.0.0", "graphql": "16.8.1" } } diff --git a/src/main/java/org/opentripplanner/apis/gtfs/generated/yarn.lock b/src/main/java/org/opentripplanner/apis/gtfs/generated/yarn.lock index 350a1b440fc..7e3c815cfb1 100644 --- a/src/main/java/org/opentripplanner/apis/gtfs/generated/yarn.lock +++ b/src/main/java/org/opentripplanner/apis/gtfs/generated/yarn.lock @@ -707,16 +707,16 @@ "@graphql-codegen/plugin-helpers" "^5.0.0" tslib "~2.5.0" -"@graphql-codegen/cli@4.0.1": - version "4.0.1" - resolved "https://registry.yarnpkg.com/@graphql-codegen/cli/-/cli-4.0.1.tgz#2bd494d55aaef0dfe86eefe1fa42bff81f5147fe" - integrity sha512-/H4imnGOl3hoPXLKmIiGUnXpmBmeIClSZie/YHDzD5N59cZlGGJlIOOrUlOTDpJx5JNU1MTQcRjyTToOYM5IfA== +"@graphql-codegen/cli@5.0.0": + version "5.0.0" + resolved "https://registry.yarnpkg.com/@graphql-codegen/cli/-/cli-5.0.0.tgz#761dcf08cfee88bbdd9cdf8097b2343445ec6f0a" + integrity sha512-A7J7+be/a6e+/ul2KI5sfJlpoqeqwX8EzktaKCeduyVKgOLA6W5t+NUGf6QumBDXU8PEOqXk3o3F+RAwCWOiqA== dependencies: "@babel/generator" "^7.18.13" "@babel/template" "^7.18.10" "@babel/types" "^7.18.13" "@graphql-codegen/core" "^4.0.0" - "@graphql-codegen/plugin-helpers" "^5.0.0" + "@graphql-codegen/plugin-helpers" "^5.0.1" "@graphql-tools/apollo-engine-loader" "^8.0.0" "@graphql-tools/code-file-loader" "^8.0.0" "@graphql-tools/git-loader" "^8.0.0" @@ -727,7 +727,6 @@ "@graphql-tools/prisma-loader" "^8.0.0" "@graphql-tools/url-loader" "^8.0.0" "@graphql-tools/utils" "^10.0.0" - "@parcel/watcher" "^2.1.0" "@whatwg-node/fetch" "^0.8.0" chalk "^4.1.0" cosmiconfig "^8.1.3" @@ -745,7 +744,7 @@ string-env-interpolation "^1.0.1" ts-log "^2.2.3" tslib "^2.4.0" - yaml "^1.10.0" + yaml "^2.3.1" yargs "^17.0.0" "@graphql-codegen/core@^4.0.0": @@ -758,37 +757,37 @@ "@graphql-tools/utils" "^10.0.0" tslib "~2.5.0" -"@graphql-codegen/java-common@^2.2.6": - version "2.2.6" - resolved "https://registry.yarnpkg.com/@graphql-codegen/java-common/-/java-common-2.2.6.tgz#a281cc99281427f05c03493fa8be847e0b1d10cd" - integrity sha512-r5uDanlZaZrMgavq0e+foP7FLQg9lrtzlMjsyW5+hsu7vzLRxz9J2MRnUeZwzd8pSed5ByKz167q+hqZdIVLNA== +"@graphql-codegen/java-common@^3.0.0": + version "3.0.0" + resolved "https://registry.yarnpkg.com/@graphql-codegen/java-common/-/java-common-3.0.0.tgz#96f736b4b0f25b77ad0fe1a5b528eecc6c06c215" + integrity sha512-yO31Lj8vBeuY43PIf8T6P1qr4XYytEvmYTkoL9KjfAa3w0u/0ld5e4xQz5QiZU0aE8q45tzBufsmE7ZmVGGKsA== dependencies: - "@graphql-codegen/plugin-helpers" "^2.7.2" + "@graphql-codegen/plugin-helpers" "^3.0.0" "@graphql-codegen/visitor-plugin-common" "2.13.1" auto-bind "~4.0.0" min-indent "1.0.1" - tslib "~2.4.0" + tslib "~2.6.0" unixify "^1.0.0" -"@graphql-codegen/java-resolvers@2.3.6": - version "2.3.6" - resolved "https://registry.yarnpkg.com/@graphql-codegen/java-resolvers/-/java-resolvers-2.3.6.tgz#78bd6acefec482d99ca70a57cc6893ce7d5a5abd" - integrity sha512-bZdPYmvcIVsbQ5wc9gHY7/iA5ysCHVlXkQ/JOCQFfZ+Dn98FggN/n4/ACaahmVRIkLguc9439CsME0uPS9xC1w== +"@graphql-codegen/java-resolvers@3.0.0": + version "3.0.0" + resolved "https://registry.yarnpkg.com/@graphql-codegen/java-resolvers/-/java-resolvers-3.0.0.tgz#822da4d0a870857ba005659189c7dacda2f7331a" + integrity sha512-apUCnrJb1179uu7Z/r/G8d71sbrdp6TsFO8pEOFJQz3MjrP5PfhrJr4JW8d8IsVdLng+OnvkfQGSbsGxgxhJHA== dependencies: - "@graphql-codegen/java-common" "^2.2.6" - "@graphql-codegen/plugin-helpers" "^2.7.2" + "@graphql-codegen/java-common" "^3.0.0" + "@graphql-codegen/plugin-helpers" "^3.0.0" "@graphql-codegen/visitor-plugin-common" "2.13.1" - tslib "~2.4.0" + tslib "~2.6.0" -"@graphql-codegen/java@3.3.6": - version "3.3.6" - resolved "https://registry.yarnpkg.com/@graphql-codegen/java/-/java-3.3.6.tgz#54769708ce72aa31b2b6b67ab4939f0ea752f3b5" - integrity sha512-df8YCInX1dX3WcDb9qM36AAylt+rQ67tcjkagXSdrNuKyXn7dfPFcjR3rStVJASy089SQVdRfkRvWVDeoEPCNQ== +"@graphql-codegen/java@4.0.0": + version "4.0.0" + resolved "https://registry.yarnpkg.com/@graphql-codegen/java/-/java-4.0.0.tgz#245e4403f19c390d5b6a03956558e34c3c4d7848" + integrity sha512-7pxwkgm0eFRDpq6PZx3n2tErBLr1wsG75USvCDnkkoz0145UCErk9GMAEfYgGx1mAm9+oUT+1wjZozjEYWjSow== dependencies: - "@graphql-codegen/java-common" "^2.2.6" - "@graphql-codegen/plugin-helpers" "^2.7.2" + "@graphql-codegen/java-common" "^3.0.0" + "@graphql-codegen/plugin-helpers" "^3.0.0" "@graphql-codegen/visitor-plugin-common" "2.13.1" - tslib "~2.4.0" + tslib "~2.6.0" "@graphql-codegen/plugin-helpers@^2.7.2": version "2.7.2" @@ -802,6 +801,18 @@ lodash "~4.17.0" tslib "~2.4.0" +"@graphql-codegen/plugin-helpers@^3.0.0": + version "3.1.2" + resolved "https://registry.yarnpkg.com/@graphql-codegen/plugin-helpers/-/plugin-helpers-3.1.2.tgz#69a2e91178f478ea6849846ade0a59a844d34389" + integrity sha512-emOQiHyIliVOIjKVKdsI5MXj312zmRDwmHpyUTZMjfpvxq/UVAHUJIVdVf+lnjjrI+LXBTgMlTWTgHQfmICxjg== + dependencies: + "@graphql-tools/utils" "^9.0.0" + change-case-all "1.0.15" + common-tags "1.8.2" + import-from "4.0.0" + lodash "~4.17.0" + tslib "~2.4.0" + "@graphql-codegen/plugin-helpers@^5.0.0": version "5.0.0" resolved "https://registry.yarnpkg.com/@graphql-codegen/plugin-helpers/-/plugin-helpers-5.0.0.tgz#40c18217454af5cf8317e5f46cf4d38e8cc78ae4" @@ -814,6 +825,18 @@ lodash "~4.17.0" tslib "~2.5.0" +"@graphql-codegen/plugin-helpers@^5.0.1": + version "5.0.1" + resolved "https://registry.yarnpkg.com/@graphql-codegen/plugin-helpers/-/plugin-helpers-5.0.1.tgz#e2429fcfba3f078d5aa18aa062d46c922bbb0d55" + integrity sha512-6L5sb9D8wptZhnhLLBcheSPU7Tg//DGWgc5tQBWX46KYTOTQHGqDpv50FxAJJOyFVJrveN9otWk9UT9/yfY4ww== + dependencies: + "@graphql-tools/utils" "^10.0.0" + change-case-all "1.0.15" + common-tags "1.8.2" + import-from "4.0.0" + lodash "~4.17.0" + tslib "~2.5.0" + "@graphql-codegen/visitor-plugin-common@2.13.1": version "2.13.1" resolved "https://registry.yarnpkg.com/@graphql-codegen/visitor-plugin-common/-/visitor-plugin-common-2.13.1.tgz#2228660f6692bcdb96b1f6d91a0661624266b76b" @@ -1098,6 +1121,14 @@ dependencies: tslib "^2.4.0" +"@graphql-tools/utils@^9.0.0": + version "9.2.1" + resolved "https://registry.yarnpkg.com/@graphql-tools/utils/-/utils-9.2.1.tgz#1b3df0ef166cfa3eae706e3518b17d5922721c57" + integrity sha512-WUw506Ql6xzmOORlriNrD6Ugx+HjVgYxt9KCXD9mHAak+eaXSwuGGPyE60hy9xaDEoXKBsG7SkG69ybitaVl6A== + dependencies: + "@graphql-typed-document-node/core" "^3.1.1" + tslib "^2.4.0" + "@graphql-tools/wrap@^10.0.0": version "10.0.0" resolved "https://registry.yarnpkg.com/@graphql-tools/wrap/-/wrap-10.0.0.tgz#573ab111482387d4acf4757d5fb7f9553a504bc1" @@ -1180,16 +1211,6 @@ "@nodelib/fs.scandir" "2.1.5" fastq "^1.6.0" -"@parcel/watcher@^2.1.0": - version "2.1.0" - resolved "https://registry.yarnpkg.com/@parcel/watcher/-/watcher-2.1.0.tgz#5f32969362db4893922c526a842d8af7a8538545" - integrity sha512-8s8yYjd19pDSsBpbkOHnT6Z2+UJSuLQx61pCFM0s5wSRvKCEMDjd/cHY3/GI1szHIWbpXpsJdg3V6ISGGx9xDw== - dependencies: - is-glob "^4.0.3" - micromatch "^4.0.5" - node-addon-api "^3.2.1" - node-gyp-build "^4.3.0" - "@peculiar/asn1-schema@^2.1.6", "@peculiar/asn1-schema@^2.3.0": version "2.3.3" resolved "https://registry.yarnpkg.com/@peculiar/asn1-schema/-/asn1-schema-2.3.3.tgz#21418e1f3819e0b353ceff0c2dad8ccb61acd777" @@ -2205,7 +2226,7 @@ is-fullwidth-code-point@^3.0.0: resolved "https://registry.yarnpkg.com/is-fullwidth-code-point/-/is-fullwidth-code-point-3.0.0.tgz#f116f8064fe90b3f7844a38997c0b75051269f1d" integrity sha512-zymm5+u+sCsSWyD9qNaejV3DFvhCKclKdizYaJUuHA83RLjb7nSuGnddCHGv0hk+KY7BMAlsWeK4Ueg6EV6XQg== -is-glob@4.0.3, is-glob@^4.0.1, is-glob@^4.0.3: +is-glob@4.0.3, is-glob@^4.0.1: version "4.0.3" resolved "https://registry.yarnpkg.com/is-glob/-/is-glob-4.0.3.tgz#64f61e42cbbb2eec2071a9dac0b28ba1e65d5084" integrity sha512-xelSayHH36ZgE7ZWhli7pW34hNbNl8Ojv5KVmkJD4hBdD3th8Tfk9vYasLM+mXWOZhFkgZfxhLSnrwRr4elSSg== @@ -2470,11 +2491,6 @@ no-case@^3.0.4: lower-case "^2.0.2" tslib "^2.0.3" -node-addon-api@^3.2.1: - version "3.2.1" - resolved "https://registry.yarnpkg.com/node-addon-api/-/node-addon-api-3.2.1.tgz#81325e0a2117789c0128dab65e7e38f07ceba161" - integrity sha512-mmcei9JghVNDYydghQmeDX8KoAm0FAiYyIcUt/N4nhyAipB17pllZQDOJD2fotxABnt4Mdz+dKTO7eftLg4d0A== - node-fetch@2.6.1: version "2.6.1" resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-2.6.1.tgz#045bd323631f76ed2e2b55573394416b639a0052" @@ -2494,11 +2510,6 @@ node-fetch@^2.6.1: dependencies: whatwg-url "^5.0.0" -node-gyp-build@^4.3.0: - version "4.6.0" - resolved "https://registry.yarnpkg.com/node-gyp-build/-/node-gyp-build-4.6.0.tgz#0c52e4cbf54bbd28b709820ef7b6a3c2d6209055" - integrity sha512-NTZVKn9IylLwUzaKjkas1e4u2DLNcV4rdYagA4PWdPwW87Bi7z+BznyKSRwS/761tV/lzCGXplWsiaMjLqP2zQ== - node-int64@^0.4.0: version "0.4.0" resolved "https://registry.yarnpkg.com/node-int64/-/node-int64-0.4.0.tgz#87a9065cdb355d3182d8f94ce11188b825c68a3b" @@ -3058,6 +3069,11 @@ tslib@^2.5.0: resolved "https://registry.yarnpkg.com/tslib/-/tslib-2.6.0.tgz#b295854684dbda164e181d259a22cd779dcd7bc3" integrity sha512-7At1WUettjcSRHXCyYtTselblcHl9PJFFVKiCAy/bY97+BPZXSQ2wbq0P9s8tK2G7dFQfNnlJnPAiArVBVBsfA== +tslib@~2.6.0: + version "2.6.2" + resolved "https://registry.yarnpkg.com/tslib/-/tslib-2.6.2.tgz#703ac29425e7b37cd6fd456e92404d46d1f3e4ae" + integrity sha512-AEYxH93jGFPn/a2iVAwW87VuUIkR1FVUKB77NwMF7nBTDkDrrT/Hpt/IrCJ0QXhW27jTBDcf5ZY7w6RiqTMw2Q== + type-fest@^0.21.3: version "0.21.3" resolved "https://registry.yarnpkg.com/type-fest/-/type-fest-0.21.3.tgz#d260a24b0198436e133fa26a524a6d65fa3b2e37" @@ -3218,10 +3234,10 @@ yaml-ast-parser@^0.0.43: resolved "https://registry.yarnpkg.com/yaml-ast-parser/-/yaml-ast-parser-0.0.43.tgz#e8a23e6fb4c38076ab92995c5dca33f3d3d7c9bb" integrity sha512-2PTINUwsRqSd+s8XxKaJWQlUuEMHJQyEuh2edBbW8KNJz0SJPwUSD2zRWqezFEdN7IzAgeuYHFUCF7o8zRdZ0A== -yaml@^1.10.0: - version "1.10.2" - resolved "https://registry.yarnpkg.com/yaml/-/yaml-1.10.2.tgz#2301c5ffbf12b467de8da2333a459e29e7920e4b" - integrity sha512-r3vXyErRCYJ7wg28yvBY5VSoAF8ZvlcW9/BwUzEtUsjvX/DKs24dIkuwjtuprwJJHsbyUbLApepYTR1BN4uHrg== +yaml@^2.3.1: + version "2.3.2" + resolved "https://registry.yarnpkg.com/yaml/-/yaml-2.3.2.tgz#f522db4313c671a0ca963a75670f1c12ea909144" + integrity sha512-N/lyzTPaJasoDmfV7YTrYCI0G/3ivm/9wdG0aHuheKowWQwGTsK0Eoiw6utmzAnI6pkJa0DUVygvp3spqqEKXg== yargs-parser@^18.1.2: version "18.1.3" From dd92820ddce424bf245dd4fc0b7ad3ff524dbd02 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Fri, 6 Oct 2023 16:57:10 +0200 Subject: [PATCH 29/53] Fix combined interlined leg fare calculation --- .../impl/CombinedInterlinedLegsFareServiceTest.java | 13 +++++++++++-- .../fares/impl/CombinedInterlinedTransitLeg.java | 8 ++++++++ .../ext/fares/impl/DefaultFareService.java | 7 ++++++- .../java/org/opentripplanner/model/plan/Leg.java | 1 - 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/ext-test/java/org/opentripplanner/ext/fares/impl/CombinedInterlinedLegsFareServiceTest.java b/src/ext-test/java/org/opentripplanner/ext/fares/impl/CombinedInterlinedLegsFareServiceTest.java index b1ad2c02f14..e601b8361c1 100644 --- a/src/ext-test/java/org/opentripplanner/ext/fares/impl/CombinedInterlinedLegsFareServiceTest.java +++ b/src/ext-test/java/org/opentripplanner/ext/fares/impl/CombinedInterlinedLegsFareServiceTest.java @@ -54,7 +54,7 @@ class CombinedInterlinedLegsFareServiceTest implements PlanTestConstants { name = "Itinerary with {3} and combination mode {0} should lead to a fare of {2}" ) @VariableSource("testCases") - void modeAlways(CombinationMode mode, Itinerary itinerary, Money expectedPrice, String hint) { + void modeAlways(CombinationMode mode, Itinerary itinerary, Money totalPrice, String hint) { var service = new CombinedInterlinedLegsFareService(mode); service.addFareRules( FareType.regular, @@ -65,7 +65,16 @@ void modeAlways(CombinationMode mode, Itinerary itinerary, Money expectedPrice, assertNotNull(fare); var price = fare.getFare(FareType.regular); + assertEquals(totalPrice, price); - assertEquals(expectedPrice, price); + var firstLeg = itinerary.getTransitLeg(0); + var uses = List.copyOf(fare.legProductsFromComponents().get(firstLeg)); + assertEquals(1, uses.size()); + + var firstProduct = uses.get(0).product(); + + var secondLeg = itinerary.getTransitLeg(1); + uses = List.copyOf(fare.legProductsFromComponents().get(secondLeg)); + assertEquals(1, uses.size()); } } diff --git a/src/ext/java/org/opentripplanner/ext/fares/impl/CombinedInterlinedTransitLeg.java b/src/ext/java/org/opentripplanner/ext/fares/impl/CombinedInterlinedTransitLeg.java index 4df3b77d3fe..0e9c10de7eb 100644 --- a/src/ext/java/org/opentripplanner/ext/fares/impl/CombinedInterlinedTransitLeg.java +++ b/src/ext/java/org/opentripplanner/ext/fares/impl/CombinedInterlinedTransitLeg.java @@ -9,6 +9,7 @@ import org.locationtech.jts.geom.LineString; import org.opentripplanner.framework.collection.ListUtils; import org.opentripplanner.model.fare.FareProductUse; +import org.opentripplanner.model.plan.Leg; import org.opentripplanner.model.plan.Place; import org.opentripplanner.model.plan.StopArrival; import org.opentripplanner.model.plan.TransitLeg; @@ -116,4 +117,11 @@ public void setFareProducts(List products) {} public List fareProducts() { return List.of(); } + + /** + * The two legs that this combined leg originally consisted of. + */ + public List originalLegs() { + return List.of(first, second); + } } diff --git a/src/ext/java/org/opentripplanner/ext/fares/impl/DefaultFareService.java b/src/ext/java/org/opentripplanner/ext/fares/impl/DefaultFareService.java index 583eca3f4e8..450961803f1 100644 --- a/src/ext/java/org/opentripplanner/ext/fares/impl/DefaultFareService.java +++ b/src/ext/java/org/opentripplanner/ext/fares/impl/DefaultFareService.java @@ -259,7 +259,12 @@ protected boolean populateFare( var componentLegs = new ArrayList(); for (int i = start; i <= via; ++i) { - componentLegs.add(legs.get(i)); + final Leg e = legs.get(i); + if(e instanceof CombinedInterlinedTransitLeg transitLeg) { + componentLegs.addAll(transitLeg.originalLegs()); + } else { + componentLegs.add(e); + } } components.add( new FareComponent(fareId, Money.ofFractionalAmount(currency, cost), componentLegs) diff --git a/src/main/java/org/opentripplanner/model/plan/Leg.java b/src/main/java/org/opentripplanner/model/plan/Leg.java index aa9949dae24..f561f2713e1 100644 --- a/src/main/java/org/opentripplanner/model/plan/Leg.java +++ b/src/main/java/org/opentripplanner/model/plan/Leg.java @@ -19,7 +19,6 @@ import org.opentripplanner.routing.alertpatch.TransitAlert; import org.opentripplanner.street.model.note.StreetNote; import org.opentripplanner.transit.model.basic.Accessibility; -import org.opentripplanner.transit.model.framework.FeedScopedId; import org.opentripplanner.transit.model.network.Route; import org.opentripplanner.transit.model.organization.Agency; import org.opentripplanner.transit.model.organization.Operator; From d19e3a83b1ee6a9e3171a95dedbb4acbd9fe5f7f Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Fri, 6 Oct 2023 17:12:49 +0200 Subject: [PATCH 30/53] Add test for leg fares --- ...CombinedInterlinedLegsFareServiceTest.java | 32 +++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/src/ext-test/java/org/opentripplanner/ext/fares/impl/CombinedInterlinedLegsFareServiceTest.java b/src/ext-test/java/org/opentripplanner/ext/fares/impl/CombinedInterlinedLegsFareServiceTest.java index e601b8361c1..4998184400d 100644 --- a/src/ext-test/java/org/opentripplanner/ext/fares/impl/CombinedInterlinedLegsFareServiceTest.java +++ b/src/ext-test/java/org/opentripplanner/ext/fares/impl/CombinedInterlinedLegsFareServiceTest.java @@ -13,6 +13,7 @@ import java.util.List; import java.util.stream.Stream; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.opentripplanner.ext.fares.impl.CombinedInterlinedLegsFareService.CombinationMode; @@ -54,7 +55,7 @@ class CombinedInterlinedLegsFareServiceTest implements PlanTestConstants { name = "Itinerary with {3} and combination mode {0} should lead to a fare of {2}" ) @VariableSource("testCases") - void modeAlways(CombinationMode mode, Itinerary itinerary, Money totalPrice, String hint) { + void modes(CombinationMode mode, Itinerary itinerary, Money totalPrice, String hint) { var service = new CombinedInterlinedLegsFareService(mode); service.addFareRules( FareType.regular, @@ -71,10 +72,37 @@ void modeAlways(CombinationMode mode, Itinerary itinerary, Money totalPrice, Str var uses = List.copyOf(fare.legProductsFromComponents().get(firstLeg)); assertEquals(1, uses.size()); - var firstProduct = uses.get(0).product(); + var secondLeg = itinerary.getTransitLeg(1); + uses = List.copyOf(fare.legProductsFromComponents().get(secondLeg)); + assertEquals(1, uses.size()); + } + + @Test + void legFares() { + var itinerary = interlinedWithSameRoute; + var service = new CombinedInterlinedLegsFareService(ALWAYS); + service.addFareRules( + FareType.regular, + List.of(AIRPORT_TO_CITY_CENTER_SET, INSIDE_CITY_CENTER_SET) + ); + + var fare = service.calculateFares(itinerary); + + var firstLeg = itinerary.getTransitLeg(0); + var uses = List.copyOf(fare.legProductsFromComponents().get(firstLeg)); + assertEquals(1, uses.size()); + + var firstLegUse = uses.get(0); + assertEquals(tenDollars, firstLegUse.product().price()); var secondLeg = itinerary.getTransitLeg(1); uses = List.copyOf(fare.legProductsFromComponents().get(secondLeg)); assertEquals(1, uses.size()); + + var secondLegUse = uses.get(0); + assertEquals(tenDollars, secondLegUse.product().price()); + + // the same far product is used for both legs as you only need to buy one + assertEquals(secondLegUse, firstLegUse); } } From c511d44cb9e304059ef32a046244a7a68d2e4689 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Mon, 9 Oct 2023 13:53:07 +0200 Subject: [PATCH 31/53] Improve documentation --- .../ext/fares/impl/DefaultFareService.java | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/ext/java/org/opentripplanner/ext/fares/impl/DefaultFareService.java b/src/ext/java/org/opentripplanner/ext/fares/impl/DefaultFareService.java index 450961803f1..fcdcfe05f2d 100644 --- a/src/ext/java/org/opentripplanner/ext/fares/impl/DefaultFareService.java +++ b/src/ext/java/org/opentripplanner/ext/fares/impl/DefaultFareService.java @@ -259,11 +259,16 @@ protected boolean populateFare( var componentLegs = new ArrayList(); for (int i = start; i <= via; ++i) { - final Leg e = legs.get(i); - if(e instanceof CombinedInterlinedTransitLeg transitLeg) { - componentLegs.addAll(transitLeg.originalLegs()); + final var leg = legs.get(i); + // if we have a leg that is combined for the purpose of fare calculation we need to + // retrieve the original legs so that the fare products are assigned back to the original + // legs that the combined one originally consisted of. + // (remember that the combined leg only exists during fare calculation and is thrown away + // afterwards to associating fare products with it will result in the API not showing any.) + if (leg instanceof CombinedInterlinedTransitLeg combinedLeg) { + componentLegs.addAll(combinedLeg.originalLegs()); } else { - componentLegs.add(e); + componentLegs.add(leg); } } components.add( @@ -379,12 +384,13 @@ protected Money getFarePrice(FareAttribute fare, FareType type) { /** * Returns true if two interlined legs (those with a stay-seated transfer between them) should be - * treated as a single leg. + * treated as a single leg for the purposes of fare calculation. *

* By default it's disabled since this is unspecified in the GTFS fares spec. * * @see DefaultFareService#combineInterlinedLegs(List) * @see HighestFareInFreeTransferWindowFareService#shouldCombineInterlinedLegs(ScheduledTransitLeg, ScheduledTransitLeg) + * @see HSLFareService#shouldCombineInterlinedLegs(ScheduledTransitLeg, ScheduledTransitLeg) */ protected boolean shouldCombineInterlinedLegs( ScheduledTransitLeg previousLeg, From 8d22b09bb370acf4875d9c14c5a5cf8df6cc3ec4 Mon Sep 17 00:00:00 2001 From: Daniel Heppner Date: Mon, 9 Oct 2023 14:53:54 -0700 Subject: [PATCH 32/53] fix(OrcaFares): update a bunch of fares Oct 2023 --- .../ext/fares/impl/OrcaFareService.java | 28 +++++++++---------- .../ext/fares/impl/OrcaFaresData.java | 28 +++++++++---------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java b/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java index 6482424e4a7..63e451a999e 100644 --- a/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java +++ b/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java @@ -299,19 +299,22 @@ private Money getLiftFare(RideType rideType, Money defaultFare, Route route) { case COMM_TRANS_COMMUTER_EXPRESS -> usDollars(2f); case KC_WATER_TAXI_VASHON_ISLAND -> usDollars(4.5f); case KC_WATER_TAXI_WEST_SEATTLE -> usDollars(3.75f); - case KITSAP_TRANSIT -> usDollars(1f); case KC_METRO, SOUND_TRANSIT, SOUND_TRANSIT_BUS, SOUND_TRANSIT_LINK, SOUND_TRANSIT_SOUNDER, + KITSAP_TRANSIT, EVERETT_TRANSIT, - SEATTLE_STREET_CAR -> usDollars(1.5f); + PIERCE_COUNTY_TRANSIT, + SEATTLE_STREET_CAR -> usDollars(1.00f); case WASHINGTON_STATE_FERRIES -> getWashingtonStateFerriesFare( route.getLongName(), FareType.electronicSpecial, defaultFare ); + case KITSAP_TRANSIT_FAST_FERRY_EASTBOUND -> usDollars((1f)); + case KITSAP_TRANSIT_FAST_FERRY_WESTBOUND -> usDollars((5f)); default -> defaultFare; }; } @@ -329,21 +332,19 @@ private Money getSeniorFare( case COMM_TRANS_LOCAL_SWIFT -> usDollars(1.25f); case COMM_TRANS_COMMUTER_EXPRESS -> usDollars(2f); case EVERETT_TRANSIT, SKAGIT_TRANSIT -> usDollars(0.5f); - case PIERCE_COUNTY_TRANSIT, SEATTLE_STREET_CAR, KITSAP_TRANSIT -> fareType.equals( // Pierce, Seattle Streetcar, and Kitsap only provide discounted senior fare for orca. - FareType.electronicSenior - ) - ? usDollars(1f) - : defaultFare; case KITSAP_TRANSIT_FAST_FERRY_EASTBOUND -> fareType.equals(FareType.electronicSenior) // Kitsap only provide discounted senior fare for orca. ? usDollars(1f) : usDollars(2f); case KC_WATER_TAXI_VASHON_ISLAND -> usDollars(3f); case KC_WATER_TAXI_WEST_SEATTLE -> usDollars(2.5f); - case KC_METRO, - SOUND_TRANSIT, + case SOUND_TRANSIT, SOUND_TRANSIT_BUS, SOUND_TRANSIT_LINK, - SOUND_TRANSIT_SOUNDER -> usDollars(1f); + SOUND_TRANSIT_SOUNDER, + KC_METRO, + PIERCE_COUNTY_TRANSIT, + SEATTLE_STREET_CAR, + KITSAP_TRANSIT -> fareType.equals(FareType.electronicSenior) ? usDollars(1f) : defaultFare; case KITSAP_TRANSIT_FAST_FERRY_WESTBOUND -> fareType.equals(FareType.electronicSenior) ? usDollars(5f) : usDollars(10f); @@ -358,16 +359,15 @@ private Money getSeniorFare( } /** - * Apply youth discount fares based on the ride type. - * Youth ride free in Washington. + * Apply youth discount fares based on the ride type. Youth ride free in Washington. */ private Money getYouthFare() { return Money.ZERO_USD; } /** - * Get the washington state ferries fare matching the route long name and fare type. If no match is found, return - * the default fare. + * Get the washington state ferries fare matching the route long name and fare type. If no match + * is found, return the default fare. */ private Money getWashingtonStateFerriesFare( I18NString routeLongName, diff --git a/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFaresData.java b/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFaresData.java index dbb04c552da..44d371590aa 100644 --- a/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFaresData.java +++ b/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFaresData.java @@ -236,20 +236,20 @@ class OrcaFaresData { // Spaces have been removed from the route name because of inconsistencies in the WSF GTFS route dataset. public static Map> washingtonStateFerriesFares = Map.ofEntries( - sEntry("Seattle-BainbridgeIsland", 9.25f, 4.60f), - sEntry("Seattle-Bremerton", 9.25f, 4.60f), - sEntry("Mukilteo-Clinton", 5.65f, 2.80f), - sEntry("Fauntleroy-VashonIsland", 6.10f, 3.05f), - sEntry("Fauntleroy-Southworth", 7.20f, 3.60f), - sEntry("Edmonds-Kingston", 9.25f, 4.60f), - sEntry("PointDefiance-Tahlequah", 6.10f, 3.05f), - sEntry("Anacortes-FridayHarbor", 14.85f, 7.40f), - sEntry("Anacortes-LopezIsland", 14.85f, 7.40f), - sEntry("Anacortes-OrcasIsland", 14.85f, 7.40f), - sEntry("Anacortes-ShawIsland", 14.85f, 7.40f), - sEntry("Coupeville-PortTownsend", 3.85f, 1.90f), - sEntry("PortTownsend-Coupeville", 3.85f, 1.90f), - sEntry("Southworth-VashonIsland", 6.10f, 3.05f) + sEntry("Seattle-BainbridgeIsland", 9.85f, 4.90f), + sEntry("Seattle-Bremerton", 9.85f, 4.90f), + sEntry("Mukilteo-Clinton", 6f, 3f), + sEntry("Fauntleroy-VashonIsland", 6.50f, 3.25f), + sEntry("Fauntleroy-Southworth", 7.70f, 3.85f), + sEntry("Edmonds-Kingston", 9.85f, 4.90f), + sEntry("PointDefiance-Tahlequah", 6.50f, 3.25f), + sEntry("Anacortes-FridayHarbor", 15.85f, 7.90f), + sEntry("Anacortes-LopezIsland", 15.85f, 7.90f), + sEntry("Anacortes-OrcasIsland", 15.85f, 7.90f), + sEntry("Anacortes-ShawIsland", 15.85f, 7.90f), + sEntry("Coupeville-PortTownsend", 4.10f, 2.05f), + sEntry("PortTownsend-Coupeville", 4.10f, 2.05f), + sEntry("Southworth-VashonIsland", 6.50f, 3.25f) ); private static Map.Entry> entry( From abb01032e041c2695200759a144f7ad1c436a451 Mon Sep 17 00:00:00 2001 From: Daniel Heppner Date: Mon, 9 Oct 2023 14:54:03 -0700 Subject: [PATCH 33/53] run formatter --- .../ext/fares/impl/OrcaFareService.java | 43 ++++++++++--------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java b/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java index 63e451a999e..35c24a9cabc 100644 --- a/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java +++ b/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java @@ -183,9 +183,10 @@ private static String cleanStationName(String s) { } /** - * Classify the ride type based on the route information provided. In most cases the agency name is sufficient. In - * some cases the route description and short name are needed to define inner agency ride types. For Kitsap, the - * route data is enough to define the agency, but addition trip id checks are needed to define the fast ferry direction. + * Classify the ride type based on the route information provided. In most cases the agency name + * is sufficient. In some cases the route description and short name are needed to define inner + * agency ride types. For Kitsap, the route data is enough to define the agency, but addition trip + * id checks are needed to define the fast ferry direction. */ private static RideType classify(Route route, String tripId) { var rideType = getRideType(route.getAgency().getId().getId(), route); @@ -217,8 +218,8 @@ private static RideType classify(Route route, String tripId) { } /** - * Define which discount fare should be applied based on the fare type. If the ride type is unknown the discount - * fare can not be applied, use the default fare. + * Define which discount fare should be applied based on the fare type. If the ride type is + * unknown the discount fare can not be applied, use the default fare. */ private Money getLegFare(FareType fareType, RideType rideType, Money defaultFare, Leg leg) { if (rideType == null) { @@ -265,7 +266,7 @@ private Money getRegularFare(FareType fareType, RideType rideType, Money default } /** - * Calculate the correct Link fare from a "ride" including start and end stations. + * Calculate the correct Link fare from a "ride" including start and end stations. */ private Money getSoundTransitFare( Leg leg, @@ -403,8 +404,9 @@ private Money getWashingtonStateFerriesFare( } /** - * Get the ride price for a single leg. If testing, this class is being called directly so the required agency cash - * values are not available therefore the default test price is used instead. + * Get the ride price for a single leg. If testing, this class is being called directly so the + * required agency cash values are not available therefore the default test price is used + * instead. */ protected Optional getRidePrice( Leg leg, @@ -415,12 +417,14 @@ protected Optional getRidePrice( } /** - * Calculate the cost of a journey. Where free transfers are not permitted the cash price is used. If free transfers - * are applicable, the most expensive discount fare across all legs is added to the final cumulative price. - * - * The computed fare for Orca card users takes into account realtime trip updates where available, so that, for - * instance, when a leg on a long itinerary is delayed to begin after the initial two hour window has expired, - * the calculated fare for that trip will be two one-way fares instead of one. + * Calculate the cost of a journey. Where free transfers are not permitted the cash price is used. + * If free transfers are applicable, the most expensive discount fare across all legs is added to + * the final cumulative price. + *

+ * The computed fare for Orca card users takes into account realtime trip updates where available, + * so that, for instance, when a leg on a long itinerary is delayed to begin after the initial two + * hour window has expired, the calculated fare for that trip will be two one-way fares instead of + * one. */ @Override public boolean populateFare( @@ -509,10 +513,11 @@ public boolean populateFare( /** * Adds a leg fare product to the given itinerary fares object - * @param leg The leg to create a fareproduct for - * @param itineraryFares The itinerary fares to store the fare product in - * @param fareType Fare type (split into container and rider category) - * @param totalFare Total fare paid after transfer + * + * @param leg The leg to create a fareproduct for + * @param itineraryFares The itinerary fares to store the fare product in + * @param fareType Fare type (split into container and rider category) + * @param totalFare Total fare paid after transfer * @param transferDiscount Transfer discount applied */ private static void addLegFareProduct( @@ -556,8 +561,6 @@ protected Collection fareRulesForFeed(FareType fareType, String fee /** * Check if trip falls within the transfer time window. - * @param freeTransferStartTime - * @param currentLegStartTime */ private boolean inFreeTransferWindow( ZonedDateTime freeTransferStartTime, From f8264327a80b9344d9aaa76e6f75298d056a85f7 Mon Sep 17 00:00:00 2001 From: Daniel Heppner Date: Mon, 9 Oct 2023 16:53:57 -0700 Subject: [PATCH 34/53] improvement(OrcaFares): calculate WTA and SKAT fares & support optional fares --- .../ext/fares/impl/OrcaFareService.java | 159 ++++++++++++------ 1 file changed, 106 insertions(+), 53 deletions(-) diff --git a/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java b/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java index 35c24a9cabc..0e6777104d6 100644 --- a/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java +++ b/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java @@ -12,6 +12,7 @@ import java.util.Optional; import javax.annotation.Nullable; import org.opentripplanner.ext.fares.model.FareRuleSet; +import org.opentripplanner.ext.ridehailing.model.Ride; import org.opentripplanner.framework.i18n.I18NString; import org.opentripplanner.model.fare.FareMedium; import org.opentripplanner.model.fare.FareProduct; @@ -40,6 +41,7 @@ public class OrcaFareService extends DefaultFareService { public static final String SEATTLE_STREET_CAR_AGENCY_ID = "23"; public static final String WASHINGTON_STATE_FERRIES_AGENCY_ID = "WSF"; public static final String KITSAP_TRANSIT_AGENCY_ID = "kt"; + public static final String WHATCOM_AGENCY_ID = "14"; public static final int ROUTE_TYPE_FERRY = 4; public static final String FEED_ID = "orca"; private static final FareMedium ELECTRONIC_MEDIUM = new FareMedium( @@ -69,6 +71,10 @@ protected enum RideType { SOUND_TRANSIT_SOUNDER, SOUND_TRANSIT_LINK, WASHINGTON_STATE_FERRIES, + WHATCOM_LOCAL, + WHATCOM_CROSS_COUNTY, + SKAGIT_LOCAL, + SKAGIT_CROSS_COUNTY, UNKNOWN, } @@ -127,10 +133,29 @@ static RideType getRideType(String agencyId, Route route) { } case SOUND_TRANSIT_AGENCY_ID -> RideType.SOUND_TRANSIT; case EVERETT_TRANSIT_AGENCY_ID -> RideType.EVERETT_TRANSIT; - case SKAGIT_TRANSIT_AGENCY_ID -> RideType.SKAGIT_TRANSIT; + case SKAGIT_TRANSIT_AGENCY_ID -> { + try { + yield route.getShortName().equals("80X") || route.getShortName().equals("90X") + ? RideType.SKAGIT_CROSS_COUNTY + : RideType.SKAGIT_LOCAL; + } catch (NumberFormatException e) { + LOG.warn("Unable to determine skagit route id from {}.", route.getShortName(), e); + yield RideType.SKAGIT_LOCAL; + } + } case SEATTLE_STREET_CAR_AGENCY_ID -> RideType.SEATTLE_STREET_CAR; case WASHINGTON_STATE_FERRIES_AGENCY_ID -> RideType.WASHINGTON_STATE_FERRIES; case KITSAP_TRANSIT_AGENCY_ID -> RideType.KITSAP_TRANSIT; + case WHATCOM_AGENCY_ID -> { + try { + yield route.getShortName().equals("80X") + ? RideType.WHATCOM_CROSS_COUNTY + : RideType.WHATCOM_LOCAL; + } catch (NumberFormatException e) { + LOG.warn("Unable to determine whatcom route id from {}.", route.getShortName(), e); + yield RideType.WHATCOM_LOCAL; + } + } default -> RideType.UNKNOWN; }; } @@ -221,12 +246,21 @@ private static RideType classify(Route route, String tripId) { * Define which discount fare should be applied based on the fare type. If the ride type is * unknown the discount fare can not be applied, use the default fare. */ - private Money getLegFare(FareType fareType, RideType rideType, Money defaultFare, Leg leg) { + private Optional getLegFare( + FareType fareType, + RideType rideType, + Money defaultFare, + Leg leg + ) { if (rideType == null) { - return defaultFare; + return Optional.of(defaultFare); + } + // Filter out agencies that don't accept ORCA from the electronic fare type + if (usesOrca(fareType) && !agencyAcceptsOrca(rideType)) { + return Optional.empty(); } return switch (fareType) { - case youth, electronicYouth -> getYouthFare(); + case youth, electronicYouth -> Optional.of(getYouthFare()); case electronicSpecial -> getLiftFare(rideType, defaultFare, leg.getRoute()); case electronicSenior, senior -> getSeniorFare( fareType, @@ -235,33 +269,37 @@ private Money getLegFare(FareType fareType, RideType rideType, Money defaultFare leg.getRoute() ); case regular, electronicRegular -> getRegularFare(fareType, rideType, defaultFare, leg); - default -> defaultFare; + default -> Optional.of(defaultFare); }; } + private static Optional optionalUSD(float amount) { + return Optional.of(usDollars(amount)); + } + /** * Apply regular discount fares. If the ride type cannot be matched the default fare is used. */ - private Money getRegularFare(FareType fareType, RideType rideType, Money defaultFare, Leg leg) { + private Optional getRegularFare( + FareType fareType, + RideType rideType, + Money defaultFare, + Leg leg + ) { Route route = leg.getRoute(); return switch (rideType) { - case KC_WATER_TAXI_VASHON_ISLAND -> usDollars(5.75f); - case KC_WATER_TAXI_WEST_SEATTLE -> usDollars(5f); - case KITSAP_TRANSIT_FAST_FERRY_EASTBOUND -> usDollars(2f); - case KITSAP_TRANSIT_FAST_FERRY_WESTBOUND -> usDollars(10f); - case WASHINGTON_STATE_FERRIES -> getWashingtonStateFerriesFare( - route.getLongName(), - fareType, - defaultFare + case KC_WATER_TAXI_VASHON_ISLAND -> optionalUSD(5.75f); + case KC_WATER_TAXI_WEST_SEATTLE -> optionalUSD(5f); + case KITSAP_TRANSIT_FAST_FERRY_EASTBOUND -> optionalUSD(2f); + case KITSAP_TRANSIT_FAST_FERRY_WESTBOUND -> optionalUSD(10f); + case WASHINGTON_STATE_FERRIES -> Optional.of( + getWashingtonStateFerriesFare(route.getLongName(), fareType, defaultFare) ); - case SOUND_TRANSIT_LINK, SOUND_TRANSIT_SOUNDER -> getSoundTransitFare( - leg, - fareType, - defaultFare, - rideType + case SOUND_TRANSIT_LINK, SOUND_TRANSIT_SOUNDER -> Optional.of( + getSoundTransitFare(leg, fareType, defaultFare, rideType) ); - case SOUND_TRANSIT_BUS -> usDollars(3.25f); - default -> defaultFare; + case SOUND_TRANSIT_BUS -> optionalUSD(3.25f); + default -> Optional.of(defaultFare); }; } @@ -294,12 +332,12 @@ private Money getSoundTransitFare( /** * Apply Orca lift discount fares based on the ride type. */ - private Money getLiftFare(RideType rideType, Money defaultFare, Route route) { + private Optional getLiftFare(RideType rideType, Money defaultFare, Route route) { return switch (rideType) { - case COMM_TRANS_LOCAL_SWIFT -> usDollars(1.25f); - case COMM_TRANS_COMMUTER_EXPRESS -> usDollars(2f); - case KC_WATER_TAXI_VASHON_ISLAND -> usDollars(4.5f); - case KC_WATER_TAXI_WEST_SEATTLE -> usDollars(3.75f); + case COMM_TRANS_LOCAL_SWIFT -> optionalUSD(1.25f); + case COMM_TRANS_COMMUTER_EXPRESS -> optionalUSD(2f); + case KC_WATER_TAXI_VASHON_ISLAND -> optionalUSD(4.5f); + case KC_WATER_TAXI_WEST_SEATTLE -> optionalUSD(3.75f); case KC_METRO, SOUND_TRANSIT, SOUND_TRANSIT_BUS, @@ -308,36 +346,37 @@ private Money getLiftFare(RideType rideType, Money defaultFare, Route route) { KITSAP_TRANSIT, EVERETT_TRANSIT, PIERCE_COUNTY_TRANSIT, - SEATTLE_STREET_CAR -> usDollars(1.00f); - case WASHINGTON_STATE_FERRIES -> getWashingtonStateFerriesFare( - route.getLongName(), - FareType.electronicSpecial, - defaultFare + SEATTLE_STREET_CAR -> optionalUSD(1.00f); + case WASHINGTON_STATE_FERRIES -> Optional.of( + getWashingtonStateFerriesFare(route.getLongName(), FareType.electronicSpecial, defaultFare) ); - case KITSAP_TRANSIT_FAST_FERRY_EASTBOUND -> usDollars((1f)); - case KITSAP_TRANSIT_FAST_FERRY_WESTBOUND -> usDollars((5f)); - default -> defaultFare; + case KITSAP_TRANSIT_FAST_FERRY_EASTBOUND -> optionalUSD((1f)); + case KITSAP_TRANSIT_FAST_FERRY_WESTBOUND -> optionalUSD((5f)); + default -> Optional.of(defaultFare); }; } /** * Apply senior discount fares based on the fare and ride types. */ - private Money getSeniorFare( + private Optional getSeniorFare( FareType fareType, RideType rideType, Money defaultFare, Route route ) { return switch (rideType) { - case COMM_TRANS_LOCAL_SWIFT -> usDollars(1.25f); - case COMM_TRANS_COMMUTER_EXPRESS -> usDollars(2f); - case EVERETT_TRANSIT, SKAGIT_TRANSIT -> usDollars(0.5f); + case COMM_TRANS_LOCAL_SWIFT -> optionalUSD(1.25f); + case COMM_TRANS_COMMUTER_EXPRESS -> optionalUSD(2f); + case EVERETT_TRANSIT, + SKAGIT_TRANSIT, + WHATCOM_LOCAL, + SKAGIT_LOCAL -> optionalUSD(0.5f); case KITSAP_TRANSIT_FAST_FERRY_EASTBOUND -> fareType.equals(FareType.electronicSenior) // Kitsap only provide discounted senior fare for orca. - ? usDollars(1f) - : usDollars(2f); - case KC_WATER_TAXI_VASHON_ISLAND -> usDollars(3f); - case KC_WATER_TAXI_WEST_SEATTLE -> usDollars(2.5f); + ? optionalUSD(1f) + : optionalUSD(2f); + case KC_WATER_TAXI_VASHON_ISLAND -> optionalUSD(3f); + case KC_WATER_TAXI_WEST_SEATTLE -> optionalUSD(2.5f); case SOUND_TRANSIT, SOUND_TRANSIT_BUS, SOUND_TRANSIT_LINK, @@ -345,17 +384,18 @@ private Money getSeniorFare( KC_METRO, PIERCE_COUNTY_TRANSIT, SEATTLE_STREET_CAR, - KITSAP_TRANSIT -> fareType.equals(FareType.electronicSenior) ? usDollars(1f) : defaultFare; + KITSAP_TRANSIT -> fareType.equals(FareType.electronicSenior) + ? optionalUSD(1f) + : Optional.of(defaultFare); case KITSAP_TRANSIT_FAST_FERRY_WESTBOUND -> fareType.equals(FareType.electronicSenior) - ? usDollars(5f) - : usDollars(10f); + ? optionalUSD(5f) + : optionalUSD(10f); // Discount specific to Skagit transit and not Orca. - case WASHINGTON_STATE_FERRIES -> getWashingtonStateFerriesFare( - route.getLongName(), - fareType, - defaultFare + case WASHINGTON_STATE_FERRIES -> Optional.of( + getWashingtonStateFerriesFare(route.getLongName(), fareType, defaultFare) ); - default -> defaultFare; + case WHATCOM_CROSS_COUNTY, SKAGIT_CROSS_COUNTY -> optionalUSD(1f); + default -> Optional.of(defaultFare); }; } @@ -445,9 +485,15 @@ public boolean populateFare( freeTransferStartTime = leg.getStartTime(); } Optional singleLegPrice = getRidePrice(leg, fareType, fareRules); - Money legFare = singleLegPrice - .map(slp -> getLegFare(fareType, rideType, slp, leg)) - .orElse(Money.ZERO_USD); + Optional optionalLegFare = singleLegPrice.flatMap(slp -> + getLegFare(fareType, rideType, slp, leg) + ); + if (optionalLegFare.isEmpty()) { + // If there is no fare for this leg then skip the rest of the logic. + continue; + } + Money legFare = optionalLegFare.get(); + boolean inFreeTransferWindow = inFreeTransferWindow( freeTransferStartTime, leg.getStartTime() @@ -593,6 +639,13 @@ private boolean permitsFreeTransfers(RideType rideType) { }; } + private static boolean agencyAcceptsOrca(RideType rideType) { + return switch (rideType) { + case WHATCOM_LOCAL, WHATCOM_CROSS_COUNTY, SKAGIT_CROSS_COUNTY, SKAGIT_LOCAL -> false; + default -> true; + }; + } + /** * Define Orca fare types. */ From 8f9be3fee8c5e87b2b940a3b51f84312b92ab9ae Mon Sep 17 00:00:00 2001 From: bartosz Date: Tue, 10 Oct 2023 10:53:53 +0200 Subject: [PATCH 35/53] Update service bus dependencies --- pom.xml | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/pom.xml b/pom.xml index adca0be9444..3592e499f4f 100644 --- a/pom.xml +++ b/pom.xml @@ -940,22 +940,17 @@ com.azure azure-core - 1.36.0 + 1.43.0 com.azure azure-messaging-servicebus - 7.13.2 - - - com.microsoft.azure - azure-servicebus - 3.6.7 + 7.14.4 com.azure.resourcemanager azure-resourcemanager-servicebus - 2.24.0 + 2.30.0 ch.poole From 9867c672c064e8d46c072af9b05abef482cda6ac Mon Sep 17 00:00:00 2001 From: OTP Changelog Bot Date: Tue, 10 Oct 2023 09:37:06 +0000 Subject: [PATCH 36/53] Add changelog entry for #5379 [ci skip] --- docs/Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/Changelog.md b/docs/Changelog.md index 56d47aae729..fb32a48c68f 100644 --- a/docs/Changelog.md +++ b/docs/Changelog.md @@ -14,6 +14,7 @@ based on merged pull requests. Search GitHub issues and pull requests for smalle - Apply correct traversal permissions to barrier vertex [#5369](https://github.com/opentripplanner/OpenTripPlanner/pull/5369) - Move GTFS GraphQL API out of the sandbox [#5339](https://github.com/opentripplanner/OpenTripPlanner/pull/5339) - Transmodel GraphQL API for pass-through searches [#5320](https://github.com/opentripplanner/OpenTripPlanner/pull/5320) +- Fix check for OSM relation members not being present in the extract [#5379](https://github.com/opentripplanner/OpenTripPlanner/pull/5379) [](AUTOMATIC_CHANGELOG_PLACEHOLDER_DO_NOT_REMOVE) ## 2.4.0 (2023-09-13) From ce21dae5925dac09331cb216622b7289ad25644d Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 10 Oct 2023 09:37:08 +0000 Subject: [PATCH 37/53] Update micrometer.version to v1.11.5 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index adca0be9444..e3c9fddb587 100644 --- a/pom.xml +++ b/pom.xml @@ -63,7 +63,7 @@ 2.15.2 3.1.3 5.10.0 - 1.11.4 + 1.11.5 5.5.3 1.4.11 9.8.0 From 407ecd108ddb59cf2942daff8b2194f60289b753 Mon Sep 17 00:00:00 2001 From: Vincent Paturet Date: Tue, 10 Oct 2023 14:19:32 +0200 Subject: [PATCH 38/53] Apply review suggestions --- docs/RouterConfiguration.md | 2 ++ .../raptoradapter/transit/TransitTuningParameters.java | 2 ++ .../standalone/config/routerconfig/TransitRoutingConfig.java | 2 ++ 3 files changed, 6 insertions(+) diff --git a/docs/RouterConfiguration.md b/docs/RouterConfiguration.md index 71d4e1e56b3..d07ebdc86c4 100644 --- a/docs/RouterConfiguration.md +++ b/docs/RouterConfiguration.md @@ -217,6 +217,8 @@ Due to the way timetable data are collected before a Raptor trip search, using a search window larger than 24 hours may lead to inconsistent search results. Limiting the search window prevents also potential performance issues. The recommended maximum value is 24 hours. +This parameter does not restrict the maximum duration of a dynamic search window (use +the parameter transit.dynamicSearchWindow.maxWindow to specify such a restriction).

scheduledTripBinarySearchThreshold

diff --git a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransitTuningParameters.java b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransitTuningParameters.java index caa5390633c..31c602b6133 100644 --- a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransitTuningParameters.java +++ b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransitTuningParameters.java @@ -87,6 +87,8 @@ public List transferCacheRequests() { * way timetable data are collected before a Raptor trip search, using a search window larger than * 24 hours may lead to inconsistent search results. Limiting the search window prevents also * potential performance issues. The recommended maximum value is 24 hours. + * This parameter does not restrict the maximum duration of a dynamic search window (use + * the parameter transit.dynamicSearchWindow.maxWindow to specify such a restriction). */ Duration maxSearchWindow(); diff --git a/src/main/java/org/opentripplanner/standalone/config/routerconfig/TransitRoutingConfig.java b/src/main/java/org/opentripplanner/standalone/config/routerconfig/TransitRoutingConfig.java index e1e67a1a781..81a93d5a226 100644 --- a/src/main/java/org/opentripplanner/standalone/config/routerconfig/TransitRoutingConfig.java +++ b/src/main/java/org/opentripplanner/standalone/config/routerconfig/TransitRoutingConfig.java @@ -214,6 +214,8 @@ public TransitRoutingConfig( using a search window larger than 24 hours may lead to inconsistent search results. Limiting the search window prevents also potential performance issues. The recommended maximum value is 24 hours. + This parameter does not restrict the maximum duration of a dynamic search window (use + the parameter transit.dynamicSearchWindow.maxWindow to specify such a restriction). """ ) .asDuration(Duration.ofHours(24)); From 719111385bfeee44b380beadc0763ad680b1a214 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Tue, 10 Oct 2023 14:39:44 +0200 Subject: [PATCH 39/53] Update micrometer only monthly [ci skip] --- renovate.json5 | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/renovate.json5 b/renovate.json5 index 934001eae04..b600b9593d6 100644 --- a/renovate.json5 +++ b/renovate.json5 @@ -47,7 +47,9 @@ "@graphql-codegen/cli", "@graphql-codegen/java", "@graphql-codegen/java-resolvers", - "graphql" + "graphql", + "io.micrometer:micrometer-registry-prometheus", + "io.micrometer:micrometer-registry-influx" ], // we don't use the 'monthly' preset because that only fires on the first day of the month // when there might already other PRs open From 79ba4982faf28d651f1918189b8d3323b95490c6 Mon Sep 17 00:00:00 2001 From: vpaturet <46598384+vpaturet@users.noreply.github.com> Date: Tue, 10 Oct 2023 15:16:29 +0200 Subject: [PATCH 40/53] Apply review suggestions --- docs/RouterConfiguration.md | 2 +- .../standalone/config/routerconfig/TransitRoutingConfig.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/RouterConfiguration.md b/docs/RouterConfiguration.md index d07ebdc86c4..8d749511f1a 100644 --- a/docs/RouterConfiguration.md +++ b/docs/RouterConfiguration.md @@ -218,7 +218,7 @@ using a search window larger than 24 hours may lead to inconsistent search resul Limiting the search window prevents also potential performance issues. The recommended maximum value is 24 hours. This parameter does not restrict the maximum duration of a dynamic search window (use -the parameter transit.dynamicSearchWindow.maxWindow to specify such a restriction). +the parameter `transit.dynamicSearchWindow.maxWindow` to specify such a restriction).

scheduledTripBinarySearchThreshold

diff --git a/src/main/java/org/opentripplanner/standalone/config/routerconfig/TransitRoutingConfig.java b/src/main/java/org/opentripplanner/standalone/config/routerconfig/TransitRoutingConfig.java index 81a93d5a226..d4c99004a59 100644 --- a/src/main/java/org/opentripplanner/standalone/config/routerconfig/TransitRoutingConfig.java +++ b/src/main/java/org/opentripplanner/standalone/config/routerconfig/TransitRoutingConfig.java @@ -215,7 +215,7 @@ public TransitRoutingConfig( Limiting the search window prevents also potential performance issues. The recommended maximum value is 24 hours. This parameter does not restrict the maximum duration of a dynamic search window (use - the parameter transit.dynamicSearchWindow.maxWindow to specify such a restriction). + the parameter `transit.dynamicSearchWindow.maxWindow` to specify such a restriction). """ ) .asDuration(Duration.ofHours(24)); From 16416969495484d2fd8598518efcb96686e9f2a0 Mon Sep 17 00:00:00 2001 From: Daniel Heppner Date: Tue, 10 Oct 2023 11:29:10 -0700 Subject: [PATCH 41/53] fix(OrcaFares): add support for T-Link --- .../org/opentripplanner/ext/fares/impl/OrcaFareService.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java b/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java index 0e6777104d6..2251dcb22a4 100644 --- a/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java +++ b/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java @@ -35,6 +35,7 @@ public class OrcaFareService extends DefaultFareService { public static final String COMM_TRANS_AGENCY_ID = "29"; public static final String KC_METRO_AGENCY_ID = "1"; public static final String SOUND_TRANSIT_AGENCY_ID = "40"; + public static final String T_LINK_AGENCY_ID = "F1"; public static final String EVERETT_TRANSIT_AGENCY_ID = "97"; public static final String PIERCE_COUNTY_TRANSIT_AGENCY_ID = "3"; public static final String SKAGIT_TRANSIT_AGENCY_ID = "e0e4541a-2714-487b-b30c-f5c6cb4a310f"; @@ -69,6 +70,7 @@ protected enum RideType { SOUND_TRANSIT, SOUND_TRANSIT_BUS, SOUND_TRANSIT_SOUNDER, + SOUND_TRANSIT_T_LINK, SOUND_TRANSIT_LINK, WASHINGTON_STATE_FERRIES, WHATCOM_LOCAL, @@ -145,6 +147,7 @@ static RideType getRideType(String agencyId, Route route) { } case SEATTLE_STREET_CAR_AGENCY_ID -> RideType.SEATTLE_STREET_CAR; case WASHINGTON_STATE_FERRIES_AGENCY_ID -> RideType.WASHINGTON_STATE_FERRIES; + case T_LINK_AGENCY_ID -> RideType.SOUND_TRANSIT_T_LINK; case KITSAP_TRANSIT_AGENCY_ID -> RideType.KITSAP_TRANSIT; case WHATCOM_AGENCY_ID -> { try { @@ -343,6 +346,7 @@ private Optional getLiftFare(RideType rideType, Money defaultFare, Route SOUND_TRANSIT_BUS, SOUND_TRANSIT_LINK, SOUND_TRANSIT_SOUNDER, + SOUND_TRANSIT_T_LINK, KITSAP_TRANSIT, EVERETT_TRANSIT, PIERCE_COUNTY_TRANSIT, @@ -381,6 +385,7 @@ private Optional getSeniorFare( SOUND_TRANSIT_BUS, SOUND_TRANSIT_LINK, SOUND_TRANSIT_SOUNDER, + SOUND_TRANSIT_T_LINK, KC_METRO, PIERCE_COUNTY_TRANSIT, SEATTLE_STREET_CAR, From ff13046ab0c79740ca22051b66c2efbb9f24c966 Mon Sep 17 00:00:00 2001 From: Daniel Heppner Date: Tue, 10 Oct 2023 12:35:50 -0700 Subject: [PATCH 42/53] fix(OrcaFares): temporary hack until we remove FareType entirely --- .../org/opentripplanner/ext/fares/impl/OrcaFareService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java b/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java index 2251dcb22a4..0c6dbac1dc7 100644 --- a/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java +++ b/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java @@ -489,7 +489,7 @@ public boolean populateFare( // The start of a free transfer must be with a transit agency that permits it! freeTransferStartTime = leg.getStartTime(); } - Optional singleLegPrice = getRidePrice(leg, fareType, fareRules); + Optional singleLegPrice = getRidePrice(leg, FareType.regular, fareRules); Optional optionalLegFare = singleLegPrice.flatMap(slp -> getLegFare(fareType, rideType, slp, leg) ); From c2a421b98b3cff702482d71d283f8bbec291da9c Mon Sep 17 00:00:00 2001 From: Daniel Heppner Date: Tue, 10 Oct 2023 13:16:54 -0700 Subject: [PATCH 43/53] chore: update tests --- .../ext/fares/impl/OrcaFareServiceTest.java | 52 ++++++++++--------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/src/ext-test/java/org/opentripplanner/ext/fares/impl/OrcaFareServiceTest.java b/src/ext-test/java/org/opentripplanner/ext/fares/impl/OrcaFareServiceTest.java index 9e8376ec635..f3546dd4060 100644 --- a/src/ext-test/java/org/opentripplanner/ext/fares/impl/OrcaFareServiceTest.java +++ b/src/ext-test/java/org/opentripplanner/ext/fares/impl/OrcaFareServiceTest.java @@ -51,10 +51,9 @@ public class OrcaFareServiceTest { private static final Money ONE_DOLLAR = usDollars(1f); private static final Money TWO_DOLLARS = usDollars(2); - private static final Money FERRY_FARE = usDollars(6.10f); - private static final Money HALF_FERRY_FARE = usDollars(3.05f); - private static final Money ORCA_REGULAR_FARE = usDollars(2.50f); - private static final Money ORCA_SPECIAL_FARE = usDollars(1.50f); + private static final Money FERRY_FARE = usDollars(6.50f); + private static final Money HALF_FERRY_FARE = usDollars(3.25f); + private static final Money ORCA_SPECIAL_FARE = usDollars(1.00f); private static TestOrcaFareService orcaFareService; public static final Money DEFAULT_TEST_RIDE_PRICE = usDollars(3.49f); @@ -213,7 +212,7 @@ public void calculateFareThatIncludesNoFreeTransfers() { getLeg(WASHINGTON_STATE_FERRIES_AGENCY_ID, 30, "VashonIsland-Fauntelroy"), getLeg(KITSAP_TRANSIT_AGENCY_ID, 60), getLeg(SKAGIT_TRANSIT_AGENCY_ID, 90), - getLeg(KITSAP_TRANSIT_AGENCY_ID, 120), + getLeg(KITSAP_TRANSIT_AGENCY_ID, 121), getLeg(WASHINGTON_STATE_FERRIES_AGENCY_ID, 150, "Fauntleroy-VashonIsland") ); calculateFare(rides, regular, DEFAULT_TEST_RIDE_PRICE.times(4).plus(FERRY_FARE)); @@ -223,20 +222,21 @@ public void calculateFareThatIncludesNoFreeTransfers() { DEFAULT_TEST_RIDE_PRICE.times(3).plus(usDollars(.50f)).plus(HALF_FERRY_FARE) ); calculateFare(rides, FareType.youth, Money.ZERO_USD); + // We don't get any fares for the skagit transit leg below here because they don't accept ORCA (electronic) calculateFare( rides, FareType.electronicSpecial, - ONE_DOLLAR.plus(DEFAULT_TEST_RIDE_PRICE).plus(ONE_DOLLAR).plus(FERRY_FARE) + ONE_DOLLAR.plus(ONE_DOLLAR).plus(FERRY_FARE) ); calculateFare( rides, FareType.electronicRegular, - DEFAULT_TEST_RIDE_PRICE.times(3).plus(FERRY_FARE) + DEFAULT_TEST_RIDE_PRICE.times(2).plus(FERRY_FARE) ); calculateFare( rides, FareType.electronicSenior, - ONE_DOLLAR.plus(usDollars(0.5f)).plus(ONE_DOLLAR).plus(HALF_FERRY_FARE) + ONE_DOLLAR.plus(ONE_DOLLAR).plus(HALF_FERRY_FARE) ); calculateFare(rides, FareType.electronicYouth, ZERO_USD); } @@ -303,7 +303,7 @@ public void calculateFareForKitsapFastFerryEastAgency() { calculateFare(rides, regular, TWO_DOLLARS); calculateFare(rides, FareType.senior, TWO_DOLLARS); calculateFare(rides, FareType.youth, Money.ZERO_USD); - calculateFare(rides, FareType.electronicSpecial, DEFAULT_TEST_RIDE_PRICE); + calculateFare(rides, FareType.electronicSpecial, ONE_DOLLAR); calculateFare(rides, FareType.electronicRegular, TWO_DOLLARS); calculateFare(rides, FareType.electronicSenior, ONE_DOLLAR); calculateFare(rides, FareType.electronicYouth, Money.ZERO_USD); @@ -331,15 +331,15 @@ public void calculateFareForWSFPtToTahlequah() { */ @Test public void calculateFareForLightRailLeg() { + var regularFare = usDollars(2.50f); List rides = List.of( getLeg(SOUND_TRANSIT_AGENCY_ID, "1-Line", 0, "Roosevelt Station", "Int'l Dist/Chinatown") ); - - calculateFare(rides, regular, ORCA_REGULAR_FARE); - calculateFare(rides, FareType.senior, ONE_DOLLAR); + calculateFare(rides, regular, regularFare); + calculateFare(rides, FareType.senior, regularFare); calculateFare(rides, FareType.youth, Money.ZERO_USD); calculateFare(rides, FareType.electronicSpecial, ORCA_SPECIAL_FARE); - calculateFare(rides, FareType.electronicRegular, ORCA_REGULAR_FARE); + calculateFare(rides, FareType.electronicRegular, regularFare); calculateFare(rides, FareType.electronicSenior, ONE_DOLLAR); calculateFare(rides, FareType.electronicYouth, Money.ZERO_USD); // Ensure that it works in reverse @@ -347,11 +347,11 @@ public void calculateFareForLightRailLeg() { List.of( getLeg(SOUND_TRANSIT_AGENCY_ID, "1-Line", 0, "Int'l Dist/Chinatown", "Roosevelt Station") ); - calculateFare(rides, regular, ORCA_REGULAR_FARE); - calculateFare(rides, FareType.senior, ONE_DOLLAR); + calculateFare(rides, regular, regularFare); + calculateFare(rides, FareType.senior, regularFare); calculateFare(rides, FareType.youth, Money.ZERO_USD); calculateFare(rides, FareType.electronicSpecial, ORCA_SPECIAL_FARE); - calculateFare(rides, FareType.electronicRegular, ORCA_REGULAR_FARE); + calculateFare(rides, FareType.electronicRegular, regularFare); calculateFare(rides, FareType.electronicSenior, ONE_DOLLAR); calculateFare(rides, FareType.electronicYouth, Money.ZERO_USD); } @@ -362,7 +362,7 @@ public void calculateFareForSounderLeg() { getLeg(SOUND_TRANSIT_AGENCY_ID, "S Line", 0, "King Street Station", "Auburn Station") ); calculateFare(rides, regular, usDollars(4.25f)); - calculateFare(rides, FareType.senior, ONE_DOLLAR); + calculateFare(rides, FareType.senior, usDollars(4.25f)); calculateFare(rides, FareType.youth, Money.ZERO_USD); calculateFare(rides, FareType.electronicSpecial, ORCA_SPECIAL_FARE); calculateFare(rides, FareType.electronicRegular, usDollars(4.25f)); @@ -374,7 +374,7 @@ public void calculateFareForSounderLeg() { getLeg(SOUND_TRANSIT_AGENCY_ID, "N Line", 0, "King Street Station", "Everett Station") ); calculateFare(rides, regular, usDollars(5.00f)); - calculateFare(rides, FareType.senior, ONE_DOLLAR); + calculateFare(rides, FareType.senior, usDollars(5.00f)); calculateFare(rides, FareType.youth, Money.ZERO_USD); calculateFare(rides, FareType.electronicSpecial, ORCA_SPECIAL_FARE); calculateFare(rides, FareType.electronicRegular, usDollars(5.00f)); @@ -395,9 +395,10 @@ public void calculateSoundTransitBusFares() { getLeg(KC_METRO_AGENCY_ID, "550", 240) ); calculateFare(rides, regular, usDollars(9.75f)); - calculateFare(rides, FareType.senior, usDollars(3)); + // Sound Transit does not accept senior fares in cash + calculateFare(rides, FareType.senior, usDollars(9.75f)); calculateFare(rides, FareType.youth, Money.ZERO_USD); - calculateFare(rides, FareType.electronicSpecial, usDollars(4.50f)); + calculateFare(rides, FareType.electronicSpecial, usDollars(3f)); calculateFare(rides, FareType.electronicRegular, usDollars(9.75f)); calculateFare(rides, FareType.electronicSenior, usDollars(3.00f)); calculateFare(rides, FareType.electronicYouth, Money.ZERO_USD); @@ -411,7 +412,7 @@ public void calculateSoundTransitBusFares() { calculateFare(rides, regular, DEFAULT_TEST_RIDE_PRICE.times(2)); calculateFare(rides, FareType.senior, DEFAULT_TEST_RIDE_PRICE.times(2)); calculateFare(rides, FareType.youth, Money.ZERO_USD); - calculateFare(rides, FareType.electronicSpecial, DEFAULT_TEST_RIDE_PRICE); + calculateFare(rides, FareType.electronicSpecial, usDollars(1f)); calculateFare(rides, FareType.electronicRegular, DEFAULT_TEST_RIDE_PRICE); calculateFare(rides, FareType.electronicSenior, ONE_DOLLAR); calculateFare(rides, FareType.electronicYouth, Money.ZERO_USD); @@ -427,9 +428,9 @@ public void calculateCashFreeTransferKCMetro() { getLeg(KC_METRO_AGENCY_ID, 130) ); calculateFare(rides, regular, DEFAULT_TEST_RIDE_PRICE.times(3)); - calculateFare(rides, FareType.senior, usDollars(3.25f)); + calculateFare(rides, FareType.senior, DEFAULT_TEST_RIDE_PRICE.times(2).plus(usDollars(1.25f))); calculateFare(rides, FareType.youth, Money.ZERO_USD); - calculateFare(rides, FareType.electronicSpecial, usDollars(3)); + calculateFare(rides, FareType.electronicSpecial, usDollars(1.25f)); calculateFare(rides, FareType.electronicRegular, DEFAULT_TEST_RIDE_PRICE.times(2)); calculateFare(rides, FareType.electronicSenior, usDollars(1.25f)); // Transfer extended by CT ride calculateFare(rides, FareType.electronicYouth, ZERO_USD); @@ -442,8 +443,9 @@ public void calculateTransferExtension() { getLeg(SOUND_TRANSIT_AGENCY_ID, "1-Line", 60, "Roosevelt Station", "Angle Lake Station"), // 3.25, should extend transfer getLeg(SOUND_TRANSIT_AGENCY_ID, "1-Line", 140, "Int'l Dist/Chinatown", "Angle Lake Station") // 3.00, should be free under extended transfer ); - calculateFare(rides, regular, ORCA_REGULAR_FARE.plus(usDollars(3.25f)).plus(usDollars(3.00f))); - calculateFare(rides, FareType.senior, usDollars(3)); + var regularFare = usDollars(2.50f).plus(usDollars(3.25f)).plus(usDollars(3.00f)); + calculateFare(rides, regular, regularFare); + calculateFare(rides, FareType.senior, regularFare); calculateFare(rides, FareType.youth, Money.ZERO_USD); calculateFare(rides, FareType.electronicSpecial, ORCA_SPECIAL_FARE.times(2)); calculateFare(rides, FareType.electronicRegular, usDollars(3.25f)); // transfer extended on second leg From 9a8b363630d42776f66fc29643bbab9e467750f0 Mon Sep 17 00:00:00 2001 From: Daniel Heppner Date: Tue, 10 Oct 2023 13:17:42 -0700 Subject: [PATCH 44/53] =?UTF-8?q?use=20regular=20fare=20when=20senior=20ca?= =?UTF-8?q?sh=20isn=E2=80=99t=20available?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../ext/fares/impl/OrcaFareService.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java b/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java index 0c6dbac1dc7..d7d0e4780e8 100644 --- a/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java +++ b/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java @@ -269,7 +269,7 @@ private Optional getLegFare( fareType, rideType, defaultFare, - leg.getRoute() + leg ); case regular, electronicRegular -> getRegularFare(fareType, rideType, defaultFare, leg); default -> Optional.of(defaultFare); @@ -299,7 +299,7 @@ private Optional getRegularFare( getWashingtonStateFerriesFare(route.getLongName(), fareType, defaultFare) ); case SOUND_TRANSIT_LINK, SOUND_TRANSIT_SOUNDER -> Optional.of( - getSoundTransitFare(leg, fareType, defaultFare, rideType) + getSoundTransitFare(leg, defaultFare, rideType) ); case SOUND_TRANSIT_BUS -> optionalUSD(3.25f); default -> Optional.of(defaultFare); @@ -311,7 +311,6 @@ private Optional getRegularFare( */ private Money getSoundTransitFare( Leg leg, - FareType fareType, Money defaultFare, RideType rideType ) { @@ -329,7 +328,7 @@ private Money getSoundTransitFare( .ofNullable(fareModel.get(lookupKey)) .orElseGet(() -> fareModel.get(reverseLookupKey)); - return (fare != null) ? fare.get(fareType) : defaultFare; + return (fare != null) ? fare.get(FareType.regular) : defaultFare; } /** @@ -367,8 +366,9 @@ private Optional getSeniorFare( FareType fareType, RideType rideType, Money defaultFare, - Route route + Leg leg ) { + var route = leg.getRoute(); return switch (rideType) { case COMM_TRANS_LOCAL_SWIFT -> optionalUSD(1.25f); case COMM_TRANS_COMMUTER_EXPRESS -> optionalUSD(2f); @@ -391,7 +391,7 @@ private Optional getSeniorFare( SEATTLE_STREET_CAR, KITSAP_TRANSIT -> fareType.equals(FareType.electronicSenior) ? optionalUSD(1f) - : Optional.of(defaultFare); + : getRegularFare(fareType, rideType, defaultFare, leg); case KITSAP_TRANSIT_FAST_FERRY_WESTBOUND -> fareType.equals(FareType.electronicSenior) ? optionalUSD(5f) : optionalUSD(10f); From d169ae402b865aa13b8d995f147b302d534eaadf Mon Sep 17 00:00:00 2001 From: Daniel Heppner Date: Tue, 10 Oct 2023 15:14:40 -0700 Subject: [PATCH 45/53] run formatter --- .../ext/fares/impl/OrcaFareServiceTest.java | 6 +----- .../ext/fares/impl/OrcaFareService.java | 18 +++--------------- 2 files changed, 4 insertions(+), 20 deletions(-) diff --git a/src/ext-test/java/org/opentripplanner/ext/fares/impl/OrcaFareServiceTest.java b/src/ext-test/java/org/opentripplanner/ext/fares/impl/OrcaFareServiceTest.java index 202d7627b1b..f505d001bc4 100644 --- a/src/ext-test/java/org/opentripplanner/ext/fares/impl/OrcaFareServiceTest.java +++ b/src/ext-test/java/org/opentripplanner/ext/fares/impl/OrcaFareServiceTest.java @@ -228,11 +228,7 @@ void calculateFareThatIncludesNoFreeTransfers() { ); calculateFare(rides, FareType.youth, Money.ZERO_USD); // We don't get any fares for the skagit transit leg below here because they don't accept ORCA (electronic) - calculateFare( - rides, - FareType.electronicSpecial, - ONE_DOLLAR.plus(ONE_DOLLAR).plus(FERRY_FARE) - ); + calculateFare(rides, FareType.electronicSpecial, ONE_DOLLAR.plus(ONE_DOLLAR).plus(FERRY_FARE)); calculateFare( rides, FareType.electronicRegular, diff --git a/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java b/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java index 75e1761be4d..b229dc26940 100644 --- a/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java +++ b/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java @@ -265,12 +265,7 @@ private Optional getLegFare( return switch (fareType) { case youth, electronicYouth -> Optional.of(getYouthFare()); case electronicSpecial -> getLiftFare(rideType, defaultFare, leg.getRoute()); - case electronicSenior, senior -> getSeniorFare( - fareType, - rideType, - defaultFare, - leg - ); + case electronicSenior, senior -> getSeniorFare(fareType, rideType, defaultFare, leg); case regular, electronicRegular -> getRegularFare(fareType, rideType, defaultFare, leg); default -> Optional.of(defaultFare); }; @@ -309,11 +304,7 @@ private Optional getRegularFare( /** * Calculate the correct Link fare from a "ride" including start and end stations. */ - private Money getSoundTransitFare( - Leg leg, - Money defaultFare, - RideType rideType - ) { + private Money getSoundTransitFare(Leg leg, Money defaultFare, RideType rideType) { String start = cleanStationName(leg.getFrom().name.toString()); String end = cleanStationName(leg.getTo().name.toString()); // Fares are the same no matter the order of the stations @@ -372,10 +363,7 @@ private Optional getSeniorFare( return switch (rideType) { case COMM_TRANS_LOCAL_SWIFT -> optionalUSD(1.25f); case COMM_TRANS_COMMUTER_EXPRESS -> optionalUSD(2f); - case EVERETT_TRANSIT, - SKAGIT_TRANSIT, - WHATCOM_LOCAL, - SKAGIT_LOCAL -> optionalUSD(0.5f); + case EVERETT_TRANSIT, SKAGIT_TRANSIT, WHATCOM_LOCAL, SKAGIT_LOCAL -> optionalUSD(0.5f); case KITSAP_TRANSIT_FAST_FERRY_EASTBOUND -> fareType.equals(FareType.electronicSenior) // Kitsap only provide discounted senior fare for orca. ? optionalUSD(1f) : optionalUSD(2f); From a0f7c89a127c459573e5c6d3224ea5b64f7f6abb Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Wed, 11 Oct 2023 09:52:01 +0200 Subject: [PATCH 46/53] Make Money serializable --- .../java/org/opentripplanner/transit/model/basic/Money.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/opentripplanner/transit/model/basic/Money.java b/src/main/java/org/opentripplanner/transit/model/basic/Money.java index 71dd2af328d..e6cb8f53887 100644 --- a/src/main/java/org/opentripplanner/transit/model/basic/Money.java +++ b/src/main/java/org/opentripplanner/transit/model/basic/Money.java @@ -1,5 +1,6 @@ package org.opentripplanner.transit.model.basic; +import java.io.Serializable; import java.math.BigDecimal; import java.math.RoundingMode; import java.text.NumberFormat; @@ -13,7 +14,7 @@ /** * Represents an amount of money. */ -public class Money implements Comparable { +public class Money implements Comparable, Serializable { public static final Currency USD = Currency.getInstance("USD"); public static final Money ZERO_USD = Money.usDollars(0); From faed0c20df8ec58b5044ec7e7c5abba101b4d4a5 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Wed, 11 Oct 2023 12:48:55 +0200 Subject: [PATCH 47/53] Add test for serializable Money --- .../java/org/opentripplanner/routing/core/MoneyTest.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/opentripplanner/routing/core/MoneyTest.java b/src/test/java/org/opentripplanner/routing/core/MoneyTest.java index 1a6e91e9881..5d3ed8a1b46 100644 --- a/src/test/java/org/opentripplanner/routing/core/MoneyTest.java +++ b/src/test/java/org/opentripplanner/routing/core/MoneyTest.java @@ -2,11 +2,12 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.params.provider.Arguments.of; import static org.opentripplanner.transit.model.basic.Locales.NORWEGIAN_BOKMAL; -import static org.opentripplanner.transit.model.basic.Locales.NORWEGIAN_NYNORSK; +import java.io.Serializable; import java.util.Currency; import java.util.Locale; import java.util.stream.Stream; @@ -86,4 +87,9 @@ void greaterThan() { assertFalse(oneDollar.greaterThan(oneDollar)); assertFalse(oneDollar.greaterThan(twoDollars)); } + + @Test + void serializable() { + assertInstanceOf(Serializable.class, oneDollar); + } } From 941b2bdf7c9c7466cc3713ffc3640fe45fae7ab9 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Wed, 11 Oct 2023 17:40:29 +0200 Subject: [PATCH 48/53] Remove unecessary list creation --- .../ext/fares/impl/CombinedInterlinedLegsFareServiceTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ext-test/java/org/opentripplanner/ext/fares/impl/CombinedInterlinedLegsFareServiceTest.java b/src/ext-test/java/org/opentripplanner/ext/fares/impl/CombinedInterlinedLegsFareServiceTest.java index 4998184400d..c5901a0269e 100644 --- a/src/ext-test/java/org/opentripplanner/ext/fares/impl/CombinedInterlinedLegsFareServiceTest.java +++ b/src/ext-test/java/org/opentripplanner/ext/fares/impl/CombinedInterlinedLegsFareServiceTest.java @@ -69,11 +69,11 @@ void modes(CombinationMode mode, Itinerary itinerary, Money totalPrice, String h assertEquals(totalPrice, price); var firstLeg = itinerary.getTransitLeg(0); - var uses = List.copyOf(fare.legProductsFromComponents().get(firstLeg)); + var uses = fare.legProductsFromComponents().get(firstLeg); assertEquals(1, uses.size()); var secondLeg = itinerary.getTransitLeg(1); - uses = List.copyOf(fare.legProductsFromComponents().get(secondLeg)); + uses = fare.legProductsFromComponents().get(secondLeg); assertEquals(1, uses.size()); } From 0bad5f0378ab8c118fbaeed035234f8a2364eef1 Mon Sep 17 00:00:00 2001 From: Daniel Heppner Date: Wed, 11 Oct 2023 15:38:02 -0700 Subject: [PATCH 49/53] refactor(OrcaFares): address PR concerns --- .../ext/fares/impl/OrcaFareService.java | 75 +++++++++---------- 1 file changed, 35 insertions(+), 40 deletions(-) diff --git a/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java b/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java index b229dc26940..95d23fda87c 100644 --- a/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java +++ b/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java @@ -10,6 +10,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import javax.annotation.Nullable; import org.opentripplanner.ext.fares.model.FareRuleSet; import org.opentripplanner.ext.ridehailing.model.Ride; @@ -77,7 +78,24 @@ protected enum RideType { WHATCOM_CROSS_COUNTY, SKAGIT_LOCAL, SKAGIT_CROSS_COUNTY, - UNKNOWN, + UNKNOWN; + + /** + * All transit agencies permit free transfers, apart from these. + */ + public boolean permitsFreeTransfers() { + return switch (this) { + case WASHINGTON_STATE_FERRIES, SKAGIT_TRANSIT -> false; + default -> true; + }; + } + + public boolean agencyAcceptsOrca() { + return switch (this) { + case WHATCOM_LOCAL, WHATCOM_CROSS_COUNTY, SKAGIT_CROSS_COUNTY, SKAGIT_LOCAL -> false; + default -> true; + }; + } } static RideType getRideType(String agencyId, Route route) { @@ -135,30 +153,16 @@ static RideType getRideType(String agencyId, Route route) { } case SOUND_TRANSIT_AGENCY_ID -> RideType.SOUND_TRANSIT; case EVERETT_TRANSIT_AGENCY_ID -> RideType.EVERETT_TRANSIT; - case SKAGIT_TRANSIT_AGENCY_ID -> { - try { - yield route.getShortName().equals("80X") || route.getShortName().equals("90X") - ? RideType.SKAGIT_CROSS_COUNTY - : RideType.SKAGIT_LOCAL; - } catch (NumberFormatException e) { - LOG.warn("Unable to determine skagit route id from {}.", route.getShortName(), e); - yield RideType.SKAGIT_LOCAL; - } - } + case SKAGIT_TRANSIT_AGENCY_ID -> Set.of("80X", "90X").contains(route.getShortName()) + ? RideType.SKAGIT_CROSS_COUNTY + : RideType.SKAGIT_LOCAL; case SEATTLE_STREET_CAR_AGENCY_ID -> RideType.SEATTLE_STREET_CAR; case WASHINGTON_STATE_FERRIES_AGENCY_ID -> RideType.WASHINGTON_STATE_FERRIES; case T_LINK_AGENCY_ID -> RideType.SOUND_TRANSIT_T_LINK; case KITSAP_TRANSIT_AGENCY_ID -> RideType.KITSAP_TRANSIT; - case WHATCOM_AGENCY_ID -> { - try { - yield route.getShortName().equals("80X") - ? RideType.WHATCOM_CROSS_COUNTY - : RideType.WHATCOM_LOCAL; - } catch (NumberFormatException e) { - LOG.warn("Unable to determine whatcom route id from {}.", route.getShortName(), e); - yield RideType.WHATCOM_LOCAL; - } - } + case WHATCOM_AGENCY_ID -> "80X".equals(route.getShortName()) + ? RideType.WHATCOM_CROSS_COUNTY + : RideType.WHATCOM_LOCAL; default -> RideType.UNKNOWN; }; } @@ -259,7 +263,7 @@ private Optional getLegFare( return Optional.of(defaultFare); } // Filter out agencies that don't accept ORCA from the electronic fare type - if (usesOrca(fareType) && !agencyAcceptsOrca(rideType)) { + if (usesOrca(fareType) && !rideType.agencyAcceptsOrca()) { return Optional.empty(); } return switch (fareType) { @@ -297,6 +301,12 @@ private Optional getRegularFare( getSoundTransitFare(leg, defaultFare, rideType) ); case SOUND_TRANSIT_BUS -> optionalUSD(3.25f); + case WHATCOM_LOCAL, + WHATCOM_CROSS_COUNTY, + SKAGIT_LOCAL, + SKAGIT_CROSS_COUNTY -> fareType.equals(FareType.electronicRegular) + ? Optional.empty() + : Optional.of(defaultFare); default -> Optional.of(defaultFare); }; } @@ -472,7 +482,8 @@ public boolean populateFare( Money orcaFareDiscount = Money.ZERO_USD; for (Leg leg : legs) { RideType rideType = classify(leg.getRoute(), leg.getTrip().getId().getId()); - boolean ridePermitsFreeTransfers = permitsFreeTransfers(rideType); + assert rideType != null; + boolean ridePermitsFreeTransfers = rideType.permitsFreeTransfers(); if (freeTransferStartTime == null && ridePermitsFreeTransfers) { // The start of a free transfer must be with a transit agency that permits it! freeTransferStartTime = leg.getStartTime(); @@ -621,27 +632,11 @@ private boolean inFreeTransferWindow( private boolean hasFreeTransfers(FareType fareType, RideType rideType) { // King County Metro allows transfers on cash fare return ( - (permitsFreeTransfers(rideType) && usesOrca(fareType)) || + (rideType.permitsFreeTransfers() && usesOrca(fareType)) || (rideType == RideType.KC_METRO && !usesOrca(fareType)) ); } - /** - * All transit agencies permit free transfers, apart from these. - */ - private boolean permitsFreeTransfers(RideType rideType) { - return switch (rideType) { - case WASHINGTON_STATE_FERRIES, SKAGIT_TRANSIT -> false; - default -> true; - }; - } - - private static boolean agencyAcceptsOrca(RideType rideType) { - return switch (rideType) { - case WHATCOM_LOCAL, WHATCOM_CROSS_COUNTY, SKAGIT_CROSS_COUNTY, SKAGIT_LOCAL -> false; - default -> true; - }; - } /** * Define Orca fare types. From 27699aaea46a23dd7295737575327b3bf1941121 Mon Sep 17 00:00:00 2001 From: Daniel Heppner Date: Wed, 11 Oct 2023 15:47:54 -0700 Subject: [PATCH 50/53] run formatter --- .../java/org/opentripplanner/ext/fares/impl/OrcaFareService.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java b/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java index 95d23fda87c..724de3e225f 100644 --- a/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java +++ b/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java @@ -637,7 +637,6 @@ private boolean hasFreeTransfers(FareType fareType, RideType rideType) { ); } - /** * Define Orca fare types. */ From bc8db3ae3eb833ef7b151f540f835ecdd290e105 Mon Sep 17 00:00:00 2001 From: OTP Changelog Bot Date: Thu, 12 Oct 2023 12:52:37 +0000 Subject: [PATCH 51/53] Add changelog entry for #5293 [ci skip] --- docs/Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/Changelog.md b/docs/Changelog.md index fb32a48c68f..320db13e4e9 100644 --- a/docs/Changelog.md +++ b/docs/Changelog.md @@ -15,6 +15,7 @@ based on merged pull requests. Search GitHub issues and pull requests for smalle - Move GTFS GraphQL API out of the sandbox [#5339](https://github.com/opentripplanner/OpenTripPlanner/pull/5339) - Transmodel GraphQL API for pass-through searches [#5320](https://github.com/opentripplanner/OpenTripPlanner/pull/5320) - Fix check for OSM relation members not being present in the extract [#5379](https://github.com/opentripplanner/OpenTripPlanner/pull/5379) +- Add a configurable limit for the search window [#5293](https://github.com/opentripplanner/OpenTripPlanner/pull/5293) [](AUTOMATIC_CHANGELOG_PLACEHOLDER_DO_NOT_REMOVE) ## 2.4.0 (2023-09-13) From 5d8dc22e5e3725fc47de41c65dc0831109d3f1be Mon Sep 17 00:00:00 2001 From: OTP Serialization Version Bot Date: Thu, 12 Oct 2023 14:55:22 +0000 Subject: [PATCH 52/53] Bump serialization version id for #5419 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 68d2e1088ee..bc4c0f4e81c 100644 --- a/pom.xml +++ b/pom.xml @@ -56,7 +56,7 @@ - 120 + 121 29.2 2.48.1 From 0654eb436d30da615c19352ebf5e187e9d603adf Mon Sep 17 00:00:00 2001 From: OTP Changelog Bot Date: Thu, 12 Oct 2023 17:39:10 +0000 Subject: [PATCH 53/53] Add changelog entry for #5408 [ci skip] --- docs/Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/Changelog.md b/docs/Changelog.md index 320db13e4e9..979e6579ba2 100644 --- a/docs/Changelog.md +++ b/docs/Changelog.md @@ -16,6 +16,7 @@ based on merged pull requests. Search GitHub issues and pull requests for smalle - Transmodel GraphQL API for pass-through searches [#5320](https://github.com/opentripplanner/OpenTripPlanner/pull/5320) - Fix check for OSM relation members not being present in the extract [#5379](https://github.com/opentripplanner/OpenTripPlanner/pull/5379) - Add a configurable limit for the search window [#5293](https://github.com/opentripplanner/OpenTripPlanner/pull/5293) +- Fix fare calculation for combined interlined legs [#5408](https://github.com/opentripplanner/OpenTripPlanner/pull/5408) [](AUTOMATIC_CHANGELOG_PLACEHOLDER_DO_NOT_REMOVE) ## 2.4.0 (2023-09-13)