Setting existing property from single value to multi-value

[Julian Sedding <js...@gmail.com>]

Hi all

Let's assume we have a Node N of primary type "nt:unstructured" with
property P that has a String value "foo".

Now when we try to change the value of P to a String[] value of
["foo", "bar"] a ValueException is thrown.

This behaviour was introduced in OAK-273. Unfortunately the ticket
does not give any explanation why this behaviour should be desired.

Reading the java-docs for Node#setProperty(String name, Value value)
and Node#setProperty(String name, Value[] values) I got the impression
that no ValueFormatException should be thrown in that case.

The following are paragraphs 4-6 from the java-doc, the ones I
consider relevant to this issue:

(4) "The property type of the property will be that specified by the
node type of this node. If the property type of one or more of the
supplied Value objects is different from that required, then a
best-effort conversion is attempted, according to an
implemention-dependent definition of "best effort". If the conversion
fails, a ValueFormatException is thrown."

(5) "If the property is not multi-valued then a ValueFormatException
is also thrown. If another error occurs, a RepositoryException is
thrown."

(6) "If the node type of this node does not indicate a specific
property type, then the property type of the supplied Value objects is
used and if the property already exists it assumes both the new values
and the new property type."

The way I read this, paragraph (5) applies to properties where the
property type is specified by the node type. The reason is that (a) it
follows directly after paragraph (4), which is about node type defined
properties and (b) the word "also" in the phrase "... a
ValueFormatException is _also_ thrown" seems to refer back to (4).

Therefore paragraph (6) would be the one relevant to properties that
have no node type defined property type. And that is very clear that
the property should be changed to the new values and property type.

Does anyone have a good explanation why my reading is incorrect? Or
should I create a JIRA ticket to fix this?

Regards
Julian

Re: Setting existing property from single value to multi-value

[Julian Sedding <js...@gmail.com>]

Thanks Julian for looking onto it!

Deleting the property first is indeed my workaround for the issue and it works

It's not a big deal (clearly, as it didn't pop up for 6 years or so),
but the behaviour was unexpected and seems unnecessarily restrictive
to me. It caused a minor production issue for a client in a very
generic code-path that hits Oak via Sling's ModifiableValueMap. Given
all layers involved I was surprised that I ended up in Oak ;)

If my question helps make Oak a little bit better, that's great. If we
can clarify the question and document it in the list's archive that's
also great.

Regards
Julian

On Wed, Jul 24, 2019 at 6:52 AM Julian Reschke <ju...@gmx.de> wrote:
>
> On 24.07.2019 05:55, Julian Reschke wrote:
> > On 23.07.2019 23:57, Julian Sedding wrote:
> >> Hi all
> >>
> >> Let's assume we have a Node N of primary type "nt:unstructured" with
> >> property P that has a String value "foo".
> >>
> >> Now when we try to change the value of P to a String[] value of
> >> ["foo", "bar"] a ValueException is thrown.
> >>
> >> This behaviour was introduced in OAK-273. Unfortunately the ticket
> >> does not give any explanation why this behaviour should be desired.
> >> ...
> >
> > I was curious and looked, and, surprise, I raised this issue back then.
> >
> > I would assume that this came up while running the TCK. That is, if we
> > undo this change, we are likely to see TCK tests failing.
> >
> > (not sure, but worth trying)
> >
> > Now that doesn't necessarily mean that the TCK is correct - I'll need
> > more time to re-read things.
> >
> > Best regards, Julian
>
> FWIW, did you try to delete the property first?
>
> Best regards, Julian

Re: Setting existing property from single value to multi-value

[Julian Reschke <ju...@gmx.de>]

On 24.07.2019 05:55, Julian Reschke wrote:
> On 23.07.2019 23:57, Julian Sedding wrote:
>> Hi all
>>
>> Let's assume we have a Node N of primary type "nt:unstructured" with
>> property P that has a String value "foo".
>>
>> Now when we try to change the value of P to a String[] value of
>> ["foo", "bar"] a ValueException is thrown.
>>
>> This behaviour was introduced in OAK-273. Unfortunately the ticket
>> does not give any explanation why this behaviour should be desired.
>> ...
>
> I was curious and looked, and, surprise, I raised this issue back then.
>
> I would assume that this came up while running the TCK. That is, if we
> undo this change, we are likely to see TCK tests failing.
>
> (not sure, but worth trying)
>
> Now that doesn't necessarily mean that the TCK is correct - I'll need
> more time to re-read things.
>
> Best regards, Julian

FWIW, did you try to delete the property first?

Best regards, Julian

Re: Setting existing property from single value to multi-value

[Julian Reschke <ju...@gmx.de>]

On 23.07.2019 23:57, Julian Sedding wrote:
> Hi all
>
> Let's assume we have a Node N of primary type "nt:unstructured" with
> property P that has a String value "foo".
>
> Now when we try to change the value of P to a String[] value of
> ["foo", "bar"] a ValueException is thrown.
>
> This behaviour was introduced in OAK-273. Unfortunately the ticket
> does not give any explanation why this behaviour should be desired.
> ...

I was curious and looked, and, surprise, I raised this issue back then.

I would assume that this came up while running the TCK. That is, if we
undo this change, we are likely to see TCK tests failing.

(not sure, but worth trying)

Now that doesn't necessarily mean that the TCK is correct - I'll need
more time to re-read things.

Best regards, Julian