From d9ede476d8af3de849f3cd65f46bbe3fb1463692 Mon Sep 17 00:00:00 2001 From: Michael Kearney Date: Fri, 1 Mar 2024 16:01:58 -0500 Subject: [PATCH] fix: make stop_without_zone_id conditional on fare rule type (#1663) - Update `StopZoneIdValidator` to issue notice about a stop without `zone_id` defined only when the stop is contained in a trip contained in a route defined in a fare rule with zone fields defined. - Change from previous logic which warned about stops without `zone_id` defined if any fare rules had zone fields defined. - The warning is still only triggered for stops of location type `0`. - Zone fields in `fare_rules.txt` are `origin_id`, `destination_id`, and `contains_id`. --- .../table/GtfsFareRuleSchema.java | 3 + .../validator/StopZoneIdValidator.java | 116 +++-- .../validator/StopZoneIdValidatorTest.java | 412 ++++++++++++++---- 3 files changed, 406 insertions(+), 125 deletions(-) diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsFareRuleSchema.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsFareRuleSchema.java index 92d45cd202..a34ad84aa3 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsFareRuleSchema.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsFareRuleSchema.java @@ -22,6 +22,7 @@ import org.mobilitydata.gtfsvalidator.annotation.FieldTypeEnum; import org.mobilitydata.gtfsvalidator.annotation.ForeignKey; import org.mobilitydata.gtfsvalidator.annotation.GtfsTable; +import org.mobilitydata.gtfsvalidator.annotation.Index; import org.mobilitydata.gtfsvalidator.annotation.PrimaryKey; import org.mobilitydata.gtfsvalidator.annotation.Required; @@ -31,11 +32,13 @@ public interface GtfsFareRuleSchema extends GtfsEntity { @Required @ForeignKey(table = "fare_attributes.txt", field = "fare_id") @PrimaryKey(translationRecordIdType = UNSUPPORTED) + @Index String fareId(); @FieldType(FieldTypeEnum.ID) @ForeignKey(table = "routes.txt", field = "route_id") @PrimaryKey(translationRecordIdType = UNSUPPORTED) + @Index String routeId(); @FieldType(FieldTypeEnum.ID) 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 d759df1bf5..024beac995 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/StopZoneIdValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/StopZoneIdValidator.java @@ -17,30 +17,23 @@ import static org.mobilitydata.gtfsvalidator.notice.SeverityLevel.ERROR; +import com.google.common.collect.Multimap; +import com.google.common.collect.Multimaps; +import java.util.HashSet; +import java.util.Set; import javax.inject.Inject; import org.mobilitydata.gtfsvalidator.annotation.GtfsValidationNotice; import org.mobilitydata.gtfsvalidator.annotation.GtfsValidationNotice.FileRefs; import org.mobilitydata.gtfsvalidator.annotation.GtfsValidator; import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; import org.mobilitydata.gtfsvalidator.notice.ValidationNotice; -import org.mobilitydata.gtfsvalidator.table.GtfsFareRule; -import org.mobilitydata.gtfsvalidator.table.GtfsFareRuleSchema; -import org.mobilitydata.gtfsvalidator.table.GtfsFareRuleTableContainer; -import org.mobilitydata.gtfsvalidator.table.GtfsLocationType; -import org.mobilitydata.gtfsvalidator.table.GtfsStop; -import org.mobilitydata.gtfsvalidator.table.GtfsStopSchema; -import org.mobilitydata.gtfsvalidator.table.GtfsStopTableContainer; +import org.mobilitydata.gtfsvalidator.table.*; /** - * 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: - * - * + * If {@code fare_rules.txt} is provided, checks that all stops and platforms (location_type = 0) + * have {@code stops.zone_id} defined if the stop is defined as part of a {@code trip_id} in {@code + * stop_times.txt} whose {@code route_id} defines an {@code origin_id}, {@code destination_id}, or + * {@code contains_id} in {@code fare_rules.txt}. * *

Generated notice: {@link StopWithoutZoneIdNotice}. */ @@ -48,13 +41,40 @@ public class StopZoneIdValidator extends FileValidator { private final GtfsStopTableContainer stopTable; - private final GtfsFareRuleTableContainer fareRuleTable; + private final GtfsStopTimeTableContainer stopTimeTable; + private final GtfsTripTableContainer tripTable; + private final GtfsRouteTableContainer routeTable; @Inject - StopZoneIdValidator(GtfsStopTableContainer stopTable, GtfsFareRuleTableContainer fareRuleTable) { + StopZoneIdValidator( + GtfsStopTableContainer stopTable, + GtfsFareRuleTableContainer fareRuleTable, + GtfsStopTimeTableContainer stopTimeTable, + GtfsTripTableContainer tripTable, + GtfsRouteTableContainer routeTable) { this.stopTable = stopTable; this.fareRuleTable = fareRuleTable; + this.stopTimeTable = stopTimeTable; + this.tripTable = tripTable; + this.routeTable = routeTable; + } + + /** + * 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; } @Override @@ -65,39 +85,69 @@ public void validate(NoticeContainer noticeContainer) { if (!hasFareZoneStructure(fareRuleTable)) { return; } + + Multimap routesWithZoneFieldsDefined = + Multimaps.filterValues( + fareRuleTable.byRouteIdMap(), + fareRule -> + fareRule.hasOriginId() || fareRule.hasDestinationId() || fareRule.hasContainsId()); for (GtfsStop stop : stopTable.getEntities()) { if (!stop.locationType().equals(GtfsLocationType.STOP)) { continue; } - if (!stop.hasZoneId()) { - noticeContainer.addValidationNotice(new StopWithoutZoneIdNotice(stop)); + if (stop.hasZoneId()) { + continue; + } + + // check that a stop without zone_id does not have a route_id in a fare_rule with + // zone-dependent fields + for (GtfsRoute route : getRoutesIncludingStop(stop)) { + if (routesWithZoneFieldsDefined.containsKey(route.routeId())) { + noticeContainer.addValidationNotice(new StopWithoutZoneIdNotice(stop)); + break; + } } } } /** - * Checks if the {@code GtfsFareRuleTableContainer} provided as parameter has a fare structure - * that uses zones. + * Gets a deduplicated set of all trips which contain a stop. A trip "contains" a stop if an entry + * in {@code stop_times.txt} defines the stop and the trip. * - * @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. + * @param stop the {@code GtfsStop} for which to get containing {@code GtfsTrip}s + * @return a {@code Set} of {@code GtfsTrip}s containing {@code stop} */ - private static boolean hasFareZoneStructure(GtfsFareRuleTableContainer fareRuleTable) { - for (GtfsFareRule fareRule : fareRuleTable.getEntities()) { - if (fareRule.hasContainsId() || fareRule.hasDestinationId() || fareRule.hasOriginId()) { - return true; - } + private Set getTripsIncludingStop(GtfsStop stop) { + Set trips = new HashSet<>(); + for (GtfsStopTime stopTime : stopTimeTable.byStopId(stop.stopId())) { + tripTable.byTripId(stopTime.tripId()).ifPresent(trips::add); } - return false; + return trips; + } + + /** + * Gets a deduplicated set of all routes which contain a stop. A route "contains" a stop if an + * entry in {@code trips.txt} defines the route and a trip which contains the stop. + * + * @param stop the {@code GtfsStop} for which to get containing {@code GtfsRoute}s + * @return a {@code Set} of {@code GtfsRoute}s containing {@code stop} + */ + private Set getRoutesIncludingStop(GtfsStop stop) { + Set routes = new HashSet<>(); + for (GtfsTrip trip : getTripsIncludingStop(stop)) { + routeTable.byRouteId(trip.routeId()).ifPresent(routes::add); + } + return routes; } /** - * Stop without value for `stops.zone_id`. + * Stop without value for `stops.zone_id` contained in a route with a zone-dependent fare rule. * *

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. + * 0) must have `stops.zone_id` assigned if they are defined in a trip defined in a route defined + * in a fare rule which also defines at least one of `origin_id`, `destination_id`, or + * `contains_id`. */ @GtfsValidationNotice( severity = ERROR, 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 c59c4157fa..faf7cd24cc 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/StopZoneIdValidatorTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/StopZoneIdValidatorTest.java @@ -19,17 +19,14 @@ import static com.google.common.truth.Truth.assertThat; import com.google.common.collect.ImmutableList; +import java.util.Arrays; import java.util.List; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; 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; +import org.mobilitydata.gtfsvalidator.table.*; import org.mobilitydata.gtfsvalidator.validator.StopZoneIdValidator.StopWithoutZoneIdNotice; @RunWith(JUnit4.class) @@ -45,146 +42,377 @@ private static GtfsStop createStop( .build(); } - private static GtfsFareRule createFareRuleWithZoneStructure(int csvRowNumber) { + /** + * Generates a fare rule + * + * @param csvRowNumber row of rule in fare_rules.txt and disambiguating index for cell values + * @param route whether to define a route_id in the fare rule + * @param origin whether to define an origin_id in the fare rule + * @param destination whether to define a destination_id in the fare rule + * @param contains whether to define a contains_id in the fare rule + * @return a `GtfsFareRule` with fare_rule_id and the indicated fields defined + */ + private static GtfsFareRule createFareRule( + int csvRowNumber, boolean route, boolean origin, boolean destination, boolean contains) { return new GtfsFareRule.Builder() .setCsvRowNumber(csvRowNumber) .setFareId(toFareRuleId(csvRowNumber)) - .setOriginId("origin id") + .setRouteId(route ? toRouteId(csvRowNumber) : null) + .setOriginId(origin ? toZoneId(csvRowNumber) : null) + .setDestinationId(destination ? toZoneId(csvRowNumber) : null) + .setContainsId(contains ? toZoneId(csvRowNumber) : null) .build(); } + /** + * Generates a zone-based fare rule + * + * @param csvRowNumber row of rule in fare_rules.txt and disambiguating index for cell values + * @return a `GtfsFareRule` with only fare_rule_id and origin_id defined + */ + private static GtfsFareRule createFareRuleWithZoneStructure(int csvRowNumber) { + return createFareRule(csvRowNumber, false, true, false, false); + } + + /** + * Generates a route-based fare rule + * + * @param csvRowNumber row of rule in fare_rules.txt and disambiguating index for cell values + * @return a `GtfsFareRule` with only fare_rule_id and route_id defined + */ private static GtfsFareRule createFareRuleWithoutZoneStructure(int csvRowNumber) { - return new GtfsFareRule.Builder() - .setCsvRowNumber(csvRowNumber) - .setFareId(toFareRuleId(csvRowNumber)) - .setRouteId("route id value") - .build(); + return createFareRule(csvRowNumber, true, false, false, false); + } + + /** + * Generates string id. Can maintain consistency across tables in tests when accurate rows and + * field names are passed. + * + * @param item string description of the object; should be the name of the field, but can be + * arbitrary + * @param csvRowNumber number to disambiguate the id; should be the index of the row but can be + * arbitrary + * @return a string id + */ + private static String toId(String item, int csvRowNumber) { + return String.format("%s id %d", item, csvRowNumber); } + /** + * Shortcut for toId for generating string id's for `GtfsLocationType`s + * + * @param locationType the `GtfsLocationType` to id + * @param csvRowNumber number to disambiguate the id; should be the index of the row but can be + * arbitrary + * @return a string id + */ private static String toLocationId(GtfsLocationType locationType, int csvRowNumber) { return locationType.toString() + csvRowNumber; } + /** + * Shortcut for toId for generating string id's for `GtfsFareRule`s + * + * @param csvRowNumber number to disambiguate the id; should be the index of the row but can be + * arbitrary + * @return a string id + */ private static String toFareRuleId(int csvRowNumber) { - return String.format("fare rule id %s", csvRowNumber); + return toId("fare_rule", csvRowNumber); + } + + /** + * Shortcut for toId for generating string id's for zones + * + * @param csvRowNumber number to disambiguate the id; should be the index of the row but can be + * arbitrary + * @return a string id + */ + private static String toZoneId(int csvRowNumber) { + return toId("zone", csvRowNumber); } + /** + * Shortcut for toId for generating string id's for `GtfsRoute`s + * + * @param csvRowNumber number to disambiguate the id; should be the index of the row but can be + * arbitrary + * @return a string id + */ + private static String toRouteId(int csvRowNumber) { + return toId("route", csvRowNumber); + } + + /** + * Shortcut for toId for generating string id's for `GtfsTrip`s + * + * @param csvRowNumber number to disambiguate the id; should be the index of the row but can be + * arbitrary + * @return a string id + */ + private static String toTripId(int csvRowNumber) { + return toId("trip", csvRowNumber); + } + + /** + * Shortcut for toId for generating string id's for `GtfsStop`s + * + * @param csvRowNumber number to disambiguate the id; should be the index of the row but can be + * arbitrary + * @return a string id + */ + private static String toStopId(int csvRowNumber) { + return toId("stop", csvRowNumber); + } + + /** + * Generates StopZoneId validation notices for mock data + * + * @param stops a list of mock stops to test + * @param fareRules a list of mock fare rules to test + * @param stopTimes a list of mock stop times to test + * @param trips a list of mock trips to test + * @param routes a list of mock routes to test + * @return a list of validation notices for the mock data + */ private static List generateNotices( - List stops, List fareRules) { + List stops, + List fareRules, + List stopTimes, + List trips, + List routes) { NoticeContainer noticeContainer = new NoticeContainer(); new StopZoneIdValidator( GtfsStopTableContainer.forEntities(stops, noticeContainer), - GtfsFareRuleTableContainer.forEntities(fareRules, noticeContainer)) + GtfsFareRuleTableContainer.forEntities(fareRules, noticeContainer), + GtfsStopTimeTableContainer.forEntities(stopTimes, noticeContainer), + GtfsTripTableContainer.forEntities(trips, noticeContainer), + GtfsRouteTableContainer.forEntities(routes, noticeContainer)) .validate(noticeContainer); return noticeContainer.getValidationNotices(); } - @Test - public void stop_zoneIdNotProvided_zoneStructure_yieldsNotice() { - ImmutableList stops = ImmutableList.of(createStop(3, GtfsLocationType.STOP, null)); - 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(); + /** + * Generates StopZoneId validation notices for mock data. Generates mock stopTime, trip, and route + * data such that the trip and route contain the passed stop. + * + * @param csvRowNumber the index of the row and a disambiguator for the generated ids for + * stopTime, trip, and route. Must be the same as the csvRowNumber used to create stop and the + * fareRules in order to trigger tests. + * @param stop a list of mock stops to test + * @param fareRules a list of mock fare rules to test + * @return a list of validation notices for the mock data + */ + private static List generateNoticesFromStopAndFareRules( + int csvRowNumber, GtfsStop stop, List fareRules) { + List stops = ImmutableList.of(stop); + List routes = + ImmutableList.of( + new GtfsRoute.Builder() + .setCsvRowNumber(csvRowNumber) + .setRouteId(toRouteId(csvRowNumber)) + .build()); + List trips = + ImmutableList.of( + new GtfsTrip.Builder() + .setCsvRowNumber(csvRowNumber) + .setTripId(toTripId(csvRowNumber)) + .setRouteId(toRouteId(csvRowNumber)) + .build()); + List stopTimes = + ImmutableList.of( + new GtfsStopTime.Builder() + .setCsvRowNumber(csvRowNumber) + .setStopId(stop.stopId()) + .setTripId(toTripId(csvRowNumber)) + .build()); + return generateNotices(stops, fareRules, stopTimes, trips, routes); } @Test - public void stop_zoneIdProvided_noNotice() { - assertThat( - generateNotices( - ImmutableList.of(createStop(3, GtfsLocationType.STOP, "zone id value")), - ImmutableList.of(createFareRuleWithZoneStructure(5)))) - .isEmpty(); + public void stop_zoneIdNotProvided_routeNotInFareRules_noNotice() { + int csvRowNumber = 0; + GtfsStop stop = + new GtfsStop.Builder() + .setCsvRowNumber(csvRowNumber) + .setLocationType(GtfsLocationType.STOP) + .setZoneId(null) + .build(); + List fareRules = + ImmutableList.of( + createFareRule(csvRowNumber, false, false, false, false), + createFareRule(csvRowNumber, false, true, false, false), + createFareRule(csvRowNumber, false, false, true, false), + createFareRule(csvRowNumber, false, false, false, true)); + assertThat(generateNoticesFromStopAndFareRules(csvRowNumber, stop, fareRules)).isEmpty(); } @Test - public void station_zoneIdNotProvided_noNotice() { - assertThat( - generateNotices( - ImmutableList.of(createStop(3, GtfsLocationType.STATION, null)), - ImmutableList.of(createFareRuleWithZoneStructure(5)))) - .isEmpty(); + public void stop_zoneIdNotProvided_routeInFareRulesWithoutZoneFields_noNotice() { + int csvRowNumber = 0; + GtfsStop stop = + new GtfsStop.Builder() + .setCsvRowNumber(csvRowNumber) + .setLocationType(GtfsLocationType.STOP) + .setZoneId(null) + .build(); + List fareRules = + ImmutableList.of(createFareRule(csvRowNumber, true, false, false, false)); + assertThat(generateNoticesFromStopAndFareRules(csvRowNumber, stop, fareRules)).isEmpty(); } @Test - public void station_zoneIdProvided_noNotice() { - assertThat( - generateNotices( - ImmutableList.of(createStop(3, GtfsLocationType.STATION, "zone id value")), - ImmutableList.of(createFareRuleWithZoneStructure(5)))) - .isEmpty(); + public void stop_zoneIdNotProvided_routeInFareRulesWithOriginId_yieldsNotice() { + int csvRowNumber = 0; + GtfsStop stop = + new GtfsStop.Builder() + .setCsvRowNumber(csvRowNumber) + .setLocationType(GtfsLocationType.STOP) + .setZoneId(null) + .build(); + List fareRules = + ImmutableList.of(createFareRule(csvRowNumber, true, true, false, false)); + assertThat(generateNoticesFromStopAndFareRules(csvRowNumber, stop, fareRules)) + .containsExactly(new StopWithoutZoneIdNotice(stop)); } @Test - public void entrance_zoneIdNotProvided_noNotice() { - assertThat( - generateNotices( - ImmutableList.of(createStop(3, GtfsLocationType.ENTRANCE, null)), - ImmutableList.of(createFareRuleWithZoneStructure(5)))) - .isEmpty(); + public void stop_zoneIdNotProvided_routeInFareRulesWithDestinationId_yieldsNotice() { + int csvRowNumber = 0; + GtfsStop stop = + new GtfsStop.Builder() + .setCsvRowNumber(csvRowNumber) + .setLocationType(GtfsLocationType.STOP) + .setZoneId(null) + .build(); + List fareRules = + ImmutableList.of(createFareRule(csvRowNumber, true, false, true, false)); + assertThat(generateNoticesFromStopAndFareRules(csvRowNumber, stop, fareRules)) + .containsExactly(new StopWithoutZoneIdNotice(stop)); } @Test - public void entrance_zoneIdProvided_noNotice() { - assertThat( - generateNotices( - ImmutableList.of(createStop(3, GtfsLocationType.ENTRANCE, "zone id value")), - ImmutableList.of(createFareRuleWithZoneStructure(5)))) - .isEmpty(); + public void stop_zoneIdNotProvided_routeInFareRulesWithContainsId_yieldsNotice() { + int csvRowNumber = 0; + GtfsStop stop = + new GtfsStop.Builder() + .setCsvRowNumber(csvRowNumber) + .setLocationType(GtfsLocationType.STOP) + .setZoneId(null) + .build(); + List fareRules = + ImmutableList.of(createFareRule(csvRowNumber, true, false, false, true)); + assertThat(generateNoticesFromStopAndFareRules(csvRowNumber, stop, fareRules)) + .containsExactly(new StopWithoutZoneIdNotice(stop)); } @Test - public void genericNode_zoneIdNotProvided_noNotice() { - assertThat( - generateNotices( - ImmutableList.of(createStop(3, GtfsLocationType.GENERIC_NODE, null)), - ImmutableList.of(createFareRuleWithZoneStructure(5)))) - .isEmpty(); + public void stop_zoneIdNotProvided_routeInFareRulesWithAndWithoutZoneFields_yieldsNotice() { + int csvRowNumber = 0; + GtfsStop stop = + new GtfsStop.Builder() + .setCsvRowNumber(csvRowNumber) + .setLocationType(GtfsLocationType.STOP) + .setZoneId(null) + .build(); + List fareRules = + ImmutableList.of( + createFareRule(csvRowNumber, true, false, false, false), + createFareRule(csvRowNumber, true, true, false, false)); + assertThat(generateNoticesFromStopAndFareRules(csvRowNumber, stop, fareRules)) + .containsExactly(new StopWithoutZoneIdNotice(stop)); } @Test - public void genericNode_zoneIdProvided_noNotice() { - assertThat( - generateNotices( - ImmutableList.of(createStop(3, GtfsLocationType.GENERIC_NODE, "zone id value")), - ImmutableList.of(createFareRuleWithZoneStructure(5)))) - .isEmpty(); + public void stop_zoneIdProvided_noNotice() { + int csvRowNumber = 0; + GtfsStop stop = + new GtfsStop.Builder() + .setCsvRowNumber(csvRowNumber) + .setLocationType(GtfsLocationType.STOP) + .setZoneId(toZoneId(csvRowNumber)) + .build(); + List fareRules = + ImmutableList.of( + createFareRule(csvRowNumber, false, false, false, false), + createFareRule(csvRowNumber, true, false, false, false), + createFareRule(csvRowNumber, true, true, false, false), + createFareRule(csvRowNumber, false, true, false, false), + createFareRule(csvRowNumber, false, false, true, false), + createFareRule(csvRowNumber, true, false, false, true)); + assertThat(generateNoticesFromStopAndFareRules(csvRowNumber, stop, fareRules)).isEmpty(); } @Test - public void boardingArea_zoneIdNotProvided_noNotice() { - assertThat( - generateNotices( - ImmutableList.of(createStop(3, GtfsLocationType.BOARDING_AREA, null)), - ImmutableList.of(createFareRuleWithZoneStructure(5)))) - .isEmpty(); + public void allLocationTypesExceptStop_noNotice() { + int csvRowNumber = 0; + List fareRules = + ImmutableList.of( + createFareRule(csvRowNumber, false, false, false, false), + createFareRule(csvRowNumber, true, false, false, false), + createFareRule(csvRowNumber, true, true, false, false), + createFareRule(csvRowNumber, false, true, false, false), + createFareRule(csvRowNumber, false, false, true, false), + createFareRule(csvRowNumber, true, false, false, true)); + for (GtfsLocationType locationType : GtfsLocationType.values()) { + if (!locationType.equals(GtfsLocationType.STOP)) { + assertThat( + generateNoticesFromStopAndFareRules( + csvRowNumber, + new GtfsStop.Builder() + .setCsvRowNumber(csvRowNumber) + .setLocationType(locationType) + .setZoneId(null) + .build(), + fareRules)) + .isEmpty(); + assertThat( + generateNoticesFromStopAndFareRules( + csvRowNumber, + new GtfsStop.Builder() + .setCsvRowNumber(csvRowNumber) + .setLocationType(locationType) + .setZoneId(toZoneId(csvRowNumber)) + .build(), + fareRules)) + .isEmpty(); + } + } } @Test - public void boardingArea_zoneIdProvided_noNotice() { - assertThat( - generateNotices( - ImmutableList.of(createStop(3, GtfsLocationType.BOARDING_AREA, "zone id value")), - ImmutableList.of(createFareRuleWithZoneStructure(5)))) - .isEmpty(); + public void emptyFareRule_allStopLocationTypes_noZoneId_noNotice() { + int csvRowNumber = 0; + Arrays.stream(GtfsLocationType.values()) + .forEach( + gtfsLocationType -> + assertThat( + generateNoticesFromStopAndFareRules( + csvRowNumber, + new GtfsStop.Builder() + .setCsvRowNumber(csvRowNumber) + .setLocationType(gtfsLocationType) + .setZoneId(null) + .build(), + ImmutableList.of())) + .isEmpty()); } @Test - public void emptyFareRule_allStopLocation_noZoneId_noNotice() { - assertThat( - generateNotices( - ImmutableList.of( - createStop(4, GtfsLocationType.STOP, null), - createStop(6, GtfsLocationType.STATION, null), - createStop(7, GtfsLocationType.ENTRANCE, null), - createStop(10, GtfsLocationType.GENERIC_NODE, null), - createStop(3, GtfsLocationType.BOARDING_AREA, null)), - ImmutableList.of())) - .isEmpty(); + public void emptyFareRule_allStopLocationTypes_zoneId_noNotice() { + int csvRowNumber = 0; + Arrays.stream(GtfsLocationType.values()) + .forEach( + gtfsLocationType -> + assertThat( + generateNoticesFromStopAndFareRules( + csvRowNumber, + new GtfsStop.Builder() + .setCsvRowNumber(csvRowNumber) + .setLocationType(gtfsLocationType) + .setZoneId(toStopId(csvRowNumber)) + .build(), + List.of())) + .isEmpty()); } }