SONAR-7371 Remove inherited properties from analysis.log

3 files changed, 91 insertions, 26 deletions

diff --git a/sonar-batch/src/main/java/org/sonar/batch/report/AnalysisContextReportPublisher.java b/sonar-batch/src/main/java/org/sonar/batch/report/AnalysisContextReportPublisher.java
index a3d6f2fbb6e..a15cb2b2ed6 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/report/AnalysisContextReportPublisher.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/report/AnalysisContextReportPublisher.java
@@ -25,18 +25,19 @@ import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.StandardOpenOption;
+import java.util.HashMap;
 import java.util.Map;
 import java.util.Properties;
 import java.util.TreeSet;
 import org.sonar.api.batch.AnalysisMode;
 import org.sonar.api.batch.BatchSide;
 import org.sonar.api.batch.bootstrap.ProjectDefinition;
-import org.sonar.api.config.Settings;
 import org.sonar.api.utils.System2;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
 import org.sonar.batch.bootstrap.BatchPluginRepository;
 import org.sonar.batch.protocol.output.BatchReportWriter;
+import org.sonar.batch.repository.ProjectRepositories;
 import org.sonar.core.platform.PluginInfo;
 
 @BatchSide
@@ -49,13 +50,15 @@ public class AnalysisContextReportPublisher {
   private final BatchPluginRepository pluginRepo;
   private final AnalysisMode mode;
   private final System2 system;
+  private final ProjectRepositories projectRepos;
 
   private BatchReportWriter writer;
 
-  public AnalysisContextReportPublisher(AnalysisMode mode, BatchPluginRepository pluginRepo, System2 system) {
+  public AnalysisContextReportPublisher(AnalysisMode mode, BatchPluginRepository pluginRepo, System2 system, ProjectRepositories projectRepos) {
     this.mode = mode;
     this.pluginRepo = pluginRepo;
     this.system = system;
+    this.projectRepos = projectRepos;
   }
 
   public void init(BatchReportWriter writer) {
@@ -101,25 +104,48 @@ public class AnalysisContextReportPublisher {
     }
   }
 
-  public void dumpSettings(ProjectDefinition moduleDefinition, Settings settings) {
+  public void dumpSettings(ProjectDefinition moduleDefinition) {
     if (mode.isIssues()) {
       return;
     }
+
     File analysisLog = writer.getFileStructure().analysisLog();
     try (BufferedWriter fileWriter = Files.newBufferedWriter(analysisLog.toPath(), StandardCharsets.UTF_8, StandardOpenOption.WRITE, StandardOpenOption.APPEND)) {
+      Map<String, String> moduleSpecificProps = collectModuleSpecificProps(moduleDefinition);
       fileWriter.append(String.format("Settings for module: %s", moduleDefinition.getKey())).append('\n');
-      Map<String, String> moduleSettings = settings.getProperties();
-      for (String prop : new TreeSet<>(moduleSettings.keySet())) {
+      for (String prop : new TreeSet<>(moduleSpecificProps.keySet())) {
         if (isSystemProp(prop) || isEnvVariable(prop) || !isSqProp(prop)) {
           continue;
         }
-        fileWriter.append(String.format("  - %s=%s", prop, sensitive(prop) ? "******" : moduleSettings.get(prop))).append('\n');
+        fileWriter.append(String.format("  - %s=%s", prop, sensitive(prop) ? "******" : moduleSpecificProps.get(prop))).append('\n');
       }
     } catch (IOException e) {
       throw new IllegalStateException("Unable to write analysis log", e);
     }
   }
 
+  /**
+   * Only keep props that are not in parent
+   */
+  private Map<String, String> collectModuleSpecificProps(ProjectDefinition moduleDefinition) {
+    Map<String, String> moduleSpecificProps = new HashMap<>();
+    if (projectRepos.moduleExists(moduleDefinition.getKeyWithBranch())) {
+      moduleSpecificProps.putAll(projectRepos.settings(moduleDefinition.getKeyWithBranch()));
+    }
+    ProjectDefinition parent = moduleDefinition.getParent();
+    if (parent == null) {
+      moduleSpecificProps.putAll(moduleDefinition.properties());
+    } else {
+      Map<String, String> parentProps = parent.properties();
+      for (Map.Entry<String, String> entry : moduleDefinition.properties().entrySet()) {
+        if (!parentProps.containsKey(entry.getKey()) || !parentProps.get(entry.getKey()).equals(entry.getValue())) {
+          moduleSpecificProps.put(entry.getKey(), entry.getValue());
+        }
+      }
+    }
+    return moduleSpecificProps;
+  }
+
   private static boolean isSqProp(String propKey) {
     return propKey.startsWith(SONAR_PROP_PREFIX);
   }
diff --git a/sonar-batch/src/main/java/org/sonar/batch/scan/ModuleSettings.java b/sonar-batch/src/main/java/org/sonar/batch/scan/ModuleSettings.java
index 6e58dfbaa63..2176ad347fe 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/scan/ModuleSettings.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/scan/ModuleSettings.java
@@ -46,7 +46,7 @@ public class ModuleSettings extends Settings {
     getEncryption().setPathToSecretKey(batchSettings.getString(CoreProperties.ENCRYPTION_SECRET_KEY_PATH));
 
     init(moduleDefinition, batchSettings);
-    contextReportPublisher.dumpSettings(moduleDefinition, this);
+    contextReportPublisher.dumpSettings(moduleDefinition);
   }
 
   private ModuleSettings init(ProjectDefinition moduleDefinition, GlobalSettings batchSettings) {
@@ -55,9 +55,8 @@ public class ModuleSettings extends Settings {
     return this;
   }
 
-  private void addProjectProperties(ProjectDefinition moduleDefinition, GlobalSettings batchSettings) {
+  private void addProjectProperties(ProjectDefinition def, GlobalSettings batchSettings) {
     addProperties(batchSettings.getProperties());
-    ProjectDefinition def = moduleDefinition;
     do {
       if (projectRepos.moduleExists(def.getKeyWithBranch())) {
         addProperties(projectRepos.settings(def.getKeyWithBranch()));
diff --git a/sonar-batch/src/test/java/org/sonar/batch/report/AnalysisContextReportPublisherTest.java b/sonar-batch/src/test/java/org/sonar/batch/report/AnalysisContextReportPublisherTest.java
index 99418d22dcf..c8fd502117f 100644
--- a/sonar-batch/src/test/java/org/sonar/batch/report/AnalysisContextReportPublisherTest.java
+++ b/sonar-batch/src/test/java/org/sonar/batch/report/AnalysisContextReportPublisherTest.java
@@ -19,6 +19,7 @@
  */
 package org.sonar.batch.report;
 
+import com.google.common.collect.ImmutableMap;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
@@ -30,12 +31,12 @@ import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 import org.sonar.api.batch.AnalysisMode;
 import org.sonar.api.batch.bootstrap.ProjectDefinition;
-import org.sonar.api.config.Settings;
 import org.sonar.api.utils.System2;
 import org.sonar.api.utils.log.LogTester;
 import org.sonar.api.utils.log.LoggerLevel;
 import org.sonar.batch.bootstrap.BatchPluginRepository;
 import org.sonar.batch.protocol.output.BatchReportWriter;
+import org.sonar.batch.repository.ProjectRepositories;
 import org.sonar.core.platform.PluginInfo;
 import org.sonar.updatecenter.common.Version;
 
@@ -61,13 +62,15 @@ public class AnalysisContextReportPublisherTest {
   private AnalysisContextReportPublisher publisher;
   private AnalysisMode analysisMode = mock(AnalysisMode.class);
   private System2 system2;
+  private ProjectRepositories projectRepos;
 
   @Before
   public void prepare() throws Exception {
     logTester.setLevel(LoggerLevel.INFO);
     system2 = mock(System2.class);
     when(system2.properties()).thenReturn(new Properties());
-    publisher = new AnalysisContextReportPublisher(analysisMode, pluginRepo, system2);
+    projectRepos = mock(ProjectRepositories.class);
+    publisher = new AnalysisContextReportPublisher(analysisMode, pluginRepo, system2, projectRepos);
   }
 
   @Test
@@ -89,12 +92,29 @@ public class AnalysisContextReportPublisherTest {
 
     BatchReportWriter writer = new BatchReportWriter(temp.newFolder());
     publisher.init(writer);
-    publisher.dumpSettings(ProjectDefinition.create().setProperty("sonar.projectKey", "foo"), new Settings());
+    publisher.dumpSettings(ProjectDefinition.create().setProperty("sonar.projectKey", "foo"));
 
     assertThat(writer.getFileStructure().analysisLog()).doesNotExist();
   }
 
   @Test
+  public void dumpServerSideProps() throws Exception {
+    logTester.setLevel(LoggerLevel.DEBUG);
+    BatchReportWriter writer = new BatchReportWriter(temp.newFolder());
+    publisher.init(writer);
+
+    when(projectRepos.moduleExists("foo")).thenReturn(true);
+    when(projectRepos.settings("foo")).thenReturn(ImmutableMap.of(COM_FOO, "bar", SONAR_SKIP, "true"));
+
+    publisher.dumpSettings(ProjectDefinition.create()
+      .setProperty("sonar.projectKey", "foo"));
+
+    String content = FileUtils.readFileToString(writer.getFileStructure().analysisLog());
+    assertThat(content).doesNotContain(COM_FOO);
+    assertThat(content).containsOnlyOnce(SONAR_SKIP);
+  }
+
+  @Test
   public void shouldNotDumpSQPropsInSystemProps() throws Exception {
     logTester.setLevel(LoggerLevel.DEBUG);
     BatchReportWriter writer = new BatchReportWriter(temp.newFolder());
@@ -108,11 +128,10 @@ public class AnalysisContextReportPublisherTest {
     assertThat(content).containsOnlyOnce(COM_FOO);
     assertThat(content).doesNotContain(SONAR_SKIP);
 
-    Settings settings = new Settings();
-    settings.setProperty(COM_FOO, "bar");
-    settings.setProperty(SONAR_SKIP, "true");
-
-    publisher.dumpSettings(ProjectDefinition.create().setProperty("sonar.projectKey", "foo"), settings);
+    publisher.dumpSettings(ProjectDefinition.create()
+      .setProperty("sonar.projectKey", "foo")
+      .setProperty(COM_FOO, "bar")
+      .setProperty(SONAR_SKIP, "true"));
 
     content = FileUtils.readFileToString(writer.getFileStructure().analysisLog());
     assertThat(content).containsOnlyOnce(COM_FOO);
@@ -135,10 +154,9 @@ public class AnalysisContextReportPublisherTest {
     assertThat(content).containsOnlyOnce(BIZ);
     assertThat(content).containsSequence(BIZ, FOO);
 
-    Settings settings = new Settings();
-    settings.setProperty("env." + FOO, "BAR");
-
-    publisher.dumpSettings(ProjectDefinition.create().setProperty("sonar.projectKey", "foo"), settings);
+    publisher.dumpSettings(ProjectDefinition.create()
+      .setProperty("sonar.projectKey", "foo")
+      .setProperty("env." + FOO, "BAR"));
 
     content = FileUtils.readFileToString(writer.getFileStructure().analysisLog());
     assertThat(content).containsOnlyOnce(FOO);
@@ -153,15 +171,37 @@ public class AnalysisContextReportPublisherTest {
 
     assertThat(writer.getFileStructure().analysisLog()).exists();
 
-    Settings settings = new Settings();
-    settings.setProperty("sonar.projectKey", "foo");
-    settings.setProperty("sonar.password", "azerty");
-    settings.setProperty("sonar.cpp.license.secured", "AZERTY");
-    publisher.dumpSettings(ProjectDefinition.create().setProperty("sonar.projectKey", "foo"), settings);
+    publisher.dumpSettings(ProjectDefinition.create()
+      .setProperty("sonar.projectKey", "foo")
+      .setProperty("sonar.projectKey", "foo")
+      .setProperty("sonar.password", "azerty")
+      .setProperty("sonar.cpp.license.secured", "AZERTY"));
 
     assertThat(FileUtils.readFileToString(writer.getFileStructure().analysisLog())).containsSequence(
       "sonar.cpp.license.secured=******",
       "sonar.password=******",
       "sonar.projectKey=foo");
   }
+
+  // SONAR-7371
+  @Test
+  public void dontDumpParentProps() throws Exception {
+    logTester.setLevel(LoggerLevel.DEBUG);
+    BatchReportWriter writer = new BatchReportWriter(temp.newFolder());
+    publisher.init(writer);
+
+    ProjectDefinition module = ProjectDefinition.create()
+      .setProperty("sonar.projectKey", "foo")
+      .setProperty(SONAR_SKIP, "true");
+
+    ProjectDefinition.create()
+      .setProperty("sonar.projectKey", "parent")
+      .setProperty(SONAR_SKIP, "true")
+      .addSubProject(module);
+
+    publisher.dumpSettings(module);
+
+    String content = FileUtils.readFileToString(writer.getFileStructure().analysisLog());
+    assertThat(content).doesNotContain(SONAR_SKIP);
+  }
 }