Skip to content
This repository has been archived by the owner on Apr 16, 2022. It is now read-only.

Forms with relative references cause pull and export to fail #767

Closed
batkinson opened this issue Jul 30, 2019 · 15 comments
Closed

Forms with relative references cause pull and export to fail #767

batkinson opened this issue Jul 30, 2019 · 15 comments

Comments

@batkinson
Copy link
Contributor

Software versions

Briefcase v1.13.1
openjdk version "1.8.0_212"
OpenJDK Runtime Environment (build 1.8.0_212-8u212-b03-0ubuntu1.18.04.1-b03)
OpenJDK 64-Bit Server VM (build 25.212-b03, mixed mode)
Linux Mint 19.1 Tessa
Pyxform v0.12.1

Problem description

Data managers using older versions of briefcase to pull and export form submissions are having problems when working with forms built with pyxform v0.12.1 or later. The issue appears to be an inability to handle relative references generated for repeats within a group, which was introduced in XLSForm/pyxform#187. Other forms appear to be unaffected.

Steps to reproduce the problem

  • Use a tool bindling pyxform v0.12.1 (XLS Offline, Online, Pyxform CLI) to generate a form using a repeat within a group, with a repeat_count based on another variable.
  • Deploy the form to an Aggregate-compliant server, use a recent version of Collect to download the form, create a form, submit
  • Use the Briefcase to pull forms and attempt an export

Expected behavior

The pull and export succeeds and produces the normal CSV output. Instead, it dies with:

Fetching form definition
resolving against briefcase form definitions
Error parsing form definition: org.opendatakit.aggregate.exception.ODKIncompleteSubmissionData: Javarosa failed to construct a FormDef. Is this an XForm definition?

Other information

The problem appears to be that the version of javarosa bundled in briefcase 1.31.1 does not play well with the relative references used within repeats like this:

<group ref="/data/individual/travelcount2weeks">
  <label ref="jr:itext('/data/individual/travelcount2weeks:label')" />
  <repeat jr:count=" ../travelcount2weeks_count" nodeset="/data/individual/travelcount2weeks">
...

