Skip to content

Commit

Permalink
SONAR-6465 Replace groupsCount by groups
Browse files Browse the repository at this point in the history
  • Loading branch information
jblievremont committed May 26, 2015
1 parent 4788151 commit 63efb09
Show file tree
Hide file tree
Showing 10 changed files with 58 additions and 50 deletions.
Expand Up @@ -24,9 +24,10 @@
import com.google.common.collect.Collections2; import com.google.common.collect.Collections2;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps; 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.Collection;
import java.util.List; import java.util.List;
import java.util.Map;
import java.util.Set; import java.util.Set;
import javax.annotation.Nonnull; import javax.annotation.Nonnull;
import javax.annotation.Nullable; import javax.annotation.Nullable;
Expand All @@ -48,8 +49,8 @@ public class SearchAction implements UsersWsAction {
private static final String FIELD_NAME = "name"; private static final String FIELD_NAME = "name";
private static final String FIELD_EMAIL = "email"; private static final String FIELD_EMAIL = "email";
private static final String FIELD_SCM_ACCOUNTS = "scmAccounts"; private static final String FIELD_SCM_ACCOUNTS = "scmAccounts";
private static final String FIELD_GROUPS_COUNT = "groupsCount"; private static final String FIELD_GROUPS = "groups";
private static final Set<String> FIELDS = ImmutableSet.of(FIELD_LOGIN, FIELD_NAME, FIELD_EMAIL, FIELD_SCM_ACCOUNTS, FIELD_GROUPS_COUNT); private static final Set<String> FIELDS = ImmutableSet.of(FIELD_LOGIN, FIELD_NAME, FIELD_EMAIL, FIELD_SCM_ACCOUNTS, FIELD_GROUPS);


private final UserIndex userIndex; private final UserIndex userIndex;
private final DbClient dbClient; private final DbClient dbClient;
Expand Down Expand Up @@ -81,7 +82,7 @@ public void handle(Request request, Response response) throws Exception {
List<String> fields = request.paramAsStrings(Param.FIELDS); List<String> fields = request.paramAsStrings(Param.FIELDS);
SearchResult<UserDoc> result = userIndex.search(request.param(Param.TEXT_QUERY), options); SearchResult<UserDoc> result = userIndex.search(request.param(Param.TEXT_QUERY), options);


Map<String, Integer> groupsByLogin = Maps.newHashMap(); Multimap<String, String> groupsByLogin = Multimaps.forMap(Maps.<String, String>newHashMap());
DbSession session = dbClient.openSession(false); DbSession session = dbClient.openSession(false);
try { try {
Collection<String> logins = Collections2.transform(result.getDocs(), new Function<UserDoc, String>() { Collection<String> logins = Collections2.transform(result.getDocs(), new Function<UserDoc, String>() {
Expand All @@ -90,7 +91,7 @@ public String apply(@Nonnull UserDoc input) {
return input.login(); return input.login();
} }
}); });
groupsByLogin = dbClient.groupMembershipDao().countGroupsByLogins(session, logins); groupsByLogin = dbClient.groupMembershipDao().selectGroupsByLogins(session, logins);
} finally { } finally {
session.close(); session.close();
} }
Expand All @@ -101,15 +102,15 @@ public String apply(@Nonnull UserDoc input) {
json.endObject().close(); json.endObject().close();
} }


