diff --git a/src/main/java/org/javarosa/xpath/expr/XPathFuncExpr.java b/src/main/java/org/javarosa/xpath/expr/XPathFuncExpr.java index 6f11159ef..1484a36ef 100644 --- a/src/main/java/org/javarosa/xpath/expr/XPathFuncExpr.java +++ b/src/main/java/org/javarosa/xpath/expr/XPathFuncExpr.java @@ -705,12 +705,18 @@ public static Double toDouble(Object o) { } /** - * convert a string value to an integer by: + * Convert a non-zero-length string value to an integer by: * - encoding it as utf-8 * - hashing it with sha256 (available cross-platform, including via browser crypto API) * - interpreting the first 8 bytes of the hash as a long + * + * A zero-length string results in a 0L — this is the classic behaviour that we want + * to conserve for backward compatibility with a case that is expected to be common. + * In practical terms, we don't want the sort order of choice lists using an empty string as + * a seed to change with the introduction of this seed derivation mechanism. */ public static long toLongHash(String sourceString) { + if (sourceString.length() == 0) return 0L; byte[] hasheeBuf = sourceString.getBytes(Charset.forName("UTF-8")); SHA256Digest hasher = new SHA256Digest(); hasher.update(hasheeBuf, 0, hasheeBuf.length); diff --git a/src/test/java/org/javarosa/xpath/expr/RandomizeTypesTest.java b/src/test/java/org/javarosa/xpath/expr/RandomizeTypesTest.java index 091df0c4a..7bd034639 100644 --- a/src/test/java/org/javarosa/xpath/expr/RandomizeTypesTest.java +++ b/src/test/java/org/javarosa/xpath/expr/RandomizeTypesTest.java @@ -5,6 +5,7 @@ import org.junit.Test; import java.io.IOException; +import java.util.Arrays; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; @@ -30,21 +31,35 @@ public void stringNumberSeedConvertsWhenUsedInNodesetExpression() throws IOExcep title("Randomize non-numeric seed"), model( mainInstance(t("data id=\"rand-non-numeric\"", - t("choice") + t("choices_numeric_seed"), + t("choices_stringified_numeric_seed") )), instance("choices", item("a", "A"), - item("b", "B") + item("b", "B"), + item("c", "C"), + item("d", "D"), + item("e", "E"), + item("f", "F"), + item("g", "G"), + item("h", "H") ), - bind("/data/choice").type("string") + bind("/data/choices_numeric_seed").type("string"), + bind("/data/choices_stringified_numeric_seed").type("string") ) ), body( - select1Dynamic("/data/choice", "randomize(instance('choices')/root/item, '1')") + select1Dynamic("/data/choices_numeric_seed", "randomize(instance('choices')/root/item, 1234)"), + select1Dynamic("/data/choices_stringified_numeric_seed", "randomize(instance('choices')/root/item, '1234')") ) )); - - assertThat(scenario.choicesOf("/data/choice").get(0).getValue(), is("b")); + String[] shuffled = {"g", "f", "e", "d", "a", "h", "b", "c"}; + String[] nodes = {"/data/choices_numeric_seed", "/data/choices_stringified_numeric_seed"}; + for (int i = 0; i < shuffled.length; i++) { + for (String node : nodes) { + assertThat(scenario.choicesOf(node).get(i).getValue(), is(shuffled[i])); + } + } } @Test @@ -54,21 +69,30 @@ public void stringNumberSeedConvertsWhenUsedInCalculate() throws IOException, XF title("Randomize non-numeric seed"), model( mainInstance(t("data id=\"rand-non-numeric\"", - t("choice") + t("choices_numeric_seed"), + t("choices_stringified_numeric_seed") )), instance("choices", item("a", "A"), - item("b", "B") + item("b", "B"), + item("c", "C"), + item("d", "D"), + item("e", "E"), + item("f", "F"), + item("g", "G"), + item("h", "H") ), - bind("/data/choice").type("string").calculate("selected-at(join(' ', randomize(instance('choices')/root/item/label, '1')), 0)") + bind("/data/choices_numeric_seed").type("string").calculate("join('', randomize(instance('choices')/root/item/label, 1234))"), + bind("/data/choices_stringified_numeric_seed").type("string").calculate("join('', randomize(instance('choices')/root/item/label, '1234'))") ) ), body( - input("/data/choice") + input("/data/choices_numeric_seed"), + input("/data/choices_stringified_numeric_seed") ) )); - - assertThat(scenario.answerOf("/data/choice").getDisplayText(), is("B")); + assertThat(scenario.answerOf("/data/choices_numeric_seed").getDisplayText(), is(scenario.answerOf("/data/choices_stringified_numeric_seed").getDisplayText())); + assertThat(scenario.answerOf("/data/choices_numeric_seed").getDisplayText(), is("GFEDAHBC")); } @Test @@ -82,7 +106,13 @@ public void stringTextSeedConvertsWhenUsedInNodesetExpression() throws IOExcepti )), instance("choices", item("a", "A"), - item("b", "B") + item("b", "B"), + item("c", "C"), + item("d", "D"), + item("e", "E"), + item("f", "F"), + item("g", "G"), + item("h", "H") ), bind("/data/choice").type("string") ) @@ -91,8 +121,10 @@ public void stringTextSeedConvertsWhenUsedInNodesetExpression() throws IOExcepti select1Dynamic("/data/choice", "randomize(instance('choices')/root/item, 'foo')") ) )); - - assertThat(scenario.choicesOf("/data/choice").get(0).getValue(), is("b")); + String[] shuffled = {"e", "a", "d", "b", "h", "g", "c", "f"}; + for (int i = 0; i < shuffled.length; i++) { + assertThat(scenario.choicesOf("/data/choice").get(i).getValue(), is(shuffled[i])); + } } @Test @@ -106,9 +138,15 @@ public void stringTextSeedConvertsWhenUsedInCalculate() throws IOException, XFor )), instance("choices", item("a", "A"), - item("b", "B") + item("b", "B"), + item("c", "C"), + item("d", "D"), + item("e", "E"), + item("f", "F"), + item("g", "G"), + item("h", "H") ), - bind("/data/choice").type("string").calculate("selected-at(join(' ', randomize(instance('choices')/root/item/label, 'foo')), 0)") + bind("/data/choice").type("string").calculate("join('', randomize(instance('choices')/root/item/label, 'foo'))") ) ), body( @@ -116,7 +154,7 @@ public void stringTextSeedConvertsWhenUsedInCalculate() throws IOException, XFor ) )); - assertThat(scenario.answerOf("/data/choice").getDisplayText(), is("B")); + assertThat(scenario.answerOf("/data/choice").getDisplayText(), is("EADBHGCF")); } @Test @@ -155,4 +193,98 @@ public void seedInRepeatIsEvaluatedForEachInstance() throws IOException, XFormPa assertThat(scenario.choicesOf("/data/repeat[2]/choice").get(0).getValue(), is("b")); assertThat(scenario.choicesOf("/data/repeat[1]/choice").get(0).getValue(), is("a")); } + + @Test + public void seedFromArbitraryInputCanBeUsed() throws IOException, XFormParser.ParseException { + Scenario scenario = Scenario.init("Randomize non-numeric seed", html( + head( + title("Randomize non-numeric seed"), + model( + mainInstance(t("data id=\"rand-non-numeric\"", + t("input"), + t("choice") + )), + instance("choices", + item("a", "A"), + item("b", "B"), + item("c", "C"), + item("d", "D"), + item("e", "E"), + item("f", "F"), + item("g", "G"), + item("h", "H") + ), + bind("/data/input").type("geopoint"), + bind("/data/choice").type("string") + ) + ), + body( + input("/data/input"), + select1Dynamic("/data/choice", "randomize(instance('choices')/root/item, /data/input)") + ) + )); + + scenario.answer("/data/input", "-6.8137120026589315 39.29392995851879"); + String[] shuffled = {"h", "b", "d", "f", "a", "g", "c", "e"}; + for (int i = 0; i < shuffled.length; i++) { + assertThat(scenario.choicesOf("/data/choice").get(i).getValue(), is(shuffled[i])); + } + } + + @Test + public void seed0FromNaNs() throws IOException, XFormParser.ParseException { + Scenario scenario = Scenario.init("Randomize non-numeric seed", html( + head( + title("Randomize non-numeric seed"), + model( + mainInstance(t("data id=\"rand-non-numeric\"", + t("input_emptystring"), + t("input_somestring"), + t("input_int"), + t("choice_emptystring"), + t("choice_somestring"), + t("choice_int") + )), + instance("choices", + item("a", "A"), + item("b", "B"), + item("c", "C"), + item("d", "D"), + item("e", "E"), + item("f", "F"), + item("g", "G"), + item("h", "H") + ), + bind("/data/input_emptystring").type("string"), + bind("/data/input_somestring").type("string"), + bind("/data/input_int").type("int") + ) + ), + body( + input("/data/input_emptystring"), + input("/data/input_somestring"), + input("/data/input_int"), + select1Dynamic("/data/choice_emptystring", "randomize(instance('choices')/root/item, /data/input_emptystring)"), + select1Dynamic("/data/choice_somestring", "randomize(instance('choices')/root/item, /data/input_somestring)"), + select1Dynamic("/data/choice_int", "randomize(instance('choices')/root/item, /data/input_int)") + ) + )); + + scenario.answer("/data/input_emptystring", ""); + scenario.answer("/data/input_somestring", "somestring"); + scenario.answer("/data/input_int", "0"); + + String[] shuffled_NaN_or_0 = {"c", "b", "h", "a", "f", "d", "g", "e"}; + String[] shuffled_somestring = {"e", "b", "c", "g", "d", "a", "f", "h"}; + assertThat("somestring-seeded expected order is distinct from 0-seeded expected order", !Arrays.equals(shuffled_NaN_or_0, shuffled_somestring)); + String[] shuffledfields = {"/data/choice_emptystring", "/data/choice_int"}; + for (int i = 0; i < shuffled_NaN_or_0.length; i++) { + for (String shuffledfield : shuffledfields) { + assertThat(scenario.choicesOf(shuffledfield).get(i).getValue(), is(shuffled_NaN_or_0[i])); + } + } + for (int i = 0; i < shuffled_somestring.length; i++) { + assertThat(scenario.choicesOf("/data/choice_somestring").get(i).getValue(), is(shuffled_somestring[i])); + } + } } diff --git a/src/test/java/org/javarosa/xpath/expr/XPathFuncAsSomethingTest.java b/src/test/java/org/javarosa/xpath/expr/XPathFuncAsSomethingTest.java index 03aa535d1..5ba3f65e7 100644 --- a/src/test/java/org/javarosa/xpath/expr/XPathFuncAsSomethingTest.java +++ b/src/test/java/org/javarosa/xpath/expr/XPathFuncAsSomethingTest.java @@ -15,7 +15,7 @@ public class XPathFuncAsSomethingTest { @Test public void toLongHashHashesWell() { assertThat(toLongHash("Hello"), equalTo(1756278180214341157L)); - assertThat(toLongHash(""), equalTo(-2039914840885289964L)); + assertThat(toLongHash(""), equalTo(0L)); // the empty string would actually hash to -2039914840885289964L; but we've added this quirk for backward compatibility. } @Test