Re: [RT] Support for deep reads from a value map

2014-03-17 Thread Justin Edelson
OK. Thanks for explaining. Just wanted to make sure I wasn't missing something. Justin On Sun, Mar 16, 2014 at 5:46 AM, Carsten Ziegeler wrote: > Hi Justin > > it's right we can get the same result without adding this method, however > with adding this method we really make clear that there is a

Re: [RT] Support for deep reads from a value map

2014-03-16 Thread Carsten Ziegeler
Hi Justin it's right we can get the same result without adding this method, however with adding this method we really make clear that there is a contract change. We don't need a refactoring of the Resource api, just an extension - which imho we should have done from the beginning. Carsten 2014-

Re: [RT] Support for deep reads from a value map

2014-03-15 Thread Justin Edelson
Hi Carsten, I don't understand the coupling between the addition of Resource.getValueMap() and having the Mongo, Merged, and JMX ResourceProviders start using DeepReadValueMapDecorator. IIOW, couldn't we get the same effect *without* changing the Resource API? That has an impact on downstream user

Re: [RT] Support for deep reads from a value map

2014-03-15 Thread Carsten Ziegeler
Exactly, if you have a look at the implementation or read my description, you see that the jcr implementation did not change at all. So, for all jcr backed resources nothing has changed and the deep read is done there. Implementations are free to implement the deep read themselves or can use the pr

Re: [RT] Support for deep reads from a value map

2014-03-14 Thread Alexander Klimetschek
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.

Re: [RT] Support for deep reads from a value map

2014-03-14 Thread Carsten Ziegeler
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 : > Hi Felix, > > yes, the changes I just

Re: [RT] Support for deep reads from a value map

2014-03-14 Thread Carsten Ziegeler
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 DeepReadValueMapDe

Re: [RT] Support for deep reads from a value map

2014-03-14 Thread Carsten Ziegeler
I've commited a first version based on this. The good news is that only a few bundles are affected. Apart from API, we need to update the JMX Resource Provider, the Merge Resource Provider and the Mongo DB Resource Provider. The JCR Resource Provider has already an implementation - while this is wr

Re: [RT] Support for deep reads from a value map

2014-03-14 Thread Felix Meschberger
Hi Am 14.03.2014 um 11:42 schrieb Carsten Ziegeler : > 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() m

Re: [RT] Support for deep reads from a value map

2014-03-14 Thread Julian Sedding
I believe it would be possible to implement AbstractResource#adaptTo(ValueMap.class) in a generic way supporting relative paths. A rarely used feature (at least of the JcrResourceProvider) is to request a property as a Resource (e.g. resource.getResource("jcr:title").adaptTo(String.class)). If the

Re: [RT] Support for deep reads from a value map

2014-03-14 Thread Carsten Ziegeler
Well, in practice, bundles implementing a previous version will not resolve anyway 2014-03-14 11:56 GMT+01:00 Bertrand Delacretaz : > On Fri, Mar 14, 2014 at 11:53 AM, Carsten Ziegeler > wrote: > > All implementations should inherit from AbstractResource for exactly > those > > reasons :) > > *

Re: [RT] Support for deep reads from a value map

2014-03-14 Thread Bertrand Delacretaz
On Fri, Mar 14, 2014 at 11:53 AM, Carsten Ziegeler wrote: > All implementations should inherit from AbstractResource for exactly those > reasons :) *in theory*, yes. -Bertrand

Re: [RT] Support for deep reads from a value map

2014-03-14 Thread Carsten Ziegeler
All implementations should inherit from AbstractResource for exactly those reasons :) Carsten 2014-03-14 11:48 GMT+01:00 Bertrand Delacretaz : > Hi, > > On Fri, Mar 14, 2014 at 11:42 AM, Carsten Ziegeler > wrote: > > ...The only other option I see is to make ValueMap a first class citizen > an

Re: [RT] Support for deep reads from a value map

2014-03-14 Thread Bertrand Delacretaz
Hi, On Fri, Mar 14, 2014 at 11:42 AM, Carsten Ziegeler wrote: > ...The only other option I see is to make ValueMap a first class citizen and: > a) add a getValueMap() method to Resource - default implementation in > AbstractResource does the same as ResourceUtil does today > b) clearly state that

Re: [RT] Support for deep reads from a value map

2014-03-14 Thread Carsten Ziegeler
The only other option I see is to make ValueMap a first class citizen and: a) add a getValueMap() method to Resource - default implementation in AbstractResource does the same as ResourceUtil does today b) clearly state that deep gets are allowed for reading from those maps c) provide utility code

Re: [RT] Support for deep reads from a value map

2014-03-14 Thread Carsten Ziegeler
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.. Gup

RE: [RT] Support for deep reads from a value map

2014-03-14 Thread Amit.. Gupta.
> FWIW, there are lots of calls to resource.adaptTo(ValueMap) in rendering code. +1

Re: [RT] Support for deep reads from a value map

2014-03-14 Thread Jeff Young
FWIW, there are lots of calls to resource.adaptTo(ValueMap) in rendering code. Cheers, Jeff. On 14/03/2014 10:19, "Bertrand Delacretaz" wrote: >On Fri, Mar 14, 2014 at 11:03 AM, Carsten Ziegeler >wrote: >> ...Just for rendering code, it is quiet handy to have deep reads - and >> rendering cod

Re: [RT] Support for deep reads from a value map

2014-03-14 Thread Bertrand Delacretaz
On Fri, Mar 14, 2014 at 11:19 AM, Bertrand Delacretaz wrote: > ...I'm not opposed to changing just ResourceUtil.getValueMap() to provide > "deep" value maps, just trying to understand the reasoning behind it Thinking about it...if we require people to use a utility method, we might as well cr

Re: [RT] Support for deep reads from a value map

2014-03-14 Thread Bertrand Delacretaz
On Fri, Mar 14, 2014 at 11:03 AM, Carsten Ziegeler wrote: > ...Just for rendering code, it is quiet handy to have deep reads - and > rendering code is using ResourceUtil.getValueMap()... Why would rendering code use this over Resource.adaptTo? Sling makes the current resource available to scripts

Re: [RT] Support for deep reads from a value map

2014-03-14 Thread Carsten Ziegeler
2014-03-14 10:58 GMT+01:00 Bertrand Delacretaz : > On Fri, Mar 14, 2014 at 9:19 AM, Carsten Ziegeler > wrote: > > ...The simplest solution would be to enhance ResourceUtil.getValueMap() > to > > return a wrapper of the value map which allows deep reads... > > Sounds reasonable, but will that also

Re: [RT] Support for deep reads from a value map

2014-03-14 Thread Bertrand Delacretaz
On Fri, Mar 14, 2014 at 9:19 AM, Carsten Ziegeler wrote: > ...The simplest solution would be to enhance ResourceUtil.getValueMap() to > return a wrapper of the value map which allows deep reads... Sounds reasonable, but will that also work if I just call Resource.adaptTo(ValueMap.class)? Or do we