private void writeUsers(JsonWriter json, SearchResult<UserDoc> result, @Nullable List<String> fields, Map<String, Integer> groupsByLogin) { private void writeUsers(JsonWriter json, SearchResult<UserDoc> result, @Nullable List<String> fields, Multimap<String, String> groupsByLogin) {


json.name("users").beginArray(); json.name("users").beginArray();
for (UserDoc user : result.getDocs()) { for (UserDoc user : result.getDocs()) {
json.beginObject(); json.beginObject();
writeIfNeeded(json, user.login(), FIELD_LOGIN, fields); writeIfNeeded(json, user.login(), FIELD_LOGIN, fields);
writeIfNeeded(json, user.name(), FIELD_NAME, fields); writeIfNeeded(json, user.name(), FIELD_NAME, fields);
writeIfNeeded(json, user.email(), FIELD_EMAIL, 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)) { if (fieldIsWanted(FIELD_SCM_ACCOUNTS, fields)) {
json.name(FIELD_SCM_ACCOUNTS) json.name(FIELD_SCM_ACCOUNTS)
.beginArray() .beginArray()
Expand All @@ -127,9 +128,13 @@ private void writeIfNeeded(JsonWriter json, String value, String field, @Nullabl
} }
} }


private void writeIfNeeded(JsonWriter json, Integer value, String field, @Nullable List<String> fields) { private void writeGroupsIfNeeded(JsonWriter json, Collection<String> groups, @Nullable List<String> fields) {
if (fieldIsWanted(field, fields)) { if (fieldIsWanted(FIELD_GROUPS, fields)) {
json.prop(field, value); json.name(FIELD_GROUPS).beginArray();
for (String groupName : groups) {
json.value(groupName);
}
json.endArray();
} }
} }


Expand Down
Expand Up @@ -4,13 +4,17 @@
"login": "fmallet", "login": "fmallet",
"name": "Freddy Mallet", "name": "Freddy Mallet",
"active": true, "active": true,
"email": "f@m.com" "email": "f@m.com",
"scmAccounts": [],
"groups": ["sonar-users", "sonar-administrators"]
}, },
{ {
"login": "sbrandhof", "login": "sbrandhof",
"name": "Simon", "name": "Simon",
"active": true, "active": true,
"scmAccounts": ["simon.brandhof", "s.brandhof@company.tld"] "email": "s.brandhof@company.tld",
"scmAccounts": ["simon.brandhof", "s.brandhof@company.tld"],
"groups": ["sonar-users"]
} }
] ]
} }
Expand Up @@ -124,35 +124,35 @@ public void search_with_fields() throws Exception {
.contains("name") .contains("name")
.contains("email") .contains("email")
.contains("scmAccounts") .contains("scmAccounts")
.contains("groupsCount"); .contains("groups");


assertThat(tester.newGetRequest("api/users", "search").setParam("f", "").execute().outputAsString()) assertThat(tester.newGetRequest("api/users", "search").setParam("f", "").execute().outputAsString())
.contains("login") .contains("login")
.contains("name") .contains("name")
.contains("email") .contains("email")
.contains("scmAccounts") .contains("scmAccounts")
.contains("groupsCount"); .contains("groups");


assertThat(tester.newGetRequest("api/users", "search").setParam("f", "login").execute().outputAsString()) assertThat(tester.newGetRequest("api/users", "search").setParam("f", "login").execute().outputAsString())
.contains("login") .contains("login")
.doesNotContain("name") .doesNotContain("name")
.doesNotContain("email") .doesNotContain("email")
.doesNotContain("scmAccounts") .doesNotContain("scmAccounts")
.doesNotContain("groupsCount"); .doesNotContain("groups");


assertThat(tester.newGetRequest("api/users", "search").setParam("f", "scmAccounts").execute().outputAsString()) assertThat(tester.newGetRequest("api/users", "search").setParam("f", "scmAccounts").execute().outputAsString())
.doesNotContain("login") .doesNotContain("login")
.doesNotContain("name") .doesNotContain("name")
.doesNotContain("email") .doesNotContain("email")
.contains("scmAccounts") .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("login")
.doesNotContain("name") .doesNotContain("name")
.doesNotContain("email") .doesNotContain("email")
.doesNotContain("scmAccounts") .doesNotContain("scmAccounts")
.contains("groupsCount"); .contains("groups");
} }


@Test @Test
Expand Down
Expand Up @@ -10,7 +10,7 @@
"scmAccounts": [ "scmAccounts": [
"user-0" "user-0"
], ],
"groupsCount": 0 "groups": []
}, },
{ {
"login": "user-1", "login": "user-1",
Expand All @@ -19,7 +19,7 @@
"scmAccounts": [ "scmAccounts": [
"user-1" "user-1"
], ],
"groupsCount": 0 "groups": []
}, },
{ {
"login": "user-2", "login": "user-2",
Expand All @@ -28,7 +28,7 @@
"scmAccounts": [ "scmAccounts": [
"user-2" "user-2"
], ],
"groupsCount": 0 "groups": []
}, },
{ {
"login": "user-3", "login": "user-3",
Expand All @@ -37,7 +37,7 @@
"scmAccounts": [ "scmAccounts": [
"user-3" "user-3"
], ],
"groupsCount": 0 "groups": []
}, },
{ {
"login": "user-4", "login": "user-4",
Expand All @@ -46,7 +46,7 @@
"scmAccounts": [ "scmAccounts": [
"user-4" "user-4"
], ],
"groupsCount": 0 "groups": []
} }
] ]
} }
Expand Up @@ -10,7 +10,7 @@
"scmAccounts": [ "scmAccounts": [
"user-0" "user-0"
], ],
"groupsCount": 2 "groups": ["sonar-admins", "sonar-users"]
} }
] ]
} }
Expand Up @@ -22,8 +22,10 @@


import com.google.common.annotations.VisibleForTesting; import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Function; import com.google.common.base.Function;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps; import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
import java.util.Collection; import java.util.Collection;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
Expand Down Expand Up @@ -80,16 +82,16 @@ public List<GroupUserCount> apply(@Nonnull List<Long> input) {
return result; return result;
} }


