diff --git a/it/it-projects/issue/workflow/sonar-project.properties b/it/it-projects/issue/workflow/sonar-project.properties new file mode 100644 index 00000000000..fbfcc6930d0 --- /dev/null +++ b/it/it-projects/issue/workflow/sonar-project.properties @@ -0,0 +1,4 @@ +sonar.projectKey=workflow +sonar.projectName=Workflow +sonar.projectVersion=1.0-SNAPSHOT +sonar.sources=src diff --git a/it/it-projects/issue/workflow/src/Sample.xoo b/it/it-projects/issue/workflow/src/Sample.xoo new file mode 100644 index 00000000000..e2ae9e37278 --- /dev/null +++ b/it/it-projects/issue/workflow/src/Sample.xoo @@ -0,0 +1,3 @@ +this is some +xoo +code diff --git a/it/it-projects/issue/workflow/src/Sample.xoo.measures b/it/it-projects/issue/workflow/src/Sample.xoo.measures new file mode 100644 index 00000000000..16cc5ac1540 --- /dev/null +++ b/it/it-projects/issue/workflow/src/Sample.xoo.measures @@ -0,0 +1,10 @@ +lines:120 +ncloc:100 +complexity:7 +comment_lines:3 +public_api:5 +public_undocumented_api:2 +lines_to_cover:80 +uncovered_lines:70 +conditions_to_cover:10 +uncovered_conditions:9 diff --git a/it/it-tests/src/test/java/issue/suite/IssueTestSuite.java b/it/it-tests/src/test/java/issue/suite/IssueTestSuite.java index 4f61aa5edc4..b0f83bec168 100644 --- a/it/it-tests/src/test/java/issue/suite/IssueTestSuite.java +++ b/it/it-tests/src/test/java/issue/suite/IssueTestSuite.java @@ -13,7 +13,7 @@ import util.ItUtils; @RunWith(Suite.class) @Suite.SuiteClasses({ - ManualRulesTest.class, CommonRulesTest.class + CommonRulesTest.class, IssueWorkflowTest.class, ManualRulesTest.class, }) public class IssueTestSuite { diff --git a/it/it-tests/src/test/java/issue/suite/IssueWorkflowTest.java b/it/it-tests/src/test/java/issue/suite/IssueWorkflowTest.java new file mode 100644 index 00000000000..4e8b67e2c3c --- /dev/null +++ b/it/it-tests/src/test/java/issue/suite/IssueWorkflowTest.java @@ -0,0 +1,55 @@ +package issue.suite; + +import com.sonar.orchestrator.Orchestrator; +import com.sonar.orchestrator.build.SonarRunner; +import com.sonar.orchestrator.locator.FileLocation; +import java.util.List; +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Test; +import org.sonar.wsclient.issue.Issue; +import org.sonar.wsclient.issue.IssueClient; +import org.sonar.wsclient.issue.IssueQuery; + +import static org.assertj.core.api.Assertions.assertThat; +import static util.ItUtils.projectDir; + +public class IssueWorkflowTest { + + @ClassRule + public static Orchestrator orchestrator = IssueTestSuite.ORCHESTRATOR; + + @Before + public void setUp() { + orchestrator.resetData(); + } + + /** + * Issue on a disabled rule (uninstalled plugin or rule deactivated from quality profile) must + * be CLOSED with resolution REMOVED + */ + @Test + public void issue_is_closed_as_removed_when_rule_is_disabled() throws Exception { + orchestrator.getServer().restoreProfile(FileLocation.ofClasspath("/issue/suite/IssueWorkflowTest/xoo-one-issue-per-line-profile.xml")); + orchestrator.getServer().provisionProject("workflow", "Workflow"); + orchestrator.getServer().associateProjectToQualityProfile("workflow", "xoo", "xoo-one-issue-per-line-profile"); + + SonarRunner analysis = SonarRunner.create(projectDir("issue/workflow")); + orchestrator.executeBuild(analysis); + + IssueClient issueClient = orchestrator.getServer().wsClient().issueClient(); + List issues = issueClient.find(IssueQuery.create().rules("xoo:OneIssuePerLine")).list(); + assertThat(issues).isNotEmpty(); + + // re-analyze with profile "empty". The rule is disabled so the issues must be closed + orchestrator.getServer().associateProjectToQualityProfile("workflow", "xoo", "empty"); + analysis = SonarRunner.create(projectDir("issue/workflow")); + orchestrator.executeBuild(analysis); + issues = issueClient.find(IssueQuery.create().rules("xoo:OneIssuePerLine").componentRoots("workflow")).list(); + assertThat(issues).isNotEmpty(); + for (Issue issue : issues) { + assertThat(issue.status()).isEqualTo("CLOSED"); + assertThat(issue.resolution()).isEqualTo("REMOVED"); + } + } +} diff --git a/it/it-tests/src/test/resources/issue/suite/IssueWorkflowTest/xoo-one-issue-per-line-profile.xml b/it/it-tests/src/test/resources/issue/suite/IssueWorkflowTest/xoo-one-issue-per-line-profile.xml new file mode 100644 index 00000000000..608f80cae96 --- /dev/null +++ b/it/it-tests/src/test/resources/issue/suite/IssueWorkflowTest/xoo-one-issue-per-line-profile.xml @@ -0,0 +1,12 @@ + + + xoo-one-issue-per-line-profile + xoo + + + xoo + OneIssuePerLine + CRITICAL + + + diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/RuleRepository.java b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/RuleRepository.java index 3b86d2c24e5..57bd78c1e19 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/RuleRepository.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/RuleRepository.java @@ -25,4 +25,5 @@ public interface RuleRepository { Rule getByKey(RuleKey key); + boolean hasKey(RuleKey key); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/RuleRepositoryImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/RuleRepositoryImpl.java index ee3314cee5a..63c96ab24b6 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/RuleRepositoryImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/RuleRepositoryImpl.java @@ -34,4 +34,9 @@ public class RuleRepositoryImpl implements RuleRepository { public Rule getByKey(RuleKey key) { return cache.get(key); } + + @Override + public boolean hasKey(RuleKey key) { + return cache.getNullable(key) != null; + } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/step/FeedActiveRulesStep.java b/server/sonar-server/src/main/java/org/sonar/server/computation/step/FeedActiveRulesStep.java index ce577489ae1..18fb9e5001b 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/step/FeedActiveRulesStep.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/step/FeedActiveRulesStep.java @@ -19,25 +19,34 @@ */ package org.sonar.server.computation.step; +import com.google.common.base.Predicate; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import javax.annotation.Nonnull; import org.sonar.api.rule.RuleKey; +import org.sonar.api.rule.RuleStatus; import org.sonar.batch.protocol.output.BatchReport; import org.sonar.core.util.CloseableIterator; import org.sonar.server.computation.batch.BatchReportReader; +import org.sonar.server.computation.issue.Rule; +import org.sonar.server.computation.issue.RuleRepository; import org.sonar.server.computation.qualityprofile.ActiveRule; import org.sonar.server.computation.qualityprofile.ActiveRulesHolderImpl; +import static com.google.common.collect.FluentIterable.from; + public class FeedActiveRulesStep implements ComputationStep { private final BatchReportReader batchReportReader; private final ActiveRulesHolderImpl activeRulesHolder; + private final RuleRepository ruleRepository; - public FeedActiveRulesStep(BatchReportReader batchReportReader, ActiveRulesHolderImpl activeRulesHolder) { + public FeedActiveRulesStep(BatchReportReader batchReportReader, ActiveRulesHolderImpl activeRulesHolder, RuleRepository ruleRepository) { this.batchReportReader = batchReportReader; this.activeRulesHolder = activeRulesHolder; + this.ruleRepository = ruleRepository; } @Override @@ -49,7 +58,20 @@ public class FeedActiveRulesStep implements ComputationStep { activeRules.add(convert(batchActiveRule)); } } - activeRulesHolder.set(activeRules); + + List validActiveRules = from(activeRules).filter(new IsValid()).toList(); + activeRulesHolder.set(validActiveRules); + } + + private class IsValid implements Predicate { + @Override + public boolean apply(@Nonnull ActiveRule input) { + if (ruleRepository.hasKey(input.getRuleKey())) { + Rule rule = ruleRepository.getByKey(input.getRuleKey()); + return rule.getStatus() != RuleStatus.REMOVED; + } + return false; + } } @Override diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/RuleRepositoryImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/RuleRepositoryImplTest.java index b4c723a3c2b..1497bbce6fd 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/RuleRepositoryImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/RuleRepositoryImplTest.java @@ -27,6 +27,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.mockito.internal.verification.VerificationModeFactory.times; import static org.sonar.db.rule.RuleTesting.XOO_X1; +import static org.sonar.db.rule.RuleTesting.XOO_X2; public class RuleRepositoryImplTest { @@ -43,4 +44,12 @@ public class RuleRepositoryImplTest { assertThat(underTest.getByKey(XOO_X1).getKey()).isEqualTo(XOO_X1); verify(cacheLoader, times(1)).load(XOO_X1); } + + @Test + public void hasKey() { + when(cacheLoader.load(XOO_X1)).thenReturn(new DumbRule(XOO_X1)); + + assertThat(underTest.hasKey(XOO_X1)).isTrue(); + assertThat(underTest.hasKey(XOO_X2)).isFalse(); + } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/RuleRepositoryRule.java b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/RuleRepositoryRule.java index 6f1e7e5810c..d50be77dafa 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/RuleRepositoryRule.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/RuleRepositoryRule.java @@ -43,6 +43,11 @@ public class RuleRepositoryRule extends ExternalResource implements RuleReposito return rule; } + @Override + public boolean hasKey(RuleKey key) { + return rulesByKey.containsKey(key); + } + public DumbRule add(RuleKey key) { DumbRule rule = new DumbRule(key); rulesByKey.put(key, rule); diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/step/FeedActiveRulesStepTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/step/FeedActiveRulesStepTest.java index 6b4180d8af6..4003d77a4f0 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/step/FeedActiveRulesStepTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/step/FeedActiveRulesStepTest.java @@ -20,20 +20,24 @@ package org.sonar.server.computation.step; import com.google.common.base.Optional; -import java.util.Arrays; import org.assertj.core.data.MapEntry; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; -import org.sonar.api.rule.RuleKey; +import org.sonar.api.rule.RuleStatus; import org.sonar.api.rule.Severity; import org.sonar.batch.protocol.Constants; import org.sonar.batch.protocol.output.BatchReport; import org.sonar.server.computation.batch.BatchReportReaderRule; +import org.sonar.server.computation.issue.DumbRule; +import org.sonar.server.computation.issue.RuleRepositoryRule; import org.sonar.server.computation.qualityprofile.ActiveRule; import org.sonar.server.computation.qualityprofile.ActiveRulesHolderImpl; +import static java.util.Arrays.asList; import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.db.rule.RuleTesting.XOO_X1; +import static org.sonar.db.rule.RuleTesting.XOO_X2; public class FeedActiveRulesStepTest { @@ -43,31 +47,62 @@ public class FeedActiveRulesStepTest { @Rule public BatchReportReaderRule batchReportReader = new BatchReportReaderRule(); + @Rule + public RuleRepositoryRule ruleRepository = new RuleRepositoryRule(); + ActiveRulesHolderImpl activeRulesHolder = new ActiveRulesHolderImpl(); - FeedActiveRulesStep underTest = new FeedActiveRulesStep(batchReportReader, activeRulesHolder); + FeedActiveRulesStep underTest = new FeedActiveRulesStep(batchReportReader, activeRulesHolder, ruleRepository); @Test - public void write() throws Exception { + public void feed_active_rules() throws Exception { + ruleRepository.add(XOO_X1); + ruleRepository.add(XOO_X2); + BatchReport.ActiveRule.Builder batch1 = BatchReport.ActiveRule.newBuilder() - .setRuleRepository("java").setRuleKey("S001") + .setRuleRepository(XOO_X1.repository()).setRuleKey(XOO_X1.rule()) .setSeverity(Constants.Severity.BLOCKER); batch1.addParamBuilder().setKey("p1").setValue("v1").build(); BatchReport.ActiveRule.Builder batch2 = BatchReport.ActiveRule.newBuilder() - .setRuleRepository("java").setRuleKey("S002").setSeverity(Constants.Severity.MAJOR); - batchReportReader.putActiveRules(Arrays.asList(batch1.build(), batch2.build())); + .setRuleRepository(XOO_X2.repository()).setRuleKey(XOO_X2.rule()).setSeverity(Constants.Severity.MAJOR); + batchReportReader.putActiveRules(asList(batch1.build(), batch2.build())); underTest.execute(); assertThat(activeRulesHolder.getAll()).hasSize(2); - - Optional ar1 = activeRulesHolder.get(RuleKey.of("java", "S001")); + + Optional ar1 = activeRulesHolder.get(XOO_X1); assertThat(ar1.get().getSeverity()).isEqualTo(Severity.BLOCKER); assertThat(ar1.get().getParams()).containsExactly(MapEntry.entry("p1", "v1")); - - Optional ar2 = activeRulesHolder.get(RuleKey.of("java", "S002")); + + Optional ar2 = activeRulesHolder.get(XOO_X2); assertThat(ar2.get().getSeverity()).isEqualTo(Severity.MAJOR); assertThat(ar2.get().getParams()).isEmpty(); + } + @Test + public void ignore_rules_with_status_REMOVED() throws Exception { + ruleRepository.add(new DumbRule(XOO_X1).setStatus(RuleStatus.REMOVED)); + + BatchReport.ActiveRule.Builder batch1 = BatchReport.ActiveRule.newBuilder() + .setRuleRepository(XOO_X1.repository()).setRuleKey(XOO_X1.rule()) + .setSeverity(Constants.Severity.BLOCKER); + batchReportReader.putActiveRules(asList(batch1.build())); + + underTest.execute(); + + assertThat(activeRulesHolder.getAll()).isEmpty(); + } + + @Test + public void ignore_not_found_rules() throws Exception { + BatchReport.ActiveRule.Builder batch1 = BatchReport.ActiveRule.newBuilder() + .setRuleRepository(XOO_X1.repository()).setRuleKey(XOO_X1.rule()) + .setSeverity(Constants.Severity.BLOCKER); + batchReportReader.putActiveRules(asList(batch1.build())); + + underTest.execute(); + + assertThat(activeRulesHolder.getAll()).isEmpty(); } }