Refacoting of RecordService implementation based on SONAR report

* detangle of services
 * simplification of spring dependancies
 * use of base service impl
 * unit tests and Ui integraion tests run locally 



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

rm-server/config/alfresco/module/org_alfresco_module_rm/rm-model-context.xml

@@ -49,9 +49,7 @@
       <property name="recordFolderService" ref="RecordFolderService"/>
    </bean>
 
-   <bean id="rm.baseBehaviour" abstract="true">
+   <bean id="rm.baseBehaviour" abstract="true" parent="baseService">
-        <property name="nodeService" ref="NodeService" />
-      <property name="dictionaryService" ref="DictionaryService" />
       <property name="behaviourFilter" ref="policyBehaviourFilter" />
    </bean>
 

rm-server/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml

@@ -46,8 +46,7 @@
 
     <!--  Vital Record Service -->
 
-    <bean id="vitalRecordService" class="org.alfresco.module.org_alfresco_module_rm.vital.VitalRecordServiceImpl">
+    <bean id="vitalRecordService" parent="baseService" class="org.alfresco.module.org_alfresco_module_rm.vital.VitalRecordServiceImpl">
-        <property name="nodeService" ref="NodeService"/>
         <property name="filePlanService" ref="FilePlanService" />
     </bean>
 
@@ -98,10 +97,8 @@
     </bean>
 
     <!--  Disposition Service -->
-    <bean id="dispositionService" class="org.alfresco.module.org_alfresco_module_rm.disposition.DispositionServiceImpl">
+    <bean id="dispositionService" parent="baseService" class="org.alfresco.module.org_alfresco_module_rm.disposition.DispositionServiceImpl">
         <property name="recordsManagementServiceRegistry" ref="RecordsManagementServiceRegistry"/>
-        <property name="nodeService" ref="nodeService"/>
-        <property name="dictionaryService" ref="DictionaryService"/>
         <property name="behaviourFilter" ref="policyBehaviourFilter" />
         <property name="filePlanService" ref="FilePlanService" />
         <property name="recordFolderService" ref="RecordFolderService"/>
@@ -633,7 +630,6 @@
     <bean id="extendedSecurityService"
           class="org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityServiceImpl"
           parent="baseService">
-        <property name="recordService" ref="recordService"/>
         <property name="nodeService" ref="nodeService"/>
         <property name="filePlanService" ref="filePlanService" />
         <property name="filePlanRoleService" ref="filePlanRoleService" />
@@ -1166,14 +1162,11 @@
 
     <!--  Record Service -->
 
-    <bean id="recordService" class="org.alfresco.module.org_alfresco_module_rm.record.RecordServiceImpl" init-method="init">
+    <bean id="recordService" parent="rm.baseBehaviour" class="org.alfresco.module.org_alfresco_module_rm.record.RecordServiceImpl" init-method="init">
-        <property name="nodeService" ref="NodeService"/>
-        <property name="dictionaryService" ref="DictionaryService"/>
         <property name="identifierService" ref="recordsManagementIdentifierService"/>
         <property name="extendedPermissionService" ref="ExtendedPermissionService"/>
         <property name="extendedSecurityService" ref="ExtendedSecurityService" />
         <property name="policyComponent" ref="policyComponent" />
-        <property name="dispositionService" ref="DispositionService" />
         <property name="filePlanService" ref="FilePlanService" />
         <property name="ownableService" ref="OwnableService" />
         <property name="notificationHelper" ref="recordsManagementNotificationHelper"/>
@@ -1181,10 +1174,8 @@
         <property name="ruleService" ref="RuleService" />
         <property name="fileFolderService" ref="FileFolderService" />
         <property name="recordFolderService" ref="RecordFolderService" />
-        <property name="vitalRecordService" ref="VitalRecordService" />
         <property name="filePlanRoleService" ref="FilePlanRoleService" />
         <property name="permissionService" ref="permissionService" />
-        <property name="recordAspect" ref="rma.record" />
     </bean>
 
     <bean id="RecordService" class="org.springframework.aop.framework.ProxyFactoryBean">

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/RecordsManagementPolicies.java

@@ -71,9 +71,50 @@ public interface RecordsManagementPolicies
         public void beforeRemoveReference(NodeRef fromNodeRef, NodeRef toNodeRef, QName reference);
     }
     
-    /** On removal of reference */
+    /** 
+     * On removal of reference
+     * 
+     * @since 1.0 
+     */
     public interface OnRemoveReference extends ClassPolicy
     {
+        /**
+         * @param fromNodeRef   from node reference
+         * @param toNodeRef     to node reference
+         * @param reference     name of reference
+         */
         public void onRemoveReference(NodeRef fromNodeRef, NodeRef toNodeRef, QName reference);
     }
+    
+    /**
+     * Before record file policy
+     * 
+     * @since 2.2 
+     */
+    public interface BeforeFileRecord extends ClassPolicy
+    {
+        /** policy name */
+        public static final QName QNAME = QName.createQName(NamespaceService.ALFRESCO_URI, "beforeRecordFile");
+        
+        /**
+         * @param nodeRef   node reference
+         */
+        public void beforeFileRecord(NodeRef nodeRef);
+    }
+    
+    /**
+     * On record file policy
+     * 
+     * @since 2.2
+     */
+    public interface OnFileRecord extends ClassPolicy
+    {
+        /** policy name */
+        public static final QName QNAME = QName.createQName(NamespaceService.ALFRESCO_URI, "onRecordFile");
+        
+        /**
+         * @param nodeRef   node reference
+         */
+        public void onFileRecord(NodeRef nodeRef);
+    }
 }

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/action/impl/CutOffAction.java

@@ -38,17 +38,8 @@ public class CutOffAction extends RMDispositionActionExecuterAbstractBase
     @Override
     protected void executeRecordFolderLevelDisposition(Action action, NodeRef recordFolder)
     {
-        // Close the record folder
-        recordFolderService.closeRecordFolder(recordFolder);
-
         // Mark the folder as cut off
         dispositionService.cutoffDisposableItem(recordFolder);
-
-        // Mark all the declared children of the folder as cut off
-        for (NodeRef record : recordService.getRecords(recordFolder))
-        {
-            dispositionService.cutoffDisposableItem(record);
-        }
     }
 
     /**

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/capability/declarative/DeclarativeCapability.java

@@ -216,7 +216,7 @@ public class DeclarativeCapability extends AbstractCapability
                     
                     if (logger.isDebugEnabled() == true)
                     {
-                        logger.debug("Condition " + condition.getName() + " failed for capability " + getName() + " on nodeRef " + nodeRef.toString());
+                        logger.debug("FAIL: Condition " + condition.getName() + " failed for capability " + getName() + " on nodeRef " + nodeRef.toString());
                     }
                     
                     break;
@@ -305,7 +305,7 @@ public class DeclarativeCapability extends AbstractCapability
         // log access denied to help with debug
         if (logger.isDebugEnabled() == true && AccessDecisionVoter.ACCESS_DENIED == result)
         {
-            logger.debug("Capability " + getName() + " returned an Access Denied result during evaluation of node " + nodeRef.toString());
+            logger.debug("FAIL: Capability " + getName() + " returned an Access Denied result during evaluation of node " + nodeRef.toString());
         }
         
         return result;

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/capability/declarative/condition/CutoffCapabilityCondition.java

@@ -22,6 +22,8 @@ import org.alfresco.module.org_alfresco_module_rm.capability.declarative.Abstrac
 import org.alfresco.service.cmr.repository.NodeRef;
 
 /**
+ * Cut off capability condition.
+ * 
  * @author Roy Wetherall
  */
 public class CutoffCapabilityCondition extends AbstractCapabilityCondition
@@ -32,6 +34,7 @@ public class CutoffCapabilityCondition extends AbstractCapabilityCondition
     @Override
     public boolean evaluate(NodeRef nodeRef)
     {
+        // determine whether the item is already cutt off
         return dispositionService.isDisposableItemCutoff(nodeRef);
     }
 

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/capability/impl/UpdateCapability.java

@@ -32,25 +32,23 @@ import org.alfresco.service.namespace.QName;
  */
 public class UpdateCapability extends DeclarativeCompositeCapability
 {
+    @Override
+    public int evaluate(NodeRef nodeRef)
+    {
+        // TODO Auto-generated method stub
+        return super.evaluate(nodeRef);
+    }
+    
     /**
+     * Evaluate capability
      * 
-     * @param nodeRef
+     * @param nodeRef       node reference
-     * @param aspectQName
+     * @param aspectQName   aspect qname
-     * @param properties
+     * @param properties    property values
      * @return 
      */
     public int evaluate(NodeRef nodeRef, QName aspectQName, Map<QName, Serializable> properties)
     {
-       // if ((aspectQName != null) && (voter.isProtectedAspect(nodeRef, aspectQName)))
-       // {
-       //     return AccessDecisionVoter.ACCESS_DENIED;
-       // }
-        
-      //  if ((properties != null) && (voter.includesProtectedPropertyChange(nodeRef, properties)))
-      //  {
-      //      return AccessDecisionVoter.ACCESS_DENIED;
-      //  }
-        
         return evaluate(nodeRef);
     }
 }

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/disposition/DispositionServiceImpl.java

