[ 
http://issues.apache.org/jira/browse/XMLBEANS-228?page=comments#action_12361189 
] 

Andreas Loew commented on XMLBEANS-228:
---------------------------------------

Lawrence,

many thanks for looking into this truly rather complex issue.

first of all, just shortly regarding the QName set patch, I am also comfortable 
with your way of doing it.

Secondly, unfortunately, your short-term workaround is not applicable to our 
scenario, because we are using a generic (and schema-agnostic) bean converter 
(based on the reflection API) which copies plain old JavaBeans into XMLBeans 
based on reflection and matching getter/setter method names, so we won't be 
able to determine the order in which we will call the XMLBeans setters from the 
schema.

Sadly, I won't have enough time before the Christmas holidays to have a truly 
detailed look into what you are saying. Let me just quickly reply that, after a 
first glance, I think your current view on the issue still was not fully 
correct, but your proposed solution is:

While I got your point and fully agree that, based on the substGroupTests.xsd 
schema that you have attached, my suggested patch will still fail to produce a 
valid document, I think you were wrong when concluding the following about my 
patch:

"Your suggested solution (as far as I can see) is to not worry about this [i.e. 
inserting elements into the store at their proper place when calling a setter], 
but when it comes time to marshal the object, collect all the instances of 
"things in the same substitution group" together and output them as if they 
were all in the one place (the place where the substitution group head would be 
in the schema)."

My patch also has already tried fo fix insertion into the store: It was adding 
the additional checks to XmlObjectBase#get_element_ending_delimiters(QName 
eltname) which will exactly be called in order to calculate the position within 
the store where the element will be inserted.

>From what I think to have understood now, the problem rather seems to be that, 
>even with my patch included, for your sample schema, this method would still 
>calculate the wrong place for insertion of Subst1Element and Subst2Element, 
>because it would try to insert these elements at the same position where 
>SubstHead would have been inserted/stored before.

So I fully agree again with your conclusion about the correct solution that is 
needed: It is simply not sufficient to only store the element name for which an 
element is substituting in the JavaElementSetterModel, but this store will have 
to contain the original element name, as you have pointed out correctly.
Trying to fix this issue in XmlObjectBase simply was too late to distinguish 
whether we have happened to run into a SubstHead, a Subst1Element or a 
Subst2Element - as the only information available from the current 
JavaElementSetterModel is that it is "a SubstHead or any of its legal 
substitutions".

Just one more suggestion: Maybe rather than storing both QNames - the element's 
own name and the name of the associated head of its substitution group -  in 
the store, it would also be possible to only store the leaf element's QName (in 
order not to change the type structure of the store), but to calculate the 
element QName for which it is substituting (i.e. the head its substitution 
group) on demand from the leaf element in case that the SG head QName is needed 
for marshalling and/or querying.

So what do you think about this?

I'll be back at work on January 2nd, and be able to invest some time to see 
whether my new suggestion seems feasible.

Happy holidays & thanks again,

Andreas


> Element order is mixed up on document creation after calling substitute()
> -------------------------------------------------------------------------
>
>          Key: XMLBEANS-228
>          URL: http://issues.apache.org/jira/browse/XMLBEANS-228
>      Project: XMLBeans
>         Type: Bug
>   Components: XmlObject
>     Versions: Version 2, Version 2.1
>  Environment: N/A
>     Reporter: Andreas Loew
>      Fix For: Version 2, Version 2.1
>  Attachments: SubstitutionGroupTest.java, substGroupTests.xsd, 
> substitutionGroupFix.patch, substitutionGroupInSequence.xsd, 
> xmlbeans228_QNameSet.patch
>
> When trying to create XML documents with XMLBeans 2.x using xmlText() or 
> save() that use substitution groups, usage of the substitute() method will 
> mix up the order of the XML document (the order will remain correct when 
> substitute() will not be called).
> We have attached a test case (schema XSD, JUnit test) that demonstrates the 
> failure and also attached a patch against plain XMLBeans 2.0.0 that shows 
> where the problem is and what is needed to fix problem (although currently at 
> the cost of a quite severe perfomance penalty, as I am not an expert in 
> XMLBeans internals):
> For the test case, XMLBeans 2.0.0 as well as the current SVN snapshot both 
> fail as they return
> <?xml version="1.0" encoding="UTF-8"?>
> <Person 
> xmlns="urn:www-apache-org:SubstitutionGroup/substitutionGroupInSequence">
>   <FirstCommentElement>ThirdElement</FirstCommentElement>
>   <FirstName>FirstElement</FirstName>
>   <LastName>SecondElement</LastName>
> </Person>
> instead of
> <?xml version="1.0" encoding="UTF-8"?>
> <Person 
> xmlns="urn:www-apache-org:SubstitutionGroup/substitutionGroupInSequence">
>   <FirstName>FirstElement</FirstName>
>   <LastName>SecondElement</LastName>
>   <FirstCommentElement>ThirdElement</FirstCommentElement>
> </Person>
> as would be correct (and will be returned after applying the patches 
> provided).
> Basically, the main problem is that  
> XmlObjectBase#getElementEndingDelimiters() after having called 
> SchemaProperty#getJavaSetterDelimiter(), does not take into account that the 
> QNameSet returned by getJavaSetterDelimiter() does not yet include the "alias 
> names" - i.e. possible substitutions - of those elements that happen to be 
> heads of substitution groups.
> The patch for XmlObjectBase adds the missing substitutions to this QNameSet. 
> This will fix the base problem, but so far at the cost of performance: In 
> order to simulate a (non-existing) QNameSet iterator on the set of 
> JavaSetterDelimiters, we have not been able to find a more intelligent 
> solution than to iterate over the complete array of SchemaGlobalElements and 
> check each head of a substitution group for inclusion in the QNameSet (using 
> QNameSet#contains()). This definitely is suboptimal and should be replaced by 
> an optimized implementation done by somebody who is familiar with the 
> internals of XMLBeans. (Sorry!)
> Unfortunately, while creating the first fix, we ran into a second (small) 
> issue:
> The static method QNameSet#forArray() does mistakenly pass null instead of 
> new HashSet() to the private constructor, which results in a 
> NullPointerException. The second fix (also included in the patch) will fix 
> this problem.
> Please get back to me in case you need any additional details.
> Thanks & best regards,
> Andreas Loew & Patrik Streicher

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to