Skip to content

Add validation on NeTEx flexible area import - #4765

Merged
vpaturet merged 2 commits into
opentripplanner:dev-2.xfrom
entur:otp2_add_validation_on_flexible_area
Jan 25, 2023
Merged

Add validation on NeTEx flexible area import#4765
vpaturet merged 2 commits into
opentripplanner:dev-2.xfrom
entur:otp2_add_validation_on_flexible_area

Conversation

@vpaturet

Copy link
Copy Markdown
Contributor

Summary

As mentioned in #4764, the graph builder crashes when importing invalid geometries in NeTEx FlexibleStopPlace.
This PR adds validation logic so that StopPlaces containing invalid geometries are ignored and invalid data is reported in the validation report.

Issue

Fixes #4764

Unit tests

Added unit tests

Documentation

No

@vpaturet vpaturet self-assigned this Jan 25, 2023
@vpaturet vpaturet added the !Bug Apply to issues describing a bug and PRs witch fixes it. label Jan 25, 2023
@vpaturet
vpaturet marked this pull request as ready for review January 25, 2023 14:40
@vpaturet
vpaturet requested a review from a team as a code owner January 25, 2023 14:40
@codecov

codecov Bot commented Jan 25, 2023

Copy link
Copy Markdown

Codecov Report

Base: 61.85% // Head: 61.86% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (d09124f) compared to base (46dd2cd).
Patch coverage: 70.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             dev-2.x    #4765   +/-   ##
==========================================
  Coverage      61.85%   61.86%           
- Complexity     12766    12770    +4     
==========================================
  Files           1614     1614           
  Lines          64468    64484   +16     
  Branches        7042     7044    +2     
==========================================
+ Hits           39879    39891   +12     
- Misses         22321    22325    +4     
  Partials        2268     2268           
Impacted Files Coverage Δ
...org/opentripplanner/netex/mapping/NetexMapper.java 80.33% <0.00%> (ø)
...opentripplanner/netex/mapping/FlexStopsMapper.java 84.12% <73.68%> (-5.24%) ⬇️
...g/opentripplanner/api/parameter/QualifiedMode.java 68.42% <0.00%> (+2.63%) ⬆️

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.

Comment on lines +87 to +97
static final Collection<Double> INVALID_AREA_POS_LIST = new ArrayList<>(
Arrays.asList(
59.62575084033623,
6.3023991052849,
59.62883380609349,
6.289718020117876,
59.6346950024935,
6.293494451572027
)
);

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.

Suggested change
static final Collection<Double> INVALID_AREA_POS_LIST = new ArrayList<>(
Arrays.asList(
59.62575084033623,
6.3023991052849,
59.62883380609349,
6.289718020117876,
59.6346950024935,
6.293494451572027
)
);
static final Collection<Double> INVALID_AREA_POS_LIST = List.of(
59.62575084033623,
6.3023991052849,
59.62883380609349,
6.289718020117876,
59.6346950024935,
6.293494451572027
)
);

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.

true, and the original test data AREA_POS_LIST could also be simplified.
Done for both

@leonardehrenfried leonardehrenfried left a comment

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.

I have only a very minor comment.

@vpaturet
vpaturet enabled auto-merge January 25, 2023 15:04
private Geometry mapGeometry(FlexibleArea area) {
try {
return OpenGisMapper.mapGeometry(area.getPolygon());
} catch (Exception e) {

@hannesj hannesj Jan 25, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this be a more narrow type, which we even could include in the method contract?

jts seems to use IllegalArgumentException, so no

@vpaturet
vpaturet merged commit f70bd7a into opentripplanner:dev-2.x Jan 25, 2023
t2gran pushed a commit that referenced this pull request Jan 25, 2023
@vpaturet
vpaturet deleted the otp2_add_validation_on_flexible_area branch January 26, 2023 10:19
@t2gran t2gran added this to the 2.3 milestone Feb 15, 2023
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.

Graph Builder fails when importing invalid NeTEx flex area

4 participants