ALFCOM-3024 - re-enabled (via new cm:autoVersionOnUpdateProps) and added additional tests

git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@14932 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

config/alfresco/core-services-context.xml

@@ -906,9 +906,6 @@
         <property name="nodeService">
             <ref bean="nodeService"/>
         </property>
-        <property name="deprecatedNotOnUpdateProperties">
-            <value>true</value>
-        </property>
     </bean>
 
     <!--                 -->

config/alfresco/model/contentModel.xml

@@ -822,6 +822,11 @@
                <type>d:boolean</type>
                <default>true</default>
             </property>
+            <property name="cm:autoVersionOnUpdateProps">
+               <title>Auto Version - on update properties only</title>
+               <type>d:boolean</type>
+               <default>true</default>
+            </property>
           </properties>
       </aspect>
       

source/java/org/alfresco/model/ContentModel.java

@@ -161,6 +161,7 @@ public interface ContentModel
     static final QName PROP_VERSION_LABEL = QName.createQName(NamespaceService.CONTENT_MODEL_1_0_URI, "versionLabel");
     static final QName PROP_INITIAL_VERSION = QName.createQName(NamespaceService.CONTENT_MODEL_1_0_URI, "initialVersion");
     static final QName PROP_AUTO_VERSION = QName.createQName(NamespaceService.CONTENT_MODEL_1_0_URI, "autoVersion");
+    static final QName PROP_AUTO_VERSION_PROPS = QName.createQName(NamespaceService.CONTENT_MODEL_1_0_URI, "autoVersionOnUpdateProps");
     
     // folders
     static final QName TYPE_SYSTEM_FOLDER = QName.createQName(NamespaceService.CONTENT_MODEL_1_0_URI, "systemfolder");

source/java/org/alfresco/repo/coci/CheckOutCheckInServiceImplTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005-2007 Alfresco Software Limited.
+ * Copyright (C) 2005-2009 Alfresco Software Limited.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -514,4 +514,95 @@ public class CheckOutCheckInServiceImplTest extends BaseSpringTest
     
     }
     
+    public void testMultipleCheckoutsCheckInsWithPropChange()
+    {
+        // Note: this test assumes cm:autoVersionProps=true by default (refer to cm:versionableAspect in contentModel.xml)
+        
+        // Create a new node 
+        ChildAssociationRef childAssocRef = this.nodeService.createNode(
+                rootNodeRef,
+                ContentModel.ASSOC_CHILDREN,
+                QName.createQName("{test}test"),
+                ContentModel.TYPE_CONTENT,
+                null);
+        final NodeRef testNodeRef = childAssocRef.getChildRef();
+        
+        // Add the version aspect to the created node
+        this.nodeService.addAspect(testNodeRef, ContentModel.ASPECT_VERSIONABLE, null);
+        
+        setComplete();
+        endTransaction();
+        
+        // Checkout
+        final NodeRef workingCopy1 = transactionService.getRetryingTransactionHelper().doInTransaction(new RetryingTransactionCallback<NodeRef>()
+        {
+            public NodeRef execute() throws Exception
+            {
+                return cociService.checkout(testNodeRef);
+            }
+        });
+        
+        // Change property and checkin
+        transactionService.getRetryingTransactionHelper().doInTransaction(new RetryingTransactionCallback<Object>()
+        {
+            public Object execute() throws Exception
+            {
+                nodeService.setProperty(workingCopy1, ContentModel.PROP_AUTHOR, "author1");
+                
+                Map<String, Serializable> versionProperties = new HashMap<String, Serializable>();
+                versionProperties.put(Version.PROP_DESCRIPTION, "This is a test version 1");
+                cociService.checkin(workingCopy1, versionProperties);
+                
+                return null;
+            }
+        });
+        
+        // Checkout
+        final NodeRef workingCopy2 = transactionService.getRetryingTransactionHelper().doInTransaction(new RetryingTransactionCallback<NodeRef>()
+        {
+            public NodeRef execute() throws Exception
+            {
+                return cociService.checkout(testNodeRef);
+            }
+        });
+        
+        // Change property and checkin
+        transactionService.getRetryingTransactionHelper().doInTransaction(new RetryingTransactionCallback<Object>()
+        {
+            public Object execute() throws Exception
+            {
+                nodeService.setProperty(workingCopy2, ContentModel.PROP_AUTHOR, "author2");
+                
+                Map<String, Serializable> versionProperties = new HashMap<String, Serializable>();
+                versionProperties.put(Version.PROP_DESCRIPTION, "This is a test version 2");
+                cociService.checkin(workingCopy2, versionProperties);
+                
+                return null;
+            }
+        });
+        
+        // Checkout
+        final NodeRef workingCopy3 = transactionService.getRetryingTransactionHelper().doInTransaction(new RetryingTransactionCallback<NodeRef>()
+        {
+            public NodeRef execute() throws Exception
+            {
+                return cociService.checkout(testNodeRef);
+            }
+        });
+        
+        // Change property and checkin
+        transactionService.getRetryingTransactionHelper().doInTransaction(new RetryingTransactionCallback<Object>()
+        {
+            public Object execute() throws Exception
+            {
+                nodeService.setProperty(workingCopy3, ContentModel.PROP_AUTHOR, "author3");
+                
+                Map<String, Serializable> versionProperties = new HashMap<String, Serializable>();
+                versionProperties.put(Version.PROP_DESCRIPTION, "This is a test version 3");
+                cociService.checkin(workingCopy3, versionProperties);
+                
+                return null;
+            }
+        });
+    }
 }

