ALF-8498. Reimplemented the Comment Count Rollups to take pre-Swift, commented nodes into account. Also added a trigger for a recalculation.

Major refactoring of existing onCreateNode/beforeDeleteNode(fm:post) behaviours. They now distinguish between
    1. increment/decrement of previously rolled-up commentCounts
    2. full recalculation of comment count for nodes that have no previous rollup (which would include nodes from pre-Swift repos).
  Added a new registered behaviour: onUpdateProperties(fm:commentsRollup) in order to detect fm:commentCount being set to a "trigger value".
    If this property is set to a negative number, then a full recalculation of the commentCount for that node will be performed.
  New test cases for preSwift content & the recount trigger.
  Added a skeleton (placeholder) CommentService to hold some comment-related methods I needed.


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

config/alfresco/application-context-highlevel.xml

@@ -9,6 +9,7 @@
     <import resource="classpath*:alfresco/office-addin-context.xml"/>
     <import resource="classpath*:alfresco/portlets-context.xml"/>
     <import resource="classpath:alfresco/blog-context.xml"/>
+    <import resource="classpath:alfresco/comment-services-context.xml"/>
     <import resource="classpath:alfresco/rating-services-context.xml"/>
     <import resource="classpath:alfresco/rendition-services-context.xml"/>
     <import resource="classpath:alfresco/replication-services-context.xml"/>

config/alfresco/comment-services-context.xml

@@ -0,0 +1,42 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<!DOCTYPE beans PUBLIC '-//SPRING//DTD BEAN//EN' 'http://www.springframework.org/dtd/spring-beans.dtd'>
+
+<beans>
+    <!-- Comment Service (Management of Share comments) -->
+    <bean id="CommentService" class="org.springframework.aop.framework.ProxyFactoryBean">
+       <property name="proxyInterfaces">
+          <value>org.alfresco.repo.forum.CommentService</value>
+       </property>
+       <property name="target">
+          <ref bean="commentService" />
+       </property>
+       <property name="interceptorNames">
+          <list>
+             <idref local="CommentService_transaction" />
+             <idref bean="AuditMethodInterceptor" />
+             <idref bean="exceptionTranslator" />
+             <idref local="CommentService_security" />
+          </list>
+       </property>
+    </bean>
+   
+    <!-- Comment service transaction bean -->
+    <bean id="CommentService_transaction"
+          class="org.springframework.transaction.interceptor.TransactionInterceptor">
+       <property name="transactionManager">
+          <ref bean="transactionManager" />
+       </property>
+       <property name="transactionAttributes">
+          <props>
+             <prop key="*">${server.transaction.mode.default}</prop>
+          </props>
+       </property>
+    </bean>
+   
+    <bean id="CommentService_security" class="org.alfresco.repo.security.permissions.impl.AlwaysProceedMethodInterceptor"/>
+    
+    <!-- Comment Service base bean -->
+    <bean id="commentService" class="org.alfresco.repo.forum.CommentServiceImpl">
+       <property name="nodeService" ref="NodeService"/>
+    </bean>
+</beans>

config/alfresco/core-services-context.xml

@@ -1319,9 +1319,9 @@
         <property name="policyComponent">
             <ref bean="policyComponent" />
         </property>
-        <property name="nodeService">
+        <property name="commentService" ref="CommentService"/>
-            <ref bean="NodeService" />
+        <property name="rawNodeService" ref="nodeService"/> <!-- Intentional small 'n' -->
-        </property>
+        <property name="nodeService" ref="NodeService"/>    <!-- Intentional large 'N' -->
     </bean>
     <bean id="commentsRollupAspect" class="org.alfresco.repo.forum.CommentsRollupAspect" init-method="init">
        <property name="policyComponent" ref="policyComponent"/>

config/alfresco/model/forumModel.xml

@@ -59,8 +59,6 @@
             <property name="fm:commentCount">
                <title>Comment count rollup for this node</title>
                <type>d:int</type>
-                <mandatory>true</mandatory>
-                <default>0</default>
                <index enabled="true">
                   <atomic>true</atomic>
                   <stored>true</stored>

source/java/org/alfresco/repo/forum/CommentService.java

