You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Carsten Ziegeler <cz...@apache.org> on 2018/08/19 16:18:42 UTC
[API] Resource traversal vs Resource Resolver traversal
Hi,
we've currently two APIs for resource traversal, the Resource interface
allows to get the parent, list children etc., and the same methods are
part of the resource resolver interface. In fact we have a third variant
using ResourceUtil.
Now actually, all of these should return the same results. As the
resource resolver implementation is the source of truth, the resource
implementations should delegate to the resource resolver for all these
methods.
Unfortunately, not all of them do. Will the AbstractResource class does
so, sub classes sometimes override the traversal methods and take
shortcuts. However by taking these shortcuts, the result might differ
from the resource resolver equivalents. For example resource decorators
might not be called, children from different providers might not be
merged etc.
To fix this problem we should probably declare those methods in
AbstractResource as final. This forces all implementations to use those
implementations. We would also need to fix the NonExistingResource
implementation in the API package, but that's easy to do.
However, this most likely will break some of the existing
implementations. So we would need to update all resource provider
implementations as well. It's not a big deal as we only have a couple of
them, but still it's not nice.
On the other hand, simply trusting that every implementation does it
right, does not work as we see now here and there.
Thoughts?
Regards
Carsten
--
Carsten Ziegeler
Adobe Research Switzerland
cziegeler@apache.org
Re: [API] Resource traversal vs Resource Resolver traversal
Posted by Carsten Ziegeler <cz...@apache.org>.
In fact it doesn't look too bad, from our three main resources (bundle
resource, servlet resource and jcr resource) only bundle resource seems
to be doing it wrong (which I believe is the original problem of
SLING-7833).
Regards
Carsten
Carsten Ziegeler wrote
> Hi,
>
> we've currently two APIs for resource traversal, the Resource interface
> allows to get the parent, list children etc., and the same methods are
> part of the resource resolver interface. In fact we have a third variant
> using ResourceUtil.
>
> Now actually, all of these should return the same results. As the
> resource resolver implementation is the source of truth, the resource
> implementations should delegate to the resource resolver for all these
> methods.
>
> Unfortunately, not all of them do. Will the AbstractResource class does
> so, sub classes sometimes override the traversal methods and take
> shortcuts. However by taking these shortcuts, the result might differ
> from the resource resolver equivalents. For example resource decorators
> might not be called, children from different providers might not be
> merged etc.
>
> To fix this problem we should probably declare those methods in
> AbstractResource as final. This forces all implementations to use those
> implementations. We would also need to fix the NonExistingResource
> implementation in the API package, but that's easy to do.
>
> However, this most likely will break some of the existing
> implementations. So we would need to update all resource provider
> implementations as well. It's not a big deal as we only have a couple of
> them, but still it's not nice.
>
> On the other hand, simply trusting that every implementation does it
> right, does not work as we see now here and there.
>
> Thoughts?
>
> Regards
> Carsten
>
--
Carsten Ziegeler
Adobe Research Switzerland
cziegeler@apache.org
Re: [API] Resource traversal vs Resource Resolver traversal
Posted by Robert Munteanu <ro...@apache.org>.
On Mon, 2018-08-20 at 07:57 +0000, Stefan Seifert wrote:
> with Java 8 we can use default implementations and declare them as
> final as well.
At least with Eclipse I'm getting
"Illegal modifier for the interface method bar; only public, abstract,
default, static and strictfp are permitted"
This is supported by Brian Goetz's explanation at [1].
[1]:
https://stackoverflow.com/questions/23453287/why-is-final-not-allowed-in-java-8-interface-methods
Re: [API] Resource traversal vs Resource Resolver traversal
Posted by Bertrand Delacretaz <bd...@apache.org>.
On Mon, Aug 20, 2018 at 10:46 AM Robert Munteanu <ro...@apache.org> wrote:
> ...Additionally we could provide a ResourceProvider 'test harness' that
> checks that the implementation behaves correctly in terms of the
> expected invariants...
+1
-Bertrand
Re: [API] Resource traversal vs Resource Resolver traversal
Posted by Jason E Bailey <ja...@24601.org>.
Additionally, it may be helpful to move the implementation from the AbstractResource to being a default method for the Resource itself. You have an option then to remove the other implementations, and if you are implementing the interface then IDE's like eclipse will skip default methods.
--
Jason
On Mon, Aug 20, 2018, at 4:46 AM, Robert Munteanu wrote:
> On Mon, 2018-08-20 at 10:16 +0200, Carsten Ziegeler wrote:
> > Right, while we encourage that implementations use AbstractResource
> > we
> > can't force them to use this either.
> >
> > I would like to avoid a major version increase, we could (not saying
> > we
> > should) declare these as final while only increasing the minor
> > version.
> > Clearly this might break some implementations.
> >
> > The only other option is that we check all implementations, fix them
> > and
> > hope that they stay like this from now on...
> >
>
> Additionally we could provide a ResourceProvider 'test harness' that
> checks that the implementation behaves correctly in terms of the
> expected invariants.
>
> That may help identify incorrect behaviours. Of course, it relies on
> the implementors to run it and it's much weaker compared to a compile-
> time check. But it seems that a 100% fool-proof compile-time check is
> not possible ATM.
>
> Thanks,
>
> Robert
>
> > Regards
> >
> > Carsten
> >
> >
> > Stefan Seifert wrote
> > > i'm in general +1 on declaring these "delating" methods in
> > > AbstractResource as final, but have some remarks:
> > >
> > > 1. doing this would fource us to raise the package version to
> > > 3.0.0, breaking all bundles relying on 2.x unless we provide and
> > > implementation for the old package version as well.
> > >
> > > 2. it's possible to implement the Resource interface without using
> > > AbstractResource - in this case it still could be possible to
> > > provide wrong/derivating implementations. so maybe a better
> > > solution would be to move the implementations to the interface
> > > itself - with Java 8 we can use default implementations and declare
> > > them as final as well. but i assume this as well will lead to a
> > > major version increase.
> > >
> > > stefan
> > >
> > >
> > > > -----Original Message-----
> > > > From: Carsten Ziegeler [mailto:cziegeler@apache.org]
> > > > Sent: Sunday, August 19, 2018 6:19 PM
> > > > To: Sling Developers
> > > > Subject: [API] Resource traversal vs Resource Resolver traversal
> > > >
> > > > Hi,
> > > >
> > > > we've currently two APIs for resource traversal, the Resource
> > > > interface
> > > > allows to get the parent, list children etc., and the same
> > > > methods are
> > > > part of the resource resolver interface. In fact we have a third
> > > > variant
> > > > using ResourceUtil.
> > > >
> > > > Now actually, all of these should return the same results. As the
> > > > resource resolver implementation is the source of truth, the
> > > > resource
> > > > implementations should delegate to the resource resolver for all
> > > > these
> > > > methods.
> > > >
> > > > Unfortunately, not all of them do. Will the AbstractResource
> > > > class does
> > > > so, sub classes sometimes override the traversal methods and take
> > > > shortcuts. However by taking these shortcuts, the result might
> > > > differ
> > > > from the resource resolver equivalents. For example resource
> > > > decorators
> > > > might not be called, children from different providers might not
> > > > be
> > > > merged etc.
> > > >
> > > > To fix this problem we should probably declare those methods in
> > > > AbstractResource as final. This forces all implementations to use
> > > > those
> > > > implementations. We would also need to fix the
> > > > NonExistingResource
> > > > implementation in the API package, but that's easy to do.
> > > >
> > > > However, this most likely will break some of the existing
> > > > implementations. So we would need to update all resource provider
> > > > implementations as well. It's not a big deal as we only have a
> > > > couple of
> > > > them, but still it's not nice.
> > > >
> > > > On the other hand, simply trusting that every implementation does
> > > > it
> > > > right, does not work as we see now here and there.
> > > >
> > > > Thoughts?
> > > >
> > > > Regards
> > > > Carsten
> > > > --
> > > > Carsten Ziegeler
> > > > Adobe Research Switzerland
> > > > cziegeler@apache.org
>
>
Re: [API] Resource traversal vs Resource Resolver traversal
Posted by Robert Munteanu <ro...@apache.org>.
On Mon, 2018-08-20 at 10:16 +0200, Carsten Ziegeler wrote:
> Right, while we encourage that implementations use AbstractResource
> we
> can't force them to use this either.
>
> I would like to avoid a major version increase, we could (not saying
> we
> should) declare these as final while only increasing the minor
> version.
> Clearly this might break some implementations.
>
> The only other option is that we check all implementations, fix them
> and
> hope that they stay like this from now on...
>
Additionally we could provide a ResourceProvider 'test harness' that
checks that the implementation behaves correctly in terms of the
expected invariants.
That may help identify incorrect behaviours. Of course, it relies on
the implementors to run it and it's much weaker compared to a compile-
time check. But it seems that a 100% fool-proof compile-time check is
not possible ATM.
Thanks,
Robert
> Regards
>
> Carsten
>
>
> Stefan Seifert wrote
> > i'm in general +1 on declaring these "delating" methods in
> > AbstractResource as final, but have some remarks:
> >
> > 1. doing this would fource us to raise the package version to
> > 3.0.0, breaking all bundles relying on 2.x unless we provide and
> > implementation for the old package version as well.
> >
> > 2. it's possible to implement the Resource interface without using
> > AbstractResource - in this case it still could be possible to
> > provide wrong/derivating implementations. so maybe a better
> > solution would be to move the implementations to the interface
> > itself - with Java 8 we can use default implementations and declare
> > them as final as well. but i assume this as well will lead to a
> > major version increase.
> >
> > stefan
> >
> >
> > > -----Original Message-----
> > > From: Carsten Ziegeler [mailto:cziegeler@apache.org]
> > > Sent: Sunday, August 19, 2018 6:19 PM
> > > To: Sling Developers
> > > Subject: [API] Resource traversal vs Resource Resolver traversal
> > >
> > > Hi,
> > >
> > > we've currently two APIs for resource traversal, the Resource
> > > interface
> > > allows to get the parent, list children etc., and the same
> > > methods are
> > > part of the resource resolver interface. In fact we have a third
> > > variant
> > > using ResourceUtil.
> > >
> > > Now actually, all of these should return the same results. As the
> > > resource resolver implementation is the source of truth, the
> > > resource
> > > implementations should delegate to the resource resolver for all
> > > these
> > > methods.
> > >
> > > Unfortunately, not all of them do. Will the AbstractResource
> > > class does
> > > so, sub classes sometimes override the traversal methods and take
> > > shortcuts. However by taking these shortcuts, the result might
> > > differ
> > > from the resource resolver equivalents. For example resource
> > > decorators
> > > might not be called, children from different providers might not
> > > be
> > > merged etc.
> > >
> > > To fix this problem we should probably declare those methods in
> > > AbstractResource as final. This forces all implementations to use
> > > those
> > > implementations. We would also need to fix the
> > > NonExistingResource
> > > implementation in the API package, but that's easy to do.
> > >
> > > However, this most likely will break some of the existing
> > > implementations. So we would need to update all resource provider
> > > implementations as well. It's not a big deal as we only have a
> > > couple of
> > > them, but still it's not nice.
> > >
> > > On the other hand, simply trusting that every implementation does
> > > it
> > > right, does not work as we see now here and there.
> > >
> > > Thoughts?
> > >
> > > Regards
> > > Carsten
> > > --
> > > Carsten Ziegeler
> > > Adobe Research Switzerland
> > > cziegeler@apache.org
Re: [API] Resource traversal vs Resource Resolver traversal
Posted by Carsten Ziegeler <cz...@apache.org>.
Right, while we encourage that implementations use AbstractResource we
can't force them to use this either.
I would like to avoid a major version increase, we could (not saying we
should) declare these as final while only increasing the minor version.
Clearly this might break some implementations.
The only other option is that we check all implementations, fix them and
hope that they stay like this from now on...
Regards
Carsten
Stefan Seifert wrote
> i'm in general +1 on declaring these "delating" methods in AbstractResource as final, but have some remarks:
>
> 1. doing this would fource us to raise the package version to 3.0.0, breaking all bundles relying on 2.x unless we provide and implementation for the old package version as well.
>
> 2. it's possible to implement the Resource interface without using AbstractResource - in this case it still could be possible to provide wrong/derivating implementations. so maybe a better solution would be to move the implementations to the interface itself - with Java 8 we can use default implementations and declare them as final as well. but i assume this as well will lead to a major version increase.
>
> stefan
>
>
>> -----Original Message-----
>> From: Carsten Ziegeler [mailto:cziegeler@apache.org]
>> Sent: Sunday, August 19, 2018 6:19 PM
>> To: Sling Developers
>> Subject: [API] Resource traversal vs Resource Resolver traversal
>>
>> Hi,
>>
>> we've currently two APIs for resource traversal, the Resource interface
>> allows to get the parent, list children etc., and the same methods are
>> part of the resource resolver interface. In fact we have a third variant
>> using ResourceUtil.
>>
>> Now actually, all of these should return the same results. As the
>> resource resolver implementation is the source of truth, the resource
>> implementations should delegate to the resource resolver for all these
>> methods.
>>
>> Unfortunately, not all of them do. Will the AbstractResource class does
>> so, sub classes sometimes override the traversal methods and take
>> shortcuts. However by taking these shortcuts, the result might differ
>>from the resource resolver equivalents. For example resource decorators
>> might not be called, children from different providers might not be
>> merged etc.
>>
>> To fix this problem we should probably declare those methods in
>> AbstractResource as final. This forces all implementations to use those
>> implementations. We would also need to fix the NonExistingResource
>> implementation in the API package, but that's easy to do.
>>
>> However, this most likely will break some of the existing
>> implementations. So we would need to update all resource provider
>> implementations as well. It's not a big deal as we only have a couple of
>> them, but still it's not nice.
>>
>> On the other hand, simply trusting that every implementation does it
>> right, does not work as we see now here and there.
>>
>> Thoughts?
>>
>> Regards
>> Carsten
>> --
>> Carsten Ziegeler
>> Adobe Research Switzerland
>> cziegeler@apache.org
>
--
Carsten Ziegeler
Adobe Research Switzerland
cziegeler@apache.org
RE: [API] Resource traversal vs Resource Resolver traversal
Posted by Stefan Seifert <ss...@pro-vision.de>.
i'm in general +1 on declaring these "delating" methods in AbstractResource as final, but have some remarks:
1. doing this would fource us to raise the package version to 3.0.0, breaking all bundles relying on 2.x unless we provide and implementation for the old package version as well.
2. it's possible to implement the Resource interface without using AbstractResource - in this case it still could be possible to provide wrong/derivating implementations. so maybe a better solution would be to move the implementations to the interface itself - with Java 8 we can use default implementations and declare them as final as well. but i assume this as well will lead to a major version increase.
stefan
>-----Original Message-----
>From: Carsten Ziegeler [mailto:cziegeler@apache.org]
>Sent: Sunday, August 19, 2018 6:19 PM
>To: Sling Developers
>Subject: [API] Resource traversal vs Resource Resolver traversal
>
>Hi,
>
>we've currently two APIs for resource traversal, the Resource interface
>allows to get the parent, list children etc., and the same methods are
>part of the resource resolver interface. In fact we have a third variant
>using ResourceUtil.
>
>Now actually, all of these should return the same results. As the
>resource resolver implementation is the source of truth, the resource
>implementations should delegate to the resource resolver for all these
>methods.
>
>Unfortunately, not all of them do. Will the AbstractResource class does
>so, sub classes sometimes override the traversal methods and take
>shortcuts. However by taking these shortcuts, the result might differ
>from the resource resolver equivalents. For example resource decorators
>might not be called, children from different providers might not be
>merged etc.
>
>To fix this problem we should probably declare those methods in
>AbstractResource as final. This forces all implementations to use those
>implementations. We would also need to fix the NonExistingResource
>implementation in the API package, but that's easy to do.
>
>However, this most likely will break some of the existing
>implementations. So we would need to update all resource provider
>implementations as well. It's not a big deal as we only have a couple of
>them, but still it's not nice.
>
>On the other hand, simply trusting that every implementation does it
>right, does not work as we see now here and there.
>
>Thoughts?
>
>Regards
>Carsten
>--
>Carsten Ziegeler
>Adobe Research Switzerland
>cziegeler@apache.org