Remove FareComponent

[leonardehrenfried]

Summary

This removes the class FareComponent which has been deprecated for over a year.

The problem with the FareComponent is that it clashes with properly leg-based fares that we want to have. This created a very confusing parallel world where both system co-existed and had to be kept in sync, which led to many bugs.

Schema changes

The schema of the GraphQL API has not changed however the implementation has: it never returns any value for the FareComponent only ever an empty map.

The REST API was also changed but since it's scheduled for removal I think it's ok.

API changes

This changes the output of the Leg-based API a little: it doesn't include the internal identifiers like regular, senior and student anymore but uses the IDs that the GTFS data actually provides.

Here is an example of what the output in Philadelphia looks like: https://gist.github.com/leonardehrenfried/b1aeade7f7a00c3e28610a2896d075f1 (cc @daniel-heppner-ibigroup)

In Helsinki, which is the other user of this API, the result almost hasn't changed and looks like this: https://gist.github.com/leonardehrenfried/ae711633a1c10d4db24d3ab38421a51a (cc @vesameskanen @nurmAV @optionsome)

Issue

Closes #5396

Unit tests

Updated.

Bumping the serialization version id

✔️

[codecov]

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (ab694b8) 67.55% compared to head (a98f513) 67.54%.
Report is 1 commits behind head on dev-2.x.

Files	Patch %	Lines
...tripplanner/ext/fares/impl/DefaultFareService.java	94.11%	0 Missing and 1 partial ⚠️
...org/opentripplanner/model/fare/ItineraryFares.java	50.00%	1 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             dev-2.x    #5613      +/-   ##
=============================================
- Coverage      67.55%   67.54%   -0.01%     
+ Complexity     16223    16204      -19     
=============================================
  Files           1865     1863       -2     
  Lines          71204    71155      -49     
  Branches        7398     7394       -4     
=============================================
- Hits           48103    48064      -39     
+ Misses         20607    20598       -9     
+ Partials        2494     2493       -1     

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

[vesameskanen]

First impressions:

• Something got broken by this PR. With HSL fares, every leg product has always a different id, although in reality the same ticket (say HSL:ABC) would be valid.
• With HSL fares, transit legs still have an additional product called 'HSL:regular'. This is not new, but undesirable, as there is no such thing defined in HSL fare attributes.

I noticed that Philadelphia fare example above does not suffer from these two errors. So, maybe the error is in HSL fare calculator or its integration with default fares, and this is something we have to fix ourselves.

[leonardehrenfried]

@vesameskanen I will check this out. What you could do is provide a test which checks the individual legs rather than just the "fare".

[leonardehrenfried]

Thanks for catching that @vesameskanen . I think I fixed it.

Does this look better to you? https://gist.github.com/leonardehrenfried/48f0f1bc0d0b68bec2cd7604597b5a28

[leonardehrenfried]

BTW, do you still need the -0.01 cents if a leg doesn't have a fare product?

[vesameskanen]

HSL fares work again, thanks for the fix!

I will examine why the artificial "HSL:regular" ticket shows up, and finish the review today.

[leonardehrenfried]

I know where the extra fare product comes from. On top of the individual fare products we also calculate a "fare" which applies to the entire itinerary. I would also like to remove it but because it's so easy to break this code I'm going bit by bit.

[binh-dam-ibigroup]

Looks solid. There is a fare components() getter that is still around, should it be removed?

[binh-dam-ibigroup]

Does components refer to FareComponents? If so, can/should the components method of GraphQLFare be removed too? Note that you removed a similar method from ItineraryFares in this PR, and I don't see a reference to that in the OTP-react-redux/OTP-UI code.

[leonardehrenfried]

We keep the API schema backwards compatible forever so I cannot remove it (even though it always returns an empty list).

This is so that queries from old clients never fail even if they don't return any useful information.