From cbd8ee77487227cc50afa1fd12a25943aa909eff Mon Sep 17 00:00:00 2001 From: Fabrice Bellingard Date: Thu, 28 Apr 2011 16:09:04 +0200 Subject: [PATCH] SONAR-2381 The "violations" web service API must return violations decorated with review --- .../app/controllers/api/reviews_controller.rb | 66 +------------------ .../controllers/api/violations_controller.rb | 4 +- .../WEB-INF/app/helpers/reviews_helper.rb | 65 ++++++++++++++++++ .../webapp/WEB-INF/app/models/rule_failure.rb | 18 ++++- .../sonar/wsclient/services/Violation.java | 10 +-- .../wsclient/services/ViolationQuery.java | 17 +++++ .../unmarshallers/ViolationUnmarshaller.java | 7 +- .../ViolationUnmarshallerTest.java | 15 ++++- .../resources/violations/false-positive.json | 2 +- 9 files changed, 125 insertions(+), 79 deletions(-) diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/reviews_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/reviews_controller.rb index bcce88cc58c..79b00afb83f 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/reviews_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/reviews_controller.rb @@ -22,7 +22,7 @@ require "json" class Api::ReviewsController < Api::ApiController - include MarkdownHelper + include MarkdownHelper, ReviewsHelper def index convert_markdown=(params[:html]=='true') @@ -35,68 +35,4 @@ class Api::ReviewsController < Api::ApiController end end - - private - - def to_xml(reviews, convert_markdown) - xml = Builder::XmlMarkup.new(:indent => 0) - xml.instruct! - - xml.reviews do - reviews.each do |review| - xml.review do - xml.id(review.id.to_i) - xml.createdAt(format_datetime(review.created_at)) - xml.updatedAt(format_datetime(review.updated_at)) - xml.user(review.user.login) - xml.assignee(review.assignee.login) if review.assignee - xml.title(review.title) - xml.type(review.review_type) - xml.status(review.status) - xml.severity(review.severity) - xml.resource(review.resource.kee) if review.resource - xml.line(review.resource_line) if review.resource_line > 0 - xml.comments do - review.review_comments.each do |comment| - xml.comment do - xml.author(comment.user.login) - xml.updatedAt(format_datetime(comment.updated_at)) - xml.text(convert_markdown ? markdown_to_html(comment.review_text): comment.review_text) - end - end - end - end - end - end - end - - def to_json(reviews, convert_markdown=false) - JSON(reviews.collect{|review| review_to_json(review, convert_markdown)}) - end - - def review_to_json(review, html=false) - json = {} - json['id'] = review.id.to_i - json['createdAt'] = format_datetime(review.created_at) - json['updatedAt'] = format_datetime(review.updated_at) - json['author'] = review.user.login - json['assignee'] = review.assignee.login if review.assignee - json['title'] = review.title if review.title - json['type'] = review.review_type - json['status'] = review.status - json['severity'] = review.severity - json['resource'] = review.resource.kee if review.resource - json['line'] = review.resource_line if review.resource_line > 0 - comments = [] - review.review_comments.each do |comment| - comments << { - 'author' => comment.user.login, - 'updatedAt' => format_datetime(comment.updated_at), - 'text' => (html ? markdown_to_html(comment.review_text): comment.review_text) - } - end - json['comments'] = comments - json - end - end \ No newline at end of file diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/violations_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/violations_controller.rb index 679d68dc3d0..0f38dda8b02 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/violations_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/violations_controller.rb @@ -92,7 +92,7 @@ class Api::ViolationsController < Api::ResourceRestController end def rest_to_json(rule_failures) - JSON(rule_failures.collect{|rule_failure| rule_failure.to_hash_json}) + JSON(rule_failures.collect{|rule_failure| rule_failure.to_hash_json(params['include_review']=="true"?true:false)}) end def rest_to_xml(rule_failures) @@ -100,7 +100,7 @@ class Api::ViolationsController < Api::ResourceRestController xml.instruct! xml.violations do rule_failures.each do |rule_failure| - rule_failure.to_xml(xml) + rule_failure.to_xml(xml, params['include_review']=="true"?true:false) end end end diff --git a/sonar-server/src/main/webapp/WEB-INF/app/helpers/reviews_helper.rb b/sonar-server/src/main/webapp/WEB-INF/app/helpers/reviews_helper.rb index 2cf6e97b6ee..41da95a1847 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/helpers/reviews_helper.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/helpers/reviews_helper.rb @@ -23,4 +23,69 @@ module ReviewsHelper Project.find(:all, :select => 'id,name,long_name', :conditions => ['enabled=? AND scope=? AND qualifier IN (?)', true, 'PRJ', ['TRK', 'VW','SVW']], :order => 'name ASC') end + def to_xml(reviews, convert_markdown) + xml = Builder::XmlMarkup.new(:indent => 0) + xml.instruct! + + xml.reviews do + reviews.each do |review| + review_to_xml(xml, review, convert_markdown) + end + end + end + + def review_to_xml(xml, review, html=false) + xml.review do + xml.id(review.id.to_i) + xml.createdAt(format_datetime(review.created_at)) + xml.updatedAt(format_datetime(review.updated_at)) + xml.user(review.user.login) + xml.assignee(review.assignee.login) if review.assignee + xml.title(review.title) + xml.type(review.review_type) + xml.status(review.status) + xml.severity(review.severity) + xml.resource(review.resource.kee) if review.resource + xml.line(review.resource_line) if review.resource_line > 0 + xml.comments do + review.review_comments.each do |comment| + xml.comment do + xml.author(comment.user.login) + xml.updatedAt(format_datetime(comment.updated_at)) + xml.text(html ? markdown_to_html(comment.review_text): comment.review_text) + end + end + end + end + end + + def to_json(reviews, convert_markdown=false) + JSON(reviews.collect{|review| review_to_json(review, convert_markdown)}) + end + + def review_to_json(review, html=false) + json = {} + json['id'] = review.id.to_i + json['createdAt'] = format_datetime(review.created_at) + json['updatedAt'] = format_datetime(review.updated_at) + json['author'] = review.user.login + json['assignee'] = review.assignee.login if review.assignee + json['title'] = review.title if review.title + json['type'] = review.review_type + json['status'] = review.status + json['severity'] = review.severity + json['resource'] = review.resource.kee if review.resource + json['line'] = review.resource_line if review.resource_line > 0 + comments = [] + review.review_comments.each do |comment| + comments << { + 'author' => comment.user.login, + 'updatedAt' => format_datetime(comment.updated_at), + 'text' => (html ? markdown_to_html(comment.review_text): comment.review_text) + } + end + json['comments'] = comments + json + end + end 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 01c7acb2f56..f2685534f5d 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 @@ -20,6 +20,8 @@ class RuleFailure < ActiveRecord::Base + include MarkdownHelper, ReviewsHelper + belongs_to :rule belongs_to :snapshot has_one :review, :primary_key => "permanent_id", :foreign_key => "rule_failure_permanent_id", :order => "created_at" @@ -41,7 +43,12 @@ class RuleFailure < ActiveRecord::Base end end + # in case to_has_json was used somewhere else before the "include_review" param was introduced def to_hash_json + to_hash_json(false) + end + + def to_hash_json(include_review=false) json = {} json['message'] = message json['line'] = line if line && line>=1 @@ -61,11 +68,16 @@ class RuleFailure < ActiveRecord::Base :qualifier => snapshot.project.qualifier, :language => snapshot.project.language } - json['review']=review.id.to_i if review + json['review'] = review_to_json(review, true) if review && include_review json end - def to_xml(xml=Builder::XmlMarkup.new(:indent => 0)) + # in case to_xml was used somewhere else before the "include_review" param was introduced + def to_xml(xml) + to_xml(xml, false) + end + + def to_xml(xml=Builder::XmlMarkup.new(:indent => 0), include_review=false) xml.violation do xml.message(message) xml.line(line) if line && line>=1 @@ -85,7 +97,7 @@ class RuleFailure < ActiveRecord::Base xml.qualifier(snapshot.project.qualifier) xml.language(snapshot.project.language) end - xml.review(review.id.to_i) if review + review_to_xml(xml, review, true) if review && include_review end 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 1c74a823b52..704bc76efa8 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 @@ -34,7 +34,7 @@ public class Violation extends Model { private String resourceQualifier = null; private Date createdAt = null; private boolean switchedOff; - private Long reviewId = null; + private Review review = null; public String getMessage() { return message; @@ -195,15 +195,15 @@ public class Violation extends Model { /** * @since 2.8 */ - public Long getReviewId() { - return reviewId; + public Review getReview() { + return review; } /** * @since 2.8 */ - public Violation setReviewId(Long l) { - this.reviewId = l; + public Violation setReview(Review review) { + this.review = review; return this; } } 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 d75e2b64808..e3eb5dd60de 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 @@ -31,6 +31,7 @@ public class ViolationQuery extends Query { private String[] categories; private String[] severities; private Integer limit; + private Boolean includeReview; public ViolationQuery(String resourceKeyOrId) { this.resourceKeyOrId = resourceKeyOrId; @@ -128,6 +129,21 @@ public class ViolationQuery extends Query { return this; } + /** + * @since 2.8 + */ + public Boolean getIncludeReview() { + return includeReview; + } + + /** + * @since 2.8 + */ + public ViolationQuery setIncludeReview(Boolean includeReview) { + this.includeReview = includeReview; + return this; + } + @Override public String getUrl() { StringBuilder url = new StringBuilder(BASE_URL); @@ -142,6 +158,7 @@ public class ViolationQuery extends Query { appendUrlParameter(url, "rules", ruleKeys); appendUrlParameter(url, "categories", categories); appendUrlParameter(url, "priorities", severities); + appendUrlParameter(url, "include_review", includeReview); 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 a73d4b3c4fa..c8b5558eb22 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 @@ -34,7 +34,12 @@ public class ViolationUnmarshaller extends AbstractUnmarshaller { violation.setSeverity(utils.getString(json, "priority")); violation.setCreatedAt(utils.getDateTime(json, "createdAt")); violation.setSwitchedOff(utils.getBoolean(json, "switchedOff")); - violation.setReviewId(utils.getLong(json, "review")); + + Object review = utils.getField(json, "review"); + if (review != null) { + ReviewUnmarshaller reviewUnmarshaller = new ReviewUnmarshaller(); + violation.setReview(reviewUnmarshaller.parse(review)); + } 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 16e07efff38..32e00d2dbd8 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 @@ -20,6 +20,7 @@ package org.sonar.wsclient.unmarshallers; import org.junit.Test; +import org.sonar.wsclient.services.Review; import org.sonar.wsclient.services.Violation; import java.util.List; @@ -28,6 +29,7 @@ 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.assertNotNull; import static org.junit.Assert.assertThat; public class ViolationUnmarshallerTest extends UnmarshallerTestCase { @@ -54,7 +56,7 @@ public class ViolationUnmarshallerTest extends UnmarshallerTestCase { assertThat(violation.getResourceQualifier(), is("CLA")); assertThat(violation.getResourceScope(), is("FIL")); assertThat(violation.isSwitchedOff(), is(false)); - assertThat(violation.getReviewId(), nullValue()); + assertThat(violation.getReview(), nullValue()); } @Test @@ -70,7 +72,16 @@ public class ViolationUnmarshallerTest extends UnmarshallerTestCase { public void testSwitchedOff() { Violation violation = new ViolationUnmarshaller().toModel(loadFile("/violations/false-positive.json")); assertThat(violation.isSwitchedOff(), is(true)); - assertThat(violation.getReviewId(), is(123L)); + } + + @Test + public void testViolationDecoratedWithReview() { + Violation violation = new ViolationUnmarshaller().toModel(loadFile("/violations/violation-with-review.json")); + Review review = violation.getReview(); + assertNotNull(review); + assertThat(review.getId(), is(3L)); + assertThat(review.getComments().size(), is(4)); + assertThat(review.getComments().get(1).getText(), is("Bold on multiple line?")); } /** diff --git a/sonar-ws-client/src/test/resources/violations/false-positive.json b/sonar-ws-client/src/test/resources/violations/false-positive.json index 2165e9455c1..e96a394d911 100644 --- a/sonar-ws-client/src/test/resources/violations/false-positive.json +++ b/sonar-ws-client/src/test/resources/violations/false-positive.json @@ -1,3 +1,3 @@ [ - {"message":"throw java.lang.Exception","switchedOff": true, "review": 123, "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","switchedOff": true, "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"}} ] \ No newline at end of file -- 2.39.5