Skip to content

Commit

Permalink
Respect Asset Compute Rendition Metadata provided by AEMaaCS (#66)
Browse files Browse the repository at this point in the history
  • Loading branch information
stefanseifert authored Aug 27, 2024
1 parent 393bf76 commit 16d251d
Show file tree
Hide file tree
Showing 6 changed files with 154 additions and 12 deletions.
7 changes: 7 additions & 0 deletions changes.xml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@
xsi:schemaLocation="http://maven.apache.org/changes/1.0.0 http://maven.apache.org/plugins/maven-changes-plugin/xsd/changes-1.0.0.xsd">
<body>

<release version="2.2.2" date="not released">
<action type="update" dev="sseifert" issue="66"><![CDATA[
Respect Asset Compute Rendition Metadata provided by AEMaaCS: If rendition dimension is available at <code>&lt;rendition&gt;/jcr:content/metadata</code>,
don't generate additional metadata by Media Handler and read it directly from there instead.
]]></action>
</release>

<release version="2.2.0" date="2024-08-26">
<action type="update" dev="cnagel" issue="62">
Web-Optimized Image Delivery: Use relative (percentage) parameters for cropping instead of absolute parameters. This should create more reliable renditions esp. for original images in low resolution.
Expand Down
38 changes: 31 additions & 7 deletions src/main/java/io/wcm/handler/mediasource/dam/AssetRendition.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import static com.day.cq.commons.jcr.JcrConstants.JCR_CONTENT;
import static com.day.cq.dam.api.DamConstants.EXIF_PIXELXDIMENSION;
import static com.day.cq.dam.api.DamConstants.EXIF_PIXELYDIMENSION;
import static com.day.cq.dam.api.DamConstants.METADATA_FOLDER;
import static com.day.cq.dam.api.DamConstants.ORIGINAL_FILE;
import static com.day.cq.dam.api.DamConstants.TIFF_IMAGELENGTH;
import static com.day.cq.dam.api.DamConstants.TIFF_IMAGEWIDTH;
Expand Down Expand Up @@ -106,9 +107,14 @@ private AssetRendition() {

// dimensions for non-original renditions only supported for image binaries
if (MediaFileType.isImage(fileExtension)) {
if (dimension == null) {
// check if rendition metadata is present in <rendition>/jcr:content/metadata provided by AEMaaCS asset compute
dimension = getDimensionFromAemRenditionMetadata(rendition);
}

if (dimension == null) {
// otherwise get from rendition metadata written by {@link DamRenditionMetadataService}
dimension = getDimensionFromRenditionMetadata(rendition);
dimension = getDimensionFromMediaHandlerRenditionMetadata(rendition);
}

// fallback: if width/height could not be read from either asset or rendition metadata load the image
Expand All @@ -131,7 +137,7 @@ private AssetRendition() {
// asset may have stored dimension in different property names
long width = getAssetMetadataValueAsLong(asset, TIFF_IMAGEWIDTH, EXIF_PIXELXDIMENSION);
long height = getAssetMetadataValueAsLong(asset, TIFF_IMAGELENGTH, EXIF_PIXELYDIMENSION);
return toDimension(width, height);
return toValidDimension(width, height);
}

private static long getAssetMetadataValueAsLong(Asset asset, String... propertyNames) {
Expand All @@ -150,15 +156,33 @@ private static long getAssetMetadataValueAsLong(Asset asset, String... propertyN
* @return Dimension or null
*/
@SuppressWarnings("java:S1075") // not a file path
private static @Nullable Dimension getDimensionFromRenditionMetadata(@NotNull Rendition rendition) {
private static @Nullable Dimension getDimensionFromMediaHandlerRenditionMetadata(@NotNull Rendition rendition) {
Asset asset = rendition.getAsset();
String metadataPath = JCR_CONTENT + "/" + NN_RENDITIONS_METADATA + "/" + rendition.getName();
Resource metadataResource = AdaptTo.notNull(asset, Resource.class).getChild(metadataPath);
if (metadataResource != null) {
ValueMap props = metadataResource.getValueMap();
long width = props.get(PN_IMAGE_WIDTH, 0L);
long height = props.get(PN_IMAGE_HEIGHT, 0L);
return toDimension(width, height);
return toValidDimension(width, height);
}
return null;
}

/**
* Asset Compute from AEMaaCS writes rendition metadata including width/height to jcr:content/metadata of the
* rendition resource - try to read it from there (it may be missing for not fully processed assets, or in local
* AEMaaCS SDK or AEM 6.5 instances).
* @param rendition Rendition
* @return Dimension or null
*/
private static @Nullable Dimension getDimensionFromAemRenditionMetadata(@NotNull Rendition rendition) {
Resource metadataResource = rendition.getChild(JCR_CONTENT + "/" + METADATA_FOLDER);
if (metadataResource != null) {
ValueMap props = metadataResource.getValueMap();
long width = props.get(TIFF_IMAGEWIDTH, 0L);
long height = props.get(TIFF_IMAGELENGTH, 0L);
return toValidDimension(width, height);
}
return null;
}
Expand All @@ -178,7 +202,7 @@ private static long getAssetMetadataValueAsLong(Asset asset, String... propertyN
Layer layer = new Layer(is);
long width = layer.getWidth();
long height = layer.getHeight();
Dimension dimension = toDimension(width, height);
Dimension dimension = toValidDimension(width, height);
if (!suppressLogWarningNoRenditionsMetadata) {
log.warn("Unable to detect rendition metadata for {}, "
+ "fallback to inefficient detection by loading image into in memory (detected dimension={}). "
Expand All @@ -198,12 +222,12 @@ private static long getAssetMetadataValueAsLong(Asset asset, String... propertyN
}

/**
* Convert width/height to dimension
* Convert width/height to dimension.
* @param width Width
* @param height Height
* @return Dimension or null if width or height are not valid
*/
private static @Nullable Dimension toDimension(long width, long height) {
private static @Nullable Dimension toValidDimension(long width, long height) {
if (width > 0L && height > 0L) {
return new Dimension(width, height);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,11 @@
import static com.day.cq.commons.jcr.JcrConstants.JCR_MIMETYPE;
import static com.day.cq.commons.jcr.JcrConstants.JCR_PRIMARYTYPE;
import static com.day.cq.commons.jcr.JcrConstants.NT_UNSTRUCTURED;
import static com.day.cq.dam.api.DamConstants.METADATA_FOLDER;
import static com.day.cq.dam.api.DamConstants.ORIGINAL_FILE;
import static com.day.cq.dam.api.DamConstants.RENDITIONS_FOLDER;
import static com.day.cq.dam.api.DamConstants.TIFF_IMAGELENGTH;
import static com.day.cq.dam.api.DamConstants.TIFF_IMAGEWIDTH;
import static io.wcm.handler.mediasource.dam.impl.metadata.RenditionMetadataNameConstants.NN_RENDITIONS_METADATA;
import static io.wcm.handler.mediasource.dam.impl.metadata.RenditionMetadataNameConstants.PN_IMAGE_HEIGHT;
import static io.wcm.handler.mediasource.dam.impl.metadata.RenditionMetadataNameConstants.PN_IMAGE_WIDTH;
Expand All @@ -47,6 +50,7 @@
import org.apache.sling.api.resource.Resource;
import org.apache.sling.api.resource.ResourceResolver;
import org.apache.sling.api.resource.ResourceUtil;
import org.apache.sling.api.resource.ValueMap;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.slf4j.Logger;
Expand Down Expand Up @@ -85,7 +89,10 @@ public RenditionMetadataGenerator(ResourceResolver resourceResolver, AssetStore
* Generate/validate rendition metadata of all renditions of this asset.
* @param asset Asset
*/
@SuppressWarnings("java:S1075") // not a file path
@SuppressWarnings({
"java:S1075", // not a file path
"java:S3776" // complexity
})
public void processAllRenditions(Asset asset) {
Set<String> existingRenditionNames = new HashSet<>();
List<String> renditionPaths = new ArrayList<>();
Expand All @@ -96,6 +103,10 @@ public void processAllRenditions(Asset asset) {

// get existing rendition names and paths
for (Rendition rendition : asset.getRenditions()) {
// skip renditions where AEMaaCS asset compute already provided metadata
if (hasAemRenditionMetadata(rendition.getPath())) {
continue;
}
existingRenditionNames.add(rendition.getName());
renditionPaths.add(rendition.getPath());
}
Expand Down Expand Up @@ -174,6 +185,12 @@ public boolean renditionAddedOrUpdated(String renditionPath) throws PersistenceE
log.debug("Skip non-image rendition {}", renditionPath);
return false;
}

// skip renditions where AEMaaCS asset compute already provided metadata
if (hasAemRenditionMetadata(renditionPath)) {
log.debug("Skip rendition with existing AEM rendition metadata {}", renditionPath);
return false;
}
}

// Compare timestamps of rendition and rendition metadata
Expand Down Expand Up @@ -257,7 +274,7 @@ public boolean renditionRemoved(String renditionPath) throws PersistenceExceptio

// check if rendition still exist (or exists again) - in this case skip removing of renditions metadata
Resource renditionResource = resourceResolver.getResource(renditionPath);
if (renditionResource != null) {
if (renditionResource != null && !hasAemRenditionMetadata(renditionResource.getPath())) {
log.debug("Skip removing of metadata for existing rendition {}", renditionPath);
return false;
}
Expand Down Expand Up @@ -304,4 +321,22 @@ private String getRenditionMetadataResourcePath(String renditionPath) {
return assetPath + "/" + JCR_CONTENT + "/" + NN_RENDITIONS_METADATA + "/" + renditionName;
}

/**
* Checks if rendition metadata provided by AEMaaCS asset compute already exists at rendition/jcr:content/metadata.
* @param renditionPath Rendition path
* @return true if metadata with a valid dimension exists
*/
@SuppressWarnings("java:S1075") // not a file path
private boolean hasAemRenditionMetadata(String renditionPath) {
String metadatPath = renditionPath + "/" + JCR_CONTENT + "/" + METADATA_FOLDER;
Resource metadataResource = resourceResolver.getResource(metadatPath);
if (metadataResource != null) {
ValueMap props = metadataResource.getValueMap();
long width = props.get(TIFF_IMAGEWIDTH, 0L);
long height = props.get(TIFF_IMAGELENGTH, 0L);
return width > 0 && height > 0;
}
return false;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
package io.wcm.handler.mediasource.dam;

import static com.day.cq.commons.jcr.JcrConstants.JCR_CONTENT;
import static com.day.cq.dam.api.DamConstants.METADATA_FOLDER;
import static com.day.cq.dam.api.DamConstants.TIFF_IMAGELENGTH;
import static com.day.cq.dam.api.DamConstants.TIFF_IMAGEWIDTH;
import static io.wcm.handler.mediasource.dam.impl.metadata.RenditionMetadataNameConstants.NN_RENDITIONS_METADATA;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
Expand Down Expand Up @@ -87,6 +90,16 @@ void testGetDimension() {
assertEquals(new Dimension(10, 5), AssetRendition.getDimension(rendition));
}

@Test
void testGetDimension_WithAemRenditionMetadata() {
// add rendition metadata as provided by AEMaaCS asset compute (has higher priority)
context.create().resource(rendition, JCR_CONTENT + "/" + METADATA_FOLDER,
TIFF_IMAGEWIDTH, 100, TIFF_IMAGELENGTH, 50);

assertEquals(new Dimension(16, 9), AssetRendition.getDimension(original));
assertEquals(new Dimension(100, 50), AssetRendition.getDimension(rendition));
}

@Test
@SuppressWarnings("null")
void testGetDimension_SVG() {
Expand Down Expand Up @@ -115,6 +128,22 @@ void testGetDimensionWithoutRenditionsMetadata() throws PersistenceException {
assertEquals(new Dimension(10, 5), AssetRendition.getDimension(rendition, true));
}

@Test
void testGetDimensionWithoutRenditionsMetadata_WithAemRenditionMetadata() throws PersistenceException {
// remove renditions metadata generated by DamRenditionMetadataService
Resource renditionsMetadata = AdaptTo.notNull(asset, Resource.class).getChild(JCR_CONTENT + "/" + NN_RENDITIONS_METADATA);
if (renditionsMetadata != null) {
context.resourceResolver().delete(renditionsMetadata);
}

// add rendition metadata as provided by AEMaaCS asset compute
context.create().resource(rendition, JCR_CONTENT + "/" + METADATA_FOLDER,
TIFF_IMAGEWIDTH, 100, TIFF_IMAGELENGTH, 50);

assertEquals(new Dimension(16, 9), AssetRendition.getDimension(original, true));
assertEquals(new Dimension(100, 50), AssetRendition.getDimension(rendition, true));
}

@Test
void testIsThumbnailRendition() {
assertTrue(AssetRendition.isThumbnailRendition(renditionByName("cq5dam.thumbnail.10.10.png")));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,12 @@
*/
package io.wcm.handler.mediasource.dam.impl.metadata;

import static com.day.cq.commons.jcr.JcrConstants.JCR_CONTENT;
import static com.day.cq.commons.jcr.JcrConstants.JCR_LASTMODIFIED;
import static com.day.cq.commons.jcr.JcrConstants.JCR_LAST_MODIFIED_BY;
import static com.day.cq.dam.api.DamConstants.METADATA_FOLDER;
import static com.day.cq.dam.api.DamConstants.TIFF_IMAGELENGTH;
import static com.day.cq.dam.api.DamConstants.TIFF_IMAGEWIDTH;
import static io.wcm.handler.mediasource.dam.impl.metadata.RenditionMetadataNameConstants.NN_RENDITIONS_METADATA;
import static io.wcm.handler.mediasource.dam.impl.metadata.RenditionMetadataNameConstants.PN_IMAGE_HEIGHT;
import static io.wcm.handler.mediasource.dam.impl.metadata.RenditionMetadataNameConstants.PN_IMAGE_WIDTH;
Expand Down Expand Up @@ -78,6 +82,14 @@ void testAddRendition_Metadata() {
assertRenditionMetadata("test.jpg", 215, 102, true);
}

@Test
void testAddRendition_Metadata_ExistingAemRenditionMetadata() {
underTest = context.registerInjectActivateService(new RenditionMetadataListenerService(),
"threadPoolSize", 0);
addRenditionWithAemRenditionMetadata("test.jpg");
assertNoRenditionMetadata("test.jpg");
}

@Test
@SuppressWarnings("null")
void testAddRendition_Metadata_createMetadataNode() throws PersistenceException {
Expand Down Expand Up @@ -208,6 +220,13 @@ private void addRendition(String renditionName) {
underTest.handleEvent(DamEvent.renditionUpdated(assetResource.getPath(), null, rendition.getPath()).toEvent());
}

private void addRenditionWithAemRenditionMetadata(String renditionName) {
Resource rendition = context.load().binaryFile("/sample_image_215x102.jpg", RENDITIONS_PATH + "/" + renditionName);
context.create().resource(rendition, JCR_CONTENT + "/" + METADATA_FOLDER,
TIFF_IMAGEWIDTH, 215, TIFF_IMAGELENGTH, 102);
underTest.handleEvent(DamEvent.renditionUpdated(assetResource.getPath(), null, rendition.getPath()).toEvent());
}

@SuppressWarnings("null")
private void updateRendition(String renditionName) throws PersistenceException {
String renditionPath = RENDITIONS_PATH + "/" + renditionName;
Expand All @@ -233,7 +252,6 @@ private void sendRenditionRemovedEvent(String renditionName) {
underTest.handleEvent(DamEvent.renditionRemoved(assetResource.getPath(), null, renditionPath).toEvent());
}

@SuppressWarnings("null")
private void assertRenditionMetadata(String renditionName, int width, int height, boolean withLastModified) {
String path = RENDITIONS_METADATA_PATH + "/" + renditionName;
Resource metadata = context.resourceResolver().getResource(path);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,12 @@
*/
package io.wcm.handler.mediasource.dam.impl.metadata;

import static com.day.cq.commons.jcr.JcrConstants.JCR_CONTENT;
import static com.day.cq.commons.jcr.JcrConstants.JCR_LASTMODIFIED;
import static com.day.cq.commons.jcr.JcrConstants.JCR_LAST_MODIFIED_BY;
import static com.day.cq.dam.api.DamConstants.METADATA_FOLDER;
import static com.day.cq.dam.api.DamConstants.TIFF_IMAGELENGTH;
import static com.day.cq.dam.api.DamConstants.TIFF_IMAGEWIDTH;
import static io.wcm.handler.mediasource.dam.impl.metadata.RenditionMetadataNameConstants.NN_RENDITIONS_METADATA;
import static io.wcm.handler.mediasource.dam.impl.metadata.RenditionMetadataNameConstants.PN_IMAGE_HEIGHT;
import static io.wcm.handler.mediasource.dam.impl.metadata.RenditionMetadataNameConstants.PN_IMAGE_WIDTH;
Expand Down Expand Up @@ -49,6 +53,7 @@
import com.adobe.granite.workflow.exec.WorkflowData;
import com.adobe.granite.workflow.metadata.MetaDataMap;
import com.day.cq.dam.api.Asset;
import com.day.cq.dam.api.Rendition;

import io.wcm.handler.media.testcontext.AppAemContext;
import io.wcm.testing.mock.aem.junit5.AemContext;
Expand Down Expand Up @@ -96,18 +101,27 @@ void testWithAssetPayload() {
context.create().assetRendition(asset, "rendition1.jpg", 12, 12, "image/jpg");
context.create().assetRendition(asset, "rendition2.png", 10, 5, "image/png");
context.create().assetRendition(asset, "rendition3.txt", "/sample.txt", "text/plain");

// prepare rendition with asset compute metadata
Rendition rendition4 = context.create().assetRendition(asset, "rendition4.jpg", 15, 15, "image/jpg");
context.create().resource(rendition4, JCR_CONTENT + "/" + METADATA_FOLDER,
TIFF_IMAGEWIDTH, 15, TIFF_IMAGELENGTH, 15);

when(workflowData.getPayload()).thenReturn(asset.getPath());

assertNoRenditionMetadata(asset, "rendition1.jpg");
assertNoRenditionMetadata(asset, "rendition2.png");
assertNoRenditionMetadata(asset, "rendition3.txt");
assertNoRenditionMetadata(asset, "rendition4.jpg");

underTest.execute(workItem, workflowSession, metaDataMap);

assertRenditionMetadata(asset, "rendition1.jpg", 12, 12);
assertRenditionMetadata(asset, "rendition2.png", 10, 5);
// no metadata expected for non-image rendition
assertNoRenditionMetadata(asset, "rendition3.txt");
// no metadata expected when asset compute metadata is present
assertNoRenditionMetadata(asset, "rendition4.jpg");
}

@Test
Expand Down Expand Up @@ -150,26 +164,41 @@ void testWithWorkflowPackagePayload() {
void testRemovalOfObsoleteRenditionMetadata() {
Asset asset = context.create().asset("/content/dam/asset1.jpg", 10, 10, "image/jpeg");
context.create().assetRendition(asset, "rendition1.jpg", 12, 12, "image/jpg");

// prepare rendition with asset compute metadata
Rendition rendition4 = context.create().assetRendition(asset, "rendition4.jpg", 15, 15, "image/jpg");
context.create().resource(rendition4, JCR_CONTENT + "/" + METADATA_FOLDER,
TIFF_IMAGEWIDTH, 15, TIFF_IMAGELENGTH, 15);

when(workflowData.getPayload()).thenReturn(asset.getPath());

// create some obsolete rendition metadata with no rendition attached
String obsoletePath = asset.getPath() + "/jcr:content/" + NN_RENDITIONS_METADATA + "/obsolete.jpg";
String obsoletePath = asset.getPath() + "/" + JCR_CONTENT + "/" + NN_RENDITIONS_METADATA + "/obsolete.jpg";
context.create().resource(obsoletePath,
PN_IMAGE_WIDTH, 20,
PN_IMAGE_HEIGHT, 10,
JCR_LASTMODIFIED, Calendar.getInstance(),
JCR_LAST_MODIFIED_BY, "dummy");

// create some obsolete rendition metadata for rendition with asset compute metadata
obsoletePath = asset.getPath() + "/" + JCR_CONTENT + "/" + NN_RENDITIONS_METADATA + "/rendition4.jpg";
context.create().resource(obsoletePath,
PN_IMAGE_WIDTH, 15,
PN_IMAGE_HEIGHT, 15,
JCR_LASTMODIFIED, Calendar.getInstance(),
JCR_LAST_MODIFIED_BY, "dummy");

assertNoRenditionMetadata(asset, "rendition1.jpg");
assertRenditionMetadata(asset, "obsolete.jpg", 20, 10);
assertRenditionMetadata(asset, "rendition4.jpg", 15, 15);

underTest.execute(workItem, workflowSession, metaDataMap);

assertRenditionMetadata(asset, "rendition1.jpg", 12, 12);
assertNoRenditionMetadata(asset, "obsolete.jpg");
assertNoRenditionMetadata(asset, "rendition4.jpg");
}

@SuppressWarnings("null")
private void assertRenditionMetadata(Asset asset, String renditionName, int width, int height) {
String renditionPath = asset.getPath() + "/jcr:content/" + NN_RENDITIONS_METADATA + "/" + renditionName;
Resource metadata = context.resourceResolver().getResource(renditionPath);
Expand Down

0 comments on commit 16d251d

Please sign in to comment.