From 356cbfd842ba79f25dd8291d019f21fa2b260db6 Mon Sep 17 00:00:00 2001 From: Joel Lappalainen Date: Tue, 10 Oct 2023 16:47:03 +0300 Subject: [PATCH 1/2] Make vehicleRentalStation query optionally accept id without feed --- docs/Configuration.md | 61 ++++++++++--------- .../apis/gtfs/datafetchers/QueryTypeImpl.java | 30 ++++++++- .../framework/application/OTPFeature.java | 5 ++ .../apis/gtfs/GraphQLIntegrationTest.java | 18 +++++- .../expectations/vehicle-rental-station.json | 12 ++++ .../queries/vehicle-rental-station.graphql | 10 +++ 6 files changed, 104 insertions(+), 32 deletions(-) create mode 100644 src/test/resources/org/opentripplanner/apis/gtfs/expectations/vehicle-rental-station.json create mode 100644 src/test/resources/org/opentripplanner/apis/gtfs/queries/vehicle-rental-station.graphql diff --git a/docs/Configuration.md b/docs/Configuration.md index 5073dafed31..5692b5f87d0 100644 --- a/docs/Configuration.md +++ b/docs/Configuration.md @@ -219,36 +219,37 @@ Here is a list of all features which can be toggled on/off and their default val -| Feature | Description | Enabled by default | Sandbox | -|--------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:------------------:|:-------:| -| `APIBikeRental` | Enable the bike rental endpoint. | ✓️ | | -| `APIServerInfo` | Enable the server info endpoint. | ✓️ | | -| `APIGraphInspectorTile` | Enable the inspector endpoint for graph information for inspection/debugging purpose. | ✓️ | | -| `APIUpdaterStatus` | Enable endpoint for graph updaters status. | ✓️ | | -| `ConsiderPatternsForDirectTransfers` | Enable limiting transfers so that there is only a single transfer to each pattern. | ✓️ | | -| `DebugClient` | Enable the debug web client located at the root of the web server. | ✓️ | | -| `FloatingBike` | Enable floating bike routing. | ✓️ | | -| `GtfsGraphQlApi` | Enable GTFS GraphQL API. | ✓️ | | -| `MinimumTransferTimeIsDefinitive` | If the minimum transfer time is a lower bound (default) or the definitive time for the transfer. Set this to `true` if you want to set a transfer time lower than what OTP derives from OSM data. | | | -| `OptimizeTransfers` | OTP will inspect all itineraries found and optimize where (which stops) the transfer will happen. Waiting time, priority and guaranteed transfers are taken into account. | ✓️ | | -| `ParallelRouting` | Enable performing parts of the trip planning in parallel. | | | -| `TransferConstraints` | Enforce transfers to happen according to the _transfers.txt_(GTFS) and Interchanges(NeTEx). Turing this _off_ will increase the routing performance a little. | ✓️ | | -| `ActuatorAPI` | Endpoint for actuators (service health status). | | ✓️ | -| `AsyncGraphQLFetchers` | Whether the @async annotation in the GraphQL schema should lead to the fetch being executed asynchronously. This allows batch or alias queries to run in parallel at the cost of consuming extra threads. | | | -| `DataOverlay` | Enable usage of data overlay when calculating costs for the street network. | | ✓️ | -| `FaresV2` | Enable import of GTFS-Fares v2 data. | | ✓️ | -| `FlexRouting` | Enable FLEX routing. | | ✓️ | -| `GoogleCloudStorage` | Enable Google Cloud Storage integration. | | ✓️ | -| `RealtimeResolver` | When routing with ignoreRealtimeUpdates=true, add an extra step which populates results with realtime data | | ✓️ | -| `ReportApi` | Enable the report API. | | ✓️ | -| `RestAPIPassInDefaultConfigAsJson` | Enable a default RouteRequest to be passed in as JSON on the REST API - FOR DEBUGGING ONLY! | | | -| `SandboxAPIGeocoder` | Enable the Geocoder API. | | ✓️ | -| `SandboxAPIMapboxVectorTilesApi` | Enable Mapbox vector tiles API. | | ✓️ | -| `SandboxAPIParkAndRideApi` | Enable park-and-ride endpoint. | | ✓️ | -| `SandboxAPITransmodelApi` | Enable Entur Transmodel(NeTEx) GraphQL API. | | ✓️ | -| `SandboxAPITravelTime` | Enable the isochrone/travel time surface API. | | ✓️ | -| `TransferAnalyzer` | Analyze transfers during graph build. | | ✓️ | -| `VehicleToStopHeuristics` | Enable improved heuristic for park-and-ride queries. | | ✓️ | +| Feature | Description | Enabled by default | Sandbox | +|--------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:------------------:|:-------:| +| `APIBikeRental` | Enable the bike rental endpoint. | ✓️ | | +| `APIServerInfo` | Enable the server info endpoint. | ✓️ | | +| `APIGraphInspectorTile` | Enable the inspector endpoint for graph information for inspection/debugging purpose. | ✓️ | | +| `APIUpdaterStatus` | Enable endpoint for graph updaters status. | ✓️ | | +| `ConsiderPatternsForDirectTransfers` | Enable limiting transfers so that there is only a single transfer to each pattern. | ✓️ | | +| `DebugClient` | Enable the debug web client located at the root of the web server. | ✓️ | | +| `FloatingBike` | Enable floating bike routing. | ✓️ | | +| `GtfsGraphQlApi` | Enable GTFS GraphQL API. | ✓️ | | +| `GtfsGraphQlApiRentalStationFuzzyMatching` | Does vehicleRentalStation query also allow ids that are not feed scoped. | | | +| `MinimumTransferTimeIsDefinitive` | If the minimum transfer time is a lower bound (default) or the definitive time for the transfer. Set this to `true` if you want to set a transfer time lower than what OTP derives from OSM data. | | | +| `OptimizeTransfers` | OTP will inspect all itineraries found and optimize where (which stops) the transfer will happen. Waiting time, priority and guaranteed transfers are taken into account. | ✓️ | | +| `ParallelRouting` | Enable performing parts of the trip planning in parallel. | | | +| `TransferConstraints` | Enforce transfers to happen according to the _transfers.txt_(GTFS) and Interchanges(NeTEx). Turing this _off_ will increase the routing performance a little. | ✓️ | | +| `ActuatorAPI` | Endpoint for actuators (service health status). | | ✓️ | +| `AsyncGraphQLFetchers` | Whether the @async annotation in the GraphQL schema should lead to the fetch being executed asynchronously. This allows batch or alias queries to run in parallel at the cost of consuming extra threads. | | | +| `DataOverlay` | Enable usage of data overlay when calculating costs for the street network. | | ✓️ | +| `FaresV2` | Enable import of GTFS-Fares v2 data. | | ✓️ | +| `FlexRouting` | Enable FLEX routing. | | ✓️ | +| `GoogleCloudStorage` | Enable Google Cloud Storage integration. | | ✓️ | +| `RealtimeResolver` | When routing with ignoreRealtimeUpdates=true, add an extra step which populates results with realtime data | | ✓️ | +| `ReportApi` | Enable the report API. | | ✓️ | +| `RestAPIPassInDefaultConfigAsJson` | Enable a default RouteRequest to be passed in as JSON on the REST API - FOR DEBUGGING ONLY! | | | +| `SandboxAPIGeocoder` | Enable the Geocoder API. | | ✓️ | +| `SandboxAPIMapboxVectorTilesApi` | Enable Mapbox vector tiles API. | | ✓️ | +| `SandboxAPIParkAndRideApi` | Enable park-and-ride endpoint. | | ✓️ | +| `SandboxAPITransmodelApi` | Enable Entur Transmodel(NeTEx) GraphQL API. | | ✓️ | +| `SandboxAPITravelTime` | Enable the isochrone/travel time surface API. | | ✓️ | +| `TransferAnalyzer` | Analyze transfers during graph build. | | ✓️ | +| `VehicleToStopHeuristics` | Enable improved heuristic for park-and-ride queries. | | ✓️ | diff --git a/src/main/java/org/opentripplanner/apis/gtfs/datafetchers/QueryTypeImpl.java b/src/main/java/org/opentripplanner/apis/gtfs/datafetchers/QueryTypeImpl.java index dfa4a60ce1c..2e6e9fe219b 100644 --- a/src/main/java/org/opentripplanner/apis/gtfs/datafetchers/QueryTypeImpl.java +++ b/src/main/java/org/opentripplanner/apis/gtfs/datafetchers/QueryTypeImpl.java @@ -33,6 +33,7 @@ import org.opentripplanner.ext.fares.impl.DefaultFareService; import org.opentripplanner.ext.fares.impl.GtfsFaresService; import org.opentripplanner.ext.fares.model.FareRuleSet; +import org.opentripplanner.framework.application.OTPFeature; import org.opentripplanner.framework.time.ServiceDateUtils; import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore; import org.opentripplanner.gtfs.mapping.DirectionMapper; @@ -841,11 +842,16 @@ public DataFetcher vehicleRentalStation() { .getContext() .vehicleRentalService(); + var id = args.getGraphQLId(); + + // TODO the fuzzy matching can be potentially removed after a while. return vehicleRentalStationService .getVehicleRentalStations() .stream() .filter(vehicleRentalStation -> - vehicleRentalStation.getId().toString().equals(args.getGraphQLId()) + OTPFeature.GtfsGraphQlApiRentalStationFuzzyMatching.isOn() + ? isFuzzyMatchRentalStationIds(vehicleRentalStation, id) + : isMatchRentalStationIds(vehicleRentalStation, id) ) .findAny() .orElse(null); @@ -890,6 +896,28 @@ public DataFetcher viewer() { return environment -> new Object(); } + /** + * This matches station's feedScopedId to the given string. + */ + private boolean isMatchRentalStationIds(VehicleRentalStation station, String feedScopedId) { + return station.getId().toString().equals(feedScopedId); + } + + /** + * This matches station's feedScopedId to the given string if the string is feed scoped (i.e + * contains a `:` separator) or only matches the station's id without the feed to the given + * string. This approach can lead to a random station matching the criteria if there are multiple + * stations with the same id in different feeds. + *

+ * TODO this can be potentially removed after a while, only used by Digitransit as of now. + */ + private boolean isFuzzyMatchRentalStationIds(VehicleRentalStation station, String idWithoutFeed) { + if (idWithoutFeed != null && idWithoutFeed.contains(":")) { + return isMatchRentalStationIds(station, idWithoutFeed); + } + return station.getId().getId().equals(idWithoutFeed); + } + private TransitService getTransitService(DataFetchingEnvironment environment) { return environment.getContext().transitService(); } diff --git a/src/main/java/org/opentripplanner/framework/application/OTPFeature.java b/src/main/java/org/opentripplanner/framework/application/OTPFeature.java index 0652fb667a8..a80bd26bad1 100644 --- a/src/main/java/org/opentripplanner/framework/application/OTPFeature.java +++ b/src/main/java/org/opentripplanner/framework/application/OTPFeature.java @@ -30,6 +30,11 @@ public enum OTPFeature { DebugClient(true, false, "Enable the debug web client located at the root of the web server."), FloatingBike(true, false, "Enable floating bike routing."), GtfsGraphQlApi(true, false, "Enable GTFS GraphQL API."), + GtfsGraphQlApiRentalStationFuzzyMatching( + false, + false, + "Does vehicleRentalStation query also allow ids that are not feed scoped." + ), /** * If this feature flag is switched on, then the minimum transfer time is not the minimum transfer * time, but the definitive transfer time. Use this to override what we think the transfer will diff --git a/src/test/java/org/opentripplanner/apis/gtfs/GraphQLIntegrationTest.java b/src/test/java/org/opentripplanner/apis/gtfs/GraphQLIntegrationTest.java index bd3c10f491e..6dd79460e8b 100644 --- a/src/test/java/org/opentripplanner/apis/gtfs/GraphQLIntegrationTest.java +++ b/src/test/java/org/opentripplanner/apis/gtfs/GraphQLIntegrationTest.java @@ -70,6 +70,7 @@ import org.opentripplanner.routing.vehicle_parking.VehicleParking; import org.opentripplanner.service.vehiclepositions.internal.DefaultVehiclePositionService; import org.opentripplanner.service.vehiclerental.internal.DefaultVehicleRentalService; +import org.opentripplanner.service.vehiclerental.model.VehicleRentalStation; import org.opentripplanner.standalone.config.framework.json.JsonSupport; import org.opentripplanner.test.support.FilePatternSource; import org.opentripplanner.transit.model._data.TransitModelForTest; @@ -216,13 +217,16 @@ public TransitAlertService getTransitAlertService() { var alerts = ListUtils.combine(List.of(alert), getTransitAlert(entitySelector)); transitService.getTransitAlertService().setAlerts(alerts); + var rentalService = new DefaultVehicleRentalService(); + rentalService.addVehicleRentalStation(vehicleRentalStation("abc")); + context = new GraphQLRequestContext( new TestRoutingService(List.of(i1)), transitService, new DefaultFareService(), graph.getVehicleParkingService(), - new DefaultVehicleRentalService(), + rentalService, new DefaultVehiclePositionService(), GraphFinder.getInstance(graph, transitService::findRegularStop), new RouteRequest() @@ -304,6 +308,18 @@ private static FareProduct fareProduct(String name) { ); } + @Nonnull + private static VehicleRentalStation vehicleRentalStation(String name) { + var station = new VehicleRentalStation(); + station.id = id(name); + station.name = I18NString.of(name); + station.longitude = 10; + station.latitude = 20; + station.vehiclesAvailable = 3; + station.spacesAvailable = 2; + return station; + } + /** * Locate 'expectations' relative to the given query input file. The 'expectations' and 'queries' * subdirectories are expected to be in the same directory. diff --git a/src/test/resources/org/opentripplanner/apis/gtfs/expectations/vehicle-rental-station.json b/src/test/resources/org/opentripplanner/apis/gtfs/expectations/vehicle-rental-station.json new file mode 100644 index 00000000000..cf4ed49c538 --- /dev/null +++ b/src/test/resources/org/opentripplanner/apis/gtfs/expectations/vehicle-rental-station.json @@ -0,0 +1,12 @@ +{ + "data" : { + "vehicleRentalStation" : { + "stationId": "F:abc", + "name" : "abc", + "lat" : 20.0, + "lon" : 10.0, + "spacesAvailable" : 2, + "vehiclesAvailable" : 3 + } + } +} \ No newline at end of file diff --git a/src/test/resources/org/opentripplanner/apis/gtfs/queries/vehicle-rental-station.graphql b/src/test/resources/org/opentripplanner/apis/gtfs/queries/vehicle-rental-station.graphql new file mode 100644 index 00000000000..836e12a9c04 --- /dev/null +++ b/src/test/resources/org/opentripplanner/apis/gtfs/queries/vehicle-rental-station.graphql @@ -0,0 +1,10 @@ +{ + vehicleRentalStation(id:"F:abc") { + stationId + name + lat + lon + spacesAvailable + vehiclesAvailable + } +} \ No newline at end of file From bce7526ce1e7263cb9cda66e7e38989e3f992e81 Mon Sep 17 00:00:00 2001 From: Joel Lappalainen Date: Tue, 10 Oct 2023 18:40:32 +0300 Subject: [PATCH 2/2] Apply review feedback --- .../apis/gtfs/datafetchers/QueryTypeImpl.java | 10 +++++----- .../apis/gtfs/GraphQLIntegrationTest.java | 20 +++++++++---------- .../expectations/vehicle-rental-station.json | 4 ++-- .../queries/vehicle-rental-station.graphql | 2 +- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/opentripplanner/apis/gtfs/datafetchers/QueryTypeImpl.java b/src/main/java/org/opentripplanner/apis/gtfs/datafetchers/QueryTypeImpl.java index 2e6e9fe219b..750cb0a4929 100644 --- a/src/main/java/org/opentripplanner/apis/gtfs/datafetchers/QueryTypeImpl.java +++ b/src/main/java/org/opentripplanner/apis/gtfs/datafetchers/QueryTypeImpl.java @@ -850,8 +850,8 @@ public DataFetcher vehicleRentalStation() { .stream() .filter(vehicleRentalStation -> OTPFeature.GtfsGraphQlApiRentalStationFuzzyMatching.isOn() - ? isFuzzyMatchRentalStationIds(vehicleRentalStation, id) - : isMatchRentalStationIds(vehicleRentalStation, id) + ? stationIdFuzzyMatches(vehicleRentalStation, id) + : stationIdMatches(vehicleRentalStation, id) ) .findAny() .orElse(null); @@ -899,7 +899,7 @@ public DataFetcher viewer() { /** * This matches station's feedScopedId to the given string. */ - private boolean isMatchRentalStationIds(VehicleRentalStation station, String feedScopedId) { + private boolean stationIdMatches(VehicleRentalStation station, String feedScopedId) { return station.getId().toString().equals(feedScopedId); } @@ -911,9 +911,9 @@ private boolean isMatchRentalStationIds(VehicleRentalStation station, String fee *

* TODO this can be potentially removed after a while, only used by Digitransit as of now. */ - private boolean isFuzzyMatchRentalStationIds(VehicleRentalStation station, String idWithoutFeed) { + private boolean stationIdFuzzyMatches(VehicleRentalStation station, String idWithoutFeed) { if (idWithoutFeed != null && idWithoutFeed.contains(":")) { - return isMatchRentalStationIds(station, idWithoutFeed); + return stationIdMatches(station, idWithoutFeed); } return station.getId().getId().equals(idWithoutFeed); } diff --git a/src/test/java/org/opentripplanner/apis/gtfs/GraphQLIntegrationTest.java b/src/test/java/org/opentripplanner/apis/gtfs/GraphQLIntegrationTest.java index 6dd79460e8b..d961de61f57 100644 --- a/src/test/java/org/opentripplanner/apis/gtfs/GraphQLIntegrationTest.java +++ b/src/test/java/org/opentripplanner/apis/gtfs/GraphQLIntegrationTest.java @@ -70,6 +70,7 @@ import org.opentripplanner.routing.vehicle_parking.VehicleParking; import org.opentripplanner.service.vehiclepositions.internal.DefaultVehiclePositionService; import org.opentripplanner.service.vehiclerental.internal.DefaultVehicleRentalService; +import org.opentripplanner.service.vehiclerental.model.TestVehicleRentalStationBuilder; import org.opentripplanner.service.vehiclerental.model.VehicleRentalStation; import org.opentripplanner.standalone.config.framework.json.JsonSupport; import org.opentripplanner.test.support.FilePatternSource; @@ -218,7 +219,7 @@ public TransitAlertService getTransitAlertService() { transitService.getTransitAlertService().setAlerts(alerts); var rentalService = new DefaultVehicleRentalService(); - rentalService.addVehicleRentalStation(vehicleRentalStation("abc")); + rentalService.addVehicleRentalStation(vehicleRentalStation()); context = new GraphQLRequestContext( @@ -309,15 +310,14 @@ private static FareProduct fareProduct(String name) { } @Nonnull - private static VehicleRentalStation vehicleRentalStation(String name) { - var station = new VehicleRentalStation(); - station.id = id(name); - station.name = I18NString.of(name); - station.longitude = 10; - station.latitude = 20; - station.vehiclesAvailable = 3; - station.spacesAvailable = 2; - return station; + private static VehicleRentalStation vehicleRentalStation() { + return TestVehicleRentalStationBuilder + .of() + .withLongitude(10) + .withLatitude(20) + .withVehicles(3) + .withSpaces(2) + .build(); } /** diff --git a/src/test/resources/org/opentripplanner/apis/gtfs/expectations/vehicle-rental-station.json b/src/test/resources/org/opentripplanner/apis/gtfs/expectations/vehicle-rental-station.json index cf4ed49c538..892b401ab7b 100644 --- a/src/test/resources/org/opentripplanner/apis/gtfs/expectations/vehicle-rental-station.json +++ b/src/test/resources/org/opentripplanner/apis/gtfs/expectations/vehicle-rental-station.json @@ -1,8 +1,8 @@ { "data" : { "vehicleRentalStation" : { - "stationId": "F:abc", - "name" : "abc", + "stationId": "Network-1:FooStation", + "name" : "FooStation", "lat" : 20.0, "lon" : 10.0, "spacesAvailable" : 2, diff --git a/src/test/resources/org/opentripplanner/apis/gtfs/queries/vehicle-rental-station.graphql b/src/test/resources/org/opentripplanner/apis/gtfs/queries/vehicle-rental-station.graphql index 836e12a9c04..fc40e65b63e 100644 --- a/src/test/resources/org/opentripplanner/apis/gtfs/queries/vehicle-rental-station.graphql +++ b/src/test/resources/org/opentripplanner/apis/gtfs/queries/vehicle-rental-station.graphql @@ -1,5 +1,5 @@ { - vehicleRentalStation(id:"F:abc") { + vehicleRentalStation(id:"Network-1:FooStation") { stationId name lat