Merge remote-tracking branch 'entur/otp2_fix_tripTimes_min_sanity_che…

…ck' into otp2_entur_develop
entur · Dec 21, 2023 · 464a0bc · 464a0bc
2 parents ada79c0 + e55b0db
commit 464a0bc
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 9 deletions.
diff --git a/src/main/java/org/opentripplanner/framework/error/DefaultOtpError.java b/src/main/java/org/opentripplanner/framework/error/DefaultOtpError.java
@@ -0,0 +1,29 @@
+package org.opentripplanner.framework.error;
+
+class DefaultOtpError implements OtpError {
+
+  private final String errorCode;
+  private final String messageTemplate;
+  private final Object[] messageArguments;
+
+  public DefaultOtpError(String errorCode, String messageTemplate, Object... messageArguments) {
+    this.errorCode = errorCode;
+    this.messageTemplate = messageTemplate;
+    this.messageArguments = messageArguments;
+  }
+
+  @Override
+  public String errorCode() {
+    return errorCode;
+  }
+
+  @Override
+  public String messageTemplate() {
+    return messageTemplate;
+  }
+
+  @Override
+  public Object[] messageArguments() {
+    return messageArguments;
+  }
+}
diff --git a/src/main/java/org/opentripplanner/framework/error/OtpError.java b/src/main/java/org/opentripplanner/framework/error/OtpError.java
@@ -34,4 +34,11 @@ public interface OtpError {
   default String message() {
     return String.format(Locale.ROOT, messageTemplate(), messageArguments());
   }
+
+  /**
+   * Factory method to create an OTPError.
+   */
+  static OtpError of(String errorCode, String messageTemplate, Object... messageArguments) {
+    return new DefaultOtpError(errorCode, messageTemplate, messageArguments);
+  }
 }
diff --git a/src/main/java/org/opentripplanner/transit/model/timetable/ScheduledTripTimes.java b/src/main/java/org/opentripplanner/transit/model/timetable/ScheduledTripTimes.java
@@ -11,9 +11,11 @@
 import java.util.OptionalInt;
 import java.util.function.Supplier;
 import javax.annotation.Nullable;
+import org.opentripplanner.framework.error.OtpError;
 import org.opentripplanner.framework.i18n.I18NString;
 import org.opentripplanner.framework.lang.IntUtils;
 import org.opentripplanner.framework.time.DurationUtils;
+import org.opentripplanner.framework.time.TimeUtils;
 import org.opentripplanner.model.BookingInfo;
 import org.opentripplanner.transit.model.basic.Accessibility;
 import org.opentripplanner.transit.model.framework.DataValidationException;
@@ -134,7 +136,7 @@ public int getServiceCode() {
 
   @Override
   public int getScheduledArrivalTime(final int stop) {
-    return arrivalTimes[stop] + timeShift;
+    return timeShifted(arrivalTimes[stop]);
   }
 
   @Override
@@ -144,12 +146,12 @@ public int getArrivalTime(final int stop) {
 
   @Override
   public int getArrivalDelay(final int stop) {
-    return getArrivalTime(stop) - (arrivalTimes[stop] + timeShift);
+    return getArrivalTime(stop) - timeShifted(arrivalTimes[stop]);
   }
 
   @Override
   public int getScheduledDepartureTime(final int stop) {
-    return departureTimes[stop] + timeShift;
+    return timeShifted(departureTimes[stop]);
   }
 
   @Override
@@ -159,7 +161,7 @@ public int getDepartureTime(final int stop) {
 
   @Override
   public int getDepartureDelay(final int stop) {
-    return getDepartureTime(stop) - (departureTimes[stop] + timeShift);
+    return getDepartureTime(stop) - timeShifted(departureTimes[stop]);
   }
 
   @Override
@@ -314,9 +316,9 @@ I18NString[] copyHeadsigns(Supplier<I18NString[]> defaultValue) {
   /* private methods */
 
   private void validate() {
-    int lastStop = departureTimes.length - 1;
-    IntUtils.requireInRange(departureTimes[0], MIN_TIME, MAX_TIME);
-    IntUtils.requireInRange(arrivalTimes[lastStop], MIN_TIME, MAX_TIME);
+    // Validate first departure time and last arrival time
+    validateTimeInRange("departureTime", departureTimes, 0);
+    validateTimeInRange("arrivalTime", arrivalTimes, arrivalTimes.length - 1);
     // TODO: This class is used by FLEX, so we can not validate increasing TripTimes
     // validateNonIncreasingTimes();
   }
@@ -356,4 +358,27 @@ private void validateNonIncreasingTimes() {
       prevDep = dep;
     }
   }
+
+  private int timeShifted(int time) {
+    return timeShift + time;
+  }
+
+  private void validateTimeInRange(String field, int[] times, int stopPos) {
+    int t = timeShifted(times[stopPos]);
+
+    if (t < MIN_TIME || t > MAX_TIME) {
+      throw new DataValidationException(
+        OtpError.of(
+          "TripTimeOutOfRange",
+          "The %s is not in range[%s, %s]. Time: %s, stop-pos: %d, trip: %s.",
+          field,
+          DurationUtils.durationToStr(MIN_TIME),
+          DurationUtils.durationToStr(MAX_TIME),
+          TimeUtils.timeToStrLong(t),
+          stopPos,
+          trip.getId()
+        )
+      );
+    }
+  }
 }
diff --git a/src/test/java/org/opentripplanner/transit/model/timetable/ScheduledTripTimesTest.java b/src/test/java/org/opentripplanner/transit/model/timetable/ScheduledTripTimesTest.java
@@ -13,6 +13,7 @@
 import org.opentripplanner.framework.time.TimeUtils;
 import org.opentripplanner.transit.model._data.TransitModelForTest;
 import org.opentripplanner.transit.model.basic.Accessibility;
+import org.opentripplanner.transit.model.framework.DataValidationException;
 import org.opentripplanner.transit.model.framework.FeedScopedId;
 
 class ScheduledTripTimesTest {
@@ -102,7 +103,7 @@ void isTimepoint() {
   @Test
   void validateLastArrivalTimeIsNotMoreThan20DaysAfterFirstDepartureTime() {
     var ex = assertThrows(
-      IllegalArgumentException.class,
+      DataValidationException.class,
       () ->
         ScheduledTripTimes
           .of()
@@ -111,7 +112,10 @@ void validateLastArrivalTimeIsNotMoreThan20DaysAfterFirstDepartureTime() {
           .withTrip(TRIP)
           .build()
     );
-    assertEquals("The value is not in range[-43200, 1728000]: 1728001", ex.getMessage());
+    assertEquals(
+      "The arrivalTime is not in range[-12h, 20d]. Time: 10:00:01+20d, stop-pos: 2, trip: F:Trip-1.",
+      ex.getMessage()
+    );
   }
 
   @Test