From aade17e6b4a29d13078efaf98b78800660e2902d Mon Sep 17 00:00:00 2001 From: Brian Donahue Date: Tue, 24 Jan 2023 16:47:45 -0500 Subject: [PATCH 1/9] fix: require agency_id when more than one agency, warn when only one --- .../validator/AgencyConsistencyValidator.java | 22 +++++ .../validator/RouteAgencyIdValidator.java | 83 ++++++++++++++++++ .../validator/TripAgencyIdValidator.java | 60 ------------- .../AgencyConsistencyValidatorTest.java | 57 +++++++++---- .../validator/RouteAgencyIdValidatorTest.java | 84 +++++++++++++++++++ 5 files changed, 228 insertions(+), 78 deletions(-) create mode 100644 main/src/main/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidator.java delete mode 100644 main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TripAgencyIdValidator.java create mode 100644 main/src/test/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidatorTest.java diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidator.java index d3d8bdf2bd..b1c2e92389 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidator.java @@ -52,6 +52,13 @@ public class AgencyConsistencyValidator extends FileValidator { public void validate(NoticeContainer noticeContainer) { final int agencyCount = agencyTable.entityCount(); if (agencyCount < 2) { + GtfsAgency agency = agencyTable.getEntities().get(0); + // agency_id is recommended even when there is only 1 agency + if (!agency.hasAgencyId()) { + noticeContainer.addValidationNotice( + new AgencyIdRecommendedForSingleAgency(agency.csvRowNumber(), agency.agencyName())); + } + // no further validation required return; } @@ -116,6 +123,21 @@ static class InconsistentAgencyLangNotice extends ValidationNotice { } } + /** + * {@code agency.agencyId } recommended, even for single agency record. + * + *

Severity: {@code SeverityLevel.WARNING} + */ + static class AgencyIdRecommendedForSingleAgency extends ValidationNotice { + private final long csvRowNumber; + private final String agencyName; + + AgencyIdRecommendedForSingleAgency(long csvRowNumber, String agencyName) { + super(SeverityLevel.WARNING); + this.csvRowNumber = csvRowNumber; + this.agencyName = agencyName; + } + } /** * Inconsistent timezone among agencies. * diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidator.java new file mode 100644 index 0000000000..82dca48111 --- /dev/null +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidator.java @@ -0,0 +1,83 @@ +/* + * Copyright 2020 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.validator; + +import javax.inject.Inject; +import org.mobilitydata.gtfsvalidator.annotation.GtfsValidator; +import org.mobilitydata.gtfsvalidator.notice.MissingRequiredFieldNotice; +import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; +import org.mobilitydata.gtfsvalidator.notice.SeverityLevel; +import org.mobilitydata.gtfsvalidator.notice.ValidationNotice; +import org.mobilitydata.gtfsvalidator.table.*; + +/** + * Checks that agency_id field in "routes.txt" is defined for every row if there is more than 1 + * agency in the feed. + * + *

Generated notice: {@link MissingRequiredFieldNotice}. + * + *

Generated notice: {@link AgencyIdRecommendedNotice}. + */ +@GtfsValidator +public class RouteAgencyIdValidator extends FileValidator { + private final GtfsAgencyTableContainer agencyTable; + private final GtfsRouteTableContainer routeTable; + + @Inject + RouteAgencyIdValidator(GtfsAgencyTableContainer agencyTable, GtfsRouteTableContainer routeTable) { + this.agencyTable = agencyTable; + this.routeTable = routeTable; + } + + @Override + public void validate(NoticeContainer noticeContainer) { + + // routes.agency_id is required when there are multiple agencies + int totalAgencies = agencyTable.entityCount(); + + for (GtfsRoute route : routeTable.getEntities()) { + if (!route.hasAgencyId()) { + if (totalAgencies > 1) { + // add error notice if more than one agency + noticeContainer.addValidationNotice( + new MissingRequiredFieldNotice( + routeTable.gtfsFilename(), + route.csvRowNumber(), + GtfsRoute.AGENCY_ID_FIELD_NAME)); + } else { + // add warning notice if only one agency + noticeContainer.addValidationNotice(new AgencyIdRecommendedNotice(route.csvRowNumber())); + } + } + } + // No need to check reference integrity because it is done by a validator generated from + // @ForeignKey annotation. + } + /** + * AgencyId field is recommended even if only one agency. + * + *

Severity: {@code SeverityLevel.WARNING} + */ + static class AgencyIdRecommendedNotice extends ValidationNotice { + private final long csvRowNumber; + + AgencyIdRecommendedNotice(long csvRowNumber) { + super(SeverityLevel.WARNING); + this.csvRowNumber = csvRowNumber; + } + } +} diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TripAgencyIdValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TripAgencyIdValidator.java deleted file mode 100644 index f017269f8b..0000000000 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TripAgencyIdValidator.java +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Copyright 2020 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.validator; - -import javax.inject.Inject; -import org.mobilitydata.gtfsvalidator.annotation.GtfsValidator; -import org.mobilitydata.gtfsvalidator.notice.MissingRequiredFieldNotice; -import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; -import org.mobilitydata.gtfsvalidator.table.GtfsAgencyTableContainer; -import org.mobilitydata.gtfsvalidator.table.GtfsRoute; -import org.mobilitydata.gtfsvalidator.table.GtfsRouteTableContainer; - -/** - * Checks that agency_id field in "routes.txt" is defined for every row if there is more than 1 - * agency in the feed. - * - *

Generated notice: {@link MissingRequiredFieldNotice}. - */ -@GtfsValidator -public class TripAgencyIdValidator extends FileValidator { - private final GtfsAgencyTableContainer agencyTable; - private final GtfsRouteTableContainer routeTable; - - @Inject - TripAgencyIdValidator(GtfsAgencyTableContainer agencyTable, GtfsRouteTableContainer routeTable) { - this.agencyTable = agencyTable; - this.routeTable = routeTable; - } - - @Override - public void validate(NoticeContainer noticeContainer) { - if (agencyTable.entityCount() < 2) { - // routes.agency_id is not required when there is a single agency. - return; - } - for (GtfsRoute route : routeTable.getEntities()) { - if (!route.hasAgencyId()) { - noticeContainer.addValidationNotice( - new MissingRequiredFieldNotice( - routeTable.gtfsFilename(), route.csvRowNumber(), GtfsRoute.AGENCY_ID_FIELD_NAME)); - } - // No need to check reference integrity because it is done by a validator generated from - // @ForeignKey annotation. - } - } -} diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidatorTest.java index 33c46f7ebc..8ee9a898ef 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidatorTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidatorTest.java @@ -28,6 +28,7 @@ import org.junit.runners.JUnit4; import org.mobilitydata.gtfsvalidator.notice.MissingRequiredFieldNotice; import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; +import org.mobilitydata.gtfsvalidator.notice.SeverityLevel; import org.mobilitydata.gtfsvalidator.notice.ValidationNotice; import org.mobilitydata.gtfsvalidator.table.GtfsAgency; import org.mobilitydata.gtfsvalidator.table.GtfsAgencyTableContainer; @@ -62,25 +63,45 @@ public static GtfsAgency createAgency( } @Test - public void multipleAgenciesPresentButNoAgencyIdSetShouldGenerateNotice() { - assertThat( - generateNotices( - ImmutableList.of( - createAgency( - 0, - "first agency", - "agency name", - "www.mobilitydata.org", - ZoneId.of("America/Montreal"), - Locale.CANADA), - createAgency( - 1, - null, - "agency name", - "www.mobilitydata.org", - ZoneId.of("America/Montreal"), - Locale.CANADA)))) + public void singleAgencyPresentButNoAgencyIdSetShouldGenerateWarningNotice() { + List notices = + generateNotices( + ImmutableList.of( + createAgency( + 0, + null, + "agency name", + "www.mobilitydata.org", + ZoneId.of("America/Montreal"), + Locale.CANADA))); + assertThat(notices) + .containsExactly( + new AgencyConsistencyValidator.AgencyIdRecommendedForSingleAgency(0, "agency name")); + assertThat(notices.get(0).getSeverityLevel()).isEqualTo(SeverityLevel.WARNING); + } + + @Test + public void multipleAgenciesPresentButNoAgencyIdSetShouldGenerateErrorNotice() { + List notices = + generateNotices( + ImmutableList.of( + createAgency( + 0, + "first agency", + "agency name", + "www.mobilitydata.org", + ZoneId.of("America/Montreal"), + Locale.CANADA), + createAgency( + 1, + null, + "agency name", + "www.mobilitydata.org", + ZoneId.of("America/Montreal"), + Locale.CANADA))); + assertThat(notices) .containsExactly(new MissingRequiredFieldNotice("agency.txt", 1, "agency_id")); + assertThat(notices.get(0).getSeverityLevel()).isEqualTo(SeverityLevel.ERROR); } @Test diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidatorTest.java new file mode 100644 index 0000000000..bf8441f69d --- /dev/null +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidatorTest.java @@ -0,0 +1,84 @@ +package org.mobilitydata.gtfsvalidator.validator; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableList; +import org.junit.Test; +import org.mobilitydata.gtfsvalidator.notice.MissingRequiredFieldNotice; +import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; +import org.mobilitydata.gtfsvalidator.table.*; + +public class RouteAgencyIdValidatorTest { + + public static GtfsRoute createRoute( + int rowNumber, String routeId, String agencyId, String shortName) { + + return new GtfsRoute.Builder() + .setCsvRowNumber(rowNumber) + .setAgencyId(agencyId) + .setRouteId(routeId) + .setRouteShortName(shortName) + .build(); + } + + public static GtfsAgency createAgency(int csvRowNumber, String agencyId, String agencyName) { + return new GtfsAgency.Builder() + .setCsvRowNumber(csvRowNumber) + .setAgencyId(agencyId) + .setAgencyName(agencyName) + .build(); + } + + @Test + public void agencyIdRequiredWhenMoreThanOneAgency() { + + NoticeContainer noticeContainer = new NoticeContainer(); + GtfsAgencyTableContainer agencyTable = + GtfsAgencyTableContainer.forEntities( + ImmutableList.of( + createAgency(0, "oneAgencyId", "Agency with Id"), + createAgency(1, null, "Agency with no ID")), + noticeContainer); + GtfsRouteTableContainer routeTable = + GtfsRouteTableContainer.forEntities( + ImmutableList.of( + createRoute(0, "route_0", "agency0", "Route 0"), + createRoute(1, "route_1", null, "Route 1")), + noticeContainer); + new RouteAgencyIdValidator(agencyTable, routeTable).validate(noticeContainer); + assertThat(noticeContainer.getValidationNotices()) + .containsExactly( + new MissingRequiredFieldNotice( + routeTable.gtfsFilename(), 1, GtfsRoute.AGENCY_ID_FIELD_NAME)); + } + + @Test + public void agencyIdWarningWhenOnlyOneAgencyWithNoAgencyId() { + + NoticeContainer noticeContainer = new NoticeContainer(); + GtfsAgencyTableContainer agencyTable = + GtfsAgencyTableContainer.forEntities( + ImmutableList.of(createAgency(1, null, "Agency with no ID")), noticeContainer); + GtfsRouteTableContainer routeTable = + GtfsRouteTableContainer.forEntities( + ImmutableList.of(createRoute(0, "route_0", null, "Route 0")), noticeContainer); + new RouteAgencyIdValidator(agencyTable, routeTable).validate(noticeContainer); + assertThat(noticeContainer.getValidationNotices()) + .containsExactly(new RouteAgencyIdValidator.AgencyIdRecommendedNotice(0)); + } + + @Test + public void agencyIdWarningWhenOnlyOneAgencyWithAgencyId() { + + NoticeContainer noticeContainer = new NoticeContainer(); + GtfsAgencyTableContainer agencyTable = + GtfsAgencyTableContainer.forEntities( + ImmutableList.of(createAgency(1, "agencyId", "Agency with ID")), noticeContainer); + GtfsRouteTableContainer routeTable = + GtfsRouteTableContainer.forEntities( + ImmutableList.of(createRoute(0, "route_0", null, "Route 0")), noticeContainer); + new RouteAgencyIdValidator(agencyTable, routeTable).validate(noticeContainer); + assertThat(noticeContainer.getValidationNotices()) + .containsExactly(new RouteAgencyIdValidator.AgencyIdRecommendedNotice(0)); + } +} From 97f987fc56a2bd2f5458db07ad3cc0d75932345b Mon Sep 17 00:00:00 2001 From: Brian Donahue Date: Wed, 25 Jan 2023 13:06:41 -0500 Subject: [PATCH 2/9] Remove custom error, use MissingRecommendedField --- .../validator/AgencyConsistencyValidator.java | 23 ++------------- .../validator/RouteAgencyIdValidator.java | 28 ++++--------------- .../AgencyConsistencyValidatorTest.java | 7 ++--- .../validator/RouteAgencyIdValidatorTest.java | 24 ++++------------ 4 files changed, 17 insertions(+), 65 deletions(-) diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidator.java index b1c2e92389..f75ea79b16 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidator.java @@ -20,10 +20,7 @@ import java.util.Locale; import javax.inject.Inject; import org.mobilitydata.gtfsvalidator.annotation.GtfsValidator; -import org.mobilitydata.gtfsvalidator.notice.MissingRequiredFieldNotice; -import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; -import org.mobilitydata.gtfsvalidator.notice.SeverityLevel; -import org.mobilitydata.gtfsvalidator.notice.ValidationNotice; +import org.mobilitydata.gtfsvalidator.notice.*; import org.mobilitydata.gtfsvalidator.table.GtfsAgency; import org.mobilitydata.gtfsvalidator.table.GtfsAgencyTableContainer; @@ -35,6 +32,7 @@ * *

@@ -56,7 +54,7 @@ public void validate(NoticeContainer noticeContainer) { // agency_id is recommended even when there is only 1 agency if (!agency.hasAgencyId()) { noticeContainer.addValidationNotice( - new AgencyIdRecommendedForSingleAgency(agency.csvRowNumber(), agency.agencyName())); + new MissingRecommendedFieldNotice(agencyTable.gtfsFilename(), agency.csvRowNumber(), agency.agencyName())); } // no further validation required return; @@ -123,21 +121,6 @@ static class InconsistentAgencyLangNotice extends ValidationNotice { } } - /** - * {@code agency.agencyId } recommended, even for single agency record. - * - *

Severity: {@code SeverityLevel.WARNING} - */ - static class AgencyIdRecommendedForSingleAgency extends ValidationNotice { - private final long csvRowNumber; - private final String agencyName; - - AgencyIdRecommendedForSingleAgency(long csvRowNumber, String agencyName) { - super(SeverityLevel.WARNING); - this.csvRowNumber = csvRowNumber; - this.agencyName = agencyName; - } - } /** * Inconsistent timezone among agencies. * diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidator.java index 82dca48111..8ffa85a494 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidator.java @@ -18,10 +18,7 @@ import javax.inject.Inject; import org.mobilitydata.gtfsvalidator.annotation.GtfsValidator; -import org.mobilitydata.gtfsvalidator.notice.MissingRequiredFieldNotice; -import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; -import org.mobilitydata.gtfsvalidator.notice.SeverityLevel; -import org.mobilitydata.gtfsvalidator.notice.ValidationNotice; +import org.mobilitydata.gtfsvalidator.notice.*; import org.mobilitydata.gtfsvalidator.table.*; /** @@ -30,7 +27,7 @@ * *

Generated notice: {@link MissingRequiredFieldNotice}. * - *

Generated notice: {@link AgencyIdRecommendedNotice}. + *

Generated notice: {@link MissingRecommendedFieldNotice}. */ @GtfsValidator public class RouteAgencyIdValidator extends FileValidator { @@ -55,29 +52,16 @@ public void validate(NoticeContainer noticeContainer) { // add error notice if more than one agency noticeContainer.addValidationNotice( new MissingRequiredFieldNotice( - routeTable.gtfsFilename(), - route.csvRowNumber(), - GtfsRoute.AGENCY_ID_FIELD_NAME)); + routeTable.gtfsFilename(), route.csvRowNumber(), GtfsRoute.AGENCY_ID_FIELD_NAME)); } else { // add warning notice if only one agency - noticeContainer.addValidationNotice(new AgencyIdRecommendedNotice(route.csvRowNumber())); + noticeContainer.addValidationNotice( + new MissingRecommendedFieldNotice( + routeTable.gtfsFilename(), route.csvRowNumber(), GtfsRoute.AGENCY_ID_FIELD_NAME)); } } } // No need to check reference integrity because it is done by a validator generated from // @ForeignKey annotation. } - /** - * AgencyId field is recommended even if only one agency. - * - *

Severity: {@code SeverityLevel.WARNING} - */ - static class AgencyIdRecommendedNotice extends ValidationNotice { - private final long csvRowNumber; - - AgencyIdRecommendedNotice(long csvRowNumber) { - super(SeverityLevel.WARNING); - this.csvRowNumber = csvRowNumber; - } - } } diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidatorTest.java index 8ee9a898ef..d90660aecd 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidatorTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidatorTest.java @@ -26,10 +26,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -import org.mobilitydata.gtfsvalidator.notice.MissingRequiredFieldNotice; -import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; -import org.mobilitydata.gtfsvalidator.notice.SeverityLevel; -import org.mobilitydata.gtfsvalidator.notice.ValidationNotice; +import org.mobilitydata.gtfsvalidator.notice.*; import org.mobilitydata.gtfsvalidator.table.GtfsAgency; import org.mobilitydata.gtfsvalidator.table.GtfsAgencyTableContainer; import org.mobilitydata.gtfsvalidator.validator.AgencyConsistencyValidator.InconsistentAgencyLangNotice; @@ -76,7 +73,7 @@ public void singleAgencyPresentButNoAgencyIdSetShouldGenerateWarningNotice() { Locale.CANADA))); assertThat(notices) .containsExactly( - new AgencyConsistencyValidator.AgencyIdRecommendedForSingleAgency(0, "agency name")); + new MissingRecommendedFieldNotice("agency.txt", 0, "agency name")); assertThat(notices.get(0).getSeverityLevel()).isEqualTo(SeverityLevel.WARNING); } diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidatorTest.java index bf8441f69d..522c569173 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidatorTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidatorTest.java @@ -4,6 +4,7 @@ import com.google.common.collect.ImmutableList; import org.junit.Test; +import org.mobilitydata.gtfsvalidator.notice.MissingRecommendedFieldNotice; import org.mobilitydata.gtfsvalidator.notice.MissingRequiredFieldNotice; import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; import org.mobilitydata.gtfsvalidator.table.*; @@ -30,7 +31,7 @@ public static GtfsAgency createAgency(int csvRowNumber, String agencyId, String } @Test - public void agencyIdRequiredWhenMoreThanOneAgency() { + public void agencyIdRequiredErrorWhenMoreThanOneAgency() { NoticeContainer noticeContainer = new NoticeContainer(); GtfsAgencyTableContainer agencyTable = @@ -53,7 +54,7 @@ public void agencyIdRequiredWhenMoreThanOneAgency() { } @Test - public void agencyIdWarningWhenOnlyOneAgencyWithNoAgencyId() { + public void agencyIdRecommendedWarningWhenOnlyOneAgency() { NoticeContainer noticeContainer = new NoticeContainer(); GtfsAgencyTableContainer agencyTable = @@ -64,21 +65,8 @@ public void agencyIdWarningWhenOnlyOneAgencyWithNoAgencyId() { ImmutableList.of(createRoute(0, "route_0", null, "Route 0")), noticeContainer); new RouteAgencyIdValidator(agencyTable, routeTable).validate(noticeContainer); assertThat(noticeContainer.getValidationNotices()) - .containsExactly(new RouteAgencyIdValidator.AgencyIdRecommendedNotice(0)); - } - - @Test - public void agencyIdWarningWhenOnlyOneAgencyWithAgencyId() { - - NoticeContainer noticeContainer = new NoticeContainer(); - GtfsAgencyTableContainer agencyTable = - GtfsAgencyTableContainer.forEntities( - ImmutableList.of(createAgency(1, "agencyId", "Agency with ID")), noticeContainer); - GtfsRouteTableContainer routeTable = - GtfsRouteTableContainer.forEntities( - ImmutableList.of(createRoute(0, "route_0", null, "Route 0")), noticeContainer); - new RouteAgencyIdValidator(agencyTable, routeTable).validate(noticeContainer); - assertThat(noticeContainer.getValidationNotices()) - .containsExactly(new RouteAgencyIdValidator.AgencyIdRecommendedNotice(0)); + .containsExactly( + new MissingRecommendedFieldNotice( + routeTable.gtfsFilename(), 0, GtfsRoute.AGENCY_ID_FIELD_NAME)); } } From dffd354cf36d0ae064ab6c697250eba0c41d8815 Mon Sep 17 00:00:00 2001 From: Brian Donahue Date: Wed, 25 Jan 2023 14:52:36 -0500 Subject: [PATCH 3/9] Add agency_id validation for fare_attributes.txt, make consistent with routes.txt validator --- .../FareAttributeAgencyIdValidator.java | 72 +++++++++++++++++++ .../validator/RouteAgencyIdValidator.java | 2 +- .../FareAttributeAgencyIdValidatorTest.java | 70 ++++++++++++++++++ .../validator/RouteAgencyIdValidatorTest.java | 3 +- 4 files changed, 144 insertions(+), 3 deletions(-) create mode 100644 main/src/main/java/org/mobilitydata/gtfsvalidator/validator/FareAttributeAgencyIdValidator.java create mode 100644 main/src/test/java/org/mobilitydata/gtfsvalidator/validator/FareAttributeAgencyIdValidatorTest.java diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/FareAttributeAgencyIdValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/FareAttributeAgencyIdValidator.java new file mode 100644 index 0000000000..8ac49b7b08 --- /dev/null +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/FareAttributeAgencyIdValidator.java @@ -0,0 +1,72 @@ +/* + * Copyright 2020 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.validator; + +import javax.inject.Inject; +import org.mobilitydata.gtfsvalidator.annotation.GtfsValidator; +import org.mobilitydata.gtfsvalidator.notice.*; +import org.mobilitydata.gtfsvalidator.table.*; + +/** + * Checks that agency_id field in "fare_attributes.txt" is defined for every row if there is more + * than 1 agency in the feed, recommended if only 1 agency. + * + *

Generated notice: {@link MissingRequiredFieldNotice}. + * + *

Generated notice: {@link MissingRecommendedFieldNotice}. + */ +@GtfsValidator +public class FareAttributeAgencyIdValidator extends FileValidator { + private final GtfsAgencyTableContainer agencyTable; + private final GtfsFareAttributeTableContainer attributeTable; + + @Inject + FareAttributeAgencyIdValidator( + GtfsAgencyTableContainer agencyTable, GtfsFareAttributeTableContainer attributeTable) { + this.agencyTable = agencyTable; + this.attributeTable = attributeTable; + } + + @Override + public void validate(NoticeContainer noticeContainer) { + + // routes.agency_id is required when there are multiple agencies + int totalAgencies = agencyTable.entityCount(); + + for (GtfsFareAttribute fare : attributeTable.getEntities()) { + if (!fare.hasAgencyId()) { + if (totalAgencies > 1) { + // add error notice if more than one agency + noticeContainer.addValidationNotice( + new MissingRequiredFieldNotice( + attributeTable.gtfsFilename(), + fare.csvRowNumber(), + GtfsFareAttribute.AGENCY_ID_FIELD_NAME)); + } else { + // add warning notice if only one agency + noticeContainer.addValidationNotice( + new MissingRecommendedFieldNotice( + attributeTable.gtfsFilename(), + fare.csvRowNumber(), + GtfsFareAttribute.AGENCY_ID_FIELD_NAME)); + } + } + } + // No need to check reference integrity because it is done by a validator generated from + // @ForeignKey annotation. + } +} diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidator.java index 8ffa85a494..b3fdbf118e 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidator.java @@ -23,7 +23,7 @@ /** * Checks that agency_id field in "routes.txt" is defined for every row if there is more than 1 - * agency in the feed. + * agency in the feed, recommended if only 1 agency. * *

Generated notice: {@link MissingRequiredFieldNotice}. * diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/FareAttributeAgencyIdValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/FareAttributeAgencyIdValidatorTest.java new file mode 100644 index 0000000000..6c0b23e935 --- /dev/null +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/FareAttributeAgencyIdValidatorTest.java @@ -0,0 +1,70 @@ +package org.mobilitydata.gtfsvalidator.validator; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableList; +import org.junit.Test; +import org.mobilitydata.gtfsvalidator.notice.MissingRecommendedFieldNotice; +import org.mobilitydata.gtfsvalidator.notice.MissingRequiredFieldNotice; +import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsAgency; +import org.mobilitydata.gtfsvalidator.table.GtfsAgencyTableContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsFareAttribute; +import org.mobilitydata.gtfsvalidator.table.GtfsFareAttributeTableContainer; + +public class FareAttributeAgencyIdValidatorTest { + + public static GtfsFareAttribute createFare(int rowNumber, String fareId, String agencyId) { + + return new GtfsFareAttribute.Builder() + .setCsvRowNumber(rowNumber) + .setAgencyId(agencyId) + .setFareId(fareId) + .build(); + } + + public static GtfsAgency createAgency(int csvRowNumber, String agencyId, String agencyName) { + return new GtfsAgency.Builder() + .setCsvRowNumber(csvRowNumber) + .setAgencyId(agencyId) + .setAgencyName(agencyName) + .build(); + } + + @Test + public void agencyIdRequiredErrorWhenMoreThanOneAgency() { + + NoticeContainer noticeContainer = new NoticeContainer(); + GtfsAgencyTableContainer agencyTable = + GtfsAgencyTableContainer.forEntities( + ImmutableList.of( + createAgency(0, "Agency 1", "Agency 1"), createAgency(1, "Agency 2", "Agency 2")), + noticeContainer); + GtfsFareAttributeTableContainer fareTable = + GtfsFareAttributeTableContainer.forEntities( + ImmutableList.of(createFare(0, "fare 0", "agency0"), createFare(1, "fare_1", null)), + noticeContainer); + new FareAttributeAgencyIdValidator(agencyTable, fareTable).validate(noticeContainer); + assertThat(noticeContainer.getValidationNotices()) + .containsExactly( + new MissingRequiredFieldNotice( + fareTable.gtfsFilename(), 1, GtfsFareAttribute.AGENCY_ID_FIELD_NAME)); + } + + @Test + public void agencyIdRecommendedWarningWhenOnlyOneAgency() { + + NoticeContainer noticeContainer = new NoticeContainer(); + GtfsAgencyTableContainer agencyTable = + GtfsAgencyTableContainer.forEntities( + ImmutableList.of(createAgency(1, null, "Agency with no ID")), noticeContainer); + GtfsFareAttributeTableContainer fareTable = + GtfsFareAttributeTableContainer.forEntities( + ImmutableList.of(createFare(0, "fare_0", null)), noticeContainer); + new FareAttributeAgencyIdValidator(agencyTable, fareTable).validate(noticeContainer); + assertThat(noticeContainer.getValidationNotices()) + .containsExactly( + new MissingRecommendedFieldNotice( + fareTable.gtfsFilename(), 0, GtfsFareAttribute.AGENCY_ID_FIELD_NAME)); + } +} diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidatorTest.java index 522c569173..fb79e69800 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidatorTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidatorTest.java @@ -37,8 +37,7 @@ public void agencyIdRequiredErrorWhenMoreThanOneAgency() { GtfsAgencyTableContainer agencyTable = GtfsAgencyTableContainer.forEntities( ImmutableList.of( - createAgency(0, "oneAgencyId", "Agency with Id"), - createAgency(1, null, "Agency with no ID")), + createAgency(0, "Agency 1", "Agency 1"), createAgency(1, "Agency 2", "Agency 2")), noticeContainer); GtfsRouteTableContainer routeTable = GtfsRouteTableContainer.forEntities( From e598d5431c896f55e1bffd0abc453d20a09e2321 Mon Sep 17 00:00:00 2001 From: Brian Donahue Date: Wed, 25 Jan 2023 14:52:46 -0500 Subject: [PATCH 4/9] formatting --- .../gtfsvalidator/validator/AgencyConsistencyValidator.java | 3 ++- .../validator/AgencyConsistencyValidatorTest.java | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidator.java index f75ea79b16..23693f0107 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidator.java @@ -54,7 +54,8 @@ public void validate(NoticeContainer noticeContainer) { // agency_id is recommended even when there is only 1 agency if (!agency.hasAgencyId()) { noticeContainer.addValidationNotice( - new MissingRecommendedFieldNotice(agencyTable.gtfsFilename(), agency.csvRowNumber(), agency.agencyName())); + new MissingRecommendedFieldNotice( + agencyTable.gtfsFilename(), agency.csvRowNumber(), agency.agencyName())); } // no further validation required return; diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidatorTest.java index d90660aecd..01139798ac 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidatorTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidatorTest.java @@ -72,8 +72,7 @@ public void singleAgencyPresentButNoAgencyIdSetShouldGenerateWarningNotice() { ZoneId.of("America/Montreal"), Locale.CANADA))); assertThat(notices) - .containsExactly( - new MissingRecommendedFieldNotice("agency.txt", 0, "agency name")); + .containsExactly(new MissingRecommendedFieldNotice("agency.txt", 0, "agency name")); assertThat(notices.get(0).getSeverityLevel()).isEqualTo(SeverityLevel.WARNING); } From 8d9600409b16634f1c514fe944f5389488a12c6f Mon Sep 17 00:00:00 2001 From: Brian Donahue Date: Wed, 25 Jan 2023 14:53:17 -0500 Subject: [PATCH 5/9] Update references for missing_recommended_field warning --- RULES.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/RULES.md b/RULES.md index 261279483e..59af2bc8e8 100644 --- a/RULES.md +++ b/RULES.md @@ -1994,6 +1994,8 @@ The given field has no value in some input row, even though values are recommend #### References * [feed_info.txt best practices](https://github.com/MobilityData/GTFS_Schedule_Best-Practices/blob/master/en/best-practices.md#feed_infotxt) +* [agency.txt best practices](https://github.com/MobilityData/GTFS_Schedule_Best-Practices/blob/master/en/best-practices.md#agencytxt) +* [fare_attributes.txt best practices](https://github.com/MobilityData/GTFS_Schedule_Best-Practices/blob/master/en/best-practices.md#fare_attributestxt)

#### Notice fields description From c338a65d86a3329a1f581baca66c0bb92f3d85c0 Mon Sep 17 00:00:00 2001 From: Brian Donahue Date: Fri, 10 Feb 2023 16:57:57 -0500 Subject: [PATCH 6/9] Account for possibility of 0 agencies --- .../validator/AgencyConsistencyValidator.java | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidator.java index 23693f0107..ad57b4ddbf 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidator.java @@ -49,7 +49,7 @@ public class AgencyConsistencyValidator extends FileValidator { @Override public void validate(NoticeContainer noticeContainer) { final int agencyCount = agencyTable.entityCount(); - if (agencyCount < 2) { + if (agencyCount == 1) { GtfsAgency agency = agencyTable.getEntities().get(0); // agency_id is recommended even when there is only 1 agency if (!agency.hasAgencyId()) { @@ -61,14 +61,16 @@ public void validate(NoticeContainer noticeContainer) { return; } - for (GtfsAgency agency : agencyTable.getEntities()) { - // agency_id is required when there are 2 or more agencies. - if (!agency.hasAgencyId()) { - noticeContainer.addValidationNotice( - new MissingRequiredFieldNotice( - agencyTable.gtfsFilename(), - agency.csvRowNumber(), - GtfsAgency.AGENCY_ID_FIELD_NAME)); + if(agencyCount > 1) { + for (GtfsAgency agency : agencyTable.getEntities()) { + // agency_id is required when there are 2 or more agencies. + if (!agency.hasAgencyId()) { + noticeContainer.addValidationNotice( + new MissingRequiredFieldNotice( + agencyTable.gtfsFilename(), + agency.csvRowNumber(), + GtfsAgency.AGENCY_ID_FIELD_NAME)); + } } } From 70385ace8c494001d4b757e47383cffbb1a070e8 Mon Sep 17 00:00:00 2001 From: Brian Donahue Date: Fri, 10 Feb 2023 17:12:47 -0500 Subject: [PATCH 7/9] add no notice test cases for routes.txt --- .../validator/AgencyConsistencyValidator.java | 10 +- .../validator/RouteAgencyIdValidatorTest.java | 136 ++++++++++++++---- 2 files changed, 117 insertions(+), 29 deletions(-) diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidator.java index ad57b4ddbf..d8f340c984 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidator.java @@ -61,15 +61,15 @@ public void validate(NoticeContainer noticeContainer) { return; } - if(agencyCount > 1) { + if (agencyCount > 1) { for (GtfsAgency agency : agencyTable.getEntities()) { // agency_id is required when there are 2 or more agencies. if (!agency.hasAgencyId()) { noticeContainer.addValidationNotice( - new MissingRequiredFieldNotice( - agencyTable.gtfsFilename(), - agency.csvRowNumber(), - GtfsAgency.AGENCY_ID_FIELD_NAME)); + new MissingRequiredFieldNotice( + agencyTable.gtfsFilename(), + agency.csvRowNumber(), + GtfsAgency.AGENCY_ID_FIELD_NAME)); } } } diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidatorTest.java index fb79e69800..a7c090ad8e 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidatorTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidatorTest.java @@ -11,25 +11,6 @@ public class RouteAgencyIdValidatorTest { - public static GtfsRoute createRoute( - int rowNumber, String routeId, String agencyId, String shortName) { - - return new GtfsRoute.Builder() - .setCsvRowNumber(rowNumber) - .setAgencyId(agencyId) - .setRouteId(routeId) - .setRouteShortName(shortName) - .build(); - } - - public static GtfsAgency createAgency(int csvRowNumber, String agencyId, String agencyName) { - return new GtfsAgency.Builder() - .setCsvRowNumber(csvRowNumber) - .setAgencyId(agencyId) - .setAgencyName(agencyName) - .build(); - } - @Test public void agencyIdRequiredErrorWhenMoreThanOneAgency() { @@ -37,13 +18,33 @@ public void agencyIdRequiredErrorWhenMoreThanOneAgency() { GtfsAgencyTableContainer agencyTable = GtfsAgencyTableContainer.forEntities( ImmutableList.of( - createAgency(0, "Agency 1", "Agency 1"), createAgency(1, "Agency 2", "Agency 2")), + new GtfsAgency.Builder() + .setCsvRowNumber(0) + .setAgencyId("Agency 1") + .setAgencyName("Agency 1") + .build(), + new GtfsAgency.Builder() + .setCsvRowNumber(1) + .setAgencyId("Agency 2") + .setAgencyName("Agency 2") + .build()), noticeContainer); + GtfsRouteTableContainer routeTable = GtfsRouteTableContainer.forEntities( ImmutableList.of( - createRoute(0, "route_0", "agency0", "Route 0"), - createRoute(1, "route_1", null, "Route 1")), + new GtfsRoute.Builder() + .setCsvRowNumber(0) + .setAgencyId("agency0") + .setRouteId("route_0") + .setRouteShortName("Route 0") + .build(), + new GtfsRoute.Builder() + .setCsvRowNumber(1) + .setAgencyId(null) + .setRouteId("route_1") + .setRouteShortName("Route 1") + .build()), noticeContainer); new RouteAgencyIdValidator(agencyTable, routeTable).validate(noticeContainer); assertThat(noticeContainer.getValidationNotices()) @@ -58,14 +59,101 @@ public void agencyIdRecommendedWarningWhenOnlyOneAgency() { NoticeContainer noticeContainer = new NoticeContainer(); GtfsAgencyTableContainer agencyTable = GtfsAgencyTableContainer.forEntities( - ImmutableList.of(createAgency(1, null, "Agency with no ID")), noticeContainer); + ImmutableList.of( + new GtfsAgency.Builder() + .setCsvRowNumber(1) + .setAgencyId(null) + .setAgencyName("Agency with no ID") + .build()), + noticeContainer); + GtfsRouteTableContainer routeTable = GtfsRouteTableContainer.forEntities( - ImmutableList.of(createRoute(0, "route_0", null, "Route 0")), noticeContainer); + ImmutableList.of( + new GtfsRoute.Builder() + .setCsvRowNumber(0) + .setAgencyId(null) + .setRouteId("route_0") + .setRouteShortName("Route 0") + .build()), + noticeContainer); new RouteAgencyIdValidator(agencyTable, routeTable).validate(noticeContainer); assertThat(noticeContainer.getValidationNotices()) .containsExactly( new MissingRecommendedFieldNotice( routeTable.gtfsFilename(), 0, GtfsRoute.AGENCY_ID_FIELD_NAME)); } + + @Test + public void SingleAgencyAndAgencyIdSpecified_noNotice() { + + NoticeContainer noticeContainer = new NoticeContainer(); + GtfsAgencyTableContainer agencyTable = + GtfsAgencyTableContainer.forEntities( + ImmutableList.of( + new GtfsAgency.Builder() + .setCsvRowNumber(0) + .setAgencyId("agency1") + .setAgencyName("Agency 1") + .build()), + noticeContainer); + + GtfsRouteTableContainer routeTable = + GtfsRouteTableContainer.forEntities( + ImmutableList.of( + new GtfsRoute.Builder() + .setCsvRowNumber(0) + .setAgencyId("agency1") + .setRouteId("route_0") + .setRouteShortName("Route 0") + .build(), + new GtfsRoute.Builder() + .setCsvRowNumber(1) + .setAgencyId("agency1") + .setRouteId("route_1") + .setRouteShortName("Route 1") + .build()), + noticeContainer); + new RouteAgencyIdValidator(agencyTable, routeTable).validate(noticeContainer); + assertThat(noticeContainer.getValidationNotices()).isEmpty(); + } + + @Test + public void MoreThanOneAgencyAndAgencyIdsSpecified_noNotice() { + + NoticeContainer noticeContainer = new NoticeContainer(); + GtfsAgencyTableContainer agencyTable = + GtfsAgencyTableContainer.forEntities( + ImmutableList.of( + new GtfsAgency.Builder() + .setCsvRowNumber(0) + .setAgencyId("agency1") + .setAgencyName("Agency 1") + .build(), + new GtfsAgency.Builder() + .setCsvRowNumber(1) + .setAgencyId("agency2") + .setAgencyName("Agency 2") + .build()), + noticeContainer); + + GtfsRouteTableContainer routeTable = + GtfsRouteTableContainer.forEntities( + ImmutableList.of( + new GtfsRoute.Builder() + .setCsvRowNumber(0) + .setAgencyId("agency1") + .setRouteId("route_0") + .setRouteShortName("Route 0") + .build(), + new GtfsRoute.Builder() + .setCsvRowNumber(1) + .setAgencyId("agency2") + .setRouteId("route_1") + .setRouteShortName("Route 1") + .build()), + noticeContainer); + new RouteAgencyIdValidator(agencyTable, routeTable).validate(noticeContainer); + assertThat(noticeContainer.getValidationNotices()).isEmpty(); + } } From 05a725deb14c82e7fe0e808023e564e7ab5cc3b8 Mon Sep 17 00:00:00 2001 From: Brian Donahue Date: Fri, 10 Feb 2023 17:16:32 -0500 Subject: [PATCH 8/9] inline method --- .../FareAttributeAgencyIdValidatorTest.java | 58 ++++++++++++------- 1 file changed, 37 insertions(+), 21 deletions(-) diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/FareAttributeAgencyIdValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/FareAttributeAgencyIdValidatorTest.java index 6c0b23e935..9e42e8d8fc 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/FareAttributeAgencyIdValidatorTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/FareAttributeAgencyIdValidatorTest.java @@ -14,23 +14,6 @@ public class FareAttributeAgencyIdValidatorTest { - public static GtfsFareAttribute createFare(int rowNumber, String fareId, String agencyId) { - - return new GtfsFareAttribute.Builder() - .setCsvRowNumber(rowNumber) - .setAgencyId(agencyId) - .setFareId(fareId) - .build(); - } - - public static GtfsAgency createAgency(int csvRowNumber, String agencyId, String agencyName) { - return new GtfsAgency.Builder() - .setCsvRowNumber(csvRowNumber) - .setAgencyId(agencyId) - .setAgencyName(agencyName) - .build(); - } - @Test public void agencyIdRequiredErrorWhenMoreThanOneAgency() { @@ -38,11 +21,31 @@ public void agencyIdRequiredErrorWhenMoreThanOneAgency() { GtfsAgencyTableContainer agencyTable = GtfsAgencyTableContainer.forEntities( ImmutableList.of( - createAgency(0, "Agency 1", "Agency 1"), createAgency(1, "Agency 2", "Agency 2")), + new GtfsAgency.Builder() + .setCsvRowNumber(0) + .setAgencyId("Agency 1") + .setAgencyName("Agency 1") + .build(), + new GtfsAgency.Builder() + .setCsvRowNumber(1) + .setAgencyId("Agency 2") + .setAgencyName("Agency 2") + .build()), noticeContainer); + GtfsFareAttributeTableContainer fareTable = GtfsFareAttributeTableContainer.forEntities( - ImmutableList.of(createFare(0, "fare 0", "agency0"), createFare(1, "fare_1", null)), + ImmutableList.of( + new GtfsFareAttribute.Builder() + .setCsvRowNumber(0) + .setAgencyId("agency0") + .setFareId("fare 0") + .build(), + new GtfsFareAttribute.Builder() + .setCsvRowNumber(1) + .setAgencyId(null) + .setFareId("fare_1") + .build()), noticeContainer); new FareAttributeAgencyIdValidator(agencyTable, fareTable).validate(noticeContainer); assertThat(noticeContainer.getValidationNotices()) @@ -57,10 +60,23 @@ public void agencyIdRecommendedWarningWhenOnlyOneAgency() { NoticeContainer noticeContainer = new NoticeContainer(); GtfsAgencyTableContainer agencyTable = GtfsAgencyTableContainer.forEntities( - ImmutableList.of(createAgency(1, null, "Agency with no ID")), noticeContainer); + ImmutableList.of( + new GtfsAgency.Builder() + .setCsvRowNumber(1) + .setAgencyId(null) + .setAgencyName("Agency with no ID") + .build()), + noticeContainer); + GtfsFareAttributeTableContainer fareTable = GtfsFareAttributeTableContainer.forEntities( - ImmutableList.of(createFare(0, "fare_0", null)), noticeContainer); + ImmutableList.of( + new GtfsFareAttribute.Builder() + .setCsvRowNumber(0) + .setAgencyId(null) + .setFareId("fare_0") + .build()), + noticeContainer); new FareAttributeAgencyIdValidator(agencyTable, fareTable).validate(noticeContainer); assertThat(noticeContainer.getValidationNotices()) .containsExactly( From 87ca9478363a77a6aa16fe513df43402a1130c01 Mon Sep 17 00:00:00 2001 From: Brian Donahue Date: Fri, 10 Feb 2023 17:23:20 -0500 Subject: [PATCH 9/9] add no notice test cases for fare_attributes.txt --- .../FareAttributeAgencyIdValidatorTest.java | 75 ++++++++++++++++++- 1 file changed, 72 insertions(+), 3 deletions(-) diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/FareAttributeAgencyIdValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/FareAttributeAgencyIdValidatorTest.java index 9e42e8d8fc..e80aa8fb22 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/FareAttributeAgencyIdValidatorTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/FareAttributeAgencyIdValidatorTest.java @@ -23,12 +23,12 @@ public void agencyIdRequiredErrorWhenMoreThanOneAgency() { ImmutableList.of( new GtfsAgency.Builder() .setCsvRowNumber(0) - .setAgencyId("Agency 1") + .setAgencyId("agency1") .setAgencyName("Agency 1") .build(), new GtfsAgency.Builder() .setCsvRowNumber(1) - .setAgencyId("Agency 2") + .setAgencyId("agency2") .setAgencyName("Agency 2") .build()), noticeContainer); @@ -38,7 +38,7 @@ public void agencyIdRequiredErrorWhenMoreThanOneAgency() { ImmutableList.of( new GtfsFareAttribute.Builder() .setCsvRowNumber(0) - .setAgencyId("agency0") + .setAgencyId("agency1") .setFareId("fare 0") .build(), new GtfsFareAttribute.Builder() @@ -83,4 +83,73 @@ public void agencyIdRecommendedWarningWhenOnlyOneAgency() { new MissingRecommendedFieldNotice( fareTable.gtfsFilename(), 0, GtfsFareAttribute.AGENCY_ID_FIELD_NAME)); } + + @Test + public void WhenMoreThanOneAgencyAndAgencyIdsSpecified_noNotice() { + + NoticeContainer noticeContainer = new NoticeContainer(); + GtfsAgencyTableContainer agencyTable = + GtfsAgencyTableContainer.forEntities( + ImmutableList.of( + new GtfsAgency.Builder() + .setCsvRowNumber(0) + .setAgencyId("agency1") + .setAgencyName("Agency 1") + .build(), + new GtfsAgency.Builder() + .setCsvRowNumber(1) + .setAgencyId("Agency 2") + .setAgencyName("Agency 2") + .build()), + noticeContainer); + + GtfsFareAttributeTableContainer fareTable = + GtfsFareAttributeTableContainer.forEntities( + ImmutableList.of( + new GtfsFareAttribute.Builder() + .setCsvRowNumber(0) + .setAgencyId("agency1") + .setFareId("fare 0") + .build(), + new GtfsFareAttribute.Builder() + .setCsvRowNumber(1) + .setAgencyId("agency2") + .setFareId("fare_1") + .build()), + noticeContainer); + new FareAttributeAgencyIdValidator(agencyTable, fareTable).validate(noticeContainer); + assertThat(noticeContainer.getValidationNotices()).isEmpty(); + } + + @Test + public void WhenMoreSingleAgencyAndAgencyIdSpecified_noNotice() { + + NoticeContainer noticeContainer = new NoticeContainer(); + GtfsAgencyTableContainer agencyTable = + GtfsAgencyTableContainer.forEntities( + ImmutableList.of( + new GtfsAgency.Builder() + .setCsvRowNumber(0) + .setAgencyId("agency1") + .setAgencyName("Agency 1") + .build()), + noticeContainer); + + GtfsFareAttributeTableContainer fareTable = + GtfsFareAttributeTableContainer.forEntities( + ImmutableList.of( + new GtfsFareAttribute.Builder() + .setCsvRowNumber(0) + .setAgencyId("agency1") + .setFareId("fare 0") + .build(), + new GtfsFareAttribute.Builder() + .setCsvRowNumber(1) + .setAgencyId("agency1") + .setFareId("fare_1") + .build()), + noticeContainer); + new FareAttributeAgencyIdValidator(agencyTable, fareTable).validate(noticeContainer); + assertThat(noticeContainer.getValidationNotices()).isEmpty(); + } }