Merge pull request #5440 from entur/validate_stop_in_scheduled_transi…

…t_leg_reference

Validate stop id in Transit leg reference

src/main/java/org/opentripplanner/model/plan/ScheduledTransitLeg.java

@@ -350,7 +350,9 @@ public LegReference getLegReference() {
       tripTimes.getTrip().getId(),
       serviceDate,
       boardStopPosInPattern,
-      alightStopPosInPattern
+      alightStopPosInPattern,
+      tripPattern.getStops().get(boardStopPosInPattern).getId(),
+      tripPattern.getStops().get(alightStopPosInPattern).getId()
     );
   }
 

src/main/java/org/opentripplanner/model/plan/legreference/LegReferenceSerializer.java

@@ -1,18 +1,12 @@
 package org.opentripplanner.model.plan.legreference;
 
-import static java.time.temporal.ChronoField.DAY_OF_MONTH;
-import static java.time.temporal.ChronoField.MONTH_OF_YEAR;
-import static java.time.temporal.ChronoField.YEAR;
-
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
-import java.text.ParseException;
 import java.time.LocalDate;
 import java.time.format.DateTimeFormatter;
-import java.time.format.DateTimeFormatterBuilder;
 import java.util.Base64;
 import javax.annotation.Nullable;
 import org.opentripplanner.transit.model.framework.FeedScopedId;
@@ -26,19 +20,6 @@ public class LegReferenceSerializer {
 
   private static final Logger LOG = LoggerFactory.getLogger(LegReferenceSerializer.class);
 
-  // TODO: This is for backwards compatibility. Change to use ISO_LOCAL_DATE after OTP v2.2 is released
-  private static final DateTimeFormatter LENIENT_ISO_LOCAL_DATE = new DateTimeFormatterBuilder()
-    .appendValue(YEAR, 4)
-    .optionalStart()
-    .appendLiteral('-')
-    .optionalEnd()
-    .appendValue(MONTH_OF_YEAR, 2)
-    .optionalStart()
-    .appendLiteral('-')
-    .optionalEnd()
-    .appendValue(DAY_OF_MONTH, 2)
-    .toFormatter();
-
   /** private constructor to prevent instantiating this utility class */
   private LegReferenceSerializer() {}
 
@@ -80,31 +61,59 @@ public static LegReference decode(String legReference) {
 
       var type = readEnum(in, LegReferenceType.class);
       return type.getDeserializer().read(in);
-    } catch (IOException | ParseException e) {
+    } catch (IOException e) {
       LOG.error("Unable to decode leg reference: '" + legReference + "'", e);
       return null;
     }
   }
 
-  static void writeScheduledTransitLeg(LegReference ref, ObjectOutputStream out)
+  static void writeScheduledTransitLegV1(LegReference ref, ObjectOutputStream out)
+    throws IOException {
+    if (ref instanceof ScheduledTransitLegReference s) {
+      out.writeUTF(s.tripId().toString());
+      out.writeUTF(s.serviceDate().toString());
+      out.writeInt(s.fromStopPositionInPattern());
+      out.writeInt(s.toStopPositionInPattern());
+    } else {
+      throw new IllegalArgumentException("Invalid LegReference type");
+    }
+  }
+
+  static void writeScheduledTransitLegV2(LegReference ref, ObjectOutputStream out)
     throws IOException {
     if (ref instanceof ScheduledTransitLegReference s) {
       out.writeUTF(s.tripId().toString());
       out.writeUTF(s.serviceDate().toString());
       out.writeInt(s.fromStopPositionInPattern());
       out.writeInt(s.toStopPositionInPattern());
+      out.writeUTF(s.fromStopId().toString());
+      out.writeUTF(s.toStopId().toString());
     } else {
       throw new IllegalArgumentException("Invalid LegReference type");
     }
   }
 
