Skip to content

Changes to the StopTimes method used by the graphql APIs - #3576

Merged
gmellemstrand merged 10 commits into
opentripplanner:dev-2.xfrom
entur:otp2_stoptimes_refactor
Aug 13, 2021
Merged

Changes to the StopTimes method used by the graphql APIs#3576
gmellemstrand merged 10 commits into
opentripplanner:dev-2.xfrom
entur:otp2_stoptimes_refactor

Conversation

@gmellemstrand

@gmellemstrand gmellemstrand commented Aug 4, 2021

Copy link
Copy Markdown
Contributor

Summary

This builds on top of #3571 and #3575. It changes the retrieval of StopTimes via both GraphQL APIs in the following ways:

  • You can specify if you want to include only departures, only arrivals or both.
  • You can specify if you want to include cancellations or not. This includes planned cancellations, realtime cancellations of an entire Trip and realtime cancellations of a specific StopTime.

The main use case for this is generating "departure boards": lists of all planned or realtime-created vehicle departures on any pattern passing through a given stop, optionally with information about planned vehicle departures that have been cancelled or changed.

Issue

#3119

Unit tests

No new unit tests. Functionality has been tested manually by creating test Siri messages.

Documentation

Added javadoc to StopTimesHelper and expanded documentation for the Transmodel api.

Changelog

Changelog added

@gmellemstrand gmellemstrand added the Entur Test This is currently being tested at Entur label Aug 4, 2021
@gmellemstrand
gmellemstrand requested a review from a team as a code owner August 4, 2021 13:41

@abyrd abyrd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I am only requesting some changes to the PR description for future reference. The title and description should be changed to clarify what exactly the expression "StopTimes call" refers to. It appears from code that it refers to the legacy and Transmodel GraphQL APIs. The description says it builds on #3571 which is true, but it also builds on #3575. Knowing this helps isolate the commits unique to this PR for review, and it can be useful to state the commit hash where they diverge. We can complete and merge this during today's meeting.

@abyrd abyrd changed the title Changes to the StopTimes call Changes to StopTimes in GraphQL APIs Aug 12, 2021
@gmellemstrand gmellemstrand changed the title Changes to StopTimes in GraphQL APIs Changes to the StopTimes method used by the graphql APIs Aug 12, 2021

// Realtime patterns may have been cancelled and replaced. Do not include the patterns that
// have been replaced
Collection<TripPattern> replacedPatterns = realTimePatterns

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be replaced with built-in method realtimePatterns.removeIf(tp -> tp.getOriginalTripPattern() != null)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed a detail during the discussion. Gard looked into this: "it is not so easy to replace the removal logic with removeIf. It doesn’t remove the patterns that have an original pattern reference, but all the patterns that are referred to as original patterns by other patterns."

abyrd
abyrd previously approved these changes Aug 12, 2021

@abyrd abyrd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed in detail during the call, and I reviewed core logic in StopTimesHelper around line 260.

@gmellemstrand
gmellemstrand merged commit 825da39 into opentripplanner:dev-2.x Aug 13, 2021
@gmellemstrand
gmellemstrand deleted the otp2_stoptimes_refactor branch August 13, 2021 07:16
@t2gran t2gran added this to the 2.1 milestone Sep 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Entur Test This is currently being tested at Entur

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants