refactor(Updates and house keeping following review):

ibi-group · Jul 11, 2024 · 3b25aa1 · 3b25aa1
1 parent 7667a27
commit 3b25aa1
Show file tree

Hide file tree

Showing 13 changed files with 15 additions and 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
@@ -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
@@ -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<LocationGroup> locationGroups;
     public final TableReader<Agency> agencies;
     public final TableReader<BookingRule> bookingRules;
     public final TableReader<Calendar> calendars;
@@ -39,6 +38,7 @@ public class Feed {
     public final TableReader<FareRule> fareRules;
     public final TableReader<Frequency> frequencies;
     public final TableReader<Location> locations;
+    public final TableReader<LocationGroup> locationGroups;
     public final TableReader<LocationShape> locationShapes;
     public final TableReader<Route> routes;
     public final TableReader<LocationGroupStop> 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -90,7 +90,7 @@ public Set<NewGTFSError> 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
@@ -176,13 +176,6 @@ private void processTrip (List<StopTime> 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
@@ -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
@@ -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
@@ -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),