Merge branch 'develop'

wcm-io · Oct 20, 2022 · 7c6faac · 7c6faac
2 parents 112eb83 + 59386f6
commit 7c6faac
Show file tree

Hide file tree

Showing 22 changed files with 546 additions and 76 deletions.
diff --git a/changes.xml b/changes.xml
@@ -23,6 +23,12 @@
     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="1.14.14" date="2022-10-20">
+      <action type="fix" dev="sseifert">
+        Dynamic Media Support: Ensure smart-cropped renditions fulfill minimum size requirements.
+      </action>
+    </release>
+
     <release version="1.14.12" date="2022-10-10">
       <action type="update" dev="sseifert">
         Dynamic Media Support: Do not rely on Dynamic Media feature flag to detect DM capability on publish instances. In "AUTO" mode only the availability of DM metadata on a given asset is checked.

diff --git a/pom.xml b/pom.xml
@@ -31,7 +31,7 @@
 
   <groupId>io.wcm</groupId>
   <artifactId>io.wcm.handler.media</artifactId>
-  <version>1.14.12</version>
+  <version>1.14.14</version>
   <packaging>jar</packaging>
 
   <name>Media Handler</name>
@@ -49,7 +49,7 @@
     <site.url.module.prefix>handler/media</site.url.module.prefix>
 
     <!-- Enable reproducible builds -->
-    <project.build.outputTimestamp>2022-10-10T08:22:12Z</project.build.outputTimestamp>
+    <project.build.outputTimestamp>2022-10-20T15:25:23Z</project.build.outputTimestamp>
   </properties>
 
   <dependencies>
@@ -131,7 +131,7 @@
     <dependency>
       <groupId>io.wcm</groupId>
       <artifactId>io.wcm.testing.aem-mock.junit5</artifactId>
-      <version>5.0.0</version>
+      <version>5.1.0</version>
       <scope>test</scope>
     </dependency>
     <dependency>
@@ -161,7 +161,7 @@
     <dependency>
       <groupId>io.wcm</groupId>
       <artifactId>io.wcm.testing.wcm-io-mock.caconfig</artifactId>
-      <version>1.1.0</version>
+      <version>1.2.0</version>
       <scope>test</scope>
     </dependency>
     <dependency>