@@ -27,15 +27,23 @@ import java.util.List;
 import java.util.Map;
 
 import org.alfresco.error.AlfrescoRuntimeException;
+import org.alfresco.module.org_alfresco_module_rm.RecordsManagementPolicies;
 import org.alfresco.module.org_alfresco_module_rm.RecordsManagementServiceRegistry;
 import org.alfresco.module.org_alfresco_module_rm.disposition.property.DispositionProperty;
 import org.alfresco.module.org_alfresco_module_rm.event.RecordsManagementEvent;
 import org.alfresco.module.org_alfresco_module_rm.event.RecordsManagementEventType;
+import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanComponentKind;
 import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService;
 import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel;
 import org.alfresco.module.org_alfresco_module_rm.record.RecordService;
 import org.alfresco.module.org_alfresco_module_rm.recordfolder.RecordFolderService;
+import org.alfresco.module.org_alfresco_module_rm.util.ServiceBaseImpl;
 import org.alfresco.repo.policy.BehaviourFilter;
+import org.alfresco.repo.policy.annotation.Behaviour;
+import org.alfresco.repo.policy.annotation.BehaviourBean;
+import org.alfresco.repo.policy.annotation.BehaviourKind;
+import org.alfresco.repo.security.authentication.AuthenticationUtil;
+import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
 import org.alfresco.service.cmr.dictionary.DictionaryService;
 import org.alfresco.service.cmr.repository.ChildAssociationRef;
 import org.alfresco.service.cmr.repository.NodeRef;
@@ -53,19 +61,15 @@ import org.apache.commons.logging.LogFactory;
  *
  * @author Roy Wetherall
  */
-public class DispositionServiceImpl implements
+@BehaviourBean
-                                        DispositionService,
+public class DispositionServiceImpl extends    ServiceBaseImpl
-                                        RecordsManagementModel
+                                    implements DispositionService,
+                                               RecordsManagementModel,
+                                               RecordsManagementPolicies.OnFileRecord
 {
     /** Logger */
     private static Log logger = LogFactory.getLog(DispositionServiceImpl.class);
 
-    /** Node service */
-    private NodeService nodeService;
-
-    /** Dictionary service */
-    private DictionaryService dictionaryService;
-
     /** Behaviour filter */
     private BehaviourFilter behaviourFilter;
 
@@ -161,6 +165,26 @@ public class DispositionServiceImpl implements
         this.dispositionSelectionStrategy = dispositionSelectionStrategy;
     }
     
+    /**
+     * Behavior to initialize the disposition schedule of a newly filed record.
+     * 
+     * @see org.alfresco.module.org_alfresco_module_rm.RecordsManagementPolicies.OnFileRecord#onFileRecord(org.alfresco.service.cmr.repository.NodeRef)
+     */
+    @Override
+    @Behaviour(kind=BehaviourKind.CLASS, type="rma:record")
+    public void onFileRecord(NodeRef nodeRef) 
+    {
+        // initialise disposition details
+        if (nodeService.hasAspect(nodeRef, ASPECT_DISPOSITION_LIFECYCLE) == false)
+        {
+            DispositionSchedule di = getDispositionSchedule(nodeRef);
+            if (di != null && di.isRecordLevelDisposition() == true)
+            {
+                nodeService.addAspect(nodeRef, ASPECT_DISPOSITION_LIFECYCLE, null);
+            }
+        }
+    };
+
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.disposition.DispositionService#refreshDispositionAction(NodeRef)
      */
@@ -233,7 +257,7 @@ public class DispositionServiceImpl implements
     {
         DispositionSchedule di = null;
         NodeRef diNodeRef = null;
-        if (recordService.isRecord(nodeRef) == true)
+        if (isRecord(nodeRef) == true)
         {
             // Get the record folders for the record
             List<NodeRef> recordFolders = recordFolderService.getRecordFolders(nodeRef);
@@ -769,7 +793,9 @@ public class DispositionServiceImpl implements
 
     /** ========= Disposition Action History Methods ========= */
 
-
+    /**
+     * @see org.alfresco.module.org_alfresco_module_rm.disposition.DispositionService#getCompletedDispositionActions(org.alfresco.service.cmr.repository.NodeRef)
+     */
     public List<DispositionAction> getCompletedDispositionActions(NodeRef nodeRef)
     {
         List<ChildAssociationRef> assocs = nodeService.getChildAssocs(nodeRef, ASSOC_DISPOSITION_ACTION_HISTORY, RegexQNamePattern.MATCH_ALL);
@@ -782,6 +808,9 @@ public class DispositionServiceImpl implements
         return result;
     }
 
+    /**
+     * @see org.alfresco.module.org_alfresco_module_rm.disposition.DispositionService#getLastCompletedDispostionAction(org.alfresco.service.cmr.repository.NodeRef)
+     */
     public DispositionAction getLastCompletedDispostionAction(NodeRef nodeRef)
     {
        DispositionAction result = null;
@@ -808,10 +837,18 @@ public class DispositionServiceImpl implements
      * @see org.alfresco.module.org_alfresco_module_rm.disposition.DispositionService#updateNextDispositionAction(NodeRef)
      */
     @Override
-    public void updateNextDispositionAction(NodeRef nodeRef)
+    public void updateNextDispositionAction(final NodeRef nodeRef)
     {
         ParameterCheck.mandatory("nodeRef", nodeRef);
 
+        RunAsWork<Void> runAsWork = new RunAsWork<Void>()
+        {
+            /**
+             * @see org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork#doWork()
+             */
+            @Override
+            public Void doWork() throws Exception
+            {
                 // Get this disposition instructions for the node
                 DispositionSchedule di = getDispositionSchedule(nodeRef);
                 if (di != null)
@@ -926,6 +963,12 @@ public class DispositionServiceImpl implements
                         }
                     }
                 }
+                
+                return null;
+            }
+        };
+
+        AuthenticationUtil.runAsSystem(runAsWork);
     }
 
     /**
@@ -936,12 +979,42 @@ public class DispositionServiceImpl implements
     {
         ParameterCheck.mandatory("nodeRef", nodeRef);
         
+        // check that the node ref is a filed record or record folder
+        if (FilePlanComponentKind.RECORD_FOLDER.equals(filePlanService.getFilePlanComponentKind(nodeRef)) ||
+            FilePlanComponentKind.RECORD.equals(filePlanService.getFilePlanComponentKind(nodeRef)))
+        {
             if (isDisposableItemCutoff(nodeRef) == false)
+            {
+                if (recordFolderService.isRecordFolder(nodeRef))
+                {
+                    // cut off all the children first
+                    for (NodeRef record : recordService.getRecords(nodeRef))
+                    {
+                        applyCutoff(record);
+                    }
+                }
+                
+                // apply cut off
+                applyCutoff(nodeRef);
+                
+                if (recordFolderService.isRecordFolder(nodeRef))
+                {
+                    // close folder (manually since we can't normall close a folder that is cut off!!
+                    nodeService.setProperty(nodeRef, PROP_IS_CLOSED, true);
+                }
+            }
+        }
+        else
+        {
+            throw new AlfrescoRuntimeException("Unable to peform cutoff, because node is not a disposible item. (nodeRef=" + nodeRef.toString() + ")");
+        }
+    }
+    
+    private void applyCutoff(NodeRef nodeRef)
     {
         // Apply the cut off aspect and set cut off date
         Map<QName, Serializable> cutOffProps = new HashMap<QName, Serializable>(1);
         cutOffProps.put(PROP_CUT_OFF_DATE, new Date());
         nodeService.addAspect(nodeRef, ASPECT_CUT_OFF, cutOffProps);
     }
-    }
 }

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/model/BaseBehaviourBean.java

@@ -7,13 +7,9 @@ import java.util.HashMap;
 import java.util.Map;
 
 import org.alfresco.error.AlfrescoRuntimeException;
+import org.alfresco.module.org_alfresco_module_rm.util.ServiceBaseImpl;
 import org.alfresco.repo.policy.BehaviourFilter;
 import org.alfresco.repo.policy.annotation.BehaviourRegistry;
-import org.alfresco.service.cmr.dictionary.DictionaryService;
-import org.alfresco.service.cmr.repository.NodeRef;
-import org.alfresco.service.cmr.repository.NodeService;
-import org.alfresco.service.namespace.QName;
-import org.alfresco.util.ParameterCheck;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
@@ -23,40 +19,19 @@ import org.apache.commons.logging.LogFactory;
  * @author Roy Wetherall
  * @since 2.2
  */
