MNT-20139: CmisConnector returns wrong values for changeLogToken and hasMoreItems (#271)

- ensure that the returned token is always updated. This is currently not the case when there are no more items. The next token is set to null in that case, even though there might be (maxItems-1) results.
- added Junit tests

src/main/java/org/alfresco/opencmis/CMISConnector.java

@@ -3675,11 +3675,15 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen
         final ObjectListImpl result = new ObjectListImpl();
         result.setObjects(new ArrayList<ObjectData>());
 
+        // Collect entryIds to use a counter and a way to find the last changeLogToken
+        final List<Long> entryIds = new ArrayList<Long>();
+
         EntryIdCallback changeLogCollectingCallback = new EntryIdCallback(true)
         {
             @Override
             public boolean handleAuditEntry(Long entryId, String user, long time, Map<String, Serializable> values)
             {
+                entryIds.add(entryId);
                 result.getObjects().addAll(createChangeEvents(time, values));
                 return super.handleAuditEntry(entryId, user, time, values);
             }
@@ -3694,7 +3698,7 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen
             }
             catch (NumberFormatException e)
             {
-                throw new CmisInvalidArgumentException("Invalid change log token: " + changeLogToken);
+                throw new CmisInvalidArgumentException("Invalid change log token: " + changeLogToken.getValue());
             }
         }
 
@@ -3712,27 +3716,26 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen
 
         auditService.auditQuery(changeLogCollectingCallback, params, queryFor);
 
-        String newChangeLogToken = null;
+        int resultSize = result.getObjects().size();
+
+        // Use the entryIds as a counter is more reliable then the result.getObjects().
+        // result.getObjects() can be more or less then the requested maxResults, because it is filtered based on the content.
+        boolean hasMoreItems = entryIds.size() >= maxResults;
+        result.setHasMoreItems(hasMoreItems);
         // Check if we got more than the client requested
-        if (result.getObjects().size() >= maxResults)
+        if (hasMoreItems && resultSize >= maxResults)
         {
-            // Build the change log token from the last item
+            // We are assuming there are there is only one extra document now in line with how it used to behave
-            StringBuilder clt = new StringBuilder();
-            newChangeLogToken = (from == null ? clt.append(maxItems.intValue() + 1).toString() : clt.append(from.longValue() + maxItems.intValue()).toString());    // TODO: Make this readable
             // Remove extra item that was not actually requested
-            result.getObjects().remove(result.getObjects().size() - 1).getId();
+            result.getObjects().remove(resultSize - 1);
-            // Note to client that there are more items
+            entryIds.remove(resultSize - 1);
-            result.setHasMoreItems(true);
-        }
-        else
-        {
-            // We got the same or fewer than the number requested, so there are no more items
-            result.setHasMoreItems(false);
         }
 
         if (changeLogToken != null)
         {
-            changeLogToken.setValue(newChangeLogToken);
+            //Update the changelog after removing the last item if there are more items.
+            Long newChangeLogToken = entryIds.isEmpty() ? from : entryIds.get(entryIds.size() - 1);
+            changeLogToken.setValue(String.valueOf(newChangeLogToken));
         }
 
         return result;

src/test/java/org/alfresco/opencmis/CMISTest.java

@@ -2052,19 +2052,72 @@ public class CMISTest
         }
     }
 
+    /**
+     * MNT-20139
+     * CmisConnector returns wrong values for changeLogToken and hasMoreItems
+     */
+    @Test
     public void testGetContentChanges()
     {
-        // create folder with file
+        setupAudit();
-        String folderName = "testfolder" + GUID.generate();
+
-        String docName = "testdoc.txt" + GUID.generate();
+        AuthenticationUtil.pushAuthentication();
-        createContent(folderName, docName, false);
+        AuthenticationUtil.setFullyAuthenticatedUser(AuthenticationUtil.getAdminUserName());
-        folderName = "testfolder" + GUID.generate();
+
-        docName = "testdoc.txt" + GUID.generate();
+        try
-        createContent(folderName, docName, false);
+        {
-        Holder<String> changeLogToken = new Holder<String>();
+            // create folders with files
-        ObjectList ol = this.cmisConnector.getContentChanges(changeLogToken, new BigInteger("2"));
+            createContent("testfolder" + GUID.generate(), "testdoc.txt" + GUID.generate(), false);
-        assertEquals(2, ol.getNumItems());
+            createContent("testfolder" + GUID.generate(), "testdoc.txt" + GUID.generate(), false);
-        assertEquals("3", changeLogToken.getValue());
+            createContent("testfolder" + GUID.generate(), "testdoc.txt" + GUID.generate(), false);
+
+            Holder<String> changeLogToken = new Holder<String>();
+
+            /*
+             * GetContentChanges with maxitems = 2 and null changeLogToken
+             * Check that changeLogToken should be the latest from the retrieved entries
+             */
+            ObjectList ol = this.cmisConnector.getContentChanges(changeLogToken, new BigInteger("2"));
+            assertEquals(2, ol.getObjects().size());
+            assertEquals("ChangeLogToken should be latest from retrieved entries.", "2", changeLogToken.getValue());
+            assertTrue(ol.hasMoreItems());
+
+            /*
+             * GetContentChanges with maxitems = 2 and changeLogToken = 0
+             * Check that changeLogToken should be the latest from the retrieved entries
+             */
+            changeLogToken.setValue(Integer.toString(0));
+            ol = this.cmisConnector.getContentChanges(changeLogToken, new BigInteger("2"));
+            assertEquals(2, ol.getObjects().size());
+            assertEquals("ChangeLogToken should be latest from retrieved entries.", "2", changeLogToken.getValue());
+            assertTrue(ol.hasMoreItems());
+
+            /*
+             * GetContentChanges with changeLogToken = maxChangeLogToken - 2
+             * Check that changeLogToken is not null when the latest entries (fromToken) are retrieved
+             */
+            Long latestToken = transactionService.getRetryingTransactionHelper().doInTransaction(new RetryingTransactionCallback<Long>()
+            {
+                public Long execute() throws Exception
+                {
+                    return Long.parseLong(cmisConnector.getRepositoryInfo(CmisVersion.CMIS_1_1).getLatestChangeLogToken());
+                }
+            }, true, false);
+
+            Long fromToken = latestToken - 2;
+            changeLogToken.setValue(fromToken.toString());
+
+            ol = this.cmisConnector.getContentChanges(changeLogToken, new BigInteger("20"));
+            assertEquals(3, ol.getObjects().size());
+            assertNotNull(changeLogToken.getValue());
+            assertEquals("ChangeLogToken should be the latest from all entries.", latestToken.toString(), changeLogToken.getValue());
+            assertFalse(ol.hasMoreItems());
+        }
+        finally
+        {
+            auditSubsystem.destroy();
+            AuthenticationUtil.popAuthentication();
+        };
     }
 
     /**