Re: [jira] Created: (SLING-1420) Implement JCR 2.0 methods for FakeNode and FakeNodeType

[Bertrand Delacretaz <bd...@apache.org>]

Hi Carsten,

On Tue, Mar 2, 2010 at 12:05 PM, Carsten Ziegeler <cz...@apache.org> wrote:
> Carsten Ziegeler wrote:
>> Carsten Ziegeler wrote:
>> ...the fake node is used inside
>> the StarResource to determine the resource type...which seems a little
>> bit strange to me...
>>
> In fact this causes for trouble as we pass the fake node to possibly
> registered jcr resource type providers. As soon as they call any method
> other
> the two supported methods of the fake node, we might get NPE's and whatnot.
>
> So I strongly suggest we remove this...

This broke the espblog sample, and a number of other samples (in blog
posts, presentations etc) where we need to have a resource type for
URLs like /content/foo/*.html - I have reopened SLING-1420, can you
have a look?

-Bertrand

Re: [jira] Created: (SLING-1420) Implement JCR 2.0 methods for FakeNode and FakeNodeType

[Felix Meschberger <fm...@gmail.com>]

Hi,

On 04.03.2010 14:46, Bertrand Delacretaz wrote:
> Hi Carsten,
> 
> On Tue, Mar 2, 2010 at 12:05 PM, Carsten Ziegeler <cz...@apache.org> wrote:
>> Carsten Ziegeler wrote:
>>> Carsten Ziegeler wrote:
>>> ...the fake node is used inside
>>> the StarResource to determine the resource type...which seems a little
>>> bit strange to me...
>>>
>> In fact this causes for trouble as we pass the fake node to possibly
>> registered jcr resource type providers. As soon as they call any method
>> other
>> the two supported methods of the fake node, we might get NPE's and whatnot.
>>
>> So I strongly suggest we remove this...
> 
> This broke the espblog sample, and a number of other samples (in blog
> posts, presentations etc) where we need to have a resource type for
> URLs like /content/foo/*.html - I have reopened SLING-1420, can you
> have a look?

The only reason IIRC to have the FakeNode etc. was to be able to use the
JcrResourceTypeProvider to try to find a resource type for the resource
and that this one uses a JCR Node to find a type.

Maybe we have to fix this to take a Resource (the path provided by the
resource is all the PathBasedResourceTypeProvider cares about anyway).

WDYT ?

Regards
Felix

Re: [jira] Created: (SLING-1420) Implement JCR 2.0 methods for FakeNode and FakeNodeType

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

Bertrand Delacretaz  wrote
> That would not work for the StarResource as that does not provide a
> Node anymore, so we'd have a somewhat hidden incompatible change.
> 
> So I'd prefer removing the JcrResourceTypeProvider to make it clear
> that things have changed, as opposed to keeping it with slightly
> different semantics.
> 
I tend to agree here; I mean even if we follow Felix's suggestion and
make some kind of compatibility stuff, we will remove the old jcr
resource type provider sometime later on anyway. So we could just do it
right away :)

Carsten

-- 
Carsten Ziegeler
cziegeler@apache.org

Re: [jira] Created: (SLING-1420) Implement JCR 2.0 methods for FakeNode and FakeNodeType

[Bertrand Delacretaz <bd...@apache.org>]

Hi,

On Thu, Mar 4, 2010 at 3:22 PM, Felix Meschberger <fm...@gmail.com> wrote:
> On 04.03.2010 15:14, Bertrand Delacretaz wrote:
>>...so we'd move from a JcrResourceTypeProvider
>> interface to a new ResourceTypeProvider one, that maps a Resource to a
>> path?
>
> If you mean the PathBasedResourceTypeProvider would have to migrated to
> take a Resource (and thus implement the new ResourceTypeProvider
> interface), then yes.

Yes that's what I mean.

>>
>> This would be not be backwards-compatible, so I think we need a vote
>> to remove JcrResourceTypeProvider - but adapting existing
>> implementations would be easy.
>
> I don't think we have to immediately start voting, if everybody is ok
> with the change....

I think a vote is needed if we want to remove the
JcrResourceTypeProvider interface, as that's a backward-incompatible
change (read on as to why I prefer removing it).

>
> ...In addition, we can keep the JcrResourceTypeProvider interface and mark
> it deprecated (to be removed sometime in the future) and implement a
> compatibility layer, which calls into the old JcrResourceTypeProvider
> services if the resource adapts to a node....

That would not work for the StarResource as that does not provide a
Node anymore, so we'd have a somewhat hidden incompatible change.

So I'd prefer removing the JcrResourceTypeProvider to make it clear
that things have changed, as opposed to keeping it with slightly
different semantics.

-Bertrand

Re: [jira] Created: (SLING-1420) Implement JCR 2.0 methods for FakeNode and FakeNodeType

[Felix Meschberger <fm...@gmail.com>]

Hi,

On 04.03.2010 15:14, Bertrand Delacretaz wrote:
> On Thu, Mar 4, 2010 at 2:56 PM, Carsten Ziegeler <cz...@apache.org> wrote:
>> ...I think we should go with Felix suggestion (which I think we already
>> suggested some months ago) and provide a better interface for the
>> resource type provider...
> 
> Agree, that makes sense, so we'd move from a JcrResourceTypeProvider
> interface to a new ResourceTypeProvider one, that maps a Resource to a
> path?

If you mean the PathBasedResourceTypeProvider would have to migrated to
take a Resource (and thus implement the new ResourceTypeProvider
interface), then yes.

> 
> This would be not be backwards-compatible, so I think we need a vote
> to remove JcrResourceTypeProvider - but adapting existing
> implementations would be easy.

I don't think we have to immediately start voting, if everybody is ok
with the change.

In addition, we can keep the JcrResourceTypeProvider interface and mark
it deprecated (to be removed sometime in the future) and implement a
compatibility layer, which calls into the old JcrResourceTypeProvider
services if the resource adapts to a node.

Regards
Felix

Re: [jira] Created: (SLING-1420) Implement JCR 2.0 methods for FakeNode and FakeNodeType

[Bertrand Delacretaz <bd...@apache.org>]

On Thu, Mar 4, 2010 at 2:56 PM, Carsten Ziegeler <cz...@apache.org> wrote:
> ...I think we should go with Felix suggestion (which I think we already
> suggested some months ago) and provide a better interface for the
> resource type provider...

Agree, that makes sense, so we'd move from a JcrResourceTypeProvider
interface to a new ResourceTypeProvider one, that maps a Resource to a
path?

This would be not be backwards-compatible, so I think we need a vote
to remove JcrResourceTypeProvider - but adapting existing
implementations would be easy.

-Bertrand

Re: [jira] Created: (SLING-1420) Implement JCR 2.0 methods for FakeNode and FakeNodeType

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

Bertrand Delacretaz  wrote
> Hi Carsten,
> 
> On Tue, Mar 2, 2010 at 12:05 PM, Carsten Ziegeler <cz...@apache.org> wrote:
>> Carsten Ziegeler wrote:
>>> Carsten Ziegeler wrote:
>>> ...the fake node is used inside
>>> the StarResource to determine the resource type...which seems a little
>>> bit strange to me...
>>>
>> In fact this causes for trouble as we pass the fake node to possibly
>> registered jcr resource type providers. As soon as they call any method
>> other
>> the two supported methods of the fake node, we might get NPE's and whatnot.
>>
>> So I strongly suggest we remove this...
> 
> This broke the espblog sample, and a number of other samples (in blog
> posts, presentations etc) where we need to have a resource type for
> URLs like /content/foo/*.html - I have reopened SLING-1420, can you
> have a look?
> 
Actually I don't know what the best solution is - however the former
solution is way too fragile - passing in a fake node into the
JcrResourceTypeProvider is bound to fail. As soon as the provider calls
any other method than getPath() it fails - and that's imho not acceptable.

So I think we should go with Felix suggestion (which I think we already
suggested some months ago) and provide a better interface for the
resource type provider.

Carsten
-- 
Carsten Ziegeler
cziegeler@apache.org