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 2013/03/01 17:48:30 UTC

[RT] ResourceMetadata

Hi,

while looking into some issues, I realized that ResourceMetadata is
not only extending a HashMap (which makes handling easier), but we
have absolutely no information if this map can be changed by client
code or is a read-only map.

I think we should add this to the documentation and make this
read-only. We could either just document it or add a "make read-only"
method to ResourceMetadata which is called by the resource resolver
before the resource object is returned to the client code.

But I think we should not allow client code to change/add/remove to
ResourceMetadata.

WDYT

Regards
Carsten
-- 
Carsten Ziegeler
cziegeler@apache.org

Re: [RT] ResourceMetadata

Posted by Carsten Ziegeler <cz...@apache.org>.
I've created SLING-2780 and made an implementation (well I made three
different ones to see how they work).

If no one complains, I'll set this to resolved.

Thanks
Carsten

2013/3/6 Carsten Ziegeler <cz...@apache.org>:
> I've asked @work, but it seems no one is using ResourceMetadata for writing.
> As no one else complained so far (which is a good sign), I think we
> should proceed.
>
> Carsten
>
> 2013/3/5 Felix Meschberger <fm...@adobe.com>:
>> Hi,
>>
>> As much as I would like to make ResourceMetadata read-only, I am not really comfortable ...
>>
>> How about running this by our users list and see, whether there is some feedback ?
>>
>> Regards
>> Felix
>>
>> Am 04.03.2013 um 14:28 schrieb Carsten Ziegeler:
>>
>>> 2013/3/4 Felix Meschberger <fm...@adobe.com>:
>>>> Hi,
>>>>
>>>> I have been looking into this some time ago. And I am also somewhat unhappy with the read/write nature of the ResourceMetadata.
>>>>
>>>> While I think in general we can make the ResourceMetadata read-only, we have a problem with code like:
>>>>
>>>>> resource.getResourceMetadata().setResolutionPathInfo(rpi);
>>>>
>>>> which is in ResourceResolverImpl.resolveInternal (line 805). This would break.
>>>
>>> Yepp.
>>>
>>>>
>>>> We could see, whether it would be possible to make the ResourceMetadata read-only after resource resolution has completed, e.g. ResourceMetadata.lock(). But I am not really convinced.
>>>
>>> Yes, that's why I suggest that the resource resolver calls this method
>>> before returning the resource object.
>>>
>>>>
>>>> I am also not sure, what client application we would break with this change ?
>>> We will never know - so far I don't know any code putting stuff in
>>> here outside of the resource resolver.
>>>
>>>>
>>>> What happens in the Sling Launchpad Testing build if this would be read-only ?
>>>
>>> I couldn't find a reference to metadata in that module
>>>
>>> Carsten
>>>
>>>>
>>>> Regards
>>>> Felix
>>>>
>>>>
>>>> Am 01.03.2013 um 17:48 schrieb Carsten Ziegeler:
>>>>
>>>>> Hi,
>>>>>
>>>>> while looking into some issues, I realized that ResourceMetadata is
>>>>> not only extending a HashMap (which makes handling easier), but we
>>>>> have absolutely no information if this map can be changed by client
>>>>> code or is a read-only map.
>>>>
>>>>>
>>>>> I think we should add this to the documentation and make this
>>>>> read-only. We could either just document it or add a "make read-only"
>>>>> method to ResourceMetadata which is called by the resource resolver
>>>>> before the resource object is returned to the client code.
>>>>>
>>>>> But I think we should not allow client code to change/add/remove to
>>>>> ResourceMetadata.
>>>>>
>>>>> WDYT
>>>>>
>>>>> Regards
>>>>> Carsten
>>>>> --
>>>>> Carsten Ziegeler
>>>>> cziegeler@apache.org
>>>>
>>>>
>>>> --
>>>> Felix Meschberger | Principal Scientist | Adobe
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>> --
>>> Carsten Ziegeler
>>> cziegeler@apache.org
>>
>>
>> --
>> Felix Meschberger | Principal Scientist | Adobe
>>
>>
>>
>>
>>
>>
>>
>
>
>
> --
> Carsten Ziegeler
> cziegeler@apache.org



-- 
Carsten Ziegeler
cziegeler@apache.org

Re: [RT] ResourceMetadata

Posted by Carsten Ziegeler <cz...@apache.org>.
I've asked @work, but it seems no one is using ResourceMetadata for writing.
As no one else complained so far (which is a good sign), I think we
should proceed.

Carsten

