Add HSL specific fare computation in the sandbox #4414

nurmAV · 2022-08-25T12:20:33Z

Summary

Adds HSL specific fare computation previously implemented in the HSL fork for OTP 1.

Implements the HSLFareServiceImpl and HSLFareServiceFactory that override the default implementation to fit HSL needs
Implements the ticket type query in the legacy GraphQL API
Adds some new data structures to support HSL specific fare rules and makes some methods it the default implementations protected instead of private so that they can be overridden.

Unit tests

The changes include new unit tests for HSL fares
The correctness of the fares output has also been validated using the HSL journey planner UI.

src/main/java/org/opentripplanner/common/model/T3.java

src/main/java/org/opentripplanner/routing/core/TicketType.java

leonardehrenfried · 2022-08-25T12:38:38Z

src/main/java/org/opentripplanner/routing/core/FareRuleSet.java

@@ -33,9 +37,20 @@ public Set<P2<String>> getOriginDestinations() {
    return originDestinations;
  }

+  public void addRouteOriginDestination(String route, String origin, String destination) {


Can you add some information about what these methods mean?

This is used for special routes in HSL where some tickets can be used outside the HSL service area when travelling between some origin and destination on a certain route. All three pieces of information are needed for determining whether a special route rule needs to be applied when computing fare information

src/main/java/org/opentripplanner/common/model/P3.java

leonardehrenfried · 2022-08-25T12:46:15Z

You need to reformat your code with mvn prettier:write.

src/main/java/org/opentripplanner/routing/core/FareRuleSet.java

hannesj · 2022-08-25T14:22:17Z

Please add documentation

src/ext/java/org/opentripplanner/ext/fares/impl/HSLFareServiceFactory.java

leonardehrenfried · 2022-08-25T20:17:13Z

Also, please add some contact info to this page: https://github.com/opentripplanner/OpenTripPlanner/blob/dev-2.x/docs/sandbox/Fares.md#custom-fare-calculators

src/ext/java/org/opentripplanner/ext/fares/impl/HSLFareServiceImpl.java

codecov-commenter · 2022-08-30T13:23:37Z

Codecov Report

Merging #4414 (d4d4869) into dev-2.x (b23dcbc) will decrease coverage by 0.04%.
The diff coverage is 38.81%.

@@              Coverage Diff              @@
##             dev-2.x    #4414      +/-   ##
=============================================
- Coverage      58.21%   58.16%   -0.05%     
- Complexity     11204    11229      +25     
=============================================
  Files           1480     1484       +4     
  Lines          59173    59331     +158     
  Branches        6783     6813      +30     
=============================================
+ Hits           34445    34508      +63     
- Misses         22667    22759      +92     
- Partials        2061     2064       +3

Impacted Files	Coverage Δ
.../opentripplanner/ext/fares/FaresConfiguration.java	`43.33% <0.00%> (-1.50%)`	⬇️
...nner/ext/fares/impl/DefaultFareServiceFactory.java	`85.36% <ø> (ø)`
...planner/ext/fares/impl/DefaultFareServiceImpl.java	`89.82% <0.00%> (-0.55%)`	⬇️
...pplanner/ext/fares/impl/HSLFareServiceFactory.java	`0.00% <0.00%> (ø)`
...pl/HighestFareInFreeTransferWindowFareService.java	`92.00% <ø> (ø)`
...estFareInFreeTransferWindowFareServiceFactory.java	`84.61% <ø> (ø)`
...ipplanner/ext/fares/impl/SFBayFareServiceImpl.java	`0.00% <ø> (ø)`
.../legacygraphqlapi/LegacyGraphQLRequestContext.java	`0.00% <0.00%> (ø)`
...pi/datafetchers/LegacyGraphQLNodeTypeResolver.java	`2.08% <ø> (ø)`
...raphqlapi/generated/LegacyGraphQLDataFetchers.java	`0.00% <ø> (ø)`
... and 23 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

hannesj · 2022-09-01T07:49:26Z

src/main/java/org/opentripplanner/common/model/RouteOriginDestination.java

+
+import java.io.Serializable;
+
+public record RouteOriginDestination(String route, String origin, String destination)


Move to org.opentripplanner.ext.fares.model

I think that would cause the FareRuleSet in org.opentripplanner.routing.core to refer to this class in ext. Is that ok?

I think it is ok, it already has a reference to org.opentripplanner.ext.fares.model.FareAttribute

Actually, the entire FareRuleSet could be moved to org.opentripplanner.ext.fares.model, as it is only used by sandbox extensions.

hannesj

Couple of small things

Co-authored-by: Hannes Junnila <[email protected]>

leonardehrenfried · 2022-09-01T08:47:09Z

You need to run mvn prettier:write.

…manual edits in autogenerated files

src/ext/resources/legacygraphqlapi/schema.graphqls

hannesj · 2022-09-06T07:22:53Z

src/ext/java/org/opentripplanner/ext/legacygraphqlapi/generated/LegacyGraphQLDataFetchers.java

@@ -2,16 +2,23 @@
 package org.opentripplanner.ext.legacygraphqlapi.generated;

 import graphql.relay.Connection;
+import graphql.relay.Connection;


Can you run intelliJ "Optimize imports" for this file

...t/java/org/opentripplanner/ext/legacygraphqlapi/datafetchers/LegacyGraphQLQueryTypeImpl.java

hannesj · 2022-09-06T07:35:54Z

It seems you need to merge in master, as there are some small conflicts with work in #4338

…hers/LegacyGraphQLQueryTypeImpl.java Co-authored-by: Hannes Junnila <[email protected]>

nurmAV · 2022-09-06T11:25:03Z

I found a ClassCastException, that should be fixed before this is merged

leonardehrenfried · 2022-09-08T14:08:36Z

If hsl want to merge this before I'm back from my holiday, I nominate Joel or Vesa to take over my review. ( Last time I checked there were only superficial issues left.)

P3 and T3 are converted to named records

viljaminurminen-cgi added 7 commits August 11, 2022 11:07

Add HSL specifc fares implementation

7785264

Add tests for HSL fare service

2c21af7

Add ticket type query to legacy GraphQL API.

237c932

Fix itineraries needing multiple tickets

6288bbe

Fix tickets for special routes

a1369f8

Move HSL specific fare rule setup into HSLFareServiceFactory.

b2adb31

Merge remote-tracking branch 'otp/dev-2.x' into hsl_fares

4bad0a6

nurmAV requested a review from a team as a code owner August 25, 2022 12:20

leonardehrenfried reviewed Aug 25, 2022

View reviewed changes

src/main/java/org/opentripplanner/common/model/T3.java Outdated Show resolved Hide resolved

leonardehrenfried reviewed Aug 25, 2022

View reviewed changes

src/main/java/org/opentripplanner/routing/core/TicketType.java Outdated Show resolved Hide resolved

leonardehrenfried reviewed Aug 25, 2022

View reviewed changes

Fix errors due to merge

c626511

leonardehrenfried reviewed Aug 25, 2022

View reviewed changes

src/main/java/org/opentripplanner/common/model/P3.java Outdated Show resolved Hide resolved

Fix formatting

158627f

hannesj reviewed Aug 25, 2022

View reviewed changes

src/main/java/org/opentripplanner/routing/core/FareRuleSet.java Outdated Show resolved Hide resolved

hannesj added New Feature GTFS Related to import of GTFS data Sandbox Skip Changelog labels Aug 25, 2022

hannesj reviewed Aug 25, 2022

View reviewed changes

src/ext/java/org/opentripplanner/ext/fares/impl/HSLFareServiceFactory.java Outdated Show resolved Hide resolved

hannesj reviewed Aug 30, 2022

View reviewed changes

src/ext/java/org/opentripplanner/ext/fares/impl/HSLFareServiceImpl.java Outdated Show resolved Hide resolved

viljaminurminen-cgi added 5 commits August 30, 2022 13:03

Fix exception route handling

356be37

Add documentation to addRouteOriginDestination

65d689f

Move TicketType to LegacyGraphQL API sandbox

7916690

Make T3 into a record

dd4fe63

Fix formatting and remove old license header

85b5bc5

hannesj reviewed Sep 1, 2022

View reviewed changes

hannesj requested changes Sep 1, 2022

View reviewed changes

leonardehrenfried added the bump serialization id Add this label if you want the serialization id automatically bumped after merging the PR label Sep 1, 2022

nurmAV and others added 2 commits September 1, 2022 11:06

Update docs/sandbox/Fares.md

12b296b

Co-authored-by: Hannes Junnila <[email protected]>

Update src/main/java/org/opentripplanner/routing/core/FareRuleSet.java

ae84f24

Co-authored-by: Hannes Junnila <[email protected]>

viljaminurminen-cgi added 2 commits September 1, 2022 13:22

Change LegacyGraphQLTicketTypeImpl to operate on FareRuleSet and fix …

4279fb2

…manual edits in autogenerated files

Fix formatting

7d63633

leonardehrenfried reviewed Sep 1, 2022

View reviewed changes

src/ext/resources/legacygraphqlapi/schema.graphqls Outdated Show resolved Hide resolved

Remove breaking typo and unnecessary type conversion

5b7dd97

hannesj self-requested a review September 1, 2022 13:30

hannesj reviewed Sep 6, 2022

View reviewed changes

...t/java/org/opentripplanner/ext/legacygraphqlapi/datafetchers/LegacyGraphQLQueryTypeImpl.java Outdated Show resolved Hide resolved

viljaminurminen-cgi and others added 3 commits September 6, 2022 11:20

Merge remote-tracking branch 'otp/dev-2.x' into hsl_fares

2cdc76e

Update src/ext/java/org/opentripplanner/ext/legacygraphqlapi/datafetc…

c76337c

…hers/LegacyGraphQLQueryTypeImpl.java Co-authored-by: Hannes Junnila <[email protected]>

Move RouteOriginDestination and FareRuleSet into ext/fares/model

3121cc3

hannesj previously approved these changes Sep 6, 2022

View reviewed changes

Fix ClassCastException in LegacyGraphQLfareImpl

d4d4869

nurmAV dismissed hannesj’s stale review via d4d4869 September 8, 2022 06:18

hannesj approved these changes Sep 8, 2022

View reviewed changes

hannesj requested a review from leonardehrenfried September 8, 2022 13:22

vesameskanen approved these changes Sep 12, 2022

View reviewed changes

vesameskanen merged commit 807734a into opentripplanner:dev-2.x Sep 12, 2022

vesameskanen deleted the hsl_fares branch September 12, 2022 12:40

t2gran pushed a commit that referenced this pull request Sep 12, 2022

Bump serialization version id for #4414

8233f41

t2gran added this to the 2.2 milestone Oct 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add HSL specific fare computation in the sandbox #4414

Add HSL specific fare computation in the sandbox #4414

nurmAV commented Aug 25, 2022

leonardehrenfried Aug 25, 2022

nurmAV Aug 25, 2022

leonardehrenfried commented Aug 25, 2022

hannesj commented Aug 25, 2022

leonardehrenfried commented Aug 25, 2022

codecov-commenter commented Aug 30, 2022 •

edited

Loading

hannesj Sep 1, 2022

nurmAV Sep 1, 2022

hannesj Sep 6, 2022

hannesj Sep 6, 2022 •

edited

Loading

hannesj left a comment •

edited

Loading

leonardehrenfried commented Sep 1, 2022

hannesj Sep 6, 2022

hannesj commented Sep 6, 2022

nurmAV commented Sep 6, 2022

leonardehrenfried commented Sep 8, 2022


		import java.io.Serializable;

		public record RouteOriginDestination(String route, String origin, String destination)

Add HSL specific fare computation in the sandbox #4414

Add HSL specific fare computation in the sandbox #4414

Conversation

nurmAV commented Aug 25, 2022

Summary

Unit tests

leonardehrenfried Aug 25, 2022

Choose a reason for hiding this comment

nurmAV Aug 25, 2022

Choose a reason for hiding this comment

leonardehrenfried commented Aug 25, 2022

hannesj commented Aug 25, 2022

leonardehrenfried commented Aug 25, 2022

codecov-commenter commented Aug 30, 2022 • edited Loading

Codecov Report

hannesj Sep 1, 2022

Choose a reason for hiding this comment

nurmAV Sep 1, 2022

Choose a reason for hiding this comment

hannesj Sep 6, 2022

Choose a reason for hiding this comment

hannesj Sep 6, 2022 • edited Loading

Choose a reason for hiding this comment

hannesj left a comment • edited Loading

Choose a reason for hiding this comment

leonardehrenfried commented Sep 1, 2022

hannesj Sep 6, 2022

Choose a reason for hiding this comment

hannesj commented Sep 6, 2022

nurmAV commented Sep 6, 2022

leonardehrenfried commented Sep 8, 2022

codecov-commenter commented Aug 30, 2022 •

edited

Loading

hannesj Sep 6, 2022 •

edited

Loading

hannesj left a comment •

edited

Loading