diff --git a/mycore-xeditor/pom.xml b/mycore-xeditor/pom.xml index 37dec93696..1e2e3819e2 100644 --- a/mycore-xeditor/pom.xml +++ b/mycore-xeditor/pom.xml @@ -74,5 +74,10 @@ xalan xalan + + org.junit.jupiter + junit-jupiter-api + test + diff --git a/mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRExternalValidator.java b/mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRExternalValidator.java index fce72ae67e..62940bead9 100644 --- a/mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRExternalValidator.java +++ b/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; diff --git a/mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRRequiredValidator.java b/mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRRequiredValidator.java index 60e7a947d6..ef7e87ce09 100644 --- a/mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRRequiredValidator.java +++ b/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; } diff --git a/mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRValidationResults.java b/mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRValidationResults.java index 1b8fe7b8f8..b86513c3cc 100644 --- a/mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRValidationResults.java +++ b/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. + *

+ * 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 xPath2Marker = new HashMap<>(); - private SequencedMap xPath2FailedRule = new LinkedHashMap<>(); + private SequencedMap 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 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 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); + } + } } diff --git a/mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRValidator.java b/mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRValidator.java index d6df079917..260f1b8811 100644 --- a/mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRValidator.java +++ b/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; diff --git a/mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRXEditorValidator.java b/mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRXEditorValidator.java index 4bde52ebe0..cf2f20cc78 100644 --- a/mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRXEditorValidator.java +++ b/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 getFailedRules() { return results.getFailedRules(); } diff --git a/mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRXPathTestValidator.java b/mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRXPathTestValidator.java index 26181b4510..74497b0736 100644 --- a/mycore-xeditor/src/main/java/org/mycore/frontend/xeditor/validation/MCRXPathTestValidator.java +++ b/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)) { diff --git a/mycore-xeditor/src/test/java/org/mycore/frontend/xeditor/validation/MCRXEditorValidatorValidationResultsTest.java b/mycore-xeditor/src/test/java/org/mycore/frontend/xeditor/validation/MCRXEditorValidatorValidationResultsTest.java new file mode 100644 index 0000000000..86afc2e143 --- /dev/null +++ b/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 . + */ + +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 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 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 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(); + } +}