diff --git a/doc-templates/sandbox/VehicleRentalServiceDirectory.md b/doc-templates/sandbox/VehicleRentalServiceDirectory.md new file mode 100644 index 00000000000..d9908de1a90 --- /dev/null +++ b/doc-templates/sandbox/VehicleRentalServiceDirectory.md @@ -0,0 +1,39 @@ +# Vehicle Rental Service Directory API support. + +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. + + +## 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) +- Enable GBFS geofencing with VehicleRentalServiceDirectory [#5324](https://github.com/opentripplanner/OpenTripPlanner/pull/5324) + + +## 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/Changelog.md b/docs/Changelog.md index 56d47aae729..979e6579ba2 100644 --- a/docs/Changelog.md +++ b/docs/Changelog.md @@ -14,6 +14,9 @@ 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) +- 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) diff --git a/docs/RouterConfiguration.md b/docs/RouterConfiguration.md index 1e77d3513a2..beaa45d599e 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 | @@ -67,13 +68,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* | | 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 | +| [vehicleRentalServiceDirectory](sandbox/VehicleRentalServiceDirectory.md) | `object` | Configuration for the vehicle rental service directory. | *Optional* | | 2.0 | @@ -204,6 +199,22 @@ 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. +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

**Since version:** `na` ∙ **Type:** `integer` ∙ **Cardinality:** `Optional` ∙ **Default value:** `50` @@ -415,13 +426,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:** `na` ∙ **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..8009a62f912 100644 --- a/docs/sandbox/VehicleRentalServiceDirectory.md +++ b/docs/sandbox/VehicleRentalServiceDirectory.md @@ -1,24 +1,102 @@ # Vehicle Rental Service Directory API support. +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. + + ## 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) - -## Documentation +- 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) -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 | +| [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 | + + + + +### 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. + +

networks

