From 76c0c2a9d046073770e6cc1142480966a7da75b3 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Wed, 20 Dec 2017 13:46:48 +0100 Subject: [PATCH] SONAR-10085 rename IssueChangeTrigger to QGChangeEventFactory --- .../server/issue/ws/BulkChangeAction.java | 30 ++-- .../server/issue/ws/DoTransitionAction.java | 26 +-- .../sonar/server/issue/ws/IssueWsModule.java | 4 +- .../sonar/server/issue/ws/SetTypeAction.java | 25 +-- ...Trigger.java => QGChangeEventFactory.java} | 4 +- ...mpl.java => QGChangeEventFactoryImpl.java} | 28 ++-- .../changeevent/QGChangeEventListeners.java | 2 +- .../QGChangeEventListenersImpl.java | 7 +- .../server/issue/ws/BulkChangeActionTest.java | 148 +++++++++++++----- .../issue/ws/DoTransitionActionTest.java | 44 +++--- .../server/issue/ws/SetTypeActionTest.java | 35 +++-- .../changeevent/IssueChangeTriggerTest.java | 88 ----------- ...java => QGChangeEventFactoryImplTest.java} | 70 ++++----- .../changeevent/QGChangeEventFactoryTest.java | 88 +++++++++++ .../QGChangeEventListenersImplTest.java | 61 ++++++-- 15 files changed, 386 insertions(+), 274 deletions(-) rename server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/{IssueChangeTrigger.java => QGChangeEventFactory.java} (96%) rename server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/{IssueChangeTriggerImpl.java => QGChangeEventFactoryImpl.java} (86%) delete mode 100644 server/sonar-server/src/test/java/org/sonar/server/qualitygate/changeevent/IssueChangeTriggerTest.java rename server/sonar-server/src/test/java/org/sonar/server/qualitygate/changeevent/{IssueChangeTriggerImplTest.java => QGChangeEventFactoryImplTest.java} (88%) create mode 100644 server/sonar-server/src/test/java/org/sonar/server/qualitygate/changeevent/QGChangeEventFactoryTest.java diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/BulkChangeAction.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/BulkChangeAction.java index 032cc02cd50..1836b75e7a6 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/BulkChangeAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/BulkChangeAction.java @@ -60,7 +60,9 @@ import org.sonar.server.issue.SetTypeAction; import org.sonar.server.issue.TransitionAction; import org.sonar.server.issue.notification.IssueChangeNotification; import org.sonar.server.notification.NotificationManager; -import org.sonar.server.qualitygate.changeevent.IssueChangeTrigger; +import org.sonar.server.qualitygate.changeevent.QGChangeEvent; +import org.sonar.server.qualitygate.changeevent.QGChangeEventFactory; +import org.sonar.server.qualitygate.changeevent.QGChangeEventListeners; import org.sonar.server.user.UserSession; import org.sonarqube.ws.Issues; @@ -109,17 +111,20 @@ public class BulkChangeAction implements IssuesWsAction { private final IssueStorage issueStorage; private final NotificationManager notificationService; private final List actions; - private final IssueChangeTrigger issueChangeTrigger; + private final QGChangeEventFactory qgChangeEventFactory; + private final QGChangeEventListeners qgChangeEventListeners; - public BulkChangeAction(System2 system2, UserSession userSession, DbClient dbClient, IssueStorage issueStorage, NotificationManager notificationService, List actions, - IssueChangeTrigger issueChangeTrigger) { + public BulkChangeAction(System2 system2, UserSession userSession, DbClient dbClient, IssueStorage issueStorage, + NotificationManager notificationService, List actions, + QGChangeEventFactory qgChangeEventFactory, QGChangeEventListeners qgChangeEventListeners) { this.system2 = system2; this.userSession = userSession; this.dbClient = dbClient; this.issueStorage = issueStorage; this.notificationService = notificationService; this.actions = actions; - this.issueChangeTrigger = issueChangeTrigger; + this.qgChangeEventFactory = qgChangeEventFactory; + this.qgChangeEventListeners = qgChangeEventListeners; } @Override @@ -206,17 +211,18 @@ public class BulkChangeAction implements IssuesWsAction { issueStorage.save(items); items.forEach(sendNotification(issueChangeContext, bulkChangeData)); buildWebhookIssueChange(bulkChangeData.propertiesByActions) - .ifPresent(issueChange -> issueChangeTrigger.onChange( - new IssueChangeTrigger.IssueChangeData( + .ifPresent(issueChange -> { + QGChangeEventFactory.IssueChangeData issueChangeData = new QGChangeEventFactory.IssueChangeData( bulkChangeData.issues.stream().filter(i -> result.success.contains(i.key())).collect(MoreCollectors.toList()), - copyOf(bulkChangeData.componentsByUuid.values())), - issueChange, - issueChangeContext)); + copyOf(bulkChangeData.componentsByUuid.values())); + List qgChangeEvents = qgChangeEventFactory.from(issueChangeData, issueChange, issueChangeContext); + qgChangeEventListeners.broadcastOnIssueChange(issueChangeData, qgChangeEvents); + }); return result; }; } - private static Optional buildWebhookIssueChange(Map> propertiesByActions) { + private static Optional buildWebhookIssueChange(Map> propertiesByActions) { RuleType ruleType = Optional.ofNullable(propertiesByActions.get(SetTypeAction.SET_TYPE_KEY)) .map(t -> (String) t.get(SetTypeAction.TYPE_PARAMETER)) .map(RuleType::valueOf) @@ -227,7 +233,7 @@ public class BulkChangeAction implements IssuesWsAction { if (ruleType == null && transitionKey == null) { return Optional.empty(); } - return Optional.of(new IssueChangeTrigger.IssueChange(ruleType, transitionKey)); + return Optional.of(new QGChangeEventFactory.IssueChange(ruleType, transitionKey)); } private static Predicate bulkChange(IssueChangeContext issueChangeContext, BulkChangeData bulkChangeData, BulkChangeResult result) { 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 43a0d0056a7..2f409d7b72b 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 @@ -21,6 +21,7 @@ package org.sonar.server.issue.ws; import com.google.common.io.Resources; import java.util.Date; +import java.util.List; import org.sonar.api.issue.DefaultTransitions; import org.sonar.api.server.ws.Change; import org.sonar.api.server.ws.Request; @@ -30,17 +31,19 @@ import org.sonar.api.utils.System2; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.IssueChangeContext; import org.sonar.core.util.Uuids; -import org.sonar.core.util.stream.MoreCollectors; 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.qualitygate.changeevent.IssueChangeTrigger; +import org.sonar.server.qualitygate.changeevent.QGChangeEvent; +import org.sonar.server.qualitygate.changeevent.QGChangeEventFactory; +import org.sonar.server.qualitygate.changeevent.QGChangeEventListeners; import org.sonar.server.user.UserSession; import static com.google.common.collect.ImmutableList.copyOf; +import static org.sonar.core.util.stream.MoreCollectors.toList; import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_DO_TRANSITION; import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_ISSUE; import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_TRANSITION; @@ -54,10 +57,11 @@ public class DoTransitionAction implements IssuesWsAction { private final TransitionService transitionService; private final OperationResponseWriter responseWriter; private final System2 system2; - private final IssueChangeTrigger issueChangeTrigger; + private final QGChangeEventFactory qgChangeEventFactory; + private final QGChangeEventListeners qgChangeEventListeners; public DoTransitionAction(DbClient dbClient, UserSession userSession, IssueFinder issueFinder, IssueUpdater issueUpdater, TransitionService transitionService, - OperationResponseWriter responseWriter, System2 system2, IssueChangeTrigger issueChangeTrigger) { + OperationResponseWriter responseWriter, System2 system2, QGChangeEventFactory qgChangeEventFactory, QGChangeEventListeners qgChangeEventListeners) { this.dbClient = dbClient; this.userSession = userSession; this.issueFinder = issueFinder; @@ -65,7 +69,8 @@ public class DoTransitionAction implements IssuesWsAction { this.transitionService = transitionService; this.responseWriter = responseWriter; this.system2 = system2; - this.issueChangeTrigger = issueChangeTrigger; + this.qgChangeEventFactory = qgChangeEventFactory; + this.qgChangeEventListeners = qgChangeEventListeners; } @Override @@ -108,12 +113,11 @@ public class DoTransitionAction implements IssuesWsAction { transitionService.checkTransitionPermission(transitionKey, defaultIssue); if (transitionService.doTransition(defaultIssue, context, transitionKey)) { SearchResponseData searchResponseData = issueUpdater.saveIssueAndPreloadSearchResponseData(session, defaultIssue, context, null); - issueChangeTrigger.onChange( - new IssueChangeTrigger.IssueChangeData( - searchResponseData.getIssues().stream().map(IssueDto::toDefaultIssue).collect(MoreCollectors.toList(searchResponseData.getIssues().size())), - copyOf(searchResponseData.getComponents())), - new IssueChangeTrigger.IssueChange(transitionKey), - context); + QGChangeEventFactory.IssueChangeData issueChangeData = new QGChangeEventFactory.IssueChangeData( + searchResponseData.getIssues().stream().map(IssueDto::toDefaultIssue).collect(toList(searchResponseData.getIssues().size())), + copyOf(searchResponseData.getComponents())); + List qgChangeEvents = qgChangeEventFactory.from(issueChangeData, new QGChangeEventFactory.IssueChange(transitionKey), context); + qgChangeEventListeners.broadcastOnIssueChange(issueChangeData, qgChangeEvents); return searchResponseData; } return new SearchResponseData(issueDto); 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 5a0e17d044e..9441ed4bbb4 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 @@ -29,7 +29,7 @@ import org.sonar.server.issue.TransitionService; import org.sonar.server.issue.workflow.FunctionExecutor; import org.sonar.server.issue.workflow.IssueWorkflow; import org.sonar.server.qualitygate.LiveQualityGateFactoryImpl; -import org.sonar.server.qualitygate.changeevent.IssueChangeTriggerImpl; +import org.sonar.server.qualitygate.changeevent.QGChangeEventFactoryImpl; import org.sonar.server.qualitygate.changeevent.QGChangeEventListenersImpl; import org.sonar.server.settings.ProjectConfigurationLoaderImpl; import org.sonar.server.webhook.WebhookQGChangeEventListener; @@ -69,7 +69,7 @@ public class IssueWsModule extends Module { BulkChangeAction.class, ProjectConfigurationLoaderImpl.class, LiveQualityGateFactoryImpl.class, - IssueChangeTriggerImpl.class, + QGChangeEventFactoryImpl.class, WebhookQGChangeEventListener.class, QGChangeEventListenersImpl.class); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SetTypeAction.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SetTypeAction.java index e30f64e749a..00e8e46bfde 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SetTypeAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SetTypeAction.java @@ -21,6 +21,7 @@ package org.sonar.server.issue.ws; import com.google.common.io.Resources; import java.util.Date; +import java.util.List; import org.sonar.api.rules.RuleType; import org.sonar.api.server.ws.Change; import org.sonar.api.server.ws.Request; @@ -37,7 +38,9 @@ import org.sonar.db.issue.IssueDto; import org.sonar.server.issue.IssueFieldsSetter; import org.sonar.server.issue.IssueFinder; import org.sonar.server.issue.IssueUpdater; -import org.sonar.server.qualitygate.changeevent.IssueChangeTrigger; +import org.sonar.server.qualitygate.changeevent.QGChangeEvent; +import org.sonar.server.qualitygate.changeevent.QGChangeEventFactory; +import org.sonar.server.qualitygate.changeevent.QGChangeEventListeners; import org.sonar.server.user.UserSession; import static com.google.common.collect.ImmutableList.copyOf; @@ -55,10 +58,12 @@ public class SetTypeAction implements IssuesWsAction { private final IssueUpdater issueUpdater; private final OperationResponseWriter responseWriter; private final System2 system2; - private final IssueChangeTrigger issueChangeTrigger; + private final QGChangeEventFactory qgChangeEventFactory; + private final QGChangeEventListeners qgChangeEventListeners; public SetTypeAction(UserSession userSession, DbClient dbClient, IssueFinder issueFinder, IssueFieldsSetter issueFieldsSetter, IssueUpdater issueUpdater, - OperationResponseWriter responseWriter, System2 system2, IssueChangeTrigger issueChangeTrigger) { + OperationResponseWriter responseWriter, System2 system2, + QGChangeEventFactory qgChangeEventFactory, QGChangeEventListeners qgChangeEventListeners) { this.userSession = userSession; this.dbClient = dbClient; this.issueFinder = issueFinder; @@ -66,7 +71,8 @@ public class SetTypeAction implements IssuesWsAction { this.issueUpdater = issueUpdater; this.responseWriter = responseWriter; this.system2 = system2; - this.issueChangeTrigger = issueChangeTrigger; + this.qgChangeEventFactory = qgChangeEventFactory; + this.qgChangeEventListeners = qgChangeEventListeners; } @Override @@ -116,12 +122,11 @@ public class SetTypeAction implements IssuesWsAction { IssueChangeContext context = IssueChangeContext.createUser(new Date(system2.now()), userSession.getLogin()); if (issueFieldsSetter.setType(issue, ruleType, context)) { SearchResponseData searchResponseData = issueUpdater.saveIssueAndPreloadSearchResponseData(session, issue, context, null); - issueChangeTrigger.onChange( - new IssueChangeTrigger.IssueChangeData( - searchResponseData.getIssues().stream().map(IssueDto::toDefaultIssue).collect(MoreCollectors.toList(searchResponseData.getIssues().size())), - copyOf(searchResponseData.getComponents())), - new IssueChangeTrigger.IssueChange(ruleType), - context); + QGChangeEventFactory.IssueChangeData issueChangeData = new QGChangeEventFactory.IssueChangeData( + searchResponseData.getIssues().stream().map(IssueDto::toDefaultIssue).collect(MoreCollectors.toList(searchResponseData.getIssues().size())), + copyOf(searchResponseData.getComponents())); + List qgChangeEvents = qgChangeEventFactory.from(issueChangeData, new QGChangeEventFactory.IssueChange(ruleType), context); + qgChangeEventListeners.broadcastOnIssueChange(issueChangeData, qgChangeEvents); return searchResponseData; } return new SearchResponseData(issueDto); diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/IssueChangeTrigger.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventFactory.java similarity index 96% rename from server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/IssueChangeTrigger.java rename to server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventFactory.java index 4fe0b86c1b3..57afd3bb56d 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/IssueChangeTrigger.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventFactory.java @@ -32,12 +32,12 @@ import org.sonar.server.issue.ws.SearchResponseData; import static com.google.common.base.Preconditions.checkArgument; -public interface IssueChangeTrigger { +public interface QGChangeEventFactory { /** * Will call webhooks once for any short living branch which has at least one issue in {@link SearchResponseData} and * if change described in {@link IssueChange} can alter the status of the short living branch. */ - void onChange(IssueChangeData issueChangeData, IssueChange issueChange, IssueChangeContext context); + List from(IssueChangeData issueChangeData, IssueChange issueChange, IssueChangeContext context); final class IssueChange { private final RuleType ruleType; diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/IssueChangeTriggerImpl.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventFactoryImpl.java similarity index 86% rename from server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/IssueChangeTriggerImpl.java rename to server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventFactoryImpl.java index bdca8f10ad7..9c359001603 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/IssueChangeTriggerImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventFactoryImpl.java @@ -42,36 +42,35 @@ import org.sonar.db.component.SnapshotDto; import org.sonar.server.qualitygate.LiveQualityGateFactory; import org.sonar.server.settings.ProjectConfigurationLoader; +import static java.util.Collections.emptyList; import static org.sonar.core.util.stream.MoreCollectors.toSet; import static org.sonar.core.util.stream.MoreCollectors.uniqueIndex; -public class IssueChangeTriggerImpl implements IssueChangeTrigger { +public class QGChangeEventFactoryImpl implements QGChangeEventFactory { private static final Set MEANINGFUL_TRANSITIONS = ImmutableSet.of( DefaultTransitions.RESOLVE, DefaultTransitions.FALSE_POSITIVE, DefaultTransitions.WONT_FIX, DefaultTransitions.REOPEN); private final DbClient dbClient; private final ProjectConfigurationLoader projectConfigurationLoader; - private final QGChangeEventListeners qgEventListeners; private final LiveQualityGateFactory liveQualityGateFactory; - public IssueChangeTriggerImpl(DbClient dbClient, ProjectConfigurationLoader projectConfigurationLoader, - QGChangeEventListeners qgEventListeners, LiveQualityGateFactory liveQualityGateFactory) { + public QGChangeEventFactoryImpl(DbClient dbClient, ProjectConfigurationLoader projectConfigurationLoader, + LiveQualityGateFactory liveQualityGateFactory) { this.dbClient = dbClient; this.projectConfigurationLoader = projectConfigurationLoader; - this.qgEventListeners = qgEventListeners; this.liveQualityGateFactory = liveQualityGateFactory; } @Override - public void onChange(IssueChangeData issueChangeData, IssueChange issueChange, IssueChangeContext context) { - if (isEmpty(issueChangeData) || !isUserChangeContext(context) || !isRelevant(issueChange) || qgEventListeners.isEmpty()) { - return; + public List from(IssueChangeData issueChangeData, IssueChange issueChange, IssueChangeContext context) { + if (isEmpty(issueChangeData) || !isUserChangeContext(context) || !isRelevant(issueChange)) { + return emptyList(); } - broadcastToListeners(issueChangeData); + return from(issueChangeData); } private static boolean isRelevant(IssueChange issueChange) { - return issueChange.getTransitionKey().map(IssueChangeTriggerImpl::isMeaningfulTransition).orElse(true); + return issueChange.getTransitionKey().map(QGChangeEventFactoryImpl::isMeaningfulTransition).orElse(true); } private static boolean isEmpty(IssueChangeData issueChangeData) { @@ -86,11 +85,11 @@ public class IssueChangeTriggerImpl implements IssueChangeTrigger { return MEANINGFUL_TRANSITIONS.contains(transitionKey); } - private void broadcastToListeners(IssueChangeData issueChangeData) { + private List from(IssueChangeData issueChangeData) { try (DbSession dbSession = dbClient.openSession(false)) { Map branchesByUuid = getBranchComponents(dbSession, issueChangeData); if (branchesByUuid.isEmpty()) { - return; + return emptyList(); } Set branchProjectUuids = branchesByUuid.values().stream() @@ -101,7 +100,7 @@ public class IssueChangeTriggerImpl implements IssueChangeTrigger { .filter(branchDto -> branchDto.getBranchType() == BranchType.SHORT) .collect(toSet(branchesByUuid.size())); if (shortBranches.isEmpty()) { - return; + return emptyList(); } Map configurationByUuid = projectConfigurationLoader.loadProjectConfigurations(dbSession, @@ -113,7 +112,7 @@ public class IssueChangeTriggerImpl implements IssueChangeTrigger { .stream() .collect(uniqueIndex(SnapshotDto::getComponentUuid)); - List qgChangeEvents = shortBranches + return shortBranches .stream() .map(shortBranch -> { ComponentDto branch = branchesByUuid.get(shortBranch.getUuid()); @@ -128,7 +127,6 @@ public class IssueChangeTriggerImpl implements IssueChangeTrigger { }) .filter(Objects::nonNull) .collect(MoreCollectors.toList(shortBranches.size())); - qgEventListeners.broadcast(Trigger.ISSUE_CHANGE, qgChangeEvents); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListeners.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListeners.java index 50065d9bb26..1c6b434489f 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListeners.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListeners.java @@ -24,5 +24,5 @@ import java.util.Collection; public interface QGChangeEventListeners { boolean isEmpty(); - void broadcast(Trigger trigger, Collection changeEvents); + void broadcastOnIssueChange(QGChangeEventFactory.IssueChangeData issueChangeData, Collection qgChangeEvents); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListenersImpl.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListenersImpl.java index a697aad3f36..892bd828916 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListenersImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListenersImpl.java @@ -57,14 +57,15 @@ public class QGChangeEventListenersImpl implements QGChangeEventListeners { } @Override - public void broadcast(Trigger trigger, Collection changeEvents) { - if (changeEvents.isEmpty()) { + public void broadcastOnIssueChange(QGChangeEventFactory.IssueChangeData issueChangeData, Collection changeEvents) { + if (listeners.length == 0 || issueChangeData.getComponents().isEmpty() || issueChangeData.getIssues().isEmpty() || changeEvents.isEmpty()) { return; } try { List immutableChangeEvents = ImmutableList.copyOf(changeEvents); - Arrays.stream(listeners).forEach(listener -> broadcastTo(trigger, immutableChangeEvents, listener)); + Arrays.stream(listeners) + .forEach(listener -> broadcastTo(Trigger.ISSUE_CHANGE, immutableChangeEvents, listener)); } catch (Error e) { LOG.warn(format("Broadcasting to listeners failed for %s events", changeEvents.size()), e); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/BulkChangeActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/BulkChangeActionTest.java index dfb147d3728..b4a59f9e904 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/BulkChangeActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/BulkChangeActionTest.java @@ -21,6 +21,7 @@ package org.sonar.server.issue.ws; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Date; import java.util.List; import java.util.stream.Collectors; @@ -62,7 +63,9 @@ import org.sonar.server.issue.workflow.IssueWorkflow; import org.sonar.server.notification.NotificationManager; import org.sonar.server.organization.DefaultOrganizationProvider; import org.sonar.server.organization.TestDefaultOrganizationProvider; -import org.sonar.server.qualitygate.changeevent.IssueChangeTrigger; +import org.sonar.server.qualitygate.changeevent.QGChangeEvent; +import org.sonar.server.qualitygate.changeevent.QGChangeEventFactory; +import org.sonar.server.qualitygate.changeevent.QGChangeEventListeners; import org.sonar.server.rule.DefaultRuleFinder; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.TestRequest; @@ -76,7 +79,9 @@ import static java.util.Collections.singletonList; import static java.util.Objects.requireNonNull; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.tuple; +import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; +import static org.mockito.Matchers.same; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyZeroInteractions; @@ -120,7 +125,8 @@ public class BulkChangeActionTest { private IssueStorage issueStorage = new ServerIssueStorage(system2, new DefaultRuleFinder(dbClient, defaultOrganizationProvider), dbClient, new IssueIndexer(es.client(), dbClient, new IssueIteratorFactory(dbClient))); private NotificationManager notificationManager = mock(NotificationManager.class); - private IssueChangeTrigger issueChangeTrigger = mock(IssueChangeTrigger.class); + private QGChangeEventFactory qgChangeEventFactory = mock(QGChangeEventFactory.class); + private QGChangeEventListeners qgChangeEventListeners = mock(QGChangeEventListeners.class); private List actions = new ArrayList<>(); private RuleDto rule; @@ -129,10 +135,11 @@ public class BulkChangeActionTest { private ComponentDto file; private UserDto user; - private WsActionTester tester = new WsActionTester(new BulkChangeAction(system2, userSession, dbClient, issueStorage, notificationManager, actions, issueChangeTrigger)); + private WsActionTester tester = new WsActionTester( + new BulkChangeAction(system2, userSession, dbClient, issueStorage, notificationManager, actions, qgChangeEventFactory, qgChangeEventListeners)); @Before - public void setUp() throws Exception { + public void setUp() { issueWorkflow.start(); rule = db.rules().insertRule(newRuleDto()); organization = db.organizations().insert(); @@ -144,9 +151,10 @@ public class BulkChangeActionTest { } @Test - public void set_type() throws Exception { + public void set_type() { setUserProjectPermissions(USER, ISSUE_ADMIN); IssueDto issueDto = db.issues().insertIssue(newUnresolvedIssue().setType(BUG)); + List qgChangeEvents = mockQGChangeEvents(CODE_SMELL, null); BulkChangeWsResponse response = call(builder() .setIssues(singletonList(issueDto.getKey())) @@ -158,11 +166,12 @@ public class BulkChangeActionTest { assertThat(reloaded.getType()).isEqualTo(RuleType.CODE_SMELL.getDbConstant()); assertThat(reloaded.getUpdatedAt()).isEqualTo(NOW); - verifyIssueChangeWebhookCalled(CODE_SMELL, null, new String[] {file.uuid()}, issueDto); + QGChangeEventFactory.IssueChangeData issueChangeData = verifyIssueChangeData(CODE_SMELL, null, new String[] {file.uuid()}, issueDto); + verifyBroadcastOnIssueChange(issueChangeData, qgChangeEvents); } @Test - public void set_severity() throws Exception { + public void set_severity() { setUserProjectPermissions(USER, ISSUE_ADMIN); IssueDto issueDto = db.issues().insertIssue(newUnresolvedIssue().setSeverity(MAJOR)); @@ -176,11 +185,11 @@ public class BulkChangeActionTest { assertThat(reloaded.getSeverity()).isEqualTo(MINOR); assertThat(reloaded.getUpdatedAt()).isEqualTo(NOW); - verifyZeroInteractions(issueChangeTrigger); + verifyZeroInteractions(qgChangeEventFactory); } @Test - public void add_tags() throws Exception { + public void add_tags() { setUserProjectPermissions(USER, ISSUE_ADMIN); IssueDto issueDto = db.issues().insertIssue(newUnresolvedIssue().setTags(asList("tag1", "tag2"))); @@ -194,11 +203,11 @@ public class BulkChangeActionTest { assertThat(reloaded.getTags()).containsOnly("tag1", "tag2", "tag3"); assertThat(reloaded.getUpdatedAt()).isEqualTo(NOW); - verifyZeroInteractions(issueChangeTrigger); + verifyZeroInteractions(qgChangeEventFactory); } @Test - public void remove_assignee() throws Exception { + public void remove_assignee() { setUserProjectPermissions(USER); IssueDto issueDto = db.issues().insertIssue(newUnresolvedIssue().setAssignee("arthur")); @@ -212,13 +221,14 @@ public class BulkChangeActionTest { assertThat(reloaded.getAssignee()).isNull(); assertThat(reloaded.getUpdatedAt()).isEqualTo(NOW); - verifyZeroInteractions(issueChangeTrigger); + verifyZeroInteractions(qgChangeEventFactory); } @Test - public void bulk_change_with_comment() throws Exception { + public void bulk_change_with_comment() { setUserProjectPermissions(USER); IssueDto issueDto = db.issues().insertIssue(newUnresolvedIssue().setType(BUG)); + List qgChangeEvents = mockQGChangeEvents(null, "confirm"); BulkChangeWsResponse response = call(builder() .setIssues(singletonList(issueDto.getKey())) @@ -231,11 +241,12 @@ public class BulkChangeActionTest { assertThat(issueComment.getUserLogin()).isEqualTo("john"); assertThat(issueComment.getChangeData()).isEqualTo("type was badly defined"); - verifyIssueChangeWebhookCalled(null, "confirm", new String[] {file.uuid()}, issueDto); + QGChangeEventFactory.IssueChangeData issueChangeData = verifyIssueChangeData(null, "confirm", new String[] {file.uuid()}, issueDto); + verifyBroadcastOnIssueChange(issueChangeData, qgChangeEvents); } @Test - public void bulk_change_many_issues() throws Exception { + public void bulk_change_many_issues() { setUserProjectPermissions(USER, ISSUE_ADMIN); UserDto userToAssign = db.users().insertUser("arthur"); db.organizations().addMember(organization, user); @@ -243,6 +254,7 @@ public class BulkChangeActionTest { IssueDto issue1 = db.issues().insertIssue(newUnresolvedIssue().setAssignee(user.getLogin())).setType(BUG).setSeverity(MINOR); IssueDto issue2 = db.issues().insertIssue(newUnresolvedIssue().setAssignee(userToAssign.getLogin())).setType(BUG).setSeverity(MAJOR); IssueDto issue3 = db.issues().insertIssue(newUnresolvedIssue().setAssignee(null)).setType(VULNERABILITY).setSeverity(MAJOR); + List qgChangeEvents = mockQGChangeEvents(VULNERABILITY, null); BulkChangeWsResponse response = call(builder() .setIssues(asList(issue1.getKey(), issue2.getKey(), issue3.getKey())) @@ -259,13 +271,15 @@ public class BulkChangeActionTest { tuple(issue2.getKey(), userToAssign.getLogin(), VULNERABILITY.getDbConstant(), MINOR, NOW), tuple(issue3.getKey(), userToAssign.getLogin(), VULNERABILITY.getDbConstant(), MINOR, NOW)); - verifyIssueChangeWebhookCalled(VULNERABILITY, null, new String[] {file.uuid()}, issue1, issue2, issue3); + QGChangeEventFactory.IssueChangeData issueChangeData = verifyIssueChangeData(VULNERABILITY, null, new String[] {file.uuid()}, issue1, issue2, issue3); + verifyBroadcastOnIssueChange(issueChangeData, qgChangeEvents); } @Test - public void send_notification() throws Exception { + public void send_notification() { setUserProjectPermissions(USER); IssueDto issueDto = db.issues().insertIssue(newUnresolvedIssue().setType(BUG)); + List qgChangeEvents = mockQGChangeEvents(null, "confirm"); BulkChangeWsResponse response = call(builder() .setIssues(singletonList(issueDto.getKey())) @@ -285,17 +299,19 @@ public class BulkChangeActionTest { assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("changeAuthor")).isEqualTo(user.getLogin()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("branch")).isNull(); - verifyIssueChangeWebhookCalled(null, "confirm", new String[] {file.uuid()}, issueDto); + QGChangeEventFactory.IssueChangeData issueChangeData = verifyIssueChangeData(null, "confirm", new String[] {file.uuid()}, issueDto); + verifyBroadcastOnIssueChange(issueChangeData, qgChangeEvents); } @Test - public void send_notification_on_branch() throws Exception { + public void send_notification_on_branch() { setUserProjectPermissions(USER); String branchName = "feature1"; ComponentDto branch = db.components().insertProjectBranch(project, b -> b.setKey(branchName)); ComponentDto fileOnBranch = db.components().insertComponent(newFileDto(branch)); IssueDto issueDto = db.issues().insertIssue(newUnresolvedIssue(rule, fileOnBranch, branch).setType(BUG)); + List qgChangeEvents = mockQGChangeEvents(null, "confirm"); BulkChangeWsResponse response = call(builder() .setIssues(singletonList(issueDto.getKey())) @@ -315,16 +331,18 @@ public class BulkChangeActionTest { assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("changeAuthor")).isEqualTo(user.getLogin()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("branch")).isEqualTo(branchName); - verifyIssueChangeWebhookCalled(null, "confirm", new String[] {fileOnBranch.uuid()}, issueDto); + QGChangeEventFactory.IssueChangeData issueChangeData = verifyIssueChangeData(null, "confirm", new String[] {fileOnBranch.uuid()}, issueDto); + verifyBroadcastOnIssueChange(issueChangeData, qgChangeEvents); } @Test - public void send_notification_only_on_changed_issues() throws Exception { + public void send_notification_only_on_changed_issues() { setUserProjectPermissions(USER, ISSUE_ADMIN); IssueDto issue1 = db.issues().insertIssue(newUnresolvedIssue().setType(BUG)); IssueDto issue2 = db.issues().insertIssue(newUnresolvedIssue().setType(BUG)); IssueDto issue3 = db.issues().insertIssue(newUnresolvedIssue().setType(VULNERABILITY)); ArgumentCaptor issueChangeNotificationCaptor = ArgumentCaptor.forClass(IssueChangeNotification.class); + List qgChangeEvents = mockQGChangeEvents(BUG, null); BulkChangeWsResponse response = call(builder() .setIssues(asList(issue1.getKey(), issue2.getKey(), issue3.getKey())) @@ -337,16 +355,18 @@ public class BulkChangeActionTest { assertThat(issueChangeNotificationCaptor.getAllValues()).hasSize(1); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("key")).isEqualTo(issue3.getKey()); - verifyIssueChangeWebhookCalled(BUG, null, new String[] {file.uuid()}, issue3); + QGChangeEventFactory.IssueChangeData issueChangeData = verifyIssueChangeData(BUG, null, new String[] {file.uuid()}, issue3); + verifyBroadcastOnIssueChange(issueChangeData, qgChangeEvents); } @Test - public void ignore_issues_when_condition_does_not_match() throws Exception { + public void ignore_issues_when_condition_does_not_match() { setUserProjectPermissions(USER, ISSUE_ADMIN); IssueDto issue1 = db.issues().insertIssue(newUnresolvedIssue().setType(BUG)); // These 2 issues will be ignored as they are resolved, changing type is not possible IssueDto issue2 = db.issues().insertIssue(newResolvedIssue().setType(BUG)); IssueDto issue3 = db.issues().insertIssue(newResolvedIssue().setType(BUG)); + List qgChangeEvents = mockQGChangeEvents(VULNERABILITY, null); BulkChangeWsResponse response = call(builder() .setIssues(asList(issue1.getKey(), issue2.getKey(), issue3.getKey())) @@ -361,16 +381,18 @@ public class BulkChangeActionTest { tuple(issue3.getKey(), BUG.getDbConstant(), issue2.getUpdatedAt()), tuple(issue2.getKey(), BUG.getDbConstant(), issue3.getUpdatedAt())); - verifyIssueChangeWebhookCalled(VULNERABILITY, null, new String[] {file.uuid()}, issue1); + QGChangeEventFactory.IssueChangeData issueChangeData = verifyIssueChangeData(VULNERABILITY, null, new String[] {file.uuid()}, issue1); + verifyBroadcastOnIssueChange(issueChangeData, qgChangeEvents); } @Test - public void ignore_issues_when_there_is_nothing_to_do() throws Exception { + public void ignore_issues_when_there_is_nothing_to_do() { setUserProjectPermissions(USER, ISSUE_ADMIN); IssueDto issue1 = db.issues().insertIssue(newUnresolvedIssue().setType(BUG).setSeverity(MINOR)); // These 2 issues will be ignored as there's nothing to do IssueDto issue2 = db.issues().insertIssue(newUnresolvedIssue().setType(VULNERABILITY)); IssueDto issue3 = db.issues().insertIssue(newUnresolvedIssue().setType(VULNERABILITY)); + List qgChangeEvents = mockQGChangeEvents(VULNERABILITY, null); BulkChangeWsResponse response = call(builder() .setIssues(asList(issue1.getKey(), issue2.getKey(), issue3.getKey())) @@ -385,16 +407,18 @@ public class BulkChangeActionTest { tuple(issue2.getKey(), VULNERABILITY.getDbConstant(), issue2.getUpdatedAt()), tuple(issue3.getKey(), VULNERABILITY.getDbConstant(), issue3.getUpdatedAt())); - verifyIssueChangeWebhookCalled(VULNERABILITY, null, new String[] {file.uuid()}, issue1); + QGChangeEventFactory.IssueChangeData issueChangeData = verifyIssueChangeData(VULNERABILITY, null, new String[] {file.uuid()}, issue1); + verifyBroadcastOnIssueChange(issueChangeData, qgChangeEvents); } @Test - public void add_comment_only_on_changed_issues() throws Exception { + public void add_comment_only_on_changed_issues() { setUserProjectPermissions(USER, ISSUE_ADMIN); IssueDto issue1 = db.issues().insertIssue(newUnresolvedIssue().setType(BUG).setSeverity(MINOR)); // These 2 issues will be ignored as there's nothing to do IssueDto issue2 = db.issues().insertIssue(newUnresolvedIssue().setType(VULNERABILITY)); IssueDto issue3 = db.issues().insertIssue(newUnresolvedIssue().setType(VULNERABILITY)); + List qgChangeEvents = mockQGChangeEvents(VULNERABILITY, null); BulkChangeWsResponse response = call(builder() .setIssues(asList(issue1.getKey(), issue2.getKey(), issue3.getKey())) @@ -407,11 +431,12 @@ public class BulkChangeActionTest { assertThat(dbClient.issueChangeDao().selectByTypeAndIssueKeys(db.getSession(), singletonList(issue2.getKey()), TYPE_COMMENT)).isEmpty(); assertThat(dbClient.issueChangeDao().selectByTypeAndIssueKeys(db.getSession(), singletonList(issue3.getKey()), TYPE_COMMENT)).isEmpty(); - verifyIssueChangeWebhookCalled(VULNERABILITY, null, new String[] {file.uuid()}, issue1); + QGChangeEventFactory.IssueChangeData issueChangeData = verifyIssueChangeData(VULNERABILITY, null, new String[] {file.uuid()}, issue1); + verifyBroadcastOnIssueChange(issueChangeData, qgChangeEvents); } @Test - public void issues_on_which_user_has_not_browse_permission_are_ignored() throws Exception { + public void issues_on_which_user_has_not_browse_permission_are_ignored() { setUserProjectPermissions(USER, ISSUE_ADMIN); ComponentDto anotherProject = db.components().insertPrivateProject(); ComponentDto anotherFile = db.components().insertComponent(newFileDto(anotherProject)); @@ -419,6 +444,7 @@ public class BulkChangeActionTest { // User has not browse permission on these 2 issues IssueDto notAuthorizedIssue1 = db.issues().insertIssue(newUnresolvedIssue(rule, anotherFile, anotherProject).setType(BUG)); IssueDto notAuthorizedIssue2 = db.issues().insertIssue(newUnresolvedIssue(rule, anotherFile, anotherProject).setType(BUG)); + List qgChangeEvents = mockQGChangeEvents(VULNERABILITY, null); BulkChangeWsResponse response = call(builder() .setIssues(asList(authorizedIssue.getKey(), notAuthorizedIssue1.getKey(), notAuthorizedIssue2.getKey())) @@ -433,11 +459,12 @@ public class BulkChangeActionTest { tuple(notAuthorizedIssue1.getKey(), BUG.getDbConstant(), notAuthorizedIssue1.getUpdatedAt()), tuple(notAuthorizedIssue2.getKey(), BUG.getDbConstant(), notAuthorizedIssue2.getUpdatedAt())); - verifyIssueChangeWebhookCalled(VULNERABILITY, null, new String[] {file.uuid()}, authorizedIssue); + QGChangeEventFactory.IssueChangeData issueChangeData = verifyIssueChangeData(VULNERABILITY, null, new String[] {file.uuid()}, authorizedIssue); + verifyBroadcastOnIssueChange(issueChangeData, qgChangeEvents); } @Test - public void does_not_update_type_when_no_issue_admin_permission() throws Exception { + public void does_not_update_type_when_no_issue_admin_permission() { setUserProjectPermissions(USER, ISSUE_ADMIN); ComponentDto anotherProject = db.components().insertPrivateProject(); ComponentDto anotherFile = db.components().insertComponent(newFileDto(anotherProject)); @@ -463,7 +490,7 @@ public class BulkChangeActionTest { } @Test - public void does_not_update_severity_when_no_issue_admin_permission() throws Exception { + public void does_not_update_severity_when_no_issue_admin_permission() { setUserProjectPermissions(USER, ISSUE_ADMIN); ComponentDto anotherProject = db.components().insertPrivateProject(); ComponentDto anotherFile = db.components().insertComponent(newFileDto(anotherProject)); @@ -489,7 +516,7 @@ public class BulkChangeActionTest { } @Test - public void fail_when_only_comment_action() throws Exception { + public void fail_when_only_comment_action() { setUserProjectPermissions(USER); IssueDto issueDto = db.issues().insertIssue(newUnresolvedIssue().setType(BUG)); expectedException.expectMessage("At least one action must be provided"); @@ -502,7 +529,7 @@ public class BulkChangeActionTest { } @Test - public void fail_when_number_of_issues_is_more_than_500() throws Exception { + public void fail_when_number_of_issues_is_more_than_500() { userSession.logIn("john"); expectedException.expectMessage("Number of issues is limited to 500"); expectedException.expect(IllegalArgumentException.class); @@ -514,14 +541,14 @@ public class BulkChangeActionTest { } @Test - public void fail_when_not_authenticated() throws Exception { + public void fail_when_not_authenticated() { expectedException.expect(UnauthorizedException.class); call(builder().setIssues(singletonList("ABCD")).build()); } @Test - public void test_definition() throws Exception { + public void test_definition() { WebService.Action action = tester.getDef(); assertThat(action.key()).isEqualTo("bulk_change"); assertThat(action.isPost()).isTrue(); @@ -533,18 +560,55 @@ public class BulkChangeActionTest { private void verifyIssueChangeWebhookCalled(@Nullable RuleType expectedRuleType, @Nullable String transitionKey, String[] componentUUids, IssueDto... issueDtos) { - ArgumentCaptor issueChangeDataCaptor = ArgumentCaptor.forClass(IssueChangeTrigger.IssueChangeData.class); - verify(issueChangeTrigger).onChange( - issueChangeDataCaptor.capture(), - eq(new IssueChangeTrigger.IssueChange(expectedRuleType, transitionKey)), - eq(IssueChangeContext.createUser(new Date(NOW), userSession.getLogin()))); - IssueChangeTrigger.IssueChangeData issueChangeData = issueChangeDataCaptor.getValue(); + QGChangeEventFactory.IssueChange issueChange = new QGChangeEventFactory.IssueChange(expectedRuleType, transitionKey); + IssueChangeContext user = IssueChangeContext.createUser(new Date(NOW), userSession.getLogin()); + List changeEvents = Collections.singletonList(mock(QGChangeEvent.class)); + when(qgChangeEventFactory.from(any(QGChangeEventFactory.IssueChangeData.class), eq(issueChange), eq(user))).thenReturn(changeEvents); + + ArgumentCaptor issueChangeDataCaptor = ArgumentCaptor.forClass(QGChangeEventFactory.IssueChangeData.class); + verify(qgChangeEventFactory).from(issueChangeDataCaptor.capture(), eq(issueChange), eq(user)); + + QGChangeEventFactory.IssueChangeData issueChangeData = issueChangeDataCaptor.getValue(); + assertThat(issueChangeData.getIssues()) + .extracting(DefaultIssue::key) + .containsOnly(Arrays.stream(issueDtos).map(IssueDto::getKey).toArray(String[]::new)); + assertThat(issueChangeData.getComponents()) + .extracting(ComponentDto::uuid) + .containsOnly(componentUUids); + + verify(qgChangeEventListeners).broadcastOnIssueChange(same(issueChangeData), same(changeEvents)); + } + + private List mockQGChangeEvents(@Nullable RuleType expectedRuleType, @Nullable String transitionKey) { + QGChangeEventFactory.IssueChange issueChange = new QGChangeEventFactory.IssueChange(expectedRuleType, transitionKey); + IssueChangeContext user = IssueChangeContext.createUser(new Date(NOW), userSession.getLogin()); + List changeEvents = Collections.singletonList(mock(QGChangeEvent.class)); + when(qgChangeEventFactory.from(any(QGChangeEventFactory.IssueChangeData.class), eq(issueChange), eq(user))).thenReturn(changeEvents); + + return changeEvents; + } + + private QGChangeEventFactory.IssueChangeData verifyIssueChangeData(@Nullable RuleType expectedRuleType, @Nullable String transitionKey, + String[] componentUUids, + IssueDto... issueDtos) { + QGChangeEventFactory.IssueChange issueChange = new QGChangeEventFactory.IssueChange(expectedRuleType, transitionKey); + IssueChangeContext user = IssueChangeContext.createUser(new Date(NOW), userSession.getLogin()); + + ArgumentCaptor issueChangeDataCaptor = ArgumentCaptor.forClass(QGChangeEventFactory.IssueChangeData.class); + verify(qgChangeEventFactory).from(issueChangeDataCaptor.capture(), eq(issueChange), eq(user)); + + QGChangeEventFactory.IssueChangeData issueChangeData = issueChangeDataCaptor.getValue(); assertThat(issueChangeData.getIssues()) .extracting(DefaultIssue::key) .containsOnly(Arrays.stream(issueDtos).map(IssueDto::getKey).toArray(String[]::new)); assertThat(issueChangeData.getComponents()) .extracting(ComponentDto::uuid) .containsOnly(componentUUids); + return issueChangeData; + } + + private void verifyBroadcastOnIssueChange(QGChangeEventFactory.IssueChangeData issueChangeData, List changeEvents) { + verify(qgChangeEventListeners).broadcastOnIssueChange(same(issueChangeData), same(changeEvents)); } private BulkChangeWsResponse call(BulkChangeRequest bulkChangeRequest) { 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 69a4815f72f..b4c68aed9b6 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,7 +19,9 @@ */ package org.sonar.server.issue.ws; +import java.util.Collections; import java.util.Date; +import java.util.List; import javax.annotation.Nullable; import org.junit.Before; import org.junit.Rule; @@ -58,7 +60,9 @@ import org.sonar.server.issue.workflow.IssueWorkflow; import org.sonar.server.notification.NotificationManager; import org.sonar.server.organization.DefaultOrganizationProvider; import org.sonar.server.organization.TestDefaultOrganizationProvider; -import org.sonar.server.qualitygate.changeevent.IssueChangeTrigger; +import org.sonar.server.qualitygate.changeevent.QGChangeEvent; +import org.sonar.server.qualitygate.changeevent.QGChangeEventFactory; +import org.sonar.server.qualitygate.changeevent.QGChangeEventListeners; import org.sonar.server.rule.DefaultRuleFinder; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.TestRequest; @@ -82,6 +86,7 @@ import static org.sonar.db.rule.RuleTesting.newRuleDto; public class DoTransitionActionTest { + private static final long NOW = 999_776_888L; private System2 system2 = mock(System2.class); @Rule @@ -113,10 +118,11 @@ public class DoTransitionActionTest { private ComponentDto project; private ComponentDto file; private ArgumentCaptor preloadedSearchResponseDataCaptor = ArgumentCaptor.forClass(SearchResponseData.class); - private IssueChangeTrigger issueChangeTrigger = mock(IssueChangeTrigger.class); + private QGChangeEventFactory qgChangeEventFactory = mock(QGChangeEventFactory.class); + private QGChangeEventListeners qgChangeEventListeners = mock(QGChangeEventListeners.class); private WsAction underTest = new DoTransitionAction(dbClient, userSession, new IssueFinder(dbClient, userSession), issueUpdater, transitionService, responseWriter, system2, - issueChangeTrigger); + qgChangeEventFactory, qgChangeEventListeners); private WsActionTester tester = new WsActionTester(underTest); @Before @@ -127,11 +133,15 @@ public class DoTransitionActionTest { } @Test - public void do_transition() throws Exception { - long now = 999_776_888L; - when(system2.now()).thenReturn(now); + public void do_transition() { + when(system2.now()).thenReturn(NOW); IssueDto issueDto = issueDbTester.insertIssue(newIssue().setStatus(STATUS_OPEN).setResolution(null)); userSession.logIn("john").addProjectPermission(USER, project, file); + IssueChangeContext changeContext = IssueChangeContext.createUser(new Date(NOW), userSession.getLogin()); + QGChangeEventFactory.IssueChange issueChange = new QGChangeEventFactory.IssueChange(null, "confirm"); + List qgChangeEvents = Collections.singletonList(mock(QGChangeEvent.class)); + when(qgChangeEventFactory.from(any(QGChangeEventFactory.IssueChangeData.class), eq(issueChange), eq(changeContext))) + .thenReturn(qgChangeEvents); call(issueDto.getKey(), "confirm"); @@ -140,22 +150,20 @@ public class DoTransitionActionTest { IssueDto issueReloaded = dbClient.issueDao().selectByKey(dbTester.getSession(), issueDto.getKey()).get(); assertThat(issueReloaded.getStatus()).isEqualTo(STATUS_CONFIRMED); - ArgumentCaptor issueChangeDataCaptor = ArgumentCaptor.forClass(IssueChangeTrigger.IssueChangeData.class); - verify(issueChangeTrigger).onChange( - issueChangeDataCaptor.capture(), - eq(new IssueChangeTrigger.IssueChange(null, "confirm")), - eq(IssueChangeContext.createUser(new Date(now), userSession.getLogin()))); - IssueChangeTrigger.IssueChangeData issueChangeData = issueChangeDataCaptor.getValue(); + ArgumentCaptor issueChangeDataCaptor = ArgumentCaptor.forClass(QGChangeEventFactory.IssueChangeData.class); + verify(qgChangeEventFactory).from(issueChangeDataCaptor.capture(), eq(issueChange), eq(changeContext)); + QGChangeEventFactory.IssueChangeData issueChangeData = issueChangeDataCaptor.getValue(); assertThat(issueChangeData.getIssues()) .extracting(DefaultIssue::key) .containsOnly(issueDto.getKey()); assertThat(issueChangeData.getComponents()) .extracting(ComponentDto::uuid) .containsOnly(issueDto.getComponentUuid(), issueDto.getProjectUuid()); + verify(qgChangeEventListeners).broadcastOnIssueChange(issueChangeData, qgChangeEvents); } @Test - public void fail_if_issue_does_not_exist() throws Exception { + public void fail_if_issue_does_not_exist() { userSession.logIn("john"); expectedException.expect(NotFoundException.class); @@ -163,7 +171,7 @@ public class DoTransitionActionTest { } @Test - public void fail_if_no_issue_param() throws Exception { + public void fail_if_no_issue_param() { userSession.logIn("john"); expectedException.expect(IllegalArgumentException.class); @@ -171,7 +179,7 @@ public class DoTransitionActionTest { } @Test - public void fail_if_no_transition_param() throws Exception { + public void fail_if_no_transition_param() { IssueDto issueDto = issueDbTester.insertIssue(newIssue().setStatus(STATUS_OPEN).setResolution(null)); userSession.logIn("john").addProjectPermission(USER, project, file); @@ -180,7 +188,7 @@ public class DoTransitionActionTest { } @Test - public void fail_if_not_enough_permission_to_access_issue() throws Exception { + public void fail_if_not_enough_permission_to_access_issue() { IssueDto issueDto = issueDbTester.insertIssue(newIssue().setStatus(STATUS_OPEN).setResolution(null)); userSession.logIn("john").addProjectPermission(CODEVIEWER, project, file); @@ -189,7 +197,7 @@ public class DoTransitionActionTest { } @Test - public void fail_if_not_enough_permission_to_apply_transition() throws Exception { + public void fail_if_not_enough_permission_to_apply_transition() { IssueDto issueDto = issueDbTester.insertIssue(newIssue().setStatus(STATUS_OPEN).setResolution(null)); userSession.logIn("john").addProjectPermission(USER, project, file); @@ -199,7 +207,7 @@ public class DoTransitionActionTest { } @Test - public void fail_if_not_authenticated() throws Exception { + public void fail_if_not_authenticated() { expectedException.expect(UnauthorizedException.class); call("ISSUE_KEY", "confirm"); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetTypeActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetTypeActionTest.java index 56b46375d01..03cc73c3e21 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetTypeActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetTypeActionTest.java @@ -19,6 +19,7 @@ */ package org.sonar.server.issue.ws; +import java.util.Collections; import java.util.Date; import java.util.List; import javax.annotation.Nullable; @@ -54,7 +55,9 @@ import org.sonar.server.issue.index.IssueIteratorFactory; import org.sonar.server.notification.NotificationManager; import org.sonar.server.organization.DefaultOrganizationProvider; import org.sonar.server.organization.TestDefaultOrganizationProvider; -import org.sonar.server.qualitygate.changeevent.IssueChangeTrigger; +import org.sonar.server.qualitygate.changeevent.QGChangeEvent; +import org.sonar.server.qualitygate.changeevent.QGChangeEventFactory; +import org.sonar.server.qualitygate.changeevent.QGChangeEventListeners; import org.sonar.server.rule.DefaultRuleFinder; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.TestRequest; @@ -97,18 +100,24 @@ public class SetTypeActionTest { private ArgumentCaptor preloadedSearchResponseDataCaptor = ArgumentCaptor.forClass(SearchResponseData.class); private IssueIndexer issueIndexer = new IssueIndexer(esTester.client(), dbClient, new IssueIteratorFactory(dbClient)); - private IssueChangeTrigger issueChangeTrigger = mock(IssueChangeTrigger.class); + private QGChangeEventFactory qgChangeEventFactory = mock(QGChangeEventFactory.class); + private QGChangeEventListeners qgChangeEventListeners = mock(QGChangeEventListeners.class); private WsActionTester tester = new WsActionTester(new SetTypeAction(userSession, dbClient, new IssueFinder(dbClient, userSession), new IssueFieldsSetter(), new IssueUpdater(dbClient, new ServerIssueStorage(system2, new DefaultRuleFinder(dbClient, defaultOrganizationProvider), dbClient, issueIndexer), mock(NotificationManager.class)), - responseWriter, system2, issueChangeTrigger)); + responseWriter, system2, qgChangeEventFactory, qgChangeEventListeners)); @Test - public void set_type() throws Exception { + public void set_type() { long now = 1_999_777_234L; when(system2.now()).thenReturn(now); IssueDto issueDto = issueDbTester.insertIssue(newIssue().setType(CODE_SMELL)); setUserWithBrowseAndAdministerIssuePermission(issueDto); + QGChangeEventFactory.IssueChange issueChange = new QGChangeEventFactory.IssueChange(BUG, null); + IssueChangeContext changeContext = IssueChangeContext.createUser(new Date(now), userSession.getLogin()); + List qgChangeEvents = Collections.singletonList(mock(QGChangeEvent.class)); + when(qgChangeEventFactory.from(any(QGChangeEventFactory.IssueChangeData.class), eq(issueChange), eq(changeContext))) + .thenReturn(qgChangeEvents); call(issueDto.getKey(), BUG.name()); @@ -117,22 +126,20 @@ public class SetTypeActionTest { IssueDto issueReloaded = dbClient.issueDao().selectByKey(dbTester.getSession(), issueDto.getKey()).get(); assertThat(issueReloaded.getType()).isEqualTo(BUG.getDbConstant()); - ArgumentCaptor issueChangeDataCaptor = ArgumentCaptor.forClass(IssueChangeTrigger.IssueChangeData.class); - verify(issueChangeTrigger).onChange( - issueChangeDataCaptor.capture(), - eq(new IssueChangeTrigger.IssueChange(BUG, null)), - eq(IssueChangeContext.createUser(new Date(now), userSession.getLogin()))); - IssueChangeTrigger.IssueChangeData issueChangeData = issueChangeDataCaptor.getValue(); + ArgumentCaptor issueChangeDataCaptor = ArgumentCaptor.forClass(QGChangeEventFactory.IssueChangeData.class); + verify(qgChangeEventFactory).from(issueChangeDataCaptor.capture(), eq(issueChange), eq(changeContext)); + QGChangeEventFactory.IssueChangeData issueChangeData = issueChangeDataCaptor.getValue(); assertThat(issueChangeData.getIssues()) .extracting(DefaultIssue::key) .containsOnly(issueDto.getKey()); assertThat(issueChangeData.getComponents()) .extracting(ComponentDto::uuid) .containsOnly(issueDto.getComponentUuid(), issueDto.getProjectUuid()); + verify(qgChangeEventListeners).broadcastOnIssueChange(issueChangeData, qgChangeEvents); } @Test - public void insert_entry_in_changelog_when_setting_type() throws Exception { + public void insert_entry_in_changelog_when_setting_type() { IssueDto issueDto = issueDbTester.insertIssue(newIssue().setType(CODE_SMELL)); setUserWithBrowseAndAdministerIssuePermission(issueDto); @@ -156,13 +163,13 @@ public class SetTypeActionTest { } @Test - public void fail_when_not_authenticated() throws Exception { + public void fail_when_not_authenticated() { expectedException.expect(UnauthorizedException.class); call("ABCD", BUG.name()); } @Test - public void fail_when_missing_browse_permission() throws Exception { + public void fail_when_missing_browse_permission() { IssueDto issueDto = issueDbTester.insertIssue(); String login = "john"; String permission = ISSUE_ADMIN; @@ -173,7 +180,7 @@ public class SetTypeActionTest { } @Test - public void fail_when_missing_administer_issue_permission() throws Exception { + public void fail_when_missing_administer_issue_permission() { IssueDto issueDto = issueDbTester.insertIssue(); logInAndAddProjectPermission("john", issueDto, USER); diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/changeevent/IssueChangeTriggerTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/changeevent/IssueChangeTriggerTest.java deleted file mode 100644 index 1a05fce5719..00000000000 --- a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/changeevent/IssueChangeTriggerTest.java +++ /dev/null @@ -1,88 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2017 SonarSource SA - * mailto:info 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.qualitygate.changeevent; - -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; -import org.sonar.api.rules.RuleType; - -import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; -import static org.assertj.core.api.Assertions.assertThat; - -public class IssueChangeTriggerTest { - @Rule - public ExpectedException expectedException = ExpectedException.none(); - - @Test - public void IssueChange_constructor_throws_IAE_if_both_args_are_null() { - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("At least one of ruleType and transitionKey must be non null"); - - new IssueChangeTrigger.IssueChange(null, null); - } - - @Test - public void verify_IssueChange_getters() { - IssueChangeTrigger.IssueChange transitionKeyOnly = new IssueChangeTriggerImpl.IssueChange("foo"); - assertThat(transitionKeyOnly.getTransitionKey()).contains("foo"); - assertThat(transitionKeyOnly.getRuleType()).isEmpty(); - IssueChangeTrigger.IssueChange ruleTypeOnly = new IssueChangeTriggerImpl.IssueChange(RuleType.BUG); - assertThat(ruleTypeOnly.getTransitionKey()).isEmpty(); - assertThat(ruleTypeOnly.getRuleType()).contains(RuleType.BUG); - IssueChangeTrigger.IssueChange transitionKeyAndRuleType = new IssueChangeTriggerImpl.IssueChange(RuleType.VULNERABILITY, "bar"); - assertThat(transitionKeyAndRuleType.getTransitionKey()).contains("bar"); - assertThat(transitionKeyAndRuleType.getRuleType()).contains(RuleType.VULNERABILITY); - } - - @Test - public void verify_IssueChange_equality() { - IssueChangeTrigger.IssueChange underTest = new IssueChangeTrigger.IssueChange(RuleType.BUG); - - assertThat(underTest).isEqualTo(new IssueChangeTrigger.IssueChange(RuleType.BUG)); - assertThat(underTest).isEqualTo(new IssueChangeTrigger.IssueChange(RuleType.BUG, null)); - - assertThat(underTest).isNotEqualTo(null); - assertThat(underTest).isNotEqualTo(new IssueChangeTrigger.IssueChange(RuleType.BUG, randomAlphanumeric(10))); - assertThat(underTest).isNotEqualTo(new IssueChangeTrigger.IssueChange(RuleType.CODE_SMELL)); - assertThat(underTest).isNotEqualTo(new IssueChangeTrigger.IssueChange(RuleType.CODE_SMELL, randomAlphanumeric(10))); - assertThat(underTest).isNotEqualTo(new IssueChangeTrigger.IssueChange(RuleType.VULNERABILITY)); - assertThat(underTest).isNotEqualTo(new IssueChangeTrigger.IssueChange(RuleType.VULNERABILITY, randomAlphanumeric(10))); - assertThat(underTest).isNotEqualTo(new IssueChangeTrigger.IssueChange(randomAlphanumeric(10))); - - String transitionKey = randomAlphanumeric(10); - underTest = new IssueChangeTrigger.IssueChange(transitionKey); - - assertThat(underTest).isEqualTo(new IssueChangeTrigger.IssueChange(transitionKey)); - assertThat(underTest).isEqualTo(new IssueChangeTrigger.IssueChange(null, transitionKey)); - - assertThat(underTest).isNotEqualTo(null); - assertThat(underTest).isNotEqualTo(new IssueChangeTrigger.IssueChange(RuleType.BUG)); - assertThat(underTest).isNotEqualTo(new IssueChangeTrigger.IssueChange(RuleType.BUG, transitionKey)); - assertThat(underTest).isNotEqualTo(new IssueChangeTrigger.IssueChange(RuleType.BUG, randomAlphanumeric(10))); - assertThat(underTest).isNotEqualTo(new IssueChangeTrigger.IssueChange(RuleType.CODE_SMELL)); - assertThat(underTest).isNotEqualTo(new IssueChangeTrigger.IssueChange(RuleType.CODE_SMELL, randomAlphanumeric(10))); - assertThat(underTest).isNotEqualTo(new IssueChangeTrigger.IssueChange(RuleType.CODE_SMELL, transitionKey)); - assertThat(underTest).isNotEqualTo(new IssueChangeTrigger.IssueChange(RuleType.VULNERABILITY)); - assertThat(underTest).isNotEqualTo(new IssueChangeTrigger.IssueChange(RuleType.VULNERABILITY, randomAlphanumeric(10))); - assertThat(underTest).isNotEqualTo(new IssueChangeTrigger.IssueChange(RuleType.VULNERABILITY, transitionKey)); - assertThat(underTest).isNotEqualTo(new IssueChangeTrigger.IssueChange(randomAlphanumeric(9))); - } -} diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/changeevent/IssueChangeTriggerImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/changeevent/QGChangeEventFactoryImplTest.java similarity index 88% rename from server/sonar-server/src/test/java/org/sonar/server/qualitygate/changeevent/IssueChangeTriggerImplTest.java rename to server/sonar-server/src/test/java/org/sonar/server/qualitygate/changeevent/QGChangeEventFactoryImplTest.java index 9489f469510..f18724a2beb 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/changeevent/IssueChangeTriggerImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/changeevent/QGChangeEventFactoryImplTest.java @@ -41,7 +41,6 @@ import javax.annotation.Nullable; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; -import org.mockito.ArgumentCaptor; import org.mockito.Matchers; import org.mockito.Mockito; import org.sonar.api.config.Configuration; @@ -67,7 +66,7 @@ import org.sonar.db.component.SnapshotDto; import org.sonar.db.issue.IssueDto; import org.sonar.db.organization.OrganizationDto; import org.sonar.server.qualitygate.LiveQualityGateFactory; -import org.sonar.server.qualitygate.changeevent.IssueChangeTrigger.IssueChange; +import org.sonar.server.qualitygate.changeevent.QGChangeEventFactory.IssueChange; import org.sonar.server.settings.ProjectConfigurationLoader; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.webhook.WebhookPayloadFactory; @@ -85,7 +84,7 @@ import static org.sonar.core.util.stream.MoreCollectors.toArrayList; import static org.sonar.db.component.ComponentTesting.newBranchDto; @RunWith(DataProviderRunner.class) -public class IssueChangeTriggerImplTest { +public class QGChangeEventFactoryImplTest { @Rule public DbTester dbTester = DbTester.create(System2.INSTANCE); @Rule @@ -101,29 +100,28 @@ public class IssueChangeTriggerImplTest { private WebhookPayloadFactory webhookPayloadFactory = mock(WebhookPayloadFactory.class); private DbClient spiedOnDbClient = Mockito.spy(dbClient); private ProjectConfigurationLoader projectConfigurationLoader = mock(ProjectConfigurationLoader.class); - private QGChangeEventListeners qgChangeEventListeners = mock(QGChangeEventListeners.class); private LiveQualityGateFactory liveQualityGateFactory = mock(LiveQualityGateFactory.class); - private IssueChangeTriggerImpl underTest = new IssueChangeTriggerImpl(spiedOnDbClient, projectConfigurationLoader, qgChangeEventListeners, liveQualityGateFactory); + private QGChangeEventFactoryImpl underTest = new QGChangeEventFactoryImpl(spiedOnDbClient, projectConfigurationLoader, liveQualityGateFactory); private DbClient mockedDbClient = mock(DbClient.class); - private IssueChangeTriggerImpl mockedUnderTest = new IssueChangeTriggerImpl(mockedDbClient, projectConfigurationLoader, qgChangeEventListeners, liveQualityGateFactory); + private QGChangeEventFactoryImpl mockedUnderTest = new QGChangeEventFactoryImpl(mockedDbClient, projectConfigurationLoader, liveQualityGateFactory); @Test public void on_type_change_has_no_effect_if_SearchResponseData_has_no_issue() { - mockedUnderTest.onChange(issueChangeData(), new IssueChange(randomRuleType), userChangeContext); + mockedUnderTest.from(issueChangeData(), new IssueChange(randomRuleType), userChangeContext); Mockito.verifyZeroInteractions(mockedDbClient, projectConfigurationLoader, webhookPayloadFactory); } @Test public void on_type_change_has_no_effect_if_scan_IssueChangeContext() { - mockedUnderTest.onChange(issueChangeData(), new IssueChange(randomRuleType), scanChangeContext); + mockedUnderTest.from(issueChangeData(), new IssueChange(randomRuleType), scanChangeContext); Mockito.verifyZeroInteractions(mockedDbClient, projectConfigurationLoader, webhookPayloadFactory); } @Test public void on_transition_change_has_no_effect_if_SearchResponseData_has_no_issue() { - mockedUnderTest.onChange(issueChangeData(), new IssueChange(randomAlphanumeric(12)), userChangeContext); + mockedUnderTest.from(issueChangeData(), new IssueChange(randomAlphanumeric(12)), userChangeContext); Mockito.verifyZeroInteractions(mockedDbClient, projectConfigurationLoader, webhookPayloadFactory); } @@ -150,28 +148,28 @@ public class IssueChangeTriggerImplTest { private void on_transition_changeHasNoEffectForTransitionKey(@Nullable String transitionKey) { Mockito.reset(mockedDbClient, projectConfigurationLoader, webhookPayloadFactory); - mockedUnderTest.onChange(issueChangeData(newIssueDto()), new IssueChange(transitionKey), userChangeContext); + mockedUnderTest.from(issueChangeData(newIssueDto()), new IssueChange(transitionKey), userChangeContext); Mockito.verifyZeroInteractions(mockedDbClient, projectConfigurationLoader, webhookPayloadFactory); } @Test public void on_transition_change_has_no_effect_if_scan_IssueChangeContext() { - mockedUnderTest.onChange(issueChangeData(newIssueDto()), new IssueChange(randomAlphanumeric(12)), scanChangeContext); + mockedUnderTest.from(issueChangeData(newIssueDto()), new IssueChange(randomAlphanumeric(12)), scanChangeContext); Mockito.verifyZeroInteractions(mockedDbClient, projectConfigurationLoader, webhookPayloadFactory); } @Test public void on_type_and_transition_change_has_no_effect_if_SearchResponseData_has_no_issue() { - mockedUnderTest.onChange(issueChangeData(), new IssueChange(randomRuleType, randomAlphanumeric(3)), userChangeContext); + mockedUnderTest.from(issueChangeData(), new IssueChange(randomRuleType, randomAlphanumeric(3)), userChangeContext); Mockito.verifyZeroInteractions(mockedDbClient, projectConfigurationLoader, webhookPayloadFactory); } @Test public void on_type_and_transition_change_has_no_effect_if_scan_IssueChangeContext() { - mockedUnderTest.onChange(issueChangeData(), new IssueChange(randomRuleType, randomAlphanumeric(3)), scanChangeContext); + mockedUnderTest.from(issueChangeData(), new IssueChange(randomRuleType, randomAlphanumeric(3)), scanChangeContext); Mockito.verifyZeroInteractions(mockedDbClient, projectConfigurationLoader, webhookPayloadFactory); } @@ -198,7 +196,7 @@ public class IssueChangeTriggerImplTest { private void on_type_and_transition_changeHasNoEffectForTransitionKey(@Nullable String transitionKey) { Mockito.reset(mockedDbClient, projectConfigurationLoader, webhookPayloadFactory); - mockedUnderTest.onChange(issueChangeData(newIssueDto()), new IssueChange(randomRuleType, transitionKey), userChangeContext); + mockedUnderTest.from(issueChangeData(newIssueDto()), new IssueChange(randomRuleType, transitionKey), userChangeContext); Mockito.verifyZeroInteractions(mockedDbClient, projectConfigurationLoader, webhookPayloadFactory); } @@ -217,9 +215,8 @@ public class IssueChangeTriggerImplTest { properties.put("sonar.analysis.test2", randomAlphanumeric(5000)); insertPropertiesFor(analysis.getUuid(), properties); - underTest.onChange(issueChangeData(newIssueDto(branch)), issueChange, userChangeContext); + Collection events = underTest.from(issueChangeData(newIssueDto(branch)), issueChange, userChangeContext); - Collection events = verifyListenersBroadcastedTo(); assertThat(events).hasSize(1); QGChangeEvent event = events.iterator().next(); assertThat(event.getProject()).isEqualTo(branch.component); @@ -238,7 +235,7 @@ public class IssueChangeTriggerImplTest { SnapshotDao snapshotDaoSpy = Mockito.spy(dbClient.snapshotDao()); Mockito.when(spiedOnDbClient.snapshotDao()).thenReturn(snapshotDaoSpy); - underTest.onChange(issueChangeData(issueDtos), new IssueChange(randomRuleType), userChangeContext); + underTest.from(issueChangeData(issueDtos), new IssueChange(randomRuleType), userChangeContext); Mockito.verifyZeroInteractions(projectConfigurationLoader); Mockito.verify(snapshotDaoSpy, Mockito.times(0)).selectLastAnalysesByRootComponentUuids(Matchers.any(DbSession.class), Matchers.anyCollectionOf(String.class)); @@ -276,9 +273,9 @@ public class IssueChangeTriggerImplTest { Mockito.when(spiedOnDbClient.componentDao()).thenReturn(componentDaoSpy); Mockito.when(spiedOnDbClient.branchDao()).thenReturn(branchDaoSpy); Mockito.when(spiedOnDbClient.snapshotDao()).thenReturn(snapshotDaoSpy); - underTest.onChange(issueChangeData(issueDtos), new IssueChange(randomRuleType), userChangeContext); + Collection qgChangeEvents = underTest.from(issueChangeData(issueDtos), + new IssueChange(randomRuleType), userChangeContext); - Collection qgChangeEvents = verifyListenersBroadcastedTo(); assertThat(qgChangeEvents) .hasSize(3) .extracting(QGChangeEvent::getBranch, QGChangeEvent::getProjectConfiguration, QGChangeEvent::getAnalysis) @@ -314,12 +311,11 @@ public class IssueChangeTriggerImplTest { Mockito.when(spiedOnDbClient.componentDao()).thenReturn(componentDaoSpy); Mockito.when(spiedOnDbClient.branchDao()).thenReturn(branchDaoSpy); Mockito.when(spiedOnDbClient.snapshotDao()).thenReturn(snapshotDaoSpy); - underTest.onChange( + Collection qgChangeEvents = underTest.from( issueChangeData(asList(newIssueDto(shortBranch), newIssueDto(longBranch))), new IssueChange(randomRuleType), userChangeContext); - Collection qgChangeEvents = verifyListenersBroadcastedTo(); assertThat(qgChangeEvents) .hasSize(1) .extracting(QGChangeEvent::getBranch, QGChangeEvent::getProjectConfiguration, QGChangeEvent::getAnalysis) @@ -352,12 +348,11 @@ public class IssueChangeTriggerImplTest { ComponentDao componentDaoSpy = Mockito.spy(dbClient.componentDao()); Mockito.when(spiedOnDbClient.componentDao()).thenReturn(componentDaoSpy); - underTest.onChange( + Collection qgChangeEvents = underTest.from( issueChangeData(issueDtos, branch1, branch2, branch3), new IssueChange(randomRuleType), userChangeContext); - Collection qgChangeEvents = verifyListenersBroadcastedTo(); assertThat(qgChangeEvents) .hasSize(3) .extracting(QGChangeEvent::getBranch, QGChangeEvent::getProjectConfiguration, QGChangeEvent::getAnalysis) @@ -394,12 +389,12 @@ public class IssueChangeTriggerImplTest { Mockito.when(spiedOnDbClient.componentDao()).thenReturn(componentDaoSpy); Mockito.when(spiedOnDbClient.branchDao()).thenReturn(branchDaoSpy); Mockito.when(spiedOnDbClient.snapshotDao()).thenReturn(snapshotDaoSpy); - underTest.onChange( + Collection qgChangeEvents = underTest.from( issueChangeData(issueDtos, branch1, branch3), new IssueChange(randomRuleType), userChangeContext); - assertThat(verifyListenersBroadcastedTo()).hasSize(3); + assertThat(qgChangeEvents).hasSize(3); Set uuids = ImmutableSet.of(branch1.uuid(), branch2.uuid(), branch3.uuid()); Mockito.verify(componentDaoSpy).selectByUuids(Matchers.any(DbSession.class), Matchers.eq(ImmutableSet.of(branch2.uuid()))); @@ -432,9 +427,9 @@ public class IssueChangeTriggerImplTest { Mockito.when(spiedOnDbClient.componentDao()).thenReturn(componentDaoSpy); Mockito.when(spiedOnDbClient.branchDao()).thenReturn(branchDaoSpy); Mockito.when(spiedOnDbClient.snapshotDao()).thenReturn(snapshotDaoSpy); - underTest.onChange(issueChangeData(issueDtos), new IssueChange(randomRuleType), userChangeContext); + Collection qgChangeEvents = underTest.from(issueChangeData(issueDtos), + new IssueChange(randomRuleType), userChangeContext); - Collection qgChangeEvents = verifyListenersBroadcastedTo(); assertThat(qgChangeEvents) .hasSize(1) .extracting(QGChangeEvent::getBranch, QGChangeEvent::getProjectConfiguration, QGChangeEvent::getAnalysis) @@ -526,15 +521,6 @@ public class IssueChangeTriggerImplTest { return dbTester.components().insertSnapshot(component); } - private Collection verifyListenersBroadcastedTo() { - Class> clazz = (Class>) (Class) Collection.class; - ArgumentCaptor> supplierCaptor = ArgumentCaptor.forClass(clazz); - Mockito.verify(qgChangeEventListeners).broadcast( - Matchers.same(Trigger.ISSUE_CHANGE), - supplierCaptor.capture()); - return supplierCaptor.getValue(); - } - @DataProvider public static Object[][] validIssueChanges() { return new Object[][] { @@ -560,16 +546,16 @@ public class IssueChangeTriggerImplTest { }; } - private IssueChangeTrigger.IssueChangeData issueChangeData() { - return new IssueChangeTrigger.IssueChangeData(emptyList(), emptyList()); + private QGChangeEventFactory.IssueChangeData issueChangeData() { + return new QGChangeEventFactory.IssueChangeData(emptyList(), emptyList()); } - private IssueChangeTrigger.IssueChangeData issueChangeData(IssueDto issueDto) { - return new IssueChangeTrigger.IssueChangeData(singletonList(issueDto.toDefaultIssue()), emptyList()); + private QGChangeEventFactory.IssueChangeData issueChangeData(IssueDto issueDto) { + return new QGChangeEventFactory.IssueChangeData(singletonList(issueDto.toDefaultIssue()), emptyList()); } - private IssueChangeTrigger.IssueChangeData issueChangeData(Collection issueDtos, ComponentAndBranch... components) { - return new IssueChangeTrigger.IssueChangeData( + private QGChangeEventFactory.IssueChangeData issueChangeData(Collection issueDtos, ComponentAndBranch... components) { + return new QGChangeEventFactory.IssueChangeData( issueDtos.stream().map(IssueDto::toDefaultIssue).collect(Collectors.toList()), Arrays.stream(components).map(ComponentAndBranch::getComponent).collect(Collectors.toList())); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/changeevent/QGChangeEventFactoryTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/changeevent/QGChangeEventFactoryTest.java new file mode 100644 index 00000000000..55214d07b2b --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/changeevent/QGChangeEventFactoryTest.java @@ -0,0 +1,88 @@ +/* + * SonarQube + * Copyright (C) 2009-2017 SonarSource SA + * mailto:info 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.qualitygate.changeevent; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.sonar.api.rules.RuleType; + +import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; +import static org.assertj.core.api.Assertions.assertThat; + +public class QGChangeEventFactoryTest { + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + @Test + public void IssueChange_constructor_throws_IAE_if_both_args_are_null() { + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("At least one of ruleType and transitionKey must be non null"); + + new QGChangeEventFactory.IssueChange(null, null); + } + + @Test + public void verify_IssueChange_getters() { + QGChangeEventFactory.IssueChange transitionKeyOnly = new QGChangeEventFactoryImpl.IssueChange("foo"); + assertThat(transitionKeyOnly.getTransitionKey()).contains("foo"); + assertThat(transitionKeyOnly.getRuleType()).isEmpty(); + QGChangeEventFactory.IssueChange ruleTypeOnly = new QGChangeEventFactoryImpl.IssueChange(RuleType.BUG); + assertThat(ruleTypeOnly.getTransitionKey()).isEmpty(); + assertThat(ruleTypeOnly.getRuleType()).contains(RuleType.BUG); + QGChangeEventFactory.IssueChange transitionKeyAndRuleType = new QGChangeEventFactoryImpl.IssueChange(RuleType.VULNERABILITY, "bar"); + assertThat(transitionKeyAndRuleType.getTransitionKey()).contains("bar"); + assertThat(transitionKeyAndRuleType.getRuleType()).contains(RuleType.VULNERABILITY); + } + + @Test + public void verify_IssueChange_equality() { + QGChangeEventFactory.IssueChange underTest = new QGChangeEventFactory.IssueChange(RuleType.BUG); + + assertThat(underTest).isEqualTo(new QGChangeEventFactory.IssueChange(RuleType.BUG)); + assertThat(underTest).isEqualTo(new QGChangeEventFactory.IssueChange(RuleType.BUG, null)); + + assertThat(underTest).isNotEqualTo(null); + assertThat(underTest).isNotEqualTo(new QGChangeEventFactory.IssueChange(RuleType.BUG, randomAlphanumeric(10))); + assertThat(underTest).isNotEqualTo(new QGChangeEventFactory.IssueChange(RuleType.CODE_SMELL)); + assertThat(underTest).isNotEqualTo(new QGChangeEventFactory.IssueChange(RuleType.CODE_SMELL, randomAlphanumeric(10))); + assertThat(underTest).isNotEqualTo(new QGChangeEventFactory.IssueChange(RuleType.VULNERABILITY)); + assertThat(underTest).isNotEqualTo(new QGChangeEventFactory.IssueChange(RuleType.VULNERABILITY, randomAlphanumeric(10))); + assertThat(underTest).isNotEqualTo(new QGChangeEventFactory.IssueChange(randomAlphanumeric(10))); + + String transitionKey = randomAlphanumeric(10); + underTest = new QGChangeEventFactory.IssueChange(transitionKey); + + assertThat(underTest).isEqualTo(new QGChangeEventFactory.IssueChange(transitionKey)); + assertThat(underTest).isEqualTo(new QGChangeEventFactory.IssueChange(null, transitionKey)); + + assertThat(underTest).isNotEqualTo(null); + assertThat(underTest).isNotEqualTo(new QGChangeEventFactory.IssueChange(RuleType.BUG)); + assertThat(underTest).isNotEqualTo(new QGChangeEventFactory.IssueChange(RuleType.BUG, transitionKey)); + assertThat(underTest).isNotEqualTo(new QGChangeEventFactory.IssueChange(RuleType.BUG, randomAlphanumeric(10))); + assertThat(underTest).isNotEqualTo(new QGChangeEventFactory.IssueChange(RuleType.CODE_SMELL)); + assertThat(underTest).isNotEqualTo(new QGChangeEventFactory.IssueChange(RuleType.CODE_SMELL, randomAlphanumeric(10))); + assertThat(underTest).isNotEqualTo(new QGChangeEventFactory.IssueChange(RuleType.CODE_SMELL, transitionKey)); + assertThat(underTest).isNotEqualTo(new QGChangeEventFactory.IssueChange(RuleType.VULNERABILITY)); + assertThat(underTest).isNotEqualTo(new QGChangeEventFactory.IssueChange(RuleType.VULNERABILITY, randomAlphanumeric(10))); + assertThat(underTest).isNotEqualTo(new QGChangeEventFactory.IssueChange(RuleType.VULNERABILITY, transitionKey)); + assertThat(underTest).isNotEqualTo(new QGChangeEventFactory.IssueChange(randomAlphanumeric(9))); + } +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListenersImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListenersImplTest.java index 9df7be5af3b..64fee2db04e 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListenersImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListenersImplTest.java @@ -32,7 +32,11 @@ import org.mockito.InOrder; import org.mockito.Mockito; import org.sonar.api.utils.log.LogTester; import org.sonar.api.utils.log.LoggerLevel; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.db.component.ComponentDto; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doThrow; @@ -47,6 +51,7 @@ public class QGChangeEventListenersImplTest { private QGChangeEventListener listener1 = mock(QGChangeEventListener.class); private QGChangeEventListener listener2 = mock(QGChangeEventListener.class); private QGChangeEventListener listener3 = mock(QGChangeEventListener.class); + private QGChangeEventFactory.IssueChangeData issueChangeData = new QGChangeEventFactory.IssueChangeData(singletonList(new DefaultIssue()), singletonList(new ComponentDto())); private InOrder inOrder = Mockito.inOrder(listener1, listener2, listener3); private List threeChangeEvents = Arrays.asList(mock(QGChangeEvent.class), mock(QGChangeEvent.class)); @@ -74,15 +79,43 @@ public class QGChangeEventListenersImplTest { } @Test - public void no_effect_when_no_changeEvent() { - underTest.broadcast(Trigger.ISSUE_CHANGE, Collections.emptySet()); + public void broadcastOnIssueChange_has_no_effect_when_issueChangeData_is_empty() { + QGChangeEventFactory.IssueChangeData issueChangeData = new QGChangeEventFactory.IssueChangeData(emptyList(), emptyList()); + + underTest.broadcastOnIssueChange(issueChangeData, threeChangeEvents); + + verifyZeroInteractions(listener1, listener2, listener3); + } + + @Test + public void broadcastOnIssueChange_has_no_effect_when_issueChangeData_has_no_issue() { + QGChangeEventFactory.IssueChangeData issueChangeData = new QGChangeEventFactory.IssueChangeData(emptyList(), singletonList(new ComponentDto())); + + underTest.broadcastOnIssueChange(issueChangeData, threeChangeEvents); + + verifyZeroInteractions(listener1, listener2, listener3); + } + + @Test + public void broadcastOnIssueChange_has_no_effect_when_issueChangeData_has_no_component() { + QGChangeEventFactory.IssueChangeData issueChangeData = new QGChangeEventFactory.IssueChangeData(singletonList(new DefaultIssue()), emptyList()); + + underTest.broadcastOnIssueChange(issueChangeData, threeChangeEvents); + + verifyZeroInteractions(listener1, listener2, listener3); + } + + @Test + public void broadcastOnIssueChange_has_no_effect_when_no_changeEvent() { + + underTest.broadcastOnIssueChange(issueChangeData, Collections.emptySet()); verifyZeroInteractions(listener1, listener2, listener3); } @Test - public void broadcast_passes_Trigger_and_collection_to_all_listeners_in_order_of_addition_to_constructor() { - underTest.broadcast(Trigger.ISSUE_CHANGE, threeChangeEvents); + public void broadcastOnIssueChange_passes_Trigger_and_collection_to_all_listeners_in_order_of_addition_to_constructor() { + underTest.broadcastOnIssueChange(issueChangeData, threeChangeEvents); inOrder.verify(listener1).onChanges(Trigger.ISSUE_CHANGE, threeChangeEvents); inOrder.verify(listener2).onChanges(Trigger.ISSUE_CHANGE, threeChangeEvents); @@ -91,13 +124,13 @@ public class QGChangeEventListenersImplTest { } @Test - public void broadcast_calls_all_listeners_even_if_one_throws_an_exception() { + public void broadcastOnIssueChange_calls_all_listeners_even_if_one_throws_an_exception() { QGChangeEventListener failingListener = new QGChangeEventListener[] {listener1, listener2, listener3}[new Random().nextInt(3)]; doThrow(new RuntimeException("Faking an exception thrown by onChanges")) .when(failingListener) .onChanges(Trigger.ISSUE_CHANGE, threeChangeEvents); - underTest.broadcast(Trigger.ISSUE_CHANGE, threeChangeEvents); + underTest.broadcastOnIssueChange(issueChangeData, threeChangeEvents); inOrder.verify(listener1).onChanges(Trigger.ISSUE_CHANGE, threeChangeEvents); inOrder.verify(listener2).onChanges(Trigger.ISSUE_CHANGE, threeChangeEvents); @@ -108,12 +141,12 @@ public class QGChangeEventListenersImplTest { } @Test - public void broadcast_stops_calling_listeners_when_one_throws_an_ERROR() { + public void broadcastOnIssueChange_stops_calling_listeners_when_one_throws_an_ERROR() { doThrow(new Error("Faking an error thrown by a listener")) .when(listener2) .onChanges(Trigger.ISSUE_CHANGE, threeChangeEvents); - underTest.broadcast(Trigger.ISSUE_CHANGE, threeChangeEvents); + underTest.broadcastOnIssueChange(issueChangeData, threeChangeEvents); inOrder.verify(listener1).onChanges(Trigger.ISSUE_CHANGE, threeChangeEvents); inOrder.verify(listener2).onChanges(Trigger.ISSUE_CHANGE, threeChangeEvents); @@ -123,8 +156,8 @@ public class QGChangeEventListenersImplTest { } @Test - public void broadcast_logs_each_listener_call_at_TRACE_level() { - underTest.broadcast(Trigger.ISSUE_CHANGE, threeChangeEvents); + public void broadcastOnIssueChange_logs_each_listener_call_at_TRACE_level() { + underTest.broadcastOnIssueChange(issueChangeData, threeChangeEvents); assertThat(logTester.logs()).hasSize(3); List traceLogs = logTester.logs(LoggerLevel.TRACE); @@ -136,10 +169,10 @@ public class QGChangeEventListenersImplTest { } @Test - public void broadcast_passes_immutable_list_of_events() { + public void broadcastOnIssueChange_passes_immutable_list_of_events() { QGChangeEventListenersImpl underTest = new QGChangeEventListenersImpl(new QGChangeEventListener[] {listener1}); - underTest.broadcast(Trigger.ISSUE_CHANGE, threeChangeEvents); + underTest.broadcastOnIssueChange(issueChangeData, threeChangeEvents); ArgumentCaptor collectionCaptor = ArgumentCaptor.forClass(Collection.class); verify(listener1).onChanges(eq(Trigger.ISSUE_CHANGE), collectionCaptor.capture()); @@ -147,10 +180,10 @@ public class QGChangeEventListenersImplTest { } @Test - public void no_effect_when_no_listener() { + public void broadcastOnIssueChange_has_no_effect_when_no_listener() { QGChangeEventListenersImpl underTest = new QGChangeEventListenersImpl(); - underTest.broadcast(Trigger.ISSUE_CHANGE, Collections.emptySet()); + underTest.broadcastOnIssueChange(issueChangeData, threeChangeEvents); verifyZeroInteractions(listener1, listener2, listener3); } -- 2.39.5