SONAR-15352 Reopened closed hotspots should bring back their previous state

server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ComponentIssuesLoader.java

@@ -46,6 +46,7 @@ import org.sonar.db.issue.IssueMapper;
 
 import static com.google.common.base.Preconditions.checkState;
 import static java.util.Collections.emptyList;
+import static java.util.Collections.unmodifiableList;
 import static java.util.stream.Collectors.groupingBy;
 import static java.util.stream.Collectors.toList;
 import static org.sonar.api.issue.Issue.STATUS_CLOSED;
@@ -203,7 +204,7 @@ public class ComponentIssuesLoader {
 
   /**
    * Load closed issues for the specified Component, which have at least one line diff in changelog AND are
-   * neither hotspots nor manual vulnerabilities.
+   * not manual vulnerabilities.
    * <p>
    * Closed issues do not have a line number in DB (it is unset when the issue is closed), this method
    * returns {@link DefaultIssue} objects which line number is populated from the most recent diff logging
@@ -230,7 +231,7 @@ public class ComponentIssuesLoader {
   private static List<DefaultIssue> loadClosedIssues(DbSession dbSession, String componentUuid, long closeDateAfter) {
     ClosedIssuesResultHandler handler = new ClosedIssuesResultHandler();
     dbSession.getMapper(IssueMapper.class).scrollClosedByComponentUuid(componentUuid, closeDateAfter, handler);
-    return ImmutableList.copyOf(handler.issues);
+    return unmodifiableList(handler.issues);
   }
 
   private static class ClosedIssuesResultHandler implements ResultHandler<IssueDto> {

server/sonar-db-dao/src/main/resources/org/sonar/db/issue/IssueMapper.xml

@@ -228,7 +228,6 @@
       and i.status = 'CLOSED'
       and i.issue_close_date is not null
       and i.issue_close_date >= #{closeDateAfter,jdbcType=BIGINT}
-      and i.issue_type <> 4
     order by
       i.kee, ic.issue_change_creation_date desc
   </select>

server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueMapperTest.java

@@ -313,11 +313,11 @@ public class IssueMapperTest {
   }
 
   @Test
-  public void scrollClosedByComponentUuid_does_not_return_closed_issues_of_type_SECURITY_HOTSPOT() {
+  public void scrollClosedByComponentUuid_returns_closed_issues_of_type_SECURITY_HOTSPOT() {
     RuleType ruleType = randomSupportedRuleType();
     ComponentDto component = randomComponent();
     IssueDto securityHotspotIssue = insertNewClosedIssue(component, RuleType.SECURITY_HOTSPOT);
-    insertToClosedDiff(securityHotspotIssue);
+    IssueChangeDto hotspotChangedData = insertToClosedDiff(securityHotspotIssue);
     IssueDto issue = insertNewClosedIssue(component, ruleType);
     IssueChangeDto issueChange = insertToClosedDiff(issue);
 
@@ -326,7 +326,9 @@ public class IssueMapperTest {
 
     assertThat(resultHandler.issues)
       .extracting(IssueDto::getKey, t -> t.getClosedChangeData().get())
-      .containsOnly(tuple(issue.getKey(), issueChange.getChangeData()));
+      .containsOnly(
+        tuple(issue.getKey(), issueChange.getChangeData()),
+        tuple(securityHotspotIssue.getKey(), hotspotChangedData.getChangeData()));
   }
 
   @Test

server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/IsHotspot.java

@@ -0,0 +1,33 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2021 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.server.issue.workflow;
+
+import org.sonar.api.issue.Issue;
+import org.sonar.api.rules.RuleType;
+import org.sonar.core.issue.DefaultIssue;
+
+enum IsHotspot implements Condition {
+  INSTANCE;
+
+  @Override
+  public boolean matches(Issue issue) {
+    return ((DefaultIssue) issue).type() == RuleType.SECURITY_HOTSPOT;
+  }
+}

server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/IssueWorkflow.java

@@ -283,6 +283,26 @@ public class IssueWorkflow implements Startable {
           IsNotHotspot.INSTANCE)
         .functions(RestoreResolutionFunction.INSTANCE, UnsetCloseDate.INSTANCE)
         .automatic()
+        .build())
+
+    // reopen closed hotspots
+      .transition(Transition.builder("automaticunclosetoreview")
+        .from(STATUS_CLOSED).to(STATUS_TO_REVIEW)
+        .conditions(
+          new PreviousStatusWas(STATUS_TO_REVIEW),
+          new HasResolution(RESOLUTION_REMOVED, RESOLUTION_FIXED),
+          IsHotspot.INSTANCE)
+        .functions(RestoreResolutionFunction.INSTANCE, UnsetCloseDate.INSTANCE)
+        .automatic()
+        .build())
+      .transition(Transition.builder("automaticunclosereviewed")
+        .from(STATUS_CLOSED).to(STATUS_REVIEWED)
+        .conditions(
+          new PreviousStatusWas(STATUS_REVIEWED),
+          new HasResolution(RESOLUTION_REMOVED, RESOLUTION_FIXED),
+          IsHotspot.INSTANCE)
+        .functions(RestoreResolutionFunction.INSTANCE, UnsetCloseDate.INSTANCE)
+        .automatic()
         .build());
   }
 

server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/IssueWorkflowForSecurityHotspotsTest.java

@@ -22,7 +22,6 @@ package org.sonar.server.issue.workflow;
 import com.tngtech.java.junit.dataprovider.DataProvider;
 import com.tngtech.java.junit.dataprovider.DataProviderRunner;
 import com.tngtech.java.junit.dataprovider.UseDataProvider;
-import java.util.Arrays;
 import java.util.Calendar;
 import java.util.Collection;
 import java.util.Date;
@@ -53,15 +52,14 @@ import static org.sonar.api.issue.Issue.STATUS_REVIEWED;
 import static org.sonar.api.issue.Issue.STATUS_TO_REVIEW;
 import static org.sonar.api.rules.RuleType.SECURITY_HOTSPOT;
 import static org.sonar.db.rule.RuleTesting.XOO_X1;
+import static org.sonar.server.issue.workflow.IssueWorkflowTest.emptyIfNull;
 
 @RunWith(DataProviderRunner.class)
 public class IssueWorkflowForSecurityHotspotsTest {
-
   private static final IssueChangeContext SOME_CHANGE_CONTEXT = IssueChangeContext.createUser(new Date(), "USER1");
 
-  private IssueFieldsSetter updater = new IssueFieldsSetter();
-
-  private IssueWorkflow underTest = new IssueWorkflow(new FunctionExecutor(updater), updater);
+  private final IssueFieldsSetter updater = new IssueFieldsSetter();
+  private final IssueWorkflow underTest = new IssueWorkflow(new FunctionExecutor(updater), updater);
 
   @Test
   @UseDataProvider("anyResolutionIncludingNone")
@@ -255,31 +253,26 @@ public class IssueWorkflowForSecurityHotspotsTest {
   }
 
   @Test
-  @UseDataProvider("allStatusesLeadingToClosed")
-  public void do_not_automatically_reopen_closed_issues_of_security_hotspots(String previousStatus) {
-    DefaultIssue[] hotspots = Stream.of(RESOLUTION_FIXED, RESOLUTION_REMOVED)
-      .map(resolution -> {
-        DefaultIssue issue = newClosedHotspot(resolution);
-        setStatusPreviousToClosed(issue, previousStatus);
-        return issue;
-      })
-      .toArray(DefaultIssue[]::new);
+  public void automatically_reopen_closed_security_hotspots() {
+    DefaultIssue hotspot1 = newClosedHotspot(RESOLUTION_REMOVED);
+    setStatusPreviousToClosed(hotspot1, STATUS_REVIEWED, RESOLUTION_SAFE, RESOLUTION_REMOVED);
+
+    DefaultIssue hotspot2 = newClosedHotspot(RESOLUTION_FIXED);
+    setStatusPreviousToClosed(hotspot2, STATUS_TO_REVIEW, null, RESOLUTION_FIXED);
+
     Date now = new Date();
     underTest.start();
 
-    Arrays.stream(hotspots).forEach(hotspot -> {
-      underTest.doAutomaticTransition(hotspot, IssueChangeContext.createScan(now));
+    underTest.doAutomaticTransition(hotspot1,  IssueChangeContext.createScan(now));
+    underTest.doAutomaticTransition(hotspot2,  IssueChangeContext.createScan(now));
 
-      assertThat(hotspot.status()).isEqualTo(STATUS_CLOSED);
-      assertThat(hotspot.updateDate()).isNull();
-    });
-  }
+    assertThat(hotspot1.updateDate()).isNotNull();
+    assertThat(hotspot1.status()).isEqualTo(STATUS_REVIEWED);
+    assertThat(hotspot1.resolution()).isEqualTo(RESOLUTION_SAFE);
 
-  @DataProvider
-  public static Object[][] allStatusesLeadingToClosed() {
-    return Stream.of(STATUS_TO_REVIEW, STATUS_REVIEWED)
-      .map(t -> new Object[] {t})
-      .toArray(Object[][]::new);
+    assertThat(hotspot2.updateDate()).isNotNull();
+    assertThat(hotspot2.status()).isEqualTo(STATUS_TO_REVIEW);
+    assertThat(hotspot2.resolution()).isNull();
   }
 
   @Test
@@ -299,12 +292,15 @@ public class IssueWorkflowForSecurityHotspotsTest {
     return transitions.stream().map(Transition::key).collect(MoreCollectors.toList());
   }
 
-  private static void setStatusPreviousToClosed(DefaultIssue hotspot, String previousStatus) {
-    addStatusChange(hotspot, new Date(), previousStatus, STATUS_CLOSED);
+  private static void setStatusPreviousToClosed(DefaultIssue hotspot, String previousStatus, @Nullable String previousResolution, @Nullable String newResolution) {
+    addStatusChange(hotspot, new Date(), previousStatus, STATUS_CLOSED, previousResolution, newResolution);
   }
 
-  private static void addStatusChange(DefaultIssue issue, Date date, String previousStatus, String newStatus) {
-    issue.addChange(new FieldDiffs().setCreationDate(date).setDiff("status", previousStatus, newStatus));
+  private static void addStatusChange(DefaultIssue issue, Date date, String previousStatus, String newStatus, @Nullable String previousResolution, @Nullable String newResolution) {
+    issue.addChange(new FieldDiffs()
+      .setCreationDate(date)
+      .setDiff("status", previousStatus, newStatus)
+      .setDiff("resolution", emptyIfNull(previousResolution), emptyIfNull(newResolution)));
   }
 
   private static DefaultIssue newClosedHotspot(String resolution) {

server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/IssueWorkflowTest.java

@@ -466,7 +466,7 @@ public class IssueWorkflowTest {
     issue.addChange(fieldDiffs);
   }
 
-  private static String emptyIfNull(@Nullable String newResolution) {
+  static String emptyIfNull(@Nullable String newResolution) {
     return newResolution == null ? "" : newResolution;
   }