From e8bf534ec680e583b0e7683c92f1be31ab66a74e Mon Sep 17 00:00:00 2001 From: Fabrice Bellingard Date: Thu, 21 Apr 2011 16:56:21 +0200 Subject: [PATCH] SONAR-2380 The "violations" web service API must not return "false-positive" violation Also modified the batch side ViolationQuery class that I created so that they have the same API. --- .../org/sonar/batch/index/DefaultIndex.java | 35 +++++++++++++------ .../sonar/batch/index/DefaultIndexTest.java | 2 +- .../java/org/sonar/api/batch/SonarIndex.java | 2 +- .../sonar/api/violations/ViolationQuery.java | 14 ++++---- .../webapp/WEB-INF/app/models/rule_failure.rb | 2 ++ .../sonar/wsclient/services/Violation.java | 19 +++++++++- .../wsclient/services/ViolationQuery.java | 19 ++++++++++ .../unmarshallers/ViolationUnmarshaller.java | 1 + .../ViolationUnmarshallerTest.java | 11 +++--- .../violation-without-optional-fields.json | 2 +- .../test/resources/violations/violations.json | 4 +-- 11 files changed, 83 insertions(+), 28 deletions(-) diff --git a/sonar-batch/src/main/java/org/sonar/batch/index/DefaultIndex.java b/sonar-batch/src/main/java/org/sonar/batch/index/DefaultIndex.java index 5b4dd14c5fd..1accc7b4ea6 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/index/DefaultIndex.java +++ b/sonar-batch/src/main/java/org/sonar/batch/index/DefaultIndex.java @@ -19,9 +19,15 @@ */ package org.sonar.batch.index; -import com.google.common.collect.Lists; -import com.google.common.collect.Maps; -import com.google.common.collect.Sets; +import java.util.Collection; +import java.util.Collections; +import java.util.Date; +import java.util.HashMap; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Set; + import org.apache.commons.lang.ObjectUtils; import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; @@ -30,9 +36,17 @@ import org.sonar.api.batch.Event; import org.sonar.api.batch.SonarIndex; import org.sonar.api.database.model.ResourceModel; import org.sonar.api.design.Dependency; -import org.sonar.api.measures.*; +import org.sonar.api.measures.Measure; +import org.sonar.api.measures.MeasuresFilter; +import org.sonar.api.measures.MeasuresFilters; +import org.sonar.api.measures.Metric; +import org.sonar.api.measures.MetricFinder; import org.sonar.api.profiles.RulesProfile; -import org.sonar.api.resources.*; +import org.sonar.api.resources.Project; +import org.sonar.api.resources.ProjectLink; +import org.sonar.api.resources.Resource; +import org.sonar.api.resources.ResourceUtils; +import org.sonar.api.resources.Scopes; import org.sonar.api.rules.ActiveRule; import org.sonar.api.rules.Violation; import org.sonar.api.utils.SonarException; @@ -42,7 +56,9 @@ import org.sonar.batch.ProjectTree; import org.sonar.batch.ResourceFilters; import org.sonar.batch.ViolationFilters; -import java.util.*; +import com.google.common.collect.Lists; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; public class DefaultIndex extends SonarIndex { @@ -297,12 +313,11 @@ public class DefaultIndex extends SonarIndex { return Collections.emptyList(); } List filteredViolations = Lists.newArrayList(); - boolean ignoreSwitchedOff = violationQuery.ignoreSwitchedOff(); + boolean isSwitchedOff = violationQuery.isSwitchedOff(); for (Violation violation : bucket.getViolations()) { - if ( ignoreSwitchedOff && violation.isSwitchedOff()) { - continue; + if ( violation.isSwitchedOff() == isSwitchedOff) { + filteredViolations.add(violation); } - filteredViolations.add(violation); } return filteredViolations; } diff --git a/sonar-batch/src/test/java/org/sonar/batch/index/DefaultIndexTest.java b/sonar-batch/src/test/java/org/sonar/batch/index/DefaultIndexTest.java index 48946226df4..0a5bb5ccc7a 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/index/DefaultIndexTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/index/DefaultIndexTest.java @@ -227,7 +227,7 @@ public class DefaultIndexTest { violation3.setSwitchedOff(true); index.addViolation(violation3); - assertThat(index.getViolations(ViolationQuery.create().forResource(file).ignoreSwitchedOff(false)).size(), is(3)); + assertThat(index.getViolations(ViolationQuery.create().forResource(file).setSwitchedOff(true)).size(), is(2)); } @Test(expected = IllegalArgumentException.class) diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/SonarIndex.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/SonarIndex.java index 89133b1c6b8..4c82198ffdd 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/SonarIndex.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/SonarIndex.java @@ -138,7 +138,7 @@ public abstract class SonarIndex implements DirectedGraphAccessor getViolations(Resource resource) { - return getViolations(ViolationQuery.create().forResource(resource).ignoreSwitchedOff(true)); + return getViolations(ViolationQuery.create().forResource(resource)); } /** diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/violations/ViolationQuery.java b/sonar-plugin-api/src/main/java/org/sonar/api/violations/ViolationQuery.java index 10be756b640..28b9082693c 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/violations/ViolationQuery.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/violations/ViolationQuery.java @@ -28,7 +28,7 @@ import org.sonar.api.resources.Resource; */ public final class ViolationQuery { - private boolean ignoreSwitchedOff; + private boolean isSwitchedOff; private Resource resource; /** @@ -50,21 +50,21 @@ public final class ViolationQuery { * Specifies if the query should returned switched-off violations or not. * * @param ignore - * if true, the query will return only active violations. + * if true, the query will return only switched-off violations. if false, it will return only active violations. * @return the current violation query */ - public ViolationQuery ignoreSwitchedOff(boolean ignore) { - this.ignoreSwitchedOff = ignore; + public ViolationQuery setSwitchedOff(boolean ignore) { + this.isSwitchedOff = ignore; return this; } /** - * Tells if the query should returned switched-off violations or not. + * Tells if the query should returned switched-off violations or active violations. * * @return */ - public boolean ignoreSwitchedOff() { - return ignoreSwitchedOff; + public boolean isSwitchedOff() { + return isSwitchedOff; } /** diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/rule_failure.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/rule_failure.rb index bb62d4e80f2..cef150fef10 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/models/rule_failure.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/models/rule_failure.rb @@ -38,6 +38,7 @@ class RuleFailure < ActiveRecord::Base json['message'] = message json['line'] = line if line json['priority'] = Sonar::RulePriority.to_s(failure_level).upcase + json['switchedOff'] = switched_off ? true : false if created_at json['createdAt'] = format_datetime(created_at) end @@ -60,6 +61,7 @@ class RuleFailure < ActiveRecord::Base xml.message(message) xml.line(line) if line xml.priority(Sonar::RulePriority.to_s(failure_level)) + xml.switchedOff(switched_off ? true : false) if created_at xml.createdAt(format_datetime(created_at)) end diff --git a/sonar-ws-client/src/main/java/org/sonar/wsclient/services/Violation.java b/sonar-ws-client/src/main/java/org/sonar/wsclient/services/Violation.java index f993b14321f..e29c37396ba 100644 --- a/sonar-ws-client/src/main/java/org/sonar/wsclient/services/Violation.java +++ b/sonar-ws-client/src/main/java/org/sonar/wsclient/services/Violation.java @@ -33,6 +33,7 @@ public class Violation extends Model { private String resourceScope = null; private String resourceQualifier = null; private Date createdAt = null; + private boolean switchedOff; public String getMessage() { return message; @@ -152,6 +153,22 @@ public class Violation extends Model { * @since 2.5 */ public boolean isCreatedAfter(Date date) { - return createdAt!=null && date!=null && createdAt.after(date); + return createdAt != null && date != null && createdAt.after(date); } + + /** + * @since 2.8 + */ + public Violation setSwitchedOff(boolean switchedOff) { + this.switchedOff = switchedOff; + return this; + } + + /** + * @since 2.8 + */ + public boolean isSwitchedOff() { + return switchedOff; + } + } diff --git a/sonar-ws-client/src/main/java/org/sonar/wsclient/services/ViolationQuery.java b/sonar-ws-client/src/main/java/org/sonar/wsclient/services/ViolationQuery.java index 0fc6d244d57..06a75f94c9e 100644 --- a/sonar-ws-client/src/main/java/org/sonar/wsclient/services/ViolationQuery.java +++ b/sonar-ws-client/src/main/java/org/sonar/wsclient/services/ViolationQuery.java @@ -20,6 +20,7 @@ package org.sonar.wsclient.services; public class ViolationQuery extends Query { + public static final String BASE_URL = "/api/violations"; private String resourceKeyOrId; @@ -30,6 +31,7 @@ public class ViolationQuery extends Query { private String[] categories; private String[] severities; private Integer limit; + private boolean isSwitchedOff; public ViolationQuery(String resourceKeyOrId) { this.resourceKeyOrId = resourceKeyOrId; @@ -127,6 +129,21 @@ public class ViolationQuery extends Query { return this; } + /** + * @since 2.8 + */ + public ViolationQuery setSwitchedOff(boolean ignore) { + this.isSwitchedOff = ignore; + return this; + } + + /** + * @since 2.8 + */ + public boolean isSwitchedOff() { + return isSwitchedOff; + } + @Override public String getUrl() { StringBuilder url = new StringBuilder(BASE_URL); @@ -141,6 +158,8 @@ public class ViolationQuery extends Query { appendUrlParameter(url, "rules", ruleKeys); appendUrlParameter(url, "categories", categories); appendUrlParameter(url, "priorities", severities); + appendUrlParameter(url, "switched_off", isSwitchedOff); + return url.toString(); } diff --git a/sonar-ws-client/src/main/java/org/sonar/wsclient/unmarshallers/ViolationUnmarshaller.java b/sonar-ws-client/src/main/java/org/sonar/wsclient/unmarshallers/ViolationUnmarshaller.java index 0df77ae74fc..df62e5c0e16 100644 --- a/sonar-ws-client/src/main/java/org/sonar/wsclient/unmarshallers/ViolationUnmarshaller.java +++ b/sonar-ws-client/src/main/java/org/sonar/wsclient/unmarshallers/ViolationUnmarshaller.java @@ -33,6 +33,7 @@ public class ViolationUnmarshaller extends AbstractUnmarshaller { violation.setLine(utils.getInteger(json, "line")); violation.setSeverity(utils.getString(json, "priority")); violation.setCreatedAt(utils.getDateTime(json, "createdAt")); + violation.setSwitchedOff(utils.getBoolean(json, "switchedOff")); Object rule = utils.getField(json, "rule"); if (rule != null) { diff --git a/sonar-ws-client/src/test/java/org/sonar/wsclient/unmarshallers/ViolationUnmarshallerTest.java b/sonar-ws-client/src/test/java/org/sonar/wsclient/unmarshallers/ViolationUnmarshallerTest.java index 614ed46b79f..3f3da0e81fc 100644 --- a/sonar-ws-client/src/test/java/org/sonar/wsclient/unmarshallers/ViolationUnmarshallerTest.java +++ b/sonar-ws-client/src/test/java/org/sonar/wsclient/unmarshallers/ViolationUnmarshallerTest.java @@ -19,17 +19,17 @@ */ package org.sonar.wsclient.unmarshallers; -import org.junit.Test; -import org.sonar.wsclient.services.Violation; - -import java.util.List; - import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.core.IsNot.not; import static org.junit.Assert.assertThat; +import java.util.List; + +import org.junit.Test; +import org.sonar.wsclient.services.Violation; + public class ViolationUnmarshallerTest extends UnmarshallerTestCase { @Test @@ -52,6 +52,7 @@ public class ViolationUnmarshallerTest extends UnmarshallerTestCase { assertThat(violation.getResourceName(), is("TraceableResourceLimitingPool")); assertThat(violation.getResourceQualifier(), is("CLA")); assertThat(violation.getResourceScope(), is("FIL")); + assertThat(violation.isSwitchedOff(), is(true)); } @Test diff --git a/sonar-ws-client/src/test/resources/violations/violation-without-optional-fields.json b/sonar-ws-client/src/test/resources/violations/violation-without-optional-fields.json index acfffe3e152..7c8aa24b70a 100644 --- a/sonar-ws-client/src/test/resources/violations/violation-without-optional-fields.json +++ b/sonar-ws-client/src/test/resources/violations/violation-without-optional-fields.json @@ -1,3 +1,3 @@ [ - {"message":"throw java.lang.Exception","priority":"MAJOR","rule":{"key":"pmd:SignatureDeclareThrowsException","name":"Signature Declare Throws Exception","category":"Maintainability"},"resource":{"key":"org.apache.excalibur.components:excalibur-pool-instrumented:org.apache.avalon.excalibur.pool.TraceableResourceLimitingPool","name":"TraceableResourceLimitingPool","scope":"FIL","qualifier":"CLA","language":"java"}}, + {"message":"throw java.lang.Exception","priority":"MAJOR","switchedOff":true,"rule":{"key":"pmd:SignatureDeclareThrowsException","name":"Signature Declare Throws Exception","category":"Maintainability"},"resource":{"key":"org.apache.excalibur.components:excalibur-pool-instrumented:org.apache.avalon.excalibur.pool.TraceableResourceLimitingPool","name":"TraceableResourceLimitingPool","scope":"FIL","qualifier":"CLA","language":"java"}}, ] \ No newline at end of file diff --git a/sonar-ws-client/src/test/resources/violations/violations.json b/sonar-ws-client/src/test/resources/violations/violations.json index 92c98b332f0..1dbad54bb6e 100644 --- a/sonar-ws-client/src/test/resources/violations/violations.json +++ b/sonar-ws-client/src/test/resources/violations/violations.json @@ -1,4 +1,4 @@ [ - {"message":"throw java.lang.Exception","line":97,"createdAt":"2010-12-01T13:55:22+0300","priority":"MAJOR","rule":{"key":"pmd:SignatureDeclareThrowsException","name":"Signature Declare Throws Exception","category":"Maintainability"},"resource":{"key":"org.apache.excalibur.components:excalibur-pool-instrumented:org.apache.avalon.excalibur.pool.TraceableResourceLimitingPool","name":"TraceableResourceLimitingPool","scope":"FIL","qualifier":"CLA","language":"java"}}, - {"message":"The user-supplied array 'threads' is stored directly.","line":242,"createdAt":"2010-12-01T13:55:22+0300","priority":"MAJOR","rule":{"key":"pmd:ArrayIsStoredDirectly","name":"Security - Array is stored directly","category":"Reliability"},"resource":{"key":"org.apache.excalibur.components:excalibur-pool-instrumented:org.apache.avalon.excalibur.pool.TraceableResourceLimitingPool","name":"TraceableResourceLimitingPool","scope":"FIL","qualifier":"CLA","language":"java"}} + {"message":"throw java.lang.Exception","line":97,"createdAt":"2010-12-01T13:55:22+0300","priority":"MAJOR","switchedOff":true,"rule":{"key":"pmd:SignatureDeclareThrowsException","name":"Signature Declare Throws Exception","category":"Maintainability"},"resource":{"key":"org.apache.excalibur.components:excalibur-pool-instrumented:org.apache.avalon.excalibur.pool.TraceableResourceLimitingPool","name":"TraceableResourceLimitingPool","scope":"FIL","qualifier":"CLA","language":"java"}}, + {"message":"The user-supplied array 'threads' is stored directly.","line":242,"createdAt":"2010-12-01T13:55:22+0300","priority":"MAJOR","switchedOff":false,"rule":{"key":"pmd:ArrayIsStoredDirectly","name":"Security - Array is stored directly","category":"Reliability"},"resource":{"key":"org.apache.excalibur.components:excalibur-pool-instrumented:org.apache.avalon.excalibur.pool.TraceableResourceLimitingPool","name":"TraceableResourceLimitingPool","scope":"FIL","qualifier":"CLA","language":"java"}} ] \ No newline at end of file -- 2.39.5