-public abstract class BaseBehaviourBean implements RecordsManagementModel,
+public abstract class BaseBehaviourBean extends ServiceBaseImpl
+                                        implements RecordsManagementModel,
                                                    BehaviourRegistry
 {
     /** Logger */
     protected static Log logger = LogFactory.getLog(BaseBehaviourBean.class);
     
-    /** node service */
-    protected NodeService nodeService;
-    
-    /** dictionary service */
-    protected DictionaryService dictionaryService;
-    
     /** behaviour filter */
     protected BehaviourFilter behaviourFilter;
     
     /** behaviour map */
     protected Map<String, org.alfresco.repo.policy.Behaviour> behaviours = new HashMap<String, org.alfresco.repo.policy.Behaviour>(7);
     
-    /**
-     * @param nodeService   node service
-     */
-    public void setNodeService(NodeService nodeService)
-    {
-        this.nodeService = nodeService;
-    }
-    
-    /**
-     * @param dictionaryService dictionary service
-     */
-    public void setDictionaryService(DictionaryService dictionaryService)
-    {
-        this.dictionaryService = dictionaryService;
-    }
-    
     /**
      * @param behaviourFilter   behaviour filter
      */
@@ -65,26 +40,6 @@ public abstract class BaseBehaviourBean implements RecordsManagementModel,
         this.behaviourFilter = behaviourFilter;
     }
 
-    /**
-     * Utility method to safely and quickly determine if a node is a type (or sub-type) of the one specified.
-     * 
-     * @param nodeRef       node reference
-     * @param ofClassName   class name to check
-     */
-    protected boolean instanceOf(NodeRef nodeRef, QName ofClassName)
-    {
-        ParameterCheck.mandatory("nodeRef", nodeRef);
-        ParameterCheck.mandatory("ofClassName", ofClassName);
-        boolean result = false;
-        if (nodeService.exists(nodeRef) == true &&
-            (ofClassName.equals(nodeService.getType(nodeRef)) == true ||
-             dictionaryService.isSubClass(nodeService.getType(nodeRef), ofClassName) == true))            
-        {
-            result = true;
-        }    
-        return result;
-    }
-
     /**
      * @see org.alfresco.repo.policy.annotation.BehaviourRegistry#registerBehaviour(java.lang.String, org.alfresco.repo.policy.Behaviour)
      */

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspect.java

@@ -19,7 +19,6 @@
 package org.alfresco.module.org_alfresco_module_rm.model.rma.aspect;
 
 import java.io.Serializable;
-import java.util.Date;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
@@ -29,7 +28,6 @@ import org.alfresco.module.org_alfresco_module_rm.RecordsManagementPolicies;
 import org.alfresco.module.org_alfresco_module_rm.model.BaseBehaviourBean;
 import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementCustomModel;
 import org.alfresco.module.org_alfresco_module_rm.model.behaviour.RecordsManagementSearchBehaviour;
-import org.alfresco.module.org_alfresco_module_rm.model.security.ModelAccessDeniedException;
 import org.alfresco.module.org_alfresco_module_rm.record.RecordService;
 import org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService;
 import org.alfresco.repo.copy.CopyBehaviourCallback;
@@ -46,7 +44,6 @@ import org.alfresco.service.cmr.repository.ChildAssociationRef;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.ScriptService;
 import org.alfresco.service.namespace.QName;
-import org.alfresco.util.EqualsHelper;
 
 /**
  * rma:record behaviour bean
@@ -62,8 +59,7 @@ public class RecordAspect extends    BaseBehaviourBean
                           implements NodeServicePolicies.OnCreateChildAssociationPolicy,
                                      RecordsManagementPolicies.OnCreateReference,
                                      RecordsManagementPolicies.OnRemoveReference,
-                                     NodeServicePolicies.OnMoveNodePolicy,
+                                     NodeServicePolicies.OnMoveNodePolicy
-                                     NodeServicePolicies.OnUpdatePropertiesPolicy
 {
     /** Well-known location of the scripts folder. */
     // TODO make configurable
@@ -252,66 +248,6 @@ public class RecordAspect extends    BaseBehaviourBean
         }
     }
     
-    /**
-     * Ensure that the user only updates record properties that they have permission to.
-     *
-     * @see org.alfresco.repo.node.NodeServicePolicies.OnUpdatePropertiesPolicy#onUpdateProperties(org.alfresco.service.cmr.repository.NodeRef, java.util.Map, java.util.Map)
-     */
-    @Override
-    @Behaviour
-    (
-            name = "onUpdateProperties",
-            kind = BehaviourKind.CLASS
-    )
-    public void onUpdateProperties(final NodeRef nodeRef, final Map<QName, Serializable> before, final Map<QName, Serializable> after)
-    {
-        if (AuthenticationUtil.getFullyAuthenticatedUser() != null &&
-            AuthenticationUtil.isRunAsUserTheSystemUser() == false &&
-            nodeService.exists(nodeRef) == true)
-        {
-            if (recordService.isRecord(nodeRef) == true)
-            {
-                for (QName property : after.keySet())
-                {
-                    Serializable beforeValue = null;
-                    if (before != null)
-                    {
-                        beforeValue = before.get(property);
-                    }
-
-                    Serializable afterValue = null;
-                    if (after != null)
-                    {
-                        afterValue = after.get(property);
-                    }
-
-                    boolean propertyUnchanged = false;
-                    if (beforeValue != null && afterValue != null &&
-                        beforeValue instanceof Date && afterValue instanceof Date)
-                    {
-                        // deal with date values
-                        propertyUnchanged = (((Date)beforeValue).compareTo((Date)afterValue) == 0);
-                    }
-                    else
-                    {
-                        // otherwise
-                        propertyUnchanged = EqualsHelper.nullSafeEquals(beforeValue, afterValue);
-                    }
-
-                    if (propertyUnchanged == false &&
-                        recordService.isPropertyEditable(nodeRef, property) == false)
-                    {
-                        // the user can't edit the record property
-                        throw new ModelAccessDeniedException(
-                            "The user " + AuthenticationUtil.getFullyAuthenticatedUser() +
-                            " does not have the permission to edit the record property " + property.toString() +
-                            " on the node " + nodeRef.toString());
-                    }
-                }
-            }
-        }
-    }
-    
     /**
      * Executes a reference script if present
      *

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/record/RecordServiceImpl.java

@@ -31,25 +31,26 @@ import java.util.Set;
 
 import org.alfresco.error.AlfrescoRuntimeException;
 import org.alfresco.model.ContentModel;
+import org.alfresco.module.org_alfresco_module_rm.RecordsManagementPolicies.BeforeFileRecord;
+import org.alfresco.module.org_alfresco_module_rm.RecordsManagementPolicies.OnFileRecord;
 import org.alfresco.module.org_alfresco_module_rm.capability.Capability;
 import org.alfresco.module.org_alfresco_module_rm.capability.CapabilityService;
 import org.alfresco.module.org_alfresco_module_rm.capability.RMPermissionModel;
-import org.alfresco.module.org_alfresco_module_rm.disposition.DispositionSchedule;
-import org.alfresco.module.org_alfresco_module_rm.disposition.DispositionService;
 import org.alfresco.module.org_alfresco_module_rm.dod5015.DOD5015Model;
 import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService;
 import org.alfresco.module.org_alfresco_module_rm.identifier.IdentifierService;
+import org.alfresco.module.org_alfresco_module_rm.model.BaseBehaviourBean;
 import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementCustomModel;
 import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel;
-import org.alfresco.module.org_alfresco_module_rm.model.rma.aspect.RecordAspect;
+import org.alfresco.module.org_alfresco_module_rm.model.security.ModelAccessDeniedException;
 import org.alfresco.module.org_alfresco_module_rm.notification.RecordsManagementNotificationHelper;
 import org.alfresco.module.org_alfresco_module_rm.recordfolder.RecordFolderService;
 import org.alfresco.module.org_alfresco_module_rm.role.FilePlanRoleService;
 import org.alfresco.module.org_alfresco_module_rm.role.Role;
 import org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService;
-import org.alfresco.module.org_alfresco_module_rm.vital.VitalRecordService;
 import org.alfresco.repo.node.NodeServicePolicies;
 import org.alfresco.repo.policy.Behaviour.NotificationFrequency;
+import org.alfresco.repo.policy.ClassPolicyDelegate;
 import org.alfresco.repo.policy.JavaBehaviour;
 import org.alfresco.repo.policy.PolicyComponent;
 import org.alfresco.repo.policy.annotation.Behaviour;
@@ -62,7 +63,6 @@ import org.alfresco.repo.security.permissions.impl.ExtendedPermissionService;
 import org.alfresco.repo.transaction.TransactionalResourceHelper;
 import org.alfresco.service.cmr.dictionary.AspectDefinition;
 import org.alfresco.service.cmr.dictionary.ClassDefinition;
-import org.alfresco.service.cmr.dictionary.DictionaryService;
 import org.alfresco.service.cmr.dictionary.PropertyDefinition;
 import org.alfresco.service.cmr.model.FileExistsException;
 import org.alfresco.service.cmr.model.FileFolderService;
@@ -73,7 +73,6 @@ import org.alfresco.service.cmr.repository.ContentReader;
 import org.alfresco.service.cmr.repository.ContentWriter;
 import org.alfresco.service.cmr.repository.InvalidNodeRefException;
 import org.alfresco.service.cmr.repository.NodeRef;
-import org.alfresco.service.cmr.repository.NodeService;
 import org.alfresco.service.cmr.rule.RuleService;
 import org.alfresco.service.cmr.security.AccessPermission;
 import org.alfresco.service.cmr.security.AccessStatus;
@@ -82,6 +81,7 @@ import org.alfresco.service.cmr.security.PermissionService;
 import org.alfresco.service.namespace.NamespaceService;
 import org.alfresco.service.namespace.QName;
 import org.alfresco.service.namespace.RegexQNamePattern;
+import org.alfresco.util.EqualsHelper;
 import org.alfresco.util.ParameterCheck;
 import org.apache.commons.lang.ArrayUtils;
 import org.apache.commons.lang.StringUtils;
@@ -96,12 +96,14 @@ import org.springframework.extensions.surf.util.I18NUtil;
  * @since 2.1
  */
 @BehaviourBean
