opentripplanner / OpenTripPlanner

An open source multi-modal trip planner
http://www.opentripplanner.org
Other
2.18k stars 1.02k forks source link

Add via to the Transmodel trip query and make a proper Raptor implementation for it #6084

Open t2gran opened 1 week ago

t2gran commented 1 week ago

Summary

This PR add back the via search to the Transmodel plan query. It does so by "chaining" Raptor requests. To be able to do this I had to refactor quite a bit. The functional changes is not that big.

This PR changes the Transmodel API and add a via location to the plan request. The passThrough feature works as before, but is merged with the new via API, and the old passThrough arguments are deprecated.

There are a few things left to implement:

I will create issues for reminding features, when this is merged.

The feature is a bit experimental and need "real life" testing - small changes and bug fixes are expected.

If this PR is too massive, then I can split it - but I believe there is little risk in breaking existing functionality. Splitting it in a pure feature PR and refactorings is difficult.

Issue

Related to : #4887

Unit tests

This PR refactor a lot of code and also add missing unit-test to some of the refactored classes. Almost all new code have unit-tests.

Documentation

✅ The API is documented and all new public classes/methods should have JavaDoc.

Changelog

Bumping the serialization version id

✅ The routing request is changed

codecov[bot] commented 1 week ago

Codecov Report

Attention: Patch coverage is 79.51426% with 194 lines in your changes missing coverage. Please review.

Project coverage is 69.78%. Comparing base (dc56145) to head (68e8867). Report is 2 commits behind head on dev-2.x.

Files with missing lines Patch % Lines
...er/raptor/service/ViaRangeRaptorDynamicSearch.java 0.00% 114 Missing :warning:
...pplanner/raptor/api/request/RaptorViaLocation.java 83.87% 2 Missing and 8 partials :warning:
...oradapter/transit/mappers/RaptorRequestMapper.java 70.37% 5 Missing and 3 partials :warning:
...r/multicriteria/arrivals/McStopArrivalFactory.java 33.33% 5 Missing and 1 partial :warning:
.../algorithm/raptoradapter/router/TransitRouter.java 14.28% 6 Missing :warning:
...nner/apis/transmodel/mapping/ViaRequestMapper.java 0.00% 4 Missing :warning:
...lanner/raptor/api/request/RaptorViaConnection.java 86.20% 1 Missing and 3 partials :warning:
...entripplanner/raptor/api/request/SearchParams.java 55.55% 3 Missing and 1 partial :warning:
...ptor/rangeraptor/multicriteria/McStopArrivals.java 90.00% 1 Missing and 3 partials :warning:
...nner/routing/api/request/via/VisitViaLocation.java 86.66% 2 Missing and 2 partials :warning:
... and 22 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev-2.x #6084 +/- ## ============================================= - Coverage 69.79% 69.78% -0.02% - Complexity 17369 17545 +176 ============================================= Files 1964 1983 +19 Lines 74427 75004 +577 Branches 7632 7694 +62 ============================================= + Hits 51947 52339 +392 - Misses 19832 19994 +162 - Partials 2648 2671 +23 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

leonardehrenfried commented 1 week ago

@t2gran Do you expect this PR to change the performance?

leonardehrenfried commented 1 week ago

Also, you need to resolve some conflicts.

t2gran commented 1 week ago

@t2gran Do you expect this PR to change the performance?

No, chaning Raptor Request should only affect performance for via searches. There are minor changes in the Raptor instrumentation (not in logic), so it might cause some problems for the JIT compiler - but I expect that it should not change.