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