Merge pull request #49 from ibi-group/add-agency-id-to-route-short

Feed ID and Agency ID Improvements

src/main/java/org/opentripplanner/graph_builder/module/GtfsFeedId.java

@@ -6,6 +6,8 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
+import java.util.HashSet;
+import java.util.Set;
 
 /**
  * Represent a feed id in a GTFS feed.
@@ -16,26 +18,41 @@ public class GtfsFeedId {
      */
     private static int FEED_ID_COUNTER = 1;
 
+    /**
+     * 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<String> seenIds = new HashSet<>();
+
     /**
      * The id for the feed
      */
     private final String id;
 
+    /**
+     * Whether the ID was autogenerated using the FEED_ID_COUNTER.
+     */
+    private final boolean autoGenerated;
+
     /**
      * Constructs a new feed id.
      *
      * If the passed id is null or an empty string a unique feed id will be generated.
      *
      * @param id The feed id
      */
-    private GtfsFeedId(String id) {
+    private GtfsFeedId(String id, boolean autoGenerated) {
         this.id = id;
+        this.autoGenerated = autoGenerated;
     }
 
     public String getId() {
         return id;
     }
 
+    public boolean isAutoGenerated() {
+        return autoGenerated;
+    }
 
     public static class Builder {
 
@@ -99,12 +116,25 @@ protected String cleanId(String id) {
          * @return A GtfsFeedId
          */
         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;
+            }
+            // 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!",
+                        id
+                    )
+                );
             }
             FEED_ID_COUNTER++;
-            return new GtfsFeedId(id);
+            return new GtfsFeedId(id, autoGenerated);
         }
     }
 }

src/main/java/org/opentripplanner/graph_builder/module/GtfsModule.java

@@ -3,10 +3,13 @@
 import java.awt.Color;
 import java.io.File;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.Serializable;
+import java.nio.charset.StandardCharsets;
 import java.util.*;
 import java.util.stream.Collectors;
 
+import com.csvreader.CsvReader;
 import org.onebusaway.csv_entities.EntityHandler;
 import org.onebusaway.gtfs.impl.GtfsRelationalDaoImpl;
 import org.onebusaway.gtfs.model.Agency;
@@ -63,6 +66,23 @@ public class GtfsModule implements GraphBuilderModule {
 
     private List<GtfsBundle> gtfsBundles;
 
+    /**
+     * A Set of bundleFilenames that had an auto-generated Feed ID. This helps keep track of whether it is possible to
+     * generate deterministic Feed IDs with this set of bundles.
+     */
+    private final Set<String> bundleFilenamesWithAutoGeneratedFeedId = new HashSet<>();
+
+    /**
+     * A Set to keep track of which gtfsFeedIds were auto-generated.
+     */
+    private final Set<String> autoGeneratedFeedIds = new HashSet<>();
+
+    /**
+     * A Set that keeps track of which agencies had auto-generated IDs. The values in here consist of the strings in the
+     * template `{FeedID}{AgencyID}`.
+     */
+    private final Set<String> autoGeneratedAgencyIds = new HashSet<>();
+
     public Boolean getUseCached() {
         return useCached;
     }
@@ -165,6 +185,32 @@ public void buildGraph(Graph graph, GraphBuilderModuleSummary graphBuilderModule
 
         graph.hasTransit = true;
         graph.calculateTransitCenter();
+
+        // log information about the feed and agency IDs now present in the graph
+        LOG.info("Loaded the following feeds and agencies into the graph:");
+        for (String feedId : graph.getFeedIds()) {
+            Collection<org.opentripplanner.model.Agency> feedAgencies = graph.getAgencies(feedId);
+            if (feedAgencies.size() > 0) {
+                LOG.info(
+                    "Feed with ID `{}`{}",
+                    feedId,
+                    autoGeneratedFeedIds.contains(feedId)
+                        ? " (ID was auto-generated)"
+                        : ""
+                );
+                for (org.opentripplanner.model.Agency feedAgency : feedAgencies) {
+                    LOG.info(
+                        " - Agency `{}` with ID `{}`{}",
+                        feedAgency.getName(),
+                        feedAgency.getId(),
+                        autoGeneratedAgencyIds.contains(feedId + feedAgency.getId())
+                            ? " (ID was auto-generated)"
+                            : ""
+                    );
+                }
+            }
+        }
+
         LOG.info(postBundleTask.finish());
     }
 
@@ -181,10 +227,45 @@ private GtfsMutableRelationalDao loadBundle(GtfsBundle gtfsBundle)
 
         GtfsFeedId gtfsFeedId = gtfsBundle.getFeedId();
 
+        // check for duplicate bundle names and make sure that at most one of those bundles contains an auto-generated
+        // feed ID. If there is more than one, non-deterministic Feed IDs could be generated.
+        String bundleFilename = gtfsBundle.getPath().getName();
+        if (bundleFilenamesWithAutoGeneratedFeedId.contains(bundleFilename)) {
+            throw new RuntimeException(
+                String.format(
+                    "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
+                )
+            );
+        }
+        if (gtfsFeedId.isAutoGenerated()) {
+            bundleFilenamesWithAutoGeneratedFeedId.add(bundleFilename);
+            autoGeneratedFeedIds.add(gtfsFeedId.getId());
+        }
+
+        // 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 (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");
+        try {
+            CsvReader result = new CsvReader(agencyInputStream, StandardCharsets.UTF_8);
+            result.readHeaders();
+            result.readRecord();
+            String firstAgencyId = result.get("agency_id");
+            if (firstAgencyId == null || firstAgencyId.trim().length() == 0) {
+                agencyFileContainedOneAgencyWithoutId = true;
+            }
+        } finally {
+            agencyInputStream.close();
+        }
+
         GtfsReader reader = new GtfsReader();
         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())
