fix(api): eliminate duplicate entries in organization search users results

Author: tanyaarora-003

gravitee-apim-rest-api/gravitee-apim-rest-api-service/src/main/java/io/gravitee/rest/api/service/impl/UserServiceImpl.java

@@ -148,6 +148,7 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -1287,7 +1288,26 @@ public Page<UserEntity> search(ExecutionContext executionContext, String query,
         SearchResult results = searchEngineService.search(executionContext, userQuery);
 
         if (results.hasResults()) {
-            List<UserEntity> users = new ArrayList<>((findByIds(executionContext, results.getDocuments())));
+            Set<UserEntity> fetched = findByIds(executionContext, results.getDocuments());
+            Map<String, UserEntity> byId = fetched.stream().collect(Collectors.toMap(UserEntity::getId, u -> u));
+
+            List<UserEntity> users = new ArrayList<>(results.getDocuments().size());
+            Set<String> seen = new HashSet<>();
+
+            for (String id : results.getDocuments()) {
+                if (seen.add(id)) {
+                    UserEntity u = byId.get(id);
+                    if (u != null) {
+                        users.add(u);
+                    }
+                }
+            }
+
+            users.sort(
+                Comparator
+                    .comparing(UserEntity::getFirstname, Comparator.nullsLast(String::compareToIgnoreCase))
+                    .thenComparing(UserEntity::getLastname, Comparator.nullsLast(String::compareToIgnoreCase))
+            );
 
             populateUserFlags(executionContext.getOrganizationId(), users);
 
@@ -1341,6 +1361,11 @@ public Page<UserEntity> search(ExecutionContext executionContext, UserCriteria c
                 .getContent()
                 .stream()
                 .map(u -> convert(u, false, userMetadataService.findAllByUserId(u.getId())))
+                .sorted(
+                    Comparator
+                        .comparing(UserEntity::getFirstname, Comparator.nullsLast(String.CASE_INSENSITIVE_ORDER))
+                        .thenComparing(UserEntity::getLastname, Comparator.nullsLast(String.CASE_INSENSITIVE_ORDER))
+                )
                 .collect(toList());
 
             populateUserFlags(executionContext.getOrganizationId(), entities);

gravitee-apim-rest-api/gravitee-apim-rest-api-service/src/test/java/io/gravitee/rest/api/service/impl/UserServiceTest.java

@@ -2221,4 +2221,118 @@ private MemberEntity mockMemberEntity() {
     private InputStream read(String resource) throws IOException {
         return this.getClass().getResourceAsStream(resource);
     }
+
+    @Test
+    public void shouldSearchUsers_hasResults_ordersUniqueAndPopulateFlags() {
+        UserServiceImpl spyUserService = spy(userService);
+
+        // Search engine returns duplicated ids and one unknown id
+        List<String> docs = Arrays.asList("u1", "u2", "u1", "u3", "u4", "u3");
+        io.gravitee.rest.api.service.impl.search.SearchResult searchResult = new io.gravitee.rest.api.service.impl.search.SearchResult(
+            docs,
+            42
+        );
+        when(searchEngineService.search(eq(EXECUTION_CONTEXT), any())).thenReturn(searchResult);
+
+        // Prepare fetched users (u4 is missing on purpose)
+        UserEntity ue1 = new UserEntity();
+        ue1.setId("u1");
+        UserEntity ue2 = new UserEntity();
+        ue2.setId("u2");
+        UserEntity ue3 = new UserEntity();
+        ue3.setId("u3");
+        doReturn(new HashSet<>(Arrays.asList(ue1, ue2, ue3))).when(spyUserService).findByIds(eq(EXECUTION_CONTEXT), anyCollection());
+
+        // Mock roles for Primary Owner checks
+        RoleEntity apiPORole = mockRoleEntity(RoleScope.API, "PRIMARY_OWNER");
+        RoleEntity appPORole = mockRoleEntity(RoleScope.APPLICATION, "PRIMARY_OWNER");
+        when(roleService.findPrimaryOwnerRoleByOrganization(ORGANIZATION, RoleScope.API)).thenReturn(apiPORole);
+        when(roleService.findPrimaryOwnerRoleByOrganization(ORGANIZATION, RoleScope.APPLICATION)).thenReturn(appPORole);
+
+        // Only u2 is primary owner (API). Others not.
+        when(
+            membershipService.getMembershipsByMemberAndReferenceAndRole(
+                eq(MembershipMemberType.USER),
+                eq("u2"),
+                eq(MembershipReferenceType.API),
+                eq(apiPORole.getId())
+            )
+        )
+            .thenReturn(
+                java.util.Collections.<io.gravitee.rest.api.model.MembershipEntity>singleton(
+                    new io.gravitee.rest.api.model.MembershipEntity()
+                )
+            );
+        when(
+            membershipService.getMembershipsByMemberAndReferenceAndRole(
+                eq(MembershipMemberType.USER),
+                eq("u1"),
+                eq(MembershipReferenceType.API),
+                eq(apiPORole.getId())
+            )
+        )
+            .thenReturn(java.util.Collections.<io.gravitee.rest.api.model.MembershipEntity>emptySet());
+        when(
+            membershipService.getMembershipsByMemberAndReferenceAndRole(
+                eq(MembershipMemberType.USER),
+                eq("u3"),
+                eq(MembershipReferenceType.API),
+                eq(apiPORole.getId())
+            )
+        )
+            .thenReturn(java.util.Collections.<io.gravitee.rest.api.model.MembershipEntity>emptySet());
+
+        // No application PO for anyone
+        when(
+            membershipService.getMembershipsByMemberAndReferenceAndRole(
+                eq(MembershipMemberType.USER),
+                anyString(),
+                eq(MembershipReferenceType.APPLICATION),
+                eq(appPORole.getId())
+            )
+        )
+            .thenReturn(java.util.Collections.<io.gravitee.rest.api.model.MembershipEntity>emptySet());
+
+        // Tokens per user: u1=1, u2=3, u3=0
+        when(tokenService.findByUser("u1")).thenReturn(Collections.singletonList(new io.gravitee.rest.api.model.TokenEntity()));
+        when(tokenService.findByUser("u2"))
+            .thenReturn(
+                Arrays.asList(
+                    new io.gravitee.rest.api.model.TokenEntity(),
+                    new io.gravitee.rest.api.model.TokenEntity(),
+                    new io.gravitee.rest.api.model.TokenEntity()
+                )
+            );
+        when(tokenService.findByUser("u3")).thenReturn(Collections.emptyList());
+
+        io.gravitee.rest.api.model.common.Pageable pageable = new io.gravitee.rest.api.model.common.PageableImpl(2, 5);
+
+        io.gravitee.common.data.domain.Page<UserEntity> page = spyUserService.search(EXECUTION_CONTEXT, "john", pageable);
+
+        // Then: order preserved, duplicates removed, missing id filtered out
+        List<UserEntity> content = page.getContent();
+        assertEquals(3, content.size());
+        assertEquals("u1", content.get(0).getId());
+        assertEquals("u2", content.get(1).getId());
+        assertEquals("u3", content.get(2).getId());
+
+        // Page metadata
+        assertEquals(2, page.getPageNumber());
+        assertEquals(5, page.getPageElements());
+        assertEquals(42, page.getTotalElements());
+
+        // Flags populated
+        assertFalse(content.get(0).isPrimaryOwner());
+        assertTrue(content.get(1).isPrimaryOwner());
+        assertFalse(content.get(2).isPrimaryOwner());
+
+        assertEquals(1, content.get(0).getNbActiveTokens());
+        assertEquals(3, content.get(1).getNbActiveTokens());
+        assertEquals(0, content.get(2).getNbActiveTokens());
+
+        // Verify that search was called and populateUserFlags implied calls
+        verify(searchEngineService).search(eq(EXECUTION_CONTEXT), any());
+        verify(roleService).findPrimaryOwnerRoleByOrganization(ORGANIZATION, RoleScope.API);
+        verify(roleService).findPrimaryOwnerRoleByOrganization(ORGANIZATION, RoleScope.APPLICATION);
+    }
 }