Merge pull request #238 from conveyal/stop_unused-fix

Stop unused fix
conveyal · Jun 3, 2019 · da0168e · da0168e
2 parents 7fe6c6d + 9815a28
commit da0168e
Show file tree

Hide file tree

Showing 19 changed files with 341 additions and 39 deletions.
diff --git a/src/main/java/com/conveyal/gtfs/loader/Table.java b/src/main/java/com/conveyal/gtfs/loader/Table.java
@@ -589,6 +589,14 @@ public String generateSelectSql (String namespace, Requirement minimumRequiremen
         return String.format("select %s from %s", fieldsString, tableName);
     }
 
+    /**
+     * Shorthand wrapper for calling {@link #generateSelectSql(String, Requirement)}. Note: this does not prefix field
+     * names with the namespace, so cannot serve as a replacement for {@link #generateSelectAllExistingFieldsSql}.
+     */
+    public String generateSelectAllSql (String namespace) {
+        return generateSelectSql(namespace, Requirement.PROPRIETARY);
+    }
+
     /**
      * Generate a select statement from the columns that actually exist in the database table.  This method is intended
      * to be used when exporting to a GTFS and eventually generates the select all with each individual field and

diff --git a/src/main/java/com/conveyal/gtfs/validator/ReferencesTripValidator.java b/src/main/java/com/conveyal/gtfs/validator/ReferencesTripValidator.java
@@ -15,6 +15,12 @@
 import static com.conveyal.gtfs.error.NewGTFSErrorType.*;
 
 /**
+ * This validator checks for unused entities in the dataset.
+ *
+ * It iterates over each trip and collects all of the route, trip, and stop IDs referenced by all of the trips found
+ * within the feed. In the completion stage of the validator it verifies that there are no stops, trips, or routes in
+ * the feed that do not actually get used by at least one trip.
+ *
  * Created by abyrd on 2017-04-18
  */
 public class ReferencesTripValidator extends TripValidator {
@@ -31,7 +37,13 @@ public ReferencesTripValidator(Feed feed, SQLErrorStorage errorStorage) {
     public void validateTrip(Trip trip, Route route, List<StopTime> stopTimes, List<Stop> stops) {
         if (trip != null) referencedTrips.add(trip.trip_id);
         if (route != null) referencedRoutes.add(route.route_id);
-        for (Stop stop : stops) referencedStops.add(stop.stop_id);
+        for (Stop stop : stops) {
+            referencedStops.add(stop.stop_id);
+            // If a stop used by the trip has a parent station, count this among the referenced stops, too. While the
+            // parent station may not be referenced directly, the relationship is functioning correctly and there is
+            // not an issue with this stop being unreferenced.
+            if (stop.parent_station != null) referencedStops.add(stop.parent_station);
+        }
     }
 
     @Override

diff --git a/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java b/src/test/java/com/conveyal/gtfs/GTFSFeedTest.java
@@ -253,7 +253,7 @@ public void canGetSpatialIndex() {
         assertThat(
             feed.getSpatialIndex().size(),
             // This should reflect the number of stops in src/test/resources/fake-agency/stops.txt
-            equalTo(4)
+            equalTo(5)
         );
     }
 

diff --git a/src/test/java/com/conveyal/gtfs/GTFSTest.java b/src/test/java/com/conveyal/gtfs/GTFSTest.java
@@ -102,8 +102,7 @@ public void canLoadAndExportSimpleAgency() {
             new ErrorExpectation(NewGTFSErrorType.MISSING_FIELD),
             new ErrorExpectation(NewGTFSErrorType.ROUTE_LONG_NAME_CONTAINS_SHORT_NAME),
             new ErrorExpectation(NewGTFSErrorType.FEED_TRAVEL_TIMES_ROUNDED),
-            new ErrorExpectation(NewGTFSErrorType.STOP_UNUSED),
-            new ErrorExpectation(NewGTFSErrorType.STOP_UNUSED),
+            new ErrorExpectation(NewGTFSErrorType.STOP_UNUSED, equalTo("1234567")),
             new ErrorExpectation(NewGTFSErrorType.DATE_NO_SERVICE)
         );
         assertThat(
@@ -215,7 +214,6 @@ public void canLoadAndExportSimpleAgencyInSubDirectory() {
             new ErrorExpectation(NewGTFSErrorType.ROUTE_LONG_NAME_CONTAINS_SHORT_NAME),
             new ErrorExpectation(NewGTFSErrorType.FEED_TRAVEL_TIMES_ROUNDED),
             new ErrorExpectation(NewGTFSErrorType.STOP_UNUSED),
-            new ErrorExpectation(NewGTFSErrorType.STOP_UNUSED),
             new ErrorExpectation(NewGTFSErrorType.DATE_NO_SERVICE)
         );
         assertThat(

diff --git a/src/test/resources/fake-agency/stop_times.txt b/src/test/resources/fake-agency/stop_times.txt
@@ -2,4 +2,4 @@ trip_id,arrival_time,departure_time,stop_id,stop_sequence,stop_headsign,pickup_t
 a30277f8-e50a-4a85-9141-b1e0da9d429d,07:00:00,07:00:00,4u6g,1,,0,0,0.0000000,
 a30277f8-e50a-4a85-9141-b1e0da9d429d,07:01:00,07:01:00,johv,2,,0,0,341.4491961,
 frequency-trip,08:00:00,08:00:00,4u6g,1,,0,0,0.0000000,
-frequency-trip,08:01:00,08:01:00,johv,2,,0,0,341.4491961,
+frequency-trip,08:29:00,08:29:00,1234,2,,0,0,341.4491961,
diff --git a/src/test/resources/fake-agency/stops.txt b/src/test/resources/fake-agency/stops.txt
@@ -2,4 +2,5 @@ stop_id,stop_code,stop_name,stop_desc,stop_lat,stop_lon,zone_id,stop_url,locatio
 4u6g,,Butler Ln,,37.0612132,-122.0074332,,,0,,,
 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,,
+1234,,Child Stop,,37.06662,-122.07772,,,0,123,,
+1234567,,Unused stop,,37.06668,-122.07781,,,0,123,,
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
@@ -26,27 +26,19 @@
         "error_type" : "FEED_TRAVEL_TIMES_ROUNDED",
         "line_number" : null
       }, {
-        "bad_value" : "123",
-        "entity_id" : "123",
+        "bad_value" : "1234567",
+        "entity_id" : "1234567",
         "entity_sequence" : null,
         "entity_type" : "Stop",
         "error_id" : 3,
         "error_type" : "STOP_UNUSED",
-        "line_number" : 4
-      }, {
-        "bad_value" : "1234",
-        "entity_id" : "1234",
-        "entity_sequence" : null,
-        "entity_type" : "Stop",
-        "error_id" : 4,
-        "error_type" : "STOP_UNUSED",
-        "line_number" : 5
+        "line_number" : 6
       }, {
         "bad_value" : "20170916",
         "entity_id" : null,
         "entity_sequence" : null,
         "entity_type" : null,
-        "error_id" : 5,
+        "error_id" : 4,
         "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,10 +6,10 @@
         "agency" : 1,
         "calendar" : 1,
         "calendar_dates" : 1,
-        "errors" : 6,
+        "errors" : 5,
         "routes" : 1,
         "stop_times" : 4,
-        "stops" : 4,
+        "stops" : 5,
         "trips" : 2
       },
       "snapshot_of" : null

diff --git a/...es/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchMultiNestedEntities-0.json b/...es/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchMultiNestedEntities-0.json
@@ -14,6 +14,8 @@
                   "stop_id" : "4u6g"
                 }, {
                   "stop_id" : "johv"
+                }, {
+                  "stop_id" : "1234"
                 } ]
               } ],
               "stop_id" : "4u6g"
