From ffe4654f453be47c264e942f29433083e290e345 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Wed, 22 Jan 2014 07:48:36 +0100 Subject: [PATCH] Refactor WS api --- .../java/org/sonar/api/server/ws/Request.java | 28 ++++- .../sonar/api/server/ws/SimpleRequest.java | 32 ++---- .../org/sonar/api/server/ws/WebService.java | 79 +++++++++++++- .../sonar/api/server/ws/WebServiceTest.java | 62 +++++++++-- .../issue/InternalRubyIssueService.java | 4 - .../org/sonar/server/issue/IssueService.java | 1 + .../server/issue/ws/IssueShowWsHandler.java | 103 ++++++++++++++++++ .../org/sonar/server/issue/ws/IssuesWs.java | 46 ++++++++ .../sonar/server/issue/ws/package-info.java | 23 ++++ .../org/sonar/server/platform/Platform.java | 7 +- .../server/rule/{RuleWs.java => RulesWs.java} | 5 +- .../java/org/sonar/server/ws/ListingWs.java | 51 +++++++-- .../org/sonar/server/ws/ServletRequest.java | 18 +-- .../org/sonar/server/ws/WebServiceEngine.java | 15 ++- .../app/controllers/api/java_ws_controller.rb | 1 - .../issue/ws/IssueShowWsHandlerTest.java | 29 +++++ .../sonar/server/issue/ws/IssuesWsTest.java | 49 +++++++++ .../{RuleWsTest.java => RulesWsTest.java} | 5 +- .../org/sonar/server/ws/ListingWsTest.java | 10 +- .../sonar/server/ws/WebServiceEngineTest.java | 47 +++++++- .../{RuleWsTest => RulesWsTest}/search.json | 0 .../ws/ListingWebServiceTest/index.json | 66 ++++++----- 22 files changed, 574 insertions(+), 107 deletions(-) create mode 100644 sonar-server/src/main/java/org/sonar/server/issue/ws/IssueShowWsHandler.java create mode 100644 sonar-server/src/main/java/org/sonar/server/issue/ws/IssuesWs.java create mode 100644 sonar-server/src/main/java/org/sonar/server/issue/ws/package-info.java rename sonar-server/src/main/java/org/sonar/server/rule/{RuleWs.java => RulesWs.java} (93%) create mode 100644 sonar-server/src/test/java/org/sonar/server/issue/ws/IssueShowWsHandlerTest.java create mode 100644 sonar-server/src/test/java/org/sonar/server/issue/ws/IssuesWsTest.java rename sonar-server/src/test/java/org/sonar/server/rule/{RuleWsTest.java => RulesWsTest.java} (95%) rename sonar-server/src/test/resources/org/sonar/server/rule/{RuleWsTest => RulesWsTest}/search.json (100%) diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java index 327c0557ba0..a3e00fdc02a 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java @@ -19,6 +19,8 @@ */ package org.sonar.api.server.ws; +import org.apache.commons.lang.StringUtils; + import javax.annotation.CheckForNull; /** @@ -26,12 +28,32 @@ import javax.annotation.CheckForNull; */ public abstract class Request { + /** + * Returns the name of the HTTP method with which this request was made. Possible + * values are GET and POST. Others are not supported. + */ + public abstract String method(); + @CheckForNull public abstract String param(String key); - public abstract String mediaType(); + /** + * Returns value of a required parameter + * + * @throws java.lang.IllegalArgumentException is value is null or blank + */ + public String requiredParam(String key) { + String value = param(key); + if (StringUtils.isBlank(value)) { + throw new IllegalArgumentException(String.format("Parameter '%s' is missing", key)); + } + return value; + } - public abstract boolean isPost(); + @CheckForNull + public String param(String key, @CheckForNull String defaultValue) { + return StringUtils.defaultString(param(key), defaultValue); + } @CheckForNull public Integer intParam(String key) { @@ -43,6 +65,4 @@ public abstract class Request { String s = param(key); return s == null ? defaultValue : Integer.parseInt(s); } - - } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/SimpleRequest.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/SimpleRequest.java index e24def28ec3..9689d060db6 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/SimpleRequest.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/SimpleRequest.java @@ -25,12 +25,18 @@ import javax.annotation.CheckForNull; import java.util.Map; public class SimpleRequest extends Request { + + private String method = "GET"; private Map params = Maps.newHashMap(); - private String mediaType = "application/json"; - private boolean post = false; + @Override + public String method() { + return method; + } - public SimpleRequest() { + public SimpleRequest setMethod(String s) { + this.method = s; + return this; } public SimpleRequest setParams(Map m) { @@ -50,24 +56,4 @@ public class SimpleRequest extends Request { public String param(String key) { return params.get(key); } - - @Override - public String mediaType() { - return mediaType; - } - - public SimpleRequest setMediaType(String s) { - this.mediaType = s; - return this; - } - - @Override - public boolean isPost() { - return post; - } - - public SimpleRequest setPost(boolean b) { - this.post = b; - return this; - } } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java index 518eae3bf6c..218b5c0c3a1 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java @@ -146,13 +146,12 @@ public interface WebService extends ServerExtension { } } - // TODO define supported parameters class NewAction { private final String key; private String description, since; - private boolean post = false; - private boolean isPrivate = false; + private boolean post = false, isPrivate = false; private RequestHandler handler; + private Map newParams = Maps.newHashMap(); private NewAction(String key) { this.key = key; @@ -182,12 +181,29 @@ public interface WebService extends ServerExtension { this.handler = h; return this; } + + public NewParam newParam(String paramKey) { + if (newParams.containsKey(paramKey)) { + throw new IllegalStateException( + String.format("The parameter '%s' is defined multiple times in the action '%s'", paramKey, key) + ); + } + NewParam newParam = new NewParam(paramKey); + newParams.put(paramKey, newParam); + return newParam; + } + + public NewAction newParam(String paramKey, @Nullable String description) { + newParam(paramKey).setDescription(description); + return this; + } } class Action { private final String key, path, description, since; private final boolean post, isPrivate; private final RequestHandler handler; + private final Map params; private Action(Controller controller, NewAction newAction) { this.key = newAction.key; @@ -196,10 +212,17 @@ public interface WebService extends ServerExtension { this.since = StringUtils.defaultIfBlank(newAction.since, controller.since); this.post = newAction.post; this.isPrivate = newAction.isPrivate; + if (newAction.handler == null) { throw new IllegalStateException("RequestHandler is not set on action " + path); } this.handler = newAction.handler; + + ImmutableMap.Builder mapBuilder = ImmutableMap.builder(); + for (NewParam newParam : newAction.newParams.values()) { + mapBuilder.put(newParam.key, new Param(newParam)); + } + this.params = mapBuilder.build(); } public String key() { @@ -235,12 +258,62 @@ public interface WebService extends ServerExtension { return handler; } + @CheckForNull + public Param param(String key) { + return params.get(key); + } + + public Collection params() { + return params.values(); + } + @Override public String toString() { return path; } } + class NewParam { + private String key, description; + + private NewParam(String key) { + this.key = key; + } + + public NewParam setDescription(@Nullable String s) { + this.description = s; + return this; + } + + @Override + public String toString() { + return key; + } + } + + class Param { + private final String key, description; + + public Param(NewParam newParam) { + this.key = newParam.key; + this.description = newParam.description; + } + + public String key() { + return key; + } + + @CheckForNull + public String description() { + return description; + } + + @Override + public String toString() { + return key; + } + } + /** * Executed at server startup. */ diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/WebServiceTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/WebServiceTest.java index fad7b75a105..867af612708 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/WebServiceTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/WebServiceTest.java @@ -29,19 +29,22 @@ public class WebServiceTest { static class MetricWebService implements WebService { boolean showCalled = false, createCalled = false; + @Override public void define(Context context) { NewController newController = context.newController("api/metric") .setDescription("Metrics") .setSince("3.2"); + newController.newAction("show") - .setDescription("Show metric") - .setHandler(new RequestHandler() { - @Override - public void handle(Request request, Response response) { - show(request, response); - } - }); + .setDescription("Show metric") + .setHandler(new RequestHandler() { + @Override + public void handle(Request request, Response response) { + show(request, response); + } + }); + newController.newAction("create") .setDescription("Create metric") .setSince("4.1") @@ -53,6 +56,7 @@ public class WebServiceTest { create(request, response); } }); + newController.done(); } @@ -204,4 +208,48 @@ public class WebServiceTest { context.controller("api/metric").action("create").handler().handle(mock(Request.class), mock(Response.class)); assertThat(metricWs.createCalled).isTrue(); } + + @Test + public void action_parameters() { + new WebService() { + @Override + public void define(Context context) { + NewController newController = context.newController("api/rule"); + NewAction create = newController.newAction("create").setHandler(mock(RequestHandler.class)); + create.newParam("key").setDescription("Key of the new rule"); + create.newParam("severity"); + newController.done(); + } + }.define(context); + + WebService.Action action = context.controller("api/rule").action("create"); + assertThat(action.params()).hasSize(2); + + assertThat(action.param("key").key()).isEqualTo("key"); + assertThat(action.param("key").description()).isEqualTo("Key of the new rule"); + assertThat(action.param("key").toString()).isEqualTo("key"); + + assertThat(action.param("severity").key()).isEqualTo("severity"); + assertThat(action.param("severity").description()).isNull(); + } + + @Test + public void fail_if_duplicated_action_parameters() { + try { + new WebService() { + @Override + public void define(Context context) { + NewController controller = context.newController("api/rule"); + NewAction action = controller.newAction("create").setHandler(mock(RequestHandler.class)); + action.newParam("key"); + action.newParam("key"); + controller.done(); + } + }.define(context); + fail(); + } catch (IllegalStateException e) { + assertThat(e).hasMessage("The parameter 'key' is defined multiple times in the action 'create'"); + } + } + } diff --git a/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java b/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java index 3228d3c0475..8f991f7f24a 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java @@ -423,10 +423,6 @@ public class InternalRubyIssueService implements ServerComponent { return issueFilterService.find(id, UserSession.get()); } - public List findUserIssueFilters() { - return issueFilterService.findByUser(UserSession.get()); - } - public boolean isUserAuthorized(DefaultIssueFilter issueFilter) { try { UserSession userSession = UserSession.get(); diff --git a/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java b/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java index 6b53b89f4a3..fdf674289f3 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java @@ -108,6 +108,7 @@ public class IssueService implements ServerComponent { * Never return null, but an empty list if the issue does not exist. * No security check is done since it should already have been done to get the issue */ + // TODO remove userSession parameter ? public List listTransitions(@Nullable Issue issue, UserSession userSession) { if (issue == null) { return Collections.emptyList(); diff --git a/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueShowWsHandler.java b/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueShowWsHandler.java new file mode 100644 index 00000000000..adcd334125c --- /dev/null +++ b/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueShowWsHandler.java @@ -0,0 +1,103 @@ +/* + * 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.server.issue.ws; + +import org.sonar.api.issue.*; +import org.sonar.api.server.ws.Request; +import org.sonar.api.server.ws.RequestHandler; +import org.sonar.api.server.ws.Response; +import org.sonar.api.utils.text.JsonWriter; +import org.sonar.core.issue.workflow.Transition; +import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.issue.IssueService; +import org.sonar.server.user.UserSession; + +import java.util.Arrays; +import java.util.List; + +public class IssueShowWsHandler implements RequestHandler { + + private final IssueFinder issueFinder; + private final IssueService issueService; + + public IssueShowWsHandler(IssueFinder issueFinder, IssueService issueService) { + this.issueFinder = issueFinder; + this.issueService = issueService; + } + + @Override + public void handle(Request request, Response response) { + String issueKey = request.requiredParam("key"); + IssueQueryResult queryResult = issueFinder.find(IssueQuery.builder().issueKeys(Arrays.asList(issueKey)).build()); + if (queryResult.issues().size() != 1) { + throw new NotFoundException("Issue not found: " + issueKey); + } + Issue issue = queryResult.first(); + + JsonWriter json = response.newJsonWriter(); + json.beginObject().name("issue").beginObject(); + + writeIssue(queryResult, issue, json); + writeTransitions(issue, json); + writeComments(queryResult, issue, json); + //TODO write rule, component, changelog and available commands + + json.endObject().endObject().close(); + } + + private void writeComments(IssueQueryResult queryResult, Issue issue, JsonWriter json) { + json.name("comments").beginArray(); + for (IssueComment comment : issue.comments()) { + json + .beginObject() + .prop("key", comment.key()) + .prop("userLogin", comment.userLogin()) + .prop("userName", queryResult.user(comment.userLogin()).name()) + // TODO HTML content + .endObject(); + } + json.endArray(); + } + + private void writeIssue(IssueQueryResult result, Issue issue, JsonWriter json) { + json + .prop("key", issue.key()) + .prop("line", issue.line()) + .prop("message", issue.message()) + .prop("resolution", issue.resolution()) + .prop("status", issue.status()) + .prop("status", issue.status()) + .prop("severity", issue.severity()); + if (issue.assignee() != null) { + json + .prop("assignee", issue.assignee()) + .prop("assigneeName", result.user(issue.assignee()).name()); + } + } + + private void writeTransitions(Issue issue, JsonWriter json) { + List transitions = issueService.listTransitions(issue, UserSession.get()); + json.name("transitions").beginArray(); + for (Transition transition : transitions) { + json.value(transition.key()); + } + json.endArray(); + } +} diff --git a/sonar-server/src/main/java/org/sonar/server/issue/ws/IssuesWs.java b/sonar-server/src/main/java/org/sonar/server/issue/ws/IssuesWs.java new file mode 100644 index 00000000000..488b7b78353 --- /dev/null +++ b/sonar-server/src/main/java/org/sonar/server/issue/ws/IssuesWs.java @@ -0,0 +1,46 @@ +/* + * 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.server.issue.ws; + +import org.sonar.api.server.ws.WebService; + +public class IssuesWs implements WebService { + + private final IssueShowWsHandler detailHandler; + + public IssuesWs(IssueShowWsHandler detailHandler) { + this.detailHandler = detailHandler; + } + + @Override + public void define(Context context) { + NewController controller = context.newController("api/issues"); + controller.setDescription("Coding rule issues"); + + controller.newAction("show") + .setDescription("Detail of issue") + .setSince("4.2") + .setPrivate(true) + .setHandler(detailHandler) + .newParam("key", "Issue key"); + + controller.done(); + } +} diff --git a/sonar-server/src/main/java/org/sonar/server/issue/ws/package-info.java b/sonar-server/src/main/java/org/sonar/server/issue/ws/package-info.java new file mode 100644 index 00000000000..1c107b6d298 --- /dev/null +++ b/sonar-server/src/main/java/org/sonar/server/issue/ws/package-info.java @@ -0,0 +1,23 @@ +/* + * 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. + */ +@ParametersAreNonnullByDefault +package org.sonar.server.issue.ws; + +import javax.annotation.ParametersAreNonnullByDefault; diff --git a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java index bb1ed4c77e9..49be4c17c1a 100644 --- a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java +++ b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java @@ -87,6 +87,8 @@ import org.sonar.server.group.InternalGroupMembershipService; import org.sonar.server.issue.*; import org.sonar.server.issue.filter.IssueFilterService; import org.sonar.server.issue.filter.IssueFilterWs; +import org.sonar.server.issue.ws.IssueShowWsHandler; +import org.sonar.server.issue.ws.IssuesWs; import org.sonar.server.notifications.NotificationCenter; import org.sonar.server.notifications.NotificationService; import org.sonar.server.permission.InternalPermissionService; @@ -325,6 +327,9 @@ public final class Platform { servicesContainer.addSingleton(IssueBulkChangeService.class); servicesContainer.addSingleton(IssueChangelogFormatter.class); servicesContainer.addSingleton(IssueFilterWs.class); + servicesContainer.addSingleton(IssueShowWsHandler.class); + servicesContainer.addSingleton(IssuesWs.class); + // issues actions servicesContainer.addSingleton(AssignAction.class); servicesContainer.addSingleton(PlanAction.class); @@ -338,7 +343,7 @@ public final class Platform { servicesContainer.addSingleton(RuleTagLookup.class); servicesContainer.addSingleton(RubyRuleService.class); servicesContainer.addSingleton(RuleRepositories.class); - servicesContainer.addSingleton(RuleWs.class); + servicesContainer.addSingleton(RulesWs.class); // technical debt servicesContainer.addSingleton(InternalRubyTechnicalDebtService.class); diff --git a/sonar-server/src/main/java/org/sonar/server/rule/RuleWs.java b/sonar-server/src/main/java/org/sonar/server/rule/RulesWs.java similarity index 93% rename from sonar-server/src/main/java/org/sonar/server/rule/RuleWs.java rename to sonar-server/src/main/java/org/sonar/server/rule/RulesWs.java index 898cb4807a2..24146226a25 100644 --- a/sonar-server/src/main/java/org/sonar/server/rule/RuleWs.java +++ b/sonar-server/src/main/java/org/sonar/server/rule/RulesWs.java @@ -24,15 +24,14 @@ import org.sonar.api.server.ws.RequestHandler; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; -public class RuleWs implements WebService { +public class RulesWs implements WebService { @Override public void define(Context context) { NewController controller = context.newController("api/rules") .setDescription("Coding rules"); - NewAction search = controller.newAction("search"); - search + controller.newAction("search") .setDescription("Search for coding rules") .setSince("4.2") .setHandler(new RequestHandler() { diff --git a/sonar-server/src/main/java/org/sonar/server/ws/ListingWs.java b/sonar-server/src/main/java/org/sonar/server/ws/ListingWs.java index 54c55f874f1..3176315cd66 100644 --- a/sonar-server/src/main/java/org/sonar/server/ws/ListingWs.java +++ b/sonar-server/src/main/java/org/sonar/server/ws/ListingWs.java @@ -19,11 +19,13 @@ */ package org.sonar.server.ws; -import org.sonar.api.utils.text.JsonWriter; +import com.google.common.base.Function; +import com.google.common.collect.Ordering; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.RequestHandler; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; +import org.sonar.api.utils.text.JsonWriter; import java.util.List; @@ -42,11 +44,11 @@ public class ListingWs implements WebService { .setSince("4.2"); controller.newAction("index") .setHandler(new RequestHandler() { - @Override - public void handle(Request request, Response response) { - list(context.controllers(), response); - } - }); + @Override + public void handle(Request request, Response response) { + list(context.controllers(), response); + } + }); controller.done(); } @@ -54,7 +56,14 @@ public class ListingWs implements WebService { JsonWriter writer = response.newJsonWriter(); writer.beginObject(); writer.name("webServices").beginArray(); - for (Controller controller : controllers) { + + // sort controllers by path + Ordering ordering = Ordering.natural().onResultOf(new Function() { + public String apply(Controller controller) { + return controller.path(); + } + }); + for (Controller controller : ordering.sortedCopy(controllers)) { write(writer, controller); } writer.endArray(); @@ -68,8 +77,14 @@ public class ListingWs implements WebService { writer.prop("since", controller.since()); writer.prop("description", controller.description()); if (!controller.actions().isEmpty()) { + // sort actions by key + Ordering ordering = Ordering.natural().onResultOf(new Function() { + public String apply(Action action) { + return action.key(); + } + }); writer.name("actions").beginArray(); - for (Action action : controller.actions()) { + for (Action action : ordering.sortedCopy(controller.actions())) { write(writer, action); } writer.endArray(); @@ -83,6 +98,26 @@ public class ListingWs implements WebService { writer.prop("description", action.description()); writer.prop("since", action.since()); writer.prop("post", action.isPost()); + if (!action.params().isEmpty()) { + // sort parameters by key + Ordering ordering = Ordering.natural().onResultOf(new Function() { + public String apply(Param param) { + return param.key(); + } + }); + writer.name("params").beginArray(); + for (Param param : ordering.sortedCopy(action.params())) { + write(writer, param); + } + writer.endArray(); + } + writer.endObject(); + } + + private void write(JsonWriter writer, Param param) { + writer.beginObject(); + writer.prop("key", param.key()); + writer.prop("description", param.description()); writer.endObject(); } } diff --git a/sonar-server/src/main/java/org/sonar/server/ws/ServletRequest.java b/sonar-server/src/main/java/org/sonar/server/ws/ServletRequest.java index d8c62790c92..4e425c74582 100644 --- a/sonar-server/src/main/java/org/sonar/server/ws/ServletRequest.java +++ b/sonar-server/src/main/java/org/sonar/server/ws/ServletRequest.java @@ -26,29 +26,19 @@ import javax.servlet.http.HttpServletRequest; public class ServletRequest extends Request { private final HttpServletRequest source; - private String mediaType = "application/json"; public ServletRequest(HttpServletRequest source) { this.source = source; } @Override - public String param(String key) { - return source.getParameter(key); + public String method() { + return source.getMethod(); } @Override - public String mediaType() { - return mediaType; - } - - public ServletRequest setMediaType(String s) { - this.mediaType = s; - return this; + public String param(String key) { + return source.getParameter(key); } - @Override - public boolean isPost() { - return "POST".equals(source.getMethod()); - } } diff --git a/sonar-server/src/main/java/org/sonar/server/ws/WebServiceEngine.java b/sonar-server/src/main/java/org/sonar/server/ws/WebServiceEngine.java index 43f10176456..f0a92d9b4dc 100644 --- a/sonar-server/src/main/java/org/sonar/server/ws/WebServiceEngine.java +++ b/sonar-server/src/main/java/org/sonar/server/ws/WebServiceEngine.java @@ -20,6 +20,7 @@ package org.sonar.server.ws; import org.picocontainer.Startable; +import org.slf4j.LoggerFactory; import org.sonar.api.ServerComponent; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; @@ -71,12 +72,18 @@ public class WebServiceEngine implements ServerComponent, Startable { verifyRequest(action, request); action.handler().handle(request, response); + } catch (IllegalArgumentException e) { + // TODO replace by BadRequestException in Request#requiredParam() + sendError(400, e.getMessage(), response); + } catch (ServerException e) { // TODO support ServerException l10n messages sendError(e.httpCode(), e.getMessage(), response); } catch (Exception e) { - sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, e.getMessage(), response); + // TODO implement Request.toString() + LoggerFactory.getLogger(getClass()).error("Fail to process request " + request, e); + sendError(500, e.getMessage(), response); } } @@ -93,10 +100,10 @@ public class WebServiceEngine implements ServerComponent, Startable { } private void verifyRequest(WebService.Action action, Request request) { - if (request.isPost() != action.isPost()) { - throw new ServerException(HttpServletResponse.SC_METHOD_NOT_ALLOWED, "Method POST is required"); + // verify the HTTP verb + if (action.isPost() && !"POST".equals(request.method())) { + throw new ServerException(HttpServletResponse.SC_METHOD_NOT_ALLOWED, "HTTP method POST is required"); } - // TODO verify required parameters } private void sendError(int status, String message, Response response) { diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/java_ws_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/java_ws_controller.rb index 212c235297d..d306d65d8c8 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/java_ws_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/java_ws_controller.rb @@ -32,7 +32,6 @@ class Api::JavaWsController < Api::ApiController private def execute(media_type) ws_request = Java::OrgSonarServerWs::ServletRequest.new(servlet_request) - ws_request.setMediaType(media_type) # servlet_request is declared in jruby-rack but not servlet_response ! # request.env must be used. diff --git a/sonar-server/src/test/java/org/sonar/server/issue/ws/IssueShowWsHandlerTest.java b/sonar-server/src/test/java/org/sonar/server/issue/ws/IssueShowWsHandlerTest.java new file mode 100644 index 00000000000..96771009335 --- /dev/null +++ b/sonar-server/src/test/java/org/sonar/server/issue/ws/IssueShowWsHandlerTest.java @@ -0,0 +1,29 @@ +/* + * 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.server.issue.ws; + +import org.junit.Test; + +public class IssueShowWsHandlerTest { + @Test + public void testHandle() throws Exception { + // TODO + } +} diff --git a/sonar-server/src/test/java/org/sonar/server/issue/ws/IssuesWsTest.java b/sonar-server/src/test/java/org/sonar/server/issue/ws/IssuesWsTest.java new file mode 100644 index 00000000000..bf66f868edf --- /dev/null +++ b/sonar-server/src/test/java/org/sonar/server/issue/ws/IssuesWsTest.java @@ -0,0 +1,49 @@ +/* + * 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.server.issue.ws; + +import org.junit.Test; +import org.sonar.api.server.ws.WebService; +import org.sonar.server.ws.WsTester; + +import static org.fest.assertions.Assertions.assertThat; +import static org.mockito.Mockito.mock; + +public class IssuesWsTest { + + IssueShowWsHandler showHandler = mock(IssueShowWsHandler.class); + WsTester tester = new WsTester(new IssuesWs(showHandler)); + + @Test + public void define_ws() throws Exception { + WebService.Controller controller = tester.controller("api/issues"); + assertThat(controller).isNotNull(); + assertThat(controller.description()).isNotEmpty(); + + WebService.Action page = controller.action("show"); + assertThat(page).isNotNull(); + assertThat(page.handler()).isNotNull(); + assertThat(page.since()).isEqualTo("4.2"); + assertThat(page.isPost()).isFalse(); + assertThat(page.isPrivate()).isTrue(); + assertThat(page.handler()).isSameAs(showHandler); + } + +} diff --git a/sonar-server/src/test/java/org/sonar/server/rule/RuleWsTest.java b/sonar-server/src/test/java/org/sonar/server/rule/RulesWsTest.java similarity index 95% rename from sonar-server/src/test/java/org/sonar/server/rule/RuleWsTest.java rename to sonar-server/src/test/java/org/sonar/server/rule/RulesWsTest.java index 8778dfca58f..8487236a966 100644 --- a/sonar-server/src/test/java/org/sonar/server/rule/RuleWsTest.java +++ b/sonar-server/src/test/java/org/sonar/server/rule/RulesWsTest.java @@ -26,10 +26,9 @@ import org.sonar.server.ws.WsTester; import static org.fest.assertions.Assertions.assertThat; -public class RuleWsTest { +public class RulesWsTest { - RuleWs ws = new RuleWs(); - WsTester tester = new WsTester(ws); + WsTester tester = new WsTester(new RulesWs()); @Test public void define_ws() throws Exception { diff --git a/sonar-server/src/test/java/org/sonar/server/ws/ListingWsTest.java b/sonar-server/src/test/java/org/sonar/server/ws/ListingWsTest.java index ce1388bd33e..de28438d8fe 100644 --- a/sonar-server/src/test/java/org/sonar/server/ws/ListingWsTest.java +++ b/sonar-server/src/test/java/org/sonar/server/ws/ListingWsTest.java @@ -71,14 +71,18 @@ public class ListingWsTest { NewController newController = context.newController("api/metric") .setDescription("Metrics") .setSince("3.2"); + + // action with default values newController.newAction("show") - .setDescription("Show metric") .setHandler(new RequestHandler() { @Override public void handle(Request request, Response response) { } }); - newController.newAction("create") + + + // action with a lot of overridden values + NewAction create = newController.newAction("create") .setDescription("Create metric") .setSince("4.1") .setPost(true) @@ -87,6 +91,8 @@ public class ListingWsTest { public void handle(Request request, Response response) { } }); + create.newParam("key").setDescription("Key of new metric"); + create.newParam("name"); newController.done(); } } diff --git a/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java b/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java index 6893f691f58..e681e2a44fe 100644 --- a/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java +++ b/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java @@ -19,6 +19,7 @@ */ package org.sonar.server.ws; +import org.apache.commons.io.IOUtils; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -83,7 +84,39 @@ public class WebServiceEngineTest { engine.execute(request, response, "api/system", "ping"); assertThat(response.status()).isEqualTo(405); - assertThat(response.outputAsString()).isEqualTo("{\"errors\":[{\"msg\":\"Method POST is required\"}]}"); + assertThat(response.outputAsString()).isEqualTo("{\"errors\":[{\"msg\":\"HTTP method POST is required\"}]}"); + } + + @Test + public void required_parameter_is_not_set() throws Exception { + Request request = new SimpleRequest(); + SimpleResponse response = new SimpleResponse(); + engine.execute(request, response, "api/system", "print"); + + assertThat(response.status()).isEqualTo(400); + assertThat(response.outputAsString()).isEqualTo("{\"errors\":[{\"msg\":\"Parameter 'message' is missing\"}]}"); + } + + @Test + public void optional_parameter_is_not_set() throws Exception { + Request request = new SimpleRequest().setParam("message", "Hello World"); + SimpleResponse response = new SimpleResponse(); + engine.execute(request, response, "api/system", "print"); + + assertThat(response.status()).isEqualTo(200); + assertThat(response.outputAsString()).isEqualTo("Hello World by -"); + } + + @Test + public void optional_parameter_is_set() throws Exception { + Request request = new SimpleRequest() + .setParam("message", "Hello World") + .setParam("author", "Marcel"); + SimpleResponse response = new SimpleResponse(); + engine.execute(request, response, "api/system", "print"); + + assertThat(response.status()).isEqualTo(200); + assertThat(response.outputAsString()).isEqualTo("Hello World by Marcel"); } @Test @@ -122,6 +155,18 @@ public class WebServiceEngineTest { throw new IllegalStateException("Unexpected"); } }); + + // parameter "message" is required but not "author" + newController.newAction("print") + .newParam("message", "required message") + .newParam("author", "optional author") + .setHandler(new RequestHandler() { + @Override + public void handle(Request request, Response response) throws Exception { + IOUtils.write( + request.requiredParam("message") + " by " + request.param("author", "-"), response.stream()); + } + }); newController.done(); } } diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/RuleWsTest/search.json b/sonar-server/src/test/resources/org/sonar/server/rule/RulesWsTest/search.json similarity index 100% rename from sonar-server/src/test/resources/org/sonar/server/rule/RuleWsTest/search.json rename to sonar-server/src/test/resources/org/sonar/server/rule/RulesWsTest/search.json diff --git a/sonar-server/src/test/resources/org/sonar/server/ws/ListingWebServiceTest/index.json b/sonar-server/src/test/resources/org/sonar/server/ws/ListingWebServiceTest/index.json index 16bb8e05193..6ee68a24252 100644 --- a/sonar-server/src/test/resources/org/sonar/server/ws/ListingWebServiceTest/index.json +++ b/sonar-server/src/test/resources/org/sonar/server/ws/ListingWebServiceTest/index.json @@ -1,34 +1,42 @@ { - "webServices": [ - { - "path": "api/metric", - "since": "3.2", - "description": "Metrics", - "actions": [ + "webServices": [ { - "key": "show", - "description": "Show metric", - "since": "3.2", - "post": false + "path": "api/metric", + "since": "3.2", + "description": "Metrics", + "actions": [ + { + "key": "create", + "description": "Create metric", + "since": "4.1", + "post": true, + "params": [ + { + "key": "key", + "description": "Key of new metric" + }, + { + "key": "name" + } + ] + }, + { + "key": "show", + "since": "3.2", + "post": false + } + ] }, { - "key": "create", - "description": "Create metric", - "since": "4.1", - "post": true + "path": "api/webservices", + "since": "4.2", + "description": "List web services", + "actions": [ + { + "key": "index", + "since": "4.2", + "post": false + } + ] } - ] - }, - { - "path": "api/webservices", - "since": "4.2", - "description": "List web services", - "actions": [ - { - "key": "index", - "since": "4.2", - "post": false - } - ] - } - ]} + ]} -- 2.39.5