Add timePenalty to Transmodel API

[t2gran]

Summary

The time-penalty is an input parameter adding a virtual time penalty and cost penalty to itineraries with less preferable access/egress. To test this feature it would be nice to be able to list the actual calculated penalty for the access/egress
it is applied to. The internal model operate with generalized-cost and generalized-cost-including-penalty. This is NOT copied over to the API. The reason for this is that we use the generalized-cost without penalty rarely (in itinerary-filtering). So, to make the API less confusing the API only have one generalized-cost. The calculated time-penalty is however added as an output field. So, it should be easy to subtract the penalty in cases where the internal generalized-cost is debugged.

Issue

These is not issue for this. This is just a small pice missing from the #5180 PR.

Unit tests

✅ Only a small mapper with logic is added, and a unit test on this is provided. The old REST API is changed to return generalized-cost including time-penalty. I did not add a test for this, since this code is going away soon.

Documentation

✅ All new API type and fields are documented, so are new classes and method.

Changelog

🟥 This allow debugging timePenalty in the Transmodel API easier - not worth mentioning in the release log.

Bumping the serialization version id

🟥 This do not change any serialized types.

[codecov]

Codecov Report

Attention: Patch coverage is 93.87755% with 3 lines in your changes missing coverage. Please review.

Project coverage is 69.47%. Comparing base (b1a7c53) to head (7f5ba09).
Report is 12 commits behind head on dev-2.x.

Files	Patch %	Lines
...nsmodel/model/plan/TripPatternTimePenaltyType.java	89.65%	3 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             dev-2.x    #5910      +/-   ##
=============================================
+ Coverage      69.45%   69.47%   +0.01%     
- Complexity     17066    17074       +8     
=============================================
  Files           1928     1930       +2     
  Lines          73580    73625      +45     
  Branches        7550     7550              
=============================================
+ Hits           51108    51152      +44     
- Misses         19847    19849       +2     
+ Partials        2625     2624       -1     

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

[t2gran]

When testing this we noticed that time-penalty is added to the entire FLEX access/egress. The access may or may not include a walking leg from the FLEX leg to a regular stop where the journey can continue using regular transit. If FLEX trip ends at a regular stop, then so do the access leg. But, Raptor will now add transfers to this. In this case the time-penalty is only applied to the transfer when it is part of the access, not the one added by Raptor. The result set of legs can be identical. This is bad, and we need to decide how to apply the penalty. I will bring this up in a developer meeting and then create an issue for it.

[leonardehrenfried]

TimePenalty having a cost and a time penalty is weirdly recursive.

[habrahamsson-skanetrafiken]

In the RouteRequest this concept seems to be called simply "penalty" under accessEgress if I understand correctly? And in the source code it's mostly called TimeAndCostPenalty. Would one of those names make sense instead?

[t2gran]

Naming is hard. The reason why I struggle with this one is that it is the penalty on time witch is the main thing looking at this from a use-case perspective. Propagating a penalty on time to cost is secondary. We could apply the penalty to time only, and not to cost, adjusting the cost mode reluctance instead. But, since cost is highly dependent on time it feels natural to do it. All parts of a journey that take time has a reluctance and propagate to generalized-cost. Calling this TimeAndCostPenalty is misleading - yes it affect both, but the ONLY reason to use it is to apply the penalty on time. TimePenaltyThatPropagatesToCost is a bit long ;-)

So maybe TimePenaltyWithCost is ok? It at least indicate that cost is secondary.

Should I rename this and the internal type? The AccessEgressPreferences.penalty should probably be timePenalty - it is a field and the name should not be to long and it should reflect the purpouse, not describe the content.

[habrahamsson-skanetrafiken]

Maybe I was a bit confused about the cost part of the penalty. For the AccessEgress penalty I assumed that the costFactor was applied to the whole AccessEgress. Now that I'm reading the documentation I can see that the cost factor actually only applies to the extra time added by the timePenalty. So it wouldn't make sense to set a penalty with { costFactor: 2, timePenalty: "0" }.

Then I understand why you want to highlight that the cost is secondary.

I'm ok with TimePenaltyWithCost.