REPO-2520: Community: SiteMembership Java API calls should tell which site resulted in errors

Changed tests asserts from (actual value, expected value) to (expected value, actual value) as recommended in the Junit Assert Javadoc. git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/BRANCHES/DEV/5.2.N/root@137771 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261
2025-10-08 14:51:49 +00:00 · 2017-06-22 12:57:06 +00:00
parent 4191e3beed
commit df009a5e27
51 changed files with 962 additions and 234 deletions
--- a/source/java/org/alfresco/repo/forum/CommentServiceImpl.java
+++ b/source/java/org/alfresco/repo/forum/CommentServiceImpl.java
@@ -613,17 +613,18 @@ public class CommentServiceImpl extends AbstractLifecycleBean implements Comment
        NodeRef discussableNodeRef = getDiscussableAncestor(commentNodeRef);
        if (discussableNodeRef.equals(discussableNode))
        {
-            if (!isWorkingCopyOrLocked(discussableNode))
+            if (!isWorkingCopyOrLocked(discussableNode)
+                    || isLockOwner(discussableNode))
            {
                canEdit = canEditPermission(commentNodeRef);
                canDelete = canDeletePermission(commentNodeRef);
            }
        }
-        
+
        Map<String, Boolean> map = new HashMap<>(2);
        map.put(CommentService.CAN_EDIT, canEdit);
        map.put(CommentService.CAN_DELETE, canDelete);
-        
+
        return map;
    }

@@ -637,12 +638,17 @@ public class CommentServiceImpl extends AbstractLifecycleBean implements Comment
        boolean isCoordinator = permissionService.hasPermission(commentNodeRef, PermissionService.COORDINATOR) == (AccessStatus.ALLOWED);
        return (isSiteManager || isCoordinator || currentUser.equals(creator) || currentUser.equals(owner));
    }
-    
+
    private boolean canDeletePermission(NodeRef commentNodeRef)
    {
        return permissionService.hasPermission(commentNodeRef, PermissionService.DELETE) == AccessStatus.ALLOWED;
    }

+    private boolean isLockOwner(NodeRef nodeRef)
+    {
+        return lockService.getLockStatus(nodeRef) == LockStatus.LOCK_OWNER;
+    }
+
    private boolean isWorkingCopyOrLocked(NodeRef nodeRef)
    {
        boolean isWorkingCopy = false;
@@ -667,7 +673,7 @@ public class CommentServiceImpl extends AbstractLifecycleBean implements Comment
        }
        return (isWorkingCopy || isLocked);
    }
-    
+
    @Override
    public void onUpdateProperties(NodeRef commentNodeRef, Map<QName, Serializable> before, Map<QName, Serializable> after)
    {
@@ -675,10 +681,11 @@ public class CommentServiceImpl extends AbstractLifecycleBean implements Comment
        if (discussableNodeRef != null)
        {
            if (behaviourFilter.isEnabled(ContentModel.ASPECT_LOCKABLE)
-                    && isWorkingCopyOrLocked(discussableNodeRef))
+                    && isWorkingCopyOrLocked(discussableNodeRef)
+                    && !isLockOwner(discussableNodeRef))
            {
                List<QName> changedProperties = getChangedProperties(before, after);
-                
+
                // check if comment updated (rather than some other change, eg. addition of lockable aspect only)
                boolean commentUpdated = false;
                for (QName changedProperty : changedProperties)
@@ -693,7 +700,7 @@ public class CommentServiceImpl extends AbstractLifecycleBean implements Comment
                        }
                    }
                }
-                
+
                if (commentUpdated)
                {
                    throw new NodeLockedException(discussableNodeRef);
@@ -741,20 +748,22 @@ public class CommentServiceImpl extends AbstractLifecycleBean implements Comment

        return results;
    }
