Skip to content

Commit

Permalink
re #2960 SCP BVP replace reflection with proper LazyBindings types
Browse files Browse the repository at this point in the history
  • Loading branch information
@adamcin
adamcin committed Oct 9, 2022
1 parent c610972 commit 027f47f
Show file tree
Hide file tree
Showing 9 changed files with 88 additions and 388 deletions.
42 changes: 5 additions & 37 deletions bundle/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,37 +41,6 @@
</archive>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>
<executions>
<execution>
<!--
this execution downloads org.apache.sling.api-2.22.0.jar for
*TESTING* pre-6.5.7 LazyBindings support in our BindingsValuesProvider services,
and specifically, SharedComponentPropertiesBindingsValuesProvider.
-->
<id>download-sling-api-2-22-0-jar</id>
<phase>process-test-resources</phase>
<goals>
<goal>copy</goal>
</goals>
<configuration>
<artifactItems>
<item>
<groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.api</artifactId>
<version>2.22.0</version>
<type>jar</type>
</item>
</artifactItems>
<!-- drop it into the target/test-classes directory -->
<!-- required by SharedComponentPropertiesBindingsValuesProviderTest -->
<outputDirectory>${project.build.testOutputDirectory}</outputDirectory>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<groupId>biz.aQute.bnd</groupId>
<artifactId>bnd-baseline-maven-plugin</artifactId>
Expand Down Expand Up @@ -356,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 @@ -383,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 @@ -423,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 @@ -745,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
156 changes: 7 additions & 149 deletions ...s/commons/wcm/properties/shared/impl/SharedComponentPropertiesBindingsValuesProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
package com.adobe.acs.commons.wcm.properties.shared.impl;

import com.adobe.acs.commons.wcm.properties.shared.SharedComponentProperties;
import org.apache.felix.scr.annotations.Activate;
import org.apache.felix.scr.annotations.Component;
import org.apache.felix.scr.annotations.Reference;
import org.apache.felix.scr.annotations.ReferenceCardinality;
Expand All @@ -29,18 +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.lang.reflect.InvocationHandler;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
import java.util.Optional;
import java.util.function.Supplier;
import java.util.stream.Stream;

/**
* Bindings Values Provider that adds bindings for globalProperties,
Expand All @@ -62,170 +58,32 @@
public class SharedComponentPropertiesBindingsValuesProvider implements BindingsValuesProvider {
private static final Logger log = LoggerFactory.getLogger(SharedComponentPropertiesBindingsValuesProvider.class);

/**
* The LazyBindings class, and its Supplier child interface, are introduced in org.apache.sling.api version 2.22.0,
* which is first included in AEM 6.5 SP7.
*/
protected static final String FQDN_LAZY_BINDINGS = "org.apache.sling.api.scripting.LazyBindings";
protected static final String SUPPLIER_PROXY_LABEL = "ACS AEM Commons SCP BVP reflective Proxy for LazyBindings.Supplier";

/**
* Bind if available, check for null when reading.
*/
@Reference(policyOption = ReferencePolicyOption.GREEDY, cardinality = ReferenceCardinality.OPTIONAL_UNARY)
SharedComponentProperties sharedComponentProperties;

/**
* Added for pre-6.5.7 support for LazyBindings. This holds the LazyBindings interface
* if it is discovered on activation, and is used to check if the {@link #addBindings(Bindings)} param
* is an instance of LazyBindings. This hack is necessary until this bundle can drop support for
* AEM versions prior to 6.5.7, at which point this variable can be removed, and the {@link #isLazy(Bindings)}
* method can be simplified to return {@code bindings instanceof LazyBindings}.
*/
private Class<? extends Bindings> lazyBindingsType;

/**
* Added for pre-6.5.7 support for LazyBindings. This holds the LazyBindings.Supplier interface
* if it is discovered on activation, and is used to create reflection Proxy instances as a hack
* until this bundle can drop support for AEM versions prior to 6.5.7, at which point this variable
* can be removed, and the {@link #wrapSupplier(Supplier)} method can be simplified to accept a
* LazyBindings.Supplier instead of a java.util.function.Supplier and return it (for matching a
* lambda expression passed at the call site), or to simply return a lambda that calls the get()
* method on the java.util.function.Supplier argument.
*/
private Class<? extends Supplier> supplierType;

/**
* This variable only exists to facilitate testing for pre-6.5.7 LazyBindings support, so that a non-classpath
* class loader can be injected, to provide the LazyBindings class.
*/
private ClassLoader lazyBindingsClassLoader = SlingBindings.class.getClassLoader();

/**
* Called by the unit test to inject a URL class loader that provides a LazyBindings instance
* at {@link #FQDN_LAZY_BINDINGS}.
*
* @param classLoader a new class loader
* @return the old class loader
*/
protected ClassLoader swapLazyBindingsClassLoaderForTesting(ClassLoader classLoader) {
if (classLoader != null) {
ClassLoader oldClassLoader = this.lazyBindingsClassLoader;
this.lazyBindingsClassLoader = classLoader;
return oldClassLoader;
}
return null;
}

/**
* Return the resolved lazyBindingsType for testing.
*
* @return the lazyBindingsType
*/
protected Class<? extends Bindings> getLazyBindingsType() {
return this.lazyBindingsType;
}

/**
* Return the resolved supplierType for testing.
*
* @return the supplierType
*/
protected Class<? extends Supplier> getSupplierType() {
return this.supplierType;
}

