Fix handling of missing aimed departure time

vpaturet commented 4 months ago

Summary

As detailed in #5864 , some SIRI updates fail with a NullPointerException. The root cause is a missing aimed departure date on the first call in the estimated vehicle journey (mandatory field in the Nordic SIRI profile). This PR:

improve the error message when aimed departure date is missing on the first call.
add test coverage for additional use cases (SIRI updates matched by framed vehicle journey ref, fuzzy-matching)

Note: the null-check on aimed departure date is moved early enough in the process to provide context for the error message, but without changing the outcome.

Note: the SIRI test framework is extended to support:

the creation of invalid SIRI messages (missing mandatory date fields),
the creation of SIRI messages matched by framed vehicle journey ref,
the testing of fuzzy-matching.

Issue

Closes #5864

Unit tests

Added unit tests

Documentation

No

codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 77.77778% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 68.46%. Comparing base (b1314cd) to head (61f9fd9). Report is 25 commits behind head on dev-2.x.

:exclamation: Current head 61f9fd9 differs from pull request most recent head 3a12663

Please upload reports for the commit 3a12663 to get more accurate results.

Files	Patch %	Lines
...opentripplanner/ext/siri/SiriFuzzyTripMatcher.java	77.77%	2 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## dev-2.x #5865 +/- ## ============================================= + Coverage 68.37% 68.46% +0.08% - Complexity 16676 16690 +14 ============================================= Files 1914 1914 Lines 72652 72654 +2 Branches 7452 7453 +1 ============================================= + Hits 49677 49742 +65 + Misses 20418 20341 -77 - Partials 2557 2571 +14 ```

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

vpaturet commented 4 months ago

@lassetyr In this specific case, does the error need to be reported in the OTP logs? Is this useful for debugging? There is a number of other edge cases where missing/incomplete data is silently ignored.

lassetyr commented 4 months ago

I do not see the need for logging this error, so it can fail silently.

leonardehrenfried commented 4 months ago

It's great that we now have a way to write tests for all these kinds strange edge cases. Well done, @habrahamsson-skanetrafiken

opentripplanner / OpenTripPlanner