]> source.dussan.org Git - sonarqube.git/commitdiff
SONARCLOUD-582 split loading of profiles in rule activation
authorSimon Brandhof <simon.brandhof@sonarsource.com>
Wed, 17 Apr 2019 15:42:50 +0000 (17:42 +0200)
committerSonarTech <sonartech@sonarsource.com>
Wed, 24 Apr 2019 18:21:04 +0000 (20:21 +0200)
server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleDao.java
server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/ActiveRuleDaoTest.java
server/sonar-server/src/main/java/org/sonar/server/qualityprofile/DescendantProfilesSupplier.java [new file with mode: 0644]
server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivationContext.java
server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java

index 6607be8dd1016fc37087bb6a53444add54674490..95adfa315fc1e864d54899471852d64574980e4d 100644 (file)
@@ -25,12 +25,10 @@ import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.function.Consumer;
-import org.sonar.core.util.stream.MoreCollectors;
 import org.sonar.db.Dao;
 import org.sonar.db.DatabaseUtils;
 import org.sonar.db.DbSession;
 import org.sonar.db.organization.OrganizationDto;
-import org.sonar.db.rule.RuleDefinitionDto;
 import org.sonar.db.rule.RuleParamDto;
 
 import static com.google.common.base.Preconditions.checkArgument;
@@ -84,11 +82,10 @@ public class ActiveRuleDao implements Dao {
     return mapper(dbSession).selectByRuleProfileUuid(ruleProfileDto.getKee());
   }
 
