Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

issue #2960 shared-component-properties add support for lazy bindings, require AEM 6.5.7+ #2964

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com)
- #2950 - Rewriter Packagers (x4) to use TouchUI instead of ClassicUI
- #2888 - Removed deprecated ComponentHelper

### Fixed

- #2960 - SharedComponentPropertiesBindingsValuesProvider should support LazyBindings


## 5.3.4 - 2022-08-22

Expand Down
11 changes: 5 additions & 6 deletions bundle/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,8 @@
<dependency>
<groupId>org.osgi</groupId>
<artifactId>org.osgi.dto</artifactId>
<!-- defer to io.wcm.maven:io.wcm.maven.aem-dependencies managed version if possible -->
<version>1.1.0</version>
<scope>provided</scope>
</dependency>
<dependency>
Expand Down Expand Up @@ -352,11 +354,6 @@
<artifactId>slf4j-api</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>javax.mail</groupId>
<artifactId>mail</artifactId>
<scope>provided</scope>
</dependency>
<!-- for com.adobe.acs.commons.logging.impl.SyslogAppender -->
<dependency>
<groupId>ch.qos.logback</groupId>
Expand Down Expand Up @@ -392,6 +389,8 @@
<dependency>
<groupId>com.github.jknack</groupId>
<artifactId>handlebars</artifactId>
<!-- defer to io.wcm.maven:io.wcm.maven.aem-dependencies managed version if possible -->
<version>4.0.5</version>
<scope>provided</scope>
</dependency>
<!-- END runtime dependencies not contained in uber-jar -->
Expand Down Expand Up @@ -714,7 +713,7 @@
<dependency>
<groupId>com.adobe.aem</groupId>
<artifactId>uber-jar</artifactId>
<classifier>apis</classifier>
<classifier>apis-with-deprecations</classifier>
<scope>provided</scope>
</dependency>
</dependencies>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@ public final Layer transform(final Layer layer, final ValueMap properties) {

log.debug("Transforming with [ {} ]", TYPE);

int brightness = properties.get(KEY_BRIGHTNESS, properties.get(KEY_BRIGHTNESS_ALIAS, 0));
float contrast = properties.get(KEY_CONTRAST, properties.get(KEY_CONTRAST_ALIAS, 1.0)).floatValue();
Long brightness = properties.get(KEY_BRIGHTNESS, properties.get(KEY_BRIGHTNESS_ALIAS, 0L));
Double contrast = properties.get(KEY_CONTRAST, properties.get(KEY_CONTRAST_ALIAS, 1.0D));

layer.adjust(brightness, contrast);
layer.adjust(brightness.intValue(), contrast.floatValue());

return layer;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ public final Layer transform(final Layer layer, final ValueMap properties) {

log.debug("Transforming with [ {} ]", TYPE);

int width = properties.get(KEY_WIDTH, properties.get(KEY_WIDTH_ALIAS, 0));
int height = properties.get(KEY_HEIGHT, properties.get(KEY_HEIGHT_ALIAS, 0));
int width = properties.get(KEY_WIDTH, properties.get(KEY_WIDTH_ALIAS, 0L)).intValue();
int height = properties.get(KEY_HEIGHT, properties.get(KEY_HEIGHT_ALIAS, 0L)).intValue();

if (width > maxDimension) {
width = maxDimension;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public final Layer transform(Layer layer, final ValueMap properties) {

log.debug("Transforming with [ {} ]", TYPE);

Double scale = properties.get(KEY_SCALE, 1D);
Double scale = properties.get(KEY_SCALE, Double.class);
String round = StringUtils.trim(properties.get(KEY_ROUND, String.class));

if (scale == null) {
Expand All @@ -89,27 +89,28 @@ public final Layer transform(Layer layer, final ValueMap properties) {
int currentWidth = layer.getWidth();
int currentHeight = layer.getHeight();

double newWidth = scale * currentWidth;
double newHeight = scale * currentHeight;
double scaledWidth = scale * currentWidth;
double scaledHeight = scale * currentHeight;

long newWidth, newHeight;
if (StringUtils.equals(ROUND_UP, round)) {
newWidth = (int) Math.ceil(newWidth);
newHeight = (int) Math.ceil(newHeight);
newWidth = (long) Math.ceil(scaledWidth);
newHeight = (long) Math.ceil(scaledHeight);
} else if (StringUtils.equals(ROUND_DOWN, round)) {
newWidth = (int) Math.floor(newWidth);
newHeight = (int) Math.floor(newHeight);
newWidth = (long) Math.floor(scaledWidth);
newHeight = (long) Math.floor(scaledHeight);
} else {
// "round"
newWidth = (int) Math.round(newWidth);
newHeight = (int) Math.round(newHeight);
newWidth = Math.round(scaledWidth);
newHeight = Math.round(scaledHeight);
}


// Invoke the ResizeImageTransformer with the new values

final ValueMap params = new ValueMapDecorator(new HashMap<String, Object>());
params.put(ResizeImageTransformerImpl.KEY_WIDTH, (int)newWidth);
params.put(ResizeImageTransformerImpl.KEY_HEIGHT, (int)newHeight);
params.put(ResizeImageTransformerImpl.KEY_WIDTH, newWidth);
params.put(ResizeImageTransformerImpl.KEY_HEIGHT, newHeight);

layer = resizeImageTransformer.transform(layer, params);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,15 @@
import org.apache.sling.api.SlingHttpServletRequest;
import org.apache.sling.api.resource.Resource;
import org.apache.sling.api.resource.ValueMap;
import org.apache.sling.api.scripting.LazyBindings;
import org.apache.sling.api.scripting.SlingBindings;
import org.apache.sling.scripting.api.BindingsValuesProvider;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.script.Bindings;
import java.util.Optional;
import java.util.function.Supplier;

/**
* Bindings Values Provider that adds bindings for globalProperties,
Expand Down Expand Up @@ -61,6 +64,88 @@ public class SharedComponentPropertiesBindingsValuesProvider implements Bindings
@Reference(policyOption = ReferencePolicyOption.GREEDY, cardinality = ReferenceCardinality.OPTIONAL_UNARY)
SharedComponentProperties sharedComponentProperties;

/**
* This method ensures that the provided supplier is appropriately typed for insertion into a SlingBindings
* object. It primarily facilitates lambda type inference (i.e., {@code wrapSupplier(() -> something)} forces
* inference to the functional interface type of the method parameter).
*
* @param supplier the provided supplier
* @return the Supplier as a LazyBindings.Supplier
*/
protected LazyBindings.Supplier wrapSupplier(final Supplier<?> supplier) {
return () -> supplier != null ? supplier.get() : null;
}

/**
* Check if provided {@code bindings} is an instance of {@link LazyBindings}.
*
* @param bindings the parameter from {@link #addBindings(Bindings)}
* @return true if bindings implements LazyBindings
*/
private boolean isLazy(Bindings bindings) {
return bindings instanceof LazyBindings;
}

/**
* Injects Global SCP keys into the provided bindings in one of two ways:
* 1. lazily, if {@code bindings} is an instance of {@code LazyBindings}
* 2. immediately, for all other kinds of {@code Bindings}
*
* @param bindings the bindings
* @param supplier a global SCP resource supplier
*/
protected void injectGlobalProps(Bindings bindings, Supplier<Optional<Resource>> supplier) {
if (isLazy(bindings)) {
bindings.put(SharedComponentProperties.GLOBAL_PROPERTIES_RESOURCE,
wrapSupplier(() -> supplier.get().orElse(null)));
bindings.put(SharedComponentProperties.GLOBAL_PROPERTIES,
wrapSupplier(() -> supplier.get().map(Resource::getValueMap).orElse(null)));
} else {
supplier.get().ifPresent(value -> {
bindings.put(SharedComponentProperties.GLOBAL_PROPERTIES_RESOURCE, value);
bindings.put(SharedComponentProperties.GLOBAL_PROPERTIES, value.getValueMap());
});
}
}

/**
* Injects Shared SCP keys into the provided bindings in one of two ways:
* 1. lazily, if {@code bindings} is an instance of {@code LazyBindings}
* 2. immediately, for all other kinds of {@code Bindings}
*
* @param bindings the bindings
* @param supplier a shared SCP resource supplier
*/
protected void injectSharedProps(Bindings bindings, Supplier<Optional<Resource>> supplier) {
if (isLazy(bindings)) {
bindings.put(SharedComponentProperties.SHARED_PROPERTIES_RESOURCE,
wrapSupplier(() -> supplier.get().orElse(null)));
bindings.put(SharedComponentProperties.SHARED_PROPERTIES,
wrapSupplier(() -> supplier.get().map(Resource::getValueMap).orElse(null)));
} else {
supplier.get().ifPresent(value -> {
bindings.put(SharedComponentProperties.SHARED_PROPERTIES_RESOURCE, value);
bindings.put(SharedComponentProperties.SHARED_PROPERTIES, value.getValueMap());
});
}
}

/**
* Injects the Merged SCP Properties key into the provided bindings in one of two ways:
* 1. lazily, if {@code bindings} is an instance of {@code LazyBindings}
* 2. immediately, for all other kinds of {@code Bindings}
*
* @param bindings the bindings
* @param supplier a merged SCP ValueMap supplier
*/
protected void injectMergedProps(Bindings bindings, Supplier<ValueMap> supplier) {
if (isLazy(bindings)) {
bindings.put(SharedComponentProperties.MERGED_PROPERTIES, wrapSupplier(supplier));
} else {
bindings.put(SharedComponentProperties.MERGED_PROPERTIES, supplier.get());
}
}

@Override
public void addBindings(final Bindings bindings) {
final SlingHttpServletRequest request = (SlingHttpServletRequest) bindings.get(SlingBindings.REQUEST);
Expand All @@ -83,38 +168,35 @@ private void setSharedProperties(final Bindings bindings,
if (rootPagePath != null) {
// set this value even when global or shared resources are not found to indicate cache validity downstream
bindings.put(SharedComponentProperties.SHARED_PROPERTIES_PAGE_PATH, rootPagePath);

String globalPropsPath = sharedComponentProperties.getGlobalPropertiesPath(resource);
if (globalPropsPath != null) {
bindings.putAll(cache.getBindings(globalPropsPath, (newBindings) -> {
final Resource globalPropsResource = resource.getResourceResolver().getResource(globalPropsPath);
if (globalPropsResource != null) {
newBindings.put(SharedComponentProperties.GLOBAL_PROPERTIES, globalPropsResource.getValueMap());
newBindings.put(SharedComponentProperties.GLOBAL_PROPERTIES_RESOURCE, globalPropsResource);
}
}));
}
// perform null path check within the supplier
final Supplier<Optional<Resource>> supplyGlobalResource = () ->
globalPropsPath != null
? cache.getResource(globalPropsPath, resource.getResourceResolver()::getResource)
: Optional.empty();
injectGlobalProps(bindings, supplyGlobalResource);

final String sharedPropsPath = sharedComponentProperties.getSharedPropertiesPath(resource);
if (sharedPropsPath != null) {
bindings.putAll(cache.getBindings(sharedPropsPath, (newBindings) -> {
Resource sharedPropsResource = resource.getResourceResolver().getResource(sharedPropsPath);
if (sharedPropsResource != null) {
newBindings.put(SharedComponentProperties.SHARED_PROPERTIES, sharedPropsResource.getValueMap());
newBindings.put(SharedComponentProperties.SHARED_PROPERTIES_RESOURCE, sharedPropsResource);
}
}));
bindings.put(SharedComponentProperties.SHARED_PROPERTIES_PATH, sharedPropsPath);
}
// perform null path check within the supplier
final Supplier<Optional<Resource>> supplySharedResource = () ->
sharedPropsPath != null
? cache.getResource(sharedPropsPath, resource.getResourceResolver()::getResource)
: Optional.empty();
injectSharedProps(bindings, supplySharedResource);
bindings.put(SharedComponentProperties.SHARED_PROPERTIES_PATH, sharedPropsPath);

final String mergedPropertiesPath = resource.getPath();
bindings.putAll(cache.getBindings(mergedPropertiesPath, (newBindings) -> {
ValueMap globalPropertyMap = (ValueMap) bindings.get(SharedComponentProperties.GLOBAL_PROPERTIES);
ValueMap sharedPropertyMap = (ValueMap) bindings.get(SharedComponentProperties.SHARED_PROPERTIES);
newBindings.put(SharedComponentProperties.MERGED_PROPERTIES,
sharedComponentProperties.mergeProperties(globalPropertyMap, sharedPropertyMap, resource));
}));
final Supplier<ValueMap> supplyMergedProperties = () ->
cache.getMergedProperties(mergedPropertiesPath, (path) -> {
ValueMap globalPropertyMap = supplyGlobalResource.get().map(Resource::getValueMap).orElse(ValueMap.EMPTY);
ValueMap sharedPropertyMap = supplySharedResource.get().map(Resource::getValueMap).orElse(ValueMap.EMPTY);
return sharedComponentProperties.mergeProperties(globalPropertyMap, sharedPropertyMap, resource);
});
injectMergedProps(bindings, supplyMergedProperties);

// set this value to indicate cache validity downstream
bindings.put(SharedComponentProperties.MERGED_PROPERTIES_PATH, resource.getPath());
bindings.put(SharedComponentProperties.MERGED_PROPERTIES_PATH, mergedPropertiesPath);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,23 @@
package com.adobe.acs.commons.wcm.properties.shared.impl;


import javax.script.Bindings;
import javax.script.SimpleBindings;
import org.apache.sling.api.resource.Resource;
import org.apache.sling.api.resource.ValueMap;

import javax.servlet.ServletRequest;
import java.util.HashMap;
import java.util.Map;
import java.util.function.Consumer;
import java.util.Optional;
import java.util.function.Function;

/**
* Simple cache for global and shared properties bindings keyed by path and persisted in a request attribute.
*/
public final class SharedPropertiesRequestCache {
private static final String REQUEST_ATTRIBUTE_NAME = SharedPropertiesRequestCache.class.getName();

private final Map<String, Bindings> cache = new HashMap<>();
private final Map<String, Optional<Resource>> resourceCache = new HashMap<>();
private final Map<String, ValueMap> mergedCache = new HashMap<>();

/**
* Constructor.
Expand All @@ -42,13 +45,14 @@ private SharedPropertiesRequestCache() {
/* only me */
}

public Bindings getBindings(final String propertiesPath,
final Consumer<Bindings> computeIfNotFound) {
return cache.computeIfAbsent(propertiesPath, key -> {
final Bindings bindings = new SimpleBindings();
computeIfNotFound.accept(bindings);
return bindings;
});
public Optional<Resource> getResource(final String path, final Function<String, Resource> computeIfNotFound) {
return resourceCache.computeIfAbsent(path,
key -> Optional.ofNullable(computeIfNotFound.apply(key)));
}

public ValueMap getMergedProperties(final String path, final Function<String, ValueMap> computeIfNotFound) {
return mergedCache.computeIfAbsent(path,
key -> Optional.ofNullable(computeIfNotFound.apply(key)).orElse(ValueMap.EMPTY));
}

public static SharedPropertiesRequestCache fromRequest(ServletRequest req) {
Expand Down
Loading