SONAR-10202 Can't override Long-lived branch property regex from the scanner side when project-specific regex is set on server side

2 files changed, 56 insertions, 33 deletions

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 276ba411c28..3441b0ade75 100644
--- 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
@@ -22,9 +22,11 @@ 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.ProjectKey;
+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;
@@ -38,14 +40,14 @@ public class BranchConfigurationProvider extends ProviderAdapter {
 
   private BranchConfiguration branchConfiguration = null;
 
-  public BranchConfiguration provide(@Nullable BranchConfigurationLoader loader, GlobalConfiguration globalConfiguration, ProjectKey projectKey,
+  public BranchConfiguration provide(@Nullable BranchConfigurationLoader loader, GlobalConfiguration globalConfiguration, ProjectReactor reactor,
     SettingsLoader settingsLoader, 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(globalConfiguration, projectKey, settingsLoader);
+        Supplier<Map<String, String>> settingsSupplier = createSettingsSupplier(globalConfiguration, reactor.getRoot(), settingsLoader);
         branchConfiguration = loader.load(globalConfiguration.getProperties(), settingsSupplier, branches, pullRequests);
         profiler.stopInfo();
       }
@@ -53,11 +55,14 @@ public class BranchConfigurationProvider extends ProviderAdapter {
     return branchConfiguration;
   }
 
-  private static Supplier<Map<String, String>> createSettingsSupplier(GlobalConfiguration globalConfiguration, ProjectKey projectKey, SettingsLoader settingsLoader) {
+  private static Supplier<Map<String, String>> createSettingsSupplier(GlobalConfiguration globalConfiguration, ProjectDefinition root, SettingsLoader settingsLoader) {
+    // we can't get ProjectSettings 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(globalConfiguration.getProperties());
-      settings.putAll(settingsLoader.load(projectKey.get()));
+      settings.putAll(settingsLoader.load(root.getKeyWithBranch()));
+      settings.putAll(root.properties());
       return settings;
     };
   }
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 a8fbc75cc8b..ba55c21b7c6 100644
--- 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,66 +19,84 @@
  */
 package org.sonar.scanner.scan.branch;
 
+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.anyString;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.function.Supplier;
+
 import org.junit.Before;
 import org.junit.Test;
-import org.sonar.api.batch.bootstrap.ProjectKey;
+import org.mockito.ArgumentCaptor;
+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.repository.settings.SettingsLoader;
 
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyString;
-import static org.mockito.ArgumentMatchers.eq;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-
 public class BranchConfigurationProviderTest {
   private BranchConfigurationProvider provider = new BranchConfigurationProvider();
-  private GlobalConfiguration globalConfiguration;
-  private BranchConfigurationLoader loader;
-  private BranchConfiguration config;
-  private ProjectBranches branches;
-  private ProjectPullRequests pullRequests;
-  private ProjectKey projectKey;
-  private Map<String, String> globalPropertiesMap;
+  private GlobalConfiguration globalConfiguration = mock(GlobalConfiguration.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> remoteProjectSettings;
-  private SettingsLoader settingsLoader;
+  private ProjectDefinition root = mock(ProjectDefinition.class);
+  private SettingsLoader settingsLoader = mock(SettingsLoader.class);
+
+  @Captor
+  private ArgumentCaptor<Supplier<Map<String, String>>> settingsCaptor;
 
   @Before
   public void setUp() {
-    globalConfiguration = mock(GlobalConfiguration.class);
-    loader = mock(BranchConfigurationLoader.class);
-    config = mock(BranchConfiguration.class);
-    branches = mock(ProjectBranches.class);
-    pullRequests = mock(ProjectPullRequests.class);
-    settingsLoader = mock(SettingsLoader.class);
-    projectKey = mock(ProjectKey.class);
-    globalPropertiesMap = new HashMap<>();
+    MockitoAnnotations.initMocks(this);
     when(globalConfiguration.getProperties()).thenReturn(globalPropertiesMap);
     when(settingsLoader.load(anyString())).thenReturn(remoteProjectSettings);
+    when(reactor.getRoot()).thenReturn(root);
   }
 
   @Test
   public void should_cache_config() {
-    BranchConfiguration configuration = provider.provide(null, globalConfiguration, projectKey, settingsLoader, branches, pullRequests);
-    assertThat(provider.provide(null, globalConfiguration, projectKey, settingsLoader, branches, pullRequests)).isSameAs(configuration);
+    BranchConfiguration configuration = provider.provide(null, globalConfiguration, reactor, settingsLoader, branches, pullRequests);
+    assertThat(provider.provide(null, globalConfiguration, reactor, settingsLoader, branches, pullRequests)).isSameAs(configuration);
   }
 
   @Test
   public void should_use_loader() {
     when(loader.load(eq(globalPropertiesMap), any(Supplier.class), eq(branches), eq(pullRequests))).thenReturn(config);
 
-    BranchConfiguration result = provider.provide(loader, globalConfiguration, projectKey, settingsLoader, branches, pullRequests);
+    BranchConfiguration result = provider.provide(loader, globalConfiguration, reactor, settingsLoader, 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(settingsLoader.load(anyString())).thenReturn(Collections.singletonMap("key", "settings"));
+    when(root.properties()).thenReturn(Collections.singletonMap("key", "root"));
+    provider.provide(loader, globalConfiguration, reactor, settingsLoader, 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, projectKey, settingsLoader, branches, pullRequests);
+    BranchConfiguration result = provider.provide(null, globalConfiguration, reactor, settingsLoader, branches, pullRequests);
 
     assertThat(result.branchTarget()).isNull();
     assertThat(result.branchType()).isEqualTo(BranchType.LONG);