From 7fcd08e10c3a0cb39adde997ed9d63545a234e42 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Thu, 24 Aug 2023 08:17:35 +0100 Subject: [PATCH 01/11] refactor(Persisting patterns): initial commit --- src/main/java/com/conveyal/gtfs/GTFSFeed.java | 22 +- .../com/conveyal/gtfs/PatternBuilder.java | 242 ++++++++++++++++++ .../java/com/conveyal/gtfs/PatternFinder.java | 24 +- .../conveyal/gtfs/loader/EntityPopulator.java | 8 + .../java/com/conveyal/gtfs/loader/Feed.java | 2 + .../conveyal/gtfs/loader/FeedLoadResult.java | 1 + .../gtfs/loader/JdbcGtfsExporter.java | 2 + .../conveyal/gtfs/loader/JdbcGtfsLoader.java | 1 + .../java/com/conveyal/gtfs/model/Pattern.java | 30 +++ .../validator/PatternFinderValidator.java | 178 ++----------- 10 files changed, 303 insertions(+), 207 deletions(-) create mode 100644 src/main/java/com/conveyal/gtfs/PatternBuilder.java diff --git a/src/main/java/com/conveyal/gtfs/GTFSFeed.java b/src/main/java/com/conveyal/gtfs/GTFSFeed.java index 8e6de45e2..2051505e3 100644 --- a/src/main/java/com/conveyal/gtfs/GTFSFeed.java +++ b/src/main/java/com/conveyal/gtfs/GTFSFeed.java @@ -216,6 +216,7 @@ public void toFile (String file) { new Transfer.Writer(this).writeTable(zip); new Trip.Writer(this).writeTable(zip); new StopTime.Writer(this).writeTable(zip); + new Pattern.Writer(this).writeTable(zip); zip.close(); @@ -343,27 +344,6 @@ public Shape getShape (String shape_id) { return shape.shape_dist_traveled.length > 0 ? shape : null; } - /** - * MapDB-based implementation to find patterns. - * - * FIXME: Remove and make pattern finding happen during validation? We want to share the pattern finder between the - * two implementations (MapDB and RDBMS), apply the same validation process to both kinds of storage, and produce - * Patterns in the same way in both cases, during validation. This prevents us from iterating over every stopTime - * twice, since we're already iterating over all of them in validation. However, in this case it might not be costly - * to simply retrieve the stop times from the stop_times map. - */ - public void findPatterns () { - PatternFinder patternFinder = new PatternFinder(); - // Iterate over trips and process each trip and its stop times. - for (Trip trip : this.trips.values()) { - Iterable orderedStopTimesForTrip = this.getOrderedStopTimesForTrip(trip.trip_id); - patternFinder.processTrip(trip, orderedStopTimesForTrip); - } - Map patternObjects = patternFinder.createPatternObjects(this.stops, null); - this.patterns.putAll(patternObjects.values().stream() - .collect(Collectors.toMap(Pattern::getId, pattern -> pattern))); - } - /** * For the given trip ID, fetch all the stop times in order, and interpolate stop-to-stop travel times. */ diff --git a/src/main/java/com/conveyal/gtfs/PatternBuilder.java b/src/main/java/com/conveyal/gtfs/PatternBuilder.java new file mode 100644 index 000000000..f45681f20 --- /dev/null +++ b/src/main/java/com/conveyal/gtfs/PatternBuilder.java @@ -0,0 +1,242 @@ +package com.conveyal.gtfs; + +import com.conveyal.gtfs.loader.BatchTracker; +import com.conveyal.gtfs.loader.Feed; +import com.conveyal.gtfs.loader.Requirement; +import com.conveyal.gtfs.loader.Table; +import com.conveyal.gtfs.model.Pattern; +import com.conveyal.gtfs.model.PatternStop; +import org.apache.commons.dbutils.DbUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.BufferedOutputStream; +import java.io.File; +import java.io.IOException; +import java.io.PrintStream; +import java.nio.file.Files; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.Map; +import java.util.Set; + +import static com.conveyal.gtfs.loader.JdbcGtfsLoader.copyFromFile; +import static com.conveyal.gtfs.model.Entity.INT_MISSING; +import static com.conveyal.gtfs.model.Entity.setDoubleParameter; +import static com.conveyal.gtfs.model.Entity.setIntParameter; + +public class PatternBuilder { + + private static final Logger LOG = LoggerFactory.getLogger(PatternBuilder.class); + + private final Feed feed; + private static final String TEMP_FILE_NAME = "pattern_for_trips"; + + private final Connection connection; + public PatternBuilder(Feed feed) throws SQLException { + this.feed = feed; + connection = feed.getConnection(); + } + + private String getTableName(String tableName) { + return feed.tablePrefix + tableName; + } + + public void create(Map patterns, Set patternIdsLoadedFromFile) { + String patternsTableName = getTableName("patterns"); + String tripsTableName = getTableName("trips"); + String patternStopsTableName = getTableName("pattern_stops"); + + Table patternsTable = new Table(patternsTableName, Pattern.class, Requirement.EDITOR, Table.PATTERNS.fields); + Table patternStopsTable = new Table(patternStopsTableName, PatternStop.class, Requirement.EDITOR, Table.PATTERN_STOP.fields); + + try { + File tempPatternForTripsTextFile = File.createTempFile(TEMP_FILE_NAME, "text"); + LOG.info("Creating pattern and pattern stops tables."); + Statement statement = connection.createStatement(); + statement.execute(String.format("alter table %s add column pattern_id varchar", tripsTableName)); + if (patternIdsLoadedFromFile.isEmpty()) { + // No patterns were loaded from file so the pattern table has not previously been created. + patternsTable.createSqlTable(connection, null, true); + } + patternStopsTable.createSqlTable(connection, null, true); + PrintStream patternForTripsFileStream = createTempPatternForTripsTable(tempPatternForTripsTextFile, statement); + processPatternAndPatternStops(patternsTable, patternStopsTable, patternForTripsFileStream, patterns, patternIdsLoadedFromFile); + updateTripPatternIds(tempPatternForTripsTextFile, patternForTripsFileStream, statement, tripsTableName); + createIndexes(statement, patternsTableName, patternStopsTableName, tripsTableName); + connection.commit(); + } catch (SQLException | IOException e) { + // Rollback transaction if failure occurs on creating patterns. + DbUtils.rollbackAndCloseQuietly(connection); + // This exception will be stored as a validator failure. + throw new RuntimeException(e); + } finally { + // Close transaction finally. + if (connection != null) DbUtils.closeQuietly(connection); + } + } + + private void processPatternAndPatternStops( + Table patternsTable, + Table patternStopsTable, + PrintStream patternForTripsFileStream, + Map patterns, + Set patternIdsLoadedFromFile + ) throws SQLException { + // Generate prepared statements for inserts. + String insertPatternSql = patternsTable.generateInsertSql(true); + PreparedStatement insertPatternStatement = connection.prepareStatement(insertPatternSql); + BatchTracker patternTracker = new BatchTracker("pattern", insertPatternStatement); + LOG.info("Storing patterns and pattern stops."); + for (Map.Entry entry : patterns.entrySet()) { + Pattern pattern = entry.getValue(); + LOG.debug("Batching pattern {}", pattern.pattern_id); + if (!patternIdsLoadedFromFile.contains(pattern.pattern_id)) { + // Only insert the pattern if it has not already been imported from file. + pattern.setStatementParameters(insertPatternStatement, true); + patternTracker.addBatch(); + } + createPatternStops(entry.getKey(), pattern.pattern_id, patternStopsTable); + updateTripPatternReferences(patternForTripsFileStream, pattern); + } + // Send any remaining prepared statement calls to the database backend. + patternTracker.executeRemaining(); + LOG.info("Done storing patterns and pattern stops."); + } + + /** + * Create temp table for updating trips with pattern IDs to be dropped at the end of the transaction. + * NOTE: temp table name must NOT be prefixed with schema because temp tables are prefixed with their own + * connection-unique schema. + */ + private PrintStream createTempPatternForTripsTable( + File tempPatternForTripsTextFile, + Statement statement + ) throws SQLException, IOException { + LOG.info("Loading via temporary text file at {}.", tempPatternForTripsTextFile.getAbsolutePath()); + String createTempSql = String.format("create temp table %s(trip_id varchar, pattern_id varchar) on commit drop", TEMP_FILE_NAME); + LOG.info(createTempSql); + statement.execute(createTempSql); + return new PrintStream(new BufferedOutputStream(Files.newOutputStream(tempPatternForTripsTextFile.toPath()))); + } + + /** + * Update all trips on this pattern to reference this pattern's ID. + */ + private void updateTripPatternReferences(PrintStream patternForTripsFileStream, Pattern pattern) { + // Prepare each trip in pattern to update trips table. + for (String tripId : pattern.associatedTrips) { + // Add line to temp csv file if using postgres. + // No need to worry about null trip IDs because the trips have already been processed. + String[] strings = new String[]{tripId, pattern.pattern_id}; + // Print a new line in the standard postgres text format: + // https://www.postgresql.org/docs/9.1/static/sql-copy.html#AEN64380 + patternForTripsFileStream.println(String.join("\t", strings)); + } + } + + /** + * Copy the pattern for trips text file into a table, create an index on trip IDs, and update the trips + * table. + */ + private void updateTripPatternIds( + File tempPatternForTripsTextFile, + PrintStream patternForTripsFileStream, + Statement statement, + String tripsTableName + ) throws SQLException, IOException { + + LOG.info("Updating trips with pattern IDs."); + patternForTripsFileStream.close(); + // Copy file contents into temp pattern for trips table. + copyFromFile(connection, tempPatternForTripsTextFile, TEMP_FILE_NAME); + // Before updating the trips with pattern IDs, index the table on trip_id. + String patternForTripsIndexSql = String.format( + "create index temp_trips_pattern_id_idx on %s (trip_id)", + TEMP_FILE_NAME + ); + LOG.info(patternForTripsIndexSql); + statement.execute(patternForTripsIndexSql); + // Finally, execute the update statement. + String updateTripsSql = String.format( + "update %s set pattern_id = %s.pattern_id from %s where %s.trip_id = %s.trip_id", + tripsTableName, + TEMP_FILE_NAME, + TEMP_FILE_NAME, + tripsTableName, + TEMP_FILE_NAME + ); + LOG.info(updateTripsSql); + statement.executeUpdate(updateTripsSql); + // Delete temp file. Temp table will be dropped after the transaction is committed. + Files.delete(tempPatternForTripsTextFile.toPath()); + LOG.info("Updating trips complete."); + } + + private void createIndexes( + Statement statement, + String patternsTableName, + String patternStopsTableName, + String tripsTableName + ) throws SQLException { + LOG.info("Creating index on patterns."); + statement.executeUpdate(String.format("alter table %s add primary key (pattern_id)", patternsTableName)); + LOG.info("Creating index on pattern stops."); + statement.executeUpdate(String.format("alter table %s add primary key (pattern_id, stop_sequence)", patternStopsTableName)); + // Index new pattern_id column on trips. The other tables are already indexed because they have primary keys. + LOG.info("Indexing trips on pattern id."); + statement.execute(String.format("create index trips_pattern_id_idx on %s (pattern_id)", tripsTableName)); + LOG.info("Done indexing."); + } + + /** + * Construct pattern stops based on values in trip pattern key. + */ + private void createPatternStops(TripPatternKey key, String patternId, Table patternStopsTable) throws SQLException { + + String insertPatternStopSql = patternStopsTable.generateInsertSql(true); + PreparedStatement insertPatternStopStatement = connection.prepareStatement(insertPatternStopSql); + BatchTracker patternStopTracker = new BatchTracker("pattern stop", insertPatternStopStatement); + + int lastValidDeparture = key.departureTimes.get(0); + for (int i = 0; i < key.stops.size(); i++) { + int travelTime = 0; + String stopId = key.stops.get(i); + int arrival = key.arrivalTimes.get(i); + if (i > 0) { + int prevDeparture = key.departureTimes.get(i - 1); + // Set travel time for all stops except the first. + if (prevDeparture != INT_MISSING) { + // Update the previous departure if it's not missing. Otherwise, base travel time based on the + // most recent valid departure. + lastValidDeparture = prevDeparture; + } + travelTime = arrival == INT_MISSING || lastValidDeparture == INT_MISSING + ? INT_MISSING + : arrival - lastValidDeparture; + } + int departure = key.departureTimes.get(i); + int dwellTime = arrival == INT_MISSING || departure == INT_MISSING + ? INT_MISSING + : departure - arrival; + + insertPatternStopStatement.setString(1, patternId); + // Stop sequence is zero-based. + setIntParameter(insertPatternStopStatement, 2, i); + insertPatternStopStatement.setString(3, stopId); + insertPatternStopStatement.setString(4, key.stopHeadsigns.get(i)); + setIntParameter(insertPatternStopStatement,5, travelTime); + setIntParameter(insertPatternStopStatement,6, dwellTime); + setIntParameter(insertPatternStopStatement,7, key.dropoffTypes.get(i)); + setIntParameter(insertPatternStopStatement,8, key.pickupTypes.get(i)); + setDoubleParameter(insertPatternStopStatement, 9, key.shapeDistances.get(i)); + setIntParameter(insertPatternStopStatement,10, key.timepoints.get(i)); + setIntParameter(insertPatternStopStatement,11, key.continuous_pickup.get(i)); + setIntParameter(insertPatternStopStatement,12, key.continuous_drop_off.get(i)); + patternStopTracker.addBatch(); + } + patternStopTracker.executeRemaining(); + } +} diff --git a/src/main/java/com/conveyal/gtfs/PatternFinder.java b/src/main/java/com/conveyal/gtfs/PatternFinder.java index ab32c7d52..d251b6820 100644 --- a/src/main/java/com/conveyal/gtfs/PatternFinder.java +++ b/src/main/java/com/conveyal/gtfs/PatternFinder.java @@ -4,18 +4,12 @@ import com.conveyal.gtfs.error.NewGTFSErrorType; import com.conveyal.gtfs.error.SQLErrorStorage; import com.conveyal.gtfs.model.Pattern; -import com.conveyal.gtfs.model.PatternStop; -import com.conveyal.gtfs.model.ShapePoint; import com.conveyal.gtfs.model.Stop; import com.conveyal.gtfs.model.StopTime; import com.conveyal.gtfs.model.Trip; -import com.conveyal.gtfs.validator.service.GeoUtils; import com.google.common.collect.HashMultimap; import com.google.common.collect.LinkedHashMultimap; import com.google.common.collect.Multimap; -import org.locationtech.jts.geom.Coordinate; -import org.locationtech.jts.geom.CoordinateList; -import org.locationtech.jts.geom.LineString; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -29,7 +23,6 @@ import java.util.Locale; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; import static com.conveyal.gtfs.util.Util.human; @@ -50,21 +43,6 @@ public class PatternFinder { private int nTripsProcessed = 0; - /** - * Bin all trips by the sequence of stops they visit. - * @return A map from a list of stop IDs to a list of Trip IDs that visit those stops in that sequence. - */ -// public void findPatterns(Feed feed) { -// -// for (Trip trip : trips) { -// } -// feed.patterns.stream().forEach(p -> { -// feed.patterns.put(p.pattern_id, p); -// p.associatedTrips.stream().forEach(t -> feed.tripPatternMap.put(t, p.pattern_id)); -// }); -// -// } - public void processTrip(Trip trip, Iterable orderedStopTimes) { if (++nTripsProcessed % 100000 == 0) { LOG.info("trip {}", human(nTripsProcessed)); @@ -121,7 +99,7 @@ public Map createPatternObjects(Map stopB * This process requires access to all the stops in the feed. * Some validators already cache a map of all the stops. There's probably a cleaner way to do this. */ - public static void renamePatterns(Collection patterns, Map stopById) { + private static void renamePatterns(Collection patterns, Map stopById) { LOG.info("Generating unique names for patterns"); Map namingInfoForRoute = new HashMap<>(); diff --git a/src/main/java/com/conveyal/gtfs/loader/EntityPopulator.java b/src/main/java/com/conveyal/gtfs/loader/EntityPopulator.java index 36d40a427..aeeccc3d0 100644 --- a/src/main/java/com/conveyal/gtfs/loader/EntityPopulator.java +++ b/src/main/java/com/conveyal/gtfs/loader/EntityPopulator.java @@ -6,6 +6,7 @@ import com.conveyal.gtfs.model.Entity; import com.conveyal.gtfs.model.FareAttribute; import com.conveyal.gtfs.model.Frequency; +import com.conveyal.gtfs.model.Pattern; import com.conveyal.gtfs.model.PatternStop; import com.conveyal.gtfs.model.Route; import com.conveyal.gtfs.model.ScheduleException; @@ -70,6 +71,13 @@ public interface EntityPopulator { return patternStop; }; + EntityPopulator PATTERN = (result, columnForName) -> { + Pattern pattern = new Pattern(); + pattern.pattern_id = getStringIfPresent(result, "pattern_id", columnForName); + pattern.name = getStringIfPresent(result, "name", columnForName); + return pattern; + }; + T populate (ResultSet results, TObjectIntMap columnForName) throws SQLException; EntityPopulator AGENCY = (result, columnForName) -> { diff --git a/src/main/java/com/conveyal/gtfs/loader/Feed.java b/src/main/java/com/conveyal/gtfs/loader/Feed.java index 075600101..5b26a41a3 100644 --- a/src/main/java/com/conveyal/gtfs/loader/Feed.java +++ b/src/main/java/com/conveyal/gtfs/loader/Feed.java @@ -39,6 +39,7 @@ public class Feed { public final TableReader stops; public final TableReader trips; public final TableReader stopTimes; + public final TableReader patterns; /** * Create a feed that reads tables over a JDBC connection. The connection should already be set to the right @@ -59,6 +60,7 @@ public Feed (DataSource dataSource, String tablePrefix) { stops = new JDBCTableReader(Table.STOPS, dataSource, tablePrefix, EntityPopulator.STOP); trips = new JDBCTableReader(Table.TRIPS, dataSource, tablePrefix, EntityPopulator.TRIP); stopTimes = new JDBCTableReader(Table.STOP_TIMES, dataSource, tablePrefix, EntityPopulator.STOP_TIME); + patterns = new JDBCTableReader(Table.PATTERNS, dataSource, tablePrefix, EntityPopulator.PATTERN); } /** diff --git a/src/main/java/com/conveyal/gtfs/loader/FeedLoadResult.java b/src/main/java/com/conveyal/gtfs/loader/FeedLoadResult.java index 3a61fdd50..301ed0d6e 100644 --- a/src/main/java/com/conveyal/gtfs/loader/FeedLoadResult.java +++ b/src/main/java/com/conveyal/gtfs/loader/FeedLoadResult.java @@ -28,6 +28,7 @@ public class FeedLoadResult implements Serializable { public TableLoadResult fareRules; public TableLoadResult feedInfo; public TableLoadResult frequencies; + public TableLoadResult patterns; public TableLoadResult routes; public TableLoadResult shapes; public TableLoadResult stops; diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java index cd3b25aef..460e393b3 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java @@ -324,6 +324,8 @@ public FeedLoadResult exportTables() { result.trips = export(Table.TRIPS, connection); } + result.patterns = export(Table.PATTERNS, connection); + zipOutputStream.close(); // Run clean up on the resulting zip file. cleanUpZipFile(); diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java index b17a812c0..4d509b1f8 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java @@ -158,6 +158,7 @@ public FeedLoadResult loadTables() { result.routes = load(Table.ROUTES); result.fareAttributes = load(Table.FARE_ATTRIBUTES); result.feedInfo = load(Table.FEED_INFO); + result.patterns = load(Table.PATTERNS); result.shapes = load(Table.SHAPES); result.stops = load(Table.STOPS); result.fareRules = load(Table.FARE_RULES); diff --git a/src/main/java/com/conveyal/gtfs/model/Pattern.java b/src/main/java/com/conveyal/gtfs/model/Pattern.java index 5d8d1c5cf..800872035 100644 --- a/src/main/java/com/conveyal/gtfs/model/Pattern.java +++ b/src/main/java/com/conveyal/gtfs/model/Pattern.java @@ -1,11 +1,14 @@ package com.conveyal.gtfs.model; +import com.conveyal.gtfs.GTFSFeed; import com.google.common.base.Joiner; import org.locationtech.jts.geom.LineString; +import java.io.IOException; import java.sql.PreparedStatement; import java.sql.SQLException; import java.util.Collection; +import java.util.Iterator; import java.util.List; import java.util.Set; import java.util.UUID; @@ -102,6 +105,33 @@ else if (exemplarTrip.direction_id >= 0){ } + public Pattern () {} + + public static class Writer extends Entity.Writer { + public Writer (GTFSFeed feed) { + super(feed, "patterns"); + } + + @Override + protected void writeHeaders() throws IOException { + writer.writeRecord(new String[] {"pattern_id", "name"}); + } + + @Override + protected void writeOneRow(Pattern pattern) throws IOException { + writeStringField(pattern.pattern_id); + writeStringField(pattern.name); + endRecord(); + } + + @Override + protected Iterator iterator() { + return feed.patterns.values().iterator(); + } + + + } + /** * Sets the parameters for a prepared statement following the parameter order defined in * {@link com.conveyal.gtfs.loader.Table#PATTERNS}. JDBC prepared statement parameters use a one-based index. diff --git a/src/main/java/com/conveyal/gtfs/validator/PatternFinderValidator.java b/src/main/java/com/conveyal/gtfs/validator/PatternFinderValidator.java index 96c7956f2..88fe62b9f 100644 --- a/src/main/java/com/conveyal/gtfs/validator/PatternFinderValidator.java +++ b/src/main/java/com/conveyal/gtfs/validator/PatternFinderValidator.java @@ -1,5 +1,6 @@ package com.conveyal.gtfs.validator; +import com.conveyal.gtfs.PatternBuilder; import com.conveyal.gtfs.PatternFinder; import com.conveyal.gtfs.TripPatternKey; import com.conveyal.gtfs.error.SQLErrorStorage; @@ -28,8 +29,10 @@ import java.sql.Statement; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import static com.conveyal.gtfs.loader.JdbcGtfsLoader.copyFromFile; import static com.conveyal.gtfs.model.Entity.INT_MISSING; @@ -46,13 +49,16 @@ public class PatternFinderValidator extends TripValidator { private static final Logger LOG = LoggerFactory.getLogger(PatternFinderValidator.class); PatternFinder patternFinder; - private File tempPatternForTripsTextFile; - private PrintStream patternForTripsFileStream; - private String tempPatternForTripsTable; + PatternBuilder patternBuilder; public PatternFinderValidator(Feed feed, SQLErrorStorage errorStorage) { super(feed, errorStorage); patternFinder = new PatternFinder(); + try { + patternBuilder = new PatternBuilder(feed); + } catch (SQLException e) { + throw new RuntimeException("Unable to construct pattern builder.", e); + } } @Override @@ -66,172 +72,18 @@ public void validateTrip (Trip trip, Route route, List stopTimes, List */ @Override public void complete(ValidationResult validationResult) { + Set patternIds = new HashSet<>(); + for(Pattern pattern : feed.patterns) { + patternIds.add(pattern.pattern_id); + } LOG.info("Finding patterns..."); - // FIXME: There may be a better way to handle getting the full list of stops Map stopById = new HashMap<>(); for (Stop stop : feed.stops) { stopById.put(stop.stop_id, stop); } - // FIXME In the editor we need patterns to exist separately from and before trips themselves, so me make another table. + // Although patterns may have already been loaded from file, the trip patterns are still required. Map patterns = patternFinder.createPatternObjects(stopById, errorStorage); - Connection connection = null; - try { - // TODO this assumes gtfs-lib is using an SQL database and not a MapDB. - // Maybe we should just create patterns in a separate step, but that would mean iterating over the - // stop_times twice. - LOG.info("Creating pattern and pattern stops tables."); - connection = feed.getConnection(); - Statement statement = connection.createStatement(); - String tripsTableName = feed.tablePrefix + "trips"; - String patternsTableName = feed.tablePrefix + "patterns"; - String patternStopsTableName = feed.tablePrefix + "pattern_stops"; - statement.execute(String.format("alter table %s add column pattern_id varchar", tripsTableName)); - // FIXME: Here we're creating a pattern table that has an integer ID field (similar to the other GTFS tables) - // AND a varchar pattern_id with essentially the same value cast to a string. Perhaps the pattern ID should - // be a UUID or something, just to better distinguish it from the int ID? - Table patternsTable = new Table(patternsTableName, Pattern.class, Requirement.EDITOR, Table.PATTERNS.fields); - Table patternStopsTable = new Table(patternStopsTableName, PatternStop.class, Requirement.EDITOR, - Table.PATTERN_STOP.fields); - // Create pattern and pattern stops table, each with serial ID fields. - patternsTable.createSqlTable(connection, null, true); - patternStopsTable.createSqlTable(connection, null, true); - // Generate prepared statements for inserts. - String insertPatternSql = patternsTable.generateInsertSql(true); - String insertPatternStopSql = patternStopsTable.generateInsertSql(true); - PreparedStatement insertPatternStatement = connection.prepareStatement(insertPatternSql); - BatchTracker patternTracker = new BatchTracker("pattern", insertPatternStatement); - PreparedStatement insertPatternStopStatement = connection.prepareStatement(insertPatternStopSql); - BatchTracker patternStopTracker = new BatchTracker("pattern stop", insertPatternStopStatement); - int currentPatternIndex = 0; - LOG.info("Storing patterns and pattern stops"); - // If using Postgres, load pattern to trips mapping into temp table for quick updating. - boolean postgresText = (connection.getMetaData().getDatabaseProductName().equals("PostgreSQL")); - if (postgresText) { - // NOTE: temp table name must NOT be prefixed with schema because temp tables are prefixed with their own - // connection-unique schema. - tempPatternForTripsTable = "pattern_for_trips"; - tempPatternForTripsTextFile = File.createTempFile(tempPatternForTripsTable, "text"); - LOG.info("Loading via temporary text file at {}", tempPatternForTripsTextFile.getAbsolutePath()); - // Create temp table for updating trips with pattern IDs to be dropped at the end of the transaction. - String createTempSql = String.format("create temp table %s(trip_id varchar, pattern_id varchar) on commit drop", tempPatternForTripsTable); - LOG.info(createTempSql); - statement.execute(createTempSql); - patternForTripsFileStream = new PrintStream(new BufferedOutputStream(new FileOutputStream(tempPatternForTripsTextFile))); - } - for (Map.Entry entry : patterns.entrySet()) { - Pattern pattern = entry.getValue(); - LOG.debug("Batching pattern {}", pattern.pattern_id); - TripPatternKey key = entry.getKey(); - pattern.setStatementParameters(insertPatternStatement, true); - patternTracker.addBatch(); - // Construct pattern stops based on values in trip pattern key. - // FIXME: Use pattern stops table here? - int lastValidDeparture = key.departureTimes.get(0); - for (int i = 0; i < key.stops.size(); i++) { - int travelTime = 0; - String stopId = key.stops.get(i); - int arrival = key.arrivalTimes.get(i); - if (i > 0) { - int prevDeparture = key.departureTimes.get(i - 1); - // Set travel time for all stops except the first. - if (prevDeparture != INT_MISSING) { - // Update the previous departure if it's not missing. Otherwise, base travel time based on the - // most recent valid departure. - lastValidDeparture = prevDeparture; - } - travelTime = arrival == INT_MISSING || lastValidDeparture == INT_MISSING - ? INT_MISSING - : arrival - lastValidDeparture; - } - int departure = key.departureTimes.get(i); - int dwellTime = arrival == INT_MISSING || departure == INT_MISSING - ? INT_MISSING - : departure - arrival; - - insertPatternStopStatement.setString(1, pattern.pattern_id); - // Stop sequence is zero-based. - setIntParameter(insertPatternStopStatement, 2, i); - insertPatternStopStatement.setString(3, stopId); - insertPatternStopStatement.setString(4, key.stopHeadsigns.get(i)); - setIntParameter(insertPatternStopStatement,5, travelTime); - setIntParameter(insertPatternStopStatement,6, dwellTime); - setIntParameter(insertPatternStopStatement,7, key.dropoffTypes.get(i)); - setIntParameter(insertPatternStopStatement,8, key.pickupTypes.get(i)); - setDoubleParameter(insertPatternStopStatement, 9, key.shapeDistances.get(i)); - setIntParameter(insertPatternStopStatement,10, key.timepoints.get(i)); - setIntParameter(insertPatternStopStatement,11, key.continuous_pickup.get(i)); - setIntParameter(insertPatternStopStatement,12, key.continuous_drop_off.get(i)); - patternStopTracker.addBatch(); - } - // Finally, update all trips on this pattern to reference this pattern's ID. - String questionMarks = String.join(", ", Collections.nCopies(pattern.associatedTrips.size(), "?")); - PreparedStatement updateTripStatement = connection.prepareStatement( - String.format("update %s set pattern_id = ? where trip_id in (%s)", tripsTableName, questionMarks)); - int oneBasedIndex = 1; - updateTripStatement.setString(oneBasedIndex++, pattern.pattern_id); - // Prepare each trip in pattern to update trips table. - for (String tripId : pattern.associatedTrips) { - if (postgresText) { - // Add line to temp csv file if using postgres. - // No need to worry about null trip IDs because the trips have already been processed. - String[] strings = new String[]{tripId, pattern.pattern_id}; - // Print a new line in the standard postgres text format: - // https://www.postgresql.org/docs/9.1/static/sql-copy.html#AEN64380 - patternForTripsFileStream.println(String.join("\t", strings)); - } else { - // Otherwise, set statement parameter. - updateTripStatement.setString(oneBasedIndex++, tripId); - } - } - if (!postgresText) { - // Execute trip update statement if not using temp text file. - LOG.info("Updating {} trips with pattern ID {} (%d/%d)", pattern.associatedTrips.size(), pattern.pattern_id, currentPatternIndex, patterns.size()); - updateTripStatement.executeUpdate(); - } - currentPatternIndex += 1; - } - // Send any remaining prepared statement calls to the database backend. - patternTracker.executeRemaining(); - patternStopTracker.executeRemaining(); - LOG.info("Done storing patterns and pattern stops."); - if (postgresText) { - // Finally, copy the pattern for trips text file into a table, create an index on trip IDs, and update - // the trips table. - LOG.info("Updating trips with pattern IDs"); - patternForTripsFileStream.close(); - // Copy file contents into temp pattern for trips table. - copyFromFile(connection, tempPatternForTripsTextFile, tempPatternForTripsTable); - // Before updating the trips with pattern IDs, index the table on trip_id. - String patternForTripsIndexSql = String.format("create index temp_trips_pattern_id_idx on %s (trip_id)", tempPatternForTripsTable); - LOG.info(patternForTripsIndexSql); - statement.execute(patternForTripsIndexSql); - // Finally, execute the update statement. - String updateTripsSql = String.format("update %s set pattern_id = %s.pattern_id from %s where %s.trip_id = %s.trip_id", tripsTableName, tempPatternForTripsTable, tempPatternForTripsTable, tripsTableName, tempPatternForTripsTable); - LOG.info(updateTripsSql); - statement.executeUpdate(updateTripsSql); - // Delete temp file. Temp table will be dropped after the transaction is committed. - tempPatternForTripsTextFile.delete(); - LOG.info("Updating trips complete"); - } - LOG.info("Creating index on patterns"); - statement.executeUpdate(String.format("alter table %s add primary key (pattern_id)", patternsTableName)); - LOG.info("Creating index on pattern stops"); - statement.executeUpdate(String.format("alter table %s add primary key (pattern_id, stop_sequence)", patternStopsTableName)); - // Index new pattern_id column on trips. The other tables are already indexed because they have primary keys. - LOG.info("Indexing trips on pattern id."); - statement.execute(String.format("create index trips_pattern_id_idx on %s (pattern_id)", tripsTableName)); - LOG.info("Done indexing."); - connection.commit(); - } catch (SQLException | IOException e) { - // Rollback transaction if failure occurs on creating patterns. - DbUtils.rollbackAndCloseQuietly(connection); - // This exception will be stored as a validator failure. - throw new RuntimeException(e); - } finally { - // Close transaction finally. - if (connection != null) DbUtils.closeQuietly(connection); - } - + patternBuilder.create(patterns, patternIds); } } From e264342da6470c4ac1be1c0c45b64d40be8aed92 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Thu, 31 Aug 2023 12:05:55 +0100 Subject: [PATCH 02/11] refactor(Further updates to allow exporting of patterns): Added proprietary table/file update --- src/main/java/com/conveyal/gtfs/GTFSFeed.java | 1 + .../conveyal/gtfs/loader/EntityPopulator.java | 4 ++ .../java/com/conveyal/gtfs/loader/Feed.java | 2 +- .../gtfs/loader/JdbcGtfsExporter.java | 10 +--- .../conveyal/gtfs/loader/JdbcGtfsLoader.java | 2 +- .../java/com/conveyal/gtfs/loader/Table.java | 30 +++++++++- .../java/com/conveyal/gtfs/model/Pattern.java | 57 +++++++++++++++---- .../java/com/conveyal/gtfs/GTFSFeedTest.java | 10 ++++ .../fake-agency/datatools_patterns.txt | 2 + 9 files changed, 95 insertions(+), 23 deletions(-) create mode 100644 src/test/resources/fake-agency/datatools_patterns.txt diff --git a/src/main/java/com/conveyal/gtfs/GTFSFeed.java b/src/main/java/com/conveyal/gtfs/GTFSFeed.java index 2051505e3..fe182a8e8 100644 --- a/src/main/java/com/conveyal/gtfs/GTFSFeed.java +++ b/src/main/java/com/conveyal/gtfs/GTFSFeed.java @@ -169,6 +169,7 @@ else if (feedId == null || feedId.isEmpty()) { this.fares.putAll(fares); fares = null; // free memory + new Pattern.Loader(this).loadTable(zip); new Route.Loader(this).loadTable(zip); new ShapePoint.Loader(this).loadTable(zip); new Stop.Loader(this).loadTable(zip); diff --git a/src/main/java/com/conveyal/gtfs/loader/EntityPopulator.java b/src/main/java/com/conveyal/gtfs/loader/EntityPopulator.java index aeeccc3d0..c5c59d900 100644 --- a/src/main/java/com/conveyal/gtfs/loader/EntityPopulator.java +++ b/src/main/java/com/conveyal/gtfs/loader/EntityPopulator.java @@ -74,7 +74,11 @@ public interface EntityPopulator { EntityPopulator PATTERN = (result, columnForName) -> { Pattern pattern = new Pattern(); pattern.pattern_id = getStringIfPresent(result, "pattern_id", columnForName); + pattern.route_id = getStringIfPresent(result, "route_id", columnForName); pattern.name = getStringIfPresent(result, "name", columnForName); + pattern.direction_id = getIntIfPresent(result, "direction_id", columnForName); + pattern.use_frequency = getIntIfPresent(result, "use_frequency", columnForName); + pattern.shape_id = getStringIfPresent(result, "shape_id", columnForName); return pattern; }; diff --git a/src/main/java/com/conveyal/gtfs/loader/Feed.java b/src/main/java/com/conveyal/gtfs/loader/Feed.java index 5b26a41a3..525c128f7 100644 --- a/src/main/java/com/conveyal/gtfs/loader/Feed.java +++ b/src/main/java/com/conveyal/gtfs/loader/Feed.java @@ -39,7 +39,7 @@ public class Feed { public final TableReader stops; public final TableReader trips; public final TableReader stopTimes; - public final TableReader patterns; + public final TableReader patterns; /** * Create a feed that reads tables over a JDBC connection. The connection should already be set to the right diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java index 460e393b3..4f99d1aea 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java @@ -242,11 +242,7 @@ public FeedLoadResult exportTables() { result.routes = export(Table.ROUTES, connection); } - // FIXME: Find some place to store errors encountered on export for patterns and pattern stops. - // FIXME: Is there a need to export patterns or pattern stops? Should these be iterated over to ensure that - // frequency-based pattern travel times match stop time arrivals/departures? -// export(Table.PATTERNS); -// export(Table.PATTERN_STOP); + result.patterns = export(Table.PATTERNS, connection); // Only write shapes for "approved" routes using COPY TO with results of select query if (fromEditor) { // Generate filter SQL for shapes if exporting a feed/schema that represents an editor snapshot. @@ -324,8 +320,6 @@ public FeedLoadResult exportTables() { result.trips = export(Table.TRIPS, connection); } - result.patterns = export(Table.PATTERNS, connection); - zipOutputStream.close(); // Run clean up on the resulting zip file. cleanUpZipFile(); @@ -414,7 +408,7 @@ private TableLoadResult export (Table table, String filterSql) { } // Create entry for table - String textFileName = table.name + ".txt"; + String textFileName = table.getFileName(); ZipEntry zipEntry = new ZipEntry(textFileName); zipOutputStream.putNextEntry(zipEntry); diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java index 4d509b1f8..3b255f4f9 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java @@ -322,7 +322,7 @@ private int getTableSize(Table table) { private int loadInternal(Table table) throws Exception { CsvReader csvReader = table.getCsvReader(zip, errorStorage); if (csvReader == null) { - LOG.info(String.format("file %s.txt not found in gtfs zipfile", table.name)); + LOG.info("File {} not found in gtfs zip file.", table.getFileName()); // This GTFS table could not be opened in the zip, even in a subdirectory. if (table.isRequired()) errorStorage.storeError(NewGTFSError.forTable(table, MISSING_TABLE)); return 0; diff --git a/src/main/java/com/conveyal/gtfs/loader/Table.java b/src/main/java/com/conveyal/gtfs/loader/Table.java index b3344c5c5..2e179ae15 100644 --- a/src/main/java/com/conveyal/gtfs/loader/Table.java +++ b/src/main/java/com/conveyal/gtfs/loader/Table.java @@ -103,6 +103,12 @@ public class Table { /** Key(s) that uniquely identify an entry in the respective table.*/ private String[] primaryKeyNames; + /** Prefixed to exported tables/files that are not part of the GTFS spec. */ + public static String proprietaryFilePrefix = "datatools_"; + + /** Identifies which tables on export are proprietary and should have the proprietary prefix applied. */ + public boolean isProprietaryTable = false; + public Table (String name, Class entityClass, Requirement required, Field... fields) { // TODO: verify table name is OK for use in constructing dynamic SQL queries this.name = name; @@ -252,8 +258,10 @@ public Table (String name, Class entityClass, Requirement requ new ShortField("direction_id", EDITOR, 1), new ShortField("use_frequency", EDITOR, 1), new StringField("shape_id", EDITOR).isReferenceTo(SHAPES) - ).addPrimaryKey() - .addPrimaryKeyNames("pattern_id"); + ) + .addPrimaryKey() + .addPrimaryKeyNames("pattern_id") + .hasProprietaryFilePrefix(); public static final Table STOPS = new Table("stops", Stop.class, REQUIRED, new StringField("stop_id", REQUIRED), @@ -487,6 +495,15 @@ public Table addPrimaryKeyNames(String ...keys) { return this; } + /** + * Fluent method that defines tables that on export should have the proprietary prefixed value applied to the file + * name. + */ + public Table hasProprietaryFilePrefix() { + this.isProprietaryTable = true; + return this; + } + /** * Get field names that are primary keys for a table. */ @@ -618,6 +635,13 @@ public String generateInsertSql (String namespace, boolean setDefaultId) { ); } + public String getFileName() { + String fileName = this.name + ".txt"; + return this.isProprietaryTable + ? Table.proprietaryFilePrefix + fileName + : fileName; + } + /** * In GTFS feeds, all files are supposed to be in the root of the zip file, but feed producers often put them * in a subdirectory. This function will search subdirectories if the entry is not found in the root. @@ -625,7 +649,7 @@ public String generateInsertSql (String namespace, boolean setDefaultId) { * It then creates a CSV reader for that table if it's found. */ public CsvReader getCsvReader(ZipFile zipFile, SQLErrorStorage sqlErrorStorage) { - final String tableFileName = this.name + ".txt"; + final String tableFileName = getFileName(); ZipEntry entry = zipFile.getEntry(tableFileName); if (entry == null) { // Table was not found, check if it is in a subdirectory. diff --git a/src/main/java/com/conveyal/gtfs/model/Pattern.java b/src/main/java/com/conveyal/gtfs/model/Pattern.java index 800872035..578dbe8b0 100644 --- a/src/main/java/com/conveyal/gtfs/model/Pattern.java +++ b/src/main/java/com/conveyal/gtfs/model/Pattern.java @@ -1,6 +1,8 @@ package com.conveyal.gtfs.model; import com.conveyal.gtfs.GTFSFeed; +import com.conveyal.gtfs.error.NoAgencyInFeedError; +import com.conveyal.gtfs.loader.Table; import com.google.common.base.Joiner; import org.locationtech.jts.geom.LineString; @@ -47,6 +49,8 @@ public String getId () { public String name; public String route_id; public int direction_id = INT_MISSING; + public int use_frequency = INT_MISSING; + public String shape_id; public static Joiner joiner = Joiner.on("-").skipNulls(); public String feed_id; @@ -107,20 +111,53 @@ else if (exemplarTrip.direction_id >= 0){ public Pattern () {} + public static class Loader extends Entity.Loader { + + public Loader(GTFSFeed feed) { + super(feed, Table.proprietaryFilePrefix + "patterns"); + } + + @Override + protected boolean isRequired() { + return false; + } + + @Override + public void loadOneRow() throws IOException { + Pattern pattern = new Pattern(); + pattern.id = row + 1; // offset line number by 1 to account for 0-based row index + pattern.pattern_id = getStringField("pattern_id", true); + pattern.route_id = getStringField("route_id", true); + pattern.name = getStringField("name", false); + pattern.direction_id = getIntField("direction_id", false, 0, 1); + pattern.use_frequency = getIntField("use_frequency", false, 0, 1); + pattern.shape_id = getStringField("shape_id", false); + pattern.feed = feed; + pattern.feed_id = feed.feedId; + // Attempting to put a null key or value will cause an NPE in BTreeMap. + if (pattern.pattern_id != null) feed.patterns.put(pattern.pattern_id, pattern); + } + + } + public static class Writer extends Entity.Writer { public Writer (GTFSFeed feed) { - super(feed, "patterns"); + super(feed, Table.proprietaryFilePrefix + "patterns"); } @Override protected void writeHeaders() throws IOException { - writer.writeRecord(new String[] {"pattern_id", "name"}); + writer.writeRecord(new String[] {"pattern_id", "route_id", "name", "direction_id", "use_frequency", "shape_id"}); } @Override protected void writeOneRow(Pattern pattern) throws IOException { writeStringField(pattern.pattern_id); + writeStringField(pattern.route_id); writeStringField(pattern.name); + writeIntField(pattern.direction_id); + writeIntField(pattern.use_frequency); + writeStringField(pattern.shape_id); endRecord(); } @@ -128,8 +165,6 @@ protected void writeOneRow(Pattern pattern) throws IOException { protected Iterator iterator() { return feed.patterns.values().iterator(); } - - } /** @@ -143,12 +178,14 @@ public void setStatementParameters(PreparedStatement statement, boolean setDefau statement.setString(oneBasedIndex++, pattern_id); statement.setString(oneBasedIndex++, route_id); statement.setString(oneBasedIndex++, name); - // Editor-specific fields + // Editor-specific fields. setIntParameter(statement, oneBasedIndex++, direction_id); - // Note: pattern#use_frequency is set in JdbcGtfsSnapshotter here: - // https://github.com/conveyal/gtfs-lib/blob/0c6aca98a83d534853b74011e6cc7bf376592581/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsSnapshotter.java#L196-L211 - setIntParameter(statement, oneBasedIndex++, INT_MISSING); // use_frequency - // FIXME: Shape set might be null? - statement.setString(oneBasedIndex++, associatedShapes.iterator().next()); + // Note: pattern#use_frequency is set in JdbcGtfsSnapshotter#populateDefaultEditorValues. + setIntParameter(statement, oneBasedIndex++, INT_MISSING); + if (associatedShapes != null) { + statement.setString(oneBasedIndex, associatedShapes.iterator().next()); + } else { + statement.setString(oneBasedIndex, ""); + } } } diff --git a/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java b/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java index 305c1bb2d..fb8018a4f 100644 --- a/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java +++ b/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java @@ -128,6 +128,16 @@ public void canDoRoundtripLoadAndWriteToZipFile() throws IOException { new DataExpectation("trip_id", "a30277f8-e50a-4a85-9141-b1e0da9d429d"), new DataExpectation("service_id", "04100312-8fe1-46a5-a9f2-556f39478f57") } + ), + new FileTestCase( + "datatools_patterns.txt", + new DataExpectation[]{ + new DataExpectation("pattern_id", "1"), + new DataExpectation("route_id", "1"), + new DataExpectation("name", "2 stops from Butler Ln to Scotts Valley Dr & Victor Sq (3 trips)"), + new DataExpectation("direction_id", "0"), + new DataExpectation("shape_id", "5820f377-f947-4728-ac29-ac0102cbc34e") + } ) }; diff --git a/src/test/resources/fake-agency/datatools_patterns.txt b/src/test/resources/fake-agency/datatools_patterns.txt new file mode 100644 index 000000000..36c7ce5d4 --- /dev/null +++ b/src/test/resources/fake-agency/datatools_patterns.txt @@ -0,0 +1,2 @@ +pattern_id,route_id,name,direction_id,use_frequency,shape_id +1,1,2 stops from Butler Ln to Scotts Valley Dr & Victor Sq (3 trips),0,,5820f377-f947-4728-ac29-ac0102cbc34e From 03c5f7e8c787fcbc4c1e2bc8a8921ce3165e99ec Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Thu, 31 Aug 2023 16:06:06 +0100 Subject: [PATCH 03/11] refactor(Various update to unit tests): --- .../conveyal/gtfs/loader/JdbcGtfsLoader.java | 8 ++++++- .../java/com/conveyal/gtfs/loader/Table.java | 6 ++--- .../java/com/conveyal/gtfs/GTFSFeedTest.java | 2 +- src/test/java/com/conveyal/gtfs/GTFSTest.java | 23 ++++++++++++++++++- .../fake-agency/datatools_patterns.txt | 2 +- 5 files changed, 34 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java index 3b255f4f9..0804926e9 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java @@ -353,7 +353,13 @@ private int loadInternal(Table table) throws Exception { // SQLite also doesn't support schemas, but you can attach additional database files with schema-like naming. // We'll just literally prepend feed identifiers to table names when supplied. // Some databases require the table to exist before a statement can be prepared. - targetTable.createSqlTable(connection); + if (table.name.equals("patterns")) { + // When creating the patterns table the id field must be flagged as serial and not bigint. This then allows + // the addition of new patterns in PatternBuilder#processPatternAndPatternStops. + targetTable.createSqlTable(connection, true); + } else { + targetTable.createSqlTable(connection); + } // TODO are we loading with or without a header row in our Postgres text file? if (postgresText) { diff --git a/src/main/java/com/conveyal/gtfs/loader/Table.java b/src/main/java/com/conveyal/gtfs/loader/Table.java index 2e179ae15..cf9b0fe70 100644 --- a/src/main/java/com/conveyal/gtfs/loader/Table.java +++ b/src/main/java/com/conveyal/gtfs/loader/Table.java @@ -255,9 +255,9 @@ public Table (String name, Class entityClass, Requirement requ new StringField("name", OPTIONAL), // Editor-specific fields. // direction_id and shape_id are exemplar fields applied to all trips for a pattern. - new ShortField("direction_id", EDITOR, 1), - new ShortField("use_frequency", EDITOR, 1), - new StringField("shape_id", EDITOR).isReferenceTo(SHAPES) + new ShortField("direction_id", OPTIONAL, 1), + new ShortField("use_frequency", OPTIONAL, 1), + new StringField("shape_id", OPTIONAL).isReferenceTo(SHAPES) ) .addPrimaryKey() .addPrimaryKeyNames("pattern_id") diff --git a/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java b/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java index fb8018a4f..e92c54167 100644 --- a/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java +++ b/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java @@ -134,7 +134,7 @@ public void canDoRoundtripLoadAndWriteToZipFile() throws IOException { new DataExpectation[]{ new DataExpectation("pattern_id", "1"), new DataExpectation("route_id", "1"), - new DataExpectation("name", "2 stops from Butler Ln to Scotts Valley Dr & Victor Sq (3 trips)"), + new DataExpectation("name", "2 stops from Butler Ln to Scotts Valley Dr & Victor Sq (1 trips)"), new DataExpectation("direction_id", "0"), new DataExpectation("shape_id", "5820f377-f947-4728-ac29-ac0102cbc34e") } diff --git a/src/test/java/com/conveyal/gtfs/GTFSTest.java b/src/test/java/com/conveyal/gtfs/GTFSTest.java index 19b0413d8..0124c020a 100644 --- a/src/test/java/com/conveyal/gtfs/GTFSTest.java +++ b/src/test/java/com/conveyal/gtfs/GTFSTest.java @@ -109,6 +109,7 @@ public void canLoadAndExportSimpleAgency() { ErrorExpectation[] fakeAgencyErrorExpectations = ErrorExpectation.list( new ErrorExpectation(NewGTFSErrorType.MISSING_FIELD), new ErrorExpectation(NewGTFSErrorType.REFERENTIAL_INTEGRITY), + new ErrorExpectation(NewGTFSErrorType.REFERENTIAL_INTEGRITY), new ErrorExpectation(NewGTFSErrorType.ROUTE_LONG_NAME_CONTAINS_SHORT_NAME), new ErrorExpectation(NewGTFSErrorType.FEED_TRAVEL_TIMES_ROUNDED), new ErrorExpectation(NewGTFSErrorType.STOP_UNUSED, equalTo("1234567")), @@ -996,6 +997,15 @@ private static int countValidationErrorsOfType( return errorCount; } + /** + * Proprietary table file names are prefix with "datatools_" to distinguish them from GTFS spec files. + */ + private String getTableFileName(String tableName) { + return (tableName.equals("patterns")) + ? String.format("datatools_%s.txt", tableName) + : tableName + ".txt"; + } + /** * Helper to assert that the GTFS that was exported to a zip file matches all data expectations defined in the * persistence expectations. @@ -1014,7 +1024,7 @@ private void assertThatExportedGtfsMeetsExpectations( if (persistenceExpectation.appliesToEditorDatabaseOnly) continue; // No need to check that errors were exported because it is an internal table only. if ("errors".equals(persistenceExpectation.tableName)) continue; - final String tableFileName = persistenceExpectation.tableName + ".txt"; + final String tableFileName = getTableFileName(persistenceExpectation.tableName); LOG.info(String.format("reading table: %s", tableFileName)); ZipEntry entry = gtfsZipfile.getEntry(tableFileName); @@ -1268,6 +1278,17 @@ private void assertThatPersistenceExpectationRecordWasFound( new RecordExpectation("route_color", "7CE6E7") } ), + new PersistenceExpectation( + "patterns", + new RecordExpectation[]{ + new RecordExpectation("pattern_id", "1"), + new RecordExpectation("route_id", "1"), + new RecordExpectation("name", "2 stops from Butler Ln to Scotts Valley Dr & Victor Sq (1 trips)"), + new RecordExpectation("direction_id", "0"), + new RecordExpectation("use_frequency", null), + new RecordExpectation("shape_id", "5820f377-f947-4728-ac29-ac0102cbc34e") + } + ), new PersistenceExpectation( "shapes", new RecordExpectation[]{ diff --git a/src/test/resources/fake-agency/datatools_patterns.txt b/src/test/resources/fake-agency/datatools_patterns.txt index 36c7ce5d4..7e72c5ac0 100644 --- a/src/test/resources/fake-agency/datatools_patterns.txt +++ b/src/test/resources/fake-agency/datatools_patterns.txt @@ -1,2 +1,2 @@ pattern_id,route_id,name,direction_id,use_frequency,shape_id -1,1,2 stops from Butler Ln to Scotts Valley Dr & Victor Sq (3 trips),0,,5820f377-f947-4728-ac29-ac0102cbc34e +1,1,2 stops from Butler Ln to Scotts Valley Dr & Victor Sq (1 trips),0,,5820f377-f947-4728-ac29-ac0102cbc34e From 6122629ffc337f90be96bcce2ba2067e89f4dd15 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Fri, 1 Sep 2023 08:42:29 +0100 Subject: [PATCH 04/11] refactor(GTFSTest.java): Fixed failing test and migrated temp folder creation to Java utils --- src/test/java/com/conveyal/gtfs/GTFSTest.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/test/java/com/conveyal/gtfs/GTFSTest.java b/src/test/java/com/conveyal/gtfs/GTFSTest.java index 0124c020a..bd5b0b0fc 100644 --- a/src/test/java/com/conveyal/gtfs/GTFSTest.java +++ b/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; @@ -247,12 +247,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()); @@ -272,6 +272,8 @@ public void canLoadAndExportSimpleAgencyInSubDirectory() { new ErrorExpectation(NewGTFSErrorType.TABLE_IN_SUBDIRECTORY), new ErrorExpectation(NewGTFSErrorType.TABLE_IN_SUBDIRECTORY), new ErrorExpectation(NewGTFSErrorType.TABLE_IN_SUBDIRECTORY), + new ErrorExpectation(NewGTFSErrorType.REFERENTIAL_INTEGRITY), + new ErrorExpectation(NewGTFSErrorType.TABLE_IN_SUBDIRECTORY), new ErrorExpectation(NewGTFSErrorType.TABLE_IN_SUBDIRECTORY), new ErrorExpectation(NewGTFSErrorType.TABLE_IN_SUBDIRECTORY), new ErrorExpectation(NewGTFSErrorType.TABLE_IN_SUBDIRECTORY), From 8e28e0976342c90ad44c62933ce43626016c0148 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Fri, 1 Sep 2023 15:44:50 +0100 Subject: [PATCH 05/11] refactor(Ordering to meet referential integrity): shapes are now loaded before patterns --- src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java | 5 +++-- src/test/java/com/conveyal/gtfs/GTFSTest.java | 2 -- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java index 0804926e9..860084860 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java @@ -151,15 +151,16 @@ public FeedLoadResult loadTables() { // This allows everything to work even when there's no prefix. this.tablePrefix += "."; } - // Load each table in turn, saving some summary information about what happened during each table load + // Load each table in turn, saving some summary information about what happened during each table load. + // The loading order is needed for referential integrity. result.agency = load(Table.AGENCY); result.calendar = load(Table.CALENDAR); result.calendarDates = load(Table.CALENDAR_DATES); result.routes = load(Table.ROUTES); result.fareAttributes = load(Table.FARE_ATTRIBUTES); result.feedInfo = load(Table.FEED_INFO); - result.patterns = load(Table.PATTERNS); result.shapes = load(Table.SHAPES); + result.patterns = load(Table.PATTERNS); // refs shapes and routes. result.stops = load(Table.STOPS); result.fareRules = load(Table.FARE_RULES); result.transfers = load(Table.TRANSFERS); diff --git a/src/test/java/com/conveyal/gtfs/GTFSTest.java b/src/test/java/com/conveyal/gtfs/GTFSTest.java index bd5b0b0fc..63631e10c 100644 --- a/src/test/java/com/conveyal/gtfs/GTFSTest.java +++ b/src/test/java/com/conveyal/gtfs/GTFSTest.java @@ -109,7 +109,6 @@ public void canLoadAndExportSimpleAgency() { ErrorExpectation[] fakeAgencyErrorExpectations = ErrorExpectation.list( new ErrorExpectation(NewGTFSErrorType.MISSING_FIELD), new ErrorExpectation(NewGTFSErrorType.REFERENTIAL_INTEGRITY), - new ErrorExpectation(NewGTFSErrorType.REFERENTIAL_INTEGRITY), new ErrorExpectation(NewGTFSErrorType.ROUTE_LONG_NAME_CONTAINS_SHORT_NAME), new ErrorExpectation(NewGTFSErrorType.FEED_TRAVEL_TIMES_ROUNDED), new ErrorExpectation(NewGTFSErrorType.STOP_UNUSED, equalTo("1234567")), @@ -272,7 +271,6 @@ public void canLoadAndExportSimpleAgencyInSubDirectory() throws IOException { new ErrorExpectation(NewGTFSErrorType.TABLE_IN_SUBDIRECTORY), new ErrorExpectation(NewGTFSErrorType.TABLE_IN_SUBDIRECTORY), new ErrorExpectation(NewGTFSErrorType.TABLE_IN_SUBDIRECTORY), - new ErrorExpectation(NewGTFSErrorType.REFERENTIAL_INTEGRITY), new ErrorExpectation(NewGTFSErrorType.TABLE_IN_SUBDIRECTORY), new ErrorExpectation(NewGTFSErrorType.TABLE_IN_SUBDIRECTORY), new ErrorExpectation(NewGTFSErrorType.TABLE_IN_SUBDIRECTORY), From 5ccdc8b5e0362bc15c79bb4a181e9d3777d3f9a8 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Mon, 4 Sep 2023 13:50:10 +0100 Subject: [PATCH 06/11] refactor(Addressed PR feedback): --- .../com/conveyal/gtfs/PatternBuilder.java | 19 +++++------- .../java/com/conveyal/gtfs/loader/Feed.java | 10 ++++-- .../gtfs/loader/JdbcGtfsExporter.java | 2 +- .../conveyal/gtfs/loader/JdbcGtfsLoader.java | 2 +- .../java/com/conveyal/gtfs/loader/Table.java | 31 ++++++------------- .../java/com/conveyal/gtfs/model/Pattern.java | 5 ++- .../gtfs/validator/ServiceValidator.java | 6 ++-- src/test/java/com/conveyal/gtfs/GTFSTest.java | 12 ++----- 8 files changed, 34 insertions(+), 53 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/PatternBuilder.java b/src/main/java/com/conveyal/gtfs/PatternBuilder.java index f45681f20..f8dcf0dcf 100644 --- a/src/main/java/com/conveyal/gtfs/PatternBuilder.java +++ b/src/main/java/com/conveyal/gtfs/PatternBuilder.java @@ -40,14 +40,10 @@ public PatternBuilder(Feed feed) throws SQLException { connection = feed.getConnection(); } - private String getTableName(String tableName) { - return feed.tablePrefix + tableName; - } - public void create(Map patterns, Set patternIdsLoadedFromFile) { - String patternsTableName = getTableName("patterns"); - String tripsTableName = getTableName("trips"); - String patternStopsTableName = getTableName("pattern_stops"); + String patternsTableName = feed.getTableName("patterns"); + String tripsTableName = feed.getTableName("trips"); + String patternStopsTableName = feed.getTableName("pattern_stops"); Table patternsTable = new Table(patternsTableName, Pattern.class, Requirement.EDITOR, Table.PATTERNS.fields); Table patternStopsTable = new Table(patternStopsTableName, PatternStop.class, Requirement.EDITOR, Table.PATTERN_STOP.fields); @@ -62,9 +58,10 @@ public void create(Map patterns, Set patternIds patternsTable.createSqlTable(connection, null, true); } patternStopsTable.createSqlTable(connection, null, true); - PrintStream patternForTripsFileStream = createTempPatternForTripsTable(tempPatternForTripsTextFile, statement); - processPatternAndPatternStops(patternsTable, patternStopsTable, patternForTripsFileStream, patterns, patternIdsLoadedFromFile); - updateTripPatternIds(tempPatternForTripsTextFile, patternForTripsFileStream, statement, tripsTableName); + try (PrintStream patternForTripsFileStream = createTempPatternForTripsTable(tempPatternForTripsTextFile, statement)) { + processPatternAndPatternStops(patternsTable, patternStopsTable, patternForTripsFileStream, patterns, patternIdsLoadedFromFile); + } + updateTripPatternIds(tempPatternForTripsTextFile, statement, tripsTableName); createIndexes(statement, patternsTableName, patternStopsTableName, tripsTableName); connection.commit(); } catch (SQLException | IOException e) { @@ -143,13 +140,11 @@ private void updateTripPatternReferences(PrintStream patternForTripsFileStream, */ private void updateTripPatternIds( File tempPatternForTripsTextFile, - PrintStream patternForTripsFileStream, Statement statement, String tripsTableName ) throws SQLException, IOException { LOG.info("Updating trips with pattern IDs."); - patternForTripsFileStream.close(); // Copy file contents into temp pattern for trips table. copyFromFile(connection, tempPatternForTripsTextFile, TEMP_FILE_NAME); // Before updating the trips with pattern IDs, index the table on trip_id. diff --git a/src/main/java/com/conveyal/gtfs/loader/Feed.java b/src/main/java/com/conveyal/gtfs/loader/Feed.java index 525c128f7..a4844129f 100644 --- a/src/main/java/com/conveyal/gtfs/loader/Feed.java +++ b/src/main/java/com/conveyal/gtfs/loader/Feed.java @@ -26,9 +26,9 @@ public class Feed { private final DataSource dataSource; - // The unique database schema name for this particular feed, including the separator charater (dot). + // The unique database schema name for this particular feed, including the separator character (dot). // This may be the empty string if the feed is stored in the root ("public") schema. - public final String tablePrefix; + private final String tablePrefix; public final TableReader agencies; public final TableReader calendars; @@ -154,4 +154,10 @@ public Connection getConnection() throws SQLException { return dataSource.getConnection(); } + /** + * @return the table name prefixed with this feed's database schema. + */ + public String getTableName(String tableName) { + return tablePrefix + tableName; + } } diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java index 4f99d1aea..ad1c72ddb 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java @@ -408,7 +408,7 @@ private TableLoadResult export (Table table, String filterSql) { } // Create entry for table - String textFileName = table.getFileName(); + String textFileName = Table.getTableFileName(table.name); ZipEntry zipEntry = new ZipEntry(textFileName); zipOutputStream.putNextEntry(zipEntry); diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java index 860084860..a666b3c3f 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java @@ -323,7 +323,7 @@ private int getTableSize(Table table) { private int loadInternal(Table table) throws Exception { CsvReader csvReader = table.getCsvReader(zip, errorStorage); if (csvReader == null) { - LOG.info("File {} not found in gtfs zip file.", table.getFileName()); + LOG.info("File {} not found in gtfs zip file.", Table.getTableFileName(table.name)); // This GTFS table could not be opened in the zip, even in a subdirectory. if (table.isRequired()) errorStorage.storeError(NewGTFSError.forTable(table, MISSING_TABLE)); return 0; diff --git a/src/main/java/com/conveyal/gtfs/loader/Table.java b/src/main/java/com/conveyal/gtfs/loader/Table.java index cf9b0fe70..587d3aeb6 100644 --- a/src/main/java/com/conveyal/gtfs/loader/Table.java +++ b/src/main/java/com/conveyal/gtfs/loader/Table.java @@ -104,10 +104,7 @@ public class Table { private String[] primaryKeyNames; /** Prefixed to exported tables/files that are not part of the GTFS spec. */ - public static String proprietaryFilePrefix = "datatools_"; - - /** Identifies which tables on export are proprietary and should have the proprietary prefix applied. */ - public boolean isProprietaryTable = false; + public static final String PROPRIETARY_FILE_PREFIX = "datatools_"; public Table (String name, Class entityClass, Requirement required, Field... fields) { // TODO: verify table name is OK for use in constructing dynamic SQL queries @@ -260,8 +257,7 @@ public Table (String name, Class entityClass, Requirement requ new StringField("shape_id", OPTIONAL).isReferenceTo(SHAPES) ) .addPrimaryKey() - .addPrimaryKeyNames("pattern_id") - .hasProprietaryFilePrefix(); + .addPrimaryKeyNames("pattern_id"); public static final Table STOPS = new Table("stops", Stop.class, REQUIRED, new StringField("stop_id", REQUIRED), @@ -495,15 +491,6 @@ public Table addPrimaryKeyNames(String ...keys) { return this; } - /** - * Fluent method that defines tables that on export should have the proprietary prefixed value applied to the file - * name. - */ - public Table hasProprietaryFilePrefix() { - this.isProprietaryTable = true; - return this; - } - /** * Get field names that are primary keys for a table. */ @@ -635,11 +622,13 @@ public String generateInsertSql (String namespace, boolean setDefaultId) { ); } - public String getFileName() { - String fileName = this.name + ".txt"; - return this.isProprietaryTable - ? Table.proprietaryFilePrefix + fileName - : fileName; + /** + * Proprietary table file names are prefix with "datatools_" to distinguish them from GTFS spec files. + */ + public static String getTableFileName(String tableName) { + return (tableName.equals("patterns")) + ? String.format("%s%s.txt", PROPRIETARY_FILE_PREFIX, tableName) + : tableName + ".txt"; } /** @@ -649,7 +638,7 @@ public String getFileName() { * It then creates a CSV reader for that table if it's found. */ public CsvReader getCsvReader(ZipFile zipFile, SQLErrorStorage sqlErrorStorage) { - final String tableFileName = getFileName(); + final String tableFileName = getTableFileName(this.name); ZipEntry entry = zipFile.getEntry(tableFileName); if (entry == null) { // Table was not found, check if it is in a subdirectory. diff --git a/src/main/java/com/conveyal/gtfs/model/Pattern.java b/src/main/java/com/conveyal/gtfs/model/Pattern.java index 578dbe8b0..ef20785d1 100644 --- a/src/main/java/com/conveyal/gtfs/model/Pattern.java +++ b/src/main/java/com/conveyal/gtfs/model/Pattern.java @@ -1,7 +1,6 @@ package com.conveyal.gtfs.model; import com.conveyal.gtfs.GTFSFeed; -import com.conveyal.gtfs.error.NoAgencyInFeedError; import com.conveyal.gtfs.loader.Table; import com.google.common.base.Joiner; import org.locationtech.jts.geom.LineString; @@ -114,7 +113,7 @@ public Pattern () {} public static class Loader extends Entity.Loader { public Loader(GTFSFeed feed) { - super(feed, Table.proprietaryFilePrefix + "patterns"); + super(feed, Table.PROPRIETARY_FILE_PREFIX + "patterns"); } @Override @@ -142,7 +141,7 @@ public void loadOneRow() throws IOException { public static class Writer extends Entity.Writer { public Writer (GTFSFeed feed) { - super(feed, Table.proprietaryFilePrefix + "patterns"); + super(feed, Table.PROPRIETARY_FILE_PREFIX + "patterns"); } @Override diff --git a/src/main/java/com/conveyal/gtfs/validator/ServiceValidator.java b/src/main/java/com/conveyal/gtfs/validator/ServiceValidator.java index 83036fe4a..8403c18c9 100644 --- a/src/main/java/com/conveyal/gtfs/validator/ServiceValidator.java +++ b/src/main/java/com/conveyal/gtfs/validator/ServiceValidator.java @@ -268,7 +268,7 @@ select durations.service_id, duration_seconds, days_active from ( // Except that some service IDs may have no trips on them, or may not be referenced in any calendar or // calendar exception, which would keep them from appearing in either of those tables. So we just create // this somewhat redundant materialized view to serve as a master list of all services. - String servicesTableName = feed.tablePrefix + "services"; + String servicesTableName = feed.getTableName("services"); String sql = String.format("create table %s (service_id varchar, n_days_active integer, duration_seconds integer, n_trips integer)", servicesTableName); LOG.info(sql); statement.execute(sql); @@ -285,7 +285,7 @@ select durations.service_id, duration_seconds, days_active from ( serviceTracker.executeRemaining(); // Create a table that shows on which dates each service is active. - String serviceDatesTableName = feed.tablePrefix + "service_dates"; + String serviceDatesTableName = feed.getTableName("service_dates"); sql = String.format("create table %s (service_date varchar, service_id varchar)", serviceDatesTableName); LOG.info(sql); statement.execute(sql); @@ -318,7 +318,7 @@ select durations.service_id, duration_seconds, days_active from ( // where dates.service_id = durations.service_id // group by service_date, route_type order by service_date, route_type; - String serviceDurationsTableName = feed.tablePrefix + "service_durations"; + String serviceDurationsTableName = feed.getTableName("service_durations"); sql = String.format("create table %s (service_id varchar, route_type integer, " + "duration_seconds integer, primary key (service_id, route_type))", serviceDurationsTableName); LOG.info(sql); diff --git a/src/test/java/com/conveyal/gtfs/GTFSTest.java b/src/test/java/com/conveyal/gtfs/GTFSTest.java index 63631e10c..7fbc27698 100644 --- a/src/test/java/com/conveyal/gtfs/GTFSTest.java +++ b/src/test/java/com/conveyal/gtfs/GTFSTest.java @@ -5,6 +5,7 @@ import com.conveyal.gtfs.loader.FeedLoadResult; import com.conveyal.gtfs.loader.JdbcGtfsExporter; import com.conveyal.gtfs.loader.SnapshotResult; +import com.conveyal.gtfs.loader.Table; import com.conveyal.gtfs.storage.ErrorExpectation; import com.conveyal.gtfs.storage.ExpectedFieldType; import com.conveyal.gtfs.storage.PersistenceExpectation; @@ -997,15 +998,6 @@ private static int countValidationErrorsOfType( return errorCount; } - /** - * Proprietary table file names are prefix with "datatools_" to distinguish them from GTFS spec files. - */ - private String getTableFileName(String tableName) { - return (tableName.equals("patterns")) - ? String.format("datatools_%s.txt", tableName) - : tableName + ".txt"; - } - /** * Helper to assert that the GTFS that was exported to a zip file matches all data expectations defined in the * persistence expectations. @@ -1024,7 +1016,7 @@ private void assertThatExportedGtfsMeetsExpectations( if (persistenceExpectation.appliesToEditorDatabaseOnly) continue; // No need to check that errors were exported because it is an internal table only. if ("errors".equals(persistenceExpectation.tableName)) continue; - final String tableFileName = getTableFileName(persistenceExpectation.tableName); + final String tableFileName = Table.getTableFileName(persistenceExpectation.tableName); LOG.info(String.format("reading table: %s", tableFileName)); ZipEntry entry = gtfsZipfile.getEntry(tableFileName); From e648c3814e755ebff08e15f491776e148846ad71 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Tue, 5 Sep 2023 17:53:53 +0100 Subject: [PATCH 07/11] refactor(Addressed PR feedback): Update table file name creation --- .../com/conveyal/gtfs/PatternBuilder.java | 6 ++-- .../java/com/conveyal/gtfs/loader/Feed.java | 36 +++++++++---------- .../gtfs/loader/JdbcGtfsExporter.java | 2 +- .../conveyal/gtfs/loader/JdbcGtfsLoader.java | 2 +- .../java/com/conveyal/gtfs/loader/Table.java | 16 ++++++--- .../java/com/conveyal/gtfs/model/Pattern.java | 4 +-- .../gtfs/validator/ServiceValidator.java | 6 ++-- src/test/java/com/conveyal/gtfs/GTFSTest.java | 2 +- 8 files changed, 41 insertions(+), 33 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/PatternBuilder.java b/src/main/java/com/conveyal/gtfs/PatternBuilder.java index f8dcf0dcf..9b3c879ef 100644 --- a/src/main/java/com/conveyal/gtfs/PatternBuilder.java +++ b/src/main/java/com/conveyal/gtfs/PatternBuilder.java @@ -41,9 +41,9 @@ public PatternBuilder(Feed feed) throws SQLException { } public void create(Map patterns, Set patternIdsLoadedFromFile) { - String patternsTableName = feed.getTableName("patterns"); - String tripsTableName = feed.getTableName("trips"); - String patternStopsTableName = feed.getTableName("pattern_stops"); + String patternsTableName = feed.getTableNameWithSchemaPrefix("patterns"); + String tripsTableName = feed.getTableNameWithSchemaPrefix("trips"); + String patternStopsTableName = feed.getTableNameWithSchemaPrefix("pattern_stops"); Table patternsTable = new Table(patternsTableName, Pattern.class, Requirement.EDITOR, Table.PATTERNS.fields); Table patternStopsTable = new Table(patternStopsTableName, PatternStop.class, Requirement.EDITOR, Table.PATTERN_STOP.fields); diff --git a/src/main/java/com/conveyal/gtfs/loader/Feed.java b/src/main/java/com/conveyal/gtfs/loader/Feed.java index a4844129f..1430b5751 100644 --- a/src/main/java/com/conveyal/gtfs/loader/Feed.java +++ b/src/main/java/com/conveyal/gtfs/loader/Feed.java @@ -28,7 +28,7 @@ public class Feed { // The unique database schema name for this particular feed, including the separator character (dot). // This may be the empty string if the feed is stored in the root ("public") schema. - private final String tablePrefix; + private final String databaseSchemaPrefix; public final TableReader agencies; public final TableReader calendars; @@ -44,23 +44,23 @@ public class Feed { /** * Create a feed that reads tables over a JDBC connection. The connection should already be set to the right * schema within the database. - * @param tablePrefix the unique prefix for the table (may be null for no prefix) + * @param databaseSchemaPrefix the unique prefix for the table (may be null for no prefix) */ - public Feed (DataSource dataSource, String tablePrefix) { + public Feed (DataSource dataSource, String databaseSchemaPrefix) { this.dataSource = dataSource; // Ensure separator dot is present - if (tablePrefix != null && !tablePrefix.endsWith(".")) tablePrefix += "."; - this.tablePrefix = tablePrefix == null ? "" : tablePrefix; - agencies = new JDBCTableReader(Table.AGENCY, dataSource, tablePrefix, EntityPopulator.AGENCY); - fareAttributes = new JDBCTableReader(Table.FARE_ATTRIBUTES, dataSource, tablePrefix, EntityPopulator.FARE_ATTRIBUTE); - frequencies = new JDBCTableReader(Table.FREQUENCIES, dataSource, tablePrefix, EntityPopulator.FREQUENCY); - calendars = new JDBCTableReader(Table.CALENDAR, dataSource, tablePrefix, EntityPopulator.CALENDAR); - calendarDates = new JDBCTableReader(Table.CALENDAR_DATES, dataSource, tablePrefix, EntityPopulator.CALENDAR_DATE); - routes = new JDBCTableReader(Table.ROUTES, dataSource, tablePrefix, EntityPopulator.ROUTE); - stops = new JDBCTableReader(Table.STOPS, dataSource, tablePrefix, EntityPopulator.STOP); - trips = new JDBCTableReader(Table.TRIPS, dataSource, tablePrefix, EntityPopulator.TRIP); - stopTimes = new JDBCTableReader(Table.STOP_TIMES, dataSource, tablePrefix, EntityPopulator.STOP_TIME); - patterns = new JDBCTableReader(Table.PATTERNS, dataSource, tablePrefix, EntityPopulator.PATTERN); + if (databaseSchemaPrefix != null && !databaseSchemaPrefix.endsWith(".")) databaseSchemaPrefix += "."; + this.databaseSchemaPrefix = databaseSchemaPrefix == null ? "" : databaseSchemaPrefix; + agencies = new JDBCTableReader(Table.AGENCY, dataSource, databaseSchemaPrefix, EntityPopulator.AGENCY); + fareAttributes = new JDBCTableReader(Table.FARE_ATTRIBUTES, dataSource, databaseSchemaPrefix, EntityPopulator.FARE_ATTRIBUTE); + frequencies = new JDBCTableReader(Table.FREQUENCIES, dataSource, databaseSchemaPrefix, EntityPopulator.FREQUENCY); + calendars = new JDBCTableReader(Table.CALENDAR, dataSource, databaseSchemaPrefix, EntityPopulator.CALENDAR); + calendarDates = new JDBCTableReader(Table.CALENDAR_DATES, dataSource, databaseSchemaPrefix, EntityPopulator.CALENDAR_DATE); + routes = new JDBCTableReader(Table.ROUTES, dataSource, databaseSchemaPrefix, EntityPopulator.ROUTE); + stops = new JDBCTableReader(Table.STOPS, dataSource, databaseSchemaPrefix, EntityPopulator.STOP); + trips = new JDBCTableReader(Table.TRIPS, dataSource, databaseSchemaPrefix, EntityPopulator.TRIP); + stopTimes = new JDBCTableReader(Table.STOP_TIMES, dataSource, databaseSchemaPrefix, EntityPopulator.STOP_TIME); + patterns = new JDBCTableReader(Table.PATTERNS, dataSource, databaseSchemaPrefix, EntityPopulator.PATTERN); } /** @@ -80,7 +80,7 @@ public ValidationResult validate (FeedValidatorCreator... additionalValidators) // Reconnect to the existing error tables. SQLErrorStorage errorStorage; try { - errorStorage = new SQLErrorStorage(dataSource.getConnection(), tablePrefix, false); + errorStorage = new SQLErrorStorage(dataSource.getConnection(), databaseSchemaPrefix, false); } catch (SQLException | InvalidNamespaceException ex) { throw new StorageException(ex); } @@ -157,7 +157,7 @@ public Connection getConnection() throws SQLException { /** * @return the table name prefixed with this feed's database schema. */ - public String getTableName(String tableName) { - return tablePrefix + tableName; + public String getTableNameWithSchemaPrefix(String tableName) { + return databaseSchemaPrefix + tableName; } } diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java index ad1c72ddb..9e84f7301 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java @@ -408,7 +408,7 @@ private TableLoadResult export (Table table, String filterSql) { } // Create entry for table - String textFileName = Table.getTableFileName(table.name); + String textFileName = Table.getTableFileNameWithExtension(table.name); ZipEntry zipEntry = new ZipEntry(textFileName); zipOutputStream.putNextEntry(zipEntry); diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java index a666b3c3f..37f55ba96 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java @@ -323,7 +323,7 @@ private int getTableSize(Table table) { private int loadInternal(Table table) throws Exception { CsvReader csvReader = table.getCsvReader(zip, errorStorage); if (csvReader == null) { - LOG.info("File {} not found in gtfs zip file.", Table.getTableFileName(table.name)); + LOG.info("File {} not found in gtfs zip file.", Table.getTableFileNameWithExtension(table.name)); // This GTFS table could not be opened in the zip, even in a subdirectory. if (table.isRequired()) errorStorage.storeError(NewGTFSError.forTable(table, MISSING_TABLE)); return 0; diff --git a/src/main/java/com/conveyal/gtfs/loader/Table.java b/src/main/java/com/conveyal/gtfs/loader/Table.java index 587d3aeb6..a052db565 100644 --- a/src/main/java/com/conveyal/gtfs/loader/Table.java +++ b/src/main/java/com/conveyal/gtfs/loader/Table.java @@ -622,13 +622,21 @@ public String generateInsertSql (String namespace, boolean setDefaultId) { ); } + public static String getTableFileNameWithExtension(String tableName) { + return getTableFileName(tableName, ".txt"); + } + + public static String getTableFileName(String tableName) { + return getTableFileName(tableName, ""); + } + /** * Proprietary table file names are prefix with "datatools_" to distinguish them from GTFS spec files. */ - public static String getTableFileName(String tableName) { + private static String getTableFileName(String tableName, String fileExtension) { return (tableName.equals("patterns")) - ? String.format("%s%s.txt", PROPRIETARY_FILE_PREFIX, tableName) - : tableName + ".txt"; + ? String.format("%s%s%s", PROPRIETARY_FILE_PREFIX, tableName, fileExtension) + : String.format("%s%s", tableName, fileExtension); } /** @@ -638,7 +646,7 @@ public static String getTableFileName(String tableName) { * It then creates a CSV reader for that table if it's found. */ public CsvReader getCsvReader(ZipFile zipFile, SQLErrorStorage sqlErrorStorage) { - final String tableFileName = getTableFileName(this.name); + final String tableFileName = getTableFileNameWithExtension(this.name); ZipEntry entry = zipFile.getEntry(tableFileName); if (entry == null) { // Table was not found, check if it is in a subdirectory. diff --git a/src/main/java/com/conveyal/gtfs/model/Pattern.java b/src/main/java/com/conveyal/gtfs/model/Pattern.java index ef20785d1..559468208 100644 --- a/src/main/java/com/conveyal/gtfs/model/Pattern.java +++ b/src/main/java/com/conveyal/gtfs/model/Pattern.java @@ -113,7 +113,7 @@ public Pattern () {} public static class Loader extends Entity.Loader { public Loader(GTFSFeed feed) { - super(feed, Table.PROPRIETARY_FILE_PREFIX + "patterns"); + super(feed, Table.getTableFileName("patterns")); } @Override @@ -141,7 +141,7 @@ public void loadOneRow() throws IOException { public static class Writer extends Entity.Writer { public Writer (GTFSFeed feed) { - super(feed, Table.PROPRIETARY_FILE_PREFIX + "patterns"); + super(feed, Table.getTableFileName("patterns")); } @Override diff --git a/src/main/java/com/conveyal/gtfs/validator/ServiceValidator.java b/src/main/java/com/conveyal/gtfs/validator/ServiceValidator.java index 8403c18c9..c2ede5888 100644 --- a/src/main/java/com/conveyal/gtfs/validator/ServiceValidator.java +++ b/src/main/java/com/conveyal/gtfs/validator/ServiceValidator.java @@ -268,7 +268,7 @@ select durations.service_id, duration_seconds, days_active from ( // Except that some service IDs may have no trips on them, or may not be referenced in any calendar or // calendar exception, which would keep them from appearing in either of those tables. So we just create // this somewhat redundant materialized view to serve as a master list of all services. - String servicesTableName = feed.getTableName("services"); + String servicesTableName = feed.getTableNameWithSchemaPrefix("services"); String sql = String.format("create table %s (service_id varchar, n_days_active integer, duration_seconds integer, n_trips integer)", servicesTableName); LOG.info(sql); statement.execute(sql); @@ -285,7 +285,7 @@ select durations.service_id, duration_seconds, days_active from ( serviceTracker.executeRemaining(); // Create a table that shows on which dates each service is active. - String serviceDatesTableName = feed.getTableName("service_dates"); + String serviceDatesTableName = feed.getTableNameWithSchemaPrefix("service_dates"); sql = String.format("create table %s (service_date varchar, service_id varchar)", serviceDatesTableName); LOG.info(sql); statement.execute(sql); @@ -318,7 +318,7 @@ select durations.service_id, duration_seconds, days_active from ( // where dates.service_id = durations.service_id // group by service_date, route_type order by service_date, route_type; - String serviceDurationsTableName = feed.getTableName("service_durations"); + String serviceDurationsTableName = feed.getTableNameWithSchemaPrefix("service_durations"); sql = String.format("create table %s (service_id varchar, route_type integer, " + "duration_seconds integer, primary key (service_id, route_type))", serviceDurationsTableName); LOG.info(sql); diff --git a/src/test/java/com/conveyal/gtfs/GTFSTest.java b/src/test/java/com/conveyal/gtfs/GTFSTest.java index 7fbc27698..1aad4b97e 100644 --- a/src/test/java/com/conveyal/gtfs/GTFSTest.java +++ b/src/test/java/com/conveyal/gtfs/GTFSTest.java @@ -1016,7 +1016,7 @@ private void assertThatExportedGtfsMeetsExpectations( if (persistenceExpectation.appliesToEditorDatabaseOnly) continue; // No need to check that errors were exported because it is an internal table only. if ("errors".equals(persistenceExpectation.tableName)) continue; - final String tableFileName = Table.getTableFileName(persistenceExpectation.tableName); + final String tableFileName = Table.getTableFileNameWithExtension(persistenceExpectation.tableName); LOG.info(String.format("reading table: %s", tableFileName)); ZipEntry entry = gtfsZipfile.getEntry(tableFileName); From 90bf5f0e34deedcd12079fd083e945be8d8a84f4 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Thu, 5 Oct 2023 13:26:25 +0100 Subject: [PATCH 08/11] refactor(PR feedback): Just removed a few redundant lines --- src/main/java/com/conveyal/gtfs/PatternBuilder.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/PatternBuilder.java b/src/main/java/com/conveyal/gtfs/PatternBuilder.java index 9b3c879ef..c3361e5ca 100644 --- a/src/main/java/com/conveyal/gtfs/PatternBuilder.java +++ b/src/main/java/com/conveyal/gtfs/PatternBuilder.java @@ -143,7 +143,6 @@ private void updateTripPatternIds( Statement statement, String tripsTableName ) throws SQLException, IOException { - LOG.info("Updating trips with pattern IDs."); // Copy file contents into temp pattern for trips table. copyFromFile(connection, tempPatternForTripsTextFile, TEMP_FILE_NAME); @@ -190,7 +189,6 @@ private void createIndexes( * Construct pattern stops based on values in trip pattern key. */ private void createPatternStops(TripPatternKey key, String patternId, Table patternStopsTable) throws SQLException { - String insertPatternStopSql = patternStopsTable.generateInsertSql(true); PreparedStatement insertPatternStopStatement = connection.prepareStatement(insertPatternStopSql); BatchTracker patternStopTracker = new BatchTracker("pattern stop", insertPatternStopStatement); From a5f60417e38043d35148aa6873c93056e69e9c87 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Thu, 5 Oct 2023 15:35:20 +0100 Subject: [PATCH 09/11] refactor(Added export prop file param): --- .../conveyal/gtfs/loader/JdbcGtfsExporter.java | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java index 9e84f7301..b0e5d9666 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java @@ -44,6 +44,9 @@ public class JdbcGtfsExporter { private final DataSource dataSource; private final boolean fromEditor; + /** If this is true will export tables prefixed with {@link Table#PROPRIETARY_FILE_PREFIX} **/ + private final boolean exportProprietaryFiles; + // These fields will be filled in once feed snapshot begins. private Connection connection; private ZipOutputStream zipOutputStream; @@ -66,6 +69,15 @@ public JdbcGtfsExporter(String feedId, String outFile, DataSource dataSource, bo this.outFile = outFile; this.dataSource = dataSource; this.fromEditor = fromEditor; + this.exportProprietaryFiles = true; + } + + public JdbcGtfsExporter(String feedId, String outFile, DataSource dataSource, boolean fromEditor, boolean exportProprietaryFiles) { + this.feedIdToExport = feedId; + this.outFile = outFile; + this.dataSource = dataSource; + this.fromEditor = fromEditor; + this.exportProprietaryFiles = exportProprietaryFiles; } /** @@ -242,7 +254,9 @@ public FeedLoadResult exportTables() { result.routes = export(Table.ROUTES, connection); } - result.patterns = export(Table.PATTERNS, connection); + if (exportProprietaryFiles) { + result.patterns = export(Table.PATTERNS, connection); + } // Only write shapes for "approved" routes using COPY TO with results of select query if (fromEditor) { // Generate filter SQL for shapes if exporting a feed/schema that represents an editor snapshot. From 026599ba4907f4c42b24e883d154e63b0777dd31 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Mon, 9 Oct 2023 15:46:42 +0100 Subject: [PATCH 10/11] refactor(export proprietary files): publish proprietary files if required --- src/main/java/com/conveyal/gtfs/GTFS.java | 12 +++--- .../gtfs/loader/JdbcGtfsExporter.java | 30 +++++++------ src/test/java/com/conveyal/gtfs/GTFSTest.java | 42 ++++++++++++++++--- 3 files changed, 61 insertions(+), 23 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/GTFS.java b/src/main/java/com/conveyal/gtfs/GTFS.java index aec965db5..5c85d9148 100644 --- a/src/main/java/com/conveyal/gtfs/GTFS.java +++ b/src/main/java/com/conveyal/gtfs/GTFS.java @@ -46,10 +46,9 @@ public abstract class GTFS { /** * Export a feed ID from the database to a zipped GTFS file in the specified export directory. */ - public static FeedLoadResult export (String feedId, String outFile, DataSource dataSource, boolean fromEditor) { - JdbcGtfsExporter exporter = new JdbcGtfsExporter(feedId, outFile, dataSource, fromEditor); - FeedLoadResult result = exporter.exportTables(); - return result; + public static FeedLoadResult export (String feedId, String outFile, DataSource dataSource, boolean fromEditor, boolean publishProprietaryFiles) { + JdbcGtfsExporter exporter = new JdbcGtfsExporter(feedId, outFile, dataSource, fromEditor, publishProprietaryFiles); + return exporter.exportTables(); } /** @@ -299,13 +298,16 @@ public static void main (String[] args) throws IOException { if (cmd.hasOption("export")) { String namespaceToExport = cmd.getOptionValue("export"); + boolean exportProprietaryFiles = (cmd.getOptionValue("exportProprietaryFiles") != null) + && Boolean.parseBoolean(cmd.getOptionValue("exportProprietaryFiles")); + String outFile = cmd.getOptionValue("outFile"); if (namespaceToExport == null && loadResult != null) { namespaceToExport = loadResult.uniqueIdentifier; } if (namespaceToExport != null) { LOG.info("Exporting feed with unique identifier {}", namespaceToExport); - FeedLoadResult exportResult = export(namespaceToExport, outFile, dataSource, true); + export(namespaceToExport, outFile, dataSource, true, exportProprietaryFiles); LOG.info("Done exporting."); } else { LOG.error("No feed to export. Specify one, or load a feed in the same command."); diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java index b0e5d9666..3e219eba3 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java @@ -45,7 +45,7 @@ public class JdbcGtfsExporter { private final boolean fromEditor; /** If this is true will export tables prefixed with {@link Table#PROPRIETARY_FILE_PREFIX} **/ - private final boolean exportProprietaryFiles; + private final boolean publishProprietaryFiles; // These fields will be filled in once feed snapshot begins. private Connection connection; @@ -63,21 +63,17 @@ public class JdbcGtfsExporter { Table.TRIPS.fileName ); + public static final List proprietaryFileList = Lists.newArrayList( + String.format("%s%s", Table.PROPRIETARY_FILE_PREFIX, Table.PATTERNS.fileName) + ); - public JdbcGtfsExporter(String feedId, String outFile, DataSource dataSource, boolean fromEditor) { - this.feedIdToExport = feedId; - this.outFile = outFile; - this.dataSource = dataSource; - this.fromEditor = fromEditor; - this.exportProprietaryFiles = true; - } - public JdbcGtfsExporter(String feedId, String outFile, DataSource dataSource, boolean fromEditor, boolean exportProprietaryFiles) { + public JdbcGtfsExporter(String feedId, String outFile, DataSource dataSource, boolean fromEditor, boolean publishProprietaryFiles) { this.feedIdToExport = feedId; this.outFile = outFile; this.dataSource = dataSource; this.fromEditor = fromEditor; - this.exportProprietaryFiles = exportProprietaryFiles; + this.publishProprietaryFiles = publishProprietaryFiles; } /** @@ -254,9 +250,6 @@ public FeedLoadResult exportTables() { result.routes = export(Table.ROUTES, connection); } - if (exportProprietaryFiles) { - result.patterns = export(Table.PATTERNS, connection); - } // Only write shapes for "approved" routes using COPY TO with results of select query if (fromEditor) { // Generate filter SQL for shapes if exporting a feed/schema that represents an editor snapshot. @@ -334,6 +327,8 @@ public FeedLoadResult exportTables() { result.trips = export(Table.TRIPS, connection); } + exportProprietaryFiles(result); + zipOutputStream.close(); // Run clean up on the resulting zip file. cleanUpZipFile(); @@ -354,6 +349,15 @@ public FeedLoadResult exportTables() { return result; } + /** + * Export proprietary files, if they are required. + */ + private void exportProprietaryFiles(FeedLoadResult result) { + if (publishProprietaryFiles) { + result.patterns = export(Table.PATTERNS, connection); + } + } + /** * Removes any empty zip files from the final zip file unless they are required files. */ diff --git a/src/test/java/com/conveyal/gtfs/GTFSTest.java b/src/test/java/com/conveyal/gtfs/GTFSTest.java index 1aad4b97e..470711d0b 100644 --- a/src/test/java/com/conveyal/gtfs/GTFSTest.java +++ b/src/test/java/com/conveyal/gtfs/GTFSTest.java @@ -635,7 +635,7 @@ void canExportEmptyMandatoryFiles() { } // Confirm that the mandatory files are present in the zip file. - tempFile = exportGtfs(namespace, dataSource, false); + tempFile = exportGtfs(namespace, dataSource, false, true); ZipFile gtfsZipFile = new ZipFile(tempFile.getAbsolutePath()); for (String fileName : JdbcGtfsExporter.mandatoryFileList) { Assert.assertNotNull(gtfsZipFile.getEntry(fileName)); @@ -647,6 +647,33 @@ void canExportEmptyMandatoryFiles() { if (tempFile != null) tempFile.deleteOnExit(); } } + /** + * Load a feed and then export minus proprietary files. Confirm proprietary files are not present in export. + */ + @Test + void canOmitProprietaryFiles() { + String testDBName = TestUtils.generateNewDB(); + File tempFile = null; + try { + String zipFileName = TestUtils.zipFolderFiles("fake-agency", true); + String dbConnectionUrl = String.join("/", JDBC_URL, testDBName); + DataSource dataSource = TestUtils.createTestDataSource(dbConnectionUrl); + FeedLoadResult loadResult = GTFS.load(zipFileName, dataSource); + String namespace = loadResult.uniqueIdentifier; + + // Confirm that the proprietary files are not present in the zip file. + tempFile = exportGtfs(namespace, dataSource, false, false); + ZipFile gtfsZipFile = new ZipFile(tempFile.getAbsolutePath()); + for (String fileName : JdbcGtfsExporter.proprietaryFileList) { + Assert.assertNull(gtfsZipFile.getEntry(fileName)); + } + } catch (IOException e) { + LOG.error("An error occurred while attempting to test exporting of proprietary files.", e); + } finally { + TestUtils.dropDB(testDBName); + if (tempFile != null) tempFile.deleteOnExit(); + } + } /** * A helper method that will run GTFS#main with a certain zip file. @@ -693,7 +720,7 @@ private boolean runIntegrationTestOnZipFile( // Verify that exporting the feed (in non-editor mode) completes and data is outputted properly LOG.info("export GTFS from created namespace"); - File tempFile = exportGtfs(namespace, dataSource, false); + File tempFile = exportGtfs(namespace, dataSource, false, true); assertThatExportedGtfsMeetsExpectations(tempFile, persistenceExpectations, false); // Verify that making a snapshot from an existing feed database, then exporting that snapshot to a GTFS zip @@ -794,9 +821,14 @@ private void assertThatSnapshotIsErrorFree(SnapshotResult snapshotResult) { /** * Helper function to export a GTFS from the database to a temporary zip file. */ - private File exportGtfs(String namespace, DataSource dataSource, boolean fromEditor) throws IOException { +// private File exportGtfs(String namespace, DataSource dataSource, boolean fromEditor) throws IOException { +// File tempFile = File.createTempFile("snapshot", ".zip"); +// GTFS.export(namespace, tempFile.getAbsolutePath(), dataSource, fromEditor, false); +// return tempFile; +// } + private File exportGtfs(String namespace, DataSource dataSource, boolean fromEditor, boolean publishProprietaryFiles) throws IOException { File tempFile = File.createTempFile("snapshot", ".zip"); - GTFS.export(namespace, tempFile.getAbsolutePath(), dataSource, fromEditor); + GTFS.export(namespace, tempFile.getAbsolutePath(), dataSource, fromEditor, publishProprietaryFiles); return tempFile; } @@ -833,7 +865,7 @@ private boolean assertThatSnapshotIsSuccessful( true ); LOG.info("export GTFS from copied namespace"); - File tempFile = exportGtfs(copyResult.uniqueIdentifier, dataSource, true); + File tempFile = exportGtfs(copyResult.uniqueIdentifier, dataSource, true, true); assertThatExportedGtfsMeetsExpectations(tempFile, persistenceExpectations, true); } catch (IOException | SQLException e) { e.printStackTrace(); From 10307d43c6610d5dd1cc07d30091596816772727 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Tue, 10 Oct 2023 12:13:09 +0100 Subject: [PATCH 11/11] refactor(JdbcGtfsExporter.java): Added additional logging. --- src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java index 3e219eba3..bf77ba7e1 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsExporter.java @@ -354,7 +354,10 @@ public FeedLoadResult exportTables() { */ private void exportProprietaryFiles(FeedLoadResult result) { if (publishProprietaryFiles) { + LOG.info("Exporting proprietary files."); result.patterns = export(Table.PATTERNS, connection); + } else { + LOG.info("Proprietary files not exported."); } }