pjfanning opened a new pull request, #106: URL: https://github.com/apache/poi-xmlbeans/pull/106
Fixes [XMLBEANS-658](https://issues.apache.org/jira/browse/XMLBEANS-658): `valueEquals` always returns true for complex values. `XmlComplexContentImpl.equal_to` compared the two schema types and stopped there, with a `BUGBUG: by-value structure comparison undone` where the comparison should have been, so any two complex values of the same type were reported equal whatever they contained. The `XmlObject.valueEquals` javadoc said as much: *"as of XMLBeans 2.2.1 only implemented for simple type values"*. ### What it does now `equal_to` walks both trees: * **attributes** - compared by name, regardless of order; namespace declarations are not attributes and take no part * **child elements** - in document order, name first, then each child compared *by value*, so a child in a different lexical form but the same value space still matches (`<NumberCheckDigit>7</>` equals `<NumberCheckDigit>007</>`) * **text** - compared for mixed content; 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, so the attributes the type adds to the value were ignored - and the tree walk would have inherited that hole for every such child. `valueEquals` now compares those attributes once the text has matched. This is a little beyond the letter of the JIRA; say the word and I will split it out. Two implementation notes: * 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 - otherwise two trees in different synchronization domains would take 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 - `CarLocationMessage.xml` in this repo has `12:34` in an `xs:time`, which is not a valid time. Such a child falls back to comparing the text as written, so one bad value cannot turn a comparison into a throw. ### Compatibility This changes what `valueEquals` answers for complex values, from "same schema type" to "same value". Worth being explicit about who could notice: * assertions that have been passing vacuously (`assertTrue(a.valueEquals(b))` over complex types has been unconditional) will start to mean something, and some will fail * anyone using it as a cheap "same type?" check gets a content-sensitive answer instead, with no exception to warn them * new false results where a user might expect equality: children of an `xs:all` in a different order, a value written out explicitly vs left to a schema default, `xsi:schemaLocation`/`xsi:type` present on one side only, and - because untyped content is mixed - indentation differences between untyped (`anyType`, `xs:any`) fragments * cost goes from a constant-time type check to a walk of the tree, with a cursor pair and a typed object per child Nothing inside xmlbeans depends on the old answer: the fixed-value check in `Validator`, `StscSimpleTypeResolver`, `XmlUnionImpl` and `XmlObjectList` all compare simple values, and the Validator's operands are unattached, so the new attribute check is a no-op there. `valueHashCode` still throws for complex types, so the `equals`/`hashCode` contract is untouched. Given the semantics change, this seems better suited to 5.5.0 than to a patch release, even though the javadoc never promised the old answer. ### Tests `ValueEqualsTest` grows from 2 tests to 9: the `@Disabled` XMLBEANS-658 case is enabled, plus a missing child, a difference nested two levels down, attributes on the element and on a simple-content child, children compared by value rather than lexically, whitespace and comments ignored, text counted for untyped content, and a value that does not fit its type. Full suite: 3167 tests, 0 failures. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