@@ -0,0 +1,56 @@
+/*
+ * Copyright (C) 2005-2011 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.repo.forum;
+
+import org.alfresco.error.AlfrescoRuntimeException;
+import org.alfresco.model.ForumModel;
+import org.alfresco.service.cmr.repository.NodeRef;
+import org.alfresco.service.namespace.QName;
+
+/**
+ * This is a starting point for a future service for handling Share comments.
+ * <p/>
+ * This class may change in the future as requirements become clearer.
+ * 
+ * @author Neil Mc Erlean
+ * @since 4.0
+ */
+public interface CommentService
+{
+    /**
+     * Thi method retrieves the ancestor in the repository containment hierarchy having the
+     * {@link ForumModel#ASPECT_DISCUSSABLE fm:discussable} aspect.
+     * 
+     * @param descendantNodeRef The nodeRef which descends from the f:discussable node.
+     * @param expectedNodeType if not <tt>null</tt>, this is an assertion by calling code that the descendantNodeRef
+     *                           is of the specified type.
+     * @return the fm:discussable ancestor if there is one, else <tt>null</tt>
+     * @throws AlfrescoRuntimeException if the specified expectedNodeType is not correct.
+     */
+    NodeRef getDiscussableAncestor(NodeRef descendantNodeRef, QName expectedNodeType);
+    
+    /**
+     * This method retrieves the {@link ForumModel#TYPE_TOPIC fm:topic} NodeRef which holds the Share comments for
+     * the specified {@link ForumModel#ASPECT_DISCUSSABLE fm:discussable} node.
+     * 
+     * @param discussableNode the node whose Share comments are sought.
+     * @return the fm:topic NodeRef, if one exists, else <tt>null</tt>.
+     */
+    NodeRef getShareCommentsTopic(NodeRef discussableNode);
+}

source/java/org/alfresco/repo/forum/CommentServiceImpl.java

@@ -0,0 +1,107 @@
+/*
+ * Copyright (C) 2005-2011 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.repo.forum;
+
+import java.util.List;
+
+import org.alfresco.error.AlfrescoRuntimeException;
+import org.alfresco.model.ContentModel;
+import org.alfresco.model.ForumModel;
+import org.alfresco.service.cmr.repository.ChildAssociationRef;
+import org.alfresco.service.cmr.repository.NodeRef;
+import org.alfresco.service.cmr.repository.NodeService;
+import org.alfresco.service.namespace.NamespaceService;
+import org.alfresco.service.namespace.QName;
+
+/**
+ * @author Neil Mc Erlean
+ * @since 4.0
+ */
+public class CommentServiceImpl implements CommentService
+{
+    /**
+     * Naming convention for Share comment model. fm:forum contains fm:topic
+     */
+    private static final QName FORUM_TO_TOPIC_ASSOC_QNAME = QName.createQName(NamespaceService.CONTENT_MODEL_1_0_URI, "Comments");
+    
+    // Injected services
+    private NodeService nodeService;
+    
+    public void setNodeService(NodeService nodeService)
+    {
+        this.nodeService = nodeService;
+    }
+    
+    @Override
+    public NodeRef getDiscussableAncestor(NodeRef descendantNodeRef, QName expectedNodeType)
+    {
+        final QName actualNodeType = nodeService.getType(descendantNodeRef);
+        if (expectedNodeType != null && !actualNodeType.equals(expectedNodeType))
+        {
+            StringBuilder msg = new StringBuilder();
+            msg.append("Node ").append(descendantNodeRef)
+               .append(" is of type ").append(actualNodeType)
+               .append(", not ").append(expectedNodeType);
+            throw new AlfrescoRuntimeException(msg.toString());
+        }
+        
+        NodeRef result = null;
+        for (ChildAssociationRef parentAssoc = nodeService.getPrimaryParent(descendantNodeRef);
+             parentAssoc != null;
+             parentAssoc = nodeService.getPrimaryParent(parentAssoc.getParentRef()))
+        {
+            if (nodeService.hasAspect(parentAssoc.getParentRef(), ForumModel.ASPECT_DISCUSSABLE))
+            {
+                result = parentAssoc.getParentRef();
+                break;
+            }
+        }
+        
+        return result;
+    }
+    
+    @Override
+    public NodeRef getShareCommentsTopic(NodeRef discussableNode)
+    {
+        NodeRef result = null;
+        
+        if (nodeService.hasAspect(discussableNode, ForumModel.ASPECT_DISCUSSABLE))
+        {
+            // We navigate down the "Share comments" containment model, which is based on the more general forum model,
+            // but with certain naming conventions.
+            List<ChildAssociationRef> fora = nodeService.getChildAssocs(discussableNode, ForumModel.ASSOC_DISCUSSION, ForumModel.ASSOC_DISCUSSION, true);
+            
+            // There should only be one such assoc.
+            if ( !fora.isEmpty())
+            {
+                final NodeRef firstForumNode = fora.get(0).getChildRef();
+                List<ChildAssociationRef> topics = nodeService.getChildAssocs(firstForumNode, ContentModel.ASSOC_CONTAINS, FORUM_TO_TOPIC_ASSOC_QNAME, true);
+                
+                // Likewise, only one.
+                if ( !topics.isEmpty())
+                {
+                    final NodeRef firstTopicNode = topics.get(0).getChildRef();
+                    result = firstTopicNode;
+                }
+            }
+        }
+        
+        return result;
+    }
+}

