diff --git a/src/main/java/com/conveyal/gtfs/GTFS.java b/src/main/java/com/conveyal/gtfs/GTFS.java index 52ba26165..59e5ec83d 100644 --- a/src/main/java/com/conveyal/gtfs/GTFS.java +++ b/src/main/java/com/conveyal/gtfs/GTFS.java @@ -1,6 +1,5 @@ package com.conveyal.gtfs; -import com.conveyal.gtfs.error.SQLErrorStorage; import com.conveyal.gtfs.loader.Feed; import com.conveyal.gtfs.loader.FeedLoadResult; import com.conveyal.gtfs.loader.JdbcGtfsExporter; @@ -8,7 +7,7 @@ import com.conveyal.gtfs.loader.JdbcGtfsSnapshotter; import com.conveyal.gtfs.loader.SnapshotResult; import com.conveyal.gtfs.util.InvalidNamespaceException; -import com.conveyal.gtfs.validator.FeedValidator; +import com.conveyal.gtfs.validator.FeedValidatorCreator; import com.conveyal.gtfs.validator.ValidationResult; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.io.Files; @@ -94,19 +93,13 @@ public static SnapshotResult makeSnapshot (String feedId, DataSource dataSource) return result; } - /** - * Shorthand for calling validate() without custom validators. - */ - public static ValidationResult validate (String feedId, DataSource dataSource) { - return validate(feedId, dataSource, null); - } - /** * Once a feed has been loaded into the database, examine its contents looking for various problems and errors. */ - public static ValidationResult validate (String feedId, DataSource dataSource, BiFunction> customValidatorRequest) { + public static ValidationResult validate (String feedId, DataSource dataSource, FeedValidatorCreator... additionalValidators) { Feed feed = new Feed(dataSource, feedId); - return feed.validate(customValidatorRequest); + ValidationResult result = feed.validate(additionalValidators); + return result; } /** diff --git a/src/main/java/com/conveyal/gtfs/loader/Feed.java b/src/main/java/com/conveyal/gtfs/loader/Feed.java index 180095a44..46b294b40 100644 --- a/src/main/java/com/conveyal/gtfs/loader/Feed.java +++ b/src/main/java/com/conveyal/gtfs/loader/Feed.java @@ -1,6 +1,5 @@ package com.conveyal.gtfs.loader; -import com.conveyal.gtfs.error.GTFSError; import com.conveyal.gtfs.error.NewGTFSError; import com.conveyal.gtfs.error.SQLErrorStorage; import com.conveyal.gtfs.model.*; @@ -14,9 +13,7 @@ import javax.sql.DataSource; import java.sql.Connection; import java.sql.SQLException; -import java.util.ArrayList; import java.util.List; -import java.util.function.BiFunction; import static com.conveyal.gtfs.error.NewGTFSErrorType.VALIDATOR_FAILED; @@ -33,20 +30,15 @@ public class Feed { // This may be the empty string if the feed is stored in the root ("public") schema. public final String tablePrefix; - public final TableReader agencies; - public final TableReader calendars; - public final TableReader calendarDates; + public final TableReader agencies; + public final TableReader calendars; + public final TableReader calendarDates; public final TableReader fareAttributes; - public final TableReader frequencies; - public final TableReader routes; - public final TableReader stops; - public final TableReader trips; -// public final TableReader shapePoints; - public final TableReader stopTimes; - - /* A place to accumulate errors while the feed is loaded. Tolerate as many errors as possible and keep on loading. */ - // TODO remove this and use only NewGTFSErrors in Validators, loaded into a JDBC table - public final List errors = new ArrayList<>(); + public final TableReader frequencies; + public final TableReader routes; + public final TableReader stops; + public final TableReader trips; + public final TableReader stopTimes; /** * Create a feed that reads tables over a JDBC connection. The connection should already be set to the right @@ -66,36 +58,32 @@ public Feed (DataSource dataSource, String tablePrefix) { routes = new JDBCTableReader(Table.ROUTES, dataSource, tablePrefix, EntityPopulator.ROUTE); stops = new JDBCTableReader(Table.STOPS, dataSource, tablePrefix, EntityPopulator.STOP); trips = new JDBCTableReader(Table.TRIPS, dataSource, tablePrefix, EntityPopulator.TRIP); -// shapePoints = new JDBCTableReader(Table.SHAPES, dataSource, tablePrefix, EntityPopulator.SHAPE_POINT); stopTimes = new JDBCTableReader(Table.STOP_TIMES, dataSource, tablePrefix, EntityPopulator.STOP_TIME); } /** - * Shorthand for calling validate() without custom validators. - */ - public ValidationResult validate () { - return validate(null); - } - - /** + * Run the standard validation checks for this feed and store the validation errors in the database. Optionally, + * takes one or more {@link FeedValidatorCreator} in the form of lambda method refs (e.g., {@code MTCValidator::new}), + * which this method will instantiate and run after the standard validation checks have been completed. + * * TODO check whether validation has already occurred, overwrite results. - * TODO allow validation within feed loading process, so the same connection can be used, and we're certain loaded data is 100% visible. - * That would also avoid having to reconnect the error storage to the DB. + * TODO allow validation within feed loading process, so the same connection can be used, and we're certain loaded + * data is 100% visible. That would also avoid having to reconnect the error storage to the DB. */ - public ValidationResult validate (BiFunction> customValidatorRequest) { + public ValidationResult validate (FeedValidatorCreator... additionalValidators) { long validationStartTime = System.currentTimeMillis(); // Create an empty validation result that will have its fields populated by certain validators. ValidationResult validationResult = new ValidationResult(); // Error tables should already be present from the initial load. // Reconnect to the existing error tables. - SQLErrorStorage errorStorage = null; + SQLErrorStorage errorStorage; try { errorStorage = new SQLErrorStorage(dataSource.getConnection(), tablePrefix, false); } catch (SQLException | InvalidNamespaceException ex) { throw new StorageException(ex); } int errorCountBeforeValidation = errorStorage.getErrorCount(); - + // Create list of standard validators to run on every feed. List feedValidators = Lists.newArrayList( new MisplacedStopValidator(this, errorStorage, validationResult), new DuplicateStopsValidator(this, errorStorage), @@ -105,18 +93,16 @@ public ValidationResult validate (BiFunction customValidators = null; - if (customValidatorRequest != null) customValidators = customValidatorRequest.apply(this, errorStorage); - if (customValidators != null) feedValidators.addAll(customValidators); + // Create additional validators specified in this method's args and add to list of feed validators to run. + for (FeedValidatorCreator creator : additionalValidators) { + if (creator != null) feedValidators.add(creator.create(this, errorStorage)); + } for (FeedValidator feedValidator : feedValidators) { String validatorName = feedValidator.getClass().getSimpleName(); try { LOG.info("Running {}.", validatorName); int errorCountBefore = errorStorage.getErrorCount(); - // todo why not just pass the feed and errorstorage in here? feedValidator.validate(); LOG.info("{} found {} errors.", validatorName, errorStorage.getErrorCount() - errorCountBefore); } catch (Exception e) { diff --git a/src/main/java/com/conveyal/gtfs/validator/FeedValidatorCreator.java b/src/main/java/com/conveyal/gtfs/validator/FeedValidatorCreator.java new file mode 100644 index 000000000..39fbc67e4 --- /dev/null +++ b/src/main/java/com/conveyal/gtfs/validator/FeedValidatorCreator.java @@ -0,0 +1,24 @@ +package com.conveyal.gtfs.validator; + +import com.conveyal.gtfs.error.SQLErrorStorage; +import com.conveyal.gtfs.loader.Feed; + +/** + * A functional interface used to create {@link FeedValidator} instances. This interface supports the ability to pass + * validators by lambda method references ({@code SomeValidator::new}) to the {@link Feed#validate(FeedValidatorCreator...)} + * method in order to run special validation checks on specific feeds (e.g., {@link MTCValidator} should be run only on + * GTFS files loaded for those MTC operators). To instantiate the FeedValidator, simply call the + * {@link #create(Feed, SQLErrorStorage)} method, passing in the feed and errorStorage arguments. This is in lieu of + * instantiating the FeedValidator with those arguments in the constructor because these objects are not available before + * the validate method is invoked. + */ +@FunctionalInterface +public interface FeedValidatorCreator { + /** + * The callback that instantiates and returns instances of custom FeedValidator objects + * constructed using the provided feed and error storage objects. + * @param feed The feed being validated. + * @param errorStorage The object that handles error storage. + */ + FeedValidator create(Feed feed, SQLErrorStorage errorStorage); +} diff --git a/src/main/java/com/conveyal/gtfs/validator/ReversedTripValidator.java b/src/main/java/com/conveyal/gtfs/validator/ReversedTripValidator.java index ebe1c1d1c..b2a4c7f90 100644 --- a/src/main/java/com/conveyal/gtfs/validator/ReversedTripValidator.java +++ b/src/main/java/com/conveyal/gtfs/validator/ReversedTripValidator.java @@ -50,7 +50,8 @@ public boolean validate(Feed feed, boolean repair) { if (trip.shape_id == null) { isValid = false; if (missingShapeErrorCount < errorLimit) { - feed.errors.add(new MissingShapeError(trip)); + // FIXME store MissingShape errors + // feed.errors.add(new MissingShapeError(trip)); } missingShapeErrorCount++; continue; @@ -111,7 +112,8 @@ public boolean validate(Feed feed, boolean repair) { // check if first stop is x times closer to end of shape than the beginning or last stop is x times closer to start than the end if (distanceFirstStopToStart > (distanceFirstStopToEnd * distanceMultiplier) && distanceLastStopToEnd > (distanceLastStopToStart * distanceMultiplier)) { if (reversedTripShapeErrorCount < errorLimit) { - feed.errors.add(new ReversedTripShapeError(trip)); + // FIXME store ReversedTripShape errors + // feed.errors.add(new ReversedTripShapeError(trip)); } reversedTripShapeErrorCount++; isValid = false; @@ -120,7 +122,8 @@ public boolean validate(Feed feed, boolean repair) { if (missingCoordinatesErrorCount > 0) { for (Map.Entry> shapeError : missingShapesMap.entrySet()) { String[] tripIdList = shapeError.getValue().toArray(new String[shapeError.getValue().size()]); - feed.errors.add(new ShapeMissingCoordinatesError(shapeError.getKey(), tripIdList)); + // FIXME store ShapeMissingCoordinates errors + // feed.errors.add(new ShapeMissingCoordinatesError(shapeError.getKey(), tripIdList)); } } return isValid; diff --git a/src/test/java/com/conveyal/gtfs/GTFSTest.java b/src/test/java/com/conveyal/gtfs/GTFSTest.java index bd361042e..b7a22c5e2 100644 --- a/src/test/java/com/conveyal/gtfs/GTFSTest.java +++ b/src/test/java/com/conveyal/gtfs/GTFSTest.java @@ -2,8 +2,6 @@ import com.conveyal.gtfs.error.NewGTFSErrorType; -import com.conveyal.gtfs.error.SQLErrorStorage; -import com.conveyal.gtfs.loader.Feed; import com.conveyal.gtfs.loader.FeedLoadResult; import com.conveyal.gtfs.loader.SnapshotResult; import com.conveyal.gtfs.storage.ErrorExpectation; @@ -11,12 +9,11 @@ import com.conveyal.gtfs.storage.PersistenceExpectation; import com.conveyal.gtfs.storage.RecordExpectation; import com.conveyal.gtfs.util.InvalidNamespaceException; -import com.conveyal.gtfs.validator.FeedValidator; +import com.conveyal.gtfs.validator.FeedValidatorCreator; import com.conveyal.gtfs.validator.MTCValidator; import com.conveyal.gtfs.validator.ValidationResult; import com.csvreader.CsvReader; import com.google.common.collect.ArrayListMultimap; -import com.google.common.collect.Lists; import com.google.common.collect.Multimap; import com.google.common.io.Files; import org.apache.commons.io.FileUtils; @@ -38,12 +35,9 @@ import java.sql.Connection; import java.sql.ResultSet; import java.sql.SQLException; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Iterator; -import java.util.List; -import java.util.function.BiFunction; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; @@ -326,10 +320,13 @@ public void canLoadFeedWithLongFieldValues () { ); assertThat( "Long-field-value test passes", - runIntegrationTestOnFolder("fake-agency-mtc-long-fields", nullValue(), expectations, errorExpectations, - (feed, errorSource) -> { - return Lists.newArrayList(new MTCValidator(feed, errorSource)); - }), + runIntegrationTestOnFolder( + "fake-agency-mtc-long-fields", + nullValue(), + expectations, + errorExpectations, + MTCValidator::new + ), equalTo(true) ); } @@ -361,7 +358,7 @@ private boolean runIntegrationTestOnFolder( Matcher fatalExceptionExpectation, PersistenceExpectation[] persistenceExpectations, ErrorExpectation[] errorExpectations, - BiFunction> customValidatorRequest + FeedValidatorCreator customValidator ) { LOG.info("Running integration test on folder {}", folderName); // zip up test folder into temp zip file @@ -372,7 +369,13 @@ private boolean runIntegrationTestOnFolder( e.printStackTrace(); return false; } - return runIntegrationTestOnZipFile(zipFileName, fatalExceptionExpectation, persistenceExpectations, errorExpectations, customValidatorRequest); + return runIntegrationTestOnZipFile( + zipFileName, + fatalExceptionExpectation, + persistenceExpectations, + errorExpectations, + customValidator + ); } /** @@ -407,7 +410,7 @@ private boolean runIntegrationTestOnZipFile( Matcher fatalExceptionExpectation, PersistenceExpectation[] persistenceExpectations, ErrorExpectation[] errorExpectations, - BiFunction> customValidatorRequest + FeedValidatorCreator customValidator ) { String testDBName = TestUtils.generateNewDB(); String dbConnectionUrl = String.join("/", JDBC_URL, testDBName); @@ -424,7 +427,7 @@ private boolean runIntegrationTestOnZipFile( // load and validate feed LOG.info("load and validate GTFS file {}", zipFileName); FeedLoadResult loadResult = GTFS.load(zipFileName, dataSource); - ValidationResult validationResult = GTFS.validate(loadResult.uniqueIdentifier, dataSource, customValidatorRequest); + ValidationResult validationResult = GTFS.validate(loadResult.uniqueIdentifier, dataSource, customValidator); assertThat(validationResult.fatalException, is(fatalExceptionExpectation)); namespace = loadResult.uniqueIdentifier;