From: Duarte Meneses Date: Wed, 4 Apr 2018 14:27:48 +0000 (+0200) Subject: SONAR-10202 Can't override Long-lived branch property regex from the scanner side... X-Git-Tag: 7.5~1412 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=b38ba09393410b3511a00c76b6a788d74bf10a47;p=sonarqube.git SONAR-10202 Can't override Long-lived branch property regex from the scanner side when project-specific regex is set on server side --- 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> settingsSupplier = createSettingsSupplier(globalConfiguration, projectKey, settingsLoader); + Supplier> 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> createSettingsSupplier(GlobalConfiguration globalConfiguration, ProjectKey projectKey, SettingsLoader settingsLoader) { + private static Supplier> 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 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 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 globalPropertiesMap = new HashMap<>();; private Map remoteProjectSettings; - private SettingsLoader settingsLoader; + private ProjectDefinition root = mock(ProjectDefinition.class); + private SettingsLoader settingsLoader = mock(SettingsLoader.class); + + @Captor + private ArgumentCaptor>> 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 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);