-  static LegReference readScheduledTransitLeg(ObjectInputStream objectInputStream)
+  static LegReference readScheduledTransitLegV1(ObjectInputStream objectInputStream)
+    throws IOException {
+    return new ScheduledTransitLegReference(
+      FeedScopedId.parse(objectInputStream.readUTF()),
+      LocalDate.parse(objectInputStream.readUTF(), DateTimeFormatter.ISO_LOCAL_DATE),
+      objectInputStream.readInt(),
+      objectInputStream.readInt(),
+      null,
+      null
+    );
+  }
+
+  static LegReference readScheduledTransitLegV2(ObjectInputStream objectInputStream)
     throws IOException {
     return new ScheduledTransitLegReference(
       FeedScopedId.parse(objectInputStream.readUTF()),
-      LocalDate.parse(objectInputStream.readUTF(), LENIENT_ISO_LOCAL_DATE),
+      LocalDate.parse(objectInputStream.readUTF(), DateTimeFormatter.ISO_LOCAL_DATE),
+      objectInputStream.readInt(),
       objectInputStream.readInt(),
-      objectInputStream.readInt()
+      FeedScopedId.parse(objectInputStream.readUTF()),
+      FeedScopedId.parse(objectInputStream.readUTF())
     );
   }
 

src/main/java/org/opentripplanner/model/plan/legreference/LegReferenceType.java

@@ -3,40 +3,60 @@
 import java.io.IOException;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
