From 3b25aa17d8cf5117b2127ca06ef45dda5c3d1f71 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Thu, 11 Jul 2024 14:36:45 +0100 Subject: [PATCH] refactor(Updates and house keeping following review): --- .../gtfs/graphql/fetchers/NestedJDBCFetcher.java | 2 -- src/main/java/com/conveyal/gtfs/loader/Feed.java | 4 ++-- .../java/com/conveyal/gtfs/loader/FeedLoadResult.java | 1 + .../com/conveyal/gtfs/loader/JdbcGTFSFeedConverter.java | 2 +- .../java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java | 4 ++-- .../com/conveyal/gtfs/loader/JdbcGtfsSnapshotter.java | 2 +- .../java/com/conveyal/gtfs/loader/JdbcTableWriter.java | 2 +- .../com/conveyal/gtfs/loader/PatternReconciliation.java | 9 --------- .../java/com/conveyal/gtfs/loader/ReferenceTracker.java | 2 +- .../conveyal/gtfs/validator/NewTripTimesValidator.java | 7 ------- .../com/conveyal/gtfs/validator/ServiceValidator.java | 5 +++++ src/test/java/com/conveyal/gtfs/GTFSFeedTest.java | 2 -- src/test/java/com/conveyal/gtfs/GTFSTest.java | 2 +- 13 files changed, 15 insertions(+), 29 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/graphql/fetchers/NestedJDBCFetcher.java b/src/main/java/com/conveyal/gtfs/graphql/fetchers/NestedJDBCFetcher.java index e7d30f364..7478a6304 100644 --- a/src/main/java/com/conveyal/gtfs/graphql/fetchers/NestedJDBCFetcher.java +++ b/src/main/java/com/conveyal/gtfs/graphql/fetchers/NestedJDBCFetcher.java @@ -53,8 +53,6 @@ public static GraphQLFieldDefinition field (String tableName) { // Auto limit is set to false in first fetchers because otherwise the limit could cut short the // queries which first get the fields to join to routes. new JDBCFetcher("pattern_stops", "stop_id", null, false), - new JDBCFetcher("pattern_locations", "location_id", null, false), - new JDBCFetcher("pattern_location_group_stops", "location_group_id", null, false), new JDBCFetcher("patterns", "pattern_id", null, false), new JDBCFetcher("routes", "route_id"))) .build(); diff --git a/src/main/java/com/conveyal/gtfs/loader/Feed.java b/src/main/java/com/conveyal/gtfs/loader/Feed.java index 1bfbf56c2..1fd8fce2d 100644 --- a/src/main/java/com/conveyal/gtfs/loader/Feed.java +++ b/src/main/java/com/conveyal/gtfs/loader/Feed.java @@ -30,7 +30,6 @@ public class Feed { // This may be the empty string if the feed is stored in the root ("public") schema. public final String databaseSchemaPrefix; - public final TableReader locationGroups; public final TableReader agencies; public final TableReader bookingRules; public final TableReader calendars; @@ -39,6 +38,7 @@ public class Feed { public final TableReader fareRules; public final TableReader frequencies; public final TableReader locations; + public final TableReader locationGroups; public final TableReader locationShapes; public final TableReader routes; public final TableReader locationGroupStops; @@ -57,13 +57,13 @@ public Feed (DataSource dataSource, String databaseSchemaPrefix) { // Ensure separator dot is present if (databaseSchemaPrefix != null && !databaseSchemaPrefix.endsWith(".")) databaseSchemaPrefix += "."; this.databaseSchemaPrefix = databaseSchemaPrefix == null ? "" : databaseSchemaPrefix; - locationGroups = new JDBCTableReader(Table.LOCATION_GROUP, dataSource, databaseSchemaPrefix, EntityPopulator.LOCATION_GROUPS); agencies = new JDBCTableReader(Table.AGENCY, dataSource, databaseSchemaPrefix, EntityPopulator.AGENCY); bookingRules = new JDBCTableReader(Table.BOOKING_RULES, dataSource, databaseSchemaPrefix, EntityPopulator.BOOKING_RULE); fareAttributes = new JDBCTableReader(Table.FARE_ATTRIBUTES, dataSource, databaseSchemaPrefix, EntityPopulator.FARE_ATTRIBUTE); fareRules = new JDBCTableReader(Table.FARE_RULES, dataSource, databaseSchemaPrefix, EntityPopulator.FARE_RULE); frequencies = new JDBCTableReader(Table.FREQUENCIES, dataSource, databaseSchemaPrefix, EntityPopulator.FREQUENCY); locations = new JDBCTableReader(Table.LOCATIONS, dataSource, databaseSchemaPrefix, EntityPopulator.LOCATION); + locationGroups = new JDBCTableReader(Table.LOCATION_GROUP, dataSource, databaseSchemaPrefix, EntityPopulator.LOCATION_GROUPS); locationShapes = new JDBCTableReader(Table.LOCATION_SHAPES, dataSource, databaseSchemaPrefix, EntityPopulator.LOCATION_SHAPES); calendars = new JDBCTableReader(Table.CALENDAR, dataSource, databaseSchemaPrefix, EntityPopulator.CALENDAR); calendarDates = new JDBCTableReader(Table.CALENDAR_DATES, dataSource, databaseSchemaPrefix, EntityPopulator.CALENDAR_DATE); diff --git a/src/main/java/com/conveyal/gtfs/loader/FeedLoadResult.java b/src/main/java/com/conveyal/gtfs/loader/FeedLoadResult.java index 5562d0edf..1250a466a 100644 --- a/src/main/java/com/conveyal/gtfs/loader/FeedLoadResult.java +++ b/src/main/java/com/conveyal/gtfs/loader/FeedLoadResult.java @@ -84,6 +84,7 @@ public boolean isGTFSFlex() { return (bookingRules != null && bookingRules.rowCount > 0) || (locations != null && locations.rowCount > 0) || + (locationGroup != null && locationGroup.rowCount > 0) || (locationGroupStops != null && locationGroupStops.rowCount > 0) || (locationShapes != null && locationShapes.rowCount > 0); } diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcGTFSFeedConverter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcGTFSFeedConverter.java index 872e642e3..a2b2c546e 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcGTFSFeedConverter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcGTFSFeedConverter.java @@ -107,7 +107,6 @@ public FeedLoadResult loadTables () { // Copy all tables (except for PATTERN_STOPS, which does not exist in GTFSFeed). copyEntityToSql(gtfsFeed.agency.values(), Table.AGENCY); - copyEntityToSql(gtfsFeed.locationGroup.values(), Table.LOCATION_GROUP); copyEntityToSql(gtfsFeed.bookingRules.values(), Table.BOOKING_RULES); copyEntityToSql(calendars, Table.CALENDAR); copyEntityToSql(calendarDates, Table.CALENDAR_DATES); @@ -125,6 +124,7 @@ public FeedLoadResult loadTables () { copyEntityToSql(frequencies, Table.FREQUENCIES); // refs trips copyEntityToSql(gtfsFeed.stop_times.values(), Table.STOP_TIMES); copyEntityToSql(gtfsFeed.locations.values(), Table.LOCATIONS); + copyEntityToSql(gtfsFeed.locationGroup.values(), Table.LOCATION_GROUP); copyEntityToSql(gtfsFeed.locationGroupStops.values(), Table.LOCATION_GROUP_STOPS); copyEntityToSql(gtfsFeed.locationShapes.values(), Table.LOCATION_SHAPES); // result.errorCount = errorStorage.getErrorCount(); diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java index 053bd1d96..544794d7b 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsLoader.java @@ -154,7 +154,6 @@ public FeedLoadResult loadTables() { // 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.locationGroup = load(Table.LOCATION_GROUP); result.calendar = load(Table.CALENDAR); result.calendarDates = load(Table.CALENDAR_DATES); result.routes = load(Table.ROUTES); @@ -168,8 +167,9 @@ public FeedLoadResult loadTables() { result.transfers = load(Table.TRANSFERS); // refs trips. result.frequencies = load(Table.FREQUENCIES); // refs trips result.locations = load(Table.LOCATIONS); + result.locationGroup = load(Table.LOCATION_GROUP); result.locationGroupStops = load(Table.LOCATION_GROUP_STOPS); // refs location groups. - result.stopTimes = load(Table.STOP_TIMES); // refs stop areas, locations and stops + result.stopTimes = load(Table.STOP_TIMES); // refs location groups, locations and stops result.translations = load(Table.TRANSLATIONS); result.attributions = load(Table.ATTRIBUTIONS); result.bookingRules = load(Table.BOOKING_RULES); diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsSnapshotter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsSnapshotter.java index c8cd7b0be..0df83eae5 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsSnapshotter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcGtfsSnapshotter.java @@ -93,7 +93,6 @@ public SnapshotResult copyTables() { // Copy each table in turn // FIXME: NO non-fatal exception errors are being captured during copy operations. result.agency = copy(Table.AGENCY, true); - result.locationGroup = copy(Table.LOCATION_GROUP, true); result.calendar = copy(Table.CALENDAR, true); result.bookingRules = copy(Table.BOOKING_RULES, true); result.calendarDates = copy(Table.CALENDAR_DATES, true); @@ -102,6 +101,7 @@ public SnapshotResult copyTables() { result.feedInfo = copy(Table.FEED_INFO, true); result.frequencies = copy(Table.FREQUENCIES, true); result.locations = copy(Table.LOCATIONS, true); + result.locationGroup = copy(Table.LOCATION_GROUP, true); result.locationGroupStops = copy(Table.LOCATION_GROUP_STOPS, true); result.locationShapes = copy(Table.LOCATION_SHAPES, true); result.routes = copy(Table.ROUTES, true); diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java index a07a1f2ec..c353599a3 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java @@ -652,7 +652,7 @@ private String updateChildTable( // If handling first iteration, create the prepared statement (later iterations will add to batch). insertStatement = createPreparedUpdate(id, true, subEntity, subTable, connection, true); } - if (isPatternTable && (Table.PATTERN_STOP.name.equals(subTable.name))) { + if (isPatternTable) { // Update linked stop times fields for each updated pattern stop (e.g., timepoint, pickup/drop off type). // These fields should be updated for all patterns (e.g., timepoint, pickup/drop off type). updateLinkedFields( diff --git a/src/main/java/com/conveyal/gtfs/loader/PatternReconciliation.java b/src/main/java/com/conveyal/gtfs/loader/PatternReconciliation.java index 82ac23943..f02490e6a 100644 --- a/src/main/java/com/conveyal/gtfs/loader/PatternReconciliation.java +++ b/src/main/java/com/conveyal/gtfs/loader/PatternReconciliation.java @@ -47,13 +47,6 @@ private enum ReconciliationOperation { ADD_ONE, ADD_MULTIPLE, DELETE, TRANSPOSE, NONE } - /** - * Enum containing available pattern types. - */ - public enum PatternType { - STOP - } - public PatternReconciliation(Connection connection, String tablePrefix) throws SQLException { this.connection = connection; this.tablePrefix = tablePrefix; @@ -498,10 +491,8 @@ static class GenericStop { public final String referenceId; // This stopTime object is a template that will be used to build database statements. StopTime stopTime; - PatternType patternType; public GenericStop(PatternStop patternStop) { - patternType = PatternType.STOP; referenceId = getReferenceId(patternStop.stop_id, patternStop.location_group_id, patternStop.location_id); stopTime = new StopTime(); stopTime.stop_id = patternStop.stop_id; diff --git a/src/main/java/com/conveyal/gtfs/loader/ReferenceTracker.java b/src/main/java/com/conveyal/gtfs/loader/ReferenceTracker.java index edff0819e..7f9137ab8 100644 --- a/src/main/java/com/conveyal/gtfs/loader/ReferenceTracker.java +++ b/src/main/java/com/conveyal/gtfs/loader/ReferenceTracker.java @@ -90,7 +90,7 @@ public Set checkReferencesAndUniqueness(String keyValue, int lineN for (Table referenceTable : field.referenceTables) { String referenceField = referenceTable.getKeyFieldName(); if (table.name.equals(Table.LOCATION_GROUP_STOPS.name) && field.name.equals("stop_id") && value.contains(",")) { - // Special case for stop areas as the area id can be an array of location and stop ids. + // Special case for location group stops as the stop id field can be an array of stop ids. for (String reference : value.split(",")) { if (checkReference(referenceField, reference, badValues)) { hasMatchingReference = true; diff --git a/src/main/java/com/conveyal/gtfs/validator/NewTripTimesValidator.java b/src/main/java/com/conveyal/gtfs/validator/NewTripTimesValidator.java index d796f37d9..de972df3a 100644 --- a/src/main/java/com/conveyal/gtfs/validator/NewTripTimesValidator.java +++ b/src/main/java/com/conveyal/gtfs/validator/NewTripTimesValidator.java @@ -176,13 +176,6 @@ private void processTrip (List stopTimes) { if (stopTime.location_id != null) { locations.add(locationById.get(stopTime.location_id)); } -// if (stop == null && location == null) { -// locationGroups.add(locationGroup); -// } else if (stop == null && locationGroup == null) { -// locations.add(location); -// } else { -// stops.add(stop); -// } } } diff --git a/src/main/java/com/conveyal/gtfs/validator/ServiceValidator.java b/src/main/java/com/conveyal/gtfs/validator/ServiceValidator.java index 6310c36cc..8c9e4864a 100644 --- a/src/main/java/com/conveyal/gtfs/validator/ServiceValidator.java +++ b/src/main/java/com/conveyal/gtfs/validator/ServiceValidator.java @@ -106,6 +106,11 @@ public void validateTrip( // ERR return; } + } else { + LOG.warn( + "Trip duration not calculated for trip id {} because it starts or ends with a flex stop.", + trip.trip_id + ); } // Get the map from modes to service durations in seconds for this trip's service ID. // Create a new empty map if it doesn't yet exist. diff --git a/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java b/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java index 36d24569b..115a814fe 100644 --- a/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java +++ b/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java @@ -7,8 +7,6 @@ import com.conveyal.gtfs.TestUtils.FileTestCase; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.File; import java.io.IOException; diff --git a/src/test/java/com/conveyal/gtfs/GTFSTest.java b/src/test/java/com/conveyal/gtfs/GTFSTest.java index c1308559a..b5cd97af7 100644 --- a/src/test/java/com/conveyal/gtfs/GTFSTest.java +++ b/src/test/java/com/conveyal/gtfs/GTFSTest.java @@ -307,7 +307,6 @@ 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.TABLE_IN_SUBDIRECTORY), new ErrorExpectation(NewGTFSErrorType.GEO_JSON_PARSING), new ErrorExpectation(NewGTFSErrorType.GEO_JSON_PARSING), new ErrorExpectation(NewGTFSErrorType.TABLE_IN_SUBDIRECTORY), @@ -316,6 +315,7 @@ 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.TABLE_IN_SUBDIRECTORY), new ErrorExpectation(NewGTFSErrorType.GEO_JSON_PARSING), new ErrorExpectation(NewGTFSErrorType.GEO_JSON_PARSING), new ErrorExpectation(NewGTFSErrorType.REFERENTIAL_INTEGRITY),