From 3583e56ba2fdde32b95172da0ef5b8c24467a58b Mon Sep 17 00:00:00 2001 From: Andrei Forascu Date: Thu, 8 Nov 2018 11:20:54 +0200 Subject: [PATCH] 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 --- .../org/alfresco/opencmis/CMISConnector.java | 33 ++++---- .../java/org/alfresco/opencmis/CMISTest.java | 75 ++++++++++++++++--- 2 files changed, 82 insertions(+), 26 deletions(-) diff --git a/src/main/java/org/alfresco/opencmis/CMISConnector.java b/src/main/java/org/alfresco/opencmis/CMISConnector.java index c6be813f26..d27f7b4a08 100644 --- a/src/main/java/org/alfresco/opencmis/CMISConnector.java +++ b/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()); + // Collect entryIds to use a counter and a way to find the last changeLogToken + final List entryIds = new ArrayList(); + EntryIdCallback changeLogCollectingCallback = new EntryIdCallback(true) { @Override public boolean handleAuditEntry(Long entryId, String user, long time, Map 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 - StringBuilder clt = new StringBuilder(); - newChangeLogToken = (from == null ? clt.append(maxItems.intValue() + 1).toString() : clt.append(from.longValue() + maxItems.intValue()).toString()); // TODO: Make this readable + // We are assuming there are there is only one extra document now in line with how it used to behave // Remove extra item that was not actually requested - result.getObjects().remove(result.getObjects().size() - 1).getId(); - // Note to client that there are more items - result.setHasMoreItems(true); - } - else - { - // We got the same or fewer than the number requested, so there are no more items - result.setHasMoreItems(false); + result.getObjects().remove(resultSize - 1); + entryIds.remove(resultSize - 1); } 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; diff --git a/src/test/java/org/alfresco/opencmis/CMISTest.java b/src/test/java/org/alfresco/opencmis/CMISTest.java index 4cd74da136..d1f388a057 100644 --- a/src/test/java/org/alfresco/opencmis/CMISTest.java +++ b/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 - String folderName = "testfolder" + GUID.generate(); - String docName = "testdoc.txt" + GUID.generate(); - createContent(folderName, docName, false); - folderName = "testfolder" + GUID.generate(); - docName = "testdoc.txt" + GUID.generate(); - createContent(folderName, docName, false); - Holder changeLogToken = new Holder(); - ObjectList ol = this.cmisConnector.getContentChanges(changeLogToken, new BigInteger("2")); - assertEquals(2, ol.getNumItems()); - assertEquals("3", changeLogToken.getValue()); + setupAudit(); + + AuthenticationUtil.pushAuthentication(); + AuthenticationUtil.setFullyAuthenticatedUser(AuthenticationUtil.getAdminUserName()); + + try + { + // create folders with files + createContent("testfolder" + GUID.generate(), "testdoc.txt" + GUID.generate(), false); + createContent("testfolder" + GUID.generate(), "testdoc.txt" + GUID.generate(), false); + createContent("testfolder" + GUID.generate(), "testdoc.txt" + GUID.generate(), false); + + Holder changeLogToken = new Holder(); + + /* + * 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() + { + 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(); + }; } /**