REPO-1144: LockService - add isLocked & isLockedAndReadOnly common helper methods to public API

- remove duplicated code from various locations to use the new methods - add sanity checks to Lock*Test git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/BRANCHES/DEV/5.2.N/root@129764 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261
2025-08-14 17:58:59 +00:00 · 2016-08-22 14:20:52 +00:00
parent 2c62e68f71
commit 940fa92b21
3 changed files with 376 additions and 380 deletions
--- a/source/java/org/alfresco/rest/api/impl/NodesImpl.java
+++ b/source/java/org/alfresco/rest/api/impl/NodesImpl.java
@@ -22,9 +22,9 @@
 * You should have received a copy of the GNU Lesser General Public License
 * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
 * #L%
- */
-package org.alfresco.rest.api.impl;
-
+ */
+package org.alfresco.rest.api.impl;
+
 import java.io.BufferedInputStream;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
@@ -161,37 +161,37 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.springframework.dao.ConcurrencyFailureException;
 import org.springframework.extensions.surf.util.Content;
-import org.springframework.extensions.webscripts.servlet.FormData;
-
-/**
- * Centralises access to file/folder/node services and maps between representations.
- *
+import org.springframework.extensions.webscripts.servlet.FormData;
+
+/**
+ * Centralises access to file/folder/node services and maps between representations.
+ *
 * Note:
 * This class was originally used for returning some basic node info when listing Favourites.
 *
 * It has now been re-purposed and extended to implement the new Nodes (RESTful) API for
 * managing files & folders, as well as custom node types.
- * 
- * @author steveglover
+ * 
+ * @author steveglover
 * @author janv
 * @author Jamal Kaabi-Mofrad
 * 
- * @since publicapi1.0
- */
-public class NodesImpl implements Nodes
-{
+ * @since publicapi1.0
+ */
+public class NodesImpl implements Nodes
+{
    private static final Log logger = LogFactory.getLog(NodesImpl.class);


    private enum Type
-    {
+    {
        // Note: ordered
-        DOCUMENT, FOLDER
-    }
-
-    private NodeService nodeService;
-    private DictionaryService dictionaryService;
-    private FileFolderService fileFolderService;
+        DOCUMENT, FOLDER
+    }
+
+    private NodeService nodeService;
+    private DictionaryService dictionaryService;
+    private FileFolderService fileFolderService;
    private NamespaceService namespaceService;
    private PermissionService permissionService;
    private MimetypeService mimetypeService;
@@ -203,13 +203,13 @@ public class NodesImpl implements Nodes
    private AuthorityService authorityService;
    private ThumbnailService thumbnailService;
    private SiteService siteService;
-    private ActivityPoster poster;
-    private RetryingTransactionHelper retryingTransactionHelper;
-    private NodeAssocService nodeAssocService;
+    private ActivityPoster poster;
+    private RetryingTransactionHelper retryingTransactionHelper;
+    private NodeAssocService nodeAssocService;
    private LockService lockService;
-
-    private enum Activity_Type
-    {
+
+    private enum Activity_Type
+    {
        ADDED, UPDATED, DELETED, DOWNLOADED
    }

@@ -221,7 +221,7 @@ public class NodesImpl implements Nodes
    private Repository repositoryHelper;
    private ServiceRegistry sr;
    private Set<String> defaultIgnoreTypesAndAspects;
-
+
    // ignore types/aspects
    private Set<QName> ignoreQNames;

@@ -254,13 +254,13 @@ public class NodesImpl implements Nodes
        this.personService = sr.getPersonService();
        this.ownableService = sr.getOwnableService();
        this.authorityService = sr.getAuthorityService();
-        this.thumbnailService = sr.getThumbnailService();
-        this.siteService =  sr.getSiteService();
-        this.retryingTransactionHelper = sr.getRetryingTransactionHelper();
+        this.thumbnailService = sr.getThumbnailService();
+        this.siteService =  sr.getSiteService();
+        this.retryingTransactionHelper = sr.getRetryingTransactionHelper();
        this.lockService = sr.getLockService();
-
-        if (defaultIgnoreTypesAndAspects != null)
-        {
+
+        if (defaultIgnoreTypesAndAspects != null)
+        {
            ignoreQNames = new HashSet<>(defaultIgnoreTypesAndAspects.size());
            for (String type : defaultIgnoreTypesAndAspects)
            {
@@ -273,7 +273,7 @@ public class NodesImpl implements Nodes
    {
        this.sr = sr;
    }
-
+
    public void setBehaviourFilter(BehaviourFilter behaviourFilter)
    {
        this.behaviourFilter = behaviourFilter;
@@ -363,20 +363,20 @@ public class NodesImpl implements Nodes
    @Override
    public NodeRef validateNode(String nodeId)
    {
-        //belts-and-braces
-        if (nodeId == null)
-        {
-            throw new InvalidArgumentException("Missing nodeId");
-        }
-
+        //belts-and-braces
+        if (nodeId == null)
+        {
+            throw new InvalidArgumentException("Missing nodeId");
+        }
+
        return validateNode(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, nodeId);
    }
-
+
    @Override
    public NodeRef validateNode(StoreRef storeRef, String nodeId)
    {
        String versionLabel = null;
-
+
        int idx = nodeId.indexOf(";");
        if (idx != -1)
        {
@@ -388,11 +388,11 @@ public class NodesImpl implements Nodes
                throw new EntityNotFoundException(nodeId);
            }
        }
-
+
        NodeRef nodeRef = new NodeRef(storeRef, nodeId);
        return validateNode(nodeRef);
    }
-
+
    @Override
    public NodeRef validateNode(NodeRef nodeRef)
    {
@@ -400,10 +400,10 @@ public class NodesImpl implements Nodes
        {
            throw new EntityNotFoundException(nodeRef.getId());
        }
-
+
        return nodeRef;
    }
-
+
    /*
     * Check that nodes exists and matches given expected/excluded type(s).
     */
@@ -429,10 +429,10 @@ public class NodesImpl implements Nodes
        {
            throw new EntityNotFoundException(nodeRef.getId());
        }
-
+
        return typeMatches(getNodeType(nodeRef), expectedTypes, excludedTypes);
    }
-
+
    private QName getNodeType(NodeRef nodeRef)
    {
        return nodeService.getType(nodeRef);
@@ -460,7 +460,7 @@ public class NodesImpl implements Nodes
                allExpectedTypes.addAll(dictionaryService.getSubTypes(expectedType, true));
            }
        }
-
+
        Set<QName> allExcludedTypes = new HashSet<>();
        if (excludedTypes != null)
        {
@@ -469,7 +469,7 @@ public class NodesImpl implements Nodes
                allExcludedTypes.addAll(dictionaryService.getSubTypes(excludedType, true));
            }
        }
-
+
        boolean inExpected = allExpectedTypes.contains(type);
        boolean excluded = allExcludedTypes.contains(type);
        return (inExpected && !excluded);
@@ -479,23 +479,23 @@ public class NodesImpl implements Nodes
     * @deprecated review usage (backward compat')
     */
    @Override
-    public Node getNode(String nodeId)
-    {
+    public Node getNode(String nodeId)
+    {
        NodeRef nodeRef = validateNode(nodeId);
-
+
        return new Node(nodeRef, null, nodeService.getProperties(nodeRef), null, sr);
-    }
+    }

    /**
     * @deprecated review usage (backward compat')
     */
-    public Node getNode(NodeRef nodeRef)
-    {
+    public Node getNode(NodeRef nodeRef)
+    {
        return new Node(nodeRef, null, nodeService.getProperties(nodeRef), null, sr);
-    }
+    }

-    private Type getType(NodeRef nodeRef)
-    {
+    private Type getType(NodeRef nodeRef)
+    {
        return getType(getNodeType(nodeRef), nodeRef);
    }

@@ -554,14 +554,14 @@ public class NodesImpl implements Nodes
        }

        return null; // unknown
-    }
-
-    /**
+    }
+
+    /**
     * @deprecated note: currently required for backwards compat' (Favourites API)
-     */
+     */
    @Override
-    public Document getDocument(NodeRef nodeRef)
-    {
+    public Document getDocument(NodeRef nodeRef)
+    {
        Type type = getType(nodeRef);
        if ((type != null) && type.equals(Type.DOCUMENT))
        {
@@ -584,7 +584,7 @@ public class NodesImpl implements Nodes
        {
            throw new InvalidArgumentException("Node is not a file: "+nodeRef.getId());
        }
-    }
+    }

    private void setCommonProps(Node node, NodeRef nodeRef, Map<QName,Serializable> properties)
    {
@@ -595,12 +595,12 @@ public class NodesImpl implements Nodes
        node.setCreatedBy((String)properties.get(ContentModel.PROP_CREATOR));
    }

-    /**
+    /**
     * @deprecated note: currently required for backwards compat' (Favourites API)
-     */
+     */
    @Override
-    public Folder getFolder(NodeRef nodeRef)
-    {
+    public Folder getFolder(NodeRef nodeRef)
+    {
        Type type = getType(nodeRef);
        if ((type != null) && type.equals(Type.FOLDER))
        {
@@ -614,7 +614,7 @@ public class NodesImpl implements Nodes
        {
            throw new InvalidArgumentException("Node is not a folder: "+nodeRef.getId());
        }
-    }
+    }

    private NodeRef getParentNodeRef(NodeRef nodeRef)
    {
@@ -916,9 +916,9 @@ public class NodesImpl implements Nodes
                String perm = kv.getKey();
                String op = kv.getValue();

-                if (perm.equals(PermissionService.ADD_CHILDREN) && Type.DOCUMENT.equals(type))
+                if (perm.equals(PermissionService.ADD_CHILDREN) && Type.DOCUMENT.equals(type))
                {
-                    // special case: do not return "create" (as an allowable op) for file/content types - note: 'type' can be null
+                    // special case: do not return "create" (as an allowable op) for file/content types - note: 'type' can be null
                    continue;
                }
                else if (perm.equals(PermissionService.DELETE) && (isSpecialNode(nodeRef, nodeTypeQName)))
@@ -1602,10 +1602,10 @@ public class NodesImpl implements Nodes
            props = mapToNodeProperties(nodeInfo.getProperties());
        }

-        // Optionally, lookup by relative path
-        String relativePath = nodeInfo.getRelativePath();
-        parentNodeRef = getOrCreatePath(parentNodeRef, relativePath);
-
+        // Optionally, lookup by relative path
+        String relativePath = nodeInfo.getRelativePath();
+        parentNodeRef = getOrCreatePath(parentNodeRef, relativePath);
+
        // Existing file/folder name handling
        boolean autoRename = Boolean.valueOf(parameters.getParameter(PARAM_AUTO_RENAME));
        if (autoRename && (isContent || isSubClass(nodeTypeQName, ContentModel.TYPE_FOLDER)))
@@ -1623,29 +1623,29 @@ public class NodesImpl implements Nodes
        {
            assocTypeQName = getAssocType(nodeInfo.getAssociation().getAssocType());
        }
-        
-        Boolean versionMajor = null;
-        String str = parameters.getParameter(PARAM_VERSION_MAJOR);
-        if (str != null)
-        {
-            versionMajor = new Boolean(str);
-        }
-        String versionComment = parameters.getParameter(PARAM_VERSION_COMMENT);
+        
+        Boolean versionMajor = null;
+        String str = parameters.getParameter(PARAM_VERSION_MAJOR);
+        if (str != null)
+        {
+            versionMajor = new Boolean(str);
+        }
+        String versionComment = parameters.getParameter(PARAM_VERSION_COMMENT);

        // Create the node
-        NodeRef nodeRef;
+        NodeRef nodeRef;
+
+        if (isContent)
+        {
+            // create empty file node - note: currently will be set to default encoding only (UTF-8)
+            nodeRef = createNewFile(parentNodeRef, nodeName, nodeTypeQName, null, props, assocTypeQName, parameters, versionMajor, versionComment);
+        }
+        else
+        {
+            // create non-content node
+            nodeRef = createNodeImpl(parentNodeRef, nodeName, nodeTypeQName, props, assocTypeQName);
+        }

-        if (isContent)
-        {
-            // create empty file node - note: currently will be set to default encoding only (UTF-8)
-            nodeRef = createNewFile(parentNodeRef, nodeName, nodeTypeQName, null, props, assocTypeQName, parameters, versionMajor, versionComment);
-        }
-        else
-        {
-            // create non-content node
-            nodeRef = createNodeImpl(parentNodeRef, nodeName, nodeTypeQName, props, assocTypeQName);
-        }
-
        List<String> aspectNames = nodeInfo.getAspectNames();
        if (aspectNames != null)
        {
@@ -1661,7 +1661,7 @@ public class NodesImpl implements Nodes
                nodeService.addAspect(nodeRef, aspectQName, null);
            }
        }
-        
+        
        // eg. to create mandatory assoc(s)

        if (nodeInfo.getTargets() != null)
@@ -1708,17 +1708,17 @@ public class NodesImpl implements Nodes

        for (AssocChild assoc : entities)
        {
-            String childId = assoc.getChildId();
-            if (childId == null)
-            {
-                throw new InvalidArgumentException("Missing childId");
-            }
-
+            String childId = assoc.getChildId();
+            if (childId == null)
+            {
+                throw new InvalidArgumentException("Missing childId");
+            }
+
            QName assocTypeQName = getAssocType(assoc.getAssocType());

            try
            {
-                NodeRef childNodeRef = validateNode(childId);
+                NodeRef childNodeRef = validateNode(childId);

                String nodeName = (String)nodeService.getProperty(childNodeRef, ContentModel.PROP_NAME);
                QName assocChildQName = QName.createQName(NamespaceService.CONTENT_MODEL_1_0_URI, QName.createValidLocalName(nodeName));
@@ -1748,12 +1748,12 @@ public class NodesImpl implements Nodes

        for (AssocTarget assoc : entities)
        {
-            String targetNodeId = assoc.getTargetId();
-            if (targetNodeId == null)
-            {
-                throw new InvalidArgumentException("Missing targetId");
-            }
-
+            String targetNodeId = assoc.getTargetId();
+            if (targetNodeId == null)
+            {
+                throw new InvalidArgumentException("Missing targetId");
+            }
+
            String assocTypeStr = assoc.getAssocType();
            QName assocTypeQName = getAssocType(assocTypeStr);
            try
@@ -1987,11 +1987,7 @@ public class NodesImpl implements Nodes
        if (((aspects != null) && aspects.contains(ContentModel.ASPECT_LOCKABLE)) 
           || nodeService.hasAspect(nodeRef, ContentModel.ASPECT_LOCKABLE))
        {
-            LockStatus status = lockService.getLockStatus(nodeRef);
-            if (status == LockStatus.LOCKED || status == LockStatus.LOCK_OWNER)
-            {
-                locked = true;
-            }
+            locked = lockService.isLocked(nodeRef);
        }

        return locked;
@@ -2000,31 +1996,31 @@ public class NodesImpl implements Nodes
    @Override
    public Node updateNode(String nodeId, Node nodeInfo, Parameters parameters)
    {
-        retryingTransactionHelper.doInTransaction(new RetryingTransactionHelper.RetryingTransactionCallback<Void>()
-        {
-            @Override
-            public Void execute() throws Throwable
-            {
-                NodeRef nodeRef = updateNodeImpl(nodeId, nodeInfo, parameters);
-                ActivityInfo activityInfo =  getActivityInfo(getParentNodeRef(nodeRef), nodeRef);
-                postActivity(Activity_Type.UPDATED, activityInfo, false);
-                
-                return null;
-            }
-        }, false, true);
-
-        return retryingTransactionHelper.doInTransaction(new RetryingTransactionHelper.RetryingTransactionCallback<Node>()
-        {
-            @Override
-            public Node execute() throws Throwable
-            {
-                return getFolderOrDocument(nodeId, parameters);
-            }
-        }, false, false);
-    }
-    
-    protected NodeRef updateNodeImpl(String nodeId, Node nodeInfo, Parameters parameters)
-    {
+        retryingTransactionHelper.doInTransaction(new RetryingTransactionHelper.RetryingTransactionCallback<Void>()
+        {
+            @Override
+            public Void execute() throws Throwable
+            {
+                NodeRef nodeRef = updateNodeImpl(nodeId, nodeInfo, parameters);
+                ActivityInfo activityInfo =  getActivityInfo(getParentNodeRef(nodeRef), nodeRef);
+                postActivity(Activity_Type.UPDATED, activityInfo, false);
+                
+                return null;
+            }
+        }, false, true);
+
+        return retryingTransactionHelper.doInTransaction(new RetryingTransactionHelper.RetryingTransactionCallback<Node>()
+        {
+            @Override
+            public Node execute() throws Throwable
+            {
+                return getFolderOrDocument(nodeId, parameters);
+            }
+        }, false, false);
+    }
+    
+    protected NodeRef updateNodeImpl(String nodeId, Node nodeInfo, Parameters parameters)
+    {
        final NodeRef nodeRef = validateNode(nodeId);

        QName nodeTypeQName = getNodeType(nodeRef);
@@ -2165,8 +2161,8 @@ public class NodesImpl implements Nodes
                throw new ConstraintViolatedException(dcne.getMessage());
            }
        }
-        
-        return nodeRef;
+        
+        return nodeRef;
    }

    @Override
@@ -2319,17 +2315,17 @@ public class NodesImpl implements Nodes
        }
        String versionComment = parameters.getParameter(PARAM_VERSION_COMMENT);

-        String fileName = parameters.getParameter(PARAM_NAME);
-        if (fileName != null)
-        {
-            // optionally rename, before updating the content
-            nodeService.setProperty(nodeRef, ContentModel.PROP_NAME, fileName);
-        }
-        else
-        {
-            fileName = (String)nodeService.getProperty(nodeRef, ContentModel.PROP_NAME);
-        }
-        
+        String fileName = parameters.getParameter(PARAM_NAME);
+        if (fileName != null)
+        {
+            // optionally rename, before updating the content
+            nodeService.setProperty(nodeRef, ContentModel.PROP_NAME, fileName);
+        }
+        else
+        {
+            fileName = (String)nodeService.getProperty(nodeRef, ContentModel.PROP_NAME);
+        }
+        
        return updateExistingFile(null, nodeRef, fileName, contentInfo, stream, parameters, versionMajor, versionComment);
    }

@@ -2340,28 +2336,28 @@ public class NodesImpl implements Nodes
        behaviourFilter.disableBehaviour(nodeRef, ContentModel.ASPECT_VERSIONABLE);
        try
        {
-            writeContent(nodeRef, fileName, stream, true);
+            writeContent(nodeRef, fileName, stream, true);

            if ((isVersioned) || (versionMajor != null) || (versionComment != null) )
            {
-                VersionType versionType = null;
-                if (versionMajor != null)
+                VersionType versionType = null;
+                if (versionMajor != null)
                {
-                    versionType = (versionMajor ? VersionType.MAJOR : VersionType.MINOR);
+                    versionType = (versionMajor ? VersionType.MAJOR : VersionType.MINOR);
                }
-                else
-                {
-                    // note: it is possible to have versionable aspect but no versions (=> no version label)
-                    if ((! isVersioned) || (nodeService.getProperty(nodeRef, ContentModel.PROP_VERSION_LABEL) == null))
-                    {
-                        versionType = VersionType.MAJOR;
-                    }
-                    else
-                    {
-                        versionType = VersionType.MINOR;
-                    }
-                }
-
+                else
+                {
+                    // note: it is possible to have versionable aspect but no versions (=> no version label)
+                    if ((! isVersioned) || (nodeService.getProperty(nodeRef, ContentModel.PROP_VERSION_LABEL) == null))
+                    {
+                        versionType = VersionType.MAJOR;
+                    }
+                    else
+                    {
+                        versionType = VersionType.MINOR;
+                    }
+                }
+
                createVersion(nodeRef, isVersioned, versionType, versionComment);
            }

@@ -2378,100 +2374,100 @@ public class NodesImpl implements Nodes
        return getFolderOrDocumentFullInfo(nodeRef, null, null, parameters);
    }

-    private void writeContent(NodeRef nodeRef, String fileName, InputStream stream, boolean guessEncoding)
+    private void writeContent(NodeRef nodeRef, String fileName, InputStream stream, boolean guessEncoding)
    {
        ContentWriter writer = contentService.getWriter(nodeRef, ContentModel.PROP_CONTENT, true);
-
-        String mimeType = mimetypeService.guessMimetype(fileName);
-        if ((mimeType != null) && (! mimeType.equals(MimetypeMap.MIMETYPE_BINARY)))
-        {
-            // quick/weak guess based on file extension
-            writer.setMimetype(mimeType);
-        }
-        else
-        {
-            // stronger guess based on file stream
-            writer.guessMimetype(fileName);
-        }
-
-        InputStream is = null;
-
-        if (guessEncoding)
-        {
-            is = new BufferedInputStream(stream);
-            is.mark(1024);
-            writer.setEncoding(guessEncoding(is, mimeType, false));
-            try
-            {
-                is.reset();
-            }
-            catch (IOException ioe)
-            {
-                if (logger.isWarnEnabled())
-                {
-                    logger.warn("Failed to reset stream after trying to guess encoding: " + ioe.getMessage());
-                }
-            }
-        }
-        else
-        {
-            is = stream;
-        }
-
-        writer.putContent(is);
+
+        String mimeType = mimetypeService.guessMimetype(fileName);
+        if ((mimeType != null) && (! mimeType.equals(MimetypeMap.MIMETYPE_BINARY)))
+        {
+            // quick/weak guess based on file extension
+            writer.setMimetype(mimeType);
+        }
+        else
+        {
+            // stronger guess based on file stream
+            writer.guessMimetype(fileName);
+        }
+
+        InputStream is = null;
+
+        if (guessEncoding)
+        {
+            is = new BufferedInputStream(stream);
+            is.mark(1024);
+            writer.setEncoding(guessEncoding(is, mimeType, false));
+            try
+            {
+                is.reset();
+            }
+            catch (IOException ioe)
+            {
+                if (logger.isWarnEnabled())
+                {
+                    logger.warn("Failed to reset stream after trying to guess encoding: " + ioe.getMessage());
+                }
+            }
+        }
+        else
+        {
+            is = stream;
+        }
+
+        writer.putContent(is);
+    }
+
+    private String guessEncoding(InputStream in, String mimeType, boolean close)
+    {
+        String encoding = "UTF-8";
+        try
+        {
+            if (in != null)
+            {
+                Charset charset = mimetypeService.getContentCharsetFinder().getCharset(in, mimeType);
+                encoding = charset.name();
+            }
+        }
+        finally
+        {
+            try
+            {
+                if (close && (in != null))
+                {
+                    in.close();
+                }
+            }
+            catch (IOException ioe)
+            {
+                if (logger.isWarnEnabled())
+                {
+                    logger.warn("Failed to close stream after trying to guess encoding: " + ioe.getMessage());
+                }
+            }
+        }
+        return encoding;
    }

-    private String guessEncoding(InputStream in, String mimeType, boolean close)
-    {
-        String encoding = "UTF-8";
-        try
-        {
-            if (in != null)
-            {
-                Charset charset = mimetypeService.getContentCharsetFinder().getCharset(in, mimeType);
-                encoding = charset.name();
-            }
-        }
-        finally
-        {
-            try
-            {
-                if (close && (in != null))
-                {
-                    in.close();
-                }
-            }
-            catch (IOException ioe)
-            {
-                if (logger.isWarnEnabled())
-                {
-                    logger.warn("Failed to close stream after trying to guess encoding: " + ioe.getMessage());
-                }
-            }
-        }
-        return encoding;
-    }
-
    protected void createVersion(NodeRef nodeRef, boolean isVersioned, VersionType versionType, String reason)
    {
        if (! isVersioned)
        {
-            // Ensure versioning is enabled for the file (autoVersion = true, autoVersionProps = false)
-            Map<QName, Serializable> props = new HashMap<>(2);
-            props.put(ContentModel.PROP_AUTO_VERSION, true);
-            props.put(ContentModel.PROP_AUTO_VERSION_PROPS, false);
-
-            nodeService.addAspect(nodeRef, ContentModel.ASPECT_VERSIONABLE, props);
-        }
-
-        Map<String, Serializable> versionProperties = new HashMap<>(2);
-        versionProperties.put(VersionModel.PROP_VERSION_TYPE, versionType);
-        if (reason != null)
-        {
-            versionProperties.put(VersionModel.PROP_DESCRIPTION, reason);
-        }
+            // Ensure versioning is enabled for the file (autoVersion = true, autoVersionProps = false)
+            Map<QName, Serializable> props = new HashMap<>(2);
+            props.put(ContentModel.PROP_AUTO_VERSION, true);
+            props.put(ContentModel.PROP_AUTO_VERSION_PROPS, false);

-        versionService.createVersion(nodeRef, versionProperties);
+            nodeService.addAspect(nodeRef, ContentModel.ASPECT_VERSIONABLE, props);
+        }
+
+        Map<String, Serializable> versionProperties = new HashMap<>(2);
+        versionProperties.put(VersionModel.PROP_VERSION_TYPE, versionType);
+        if (reason != null)
+        {
+            versionProperties.put(VersionModel.PROP_DESCRIPTION, reason);
+        }
+
+        versionService.createVersion(nodeRef, versionProperties);
    }

    @Override
@@ -2491,9 +2487,9 @@ public class NodesImpl implements Nodes
        String fileName = null;
        Content content = null;
        boolean autoRename = false;
-        QName nodeTypeQName = ContentModel.TYPE_CONTENT;
+        QName nodeTypeQName = ContentModel.TYPE_CONTENT;
        boolean overwrite = false; // If a fileName clashes for a versionable file
-        Boolean versionMajor = null;
+        Boolean versionMajor = null;
        String versionComment = null;
        String relativePath = null;
        String renditionNames = null;
@@ -2538,7 +2534,7 @@ public class NodesImpl implements Nodes
                    break;

                case "majorversion":
-                    versionMajor = Boolean.valueOf(field.getValue());
+                    versionMajor = Boolean.valueOf(field.getValue());
                    break;

                case "comment":
@@ -2613,7 +2609,7 @@ public class NodesImpl implements Nodes
                {
                    // overwrite existing (versionable) file
                    BasicContentInfo contentInfo = new ContentInfoImpl(content.getMimetype(), content.getEncoding(), -1, null);
-                    return updateExistingFile(parentNodeRef, existingFile, fileName, contentInfo, content.getInputStream(), parameters, versionMajor, versionComment);
+                    return updateExistingFile(parentNodeRef, existingFile, fileName, contentInfo, content.getInputStream(), parameters, versionMajor, versionComment);
                }
                else
                {
@@ -2621,18 +2617,18 @@ public class NodesImpl implements Nodes
                    throw new ConstraintViolatedException(fileName + " already exists.");
                }
            }
-            
-            // Note: pending REPO-159, we currently auto-enable versioning on new upload (but not when creating empty file)
-            if (versionMajor == null)
-            {
-                versionMajor = true;
-            }
+            
+            // Note: pending REPO-159, we currently auto-enable versioning on new upload (but not when creating empty file)
+            if (versionMajor == null)
+            {
+                versionMajor = true;
+            }

            // Create a new file.
-            NodeRef nodeRef = createNewFile(parentNodeRef, fileName, nodeTypeQName, content, properties, assocTypeQName, parameters, versionMajor, versionComment);
-            
-            // Create the response
-            final Node fileNode = getFolderOrDocumentFullInfo(nodeRef, parentNodeRef, nodeTypeQName, parameters);
+            NodeRef nodeRef = createNewFile(parentNodeRef, fileName, nodeTypeQName, content, properties, assocTypeQName, parameters, versionMajor, versionComment);
+            
+            // Create the response
+            final Node fileNode = getFolderOrDocumentFullInfo(nodeRef, parentNodeRef, nodeTypeQName, parameters);

            // RA-1052
            try
@@ -2684,44 +2680,44 @@ public class NodesImpl implements Nodes
        }
    }

