Compute engine must not load ActiveRules with status REMOVED

diff --git a/it/it-projects/issue/workflow/sonar-project.properties b/it/it-projects/issue/workflow/sonar-project.properties
new file mode 100644 (file)
index 0000000..fbfcc69
--- /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 (file)
index 0000000..e2ae9e3
--- /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 (file)
index 0000000..16cc5ac
--- /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 4f61aa5edc4484d0d6b7710f71f9d90b9371a7d4..b0f83bec16897f1be05c75df52bf776f4ef28fa3 100644 (file)
--- 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 (file)
index 0000000..4e8b67e
--- /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<Issue> 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 (file)
index 0000000..608f80c
--- /dev/null
+++ b/it/it-tests/src/test/resources/issue/suite/IssueWorkflowTest/xoo-one-issue-per-line-profile.xml
@@ -0,0 +1,12 @@
+<?xml version="1.0"?><!-- Generated by Sonar -->
+<profile>
+  <name>xoo-one-issue-per-line-profile</name>
+  <language>xoo</language>
+  <rules>
+    <rule>
+      <repositoryKey>xoo</repositoryKey>
+      <key>OneIssuePerLine</key>
+      <priority>CRITICAL</priority>
+    </rule>
+  </rules>
+</profile>

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 3b86d2c24e5b2a32948428b5b668f47b226f22fa..57bd78c1e1960fae4c6bc4833ce471e48a728787 100644 (file)
--- 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 ee3314cee5ad5a58f821fda251da7abbd32bd53f..63c96ab24b616c3e36ce009a323fda670be3e0e1 100644 (file)
--- 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 ce577489ae1a60aadb68e37b2efed9e23782e6df..18fb9e5001b1fcccf521e82af5ce76b03f23cca7 100644 (file)
--- 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<ActiveRule> validActiveRules = from(activeRules).filter(new IsValid()).toList();
+    activeRulesHolder.set(validActiveRules);
+  }
+
+  private class IsValid implements Predicate<ActiveRule> {
+    @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 b4c723a3c2bc5b7428eb40cf4add00e93ba8ac30..1497bbce6fd39b294582365bfe674935705a3cd4 100644 (file)
--- 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 6f1e7e5810c25a977403ff7f1b21176d6e3a195d..d50be77dafa6cb2341450830607c1f1289fc05ef 100644 (file)
--- 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 6b4180d8af68c99aa406962ed53af71984a70091..4003d77a4f00a7329c8671d5c6dc8a7b06fd3f60 100644 (file)
--- 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<ActiveRule> ar1 = activeRulesHolder.get(RuleKey.of("java", "S001"));
+
+    Optional<ActiveRule> ar1 = activeRulesHolder.get(XOO_X1);
     assertThat(ar1.get().getSeverity()).isEqualTo(Severity.BLOCKER);
     assertThat(ar1.get().getParams()).containsExactly(MapEntry.entry("p1", "v1"));
-    
-    Optional<ActiveRule> ar2 = activeRulesHolder.get(RuleKey.of("java", "S002"));
+
+    Optional<ActiveRule> 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();
   }
 }