From c07988f6dc6ec06d9f88c88eaa0722e2e6f3f458 Mon Sep 17 00:00:00 2001 From: Teryk Bellahsene Date: Wed, 23 Mar 2016 10:33:03 +0100 Subject: [PATCH] SONAR-6717 Deprecate WS api/issues/do_action --- .../org/sonar/server/issue/ActionService.java | 104 +-------------- .../issue/InternalRubyIssueService.java | 10 -- .../org/sonar/server/issue/ws/IssuesWs.java | 42 ++++-- .../sonar/server/issue/ActionServiceTest.java | 122 ++---------------- .../app/controllers/api/issues_controller.rb | 22 ---- 5 files changed, 38 insertions(+), 262 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ActionService.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ActionService.java index 0b2539893e3..db03b17d1c2 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ActionService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ActionService.java @@ -19,28 +19,13 @@ */ package org.sonar.server.issue; -import com.google.common.base.Predicate; -import com.google.common.base.Strings; -import java.util.Date; import java.util.List; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; -import org.sonar.api.config.Settings; import org.sonar.api.issue.Issue; -import org.sonar.api.issue.action.Action; -import org.sonar.api.issue.action.Actions; -import org.sonar.api.issue.action.Function; import org.sonar.api.server.ServerSide; -import org.sonar.core.issue.DefaultIssue; -import org.sonar.core.issue.IssueChangeContext; import org.sonar.db.DbClient; import org.sonar.db.DbSession; -import org.sonar.server.properties.ProjectSettingsFactory; import org.sonar.server.user.UserSession; -import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Preconditions.checkState; -import static com.google.common.collect.Iterables.find; import static com.google.common.collect.Lists.newArrayList; import static org.sonar.api.web.UserRole.ISSUE_ADMIN; @@ -52,21 +37,12 @@ public class ActionService { private final DbClient dbClient; private final UserSession userSession; - private final Actions actions; private final IssueService issueService; - private final IssueUpdater updater; - private final ProjectSettingsFactory projectSettingsFactory; - private final IssueStorage issueStorage; - public ActionService(DbClient dbClient, UserSession userSession, ProjectSettingsFactory projectSettingsFactory, Actions actions, IssueService issueService, IssueUpdater updater, - IssueStorage issueStorage) { + public ActionService(DbClient dbClient, UserSession userSession, IssueService issueService) { this.dbClient = dbClient; this.userSession = userSession; - this.actions = actions; this.issueService = issueService; - this.updater = updater; - this.projectSettingsFactory = projectSettingsFactory; - this.issueStorage = issueStorage; } public List listAvailableActions(String issueKey) { @@ -100,82 +76,4 @@ public class ActionService { return availableActions; } - public Issue execute(String issueKey, String actionKey) { - checkArgument(!Strings.isNullOrEmpty(actionKey), "Missing action"); - - DbSession session = dbClient.openSession(false); - try { - DefaultIssue issue = issueService.getByKeyForUpdate(session, issueKey).toDefaultIssue(); - Action action = getAction(actionKey, issue); - - IssueChangeContext changeContext = IssueChangeContext.createUser(new Date(), userSession.getLogin()); - FunctionContext functionContext = new FunctionContext(issue, updater, changeContext, projectSettingsFactory.newProjectSettings(issue.projectKey())); - for (Function function : action.functions()) { - function.execute(functionContext); - } - issueStorage.save(issue); - return issue; - } finally { - dbClient.closeSession(session); - } - } - - private Action getAction(String actionKey, Issue issue) { - Action action = find(actions.list(), new MatchActionKey(actionKey), null); - checkArgument(action != null, String.format("Action is not found : %s", actionKey)); - checkState(action.supports(issue), "A condition is not respected"); - return action; - } - - static class FunctionContext implements Function.Context { - - private final DefaultIssue issue; - private final IssueUpdater updater; - private final IssueChangeContext changeContext; - private final Settings projectSettings; - - FunctionContext(DefaultIssue issue, IssueUpdater updater, IssueChangeContext changeContext, Settings projectSettings) { - this.updater = updater; - this.issue = issue; - this.changeContext = changeContext; - this.projectSettings = projectSettings; - } - - @Override - public Issue issue() { - return issue; - } - - @Override - public Settings projectSettings() { - return projectSettings; - } - - @Override - public Function.Context setAttribute(String key, @Nullable String value) { - updater.setAttribute(issue, key, value, changeContext); - return this; - } - - @Override - public Function.Context addComment(@Nullable String text) { - if (text != null) { - updater.addComment(issue, text, changeContext); - } - return this; - } - } - - private static class MatchActionKey implements Predicate { - private final String actionKey; - - private MatchActionKey(String actionKey) { - this.actionKey = actionKey; - } - - @Override - public boolean apply(@Nonnull Action action) { - return action.key().equals(actionKey); - } - } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java b/server/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java index 2d9cf99c9f0..119d057c0f3 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java @@ -311,16 +311,6 @@ public class InternalRubyIssueService { return result; } - public Result executeAction(String issueKey, String actionKey) { - Result result = Result.of(); - try { - result.set(actionService.execute(issueKey, actionKey)); - } catch (Exception e) { - result.addError(e.getMessage()); - } - return result; - } - public List listActions(String issueKey) { return actionService.listAvailableActions(issueKey); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/IssuesWs.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/IssuesWs.java index 926fc3197f9..47d0bcc832d 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/IssuesWs.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/IssuesWs.java @@ -24,7 +24,10 @@ import org.sonar.api.issue.DefaultTransitions; import org.sonar.api.rule.Severity; import org.sonar.api.rules.RuleType; import org.sonar.api.server.ws.RailsHandler; +import org.sonar.api.server.ws.Request; +import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; +import org.sonar.server.ws.WsAction; public class IssuesWs implements WebService { @@ -185,20 +188,33 @@ public class IssuesWs implements WebService { } private static void defineDoActionAction(NewController controller) { - WebService.NewAction action = controller.createAction(DO_ACTION_ACTION) - .setDescription("Do workflow transition on an issue. Requires authentication and Browse permission on project") - .setSince("3.6") - .setHandler(RailsHandler.INSTANCE) - .setPost(true); + new DoAction().define(controller); + } - action.createParam("issue") - .setDescription("Key of the issue") - .setRequired(true) - .setExampleValue("5bccd6e8-f525-43a2-8d76-fcb13dde79ef"); - action.createParam("actionKey") - .setDescription("Action to perform") - .setExampleValue("link-to-jira"); - RailsHandler.addFormatParam(action); + private static class DoAction implements WsAction { + @Override + public void define(NewController context) { + WebService.NewAction action = context.createAction(DO_ACTION_ACTION) + .setDescription("Deprecated web service to do custom workflow transition on an issue. Custom issue are dropped in 5.5. This web service has no effect.") + .setSince("3.6") + .setDeprecatedSince("5.5") + .setHandler(this) + .setPost(true); + + action.createParam("issue") + .setDescription("Key of the issue") + .setRequired(true) + .setExampleValue("5bccd6e8-f525-43a2-8d76-fcb13dde79ef"); + action.createParam("actionKey") + .setDescription("Action to perform") + .setExampleValue("link-to-jira"); + RailsHandler.addFormatParam(action); + } + + @Override + public void handle(Request request, Response response) throws Exception { + response.noContent(); + } } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ActionServiceTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ActionServiceTest.java index 8857fd70985..cb8825b47db 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ActionServiceTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ActionServiceTest.java @@ -24,12 +24,6 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; import org.sonar.api.config.Settings; -import org.sonar.api.issue.Issue; -import org.sonar.api.issue.action.Actions; -import org.sonar.api.issue.action.Function; -import org.sonar.api.issue.condition.Condition; -import org.sonar.core.issue.DefaultIssue; -import org.sonar.core.issue.IssueChangeContext; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.component.ComponentDto; @@ -38,11 +32,7 @@ import org.sonar.server.properties.ProjectSettingsFactory; import org.sonar.server.tester.UserSessionRule; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import static org.sonar.api.issue.Issue.RESOLUTION_FIXED; import static org.sonar.api.web.UserRole.ISSUE_ADMIN; @@ -56,7 +46,6 @@ public class ActionServiceTest { static final String PROJECT_UUID = "PROJECT_UUID"; static final String ISSUE_KEY = "ISSUE_KEY"; - static final String PLUGIN_ACTION = "link-to-jira"; @Rule public ExpectedException thrown = ExpectedException.none(); @@ -68,12 +57,9 @@ public class ActionServiceTest { DbSession session = mock(DbSession.class); IssueService issueService = mock(IssueService.class); - IssueStorage issueStorage = mock(IssueStorage.class); - IssueUpdater updater = mock(IssueUpdater.class); ProjectSettingsFactory projectSettingsFactory = mock(ProjectSettingsFactory.class); Settings settings = new Settings(); - Actions actions = new Actions(); - ActionService actionService; + ActionService underTest; ComponentDto project; IssueDto issue; @@ -86,135 +72,43 @@ public class ActionServiceTest { project = newProjectDto(PROJECT_UUID).setKey(PROJECT_KEY); issue = IssueTesting.newDto(newXooX1().setId(10), newFileDto(project), project).setKee(ISSUE_KEY); - actionService = new ActionService(dbClient, userSession, projectSettingsFactory, actions, issueService, updater, issueStorage); - } - - @Test - public void execute_functions() { - Function function1 = mock(Function.class); - Function function2 = mock(Function.class); - - when(issueService.getByKeyForUpdate(session, ISSUE_KEY)).thenReturn(issue); - - actions.add(PLUGIN_ACTION).setConditions(new AlwaysMatch()).setFunctions(function1, function2); - - assertThat(actionService.execute(ISSUE_KEY, PLUGIN_ACTION)).isNotNull(); - - verify(function1).execute(any(Function.Context.class)); - verify(function2).execute(any(Function.Context.class)); - verifyNoMoreInteractions(function1, function2); - } - - @Test - public void modify_issue_when_executing_a_function() { - Function function = new TweetFunction(); - when(issueService.getByKeyForUpdate(session, ISSUE_KEY)).thenReturn(issue); - - actions.add(PLUGIN_ACTION).setConditions(new AlwaysMatch()).setFunctions(function); - assertThat(actionService.execute(ISSUE_KEY, PLUGIN_ACTION)).isNotNull(); - - verify(updater).addComment(any(DefaultIssue.class), eq("New tweet on issue ISSUE_KEY"), any(IssueChangeContext.class)); - verify(updater).setAttribute(any(DefaultIssue.class), eq("tweet"), eq("tweet sent"), any(IssueChangeContext.class)); - } - - @Test - public void use_project_settings_when_executing_a_function() { - Function function = new SettingsFunction(); - when(issueService.getByKeyForUpdate(session, ISSUE_KEY)).thenReturn(issue); - settings.setProperty("key", "value"); - - actions.add(PLUGIN_ACTION).setConditions(new AlwaysMatch()).setFunctions(function); - actionService.execute(ISSUE_KEY, PLUGIN_ACTION); - - verify(updater).addComment(any(DefaultIssue.class), eq("Property 'key' is 'value'"), any(IssueChangeContext.class)); - } - - @Test - public void not_execute_function_if_action_not_found() { - thrown.expect(IllegalArgumentException.class); - thrown.expectMessage("Action is not found : tweet"); - - Function function = mock(Function.class); - when(issueService.getByKeyForUpdate(session, ISSUE_KEY)).thenReturn(issue); - actions.add(PLUGIN_ACTION).setConditions(new AlwaysMatch()).setFunctions(function); - actionService.execute(ISSUE_KEY, "tweet"); - } - - @Test - public void not_execute_function_if_action_is_not_supported() { - thrown.expect(IllegalStateException.class); - thrown.expectMessage("A condition is not respected"); - - Function function = mock(Function.class); - - when(issueService.getByKeyForUpdate(session, ISSUE_KEY)).thenReturn(issue); - actions.add(PLUGIN_ACTION).setConditions(new NeverMatch()).setFunctions(function); - actionService.execute(ISSUE_KEY, PLUGIN_ACTION); + underTest = new ActionService(dbClient, userSession, issueService); } @Test public void return_provided_actions_without_set_severity_when_not_issue_admin() { - assertThat(actionService.listAvailableActions(issue.toDefaultIssue())).containsOnly("comment", "assign", "set_tags", "set_type", "assign_to_me", "plan"); + assertThat(underTest.listAvailableActions(issue.toDefaultIssue())).containsOnly("comment", "assign", "set_tags", "set_type", "assign_to_me", "plan"); } @Test public void return_provided_actions_with_set_severity_when_issue_admin() { userSession.addProjectUuidPermissions(ISSUE_ADMIN, PROJECT_UUID); - assertThat(actionService.listAvailableActions(issue.toDefaultIssue())).containsOnly("comment", "assign", "set_tags", "set_type", "assign_to_me", "plan", "set_severity"); + assertThat(underTest.listAvailableActions(issue.toDefaultIssue())).containsOnly("comment", "assign", "set_tags", "set_type", "assign_to_me", "plan", "set_severity"); } @Test public void return_no_actions_when_not_logged() { userSession.anonymous(); - assertThat(actionService.listAvailableActions(issue.toDefaultIssue())).isEmpty(); + assertThat(underTest.listAvailableActions(issue.toDefaultIssue())).isEmpty(); } @Test public void doest_not_return_assign_to_me_action_when_issue_already_assigned_to_user() { userSession.login("julien"); IssueDto issue = IssueTesting.newDto(newXooX1().setId(10), newFileDto(project), project).setKee(ISSUE_KEY).setAssignee("julien"); - assertThat(actionService.listAvailableActions(issue.toDefaultIssue())).doesNotContain("assign_to_me"); + assertThat(underTest.listAvailableActions(issue.toDefaultIssue())).doesNotContain("assign_to_me"); } @Test public void return_only_comment_action_when_issue_has_a_resolution() { IssueDto issue = IssueTesting.newDto(newXooX1().setId(10), newFileDto(project), project).setKee(ISSUE_KEY).setResolution(RESOLUTION_FIXED); - assertThat(actionService.listAvailableActions(issue.toDefaultIssue())).containsOnly("comment"); + assertThat(underTest.listAvailableActions(issue.toDefaultIssue())).containsOnly("comment"); } @Test public void return_actions_by_issue_key() { when(issueService.getByKeyForUpdate(session, ISSUE_KEY)).thenReturn(issue); - assertThat(actionService.listAvailableActions(ISSUE_KEY)).isNotEmpty(); - } - - public class AlwaysMatch implements Condition { - @Override - public boolean matches(Issue issue) { - return true; - } - } - - public class NeverMatch implements Condition { - @Override - public boolean matches(Issue issue) { - return false; - } - } - - public class TweetFunction implements Function { - @Override - public void execute(Context context) { - context.addComment("New tweet on issue " + context.issue().key()); - context.setAttribute("tweet", "tweet sent"); - } - } - - public class SettingsFunction implements Function { - @Override - public void execute(Context context) { - context.addComment(String.format("Property 'key' is '%s'", context.projectSettings().getString("key"))); - } + assertThat(underTest.listAvailableActions(ISSUE_KEY)).isNotEmpty(); } } diff --git a/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb b/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb index e6488d26abc..18d0f2a3ecc 100644 --- a/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb +++ b/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb @@ -147,28 +147,6 @@ class Api::IssuesController < Api::ApiController ) end - # - # POST /api/issues/do_action?issue=&actionKey= - # - # -- Example - # curl -X POST -v -u admin:admin 'http://localhost:9000/api/issues/do_action?issue=9b6f89c0-3347-46f6-a6d1-dd6c761240e0&actionKey=link-to-jira' - # - def do_action - verify_post_request - require_parameters :issue, :actionKey - - result = Internal.issues.executeAction(params[:issue], params[:actionKey]) - - http_status = (result.ok ? 200 : 400) - hash = result_to_hash(result) - - respond_to do |format| - # if the request header "Accept" is "*/*", then the default format is the first one (json) - format.json { render :json => jsonp(hash), :status => result.httpStatus } - format.xml { render :xml => hash.to_xml(:skip_types => true, :root => 'sonar', :status => http_status) } - end - end - # # Execute a bulk change on a list of issues # -- 2.39.5