+ +**Since version:** `2.4` ∙ **Type:** `object[]` ∙ **Cardinality:** `Optional` +**Path:** /vehicleRentalServiceDirectory + +List all networks to include. Use "network": "default-network" to set defaults. + +If no default network exists only the listed networks are used. Configure a network with +name "default-network" to include all unlisted networks. If not present, all unlisted +networks are dropped. Note! The values in the "default-network" are not used to set +missing field values in networks listed. + + + + + + +### Example + + + + +```JSON +// router-config.json +{ + "vehicleRentalServiceDirectory" : { + "url" : "https://example.com", + "sourcesName" : "systems", + "updaterUrlName" : "url", + "updaterNetworkName" : "id", + "headers" : { + "ET-Client-Name" : "otp" + }, + "networks" : [ + { + "network" : "oslo-by-sykkel", + "geofencingZones" : true + } + ] + } +} +``` + + diff --git a/pom.xml b/pom.xml index 71a62274d1c..d0e418ae73b 100644 --- a/pom.xml +++ b/pom.xml @@ -56,14 +56,14 @@ - 120 + 121 29.2 2.48.1 2.15.2 3.1.3 5.10.0 - 1.11.4 + 1.11.5 5.5.3 1.4.11 9.8.0 @@ -579,7 +579,7 @@ com.google.cloud libraries-bom - 26.22.0 + 26.24.0 pom import @@ -954,22 +954,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 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 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..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 @@ -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 expectedPrice, String hint) { + void modes(CombinationMode mode, Itinerary itinerary, Money totalPrice, String hint) { var service = new CombinedInterlinedLegsFareService(mode); service.addFareRules( FareType.regular, @@ -65,7 +66,43 @@ 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 = fare.legProductsFromComponents().get(firstLeg); + assertEquals(1, uses.size()); + + var secondLeg = itinerary.getTransitLeg(1); + uses = 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); } } 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 385b4e96e2c..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 @@ -52,10 +52,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 final String FEED_ID = "A"; private static TestOrcaFareService orcaFareService; public static final Money DEFAULT_TEST_RIDE_PRICE = usDollars(3.49f); @@ -218,7 +217,7 @@ 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)); @@ -228,20 +227,17 @@ void calculateFareThatIncludesNoFreeTransfers() { DEFAULT_TEST_RIDE_PRICE.times(3).plus(usDollars(.50f)).plus(HALF_FERRY_FARE) ); calculateFare(rides, FareType.youth, Money.ZERO_USD); - calculateFare( - rides, - FareType.electronicSpecial, - ONE_DOLLAR.plus(DEFAULT_TEST_RIDE_PRICE).plus(ONE_DOLLAR).plus(FERRY_FARE) - ); + // 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.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); } @@ -308,7 +304,7 @@ 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); @@ -336,15 +332,15 @@ void calculateFareForWSFPtToTahlequah() { */ @Test 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 @@ -352,11 +348,11 @@ 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); } @@ -367,7 +363,7 @@ 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)); @@ -379,7 +375,7 @@ 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)); @@ -400,9 +396,10 @@ 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); @@ -416,7 +413,7 @@ 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); @@ -432,9 +429,9 @@ 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); @@ -447,8 +444,9 @@ 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 diff --git a/src/ext-test/java/org/opentripplanner/ext/vehiclerentalservicedirectory/generatedoc/VehicleRentalServiceDirectoryConfigDocTest.java b/src/ext-test/java/org/opentripplanner/ext/vehiclerentalservicedirectory/generatedoc/VehicleRentalServiceDirectoryConfigDocTest.java new file mode 100644 index 00000000000..51f72c05e3e --- /dev/null +++ b/src/ext-test/java/org/opentripplanner/ext/vehiclerentalservicedirectory/generatedoc/VehicleRentalServiceDirectoryConfigDocTest.java @@ -0,0 +1,68 @@ +package org.opentripplanner.ext.vehiclerentalservicedirectory.generatedoc; + +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 = + "org/opentripplanner/ext/vehiclerentalservicedirectory/generatedoc/" + ROUTER_CONFIG_FILENAME; + 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/ext-test/resources/org/opentripplanner/ext/vehiclerentalservicedirectory/generatedoc/router-config.json b/src/ext-test/resources/org/opentripplanner/ext/vehiclerentalservicedirectory/generatedoc/router-config.json new file mode 100644 index 00000000000..0c772551549 --- /dev/null +++ b/src/ext-test/resources/org/opentripplanner/ext/vehiclerentalservicedirectory/generatedoc/router-config.json @@ -0,0 +1,17 @@ +{ + "vehicleRentalServiceDirectory": { + "url": "https://example.com", + "sourcesName": "systems", + "updaterUrlName": "url", + "updaterNetworkName": "id", + "headers": { + "ET-Client-Name": "otp" + }, + "networks": [ + { + "network" : "oslo-by-sykkel", + "geofencingZones" : true + } + ] + } +} 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..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,7 +259,17 @@ protected boolean populateFare( var componentLegs = new ArrayList(); for (int i = start; i <= via; ++i) { - componentLegs.add(legs.get(i)); + 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(leg); + } } components.add( new FareComponent(fareId, Money.ofFractionalAmount(currency, cost), componentLegs) @@ -374,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, 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 6bc0ad668cf..724de3e225f 100644 --- a/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java +++ b/src/ext/java/org/opentripplanner/ext/fares/impl/OrcaFareService.java @@ -10,8 +10,10 @@ 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; import org.opentripplanner.framework.i18n.I18NString; import org.opentripplanner.model.fare.FareMedium; import org.opentripplanner.model.fare.FareProduct; @@ -34,12 +36,14 @@ 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"; 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( @@ -67,9 +71,31 @@ protected enum RideType { SOUND_TRANSIT, SOUND_TRANSIT_BUS, SOUND_TRANSIT_SOUNDER, + SOUND_TRANSIT_T_LINK, SOUND_TRANSIT_LINK, WASHINGTON_STATE_FERRIES, - UNKNOWN, + WHATCOM_LOCAL, + WHATCOM_CROSS_COUNTY, + SKAGIT_LOCAL, + SKAGIT_CROSS_COUNTY, + 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) { @@ -127,10 +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 -> RideType.SKAGIT_TRANSIT; + 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 -> "80X".equals(route.getShortName()) + ? RideType.WHATCOM_CROSS_COUNTY + : RideType.WHATCOM_LOCAL; default -> RideType.UNKNOWN; }; } @@ -183,9 +215,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,62 +250,71 @@ 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) { + 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) && !rideType.agencyAcceptsOrca()) { + 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, - rideType, - defaultFare, - leg.getRoute() - ); + case electronicSenior, senior -> getSeniorFare(fareType, rideType, defaultFare, leg); 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, defaultFare, rideType) ); - case SOUND_TRANSIT_BUS -> usDollars(3.25f); - default -> defaultFare; + 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); }; } /** - * 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, - FareType fareType, - 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 @@ -287,87 +329,89 @@ 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; } /** * 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 KITSAP_TRANSIT -> usDollars(1f); + 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, SOUND_TRANSIT_LINK, SOUND_TRANSIT_SOUNDER, + SOUND_TRANSIT_T_LINK, + KITSAP_TRANSIT, EVERETT_TRANSIT, - SEATTLE_STREET_CAR -> usDollars(1.5f); - case WASHINGTON_STATE_FERRIES -> getWashingtonStateFerriesFare( - route.getLongName(), - FareType.electronicSpecial, - defaultFare + PIERCE_COUNTY_TRANSIT, + SEATTLE_STREET_CAR -> optionalUSD(1.00f); + case WASHINGTON_STATE_FERRIES -> Optional.of( + getWashingtonStateFerriesFare(route.getLongName(), FareType.electronicSpecial, defaultFare) ); - 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 + Leg leg ) { + var route = leg.getRoute(); 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 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 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); - case KC_METRO, - SOUND_TRANSIT, + ? 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, - SOUND_TRANSIT_SOUNDER -> usDollars(1f); + SOUND_TRANSIT_SOUNDER, + SOUND_TRANSIT_T_LINK, + KC_METRO, + PIERCE_COUNTY_TRANSIT, + SEATTLE_STREET_CAR, + KITSAP_TRANSIT -> fareType.equals(FareType.electronicSenior) + ? optionalUSD(1f) + : getRegularFare(fareType, rideType, defaultFare, leg); 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); }; } /** - * 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, @@ -403,8 +447,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 +460,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( @@ -435,15 +482,22 @@ 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(); } - Optional singleLegPrice = getRidePrice(leg, fareType, fareRules); - Money legFare = singleLegPrice - .map(slp -> getLegFare(fareType, rideType, slp, leg)) - .orElse(Money.ZERO_USD); + Optional singleLegPrice = getRidePrice(leg, FareType.regular, fareRules); + 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() @@ -509,10 +563,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( @@ -560,8 +615,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, @@ -579,21 +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; - }; - } - /** * Define Orca fare types. */ 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( diff --git a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/VehicleRentalServiceDirectoryFetcher.java b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/VehicleRentalServiceDirectoryFetcher.java index 551dfd5c035..d51d0d70b6c 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; @@ -20,16 +24,29 @@ /** * 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 { 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, @@ -37,68 +54,127 @@ 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; - } + int maxHttpConnections = sources.size(); + var otpHttpClient = new OtpHttpClient(maxHttpConnections); - 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); + var serviceDirectory = new VehicleRentalServiceDirectoryFetcher( + vertexLinker, + repository, + otpHttpClient + ); + return serviceDirectory.createUpdatersFromEndpoint(parameters, sources); + } - var dataSource = VehicleRentalDataSourceFactory.create( - vehicleRentalParameters.sourceParameters(), - otpHttpClient - ); - GraphUpdater updater = new VehicleRentalUpdater( - vehicleRentalParameters, - dataSource, - vertexLinker, - repository + public List createUpdatersFromEndpoint( + VehicleRentalServiceDirectoryFetcherParameters parameters, + JsonNode sources + ) { + 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()); + 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); + } else { + var networkName = network.get(); + var config = parameters.networkParameters(networkName); + + if (config.isPresent()) { + var networkParams = config.get(); + dataSources.add( + new GbfsVehicleRentalDataSourceParameters( + updaterUrl.get(), + parameters.getLanguage(), + // allowKeepingRentedVehicleAtDestination - not part of GBFS, not supported here + false, + parameters.getHeaders(), + networkName, + networkParams.geofencingZones(), + // overloadingAllowed - not part of GBFS, not supported here + false + ) + ); + } else { + LOG.warn("Network not configured in OTP: {}", networkName); + } } } + 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 VehicleRentalUpdater fetchAndCreateUpdater( + GbfsVehicleRentalDataSourceParameters parameters + ) { + LOG.info("Fetched updater info for {} at url {}", parameters.network(), parameters.url()); + + VehicleRentalParameters vehicleRentalParameters = new VehicleRentalParameters( + "vehicle-rental-service-directory:" + parameters.network(), + DEFAULT_FREQUENCY, + parameters + ); + + var dataSource = VehicleRentalDataSourceFactory.create( + vehicleRentalParameters.sourceParameters(), + otpHttpClient + ); + return new VehicleRentalUpdater(vehicleRentalParameters, dataSource, vertexLinker, repository); + } + + private static JsonNode listSources(VehicleRentalServiceDirectoryFetcherParameters parameters) { + JsonNode node; + URI url = parameters.getUrl(); + try (OtpHttpClient otpHttpClient = new OtpHttpClient()) { + node = otpHttpClient.getAndMapAsJsonNode(url, Map.of(), new ObjectMapper()); + } catch (OtpHttpClientException e) { + LOG.warn("Error fetching list of vehicle rental endpoints from {}", url, e); + return MissingNode.getInstance(); + } + if (node == null) { + LOG.warn("Error reading json from {}. Node is null!", url); + return MissingNode.getInstance(); + } + + String sourcesName = parameters.getSourcesName(); + JsonNode sources = node.get(sourcesName); + if (sources == null) { + LOG.warn( + "Error reading json from {}. No JSON node for sources name '{}' found.", + url, + sourcesName + ); + return MissingNode.getInstance(); + } + return sources; + } } diff --git a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/NetworkParameters.java b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/NetworkParameters.java new file mode 100644 index 00000000000..25d394f72ec --- /dev/null +++ b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/NetworkParameters.java @@ -0,0 +1,19 @@ +package org.opentripplanner.ext.vehiclerentalservicedirectory.api; + +import org.opentripplanner.updater.vehicle_rental.datasources.params.GbfsVehicleRentalDataSourceParameters; + +/** + * Parameters for a specific network. + *

