From: Sébastien Lesaint Date: Tue, 7 Mar 2017 14:56:22 +0000 (+0100) Subject: SONAR-6315 cache list of children of quality profiles X-Git-Tag: 6.4-RC1~651 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=53c6f5d950b0329765020b28611ffe0ee7f1c8fa;p=sonarqube.git SONAR-6315 cache list of children of quality profiles to avoid doing the same SQL requests for each activated rule --- 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 4140b26359e..07797e41e32 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 @@ -25,6 +25,7 @@ import org.sonar.server.organization.DefaultOrganizationEnforcer; import org.sonar.server.platform.ServerLifecycleNotifier; import org.sonar.server.platform.web.RegisterServletFilters; import org.sonar.server.qualitygate.RegisterQualityGates; +import org.sonar.server.qualityprofile.CachingRuleActivator; import org.sonar.server.qualityprofile.RegisterQualityProfiles; import org.sonar.server.rule.RegisterRules; import org.sonar.server.startup.ClearRulesOverloadedDebt; @@ -52,6 +53,7 @@ public class PlatformLevelStartup extends PlatformLevel { IndexerStartupTask.class, RegisterMetrics.class, RegisterQualityGates.class, + CachingRuleActivator.class, RegisterRules.class, RegisterQualityProfiles.class, RegisterPermissionTemplates.class, diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/CachingRuleActivator.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/CachingRuleActivator.java new file mode 100644 index 00000000000..478136c9dfe --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/CachingRuleActivator.java @@ -0,0 +1,55 @@ +/* + * 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 com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; +import java.util.List; +import org.sonar.api.utils.System2; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.qualityprofile.QualityProfileDto; +import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; +import org.sonar.server.rule.index.RuleIndex; +import org.sonar.server.user.UserSession; +import org.sonar.server.util.TypeValidations; + +public class CachingRuleActivator extends RuleActivator { + private final Cache> childrenByParentKey = CacheBuilder.newBuilder() + .maximumSize(10_000) + .build(); + + public CachingRuleActivator(System2 system2, DbClient db, RuleIndex ruleIndex, RuleActivatorContextFactory contextFactory, TypeValidations typeValidations, + ActiveRuleIndexer activeRuleIndexer, UserSession userSession) { + super(system2, db, ruleIndex, contextFactory, typeValidations, activeRuleIndexer, userSession); + } + + @Override + protected List getChildren(DbSession session, String qualityProfileKey) { + List res = childrenByParentKey.getIfPresent(qualityProfileKey); + if (res != null) { + return res; + } + res = super.getChildren(session, qualityProfileKey); + childrenByParentKey.put(qualityProfileKey, res); + return res; + } + +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RegisterQualityProfiles.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RegisterQualityProfiles.java index e90f0d93a0c..8bdeb5c37d4 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RegisterQualityProfiles.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RegisterQualityProfiles.java @@ -69,13 +69,13 @@ public class RegisterQualityProfiles { * To be kept when no ProfileDefinition are injected */ public RegisterQualityProfiles(DbClient dbClient, - QProfileFactory profileFactory, RuleActivator ruleActivator, Languages languages, ActiveRuleIndexer activeRuleIndexer, + QProfileFactory profileFactory, CachingRuleActivator ruleActivator, Languages languages, ActiveRuleIndexer activeRuleIndexer, DefaultOrganizationProvider defaultOrganizationProvider) { this(dbClient, profileFactory, ruleActivator, Collections.emptyList(), languages, activeRuleIndexer, defaultOrganizationProvider); } public RegisterQualityProfiles(DbClient dbClient, - QProfileFactory profileFactory, RuleActivator ruleActivator, + QProfileFactory profileFactory, CachingRuleActivator ruleActivator, List definitions, Languages languages, ActiveRuleIndexer activeRuleIndexer, DefaultOrganizationProvider defaultOrganizationProvider) { this.dbClient = dbClient; diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java index c3883a898d7..d4751fb20f3 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java @@ -233,7 +233,8 @@ public class RuleActivator { List changes = Lists.newArrayList(); // get all inherited profiles - List children = db.qualityProfileDao().selectChildren(session, qualityProfileDto.getKey()); + String qualityProfileKey = qualityProfileDto.getKey(); + List children = getChildren(session, qualityProfileKey); for (QualityProfileDto child : children) { RuleActivation childActivation = new RuleActivation(activation).setCascade(true); changes.addAll(activate(session, childActivation, child)); @@ -241,6 +242,10 @@ public class RuleActivator { return changes; } + protected List getChildren(DbSession session, String qualityProfileKey) { + return db.qualityProfileDao().selectChildren(session, qualityProfileKey); + } + private ActiveRuleDto persist(ActiveRuleChange change, RuleActivatorContext context, DbSession dbSession) { ActiveRuleDto activeRule = null; if (change.getType() == ActiveRuleChange.Type.ACTIVATED) { @@ -375,7 +380,7 @@ public class RuleActivator { persist(change, context, dbSession); // get all inherited profiles - List profiles = db.qualityProfileDao().selectChildren(dbSession, key.qProfile()); + List profiles = getChildren(dbSession, key.qProfile()); for (QualityProfileDto profile : profiles) { ActiveRuleKey activeRuleKey = ActiveRuleKey.of(profile.getKey(), key.ruleKey()); diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/CachingRuleActivatorTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/CachingRuleActivatorTest.java new file mode 100644 index 00000000000..66664b46126 --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/CachingRuleActivatorTest.java @@ -0,0 +1,108 @@ +/* + * 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.Arrays; +import java.util.stream.Collectors; +import org.junit.Before; +import org.junit.Test; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.qualityprofile.QualityProfileDao; +import org.sonar.db.qualityprofile.QualityProfileDto; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Matchers.anyBoolean; +import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +public class CachingRuleActivatorTest { + private DbClient dbClient = mock(DbClient.class); + private DbSession dbSession = mock(DbSession.class); + private QualityProfileDao qualityProfileDao = mock(QualityProfileDao.class); + private CachingRuleActivator underTest = new CachingRuleActivator(null, dbClient, null, null, null, null, null); + + @Before + public void wire_mocks() throws Exception { + when(dbClient.openSession(anyBoolean())).thenReturn(dbSession); + when(dbClient.qualityProfileDao()).thenReturn(qualityProfileDao); + } + + @Test + public void getChildren_caches_that_qp_has_no_children() { + mockSelectChildrenForKey("no_children"); + + assertThat(underTest.getChildren(dbSession, "no_children")) + .isEmpty(); + assertThat(underTest.getChildren(dbSession, "no_children")) + .isEmpty(); + assertThat(underTest.getChildren(dbSession, "no_children")) + .isEmpty(); + verify(qualityProfileDao, times(1)).selectChildren(eq(dbSession), anyString()); + } + + @Test + public void getChildren_caches_that_sq_has_one_or_more_children() { + mockSelectChildrenForKey("0", "1"); + mockSelectChildrenForKey("1", "2", "3"); + + assertThat(underTest.getChildren(dbSession, "0")) + .extracting(QualityProfileDto::getKey) + .containsExactly("1"); + assertThat(underTest.getChildren(dbSession, "0")) + .extracting(QualityProfileDto::getKey) + .containsExactly("1"); + assertThat(underTest.getChildren(dbSession, "0")) + .extracting(QualityProfileDto::getKey) + .containsExactly("1"); + assertThat(underTest.getChildren(dbSession, "1")) + .extracting(QualityProfileDto::getKey) + .containsExactly("2", "3"); + assertThat(underTest.getChildren(dbSession, "1")) + .extracting(QualityProfileDto::getKey) + .containsExactly("2", "3"); + assertThat(underTest.getChildren(dbSession, "1")) + .extracting(QualityProfileDto::getKey) + .containsExactly("2", "3"); + verify(qualityProfileDao, times(1)).selectChildren(dbSession, "0"); + verify(qualityProfileDao, times(1)).selectChildren(dbSession, "1"); + verifyNoMoreInteractions(qualityProfileDao); + } + + private void mockSelectChildrenForKey(String key, String... children) { + when(qualityProfileDao.selectChildren(dbSession, key)) + .thenReturn(Arrays.stream(children).map(this::dto).collect(Collectors.toList())) + .thenThrow(new IllegalStateException("selectChildren should be called only once for key " + key)); + } + + private QualityProfileDto dto(String key) { + return new QualityProfileDto() { + @Override + public String toString() { + return getKey(); + } + }.setKey(key); + } +}