diff --git a/src/main/java/io/wcm/handler/media/format/MediaFormat.java b/src/main/java/io/wcm/handler/media/format/MediaFormat.java
@@ -213,7 +213,7 @@ public double getRatioHeightAsDouble() {
 
   /**
    * Returns the ratio defined in the media format definition.
-   * If no ratio is defined an the media format has a fixed with/height it is calculated automatically.
+   * If no ratio is defined an the media format has a fixed width/height it is calculated automatically.
    * Otherwise 0 is returned.
    * @return Ratio
    */
@@ -505,7 +505,7 @@ String getCombinedTitle() {
 
       List<String> extParts = new ArrayList<>();
 
-      // with/height restrictions
+      // width/height restrictions
       if (minWidthHeight != 0) {
         extParts.add("min. " + minWidthHeight + "px width/height");
       }

diff --git a/src/main/java/io/wcm/handler/media/impl/DummyImageServlet.java b/src/main/java/io/wcm/handler/media/impl/DummyImageServlet.java
@@ -80,7 +80,7 @@ protected Layer createLayer(ImageContext ctx) throws RepositoryException, IOExce
     int height = parser.get(SUFFIX_HEIGHT, 0);
     String name = parser.get(SUFFIX_MEDIA_FORMAT_NAME, String.class);
 
-    // validate with/height
+    // validate width/height
     if (width < 1 || height < 1) {
       return new Layer(1, 1, null);
     }

diff --git a/src/main/java/io/wcm/handler/mediasource/dam/AssetRendition.java b/src/main/java/io/wcm/handler/mediasource/dam/AssetRendition.java
@@ -94,7 +94,7 @@ private AssetRendition() {
    * </p>
    * @param rendition Rendition
    * @param suppressLogWarningNoRenditionsMetadata If set to true, no log warnings is generated when
-   *          renditions metadata containing the with/height of the rendition does not exist (yet).
+   *          renditions metadata containing the width/height of the rendition does not exist (yet).
    * @return Dimension or null if dimension could not be detected, not even in fallback mode
    */
   public static @Nullable Dimension getDimension(@NotNull Rendition rendition,
@@ -172,7 +172,7 @@ private static long getAssetMetadataValueAsLong(Asset asset, String... propertyN
    * Fallback: Read dimension by loading image binary into memory.
    * @param rendition Rendition
    * @param suppressLogWarningNoRenditionsMetadata If set to true, no log warnings is generated when
-   *          renditions metadata containing the with/height of the rendition does not exist (yet).
+   *          renditions metadata containing the width/height of the rendition does not exist (yet).
    * @return Dimension or null
    */
   @SuppressWarnings("PMD.GuardLogStatement")
@@ -202,7 +202,7 @@ private static long getAssetMetadataValueAsLong(Asset asset, String... propertyN
   }
 
   /**
-   * Convert with/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

diff --git a/src/main/java/io/wcm/handler/mediasource/dam/impl/DamAsset.java b/src/main/java/io/wcm/handler/mediasource/dam/impl/DamAsset.java
@@ -70,7 +70,7 @@ public DamAsset(Media media, com.day.cq.dam.api.Asset damAsset, MediaHandlerConf
     this.cropDimension = media.getCropDimension();
     this.rotation = media.getRotation();
     this.defaultMediaArgs = media.getMediaRequest().getMediaArgs();
-    this.damContext = new DamContext(damAsset, defaultMediaArgs.getUrlMode(), mediaHandlerConfig,
+    this.damContext = new DamContext(damAsset, defaultMediaArgs, mediaHandlerConfig,
         dynamicMediaSupportService, adaptable);
   }
 

diff --git a/src/main/java/io/wcm/handler/mediasource/dam/impl/DamContext.java b/src/main/java/io/wcm/handler/mediasource/dam/impl/DamContext.java
@@ -23,27 +23,30 @@
 import java.util.List;
 
 import org.apache.commons.lang3.StringUtils;
+import org.apache.sling.api.SlingHttpServletRequest;
 import org.apache.sling.api.adapter.Adaptable;
+import org.apache.sling.api.resource.Resource;
+import org.apache.sling.api.resource.ResourceResolver;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
 import com.day.cq.dam.api.Asset;
 import com.day.cq.dam.scene7.api.constants.Scene7Constants;
 
 import io.wcm.handler.media.Dimension;
+import io.wcm.handler.media.MediaArgs;
 import io.wcm.handler.media.spi.MediaHandlerConfig;
 import io.wcm.handler.mediasource.dam.impl.dynamicmedia.DynamicMediaSupportService;
 import io.wcm.handler.mediasource.dam.impl.dynamicmedia.ImageProfile;
 import io.wcm.handler.mediasource.dam.impl.dynamicmedia.NamedDimension;
-import io.wcm.handler.url.UrlMode;
 
 /**
  * Context objects require in DAM support implementation.
  */
 public final class DamContext implements Adaptable {
 
   private final Asset asset;
-  private final UrlMode urlMode;
+  private final MediaArgs mediaArgs;
   private final MediaHandlerConfig mediaHandlerConfig;
   private final DynamicMediaSupportService dynamicMediaSupportService;
   private final Adaptable adaptable;
@@ -62,15 +65,15 @@ public final class DamContext implements Adaptable {
 
   /**
    * @param asset DAM asset
-   * @param urlMode urlMode
+   * @param mediaArgs Media Args from media request
    * @param mediaHandlerConfig Media handler config
    * @param dynamicMediaSupportService Dynamic media support service
    * @param adaptable Adaptable from current context
    */
-  public DamContext(@NotNull Asset asset, @Nullable UrlMode urlMode, @NotNull MediaHandlerConfig mediaHandlerConfig,
+  public DamContext(@NotNull Asset asset, @NotNull MediaArgs mediaArgs, @NotNull MediaHandlerConfig mediaHandlerConfig,
       @NotNull DynamicMediaSupportService dynamicMediaSupportService, @NotNull Adaptable adaptable) {
     this.asset = asset;
-    this.urlMode = urlMode;
+    this.mediaArgs = mediaArgs;
     this.mediaHandlerConfig = mediaHandlerConfig;
     this.dynamicMediaSupportService = dynamicMediaSupportService;
     this.adaptable = adaptable;
@@ -94,8 +97,12 @@ public MediaHandlerConfig getMediaHandlerConfig() {
    * @return Whether dynamic media is enabled on this AEM instance
    */
   public boolean isDynamicMediaEnabled() {
+    // check that DM is not disabled globally
     return dynamicMediaSupportService.isDynamicMediaEnabled()
-        && dynamicMediaSupportService.isDynamicMediaCapabilityEnabled(isDynamicMediaAsset());
+        // check that DM capability is enabled for the given asset
+        && dynamicMediaSupportService.isDynamicMediaCapabilityEnabled(isDynamicMediaAsset())
+        // ensure DM is not disabled within MediaArgs for this media request
+        && !mediaArgs.isDynamicMediaDisabled();
   }
 
   /**
@@ -128,7 +135,7 @@ public boolean isDynamicMediaAsset() {
    */
   public @Nullable String getDynamicMediaServerUrl() {
     if (dynamicMediaServerUrl == null) {
-      dynamicMediaServerUrl = dynamicMediaSupportService.getDynamicMediaServerUrl(asset, urlMode, adaptable);
+      dynamicMediaServerUrl = dynamicMediaSupportService.getDynamicMediaServerUrl(asset, mediaArgs.getUrlMode(), adaptable);
     }
     return dynamicMediaServerUrl;
   }
@@ -162,6 +169,21 @@ public boolean isDynamicMediaAsset() {
     }
   }
 
+  /**
+   * @return Resource resolver from current context
+   */
+  public @NotNull ResourceResolver getResourceResolver() {
+    if (adaptable instanceof Resource) {
+      return ((Resource)adaptable).getResourceResolver();
+    }
+    else if (adaptable instanceof SlingHttpServletRequest) {
+      return ((SlingHttpServletRequest)adaptable).getResourceResolver();
+    }
+    else {
+      throw new IllegalStateException("Adaptable is neither Resoucre nor SlingHttpServletRequest");
+    }
+  }
+
   @Override
   public <AdapterType> @Nullable AdapterType adaptTo(@NotNull Class<AdapterType> type) {
     return adaptable.adaptTo(type);

diff --git a/src/main/java/io/wcm/handler/mediasource/dam/impl/DamRendition.java b/src/main/java/io/wcm/handler/mediasource/dam/impl/DamRendition.java
@@ -27,6 +27,7 @@
 import org.apache.sling.api.resource.Resource;
 import org.apache.sling.api.resource.ValueMap;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -113,42 +114,55 @@ class DamRendition extends SlingAdaptable implements Rendition {
 
   @Override
   public String getUrl() {
-    if (this.rendition == null) {
+    if (rendition == null) {
       return null;
     }
     String url = null;
-    boolean dynamicMediaEnabled = damContext.isDynamicMediaEnabled() && !mediaArgs.isDynamicMediaDisabled();
-    if (dynamicMediaEnabled && damContext.isDynamicMediaAsset()) {
-      // if DM is enabled: try to get rendition URL from dynamic media
-      String dynamicMediaPath = this.rendition.getDynamicMediaPath(this.mediaArgs.isContentDispositionAttachment(), damContext);
-      String productionAssetUrl = damContext.getDynamicMediaServerUrl();
-      if (productionAssetUrl != null) {
-        url = productionAssetUrl + dynamicMediaPath;
+    if (damContext.isDynamicMediaEnabled()) {
+      if (damContext.isDynamicMediaAsset()) {
+        url = buildDynamicMediaUrl();
+        if (url == null) {
+          // asset is valid DM asset, but no valid rendition could be generated
+          // reason might be that the smart-cropped rendition was too small for the requested size
+          return null;
+        }
       }
-    }
-    if (url == null) {
-      if (dynamicMediaEnabled) {
+      else {
+        // DM is enabled, but given asset is not a DM asset
         if (damContext.isDynamicMediaAemFallbackDisabled()) {
-          if (log.isWarnEnabled()) {
-            log.warn("Asset is not a valid DM asset, fallback disabled, rendition invalid: {}", this.rendition.getRendition().getPath());
-          }
+          log.warn("Asset is not a valid DM asset, fallback disabled, rendition invalid: {}", rendition.getRendition().getPath());
           return null;
         }
         else {
-          if (log.isTraceEnabled()) {
-            log.trace("Asset is not a valid DM asset, fallback to AEM-rendered rendition: {}", this.rendition.getRendition().getPath());
-          }
+          log.trace("Asset is not a valid DM asset, fallback to AEM-rendered rendition: {}", rendition.getRendition().getPath());
         }
       }
+    }
+    if (url == null) {
       // Render renditions in AEM: build externalized URL
       UrlHandler urlHandler = AdaptTo.notNull(damContext, UrlHandler.class);
-      String mediaPath = this.rendition.getMediaPath(this.mediaArgs.isContentDispositionAttachment());
-      url = urlHandler.get(mediaPath).urlMode(this.mediaArgs.getUrlMode())
-          .buildExternalResourceUrl(this.rendition.adaptTo(Resource.class));
+      String mediaPath = rendition.getMediaPath(mediaArgs.isContentDispositionAttachment());
+      url = urlHandler.get(mediaPath).urlMode(mediaArgs.getUrlMode())
+          .buildExternalResourceUrl(rendition.adaptTo(Resource.class));
     }
     return url;
   }
 
+  /**
+   * Build DM URL for this rendition based on the calculated DM path and the configured DM hostname.
+   * @return DM URL or null if either DM path or configured DM hostname is null
+   */
+  private @Nullable String buildDynamicMediaUrl() {
+    String dynamicMediaPath = rendition.getDynamicMediaPath(mediaArgs.isContentDispositionAttachment(), damContext);
+    String productionAssetUrl = damContext.getDynamicMediaServerUrl();
+    if (dynamicMediaPath != null && productionAssetUrl != null) {
+      return productionAssetUrl + dynamicMediaPath;
+    }
+    else {
+      return null;
+    }
+  }
+
   @Override
   public String getPath() {
     if (this.rendition != null) {

diff --git a/src/main/java/io/wcm/handler/mediasource/dam/impl/DamUriTemplate.java b/src/main/java/io/wcm/handler/mediasource/dam/impl/DamUriTemplate.java
@@ -52,7 +52,7 @@ class DamUriTemplate implements UriTemplate {
   private static String buildUriTemplate(@NotNull UriTemplateType type, @NotNull DamContext damContext,
       @NotNull MediaArgs mediaArgs) {
     String url = null;
-    if (!mediaArgs.isDynamicMediaDisabled() && damContext.isDynamicMediaEnabled() && damContext.isDynamicMediaAsset()) {
+    if (damContext.isDynamicMediaEnabled() && damContext.isDynamicMediaAsset()) {
       // if DM is enabled: try to get rendition URL from dynamic media
       String productionAssetUrl = damContext.getDynamicMediaServerUrl();
       if (productionAssetUrl != null) {

diff --git a/src/main/java/io/wcm/handler/mediasource/dam/impl/DefaultRenditionHandler.java b/src/main/java/io/wcm/handler/mediasource/dam/impl/DefaultRenditionHandler.java
@@ -333,7 +333,7 @@ else if (!candidates.isEmpty()) {
    */
   private RenditionMetadata getVirtualRendition(final Set<RenditionMetadata> candidates, MediaArgs mediaArgs) {
 
-    // get from fixed with/height
+    // get from fixed width/height
     if (mediaArgs.getFixedWidth() > 0 || mediaArgs.getFixedHeight() > 0) {
       long destWidth = mediaArgs.getFixedWidth();
       long destHeight = mediaArgs.getFixedHeight();

diff --git a/src/main/java/io/wcm/handler/mediasource/dam/impl/RenditionMetadata.java b/src/main/java/io/wcm/handler/mediasource/dam/impl/RenditionMetadata.java
@@ -28,6 +28,7 @@
 import org.apache.sling.api.adapter.SlingAdaptable;
 import org.apache.sling.api.resource.Resource;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 import com.day.cq.dam.api.Rendition;
 import com.day.image.Layer;
@@ -196,7 +197,7 @@ else if (MediaFileType.isBrowserImage(getFileExtension()) || !MediaFileType.isIm
    * @param damContext DAM context
    * @return Dynamic media path part or null if dynamic media not supported for this rendition
    */
-  public @NotNull String getDynamicMediaPath(boolean contentDispositionAttachment, DamContext damContext) {
+  public @Nullable String getDynamicMediaPath(boolean contentDispositionAttachment, DamContext damContext) {
     if (contentDispositionAttachment) {
       // serve static content from dynamic media for content disposition attachment
       return DynamicMediaPath.buildContent(damContext, true);
@@ -316,7 +317,7 @@ else if (otherIsOriginalRendition && !thisIsOriginalRendition) {
         String thisPath = getRendition().getPath();
         String otherPath = obj.getRendition().getPath();
         if (!StringUtils.equals(thisPath, otherPath)) {
-          // same with/height - compare paths as last resort
+          // same width/height - compare paths as last resort
           return thisPath.compareTo(otherPath);
         }
         else {

diff --git a/src/main/java/io/wcm/handler/mediasource/dam/impl/VirtualRenditionMetadata.java b/src/main/java/io/wcm/handler/mediasource/dam/impl/VirtualRenditionMetadata.java
@@ -24,6 +24,7 @@
 import org.apache.commons.lang3.builder.EqualsBuilder;
 import org.apache.commons.lang3.builder.HashCodeBuilder;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 import com.day.cq.dam.api.Rendition;
 import com.day.image.Layer;
@@ -88,7 +89,7 @@ public long getHeight() {
   }
 
   @Override
-  public @NotNull String getDynamicMediaPath(boolean contentDispositionAttachment, DamContext damContext) {
+  public @Nullable String getDynamicMediaPath(boolean contentDispositionAttachment, DamContext damContext) {
     if (contentDispositionAttachment) {
       // serve static content from dynamic media for content disposition attachment
       return DynamicMediaPath.buildContent(damContext, true);

diff --git a/src/main/java/io/wcm/handler/mediasource/dam/impl/VirtualTransformedRenditionMetadata.java b/src/main/java/io/wcm/handler/mediasource/dam/impl/VirtualTransformedRenditionMetadata.java
@@ -24,6 +24,7 @@
 import org.apache.commons.lang3.builder.EqualsBuilder;
 import org.apache.commons.lang3.builder.HashCodeBuilder;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 import com.day.cq.dam.api.Rendition;
 import com.day.image.Layer;
@@ -93,7 +94,7 @@ public Integer getRotation() {
   }
 
   @Override
-  public @NotNull String getDynamicMediaPath(boolean contentDispositionAttachment, DamContext damContext) {
+  public @Nullable String getDynamicMediaPath(boolean contentDispositionAttachment, DamContext damContext) {
     // render virtual rendition with dynamic media (we ignore contentDispositionAttachment here)
     return DynamicMediaPath.buildImage(damContext, getWidth(), getHeight(), this.cropDimension, this.rotation);
   }