source/java/org/alfresco/repo/forum/CommentsTest.java

@@ -20,6 +20,7 @@
 package org.alfresco.repo.forum;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 import java.io.Serializable;
@@ -32,6 +33,7 @@ import org.alfresco.model.ContentModel;
 import org.alfresco.model.ForumModel;
 import org.alfresco.repo.content.MimetypeMap;
 import org.alfresco.repo.model.Repository;
+import org.alfresco.repo.policy.BehaviourFilter;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.repo.transaction.RetryingTransactionHelper;
 import org.alfresco.service.cmr.repository.ChildAssociationRef;
@@ -62,6 +64,7 @@ public class CommentsTest
     private static final ApplicationContext testContext = ApplicationContextHelper.getApplicationContext();
     
     // Services
+    private static BehaviourFilter behaviourFilter;
     private static ContentService contentService;
     private static NodeService nodeService;
     private static Repository repositoryHelper;
@@ -76,6 +79,7 @@ public class CommentsTest
      */
     @BeforeClass public static void initTestsContext() throws Exception
     {
+        behaviourFilter = (BehaviourFilter)testContext.getBean("policyBehaviourFilter");
         contentService = (ContentService)testContext.getBean("ContentService");
         nodeService = (NodeService)testContext.getBean("NodeService");
         repositoryHelper = (Repository)testContext.getBean("repositoryHelper");
@@ -208,6 +212,93 @@ public class CommentsTest
             });
     }
     
