Include multiplicities only for repeatable groups in TreeReference.toString() method

[grzesiek2010]

Closes getodk/collect#3126

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

I tested the attached form with changes on the Collect side.

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

I decided to the safer option. I don't change setting multiplicities (what is used in multiple classes) I just include them in TreeReference.toString() if a level represents a repeatable group.

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?

As I said above I decided to use the safer solution so I don't think it's risky. Generally, I changed that toString method only so testing it would be fine.

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

I used this one:
test.xml.txt
but any form with regular groups and repeatable groups is fine.

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

No, I thought that we have some samples in the doc that we would need to fix but looks as if I was wrong.

[lognaturel]

Thanks for looking into this tricky issue, @grzesiek2010! I knew it was subtle but there was more here than I had anticipated.

At first glance, I agreed that this was a safe, straightforward solution. But thinking more through it, I think it has a number of issues. The biggest is that this implicitly ties TreeReferences to a specific form's primary instance. That is, TreeReferences are currently just convenient representations of XPath paths that are portable -- nothing ties them to a specific form. But by introducing isRepeatable, now they only make sense in the context of a specific form definition.

Related to that, TreeReferences are not only used to represent paths in the primary instance (which defines the form structure) but also to represent paths in secondary data instances which define select itemsets or just documents to look values up in. Those secondary instances generally do have sequences (e.g. multiple item blocks) but they're not backed by a repeat. This would mean those sequences' paths would be represented without position predicates.

Finally, even though it's unlikely, a client could be relying on the existing structure of the toString's output.

An additional note for the future -- if we did want to go this route of adding a field to TreeReferenceLevel, we'd have to make sure that field was included in readExternal and writeExternal so that it would be serialized.

I've proposed an alternative that I hope you will like at getodk/collect#3231 that doesn't require new state or coupling.