Adobe-Consulting-Services / acs-aem-samples

AEM Code Samples repository
http://adobe-consulting-services.github.io/acs-aem-samples/
Apache License 2.0
207 stars 195 forks source link

SampleResourceWrapper behaves differently than a JCR resource #56

Open joerghoh opened 7 years ago

joerghoh commented 7 years ago

Today we encountered the problem, that the application behaved differently when called on a JCR resource than on wrapped resource (which is built according to the SampleResourceWrapper).

The application code looked like this:

ValueMap map = resource.adaptTo(ValueMap.class);
Date lastModified = map.get("cq:lastModified", Date.class);

When the resource is a JCR resource, a correct Date object is returned, if the Resource is a SampleResourceWrapper, null is returned (even if the property comes from the wrapped resource and is not injected by the wrapper).

It took us some time to track that back to SLING-6420 (which is likely to be part of AEM 6.3); and our solution was to change the ValueMapDecorator [1] to a CompositeValueMap, which provided the proper semantics.

So can you please adjust the sample?

[1] https://github.com/Adobe-Consulting-Services/acs-aem-samples/blob/master/bundle/src/main/java/com/adobe/acs/samples/resources/SampleResourceWrapper.java#L46

davidjgonzalez commented 7 years ago

@joerghoh thanks. How does this [1] look?

[1] https://github.com/Adobe-Consulting-Services/acs-aem-samples/commit/b50a7d01e31db379604972a7df1531578c35821d

joerghoh commented 7 years ago

We had the parameters the other way around:

this.properties = new CompositeValueMap(super.getValueMap(),overlayProperties);

If you fix that it's ok :-)

davidjgonzalez commented 7 years ago

@joerghoh isn't youre backwards?

public CompositeValueMap(ValueMap properties, ValueMap defaults)
properties - The ValueMap to read from
defaults - The default ValueMap to use as fallback

So you'd want your overlayProperties to take precedence (thus the first valueMap) and the the resource's real properties (super.getValueMap()) as the "backups", no? I think you're effectively making the overlayProperties, act as underlayProperties ?

[1] https://sling.apache.org/apidocs/sling8/org/apache/sling/api/wrappers/CompositeValueMap.html