-    
+
    @Override
    public void beforeDeleteNode(final NodeRef commentNodeRef)
    {
        NodeRef discussableNodeRef = getDiscussableAncestor(commentNodeRef);
        if (discussableNodeRef != null)
        {
+            boolean canDelete = canDeletePermission(commentNodeRef);
            if (behaviourFilter.isEnabled(ContentModel.ASPECT_LOCKABLE) // eg. delete site (MNT-14671)
-                    && isWorkingCopyOrLocked(discussableNodeRef))
+                    && isWorkingCopyOrLocked(discussableNodeRef)
+                    && !isLockOwner(discussableNodeRef)
+                    && !canDelete)
            {
                throw new NodeLockedException(discussableNodeRef);
            }

-            boolean canDelete = canDeletePermission(commentNodeRef);
            if (! canDelete)
            {
                throw new AccessDeniedException("Cannot delete comment");
--- a/source/java/org/alfresco/repo/site/SiteMembership.java
+++ b/source/java/org/alfresco/repo/site/SiteMembership.java
@@ -34,134 +34,142 @@ import org.alfresco.service.cmr.site.SiteRole;
 * @author steveglover
 *
 */
-public class SiteMembership
-{
-	private SiteInfo siteInfo;
-	private String personId;
-	private String firstName;
-	private String lastName;
-	private String role;
-
-	public SiteMembership(SiteInfo siteInfo, String personId, String firstName, String lastName, String role)
-	{
-		super();
-		if(siteInfo == null)
-		{
-			throw new java.lang.IllegalArgumentException();
-		}
-		if(personId == null)
-		{
-			throw new java.lang.IllegalArgumentException();
-		}
-		if(firstName == null)
-		{
-			throw new java.lang.IllegalArgumentException();
-		}
-		if(lastName == null)
-		{
-			throw new java.lang.IllegalArgumentException();
-		}
-		if(role == null)
-		{
-			throw new java.lang.IllegalArgumentException();
-		}
-		this.siteInfo = siteInfo;
-		this.personId = personId;
-		this.firstName = firstName;
-		this.lastName = lastName;
-		this.role = role;
-	}
-
-	public SiteMembership(SiteInfo siteInfo, String personId, String role)
-	{
-		super();
-		if(siteInfo == null)
-		{
-			throw new java.lang.IllegalArgumentException();
-		}
-		if(personId == null)
-		{
-			throw new java.lang.IllegalArgumentException();
-		}
-		if(role == null)
-		{
-			throw new java.lang.IllegalArgumentException();
-		}
-
-		this.siteInfo = siteInfo;
-		this.personId = personId;
-		this.role = role;
-	}
-
-	public SiteInfo getSiteInfo()
-	{
-		return siteInfo;
-	}
-
-	public String getPersonId()
-	{
-		return personId;
-	}
-
-	public String getFirstName()
-	{
-		return firstName;
-	}
-
-	public String getLastName()
-	{
-		return lastName;
-	}
-
-	public String getRole()
-	{
-		return role;
-	}
-
-	@Override
-	public int hashCode()
-	{
-		final int prime = 31;
-		int result = 1;
-		result = prime * result
-				+ ((personId == null) ? 0 : personId.hashCode());
-		result = prime * result + ((role == null) ? 0 : role.hashCode());
-		result = prime * result
-				+ ((getSiteInfo() == null) ? 0 : getSiteInfo().hashCode());
-		return result;
-	}
-
-	@Override
-	public boolean equals(Object obj)
-	{
-		if (this == obj)
-			return true;
-		if (obj == null)
-			return false;
-		if (getClass() != obj.getClass())
-			return false;
-		SiteMembership other = (SiteMembership) obj;
-		if (personId == null) {
-			if (other.personId != null)
-				return false;
-		} else if (!personId.equals(other.personId))
-			return false;
-		if (role != other.role)
-			return false;
-		if (getSiteInfo() == null) {
-			if (other.getSiteInfo() != null)
-				return false;
-		} else if (!getSiteInfo().equals(other.getSiteInfo()))
-			return false;
-		return true;
-	}
-
-	@Override
-	public String toString()
-	{
-		return "SiteMembership [siteInfo=" + getSiteInfo()
-				+ ", personId=" + personId + ", firstName=" + firstName
-				+ ", lastName=" + lastName + ", role=" + role + "]";
-	}
-
-}
+public class SiteMembership
+{
+    private SiteInfo siteInfo;
+    private String personId;
+    private String firstName;
+    private String lastName;
+    private String role;
+
+    public SiteMembership(SiteInfo siteInfo, String personId, String firstName, String lastName,
+            String role)
+    {
+        super();
+        if (siteInfo == null)
+        {
+            throw new java.lang.IllegalArgumentException();
+        }
+        if (personId == null)
+        {
+            throw new java.lang.IllegalArgumentException(
+                    "Person required building site membership of " + siteInfo.getShortName());
+        }
+        if (firstName == null)
+        {
+            throw new java.lang.IllegalArgumentException(
+                    "FirstName required building site membership of " + siteInfo.getShortName());
+        }
+        if (lastName == null)
+        {
+            throw new java.lang.IllegalArgumentException(
+                    "LastName required building site membership of " + siteInfo.getShortName());
+        }
+        if (role == null)
+        {
+            throw new java.lang.IllegalArgumentException(
+                    "Role required building site membership of " + siteInfo.getShortName());
+        }
+        this.siteInfo = siteInfo;
+        this.personId = personId;
+        this.firstName = firstName;
+        this.lastName = lastName;
+        this.role = role;
+    }
+
+    public SiteMembership(SiteInfo siteInfo, String personId, String role)
+    {
+        super();
+        if (siteInfo == null)
+        {
+            throw new java.lang.IllegalArgumentException();
+        }
+        if (personId == null)
+        {
+            throw new java.lang.IllegalArgumentException(
+                    "Person required building site membership of " + siteInfo.getShortName());
+        }
+        if (role == null)
+        {
+            throw new java.lang.IllegalArgumentException(
+                    "Role required building site membership of " + siteInfo.getShortName());
+        }
+
+        this.siteInfo = siteInfo;
+        this.personId = personId;
+        this.role = role;
+    }
+
+    public SiteInfo getSiteInfo()
+    {
+        return siteInfo;
+    }
+
+    public String getPersonId()
+    {
+        return personId;
+    }
+
+    public String getFirstName()
+    {
+        return firstName;
+    }
+
+    public String getLastName()
+    {
+        return lastName;
+    }
+
+    public String getRole()
+    {
+        return role;
+    }
+
+    @Override
+    public int hashCode()
+    {
+        final int prime = 31;
+        int result = 1;
+        result = prime * result + ((personId == null) ? 0 : personId.hashCode());
+        result = prime * result + ((role == null) ? 0 : role.hashCode());
+        result = prime * result + ((getSiteInfo() == null) ? 0 : getSiteInfo().hashCode());
+        return result;
+    }
+
+    @Override
+    public boolean equals(Object obj)
+    {
+        if (this == obj)
+            return true;
+        if (obj == null)
+            return false;
+        if (getClass() != obj.getClass())
+            return false;
+        SiteMembership other = (SiteMembership) obj;
+        if (personId == null)
+        {
+            if (other.personId != null)
+                return false;
+        }
+        else if (!personId.equals(other.personId))
+            return false;
+        if (role != other.role)
+            return false;
+        if (getSiteInfo() == null)
+        {
+            if (other.getSiteInfo() != null)
+                return false;
+        }
+        else if (!getSiteInfo().equals(other.getSiteInfo()))
+            return false;
+        return true;
+    }
+
+    @Override
+    public String toString()
+    {
+        return "SiteMembership [siteInfo=" + getSiteInfo() + ", personId=" + personId
+                + ", firstName=" + firstName + ", lastName=" + lastName + ", role=" + role + "]";
+    }
+
+}