SONAR-12026 Security Hotspots are created with resolution "To Review"

diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java
index c2314f17246ee44b9ca1ded13c6afff8e6f43a9d..081c32171384c856dcd92b4ee5ccbde7638819c3 100644 (file)
--- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java
+++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java
@@ -35,6 +35,8 @@ import org.sonar.db.component.KeyType;
 import org.sonar.server.issue.IssueFieldsSetter;
 import org.sonar.server.issue.workflow.IssueWorkflow;
 
+import static java.util.Objects.requireNonNull;
+
 /**
  * Sets the appropriate fields when an issue is :
  * <ul>
@@ -70,20 +72,29 @@ public class IssueLifecycle {
 
   public void initNewOpenIssue(DefaultIssue issue) {
     Preconditions.checkArgument(issue.isFromExternalRuleEngine() != (issue.type() == null), "At this stage issue type should be set for and only for external issues");
+    Rule rule = ruleRepository.getByKey(issue.ruleKey());
     issue.setKey(Uuids.create());
     issue.setCreationDate(changeContext.date());
     issue.setUpdateDate(changeContext.date());
-    issue.setStatus(Issue.STATUS_OPEN);
     issue.setEffort(debtCalculator.calculate(issue));
-    setType(issue);
+    issue.setIsFromHotspot(rule.getType() == RuleType.SECURITY_HOTSPOT);
+    setType(issue, rule);
+    setStatus(issue, rule);
   }
 
-  private void setType(DefaultIssue issue) {
-    if (!issue.isFromExternalRuleEngine()) {
-      Rule rule = ruleRepository.getByKey(issue.ruleKey());
-      issue.setType(rule.getType());
+  private void setType(DefaultIssue issue, Rule rule) {
+    if (issue.isFromExternalRuleEngine()) {
+      return;
+    }
+    issue.setType(requireNonNull(rule.getType(), "No rule type"));
+  }
+
+  private void setStatus(DefaultIssue issue, Rule rule) {
+    if (issue.isFromExternalRuleEngine() || rule.getType() != RuleType.SECURITY_HOTSPOT) {
+      issue.setStatus(Issue.STATUS_OPEN);
+    } else {
+      issue.setStatus(Issue.STATUS_TO_REVIEW);
     }
-    issue.setIsFromHotspot(issue.type() == RuleType.SECURITY_HOTSPOT);
   }
 
   public void copyExistingOpenIssueFromLongLivingBranch(DefaultIssue raw, DefaultIssue base, String fromLongBranchName) {
@@ -148,13 +159,15 @@ public class IssueLifecycle {
 
   public void mergeExistingOpenIssue(DefaultIssue raw, DefaultIssue base) {
     Preconditions.checkArgument(raw.isFromExternalRuleEngine() != (raw.type() == null), "At this stage issue type should be set for and only for external issues");
+    Rule rule = ruleRepository.getByKey(raw.ruleKey());
     raw.setKey(base.key());
     raw.setNew(false);
     if (base.isChanged()) {
       // In case issue was moved from module or folder to the root project
       raw.setChanged(true);
     }
-    setType(raw);
+    raw.setIsFromHotspot(rule.getType() == RuleType.SECURITY_HOTSPOT);
+    setType(raw, rule);
     copyFields(raw, base);
     base.changes().forEach(raw::addChange);
     if (raw.isFromHotspot() != base.isFromHotspot()) {
@@ -164,7 +177,7 @@ public class IssueLifecycle {
     if (raw.isFromHotspot() && !base.isFromHotspot()) {
       // First analysis after rule type was changed to security_hotspot. Issue will be reset to an open hotspot
       updater.setType(raw, RuleType.SECURITY_HOTSPOT, changeContext);
-      updater.setStatus(raw, Issue.STATUS_REOPENED, changeContext);
+      updater.setStatus(raw, Issue.STATUS_TO_REVIEW, changeContext);
       updater.setResolution(raw, null, changeContext);
     }
 

diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycleTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycleTest.java
index ed2882a39bcb6fd17dfd480284c21bba4a52b0f7..b9891a2457f0c0c3cfb25877c9f10d74bf252b56 100644 (file)
--- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycleTest.java
+++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycleTest.java
@@ -23,7 +23,6 @@ import com.google.common.collect.ImmutableMap;
 import java.util.Date;
 import org.junit.Rule;
 import org.junit.Test;
-import org.sonar.api.issue.Issue;
 import org.sonar.api.rules.RuleType;
 import org.sonar.api.utils.Duration;
 import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolderRule;
@@ -53,6 +52,7 @@ import static org.sonar.api.issue.Issue.RESOLUTION_FIXED;
 import static org.sonar.api.issue.Issue.STATUS_CLOSED;
 import static org.sonar.api.issue.Issue.STATUS_OPEN;
 import static org.sonar.api.issue.Issue.STATUS_RESOLVED;
+import static org.sonar.api.issue.Issue.STATUS_TO_REVIEW;
 import static org.sonar.api.rule.Severity.BLOCKER;
 import static org.sonar.api.utils.DateUtils.parseDate;
 import static org.sonar.db.rule.RuleTesting.XOO_X1;
@@ -111,7 +111,8 @@ public class IssueLifecycleTest {
     assertThat(issue.key()).isNotNull();
     assertThat(issue.creationDate()).isNotNull();
     assertThat(issue.updateDate()).isNotNull();
-    assertThat(issue.status()).isEqualTo(STATUS_OPEN);
+    assertThat(issue.status()).isEqualTo(STATUS_TO_REVIEW);
+    assertThat(issue.resolution()).isNull();
     assertThat(issue.effort()).isEqualTo(DEFAULT_DURATION);
     assertThat(issue.isNew()).isTrue();
     assertThat(issue.isCopied()).isFalse();
@@ -419,7 +420,7 @@ public class IssueLifecycleTest {
   }
 
   @Test
-  public void mergeExistingOpenIssue_vulnerability_changed_to_hotspot_should_reopen() {
+  public void mergeExistingOpenIssue_vulnerability_changed_to_hotspot_should_be_to_review() {
     rule.setType(RuleType.SECURITY_HOTSPOT);
     DefaultIssue raw = new DefaultIssue()
       .setNew(true)
@@ -473,7 +474,7 @@ public class IssueLifecycleTest {
     assertThat(raw.isChanged()).isTrue();
 
     verify(updater).setType(raw, RuleType.SECURITY_HOTSPOT, issueChangeContext);
-    verify(updater).setStatus(raw, Issue.STATUS_REOPENED, issueChangeContext);
+    verify(updater).setStatus(raw, STATUS_TO_REVIEW, issueChangeContext);
     verify(updater).setResolution(raw, null, issueChangeContext);
     verify(updater).setPastSeverity(raw, BLOCKER, issueChangeContext);
     verify(updater).setPastLine(raw, 10);

diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/IssueWorkflow.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/IssueWorkflow.java
index 2afa5feb83846b5e92f7ca624758913cdb54d7a3..1a7cfe32e1bc6dc162fca419d9ce195ab98345dc 100644 (file)
--- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/IssueWorkflow.java
+++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/IssueWorkflow.java
@@ -33,6 +33,16 @@ import org.sonar.server.issue.IssueFieldsSetter;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
+import static org.sonar.api.issue.Issue.RESOLUTION_FALSE_POSITIVE;
+import static org.sonar.api.issue.Issue.RESOLUTION_FIXED;
+import static org.sonar.api.issue.Issue.RESOLUTION_REMOVED;
+import static org.sonar.api.issue.Issue.RESOLUTION_WONT_FIX;
+import static org.sonar.api.issue.Issue.STATUS_CLOSED;
+import static org.sonar.api.issue.Issue.STATUS_CONFIRMED;
+import static org.sonar.api.issue.Issue.STATUS_OPEN;
+import static org.sonar.api.issue.Issue.STATUS_REOPENED;
+import static org.sonar.api.issue.Issue.STATUS_RESOLVED;
+import static org.sonar.api.issue.Issue.STATUS_TO_REVIEW;
 
 @ServerSide
 @ComputeEngineSide
@@ -51,9 +61,7 @@ public class IssueWorkflow implements Startable {
   @Override
   public void start() {
     StateMachine.Builder builder = StateMachine.builder()
-      // order is important for UI
-      .states(Issue.STATUS_OPEN, Issue.STATUS_CONFIRMED, Issue.STATUS_REOPENED, Issue.STATUS_RESOLVED, Issue.STATUS_CLOSED);
-
+      .states(STATUS_OPEN, STATUS_CONFIRMED, STATUS_REOPENED, STATUS_RESOLVED, STATUS_CLOSED, STATUS_TO_REVIEW);
     buildManualTransitions(builder);
     buildAutomaticTransitions(builder);
     buildSecurityHotspotTransitions(builder);
@@ -63,78 +71,78 @@ public class IssueWorkflow implements Startable {
   private static void buildManualTransitions(StateMachine.Builder builder) {
     builder
       .transition(Transition.builder(DefaultTransitions.CONFIRM)
-        .from(Issue.STATUS_OPEN).to(Issue.STATUS_CONFIRMED)
+        .from(STATUS_OPEN).to(STATUS_CONFIRMED)
         .conditions(IsNotHotspotNorManualVulnerability.INSTANCE)
         .functions(new SetResolution(null))
         .build())
       .transition(Transition.builder(DefaultTransitions.CONFIRM)
-        .from(Issue.STATUS_REOPENED).to(Issue.STATUS_CONFIRMED)
+        .from(STATUS_REOPENED).to(STATUS_CONFIRMED)
         .conditions(IsNotHotspotNorManualVulnerability.INSTANCE)
         .functions(new SetResolution(null))
         .build())
       .transition(Transition.builder(DefaultTransitions.UNCONFIRM)
-        .from(Issue.STATUS_CONFIRMED).to(Issue.STATUS_REOPENED)
+        .from(STATUS_CONFIRMED).to(STATUS_REOPENED)
         .conditions(IsNotHotspotNorManualVulnerability.INSTANCE)
         .functions(new SetResolution(null))
         .build())
       .transition(Transition.builder(DefaultTransitions.RESOLVE)
-        .from(Issue.STATUS_OPEN).to(Issue.STATUS_RESOLVED)
+        .from(STATUS_OPEN).to(STATUS_RESOLVED)
         .conditions(IsNotHotspotNorManualVulnerability.INSTANCE)
-        .functions(new SetResolution(Issue.RESOLUTION_FIXED))
+        .functions(new SetResolution(RESOLUTION_FIXED))
         .build())
       .transition(Transition.builder(DefaultTransitions.RESOLVE)
-        .from(Issue.STATUS_REOPENED).to(Issue.STATUS_RESOLVED)
+        .from(STATUS_REOPENED).to(STATUS_RESOLVED)
         .conditions(IsNotHotspotNorManualVulnerability.INSTANCE)
-        .functions(new SetResolution(Issue.RESOLUTION_FIXED))
+        .functions(new SetResolution(RESOLUTION_FIXED))
         .build())
       .transition(Transition.builder(DefaultTransitions.RESOLVE)
-        .from(Issue.STATUS_CONFIRMED).to(Issue.STATUS_RESOLVED)
+        .from(STATUS_CONFIRMED).to(STATUS_RESOLVED)
         .conditions(IsNotHotspotNorManualVulnerability.INSTANCE)
-        .functions(new SetResolution(Issue.RESOLUTION_FIXED))
+        .functions(new SetResolution(RESOLUTION_FIXED))
         .build())
       .transition(Transition.builder(DefaultTransitions.REOPEN)
-        .from(Issue.STATUS_RESOLVED).to(Issue.STATUS_REOPENED)
+        .from(STATUS_RESOLVED).to(STATUS_REOPENED)
         .conditions(IsNotHotspotNorManualVulnerability.INSTANCE)
         .functions(new SetResolution(null))
         .build())
 
       // resolve as false-positive
       .transition(Transition.builder(DefaultTransitions.FALSE_POSITIVE)
-        .from(Issue.STATUS_OPEN).to(Issue.STATUS_RESOLVED)
+        .from(STATUS_OPEN).to(STATUS_RESOLVED)
         .conditions(IsNotHotspotNorManualVulnerability.INSTANCE)
-        .functions(new SetResolution(Issue.RESOLUTION_FALSE_POSITIVE), UnsetAssignee.INSTANCE)
+        .functions(new SetResolution(RESOLUTION_FALSE_POSITIVE), UnsetAssignee.INSTANCE)
         .requiredProjectPermission(UserRole.ISSUE_ADMIN)
         .build())
       .transition(Transition.builder(DefaultTransitions.FALSE_POSITIVE)
-        .from(Issue.STATUS_REOPENED).to(Issue.STATUS_RESOLVED)
+        .from(STATUS_REOPENED).to(STATUS_RESOLVED)
         .conditions(IsNotHotspotNorManualVulnerability.INSTANCE)
-        .functions(new SetResolution(Issue.RESOLUTION_FALSE_POSITIVE), UnsetAssignee.INSTANCE)
+        .functions(new SetResolution(RESOLUTION_FALSE_POSITIVE), UnsetAssignee.INSTANCE)
         .requiredProjectPermission(UserRole.ISSUE_ADMIN)
         .build())
       .transition(Transition.builder(DefaultTransitions.FALSE_POSITIVE)
-        .from(Issue.STATUS_CONFIRMED).to(Issue.STATUS_RESOLVED)
+        .from(STATUS_CONFIRMED).to(STATUS_RESOLVED)
         .conditions(IsNotHotspotNorManualVulnerability.INSTANCE)
-        .functions(new SetResolution(Issue.RESOLUTION_FALSE_POSITIVE), UnsetAssignee.INSTANCE)
+        .functions(new SetResolution(RESOLUTION_FALSE_POSITIVE), UnsetAssignee.INSTANCE)
         .requiredProjectPermission(UserRole.ISSUE_ADMIN)
         .build())
 
       // resolve as won't fix
       .transition(Transition.builder(DefaultTransitions.WONT_FIX)
-        .from(Issue.STATUS_OPEN).to(Issue.STATUS_RESOLVED)
+        .from(STATUS_OPEN).to(STATUS_RESOLVED)
         .conditions(IsNotHotspotNorManualVulnerability.INSTANCE)
-        .functions(new SetResolution(Issue.RESOLUTION_WONT_FIX), UnsetAssignee.INSTANCE)
+        .functions(new SetResolution(RESOLUTION_WONT_FIX), UnsetAssignee.INSTANCE)
         .requiredProjectPermission(UserRole.ISSUE_ADMIN)
         .build())
       .transition(Transition.builder(DefaultTransitions.WONT_FIX)
-        .from(Issue.STATUS_REOPENED).to(Issue.STATUS_RESOLVED)
+        .from(STATUS_REOPENED).to(STATUS_RESOLVED)
         .conditions(IsNotHotspotNorManualVulnerability.INSTANCE)
-        .functions(new SetResolution(Issue.RESOLUTION_WONT_FIX), UnsetAssignee.INSTANCE)
+        .functions(new SetResolution(RESOLUTION_WONT_FIX), UnsetAssignee.INSTANCE)
         .requiredProjectPermission(UserRole.ISSUE_ADMIN)
         .build())
       .transition(Transition.builder(DefaultTransitions.WONT_FIX)
-        .from(Issue.STATUS_CONFIRMED).to(Issue.STATUS_RESOLVED)
+        .from(STATUS_CONFIRMED).to(STATUS_RESOLVED)
         .conditions(IsNotHotspotNorManualVulnerability.INSTANCE)
-        .functions(new SetResolution(Issue.RESOLUTION_WONT_FIX), UnsetAssignee.INSTANCE)
+        .functions(new SetResolution(RESOLUTION_WONT_FIX), UnsetAssignee.INSTANCE)
         .requiredProjectPermission(UserRole.ISSUE_ADMIN)
         .build());
   }
@@ -142,146 +150,137 @@ public class IssueWorkflow implements Startable {
   private static void buildSecurityHotspotTransitions(StateMachine.Builder builder) {
     builder
       .transition(Transition.builder(DefaultTransitions.DETECT)
-        .from(Issue.STATUS_OPEN).to(Issue.STATUS_OPEN)
-        .conditions(new HasType(RuleType.SECURITY_HOTSPOT))
-        .functions(new SetType(RuleType.VULNERABILITY))
-        .requiredProjectPermission(UserRole.SECURITYHOTSPOT_ADMIN)
-        .build())
-      .transition(Transition.builder(DefaultTransitions.DETECT)
-        .from(Issue.STATUS_REOPENED).to(Issue.STATUS_OPEN)
+        .from(STATUS_TO_REVIEW).to(STATUS_OPEN)
         .conditions(new HasType(RuleType.SECURITY_HOTSPOT))
         .functions(new SetType(RuleType.VULNERABILITY))
         .requiredProjectPermission(UserRole.SECURITYHOTSPOT_ADMIN)
         .build())
       .transition(Transition.builder(DefaultTransitions.DETECT)
-        .from(Issue.STATUS_RESOLVED).to(Issue.STATUS_OPEN)
-        .conditions(new HasType(RuleType.SECURITY_HOTSPOT), new HasResolution(Issue.RESOLUTION_WONT_FIX))
+        .from(STATUS_RESOLVED).to(STATUS_OPEN)
+        .conditions(new HasType(RuleType.SECURITY_HOTSPOT), new HasResolution(RESOLUTION_WONT_FIX))
         .functions(new SetType(RuleType.VULNERABILITY), new SetResolution(null))
         .requiredProjectPermission(UserRole.SECURITYHOTSPOT_ADMIN)
         .build())
       .transition(Transition.builder(DefaultTransitions.DISMISS)
-        .from(Issue.STATUS_OPEN).to(Issue.STATUS_REOPENED)
+        .from(STATUS_OPEN).to(STATUS_TO_REVIEW)
         .conditions(IsManualVulnerability.INSTANCE)
         .functions(new SetType(RuleType.SECURITY_HOTSPOT))
         .requiredProjectPermission(UserRole.SECURITYHOTSPOT_ADMIN)
         .build())
       .transition(Transition.builder(DefaultTransitions.REQUEST_REVIEW)
-        .from(Issue.STATUS_OPEN).to(Issue.STATUS_RESOLVED)
+        .from(STATUS_OPEN).to(STATUS_RESOLVED)
         .conditions(IsManualVulnerability.INSTANCE)
-        .functions(new SetType(RuleType.SECURITY_HOTSPOT), new SetResolution(Issue.RESOLUTION_FIXED))
+        .functions(new SetType(RuleType.SECURITY_HOTSPOT), new SetResolution(RESOLUTION_FIXED))
         .build())
       .transition(Transition.builder(DefaultTransitions.REQUEST_REVIEW)
-        .from(Issue.STATUS_REOPENED).to(Issue.STATUS_RESOLVED)
+        .from(STATUS_REOPENED).to(STATUS_RESOLVED)
         .conditions(IsManualVulnerability.INSTANCE)
-        .functions(new SetType(RuleType.SECURITY_HOTSPOT), new SetResolution(Issue.RESOLUTION_FIXED))
+        .functions(new SetType(RuleType.SECURITY_HOTSPOT), new SetResolution(RESOLUTION_FIXED))
         .build())
       .transition(Transition.builder(DefaultTransitions.REJECT)
-        .from(Issue.STATUS_RESOLVED).to(Issue.STATUS_REOPENED)
-        .conditions(new HasType(RuleType.SECURITY_HOTSPOT), new HasResolution(Issue.RESOLUTION_FIXED))
+        .from(STATUS_RESOLVED).to(STATUS_REOPENED)
+        .conditions(new HasType(RuleType.SECURITY_HOTSPOT), new HasResolution(RESOLUTION_FIXED))
         .functions(new SetType(RuleType.VULNERABILITY), new SetResolution(null))
         .requiredProjectPermission(UserRole.SECURITYHOTSPOT_ADMIN)
         .build())
       .transition(Transition.builder(DefaultTransitions.ACCEPT)
-        .from(Issue.STATUS_RESOLVED).to(Issue.STATUS_RESOLVED)
-        .conditions(new HasType(RuleType.SECURITY_HOTSPOT), new HasResolution(Issue.RESOLUTION_FIXED))
-        .functions(new SetResolution(Issue.RESOLUTION_WONT_FIX))
-        .requiredProjectPermission(UserRole.SECURITYHOTSPOT_ADMIN)
-        .build())
-      .transition(Transition.builder(DefaultTransitions.CLEAR)
-        .from(Issue.STATUS_OPEN).to(Issue.STATUS_RESOLVED)
-        .conditions(new HasType(RuleType.SECURITY_HOTSPOT))
-        .functions(new SetResolution(Issue.RESOLUTION_WONT_FIX))
+        .from(STATUS_RESOLVED).to(STATUS_RESOLVED)
+        .conditions(new HasType(RuleType.SECURITY_HOTSPOT), new HasResolution(RESOLUTION_FIXED))
+        .functions(new SetResolution(RESOLUTION_WONT_FIX))
         .requiredProjectPermission(UserRole.SECURITYHOTSPOT_ADMIN)
         .build())
       .transition(Transition.builder(DefaultTransitions.CLEAR)
-        .from(Issue.STATUS_REOPENED).to(Issue.STATUS_RESOLVED)
+        .from(STATUS_TO_REVIEW).to(STATUS_RESOLVED)
         .conditions(new HasType(RuleType.SECURITY_HOTSPOT))
-        .functions(new SetResolution(Issue.RESOLUTION_WONT_FIX))
+        .functions(new SetResolution(RESOLUTION_WONT_FIX))
         .requiredProjectPermission(UserRole.SECURITYHOTSPOT_ADMIN)
         .build())
       .transition(Transition.builder(DefaultTransitions.REOPEN_HOTSPOT)
-        .from(Issue.STATUS_RESOLVED).to(Issue.STATUS_REOPENED)
-        .conditions(new HasType(RuleType.SECURITY_HOTSPOT), new HasResolution(Issue.RESOLUTION_WONT_FIX))
+        .from(STATUS_RESOLVED).to(STATUS_TO_REVIEW)
+        .conditions(new HasType(RuleType.SECURITY_HOTSPOT))
         .functions(new SetResolution(null))
         .requiredProjectPermission(UserRole.SECURITYHOTSPOT_ADMIN)
         .build());
-
   }
 
   private static void buildAutomaticTransitions(StateMachine.Builder builder) {
     // Close the "end of life" issues (disabled/deleted rule, deleted component)
     builder
       .transition(Transition.builder(AUTOMATIC_CLOSE_TRANSITION)
-        .from(Issue.STATUS_OPEN).to(Issue.STATUS_CLOSED)
+        .from(STATUS_OPEN).to(STATUS_CLOSED)
         .conditions(IsBeingClosed.INSTANCE)
         .functions(SetClosed.INSTANCE, SetCloseDate.INSTANCE)
         .automatic()
         .build())
       .transition(Transition.builder(AUTOMATIC_CLOSE_TRANSITION)
-        .from(Issue.STATUS_REOPENED).to(Issue.STATUS_CLOSED)
+        .from(STATUS_REOPENED).to(STATUS_CLOSED)
         .conditions(IsBeingClosed.INSTANCE)
         .functions(SetClosed.INSTANCE, SetCloseDate.INSTANCE)
         .automatic()
         .build())
       .transition(Transition.builder(AUTOMATIC_CLOSE_TRANSITION)
-        .from(Issue.STATUS_CONFIRMED).to(Issue.STATUS_CLOSED)
+        .from(STATUS_CONFIRMED).to(STATUS_CLOSED)
         .conditions(IsBeingClosed.INSTANCE)
         .functions(SetClosed.INSTANCE, SetCloseDate.INSTANCE)
         .automatic()
         .build())
       .transition(Transition.builder(AUTOMATIC_CLOSE_TRANSITION)
-        .from(Issue.STATUS_RESOLVED).to(Issue.STATUS_CLOSED)
+        .from(STATUS_RESOLVED).to(STATUS_CLOSED)
         .conditions(IsBeingClosed.INSTANCE)
         .functions(SetClosed.INSTANCE, SetCloseDate.INSTANCE)
         .automatic()
         .build())
+      .transition(Transition.builder(AUTOMATIC_CLOSE_TRANSITION)
+        .from(STATUS_TO_REVIEW).to(STATUS_CLOSED)
+        .conditions(IsBeingClosed.INSTANCE, new HasType(RuleType.SECURITY_HOTSPOT))
+        .functions(SetClosed.INSTANCE, SetCloseDate.INSTANCE)
+        .automatic()
+        .build())
 
       // Reopen issues that are marked as resolved but that are still alive.
       .transition(Transition.builder("automaticreopen")
-        .from(Issue.STATUS_RESOLVED).to(Issue.STATUS_REOPENED)
-        .conditions(new NotCondition(IsBeingClosed.INSTANCE), new HasResolution(Issue.RESOLUTION_FIXED), IsNotHotspotNorManualVulnerability.INSTANCE)
+        .from(STATUS_RESOLVED).to(STATUS_REOPENED)
+        .conditions(new NotCondition(IsBeingClosed.INSTANCE), new HasResolution(RESOLUTION_FIXED), IsNotHotspotNorManualVulnerability.INSTANCE)
         .functions(new SetResolution(null), UnsetCloseDate.INSTANCE)
         .automatic()
         .build())
 
       .transition(Transition.builder("automaticuncloseopen")
-        .from(Issue.STATUS_CLOSED).to(Issue.STATUS_OPEN)
+        .from(STATUS_CLOSED).to(STATUS_OPEN)
         .conditions(
-          new PreviousStatusWas(Issue.STATUS_OPEN),
-          new HasResolution(Issue.RESOLUTION_REMOVED, Issue.RESOLUTION_FIXED),
+          new PreviousStatusWas(STATUS_OPEN),
+          new HasResolution(RESOLUTION_REMOVED, RESOLUTION_FIXED),
           IsNotHotspotNorManualVulnerability.INSTANCE)
         .functions(RestoreResolutionFunction.INSTANCE, UnsetCloseDate.INSTANCE)
         .automatic()
         .build())
       .transition(Transition.builder("automaticunclosereopen")
-        .from(Issue.STATUS_CLOSED).to(Issue.STATUS_REOPENED)
+        .from(STATUS_CLOSED).to(STATUS_REOPENED)
         .conditions(
-          new PreviousStatusWas(Issue.STATUS_REOPENED),
-          new HasResolution(Issue.RESOLUTION_REMOVED, Issue.RESOLUTION_FIXED),
+          new PreviousStatusWas(STATUS_REOPENED),
+          new HasResolution(RESOLUTION_REMOVED, RESOLUTION_FIXED),
           IsNotHotspotNorManualVulnerability.INSTANCE)
         .functions(RestoreResolutionFunction.INSTANCE, UnsetCloseDate.INSTANCE)
         .automatic()
         .build())
       .transition(Transition.builder("automaticuncloseconfirmed")
-        .from(Issue.STATUS_CLOSED).to(Issue.STATUS_CONFIRMED)
+        .from(STATUS_CLOSED).to(STATUS_CONFIRMED)
         .conditions(
-          new PreviousStatusWas(Issue.STATUS_CONFIRMED),
-          new HasResolution(Issue.RESOLUTION_REMOVED, Issue.RESOLUTION_FIXED),
+          new PreviousStatusWas(STATUS_CONFIRMED),
+          new HasResolution(RESOLUTION_REMOVED, RESOLUTION_FIXED),
           IsNotHotspotNorManualVulnerability.INSTANCE)
         .functions(RestoreResolutionFunction.INSTANCE, UnsetCloseDate.INSTANCE)
         .automatic()
         .build())
       .transition(Transition.builder("automaticuncloseresolved")
-        .from(Issue.STATUS_CLOSED).to(Issue.STATUS_RESOLVED)
+        .from(STATUS_CLOSED).to(STATUS_RESOLVED)
         .conditions(
-          new PreviousStatusWas(Issue.STATUS_RESOLVED),
-          new HasResolution(Issue.RESOLUTION_REMOVED, Issue.RESOLUTION_FIXED),
+          new PreviousStatusWas(STATUS_RESOLVED),
+          new HasResolution(RESOLUTION_REMOVED, RESOLUTION_FIXED),
           IsNotHotspotNorManualVulnerability.INSTANCE)
         .functions(RestoreResolutionFunction.INSTANCE, UnsetCloseDate.INSTANCE)
         .automatic()
-        .build())
-
-    ;
+        .build());
   }
 
   @Override
@@ -291,7 +290,7 @@ public class IssueWorkflow implements Startable {
 
   public boolean doManualTransition(DefaultIssue issue, String transitionKey, IssueChangeContext issueChangeContext) {
     Transition transition = stateOf(issue).transition(transitionKey);
-    if (!transition.automatic()) {
+    if (transition.supports(issue) && !transition.automatic()) {
       functionExecutor.execute(transition.functions(), issue, issueChangeContext);
       updater.setStatus(issue, transition.to(), issueChangeContext);
       return true;
@@ -326,7 +325,4 @@ public class IssueWorkflow implements Startable {
     return state;
   }
 
-  StateMachine machine() {
-    return machine;
-  }
 }

diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/IssueWorkflowTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/IssueWorkflowTest.java
index 1028b9c69ce9b222cb7b6c8426925a6794256adf..4151f7280b6e85fc573f87957796a4484ccaabc8 100644 (file)
--- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/IssueWorkflowTest.java
+++ b/server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/IssueWorkflowTest.java
@@ -55,85 +55,84 @@ import static org.sonar.api.issue.Issue.STATUS_CONFIRMED;
 import static org.sonar.api.issue.Issue.STATUS_OPEN;
 import static org.sonar.api.issue.Issue.STATUS_REOPENED;
 import static org.sonar.api.issue.Issue.STATUS_RESOLVED;
+import static org.sonar.api.issue.Issue.STATUS_TO_REVIEW;
+import static org.sonar.db.rule.RuleTesting.XOO_X1;
 
 @RunWith(DataProviderRunner.class)
 public class IssueWorkflowTest {
 
   private IssueFieldsSetter updater = new IssueFieldsSetter();
-  private IssueWorkflow workflow = new IssueWorkflow(new FunctionExecutor(updater), updater);
 
-  @Test
-  public void init_state_machine() {
-    assertThat(workflow.machine()).isNull();
-    workflow.start();
-    assertThat(workflow.machine()).isNotNull();
-    assertThat(workflow.machine().state(STATUS_OPEN)).isNotNull();
-    assertThat(workflow.machine().state(STATUS_CONFIRMED)).isNotNull();
-    assertThat(workflow.machine().state(STATUS_CLOSED)).isNotNull();
-    assertThat(workflow.machine().state(STATUS_REOPENED)).isNotNull();
-    assertThat(workflow.machine().state(STATUS_RESOLVED)).isNotNull();
-    workflow.stop();
-  }
+  private IssueWorkflow underTest = new IssueWorkflow(new FunctionExecutor(updater), updater);
 
   @Test
   public void list_statuses() {
-    workflow.start();
-    // order is important for UI
-    assertThat(workflow.statusKeys()).containsSubsequence(STATUS_OPEN, STATUS_CONFIRMED, STATUS_REOPENED, STATUS_RESOLVED, STATUS_CLOSED);
+    underTest.start();
+    assertThat(underTest.statusKeys()).containsExactly(STATUS_OPEN, STATUS_CONFIRMED, STATUS_REOPENED, STATUS_RESOLVED, STATUS_CLOSED, STATUS_TO_REVIEW);
   }
 
   @Test
   public void list_out_transitions_from_status_open() {
-    workflow.start();
+    underTest.start();
 
     DefaultIssue issue = new DefaultIssue().setStatus(STATUS_OPEN);
-    List<Transition> transitions = workflow.outTransitions(issue);
+    List<Transition> transitions = underTest.outTransitions(issue);
     assertThat(keys(transitions)).containsOnly("confirm", "falsepositive", "resolve", "wontfix");
   }
 
   @Test
   public void list_out_transitions_from_status_confirmed() {
-    workflow.start();
+    underTest.start();
 
     DefaultIssue issue = new DefaultIssue().setStatus(STATUS_CONFIRMED);
-    List<Transition> transitions = workflow.outTransitions(issue);
+    List<Transition> transitions = underTest.outTransitions(issue);
     assertThat(keys(transitions)).containsOnly("unconfirm", "falsepositive", "resolve", "wontfix");
   }
 
   @Test
   public void list_out_transitions_from_status_resolved() {
-    workflow.start();
+    underTest.start();
 
     DefaultIssue issue = new DefaultIssue().setStatus(STATUS_RESOLVED);
-    List<Transition> transitions = workflow.outTransitions(issue);
+    List<Transition> transitions = underTest.outTransitions(issue);
     assertThat(keys(transitions)).containsOnly("reopen");
   }
 
   @Test
   public void list_out_transitions_from_status_reopen() {
-    workflow.start();
+    underTest.start();
 
     DefaultIssue issue = new DefaultIssue().setStatus(STATUS_REOPENED);
-    List<Transition> transitions = workflow.outTransitions(issue);
+    List<Transition> transitions = underTest.outTransitions(issue);
     assertThat(keys(transitions)).containsOnly("confirm", "resolve", "falsepositive", "wontfix");
   }
 
   @Test
   public void list_no_out_transition_from_status_closed() {
-    workflow.start();
+    underTest.start();
 
     DefaultIssue issue = new DefaultIssue().setStatus(STATUS_CLOSED).setRuleKey(RuleKey.of("java", "R1  "));
-    List<Transition> transitions = workflow.outTransitions(issue);
+    List<Transition> transitions = underTest.outTransitions(issue);
     assertThat(transitions).isEmpty();
   }
 
+  @Test
+  public void list_out_transitions_from_security_hotspot_in_status_to_review() {
+    underTest.start();
+    DefaultIssue issue = new DefaultIssue().setType(RuleType.SECURITY_HOTSPOT).setStatus(STATUS_TO_REVIEW);
+
+    List<Transition> transitions = underTest.outTransitions(issue);
+
+    assertThat(keys(transitions)).containsOnly("detect", "clear");
+  }
+
   @Test
   public void fail_if_unknown_status_when_listing_transitions() {
-    workflow.start();
+    underTest.start();
 
     DefaultIssue issue = new DefaultIssue().setStatus("xxx");
     try {
-      workflow.outTransitions(issue);
+      underTest.outTransitions(issue);
       fail();
     } catch (IllegalArgumentException e) {
       assertThat(e).hasMessage("Unknown status: xxx");
@@ -142,7 +141,7 @@ public class IssueWorkflowTest {
 
   @Test
   public void automatically_close_resolved_issue() {
-    workflow.start();
+    underTest.start();
 
     DefaultIssue issue = new DefaultIssue()
       .setKey("ABCDE")
@@ -152,7 +151,28 @@ public class IssueWorkflowTest {
       .setNew(false)
       .setBeingClosed(true);
     Date now = new Date();
-    workflow.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
+    underTest.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
+    assertThat(issue.resolution()).isEqualTo(RESOLUTION_FIXED);
+    assertThat(issue.status()).isEqualTo(STATUS_CLOSED);
+    assertThat(issue.closeDate()).isNotNull();
+    assertThat(issue.updateDate()).isEqualTo(DateUtils.truncate(now, Calendar.SECOND));
+  }
+
+  @Test
+  public void automatically_close_resolved_security_hotspots_in_to_review() {
+    underTest.start();
+    DefaultIssue issue = new DefaultIssue()
+      .setKey("ABCDE")
+      .setType(RuleType.SECURITY_HOTSPOT)
+      .setRuleKey(XOO_X1)
+      .setResolution(null)
+      .setStatus(STATUS_TO_REVIEW)
+      .setNew(false)
+      .setBeingClosed(true);
+    Date now = new Date();
+
+    underTest.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
+
     assertThat(issue.resolution()).isEqualTo(RESOLUTION_FIXED);
     assertThat(issue.status()).isEqualTo(STATUS_CLOSED);
     assertThat(issue.closeDate()).isNotNull();
@@ -170,10 +190,10 @@ public class IssueWorkflowTest {
       })
       .toArray(DefaultIssue[]::new);
     Date now = new Date();
-    workflow.start();
+    underTest.start();
 
     Arrays.stream(issues).forEach(issue -> {
-      workflow.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
+      underTest.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
 
       assertThat(issue.status()).isEqualTo(previousStatus);
       assertThat(issue.updateDate()).isEqualTo(DateUtils.truncate(now, Calendar.SECOND));
@@ -196,10 +216,10 @@ public class IssueWorkflowTest {
       })
       .toArray(DefaultIssue[]::new);
     Date now = new Date();
-    workflow.start();
+    underTest.start();
 
     Arrays.stream(issues).forEach(issue -> {
-      workflow.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
+      underTest.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
 
       assertThat(issue.status()).isEqualTo(previousStatus);
       assertThat(issue.updateDate()).isEqualTo(DateUtils.truncate(now, Calendar.SECOND));
@@ -220,10 +240,10 @@ public class IssueWorkflowTest {
       })
       .toArray(DefaultIssue[]::new);
     Date now = new Date();
-    workflow.start();
+    underTest.start();
 
     Arrays.stream(issues).forEach(issue -> {
-      workflow.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
+      underTest.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
 
       assertThat(issue.status()).isEqualTo(randomPreviousStatus);
       assertThat(issue.resolution()).isEqualTo(resolutionBeforeClosed);
@@ -244,10 +264,10 @@ public class IssueWorkflowTest {
       })
       .toArray(DefaultIssue[]::new);
     Date now = new Date();
-    workflow.start();
+    underTest.start();
 
     Arrays.stream(issues).forEach(issue -> {
-      workflow.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
+      underTest.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
 
       assertThat(issue.status()).isEqualTo(randomPreviousStatus);
       assertThat(issue.resolution()).isNull();
@@ -272,10 +292,10 @@ public class IssueWorkflowTest {
       })
       .toArray(DefaultIssue[]::new);
     Date now = new Date();
-    workflow.start();
+    underTest.start();
 
     Arrays.stream(issues).forEach(issue -> {
-      workflow.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
+      underTest.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
 
       assertThat(issue.status()).isEqualTo(randomPreviousStatus);
       assertThat(issue.resolution()).isEqualTo(resolutionBeforeClosed);
@@ -285,16 +305,23 @@ public class IssueWorkflowTest {
     });
   }
 
+  @DataProvider
+  public static Object[][] allResolutionsBeforeClosing() {
+    return Arrays.stream(ALL_RESOLUTIONS_BEFORE_CLOSING)
+      .map(t -> new Object[] {t})
+      .toArray(Object[][]::new);
+  }
+
   @Test
   public void do_not_automatically_reopen_closed_issue_which_have_no_previous_status_in_changelog() {
     DefaultIssue[] issues = Arrays.stream(SUPPORTED_RESOLUTIONS_FOR_UNCLOSING)
       .map(IssueWorkflowTest::newClosedIssue)
       .toArray(DefaultIssue[]::new);
     Date now = new Date();
-    workflow.start();
+    underTest.start();
 
     Arrays.stream(issues).forEach(issue -> {
-      workflow.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
+      underTest.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
 
       assertThat(issue.status()).isEqualTo(STATUS_CLOSED);
       assertThat(issue.updateDate()).isNull();
@@ -313,16 +340,31 @@ public class IssueWorkflowTest {
       })
       .toArray(DefaultIssue[]::new);
     Date now = new Date();
-    workflow.start();
+    underTest.start();
 
     Arrays.stream(issues).forEach(issue -> {
-      workflow.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
+      underTest.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
 
       assertThat(issue.status()).isEqualTo(STATUS_CLOSED);
       assertThat(issue.updateDate()).isNull();
     });
   }
 
+  @Test
+  public void doAutomaticTransition_does_nothing_on_security_hotspots_in_to_review_status() {
+    DefaultIssue issue = new DefaultIssue()
+      .setKey("ABCDE")
+      .setRuleKey(XOO_X1)
+      .setResolution(null)
+      .setStatus(STATUS_TO_REVIEW);
+
+    underTest.start();
+    underTest.doAutomaticTransition(issue, IssueChangeContext.createScan(new Date()));
+
+    assertThat(issue.status()).isEqualTo(STATUS_TO_REVIEW);
+    assertThat(issue.resolution()).isNull();
+  }
+
   @Test
   @UseDataProvider("allStatusesLeadingToClosed")
   public void do_not_automatically_reopen_closed_issues_of_manual_vulnerability(String previousStatus) {
@@ -335,16 +377,15 @@ public class IssueWorkflowTest {
       })
       .toArray(DefaultIssue[]::new);
     Date now = new Date();
-    workflow.start();
+    underTest.start();
 
     Arrays.stream(issues).forEach(issue -> {
-      workflow.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
+      underTest.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
 
       assertThat(issue.status()).isEqualTo(STATUS_CLOSED);
       assertThat(issue.updateDate()).isNull();
     });
   }
-
   private static final String[] ALL_STATUSES_LEADING_TO_CLOSED = new String[] {STATUS_OPEN, STATUS_REOPENED, STATUS_CONFIRMED, STATUS_RESOLVED};
   private static final String[] ALL_RESOLUTIONS_BEFORE_CLOSING = new String[] {
     null,
@@ -352,6 +393,7 @@ public class IssueWorkflowTest {
     RESOLUTION_WONT_FIX,
     RESOLUTION_FALSE_POSITIVE
   };
+
   private static final String[] SUPPORTED_RESOLUTIONS_FOR_UNCLOSING = new String[] {RESOLUTION_FIXED, RESOLUTION_REMOVED};
 
   @DataProvider
@@ -361,58 +403,9 @@ public class IssueWorkflowTest {
       .toArray(Object[][]::new);
   }
 
-  @DataProvider
-  public static Object[][] allResolutionsBeforeClosing() {
-    return Arrays.stream(ALL_RESOLUTIONS_BEFORE_CLOSING)
-      .map(t -> new Object[] {t})
-      .toArray(Object[][]::new);
-  }
-
-  private static DefaultIssue newClosedIssue(String resolution) {
-    return new DefaultIssue()
-      .setKey("ABCDE")
-      .setRuleKey(RuleKey.of("js", "S001"))
-      .setResolution(resolution)
-      .setStatus(STATUS_CLOSED)
-      .setNew(false)
-      .setCloseDate(new Date(5_999_999L));
-  }
-
-  private static void setStatusPreviousToClosed(DefaultIssue issue, String previousStatus) {
-    addStatusChange(issue, new Date(), previousStatus, STATUS_CLOSED);
-  }
-
-  private static void addStatusChange(DefaultIssue issue, Date date, String previousStatus, String newStatus) {
-    issue.addChange(new FieldDiffs().setCreationDate(date).setDiff("status", previousStatus, newStatus));
-  }
-
-  private void addResolutionChange(DefaultIssue issue, Date creationDate,
-    @Nullable String previousResolution, @Nullable String newResolution) {
-    checkArgument(previousResolution != null || newResolution != null, "At least one resolution must be non null");
-
-    FieldDiffs fieldDiffs = new FieldDiffs().setCreationDate(creationDate)
-      .setDiff("resolution", emptyIfNull(previousResolution), emptyIfNull(newResolution));
-    issue.addChange(fieldDiffs);
-  }
-
-  private void addResolutionAndStatusChange(DefaultIssue issue, Date creationDate,
-    String previousStatus, String newStatus,
-    @Nullable String previousResolution, @Nullable String newResolution) {
-    checkArgument(previousResolution != null || newResolution != null, "At least one resolution must be non null");
-
-    FieldDiffs fieldDiffs = new FieldDiffs().setCreationDate(creationDate)
-      .setDiff("status", previousStatus, newStatus)
-      .setDiff("resolution", emptyIfNull(previousResolution), emptyIfNull(newResolution));
-    issue.addChange(fieldDiffs);
-  }
-
-  private static String emptyIfNull(@Nullable String newResolution) {
-    return newResolution == null ? "" : newResolution;
-  }
-
   @Test
   public void close_open_dead_issue() {
-    workflow.start();
+    underTest.start();
 
     DefaultIssue issue = new DefaultIssue()
       .setKey("ABCDE")
@@ -421,7 +414,7 @@ public class IssueWorkflowTest {
       .setNew(false)
       .setBeingClosed(true);
     Date now = new Date();
-    workflow.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
+    underTest.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
     assertThat(issue.resolution()).isEqualTo(RESOLUTION_FIXED);
     assertThat(issue.status()).isEqualTo(STATUS_CLOSED);
     assertThat(issue.closeDate()).isNotNull();
@@ -430,7 +423,7 @@ public class IssueWorkflowTest {
 
   @Test
   public void close_reopened_dead_issue() {
-    workflow.start();
+    underTest.start();
 
     DefaultIssue issue = new DefaultIssue()
       .setKey("ABCDE")
@@ -439,7 +432,7 @@ public class IssueWorkflowTest {
       .setNew(false)
       .setBeingClosed(true);
     Date now = new Date();
-    workflow.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
+    underTest.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
     assertThat(issue.resolution()).isEqualTo(RESOLUTION_FIXED);
     assertThat(issue.status()).isEqualTo(STATUS_CLOSED);
     assertThat(issue.closeDate()).isNotNull();
@@ -448,7 +441,7 @@ public class IssueWorkflowTest {
 
   @Test
   public void close_confirmed_dead_issue() {
-    workflow.start();
+    underTest.start();
 
     DefaultIssue issue = new DefaultIssue()
       .setKey("ABCDE")
@@ -457,7 +450,7 @@ public class IssueWorkflowTest {
       .setNew(false)
       .setBeingClosed(true);
     Date now = new Date();
-    workflow.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
+    underTest.doAutomaticTransition(issue, IssueChangeContext.createScan(now));
     assertThat(issue.resolution()).isEqualTo(RESOLUTION_FIXED);
     assertThat(issue.status()).isEqualTo(STATUS_CLOSED);
     assertThat(issue.closeDate()).isNotNull();
@@ -466,7 +459,7 @@ public class IssueWorkflowTest {
 
   @Test
   public void fail_if_unknown_status_on_automatic_trans() {
-    workflow.start();
+    underTest.start();
 
     DefaultIssue issue = new DefaultIssue()
       .setKey("ABCDE")
@@ -475,7 +468,7 @@ public class IssueWorkflowTest {
       .setNew(false)
       .setBeingClosed(true);
     try {
-      workflow.doAutomaticTransition(issue, IssueChangeContext.createScan(new Date()));
+      underTest.doAutomaticTransition(issue, IssueChangeContext.createScan(new Date()));
       fail();
     } catch (IllegalStateException e) {
       assertThat(e).hasMessage("Unknown status: xxx [issue=ABCDE]");
@@ -490,8 +483,8 @@ public class IssueWorkflowTest {
       .setRuleKey(RuleKey.of("squid", "AvoidCycle"))
       .setAssigneeUuid("morgan");
 
-    workflow.start();
-    workflow.doManualTransition(issue, DefaultTransitions.FALSE_POSITIVE, IssueChangeContext.createScan(new Date()));
+    underTest.start();
+    underTest.doManualTransition(issue, DefaultTransitions.FALSE_POSITIVE, IssueChangeContext.createScan(new Date()));
 
     assertThat(issue.resolution()).isEqualTo(RESOLUTION_FALSE_POSITIVE);
     assertThat(issue.status()).isEqualTo(STATUS_RESOLVED);
@@ -508,8 +501,8 @@ public class IssueWorkflowTest {
       .setRuleKey(RuleKey.of("squid", "AvoidCycle"))
       .setAssigneeUuid("morgan");
 
-    workflow.start();
-    workflow.doManualTransition(issue, DefaultTransitions.WONT_FIX, IssueChangeContext.createScan(new Date()));
+    underTest.start();
+    underTest.doManualTransition(issue, DefaultTransitions.WONT_FIX, IssueChangeContext.createScan(new Date()));
 
     assertThat(issue.resolution()).isEqualTo(RESOLUTION_WONT_FIX);
     assertThat(issue.status()).isEqualTo(STATUS_RESOLVED);
@@ -518,6 +511,63 @@ public class IssueWorkflowTest {
     assertThat(issue.assignee()).isNull();
   }
 
+  @Test
+  public void do_not_allow_to_doManualTransition_when_condition_fails() {
+    underTest.start();
+    DefaultIssue issue = new DefaultIssue()
+      .setKey("ABCDE")
+      // Detect is only available on hotspot
+      .setType(RuleType.VULNERABILITY)
+      .setIsFromHotspot(true)
+      .setStatus(STATUS_RESOLVED)
+      .setResolution(RESOLUTION_WONT_FIX)
+      .setRuleKey(XOO_X1);
+
+    assertThat(underTest.doManualTransition(issue, DefaultTransitions.DETECT, IssueChangeContext.createScan(new Date()))).isFalse();
+  }
+
+  private static DefaultIssue newClosedIssue(String resolution) {
+    return new DefaultIssue()
+      .setKey("ABCDE")
+      .setRuleKey(RuleKey.of("js", "S001"))
+      .setResolution(resolution)
+      .setStatus(STATUS_CLOSED)
+      .setNew(false)
+      .setCloseDate(new Date(5_999_999L));
+  }
+
+  private static void setStatusPreviousToClosed(DefaultIssue issue, String previousStatus) {
+    addStatusChange(issue, new Date(), previousStatus, STATUS_CLOSED);
+  }
+
+  private static void addStatusChange(DefaultIssue issue, Date date, String previousStatus, String newStatus) {
+    issue.addChange(new FieldDiffs().setCreationDate(date).setDiff("status", previousStatus, newStatus));
+  }
+
+  private void addResolutionChange(DefaultIssue issue, Date creationDate,
+    @Nullable String previousResolution, @Nullable String newResolution) {
+    checkArgument(previousResolution != null || newResolution != null, "At least one resolution must be non null");
+
+    FieldDiffs fieldDiffs = new FieldDiffs().setCreationDate(creationDate)
+      .setDiff("resolution", emptyIfNull(previousResolution), emptyIfNull(newResolution));
+    issue.addChange(fieldDiffs);
+  }
+
+  private void addResolutionAndStatusChange(DefaultIssue issue, Date creationDate,
+    String previousStatus, String newStatus,
+    @Nullable String previousResolution, @Nullable String newResolution) {
+    checkArgument(previousResolution != null || newResolution != null, "At least one resolution must be non null");
+
+    FieldDiffs fieldDiffs = new FieldDiffs().setCreationDate(creationDate)
+      .setDiff("status", previousStatus, newStatus)
+      .setDiff("resolution", emptyIfNull(previousResolution), emptyIfNull(newResolution));
+    issue.addChange(fieldDiffs);
+  }
+
+  private static String emptyIfNull(@Nullable String newResolution) {
+    return newResolution == null ? "" : newResolution;
+  }
+
   private Collection<String> keys(List<Transition> transitions) {
     return Collections2.transform(transitions, new Function<Transition, String>() {
       @Override

diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListener.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListener.java
index a2d55a04a4a0953560d37c9e32ef3a3ce994a22d..0f7b94220e69a500632025c3a779d766c526379e 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListener.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListener.java
@@ -55,7 +55,8 @@ public interface QGChangeEventListener {
     REOPENED,
     RESOLVED_FP,
     RESOLVED_WF,
-    RESOLVED_FIXED;
+    RESOLVED_FIXED,
+    TO_REVIEW;
 
     protected static final Set<Status> CLOSED_STATUSES = EnumSet.of(CONFIRMED, RESOLVED_FIXED, RESOLVED_FP, RESOLVED_WF);
   }

diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListenersImpl.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListenersImpl.java
index bb8bea4b7ed56061000065bfed95f62f9f1b4165..e1100812ca9b541a46d5b1dbfb624f857ad2a83a 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListenersImpl.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListenersImpl.java
@@ -118,6 +118,8 @@ public class QGChangeEventListenersImpl implements QGChangeEventListeners {
           return QGChangeEventListener.Status.CONFIRMED;
         case Issue.STATUS_REOPENED:
           return QGChangeEventListener.Status.REOPENED;
+        case Issue.STATUS_TO_REVIEW:
+          return QGChangeEventListener.Status.TO_REVIEW;
         case Issue.STATUS_RESOLVED:
           return statusOfResolved(issue);
         default:

diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListenersImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListenersImplTest.java
index 73ca7f593f904a4a599c1defef4863408282a511..021a872cefb7ca98ddf9835dde31beaa334108ee 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListenersImplTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListenersImplTest.java
@@ -284,6 +284,12 @@ public class QGChangeEventListenersImplTest {
     }
   }
 
+  @Test
+  public void test_status_mapping_on_security_hotspots() {
+    assertThat(ChangedIssueImpl.statusOf(new DefaultIssue().setType(RuleType.SECURITY_HOTSPOT).setStatus(Issue.STATUS_TO_REVIEW)))
+      .isEqualTo(QGChangeEventListener.Status.TO_REVIEW);
+  }
+
   private void verifyListenerCalled(QGChangeEventListener listener, QGChangeEvent changeEvent, DefaultIssue... issues) {
     ArgumentCaptor<Set<ChangedIssue>> changedIssuesCaptor = newSetCaptor();
     verify(listener).onIssueChanges(same(changeEvent), changedIssuesCaptor.capture());

diff --git a/sonar-core/src/main/resources/org/sonar/l10n/core.properties b/sonar-core/src/main/resources/org/sonar/l10n/core.properties
index 487bd2071263d4607ce0cf5727dd165d3e38b293..7c359ebaff4c98824ba8dc842c4c11abf5a3a92f 100644 (file)
--- a/sonar-core/src/main/resources/org/sonar/l10n/core.properties
+++ b/sonar-core/src/main/resources/org/sonar/l10n/core.properties
@@ -644,7 +644,6 @@ issue.type.BUG.plural=Bugs
 issue.type.VULNERABILITY.plural=Vulnerabilities
 issue.type.SECURITY_HOTSPOT.plural=Security Hotspots
 
-
 issue.status.REOPENED=Reopened
 issue.status.REOPENED.description=Transitioned to and then back from some other status.
 issue.status.RESOLVED=Resolved
@@ -655,6 +654,8 @@ issue.status.CONFIRMED=Confirmed
 issue.status.CONFIRMED.description=Manually examined and affirmed as an issue that needs attention.
 issue.status.CLOSED=Closed
 issue.status.CLOSED.description=Non-active and no longer requiring attention.
+issue.status.TOREVIEW=To Review
+issue.status.TOREVIEW.description=A review is required to check for a vulnerability.
 
 issue.resolution.FALSE-POSITIVE=False Positive
 issue.resolution.FALSE-POSITIVE.description=Issues that manual review determined were False Positives. Effort from these issues is ignored.

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/issue/Issue.java b/sonar-plugin-api/src/main/java/org/sonar/api/issue/Issue.java
index 681c0c56f9645a29532b3d6735221e6019ce428b..c5add0eab45362d1d691374ad54043cf2d336de2 100644 (file)
--- a/sonar-plugin-api/src/main/java/org/sonar/api/issue/Issue.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/issue/Issue.java
@@ -75,6 +75,11 @@ public interface Issue extends Serializable {
 
   List<String> RESOLUTIONS = asList(RESOLUTION_FALSE_POSITIVE, RESOLUTION_WONT_FIX, RESOLUTION_FIXED, RESOLUTION_REMOVED);
 
+  /**
+   * @since 7.8
+   */
+  String STATUS_TO_REVIEW = "TOREVIEW";
+
   /**
    * Return all available statuses
    *