javarosa icon indicating copy to clipboard operation
javarosa copied to clipboard

Add predicate support for treating randomize() as nodeset

Open tiritea opened this issue 5 years ago • 3 comments

copied from slack discussion with @lognaturel : The randomize(...) function is (also) supposed to return a nodeset: https://docs.opendatakit.org/form-operators-functions/#randomize So if this is ok: instance('countries')/root/item[position() = 1] then presumably this is also ok: randomize(instance('countries'))/root/item[position() = 1]

This works under Enketo, but not Collect.

I don't think the issue is with randomize but rather with incomplete predicate support in JavaRosa

This would allow exploiting randomize() for other purposes, specifically randomizing form questions.

Software versions

JavaRosa v1.x.x, Java v1.x.x, operating system

Problem description

Steps to reproduce the problem

Expected behavior

Other information

tiritea avatar Apr 16 '19 01:04 tiritea

Thanks! I suspect the [position() = 1] is what's being choked on and XPathPathExpr is probably the first place to look.

lognaturel avatar Apr 16 '19 03:04 lognaturel

So, I've tried this in a test and it turns out that [position() = 1] works as expected. The error I'm actually getting is that XPathPathExpr doesn't know what to do with randomize(), as expected.

This is the test I have:

@Test
public void name() throws IOException {
    Scenario scenario = Scenario.init("Some form", html(head(
        title("Some form"),
        model(
            mainInstance(t("data id=\"some-form\"",
                t("first-choice"),
                t("first-choice-randomized")
            )),
            secondaryInstance("choices",
                t("item", t("value", "a")),
                t("item", t("value", "b")),
                t("item", t("value", "c")),
                t("item", t("value", "d"))
            ),
            bind("/data/first-choice").type("string").calculate("instance('choices')/root/item[position() = 1]/value"),
            bind("/data/first-choice-randomized").type("string").calculate("randomize(instance('choices'), 42)/root/item[position() = 1]/value")
        )
    )));

    assertThat(scenario.answerOf("/data/first-choice"), is(stringAnswer("a")));
    assertThat(scenario.answerOf("/data/first-choice-randomized"), is(not(stringAnswer("a"))));
}

Then I changed XPathPathExpr to parse randomize() just like instance() but the references don't get the shuffled nodeset, obviously. This was just to remove the error and further think about it.

I would like to run an idea to solve this through you: we could add new secondary instances on the run with the shuffled contents of the secondary instances used in the randomize(instance('foo')) combo. Then, in XPathPathExpr, we bind refs to the new shuffled instances, and everything else should fall in place after that.

We would create a shuffled instance for all unseeded randomize() calls and specific shuffled instances for randomize() calls that have seeds. randomize(instance('foo')) would produce a foo_randomized instance, and randomize(instance('foo'), 42) would produce a foo_randomized_42 instance. If the instance already exists, we do nothing. This means that all unseeded randomize() calls will be sharing the same shuffled instance (this is a problem, maybe?), and all seeded randomize() calls share the same shuffled instance if they share the same seed.

This dynamic shuffled instance creation could happen anytime before the initialization of the DAG, and it could also be used to simplify the code of the randomize() function implementation in XPathFuncExpr (it would become a simple function that basically translates randomize(instance('foo'), 42) to instance('foo_randomized_42')).

What do you think about this?

Check this at https://github.com/ggalmazor/javarosa/commits/issue_424_randomize_returns_nodesets

ggalmazor avatar Mar 05 '20 12:03 ggalmazor

@ggalmazor and I had discussed a long time ago and were not yet ready to take any action. But yes, the fundamental issue is that randomize only has special support in itemset nodeset declarations.

Looking at the original issue again, though, randomize(instance('countries'))/root/item[position() = 1] doesn't look right. I would expect that to always return the same, first item from the countries instance because instance('countries') is a nodeset with a single element (instance('countries')/root). Or am I getting that wrong? I would expect something like randomize(instance('countries')/root/item)[position() = 1]. That means to take the nodeset which contains all items, randomize their order, and pick the first one out of the resulting new order.

lognaturel avatar Jan 08 '21 21:01 lognaturel