REPO-2182 : Community: ALF-21852: setContentStream() returns the wrong document version when auto-versioning is turned on (#63)

* REPO-2182 : Community: ALF-21852: setContentStream() returns the wrong document version when auto-versioning is turned on
   Separete into different new transactions writing content from creatingObjectId for setContentStream()
   Separate into different new transactions deleteing content from creatingObjectId for deleteContentStream()
   Separate into different new transations appending content from creatingObjectId for appendContentStream()
   Create JUnit CMISTest.testSetDeleteAppendContentStreamVersioning() testing that set/delete/apped calls return the new version.

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

@@ -65,6 +65,7 @@ import org.alfresco.repo.node.getchildren.GetChildrenCannedQuery;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.repo.security.authentication.Authorization;
 import org.alfresco.repo.transaction.RetryingTransactionHelper;
+import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback;
 import org.alfresco.repo.version.VersionModel;
 import org.alfresco.service.cmr.model.FileInfo;
 import org.alfresco.service.cmr.model.FileNotFoundException;
@@ -1526,15 +1527,34 @@ public class AlfrescoCmisServiceImpl extends AbstractCmisService implements Alfr
             throw new CmisStreamNotSupportedException("Document type doesn't allow content!");
         }
 
+        //ALF-21852 - Separated appendContent and objectId.setValue in two different transactions because
+        //after executing appendContent, the new objectId is not visible.
+        RetryingTransactionHelper helper = connector.getRetryingTransactionHelper();
+        helper.doInTransaction(new RetryingTransactionCallback<Void>()
+        {
+           public Void execute() throws Throwable
+           {
                try
                {
                    connector.appendContent(info, contentStream, isLastChunk);
-            objectId.setValue(connector.createObjectId(nodeRef));
                }
                catch(IOException e)
                {
                     throw new ContentIOException("", e);
                }
+                return null;
+           }
+        }, false, true);
+
+       String objId = helper.doInTransaction(new RetryingTransactionCallback<String>()
+       {
+           public String execute() throws Throwable
+           {
+               return connector.createObjectId(nodeRef);
+           }
+       }, true, true);
+
+       objectId.setValue(objId);
     }
 
     @Override
@@ -1569,6 +1589,13 @@ public class AlfrescoCmisServiceImpl extends AbstractCmisService implements Alfr
             throw new CmisInvalidArgumentException("No content!");
         }
 
+        //ALF-21852 - Separated setContent and objectId.setValue in two different transactions because
+        //after executing setContent, the new objectId is not visible.
+        RetryingTransactionHelper helper = connector.getRetryingTransactionHelper();
+        helper.doInTransaction(new RetryingTransactionCallback<Void>()
+        {
+            public Void execute() throws Throwable
+            {
                 String mimeType = parseMimeType(contentStream);
                 final File tempFile = copyToTempFile(contentStream);
                 String encoding = getEncoding(tempFile, mimeType);
@@ -1585,9 +1612,20 @@ public class AlfrescoCmisServiceImpl extends AbstractCmisService implements Alfr
                     removeTempFile(tempFile);
                 }
 
-        objectId.setValue(connector.createObjectId(nodeRef));
-
                 connector.getActivityPoster().postFileFolderUpdated(info.isFolder(), nodeRef);
+                return null;
+            }
+        }, false, true);
+
+        String objId = helper.doInTransaction(new RetryingTransactionCallback<String>()
+        {
+            public String execute() throws Throwable
+            {
+                return connector.createObjectId(nodeRef);
+            }
+        }, true, true);
+
+        objectId.setValue(objId);
     }
 
     @Override
@@ -1611,13 +1649,32 @@ public class AlfrescoCmisServiceImpl extends AbstractCmisService implements Alfr
             throw new CmisInvalidArgumentException("Document type requires content!");
         }
 
+        //ALF-21852 - Separated deleteContent and objectId.setValue in two different transactions because
+        //after executing deleteContent, the new objectId is not visible.
+        RetryingTransactionHelper helper = connector.getRetryingTransactionHelper();
+        helper.doInTransaction(new RetryingTransactionCallback<Void>()
+        {
+            public Void execute() throws Throwable
+            {
+
                 connector.getNodeService().setProperty(nodeRef, ContentModel.PROP_CONTENT, null);
 
 //              connector.createVersion(nodeRef, VersionType.MINOR, "Delete content");
 
                 connector.getActivityPoster().postFileFolderUpdated(info.isFolder(), nodeRef);
+                return null;
+            }
+        }, false, true);
 
-        objectId.setValue(connector.createObjectId(nodeRef));
+        String objId = helper.doInTransaction(new RetryingTransactionCallback<String>()
+        {
+            public String execute() throws Throwable
+            {
+                return connector.createObjectId(nodeRef);
+            }
+        }, true, true);
+
+        objectId.setValue(objId);
     }
 
     @Override

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

@@ -162,6 +162,7 @@ import org.apache.chemistry.opencmis.commons.impl.dataobjects.PropertyStringImpl
 import org.apache.chemistry.opencmis.commons.impl.server.AbstractServiceFactory;
 import org.apache.chemistry.opencmis.commons.server.CallContext;
 import org.apache.chemistry.opencmis.commons.server.CmisService;
