From 452823e5e06e6a2e2f4768ae3396e985d5b573b2 Mon Sep 17 00:00:00 2001 From: Stefan Seifert Date: Mon, 9 Sep 2024 14:15:24 +0200 Subject: [PATCH] Dynamic Media with Open API: Correctly calculate rendition width/height based on requested dimension or original dimension (#67) --- changes.xml | 3 + .../ngdm/NextGenDynamicMediaRendition.java | 98 +++++++++++++------ ...namicMedia_LocalAssetWithMetadataTest.java | 93 +++++++++++++++--- ...amicMedia_RemoteAssetWithMetadataTest.java | 28 ++++++ ...cMedia_RemoteAssetWithoutMetadataTest.java | 61 ++++++++++++ 5 files changed, 239 insertions(+), 44 deletions(-) diff --git a/changes.xml b/changes.xml index 14ab95bf..0cc0eb85 100644 --- a/changes.xml +++ b/changes.xml @@ -24,6 +24,9 @@ + + Dynamic Media with Open API: Correctly calculate rendition width/height based on requested dimension or original dimension. + <rendition>/jcr:content/metadata, don't generate additional metadata by Media Handler and read it directly from there instead. diff --git a/src/main/java/io/wcm/handler/mediasource/ngdm/NextGenDynamicMediaRendition.java b/src/main/java/io/wcm/handler/mediasource/ngdm/NextGenDynamicMediaRendition.java index 9287371f..221c71a3 100644 --- a/src/main/java/io/wcm/handler/mediasource/ngdm/NextGenDynamicMediaRendition.java +++ b/src/main/java/io/wcm/handler/mediasource/ngdm/NextGenDynamicMediaRendition.java @@ -37,6 +37,7 @@ import io.wcm.handler.media.UriTemplate; import io.wcm.handler.media.UriTemplateType; import io.wcm.handler.media.format.MediaFormat; +import io.wcm.handler.media.format.Ratio; import io.wcm.handler.media.impl.ImageQualityPercentage; import io.wcm.handler.mediasource.ngdm.impl.MediaArgsDimension; import io.wcm.handler.mediasource.ngdm.impl.NextGenDynamicMediaBinaryUrlBuilder; @@ -58,6 +59,8 @@ final class NextGenDynamicMediaRendition implements Rendition { private final MediaArgs mediaArgs; private final String url; private MediaFormat resolvedMediaFormat; + private long requestedWidth; + private long requestedHeight; private long width; private long height; private String fileExtension; @@ -75,16 +78,16 @@ final class NextGenDynamicMediaRendition implements Rendition { } this.reference = context.getReference(); this.mediaArgs = mediaArgs; - this.width = mediaArgs.getFixedWidth(); - this.height = mediaArgs.getFixedHeight(); + this.requestedWidth = mediaArgs.getFixedWidth(); + this.requestedHeight = mediaArgs.getFixedHeight(); // set first media format as resolved format - because only the first is supported MediaFormat firstMediaFormat = MediaArgsDimension.getFirstMediaFormat(mediaArgs); if (firstMediaFormat != null) { this.resolvedMediaFormat = firstMediaFormat; - if (this.width == 0) { - this.width = firstMediaFormat.getEffectiveMinWidth(); - this.height = firstMediaFormat.getEffectiveMinHeight(); + if (this.requestedWidth == 0) { + this.requestedWidth = firstMediaFormat.getEffectiveMinWidth(); + this.requestedHeight = firstMediaFormat.getEffectiveMinHeight(); } } @@ -97,37 +100,76 @@ final class NextGenDynamicMediaRendition implements Rendition { // deliver as binary this.url = buildBinaryUrl(); } - else if (isRequestedDimensionLargerThanOriginal()) { - // image upscaling is not supported - this.url = null; - } else { - // deliver scaled image rendition - this.url = buildImageRenditionUrl(); - this.fileExtension = new NextGenDynamicMediaImageUrlBuilder(context).getFileExtension(); + // calculate width/height for rendition metadata + calculateWidthHeight(); + if (isRequestedDimensionLargerThanOriginal()) { + // image upscaling is not supported + this.url = null; + } + else { + // deliver scaled image rendition + this.url = buildImageRenditionUrl(); + this.fileExtension = new NextGenDynamicMediaImageUrlBuilder(context).getFileExtension(); + } } } /** - * Build image rendition URL which is dynamically scaled and/or cropped. + * Recalculates width and/or height based on requested media format, ratio and original dimensions. */ - private String buildImageRenditionUrl() { - // calculate height - if (this.width > 0) { - double ratio = MediaArgsDimension.getRequestedRatio(mediaArgs); - if (ratio > 0) { - this.height = Math.round(this.width / ratio); + private void calculateWidthHeight() { + double requestedRatio = MediaArgsDimension.getRequestedRatio(mediaArgs); + + // use given width/height if fixed dimension is requested + if (requestedWidth > 0 && requestedHeight > 0) { + this.width = requestedWidth; + this.height = requestedHeight; + } + + // set original sizes if not width/height is requested + else if (this.requestedWidth == 0 && this.requestedHeight == 0 && this.originalDimension != null) { + this.width = this.originalDimension.getWidth(); + this.height = this.originalDimension.getHeight(); + } + + // calculate height if only width is requested + else if (this.requestedWidth > 0 && this.requestedHeight == 0) { + this.width = requestedWidth; + if (requestedRatio > 0) { + this.height = Math.round(this.requestedWidth / requestedRatio); + this.requestedHeight = this.height; + } + else if (originalDimension != null) { + this.height = Math.round(this.requestedWidth / Ratio.get(originalDimension)); } } + // calculate width if only height is requested + else if (this.requestedHeight > 0 && this.requestedWidth == 0) { + this.height = requestedHeight; + if (requestedRatio > 0) { + this.width = Math.round(this.requestedHeight * requestedRatio); + this.requestedWidth = this.width; + } + else if (originalDimension != null) { + this.width = Math.round(this.requestedHeight * Ratio.get(originalDimension)); + } + } + } + + /** + * Build image rendition URL which is dynamically scaled and/or cropped. + */ + private String buildImageRenditionUrl() { NextGenDynamicMediaImageDeliveryParams params = new NextGenDynamicMediaImageDeliveryParams() .rotation(context.getMedia().getRotation()) .quality(ImageQualityPercentage.getAsInteger(mediaArgs, context.getMediaHandlerConfig())); - if (this.width > 0) { - params.width(this.width); + if (this.requestedWidth > 0) { + params.width(this.requestedWidth); } - if (this.height > 0) { - params.height(this.height); + if (this.requestedHeight > 0) { + params.height(this.requestedHeight); } Dimension ratioDimension = MediaArgsDimension.getRequestedRatioAsWidthHeight(mediaArgs); if (ratioDimension != null) { @@ -144,10 +186,10 @@ private String buildImageRenditionUrl() { */ private boolean isRequestedDimensionLargerThanOriginal() { if (originalDimension != null - && (this.width > originalDimension.getWidth() || this.height > originalDimension.getHeight())) { + && (this.requestedWidth > originalDimension.getWidth() || this.requestedHeight > originalDimension.getHeight())) { if (log.isTraceEnabled()) { log.trace("Requested dimension {} is larger than original image dimension {} of {}", - new Dimension(this.width, this.height), originalDimension, context.getReference()); + new Dimension(this.requestedWidth, this.requestedHeight), originalDimension, context.getReference()); } return true; } @@ -230,17 +272,11 @@ public boolean isDownload() { @Override public long getWidth() { - if (width == 0 && originalDimension != null) { - return originalDimension.getWidth(); - } return width; } @Override public long getHeight() { - if (height == 0 && originalDimension != null) { - return originalDimension.getHeight(); - } return height; } diff --git a/src/test/java/io/wcm/handler/mediasource/ngdm/NextGenDynamicMedia_LocalAssetWithMetadataTest.java b/src/test/java/io/wcm/handler/mediasource/ngdm/NextGenDynamicMedia_LocalAssetWithMetadataTest.java index 592cc327..f570e4ac 100644 --- a/src/test/java/io/wcm/handler/mediasource/ngdm/NextGenDynamicMedia_LocalAssetWithMetadataTest.java +++ b/src/test/java/io/wcm/handler/mediasource/ngdm/NextGenDynamicMedia_LocalAssetWithMetadataTest.java @@ -22,14 +22,18 @@ import static com.day.cq.dam.api.DamConstants.ASSET_STATUS_APPROVED; import static com.day.cq.dam.api.DamConstants.ASSET_STATUS_PROPERTY; import static io.wcm.handler.mediasource.ngdm.impl.NextGenDynamicMediaReferenceSample.SAMPLE_ASSET_ID; +import static io.wcm.handler.mediasource.ngdm.impl.NextGenDynamicMediaReferenceSample.SAMPLE_FILENAME; import static io.wcm.handler.mediasource.ngdm.impl.NextGenDynamicMediaReferenceSample.SAMPLE_REFERENCE; import static io.wcm.handler.mediasource.ngdm.impl.NextGenDynamicMediaReferenceSample.SAMPLE_UUID; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import org.apache.sling.api.resource.ModifiableValueMap; import org.apache.sling.api.resource.Resource; +import org.apache.sling.api.resource.ValueMap; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -44,6 +48,7 @@ import io.wcm.handler.media.UriTemplate; import io.wcm.handler.media.UriTemplateType; import io.wcm.handler.media.testcontext.AppAemContext; +import io.wcm.handler.media.testcontext.DummyMediaFormats; import io.wcm.handler.mediasource.ngdm.impl.NextGenDynamicMediaConfigServiceImpl; import io.wcm.sling.commons.adapter.AdaptTo; import io.wcm.testing.mock.aem.junit5.AemContext; @@ -73,15 +78,7 @@ void setUp() { @Test @SuppressWarnings("null") void testLocalAsset() { - com.day.cq.dam.api.Asset asset = context.create().asset("/content/dam/my-image.jpg", 20, 10, ContentType.JPEG, - ASSET_STATUS_PROPERTY, ASSET_STATUS_APPROVED); - ModifiableValueMap props = AdaptTo.notNull(asset, ModifiableValueMap.class); - props.put(JcrConstants.JCR_UUID, SAMPLE_UUID); - - resource = context.create().resource(context.currentPage(), "local-asset", - MediaNameConstants.PN_MEDIA_REF, asset.getPath()); - - Media media = mediaHandler.get(resource) + Media media = mediaHandler.get(prepareResourceWithApprovedLocalAsset()) .build(); assertTrue(media.isValid()); assertUrl(media, "preferwebp=true&quality=85", "jpg"); @@ -93,15 +90,74 @@ void testLocalAsset() { assertEquals("https://repo1/adobe/assets/" + SAMPLE_ASSET_ID + "/as/my-image.jpg?preferwebp=true&quality=85&width={width}", uriTemplateScaleWidth.getUriTemplate()); assertEquals(UriTemplateType.SCALE_WIDTH, uriTemplateScaleWidth.getType()); - assertEquals(20, uriTemplateScaleWidth.getMaxWidth()); - assertEquals(10, uriTemplateScaleWidth.getMaxHeight()); + assertEquals(1200, uriTemplateScaleWidth.getMaxWidth()); + assertEquals(800, uriTemplateScaleWidth.getMaxHeight()); UriTemplate uriTemplateScaleHeight = rendition.getUriTemplate(UriTemplateType.SCALE_HEIGHT); assertEquals("https://repo1/adobe/assets/" + SAMPLE_ASSET_ID + "/as/my-image.jpg?height={height}&preferwebp=true&quality=85", uriTemplateScaleHeight.getUriTemplate()); assertEquals(UriTemplateType.SCALE_HEIGHT, uriTemplateScaleHeight.getType()); - assertEquals(20, uriTemplateScaleHeight.getMaxWidth()); - assertEquals(10, uriTemplateScaleHeight.getMaxHeight()); + assertEquals(1200, uriTemplateScaleHeight.getMaxWidth()); + assertEquals(800, uriTemplateScaleHeight.getMaxHeight()); + } + + @Test + void testRendition_SetWidth() { + Media media = mediaHandler.get(prepareResourceWithApprovedLocalAsset()) + .fixedWidth(120) + .build(); + assertTrue(media.isValid()); + assertUrl(media, "preferwebp=true&quality=85&width=120", "jpg"); + + Rendition rendition = media.getRendition(); + assertNotNull(rendition); + assertEquals(120, rendition.getWidth()); + assertEquals(80, rendition.getHeight()); + } + + @Test + void testRendition_SetHeight() { + Media media = mediaHandler.get(prepareResourceWithApprovedLocalAsset()) + .fixedHeight(80) + .build(); + assertTrue(media.isValid()); + assertUrl(media, "height=80&preferwebp=true&quality=85", "jpg"); + + Rendition rendition = media.getRendition(); + assertNotNull(rendition); + assertEquals(120, rendition.getWidth()); + assertEquals(80, rendition.getHeight()); + } + + @Test + void testRendition_16_9() { + Media media = mediaHandler.get(prepareResourceWithApprovedLocalAsset()) + .mediaFormat(DummyMediaFormats.RATIO_16_9) + .fixedWidth(1024) + .build(); + assertTrue(media.isValid()); + assertUrl(media, "crop=0%2C63%2C1200%2C675&preferwebp=true&quality=85&width=1024", "jpg"); + + Rendition rendition = media.getRendition(); + assertNotNull(rendition); + + assertNull(rendition.getPath()); + assertEquals(SAMPLE_FILENAME, rendition.getFileName()); + assertEquals("jpg", rendition.getFileExtension()); + assertEquals(-1, rendition.getFileSize()); + assertEquals(ContentType.JPEG, rendition.getMimeType()); + assertEquals(DummyMediaFormats.RATIO_16_9, rendition.getMediaFormat()); + assertEquals(ValueMap.EMPTY, rendition.getProperties()); + assertTrue(rendition.isImage()); + assertTrue(rendition.isBrowserImage()); + assertFalse(rendition.isVectorImage()); + assertFalse(rendition.isDownload()); + assertEquals(1024, rendition.getWidth()); + assertEquals(576, rendition.getHeight()); + assertNull(rendition.getModificationDate()); + assertFalse(rendition.isFallback()); + assertNull(rendition.adaptTo(Resource.class)); + assertNotNull(rendition.toString()); } @Test @@ -144,4 +200,15 @@ private static String buildUrl(String urlParams, String extension) { + extension + "?" + urlParams; } + @SuppressWarnings("null") + private Resource prepareResourceWithApprovedLocalAsset() { + com.day.cq.dam.api.Asset asset = context.create().asset("/content/dam/my-image.jpg", 1200, 800, ContentType.JPEG, + ASSET_STATUS_PROPERTY, ASSET_STATUS_APPROVED); + ModifiableValueMap props = AdaptTo.notNull(asset, ModifiableValueMap.class); + props.put(JcrConstants.JCR_UUID, SAMPLE_UUID); + + return context.create().resource(context.currentPage(), "local-asset", + MediaNameConstants.PN_MEDIA_REF, asset.getPath()); + } + } diff --git a/src/test/java/io/wcm/handler/mediasource/ngdm/NextGenDynamicMedia_RemoteAssetWithMetadataTest.java b/src/test/java/io/wcm/handler/mediasource/ngdm/NextGenDynamicMedia_RemoteAssetWithMetadataTest.java index 2d75fcc6..a1076417 100644 --- a/src/test/java/io/wcm/handler/mediasource/ngdm/NextGenDynamicMedia_RemoteAssetWithMetadataTest.java +++ b/src/test/java/io/wcm/handler/mediasource/ngdm/NextGenDynamicMedia_RemoteAssetWithMetadataTest.java @@ -131,6 +131,34 @@ void testAsset() { assertNull(tooLargeRendition); } + @Test + void testRendition_SetWidth() { + Media media = mediaHandler.get(resource) + .fixedWidth(120) + .build(); + assertTrue(media.isValid()); + assertUrl(media, "preferwebp=true&quality=85&width=120", "jpg"); + + Rendition rendition = media.getRendition(); + assertNotNull(rendition); + assertEquals(120, rendition.getWidth()); + assertEquals(80, rendition.getHeight()); + } + + @Test + void testRendition_SetHeight() { + Media media = mediaHandler.get(resource) + .fixedHeight(80) + .build(); + assertTrue(media.isValid()); + assertUrl(media, "height=80&preferwebp=true&quality=85", "jpg"); + + Rendition rendition = media.getRendition(); + assertNotNull(rendition); + assertEquals(120, rendition.getWidth()); + assertEquals(80, rendition.getHeight()); + } + @Test void testRendition_16_9() { Media media = mediaHandler.get(resource) diff --git a/src/test/java/io/wcm/handler/mediasource/ngdm/NextGenDynamicMedia_RemoteAssetWithoutMetadataTest.java b/src/test/java/io/wcm/handler/mediasource/ngdm/NextGenDynamicMedia_RemoteAssetWithoutMetadataTest.java index 2cbf1e53..3176cf29 100644 --- a/src/test/java/io/wcm/handler/mediasource/ngdm/NextGenDynamicMedia_RemoteAssetWithoutMetadataTest.java +++ b/src/test/java/io/wcm/handler/mediasource/ngdm/NextGenDynamicMedia_RemoteAssetWithoutMetadataTest.java @@ -23,6 +23,7 @@ import static io.wcm.handler.mediasource.ngdm.impl.NextGenDynamicMediaReferenceSample.SAMPLE_FILENAME; import static io.wcm.handler.mediasource.ngdm.impl.NextGenDynamicMediaReferenceSample.SAMPLE_REFERENCE; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -42,6 +43,7 @@ import io.wcm.handler.media.UriTemplate; import io.wcm.handler.media.UriTemplateType; import io.wcm.handler.media.testcontext.AppAemContext; +import io.wcm.handler.media.testcontext.DummyMediaFormats; import io.wcm.handler.mediasource.ngdm.impl.NextGenDynamicMediaConfigServiceImpl; import io.wcm.sling.commons.adapter.AdaptTo; import io.wcm.testing.mock.aem.dam.ngdm.MockNextGenDynamicMediaConfig; @@ -113,6 +115,65 @@ void testAsset() { "preferwebp=true&quality=85&width={width}", "jpg"); } + @Test + void testRendition_SetWidth() { + Media media = mediaHandler.get(resource) + .fixedWidth(120) + .build(); + assertTrue(media.isValid()); + assertUrl(media, "preferwebp=true&quality=85&width=120", "jpg"); + + Rendition rendition = media.getRendition(); + assertNotNull(rendition); + assertEquals(120, rendition.getWidth()); + assertEquals(0, rendition.getHeight()); + } + + @Test + void testRendition_SetHeight() { + Media media = mediaHandler.get(resource) + .fixedHeight(80) + .build(); + assertTrue(media.isValid()); + assertUrl(media, "height=80&preferwebp=true&quality=85", "jpg"); + + Rendition rendition = media.getRendition(); + assertNotNull(rendition); + assertEquals(0, rendition.getWidth()); + assertEquals(80, rendition.getHeight()); + } + + @Test + void testRendition_16_9() { + Media media = mediaHandler.get(resource) + .mediaFormat(DummyMediaFormats.RATIO_16_9) + .fixedWidth(1024) + .build(); + assertTrue(media.isValid()); + + Rendition rendition = media.getRendition(); + assertNotNull(rendition); + assertUrl(rendition, "height=576&preferwebp=true&quality=85&width=1024", "jpg"); + + assertNull(rendition.getPath()); + assertEquals(SAMPLE_FILENAME, rendition.getFileName()); + assertEquals("jpg", rendition.getFileExtension()); + assertEquals(-1, rendition.getFileSize()); + assertEquals(ContentType.JPEG, rendition.getMimeType()); + assertEquals(DummyMediaFormats.RATIO_16_9, rendition.getMediaFormat()); + assertEquals(ValueMap.EMPTY, rendition.getProperties()); + assertTrue(rendition.isImage()); + assertTrue(rendition.isBrowserImage()); + assertFalse(rendition.isVectorImage()); + assertFalse(rendition.isDownload()); + assertEquals(1024, rendition.getWidth()); + assertEquals(576, rendition.getHeight()); + assertNull(rendition.getModificationDate()); + assertFalse(rendition.isFallback()); + assertNull(rendition.adaptTo(Resource.class)); + assertNotNull(rendition.toString()); + } + @Test @SuppressWarnings("null") void testPDFDownload() {