IIUC, a deep read is now done generically by fetching sub resources first and 
each value map will from now on only read their "local" values.

I think this introduces overhead for a lot of common operations. For every 
descendant node you now have to read and instantiate a resource and value map. 
So far with the JCR property map this was left to the JCR repo.

IMO we should not change much here and simply support deep reads whereever 
possible in the resource implemention (and the missing part here is in the 
/mnt/overlay resource merger AFAIU).

Cheers,
Alex

On 14.03.2014, at 08:45, Carsten Ziegeler <cziege...@apache.org> wrote:

> I just found out that we have to change our idea a little bit, we can't
> demand that Resource#adaptTo(ValueMap.class) always returns a value map.
> The nice exception is the JcrPropertyResource...
> 
> Regards
> Carsten
> 
> 
> 2014-03-14 15:08 GMT+01:00 Carsten Ziegeler <cziege...@apache.org>:
> 
>> Hi Felix,
>> 
>> yes, the changes I just committed in fact deprecate
>> ResourceUtil.getValueMap() and let that method call resource.getValueMap()
>> (if resource is not null of course).
>> 
>> The utility code is an implementation of the value map which supports the
>> deep read methods. See the new DeepReadValueMapDecorator class.
>> 
>> Existing implementations which currently return a value map simply need to
>> wrap their value map with this new decorator. Of course they are free to
>> implement the deep reads by themselves.
>> 
>> I wanted to change the jcr resource provider, but that would introduce
>> incompatibilities to a lot of wired path encoding stuff in the
>> implementation. So it's safer to leave it as is.
>> 
>> Carsten
>> 
>> 
>> 2014-03-14 14:58 GMT+01:00 Felix Meschberger <fmesc...@adobe.com>:
>> 
>> Hi
>>> 
>>> Am 14.03.2014 um 11:42 schrieb Carsten Ziegeler <cziege...@apache.org>:
>>> 
>>>> The only other option I see is to make ValueMap a first class citizen
>>> and:
>>> 
>>> I think this makes sense independently of deep reads or not -- and maybe
>>> we should have done this right when we introduced the ValueMap...
>>> 
>>>> a) add a getValueMap() method to Resource - default implementation in
>>>> AbstractResource does the same as ResourceUtil does today
>>> 
>>> In essence ?
>>> 
>>>> ValueMap AbstractResource.getValueMap() {
>>>>    return ResourceUtil.getValueMap(this);
>>>> }
>>> 
>>> or rather copy the ResourceUtil.getValueMap(Resource) method to
>>> AbstractResource.getValueMap(), implement the deep-read logic there and do
>>> 
>>>> ValueMap ResourceUtil.getValueMap(Resource r) {
>>>>    return r.getValueMap();
>>>> }
>>> 
>>> while at the same time deprecating ResourceUtil.getValueMap(Resource) ?
>>> 
>>> 
>>>> b) clearly state that deep gets are allowed for reading from those maps
>>> 
>>> That would go to the JavaDoc of Resource.getValueMap() along with the
>>> requirement to never return null.
>>> 
>>>> c) provide utility code in AbstractResource (or maybe somewhere else) so
>>>> implementations do not need to copy the same code over and over again
>>> 
>>> What kind of utility code ?
>>> 
>>> What would existing implementations do ?
>>> 
>>> I would assume the JCR-based one would have to be touched to not support
>>> deep reads itself, others will benefit. Right ?
>>> 
>>> Regards
>>> Felix
>>> 
>>>> 
>>>> This would mean:
>>>> a) current code does not need to change, regardless whether
>>> ResourceUtil or
>>>> adaptTo is used
>>>> b) a value map is always provided
>>>> c) all value maps support deep reads
>>>> d) no code duplication necessary
>>>> 
>>>> Carsten
>>>> 
>>>> 
>>>> 2014-03-14 11:37 GMT+01:00 Carsten Ziegeler <cziege...@apache.org>:
>>>> 
>>>>> The idea behind ResourceUtil.getValueMap is that it never returns null
>>>>> while resource.adaptTo(ValueMap) can.
>>>>> That's the whole point of this utility method.
>>>>> 
>>>>> I assume people who do resource.adaptTo(ValueMap) never check for null
>>>>> which is bound to fail
>>>>> 
>>>>> Carsten
>>>>> 
>>>>> 
>>>>> 2014-03-14 11:30 GMT+01:00 Amit.. Gupta. <amitg...@adobe.com>:
>>>>> 
>>>>>> FWIW, there are lots of calls to resource.adaptTo(ValueMap) in
>>> rendering
>>>>>> code.
>>>>>> +1
>>>>>> 
>>>>>> 
>>>>> 
>>>>> 
>>>>> --
>>>>> Carsten Ziegeler
>>>>> cziege...@apache.org
>>>>> 
>>>> 
>>>> 
>>>> 
>>>> --
>>>> Carsten Ziegeler
>>>> cziege...@apache.org
>>> 
>>> 
>> 
>> 
>> --
>> Carsten Ziegeler
>> cziege...@apache.org
>> 
> 
> 
> 
> -- 
> Carsten Ziegeler
> cziege...@apache.org

Reply via email to