From 108f0a21437c8eb76e6479facaedd2b3dbd30fc0 Mon Sep 17 00:00:00 2001
From: Vincent Paturet <vincent.paturet@entur.org>
Date: Thu, 27 Jun 2024 16:45:00 +0200
Subject: [PATCH] Prevent modifications on read-only timetables

---
 .../model/TimetableSnapshot.java              | 14 +++---
 .../model/TimetableSnapshotTest.java          | 47 +++++++++++++++++++
 2 files changed, 54 insertions(+), 7 deletions(-)

diff --git a/src/main/java/org/opentripplanner/model/TimetableSnapshot.java b/src/main/java/org/opentripplanner/model/TimetableSnapshot.java
index ac4de0ce38c..9b3cacc4c47 100644
--- a/src/main/java/org/opentripplanner/model/TimetableSnapshot.java
+++ b/src/main/java/org/opentripplanner/model/TimetableSnapshot.java
@@ -266,7 +266,7 @@ public TimetableSnapshot commit(TransitLayerUpdater transitLayerUpdater, boolean
     this.dirtyTimetables.clear();
     this.dirty = false;
 
-    ret.setPatternsForStop(ImmutableSetMultimap.copyOf(patternsForStop));
+    ret.patternsForStop = ImmutableSetMultimap.copyOf(patternsForStop);
 
     ret.readOnly = true; // mark the snapshot as henceforth immutable
     return ret;
@@ -303,6 +303,10 @@ public void clear(String feedId) {
    * message and an attempt was made to re-associate it with its originally scheduled trip pattern.
    */
   public boolean revertTripToScheduledTripPattern(FeedScopedId tripId, LocalDate serviceDate) {
+    if (readOnly) {
+      throw new ConcurrentModificationException("This TimetableSnapshot is read-only.");
+    }
+
     boolean success = false;
 
     final TripPattern pattern = getRealtimeAddedTripPattern(tripId, serviceDate);
@@ -406,10 +410,6 @@ public Collection<TripPattern> getPatternsForStop(StopLocation stop) {
     return patternsForStop.get(stop);
   }
 
-  public void setPatternsForStop(SetMultimap<StopLocation, TripPattern> patternsForStop) {
-    this.patternsForStop = patternsForStop;
-  }
-
   /**
    * Does this snapshot contain any realtime data or is it completely empty?
    */
@@ -423,7 +423,7 @@ public boolean isEmpty() {
    * @param feedId feed id to clear out
    * @return true if the timetable changed as a result of the call
    */
-  protected boolean clearTimetable(String feedId) {
+  private boolean clearTimetable(String feedId) {
     return timetables.keySet().removeIf(tripPattern -> feedId.equals(tripPattern.getFeedId()));
   }
 
@@ -433,7 +433,7 @@ protected boolean clearTimetable(String feedId) {
    * @param feedId feed id to clear out
    * @return true if the realtimeAddedTripPattern changed as a result of the call
    */
-  protected boolean clearRealtimeAddedTripPattern(String feedId) {
+  private boolean clearRealtimeAddedTripPattern(String feedId) {
     return realtimeAddedTripPattern
       .keySet()
       .removeIf(realtimeAddedTripPattern ->
diff --git a/src/test/java/org/opentripplanner/model/TimetableSnapshotTest.java b/src/test/java/org/opentripplanner/model/TimetableSnapshotTest.java
index 26d93f6a848..89c30da79e4 100644
--- a/src/test/java/org/opentripplanner/model/TimetableSnapshotTest.java
+++ b/src/test/java/org/opentripplanner/model/TimetableSnapshotTest.java
@@ -5,6 +5,7 @@
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNotSame;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import com.google.transit.realtime.GtfsRealtime.TripDescriptor;
@@ -261,6 +262,52 @@ public void testPurge() {
     assertFalse(resolver.isDirty());
   }
 
+  @Test
+  void testCannotUpdateReadOnlyTimetableSnapshot() {
+    TimetableSnapshot committedSnapshot = createCommittedSnapshot();
+    LocalDate today = LocalDate.now(timeZone);
+    TripPattern pattern = patternIndex.get(new FeedScopedId(feedId, "1.1"));
+    assertThrows(
+      ConcurrentModificationException.class,
+      () -> committedSnapshot.update(pattern, null, today)
+    );
+  }
+
+  @Test
+  void testCannotCommitReadOnlyTimetableSnapshot() {
+    TimetableSnapshot committedSnapshot = createCommittedSnapshot();
+    assertThrows(ConcurrentModificationException.class, () -> committedSnapshot.commit(null, true));
+  }
+
+  @Test
+  void testCannotClearReadOnlyTimetableSnapshot() {
+    TimetableSnapshot committedSnapshot = createCommittedSnapshot();
+    assertThrows(ConcurrentModificationException.class, () -> committedSnapshot.clear(null));
+  }
+
+  @Test
+  void testCannotPurgeReadOnlyTimetableSnapshot() {
+    TimetableSnapshot committedSnapshot = createCommittedSnapshot();
+    assertThrows(
+      ConcurrentModificationException.class,
+      () -> committedSnapshot.purgeExpiredData(null)
+    );
+  }
+
+  @Test
+  void testCannotRevertReadOnlyTimetableSnapshot() {
+    TimetableSnapshot committedSnapshot = createCommittedSnapshot();
+    assertThrows(
+      ConcurrentModificationException.class,
+      () -> committedSnapshot.revertTripToScheduledTripPattern(null, null)
+    );
+  }
+
+  private static TimetableSnapshot createCommittedSnapshot() {
+    TimetableSnapshot timetableSnapshot = new TimetableSnapshot();
+    return timetableSnapshot.commit(null, true);
+  }
+
   private Result<?, UpdateError> updateResolver(
     TimetableSnapshot resolver,
     TripPattern pattern,