diff --git a/src/main/java/com/conveyal/gtfs/loader/Feed.java b/src/main/java/com/conveyal/gtfs/loader/Feed.java index 46b294b40..075600101 100644 --- 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 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 index b397d9958..c2e52cac7 100644 --- 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 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 new file mode 100644 index 000000000..26d74f841 --- /dev/null +++ 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 stopsForParentStations = HashMultimap.create(); + Set 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 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 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 index dca6a72bc..dbfb8a90e 100644 --- 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 index 49e996baa..a7bbda600 100755 --- 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,, \ No newline at end of file +1234567,,Unused stop,,37.06668,-122.07781,,,0,bad_stop_id_ref,, \ No newline at end of file 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 index d49e48ca4..743035e50 100644 --- 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,12 +40,20 @@ "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 }, { @@ -48,7 +61,7 @@ "entity_id" : null, "entity_sequence" : null, "entity_type" : null, - "error_id" : 2, + "error_id" : 3, "error_type" : "FEED_TRAVEL_TIMES_ROUNDED", "line_number" : null }, { @@ -56,7 +69,7 @@ "entity_id" : "1234567", "entity_sequence" : null, "entity_type" : "Stop", - "error_id" : 3, + "error_id" : 4, "error_type" : "STOP_UNUSED", "line_number" : 6 }, { @@ -64,7 +77,7 @@ "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/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchFeedRowCounts-0.json b/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchFeedRowCounts-0.json index 41e8dd151..4f2b230be 100644 --- a/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchFeedRowCounts-0.json +++ b/src/test/resources/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/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchStopWithChildren-0.json b/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchStopWithChildren-0.json index b1cc7007f..81ddf633f 100644 --- a/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchStopWithChildren-0.json +++ b/src/test/resources/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 index 9b1fd901c..0594014cd 100644 --- 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,