Add via to GTFS GraphQL API

[leonardehrenfried]

Summary

We discussed this previously in the dev meeting with @optionsome. It adds the pass-through points to the plan() query of the GTFS API.

I remember that HSL also wants to use the via point routing and @optionsome was hoping it could be expressed the same input but I'm struggling to add it in an elegant way. Let's discuss this in the dev meeting.

Unit tests

Added.

[codecov]

Codecov Report

Attention: Patch coverage is 34.21053% with 50 lines in your changes missing coverage. Please review.

Project coverage is 69.71%. Comparing base (5e7b3a6) to head (d2f4108).
Report is 97 commits behind head on dev-2.x.

Files with missing lines	Patch %	Lines
...ntripplanner/apis/gtfs/generated/GraphQLTypes.java	4.16%	46 Missing ⚠️
...s/gtfs/mapping/routerequest/ViaLocationMapper.java	80.00%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             dev-2.x    #5958      +/-   ##
=============================================
- Coverage      69.74%   69.71%   -0.03%     
- Complexity     17646    17661      +15     
=============================================
  Files           2006     2008       +2     
  Lines          75529    75649     +120     
  Branches        7730     7741      +11     
=============================================
+ Hits           52678    52740      +62     
- Misses         20135    20192      +57     
- Partials        2716     2717       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[leonardehrenfried]

@optionsome The Transmodel GraphQL query is here:

OpenTripPlanner/src/main/resources/org/opentripplanner/apis/transmodel/schema.graphql

Lines 915 to 946 in a4ab59a

	viaTrip(
	"Date and time for the earliest time the user is willing to start the journey (if arriveBy=false/not set) or the latest acceptable time of arriving (arriveBy=true). Defaults to now"
	dateTime: DateTime,
	"The start location"
	from: Location!,
	"The preferable language to use for text targeted the end user. Note! The data quality is limited, only stop and quay names are translates, and not in all places of the API."
	locale: Locale = no,
	"The maximum number of trip patterns segment to return. Note! This reduces the number of trip patterns AFTER the OTP travel search is done in a post-filtering process. There is little/no performance gain in reducing the number of trip patterns returned. See also the trip meta-data on how to implement paging."
	numTripPatterns: Int = 50,
	"Use the cursor to go to the next \"page\" of itineraries. Copy the cursor from the last response and keep the original request as is. This will enable you to search for itineraries in the next or previous time-window."
	pageCursor: String,
	"""
	The length of the search-window. This parameter is optional.
	The search-window is defined as the duration between the earliest-departure-time(EDT) and the latest-departure-time(LDT). OTP will search for all itineraries in this departure window. If `arriveBy=true` the `dateTime` parameter is the latest-arrival-time, so OTP will dynamically calculate the EDT. Using a short search-window is faster than using a longer one, but the search duration is not linear. Using a "too" short search-window will waste resources server side, while using a search-window that is too long will be slow.
	OTP will dynamically calculate a reasonable value for the search-window, if not provided. The calculation comes with a significant overhead (10-20% extra). Whether you should use the dynamic calculated value or pass in a value depends on your use-case. For a travel planner in a small geographical area, with a dense network of public transportation, a fixed value between 40 minutes and 2 hours makes sense. To find the appropriate search-window, adjust it so that the number of itineraries on average is around the wanted `numItineraries`. Make sure you set the `numItineraries` to a high number while testing. For a country wide area like Norway, using the dynamic search-window is the best.
	When paginating, the search-window is calculated using the `numItineraries` in the original search together with statistics from the search for the last page. This behaviour is configured server side, and can not be overridden from the client.
	The search-window used is returned to the response metadata as `searchWindowUsed` for debugging purposes.
	"""
	searchWindow: Duration!,
	"The requests for the individual segments of the search. The first segment is from the start location(`from`) to the first entry in the `via` locations list and the last is from the last entry in the `via` locations list to the end location(`to`). Note that the list must have length of exactly one greater than the `via` field."
	segments: [ViaSegmentInput!],
	"The destination location"
	to: Location!,
	"The locations needed to be visited along the route."
	via: [ViaLocationInput!]!,
	"Whether the trip must be wheelchair accessible. Supported for the street part to the search, not implemented for the transit yet."
	wheelchairAccessible: Boolean = false
	): ViaTrip! @timingData

Do we want to paper over all these differences?

[t2gran]

While still working on the new via implementation - this is in the line of how I think the new APIs should look like. I think it would be nice if all 2 APIs did it almost the same, at least the structure:

