SONAR-12068 Use project settings to load branch config

diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/BranchConfigurationLoader.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/BranchConfigurationLoader.java
index 149afa02eda7320195457f80f034bfd8f8333472..6cfb8aeed4c25555636074806fb9c456d7788c74 100644 (file)
--- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/BranchConfigurationLoader.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/BranchConfigurationLoader.java
@@ -20,10 +20,9 @@
 package org.sonar.scanner.scan.branch;
 
 import java.util.Map;
-import java.util.function.Supplier;
 import org.sonar.api.scanner.ScannerSide;
 
 @ScannerSide
 public interface BranchConfigurationLoader {
-  BranchConfiguration load(Map<String, String> localSettings, Supplier<Map<String, String>> remoteSettingsSupplier, ProjectBranches branches, ProjectPullRequests pullRequests);
+  BranchConfiguration load(Map<String, String> projectSettings, ProjectBranches branches, ProjectPullRequests pullRequests);
 }

diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/BranchConfigurationProvider.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/BranchConfigurationProvider.java
index ab450cbedef6540be6c764462eb3681eee90a5f7..c69c1df0e2bd16fef340c867116cdaaa4297fcd9 100644 (file)
--- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/BranchConfigurationProvider.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/BranchConfigurationProvider.java
@@ -19,19 +19,12 @@
  */
 package org.sonar.scanner.scan.branch;
 
-import java.util.HashMap;
-import java.util.Map;
-import java.util.function.Supplier;
 import org.picocontainer.annotations.Nullable;
 import org.picocontainer.injectors.ProviderAdapter;
-import org.sonar.api.batch.bootstrap.ProjectDefinition;
-import org.sonar.api.batch.bootstrap.ProjectReactor;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
 import org.sonar.api.utils.log.Profiler;