source/java/org/alfresco/repo/version/VersionServiceImplTest.java

@@ -721,6 +721,7 @@ public class VersionServiceImplTest extends BaseVersionStoreTest
     {
         // Create a versionable node
         final NodeRef versionableNode = createNewVersionableNode();
+        
         this.dbNodeService.setProperty(versionableNode, ContentModel.PROP_AUTO_VERSION, false);
         
         setComplete();
@@ -919,6 +920,82 @@ public class VersionServiceImplTest extends BaseVersionStoreTest
         });
     }
     
+    public void testAutoVersionOnUpdatePropsOnly()
+    {
+        // test auto-version props on
+        final NodeRef versionableNode = createNewVersionableNode();
+        this.dbNodeService.setProperty(versionableNode, ContentModel.PROP_AUTO_VERSION_PROPS, true);
+        
+        setComplete();
+        endTransaction();
+        
+        transactionService.getRetryingTransactionHelper().doInTransaction(new RetryingTransactionCallback<Object>()
+        {
+            public Object execute() throws Exception
+            {
+                VersionHistory versionHistory = versionService.getVersionHistory(versionableNode);
+                assertNotNull(versionHistory);
+                assertEquals(1, versionHistory.getAllVersions().size());
+                
+                nodeService.setProperty(versionableNode, ContentModel.PROP_AUTHOR, "ano author");
+                
+                return null;
+            }
+        });
+        
+        // Now lets have a look and make sure we have the correct number of entries in the version history
+        transactionService.getRetryingTransactionHelper().doInTransaction(new RetryingTransactionCallback<Object>()
+        {
+            public Object execute() throws Exception
+            {
+                VersionHistory versionHistory = versionService.getVersionHistory(versionableNode);
+                assertNotNull(versionHistory);
+                assertEquals(2, versionHistory.getAllVersions().size());
+                
+                return null;
+            }
+        
+        });
+        
+        // test auto-version props off
+        
+        startNewTransaction();
+        
+        final NodeRef versionableNode2 = createNewVersionableNode();
+        this.dbNodeService.setProperty(versionableNode2, ContentModel.PROP_AUTO_VERSION_PROPS, false);
+        
+        setComplete();
+        endTransaction();
+        
+        transactionService.getRetryingTransactionHelper().doInTransaction(new RetryingTransactionCallback<Object>()
+        {
+            public Object execute() throws Exception
+            {
+                VersionHistory versionHistory = versionService.getVersionHistory(versionableNode2);
+                assertNotNull(versionHistory);
+                assertEquals(1, versionHistory.getAllVersions().size());
+                
+                nodeService.setProperty(versionableNode2, ContentModel.PROP_AUTHOR, "ano author");
+                
+                return null;
+            }
+        });
+        
+        // Now lets have a look and make sure we have the correct number of entries in the version history
+        transactionService.getRetryingTransactionHelper().doInTransaction(new RetryingTransactionCallback<Object>()
+        {
+            public Object execute() throws Exception
+            {
+                VersionHistory versionHistory = versionService.getVersionHistory(versionableNode2);
+                assertNotNull(versionHistory);
+                assertEquals(1, versionHistory.getAllVersions().size());
+                
+                return null;
+            }
+        
+        });
+    }
+    
     public void testAR807() 
     {
     	QName prop = QName.createQName("http://www.alfresco.org/test/versionstorebasetest/1.0", "intProp");

source/java/org/alfresco/repo/version/VersionableAspect.java

@@ -27,9 +27,7 @@ package org.alfresco.repo.version;
 import java.io.Serializable;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Map;
-import java.util.Set;
 
 import org.alfresco.i18n.I18NUtil;
 import org.alfresco.model.ContentModel;
@@ -43,7 +41,6 @@ import org.alfresco.repo.policy.Behaviour;
 import org.alfresco.repo.policy.JavaBehaviour;
 import org.alfresco.repo.policy.PolicyComponent;
 import org.alfresco.repo.transaction.AlfrescoTransactionSupport;
-import org.alfresco.repo.transaction.TransactionListenerAdapter;
 import org.alfresco.service.cmr.repository.ChildAssociationRef;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.NodeService;
@@ -68,10 +65,10 @@ public class VersionableAspect implements ContentServicePolicies.OnContentUpdate
     /** The i18n'ized messages */
     private static final String MSG_INITIAL_VERSION = "create_version.initial_version";
     private static final String MSG_AUTO_VERSION = "create_version.auto_version";
+    private static final String MSG_AUTO_VERSION_PROPS = "create_version.auto_version_props";
     
     /** Transaction resource key */
     private static final String KEY_VERSIONED_NODEREFS = "versioned_noderefs";
-    private static final String KEY_PENDING_NODEREFS = "pending_noderefs";
     
     /** The policy component */
     private PolicyComponent policyComponent;
@@ -82,11 +79,6 @@ public class VersionableAspect implements ContentServicePolicies.OnContentUpdate
     /** The Version service */
     private VersionService versionService;
     
-    /** If true, property changes are not auto-versioned (and contentUpdate is not deferred to beforeCommit) */
-    private boolean deprecatedNotOnUpdateProperties = false;
-    
-    private VersionableAspectTransactionListener transactionListener;
-    
     /**
      * Set the policy component
      * 
@@ -117,16 +109,6 @@ public class VersionableAspect implements ContentServicePolicies.OnContentUpdate
         this.nodeService = nodeService;
     }
     
-    /**
-     * Set to TRUE for to disable auto-versioned property changes (pre-3.2 behaviour)
-     * 
-     * @param deprecatedNotOnUpdateProperties
-     */
-    public void setDeprecatedNotOnUpdateProperties(boolean deprecatedNotOnUpdateProperties)
-    {
-        this.deprecatedNotOnUpdateProperties = deprecatedNotOnUpdateProperties;
-    }
-    
     /**
      * Initialise the versionable aspect policies
      */
@@ -135,15 +117,18 @@ public class VersionableAspect implements ContentServicePolicies.OnContentUpdate
         this.policyComponent.bindClassBehaviour(
                 QName.createQName(NamespaceService.ALFRESCO_URI, "onAddAspect"),
                 ContentModel.ASPECT_VERSIONABLE,
+                
                 new JavaBehaviour(this, "onAddAspect", Behaviour.NotificationFrequency.TRANSACTION_COMMIT));
         this.policyComponent.bindClassBehaviour(
                 QName.createQName(NamespaceService.ALFRESCO_URI, "onRemoveAspect"),
                 ContentModel.ASPECT_VERSIONABLE,
                 new JavaBehaviour(this, "onRemoveAspect", Behaviour.NotificationFrequency.TRANSACTION_COMMIT));
+        
         this.policyComponent.bindClassBehaviour(
                 QName.createQName(NamespaceService.ALFRESCO_URI, "onDeleteNode"),
                 ContentModel.ASPECT_VERSIONABLE, 
                 new JavaBehaviour(this, "onDeleteNode", Behaviour.NotificationFrequency.TRANSACTION_COMMIT));
+        
         this.policyComponent.bindClassBehaviour(
                 QName.createQName(NamespaceService.ALFRESCO_URI, "afterCreateVersion"),
                 ContentModel.ASPECT_VERSIONABLE,
@@ -154,22 +139,15 @@ public class VersionableAspect implements ContentServicePolicies.OnContentUpdate
                 ContentModel.ASPECT_VERSIONABLE,
                 new JavaBehaviour(this, "onContentUpdate", Behaviour.NotificationFrequency.TRANSACTION_COMMIT));
         
-        if (! deprecatedNotOnUpdateProperties)
-        {
         this.policyComponent.bindClassBehaviour(
             QName.createQName(NamespaceService.ALFRESCO_URI, "onUpdateProperties"),
             ContentModel.ASPECT_VERSIONABLE,
             new JavaBehaviour(this, "onUpdateProperties", Behaviour.NotificationFrequency.TRANSACTION_COMMIT));
-        }
         
-        // Register the copy behaviour
         this.policyComponent.bindClassBehaviour(
                 QName.createQName(NamespaceService.ALFRESCO_URI, "getCopyCallback"),
                 ContentModel.ASPECT_VERSIONABLE,
                 new JavaBehaviour(this, "getCopyCallback"));
-        
-        // Create the transaction listener
-        this.transactionListener = new VersionableAspectTransactionListener();
     }
     
     /**
@@ -204,7 +182,7 @@ public class VersionableAspect implements ContentServicePolicies.OnContentUpdate
         private static final CopyBehaviourCallback INSTANCE = new VersionableAspectCopyBehaviourCallback();
 
         /**
-         * Copy the aspect, but only the {@link ContentModel#PROP_AUTO_VERSION} property
+         * Copy the aspect, but only the {@link ContentModel#PROP_AUTO_VERSION} and {@link ContentModel#PROP_AUTO_VERSION_PROPS} properties
          */
         @Override
         public Map<QName, Serializable> getCopyProperties(
@@ -212,10 +190,24 @@ public class VersionableAspect implements ContentServicePolicies.OnContentUpdate
                 CopyDetails copyDetails,
                 Map<QName, Serializable> properties)
         {
-            Serializable value = properties.get(ContentModel.PROP_AUTO_VERSION);
+            Serializable value1 = properties.get(ContentModel.PROP_AUTO_VERSION);
-            if (value != null)
+            Serializable value2 = properties.get(ContentModel.PROP_AUTO_VERSION_PROPS);
+            
+            if ((value1 != null) || (value2 != null))
             {
-                return Collections.singletonMap(ContentModel.PROP_AUTO_VERSION, value);
+                Map<QName, Serializable> newProperties = new HashMap<QName, Serializable>(2);
+                
+                if (value1 != null)
+                {
+                    newProperties.put(ContentModel.PROP_AUTO_VERSION, value1);
+                }
+                
+                if (value2 != null)
+                {
+                    newProperties.put(ContentModel.PROP_AUTO_VERSION_PROPS, value2);
+                }
+                
+                return newProperties;
             }
             else
             {
@@ -248,11 +240,10 @@ public class VersionableAspect implements ContentServicePolicies.OnContentUpdate
                 Map<NodeRef, NodeRef> versionedNodeRefs = (Map<NodeRef, NodeRef>) AlfrescoTransactionSupport.getResource(KEY_VERSIONED_NODEREFS);
                 if (versionedNodeRefs == null || versionedNodeRefs.containsKey(nodeRef) == false)
                 {
-                    // Queue create version action
+                    // Create the initial-version
-                    Map<String, Serializable> versionDetails = new HashMap<String, Serializable>(1);
+                    Map<String, Serializable> versionProperties = new HashMap<String, Serializable>(1);
-                    versionDetails.put(Version.PROP_DESCRIPTION, I18NUtil.getMessage(MSG_INITIAL_VERSION));
+                    versionProperties.put(Version.PROP_DESCRIPTION, I18NUtil.getMessage(MSG_INITIAL_VERSION));
-                    
+                    this.versionService.createVersion(nodeRef, versionProperties);
-                    this.versionService.createVersion(nodeRef, versionDetails);
                 }
             }
         }
