Exception thrown from jcrPropertyMap.containsKey("")

[Antonio Sanso <as...@adobe.com>]

Hi *,

lately updating the org.apache.sling.jcr.resource from version 2.0.10 to version  2.1 I hit a little issue.

The same call to the JcrPropertyMap#containsKey that used to work in the former version fails in the latest with stacktrace

java.lang.IllegalArgumentException: javax.jcr.RepositoryException: failed to resolve path  relative to node /test1340468918488
at org.apache.sling.jcr.resource.JcrPropertyMap.read(JcrPropertyMap.java:355)
at org.apache.sling.jcr.resource.JcrPropertyMap.get(JcrPropertyMap.java:162)
at org.apache.sling.jcr.resource.JcrPropertyMap.containsKey(JcrPropertyMap.java:171)
at org.apache.sling.jcr.resource.internal.JcrPropertyMapTest.testDefaultValue(JcrPropertyMapTest.java:206)
at org.apache.sling.jcr.resource.internal.JcrPropertyMapTest.testDefaultValue(JcrPropertyMapTest.java:134)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)


if containsKey  empty string argument e.g. jcrPropertyMap.containsKey("").

Now, java.lang.IllegalArgumentException is a runtime exception so fair enough, but since I have starting to grasp the essence of semantic versioning I wonder if this is "legitimate" for the consumer.

Here some related notes:

- the package version is kept the same  org.apache.sling.jcr.resource;version=2.1  in both bundle version
- the containsKey method is not part of any API (AFAIK) but is part of the  org.apache.sling.jcr.resource package and is public
- the exception is thrown from the read method that now is public and declares the runtime exception in the Java Doc (before the method was package protected)
- previously the Exception was "swallowed" in the read method

Again probably is not an issue at all but I am still "curious" to know WDYT :)

Regards

Antonio

Re: Exception thrown from jcrPropertyMap.containsKey("")

[Antonio Sanso <as...@adobe.com>]

Hi Carsten,

thanks for your answer.
I have opened a Jira for it [0].

see also some inline below

[0] https://issues.apache.org/jira/browse/SLING-2518

On Jun 24, 2012, at 11:26 AM, Carsten Ziegeler wrote:

> Hi Antonio,
> 
>> Here some related notes:
>> 
>> - the package version is kept the same  org.apache.sling.jcr.resource;version=2.1  in both bundle version
>> - the containsKey method is not part of any API (AFAIK) but is part of the  org.apache.sling.jcr.resource package and is public
> It's part of the Map API and ValueMap extends from Map.

ooops I have missed it :S

> 
>> - the exception is thrown from the read method that now is public and declares the runtime exception in the Java Doc (before the method was package protected)
> Hmm, as far as I see read() is still package protected.

correct again :) my workspace was out of date :)

> 
>> - previously the Exception was "swallowed" in the read method
> So what did containsKey("") return? Just false?

yes it use to return false

> 
>> 
>> Again probably is not an issue at all but I am still "curious" to know WDYT :)
> I think you're bringing up an important point. We shouldn't change the
> behaviour unless the version change is a major version change. So,
> even if we would have exported the api package as 2.2, the behaviour
> should still be the same.

I completely agree here ..... 

> The only exception would be if the previous
> behaviour is a bug :)
> So, in short, if containsKey("") worked before correctly (returning
> false), then it should work the same now.

+1

Regards

Antonio

> 
> Regards
> Carsten
> 
>> 
>> Regards
>> 
>> Antonio
> 
> 
> 
> -- 
> Carsten Ziegeler
> cziegeler@apache.org

Re: Exception thrown from jcrPropertyMap.containsKey("")

[Carsten Ziegeler <cz...@apache.org>]

Hi Antonio,

> Here some related notes:
>
> - the package version is kept the same  org.apache.sling.jcr.resource;version=2.1  in both bundle version
> - the containsKey method is not part of any API (AFAIK) but is part of the  org.apache.sling.jcr.resource package and is public
It's part of the Map API and ValueMap extends from Map.

> - the exception is thrown from the read method that now is public and declares the runtime exception in the Java Doc (before the method was package protected)
Hmm, as far as I see read() is still package protected.

> - previously the Exception was "swallowed" in the read method
So what did containsKey("") return? Just false?

>
> Again probably is not an issue at all but I am still "curious" to know WDYT :)
I think you're bringing up an important point. We shouldn't change the
behaviour unless the version change is a major version change. So,
even if we would have exported the api package as 2.2, the behaviour
should still be the same. The only exception would be if the previous
behaviour is a bug :)
So, in short, if containsKey("") worked before correctly (returning
false), then it should work the same now.

Regards
Carsten

>
> Regards
>
> Antonio



-- 
Carsten Ziegeler
cziegeler@apache.org