From b5e0ee6a5f609eb53b1892050a39b1eccc19ec66 Mon Sep 17 00:00:00 2001 From: Vincent Paturet Date: Tue, 16 Jul 2024 09:47:04 +0200 Subject: [PATCH 1/3] Enforce non-null coordinates on multimodal station --- .../netex/mapping/MultiModalStationMapper.java | 8 +++++--- .../org/opentripplanner/netex/mapping/NetexMapper.java | 5 +++-- .../transit/model/site/MultiModalStation.java | 2 +- .../transit/model/site/MultiModalStationTest.java | 2 ++ 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/opentripplanner/netex/mapping/MultiModalStationMapper.java b/src/main/java/org/opentripplanner/netex/mapping/MultiModalStationMapper.java index ef4b0a25cfd..5a4fb23bbb9 100644 --- a/src/main/java/org/opentripplanner/netex/mapping/MultiModalStationMapper.java +++ b/src/main/java/org/opentripplanner/netex/mapping/MultiModalStationMapper.java @@ -1,6 +1,7 @@ package org.opentripplanner.netex.mapping; import java.util.Collection; +import javax.annotation.Nullable; import org.opentripplanner.framework.geometry.WgsCoordinate; import org.opentripplanner.framework.i18n.NonLocalizedString; import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore; @@ -21,6 +22,7 @@ public MultiModalStationMapper(DataImportIssueStore issueStore, FeedScopedIdFact this.idFactory = idFactory; } + @Nullable MultiModalStation map(StopPlace stopPlace, Collection childStations) { MultiModalStationBuilder multiModalStation = MultiModalStation .of(idFactory.createId(stopPlace.getId())) @@ -34,13 +36,13 @@ MultiModalStation map(StopPlace stopPlace, Collection childStations) { if (coordinate == null) { issueStore.add( "MultiModalStationWithoutCoordinates", - "MultiModal station {} does not contain any coordinates.", + "MultiModal station %s does not contain any coordinates.", multiModalStation.getId() ); + return null; } else { multiModalStation.withCoordinate(coordinate); + return multiModalStation.build(); } - - return multiModalStation.build(); } } diff --git a/src/main/java/org/opentripplanner/netex/mapping/NetexMapper.java b/src/main/java/org/opentripplanner/netex/mapping/NetexMapper.java index c3c9ad2d0ae..991c0477266 100644 --- a/src/main/java/org/opentripplanner/netex/mapping/NetexMapper.java +++ b/src/main/java/org/opentripplanner/netex/mapping/NetexMapper.java @@ -332,8 +332,9 @@ private void mapMultiModalStopPlaces() { .getStationsByMultiModalStationRfs() .get(multiModalStopPlace.getId()); var multiModalStation = mapper.map(multiModalStopPlace, stations); - - transitBuilder.stopModel().withMultiModalStation(multiModalStation); + if (multiModalStation != null) { + transitBuilder.stopModel().withMultiModalStation(multiModalStation); + } } } diff --git a/src/main/java/org/opentripplanner/transit/model/site/MultiModalStation.java b/src/main/java/org/opentripplanner/transit/model/site/MultiModalStation.java index 749b156656e..70f4924e8e1 100644 --- a/src/main/java/org/opentripplanner/transit/model/site/MultiModalStation.java +++ b/src/main/java/org/opentripplanner/transit/model/site/MultiModalStation.java @@ -37,11 +37,11 @@ public class MultiModalStation super(builder.getId()); // Required fields this.childStations = Objects.requireNonNull(builder.childStations()); + this.coordinate = Objects.requireNonNull(builder.coordinate()); this.name = I18NString.assertHasValue(builder.name()); // Optional fields // TODO Make required - this.coordinate = builder.coordinate(); this.code = builder.code(); this.description = builder.description(); this.url = builder.url(); diff --git a/src/test/java/org/opentripplanner/transit/model/site/MultiModalStationTest.java b/src/test/java/org/opentripplanner/transit/model/site/MultiModalStationTest.java index 4ca8e8d33ea..615b0c49674 100644 --- a/src/test/java/org/opentripplanner/transit/model/site/MultiModalStationTest.java +++ b/src/test/java/org/opentripplanner/transit/model/site/MultiModalStationTest.java @@ -8,6 +8,7 @@ import java.util.Set; import org.junit.jupiter.api.Test; +import org.opentripplanner.framework.geometry.WgsCoordinate; import org.opentripplanner.framework.i18n.I18NString; import org.opentripplanner.framework.i18n.NonLocalizedString; import org.opentripplanner.transit.model._data.TransitModelForTest; @@ -26,6 +27,7 @@ class MultiModalStationTest { .of(TransitModelForTest.id(ID)) .withName(NAME) .withChildStations(CHILD_STATIONS) + .withCoordinate(new WgsCoordinate(1, 1)) .build(); @Test From 5bc0b78079eb2ec97a8e575cd3ec0ba9a2ee58a0 Mon Sep 17 00:00:00 2001 From: Vincent Paturet Date: Tue, 16 Jul 2024 13:55:27 +0200 Subject: [PATCH 2/3] Apply review suggestion --- .../mapping/MultiModalStationMapperTest.java | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 src/test/java/org/opentripplanner/netex/mapping/MultiModalStationMapperTest.java diff --git a/src/test/java/org/opentripplanner/netex/mapping/MultiModalStationMapperTest.java b/src/test/java/org/opentripplanner/netex/mapping/MultiModalStationMapperTest.java new file mode 100644 index 00000000000..435ba2878f2 --- /dev/null +++ b/src/test/java/org/opentripplanner/netex/mapping/MultiModalStationMapperTest.java @@ -0,0 +1,32 @@ +package org.opentripplanner.netex.mapping; + +import java.util.List; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore; +import org.opentripplanner.graph_builder.issue.service.DefaultDataImportIssueStore; +import org.opentripplanner.netex.NetexTestDataSupport; +import org.opentripplanner.netex.mapping.support.FeedScopedIdFactory; +import org.opentripplanner.transit.model._data.TransitModelForTest; +import org.rutebanken.netex.model.StopPlace; + +class MultiModalStationMapperTest { + + @Test + void testMissingCoordinates() { + DataImportIssueStore dataIssueStore = new DefaultDataImportIssueStore(); + FeedScopedIdFactory feedScopeIdFactory = new FeedScopedIdFactory(TransitModelForTest.FEED_ID); + MultiModalStationMapper multiModalStationMapper = new MultiModalStationMapper( + dataIssueStore, + feedScopeIdFactory + ); + StopPlace stopPlace = new StopPlace(); + stopPlace.setId(NetexTestDataSupport.STOP_PLACE_ID); + Assertions.assertNull(multiModalStationMapper.map(stopPlace, List.of())); + Assertions.assertEquals(1, dataIssueStore.listIssues().size()); + Assertions.assertEquals( + "MultiModalStationWithoutCoordinates", + dataIssueStore.listIssues().getFirst().getType() + ); + } +} From e58ff95647191fdfaed11ad7fbd1d7962942c801 Mon Sep 17 00:00:00 2001 From: Vincent Paturet Date: Tue, 16 Jul 2024 15:36:52 +0200 Subject: [PATCH 3/3] Apply review suggestion --- .../netex/mapping/MultiModalStationMapperTest.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/test/java/org/opentripplanner/netex/mapping/MultiModalStationMapperTest.java b/src/test/java/org/opentripplanner/netex/mapping/MultiModalStationMapperTest.java index 435ba2878f2..218942d29ef 100644 --- a/src/test/java/org/opentripplanner/netex/mapping/MultiModalStationMapperTest.java +++ b/src/test/java/org/opentripplanner/netex/mapping/MultiModalStationMapperTest.java @@ -1,7 +1,8 @@ package org.opentripplanner.netex.mapping; +import static org.junit.jupiter.api.Assertions.*; + import java.util.List; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore; import org.opentripplanner.graph_builder.issue.service.DefaultDataImportIssueStore; @@ -22,9 +23,9 @@ void testMissingCoordinates() { ); StopPlace stopPlace = new StopPlace(); stopPlace.setId(NetexTestDataSupport.STOP_PLACE_ID); - Assertions.assertNull(multiModalStationMapper.map(stopPlace, List.of())); - Assertions.assertEquals(1, dataIssueStore.listIssues().size()); - Assertions.assertEquals( + assertNull(multiModalStationMapper.map(stopPlace, List.of())); + assertEquals(1, dataIssueStore.listIssues().size()); + assertEquals( "MultiModalStationWithoutCoordinates", dataIssueStore.listIssues().getFirst().getType() );