OF-1515: Migrate Private XML Storage to PEP

[guusdk]

Note that this PR is based on the code that is in a different PR: #1049

[sco0ter]

Instead of writing at least 22x duplicate code: booleanValue = ((value != null ? value : "1")); "1".equals(booleanValue) || "true".equalsIgnoreCase(booleanValue), can you put that code in FormField, please?

It's a central piece of code which is not only used by PubSub, but also by MUC and other code, therefore it should be handled centrally by DataForms/FormField.

I suggest having a method FormField#getValueAsBoolean(variable) or some static utility method FormField#parseBoolean(String)

[dwd]

I'm not sure the models of XEP-0049 will support this. I need to review this in more detail, but loosely, I'm aware that <iq><query xmlns='..'><elementone xmlns='1'/><elementtwo xmlns='1'/></query></iq> is legal, but it's never been completely clear what that actually means, and what a corresponding get of <elementone xmlns='1'/> would then return. I'll try to review our current behaviour and the new code within a few days.

[guusdk]

@dwd As far as I can see, that does not apply to this PR. We're having <iq><query xmlns='..'><something><elementone/><elementtwo/></something</query></iq>, not <iq><query xmlns='..'><elementone/><elementtwo/></query></iq>

For what it's worth, I believe that the corresponding 'get' would not be of a child element of another child element, but for the 'outer' child element, thus returning all.

[guusdk]

(note that @sco0ter's concerns were addressed in #1049, after which this branch was rebased on that. It makes the code changes he's referring disappear from this PR, which could cause confusion).

[dwd]

Right, so here's my concern with this - and note, this might not be a concern in practise.

XEP-0049 allows multiple elements within a particular namespace to be stored. So it'll allow multiple <storage xmlns='a'/> elements, for example, or a <storage xmlns='a'/> and a <configuration xmlns='a'/> to both be stored. Asking for either gets you both, which is remarkably odd behaviour - but does, thankfully, mean that Private XML can be keyed exclusively on namespace. The way XEP-0049 is written is, however, at odds with how it is used.

XEP-0048, for instance, says explicitly to use only one element. I have no idea if any server supports using more than one. If everything only uses one element, then this PR will probably work in this case.

Second problem: this patch assumes that all existing XEP-0049 cases can be recast, verbatim, as XEP-0223 cases. This is true for XEP-0048, but there is no certainty that this would always be the case, despite the fact it is likely.

Third problem: @iNPUTmice pointed out on the standards list that clients which attempt to synchronize XEP-0049 and XEP-0223 stored might get thoroughly confused if this is done automatically.

I don't think any of these problems are definitely showstoppers, but I'm aware that as things stand, there is no way to undo this action if there does turn out to be problems. If we hit operational problems with this patch, what would we do?

[guusdk]

The implementation of XEP-0049 stores only one element (and disregards others). For example, in this example, only the 'dave' element would be stored, and not the 'guus' element. That's probably a bug (I particularly dislike the silent ignore), but given that I'm only finding out about this now, I don't think it affects to many people.

I'd be fine by modifying the XEP-0049 implementation to explicitly return errors if more than one element is added. This should retain compatibility with XEP-0048. This should tackle your #1 and #2 concerns, I believe.

<iq type="set">
  <query xmlns="jabber:iq:private">
    <foo xmlns="bar">
      <dave/>
    </foo>
    <foo xmlns="bar">
      <guus/
    </foo>
  </query>
</iq>

My Google foo isn't strong enough to find the issue raised by Daniel. Could you kindly provide a pointer?