From 39494a3895c43b1b00768e7f3a23633d93491e22 Mon Sep 17 00:00:00 2001 From: Julien HENRY Date: Wed, 16 Mar 2016 18:14:01 +0100 Subject: [PATCH] SONAR-7371 Remove inherited properties from analysis.log --- .../AnalysisContextReportPublisher.java | 38 ++++++++-- .../org/sonar/batch/scan/ModuleSettings.java | 5 +- .../AnalysisContextReportPublisherTest.java | 74 ++++++++++++++----- 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 moduleSpecificProps = collectModuleSpecificProps(moduleDefinition); fileWriter.append(String.format("Settings for module: %s", moduleDefinition.getKey())).append('\n'); - Map 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 collectModuleSpecificProps(ProjectDefinition moduleDefinition) { + Map 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 parentProps = parent.properties(); + for (Map.Entry 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,11 +92,28 @@ 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); @@ -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); + } } -- 2.39.5