From f18954d4d3ac4dea6436356667444f0b62ea4d92 Mon Sep 17 00:00:00 2001 From: Dave Briccetti Date: Thu, 26 Oct 2017 11:05:53 -0700 Subject: [PATCH] Fix Unexpected multiplicity mismatch IllegalState exception with template repeat (#207) Fix a bug in allChildrenAreElementsAndHaveTheSameName (now called childOptimizationsOk) where the special nature of elements with the jr:template attribute went unnoticed, causing the Unexpected multiplicity mismatch IllegalState exception reported in https://github.com/opendatakit/javarosa/issues/202. Extract duplicate code into new isTemplate method in XFormParser. --- src/org/javarosa/xform/parse/XFormParser.java | 24 +++++++++++-------- .../xform/parse/ChildProcessingTest.java | 22 ++++++++++++----- .../javarosa/xform/parse/XFormParserTest.java | 2 +- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/org/javarosa/xform/parse/XFormParser.java b/src/org/javarosa/xform/parse/XFormParser.java index 9a8e0ac04..b33695fce 100644 --- a/src/org/javarosa/xform/parse/XFormParser.java +++ b/src/org/javarosa/xform/parse/XFormParser.java @@ -1793,7 +1793,7 @@ public static TreeElement buildInstanceStructure (Element node, TreeElement pare //check for repeat templating final String name = node.getName(); final int multiplicity; - if (node.getAttributeValue(NAMESPACE_JAVAROSA, "template") != null) { + if (isTemplate(node)) { multiplicity = TreeReference.INDEX_TEMPLATE; if (parent != null && parent.getChild(name, TreeReference.INDEX_TEMPLATE) != null) { throw new XFormParseException("More than one node declared as the template for the same repeated set [" + name + "]",node); @@ -1833,7 +1833,7 @@ public static TreeElement buildInstanceStructure (Element node, TreeElement pare if (hasElements) { - Integer newMultiplicityFromGroup = allChildrenAreElementsAndHaveTheSameName(node) ? 0 : null; + Integer newMultiplicityFromGroup = childOptimizationsOk(node) ? 0 : null; for (int i = 0; i < numChildren; i++) { if (node.getType(i) == Node.ELEMENT) { TreeElement newChild = buildInstanceStructure(node.getElement(i), element, @@ -1865,27 +1865,31 @@ public static TreeElement buildInstanceStructure (Element node, TreeElement pare return element; } + private static boolean isTemplate(Element node) { + return node.getAttributeValue(NAMESPACE_JAVAROSA, "template") != null; + } + /** * If all children of {@code parent} are {@link Element}s ({@link Element#getElement} returns non-null), - * and the names of the children are all the same, more efficient methods may be used to build the - * collection of children. This method makes that determination. + * and the names of the children are all the same, and none of the children contain the template + * attribute, more efficient methods may be used to build the collection of children. This method makes + * that determination. * * @param parent the parent whose children are to be examined - * @return true iff {@code parent} has children, all children are {@link Element}s, - * and their names are all the same + * @return the determination described above */ - static boolean allChildrenAreElementsAndHaveTheSameName(Element parent) { + static boolean childOptimizationsOk(Element parent) { if (parent.getChildCount() == 0) { return false; } final Element firstChild = parent.getElement(0); - if (firstChild == null) { + if (firstChild == null || isTemplate(firstChild)) { return false; } final String firstName = firstChild.getName(); for (int i = 1; i < parent.getChildCount(); i++) { Element child = parent.getElement(i); - if (child == null || !child.getName().equals(firstName)) { + if (child == null || isTemplate(child) || !child.getName().equals(firstName)) { return false; } } @@ -1914,7 +1918,7 @@ private static void loadInstanceData (Element node, TreeElement cur, FormDef f) String name = child.getName(); int index; - boolean isTemplate = (child.getAttributeValue(NAMESPACE_JAVAROSA, "template") != null); + boolean isTemplate = isTemplate(child); if (isTemplate) { index = TreeReference.INDEX_TEMPLATE; diff --git a/test/org/javarosa/xform/parse/ChildProcessingTest.java b/test/org/javarosa/xform/parse/ChildProcessingTest.java index 8a8bef534..02fe143f6 100644 --- a/test/org/javarosa/xform/parse/ChildProcessingTest.java +++ b/test/org/javarosa/xform/parse/ChildProcessingTest.java @@ -3,6 +3,7 @@ import org.junit.Test; import org.kxml2.kdom.Element; +import static org.javarosa.xform.parse.XFormParser.NAMESPACE_JAVAROSA; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.kxml2.kdom.Node.COMMENT; @@ -12,32 +13,41 @@ public class ChildProcessingTest { @Test public void worksWithOneChild() { Element el = new Element(); el.addChild(ELEMENT, el.createElement(null, "Child Name")); - assertTrue(XFormParser.allChildrenAreElementsAndHaveTheSameName(el)); + assertTrue(XFormParser.childOptimizationsOk(el)); } @Test public void worksWithTwoMatchingChildren() { Element el = new Element(); el.addChild(ELEMENT, el.createElement(null, "Child Name")); el.addChild(ELEMENT, el.createElement(null, "Child Name")); - assertTrue(XFormParser.allChildrenAreElementsAndHaveTheSameName(el)); + assertTrue(XFormParser.childOptimizationsOk(el)); } @Test public void worksWithTwoNotMatchingChildren() { Element el = new Element(); el.addChild(ELEMENT, el.createElement(null, "Child Name 1")); el.addChild(ELEMENT, el.createElement(null, "Child Name 2")); - assertFalse(XFormParser.allChildrenAreElementsAndHaveTheSameName(el)); + assertFalse(XFormParser.childOptimizationsOk(el)); } - @Test public void WorksWithNoChildren() { + @Test public void recognizesThatOneChildIsATemplate() { Element el = new Element(); - assertFalse(XFormParser.allChildrenAreElementsAndHaveTheSameName(el)); + Element child1 = el.createElement(null, "Child Name"); + child1.setAttribute(NAMESPACE_JAVAROSA, "template", ""); + el.addChild(ELEMENT, child1); + el.addChild(ELEMENT, el.createElement(null, "Child Name")); + assertFalse(XFormParser.childOptimizationsOk(el)); + } + + @Test public void worksWithNoChildren() { + Element el = new Element(); + assertFalse(XFormParser.childOptimizationsOk(el)); } @Test public void discoversNonElementChild() { Element el = new Element(); el.addChild(COMMENT, "A string"); - assertFalse(XFormParser.allChildrenAreElementsAndHaveTheSameName(el)); + assertFalse(XFormParser.childOptimizationsOk(el)); } } diff --git a/test/org/javarosa/xform/parse/XFormParserTest.java b/test/org/javarosa/xform/parse/XFormParserTest.java index b46ff1036..517b4fffa 100644 --- a/test/org/javarosa/xform/parse/XFormParserTest.java +++ b/test/org/javarosa/xform/parse/XFormParserTest.java @@ -265,7 +265,7 @@ public void serializeAndRestoreMetaNamespaceFormInstance() throws IOException { assertEquals(AUDIT_3_ANSWER, audit3.getValue().getValue()); } - /* todo restore when fixed @Test */ public void parseFormWithTemplateRepeat() throws IOException { + @Test public void parseFormWithTemplateRepeat() throws IOException { // Given & When ParseResult parseResult = parse(r("template-repeat.xml"));