From d41338e67fd95bfe18aa7bcf10e50b98898928c2 Mon Sep 17 00:00:00 2001 From: Janos Gyerik Date: Fri, 20 Oct 2017 10:23:38 +0200 Subject: [PATCH] SONAR-9969 Change the validation logic of the new branch parameter (#2666) * Fail-fast when branch name or target used without branch plugin * Allow missing BranchParamsValidator, improve the check for missing * Reuse branches doc link constant --- .../sonar/core/config/ScannerProperties.java | 2 ++ .../scanner/scan/ProjectReactorValidator.java | 32 ++++++++++++++--- .../scanner/scan/ProjectScanContainer.java | 3 +- .../mediumtest/fs/FileSystemMediumTest.java | 8 +++-- .../scan/ProjectReactorValidatorTest.java | 36 ++++++++++++++++++- 5 files changed, 71 insertions(+), 10 deletions(-) diff --git a/sonar-core/src/main/java/org/sonar/core/config/ScannerProperties.java b/sonar-core/src/main/java/org/sonar/core/config/ScannerProperties.java index b8e25cb324e..6c91f2f6614 100644 --- a/sonar-core/src/main/java/org/sonar/core/config/ScannerProperties.java +++ b/sonar-core/src/main/java/org/sonar/core/config/ScannerProperties.java @@ -30,6 +30,8 @@ import static org.sonar.api.PropertyType.BOOLEAN; public class ScannerProperties { + public static final String BRANCHES_DOC_LINK = "https://redirect.sonarsource.com/doc/branches.html"; + public static final String BRANCH_NAME = "sonar.branch.name"; public static final String BRANCH_TARGET = "sonar.branch.target"; public static final String ORGANIZATION = "sonar.organization"; diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/ProjectReactorValidator.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/ProjectReactorValidator.java index 285a33e81cf..e9b5a5bd054 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/ProjectReactorValidator.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/ProjectReactorValidator.java @@ -21,6 +21,7 @@ package org.sonar.scanner.scan; import com.google.common.base.Joiner; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import javax.annotation.Nullable; import org.apache.commons.lang.StringUtils; @@ -29,8 +30,9 @@ import org.sonar.api.batch.bootstrap.ProjectDefinition; import org.sonar.api.batch.bootstrap.ProjectReactor; import org.sonar.api.utils.MessageException; import org.sonar.core.component.ComponentKeys; +import org.sonar.core.config.ScannerProperties; +import org.sonar.scanner.bootstrap.GlobalConfiguration; import org.sonar.scanner.scan.branch.BranchParamsValidator; -import org.sonar.scanner.scan.branch.DefaultBranchParamsValidator; /** * This class aims at validating project reactor @@ -38,15 +40,20 @@ import org.sonar.scanner.scan.branch.DefaultBranchParamsValidator; */ public class ProjectReactorValidator { private final AnalysisMode mode; + private final GlobalConfiguration settings; + + // null = branch plugin is not available + @Nullable private final BranchParamsValidator branchParamsValidator; - public ProjectReactorValidator(AnalysisMode mode, BranchParamsValidator branchParamsValidator) { + public ProjectReactorValidator(AnalysisMode mode, GlobalConfiguration settings, @Nullable BranchParamsValidator branchParamsValidator) { this.mode = mode; + this.settings = settings; this.branchParamsValidator = branchParamsValidator; } - public ProjectReactorValidator(AnalysisMode mode) { - this(mode, new DefaultBranchParamsValidator()); + public ProjectReactorValidator(AnalysisMode mode, GlobalConfiguration settings) { + this(mode, settings, null); } public void validate(ProjectReactor reactor) { @@ -62,7 +69,13 @@ public class ProjectReactorValidator { String deprecatedBranchName = reactor.getRoot().getBranch(); - branchParamsValidator.validate(validationMessages, deprecatedBranchName); + if (branchParamsValidator != null) { + // branch plugin is present + branchParamsValidator.validate(validationMessages, deprecatedBranchName); + } else { + validateBranchParamsWhenPluginAbsent(validationMessages); + } + validateBranch(validationMessages, deprecatedBranchName); if (!validationMessages.isEmpty()) { @@ -70,6 +83,15 @@ public class ProjectReactorValidator { } } + private void validateBranchParamsWhenPluginAbsent(List validationMessages) { + for (String param : Arrays.asList(ScannerProperties.BRANCH_NAME, ScannerProperties.BRANCH_TARGET)) { + if (StringUtils.isNotEmpty(settings.get(param).orElse(null))) { + validationMessages.add(String.format("To use the property \"%s\", the branch plugin is required but not installed. " + + "See the documentation of branch support: %s.", param, ScannerProperties.BRANCHES_DOC_LINK)); + } + } + } + private static void validateModuleIssuesMode(ProjectDefinition moduleDef, List validationMessages) { if (!ComponentKeys.isValidModuleKeyIssuesMode(moduleDef.getKey())) { validationMessages.add(String.format("\"%s\" is not a valid project or module key. " diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/ProjectScanContainer.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/ProjectScanContainer.java index 720e39e992b..2fa46059167 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/ProjectScanContainer.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/ProjectScanContainer.java @@ -247,7 +247,8 @@ public class ProjectScanContainer extends ComponentContainer { String branch = tree.root().definition().getBranch(); if (branch != null) { LOG.info("Branch key: {}", branch); - LOG.warn("The use of \"sonar.branch\" is deprecated and replaced by \"sonar.branch.name\". See https://redirect.sonarsource.com/doc/branches.html."); + LOG.warn("The use of \"sonar.branch\" is deprecated and replaced by \"{}\". See {}.", + ScannerProperties.BRANCH_NAME, ScannerProperties.BRANCHES_DOC_LINK); } String branchName = props.property(ScannerProperties.BRANCH_NAME); diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/fs/FileSystemMediumTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/fs/FileSystemMediumTest.java index 0de22581f63..8a5dada204c 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/fs/FileSystemMediumTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/fs/FileSystemMediumTest.java @@ -174,14 +174,16 @@ public class FileSystemMediumTest { File srcDir = new File(baseDir, "src"); assertThat(srcDir.mkdir()).isTrue(); + // Using sonar.branch.name when the branch plugin is not installed is an error. + // IllegalStateException is expected here, because this test is in a bit artificial, + // the fail-fast mechanism in the scanner should have prevented reaching this point. + thrown.expect(IllegalStateException.class); + tester.newTask() .properties(builder .put("sonar.sources", "src") .build()) .execute(); - - assertThat(logs.getAllAsString()).contains("Project key: com.foo.project"); - assertThat(logs.getAllAsString()).contains("Branch name: my-branch, type: long living"); } @Test diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/ProjectReactorValidatorTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/ProjectReactorValidatorTest.java index 1cb2a037206..9a3c0a86158 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/ProjectReactorValidatorTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/ProjectReactorValidatorTest.java @@ -19,6 +19,7 @@ */ package org.sonar.scanner.scan; +import java.util.Optional; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -28,7 +29,11 @@ import org.sonar.api.batch.AnalysisMode; import org.sonar.api.batch.bootstrap.ProjectDefinition; import org.sonar.api.batch.bootstrap.ProjectReactor; import org.sonar.api.utils.MessageException; +import org.sonar.core.config.ScannerProperties; +import org.sonar.scanner.bootstrap.GlobalConfiguration; +import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -39,11 +44,14 @@ public class ProjectReactorValidatorTest { private AnalysisMode mode; private ProjectReactorValidator validator; + private GlobalConfiguration settings; @Before public void prepare() { mode = mock(AnalysisMode.class); - validator = new ProjectReactorValidator(mode); + settings = mock(GlobalConfiguration.class); + when(settings.get(anyString())).thenReturn(Optional.empty()); + validator = new ProjectReactorValidator(mode, settings); } @Test @@ -153,6 +161,32 @@ public class ProjectReactorValidatorTest { validator.validate(reactor); } + @Test + public void fail_when_branch_name_is_specified_but_branch_plugin_not_present() { + ProjectDefinition def = ProjectDefinition.create().setProperty(CoreProperties.PROJECT_KEY_PROPERTY, "foo"); + ProjectReactor reactor = new ProjectReactor(def); + + when(settings.get(eq(ScannerProperties.BRANCH_NAME))).thenReturn(Optional.of("feature1")); + + thrown.expect(MessageException.class); + thrown.expectMessage("the branch plugin is required but not installed"); + + validator.validate(reactor); + } + + @Test + public void fail_when_branch_target_is_specified_but_branch_plugin_not_present() { + ProjectDefinition def = ProjectDefinition.create().setProperty(CoreProperties.PROJECT_KEY_PROPERTY, "foo"); + ProjectReactor reactor = new ProjectReactor(def); + + when(settings.get(eq(ScannerProperties.BRANCH_TARGET))).thenReturn(Optional.of("feature1")); + + thrown.expect(MessageException.class); + thrown.expectMessage("the branch plugin is required but not installed"); + + validator.validate(reactor); + } + private ProjectReactor createProjectReactor(String projectKey) { ProjectDefinition def = ProjectDefinition.create().setProperty(CoreProperties.PROJECT_KEY_PROPERTY, projectKey); ProjectReactor reactor = new ProjectReactor(def); -- 2.39.5