From 63efb0936a910d2051ae806bf60f80ba3497ab17 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Lievremont Date: Fri, 22 May 2015 11:08:06 +0200 Subject: [PATCH] SONAR-6465 Replace groupsCount by groups --- .../sonar/server/user/ws/SearchAction.java | 25 +++++++++++-------- .../sonar/server/user/ws/example-search.json | 8 ++++-- .../server/user/ws/SearchActionTest.java | 12 ++++----- .../user/ws/SearchActionTest/five_users.json | 10 ++++---- .../ws/SearchActionTest/user_with_groups.json | 2 +- .../sonar/core/user/GroupMembershipDao.java | 18 +++++++------ .../core/user/GroupMembershipMapper.java | 2 +- .../{UserGroupCount.java => LoginGroup.java} | 10 ++++---- .../sonar/core/user/GroupMembershipMapper.xml | 6 ++--- .../core/user/GroupMembershipDaoTest.java | 15 +++++------ 10 files changed, 58 insertions(+), 50 deletions(-) rename sonar-core/src/main/java/org/sonar/core/user/{UserGroupCount.java => LoginGroup.java} (90%) diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/ws/SearchAction.java b/server/sonar-server/src/main/java/org/sonar/server/user/ws/SearchAction.java index 0e6e76c6208..2868829d065 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/ws/SearchAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/ws/SearchAction.java @@ -24,9 +24,10 @@ import com.google.common.base.Function; import com.google.common.collect.Collections2; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; +import com.google.common.collect.Multimap; +import com.google.common.collect.Multimaps; import java.util.Collection; import java.util.List; -import java.util.Map; import java.util.Set; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -48,8 +49,8 @@ public class SearchAction implements UsersWsAction { private static final String FIELD_NAME = "name"; private static final String FIELD_EMAIL = "email"; private static final String FIELD_SCM_ACCOUNTS = "scmAccounts"; - private static final String FIELD_GROUPS_COUNT = "groupsCount"; - private static final Set FIELDS = ImmutableSet.of(FIELD_LOGIN, FIELD_NAME, FIELD_EMAIL, FIELD_SCM_ACCOUNTS, FIELD_GROUPS_COUNT); + private static final String FIELD_GROUPS = "groups"; + private static final Set FIELDS = ImmutableSet.of(FIELD_LOGIN, FIELD_NAME, FIELD_EMAIL, FIELD_SCM_ACCOUNTS, FIELD_GROUPS); private final UserIndex userIndex; private final DbClient dbClient; @@ -81,7 +82,7 @@ public class SearchAction implements UsersWsAction { List fields = request.paramAsStrings(Param.FIELDS); SearchResult result = userIndex.search(request.param(Param.TEXT_QUERY), options); - Map groupsByLogin = Maps.newHashMap(); + Multimap groupsByLogin = Multimaps.forMap(Maps.newHashMap()); DbSession session = dbClient.openSession(false); try { Collection logins = Collections2.transform(result.getDocs(), new Function() { @@ -90,7 +91,7 @@ public class SearchAction implements UsersWsAction { return input.login(); } }); - groupsByLogin = dbClient.groupMembershipDao().countGroupsByLogins(session, logins); + groupsByLogin = dbClient.groupMembershipDao().selectGroupsByLogins(session, logins); } finally { session.close(); } @@ -101,7 +102,7 @@ public class SearchAction implements UsersWsAction { json.endObject().close(); } - private void writeUsers(JsonWriter json, SearchResult result, @Nullable List fields, Map groupsByLogin) { + private void writeUsers(JsonWriter json, SearchResult result, @Nullable List fields, Multimap groupsByLogin) { json.name("users").beginArray(); for (UserDoc user : result.getDocs()) { @@ -109,7 +110,7 @@ public class SearchAction implements UsersWsAction { writeIfNeeded(json, user.login(), FIELD_LOGIN, fields); writeIfNeeded(json, user.name(), FIELD_NAME, fields); writeIfNeeded(json, user.email(), FIELD_EMAIL, fields); - writeIfNeeded(json, groupsByLogin.get(user.login()), FIELD_GROUPS_COUNT, fields); + writeGroupsIfNeeded(json, groupsByLogin.get(user.login()), fields); if (fieldIsWanted(FIELD_SCM_ACCOUNTS, fields)) { json.name(FIELD_SCM_ACCOUNTS) .beginArray() @@ -127,9 +128,13 @@ public class SearchAction implements UsersWsAction { } } - private void writeIfNeeded(JsonWriter json, Integer value, String field, @Nullable List fields) { - if (fieldIsWanted(field, fields)) { - json.prop(field, value); + private void writeGroupsIfNeeded(JsonWriter json, Collection groups, @Nullable List fields) { + if (fieldIsWanted(FIELD_GROUPS, fields)) { + json.name(FIELD_GROUPS).beginArray(); + for (String groupName : groups) { + json.value(groupName); + } + json.endArray(); } } diff --git a/server/sonar-server/src/main/resources/org/sonar/server/user/ws/example-search.json b/server/sonar-server/src/main/resources/org/sonar/server/user/ws/example-search.json index 7b959b2c594..39448ce788d 100644 --- a/server/sonar-server/src/main/resources/org/sonar/server/user/ws/example-search.json +++ b/server/sonar-server/src/main/resources/org/sonar/server/user/ws/example-search.json @@ -4,13 +4,17 @@ "login": "fmallet", "name": "Freddy Mallet", "active": true, - "email": "f@m.com" + "email": "f@m.com", + "scmAccounts": [], + "groups": ["sonar-users", "sonar-administrators"] }, { "login": "sbrandhof", "name": "Simon", "active": true, - "scmAccounts": ["simon.brandhof", "s.brandhof@company.tld"] + "email": "s.brandhof@company.tld", + "scmAccounts": ["simon.brandhof", "s.brandhof@company.tld"], + "groups": ["sonar-users"] } ] } diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/ws/SearchActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/ws/SearchActionTest.java index a2456afbac0..c5177efbcaa 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/user/ws/SearchActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/user/ws/SearchActionTest.java @@ -124,35 +124,35 @@ public class SearchActionTest { .contains("name") .contains("email") .contains("scmAccounts") - .contains("groupsCount"); + .contains("groups"); assertThat(tester.newGetRequest("api/users", "search").setParam("f", "").execute().outputAsString()) .contains("login") .contains("name") .contains("email") .contains("scmAccounts") - .contains("groupsCount"); + .contains("groups"); assertThat(tester.newGetRequest("api/users", "search").setParam("f", "login").execute().outputAsString()) .contains("login") .doesNotContain("name") .doesNotContain("email") .doesNotContain("scmAccounts") - .doesNotContain("groupsCount"); + .doesNotContain("groups"); assertThat(tester.newGetRequest("api/users", "search").setParam("f", "scmAccounts").execute().outputAsString()) .doesNotContain("login") .doesNotContain("name") .doesNotContain("email") .contains("scmAccounts") - .doesNotContain("groupsCount"); + .doesNotContain("groups"); - assertThat(tester.newGetRequest("api/users", "search").setParam("f", "groupsCount").execute().outputAsString()) + assertThat(tester.newGetRequest("api/users", "search").setParam("f", "groups").execute().outputAsString()) .doesNotContain("login") .doesNotContain("name") .doesNotContain("email") .doesNotContain("scmAccounts") - .contains("groupsCount"); + .contains("groups"); } @Test diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/five_users.json b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/five_users.json index d568e8b0b7e..86b57c314ca 100644 --- a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/five_users.json +++ b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/five_users.json @@ -10,7 +10,7 @@ "scmAccounts": [ "user-0" ], - "groupsCount": 0 + "groups": [] }, { "login": "user-1", @@ -19,7 +19,7 @@ "scmAccounts": [ "user-1" ], - "groupsCount": 0 + "groups": [] }, { "login": "user-2", @@ -28,7 +28,7 @@ "scmAccounts": [ "user-2" ], - "groupsCount": 0 + "groups": [] }, { "login": "user-3", @@ -37,7 +37,7 @@ "scmAccounts": [ "user-3" ], - "groupsCount": 0 + "groups": [] }, { "login": "user-4", @@ -46,7 +46,7 @@ "scmAccounts": [ "user-4" ], - "groupsCount": 0 + "groups": [] } ] } diff --git a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/user_with_groups.json b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/user_with_groups.json index a49e8252732..6b2d6543f65 100644 --- a/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/user_with_groups.json +++ b/server/sonar-server/src/test/resources/org/sonar/server/user/ws/SearchActionTest/user_with_groups.json @@ -10,7 +10,7 @@ "scmAccounts": [ "user-0" ], - "groupsCount": 2 + "groups": ["sonar-admins", "sonar-users"] } ] } diff --git a/sonar-core/src/main/java/org/sonar/core/user/GroupMembershipDao.java b/sonar-core/src/main/java/org/sonar/core/user/GroupMembershipDao.java index 31ff32405c2..84447d42d1f 100644 --- a/sonar-core/src/main/java/org/sonar/core/user/GroupMembershipDao.java +++ b/sonar-core/src/main/java/org/sonar/core/user/GroupMembershipDao.java @@ -22,8 +22,10 @@ package org.sonar.core.user; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; +import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; +import com.google.common.collect.Multimap; import java.util.Collection; import java.util.List; import java.util.Map; @@ -80,16 +82,16 @@ public class GroupMembershipDao implements DaoComponent { return result; } - public Map countGroupsByLogins(final DbSession session, Collection logins) { - final Map result = Maps.newHashMap(); - DaoUtils.executeLargeInputs(logins, new NonNullInputFunction, List>() { + public Multimap selectGroupsByLogins(final DbSession session, Collection logins) { + final Multimap result = ArrayListMultimap.create(); + DaoUtils.executeLargeInputs(logins, new NonNullInputFunction, List>() { @Override - protected List doApply(List input) { - List groupCounts = mapper(session).countGroupsByLogins(input); - for (UserGroupCount count : groupCounts) { - result.put(count.login(), count.groupCount()); + protected List doApply(List input) { + List groupMemberships = mapper(session).selectGroupsByLogins(input); + for (LoginGroup membership : groupMemberships) { + result.put(membership.login(), membership.groupName()); } - return groupCounts; + return groupMemberships; } }); diff --git a/sonar-core/src/main/java/org/sonar/core/user/GroupMembershipMapper.java b/sonar-core/src/main/java/org/sonar/core/user/GroupMembershipMapper.java index b4a66a5ba9a..777aa1eea00 100644 --- a/sonar-core/src/main/java/org/sonar/core/user/GroupMembershipMapper.java +++ b/sonar-core/src/main/java/org/sonar/core/user/GroupMembershipMapper.java @@ -34,5 +34,5 @@ public interface GroupMembershipMapper { List countUsersByGroup(@Param("groupIds") List groupIds); - List countGroupsByLogins(@Param("logins") List logins); + List selectGroupsByLogins(@Param("logins") List logins); } diff --git a/sonar-core/src/main/java/org/sonar/core/user/UserGroupCount.java b/sonar-core/src/main/java/org/sonar/core/user/LoginGroup.java similarity index 90% rename from sonar-core/src/main/java/org/sonar/core/user/UserGroupCount.java rename to sonar-core/src/main/java/org/sonar/core/user/LoginGroup.java index 712c1b435f1..7f25a8cee5f 100644 --- a/sonar-core/src/main/java/org/sonar/core/user/UserGroupCount.java +++ b/sonar-core/src/main/java/org/sonar/core/user/LoginGroup.java @@ -19,16 +19,16 @@ */ package org.sonar.core.user; -public class UserGroupCount { +public class LoginGroup { private String login; - private int groupCount; + private String groupName; public String login() { return login; } - public int groupCount() { - return groupCount; + public String groupName() { + return groupName; } -} \ No newline at end of file +} diff --git a/sonar-core/src/main/resources/org/sonar/core/user/GroupMembershipMapper.xml b/sonar-core/src/main/resources/org/sonar/core/user/GroupMembershipMapper.xml index 7774f9eb4b0..2663c390189 100644 --- a/sonar-core/src/main/resources/org/sonar/core/user/GroupMembershipMapper.xml +++ b/sonar-core/src/main/resources/org/sonar/core/user/GroupMembershipMapper.xml @@ -45,17 +45,17 @@ GROUP BY g.name - + SELECT u.login as login, g.name as groupName FROM users u LEFT JOIN groups_users gu ON gu.user_id=u.id + INNER JOIN groups g ON gu.group_id=g.id u.login in #{login} - GROUP BY u.login diff --git a/sonar-core/src/test/java/org/sonar/core/user/GroupMembershipDaoTest.java b/sonar-core/src/test/java/org/sonar/core/user/GroupMembershipDaoTest.java index 570a7921777..ed86008d099 100644 --- a/sonar-core/src/test/java/org/sonar/core/user/GroupMembershipDaoTest.java +++ b/sonar-core/src/test/java/org/sonar/core/user/GroupMembershipDaoTest.java @@ -20,6 +20,7 @@ package org.sonar.core.user; +import com.google.common.collect.Multimap; import java.util.Arrays; import java.util.List; import org.junit.Before; @@ -192,15 +193,11 @@ public class GroupMembershipDaoTest { DbSession session = dbTester.myBatis().openSession(false); try { - assertThat(dao.countGroupsByLogins(session, Arrays.asList())).isEmpty(); - assertThat(dao.countGroupsByLogins(session, Arrays.asList("two-hundred"))) - .containsExactly(entry("two-hundred", 3)); - assertThat(dao.countGroupsByLogins(session, Arrays.asList("two-hundred", "two-hundred-one"))) - .containsOnly(entry("two-hundred", 3), entry("two-hundred-one", 1)); - assertThat(dao.countGroupsByLogins(session, Arrays.asList("two-hundred", "two-hundred-one", "two-hundred-two"))) - .containsOnly(entry("two-hundred", 3), entry("two-hundred-one", 1), entry("two-hundred-two", 0)); - assertThat(dao.countGroupsByLogins(session, Arrays.asList("two-hundred-two"))) - .containsOnly(entry("two-hundred-two", 0)); + assertThat(dao.selectGroupsByLogins(session, Arrays.asList()).keys()).isEmpty(); + Multimap groupsByLogin = dao.selectGroupsByLogins(session, Arrays.asList("two-hundred", "two-hundred-one", "two-hundred-two")); + assertThat(groupsByLogin.get("two-hundred")).containsOnly("sonar-administrators", "sonar-users", "sonar-reviewers"); + assertThat(groupsByLogin.get("two-hundred-one")).containsOnly("sonar-users"); + assertThat(groupsByLogin.get("two-hundred-two")).isEmpty(); } finally { session.close(); } -- 2.39.5