diff --git a/src/main/java/org/opentripplanner/graph_builder/module/GtfsFeedId.java b/src/main/java/org/opentripplanner/graph_builder/module/GtfsFeedId.java index dd930d7190c..a9522892e43 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/GtfsFeedId.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/GtfsFeedId.java @@ -19,7 +19,7 @@ public class GtfsFeedId { private static int FEED_ID_COUNTER = 1; /** - * A Map that keeps track of which feed ids were generated in the {@link Builder#build()} method to make sure there + * A Set that keeps track of which feed ids were generated in the {@link Builder#build()} method to make sure there * are no duplicates. */ private static final Set seenIds = new HashSet<>(); @@ -118,11 +118,14 @@ protected String cleanId(String id) { public GtfsFeedId build() { boolean autoGenerated = false; id = cleanId(id); + // auto-generate a new ID if it is null or a zero-length string if (id == null || id.trim().length() == 0) { id = String.valueOf(FEED_ID_COUNTER); autoGenerated = true; } - if (seenIds.contains(id)) { + // add the feed ID to the set of IDs seen so far. If the set already contained the ID, throw an error in + // order to avoid using duplicate feed IDs. + if (!seenIds.add(id)) { throw new RuntimeException( String.format( "A duplicate Feed ID of `%s` was encountered when loading GTFS data!", @@ -130,7 +133,6 @@ public GtfsFeedId build() { ) ); } - seenIds.add(id); FEED_ID_COUNTER++; return new GtfsFeedId(id, autoGenerated); } diff --git a/src/main/java/org/opentripplanner/graph_builder/module/GtfsModule.java b/src/main/java/org/opentripplanner/graph_builder/module/GtfsModule.java index 0e91494232b..86fe4862763 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/GtfsModule.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/GtfsModule.java @@ -233,7 +233,7 @@ private GtfsMutableRelationalDao loadBundle(GtfsBundle gtfsBundle) if (bundleFilenamesWithAutoGeneratedFeedId.contains(bundleFilename)) { throw new RuntimeException( String.format( - "There are at least 2 bundles with the name `%s` that lack a feed_id value in a record in the feed_info.txt table. This will result in non-deterministic Feed ID generation!", + "There are at least 2 GTFS bundles with the name `%s` that lack a feed_id value in a record in the feed_info.txt table. This will result in non-deterministic Feed ID generation!", bundleFilename ) ); @@ -244,7 +244,8 @@ private GtfsMutableRelationalDao loadBundle(GtfsBundle gtfsBundle) } // Read the agency table to determine if there is a single agency entry without an agency ID. This is used later - // to determine whether an agency ID was auto-generated. According to the GTFS spec, if there are multiple + // to determine whether an agency ID was auto-generated (it will be auto-generated to the bundle's feed ID if + // the agency ID is missing in a single-agency feed). According to the GTFS spec, if there are multiple // agencies, then they must have an agency ID. Therefore, it is only necessary to read the first record. boolean agencyFileContainedOneAgencyWithoutId = false; InputStream agencyInputStream = gtfsBundle.getCsvInputSource().getResource("agency.txt"); @@ -264,6 +265,7 @@ private GtfsMutableRelationalDao loadBundle(GtfsBundle gtfsBundle) reader.setInputSource(gtfsBundle.getCsvInputSource()); reader.setEntityStore(store); reader.setInternStrings(true); + // Set the default Agency ID to be the Feed ID of this bundle. reader.setDefaultAgencyId(gtfsFeedId.getId()); if (LOG.isDebugEnabled()) diff --git a/src/test/java/org/opentripplanner/graph_builder/module/GtfsFeedIdTest.java b/src/test/java/org/opentripplanner/graph_builder/module/GtfsFeedIdTest.java new file mode 100644 index 00000000000..26d98bb306b --- /dev/null +++ b/src/test/java/org/opentripplanner/graph_builder/module/GtfsFeedIdTest.java @@ -0,0 +1,21 @@ +package org.opentripplanner.graph_builder.module; + +import org.junit.Test; + +public class GtfsFeedIdTest { + @Test + public void shouldUseProvidedFeedId() { + assert(new GtfsFeedId.Builder().id("abcd").build().getId().equals("abcd")); + } + + @Test + public void canAutoGenerateFeedId() { + assert(Integer.valueOf(new GtfsFeedId.Builder().id("").build().getId()) > 0); + } + + @Test(expected = RuntimeException.class) + public void throwsErrorOnDuplicateFeedId() { + new GtfsFeedId.Builder().id("abc123").build(); + new GtfsFeedId.Builder().id("abc123").build(); + } +} \ No newline at end of file