Prevent feed id from being used twice

src/main/java/org/opentripplanner/gtfs/graphbuilder/GtfsModule.java

@@ -120,12 +120,20 @@ public void buildGraph() {
 
     boolean hasTransit = false;
 
+    Map<String, GtfsBundle> feedIdsEncountered = new HashMap<>();
+
     try {
       for (GtfsBundle gtfsBundle : gtfsBundles) {
         GtfsMutableRelationalDao gtfsDao = loadBundle(gtfsBundle);
+
+        final String feedId = gtfsBundle.getFeedId().getId();
+        verifyUniqueFeedId(gtfsBundle, feedIdsEncountered, feedId);
+
+        feedIdsEncountered.put(feedId, gtfsBundle);
+
         GTFSToOtpTransitServiceMapper mapper = new GTFSToOtpTransitServiceMapper(
           new OtpTransitServiceBuilder(transitModel.getStopModel(), issueStore),
-          gtfsBundle.getFeedId().getId(),
+          feedId,
           issueStore,
           gtfsBundle.discardMinTransferTimes(),
           gtfsDao,
@@ -203,6 +211,32 @@ public void buildGraph() {
     transitModel.updateCalendarServiceData(hasTransit, calendarServiceData, issueStore);
   }
 
+  /**
+   * Verifies that a feed id is not assigned twice.
+   * <p>
+   * Duplicates can happen in the following cases:
+   *  - the feed id is configured twice in build-config.json
+   *  - two GTFS feeds have the same feed_info.feed_id
+   *  - a GTFS feed defines a feed_info.feed_id like '3' that collides with an auto-generated one
+   * <p>
+   * Debugging these cases is very confusing, so we prevent it from happening.
+   */
+  private static void verifyUniqueFeedId(
+    GtfsBundle gtfsBundle,
+    Map<String, GtfsBundle> feedIdsEncountered,
+    String feedId
+  ) {
+    if (feedIdsEncountered.containsKey(feedId)) {
+      LOG.error(
+        "Feed id '{}' has been used for {} but it was already assigned to {}.",
+        feedId,
+        gtfsBundle,
+        feedIdsEncountered.get(feedId)
+      );
+      throw new IllegalArgumentException("Duplicate feed id: '%s'".formatted(feedId));
+    }
+  }
+
   @Override
   public void checkInputs() {
     for (GtfsBundle bundle : gtfsBundles) {

src/test/java/org/opentripplanner/graph_builder/module/GtfsModuleTest.java

@@ -2,6 +2,7 @@
 
 import static graphql.Assert.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import java.util.List;
 import java.util.stream.Collectors;
@@ -23,7 +24,7 @@
 class GtfsModuleTest {
 
   @Test
-  public void addShapesForFrequencyTrips() {
+  void addShapesForFrequencyTrips() {
     var model = buildTestModel();
 
     var bundle = new GtfsBundle(ConstantsForTests.SIMPLE_GTFS);
@@ -44,7 +45,7 @@ public void addShapesForFrequencyTrips() {
 
     assertEquals(1, frequencyTripPattern.size());
 
-    var tripPattern = frequencyTripPattern.get(0);
+    var tripPattern = frequencyTripPattern.getFirst();
     assertNotNull(tripPattern.getGeometry());
     assertNotNull(tripPattern.getHopGeometry(0));
 
@@ -53,6 +54,21 @@ public void addShapesForFrequencyTrips() {
     assertNotNull(pattern.getHopGeometry(0));
   }
 
+  @Test
+  void duplicateFeedId() {
+
+    var bundles = List.of(bundle("A"), bundle("A"));
+    var model = buildTestModel();
+
+    var module = new GtfsModule(
+      bundles,
+      model.transitModel,
+      model.graph,
+      ServiceDateInterval.unbounded()
+    );
+    assertThrows(IllegalArgumentException.class, module::buildGraph);
+  }
+
   private static TestModels buildTestModel() {
     var deduplicator = new Deduplicator();
     var stopModel = new StopModel();
@@ -63,15 +79,15 @@ private static TestModels buildTestModel() {
 
   record TestModels(Graph graph, TransitModel transitModel) {}
 
+  static GtfsBundle bundle(String feedId) {
+    var b = new GtfsBundle(ResourceLoader.of(GtfsModuleTest.class).file("/gtfs/interlining"));
+    b.setFeedId(new GtfsFeedId.Builder().id(feedId).build());
+    return b;
+  }
+
   @Nested
   class Interlining {
 
-    static GtfsBundle bundle(String feedId) {
-      var b = new GtfsBundle(ResourceLoader.of(GtfsModuleTest.class).file("/gtfs/interlining"));
-      b.setFeedId(new GtfsFeedId.Builder().id(feedId).build());
-      return b;
-    }
-
     static List<Arguments> interliningCases() {
       return List.of(
         Arguments.of(List.of(bundle("A")), 2),
@@ -86,7 +102,7 @@ static List<Arguments> interliningCases() {
      */
     @ParameterizedTest(name = "Bundles {0} should generate {1} stay-seated transfers")
     @MethodSource("interliningCases")
-    public void interline(List<GtfsBundle> bundles, int expectedTransfers) {
+    void interline(List<GtfsBundle> bundles, int expectedTransfers) {
       var model = buildTestModel();
 
       var feedIds = bundles.stream().map(GtfsBundle::getFeedId).collect(Collectors.toSet());