This was generated by a change to the pyxform library (XLSForm/pyxform#187) for a question
using a variable reference as a repeat_count.

The relevant stack trace for the example above:

org.javarosa.xform.parse.XFormParseException: Binding path [null] not allowed with parent binding of [/data/individual/travelcount2weeks]
	at org.javarosa.core.model.FormDef.getAbsRef(FormDef.java:127)
	at org.javarosa.xform.parse.XFormParser.getAbsRef(XFormParser.java:1545)
	at org.javarosa.xform.parse.XFormParser.parseGroup(XFormParser.java:1473)
	at org.javarosa.xform.parse.XFormParser.access$300(XFormParser.java:119)
	at org.javarosa.xform.parse.XFormParser$1$8.handle(XFormParser.java:253)
	at org.javarosa.xform.parse.XFormParser.parseElement(XFormParser.java:539)
	at org.javarosa.xform.parse.XFormParser.parseGroup(XFormParser.java:1513)
	at org.javarosa.xform.parse.XFormParser.access$300(XFormParser.java:119)
	at org.javarosa.xform.parse.XFormParser$1$7.handle(XFormParser.java:247)
	at org.javarosa.xform.parse.XFormParser.parseElement(XFormParser.java:539)
	at org.javarosa.xform.parse.XFormParser.parseGroup(XFormParser.java:1513)
	at org.javarosa.xform.parse.XFormParser.access$300(XFormParser.java:119)
	at org.javarosa.xform.parse.XFormParser$1$8.handle(XFormParser.java:253)
	at org.javarosa.xform.parse.XFormParser.parseElement(XFormParser.java:539)
	at org.javarosa.xform.parse.XFormParser.parseGroup(XFormParser.java:1513)
	at org.javarosa.xform.parse.XFormParser.access$300(XFormParser.java:119)
	at org.javarosa.xform.parse.XFormParser$1$7.handle(XFormParser.java:247)
	at org.javarosa.xform.parse.XFormParser.parseElement(XFormParser.java:539)
	at org.javarosa.xform.parse.XFormParser.parseElement(XFormParser.java:548)
	at org.javarosa.xform.parse.XFormParser.parseElement(XFormParser.java:548)
	at org.javarosa.xform.parse.XFormParser.parseDoc(XFormParser.java:455)
	at org.javarosa.xform.parse.XFormParser.parse(XFormParser.java:366)
	at org.opendatakit.aggregate.parser.BaseFormParserForJavaRosa.<init>(BaseFormParserForJavaRosa.java:462)
	at org.opendatakit.briefcase.util.JavaRosaParserWrapper.<init>(JavaRosaParserWrapper.java:13)
	at org.opendatakit.briefcase.model.BriefcaseFormDefinition.resolveAgainstBriefcaseDefn(BriefcaseFormDefinition.java:97)
	at org.opendatakit.briefcase.model.BriefcaseFormDefinition.resolveAgainstBriefcaseDefn(BriefcaseFormDefinition.java:83)
	at org.opendatakit.briefcase.util.ServerFetcher.downloadFormAndSubmissionFiles(ServerFetcher.java:124)
	at org.opendatakit.briefcase.util.TransferFromServer.doAction(TransferFromServer.java:52)
	at org.opendatakit.briefcase.util.TransferAction$GatherTransferRunnable.run(TransferAction.java:82)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
12:39:28.422 [pool-2-thread-1] WARN org.opendatakit.briefcase.model.BriefcaseFormDefinition - bad form definition
org.opendatakit.aggregate.exception.ODKIncompleteSubmissionData: Javarosa failed to construct a FormDef. Is this an XForm definition?
	at org.opendatakit.aggregate.parser.BaseFormParserForJavaRosa.<init>(BaseFormParserForJavaRosa.java:464)
	at org.opendatakit.briefcase.util.JavaRosaParserWrapper.<init>(JavaRosaParserWrapper.java:13)
	at org.opendatakit.briefcase.model.BriefcaseFormDefinition.resolveAgainstBriefcaseDefn(BriefcaseFormDefinition.java:97)
	at org.opendatakit.briefcase.model.BriefcaseFormDefinition.resolveAgainstBriefcaseDefn(BriefcaseFormDefinition.java:83)
	at org.opendatakit.briefcase.util.ServerFetcher.downloadFormAndSubmissionFiles(ServerFetcher.java:124)
	at org.opendatakit.briefcase.util.TransferFromServer.doAction(TransferFromServer.java:52)
	at org.opendatakit.briefcase.util.TransferAction$GatherTransferRunnable.run(TransferAction.java:82)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
Caused by: org.javarosa.xform.parse.XFormParseException: Binding path [null] not allowed with parent binding of [/data/individual/travelcount2weeks]
	at org.javarosa.core.model.FormDef.getAbsRef(FormDef.java:127)
	at org.javarosa.xform.parse.XFormParser.getAbsRef(XFormParser.java:1545)
	at org.javarosa.xform.parse.XFormParser.parseGroup(XFormParser.java:1473)
	at org.javarosa.xform.parse.XFormParser.access$300(XFormParser.java:119)
	at org.javarosa.xform.parse.XFormParser$1$8.handle(XFormParser.java:253)
	at org.javarosa.xform.parse.XFormParser.parseElement(XFormParser.java:539)
	at org.javarosa.xform.parse.XFormParser.parseGroup(XFormParser.java:1513)
	at org.javarosa.xform.parse.XFormParser.access$300(XFormParser.java:119)
	at org.javarosa.xform.parse.XFormParser$1$7.handle(XFormParser.java:247)
	at org.javarosa.xform.parse.XFormParser.parseElement(XFormParser.java:539)
	at org.javarosa.xform.parse.XFormParser.parseGroup(XFormParser.java:1513)
	at org.javarosa.xform.parse.XFormParser.access$300(XFormParser.java:119)
	at org.javarosa.xform.parse.XFormParser$1$8.handle(XFormParser.java:253)
	at org.javarosa.xform.parse.XFormParser.parseElement(XFormParser.java:539)
	at org.javarosa.xform.parse.XFormParser.parseGroup(XFormParser.java:1513)
	at org.javarosa.xform.parse.XFormParser.access$300(XFormParser.java:119)
	at org.javarosa.xform.parse.XFormParser$1$7.handle(XFormParser.java:247)
	at org.javarosa.xform.parse.XFormParser.parseElement(XFormParser.java:539)
	at org.javarosa.xform.parse.XFormParser.parseElement(XFormParser.java:548)
	at org.javarosa.xform.parse.XFormParser.parseElement(XFormParser.java:548)
	at org.javarosa.xform.parse.XFormParser.parseDoc(XFormParser.java:455)
	at org.javarosa.xform.parse.XFormParser.parse(XFormParser.java:366)
	at org.opendatakit.aggregate.parser.BaseFormParserForJavaRosa.<init>(BaseFormParserForJavaRosa.java:462)
	... 9 common frames omitted
@batkinson batkinson changed the title Briefcase chokes on newer forms with relative references Forms with relative references cause pull and export to fail Jul 30, 2019
@ggalmazor
Copy link
Contributor

ggalmazor commented Aug 1, 2019

#770 will deal with this in v1.16.2

Closing, since newer versions should work

@lognaturel
Copy link
Member

Apologies for the re-opening and closing -- fat fingers.

This is surprising and unexpected to me. @ggalmazor, was it expected to you? Were you able to identify the JavaRosa change that fixed it? I can dig deeper if you haven't but I do think it would be helpful to understand. We discussed the pyxform change at very long length and did not expect this kind of incompatibility. We did have to make context fixes in JavaRosa for evaluating expressions in predicates but I'm not immediately seeing what that would have changed here.

@ggalmazor
Copy link
Contributor

ggalmazor commented Aug 3, 2019

I assume it's due to the JR version in Briefcase 1.13.1 doesn't have the changes we made to support the ‘..‘ relative notation, but now it looks like my heuristics can be wrong.

Even though the process for generating a form that reproduces the issue is described in the issue, I think I'd need to be provided a specific XML xform to tie it to a regression test in Briefcase. @batkinson, could you produce such form and attach it to the issue, please?

I could look into this first thing Monday, and we can discuss whether it's a JR or a Briefcase issue.

@ggalmazor ggalmazor reopened this Aug 3, 2019
@lognaturel
Copy link
Member

lognaturel commented Aug 4, 2019

I believe a form with a nested repeat and a value in the outer repeat that is used as a jr:count for the inner repeat would repro if converted with the latest pyxform: relative-jr-count Google Sheets, relative-jr-count.xml.txt.

@ggalmazor
Copy link
Contributor

Thanks, @lognaturel! I'll work on that tomorrow :)

@ggalmazor
Copy link
Contributor

ggalmazor commented Aug 5, 2019

Using the form Hélène has attached, I've verified that Briefcase v1.16.1 (JR 2.13) can pull and export this form using the Aggregate sandbox

I'll wait for @batkinson's ACK to close the issue.

@batkinson
Copy link
Contributor Author

Hi, I am happy to take a look though I am confused about what the problem is and what I should be confirming. Should I just evaluate whether v1.16.1 works for the case I reported?

@batkinson
Copy link
Contributor Author

batkinson commented Aug 5, 2019

Hi, I am happy to take a look though I am confused about what the problem is and what I should be confirming. Should I just evaluate whether v1.16.1 works for the case I reported?

So, for what it's worth I confirmed that I was able to pull the form and submissions successfully using v1.16.1 (after I merged in the cursor fix in #768 to fix submissions). Let me know if I missed the mark and/or can help with more information.

@ggalmazor
Copy link
Contributor

Thanks, Brent! I think that what matters is that we have confirmed separately that this issue has been addressed somewhere between Briefcase v1.13 and v1.16.

@lognaturel
Copy link
Member

Thanks, @batkinson. We'll dig deeper into why there as an incompatibility in the first place. Your description said "relative references generated for repeats within a group" but it'd be helpful to confirm that your form has a structure like mine with nested repeats. I don't think the problem would show up with a repeat nested in a group (not a repeat) but it would be helpful to confirm that we have that right.

@batkinson
Copy link
Contributor Author

it'd be helpful to confirm that your form has a structure like mine with nested repeats

I likely misidentified the portion of the structure that was triggering the behavior. I knew that the relative ref was failing at the line of code ref'ing its parent (debugging it) and I think I just referenced the form and climbed the inference ladder. It definitely was not a rigorous identification of the culprit since I was scrambling to fix the issues quickly.

I suspect it does in fact match yours and I will try and verify that now.

@batkinson
Copy link
Contributor Author

@lognaturel, it does appear that the form we experienced this with matches the general structure of yours. The nesting was not obvious since the outer repeat was quite long. I also forgot that repeats are rendered as a group and repeat element in xml.

    <group ref="/data/individual">
      <label ref="jr:itext('/data/individual:label')" />
      <repeat jr:count=" /data/individual_count " nodeset="/data/individual">
...
        <group ref="/data/individual/travelcount2weeks">
          <label ref="jr:itext('/data/individual/travelcount2weeks:label')" />
          <repeat jr:count=" ../travelcount2weeks_count " nodeset="/data/individual/travelcount2weeks">

@lognaturel
Copy link
Member

lognaturel commented Aug 6, 2019

I misread @ggalmazor's message here. He said my test form works on 1.16.1 which is the unpatched version. That means the nested repeat is not the culprit.

Any chance you could share your full form, @batkinson? If not, is this a form that you modify by hand? Is it possible that you define a select that pulls choices from a repeat? Or have a choice filter with a relative reference?

@batkinson
Copy link
Contributor Author

Any chance you could share your full form, @batkinson? If not, is this a form that you modify by hand?

I likely can't publish it, but I can share it privately if it helps. Just fair warning, it's a bit of a monster. To answer your question, yes, we do edit this manually.

Is it possible that you define a select that pulls choices from a repeat? Or have a choice filter with a relative reference?

Yes, this form is edited to add two selects that select from repeats. However, the reason I highlighted the question above is that it is the one causing the failure. The stack trace is from that question, which I captured in the debugger.

@lognaturel
Copy link
Member

lognaturel commented Aug 6, 2019

To be clear, I'm trying to hunt this down so we can evaluate the likely impact and see if there's any further action needed. Thanks for sharing your form, @batkinson. It turns out my little toy sample was sufficient to reproduce after all. I didn't realize this was a report on an old version and got confused between v1.16.1 and v1.13.1.

It looks like the fix at getodk/javarosa#335 and especially getodk/javarosa@507f56d#diff-96c7e556ff24192dbc4851b9b4410bbf fixed some relative path expressions we didn't realize were broken, and in particular, this jr:count case. We have tests to support the intended behavior.

I believe that users who use nested repeats, refer to values from the outer repeat in the inner repeat, and use tools that embed JR < 2.12.0 are affected in this way by the pyxform change. One thing we can do is make sure we have more testing around nested repeats. Those aren't very common and we don't have great coverage.

Other than that, the change to pyxform is a really good one that improves things overall, in particular making choice filters in repeats available to all users. The JavaRosa change was a bugfix. Some of the changes I described at #770 (comment) might help.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants