ACS-295: Fix bug with new REST API for Version Renditions (#665)

* ACS-295: Fix bug with new REST API for Version Renditions

- also added test that fails before fix / succeeds after fix
- note: the issue existed in previous versions of Repo (but was never exposed) ...
- ...so needs to be handled for upgrades to 6.2.2 (+)

- see also ACS-197 / ACS-207

src/main/java/org/alfresco/rest/api/impl/RenditionsImpl.java

@@ -96,7 +96,7 @@ import java.util.TreeMap;
  */
 public class RenditionsImpl implements Renditions, ResourceLoaderAware
 {
-    private static final Log LOGGER = LogFactory.getLog(RenditionsImpl.class);
+    private static final Log logger = LogFactory.getLog(RenditionsImpl.class);
 
     private static final Set<String> RENDITION_STATUS_COLLECTION_EQUALS_QUERY_PROPERTIES = Collections.singleton(PARAM_STATUS);
 
@@ -183,13 +183,14 @@ public class RenditionsImpl implements Renditions, ResourceLoaderAware
             includeNotCreated = !includeCreated;
         }
 
+        // List all available rendition definitions
+        long size = contentData.getSize();
+        RenditionDefinitionRegistry2 renditionDefinitionRegistry2 = renditionService2.getRenditionDefinitionRegistry2();
+        Set<String> renditionNames = renditionDefinitionRegistry2.getRenditionNamesFrom(sourceMimetype, size);
+
         Map<String, Rendition> apiRenditions = new TreeMap<>();
         if (includeNotCreated)
         {
-            // List all available rendition definitions
-            long size = contentData.getSize();
-            RenditionDefinitionRegistry2 renditionDefinitionRegistry2 = renditionService2.getRenditionDefinitionRegistry2();
-            Set<String> renditionNames = renditionDefinitionRegistry2.getRenditionNamesFrom(sourceMimetype, size);
             for (String renditionName : renditionNames)
             {
                 apiRenditions.put(renditionName, toApiRendition(renditionName));
@@ -203,16 +204,27 @@ public class RenditionsImpl implements Renditions, ResourceLoaderAware
             {
                 NodeRef renditionNodeRef = childAssociationRef.getChildRef();
                 Rendition apiRendition = toApiRendition(renditionNodeRef);
-                if (includeCreated)
+                String renditionName = apiRendition.getId();
+                if (renditionNames.contains(renditionName))
                 {
-                    // Replace/append any thumbnail definitions with created rendition info
+                    if (includeCreated)
-                    apiRenditions.put(apiRendition.getId(), apiRendition);
+                    {
+                        // Replace/append any thumbnail definitions with created rendition info
+                        apiRenditions.put(renditionName, apiRendition);
+                    }
+                    else
+                    {
+                        // Remove any thumbnail definitions that has been created from the list,
+                        // as the filter requires only the Not_Created renditions
+                        apiRenditions.remove(renditionName);
+                    }
                 }
                 else
                 {
-                    // Remove any thumbnail definitions that has been created from the list,
+                    if (logger.isTraceEnabled())
-                    // as the filter requires only the Not_Created renditions
+                    {
-                    apiRenditions.remove(apiRendition.getId());
+                        logger.trace("Skip unknown rendition [" + renditionName + ", " + renditionNodeRef + "]");
+                    }
                 }
             }
         }
@@ -509,9 +521,9 @@ public class RenditionsImpl implements Renditions, ResourceLoaderAware
             }
             else
             {
-                if (LOGGER.isDebugEnabled())
+                if (logger.isDebugEnabled())
                 {
-                    LOGGER.debug("Retrieving content from resource path [" + phPath + ']');
+                    logger.debug("Retrieving content from resource path [" + phPath + ']');
                 }
                 // get extension of resource
                 String ext = "";
@@ -531,9 +543,9 @@ public class RenditionsImpl implements Renditions, ResourceLoaderAware
                 }
                 catch (Exception ex)
                 {
-                    if (LOGGER.isErrorEnabled())
+                    if (logger.isErrorEnabled())
                     {
-                        LOGGER.error("Couldn't load the placeholder." + ex.getMessage());
+                        logger.error("Couldn't load the placeholder." + ex.getMessage());
                     }
                     throw new ApiException("Couldn't load the placeholder.");
                 }

src/test/java/org/alfresco/rest/api/tests/NodeVersionRenditionsApiTest.java

@@ -57,6 +57,9 @@ public class NodeVersionRenditionsApiTest extends AbstractSingleNetworkSiteTest
 {
     private final static long DELAY_IN_MS = 500;
 
+    private static final List<String> DEFAULT_RENDITIONS_FOR_TXT =
+            new ArrayList<>(List.of("avatar", "avatar32", "doclib", "imgpreview", "medium", "pdf"));
+
     /**
      * Upload some versions and then create and retrieve version renditions
      *
@@ -90,8 +93,10 @@ public class NodeVersionRenditionsApiTest extends AbstractSingleNetworkSiteTest
             String contentName = "content-2-" + System.currentTimeMillis();
             String content = textContentSuffix + cnt;
 
+            // request minor version on upload (& no pre-request for renditions for live node)
+            Boolean majorVersion = true;
             Map<String, String> params = new HashMap<>();
-            params.put("majorVersion", "true");
+            params.put("majorVersion", majorVersion.toString());
 
             // create a new file
             Document documentResp = createTextFile(f1Id, contentName, content, "UTF-8", params);
@@ -101,7 +106,7 @@ public class NodeVersionRenditionsApiTest extends AbstractSingleNetworkSiteTest
             assertEquals(versionLabel, documentResp.getProperties().get("cm:versionLabel"));
 
             cnt = 2;
-            versionLabel = updateFileVersions(user1, docId, cnt, textContentSuffix, verCnt, true, versionLabel);
+            versionLabel = updateFileVersions(user1, docId, cnt, textContentSuffix, verCnt, majorVersion, versionLabel);
             verCnt = verCnt+cnt;
 
             assertEquals("3.0", versionLabel);
@@ -130,6 +135,70 @@ public class NodeVersionRenditionsApiTest extends AbstractSingleNetworkSiteTest
         }
     }
 
+    @Test
+    public void testUpFileVersionRenditionsWithDoclib() throws Exception
+    {
+        setRequestContext(user1);
+
+        String myFolderNodeId = getMyNodeId();
+
+        // create folder
+        String f1Id = createFolder(myFolderNodeId, "f1").getId();
+
+        try
+        {
+            int verCnt = 1;
+            int cnt = 1;
+            String versionLabel = "0.1";
+
+            String textContentSuffix = "Amazingly few discotheques provide jukeboxes ";
+            String contentName = "content-2-" + System.currentTimeMillis();
+            String content = textContentSuffix + cnt;
+
+            // request minor version on upload & also pre-request "doclib" rendition (for live node)
+            Boolean majorVersion = false;
+            Map<String, String> params = new HashMap<>();
+            params.put("majorVersion", majorVersion.toString());
+            params.put("renditions", "doclib");
+
+            // create a new file
+            Document documentResp = createTextFile(f1Id, contentName, content, "UTF-8", params);
+            String docId = documentResp.getId();
+            assertTrue(documentResp.getAspectNames().contains("cm:versionable"));
+            assertNotNull(documentResp.getProperties());
+            assertEquals(versionLabel, documentResp.getProperties().get("cm:versionLabel"));
+
+            // check live node
+            checkAndGetVersionRendition(docId, null, "doclib");
+
+            cnt = 1;
+            versionLabel = updateFileVersions(user1, docId, cnt, textContentSuffix, verCnt, majorVersion, versionLabel);
+            verCnt = verCnt+cnt;
+
+            assertEquals("0.2", versionLabel);
+            assertEquals(2, verCnt);
+
+            // check version history count
+            HttpResponse response = getAll(getNodeVersionsUrl(docId), null, null, 200);
+            List<Node> nodes = RestApiUtil.parseRestApiEntries(response.getJsonResponse(), Node.class);
+            assertEquals(verCnt, nodes.size());
+
+            // pause briefly
+            Thread.sleep(DELAY_IN_MS);
+
+            checkCreateAndGetVersionRendition(docId, "0.2", "doclib");
+
+            // check live node
+            checkAndGetVersionRendition(docId, null, "doclib");
+        }
+        finally
+        {
+            // some cleanup
+            setRequestContext(user1);
+            deleteNode(f1Id, true, 204);
+        }
+    }
+
     @Test
     public void testNegative() throws Exception
     {
@@ -223,11 +292,12 @@ public class NodeVersionRenditionsApiTest extends AbstractSingleNetworkSiteTest
         Paging paging = getPaging(0, 50);
         HttpResponse response = getAll(getRenditionsUrl, paging, 200);
         List<Rendition> renditions = RestApiUtil.parseRestApiEntries(response.getJsonResponse(), Rendition.class);
-        assertTrue(renditions.size() >= 5);
+        assertTrue(renditions.size() == DEFAULT_RENDITIONS_FOR_TXT.size());
 
         for (Rendition rendition : renditions)
         {
             assertEquals(Rendition.RenditionStatus.NOT_CREATED, rendition.getStatus());
+            assertTrue(DEFAULT_RENDITIONS_FOR_TXT.contains(rendition.getId()));
         }
 
         // Get rendition (not created yet) information for node
@@ -259,6 +329,41 @@ public class NodeVersionRenditionsApiTest extends AbstractSingleNetworkSiteTest
         assertTrue(contentInfo.getSizeInBytes() > 0);
     }
 
+    private void checkAndGetVersionRendition(String docId, String versionId, String renditionId) throws Exception
+    {
+        String getRenditionsUrl;
+        if ((versionId != null) && (! versionId.isEmpty()))
+        {
+            getRenditionsUrl = getNodeVersionRenditionsUrl(docId, versionId);
+        }
+        else
+        {
+            getRenditionsUrl = getNodeRenditionsUrl(docId);
+        }
+
+        // List renditions for version
+        Paging paging = getPaging(0, 50);
+        HttpResponse response = getAll(getRenditionsUrl, paging, 200);
+        List<Rendition> renditions = RestApiUtil.parseRestApiEntries(response.getJsonResponse(), Rendition.class);
+        assertTrue(renditions.size() == DEFAULT_RENDITIONS_FOR_TXT.size());
+
+        for (Rendition rendition : renditions)
+        {
+            assertTrue(DEFAULT_RENDITIONS_FOR_TXT.contains(rendition.getId()));
+        }
+
+        // Get version rendition
+        Rendition rendition = waitAndGetRendition(docId, versionId, renditionId);
+        assertNotNull(rendition);
+        assertEquals(Rendition.RenditionStatus.CREATED, rendition.getStatus());
+        ContentInfo contentInfo = rendition.getContent();
+        assertNotNull(contentInfo);
+        assertEquals(MimetypeMap.MIMETYPE_IMAGE_PNG, contentInfo.getMimeType());
+        assertEquals("PNG Image", contentInfo.getMimeTypeName());
+        assertNotNull(contentInfo.getEncoding());
+        assertTrue(contentInfo.getSizeInBytes() > 0);
+    }
+
     @Override
     public String getScope()
     {