SONAR-4304 close manual issues marked as resolved

4 files changed, 186 insertions, 14 deletions

diff --git a/sonar-core/src/main/java/org/sonar/core/issue/workflow/IssueWorkflow.java b/sonar-core/src/main/java/org/sonar/core/issue/workflow/IssueWorkflow.java
index f6b8dc2511d..ef38b89798a 100644
--- a/sonar-core/src/main/java/org/sonar/core/issue/workflow/IssueWorkflow.java
+++ b/sonar-core/src/main/java/org/sonar/core/issue/workflow/IssueWorkflow.java
@@ -100,7 +100,7 @@ public class IssueWorkflow implements BatchComponent, ServerComponent, Startable
 
         // automatic transitions
 
-        // Close the issues that do not exist anymore. Note that isEndOfLife() is false on manual issues
+        // Close the "end of life" issues (disabled/deleted rule, deleted component)
       .transition(Transition.builder("automaticclose")
         .from(Issue.STATUS_OPEN).to(Issue.STATUS_CLOSED)
         .conditions(new IsEndOfLife(true))
@@ -119,20 +119,28 @@ public class IssueWorkflow implements BatchComponent, ServerComponent, Startable
         .functions(new SetEndOfLifeResolution(), new SetCloseDate(true))
         .automatic()
         .build())
-        // Close the issues marked as resolved and that do not exist anymore.
-        // Note that false-positives are kept resolved and are not closed.
       .transition(Transition.builder("automaticclose")
         .from(Issue.STATUS_RESOLVED).to(Issue.STATUS_CLOSED)
         .conditions(new IsEndOfLife(true))
         .functions(new SetEndOfLifeResolution(), new SetCloseDate(true))
         .automatic()
         .build())
+      .transition(Transition.builder("automaticclosemanual")
+        .from(Issue.STATUS_RESOLVED).to(Issue.STATUS_CLOSED)
+        .conditions(new IsEndOfLife(false), new HasResolution(Issue.RESOLUTION_FIXED), new IsManual(true))
+        .functions(new SetCloseDate(true))
+        .automatic()
+        .build())
+
+      // Reopen issues that are marked as resolved but that are still alive.
+      // Manual issues are kept resolved.
       .transition(Transition.builder("automaticreopen")
         .from(Issue.STATUS_RESOLVED).to(Issue.STATUS_REOPENED)
-        .conditions(new IsEndOfLife(false), new HasResolution(Issue.RESOLUTION_FIXED))
+        .conditions(new IsEndOfLife(false), new HasResolution(Issue.RESOLUTION_FIXED), new IsManual(false))
         .functions(new SetResolution(null), new SetCloseDate(false))
         .automatic()
-        .build())
+        .build()
+      )
       .build();
   }
 
diff --git a/sonar-core/src/main/java/org/sonar/core/issue/workflow/Transition.java b/sonar-core/src/main/java/org/sonar/core/issue/workflow/Transition.java
index ce4b4aecdb2..3e8d524665e 100644
--- a/sonar-core/src/main/java/org/sonar/core/issue/workflow/Transition.java
+++ b/sonar-core/src/main/java/org/sonar/core/issue/workflow/Transition.java
@@ -77,6 +77,43 @@ public class Transition {
     return true;
   }
 
