Details
Bug
Closed
Major
Description
See the code attached. For a class with multiple implicit collections (an array and an ArrayList in this case), I get intermittent errors loading instances from XML, where the array field is null when it shouldn't be. Run BugTest.java with XStream libs and the input XML file on the classpath. Every few runs you get a NullPointerException trying to access the array (but it works perfectly on other runs).
I tried different variants (such as adding a 'filler' field between them), and it still occurs. I'm 99% sure that it also occurred with two ArrayLists (instead of an array and an ArrayList), so it's not specific to the array/collection combination.
It seems that making the array non-implicit fixes it (leaving the other implicit), but I need to more thoroughly test it. This seems to be independent of JRE version/vendor and OS. (Well, it occurred at least on Sun JRE 6/7 on Windows and OpenJDK 6 on Linux.)
The error occurs maybe 3 in 20 times (but is variable), and it's something caused by the 'global' initialisation code: if I do the fromXML in a loop within the code (re-instantiating the XStream object each time) I either get them all failing or none. (So I assume XStream does some initialisation in other classes that isn't redone when new XStream instances are produced.)
Activity
Actually you were right. I've modified my new ImplicitTest that reveals the problem. The assertObjectEquels method succeeded, because teh XML representation of the wrong deserialized object was equal to the original. At deserialization time the individual elements get collected in the wrong collection, but their overall sequence is still the same.
Jörg Schaible
added a comment - Actually you were right. I've modified my new ImplicitTest that reveals the problem. The assertObjectEquels method succeeded, because teh XML representation of the wrong deserialized object was equal to the original. At deserialization time the individual elements get collected in the wrong collection, but their overall sequence is still the same. Thanks for looking further. I can see how the filler fields you added will now trap the error (even with the fall-back 'XML serialisations are the same' object comparison), and why it passed previously. Your description of the problem would also explain why leaving only one array/collection implicit would avoid the problem.
Do you currently know the root cause (and hence why it's intermittent)? Is this likely to be a high priority fix?
P.S. Does this type of problem (XML serialisation the same even when objects not the same) suggest that, in general, equals methods in test classes should ideally always be coded? Obviously there's effort in coding them though...
Stuart Rossiter
added a comment - Thanks for looking further. I can see how the filler fields you added will now trap the error (even with the fall-back 'XML serialisations are the same' object comparison), and why it passed previously. Your description of the problem would also explain why leaving only one array/collection implicit would avoid the problem.
Do you currently know the root cause (and hence why it's intermittent)? Is this likely to be a high priority fix?
P.S. Does this type of problem (XML serialisation the same even when objects not the same) suggest that, in general, equals methods in test classes should ideally always be coded? Obviously there's effort in coding them though... Sorry for the delay, but some real life problems ate my spare time.
After having a closer look again, the root problem in the test is that the definition of the implicit collection for the list is actually a catch-all, because you did not define the type of objects that are collected in the list. The generic type is not available at runtime and the list can take therefore any object. Now, if the ReflectionConverter detect a element that does not match a member, if looks for an implicit collection/array/map that will take the element. However, there is no defined sequence for such implicit definitions and when it asks for an A it gets by change the bList ... because that one takes any Object. You can solve this by using the 3rd parameter and declare that the list will only take elements of type B.
P.S. Does this type of problem (XML serialisation the same even when objects not the same) suggest that, in general, equals methods in test classes should ideally always be coded? Obviously there's effort in coding them though...
It simply meant that the XML representation of an object is not necessarily unique.
Jörg Schaible
added a comment - Sorry for the delay, but some real life problems ate my spare time.
After having a closer look again, the root problem in the test is that the definition of the implicit collection for the list is actually a catch-all, because you did not define the type of objects that are collected in the list. The generic type is not available at runtime and the list can take therefore any object. Now, if the ReflectionConverter detect a element that does not match a member, if looks for an implicit collection/array/map that will take the element. However, there is no defined sequence for such implicit definitions and when it asks for an A it gets by change the bList ... because that one takes any Object. You can solve this by using the 3rd parameter and declare that the list will only take elements of type B.
P.S. Does this type of problem (XML serialisation the same even when objects not the same) suggest that, in general, equals methods in test classes should ideally always be coded? Obviously there's effort in coding them though...
It simply meant that the XML representation of an object is not necessarily unique. Ah great, many thanks for having a further look. I confirm that specifying the implicit array type via the optional 3rd parameter works.
Is it worth making a change so that multiple implicit collections are not allowed to be specified (for the same class) without providing the explicit type? Otherwise I guess there is no way round it in the code for the reasons you mention.
If there are reasons not to do such a change (e.g., to avoid breaking existing code, but might then want to defer it for a next major release), sounds like an FAQ entry for this would be very useful.
Stuart Rossiter
added a comment - Ah great, many thanks for having a further look. I confirm that specifying the implicit array type via the optional 3rd parameter works.
Is it worth making a change so that multiple implicit collections are not allowed to be specified (for the same class) without providing the explicit type? Otherwise I guess there is no way round it in the code for the reasons you mention.
If there are reasons not to do such a change (e.g., to avoid breaking existing code, but might then want to defer it for a next major release), sounds like an FAQ entry for this would be very useful. I changed the algorithm for the element assignment a bit. If multiple implicit collections exist and an element may belong into more than one, it is assigned to the collection with the nearest type definition, e.g. if you have an implicit List for Object, one for CharSequence and one for String, then a String will always be assigned to the 3rd list, a StringBuffer to the 2nd while a Boolean goes into the first. Improved in HEAD.
Jörg Schaible
added a comment - I changed the algorithm for the element assignment a bit. If multiple implicit collections exist and an element may belong into more than one, it is assigned to the collection with the nearest type definition, e.g. if you have an implicit List for Object, one for CharSequence and one for String, then a String will always be assigned to the 3rd list, a StringBuffer to the 2nd while a Boolean goes into the first. Improved in HEAD. 
Acceptance tests with all implicit types at once and in a loop.