feat: separate out missing_prior_day_booking_field_value (#1996)

Author: cka-y

core/src/main/java/org/mobilitydata/gtfsvalidator/notice/MissingPriorNoticeLastDayNotice.java

@@ -0,0 +1,20 @@
+package org.mobilitydata.gtfsvalidator.notice;
+
+import org.mobilitydata.gtfsvalidator.annotation.GtfsValidationNotice;
+
+/**
+ * The `prior_notice_last_day` value is required for prior day `booking_type` in booking_rules.txt.
+ */
+@GtfsValidationNotice(severity = SeverityLevel.ERROR)
+public class MissingPriorNoticeLastDayNotice 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;
+
+  public MissingPriorNoticeLastDayNotice(int csvRowNumber, String bookingRuleId) {
+    this.csvRowNumber = csvRowNumber;
+    this.bookingRuleId = bookingRuleId;
+  }
+}

core/src/main/java/org/mobilitydata/gtfsvalidator/notice/MissingPriorNoticeLastTimeNotice.java

@@ -0,0 +1,20 @@
+package org.mobilitydata.gtfsvalidator.notice;
+
+import org.mobilitydata.gtfsvalidator.annotation.GtfsValidationNotice;
+
+/**
+ * The `prior_notice_last_time` value is required for prior day `booking_type` in booking_rules.txt.
+ */
+@GtfsValidationNotice(severity = SeverityLevel.ERROR)
+public class MissingPriorNoticeLastTimeNotice 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;
+
+  public MissingPriorNoticeLastTimeNotice(int csvRowNumber, String bookingRuleId) {
+    this.csvRowNumber = csvRowNumber;
+    this.bookingRuleId = bookingRuleId;
+  }
+}

core/src/main/java/org/mobilitydata/gtfsvalidator/notice/deprecated/MissingPriorDayBookingFieldValueNotice.java

@@ -0,0 +1,34 @@
+package org.mobilitydata.gtfsvalidator.notice.deprecated;
+
+import org.mobilitydata.gtfsvalidator.annotation.GtfsValidationNotice;
+import org.mobilitydata.gtfsvalidator.notice.MissingPriorNoticeLastDayNotice;
+import org.mobilitydata.gtfsvalidator.notice.MissingPriorNoticeLastTimeNotice;
+import org.mobilitydata.gtfsvalidator.notice.SeverityLevel;
+import org.mobilitydata.gtfsvalidator.notice.ValidationNotice;
+
+/**
+ * The `prior_notice_last_day` and the `prior_notice_last_time` values are required for prior day
+ * `booking_type` in booking_rules.txt.
+ */
+@GtfsValidationNotice(
+    severity = SeverityLevel.ERROR,
+    deprecated = true,
+    deprecationVersion = "7.0.0",
+    deprecationReason =
+        "Separated into `missing_prior_notice_last_day` and `missing_prior_notice_last_time` notices",
+    replacementNotices = {
+      MissingPriorNoticeLastDayNotice.class,
+      MissingPriorNoticeLastTimeNotice.class
+    })
+public class MissingPriorDayBookingFieldValueNotice 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;
+
+  MissingPriorDayBookingFieldValueNotice(int csvRowNumber, String bookingRuleId) {
+    this.csvRowNumber = csvRowNumber;
+    this.bookingRuleId = bookingRuleId;
+  }
+}

core/src/main/java/org/mobilitydata/gtfsvalidator/notice/deprecated/UnusedParentStationNotice.java

@@ -16,7 +16,7 @@
     deprecated = true,
     deprecationVersion = "7.0.0",
     deprecationReason = "Renamed to `unused_station`",