-public class RecordServiceImpl implements RecordService,
+public class RecordServiceImpl extends BaseBehaviourBean 
+                               implements RecordService,
                                           RecordsManagementModel,
                                           RecordsManagementCustomModel,
                                           NodeServicePolicies.OnCreateChildAssociationPolicy,
                                           NodeServicePolicies.OnAddAspectPolicy,
-                                          NodeServicePolicies.OnRemoveAspectPolicy
+                                          NodeServicePolicies.OnRemoveAspectPolicy,
+                                          NodeServicePolicies.OnUpdatePropertiesPolicy
 {
     /** Logger */
     private static Log logger = LogFactory.getLog(RecordServiceImpl.class);
@@ -146,24 +148,15 @@ public class RecordServiceImpl implements RecordService,
 
     };
 
-    /** Node service **/
-    private NodeService nodeService;
-
     /** Indentity service */
     private IdentifierService identifierService;
 
-    /** Dictionary service */
-    private DictionaryService dictionaryService;
-
     /** Extended permission service */
     private ExtendedPermissionService extendedPermissionService;
 
     /** Extended security service */
     private ExtendedSecurityService extendedSecurityService;
 
-    /** Disposition service */
-    private DispositionService dispositionService;
-
     /** File plan service */
     private FilePlanService filePlanService;
 
@@ -188,21 +181,19 @@ public class RecordServiceImpl implements RecordService,
     /** Record folder service */
     private RecordFolderService recordFolderService;
 
-    /** Vital record service */
-    private VitalRecordService vitalRecordService;
-
     /** File plan role service */
     private FilePlanRoleService filePlanRoleService;
 
     /** Permission service */
     private PermissionService permissionService;
 
-    /** Record aspect */
-    private RecordAspect recordAspect;
-
     /** List of available record meta-data aspects */
     private Set<QName> recordMetaDataAspects;
     
+    /** policies */
+    private ClassPolicyDelegate<BeforeFileRecord> beforeFileRecord;
+    private ClassPolicyDelegate<OnFileRecord> onFileRecord;
+
     /** Behaviours */
     private JavaBehaviour onCreateChildAssociation = new JavaBehaviour(
                                                             this,
@@ -213,14 +204,6 @@ public class RecordServiceImpl implements RecordService,
                                                             "onDeleteDeclaredRecordLink",
                                                             NotificationFrequency.FIRST_EVENT);
 
-    /**
-     * @param nodeService node service
-     */
-    public void setNodeService(NodeService nodeService)
-    {
-        this.nodeService = nodeService;
-    }
-
     /**
      * @param identifierService identifier service
      */
@@ -229,14 +212,6 @@ public class RecordServiceImpl implements RecordService,
         this.identifierService = identifierService;
     }
 
-    /**
-     * @param dictionaryService dictionary service
-     */
-    public void setDictionaryService(DictionaryService dictionaryService)
-    {
-        this.dictionaryService = dictionaryService;
-    }
-
     /**
      * @param extendedPermissionService extended permission service
      */
@@ -253,14 +228,6 @@ public class RecordServiceImpl implements RecordService,
         this.extendedSecurityService = extendedSecurityService;
     }
 
-    /**
-     * @param dispositionService    disposition service
-     */
-    public void setDispositionService(DispositionService dispositionService)
-    {
-        this.dispositionService = dispositionService;
-    }
-
     /**
      * @param filePlanService   file plan service
      */
@@ -325,14 +292,6 @@ public class RecordServiceImpl implements RecordService,
         this.recordFolderService = recordFolderService;
     }
 
-    /**
-     * @param vitalRecordService vital record service
-     */
-    public void setVitalRecordService(VitalRecordService vitalRecordService)
-    {
-        this.vitalRecordService = vitalRecordService;
-    }
-
     /**
      * @param filePlanRoleService file plan role service
      */
@@ -349,25 +308,21 @@ public class RecordServiceImpl implements RecordService,
         this.permissionService = permissionService;
     }
 
