In audit log, only include position predicates for repeats

[lognaturel]

Closes #3126
Alternative to getodk/javarosa#469, builds on #3227

What has been done to verify that this works as intended?

Look at audit generated by nested-repeats-for-audit.xml.txt which has nesting of groups and repeats.

Why is this the best possible solution? Were any other approaches considered?

My first inclination was to modify the TreeReference.toString as done in getodk/javarosa#469. But as I was reviewing that solution, I saw a number of issues with it:

• clients could be depending on the current toString implementation
• TreeReferences can be used to represent references in secondary data instances which may have sequences of elements that are not backed by XForms repeats
• TreeReferences don't currently have a link to a specific FormDef

I didn't realize these subtle issues were there until I thought through it deeply.

My main goal in designing an alternative was not to increase coupling between TreeReference and FormDef. FormIndex actually already bridges the two and AuditEvent keeps track of FormIndex objects so it seems natural to use them.

FormIndex objects are only related to the primary instance and they only have a value other than -1 for instanceIndex at layers that represent repeats.

This solution walks the FormIndex linked list to get the position predicates while using the TreeReference field to get the names of the nodes. I think it's the simplest possible solution.

I think we'll want to move this into FormIndex but it will be easier to review and QA here for now. An unfortunate side effect of making getXPathPath a static method is that we end up needing real FormIndex objects for tests. I think it's still more sensible than making it an instance method in AuditEvent and we can either go back to mocking the toString or perhaps restructure the tests to make the cases clearer when the method moves. AuditEventSaveTaskTest.getTestFormIndex is certainly not good because it does a whole bunch of work that is not real Collect code.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Position predicates will only be added to nodes that are backed by repeats. This has the potential to be disruptive depending on the kind of analysis people are doing but it's important to make sure we're spec-compliant.

Do we need any specific form for testing your changes? If so, please attach one.

nested-repeats-for-audit.xml.txt and audit-group-multiplicity.xml.txt
test.xml.txt

Does this change require updates to documentation? If so, please file an issue here and include the link below.

getodk/xforms-spec#248

Before submitting this PR, please make sure you have:

• run ./gradlew checkAll and confirmed all checks still pass OR confirm CircleCI build passes and run ./gradlew connectedDebugAndroidTest locally.
• verified that any code or assets from external sources are properly credited in comments and/or in the about file.
• verified that any new UI elements use theme colors. UI Components Style guidelines

[lognaturel]

I hope @grzesiek2010 will be taking it easy and healing tomorrow and not doing too much code review! @kkrawczyk123, @mmarciniak90, would you consider testing without code review since this is so targeted and can easily be swapped out later if a better approach is found?

Alternately, I know there's a lot of JavaRosa context involved that you probably haven't looked at yet, @seadowg, so I understand if a deep review feels like it's hard to do but perhaps you could evaluate that and review if you feel like you can? I added a bunch of comments to FormIndex that hopefully help in making sense of this.

[seadowg]

@lognaturel I'm going to have to look at some docs around how this feature works but will have a go.

[yanokwa]

This PR is blocking the beta and hasn't been tested. I think it's important to get the beta out today, so I'm going to merge and ask the testers to try this after the code freeze. If there are issues, those will be filed and fixed before final release.

This is a change in process, but I've confirmed with @lognaturel that this PR is low-risk.

[mmarciniak90]

Tested with success

Verified on Android: 4.2, 4.4, 5.1, 6.0, 7.0

The noticed difference is highlighted in blue.
Groups do not have [...]. Repeat groups contain information in [...].

Form	1.22.2	Master 1.23
nested-repeats-for-audit.xml
test.xml

@opendatakit-bot unlabel "needs testing"
@opendatakit-bot label "behavior verified"

[mmarciniak90]

@opendatakit-bot unlabel "needs testing"
@opendatakit-bot label "behavior verified"

[lognaturel]

Thank you, @mmarciniak90!