-    private NodeRef createNewFile(NodeRef parentNodeRef, String fileName, QName nodeType, Content content, Map<QName, Serializable> props, QName assocTypeQName, Parameters params,
-                                  Boolean versionMajor, String versionComment)
+    private NodeRef createNewFile(NodeRef parentNodeRef, String fileName, QName nodeType, Content content, Map<QName, Serializable> props, QName assocTypeQName, Parameters params,
+                                  Boolean versionMajor, String versionComment)
    {
-        NodeRef nodeRef = createNodeImpl(parentNodeRef, fileName, nodeType, props, assocTypeQName);
-        
-        if (content == null)
+        NodeRef nodeRef = createNodeImpl(parentNodeRef, fileName, nodeType, props, assocTypeQName);
+        
+        if (content == null)
        {
-            // Write "empty" content
-            writeContent(nodeRef, fileName, new ByteArrayInputStream("".getBytes()), false);
+            // Write "empty" content
+            writeContent(nodeRef, fileName, new ByteArrayInputStream("".getBytes()), false);
        }
-        else
-        {
-            // Write content
-            writeContent(nodeRef, fileName, content.getInputStream(), true);
-        }
-        
-        if ((versionMajor != null) || (versionComment != null))
-        {
-            behaviourFilter.disableBehaviour(nodeRef, ContentModel.ASPECT_VERSIONABLE);
-            try
-            {
-                // by default, first version is major, unless specified otherwise
-                VersionType versionType = VersionType.MAJOR;
-                if ((versionMajor != null) && (!versionMajor))
-                {
-                    versionType = VersionType.MINOR;
-                }
+        else
+        {
+            // Write content
+            writeContent(nodeRef, fileName, content.getInputStream(), true);
+        }
+        
+        if ((versionMajor != null) || (versionComment != null))
+        {
+            behaviourFilter.disableBehaviour(nodeRef, ContentModel.ASPECT_VERSIONABLE);
+            try
+            {
+                // by default, first version is major, unless specified otherwise
+                VersionType versionType = VersionType.MAJOR;
+                if ((versionMajor != null) && (!versionMajor))
+                {
+                    versionType = VersionType.MINOR;
+                }

-                createVersion(nodeRef, false, versionType, versionComment);
-
-                extractMetadata(nodeRef);
-            } finally
-            {
-                behaviourFilter.enableBehaviour(nodeRef, ContentModel.ASPECT_VERSIONABLE);
-            }
-        }
-
-        return nodeRef;
+                createVersion(nodeRef, false, versionType, versionComment);
+
+                extractMetadata(nodeRef);
+            } finally
+            {
+                behaviourFilter.enableBehaviour(nodeRef, ContentModel.ASPECT_VERSIONABLE);
+            }
+        }
+
+        return nodeRef;
    }

    private String getStringOrNull(String value)
