Improve robustness for model validation

Author: sven1103

src/main/java/life/qbic/compass/SignPostingProcessor.java

@@ -242,7 +242,7 @@ private static List<WebLink> applyModelValidation(List<WebLink> webLinks,
     var result = modelValidator.validate(webLinks);
     var sanitizedLinks = new ArrayList<WebLink>();
     for (int index = 0; index < webLinks.size(); index++) {
-      if (!result.blockingLinkByIndex()[index]) {
+      if (!result.blockingIndices().get(index)) {
         sanitizedLinks.add(webLinks.get(index));
       }
     }

src/main/java/life/qbic/compass/spi/WebLinkModelValidator.java

@@ -1,6 +1,6 @@
 package life.qbic.compass.spi;
 
-import java.util.Arrays;
+import java.util.BitSet;
 import java.util.List;
 import java.util.Objects;
 import life.qbic.linksmith.model.WebLink;
@@ -10,7 +10,7 @@
  * Contract for validating {@link WebLink} objects at the <em>model level</em>.
  *
  * <p><strong>Audience:</strong> Maintainers of the Compass library.
- * This interface is internal to the validation layer and is not intended as a public extension
+ * This interface is part of the internal validation layer and is not intended as a public extension
  * point for end users.</p>
  *
  * <h2>Purpose</h2>
@@ -36,8 +36,8 @@
  * </p>
  * <ul>
  *   <li>Parsers produce {@link WebLink} instances (possibly permissive).</li>
- *   <li>{@code WebLinkModelValidator}s ensure the model obeys core Web Linking rules
- *       (e.g. RFC&nbsp;8288 invariants).</li>
+ *   <li>{@code WebLinkModelValidator}s ensure the model obeys core Web Linking
+ *       invariants (e.g. RFC&nbsp;8288 constraints).</li>
  *   <li>Signposting validators operate on a trusted model to apply FAIR-specific semantics.</li>
  * </ul>
  *
@@ -84,45 +84,58 @@ public interface WebLinkModelValidator {
    *
    * <p>
    * Implementations must inspect each element independently and accumulate all detected issues into
-   * the returned {@link IssueReport}. Validation must not stop after the first failure.
+   * the returned {@link ModelValidationResult}. Validation must not stop after the first failure.
    * </p>
    *
    * @param webLinks the list of {@link WebLink} instances to validate (must not be {@code null})
-   * @return an {@link ModelValidationResult} containing all detected issues and the indices of
-   * weblinks with recorded ERROR
+   * @return a {@link ModelValidationResult} containing all detected issues and information about
+   * which input elements are considered blocking
    * @throws NullPointerException if {@code webLinks} is {@code null}
    */
   ModelValidationResult validate(List<WebLink> webLinks);
 
-
-  record ModelValidationResult(IssueReport issueReport, boolean[] blockingLinkByIndex) {
+  /**
+   * Result object returned by {@link WebLinkModelValidator} implementations.
+   *
+   * <p>
+   * The result consists of:
+   * </p>
+   * <ul>
+   *   <li>an {@link IssueReport} containing all detected validation issues, and</li>
+   *   <li>a {@link BitSet} indicating which input indices correspond to
+   *       <em>blocking</em> model violations.</li>
+   * </ul>
+   *
+   * <p>
+   * A blocking index represents a {@link WebLink} that must not be used for
+   * downstream semantic processing (e.g. Signposting validation).
+   * </p>
+   *
+   * <p>
+   * This record is <strong>immutable</strong>:
+   * </p>
+   * <ul>
+   *   <li>The contained {@link IssueReport} is defensively copied.</li>
+   *   <li>The {@link BitSet} is defensively copied using {@link BitSet#clone()}.</li>
+   * </ul>
+   *
+   * <p>
+   * Modifying the original {@link IssueReport} or {@link BitSet} passed to the
+   * constructor has no effect on the created result instance.
+   * </p>
+   *
+   * @param issueReport     all detected validation issues
+   * @param blockingIndices bit set marking indices of blocking WebLinks
+   * @since 1.0.0
+   */
+  record ModelValidationResult(IssueReport issueReport, BitSet blockingIndices) {
 
     public ModelValidationResult {
-      issueReport = new IssueReport(List.copyOf(issueReport.issues()));
-      blockingLinkByIndex = Arrays.copyOf(blockingLinkByIndex, blockingLinkByIndex.length);
-    }
+      Objects.requireNonNull(issueReport, "issueReport");
+      Objects.requireNonNull(blockingIndices, "blockingIndices");
 
-    @Override
-    public boolean equals(Object o) {
-      if (o == null || getClass() != o.getClass()) {
-        return false;
-      }
-      ModelValidationResult that = (ModelValidationResult) o;
-      return Objects.equals(issueReport, that.issueReport) && Objects.deepEquals(
-          blockingLinkByIndex, that.blockingLinkByIndex);
-    }
-
-    @Override
-    public int hashCode() {
-      return Objects.hash(issueReport, Arrays.hashCode(blockingLinkByIndex));
-    }
-
-    @Override
-    public String toString() {
-      return "ModelValidationResult{" +
-          "issueReport=" + issueReport +
-          ", blockingLinkByIndex=" + Arrays.toString(blockingLinkByIndex) +
-          '}';
+      issueReport = new IssueReport(List.copyOf(issueReport.issues()));
+      blockingIndices = (BitSet) blockingIndices.clone();
     }
   }
 }

src/main/java/life/qbic/compass/validation/Rfc8288ModelValidator.java

@@ -2,6 +2,7 @@
 
 import java.net.URI;
 import java.util.ArrayList;
+import java.util.BitSet;
 import java.util.List;
 import java.util.Objects;
 import java.util.regex.Pattern;
@@ -147,7 +148,17 @@ public ModelValidationResult validate(List<WebLink> webLinks) {
       }
       validate(currentLink, issueSink);
     }
-    return new ModelValidationResult(new IssueReport(issueSink.issues), issueSink.blockingLinkIndices);
+    return new ModelValidationResult(new IssueReport(issueSink.issues), toBitSet(issueSink.blockingLinkIndices));
+  }
+
+  private static BitSet toBitSet(boolean[] flags) {
+    BitSet bitset = new BitSet(flags.length);
+    for (int index = 0; index < flags.length; index++) {
+      if (flags[index]) {
+        bitset.set(index);
+      }
+    }
+    return bitset;
   }
 
   /**

src/test/groovy/life/qbic/compass/spi/WebLinkModelValidatorSpec.groovy

@@ -0,0 +1,87 @@
+package life.qbic.compass.spi
+
+import life.qbic.linksmith.spi.WebLinkValidator
+import spock.lang.Specification
+
+class WebLinkModelValidatorSpec extends Specification {
+    def "constructor performs defensive copy of IssueReport issues list"() {
+        given:
+        def originalIssues = new ArrayList<WebLinkValidator.Issue>()
+        originalIssues.add(WebLinkValidator.Issue.warning("w1"))
+        def originalReport = new WebLinkValidator.IssueReport(originalIssues)
+
+        and:
+        def flags = new BitSet()
+        flags.set(1)
+
+        when:
+        def result = new WebLinkModelValidator.ModelValidationResult(originalReport, flags)
+
+        and: "mutate original list after construction"
+        originalIssues.add(WebLinkValidator.Issue.error("e1"))
+
+        then: "result's report is unaffected"
+        result.issueReport().issues()*.message() == ["w1"]
+    }
+
+    def "constructor performs defensive copy of BitSet"() {
+        given:
+        def report = new WebLinkValidator.IssueReport([WebLinkValidator.Issue.warning("w1")])
+
+        and:
+        def original = new BitSet()
+        original.set(0)
+        original.set(3)
+
+        when:
+        def result = new WebLinkModelValidator.ModelValidationResult(report, original)
+
+        and: "mutate original BitSet after construction"
+        original.clear(0)
+        original.set(5)
+
+        then: "result's BitSet is unaffected (still has original bits)"
+        result.blockingIndices().get(0)
+        result.blockingIndices().get(3)
+        !result.blockingIndices().get(5)
+    }
+
+    def "equals and hashCode consider IssueReport and BitSet content"() {
+        given:
+        def report1 = new WebLinkValidator.IssueReport([WebLinkValidator.Issue.warning("w1"), WebLinkValidator.Issue.error("e1")])
+        def report2 = new WebLinkValidator.IssueReport([WebLinkValidator.Issue.warning("w1"), WebLinkValidator.Issue.error("e1")])
+
+        and:
+        def b1 = new BitSet()
+        b1.set(2); b1.set(7)
+
+        def b2 = new BitSet()
+        b2.set(2); b2.set(7)
+
+        when:
+        def r1 = new WebLinkModelValidator.ModelValidationResult(report1, b1)
+        def r2 = new WebLinkModelValidator.ModelValidationResult(report2, b2)
+
+        then:
+        r1 == r2
+        r1.hashCode() == r2.hashCode()
+    }
+
+    def "constructor throws NullPointerException for null fields"() {
+        given:
+        def report = new WebLinkValidator.IssueReport([])
+        def bits = new BitSet()
+
+        when:
+        new WebLinkModelValidator.ModelValidationResult(null, bits)
+
+        then:
+        thrown(NullPointerException)
+
+        when:
+        new WebLinkModelValidator.ModelValidationResult(report, null)
+
+        then:
+        thrown(NullPointerException)
+    }
+}