From 57eed2fe0914156bf346de1483c5c0676d7fb924 Mon Sep 17 00:00:00 2001 From: Jingsi Lu <5789435+qcdyx@users.noreply.github.com> Date: Fri, 15 Sep 2023 10:36:29 -0400 Subject: [PATCH] feat: 1537 get list of validators that couldnt run because of a parsing problem (#1565) * update comments for skippedValidator * added in validators with parsing errors logic and fixed tests * continue fix broken tests * applied the same logic to singleEntityValidators * fix RunTimeException ClassCastError * use Mockito's ArgumentCaptor to capture the argument for fixing "Strict stubbing argument mismatch" error * minor change * formatted code * renamed validatorsWithParsingErrors to singleFileValidatorsWithParsingErrors * renamed skippedFileValidators to multiFileValidatorsWithParsingErrors formatted code * combine three types of validators with parsing errors together and output validators that couldn't run due to parsing problems * renamed skippedValidators to multiFileValidatorsWithParsingErrors * formatted code * Added a test for single file skipped validators. Reformatted the output --------- Co-authored-by: jcpitre Co-authored-by: jcpitre <106176106+jcpitre@users.noreply.github.com> --- .../gtfsvalidator/table/AnyTableLoader.java | 26 +++- .../gtfsvalidator/table/GtfsFeedLoader.java | 12 +- .../validator/DefaultValidatorProvider.java | 29 +++-- .../validator/ValidatorLoader.java | 37 ++++-- .../validator/ValidatorProvider.java | 6 +- .../table/AnyTableLoaderTest.java | 13 +- .../table/GtfsFeedLoaderTest.java | 9 +- .../testgtfs/GtfsTestMultiFileValidator.java | 36 ++++++ ....java => GtfsTestSingleFileValidator.java} | 4 +- .../testgtfs/GtfsTestTableContainer2.java | 79 ++++++++++++ .../testgtfs/GtfsTestTableDescriptor2.java | 115 ++++++++++++++++++ .../DefaultValidatorProviderTest.java | 53 ++++++-- .../validator/ValidatorLoaderTest.java | 16 +-- .../runner/ValidationRunner.java | 56 +++++++-- 14 files changed, 428 insertions(+), 63 deletions(-) create mode 100644 core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestMultiFileValidator.java rename core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/{GtfsTestFileValidator.java => GtfsTestSingleFileValidator.java} (93%) create mode 100644 core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestTableContainer2.java create mode 100644 core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestTableDescriptor2.java diff --git a/core/src/main/java/org/mobilitydata/gtfsvalidator/table/AnyTableLoader.java b/core/src/main/java/org/mobilitydata/gtfsvalidator/table/AnyTableLoader.java index 78dd9abfec..3765bf22cb 100644 --- a/core/src/main/java/org/mobilitydata/gtfsvalidator/table/AnyTableLoader.java +++ b/core/src/main/java/org/mobilitydata/gtfsvalidator/table/AnyTableLoader.java @@ -7,6 +7,7 @@ import com.univocity.parsers.csv.CsvParserSettings; import java.io.InputStream; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Optional; import java.util.stream.Collectors; @@ -21,6 +22,7 @@ import org.mobilitydata.gtfsvalidator.parsing.CsvRow; import org.mobilitydata.gtfsvalidator.parsing.FieldCache; import org.mobilitydata.gtfsvalidator.parsing.RowParser; +import org.mobilitydata.gtfsvalidator.validator.FileValidator; import org.mobilitydata.gtfsvalidator.validator.SingleEntityValidator; import org.mobilitydata.gtfsvalidator.validator.ValidatorProvider; import org.mobilitydata.gtfsvalidator.validator.ValidatorUtil; @@ -28,6 +30,19 @@ public final class AnyTableLoader { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private static final List> singleFileValidatorsWithParsingErrors = + new ArrayList<>(); + + private static final List> + singleEntityValidatorsWithParsingErrors = new ArrayList<>(); + + public List> getValidatorsWithParsingErrors() { + return Collections.unmodifiableList(singleFileValidatorsWithParsingErrors); + } + + public List> getSingleEntityValidatorsWithParsingErrors() { + return Collections.unmodifiableList(singleEntityValidatorsWithParsingErrors); + } public static GtfsTableContainer load( GtfsTableDescriptor tableDescriptor, @@ -84,7 +99,8 @@ public static GtfsTableContainer load( final List entities = new ArrayList<>(); boolean hasUnparsableRows = false; final List> singleEntityValidators = - validatorProvider.createSingleEntityValidators(tableDescriptor.getEntityClass()); + validatorProvider.createSingleEntityValidators( + tableDescriptor.getEntityClass(), singleEntityValidatorsWithParsingErrors::add); try { for (CsvRow row : csvFile) { if (row.getRowNumber() % 200000 == 0) { @@ -130,7 +146,9 @@ public static GtfsTableContainer load( GtfsTableContainer table = tableDescriptor.createContainerForHeaderAndEntities(header, entities, noticeContainer); ValidatorUtil.invokeSingleFileValidators( - validatorProvider.createSingleFileValidators(table), noticeContainer); + validatorProvider.createSingleFileValidators( + table, singleFileValidatorsWithParsingErrors::add), + noticeContainer); return table; } @@ -194,7 +212,9 @@ public static GtfsTableContainer loadMissingFile( noticeContainer.addValidationNotice(new MissingRequiredFileNotice(gtfsFilename)); } ValidatorUtil.invokeSingleFileValidators( - validatorProvider.createSingleFileValidators(table), noticeContainer); + validatorProvider.createSingleFileValidators( + table, singleFileValidatorsWithParsingErrors::add), + noticeContainer); return table; } } diff --git a/core/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsFeedLoader.java b/core/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsFeedLoader.java index f8059d9683..e79a5105b1 100644 --- a/core/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsFeedLoader.java +++ b/core/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsFeedLoader.java @@ -50,9 +50,10 @@ public class GtfsFeedLoader { /** * The set of validators that were skipped during validation because their file dependencies had - * parse errors. + * parse errors plus validators that are optional. */ - private final List> skippedValidators = new ArrayList<>(); + private final List> multiFileValidatorsWithParsingErrors = + new ArrayList<>(); public GtfsFeedLoader( ImmutableList>> tableDescriptorClasses) { @@ -79,8 +80,8 @@ public void setNumThreads(int numThreads) { this.numThreads = numThreads; } - public List> getSkippedValidators() { - return Collections.unmodifiableList(skippedValidators); + public List> getMultiFileValidatorsWithParsingErrors() { + return Collections.unmodifiableList(multiFileValidatorsWithParsingErrors); } @SuppressWarnings("unchecked") @@ -147,7 +148,8 @@ public GtfsFeedContainer loadAndValidate( // Validators with parser-error dependencies will not be returned here, but instead added to // the skippedValidators list. for (FileValidator validator : - validatorProvider.createMultiFileValidators(feed, skippedValidators::add)) { + validatorProvider.createMultiFileValidators( + feed, multiFileValidatorsWithParsingErrors::add)) { validatorCallables.add( () -> { NoticeContainer validatorNotices = new NoticeContainer(); diff --git a/core/src/main/java/org/mobilitydata/gtfsvalidator/validator/DefaultValidatorProvider.java b/core/src/main/java/org/mobilitydata/gtfsvalidator/validator/DefaultValidatorProvider.java index e694fc9c02..f3fa18ac68 100644 --- a/core/src/main/java/org/mobilitydata/gtfsvalidator/validator/DefaultValidatorProvider.java +++ b/core/src/main/java/org/mobilitydata/gtfsvalidator/validator/DefaultValidatorProvider.java @@ -76,14 +76,23 @@ public TableHeaderValidator getTableHeaderValidator() { @Override @SuppressWarnings("unchecked") public List> createSingleEntityValidators( - Class clazz) { + Class clazz, + Consumer>> singleEntityValidatorsWithParsingErrors) { List> validators = new ArrayList<>(); for (Class> validatorClass : singleEntityValidators.get(clazz)) { try { - validators.add( - ValidatorLoader.createValidatorWithContext( - ((Class>) validatorClass), validationContext)); + ValidatorWithDependencyStatus> + validatorWithDependencyStatus = + ValidatorLoader.createValidatorWithContext( + ((Class>) validatorClass), + validationContext); + if (validatorWithDependencyStatus.dependenciesHaveErrors()) { + singleEntityValidatorsWithParsingErrors.accept( + (Class>) validatorClass); + } else { + validators.add((SingleEntityValidator) validatorWithDependencyStatus.validator()); + } } catch (ReflectiveOperationException e) { logger.atSevere().withCause(e).log( "Cannot instantiate validator %s", validatorClass.getCanonicalName()); @@ -95,13 +104,19 @@ public List> createSingleEntityV @Override @SuppressWarnings("unchecked") public List createSingleFileValidators( - GtfsTableContainer table) { + GtfsTableContainer table, + Consumer> validatorsWithParsingErrors) { List validators = new ArrayList<>(); for (Class validatorClass : singleFileValidators.get((Class>) table.getClass())) { try { - validators.add( - ValidatorLoader.createSingleFileValidator(validatorClass, table, validationContext)); + ValidatorWithDependencyStatus validatorWithStatus = + ValidatorLoader.createSingleFileValidator(validatorClass, table, validationContext); + if (validatorWithStatus.dependenciesHaveErrors()) { + validatorsWithParsingErrors.accept(validatorClass); + } else { + validators.add(validatorWithStatus.validator()); + } } catch (ReflectiveOperationException e) { logger.atSevere().withCause(e).log( "Cannot instantiate validator %s", validatorClass.getCanonicalName()); diff --git a/core/src/main/java/org/mobilitydata/gtfsvalidator/validator/ValidatorLoader.java b/core/src/main/java/org/mobilitydata/gtfsvalidator/validator/ValidatorLoader.java index c80968a304..9fd3578f48 100644 --- a/core/src/main/java/org/mobilitydata/gtfsvalidator/validator/ValidatorLoader.java +++ b/core/src/main/java/org/mobilitydata/gtfsvalidator/validator/ValidatorLoader.java @@ -169,7 +169,11 @@ private static Constructor chooseConstructor(Class validatorClass) validatorClass.getCanonicalName())); } - /** Instantiates a validator of given class and injects its dependencies. */ + /** + * Instantiates a validator of given class and injects its dependencies. A dependency is typically + * a table that is used by the validator. If any of the dependencies have errors, we put this + * information in the returned {@link ValidatorWithDependencyStatus}. + */ private static ValidatorWithDependencyStatus createValidator( Class clazz, DependencyResolver dependencyResolver) throws ReflectiveOperationException { Constructor chosenConstructor; @@ -199,20 +203,22 @@ private static ValidatorWithDependencyStatus createValidator( * @param type of the validator to instantiate * @return a new validator */ - public static T createValidatorWithContext( - Class clazz, ValidationContext validationContext) throws ReflectiveOperationException { - return createValidator(clazz, new DependencyResolver(validationContext, null, null)) - .validator(); + public static + ValidatorWithDependencyStatus createValidatorWithContext( + Class clazz, ValidationContext validationContext) throws ReflectiveOperationException { + return (ValidatorWithDependencyStatus) + createValidator(clazz, new DependencyResolver(validationContext, null, null)); } /** Instantiates a {@code FileValidator} for a single table. */ - public static FileValidator createSingleFileValidator( - Class clazz, - GtfsTableContainer table, - ValidationContext validationContext) - throws ReflectiveOperationException { - return createValidator(clazz, new DependencyResolver(validationContext, table, null)) - .validator(); + public static + ValidatorWithDependencyStatus createSingleFileValidator( + Class clazz, + GtfsTableContainer table, + ValidationContext validationContext) + throws ReflectiveOperationException { + return (ValidatorWithDependencyStatus) + createValidator(clazz, new DependencyResolver(validationContext, table, null)); } /** Instantiates a {@code FileValidator} for multiple tables in a given feed. */ @@ -244,6 +250,13 @@ public DependencyResolver( this.feedContainer = feedContainer; } + /** + * This is typically called to obtain the container related to each parameter in a validator's + * constructor or a basic type related to country code or date. + * + * @param parameterClass The parameter of the constructor we want to examine. + * @return + */ public Object resolveDependency(Class parameterClass) { if (feedContainer != null && parameterClass.isAssignableFrom(GtfsFeedContainer.class)) { if (!feedContainer.isParsedSuccessfully()) { diff --git a/core/src/main/java/org/mobilitydata/gtfsvalidator/validator/ValidatorProvider.java b/core/src/main/java/org/mobilitydata/gtfsvalidator/validator/ValidatorProvider.java index c928f67e26..2c243a8dc9 100644 --- a/core/src/main/java/org/mobilitydata/gtfsvalidator/validator/ValidatorProvider.java +++ b/core/src/main/java/org/mobilitydata/gtfsvalidator/validator/ValidatorProvider.java @@ -46,7 +46,8 @@ public interface ValidatorProvider { * @return a list of validators */ List> createSingleEntityValidators( - Class clazz); + Class clazz, + Consumer>> singleEntityValidatorsWithParsingErrors); /** * Creates a list of validators for the given table. @@ -57,7 +58,8 @@ List> createSingleEntityValidato * @param type of the GTFS entity */ List createSingleFileValidators( - GtfsTableContainer table); + GtfsTableContainer table, + Consumer> validatorsWithParsingErrors); /** * Creates a list of cross-table validators. Any validator that has a dependency with parse errors diff --git a/core/src/test/java/org/mobilitydata/gtfsvalidator/table/AnyTableLoaderTest.java b/core/src/test/java/org/mobilitydata/gtfsvalidator/table/AnyTableLoaderTest.java index 29802babef..6c6da42f6e 100644 --- a/core/src/test/java/org/mobilitydata/gtfsvalidator/table/AnyTableLoaderTest.java +++ b/core/src/test/java/org/mobilitydata/gtfsvalidator/table/AnyTableLoaderTest.java @@ -9,6 +9,7 @@ import java.io.InputStream; import java.util.List; import java.util.Set; +import java.util.function.Consumer; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -16,11 +17,14 @@ import org.mobilitydata.gtfsvalidator.notice.*; import org.mobilitydata.gtfsvalidator.parsing.CsvHeader; import org.mobilitydata.gtfsvalidator.testgtfs.GtfsTestEntity; -import org.mobilitydata.gtfsvalidator.testgtfs.GtfsTestFileValidator; +import org.mobilitydata.gtfsvalidator.testgtfs.GtfsTestSingleFileValidator; import org.mobilitydata.gtfsvalidator.testgtfs.GtfsTestTableDescriptor; +import org.mobilitydata.gtfsvalidator.validator.FileValidator; import org.mobilitydata.gtfsvalidator.validator.GtfsFieldValidator; import org.mobilitydata.gtfsvalidator.validator.TableHeaderValidator; import org.mobilitydata.gtfsvalidator.validator.ValidatorProvider; +import org.mockito.ArgumentCaptor; +import org.mockito.ArgumentMatchers; import org.mockito.Mock; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; @@ -124,8 +128,11 @@ public void parsableTableRows() { var testTableDescriptor = new GtfsTestTableDescriptor(); when(validatorProvider.getTableHeaderValidator()).thenReturn(mock(TableHeaderValidator.class)); when(validatorProvider.getFieldValidator()).thenReturn(mock(GtfsFieldValidator.class)); - GtfsTestFileValidator validator = mock(GtfsTestFileValidator.class); - when(validatorProvider.createSingleFileValidators(any())).thenReturn(List.of(validator)); + GtfsTestSingleFileValidator validator = mock(GtfsTestSingleFileValidator.class); + ArgumentCaptor>> captor = + ArgumentCaptor.forClass(Consumer.class); + when(validatorProvider.createSingleFileValidators(ArgumentMatchers.any(), captor.capture())) + .thenReturn((List.of(validator))); InputStream inputStream = toInputStream("id,stop_lat,_no_name_\n" + "s1, 23.00, no_value\n"); var loadedContainer = diff --git a/core/src/test/java/org/mobilitydata/gtfsvalidator/table/GtfsFeedLoaderTest.java b/core/src/test/java/org/mobilitydata/gtfsvalidator/table/GtfsFeedLoaderTest.java index 135a752854..d70d125efd 100644 --- a/core/src/test/java/org/mobilitydata/gtfsvalidator/table/GtfsFeedLoaderTest.java +++ b/core/src/test/java/org/mobilitydata/gtfsvalidator/table/GtfsFeedLoaderTest.java @@ -13,7 +13,7 @@ import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; import org.mobilitydata.gtfsvalidator.testgtfs.GtfsTestEntity; import org.mobilitydata.gtfsvalidator.testgtfs.GtfsTestEntityValidator; -import org.mobilitydata.gtfsvalidator.testgtfs.GtfsTestFileValidator; +import org.mobilitydata.gtfsvalidator.testgtfs.GtfsTestSingleFileValidator; import org.mobilitydata.gtfsvalidator.testgtfs.GtfsTestTableContainer; import org.mobilitydata.gtfsvalidator.testgtfs.GtfsTestTableDescriptor; import org.mobilitydata.gtfsvalidator.testgtfs.WholeFeedValidator; @@ -26,7 +26,9 @@ public class GtfsFeedLoaderTest { private static final ImmutableList> VALIDATOR_CLASSES = ImmutableList.of( - GtfsTestEntityValidator.class, GtfsTestFileValidator.class, WholeFeedValidator.class); + GtfsTestEntityValidator.class, + GtfsTestSingleFileValidator.class, + WholeFeedValidator.class); private MockGtfs mockGtfs; private NoticeContainer noticeContainer = new NoticeContainer(); @@ -74,6 +76,7 @@ public void testInvalidDataInTable() throws Exception { GtfsTestTableContainer container = feedContainer.getTable(GtfsTestTableContainer.class); assertThat(container.getEntities()).isEmpty(); - assertThat(loader.getSkippedValidators()).containsExactly(WholeFeedValidator.class); + assertThat(loader.getMultiFileValidatorsWithParsingErrors()) + .containsExactly(WholeFeedValidator.class); } } diff --git a/core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestMultiFileValidator.java b/core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestMultiFileValidator.java new file mode 100644 index 0000000000..7585b3624f --- /dev/null +++ b/core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestMultiFileValidator.java @@ -0,0 +1,36 @@ +/* + * Copyright 2021 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.mobilitydata.gtfsvalidator.testgtfs; + +import javax.inject.Inject; +import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; +import org.mobilitydata.gtfsvalidator.validator.FileValidator; + +public class GtfsTestMultiFileValidator extends FileValidator { + + private final GtfsTestTableContainer table; + private final GtfsTestTableContainer2 unparsableTable; + + @Inject + public GtfsTestMultiFileValidator(GtfsTestTableContainer table, GtfsTestTableContainer2 table2) { + this.table = table; + this.unparsableTable = table2; + } + + @Override + public void validate(NoticeContainer noticeContainer) {} +} diff --git a/core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestFileValidator.java b/core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestSingleFileValidator.java similarity index 93% rename from core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestFileValidator.java rename to core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestSingleFileValidator.java index 5179a94749..9b9608f12b 100644 --- a/core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestFileValidator.java +++ b/core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestSingleFileValidator.java @@ -22,14 +22,14 @@ import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; import org.mobilitydata.gtfsvalidator.validator.FileValidator; -public class GtfsTestFileValidator extends FileValidator { +public class GtfsTestSingleFileValidator extends FileValidator { private final GtfsTestTableContainer table; private final CountryCode countryCode; private final CurrentDateTime currentDateTime; @Inject - public GtfsTestFileValidator( + public GtfsTestSingleFileValidator( GtfsTestTableContainer table, CountryCode countryCode, CurrentDateTime currentDateTime) { this.table = table; this.countryCode = countryCode; diff --git a/core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestTableContainer2.java b/core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestTableContainer2.java new file mode 100644 index 0000000000..962b5bb6e7 --- /dev/null +++ b/core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestTableContainer2.java @@ -0,0 +1,79 @@ +/* + * Copyright 2021 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.mobilitydata.gtfsvalidator.testgtfs; + +import com.google.common.collect.ImmutableList; +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; +import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; +import org.mobilitydata.gtfsvalidator.parsing.CsvHeader; +import org.mobilitydata.gtfsvalidator.table.GtfsTableContainer; + +// We need a second test table class to test multi file validators. +public class GtfsTestTableContainer2 extends GtfsTableContainer { + private static final ImmutableList KEY_COLUMN_NAMES = + ImmutableList.of(GtfsTestEntity.ID_FIELD_NAME); + + private List entities; + + private GtfsTestTableContainer2( + GtfsTestTableDescriptor2 descriptor, CsvHeader header, List entities) { + super(descriptor, TableStatus.PARSABLE_HEADERS_AND_ROWS, header); + this.entities = entities; + } + + public GtfsTestTableContainer2(TableStatus tableStatus) { + super(new GtfsTestTableDescriptor2(), tableStatus, CsvHeader.EMPTY); + this.entities = new ArrayList<>(); + } + + @Override + public Class getEntityClass() { + return GtfsTestEntity.class; + } + + @Override + public String gtfsFilename() { + return GtfsTestEntity.FILENAME + "2"; + } + + @Override + public List getEntities() { + return entities; + } + + /** Creates a table with given header and entities */ + public static GtfsTestTableContainer2 forHeaderAndEntities( + GtfsTestTableDescriptor2 descriptor, + CsvHeader header, + List entities, + NoticeContainer noticeContainer) { + GtfsTestTableContainer2 table = new GtfsTestTableContainer2(descriptor, header, entities); + return table; + } + + @Override + public ImmutableList getKeyColumnNames() { + return KEY_COLUMN_NAMES; + } + + @Override + public Optional byTranslationKey(String recordId, String recordSubId) { + return Optional.empty(); + } +} diff --git a/core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestTableDescriptor2.java b/core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestTableDescriptor2.java new file mode 100644 index 0000000000..c8442428a5 --- /dev/null +++ b/core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestTableDescriptor2.java @@ -0,0 +1,115 @@ +package org.mobilitydata.gtfsvalidator.testgtfs; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import java.util.List; +import java.util.Optional; +import org.mobilitydata.gtfsvalidator.annotation.FieldLevelEnum; +import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; +import org.mobilitydata.gtfsvalidator.parsing.CsvHeader; +import org.mobilitydata.gtfsvalidator.parsing.FieldCache; +import org.mobilitydata.gtfsvalidator.parsing.RowParser; +import org.mobilitydata.gtfsvalidator.table.*; + +// We need a second test table descriptor to test multi file contaioners +public class GtfsTestTableDescriptor2 extends GtfsTableDescriptor { + @Override + public GtfsTableContainer createContainerForInvalidStatus( + GtfsTableContainer.TableStatus tableStatus) { + return new GtfsTestTableContainer2(tableStatus); + } + + @Override + public GtfsTableContainer createContainerForHeaderAndEntities( + CsvHeader header, List entities, NoticeContainer noticeContainer) { + return GtfsTestTableContainer2.forHeaderAndEntities(this, header, entities, noticeContainer); + } + + @Override + public GtfsEntityBuilder createEntityBuilder() { + return new GtfsTestEntity.Builder(); + } + + @Override + public Class getEntityClass() { + return GtfsTestEntity.class; + } + + @Override + public ImmutableList getColumns() { + ImmutableList.Builder builder = ImmutableList.builder(); + builder.add( + GtfsColumnDescriptor.builder() + .setColumnName(GtfsTestEntity.ID_FIELD_NAME) + .setHeaderRequired(true) + .setHeaderRecommended(false) + .setFieldLevel(FieldLevelEnum.REQUIRED) + .setIsMixedCase(false) + .setIsCached(false) + .build()); + builder.add( + GtfsColumnDescriptor.builder() + .setColumnName(GtfsTestEntity.CODE_FIELD_NAME) + .setHeaderRequired(false) + .setHeaderRecommended(false) + .setFieldLevel(FieldLevelEnum.OPTIONAL) + .setIsMixedCase(false) + .setIsCached(false) + .build()); + return builder.build(); + } + + @Override + public ImmutableMap getFieldLoaders() { + ImmutableMap.Builder builder = ImmutableMap.builder(); + builder.put( + GtfsTestEntity.ID_FIELD_NAME, + new GtfsFieldLoader() { + @Override + public void load( + RowParser rowParser, + int columnIndex, + GtfsColumnDescriptor columnDescriptor, + FieldCache fieldCache, + GtfsTestEntity.Builder builder) { + builder.setId( + addToCacheIfPresent(rowParser.asId(columnIndex, columnDescriptor), fieldCache)); + } + }); + builder.put( + GtfsTestEntity.CODE_FIELD_NAME, + new GtfsFieldLoader() { + @Override + public void load( + RowParser rowParser, + int columnIndex, + GtfsColumnDescriptor columnDescriptor, + FieldCache fieldCache, + GtfsTestEntity.Builder builder) { + builder.setCode( + addToCacheIfPresent(rowParser.asText(columnIndex, columnDescriptor), fieldCache)); + } + }); + return builder.build(); + } + + @Override + public String gtfsFilename() { + return GtfsTestEntity.FILENAME + "2"; + } + + @Override + public boolean isRecommended() { + return false; + } + + @Override + public boolean isRequired() { + return true; + } + + @Override + public Optional maxCharsPerColumn() { + return Optional.empty(); + } +} diff --git a/core/src/test/java/org/mobilitydata/gtfsvalidator/validator/DefaultValidatorProviderTest.java b/core/src/test/java/org/mobilitydata/gtfsvalidator/validator/DefaultValidatorProviderTest.java index 2c462a32b1..90d10fa10f 100644 --- a/core/src/test/java/org/mobilitydata/gtfsvalidator/validator/DefaultValidatorProviderTest.java +++ b/core/src/test/java/org/mobilitydata/gtfsvalidator/validator/DefaultValidatorProviderTest.java @@ -14,8 +14,10 @@ import org.mobilitydata.gtfsvalidator.table.GtfsTableContainer.TableStatus; import org.mobilitydata.gtfsvalidator.testgtfs.GtfsTestEntity; import org.mobilitydata.gtfsvalidator.testgtfs.GtfsTestEntityValidator; -import org.mobilitydata.gtfsvalidator.testgtfs.GtfsTestFileValidator; +import org.mobilitydata.gtfsvalidator.testgtfs.GtfsTestMultiFileValidator; +import org.mobilitydata.gtfsvalidator.testgtfs.GtfsTestSingleFileValidator; import org.mobilitydata.gtfsvalidator.testgtfs.GtfsTestTableContainer; +import org.mobilitydata.gtfsvalidator.testgtfs.GtfsTestTableContainer2; import org.mobilitydata.gtfsvalidator.testgtfs.WholeFeedValidator; @RunWith(JUnit4.class) @@ -29,20 +31,30 @@ public void testCreateValidators() throws ValidatorLoaderException { ValidatorLoader.createForClasses( ImmutableList.of( GtfsTestEntityValidator.class, - GtfsTestFileValidator.class, + GtfsTestSingleFileValidator.class, WholeFeedValidator.class))); GtfsTestTableContainer tableContainer = new GtfsTestTableContainer(TableStatus.PARSABLE_HEADERS_AND_ROWS); GtfsFeedContainer feedContainer = new GtfsFeedContainer(ImmutableList.of(tableContainer)); - + List> singleEntityValidatorsWithParsingErrors = + new ArrayList<>(); assertThat( - provider.createSingleEntityValidators(GtfsTestEntity.class).stream() + provider + .createSingleEntityValidators( + GtfsTestEntity.class, singleEntityValidatorsWithParsingErrors::add) + .stream() .map(Object::getClass)) .containsExactly(GtfsTestEntityValidator.class); - assertThat(provider.createSingleFileValidators(tableContainer).stream().map(Object::getClass)) - .containsExactly(GtfsTestFileValidator.class); + List> singleFileValidatorsWithParsingErrors = new ArrayList<>(); + assertThat( + provider + .createSingleFileValidators( + tableContainer, singleFileValidatorsWithParsingErrors::add) + .stream() + .map(Object::getClass)) + .containsExactly(GtfsTestSingleFileValidator.class); List> skippedValidators = new ArrayList<>(); assertThat( @@ -60,15 +72,36 @@ public void testCreateValidators_skippedValidators() throws ValidatorLoaderExcep ValidatorLoader.createForClasses( ImmutableList.of( GtfsTestEntityValidator.class, - GtfsTestFileValidator.class, + GtfsTestMultiFileValidator.class, + GtfsTestSingleFileValidator.class, WholeFeedValidator.class))); - // Our table had invalid data! + // Create 2 tables, one with errors and the other not. + // This will let us test the multi-file validator. GtfsTestTableContainer tableContainer = new GtfsTestTableContainer(TableStatus.UNPARSABLE_ROWS); - GtfsFeedContainer feedContainer = new GtfsFeedContainer(ImmutableList.of(tableContainer)); + GtfsTestTableContainer2 tableContainer2 = + new GtfsTestTableContainer2(TableStatus.PARSABLE_HEADERS_AND_ROWS); + + GtfsFeedContainer feedContainer = + new GtfsFeedContainer(ImmutableList.of(tableContainer, tableContainer2)); List> skippedValidators = new ArrayList<>(); + // First test the multi file validators. Apparently the FeedContainerValidator is considered a + // multi-file validator. + // We should not be able to create any validator since the dependant file container has parsing + // errors. For the WholeFeedValidator the feedContainer is also in error since one of its + // file is in error. assertThat(provider.createMultiFileValidators(feedContainer, skippedValidators::add)).isEmpty(); - assertThat(skippedValidators).containsExactly(WholeFeedValidator.class); + // And the 2 validators should be skipped + assertThat(skippedValidators) + .containsExactly(WholeFeedValidator.class, GtfsTestMultiFileValidator.class); + + skippedValidators.clear(); + // Try with the single file validator. We should not be able to build any validator since the + // file has errors. + assertThat(provider.createSingleFileValidators(tableContainer, skippedValidators::add)) + .isEmpty(); + // And it should tell us that the single file validator was skipped + assertThat(skippedValidators).containsExactly(GtfsTestSingleFileValidator.class); } } diff --git a/core/src/test/java/org/mobilitydata/gtfsvalidator/validator/ValidatorLoaderTest.java b/core/src/test/java/org/mobilitydata/gtfsvalidator/validator/ValidatorLoaderTest.java index 8607f56fcc..66b64fc632 100644 --- a/core/src/test/java/org/mobilitydata/gtfsvalidator/validator/ValidatorLoaderTest.java +++ b/core/src/test/java/org/mobilitydata/gtfsvalidator/validator/ValidatorLoaderTest.java @@ -27,7 +27,7 @@ import org.mobilitydata.gtfsvalidator.table.GtfsFeedContainer; import org.mobilitydata.gtfsvalidator.table.GtfsTableContainer.TableStatus; import org.mobilitydata.gtfsvalidator.testgtfs.GtfsTestEntityValidator; -import org.mobilitydata.gtfsvalidator.testgtfs.GtfsTestFileValidator; +import org.mobilitydata.gtfsvalidator.testgtfs.GtfsTestSingleFileValidator; import org.mobilitydata.gtfsvalidator.testgtfs.GtfsTestTableContainer; import org.mobilitydata.gtfsvalidator.testgtfs.WholeFeedValidator; import org.mobilitydata.gtfsvalidator.validator.ValidatorLoader.ValidatorWithDependencyStatus; @@ -46,7 +46,8 @@ public class ValidatorLoaderTest { public void createValidatorWithContext_injectsContext() throws ReflectiveOperationException { GtfsTestEntityValidator validator = ValidatorLoader.createValidatorWithContext( - GtfsTestEntityValidator.class, VALIDATION_CONTEXT); + GtfsTestEntityValidator.class, VALIDATION_CONTEXT) + .validator(); assertThat(validator.getCountryCode()).isEqualTo(VALIDATION_CONTEXT.countryCode()); assertThat(validator.getCurrentDateTime()).isEqualTo(VALIDATION_CONTEXT.currentDateTime()); @@ -56,10 +57,11 @@ public void createValidatorWithContext_injectsContext() throws ReflectiveOperati public void createSingleFileValidator_injectsTableContainerAndContext() throws ReflectiveOperationException { GtfsTestTableContainer table = new GtfsTestTableContainer(TableStatus.EMPTY_FILE); - GtfsTestFileValidator validator = - (GtfsTestFileValidator) + GtfsTestSingleFileValidator validator = + (GtfsTestSingleFileValidator) ValidatorLoader.createSingleFileValidator( - GtfsTestFileValidator.class, table, VALIDATION_CONTEXT); + GtfsTestSingleFileValidator.class, table, VALIDATION_CONTEXT) + .validator(); assertThat(validator.getCountryCode()).isEqualTo(VALIDATION_CONTEXT.countryCode()); assertThat(validator.getCurrentDateTime()).isEqualTo(VALIDATION_CONTEXT.currentDateTime()); @@ -90,9 +92,9 @@ public void createMultiFileValidator_singleContainer_dependenciesHaveErrors() GtfsTestTableContainer table = new GtfsTestTableContainer(TableStatus.UNPARSABLE_ROWS); GtfsFeedContainer feedContainer = new GtfsFeedContainer(ImmutableList.of(table)); - ValidatorWithDependencyStatus validatorWithStatus = + ValidatorWithDependencyStatus validatorWithStatus = ValidatorLoader.createMultiFileValidator( - GtfsTestFileValidator.class, feedContainer, VALIDATION_CONTEXT); + GtfsTestSingleFileValidator.class, feedContainer, VALIDATION_CONTEXT); assertThat(validatorWithStatus.dependenciesHaveErrors()).isTrue(); assertThat(validatorWithStatus.validator().getStopTable()).isEqualTo(table); diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/runner/ValidationRunner.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/runner/ValidationRunner.java index d7ff32c3c9..b06f52a4ee 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/runner/ValidationRunner.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/runner/ValidationRunner.java @@ -41,6 +41,7 @@ import org.mobilitydata.gtfsvalidator.report.JsonReport; import org.mobilitydata.gtfsvalidator.report.JsonReportGenerator; import org.mobilitydata.gtfsvalidator.report.model.FeedMetadata; +import org.mobilitydata.gtfsvalidator.table.AnyTableLoader; import org.mobilitydata.gtfsvalidator.table.GtfsFeedContainer; import org.mobilitydata.gtfsvalidator.table.GtfsFeedLoader; import org.mobilitydata.gtfsvalidator.util.VersionInfo; @@ -89,6 +90,7 @@ public Status run(ValidationRunnerConfig config) { return Status.EXCEPTION; } GtfsFeedLoader feedLoader = new GtfsFeedLoader(ClassGraphDiscovery.discoverTables()); + AnyTableLoader anyTableLoader = new AnyTableLoader(); logger.atInfo().log("validation config:\n%s", config); logger.atInfo().log("validators:\n%s", validatorLoader.listValidators()); @@ -134,7 +136,7 @@ public Status run(ValidationRunnerConfig config) { // Output exportReport(feedMetadata, noticeContainer, config, versionInfo); - printSummary(startNanos, feedContainer, feedLoader); + printSummary(startNanos, feedContainer, feedLoader, anyTableLoader); return Status.SUCCESS; } @@ -145,18 +147,54 @@ public Status run(ValidationRunnerConfig config) { * @param feedContainer the {@code GtfsFeedContainer} */ public static void printSummary( - long startNanos, GtfsFeedContainer feedContainer, GtfsFeedLoader loader) { + long startNanos, + GtfsFeedContainer feedContainer, + GtfsFeedLoader loader, + AnyTableLoader anyTableLoader) { final long endNanos = System.nanoTime(); - List> skippedValidators = loader.getSkippedValidators(); - if (!skippedValidators.isEmpty()) { + List> multiFileValidatorsWithParsingErrors = + loader.getMultiFileValidatorsWithParsingErrors(); + List> singleFileValidatorsWithParsingErrors = + anyTableLoader.getValidatorsWithParsingErrors(); + // In theory single entity validators do not depend on files so there should not be any of these + // with parsing errors + List> singleEntityValidatorsWithParsingErrors = + anyTableLoader.getSingleEntityValidatorsWithParsingErrors(); + if (!singleFileValidatorsWithParsingErrors.isEmpty() + || !singleEntityValidatorsWithParsingErrors.isEmpty() + || !multiFileValidatorsWithParsingErrors.isEmpty()) { StringBuilder b = new StringBuilder(); b.append("\n"); - b.append(" ----------------------------------------- \n"); - b.append("| Some validators were never invoked. |\n"); - b.append(" ----------------------------------------- \n"); - b.append("Skipped validators: "); b.append( - skippedValidators.stream().map(Class::getSimpleName).collect(Collectors.joining(","))); + "---------------------------------------------------------------------------------------- \n"); + b.append( + "| Some validators were skipped because the GTFS files they rely on could not be parsed | \n"); + b.append( + "---------------------------------------------------------------------------------------- \n"); + if (!multiFileValidatorsWithParsingErrors.isEmpty()) { + // Add some spaces to the delimiter so the validator names are indented. Easier to read. + b.append("Multi-file validators:\n "); + b.append( + multiFileValidatorsWithParsingErrors.stream() + .map(Class::getSimpleName) + .collect(Collectors.joining("\n "))); + } + + if (!singleFileValidatorsWithParsingErrors.isEmpty()) { + b.append("Single-file validators:\n "); + b.append( + singleFileValidatorsWithParsingErrors.stream() + .map(Class::getSimpleName) + .collect(Collectors.joining("\n "))); + } + if (!singleEntityValidatorsWithParsingErrors.isEmpty()) { + b.append("Single-entity validators:\n "); + b.append( + singleEntityValidatorsWithParsingErrors.stream() + .map(Class::getSimpleName) + .collect(Collectors.joining("\n "))); + } + logger.atSevere().log(b.toString()); } logger.atInfo().log("Validation took %.3f seconds%n", (endNanos - startNanos) / 1e9);