From d53f84fda5ad7ac55c7593f20667334f17da1bbf Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Mon, 17 Oct 2016 16:45:35 +0200 Subject: [PATCH] SONAR-8227 Remove source from issue/authorization index --- .../issue/index/IssueIndexDefinition.java | 2 + .../index/AuthorizationIndexer.java | 37 ++++---- .../step/ApplyPermissionsStepTest.java | 26 ++---- .../index/AuthorizationIndexerTest.java | 41 +++------ .../index/AuthorizationIndexerTester.java | 85 +++++++++++++++++++ .../project/ws/BulkDeleteActionTest.java | 4 +- .../server/project/ws/DeleteActionTest.java | 3 +- 7 files changed, 128 insertions(+), 70 deletions(-) create mode 100644 server/sonar-server/src/test/java/org/sonar/server/permission/index/AuthorizationIndexerTester.java diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndexDefinition.java b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndexDefinition.java index bd37ad5738a..f07c6d54d5c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndexDefinition.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndexDefinition.java @@ -131,5 +131,7 @@ public class IssueIndexDefinition implements IndexDefinition { authorizationMapping.stringFieldBuilder(FIELD_AUTHORIZATION_PROJECT_UUID).disableNorms().build(); authorizationMapping.stringFieldBuilder(FIELD_AUTHORIZATION_GROUPS).disableNorms().build(); authorizationMapping.stringFieldBuilder(FIELD_AUTHORIZATION_USERS).disableNorms().build(); + // do not store document but only indexation of information + authorizationMapping.setEnableSource(false); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/index/AuthorizationIndexer.java b/server/sonar-server/src/main/java/org/sonar/server/permission/index/AuthorizationIndexer.java index 83a0870d609..e474ca96b11 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/permission/index/AuthorizationIndexer.java +++ b/server/sonar-server/src/main/java/org/sonar/server/permission/index/AuthorizationIndexer.java @@ -24,14 +24,20 @@ import com.google.common.collect.ImmutableMap; import java.util.Collection; import java.util.Date; import java.util.Map; -import org.elasticsearch.action.ActionRequest; -import org.elasticsearch.action.update.UpdateRequest; +import org.elasticsearch.action.index.IndexRequest; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.server.es.BaseIndexer; import org.sonar.server.es.BulkIndexer; import org.sonar.server.es.EsClient; -import org.sonar.server.issue.index.IssueIndexDefinition; + +import static org.sonar.server.issue.index.IssueIndexDefinition.FIELD_AUTHORIZATION_GROUPS; +import static org.sonar.server.issue.index.IssueIndexDefinition.FIELD_AUTHORIZATION_PROJECT_UUID; +import static org.sonar.server.issue.index.IssueIndexDefinition.FIELD_AUTHORIZATION_UPDATED_AT; +import static org.sonar.server.issue.index.IssueIndexDefinition.FIELD_AUTHORIZATION_USERS; +import static org.sonar.server.issue.index.IssueIndexDefinition.FIELD_ISSUE_TECHNICAL_UPDATED_AT; +import static org.sonar.server.issue.index.IssueIndexDefinition.INDEX; +import static org.sonar.server.issue.index.IssueIndexDefinition.TYPE_AUTHORIZATION; /** * Manages the synchronization of index issues/authorization with authorization settings defined in database : @@ -45,7 +51,7 @@ public class AuthorizationIndexer extends BaseIndexer { private final DbClient dbClient; public AuthorizationIndexer(DbClient dbClient, EsClient esClient) { - super(esClient, 0L, IssueIndexDefinition.INDEX, IssueIndexDefinition.TYPE_AUTHORIZATION, IssueIndexDefinition.FIELD_ISSUE_TECHNICAL_UPDATED_AT); + super(esClient, 0L, INDEX, TYPE_AUTHORIZATION, FIELD_ISSUE_TECHNICAL_UPDATED_AT); this.dbClient = dbClient; } @@ -54,7 +60,7 @@ public class AuthorizationIndexer extends BaseIndexer { try (DbSession dbSession = dbClient.openSession(false)) { // warning - do not enable large mode, else disabling of replicas // will impact the type "issue" which is much bigger than issueAuthorization - BulkIndexer bulk = new BulkIndexer(esClient, IssueIndexDefinition.INDEX); + BulkIndexer bulk = new BulkIndexer(esClient, INDEX); AuthorizationDao dao = new AuthorizationDao(); Collection authorizations = dao.selectAfterDate(dbClient, dbSession, lastUpdatedAt); @@ -64,7 +70,7 @@ public class AuthorizationIndexer extends BaseIndexer { @VisibleForTesting public void index(Collection authorizations) { - final BulkIndexer bulk = new BulkIndexer(esClient, IssueIndexDefinition.INDEX); + final BulkIndexer bulk = new BulkIndexer(esClient, INDEX); doIndex(bulk, authorizations); } @@ -72,7 +78,7 @@ public class AuthorizationIndexer extends BaseIndexer { long maxDate = 0L; bulk.start(); for (AuthorizationDao.Dto authorization : authorizations) { - bulk.add(newIssueUpdateRequest(authorization)); + bulk.add(newIndexRequest(authorization)); maxDate = Math.max(maxDate, authorization.getUpdatedAt()); } bulk.stop(); @@ -81,21 +87,20 @@ public class AuthorizationIndexer extends BaseIndexer { public void deleteProject(String uuid, boolean refresh) { esClient - .prepareDelete(IssueIndexDefinition.INDEX, IssueIndexDefinition.TYPE_AUTHORIZATION, uuid) + .prepareDelete(INDEX, TYPE_AUTHORIZATION, uuid) .setRefresh(refresh) .setRouting(uuid) .get(); } - private static ActionRequest newIssueUpdateRequest(AuthorizationDao.Dto dto) { + private static IndexRequest newIndexRequest(AuthorizationDao.Dto dto) { Map doc = ImmutableMap.of( - IssueIndexDefinition.FIELD_AUTHORIZATION_PROJECT_UUID, dto.getProjectUuid(), - IssueIndexDefinition.FIELD_AUTHORIZATION_GROUPS, dto.getGroups(), - IssueIndexDefinition.FIELD_AUTHORIZATION_USERS, dto.getUsers(), - IssueIndexDefinition.FIELD_AUTHORIZATION_UPDATED_AT, new Date(dto.getUpdatedAt())); - return new UpdateRequest(IssueIndexDefinition.INDEX, IssueIndexDefinition.TYPE_AUTHORIZATION, dto.getProjectUuid()) + FIELD_AUTHORIZATION_PROJECT_UUID, dto.getProjectUuid(), + FIELD_AUTHORIZATION_GROUPS, dto.getGroups(), + FIELD_AUTHORIZATION_USERS, dto.getUsers(), + FIELD_AUTHORIZATION_UPDATED_AT, new Date(dto.getUpdatedAt())); + return new IndexRequest(INDEX, TYPE_AUTHORIZATION, dto.getProjectUuid()) .routing(dto.getProjectUuid()) - .doc(doc) - .upsert(doc); + .source(doc); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/step/ApplyPermissionsStepTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/step/ApplyPermissionsStepTest.java index 0ae9354a80c..76c22738502 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/step/ApplyPermissionsStepTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/step/ApplyPermissionsStepTest.java @@ -19,9 +19,6 @@ */ package org.sonar.server.computation.task.projectanalysis.step; -import java.util.List; -import java.util.Map; -import org.elasticsearch.search.SearchHit; import org.junit.After; import org.junit.Before; import org.junit.Rule; @@ -47,14 +44,16 @@ import org.sonar.server.computation.task.step.ComputationStep; import org.sonar.server.es.EsTester; import org.sonar.server.issue.index.IssueIndexDefinition; import org.sonar.server.permission.index.AuthorizationIndexer; +import org.sonar.server.permission.index.AuthorizationIndexerTester; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; import static org.sonar.db.component.ComponentTesting.newView; import static org.sonar.db.permission.template.PermissionTemplateTesting.newPermissionTemplateDto; import static org.sonar.server.computation.task.projectanalysis.component.Component.Type.PROJECT; import static org.sonar.server.computation.task.projectanalysis.component.Component.Type.VIEW; - public class ApplyPermissionsStepTest extends BaseStepTest { private static final String ROOT_KEY = "ROOT_KEY"; @@ -73,19 +72,17 @@ public class ApplyPermissionsStepTest extends BaseStepTest { @Rule public MutableDbIdsRepositoryRule dbIdsRepository = MutableDbIdsRepositoryRule.create(treeRootHolder); + private AuthorizationIndexerTester authorizationIndexerTester = new AuthorizationIndexerTester(esTester); + private DbSession dbSession; private DbClient dbClient = dbTester.getDbClient(); private Settings settings = new MapSettings(); - private AuthorizationIndexer issueAuthorizationIndexer; private ApplyPermissionsStep step; @Before public void setUp() { dbSession = dbClient.openSession(false); - - issueAuthorizationIndexer = new AuthorizationIndexer(dbClient, esTester.client()); - - step = new ApplyPermissionsStep(dbClient, dbIdsRepository, issueAuthorizationIndexer, new PermissionRepository(dbClient, settings), treeRootHolder); + step = new ApplyPermissionsStep(dbClient, dbIdsRepository, new AuthorizationIndexer(dbClient, esTester.client()), new PermissionRepository(dbClient, settings), treeRootHolder); } @After @@ -110,7 +107,7 @@ public class ApplyPermissionsStepTest extends BaseStepTest { assertThat(dbClient.componentDao().selectOrFailByKey(dbSession, ROOT_KEY).getAuthorizationUpdatedAt()).isNotNull(); assertThat(dbClient.roleDao().selectGroupPermissions(dbSession, DefaultGroups.ANYONE, projectDto.getId())).containsOnly(UserRole.USER); - verifyAuthorisationIndex(ROOT_UUID, DefaultGroups.ANYONE); + authorizationIndexerTester.verifyProjectAuthorization(ROOT_UUID, singletonList(DefaultGroups.ANYONE), emptyList()); } @Test @@ -178,15 +175,6 @@ public class ApplyPermissionsStepTest extends BaseStepTest { dbSession.commit(); } - private void verifyAuthorisationIndex(String rootUuid, String groupPermission){ - List issueAuthorizationHits = esTester.getDocuments(IssueIndexDefinition.INDEX, IssueIndexDefinition.TYPE_AUTHORIZATION); - assertThat(issueAuthorizationHits).hasSize(1); - Map issueAuthorization = issueAuthorizationHits.get(0).sourceAsMap(); - assertThat(issueAuthorization.get("project")).isEqualTo(rootUuid); - assertThat((List) issueAuthorization.get("groups")).containsOnly(groupPermission); - assertThat((List) issueAuthorization.get("users")).isEmpty(); - } - @Override protected ComputationStep step() { return step; diff --git a/server/sonar-server/src/test/java/org/sonar/server/permission/index/AuthorizationIndexerTest.java b/server/sonar-server/src/test/java/org/sonar/server/permission/index/AuthorizationIndexerTest.java index 1e24bc1d197..bf888f56f95 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/permission/index/AuthorizationIndexerTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/permission/index/AuthorizationIndexerTest.java @@ -19,10 +19,6 @@ */ package org.sonar.server.permission.index; -import java.util.Arrays; -import java.util.Collection; -import java.util.List; -import org.elasticsearch.search.SearchHit; import org.junit.Rule; import org.junit.Test; import org.sonar.api.config.MapSettings; @@ -36,6 +32,8 @@ import org.sonar.db.user.UserDto; import org.sonar.server.es.EsTester; import org.sonar.server.issue.index.IssueIndexDefinition; +import static java.util.Arrays.asList; +import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; import static org.sonar.api.security.DefaultGroups.ANYONE; import static org.sonar.api.web.UserRole.ADMIN; @@ -51,6 +49,7 @@ public class AuthorizationIndexerTest { ComponentDbTester componentDbTester = new ComponentDbTester(dbTester); UserDbTester userDbTester = new UserDbTester(dbTester); + AuthorizationIndexerTester authorizationIndexerTester = new AuthorizationIndexerTester(esTester); AuthorizationIndexer underTest = new AuthorizationIndexer(dbTester.getDbClient(), esTester.client()); @@ -73,51 +72,33 @@ public class AuthorizationIndexerTest { underTest.doIndex(0L); - List docs = esTester.getDocuments("issues", "authorization"); - assertThat(docs).hasSize(1); - SearchHit doc = docs.get(0); - assertThat(doc.getSource().get("project")).isEqualTo(project.uuid()); - assertThat((Collection) doc.getSource().get("groups")).containsOnly(group.getName(), ANYONE); - assertThat((Collection) doc.getSource().get("users")).containsOnly(user.getLogin()); + authorizationIndexerTester.verifyProjectAuthorization(project.uuid(), asList(group.getName(), ANYONE), singletonList(user.getLogin())); } @Test public void delete_project() { - AuthorizationDao.Dto authorization = new AuthorizationDao.Dto("ABC", System.currentTimeMillis()); - authorization.addUser("guy"); - authorization.addGroup("dev"); - underTest.index(Arrays.asList(authorization)); + authorizationIndexerTester.insertProjectAuthorization("ABC", singletonList("guy"), singletonList("dev")); underTest.deleteProject("ABC", true); - assertThat(esTester.countDocuments("issues", "authorization")).isZero(); + authorizationIndexerTester.verifyEmptyProjectAuthorization(); } @Test public void do_not_fail_when_deleting_unindexed_project() { underTest.deleteProject("UNKNOWN", true); - assertThat(esTester.countDocuments("issues", "authorization")).isZero(); + authorizationIndexerTester.verifyEmptyProjectAuthorization(); } @Test - public void delete_permissions() { - AuthorizationDao.Dto authorization = new AuthorizationDao.Dto("ABC", System.currentTimeMillis()); - authorization.addUser("guy"); - authorization.addGroup("dev"); - underTest.index(Arrays.asList(authorization)); - - // has permissions - assertThat(esTester.countDocuments("issues", "authorization")).isEqualTo(1); + public void update_existing_permissions() { + authorizationIndexerTester.insertProjectAuthorization("ABC", singletonList("guy"), singletonList("dev")); // remove permissions -> dto has no users nor groups - authorization = new AuthorizationDao.Dto("ABC", System.currentTimeMillis()); - underTest.index(Arrays.asList(authorization)); + underTest.index(singletonList(new AuthorizationDao.Dto("ABC", System.currentTimeMillis()))); - List docs = esTester.getDocuments("issues", "authorization"); - assertThat(docs).hasSize(1); - assertThat((Collection) docs.get(0).sourceAsMap().get(IssueIndexDefinition.FIELD_AUTHORIZATION_USERS)).hasSize(0); - assertThat((Collection) docs.get(0).sourceAsMap().get(IssueIndexDefinition.FIELD_AUTHORIZATION_GROUPS)).hasSize(0); + authorizationIndexerTester.verifyProjectAsNoAuthorization("ABC"); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/permission/index/AuthorizationIndexerTester.java b/server/sonar-server/src/test/java/org/sonar/server/permission/index/AuthorizationIndexerTester.java new file mode 100644 index 00000000000..6591022a721 --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/permission/index/AuthorizationIndexerTester.java @@ -0,0 +1,85 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.server.permission.index; + +import java.util.List; +import org.elasticsearch.action.search.SearchRequestBuilder; +import org.elasticsearch.index.query.BoolQueryBuilder; +import org.sonar.server.es.EsTester; + +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; +import static org.assertj.core.api.Assertions.assertThat; +import static org.elasticsearch.index.query.QueryBuilders.boolQuery; +import static org.elasticsearch.index.query.QueryBuilders.existsQuery; +import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; +import static org.elasticsearch.index.query.QueryBuilders.termQuery; +import static org.elasticsearch.index.query.QueryBuilders.termsQuery; +import static org.sonar.server.issue.index.IssueIndexDefinition.FIELD_AUTHORIZATION_GROUPS; +import static org.sonar.server.issue.index.IssueIndexDefinition.FIELD_AUTHORIZATION_PROJECT_UUID; +import static org.sonar.server.issue.index.IssueIndexDefinition.FIELD_AUTHORIZATION_USERS; + +public class AuthorizationIndexerTester { + + private final EsTester esTester; + + private final AuthorizationIndexer authorizationIndexer; + + public AuthorizationIndexerTester(EsTester esTester) { + this.esTester = esTester; + this.authorizationIndexer = new AuthorizationIndexer(null, esTester.client()); + } + + public void insertProjectAuthorization(String projectUuid, List groupNames, List userLogins) { + AuthorizationDao.Dto authorization = new AuthorizationDao.Dto(projectUuid, System.currentTimeMillis()); + groupNames.forEach(authorization::addUser); + userLogins.forEach(authorization::addGroup); + authorizationIndexer.index(singletonList(authorization)); + } + + public void verifyEmptyProjectAuthorization() { + assertThat(esTester.countDocuments("issues", "authorization")).isZero(); + } + + public void verifyProjectAsNoAuthorization(String projectUuid) { + verifyProjectAuthorization(projectUuid, emptyList(), emptyList()); + } + + public void verifyProjectAuthorization(String projectUuid, List groupNames, List userLogins) { + assertThat(esTester.getIds("issues", "authorization")).containsOnly(projectUuid); + BoolQueryBuilder queryBuilder = boolQuery().must(termQuery(FIELD_AUTHORIZATION_PROJECT_UUID, projectUuid)); + if (groupNames.isEmpty()) { + queryBuilder.mustNot(existsQuery(FIELD_AUTHORIZATION_GROUPS)); + } else { + queryBuilder.must(termsQuery(FIELD_AUTHORIZATION_GROUPS, groupNames)); + } + if (userLogins.isEmpty()) { + queryBuilder.mustNot(existsQuery(FIELD_AUTHORIZATION_USERS)); + } else { + queryBuilder.must(termsQuery(FIELD_AUTHORIZATION_USERS, userLogins)); + } + SearchRequestBuilder request = esTester.client() + .prepareSearch("issues") + .setTypes("authorization") + .setQuery(boolQuery().must(matchAllQuery()).filter(queryBuilder)); + assertThat(request.get().getHits()).hasSize(1); + } +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/project/ws/BulkDeleteActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/project/ws/BulkDeleteActionTest.java index 9439a58a2ad..218e0f1f48a 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/project/ws/BulkDeleteActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/project/ws/BulkDeleteActionTest.java @@ -62,7 +62,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import static org.sonar.server.issue.index.IssueIndexDefinition.FIELD_AUTHORIZATION_PROJECT_UUID; import static org.sonar.server.issue.index.IssueIndexDefinition.TYPE_AUTHORIZATION; import static org.sonar.server.issue.index.IssueIndexDefinition.TYPE_ISSUE; import static org.sonar.server.project.ws.BulkDeleteAction.PARAM_IDS; @@ -159,8 +158,7 @@ public class BulkDeleteActionTest { String remainingProjectUuid = "project-uuid-2"; assertThat(es.getDocumentFieldValues(IssueIndexDefinition.INDEX, TYPE_ISSUE, IssueIndexDefinition.FIELD_ISSUE_PROJECT_UUID)) .containsOnly(remainingProjectUuid); - assertThat(es.getDocumentFieldValues(IssueIndexDefinition.INDEX, TYPE_AUTHORIZATION, FIELD_AUTHORIZATION_PROJECT_UUID)) - .containsOnly(remainingProjectUuid); + assertThat(es.getIds(IssueIndexDefinition.INDEX, TYPE_AUTHORIZATION)).containsOnly(remainingProjectUuid); assertThat(es.getDocumentFieldValues(TestIndexDefinition.INDEX, TestIndexDefinition.TYPE, TestIndexDefinition.FIELD_PROJECT_UUID)) .containsOnly(remainingProjectUuid); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/project/ws/DeleteActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/project/ws/DeleteActionTest.java index 8ba806700dd..2f318e4d3f1 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/project/ws/DeleteActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/project/ws/DeleteActionTest.java @@ -176,8 +176,7 @@ public class DeleteActionTest { String remainingProjectUuid = "project-uuid-2"; assertThat(es.getDocumentFieldValues(IssueIndexDefinition.INDEX, IssueIndexDefinition.TYPE_ISSUE, IssueIndexDefinition.FIELD_ISSUE_PROJECT_UUID)) .containsOnly(remainingProjectUuid); - assertThat(es.getDocumentFieldValues(IssueIndexDefinition.INDEX, IssueIndexDefinition.TYPE_AUTHORIZATION, IssueIndexDefinition.FIELD_AUTHORIZATION_PROJECT_UUID)) - .containsOnly(remainingProjectUuid); + assertThat(es.getIds(IssueIndexDefinition.INDEX, IssueIndexDefinition.TYPE_AUTHORIZATION)).containsOnly(remainingProjectUuid); assertThat(es.getDocumentFieldValues(TestIndexDefinition.INDEX, TestIndexDefinition.TYPE, TestIndexDefinition.FIELD_PROJECT_UUID)) .containsOnly(remainingProjectUuid); } -- 2.39.5