From ddc9196bb76ac88fc1a652d0585515eb43190218 Mon Sep 17 00:00:00 2001 From: Julien HENRY Date: Tue, 25 Jun 2019 23:26:20 +0200 Subject: [PATCH] SONAR-12068 Use project settings to load branch config --- .../branch/BranchConfigurationLoader.java | 3 +- .../branch/BranchConfigurationProvider.java | 30 +++------------- .../mediumtest/ScannerMediumTester.java | 3 +- .../BranchConfigurationProviderTest.java | 34 +++++-------------- 4 files changed, 15 insertions(+), 55 deletions(-) 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 149afa02eda..6cfb8aeed4c 100644 --- 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 localSettings, Supplier> remoteSettingsSupplier, ProjectBranches branches, ProjectPullRequests pullRequests); + BranchConfiguration load(Map 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 ab450cbedef..c69c1df0e2b 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 @@ -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> 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> 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 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 2eb7a188e8b..530102bed50 100644 --- 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 localSettings, Supplier> settingsSupplier, ProjectBranches branches, ProjectPullRequests pullRequests) { + public BranchConfiguration load(Map 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 6b79b11276e..b77274c5d1a 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,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 globalPropertiesMap = new HashMap<>();; + private Map 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 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); -- 2.39.5