From 52668e1bfaaabdbf153a6dd1dfd00872faddd842 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Fri, 4 Sep 2026 12:57:44 +0100 Subject: [PATCH] Compare complex values by structure in valueEquals valueEquals answered true for any two complex values whose schema types matched, whatever they contained: XmlComplexContentImpl.equal_to compared the two schema types and left a "BUGBUG: by-value structure comparison undone" behind it. Two documents built from the same type are reported equal even when one carries a child the other does not (XMLBEANS-658). Compare the values instead. equal_to now walks both trees: the attributes, regardless of their order, and the child elements in document order, each compared by value in turn - so a child written in a different lexical form but with the same value still matches. For mixed content the text between the children is compared as well; comments, processing instructions and whitespace outside of mixed content carry no value and are skipped. A complex type with simple content inherits its comparison from the simple value implementation, which sees only the text - the attributes that the type adds to that value were ignored, and the tree walk would have inherited that hole for every such child. valueEquals compares those attributes once the text matches. The walk runs under the monitors valueEquals has already taken, and every object in a tree shares the monitor of its root, so children are compared without locking again - which for two trees in different synchronization domains would mean taking the global lock once per child. A value whose text does not fit its type has no value space to be compared in, and computing it throws: the CarLocationMessage test document has 12:34 in an xs:time, which is not a valid time. Comparing such a child falls back to the text as the document wrote it, so one bad value cannot turn a comparison into a throw. Co-authored-by: Claude Opus 5 (1M context) --- .../java/org/apache/xmlbeans/XmlObject.java | 7 +- .../impl/values/XmlComplexContentImpl.java | 30 +- .../xmlbeans/impl/values/XmlObjectBase.java | 27 +- .../impl/values/XmlValueComparison.java | 256 ++++++++++++++++++ .../xmlobject/detailed/ValueEqualsTest.java | 114 +++++++- 5 files changed, 424 insertions(+), 10 deletions(-) create mode 100644 src/main/java/org/apache/xmlbeans/impl/values/XmlValueComparison.java diff --git a/src/main/java/org/apache/xmlbeans/XmlObject.java b/src/main/java/org/apache/xmlbeans/XmlObject.java index 0db9d3fd3..389bda47d 100644 --- a/src/main/java/org/apache/xmlbeans/XmlObject.java +++ b/src/main/java/org/apache/xmlbeans/XmlObject.java @@ -488,7 +488,12 @@ public interface XmlObject extends XmlTokenSource { * value "1.0", the decimal "1", and the GYear "1", even though * all these objects will compare unequal to each other since they * lie in different value spaces. - * Note: as of XMLBeans 2.2.1 only implemented for simple type values. + *

+ * Complex values are compared by structure: the attributes, regardless of + * their order, and the child elements in document order, each compared by + * value in turn. For mixed content the text between the children is + * compared as well. Comments, processing instructions and whitespace that + * is not part of mixed content are ignored. */ boolean valueEquals(XmlObject obj); diff --git a/src/main/java/org/apache/xmlbeans/impl/values/XmlComplexContentImpl.java b/src/main/java/org/apache/xmlbeans/impl/values/XmlComplexContentImpl.java index eb2c76c76..d1f918696 100644 --- a/src/main/java/org/apache/xmlbeans/impl/values/XmlComplexContentImpl.java +++ b/src/main/java/org/apache/xmlbeans/impl/values/XmlComplexContentImpl.java @@ -67,11 +67,35 @@ protected void update_from_complex_content() { public void set_nil() { /* BUGBUG: what to do? */ } - // LEFT + /** + * Compares two complex values by structure: the attributes, regardless of + * their order, and the child elements in document order - each compared by + * value, so that a child written in a different lexical form but with the + * same value still matches - plus, for mixed content, the text between them. + *

+ * Comments and processing instructions are ignored, as is whitespace that is + * not part of mixed content. + */ public boolean equal_to(XmlObject complexObject) { - return _schemaType.equals(complexObject.schemaType()); + if (complexObject == this) { + return true; + } + if (complexObject == null) { + return false; + } + + SchemaType otherType = complexObject.schemaType(); + if (otherType == null || otherType.isSimpleType()) { + return false; + } + + boolean mixed = is_mixed(_schemaType) || is_mixed(otherType); + + return XmlValueComparison.complex_values_equal(this, complexObject, mixed); + } - // BUGBUG: by-value structure comparison undone + private static boolean is_mixed(SchemaType type) { + return type.getContentType() == SchemaType.MIXED_CONTENT; } // LEFT diff --git a/src/main/java/org/apache/xmlbeans/impl/values/XmlObjectBase.java b/src/main/java/org/apache/xmlbeans/impl/values/XmlObjectBase.java index 8c50ed351..b80f9f86f 100644 --- a/src/main/java/org/apache/xmlbeans/impl/values/XmlObjectBase.java +++ b/src/main/java/org/apache/xmlbeans/impl/values/XmlObjectBase.java @@ -2410,11 +2410,32 @@ private boolean valueEqualsImpl(XmlObject xmlobj) { return false; } - if (xmlobj.schemaType().getSimpleVariety() == SchemaType.UNION) { - return (underlying(xmlobj)).equal_to(this); + boolean equal = (xmlobj.schemaType().getSimpleVariety() == SchemaType.UNION) + ? (underlying(xmlobj)).equal_to(this) + : equal_to(xmlobj); + + if (!equal) { + return false; } - return equal_to(xmlobj); + // A complex type with simple content has just had its text value + // compared, by the simple value implementation it inherits; the + // attributes the type adds to that value are part of it too. + if (typethis.getContentType() == SchemaType.SIMPLE_CONTENT && + typeother.getContentType() == SchemaType.SIMPLE_CONTENT) { + return XmlValueComparison.attributes_equal(this, xmlobj); + } + + return true; + } + + /** + * Compares values when the caller already holds the monitor of both + * objects, as complex content comparison does while walking two trees: + * every object in a tree shares the monitor of its root. + */ + final boolean value_equals_locked(XmlObject xmlobj) { + return valueEqualsImpl(xmlobj); } public final boolean valueEquals(XmlObject xmlobj) { diff --git a/src/main/java/org/apache/xmlbeans/impl/values/XmlValueComparison.java b/src/main/java/org/apache/xmlbeans/impl/values/XmlValueComparison.java new file mode 100644 index 000000000..6b0b66be5 --- /dev/null +++ b/src/main/java/org/apache/xmlbeans/impl/values/XmlValueComparison.java @@ -0,0 +1,256 @@ +/* Copyright 2004 The Apache Software Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.xmlbeans.impl.values; + +import org.apache.xmlbeans.XmlCursor; +import org.apache.xmlbeans.XmlCursor.TokenType; +import org.apache.xmlbeans.XmlObject; +import org.apache.xmlbeans.impl.common.XMLChar; + +import javax.xml.namespace.QName; +import java.util.HashMap; +import java.util.Map; + +/** + * Compares complex values by structure, on behalf of + * {@link XmlObjectBase#valueEquals(XmlObject)}. + *

+ * The callers hold the monitor of both objects being compared, and every object + * in a tree shares the monitor of its root, so nothing here locks again. + */ +final class XmlValueComparison { + private XmlValueComparison() { + } + + /** + * Compares two complex values: their attributes, regardless of order, and + * their content - the child elements in document order, each compared by + * value, plus the text between them when the content is mixed. + *

+ * Comments and processing instructions are ignored, as is whitespace that is + * not part of mixed content. + */ + static boolean complex_values_equal(XmlObject a, XmlObject b, boolean mixed) { + if (!has_store(a) || !has_store(b)) { + // a value that is not attached to a store has no content to walk + return !has_store(a) && !has_store(b); + } + + try (XmlCursor cursorA = a.newCursor(); + XmlCursor cursorB = b.newCursor()) { + return attributes_equal(cursorA, cursorB) && + content_equal(cursorA, cursorB, mixed); + } + } + + /** + * Compares the attributes of two values whose content was compared + * elsewhere, as is the case for a complex type with simple content: the + * simple value implementation compares the text but knows nothing of the + * attributes the type adds to it. + *

+ * A value object without a store cannot carry attributes, so it is equal + * only to a value that has none. + */ + static boolean attributes_equal(XmlObject a, XmlObject b) { + boolean storedA = has_store(a); + boolean storedB = has_store(b); + + if (!storedA && !storedB) { + return true; + } + if (!storedA) { + return !has_attributes(b); + } + if (!storedB) { + return !has_attributes(a); + } + + try (XmlCursor cursorA = a.newCursor(); + XmlCursor cursorB = b.newCursor()) { + return attributes_equal(cursorA, cursorB); + } + } + + private static boolean has_store(XmlObject o) { + return !(o instanceof XmlObjectBase) || ((XmlObjectBase) o).has_store(); + } + + private static boolean has_attributes(XmlObject o) { + try (XmlCursor c = o.newCursor()) { + return c.toFirstAttribute(); + } + } + + /** + * Compares the attributes of the elements the cursors are on. Namespace + * declarations are not attributes and take no part in the comparison. Both + * cursors are left where they were found. + */ + private static boolean attributes_equal(XmlCursor a, XmlCursor b) { + Map attributesA = attributes(a); + Map attributesB = attributes(b); + + if (!attributesA.keySet().equals(attributesB.keySet())) { + return false; + } + + for (Map.Entry attribute : attributesA.entrySet()) { + if (!values_equal(attribute.getValue(), attributesB.get(attribute.getKey()))) { + return false; + } + } + + return true; + } + + private static Map attributes(XmlCursor c) { + Map attributes = new HashMap<>(); + + c.push(); + if (c.toFirstAttribute()) { + do { + attributes.put(c.getName(), c.getObject()); + } while (c.toNextAttribute()); + } + c.pop(); + + return attributes; + } + + /** + * Walks the content of both elements in step. The cursors are left at the + * point where the walk stopped. + */ + private static boolean content_equal(XmlCursor a, XmlCursor b, boolean mixed) { + a.toFirstContentToken(); + b.toFirstContentToken(); + + for (; ; ) { + TokenType tokenA = skip_ignorable(a, mixed); + TokenType tokenB = skip_ignorable(b, mixed); + + if (tokenA != tokenB) { + return false; + } + + if (tokenA.isText()) { + // collect_text moves each cursor past the text it returns + if (!collect_text(a, mixed).equals(collect_text(b, mixed))) { + return false; + } + } else if (tokenA.isStart()) { + if (!a.getName().equals(b.getName()) || + !values_equal(a.getObject(), b.getObject())) { + return false; + } + a.toEndToken(); + a.toNextToken(); + b.toEndToken(); + b.toNextToken(); + } else { + // both sides ran out of content at the same place + return true; + } + } + } + + /** + * Moves the cursor past what carries no value - comments, processing + * instructions and, outside of mixed content, whitespace - and returns the + * type of the token it comes to rest on. + */ + private static TokenType skip_ignorable(XmlCursor c, boolean mixed) { + for (; ; ) { + TokenType token = c.currentTokenType(); + + if (token.isComment() || token.isProcinst() || + (!mixed && token.isText() && is_whitespace(c.getChars()))) { + c.toNextToken(); + } else { + return token; + } + } + } + + /** + * Returns the run of text starting at the cursor, joining the chunks that a + * comment or processing instruction splits it into, and leaves the cursor on + * the token that ends the run. + */ + private static String collect_text(XmlCursor c, boolean mixed) { + StringBuilder text = new StringBuilder(); + + for (; ; ) { + TokenType token = c.currentTokenType(); + + if (token.isText()) { + String chars = c.getChars(); + if (mixed || !is_whitespace(chars)) { + text.append(chars); + } + c.toNextToken(); + } else if (token.isComment() || token.isProcinst()) { + c.toNextToken(); + } else { + return text.toString(); + } + } + } + + private static boolean is_whitespace(String text) { + for (int i = 0; i < text.length(); i++) { + if (!XMLChar.isSpace(text.charAt(i))) { + return false; + } + } + return true; + } + + /** + * Compares two values found inside the values being compared. valueEquals + * would lock both trees again - and for trees in different synchronization + * domains that means taking the global lock - once per child, for nothing. + */ + private static boolean values_equal(XmlObject a, XmlObject b) { + if (a == null || b == null) { + return a == b; + } + + try { + if (a instanceof XmlObjectBase && b instanceof XmlObjectBase) { + return ((XmlObjectBase) a).value_equals_locked(b); + } + return a.valueEquals(b); + } catch (XmlValueOutOfRangeException e) { + // A value that does not fit its type has no value space to be + // compared in - compare what the document says instead, rather than + // letting one bad value in a tree turn a comparison into a throw. + return text_equal(a, b); + } + } + + private static boolean text_equal(XmlObject a, XmlObject b) { + if (!has_store(a) || !has_store(b)) { + return false; + } + + try (XmlCursor cursorA = a.newCursor(); + XmlCursor cursorB = b.newCursor()) { + return cursorA.getTextValue().equals(cursorB.getTextValue()); + } + } +} diff --git a/src/test/java/xmlobject/detailed/ValueEqualsTest.java b/src/test/java/xmlobject/detailed/ValueEqualsTest.java index 5ebf9654a..f8ef962a8 100755 --- a/src/test/java/xmlobject/detailed/ValueEqualsTest.java +++ b/src/test/java/xmlobject/detailed/ValueEqualsTest.java @@ -17,7 +17,6 @@ package xmlobject.detailed; import org.apache.xmlbeans.XmlObject; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.tranxml.tranXML.version40.CarLocationMessageDocument; import xmlcursor.common.Common; @@ -27,6 +26,20 @@ import static xmlcursor.common.BasicCursorTestCase.jobj; public class ValueEqualsTest { + private static final String CLM = + "" + + "FLEETNAME" + + "CSXT" + + "" + + "" + + "GATX" + + "123456" + + "7" + + "" + + "DALLAS" + + "" + + ""; + @Test void testValueEqualsTrue() throws Exception { CarLocationMessageDocument clmDoc = (CarLocationMessageDocument) jobj(Common.TRANXML_FILE_CLM); @@ -34,7 +47,6 @@ void testValueEqualsTrue() throws Exception { assertTrue(clmDoc.valueEquals(m_xo)); } - @Disabled // https://issues.apache.org/jira/browse/XMLBEANS-658 @Test void testIssue658() throws Exception { CarLocationMessageDocument clm1 = CarLocationMessageDocument.Factory.newInstance(); @@ -51,5 +63,101 @@ void testIssue658() throws Exception { assertFalse(clm2.valueEquals(clm1)); } -} + @Test + void testMissingChildElement() throws Exception { + CarLocationMessageDocument clm1 = CarLocationMessageDocument.Factory.newInstance(); + clm1.addNewCarLocationMessage().setFleetID("fleet1"); + + CarLocationMessageDocument clm2 = CarLocationMessageDocument.Factory.newInstance(); + clm2.addNewCarLocationMessage(); + + assertFalse(clm1.valueEquals(clm2)); + assertFalse(clm2.valueEquals(clm1)); + + clm2.getCarLocationMessage().setFleetID("fleet1"); + assertTrue(clm1.valueEquals(clm2)); + } + + @Test + void testNestedElementDifference() throws Exception { + CarLocationMessageDocument clm1 = CarLocationMessageDocument.Factory.parse(CLM); + CarLocationMessageDocument clm2 = CarLocationMessageDocument.Factory.parse(CLM); + assertTrue(clm1.valueEquals(clm2)); + + CarLocationMessageDocument clm3 = + CarLocationMessageDocument.Factory.parse(CLM.replace("DALLAS", "AUSTIN")); + assertFalse(clm1.valueEquals(clm3)); + assertFalse(clm3.valueEquals(clm1)); + + // the elements above the changed one differ as well + assertFalse(clm1.getCarLocationMessage().valueEquals(clm3.getCarLocationMessage())); + assertFalse(clm1.getCarLocationMessage().getEventStatusArray(0) + .valueEquals(clm3.getCarLocationMessage().getEventStatusArray(0))); + } + + @Test + void testAttributesCompared() throws Exception { + CarLocationMessageDocument clm1 = CarLocationMessageDocument.Factory.parse(CLM); + // an attribute of the element itself + CarLocationMessageDocument clm2 = + CarLocationMessageDocument.Factory.parse(CLM.replace("Transaction=\"CLM\"", "Transaction=\"CLM\" Version=\"4.0\"")); + assertFalse(clm1.valueEquals(clm2)); + assertFalse(clm2.valueEquals(clm1)); + + // an attribute of a nested element with simple content + CarLocationMessageDocument clm3 = + CarLocationMessageDocument.Factory.parse(CLM.replace("", "")); + assertFalse(clm1.valueEquals(clm3)); + assertFalse(clm3.valueEquals(clm1)); + } + + @Test + void testChildrenComparedByValue() throws Exception { + // the same integer, written two ways: one value, two lexical forms + CarLocationMessageDocument clm1 = CarLocationMessageDocument.Factory.parse(CLM); + CarLocationMessageDocument clm2 = CarLocationMessageDocument.Factory.parse( + CLM.replace("7<", "007<")); + assertTrue(clm1.valueEquals(clm2)); + + CarLocationMessageDocument clm3 = CarLocationMessageDocument.Factory.parse( + CLM.replace("7<", "8<")); + assertFalse(clm1.valueEquals(clm3)); + } + + @Test + void testValueThatDoesNotFitItsType() throws Exception { + // 12:34 is not a valid xs:time - a value that cannot be computed is + // compared as it was written rather than throwing + String withTime = CLM.replace("", ""); + + CarLocationMessageDocument clm1 = CarLocationMessageDocument.Factory.parse(withTime); + CarLocationMessageDocument clm2 = CarLocationMessageDocument.Factory.parse(withTime); + assertTrue(clm1.valueEquals(clm2)); + + CarLocationMessageDocument clm3 = + CarLocationMessageDocument.Factory.parse(withTime.replace("12:34", "12:35")); + assertFalse(clm1.valueEquals(clm3)); + } + + @Test + void testWhitespaceAndCommentsIgnored() throws Exception { + CarLocationMessageDocument clm1 = CarLocationMessageDocument.Factory.parse(CLM); + CarLocationMessageDocument clm2 = CarLocationMessageDocument.Factory.parse( + CLM.replace("><", ">\n <").replace("", "")); + assertTrue(clm1.valueEquals(clm2)); + } + + @Test + void testUntypedTextIsCompared() throws Exception { + // untyped content is mixed, so the text around the children counts + XmlObject xo1 = XmlObject.Factory.parse("onetwo"); + XmlObject xo2 = XmlObject.Factory.parse("onetwo"); + XmlObject xo3 = XmlObject.Factory.parse("onethree"); + XmlObject xo4 = XmlObject.Factory.parse(""); + + assertTrue(xo1.valueEquals(xo2)); + assertFalse(xo1.valueEquals(xo3)); + assertFalse(xo1.valueEquals(xo4)); + } +}