MCR-3773 Fix XEditor validation failures sharing the same XPath

mycore-xeditor/pom.xml

@@ -74,5 +74,10 @@
       <groupId>xalan</groupId>
       <artifactId>xalan</artifactId>
     </dependency>
+    <dependency>
+      <groupId>org.junit.jupiter</groupId>
+      <artifactId>junit-jupiter-api</artifactId>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 </project>

mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRExternalValidator.java

@@ -62,10 +62,6 @@ public boolean validateBinding(MCRValidationResults results, MCRBinding binding)
         boolean isValid = true; // all nodes must validate
         for (Object node : binding.getBoundNodes()) {
             String absPath = MCRXPathBuilder.buildXPath(node);
-            if (results.hasError(absPath)) {
-                continue;
-            }
-
             Boolean result = isValid(node);
             if (result == null) {
                 continue;

mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRRequiredValidator.java

@@ -46,10 +46,6 @@ public boolean validateBinding(MCRValidationResults results, MCRBinding binding)
         }
 
         String absPath = binding.getAbsoluteXPath();
-        if (results.hasError(absPath)) {
-            return true;
-        }
-
         if (!isRelevant(binding)) {
             return true;
         }

mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRValidationResults.java

@@ -25,7 +25,16 @@
 import java.util.SequencedMap;
 
 import org.mycore.common.config.MCRConfiguration2;
-
+import org.w3c.dom.Node;
+
+/**
+ * Collects validation markers and failed validation rules.
+ * <p>
+ * A marker describes the combined validation state of an absolute XPath: once a rule fails there, the marker remains
+ * {@link #MARKER_ERROR}. Failed rules are tracked separately by absolute XPath and originating validation rule element.
+ * Thus, distinct {@code xed:validate} declarations can be reported for the same XPath, while multiple internal
+ * validators created from one declaration produce only one failure for that XPath.
+ */
 public class MCRValidationResults {
 
     static final String MARKER_DEFAULT;
@@ -43,7 +52,7 @@ public class MCRValidationResults {
 
     private Map<String, String> xPath2Marker = new HashMap<>();
 
-    private SequencedMap<String, MCRValidator> xPath2FailedRule = new LinkedHashMap<>();
+    private SequencedMap<FailedRuleKey, MCRValidator> failedRules = new LinkedHashMap<>();
 
     private boolean isValid = true;
 
@@ -52,15 +61,14 @@ public boolean hasError(String xPath) {
     }
 
     public void mark(String xPath, boolean isValid, MCRValidator failedRule) {
-        if (hasError(xPath)) {
-            return;
-        }
-
         if (isValid) {
-            xPath2Marker.put(xPath, MARKER_SUCCESS);
+            if (!hasError(xPath)) {
+                xPath2Marker.put(xPath, MARKER_SUCCESS);
+            }
         } else {
             xPath2Marker.put(xPath, MARKER_ERROR);
-            xPath2FailedRule.put(xPath, failedRule);
+            Node ruleElement = failedRule == null ? null : failedRule.getRuleElement();
+            failedRules.putIfAbsent(new FailedRuleKey(xPath, ruleElement), failedRule);
             this.isValid = false;
         }
     }
@@ -69,15 +77,49 @@ public String getValidationMarker(String xPath) {
         return xPath2Marker.getOrDefault(xPath, MARKER_DEFAULT);
     }
 
+    /**
+     * Returns the first failed rule for the given absolute XPath, or {@code null} if no rule failed there.
+     */
     public MCRValidator getFailedRule(String xPath) {
-        return xPath2FailedRule.get(xPath);
+        for (Map.Entry<FailedRuleKey, MCRValidator> failedRule : failedRules.entrySet()) {
+            if (xPath.equals(failedRule.getKey().xPath())) {
+                return failedRule.getValue();
+            }
+        }
+        return null;
     }
 
+    /**
+     * Returns all distinct failed rules in validation order. A rule that fails for multiple absolute XPaths occurs once
+     * per XPath, preserving the established behavior.
+     */
     public Collection<MCRValidator> getFailedRules() {
-        return xPath2FailedRule.values();
+        return failedRules.values();
     }
 
     public boolean isValid() {
         return isValid;
     }
+
+    private record FailedRuleKey(String xPath, Node ruleElement) {
+
+        /**
+         * DOM {@link Node} does not define Java {@link Object#equals(Object)} semantics. Use object identity here so
+         * equality consistently means that failures originated from the exact same validation rule element,
+         * independent of the DOM implementation.
+         */
+        @Override
+        public boolean equals(Object obj) {
+            return obj == this || obj instanceof FailedRuleKey other
+                && xPath.equals(other.xPath) && ruleElement == other.ruleElement;
+        }
+
+        /**
+         * Uses the identity hash corresponding to the identity comparison in {@link #equals(Object)}.
+         */
+        @Override
+        public int hashCode() {
+            return 31 * xPath.hashCode() + System.identityHashCode(ruleElement);
+        }
+    }
 }

mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRValidator.java

@@ -84,10 +84,6 @@ public boolean validateBinding(MCRValidationResults results, MCRBinding binding)
             Object node = boundNodes.get(i);
 
             String absPath = MCRXPathBuilder.buildXPath(node);
-            if (results.hasError(absPath)) {
-                continue;
-            }
-
             MCRBinding nodeBinding = new MCRBinding(i + 1, binding);
             if (!isRelevant(nodeBinding)) {
                 continue;

mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRXEditorValidator.java

@@ -107,6 +107,9 @@ public boolean hasError(MCRBinding binding) {
         return results.hasError(binding.getAbsoluteXPath());
     }
 
+    /**
+     * Returns the first failed rule for the binding's absolute XPath.
+     */
     public MCRValidator getFailedRule(MCRBinding binding) {
         return results.getFailedRule(binding.getAbsoluteXPath());
     }
@@ -117,6 +120,10 @@ public void setValidationMarker(MCRBinding binding) {
         session.getVariables().put(XED_VALIDATION_MARKER, marker);
     }
 
+    /**
+     * Returns failed validation rules in validation order. The validation marker is maintained independently per XPath;
+     * multiple distinct rule elements can therefore be returned for one error-marked XPath.
+     */
     public Collection<MCRValidator> getFailedRules() {
         return results.getFailedRules();
     }

mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRXPathTestValidator.java

@@ -56,10 +56,6 @@ public boolean validateBinding(MCRValidationResults results, MCRBinding binding)
             Object node = boundNodes.get(i);
 
             String absPath = MCRXPathBuilder.buildXPath(node);
-            if (results.hasError(absPath)) {
-                continue;
-            }
-
             MCRBinding nodeBinding = new MCRBinding(i + 1, binding);
             MCRXPathEvaluator evaluator = nodeBinding.getXPathEvaluator();
             if (!isRelevant(nodeBinding)) {

mycore-xeditor/src/test/java/org/mycore/frontend/xeditor/validation/MCRXEditorValidatorValidationResultsTest.java

@@ -0,0 +1,137 @@
+/*
+ * This file is part of ***  M y C o R e  ***
+ * See https://www.mycore.de/ for details.
+ *
+ * MyCoRe is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * MyCoRe is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with MyCoRe.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package org.mycore.frontend.xeditor.validation;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertSame;
+
+import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.jaxen.JaxenException;
+import org.jdom2.Document;
+import org.jdom2.Element;
+import org.jdom2.JDOMException;
+import org.jdom2.output.DOMOutputter;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mycore.common.xml.MCRNodeBuilder;
+import org.mycore.frontend.xeditor.MCRBinding;
+import org.mycore.frontend.xeditor.MCREditorSession;
+import org.mycore.test.MyCoReTest;
+
+@MyCoReTest
+class MCRXEditorValidatorValidationResultsTest {
+
+    private static final AtomicInteger EXTERNAL_VALIDATIONS = new AtomicInteger();
+
+    @BeforeEach
+    void resetExternalValidations() {
+        EXTERNAL_VALIDATIONS.set(0);
+    }
+
+    @Test
+    void reportsDistinctRulesForSameXPathInDeclarationOrder() throws JaxenException, JDOMException {
+        MCREditorSession session = buildSession("document[value='abc']");
+        org.w3c.dom.Element firstRule = addRule(session, "/document/value", "test", "false()", "message", "first");
+        org.w3c.dom.Element secondRule = addRule(session, "/document/value", "test", "false()", "message", "second");
+
+        assertFalse(session.getValidator().isValid());
+
+        List<MCRValidator> failedRules = List.copyOf(session.getValidator().getFailedRules());
+        assertEquals(2, failedRules.size());
+        assertSame(firstRule, failedRules.get(0).getRuleElement());
+        assertSame(secondRule, failedRules.get(1).getRuleElement());
+
+        MCRBinding binding = new MCRBinding("/document/value", false, session.getRootBinding());
+        assertSame(failedRules.get(0), session.getValidator().getFailedRule(binding));
+        binding.detach();
+    }
+
+    @Test
+    void evaluatesDistinctRuleAfterFailureAndKeepsErrorMarker() throws JaxenException, JDOMException {
+        MCREditorSession session = buildSession("document[value='abc']");
+        addRule(session, "/document/value", "test", "false()");
+        addRule(session, "/document/value", "class", getClass().getName(), "method", "recordValidation");
+
+        assertFalse(session.getValidator().isValid());
+        assertEquals(1, EXTERNAL_VALIDATIONS.get());
+        checkResult(session, "/document/value", MCRValidationResults.MARKER_ERROR);
+    }
+
+    @Test
+    void reportsOnlyOneFailureForMultipleValidatorsFromSameRule() throws JaxenException, JDOMException {
+        MCREditorSession session = buildSession("document[value='abc']");
+        org.w3c.dom.Element rule = addRule(session, "/document/value", "matches", "[0-9]+", "test", "false()");
+
+        assertFalse(session.getValidator().isValid());
+
+        List<MCRValidator> failedRules = List.copyOf(session.getValidator().getFailedRules());
+        assertEquals(1, failedRules.size());
+        assertSame(rule, failedRules.get(0).getRuleElement());
+    }
+
+    @Test
+    void reportsOneRuleOnceForEachFailedBoundNode() throws JaxenException, JDOMException {
+        MCREditorSession session = buildSession("document[value='a'][value='b']");
+        org.w3c.dom.Element rule = addRule(session, "/document/value", "test", "false()");
+
+        assertFalse(session.getValidator().isValid());
+
+        List<MCRValidator> failedRules = List.copyOf(session.getValidator().getFailedRules());
+        assertEquals(2, failedRules.size());
+        assertSame(failedRules.get(0), failedRules.get(1));
+        assertSame(rule, failedRules.get(0).getRuleElement());
+        checkResult(session, "/document/value[1]", MCRValidationResults.MARKER_ERROR);
+        checkResult(session, "/document/value[2]", MCRValidationResults.MARKER_ERROR);
+    }
+
+    public static boolean recordValidation(String value) {
+        EXTERNAL_VALIDATIONS.incrementAndGet();
+        return true;
+    }
+
+    private MCREditorSession buildSession(String template) throws JaxenException {
+        MCREditorSession session = new MCREditorSession();
+        Document editedXML = new Document(new MCRNodeBuilder().buildElement(template, null, null));
+        session.setEditedXML(editedXML);
+        return session;
+    }
+
+    private org.w3c.dom.Element addRule(MCREditorSession session, String baseXPath, String... attributes)
+        throws JDOMException {
+        Element rule = new Element("validation-rule");
+        for (int i = 0; i < attributes.length;) {
+            rule.setAttribute(attributes[i++], attributes[i++]);
+        }
+        new Document(rule);
+        org.w3c.dom.Element ruleAsDOMElement = new DOMOutputter().output(rule);
+        session.getValidator().addRule(baseXPath, ruleAsDOMElement);
+        return ruleAsDOMElement;
+    }
+
+    private void checkResult(MCREditorSession session, String xPath, String marker)
+        throws JaxenException, JDOMException {
+        MCRBinding binding = new MCRBinding(xPath, false, session.getRootBinding());
+        session.getValidator().setValidationMarker(binding);
+        assertEquals(marker, session.getVariables().get(MCRXEditorValidator.XED_VALIDATION_MARKER));
+        binding.detach();
+    }
+}