You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Felix Meschberger <fm...@gmail.com> on 2009/04/03 08:20:09 UTC
[POLL] What to do with ResourceResolver.resolve(String path) (was:
[jira] Commented: (SLING-909) JcrResourceResolver2.resolve returns null for
non-existing resources while api doc says otherwise)
Hi all,
In SLING-909 [1] and SLING-752 [2] an API inconsistency has been uncovered.
(1) Intent:
ResourceResolver.getResource methods return null if the
requested resource cannot be found
ResourceResolver.resolve methods return NonExistingResource
if the requested resource cannot be found
(2) Reality:
ResourceResolver.resolve(String path) returns null if the
requested resource cannot be found
All other methods behave as intended.
To fix the "wrong" resolve(String) method now would mean to break
backwards compatibility with the released version of both the Sling API
and the actual implementation in the jcr.resource bundle.
In the mail thread accompanying SLING-752 [3] we decided to value
backwards compatibility higher than consistency and correctness.
Therefore we kept the resolve(String) method in the wrong implemenation,
though the API document currently documents the "correct" behaviour,
which is not what has been decided.
I am not sure, what the correct behaviour really is, therefore, I would
like to repoen this discussion again (for the last time, I promise).
Do we want to keep the ResourceResolver.resolve(String path) method
return null if the reource cannot be found (backwards compatible case)
or do we want to have the method return NonExistingResource (consistency
case).
I myself am undecided, though I tend to favour the consistency case.
WDYT?
Regards
Felix
[1] https://issues.apache.org/jira/browse/SLING-909
[2] https://issues.apache.org/jira/browse/SLING-752
[3] http://markmail.org/message/ulqwzjilp6prq75m
Felix Meschberger (JIRA) schrieb:
> [ https://issues.apache.org/jira/browse/SLING-909?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12695253#action_12695253 ]
>
> Felix Meschberger commented on SLING-909:
> -----------------------------------------
>
> Absolutely true, but ... As the mail thread accompanying SLING-752 indicates, we had the inconsistency in the first Sling release. And yes, this was a bug and yes it would be better to fix it.
>
> But, at that time (sorry to repeat myself), we decided to stay backwards compatible with the released API/implementation.
>
>> JcrResourceResolver2.resolve returns null for non-existing resources while api doc says otherwise
>> -------------------------------------------------------------------------------------------------
>>
>> Key: SLING-909
>> URL: https://issues.apache.org/jira/browse/SLING-909
>> Project: Sling
>> Issue Type: Bug
>> Components: JCR Resource
>> Reporter: Alexander Klimetschek
>>
>> (This is a basically a re-opening of SLING-752, because I don't have rights to reopen that bug).
>> In current trunk, although to be said to be fixed with SLING-752, the API doc of ResourceResolver.resolve(String) says:
>> * @return The {@link Resource} addressed by the <code>absPath</code> or a
>> * {@link NonExistingResource} if no such resource can be resolved.
>> The implementation of this method in JcrResourceResolver2 however will return null (it calls resolveInternal() with requireResource=false).
>> I have a use case where I don't have a request object (running in an event job) but still like to get a NonExistingResource, not null. Therefore I'd suggest to change JcrResourceResolver2.resolve() to not return null (and keep the javadoc).
>
Re: [POLL] What to do with ResourceResolver.resolve(String path)
(was: [jira] Commented: (SLING-909) JcrResourceResolver2.resolve returns null
for non-existing resources while api doc says otherwise)
Posted by Alexander Klimetschek <ak...@day.com>.
On Fri, Apr 3, 2009 at 8:20 AM, Felix Meschberger <fm...@gmail.com> wrote:
> Do we want to keep the ResourceResolver.resolve(String path) method
> return null if the reource cannot be found (backwards compatible case)
> or do we want to have the method return NonExistingResource (consistency
> case).
As mentioned in the issue already, I would favour to correctly
implement the API docs in the JcrResourceResolver2, ie. return
NonExistingResource. It should not be an issue of backwards
compatability as the JRR2 is not part of an official sling release
yet.
Regards,
Alex
--
Alexander Klimetschek
alexander.klimetschek@day.com
Re: [POLL] What to do with ResourceResolver.resolve(String path)
(was: [jira] Commented: (SLING-909) JcrResourceResolver2.resolve returns
null for non-existing resources while api doc says otherwise)
Posted by Felix Meschberger <fm...@gmail.com>.
Hi,
Carsten Ziegeler schrieb:
> Vidar Ramdal wrote:
>>>> On Fri, Apr 3, 2009 at 1:34 PM, Vidar Ramdal <vi...@idium.no> wrote:
>>>>> If I can disregard the points of backwards compatibility and
>>>>> documentation for a second, I'd say: I've allways regarded
>>>>> NonExistingResource as something of a hack, so I wouldn't mind trying
>>>>> to avoid it as much as possible. A null returned when a resource does
>>>>> not exist, is certainly easier to grasp for Sling newbies.
>>> Alexander Klimetschek schrieb:
>>>> Yes, but that's why there are the "getResource()" methods (returning
>>>> null) [...]
>> On Fri, Apr 3, 2009 at 2:06 PM, Felix Meschberger <fm...@gmail.com> wrote:
>>> In addition to what Alex said: resolve() is really used by the Sling
>>> engine to resolve the URL to a resource. Not returning null here makes
>>> the whole processing a lot easier and simpler. [...]
>> OK, you convinced me :)
>>
> I'm in favour of the consistency solution as well; resolve() should
> always return a resource.
Ok, we don't have any voices rised in favore of backwards compatibility
(except for my slight bias expressed initially).
Also, the longer I think of it, the longer I am convinced that in this
special case, it might be better to be consistent than to be backwards
compatible for the following reason: The difference between resolve and
getResource does not seem to be well understood and the resolve method
may not be that widely used.
Therefore, I think, I am going to be consistent and overthrow the
earlier decision to remain backwards compatible.
Thanks to all the participants.
Regards
Felix
Re: [POLL] What to do with ResourceResolver.resolve(String path)
(was: [jira] Commented: (SLING-909) JcrResourceResolver2.resolve returns
null for non-existing resources while api doc says otherwise)
Posted by Carsten Ziegeler <cz...@apache.org>.
Vidar Ramdal wrote:
>>> On Fri, Apr 3, 2009 at 1:34 PM, Vidar Ramdal <vi...@idium.no> wrote:
>>>> If I can disregard the points of backwards compatibility and
>>>> documentation for a second, I'd say: I've allways regarded
>>>> NonExistingResource as something of a hack, so I wouldn't mind trying
>>>> to avoid it as much as possible. A null returned when a resource does
>>>> not exist, is certainly easier to grasp for Sling newbies.
>
>> Alexander Klimetschek schrieb:
>>> Yes, but that's why there are the "getResource()" methods (returning
>>> null) [...]
>
> On Fri, Apr 3, 2009 at 2:06 PM, Felix Meschberger <fm...@gmail.com> wrote:
>> In addition to what Alex said: resolve() is really used by the Sling
>> engine to resolve the URL to a resource. Not returning null here makes
>> the whole processing a lot easier and simpler. [...]
>
> OK, you convinced me :)
>
I'm in favour of the consistency solution as well; resolve() should
always return a resource.
Carsten
--
Carsten Ziegeler
cziegeler@apache.org
Re: [POLL] What to do with ResourceResolver.resolve(String path)
(was: [jira] Commented: (SLING-909) JcrResourceResolver2.resolve returns null
for non-existing resources while api doc says otherwise)
Posted by Vidar Ramdal <vi...@idium.no>.
>> On Fri, Apr 3, 2009 at 1:34 PM, Vidar Ramdal <vi...@idium.no> wrote:
>>> If I can disregard the points of backwards compatibility and
>>> documentation for a second, I'd say: I've allways regarded
>>> NonExistingResource as something of a hack, so I wouldn't mind trying
>>> to avoid it as much as possible. A null returned when a resource does
>>> not exist, is certainly easier to grasp for Sling newbies.
> Alexander Klimetschek schrieb:
>> Yes, but that's why there are the "getResource()" methods (returning
>> null) [...]
On Fri, Apr 3, 2009 at 2:06 PM, Felix Meschberger <fm...@gmail.com> wrote:
> In addition to what Alex said: resolve() is really used by the Sling
> engine to resolve the URL to a resource. Not returning null here makes
> the whole processing a lot easier and simpler. [...]
OK, you convinced me :)
--
Vidar S. Ramdal <vi...@idium.no> - http://www.idium.no
Akersgata 16, N-0158 Oslo, Norway
+47 21 531941, ext 2070
Re: [POLL] What to do with ResourceResolver.resolve(String path)
(was: [jira] Commented: (SLING-909) JcrResourceResolver2.resolve returns
null for non-existing resources while api doc says otherwise)
Posted by Felix Meschberger <fm...@gmail.com>.
Hi,
Alexander Klimetschek schrieb:
> On Fri, Apr 3, 2009 at 1:34 PM, Vidar Ramdal <vi...@idium.no> wrote:
>> If I can disregard the points of backwards compatibility and
>> documentation for a second, I'd say: I've allways regarded
>> NonExistingResource as something of a hack, so I wouldn't mind trying
>> to avoid it as much as possible. A null returned when a resource does
>> not exist, is certainly easier to grasp for Sling newbies.
>
> Yes, but that's why there are the "getResource()" methods (returning
> null) and the "advanced" resolve() methods that do all kind of fancy
> stuff (synthetic resources, virtual path mappings, etc.), which is
> required for request processing and other cases. It would only be
> consistent if all resolve methods would return nonexistent and all
> getResource would return null.
In addition to what Alex said: resolve() is really used by the Sling
engine to resolve the URL to a resource. Not returning null here makes
the whole processing a lot easier and simpler.
In the very early days, we in fact return null on resolve() and had all
kind of if (resource == null) checks spread out in the system. Now we
say, resolve() always (except for the inconsistent resolve(String)
method) returns a non-null result and we reduce the special case
checking to a very limited number of places. We can even handle the case
of a non-existing resource with a request script.
Of course we could have created the NonExistingResource outside of the
resolver, but we chose not to do that. I think just because it makes it
so much easier to use (once you know it, agreed).
Regards
Felix
>
> Regards,
> Alex
>
Re: [POLL] What to do with ResourceResolver.resolve(String path)
(was: [jira] Commented: (SLING-909) JcrResourceResolver2.resolve returns null
for non-existing resources while api doc says otherwise)
Posted by Alexander Klimetschek <ak...@day.com>.
On Fri, Apr 3, 2009 at 1:34 PM, Vidar Ramdal <vi...@idium.no> wrote:
> If I can disregard the points of backwards compatibility and
> documentation for a second, I'd say: I've allways regarded
> NonExistingResource as something of a hack, so I wouldn't mind trying
> to avoid it as much as possible. A null returned when a resource does
> not exist, is certainly easier to grasp for Sling newbies.
Yes, but that's why there are the "getResource()" methods (returning
null) and the "advanced" resolve() methods that do all kind of fancy
stuff (synthetic resources, virtual path mappings, etc.), which is
required for request processing and other cases. It would only be
consistent if all resolve methods would return nonexistent and all
getResource would return null.
Regards,
Alex
--
Alexander Klimetschek
alexander.klimetschek@day.com
Re: [POLL] What to do with ResourceResolver.resolve(String path)
(was: [jira] Commented: (SLING-909) JcrResourceResolver2.resolve returns null
for non-existing resources while api doc says otherwise)
Posted by Vidar Ramdal <vi...@idium.no>.
On Fri, Apr 3, 2009 at 8:20 AM, Felix Meschberger <fm...@gmail.com> wrote:
> [...]
> Do we want to keep the ResourceResolver.resolve(String path) method
> return null if the reource cannot be found (backwards compatible case)
> or do we want to have the method return NonExistingResource (consistency
> case).
If I can disregard the points of backwards compatibility and
documentation for a second, I'd say: I've allways regarded
NonExistingResource as something of a hack, so I wouldn't mind trying
to avoid it as much as possible. A null returned when a resource does
not exist, is certainly easier to grasp for Sling newbies.
--
Vidar S. Ramdal <vi...@idium.no> - http://www.idium.no
Akersgata 16, N-0158 Oslo, Norway
+47 21 531941, ext 2070