diff options
9 files changed, 278 insertions, 203 deletions
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 c2314f17246..081c3217138 100644 --- 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 ed2882a39bc..b9891a2457f 100644 --- 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 2afa5feb838..1a7cfe32e1b 100644 --- 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 1028b9c69ce..4151f7280b6 100644 --- 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,10 +340,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(STATUS_CLOSED); assertThat(issue.updateDate()).isNull(); @@ -324,6 +351,21 @@ public class IssueWorkflowTest { } @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) { DefaultIssue[] issues = Arrays.stream(SUPPORTED_RESOLUTIONS_FOR_UNCLOSING) @@ -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 a2d55a04a4a..0f7b94220e6 100644 --- 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 bb8bea4b7ed..e1100812ca9 100644 --- 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 73ca7f593f9..021a872cefb 100644 --- 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 487bd207126..7c359ebaff4 100644 --- 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 681c0c56f96..c5add0eab45 100644 --- 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 @@ -76,6 +76,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 * * @since 4.4 |