Merge pull request #750 from lognaturel/relative-refs

Change absolute refs in repeats to relative
getodk · Mar 21, 2024 · 88ee511 · 88ee511
2 parents 6a32b4b + c70bd07
commit 88ee511
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 8 deletions.
diff --git a/src/test/java/org/javarosa/core/model/RepeatTest.java b/src/test/java/org/javarosa/core/model/RepeatTest.java
@@ -1,5 +1,6 @@
 package org.javarosa.core.model;
 
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
 import static org.javarosa.core.util.BindBuilderXFormsElement.bind;
 import static org.javarosa.core.util.XFormsElement.body;
@@ -13,10 +14,12 @@
 import static org.javarosa.core.util.XFormsElement.select1;
 import static org.javarosa.core.util.XFormsElement.t;
 import static org.javarosa.core.util.XFormsElement.title;
-import static org.junit.Assert.assertThat;
 
+import java.io.IOException;
+import org.javarosa.core.model.data.IntegerData;
 import org.javarosa.core.test.Scenario;
 import org.javarosa.form.api.FormEntryController;
+import org.javarosa.xform.parse.XFormParser;
 import org.junit.Test;
 
 public class RepeatTest {
@@ -104,4 +107,65 @@ public void whenRepeatAndTopLevelNodeHaveSameRelevanceExpression_andExpressionEv
 
         assertThat(event, is(FormEntryController.EVENT_END_OF_FORM));
     }
+
+    /**
+     * The original ODK XForms spec deviated from XPath rules by stating that path expressions representing single nodes
+     * should be evaluated as relative to the current nodeset. That has since been removed and all known form builders
+     * create relative references in expressions within a repeat. We currently maintain this behavior for legacy purposes.
+     */
+    @Test
+    public void absoluteSingleNodePaths_areQualified_forLegacyPurposes() throws IOException, XFormParser.ParseException {
+        Scenario scenario = Scenario.init("Absolute relative ref", html(
+            head(
+                title("Absolute relative ref"),
+                model(
+                    mainInstance(t("data id=\"data\"",
+                        t("outer",
+                            t("outerq1"),
+                            t("outercalcabs"),
+                            t("outercalcrel"),
+                            t("inner",
+                                t("innerq1"),
+                                t("innercalcabs"),
+                                t("innercalcrel")
+                            )
+                        )
+                    )),
+                    bind("/data/outer/outercalcabs").calculate("/data/outer/outerq1 + 1"),
+                    bind("/data/outer/outercalcrel").calculate("../outerq1 + 1"),
+                    bind("/data/outer/inner/innercalcabs").calculate("/data/outer/inner/innerq1 + 2"),
+                    bind("/data/outer/inner/innercalcrel").calculate("../innerq1 + 2")
+                )),
+            body(
+                repeat("/data/outer",
+                    input("/data/outer/outerq1"),
+                    repeat("/data/outer/inner",
+                        input("/data/outer/inner/innerq1"))
+                )
+            )));
+
+        scenario.answer("/data/outer[0]/outerq1", "5");
+        assertThat(scenario.answerOf("/data/outer[0]/outercalcabs"), is(new IntegerData(6)));
+        assertThat(scenario.answerOf("/data/outer[0]/outercalcrel"), is(new IntegerData(6)));
+
+        scenario.createNewRepeat("/data/outer");
+
+        scenario.answer("/data/outer[1]/outerq1", "23");
+        // In a standards-compliant XPath engine, this would be 6 because /data/outer/outerq1 in the calculate expression
+        // would always be equivalent to /data/outer[0]/outerq1
+        assertThat(scenario.answerOf("/data/outer[1]/outercalcabs"), is(new IntegerData(24)));
+        assertThat(scenario.answerOf("/data/outer[1]/outercalcrel"), is(new IntegerData(24)));
+
+        scenario.answer("/data/outer[0]/inner[0]/innerq1", 18);
+        assertThat(scenario.answerOf("/data/outer[0]/inner[0]/innercalcabs"), is(new IntegerData(20)));
+        assertThat(scenario.answerOf("/data/outer[0]/inner[0]/innercalcrel"), is(new IntegerData(20)));
+
+        scenario.createNewRepeat("/data/outer[0]/inner");
+
+        scenario.answer("/data/outer[0]/inner[1]/innerq1", 19);
+        // In a standards-compliant XPath engine, this would be 20 because /data/outer/inner/innerq1 in the calculate expression
+        // would always be equivalent to /data/outer[0]/inner[0]/innerq1
+        assertThat(scenario.answerOf("/data/outer[0]/inner[1]/innercalcabs"), is(new IntegerData(21)));
+        assertThat(scenario.answerOf("/data/outer[0]/inner[1]/innercalcrel"), is(new IntegerData(21)));
+    }
 }
