From 07dc8af3d50620da46e1f89bae319fa468b749f7 Mon Sep 17 00:00:00 2001 From: Teryk Bellahsene Date: Wed, 1 Feb 2017 18:46:37 +0100 Subject: [PATCH] SONAR-8059 WS api/user_groups/users searches users by name, login and email --- .../server/usergroups/ws/UsersAction.java | 58 +++++++++---------- .../server/usergroups/ws/UsersActionTest.java | 31 ++++++---- .../usergroups/ws/UsersActionTest/all.json | 4 +- .../ws/UsersActionTest/all_ada.json | 8 ++- .../sonar/db/user/UserMembershipQuery.java | 20 +++---- .../sonar/db/user/GroupMembershipMapper.xml | 6 +- .../sonar/db/user/GroupMembershipDaoTest.java | 19 +++--- .../shared_plus_empty_group.xml | 6 +- 8 files changed, 82 insertions(+), 70 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/UsersAction.java b/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/UsersAction.java index 9f74f59add6..126f2687178 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/UsersAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/UsersAction.java @@ -56,35 +56,6 @@ public class UsersAction implements UserGroupsWsAction { this.support = support; } - private static void writeMembers(JsonWriter json, List users) { - json.name("users").beginArray(); - for (UserMembershipDto user : users) { - json.beginObject() - .prop(FIELD_LOGIN, user.getLogin()) - .prop(FIELD_NAME, user.getName()) - .prop(FIELD_SELECTED, user.getGroupId() != null) - .endObject(); - } - json.endArray(); - } - - private static void writePaging(JsonWriter json, Paging paging) { - json.prop(Param.PAGE, paging.pageIndex()) - .prop(Param.PAGE_SIZE, paging.pageSize()) - .prop("total", paging.total()); - } - - private static String getMembership(String selected) { - SelectionMode selectionMode = SelectionMode.fromParam(selected); - String membership = GroupMembershipQuery.ANY; - if (SelectionMode.SELECTED == selectionMode) { - membership = GroupMembershipQuery.IN; - } else if (SelectionMode.DESELECTED == selectionMode) { - membership = GroupMembershipQuery.OUT; - } - return membership; - } - @Override public void define(NewController context) { NewAction action = context.createAction("users") @@ -128,4 +99,33 @@ public class UsersAction implements UserGroupsWsAction { json.endObject().close(); } } + + private static void writeMembers(JsonWriter json, List users) { + json.name("users").beginArray(); + for (UserMembershipDto user : users) { + json.beginObject() + .prop(FIELD_LOGIN, user.getLogin()) + .prop(FIELD_NAME, user.getName()) + .prop(FIELD_SELECTED, user.getGroupId() != null) + .endObject(); + } + json.endArray(); + } + + private static void writePaging(JsonWriter json, Paging paging) { + json.prop(Param.PAGE, paging.pageIndex()) + .prop(Param.PAGE_SIZE, paging.pageSize()) + .prop("total", paging.total()); + } + + private static String getMembership(String selected) { + SelectionMode selectionMode = SelectionMode.fromParam(selected); + String membership = GroupMembershipQuery.ANY; + if (SelectionMode.SELECTED == selectionMode) { + membership = GroupMembershipQuery.IN; + } else if (SelectionMode.DESELECTED == selectionMode) { + membership = GroupMembershipQuery.OUT; + } + return membership; + } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/UsersActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/UsersActionTest.java index 1adda704f6a..99e92aa08a5 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/UsersActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/UsersActionTest.java @@ -116,7 +116,7 @@ public class UsersActionTest { @Test public void return_members_by_group_id() throws Exception { GroupDto group = db.users().insertGroup(); - UserDto user1 = db.users().insertUser(newUserDto().setLogin("ada").setName("Ada Lovelace")); + UserDto user1 = db.users().insertUser(newUserDto().setLogin("ada.login").setName("Ada Lovelace")); db.users().insertMember(group, user1); db.users().insertUser(newUserDto().setLogin("grace").setName("Grace Hopper")); loginAsAdminOnDefaultOrganization(); @@ -132,7 +132,7 @@ public class UsersActionTest { public void references_group_by_its_name() throws Exception { OrganizationDto org = db.organizations().insert(); GroupDto group = db.users().insertGroup(org, "the-group"); - UserDto user1 = db.users().insertUser(newUserDto().setLogin("ada").setName("Ada Lovelace")); + UserDto user1 = db.users().insertUser(newUserDto().setLogin("ada.login").setName("Ada Lovelace")); db.users().insertMember(group, user1); db.users().insertUser(newUserDto().setLogin("grace").setName("Grace Hopper")); loginAsAdmin(org); @@ -148,7 +148,7 @@ public class UsersActionTest { @Test public void references_group_in_default_organization_by_its_name() throws Exception { GroupDto group = db.users().insertGroup(); - UserDto user1 = db.users().insertUser(newUserDto().setLogin("ada").setName("Ada Lovelace")); + UserDto user1 = db.users().insertUser(newUserDto().setLogin("ada.login").setName("Ada Lovelace")); db.users().insertMember(group, user1); db.users().insertUser(newUserDto().setLogin("grace").setName("Grace Hopper")); loginAsAdminOnDefaultOrganization(); @@ -169,9 +169,7 @@ public class UsersActionTest { db.users().insertMember(group, graceHopper); loginAsAdminOnDefaultOrganization(); - String response = newUsersRequest() - .setParam(PARAM_GROUP_ID, group.getId().toString()) - .execute().outputAsString(); + String response = newUsersRequest().setParam(PARAM_GROUP_ID, group.getId().toString()).execute().outputAsString(); assertThat(response).contains("Ada Lovelace", "Grace Hopper"); } @@ -236,10 +234,10 @@ public class UsersActionTest { } @Test - public void filtering() throws Exception { + public void filtering_by_name_email_and_login() throws Exception { GroupDto group = db.users().insertGroup(); - UserDto user1 = db.users().insertUser(newUserDto().setLogin("ada").setName("Ada Lovelace")); - db.users().insertUser(newUserDto().setLogin("grace").setName("Grace Hopper")); + UserDto user1 = db.users().insertUser(newUserDto().setLogin("ada.login").setName("Ada Lovelace").setEmail("ada@email.com")); + db.users().insertUser(newUserDto().setLogin("grace").setName("Grace Hopper").setEmail("grace@hopper.com")); db.users().insertMember(group, user1); loginAsAdminOnDefaultOrganization(); @@ -250,9 +248,18 @@ public class UsersActionTest { .execute() .assertJson(getClass(), "all.json"); - newUsersRequest() - .setParam("id", group.getId().toString()) - .setParam("q", "love") + newUsersRequest().setParam("id", group.getId().toString()) + .setParam("q", ".logi") + .execute() + .assertJson(getClass(), "all_ada.json"); + + newUsersRequest().setParam("id", group.getId().toString()) + .setParam("q", "OvE") + .execute() + .assertJson(getClass(), "all_ada.json"); + + newUsersRequest().setParam("id", group.getId().toString()) + .setParam("q", "mail") .execute() .assertJson(getClass(), "all_ada.json"); } diff --git a/server/sonar-server/src/test/resources/org/sonar/server/usergroups/ws/UsersActionTest/all.json b/server/sonar-server/src/test/resources/org/sonar/server/usergroups/ws/UsersActionTest/all.json index 8f0e903fdb9..b0d9576da28 100644 --- a/server/sonar-server/src/test/resources/org/sonar/server/usergroups/ws/UsersActionTest/all.json +++ b/server/sonar-server/src/test/resources/org/sonar/server/usergroups/ws/UsersActionTest/all.json @@ -2,7 +2,7 @@ "p": 1, "total": 2, "users": [ - {"login": "ada", "name": "Ada Lovelace", "selected": true}, + {"login": "ada.login", "name": "Ada Lovelace", "selected": true}, {"login": "grace", "name": "Grace Hopper", "selected": false} ] -} \ No newline at end of file +} diff --git a/server/sonar-server/src/test/resources/org/sonar/server/usergroups/ws/UsersActionTest/all_ada.json b/server/sonar-server/src/test/resources/org/sonar/server/usergroups/ws/UsersActionTest/all_ada.json index cb5b2a66035..802c557b1fa 100644 --- a/server/sonar-server/src/test/resources/org/sonar/server/usergroups/ws/UsersActionTest/all_ada.json +++ b/server/sonar-server/src/test/resources/org/sonar/server/usergroups/ws/UsersActionTest/all_ada.json @@ -2,6 +2,10 @@ "p": 1, "total": 1, "users": [ - {"login": "ada", "name": "Ada Lovelace", "selected": true} + { + "login": "ada.login", + "name": "Ada Lovelace", + "selected": true + } ] -} \ No newline at end of file +} diff --git a/sonar-db/src/main/java/org/sonar/db/user/UserMembershipQuery.java b/sonar-db/src/main/java/org/sonar/db/user/UserMembershipQuery.java index 7847fe265c4..7bcb8920a96 100644 --- a/sonar-db/src/main/java/org/sonar/db/user/UserMembershipQuery.java +++ b/sonar-db/src/main/java/org/sonar/db/user/UserMembershipQuery.java @@ -20,6 +20,7 @@ package org.sonar.db.user; import com.google.common.collect.ImmutableSet; +import java.util.Locale; import java.util.Set; import javax.annotation.CheckForNull; import javax.annotation.Nullable; @@ -28,6 +29,8 @@ import org.apache.commons.lang.StringUtils; import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; +import static org.sonar.db.DatabaseUtils.buildLikeValue; +import static org.sonar.db.WildcardPosition.BEFORE_AND_AFTER; public class UserMembershipQuery { @@ -46,6 +49,7 @@ public class UserMembershipQuery { // for internal use in MyBatis final String memberSearchSql; + final String memberSearchSqlLowercase; // max results per page private final int pageSize; @@ -57,22 +61,12 @@ public class UserMembershipQuery { this.groupId = builder.groupId; this.membership = builder.membership; this.memberSearch = builder.memberSearch; - this.memberSearchSql = memberSearchToSql(memberSearch); - + this.memberSearchSql = memberSearch == null ? null : buildLikeValue(memberSearch, BEFORE_AND_AFTER); + this.memberSearchSqlLowercase = memberSearchSql == null ? null : memberSearchSql.toLowerCase(Locale.ENGLISH); this.pageSize = builder.pageSize; this.pageIndex = builder.pageIndex; } - private String memberSearchToSql(@Nullable String s) { - String sql = null; - if (s != null) { - sql = StringUtils.replace(StringUtils.upperCase(s), "%", "/%"); - sql = StringUtils.replace(sql, "_", "/_"); - sql = "%" + sql + "%"; - } - return sql; - } - public long groupId() { return groupId; } @@ -83,7 +77,7 @@ public class UserMembershipQuery { } /** - * Search for users names/logins containing a given string + * Search for users email, login and name containing a given string */ @CheckForNull public String memberSearch() { diff --git a/sonar-db/src/main/resources/org/sonar/db/user/GroupMembershipMapper.xml b/sonar-db/src/main/resources/org/sonar/db/user/GroupMembershipMapper.xml index 73d7ee7fa54..12ba914eeed 100644 --- a/sonar-db/src/main/resources/org/sonar/db/user/GroupMembershipMapper.xml +++ b/sonar-db/src/main/resources/org/sonar/db/user/GroupMembershipMapper.xml @@ -72,8 +72,10 @@ - AND ((UPPER(u.login) LIKE #{query.memberSearchSql} ESCAPE '/') OR (UPPER(u.name) LIKE #{query.memberSearchSql} - ESCAPE '/')) + AND ( + lower(u.name) like #{query.memberSearchSqlLowercase} ESCAPE '/' + or u.login like #{query.memberSearchSql} ESCAPE '/' + or u.email like #{query.memberSearchSql} ESCAPE '/' ) AND u.active=${_true} diff --git a/sonar-db/src/test/java/org/sonar/db/user/GroupMembershipDaoTest.java b/sonar-db/src/test/java/org/sonar/db/user/GroupMembershipDaoTest.java index d2842e9967f..aa144a47916 100644 --- a/sonar-db/src/test/java/org/sonar/db/user/GroupMembershipDaoTest.java +++ b/sonar-db/src/test/java/org/sonar/db/user/GroupMembershipDaoTest.java @@ -126,7 +126,7 @@ public class GroupMembershipDaoTest { List result = underTest.selectMembers(dbTester.getSession(), UserMembershipQuery.builder().groupId(100L).memberSearch("admin").build(), 0, 10); assertThat(result).hasSize(2); - assertThat(result.get(0).getName()).isEqualTo("Admin"); + assertThat(result.get(0).getName()).isEqualTo("Admin name"); assertThat(result.get(1).getName()).isEqualTo("Not Admin"); result = underTest.selectMembers(dbTester.getSession(), UserMembershipQuery.builder().groupId(100L).memberSearch("not").build(), 0, 10); @@ -134,15 +134,18 @@ public class GroupMembershipDaoTest { } @Test - public void search_by_login_or_name_with_capitalization() { + public void search_by_login_name_or_email() { dbTester.prepareDbUnit(getClass(), "shared_plus_empty_group.xml"); - List result = underTest.selectMembers(dbTester.getSession(), UserMembershipQuery.builder().groupId(100L).memberSearch("admin").build(), 0, 10); - assertThat(result).hasSize(2); + // search is case insensitive only on name + List result = underTest.selectMembers(dbTester.getSession(), UserMembershipQuery.builder().groupId(100L).memberSearch("NaMe").build(), 0, 10); + assertThat(result).hasSize(1); - result = underTest.selectMembers(dbTester.getSession(), UserMembershipQuery.builder().groupId(100L).memberSearch("AdMiN").build(), 0, 10); - assertThat(result).hasSize(2); + result = underTest.selectMembers(dbTester.getSession(), UserMembershipQuery.builder().groupId(100L).memberSearch("login").build(), 0, 10); + assertThat(result).hasSize(1); + result = underTest.selectMembers(dbTester.getSession(), UserMembershipQuery.builder().groupId(100L).memberSearch("email").build(), 0, 10); + assertThat(result).hasSize(1); } @Test @@ -151,7 +154,7 @@ public class GroupMembershipDaoTest { List result = underTest.selectMembers(dbTester.getSession(), UserMembershipQuery.builder().groupId(100L).build(), 0, 10); assertThat(result).hasSize(2); - assertThat(result.get(0).getName()).isEqualTo("Admin"); + assertThat(result.get(0).getName()).isEqualTo("Admin name"); assertThat(result.get(1).getName()).isEqualTo("Not Admin"); } @@ -161,7 +164,7 @@ public class GroupMembershipDaoTest { List result = underTest.selectMembers(dbTester.getSession(), UserMembershipQuery.builder().groupId(100L).build(), 0, 2); assertThat(result).hasSize(2); - assertThat(result.get(0).getName()).isEqualTo("Admin"); + assertThat(result.get(0).getName()).isEqualTo("Admin name"); assertThat(result.get(1).getName()).isEqualTo("Not Admin"); result = underTest.selectMembers(dbTester.getSession(), UserMembershipQuery.builder().groupId(100L).build(), 1, 2); diff --git a/sonar-db/src/test/resources/org/sonar/db/user/GroupMembershipDaoTest/shared_plus_empty_group.xml b/sonar-db/src/test/resources/org/sonar/db/user/GroupMembershipDaoTest/shared_plus_empty_group.xml index 3d65a44cbad..a475bba797f 100644 --- a/sonar-db/src/test/resources/org/sonar/db/user/GroupMembershipDaoTest/shared_plus_empty_group.xml +++ b/sonar-db/src/test/resources/org/sonar/db/user/GroupMembershipDaoTest/shared_plus_empty_group.xml @@ -30,13 +30,15 @@ group_id="101"/>