@@ -263,25 +254,26 @@ public class VersionableAspect implements ContentServicePolicies.OnContentUpdate
      */
     public void onRemoveAspect(NodeRef nodeRef, QName aspectTypeQName) 
     {
-        // When the versionable aspect is removed from a node, then delete the associatied verison history
+        // When the versionable aspect is removed from a node, then delete the associated version history
         this.versionService.deleteVersionHistory(nodeRef);
     }
     
     /**
-     * On content update policy bahaviour
+     * On content update policy behaviour
      * 
-     * @param nodeRef   the node reference
+     * If applicable and "cm:autoVersion" is TRUE then version the node on content update (even if no property updates)
      */
     @SuppressWarnings("unchecked")
     public void onContentUpdate(NodeRef nodeRef, boolean newContent)
     {
-        if (this.nodeService.exists(nodeRef) == true && this.nodeService.hasAspect(nodeRef, ContentModel.ASPECT_VERSIONABLE) == true
+        if (this.nodeService.exists(nodeRef) == true && 
-                && this.nodeService.hasAspect(nodeRef, ContentModel.ASPECT_TEMPORARY) == false)
+            this.nodeService.hasAspect(nodeRef, ContentModel.ASPECT_VERSIONABLE) == true && 
+            this.nodeService.hasAspect(nodeRef, ContentModel.ASPECT_TEMPORARY) == false)
         {
             Map<NodeRef, NodeRef> versionedNodeRefs = (Map)AlfrescoTransactionSupport.getResource(KEY_VERSIONED_NODEREFS);
             if (versionedNodeRefs == null || versionedNodeRefs.containsKey(nodeRef) == false)
             {
-                // Determine whether the node is auto versionable or not
+                // Determine whether the node is auto versionable (for content updates) or not
                 boolean autoVersion = false;
                 Boolean value = (Boolean)this.nodeService.getProperty(nodeRef, ContentModel.PROP_AUTO_VERSION);
                 if (value != null)
@@ -292,24 +284,21 @@ public class VersionableAspect implements ContentServicePolicies.OnContentUpdate
                 // else this means that the default value has not been set and the versionable aspect was applied pre-1.1
                 
                 if (autoVersion == true)
-                {
-                    if (deprecatedNotOnUpdateProperties)
                 {
                     // Create the auto-version
                     Map<String, Serializable> versionProperties = new HashMap<String, Serializable>(1);
                     versionProperties.put(Version.PROP_DESCRIPTION, I18NUtil.getMessage(MSG_AUTO_VERSION));
                     this.versionService.createVersion(nodeRef, versionProperties);
                 }
-                    else
-                    {
-                        queueVersion(nodeRef);
-                    }
-                }
             }
         }
     }
     
     /**
+     * On update properties policy behaviour 
+     * 
+     * If applicable and "cm:autoVersionOnUpdateProps" is TRUE then version the node on properties update (even if no content updates)
+     * 
      * @since 3.2
      */
     @SuppressWarnings("unchecked")
