Add stubs so external secondary instances can pass validation

[lognaturel]

Closes #65

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

I validated the Nigeria Wards form from the issue and verified that it now passes validation.

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

It's the simplest fix I could think of that doesn't require JavaRosa changes.

An alternative I considered was avoiding the items verification in stepThroughEntireForm for external itemsets. Unfortunately I can't see any way to do that because whether the itemset is internal or external is lost on parse. We'd have to do something like add a field to ItemsetBinding. I also considered removing those checks altogether but I think they're useful for more typical form designs.

This approach narrowly supports the typical case generated by pyxform where the itemset value ref is name and the label ref is label. It does not support arbitrary node references for those. Currently, the only way to have different references for those is to author forms by hand and that's very uncommon so I think it's a reasonable fix for now. We may need to revisit later but until external secondary instances are in broader use, I don't think it's worth it.

Are there any risks to merging this code? If so, what are they?

With this change, any file reference in a form will get interpreted as referring to the new fake-itemset.xml file. Maybe that could mess up forms with media references somehow? I don't think those are touched at all, though, so overall this should be very low risk.

[ggalmazor]

Hi, @lognaturel!

Something you wrote in the description caught my eye. In Briefcase, we have a way of knowing when an Itemset is external to avoid populating it (we only want to populate dynamic itemsets coming from internal instances).

This is the gist of it:

boolean isExternal = false;          
ItemsetBinding itemsetBinding = control.getDynamicChoices();
if (itemsetBinding != null) {
  String instanceName = itemsetBinding.nodesetRef.getInstanceName();
  DataInstance secondaryInstance = formDef.getNonMainInstance(instanceName);
  isExternal = secondaryInstance != null && !(secondaryInstance instanceof ExternalDataInstance);
}

(original code at FormDefinition.getFormControls())

It's a little bit hacky because it involves an instanceof test, but it works.

[lognaturel]

@ggalmazor I think we can use your approach. We'd need to change verifyItemsetBindings in JavaRosa and stepThroughEntireForm here. I'm now thinking that might be better because I believe the JavaRosa fix would be sufficient to also fix Aggregate.

It's a little bit hacky because it involves an instanceof test, but it works.

Agreed. If we have to modify JavaRosa anyway, perhaps we could change that? What do you think about adding something like a getSource method to FormInstance that returns an enum with values for internal, external XML and external CSV? I'm not totally sure we need to differentiate between XML And CSV so it could be boolean? Or is there a better approach you'd suggest?

[ggalmazor]

Awesome!

Let's keep track of the work with corresponding issues :)

https://github.com/opendatakit/javarosa/issues/443

Could you create an issue with the change we need to make in verifyItemsetBindings?

[lognaturel]

After another round thinking through this, I've gone back to thinking it would be helpful to start with this Validate solution. The issue that initially prompted this is that pyxform adds dummy nodes to external secondary instance declarations to pacify Validate (see https://github.com/XLSForm/pyxform/issues/286). The dummy nodes mean the forms don't actually work in clients. Enketo solves this problem by identifying and removing them. I think it would be valuable to clean pyxform up once and for all and this Validate fix enables that. Then pyxform will generate forms with external secondary instances that actually work.

Then we can think through what we want the JavaRosa fix to actually do. That is, what errors or warnings should be shown to users of Collect if the external secondary instance file is missing? For Aggregate, presumably we'd want to accept the form without requiring the secondary instance files as is done with other media.

[ggalmazor]

Agreed, @lognaturel!

I think the StubReferenceManager is promising towards a generic solution. Instead of using a static file on the resources folder, we could try to reverse engineer the structure of an item in the <body> and produce a matching XML to provide a generic solution.

[lognaturel]

Thanks, @ggalmazor!

Instead of using a static file on the resources folder, we could try to reverse engineer the structure of an item in the and produce a matching XML to provide a generic solution.

That's a good idea that would work for Validate or Aggregate. I'm not sure it's what we would want to do for Collect. I've tried to capture my current understanding of the broader issue at https://github.com/opendatakit/javarosa/issues/444