// in planning-req :
(..., via : List<ViaLocationInput> = Ø, ...)

type ViaLocationInput {
  label : String = null
  minimumWaitTime : Duration = 0
  allowPassThrough : boolean = false [true => minimumWaitTime must be 0, coordinate is not allowed]
  place : ViaPlaceInput  
}

type ViaPlaceInput {
  @OneOf
  // Either stop, station or multi-modal-station, not group-of-stop-places
  stopLocationId : ID
  coordinate : Coordinate
}

Note! My via implementation is still work-in-progress, so I might stumble over new restrictions/use-cases altering this. Names are subjct for discussion. The allowPassThrough is not set dynamically (if minimumWaitTime=0 and place!=coordinate) because I want to be able to force alith/board at a given stop with zero wait time.

[optionsome]

What if we add another level so we can be more explicit with the restrictions (I didn't use effort on the type/field naming, we can figure out something better for them):

// in planning-req :
(..., via : List<ViaLocationInput> = Ø, ...)

type ViaLocationInput @oneOf {
  passThroughLocation: PassThroughLocationInput
  viaPoint: ViaPointInput
}

type PassThroughLocationInput {
  label : String = null # Not sure if this can be used
  stopLocationId : ID  
}

type ViaPointInput {
  label : String = null
  minimumWaitTime : Duration = 0
  place : ViaPlaceInput
}

type ViaPlaceInput {
  @OneOf
  // Either stop, station or multi-modal-station, not group-of-stop-places
  stopLocationId : ID
  coordinate : Coordinate
}

[leonardehrenfried]

I quite like @optionsome's idea here. Can we talk about it in tomorrows meeting?

[leonardehrenfried]

I implemented @optionsome's schema suggestion. This is ready for review.

[leonardehrenfried]

@optionsome Do you remember why it's no longer possible to use several stop IDs as a group? The internal implementation supports it.

[t2gran]

Can we merge this after my VIA PR? The API will work better, and you avoid differences in behaviour. The mapping to the new ViaLocation and ViaConnection types will replace the existing request PassThroughPoint.

[leonardehrenfried]

When is the new via search expected to land?

[leonardehrenfried]

It looks like Thomas will use the following names in the Transmodel API:

• ViaLocation
• PassThroughViaLocation
• VisitViaLocation

[vesameskanen]

Code looks OK to me, but I am getting an error as soon as I add a time value in visit via location:

via: [ {
      visit: {
        label: "test"
	minimumWaitTime: "5m"	
        stopLocationIds: ["HSL:1173101"]	
      }
    }]

"Exception while fetching data (/planConnection) : class org.opentripplanner.raptor.api.model.AbstractAccessEgressDecorator$1 cannot be cast to class org.opentripplanner.routing.algorithm.raptoradapter.transit.RoutingAccessEgress (org.opentripplanner.raptor.api.model.AbstractAccessEgressDecorator$1 and org.opentripplanner.routing.algorithm.raptoradapter.transit.RoutingAccessEgress are in unnamed module of loader 'app')",

Any ideas? The error might not be within the scope of this PR.

[leonardehrenfried]

I tried with this query from Tampere:

query Tampere {
  planConnection(
    origin: {location: {coordinate: {latitude: 61.4929, longitude: 23.8522}}}
    destination: {location: {coordinate: {latitude: 61.5001, longitude: 23.7535}}}
    modes: {direct: [WALK], transit: {transit: [{mode: BUS}, {mode: RAIL}]}}
    via: {visit: {stopLocationIds: ["tampere:2519"], label: "test", minimumWaitTime: "5m"}}
  ) {
    edges {
      node {
        start
        end
        legs {
          mode
          from {
            stop {
              gtfsId
            }
          }
          to {
            stop {
              gtfsId
            }
          }
          intermediateStops {
            gtfsId
          }
        }
      }
    }
  }
}

Let me merge dev-2.x. Lots of things were refactored and maybe you have some stray class file in your target folder.

[vesameskanen]

It seems that my first random search somehow found a troublesome routing challenge. After changing the endpoint coordinates, everything works fine. OTP logs NoSuchElementException: No value present exceptions but returns itineraries.

Too bad that I lost the bad coordinates when making new experiments.

[optionsome]

This probably is never thrown because of the @oneOf validation on the API. However, I don't know if it hurts to have it here.

[leonardehrenfried]

Let me know what you think: I'm happy either way.