SONAR-4679 Introduce a chain of responsibility for issue filtering

4 files changed, 160 insertions, 19 deletions

diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/DefaultIssueFilterChain.java b/sonar-batch/src/main/java/org/sonar/batch/issue/DefaultIssueFilterChain.java
new file mode 100644
index 00000000000..2644561e422
--- /dev/null
+++ b/sonar-batch/src/main/java/org/sonar/batch/issue/DefaultIssueFilterChain.java
@@ -0,0 +1,53 @@
+/*
+ * SonarQube, open source software quality management tool.
+ * Copyright (C) 2008-2013 SonarSource
+ * mailto:contact AT sonarsource DOT com
+ *
+ * SonarQube 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.
+ *
+ * SonarQube 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.batch.issue;
+
+import com.google.common.collect.ImmutableList;
+import org.sonar.api.issue.Issue;
+import org.sonar.api.issue.batch.IssueFilter;
+import org.sonar.api.issue.batch.IssueFilterChain;
+
+import java.util.List;
+
+public class DefaultIssueFilterChain implements IssueFilterChain {
+
+  private final List<IssueFilter> filters;
+
+  public DefaultIssueFilterChain(IssueFilter... filters) {
+    this.filters = ImmutableList.copyOf(filters);
+  }
+
+  public DefaultIssueFilterChain() {
+    this.filters = ImmutableList.of();
+  }
+
+  private DefaultIssueFilterChain(List<IssueFilter> filters) {
+    this.filters = filters;
+  }
+
+  @Override
+  public boolean accept(Issue issue) {
+    if (filters.isEmpty()) {
+      return true;
+    } else {
+      return filters.get(0).accept(issue, new DefaultIssueFilterChain(filters.subList(1, filters.size())));
+    }
+  }
+}
diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/IssueFilters.java b/sonar-batch/src/main/java/org/sonar/batch/issue/IssueFilters.java
index 190b4ea1916..dc79211d39c 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/issue/IssueFilters.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/issue/IssueFilters.java
@@ -20,7 +20,7 @@
 package org.sonar.batch.issue;
 
 import org.sonar.api.BatchExtension;
-import org.sonar.api.issue.IssueFilter;
+import org.sonar.api.issue.batch.IssueFilter;
 import org.sonar.api.issue.internal.DefaultIssue;
 import org.sonar.api.rules.Violation;
 import org.sonar.batch.ViolationFilters;
@@ -31,28 +31,39 @@ public class IssueFilters implements BatchExtension {
 
   private final ViolationFilters deprecatedFilters;
   private final DeprecatedViolations deprecatedViolations;
+  private final org.sonar.api.issue.IssueFilter[] exclusionFilters;
   private final IssueFilter[] filters;
 
-  public IssueFilters(ViolationFilters deprecatedFilters, DeprecatedViolations deprecatedViolations, IssueFilter[] filters) {
+  public IssueFilters(ViolationFilters deprecatedFilters, DeprecatedViolations deprecatedViolations, org.sonar.api.issue.IssueFilter[] exclusionFilters, IssueFilter[] filters) {
     this.deprecatedFilters = deprecatedFilters;
     this.deprecatedViolations = deprecatedViolations;
+    this.exclusionFilters = exclusionFilters;
     this.filters = filters;
   }
 
+  public IssueFilters(ViolationFilters deprecatedFilters, DeprecatedViolations deprecatedViolations, org.sonar.api.issue.IssueFilter[] exclusionFilters) {
+    this(deprecatedFilters, deprecatedViolations, exclusionFilters, new IssueFilter[0]);
+  }
+
   public IssueFilters(ViolationFilters deprecatedFilters, DeprecatedViolations deprecatedViolations) {
-    this(deprecatedFilters, deprecatedViolations, new IssueFilter[0]);
+    this(deprecatedFilters, deprecatedViolations, new org.sonar.api.issue.IssueFilter[0]);
   }
 
   public boolean accept(DefaultIssue issue, @Nullable Violation violation) {
-    for (IssueFilter filter : filters) {
-      if (!filter.accept(issue)) {
-        return false;
+    if(new DefaultIssueFilterChain(filters).accept(issue)) {
+      // Apply deprecated rules only if filter chain accepts the current issue
+      for (org.sonar.api.issue.IssueFilter filter : exclusionFilters) {
+        if (!filter.accept(issue)) {
+          return false;
+        }
       }
+      if (!deprecatedFilters.isEmpty()) {
+        Violation v = violation != null ? violation : deprecatedViolations.toViolation(issue);
+        return !deprecatedFilters.isIgnored(v);
+      }
+      return true;
+    } else {
+      return false;
     }
-    if (!deprecatedFilters.isEmpty()) {
-      Violation v = violation != null ? violation : deprecatedViolations.toViolation(issue);
-      return !deprecatedFilters.isIgnored(v);
-    }
-    return true;
   }
 }
diff --git a/sonar-batch/src/test/java/org/sonar/batch/issue/DefaultIssueFilterChainTest.java b/sonar-batch/src/test/java/org/sonar/batch/issue/DefaultIssueFilterChainTest.java
new file mode 100644
index 00000000000..ce8ff6a1b63
--- /dev/null
+++ b/sonar-batch/src/test/java/org/sonar/batch/issue/DefaultIssueFilterChainTest.java
@@ -0,0 +1,77 @@
+package org.sonar.batch.issue;
+
+import org.fest.assertions.Fail;
+
+import org.junit.Test;
+import org.sonar.api.issue.Issue;
+import org.sonar.api.issue.batch.IssueFilter;
+import org.sonar.api.issue.batch.IssueFilterChain;
+import static org.fest.assertions.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+
+public class DefaultIssueFilterChainTest {
+
+  private final Issue issue = mock(Issue.class);
+
+  @Test
+  public void should_accept_when_no_filter() throws Exception {
+    assertThat(new DefaultIssueFilterChain().accept(issue)).isTrue();
+  }
+
+  class PassingFilter implements IssueFilter {
+    @Override
+    public boolean accept(Issue issue, IssueFilterChain chain) {
+      return chain.accept(issue);
+    }
+  }
+
+  class AcceptingFilter implements IssueFilter {
+    @Override
+    public boolean accept(Issue issue, IssueFilterChain chain) {
+      return true;
+    }
+  }
+
+  class RefusingFilter implements IssueFilter {
+    @Override
+    public boolean accept(Issue issue, IssueFilterChain chain) {
+      return false;
+    }
+  }
+
+  class FailingFilter implements IssueFilter {
+    @Override
+    public boolean accept(Issue issue, IssueFilterChain chain) {
+      Fail.fail();
+      return false;
+    }
+
+  }
+
+  @Test
+  public void should_accept_if_all_filters_pass() throws Exception {
+    assertThat(new DefaultIssueFilterChain(
+        new PassingFilter(),
+        new PassingFilter(),
+        new PassingFilter()
+      ).accept(issue)).isTrue();
+  }
+
+  @Test
+  public void should_accept_and_not_go_further_if_filter_accepts() throws Exception {
+    assertThat(new DefaultIssueFilterChain(
+        new PassingFilter(),
+        new AcceptingFilter(),
+        new FailingFilter()
+      ).accept(issue)).isTrue();
+  }
+
+  @Test
+  public void should_refuse_and_not_go_further_if_filter_refuses() throws Exception {
+    assertThat(new DefaultIssueFilterChain(
+        new PassingFilter(),
+        new RefusingFilter(),
+        new FailingFilter()
+      ).accept(issue)).isFalse();
+  }
+}
diff --git a/sonar-batch/src/test/java/org/sonar/batch/issue/IssueFiltersTest.java b/sonar-batch/src/test/java/org/sonar/batch/issue/IssueFiltersTest.java
index b9583a20285..d1e35cf5492 100644
--- a/sonar-batch/src/test/java/org/sonar/batch/issue/IssueFiltersTest.java
+++ b/sonar-batch/src/test/java/org/sonar/batch/issue/IssueFiltersTest.java
@@ -21,7 +21,7 @@ package org.sonar.batch.issue;
 
 import org.junit.Test;
 import org.sonar.api.issue.Issue;
-import org.sonar.api.issue.IssueFilter;
+import org.sonar.api.issue.batch.IssueFilter;
 import org.sonar.api.issue.internal.DefaultIssue;
 import org.sonar.api.rules.Violation;
 import org.sonar.batch.ViolationFilters;
@@ -37,21 +37,21 @@ public class IssueFiltersTest {
   ViolationFilters deprecatedFilters = mock(ViolationFilters.class);
 
   @Test
-  public void accept() throws Exception {
-    IssueFilter ok = mock(IssueFilter.class);
+  public void accept_when_filter_chain_is_empty() throws Exception {
+    org.sonar.api.issue.IssueFilter ok = mock(org.sonar.api.issue.IssueFilter.class);
     when(ok.accept(any(Issue.class))).thenReturn(true);
 
-    IssueFilter ko = mock(IssueFilter.class);
+    org.sonar.api.issue.IssueFilter ko = mock(org.sonar.api.issue.IssueFilter.class);
     when(ko.accept(any(Issue.class))).thenReturn(false);
 
     when(deprecatedFilters.isEmpty()).thenReturn(true);
-    IssueFilters filters = new IssueFilters(deprecatedFilters, deprecatedViolations, new IssueFilter[]{ok, ko});
+    IssueFilters filters = new IssueFilters(deprecatedFilters, deprecatedViolations, new org.sonar.api.issue.IssueFilter[]{ok, ko});
     assertThat(filters.accept(new DefaultIssue(), null)).isFalse();
 
-    filters = new IssueFilters(deprecatedFilters, deprecatedViolations, new IssueFilter[]{ok});
+    filters = new IssueFilters(deprecatedFilters, deprecatedViolations, new org.sonar.api.issue.IssueFilter[]{ok});
     assertThat(filters.accept(new DefaultIssue(), null)).isTrue();
 
-    filters = new IssueFilters(deprecatedFilters, deprecatedViolations, new IssueFilter[]{ko});
+    filters = new IssueFilters(deprecatedFilters, deprecatedViolations, new org.sonar.api.issue.IssueFilter[]{ko});
     assertThat(filters.accept(new DefaultIssue(), null)).isFalse();
   }
 
@@ -66,7 +66,7 @@ public class IssueFiltersTest {
   public void should_check_deprecated_violation_filters() throws Exception {
     when(deprecatedFilters.isEmpty()).thenReturn(false);
     when(deprecatedFilters.isIgnored(any(Violation.class))).thenReturn(true);
-    IssueFilters filters = new IssueFilters(deprecatedFilters, deprecatedViolations, new IssueFilter[0]);
+    IssueFilters filters = new IssueFilters(deprecatedFilters, deprecatedViolations, new org.sonar.api.issue.IssueFilter[0]);
     assertThat(filters.accept(new DefaultIssue(), null)).isFalse();
 
   }