[MNT-21953] [MNT-22491] Clear rendition content data on content change. Prevent rendition from having contentHashCode without content (#752)

* [MNT-21953] [MNT-22491] Clear rendition content data on content change. Prevent rendition from having contentHashCode without content

* [MNT-21953] [MNT-22491] Added tests

* [MNT-21953] [MNT-22491] Removed update content from test

* [MNT-21953] [MNT-22491] Improve log messages

* [MNT-21953] [MNT-22491] Changed Copyright year to 2021. Minor change in test comments.

repository/src/main/java/org/alfresco/repo/rendition/RenditionServiceImpl.java

@@ -2,7 +2,7 @@
  * #%L
  * Alfresco Repository
  * %%
- * Copyright (C) 2005 - 2016 Alfresco Software Limited
+ * Copyright (C) 2005 - 2021 Alfresco Software Limited
  * %%
  * This file is part of the Alfresco software. 
  * If the software was purchased under a paid Alfresco license, the terms of 
@@ -683,6 +683,7 @@ public class RenditionServiceImpl implements
                     log.debug("OnContentUpdate calling RenditionService2.render(\""+sourceNodeRef+"\", \""+renditionName+"\" so we switch to the new service.");
                 }
                 useRenditionService2 = true;
+                renditionService2.clearRenditionContentData(sourceNodeRef, renditionName);
                 renditionService2.render(sourceNodeRef, renditionName);
             }
         }

repository/src/main/java/org/alfresco/repo/rendition2/RenditionService2Impl.java

@@ -2,7 +2,7 @@
  * #%L
  * Alfresco Repository
  * %%
- * Copyright (C) 2005 - 2020 Alfresco Software Limited
+ * Copyright (C) 2005 - 2021 Alfresco Software Limited
  * %%
  * This file is part of the Alfresco software.
  * If the software was purchased under a paid Alfresco license, the terms of
@@ -538,9 +538,8 @@ public class RenditionService2Impl implements RenditionService2, InitializingBea
                             }
                             if (logger.isDebugEnabled())
                             {
-                                logger.debug("Set rendition hashcode " + transformContentHashCode + " and ThumbnailLastModified for " + renditionName);
+                                logger.debug("Set ThumbnailLastModified for " + renditionName);
                             }
-                            nodeService.setProperty(renditionNode, RenditionModel.PROP_RENDITION_CONTENT_HASH_CODE, transformContentHashCode);
                             setThumbnailLastModified(sourceNodeRef, renditionName);
 
                             if (transformInputStream != null)
@@ -554,6 +553,11 @@ public class RenditionService2Impl implements RenditionService2, InitializingBea
                                     contentWriter.setEncoding(DEFAULT_ENCODING);
                                     ContentWriter renditionWriter = contentWriter;
                                     renditionWriter.putContent(transformInputStream);
+                                    if (logger.isDebugEnabled())
+                                    {
+                                        logger.debug("Set rendition hashcode for " + renditionName);
+                                    }
+                                    nodeService.setProperty(renditionNode, RenditionModel.PROP_RENDITION_CONTENT_HASH_CODE, transformContentHashCode);
                                 }
                                 catch (Exception e)
                                 {
@@ -563,16 +567,7 @@ public class RenditionService2Impl implements RenditionService2, InitializingBea
                             }
                             else
                             {
-                                Serializable content = nodeService.getProperty(renditionNode, PROP_CONTENT);
+                                clearRenditionContentData(renditionNode);
-                                if (content != null)
-                                {
-                                    nodeService.removeProperty(renditionNode, PROP_CONTENT);
-                                    nodeService.removeProperty(renditionNode, PROP_RENDITION_CONTENT_HASH_CODE);
-                                    if (logger.isDebugEnabled())
-                                    {
-                                        logger.debug("Cleared rendition content and hashcode");
-                                    }
-                                }
                             }
 
                             if (!sourceHasAspectRenditioned)
@@ -736,6 +731,43 @@ public class RenditionService2Impl implements RenditionService2, InitializingBea
         }
     }
 