@@ -24,9 +26,23 @@
                   "stop_id" : "4u6g"
                 }, {
                   "stop_id" : "johv"
+                }, {
+                  "stop_id" : "1234"
                 } ]
               } ],
               "stop_id" : "johv"
+            }, {
+              "routes" : [ {
+                "route_id" : "1",
+                "stops" : [ {
+                  "stop_id" : "4u6g"
+                }, {
+                  "stop_id" : "johv"
+                }, {
+                  "stop_id" : "1234"
+                } ]
+              } ],
+              "stop_id" : "1234"
             } ]
           } ],
           "stop_id" : "4u6g"
@@ -40,6 +56,8 @@
                   "stop_id" : "4u6g"
                 }, {
                   "stop_id" : "johv"
+                }, {
+                  "stop_id" : "1234"
                 } ]
               } ],
               "stop_id" : "4u6g"
@@ -50,12 +68,68 @@
                   "stop_id" : "4u6g"
                 }, {
                   "stop_id" : "johv"
+                }, {
+                  "stop_id" : "1234"
                 } ]
               } ],
               "stop_id" : "johv"
+            }, {
+              "routes" : [ {
+                "route_id" : "1",
+                "stops" : [ {
+                  "stop_id" : "4u6g"
+                }, {
+                  "stop_id" : "johv"
+                }, {
+                  "stop_id" : "1234"
+                } ]
+              } ],
+              "stop_id" : "1234"
             } ]
           } ],
           "stop_id" : "johv"
