diff --git a/CHANGELOG.md b/CHANGELOG.md index a2b9ee979a..795cdba10a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/bundle/pom.xml b/bundle/pom.xml index fd4dfb83af..f1d738db0e 100644 --- a/bundle/pom.xml +++ b/bundle/pom.xml @@ -325,6 +325,8 @@ org.osgi org.osgi.dto + + 1.1.0 provided @@ -352,11 +354,6 @@ slf4j-api provided - - javax.mail - mail - provided - ch.qos.logback @@ -392,6 +389,8 @@ com.github.jknack handlebars + + 4.0.5 provided @@ -714,7 +713,7 @@ com.adobe.aem uber-jar - apis + apis-with-deprecations provided diff --git a/bundle/src/main/java/com/adobe/acs/commons/images/transformers/impl/AdjustImageTransformerImpl.java b/bundle/src/main/java/com/adobe/acs/commons/images/transformers/impl/AdjustImageTransformerImpl.java index f8031e4bcf..eba51f79d2 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/images/transformers/impl/AdjustImageTransformerImpl.java +++ b/bundle/src/main/java/com/adobe/acs/commons/images/transformers/impl/AdjustImageTransformerImpl.java @@ -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; } diff --git a/bundle/src/main/java/com/adobe/acs/commons/images/transformers/impl/ResizeImageTransformerImpl.java b/bundle/src/main/java/com/adobe/acs/commons/images/transformers/impl/ResizeImageTransformerImpl.java index 657922fad5..8d9cdd4a39 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/images/transformers/impl/ResizeImageTransformerImpl.java +++ b/bundle/src/main/java/com/adobe/acs/commons/images/transformers/impl/ResizeImageTransformerImpl.java @@ -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; diff --git a/bundle/src/main/java/com/adobe/acs/commons/images/transformers/impl/ScaleImageTransformerImpl.java b/bundle/src/main/java/com/adobe/acs/commons/images/transformers/impl/ScaleImageTransformerImpl.java index fef52ac339..57b6dc079c 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/images/transformers/impl/ScaleImageTransformerImpl.java +++ b/bundle/src/main/java/com/adobe/acs/commons/images/transformers/impl/ScaleImageTransformerImpl.java @@ -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) { @@ -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()); - 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); } diff --git a/bundle/src/main/java/com/adobe/acs/commons/wcm/properties/shared/impl/SharedComponentPropertiesBindingsValuesProvider.java b/bundle/src/main/java/com/adobe/acs/commons/wcm/properties/shared/impl/SharedComponentPropertiesBindingsValuesProvider.java index 5bcf3cec95..9db2473c51 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/wcm/properties/shared/impl/SharedComponentPropertiesBindingsValuesProvider.java +++ b/bundle/src/main/java/com/adobe/acs/commons/wcm/properties/shared/impl/SharedComponentPropertiesBindingsValuesProvider.java @@ -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, @@ -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> 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> 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 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); @@ -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> 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> 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 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); } } diff --git a/bundle/src/main/java/com/adobe/acs/commons/wcm/properties/shared/impl/SharedPropertiesRequestCache.java b/bundle/src/main/java/com/adobe/acs/commons/wcm/properties/shared/impl/SharedPropertiesRequestCache.java index b8557f3a26..63b59af036 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/wcm/properties/shared/impl/SharedPropertiesRequestCache.java +++ b/bundle/src/main/java/com/adobe/acs/commons/wcm/properties/shared/impl/SharedPropertiesRequestCache.java @@ -20,12 +20,14 @@ 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. @@ -33,7 +35,8 @@ public final class SharedPropertiesRequestCache { private static final String REQUEST_ATTRIBUTE_NAME = SharedPropertiesRequestCache.class.getName(); - private final Map cache = new HashMap<>(); + private final Map> resourceCache = new HashMap<>(); + private final Map mergedCache = new HashMap<>(); /** * Constructor. @@ -42,13 +45,14 @@ private SharedPropertiesRequestCache() { /* only me */ } - public Bindings getBindings(final String propertiesPath, - final Consumer computeIfNotFound) { - return cache.computeIfAbsent(propertiesPath, key -> { - final Bindings bindings = new SimpleBindings(); - computeIfNotFound.accept(bindings); - return bindings; - }); + public Optional getResource(final String path, final Function computeIfNotFound) { + return resourceCache.computeIfAbsent(path, + key -> Optional.ofNullable(computeIfNotFound.apply(key))); + } + + public ValueMap getMergedProperties(final String path, final Function computeIfNotFound) { + return mergedCache.computeIfAbsent(path, + key -> Optional.ofNullable(computeIfNotFound.apply(key)).orElse(ValueMap.EMPTY)); } public static SharedPropertiesRequestCache fromRequest(ServletRequest req) { diff --git a/bundle/src/test/java/com/adobe/acs/commons/images/transformers/impl/AdjustImageTransformerImplTest.java b/bundle/src/test/java/com/adobe/acs/commons/images/transformers/impl/AdjustImageTransformerImplTest.java index b20aa041a5..f6a8191622 100644 --- a/bundle/src/test/java/com/adobe/acs/commons/images/transformers/impl/AdjustImageTransformerImplTest.java +++ b/bundle/src/test/java/com/adobe/acs/commons/images/transformers/impl/AdjustImageTransformerImplTest.java @@ -20,9 +20,9 @@ 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.jackrabbit.value.ValueFactoryImpl; import org.apache.sling.api.resource.ValueMap; import org.apache.sling.api.wrappers.ValueMapDecorator; import org.junit.After; @@ -49,37 +49,35 @@ public class AdjustImageTransformerImplTest { @Mock Layer layer; - Map map = null; @Before public void setUp() throws Exception { - map = new HashMap(); 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.put("brightness", brightness.toString()); - map.put("contrast", contrast.toString()); + final Long brightness = 100L; + final Double contrast = 0.05D; + Map map = new HashMap<>(); + map.put("brightness", ValueFactoryImpl.getInstance().createValue(brightness.toString())); + map.put("contrast", ValueFactoryImpl.getInstance().createValue(contrast.toString())); ValueMap properties = new ValueMapDecorator(map); transformer.transform(layer, properties); - verify(layer, times(1)).adjust(brightness, contrast); + verify(layer, times(1)).adjust(brightness.intValue(), contrast.floatValue()); verifyNoMoreInteractions(layer); } @Test public void testTransform_noParams() throws Exception { + Map map = new HashMap<>(); ValueMap properties = new ValueMapDecorator(map); transformer.transform(layer, properties); @@ -89,27 +87,27 @@ public void testTransform_noParams() throws Exception { @Test public void testTransform_onlyBrightness() throws Exception { - final Integer brightness = 100; - - map.put("brightness", brightness.toString()); + final Long brightness = 100L; + Map map = new HashMap<>(); + map.put("brightness", ValueFactoryImpl.getInstance().createValue(brightness.toString())); ValueMap properties = new ValueMapDecorator(map); transformer.transform(layer, properties); - verify(layer, times(1)).adjust(brightness, 1F); + verify(layer, times(1)).adjust(brightness.intValue(), 1F); verifyNoMoreInteractions(layer); } @Test public void testTransform_onlyContrast() throws Exception { - final Float contrast = 0.05F; - - map.put("contrast", contrast.toString()); + final Double contrast = 0.05D; + Map map = new HashMap<>(); + map.put("contrast", ValueFactoryImpl.getInstance().createValue(contrast.toString())); ValueMap properties = new ValueMapDecorator(map); transformer.transform(layer, properties); - verify(layer, times(1)).adjust(0, contrast); + verify(layer, times(1)).adjust(0, contrast.floatValue()); verifyNoMoreInteractions(layer); } } diff --git a/bundle/src/test/java/com/adobe/acs/commons/images/transformers/impl/ScaleImageTransformerImplTest.java b/bundle/src/test/java/com/adobe/acs/commons/images/transformers/impl/ScaleImageTransformerImplTest.java index 1f6e54eca4..95930cf6bc 100644 --- a/bundle/src/test/java/com/adobe/acs/commons/images/transformers/impl/ScaleImageTransformerImplTest.java +++ b/bundle/src/test/java/com/adobe/acs/commons/images/transformers/impl/ScaleImageTransformerImplTest.java @@ -21,6 +21,7 @@ package com.adobe.acs.commons.images.transformers.impl; import com.day.image.Layer; +import org.apache.jackrabbit.value.ValueFactoryImpl; import org.apache.sling.api.resource.ValueMap; import org.apache.sling.api.wrappers.ValueMapDecorator; import org.junit.After; @@ -77,7 +78,7 @@ public void tearDown() throws Exception { @Test public void testTransform_withMalformedScale() throws Exception { - properties.put("scale", "50%"); + properties.put("scale", ValueFactoryImpl.getInstance().createValue("50%")); transformer.transform(layer, properties); @@ -86,7 +87,7 @@ public void testTransform_withMalformedScale() throws Exception { @Test public void testTransform_scaleAsDouble() throws Exception { - properties.put("scale", ".50"); + properties.put("scale", ValueFactoryImpl.getInstance().createValue(".50")); transformer.transform(layer, properties); @@ -95,7 +96,7 @@ public void testTransform_scaleAsDouble() throws Exception { @Test public void testTransform_scaleAsDouble2() throws Exception { - properties.put("scale", "1.50"); + properties.put("scale", ValueFactoryImpl.getInstance().createValue("1.50")); transformer.transform(layer, properties); @@ -105,7 +106,7 @@ public void testTransform_scaleAsDouble2() throws Exception { @Test public void testTransform_roundDefault() throws Exception { - properties.put("scale", ".3333"); + properties.put("scale", ValueFactoryImpl.getInstance().createValue(".3333")); transformer.transform(layer, properties); @@ -114,7 +115,7 @@ public void testTransform_roundDefault() throws Exception { @Test public void testTransform_roundUp() throws Exception { - properties.put("scale", ".3333"); + properties.put("scale", ValueFactoryImpl.getInstance().createValue(".3333")); properties.put("round", "up"); transformer.transform(layer, properties); @@ -124,7 +125,7 @@ public void testTransform_roundUp() throws Exception { @Test public void testTransform_roundDown() throws Exception { - properties.put("scale", ".3333"); + properties.put("scale", ValueFactoryImpl.getInstance().createValue(".3333")); properties.put("round", "down"); transformer.transform(layer, properties); diff --git a/bundle/src/test/java/com/adobe/acs/commons/models/injectors/impl/SharedValueMapValueInjectorTest.java b/bundle/src/test/java/com/adobe/acs/commons/models/injectors/impl/SharedValueMapValueInjectorTest.java index 28874cde9f..45b2b7b6dd 100644 --- a/bundle/src/test/java/com/adobe/acs/commons/models/injectors/impl/SharedValueMapValueInjectorTest.java +++ b/bundle/src/test/java/com/adobe/acs/commons/models/injectors/impl/SharedValueMapValueInjectorTest.java @@ -29,6 +29,7 @@ import com.adobe.acs.commons.wcm.properties.shared.impl.SharedComponentPropertiesImpl; import com.day.cq.wcm.api.NameConstants; import io.wcm.testing.mock.aem.junit.AemContext; +import org.apache.jackrabbit.value.ValueFactoryImpl; import org.apache.jackrabbit.vault.util.JcrConstants; import org.apache.sling.api.resource.Resource; import org.apache.sling.api.resource.ResourceResolver; @@ -42,6 +43,7 @@ import javax.jcr.Node; import javax.jcr.RepositoryException; import javax.jcr.Session; +import javax.jcr.Value; import java.util.Arrays; import java.util.Collections; @@ -79,6 +81,7 @@ public void setup() throws RepositoryException { this.context.registerInjectActivateService(new SharedComponentPropertiesImpl()); this.context.registerInjectActivateService(new SharedValueMapValueAnnotationProcessorFactory()); this.context.registerInjectActivateService(new SharedValueMapValueInjector()); + this.context.registerInjectActivateService(new SharedComponentPropertiesBindingsValuesProvider()); this.context.addModelsForClasses(TestSharedValueMapValueModelImpl.class); this.resourceResolver = this.context.resourceResolver(); @@ -97,7 +100,7 @@ public void setup() throws RepositoryException { globalPropertiesNode.setProperty(STRING_PROP, "globalValue"); globalPropertiesNode.setProperty(STRING_PROP_2, "globalValue2"); globalPropertiesNode.setProperty(STRING_PROP_3, "globalValue3"); - globalPropertiesNode.setProperty(LONG_PROP, 123L); + globalPropertiesNode.setProperty(LONG_PROP, new Value[]{ValueFactoryImpl.getInstance().createValue(123L)}); globalPropertiesNode.setProperty(LONG_PROP_STR, "234"); globalPropertiesNode.setProperty(STRING_ARRAY_PROP, new String[] {"abc", "def"}); diff --git a/bundle/src/test/java/com/adobe/acs/commons/wcm/impl/PropertyMergePostProcessorTest.java b/bundle/src/test/java/com/adobe/acs/commons/wcm/impl/PropertyMergePostProcessorTest.java index f0907bf663..cb683f7988 100644 --- a/bundle/src/test/java/com/adobe/acs/commons/wcm/impl/PropertyMergePostProcessorTest.java +++ b/bundle/src/test/java/com/adobe/acs/commons/wcm/impl/PropertyMergePostProcessorTest.java @@ -22,6 +22,8 @@ import com.day.cq.tagging.Tag; import com.day.cq.tagging.TagManager; import java.util.ArrayList; + +import org.apache.jackrabbit.value.ValueFactoryImpl; import org.apache.sling.api.resource.ModifiableValueMap; import org.apache.sling.api.resource.Resource; import org.apache.sling.api.resource.ResourceResolver; @@ -40,11 +42,15 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.stream.Stream; + import org.apache.sling.servlets.post.Modification; import org.apache.sling.testing.mock.sling.ResourceResolverType; import org.apache.sling.testing.mock.sling.junit.SlingContext; import org.apache.sling.testing.mock.sling.servlet.MockSlingHttpServletRequest; +import javax.jcr.Value; + import static org.mockito.Mockito.when; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.any; @@ -112,9 +118,9 @@ public void testMerge_NoDuplicates_Long() throws Exception { @Test public void testMerge_NoDuplicates_Double() throws Exception { - properties.put("tenths", new Double[]{1.1D, 1.2D}); - properties.put("hundredths", 3.01D); - properties.put("duplicates", new Double[]{1.1D}); + properties.put("tenths", Stream.of(new Double[]{1.1D, 1.2D}).map(ValueFactoryImpl.getInstance()::createValue).toArray()); + properties.put("hundredths", new Value[]{ValueFactoryImpl.getInstance().createValue(3.01D)}); + properties.put("duplicates", Stream.of(new Double[]{1.1D}).map(ValueFactoryImpl.getInstance()::createValue).toArray()); when(resource.adaptTo(ModifiableValueMap.class)).thenReturn(properties); diff --git a/bundle/src/test/java/com/adobe/acs/commons/wcm/properties/shared/impl/SharedComponentPropertiesBindingsValuesProviderTest.java b/bundle/src/test/java/com/adobe/acs/commons/wcm/properties/shared/impl/SharedComponentPropertiesBindingsValuesProviderTest.java index e12e950c56..96d73b0d8a 100644 --- a/bundle/src/test/java/com/adobe/acs/commons/wcm/properties/shared/impl/SharedComponentPropertiesBindingsValuesProviderTest.java +++ b/bundle/src/test/java/com/adobe/acs/commons/wcm/properties/shared/impl/SharedComponentPropertiesBindingsValuesProviderTest.java @@ -19,26 +19,13 @@ */ package com.adobe.acs.commons.wcm.properties.shared.impl; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Optional; -import java.util.function.BiFunction; - -import javax.script.Bindings; -import javax.script.SimpleBindings; - +import com.adobe.acs.commons.wcm.PageRootProvider; +import com.adobe.acs.commons.wcm.properties.shared.SharedComponentProperties; import org.apache.sling.api.SlingHttpServletRequest; import org.apache.sling.api.resource.Resource; import org.apache.sling.api.resource.ResourceResolver; 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.api.wrappers.ValueMapDecorator; import org.junit.Before; @@ -46,111 +33,190 @@ import org.junit.runner.RunWith; import org.mockito.junit.MockitoJUnitRunner; -import com.adobe.acs.commons.wcm.PageRootProvider; -import com.adobe.acs.commons.wcm.properties.shared.SharedComponentProperties; +import javax.script.Bindings; +import javax.script.SimpleBindings; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.function.BiFunction; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) public class SharedComponentPropertiesBindingsValuesProviderTest { - public static final String SITE_ROOT = "/content/acs-commons"; - public static final String RESOURCE_TYPE = "acs-commons/components/content/generic-text"; - - private PageRootProvider pageRootProvider; - private Resource resource; - private Resource sharedPropsResource; - private Resource globalPropsResource; - private SlingHttpServletRequest request; - private Bindings bindings; - private ResourceResolver resourceResolver; - private ValueMap sharedProps; - private ValueMap globalProps; - - @Before - public void setUp() throws Exception { - resource = mock(Resource.class); - pageRootProvider = mock(PageRootProvider.class); - bindings = new SimpleBindings(); - sharedPropsResource = mock(Resource.class); - globalPropsResource = mock(Resource.class); - resourceResolver = mock(ResourceResolver.class); - request = mock(SlingHttpServletRequest.class); - - final String globalPropsPath = SITE_ROOT + "/jcr:content/" + SharedComponentProperties.NN_GLOBAL_COMPONENT_PROPERTIES; - final String sharedPropsPath = SITE_ROOT + "/jcr:content/" + SharedComponentProperties.NN_SHARED_COMPONENT_PROPERTIES + "/" - + RESOURCE_TYPE; - - bindings.put(SlingBindings.REQUEST, request); - bindings.put(SlingBindings.RESOURCE, resource); - - when(resource.getResourceResolver()).thenReturn(resourceResolver); - when(resource.getResourceType()).thenReturn(RESOURCE_TYPE); - when(resourceResolver.getSearchPath()).thenReturn(new String[]{"/apps/", "/libs/"}); - when(resourceResolver.getResource(sharedPropsPath)).thenReturn(sharedPropsResource); - when(resourceResolver.getResource(globalPropsPath)).thenReturn(globalPropsResource); - - when(resource.getPath()).thenReturn(SITE_ROOT); - when(pageRootProvider.getRootPagePath(anyString())).thenReturn(SITE_ROOT); - - - sharedProps = new ValueMapDecorator(new HashMap()); - globalProps = new ValueMapDecorator(new HashMap()); - sharedProps.put("shared", "value"); - globalProps.put("global", "value"); - - when(globalPropsResource.getValueMap()).thenReturn(globalProps); - when(sharedPropsResource.getValueMap()).thenReturn(sharedProps); - when(resource.getValueMap()).thenReturn(ValueMap.EMPTY); - } - - @Test - public void testGetCanonicalResourceTypeRelativePath() { - // make this test readable by wrapping the long method name with a function - final BiFunction, String> asFunction = - (resourceType, searchPaths) -> SharedComponentPropertiesImpl - .getCanonicalResourceTypeRelativePath(resourceType, - Optional.ofNullable(searchPaths) - .map(list -> list.toArray(new String[0])).orElse(null)); - - final List emptySearchPaths = Collections.emptyList(); - final List realSearchPaths = Arrays.asList("/apps/", "/libs/"); - assertNull("expect null for null rt", asFunction.apply(null, emptySearchPaths)); - assertNull("expect null for empty rt", asFunction.apply("", emptySearchPaths)); - assertNull("expect null for absolute rt and null search paths", - asFunction.apply("/fail/" + RESOURCE_TYPE, null)); - assertNull("expect null for cq:Page", - asFunction.apply("cq:Page", realSearchPaths)); - assertNull("expect null for nt:unstructured", - asFunction.apply("nt:unstructured", realSearchPaths)); - assertNull("expect null for absolute rt and empty search paths", - asFunction.apply("/fail/" + RESOURCE_TYPE, emptySearchPaths)); - assertNull("expect null for sling nonexisting rt", - asFunction.apply(Resource.RESOURCE_TYPE_NON_EXISTING, emptySearchPaths)); - assertEquals("expect same for relative rt", RESOURCE_TYPE, - asFunction.apply(RESOURCE_TYPE, emptySearchPaths)); - assertEquals("expect same for relative rt and real search paths", RESOURCE_TYPE, - asFunction.apply(RESOURCE_TYPE, realSearchPaths)); - assertEquals("expect relative for /apps/ + relative and real search paths", RESOURCE_TYPE, - asFunction.apply("/apps/" + RESOURCE_TYPE, realSearchPaths)); - assertEquals("expect relative for /libs/ + relative and real search paths", RESOURCE_TYPE, - asFunction.apply("/libs/" + RESOURCE_TYPE, realSearchPaths)); - assertNull("expect null for /fail/ + relative and real search paths", - asFunction.apply("/fail/" + RESOURCE_TYPE, realSearchPaths)); - } - - @Test - public void addBindings() { - final SharedComponentPropertiesImpl sharedComponentProperties = new SharedComponentPropertiesImpl(); - sharedComponentProperties.pageRootProvider = pageRootProvider; - - final SharedComponentPropertiesBindingsValuesProvider sharedComponentPropertiesBindingsValuesProvider - = new SharedComponentPropertiesBindingsValuesProvider(); - - sharedComponentPropertiesBindingsValuesProvider.sharedComponentProperties = sharedComponentProperties; - sharedComponentPropertiesBindingsValuesProvider.addBindings(bindings); - - assertEquals(sharedPropsResource, bindings.get(SharedComponentProperties.SHARED_PROPERTIES_RESOURCE)); - assertEquals(globalPropsResource, bindings.get(SharedComponentProperties.GLOBAL_PROPERTIES_RESOURCE)); - assertEquals(sharedProps, bindings.get(SharedComponentProperties.SHARED_PROPERTIES)); - assertEquals(globalProps, bindings.get(SharedComponentProperties.GLOBAL_PROPERTIES)); - } + public static final String SITE_ROOT = "/content/acs-commons"; + public static final String RESOURCE_TYPE = "acs-commons/components/content/generic-text"; + + private PageRootProvider pageRootProvider; + private Resource resource; + private Resource sharedPropsResource; + private Resource globalPropsResource; + private SlingHttpServletRequest request; + private Bindings bindings; + + private ResourceResolver resourceResolver; + private ValueMap sharedProps; + private ValueMap globalProps; + + private ValueMap localProps; + + @Before + public void setUp() throws Exception { + resource = mock(Resource.class); + pageRootProvider = mock(PageRootProvider.class); + bindings = new SimpleBindings(); + sharedPropsResource = mock(Resource.class); + globalPropsResource = mock(Resource.class); + resourceResolver = mock(ResourceResolver.class); + request = mock(SlingHttpServletRequest.class); + + final String globalPropsPath = SITE_ROOT + "/jcr:content/" + SharedComponentProperties.NN_GLOBAL_COMPONENT_PROPERTIES; + final String sharedPropsPath = SITE_ROOT + "/jcr:content/" + SharedComponentProperties.NN_SHARED_COMPONENT_PROPERTIES + "/" + + RESOURCE_TYPE; + + bindings.put(SlingBindings.REQUEST, request); + bindings.put(SlingBindings.RESOURCE, resource); + + when(resource.getResourceResolver()).thenReturn(resourceResolver); + when(resource.getResourceType()).thenReturn(RESOURCE_TYPE); + when(resourceResolver.getSearchPath()).thenReturn(new String[]{"/apps/", "/libs/"}); + when(resourceResolver.getResource(sharedPropsPath)).thenReturn(sharedPropsResource); + when(resourceResolver.getResource(globalPropsPath)).thenReturn(globalPropsResource); + + when(resource.getPath()).thenReturn(SITE_ROOT); + when(pageRootProvider.getRootPagePath(anyString())).thenReturn(SITE_ROOT); + + + sharedProps = new ValueMapDecorator(new HashMap()); + globalProps = new ValueMapDecorator(new HashMap()); + localProps = new ValueMapDecorator(new HashMap()); + + sharedProps.put("shared", "value"); + globalProps.put("global", "value"); + localProps.put("local", "value"); + + when(globalPropsResource.getValueMap()).thenReturn(globalProps); + when(sharedPropsResource.getValueMap()).thenReturn(sharedProps); + when(resource.getValueMap()).thenReturn(localProps); + } + + @Test + public void testGetCanonicalResourceTypeRelativePath() { + // make this test readable by wrapping the long method name with a function + final BiFunction, String> asFunction = + (resourceType, searchPaths) -> SharedComponentPropertiesImpl + .getCanonicalResourceTypeRelativePath(resourceType, + Optional.ofNullable(searchPaths) + .map(list -> list.toArray(new String[0])).orElse(null)); + + final List emptySearchPaths = Collections.emptyList(); + final List realSearchPaths = Arrays.asList("/apps/", "/libs/"); + assertNull("expect null for null rt", asFunction.apply(null, emptySearchPaths)); + assertNull("expect null for empty rt", asFunction.apply("", emptySearchPaths)); + assertNull("expect null for absolute rt and null search paths", + asFunction.apply("/fail/" + RESOURCE_TYPE, null)); + assertNull("expect null for cq:Page", + asFunction.apply("cq:Page", realSearchPaths)); + assertNull("expect null for nt:unstructured", + asFunction.apply("nt:unstructured", realSearchPaths)); + assertNull("expect null for absolute rt and empty search paths", + asFunction.apply("/fail/" + RESOURCE_TYPE, emptySearchPaths)); + assertNull("expect null for sling nonexisting rt", + asFunction.apply(Resource.RESOURCE_TYPE_NON_EXISTING, emptySearchPaths)); + assertEquals("expect same for relative rt", RESOURCE_TYPE, + asFunction.apply(RESOURCE_TYPE, emptySearchPaths)); + assertEquals("expect same for relative rt and real search paths", RESOURCE_TYPE, + asFunction.apply(RESOURCE_TYPE, realSearchPaths)); + assertEquals("expect relative for /apps/ + relative and real search paths", RESOURCE_TYPE, + asFunction.apply("/apps/" + RESOURCE_TYPE, realSearchPaths)); + assertEquals("expect relative for /libs/ + relative and real search paths", RESOURCE_TYPE, + asFunction.apply("/libs/" + RESOURCE_TYPE, realSearchPaths)); + assertNull("expect null for /fail/ + relative and real search paths", + asFunction.apply("/fail/" + RESOURCE_TYPE, realSearchPaths)); + } + + @Test + public void addBindings() { + final SharedComponentPropertiesImpl sharedComponentProperties = new SharedComponentPropertiesImpl(); + sharedComponentProperties.pageRootProvider = pageRootProvider; + + final SharedComponentPropertiesBindingsValuesProvider sharedComponentPropertiesBindingsValuesProvider + = new SharedComponentPropertiesBindingsValuesProvider(); + + sharedComponentPropertiesBindingsValuesProvider.sharedComponentProperties = sharedComponentProperties; + sharedComponentPropertiesBindingsValuesProvider.addBindings(bindings); + + assertEquals(sharedPropsResource, bindings.get(SharedComponentProperties.SHARED_PROPERTIES_RESOURCE)); + assertEquals(globalPropsResource, bindings.get(SharedComponentProperties.GLOBAL_PROPERTIES_RESOURCE)); + assertEquals(sharedProps, bindings.get(SharedComponentProperties.SHARED_PROPERTIES)); + assertEquals(globalProps, bindings.get(SharedComponentProperties.GLOBAL_PROPERTIES)); + + ValueMap mergedProps = (ValueMap) bindings.get(SharedComponentProperties.MERGED_PROPERTIES); + + assertEquals("value", mergedProps.get("global", String.class)); + assertEquals("value", mergedProps.get("shared", String.class)); + assertEquals("value", mergedProps.get("local", String.class)); + } + + @Test + public void addToLazyBindings() { + final SharedComponentPropertiesImpl sharedComponentProperties = new SharedComponentPropertiesImpl(); + sharedComponentProperties.pageRootProvider = pageRootProvider; + + final SharedComponentPropertiesBindingsValuesProvider sharedComponentPropertiesBindingsValuesProvider + = new SharedComponentPropertiesBindingsValuesProvider(); + sharedComponentPropertiesBindingsValuesProvider.sharedComponentProperties = sharedComponentProperties; + + // inject our own suppliers map for a side-channel to the suppliers + final Map suppliers = new HashMap<>(); + LazyBindings lazyBindings = new LazyBindings(suppliers); + lazyBindings.putAll(bindings); + sharedComponentPropertiesBindingsValuesProvider.addBindings(lazyBindings); + + // confirm that the bindings is storing a marked Supplier, rather than a resource + LazyBindings.Supplier sharedPropsObject = suppliers.get(SharedComponentProperties.SHARED_PROPERTIES_RESOURCE); + assertNotNull(sharedPropsObject); + // compare that the value returned by the supplier with the expected resource + assertEquals(sharedPropsResource, sharedPropsObject.get()); + + // confirm that the bindings is storing a marked Supplier, rather than a resource + LazyBindings.Supplier globalPropsObject = suppliers.get(SharedComponentProperties.GLOBAL_PROPERTIES_RESOURCE); + assertNotNull(globalPropsObject); + // compare that the value returned by the supplier with the expected resource + assertEquals(globalPropsResource, globalPropsObject.get()); + + // confirm that the bindings is storing a marked Supplier, rather than a ValueMap + LazyBindings.Supplier sharedPropsVmObject = suppliers.get(SharedComponentProperties.SHARED_PROPERTIES); + assertNotNull(sharedPropsVmObject); + // compare that the value returned by the supplier with the expected ValueMap + assertEquals(sharedProps, sharedPropsVmObject.get()); + + // confirm that the bindings is storing a marked Supplier, rather than a ValueMap + LazyBindings.Supplier globalPropsVmObject = suppliers.get(SharedComponentProperties.GLOBAL_PROPERTIES); + assertNotNull(globalPropsVmObject); + // compare that the value returned by the supplier with the expected ValueMap + assertEquals(globalProps, globalPropsVmObject.get()); + + // confirm that the bindings is storing a marked Supplier, rather than a resource. Acquire this Supplier BEFORE + // resetting the Global and Shared properties bindings to demonstrate that the same bindings instance + // is also accessed lazily by the Merged props supplier. + LazyBindings.Supplier mergedPropsVmObject = suppliers.get(SharedComponentProperties.MERGED_PROPERTIES); + assertNotNull(mergedPropsVmObject); + // compare that the value returned by the supplier with the expected ValueMap + ValueMap mergedProps = (ValueMap) mergedPropsVmObject.get(); + + // compare the contents of the ValueMap returned by the supplier with the expected key/values from the separate maps + assertEquals("value", mergedProps.get("global", String.class)); + assertEquals("value", mergedProps.get("shared", String.class)); + assertEquals("value", mergedProps.get("local", String.class)); + } } diff --git a/bundle/src/test/java/com/adobe/acs/commons/workflow/impl/WorkflowPackageManagerImplTest.java b/bundle/src/test/java/com/adobe/acs/commons/workflow/impl/WorkflowPackageManagerImplTest.java index a64589da35..e79d567659 100644 --- a/bundle/src/test/java/com/adobe/acs/commons/workflow/impl/WorkflowPackageManagerImplTest.java +++ b/bundle/src/test/java/com/adobe/acs/commons/workflow/impl/WorkflowPackageManagerImplTest.java @@ -31,7 +31,9 @@ import static org.mockito.Mockito.when; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; +import java.util.stream.Stream; import javax.jcr.Node; import javax.jcr.RepositoryException; @@ -189,6 +191,11 @@ public List list(final String[] strings) throws RepositoryException { return nodes; } + @Override + public boolean hasNode(String path) { + return Arrays.asList(paths).contains(path); + } + @Override public void add(final Node node) { diff --git a/oakpal-checks/pom.xml b/oakpal-checks/pom.xml index 02a4829d3d..36ac9482d1 100644 --- a/oakpal-checks/pom.xml +++ b/oakpal-checks/pom.xml @@ -131,6 +131,7 @@ + com.google.guava:* org.osgi:osgi.core @@ -293,6 +294,30 @@ osgi.core compile + + com.google.guava + guava + 31.1-jre + compile + + + com.google.j2objc + j2objc-annotations + + + com.google.code.findbugs + jsr305 + + + com.google.errorprone + error_prone_annotations + + + org.checkerframework + checker-qual + + + diff --git a/pom.xml b/pom.xml index 6011a243bf..9a40b90ced 100644 --- a/pom.xml +++ b/pom.xml @@ -467,7 +467,7 @@ Bundle-DocURL: https://adobe-consulting-services.github.io/acs-aem-commons/ io.wcm.maven io.wcm.maven.aem-dependencies - 6.4.0.0004 + 6.5.7.0002 pom import diff --git a/ui.apps/pom.xml b/ui.apps/pom.xml index 410ab8d28e..00ad19675d 100644 --- a/ui.apps/pom.xml +++ b/ui.apps/pom.xml @@ -202,7 +202,7 @@ com.adobe.aem uber-jar - apis + apis-with-deprecations provided diff --git a/ui.content/pom.xml b/ui.content/pom.xml index f426eeb450..d456af0fe9 100644 --- a/ui.content/pom.xml +++ b/ui.content/pom.xml @@ -172,7 +172,7 @@ com.adobe.aem uber-jar - apis + apis-with-deprecations provided