On Sun, Oct 23, 2016 at 05:30:55PM +0200, Pietro Saccardi wrote:
> Hello Dave,
>
> my name is Pietro. I recently started using generateDS--great job! it's
> really useful and works seamlessly out of the box.
>
> While testing, I encountered a bug. I wanted to file it to sourceforce, but
> I am unable to create tickets. I am also not very used to the platform so I
> may have missed something.
>
> Anyway, I'll describe the bug.
Pietro,
I've done some work on this. And, possibly, I have a fix. What the
fix does is to check each child element to determine if it has a
sibling with the same name. If there is a sibling with the same
name, this fix deletes that element and marks this element as
self.maxOccurs=2 (effectively maxOccurs="unbounded").
OK. So, now that we have what might be a fix, we need to ask: Should
we really make this fix? Is the snippet of the XML schema that you
sent (see below) legal?
One thing that worries me is that if you can write this (which is
your example):
<xsd:element name="authors">
<xsd:complexType>
<xsd:sequence>
<xsd:element ref="author"/>
<xsd:choice maxOccurs="unbounded" minOccurs="0">
<xsd:element ref="author"/>
<xsd:element name="cooperation" type="xsd:string"/>
</xsd:choice>
</xsd:sequence>
</xsd:complexType>
</xsd:element>
then why couldn't you write this:
<xsd:element name="authors">
<xsd:complexType>
<xsd:sequence>
<xsd:element name="author" type="xsd:string"/>
<xsd:choice maxOccurs="unbounded" minOccurs="0">
<xsd:element name="author" type="xsd:integer"/>
<xsd:element name="cooperation" type="xsd:string"/>
</xsd:choice>
</xsd:sequence>
</xsd:complexType>
</xsd:element>
where the "author" child elements have two different types but the
same name. I don't think it makes much sense for generateDS.py to
handle that. It would require generating a class with to member
data items with the same name and different types.
What I'm thinking of doing is adding the fix that I mention above,
but, when it's triggered, write out a warning message. One reason
that this makes sense to me is that in the generateDS unit tests,
this fix uncovered a bug in one of the schemas: I had two child
elements with the same name and type, one of which I likely had
intended to rename or delete, but had failed to do so. This fix
with a warning message would have uncovered that "error" in the
schema (It was in tests/validate_simpletypes.xsd). Actually, there
were a couple of other schemas in the unit tests that were
questionable and that this fix revealed.
By doing this (the above fix plus a warning), we'd have these
benefits:
- It would generate correct code from your schema (or at least it
would be better code).
- It would alert a user who has a schema that has "erroneous"
duplicate child elements with the same name.
- It would warn us in cases where this fix does something we do not
want or expect it to do, for example, with some XML schema that
has a corner case that we had not anticipated.
What do you think?
A patch is attached.
Sorry for being so wordy. It helps me think these things through.
Dave
>
> Consider such a schema:
>
> <xsd:element name="authors">
> <xsd:complexType>
> <xsd:sequence>
> <xsd:element ref="author"/>
> <xsd:choice maxOccurs="unbounded" minOccurs="0">
> <xsd:element ref="author"/>
> <xsd:element name="cooperation" type="xsd:string"/>
> </xsd:choice>
> </xsd:sequence>
> </xsd:complexType>
> </xsd:element>
>
> This will require the <authors> element to have a first child of type
> <author>, followed by zero or more instances of the elements <author> or
> <cooperation>.
>
> Such schema is not handled correctly in generateDS; it will generate
> ambiguity between the unique, initial author and the list of author
> objects. This is an excerpt of the buildChildren method of the authors
> class:
>
> def buildChildren(self, child_, node, nodeName_, fromsubclass_=False):
> if nodeName_ == 'author':
> obj_ = person.factory()
> obj_.build(child_)
> self.author = obj_
> obj_.original_tagname_ = 'author'
> elif nodeName_ == 'author':
> obj_ = person.factory()
> obj_.build(child_)
> self.author.append(obj_)
> obj_.original_tagname_ = 'author'
> elif nodeName_ == 'cooperation':
> cooperation_ = child_.text
> cooperation_ = self.gds_validate_string(cooperation_, node,
> 'cooperation')
> self.cooperation.append(cooperation_)
>
> As you can see, the second elif will never be reached and any further
> <author> will override the author property, losing the remaining elements
> in the list. Also, the exportChildren method will crash, because it will
> try to enumerate the author property of authors.
>
> I changed the structure of my XSD to work around this problem, but I think
> this should either be remarked in the documentation or fixed.
>
> I hope this can be of any use; thank you for your great work on generateDS!
>
> Best regards,
>
> Pietro Saccardi
--
Dave Kuhlman
http://www.davekuhlman.org
diff -r b7ccc0e3579a generateDS.py
--- a/generateDS.py Thu Oct 13 15:31:43 2016 -0700
+++ b/generateDS.py Wed Nov 02 11:42:59 2016 -0700
@@ -153,7 +153,6 @@
import os.path
import time
import getopt
-from pprint import pprint
if sys.version_info.major == 2:
import urllib2
@@ -918,7 +917,12 @@
self.expandGroupReferences_tree(visited)
self.collect_element_dict()
self.annotate_find_type()
- self.annotate_tree()
+ element_dict = {}
+ to_be_removed = []
+ self.annotate_tree(
+ element_dict=element_dict,
+ to_be_removed=to_be_removed)
+ self.remove_children(to_be_removed)
self.fix_dup_names()
self.coerce_attr_types()
self.checkMixedBases()
@@ -1153,7 +1157,18 @@
for child in self.children:
child.annotate_find_type()
- def annotate_tree(self):
+ def annotate_tree(self, element_dict, to_be_removed):
+ name = self.getName()
+ if name in element_dict:
+ # If we've already seen this element (name), make it a
+ # list and throw the previous one away.
+ self.maxOccurs = 2
+ to_be_removed.append(element_dict[name])
+ err_msg(
+ '\n*** warning. Removing child with duplicate '
+ 'name: "{}"\n\n'.format(name))
+ else:
+ element_dict[name] = self
# If there is a namespace, replace it with an underscore.
if self.base:
self.base = strip_namespace(self.base)
@@ -1193,7 +1208,6 @@
sys.exit(1)
self.minOccurs = minOccurs
self.maxOccurs = maxOccurs
-
# If it does not have a type, then make the type the same as the name.
if self.type == 'NoneType' and self.name:
self.type = self.name
@@ -1212,8 +1226,17 @@
parent.collapseWhiteSpace):
self.collapseWhiteSpace = 1
# Do it recursively for all descendents.
+ element_dict = {}
+ to_be_removed = []
for child in self.children:
- child.annotate_tree()
+ child.annotate_tree(
+ element_dict=element_dict,
+ to_be_removed=to_be_removed)
+ self.remove_children(to_be_removed)
+
+ def remove_children(self, to_be_removed):
+ for element in to_be_removed:
+ self.children.remove(element)
#
# For each name in the attributeGroupNameList for this element,
------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
_______________________________________________
generateds-users mailing list
generateds-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/generateds-users