+ * The {@link GbfsVehicleRentalDataSourceParameters} support {@code overloadingAllowed} and + * {@code allowKeepingRentedVehicleAtDestination} is not included here since they are not part of + * the GBFS specification. If there is a demand for these, they can be added. + *

+ * @param network The network name + * @param geofencingZones enable geofencingZones for the given network + */ +public record NetworkParameters(String network, boolean geofencingZones) { + public NetworkParameters withName(String network) { + return new NetworkParameters(network, geofencingZones); + } +} diff --git a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/VehicleRentalServiceDirectoryFetcherParameters.java b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/VehicleRentalServiceDirectoryFetcherParameters.java index 2be6ea5d1d2..9b67114b499 100644 --- a/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/VehicleRentalServiceDirectoryFetcherParameters.java +++ b/src/ext/java/org/opentripplanner/ext/vehiclerentalservicedirectory/api/VehicleRentalServiceDirectoryFetcherParameters.java @@ -1,10 +1,16 @@ package org.opentripplanner.ext.vehiclerentalservicedirectory.api; import java.net.URI; +import java.util.Collection; +import java.util.Map; +import java.util.Optional; +import java.util.stream.Collectors; +import javax.annotation.Nullable; import org.opentripplanner.updater.spi.HttpHeaders; public class VehicleRentalServiceDirectoryFetcherParameters { + public static final String DEFAULT_NETWORK_NAME = "default-network"; private final URI url; private final String sourcesName; @@ -17,13 +23,19 @@ public class VehicleRentalServiceDirectoryFetcherParameters { private final String language; + private final Map parametersForNetwork; + + @Nullable + private final NetworkParameters defaultNetwork; + public VehicleRentalServiceDirectoryFetcherParameters( URI url, String sourcesName, String updaterUrlName, String networkName, String language, - HttpHeaders headers + HttpHeaders headers, + Collection networkParameters ) { this.url = url; this.sourcesName = sourcesName; @@ -31,6 +43,9 @@ public VehicleRentalServiceDirectoryFetcherParameters( this.sourceNetworkName = networkName; this.language = language; this.headers = headers; + this.parametersForNetwork = + networkParameters.stream().collect(Collectors.toMap(NetworkParameters::network, it -> it)); + this.defaultNetwork = parametersForNetwork.get(DEFAULT_NETWORK_NAME); } /** @@ -81,4 +96,8 @@ public HttpHeaders getHeaders() { public String getLanguage() { return language; } + + public Optional networkParameters(String network) { + return Optional.ofNullable(parametersForNetwork.getOrDefault(network, defaultNetwork)); + } } diff --git a/src/main/java/org/opentripplanner/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" diff --git a/src/main/java/org/opentripplanner/framework/json/JsonUtils.java b/src/main/java/org/opentripplanner/framework/json/JsonUtils.java new file mode 100644 index 00000000000..1f235eb6483 --- /dev/null +++ b/src/main/java/org/opentripplanner/framework/json/JsonUtils.java @@ -0,0 +1,17 @@ +package org.opentripplanner.framework.json; + +import com.fasterxml.jackson.databind.JsonNode; +import java.util.Optional; +import javax.annotation.Nonnull; + +public class JsonUtils { + + public static Optional asText(@Nonnull JsonNode node, @Nonnull String field) { + JsonNode valueNode = node.get(field); + if (valueNode == null) { + return Optional.empty(); + } + String value = valueNode.asText(); + return value.isEmpty() ? Optional.empty() : Optional.of(value); + } +} diff --git a/src/main/java/org/opentripplanner/framework/logging/AbstractFilterLogger.java b/src/main/java/org/opentripplanner/framework/logging/AbstractFilterLogger.java index e753f0bbd0c..5fe1ef4c8e7 100644 --- a/src/main/java/org/opentripplanner/framework/logging/AbstractFilterLogger.java +++ b/src/main/java/org/opentripplanner/framework/logging/AbstractFilterLogger.java @@ -15,15 +15,17 @@ * The primary use-case for this class is to prevent a spamming the log with the same kind * of events. There are two concrete implementations: *

    - *
  • {@link ThrottleLogger} - Prevent more than one log event within the same second. This is - * nice for the journey planner operation.
  • *
  • {@link MaxCountLogger} - Log N events, then mute. This is suitable for data import.
  • *
* *

* This class wrap the original logger and forward some log events, based on the * implementation of the {@link #mute()} method. + *

+ * @deprecated This hide the actual logger in the log, the AbstractFilterLogger becomes thelogger - + this make it difficult to find the log statement in the code when investigating. */ +@Deprecated public abstract class AbstractFilterLogger implements Logger { private final Logger delegate; diff --git a/src/main/java/org/opentripplanner/framework/logging/MaxCountLogger.java b/src/main/java/org/opentripplanner/framework/logging/MaxCountLogger.java index 0cfa7004f3c..05df3f3e45c 100644 --- a/src/main/java/org/opentripplanner/framework/logging/MaxCountLogger.java +++ b/src/main/java/org/opentripplanner/framework/logging/MaxCountLogger.java @@ -15,20 +15,25 @@ * message. After a given limit this logger will be muted and no more log events are logged. *

* THREAD SAFETY - The implementation is not thread safe. + *

+ * @deprecated TODO: Rewrite the same way as the {@link Throttle} is done. See + * {@link AbstractFilterLogger} for deprecation details. */ +@Deprecated public class MaxCountLogger extends AbstractFilterLogger { private static final int MAX_COUNT = 10; private int count = 0; - public MaxCountLogger(Logger delegate) { + private MaxCountLogger(Logger delegate) { super(delegate); } /** - * Wrap given logger, and throttle INFO, WARN and ERROR messages. + * Wrap given logger, and throttle INFO, WARN and ERROR messages. Maximum 10 messages is + * written to the log. */ - public static MaxCountLogger maxCount(Logger log) { + public static MaxCountLogger of(Logger log) { return new MaxCountLogger(log); } diff --git a/src/main/java/org/opentripplanner/framework/logging/ThrottleLogger.java b/src/main/java/org/opentripplanner/framework/logging/Throttle.java similarity index 55% rename from src/main/java/org/opentripplanner/framework/logging/ThrottleLogger.java rename to src/main/java/org/opentripplanner/framework/logging/Throttle.java index 6958e003357..47417aa974a 100644 --- a/src/main/java/org/opentripplanner/framework/logging/ThrottleLogger.java +++ b/src/main/java/org/opentripplanner/framework/logging/Throttle.java @@ -1,52 +1,48 @@ package org.opentripplanner.framework.logging; -import org.slf4j.Logger; +import org.opentripplanner.framework.time.TimeUtils; /** - * This class can be used to throttle logging events with level: - *

    - *
  • INFO
  • - *
  • WARNING
  • - *
  • ERROR
  • - *
- * DEBUG and TRACE events are not throttled. + * This class can be used to throttle (logging) events. *

* The primary use-case for this class is to prevent a logger for degrading the performance, * because too many events are logged during a short period of time. This could happen if you are * parsing thousands or millions of records and each of them will cause a log event to happen. *

- * This class is used to wrap the original logger and it will forward only one log event for pr - * second. + * To use it, wrap the log statement: + *

+ * THROTTLE.throttle(() -> LOG.warn("Cost mismatch ...", ...));
+ * 
+ * By wrapping the log statement only one log event will occur per second. *

* THREAD SAFETY - The implementation is very simple and do not do any synchronization, so it is * possible that more than 1 log event is logged for each second, but that is the only thread - * safety issue. It is safe to use in multi-threaded cases. See the JavaDoc on the private + * safety issue. It is safe to use in a multithreaded cases. See the JavaDoc on the private * {@code throttle()} method for implementation details. */ -public class ThrottleLogger extends AbstractFilterLogger { +public class Throttle { - private static final int STALL_PERIOD_MILLISECONDS = 1000; + private final int quietPeriodMilliseconds; private long timeout = Long.MIN_VALUE; + private final String setupInfo; - private ThrottleLogger(Logger delegate) { - super(delegate); - delegate.info( - "Logger {} is throttled, only one messages is logged for every {} second interval.", - delegate.getName(), - STALL_PERIOD_MILLISECONDS / 1000 - ); + Throttle(int quietPeriodMilliseconds) { + this.quietPeriodMilliseconds = quietPeriodMilliseconds; + this.setupInfo = "(throttle " + TimeUtils.msToString(quietPeriodMilliseconds) + " interval)"; } - /** - * Wrap given logger, and throttle INFO, WARN and ERROR messages. - */ - public static Logger throttle(Logger log) { - return new ThrottleLogger(log); + public static Throttle ofOneSecond() { + return new Throttle(1000); } - @Override - boolean mute() { - return throttle(); + public String setupInfo() { + return setupInfo; + } + + public void throttle(Runnable body) { + if (!throttle()) { + body.run(); + } } /** @@ -57,17 +53,17 @@ boolean mute() { * least one event is logged for each throttle time period. This is guaranteed based on the * assumption that writing to the {@code timeout} (primitive long) is an atomic operation. *

- * In a worst case scenario, each thread keep their local version of the {@code timeout} and one + * In the worst case scenario, each thread keep their local version of the {@code timeout} and one * log message from each thread is printed every second. This can behave differently from one JVM - * to anther. + * to another. */ - private boolean throttle() { + public boolean throttle() { long time = System.currentTimeMillis(); if (time < timeout) { return true; } - timeout = time + STALL_PERIOD_MILLISECONDS; + timeout = time + quietPeriodMilliseconds; return false; } } diff --git a/src/main/java/org/opentripplanner/framework/time/TimeUtils.java b/src/main/java/org/opentripplanner/framework/time/TimeUtils.java index b8c51ac8f62..afeeb77ff5d 100644 --- a/src/main/java/org/opentripplanner/framework/time/TimeUtils.java +++ b/src/main/java/org/opentripplanner/framework/time/TimeUtils.java @@ -168,6 +168,35 @@ public static ZonedDateTime zonedDateTime(LocalDate date, int seconds, ZoneId zo return RelativeTime.ofSeconds(seconds).toZonedDateTime(date, zoneId); } + /** + * Convert system time in milliseconds to a sting: + *

+   * -1100 -> -1.1s
+   *     0 -> 0s
+   *  1000 -> 1s
+   *  1001 -> 1.001s
+   *  1010 -> 1.01s
+   *  1100 -> 1.1s
+   * 23456 -> 23.456s
+   * 
+ */ + public static String msToString(long milliseconds) { + long seconds = milliseconds / 1000L; + int decimals = Math.abs((int) (milliseconds % 1000)); + if (decimals == 0) { + return seconds + "s"; + } + if (decimals % 10 == 0) { + decimals = decimals / 10; + if (decimals % 10 == 0) { + decimals = decimals / 10; + return seconds + "." + decimals + "s"; + } + return seconds + "." + String.format("%02d", decimals) + "s"; + } + return seconds + "." + String.format("%03d", decimals) + "s"; + } + /** * Wait (compute) until the given {@code waitMs} is past. The returned long is a very random * number. If this method is called twice a grace period of 5 times the wait-time is set. All diff --git a/src/main/java/org/opentripplanner/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/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; diff --git a/src/main/java/org/opentripplanner/netex/loader/parser/ServiceFrameParser.java b/src/main/java/org/opentripplanner/netex/loader/parser/ServiceFrameParser.java index 280b9c64669..859ec92427e 100644 --- a/src/main/java/org/opentripplanner/netex/loader/parser/ServiceFrameParser.java +++ b/src/main/java/org/opentripplanner/netex/loader/parser/ServiceFrameParser.java @@ -1,7 +1,5 @@ package org.opentripplanner.netex.loader.parser; -import static org.opentripplanner.framework.logging.MaxCountLogger.maxCount; - import jakarta.xml.bind.JAXBElement; import java.util.ArrayList; import java.util.Collection; @@ -37,7 +35,7 @@ class ServiceFrameParser extends NetexParser { private static final Logger LOG = LoggerFactory.getLogger(ServiceFrameParser.class); - private static final MaxCountLogger PASSENGER_STOP_ASSIGNMENT_LOGGER = maxCount(LOG); + private static final MaxCountLogger PASSENGER_STOP_ASSIGNMENT_LOGGER = MaxCountLogger.of(LOG); private final ReadOnlyHierarchicalMapById flexibleStopPlaceById; diff --git a/src/main/java/org/opentripplanner/raptor/rangeraptor/path/DestinationArrivalPaths.java b/src/main/java/org/opentripplanner/raptor/rangeraptor/path/DestinationArrivalPaths.java index 13ce34038ba..5f0fa376f74 100644 --- a/src/main/java/org/opentripplanner/raptor/rangeraptor/path/DestinationArrivalPaths.java +++ b/src/main/java/org/opentripplanner/raptor/rangeraptor/path/DestinationArrivalPaths.java @@ -4,7 +4,7 @@ import java.util.Collection; import javax.annotation.Nullable; import org.opentripplanner.framework.lang.OtpNumberFormat; -import org.opentripplanner.framework.logging.ThrottleLogger; +import org.opentripplanner.framework.logging.Throttle; import org.opentripplanner.raptor.api.model.RaptorAccessEgress; import org.opentripplanner.raptor.api.model.RaptorConstants; import org.opentripplanner.raptor.api.model.RaptorTripSchedule; @@ -40,7 +40,7 @@ public class DestinationArrivalPaths { private static final Logger LOG = LoggerFactory.getLogger(DestinationArrivalPaths.class); - private static final Logger LOG_MISS_MATCH = ThrottleLogger.throttle(LOG); + private static final Throttle THROTTLE_MISS_MATCH = Throttle.ofOneSecond(); private final ParetoSet> paths; private final RaptorTransitCalculator transitCalculator; @@ -207,11 +207,14 @@ private void assertGeneralizedCostIsCalculatedCorrectByMapper( ) { if (path.c1() != destArrival.c1()) { // TODO - Bug: Cost mismatch stop-arrivals and paths #3623 - LOG_MISS_MATCH.warn( - "Cost mismatch - Mapper: {}, stop-arrivals: {}, path: {}", - OtpNumberFormat.formatCostCenti(path.c1()), - raptorCostsAsString(destArrival), - path.toStringDetailed(stopNameResolver) + THROTTLE_MISS_MATCH.throttle(() -> + LOG.warn( + "Cost mismatch - Mapper: {}, stop-arrivals: {}, path: {} {}", + OtpNumberFormat.formatCostCenti(path.c1()), + raptorCostsAsString(destArrival), + path.toStringDetailed(stopNameResolver), + THROTTLE_MISS_MATCH.setupInfo() + ) ); } } diff --git a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransitTuningParameters.java b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransitTuningParameters.java index 59e4436b09f..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 @@ -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,16 @@ 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. + * 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(); + /** * 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/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/routing/api/request/RouteRequest.java b/src/main/java/org/opentripplanner/routing/api/request/RouteRequest.java index 4dabf5c0d86..0a6b2b2a6c6 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.List; import java.util.Locale; import java.util.function.Consumer; +import javax.annotation.Nullable; import org.opentripplanner.framework.time.DateUtils; import org.opentripplanner.model.GenericLocation; import org.opentripplanner.model.plan.SortOrder; @@ -58,6 +59,9 @@ public class RouteRequest implements Cloneable, Serializable { private Instant dateTime = Instant.now(); + @Nullable + private Duration maxSearchWindow; + private Duration searchWindow; private PageCursor pageCursor; @@ -330,9 +334,29 @@ public Duration searchWindow() { } public void setSearchWindow(Duration searchWindow) { + if (searchWindow != null) { + 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"); + } + } this.searchWindow = searchWindow; } + private boolean hasMaxSearchWindow() { + return maxSearchWindow != null; + } + + public Duration maxSearchWindow() { + return maxSearchWindow; + } + + public void setMaxSearchWindow(@Nullable 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/main/java/org/opentripplanner/standalone/config/routerconfig/TransitRoutingConfig.java b/src/main/java/org/opentripplanner/standalone/config/routerconfig/TransitRoutingConfig.java index 79513866e43..d4c99004a59 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,24 @@ 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. + 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). + """ + ) + .asDuration(Duration.ofHours(24)); + this.dynamicSearchWindowCoefficients = new DynamicSearchWindowConfig("dynamicSearchWindow", c); } @@ -248,6 +268,11 @@ public List transferCacheRequests() { return transferCacheRequests; } + @Override + public Duration maxSearchWindow() { + return maxSearchWindow; + } + @Override public List pagingSearchWindowAdjustments() { return pagingSearchWindowAdjustments; diff --git a/src/main/java/org/opentripplanner/standalone/config/sandbox/VehicleRentalServiceDirectoryFetcherConfig.java b/src/main/java/org/opentripplanner/standalone/config/sandbox/VehicleRentalServiceDirectoryFetcherConfig.java index 79562b28e6b..b30fc3d5b3f 100644 --- a/src/main/java/org/opentripplanner/standalone/config/sandbox/VehicleRentalServiceDirectoryFetcherConfig.java +++ b/src/main/java/org/opentripplanner/standalone/config/sandbox/VehicleRentalServiceDirectoryFetcherConfig.java @@ -1,8 +1,11 @@ package org.opentripplanner.standalone.config.sandbox; -import static org.opentripplanner.standalone.config.framework.json.OtpVersion.NA; import static org.opentripplanner.standalone.config.framework.json.OtpVersion.V2_0; +import static org.opentripplanner.standalone.config.framework.json.OtpVersion.V2_1; +import static org.opentripplanner.standalone.config.framework.json.OtpVersion.V2_4; +import java.util.List; +import org.opentripplanner.ext.vehiclerentalservicedirectory.api.NetworkParameters; import org.opentripplanner.ext.vehiclerentalservicedirectory.api.VehicleRentalServiceDirectoryFetcherParameters; import org.opentripplanner.standalone.config.framework.json.NodeAdapter; import org.opentripplanner.standalone.config.routerconfig.updaters.HttpHeadersConfig; @@ -24,24 +27,60 @@ public static VehicleRentalServiceDirectoryFetcherParameters create( } return new VehicleRentalServiceDirectoryFetcherParameters( - c.of("url").since(NA).summary("Endpoint for the VehicleRentalServiceDirectory").asUri(), + c.of("url").since(V2_1).summary("Endpoint for the VehicleRentalServiceDirectory").asUri(), c .of("sourcesName") - .since(NA) + .since(V2_1) .summary("Json tag name for updater sources.") .asString("systems"), c .of("updaterUrlName") - .since(NA) + .since(V2_1) .summary("Json tag name for endpoint urls for each source.") .asString("url"), c .of("updaterNetworkName") - .since(NA) + .since(V2_1) .summary("Json tag name for the network name for each source.") .asString("id"), - c.of("language").since(NA).summary("Language code.").asString(null), - HttpHeadersConfig.headers(c, NA) + c.of("language").since(V2_1).summary("Language code.").asString(null), + HttpHeadersConfig.headers(c, V2_1), + mapNetworkParameters("networks", c) ); } + + private static List mapNetworkParameters( + String parameterName, + NodeAdapter root + ) { + return root + .of(parameterName) + .since(V2_4) + .summary( + "List all networks to include. Use \"network\": \"" + + VehicleRentalServiceDirectoryFetcherParameters.DEFAULT_NETWORK_NAME + + "\" to set defaults." + ) + .description( + """ + If no default network exists only the listed networks are used. Configure a network with + name "{{default-network}}" to include all unlisted networks. If not present, all unlisted + networks are dropped. Note! The values in the "{{default-network}}" are not used to set + missing field values in networks listed. + """.replace( + "{{default-network}}", + VehicleRentalServiceDirectoryFetcherParameters.DEFAULT_NETWORK_NAME + ) + ) + .asObjects(c -> + new NetworkParameters( + c.of("network").since(V2_4).summary("The network name").asString(), + c + .of("geofencingZones") + .since(V2_4) + .summary("Enables geofencingZones for the given network") + .asBoolean(false) + ) + ); + } } diff --git a/src/main/java/org/opentripplanner/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); diff --git a/src/main/java/org/opentripplanner/updater/vehicle_rental/VehicleRentalUpdater.java b/src/main/java/org/opentripplanner/updater/vehicle_rental/VehicleRentalUpdater.java index efecff2eddc..1e037a7c1c2 100644 --- a/src/main/java/org/opentripplanner/updater/vehicle_rental/VehicleRentalUpdater.java +++ b/src/main/java/org/opentripplanner/updater/vehicle_rental/VehicleRentalUpdater.java @@ -10,6 +10,9 @@ import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; +import org.opentripplanner.framework.lang.ObjectUtils; +import org.opentripplanner.framework.logging.Throttle; +import org.opentripplanner.framework.time.DurationUtils; import org.opentripplanner.framework.time.TimeUtils; import org.opentripplanner.framework.tostring.ToStringBuilder; import org.opentripplanner.routing.graph.Graph; @@ -45,13 +48,18 @@ public class VehicleRentalUpdater extends PollingGraphUpdater { private static final Logger LOG = LoggerFactory.getLogger(VehicleRentalUpdater.class); + + private final Throttle unlinkedPlaceThrottle; + private final VehicleRentalDatasource source; + private final String nameForLogging; + private WriteToGraphCallback saveResultOnGraph; private Map latestModifiedEdges = Map.of(); private Set latestAppliedGeofencingZones = Set.of(); - Map verticesByStation = new HashMap<>(); - Map tempEdgesByStation = new HashMap<>(); + private final Map verticesByStation = new HashMap<>(); + private final Map tempEdgesByStation = new HashMap<>(); private final VertexLinker linker; private final VehicleRentalRepository service; @@ -64,9 +72,15 @@ public VehicleRentalUpdater( ) throws IllegalArgumentException { super(parameters); // Configure updater - LOG.info("Setting up vehicle rental updater."); + LOG.info("Setting up vehicle rental updater for {}.", source); this.source = source; + this.nameForLogging = + ObjectUtils.ifNotNull( + parameters.sourceParameters().network(), + parameters.sourceParameters().url() + ); + this.unlinkedPlaceThrottle = Throttle.ofOneSecond(); // Creation of network linker library will not modify the graph this.linker = vertexLinker; @@ -78,16 +92,19 @@ public VehicleRentalUpdater( // Do any setup if needed source.setup(); } catch (UpdaterConstructionException e) { - LOG.warn("Unable to setup updater: {}", this, e); + LOG.warn("Unable to setup updater: {}", nameForLogging, e); } if (runOnlyOnce()) { - LOG.info("Creating vehicle-rental updater running once only (non-polling): {}", source); + LOG.info( + "Creating vehicle-rental updater running once only (non-polling): {}", + nameForLogging + ); } else { LOG.info( - "Creating vehicle-rental updater running every {} seconds: {}", - pollingPeriod(), - source + "Creating vehicle-rental updater running every {}: {}", + DurationUtils.durationToStr(pollingPeriod()), + nameForLogging ); } } @@ -109,9 +126,9 @@ public String getConfigRef() { @Override protected void runPolling() { - LOG.debug("Updating vehicle rental stations from {}", source); + LOG.debug("Updating vehicle rental stations from {}", nameForLogging); if (!source.update()) { - LOG.debug("No updates"); + LOG.debug("No updates from {}", nameForLogging); return; } List stations = source.getUpdates(); @@ -149,6 +166,7 @@ public void run(Graph graph, TransitModel transitModel) { service.addVehicleRentalStation(station); stationSet.add(station.getId()); VehicleRentalPlaceVertex vehicleRentalVertex = verticesByStation.get(station.getId()); + if (vehicleRentalVertex == null) { vehicleRentalVertex = vertexFactory.vehicleRentalPlace(station); DisposableEdgeCollection tempEdges = linker.linkVertexForRealTime( @@ -168,8 +186,17 @@ public void run(Graph graph, TransitModel transitModel) { ) ); if (vehicleRentalVertex.getOutgoing().isEmpty()) { - // the toString includes the text "Bike rental station" - LOG.info("VehicleRentalPlace {} is unlinked", vehicleRentalVertex); + // Copy reference to pass into lambda + var vrv = vehicleRentalVertex; + unlinkedPlaceThrottle.throttle(() -> + // the toString includes the text "Bike rental station" + LOG.warn( + "VehicleRentalPlace is unlinked for {}: {} {}", + nameForLogging, + vrv, + unlinkedPlaceThrottle.setupInfo() + ) + ); } Set formFactors = Stream .concat( @@ -188,6 +215,7 @@ public void run(Graph graph, TransitModel transitModel) { vehicleRentalVertex.setStation(station); } } + /* remove existing stations that were not present in the update */ List toRemove = new ArrayList<>(); for (Entry entry : verticesByStation.entrySet()) { @@ -206,7 +234,7 @@ public void run(Graph graph, TransitModel transitModel) { // this check relies on the generated equals for the record which also recursively checks that // the JTS geometries are equal if (!geofencingZones.isEmpty() && !geofencingZones.equals(latestAppliedGeofencingZones)) { - LOG.info("Computing geofencing zones"); + LOG.info("Computing geofencing zones for {}", nameForLogging); var start = System.currentTimeMillis(); latestModifiedEdges.forEach(StreetEdge::removeRentalExtension); @@ -218,9 +246,10 @@ public void run(Graph graph, TransitModel transitModel) { var end = System.currentTimeMillis(); var millis = Duration.ofMillis(end - start); LOG.info( - "Geofencing zones computation took {}. Added extension to {} edges.", + "Geofencing zones computation took {}. Added extension to {} edges. For {}", TimeUtils.durationToStrCompact(millis), - latestModifiedEdges.size() + latestModifiedEdges.size(), + nameForLogging ); } } diff --git a/src/main/java/org/opentripplanner/updater/vehicle_rental/datasources/GbfsVehicleRentalDataSource.java b/src/main/java/org/opentripplanner/updater/vehicle_rental/datasources/GbfsVehicleRentalDataSource.java index 40343ec6a91..8cc8897f0de 100644 --- a/src/main/java/org/opentripplanner/updater/vehicle_rental/datasources/GbfsVehicleRentalDataSource.java +++ b/src/main/java/org/opentripplanner/updater/vehicle_rental/datasources/GbfsVehicleRentalDataSource.java @@ -22,6 +22,8 @@ import org.opentripplanner.service.vehiclerental.model.VehicleRentalPlace; import org.opentripplanner.service.vehiclerental.model.VehicleRentalSystem; import org.opentripplanner.updater.vehicle_rental.datasources.params.GbfsVehicleRentalDataSourceParameters; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Created by demory on 2017-03-14. @@ -33,11 +35,14 @@ */ class GbfsVehicleRentalDataSource implements VehicleRentalDatasource { + private static final Logger LOG = LoggerFactory.getLogger(GbfsVehicleRentalDataSource.class); + private final GbfsVehicleRentalDataSourceParameters params; + private final OtpHttpClient otpHttpClient; private GbfsFeedLoader loader; private List geofencingZones = List.of(); - private final OtpHttpClient otpHttpClient; + private boolean logGeofencingZonesDoesNotExistWarning = true; public GbfsVehicleRentalDataSource( GbfsVehicleRentalDataSourceParameters parameters, @@ -126,9 +131,18 @@ public List getUpdates() { if (params.geofencingZones()) { var zones = loader.getFeed(GBFSGeofencingZones.class); - - var mapper = new GbfsGeofencingZoneMapper(system.systemId); - this.geofencingZones = mapper.mapGeofencingZone(zones); + if (zones != null) { + var mapper = new GbfsGeofencingZoneMapper(system.systemId); + this.geofencingZones = mapper.mapGeofencingZone(zones); + } else { + if (logGeofencingZonesDoesNotExistWarning) { + LOG.warn( + "GeofencingZones is enabled in OTP, but no zones exist for network: {}", + params.network() + ); + } + logGeofencingZonesDoesNotExistWarning = false; + } } return stations; } diff --git a/src/main/java/org/opentripplanner/updater/vehicle_rental/datasources/params/VehicleRentalDataSourceParameters.java b/src/main/java/org/opentripplanner/updater/vehicle_rental/datasources/params/VehicleRentalDataSourceParameters.java index 6f64bbaf64d..abf7aa805cd 100644 --- a/src/main/java/org/opentripplanner/updater/vehicle_rental/datasources/params/VehicleRentalDataSourceParameters.java +++ b/src/main/java/org/opentripplanner/updater/vehicle_rental/datasources/params/VehicleRentalDataSourceParameters.java @@ -1,6 +1,7 @@ package org.opentripplanner.updater.vehicle_rental.datasources.params; import javax.annotation.Nonnull; +import javax.annotation.Nullable; import org.opentripplanner.updater.spi.HttpHeaders; import org.opentripplanner.updater.vehicle_rental.VehicleRentalSourceType; @@ -8,6 +9,9 @@ public interface VehicleRentalDataSourceParameters { @Nonnull String url(); + @Nullable + String network(); + @Nonnull VehicleRentalSourceType sourceType(); diff --git a/src/test/java/org/opentripplanner/framework/json/JsonUtilsTest.java b/src/test/java/org/opentripplanner/framework/json/JsonUtilsTest.java new file mode 100644 index 00000000000..4f50e9ad1a5 --- /dev/null +++ b/src/test/java/org/opentripplanner/framework/json/JsonUtilsTest.java @@ -0,0 +1,35 @@ +package org.opentripplanner.framework.json; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.MissingNode; +import com.fasterxml.jackson.databind.node.NullNode; +import com.fasterxml.jackson.databind.node.TextNode; +import java.util.Optional; +import org.junit.jupiter.api.Test; + +class JsonUtilsTest { + + private static final ObjectMapper MAPPER = new ObjectMapper(); + + @Test + void testAsText() throws JsonProcessingException { + assertTrue(JsonUtils.asText(MissingNode.getInstance(), "any").isEmpty()); + assertTrue(JsonUtils.asText(NullNode.getInstance(), "any").isEmpty()); + assertTrue(JsonUtils.asText(new TextNode("foo"), "bar").isEmpty()); + + JsonNode node = MAPPER.readTree(""" + { "foo" : "bar", "array" : [] } + """); + + Optional result = JsonUtils.asText(node, "foo"); + assertTrue(result.isPresent()); + assertEquals("bar", result.get()); + + assertTrue(JsonUtils.asText(node, "array").isEmpty()); + } +} diff --git a/src/test/java/org/opentripplanner/framework/logging/ThrottleLoggerTest.java b/src/test/java/org/opentripplanner/framework/logging/ThrottleLoggerTest.java deleted file mode 100644 index 2c0a9f2b19d..00000000000 --- a/src/test/java/org/opentripplanner/framework/logging/ThrottleLoggerTest.java +++ /dev/null @@ -1,48 +0,0 @@ -package org.opentripplanner.framework.logging; - -import java.util.ArrayList; -import java.util.List; -import org.junit.jupiter.api.Disabled; -import org.junit.jupiter.api.Test; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -class ThrottleLoggerTest { - - private static final Logger LOG = LoggerFactory.getLogger(ThrottleLoggerTest.class); - private static final Logger THROTTLED_LOG = ThrottleLogger.throttle(LOG); - - @Test - @Disabled("Run this test manually") - void warn() { - List events = new ArrayList<>(); - for (int i = 0; i < 50_000_000; i++) { - events.add(i); - } - long start = System.currentTimeMillis(); - - events - .parallelStream() - .forEach(i -> - THROTTLED_LOG.warn(String.format("%3.0f p", (System.currentTimeMillis() - start) / 1000.0)) - ); - /* - EXPECTED OUTPUT - - 21:28:36.618 INFO (LogThrottle.java:30) Logger org.opentripplanner.util.logging.LogThrottleTest is throttled, only one messages is logged for every 1 second interval. - 21:28:38.812 WARN (LogThrottle.java:264) 0 p - 21:28:39.812 WARN (LogThrottle.java:264) 1 p - 21:28:40.812 WARN (LogThrottle.java:264) 2 p - 21:28:41.812 WARN (LogThrottle.java:264) 3 p - 21:28:42.812 WARN (LogThrottle.java:264) 4 p - 21:28:42.812 WARN (LogThrottle.java:264) 4 p - 21:28:43.812 WARN (LogThrottle.java:264) 5 p - 21:28:44.812 WARN (LogThrottle.java:264) 6 p - 21:28:45.812 WARN (LogThrottle.java:264) 7 p - 21:28:46.812 WARN (LogThrottle.java:264) 8 p <--- Duplicate for the 8. period - 21:28:46.812 WARN (LogThrottle.java:264) 8 p <--- Duplicate for the 8. period - 21:28:47.812 WARN (LogThrottle.java:264) 9 p - 21:28:48.812 WARN (LogThrottle.java:264) 10 p - */ - } -} diff --git a/src/test/java/org/opentripplanner/framework/logging/ThrottleTest.java b/src/test/java/org/opentripplanner/framework/logging/ThrottleTest.java new file mode 100644 index 00000000000..91f1667486d --- /dev/null +++ b/src/test/java/org/opentripplanner/framework/logging/ThrottleTest.java @@ -0,0 +1,80 @@ +package org.opentripplanner.framework.logging; + +import java.util.ArrayList; +import java.util.List; +import java.util.Locale; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; + +class ThrottleTest { + + @Test + void testSetUp() { + Assertions.assertEquals("(throttle 1s interval)", Throttle.ofOneSecond().setupInfo()); + } + + @Test + void smokeTest() { + int SIZE = 2_000; + var counter = new AtomicInteger(0); + var events = createIntegerSequence(SIZE); + var subject = Throttle.ofOneSecond(); + + events.parallelStream().forEach(i -> subject.throttle(counter::incrementAndGet)); + Assertions.assertTrue( + counter.get() > 0, + "The counter should be greater than 0: " + counter.get() + ); + Assertions.assertTrue( + counter.get() < 100, + "The counter should be less than 10: " + counter.get() + ); + } + + @Test + @Disabled("Run this test manually") + void manualTest() { + double quietPeriodMs = 50.0; + var subject = new Throttle((int) quietPeriodMs); + + List events = createIntegerSequence(20_000_000); + long start = System.currentTimeMillis(); + + events + .parallelStream() + .forEach(i -> + subject.throttle(() -> + System.err.printf(Locale.ROOT, "%d ms%n", (System.currentTimeMillis() - start)) + ) + ); + /* + We get a lot of duplicates here because of "optimistic read/write" on shared memory - this is ok, as long as + it does not fail. + + EXPECTED OUTPUT + 4 ms + 54 ms + 54 ms // Duplicate + : + 104 ms + 104 ms // Duplicate + : + 155 ms + 206 ms + 256 ms // Duplicate + 306 ms + 306 ms // Duplicate + : + */ + } + + private List createIntegerSequence(int size) { + List events = new ArrayList<>(); + for (int i = 0; i < size; i++) { + events.add(i); + } + return events; + } +} diff --git a/src/test/java/org/opentripplanner/framework/time/TimeUtilsTest.java b/src/test/java/org/opentripplanner/framework/time/TimeUtilsTest.java index 4d131a5fc1f..53a6e5d4be4 100644 --- a/src/test/java/org/opentripplanner/framework/time/TimeUtilsTest.java +++ b/src/test/java/org/opentripplanner/framework/time/TimeUtilsTest.java @@ -166,6 +166,22 @@ public void toZonedDateTimeDST() { ); } + @Test + void testMsToString() { + assertEquals("0s", TimeUtils.msToString(0)); + assertEquals("0.001s", TimeUtils.msToString(1)); + assertEquals("0.012s", TimeUtils.msToString(12)); + assertEquals("1s", TimeUtils.msToString(1000)); + assertEquals("1.1s", TimeUtils.msToString(1100)); + assertEquals("1.02s", TimeUtils.msToString(1020)); + assertEquals("1.003s", TimeUtils.msToString(1003)); + assertEquals("1.234s", TimeUtils.msToString(1234)); + + // Negative numbers + assertEquals("-1s", TimeUtils.msToString(-1000)); + assertEquals("-1.234s", TimeUtils.msToString(-1234)); + } + private static int time(int hour, int min, int sec) { return 60 * (60 * hour + min) + sec; } diff --git a/src/test/java/org/opentripplanner/generate/doc/RouterConfigurationDocTest.java b/src/test/java/org/opentripplanner/generate/doc/RouterConfigurationDocTest.java index ac6f02d9f83..d13f423ffc4 100644 --- a/src/test/java/org/opentripplanner/generate/doc/RouterConfigurationDocTest.java +++ b/src/test/java/org/opentripplanner/generate/doc/RouterConfigurationDocTest.java @@ -36,6 +36,7 @@ public class RouterConfigurationDocTest { .skip("vectorTileLayers", "sandbox/MapboxVectorTilesApi.md") .skipNestedElements("transferCacheRequests", "RouteRequest.md") .skip("rideHailingServices", "sandbox/RideHailing.md") + .skip("vehicleRentalServiceDirectory", "sandbox/VehicleRentalServiceDirectory.md") .build(); /** diff --git a/src/test/java/org/opentripplanner/graph_builder/module/osm/OsmDatabaseTest.java b/src/test/java/org/opentripplanner/graph_builder/module/osm/OsmDatabaseTest.java new file mode 100644 index 00000000000..8c5671d3bf0 --- /dev/null +++ b/src/test/java/org/opentripplanner/graph_builder/module/osm/OsmDatabaseTest.java @@ -0,0 +1,51 @@ +package org.opentripplanner.graph_builder.module.osm; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +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. + * This test assert that this is really happening. + */ + @Test + void bicycleRouteRelations() { + var osmdb = new OsmDatabase(DataImportIssueStore.NOOP); + var provider = new OsmProvider(RESOURCE_LOADER.file("ehningen-minimal.osm.pbf"), true); + provider.readOSM(osmdb); + osmdb.postLoad(); + + var way = osmdb.getWay(13876983L); + assertNotNull(way); + + assertEquals("yes", way.getTag("lcn")); + assertEquals("Gärtringer Weg", way.getTag("name")); + } + + /** + * 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 file = RESOURCE_LOADER.file("brenner-invalid-relation-reference.osm.pbf"); + var provider = new OsmProvider(file, true); + provider.readOSM(osmdb); + osmdb.postLoad(); + + var way = osmdb.getWay(302732658L); + assertNotNull(way); + assertEquals("platform", way.getTag("public_transport")); + } +} 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 3b80f0f74a4..00000000000 --- a/src/test/java/org/opentripplanner/openstreetmap/model/BicycleNetworkRelationsTest.java +++ /dev/null @@ -1,32 +0,0 @@ -package org.opentripplanner.openstreetmap.model; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; - -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; -import org.opentripplanner.test.support.ResourceLoader; - -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(ResourceLoader.of(this).file("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/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); + } } diff --git a/src/test/java/org/opentripplanner/routing/core/RouteRequestTest.java b/src/test/java/org/opentripplanner/routing/core/RouteRequestTest.java index a5c532cd323..b18c12e0485 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,12 @@ class RouteRequestTest { + 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(); + @Test public void testRequest() { // TODO VIA: looks like some parts of this test are obsolete since method no longer exist @@ -110,6 +118,37 @@ 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(); + request.setMaxSearchWindow(DURATION_24_HOURS); + 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()); } 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()); } } 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() { 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 00000000000..250e4da1a32 Binary files /dev/null and b/src/test/resources/org/opentripplanner/graph_builder/module/osm/brenner-invalid-relation-reference.osm.pbf differ 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