From 79698488cdc2f55ea322e40e12c1f79cbbe8a1c0 Mon Sep 17 00:00:00 2001
From: Alan Davis <alan.davis@alfresco.com>
Date: Wed, 1 Apr 2020 15:48:39 +0100
Subject: [PATCH] MNT-21398 Unable to override standard rendition definitions
 (#913)

Relaxed the checks so that it is possible to override rendition definitions. DEBUG is logged in this case.
---
 .../RenditionDefinitionRegistry2Impl.java        | 15 +++++++++------
 .../repo/rendition2/RenditionService2Test.java   | 12 +++++++++++-
 .../renditions/test/overrideRendition.json       | 16 ++++++++++++++++
 3 files changed, 36 insertions(+), 7 deletions(-)
 create mode 100644 src/test/resources/alfresco/renditions/test/overrideRendition.json

diff --git a/src/main/java/org/alfresco/repo/rendition2/RenditionDefinitionRegistry2Impl.java b/src/main/java/org/alfresco/repo/rendition2/RenditionDefinitionRegistry2Impl.java
index e0d740fb07..e1711cb9bf 100644
--- a/src/main/java/org/alfresco/repo/rendition2/RenditionDefinitionRegistry2Impl.java
+++ b/src/main/java/org/alfresco/repo/rendition2/RenditionDefinitionRegistry2Impl.java
@@ -252,8 +252,13 @@ public class RenditionDefinitionRegistry2Impl implements RenditionDefinitionRegi
                             {
                                 map.put(RenditionDefinition2.TIMEOUT, timeoutDefault);
                             }
+                            RenditionDefinition2 original = getRenditionDefinition(def.renditionName);
                             new RenditionDefinition2Impl(def.renditionName, def.targetMediaType, map, true,
                                     RenditionDefinitionRegistry2Impl.this);
+                            if (original != null)
+                            {
+                                log.debug(readFromMessage+" replaced the rendition "+def.renditionName);
+                            }
                         }
                     }
                 }
@@ -314,15 +319,13 @@ public class RenditionDefinitionRegistry2Impl implements RenditionDefinitionRegi
     public void register(RenditionDefinition2 renditionDefinition)
     {
         String renditionName = renditionDefinition.getRenditionName();
-        RenditionDefinition2 original = getDefinition(renditionName);
-        if (original != null)
-        {
-            throw new IllegalArgumentException("RenditionDefinition "+renditionName+" was already registered.");
-        }
         Data data = getData();
+        // There may already be a rendition defined, but an extension may replace it.
+        // This is logged in a caller of this method were the file name is known.
         data.renditionDefinitions.put(renditionName, renditionDefinition);
+
         if (renditionDefinition instanceof RenditionDefinition2Impl &&
-                !((RenditionDefinition2Impl)renditionDefinition).isDynamicallyLoaded())
+            !((RenditionDefinition2Impl)renditionDefinition).isDynamicallyLoaded())
         {
             log.debug("Adding static rendition "+renditionName+" into the registry");
             data.staticCount++;
diff --git a/src/test/java/org/alfresco/repo/rendition2/RenditionService2Test.java b/src/test/java/org/alfresco/repo/rendition2/RenditionService2Test.java
index ed6e95adba..ec2eda7cb6 100644
--- a/src/test/java/org/alfresco/repo/rendition2/RenditionService2Test.java
+++ b/src/test/java/org/alfresco/repo/rendition2/RenditionService2Test.java
@@ -56,6 +56,7 @@ import java.util.Map;
 import java.util.Set;
 
 import static junit.framework.TestCase.assertTrue;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyInt;
@@ -123,7 +124,6 @@ public class RenditionService2Test
         when(nodeService.exists(nodeRef)).thenReturn(true);
         when(nodeService.exists(nodeRefMissing)).thenReturn(false);
         when(nodeService.getProperty(nodeRef, ContentModel.PROP_CONTENT)).thenReturn(contentData);
-        when(nodeService.getProperty(nodeRef, ContentModel.PROP_MODIFIED)).thenReturn(new Date());
         when(contentData.getContentUrl()).thenReturn(contentUrl);
 
         doAnswer(invocation ->
@@ -155,6 +155,7 @@ public class RenditionService2Test
         renditionService2.setEnabled(true);
         renditionService2.setThumbnailsEnabled(true);
 
+        renditionDefinitionRegistry2.setRenditionConfigDir("alfresco/renditions/test");
         renditionDefinitionRegistry2.afterPropertiesSet();
         renditionService2.afterPropertiesSet();
 
@@ -273,4 +274,13 @@ public class RenditionService2Test
             assertTrue("Expected rendition "+name, renditionNames.contains(name));
         }
     }
+
+    @Test()
+    public void overriddenRendition()
+    {
+        // Check the standard rendition doclib has been overridden by one in alfresco/renditions/test/overrideRendition.json
+        RenditionDefinition2 doclib = renditionDefinitionRegistry2.getRenditionDefinition("doclib");
+        String resizeWidth = doclib.getTransformOptions().get("resizeWidth");
+        assertEquals("doclib has not been overridden", "180", resizeWidth);
+    }
 }
diff --git a/src/test/resources/alfresco/renditions/test/overrideRendition.json b/src/test/resources/alfresco/renditions/test/overrideRendition.json
new file mode 100644
index 0000000000..307271e42d
--- /dev/null
+++ b/src/test/resources/alfresco/renditions/test/overrideRendition.json
@@ -0,0 +1,16 @@
+{
+  "renditions": [
+    {
+      "renditionName": "doclib",
+      "targetMediaType": "image/png",
+      "options": [
+        {"name": "resizeWidth", "value": 180},
+        {"name": "resizeHeight", "value": 180},
+        {"name": "allowEnlargement", "value": false},
+        {"name": "maintainAspectRatio", "value": true},
+        {"name": "autoOrient", "value": true},
+        {"name": "thumbnail", "value": true}
+      ]
+    }
+  ]
+}