Improve test suite focusing on the DAG's behavior

src/main/java/org/javarosa/core/model/April2014DagImpl.java

@@ -179,7 +179,7 @@ public void finalizeTriggerables(FormInstance mainInstance, EvaluationContext ev
    /**
     * Get all of the elements which will need to be evaluated (in order) when
     * the triggerable is fired.
-    * 
+    *
     * @param qt
     */
    private void fillTriggeredElements(EvaluationContext evalContext, QuickTriggerable qt,
@@ -191,7 +191,7 @@ private void fillTriggeredElements(EvaluationContext evalContext, QuickTriggerab
             updatedNodes.add(target);
 
             // For certain types of triggerables, the update will affect not
-            // only the target, but also the children of the target. In that 
+            // only the target, but also the children of the target. In that
             // case, we want to add all of those nodes to the list of updated
             // elements as well.
             if (qt.t.isCascadingToChildren()) {
@@ -208,7 +208,7 @@ private void fillTriggeredElements(EvaluationContext evalContext, QuickTriggerab
                // relevancy calc
 
                // We can't make this reference generic before now or we'll
-               // lose the target information, so we'll be more inclusive 
+               // lose the target information, so we'll be more inclusive
                // than needed and see if any of our triggers are keyed on
                // the predicate-less path of this ref
                ArrayList<QuickTriggerable> triggered = triggerIndex.get(ref.hasPredicates() ? ref
@@ -266,7 +266,7 @@ public Collection<QuickTriggerable> initializeTriggerables(FormInstance mainInst
          QuickTriggerable qt = triggerablesDAG.get(i);
          for (int j = 0; j < qt.t.getTargets().size(); j++) {
             TreeReference target = qt.t.getTargets().get(j);
-            if (genericRoot.isParentOf(target, false)) {
+            if (genericRoot.isAncestorOf(target, false)) {
                applicable.add(qt);
                break;
             }

src/main/java/org/javarosa/core/model/Fast2014DagImpl.java

@@ -455,7 +455,7 @@ private Set<QuickTriggerable> initializeTriggerables(
             QuickTriggerable qt = triggerablesDAG.get(i);
             for (int j = 0; j < qt.t.getTargets().size(); j++) {
                 TreeReference target = qt.t.getTargets().get(j);
-                if (genericRoot.isParentOf(target, false)) {
+                if (genericRoot.isAncestorOf(target, false)) {
                     applicable.add(qt);
                     break;
                 }

src/main/java/org/javarosa/core/model/FormDef.java

@@ -393,7 +393,7 @@ public TreeReference getChildInstanceRef(List<IFormElement> elements, List<Integ
             IFormElement temp = elements.get(i);
             if (temp instanceof GroupDef && ((GroupDef) temp).getRepeat()) {
                 TreeReference repRef = FormInstance.unpackReference(temp.getBind());
-                if (repRef.isParentOf(ref, false)) {
+                if (repRef.isAncestorOf(ref, false)) {
                     int repMult = multiplicities.get(i);
                     ref.setMultiplicity(repRef.size() - 1, repMult);
                 } else {

src/main/java/org/javarosa/core/model/LegacyDagImpl.java

@@ -209,7 +209,7 @@ public Collection<QuickTriggerable> initializeTriggerables(FormInstance mainInst
             QuickTriggerable qt = triggerablesDAG.get(i);
             for (int j = 0; j < qt.t.getTargets().size(); j++) {
                 TreeReference target = qt.t.getTargets().get(j);
-                if (genericRoot.isParentOf(target, false)) {
+                if (genericRoot.isAncestorOf(target, false)) {
                     applicable.add(qt);
                     break;
                 }

src/main/java/org/javarosa/core/model/Safe2014DagImpl.java

@@ -261,7 +261,7 @@ public void finalizeTriggerables(FormInstance mainInstance,
    /**
     * Get all of the elements which will need to be evaluated (in order) when
     * the triggerable is fired.
-    * 
+    *
     * @param qt
     * @param destinationSet
     *            where to store the triggerables
@@ -330,7 +330,7 @@ public void fillTriggeredElements(FormInstance mainInstance,
     * directly triggered conditions, identifying which conditions should
     * further be triggered due to their update, and then dispatching all of the
     * evaluations.
-    * 
+    *
     * @param tv
     *            A set of all of the trigerrables directly triggered by the
     *            value changed
@@ -442,7 +442,7 @@ private Set<QuickTriggerable> initializeTriggerables(
          QuickTriggerable qt = triggerablesDAG.get(i);
          for (int j = 0; j < qt.t.getTargets().size(); j++) {
             TreeReference target = qt.t.getTargets().get(j);
-            if (genericRoot.isParentOf(target, false)) {
+            if (genericRoot.isAncestorOf(target, false)) {
                applicable.add(qt);
                break;
             }

src/main/java/org/javarosa/core/model/actions/SetValueAction.java

@@ -80,7 +80,7 @@ public TreeReference processAction(FormDef model, TreeReference contextRef) {
             //Note: right now we're qualifying then testing parentage to see whether
             //there was a conflict, but it's not super clear whether this is a perfect
             //strategy
-            if (!contextRef.isParentOf(targetReference, false)) {
+            if (!contextRef.isAncestorOf(targetReference, false)) {
                 return null;
             }
         }
@@ -162,4 +162,4 @@ public void writeExternal(DataOutputStream out) throws IOException {
             ExtUtil.write(out, new ExtWrapTagged(value));
         }
     }
-}
+}

src/main/java/org/javarosa/core/model/instance/TreeReference.java

@@ -338,7 +338,7 @@ public TreeReference contextualize (TreeReference contextRef) {
     }
 
     public TreeReference relativize (TreeReference parent) {
-        if (parent.isParentOf(this, false)) {
+        if (parent.isAncestorOf(this, false)) {
             TreeReference relRef = selfRef();
             for (int i = parent.size(); i < this.size(); i++) {
                 relRef.add(this.getName(i), INDEX_UNBOUND);
@@ -362,7 +362,7 @@ public TreeReference genericize () {
 
     //returns true if 'this' is parent of 'child'
     //return true if 'this' equals 'child' only if properParent is false
-    public boolean isParentOf (TreeReference child, boolean properParent) {
+    public boolean isAncestorOf(TreeReference child, boolean properParent) {
         //Instances and context types;
         if (refLevel != child.refLevel)
             return false;
@@ -667,4 +667,4 @@ public TreeReference removePredicates() {
         }
         return predicateless;
     }
-}
+}

src/main/java/org/javarosa/xform/parse/FormInstanceParser.java

@@ -272,7 +272,7 @@ private void verifyRepeatMemberBindings (IFormElement fe, FormInstance instance,
             TreeReference childBind = FormInstance.unpackReference(child.getBind());
 
             //check if current binding is within scope of repeat binding
-            if (!repeatBind.isParentOf(childBind, false)) {
+            if (!repeatBind.isAncestorOf(childBind, false)) {
                 //catch <repeat nodeset="/a/b"><input ref="/a/c" /></repeat>: repeat question is not a child of the repeated node
                 throw new XFormParseException("<repeat> member's binding [" + childBind.toString() + "] is not a descendant of <repeat> binding [" + repeatBind.toString() + "]!");
             } else if (repeatBind.equals(childBind) && isRepeat) {
@@ -313,16 +313,16 @@ private void verifyRepeatMemberBindings (IFormElement fe, FormInstance instance,
     private void verifyItemsetBindings (FormInstance instance) {
         for (ItemsetBinding itemset : itemsets) {
             //check proper parent/child relationship
-            if (!itemset.nodesetRef.isParentOf(itemset.labelRef, false)) {
+            if (!itemset.nodesetRef.isAncestorOf(itemset.labelRef, false)) {
                 throw new XFormParseException("itemset nodeset ref is not a parent of label ref");
-            } else if (itemset.copyRef != null && !itemset.nodesetRef.isParentOf(itemset.copyRef, false)) {
+            } else if (itemset.copyRef != null && !itemset.nodesetRef.isAncestorOf(itemset.copyRef, false)) {
                 throw new XFormParseException("itemset nodeset ref is not a parent of copy ref");
-            } else if (itemset.valueRef != null && !itemset.nodesetRef.isParentOf(itemset.valueRef, false)) {
+            } else if (itemset.valueRef != null && !itemset.nodesetRef.isAncestorOf(itemset.valueRef, false)) {
                 throw new XFormParseException("itemset nodeset ref is not a parent of value ref");
             }
 
             if (itemset.copyRef != null && itemset.valueRef != null) {
-                if (!itemset.copyRef.isParentOf(itemset.valueRef, false)) {
+                if (!itemset.copyRef.isAncestorOf(itemset.valueRef, false)) {
                     throw new XFormParseException("itemset <copy> is not a parent of <value>");
                 }
             }

src/main/java/org/javarosa/xpath/expr/XPathFuncExpr.java

@@ -978,7 +978,7 @@ public static Object indexedRepeat(DataInstance model, EvaluationContext ec, XPa
             }
             // confirm that the passed XPath is a parent of our overall target path
             TreeReference groupRef = ((XPathPathExpr) args[pathargi]).getReference();
-            if (!groupRef.isParentOf(targetRef, true)) {
+            if (!groupRef.isAncestorOf(targetRef, true)) {
                 throw new XPathTypeMismatchException("indexed-repeat(): parameter " + (pathargi + 1) + " must be a parent of the field in parameter 1");
             }
 

src/test/java/org/javarosa/core/model/instance/TestHelpers.java

@@ -2,15 +2,27 @@
 
 import static org.javarosa.xpath.expr.XPathPathExpr.INIT_CONTEXT_RELATIVE;
 
+import java.util.List;
 import org.javarosa.model.xform.XPathReference;
+import org.javarosa.xpath.expr.XPathExpression;
+import org.javarosa.xpath.expr.XPathNumericLiteral;
 import org.javarosa.xpath.expr.XPathPathExpr;
 import org.javarosa.xpath.expr.XPathStep;
 
 public class TestHelpers {
     public static TreeReference buildRef(String xpath) {
-        return xpath.isEmpty()
-            // Support for an empty xpath, mapping it to a relative TreeReference with no steps in it.
-            ? new XPathPathExpr(INIT_CONTEXT_RELATIVE, new XPathStep[0]).getReference()
-            : (TreeReference) new XPathReference(xpath).getReference();
+        // Support for an empty xpath, mapping it to a relative TreeReference with no steps in it.
+        if (xpath.isEmpty())
+            return new XPathPathExpr(INIT_CONTEXT_RELATIVE, new XPathStep[0]).getReference();
+
+        TreeReference ref = (TreeReference) new XPathReference(xpath).getReference();
+        // Set correct multiplicities when there's a numeric predicate pointing to an item inside an itemset as in /foo/bar[3]
+        for (int i = 0; i < ref.size(); i++) {
+            List<XPathExpression> predicates = ref.getPredicate(i);
+            if (predicates != null && predicates.size() == 1 && predicates.get(0) instanceof XPathNumericLiteral)
+                ref.setMultiplicity(i, ((Double) ((XPathNumericLiteral) predicates.get(0)).d).intValue());
+        }
+
+        return ref;
     }
 }

src/test/java/org/javarosa/core/model/instance/TreeReferenceIsAncestorOfTest.java

@@ -0,0 +1,133 @@
+/*
+ * Copyright 2019 Nafundi
+ *
+ * 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.javarosa.core.model.instance;
+
+import static org.hamcrest.Matchers.is;
+import static org.javarosa.core.model.instance.TestHelpers.buildRef;
+import static org.junit.Assert.assertThat;
+
+import java.util.Arrays;
+import java.util.Collection;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class TreeReferenceIsAncestorOfTest {
+    private static final boolean IRRELEVANT = true;
+
+    @Parameterized.Parameter(value = 0)
+    public String testCase;
+
+    @Parameterized.Parameter(value = 1)
+    public String a;
+
+    @Parameterized.Parameter(value = 2)
+    public String b;
+
+    @Parameterized.Parameter(value = 3)
+    public boolean properParent;
+
+    @Parameterized.Parameter(value = 4)
+    public boolean expectedResult;
+
+    @Parameterized.Parameters(name = "{0}")
+    public static Collection<Object[]> data() {
+        return Arrays.asList(new Object[][]{
+            // Self references
+            {"/foo is not parent of /foo if we exclude self references", "/foo", "/foo", true, false},
+            {"/foo/bar is not parent of /foo/bar if we exclude self references", "/foo/bar", "/foo/bar", true, false},
+            {"/foo is parent of /foo if we don't exclude self references", "/foo", "/foo", false, true},
+            {"/foo/bar is  parent of /foo/bar if we don't exclude self references", "/foo/bar", "/foo/bar", false, true},
+
+            // Simple scenarios
+            {"/foo/bar is not parent of /foo", "/foo/bar", "/foo", IRRELEVANT, false},
+            {"/foo is not parent of /bar", "/foo", "/bar", IRRELEVANT, false},
+            {"foo is not parent of bar", "foo", "bar", IRRELEVANT, false},
+            {"/foo is not parent of /bar/foo", "/foo", "/bar/foo", IRRELEVANT, false},
+            {"/foo is parent of /foo/bar", "/foo", "/foo/bar", IRRELEVANT, true},
+            {"/foo/bar is parent of /foo/bar/baz", "/foo/bar", "/foo/bar/baz", IRRELEVANT, true},
+
+            // Ancestry (more than parent) scenarios
+            {"/foo is parent of /foo/bar/baz", "/foo", "/foo/bar/baz", IRRELEVANT, true},
+
+            // Scenarios with a mix of relative and absolute paths
+            {"foo is not parent of /foo/bar", "foo", "/foo/bar", IRRELEVANT, false},
+            {"/foo is not parent of foo/bar", "/foo", "foo/bar", IRRELEVANT, false},
+
+            // We will consider undefined (no multiplicity predicate), [-1], and [0] multiplicities due to their special meaning and [2]
+            // as an example of a specific non-special multiplicity
+            // We will also consider first-level and following-level multiplicities, because the first level behaves differently
+
+            // Multiplicity scenario: undefined ancestor is parent of any other multiplicity, regardless of level
+            {"/foo is parent of /foo/bar", "/foo", "/foo/bar", IRRELEVANT, true},
+            {"/foo is parent of /foo[-1]/bar", "/foo", "/foo[-1]/bar", IRRELEVANT, true},
+            {"/foo is parent of /foo[0]/bar", "/foo", "/foo[0]/bar", IRRELEVANT, true},
+            {"/foo is parent of /foo[2]/bar", "/foo", "/foo[2]/bar", IRRELEVANT, true},
+            {"/foo/bar is parent of /foo/bar/baz", "/foo/bar", "/foo/bar/baz", IRRELEVANT, true},
+            {"/foo/bar is parent of /foo/bar[-1]/baz", "/foo/bar", "/foo/bar[-1]/baz", IRRELEVANT, true},
+            {"/foo/bar is parent of /foo/bar[0]/baz", "/foo/bar", "/foo/bar[0]/baz", IRRELEVANT, true},
+            {"/foo/bar is parent of /foo/bar[2]/baz", "/foo/bar", "/foo/bar[2]/baz", IRRELEVANT, true},
+
+            // Multiplicity scenario: [-1] ancestor is parent of any other multiplity, regardless of level
+            {"/foo[-1] is parent of /foo/bar", "/foo[-1]", "/foo/bar", IRRELEVANT, true},
+            {"/foo[-1] is parent of /foo[-1]/bar", "/foo[-1]", "/foo[-1]/bar", IRRELEVANT, true},
+            {"/foo[-1] is parent of /foo[0]/bar", "/foo[-1]", "/foo[0]/bar", IRRELEVANT, true},
+            {"/foo[-1] is parent of /foo[2]/bar", "/foo[-1]", "/foo[2]/bar", IRRELEVANT, true},
+            {"/foo/bar[-1] is parent of /foo/bar/baz", "/foo/bar[-1]", "/foo/bar/baz", IRRELEVANT, true},
+            {"/foo/bar[-1] is parent of /foo/bar[-1]/baz", "/foo/bar[-1]", "/foo/bar[-1]/bzr", IRRELEVANT, true},
+            {"/foo/bar[-1] is parent of /foo/bar[0]/baz", "/foo/bar[-1]", "/foo/bar[0]/bzr", IRRELEVANT, true},
+            {"/foo/bar[-1] is parent of /foo/bar[2]/baz", "/foo/bar[-1]", "/foo/bar[2]/bzr", IRRELEVANT, true},
+
+            // Multiplicity scenario: [0] ancestor is parent of other special multiplicities only at first level
+            {"/foo[0] is parent of /foo/bar", "/foo[0]", "/foo/bar", IRRELEVANT, true},
+            {"/foo[0] is parent of /foo[-1]/bar", "/foo[0]", "/foo[-1]/bar", IRRELEVANT, true},
+            {"/foo[0] is parent of /foo[0]/bar", "/foo[0]", "/foo[0]/bar", IRRELEVANT, true},
+            {"/foo[0] is not parent of /foo[2]/bar", "/foo[0]", "/foo[2]/bar", IRRELEVANT, false},
+            {"/foo/bar[0] is not parent of /foo/bar/baz", "/foo/bar[0]", "/foo/bar/baz", IRRELEVANT, false},
+            {"/foo/bar[0] is not parent of /foo/bar[-1]/baz", "/foo/bar[0]", "/foo/bar[-1]/baz", IRRELEVANT, false},
+            {"/foo/bar[0] is parent of /foo/bar[0]/baz", "/foo/bar[0]", "/foo/bar[0]/baz", IRRELEVANT, true},
+            {"/foo/bar[0] is not parent of /foo/bar[2]/baz", "/foo/bar[0]", "/foo/bar[2]/baz", IRRELEVANT, false},
+
+            // Multiplicity scenario: [2] ancestor is not parent of special multiplicities, regardless of level
+            {"/foo[2] is parent of /foo/bar", "/foo[2]", "/foo[0]/bar", IRRELEVANT, false},
+            {"/foo[2] is parent of /foo[-1]/bar", "/foo[2]", "/foo[-1]/bar", IRRELEVANT, false},
+            {"/foo[2] is parent of /foo[0]/bar", "/foo[2]", "/foo[0]/bar", IRRELEVANT, false},
+            {"/foo[2] is parent of /foo[2]/bar", "/foo[2]", "/foo[2]/bar", IRRELEVANT, true},
+            {"/foo/bar[2] is parent of /foo/bar/baz", "/foo/bar[2]", "/foo/bar[0]/baz", IRRELEVANT, false},
+            {"/foo/bar[2] is parent of /foo/bar[-1]/baz", "/foo/bar[2]", "/foo/bar[-1]/baz", IRRELEVANT, false},
+            {"/foo/bar[2] is parent of /foo/bar[0]/baz", "/foo/bar[2]", "/foo/bar[0]/baz", IRRELEVANT, false},
+            {"/foo/bar[2] is parent of /foo/bar[2]/baz", "/foo/bar[2]", "/foo/bar[2]/baz", IRRELEVANT, true},
+
+            // Multiplicity scenario: Mixed bag of other interesting examples
+            {"/foo[-1]/bar[2] is parent of /foo/bar[2]/baz", "/foo[-1]/bar[2]", "/foo/bar[2]/baz", IRRELEVANT, true},
+            {"/foo[-1]/bar[2] is parent of /foo[-1]/bar[2]/baz", "/foo[-1]/bar[2]", "/foo[-1]/bar[2]/baz", IRRELEVANT, true},
+            {"/foo[-1]/bar[2] is parent of /foo[0]/bar[2]/baz", "/foo[-1]/bar[2]", "/foo[0]/bar[2]/baz", IRRELEVANT, true},
+            {"/foo[-1]/bar[2] is parent of /foo[2]/bar[2]/baz", "/foo[-1]/bar[2]", "/foo[2]/bar[2]/baz", IRRELEVANT, true},
+            {"/foo[2]/bar[2] is parent of /foo[3]/bar[2]/baz", "/foo[2]/bar[2]", "/foo[3]/bar[2]/baz", IRRELEVANT, false},
+        });
+    }
+
+    @Test
+    public void parent_works_as_expected() {
+        assertThat(
+            buildRef(a).isAncestorOf(buildRef(b), properParent),
+            is(expectedResult)
+        );
+    }
+
+}