Merge pull request #342 from conveyal/dev

Fix release January 7, 2022
conveyal · Feb 7, 2022 · 3a579fe · 3a579fe
2 parents 88dd9ea + f840040
commit 3a579fe
Show file tree

Hide file tree

Showing 9 changed files with 87 additions and 11 deletions.
diff --git a/src/main/java/com/conveyal/gtfs/loader/Feed.java b/src/main/java/com/conveyal/gtfs/loader/Feed.java
@@ -87,6 +87,7 @@ public ValidationResult validate (FeedValidatorCreator... additionalValidators)
         List<FeedValidator> feedValidators = Lists.newArrayList(
             new MisplacedStopValidator(this, errorStorage, validationResult),
             new DuplicateStopsValidator(this, errorStorage),
+            new ParentStationValidator(this, errorStorage),
             new FaresValidator(this, errorStorage),
             new FrequencyValidator(this, errorStorage),
             new TimeZoneValidator(this, errorStorage),

diff --git a/src/main/java/com/conveyal/gtfs/loader/Table.java b/src/main/java/com/conveyal/gtfs/loader/Table.java
@@ -348,7 +348,7 @@ public Table (String name, Class<? extends Entity> entityClass, Requirement requ
             new ShortField("drop_off_type", OPTIONAL, 3),
             new ShortField("continuous_pickup", OPTIONAL, 3),
             new ShortField("continuous_drop_off", OPTIONAL, 3),
-            new DoubleField("shape_dist_traveled", OPTIONAL, 0, Double.POSITIVE_INFINITY, 2),
+            new DoubleField("shape_dist_traveled", OPTIONAL, 0, Double.POSITIVE_INFINITY, -1),
             new ShortField("timepoint", OPTIONAL, 1),
             new IntegerField("fare_units_traveled", EXTENSION) // OpenOV NL extension
     ).withParentTable(TRIPS);

diff --git a/src/main/java/com/conveyal/gtfs/validator/ParentStationValidator.java b/src/main/java/com/conveyal/gtfs/validator/ParentStationValidator.java
@@ -0,0 +1,56 @@
+package com.conveyal.gtfs.validator;
+
+import com.conveyal.gtfs.error.NewGTFSError;
+import com.conveyal.gtfs.error.SQLErrorStorage;
+import com.conveyal.gtfs.loader.Feed;
+import com.conveyal.gtfs.loader.Table;
+import com.conveyal.gtfs.model.Stop;
+import com.google.common.collect.HashMultimap;
+import com.google.common.collect.Multimap;
+import com.google.common.collect.Sets;
+
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Set;
+
+import static com.conveyal.gtfs.error.NewGTFSErrorType.REFERENTIAL_INTEGRITY;
+
+/**
+ * Find stop#parent_station values that reference non-existent stop_ids. Unfortunately, we cannot perform this check
+ * while the stops.txt table is being loaded because we do not yet have the full set of stop_ids available to check
+ * parent_station values against.
+ */
+public class ParentStationValidator extends FeedValidator {
+
+    public ParentStationValidator(Feed feed, SQLErrorStorage errorStorage) {
+        super(feed, errorStorage);
+    }
+
+    @Override
+    public void validate () {
+        Multimap<String, Stop> stopsForParentStations = HashMultimap.create();
+        Set<String> stopIds = new HashSet<>();
+        for (Stop stop : feed.stops) {
+            // Collect all stop_ids found in feed.
+            stopIds.add(stop.stop_id);
+            // Collect all non-null parent_station values.
+            if (stop.parent_station != null) {
+                stopsForParentStations.put(stop.parent_station, stop);
+            }
+        }
+        // Find parent_station values that do not reference a valid stop_id from feed.
+        Sets.SetView<String> badReferences = Sets.difference(stopsForParentStations.keySet(), stopIds);
+        for (String parentStation : badReferences) {
+            // For any bad parent_station ref (this could be more than one stop), add an error to the error storage.
+            Collection<Stop> stops = stopsForParentStations.get(parentStation);
+            for (Stop stop : stops) {
+                registerError(
+                    NewGTFSError
+                        .forLine(Table.STOPS, stop.id, REFERENTIAL_INTEGRITY, parentStation)
+                        .setEntityId(stop.stop_id)
+                );
+            }
+        }
+    }
+
+}
diff --git a/src/test/java/com/conveyal/gtfs/GTFSTest.java b/src/test/java/com/conveyal/gtfs/GTFSTest.java
@@ -106,6 +106,7 @@ public void requiresActionCommand() throws Exception {
     public void canLoadAndExportSimpleAgency() {
         ErrorExpectation[] fakeAgencyErrorExpectations = ErrorExpectation.list(
             new ErrorExpectation(NewGTFSErrorType.MISSING_FIELD),
+            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")),
@@ -226,6 +227,7 @@ 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.ROUTE_LONG_NAME_CONTAINS_SHORT_NAME),
             new ErrorExpectation(NewGTFSErrorType.FEED_TRAVEL_TIMES_ROUNDED),
             new ErrorExpectation(NewGTFSErrorType.STOP_UNUSED),
@@ -276,6 +278,13 @@ public void canLoadAndExportSimpleAgencyWithOnlyCalendarDates() {
                     new RecordExpectation("shape_dist_traveled", 0.0, 0.01)
                 }
             ),
+            // Check that the shape_dist_traveled values in stop_times are not rounded.
+            new PersistenceExpectation(
+                "stop_times",
+                new RecordExpectation[]{
+                    new RecordExpectation("shape_dist_traveled", 341.4491961, 0.00001)
+                }
+            ),
             new PersistenceExpectation(
                 "trips",
                 new RecordExpectation[]{

diff --git a/src/test/resources/fake-agency/stops.txt b/src/test/resources/fake-agency/stops.txt
@@ -3,4 +3,4 @@ stop_id,stop_code,stop_name,stop_desc,stop_lat,stop_lon,zone_id,stop_url,locatio
 johv,,Scotts Valley Dr & Victor Sq,,37.0590172,-122.0096058,,,0,,,
 123,,Parent Station,,37.0666,-122.0777,,,1,,,
 1234,,Child Stop,,37.06662,-122.07772,,,0,123,,
-1234567,,Unused stop,,37.06668,-122.07781,,,0,123,,
+1234567,,Unused stop,,37.06668,-122.07781,,,0,bad_stop_id_ref,,
diff --git a/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchErrors-0.json b/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchErrors-0.json
@@ -16,6 +16,11 @@
         "message" : "A required field was missing or empty in a particular row.",
         "priority" : "MEDIUM",
         "type" : "MISSING_FIELD"
+      }, {
+        "count" : 1,
+        "message" : "This line references an ID that does not exist in the target table.",
+        "priority" : "HIGH",
+        "type" : "REFERENTIAL_INTEGRITY"
       }, {
         "count" : 1,
         "message" : "The long name of a route should complement the short name, not include it.",
@@ -35,36 +40,44 @@
         "error_id" : 0,
         "error_type" : "MISSING_FIELD",
         "line_number" : 2
+      }, {
+        "bad_value" : "bad_stop_id_ref",
+        "entity_id" : "1234567",
+        "entity_sequence" : null,
+        "entity_type" : "Stop",
+        "error_id" : 1,
+        "error_type" : "REFERENTIAL_INTEGRITY",
+        "line_number" : 6
       }, {
         "bad_value" : "route 1",
         "entity_id" : "1",
         "entity_sequence" : null,
         "entity_type" : "Route",
-        "error_id" : 1,
+        "error_id" : 2,
         "error_type" : "ROUTE_LONG_NAME_CONTAINS_SHORT_NAME",
         "line_number" : 2
       }, {
         "bad_value" : null,
         "entity_id" : null,
         "entity_sequence" : null,
         "entity_type" : null,
-        "error_id" : 2,
+        "error_id" : 3,
         "error_type" : "FEED_TRAVEL_TIMES_ROUNDED",
         "line_number" : null
       }, {
         "bad_value" : "1234567",
         "entity_id" : "1234567",
         "entity_sequence" : null,
         "entity_type" : "Stop",
-        "error_id" : 3,
+        "error_id" : 4,
         "error_type" : "STOP_UNUSED",
         "line_number" : 6
       }, {
         "bad_value" : "20170916",
         "entity_id" : null,
         "entity_sequence" : null,
         "entity_type" : null,
-        "error_id" : 4,
+        "error_id" : 5,
         "error_type" : "DATE_NO_SERVICE",
         "line_number" : null
       } ],

diff --git a/...esources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchFeedRowCounts-0.json b/...esources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchFeedRowCounts-0.json
@@ -6,7 +6,7 @@
         "agency" : 1,
         "calendar" : 1,
         "calendar_dates" : 1,
-        "errors" : 5,
+        "errors" : 6,
         "routes" : 1,
         "stop_times" : 4,
         "stops" : 5,

diff --git a/...urces/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchStopWithChildren-0.json b/...urces/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchStopWithChildren-0.json
@@ -14,9 +14,6 @@
         "child_stops" : [ {
           "stop_id" : "1234",
           "stop_name" : "Child Stop"
-        }, {
-          "stop_id" : "1234567",
-          "stop_name" : "Unused stop"
         } ],
         "stop_id" : "123",
         "stop_name" : "Parent Station"

diff --git a/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchStops-0.json b/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchStops-0.json
@@ -107,7 +107,7 @@
       }, {
         "id" : 6,
         "location_type" : 0,
-        "parent_station" : "123",
+        "parent_station" : "bad_stop_id_ref",
         "patterns" : [ ],
         "routes" : [ ],
         "stop_code" : null,