From 9d6442856e3b66df0c02831c8108780d4d738bfe Mon Sep 17 00:00:00 2001 From: Teryk Bellahsene Date: Wed, 23 Dec 2015 10:47:42 +0100 Subject: [PATCH] SONAR-7163 WS api/user_groups/search Search group with '_', '%' or '/' characters --- .../server/usergroups/ws/SearchAction.java | 3 +- .../usergroups/ws/SearchActionTest.java | 8 +- .../ws/SearchActionTest/customers.json | 6 +- .../main/java/org/sonar/db/DatabaseUtils.java | 3 - .../main/java/org/sonar/db/user/GroupDao.java | 19 +-- .../java/org/sonar/db/user/GroupMapper.java | 5 +- .../org/sonar/db/user/GroupMapper.xml | 12 +- .../java/org/sonar/db/user/GroupDaoTest.java | 114 +++++++++--------- 8 files changed, 90 insertions(+), 80 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/SearchAction.java b/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/SearchAction.java index 5990ddc88b4..8862e36dda6 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/SearchAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/SearchAction.java @@ -36,7 +36,6 @@ import org.sonar.api.server.ws.WebService.Param; import org.sonar.api.utils.text.JsonWriter; import org.sonar.db.DbClient; import org.sonar.db.DbSession; -import org.sonar.db.MyBatis; import org.sonar.db.user.GroupDto; import org.sonar.server.es.SearchOptions; @@ -95,7 +94,7 @@ public class SearchAction implements UserGroupsWsAction { writeGroups(json, groups, userCountByGroup, fields); json.endObject().close(); } finally { - MyBatis.closeQuietly(dbSession); + dbClient.closeSession(dbSession); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/SearchActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/SearchActionTest.java index d6735607ff6..acd501b9cd0 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/SearchActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/SearchActionTest.java @@ -31,7 +31,6 @@ import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.DbTester; import org.sonar.db.user.GroupDao; -import org.sonar.db.user.GroupDto; import org.sonar.db.user.GroupMembershipDao; import org.sonar.db.user.UserGroupDao; import org.sonar.db.user.UserGroupDto; @@ -39,6 +38,7 @@ import org.sonar.server.ws.WsTester; import org.sonar.test.DbTests; import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.db.user.GroupTesting.newGroupDto; @Category(DbTests.class) public class SearchActionTest { @@ -90,10 +90,10 @@ public class SearchActionTest { @Test public void search_with_query() throws Exception { - insertGroups("users", "admins", "customer1", "customer2", "customer3"); + insertGroups("users", "admins", "customer%_%/1", "customer%_%/2", "customer%_%/3"); dbSession.commit(); - newRequest().setParam(Param.TEXT_QUERY, "custom").execute().assertJson(getClass(), "customers.json"); + newRequest().setParam(Param.TEXT_QUERY, "tomer%_%/").execute().assertJson(getClass(), "customers.json"); } @Test @@ -151,7 +151,7 @@ public class SearchActionTest { private void insertGroups(String... groupNames) { for (String groupName : groupNames) { - groupDao.insert(dbSession, new GroupDto() + groupDao.insert(dbSession, newGroupDto() .setName(groupName) .setDescription(StringUtils.capitalize(groupName))); } diff --git a/server/sonar-server/src/test/resources/org/sonar/server/usergroups/ws/SearchActionTest/customers.json b/server/sonar-server/src/test/resources/org/sonar/server/usergroups/ws/SearchActionTest/customers.json index b94a0072af9..b3bf7246d98 100644 --- a/server/sonar-server/src/test/resources/org/sonar/server/usergroups/ws/SearchActionTest/customers.json +++ b/server/sonar-server/src/test/resources/org/sonar/server/usergroups/ws/SearchActionTest/customers.json @@ -3,8 +3,8 @@ "ps": 100, "total": 3, "groups": [ - {"name": "customer1", "description": "Customer1", "membersCount": 0}, - {"name": "customer2", "description": "Customer2", "membersCount": 0}, - {"name": "customer3", "description": "Customer3", "membersCount": 0} + {"name": "customer%_%/1", "description": "Customer%_%/1", "membersCount": 0}, + {"name": "customer%_%/2", "description": "Customer%_%/2", "membersCount": 0}, + {"name": "customer%_%/3", "description": "Customer%_%/3", "membersCount": 0} ] } diff --git a/sonar-db/src/main/java/org/sonar/db/DatabaseUtils.java b/sonar-db/src/main/java/org/sonar/db/DatabaseUtils.java index 13227b1781d..dcfdbdcbd4c 100644 --- a/sonar-db/src/main/java/org/sonar/db/DatabaseUtils.java +++ b/sonar-db/src/main/java/org/sonar/db/DatabaseUtils.java @@ -81,9 +81,6 @@ public class DatabaseUtils { * * You must add "ESCAPE '/'" after your like query. It defines '/' as the escape character. */ - /** - * - */ public static String buildLikeValue(String value, WildcardPosition wildcardPosition) { String escapedValue = escapePercentAndUnderscore(value); String wildcard = "%"; diff --git a/sonar-db/src/main/java/org/sonar/db/user/GroupDao.java b/sonar-db/src/main/java/org/sonar/db/user/GroupDao.java index 2b316fec8b0..0a0724da185 100644 --- a/sonar-db/src/main/java/org/sonar/db/user/GroupDao.java +++ b/sonar-db/src/main/java/org/sonar/db/user/GroupDao.java @@ -22,14 +22,17 @@ package org.sonar.db.user; import java.util.Date; import java.util.List; +import java.util.Locale; import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.apache.commons.lang.StringUtils; import org.apache.ibatis.session.RowBounds; import org.sonar.api.utils.System2; import org.sonar.db.Dao; +import org.sonar.db.DatabaseUtils; import org.sonar.db.DbSession; import org.sonar.db.RowNotFoundException; +import org.sonar.db.WildcardPosition; public class GroupDao implements Dao { @@ -92,18 +95,18 @@ public class GroupDao implements Dao { return item; } - public List selectByUserLogin(DbSession session, String login){ + public List selectByUserLogin(DbSession session, String login) { return mapper(session).selectByUserLogin(login); } - private String groupSearchToSql(@Nullable String query) { - String sql = SQL_WILDCARD; - if (query != null) { - sql = StringUtils.replace(StringUtils.upperCase(query), SQL_WILDCARD, "/%"); - sql = StringUtils.replace(sql, "_", "/_"); - sql = SQL_WILDCARD + sql + SQL_WILDCARD; + @CheckForNull + private static String groupSearchToSql(@Nullable String query) { + if (query == null) { + return null; } - return sql; + + String upperCasedNameQuery = StringUtils.upperCase(query, Locale.ENGLISH); + return DatabaseUtils.buildLikeValue(upperCasedNameQuery, WildcardPosition.BEFORE_AND_AFTER); } private GroupMapper mapper(DbSession session) { diff --git a/sonar-db/src/main/java/org/sonar/db/user/GroupMapper.java b/sonar-db/src/main/java/org/sonar/db/user/GroupMapper.java index d1cafb62fa9..bd7a7b52ddd 100644 --- a/sonar-db/src/main/java/org/sonar/db/user/GroupMapper.java +++ b/sonar-db/src/main/java/org/sonar/db/user/GroupMapper.java @@ -22,6 +22,7 @@ package org.sonar.db.user; import java.util.List; import javax.annotation.CheckForNull; +import org.apache.ibatis.annotations.Param; import org.apache.ibatis.session.RowBounds; public interface GroupMapper { @@ -38,9 +39,9 @@ public interface GroupMapper { void update(GroupDto item); - List selectByQuery(String query, RowBounds rowBounds); + List selectByQuery(@Param("query") String query, RowBounds rowBounds); - int countByQuery(String query); + int countByQuery(@Param("query") String query); void deleteById(long groupId); } diff --git a/sonar-db/src/main/resources/org/sonar/db/user/GroupMapper.xml b/sonar-db/src/main/resources/org/sonar/db/user/GroupMapper.xml index 0991bcffc0b..d641c3b9b57 100644 --- a/sonar-db/src/main/resources/org/sonar/db/user/GroupMapper.xml +++ b/sonar-db/src/main/resources/org/sonar/db/user/GroupMapper.xml @@ -65,13 +65,21 @@ SELECT FROM groups g - WHERE UPPER(g.name) LIKE #{query} + + + UPPER(g.name) LIKE #{query} ESCAPE '/' + + ORDER BY UPPER(g.name) diff --git a/sonar-db/src/test/java/org/sonar/db/user/GroupDaoTest.java b/sonar-db/src/test/java/org/sonar/db/user/GroupDaoTest.java index c09cd3ff4af..8e34ac6b6db 100644 --- a/sonar-db/src/test/java/org/sonar/db/user/GroupDaoTest.java +++ b/sonar-db/src/test/java/org/sonar/db/user/GroupDaoTest.java @@ -20,13 +20,13 @@ package org.sonar.db.user; -import org.junit.After; -import org.junit.Before; +import java.util.List; import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; import org.sonar.api.utils.DateUtils; import org.sonar.api.utils.System2; +import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.DbTester; import org.sonar.test.DbTests; @@ -34,35 +34,26 @@ import org.sonar.test.DbTests; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static org.sonar.db.user.GroupTesting.newGroupDto; @Category(DbTests.class) public class GroupDaoTest { @Rule - public DbTester dbTester = DbTester.create(System2.INSTANCE); - - GroupDao dao; - DbSession session; - System2 system2; - - @Before - public void setUp() { - dbTester.truncateTables(); - this.session = dbTester.myBatis().openSession(false); - this.system2 = mock(System2.class); - this.dao = new GroupDao(system2); - } + public DbTester db = DbTester.create(System2.INSTANCE); - @After - public void tearDown() { - session.close(); - } + DbSession dbSession = db.getSession(); + DbClient dbClient = db.getDbClient(); + System2 system2 = mock(System2.class); + + GroupDao underTest = new GroupDao(system2); @Test public void select_by_key() { - dbTester.prepareDbUnit(getClass(), "select_by_key.xml"); + db.prepareDbUnit(getClass(), "select_by_key.xml"); + + GroupDto group = underTest.selectOrFailByName(dbSession, "sonar-users"); - GroupDto group = new GroupDao(system2).selectOrFailByName(session, "sonar-users"); assertThat(group).isNotNull(); assertThat(group.getId()).isEqualTo(1L); assertThat(group.getName()).isEqualTo("sonar-users"); @@ -73,9 +64,10 @@ public class GroupDaoTest { @Test public void select_by_id() { - dbTester.prepareDbUnit(getClass(), "select_by_key.xml"); + db.prepareDbUnit(getClass(), "select_by_key.xml"); + + GroupDto group = underTest.selectOrFailById(dbSession, 1L); - GroupDto group = new GroupDao(system2).selectOrFailById(session, 1L); assertThat(group).isNotNull(); assertThat(group.getId()).isEqualTo(1L); assertThat(group.getName()).isEqualTo("sonar-users"); @@ -86,103 +78,113 @@ public class GroupDaoTest { @Test public void find_by_user_login() { - dbTester.prepareDbUnit(getClass(), "find_by_user_login.xml"); + db.prepareDbUnit(getClass(), "find_by_user_login.xml"); - assertThat(dao.selectByUserLogin(session, "john")).hasSize(2); - assertThat(dao.selectByUserLogin(session, "max")).isEmpty(); + assertThat(underTest.selectByUserLogin(dbSession, "john")).hasSize(2); + assertThat(underTest.selectByUserLogin(dbSession, "max")).isEmpty(); } @Test public void insert() { when(system2.now()).thenReturn(DateUtils.parseDate("2014-09-08").getTime()); - - dbTester.prepareDbUnit(getClass(), "empty.xml"); - + db.prepareDbUnit(getClass(), "empty.xml"); GroupDto dto = new GroupDto() .setId(1L) .setName("sonar-users") .setDescription("Sonar Users"); - dao.insert(session, dto); - session.commit(); + underTest.insert(dbSession, dto); + dbSession.commit(); - dbTester.assertDbUnit(getClass(), "insert-result.xml", "groups"); + db.assertDbUnit(getClass(), "insert-result.xml", "groups"); } @Test public void update() { when(system2.now()).thenReturn(DateUtils.parseDate("2013-07-25").getTime()); - - dbTester.prepareDbUnit(getClass(), "update.xml"); - + db.prepareDbUnit(getClass(), "update.xml"); GroupDto dto = new GroupDto() .setId(1L) .setName("new-name") .setDescription("New Description"); - dao.update(session, dto); - session.commit(); + underTest.update(dbSession, dto); + dbSession.commit(); - dbTester.assertDbUnit(getClass(), "update-result.xml", "groups"); + db.assertDbUnit(getClass(), "update-result.xml", "groups"); } @Test public void select_by_query() { - dbTester.prepareDbUnit(getClass(), "select_by_query.xml"); + db.prepareDbUnit(getClass(), "select_by_query.xml"); /* * Ordering and paging are not fully tested, case insensitive sort is broken on MySQL */ // Null query - assertThat(new GroupDao(system2).selectByQuery(session, null, 0, 10)) + assertThat(underTest.selectByQuery(dbSession, null, 0, 10)) .hasSize(5) .extracting("name").containsOnly("customers-group1", "customers-group2", "customers-group3", "SONAR-ADMINS", "sonar-users"); // Empty query - assertThat(new GroupDao(system2).selectByQuery(session, "", 0, 10)) + assertThat(underTest.selectByQuery(dbSession, "", 0, 10)) .hasSize(5) .extracting("name").containsOnly("customers-group1", "customers-group2", "customers-group3", "SONAR-ADMINS", "sonar-users"); // Filter on name - assertThat(new GroupDao(system2).selectByQuery(session, "sonar", 0, 10)) + assertThat(underTest.selectByQuery(dbSession, "sonar", 0, 10)) .hasSize(2) .extracting("name").containsOnly("SONAR-ADMINS", "sonar-users"); // Pagination - assertThat(new GroupDao(system2).selectByQuery(session, null, 0, 3)) + assertThat(underTest.selectByQuery(dbSession, null, 0, 3)) .hasSize(3); - assertThat(new GroupDao(system2).selectByQuery(session, null, 3, 3)) + assertThat(underTest.selectByQuery(dbSession, null, 3, 3)) .hasSize(2); - assertThat(new GroupDao(system2).selectByQuery(session, null, 6, 3)).isEmpty(); - assertThat(new GroupDao(system2).selectByQuery(session, null, 0, 5)) + assertThat(underTest.selectByQuery(dbSession, null, 6, 3)).isEmpty(); + assertThat(underTest.selectByQuery(dbSession, null, 0, 5)) .hasSize(5); - assertThat(new GroupDao(system2).selectByQuery(session, null, 5, 5)).isEmpty(); + assertThat(underTest.selectByQuery(dbSession, null, 5, 5)).isEmpty(); + } + + @Test + public void select_by_query_with_special_characters() { + String groupNameWithSpecialCharacters = "group%_%/name"; + underTest.insert(dbSession, newGroupDto().setName(groupNameWithSpecialCharacters)); + db.commit(); + + List result = underTest.selectByQuery(dbSession, "roup%_%/nam", 0, 10); + int resultCount = underTest.countByQuery(dbSession, "roup%_%/nam"); + + assertThat(result).hasSize(1); + assertThat(result.get(0).getName()).isEqualTo(groupNameWithSpecialCharacters); + assertThat(resultCount).isEqualTo(1); } @Test public void count_by_query() { - dbTester.prepareDbUnit(getClass(), "select_by_query.xml"); + db.prepareDbUnit(getClass(), "select_by_query.xml"); // Null query - assertThat(new GroupDao(system2).countByQuery(session, null)).isEqualTo(5); + assertThat(underTest.countByQuery(dbSession, null)).isEqualTo(5); // Empty query - assertThat(new GroupDao(system2).countByQuery(session, "")).isEqualTo(5); + assertThat(underTest.countByQuery(dbSession, "")).isEqualTo(5); // Filter on name - assertThat(new GroupDao(system2).countByQuery(session, "sonar")).isEqualTo(2); + assertThat(underTest.countByQuery(dbSession, "sonar")).isEqualTo(2); } @Test public void delete_by_id() { - dbTester.prepareDbUnit(getClass(), "select_by_key.xml"); + db.prepareDbUnit(getClass(), "select_by_key.xml"); - GroupDao groupDao = new GroupDao(system2); - groupDao.deleteById(session, 1L); - session.commit(); + GroupDao groupDao = underTest; + groupDao.deleteById(dbSession, 1L); + dbSession.commit(); - assertThat(groupDao.countByQuery(session, null)).isZero(); + assertThat(groupDao.countByQuery(dbSession, null)).isZero(); } } -- 2.39.5