From e96182de8ff457def7058e372743ddd9c95b2944 Mon Sep 17 00:00:00 2001 From: Brian Ferris Date: Mon, 3 Oct 2022 13:20:59 -0700 Subject: [PATCH 1/4] Add validation check for conditionally required/forbidden `fare_rules.txt` file, per Fares v2 spec update. --- RULES.md | 21 ++++++ .../ConditionallyForbiddenFileNotice.java | 31 +++++++++ .../validator/FaresV1FileValidator.java | 33 ++++++++++ .../validator/FaresV1FileValidatorTest.java | 65 +++++++++++++++++++ 4 files changed, 150 insertions(+) create mode 100644 core/src/main/java/org/mobilitydata/gtfsvalidator/notice/ConditionallyForbiddenFileNotice.java create mode 100644 main/src/main/java/org/mobilitydata/gtfsvalidator/validator/FaresV1FileValidator.java create mode 100644 main/src/test/java/org/mobilitydata/gtfsvalidator/validator/FaresV1FileValidatorTest.java diff --git a/RULES.md b/RULES.md index 0d0ceeb36f..96272bca4c 100644 --- a/RULES.md +++ b/RULES.md @@ -32,6 +32,7 @@ Each Notice is associated with a severity: `INFO`, `WARNING`, `ERROR`. | Notice code | Description | |-------------------------------------------------------------------------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------| | [`block_trips_with_overlapping_stop_times`](#block_trips_with_overlapping_stop_times) | Block trips with overlapping stop times. | +| [`conditionally_forbidden_file`](#conditionally_forbidden_file) | A conditionally forbidden file is present in the feed. | | [`csv_parsing_failed`](#csv_parsing_failed) | Parsing of a CSV file failed. | | [`decreasing_shape_distance`](#decreasing_shape_distance) | Decreasing `shape_dist_traveled` in `shapes.txt`. | | [`decreasing_or_equal_stop_time_distance`](#decreasing_or_equal_stop_time_distance) | Decreasing or equal `shape_dist_traveled` in `stop_times.txt`. | @@ -184,6 +185,26 @@ Trips with the same block id have overlapping stop times. + + +### conditionally_forbidden_file + +A conditionally forbidden file is present in the feed. + +#### References +* [GTFS terms definition](https://gtfs.org/reference/static/#term-definitions) +
+ +#### Notice fields description +| Field name | Description | Type | +|-------------- |-------------------------------- |-------- | +| `filename` | The name of the forbidden file. | String | + +#### Affected files +[All GTFS files supported by the specification.](http://gtfs.org/reference/static#dataset-files) + +
+ ### csv_parsing_failed diff --git a/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/ConditionallyForbiddenFileNotice.java b/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/ConditionallyForbiddenFileNotice.java new file mode 100644 index 0000000000..d1ca4bb138 --- /dev/null +++ b/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/ConditionallyForbiddenFileNotice.java @@ -0,0 +1,31 @@ +/* + * 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.notice; + +/** + * A conditionally forbidden file is present in the feed. + * + *

Severity: {@code SeverityLevel.ERROR} + */ +public class ConditionallyForbiddenFileNotice extends ValidationNotice { + private final String filename; + + public ConditionallyForbiddenFileNotice(String filename) { + super(SeverityLevel.ERROR); + this.filename = filename; + } +} diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/FaresV1FileValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/FaresV1FileValidator.java new file mode 100644 index 0000000000..a9d8a8cfa1 --- /dev/null +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/FaresV1FileValidator.java @@ -0,0 +1,33 @@ +package org.mobilitydata.gtfsvalidator.validator; + +import javax.inject.Inject; +import org.mobilitydata.gtfsvalidator.notice.ConditionallyForbiddenFileNotice; +import org.mobilitydata.gtfsvalidator.notice.MissingRequiredFileNotice; +import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsFareAttributeTableContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsFareRuleTableContainer; + +public class FaresV1FileValidator extends FileValidator { + + private final GtfsFareAttributeTableContainer fareAttributeTable; + private final GtfsFareRuleTableContainer fareRuleTable; + + @Inject + public FaresV1FileValidator( + GtfsFareAttributeTableContainer fareAttributeTable, + GtfsFareRuleTableContainer fareRuleTable) { + this.fareAttributeTable = fareAttributeTable; + this.fareRuleTable = fareRuleTable; + } + + @Override + public void validate(NoticeContainer noticeContainer) { + if (!fareAttributeTable.isMissingFile() && fareRuleTable.isMissingFile()) { + noticeContainer.addValidationNotice( + new MissingRequiredFileNotice(fareRuleTable.gtfsFilename())); + } else if (fareAttributeTable.isMissingFile() && !fareRuleTable.isMissingFile()) { + noticeContainer.addValidationNotice( + new ConditionallyForbiddenFileNotice(fareRuleTable.gtfsFilename())); + } + } +} diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/FaresV1FileValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/FaresV1FileValidatorTest.java new file mode 100644 index 0000000000..7b18964960 --- /dev/null +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/FaresV1FileValidatorTest.java @@ -0,0 +1,65 @@ +package org.mobilitydata.gtfsvalidator.validator; + +import static com.google.common.truth.Truth.assertThat; + +import junit.framework.TestCase; +import org.junit.Test; +import org.mobilitydata.gtfsvalidator.notice.ConditionallyForbiddenFileNotice; +import org.mobilitydata.gtfsvalidator.notice.MissingRequiredFileNotice; +import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsFareAttributeTableContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsFareRuleTableContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsTableContainer.TableStatus; + +public class FaresV1FileValidatorTest extends TestCase { + + private NoticeContainer noticeContainer = new NoticeContainer(); + + @Test + public void testFareAttributesPresentAndFareRulesPresent() { + new FaresV1FileValidator( + new GtfsFareAttributeTableContainer(TableStatus.PARSABLE_HEADERS_AND_ROWS), + new GtfsFareRuleTableContainer(TableStatus.PARSABLE_HEADERS_AND_ROWS)) + .validate(noticeContainer); + assertThat(noticeContainer.getValidationNotices()).isEmpty(); + } + + @Test + public void testFareAttributesPresentAndFareRulesPresentByEmpty() { + // We still consider an empty file to be "present". + new FaresV1FileValidator( + new GtfsFareAttributeTableContainer(TableStatus.EMPTY_FILE), + new GtfsFareRuleTableContainer(TableStatus.EMPTY_FILE)) + .validate(noticeContainer); + assertThat(noticeContainer.getValidationNotices()).isEmpty(); + } + + @Test + public void testFareAttributesMissingAndFareRulesMissing() { + new FaresV1FileValidator( + new GtfsFareAttributeTableContainer(TableStatus.MISSING_FILE), + new GtfsFareRuleTableContainer(TableStatus.MISSING_FILE)) + .validate(noticeContainer); + assertThat(noticeContainer.getValidationNotices()).isEmpty(); + } + + @Test + public void testFareAttributesPresentButFareRulesMissing() { + new FaresV1FileValidator( + new GtfsFareAttributeTableContainer(TableStatus.PARSABLE_HEADERS_AND_ROWS), + new GtfsFareRuleTableContainer(TableStatus.MISSING_FILE)) + .validate(noticeContainer); + assertThat(noticeContainer.getValidationNotices()) + .containsExactly(new MissingRequiredFileNotice("fare_rules.txt")); + } + + @Test + public void testFareAttributesMissingButFareRulesPresent() { + new FaresV1FileValidator( + new GtfsFareAttributeTableContainer(TableStatus.MISSING_FILE), + new GtfsFareRuleTableContainer(TableStatus.PARSABLE_HEADERS_AND_ROWS)) + .validate(noticeContainer); + assertThat(noticeContainer.getValidationNotices()) + .containsExactly(new ConditionallyForbiddenFileNotice("fare_rules.txt")); + } +} From 7332d13aff173fd389fc79b3113837473292af0c Mon Sep 17 00:00:00 2001 From: Brian Ferris Date: Tue, 4 Oct 2022 08:24:36 -0700 Subject: [PATCH 2/4] ConditionallyForbiddenFileNotice => ForbiddenFileNotice --- RULES.md | 6 +++--- ...llyForbiddenFileNotice.java => ForbiddenFileNotice.java} | 4 ++-- .../gtfsvalidator/validator/FaresV1FileValidator.java | 5 ++--- .../gtfsvalidator/validator/FaresV1FileValidatorTest.java | 4 ++-- 4 files changed, 9 insertions(+), 10 deletions(-) rename core/src/main/java/org/mobilitydata/gtfsvalidator/notice/{ConditionallyForbiddenFileNotice.java => ForbiddenFileNotice.java} (86%) diff --git a/RULES.md b/RULES.md index 96272bca4c..3c56b38842 100644 --- a/RULES.md +++ b/RULES.md @@ -32,7 +32,6 @@ Each Notice is associated with a severity: `INFO`, `WARNING`, `ERROR`. | Notice code | Description | |-------------------------------------------------------------------------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------| | [`block_trips_with_overlapping_stop_times`](#block_trips_with_overlapping_stop_times) | Block trips with overlapping stop times. | -| [`conditionally_forbidden_file`](#conditionally_forbidden_file) | A conditionally forbidden file is present in the feed. | | [`csv_parsing_failed`](#csv_parsing_failed) | Parsing of a CSV file failed. | | [`decreasing_shape_distance`](#decreasing_shape_distance) | Decreasing `shape_dist_traveled` in `shapes.txt`. | | [`decreasing_or_equal_stop_time_distance`](#decreasing_or_equal_stop_time_distance) | Decreasing or equal `shape_dist_traveled` in `stop_times.txt`. | @@ -42,6 +41,7 @@ Each Notice is associated with a severity: `INFO`, `WARNING`, `ERROR`. | [`empty_column_name`](#empty_column_name) | A column name is empty. | | [`empty_file`](#empty_file) | A CSV file is empty. | | [`equal_shape_distance_diff_coordinates`](#equal_shape_distance_diff_coordinates) | Two consecutive points have equal `shape_dist_traveled` and different lat/lon coordinates in `shapes.txt`. | +| [`forbidden_file`](#forbidden_file) | A conditionally forbidden file is present in the feed. | | [`foreign_key_violation`](#foreign_key_violation) | Wrong foreign key. | | [`inconsistent_agency_timezone`](#inconsistent_agency_timezone) | Inconsistent Timezone among agencies. | | [`invalid_color`](#invalid_color) | A field contains an invalid color value. | @@ -185,9 +185,9 @@ Trips with the same block id have overlapping stop times. - + -### conditionally_forbidden_file +### forbidden_file A conditionally forbidden file is present in the feed. diff --git a/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/ConditionallyForbiddenFileNotice.java b/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/ForbiddenFileNotice.java similarity index 86% rename from core/src/main/java/org/mobilitydata/gtfsvalidator/notice/ConditionallyForbiddenFileNotice.java rename to core/src/main/java/org/mobilitydata/gtfsvalidator/notice/ForbiddenFileNotice.java index d1ca4bb138..6668572270 100644 --- a/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/ConditionallyForbiddenFileNotice.java +++ b/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/ForbiddenFileNotice.java @@ -21,10 +21,10 @@ * *

Severity: {@code SeverityLevel.ERROR} */ -public class ConditionallyForbiddenFileNotice extends ValidationNotice { +public class ForbiddenFileNotice extends ValidationNotice { private final String filename; - public ConditionallyForbiddenFileNotice(String filename) { + public ForbiddenFileNotice(String filename) { super(SeverityLevel.ERROR); this.filename = filename; } diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/FaresV1FileValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/FaresV1FileValidator.java index a9d8a8cfa1..b5d21a4739 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/FaresV1FileValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/FaresV1FileValidator.java @@ -1,7 +1,7 @@ package org.mobilitydata.gtfsvalidator.validator; import javax.inject.Inject; -import org.mobilitydata.gtfsvalidator.notice.ConditionallyForbiddenFileNotice; +import org.mobilitydata.gtfsvalidator.notice.ForbiddenFileNotice; import org.mobilitydata.gtfsvalidator.notice.MissingRequiredFileNotice; import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; import org.mobilitydata.gtfsvalidator.table.GtfsFareAttributeTableContainer; @@ -26,8 +26,7 @@ public void validate(NoticeContainer noticeContainer) { noticeContainer.addValidationNotice( new MissingRequiredFileNotice(fareRuleTable.gtfsFilename())); } else if (fareAttributeTable.isMissingFile() && !fareRuleTable.isMissingFile()) { - noticeContainer.addValidationNotice( - new ConditionallyForbiddenFileNotice(fareRuleTable.gtfsFilename())); + noticeContainer.addValidationNotice(new ForbiddenFileNotice(fareRuleTable.gtfsFilename())); } } } diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/FaresV1FileValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/FaresV1FileValidatorTest.java index 7b18964960..4659efd5a0 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/FaresV1FileValidatorTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/FaresV1FileValidatorTest.java @@ -4,7 +4,7 @@ import junit.framework.TestCase; import org.junit.Test; -import org.mobilitydata.gtfsvalidator.notice.ConditionallyForbiddenFileNotice; +import org.mobilitydata.gtfsvalidator.notice.ForbiddenFileNotice; import org.mobilitydata.gtfsvalidator.notice.MissingRequiredFileNotice; import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; import org.mobilitydata.gtfsvalidator.table.GtfsFareAttributeTableContainer; @@ -60,6 +60,6 @@ public void testFareAttributesMissingButFareRulesPresent() { new GtfsFareRuleTableContainer(TableStatus.PARSABLE_HEADERS_AND_ROWS)) .validate(noticeContainer); assertThat(noticeContainer.getValidationNotices()) - .containsExactly(new ConditionallyForbiddenFileNotice("fare_rules.txt")); + .containsExactly(new ForbiddenFileNotice("fare_rules.txt")); } } From e90e91de07f29db2571f48a78908e56d0b384762 Mon Sep 17 00:00:00 2001 From: Brian Ferris Date: Tue, 4 Oct 2022 09:49:50 -0700 Subject: [PATCH 3/4] Ensure FaresV1FileValidator is annotated with @GtfsValidator. --- .../gtfsvalidator/validator/FaresV1FileValidator.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/FaresV1FileValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/FaresV1FileValidator.java index b5d21a4739..3d0765e741 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/FaresV1FileValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/FaresV1FileValidator.java @@ -1,12 +1,14 @@ package org.mobilitydata.gtfsvalidator.validator; import javax.inject.Inject; +import org.mobilitydata.gtfsvalidator.annotation.GtfsValidator; import org.mobilitydata.gtfsvalidator.notice.ForbiddenFileNotice; import org.mobilitydata.gtfsvalidator.notice.MissingRequiredFileNotice; import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; import org.mobilitydata.gtfsvalidator.table.GtfsFareAttributeTableContainer; import org.mobilitydata.gtfsvalidator.table.GtfsFareRuleTableContainer; +@GtfsValidator public class FaresV1FileValidator extends FileValidator { private final GtfsFareAttributeTableContainer fareAttributeTable; From ca01524edea5dc0002f1536dca39b4b8ef2e26d5 Mon Sep 17 00:00:00 2001 From: Brian Ferris Date: Tue, 4 Oct 2022 10:13:32 -0700 Subject: [PATCH 4/4] Remove "conditionally" from comments. --- RULES.md | 4 ++-- .../gtfsvalidator/notice/ForbiddenFileNotice.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/RULES.md b/RULES.md index 5743fb2854..b36bc9084b 100644 --- a/RULES.md +++ b/RULES.md @@ -46,7 +46,7 @@ Each Notice is associated with a severity: `INFO`, `WARNING`, `ERROR`. | [`fare_transfer_rule_invalid_transfer_count`](#fare_transfer_rule_invalid_transfer_count) | A row from GTFS file `fare_transfer_rules.txt` has a defined `transfer_count` with an invalid value. | | [`fare_transfer_rule_missing_transfer_count`](#fare_transfer_rule_missing_transfer_count) | A row from `fare_transfer_rules.txt` has `from_leg_group_id` equal to `to_leg_group_id`, but has no `transfer_count` specified. | | [`fare_transfer_rule_with_forbidden_transfer_count`](#fare_transfer_rule_with_forbidden_transfer_count) | A row from `fare_transfer_rules.txt` has `from_leg_group_id` not equal to `to_leg_group_id`, but has `transfer_count` specified. | -| [`forbidden_file`](#forbidden_file) | A conditionally forbidden file is present in the feed. | +| [`forbidden_file`](#forbidden_file) | A forbidden file is present in the feed. | | [`foreign_key_violation`](#foreign_key_violation) | Wrong foreign key. | | [`inconsistent_agency_timezone`](#inconsistent_agency_timezone) | Inconsistent Timezone among agencies. | | [`invalid_color`](#invalid_color) | A field contains an invalid color value. | @@ -195,7 +195,7 @@ Trips with the same block id have overlapping stop times. ### forbidden_file -A conditionally forbidden file is present in the feed. +A forbidden file is present in the feed. #### References * [GTFS terms definition](https://gtfs.org/reference/static/#term-definitions) diff --git a/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/ForbiddenFileNotice.java b/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/ForbiddenFileNotice.java index 6668572270..2b8324692a 100644 --- a/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/ForbiddenFileNotice.java +++ b/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/ForbiddenFileNotice.java @@ -17,7 +17,7 @@ package org.mobilitydata.gtfsvalidator.notice; /** - * A conditionally forbidden file is present in the feed. + * A forbidden file is present in the feed. * *

Severity: {@code SeverityLevel.ERROR} */