+        }, {
+          "routes" : [ {
+            "route_id" : "1",
+            "stops" : [ {
+              "routes" : [ {
+                "route_id" : "1",
+                "stops" : [ {
+                  "stop_id" : "4u6g"
+                }, {
+                  "stop_id" : "johv"
+                }, {
+                  "stop_id" : "1234"
+                } ]
+              } ],
+              "stop_id" : "4u6g"
+            }, {
+              "routes" : [ {
+                "route_id" : "1",
+                "stops" : [ {
+                  "stop_id" : "4u6g"
+                }, {
+                  "stop_id" : "johv"
+                }, {
+                  "stop_id" : "1234"
+                } ]
+              } ],
+              "stop_id" : "johv"
+            }, {
+              "routes" : [ {
+                "route_id" : "1",
+                "stops" : [ {
+                  "stop_id" : "4u6g"
+                }, {
+                  "stop_id" : "johv"
+                }, {
+                  "stop_id" : "1234"
+                } ]
+              } ],
+              "stop_id" : "1234"
+            } ]
+          } ],
+          "stop_id" : "1234"
         } ]
       } ]
     }

diff --git a/...com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchMultiNestedEntitiesWithoutLimits-0.json b/...com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchMultiNestedEntitiesWithoutLimits-0.json
@@ -14,6 +14,8 @@
                   "stop_id" : "4u6g"
                 }, {
                   "stop_id" : "johv"
+                }, {
+                  "stop_id" : "1234"
                 } ]
               } ],
               "stop_id" : "4u6g"
@@ -24,9 +26,23 @@
                   "stop_id" : "4u6g"
                 }, {
                   "stop_id" : "johv"
+                }, {
+                  "stop_id" : "1234"
                 } ]
               } ],
               "stop_id" : "johv"
+            }, {
+              "routes" : [ {
+                "route_id" : "1",
+                "stops" : [ {
+                  "stop_id" : "4u6g"
+                }, {
+                  "stop_id" : "johv"
+                }, {
+                  "stop_id" : "1234"
+                } ]
+              } ],
+              "stop_id" : "1234"
             } ]
           } ],
           "stop_id" : "4u6g"
@@ -40,6 +56,8 @@
                   "stop_id" : "4u6g"
                 }, {
                   "stop_id" : "johv"
+                }, {
+                  "stop_id" : "1234"
                 } ]
               } ],
               "stop_id" : "4u6g"
@@ -50,12 +68,68 @@
                   "stop_id" : "4u6g"
                 }, {
                   "stop_id" : "johv"
+                }, {
+                  "stop_id" : "1234"
                 } ]
               } ],
               "stop_id" : "johv"
+            }, {
+              "routes" : [ {
+                "route_id" : "1",
+                "stops" : [ {
+                  "stop_id" : "4u6g"
+                }, {
+                  "stop_id" : "johv"
+                }, {
+                  "stop_id" : "1234"
+                } ]
+              } ],
+              "stop_id" : "1234"
             } ]
           } ],
           "stop_id" : "johv"
+        }, {
+          "routes" : [ {
+            "route_id" : "1",
+            "stops" : [ {
+              "routes" : [ {
+                "route_id" : "1",
+                "stops" : [ {
+                  "stop_id" : "4u6g"
+                }, {
+                  "stop_id" : "johv"
+                }, {
+                  "stop_id" : "1234"
+                } ]
+              } ],
+              "stop_id" : "4u6g"
+            }, {
+              "routes" : [ {
+                "route_id" : "1",
+                "stops" : [ {
+                  "stop_id" : "4u6g"
+                }, {
+                  "stop_id" : "johv"
+                }, {
+                  "stop_id" : "1234"
+                } ]
+              } ],
+              "stop_id" : "johv"
+            }, {
+              "routes" : [ {
+                "route_id" : "1",
+                "stops" : [ {
+                  "stop_id" : "4u6g"
+                }, {
+                  "stop_id" : "johv"
+                }, {
+                  "stop_id" : "1234"
+                } ]
+              } ],
+              "stop_id" : "1234"
+            } ]
+          } ],
+          "stop_id" : "1234"
         } ]
       } ]
     }

diff --git a/.../snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchNestedEntityWithLimit-0.json b/.../snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchNestedEntityWithLimit-0.json
@@ -21,6 +21,13 @@
         "stop_times" : [ ]
       }, {
         "stop_id" : "1234",
+        "stop_times" : [ {
+          "stop_id" : "1234",
+          "stop_sequence" : 2,
+          "trip_id" : "frequency-trip"
+        } ]
+      }, {
+        "stop_id" : "1234567",
         "stop_times" : [ ]
       } ]
     }