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 provided support.
Although, as mentioned in the beginning, the jcr implementation is wrong as
it does not use the resource tree, so a getValueMap().get("content/title")
might return something different than a
getChild("content").getValueMap().get("title"). But this has always been
the case, and as the jcr implementation is very complicated with all the
encoding stuff it contains, I think we shouldn't touch this.
Carsten
2014-03-14 20:28 GMT+01:00 Alexander Klimetschek <aklim...@adobe.com>:
> 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
>
>
--
Carsten Ziegeler
cziege...@apache.org
