From 817c6e46757efa7592f6106d3c98293853cb74ed Mon Sep 17 00:00:00 2001 From: cka-y <60586858+cka-y@users.noreply.github.com> Date: Wed, 2 Oct 2024 11:57:56 -0400 Subject: [PATCH] feat: added `forbidden_prior_day_booking_field_value`, `invalid_prior_notice_duration_min` and `forbidden_prior_notice_start_day` notices (#1860) --- .../BookingRulesEntityValidator.java | 238 +++++++++++++----- .../BookingRulesEntityValidatorTest.java | 55 +++- .../validator/NoticeFieldsTest.java | 6 +- 3 files changed, 232 insertions(+), 67 deletions(-) diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/BookingRulesEntityValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/BookingRulesEntityValidator.java index cae25bf53b..b2f4f2261e 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/BookingRulesEntityValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/BookingRulesEntityValidator.java @@ -2,6 +2,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.function.Function; import org.mobilitydata.gtfsvalidator.annotation.GtfsValidationNotice; import org.mobilitydata.gtfsvalidator.annotation.GtfsValidationNotice.FileRefs; import org.mobilitydata.gtfsvalidator.annotation.GtfsValidator; @@ -17,97 +18,127 @@ public class BookingRulesEntityValidator extends SingleEntityValidator entity.priorNoticeStartDay()) { - noticeContainer.addValidationNotice(new PriorNoticeLastDayAfterStartDayNotice(entity)); + private static void validatePriorNoticeDurationMin( + GtfsBookingRules entity, NoticeContainer noticeContainer) { + if (entity.hasPriorNoticeDurationMin() && entity.hasPriorNoticeDurationMax()) { + if (entity.priorNoticeDurationMax() < entity.priorNoticeDurationMin()) { + noticeContainer.addValidationNotice( + new InvalidPriorNoticeDurationMinNotice( + entity, entity.priorNoticeDurationMin(), entity.priorNoticeDurationMax())); + } } } - private static void validateForbiddenRealTimeFields( + private static void validatePriorNoticeStartDay( GtfsBookingRules entity, NoticeContainer noticeContainer) { - // Only validate entities with REALTIME booking type - if (entity.bookingType() != GtfsBookingType.REALTIME) { - return; - } - - // Retrieve the list of forbidden fields - List forbiddenFields = findForbiddenRealTimeFields(entity); - - // If there are any forbidden fields, add a validation notice - if (!forbiddenFields.isEmpty()) { + if (entity.hasPriorNoticeDurationMax() && entity.hasPriorNoticeStartDay()) { noticeContainer.addValidationNotice( - new ForbiddenRealTimeBookingFieldValueNotice(entity, forbiddenFields)); + new ForbiddenPriorNoticeStartDayNotice( + entity, entity.priorNoticeStartDay(), entity.priorNoticeDurationMax())); } } - private static void validateSameDayFields( - GtfsBookingRules entity, NoticeContainer noticeContainer) { - // Only validate entities with SAME_DAY booking type - if (entity.bookingType() != GtfsBookingType.SAMEDAY) { + private static void validateBookingType( + GtfsBookingRules entity, + GtfsBookingType bookingType, + Function> forbiddenFieldsFinder, + ValidationNoticeConstructor validationNoticeConstructor, + NoticeContainer noticeContainer) { + + if (entity.bookingType() != bookingType) { return; } - // Retrieve the list of forbidden fields - List forbiddenFields = findForbiddenSameDayFields(entity); + List forbiddenFields = forbiddenFieldsFinder.apply(entity); - // If there are any forbidden fields, add a validation notice if (!forbiddenFields.isEmpty()) { noticeContainer.addValidationNotice( - new ForbiddenSameDayBookingFieldValueNotice(entity, forbiddenFields)); + validationNoticeConstructor.create(entity, forbiddenFields)); } } private static List findForbiddenSameDayFields(GtfsBookingRules bookingRule) { - List fields = new ArrayList<>(); + return findForbiddenFields( + bookingRule.hasPriorNoticeLastDay(), + GtfsBookingRules.PRIOR_NOTICE_LAST_DAY_FIELD_NAME, + bookingRule.hasPriorNoticeLastTime(), + GtfsBookingRules.PRIOR_NOTICE_LAST_TIME_FIELD_NAME, + bookingRule.hasPriorNoticeServiceId(), + GtfsBookingRules.PRIOR_NOTICE_SERVICE_ID_FIELD_NAME); + } - // Check each forbidden field and add its name to the list if it's present - if (bookingRule.hasPriorNoticeLastDay()) { - fields.add(GtfsBookingRules.PRIOR_NOTICE_LAST_DAY_FIELD_NAME); - } - if (bookingRule.hasPriorNoticeLastTime()) { - fields.add(GtfsBookingRules.PRIOR_NOTICE_LAST_TIME_FIELD_NAME); - } - if (bookingRule.hasPriorNoticeServiceId()) { - fields.add(GtfsBookingRules.PRIOR_NOTICE_SERVICE_ID_FIELD_NAME); - } - return fields; + private static List findForbiddenPriorDayFields(GtfsBookingRules bookingRule) { + return findForbiddenFields( + bookingRule.hasPriorNoticeDurationMin(), + GtfsBookingRules.PRIOR_NOTICE_DURATION_MIN_FIELD_NAME, + bookingRule.hasPriorNoticeDurationMax(), + GtfsBookingRules.PRIOR_NOTICE_DURATION_MAX_FIELD_NAME); } - /** Finds forbidden fields that should not be present for real-time booking rules. */ - public static List findForbiddenRealTimeFields(GtfsBookingRules bookingRule) { - List fields = new ArrayList<>(); + private static List findForbiddenRealTimeFields(GtfsBookingRules bookingRule) { + return findForbiddenFields( + bookingRule.hasPriorNoticeDurationMin(), + GtfsBookingRules.PRIOR_NOTICE_DURATION_MIN_FIELD_NAME, + bookingRule.hasPriorNoticeDurationMax(), + GtfsBookingRules.PRIOR_NOTICE_DURATION_MAX_FIELD_NAME, + bookingRule.hasPriorNoticeLastDay(), + GtfsBookingRules.PRIOR_NOTICE_LAST_DAY_FIELD_NAME, + bookingRule.hasPriorNoticeLastTime(), + GtfsBookingRules.PRIOR_NOTICE_LAST_TIME_FIELD_NAME, + bookingRule.hasPriorNoticeStartDay(), + GtfsBookingRules.PRIOR_NOTICE_START_DAY_FIELD_NAME, + bookingRule.hasPriorNoticeStartTime(), + GtfsBookingRules.PRIOR_NOTICE_START_TIME_FIELD_NAME, + bookingRule.hasPriorNoticeServiceId(), + GtfsBookingRules.PRIOR_NOTICE_SERVICE_ID_FIELD_NAME); + } - // Check each forbidden field and add its name to the list if it's present - if (bookingRule.hasPriorNoticeDurationMin()) { - fields.add(GtfsBookingRules.PRIOR_NOTICE_DURATION_MIN_FIELD_NAME); - } - if (bookingRule.hasPriorNoticeDurationMax()) { - fields.add(GtfsBookingRules.PRIOR_NOTICE_DURATION_MAX_FIELD_NAME); - } - if (bookingRule.hasPriorNoticeLastDay()) { - fields.add(GtfsBookingRules.PRIOR_NOTICE_LAST_DAY_FIELD_NAME); - } - if (bookingRule.hasPriorNoticeLastTime()) { - fields.add(GtfsBookingRules.PRIOR_NOTICE_LAST_TIME_FIELD_NAME); - } - if (bookingRule.hasPriorNoticeStartDay()) { - fields.add(GtfsBookingRules.PRIOR_NOTICE_START_DAY_FIELD_NAME); - } - if (bookingRule.hasPriorNoticeStartTime()) { - fields.add(GtfsBookingRules.PRIOR_NOTICE_START_TIME_FIELD_NAME); + private static List findForbiddenFields(Object... conditionsAndFields) { + List fields = new ArrayList<>(); + for (int i = 0; i < conditionsAndFields.length; i += 2) { + if ((Boolean) conditionsAndFields[i]) { + fields.add((String) conditionsAndFields[i + 1]); + } } - if (bookingRule.hasPriorNoticeServiceId()) { - fields.add(GtfsBookingRules.PRIOR_NOTICE_SERVICE_ID_FIELD_NAME); + return fields; + } + + private static void validatePriorNoticeDayRange( + GtfsBookingRules entity, NoticeContainer noticeContainer) { + if (entity.hasPriorNoticeLastDay() + && entity.hasPriorNoticeStartDay() + && entity.priorNoticeLastDay() > entity.priorNoticeStartDay()) { + noticeContainer.addValidationNotice(new PriorNoticeLastDayAfterStartDayNotice(entity)); } + } - return fields; + // Abstract Notice Creation using Functional Interface + @FunctionalInterface + private interface ValidationNoticeConstructor { + ValidationNotice create(GtfsBookingRules bookingRule, List forbiddenFields); } /** A forbidden field value is present for a real-time booking rule in `booking_rules.txt`. */ @@ -154,6 +185,85 @@ static class ForbiddenSameDayBookingFieldValueNotice extends ValidationNotice { } } + /** A forbidden field value is present for a prior-day booking rule in `booking_rules.txt` */ + @GtfsValidationNotice( + severity = SeverityLevel.ERROR, + files = @FileRefs(GtfsBookingRulesSchema.class)) + static class ForbiddenPriorDayBookingFieldValueNotice extends ValidationNotice { + /** The row number of the faulty record. */ + private final int csvRowNumber; + + /** The `booking_rules.booking_rule_id` of the faulty record. */ + private final String bookingRuleId; + + /** The names of the forbidden fields comma-separated. */ + private final String fieldNames; + + ForbiddenPriorDayBookingFieldValueNotice( + GtfsBookingRules bookingRule, List forbiddenFields) { + this.csvRowNumber = bookingRule.csvRowNumber(); + this.bookingRuleId = bookingRule.bookingRuleId(); + this.fieldNames = String.join(", ", forbiddenFields); + } + } + + /** + * An invalid `prior_notice_duration_min` value is present in a booking rule. + * + *

The `prior_notice_duration_max` field value needs to be greater or equal to the + * `prior_notice_duration_min` field value. + */ + @GtfsValidationNotice( + severity = SeverityLevel.ERROR, + files = @FileRefs(GtfsBookingRulesSchema.class)) + static class InvalidPriorNoticeDurationMinNotice extends ValidationNotice { + /** The row number of the faulty record. */ + private final int csvRowNumber; + + /** The `booking_rules.booking_rule_id` of the faulty record. */ + private final String bookingRuleId; + + /** The value of the `prior_notice_duration_min` field. */ + private final int priorNoticeDurationMin; + + /** The value of the `prior_notice_duration_max` field. */ + private final int priorNoticeDurationMax; + + InvalidPriorNoticeDurationMinNotice( + GtfsBookingRules bookingRule, int priorNoticeDurationMin, int priorNoticeDurationMax) { + this.csvRowNumber = bookingRule.csvRowNumber(); + this.bookingRuleId = bookingRule.bookingRuleId(); + this.priorNoticeDurationMin = priorNoticeDurationMin; + this.priorNoticeDurationMax = priorNoticeDurationMax; + } + } + + /** `prior_notice_start_day` value is forbidden when `prior_notice_duration_max` is set. */ + @GtfsValidationNotice( + severity = SeverityLevel.ERROR, + files = @FileRefs(GtfsBookingRulesSchema.class)) + static class ForbiddenPriorNoticeStartDayNotice extends ValidationNotice { + /** The row number of the faulty record. */ + private final int csvRowNumber; + + /** The `booking_rules.booking_rule_id` of the faulty record. */ + private final String bookingRuleId; + + /** The value of the `prior_notice_duration_min` field. */ + private final int priorNoticeStartDay; + + /** The value of the `prior_notice_duration_max` field. */ + private final int priorNoticeDurationMax; + + ForbiddenPriorNoticeStartDayNotice( + GtfsBookingRules bookingRule, int priorNoticeStartDay, int priorNoticeDurationMax) { + this.csvRowNumber = bookingRule.csvRowNumber(); + this.bookingRuleId = bookingRule.bookingRuleId(); + this.priorNoticeStartDay = priorNoticeStartDay; + this.priorNoticeDurationMax = priorNoticeDurationMax; + } + } + /** * Prior notice last day should not be greater than the prior notice start day in * booking_rules.txt. diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/BookingRulesEntityValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/BookingRulesEntityValidatorTest.java index 52806fe868..4aa5dce5c1 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/BookingRulesEntityValidatorTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/BookingRulesEntityValidatorTest.java @@ -99,7 +99,7 @@ public void sameDayBookingWithForbiddenFieldsShouldGenerateNotice() { .setBookingRuleId("rule-5") .setBookingType(GtfsBookingType.SAMEDAY) .setPriorNoticeLastDay(2) // Forbidden field - .setPriorNoticeStartTime(GtfsTime.fromSecondsSinceMidnight(5000)) // Forbidden field + .setPriorNoticeStartTime(GtfsTime.fromSecondsSinceMidnight(5000)) .build(); assertThat(generateNotices(bookingRule)) @@ -120,6 +120,59 @@ public void sameDayBookingWithoutForbiddenFieldsShouldNotGenerateNotice() { assertThat(generateNotices(bookingRule)).isEmpty(); } + @Test + public void priorDayBookingWithForbiddenFieldsShouldGenerateNotice() { + GtfsBookingRules bookingRule = + new GtfsBookingRules.Builder() + .setCsvRowNumber(1) + .setBookingRuleId("rule-7") + .setBookingType(GtfsBookingType.PRIORDAY) + .setPriorNoticeDurationMin(30) // Forbidden field + .setPriorNoticeDurationMax(60) // Forbidden field + .build(); + + assertThat(generateNotices(bookingRule)) + .containsExactly( + new BookingRulesEntityValidator.ForbiddenPriorDayBookingFieldValueNotice( + bookingRule, + List.of( + GtfsBookingRules.PRIOR_NOTICE_DURATION_MIN_FIELD_NAME, + GtfsBookingRules.PRIOR_NOTICE_DURATION_MAX_FIELD_NAME))); + } + + @Test + public void invalidPriorNoticeDurationMinShouldGenerateNotice() { + GtfsBookingRules bookingRule = + new GtfsBookingRules.Builder() + .setCsvRowNumber(1) + .setBookingRuleId("rule-8") + .setBookingType(GtfsBookingType.SAMEDAY) + .setPriorNoticeDurationMin(60) // Invalid: greater than max + .setPriorNoticeDurationMax(30) + .build(); + + assertThat(generateNotices(bookingRule)) + .containsExactly( + new BookingRulesEntityValidator.InvalidPriorNoticeDurationMinNotice( + bookingRule, 60, 30)); + } + + @Test + public void forbiddenPriorNoticeStartDayShouldGenerateNotice() { + GtfsBookingRules bookingRule = + new GtfsBookingRules.Builder() + .setCsvRowNumber(1) + .setBookingRuleId("rule-9") + .setBookingType(GtfsBookingType.SAMEDAY) + .setPriorNoticeDurationMax(30) // Duration max is set + .setPriorNoticeStartDay(5) // Forbidden when duration max is set + .build(); + + assertThat(generateNotices(bookingRule)) + .containsExactly( + new BookingRulesEntityValidator.ForbiddenPriorNoticeStartDayNotice(bookingRule, 5, 30)); + } + @Test public void priorNoticeLastDayAfterStartDayShouldGenerateNotice() { GtfsBookingRules bookingRule = diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/NoticeFieldsTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/NoticeFieldsTest.java index 8f1149a163..7b8ee11c97 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/NoticeFieldsTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/NoticeFieldsTest.java @@ -202,8 +202,10 @@ public void testNoticeClassFieldNames() { "locationId", "bookingRuleId", "fieldNames", - "priorNoticeLastDay", - "priorNoticeStartDay"); + "priorNoticeDurationMin", + "priorNoticeDurationMax", + "priorNoticeStartDay", + "priorNoticeLastDay"); } private static List discoverValidationNoticeFieldNames() {