public Map<String, Integer> countGroupsByLogins(final DbSession session, Collection<String> logins) { public Multimap<String, String> selectGroupsByLogins(final DbSession session, Collection<String> logins) {
final Map<String, Integer> result = Maps.newHashMap(); final Multimap<String, String> result = ArrayListMultimap.create();
DaoUtils.executeLargeInputs(logins, new NonNullInputFunction<List<String>, List<UserGroupCount>>() { DaoUtils.executeLargeInputs(logins, new NonNullInputFunction<List<String>, List<LoginGroup>>() {
@Override @Override
protected List<UserGroupCount> doApply(List<String> input) { protected List<LoginGroup> doApply(List<String> input) {
List<UserGroupCount> groupCounts = mapper(session).countGroupsByLogins(input); List<LoginGroup> groupMemberships = mapper(session).selectGroupsByLogins(input);
for (UserGroupCount count : groupCounts) { for (LoginGroup membership : groupMemberships) {
result.put(count.login(), count.groupCount()); result.put(membership.login(), membership.groupName());
} }
return groupCounts; return groupMemberships;
} }
}); });


Expand Down
Expand Up @@ -34,5 +34,5 @@ public interface GroupMembershipMapper {


List<GroupUserCount> countUsersByGroup(@Param("groupIds") List<Long> groupIds); List<GroupUserCount> countUsersByGroup(@Param("groupIds") List<Long> groupIds);


List<UserGroupCount> countGroupsByLogins(@Param("logins") List<String> logins); List<LoginGroup> selectGroupsByLogins(@Param("logins") List<String> logins);
} }
Expand Up @@ -19,16 +19,16 @@
*/ */
package org.sonar.core.user; package org.sonar.core.user;


public class UserGroupCount { public class LoginGroup {


private String login; private String login;
private int groupCount; private String groupName;


public String login() { public String login() {
return login; return login;
} }


public int groupCount() { public String groupName() {
return groupCount; return groupName;
} }
} }
Expand Up @@ -45,17 +45,17 @@
GROUP BY g.name GROUP BY g.name
</select> </select>


<select id="countGroupsByLogins" parameterType="string" resultType="org.sonar.core.user.UserGroupCount"> <select id="selectGroupsByLogins" parameterType="string" resultType="org.sonar.core.user.LoginGroup">
SELECT u.login as login, count(gu.user_id) as groupCount SELECT u.login as login, g.name as groupName
FROM users u FROM users u
LEFT JOIN groups_users gu ON gu.user_id=u.id LEFT JOIN groups_users gu ON gu.user_id=u.id
INNER JOIN groups g ON gu.group_id=g.id
<where> <where>
u.login in u.login in
<foreach collection="logins" open="(" close=")" item="login" separator=","> <foreach collection="logins" open="(" close=")" item="login" separator=",">
#{login} #{login}
</foreach> </foreach>
</where> </where>
GROUP BY u.login
</select> </select>


</mapper> </mapper>
Expand Up @@ -20,6 +20,7 @@


package org.sonar.core.user; package org.sonar.core.user;


import com.google.common.collect.Multimap;
import java.util.Arrays; import java.util.Arrays;
import java.util.List; import java.util.List;
import org.junit.Before; import org.junit.Before;
Expand Down Expand Up @@ -192,15 +193,11 @@ public void count_groups_by_login() {
DbSession session = dbTester.myBatis().openSession(false); DbSession session = dbTester.myBatis().openSession(false);


try { try {
assertThat(dao.countGroupsByLogins(session, Arrays.<String>asList())).isEmpty(); assertThat(dao.selectGroupsByLogins(session, Arrays.<String>asList()).keys()).isEmpty();
assertThat(dao.countGroupsByLogins(session, Arrays.asList("two-hundred"))) Multimap<String, String> groupsByLogin = dao.selectGroupsByLogins(session, Arrays.asList("two-hundred", "two-hundred-one", "two-hundred-two"));
.containsExactly(entry("two-hundred", 3)); assertThat(groupsByLogin.get("two-hundred")).containsOnly("sonar-administrators", "sonar-users", "sonar-reviewers");
assertThat(dao.countGroupsByLogins(session, Arrays.asList("two-hundred", "two-hundred-one"))) assertThat(groupsByLogin.get("two-hundred-one")).containsOnly("sonar-users");
.containsOnly(entry("two-hundred", 3), entry("two-hundred-one", 1)); assertThat(groupsByLogin.get("two-hundred-two")).isEmpty();
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));
} finally { } finally {
session.close(); session.close();
} }
Expand Down

0 comments on commit 63efb09

Please sign in to comment.