From 5e45a21807f091d4269df513b432c25c755a03c7 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Thu, 18 May 2017 16:24:36 +0200 Subject: [PATCH] SONAR-8888 use DefinedQProfileInsert when creating an organization rather than DefinedQProfileCreation which does too many useless operations --- .../should_display_changelog.html | 2 +- .../OrganizationCreationImpl.java | 60 +++++++++--------- .../platformlevel/PlatformLevel4.java | 2 + .../platformlevel/PlatformLevelStartup.java | 2 - .../OrganizationCreationImplTest.java | 33 +++------- .../organization/ws/CreateActionTest.java | 5 +- .../DefinedQProfileInsertRule.java | 62 +++++++++++++++++++ 7 files changed, 102 insertions(+), 64 deletions(-) create mode 100644 server/sonar-server/src/test/java/org/sonar/server/qualityprofile/DefinedQProfileInsertRule.java diff --git a/it/it-tests/src/test/resources/organization/OrganizationQualityProfilesPageTest/should_display_changelog.html b/it/it-tests/src/test/resources/organization/OrganizationQualityProfilesPageTest/should_display_changelog.html index 417700e47db..12536be7cbf 100644 --- a/it/it-tests/src/test/resources/organization/OrganizationQualityProfilesPageTest/should_display_changelog.html +++ b/it/it-tests/src/test/resources/organization/OrganizationQualityProfilesPageTest/should_display_changelog.html @@ -47,7 +47,7 @@ assertText css=.js-profile-changelog-event - *Administrator*Activated*Has Tag*Major*tag*xoo* + *System*Activated*Has Tag*Major*tag*xoo* diff --git a/server/sonar-server/src/main/java/org/sonar/server/organization/OrganizationCreationImpl.java b/server/sonar-server/src/main/java/org/sonar/server/organization/OrganizationCreationImpl.java index c8591cf3790..327d9bd76d1 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/organization/OrganizationCreationImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/organization/OrganizationCreationImpl.java @@ -19,9 +19,7 @@ */ package org.sonar.server.organization; -import java.util.ArrayList; import java.util.Date; -import java.util.List; import java.util.Optional; import java.util.function.Consumer; import javax.annotation.Nullable; @@ -44,11 +42,9 @@ import org.sonar.db.permission.template.PermissionTemplateDto; import org.sonar.db.user.GroupDto; import org.sonar.db.user.UserDto; import org.sonar.db.user.UserGroupDto; -import org.sonar.server.qualityprofile.ActiveRuleChange; import org.sonar.server.qualityprofile.DefinedQProfile; -import org.sonar.server.qualityprofile.DefinedQProfileCreation; +import org.sonar.server.qualityprofile.DefinedQProfileInsert; import org.sonar.server.qualityprofile.DefinedQProfileRepository; -import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; import org.sonar.server.user.index.UserIndexer; import org.sonar.server.usergroups.DefaultGroupCreator; @@ -71,15 +67,14 @@ public class OrganizationCreationImpl implements OrganizationCreation { private final OrganizationValidation organizationValidation; private final Settings settings; private final DefinedQProfileRepository definedQProfileRepository; - private final DefinedQProfileCreation definedQProfileCreation; + private final DefinedQProfileInsert definedQProfileInsert; private final DefaultGroupCreator defaultGroupCreator; - private final ActiveRuleIndexer activeRuleIndexer; private final UserIndexer userIndexer; public OrganizationCreationImpl(DbClient dbClient, System2 system2, UuidFactory uuidFactory, OrganizationValidation organizationValidation, Settings settings, UserIndexer userIndexer, - DefinedQProfileRepository definedQProfileRepository, DefinedQProfileCreation definedQProfileCreation, DefaultGroupCreator defaultGroupCreator, - ActiveRuleIndexer activeRuleIndexer) { + DefinedQProfileRepository definedQProfileRepository, DefinedQProfileInsert definedQProfileInsert, + DefaultGroupCreator defaultGroupCreator) { this.dbClient = dbClient; this.system2 = system2; this.uuidFactory = uuidFactory; @@ -87,9 +82,8 @@ public class OrganizationCreationImpl implements OrganizationCreation { this.settings = settings; this.userIndexer = userIndexer; this.definedQProfileRepository = definedQProfileRepository; - this.definedQProfileCreation = definedQProfileCreation; + this.definedQProfileInsert = definedQProfileInsert; this.defaultGroupCreator = defaultGroupCreator; - this.activeRuleIndexer = activeRuleIndexer; } @Override @@ -106,17 +100,19 @@ public class OrganizationCreationImpl implements OrganizationCreation { GroupDto ownerGroup = insertOwnersGroup(dbSession, organization); GroupDto defaultGroup = defaultGroupCreator.create(dbSession, organization.getUuid()); insertDefaultTemplateOnGroups(dbSession, organization, ownerGroup, defaultGroup); - List activeRuleChanges = insertQualityProfiles(dbSession, organization); - addCurrentUserToGroup(dbSession, ownerGroup, userCreator.getId()); - addCurrentUserToGroup(dbSession, defaultGroup, userCreator.getId()); + try (DbSession batchDbSession = dbClient.openSession(true)) { + insertQualityProfiles(dbSession, batchDbSession, organization); + addCurrentUserToGroup(dbSession, ownerGroup, userCreator.getId()); + addCurrentUserToGroup(dbSession, defaultGroup, userCreator.getId()); - dbSession.commit(); + dbSession.commit(); + batchDbSession.commit(); - // Elasticsearch is updated when DB session is committed - userIndexer.index(userCreator.getLogin()); - activeRuleIndexer.index(activeRuleChanges); + // Elasticsearch is updated when DB session is committed + userIndexer.index(userCreator.getLogin()); - return organization; + return organization; + } } @Override @@ -143,16 +139,18 @@ public class OrganizationCreationImpl implements OrganizationCreation { OrganizationPermission.all() .forEach(p -> insertUserPermissions(dbSession, newUser, organization, p)); insertPersonalOrgDefaultTemplate(dbSession, organization, defaultGroup); - List activeRuleChanges = insertQualityProfiles(dbSession, organization); - addCurrentUserToGroup(dbSession, defaultGroup, newUser.getId()); + try (DbSession batchDbSession = dbClient.openSession(true)) { + insertQualityProfiles(dbSession, batchDbSession, organization); + addCurrentUserToGroup(dbSession, defaultGroup, newUser.getId()); - dbSession.commit(); + dbSession.commit(); + batchDbSession.commit(); - // Elasticsearch is updated when DB session is committed - activeRuleIndexer.index(activeRuleChanges); - userIndexer.index(newUser.getLogin()); + // Elasticsearch is updated when DB session is committed + userIndexer.index(newUser.getLogin()); - return Optional.of(organization); + return Optional.of(organization); + } } private static String nameOrLogin(UserDto newUser) { @@ -264,19 +262,17 @@ public class OrganizationCreationImpl implements OrganizationCreation { dbClient.permissionTemplateDao().insertGroupPermission(dbSession, template.getId(), group == null ? null : group.getId(), permission); } - private List insertQualityProfiles(DbSession dbSession, OrganizationDto organization) { - List changes = new ArrayList<>(); + private void insertQualityProfiles(DbSession dbSession, DbSession batchDbSession, OrganizationDto organization) { definedQProfileRepository.getQProfilesByLanguage().entrySet() .stream() .flatMap(entry -> entry.getValue().stream()) - .forEach(profile -> insertQualityProfile(dbSession, profile, organization, changes)); - return changes; + .forEach(profile -> insertQualityProfile(dbSession, batchDbSession, profile, organization)); } - private void insertQualityProfile(DbSession dbSession, DefinedQProfile profile, OrganizationDto organization, List changes) { + private void insertQualityProfile(DbSession regularSession, DbSession batchDbSession, DefinedQProfile profile, OrganizationDto organization) { LOGGER.debug("Creating quality profile {} for language {} for organization {}", profile.getName(), profile.getLanguage(), organization.getKey()); - definedQProfileCreation.create(dbSession, profile, organization, changes); + definedQProfileInsert.create(regularSession, batchDbSession, profile, organization); } /** diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel4.java b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel4.java index c626336a8b2..8805cb297f5 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel4.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel4.java @@ -142,6 +142,7 @@ import org.sonar.server.property.InternalPropertiesImpl; import org.sonar.server.property.ws.PropertiesWs; import org.sonar.server.qualitygate.QualityGateModule; import org.sonar.server.qualityprofile.DefinedQProfileCreationImpl; +import org.sonar.server.qualityprofile.DefinedQProfileInsertImpl; import org.sonar.server.qualityprofile.DefinedQProfileRepositoryImpl; import org.sonar.server.qualityprofile.QProfileBackuperImpl; import org.sonar.server.qualityprofile.QProfileComparison; @@ -260,6 +261,7 @@ public class PlatformLevel4 extends PlatformLevel { // quality profile DefinedQProfileRepositoryImpl.class, + DefinedQProfileInsertImpl.class, ActiveRuleIndexer.class, XMLProfileParser.class, XMLProfileSerializer.class, diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevelStartup.java b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevelStartup.java index c933b8f9122..e04676ae3fb 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevelStartup.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevelStartup.java @@ -28,7 +28,6 @@ import org.sonar.server.qualitygate.RegisterQualityGates; import org.sonar.server.qualityprofile.CachingDefinedQProfileCreationImpl; import org.sonar.server.qualityprofile.CachingRuleActivator; import org.sonar.server.qualityprofile.CachingRuleActivatorContextFactory; -import org.sonar.server.qualityprofile.DefinedQProfileInsertImpl; import org.sonar.server.qualityprofile.DefinedQProfileLoader; import org.sonar.server.qualityprofile.MassRegisterQualityProfiles; import org.sonar.server.qualityprofile.RegisterQualityProfiles; @@ -60,7 +59,6 @@ public class PlatformLevelStartup extends PlatformLevel { RegisterRules.class); add(DefinedQProfileLoader.class); addIfStartupLeader( - DefinedQProfileInsertImpl.class, MassRegisterQualityProfiles.class, CachingRuleActivatorContextFactory.class, CachingRuleActivator.class, diff --git a/server/sonar-server/src/test/java/org/sonar/server/organization/OrganizationCreationImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/organization/OrganizationCreationImplTest.java index 79ccdbfca0f..de7d31f798d 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/organization/OrganizationCreationImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/organization/OrganizationCreationImplTest.java @@ -19,7 +19,6 @@ */ package org.sonar.server.organization; -import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Optional; @@ -52,9 +51,8 @@ import org.sonar.server.es.SearchOptions; import org.sonar.server.language.LanguageTesting; import org.sonar.server.qualityprofile.ActiveRuleChange; import org.sonar.server.qualityprofile.DefinedQProfile; -import org.sonar.server.qualityprofile.DefinedQProfileCreationRule; +import org.sonar.server.qualityprofile.DefinedQProfileInsertRule; import org.sonar.server.qualityprofile.DefinedQProfileRepositoryRule; -import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; import org.sonar.server.user.index.UserIndex; import org.sonar.server.user.index.UserIndexDefinition; import org.sonar.server.user.index.UserIndexer; @@ -66,8 +64,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.fail; import static org.assertj.core.api.Assertions.tuple; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import static org.sonar.server.organization.OrganizationCreation.NewOrganization.newOrganizationBuilder; @@ -99,7 +95,7 @@ public class OrganizationCreationImplTest { @Rule public DefinedQProfileRepositoryRule definedQProfileRepositoryRule = new DefinedQProfileRepositoryRule(); @Rule - public DefinedQProfileCreationRule definedQProfileCreationRule = new DefinedQProfileCreationRule(); + public DefinedQProfileInsertRule definedQProfileCreationRule = new DefinedQProfileInsertRule(); private DbSession dbSession = dbTester.getSession(); @@ -110,11 +106,10 @@ public class OrganizationCreationImplTest { private MapSettings settings = new MapSettings(); private UserIndexer userIndexer = new UserIndexer(dbClient, es.client()); private UserIndex userIndex = new UserIndex(es.client()); - private ActiveRuleIndexer activeRuleIndexer = mock(ActiveRuleIndexer.class); private DefaultGroupCreator defaultGroupCreator = new DefaultGroupCreatorImpl(dbClient); private OrganizationCreationImpl underTest = new OrganizationCreationImpl(dbClient, system2, uuidFactory, organizationValidation, settings, userIndexer, - definedQProfileRepositoryRule, definedQProfileCreationRule, defaultGroupCreator, activeRuleIndexer); + definedQProfileRepositoryRule, definedQProfileCreationRule, defaultGroupCreator); private UserDto someUser; @@ -297,11 +292,6 @@ public class OrganizationCreationImplTest { DefinedQProfile definedQProfile3 = definedQProfileRepositoryRule.add(LanguageTesting.newLanguage("foo"), "qp3"); DefinedQProfile definedQProfile4 = definedQProfileRepositoryRule.add(LanguageTesting.newLanguage("foo"), "qp4"); definedQProfileRepositoryRule.initialize(); - ActiveRuleChange[] changes = {newActiveRuleChange("0"), newActiveRuleChange("1"), newActiveRuleChange("2"), newActiveRuleChange("3"), newActiveRuleChange("4")}; - definedQProfileCreationRule.addChanges(); - definedQProfileCreationRule.addChanges(changes[2], changes[1], changes[4]); - definedQProfileCreationRule.addChanges(changes[3]); - definedQProfileCreationRule.addChanges(changes[0]); mockForSuccessfulInsert(SOME_UUID, SOME_DATE); underTest.create(dbSession, someUser, FULL_POPULATED_NEW_ORGANIZATION); @@ -309,15 +299,13 @@ public class OrganizationCreationImplTest { OrganizationDto organization = dbClient.organizationDao().selectByKey(dbSession, FULL_POPULATED_NEW_ORGANIZATION.getKey()).get(); assertThat(definedQProfileCreationRule.getCallLogs()) .hasSize(4) - .extracting(DefinedQProfileCreationRule.CallLog::getOrganizationDto) + .extracting(DefinedQProfileInsertRule.CallLog::getOrganizationDto) .extracting(OrganizationDto::getUuid) .containsOnly(organization.getUuid()); assertThat(definedQProfileCreationRule.getCallLogs()) - .extracting(DefinedQProfileCreationRule.CallLog::getDefinedQProfile) + .extracting(DefinedQProfileInsertRule.CallLog::getDefinedQProfile) .extracting(DefinedQProfile::getName) .containsExactly(definedQProfile1.getName(), definedQProfile2.getName(), definedQProfile3.getName(), definedQProfile4.getName()); - verify(activeRuleIndexer).index(Arrays.asList(changes[2], changes[1], changes[4], changes[3], changes[0])); - verifyNoMoreInteractions(activeRuleIndexer); } @Test @@ -514,11 +502,6 @@ public class OrganizationCreationImplTest { DefinedQProfile definedQProfile3 = definedQProfileRepositoryRule.add(LanguageTesting.newLanguage("foo"), "qp3"); DefinedQProfile definedQProfile4 = definedQProfileRepositoryRule.add(LanguageTesting.newLanguage("foo"), "qp4"); definedQProfileRepositoryRule.initialize(); - ActiveRuleChange[] changes = {newActiveRuleChange("0"), newActiveRuleChange("1"), newActiveRuleChange("2"), newActiveRuleChange("3"), newActiveRuleChange("4")}; - definedQProfileCreationRule.addChanges(); - definedQProfileCreationRule.addChanges(changes[2], changes[1], changes[4]); - definedQProfileCreationRule.addChanges(changes[3]); - definedQProfileCreationRule.addChanges(changes[0]); mockForSuccessfulInsert(SOME_UUID, SOME_DATE); enableCreatePersonalOrg(true); @@ -527,15 +510,13 @@ public class OrganizationCreationImplTest { OrganizationDto organization = dbClient.organizationDao().selectByKey(dbSession, SLUG_OF_A_LOGIN).get(); assertThat(definedQProfileCreationRule.getCallLogs()) .hasSize(4) - .extracting(DefinedQProfileCreationRule.CallLog::getOrganizationDto) + .extracting(DefinedQProfileInsertRule.CallLog::getOrganizationDto) .extracting(OrganizationDto::getUuid) .containsOnly(organization.getUuid()); assertThat(definedQProfileCreationRule.getCallLogs()) - .extracting(DefinedQProfileCreationRule.CallLog::getDefinedQProfile) + .extracting(DefinedQProfileInsertRule.CallLog::getDefinedQProfile) .extracting(DefinedQProfile::getName) .containsExactly(definedQProfile1.getName(), definedQProfile2.getName(), definedQProfile3.getName(), definedQProfile4.getName()); - verify(activeRuleIndexer).index(Arrays.asList(changes[2], changes[1], changes[4], changes[3], changes[0])); - verifyNoMoreInteractions(activeRuleIndexer); } private static ActiveRuleChange newActiveRuleChange(String id) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/organization/ws/CreateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/organization/ws/CreateActionTest.java index 48ac414b770..a1b02e8f8ef 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/organization/ws/CreateActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/organization/ws/CreateActionTest.java @@ -55,9 +55,8 @@ import org.sonar.server.organization.OrganizationCreationImpl; import org.sonar.server.organization.OrganizationValidation; import org.sonar.server.organization.OrganizationValidationImpl; import org.sonar.server.organization.TestOrganizationFlags; -import org.sonar.server.qualityprofile.DefinedQProfileCreation; +import org.sonar.server.qualityprofile.DefinedQProfileInsert; import org.sonar.server.qualityprofile.DefinedQProfileRepository; -import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.user.index.UserIndex; import org.sonar.server.user.index.UserIndexDefinition; @@ -102,7 +101,7 @@ public class CreateActionTest { private UserIndexer userIndexer = new UserIndexer(dbClient, es.client()); private UserIndex userIndex = new UserIndex(es.client()); private OrganizationCreation organizationCreation = new OrganizationCreationImpl(dbClient, system2, uuidFactory, organizationValidation, settings, userIndexer, - mock(DefinedQProfileRepository.class), mock(DefinedQProfileCreation.class), new DefaultGroupCreatorImpl(dbClient), mock(ActiveRuleIndexer.class)); + mock(DefinedQProfileRepository.class), mock(DefinedQProfileInsert.class), new DefaultGroupCreatorImpl(dbClient)); private TestOrganizationFlags organizationFlags = TestOrganizationFlags.standalone().setEnabled(true); private UserDto user; diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/DefinedQProfileInsertRule.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/DefinedQProfileInsertRule.java new file mode 100644 index 00000000000..bbf09367d3a --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/DefinedQProfileInsertRule.java @@ -0,0 +1,62 @@ +/* + * SonarQube + * Copyright (C) 2009-2017 SonarSource SA + * mailto:info 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.qualityprofile; + +import java.util.ArrayList; +import java.util.List; +import org.junit.rules.ExternalResource; +import org.sonar.db.DbSession; +import org.sonar.db.organization.OrganizationDto; + +public class DefinedQProfileInsertRule extends ExternalResource implements DefinedQProfileInsert { + private final List callLogs = new ArrayList<>(); + + @Override + protected void before() throws Throwable { + callLogs.clear(); + } + + @Override + public void create(DbSession session, DbSession batchSession, DefinedQProfile definedQProfile, OrganizationDto organization) { + callLogs.add(new DefinedQProfileInsertRule.CallLog(definedQProfile, organization)); + } + + public List getCallLogs() { + return callLogs; + } + + public static final class CallLog { + private final DefinedQProfile definedQProfile; + private final OrganizationDto organizationDto; + + private CallLog(DefinedQProfile definedQProfile, OrganizationDto organizationDto) { + this.definedQProfile = definedQProfile; + this.organizationDto = organizationDto; + } + + public DefinedQProfile getDefinedQProfile() { + return definedQProfile; + } + + public OrganizationDto getOrganizationDto() { + return organizationDto; + } + } +} -- 2.39.5