From b19a2e65f9b0e8b03b667da86228f870ab728d0d Mon Sep 17 00:00:00 2001 From: jcpitre <106176106+jcpitre@users.noreply.github.com> Date: Wed, 28 Aug 2024 15:54:39 +0200 Subject: [PATCH] fix: Added a validator for bad networkId foreign key in fareLegRules (#1804) Added a custom validator for networkId foreign key that depends on 2 files. --- .../table/GtfsFareLegRuleSchema.java | 8 +- ...reLegRuleNetworkIdForeignKeyValidator.java | 80 ++++++++++++++ ...gRuleNetworkIdForeignKeyValidatorTest.java | 103 ++++++++++++++++++ 3 files changed, 189 insertions(+), 2 deletions(-) create mode 100644 main/src/main/java/org/mobilitydata/gtfsvalidator/validator/GtfsFareLegRuleNetworkIdForeignKeyValidator.java create mode 100644 main/src/test/java/org/mobilitydata/gtfsvalidator/validator/GtfsFareLegRuleNetworkIdForeignKeyValidatorTest.java diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsFareLegRuleSchema.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsFareLegRuleSchema.java index ef871fe875..192b3171eb 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsFareLegRuleSchema.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsFareLegRuleSchema.java @@ -32,9 +32,13 @@ public interface GtfsFareLegRuleSchema extends GtfsEntity { @Index String legGroupId(); + /** + * There is actually a foreign key validation on this field, but since it needs to check 2 files + * to establish the presence of the key a custom validator was written. See {@link + * org.mobilitydata.gtfsvalidator.annotation.ForeignKey} and {@link + * org.mobilitydata.gtfsvalidator.validator.GtfsFareLegRuleNetworkIdForeignKeyValidator}. + */ @FieldType(FieldTypeEnum.ID) - @PrimaryKey(translationRecordIdType = UNSUPPORTED) - @ForeignKey(table = "routes.txt", field = "network_id") String networkId(); @FieldType(FieldTypeEnum.ID) diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/GtfsFareLegRuleNetworkIdForeignKeyValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/GtfsFareLegRuleNetworkIdForeignKeyValidator.java new file mode 100644 index 0000000000..0f05a1dc4e --- /dev/null +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/GtfsFareLegRuleNetworkIdForeignKeyValidator.java @@ -0,0 +1,80 @@ +/* + * 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.ForeignKeyViolationNotice; +import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsFareLegRule; +import org.mobilitydata.gtfsvalidator.table.GtfsFareLegRuleTableContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsNetwork; +import org.mobilitydata.gtfsvalidator.table.GtfsNetworkTableContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsRoute; +import org.mobilitydata.gtfsvalidator.table.GtfsRouteTableContainer; + +/** + * Validates that network_id field in "fare_leg_rules.txt" references a valid network_id in + * "routes.txt" or "networks.txt". + * + *

Generated notice: {@link ForeignKeyViolationNotice}. + */ +@GtfsValidator +public class GtfsFareLegRuleNetworkIdForeignKeyValidator extends FileValidator { + private final GtfsRouteTableContainer routeParentContainer; + private final GtfsNetworkTableContainer networkParentContainer; + + private final GtfsFareLegRuleTableContainer fareLegRuleChildContainer; + + @Inject + GtfsFareLegRuleNetworkIdForeignKeyValidator( + GtfsFareLegRuleTableContainer fareLegRuleChildContainer, + GtfsRouteTableContainer routeParentContainer, + GtfsNetworkTableContainer networkParentContainer) { + this.fareLegRuleChildContainer = fareLegRuleChildContainer; + this.routeParentContainer = routeParentContainer; + this.networkParentContainer = networkParentContainer; + } + + @Override + public void validate(NoticeContainer noticeContainer) { + for (GtfsFareLegRule fareLegRule : fareLegRuleChildContainer.getEntities()) { + String foreignKey = fareLegRule.networkId(); + if (foreignKey.isEmpty()) { + continue; + } + if (!hasReferencedKey(foreignKey, routeParentContainer, networkParentContainer)) { + noticeContainer.addValidationNotice( + new ForeignKeyViolationNotice( + GtfsFareLegRule.FILENAME, + GtfsFareLegRule.NETWORK_ID_FIELD_NAME, + GtfsRoute.FILENAME + " or " + GtfsNetwork.FILENAME, + GtfsFareLegRule.NETWORK_ID_FIELD_NAME, + foreignKey, + fareLegRule.csvRowNumber())); + } + } + } + + private boolean hasReferencedKey( + String foreignKey, + GtfsRouteTableContainer routeContainer, + GtfsNetworkTableContainer networkContainer) { + return !(routeContainer.byNetworkId(foreignKey).isEmpty() + && networkContainer.byNetworkId(foreignKey).isEmpty()); + } +} diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/GtfsFareLegRuleNetworkIdForeignKeyValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/GtfsFareLegRuleNetworkIdForeignKeyValidatorTest.java new file mode 100644 index 0000000000..611a58bbb6 --- /dev/null +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/GtfsFareLegRuleNetworkIdForeignKeyValidatorTest.java @@ -0,0 +1,103 @@ +/* + * Copyright 2020 Google LLC, MobilityData IO + * + * 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 static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableList; +import java.util.List; +import org.junit.Test; +import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; +import org.mobilitydata.gtfsvalidator.notice.ValidationNotice; +import org.mobilitydata.gtfsvalidator.table.GtfsFareLegRule; +import org.mobilitydata.gtfsvalidator.table.GtfsFareLegRuleTableContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsNetwork; +import org.mobilitydata.gtfsvalidator.table.GtfsNetworkTableContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsRoute; +import org.mobilitydata.gtfsvalidator.table.GtfsRouteTableContainer; + +public class GtfsFareLegRuleNetworkIdForeignKeyValidatorTest { + + public static GtfsFareLegRule createFareLegRule(String networkId) { + return new GtfsFareLegRule.Builder().setCsvRowNumber(1).setNetworkId(networkId).build(); + } + + private static GtfsRoute createRoute(String networkId) { + return new GtfsRoute.Builder() + .setCsvRowNumber(1) + .setRouteId("testRoute") + .setRouteType(3) // Bus, but could be other type + .setNetworkId(networkId) + .build(); + } + + private static GtfsNetwork createNetwork(String networkId) { + return new GtfsNetwork.Builder().setCsvRowNumber(1).setNetworkId(networkId).build(); + } + + private static List generateNotices( + GtfsFareLegRule fareLegRule, GtfsRoute route, GtfsNetwork network) { + NoticeContainer noticeContainer = new NoticeContainer(); + new GtfsFareLegRuleNetworkIdForeignKeyValidator( + GtfsFareLegRuleTableContainer.forEntities( + ImmutableList.of(fareLegRule), noticeContainer), + GtfsRouteTableContainer.forEntities(ImmutableList.of(route), noticeContainer), + GtfsNetworkTableContainer.forEntities(ImmutableList.of(network), noticeContainer)) + .validate(noticeContainer); + return noticeContainer.getValidationNotices(); + } + + @Test + public void networkIdNotInRouteOrNetworkShouldGenerateNotice() { + List notices = + generateNotices( + createFareLegRule("testNetworkId"), + createRoute("otherNetworkId"), + createNetwork("otherNetworkId")); + assertThat(notices).isNotEmpty(); + } + + @Test + public void networkIdInRouteButNotInNetworkShouldNotGenerateNotice() { + List notices = + generateNotices( + createFareLegRule("testNetworkId"), + createRoute("testNetworkId"), + createNetwork("otherNetworkId")); + assertThat(notices).isEmpty(); + } + + @Test + public void networkIdNotInRouteButInNetworkShouldNotGenerateNotice() { + List notices = + generateNotices( + createFareLegRule("testNetworkId"), + createRoute("otherNetworkId"), + createNetwork("testNetworkId")); + assertThat(notices).isEmpty(); + } + + @Test + public void networkIdInBothRouteAndNetworkShouldNotGenerateNotice() { + List notices = + generateNotices( + createFareLegRule("testNetworkId"), + createRoute("testNetworkId"), + createNetwork("testNetworkId")); + assertThat(notices).isEmpty(); + } +}