Join improvements: the user having fewer groups is chosen for deletion

gms/src/main/java/it/inaf/ia2/gms/controller/JWTWebServiceController.java

@@ -356,10 +356,10 @@ public class JWTWebServiceController {
             throw new BadRequestException("Missing alternative subject");
         }
 
-        joinService.join(fromUser, toUser);
+        String mergedId = joinService.join(fromUser, toUser);
 
         Map<String, String> responseBody = new HashMap<>();
-        responseBody.put("mergedId", fromUser);
+        responseBody.put("mergedId", mergedId);
         return ResponseEntity.ok(responseBody);
     }
 }

gms/src/main/java/it/inaf/ia2/gms/service/JoinService.java

@@ -4,8 +4,10 @@ import it.inaf.ia2.gms.model.Permission;
 import it.inaf.ia2.gms.persistence.JoinDAO;
 import it.inaf.ia2.gms.persistence.MembershipsDAO;
 import it.inaf.ia2.gms.persistence.PermissionsDAO;
+import it.inaf.ia2.gms.persistence.model.GroupEntity;
 import it.inaf.ia2.gms.persistence.model.MembershipEntity;
 import it.inaf.ia2.gms.persistence.model.PermissionEntity;
+import java.util.List;
 import java.util.Set;
 import java.util.stream.Collectors;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -23,33 +25,43 @@ public class JoinService {
     @Autowired
     private JoinDAO joinDAO;
 
-    public void join(String userId1, String userId2) {
+    public String join(String userId1, String userId2) {
 
-        Set<MembershipEntity> existingMemberships
-                = membershipsDAO.getUserMemberships(userId1).stream()
-                        .map(g -> getMembershipEntity(g.getId(), userId1))
-                        .collect(Collectors.toSet());
+        List<GroupEntity> user1Groups = membershipsDAO.getUserMemberships(userId1);
+        List<GroupEntity> user2Groups = membershipsDAO.getUserMemberships(userId2);
 
-        Set<MembershipEntity> membershipsToAdd
-                = membershipsDAO.getUserMemberships(userId2).stream()
-                        .map(g -> getMembershipEntity(g.getId(), userId1))
-                        .filter(m -> !existingMemberships.contains(m))
-                        .collect(Collectors.toSet());
+        // The user having less groups will be deleted
+        String remainingUserId = user1Groups.size() >= user2Groups.size() ? userId1 : userId2;
+        String deletingUserId = remainingUserId.equals(userId1) ? userId2 : userId1;
+
+        List<GroupEntity> remainingUserGroups = remainingUserId.equals(userId1) ? user1Groups : user2Groups;
+        List<GroupEntity> deletingUserGroups = deletingUserId.equals(userId1) ? user1Groups : user2Groups;
+
+        Set<MembershipEntity> existingMemberships = remainingUserGroups.stream()
+                .map(g -> getMembershipEntity(g.getId(), remainingUserId))
+                .collect(Collectors.toSet());
+
+        Set<MembershipEntity> membershipsToAdd = deletingUserGroups.stream()
+                .map(g -> getMembershipEntity(g.getId(), remainingUserId))
+                .filter(m -> !existingMemberships.contains(m))
+                .collect(Collectors.toSet());
 
         Set<PermissionEntity> existingPermissions
-                = permissionsDAO.findUserPermissions(userId1).stream()
+                = permissionsDAO.findUserPermissions(remainingUserId).stream()
                         .collect(Collectors.toSet());
 
         Set<PermissionEntity> permissionsToAdd
-                = permissionsDAO.findUserPermissions(userId2).stream()
+                = permissionsDAO.findUserPermissions(deletingUserId).stream()
                         .map(p -> {
-                            p.setUserId(userId1);
+                            p.setUserId(remainingUserId);
                             return p;
                         })
                         .filter(p -> isPermissionToAdd(existingPermissions, p))
                         .collect(Collectors.toSet());
 
-        joinDAO.join(membershipsToAdd, permissionsToAdd, userId2);
+        joinDAO.join(membershipsToAdd, permissionsToAdd, deletingUserId);
+
+        return remainingUserId;
     }
 
     private MembershipEntity getMembershipEntity(String groupId, String userId) {

gms/src/test/java/it/inaf/ia2/gms/controller/JWTWebServiceControllerTest.java

@@ -16,7 +16,6 @@ import it.inaf.ia2.gms.service.JoinService;
 import it.inaf.ia2.rap.data.RapUser;
 import java.security.Principal;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.List;
 import java.util.Optional;
 import org.junit.Before;
@@ -238,9 +237,6 @@ public class JWTWebServiceControllerTest {
     @Test
     public void testRemovePermission() throws Exception {
 
-        List<String> names = Arrays.asList("LBT", "INAF");
-
-        //when(groupsService.findGroupByNames(names)).thenReturn(Optional.of(inaf));
         when(groupsDAO.findGroupByParentAndName("", "LBT")).thenReturn(Optional.of(lbt));
         when(groupsDAO.findGroupByParentAndName("lbt_id", "INAF")).thenReturn(Optional.of(inaf));
 

gms/src/test/java/it/inaf/ia2/gms/service/JoinServiceTest.java

@@ -44,7 +44,7 @@ public class JoinServiceTest {
     public void testJoin() {
 
         when(membershipsDAO.getUserMemberships(eq(USER_1)))
-                .thenReturn(Arrays.asList(groupEntity("A")));
+                .thenReturn(Arrays.asList(groupEntity("A"), groupEntity("C")));
 
         when(membershipsDAO.getUserMemberships(eq(USER_2)))
                 .thenReturn(Arrays.asList(groupEntity("A"), groupEntity("B")));
@@ -64,10 +64,48 @@ public class JoinServiceTest {
                         permissionEntity("D", USER_2, Permission.VIEW_MEMBERS)
                 ));
 
-        joinService.join(USER_1, USER_2);
+        String remainingUserId = joinService.join(USER_1, USER_2);
 
-        verify(joinDAO, times(1)).join(argThat(verifyMembershipsToAdd()),
-                argThat(verifyPermissionsToAdd()), eq(USER_2));
+        verify(joinDAO, times(1)).join(argThat(memberships -> {
+            assertEquals(1, memberships.size());
+            MembershipEntity entity = memberships.iterator().next();
+            assertEquals("B", entity.getGroupId());
+            assertEquals(USER_1, entity.getUserId());
+            return true;
+        }),
+                argThat(verifyPermissionsToAdd1()), eq(USER_2));
+
+        assertEquals(USER_1, remainingUserId);
+    }
+
+    @Test
+    public void testJoinReverseOrder() {
+
+        when(membershipsDAO.getUserMemberships(eq(USER_1)))
+                .thenReturn(Arrays.asList(groupEntity("C")));
+
+        when(membershipsDAO.getUserMemberships(eq(USER_2)))
+                .thenReturn(Arrays.asList(groupEntity("A"), groupEntity("B")));
+
+        when(permissionsDAO.findUserPermissions(eq(USER_1)))
+                .thenReturn(Arrays.asList(
+                        permissionEntity("C", USER_1, Permission.ADMIN),
+                        permissionEntity("D", USER_1, Permission.VIEW_MEMBERS)
+                ));
+
+        when(permissionsDAO.findUserPermissions(eq(USER_2)))
+                .thenReturn(Arrays.asList(
+                        permissionEntity("A", USER_2, Permission.ADMIN),
+                        permissionEntity("B", USER_2, Permission.VIEW_MEMBERS),
+                        permissionEntity("C", USER_2, Permission.MANAGE_MEMBERS)
+                ));
+
+        String remainingUserId = joinService.join(USER_1, USER_2);
+
+        verify(joinDAO, times(1)).join(argThat(verifyMembershipsToAdd2()),
+                argThat(verifyPermissionsToAdd2()), eq(USER_1));
+
+        assertEquals(USER_2, remainingUserId);
     }
 
     private GroupEntity groupEntity(String groupId) {
@@ -84,7 +122,7 @@ public class JoinServiceTest {
         return permissionEntity;
     }
 
-    private ArgumentMatcher<Set<MembershipEntity>> verifyMembershipsToAdd() {
+    private ArgumentMatcher<Set<MembershipEntity>> verifyMembershipsToAdd1() {
         return memberships -> {
             assertEquals(1, memberships.size());
             MembershipEntity entity = memberships.iterator().next();
@@ -94,11 +132,29 @@ public class JoinServiceTest {
         };
     }
 
-    private ArgumentMatcher<Set<PermissionEntity>> verifyPermissionsToAdd() {
+    private ArgumentMatcher<Set<PermissionEntity>> verifyPermissionsToAdd1() {
         return permissions -> {
             assertEquals(2, permissions.size());
             return permissions.contains(permissionEntity("B", USER_1, Permission.ADMIN))
                     && permissions.contains(permissionEntity("D", USER_1, Permission.VIEW_MEMBERS));
         };
     }
+
+    private ArgumentMatcher<Set<MembershipEntity>> verifyMembershipsToAdd2() {
+        return memberships -> {
+            assertEquals(1, memberships.size());
+            MembershipEntity entity = memberships.iterator().next();
+            assertEquals("C", entity.getGroupId());
+            assertEquals(USER_2, entity.getUserId());
+            return true;
+        };
+    }
+
+    private ArgumentMatcher<Set<PermissionEntity>> verifyPermissionsToAdd2() {
+        return permissions -> {
+            assertEquals(2, permissions.size());
+            return permissions.contains(permissionEntity("C", USER_2, Permission.ADMIN))
+                    && permissions.contains(permissionEntity("D", USER_2, Permission.VIEW_MEMBERS));
+        };
+    }
 }