REPO-1306: Delete group membership

- Added functionality and tests
   - Added minor changes to create group membership functionality ( REPO-1307 )

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

source/java/org/alfresco/rest/api/Groups.java

@@ -116,7 +116,6 @@ public interface Groups
      * @param parameters
      *            the {@link Parameters} object to get the parameters passed
      *            into the request including: - include param (parentIds, zones)
-     * @return Updated group
      */
     void delete(String groupId, Parameters parameters);
 
@@ -139,4 +138,13 @@ public interface Groups
      * @return a {@code org.alfresco.rest.api.model.GroupMember} object
      */
     GroupMember createGroupMember(String groupId, GroupMember groupMember);
+
+    /**
+     *
+     * Delete group membership
+     *
+     * @param groupId
+     * @param groupMemberId
+     */
+    void deleteGroupMembership(String groupId, String groupMemberId);
 }

source/java/org/alfresco/rest/api/groups/GroupMembersRelation.java

@@ -45,7 +45,8 @@ import org.springframework.beans.factory.InitializingBean;
  * @author cturlica
  */
 @RelationshipResource(name = "members", entityResource = GroupsEntityResource.class, title = "Group Members")
-public class GroupMembersRelation implements RelationshipResourceAction.Read<GroupMember>, RelationshipResourceAction.Create<GroupMember>, InitializingBean
+public class GroupMembersRelation
+        implements RelationshipResourceAction.Read<GroupMember>, RelationshipResourceAction.Create<GroupMember>, RelationshipResourceAction.Delete, InitializingBean
 {
     private Groups groups;
 
@@ -76,4 +77,11 @@ public class GroupMembersRelation implements RelationshipResourceAction.Read<Gro
         result.add(groups.createGroupMember(groupId, entity.get(0)));
         return result;
     }
+
+    @Override
+    @WebApiDescription(title = "Delete group membership")
+    public void delete(String entityResourceId, String id, Parameters parameters)
+    {
+        groups.deleteGroupMembership(entityResourceId, id);
+    }
 }

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

@@ -620,13 +620,14 @@ public class GroupsImpl implements Groups
 
         if (!authorityService.authorityExists(groupMember.getId()))
         {
-            throw new EntityNotFoundException("Group member with id " + groupMember.getId() + " does not exists");
+            throw new EntityNotFoundException(groupMember.getId());
         }
 
         AuthorityType existingAuthorityType = AuthorityType.getAuthorityType(groupMember.getId());
         if (existingAuthorityType != authorityType)
         {
-            throw new IllegalArgumentException("Incorrect group member type, " + existingAuthorityType + " exists with the given id");
+            throw new IllegalArgumentException("Incorrect group member type, "
+                    + (AuthorityType.USER.equals(existingAuthorityType) ? Groups.PARAM_MEMBER_TYPE_PERSON : existingAuthorityType) + " exists with the given id");
         }
 
         authorityService.addAuthority(groupId, groupMember.getId());
@@ -635,6 +636,14 @@ public class GroupsImpl implements Groups
         return getGroupMember(authority);
     }
 
+    public void deleteGroupMembership(String groupId, String groupMemberId)
+    {
+        validateGroupId(groupId, false);
+        validateGroupMemberId(groupMemberId);
+        // TODO: Verify if groupMemberId is member of groupId
+        authorityService.removeAuthority(groupId, groupMemberId);
+    }
+
     private AuthorityType getAuthorityType(String memberType)
     {
         AuthorityType authorityType = null;
@@ -735,6 +744,18 @@ public class GroupsImpl implements Groups
         }
     }
     
+    private void validateGroupMemberId(String groupMemberId)
+    {
+        if (groupMemberId == null || groupMemberId.isEmpty())
+        {
+            throw new InvalidArgumentException("group member id is null or empty");
+        }
+        if (!(personAuthorityExists(groupMemberId) || groupAuthorityExists(groupMemberId, false)))
+        {
+            throw new EntityNotFoundException(groupMemberId);
+        }
+    }
+
     private void validateGroup(Group group, boolean isUpdate)
     {
         if (group == null)
@@ -811,6 +832,11 @@ public class GroupsImpl implements Groups
         return authorityExists(AuthorityType.GROUP, authorityName, inferPrefix);
     }
 