@@ -2929,10 +2925,10 @@ public class NodesImpl implements Nodes
            {
                result.add(name);
            }
-        }
-        return result;
-    }
-
+        }
+        return result;
+    }
+
    @Override
    public Node lock(String nodeId, LockInfo lockInfo, Parameters parameters)
    {
@@ -3003,27 +2999,27 @@ public class NodesImpl implements Nodes
            }
        }
        return getFolderOrDocument(nodeId, parameters);
-    }
-
-    /**
-     * @author Jamal Kaabi-Mofrad
+    }
+
+    /**
+     * @author Jamal Kaabi-Mofrad
     */
-    /*
-    private static class ContentInfoWrapper implements BasicContentInfo
+    /*
+    private static class ContentInfoWrapper implements BasicContentInfo
    {
        private String mimeType;
        private String encoding;

-        public String getEncoding()
-        {
-            return encoding;
-        }
-
-        public String getMimeType()
-        {
-            return mimeType;
-        }
-
+        public String getEncoding()
+        {
+            return encoding;
+        }
+
+        public String getMimeType()
+        {
+            return mimeType;
+        }
+
        ContentInfoWrapper(BasicContentInfo basicContentInfo)
        {
            if (basicContentInfo != null)
@@ -3064,6 +3060,6 @@ public class NodesImpl implements Nodes
            }
        }
    }
-    */
-}
+    */
+}