+    /**
+     * This test method tests that commented nodes from before Swift have their comment counts correctly rolled up.
+     * Nodes that were commented on in prior versions of Alfresco will not have commentCount rollups -
+     * neither the aspect nor the property defined within it. Alfresco lazily calculates commentCount rollups for these
+     * nodes. So they will appear to have a count of 0 (undefined, really) and will not be given the "(count)" UI decoration.
+     * Then when a comment is added (or removed), the comment count should be recalculated from scratch.
+     */
+    @Test public void testRollupOfPreSwiftNodes() throws Exception
+    {
+        transactionHelper.doInTransaction(new RetryingTransactionHelper.RetryingTransactionCallback<Void>()
+            {
+                @Override
+                public Void execute() throws Throwable
+                {
+                    assertTrue("Not enough test docs for this test case", testDocs.size() >= 2);
+                    NodeRef node1 = testDocs.get(0);
+                    NodeRef node2 = testDocs.get(1);
+                    
+                    // We will simulate pre-Swift commenting by temporarily disabling the behaviours that add the aspect & do the rollups.
+                    behaviourFilter.disableBehaviour(ForumModel.TYPE_POST);
+                    
+                    for (NodeRef nr : new NodeRef[]{node1, node2})
+                    {
+                        // All test nodes initially do not have the commentsRollup aspect.
+                        assertFalse("Test node had comments rollup aspect.", nodeService.hasAspect(nr, ForumModel.ASPECT_COMMENTS_ROLLUP));
+                    }
+                    
+                    // Comment on each node - we need to save one comment noderef in order to delete it later.
+                    NodeRef commentOnNode1 = applyComment(node1, "Hello", true);
+                    applyComment(node1, "Bonjour", true);
+                    applyComment(node2, "Hola", true);
+                    applyComment(node2, "Bout ye?", true);
+                    
+                    // Check that the rollup comment counts are still not present. And re-enable the behaviours after we check.
+                    for (NodeRef nr : new NodeRef[]{node1, node2})
+                    {
+                        assertFalse("Test node had comments rollup aspect.", nodeService.hasAspect(nr, ForumModel.ASPECT_COMMENTS_ROLLUP));
+                    }
+                    behaviourFilter.enableBehaviour(ForumModel.TYPE_POST);
+                    
+                    // Now the addition or deletion of a comment, should trigger a recalculation of the comment rollup from scratch.
+                    applyComment(node2, "hello again");
+                    nodeService.deleteNode(commentOnNode1);
+                    assertCommentCountIs(node2, 3);
+                    assertCommentCountIs(node1, 1);
+                    
+                    return null;
+                }
+            });
+    }
+    
+    /**
+     * This test method tests that nodes whose commentCount is set to -1 have their commentCounts recalculated.
+     * This feature (see ALF-8498) is to allow customers to set their counts to -1 thus triggering a recount for that document.
+     */
+    @Test public void testTriggerCommentRecount() throws Exception
+    {
+        transactionHelper.doInTransaction(new RetryingTransactionHelper.RetryingTransactionCallback<Void>()
+            {
+                @Override
+                public Void execute() throws Throwable
+                {
+                    NodeRef testDoc = testDocs.get(0);
+                    applyComment(testDoc, "Hello 1");
+                    applyComment(testDoc, "Hello 2");
+                    applyComment(testDoc, "Hello 3");
+                    
+                    assertCommentCountIs(testDoc, 3);
+                    
+                    // We'll cheat and just set it to an arbitrary value.
+                    nodeService.setProperty(testDoc, ForumModel.PROP_COMMENT_COUNT, 42);
+                    
+                    // It should have that value - even though it's wrong.
+                    assertCommentCountIs(testDoc, 42);
+                    
+                    // Now we'll set it to the trigger value -1.
+                    nodeService.setProperty(testDoc, ForumModel.PROP_COMMENT_COUNT, ForumPostBehaviours.COUNT_TRIGGER_VALUE);
+                    
+                    // It should have the correct, recalculated value.
+                    assertCommentCountIs(testDoc, 3);
+                    
+                    return null;
+                }
+            });
+    }
+
+    
     /**
      * This method asserts that the commentCount (rollup) is as specified for the given node.
      */
@@ -228,6 +319,11 @@ public class CommentsTest
         }
     }
     
+    private NodeRef applyComment(NodeRef nr, String comment)
+    {
+        return applyComment(nr, comment, false);
+    }
+
     /**
      * This method applies the specified comment to the specified node.
      * As there is no CommentService or DiscussionService, we mimic here what the comments REST API does,
@@ -235,9 +331,12 @@ public class CommentsTest
      * of the work for us. See comments.post.json.js for comparison.
      * @param nr nodeRef to comment on.
      * @param comment the text of the comment.
+     * @param suppressRollups if true, commentsRollup aspect will not be added.
      * @return the NodeRef of the fm:post comment node.
+     * 
+     * @see CommentsTest#testRollupOfPreSwiftNodes() for use of suppressRollups.
      */
