From 9592ae6519f11fe3906d01e4b670fdaf403b7004 Mon Sep 17 00:00:00 2001 From: Julien HENRY Date: Mon, 16 Sep 2013 15:57:14 +0200 Subject: [PATCH] SONAR-4680 Module-level settings are not applied on batch side --- .../sonar/batch/bootstrap/BatchSettings.java | 35 ++++------------ .../org/sonar/batch/scan/ModuleSettings.java | 24 ++++++++--- .../batch/bootstrap/BatchSettingsTest.java | 17 +------- .../sonar/batch/scan/ModuleSettingsTest.java | 42 +++++++++---------- 4 files changed, 48 insertions(+), 70 deletions(-) diff --git a/sonar-batch/src/main/java/org/sonar/batch/bootstrap/BatchSettings.java b/sonar-batch/src/main/java/org/sonar/batch/bootstrap/BatchSettings.java index c4fc318d3fe..163f9dc232c 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/bootstrap/BatchSettings.java +++ b/sonar-batch/src/main/java/org/sonar/batch/bootstrap/BatchSettings.java @@ -19,7 +19,6 @@ */ package org.sonar.batch.bootstrap; -import com.google.common.collect.Maps; import org.apache.commons.configuration.Configuration; import org.apache.commons.lang.StringUtils; import org.json.simple.JSONValue; @@ -36,19 +35,16 @@ import java.util.List; import java.util.Map; public class BatchSettings extends Settings { + public static final String BATCH_BOOTSTRAP_PROPERTIES_URL = "/batch_bootstrap/properties"; private Configuration deprecatedConfiguration; private boolean dryRun; - // Keep module settings for initialization of ProjectSettings - // module key -> - private Map> moduleProperties = Maps.newHashMap(); - private final BootstrapSettings bootstrapSettings; private final ServerClient client; private Map savedProperties; public BatchSettings(BootstrapSettings bootstrapSettings, PropertyDefinitions propertyDefinitions, - ServerClient client, Configuration deprecatedConfiguration) { + ServerClient client, Configuration deprecatedConfiguration) { super(propertyDefinitions); getEncryption().setPathToSecretKey(bootstrapSettings.property(CoreProperties.ENCRYPTION_SECRET_KEY_PATH)); this.bootstrapSettings = bootstrapSettings; @@ -69,10 +65,10 @@ public class BatchSettings extends Settings { if (StringUtils.isNotBlank(branch)) { projectKey = String.format("%s:%s", projectKey, branch); } - downloadSettings(client, projectKey); + downloadSettings(projectKey); } else { LoggerFactory.getLogger(BatchSettings.class).info("Load batch settings"); - downloadSettings(client, null); + downloadSettings(null); } addProperties(bootstrapSettings.properties()); @@ -90,37 +86,22 @@ public class BatchSettings extends Settings { this.setProperties(savedProperties); } - private void downloadSettings(ServerClient client, @Nullable String projectKey) { + private void downloadSettings(@Nullable String projectKey) { String url; if (StringUtils.isNotBlank(projectKey)) { - url = "/batch_bootstrap/properties?project=" + projectKey + "&dryRun=" + dryRun; + url = BATCH_BOOTSTRAP_PROPERTIES_URL + "?project=" + projectKey + "&dryRun=" + dryRun; } else { - url = "/batch_bootstrap/properties?dryRun=" + dryRun; + url = BATCH_BOOTSTRAP_PROPERTIES_URL + "?dryRun=" + dryRun; } String jsonText = client.request(url); List> json = (List>) JSONValue.parse(jsonText); for (Map jsonProperty : json) { String key = jsonProperty.get("k"); String value = jsonProperty.get("v"); - String moduleKey = jsonProperty.get("p"); - if (moduleKey == null || projectKey == null || moduleKey.equals(projectKey)) { - setProperty(key, value); - } - if (moduleKey != null) { - Map map = moduleProperties.get(moduleKey); - if (map == null) { - map = Maps.newHashMap(); - moduleProperties.put(moduleKey, map); - } - map.put(key, value); - } + setProperty(key, value); } } - public Map getModuleProperties(String projectKey) { - return moduleProperties.get(projectKey); - } - @Override protected void doOnSetProperty(String key, @Nullable String value) { deprecatedConfiguration.setProperty(key, value); 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 0b37d00077c..956711a1d70 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 @@ -22,18 +22,22 @@ package org.sonar.batch.scan; import com.google.common.collect.Lists; import org.apache.commons.configuration.Configuration; import org.apache.commons.lang.StringUtils; +import org.json.simple.JSONValue; import org.slf4j.LoggerFactory; import org.sonar.api.CoreProperties; import org.sonar.api.batch.bootstrap.ProjectDefinition; import org.sonar.api.config.Settings; import org.sonar.api.utils.SonarException; import org.sonar.batch.bootstrap.BatchSettings; +import org.sonar.batch.bootstrap.ServerClient; import javax.annotation.Nullable; import java.util.List; import java.util.Map; +import static org.sonar.batch.bootstrap.BatchSettings.BATCH_BOOTSTRAP_PROPERTIES_URL; + /** * @since 2.12 */ @@ -41,9 +45,11 @@ public class ModuleSettings extends Settings { private final Configuration deprecatedCommonsConf; private boolean dryRun; + private final ServerClient client; - public ModuleSettings(BatchSettings batchSettings, ProjectDefinition project, Configuration deprecatedCommonsConf) { + public ModuleSettings(BatchSettings batchSettings, ProjectDefinition project, Configuration deprecatedCommonsConf, ServerClient client) { super(batchSettings.getDefinitions()); + this.client = client; getEncryption().setPathToSecretKey(batchSettings.getString(CoreProperties.ENCRYPTION_SECRET_KEY_PATH)); this.dryRun = "true".equals(batchSettings.getString(CoreProperties.DRY_RUN)); @@ -67,11 +73,17 @@ public class ModuleSettings extends Settings { projectKey = String.format("%s:%s", projectKey, branch); } addProperties(batchSettings.getProperties()); - Map moduleProps = batchSettings.getModuleProperties(projectKey); - if (moduleProps != null) { - for (Map.Entry entry : moduleProps.entrySet()) { - setProperty(entry.getKey(), entry.getValue()); - } + downloadSettings(projectKey); + } + + private void downloadSettings(String moduleKey) { + String url = BATCH_BOOTSTRAP_PROPERTIES_URL + "?project=" + moduleKey + "&dryRun=" + dryRun; + String jsonText = client.request(url); + List> json = (List>) JSONValue.parse(jsonText); + for (Map jsonProperty : json) { + String key = jsonProperty.get("k"); + String value = jsonProperty.get("v"); + setProperty(key, value); } } diff --git a/sonar-batch/src/test/java/org/sonar/batch/bootstrap/BatchSettingsTest.java b/sonar-batch/src/test/java/org/sonar/batch/bootstrap/BatchSettingsTest.java index 706413bc21d..e889ff0d845 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/bootstrap/BatchSettingsTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/bootstrap/BatchSettingsTest.java @@ -32,7 +32,6 @@ import org.sonar.api.config.PropertyDefinitions; import org.sonar.api.utils.SonarException; import java.util.Collections; -import java.util.Map; import static org.fest.assertions.Assertions.assertThat; import static org.mockito.Mockito.mock; @@ -146,24 +145,10 @@ public class BatchSettingsTest { assertThat(batchSettings.getString("sonar.foo.license.secured")).isEqualTo("bar2"); thrown.expect(SonarException.class); thrown - .expectMessage("Access to the secured property 'sonar.foo.secured' is not possible in local (dry run) SonarQube analysis. The SonarQube plugin which requires this property must be deactivated in dry run mode."); + .expectMessage("Access to the secured property 'sonar.foo.secured' is not possible in local (dry run) SonarQube analysis. The SonarQube plugin which requires this property must be deactivated in dry run mode."); batchSettings.getString("sonar.foo.secured"); } - @Test - public void should_keep_module_settings_for_later() { - when(client.request("/batch_bootstrap/properties?dryRun=false")).thenReturn(JSON_RESPONSE); - when(client.request("/batch_bootstrap/properties?project=struts&dryRun=false")).thenReturn(REACTOR_JSON_RESPONSE); - - BatchSettings batchSettings = new BatchSettings(bootstrapSettings, new PropertyDefinitions(), client, deprecatedConf); - batchSettings.init(new ProjectReactor(project)); - - Map moduleSettings = batchSettings.getModuleProperties("struts-core"); - - assertThat(moduleSettings).hasSize(1); - assertThat(moduleSettings.get("sonar.java.coveragePlugin")).isEqualTo("cobertura"); - } - @Test public void system_props_should_override_build_props() { when(client.request("/batch_bootstrap/properties?dryRun=false")).thenReturn(JSON_RESPONSE); diff --git a/sonar-batch/src/test/java/org/sonar/batch/scan/ModuleSettingsTest.java b/sonar-batch/src/test/java/org/sonar/batch/scan/ModuleSettingsTest.java index 34b5b88280a..9df1e2aace3 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/scan/ModuleSettingsTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/scan/ModuleSettingsTest.java @@ -30,6 +30,7 @@ import org.sonar.api.batch.bootstrap.ProjectDefinition; import org.sonar.api.config.PropertyDefinitions; import org.sonar.api.utils.SonarException; import org.sonar.batch.bootstrap.BatchSettings; +import org.sonar.batch.bootstrap.ServerClient; import java.util.List; @@ -42,6 +43,8 @@ public class ModuleSettingsTest { @Rule public ExpectedException thrown = ExpectedException.none(); + ServerClient client = mock(ServerClient.class); + @Test public void testOrderedProjects() { ProjectDefinition grandParent = ProjectDefinition.create(); @@ -61,18 +64,17 @@ public class ModuleSettingsTest { BatchSettings batchSettings = mock(BatchSettings.class); when(batchSettings.getDefinitions()).thenReturn(new PropertyDefinitions()); when(batchSettings.getProperties()).thenReturn(ImmutableMap.of( - "overridding", "batch", - "on-batch", "true" - )); - when(batchSettings.getModuleProperties("struts-core")).thenReturn(ImmutableMap.of( - "on-module", "true", - "overridding", "module" - )); + "overridding", "batch", + "on-batch", "true" + )); + when(client.request("/batch_bootstrap/properties?project=struts-core&dryRun=false")) + .thenReturn("[{\"k\":\"on-module\",\"v\":\"true\"}," + + "{\"k\":\"overridding\",\"v\":\"module\"}]"); ProjectDefinition module = ProjectDefinition.create().setKey("struts-core"); Configuration deprecatedConf = new PropertiesConfiguration(); - ModuleSettings moduleSettings = new ModuleSettings(batchSettings, module, deprecatedConf); + ModuleSettings moduleSettings = new ModuleSettings(batchSettings, module, deprecatedConf, client); assertThat(moduleSettings.getString("overridding")).isEqualTo("module"); assertThat(moduleSettings.getString("on-batch")).isEqualTo("true"); @@ -88,16 +90,15 @@ public class ModuleSettingsTest { BatchSettings batchSettings = mock(BatchSettings.class); when(batchSettings.getDefinitions()).thenReturn(new PropertyDefinitions()); when(batchSettings.getProperties()).thenReturn(ImmutableMap.of( - "sonar.foo.secured", "bar" - )); - when(batchSettings.getModuleProperties("struts-core")).thenReturn(ImmutableMap.of( - "sonar.foo.license.secured", "bar2" - )); + "sonar.foo.secured", "bar" + )); + when(client.request("/batch_bootstrap/properties?project=struts-core&dryRun=false")) + .thenReturn("[{\"k\":\"sonar.foo.license.secured\",\"v\":\"bar2\"}]"); ProjectDefinition module = ProjectDefinition.create().setKey("struts-core"); Configuration deprecatedConf = new PropertiesConfiguration(); - ModuleSettings moduleSettings = new ModuleSettings(batchSettings, module, deprecatedConf); + ModuleSettings moduleSettings = new ModuleSettings(batchSettings, module, deprecatedConf, client); assertThat(moduleSettings.getString("sonar.foo.license.secured")).isEqualTo("bar2"); assertThat(moduleSettings.getString("sonar.foo.secured")).isEqualTo("bar"); @@ -109,22 +110,21 @@ public class ModuleSettingsTest { when(batchSettings.getDefinitions()).thenReturn(new PropertyDefinitions()); when(batchSettings.getString(CoreProperties.DRY_RUN)).thenReturn("true"); when(batchSettings.getProperties()).thenReturn(ImmutableMap.of( - "sonar.foo.secured", "bar" - )); - when(batchSettings.getModuleProperties("struts-core")).thenReturn(ImmutableMap.of( - "sonar.foo.license.secured", "bar2" - )); + "sonar.foo.secured", "bar" + )); + when(client.request("/batch_bootstrap/properties?project=struts-core&dryRun=true")) + .thenReturn("[{\"k\":\"sonar.foo.license.secured\",\"v\":\"bar2\"}]"); ProjectDefinition module = ProjectDefinition.create().setKey("struts-core"); Configuration deprecatedConf = new PropertiesConfiguration(); - ModuleSettings moduleSettings = new ModuleSettings(batchSettings, module, deprecatedConf); + ModuleSettings moduleSettings = new ModuleSettings(batchSettings, module, deprecatedConf, client); assertThat(moduleSettings.getString("sonar.foo.license.secured")).isEqualTo("bar2"); thrown.expect(SonarException.class); thrown - .expectMessage("Access to the secured property 'sonar.foo.secured' is not possible in local (dry run) SonarQube analysis. The SonarQube plugin which requires this property must be deactivated in dry run mode."); + .expectMessage("Access to the secured property 'sonar.foo.secured' is not possible in local (dry run) SonarQube analysis. The SonarQube plugin which requires this property must be deactivated in dry run mode."); moduleSettings.getString("sonar.foo.secured"); } } -- 2.39.5