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

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 b8e25cb324eb970710594305222601f65a8b51c4..6c91f2f6614e12fd4804875196f07349e53c7988 100644 (file)
--- 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 285a33e81cfb3609dde5d7ab6c5da53a81a33ec7..e9b5a5bd05441ff47606a4829f5a6c8aac0ca783 100644 (file)
--- 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<String> 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<String> 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 720e39e992be3a59e809e981c3bf0e740446ce78..2fa46059167968b899755d4cf44527c114d4d542 100644 (file)
--- 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 0de22581f630f99cdfe5eb2907008502b8a81131..8a5dada204cd467d4060e7c9dc4cd24f3639f01c 100644 (file)
--- 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 1cb2a037206d6420560e0f9a323f8c5cbe75b941..9a3c0a86158b2101074024ef5c33d26dad09dd49 100644 (file)
--- 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);