From afb9a93392a7cdbd237b516b7c38fd7024a9cbf9 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Fri, 28 Apr 2017 17:08:00 +0200 Subject: [PATCH] SONAR-6315 support parent in DefinedQProfileRepository fail if a profile references a non existing parent --- .../qualityprofile/DefinedQProfile.java | 20 ++++++ .../DefinedQProfileRepositoryImpl.java | 65 ++++++++++++++++++- 2 files changed, 82 insertions(+), 3 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/DefinedQProfile.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/DefinedQProfile.java index 278caad1a8e..af4dfff22fe 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/DefinedQProfile.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/DefinedQProfile.java @@ -23,6 +23,8 @@ import com.google.common.collect.ImmutableList; import java.security.MessageDigest; import java.util.ArrayList; import java.util.List; +import javax.annotation.CheckForNull; +import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; import org.sonar.api.profiles.ProfileDefinition; @@ -41,12 +43,14 @@ public final class DefinedQProfile { private final boolean isDefault; private final String loadedTemplateType; private final List activeRules; + private final QProfileName parentQProfileName; private DefinedQProfile(Builder builder, MessageDigest messageDigest) { this.qProfileName = new QProfileName(builder.language, builder.getName()); this.isDefault = builder.declaredDefault || builder.computedDefault; this.loadedTemplateType = computeLoadedTemplateType(this.qProfileName, messageDigest); this.activeRules = ImmutableList.copyOf(builder.activeRules); + this.parentQProfileName = builder.parentName == null ? null : new QProfileName(builder.language, builder.parentName); } private static String computeLoadedTemplateType(QProfileName qProfileName, MessageDigest messageDigest) { @@ -78,12 +82,18 @@ public final class DefinedQProfile { return activeRules; } + @CheckForNull + public QProfileName getParentQProfileName() { + return parentQProfileName; + } + static final class Builder { private String language; private String name; private boolean declaredDefault; private boolean computedDefault; private final List activeRules = new ArrayList<>(); + private String parentName; public Builder setLanguage(String language) { this.language = language; @@ -118,6 +128,16 @@ public final class DefinedQProfile { return this; } + public Builder setParentName(@Nullable String parentName) { + this.parentName = parentName; + return this; + } + + @CheckForNull + public String getParentName() { + return parentName; + } + DefinedQProfile build(MessageDigest messageDigest) { return new DefinedQProfile(this, messageDigest); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/DefinedQProfileRepositoryImpl.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/DefinedQProfileRepositoryImpl.java index 97b76dc8662..4dde1a32ef2 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/DefinedQProfileRepositoryImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/DefinedQProfileRepositoryImpl.java @@ -19,18 +19,23 @@ */ package org.sonar.server.qualityprofile; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ListMultimap; import com.google.common.collect.Multimaps; import java.security.MessageDigest; import java.util.Collection; +import java.util.Comparator; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.function.Function; import javax.annotation.Nullable; import org.apache.commons.codec.digest.DigestUtils; import org.sonar.api.profiles.ProfileDefinition; @@ -128,9 +133,24 @@ public class DefinedQProfileRepositoryImpl implements DefinedQProfileRepository .entrySet() .stream() .filter(DefinedQProfileRepositoryImpl::ensureAtMostOneDeclaredDefault) + .filter(entry -> ensureParentExists(entry.getKey(), entry.getValue())) .collect(MoreCollectors.uniqueIndex(Map.Entry::getKey, entry -> toQualityProfiles(entry.getValue()), buildersByLanguage.size())); } + private boolean ensureParentExists(String language, List builders) { + Set qProfileNames = builders.stream() + .map(DefinedQProfile.Builder::getName) + .collect(MoreCollectors.toSet(builders.size())); + builders + .forEach(builder -> { + String parentName = builder.getParentName(); + checkState(parentName == null || qProfileNames.contains(parentName), + "Quality profile with name %s references Quality profile with name %s as its parent, but it does not exist for language %s", + builder.getName(), builder.getParentName(), language); + }); + return true; + } + /** * Creates {@link DefinedQProfile.Builder} for each unique quality profile name for a given language. * Builders will have the following properties populated: @@ -150,7 +170,7 @@ public class DefinedQProfileRepositoryImpl implements DefinedQProfileRepository for (RulesProfile rulesProfile : rulesProfilesByLanguage.getValue()) { qualityProfileBuildersByName.compute( rulesProfile.getName(), - (name, existingBuilder) -> updateOrCreateBuilder(language, existingBuilder, rulesProfile, name)); + (name, existingBuilder) -> updateOrCreateBuilder(language, existingBuilder, rulesProfile)); } return ImmutableList.copyOf(qualityProfileBuildersByName.values()); } @@ -167,12 +187,13 @@ public class DefinedQProfileRepositoryImpl implements DefinedQProfileRepository return true; } - private static DefinedQProfile.Builder updateOrCreateBuilder(String language, @Nullable DefinedQProfile.Builder existingBuilder, RulesProfile rulesProfile, String name) { + private static DefinedQProfile.Builder updateOrCreateBuilder(String language, @Nullable DefinedQProfile.Builder existingBuilder, RulesProfile rulesProfile) { DefinedQProfile.Builder builder = existingBuilder; if (builder == null) { builder = new DefinedQProfile.Builder() .setLanguage(language) - .setName(name); + .setName(rulesProfile.getName()) + .setParentName(rulesProfile.getParentName()); } Boolean defaultProfile = rulesProfile.getDefaultProfile(); boolean declaredDefault = defaultProfile != null && defaultProfile; @@ -194,7 +215,45 @@ public class DefinedQProfileRepositoryImpl implements DefinedQProfileRepository } MessageDigest md5Digest = DigestUtils.getMd5Digest(); return builders.stream() + .sorted(new SortByParentName(builders)) .map(builder -> builder.build(md5Digest)) .collect(MoreCollectors.toList(builders.size())); } + + @VisibleForTesting + static class SortByParentName implements Comparator { + private final Map buildersByName; + @VisibleForTesting + final Map depthByBuilder; + + @VisibleForTesting + SortByParentName(Collection builders) { + this.buildersByName = builders.stream() + .collect(MoreCollectors.uniqueIndex(DefinedQProfile.Builder::getName, Function.identity(), builders.size())); + this.depthByBuilder = buildDepthByBuilder(buildersByName, builders); + } + + private static Map buildDepthByBuilder(Map buildersByName, Collection builders) { + Map depthByBuilder = new HashMap<>(); + builders.forEach(builder -> depthByBuilder.put(builder.getName(), 0)); + builders + .stream() + .filter(builder -> builder.getParentName() != null) + .forEach(builder -> increaseDepth(buildersByName, depthByBuilder, builder)); + return ImmutableMap.copyOf(depthByBuilder); + } + + private static void increaseDepth(Map buildersByName, Map maps, DefinedQProfile.Builder builder) { + DefinedQProfile.Builder parent = buildersByName.get(builder.getParentName()); + if (parent.getParentName() != null) { + increaseDepth(buildersByName, maps, parent); + } + maps.put(builder.getName(), maps.get(parent.getName()) + 1); + } + + @Override + public int compare(DefinedQProfile.Builder o1, DefinedQProfile.Builder o2) { + return depthByBuilder.getOrDefault(o1.getName(), 0) - depthByBuilder.getOrDefault(o2.getName(), 0); + } + } } -- 2.39.5