Skip to content

Commit

Permalink
SONAR-7290 Simplify and rename ActionService to ActionFinder
Browse files Browse the repository at this point in the history
  • Loading branch information
julienlancelot committed Dec 13, 2016
1 parent 29beda1 commit 4f4c4b6
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 76 deletions.
Expand Up @@ -20,10 +20,7 @@
package org.sonar.server.issue; package org.sonar.server.issue;


import java.util.List; import java.util.List;
import org.sonar.api.issue.Issue; import org.sonar.db.issue.IssueDto;
import org.sonar.api.server.ServerSide;
import org.sonar.db.DbClient;
import org.sonar.db.DbSession;
import org.sonar.server.user.UserSession; import org.sonar.server.user.UserSession;


import static com.google.common.collect.Lists.newArrayList; import static com.google.common.collect.Lists.newArrayList;
Expand All @@ -32,41 +29,27 @@
/** /**
* @since 3.6 * @since 3.6
*/ */
@ServerSide public class ActionFinder {
public class ActionService {


private final DbClient dbClient;
private final UserSession userSession; private final UserSession userSession;
private final IssueService issueService;


public ActionService(DbClient dbClient, UserSession userSession, IssueService issueService) { public ActionFinder(UserSession userSession) {
this.dbClient = dbClient;
this.userSession = userSession; this.userSession = userSession;
this.issueService = issueService;
} }


public List<String> listAvailableActions(String issueKey) { public List<String> listAvailableActions(IssueDto issue) {
DbSession session = dbClient.openSession(false);
try {
return listAvailableActions(issueService.getByKeyForUpdate(session, issueKey).toDefaultIssue());
} finally {
dbClient.closeSession(session);
}
}

public List<String> listAvailableActions(Issue issue) {
List<String> availableActions = newArrayList(); List<String> availableActions = newArrayList();
String login = userSession.getLogin(); String login = userSession.getLogin();
if (login != null) { if (login != null) {
availableActions.add("comment"); availableActions.add("comment");
if (issue.resolution() == null) { if (issue.getResolution() == null) {
availableActions.add("assign"); availableActions.add("assign");
availableActions.add("set_tags"); availableActions.add("set_tags");
availableActions.add("set_type"); availableActions.add("set_type");
if (!login.equals(issue.assignee())) { if (!login.equals(issue.getAssignee())) {
availableActions.add("assign_to_me"); availableActions.add("assign_to_me");
} }
String projectUuid = issue.projectUuid(); String projectUuid = issue.getProjectUuid();
if (projectUuid != null && userSession.hasComponentUuidPermission(ISSUE_ADMIN, projectUuid)) { if (projectUuid != null && userSession.hasComponentUuidPermission(ISSUE_ADMIN, projectUuid)) {
availableActions.add("set_severity"); availableActions.add("set_severity");
} }
Expand Down
Expand Up @@ -144,10 +144,6 @@ public IssueComment editComment(String commentKey, String text) {
return comment; return comment;
} }


public boolean canEditOrDelete(IssueChangeDto dto) {
return userSession.isLoggedIn() && userSession.getLogin().equals(dto.getUserLogin());
}

private void verifyLoggedIn(UserSession userSession) { private void verifyLoggedIn(UserSession userSession) {
if (!userSession.isLoggedIn()) { if (!userSession.isLoggedIn()) {
throw new UnauthorizedException("User is not logged in"); throw new UnauthorizedException("User is not logged in");
Expand Down
Expand Up @@ -37,12 +37,12 @@ public OperationResponseWriter(SearchResponseLoader loader, SearchResponseFormat
this.format = format; this.format = format;
} }


public void write(String issueKey, Request request, Response response) throws Exception { public void write(String issueKey, Request request, Response response) {
SearchResponseLoader.Collector collector = new SearchResponseLoader.Collector( SearchResponseLoader.Collector collector = new SearchResponseLoader.Collector(
ALL_ADDITIONAL_FIELDS, singletonList(issueKey)); ALL_ADDITIONAL_FIELDS, singletonList(issueKey));
SearchResponseData data = loader.load(collector, null); SearchResponseData data = loader.load(collector, null);


Issues.Operation responseBody = this.format.formatOperation(data); Issues.Operation responseBody = format.formatOperation(data);


WsUtils.writeProtobuf(responseBody, request, response); WsUtils.writeProtobuf(responseBody, request, response);
} }
Expand Down
Expand Up @@ -37,9 +37,9 @@
import org.sonar.db.issue.IssueDto; import org.sonar.db.issue.IssueDto;
import org.sonar.db.protobuf.DbIssues; import org.sonar.db.protobuf.DbIssues;
import org.sonar.server.es.Facets; import org.sonar.server.es.Facets;
import org.sonar.server.issue.ActionService; import org.sonar.server.issue.ActionFinder;
import org.sonar.server.issue.IssueCommentService;
import org.sonar.server.issue.TransitionService; import org.sonar.server.issue.TransitionService;
import org.sonar.server.user.UserSession;
import org.sonarqube.ws.client.issue.IssuesWsParameters; import org.sonarqube.ws.client.issue.IssuesWsParameters;


import static com.google.common.collect.Lists.newArrayList; import static com.google.common.collect.Lists.newArrayList;
Expand All @@ -54,15 +54,15 @@
*/ */
public class SearchResponseLoader { public class SearchResponseLoader {


private final UserSession userSession;
private final DbClient dbClient; private final DbClient dbClient;
private final ActionService actionService; private final ActionFinder actionService;
private final IssueCommentService commentService;
private final TransitionService transitionService; private final TransitionService transitionService;


public SearchResponseLoader(DbClient dbClient, ActionService actionService, IssueCommentService commentService, TransitionService transitionService) { public SearchResponseLoader(UserSession userSession, DbClient dbClient, ActionFinder actionService, TransitionService transitionService) {
this.userSession = userSession;
this.dbClient = dbClient; this.dbClient = dbClient;
this.actionService = actionService; this.actionService = actionService;
this.commentService = commentService;
this.transitionService = transitionService; this.transitionService = transitionService;
} }


Expand Down Expand Up @@ -101,13 +101,17 @@ private void loadComments(Collector collector, DbSession dbSession, SearchRespon
result.setComments(comments); result.setComments(comments);
for (IssueChangeDto comment : comments) { for (IssueChangeDto comment : comments) {
collector.add(USERS, comment.getUserLogin()); collector.add(USERS, comment.getUserLogin());
if (commentService.canEditOrDelete(comment)) { if (canEditOrDelete(comment)) {
result.addUpdatableComment(comment.getKey()); result.addUpdatableComment(comment.getKey());
} }
} }
} }
} }


private boolean canEditOrDelete(IssueChangeDto dto) {
return userSession.isLoggedIn() && userSession.getLogin().equals(dto.getUserLogin());
}

private void loadRules(Collector collector, DbSession dbSession, SearchResponseData result) { private void loadRules(Collector collector, DbSession dbSession, SearchResponseData result) {
if (collector.contains(RULES)) { if (collector.contains(RULES)) {
result.setRules(dbClient.ruleDao().selectByKeys(dbSession, collector.<RuleKey>get(RULES))); result.setRules(dbClient.ruleDao().selectByKeys(dbSession, collector.<RuleKey>get(RULES)));
Expand All @@ -131,7 +135,7 @@ private void loadActionsAndTransitions(Collector collector, SearchResponseData r
for (IssueDto dto : result.getIssues()) { for (IssueDto dto : result.getIssues()) {
// so that IssueDto can be used. // so that IssueDto can be used.
if (collector.contains(ACTIONS)) { if (collector.contains(ACTIONS)) {
result.addActions(dto.getKey(), actionService.listAvailableActions(dto.toDefaultIssue())); result.addActions(dto.getKey(), actionService.listAvailableActions(dto));
} }
if (collector.contains(TRANSITIONS)) { if (collector.contains(TRANSITIONS)) {
// TODO workflow and action engines must not depend on org.sonar.api.issue.Issue but on a generic interface // TODO workflow and action engines must not depend on org.sonar.api.issue.Issue but on a generic interface
Expand Down
Expand Up @@ -57,7 +57,7 @@
import org.sonar.server.es.IndexCreator; import org.sonar.server.es.IndexCreator;
import org.sonar.server.es.IndexDefinitions; import org.sonar.server.es.IndexDefinitions;
import org.sonar.server.event.NewAlerts; import org.sonar.server.event.NewAlerts;
import org.sonar.server.issue.ActionService; import org.sonar.server.issue.ActionFinder;
import org.sonar.server.issue.AddTagsAction; import org.sonar.server.issue.AddTagsAction;
import org.sonar.server.issue.AssignAction; import org.sonar.server.issue.AssignAction;
import org.sonar.server.issue.CommentAction; import org.sonar.server.issue.CommentAction;
Expand Down Expand Up @@ -404,7 +404,7 @@ protected void configureLevel() {
IssueCommentService.class, IssueCommentService.class,
InternalRubyIssueService.class, InternalRubyIssueService.class,
IssueChangelogService.class, IssueChangelogService.class,
ActionService.class, ActionFinder.class,
IssueBulkChangeService.class, IssueBulkChangeService.class,
WsResponseCommonFormat.class, WsResponseCommonFormat.class,
IssueWsModule.class, IssueWsModule.class,
Expand Down
Expand Up @@ -19,26 +19,22 @@
*/ */
package org.sonar.server.issue; package org.sonar.server.issue;


import org.junit.Before;
import org.junit.Rule; import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
import org.junit.rules.ExpectedException; import org.junit.rules.ExpectedException;
import org.sonar.db.DbClient;
import org.sonar.db.DbSession;
import org.sonar.db.component.ComponentDto; import org.sonar.db.component.ComponentDto;
import org.sonar.db.issue.IssueDto; import org.sonar.db.issue.IssueDto;
import org.sonar.server.tester.UserSessionRule; import org.sonar.server.tester.UserSessionRule;


import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.sonar.api.issue.Issue.RESOLUTION_FIXED; import static org.sonar.api.issue.Issue.RESOLUTION_FIXED;
import static org.sonar.api.web.UserRole.ISSUE_ADMIN; import static org.sonar.api.web.UserRole.ISSUE_ADMIN;
import static org.sonar.db.component.ComponentTesting.newFileDto; import static org.sonar.db.component.ComponentTesting.newFileDto;
import static org.sonar.db.component.ComponentTesting.newProjectDto; import static org.sonar.db.component.ComponentTesting.newProjectDto;
import static org.sonar.db.rule.RuleTesting.newXooX1; import static org.sonar.db.rule.RuleTesting.newXooX1;
import static org.sonar.server.issue.IssueTesting.newDto;


public class ActionServiceTest { public class ActionFinderTest {


static final String PROJECT_KEY = "PROJECT_KEY"; static final String PROJECT_KEY = "PROJECT_KEY";
static final String PROJECT_UUID = "PROJECT_UUID"; static final String PROJECT_UUID = "PROJECT_UUID";
Expand All @@ -51,59 +47,40 @@ public class ActionServiceTest {
@Rule @Rule
public UserSessionRule userSession = UserSessionRule.standalone().login("arthur"); public UserSessionRule userSession = UserSessionRule.standalone().login("arthur");


DbClient dbClient = mock(DbClient.class); private ComponentDto project = newProjectDto(PROJECT_UUID).setKey(PROJECT_KEY);
DbSession session = mock(DbSession.class); private IssueDto issue = newDto(newXooX1().setId(10), newFileDto(project, null), project).setKee(ISSUE_KEY);


IssueService issueService = mock(IssueService.class); private ActionFinder underTest = new ActionFinder(userSession);
ActionService underTest;

ComponentDto project;
IssueDto issue;

@Before
public void before() {
when(dbClient.openSession(false)).thenReturn(session);

project = newProjectDto(PROJECT_UUID).setKey(PROJECT_KEY);
issue = IssueTesting.newDto(newXooX1().setId(10), newFileDto(project, null), project).setKee(ISSUE_KEY);

underTest = new ActionService(dbClient, userSession, issueService);
}


@Test @Test
public void return_provided_actions_without_set_severity_when_not_issue_admin() { public void return_provided_actions_without_set_severity_when_not_issue_admin() {
assertThat(underTest.listAvailableActions(issue.toDefaultIssue())).containsOnly("comment", "assign", "set_tags", "set_type", "assign_to_me"); assertThat(underTest.listAvailableActions(issue)).containsOnly("comment", "assign", "set_tags", "set_type", "assign_to_me");
} }


@Test @Test
public void return_provided_actions_with_set_severity_when_issue_admin() { public void return_provided_actions_with_set_severity_when_issue_admin() {
userSession.addProjectUuidPermissions(ISSUE_ADMIN, PROJECT_UUID); userSession.addProjectUuidPermissions(ISSUE_ADMIN, PROJECT_UUID);
assertThat(underTest.listAvailableActions(issue.toDefaultIssue())).containsOnly("comment", "assign", "set_tags", "set_type", "assign_to_me", "set_severity"); assertThat(underTest.listAvailableActions(issue)).containsOnly("comment", "assign", "set_tags", "set_type", "assign_to_me", "set_severity");
} }


@Test @Test
public void return_no_actions_when_not_logged() { public void return_no_actions_when_not_logged() {
userSession.anonymous(); userSession.anonymous();
assertThat(underTest.listAvailableActions(issue.toDefaultIssue())).isEmpty(); assertThat(underTest.listAvailableActions(issue)).isEmpty();
} }


@Test @Test
public void doest_not_return_assign_to_me_action_when_issue_already_assigned_to_user() { public void doest_not_return_assign_to_me_action_when_issue_already_assigned_to_user() {

userSession.login("julien"); userSession.login("julien");
IssueDto issue = IssueTesting.newDto(newXooX1().setId(10), newFileDto(project, null), project).setKee(ISSUE_KEY).setAssignee("julien"); IssueDto issue = newDto(newXooX1().setId(10), newFileDto(project, null), project).setKee(ISSUE_KEY).setAssignee("julien");
assertThat(underTest.listAvailableActions(issue.toDefaultIssue())).doesNotContain("assign_to_me"); assertThat(underTest.listAvailableActions(issue)).doesNotContain("assign_to_me");
} }


@Test @Test
public void return_only_comment_action_when_issue_has_a_resolution() { public void return_only_comment_action_when_issue_has_a_resolution() {
IssueDto issue = IssueTesting.newDto(newXooX1().setId(10), newFileDto(project, null), project).setKee(ISSUE_KEY).setResolution(RESOLUTION_FIXED); IssueDto issue = newDto(newXooX1().setId(10), newFileDto(project, null), project).setKee(ISSUE_KEY).setResolution(RESOLUTION_FIXED);
assertThat(underTest.listAvailableActions(issue.toDefaultIssue())).containsOnly("comment"); assertThat(underTest.listAvailableActions(issue)).containsOnly("comment");
}

@Test
public void return_actions_by_issue_key() {
when(issueService.getByKeyForUpdate(session, ISSUE_KEY)).thenReturn(issue);
assertThat(underTest.listAvailableActions(ISSUE_KEY)).isNotEmpty();
} }


} }

0 comments on commit 4f4c4b6

Please sign in to comment.