From 69b413c1d409f5b95f21c39f9a18658dcf44000f Mon Sep 17 00:00:00 2001 From: Fabrice Bellingard Date: Tue, 20 Mar 2012 12:23:57 +0100 Subject: [PATCH] SONAR-3283 Do not allow to search reviews by author/assignee ID --- .../main/webapp/WEB-INF/app/models/review.rb | 12 +--- .../sonar/wsclient/services/ReviewQuery.java | 62 ++++++++++++++----- .../wsclient/services/ReviewQueryTest.java | 28 +++++++-- 3 files changed, 73 insertions(+), 29 deletions(-) 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 56d566ae9c1..2ca3c693b7e 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 @@ -338,11 +338,7 @@ class Review < ActiveRecord::Base authors=options['authors'].split(',') if options['authors'] if authors && authors.size>0 && !authors[0].blank? conditions << 'user_id in (:authors)' - unless Api::Utils.is_number?(authors[0]) - values[:authors]=User.logins_to_ids(authors) - else - values[:authors]=authors.map { |user_id| user_id.to_i } - end + values[:authors]=User.logins_to_ids(authors) end assignees=options['assignees'].split(',') if options['assignees'] @@ -353,11 +349,7 @@ class Review < ActiveRecord::Base else # Assigned reviews conditions << 'assignee_id in (:assignees)' - unless Api::Utils.is_number?(assignees[0]) - values[:assignees]=User.logins_to_ids(assignees) - else - values[:assignees]=assignees.map { |user_id| user_id.to_i } - end + values[:assignees]=User.logins_to_ids(assignees) end end diff --git a/sonar-ws-client/src/main/java/org/sonar/wsclient/services/ReviewQuery.java b/sonar-ws-client/src/main/java/org/sonar/wsclient/services/ReviewQuery.java index 2dfbbab4e9b..81d3cc850b0 100644 --- a/sonar-ws-client/src/main/java/org/sonar/wsclient/services/ReviewQuery.java +++ b/sonar-ws-client/src/main/java/org/sonar/wsclient/services/ReviewQuery.java @@ -40,8 +40,8 @@ public class ReviewQuery extends Query { private String[] severities; private String[] projectKeysOrIds; private String[] resourceKeysOrIds; - private String[] authorLoginsOrIds; - private String[] assigneeLoginsOrIds; + private String[] authorLogins; + private String[] assigneeLogins; private String output; private String[] resolutions; @@ -165,34 +165,68 @@ public class ReviewQuery extends Query { } /** - * @return the authorLoginsOrIds + * @deprecated since 2.15. Searching by user ID is not possible anymore. Use {@link #getAuthorLogins()} instead. */ + @Deprecated public String[] getAuthorLoginsOrIds() { - return authorLoginsOrIds; + return authorLogins; } /** - * @param authorLoginsOrIds - * the authorLoginsOrIds to set + * @deprecated since 2.15. Searching by user ID is not possible anymore. Use {@link #setAuthorLogins(String...)} instead. */ + @Deprecated public ReviewQuery setAuthorLoginsOrIds(String... authorLoginsOrIds) { - this.authorLoginsOrIds = authorLoginsOrIds; + setAuthorLogins(authorLoginsOrIds); + return this; + } + + /** + * @return the authorLogins + */ + public String[] getAuthorLogins() { + return authorLogins; + } + + /** + * @param authorLogins + * the authorLogins to set + */ + public ReviewQuery setAuthorLogins(String... authorLogins) { + this.authorLogins = authorLogins; return this; } /** - * @return the assigneeLoginsOrIds + * @deprecated since 2.15. Searching by user ID is not possible anymore. Use {@link #getAssigneeLogins()} instead. */ + @Deprecated public String[] getAssigneeLoginsOrIds() { - return assigneeLoginsOrIds; + return assigneeLogins; } /** - * @param assigneeLoginsOrIds - * the assigneeLoginsOrIds to set + * @deprecated since 2.15. Searching by user ID is not possible anymore. Use {@link #setAssigneeLogins(String...)} instead. */ + @Deprecated public ReviewQuery setAssigneeLoginsOrIds(String... assigneeLoginsOrIds) { - this.assigneeLoginsOrIds = assigneeLoginsOrIds; + setAssigneeLogins(assigneeLoginsOrIds); + return this; + } + + /** + * @return the assigneeLogins + */ + public String[] getAssigneeLogins() { + return assigneeLogins; + } + + /** + * @param assigneeLogins + * the assigneeLogins to set + */ + public ReviewQuery setAssigneeLogins(String... assigneeLogins) { + this.assigneeLogins = assigneeLogins; return this; } @@ -241,8 +275,8 @@ public class ReviewQuery extends Query { appendUrlParameter(url, "severities", severities); appendUrlParameter(url, "projects", projectKeysOrIds); appendUrlParameter(url, "resources", resourceKeysOrIds); - appendUrlParameter(url, "authors", authorLoginsOrIds); - appendUrlParameter(url, "assignees", assigneeLoginsOrIds); + appendUrlParameter(url, "authors", authorLogins); + appendUrlParameter(url, "assignees", assigneeLogins); appendUrlParameter(url, "output", output); appendUrlParameter(url, "resolutions", resolutions); if (resolutions == null && reviewType != null) { diff --git a/sonar-ws-client/src/test/java/org/sonar/wsclient/services/ReviewQueryTest.java b/sonar-ws-client/src/test/java/org/sonar/wsclient/services/ReviewQueryTest.java index da52d39baf4..5632b29c79a 100644 --- a/sonar-ws-client/src/test/java/org/sonar/wsclient/services/ReviewQueryTest.java +++ b/sonar-ws-client/src/test/java/org/sonar/wsclient/services/ReviewQueryTest.java @@ -56,22 +56,40 @@ public class ReviewQueryTest extends QueryTestCase { .setSeverities("MINOR", "INFO") .setProjectKeysOrIds("com.sonar.foo:bar") .setResourceKeysOrIds("2", "3") - .setAuthorLoginsOrIds("20") - .setAssigneeLoginsOrIds("admin") + .setAuthorLogins("foo") + .setAssigneeLogins("admin") .setOutput("html"); assertThat( query.getUrl(), - is("/api/reviews?statuses=OPEN&severities=MINOR,INFO&projects=com.sonar.foo%3Abar&resources=2,3&authors=20&assignees=admin&output=html&")); + is("/api/reviews?statuses=OPEN&severities=MINOR,INFO&projects=com.sonar.foo%3Abar&resources=2,3&authors=foo&assignees=admin&output=html&")); } @Test public void resourceTreeViolationsForSonar2_8() { ReviewQuery query = new ReviewQuery(); query.setIds(10L, 11L).setReviewType("FALSE_POSITIVE").setStatuses("OPEN").setSeverities("MINOR", "INFO") - .setProjectKeysOrIds("com.sonar.foo:bar").setResourceKeysOrIds("2", "3").setAuthorLoginsOrIds("20").setAssigneeLoginsOrIds("admin") + .setProjectKeysOrIds("com.sonar.foo:bar").setResourceKeysOrIds("2", "3").setAuthorLogins("foo").setAssigneeLogins("admin") .setOutput("html"); assertThat( query.getUrl(), - is("/api/reviews?ids=10,11&statuses=OPEN&severities=MINOR,INFO&projects=com.sonar.foo%3Abar&resources=2,3&authors=20&assignees=admin&output=html&review_type=FALSE_POSITIVE&")); + is("/api/reviews?ids=10,11&statuses=OPEN&severities=MINOR,INFO&projects=com.sonar.foo%3Abar&resources=2,3&authors=foo&assignees=admin&output=html&review_type=FALSE_POSITIVE&")); + } + + // http://jira.codehaus.org/browse/SONAR-3283 + @Test + public void testDeprecatedQueryByUserOrAssigneeId() throws Exception { + // the de deprecated setters + ReviewQuery query = new ReviewQuery() + .setAuthorLoginsOrIds("20") + .setAssigneeLoginsOrIds("40"); + assertThat(query.getAuthorLogins(), is(new String[] {"20"})); + assertThat(query.getAssigneeLogins(), is(new String[] {"40"})); + + // and test the deprecated getters + query = new ReviewQuery() + .setAuthorLogins("foo") + .setAssigneeLogins("bar"); + assertThat(query.getAuthorLoginsOrIds(), is(new String[] {"foo"})); + assertThat(query.getAssigneeLoginsOrIds(), is(new String[] {"bar"})); } } -- 2.39.5