SONAR-6621 Drop support for property sonar.qualitygate to override Quality Gate

sonar-batch/src/main/java/org/sonar/batch/bootstrap/DroppedPropertyChecker.java

@@ -20,27 +20,27 @@
 package org.sonar.batch.bootstrap;
 
 import java.util.Map;
-import org.sonar.api.config.Settings;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
 
 import static java.util.Objects.requireNonNull;
 
 public class DroppedPropertyChecker {
-  private final Settings settings;
-  private final Logger logger;
+
+  private static final Logger LOG = Loggers.get(DroppedPropertyChecker.class);
+
+  private final Map<String, String> settings;
   private final Map<String, String> properties;
 
-  public DroppedPropertyChecker(Settings settings, Map<String, String> properties) {
-    this.settings = requireNonNull(settings);
-    this.logger = Loggers.get(settings.getClass());
-    this.properties = requireNonNull(properties);
+  public DroppedPropertyChecker(Map<String, String> properties, Map<String, String> droppedPropertiesAndMsg) {
+    this.settings = requireNonNull(properties);
+    this.properties = requireNonNull(droppedPropertiesAndMsg);
   }
 
   public void checkDroppedProperties() {
     for (Map.Entry<String, String> entry : properties.entrySet()) {
-      if (settings.hasKey(entry.getKey())) {
-        logger.warn("Property '{}' is not supported any more. {}", entry.getKey(), entry.getValue());
+      if (settings.containsKey(entry.getKey())) {
+        LOG.warn("Property '{}' is not supported any more. {}", entry.getKey(), entry.getValue());
       }
     }
   }

sonar-batch/src/main/java/org/sonar/batch/bootstrap/GlobalSettings.java

@@ -56,7 +56,7 @@ public class GlobalSettings extends Settings {
     this.bootstrapProps = bootstrapProps;
     this.globalReferentials = globalReferentials;
     init();
-    new DroppedPropertyChecker(this, DROPPED_PROPERTIES).checkDroppedProperties();
+    new DroppedPropertyChecker(this.getProperties(), DROPPED_PROPERTIES).checkDroppedProperties();
   }
 
   private void init() {

sonar-batch/src/main/java/org/sonar/batch/scan/ProjectReactorBuilder.java

@@ -20,6 +20,7 @@
 package org.sonar.batch.scan;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
 import java.io.File;
 import java.io.FileFilter;
@@ -49,6 +50,7 @@ import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
 import org.sonar.api.utils.log.Profiler;
 import org.sonar.batch.analysis.AnalysisProperties;
+import org.sonar.batch.bootstrap.DroppedPropertyChecker;
 import org.sonar.batch.util.BatchUtils;
 
 /**
@@ -58,6 +60,13 @@ public class ProjectReactorBuilder {
 
   private static final String INVALID_VALUE_OF_X_FOR_Y = "Invalid value of {0} for {1}";
 
+  /**
+   * A map of dropped properties as key and specific message to display for that property
+   * (what will happen, what should the user do, ...) as a value
+   */
+  private static final Map<String, String> DROPPED_PROPERTIES = ImmutableMap.of(
+    "sonar.qualitygate", "It will be ignored.");
+
   private static final Logger LOG = Loggers.get(ProjectReactorBuilder.class);
 
   /**
@@ -96,7 +105,7 @@ public class ProjectReactorBuilder {
   /**
    * Array of all mandatory properties required for a child project before its properties get merged with its parent ones.
    */
-  protected static final String[] MANDATORY_PROPERTIES_FOR_CHILD = {MODULE_KEY_PROPERTY, CoreProperties.PROJECT_NAME_PROPERTY};
+  private static final String[] MANDATORY_PROPERTIES_FOR_CHILD = {MODULE_KEY_PROPERTY, CoreProperties.PROJECT_NAME_PROPERTY};
 
   /**
    * Properties that must not be passed from the parent project to its children.
@@ -106,25 +115,26 @@ public class ProjectReactorBuilder {
 
   private static final String NON_ASSOCIATED_PROJECT_KEY = "project";
 
-  private final AnalysisProperties taskProps;
+  private final AnalysisProperties analysisProps;
   private final AnalysisMode analysisMode;
   private File rootProjectWorkDir;
 
   public ProjectReactorBuilder(AnalysisProperties props, AnalysisMode analysisMode) {
-    this.taskProps = props;
+    this.analysisProps = props;
     this.analysisMode = analysisMode;
   }
 
   public ProjectReactor execute() {
     Profiler profiler = Profiler.create(LOG).startInfo("Process project properties");
+    new DroppedPropertyChecker(analysisProps.properties(), DROPPED_PROPERTIES).checkDroppedProperties();
     Map<String, Map<String, String>> propertiesByModuleIdPath = new HashMap<>();
-    extractPropertiesByModule(propertiesByModuleIdPath, "", "", taskProps.properties());
+    extractPropertiesByModule(propertiesByModuleIdPath, "", "", analysisProps.properties());
     ProjectDefinition rootProject = defineRootProject(propertiesByModuleIdPath.get(""), null);
     rootProjectWorkDir = rootProject.getWorkDir();
     defineChildren(rootProject, propertiesByModuleIdPath, "");
     cleanAndCheckProjectDefinitions(rootProject);
     // Since task properties are now empty we should add root module properties
-    taskProps.properties().putAll(propertiesByModuleIdPath.get(""));
+    analysisProps.properties().putAll(propertiesByModuleIdPath.get(""));
     profiler.stopDebug();
     return new ProjectReactor(rootProject);
   }
@@ -403,7 +413,7 @@ public class ProjectReactorBuilder {
   @VisibleForTesting
   protected static void mergeParentProperties(Map<String, String> childProps, Map<String, String> parentProps) {
     for (Map.Entry<String, String> entry : parentProps.entrySet()) {
-      String key = (String) entry.getKey();
+      String key = entry.getKey();
       if ((!childProps.containsKey(key) || childProps.get(key).equals(entry.getValue()))
         && !NON_HERITED_PROPERTIES_FOR_CHILD.contains(key)) {
         childProps.put(entry.getKey(), entry.getValue());

sonar-batch/src/main/java/org/sonar/batch/scan/ProjectSettings.java

@@ -19,31 +19,17 @@
  */
 package org.sonar.batch.scan;
 
-import org.sonar.batch.repository.ProjectRepositories;
-
-import org.sonar.batch.analysis.DefaultAnalysisMode;
-import com.google.common.collect.ImmutableMap;
-
-import java.util.Map;
-
 import org.sonar.api.CoreProperties;
 import org.sonar.api.batch.bootstrap.ProjectReactor;
 import org.sonar.api.config.PropertyDefinitions;
 import org.sonar.api.config.Settings;
 import org.sonar.api.utils.MessageException;
-import org.sonar.batch.bootstrap.DroppedPropertyChecker;
+import org.sonar.batch.analysis.DefaultAnalysisMode;
 import org.sonar.batch.bootstrap.GlobalSettings;
+import org.sonar.batch.repository.ProjectRepositories;
 
 public class ProjectSettings extends Settings {
 
-  /**
-   * A map of dropped properties as key and specific message to display for that property
-   * (what will happen, what should the user do, ...) as a value
-   */
-  private static final Map<String, String> DROPPED_PROPERTIES = ImmutableMap.of(
-    "sonar.qualitygate", "It will be ignored."
-    );
-
   private final GlobalSettings globalSettings;
   private final ProjectRepositories projectRepositories;
   private final DefaultAnalysisMode mode;
@@ -56,7 +42,6 @@ public class ProjectSettings extends Settings {
     this.globalSettings = globalSettings;
     this.projectRepositories = projectRepositories;
     init(reactor);
-    new DroppedPropertyChecker(this, DROPPED_PROPERTIES).checkDroppedProperties();
   }
 
   private void init(ProjectReactor reactor) {

sonar-batch/src/test/java/org/sonar/batch/bootstrap/DroppedPropertyCheckerTest.java

@@ -22,7 +22,6 @@ package org.sonar.batch.bootstrap;
 import com.google.common.collect.ImmutableMap;
 import org.junit.Rule;
 import org.junit.Test;
-import org.sonar.api.config.Settings;
 import org.sonar.api.utils.log.LogTester;
 import org.sonar.api.utils.log.LoggerLevel;
 
@@ -38,27 +37,21 @@ public class DroppedPropertyCheckerTest {
 
   @Test
   public void no_log_if_no_dropped_property() {
-    Settings settings = new Settings();
-    settings.setProperty(DROPPED_PROPERTY_1, SOME_VALUE);
-
-    new DroppedPropertyChecker(settings, ImmutableMap.<String, String>of()).checkDroppedProperties();
+    new DroppedPropertyChecker(ImmutableMap.of(DROPPED_PROPERTY_1, SOME_VALUE), ImmutableMap.<String, String>of()).checkDroppedProperties();
 
     assertThat(logTester.logs()).isEmpty();
   }
 
   @Test
   public void no_log_if_settings_does_not_contain_any_dropped_property() {
-    new DroppedPropertyChecker(new Settings(), ImmutableMap.of(DROPPED_PROPERTY_1, DROPPED_PROPERTY_MSG_1)).checkDroppedProperties();
+    new DroppedPropertyChecker(ImmutableMap.<String, String>of(), ImmutableMap.of(DROPPED_PROPERTY_1, DROPPED_PROPERTY_MSG_1)).checkDroppedProperties();
 
     assertThat(logTester.logs()).isEmpty();
   }
 
   @Test
   public void warn_log_if_settings_contains_any_dropped_property() {
-    Settings settings = new Settings();
-    settings.setProperty(DROPPED_PROPERTY_1, SOME_VALUE);
-
-    new DroppedPropertyChecker(settings, ImmutableMap.of(DROPPED_PROPERTY_1, DROPPED_PROPERTY_MSG_1)).checkDroppedProperties();
+    new DroppedPropertyChecker(ImmutableMap.of(DROPPED_PROPERTY_1, SOME_VALUE), ImmutableMap.of(DROPPED_PROPERTY_1, DROPPED_PROPERTY_MSG_1)).checkDroppedProperties();
 
     assertThat(logTester.logs(LoggerLevel.ERROR)).isEmpty();
     assertThat(logTester.logs(LoggerLevel.WARN)).containsOnly("Property '" + DROPPED_PROPERTY_1 + "' is not supported any more. " + DROPPED_PROPERTY_MSG_1);

sonar-batch/src/test/java/org/sonar/batch/scan/ProjectReactorBuilderTest.java

@@ -35,6 +35,8 @@ import org.junit.rules.ExpectedException;
 import org.sonar.api.batch.AnalysisMode;
 import org.sonar.api.batch.bootstrap.ProjectDefinition;
 import org.sonar.api.batch.bootstrap.ProjectReactor;
+import org.sonar.api.utils.log.LogTester;
+import org.sonar.api.utils.log.LoggerLevel;
 import org.sonar.batch.analysis.AnalysisProperties;
 import org.sonar.test.TestUtils;
 
@@ -47,6 +49,9 @@ public class ProjectReactorBuilderTest {
   @Rule
   public ExpectedException thrown = ExpectedException.none();
 
+  @Rule
+  public LogTester logTester = new LogTester();
+
   private AnalysisMode mode;
 
   @Before
@@ -688,6 +693,16 @@ public class ProjectReactorBuilderTest {
       .isEqualTo(new File(TestUtils.getResource(this.getClass(), "multi-module-definitions-same-prefix"), ".sonar/com.foo.project_com.foo.project.module1.feature"));
   }
 
+  @Test
+  public void should_log_a_warning_when_a_dropped_property_is_present() {
+    Map<String, String> props = loadProps("simple-project");
+    props.put("sonar.qualitygate", "somevalue");
+    AnalysisProperties bootstrapProps = new AnalysisProperties(props, null);
+    new ProjectReactorBuilder(bootstrapProps, mode).execute();
+
+    assertThat(logTester.logs(LoggerLevel.WARN)).containsOnly("Property 'sonar.qualitygate' is not supported any more. It will be ignored.");
+  }
+
   private Map<String, String> loadPropsFromFile(String filePath) throws IOException {
     Properties props = new Properties();
     try (FileInputStream fileInputStream = new FileInputStream(TestUtils.getResource(this.getClass(), filePath))) {

sonar-batch/src/test/java/org/sonar/batch/scan/ProjectSettingsTest.java

@@ -19,18 +19,10 @@
  */
 package org.sonar.batch.scan;
 
-import org.sonar.batch.repository.FileData;
-
 import com.google.common.collect.HashBasedTable;
-import com.google.common.collect.Table;
 import com.google.common.collect.ImmutableTable;
-import org.sonar.batch.repository.ProjectRepositories;
-import org.sonar.batch.analysis.DefaultAnalysisMode;
-import org.sonar.batch.bootstrap.GlobalMode;
-import com.google.common.collect.ImmutableMap;
-
+import com.google.common.collect.Table;
 import java.util.Collections;
-
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -41,10 +33,14 @@ import org.sonar.api.batch.bootstrap.ProjectReactor;
 import org.sonar.api.config.PropertyDefinitions;
 import org.sonar.api.utils.MessageException;
 import org.sonar.api.utils.log.LogTester;
-import org.sonar.api.utils.log.LoggerLevel;
+import org.sonar.batch.analysis.DefaultAnalysisMode;
+import org.sonar.batch.bootstrap.GlobalMode;
 import org.sonar.batch.bootstrap.GlobalProperties;
 import org.sonar.batch.bootstrap.GlobalSettings;
 import org.sonar.batch.protocol.input.GlobalRepositories;
+import org.sonar.batch.repository.FileData;
+import org.sonar.batch.repository.ProjectRepositories;
+
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -138,19 +134,9 @@ public class ProjectSettingsTest {
     assertThat(batchSettings.getString("sonar.foo.license.secured")).isEqualTo("bar2");
     thrown.expect(MessageException.class);
     thrown
-      .expectMessage("Access to the secured property 'sonar.foo.secured' is not possible in issues mode. The SonarQube plugin which requires this property must be deactivated in issues mode.");
+      .expectMessage(
+        "Access to the secured property 'sonar.foo.secured' is not possible in issues mode. The SonarQube plugin which requires this property must be deactivated in issues mode.");
     batchSettings.getString("sonar.foo.secured");
   }
 
-  @Test
-  public void should_log_a_warning_when_a_dropper_property_is_present() {
-    GlobalSettings settings = new GlobalSettings(new GlobalProperties(ImmutableMap.of("sonar.qualitygate", "somevalue")), new PropertyDefinitions(), new GlobalRepositories(),
-      globalMode);
-    projectRef = new ProjectRepositories(emptySettings, emptyFileData, null);
-    new ProjectSettings(new ProjectReactor(project), settings, new PropertyDefinitions(), projectRef, mode);
-
-    assertThat(logTester.logs(LoggerLevel.WARN)).containsOnly("Property 'sonar.qualitygate' is not supported any more. It will be ignored.");
-
-  }
-
 }