feat: added forbidden_prior_day_booking_field_value, `invalid_prior…

…_notice_duration_min` and `forbidden_prior_notice_start_day` notices (#1860)
MobilityData · Oct 2, 2024 · 817c6e4 · 817c6e4
1 parent 63dab81
commit 817c6e4
Show file tree

Hide file tree

Showing 3 changed files with 232 additions and 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
@@ -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<GtfsBooki
 
   @Override
   public void validate(GtfsBookingRules entity, NoticeContainer noticeContainer) {
-    validateForbiddenRealTimeFields(entity, noticeContainer);
-    validateSameDayFields(entity, noticeContainer);
-    validatePriorNotice(entity, noticeContainer);
+    validateBookingType(
+        entity,
+        GtfsBookingType.REALTIME,
+        BookingRulesEntityValidator::findForbiddenRealTimeFields,
+        ForbiddenRealTimeBookingFieldValueNotice::new,
+        noticeContainer);
+    validateBookingType(
+        entity,
+        GtfsBookingType.SAMEDAY,
+        BookingRulesEntityValidator::findForbiddenSameDayFields,
+        ForbiddenSameDayBookingFieldValueNotice::new,
+        noticeContainer);
+    validateBookingType(
+        entity,
+        GtfsBookingType.PRIORDAY,
+        BookingRulesEntityValidator::findForbiddenPriorDayFields,
+        ForbiddenPriorDayBookingFieldValueNotice::new,
+        noticeContainer);
+    validatePriorNoticeDurationMin(entity, noticeContainer);
+    validatePriorNoticeStartDay(entity, noticeContainer);
+    validatePriorNoticeDayRange(entity, noticeContainer);
   }
 
-  private void validatePriorNotice(GtfsBookingRules entity, NoticeContainer noticeContainer) {
-    if (entity.hasPriorNoticeLastDay()
-        && entity.hasPriorNoticeStartDay()
-        && entity.priorNoticeLastDay() > 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<String> 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<GtfsBookingRules, List<String>> forbiddenFieldsFinder,
+      ValidationNoticeConstructor validationNoticeConstructor,
+      NoticeContainer noticeContainer) {
+
+    if (entity.bookingType() != bookingType) {
       return;
     }
 
-    // Retrieve the list of forbidden fields
-    List<String> forbiddenFields = findForbiddenSameDayFields(entity);
+    List<String> 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<String> findForbiddenSameDayFields(GtfsBookingRules bookingRule) {
-    List<String> 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<String> 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<String> findForbiddenRealTimeFields(GtfsBookingRules bookingRule) {
-    List<String> fields = new ArrayList<>();
+  private static List<String> 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<String> findForbiddenFields(Object... conditionsAndFields) {
+    List<String> 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<String> 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<String> 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.
+   *
+   * <p>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/...c/test/java/org/mobilitydata/gtfsvalidator/validator/BookingRulesEntityValidatorTest.java b/...c/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
@@ -202,8 +202,10 @@ public void testNoticeClassFieldNames() {
             "locationId",
             "bookingRuleId",
             "fieldNames",
-            "priorNoticeLastDay",
-            "priorNoticeStartDay");
+            "priorNoticeDurationMin",
+            "priorNoticeDurationMax",
+            "priorNoticeStartDay",
+            "priorNoticeLastDay");
   }
 
   private static List<String> discoverValidationNoticeFieldNames() {