From ca672c1a71098c93f0e7f1b64d73b21c7e6706a9 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Wed, 17 Apr 2019 17:42:50 +0200 Subject: [PATCH] SONARCLOUD-582 split loading of profiles in rule activation --- .../db/qualityprofile/ActiveRuleDao.java | 7 +-- .../db/qualityprofile/ActiveRuleDaoTest.java | 11 ++-- .../DescendantProfilesSupplier.java | 55 +++++++++++++++++++ .../qualityprofile/RuleActivationContext.java | 45 ++++++++++++--- .../server/qualityprofile/RuleActivator.java | 46 ++++++++++------ 5 files changed, 128 insertions(+), 36 deletions(-) create mode 100644 server/sonar-server/src/main/java/org/sonar/server/qualityprofile/DescendantProfilesSupplier.java diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleDao.java index 6607be8dd10..95adfa315fc 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleDao.java @@ -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 selectByRulesAndRuleProfileUuids(DbSession dbSession, Collection rules, Collection ruleProfileUuids) { - if (rules.isEmpty() || ruleProfileUuids.isEmpty()) { + public Collection selectByRulesAndRuleProfileUuids(DbSession dbSession, Collection ruleIds, Collection ruleProfileUuids) { + if (ruleIds.isEmpty() || ruleProfileUuids.isEmpty()) { return emptyList(); } - List 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))); } diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/ActiveRuleDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/ActiveRuleDaoTest.java index f62285355ad..abfb197c980 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/ActiveRuleDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/ActiveRuleDaoTest.java @@ -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 index 00000000000..ef073340e7a --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/DescendantProfilesSupplier.java @@ -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 profiles, Collection ruleIds); + + final class Result { + private final Collection profiles; + private final Collection activeRules; + private final Collection activeRuleParams; + + public Result(Collection profiles, Collection activeRules, Collection activeRuleParams) { + this.profiles = profiles; + this.activeRules = activeRules; + this.activeRuleParams = activeRuleParams; + } + + public Collection getProfiles() { + return profiles; + } + + public Collection getActiveRules() { + return activeRules; + } + + public Collection getActiveRuleParams() { + return activeRuleParams; + } + } +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivationContext.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivationContext.java index 3cf258cfcd9..c96a3c75ac4 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivationContext.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivationContext.java @@ -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 profilesByParentUuid = ArrayListMultimap.create(); // The rules/active rules involved in the group of activations/de-activations - private final Map rulesById; - private final Map activeRulesByKey; + private final Map rulesById = new HashMap<>(); + private final Map 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 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 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 paramsByActiveRuleId = builder.activeRuleParams.stream().collect(index(ActiveRuleParamDto::getActiveRuleId)); - for (ActiveRuleDto activeRule : builder.activeRules) { + private void register(Collection activeRules, Collection activeRuleParams) { + ListMultimap 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 getChildProfiles() { + loadDescendants(); return getProfiles().stream() .flatMap(p -> profilesByParentUuid.get(p.getKee()).stream()) .collect(Collectors.toList()); } + private void loadDescendants() { + if (descendantsLoaded) { + return; + } + Collection 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 profiles; private Collection activeRules; private Collection 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"); 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 8bef04772dd..835f4153227 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 @@ -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 rules = completeWithRules(dbSession, builder, ruleIds); - // load org profiles - List aliasedBuiltInProfiles = db.qualityProfileDao().selectQProfilesByRuleProfile(dbSession, builtInProfile); - List profiles = new ArrayList<>(aliasedBuiltInProfiles); - profiles.addAll(db.qualityProfileDao().selectDescendants(dbSession, aliasedBuiltInProfiles)); + // load org profiles. Their parents are null by nature. + List profiles = db.qualityProfileDao().selectQProfilesByRuleProfile(dbSession, builtInProfile); builder.setProfiles(profiles); builder.setBaseProfile(builtInProfile); @@ -371,20 +370,20 @@ public class RuleActivator { Collection 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 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 rules = completeWithRules(dbSession, builder, ruleIds); + completeWithRules(dbSession, builder, ruleIds); - // load descendant profiles - List profiles = new ArrayList<>(db.qualityProfileDao().selectDescendants(dbSession, singleton(profile))); + // load profiles + List 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 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 profiles = db.qualityProfileDao().selectDescendants(dbSession, parents); + Set ruleProfileUuids = profiles.stream() + .map(QProfileDto::getRulesProfileUuid) + .collect(MoreCollectors.toHashSet()); + Collection activeRules = db.activeRuleDao().selectByRulesAndRuleProfileUuids(dbSession, ruleIds, ruleProfileUuids); + List activeRuleIds = activeRules.stream().map(ActiveRuleDto::getId).collect(MoreCollectors.toArrayList(activeRules.size())); + List activeRuleParams = db.activeRuleDao().selectParamsByActiveRuleIds(dbSession, activeRuleIds); + return new DescendantProfilesSupplier.Result(profiles, activeRules, activeRuleParams); + }; + } + private List completeWithRules(DbSession dbSession, RuleActivationContext.Builder builder, Collection ruleIds) { List 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 rules, Collection ruleProfileUuids) { - Collection activeRules = db.activeRuleDao().selectByRulesAndRuleProfileUuids(dbSession, rules, ruleProfileUuids); + private void completeWithActiveRules(DbSession dbSession, RuleActivationContext.Builder builder, Collection ruleIds, Collection ruleProfileUuids) { + Collection activeRules = db.activeRuleDao().selectByRulesAndRuleProfileUuids(dbSession, ruleIds, ruleProfileUuids); builder.setActiveRules(activeRules); List activeRuleIds = activeRules.stream().map(ActiveRuleDto::getId).collect(MoreCollectors.toArrayList(activeRules.size())); builder.setActiveRuleParams(db.activeRuleDao().selectParamsByActiveRuleIds(dbSession, activeRuleIds)); -- 2.39.5