2013/3/5 Felix Meschberger <fm...@adobe.com>:
> Hi,
>
> As much as I would like to make ResourceMetadata read-only, I am not really comfortable ...
>
> How about running this by our users list and see, whether there is some feedback ?
>
> Regards
> Felix
>
> Am 04.03.2013 um 14:28 schrieb Carsten Ziegeler:
>
>> 2013/3/4 Felix Meschberger <fm...@adobe.com>:
>>> Hi,
>>>
>>> I have been looking into this some time ago. And I am also somewhat unhappy with the read/write nature of the ResourceMetadata.
>>>
>>> While I think in general we can make the ResourceMetadata read-only, we have a problem with code like:
>>>
>>>> resource.getResourceMetadata().setResolutionPathInfo(rpi);
>>>
>>> which is in ResourceResolverImpl.resolveInternal (line 805). This would break.
>>
>> Yepp.
>>
>>>
>>> We could see, whether it would be possible to make the ResourceMetadata read-only after resource resolution has completed, e.g. ResourceMetadata.lock(). But I am not really convinced.
>>
>> Yes, that's why I suggest that the resource resolver calls this method
>> before returning the resource object.
>>
>>>
>>> I am also not sure, what client application we would break with this change ?
>> We will never know - so far I don't know any code putting stuff in
>> here outside of the resource resolver.
>>
>>>
>>> What happens in the Sling Launchpad Testing build if this would be read-only ?
>>
>> I couldn't find a reference to metadata in that module
>>
>> Carsten
>>
>>>
>>> Regards
>>> Felix
>>>
>>>
>>> Am 01.03.2013 um 17:48 schrieb Carsten Ziegeler:
>>>
>>>> Hi,
>>>>
>>>> while looking into some issues, I realized that ResourceMetadata is
>>>> not only extending a HashMap (which makes handling easier), but we
>>>> have absolutely no information if this map can be changed by client
>>>> code or is a read-only map.
>>>
>>>>
>>>> I think we should add this to the documentation and make this
>>>> read-only. We could either just document it or add a "make read-only"
>>>> method to ResourceMetadata which is called by the resource resolver
>>>> before the resource object is returned to the client code.
>>>>
>>>> But I think we should not allow client code to change/add/remove to
>>>> ResourceMetadata.
>>>>
>>>> WDYT
>>>>
>>>> Regards
>>>> Carsten
>>>> --
>>>> Carsten Ziegeler
>>>> cziegeler@apache.org
>>>
>>>
>>> --
>>> Felix Meschberger | Principal Scientist | Adobe
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>
>>
>>
>> --
>> Carsten Ziegeler
>> cziegeler@apache.org
>
>
> --
> Felix Meschberger | Principal Scientist | Adobe
>
>
>
>
>
>
>



-- 
Carsten Ziegeler
cziegeler@apache.org

Re: [RT] ResourceMetadata

Posted by Felix Meschberger <fm...@adobe.com>.
Hi,

As much as I would like to make ResourceMetadata read-only, I am not really comfortable ...

How about running this by our users list and see, whether there is some feedback ?

Regards
Felix

Am 04.03.2013 um 14:28 schrieb Carsten Ziegeler:

> 2013/3/4 Felix Meschberger <fm...@adobe.com>:
>> Hi,
>> 
>> I have been looking into this some time ago. And I am also somewhat unhappy with the read/write nature of the ResourceMetadata.
>> 
>> While I think in general we can make the ResourceMetadata read-only, we have a problem with code like:
>> 
>>> resource.getResourceMetadata().setResolutionPathInfo(rpi);
>> 
>> which is in ResourceResolverImpl.resolveInternal (line 805). This would break.
> 
> Yepp.
> 
>> 
>> We could see, whether it would be possible to make the ResourceMetadata read-only after resource resolution has completed, e.g. ResourceMetadata.lock(). But I am not really convinced.
> 
> Yes, that's why I suggest that the resource resolver calls this method
> before returning the resource object.
> 
>> 
>> I am also not sure, what client application we would break with this change ?
> We will never know - so far I don't know any code putting stuff in
> here outside of the resource resolver.
> 
>> 
>> What happens in the Sling Launchpad Testing build if this would be read-only ?
> 
> I couldn't find a reference to metadata in that module
> 
> Carsten
> 
>> 
>> Regards
>> Felix
>> 
>> 
>> Am 01.03.2013 um 17:48 schrieb Carsten Ziegeler:
>> 
>>> Hi,
>>> 
>>> while looking into some issues, I realized that ResourceMetadata is
>>> not only extending a HashMap (which makes handling easier), but we
>>> have absolutely no information if this map can be changed by client
>>> code or is a read-only map.
>> 
>>> 
>>> I think we should add this to the documentation and make this
>>> read-only. We could either just document it or add a "make read-only"
>>> method to ResourceMetadata which is called by the resource resolver
>>> before the resource object is returned to the client code.
>>> 
>>> But I think we should not allow client code to change/add/remove to
>>> ResourceMetadata.
>>> 
>>> WDYT
>>> 
>>> Regards
>>> Carsten
>>> --
>>> Carsten Ziegeler
>>> cziegeler@apache.org
>> 
>> 
>> --
>> Felix Meschberger | Principal Scientist | Adobe
>> 
>> 
>> 
>> 
>> 
>> 
>> 
> 
> 
> 
> -- 
> Carsten Ziegeler
> cziegeler@apache.org


