Fix Unexpected multiplicity mismatch IllegalState exception with temp…

…late 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 #202. Extract duplicate code into new isTemplate method in XFormParser.

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;

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));
     }
 
 }

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"));