-import org.sonar.scanner.bootstrap.GlobalConfiguration;
-import org.sonar.scanner.bootstrap.GlobalServerSettings;
-import org.sonar.scanner.scan.ProjectServerSettings;
+import org.sonar.scanner.scan.ProjectConfiguration;
 
 public class BranchConfigurationProvider extends ProviderAdapter {
 
@@ -40,32 +33,17 @@ public class BranchConfigurationProvider extends ProviderAdapter {
 
   private BranchConfiguration branchConfiguration = null;
 
-  public BranchConfiguration provide(@Nullable BranchConfigurationLoader loader, GlobalConfiguration globalConfiguration, ProjectReactor reactor,
-                                     GlobalServerSettings globalServerSettings, ProjectServerSettings projectServerSettings,
-                                     ProjectBranches branches, ProjectPullRequests pullRequests) {
+  public BranchConfiguration provide(@Nullable BranchConfigurationLoader loader, ProjectConfiguration projectConfiguration,
+    ProjectBranches branches, ProjectPullRequests pullRequests) {
     if (branchConfiguration == null) {
       if (loader == null) {
         branchConfiguration = new DefaultBranchConfiguration();
       } else {
         Profiler profiler = Profiler.create(LOG).startInfo(LOG_MSG);
-        Supplier<Map<String, String>> settingsSupplier = createSettingsSupplier(reactor.getRoot(), globalServerSettings, projectServerSettings);
-        branchConfiguration = loader.load(globalConfiguration.getProperties(), settingsSupplier, branches, pullRequests);
+        branchConfiguration = loader.load(projectConfiguration.getProperties(), branches, pullRequests);
         profiler.stopInfo();
       }
     }
     return branchConfiguration;
   }
-
-  private static Supplier<Map<String, String>> createSettingsSupplier(ProjectDefinition root,
-                                                                      GlobalServerSettings globalServerSettings, ProjectServerSettings projectServerSettings) {
-    // we can't get ProjectConfiguration because it creates a circular dependency.
-    // We create our own settings which will only be loaded if needed.
-    return () -> {
-      Map<String, String> settings = new HashMap<>();
-      settings.putAll(globalServerSettings.properties());
-      settings.putAll(projectServerSettings.properties());
-      settings.putAll(root.properties());
-      return settings;
-    };
-  }
 }

diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/ScannerMediumTester.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/ScannerMediumTester.java
index 2eb7a188e8ba9bd2b882a1e81ba054fea14e5560..530102bed50733bfec6a19f6ad21b4f5f4ddc375 100644 (file)
--- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/ScannerMediumTester.java
+++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/ScannerMediumTester.java
@@ -35,7 +35,6 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
-import java.util.function.Supplier;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import org.apache.commons.io.FileUtils;
@@ -471,7 +470,7 @@ public class ScannerMediumTester extends ExternalResource {
 
   private class FakeBranchConfigurationLoader implements BranchConfigurationLoader {
     @Override
-    public BranchConfiguration load(Map<String, String> localSettings, Supplier<Map<String, String>> settingsSupplier, ProjectBranches branches, ProjectPullRequests pullRequests) {
+    public BranchConfiguration load(Map<String, String> projectSettings, ProjectBranches branches, ProjectPullRequests pullRequests) {
       return branchConfiguration;
     }
   }

diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/branch/BranchConfigurationProviderTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/branch/BranchConfigurationProviderTest.java
index 6b79b11276e8fa7572f7d82c18a7ccfd184d3e78..b77274c5d1a725fb1e534809badd5fd9a2779b0f 100644 (file)
--- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/branch/BranchConfigurationProviderTest.java
+++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/branch/BranchConfigurationProviderTest.java
@@ -19,7 +19,6 @@
  */
 package org.sonar.scanner.scan.branch;
 
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.function.Supplier;
@@ -30,27 +29,24 @@ import org.mockito.Captor;
 import org.mockito.MockitoAnnotations;
 import org.sonar.api.batch.bootstrap.ProjectDefinition;
 import org.sonar.api.batch.bootstrap.ProjectReactor;
-import org.sonar.scanner.bootstrap.GlobalConfiguration;
 import org.sonar.scanner.bootstrap.GlobalServerSettings;
+import org.sonar.scanner.scan.ProjectConfiguration;
 import org.sonar.scanner.scan.ProjectServerSettings;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyMap;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 public class BranchConfigurationProviderTest {
   private BranchConfigurationProvider provider = new BranchConfigurationProvider();
-  private GlobalConfiguration globalConfiguration = mock(GlobalConfiguration.class);
+  private ProjectConfiguration projectConfiguration = mock(ProjectConfiguration.class);
   private BranchConfigurationLoader loader = mock(BranchConfigurationLoader.class);
   private BranchConfiguration config = mock(BranchConfiguration.class);
   private ProjectBranches branches = mock(ProjectBranches.class);
   private ProjectPullRequests pullRequests = mock(ProjectPullRequests.class);
   private ProjectReactor reactor = mock(ProjectReactor.class);;
-  private Map<String, String> globalPropertiesMap = new HashMap<>();;
+  private Map<String, String> projectSettings = new HashMap<>();;
   private ProjectDefinition root = mock(ProjectDefinition.class);
   private GlobalServerSettings globalServerSettings = mock(GlobalServerSettings.class);
   private ProjectServerSettings projectServerSettings = mock(ProjectServerSettings.class);
@@ -61,40 +57,28 @@ public class BranchConfigurationProviderTest {
   @Before
   public void setUp() {
     MockitoAnnotations.initMocks(this);
-    when(globalConfiguration.getProperties()).thenReturn(globalPropertiesMap);
+    when(projectConfiguration.getProperties()).thenReturn(projectSettings);
     when(reactor.getRoot()).thenReturn(root);
   }
 
   @Test
   public void should_cache_config() {
-    BranchConfiguration configuration = provider.provide(null, globalConfiguration, reactor, globalServerSettings, projectServerSettings, branches, pullRequests);
-    assertThat(provider.provide(null, globalConfiguration, reactor, globalServerSettings, projectServerSettings, branches, pullRequests)).isSameAs(configuration);
+    BranchConfiguration configuration = provider.provide(null, projectConfiguration, branches, pullRequests);
+    assertThat(provider.provide(null, projectConfiguration, branches, pullRequests)).isSameAs(configuration);
   }
 
   @Test
   public void should_use_loader() {
-    when(loader.load(eq(globalPropertiesMap), any(Supplier.class), eq(branches), eq(pullRequests))).thenReturn(config);
+    when(loader.load(eq(projectSettings), eq(branches), eq(pullRequests))).thenReturn(config);
 
-    BranchConfiguration result = provider.provide(loader, globalConfiguration, reactor, globalServerSettings, projectServerSettings, branches, pullRequests);
+    BranchConfiguration result = provider.provide(loader, projectConfiguration, branches, pullRequests);
 
     assertThat(result).isSameAs(config);
   }
 
-  @Test
-  public void settings_should_include_command_line_args_with_highest_priority() {
-    when(globalConfiguration.getProperties()).thenReturn(Collections.singletonMap("key", "global"));
-    when(projectServerSettings.properties()).thenReturn(Collections.singletonMap("key", "settings"));
-    when(root.properties()).thenReturn(Collections.singletonMap("key", "root"));
-    provider.provide(loader, globalConfiguration, reactor, globalServerSettings, projectServerSettings, branches, pullRequests);
-    verify(loader).load(anyMap(), settingsCaptor.capture(), any(ProjectBranches.class), any(ProjectPullRequests.class));
-
-    Map<String, String> map = settingsCaptor.getValue().get();
-    assertThat(map.get("key")).isEqualTo("root");
-  }
-
   @Test
   public void should_return_default_if_no_loader() {
-    BranchConfiguration result = provider.provide(null, globalConfiguration, reactor, globalServerSettings, projectServerSettings, branches, pullRequests);
+    BranchConfiguration result = provider.provide(null, projectConfiguration, branches, pullRequests);
 
     assertThat(result.targetBranchName()).isNull();
     assertThat(result.branchType()).isEqualTo(BranchType.LONG);