From 43888ce6c210e5502bf8bb52e5762906aa50c70c Mon Sep 17 00:00:00 2001 From: simonbrandhof Date: Tue, 26 Apr 2011 15:24:57 +0200 Subject: [PATCH] SONAR-2382 Complete review web service --- .../app/controllers/api/reviews_controller.rb | 58 ++++++++++++---- .../controllers/api/violations_controller.rb | 4 +- .../WEB-INF/app/helpers/markdown_helper.rb | 2 +- .../main/webapp/WEB-INF/app/models/review.rb | 66 +++++++++++-------- .../webapp/WEB-INF/app/models/rule_failure.rb | 7 +- .../main/webapp/WEB-INF/app/models/user.rb | 8 +++ .../sonar/wsclient/services/Violation.java | 19 ++++-- .../unmarshallers/ViolationUnmarshaller.java | 3 +- .../ViolationUnmarshallerTest.java | 23 ++++--- .../resources/violations/false-positive.json | 3 + .../violation-without-optional-fields.json | 2 +- .../test/resources/violations/violations.json | 4 +- 12 files changed, 136 insertions(+), 63 deletions(-) create mode 100644 sonar-ws-client/src/test/resources/violations/false-positive.json 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 d3d329a8afb..a8f050e6dfa 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,42 +22,72 @@ require "json" class Api::ReviewsController < Api::ApiController + include MarkdownHelper + def index - @reviews = [] - @reviews << Review.find ( params[:id] ) + convert_markdown=(params[:html]=='true') + reviews=Review.search(params) respond_to do |format| - format.json { render :json => jsonp(to_json) } - format.xml {render :xml => to_xml} + format.json { render :json => jsonp(to_json(reviews, convert_markdown)) } + format.xml {render :xml => to_xml(reviews, convert_markdown)} format.text { render :text => text_not_supported } end end - - def to_xml + + + private + + def to_xml(reviews, convert_markdown) xml = Builder::XmlMarkup.new(:indent => 0) xml.instruct! xml.reviews do - @reviews.each do |review| + reviews.each do |review| xml.review do - xml.id(review.id) - xml.updatedAt(review.updated_at) + xml.id(review.id.to_i) + xml.updatedAt(format_datetime(review.updated_at)) xml.user(review.user.login) xml.assignee(review.assignee.login) xml.title(review.title) xml.type(review.review_type) xml.status(review.status) xml.severity(review.severity) - xml.resourceLine(review.resource_line) + xml.resource(review.resource.kee) if review.resource + xml.line(review.resource_line) if review.resource_line - # Continue here with resource + violation + comments + # Continue here with resource + comments end end end end - def to_json - JSON(@reviews.collect{|review| review.to_hash_json(true)}) + 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['updatedAt'] = 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 + comments = [] + review.review_comments.each do |comment| + comments << { + 'author' => comment.user.login, + 'updatedAt' => format_datetime(comment.updated_at), + 'comment' => (html ? markdown_to_html(comment.review_text): comment.review_text) + } + end + json['comments'] = comments + json['line'] = review.resource_line if review.resource_line + json['resource'] = review.resource.kee if review.resource + 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 01d521b251c..679d68dc3d0 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 @@ -74,7 +74,7 @@ class Api::ViolationsController < Api::ResourceRestController end.compact end - if params[:switched_off] && params[:switched_off] == "true" + if params[:switched_off] == "true" conditions << 'switched_off=:switched_off' values[:switched_off] = true else @@ -85,7 +85,7 @@ class Api::ViolationsController < Api::ResourceRestController limit = (params[:limit] ? [params[:limit].to_i,5000].min : 5000) violations = RuleFailure.find(:all, :conditions => [ conditions.join(' AND '), values], - :include => [:snapshot, {:snapshot => :project}, :rule], + :include => [:snapshot, {:snapshot => :project}, :rule, :review], :order => 'rule_failures.failure_level DESC', :limit => limit) rest_render(violations) diff --git a/sonar-server/src/main/webapp/WEB-INF/app/helpers/markdown_helper.rb b/sonar-server/src/main/webapp/WEB-INF/app/helpers/markdown_helper.rb index bd4f6481885..0c6aedf79bb 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/helpers/markdown_helper.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/helpers/markdown_helper.rb @@ -20,7 +20,7 @@ module MarkdownHelper def markdown_to_html(input) - input ? Java::OrgSonarServerUi::JRubyFacade.markdownToHtml(h(input)) : '' + input ? Java::OrgSonarServerUi::JRubyFacade.markdownToHtml(ERB::Util.html_escape(input)) : '' end end \ No newline at end of file diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/review.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/review.rb index b05498c097f..f6d7bdc0939 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/models/review.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/models/review.rb @@ -48,6 +48,46 @@ class Review < ActiveRecord::Base rule_failure ? rule_failure.rule : nil end end + + def self.search(options={}) + conditions=['review_type<>:not_type'] + values={:not_type => Review::TYPE_FALSE_POSITIVE} + + ids=options['ids'] + if options[:id] + conditions << "id=:id" + values[:id]=options[:id].to_i + elsif ids && ids.size>0 && !ids[0].blank? + conditions << "id in (:ids)" + values[:ids]=ids.map{|id| id.to_i} + end + + statuses=options['statuses'] + if statuses && statuses.size>0 && !statuses[0].blank? + conditions << "status in (:statuses)" + values[:statuses]=statuses + end + + severities=options['severities'] + if severities && severities.size>0 && !severities[0].blank? + conditions << "severity in (:severities)" + values[:severities]=severities + end + + authors=options['authors'] + if authors && authors.size>0 && !authors[0].blank? + conditions << "user_id in (:authors)" + values[:authors]=User.logins_to_ids(authors) + end + + assignees=options['assignees'] + if assignees && assignees.size>0 && !assignees[0].blank? + conditions << "assignee_id in (:assignees)" + values[:assignees]=User.logins_to_ids(assignees) + end + + Review.find(:all, :order => "created_at DESC", :conditions => [conditions.join(" AND "), values], :limit => 200) + end private @@ -56,31 +96,5 @@ class Review < ActiveRecord::Base self.project=self.resource.project end end - - def to_hash_json ( extended ) - json = {} - json['id'] = id - json['updatedAt'] = updated_at - json['author'] = user.login - json['assignee'] = assignee.login if assignee - json['title'] = title - json['type'] = review_type - json['status'] = status - json['severity'] = severity - comments = [] - review_comments.each do |comment| - comments << { - 'author' => comment.user.login, - 'updatedAt' => comment.updated_at, - 'comment' => comment.review_text - } - end - json['comments'] = comments - if ( extended ) - json['line'] = resource_line if resource_line - json['resource'] = resource.kee if resource - end - 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 890c4e7dd90..0d66db2da28 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 @@ -46,7 +46,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 + json['falsePositive']=true if false_positive? if created_at json['createdAt'] = format_datetime(created_at) end @@ -61,7 +61,7 @@ class RuleFailure < ActiveRecord::Base :qualifier => snapshot.project.qualifier, :language => snapshot.project.language } - json['review'] = review.to_hash_json(false) if review + json['review']=review.id if review json end @@ -70,7 +70,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) + xml.falsePositive(true) if false_positive? if created_at xml.createdAt(format_datetime(created_at)) end @@ -85,6 +85,7 @@ class RuleFailure < ActiveRecord::Base xml.qualifier(snapshot.project.qualifier) xml.language(snapshot.project.language) end + xml.review(review.id) if review end end diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/user.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/user.rb index abf3a00c846..9176f859763 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/models/user.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/models/user.rb @@ -117,6 +117,14 @@ class User < ActiveRecord::Base properties.delete(prop) end end + + def self.logins_to_ids(logins=[]) + if logins.size>0 + User.find(:all, :select => 'id', :conditions => ['login in (?)', logins]).map{|user| user.id} + else + [] + end + end #--------------------------------------------------------------------- # FAVOURITES 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 e29c37396ba..0d5b816425f 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,7 +33,8 @@ public class Violation extends Model { private String resourceScope = null; private String resourceQualifier = null; private Date createdAt = null; - private boolean switchedOff; + private boolean falsePositive; + private Long reviewId = null; public String getMessage() { return message; @@ -159,16 +160,24 @@ public class Violation extends Model { /** * @since 2.8 */ - public Violation setSwitchedOff(boolean switchedOff) { - this.switchedOff = switchedOff; + public Violation setFalsePositive(Boolean b) { + this.falsePositive = (b != null && b); return this; } /** * @since 2.8 */ - public boolean isSwitchedOff() { - return switchedOff; + public boolean isFalsePositive() { + return falsePositive; } + public Long getReviewId() { + return reviewId; + } + + public Violation setReviewId(Long l) { + this.reviewId = l; + return this; + } } 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 df62e5c0e16..45f692b582c 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,7 +33,8 @@ 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")); + violation.setFalsePositive(utils.getBoolean(json, "falsePositive")); + violation.setReviewId(utils.getLong(json, "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 3f3da0e81fc..dc9a8c4cc2d 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,21 +19,21 @@ */ 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 - public void toModels() { + public void testToModels() { Violation violation = new ViolationUnmarshaller().toModel("[]"); assertThat(violation, nullValue()); @@ -52,15 +52,22 @@ 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)); + assertThat(violation.isFalsePositive(), is(false)); + assertThat(violation.getReviewId(), nullValue()); } @Test - public void violationWithoutLineNumber() { + public void testViolationWithoutLineNumber() { Violation violation = new ViolationUnmarshaller().toModel(loadFile("/violations/violation-without-optional-fields.json")); assertThat(violation.getMessage(), not(nullValue())); assertThat(violation.getLine(), nullValue()); assertThat(violation.getCreatedAt(), nullValue()); } + @Test + public void testFalsePositive() { + Violation violation = new ViolationUnmarshaller().toModel(loadFile("/violations/false-positive.json")); + assertThat(violation.isFalsePositive(), is(true)); + assertThat(violation.getReviewId(), is(123L)); + } } diff --git a/sonar-ws-client/src/test/resources/violations/false-positive.json b/sonar-ws-client/src/test/resources/violations/false-positive.json new file mode 100644 index 00000000000..d84cb1528d5 --- /dev/null +++ b/sonar-ws-client/src/test/resources/violations/false-positive.json @@ -0,0 +1,3 @@ +[ + {"message":"throw java.lang.Exception","falsePositive": 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"}} +] \ No newline at end of file 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 7c8aa24b70a..7923febf306 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","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":"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"}} ] \ 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 1dbad54bb6e..92c98b332f0 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","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"}} + {"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"}} ] \ No newline at end of file -- 2.39.5