diff --git a/src/test/java/org/javarosa/core/model/TriggerableDagTest.java b/src/test/java/org/javarosa/core/model/TriggerableDagTest.java
@@ -5,6 +5,7 @@
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.IntStream.range;
 import static org.hamcrest.CoreMatchers.nullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.is;
 import static org.javarosa.core.test.AnswerDataMatchers.booleanAnswer;
@@ -32,7 +33,6 @@
 import static org.javarosa.core.util.XFormsElement.title;
 import static org.javarosa.form.api.FormEntryController.ANSWER_CONSTRAINT_VIOLATED;
 import static org.javarosa.form.api.FormEntryController.ANSWER_REQUIRED_BUT_EMPTY;
-import static org.hamcrest.MatcherAssert.assertThat;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -834,7 +834,7 @@ public void addingRepeat_updatesInnerCalculations_withMultipleDependencies() thr
                         ))),
                     // position(..) means the full cascade is evaulated as part of triggerTriggerables
                     bind("/data/repeat/position").calculate("position(..)"),
-                    bind("/data/repeat/position_2").calculate("/data/repeat/position * 2"),
+                    bind("/data/repeat/position_2").calculate("../position * 2"),
                     bind("/data/repeat/other").calculate("2 * 2"),
                     // concat needs to be evaluated after /data/repeat/other has a value
                     bind("/data/repeat/concatenated").calculate("concat(../position_2, '-', ../other)"))),
@@ -1017,7 +1017,7 @@ public void addingOrRemovingRepeatInstance_withReferenceToRepeatInRepeat_andOute
                     )),
                     bind("/data/sum").type("int").calculate("sum(/data/repeat/position1)"),
                     bind("/data/repeat/position1").type("int").calculate("position(..)"),
-                    bind("/data/repeat/position2").type("int").calculate("/data/repeat/position1"))),
+                    bind("/data/repeat/position2").type("int").calculate("../position1"))),
 
             body(
                 repeat("/data/repeat",
@@ -1194,7 +1194,7 @@ public void deleteSecondRepeatGroup_evaluatesTriggerables_dependentOnTheParentPo
                     )),
                     bind("/data/house/number").type("int").calculate("position(..)"),
                     bind("/data/house/name").type("string").required(),
-                    bind("/data/house/name_and_number").type("string").calculate("concat(/data/house/name, /data/house/number)")
+                    bind("/data/house/name_and_number").type("string").calculate("concat(../name, ../number)")
                 )
             ),
             body(group("/data/house", repeat("/data/house", input("/data/house/name"))))
@@ -1245,7 +1245,7 @@ public void deleteSecondRepeatGroup_doesNotEvaluateTriggerables_notDependentOnTh
                     )),
                     bind("/data/house/number").type("int").calculate("position(..)"),
                     bind("/data/house/name").type("string").required(),
-                    bind("/data/house/name_and_number").type("string").calculate("concat(/data/house/name, 'X')")
+                    bind("/data/house/name_and_number").type("string").calculate("concat(../name, 'X')")
                 )
             ),
             body(group("/data/house", repeat("/data/house", input("/data/house/name"))))
@@ -1729,7 +1729,7 @@ public void issue_119_target_question_should_be_relevant() throws IOException, X
                     bind("/data/inner_trigger").type("int"),
                     bind("/data/outer").relevant("/data/outer_trigger = 'D'"),
                     bind("/data/outer/inner_condition").type("boolean").calculate("/data/inner_trigger > 10"),
-                    bind("/data/outer/inner").relevant("/data/outer/inner_condition"),
+                    bind("/data/outer/inner").relevant("../inner_condition"),
                     bind("/data/outer/inner/target_question").type("string")
                 )
             ),

diff --git a/src/test/java/org/javarosa/core/model/actions/SetValueActionTest.java b/src/test/java/org/javarosa/core/model/actions/SetValueActionTest.java
@@ -374,7 +374,7 @@ public void setvalueInOuterRepeat_setsInnerRepeatValue() throws IOException, XFo
             body(
                 repeat("/data/repeat1",
                     input("/data/repeat1/source",
-                        setvalue("xforms-value-changed", "/data/repeat1/repeat2/destination", "/data/repeat1/source")),
+                        setvalue("xforms-value-changed", "/data/repeat1/repeat2/destination", "../../source")),
                     repeat("/data/repeat1/repeat2",
                         input("/data/repeat1/repeat2/destination")
             )))));