diff --git a/RULES.md b/RULES.md index e2f356b2da..98a399b00c 100644 --- a/RULES.md +++ b/RULES.md @@ -607,7 +607,7 @@ Missing `stop_time.arrival_time` or `stop_time.departure_time` #### StopWithoutZoneIdNotice -If `fare_rules.txt` is provided, then all stops and platforms (location_type = 0) must have `stops.zone_id` assigned. +If `fare_rules.txt` is provided, and `fare_rules.txt` uses at least one column among `origin_id`, `destination_id`, and `contains_id`, then all stops and platforms (location_type = 0) must have `stops.zone_id` assigned. ##### References: * [GTFS stops.txt specification](https://gtfs.org/reference/static#stopstxt) diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/StopZoneIdValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/StopZoneIdValidator.java index 212c6225c2..72f748e945 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/StopZoneIdValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/StopZoneIdValidator.java @@ -21,14 +21,22 @@ import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; import org.mobilitydata.gtfsvalidator.notice.SeverityLevel; import org.mobilitydata.gtfsvalidator.notice.ValidationNotice; +import org.mobilitydata.gtfsvalidator.table.GtfsFareRule; import org.mobilitydata.gtfsvalidator.table.GtfsFareRuleTableContainer; import org.mobilitydata.gtfsvalidator.table.GtfsLocationType; import org.mobilitydata.gtfsvalidator.table.GtfsStop; import org.mobilitydata.gtfsvalidator.table.GtfsStopTableContainer; /** - * Check that if {@code fare_rules.txt} is provided, then all stops and platforms (location_type = - * 0) have {@code stops.zone_id} assigned. + * Checks that all stops and platforms (location_type = 0) have {@code stops.zone_id} assigned. + * assigned if {@code fare_rules.txt} is provided and at least one of the following fields is + * provided: + * + * * *

