Closed t2gran closed 2 years ago
During our breakdown of this issue we ran into some questions:
- The router object present in the RoutingService.route method - Does it have the same Lifecycle as the RoutingService such that it can be a field in the routing service class instead of a parameter in the route method?
You can do as you like. It looks strange, but it is part of a larger refactoring and touches the work I am doing as well, when I am done the RoutingService will be gone.
- Should we separate the RouteRequest and RoutePreferences into one per Domain - such that there is a RouteRequest specific to the API domain, and one specific to the Raptor domain? a. One per "Domain"? The suggestions for the Domains would be API and Algorithm maybe?
My thoughts:
There are a few domains: REST API, 2 Graph QL APIs, RoutingService(glue on top of Street Model and Transit Model), Street Model (AStar graph), Transit Model (Raptor transit data), AStart, Raptor and so on... Right now the bounderies are not clear . The classes above replaces the org.opentripplanner.routing.api.request.RoutingRequest
and can be put in the same package. Everything behind is just refactored, @hannesj and I can contribute to cleanup the models behind afterwords (need the transit model refactoring I am doing in place first). The GraphQL APIs should not change - except adding new methods - mostly mapping. But, for the new unified API we will make it much more like the above design. For the REST API we will add support for intermediate places if requested - the org. requesting it will need to add it.
- Should the RouteRequest have some sort of interface design? a. RouteRequest interface b. DefaultRouteRequest that implements RouteRequest c. RouteViaRequest that implements RouteRequest
No. But, yes. Keep in mind that the Request classes are DTOs, so putting interfaces on top does not serve a purpose. But, we do not want to duplicate these classes in another form when passing the information down to the different domains, so the trick is to let each model define its own API interfaces and then let the RouteRequest
play the role. Here illustrated a bit generic - not saying how (adaptor, bridge or inheritance):
But for now, just keep the existing mapping code, this is a refactoring we will do gradually.
Here is a revised edition of the model
Updated Model:
package org.opentripplanner.routing.api.request;
interface RoutingService {
RoutingResponse route(RouteRequest request);
RoutingResponse routeVia(RouteViaRequest request, RoutingPreferences preferences);
}
class RouteRequest {
GenericLocation from;
GenericLocation to;
Instant dateTime = Instant.now();
Duration searchWindow;
PageCursor pageCursor;
boolean timetableView = true;
boolean arriveBy = false;
int numItineraries = 50;
Locale locale = new Locale("en", "US");
RoutingPreferences preferences = new RoutingPreferences();
JourneyRequest journey = new JourneyRequest();
ItineraryFilterParameters itineraryFilters = ItineraryFilterParameters.createDefault();
DataOverlayParameters dataOverlay;
boolean wheelchair = false;
};
/**
* User/trip cost/time/slack/reluctance search config.
*/
class RoutingPreferences {
TransitPreferences transit;
TransferPreferences transfer;
StreetPreferences street;
WalkPreferences walk;
WheelchairAccessibilityPreferences wheelchairAccessibility = WheelchairAccessibilityPreferences.DEFAULT
BikePreferences bike;
CarPreferences car;
VehicleRentalPreferences rental;
SystemPreferences system;
}
class TransitPreferences {
int walkBoardCost = 60 * 10;
int bikeBoardCost = 60 * 10;
int boardSlack = 0;
Map<TransitMode, Integer> boardSlackForMode = new EnumMap<TransitMode, Integer>(TransitMode.class);
int alightSlack = 0;
Map<TransitMode, Integer> alightSlackForMode = new EnumMap<TransitMode, Integer>(TransitMode.class);
Map<TransitMode, Double> reluctanceForMode = new HashMap<>();
int otherThanPreferredRoutesPenalty = 300;
DoubleFunction<Double> unpreferredCost = RequestFunctions.createLinearFunction(0.0,DEFAULT_ROUTE_RELUCTANCE);
boolean ignoreRealtimeUpdates = false;
boolean includePlannedCancellations = false;
RaptorOptions raptorOptions = new RaptorOptions();
};
class TransitRequest {
List<AllowedTransitMode> modes = AllowedTransitMode.getAllTransitModes();
List<FeedScopedId> whiteListedAgencies = List.of();
List<FeedScopedId> bannedAgencies = List.of();
List<FeedScopedId> preferredAgencies = List.of();
List<FeedScopedId> unpreferredAgencies = List.of();
RouteMatcher whiteListedRoutes = RouteMatcher.emptyMatcher();
RouteMatcher bannedRoutes = RouteMatcher.emptyMatcher();
List<FeedScopedId> preferredRoutes = RouteMatcher.emptyMatcher();
List<FeedScopedId> RouteMatcher unpreferredRoutes = RouteMatcher.emptyMatcher();
List<FeedScopedId> bannedTrips = List.of();
DebugRaptor raptorDebugging = new DebugRaptor();
}
class TransferPreferences {
int cost = 0;
int slack = 120;
int nonpreferredCost = 180;
double waitReluctance = 1.0;
double waitAtBeginningFactor = 0.4;
TransferOptimizationParameters optimization = new TransferOptimizationRequest();
Integer maxTransfers = 12;
}
class WalkPreferences {
double speed = 1.33;
double reluctance = 2.0;
double stairsReluctance = 2.0;
double stairsTimeFactor = 3.0;
double safetyFactor = 1.0;
}
// Direct street search
class StreetPreferences {
Duration maxAccessEgressDuration = Duration.ofMinutes(45);
Map<StreetMode, Duration> maxAccessEgressDurationForMode = new HashMap<>();
Duration maxDirectStreetDuration = Duration.ofHours(4);
Map<StreetMode, Duration> maxDirectStreetDurationForMode = new HashMap<>();
double turnReluctance = 1.0;
int elevatorBoardTime = 90;
int elevatorBoardCost = 90;
int elevatorHopTime = 20;
int elevatorHopCost = 20;
}
class WheelchairAccessibilityRequest {
WheelchairAccessibilityFeature trip;
WheelchairAccessibilityFeature stop;
WheelchairAccessibilityFeature elevator;
double inaccessibleStreetReluctance;
double maxSlope;
double slopeExceededReluctance;
double stairsReluctance;
}
record TriangleFactor {
double time;
double slope;
double safety;
}
class BikePreferences {
BicycleOptimizeType optimizeType = BicycleOptimizeType.SAFE;
double speed = 5;
double walkingSpeed = 1.33;
double walkingReluctance = 5.0;
double reluctance = 2.0;
int switchTime;
int switchCost;
int parkTime = 60;
int parkCost = 120;
TriangleFactor triangleFactor;
}
class CarPreferences {
double speed = 40.0;
double reluctance = 2.0;
int parkTime = 60;
int parkCost = 120;
int dropoffTime = 120;
int pickupTime = 60;
int pickupCost = 120;
double decelerationSpeed = 2.9;
double accelerationSpeed = 2.9;
}
class VehicleRentalPreferences {
int pickupTime = 60;
int pickupCost = 120;
int dropoffTime = 30;
int dropoffCost = 30;
double keepingVehicleAtDestinationCost = 0;
}
class VehicleRentalRequest {
Set<String> allowedNetworks = Set.of();
Set<String> bannedNetworks = Set.of();
boolean useAvailabilityInformation = false;
// this is renamed allowKeepingVehicleAtDestination
boolean allowArrivingInRentedVehicleAtDestination = false;
}
class VehicleParkingRequest {
Set<String> requiredTags = Set.of();
Set<String> bannedTags = Set.of();
boolean useAvailabilityInformation = false;
boolean parkAndRide = false
}
/** System config & debug, available on API for testing purposes */
class SystemPreferences {
Tags tags = Tags.of();
boolean geoidElevation=false;
private Duration maxJourneyDuration = Duration.ofHours(24);
}
class JourneyRequest {
// initial value should be null
TransitRequest transit = null;
StreetRequest access = null;
StreetRequest egress = null;
StreetRequest transfer = null;
StreetRequest direct = null;
// These parameters control transit and direct search
boolean hasTransit() { return transit != null; }
boolean hasDirect() { return direct != null; }
// Set transit, access, egress and transfer nonnull
JourneyRequest enableTransit();
// Set transit, access, egress and transfer nonnull
JourneyRequest enableDirect();
}
class StreetRequest {
StreetMode mode = StreetMode.WALK;
TraverseModeSet traverseModeSet;
VehicleRentalRequest vehicleRental;
VehicleParkingRequest vehicleParking;
}
/**
* User request: from/to time/location; preferences slack/cost/reluctance
*/
class RouteViaRequest {
Instant dateTime = Instant.now();
GenericLocation from;
GenericLocation to;
List<ViaLocation> viaPoints;
// Number of trips must match viaPoints + 1 and is limited to max 5 trips
List<JourneyRequest> viaJourneys;
Duration searchWindow;
PageCursor pageCursor;
// boolean timetableView = true; ONLY SUPPORT true
boolean arriveBy = false;
Locale locale = new Locale("en", "US");
int numItineraries = 50;
}
class ViaLocation {
GenericLocation point;
boolean passThroughPoint = false;
Duration slack = Duration.ofMinutes(60);
}
class DELETE_THESE {
boolean onlyTransitTrips=false;
boolean disableAlertFiltering=false;
TraverseModeSet streetSubRequestModes = new TraverseModeSet(TraverseMode.WALK); // defaults in constructor overwrite this
FeedScopedId startingTransitStopId;
FeedScopedId startingTransitTripId;
int otherThanPreferredRoutesPenalty = 300;
int useUnpreferredRoutesPenalty = 300;
String pathComparator;
Set<RentalVehicleType.FormFactor> allowedFormFactors
// The REST API should hold onto this, and pass it to the mapper, no need to include it in the
// request.
boolean showIntermediateStops=false;
private boolean disableAlertFiltering = false;
}
Request / response outline
We do not want to extend the existing request to support via search, we want to refactor the existing
RoutingRequest
so different parts of it can be reused in the "normal" and "via" request. Note! This do not apply to the http endpoint APIs, because we may want to add via search to be backwards compatible.I am not sure if we need to do the same exercise for the response.
Request Design
Suggested refactoring - I have taken the
RoutingRequest
and divided it into smaller classes. Fields names are updated in some cases where the old field name repeat the context. E.gbikeSpeed
->BikePreferences{ speed }
. I have taken all fields and moved them into separate classes. We can use records for some of these classes to simplify a bit, That is up to the person implementing this.I have moved unused fields into the DELETE_THESE class. These fields should be deleted.
The JavaDoc is not copied into this, but should of cause follow the fields in the refactoring. For records, adde the JavaDoc to the field, and for DTO classes add the JavaDoc to the getter. Avoid using the existing pattern with public fields.
Naming
Model (draft, see updated version below)
```Java package org.opentripplanner.routing.api.request; interface RoutingService { RoutingResponse route(RouteRequest request, RoutingPreferences preferences); RoutingResponse routeVia(RouteViaRequest request, RoutingPreferences preferences); } class RouteRequest { Instant dateTime = Instant.now(); GenericLocation from; GenericLocation to; Duration searchWindow; PageCursor pageCursor; boolean timetableView = true; boolean arriveBy = false; Locale locale = new Locale("en", "US"); int numItineraries = 50; }; /** * User request: from/to time/location; preferences slack/cost/reluctance */ class RouteViaRequest { Instant dateTime = Instant.now(); GenericLocation from; GenericLocation to; ListLeft for later
Apply inversion of control to the request and the routing implementation. The different part of the routing engine should not depend on the Request classes, but define Parameter interfaces witch the Request classes can implement.