+  @Override
+  public boolean equals(Object o) {
+    if (this == o) {
+      return true;
+    }
+    if (o == null || getClass() != o.getClass()) {
+      return false;
+    }
+    Transition that = (Transition) o;
+    if (!from.equals(that.from)) {
+      return false;
+    }
+    if (!key.equals(that.key)) {
+      return false;
+    }
+    if (!to.equals(that.to)) {
+      return false;
+    }
+    return true;
+  }
+
+  @Override
+  public int hashCode() {
+    int result = key.hashCode();
+    result = 31 * result + from.hashCode();
+    result = 31 * result + to.hashCode();
+    return result;
+  }
+
+  @Override
+  public String toString() {
+    return String.format("%s->%s->%s", from, key, to);
+  }
+
+  public static Transition create(String key, String from, String to) {
+    return builder(key).from(from).to(to).build();
+  }
 
   public static TransitionBuilder builder(String key) {
     return new TransitionBuilder(key);
diff --git a/sonar-core/src/test/java/org/sonar/core/issue/workflow/IssueWorkflowTest.java b/sonar-core/src/test/java/org/sonar/core/issue/workflow/IssueWorkflowTest.java
index ec9375826b1..2be74b192f9 100644
--- a/sonar-core/src/test/java/org/sonar/core/issue/workflow/IssueWorkflowTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/issue/workflow/IssueWorkflowTest.java
@@ -26,22 +26,20 @@ import org.sonar.api.issue.DefaultTransitions;
 import org.sonar.api.issue.Issue;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.core.issue.DefaultIssue;
-import org.sonar.core.issue.DefaultIssueBuilder;
 import org.sonar.core.issue.IssueChangeContext;
 import org.sonar.core.issue.IssueUpdater;
 
 import javax.annotation.Nullable;
-
 import java.util.Collection;
 import java.util.Date;
 import java.util.List;
 
 import static org.fest.assertions.Assertions.assertThat;
 import static org.fest.assertions.Fail.fail;
+import static org.mockito.Mockito.mock;
 
 public class IssueWorkflowTest {
 
-
   IssueUpdater updater = new IssueUpdater();
   IssueWorkflow workflow = new IssueWorkflow(new FunctionExecutor(updater), updater);
 
@@ -66,7 +64,7 @@ public class IssueWorkflowTest {
   }
 
   @Test
-  public void should_list_out_manual_transitions_from_status_open() throws Exception {
+  public void should_list_out_transitions_from_status_open() throws Exception {
     workflow.start();
 
     DefaultIssue issue = new DefaultIssue().setStatus(Issue.STATUS_OPEN);
@@ -76,7 +74,7 @@ public class IssueWorkflowTest {
   }
 
   @Test
-  public void should_list_out_manual_transitions_from_status_confirmed() throws Exception {
+  public void should_list_out_transitions_from_status_confirmed() throws Exception {
     workflow.start();
 
     DefaultIssue issue = new DefaultIssue().setStatus(Issue.STATUS_CONFIRMED);
@@ -86,7 +84,7 @@ public class IssueWorkflowTest {
   }
 
   @Test
-  public void should_list_out_manual_transitions_from_status_resolved() throws Exception {
+  public void should_list_out_transitions_from_status_resolved() throws Exception {
     workflow.start();
 
     DefaultIssue issue = new DefaultIssue().setStatus(Issue.STATUS_RESOLVED);
@@ -96,7 +94,7 @@ public class IssueWorkflowTest {
   }
 
   @Test
-  public void should_list_out_manual_transitions_from_status_reopen() throws Exception {
+  public void should_list_out_transitions_from_status_reopen() throws Exception {
     workflow.start();
 
     DefaultIssue issue = new DefaultIssue().setStatus(Issue.STATUS_REOPENED);
@@ -202,8 +200,7 @@ public class IssueWorkflowTest {
       .setKey("ABCDE")
       .setStatus(Issue.STATUS_OPEN)
       .setRuleKey(RuleKey.of("squid", "AvoidCycle"))
-      .setAssignee("morgan")
-      .setReporter("simon");
+      .setAssignee("morgan");
 
     workflow.start();
     workflow.doTransition(issue, DefaultTransitions.FALSE_POSITIVE, IssueChangeContext.createScan(new Date()));
@@ -215,6 +212,107 @@ public class IssueWorkflowTest {
     assertThat(issue.assignee()).isNull();
   }
 
+  @Test
+  public void manual_issues_should_be_resolved_then_closed() throws Exception {
+    // Manual issue because of reporter
+    DefaultIssue issue = new DefaultIssue()
+      .setKey("ABCDE")
+      .setStatus(Issue.STATUS_OPEN)
+      .setRuleKey(RuleKey.of("manual", "Performance"))
+      .setReporter("simon");
+
+    workflow.start();
+
+    assertThat(workflow.outTransitions(issue)).containsOnly(
+      Transition.create("confirm", "OPEN", "CONFIRMED"),
+      Transition.create("resolve", "OPEN", "RESOLVED")
+    );
+
+    workflow.doTransition(issue, "resolve", mock(IssueChangeContext.class));
+    assertThat(issue.resolution()).isEqualTo("FIXED");
+    assertThat(issue.status()).isEqualTo("RESOLVED");
+
+    assertThat(workflow.outTransitions(issue)).containsOnly(
+      Transition.create("reopen", "RESOLVED", "REOPENED")
+    );
+
+    workflow.doAutomaticTransition(issue, mock(IssueChangeContext.class));
+    assertThat(issue.resolution()).isEqualTo("FIXED");
+    assertThat(issue.status()).isEqualTo("CLOSED");
+  }
+
+  @Test
+  public void manual_issues_should_be_confirmed_then_kept_open() throws Exception {
+    // Manual issue because of reporter
+    DefaultIssue issue = new DefaultIssue()
+      .setKey("ABCDE")
+      .setStatus(Issue.STATUS_OPEN)
+      .setRuleKey(RuleKey.of("manual", "Performance"))
+      .setReporter("simon");
+
+    workflow.start();
+
+    assertThat(workflow.outTransitions(issue)).containsOnly(
+      Transition.create("confirm", "OPEN", "CONFIRMED"),
+      Transition.create("resolve", "OPEN", "RESOLVED")
+    );
+
+    workflow.doTransition(issue, "confirm", mock(IssueChangeContext.class));
+    assertThat(issue.resolution()).isNull();
+    assertThat(issue.status()).isEqualTo("CONFIRMED");
+
+    assertThat(workflow.outTransitions(issue)).containsOnly(
+      Transition.create("unconfirm", "CONFIRMED", "OPEN"),
+      Transition.create("resolve", "CONFIRMED", "RESOLVED")
+    );
+
+    // keep confirmed and unresolved
+    workflow.doAutomaticTransition(issue, mock(IssueChangeContext.class));
+    assertThat(issue.resolution()).isNull();
+    assertThat(issue.status()).isEqualTo("CONFIRMED");
+
+    // unconfirm
+    workflow.doTransition(issue, "unconfirm", mock(IssueChangeContext.class));
+    assertThat(issue.resolution()).isNull();
+    assertThat(issue.status()).isEqualTo("OPEN");
+  }
+
+  @Test
+  public void manual_issue_on_removed_rule_should_be_closed() throws Exception {
+    // Manual issue because of reporter
+    DefaultIssue issue = new DefaultIssue()
+      .setKey("ABCDE")
+      .setStatus(Issue.STATUS_OPEN)
+      .setRuleKey(RuleKey.of("manual", "Performance"))
+      .setReporter("simon")
+      .setEndOfLife(true)
+      .setOnDisabledRule(true);
+
+    workflow.start();
+
+    workflow.doAutomaticTransition(issue, mock(IssueChangeContext.class));
+    assertThat(issue.resolution()).isEqualTo("REMOVED");
+    assertThat(issue.status()).isEqualTo("CLOSED");
+  }
+
+  @Test
+  public void manual_issue_on_removed_component_should_be_closed() throws Exception {
+    // Manual issue because of reporter
+    DefaultIssue issue = new DefaultIssue()
+      .setKey("ABCDE")
+      .setStatus(Issue.STATUS_OPEN)
+      .setRuleKey(RuleKey.of("manual", "Performance"))
+      .setReporter("simon")
+      .setEndOfLife(true)
+      .setOnDisabledRule(false);
+
+    workflow.start();
+
+    workflow.doAutomaticTransition(issue, mock(IssueChangeContext.class));
+    assertThat(issue.resolution()).isEqualTo("FIXED");
+    assertThat(issue.status()).isEqualTo("CLOSED");
+  }
+
   private Collection<String> keys(List<Transition> transitions) {
     return Collections2.transform(transitions, new Function<Transition, String>() {
       @Override
diff --git a/sonar-core/src/test/java/org/sonar/core/issue/workflow/TransitionTest.java b/sonar-core/src/test/java/org/sonar/core/issue/workflow/TransitionTest.java
index caf383720f4..2dfa2689e37 100644
--- a/sonar-core/src/test/java/org/sonar/core/issue/workflow/TransitionTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/issue/workflow/TransitionTest.java
@@ -46,6 +46,7 @@ public class TransitionTest {
     assertThat(transition.to()).isEqualTo("CLOSED");
     assertThat(transition.conditions()).containsOnly(condition1, condition2);
     assertThat(transition.functions()).containsOnly(function1, function2);
+    assertThat(transition.automatic()).isFalse();
   }
 
   @Test
@@ -116,4 +117,32 @@ public class TransitionTest {
     when(condition2.matches(issue)).thenReturn(true);
     assertThat(transition.supports(issue)).isTrue();
   }
+
+  @Test
+  public void test_equals_and_hashCode() throws Exception {
+    Transition t1 = Transition.create("resolve", "OPEN", "RESOLVED");
+    Transition t2 = Transition.create("resolve", "REOPENED", "RESOLVED");
+    Transition t3 = Transition.create("confirm", "OPEN", "CONFIRMED");
+
+    assertThat(t1).isNotEqualTo(t2);
+    assertThat(t1).isNotEqualTo(t3);
+    assertThat(t1).isEqualTo(t1);
+
+    assertThat(t1.hashCode()).isEqualTo(t1.hashCode());
+  }
+
+  @Test
+  public void test_toString() throws Exception {
+    Transition t1 = Transition.create("resolve", "OPEN", "RESOLVED");
+    assertThat(t1.toString()).isEqualTo("OPEN->resolve->RESOLVED");
+  }
+
+  @Test
+  public void test_automatic_transition() throws Exception {
+    Transition transition = Transition.builder("close")
+      .from("OPEN").to("CLOSED")
+      .automatic()
+      .build();
+    assertThat(transition.automatic()).isTrue();
+  }
 }