Juan Hernandez has uploaded a new change for review. Change subject: sdk: Fix find root tag ......................................................................
sdk: Fix find root tag Currently when the SDK needs to generate the XML document for an entity it selects the root tag doing a reverse lookup inside the _rootClassMap dictionary. This isn't correct, because that dictionary contains duplicates. For example, for the "User" class there are more than one corresponding tags: "user": User, "owner": User, ... The root cause of this problem is that the _rootClassMap dictionary contains entries for all the element definitions contained in the XML schema, not only for the top level element definitions. The non top level element definitions shouldn't be used to determine root tags, as they can't be used as the root tag of valid XML documents. This _rootClassMap dictionary was introduced to overcome a limitation of generateDS.py, but that limitation was fixed in version 2.12a. It can't be removed now because users may be calling the "findRootClass" function that uses this dictionary. To address these issues this patch modifies the code generator so that it won't use the _rootClassMap dictionary, but the dictionary generated by generateDS.py. In addition the patch also introduces a new _tag_for_type dictionary to be used instead of the reverse lookup in _rootClassMap. This new dictionary only contains the types corresponding to top level elements. The _rootClassMap dictionary and the related functions are preserved for backwards compatibility. Change-Id: I67b22c07ff4b91a7bf9b2ff8883ee51bef8c8f58 Related-To: https://bugzilla.redhat.com/1122589 Signed-off-by: Juan Hernandez <juan.hernan...@redhat.com> (cherry picked from commit 26e1191239229f4b49337571a95995587a68207e) --- M generator/src/main/java/org/ovirt/engine/sdk/generator/rsdl/RsdlCodegen.java M generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/XsdCodegen.java M generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/XsdData.java M generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/templates/FindRootClassTemplate D generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/templates/GetRootTagTemplate D generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/templates/GetRootTagTemplate.java D generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/templates/ParseStringTemplate D generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/templates/ParseStringTemplate.java M src/ovirtsdk/infrastructure/errors.py M src/ovirtsdk/infrastructure/proxy.py M src/ovirtsdk/utils/parsehelper.py 11 files changed, 99 insertions(+), 129 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine-sdk refs/changes/10/32410/1 diff --git a/generator/src/main/java/org/ovirt/engine/sdk/generator/rsdl/RsdlCodegen.java b/generator/src/main/java/org/ovirt/engine/sdk/generator/rsdl/RsdlCodegen.java index a23d951..20a0b38 100644 --- a/generator/src/main/java/org/ovirt/engine/sdk/generator/rsdl/RsdlCodegen.java +++ b/generator/src/main/java/org/ovirt/engine/sdk/generator/rsdl/RsdlCodegen.java @@ -91,7 +91,7 @@ super(""); this.rsdlPath = rsdlPath; - Map<String, String> map = XsdData.getInstance().getMap(); + Map<String, String> map = XsdData.getInstance().getTypesByTag(); List<String> names = new ArrayList<>(map.keySet()); Collections.sort(names); for (String name : names) { diff --git a/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/XsdCodegen.java b/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/XsdCodegen.java index b3af117..59bab02 100644 --- a/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/XsdCodegen.java +++ b/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/XsdCodegen.java @@ -31,9 +31,7 @@ import org.ovirt.engine.sdk.generator.common.AbstractCodegen; import org.ovirt.engine.sdk.generator.templates.AbstractTemplate; import org.ovirt.engine.sdk.generator.xsd.templates.FindRootClassTemplate; -import org.ovirt.engine.sdk.generator.xsd.templates.GetRootTagTemplate; import org.ovirt.engine.sdk.generator.xsd.templates.ImportsTemplate; -import org.ovirt.engine.sdk.generator.xsd.templates.ParseStringTemplate; import org.ovirt.engine.sdk.generator.xsd.templates.SuperAttributesTemplate; public class XsdCodegen extends AbstractCodegen { @@ -94,13 +92,15 @@ addImports(); fixExternalEncoding(); addSuperAttributes(); - replaceFunctions(); // Generate the class map: source.add(""); source.add(BEGIN_NOT_GENERATED); source.add(""); generateClassMap(); + source.add(""); + source.add(""); + generateTagsMap(); source.add(""); source.add(""); appendFunctions(); @@ -166,33 +166,9 @@ addLines(lastIndex + 1, firstIndent + 4, lines); } - private void replaceFunctions() throws IOException { - replaceFunction("parseString", new ParseStringTemplate()); - replaceFunction("get_root_tag", new GetRootTagTemplate()); - } - - private void replaceFunction(String name, AbstractTemplate template) throws IOException { - // Find the first and last line of the function: - int firstIndex = findFunctionFirstLine(name); - if (firstIndex == -1) { - throw new IOException("Can't find function \"" + name + "\"."); - } - int lastIndex = findBlockLastLine(firstIndex); - - // Delete the lines: - for (int index = firstIndex; index <= lastIndex; index++) { - source.remove(firstIndex); - } - - // Insert the replacement function: - String text = template.evaluate(); - String[] lines = text.split("\n"); - addLines(firstIndex, 0, lines); - } - private void generateClassMap() throws IOException { // Sort the names of the elements: - Map<String, String> map = XsdData.getInstance().getMap(); + Map<String, String> map = XsdData.getInstance().getTypesByTag(); List<String> names = new ArrayList<>(map.keySet()); Collections.sort(names); @@ -206,6 +182,22 @@ addLines(source.size(), 16, "}"); } + private void generateTagsMap() throws IOException { + // Sort the type names: + Map<String, String> map = XsdData.getInstance().getTagsByType(); + List<String> types = new ArrayList<>(map.keySet()); + Collections.sort(types); + + // Generate a list of pairs, each containing the type and the corresponding XML tag: + addLines(source.size(), 0, "_tag_for_type = {"); + for (String type : types) { + String tag = map.get(type); + String line = String.format("%s: \"%s\",", type, tag); + addLines(source.size(), 4, line); + } + addLines(source.size(), 0, "}"); + } + private void appendFunctions() throws IOException { appendFunction(new FindRootClassTemplate()); } diff --git a/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/XsdData.java b/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/XsdData.java index a5ad9d9..ffc0062 100644 --- a/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/XsdData.java +++ b/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/XsdData.java @@ -51,7 +51,17 @@ return instance; } - private Map<String, String> map = new LinkedHashMap<>(); + /** + * This maps stores the relationship between XML tag names and Python type names. + */ + private Map<String, String> typesByTag = new LinkedHashMap<>(); + + /** + * This map stores the relationship between Python type names and XML tags. Note that this isn't the inverse of the + * previous one, as some types don't have a tag because they don't appear as top level element declarations in the + * XML schema, thus they can't appear as root elements in a valid XML document. + */ + private Map<String, String> tagsByType = new LinkedHashMap<>(); private XsdData() { } @@ -119,7 +129,8 @@ // Exclude the VM summary because it conflicts with the API summary: excluded.add("VmSummary"); - // For each element declaration add an entry to the map: + // Populate the types by tag map, including all the element definitions that appear in the XML schema, even + // those that aren't top level and thus not valid as roots of valid XML documents: try { NodeList elements = (NodeList) xpath.evaluate("//xs:element", schema, XPathConstants.NODESET); for (int i = 0; i < elements.getLength(); i++) { @@ -131,7 +142,7 @@ String name = nameAttr.getValue(); String type = typeAttr.getValue(); if (!type.startsWith("xs:") && !excluded.contains(type)) { - map.put(name, type); + typesByTag.put(name, type); } } } @@ -141,16 +152,43 @@ } // There are several conflicts with "version", so force it: - map.put("version", "VersionCaps"); + typesByTag.put("version", "VersionCaps"); + + // Populate the tags by type name, including only the top level element definitions that appear in the XML + // schema, those that can appear as roots of valid XML documents: + try { + NodeList elements = (NodeList) xpath.evaluate("/xs:schema/xs:element", schema, XPathConstants.NODESET); + for (int i = 0; i < elements.getLength(); i++) { + Node element = elements.item(i); + NamedNodeMap attrs = element.getAttributes(); + Attr nameAttr = (Attr) attrs.getNamedItem("name"); + Attr typeAttr = (Attr) attrs.getNamedItem("type"); + if (nameAttr != null && typeAttr != null) { + String name = nameAttr.getValue(); + String type = typeAttr.getValue(); + if (!type.startsWith("xs:") && !excluded.contains(type)) { + tagsByType.put(type, name); + } + } + } + } + catch (XPathExpressionException exception) { + throw new IOException("Can't find elements.", exception); + } + } - public Map<String, String> getMap() { - return map; + public Map<String, String> getTypesByTag() { + return typesByTag; + } + + public Map<String, String> getTagsByType() { + return tagsByType; } public String getXmlWrapperType(String typeName) { String tn = typeName.toLowerCase(); - for (Map.Entry<String, String> entry : map.entrySet()) { + for (Map.Entry<String, String> entry : typesByTag.entrySet()) { String k = entry.getKey(); String v = entry.getValue(); if (v.toLowerCase().equals(tn) || k.toLowerCase().equals(tn) || k.replace("_", "").equals(tn)) { @@ -164,7 +202,7 @@ String tn = typeName.toLowerCase(); String result = XML_TYPE_INSTANCE_EXCEPTIONS.get(tn); if (result == null) { - for (Map.Entry<String, String> entry : map.entrySet()) { + for (Map.Entry<String, String> entry : typesByTag.entrySet()) { String v = entry.getValue(); if (v.toLowerCase().equals(tn)) { result = entry.getKey(); @@ -181,7 +219,7 @@ public String getXmlType(String typeName) { if (typeName != null && !typeName.isEmpty()) { String tn = typeName.toLowerCase(); - for (Map.Entry<String, String> entry : map.entrySet()) { + for (Map.Entry<String, String> entry : typesByTag.entrySet()) { String k = entry.getKey(); String v = entry.getValue(); if (v.toLowerCase().equals(tn) || k.toLowerCase().equals(tn)) { diff --git a/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/templates/FindRootClassTemplate b/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/templates/FindRootClassTemplate index 89e5178..dabf745 100644 --- a/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/templates/FindRootClassTemplate +++ b/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/templates/FindRootClassTemplate @@ -1,9 +1,10 @@ def findRootClass(rootTag): """ - Helper function that enables the generated code to locate the - root element. The api does not explicitly list a root - element; hence, the generated code has a hard time deducing - which one it actually is. This function will map the first - tag in the XML (i.e. the root) to an internal class. + This function was needed before version 2.12a of generateDS.py because the + generated code had no way to determine the type corresponding to an XML + type. This has been fixed, and generateDS.py generates now a map that + solves this problem. The function is preserved only for backwards + compatibility, as callers my be using it. Please refrain from using it in + new code as it may be removed in the future. """ return _rootClassMap.get(rootTag) diff --git a/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/templates/GetRootTagTemplate b/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/templates/GetRootTagTemplate deleted file mode 100644 index 845e6d1..0000000 --- a/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/templates/GetRootTagTemplate +++ /dev/null @@ -1,11 +0,0 @@ -def get_root_tag(node): - tag = Tag_pattern_.match(node.tag).groups()[-1] - #rootClass = globals().get(tag) - # Begin NOT_GENERATED - # The api XSD does not define a single root tag. - # We need to map the classes in this file to the possible - # element roots in the XSD. - # rootClass = globals().get(tag) - rootClass = findRootClass(tag) - # End NOT_GENERATED - return tag, rootClass \ No newline at end of file diff --git a/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/templates/GetRootTagTemplate.java b/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/templates/GetRootTagTemplate.java deleted file mode 100644 index 3992936..0000000 --- a/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/templates/GetRootTagTemplate.java +++ /dev/null @@ -1,25 +0,0 @@ -// -// Copyright (c) 2014 Red Hat, Inc. -// -// 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.ovirt.engine.sdk.generator.xsd.templates; - -import org.ovirt.engine.sdk.generator.templates.AbstractTemplate; - -public class GetRootTagTemplate extends AbstractTemplate { - public GetRootTagTemplate() { - super(); - } -} diff --git a/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/templates/ParseStringTemplate b/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/templates/ParseStringTemplate deleted file mode 100644 index 35b780f..0000000 --- a/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/templates/ParseStringTemplate +++ /dev/null @@ -1,20 +0,0 @@ -def parseString(inString): - from StringIO import StringIO - doc = parsexml_(StringIO(inString)) - rootNode = doc.getroot() - rootTag, rootClass = get_root_tag(rootNode) - if rootClass is None: - rootTag = 'link' - rootClass = Link - rootObj = rootClass.factory() - rootObj.build(rootNode) - # Enable Python to collect the space used by the DOM. - doc = None - # Begin NOT_GENERATED - # Let's shut up the echoing of the received XML - # to stdout. - #sys.stdout.write('<?xml version="1.0" ?>\n') - #rootObj.export(sys.stdout, 0, name_="link", - # namespacedef_='') - # End NOT_GENERATED - return rootObj diff --git a/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/templates/ParseStringTemplate.java b/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/templates/ParseStringTemplate.java deleted file mode 100644 index fec3133..0000000 --- a/generator/src/main/java/org/ovirt/engine/sdk/generator/xsd/templates/ParseStringTemplate.java +++ /dev/null @@ -1,22 +0,0 @@ -// -// Copyright (c) 2014 Red Hat, Inc. -// -// 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.ovirt.engine.sdk.generator.xsd.templates; - -import org.ovirt.engine.sdk.generator.templates.AbstractTemplate; - -public class ParseStringTemplate extends AbstractTemplate { -} diff --git a/src/ovirtsdk/infrastructure/errors.py b/src/ovirtsdk/infrastructure/errors.py index c75e62d..09eadc9 100644 --- a/src/ovirtsdk/infrastructure/errors.py +++ b/src/ovirtsdk/infrastructure/errors.py @@ -56,7 +56,7 @@ # REST error if res and res.startswith(RESPONSE_FORMAT) and res.find(RESPONSE_FAULT_BODY) != -1: try: - f_detail = params.parseString(res) + f_detail = params.parseString(res, silence=True) except: f_detail = '' diff --git a/src/ovirtsdk/infrastructure/proxy.py b/src/ovirtsdk/infrastructure/proxy.py index ee6b1f1..a11b754 100644 --- a/src/ovirtsdk/infrastructure/proxy.py +++ b/src/ovirtsdk/infrastructure/proxy.py @@ -156,7 +156,7 @@ ''' if obj is not None and obj is not '': try: - return params.parseString(obj) + return params.parseString(obj, silence=True) except etree.XMLSyntaxError: # raised when server replies in non-XML format, # the motivation for this error is #915036 diff --git a/src/ovirtsdk/utils/parsehelper.py b/src/ovirtsdk/utils/parsehelper.py index 1559937..7d2f9ba 100644 --- a/src/ovirtsdk/utils/parsehelper.py +++ b/src/ovirtsdk/utils/parsehelper.py @@ -24,16 +24,33 @@ '''Provides parsing capabilities''' @staticmethod - def toXml(entity): - '''Parse entity to corresponding XML representation''' + def toXml(obj): + """Convert object to corresponding XML representation.""" + # The given object may be a broker. If it is then we need to extract + # the actual entity object. + entity = obj if ReflectionHelper.isModuleMember(sys.modules['ovirtsdk.infrastructure.brokers'], - type(entity)) and hasattr(entity, 'superclass'): - entity = entity.superclass + type(obj)) and hasattr(obj, 'superclass'): + entity = obj.superclass - type_name = type(entity).__name__.lower() + # Find the XML tag corresponding to the entity type. There won't be + # such a tag if the type doesn't have a top level element definition + # inside the XML schema, as those can't be used as root elements and + # aren't accepted or generated by the RESTAPI server. + entity_type = type(entity) + entity_tag = params._tag_for_type.get(entity_type) + if entity_tag is None: + raise Exception( + "The entity type \"%s\" can't be converted to XML because " + "according to the XML schema it hasn't a top level element " + "definition, thus it can't be used as the root of a valid " + "XML document." % entity_type.__name__ + ) + + # Generate the XML document: output = StringIO.StringIO() - entity.export(output, 0, name_=ParseHelper.getXmlTypeInstance(type_name)) + entity.export(output, 0, name_=entity_tag) return output.getvalue() @staticmethod -- To view, visit http://gerrit.ovirt.org/32410 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I67b22c07ff4b91a7bf9b2ff8883ee51bef8c8f58 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine-sdk Gerrit-Branch: sdk_3.5 Gerrit-Owner: Juan Hernandez <juan.hernan...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