+    /**
+     * Clears source nodeRef rendition content and content hash code using supplied rendition name
+     *
+     * @param sourceNodeRef
+     * @param renditionName
+     */
+    public void clearRenditionContentData(NodeRef sourceNodeRef, String renditionName)
+    {
+        clearRenditionContentData(getRenditionNode(sourceNodeRef, renditionName));
+    }
+
+    /**
+     * Clears supplied rendition node content (if exists) and content hash code
+     *
+     * @param renditionNode
+     */
+    private void clearRenditionContentData(NodeRef renditionNode)
+    {
+        if (renditionNode != null)
+        {
+            Serializable content = nodeService.getProperty(renditionNode, PROP_CONTENT);
+            if (content != null)
+            {
+                nodeService.removeProperty(renditionNode, PROP_CONTENT);
+                if (logger.isDebugEnabled())
+                {
+                    logger.debug("Cleared rendition content");
+                }
+            }
+            nodeService.removeProperty(renditionNode, PROP_RENDITION_CONTENT_HASH_CODE);
+            if (logger.isDebugEnabled())
+            {
+                logger.debug("Cleared rendition hashcode");
+            }
+        }
+    }
+
     /**
      * This method checks whether the specified source node is of a content class which has been registered for
      * rendition prevention.
@@ -886,6 +918,7 @@ public class RenditionService2Impl implements RenditionService2, InitializingBea
                         RenditionDefinition2 renditionDefinition = renditionDefinitionRegistry2.getRenditionDefinition(renditionName);
                         if (renditionDefinition != null)
                         {
+                            clearRenditionContentData(sourceNodeRef, renditionName);
                             render(sourceNodeRef, renditionName);
                         }
                         else

repository/src/main/java/org/alfresco/repo/thumbnail/ThumbnailServiceImpl.java

@@ -2,7 +2,7 @@
  * #%L
  * Alfresco Repository
  * %%
- * Copyright (C) 2005 - 2017 Alfresco Software Limited
+ * Copyright (C) 2005 - 2021 Alfresco Software Limited
  * %%
  * This file is part of the Alfresco software. 
  * If the software was purchased under a paid Alfresco license, the terms of 
@@ -640,7 +640,7 @@ public class ThumbnailServiceImpl implements ThumbnailService,
         boolean valid = true;
         ContentData content = (ContentData) this.nodeService.getProperty(thumbnailNode, ContentModel.PROP_CONTENT);
         // (MNT-17162) A thumbnail with an empty content is cached for post-transaction removal, to prevent the delete in read-only transactions. 
-        if (content.getSize() == 0)
+        if (content == null || content.getSize() == 0)
         {
             TransactionalResourceHelper.getSet(THUMBNAIL_TO_DELETE_NODES).add(thumbnailNode);
             TransactionSupportUtil.bindListener(this.thumbnailsToDeleteTransactionListener, 0);

repository/src/test/java/org/alfresco/repo/rendition2/AbstractRenditionIntegrationTest.java

@@ -2,7 +2,7 @@
  * #%L
  * Alfresco Repository
  * %%
- * Copyright (C) 2005 - 2018 Alfresco Software Limited
+ * Copyright (C) 2005 - 2021 Alfresco Software Limited
  * %%
  * This file is part of the Alfresco software.
  * If the software was purchased under a paid Alfresco license, the terms of
@@ -25,7 +25,16 @@
  */
 package org.alfresco.repo.rendition2;
 
-import junit.framework.AssertionFailedError;
+import static java.lang.Thread.sleep;
+import static org.alfresco.model.ContentModel.PROP_CONTENT;
+import static org.alfresco.model.RenditionModel.PROP_RENDITION_CONTENT_HASH_CODE;
+import static org.alfresco.repo.content.MimetypeMap.EXTENSION_BINARY;
+
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.Serializable;
+import java.util.Collections;
+
 import org.alfresco.model.ContentModel;
 import org.alfresco.repo.content.MimetypeMap;
 import org.alfresco.repo.content.metadata.AsynchronousExtractor;
@@ -60,15 +69,7 @@ import org.quartz.CronExpression;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.util.ResourceUtils;
 
