Update transfers inline with latest spec

src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java

@@ -161,8 +161,8 @@ public FeedLoadResult loadTables() {
             result.shapes = load(Table.SHAPES);
             result.stops = load(Table.STOPS);
             result.fareRules = load(Table.FARE_RULES);
-            result.transfers = load(Table.TRANSFERS);
             result.trips = load(Table.TRIPS); // refs routes
+            result.transfers = load(Table.TRANSFERS); // refs trips.
             result.frequencies = load(Table.FREQUENCIES); // refs trips
             result.stopTimes = load(Table.STOP_TIMES);
             result.translations = load(Table.TRANSLATIONS);

src/main/java/com/conveyal/gtfs/loader/Table.java

@@ -322,17 +322,6 @@ public Table (String name, Class<? extends Entity> entityClass, Requirement requ
     ).withParentTable(PATTERNS)
     .addPrimaryKeyNames("pattern_id", "stop_sequence");
 
-    public static final Table TRANSFERS = new Table("transfers", Transfer.class, OPTIONAL,
-            // FIXME: Do we need an index on from_ and to_stop_id
-            new StringField("from_stop_id", REQUIRED).isReferenceTo(STOPS),
-            new StringField("to_stop_id", REQUIRED).isReferenceTo(STOPS),
-            new ShortField("transfer_type", REQUIRED, 3),
-            new StringField("min_transfer_time", OPTIONAL)
-    ).addPrimaryKey()
-    .keyFieldIsNotUnique()
-    .hasCompoundKey()
-    .addPrimaryKeyNames("from_stop_id", "to_stop_id");
-
     public static final Table TRIPS = new Table("trips", Trip.class, REQUIRED,
         new StringField("trip_id", REQUIRED),
         new StringField("route_id", REQUIRED).isReferenceTo(ROUTES).indexThisColumn(),
@@ -351,6 +340,25 @@ public Table (String name, Class<? extends Entity> entityClass, Requirement requ
     ).addPrimaryKey()
     .addPrimaryKeyNames("trip_id");
 
+    // Must come after TRIPS table to which it has references.
+    public static final Table TRANSFERS = new Table("transfers", Transfer.class, OPTIONAL,
+        // Conditionally required fields (from_stop_id, to_stop_id, from_trip_id and to_trip_id) are defined here as
+        // optional so as not to trigger required field checks as part of GTFS-lib validation. Correct validation of
+        // these fields will be managed by the MobilityData validator.
+        new StringField("from_stop_id", OPTIONAL).isReferenceTo(STOPS),
+        new StringField("to_stop_id", OPTIONAL).isReferenceTo(STOPS),
+        new StringField("from_route_id", OPTIONAL).isReferenceTo(ROUTES),
+        new StringField("to_route_id", OPTIONAL).isReferenceTo(ROUTES),
+        new StringField("from_trip_id", OPTIONAL).isReferenceTo(TRIPS),
+        new StringField("to_trip_id", OPTIONAL).isReferenceTo(TRIPS),
+        new ShortField("transfer_type", REQUIRED, 3),
+        new StringField("min_transfer_time", OPTIONAL)
+    )
+    .addPrimaryKey()
+    .keyFieldIsNotUnique()
+    .hasCompoundKey()
+    .addPrimaryKeyNames("from_stop_id", "to_stop_id", "from_trip_id", "to_trip_id", "from_route_id", "to_route_id");
+
     // Must come after TRIPS and STOPS table to which it has references
     public static final Table STOP_TIMES = new Table("stop_times", StopTime.class, REQUIRED,
             new StringField("trip_id", REQUIRED).isReferenceTo(TRIPS),

src/main/java/com/conveyal/gtfs/model/Transfer.java

@@ -29,15 +29,18 @@ public void setStatementParameters(PreparedStatement statement, boolean setDefau
         if (!setDefaultId) statement.setInt(oneBasedIndex++, id);
         statement.setString(oneBasedIndex++, from_stop_id);
         statement.setString(oneBasedIndex++, to_stop_id);
+        statement.setString(oneBasedIndex++, from_trip_id);
+        statement.setString(oneBasedIndex++, to_trip_id);
+        statement.setString(oneBasedIndex++, from_route_id);
+        statement.setString(oneBasedIndex++, to_route_id);
         setIntParameter(statement, oneBasedIndex++, transfer_type);
-        setIntParameter(statement, oneBasedIndex++, min_transfer_time);
+        setIntParameter(statement, oneBasedIndex, min_transfer_time);
     }
 
-    // TODO: Add id method for Transfer.
-//    @Override
-//    public String getId() {
-////        return trip_id;
-//    }
+    @Override
+    public String getId() {
+        return createId(from_stop_id, to_stop_id, from_trip_id, to_trip_id, from_route_id, to_route_id);
+    }
 
     public static class Loader extends Entity.Loader<Transfer> {
 
@@ -54,24 +57,24 @@ protected boolean isRequired() {
         public void loadOneRow() throws IOException {
             Transfer tr = new Transfer();
             tr.id = row + 1; // offset line number by 1 to account for 0-based row index
-            tr.from_stop_id      = getStringField("from_stop_id", true);
-            tr.to_stop_id        = getStringField("to_stop_id", true);
-            tr.transfer_type     = getIntField("transfer_type", true, 0, 3);
-            tr.min_transfer_time = getIntField("min_transfer_time", false, 0, Integer.MAX_VALUE);
+            tr.from_stop_id      = getStringField("from_stop_id", false);
+            tr.to_stop_id        = getStringField("to_stop_id", false);
             tr.from_route_id     = getStringField("from_route_id", false);
             tr.to_route_id       = getStringField("to_route_id", false);
             tr.from_trip_id      = getStringField("from_trip_id", false);
             tr.to_trip_id        = getStringField("to_trip_id", false);
+            tr.transfer_type     = getIntField("transfer_type", true, 0, 3);
+            tr.min_transfer_time = getIntField("min_transfer_time", false, 0, Integer.MAX_VALUE);
 
-            getRefField("from_stop_id", true, feed.stops);
-            getRefField("to_stop_id", true, feed.stops);
+            getRefField("from_stop_id", false, feed.stops);
+            getRefField("to_stop_id", false, feed.stops);
             getRefField("from_route_id", false, feed.routes);
             getRefField("to_route_id", false, feed.routes);
             getRefField("from_trip_id", false, feed.trips);
             getRefField("to_trip_id", false, feed.trips);
 
             tr.feed = feed;
-            feed.transfers.put(Long.toString(row), tr);
+            feed.transfers.put(tr.getId(), tr);
         }
 
     }
@@ -83,13 +86,26 @@ public Writer (GTFSFeed feed) {
 
         @Override
         protected void writeHeaders() throws IOException {
-            writer.writeRecord(new String[] {"from_stop_id", "to_stop_id", "transfer_type", "min_transfer_time"});
+            writer.writeRecord(new String[] {
+                "from_stop_id",
+                "to_stop_id",
+                "from_trip_id",
+                "to_trip_id",
+                "from_route_id",
+                "to_route_id",
+                "transfer_type",
+                "min_transfer_time"
+            });
         }
 
         @Override
         protected void writeOneRow(Transfer t) throws IOException {
             writeStringField(t.from_stop_id);
             writeStringField(t.to_stop_id);
+            writeStringField(t.from_trip_id);
+            writeStringField(t.to_trip_id);
+            writeStringField(t.from_route_id);
+            writeStringField(t.to_route_id);
             writeIntField(t.transfer_type);
             writeIntField(t.min_transfer_time);
             endRecord();
@@ -99,7 +115,19 @@ protected void writeOneRow(Transfer t) throws IOException {
         protected Iterator<Transfer> iterator() {
             return feed.transfers.values().iterator();
         }
+    }
 
-
+    /**
+     * Transfer entries have no ID in GTFS so we define one based on the fields in the transfer entry.
+     */
+    private static String createId(
+        String fromStopId,
+        String toStopId,
+        String fromTripId,
+        String toTripId,
+        String fromRouteId,
+        String toRouteId
+    ) {
+        return String.format("%s_%s_%s_%s_%s_%s", fromStopId, toStopId, fromTripId, toTripId, fromRouteId, toRouteId);
     }
-}
+}

src/test/java/com/conveyal/gtfs/GTFSTest.java

@@ -16,7 +16,6 @@
 import com.csvreader.CsvReader;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.Multimap;
-import com.google.common.io.Files;
 import graphql.Assert;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.input.BOMInputStream;
@@ -34,6 +33,7 @@
 import java.io.InputStream;
 import java.io.PrintStream;
 import java.nio.charset.Charset;
+import java.nio.file.Files;
 import java.sql.Connection;
 import java.sql.ResultSet;
 import java.sql.SQLException;
@@ -246,12 +246,12 @@ public void canLoadFeedWithErrors () {
      * Tests whether or not "fake-agency" GTFS can be placed in a zipped subdirectory and loaded/exported successfully.
      */
     @Test
-    public void canLoadAndExportSimpleAgencyInSubDirectory() {
+    public void canLoadAndExportSimpleAgencyInSubDirectory() throws IOException {
         String zipFileName = null;
         // Get filename for fake-agency resource
         String resourceFolder = TestUtils.getResourceFileName("fake-agency");
         // Recursively copy folder into temp directory, which we zip up and run the integration test on.
-        File tempDir = Files.createTempDir();
+        File tempDir = Files.createTempDirectory("").toFile();
         tempDir.deleteOnExit();
         File nestedDir = new File(TestUtils.fileNameWithDir(tempDir.getAbsolutePath(), "fake-agency"));
         LOG.info("Creating temp folder with nested subdirectory at {}", tempDir.getAbsolutePath());
@@ -1313,6 +1313,19 @@ private void assertThatPersistenceExpectationRecordWasFound(
                 new RecordExpectation("bikes_allowed", 0),
                 new RecordExpectation("wheelchair_accessible", 0)
             }
+        ),
+        new PersistenceExpectation(
+            "transfers",
+            new RecordExpectation[]{
+                new RecordExpectation("from_stop_id", "4u6g"),
+                new RecordExpectation("to_stop_id", "johv"),
+                new RecordExpectation("from_trip_id", "a30277f8-e50a-4a85-9141-b1e0da9d429d"),
+                new RecordExpectation("to_trip_id", "frequency-trip"),
+                new RecordExpectation("from_route_id", "1"),
+                new RecordExpectation("to_route_id", "1"),
+                new RecordExpectation("transfer_type", "1"),
+                new RecordExpectation("min_transfer_time", "60")
+            }
         )
     };
 

src/test/resources/fake-agency/transfers.txt

@@ -1 +1,3 @@
-from_stop_id,to_stop_id,transfer_type,min_transfer_time
+from_stop_id,to_stop_id,from_trip_id,to_trip_id,from_route_id,to_route_id,transfer_type,min_transfer_time
+4u6g,johv,a30277f8-e50a-4a85-9141-b1e0da9d429d,frequency-trip,1,1,1,60
+4u6g,123,,,,,1,60