/**
* 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). And so long as pre-6.5.7 AEMs are supported,
* this method is also responsible for constructing the {@link Proxy} instance when LazyBindings is present at
* runtime, and for immediately returning {@code Supplier.get()} when it is not present.
* After support for pre-6.5.7 AEMs is dropped, the method return type can be changed from {@code Object} to
* {@code <T> LazyBindings.Supplier<T>} to fully support lazy injection.
* inference to the functional interface type of the method parameter).
*
* @param supplier the provided supplier
* @return the Supplier as a LazyBindings.Supplier if supported, or the value of the provided supplier if not
*/
protected Object wrapSupplier(final Supplier<?> supplier) {
if (this.supplierType != null) {
return Proxy.newProxyInstance(lazyBindingsClassLoader, new Class[]{this.supplierType},
new SupplierWrapper(supplier));
}
return supplier.get();
}

/**
* The only purpose of this class is to drive the pre-6.5.7 reflection-based Proxy instance returned
* by {@link #wrapSupplier(Supplier)}.
*/
protected static class SupplierWrapper implements InvocationHandler {
private final Supplier<?> wrapped;

public SupplierWrapper(final Supplier<?> supplier) {
this.wrapped = supplier;
}

@Override
public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
// we are implementing a @FunctionalInterface, so don't get carried away with implementing
// Object methods.
if ("get".equals(method.getName())) {
return wrapped.get();
} else if ("toString".equals(method.getName())) {
// return this marker string for visibility in debugging tools. Otherwise,
// the default toString is "\"null\"", which is confusing
return SUPPLIER_PROXY_LABEL;
}
return method.getDefaultValue();
}
}

/**
* The purpose of this activate method is to determine if we are running in a 6.5.7+ AEM environment
* without having to explicitly require {@code org.apache.sling.api.scripting} package version 2.5.0.
*/
@Activate
protected void activate() {
// use SlingBindings class loader to check for LazyBindings class,
// to minimize risk involved with using reflection.
try {
this.checkAndSetLazyBindingsType(lazyBindingsClassLoader.loadClass(FQDN_LAZY_BINDINGS));
} catch (ReflectiveOperationException cnfe) {
log.info("LazyBindings not found, will resort to injecting immediate Bindings values", cnfe);
}
}

/**
* Check that the provided {@code lazyBindingsType} implements {@link Bindings} and defines an enclosed marker
* interface named {@code Supplier} that extends {@link Supplier}, and if so, set {@code this.lazyBindingsType} and
* {@code this.supplierType}. Otherwise, set both to {@code null}.
* @return the Supplier as a LazyBindings.Supplier
*/
@SuppressWarnings({"squid:S1872", "unchecked"})
protected void checkAndSetLazyBindingsType(final Class<?> lazyBindingsType) {
if (lazyBindingsType != null && Bindings.class.isAssignableFrom(lazyBindingsType)) {
this.supplierType = (Class<? extends Supplier>) Stream.of(lazyBindingsType.getDeclaredClasses())
.filter(clazz -> Supplier.class.getSimpleName().equals(clazz.getSimpleName())
&& Supplier.class.isAssignableFrom(clazz)).findFirst().orElse(null);
this.lazyBindingsType = (Class<? extends Bindings>) lazyBindingsType;
} else {
log.info("Supplier interface not declared by lazyBindingsType: {}, will resort to immediate Bindings values",
lazyBindingsType);
this.supplierType = null;
this.lazyBindingsType = null;
}
protected LazyBindings.Supplier wrapSupplier(final Supplier<?> supplier) {
return () -> supplier != null ? supplier.get() : null;
}

/**
* Check if provided {@code bindings} implements LazyBindings.
* 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 Optional.ofNullable(this.lazyBindingsType)
.map(clazz -> clazz.isInstance(bindings))
.orElse(false);
return bindings instanceof LazyBindings;
}

/**
Expand Down
11 changes: 4 additions & 7 deletions ...t/java/com/adobe/acs/commons/images/transformers/impl/AdjustImageTransformerImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

package com.adobe.acs.commons.images.transformers.impl;

import com.adobe.acs.commons.images.transformers.impl.AdjustImageTransformerImpl;
import com.day.image.Layer;

import org.apache.sling.api.resource.ValueMap;
Expand Down Expand Up @@ -49,25 +48,22 @@ public class AdjustImageTransformerImplTest {
@Mock
Layer layer;

Map<String, Object> map = null;

@Before
public void setUp() throws Exception {
map = new HashMap<String, Object>();
transformer = new AdjustImageTransformerImpl();
}

@After
public void tearDown() throws Exception {
reset(layer);
map = null;
}

@Test
public void testTransform() throws Exception {
final Integer brightness = 100;
final Float contrast = 0.05F;

Map<String, Object> map = new HashMap<>();
map.put("brightness", brightness.toString());
map.put("contrast", contrast.toString());
ValueMap properties = new ValueMapDecorator(map);
Expand All @@ -80,6 +76,7 @@ public void testTransform() throws Exception {

@Test
public void testTransform_noParams() throws Exception {
Map<String, Object> map = new HashMap<>();
ValueMap properties = new ValueMapDecorator(map);

transformer.transform(layer, properties);
Expand All @@ -90,7 +87,7 @@ public void testTransform_noParams() throws Exception {
@Test
public void testTransform_onlyBrightness() throws Exception {
final Integer brightness = 100;

Map<String, Object> map = new HashMap<>();
map.put("brightness", brightness.toString());
ValueMap properties = new ValueMapDecorator(map);

Expand All @@ -103,7 +100,7 @@ public void testTransform_onlyBrightness() throws Exception {
@Test
public void testTransform_onlyContrast() throws Exception {
final Float contrast = 0.05F;

Map<String, Object> map = new HashMap<>();
map.put("contrast", contrast.toString());
ValueMap properties = new ValueMapDecorator(map);

Expand Down
Loading

0 comments on commit 027f47f

Please sign in to comment.