@@ -318,15 +307,15 @@ public class VersionableAspect implements ContentServicePolicies.OnContentUpdate
             Map<QName, Serializable> before,
             Map<QName, Serializable> after)
     {
-        if ((!deprecatedNotOnUpdateProperties) && 
+        if ((this.nodeService.exists(nodeRef) == true) && 
-            (this.nodeService.exists(nodeRef) == true) && 
             (this.nodeService.hasAspect(nodeRef, ContentModel.ASPECT_VERSIONABLE) == true) && 
-            (this.nodeService.hasAspect(nodeRef, ContentModel.ASPECT_TEMPORARY) == false))
+            (this.nodeService.hasAspect(nodeRef, ContentModel.ASPECT_TEMPORARY) == false) &&
+            (this.nodeService.hasAspect(nodeRef, ContentModel.ASPECT_LOCKABLE) == false))
         {
             Map<NodeRef, NodeRef> versionedNodeRefs = (Map)AlfrescoTransactionSupport.getResource(KEY_VERSIONED_NODEREFS);
             if (versionedNodeRefs == null || versionedNodeRefs.containsKey(nodeRef) == false)
             {
-                // Determine whether the node is auto versionable or not
+                // Determine whether the node is auto versionable (for property only updates) or not
                 boolean autoVersion = false;
                 Boolean value = (Boolean)this.nodeService.getProperty(nodeRef, ContentModel.PROP_AUTO_VERSION);
                 if (value != null)
@@ -334,11 +323,21 @@ public class VersionableAspect implements ContentServicePolicies.OnContentUpdate
                     // If the value is not null then 
                     autoVersion = value.booleanValue();
                 }
-                // else this means that the default value has not been set and the versionable aspect was applied pre-1.1
                 
-                if (autoVersion == true)
+                boolean autoVersionProps = false;
+                value = (Boolean)this.nodeService.getProperty(nodeRef, ContentModel.PROP_AUTO_VERSION_PROPS);
+                if (value != null)
                 {
-                    queueVersion(nodeRef);
+                    // If the value is not null then 
+                    autoVersionProps = value.booleanValue();
+                }
+                
+                if ((autoVersion == true) && (autoVersionProps == true))
+                {
+                    // Create the auto-version
+                    Map<String, Serializable> versionProperties = new HashMap<String, Serializable>(1);
+                    versionProperties.put(Version.PROP_DESCRIPTION, I18NUtil.getMessage(MSG_AUTO_VERSION_PROPS));
+                    this.versionService.createVersion(nodeRef, versionProperties);
                 }
             }
         }