-  public Collection<ActiveRuleDto> selectByRulesAndRuleProfileUuids(DbSession dbSession, Collection<RuleDefinitionDto> rules, Collection<String> ruleProfileUuids) {
-    if (rules.isEmpty() || ruleProfileUuids.isEmpty()) {
+  public Collection<ActiveRuleDto> selectByRulesAndRuleProfileUuids(DbSession dbSession, Collection<Integer> ruleIds, Collection<String> ruleProfileUuids) {
+    if (ruleIds.isEmpty() || ruleProfileUuids.isEmpty()) {
       return emptyList();
     }
-    List<Integer> ruleIds = rules.stream().map(RuleDefinitionDto::getId).collect(MoreCollectors.toArrayList(rules.size()));
     ActiveRuleMapper mapper = mapper(dbSession);
     return executeLargeInputs(ruleIds, ruleIdsChunk -> executeLargeInputs(ruleProfileUuids, chunk -> mapper.selectByRuleIdsAndRuleProfileUuids(ruleIdsChunk, chunk)));
   }
index f62285355ad1d23069e853a3b4c5c8942363cfe2..abfb197c9808a2961ec9a5220d897aa7f22c5813 100644 (file)
@@ -21,7 +21,6 @@ package org.sonar.db.qualityprofile;
 
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
 import java.util.function.Consumer;
@@ -236,25 +235,25 @@ public class ActiveRuleDaoTest {
     assertThat(result).isEmpty();
 
     // empty profiles
-    result = underTest.selectByRulesAndRuleProfileUuids(dbSession, asList(rule1), emptyList());
+    result = underTest.selectByRulesAndRuleProfileUuids(dbSession, asList(rule1.getId()), emptyList());
     assertThat(result).isEmpty();
 
     // match
-    result = underTest.selectByRulesAndRuleProfileUuids(dbSession, asList(rule1), asList(profile1.getRulesProfileUuid(), profile2.getRulesProfileUuid()));
+    result = underTest.selectByRulesAndRuleProfileUuids(dbSession, asList(rule1.getId()), asList(profile1.getRulesProfileUuid(), profile2.getRulesProfileUuid()));
     assertThat(result)
       .extracting(ActiveRuleDto::getId)
       .containsExactlyInAnyOrder(rule1P1.getId(), rule1P2.getId());
 
-    result = underTest.selectByRulesAndRuleProfileUuids(dbSession, asList(rule1, rule2), asList(profile1.getRulesProfileUuid(), profile2.getRulesProfileUuid()));
+    result = underTest.selectByRulesAndRuleProfileUuids(dbSession, asList(rule1.getId(), rule2.getId()), asList(profile1.getRulesProfileUuid(), profile2.getRulesProfileUuid()));
     assertThat(result)
       .extracting(ActiveRuleDto::getId)
       .containsExactlyInAnyOrder(rule1P1.getId(), rule1P2.getId(), rule2P1.getId());
 
     // do not match
-    result = underTest.selectByRulesAndRuleProfileUuids(dbSession, asList(rule3), asList(profile1.getRulesProfileUuid(), profile2.getRulesProfileUuid()));
+    result = underTest.selectByRulesAndRuleProfileUuids(dbSession, asList(rule3.getId()), asList(profile1.getRulesProfileUuid(), profile2.getRulesProfileUuid()));
     assertThat(result).isEmpty();
 
-    result = underTest.selectByRulesAndRuleProfileUuids(dbSession, asList(rule1), asList("unknown"));
+    result = underTest.selectByRulesAndRuleProfileUuids(dbSession, asList(rule1.getId()), asList("unknown"));
     assertThat(result).isEmpty();
   }
 
diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/DescendantProfilesSupplier.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/DescendantProfilesSupplier.java
new file mode 100644 (file)
index 0000000..ef07334
--- /dev/null
@@ -0,0 +1,55 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2019 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.Collection;
+import org.sonar.db.qualityprofile.ActiveRuleDto;
+import org.sonar.db.qualityprofile.ActiveRuleParamDto;
+import org.sonar.db.qualityprofile.QProfileDto;
+
+@FunctionalInterface
+public interface DescendantProfilesSupplier {
+
+  Result get(Collection<QProfileDto> profiles, Collection<Integer> ruleIds);
+
+  final class Result {
+    private final Collection<QProfileDto> profiles;
+    private final Collection<ActiveRuleDto> activeRules;
+    private final Collection<ActiveRuleParamDto> activeRuleParams;
+
+    public Result(Collection<QProfileDto> profiles, Collection<ActiveRuleDto> activeRules, Collection<ActiveRuleParamDto> activeRuleParams) {
+      this.profiles = profiles;
+      this.activeRules = activeRules;
+      this.activeRuleParams = activeRuleParams;
+    }
+
+    public Collection<QProfileDto> getProfiles() {
+      return profiles;
+    }
+
+    public Collection<ActiveRuleDto> getActiveRules() {
+      return activeRules;
+    }
+
+    public Collection<ActiveRuleParamDto> getActiveRuleParams() {
+      return activeRuleParams;
+    }
+  }
+}
index 3cf258cfcd91eb0566009a3416a50a27ef781bd1..c96a3c75ac41444ca189d6646278aa4d5e94593b 100644 (file)
@@ -21,7 +21,6 @@ package org.sonar.server.qualityprofile;
 
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ListMultimap;
-import com.google.common.collect.Maps;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
@@ -41,6 +40,7 @@ import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
 import static java.util.Objects.requireNonNull;
 import static org.sonar.core.util.stream.MoreCollectors.index;
+import static org.sonar.core.util.stream.MoreCollectors.toArrayList;
 import static org.sonar.core.util.stream.MoreCollectors.uniqueIndex;
 import static org.sonar.server.ws.WsUtils.checkRequest;
 
@@ -61,8 +61,8 @@ class RuleActivationContext {
   private final ListMultimap<String, QProfileDto> profilesByParentUuid = ArrayListMultimap.create();
 
   // The rules/active rules involved in the group of activations/de-activations
-  private final Map<Integer, RuleWrapper> rulesById;
-  private final Map<ActiveRuleKey, ActiveRuleWrapper> activeRulesByKey;
+  private final Map<Integer, RuleWrapper> rulesById = new HashMap<>();
+  private final Map<ActiveRuleKey, ActiveRuleWrapper> activeRulesByKey = new HashMap<>();
 
   // Cursors used to move in the rules and in the tree of profiles.
 
@@ -74,10 +74,13 @@ class RuleActivationContext {
   private ActiveRuleWrapper currentActiveRule;
   private ActiveRuleWrapper currentParentActiveRule;
 
+  private boolean descendantsLoaded = false;
+  private final DescendantProfilesSupplier descendantProfilesSupplier;
+
   private RuleActivationContext(Builder builder) {
     this.date = builder.date;
+    this.descendantProfilesSupplier = builder.descendantProfilesSupplier;
 
-    this.rulesById = Maps.newHashMapWithExpectedSize(builder.rules.size());
     ListMultimap<Integer, RuleParamDto> paramsByRuleId = builder.ruleParams.stream().collect(index(RuleParamDto::getRuleId));
     for (RuleDefinitionDto rule : builder.rules) {
       RuleWrapper wrapper = new RuleWrapper(rule, paramsByRuleId.get(rule.getId()));
@@ -85,16 +88,22 @@ class RuleActivationContext {
     }
 
     this.baseRulesProfile = builder.baseRulesProfile;
-    for (QProfileDto profile : builder.profiles) {
+    register(builder.profiles);
+    register(builder.activeRules, builder.activeRuleParams);
+  }
+
+  private void register(Collection<QProfileDto> profiles) {
+    for (QProfileDto profile : profiles) {
       profilesByUuid.put(profile.getKee(), profile);
       if (profile.getParentKee() != null) {
         profilesByParentUuid.put(profile.getParentKee(), profile);
       }
     }
+  }
 
-    this.activeRulesByKey = Maps.newHashMapWithExpectedSize(builder.activeRules.size());
-    ListMultimap<Integer, ActiveRuleParamDto> paramsByActiveRuleId = builder.activeRuleParams.stream().collect(index(ActiveRuleParamDto::getActiveRuleId));
-    for (ActiveRuleDto activeRule : builder.activeRules) {
+  private void register(Collection<ActiveRuleDto> activeRules, Collection<ActiveRuleParamDto> activeRuleParams) {
+    ListMultimap<Integer, ActiveRuleParamDto> paramsByActiveRuleId = activeRuleParams.stream().collect(index(ActiveRuleParamDto::getActiveRuleId));
+    for (ActiveRuleDto activeRule : activeRules) {
       ActiveRuleWrapper wrapper = new ActiveRuleWrapper(activeRule, paramsByActiveRuleId.get(activeRule.getId()));
       this.activeRulesByKey.put(activeRule.getKey(), wrapper);
     }
@@ -173,11 +182,25 @@ class RuleActivationContext {
    * The children of {@link #getProfiles()}
    */
   Collection<QProfileDto> getChildProfiles() {
+    loadDescendants();
     return getProfiles().stream()
       .flatMap(p -> profilesByParentUuid.get(p.getKee()).stream())
       .collect(Collectors.toList());
   }
 
+  private void loadDescendants() {
+    if (descendantsLoaded) {
+      return;
+    }
+    Collection<QProfileDto> baseProfiles = profilesByUuid.values().stream()
+      .filter(p -> p.getRulesProfileUuid().equals(baseRulesProfile.getKee()))
+      .collect(toArrayList(profilesByUuid.size()));
+    DescendantProfilesSupplier.Result result = descendantProfilesSupplier.get(baseProfiles, rulesById.keySet());
+    register(result.getProfiles());
+    register(result.getActiveRules(), result.getActiveRuleParams());
+    descendantsLoaded = true;
+  }
+
   /**
    * Move the cursor to the given rule and back to the base profile.
    */
@@ -226,6 +249,7 @@ class RuleActivationContext {
     private Collection<QProfileDto> profiles;
     private Collection<ActiveRuleDto> activeRules;
     private Collection<ActiveRuleParamDto> activeRuleParams;
+    private DescendantProfilesSupplier descendantProfilesSupplier;
 
     Builder setDate(long l) {
       this.date = l;
@@ -266,6 +290,11 @@ class RuleActivationContext {
       return this;
     }
 
+    Builder setDescendantProfilesSupplier(DescendantProfilesSupplier d) {
+      this.descendantProfilesSupplier = d;
+      return this;
+    }
+
     RuleActivationContext build() {
       checkArgument(date > 0, "date is not set");
       requireNonNull(baseRulesProfile, "baseRulesProfile is null");
index 8bef04772ddcff5c4f3a6f992780d148f25c2ae7..835f4153227b91a756f87a3054c393fe8d829fab 100644 (file)
@@ -25,6 +25,7 @@ import java.util.Collection;
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.stream.Stream;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
@@ -51,7 +52,6 @@ import org.sonar.server.user.UserSession;
 import org.sonar.server.util.TypeValidations;
 
 import static com.google.common.base.Preconditions.checkArgument;
-import static java.util.Collections.singleton;
 import static org.sonar.server.ws.WsUtils.checkRequest;
 
 /**
@@ -198,16 +198,16 @@ public class RuleActivator {
         String parentValue = parentActiveRule != null ? parentActiveRule.getParamValue(paramKey) : null;
         String activeRuleValue = activeRule == null ? null : activeRule.getParamValue(paramKey);
         paramValue = context.hasRequestedParamValue(request, paramKey) ?
-          // If the request contains the parameter then we're using either value from request, or parent value, or default value
+        // If the request contains the parameter then we're using either value from request, or parent value, or default value
           firstNonNull(
             context.getRequestedParamValue(request, paramKey),
             parentValue,
             rule.getParamDefaultValue(paramKey))
           // If the request doesn't contain the parameter, then we're using either value in DB, or parent value, or default value
           : firstNonNull(
-          activeRuleValue,
-          parentValue,
-          rule.getParamDefaultValue(paramKey));
+            activeRuleValue,
+            parentValue,
+            rule.getParamDefaultValue(paramKey));
       }
 
       change.setParameter(paramKey, validateParam(ruleParamDto, paramValue));
@@ -356,14 +356,13 @@ public class RuleActivator {
     checkArgument(builtInProfile.isBuiltIn(), "Rules profile with UUID %s is not built-in", builtInProfile.getKee());
 
     RuleActivationContext.Builder builder = new RuleActivationContext.Builder();
+    builder.setDescendantProfilesSupplier(createDescendantProfilesSupplier(dbSession));
 
     // load rules
     List<RuleDefinitionDto> rules = completeWithRules(dbSession, builder, ruleIds);
 
-    // load org profiles
-    List<QProfileDto> aliasedBuiltInProfiles = db.qualityProfileDao().selectQProfilesByRuleProfile(dbSession, builtInProfile);
-    List<QProfileDto> profiles = new ArrayList<>(aliasedBuiltInProfiles);
-    profiles.addAll(db.qualityProfileDao().selectDescendants(dbSession, aliasedBuiltInProfiles));
+    // load org profiles. Their parents are null by nature.
+    List<QProfileDto> profiles = db.qualityProfileDao().selectQProfilesByRuleProfile(dbSession, builtInProfile);
     builder.setProfiles(profiles);
     builder.setBaseProfile(builtInProfile);
 
@@ -371,20 +370,20 @@ public class RuleActivator {
     Collection<String> ruleProfileUuids = Stream
       .concat(Stream.of(builtInProfile.getKee()), profiles.stream().map(QProfileDto::getRulesProfileUuid))
       .collect(MoreCollectors.toHashSet(profiles.size() + 1));
-    completeWithActiveRules(dbSession, builder, rules, ruleProfileUuids);
-
+    completeWithActiveRules(dbSession, builder, ruleIds, ruleProfileUuids);
     return builder.build();
   }
 
   public RuleActivationContext createContextForUserProfile(DbSession dbSession, QProfileDto profile, Collection<Integer> ruleIds) {
     checkArgument(!profile.isBuiltIn(), "Profile with UUID %s is built-in", profile.getKee());
     RuleActivationContext.Builder builder = new RuleActivationContext.Builder();
+    builder.setDescendantProfilesSupplier(createDescendantProfilesSupplier(dbSession));
 
     // load rules
-    List<RuleDefinitionDto> rules = completeWithRules(dbSession, builder, ruleIds);
+    completeWithRules(dbSession, builder, ruleIds);
 
-    // load descendant profiles
-    List<QProfileDto> profiles = new ArrayList<>(db.qualityProfileDao().selectDescendants(dbSession, singleton(profile)));
+    // load profiles
+    List<QProfileDto> profiles = new ArrayList<>();
     profiles.add(profile);
     if (profile.getParentKee() != null) {
       profiles.add(db.qualityProfileDao().selectByUuid(dbSession, profile.getParentKee()));
@@ -396,11 +395,24 @@ public class RuleActivator {
     Collection<String> ruleProfileUuids = profiles.stream()
       .map(QProfileDto::getRulesProfileUuid)
       .collect(MoreCollectors.toHashSet(profiles.size()));
-    completeWithActiveRules(dbSession, builder, rules, ruleProfileUuids);
+    completeWithActiveRules(dbSession, builder, ruleIds, ruleProfileUuids);
 
     return builder.build();
   }
 
+  private DescendantProfilesSupplier createDescendantProfilesSupplier(DbSession dbSession) {
+    return (parents, ruleIds) -> {
+      Collection<QProfileDto> profiles = db.qualityProfileDao().selectDescendants(dbSession, parents);
+      Set<String> ruleProfileUuids = profiles.stream()
+        .map(QProfileDto::getRulesProfileUuid)
+        .collect(MoreCollectors.toHashSet());
+      Collection<ActiveRuleDto> activeRules = db.activeRuleDao().selectByRulesAndRuleProfileUuids(dbSession, ruleIds, ruleProfileUuids);
+      List<Integer> activeRuleIds = activeRules.stream().map(ActiveRuleDto::getId).collect(MoreCollectors.toArrayList(activeRules.size()));
+      List<ActiveRuleParamDto> activeRuleParams = db.activeRuleDao().selectParamsByActiveRuleIds(dbSession, activeRuleIds);
+      return new DescendantProfilesSupplier.Result(profiles, activeRules, activeRuleParams);
+    };
+  }
+
   private List<RuleDefinitionDto> completeWithRules(DbSession dbSession, RuleActivationContext.Builder builder, Collection<Integer> ruleIds) {
     List<RuleDefinitionDto> rules = db.ruleDao().selectDefinitionByIds(dbSession, ruleIds);
     builder.setRules(rules);
@@ -408,8 +420,8 @@ public class RuleActivator {
     return rules;
   }
 
-  private void completeWithActiveRules(DbSession dbSession, RuleActivationContext.Builder builder, Collection<RuleDefinitionDto> rules, Collection<String> ruleProfileUuids) {
-    Collection<ActiveRuleDto> activeRules = db.activeRuleDao().selectByRulesAndRuleProfileUuids(dbSession, rules, ruleProfileUuids);
+  private void completeWithActiveRules(DbSession dbSession, RuleActivationContext.Builder builder, Collection<Integer> ruleIds, Collection<String> ruleProfileUuids) {
+    Collection<ActiveRuleDto> activeRules = db.activeRuleDao().selectByRulesAndRuleProfileUuids(dbSession, ruleIds, ruleProfileUuids);
     builder.setActiveRules(activeRules);
     List<Integer> activeRuleIds = activeRules.stream().map(ActiveRuleDto::getId).collect(MoreCollectors.toArrayList(activeRules.size()));
     builder.setActiveRuleParams(db.activeRuleDao().selectParamsByActiveRuleIds(dbSession, activeRuleIds));