Fix filtering of inconsistent NeTEx stop times

src/main/java/org/opentripplanner/graph_builder/module/ValidateAndInterpolateStopTimesForEachTrip.java

@@ -8,7 +8,6 @@
 import org.opentripplanner.ext.flex.trip.FlexTrip;
 import org.opentripplanner.framework.application.OTPFeature;
 import org.opentripplanner.framework.geometry.SphericalDistanceLibrary;
-import org.opentripplanner.framework.logging.ProgressTracker;
 import org.opentripplanner.graph_builder.issue.api.DataImportIssue;
 import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore;
 import org.opentripplanner.graph_builder.issues.HopSpeedFast;
@@ -23,19 +22,13 @@
 import org.opentripplanner.transit.model.basic.TransitMode;
 import org.opentripplanner.transit.model.site.RegularStop;
 import org.opentripplanner.transit.model.timetable.Trip;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /**
  * This class is responsible for making sure, that all trips have arrival and departure times for
  * all stops. It also removes all stop times for trips, which have invalid stop times.
  */
 public class ValidateAndInterpolateStopTimesForEachTrip {
 
-  private static final Logger LOG = LoggerFactory.getLogger(
-    ValidateAndInterpolateStopTimesForEachTrip.class
-  );
-
   /** Do not report zero-time hops less than 1km */
   private static final double MIN_ZERO_TIME_HOP_DISTANCE_METERS = 1000.0;
 
@@ -57,10 +50,6 @@ public ValidateAndInterpolateStopTimesForEachTrip(
   }
 
   public void run() {
-    final int tripSize = stopTimesByTrip.size();
-    var progress = ProgressTracker.track("Validate StopTimes", 100_000, tripSize);
-    LOG.info(progress.startMessage());
-
     for (Trip trip : stopTimesByTrip.keys()) {
       // Fetch the stop times for this trip. Copy the list since it's immutable.
       List<StopTime> stopTimes = new ArrayList<>(stopTimesByTrip.get(trip));
@@ -82,15 +71,12 @@ public void run() {
         interpolateStopTimes(stopTimes);
         stopTimesByTrip.replace(trip, stopTimes);
       } else {
-        stopTimes.removeIf(st -> !st.isArrivalTimeSet() || !st.isDepartureTimeSet());
+        stopTimes.removeIf(st ->
+          (!st.isArrivalTimeSet() || !st.isDepartureTimeSet()) && !st.isFlexWindowSet()
+        );
         stopTimesByTrip.replace(trip, stopTimes);
       }
-
-      //noinspection Convert2MethodRef
-      progress.step(m -> LOG.info(m));
     }
-
-    LOG.info(progress.completeMessage());
   }
 
   /**

src/main/java/org/opentripplanner/model/StopTime.java

@@ -234,6 +234,10 @@ public void setFarePeriodId(String farePeriodId) {
     this.farePeriodId = farePeriodId;
   }
 
+  public boolean isFlexWindowStartSet() {
+    return flexWindowStart != MISSING_VALUE;
+  }
+
   public int getFlexWindowStart() {
     return flexWindowStart;
   }
@@ -242,6 +246,10 @@ public void setFlexWindowStart(int flexWindowStart) {
     this.flexWindowStart = flexWindowStart;
   }
 
+  public boolean isFlexWindowEndSet() {
+    return flexWindowEnd != MISSING_VALUE;
+  }
+
   public int getFlexWindowEnd() {
     return flexWindowEnd;
   }
@@ -250,6 +258,10 @@ public void setFlexWindowEnd(int flexWindowEnd) {
     this.flexWindowEnd = flexWindowEnd;
   }
 
+  public boolean isFlexWindowSet() {
+    return isFlexWindowStartSet() && isFlexWindowEndSet();
+  }
+
   /** Get either the start of the flex window or the departure time, whichever is set */
   public int getEarliestPossibleDepartureTime() {
     return getAvailableTime(getFlexWindowStart(), getDepartureTime());

src/main/java/org/opentripplanner/netex/NetexModule.java

@@ -6,9 +6,7 @@
 import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore;
 import org.opentripplanner.graph_builder.model.GraphBuilderModule;
 import org.opentripplanner.graph_builder.module.AddTransitModelEntitiesToGraph;
-import org.opentripplanner.graph_builder.module.ValidateAndInterpolateStopTimesForEachTrip;
 import org.opentripplanner.model.OtpTransitService;
-import org.opentripplanner.model.TripStopTimes;
 import org.opentripplanner.model.calendar.CalendarServiceData;
 import org.opentripplanner.model.calendar.ServiceDateInterval;
 import org.opentripplanner.model.impl.OtpTransitServiceBuilder;
@@ -79,8 +77,6 @@ public void buildGraph() {
             .addAll(FlexTripsMapper.createFlexTrips(transitBuilder, issueStore));
         }
 
-        validateStopTimesForEachTrip(transitBuilder.getStopTimesSortedByTrip());
-
         OtpTransitService otpService = transitBuilder.build();
 
         // if this or previously processed netex bundle has transit that has not been filtered out
@@ -108,10 +104,6 @@ public void buildGraph() {
     }
   }
 
-  private void validateStopTimesForEachTrip(TripStopTimes stopTimesByTrip) {
-    new ValidateAndInterpolateStopTimesForEachTrip(stopTimesByTrip, false, false, issueStore).run();
-  }
-
   @Override
   public void checkInputs() {
     netexBundles.forEach(NetexBundle::checkInputs);

src/main/java/org/opentripplanner/netex/index/hierarchy/AbstractHierarchicalMap.java

@@ -3,6 +3,7 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.function.Consumer;
+import java.util.function.Supplier;
 import org.opentripplanner.graph_builder.issue.api.DataImportIssue;
 import org.opentripplanner.netex.index.api.HMapValidationRule;
 import org.opentripplanner.netex.index.api.ReadOnlyHierarchicalMap;
@@ -55,6 +56,9 @@ public int size() {
     return localSize() + (isRoot() ? 0 : parent.localSize());
   }
 
+  /**
+   * Validate a stateless rule.
+   */
   public void validate(HMapValidationRule<K, V> rule, Consumer<DataImportIssue> warnMsgConsumer) {
     List<K> discardKeys = new ArrayList<>();
     for (K key : localKeys()) {
@@ -72,6 +76,29 @@ public void validate(HMapValidationRule<K, V> rule, Consumer<DataImportIssue> wa
     discardKeys.forEach(this::localRemove);
   }
 
+  /**
+   * Validate a stateful rule.
+   */
+  public void validate(
+    Supplier<HMapValidationRule<K, V>> ruleSupplier,
+    Consumer<DataImportIssue> warnMsgConsumer
+  ) {
+    List<K> discardKeys = new ArrayList<>();
+    for (K key : localKeys()) {
+      V value = localGet(key);
+      HMapValidationRule<K, V> rule = ruleSupplier.get();
+      HMapValidationRule.Status status = rule.validate(value);
+
+      if (status == HMapValidationRule.Status.DISCARD) {
+        discardKeys.add(key);
+      }
+      if (status != HMapValidationRule.Status.OK) {
+        warnMsgConsumer.accept(rule.logMessage(key, value));
+      }
+    }
+    discardKeys.forEach(this::localRemove);
+  }
+
   /** Return a reference to the parent. */
   public AbstractHierarchicalMap<K, V> parent() {
     return parent;

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

@@ -455,7 +455,7 @@ private void mapTripPatterns(Map<String, FeedScopedId> serviceIds) {
       .localValues()) {
       TripPatternMapperResult result = tripPatternMapper.mapTripPattern(journeyPattern);
 
-      for (Map.Entry<Trip, List<StopTime>> it : result.tripStopTimes.entrySet()) {
+      for (Map.Entry<Trip, List<StopTime>> it : result.tripStopTimes.asImmutableMap().entrySet()) {
         transitBuilder.getStopTimesSortedByTrip().put(it.getKey(), it.getValue());
         transitBuilder.getTripsById().add(it.getKey());
       }

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

@@ -11,6 +11,7 @@
 import java.util.Objects;
 import java.util.stream.Collectors;
 import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore;
+import org.opentripplanner.graph_builder.module.ValidateAndInterpolateStopTimesForEachTrip;
 import org.opentripplanner.netex.index.api.ReadOnlyHierarchicalMap;
 import org.opentripplanner.netex.index.api.ReadOnlyHierarchicalMapById;
 import org.opentripplanner.netex.mapping.support.FeedScopedIdFactory;
@@ -255,13 +256,21 @@ TripPatternMapperResult mapTripPattern(JourneyPattern_VersionStructure journeyPa
       );
 
     TripPattern tripPattern = tripPatternBuilder.build();
+
+    validateStopTimes();
+
     createTripTimes(trips, tripPattern);
 
     result.tripPatterns.put(stopPattern, tripPattern);
 
     return result;
   }
 
+  private void validateStopTimes() {
+    new ValidateAndInterpolateStopTimesForEachTrip(result.tripStopTimes, false, false, issueStore)
+      .run();
+  }
+
   private void mapDatedServiceJourney(
     JourneyPattern_VersionStructure journeyPattern,
     ServiceJourney serviceJourney,

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

@@ -4,12 +4,11 @@
 import com.google.common.collect.Multimap;
 import java.util.ArrayList;
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 import org.opentripplanner.model.StopTime;
+import org.opentripplanner.model.TripStopTimes;
 import org.opentripplanner.transit.model.network.StopPattern;
 import org.opentripplanner.transit.model.network.TripPattern;
-import org.opentripplanner.transit.model.timetable.Trip;
 import org.opentripplanner.transit.model.timetable.TripOnServiceDate;
 
 /**
@@ -23,7 +22,7 @@ class TripPatternMapperResult {
    */
   final ArrayListMultimap<String, String> scheduledStopPointsIndex = ArrayListMultimap.create();
 
-  final Map<Trip, List<StopTime>> tripStopTimes = new HashMap<>();
+  final TripStopTimes tripStopTimes = new TripStopTimes();
 
   final Multimap<StopPattern, TripPattern> tripPatterns = ArrayListMultimap.create();
 

src/main/java/org/opentripplanner/netex/support/ServiceJourneyHelper.java

@@ -0,0 +1,165 @@
+package org.opentripplanner.netex.support;
+
+import java.math.BigInteger;
+import java.time.Duration;
+import java.time.LocalTime;
+import java.util.Comparator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.stream.Collectors;
+import org.rutebanken.netex.model.EntityStructure;
+import org.rutebanken.netex.model.JourneyPattern_VersionStructure;
+import org.rutebanken.netex.model.ServiceJourney;
+import org.rutebanken.netex.model.StopPointInJourneyPattern;
+import org.rutebanken.netex.model.TimetabledPassingTime;
+
+/**
+ * Utility class with helpers for NeTEx ServiceJourney.
+ */
+public class ServiceJourneyHelper {
+
+  private ServiceJourneyHelper() {}
+
+  /**
+   * Return the JourneyPattern ID of a given ServiceJourney.
+   */
+  public static String getPatternId(ServiceJourney sj) {
+    return sj.getJourneyPatternRef().getValue().getRef();
+  }
+
+  /**
+   * Return the StopPointInJourneyPattern ID of a given TimeTabledPassingTime.
+   */
+  public static String getStopPointId(TimetabledPassingTime timetabledPassingTime) {
+    return timetabledPassingTime.getPointInJourneyPatternRef().getValue().getRef();
+  }
+
+  public static Map<String, String> getScheduledStopPointIdByStopPointId(
+    JourneyPattern_VersionStructure journeyPattern
+  ) {
+    return journeyPattern
+      .getPointsInSequence()
+      .getPointInJourneyPatternOrStopPointInJourneyPatternOrTimingPointInJourneyPattern()
+      .stream()
+      .collect(
+        Collectors.toMap(
+          EntityStructure::getId,
+          p -> ((StopPointInJourneyPattern) p).getScheduledStopPointRef().getValue().getRef()
+        )
+      );
+  }
+
+  /**
+   * Return the elapsed time since midnight for a given local time, taking into account the day
+   * offset.
+   */
+  public static int elapsedTimeSinceMidnight(LocalTime time, BigInteger dayOffset) {
+    return elapsedTimeSinceMidnight(time, getDayOffset(dayOffset));
+  }
+
+  private static int elapsedTimeSinceMidnight(LocalTime time, int dayOffset) {
+    return (int) Duration
+      .between(LocalTime.MIDNIGHT, time)
+      .plus(Duration.ofDays(dayOffset))
+      .toSeconds();
+  }
+
+  public static int normalizedDepartureTime(TimetabledPassingTime timetabledPassingTime) {
+    Objects.requireNonNull(timetabledPassingTime.getDepartureTime());
+    return elapsedTimeSinceMidnight(
+      timetabledPassingTime.getDepartureTime(),
+      timetabledPassingTime.getDepartureDayOffset()
+    );
+  }
+
+  public static int normalizedArrivalTime(TimetabledPassingTime timetabledPassingTime) {
+    Objects.requireNonNull(timetabledPassingTime.getArrivalTime());
+    return elapsedTimeSinceMidnight(
+      timetabledPassingTime.getArrivalTime(),
+      timetabledPassingTime.getArrivalDayOffset()
+    );
+  }
+
+  public static int normalizedEarliestDepartureTime(TimetabledPassingTime timetabledPassingTime) {
+    Objects.requireNonNull(timetabledPassingTime.getEarliestDepartureTime());
+    return elapsedTimeSinceMidnight(
+      timetabledPassingTime.getEarliestDepartureTime(),
+      timetabledPassingTime.getEarliestDepartureDayOffset()
+    );
+  }
+
+  public static int normalizedLatestArrivalTime(TimetabledPassingTime timetabledPassingTime) {
+    Objects.requireNonNull(timetabledPassingTime.getLatestArrivalTime());
+    return elapsedTimeSinceMidnight(
+      timetabledPassingTime.getLatestArrivalTime(),
+      timetabledPassingTime.getLatestArrivalDayOffset()
+    );
+  }
+
+  /**
+   * Return the elapsed time since midnight for a given departure time, taking into account the day
+   * offset. Fallback to arrival time if departure time is missing.
+   */
+  public static int normalizedDepartureTimeOrElseArrivalTime(
+    TimetabledPassingTime timetabledPassingTime
+  ) {
+    if (timetabledPassingTime.getDepartureTime() != null) {
+      return elapsedTimeSinceMidnight(
+        timetabledPassingTime.getDepartureTime(),
+        timetabledPassingTime.getDepartureDayOffset()
+      );
+    } else {
+      return elapsedTimeSinceMidnight(
+        timetabledPassingTime.getArrivalTime(),
+        timetabledPassingTime.getArrivalDayOffset()
+      );
+    }
+  }
+
+  /**
+   * Return the elapsed time since midnight for a given arrival time, taking into account the day
+   * offset. Fallback to departure time if arrival time is missing.
+   */
+  public static int normalizedArrivalTimeOrElseDepartureTime(
+    TimetabledPassingTime timetabledPassingTime
+  ) {
+    if (timetabledPassingTime.getArrivalTime() != null) {
+      return elapsedTimeSinceMidnight(
+        timetabledPassingTime.getArrivalTime(),
+        timetabledPassingTime.getArrivalDayOffset()
+      );
+    } else return elapsedTimeSinceMidnight(
+      timetabledPassingTime.getDepartureTime(),
+      timetabledPassingTime.getDepartureDayOffset()
+    );
+  }
+
+  /**
+   * Sort the timetabled passing times according to their order in the journey pattern.
+   */
+  public static List<TimetabledPassingTime> getOrderedPassingTimes(
+    JourneyPattern_VersionStructure journeyPattern,
+    ServiceJourney serviceJourney
+  ) {
+    Map<String, Integer> stopPointIdToOrder = journeyPattern
+      .getPointsInSequence()
+      .getPointInJourneyPatternOrStopPointInJourneyPatternOrTimingPointInJourneyPattern()
+      .stream()
+      .collect(Collectors.toMap(EntityStructure::getId, point -> point.getOrder().intValueExact()));
+    return serviceJourney
+      .getPassingTimes()
+      .getTimetabledPassingTime()
+      .stream()
+      .sorted(
+        Comparator.comparing(timetabledPassingTime ->
+          stopPointIdToOrder.get(getStopPointId(timetabledPassingTime))
+        )
+      )
+      .toList();
+  }
+
+  private static int getDayOffset(BigInteger offset) {
+    return offset != null ? offset.intValueExact() : 0;
+  }
+}