Generated notice: {@link StopWithoutZoneIdNotice}. */ @@ -49,13 +57,36 @@ public void validate(NoticeContainer noticeContainer) { if (fareRuleTable.getEntities().isEmpty()) { return; } + if (!hasFareZoneStructure(fareRuleTable)) { + return; + } for (GtfsStop stop : stopTable.getEntities()) { - if (stop.locationType().equals(GtfsLocationType.STOP) && !stop.hasZoneId()) { + if (!stop.locationType().equals(GtfsLocationType.STOP)) { + continue; + } + if (!stop.hasZoneId()) { noticeContainer.addValidationNotice(new StopWithoutZoneIdNotice(stop)); } } } + /** + * Checks if the {@code GtfsFareRuleTableContainer} provided as parameter has a fare structure + * that uses zones. + * + * @param fareRuleTable the {@code GtfsFareRuleTableContainer} to be checked + * @return true if the {@code GtfsFareRuleTableContainer} provided as parameter has a fare + * structure that uses zones; false otherwise. + */ + private static boolean hasFareZoneStructure(GtfsFareRuleTableContainer fareRuleTable) { + for (GtfsFareRule fareRule : fareRuleTable.getEntities()) { + if (fareRule.hasContainsId() || fareRule.hasDestinationId() || fareRule.hasOriginId()) { + return true; + } + } + return false; + } + /** * A {@code GtfsShape} should be referred to at least once in {@code GtfsTripTableContainer} * station). diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/StopZoneIdValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/StopZoneIdValidatorTest.java index 4400df539a..bced4dc0e9 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/StopZoneIdValidatorTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/StopZoneIdValidatorTest.java @@ -45,10 +45,19 @@ private static GtfsStop createStop( .build(); } - private static GtfsFareRule createFareRule(long csvRowNumber) { + private static GtfsFareRule createFareRuleWithZoneStructure(long csvRowNumber) { return new GtfsFareRule.Builder() .setCsvRowNumber(csvRowNumber) .setFareId(toFareRuleId(csvRowNumber)) + .setOriginId("origin id") + .build(); + } + + private static GtfsFareRule createFareRuleWithoutZoneStructure(long csvRowNumber) { + return new GtfsFareRule.Builder() + .setCsvRowNumber(csvRowNumber) + .setFareId(toFareRuleId(csvRowNumber)) + .setRouteId("route id value") .build(); } @@ -71,18 +80,25 @@ private static List generateNotices( } @Test - public void stop_zoneIdNotProvided_yieldsNotice() { + public void stop_zoneIdNotProvided_zoneStructure_yieldsNotice() { ImmutableList stops = ImmutableList.of(createStop(3, GtfsLocationType.STOP, null)); - assertThat(generateNotices(stops, ImmutableList.of(createFareRule(5)))) + assertThat(generateNotices(stops, ImmutableList.of(createFareRuleWithZoneStructure(5)))) .containsExactly(new StopWithoutZoneIdNotice(stops.get(0))); } + @Test + public void stop_zoneIdNotProvided_noZoneStructure_yieldsNotice() { + ImmutableList stops = ImmutableList.of(createStop(3, GtfsLocationType.STOP, null)); + assertThat(generateNotices(stops, ImmutableList.of(createFareRuleWithoutZoneStructure(5)))) + .isEmpty(); + } + @Test public void stop_zoneIdProvided_noNotice() { assertThat( generateNotices( ImmutableList.of(createStop(3, GtfsLocationType.STOP, "zone id value")), - ImmutableList.of(createFareRule(5)))) + ImmutableList.of(createFareRuleWithZoneStructure(5)))) .isEmpty(); } @@ -91,7 +107,7 @@ public void station_zoneIdNotProvided_noNotice() { assertThat( generateNotices( ImmutableList.of(createStop(3, GtfsLocationType.STATION, null)), - ImmutableList.of(createFareRule(5)))) + ImmutableList.of(createFareRuleWithZoneStructure(5)))) .isEmpty(); } @@ -100,7 +116,7 @@ public void station_zoneIdProvided_noNotice() { assertThat( generateNotices( ImmutableList.of(createStop(3, GtfsLocationType.STATION, "zone id value")), - ImmutableList.of(createFareRule(5)))) + ImmutableList.of(createFareRuleWithZoneStructure(5)))) .isEmpty(); } @@ -109,7 +125,7 @@ public void entrance_zoneIdNotProvided_noNotice() { assertThat( generateNotices( ImmutableList.of(createStop(3, GtfsLocationType.ENTRANCE, null)), - ImmutableList.of(createFareRule(5)))) + ImmutableList.of(createFareRuleWithZoneStructure(5)))) .isEmpty(); } @@ -118,7 +134,7 @@ public void entrance_zoneIdProvided_noNotice() { assertThat( generateNotices( ImmutableList.of(createStop(3, GtfsLocationType.ENTRANCE, "zone id value")), - ImmutableList.of(createFareRule(5)))) + ImmutableList.of(createFareRuleWithZoneStructure(5)))) .isEmpty(); } @@ -127,7 +143,7 @@ public void genericNode_zoneIdNotProvided_noNotice() { assertThat( generateNotices( ImmutableList.of(createStop(3, GtfsLocationType.GENERIC_NODE, null)), - ImmutableList.of(createFareRule(5)))) + ImmutableList.of(createFareRuleWithZoneStructure(5)))) .isEmpty(); } @@ -136,7 +152,7 @@ public void genericNode_zoneIdProvided_noNotice() { assertThat( generateNotices( ImmutableList.of(createStop(3, GtfsLocationType.GENERIC_NODE, "zone id value")), - ImmutableList.of(createFareRule(5)))) + ImmutableList.of(createFareRuleWithZoneStructure(5)))) .isEmpty(); } @@ -145,7 +161,7 @@ public void boardingArea_zoneIdNotProvided_noNotice() { assertThat( generateNotices( ImmutableList.of(createStop(3, GtfsLocationType.BOARDING_AREA, null)), - ImmutableList.of(createFareRule(5)))) + ImmutableList.of(createFareRuleWithZoneStructure(5)))) .isEmpty(); } @@ -154,7 +170,7 @@ public void boardingArea_zoneIdProvided_noNotice() { assertThat( generateNotices( ImmutableList.of(createStop(3, GtfsLocationType.BOARDING_AREA, "zone id value")), - ImmutableList.of(createFareRule(5)))) + ImmutableList.of(createFareRuleWithZoneStructure(5)))) .isEmpty(); }