From: Sébastien Lesaint Date: Mon, 23 Jan 2017 08:46:54 +0000 (+0100) Subject: SONAR-8682 fix error when filtering on name X-Git-Tag: 6.3-RC1~472 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=refs%2Fpull%2F1548%2Fhead;p=sonarqube.git SONAR-8682 fix error when filtering on name and a given group name exists in multiple organizations --- diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/template/TemplateGroupsAction.java b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/template/TemplateGroupsAction.java index 0f3b354b909..637417db7c6 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/template/TemplateGroupsAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/template/TemplateGroupsAction.java @@ -94,7 +94,7 @@ public class TemplateGroupsAction implements PermissionsWsAction { checkGlobalAdmin(userSession, template.getOrganizationUuid()); PermissionQuery query = buildPermissionQuery(wsRequest); - int total = dbClient.permissionTemplateDao().countGroupNamesByQueryAndTemplate(dbSession, query, template.getId()); + int total = dbClient.permissionTemplateDao().countGroupNamesByQueryAndTemplate(dbSession, query, template.getOrganizationUuid(), template.getId()); Paging paging = Paging.forPageIndex(wsRequest.mandatoryParamAsInt(PAGE)).withPageSize(wsRequest.mandatoryParamAsInt(PAGE_SIZE)).andTotal(total); List groups = findGroups(dbSession, query, template); List groupPermissions = findGroupPermissions(dbSession, groups, template); @@ -140,7 +140,7 @@ public class TemplateGroupsAction implements PermissionsWsAction { } private List findGroups(DbSession dbSession, PermissionQuery dbQuery, PermissionTemplateDto template) { - List orderedNames = dbClient.permissionTemplateDao().selectGroupNamesByQueryAndTemplate(dbSession, dbQuery, template.getId()); + List orderedNames = dbClient.permissionTemplateDao().selectGroupNamesByQueryAndTemplate(dbSession, dbQuery, template.getOrganizationUuid(), template.getId()); List groups = dbClient.groupDao().selectByNames(dbSession, template.getOrganizationUuid(), orderedNames); if (orderedNames.contains(DefaultGroups.ANYONE)) { groups.add(0, new GroupDto().setId(0L).setName(DefaultGroups.ANYONE)); diff --git a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/AddGroupToTemplateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/AddGroupToTemplateActionTest.java index 89d0d44e032..41c1947e548 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/AddGroupToTemplateActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/AddGroupToTemplateActionTest.java @@ -67,7 +67,7 @@ public class AddGroupToTemplateActionTest extends BasePermissionWsTest getGroupNamesInTemplateAndPermission(long templateId, String permission) { + private List getGroupNamesInTemplateAndPermission(PermissionTemplateDto template, String permission) { PermissionQuery query = PermissionQuery.builder().setPermission(permission).build(); return db.getDbClient().permissionTemplateDao() - .selectGroupNamesByQueryAndTemplate(db.getSession(), query, templateId); + .selectGroupNamesByQueryAndTemplate(db.getSession(), query, template.getOrganizationUuid(), template.getId()); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/RemoveGroupFromTemplateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/RemoveGroupFromTemplateActionTest.java index d1c018216b1..4141a091c34 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/RemoveGroupFromTemplateActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/RemoveGroupFromTemplateActionTest.java @@ -69,7 +69,7 @@ public class RemoveGroupFromTemplateActionTest extends BasePermissionWsTest getGroupNamesInTemplateAndPermission(long templateId, String permission) { + private List getGroupNamesInTemplateAndPermission(PermissionTemplateDto template, String permission) { PermissionQuery permissionQuery = PermissionQuery.builder().setPermission(permission).build(); return db.getDbClient().permissionTemplateDao() - .selectGroupNamesByQueryAndTemplate(db.getSession(), permissionQuery, templateId); + .selectGroupNamesByQueryAndTemplate(db.getSession(), permissionQuery, template.getOrganizationUuid(), template.getId()); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/TemplateGroupsActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/TemplateGroupsActionTest.java index 6419daa9f3c..42038876657 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/TemplateGroupsActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/TemplateGroupsActionTest.java @@ -83,6 +83,29 @@ public class TemplateGroupsActionTest extends BasePermissionWsTest selectGroupNamesByQueryAndTemplate(DbSession session, PermissionQuery query, long templateId) { - return mapper(session).selectGroupNamesByQueryAndTemplate(query, templateId, new RowBounds(query.getPageOffset(), query.getPageSize())); + public List selectGroupNamesByQueryAndTemplate(DbSession session, PermissionQuery query, String organizationUuid, long templateId) { + return mapper(session).selectGroupNamesByQueryAndTemplate(organizationUuid, templateId, query, new RowBounds(query.getPageOffset(), query.getPageSize())); } - public int countGroupNamesByQueryAndTemplate(DbSession session, PermissionQuery query, long templateId) { - return mapper(session).countGroupNamesByQueryAndTemplate(query, templateId); + public int countGroupNamesByQueryAndTemplate(DbSession session, PermissionQuery query, String organizationUuid, long templateId) { + return mapper(session).countGroupNamesByQueryAndTemplate(organizationUuid, query, templateId); } public List selectGroupPermissionsByTemplateIdAndGroupNames(DbSession dbSession, long templateId, List groups) { diff --git a/sonar-db/src/main/java/org/sonar/db/permission/template/PermissionTemplateMapper.java b/sonar-db/src/main/java/org/sonar/db/permission/template/PermissionTemplateMapper.java index 55b3d2569ec..2f58d738e66 100644 --- a/sonar-db/src/main/java/org/sonar/db/permission/template/PermissionTemplateMapper.java +++ b/sonar-db/src/main/java/org/sonar/db/permission/template/PermissionTemplateMapper.java @@ -47,7 +47,7 @@ public interface PermissionTemplateMapper { void deleteUserPermission(PermissionTemplateUserDto permissionTemplateUser); void deleteGroupPermissionsByTemplateId(long templateId); - + void deleteGroupPermissionsByTemplateIds(@Param("templateIds") List templateIds); void deleteGroupPermission(PermissionTemplateGroupDto permissionTemplateGroup); @@ -70,9 +70,10 @@ public interface PermissionTemplateMapper { int countUserLoginsByQueryAndTemplate(@Param("query") PermissionQuery query, @Param("templateId") long templateId); - List selectGroupNamesByQueryAndTemplate(@Param("query") PermissionQuery query, @Param("templateId") long templateId, RowBounds rowBounds); + List selectGroupNamesByQueryAndTemplate(@Param("organizationUuid") String organizationUuid, + @Param("templateId") long templateId, @Param("query") PermissionQuery query, RowBounds rowBounds); - int countGroupNamesByQueryAndTemplate(@Param("query") PermissionQuery query, @Param("templateId") long templateId); + int countGroupNamesByQueryAndTemplate(@Param("organizationUuid") String organizationUuid, @Param("query") PermissionQuery query, @Param("templateId") long templateId); List selectAll(@Param("organizationUuid") String organizationUuid, @Nullable @Param("upperCaseNameLikeSql") String upperCaseNameLikeSql); diff --git a/sonar-db/src/main/resources/org/sonar/db/permission/template/PermissionTemplateMapper.xml b/sonar-db/src/main/resources/org/sonar/db/permission/template/PermissionTemplateMapper.xml index 0001f3e18f4..fa79c9a5add 100644 --- a/sonar-db/src/main/resources/org/sonar/db/permission/template/PermissionTemplateMapper.xml +++ b/sonar-db/src/main/resources/org/sonar/db/permission/template/PermissionTemplateMapper.xml @@ -157,11 +157,22 @@ FROM - (SELECT g.id AS group_id, g.name AS name, ptg.permission_reference AS permission, ptg.template_id AS templateId + (SELECT + g.id AS group_id, + g.name AS name, + ptg.permission_reference AS permission, + ptg.template_id AS templateId FROM groups g - LEFT JOIN perm_templates_groups ptg ON ptg.group_id=g.id + LEFT JOIN perm_templates_groups ptg ON + ptg.group_id=g.id + where + g.organization_uuid=#{organizationUuid,jdbcType=VARCHAR} UNION ALL - SELECT 0 AS group_id, 'Anyone' AS name, ptg.permission_reference AS permission, ptg.template_id AS templateId + SELECT + 0 AS group_id, + 'Anyone' AS name, + ptg.permission_reference AS permission, + ptg.template_id AS templateId FROM perm_templates_groups ptg @@ -259,8 +270,9 @@ g.name AS groupName, ptg.created_at as createdAt, ptg.updated_at as updatedAt - FROM perm_templates_groups ptg - INNER JOIN groups g ON g.id=ptg.group_id + FROM perm_templates_groups ptg + INNER JOIN groups g ON + g.id=ptg.group_id UNION ALL SELECT ptg.id, diff --git a/sonar-db/src/test/java/org/sonar/db/permission/template/GroupWithPermissionTemplateDaoTest.java b/sonar-db/src/test/java/org/sonar/db/permission/template/GroupWithPermissionTemplateDaoTest.java index 6cf5abc3adc..1c197714719 100644 --- a/sonar-db/src/test/java/org/sonar/db/permission/template/GroupWithPermissionTemplateDaoTest.java +++ b/sonar-db/src/test/java/org/sonar/db/permission/template/GroupWithPermissionTemplateDaoTest.java @@ -27,6 +27,7 @@ import org.junit.Test; import org.sonar.api.utils.System2; import org.sonar.db.DbSession; import org.sonar.db.DbTester; +import org.sonar.db.organization.OrganizationDto; import org.sonar.db.permission.PermissionQuery; import org.sonar.db.user.GroupDto; @@ -43,95 +44,122 @@ public class GroupWithPermissionTemplateDaoTest { @Rule public DbTester db = DbTester.create(System2.INSTANCE); + private DbSession session = db.getSession(); private PermissionTemplateDbTester permissionTemplateDbTester = db.permissionTemplates(); private PermissionTemplateDao underTest = db.getDbClient().permissionTemplateDao(); @Test public void select_group_names_by_query_and_template() { - GroupDto group1 = db.users().insertGroup(newGroupDto().setName("Group-1")); - GroupDto group2 = db.users().insertGroup(newGroupDto().setName("Group-2")); - GroupDto group3 = db.users().insertGroup(newGroupDto().setName("Group-3")); + OrganizationDto organization = db.organizations().insert(); + GroupDto group1 = db.users().insertGroup(organization, "Group-1"); + GroupDto group2 = db.users().insertGroup(organization, "Group-2"); + GroupDto group3 = db.users().insertGroup(organization, "Group-3"); - PermissionTemplateDto template = permissionTemplateDbTester.insertTemplate(); + PermissionTemplateDto template = permissionTemplateDbTester.insertTemplate(organization); permissionTemplateDbTester.addGroupToTemplate(template.getId(), group1.getId(), USER); permissionTemplateDbTester.addGroupToTemplate(template.getId(), group1.getId(), ADMIN); permissionTemplateDbTester.addGroupToTemplate(template.getId(), group2.getId(), PROVISIONING); - PermissionTemplateDto anotherTemplate = permissionTemplateDbTester.insertTemplate(); + PermissionTemplateDto anotherTemplate = permissionTemplateDbTester.insertTemplate(organization); permissionTemplateDbTester.addGroupToTemplate(anotherTemplate.getId(), null, USER); permissionTemplateDbTester.addGroupToTemplate(anotherTemplate.getId(), group1.getId(), PROVISIONING); - assertThat(selectGroupNamesByQueryAndTemplate(builder().build(), template.getId())).containsOnly("Group-1", "Group-2", "Group-3", "Anyone"); - assertThat(selectGroupNamesByQueryAndTemplate(builder().withAtLeastOnePermission().build(), template.getId())).containsOnly("Group-1", "Group-2"); - assertThat(selectGroupNamesByQueryAndTemplate(builder().setPermission(USER).build(), template.getId())).containsOnly("Group-1"); - assertThat(selectGroupNamesByQueryAndTemplate(builder().setPermission(USER).build(), anotherTemplate.getId())).containsOnly("Anyone"); - assertThat(selectGroupNamesByQueryAndTemplate(builder().setSearchQuery("groU").build(), template.getId())).containsOnly("Group-1", "Group-2", "Group-3"); - assertThat(selectGroupNamesByQueryAndTemplate(builder().setSearchQuery("nYo").build(), template.getId())).containsOnly("Anyone"); - assertThat(selectGroupNamesByQueryAndTemplate(builder().setSearchQuery("p-2").build(), template.getId())).containsOnly("Group-2"); - - assertThat(selectGroupNamesByQueryAndTemplate(builder().withAtLeastOnePermission().build(), 123L)).isEmpty(); - assertThat(selectGroupNamesByQueryAndTemplate(builder().setSearchQuery("unknown").build(), template.getId())).isEmpty(); + assertThat(selectGroupNamesByQueryAndTemplate(builder(), organization, template)) + .containsOnly("Group-1", "Group-2", "Group-3", "Anyone"); + assertThat(selectGroupNamesByQueryAndTemplate(builder().withAtLeastOnePermission(), organization, template)) + .containsOnly("Group-1", "Group-2"); + assertThat(selectGroupNamesByQueryAndTemplate(builder().setPermission(USER), organization, template)) + .containsOnly("Group-1"); + assertThat(selectGroupNamesByQueryAndTemplate(builder().setPermission(USER), organization, anotherTemplate)) + .containsOnly("Anyone"); + assertThat(selectGroupNamesByQueryAndTemplate(builder().setSearchQuery("groU"), organization, template)) + .containsOnly("Group-1", "Group-2", "Group-3"); + assertThat(selectGroupNamesByQueryAndTemplate(builder().setSearchQuery("nYo"), organization, template)) + .containsOnly("Anyone"); + assertThat(selectGroupNamesByQueryAndTemplate(builder().setSearchQuery("p-2"), organization, template)) + .containsOnly("Group-2"); + + assertThat(selectGroupNamesByQueryAndTemplate(builder().withAtLeastOnePermission().build(), organization, 123L)) + .isEmpty(); + assertThat(selectGroupNamesByQueryAndTemplate(builder().setSearchQuery("unknown"), organization, template)) + .isEmpty(); } @Test public void select_group_names_by_query_and_template_is_ordered_by_group_names() { - GroupDto group2 = db.users().insertGroup(newGroupDto().setName("Group-2")); - db.users().insertGroup(newGroupDto().setName("Group-3")); - db.users().insertGroup(newGroupDto().setName("Group-1")); + OrganizationDto organization = db.organizations().insert(); + GroupDto group2 = db.users().insertGroup(organization, "Group-2"); + db.users().insertGroup(organization, "Group-3"); + db.users().insertGroup(organization, "Group-1"); - PermissionTemplateDto template = permissionTemplateDbTester.insertTemplate(); + PermissionTemplateDto template = permissionTemplateDbTester.insertTemplate(organization); permissionTemplateDbTester.addGroupToTemplate(template.getId(), group2.getId(), USER); - assertThat(selectGroupNamesByQueryAndTemplate(builder().build(), template.getId())).containsExactly("Anyone", "Group-1", "Group-2", "Group-3"); + assertThat(selectGroupNamesByQueryAndTemplate(builder(), organization, template)) + .containsExactly("Anyone", "Group-1", "Group-2", "Group-3"); } @Test public void select_group_names_by_query_and_template_is_paginated() { - IntStream.rangeClosed(0, 9).forEach(i -> db.users().insertGroup(newGroupDto().setName(i + "-name"))); + OrganizationDto organization = db.organizations().insert(); + IntStream.rangeClosed(0, 9).forEach(i -> db.users().insertGroup(organization, i + "-name")); - PermissionTemplateDto template = permissionTemplateDbTester.insertTemplate(); + PermissionTemplateDto template = permissionTemplateDbTester.insertTemplate(organization); - assertThat(selectGroupNamesByQueryAndTemplate(builder().setPageIndex(1).setPageSize(1).build(), template.getId())).containsExactly("0-name"); - assertThat(selectGroupNamesByQueryAndTemplate(builder().setPageIndex(2).setPageSize(3).build(), template.getId())).containsExactly("3-name", "4-name", "5-name"); + assertThat(selectGroupNamesByQueryAndTemplate(builder().setPageIndex(1).setPageSize(1), organization, template)) + .containsExactly("0-name"); + assertThat(selectGroupNamesByQueryAndTemplate(builder().setPageIndex(2).setPageSize(3), organization, template)) + .containsExactly("3-name", "4-name", "5-name"); } @Test public void select_group_names_by_query_and_template_returns_anyone() { - PermissionTemplateDto template = permissionTemplateDbTester.insertTemplate(); + OrganizationDto organization = db.organizations().insert(); + PermissionTemplateDto template = permissionTemplateDbTester.insertTemplate(organization); GroupDto group = db.users().insertGroup(newGroupDto().setName("Group")); - PermissionTemplateDto otherTemplate = permissionTemplateDbTester.insertTemplate(); + PermissionTemplateDto otherTemplate = permissionTemplateDbTester.insertTemplate(organization); permissionTemplateDbTester.addGroupToTemplate(otherTemplate.getId(), group.getId(), USER); - assertThat(selectGroupNamesByQueryAndTemplate(builder().setSearchQuery("nyo").build(), template.getId())).containsExactly("Anyone"); + assertThat(selectGroupNamesByQueryAndTemplate(builder().setSearchQuery("nyo"), organization, template)) + .containsExactly("Anyone"); } @Test public void count_group_names_by_query_and_template() { - GroupDto group1 = db.users().insertGroup(newGroupDto().setName("Group-1")); - GroupDto group2 = db.users().insertGroup(newGroupDto().setName("Group-2")); - GroupDto group3 = db.users().insertGroup(newGroupDto().setName("Group-3")); + OrganizationDto organization = db.organizations().insert(); + GroupDto group1 = db.users().insertGroup(organization, "Group-1"); + GroupDto group2 = db.users().insertGroup(organization, "Group-2"); + GroupDto group3 = db.users().insertGroup(organization, "Group-3"); - PermissionTemplateDto template = permissionTemplateDbTester.insertTemplate(); + PermissionTemplateDto template = permissionTemplateDbTester.insertTemplate(organization); permissionTemplateDbTester.addGroupToTemplate(template.getId(), group1.getId(), USER); permissionTemplateDbTester.addGroupToTemplate(template.getId(), group1.getId(), ADMIN); permissionTemplateDbTester.addGroupToTemplate(template.getId(), group2.getId(), PROVISIONING); - PermissionTemplateDto anotherTemplate = permissionTemplateDbTester.insertTemplate(); + PermissionTemplateDto anotherTemplate = permissionTemplateDbTester.insertTemplate(organization); permissionTemplateDbTester.addGroupToTemplate(anotherTemplate.getId(), null, USER); permissionTemplateDbTester.addGroupToTemplate(anotherTemplate.getId(), group1.getId(), PROVISIONING); - assertThat(countGroupNamesByQueryAndTemplate(builder().build(), template.getId())).isEqualTo(4); - assertThat(countGroupNamesByQueryAndTemplate(builder().withAtLeastOnePermission().build(), template.getId())).isEqualTo(2); - assertThat(countGroupNamesByQueryAndTemplate(builder().setPermission(USER).build(), template.getId())).isEqualTo(1); - assertThat(countGroupNamesByQueryAndTemplate(builder().setPermission(USER).build(), anotherTemplate.getId())).isEqualTo(1); - assertThat(countGroupNamesByQueryAndTemplate(builder().setSearchQuery("groU").build(), template.getId())).isEqualTo(3); - assertThat(countGroupNamesByQueryAndTemplate(builder().setSearchQuery("nYo").build(), template.getId())).isEqualTo(1); - assertThat(countGroupNamesByQueryAndTemplate(builder().setSearchQuery("p-2").build(), template.getId())).isEqualTo(1); - - assertThat(countGroupNamesByQueryAndTemplate(builder().withAtLeastOnePermission().build(), 123L)).isZero(); - assertThat(countGroupNamesByQueryAndTemplate(builder().setSearchQuery("unknown").build(), template.getId())).isZero(); + assertThat(countGroupNamesByQueryAndTemplate(builder(), organization, template)) + .isEqualTo(4); + assertThat(countGroupNamesByQueryAndTemplate(builder().withAtLeastOnePermission(), organization, template)) + .isEqualTo(2); + assertThat(countGroupNamesByQueryAndTemplate(builder().setPermission(USER), organization, template)).isEqualTo(1); + assertThat(countGroupNamesByQueryAndTemplate(builder().setPermission(USER), organization, anotherTemplate)) + .isEqualTo(1); + assertThat(countGroupNamesByQueryAndTemplate(builder().setSearchQuery("groU"), organization, template)) + .isEqualTo(3); + assertThat(countGroupNamesByQueryAndTemplate(builder().setSearchQuery("nYo"), organization, template)) + .isEqualTo(1); + assertThat(countGroupNamesByQueryAndTemplate(builder().setSearchQuery("p-2"), organization, template)) + .isEqualTo(1); + + assertThat(countGroupNamesByQueryAndTemplate(builder().withAtLeastOnePermission().build(), organization, 123L)) + .isZero(); + assertThat(countGroupNamesByQueryAndTemplate(builder().setSearchQuery("unknown"), organization, template)) + .isZero(); } @Test @@ -200,12 +228,20 @@ public class GroupWithPermissionTemplateDaoTest { assertThat(underTest.selectGroupPermissionsByTemplateId(session, 321L)).isEmpty(); } - private List selectGroupNamesByQueryAndTemplate(PermissionQuery query, long templateId) { - return underTest.selectGroupNamesByQueryAndTemplate(session, query, templateId); + private List selectGroupNamesByQueryAndTemplate(PermissionQuery.Builder queryBuilder, OrganizationDto organization, PermissionTemplateDto permissionTemplateDto) { + return selectGroupNamesByQueryAndTemplate(queryBuilder.build(), organization, permissionTemplateDto.getId()); + } + + private List selectGroupNamesByQueryAndTemplate(PermissionQuery query, OrganizationDto organization, long templateId) { + return underTest.selectGroupNamesByQueryAndTemplate(session, query, organization.getUuid(), templateId); + } + + private int countGroupNamesByQueryAndTemplate(PermissionQuery.Builder queryBuilder, OrganizationDto organization, PermissionTemplateDto permissionTemplateDto) { + return countGroupNamesByQueryAndTemplate(queryBuilder.build(), organization, permissionTemplateDto.getId()); } - private int countGroupNamesByQueryAndTemplate(PermissionQuery query, long templateId) { - return underTest.countGroupNamesByQueryAndTemplate(session, query, templateId); + private int countGroupNamesByQueryAndTemplate(PermissionQuery query, OrganizationDto organization, long templateId) { + return underTest.countGroupNamesByQueryAndTemplate(session, query, organization.getUuid(), templateId); } }