Hi Chris,
(1) What do you mean by "changes in any other attribute, bound or
otherwise, not explicitly allowed." I think you can relax this and avoid
adding more complexity in FormParserForJavarosa.
Is this done in an abundance of caution, or do you have specific examples
in mind where this is necessary? As long as odk:length is not allowed to
change, we should be OK to allow other attribute changes. Attributes are
generally ignored -- ODK Aggregate only examines a few attributes on the
top-level tag in a submission, so allowing changes to attributes in the
instance definition, body or bind statements should be OK. Were you
thinking of specific examples where this might be a concern?
(2) We can also allow changes between type="string" and type="select1"
(DATATYPE_CHOICE) (either from string to select1 or from select1 to
string). Note that type="select" (DATATYPE_CHOICE_LIST) cannot be
similarly exchanged as that is a multiple-select field.
(3) We can also allow the title to change. If it does, the update would be
applied in the FormInfo table.
If you can make the above revisions, it is probably easiest to send me a
zip directly ( mitchellsundt@gmail.com <javascript:_e({}, 'cvml',
'mitchellsundt@gmail.com');> ) of your changed files and test forms.
I don't think it is feasible for you complete the changes needed to update
the various data structures yourself. I think there is some missing code
and perhaps some assumptions later on in the processing that will need to
be examined and adjusted by me or Waylon.
For other future changes, if you were able to produce a full working
patch, the mechanism would be to create your own server-side clone off of
ODK Aggregate, submit your files to that, then share that clone with Waylon
and me. We'd then fold those changes into the main tree, or grant you
write access to the main tree and allow you to merge your changes back into
it.
Mitch
On Wed, May 16, 2012 at 8:43 AM, Christopher Robert < chrislrobert@gmail.com> wrote:
Hi Mitch, Waylon,
Okay, so I've implemented and tested a maximally-conservative comparison
function. Accordingly, I updated Form.isSameForm(...) to
call FormParserForJavaRosa.compareXml(this.getFormXml(cc).getBytes("UTF-8"),xmlBytes)
and, based on the result, call xform.setValueFromByteArray(...) to update
the form's XML. Basically, my comparison function indicates whether the XML
is entirely identical and, if not, whether changes are too major to allow
an update. The following bound attribute changes are explicitly allowed on
existing fields only:
relevant
constraint
readonly
required
calculate
appearance
jr:constraintmessage
jr:preload
jr:preloadparams
My impression is that these attributes only affect rendering in Collect.
Also, any and all changes are allowed in the body portion of the XML.
The following changes are explicitly disallowed:
changes in the id
changes in the title
changes in the public key
changes in fields or grouping of fields (i.e., any change to the instance
structure)
The following changes are implicitly disallowed:
changes in type
changes in odk:length
changes in any other attribute, bound or otherwise, not explicitly allowed
This latter set of properties -- the implicit disallowing of most changes
-- was somewhat difficult to implement. This is because bound attributes
are not stored in TreeElements' attribute lists, as one might expect. In
fact, they are not stored at all. Rather, parsed bindings lead JavaRosa to
set TreeElement.required, TreeElement.relevant, etc. I could have easily
walked through the parsed TreeElements and compared such things, but then
when new bindings/features were added the code would default to accepting
changes to new attributes rather than default to rejecting them. This would
not have been desirable. Thus, I performed some very minor surgery
on FormParserForJavaRosa to have it record all raw bindings as they come
through parseBind(...). Then, in addition to walking through TreeElements
to discover structural instance changes and changes to directly-specified
attributes (like id), I also walk through all of the raw bindings. Only
changes to attributes listed in FormParserForJavaRosa.NonSchemaAttributes
are allowed; the whole approach errs on the side of over-rejecting rather
than over-accepting.
I now have a few questions:
-
To carry this through, I need xform.setValueFromByteArray(...) (or some
cousin) to update the actual XML when I have decided that such an update
would be okay. Any ideas?
-
In total I have proposed updates to 4 source files (though the changes
to Form.java are still highly provisional), plus 13 test files (a base
case, an all-allowable-updates case, and then 11 cases that deserve
rejection). How should I communicate these? (I'm new to Mercurial and the
whole open-source thing more generally.)
Please just let me know how best to move forward.
Thanks,
Chris
On Friday, May 11, 2012, Mitch S wrote:
Yes, basically, within Form, you need to replace the comparison of the md5
hash of the already-present form definition file and the new file here:
http://code.google.com/p/opendatakit/source/browse/src/main/java/org/opendatakit/aggregate/form/Form.java?repo=aggregate#646
With a more embellished comparison of the two javarosa-parsed form
definition files.
The incoming form definition file is in xmlBytes
The existing form definition file is in byte Form.getFormXml(cc)
Javarosa is now capable of having two active form models, so you should be
able to parse the two files and stepwise compare the element trees.
The FormParserForJavarosa.constructDataModel() steps through t