-import java.io.File;
+import junit.framework.AssertionFailedError;
-import java.io.FileNotFoundException;
-import java.io.Serializable;
-import java.util.Collections;
-import java.util.Map;
-
-import static java.lang.Thread.sleep;
-import static org.alfresco.model.ContentModel.PROP_CONTENT;
-import static org.alfresco.repo.content.MimetypeMap.EXTENSION_BINARY;
 
 /**
  * Class unites common utility methods for {@link org.alfresco.repo.rendition2} package tests.
@@ -531,4 +532,41 @@ public abstract class AbstractRenditionIntegrationTest extends BaseSpringTest
         RetryingTransactionHelper txnHelper = transactionService.getRetryingTransactionHelper();
         txnHelper.doInTransaction(createUserCallback);
     }
+
+    /**
+     * Helper method to check if the supplied content hash code is valid or not
+     *
+     * @param contentHashCode
+     *            the hash code to verify
+     *
+     * @return true in case it is an actual hash code, false otherwise
+     */
+    protected boolean isValidRenditionContentHashCode(int contentHashCode)
+    {
+        return contentHashCode != RenditionService2Impl.RENDITION2_DOES_NOT_EXIST
+                && contentHashCode != RenditionService2Impl.SOURCE_HAS_NO_CONTENT;
+    }
+
+    /**
+     * Helper method which gets the content hash code from the supplied rendition node without specific validations (the
+     * equivalent method from {@link RenditionService2Impl} is not exposed)
+     *
+     * @param renditionNodeRef
+     *            the rendition node
+     *
+     * @return -1 in case of there is no content, -2 in case rendition doesn't exist, the actual content hash code
+     *         otherwise
+     */
+    protected int getRenditionContentHashCode(NodeRef renditionNodeRef)
+    {
+        int renditionContentHashCode = RenditionService2Impl.RENDITION2_DOES_NOT_EXIST;
+
+        if (renditionNodeRef != null)
+        {
+            Serializable hashCode = nodeService.getProperty(renditionNodeRef, PROP_RENDITION_CONTENT_HASH_CODE);
+            renditionContentHashCode = hashCode == null ? RenditionService2Impl.SOURCE_HAS_NO_CONTENT : (int) hashCode;
+        }
+
+        return renditionContentHashCode;
+    }
 }

repository/src/test/java/org/alfresco/repo/rendition2/RenditionService2IntegrationTest.java

@@ -2,7 +2,7 @@
  * #%L
  * Alfresco Repository
  * %%
- * Copyright (C) 2005 - 2018 Alfresco Software Limited
+ * Copyright (C) 2005 - 2021 Alfresco Software Limited
  * %%
  * This file is part of the Alfresco software.
  * If the software was purchased under a paid Alfresco license, the terms of
@@ -25,6 +25,11 @@
  */
 package org.alfresco.repo.rendition2;
 
+import static org.alfresco.model.ContentModel.PROP_CONTENT;
+import static org.junit.Assert.assertNotEquals;
+
+import java.util.List;
+
 import org.alfresco.model.ContentModel;
 import org.alfresco.model.RenditionModel;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
@@ -36,16 +41,9 @@ import org.alfresco.service.cmr.repository.datatype.DefaultTypeConverter;
 import org.alfresco.service.cmr.security.PermissionService;
 import org.alfresco.service.namespace.NamespaceService;
 import org.alfresco.service.namespace.QName;
-import org.junit.AfterClass;
-import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
-import java.util.List;
-
-import static org.alfresco.model.ContentModel.PROP_CONTENT;
-import static org.junit.Assert.assertNotEquals;
-
 /**
  * Integration tests for {@link RenditionService2}
  */
@@ -308,6 +306,99 @@ public class RenditionService2IntegrationTest extends AbstractRenditionIntegrati
         assertTrue("The rendition should be generated by new Rendition Service", hasRenditionedAspect);
     }
 
