SONAR-4680 Module-level settings are not applied on batch side

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 c4fc318d3fed0d8d6c1cfd159cf2307a800f47f8..163f9dc232c1efc488ee67179aa0421c879c7a78 100644 (file)
--- 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 -> <key,val>
-  private Map<String, Map<String, String>> moduleProperties = Maps.newHashMap();
-
   private final BootstrapSettings bootstrapSettings;
   private final ServerClient client;
   private Map<String, String> 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<Map<String, String>> json = (List<Map<String, String>>) JSONValue.parse(jsonText);
     for (Map<String, String> 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<String, String> map = moduleProperties.get(moduleKey);
-        if (map == null) {
-          map = Maps.newHashMap();
-          moduleProperties.put(moduleKey, map);
-        }
-        map.put(key, value);
-      }
+      setProperty(key, value);
     }
   }
 
-  public Map<String, String> 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 0b37d00077c3e5c419a9c5b594430aca96a06ae8..956711a1d701668a08e8304606af0f8610f53c38 100644 (file)
--- 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<String, String> moduleProps = batchSettings.getModuleProperties(projectKey);
-    if (moduleProps != null) {
-      for (Map.Entry<String, String> 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<Map<String, String>> json = (List<Map<String, String>>) JSONValue.parse(jsonText);
+    for (Map<String, String> 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 706413bc21dc223332b12be9a6f3ecdc9080414e..e889ff0d845b6857b60f6ebfb498c4a1994cf721 100644 (file)
--- 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<String, String> 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 34b5b88280a37c0db46c5fd992e4ccf30c4f7b12..9df1e2aace3e0f9f40dcaf45b3682dd7246bd9c3 100644 (file)
--- 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");
   }
 }