-    private NodeRef applyComment(NodeRef nr, String comment)
+    private NodeRef applyComment(NodeRef nr, String comment, boolean suppressRollups)
     {
         // There is no CommentService, so we have to create the node structure by hand.
         // This is what happens within e.g. comment.put.json.js when comments are submitted via the REST API.
@@ -245,7 +344,7 @@ public class CommentsTest
         {
             nodeService.addAspect(nr, ForumModel.ASPECT_DISCUSSABLE, null);
         }
-        if (!nodeService.hasAspect(nr, ForumModel.ASPECT_COMMENTS_ROLLUP))
+        if (!nodeService.hasAspect(nr, ForumModel.ASPECT_COMMENTS_ROLLUP) && !suppressRollups)
         {
             nodeService.addAspect(nr, ForumModel.ASPECT_COMMENTS_ROLLUP, null);
         }

source/java/org/alfresco/repo/forum/ForumPostBehaviours.java

@@ -18,16 +18,26 @@
  */
 package org.alfresco.repo.forum;
 
+import java.io.Serializable;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import org.alfresco.error.AlfrescoRuntimeException;
 import org.alfresco.model.ForumModel;
 import org.alfresco.repo.node.NodeServicePolicies;
 import org.alfresco.repo.node.NodeServicePolicies.BeforeDeleteNodePolicy;
 import org.alfresco.repo.node.NodeServicePolicies.OnCreateNodePolicy;
+import org.alfresco.repo.node.NodeServicePolicies.OnUpdatePropertiesPolicy;
 import org.alfresco.repo.policy.JavaBehaviour;
 import org.alfresco.repo.policy.PolicyComponent;
 import org.alfresco.service.cmr.repository.ChildAssociationRef;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.NodeService;
-import org.alfresco.service.cmr.repository.StoreRef;
+import org.alfresco.service.namespace.QName;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 
 /**
  * This class registers behaviours for the {@link ForumModel#TYPE_POST fm:post} content type.
@@ -37,26 +47,47 @@ import org.alfresco.service.cmr.repository.StoreRef;
  * @since 4.0
  */
 public class ForumPostBehaviours implements NodeServicePolicies.OnCreateNodePolicy,
-                                            NodeServicePolicies.BeforeDeleteNodePolicy
+                                            NodeServicePolicies.BeforeDeleteNodePolicy,
+                                            NodeServicePolicies.OnUpdatePropertiesPolicy
 {
+    public static final int COUNT_TRIGGER_VALUE = -1;
+    
+    private static final Log log = LogFactory.getLog(ForumPostBehaviours.class);
+    
     private PolicyComponent policyComponent;
+    private CommentService commentService;
     private NodeService nodeService;
+    private NodeService rawNodeService;
     
     public void setPolicyComponent(PolicyComponent policyComponent) 
     {
         this.policyComponent = policyComponent;
     }
     
+    public void setCommentService(CommentService commentService)
+    {
+        this.commentService = commentService;
+    }
+    
     public void setNodeService(NodeService nodeService)
     {
         this.nodeService = nodeService;
     }
     
+    public void setRawNodeService(NodeService nodeService)
+    {
+        this.rawNodeService = nodeService;
+    }
+
     /**
      * Initialise method
      */
     public void init()
     {
+        this.policyComponent.bindClassBehaviour(
+                OnUpdatePropertiesPolicy.QNAME,
+                ForumModel.ASPECT_COMMENTS_ROLLUP,
+                new JavaBehaviour(this, "onUpdateProperties"));
         this.policyComponent.bindClassBehaviour(
                 OnCreateNodePolicy.QNAME,
                 ForumModel.TYPE_POST,
@@ -67,72 +98,179 @@ public class ForumPostBehaviours implements NodeServicePolicies.OnCreateNodePoli
                 new JavaBehaviour(this, "beforeDeleteNode"));
     }
     
+    @Override
+    public void onUpdateProperties(NodeRef commentsRollupNode,
+            Map<QName, Serializable> before, Map<QName, Serializable> after)
+    {
+        // This method is only concerned with the value of fm:commentCount.
+        // If it has been set to a trigger value, then we initiate a full recalculation of the comment count.
+        Serializable newCommentCount = after.get(ForumModel.PROP_COMMENT_COUNT);
+        if (newCommentCount != null)
+        {
+            Integer newCommentCountInt = (Integer)newCommentCount;
+            if (newCommentCountInt == COUNT_TRIGGER_VALUE)
+            {
+                if (log.isDebugEnabled())
+                {
+                    StringBuilder msg = new StringBuilder();
+                    msg.append(commentsRollupNode)
+                       .append(" had its ").append(ForumModel.PROP_COMMENT_COUNT.getLocalName())
+                       .append(" property set to ").append(newCommentCountInt);
+                    log.debug(msg.toString());
+                    log.debug("Triggering a comment recount...");
+                }
+                
+                final int realCommentTotal = calculateCommentTotalByNodeCounting(commentsRollupNode);
+                if (realCommentTotal != -1)
+                {
+                    nodeService.setProperty(commentsRollupNode, ForumModel.PROP_COMMENT_COUNT, realCommentTotal);
+                }
+            }
+        }
+    }
+
+    /**
+     * 
+     * @param discussableNode discussable node.
+     * @return
+     */
+    private int calculateCommentTotalByNodeCounting(NodeRef discussableNode)
+    {
+        if ( !nodeService.hasAspect(discussableNode, ForumModel.ASPECT_DISCUSSABLE))
+        {
+            throw new IllegalArgumentException("Node did not have " + ForumModel.ASPECT_DISCUSSABLE + " aspect.");
+        }
+        
+        NodeRef topicNode = commentService.getShareCommentsTopic(discussableNode);
+        
+        if (log.isDebugEnabled())
+        {
+            StringBuilder msg = new StringBuilder();
+            msg.append("Recounting comments for node ").append(discussableNode);
+            log.debug(msg.toString());
+            
+            msg = new StringBuilder();
+            msg.append("Topic node: ").append(topicNode);
+            log.debug(msg.toString());
+        }
+        
+        if (topicNode == null)
+        {
+            throw new NullPointerException("Topic node was null");
+        }
+        
+        // Can't ask the commentService for a count, as it will give us -1.
+        // Need to recalculate by hand.
+        
+        //TODO This could be replaced with a GetChildrenCannedQuery.
+        // Look for fm:post nodes only.
+        Set<QName> childNodeTypeQNames = new HashSet<QName>(1);
+        childNodeTypeQNames.add(ForumModel.TYPE_POST);
+        
+        // We'll use the raw, small 'n' nodeService as the big 'N' NodeService's interceptors would limit results.
+        List<ChildAssociationRef> fmPostChildren = rawNodeService.getChildAssocs(topicNode, childNodeTypeQNames);
+        final int commentTotal = fmPostChildren.size();
+        
+        return commentTotal;
+    }
+    
     @Override
     public void onCreateNode(ChildAssociationRef childAssocRef)
     {
-        // We have a new comment under a discussable node.
+        adjustCommentCount(childAssocRef.getChildRef(), true);
-        // We need to find the fm:commentsCount ancestor to this comment node and increment its commentCount
-        NodeRef commentsRollupNode = getCommentsRollupAncestor(childAssocRef.getParentRef());
-        
-        if (commentsRollupNode != null)
-        {
-            int existingCommentCount = (Integer) nodeService.getProperty(commentsRollupNode, ForumModel.PROP_COMMENT_COUNT);
-            nodeService.setProperty(commentsRollupNode, ForumModel.PROP_COMMENT_COUNT, existingCommentCount + 1);
-        }
     }
     
     @Override
     public void beforeDeleteNode(NodeRef nodeRef)
     {
-        // We have one less comment under a discussable node.
+        adjustCommentCount(nodeRef, false);
-        // We need to find the fm:commentsRollup ancestor to this comment node and decrement its commentCount
+    }
-        NodeRef topicNode = nodeService.getPrimaryParent(nodeRef).getParentRef();
-        NodeRef commentsRollupNode = getCommentsRollupAncestor(topicNode);
     
-        if (commentsRollupNode != null)
+    /**
+     * This method adjusts the {@link ForumModel#PROP_COMMENT_COUNT} based on the supplied increment/decrement flag
+     * .
+     * @param fmPostNode the fm:post node (the comment node)
+     * @param incrementing <tt>true</tt> if we're incrementing the count, else <tt>false</tt>.
+     */
+    private void adjustCommentCount(NodeRef fmPostNode, boolean incrementing)
+    {
+        // We have a new or a deleted comment under a discussable node.
+        // We need to find the fm:commentsCount ancestor to this comment node and adjust its commentCount
+        NodeRef discussableAncestor = commentService.getDiscussableAncestor(fmPostNode, ForumModel.TYPE_POST);
+        
+        if (discussableAncestor != null)
         {
-            int existingCommentCount = (Integer) nodeService.getProperty(commentsRollupNode, ForumModel.PROP_COMMENT_COUNT);
+            if (discussableNodeRequiresFullRecount(discussableAncestor))
-            int newCommentCount = Math.max(0, existingCommentCount - 1); // Negative values should not occur, but we'll stop them anyway.
+            {
-            nodeService.setProperty(commentsRollupNode, ForumModel.PROP_COMMENT_COUNT, newCommentCount);
+                int recount = calculateCommentTotalByNodeCounting(discussableAncestor);
+                
+                nodeService.addAspect(discussableAncestor, ForumModel.ASPECT_COMMENTS_ROLLUP, null);
+                int newCountValue = recount;
+                // If the node is being deleted then the above node-count will include the to-be-deleted node.
+                // This is because the policies are onCreateNode and *before*DeleteNode
+                if ( !incrementing)
+                {
+                    newCountValue--;
+                }
+                
+                if (log.isDebugEnabled())
+                {
+                    log.debug(discussableAncestor + " newCountValue: " + newCountValue);
+                }
+                
+                nodeService.setProperty(discussableAncestor, ForumModel.PROP_COMMENT_COUNT, newCountValue);
+            }
+            else
+            {
+                Integer existingCommentCount = (Integer) nodeService.getProperty(discussableAncestor, ForumModel.PROP_COMMENT_COUNT);
+                int existingCommentCountInt = existingCommentCount == null ? 0 : existingCommentCount.intValue();
+                
+                int delta = incrementing ? 1 : -1;
+                
+                nodeService.setProperty(discussableAncestor, ForumModel.PROP_COMMENT_COUNT, existingCommentCountInt + delta);
+            }
         }
     }
     
     /**
-     * This method navigates up the primary parent containment path to find the ancestor with the
+     * This method checks if a {@link ForumModel#ASPECT_DISCUSSABLE} node requires a full recount of its comments.
-     * {@link ForumModel#ASPECT_COMMENTS_ROLLUP commentsRollup} aspect.
+     * This will occur if any of the following are true:
-     * 
+     * <ul>
-     * @param topicNode
+     *    <li>the node has no {@link ForumModel#ASPECT_COMMENTS_ROLLUP} aspect</li>
-     * @return the NodeRef of the commentsRollup ancestor if there is one, else <code>null</code>.
+     *    <li>the {@link ForumModel#PROP_COMMENT_COUNT} is a negative number</li>
+     * </ul>
      */
-    private NodeRef getCommentsRollupAncestor(NodeRef topicNode)
+    private boolean discussableNodeRequiresFullRecount(NodeRef discussableNode)
     {
-        // We are specifically trying to roll up "comment" counts here. In other words the number of "comments" on a node
+        boolean result;
-        // as applied through the Share UI.
-        // We are not trying to roll up generic fm:post counts. Although, of course, comments are modelled as fm:post nodes.
-        // So there are two scenarios in which we do not want to roll up changes to the count.
-        // 1. When the fm:post node is not a Share comment.
-        // 2. When the node is being deleted as part of a cascade delete.
-        // If an ancestor node to an fm:post is deleted then the parent structure may have been flattened within the archive store.
-        //
-        NodeRef result = null;
         
-        NodeRef forumNode = nodeService.getPrimaryParent(topicNode).getParentRef();
+        if ( !nodeService.hasAspect(discussableNode, ForumModel.ASPECT_DISCUSSABLE))
-        if (ForumModel.TYPE_FORUM.equals(nodeService.getType(forumNode)) && !forumNode.getStoreRef().equals(StoreRef.PROTOCOL_ARCHIVE))
         {
-            NodeRef commentsRollupNode = nodeService.getPrimaryParent(forumNode).getParentRef();
+            throw new AlfrescoRuntimeException("Node did not have fm:discussable aspect as expected.");
-            
-            if (!commentsRollupNode.getStoreRef().equals(StoreRef.PROTOCOL_ARCHIVE))
-            {
-                if (! nodeService.hasAspect(commentsRollupNode, ForumModel.ASPECT_COMMENTS_ROLLUP))
-                {
-                    result = null;
-                }
-                else
-                {
-                    result = commentsRollupNode;
-                }
-            }
         }
+        
+        if ( !nodeService.hasAspect(discussableNode, ForumModel.ASPECT_COMMENTS_ROLLUP))
+        {
+            result = true;
+        }
+        else
+        {
+            Integer existingCommentCount = (Integer) nodeService.getProperty(discussableNode, ForumModel.PROP_COMMENT_COUNT);
+            result = existingCommentCount == null || existingCommentCount <= COUNT_TRIGGER_VALUE;
+        }
+        
+        if (log.isDebugEnabled())
+        {
+            StringBuilder msg = new StringBuilder();
+            msg.append(discussableNode).append(" does");
+            if ( !result)
+            {
+                msg.append(" not");
+            }
+            msg.append(" require full comment recount");
+            log.debug(msg.toString());
+        }
+        
         return result;
     }
 }