+    /**
+     * Tests new {@link RenditionService2Impl#clearRenditionContentData(NodeRef, String)} method.
+     * <p>
+     * This method cleans a rendition content and content hash code.
+     * </p>
+     */
+    @Test
+    public void testClearRenditionContentData()
+    {
+        // Create a node
+        NodeRef sourceNodeRef = createSource(ADMIN, "quick.jpg");
+
+        // Trigger the rendition
+        render(ADMIN, sourceNodeRef, DOC_LIB);
+        NodeRef renditionNodeRef = waitForRendition(ADMIN, sourceNodeRef, DOC_LIB, true);
+        assertNotNull("Rendition was not generated", renditionNodeRef);
+        assertNotNull("Rendition was not generated", nodeService.getProperty(renditionNodeRef, ContentModel.PROP_CONTENT));
+
+        // Check the rendition content hash code is valid
+        int contentHashCode = getRenditionContentHashCode(renditionNodeRef);
+        assertTrue("Rendition content hash code was not generated", isValidRenditionContentHashCode(contentHashCode));
+
+        // Disable renditionService2
+        renditionService2.setEnabled(false);
+
+        // Call 'clearRenditionContentData' method directly to prove rendition content will be cleaned
+        AuthenticationUtil.runAs((AuthenticationUtil.RunAsWork<Void>) () ->
+            transactionService.getRetryingTransactionHelper().doInTransaction(() ->
+            {
+                renditionService2.clearRenditionContentData(sourceNodeRef, DOC_LIB);
+                return null;
+            }), ADMIN);
+
+        // The rendition should not have content by now
+        assertNull("Rendition has content", nodeService.getProperty(renditionNodeRef, ContentModel.PROP_CONTENT));
+
+        // The rendition should not have a content hash code by now
+        contentHashCode = getRenditionContentHashCode(renditionNodeRef);
+        assertFalse("Rendition has content hash code", isValidRenditionContentHashCode(contentHashCode));
+
+        // Enable renditionService2
+        renditionService2.setEnabled(true);
+    }
+
+    /**
+     * Tests if a rendition without content (but with contentHashCode) can be generated again.
+     * <p>
+     * If the rendition consumption receives a null InputStream, the contentHashCode should be cleaned from the
+     * rendition node, allowing new requests to generate the rendition.
+     * </p>
+     */
+    @Test
+    public void testRenditionWithoutContentButWithContentHashCode()
+    {
+        // Create a node with an actual rendition
+        NodeRef sourceNodeRef = createSource(ADMIN, "quick.jpg");
+        render(ADMIN, sourceNodeRef, DOC_LIB);
+        NodeRef renditionNodeRef = waitForRendition(ADMIN, sourceNodeRef, DOC_LIB, true);
+        assertNotNull("Rendition was not generated", renditionNodeRef);
+        assertNotNull("Rendition was not generated", nodeService.getProperty(renditionNodeRef, ContentModel.PROP_CONTENT));
+
+        // Clear rendition content (at this point we're forcing a rendition without content but with hash code)
+        clearContent(ADMIN, renditionNodeRef);
+        assertNull("Rendition has content", nodeService.getProperty(renditionNodeRef, ContentModel.PROP_CONTENT));
+
+        // Check that rendition still has the content hash code
+        final int contentHashCode = getRenditionContentHashCode(renditionNodeRef);
+        assertTrue("Rendition content hash code was not generated", isValidRenditionContentHashCode(contentHashCode));
+
+        // Call the 'consume' method directly supplying a null InputStream
+        // This is explicitly called to prove that rendition hash code will be cleaned (as opposite to previous behavior)
+        RenditionDefinition2 renditionDefinition = renditionDefinitionRegistry2.getRenditionDefinition(DOC_LIB);
+        AuthenticationUtil.runAs(() -> {
+            renditionService2.consume(sourceNodeRef, null, renditionDefinition, contentHashCode);
+            return null;
+        }, ADMIN);
+        waitForRendition(ADMIN, sourceNodeRef, DOC_LIB, false);
+
+        // The content hash code should have been cleaned from the rendition node
+        int contentHashCode2 = getRenditionContentHashCode(renditionNodeRef);
+        assertFalse("Rendition content hash code was not cleaned", isValidRenditionContentHashCode(contentHashCode2));
+
+        // Attempts to render the rendition again
+        render(ADMIN, sourceNodeRef, DOC_LIB);
+        NodeRef renditionNodeRef2 = waitForRendition(ADMIN, sourceNodeRef, DOC_LIB, true);
+        assertEquals("New rendition nodeRef is different", renditionNodeRef.toString(), renditionNodeRef2.toString());
+        assertNotNull("New rendition content was not generated", nodeService.getProperty(renditionNodeRef2, ContentModel.PROP_CONTENT));
+
+        // Check the new rendition content hash code
+        int contentHashCode3 = getRenditionContentHashCode(renditionNodeRef2);
+        assertTrue("New rendition content hash code was not generated", isValidRenditionContentHashCode(contentHashCode3));
+    }
+
     /**
      * @deprecated can be removed when we remove the original RenditionService
      */