+import org.apache.chemistry.opencmis.commons.server.ObjectInfo;
 import org.apache.chemistry.opencmis.commons.spi.Holder;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -3255,6 +3256,89 @@ public class CMISTest
         withCmisService(callback, CmisVersion.CMIS_1_1);
     }
 
+    /**
+     * Test auto version behavior for setContentStream, deleteContentStream and appendContentStream according to ALF-21852.
+     */
+    @Test
+    public void testSetDeleteAppendContentStreamVersioning() throws Exception
+    {
+        AuthenticationUtil.pushAuthentication();
+        AuthenticationUtil.setFullyAuthenticatedUser(AuthenticationUtil.getAdminUserName());
+
+        final String DOC1 = "documentProperties1-" + GUID.generate();
+
+        try
+        {
+            final FileInfo doc = transactionService.getRetryingTransactionHelper().doInTransaction(new RetryingTransactionCallback<FileInfo>()
+            {
+                @Override
+                public FileInfo execute() throws Throwable
+                {
+                    FileInfo document;
+                    // create document
+                    document = fileFolderService.create(repositoryHelper.getCompanyHome(), DOC1, ContentModel.TYPE_CONTENT);
+                    nodeService.setProperty(document.getNodeRef(), ContentModel.PROP_NAME, DOC1);
+
+                    Map<QName, Serializable> props = new HashMap<QName, Serializable>();
+                    props.put(ContentModel.PROP_TITLE, "Initial Title");
+                    props.put(ContentModel.PROP_DESCRIPTION, "Initial Description");
+
+                    nodeService.addAspect(document.getNodeRef(), ContentModel.ASPECT_TITLED, props);
+
+                    // apply versionable aspect with properties
+                    props = new HashMap<QName, Serializable>();
+                    // ContentModel.PROP_INITIAL_VERSION always true
+                    props.put(ContentModel.PROP_INITIAL_VERSION, true);
+                    props.put(ContentModel.PROP_AUTO_VERSION, true);
+                    props.put(ContentModel.PROP_AUTO_VERSION_PROPS, true);
+                    versionService.ensureVersioningEnabled(document.getNodeRef(), props);
+
+                    return document;
+                }
+            });
+            withCmisService(new CmisServiceCallback<Void>()
+            {
+                @Override public Void execute(CmisService cmisService)
+                {
+                    final String documentNodeRefId = doc.getNodeRef().toString();
+                    final String repositoryId = cmisService.getRepositoryInfos(null).get(0).getId();
+
+                    ObjectInfo objInfoInitialVersion = cmisService.getObjectInfo(repositoryId, documentNodeRefId);
+                    assertTrue("We had just created the document - it should be version 1.0", objInfoInitialVersion.getId().endsWith("1.0"));
+
+                    ContentStreamImpl contentStream = new ContentStreamImpl(null, MimetypeMap.MIMETYPE_TEXT_PLAIN, "Content " + GUID.generate());
+                    Holder<String> objectIdHolder = new Holder<>(documentNodeRefId);
+                    // Test setContentStream
+                    cmisService.setContentStream(repositoryId, objectIdHolder, true, null, contentStream, null);
+                    assertTrue("The \"output\" parameter should returns the newly created version id: 1.1", objectIdHolder.getValue().endsWith("1.1"));
+                    // we can use this new version id to get information about the cmis object
+                    ObjectInfo objInfoAfterSetContentStream = cmisService.getObjectInfo(repositoryId, objectIdHolder.getValue());
+                    assertTrue("The object info should reflect the version requested: 1.1", objInfoAfterSetContentStream.getId().endsWith("1.1"));
+
+                    // Test deleteContentStream
+                    cmisService.deleteContentStream(repositoryId, objectIdHolder, null, null);
+                    assertTrue("The \"output\" parameter should returns the newly created version id: 1.2", objectIdHolder.getValue().endsWith("1.2"));
+                    // we can use this new version id to get information about the cmis object
+                    objInfoAfterSetContentStream = cmisService.getObjectInfo(repositoryId, objectIdHolder.getValue());
+                    assertTrue("The object info should reflect the version requested: 1.2", objInfoAfterSetContentStream.getId().endsWith("1.2"));
+
+                    // Test appendContentStream
+                    cmisService.appendContentStream(repositoryId, objectIdHolder, null, contentStream, true, null);
+                    assertTrue("The \"output\" parameter should returns the newly created version id: 1.3", objectIdHolder.getValue().endsWith("1.3"));
+                    // we can use this new version id to get information about the cmis object
+                    objInfoAfterSetContentStream = cmisService.getObjectInfo(repositoryId, objectIdHolder.getValue());
+                    assertTrue("The object info should reflect the version requested: 1.3", objInfoAfterSetContentStream.getId().endsWith("1.3"));
+
+                    return null;
+                }
+            }, CmisVersion.CMIS_1_1);
+        }
+        finally
+        {
+            AuthenticationUtil.popAuthentication();
+        }
+    }
+
     /**
      * Test to ensure auto version behavior for update properties, set and delete content using both Alfresco and CMIS perspectives.
      * Testing different combinations of <b>cm:initialVersion</b>, <b>cm:autoVersion</b> and <b>cm:autoVersionOnUpdateProps</b> properties