diff --git a/pom.xml b/pom.xml index d7b386493..1196adf3b 100644 --- a/pom.xml +++ b/pom.xml @@ -357,5 +357,11 @@ graphql-java 11.0 + + + org.apache.commons + commons-text + 1.6 + diff --git a/src/main/java/com/conveyal/gtfs/error/NewGTFSErrorType.java b/src/main/java/com/conveyal/gtfs/error/NewGTFSErrorType.java index d0a900030..88911627d 100644 --- a/src/main/java/com/conveyal/gtfs/error/NewGTFSErrorType.java +++ b/src/main/java/com/conveyal/gtfs/error/NewGTFSErrorType.java @@ -12,6 +12,8 @@ public enum NewGTFSErrorType { LANGUAGE_FORMAT(Priority.LOW, "Language should be specified with a valid BCP47 tag."), ILLEGAL_FIELD_VALUE(Priority.MEDIUM, "Fields may not contain tabs, carriage returns or new lines."), INTEGER_FORMAT(Priority.MEDIUM, "Incorrect integer format."), + FARE_TRANSFER_MISMATCH(Priority.MEDIUM, "A fare that does not permit transfers has a non-zero transfer duration."), + FREQUENCY_PERIOD_OVERLAP(Priority.MEDIUM, "A frequency for a trip overlaps with another frequency defined for the same trip."), FLOATING_FORMAT(Priority.MEDIUM, "Incorrect floating point number format."), COLUMN_NAME_UNSAFE(Priority.HIGH, "Column header contains characters not safe in SQL, it was renamed."), NUMBER_PARSING(Priority.MEDIUM, "Unable to parse number from value."), @@ -35,7 +37,7 @@ public enum NewGTFSErrorType { TABLE_IN_SUBDIRECTORY(Priority.HIGH, "Rather than being at the root of the zip file, a table was nested in a subdirectory."), TABLE_MISSING_COLUMN_HEADERS(Priority.HIGH, "Table is missing column headers."), TABLE_TOO_LONG(Priority.MEDIUM, "Table is too long to record line numbers with a 32-bit integer, overflow will occur."), - TIME_ZONE_FORMAT(Priority.MEDIUM, "Time zone format should be X."), + TIME_ZONE_FORMAT(Priority.MEDIUM, "Time zone format should match value from the Time Zone Database https://en.wikipedia.org/wiki/List_of_tz_database_time_zones."), REQUIRED_TABLE_EMPTY(Priority.MEDIUM, "This table is required by the GTFS specification but is empty."), FEED_TRAVEL_TIMES_ROUNDED(Priority.LOW, "All travel times in the feed are rounded to the minute, which may cause unexpected results in routing applications where travel times are zero."), ROUTE_DESCRIPTION_SAME_AS_NAME(Priority.LOW, "The description of a route is identical to its name, so does not add any information."), @@ -49,7 +51,9 @@ public enum NewGTFSErrorType { STOP_LOW_POPULATION_DENSITY(Priority.HIGH, "A stop is located in a geographic area with very low human population density."), STOP_NAME_MISSING(Priority.MEDIUM, "A stop does not have a name."), STOP_GEOGRAPHIC_OUTLIER(Priority.HIGH, "This stop is located very far from the middle 90% of stops in this feed."), + STOP_TIME_UNUSED(Priority.LOW, "This stop time allows neither pickup nor drop off and is not a timepoint, so it serves no purpose and should be removed from trip."), STOP_UNUSED(Priority.MEDIUM, "This stop is not referenced by any trips."), + TIMEPOINT_MISSING_TIMES(Priority.MEDIUM, "This stop time is marked as a timepoint, but is missing both arrival and departure times."), TRIP_EMPTY(Priority.HIGH, "This trip is defined but has no stop times."), TRIP_HEADSIGN_CONTAINS_ROUTE_NAME(Priority.LOW, "A trip headsign contains the route name, but should only contain information to distinguish it from other trips for the route."), TRIP_HEADSIGN_SHOULD_DESCRIBE_DESTINATION_OR_WAYPOINTS(Priority.LOW, "A trip headsign begins with 'to' or 'towards', but should begin with destination or direction and optionally include waypoints with 'via'"), diff --git a/src/main/java/com/conveyal/gtfs/loader/EntityPopulator.java b/src/main/java/com/conveyal/gtfs/loader/EntityPopulator.java index 4d6124509..762176262 100644 --- a/src/main/java/com/conveyal/gtfs/loader/EntityPopulator.java +++ b/src/main/java/com/conveyal/gtfs/loader/EntityPopulator.java @@ -4,6 +4,8 @@ import com.conveyal.gtfs.model.Calendar; import com.conveyal.gtfs.model.CalendarDate; import com.conveyal.gtfs.model.Entity; +import com.conveyal.gtfs.model.FareAttribute; +import com.conveyal.gtfs.model.Frequency; import com.conveyal.gtfs.model.PatternStop; import com.conveyal.gtfs.model.Route; import com.conveyal.gtfs.model.ScheduleException; @@ -68,21 +70,21 @@ public interface EntityPopulator { T populate (ResultSet results, TObjectIntMap columnForName) throws SQLException; EntityPopulator AGENCY = (result, columnForName) -> { - Agency agency = new Agency(); - agency.agency_id = getStringIfPresent(result, "agency_id", columnForName); - agency.agency_name = getStringIfPresent(result, "agency_name", columnForName); - agency.agency_url = getUrlIfPresent (result, "agency_url", columnForName); - agency.agency_timezone = getStringIfPresent(result, "agency_timezone", columnForName); - agency.agency_lang = getStringIfPresent(result, "agency_lang", columnForName); - agency.agency_phone = getStringIfPresent(result, "agency_phone", columnForName); - agency.agency_fare_url = getUrlIfPresent (result, "agency_fare_url", columnForName); - agency.agency_email = getStringIfPresent(result, "agency_email", columnForName); - agency.agency_branding_url = getUrlIfPresent (result, "agency_branding_url", columnForName); + Agency agency = new Agency(); + agency.agency_id = getStringIfPresent(result, "agency_id", columnForName); + agency.agency_name = getStringIfPresent(result, "agency_name", columnForName); + agency.agency_url = getUrlIfPresent (result, "agency_url", columnForName); + agency.agency_timezone = getStringIfPresent(result, "agency_timezone", columnForName); + agency.agency_lang = getStringIfPresent(result, "agency_lang", columnForName); + agency.agency_phone = getStringIfPresent(result, "agency_phone", columnForName); + agency.agency_fare_url = getUrlIfPresent (result, "agency_fare_url", columnForName); + agency.agency_email = getStringIfPresent(result, "agency_email", columnForName); + agency.agency_branding_url = getUrlIfPresent (result, "agency_branding_url", columnForName); return agency; }; EntityPopulator CALENDAR = (result, columnForName) -> { - Calendar calendar = new Calendar(); + Calendar calendar = new Calendar(); calendar.service_id = getStringIfPresent(result, "service_id", columnForName); calendar.start_date = getDateIfPresent (result, "start_date", columnForName); calendar.end_date = getDateIfPresent (result, "end_date", columnForName); @@ -104,10 +106,31 @@ public interface EntityPopulator { return calendarDate; }; + EntityPopulator FARE_ATTRIBUTE = (result, columnForName) -> { + FareAttribute fareAttribute = new FareAttribute(); + fareAttribute.fare_id = getStringIfPresent(result, "fare_id", columnForName); + fareAttribute.agency_id = getStringIfPresent(result, "agency_id", columnForName); + fareAttribute.price = getDoubleIfPresent(result, "price", columnForName); + fareAttribute.payment_method = getIntIfPresent (result, "payment_method", columnForName); + fareAttribute.transfers = getIntIfPresent (result, "transfers", columnForName); + fareAttribute.transfer_duration = getIntIfPresent (result, "transfer_duration", columnForName); + return fareAttribute; + }; + + EntityPopulator FREQUENCY = (result, columnForName) -> { + Frequency frequency = new Frequency(); + frequency.trip_id = getStringIfPresent(result, "trip_id", columnForName); + frequency.start_time = getIntIfPresent (result, "start_time", columnForName); + frequency.end_time = getIntIfPresent (result, "end_time", columnForName); + frequency.headway_secs = getIntIfPresent (result, "headway_secs", columnForName); + frequency.exact_times = getIntIfPresent (result, "exact_times", columnForName); + return frequency; + }; + EntityPopulator SCHEDULE_EXCEPTION = (result, columnForName) -> { ScheduleException scheduleException = new ScheduleException(); - scheduleException.name = getStringIfPresent(result, "name", columnForName); - scheduleException.dates = getDateListIfPresent(result, "dates", columnForName); + scheduleException.name = getStringIfPresent (result, "name", columnForName); + scheduleException.dates = getDateListIfPresent (result, "dates", columnForName); scheduleException.exemplar = exemplarFromInt(getIntIfPresent(result, "exemplar", columnForName)); scheduleException.customSchedule = getStringListIfPresent(result, "custom_schedule", columnForName); scheduleException.addedService = getStringListIfPresent(result, "added_service", columnForName); @@ -116,22 +139,22 @@ public interface EntityPopulator { }; EntityPopulator ROUTE = (result, columnForName) -> { - Route route = new Route(); - route.route_id = getStringIfPresent(result, "route_id", columnForName); - route.agency_id = getStringIfPresent(result, "agency_id", columnForName); - route.route_short_name = getStringIfPresent(result, "route_short_name", columnForName); - route.route_long_name = getStringIfPresent(result, "route_long_name", columnForName); - route.route_desc = getStringIfPresent(result, "route_desc", columnForName); - route.route_type = getIntIfPresent (result, "route_type", columnForName); - route.route_color = getStringIfPresent(result, "route_color", columnForName); - route.route_text_color = getStringIfPresent(result, "route_text_color", columnForName); - route.route_url = getUrlIfPresent (result, "route_url", columnForName); - route.route_branding_url = getUrlIfPresent (result, "route_branding_url", columnForName); + Route route = new Route(); + route.route_id = getStringIfPresent(result, "route_id", columnForName); + route.agency_id = getStringIfPresent(result, "agency_id", columnForName); + route.route_short_name = getStringIfPresent(result, "route_short_name", columnForName); + route.route_long_name = getStringIfPresent(result, "route_long_name", columnForName); + route.route_desc = getStringIfPresent(result, "route_desc", columnForName); + route.route_type = getIntIfPresent (result, "route_type", columnForName); + route.route_color = getStringIfPresent(result, "route_color", columnForName); + route.route_text_color = getStringIfPresent(result, "route_text_color", columnForName); + route.route_url = getUrlIfPresent (result, "route_url", columnForName); + route.route_branding_url = getUrlIfPresent (result, "route_branding_url", columnForName); return route; }; EntityPopulator STOP = (result, columnForName) -> { - Stop stop = new Stop(); + Stop stop = new Stop(); stop.stop_id = getStringIfPresent(result, "stop_id", columnForName); stop.stop_code = getStringIfPresent(result, "stop_code", columnForName); stop.stop_name = getStringIfPresent(result, "stop_name", columnForName); @@ -148,7 +171,7 @@ public interface EntityPopulator { }; EntityPopulator TRIP = (result, columnForName) -> { - Trip trip = new Trip(); + Trip trip = new Trip(); trip.trip_id = getStringIfPresent(result, "trip_id", columnForName); trip.route_id = getStringIfPresent(result, "route_id", columnForName); trip.service_id = getStringIfPresent(result, "service_id", columnForName); @@ -163,26 +186,26 @@ public interface EntityPopulator { }; EntityPopulator SHAPE_POINT = (result, columnForName) -> { - ShapePoint shapePoint = new ShapePoint(); - shapePoint.shape_id = getStringIfPresent(result, "shape_id", columnForName); - shapePoint.shape_pt_lat = getDoubleIfPresent(result, "shape_pt_lat", columnForName); - shapePoint.shape_pt_lon = getDoubleIfPresent(result, "shape_pt_lon", columnForName); - shapePoint.shape_pt_sequence = getIntIfPresent(result, "shape_pt_sequence", columnForName); + ShapePoint shapePoint = new ShapePoint(); + shapePoint.shape_id = getStringIfPresent(result, "shape_id", columnForName); + shapePoint.shape_pt_lat = getDoubleIfPresent(result, "shape_pt_lat", columnForName); + shapePoint.shape_pt_lon = getDoubleIfPresent(result, "shape_pt_lon", columnForName); + shapePoint.shape_pt_sequence = getIntIfPresent (result, "shape_pt_sequence", columnForName); shapePoint.shape_dist_traveled = getDoubleIfPresent(result, "shape_dist_traveled", columnForName); return shapePoint; }; EntityPopulator STOP_TIME = (result, columnForName) -> { - StopTime stopTime = new StopTime(); - stopTime.trip_id = getStringIfPresent(result, "trip_id", columnForName); - stopTime.arrival_time = getIntIfPresent (result, "arrival_time", columnForName); - stopTime.departure_time = getIntIfPresent (result, "departure_time", columnForName); - stopTime.stop_id = getStringIfPresent(result, "stop_id", columnForName); - stopTime.stop_sequence = getIntIfPresent (result, "stop_sequence", columnForName); - stopTime.stop_headsign = getStringIfPresent(result, "stop_headsign", columnForName); - stopTime.pickup_type = getIntIfPresent (result, "pickup_type", columnForName); - stopTime.drop_off_type = getIntIfPresent (result, "drop_off_type", columnForName); - stopTime.timepoint = getIntIfPresent (result, "timepoint", columnForName); + StopTime stopTime = new StopTime(); + stopTime.trip_id = getStringIfPresent(result, "trip_id", columnForName); + stopTime.arrival_time = getIntIfPresent (result, "arrival_time", columnForName); + stopTime.departure_time = getIntIfPresent (result, "departure_time", columnForName); + stopTime.stop_id = getStringIfPresent(result, "stop_id", columnForName); + stopTime.stop_sequence = getIntIfPresent (result, "stop_sequence", columnForName); + stopTime.stop_headsign = getStringIfPresent(result, "stop_headsign", columnForName); + stopTime.pickup_type = getIntIfPresent (result, "pickup_type", columnForName); + stopTime.drop_off_type = getIntIfPresent (result, "drop_off_type", columnForName); + stopTime.timepoint = getIntIfPresent (result, "timepoint", columnForName); stopTime.shape_dist_traveled = getDoubleIfPresent(result, "shape_dist_traveled", columnForName); return stopTime; }; diff --git a/src/main/java/com/conveyal/gtfs/loader/Feed.java b/src/main/java/com/conveyal/gtfs/loader/Feed.java index bb896196f..475f1e707 100644 --- a/src/main/java/com/conveyal/gtfs/loader/Feed.java +++ b/src/main/java/com/conveyal/gtfs/loader/Feed.java @@ -35,7 +35,8 @@ public class Feed { public final TableReader agencies; public final TableReader calendars; public final TableReader calendarDates; -// public final TableReader fares; + public final TableReader fareAttributes; + public final TableReader frequencies; public final TableReader routes; public final TableReader stops; public final TableReader trips; @@ -57,7 +58,8 @@ public Feed (DataSource dataSource, String tablePrefix) { if (tablePrefix != null && !tablePrefix.endsWith(".")) tablePrefix += "."; this.tablePrefix = tablePrefix == null ? "" : tablePrefix; agencies = new JDBCTableReader(Table.AGENCY, dataSource, tablePrefix, EntityPopulator.AGENCY); -// fares = new JDBCTableReader(Table.FARES, dataSource, tablePrefix, EntityPopulator.FARE); + fareAttributes = new JDBCTableReader(Table.FARE_ATTRIBUTES, dataSource, tablePrefix, EntityPopulator.FARE_ATTRIBUTE); + frequencies = new JDBCTableReader(Table.FREQUENCIES, dataSource, tablePrefix, EntityPopulator.FREQUENCY); calendars = new JDBCTableReader(Table.CALENDAR, dataSource, tablePrefix, EntityPopulator.CALENDAR); calendarDates = new JDBCTableReader(Table.CALENDAR_DATES, dataSource, tablePrefix, EntityPopulator.CALENDAR_DATE); routes = new JDBCTableReader(Table.ROUTES, dataSource, tablePrefix, EntityPopulator.ROUTE); @@ -89,6 +91,8 @@ public ValidationResult validate () { List feedValidators = Arrays.asList( new MisplacedStopValidator(this, errorStorage, validationResult), new DuplicateStopsValidator(this, errorStorage), + new FaresValidator(this, errorStorage), + new FrequencyValidator(this, errorStorage), new TimeZoneValidator(this, errorStorage), new NewTripTimesValidator(this, errorStorage), new NamesValidator(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 4866640d7..c63a85bf9 100644 --- a/src/main/java/com/conveyal/gtfs/loader/Table.java +++ b/src/main/java/com/conveyal/gtfs/loader/Table.java @@ -349,7 +349,7 @@ public Table keyFieldIsNotUnique() { } /** Fluent method to set whether the table has a compound key, e.g., transfers#to_stop_id. */ - private Table hasCompoundKey() { + public Table hasCompoundKey() { this.compoundKey = true; return this; } diff --git a/src/main/java/com/conveyal/gtfs/validator/FaresValidator.java b/src/main/java/com/conveyal/gtfs/validator/FaresValidator.java new file mode 100644 index 000000000..fc599c346 --- /dev/null +++ b/src/main/java/com/conveyal/gtfs/validator/FaresValidator.java @@ -0,0 +1,26 @@ +package com.conveyal.gtfs.validator; + +import com.conveyal.gtfs.error.NewGTFSErrorType; +import com.conveyal.gtfs.error.SQLErrorStorage; +import com.conveyal.gtfs.loader.Feed; +import com.conveyal.gtfs.model.FareAttribute; + +/** + * Validator for fares that currently just checks that the transfers and transfer_duration fields are harmonious. + */ +public class FaresValidator extends FeedValidator { + public FaresValidator(Feed feed, SQLErrorStorage errorStorage) { + super(feed, errorStorage); + } + + @Override + public void validate() { + for (FareAttribute fareAttribute : feed.fareAttributes) { + if (fareAttribute.transfers == 0 && fareAttribute.transfer_duration > 0) { + // If a fare does not permit transfers, but defines a duration for which a transfer is valid, register + // an error. + registerError(fareAttribute, NewGTFSErrorType.FARE_TRANSFER_MISMATCH); + } + } + } +} diff --git a/src/main/java/com/conveyal/gtfs/validator/FrequencyValidator.java b/src/main/java/com/conveyal/gtfs/validator/FrequencyValidator.java new file mode 100644 index 000000000..ac3392de8 --- /dev/null +++ b/src/main/java/com/conveyal/gtfs/validator/FrequencyValidator.java @@ -0,0 +1,76 @@ +package com.conveyal.gtfs.validator; + +import com.conveyal.gtfs.error.NewGTFSErrorType; +import com.conveyal.gtfs.error.SQLErrorStorage; +import com.conveyal.gtfs.loader.Feed; +import com.conveyal.gtfs.model.Frequency; +import com.conveyal.gtfs.model.Route; +import com.conveyal.gtfs.model.Stop; +import com.conveyal.gtfs.model.StopTime; +import com.conveyal.gtfs.model.Trip; +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.HashMultimap; +import com.google.common.collect.ListMultimap; +import com.google.common.collect.Multimap; + +import java.util.Collection; +import java.util.List; + +public class FrequencyValidator extends FeedValidator { + + /** + * Validate frequency entries to ensure that there are no overlapping frequency periods defined for a single trip. + * @param feed + * @param errorStorage + */ + public FrequencyValidator(Feed feed, SQLErrorStorage errorStorage) { + super(feed, errorStorage); + } + + private ListMultimap frequenciesById = ArrayListMultimap.create(); + + @Override + public void validate() { + // First, collect all frequencies for each trip ID. + for (Frequency frequency: feed.frequencies) frequenciesById.put(frequency.trip_id, frequency); + // Next iterate over each set of trip-specific frequency periods. + for (String tripId : frequenciesById.keySet()) { + List frequencies = frequenciesById.get(tripId); + if (frequencies.size() <= 1) { + // If there are not more than one frequencies defined for the trip, there can be no risk of overlapping + // frequency intervals. + return; + } + // Iterate over each frequency and check its period against the others for overlap. + for (int i = 0; i < frequencies.size() - 1; i++) { + Frequency a = frequencies.get(i); + // Iterate over the other frequencies starting with i + 1 to avoid checking against self and re-checking + // previous pairs. + for (int j = i + 1; j < frequencies.size(); j++) { + Frequency b = frequencies.get(j); + if ( + // -- diagrams courtesy of esiroky -- + // A wraps B. + // A: |---------| + // B: ___|--|____ + b.start_time >= a.start_time && b.end_time <= a.end_time || + // B wraps A. + // A: ___|--|____ + // B: |---------| + a.start_time >= b.start_time && a.end_time <= b.end_time || + // A starts during B, but ends after B ends. + // A: ____|-----| + // B: _|----|____ + a.start_time >= b.start_time && a.start_time < b.end_time || + // B starts during A, but ends after A ends + // A: _|----|____ + // B: ____|-----| + a.end_time > b.start_time && a.end_time <= b.end_time + ) { + registerError(a, NewGTFSErrorType.FREQUENCY_PERIOD_OVERLAP); + } + } + } + } + } +} diff --git a/src/main/java/com/conveyal/gtfs/validator/NewTripTimesValidator.java b/src/main/java/com/conveyal/gtfs/validator/NewTripTimesValidator.java index 3b6cf40ff..cf395e674 100644 --- a/src/main/java/com/conveyal/gtfs/validator/NewTripTimesValidator.java +++ b/src/main/java/com/conveyal/gtfs/validator/NewTripTimesValidator.java @@ -67,7 +67,7 @@ public void validate () { String previousTripId = null; // Order stop times by trip ID and sequence number (i.e. scan through the stops in each trip in order) for (StopTime stopTime : feed.stopTimes.getAllOrdered()) { - // FIXME all bad references should already be caught elsewhere, this should just be a continue + // All bad references should already be caught elsewhere, this should just be a continue if (stopTime.trip_id == null) continue; if (!stopTime.trip_id.equals(previousTripId) && !stopTimesForTrip.isEmpty()) { processTrip(stopTimesForTrip); diff --git a/src/main/java/com/conveyal/gtfs/validator/SpeedTripValidator.java b/src/main/java/com/conveyal/gtfs/validator/SpeedTripValidator.java index ef28671a7..31f13235f 100644 --- a/src/main/java/com/conveyal/gtfs/validator/SpeedTripValidator.java +++ b/src/main/java/com/conveyal/gtfs/validator/SpeedTripValidator.java @@ -1,6 +1,7 @@ package com.conveyal.gtfs.validator; import com.conveyal.gtfs.error.NewGTFSError; +import com.conveyal.gtfs.error.NewGTFSErrorType; import com.conveyal.gtfs.error.SQLErrorStorage; import com.conveyal.gtfs.loader.Feed; import com.conveyal.gtfs.model.Entity; @@ -47,6 +48,10 @@ public void validateTrip(Trip trip, Route route, List stopTimes, List< double distanceMeters = 0; for (int i = beginIndex + 1; i < stopTimes.size(); i++) { StopTime currStopTime = stopTimes.get(i); + if (currStopTime.pickup_type == 1 && currStopTime.drop_off_type == 1 && currStopTime.timepoint == 0) { + // stop_time allows neither pickup or drop off and is not a timepoint, so it serves no purpose. + registerError(currStopTime, NewGTFSErrorType.STOP_TIME_UNUSED); + } Stop currStop = stops.get(i); // Distance is accumulated in case times are not provided for some StopTimes. distanceMeters += fastDistance(currStop.stop_lat, currStop.stop_lon, prevStop.stop_lat, prevStop.stop_lon); @@ -54,7 +59,9 @@ public void validateTrip(Trip trip, Route route, List stopTimes, List< checkShapeDistTraveled(prevStopTime, currStopTime); if (missingBothTimes(currStopTime)) { // FixMissingTimes has already been called, so both arrival and departure time are missing. - // The spec allows this. Other than accumulating distance, skip this StopTime. + // The spec allows this. Other than accumulating distance, skip this StopTime. If this stop_time serves + // as a timepoint; however, this is considered an error. + if (currStopTime.timepoint == 1) registerError(currStopTime, NewGTFSErrorType.TIMEPOINT_MISSING_TIMES); continue; } if (currStopTime.departure_time < currStopTime.arrival_time) { diff --git a/src/test/java/com/conveyal/gtfs/GTFSTest.java b/src/test/java/com/conveyal/gtfs/GTFSTest.java index 84f568548..94dfda21c 100644 --- a/src/test/java/com/conveyal/gtfs/GTFSTest.java +++ b/src/test/java/com/conveyal/gtfs/GTFSTest.java @@ -32,7 +32,9 @@ import java.sql.Connection; import java.sql.ResultSet; import java.sql.SQLException; +import java.util.Arrays; import java.util.Collection; +import java.util.Iterator; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; @@ -96,11 +98,20 @@ public void requiresActionCommand() throws Exception { */ @Test public void canLoadAndExportSimpleAgency() { + ErrorExpectation[] fakeAgencyErrorExpectations = ErrorExpectation.list( + 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.DATE_NO_SERVICE) + ); assertThat( runIntegrationTestOnFolder( "fake-agency", nullValue(), - fakeAgencyPersistenceExpectations + fakeAgencyPersistenceExpectations, + fakeAgencyErrorExpectations ), equalTo(true) ); @@ -119,28 +130,49 @@ public void canLoadFeedWithBadDates () { } ) ); + ErrorExpectation[] errorExpectations = ErrorExpectation.list( + new ErrorExpectation(NewGTFSErrorType.MISSING_FIELD), + new ErrorExpectation(NewGTFSErrorType.DATE_FORMAT), + new ErrorExpectation(NewGTFSErrorType.DATE_FORMAT), + new ErrorExpectation(NewGTFSErrorType.DATE_FORMAT), + new ErrorExpectation(NewGTFSErrorType.REFERENTIAL_INTEGRITY), + new ErrorExpectation(NewGTFSErrorType.DATE_FORMAT), + new ErrorExpectation(NewGTFSErrorType.DATE_FORMAT), + new ErrorExpectation(NewGTFSErrorType.REFERENTIAL_INTEGRITY), + new ErrorExpectation(NewGTFSErrorType.ROUTE_LONG_NAME_CONTAINS_SHORT_NAME), + new ErrorExpectation(NewGTFSErrorType.FEED_TRAVEL_TIMES_ROUNDED), + new ErrorExpectation(NewGTFSErrorType.SERVICE_NEVER_ACTIVE), + new ErrorExpectation(NewGTFSErrorType.TRIP_NEVER_ACTIVE), + new ErrorExpectation(NewGTFSErrorType.SERVICE_UNUSED), + new ErrorExpectation(NewGTFSErrorType.DATE_NO_SERVICE) + ); assertThat( "Integration test passes", - runIntegrationTestOnFolder("fake-agency-bad-calendar-date", nullValue(), expectations), + runIntegrationTestOnFolder("fake-agency-bad-calendar-date", nullValue(), expectations, errorExpectations), equalTo(true) ); } /** - * Tests that a GTFS feed with overlapping block trips will record the appropriate error. + * Tests that a GTFS feed with errors is loaded properly and that the various errors were detected and stored in the + * database. */ @Test - public void canLoadFeedWithOverlappingTrips () { - PersistenceExpectation[] expectations = PersistenceExpectation.list( - new PersistenceExpectation( - new ErrorExpectation[]{ - new ErrorExpectation("error_type", NewGTFSErrorType.TRIP_OVERLAP_IN_BLOCK.toString()) - } - ) + public void canLoadFeedWithErrors () { + PersistenceExpectation[] expectations = PersistenceExpectation.list(); + ErrorExpectation[] errorExpectations = ErrorExpectation.list( + new ErrorExpectation(NewGTFSErrorType.FARE_TRANSFER_MISMATCH, equalTo("fare-02")), + new ErrorExpectation(NewGTFSErrorType.FREQUENCY_PERIOD_OVERLAP, equalTo("freq-01_08:30:00_to_10:15:00_every_15m00s")), + new ErrorExpectation(NewGTFSErrorType.FREQUENCY_PERIOD_OVERLAP, equalTo("freq-01_08:30:00_to_10:15:00_every_15m00s")), + new ErrorExpectation(NewGTFSErrorType.FREQUENCY_PERIOD_OVERLAP), + new ErrorExpectation(NewGTFSErrorType.FREQUENCY_PERIOD_OVERLAP), + new ErrorExpectation(NewGTFSErrorType.FREQUENCY_PERIOD_OVERLAP), + new ErrorExpectation(NewGTFSErrorType.FREQUENCY_PERIOD_OVERLAP), + new ErrorExpectation(NewGTFSErrorType.TRIP_OVERLAP_IN_BLOCK, equalTo("1A00000")) ); assertThat( "Integration test passes", - runIntegrationTestOnFolder("fake-agency-overlapping-trips", nullValue(), expectations), + runIntegrationTestOnFolder("fake-agency-overlapping-trips", nullValue(), expectations, errorExpectations), equalTo(true) ); } @@ -164,9 +196,30 @@ public void canLoadAndExportSimpleAgencyInSubDirectory() { } catch (IOException e) { e.printStackTrace(); } - // TODO Add error expectations argument that expects NewGTFSErrorType.TABLE_IN_SUBDIRECTORY error type. + ErrorExpectation[] errorExpectations = ErrorExpectation.list( + new ErrorExpectation(NewGTFSErrorType.TABLE_IN_SUBDIRECTORY), + new ErrorExpectation(NewGTFSErrorType.TABLE_IN_SUBDIRECTORY), + new ErrorExpectation(NewGTFSErrorType.MISSING_FIELD), + 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.TABLE_IN_SUBDIRECTORY), + 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.TABLE_IN_SUBDIRECTORY), + new ErrorExpectation(NewGTFSErrorType.TABLE_IN_SUBDIRECTORY), + new ErrorExpectation(NewGTFSErrorType.TABLE_IN_SUBDIRECTORY), + 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( - runIntegrationTestOnZipFile(zipFileName, nullValue(), fakeAgencyPersistenceExpectations), + runIntegrationTestOnZipFile(zipFileName, nullValue(), fakeAgencyPersistenceExpectations, errorExpectations), equalTo(true) ); } @@ -231,11 +284,17 @@ public void canLoadAndExportSimpleAgencyWithOnlyCalendarDates() { } ) }; + ErrorExpectation[] errorExpectations = ErrorExpectation.list( + new ErrorExpectation(NewGTFSErrorType.MISSING_FIELD), + new ErrorExpectation(NewGTFSErrorType.ROUTE_LONG_NAME_CONTAINS_SHORT_NAME), + new ErrorExpectation(NewGTFSErrorType.FEED_TRAVEL_TIMES_ROUNDED) + ); assertThat( runIntegrationTestOnFolder( "fake-agency-only-calendar-dates", nullValue(), - persistenceExpectations + persistenceExpectations, + errorExpectations ), equalTo(true) ); @@ -244,12 +303,13 @@ public void canLoadAndExportSimpleAgencyWithOnlyCalendarDates() { /** * A helper method that will zip a specified folder in test/main/resources and call - * {@link #runIntegrationTestOnZipFile(String, Matcher, PersistenceExpectation[])} on that file. + * {@link #runIntegrationTestOnZipFile} on that file. */ private boolean runIntegrationTestOnFolder( String folderName, Matcher fatalExceptionExpectation, - PersistenceExpectation[] persistenceExpectations + PersistenceExpectation[] persistenceExpectations, + ErrorExpectation[] errorExpectations ) { LOG.info("Running integration test on folder {}", folderName); // zip up test folder into temp zip file @@ -260,7 +320,7 @@ private boolean runIntegrationTestOnFolder( e.printStackTrace(); return false; } - return runIntegrationTestOnZipFile(zipFileName, fatalExceptionExpectation, persistenceExpectations); + return runIntegrationTestOnZipFile(zipFileName, fatalExceptionExpectation, persistenceExpectations, errorExpectations); } /** @@ -273,7 +333,8 @@ private boolean runIntegrationTestOnFolder( private boolean runIntegrationTestOnZipFile( String zipFileName, Matcher fatalExceptionExpectation, - PersistenceExpectation[] persistenceExpectations + PersistenceExpectation[] persistenceExpectations, + ErrorExpectation[] errorExpectations ) { String newDBName = TestUtils.generateNewDB(); String dbConnectionUrl = String.format("jdbc:postgresql://localhost/%s", newDBName); @@ -295,7 +356,7 @@ private boolean runIntegrationTestOnZipFile( assertThat(validationResult.fatalException, is(fatalExceptionExpectation)); namespace = loadResult.uniqueIdentifier; - assertThatImportedGtfsMeetsExpectations(dataSource.getConnection(), namespace, persistenceExpectations); + assertThatImportedGtfsMeetsExpectations(dataSource.getConnection(), namespace, persistenceExpectations, errorExpectations); } catch (SQLException e) { TestUtils.dropDB(newDBName); e.printStackTrace(); @@ -385,7 +446,8 @@ private ValuePair (Object expected, Object found) { private void assertThatImportedGtfsMeetsExpectations( Connection connection, String namespace, - PersistenceExpectation[] persistenceExpectations + PersistenceExpectation[] persistenceExpectations, + ErrorExpectation[] errorExpectations ) throws SQLException { // Store field mismatches here (to provide assertion statements with more details). Multimap fieldsWithMismatches = ArrayListMultimap.create(); @@ -442,6 +504,7 @@ private void assertThatImportedGtfsMeetsExpectations( recordExpectation.fieldName, new ValuePair(recordExpectation.stringExpectation, strVal) ); + LOG.error("Expected {}, found {}", recordExpectation.stringExpectation, strVal); allFieldsMatch = false; } break; @@ -456,10 +519,43 @@ private void assertThatImportedGtfsMeetsExpectations( LOG.info("Database record satisfies expectations."); foundRecord = true; break; + } else { + LOG.error("Persistence mismatch on record {}", numRecordsSearched); } } assertThatPersistenceExpectationRecordWasFound(numRecordsSearched, foundRecord, fieldsWithMismatches); } + // Expect zero errors if errorExpectations is null. + if (errorExpectations == null) errorExpectations = new ErrorExpectation[]{}; + // Check that error expectations match errors stored in database. + LOG.info("Checking {} error expectations", errorExpectations.length); + // select all entries from error table + String sql = String.format("select * from %s.errors", namespace); + LOG.info(sql); + ResultSet rs = connection.prepareStatement(sql).executeQuery(); + int errorCount = 0; + Iterator errorExpectationIterator = Arrays.stream(errorExpectations).iterator(); + while (rs.next()) { + errorCount++; + String errorType = rs.getString("error_type"); + String entityType = rs.getString("entity_type"); + String entityId = rs.getString("entity_id"); + String badValue = rs.getString("bad_value"); + LOG.info("Found error {}: {} {} {} {}", errorCount, errorType, entityId, entityType, badValue); + // Skip error expectation if not exists. But continue iteration to count all errors. + if (!errorExpectationIterator.hasNext()) continue; + ErrorExpectation errorExpectation = errorExpectationIterator.next(); + LOG.info("Expecting error {}: {}", errorCount, errorExpectation.errorTypeMatcher); + // Error expectation must contain error type matcher. The others are optional. + assertThat(errorType, errorExpectation.errorTypeMatcher); + if (errorExpectation.entityTypeMatcher != null) assertThat(entityType, errorExpectation.entityTypeMatcher); + if (errorExpectation.entityIdMatcher != null) assertThat(entityId, errorExpectation.entityIdMatcher); + if (errorExpectation.badValueMatcher != null) assertThat(badValue, errorExpectation.badValueMatcher); + } + assertThat( + "Error count is equal to number of error expectations.", + errorCount, + equalTo(errorExpectations.length)); } private static int countValidationErrorsOfType( diff --git a/src/test/java/com/conveyal/gtfs/loader/FieldTests.java b/src/test/java/com/conveyal/gtfs/loader/FieldTests.java index 520ecee9f..112dc68d7 100644 --- a/src/test/java/com/conveyal/gtfs/loader/FieldTests.java +++ b/src/test/java/com/conveyal/gtfs/loader/FieldTests.java @@ -2,7 +2,10 @@ import com.conveyal.gtfs.error.NewGTFSError; import com.conveyal.gtfs.error.NewGTFSErrorType; +import org.apache.commons.text.StringEscapeUtils; import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import static org.hamcrest.MatcherAssert.assertThat; @@ -10,6 +13,7 @@ * Unit tests to verify functionality of classes that load fields from GTFS tables. */ public class FieldTests { + private static final Logger LOG = LoggerFactory.getLogger(FieldTests.class); /** * Make sure our date field reader catches bad dates and accepts correct ones. @@ -55,12 +59,14 @@ public void illegalCharacterParseTest() { "\n", // simple new line "\t", // simple tab "\t\n\r", // new line, tab, carriage return - "Hello\\world", // backslashes not permitted (replaced with escaped slash) + // Unescaped backslashes no longer log an error; however, the character is still replaced with escaped slash. + // "Hello\\world", "Downtown via Peachtree\n\nSt" // new line and carriage within string }; StringField stringField = new StringField("any", Requirement.REQUIRED); for (String badString : badStrings) { ValidateFieldResult result = stringField.validateAndConvert(badString); + LOG.info("{} error(s) found for input string {}", result.errors.size(), StringEscapeUtils.escapeJava(badString)); assertThat("Input with illegal characters should result in an error.", result.errors.size() > 0); NewGTFSError error = result.errors.iterator().next(); assertThat("Error type should be illegal field value.", diff --git a/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java b/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java index 5ca031da1..0930c6b96 100644 --- a/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java +++ b/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java @@ -697,7 +697,7 @@ private static String newUUID() { /** * Constructs SQL query for the specified ID and columns and returns the resulting result set. */ - private String getColumnsForId(int id, Table table, String... columns) throws SQLException { + private String getColumnsForId(int id, Table table, String... columns) { String sql = String.format( "select %s from %s.%s where id=%d", columns.length > 0 ? String.join(", ", columns) : "*", diff --git a/src/test/java/com/conveyal/gtfs/storage/ErrorExpectation.java b/src/test/java/com/conveyal/gtfs/storage/ErrorExpectation.java index d4ad653ff..65af45c39 100644 --- a/src/test/java/com/conveyal/gtfs/storage/ErrorExpectation.java +++ b/src/test/java/com/conveyal/gtfs/storage/ErrorExpectation.java @@ -1,7 +1,49 @@ package com.conveyal.gtfs.storage; -public class ErrorExpectation extends RecordExpectation { - public ErrorExpectation(String fieldName, String stringExpectation) { - super(fieldName, stringExpectation); +import com.conveyal.gtfs.error.NewGTFSErrorType; +import org.hamcrest.Matcher; + +import static org.hamcrest.Matchers.equalTo; + +/** + * Defines the expected values for an error stored in the errors table for a feed in the GTFS database. + * + * Note: the errors should be listed in order that they are expected to be encountered. Check out + * {@link com.conveyal.gtfs.loader.JdbcGtfsLoader#loadTables} to see the order in which tables are loaded, + * {@link com.conveyal.gtfs.loader.Feed#validate()} to see the order in which validators are called (trip validator + * order can be found in {@link com.conveyal.gtfs.validator.NewTripTimesValidator}). + */ +public class ErrorExpectation { + public Matcher errorTypeMatcher; + public Matcher badValueMatcher; + public Matcher entityTypeMatcher; + public Matcher entityIdMatcher; + + public ErrorExpectation(NewGTFSErrorType errorType) { + this(errorType, null, null, null); + } + + public ErrorExpectation(NewGTFSErrorType errorType, Matcher entityIdMatcher) { + this(errorType, null, null, entityIdMatcher); + } + + /** + * Note: we accept Matchers as constructor args rather than the actual string values because this gives us the + * ability to specify null values in the case that we don't care about matching a specific value for an error + * (e.g., we only want to check for a matching error type but are not concerned with a specific error's entity ID + * value). + */ + public ErrorExpectation(NewGTFSErrorType errorType, Matcher badValueMatcher, Matcher entityTypeMatcher, Matcher entityIdMatcher) { + this.errorTypeMatcher = equalTo(errorType.toString()); + this.badValueMatcher = badValueMatcher; + this.entityTypeMatcher = entityTypeMatcher; + this.entityIdMatcher = entityIdMatcher; + } + + /** + * Constructs an array of error expectations from the input args. + */ + public static ErrorExpectation[] list (ErrorExpectation... expectations) { + return expectations; } } diff --git a/src/test/java/com/conveyal/gtfs/storage/PersistenceExpectation.java b/src/test/java/com/conveyal/gtfs/storage/PersistenceExpectation.java index 60bdc80a9..ee870b77d 100644 --- a/src/test/java/com/conveyal/gtfs/storage/PersistenceExpectation.java +++ b/src/test/java/com/conveyal/gtfs/storage/PersistenceExpectation.java @@ -18,11 +18,6 @@ public PersistenceExpectation(String tableName, RecordExpectation[] recordExpect this.recordExpectations = recordExpectations; } - public PersistenceExpectation(ErrorExpectation[] errorExpectations) { - this.tableName = "errors"; - this.recordExpectations = errorExpectations; - } - public static PersistenceExpectation[] list (PersistenceExpectation... expectations) { return expectations; } diff --git a/src/test/resources/fake-agency-overlapping-trips/fare_attributes.txt b/src/test/resources/fake-agency-overlapping-trips/fare_attributes.txt new file mode 100644 index 000000000..65f408058 --- /dev/null +++ b/src/test/resources/fake-agency-overlapping-trips/fare_attributes.txt @@ -0,0 +1,3 @@ +fare_id,price,transfers,transfer_duration +fare-01,10.00,0,0 +fare-02,10.00,0,9000 \ No newline at end of file diff --git a/src/test/resources/fake-agency-overlapping-trips/frequencies.txt b/src/test/resources/fake-agency-overlapping-trips/frequencies.txt new file mode 100644 index 000000000..dca15348e --- /dev/null +++ b/src/test/resources/fake-agency-overlapping-trips/frequencies.txt @@ -0,0 +1,6 @@ +trip_id,start_time,end_time,headway_secs,exact_times +freq-01,08:30:00,10:15:00,900,0 +freq-01,09:00:00,10:00:00,600,0 +freq-01,07:00:00,11:00:00,600,0 +freq-01,11:00:00,11:30:00,600,0 +freq-01,07:00:00,10:00:00,600,0 diff --git a/src/test/resources/fake-agency-overlapping-trips/stop_times.txt b/src/test/resources/fake-agency-overlapping-trips/stop_times.txt index 2ce4c8570..3470e2750 100755 --- a/src/test/resources/fake-agency-overlapping-trips/stop_times.txt +++ b/src/test/resources/fake-agency-overlapping-trips/stop_times.txt @@ -3,3 +3,5 @@ trip_id,arrival_time,departure_time,stop_id,stop_sequence,stop_headsign,pickup_t 1A00000,07:10:30,07:10:30,B000000,2,,0,0 2A00000,07:00:00,07:00:00,A000000,1,,0,0 2A00000,07:02:00,07:02:00,B000000,2,,0,0 +freq-01,07:00:00,07:00:00,A000000,1,,0,0 +freq-01,07:02:00,07:02:00,B000000,2,,0,0 \ No newline at end of file diff --git a/src/test/resources/fake-agency-overlapping-trips/trips.txt b/src/test/resources/fake-agency-overlapping-trips/trips.txt index b1b70c9d7..7860eae7d 100755 --- a/src/test/resources/fake-agency-overlapping-trips/trips.txt +++ b/src/test/resources/fake-agency-overlapping-trips/trips.txt @@ -1,3 +1,4 @@ route_id,trip_id,direction_id,block_id,bikes_allowed,wheelchair_accessible,service_id 10000000,1A00000,0,BLOCK_1,0,0,A 10000000,2A00000,0,BLOCK_1,0,0,A +10000000,freq-01,0,,0,0,A \ No newline at end of file