@@ -204,6 +285,8 @@ private GtfsMutableRelationalDao loadBundle(GtfsBundle gtfsBundle)
                 for (Agency agency : reader.getAgencies()) {
                     String agencyId = agency.getId();
                     LOG.info("This Agency has the ID {}", agencyId);
+                    boolean agencyIdIsAutogenerated = false;
+                    // Make sure the combination of the FeedId and agencyId is unique
                     // Somehow, when the agency's id field is missing, OBA replaces it with the agency's name.
                     // TODO Figure out how and why this is happening.
                     if (agencyId == null || agencyIdsSeen.contains(gtfsFeedId.getId() + agencyId)) {
@@ -217,8 +300,19 @@ private GtfsMutableRelationalDao loadBundle(GtfsBundle gtfsBundle)
                         reader.addAgencyIdMapping(agencyId, generatedAgencyId); // NULL key should work
                         agency.setId(generatedAgencyId);
                         agencyId = generatedAgencyId;
+                        agencyIdIsAutogenerated = true;
+                    } else {
+                        // the agencyId was not null or was non-conflicting. This means that either the agencyId was
+                        // derived from a record in the agency.txt table, or it was assigned the default AgencyId which
+                        // is the feedID of this feed. Therefore, set whether the agencyID was auto-generated based on
+                        // whether the agency.txt had a single agency record without an agency_id.
+                        agencyIdIsAutogenerated = agencyFileContainedOneAgencyWithoutId;
+                    }
+                    String gtfsFeedIdAndAgencyId = gtfsFeedId.getId() + agencyId;
+                    agencyIdsSeen.add(gtfsFeedIdAndAgencyId);
+                    if (agencyIdIsAutogenerated) {
+                        autoGeneratedAgencyIds.add(gtfsFeedIdAndAgencyId);
                     }
-                    if (agencyId != null) agencyIdsSeen.add(gtfsFeedId.getId() + agencyId);
                 }
             }
         }

src/main/java/org/opentripplanner/index/model/RouteShort.java

@@ -3,6 +3,7 @@
 import java.util.Collection;
 import java.util.List;
 
+import org.opentripplanner.model.Agency;
 import org.opentripplanner.model.FeedScopedId;
 import org.opentripplanner.model.Route;
 import org.opentripplanner.gtfs.GtfsLibrary;
@@ -16,6 +17,7 @@ public class RouteShort {
     public String longName;
     public String mode;
     public String color;
+    public String agencyId;
     public String agencyName;
     public Integer sortOrder;
 
@@ -25,7 +27,9 @@ public RouteShort (Route route) {
         longName = route.getLongName();
         mode = GtfsLibrary.getTraverseMode(route).toString();
         color = route.getColor();
-        agencyName = route.getAgency().getName();
+        Agency agency = route.getAgency();
+        agencyId = agency.getId();
+        agencyName = agency.getName();
         sortOrder = route.getSortOrder();
     }
 

src/test/java/org/opentripplanner/GtfsTest.java

@@ -56,7 +56,7 @@ protected void setUp() {
         File gtfs = new File("src/test/resources/" + getFeedName());
         File gtfsRealTime = new File("src/test/resources/" + getFeedName() + ".pb");
         GtfsBundle gtfsBundle = new GtfsBundle(gtfs);
-        feedId = new GtfsFeedId.Builder().id("FEED").build();
+        feedId = new GtfsFeedId.Builder().id("").build();
         gtfsBundle.setFeedId(feedId);
         List<GtfsBundle> gtfsBundleList = Collections.singletonList(gtfsBundle);
         GtfsModule gtfsGraphBuilderImpl = new GtfsModule(gtfsBundleList);

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();
+    }
+}

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

@@ -91,7 +91,7 @@ private MockGtfs getSimpleGtfs() throws IOException {
 
     private static List<GtfsBundle> getGtfsAsBundleList (MockGtfs gtfs) {
         GtfsBundle bundle = new GtfsBundle();
-        bundle.setFeedId(new GtfsFeedId.Builder().id("FEED").build());
+        bundle.setFeedId(new GtfsFeedId.Builder().id("").build());
         bundle.setPath(gtfs.getPath());
         List<GtfsBundle> list = Lists.newArrayList();
         list.add(bundle);

src/test/java/org/opentripplanner/routing/edgetype/factory/PatternHopFactoryTest.java

@@ -26,7 +26,7 @@ public void testBikesAllowed() throws IOException {
         gtfs.putLines("frequencies.txt", "trip_id,start_time,end_time,headway_secs",
                 "t0,09:00:00,17:00:00,300");
 
-        GtfsFeedId feedId = new GtfsFeedId.Builder().id("FEED").build();
+        GtfsFeedId feedId = new GtfsFeedId.Builder().id("").build();
         PatternHopFactory factory = new PatternHopFactory(
                 GtfsLibrary.createContext(feedId, gtfs.read())
         );