-import java.text.ParseException;
+import java.util.Arrays;
+import java.util.Optional;
 
 /**
  * Enum for different types of LegReferences
  */
 enum LegReferenceType {
   SCHEDULED_TRANSIT_LEG_V1(
+    1,
     ScheduledTransitLegReference.class,
-    LegReferenceSerializer::writeScheduledTransitLeg,
-    LegReferenceSerializer::readScheduledTransitLeg
+    LegReferenceSerializer::writeScheduledTransitLegV1,
+    LegReferenceSerializer::readScheduledTransitLegV1
+  ),
+
+  SCHEDULED_TRANSIT_LEG_V2(
+    2,
+    ScheduledTransitLegReference.class,
+    LegReferenceSerializer::writeScheduledTransitLegV2,
+    LegReferenceSerializer::readScheduledTransitLegV2
   );
 
+  private final int version;
   private final Class<? extends LegReference> legReferenceClass;
 
   private final Writer<LegReference> serializer;
   private final Reader<? extends LegReference> deserializer;
 
   LegReferenceType(
+    int version,
     Class<? extends LegReference> legReferenceClass,
     Writer<LegReference> serializer,
     Reader<? extends LegReference> deserializer
   ) {
+    this.version = version;
     this.legReferenceClass = legReferenceClass;
     this.serializer = serializer;
     this.deserializer = deserializer;
   }
 
+  /**
+   * Return the latest LegReferenceType version for a given leg reference class.
+   */
   static LegReferenceType forClass(Class<? extends LegReference> legReferenceClass) {
-    for (var type : LegReferenceType.values()) {
-      if (type.legReferenceClass.equals(legReferenceClass)) {
-        return type;
-      }
-    }
-    return null;
+    Optional<LegReferenceType> latestVersion = Arrays
+      .stream(LegReferenceType.values())
+      .filter(legReferenceType -> legReferenceType.legReferenceClass.equals(legReferenceClass))
+      .reduce((legReferenceType, other) -> {
+        if (legReferenceType.version > other.version) {
+          return legReferenceType;
+        }
+        return other;
+      });
+
+    return latestVersion.orElse(null);
   }
 
   Writer<LegReference> getSerializer() {
@@ -54,6 +74,6 @@ interface Writer<T> {
 
   @FunctionalInterface
   interface Reader<T> {
-    T read(ObjectInputStream in) throws IOException, ParseException;
+    T read(ObjectInputStream in) throws IOException;
   }
 }

src/main/java/org/opentripplanner/model/plan/legreference/ScheduledTransitLegReference.java

@@ -9,6 +9,7 @@
 import org.opentripplanner.routing.algorithm.mapping.AlertToLegMapper;
 import org.opentripplanner.transit.model.framework.FeedScopedId;
 import org.opentripplanner.transit.model.network.TripPattern;
+import org.opentripplanner.transit.model.site.StopLocation;
 import org.opentripplanner.transit.model.timetable.Trip;
 import org.opentripplanner.transit.model.timetable.TripTimes;
 import org.opentripplanner.transit.service.TransitService;
@@ -23,7 +24,10 @@ public record ScheduledTransitLegReference(
   FeedScopedId tripId,
   LocalDate serviceDate,
   int fromStopPositionInPattern,
-  int toStopPositionInPattern
+  int toStopPositionInPattern,
+  FeedScopedId fromStopId,
+
+  FeedScopedId toStopId
 )
   implements LegReference {
   private static final Logger LOG = LoggerFactory.getLogger(ScheduledTransitLegReference.class);
@@ -35,6 +39,10 @@ public record ScheduledTransitLegReference(
    * rolled out, or because a realtime update has modified a trip),
    * it may not be possible to reconstruct the leg.
    * In this case the method returns null.
+   * The method checks that the referenced stop positions still refer to the same stop ids.
+   * As an exception, the reference is still considered valid if the referenced stop is different
+   * but belongs to the same parent station: this covers for example the case of a last-minute
+   * platform change in a train station that typically does not affect the validity of the leg.
    */
   @Override
   @Nullable
@@ -69,6 +77,18 @@ public ScheduledTransitLeg getLeg(TransitService transitService) {
       return null;
     }
 
+    if (
+      !matchReferencedStopInPattern(
+        tripPattern,
+        fromStopPositionInPattern,
+        fromStopId,
+        transitService
+      ) ||
+      !matchReferencedStopInPattern(tripPattern, toStopPositionInPattern, toStopId, transitService)
+    ) {
+      return null;
+    }
+
     Timetable timetable = transitService.getTimetableForTripPattern(tripPattern, serviceDate);
     TripTimes tripTimes = timetable.getTripTimes(trip);
 
@@ -123,4 +143,60 @@ public ScheduledTransitLeg getLeg(TransitService transitService) {
 
     return leg;
   }
+
+  /**
+   * Return false if the stop id in the reference does not match the actual stop id in the trip
+   * pattern.
+   * Return true in the specific case where the stop ids differ, but belong to the same parent
+   * station.
+   *
+   */
+  private boolean matchReferencedStopInPattern(
+    TripPattern tripPattern,
+    int stopPosition,
+    FeedScopedId stopId,
+    TransitService transitService
+  ) {
+    if (stopId == null) {
+      // this is a legacy reference, skip validation
+      // TODO: remove backward-compatible logic after OTP release 2.5
+      return true;
+    }
+
+    StopLocation stopLocationInPattern = tripPattern.getStops().get(stopPosition);
+    if (stopId.equals(stopLocationInPattern.getId())) {
+      return true;
+    }
+    StopLocation stopLocationInLegReference = transitService.getStopLocation(stopId);
+    if (
+      stopLocationInLegReference == null ||
+      stopLocationInPattern.getParentStation() == null ||
+      !stopLocationInPattern
+        .getParentStation()
+        .equals(stopLocationInLegReference.getParentStation())
+    ) {
+      LOG.info(
+        "Invalid transit leg reference:" +
+        " The referenced stop at position {} with id '{}' does not match" +
+        " the stop id '{}' in trip {} and service date {}",
+        stopPosition,
+        stopId,
+        stopLocationInPattern.getId(),
+        tripId,
+        serviceDate
+      );
+      return false;
+    }
+    LOG.info(
+      "Transit leg reference with modified stop id within the same station: " +
+      "The referenced stop at position {} with id '{}' does not match\" +\n" +
+      "        \" the stop id '{}' in trip {} and service date {}",
+      stopPosition,
+      stopId,
+      stopLocationInPattern.getId(),
+      tripId,
+      serviceDate
+    );
+    return true;
+  }
 }

src/test/java/org/opentripplanner/model/plan/legreference/LegReferenceSerializerTest.java

@@ -1,29 +1,49 @@
 package org.opentripplanner.model.plan.legreference;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 
 import java.time.LocalDate;
 import org.junit.jupiter.api.Test;
+import org.opentripplanner.transit.model._data.TransitModelForTest;
 import org.opentripplanner.transit.model.framework.FeedScopedId;
 
 class LegReferenceSerializerTest {
 
-  private static final FeedScopedId TRIP_ID = new FeedScopedId("F", "Trip");
+  private static final FeedScopedId TRIP_ID = TransitModelForTest.id("Trip");
   private static final LocalDate SERVICE_DATE = LocalDate.of(2022, 1, 31);
   private static final int FROM_STOP_POS = 1;
+
+  private static final FeedScopedId FROM_STOP_ID = TransitModelForTest.id("Boarding Stop");
   private static final int TO_STOP_POS = 3;
-  private static final String ENCODED_TOKEN =
+  private static final FeedScopedId TO_STOP_ID = TransitModelForTest.id("Alighting Stop");
+
+  /**
+   * Token based on the latest format, including stop ids.
+   */
+  private static final String ENCODED_TOKEN_V2 =
+    "rO0ABXdZABhTQ0hFRFVMRURfVFJBTlNJVF9MRUdfVjIABkY6VHJpcAAKMjAyMi0wMS0zMQAAAAEAAAADAA9GOkJvYXJkaW5nIFN0b3AAEEY6QWxpZ2h0aW5nIFN0b3A=";
+
+  /**
+   * Token based on the previous format, without stop ids.
+   */
+  private static final String ENCODED_TOKEN_V1 =
     "rO0ABXc2ABhTQ0hFRFVMRURfVFJBTlNJVF9MRUdfVjEABkY6VHJpcAAKMjAyMi0wMS0zMQAAAAEAAAAD";
-  private static final String ENCODED_LEGACY_TOKEN =
-    "rO0ABXc0ABhTQ0hFRFVMRURfVFJBTlNJVF9MRUdfVjEABkY6VHJpcAAIMjAyMjAxMzEAAAABAAAAAw==";
 
   @Test
   void testScheduledTransitLegReferenceRoundTrip() {
-    var ref = new ScheduledTransitLegReference(TRIP_ID, SERVICE_DATE, 1, 3);
+    var ref = new ScheduledTransitLegReference(
+      TRIP_ID,
+      SERVICE_DATE,
+      FROM_STOP_POS,
+      TO_STOP_POS,
+      FROM_STOP_ID,
+      TO_STOP_ID
+    );
 
     var out = LegReferenceSerializer.encode(ref);
 
-    assertEquals(ENCODED_TOKEN, out);
+    assertEquals(ENCODED_TOKEN_V2, out);
 
     var ref2 = LegReferenceSerializer.decode(out);
 
@@ -32,8 +52,8 @@ void testScheduledTransitLegReferenceRoundTrip() {
 
   @Test
   void testScheduledTransitLegReferenceDeserialize() {
-    var ref = (ScheduledTransitLegReference) LegReferenceSerializer.decode(ENCODED_TOKEN);
-
+    var ref = (ScheduledTransitLegReference) LegReferenceSerializer.decode(ENCODED_TOKEN_V2);
+    assertNotNull(ref);
     assertEquals(TRIP_ID, ref.tripId());
     assertEquals(SERVICE_DATE, ref.serviceDate());
     assertEquals(FROM_STOP_POS, ref.fromStopPositionInPattern());
@@ -42,8 +62,8 @@ void testScheduledTransitLegReferenceDeserialize() {
 
   @Test
   void testScheduledTransitLegReferenceLegacyDeserialize() {
-    var ref = (ScheduledTransitLegReference) LegReferenceSerializer.decode(ENCODED_LEGACY_TOKEN);
-
+    var ref = (ScheduledTransitLegReference) LegReferenceSerializer.decode(ENCODED_TOKEN_V1);
+    assertNotNull(ref);
     assertEquals(TRIP_ID, ref.tripId());
     assertEquals(SERVICE_DATE, ref.serviceDate());
     assertEquals(FROM_STOP_POS, ref.fromStopPositionInPattern());