-    replacementNotice = UnusedStationNotice.class)
+    replacementNotices = {UnusedStationNotice.class})
 class UnusedParentStationNotice extends ValidationNotice {
   /** The row number of the faulty record. */
   private final int csvRowNumber;

core/src/main/java/org/mobilitydata/gtfsvalidator/notice/schema/NoticeSchema.java

@@ -1,5 +1,6 @@
 package org.mobilitydata.gtfsvalidator.notice.schema;
 
+import java.util.List;
 import java.util.Map;
 import java.util.TreeMap;
 import javax.annotation.Nullable;
@@ -57,10 +58,10 @@ public class NoticeSchema {
   @Nullable private String deprecationVersion;
 
   /**
-   * Replacement notice code for the deprecated notice. This field is only used if {@link
-   * #deprecated} is true and the notice has a replacement.
+   * Replacement notice codes for the deprecated notice. This field is only used if {@link
+   * #deprecated} is true and the notice has replacements.
    */
-  @Nullable private String replacementNoticeCode;
+  @Nullable private List<String> replacementNoticeCodes;
 
   public NoticeSchema(String code, SeverityLevel severityLevel) {
     this.code = code;
@@ -135,11 +136,11 @@ public void setDeprecationVersion(@Nullable String deprecationVersion) {
   }
 
   @Nullable
-  public String getReplacementNoticeCode() {
-    return replacementNoticeCode;
+  public List<String> getReplacementNoticeCodes() {
+    return replacementNoticeCodes;
   }
 
-  public void setReplacementNoticeCode(@Nullable String replacementNoticeCode) {
-    this.replacementNoticeCode = replacementNoticeCode;
+  public void setReplacementNoticeCodes(@Nullable List<String> replacementNoticeCodes) {
+    this.replacementNoticeCodes = replacementNoticeCodes;
   }
 }

core/src/main/java/org/mobilitydata/gtfsvalidator/notice/schema/NoticeSchemaGenerator.java

@@ -26,11 +26,7 @@
 import java.io.InputStreamReader;
 import java.io.Reader;
 import java.lang.reflect.Field;
-import java.util.Arrays;
-import java.util.List;
-import java.util.Map;
-import java.util.Optional;
-import java.util.TreeMap;
+import java.util.*;
 import java.util.logging.Level;
 import org.mobilitydata.gtfsvalidator.annotation.GtfsJson;
 import org.mobilitydata.gtfsvalidator.annotation.GtfsTable;
@@ -89,11 +85,17 @@ static NoticeSchema generateSchemaForNotice(Class<? extends Notice> noticeClass)
         schema.setDeprecationReason(noticeAnnotation.deprecationReason());
         schema.setDeprecationVersion(noticeAnnotation.deprecationVersion());
         // Validate that replacement notice is not Void.class and that it extends ValidationNotice
-        if (noticeAnnotation.replacementNotice() != Void.class
-            && ValidationNotice.class.isAssignableFrom(noticeAnnotation.replacementNotice())) {
-          String replacementNoticeCode =
-              Notice.getCode(noticeAnnotation.replacementNotice().asSubclass(Notice.class));
-          schema.setReplacementNoticeCode(replacementNoticeCode);
+        List<String> replacementNotices = new ArrayList<>();
+        for (Class<?> replacementNotice : noticeAnnotation.replacementNotices()) {
+          if (replacementNotice == Void.class
+              || !ValidationNotice.class.isAssignableFrom(replacementNotice)) {
+            continue;
+          }
+          String replacementNoticeCode = Notice.getCode(replacementNotice.asSubclass(Notice.class));
+          replacementNotices.add(replacementNoticeCode);
+        }
+        if (!replacementNotices.isEmpty()) {
+          schema.setReplacementNoticeCodes(replacementNotices);
         }
       }
     }

main/src/main/java/org/mobilitydata/gtfsvalidator/validator/BookingRulesEntityValidator.java

@@ -6,9 +6,7 @@
 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.SeverityLevel;
-import org.mobilitydata.gtfsvalidator.notice.ValidationNotice;
+import org.mobilitydata.gtfsvalidator.notice.*;
 import org.mobilitydata.gtfsvalidator.table.GtfsBookingRules;
 import org.mobilitydata.gtfsvalidator.table.GtfsBookingRulesSchema;
 import org.mobilitydata.gtfsvalidator.table.GtfsBookingType;
@@ -60,9 +58,15 @@ private static void validatePriorNoticeDurationMin(
 
   private static void validateMissingPriorDayBookingFields(
       GtfsBookingRules entity, NoticeContainer noticeContainer) {
-    if (entity.bookingType() == GtfsBookingType.PRIORDAY
-        && (!entity.hasPriorNoticeLastDay() || !entity.hasPriorNoticeLastTime())) {
-      noticeContainer.addValidationNotice(new MissingPriorDayBookingFieldValueNotice(entity));
+    if (entity.bookingType() == GtfsBookingType.PRIORDAY) {
+      if (!entity.hasPriorNoticeLastDay()) {
+        noticeContainer.addValidationNotice(
+            new MissingPriorNoticeLastDayNotice(entity.csvRowNumber(), entity.bookingRuleId()));
+      }
+      if (!entity.hasPriorNoticeLastTime()) {
+        noticeContainer.addValidationNotice(
+            new MissingPriorNoticeLastTimeNotice(entity.csvRowNumber(), entity.bookingRuleId()));
+      }
     }
   }
 
@@ -333,26 +337,6 @@ static class PriorNoticeLastDayAfterStartDayNotice extends ValidationNotice {
     }
   }
 
-  /**
-   * `prior_notice_last_day` and `prior_notice_last_time` values are required for prior day
-   * `booking_type` in booking_rules.txt.
-   */
-  @GtfsValidationNotice(
-      severity = SeverityLevel.ERROR,
-      files = @FileRefs(GtfsBookingRulesSchema.class))
-  static class MissingPriorDayBookingFieldValueNotice 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;
-
-    MissingPriorDayBookingFieldValueNotice(GtfsBookingRules bookingRule) {
-      this.csvRowNumber = bookingRule.csvRowNumber();
-      this.bookingRuleId = bookingRule.bookingRuleId();
-    }
-  }
-
   /**
    * `prior_notice_start_time` value is forbidden when `prior_notice_start_day` value is not set in
    * booking_rules.txt.

main/src/test/java/org/mobilitydata/gtfsvalidator/validator/BookingRulesEntityValidatorTest.java

@@ -7,6 +7,8 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
+import org.mobilitydata.gtfsvalidator.notice.MissingPriorNoticeLastDayNotice;
+import org.mobilitydata.gtfsvalidator.notice.MissingPriorNoticeLastTimeNotice;
 import org.mobilitydata.gtfsvalidator.notice.NoticeContainer;
 import org.mobilitydata.gtfsvalidator.notice.ValidationNotice;
 import org.mobilitydata.gtfsvalidator.table.GtfsBookingRules;
@@ -214,10 +216,13 @@ public void missingPriorDayBookingFieldValueShouldGenerateNotice() {
             .setBookingRuleId("rule-11")
             .setBookingType(GtfsBookingType.PRIORDAY) // PRIORDAY booking type
             .build(); // No prior_notice_last_day or prior_notice_last_time set
-
-    assertThat(generateNotices(bookingRule))
-        .containsExactly(
-            new BookingRulesEntityValidator.MissingPriorDayBookingFieldValueNotice(bookingRule));
+    List<ValidationNotice> expectedNotices =
+        List.of(
+            new MissingPriorNoticeLastDayNotice(
+                bookingRule.csvRowNumber(), bookingRule.bookingRuleId()),
+            new MissingPriorNoticeLastTimeNotice(
+                bookingRule.csvRowNumber(), bookingRule.bookingRuleId()));
+    assertThat(generateNotices(bookingRule)).containsExactlyElementsIn(expectedNotices);
 
     // Case 2: Missing prior_notice_last_time only
     GtfsBookingRules bookingRuleMissingTime =
@@ -230,8 +235,8 @@ public void missingPriorDayBookingFieldValueShouldGenerateNotice() {
 
     assertThat(generateNotices(bookingRuleMissingTime))
         .containsExactly(
-            new BookingRulesEntityValidator.MissingPriorDayBookingFieldValueNotice(
-                bookingRuleMissingTime));
+            new MissingPriorNoticeLastTimeNotice(
+                bookingRuleMissingTime.csvRowNumber(), bookingRuleMissingTime.bookingRuleId()));
 
     // Case 3: Missing prior_notice_last_day only
     GtfsBookingRules bookingRuleMissingDay =
@@ -245,8 +250,8 @@ public void missingPriorDayBookingFieldValueShouldGenerateNotice() {
 
     assertThat(generateNotices(bookingRuleMissingDay))
         .containsExactly(
-            new BookingRulesEntityValidator.MissingPriorDayBookingFieldValueNotice(
-                bookingRuleMissingDay));
+            new MissingPriorNoticeLastDayNotice(
+                bookingRuleMissingDay.csvRowNumber(), bookingRuleMissingDay.bookingRuleId()));
   }
 
   @Test

model/src/main/java/org/mobilitydata/gtfsvalidator/annotation/GtfsValidationNotice.java

@@ -40,10 +40,10 @@
   String deprecationVersion() default "";
 
   /**
-   * Replacement notice class for the deprecated notice. This field is only used if {@link
-   * #deprecated()} is true and the notice has a replacement.
+   * Replacement notice classes for the deprecated notice. This field is only used if {@link
+   * #deprecated()} is true and the notice has replacements.
    */
-  Class<?> replacementNotice() default Void.class;
+  Class<?>[] replacementNotices() default {};
 
   /**
    * GTFS specification section references. For specific file references, use {@link #files()}

web/client/src/routes/rules.html/+page.svelte

@@ -234,10 +234,12 @@
             >
               {rule.deprecationVersion}
             </a></b>.
-              {#if (rule.replacementNoticeCode)}
-                It has been replaced by
-                <code><a href="#{rule.replacementNoticeCode}-rule">{rule.replacementNoticeCode}</a></code>.
-              {/if}
+            {#if rule.replacementNoticeCodes}
+              It has been replaced by
+              {#each rule.replacementNoticeCodes as noticeCode, index}
+                <code><a href="#{noticeCode}-rule">{noticeCode}</a></code>{index < rule.replacementNoticeCodes.length - 1 ? ', ' : '.'}
+              {/each}
+            {/if}
             <blockquote>
               <b>Deprecation reason:</b>
               {@html marked.parse(rule?.deprecationReason ?? '')}