-    /**
-     * @param recordAspect  record aspect
-     */
-    public void setRecordAspect(RecordAspect recordAspect)
-    {
-        this.recordAspect = recordAspect;
-    }
-
     /**
      * Init method
      */
     public void init()
     {
+        /** bind policies */
+        beforeFileRecord = policyComponent.registerClassPolicy(BeforeFileRecord.class);
+        onFileRecord = policyComponent.registerClassPolicy(OnFileRecord.class);        
+        
+        /** bind behaviours */
         policyComponent.bindAssociationBehaviour(
                 NodeServicePolicies.OnCreateChildAssociationPolicy.QNAME,
                 TYPE_RECORD_FOLDER,
                 ContentModel.ASSOC_CONTAINS,
                 onCreateChildAssociation);
-
         policyComponent.bindAssociationBehaviour(
                 NodeServicePolicies.BeforeDeleteChildAssociationPolicy.QNAME,
                 ContentModel.TYPE_FOLDER,
@@ -387,7 +342,7 @@ public class RecordServiceImpl implements RecordService,
     public void onRemoveAspect(NodeRef nodeRef, QName aspect)
     {
 
-        if (nodeService.hasAspect(nodeRef, ASPECT_RECORD) == true)
+        if (nodeService.hasAspect(nodeRef, ASPECT_RECORD) )
         {
             ContentData contentData = (ContentData) nodeService.getProperty(nodeRef, ContentModel.PROP_CONTENT);
 
@@ -401,7 +356,7 @@ public class RecordServiceImpl implements RecordService,
         {
             // check whether filling is pending aspect removal
             Set<NodeRef> pendingFilling = TransactionalResourceHelper.getSet("pendingFilling");
-            if (pendingFilling.contains(nodeRef) == true)
+            if (pendingFilling.contains(nodeRef) )
             {
                 file(nodeRef);
             }
@@ -432,7 +387,7 @@ public class RecordServiceImpl implements RecordService,
     {
         try
         {
-            if (nodeService.hasAspect(nodeRef, ASPECT_RECORD) == true)
+            if (nodeService.hasAspect(nodeRef, ASPECT_RECORD) )
             {
                 String origionalName =  (String)nodeService.getProperty(nodeRef, PROP_ORIGIONAL_NAME);
                 if (origionalName != null)
@@ -445,21 +400,21 @@ public class RecordServiceImpl implements RecordService,
         }
         catch (FileExistsException e)
         {
-            if (logger.isDebugEnabled() == true)
+            if (logger.isDebugEnabled() )
             {
                 logger.debug(e.getMessage());
             }
         }
         catch (InvalidNodeRefException e)
         {
-            if (logger.isDebugEnabled() == true)
+            if (logger.isDebugEnabled() )
             {
                 logger.debug(e.getMessage());
             }
         }
         catch (FileNotFoundException e)
         {
-            if (logger.isDebugEnabled() == true)
+            if (logger.isDebugEnabled() )
             {
                 logger.debug(e.getMessage());
             }
@@ -483,12 +438,12 @@ public class RecordServiceImpl implements RecordService,
                 try
                 {
                     NodeRef nodeRef = childAssocRef.getChildRef();
-                    if (nodeService.exists(nodeRef) == true  &&
+                    if (nodeService.exists(nodeRef)   &&
                         nodeService.hasAspect(nodeRef, ContentModel.ASPECT_TEMPORARY) == false &&
                         nodeService.getType(nodeRef).equals(TYPE_RECORD_FOLDER) == false &&
                         nodeService.getType(nodeRef).equals(TYPE_RECORD_CATEGORY) == false)
                     {
-                        if (nodeService.hasAspect(nodeRef, ContentModel.ASPECT_NO_CONTENT) == true)
+                        if (nodeService.hasAspect(nodeRef, ContentModel.ASPECT_NO_CONTENT) )
                         {
                             // we need to postpone filling until the NO_CONTENT aspect is removed
                             Set<NodeRef> pendingFilling = TransactionalResourceHelper.getSet("pendingFilling");
@@ -503,7 +458,11 @@ public class RecordServiceImpl implements RecordService,
                 }
                 catch (AlfrescoRuntimeException e)
                 {
-                    e.printStackTrace();
+                    // do nothing but log error
+                    if (logger.isDebugEnabled())
+                    {
+                        logger.debug("Unable to file pending record.", e);
+                    }
                 }
                 finally
                 {
@@ -542,7 +501,7 @@ public class RecordServiceImpl implements RecordService,
     @Override
     public void disablePropertyEditableCheck()
     {
-        recordAspect.getBehaviour("onUpdateProperties").disable();
+        getBehaviour("onUpdateProperties").disable();
     }
 
     /**
@@ -551,7 +510,68 @@ public class RecordServiceImpl implements RecordService,
     @Override
     public void enablePropertyEditableCheck()
     {
-        recordAspect.getBehaviour("onUpdateProperties").enable();
+        getBehaviour("onUpdateProperties").enable();
+    }
+    
+    /**
+     * Ensure that the user only updates record properties that they have permission to.
+     *
+     * @see org.alfresco.repo.node.NodeServicePolicies.OnUpdatePropertiesPolicy#onUpdateProperties(org.alfresco.service.cmr.repository.NodeRef, java.util.Map, java.util.Map)
+     */
+    @Override
+    @Behaviour
+    (
+            name = "onUpdateProperties",
+            kind = BehaviourKind.CLASS,
+            type= "rma:record"
+    )
+    public void onUpdateProperties(final NodeRef nodeRef, final Map<QName, Serializable> before, final Map<QName, Serializable> after)
+    {
+        if (AuthenticationUtil.getFullyAuthenticatedUser() != null &&
+            AuthenticationUtil.isRunAsUserTheSystemUser() == false &&
+            nodeService.exists(nodeRef) )
+        {
+            if (isRecord(nodeRef) )
+            {
+                for (QName property : after.keySet())
+                {
+                    Serializable beforeValue = null;
+                    if (before != null)
+                    {
+                        beforeValue = before.get(property);
+                    }
+
+                    Serializable afterValue = null;
+                    if (after != null)
+                    {
+                        afterValue = after.get(property);
+                    }
+
+                    boolean propertyUnchanged = false;
+                    if (beforeValue != null && afterValue != null &&
+                        beforeValue instanceof Date && afterValue instanceof Date)
+                    {
+                        // deal with date values
+                        propertyUnchanged = (((Date)beforeValue).compareTo((Date)afterValue) == 0);
+                    }
+                    else
+                    {
+                        // otherwise
+                        propertyUnchanged = EqualsHelper.nullSafeEquals(beforeValue, afterValue);
+                    }
+
+                    if (propertyUnchanged == false &&
+                        isPropertyEditable(nodeRef, property) == false)
+                    {
+                        // the user can't edit the record property
+                        throw new ModelAccessDeniedException(
+                            "The user " + AuthenticationUtil.getFullyAuthenticatedUser() +
+                            " does not have the permission to edit the record property " + property.toString() +
+                            " on the node " + nodeRef.toString());
+                    }
+                }
+            }
+        }
     }
 
     /**
@@ -562,15 +582,15 @@ public class RecordServiceImpl implements RecordService,
     {
         if (recordMetaDataAspects == null)
         {
-            recordMetaDataAspects = new HashSet<QName>(7);
             Collection<QName> aspects = dictionaryService.getAllAspects();
+            recordMetaDataAspects = new HashSet<QName>(aspects.size());
             for (QName aspect : aspects)
             {
                 AspectDefinition def = dictionaryService.getAspect(aspect);
                 if (def != null)
                 {
                     QName parent = def.getParentName();
-                    if (parent != null && ASPECT_RECORD_META_DATA.equals(parent) == true)
+                    if (parent != null && ASPECT_RECORD_META_DATA.equals(parent) )
                     {
                         recordMetaDataAspects.add(aspect);
                     }
@@ -580,17 +600,6 @@ public class RecordServiceImpl implements RecordService,
         return recordMetaDataAspects;
     }
 
-    /**
-     * @see org.alfresco.module.org_alfresco_module_rm.record.RecordService#isRecord(org.alfresco.service.cmr.repository.NodeRef)
-     */
-    @Override
-    public boolean isRecord(NodeRef nodeRef)
-    {
-        ParameterCheck.mandatory("nodeRef", nodeRef);
-
-        return nodeService.hasAspect(nodeRef, ASPECT_RECORD);
-    }
-
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.record.RecordService#isDeclared(org.alfresco.service.cmr.repository.NodeRef)
      */
@@ -678,7 +687,7 @@ public class RecordServiceImpl implements RecordService,
                         // make the document a record
                         makeRecord(nodeRef);
 
-                        if (isLinked == true)
+                        if (isLinked )
                         {
                             // turn off rules
                             ruleService.disableRules();
@@ -721,7 +730,7 @@ public class RecordServiceImpl implements RecordService,
         ParameterCheck.mandatory("name", name);
 
         NodeRef destination = nodeRef;
-        if (filePlanService.isFilePlan(nodeRef) == true)
+        if (filePlanService.isFilePlan(nodeRef) )
         {
             // get the unfiled record container for the file plan
             destination = filePlanService.getUnfiledContainer(nodeRef);
@@ -801,7 +810,7 @@ public class RecordServiceImpl implements RecordService,
             String recordName = prefix + " (" + recordId + ")" + postfix;
             fileFolderService.rename(document, recordName);
 
-            if (logger.isDebugEnabled() == true)
+            if (logger.isDebugEnabled() )
             {
                 logger.debug("Rename " + name + " to " + recordName);
             }
@@ -833,13 +842,13 @@ public class RecordServiceImpl implements RecordService,
 
         boolean result = false;
 
-        if (isRecord(nodeRef) == true)
+        if (isRecord(nodeRef) )
         {
             ChildAssociationRef childAssocRef = nodeService.getPrimaryParent(nodeRef);
             if (childAssocRef != null)
             {
                 NodeRef parent = childAssocRef.getParentRef();
-                if (parent != null && recordFolderService.isRecordFolder(parent) == true)
+                if (parent != null && recordFolderService.isRecordFolder(parent) )
                 {
                     result = true;
                 }
@@ -865,9 +874,12 @@ public class RecordServiceImpl implements RecordService,
         // we only support filling of content items
         // TODO composite record support needs to file containers too
         QName type = nodeService.getType(record);
-        if (ContentModel.TYPE_CONTENT.equals(type) == true ||
+        if (ContentModel.TYPE_CONTENT.equals(type)  ||
-            dictionaryService.isSubClass(type, ContentModel.TYPE_CONTENT) == true)
+            dictionaryService.isSubClass(type, ContentModel.TYPE_CONTENT) )
         {
+            // fire before file record policy
+            beforeFileRecord.get(getTypeAndApsects(record)).beforeFileRecord(record);
+            
             // check whether this item is already an item or not
             if (isRecord(record) == false)
             {
@@ -882,19 +894,8 @@ public class RecordServiceImpl implements RecordService,
                 nodeService.setProperty(record, PROP_DATE_FILED, fileCalendar.getTime());
             }
             
-            // initialise vital record details
+            // file on file record policy
-            // TODO .. change this to add the aspect which will trigger the init behaviour
+            onFileRecord.get(getTypeAndApsects(record)).onFileRecord(record);
-            vitalRecordService.initialiseVitalRecord(record);
-
-            // initialise disposition details
-            if (nodeService.hasAspect(record, ASPECT_DISPOSITION_LIFECYCLE) == false)
-            {
-                DispositionSchedule di = dispositionService.getDispositionSchedule(record);
-                if (di != null && di.isRecordLevelDisposition() == true)
-                {
-                    nodeService.addAspect(record, ASPECT_DISPOSITION_LIFECYCLE, null);
-                }
-            }
         }
     }
     
@@ -976,7 +977,7 @@ public class RecordServiceImpl implements RecordService,
                     Set<QName> aspects = nodeService.getAspects(nodeRef);
                     for (QName aspect : aspects)
                     {
-                        if (RM_URI.equals(aspect.getNamespaceURI()) == true)
+                        if (RM_URI.equals(aspect.getNamespaceURI()) )
                         {
                             // remove the aspect
                             nodeService.removeAspect(nodeRef, aspect);
@@ -994,7 +995,7 @@ public class RecordServiceImpl implements RecordService,
                     {
                         fileFolderService.rename(nodeRef, origionalName);
 
-                        if (logger.isDebugEnabled() == true)
+                        if (logger.isDebugEnabled() )
                         {
                             String name = (String)nodeService.getProperty(nodeRef, ContentModel.PROP_NAME);
                             logger.debug("Rename " + name + " to " + origionalName);
@@ -1042,7 +1043,7 @@ public class RecordServiceImpl implements RecordService,
             throw new AlfrescoRuntimeException("Can not check if the property " + property.toString() + " is editable, because node reference is not a record.");
         }
 
-        if (logger.isDebugEnabled() == true)
+        if (logger.isDebugEnabled() )
         {
             logger.debug("Checking whether property " + property.toString() + " is editable for user " + AuthenticationUtil.getRunAsUser());
         }
@@ -1051,35 +1052,35 @@ public class RecordServiceImpl implements RecordService,
         NodeRef filePlan = filePlanService.getFilePlan(record);
         Set<Role> roles = filePlanRoleService.getRolesByUser(filePlan, AuthenticationUtil.getRunAsUser());
 
-        if (logger.isDebugEnabled() == true)
+        if (logger.isDebugEnabled() )
         {
             logger.debug(" ... users roles");
         }
 
         for (Role role : roles)
         {
-            if (logger.isDebugEnabled() == true)
+            if (logger.isDebugEnabled() )
             {
                 logger.debug("     ... user has role " + role.getName() + " with capabilities ");
             }
 
             for (Capability cap : role.getCapabilities())
             {
-                if (logger.isDebugEnabled() == true)
+                if (logger.isDebugEnabled() )
                 {
                     logger.debug("         ... " + cap.getName());
                 }
             }
         }
 
-        if (logger.isDebugEnabled() == true)
+        if (logger.isDebugEnabled() )
         {
             logger.debug(" ... user has the following set permissions on the file plan");
         }
         Set<AccessPermission> perms = permissionService.getAllSetPermissions(filePlan);
         for (AccessPermission perm : perms)
         {
-            if (logger.isDebugEnabled() == true &&
+            if (logger.isDebugEnabled()  &&
                 (perm.getPermission().contains(RMPermissionModel.EDIT_NON_RECORD_METADATA) ||
                  perm.getPermission().contains(RMPermissionModel.EDIT_RECORD_METADATA)))
             {
@@ -1089,7 +1090,7 @@ public class RecordServiceImpl implements RecordService,
 
         if (permissionService.hasPermission(filePlan, RMPermissionModel.EDIT_NON_RECORD_METADATA).equals(AccessStatus.ALLOWED))
         {
-            if (logger.isDebugEnabled() == true)
+            if (logger.isDebugEnabled() )
             {
                 logger.debug(" ... user has the edit non record metadata permission on the file plan");
             }
@@ -1098,9 +1099,9 @@ public class RecordServiceImpl implements RecordService,
         // END DEBUG ...
 
         boolean result = alwaysEditProperty(property);
-        if (result == true)
+        if (result )
         {
-            if (logger.isDebugEnabled() == true)
+            if (logger.isDebugEnabled() )
             {
                 logger.debug(" ... property marked as always editable.");
             }
@@ -1114,9 +1115,9 @@ public class RecordServiceImpl implements RecordService,
             AccessStatus accessDeclaredRecord = capabilityService.getCapabilityAccessState(record, RMPermissionModel.EDIT_DECLARED_RECORD_METADATA);
             AccessStatus accessRecord = capabilityService.getCapabilityAccessState(record, RMPermissionModel.EDIT_RECORD_METADATA);
 
-            if (AccessStatus.ALLOWED.equals(accessNonRecord) == true)
+            if (AccessStatus.ALLOWED.equals(accessNonRecord) )
             {
-                if (logger.isDebugEnabled() == true)
+                if (logger.isDebugEnabled() )
                 {
                     logger.debug(" ... user has edit nonrecord metadata capability");
                 }
@@ -1124,10 +1125,10 @@ public class RecordServiceImpl implements RecordService,
                 allowNonRecordEdit = true;
             }
 
-            if (AccessStatus.ALLOWED.equals(accessRecord) == true ||
+            if (AccessStatus.ALLOWED.equals(accessRecord)  ||
-                AccessStatus.ALLOWED.equals(accessDeclaredRecord) == true)
+                AccessStatus.ALLOWED.equals(accessDeclaredRecord) )
             {
-                if (logger.isDebugEnabled() == true)
+                if (logger.isDebugEnabled() )
                 {
                     logger.debug(" ... user has edit record or declared metadata capability");
                 }
@@ -1135,21 +1136,21 @@ public class RecordServiceImpl implements RecordService,
                 allowRecordEdit = true;
             }
 
-            if (allowNonRecordEdit == true && allowRecordEdit == true)
+            if (allowNonRecordEdit && allowRecordEdit)
             {
-                if (logger.isDebugEnabled() == true)
+                if (logger.isDebugEnabled() )
                 {
                     logger.debug(" ... so all properties can be edited.");
                 }
 
                 result = true;
             }
-            else if (allowNonRecordEdit == true && allowRecordEdit == false)
+            else if (allowNonRecordEdit && allowRecordEdit == false)
             {
                 // can only edit non record properties
                 if (isRecordMetadata(property) == false)
                 {
-                    if (logger.isDebugEnabled() == true)
+                    if (logger.isDebugEnabled() )
                     {
                         logger.debug(" ... property is not considered record metadata so editable.");
                     }
@@ -1158,18 +1159,18 @@ public class RecordServiceImpl implements RecordService,
                 }
                 else
                 {
-                    if (logger.isDebugEnabled() == true)
+                    if (logger.isDebugEnabled() )
                     {
                         logger.debug(" ... property is considered record metadata so not editable.");
                     }
                 }
             }
-            else if (allowNonRecordEdit == false && allowRecordEdit == true)
+            else if (allowNonRecordEdit == false && allowRecordEdit )
             {
                 // can only edit record properties
-                if (isRecordMetadata(property) == true)
+                if (isRecordMetadata(property) )
                 {
-                    if (logger.isDebugEnabled() == true)
+                    if (logger.isDebugEnabled() )
                     {
                         logger.debug(" ... property is considered record metadata so editable.");
                     }
@@ -1178,7 +1179,7 @@ public class RecordServiceImpl implements RecordService,
                 }
                 else
                 {
-                    if (logger.isDebugEnabled() == true)
+                    if (logger.isDebugEnabled() )
                     {
                         logger.debug(" ... property is not considered record metadata so not editable.");
                     }
@@ -1205,7 +1206,7 @@ public class RecordServiceImpl implements RecordService,
             if (def != null)
             {
                 ClassDefinition parent = def.getContainerClass();
-                if (parent != null && parent.isAspect() == true)
+                if (parent != null && parent.isAspect() )
                 {
                     result = getRecordMetaDataAspects().contains(parent.getName());
                 }
@@ -1265,13 +1266,13 @@ public class RecordServiceImpl implements RecordService,
         ParameterCheck.mandatory("recordFolder", recordFolder);
 
         List<NodeRef> result = new ArrayList<NodeRef>(1);
-        if (recordFolderService.isRecordFolder(recordFolder) == true)
+        if (recordFolderService.isRecordFolder(recordFolder))
         {
             List<ChildAssociationRef> assocs = nodeService.getChildAssocs(recordFolder, ContentModel.ASSOC_CONTAINS, RegexQNamePattern.MATCH_ALL);
             for (ChildAssociationRef assoc : assocs)
             {
                 NodeRef child = assoc.getChildRef();
-                if (isRecord(child) == true)
+                if (isRecord(child) )
                 {
                     result.add(child);
                 }

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImpl.java

@@ -27,7 +27,6 @@ import java.util.Set;
 import org.alfresco.model.RenditionModel;
 import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService;
 import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel;
-import org.alfresco.module.org_alfresco_module_rm.record.RecordService;
 import org.alfresco.module.org_alfresco_module_rm.role.FilePlanRoleService;
 import org.alfresco.module.org_alfresco_module_rm.util.ServiceBaseImpl;
 import org.alfresco.service.cmr.repository.ChildAssociationRef;
@@ -51,23 +50,12 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     private final static QName PROP_EXTENDED_READER_ROLE = QName.createQName(RM_URI, "extendedReaderRole");
     private final static QName PROP_EXTENDED_WRITER_ROLE = QName.createQName(RM_URI, "extendedWriterRole");
 
-    /** Record service */
-    private RecordService recordService;
-
     /** File plan service */
     private FilePlanService filePlanService;
 
     /** File plan role service */
     private FilePlanRoleService filePlanRoleService;
 
-    /**
-     * @param recordService record service
-     */
-    public void setRecordService(RecordService recordService)
-    {
-        this.recordService = recordService;
-    }
-
     /**
      * @param filePlanService   file plan service
      */
@@ -193,7 +181,7 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
         }
 
         // apply the readers to any renditions of the content
-        if (recordService.isRecord(nodeRef) == true)
+        if (isRecord(nodeRef) == true)
         {
             List<ChildAssociationRef> assocs = nodeService.getChildAssocs(nodeRef, RenditionModel.ASSOC_RENDITION, RegexQNamePattern.MATCH_ALL);
             for (ChildAssociationRef assoc : assocs)
@@ -322,7 +310,7 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
             removeExtendedSecurityImpl(nodeRef, readers, writers);
 
             // remove the readers from any renditions of the content
-            if (recordService.isRecord(nodeRef) == true)
+            if (isRecord(nodeRef) == true)
             {
                 List<ChildAssociationRef> assocs = nodeService.getChildAssocs(nodeRef, RenditionModel.ASSOC_RENDITION, RegexQNamePattern.MATCH_ALL);
                 for (ChildAssociationRef assoc : assocs)

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/util/ServiceBaseImpl.java

@@ -18,6 +18,8 @@
  */
 package org.alfresco.module.org_alfresco_module_rm.util;
 
+import java.util.Set;
+
 import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel;
 import org.alfresco.service.cmr.dictionary.DictionaryService;
 import org.alfresco.service.cmr.repository.NodeRef;
@@ -56,6 +58,19 @@ public class ServiceBaseImpl implements RecordsManagementModel
         this.dictionaryService = dictionaryService;
     }
     
+    /**
+     * Indicates whether the given node reference is a record or not.
+     * 
+     * @param nodeRef   node reference
+     * @return boolean  true if node reference is a record, false otherwise
+     */
+    public boolean isRecord(NodeRef nodeRef)
+    {
+        ParameterCheck.mandatory("nodeRef", nodeRef);
+
+        return nodeService.hasAspect(nodeRef, ASPECT_RECORD);
+    }
+    
     /**
      * Utility method to safely and quickly determine if a node is a type (or sub-type) of the one specified.
      * 
@@ -111,4 +126,16 @@ public class ServiceBaseImpl implements RecordsManagementModel
         return counter;
     }
     
+    /**
+     * Helper method to get a set containing the node's type and all it's aspects
+     * 
+     * @param nodeRef       nodeRef
+     * @return Set<QName>   set of qname's
+     */
+    protected Set<QName> getTypeAndApsects(NodeRef nodeRef)
+    {
+        Set<QName> result = nodeService.getAspects(nodeRef);        
+        result.add(nodeService.getType(nodeRef));
+        return result;
+    }
 }

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/vital/VitalRecordService.java

@@ -70,5 +70,5 @@ public interface VitalRecordService
      *
      * @param nodeRef   node reference to initialise with vital record details
      */
-    void initialiseVitalRecord(NodeRef nodeRef);
+    //void initialiseVitalRecord(NodeRef nodeRef);
 }

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/vital/VitalRecordServiceImpl.java

@@ -23,11 +23,15 @@ import java.util.Date;
 import java.util.HashMap;
 import java.util.Map;
 
+import org.alfresco.module.org_alfresco_module_rm.RecordsManagementPolicies;
 import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanComponentKind;
 import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService;
 import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel;
+import org.alfresco.module.org_alfresco_module_rm.util.ServiceBaseImpl;
+import org.alfresco.repo.policy.annotation.Behaviour;
+import org.alfresco.repo.policy.annotation.BehaviourBean;
+import org.alfresco.repo.policy.annotation.BehaviourKind;
 import org.alfresco.service.cmr.repository.NodeRef;
-import org.alfresco.service.cmr.repository.NodeService;
 import org.alfresco.service.cmr.repository.Period;
 import org.alfresco.service.namespace.QName;
 import org.alfresco.util.ParameterCheck;
@@ -38,21 +42,15 @@ import org.alfresco.util.ParameterCheck;
  * @author Roy Wetherall
  * @since 2.0
  */
-public class VitalRecordServiceImpl implements VitalRecordService,
+@BehaviourBean
-                                               RecordsManagementModel
+public class VitalRecordServiceImpl extends ServiceBaseImpl
+                                    implements VitalRecordService,
+                                               RecordsManagementModel,
+                                               RecordsManagementPolicies.OnFileRecord
 {
-    /** Services */
+    /** file plan service */
-    private NodeService nodeService;
     private FilePlanService filePlanService;
 
-    /**
-     * @param nodeService   node service
-     */
-    public void setNodeService(NodeService nodeService)
-    {
-        this.nodeService = nodeService;
-    }
-
     /**
      * @param filePlanService	file plan service
      */
@@ -61,6 +59,48 @@ public class VitalRecordServiceImpl implements VitalRecordService,
 		this.filePlanService = filePlanService;
 	}
 
+    /**
+     * Behavior to initialize vital record.
+     * 
+     * @see org.alfresco.module.org_alfresco_module_rm.RecordsManagementPolicies.OnFileRecord#onFileRecord(org.alfresco.service.cmr.repository.NodeRef)
+     * @since 2.2
+     */
+    @Override
+    @Behaviour(kind=BehaviourKind.CLASS, type="rma:record")
+    public void onFileRecord(NodeRef nodeRef)
+    {
+       // Calculate the review schedule
+       VitalRecordDefinition viDef = getVitalRecordDefinition(nodeRef);
+       if (viDef != null && viDef.isEnabled() == true)
+       {
+          Date reviewAsOf = viDef.getNextReviewDate();
+          if (reviewAsOf != null)
+          {
+              Map<QName, Serializable> reviewProps = new HashMap<QName, Serializable>(1);
+              reviewProps.put(RecordsManagementModel.PROP_REVIEW_AS_OF, reviewAsOf);
+    
+              if (nodeService.hasAspect(nodeRef, ASPECT_VITAL_RECORD) == false)
+              {
+                  nodeService.addAspect(nodeRef, RecordsManagementModel.ASPECT_VITAL_RECORD, reviewProps);
+              }
+              else
+              {
+                  Map<QName, Serializable> props = nodeService.getProperties(nodeRef);
+                  props.putAll(reviewProps);
+                  nodeService.setProperties(nodeRef, props);
+              }
+          }
+       }
+       else
+       {
+          // if we are re-filling then remove the vital aspect if it is not longer a vital record
+          if (nodeService.hasAspect(nodeRef, ASPECT_VITAL_RECORD) == true)
+          {
+              nodeService.removeAspect(nodeRef, ASPECT_VITAL_RECORD);
+          }
+       }
+    }
+
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.vital.VitalRecordService#setupVitalRecordDefinition(org.alfresco.service.cmr.repository.NodeRef)
      */
@@ -96,43 +136,6 @@ public class VitalRecordServiceImpl implements VitalRecordService,
         }
     }
 
-    /**
-     * @see VitalRecordService#initialiseVitalRecord(NodeRef)
-     */
-    public void initialiseVitalRecord(NodeRef nodeRef)
-    {
-        // Calculate the review schedule
-        VitalRecordDefinition viDef = getVitalRecordDefinition(nodeRef);
-        if (viDef != null && viDef.isEnabled() == true)
-        {
-            Date reviewAsOf = viDef.getNextReviewDate();
-            if (reviewAsOf != null)
-            {
-                Map<QName, Serializable> reviewProps = new HashMap<QName, Serializable>(1);
-                reviewProps.put(RecordsManagementModel.PROP_REVIEW_AS_OF, reviewAsOf);
-
-                if (nodeService.hasAspect(nodeRef, ASPECT_VITAL_RECORD) == false)
-                {
-                    nodeService.addAspect(nodeRef, RecordsManagementModel.ASPECT_VITAL_RECORD, reviewProps);
-                }
-                else
-                {
-                    Map<QName, Serializable> props = nodeService.getProperties(nodeRef);
-                    props.putAll(reviewProps);
-                    nodeService.setProperties(nodeRef, props);
-                }
-            }
-        }
-        else
-        {
-            // if we are re-filling then remove the vital aspect if it is not longer a vital record
-            if (nodeService.hasAspect(nodeRef, ASPECT_VITAL_RECORD) == true)
-            {
-                nodeService.removeAspect(nodeRef, ASPECT_VITAL_RECORD);
-            }
-        }
-    }
-
     /**
      * @see VitalRecordService#getVitalRecordDefinition(NodeRef)
      */

rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/ServicesTestSuite.java

@@ -35,6 +35,7 @@ import org.alfresco.module.org_alfresco_module_rm.test.service.RecordsManagement
 import org.alfresco.module.org_alfresco_module_rm.test.service.RecordsManagementQueryDAOImplTest;
 import org.alfresco.module.org_alfresco_module_rm.test.service.RecordsManagementSearchServiceImplTest;
 import org.alfresco.module.org_alfresco_module_rm.test.service.ReportServiceImplTest;
+import org.alfresco.module.org_alfresco_module_rm.test.service.ServiceBaseImplTest;
 import org.alfresco.module.org_alfresco_module_rm.test.service.VitalRecordServiceImplTest;
 import org.junit.runner.RunWith;
 import org.junit.runners.Suite;
@@ -49,6 +50,7 @@ import org.junit.runners.Suite.SuiteClasses;
 @RunWith(Suite.class)
 @SuiteClasses(
 {
+    ServiceBaseImplTest.class,
     ExtendedSecurityServiceImplTest.class,
     ModelSecurityServiceImplTest.class,
     RecordsManagementActionServiceImplTest.class,

rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/service/ReportServiceImplTest.java

@@ -176,8 +176,10 @@ public class ReportServiceImplTest extends BaseRMTestCase implements ReportModel
                 Map<String, Serializable> params = new HashMap<String, Serializable>(1);
                 params.put(CompleteEventAction.PARAM_EVENT_NAME, CommonRMTestUtils.DEFAULT_EVENT_NAME);
                 rmActionService.executeRecordsManagementAction(rmFolder, CompleteEventAction.NAME, params);
+                
                 rmActionService.executeRecordsManagementAction(rmFolder, CutOffAction.NAME);
                 rmActionService.executeRecordsManagementAction(rmFolder, DestroyAction.NAME);
+                
                 Map<String, Serializable> fileReportParams = new HashMap<String, Serializable>(2);
                 fileReportParams.put(DestructionReportAction.REPORT_TYPE, "rmr:destructionReport");
                 fileReportParams.put(DestructionReportAction.DESTINATION, filePlan.toString());

rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/service/ServiceBaseImplTest.java

@@ -0,0 +1,109 @@
+/*
+ * Copyright (C) 2005-2014 Alfresco Software Limited.
+ *
+ * This file is part of Alfresco
+ *
+ * Alfresco is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Alfresco is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.alfresco.module.org_alfresco_module_rm.test.service;
+
+import java.util.Set;
+
+import org.alfresco.model.ContentModel;
+import org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase;
+import org.alfresco.module.org_alfresco_module_rm.test.util.TestServiceImpl;
+import org.alfresco.service.namespace.QName;
+
+/**
+ * Unit test for service base implementation.
+ *
+ * @author Roy Wetherall
+ * @since 2.2
+ */
+public class ServiceBaseImplTest extends BaseRMTestCase
+{
+    /** test service */
+    private TestServiceImpl testService;
+    
+    /**
+     * Init services
+     */
+    @Override
+    protected void initServices()
+    {
+        super.initServices();
+        
+        testService = (TestServiceImpl)applicationContext.getBean("testService");
+    }
+
+    /**
+     * test instanceOf()
+     */
+    public void testInstanceOf()
+    {
+        doTestInTransaction(new Test<Void>()
+        {
+            @Override
+            public Void run() throws Exception
+            {
+                assertTrue(testService.doInstanceOf(rmFolder, ContentModel.TYPE_FOLDER));
+                assertTrue(testService.doInstanceOf(rmFolder, TYPE_RECORD_FOLDER));
+                assertFalse(testService.doInstanceOf(rmFolder, TYPE_RECORD_CATEGORY));
+                
+                return null;
+            }
+        });
+        
+    }
+    
+    /**
+     * test getNextCounter()
+     */
+    public void testGetNextCounter()
+    {
+        doTestInTransaction(new Test<Void>()
+        {
+            @Override
+            public Void run() throws Exception
+            {
+                assertNull(nodeService.getProperty(rmFolder, PROP_COUNT));
+                assertEquals(1, testService.doGetNextCount(rmFolder));
+                assertEquals(2, testService.doGetNextCount(rmFolder));
+                assertEquals(3, testService.doGetNextCount(rmFolder));
+                
+                return null;
+            }
+        });
+        
+    }
+    
+    /**
+     * test getTypeAndAspects()
+     */
+    public void testGetTypeAndAspects()
+    {
+        doTestInTransaction(new Test<Void>()
+        {
+            @Override
+            public Void run() throws Exception
+            {
+                Set<QName> result = testService.doGetTypeAndApsects(rmFolder);
+                assertTrue(result.contains(TYPE_RECORD_FOLDER));
+        
+                return null;
+            }
+        });        
+    }
+
+}

rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/RetryingTransactionHelperBaseTest.java

@@ -0,0 +1,67 @@
+/*
+ * Copyright (C) 2005-2012 Alfresco Software Limited.
+ *
+ * This file is part of Alfresco
+ *
+ * Alfresco is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Alfresco is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.alfresco.module.org_alfresco_module_rm.test.util;
+
+import org.alfresco.repo.transaction.RetryingTransactionHelper;
+import org.alfresco.util.ApplicationContextHelper;
+import org.alfresco.util.RetryingTransactionHelperTestCase;
+import org.springframework.context.ApplicationContext;
+
+/**
+ * Base unit test for a simple retrying transaction helper test case.
+ *
+ * @author Roy Wetherall
+ * @since 2.2
+ */
+public class RetryingTransactionHelperBaseTest extends RetryingTransactionHelperTestCase
+{
+    /** Application context */
+    protected static final String[] CONFIG_LOCATIONS = new String[]
+    {
+        "classpath:alfresco/application-context.xml",
+        "classpath:test-context.xml"
+    };
+    protected ApplicationContext applicationContext;
+    
+    /** retrying transaction helper */
+    protected RetryingTransactionHelper retryingTransactionHelper;
+
+    /**
+     * @see org.alfresco.util.RetryingTransactionHelperTestCase#getRetryingTransactionHelper()
+     */
+    @Override
+    public RetryingTransactionHelper getRetryingTransactionHelper()
+    {
+        return retryingTransactionHelper;
+    }
+    
+    /**
+     * @see junit.framework.TestCase#setUp()
+     */
+    @Override
+    protected void setUp() throws Exception
+    {
+        // Get the application context
+        applicationContext = ApplicationContextHelper.getApplicationContext(CONFIG_LOCATIONS);
+        
+        // get the retrying transaction helper
+        retryingTransactionHelper = (RetryingTransactionHelper)applicationContext.getBean("retryingTransactionHelper");
+    }
+
+}

rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/TestServiceImpl.java

@@ -18,13 +18,17 @@
  */
 package org.alfresco.module.org_alfresco_module_rm.test.util;
 
+import java.util.Set;
+
+import org.alfresco.module.org_alfresco_module_rm.util.ServiceBaseImpl;
 import org.alfresco.service.cmr.repository.NodeRef;
+import org.alfresco.service.namespace.QName;
 
 /**
  * @author Roy Wetherall
  * @since 2.1
  */
-public class TestServiceImpl implements TestService
+public class TestServiceImpl extends ServiceBaseImpl implements TestService
 {
     @Override
     public void testMethodOne(NodeRef nodeRef)
@@ -35,4 +39,19 @@ public class TestServiceImpl implements TestService
     public void testMethodTwo(NodeRef nodeRef)
     {
     }
+    
+    public boolean doInstanceOf(NodeRef nodeRef, QName ofClassName)
+    {
+        return instanceOf(nodeRef, ofClassName);
+    }
+    
+    public int doGetNextCount(NodeRef nodeRef)
+    {
+        return getNextCount(nodeRef);
+    }
+
+    public Set<QName> doGetTypeAndApsects(NodeRef nodeRef)
+    {
+        return getTypeAndApsects(nodeRef);
+    }
 }

rm-server/test/resources/test-context.xml

@@ -192,7 +192,7 @@
    
    <!--  Test service -->
    
-   <bean id="testService" class="org.alfresco.module.org_alfresco_module_rm.test.util.TestServiceImpl"/>
+   <bean id="testService" class="org.alfresco.module.org_alfresco_module_rm.test.util.TestServiceImpl" parent="baseService" />
    
    <bean id="TestService" class="org.springframework.aop.framework.ProxyFactoryBean">
         <property name="proxyInterfaces">