--
Felix Meschberger | Principal Scientist | Adobe








Re: [RT] ResourceMetadata

Posted by Carsten Ziegeler <cz...@apache.org>.
2013/3/4 Felix Meschberger <fm...@adobe.com>:
> Hi,
>
> I have been looking into this some time ago. And I am also somewhat unhappy with the read/write nature of the ResourceMetadata.
>
> While I think in general we can make the ResourceMetadata read-only, we have a problem with code like:
>
>> resource.getResourceMetadata().setResolutionPathInfo(rpi);
>
> which is in ResourceResolverImpl.resolveInternal (line 805). This would break.

Yepp.

>
> We could see, whether it would be possible to make the ResourceMetadata read-only after resource resolution has completed, e.g. ResourceMetadata.lock(). But I am not really convinced.

Yes, that's why I suggest that the resource resolver calls this method
before returning the resource object.

>
> I am also not sure, what client application we would break with this change ?
We will never know - so far I don't know any code putting stuff in
here outside of the resource resolver.

>
> What happens in the Sling Launchpad Testing build if this would be read-only ?

I couldn't find a reference to metadata in that module

Carsten

>
> Regards
> Felix
>
>
> Am 01.03.2013 um 17:48 schrieb Carsten Ziegeler:
>
>> Hi,
>>
>> while looking into some issues, I realized that ResourceMetadata is
>> not only extending a HashMap (which makes handling easier), but we
>> have absolutely no information if this map can be changed by client
>> code or is a read-only map.
>
>>
>> I think we should add this to the documentation and make this
>> read-only. We could either just document it or add a "make read-only"
>> method to ResourceMetadata which is called by the resource resolver
>> before the resource object is returned to the client code.
>>
>> But I think we should not allow client code to change/add/remove to
>> ResourceMetadata.
>>
>> WDYT
>>
>> Regards
>> Carsten
>> --
>> Carsten Ziegeler
>> cziegeler@apache.org
>
>
> --
> Felix Meschberger | Principal Scientist | Adobe
>
>
>
>
>
>
>



-- 
Carsten Ziegeler
cziegeler@apache.org

Re: [RT] ResourceMetadata

Posted by Bertrand Delacretaz <bd...@apache.org>.
On Mon, Mar 4, 2013 at 5:24 AM, Felix Meschberger <fm...@adobe.com> wrote:
> ...We could see, whether it would be possible to make the ResourceMetadata read-only after resource
> resolution has completed, e.g. ResourceMetadata.lock(). But I am not really convinced....

Maybe wrap it in a ReadOnlyResourceMetadataWrapper, once resolution is done?
Looks cleaner than a lock() or setReadyOnly() method to me.

> ...I am also not sure, what client application we would break with this change ?
> What happens in the Sling Launchpad Testing build if this would be read-only ?...

IMO there's no reason to change ResourceMetadata once a resource has
been setup, so if any tests break we should rather fix them. I don't
remember touching it in tests that I wrote.

-Bertrand

Re: [RT] ResourceMetadata

Posted by Felix Meschberger <fm...@adobe.com>.
Hi,

I have been looking into this some time ago. And I am also somewhat unhappy with the read/write nature of the ResourceMetadata.

While I think in general we can make the ResourceMetadata read-only, we have a problem with code like:

> resource.getResourceMetadata().setResolutionPathInfo(rpi);

which is in ResourceResolverImpl.resolveInternal (line 805). This would break.

We could see, whether it would be possible to make the ResourceMetadata read-only after resource resolution has completed, e.g. ResourceMetadata.lock(). But I am not really convinced.

I am also not sure, what client application we would break with this change ?

What happens in the Sling Launchpad Testing build if this would be read-only ?

Regards
Felix


Am 01.03.2013 um 17:48 schrieb Carsten Ziegeler:

> Hi,
> 
> while looking into some issues, I realized that ResourceMetadata is
> not only extending a HashMap (which makes handling easier), but we
> have absolutely no information if this map can be changed by client
> code or is a read-only map.

> 
> I think we should add this to the documentation and make this
> read-only. We could either just document it or add a "make read-only"
> method to ResourceMetadata which is called by the resource resolver
> before the resource object is returned to the client code.
> 
> But I think we should not allow client code to change/add/remove to
> ResourceMetadata.
> 
> WDYT
> 
> Regards
> Carsten
> -- 
> Carsten Ziegeler
> cziegeler@apache.org


--
Felix Meschberger | Principal Scientist | Adobe