]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-6315 cache list of children of quality profiles
authorSébastien Lesaint <sebastien.lesaint@sonarsource.com>
Tue, 7 Mar 2017 14:56:22 +0000 (15:56 +0100)
committerSébastien Lesaint <sebastien.lesaint@sonarsource.com>
Thu, 23 Mar 2017 16:38:34 +0000 (17:38 +0100)
to avoid doing the same SQL requests for each activated rule

server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevelStartup.java
server/sonar-server/src/main/java/org/sonar/server/qualityprofile/CachingRuleActivator.java [new file with mode: 0644]
server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RegisterQualityProfiles.java
server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java
server/sonar-server/src/test/java/org/sonar/server/qualityprofile/CachingRuleActivatorTest.java [new file with mode: 0644]

index 4140b26359e8d34a5e21b9d72123551c44d1f666..07797e41e324aff30cc23d8ba018f923f3a35e20 100644 (file)
@@ -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 (file)
index 0000000..478136c
--- /dev/null
@@ -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<String, List<QualityProfileDto>> 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<QualityProfileDto> getChildren(DbSession session, String qualityProfileKey) {
+    List<QualityProfileDto> res = childrenByParentKey.getIfPresent(qualityProfileKey);
+    if (res != null) {
+      return res;
+    }
+    res = super.getChildren(session, qualityProfileKey);
+    childrenByParentKey.put(qualityProfileKey, res);
+    return res;
+  }
+
+}
index e90f0d93a0cb325d42a9dfa77d1b45be1ca051a3..8bdeb5c37d4733452bf25cb25229041eaee9ad7f 100644 (file)
@@ -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<ProfileDefinition> definitions, Languages languages, ActiveRuleIndexer activeRuleIndexer,
     DefaultOrganizationProvider defaultOrganizationProvider) {
     this.dbClient = dbClient;
index c3883a898d702187a90484c6a5c21557dddd8bd8..d4751fb20f38b93cdb37281f4f7c1a00647a96fc 100644 (file)
@@ -233,7 +233,8 @@ public class RuleActivator {
     List<ActiveRuleChange> changes = Lists.newArrayList();
 
     // get all inherited profiles
-    List<QualityProfileDto> children = db.qualityProfileDao().selectChildren(session, qualityProfileDto.getKey());
+    String qualityProfileKey = qualityProfileDto.getKey();
+    List<QualityProfileDto> 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<QualityProfileDto> 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<QualityProfileDto> profiles = db.qualityProfileDao().selectChildren(dbSession, key.qProfile());
+    List<QualityProfileDto> 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 (file)
index 0000000..66664b4
--- /dev/null
@@ -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);
+  }
+}