From f2241c97c015e2883d732121d50880730286679f Mon Sep 17 00:00:00 2001 From: Henrik Abrahamsson Date: Wed, 13 Dec 2023 11:25:07 +0100 Subject: [PATCH] Report issue for duplicate journey patterns in NeTEx --- .../netex/mapping/NetexMapper.java | 41 ++++++++--- .../netex/mapping/TripPatternMapper.java | 71 +++++++++++-------- .../mapping/TripPatternMapperResult.java | 31 +++----- .../netex/mapping/TripPatternMapperTest.java | 45 +++++++----- 4 files changed, 107 insertions(+), 81 deletions(-) diff --git a/src/main/java/org/opentripplanner/netex/mapping/NetexMapper.java b/src/main/java/org/opentripplanner/netex/mapping/NetexMapper.java index 26016947c09..411ac268059 100644 --- a/src/main/java/org/opentripplanner/netex/mapping/NetexMapper.java +++ b/src/main/java/org/opentripplanner/netex/mapping/NetexMapper.java @@ -11,6 +11,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.stream.Collectors; import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore; import org.opentripplanner.model.StopTime; import org.opentripplanner.model.calendar.ServiceCalendar; @@ -460,18 +461,36 @@ private void mapTripPatterns(Map serviceIds) { for (JourneyPattern_VersionStructure journeyPattern : currentNetexIndex .getJourneyPatternsById() .localValues()) { - TripPatternMapperResult result = tripPatternMapper.mapTripPattern(journeyPattern); + tripPatternMapper + .mapTripPattern(journeyPattern) + .ifPresent(result -> { + var journeyPatternExists = transitBuilder + .getTripPatterns() + .get(result.tripPattern().getStopPattern()) + .stream() + .anyMatch(tripPattern -> result.tripPattern().getId().equals(tripPattern.getId())); + if (journeyPatternExists) { + issueStore.add( + "DuplicateJourneyPattern", + "Duplicate of JourneyPattern %s found", + journeyPattern.getId() + ); + } - for (Map.Entry> it : result.tripStopTimes.entrySet()) { - transitBuilder.getStopTimesSortedByTrip().put(it.getKey(), it.getValue()); - transitBuilder.getTripsById().add(it.getKey()); - } - for (var it : result.tripPatterns.entries()) { - transitBuilder.getTripPatterns().put(it.getKey(), it.getValue()); - } - currentMapperIndexes.addStopTimesByNetexId(result.stopTimeByNetexId); - groupMapper.scheduledStopPointsIndex.putAll(Multimaps.asMap(result.scheduledStopPointsIndex)); - transitBuilder.getTripOnServiceDates().addAll(result.tripOnServiceDates); + for (Map.Entry> it : result.tripStopTimes().entrySet()) { + transitBuilder.getStopTimesSortedByTrip().put(it.getKey(), it.getValue()); + transitBuilder.getTripsById().add(it.getKey()); + } + + transitBuilder + .getTripPatterns() + .put(result.tripPattern().getStopPattern(), result.tripPattern()); + currentMapperIndexes.addStopTimesByNetexId(result.stopTimeByNetexId()); + groupMapper.scheduledStopPointsIndex.putAll( + Multimaps.asMap(result.scheduledStopPointsIndex()) + ); + transitBuilder.getTripOnServiceDates().addAll(result.tripOnServiceDates()); + }); } } diff --git a/src/main/java/org/opentripplanner/netex/mapping/TripPatternMapper.java b/src/main/java/org/opentripplanner/netex/mapping/TripPatternMapper.java index 6fa000c1049..f0d06b8871d 100644 --- a/src/main/java/org/opentripplanner/netex/mapping/TripPatternMapper.java +++ b/src/main/java/org/opentripplanner/netex/mapping/TripPatternMapper.java @@ -5,10 +5,12 @@ import jakarta.xml.bind.JAXBElement; import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.stream.Collectors; import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore; import org.opentripplanner.model.StopTime; @@ -84,8 +86,6 @@ class TripPatternMapper { private final Deduplicator deduplicator; - private TripPatternMapperResult result; - TripPatternMapper( DataImportIssueStore issueStore, FeedScopedIdFactory idFactory, @@ -157,9 +157,7 @@ class TripPatternMapper { } } - TripPatternMapperResult mapTripPattern(JourneyPattern_VersionStructure journeyPattern) { - // Make sure the result is clean, by creating a new object. - result = new TripPatternMapperResult(); + Optional mapTripPattern(JourneyPattern_VersionStructure journeyPattern) { Collection serviceJourneys = serviceJourniesByPatternId.get( journeyPattern.getId() ); @@ -170,10 +168,14 @@ TripPatternMapperResult mapTripPattern(JourneyPattern_VersionStructure journeyPa "ServiceJourneyPattern %s does not contain any serviceJourneys.", journeyPattern.getId() ); - return result; + return Optional.empty(); } List trips = new ArrayList<>(); + ArrayListMultimap scheduledStopPointsIndex = ArrayListMultimap.create(); + HashMap> tripStopTimes = new HashMap<>(); + Map stopTimeByNetexId = new HashMap<>(); + ArrayList tripOnServiceDates = new ArrayList<>(); for (ServiceJourney serviceJourney : serviceJourneys) { Trip trip = mapTrip(journeyPattern, serviceJourney); @@ -184,7 +186,7 @@ TripPatternMapperResult mapTripPattern(JourneyPattern_VersionStructure journeyPa } // Add the dated service journey to the model for this trip [if it exists] - mapDatedServiceJourney(journeyPattern, serviceJourney, trip); + tripOnServiceDates.addAll(mapDatedServiceJourney(journeyPattern, serviceJourney, trip)); StopTimesMapperResult stopTimes = stopTimesMapper.mapToStopTimes( journeyPattern, @@ -198,25 +200,22 @@ TripPatternMapperResult mapTripPattern(JourneyPattern_VersionStructure journeyPa continue; } - result.scheduledStopPointsIndex.putAll( - serviceJourney.getId(), - stopTimes.scheduledStopPointIds - ); - result.tripStopTimes.put(trip, stopTimes.stopTimes); - result.stopTimeByNetexId.putAll(stopTimes.stopTimeByNetexId); + scheduledStopPointsIndex.putAll(serviceJourney.getId(), stopTimes.scheduledStopPointIds); + tripStopTimes.put(trip, stopTimes.stopTimes); + stopTimeByNetexId.putAll(stopTimes.stopTimeByNetexId); trips.add(trip); } // No trips successfully mapped if (trips.isEmpty()) { - return result; + return Optional.empty(); } // Create StopPattern from any trip (since they are part of the same JourneyPattern) StopPattern stopPattern = deduplicator.deduplicateObject( StopPattern.class, - new StopPattern(result.tripStopTimes.get(trips.get(0))) + new StopPattern(tripStopTimes.get(trips.get(0))) ); var tripPatternModes = new HashSet(); @@ -246,7 +245,7 @@ TripPatternMapperResult mapTripPattern(JourneyPattern_VersionStructure journeyPa ); } - TripPatternBuilder tripPatternBuilder = TripPattern + var tripPattern = TripPattern .of(idFactory.createId(journeyPattern.getId())) .withRoute(lookupRoute(journeyPattern)) .withStopPattern(stopPattern) @@ -256,30 +255,35 @@ TripPatternMapperResult mapTripPattern(JourneyPattern_VersionStructure journeyPa .withName(journeyPattern.getName() == null ? "" : journeyPattern.getName().getValue()) .withHopGeometries( serviceLinkMapper.getGeometriesByJourneyPattern(journeyPattern, stopPattern) - ); - - TripPattern tripPattern = tripPatternBuilder.build(); - createTripTimes(trips, tripPattern); - - result.tripPatterns.put(stopPattern, tripPattern); - - return result; + ) + .build(); + createTripTimes(trips, tripStopTimes).forEach(tripPattern::add); + + return Optional.of( + new TripPatternMapperResult( + tripPattern, + scheduledStopPointsIndex, + tripStopTimes, + stopTimeByNetexId, + tripOnServiceDates + ) + ); } - private void mapDatedServiceJourney( + private ArrayList mapDatedServiceJourney( JourneyPattern_VersionStructure journeyPattern, ServiceJourney serviceJourney, Trip trip ) { + var tripsOnServiceDates = new ArrayList(); if (datedServiceJourneysBySJId.containsKey(serviceJourney.getId())) { for (DatedServiceJourney datedServiceJourney : datedServiceJourneysBySJId.get( serviceJourney.getId() )) { - result.tripOnServiceDates.add( - mapDatedServiceJourney(journeyPattern, trip, datedServiceJourney) - ); + tripsOnServiceDates.add(mapDatedServiceJourney(journeyPattern, trip, datedServiceJourney)); } } + return tripsOnServiceDates; } private TripOnServiceDate mapDatedServiceJourney( @@ -360,9 +364,13 @@ private org.opentripplanner.transit.model.network.Route lookupRoute( return otpRouteById.get(idFactory.createId(lineId)); } - private void createTripTimes(List trips, TripPattern tripPattern) { + private List createTripTimes( + List trips, + Map> tripStopTimes + ) { + var tripTimesResult = new ArrayList(); for (Trip trip : trips) { - List stopTimes = result.tripStopTimes.get(trip); + List stopTimes = tripStopTimes.get(trip); if (stopTimes.isEmpty()) { issueStore.add( "TripWithoutTripTimes", @@ -372,12 +380,13 @@ private void createTripTimes(List trips, TripPattern tripPattern) { } else { try { TripTimes tripTimes = TripTimesFactory.tripTimes(trip, stopTimes, deduplicator); - tripPattern.add(tripTimes); + tripTimesResult.add(tripTimes); } catch (DataValidationException e) { issueStore.add(e.error()); } } } + return tripTimesResult; } private Trip mapTrip( diff --git a/src/main/java/org/opentripplanner/netex/mapping/TripPatternMapperResult.java b/src/main/java/org/opentripplanner/netex/mapping/TripPatternMapperResult.java index 6b14fea5d31..f2f75cd2561 100644 --- a/src/main/java/org/opentripplanner/netex/mapping/TripPatternMapperResult.java +++ b/src/main/java/org/opentripplanner/netex/mapping/TripPatternMapperResult.java @@ -13,24 +13,15 @@ import org.opentripplanner.transit.model.timetable.TripOnServiceDate; /** - * This mapper returnes two collections, so we need to use a simple wraper to be able to return the - * result from the mapping method. + * Wrapper class for the result of TripPatternMapper + * + * @param scheduledStopPointsIndex A map from trip/serviceJourney id to an ordered list of scheduled stop point ids. + * @param stopTimeByNetexId stopTimes by the timetabled-passing-time id */ -class TripPatternMapperResult { - - /** - * A map from trip/serviceJourney id to an ordered list of scheduled stop point ids. - */ - final ArrayListMultimap scheduledStopPointsIndex = ArrayListMultimap.create(); - - final Map> tripStopTimes = new HashMap<>(); - - final Multimap tripPatterns = ArrayListMultimap.create(); - - /** - * stopTimes by the timetabled-passing-time id - */ - final Map stopTimeByNetexId = new HashMap<>(); - - final ArrayList tripOnServiceDates = new ArrayList<>(); -} +record TripPatternMapperResult( + TripPattern tripPattern, + ArrayListMultimap scheduledStopPointsIndex, + Map> tripStopTimes, + Map stopTimeByNetexId, + ArrayList tripOnServiceDates +) {} diff --git a/src/test/java/org/opentripplanner/netex/mapping/TripPatternMapperTest.java b/src/test/java/org/opentripplanner/netex/mapping/TripPatternMapperTest.java index f07a46ced2c..74448fb3638 100644 --- a/src/test/java/org/opentripplanner/netex/mapping/TripPatternMapperTest.java +++ b/src/test/java/org/opentripplanner/netex/mapping/TripPatternMapperTest.java @@ -1,9 +1,11 @@ package org.opentripplanner.netex.mapping; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import com.google.common.collect.ArrayListMultimap; import java.util.Map; +import java.util.Optional; import org.junit.jupiter.api.Test; import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore; import org.opentripplanner.netex.index.hierarchy.HierarchicalMap; @@ -12,7 +14,6 @@ import org.opentripplanner.transit.model.framework.Deduplicator; import org.opentripplanner.transit.model.framework.DefaultEntityById; import org.opentripplanner.transit.model.framework.FeedScopedId; -import org.opentripplanner.transit.model.network.TripPattern; import org.opentripplanner.transit.model.timetable.Trip; import org.opentripplanner.transit.model.timetable.TripAlteration; import org.opentripplanner.transit.model.timetable.TripOnServiceDate; @@ -55,26 +56,28 @@ public void testMapTripPattern() { 150 ); - TripPatternMapperResult r = tripPatternMapper.mapTripPattern(sample.getJourneyPattern()); + Optional res = tripPatternMapper.mapTripPattern( + sample.getJourneyPattern() + ); - assertEquals(1, r.tripPatterns.size()); + assertTrue(res.isPresent()); - TripPattern tripPattern = r.tripPatterns.values().stream().findFirst().orElseThrow(); + TripPatternMapperResult r = res.get(); - assertEquals(4, tripPattern.numberOfStops()); - assertEquals(1, tripPattern.scheduledTripsAsStream().count()); + assertEquals(4, r.tripPattern().numberOfStops()); + assertEquals(1, r.tripPattern().scheduledTripsAsStream().count()); - Trip trip = tripPattern.scheduledTripsAsStream().findFirst().get(); + Trip trip = r.tripPattern().scheduledTripsAsStream().findFirst().get(); assertEquals("RUT:ServiceJourney:1", trip.getId().getId()); - assertEquals("NSR:Quay:1", tripPattern.getStop(0).getId().getId()); - assertEquals("NSR:Quay:2", tripPattern.getStop(1).getId().getId()); - assertEquals("NSR:Quay:3", tripPattern.getStop(2).getId().getId()); - assertEquals("NSR:Quay:4", tripPattern.getStop(3).getId().getId()); + assertEquals("NSR:Quay:1", r.tripPattern().getStop(0).getId().getId()); + assertEquals("NSR:Quay:2", r.tripPattern().getStop(1).getId().getId()); + assertEquals("NSR:Quay:3", r.tripPattern().getStop(2).getId().getId()); + assertEquals("NSR:Quay:4", r.tripPattern().getStop(3).getId().getId()); - assertEquals(1, tripPattern.getScheduledTimetable().getTripTimes().size()); + assertEquals(1, r.tripPattern().getScheduledTimetable().getTripTimes().size()); - TripTimes tripTimes = tripPattern.getScheduledTimetable().getTripTimes().get(0); + TripTimes tripTimes = r.tripPattern().getScheduledTimetable().getTripTimes().get(0); assertEquals(4, tripTimes.getNumStops()); @@ -115,15 +118,19 @@ public void testMapTripPattern_datedServiceJourney() { 150 ); - TripPatternMapperResult r = tripPatternMapper.mapTripPattern(sample.getJourneyPattern()); + Optional res = tripPatternMapper.mapTripPattern( + sample.getJourneyPattern() + ); + + assertTrue(res.isPresent()); + + var r = res.get(); - assertEquals(1, r.tripPatterns.size()); - assertEquals(2, r.tripOnServiceDates.size()); + assertEquals(2, r.tripOnServiceDates().size()); - TripPattern tripPattern = r.tripPatterns.values().stream().findFirst().orElseThrow(); - Trip trip = tripPattern.scheduledTripsAsStream().findFirst().get(); + Trip trip = r.tripPattern().scheduledTripsAsStream().findFirst().get(); - for (TripOnServiceDate tripOnServiceDate : r.tripOnServiceDates) { + for (TripOnServiceDate tripOnServiceDate : r.tripOnServiceDates()) { assertEquals(trip, tripOnServiceDate.getTrip()); assertEquals(TripAlteration.PLANNED, tripOnServiceDate.getTripAlteration()); assertEquals(