@@ -358,42 +357,4 @@ public class VersionableAspect implements ContentServicePolicies.OnContentUpdate
         }
         versionedNodeRefs.put(versionableNode, versionableNode);
     }
-    
-    @SuppressWarnings("unchecked")
-    private void queueVersion(NodeRef nodeRef)
-    {
-        Set<NodeRef> pendingNodeRefs = (Set<NodeRef>)AlfrescoTransactionSupport.getResource(KEY_PENDING_NODEREFS);
-        if (pendingNodeRefs == null)
-        {
-            pendingNodeRefs = new HashSet<NodeRef>();
-            AlfrescoTransactionSupport.bindResource(KEY_PENDING_NODEREFS, pendingNodeRefs);
-        }
-        pendingNodeRefs.add(nodeRef);
-        
-        AlfrescoTransactionSupport.bindListener(this.transactionListener);
-    }
-    
-    public class VersionableAspectTransactionListener extends TransactionListenerAdapter
-    {
-        /**
-         * @see org.alfresco.repo.transaction.TransactionListener#beforeCommit(boolean)
-         */
-        @SuppressWarnings("unchecked")
-        @Override
-        public void beforeCommit(boolean readOnly)
-        { 
-            Set<NodeRef> pendingNodeRefs = (Set<NodeRef>)AlfrescoTransactionSupport.getResource(KEY_PENDING_NODEREFS);
-            
-            if (pendingNodeRefs != null)
-            {
-                for (NodeRef nodeRef : pendingNodeRefs)
-                {
-                    // Create the auto-version
-                    Map<String, Serializable> versionProperties = new HashMap<String, Serializable>(1);
-                    versionProperties.put(Version.PROP_DESCRIPTION, I18NUtil.getMessage(MSG_AUTO_VERSION));
-                    versionService.createVersion(nodeRef, versionProperties);
-                }
-            }
-        }
-    }
 }