Merged 5.1.N (5.1.2) to 5.2.N (5.2.1)

128375 jvonka: MNT-16446: Edit Comment permission (part 3) 
   - for CMIS (and also future v1 Nodes REST API)


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/BRANCHES/DEV/5.2.N/root@128386 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

config/alfresco/public-rest-context.xml

@@ -407,8 +407,6 @@
         <property name="nodeService" ref="NodeService" />
         <property name="commentService" ref="CommentService" />
         <property name="contentService" ref="ContentService" />
-        <property name="permissionService" ref="PermissionService" />
-        <property name="lockService" ref="LockService" />
         <property name="typeConstraint" ref="nodeTypeConstraint" />
     </bean>
 

source/java/org/alfresco/repo/web/scripts/comments/CommentsPost.java

@@ -55,7 +55,6 @@ import org.springframework.extensions.webscripts.WebScriptRequest;
  * @author Sergey Scherbovich (based on existing JavaScript webscript controller)
  * @since 4.1.7.1
  */
-
 public class CommentsPost extends AbstractCommentsWebScript
 {
     /**
@@ -146,7 +145,8 @@ public class CommentsPost extends AbstractCommentsWebScript
         {
            isUpdated = ((Date)modified).getTime() - ((Date)created).getTime() > 5000;
         }
-        
+
+        // TODO refactor v0 Comments API to use CommentService (see ACE-5437)
         Serializable owner = this.nodeService.getProperty(commentNodeRef, ContentModel.PROP_OWNER);
         String currentUser = this.serviceRegistry.getAuthenticationService().getCurrentUserName();
         

source/java/org/alfresco/rest/api/impl/CommentsImpl.java

@@ -71,8 +71,6 @@ public class CommentsImpl implements Comments
     private NodeService nodeService;
     private CommentService commentService;
     private ContentService contentService;
-    private LockService lockService;
-    private PermissionService permissionService;
     private TypeConstraint typeConstraint;
 
 	public void setTypeConstraint(TypeConstraint typeConstraint)
@@ -84,17 +82,7 @@ public class CommentsImpl implements Comments
 	{
 		this.nodes = nodes;
 	}
-	
+    
-	public void setLockService(LockService lockService)
-	{
-		this.lockService = lockService;
-	}
-
-	public void setPermissionService(PermissionService permissionService)
-	{
-		this.permissionService = permissionService;
-	}
-
 	public void setNodeService(NodeService nodeService)
     {
         this.nodeService = nodeService;
@@ -122,72 +110,16 @@ public class CommentsImpl implements Comments
 	        nodeProps.remove(ContentModel.PROP_CONTENT);
         }
 
-        boolean canEdit = false;
-        boolean canDelete = false;
 
-        if (! isWorkingCopyOrLocked(nodeRef))
+        Map<String, Boolean> map = commentService.getCommentPermissions(nodeRef, commentNodeRef);
-        {
+        boolean canEdit = map.get(CommentService.CAN_EDIT);
-            canEdit = canEditPermission(commentNodeRef);
+        boolean canDelete =  map.get(CommentService.CAN_DELETE);
-            canDelete = canDeletePermission(commentNodeRef);
-        }
 
 
         Comment comment = new Comment(commentNodeRef.getId(), nodeProps, canEdit, canDelete);
         return comment;
     }
-
+    
-    private boolean isWorkingCopyOrLocked(NodeRef nodeRef)
-    {
-        boolean isWorkingCopy = false;
-        boolean isLocked = false;
-
-        if (nodeRef != null)
-        {
-            Set<QName> aspects = nodeService.getAspects(nodeRef);
-
-            isWorkingCopy = aspects.contains(ContentModel.ASPECT_WORKING_COPY);
-            if(!isWorkingCopy)
-            {
-                if(aspects.contains(ContentModel.ASPECT_LOCKABLE))
-                {
-                    LockStatus lockStatus = lockService.getLockStatus(nodeRef);
-                    if (lockStatus == LockStatus.LOCKED || lockStatus == LockStatus.LOCK_OWNER)
-                    {
-                        isLocked = true;
-                    }
-                }
-            }
-        }
-        return (isWorkingCopy || isLocked);
-    }
-
-    private boolean canEdit(NodeRef nodeRef, NodeRef commentNodeRef)
-    {
-        return ((! isWorkingCopyOrLocked(nodeRef) && canEditPermission(commentNodeRef)));
-    }
-
-    // TODO refactor (ACE-5437) - see also CommentsPost
-    private boolean canEditPermission(NodeRef commentNodeRef)
-    {
-        String creator = (String)nodeService.getProperty(commentNodeRef, ContentModel.PROP_CREATOR);
-        Serializable owner = nodeService.getProperty(commentNodeRef, ContentModel.PROP_OWNER);
-        String currentUser = AuthenticationUtil.getFullyAuthenticatedUser();
-
-        boolean isSiteManager = permissionService.hasPermission(commentNodeRef, SiteModel.SITE_MANAGER) == (AccessStatus.ALLOWED);
-        boolean isCoordinator = permissionService.hasPermission(commentNodeRef, PermissionService.COORDINATOR) == (AccessStatus.ALLOWED);
-        return (isSiteManager || isCoordinator || currentUser.equals(creator) || currentUser.equals(owner));
-    }
-
-    private boolean canDelete(NodeRef nodeRef, NodeRef commentNodeRef)
-    {
-        return ((! isWorkingCopyOrLocked(nodeRef) || canDeletePermission(commentNodeRef)));
-    }
-
-    private boolean canDeletePermission(NodeRef commentNodeRef)
-    {
-        return permissionService.hasPermission(commentNodeRef, PermissionService.DELETE) == AccessStatus.ALLOWED;
-    }
-
     public Comment createComment(String nodeId, Comment comment)
     {
 		NodeRef nodeRef = nodes.validateNode(nodeId);
@@ -223,13 +155,7 @@ public class CommentsImpl implements Comments
 			{
 				throw new InvalidArgumentException();
 			}
-
+            
-            // MNT-16446 (pending future ACE-5437)
-            if (! canEdit(nodeRef, commentNodeRef))
-            {
-                throw new PermissionDeniedException("Cannot edit comment");
-            }
-
             commentService.updateComment(commentNodeRef, title, content);
 	        return toComment(nodeRef, commentNodeRef);
 		}
@@ -274,20 +200,18 @@ public class CommentsImpl implements Comments
     }
 
     @Override
-    // TODO validate that it is a comment of the node
     public void deleteComment(String nodeId, String commentNodeId)
     {
     	try
     	{
             NodeRef nodeRef = nodes.validateNode(nodeId);
 	        NodeRef commentNodeRef = nodes.validateNode(commentNodeId);
-
+            
-            // MNT-16446 (pending future ACE-5437)
+            if (! nodeRef.equals(commentService.getDiscussableAncestor(commentNodeRef)))
-            if (! canDelete(nodeRef, commentNodeRef))
             {
-                throw new PermissionDeniedException("Cannot delete comment");
+                throw new InvalidArgumentException("Unexpected "+nodeId+","+commentNodeId);
             }
-
+            
             commentService.deleteComment(commentNodeRef);
 		}
 		catch(IllegalArgumentException e)

source/test-java/org/alfresco/rest/api/tests/TestNodeComments.java

@@ -753,23 +753,9 @@ public class TestNodeComments extends EnterpriseTestApi
 			comment.setContent("my comment");
 			Comment createdComment = commentsProxy.createNodeComment(nodeRef1.getId(), comment);
 
-			{
+			Comment updatedComment = new Comment();
-				TenantUtil.runAsUserTenant(new TenantRunAsWork<Void>()
+			updatedComment.setContent(null);
-				{
+			commentsProxy.updateNodeComment(nodeRef1.getId(), createdComment.getId(), updatedComment);
-					@Override
-					public Void doWork() throws Exception
-					{
-						repoService.lockNode(nodeRef1);
-						return null;
-					}
-				}, person11.getId(), network1.getId());
-	
-				publicApiClient.setRequestContext(new RequestContext(network1.getId(), person11.getId()));
-
-				Comment updatedComment = new Comment();
-				updatedComment.setContent(null);
-				commentsProxy.updateNodeComment(nodeRef1.getId(), createdComment.getId(), updatedComment);
-			}
 			
 			fail();
 		}
@@ -778,9 +764,15 @@ public class TestNodeComments extends EnterpriseTestApi
 			assertEquals(HttpStatus.SC_BAD_REQUEST, e.getHttpResponse().getStatusCode());
 		}
 
-		// locked node comments
+		// locked node - cannot add/edit/delete comments (MNT-14945, MNT-16446)
 		try
 		{
+			publicApiClient.setRequestContext(new RequestContext(network1.getId(), person11.getId()));
+			
+			Comment comment = new Comment();
+			comment.setContent("my comment");
+			Comment createdComment = commentsProxy.createNodeComment(nodeRef1.getId(), comment);
+			
 			TenantUtil.runAsUserTenant(new TenantRunAsWork<Void>()
 			{
 				@Override
@@ -801,10 +793,18 @@ public class TestNodeComments extends EnterpriseTestApi
 			commentsProxy.getNodeComments(nodeRef1.getId(), createParams(paging, null));
 
 			// test POST for a locked node
+			try
+			{
+				comment = new Comment();
+				comment.setContent("my other comment");
+				createdComment = commentsProxy.createNodeComment(nodeRef1.getId(), comment);
 
-			Comment comment = new Comment();
+				fail("");
-			comment.setContent("my comment");
+			}
-			Comment createdComment = commentsProxy.createNodeComment(nodeRef1.getId(), comment);
+			catch(PublicApiException e)
+			{
+				assertEquals(HttpStatus.SC_FORBIDDEN, e.getHttpResponse().getStatusCode());
+			}
 
 			// test PUT for a locked node
 			{