Skip to content

Commit

Permalink
refactor(Updated transfers inline with latest spec):
Browse files Browse the repository at this point in the history
  • Loading branch information
br648 committed Oct 18, 2023
1 parent 80a968c commit 5f9a727
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 31 deletions.
2 changes: 1 addition & 1 deletion src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
30 changes: 19 additions & 11 deletions src/main/java/com/conveyal/gtfs/loader/Table.java
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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),
Expand Down
62 changes: 47 additions & 15 deletions src/main/java/com/conveyal/gtfs/model/Transfer.java
Original file line number Diff line number Diff line change
Expand Up @@ -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> {

Expand All @@ -54,24 +57,27 @@ 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(
createId(tr.from_stop_id, tr.to_stop_id, tr.from_trip_id, tr.to_trip_id, tr.from_route_id, tr.to_route_id),
tr
);
}

}
Expand All @@ -83,13 +89,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();
Expand All @@ -99,7 +118,20 @@ 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);
}

}
19 changes: 16 additions & 3 deletions src/test/java/com/conveyal/gtfs/GTFSTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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")
}
)
};

Expand Down
3 changes: 2 additions & 1 deletion src/test/resources/fake-agency/transfers.txt
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
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

0 comments on commit 5f9a727

Please sign in to comment.