fix(validator): check stop#parent_station references

fix #274
conveyal · Feb 18, 2020 · 707636a · 707636a
1 parent 7779097
commit 707636a
Show file tree

Hide file tree

Showing 8 changed files with 79 additions and 10 deletions.
diff --git a/src/main/java/com/conveyal/gtfs/loader/Feed.java b/src/main/java/com/conveyal/gtfs/loader/Feed.java
@@ -91,6 +91,7 @@ public ValidationResult validate () {
         List<FeedValidator> feedValidators = Arrays.asList(
                 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/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
@@ -108,6 +108,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")),
@@ -219,6 +220,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),

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,