From a3e7b997a51db49a83447c224e3c9ddf8251cdc9 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Fri, 9 Dec 2016 09:36:43 +0100 Subject: [PATCH] SONAR-7291 Extract transition methods from IssueService to TransitionService --- .../org/sonar/server/issue/IssueFinder.java | 49 ++++++ .../org/sonar/server/issue/IssueService.java | 59 +------ .../org/sonar/server/issue/IssueUpdater.java | 65 +++++++ .../sonar/server/issue/TransitionAction.java | 38 ++--- .../sonar/server/issue/TransitionService.java | 68 ++++++++ .../server/issue/workflow/IssueWorkflow.java | 13 +- .../server/issue/workflow/Transition.java | 13 +- .../server/issue/ws/DoTransitionAction.java | 43 ++++- .../sonar/server/issue/ws/IssueWsModule.java | 6 + .../server/issue/ws/SearchResponseLoader.java | 12 +- .../org/sonar/server/issue/IssueDbTester.java | 39 +++++ .../sonar/server/issue/IssueFinderTest.java | 97 +++++++++++ .../server/issue/IssueServiceMediumTest.java | 31 ---- .../sonar/server/issue/IssueUpdaterTest.java | 139 +++++++++++++++ .../server/issue/TransitionActionTest.java | 105 ++++++------ .../server/issue/TransitionServiceTest.java | 97 +++++++++++ .../server/issue/workflow/TransitionTest.java | 4 + .../issue/ws/DoTransitionActionTest.java | 158 ++++++++++++++++-- .../server/issue/ws/IssueWsModuleTest.java | 2 +- .../java/org/sonar/db/issue/IssueDao.java | 7 +- .../java/org/sonar/db/rule/RuleDbTester.java | 38 +++++ .../java/org/sonar/db/rule/RuleTesting.java | 6 + 22 files changed, 892 insertions(+), 197 deletions(-) create mode 100644 server/sonar-server/src/main/java/org/sonar/server/issue/IssueFinder.java create mode 100644 server/sonar-server/src/main/java/org/sonar/server/issue/IssueUpdater.java create mode 100644 server/sonar-server/src/main/java/org/sonar/server/issue/TransitionService.java create mode 100644 server/sonar-server/src/test/java/org/sonar/server/issue/IssueDbTester.java create mode 100644 server/sonar-server/src/test/java/org/sonar/server/issue/IssueFinderTest.java create mode 100644 server/sonar-server/src/test/java/org/sonar/server/issue/IssueUpdaterTest.java create mode 100644 server/sonar-server/src/test/java/org/sonar/server/issue/TransitionServiceTest.java create mode 100644 sonar-db/src/test/java/org/sonar/db/rule/RuleDbTester.java diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueFinder.java b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueFinder.java new file mode 100644 index 00000000000..c9bf7854849 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueFinder.java @@ -0,0 +1,49 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program 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. + * + * This program 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; + +import org.sonar.api.web.UserRole; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.issue.IssueDto; +import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.user.UserSession; + +import static java.lang.String.format; +import static java.util.Objects.requireNonNull; + +public class IssueFinder { + + private final DbClient dbClient; + private final UserSession userSession; + + public IssueFinder(DbClient dbClient, UserSession userSession) { + this.dbClient = dbClient; + this.userSession = userSession; + } + + public IssueDto getByKey(DbSession session, String issueKey) { + IssueDto issue = dbClient.issueDao().selectByKey(session, issueKey).orElseThrow(() -> new NotFoundException(format("Issue with key '%s' does not exist", issueKey))); + userSession.checkComponentUuidPermission(UserRole.USER, requireNonNull(issue.getProjectUuid())); + return issue; + } + +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java index 6b8b7b4a5e5..5eae3be0c7c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java @@ -21,14 +21,11 @@ package org.sonar.server.issue; import com.google.common.base.Optional; import com.google.common.base.Strings; -import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.Date; import java.util.List; import java.util.Map; import javax.annotation.Nullable; -import org.apache.commons.lang.StringUtils; import org.sonar.api.ce.ComputeEngineSide; import org.sonar.api.issue.Issue; import org.sonar.api.rule.RuleKey; @@ -48,8 +45,6 @@ import org.sonar.db.rule.RuleDto; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.issue.index.IssueIndex; import org.sonar.server.issue.notification.IssueChangeNotification; -import org.sonar.server.issue.workflow.IssueWorkflow; -import org.sonar.server.issue.workflow.Transition; import org.sonar.server.notification.NotificationManager; import org.sonar.server.user.UserSession; @@ -60,7 +55,6 @@ public class IssueService { private final DbClient dbClient; private final IssueIndex issueIndex; - private final IssueWorkflow workflow; private final IssueFieldsSetter issueUpdater; private final IssueStorage issueStorage; private final NotificationManager notificationService; @@ -68,7 +62,6 @@ public class IssueService { private final UserSession userSession; public IssueService(DbClient dbClient, IssueIndex issueIndex, - IssueWorkflow workflow, IssueStorage issueStorage, IssueFieldsSetter issueUpdater, NotificationManager notificationService, @@ -76,7 +69,6 @@ public class IssueService { UserSession userSession) { this.dbClient = dbClient; this.issueIndex = issueIndex; - this.workflow = workflow; this.issueStorage = issueStorage; this.issueUpdater = issueUpdater; this.notificationService = notificationService; @@ -84,53 +76,6 @@ public class IssueService { this.userSession = userSession; } - /** - * 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 - */ - public List listTransitions(@Nullable Issue issue) { - if (issue == null) { - return Collections.emptyList(); - } - List outTransitions = workflow.outTransitions(issue); - List allowedTransitions = new ArrayList<>(); - for (Transition transition : outTransitions) { - String projectUuid = issue.projectUuid(); - if (userSession.isLoggedIn() && StringUtils.isBlank(transition.requiredProjectPermission()) || - (projectUuid != null && userSession.hasComponentUuidPermission(transition.requiredProjectPermission(), projectUuid))) { - allowedTransitions.add(transition); - } - } - return allowedTransitions; - } - - public void doTransition(String issueKey, String transitionKey) { - userSession.checkLoggedIn(); - - DbSession session = dbClient.openSession(false); - try { - DefaultIssue defaultIssue = getByKeyForUpdate(session, issueKey).toDefaultIssue(); - IssueChangeContext context = IssueChangeContext.createUser(new Date(), userSession.getLogin()); - checkTransitionPermission(transitionKey, userSession, defaultIssue); - if (workflow.doTransition(defaultIssue, transitionKey, context)) { - saveIssue(session, defaultIssue, context, null); - } - - } finally { - session.close(); - } - } - - private void checkTransitionPermission(String transitionKey, UserSession userSession, DefaultIssue defaultIssue) { - List outTransitions = workflow.outTransitions(defaultIssue); - for (Transition transition : outTransitions) { - String projectKey = defaultIssue.projectKey(); - if (transition.key().equals(transitionKey) && StringUtils.isNotBlank(transition.requiredProjectPermission()) && projectKey != null) { - userSession.checkComponentPermission(transition.requiredProjectPermission(), projectKey); - } - } - } - public void assign(String issueKey, @Nullable String assignee) { userSession.checkLoggedIn(); @@ -192,12 +137,16 @@ public class IssueService { return issueIndex.getByKey(key); } + // TODO Either use IssueFinder or remove it + @Deprecated IssueDto getByKeyForUpdate(DbSession session, String key) { // Load from index to check permission : if the user has no permission to see the issue an exception will be generated Issue authorizedIssueIndex = getByKey(key); return dbClient.issueDao().selectOrFailByKey(session, authorizedIssueIndex.key()); } + // TODO Either use IssueUpdater or remove it + @Deprecated void saveIssue(DbSession session, DefaultIssue issue, IssueChangeContext context, @Nullable String comment) { String projectKey = issue.projectKey(); if (projectKey == null) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueUpdater.java b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueUpdater.java new file mode 100644 index 00000000000..a05ea7996c4 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueUpdater.java @@ -0,0 +1,65 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program 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. + * + * This program 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; + +import java.util.Optional; +import javax.annotation.Nullable; +import org.sonar.api.rule.RuleKey; +import org.sonar.api.rule.RuleStatus; +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; +import org.sonar.db.rule.RuleDto; +import org.sonar.server.issue.notification.IssueChangeNotification; +import org.sonar.server.notification.NotificationManager; + +public class IssueUpdater { + + private final DbClient dbClient; + private final IssueStorage issueStorage; + private final NotificationManager notificationService; + + public IssueUpdater(DbClient dbClient, IssueStorage issueStorage, NotificationManager notificationService) { + this.dbClient = dbClient; + this.issueStorage = issueStorage; + this.notificationService = notificationService; + } + + public void saveIssue(DbSession session, DefaultIssue issue, IssueChangeContext context, @Nullable String comment) { + issueStorage.save(session, issue); + Optional rule = getRuleByKey(session, issue.getRuleKey()); + ComponentDto project = dbClient.componentDao().selectOrFailByUuid(session, issue.projectUuid()); + notificationService.scheduleForSending(new IssueChangeNotification() + .setIssue(issue) + .setChangeAuthorLogin(context.login()) + .setRuleName(rule.isPresent() ? rule.get().getName() : null) + .setProject(project.getKey(), project.name()) + .setComponent(dbClient.componentDao().selectOrFailByUuid(session, issue.componentUuid())) + .setComment(comment)); + } + + private Optional getRuleByKey(DbSession session, RuleKey ruleKey) { + Optional rule = Optional.ofNullable(dbClient.ruleDao().selectByKey(session, ruleKey).orNull()); + return (rule.isPresent() && rule.get().getStatus() != RuleStatus.REMOVED) ? rule : Optional.empty(); + } +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/TransitionAction.java b/server/sonar-server/src/main/java/org/sonar/server/issue/TransitionAction.java index b7f84403407..ed077ca4134 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/TransitionAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/TransitionAction.java @@ -19,28 +19,28 @@ */ package org.sonar.server.issue; -import com.google.common.base.Strings; import java.util.Collection; import java.util.Map; -import org.apache.commons.lang.StringUtils; import org.sonar.api.issue.Issue; import org.sonar.api.server.ServerSide; import org.sonar.core.issue.DefaultIssue; -import org.sonar.server.issue.workflow.IssueWorkflow; +import org.sonar.core.util.stream.Collectors; +import org.sonar.server.issue.workflow.Transition; import org.sonar.server.user.UserSession; +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Strings.isNullOrEmpty; + @ServerSide public class TransitionAction extends Action { public static final String DO_TRANSITION_KEY = "do_transition"; - private final IssueWorkflow workflow; - private final UserSession userSession; + private final TransitionService transitionService; - public TransitionAction(IssueWorkflow workflow, UserSession userSession) { + public TransitionAction(TransitionService transitionService) { super(DO_TRANSITION_KEY); - this.workflow = workflow; - this.userSession = userSession; + this.transitionService = transitionService; } @Override @@ -53,26 +53,20 @@ public class TransitionAction extends Action { public boolean execute(Map properties, Context context) { DefaultIssue issue = (DefaultIssue) context.issue(); String transition = transition(properties); - if (canExecuteTransition(issue, transition)) { - return workflow.doTransition((DefaultIssue) context.issue(), transition(properties), context.issueChangeContext()); - } - return false; + return canExecuteTransition(issue, transition) && transitionService.doTransition((DefaultIssue) context.issue(), context.issueChangeContext(), transition(properties)); } - private boolean canExecuteTransition(Issue issue, final String transition) { - final DefaultIssue defaultIssue = (DefaultIssue) issue; - return workflow.outTransitions(issue).stream() - .filter(input -> input.key().equals(transition) && - (StringUtils.isBlank(input.requiredProjectPermission()) || - userSession.hasComponentPermission(input.requiredProjectPermission(), defaultIssue.projectKey()))) - .findFirst().orElseGet(() -> null) != null; + private boolean canExecuteTransition(DefaultIssue issue, String transitionKey) { + return transitionService.listTransitions(issue) + .stream() + .map(Transition::key) + .collect(Collectors.toSet()) + .contains(transitionKey); } private static String transition(Map properties) { String param = (String) properties.get("transition"); - if (Strings.isNullOrEmpty(param)) { - throw new IllegalArgumentException("Missing parameter : 'transition'"); - } + checkArgument(!isNullOrEmpty(param), "Missing parameter : 'transition'"); return param; } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/TransitionService.java b/server/sonar-server/src/main/java/org/sonar/server/issue/TransitionService.java new file mode 100644 index 00000000000..9e9496c1f54 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/TransitionService.java @@ -0,0 +1,68 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program 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. + * + * This program 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; + +import java.util.List; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.IssueChangeContext; +import org.sonar.core.util.stream.Collectors; +import org.sonar.server.issue.workflow.IssueWorkflow; +import org.sonar.server.issue.workflow.Transition; +import org.sonar.server.user.UserSession; + +import static java.util.Objects.requireNonNull; +import static org.apache.commons.lang.StringUtils.isBlank; +import static org.apache.commons.lang.StringUtils.isNotBlank; + +/** + * This service is a kind of overlay of {@link IssueWorkflow} that also deals with permission checking + */ +public class TransitionService { + + private final UserSession userSession; + private final IssueWorkflow workflow; + + public TransitionService(UserSession userSession, IssueWorkflow workflow) { + this.userSession = userSession; + this.workflow = workflow; + } + + public List listTransitions(DefaultIssue issue) { + String projectUuid = requireNonNull(issue.projectUuid()); + return workflow.outTransitions(issue) + .stream() + .filter(transition -> isBlank(transition.requiredProjectPermission()) || userSession.hasComponentUuidPermission(transition.requiredProjectPermission(), projectUuid)) + .collect(Collectors.toList()); + } + + public boolean doTransition(DefaultIssue defaultIssue, IssueChangeContext issueChangeContext, String transitionKey) { + return workflow.doTransition(defaultIssue, transitionKey, issueChangeContext); + } + + public void checkTransitionPermission(String transitionKey, DefaultIssue defaultIssue) { + String projectUuid = requireNonNull(defaultIssue.projectUuid()); + workflow.outTransitions(defaultIssue) + .stream() + .filter(transition -> transition.key().equals(transitionKey) && isNotBlank(transition.requiredProjectPermission())) + .forEach(transition -> userSession.checkComponentUuidPermission(transition.requiredProjectPermission(), projectUuid)); + } + +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/workflow/IssueWorkflow.java b/server/sonar-server/src/main/java/org/sonar/server/issue/workflow/IssueWorkflow.java index c9768eefb99..ac23a090b42 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/workflow/IssueWorkflow.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/workflow/IssueWorkflow.java @@ -32,6 +32,9 @@ import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.IssueChangeContext; import org.sonar.server.issue.IssueFieldsSetter; +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkState; + @ServerSide @ComputeEngineSide public class IssueWorkflow implements Startable { @@ -167,7 +170,7 @@ public class IssueWorkflow implements Startable { public boolean doTransition(DefaultIssue issue, String transitionKey, IssueChangeContext issueChangeContext) { Transition transition = stateOf(issue).transition(transitionKey); - if (transition != null && !transition.automatic()) { + if (!transition.automatic()) { functionExecutor.execute(transition.functions(), issue, issueChangeContext); updater.setStatus(issue, transition.to(), issueChangeContext); return true; @@ -177,9 +180,7 @@ public class IssueWorkflow implements Startable { public List outTransitions(Issue issue) { State state = machine.state(issue.status()); - if (state == null) { - throw new IllegalArgumentException("Unknown status: " + issue.status()); - } + checkArgument(state != null, "Unknown status: %s", issue.status()); return state.outManualTransitions(issue); } @@ -197,9 +198,7 @@ public class IssueWorkflow implements Startable { private State stateOf(DefaultIssue issue) { State state = machine.state(issue.status()); - if (state == null) { - throw new IllegalStateException("Unknown status: " + issue.status() + " [issue=" + issue.key() + "]"); - } + checkState(state != null, "Unknown status: %s [issue=%s]", issue.status(), issue.key()); return state; } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/workflow/Transition.java b/server/sonar-server/src/main/java/org/sonar/server/issue/workflow/Transition.java index a0659390c93..a35ba2cdf4b 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/workflow/Transition.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/workflow/Transition.java @@ -19,15 +19,17 @@ */ package org.sonar.server.issue.workflow; -import com.google.common.base.Preconditions; import com.google.common.base.Strings; import com.google.common.collect.Lists; import java.util.Arrays; import java.util.List; +import javax.annotation.CheckForNull; import org.apache.commons.lang.StringUtils; import org.sonar.api.issue.Issue; import org.sonar.api.issue.condition.Condition; +import static com.google.common.base.Preconditions.checkArgument; + public class Transition { private final String key; private final String from; @@ -80,6 +82,7 @@ public class Transition { return true; } + @CheckForNull public String requiredProjectPermission() { return requiredProjectPermission; } @@ -167,10 +170,10 @@ public class Transition { } public Transition build() { - Preconditions.checkArgument(!Strings.isNullOrEmpty(key), "Transition key must be set"); - Preconditions.checkArgument(StringUtils.isAllLowerCase(key), "Transition key must be lower-case"); - Preconditions.checkArgument(!Strings.isNullOrEmpty(from), "Originating status must be set"); - Preconditions.checkArgument(!Strings.isNullOrEmpty(to), "Destination status must be set"); + checkArgument(!Strings.isNullOrEmpty(key), "Transition key must be set"); + checkArgument(StringUtils.isAllLowerCase(key), "Transition key must be lower-case"); + checkArgument(!Strings.isNullOrEmpty(from), "Originating status must be set"); + checkArgument(!Strings.isNullOrEmpty(to), "Destination status must be set"); return new Transition(this); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/DoTransitionAction.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/DoTransitionAction.java index d3281cd4476..0fe6a4bf934 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/DoTransitionAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/DoTransitionAction.java @@ -19,21 +19,39 @@ */ package org.sonar.server.issue.ws; +import java.util.Date; import org.sonar.api.issue.DefaultTransitions; 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.issue.IssueService; +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.issue.IssueDto; +import org.sonar.server.issue.IssueFinder; +import org.sonar.server.issue.IssueUpdater; +import org.sonar.server.issue.TransitionService; +import org.sonar.server.user.UserSession; public class DoTransitionAction implements IssuesWsAction { public static final String ACTION = "do_transition"; - private final IssueService issueService; + private final DbClient dbClient; + private final UserSession userSession; + private final IssueFinder issueFinder; + private final IssueUpdater issueUpdater; + private final TransitionService transitionService; private final OperationResponseWriter responseWriter; - public DoTransitionAction(IssueService issueService, OperationResponseWriter responseWriter) { - this.issueService = issueService; + public DoTransitionAction(DbClient dbClient, UserSession userSession, IssueFinder issueFinder, IssueUpdater issueUpdater, TransitionService transitionService, + OperationResponseWriter responseWriter) { + this.dbClient = dbClient; + this.userSession = userSession; + this.issueFinder = issueFinder; + this.issueUpdater = issueUpdater; + this.transitionService = transitionService; this.responseWriter = responseWriter; } @@ -58,9 +76,20 @@ public class DoTransitionAction implements IssuesWsAction { @Override public void handle(Request request, Response response) throws Exception { - String key = request.mandatoryParam("issue"); - issueService.doTransition(key, request.mandatoryParam("transition")); + userSession.checkLoggedIn(); + String issue = request.mandatoryParam("issue"); + try (DbSession dbSession = dbClient.openSession(false)) { + IssueDto issueDto = issueFinder.getByKey(dbSession, issue); + doTransition(dbSession, issueDto.toDefaultIssue(), request.mandatoryParam("transition")); + responseWriter.write(issue, request, response); + } + } - responseWriter.write(key, request, response); + private void doTransition(DbSession session, DefaultIssue defaultIssue, String transitionKey) { + IssueChangeContext context = IssueChangeContext.createUser(new Date(), userSession.getLogin()); + transitionService.checkTransitionPermission(transitionKey, defaultIssue); + if (transitionService.doTransition(defaultIssue, context, transitionKey)) { + issueUpdater.saveIssue(session, defaultIssue, context, null); + } } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueWsModule.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueWsModule.java index b6df29298d9..60592790f70 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueWsModule.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueWsModule.java @@ -20,11 +20,17 @@ package org.sonar.server.issue.ws; import org.sonar.core.platform.Module; +import org.sonar.server.issue.IssueFinder; +import org.sonar.server.issue.IssueUpdater; +import org.sonar.server.issue.TransitionService; public class IssueWsModule extends Module { @Override protected void configureModule() { add( + IssueUpdater.class, + IssueFinder.class, + TransitionService.class, IssuesWs.class, SearchResponseLoader.class, SearchResponseFormat.class, diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseLoader.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseLoader.java index 1a03dbd8530..4123527f2c9 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseLoader.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseLoader.java @@ -39,7 +39,7 @@ import org.sonar.db.protobuf.DbIssues; import org.sonar.server.es.Facets; import org.sonar.server.issue.ActionService; import org.sonar.server.issue.IssueCommentService; -import org.sonar.server.issue.IssueService; +import org.sonar.server.issue.TransitionService; import org.sonarqube.ws.client.issue.IssuesWsParameters; import static com.google.common.collect.Lists.newArrayList; @@ -55,15 +55,15 @@ import static org.sonar.server.issue.ws.SearchAdditionalField.USERS; public class SearchResponseLoader { private final DbClient dbClient; - private final IssueService issueService; private final ActionService actionService; private final IssueCommentService commentService; + private final TransitionService transitionService; - public SearchResponseLoader(DbClient dbClient, IssueService issueService, ActionService actionService, IssueCommentService commentService) { + public SearchResponseLoader(DbClient dbClient, ActionService actionService, IssueCommentService commentService, TransitionService transitionService) { this.dbClient = dbClient; - this.issueService = issueService; this.actionService = actionService; this.commentService = commentService; + this.transitionService = transitionService; } /** @@ -136,13 +136,13 @@ public class SearchResponseLoader { if (collector.contains(TRANSITIONS)) { // TODO workflow and action engines must not depend on org.sonar.api.issue.Issue but on a generic interface DefaultIssue issue = dto.toDefaultIssue(); - result.addTransitions(issue.key(), issueService.listTransitions(issue)); + result.addTransitions(issue.key(), transitionService.listTransitions(issue)); } } } } - private void completeTotalEffortFromFacet(@Nullable Facets facets, SearchResponseData result) { + private static void completeTotalEffortFromFacet(@Nullable Facets facets, SearchResponseData result) { if (facets != null) { Map effortFacet = facets.get(IssuesWsParameters.FACET_MODE_EFFORT); if (effortFacet != null) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueDbTester.java b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueDbTester.java new file mode 100644 index 00000000000..fc323168295 --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueDbTester.java @@ -0,0 +1,39 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program 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. + * + * This program 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; + +import org.sonar.db.DbTester; +import org.sonar.db.issue.IssueDto; + +public class IssueDbTester { + + private final DbTester db; + + public IssueDbTester(DbTester db) { + this.db = db; + } + + public IssueDto insertIssue(IssueDto issueDto) { + db.getDbClient().issueDao().insert(db.getSession(), issueDto); + db.commit(); + return issueDto; + } +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueFinderTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueFinderTest.java new file mode 100644 index 00000000000..52fcf84f2ce --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueFinderTest.java @@ -0,0 +1,97 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program 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. + * + * This program 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; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.sonar.api.utils.System2; +import org.sonar.db.DbTester; +import org.sonar.db.component.ComponentDbTester; +import org.sonar.db.component.ComponentDto; +import org.sonar.db.issue.IssueDto; +import org.sonar.db.rule.RuleDbTester; +import org.sonar.db.rule.RuleDto; +import org.sonar.server.exceptions.ForbiddenException; +import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.tester.UserSessionRule; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.api.web.UserRole.CODEVIEWER; +import static org.sonar.api.web.UserRole.USER; +import static org.sonar.db.component.ComponentTesting.newFileDto; +import static org.sonar.db.rule.RuleTesting.newRuleDto; +import static org.sonar.server.issue.IssueTesting.newDto; + +public class IssueFinderTest { + + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + @Rule + public UserSessionRule userSession = UserSessionRule.standalone(); + + @Rule + public DbTester db = DbTester.create(System2.INSTANCE); + + private RuleDbTester ruleDbTester = new RuleDbTester(db); + private IssueDbTester issueDbTester = new IssueDbTester(db); + private ComponentDbTester componentDbTester = new ComponentDbTester(db); + + private IssueFinder underTest = new IssueFinder(db.getDbClient(), userSession); + + @Test + public void get_by_issue_key() throws Exception { + IssueDto issueDto = insertIssue(); + userSession.addProjectUuidPermissions(USER, issueDto.getProjectUuid()); + + IssueDto result = underTest.getByKey(db.getSession(), issueDto.getKey()); + + assertThat(result).isNotNull(); + assertThat(result.getKey()).isEqualTo(issueDto.getKey()); + } + + @Test + public void fail_when_issue_key_does_not_exist() throws Exception { + IssueDto issueDto = insertIssue(); + userSession.addProjectUuidPermissions(USER, issueDto.getProjectUuid()); + + expectedException.expect(NotFoundException.class); + expectedException.expectMessage("Issue with key 'UNKNOWN' does not exist"); + underTest.getByKey(db.getSession(), "UNKNOWN"); + } + + @Test + public void fail_when_not_enough_permission() throws Exception { + IssueDto issueDto = insertIssue(); + userSession.addProjectUuidPermissions(CODEVIEWER, issueDto.getProjectUuid()); + + expectedException.expect(ForbiddenException.class); + underTest.getByKey(db.getSession(), issueDto.getKey()); + } + + private IssueDto insertIssue() { + RuleDto rule = ruleDbTester.insertRule(newRuleDto()); + ComponentDto project = componentDbTester.insertProject(); + ComponentDto file = componentDbTester.insertComponent(newFileDto(project)); + return issueDbTester.insertIssue(newDto(rule, file, project)); + } +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceMediumTest.java index c2865d673bb..82185bb67fb 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceMediumTest.java @@ -21,13 +21,11 @@ package org.sonar.server.issue; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; -import java.util.List; import org.junit.After; import org.junit.Before; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; -import org.sonar.api.issue.DefaultTransitions; import org.sonar.api.issue.Issue; import org.sonar.api.rule.Severity; import org.sonar.api.rules.RuleType; @@ -47,7 +45,6 @@ import org.sonar.db.user.UserDto; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.issue.index.IssueIndex; import org.sonar.server.issue.index.IssueIndexer; -import org.sonar.server.issue.workflow.Transition; import org.sonar.server.permission.GroupPermissionChange; import org.sonar.server.permission.PermissionChange; import org.sonar.server.permission.PermissionUpdater; @@ -105,34 +102,6 @@ public class IssueServiceMediumTest { assertThat(service.getByKey(issue.getKey())).isNotNull(); } - @Test - public void list_transitions() { - RuleDto rule = newRule(); - ComponentDto project = newProject(); - ComponentDto file = newFile(project); - IssueDto issue = saveIssue(IssueTesting.newDto(rule, file, project).setStatus(Issue.STATUS_RESOLVED).setResolution(Issue.RESOLUTION_FALSE_POSITIVE)); - - List result = service.listTransitions(issue.toDefaultIssue()); - assertThat(result).hasSize(1); - assertThat(result.get(0).key()).isEqualTo("reopen"); - } - - @Test - public void do_transition() { - RuleDto rule = newRule(); - ComponentDto project = newProject(); - ComponentDto file = newFile(project); - userSessionRule.login("john"); - - IssueDto issue = saveIssue(IssueTesting.newDto(rule, file, project).setStatus(Issue.STATUS_OPEN)); - - assertThat(IssueIndex.getByKey(issue.getKey()).status()).isEqualTo(Issue.STATUS_OPEN); - - service.doTransition(issue.getKey(), DefaultTransitions.CONFIRM); - - assertThat(IssueIndex.getByKey(issue.getKey()).status()).isEqualTo(Issue.STATUS_CONFIRMED); - } - @Test public void assign() { RuleDto rule = newRule(); diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueUpdaterTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueUpdaterTest.java new file mode 100644 index 00000000000..33fabad44f8 --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueUpdaterTest.java @@ -0,0 +1,139 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program 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. + * + * This program 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; + +import java.util.Date; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.mockito.ArgumentCaptor; +import org.sonar.api.config.MapSettings; +import org.sonar.api.rule.RuleStatus; +import org.sonar.api.utils.System2; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.IssueChangeContext; +import org.sonar.db.DbClient; +import org.sonar.db.DbTester; +import org.sonar.db.component.ComponentDbTester; +import org.sonar.db.component.ComponentDto; +import org.sonar.db.issue.IssueDto; +import org.sonar.db.rule.RuleDbTester; +import org.sonar.db.rule.RuleDto; +import org.sonar.server.es.EsTester; +import org.sonar.server.issue.index.IssueIndexDefinition; +import org.sonar.server.issue.index.IssueIndexer; +import org.sonar.server.issue.notification.IssueChangeNotification; +import org.sonar.server.notification.NotificationManager; +import org.sonar.server.rule.DefaultRuleFinder; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.sonar.api.rule.Severity.BLOCKER; +import static org.sonar.api.rule.Severity.MAJOR; +import static org.sonar.db.component.ComponentTesting.newFileDto; +import static org.sonar.db.rule.RuleTesting.newRuleDto; +import static org.sonar.server.issue.IssueTesting.newDto; + +public class IssueUpdaterTest { + + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + @Rule + public DbTester dbTester = DbTester.create(System2.INSTANCE); + + @Rule + public EsTester esTester = new EsTester(new IssueIndexDefinition(new MapSettings())); + + private DbClient dbClient = dbTester.getDbClient(); + + private RuleDbTester ruleDbTester = new RuleDbTester(dbTester); + private IssueDbTester issueDbTester = new IssueDbTester(dbTester); + private ComponentDbTester componentDbTester = new ComponentDbTester(dbTester); + private IssueFieldsSetter issueFieldsSetter = new IssueFieldsSetter(); + private NotificationManager notificationManager = mock(NotificationManager.class); + private ArgumentCaptor notificationArgumentCaptor = ArgumentCaptor.forClass(IssueChangeNotification.class); + + private IssueUpdater underTest = new IssueUpdater(dbClient, + new ServerIssueStorage(new DefaultRuleFinder(dbClient), dbClient, new IssueIndexer(System2.INSTANCE, dbClient, esTester.client())), + notificationManager); + + @Test + public void update_issue() throws Exception { + DefaultIssue issue = issueDbTester.insertIssue(newIssue().setSeverity(MAJOR)).toDefaultIssue(); + IssueChangeContext context = IssueChangeContext.createUser(new Date(), "john"); + issueFieldsSetter.setSeverity(issue, BLOCKER, context); + + underTest.saveIssue(dbTester.getSession(), issue, context, null); + + IssueDto issueReloaded = dbClient.issueDao().selectByKey(dbTester.getSession(), issue.key()).get(); + assertThat(issueReloaded.getSeverity()).isEqualTo(BLOCKER); + } + + @Test + public void verify_notification() throws Exception { + RuleDto rule = ruleDbTester.insertRule(newRuleDto()); + ComponentDto project = componentDbTester.insertProject(); + ComponentDto file = componentDbTester.insertComponent(newFileDto(project)); + DefaultIssue issue = issueDbTester.insertIssue(newDto(rule, file, project)).setSeverity(MAJOR).toDefaultIssue(); + IssueChangeContext context = IssueChangeContext.createUser(new Date(), "john"); + issueFieldsSetter.setSeverity(issue, BLOCKER, context); + + underTest.saveIssue(dbTester.getSession(), issue, context, "increase severity"); + + verify(notificationManager).scheduleForSending(notificationArgumentCaptor.capture()); + IssueChangeNotification issueChangeNotification = notificationArgumentCaptor.getValue(); + assertThat(issueChangeNotification.getFieldValue("key")).isEqualTo(issue.key()); + assertThat(issueChangeNotification.getFieldValue("old.severity")).isEqualTo(MAJOR); + assertThat(issueChangeNotification.getFieldValue("new.severity")).isEqualTo(BLOCKER); + assertThat(issueChangeNotification.getFieldValue("componentKey")).isEqualTo(file.key()); + assertThat(issueChangeNotification.getFieldValue("componentName")).isEqualTo(file.longName()); + assertThat(issueChangeNotification.getFieldValue("projectKey")).isEqualTo(project.key()); + assertThat(issueChangeNotification.getFieldValue("projectName")).isEqualTo(project.name()); + assertThat(issueChangeNotification.getFieldValue("ruleName")).isEqualTo(rule.getName()); + assertThat(issueChangeNotification.getFieldValue("changeAuthor")).isEqualTo("john"); + assertThat(issueChangeNotification.getFieldValue("comment")).isEqualTo("increase severity"); + } + + @Test + public void verify_notification_when_issue_is_linked_on_removed_rule() throws Exception { + RuleDto rule = ruleDbTester.insertRule(newRuleDto().setStatus(RuleStatus.REMOVED)); + ComponentDto project = componentDbTester.insertProject(); + ComponentDto file = componentDbTester.insertComponent(newFileDto(project)); + DefaultIssue issue = issueDbTester.insertIssue(newDto(rule, file, project)).setSeverity(MAJOR).toDefaultIssue(); + IssueChangeContext context = IssueChangeContext.createUser(new Date(), "john"); + issueFieldsSetter.setSeverity(issue, BLOCKER, context); + + underTest.saveIssue(dbTester.getSession(), issue, context, null); + + verify(notificationManager).scheduleForSending(notificationArgumentCaptor.capture()); + assertThat(notificationArgumentCaptor.getValue().getFieldValue("ruleName")).isNull(); + } + + private IssueDto newIssue() { + RuleDto rule = ruleDbTester.insertRule(newRuleDto()); + ComponentDto project = componentDbTester.insertProject(); + ComponentDto file = componentDbTester.insertComponent(newFileDto(project)); + return newDto(rule, file, project); + } + +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/TransitionActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/TransitionActionTest.java index 2ea9787433b..a8c082badbe 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/TransitionActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/TransitionActionTest.java @@ -19,88 +19,90 @@ */ package org.sonar.server.issue; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; -import java.util.Map; +import java.util.Date; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.sonar.api.issue.Issue; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.IssueChangeContext; +import org.sonar.db.component.ComponentDto; +import org.sonar.db.component.ComponentTesting; +import org.sonar.db.issue.IssueDto; +import org.sonar.db.rule.RuleDto; +import org.sonar.server.issue.workflow.FunctionExecutor; import org.sonar.server.issue.workflow.IssueWorkflow; -import org.sonar.server.issue.workflow.Transition; import org.sonar.server.tester.UserSessionRule; -import static com.google.common.collect.Lists.newArrayList; -import static com.google.common.collect.Maps.newHashMap; +import static java.util.Collections.emptyList; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.Assert.fail; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; +import static org.sonar.api.web.UserRole.ISSUE_ADMIN; +import static org.sonar.db.component.ComponentTesting.newFileDto; +import static org.sonar.db.rule.RuleTesting.newRuleDto; +import static org.sonar.server.issue.IssueTesting.newDto; public class TransitionActionTest { + + @Rule + public ExpectedException expectedException = ExpectedException.none(); + @Rule - public UserSessionRule userSessionRule = UserSessionRule.standalone(); + public UserSessionRule userSession = UserSessionRule.standalone(); - private TransitionAction action; + private IssueFieldsSetter updater = new IssueFieldsSetter(); + private IssueWorkflow workflow = new IssueWorkflow(new FunctionExecutor(updater), updater); + private TransitionService transitionService = new TransitionService(userSession, workflow); - private IssueWorkflow workflow = mock(IssueWorkflow.class); + private Action.Context context = mock(Action.Context.class); + private DefaultIssue issue = newIssue().toDefaultIssue(); + + private TransitionAction action = new TransitionAction(transitionService); @Before - public void before() { - action = new TransitionAction(workflow, userSessionRule); + public void setUp() throws Exception { + workflow.start(); + when(context.issue()).thenReturn(issue); + when(context.issueChangeContext()).thenReturn(IssueChangeContext.createUser(new Date(), "john")); } @Test - public void should_execute() { - String transition = "reopen"; - Map properties = newHashMap(); - properties.put("transition", transition); - DefaultIssue issue = mock(DefaultIssue.class); - - Action.Context context = mock(Action.Context.class); - when(context.issue()).thenReturn(issue); + public void execute() { + userSession.addProjectUuidPermissions(ISSUE_ADMIN, issue.projectUuid()); + issue.setStatus(Issue.STATUS_RESOLVED); + issue.setResolution(Issue.RESOLUTION_FIXED); - when(workflow.outTransitions(context.issue())).thenReturn(newArrayList(Transition.create(transition, "REOPEN", "CONFIRMED"))); + action.execute(ImmutableMap.of("transition", "reopen"), context); - action.execute(properties, context); - verify(workflow).doTransition(eq(issue), eq(transition), any(IssueChangeContext.class)); + assertThat(issue.status()).isEqualTo(Issue.STATUS_REOPENED); + assertThat(issue.resolution()).isNull(); } @Test - public void should_not_execute_if_transition_is_not_available() { - String transition = "reopen"; - Map properties = newHashMap(); - properties.put("transition", transition); - DefaultIssue issue = mock(DefaultIssue.class); + public void does_not_execute_if_transition_is_not_available() { + userSession.addProjectUuidPermissions(ISSUE_ADMIN, issue.projectUuid()); + issue.setStatus(Issue.STATUS_CLOSED); - Action.Context context = mock(Action.Context.class); - when(context.issue()).thenReturn(issue); + action.execute(ImmutableMap.of("transition", "reopen"), context); - // Do not contain reopen, transition is not possible - when(workflow.outTransitions(context.issue())).thenReturn(newArrayList(Transition.create("resolve", "OPEN", "RESOLVED"))); + assertThat(issue.status()).isEqualTo(Issue.STATUS_CLOSED); + } - assertThat(action.execute(properties, context)).isFalse(); - verify(workflow, never()).doTransition(eq(issue), eq(transition), any(IssueChangeContext.class)); + @Test + public void test_verify() throws Exception { + assertThat(action.verify(ImmutableMap.of("transition", "reopen"), emptyList(), userSession)).isTrue(); + assertThat(action.verify(ImmutableMap.of("transition", "close"), emptyList(), userSession)).isTrue(); } @Test - public void should_verify_fail_if_parameter_not_found() { - String transition = "reopen"; - Map properties = newHashMap(); - properties.put("unknwown", transition); - try { - action.verify(properties, Lists.newArrayList(), userSessionRule); - fail(); - } catch (Exception e) { - assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("Missing parameter : 'transition'"); - } - verifyZeroInteractions(workflow); + public void fail_to_verify_when_parameter_not_found() { + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Missing parameter : 'transition'"); + action.verify(ImmutableMap.of("unknwown", "reopen"), Lists.newArrayList(), userSession); } @Test @@ -109,4 +111,11 @@ public class TransitionActionTest { assertThat(action.supports(new DefaultIssue().setResolution(Issue.RESOLUTION_FIXED))).isTrue(); } + private IssueDto newIssue() { + RuleDto rule = newRuleDto().setId(10); + ComponentDto project = ComponentTesting.newProjectDto(); + ComponentDto file = (newFileDto(project)); + return newDto(rule, file, project); + } + } diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/TransitionServiceTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/TransitionServiceTest.java new file mode 100644 index 00000000000..0132d1ba50d --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/TransitionServiceTest.java @@ -0,0 +1,97 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program 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. + * + * This program 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; + +import java.util.Date; +import java.util.List; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.sonar.api.web.UserRole; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.IssueChangeContext; +import org.sonar.db.component.ComponentDto; +import org.sonar.db.component.ComponentTesting; +import org.sonar.db.issue.IssueDto; +import org.sonar.db.rule.RuleDto; +import org.sonar.server.issue.workflow.FunctionExecutor; +import org.sonar.server.issue.workflow.IssueWorkflow; +import org.sonar.server.issue.workflow.Transition; +import org.sonar.server.tester.UserSessionRule; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.api.issue.Issue.STATUS_CONFIRMED; +import static org.sonar.api.issue.Issue.STATUS_OPEN; +import static org.sonar.db.component.ComponentTesting.newFileDto; +import static org.sonar.db.rule.RuleTesting.newRuleDto; +import static org.sonar.server.issue.IssueTesting.newDto; + +public class TransitionServiceTest { + + @Rule + public UserSessionRule userSession = UserSessionRule.standalone(); + + private IssueFieldsSetter updater = new IssueFieldsSetter(); + private IssueWorkflow workflow = new IssueWorkflow(new FunctionExecutor(updater), updater); + + private TransitionService underTest = new TransitionService(userSession, workflow); + + @Before + public void setUp() throws Exception { + workflow.start(); + } + + @Test + public void list_transitions() throws Exception { + IssueDto issue = newIssue().setStatus(STATUS_OPEN).setResolution(null); + userSession.addProjectUuidPermissions(UserRole.ISSUE_ADMIN, issue.getProjectUuid()); + + List result = underTest.listTransitions(issue.toDefaultIssue()); + + assertThat(result).extracting(Transition::key).containsOnly("confirm", "resolve", "falsepositive", "wontfix"); + } + + @Test + public void list_transitions_returns_only_transitions_that_do_not_requires_issue_admin_permission() throws Exception { + IssueDto issue = newIssue().setStatus(STATUS_OPEN).setResolution(null); + + List result = underTest.listTransitions(issue.toDefaultIssue()); + + assertThat(result).extracting(Transition::key).containsOnly("confirm", "resolve"); + } + + @Test + public void do_transition() { + DefaultIssue issue = newIssue().setStatus(STATUS_OPEN).setResolution(null).toDefaultIssue(); + + boolean result = underTest.doTransition(issue, IssueChangeContext.createUser(new Date(), "john"), "confirm"); + + assertThat(result).isTrue(); + assertThat(issue.status()).isEqualTo(STATUS_CONFIRMED); + } + + private IssueDto newIssue() { + RuleDto rule = newRuleDto().setId(10); + ComponentDto project = ComponentTesting.newProjectDto(); + ComponentDto file = (newFileDto(project)); + return newDto(rule, file, project); + } +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/workflow/TransitionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/workflow/TransitionTest.java index a8535a0c09c..aebcd2c31fe 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/workflow/TransitionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/workflow/TransitionTest.java @@ -21,6 +21,7 @@ package org.sonar.server.issue.workflow; import org.junit.Test; import org.sonar.api.issue.condition.Condition; +import org.sonar.api.web.UserRole; import org.sonar.core.issue.DefaultIssue; import static org.assertj.core.api.Assertions.assertThat; @@ -41,6 +42,7 @@ public class TransitionTest { .from("OPEN").to("CLOSED") .conditions(condition1, condition2) .functions(function1, function2) + .requiredProjectPermission(UserRole.ISSUE_ADMIN) .build(); assertThat(transition.key()).isEqualTo("close"); assertThat(transition.from()).isEqualTo("OPEN"); @@ -48,6 +50,7 @@ public class TransitionTest { assertThat(transition.conditions()).containsOnly(condition1, condition2); assertThat(transition.functions()).containsOnly(function1, function2); assertThat(transition.automatic()).isFalse(); + assertThat(transition.requiredProjectPermission()).isEqualTo(UserRole.ISSUE_ADMIN); } @Test @@ -60,6 +63,7 @@ public class TransitionTest { assertThat(transition.to()).isEqualTo("CLOSED"); assertThat(transition.conditions()).isEmpty(); assertThat(transition.functions()).isEmpty(); + assertThat(transition.requiredProjectPermission()).isNull(); } @Test diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/DoTransitionActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/DoTransitionActionTest.java index 8e41969b076..92cc06a3afc 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/DoTransitionActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/DoTransitionActionTest.java @@ -19,34 +19,170 @@ */ package org.sonar.server.issue.ws; +import javax.annotation.Nullable; +import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.sonar.api.config.MapSettings; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; -import org.sonar.server.issue.IssueService; +import org.sonar.api.utils.System2; +import org.sonar.db.DbClient; +import org.sonar.db.DbTester; +import org.sonar.db.component.ComponentDbTester; +import org.sonar.db.component.ComponentDto; +import org.sonar.db.issue.IssueDto; +import org.sonar.db.rule.RuleDbTester; +import org.sonar.db.rule.RuleDto; +import org.sonar.server.es.EsTester; +import org.sonar.server.exceptions.ForbiddenException; +import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.exceptions.UnauthorizedException; +import org.sonar.server.issue.IssueDbTester; +import org.sonar.server.issue.IssueFieldsSetter; +import org.sonar.server.issue.IssueFinder; +import org.sonar.server.issue.IssueUpdater; +import org.sonar.server.issue.ServerIssueStorage; +import org.sonar.server.issue.TransitionService; +import org.sonar.server.issue.index.IssueIndexDefinition; +import org.sonar.server.issue.index.IssueIndexer; +import org.sonar.server.issue.workflow.FunctionExecutor; +import org.sonar.server.issue.workflow.IssueWorkflow; +import org.sonar.server.notification.NotificationManager; +import org.sonar.server.rule.DefaultRuleFinder; +import org.sonar.server.tester.UserSessionRule; +import org.sonar.server.ws.TestRequest; +import org.sonar.server.ws.TestResponse; import org.sonar.server.ws.WsAction; import org.sonar.server.ws.WsActionTester; +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.sonar.api.issue.Issue.STATUS_CONFIRMED; +import static org.sonar.api.issue.Issue.STATUS_OPEN; +import static org.sonar.api.web.UserRole.CODEVIEWER; +import static org.sonar.api.web.UserRole.USER; +import static org.sonar.db.component.ComponentTesting.newFileDto; +import static org.sonar.db.rule.RuleTesting.newRuleDto; +import static org.sonar.server.issue.IssueTesting.newDto; public class DoTransitionActionTest { - IssueService issueService = mock(IssueService.class); - OperationResponseWriter responseWriter = mock(OperationResponseWriter.class); - WsAction underTest = new DoTransitionAction(issueService, responseWriter); - WsActionTester tester = new WsActionTester(underTest); + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + @Rule + public DbTester dbTester = DbTester.create(System2.INSTANCE); + + @Rule + public EsTester esTester = new EsTester(new IssueIndexDefinition(new MapSettings())); + + @Rule + public UserSessionRule userSession = UserSessionRule.standalone(); + + private DbClient dbClient = dbTester.getDbClient(); + + private RuleDbTester ruleDbTester = new RuleDbTester(dbTester); + private IssueDbTester issueDbTester = new IssueDbTester(dbTester); + private ComponentDbTester componentDbTester = new ComponentDbTester(dbTester); + + private IssueFieldsSetter updater = new IssueFieldsSetter(); + private IssueWorkflow workflow = new IssueWorkflow(new FunctionExecutor(updater), updater); + private TransitionService transitionService = new TransitionService(userSession, workflow); + private OperationResponseWriter responseWriter = mock(OperationResponseWriter.class); + private IssueUpdater issueUpdater = new IssueUpdater(dbClient, + new ServerIssueStorage(new DefaultRuleFinder(dbClient), dbClient, new IssueIndexer(System2.INSTANCE, dbClient, esTester.client())), mock(NotificationManager.class)); + + private WsAction underTest = new DoTransitionAction(dbClient, userSession, new IssueFinder(dbClient, userSession), issueUpdater, transitionService, responseWriter); + private WsActionTester tester = new WsActionTester(underTest); + + @Before + public void setUp() throws Exception { + workflow.start(); + } @Test public void do_transition() throws Exception { - tester.newRequest() - .setParam("issue", "ABC") - .setParam("transition", "confirm") - .execute(); + IssueDto issueDto = issueDbTester.insertIssue(newIssue().setStatus(STATUS_OPEN).setResolution(null)); + userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid()); + + call(issueDto.getKey(), "confirm"); + + verify(responseWriter).write(eq(issueDto.getKey()), any(Request.class), any(Response.class)); + IssueDto issueReloaded = dbClient.issueDao().selectByKey(dbTester.getSession(), issueDto.getKey()).get(); + assertThat(issueReloaded.getStatus()).isEqualTo(STATUS_CONFIRMED); + } + + @Test + public void fail_if_issue_does_not_exist() throws Exception { + userSession.login("john"); + + expectedException.expect(NotFoundException.class); + call("UNKNOWN", "confirm"); + } + + @Test + public void fail_if_no_issue_param() throws Exception { + userSession.login("john"); + + expectedException.expect(IllegalArgumentException.class); + call(null, "confirm"); + } + + @Test + public void fail_if_no_transition_param() throws Exception { + IssueDto issueDto = issueDbTester.insertIssue(newIssue().setStatus(STATUS_OPEN).setResolution(null)); + userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid()); + + expectedException.expect(IllegalArgumentException.class); + call(issueDto.getKey(), null); + } + + @Test + public void fail_if_not_enough_permission_to_access_issue() throws Exception { + IssueDto issueDto = issueDbTester.insertIssue(newIssue().setStatus(STATUS_OPEN).setResolution(null)); + userSession.login("john").addProjectUuidPermissions(CODEVIEWER, issueDto.getProjectUuid()); + + expectedException.expect(ForbiddenException.class); + call(issueDto.getKey(), "confirm"); + } + + @Test + public void fail_if_not_enough_permission_to_apply_transition() throws Exception { + IssueDto issueDto = issueDbTester.insertIssue(newIssue().setStatus(STATUS_OPEN).setResolution(null)); + userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid()); + + // False-positive transition is requiring issue admin permission + expectedException.expect(ForbiddenException.class); + call(issueDto.getKey(), "falsepositive"); + } + + @Test + public void fail_if_not_authenticated() throws Exception { + expectedException.expect(UnauthorizedException.class); + call("ISSUE_KEY", "confirm"); + } + + private TestResponse call(@Nullable String issueKey, @Nullable String transition) { + TestRequest request = tester.newRequest(); + if (issueKey != null) { + request.setParam("issue", issueKey); + } + if (transition != null) { + request.setParam("transition", transition); + } + return request.execute(); + } - verify(issueService).doTransition("ABC", "confirm"); - verify(responseWriter).write(eq("ABC"), any(Request.class), any(Response.class)); + private IssueDto newIssue() { + RuleDto rule = ruleDbTester.insertRule(newRuleDto()); + ComponentDto project = componentDbTester.insertProject(); + ComponentDto file = componentDbTester.insertComponent(newFileDto(project)); + return newDto(rule, file, project); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/IssueWsModuleTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/IssueWsModuleTest.java index 5021d11e4ce..fe31bf90df4 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/IssueWsModuleTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/IssueWsModuleTest.java @@ -29,6 +29,6 @@ public class IssueWsModuleTest { public void verify_count_of_added_components() { ComponentContainer container = new ComponentContainer(); new IssueWsModule().configure(container); - assertThat(container.size()).isEqualTo(2 + 13); + assertThat(container.size()).isEqualTo(2 + 16); } } diff --git a/sonar-db/src/main/java/org/sonar/db/issue/IssueDao.java b/sonar-db/src/main/java/org/sonar/db/issue/IssueDao.java index 0a4526f36db..686d3852548 100644 --- a/sonar-db/src/main/java/org/sonar/db/issue/IssueDao.java +++ b/sonar-db/src/main/java/org/sonar/db/issue/IssueDao.java @@ -20,7 +20,6 @@ package org.sonar.db.issue; import com.google.common.base.Function; -import com.google.common.base.Optional; import com.google.common.base.Predicates; import java.util.Collection; import java.util.HashMap; @@ -38,12 +37,12 @@ import static org.sonar.db.DatabaseUtils.executeLargeInputs; public class IssueDao implements Dao { - public Optional selectByKey(DbSession session, String key) { - return Optional.fromNullable(mapper(session).selectByKey(key)); + public java.util.Optional selectByKey(DbSession session, String key) { + return java.util.Optional.ofNullable(mapper(session).selectByKey(key)); } public IssueDto selectOrFailByKey(DbSession session, String key) { - Optional issue = selectByKey(session, key); + java.util.Optional issue = selectByKey(session, key); if (!issue.isPresent()) { throw new RowNotFoundException(String.format("Issue with key '%s' does not exist", key)); } diff --git a/sonar-db/src/test/java/org/sonar/db/rule/RuleDbTester.java b/sonar-db/src/test/java/org/sonar/db/rule/RuleDbTester.java new file mode 100644 index 00000000000..c5d02704b86 --- /dev/null +++ b/sonar-db/src/test/java/org/sonar/db/rule/RuleDbTester.java @@ -0,0 +1,38 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program 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. + * + * This program 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.db.rule; + +import org.sonar.db.DbTester; + +public class RuleDbTester { + + private final DbTester db; + + public RuleDbTester(DbTester db) { + this.db = db; + } + + public RuleDto insertRule(RuleDto ruleDto) { + db.getDbClient().ruleDao().insert(db.getSession(), ruleDto); + db.commit(); + return ruleDto; + } +} diff --git a/sonar-db/src/test/java/org/sonar/db/rule/RuleTesting.java b/sonar-db/src/test/java/org/sonar/db/rule/RuleTesting.java index 9bdba618a34..ce9320d6aa1 100644 --- a/sonar-db/src/test/java/org/sonar/db/rule/RuleTesting.java +++ b/sonar-db/src/test/java/org/sonar/db/rule/RuleTesting.java @@ -28,6 +28,8 @@ import org.sonar.api.rule.Severity; import org.sonar.api.rules.RuleType; import org.sonar.db.rule.RuleDto.Format; +import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; + /** * Utility class for tests involving rules */ @@ -86,6 +88,10 @@ public class RuleTesting { .setUpdatedAt(new Date().getTime()); } + public static RuleDto newRuleDto() { + return newDto(RuleKey.of(randomAlphanumeric(30), randomAlphanumeric(30))); + } + public static RuleDto newTemplateRule(RuleKey ruleKey) { return newDto(ruleKey) .setIsTemplate(true); -- 2.39.5