Merge pull request #5794 from entur/fix_trip_duplication_in_DSJ_mapping

Fix trip duplication in Graph Builder DSJ mapping

src/main/java/org/opentripplanner/netex/mapping/TripPatternMapper.java

@@ -67,7 +67,7 @@ class TripPatternMapper {
 
   private final ReadOnlyHierarchicalMap<String, Route> routeById;
 
-  private final Multimap<String, ServiceJourney> serviceJourniesByPatternId = ArrayListMultimap.create();
+  private final Multimap<String, ServiceJourney> serviceJourneysByPatternId = ArrayListMultimap.create();
 
   private final ReadOnlyHierarchicalMapById<OperatingDay> operatingDayById;
 
@@ -152,12 +152,12 @@ class TripPatternMapper {
     this.serviceJourneyById = serviceJourneyById;
     // Index service journey by pattern id
     for (ServiceJourney sj : serviceJourneyById.localValues()) {
-      this.serviceJourniesByPatternId.put(sj.getJourneyPatternRef().getValue().getRef(), sj);
+      this.serviceJourneysByPatternId.put(sj.getJourneyPatternRef().getValue().getRef(), sj);
     }
   }
 
   Optional<TripPatternMapperResult> mapTripPattern(JourneyPattern_VersionStructure journeyPattern) {
-    Collection<ServiceJourney> serviceJourneys = serviceJourniesByPatternId.get(
+    Collection<ServiceJourney> serviceJourneys = serviceJourneysByPatternId.get(
       journeyPattern.getId()
     );
 
@@ -392,9 +392,12 @@ private Trip mapTrip(
     JourneyPattern_VersionStructure journeyPattern,
     ServiceJourney serviceJourney
   ) {
-    return tripMapper.mapServiceJourney(
-      serviceJourney,
-      () -> findTripHeadsign(journeyPattern, serviceJourney)
+    return deduplicator.deduplicateObject(
+      Trip.class,
+      tripMapper.mapServiceJourney(
+        serviceJourney,
+        () -> findTripHeadsign(journeyPattern, serviceJourney)
+      )
     );
   }
 

src/test/java/org/opentripplanner/netex/mapping/NetexTestDataSample.java

@@ -40,6 +40,7 @@
 import org.rutebanken.netex.model.ScheduledStopPointRefStructure;
 import org.rutebanken.netex.model.ServiceAlterationEnumeration;
 import org.rutebanken.netex.model.ServiceJourney;
+import org.rutebanken.netex.model.ServiceJourneyRefStructure;
 import org.rutebanken.netex.model.StopPointInJourneyPattern;
 import org.rutebanken.netex.model.StopPointInJourneyPatternRefStructure;
 import org.rutebanken.netex.model.TimetabledPassingTime;
@@ -50,9 +51,11 @@
 public class NetexTestDataSample {
 
   public static final String SERVICE_JOURNEY_ID = "RUT:ServiceJourney:1";
+  public static final String DATED_SERVICE_JOURNEY_ID_1 = "RUT:DatedServiceJourney:1";
+  public static final String DATED_SERVICE_JOURNEY_ID_2 = "RUT:DatedServiceJourney:2";
   public static final List<String> DATED_SERVICE_JOURNEY_ID = List.of(
-    "RUT:DatedServiceJourney:1",
-    "RUT:DatedServiceJourney:2"
+    DATED_SERVICE_JOURNEY_ID_1,
+    DATED_SERVICE_JOURNEY_ID_2
   );
   public static final List<String> OPERATING_DAYS = List.of("2022-02-28", "2022-02-29");
   private static final DayType EVERYDAY = new DayType()
@@ -174,6 +177,11 @@ public NetexTestDataSample() {
 
         DatedServiceJourney datedServiceJourney = new DatedServiceJourney()
           .withId(DATED_SERVICE_JOURNEY_ID.get(i))
+          .withJourneyRef(
+            List.of(
+              MappingSupport.createWrappedRef(SERVICE_JOURNEY_ID, ServiceJourneyRefStructure.class)
+            )
+          )
           .withServiceAlteration(ServiceAlterationEnumeration.PLANNED)
           .withOperatingDayRef(new OperatingDayRefStructure().withRef(operatingDay.getId()));
 
@@ -218,6 +226,15 @@ public HierarchicalMapById<ServiceJourney> getServiceJourneyById() {
     return serviceJourneyById;
   }
 
+  public DatedServiceJourney getDatedServiceJourneyById(String id) {
+    return datedServiceJourneyBySjId
+      .values()
+      .stream()
+      .filter(datedServiceJourney -> datedServiceJourney.getId().equals(id))
+      .findFirst()
+      .orElse(null);
+  }
+
   public ServiceJourney getServiceJourney() {
     return serviceJourneyById.lookup(SERVICE_JOURNEY_ID);
   }

src/test/java/org/opentripplanner/netex/mapping/TripPatternMapperTest.java

@@ -1,9 +1,12 @@
 package org.opentripplanner.netex.mapping;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import com.google.common.collect.ArrayListMultimap;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import org.junit.jupiter.api.Test;
@@ -19,17 +22,19 @@
 import org.opentripplanner.transit.model.timetable.TripOnServiceDate;
 import org.opentripplanner.transit.model.timetable.TripTimes;
 import org.rutebanken.netex.model.DatedServiceJourney;
+import org.rutebanken.netex.model.DatedServiceJourneyRefStructure;
 import org.rutebanken.netex.model.OperatingDay;
+import org.rutebanken.netex.model.ServiceAlterationEnumeration;
 
 /**
  * @author Thomas Gran (Capra) - [email protected] (29.11.2017)
  */
-public class TripPatternMapperTest {
+class TripPatternMapperTest {
 
   private static final FeedScopedId SERVICE_ID = TransitModelForTest.id("S01");
 
   @Test
-  public void testMapTripPattern() {
+  void testMapTripPattern() {
     NetexTestDataSample sample = new NetexTestDataSample();
 
     TripPatternMapper tripPatternMapper = new TripPatternMapper(
@@ -88,9 +93,85 @@ public void testMapTripPattern() {
   }
 
   @Test
-  public void testMapTripPattern_datedServiceJourney() {
+  void testMapTripPattern_datedServiceJourney() {
     NetexTestDataSample sample = new NetexTestDataSample();
+    Optional<TripPatternMapperResult> res = mapTripPattern(sample);
 
+    assertTrue(res.isPresent());
+
+    var r = res.get();
+
+    assertEquals(2, r.tripOnServiceDates().size());
+
+    Trip trip = r.tripPattern().scheduledTripsAsStream().findFirst().get();
+
+    for (TripOnServiceDate tripOnServiceDate : r.tripOnServiceDates()) {
+      assertEquals(trip, tripOnServiceDate.getTrip());
+      assertEquals(TripAlteration.PLANNED, tripOnServiceDate.getTripAlteration());
+      assertEquals(
+        1,
+        sample
+          .getOperatingDaysById()
+          .localValues()
+          .stream()
+          .map(OperatingDay::getId)
+          .filter(id -> id.equals(tripOnServiceDate.getServiceDate().toString()))
+          .count()
+      );
+    }
+  }
+
+  @Test
+  void testDatedServiceJourneyReplacement() {
+    NetexTestDataSample sample = new NetexTestDataSample();
+    DatedServiceJourney dsjReplaced = sample.getDatedServiceJourneyById(
+      NetexTestDataSample.DATED_SERVICE_JOURNEY_ID_1
+    );
+    dsjReplaced.setServiceAlteration(ServiceAlterationEnumeration.REPLACED);
+    DatedServiceJourney dsjReplacing = sample.getDatedServiceJourneyById(
+      NetexTestDataSample.DATED_SERVICE_JOURNEY_ID_2
+    );
+    dsjReplacing.withJourneyRef(
+      List.of(
+        MappingSupport.createWrappedRef(dsjReplaced.getId(), DatedServiceJourneyRefStructure.class)
+      )
+    );
+    Optional<TripPatternMapperResult> res = mapTripPattern(sample);
+
+    assertTrue(res.isPresent());
+    var r = res.get();
+    Optional<TripOnServiceDate> replacedTripOnServiceDate = r
+      .tripOnServiceDates()
+      .stream()
+      .filter(tripOnServiceDate ->
+        NetexTestDataSample.DATED_SERVICE_JOURNEY_ID_1.equals(tripOnServiceDate.getId().getId())
+      )
+      .findFirst();
+
+    assertTrue(replacedTripOnServiceDate.isPresent());
+    assertEquals(TripAlteration.REPLACED, replacedTripOnServiceDate.get().getTripAlteration());
+
+    Optional<TripOnServiceDate> replacingTripOnServiceDate = r
+      .tripOnServiceDates()
+      .stream()
+      .filter(tripOnServiceDate ->
+        NetexTestDataSample.DATED_SERVICE_JOURNEY_ID_2.equals(tripOnServiceDate.getId().getId())
+      )
+      .findFirst();
+
+    assertTrue(replacingTripOnServiceDate.isPresent());
+    assertEquals(TripAlteration.PLANNED, replacingTripOnServiceDate.get().getTripAlteration());
+    assertFalse(replacingTripOnServiceDate.get().getReplacementFor().isEmpty());
+
+    // the replaced trip should refer to the same object (object identity) whether it is accessed
+    // directly from the replaced DSJ or indirectly through the replacing DSJ.
+    assertSame(
+      replacingTripOnServiceDate.get().getReplacementFor().getFirst().getTrip(),
+      replacedTripOnServiceDate.get().getTrip()
+    );
+  }
+
+  private static Optional<TripPatternMapperResult> mapTripPattern(NetexTestDataSample sample) {
     HierarchicalMapById<DatedServiceJourney> datedServiceJourneys = new HierarchicalMapById<>();
     datedServiceJourneys.addAll(sample.getDatedServiceJourneyBySjId().values());
 
@@ -121,28 +202,6 @@ public void testMapTripPattern_datedServiceJourney() {
     Optional<TripPatternMapperResult> res = tripPatternMapper.mapTripPattern(
       sample.getJourneyPattern()
     );
-
-    assertTrue(res.isPresent());
-
-    var r = res.get();
-
-    assertEquals(2, r.tripOnServiceDates().size());
-
-    Trip trip = r.tripPattern().scheduledTripsAsStream().findFirst().get();
-
-    for (TripOnServiceDate tripOnServiceDate : r.tripOnServiceDates()) {
-      assertEquals(trip, tripOnServiceDate.getTrip());
-      assertEquals(TripAlteration.PLANNED, tripOnServiceDate.getTripAlteration());
-      assertEquals(
-        1,
-        sample
-          .getOperatingDaysById()
-          .localValues()
-          .stream()
-          .map(OperatingDay::getId)
-          .filter(id -> id.equals(tripOnServiceDate.getServiceDate().toString()))
-          .count()
-      );
-    }
+    return res;
   }
 }