diff --git a/RULES.md b/RULES.md index 91c390f00d..03604e97f3 100644 --- a/RULES.md +++ b/RULES.md @@ -1990,6 +1990,8 @@ The given field has no value in some input row, even though values are recommend #### References * [feed_info.txt best practices](https://gtfs.org/schedule/best-practices/#feed_infotxt) +* [agency.txt best practices](https://gtfs.org/schedule/best-practices/#agencytxt) +* [fare_attributes.txt best practices](https://gtfs.org/schedule/best-practices/#fare_attributestxt)
#### Notice fields description 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..d8f340c984 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 @@ * * @@ -51,18 +49,28 @@ 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()) { + noticeContainer.addValidationNotice( + new MissingRecommendedFieldNotice( + agencyTable.gtfsFilename(), agency.csvRowNumber(), agency.agencyName())); + } + // no further validation required 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)); + } } } 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/TripAgencyIdValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidator.java similarity index 52% rename from main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TripAgencyIdValidator.java rename to main/src/main/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidator.java index f017269f8b..b3fdbf118e 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TripAgencyIdValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidator.java @@ -18,43 +18,50 @@ 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; +import org.mobilitydata.gtfsvalidator.notice.*; +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. + * agency in the feed, recommended if only 1 agency. * *

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

Generated notice: {@link MissingRecommendedFieldNotice}. */ @GtfsValidator -public class TripAgencyIdValidator extends FileValidator { +public class RouteAgencyIdValidator extends FileValidator { private final GtfsAgencyTableContainer agencyTable; private final GtfsRouteTableContainer routeTable; @Inject - TripAgencyIdValidator(GtfsAgencyTableContainer agencyTable, GtfsRouteTableContainer routeTable) { + RouteAgencyIdValidator(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; - } + + // routes.agency_id is required when there are multiple agencies + int totalAgencies = agencyTable.entityCount(); + for (GtfsRoute route : routeTable.getEntities()) { if (!route.hasAgencyId()) { - noticeContainer.addValidationNotice( - new MissingRequiredFieldNotice( - routeTable.gtfsFilename(), route.csvRowNumber(), GtfsRoute.AGENCY_ID_FIELD_NAME)); + 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 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. } + // 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..01139798ac 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidatorTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/AgencyConsistencyValidatorTest.java @@ -26,9 +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.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; @@ -62,25 +60,44 @@ 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 MissingRecommendedFieldNotice("agency.txt", 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/FareAttributeAgencyIdValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/FareAttributeAgencyIdValidatorTest.java new file mode 100644 index 0000000000..e80aa8fb22 --- /dev/null +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/FareAttributeAgencyIdValidatorTest.java @@ -0,0 +1,155 @@ +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 { + + @Test + public void agencyIdRequiredErrorWhenMoreThanOneAgency() { + + 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); + + GtfsFareAttributeTableContainer fareTable = + GtfsFareAttributeTableContainer.forEntities( + ImmutableList.of( + new GtfsFareAttribute.Builder() + .setCsvRowNumber(0) + .setAgencyId("agency1") + .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()) + .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( + new GtfsAgency.Builder() + .setCsvRowNumber(1) + .setAgencyId(null) + .setAgencyName("Agency with no ID") + .build()), + noticeContainer); + + GtfsFareAttributeTableContainer fareTable = + GtfsFareAttributeTableContainer.forEntities( + ImmutableList.of( + new GtfsFareAttribute.Builder() + .setCsvRowNumber(0) + .setAgencyId(null) + .setFareId("fare_0") + .build()), + noticeContainer); + new FareAttributeAgencyIdValidator(agencyTable, fareTable).validate(noticeContainer); + assertThat(noticeContainer.getValidationNotices()) + .containsExactly( + 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(); + } +} 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..a7c090ad8e --- /dev/null +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/RouteAgencyIdValidatorTest.java @@ -0,0 +1,159 @@ +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.*; + +public class RouteAgencyIdValidatorTest { + + @Test + public void agencyIdRequiredErrorWhenMoreThanOneAgency() { + + NoticeContainer noticeContainer = new NoticeContainer(); + GtfsAgencyTableContainer agencyTable = + GtfsAgencyTableContainer.forEntities( + ImmutableList.of( + 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( + 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()) + .containsExactly( + new MissingRequiredFieldNotice( + routeTable.gtfsFilename(), 1, GtfsRoute.AGENCY_ID_FIELD_NAME)); + } + + @Test + public void agencyIdRecommendedWarningWhenOnlyOneAgency() { + + NoticeContainer noticeContainer = new NoticeContainer(); + GtfsAgencyTableContainer agencyTable = + GtfsAgencyTableContainer.forEntities( + ImmutableList.of( + new GtfsAgency.Builder() + .setCsvRowNumber(1) + .setAgencyId(null) + .setAgencyName("Agency with no ID") + .build()), + noticeContainer); + + GtfsRouteTableContainer routeTable = + GtfsRouteTableContainer.forEntities( + 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(); + } +}