Skip to content

Ignore UnrestrictedPublicTransportAreas that do not contain regular stops - #4919

Merged
vpaturet merged 1 commit into
opentripplanner:dev-2.xfrom
entur:otp2_ignore_unrestricted_public_transport_area_no_stops
Mar 2, 2023
Merged

Ignore UnrestrictedPublicTransportAreas that do not contain regular stops#4919
vpaturet merged 1 commit into
opentripplanner:dev-2.xfrom
entur:otp2_ignore_unrestricted_public_transport_area_no_stops

Conversation

@vpaturet

@vpaturet vpaturet commented Mar 1, 2023

Copy link
Copy Markdown
Contributor

Summary

This PR fixes #4918: The OTP graph builder should ignore FlexibleStopPlaces of type UnrestrictedPublicTransportAreas when they do not contain any stop.
The FlexibleStopPlace is dropped and an issue is logged in the issue store.
Additionally a null-check is added in the GroupStop constructor to prevent the creation of a GroupStop without a centroid (that triggers NullPointerExceptions downstream in the code)

Issue

Fixes #4918

Unit tests

Added unit test

Documentation

No

@vpaturet vpaturet self-assigned this Mar 1, 2023
@vpaturet vpaturet added the !Bug Apply to issues describing a bug and PRs witch fixes it. label Mar 1, 2023
@codecov

codecov Bot commented Mar 1, 2023

Copy link
Copy Markdown

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.07 🎉

Comparison is base (cc411e6) 62.95% compared to head (bb2307a) 63.03%.

❗ Current head bb2307a differs from pull request most recent head 9df5ed0. Consider uploading reports for the commit 9df5ed0 to get more accurate results

Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #4919      +/-   ##
=============================================
+ Coverage      62.95%   63.03%   +0.07%     
- Complexity     13276    13310      +34     
=============================================
  Files           1663     1663              
  Lines          66341    66399      +58     
  Branches        7221     7232      +11     
=============================================
+ Hits           41768    41855      +87     
+ Misses         22194    22168      -26     
+ Partials        2379     2376       -3     
Impacted Files Coverage Δ
...opentripplanner/netex/mapping/FlexStopsMapper.java 86.11% <100.00%> (+1.98%) ⬆️
...etex/mapping/calendar/DayTypeAssignmentMapper.java 86.95% <0.00%> (-4.12%) ⬇️
...ipplanner/netex/mapping/GroupOfStationsMapper.java 15.38% <0.00%> (-2.80%) ⬇️
...entripplanner/netex/mapping/TripPatternMapper.java 70.80% <0.00%> (ø)
...org/opentripplanner/netex/mapping/RouteMapper.java 87.01% <0.00%> (+1.09%) ⬆️
...org/opentripplanner/netex/mapping/NetexMapper.java 82.23% <0.00%> (+1.89%) ⬆️
...lanner/netex/loader/parser/ServiceFrameParser.java 73.97% <0.00%> (+2.05%) ⬆️
...opentripplanner/netex/mapping/StopTimesMapper.java 66.66% <0.00%> (+3.38%) ⬆️
...in/java/org/opentripplanner/netex/NetexBundle.java 92.98% <0.00%> (+3.50%) ⬆️
...planner/netex/loader/NetexDataSourceHierarchy.java 90.69% <0.00%> (+4.65%) ⬆️
... and 5 more

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

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@vpaturet
vpaturet force-pushed the otp2_ignore_unrestricted_public_transport_area_no_stops branch 2 times, most recently from 3c12191 to ce50c42 Compare March 1, 2023 15:17
@vpaturet
vpaturet force-pushed the otp2_ignore_unrestricted_public_transport_area_no_stops branch from ce50c42 to 9df5ed0 Compare March 1, 2023 16:25
@vpaturet
vpaturet marked this pull request as ready for review March 1, 2023 16:33
@vpaturet
vpaturet requested a review from a team as a code owner March 1, 2023 16:33

@Test
void mapInvalidAreaStop() {
void testMapInvalidAreaStop() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since I am the opposite, I'm curious: why do you prefer to prefix the test methods with test?

@vpaturet vpaturet Mar 2, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a recommendation of SonarQube.
test* is the recommended pattern.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(and for the actual reasoning: a method name should reflect what the method does. The method does not map invalid area stops, it tests what happens when invalid area stops are mapped)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second is actually a good point. I had never thought about it that way.

@vpaturet
vpaturet merged commit 7ca1316 into opentripplanner:dev-2.x Mar 2, 2023
t2gran pushed a commit that referenced this pull request Mar 2, 2023
@t2gran t2gran added this to the 2.3 milestone Mar 14, 2023
@t2gran
t2gran deleted the otp2_ignore_unrestricted_public_transport_area_no_stops branch March 31, 2023 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

!Bug Apply to issues describing a bug and PRs witch fixes it.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UnrestrictedPublicTransportAreas without quays cases NPE

4 participants