+    private boolean personAuthorityExists(String authorityName)
+    {
+        return authorityExists(AuthorityType.USER, authorityName, false);
+    }
+
     private boolean authorityExists(AuthorityType authorityType, String authorityName, boolean inferPrefix)
     {
         String name = inferPrefix ? authorityService.getName(authorityType, authorityName) : authorityName;

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

@@ -69,6 +69,7 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
 {
     private static final String MEMBER_TYPE_GROUP = "GROUP";
     private static final String MEMBER_TYPE_PERSON = "PERSON";
+    private static final String GROUP_EVERYONE = "GROUP_EVERYONE";
 
     protected AuthorityService authorityService;
 
@@ -77,6 +78,7 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
     private Group groupB = null;
     private GroupMember groupMemberA = null;
     private GroupMember groupMemberB = null;
+    private GroupMember personMember = null;
 
     @Before
     public void setup() throws Exception
@@ -351,7 +353,7 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
      * @param userName
      *            The user to run as.
      */
-    private void createAuthorityContext(String userName)
+    private void createAuthorityContext(String userName) throws PublicApiException
     {
         String groupName = "Group_ROOT" + GUID.generate();
 
@@ -390,6 +392,23 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
             groupMemberB.setId(groupBAuthorityName);
             groupMemberB.setMemberType(AuthorityType.GROUP.toString());
         }
+
+        {
+            publicApiClient.setRequestContext(new RequestContext(networkOne.getId(), networkAdmin, "admin"));
+            Person personAlice = new Person();
+            String aliceId = "alice-" + UUID.randomUUID() + "@" + networkOne.getId();
+            personAlice.setUserName(aliceId);
+            personAlice.setId(aliceId);
+            personAlice.setFirstName("Alice");
+            personAlice.setEmail("alison.smith@example.com");
+            personAlice.setPassword("password");
+            personAlice.setEnabled(true);
+            PublicApiClient.People people = publicApiClient.people();
+            people.create(personAlice);
+            personMember = new GroupMember();
+            personMember.setId(personAlice.getId());
+            personMember.setMemberType(MEMBER_TYPE_PERSON);
+        }
     }
 
     /**
@@ -520,7 +539,7 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
             ListResponse<Group> groups = groupsProxy.getGroupsByPersonId(personAlice.getId(), null, "Couldn't get user's groups", 200);
             assertEquals(1L, (long) groups.getPaging().getTotalItems());
             Iterator<Group> it = groups.getList().iterator();
-            assertEquals("GROUP_EVERYONE", it.next().getId());
+            assertEquals(GROUP_EVERYONE, it.next().getId());
         }
 
         // Add the user to a couple more groups and list them.
@@ -532,7 +551,7 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
             ListResponse<Group> groups = groupsProxy.getGroupsByPersonId(personAlice.getId(), null, "Couldn't get user's groups", 200);
             assertEquals(4L, (long) groups.getPaging().getTotalItems());
             Iterator<Group> it = groups.getList().iterator();
-            assertEquals("GROUP_EVERYONE", it.next().getId());
+            assertEquals(GROUP_EVERYONE, it.next().getId());
             assertEquals(rootGroupName, it.next().getId());
             assertEquals(groupA, it.next());
             assertEquals(groupB, it.next());
@@ -565,7 +584,7 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
             ListResponse<Group> groups = groupsProxy.getGroupsByPersonId("-me-", null, "Couldn't get user's groups", 200);
             assertEquals(4L, (long) groups.getPaging().getCount());
             Iterator<Group> it = groups.getList().iterator();
-            assertEquals("GROUP_EVERYONE", it.next().getId());
+            assertEquals(GROUP_EVERYONE, it.next().getId());
             assertEquals(rootGroupName, it.next().getId());
             assertEquals(groupA, it.next());
             assertEquals(groupB, it.next());
@@ -941,25 +960,6 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
         {
             createAuthorityContext(user1);
 
-            Person personAlice;
-            {
-                publicApiClient.setRequestContext(new RequestContext(networkOne.getId(), networkAdmin, "admin"));
-                personAlice = new Person();
-                String aliceId = "alice-" + UUID.randomUUID() + "@" + networkOne.getId();
-                personAlice.setUserName(aliceId);
-                personAlice.setId(aliceId);
-                personAlice.setFirstName("Alice");
-                personAlice.setEmail("alison.smith@example.com");
-                personAlice.setPassword("password");
-                personAlice.setEnabled(true);
-                PublicApiClient.People people = publicApiClient.people();
-                people.create(personAlice);
-            }
-
-            GroupMember personMember = new GroupMember();
-            personMember.setId(personAlice.getId());
-            personMember.setMemberType(MEMBER_TYPE_PERSON);
-
             // +ve tests
             // Create a group membership (for a existing person and a sub-group)
             // within a group groupId
@@ -1165,7 +1165,7 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
         {
             setRequestContext(networkOne.getId(), networkAdmin, DEFAULT_ADMIN_PWD);
 
-            groupsProxy.deleteGroup("GROUP_EVERYONE", false, HttpServletResponse.SC_CONFLICT);
+            groupsProxy.deleteGroup(GROUP_EVERYONE, false, HttpServletResponse.SC_CONFLICT);
         }
 
         // Trying to delete a person.
@@ -1200,6 +1200,73 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
         }
     }
 
+    @Test
+    public void testDeleteGroupMembership() throws Exception
+    {
+        final Groups groupsProxy = publicApiClient.groups();
+
+        try
+        {
+            createAuthorityContext(user1);
+
+            {
+                Map<String, String> otherParams = new HashMap<>();
+                otherParams.put("include", org.alfresco.rest.api.Groups.PARAM_INCLUDE_PARENT_IDS);
+                Group createdTestGroup = groupsProxy.createGroup(generateGroup(), null, HttpServletResponse.SC_CREATED);
+                // Add new created group to groupA
+                GroupMember groupMember = new GroupMember();
+                groupMember.setId(createdTestGroup.getId());
+                groupMember.setMemberType(MEMBER_TYPE_GROUP);
+                groupsProxy.createGroupMember(groupA.getId(), groupMember, HttpServletResponse.SC_CREATED);
+
+                // If a removed sub-group no longer has any parent groups then
+                // it becomes a root group.
+                assertFalse(groupsProxy.getGroup(groupMember.getId(), otherParams, HttpServletResponse.SC_OK).getParentIds().isEmpty());
+                groupsProxy.deleteGroupMembership(groupA.getId(), groupMember.getId(), HttpServletResponse.SC_NO_CONTENT);
+                assertTrue(groupsProxy.getGroup(groupMember.getId(), otherParams, HttpServletResponse.SC_OK).getParentIds().isEmpty());
+            }
+
+            {
+                // Add new a person as a member of groupA
+                groupsProxy.createGroupMember(groupA.getId(), personMember, HttpServletResponse.SC_CREATED);
+                ListResponse<Group> groups = groupsProxy.getGroupsByPersonId(personMember.getId(), null, "Cannot retrieve user groups", 200);
+                assertEquals(3L, (long) groups.getPaging().getTotalItems());
+                Iterator<Group> it = groups.getList().iterator();
+                assertEquals(GROUP_EVERYONE, it.next().getId());
+                assertEquals(rootGroupName, it.next().getId());
+                assertEquals(groupA, it.next());
+                groupsProxy.deleteGroupMembership(groupA.getId(), personMember.getId(), HttpServletResponse.SC_NO_CONTENT);
+                groups = groupsProxy.getGroupsByPersonId(personMember.getId(), null, "Cannot retrieve user groups", 200);
+                assertEquals(1L, (long) groups.getPaging().getTotalItems());
+                it = groups.getList().iterator();
+                assertEquals(GROUP_EVERYONE, it.next().getId());
+            }
+
+            // -ve tests
+            // Group id or group member id do not exist.
+            {
+                groupsProxy.deleteGroupMembership("invalidGroupId", groupMemberA.getId(), HttpServletResponse.SC_NOT_FOUND);
+                groupsProxy.deleteGroupMembership(groupA.getId(), "invalidGroupMemberId", HttpServletResponse.SC_NOT_FOUND);
+            }
+
+            // Authentication failed
+            {
+                setRequestContext(networkOne.getId(), GUID.generate(), "password");
+                groupsProxy.deleteGroupMembership(groupA.getId(), groupMemberA.getId(), HttpServletResponse.SC_UNAUTHORIZED);
+            }
+
+            // User does not have permission to delete a group membership
+            {
+                setRequestContext(user1);
+                groupsProxy.deleteGroupMembership(groupA.getId(), groupMemberA.getId(), HttpServletResponse.SC_FORBIDDEN);
+            }
+        }
+        finally
+        {
+            clearAuthorityContext();
+        }
+    }
+
     private Group generateGroup()
     {
         Group group = new Group();

source/test-java/org/alfresco/rest/api/tests/client/PublicApiClient.java

@@ -2335,7 +2335,12 @@ public class PublicApiClient
             {
                 params = Collections.singletonMap("cascade", "true");
             }
-            remove("groups", groupId, null, null, params, "Failed to remove site", expectedStatus);
+            remove("groups", groupId, null, null, params, "Failed to remove group", expectedStatus);
+        }
+
+        public void deleteGroupMembership(String groupId, String groupMemberId, int expectedStatus) throws PublicApiException
+        {
+	        remove("groups", groupId, "members", groupMemberId, null, "Failed to remove group member", expectedStatus);
         }
 
         private Group parseGroupEntity(HttpResponse response)