From 275b95e15ef2edd7287db9e3551f67a3103dbe7c Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Mon, 2 Oct 2017 16:57:46 +0200 Subject: [PATCH] SONAR-9872 call webhook on bulk issue change on short lived branch --- .../issue/webhook/IssueChangeWebhook.java | 79 ++++++- .../issue/webhook/IssueChangeWebhookImpl.java | 31 +-- .../server/issue/ws/BulkChangeAction.java | 32 ++- .../server/issue/ws/DoTransitionAction.java | 16 +- .../sonar/server/issue/ws/SetTypeAction.java | 16 +- .../webhook/IssueChangeWebhookImplTest.java | 217 +++++++++++++++--- .../server/issue/ws/BulkChangeActionTest.java | 55 ++++- .../issue/ws/DoTransitionActionTest.java | 22 +- .../server/issue/ws/SetTypeActionTest.java | 21 +- .../sonar/core/issue/IssueChangeContext.java | 29 +++ 10 files changed, 460 insertions(+), 58 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/webhook/IssueChangeWebhook.java b/server/sonar-server/src/main/java/org/sonar/server/issue/webhook/IssueChangeWebhook.java index edb30e69dd7..6d509716194 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/webhook/IssueChangeWebhook.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/webhook/IssueChangeWebhook.java @@ -19,10 +19,15 @@ */ package org.sonar.server.issue.webhook; +import com.google.common.collect.ImmutableList; +import java.util.List; +import java.util.Objects; import java.util.Optional; import javax.annotation.Nullable; import org.sonar.api.rules.RuleType; +import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.IssueChangeContext; +import org.sonar.db.component.ComponentDto; import org.sonar.server.issue.ws.SearchResponseData; import static com.google.common.base.Preconditions.checkArgument; @@ -32,7 +37,7 @@ public interface IssueChangeWebhook { * 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(SearchResponseData searchResponseData, IssueChange issueChange, IssueChangeContext context); + void onChange(IssueChangeData issueChangeData, IssueChange issueChange, IssueChangeContext context); final class IssueChange { private final RuleType ruleType; @@ -46,7 +51,7 @@ public interface IssueChangeWebhook { this(null, transitionKey); } - IssueChange(@Nullable RuleType ruleType, @Nullable String transitionKey) { + public IssueChange(@Nullable RuleType ruleType, @Nullable String transitionKey) { checkArgument(ruleType != null || transitionKey != null, "At least one of ruleType and transitionKey must be non null"); this.ruleType = ruleType; this.transitionKey = transitionKey; @@ -59,5 +64,75 @@ public interface IssueChangeWebhook { public Optional getTransitionKey() { return Optional.ofNullable(transitionKey); } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + IssueChange that = (IssueChange) o; + return ruleType == that.ruleType && + Objects.equals(transitionKey, that.transitionKey); + } + + @Override + public int hashCode() { + return Objects.hash(ruleType, transitionKey); + } + + @Override + public String toString() { + return "IssueChange{" + + "ruleType=" + ruleType + + ", transitionKey='" + transitionKey + '\'' + + '}'; + } + } + + final class IssueChangeData { + private final List issues; + private final List components; + + public IssueChangeData(List issues, List components) { + this.issues = ImmutableList.copyOf(issues); + this.components = ImmutableList.copyOf(components); + } + + public List getIssues() { + return issues; + } + + public List getComponents() { + return components; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + IssueChangeData that = (IssueChangeData) o; + return Objects.equals(issues, that.issues) && + Objects.equals(components, that.components); + } + + @Override + public int hashCode() { + return Objects.hash(issues, components); + } + + @Override + public String toString() { + return "IssueChangeData{" + + "issues=" + issues + + ", components=" + components + + '}'; + } } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/webhook/IssueChangeWebhookImpl.java b/server/sonar-server/src/main/java/org/sonar/server/issue/webhook/IssueChangeWebhookImpl.java index a27c1ae030f..b99829800d1 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/webhook/IssueChangeWebhookImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/webhook/IssueChangeWebhookImpl.java @@ -32,6 +32,7 @@ import org.sonar.api.config.Configuration; import org.sonar.api.issue.DefaultTransitions; import org.sonar.api.measures.CoreMetrics; import org.sonar.api.rules.RuleType; +import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.IssueChangeContext; import org.sonar.core.util.stream.MoreCollectors; import org.sonar.db.DbClient; @@ -40,13 +41,11 @@ import org.sonar.db.component.BranchDto; import org.sonar.db.component.BranchType; import org.sonar.db.component.ComponentDto; import org.sonar.db.component.SnapshotDto; -import org.sonar.db.issue.IssueDto; import org.sonar.db.qualitygate.QualityGateConditionDto; import org.sonar.server.es.Facets; import org.sonar.server.es.SearchOptions; import org.sonar.server.issue.IssueQuery; import org.sonar.server.issue.index.IssueIndex; -import org.sonar.server.issue.ws.SearchResponseData; import org.sonar.server.qualitygate.ShortLivingBranchQualityGate; import org.sonar.server.rule.index.RuleIndex; import org.sonar.server.webhook.Analysis; @@ -83,20 +82,20 @@ public class IssueChangeWebhookImpl implements IssueChangeWebhook { } @Override - public void onChange(SearchResponseData searchResponseData, IssueChange issueChange, IssueChangeContext context) { - if (isEmpty(searchResponseData) || !isUserChangeContext(context) || !isRelevant(issueChange)) { + public void onChange(IssueChangeData issueChangeData, IssueChange issueChange, IssueChangeContext context) { + if (isEmpty(issueChangeData) || !isUserChangeContext(context) || !isRelevant(issueChange)) { return; } - callWebHook(searchResponseData); + callWebHook(issueChangeData); } private static boolean isRelevant(IssueChange issueChange) { return issueChange.getTransitionKey().map(IssueChangeWebhookImpl::isMeaningfulTransition).orElse(true); } - private static boolean isEmpty(SearchResponseData searchResponseData) { - return searchResponseData.getIssues().isEmpty(); + private static boolean isEmpty(IssueChangeData issueChangeData) { + return issueChangeData.getIssues().isEmpty(); } private static boolean isUserChangeContext(IssueChangeContext context) { @@ -107,16 +106,16 @@ public class IssueChangeWebhookImpl implements IssueChangeWebhook { return MEANINGFUL_TRANSITIONS.contains(transitionKey); } - private void callWebHook(SearchResponseData searchResponseData) { + private void callWebHook(IssueChangeData issueChangeData) { if (!webhooks.isEnabled(configuration)) { return; } - Set componentUuids = searchResponseData.getIssues().stream() - .map(IssueDto::getComponentUuid) + Set componentUuids = issueChangeData.getIssues().stream() + .map(DefaultIssue::projectUuid) .collect(toSet()); try (DbSession dbSession = dbClient.openSession(false)) { - Map branchesByUuid = getBranchComponents(dbSession, componentUuids, searchResponseData); + Map branchesByUuid = getBranchComponents(dbSession, componentUuids, issueChangeData); if (branchesByUuid.isEmpty()) { return; } @@ -236,20 +235,22 @@ public class IssueChangeWebhookImpl implements IssueChangeWebhook { return res; } - private Map getBranchComponents(DbSession dbSession, Set componentUuids, SearchResponseData searchResponseData) { + private Map getBranchComponents(DbSession dbSession, Set componentUuids, IssueChangeData issueChangeData) { Set missingComponentUuids = ImmutableSet.copyOf(Sets.difference( componentUuids, - searchResponseData.getComponents() + issueChangeData.getComponents() .stream() .map(ComponentDto::uuid) .collect(Collectors.toSet()))); if (missingComponentUuids.isEmpty()) { - return searchResponseData.getComponents() + return issueChangeData.getComponents() .stream() + .filter(c -> componentUuids.contains(c.uuid())) + .filter(componentDto -> componentDto.getMainBranchProjectUuid() != null) .collect(uniqueIndex(ComponentDto::uuid)); } return Stream.concat( - searchResponseData.getComponents().stream(), + issueChangeData.getComponents().stream().filter(c -> componentUuids.contains(c.uuid())), dbClient.componentDao().selectByUuids(dbSession, missingComponentUuids).stream()) .filter(componentDto -> componentDto.getMainBranchProjectUuid() != null) .collect(uniqueIndex(ComponentDto::uuid)); 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 a46c453ce4c..60ad453ba28 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 @@ -56,12 +56,16 @@ import org.sonar.server.issue.AddTagsAction; import org.sonar.server.issue.AssignAction; import org.sonar.server.issue.IssueStorage; import org.sonar.server.issue.RemoveTagsAction; +import org.sonar.server.issue.SetTypeAction; +import org.sonar.server.issue.TransitionAction; import org.sonar.server.issue.notification.IssueChangeNotification; +import org.sonar.server.issue.webhook.IssueChangeWebhook; import org.sonar.server.notification.NotificationManager; import org.sonar.server.user.UserSession; import org.sonarqube.ws.Issues; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.collect.ImmutableList.copyOf; import static com.google.common.collect.ImmutableMap.of; import static java.lang.String.format; import static java.util.function.Function.identity; @@ -105,14 +109,17 @@ public class BulkChangeAction implements IssuesWsAction { private final IssueStorage issueStorage; private final NotificationManager notificationService; private final List actions; + private final IssueChangeWebhook issueChangeWebhook; - public BulkChangeAction(System2 system2, UserSession userSession, DbClient dbClient, IssueStorage issueStorage, NotificationManager notificationService, List actions) { + public BulkChangeAction(System2 system2, UserSession userSession, DbClient dbClient, IssueStorage issueStorage, NotificationManager notificationService, List actions, + IssueChangeWebhook issueChangeWebhook) { this.system2 = system2; this.userSession = userSession; this.dbClient = dbClient; this.issueStorage = issueStorage; this.notificationService = notificationService; this.actions = actions; + this.issueChangeWebhook = issueChangeWebhook; } @Override @@ -198,11 +205,32 @@ public class BulkChangeAction implements IssuesWsAction { .collect(MoreCollectors.toList()); issueStorage.save(items); items.forEach(sendNotification(issueChangeContext, bulkChangeData)); + buildWebhookIssueChange(bulkChangeData.propertiesByActions) + .ifPresent(issueChange -> issueChangeWebhook.onChange( + new IssueChangeWebhook.IssueChangeData( + bulkChangeData.issues.stream().filter(i -> result.success.contains(i.key())).collect(MoreCollectors.toList()), + copyOf(bulkChangeData.componentsByUuid.values())), + issueChange, + issueChangeContext)); return result; }; } - private Predicate bulkChange(IssueChangeContext issueChangeContext, BulkChangeData bulkChangeData, BulkChangeResult result) { + 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) + .orElse(null); + String transitionKey = Optional.ofNullable(propertiesByActions.get(TransitionAction.DO_TRANSITION_KEY)) + .map(t -> (String) t.get(TransitionAction.TRANSITION_PARAMETER)) + .orElse(null); + if (ruleType == null && transitionKey == null) { + return Optional.empty(); + } + return Optional.of(new IssueChangeWebhook.IssueChange(ruleType, transitionKey)); + } + + private static Predicate bulkChange(IssueChangeContext issueChangeContext, BulkChangeData bulkChangeData, BulkChangeResult result) { return issue -> { ActionContext actionContext = new ActionContext(issue, issueChangeContext, bulkChangeData.projectsByUuid.get(issue.projectUuid())); bulkChangeData.getActionsWithoutComment().forEach(applyAction(actionContext, bulkChangeData, 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 6da81b9afd0..201ca3d98e5 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 @@ -26,9 +26,11 @@ import org.sonar.api.server.ws.Change; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; +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; @@ -38,6 +40,7 @@ import org.sonar.server.issue.TransitionService; import org.sonar.server.issue.webhook.IssueChangeWebhook; import org.sonar.server.user.UserSession; +import static com.google.common.collect.ImmutableList.copyOf; 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; @@ -50,16 +53,18 @@ public class DoTransitionAction implements IssuesWsAction { private final IssueUpdater issueUpdater; private final TransitionService transitionService; private final OperationResponseWriter responseWriter; + private final System2 system2; private final IssueChangeWebhook issueChangeWebhook; public DoTransitionAction(DbClient dbClient, UserSession userSession, IssueFinder issueFinder, IssueUpdater issueUpdater, TransitionService transitionService, - OperationResponseWriter responseWriter, IssueChangeWebhook issueChangeWebhook) { + OperationResponseWriter responseWriter, System2 system2, IssueChangeWebhook issueChangeWebhook) { this.dbClient = dbClient; this.userSession = userSession; this.issueFinder = issueFinder; this.issueUpdater = issueUpdater; this.transitionService = transitionService; this.responseWriter = responseWriter; + this.system2 = system2; this.issueChangeWebhook = issueChangeWebhook; } @@ -99,11 +104,16 @@ public class DoTransitionAction implements IssuesWsAction { private SearchResponseData doTransition(DbSession session, IssueDto issueDto, String transitionKey) { DefaultIssue defaultIssue = issueDto.toDefaultIssue(); - IssueChangeContext context = IssueChangeContext.createUser(new Date(), userSession.getLogin()); + IssueChangeContext context = IssueChangeContext.createUser(new Date(system2.now()), userSession.getLogin()); transitionService.checkTransitionPermission(transitionKey, defaultIssue); if (transitionService.doTransition(defaultIssue, context, transitionKey)) { SearchResponseData searchResponseData = issueUpdater.saveIssueAndPreloadSearchResponseData(session, defaultIssue, context, null); - issueChangeWebhook.onChange(searchResponseData, new IssueChangeWebhook.IssueChange(transitionKey), context); + issueChangeWebhook.onChange( + new IssueChangeWebhook.IssueChangeData( + searchResponseData.getIssues().stream().map(IssueDto::toDefaultIssue).collect(MoreCollectors.toList(searchResponseData.getIssues().size())), + copyOf(searchResponseData.getComponents())), + new IssueChangeWebhook.IssueChange(transitionKey), + context); return searchResponseData; } return new SearchResponseData(issueDto); 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 c9512b35bab..6554cdac0c8 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 @@ -26,9 +26,11 @@ import org.sonar.api.server.ws.Change; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; +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; @@ -38,6 +40,7 @@ import org.sonar.server.issue.IssueUpdater; import org.sonar.server.issue.webhook.IssueChangeWebhook; import org.sonar.server.user.UserSession; +import static com.google.common.collect.ImmutableList.copyOf; import static org.sonar.api.web.UserRole.ISSUE_ADMIN; import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_SET_TYPE; import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_ISSUE; @@ -51,16 +54,18 @@ public class SetTypeAction implements IssuesWsAction { private final IssueFieldsSetter issueFieldsSetter; private final IssueUpdater issueUpdater; private final OperationResponseWriter responseWriter; + private final System2 system2; private final IssueChangeWebhook issueChangeWebhook; public SetTypeAction(UserSession userSession, DbClient dbClient, IssueFinder issueFinder, IssueFieldsSetter issueFieldsSetter, IssueUpdater issueUpdater, - OperationResponseWriter responseWriter, IssueChangeWebhook issueChangeWebhook) { + OperationResponseWriter responseWriter, System2 system2, IssueChangeWebhook issueChangeWebhook) { this.userSession = userSession; this.dbClient = dbClient; this.issueFinder = issueFinder; this.issueFieldsSetter = issueFieldsSetter; this.issueUpdater = issueUpdater; this.responseWriter = responseWriter; + this.system2 = system2; this.issueChangeWebhook = issueChangeWebhook; } @@ -108,10 +113,15 @@ public class SetTypeAction implements IssuesWsAction { DefaultIssue issue = issueDto.toDefaultIssue(); userSession.checkComponentUuidPermission(ISSUE_ADMIN, issue.projectUuid()); - IssueChangeContext context = IssueChangeContext.createUser(new Date(), userSession.getLogin()); + IssueChangeContext context = IssueChangeContext.createUser(new Date(system2.now()), userSession.getLogin()); if (issueFieldsSetter.setType(issue, ruleType, context)) { SearchResponseData searchResponseData = issueUpdater.saveIssueAndPreloadSearchResponseData(session, issue, context, null); - issueChangeWebhook.onChange(searchResponseData, new IssueChangeWebhook.IssueChange(ruleType), context); + issueChangeWebhook.onChange( + new IssueChangeWebhook.IssueChangeData( + searchResponseData.getIssues().stream().map(IssueDto::toDefaultIssue).collect(MoreCollectors.toList(searchResponseData.getIssues().size())), + copyOf(searchResponseData.getComponents())), + new IssueChangeWebhook.IssueChange(ruleType), + context); return searchResponseData; } return new SearchResponseData(issueDto); diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/webhook/IssueChangeWebhookImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/webhook/IssueChangeWebhookImplTest.java index afd311201b7..bd35131153d 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/webhook/IssueChangeWebhookImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/webhook/IssueChangeWebhookImplTest.java @@ -24,6 +24,8 @@ import com.google.common.collect.ImmutableSet; import com.tngtech.java.junit.dataprovider.DataProvider; import com.tngtech.java.junit.dataprovider.DataProviderRunner; import com.tngtech.java.junit.dataprovider.UseDataProvider; +import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.Date; import java.util.List; @@ -31,6 +33,7 @@ import java.util.Optional; import java.util.Random; import java.util.Set; import java.util.function.Supplier; +import java.util.stream.Collectors; import java.util.stream.IntStream; import java.util.stream.Stream; import javax.annotation.Nullable; @@ -50,9 +53,14 @@ import org.sonar.api.utils.System2; import org.sonar.core.issue.IssueChangeContext; import org.sonar.core.util.stream.MoreCollectors; import org.sonar.db.DbClient; +import org.sonar.db.DbSession; import org.sonar.db.DbTester; +import org.sonar.db.component.BranchDao; import org.sonar.db.component.BranchType; +import org.sonar.db.component.ComponentDao; import org.sonar.db.component.ComponentDto; +import org.sonar.db.component.ComponentTesting; +import org.sonar.db.component.SnapshotDao; import org.sonar.db.component.SnapshotDto; import org.sonar.db.issue.IssueDto; import org.sonar.db.organization.OrganizationDto; @@ -64,7 +72,6 @@ import org.sonar.server.issue.index.IssueIndexDefinition; import org.sonar.server.issue.index.IssueIndexer; import org.sonar.server.issue.index.IssueIteratorFactory; import org.sonar.server.issue.webhook.IssueChangeWebhook.IssueChange; -import org.sonar.server.issue.ws.SearchResponseData; import org.sonar.server.permission.index.AuthorizationTypeSupport; import org.sonar.server.qualitygate.ShortLivingBranchQualityGate; import org.sonar.server.tester.UserSessionRule; @@ -78,15 +85,19 @@ import org.sonar.server.webhook.WebHooks; import org.sonar.server.webhook.WebhookPayloadFactory; import static java.lang.String.valueOf; +import static java.util.Arrays.asList; +import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; import static org.assertj.core.api.Assertions.assertThat; +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.reset; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import static org.sonar.api.measures.CoreMetrics.BUGS_KEY; @@ -121,7 +132,8 @@ public class IssueChangeWebhookImplTest { private WebhookPayloadFactory webhookPayloadFactory = mock(WebhookPayloadFactory.class); private IssueIndex issueIndex = new IssueIndex(esTester.client(), System2.INSTANCE, userSessionRule, new AuthorizationTypeSupport(userSessionRule)); private Configuration mockedConfiguration = mock(Configuration.class); - private IssueChangeWebhookImpl underTest = new IssueChangeWebhookImpl(dbClient, webHooks, mockedConfiguration, webhookPayloadFactory, issueIndex); + private DbClient spiedOnDbClient = spy(dbClient); + private IssueChangeWebhookImpl underTest = new IssueChangeWebhookImpl(spiedOnDbClient, webHooks, mockedConfiguration, webhookPayloadFactory, issueIndex); private DbClient mockedDbClient = mock(DbClient.class); private IssueIndex spiedOnIssueIndex = spy(issueIndex); private IssueChangeWebhookImpl mockedUnderTest = new IssueChangeWebhookImpl(mockedDbClient, webHooks, mockedConfiguration, webhookPayloadFactory, spiedOnIssueIndex); @@ -133,14 +145,14 @@ public class IssueChangeWebhookImplTest { @Test public void on_type_change_has_no_effect_if_SearchResponseData_has_no_issue() { - mockedUnderTest.onChange(new SearchResponseData(Collections.emptyList()), new IssueChange(randomRuleType), userChangeContext); + mockedUnderTest.onChange(issueChangeData(), new IssueChange(randomRuleType), userChangeContext); verifyZeroInteractions(mockedDbClient, webHooks, mockedConfiguration, webhookPayloadFactory, spiedOnIssueIndex); } @Test public void on_type_change_has_no_effect_if_scan_IssueChangeContext() { - mockedUnderTest.onChange(new SearchResponseData(Collections.emptyList()), new IssueChange(randomRuleType), scanChangeContext); + mockedUnderTest.onChange(issueChangeData(), new IssueChange(randomRuleType), scanChangeContext); verifyZeroInteractions(mockedDbClient, webHooks, mockedConfiguration, webhookPayloadFactory, spiedOnIssueIndex); } @@ -149,14 +161,14 @@ public class IssueChangeWebhookImplTest { public void on_type_change_has_no_effect_if_webhooks_are_disabled() { when(webHooks.isEnabled(mockedConfiguration)).thenReturn(false); - underTest.onChange(new SearchResponseData(singletonList(new IssueDto())), new IssueChange(randomRuleType), userChangeContext); + underTest.onChange(issueChangeData(newIssueDto(null)), new IssueChange(randomRuleType), userChangeContext); verifyZeroInteractions(mockedDbClient, mockedConfiguration, webhookPayloadFactory, spiedOnIssueIndex); } @Test public void on_transition_change_has_no_effect_if_SearchResponseData_has_no_issue() { - mockedUnderTest.onChange(new SearchResponseData(Collections.emptyList()), new IssueChange(randomAlphanumeric(12)), userChangeContext); + mockedUnderTest.onChange(issueChangeData(), new IssueChange(randomAlphanumeric(12)), userChangeContext); verifyZeroInteractions(mockedDbClient, webHooks, mockedConfiguration, webhookPayloadFactory, spiedOnIssueIndex); } @@ -184,7 +196,7 @@ public class IssueChangeWebhookImplTest { reset(mockedDbClient, webHooks, mockedConfiguration, webhookPayloadFactory); when(webHooks.isEnabled(mockedConfiguration)).thenReturn(true); - mockedUnderTest.onChange(new SearchResponseData(singletonList(new IssueDto())), new IssueChange(transitionKey), userChangeContext); + mockedUnderTest.onChange(issueChangeData(newIssueDto(null)), new IssueChange(transitionKey), userChangeContext); verifyZeroInteractions(mockedDbClient, webHooks, mockedConfiguration, webhookPayloadFactory, spiedOnIssueIndex); } @@ -193,7 +205,7 @@ public class IssueChangeWebhookImplTest { public void on_transition_change_has_no_effect_if_scan_IssueChangeContext() { when(webHooks.isEnabled(mockedConfiguration)).thenReturn(true); - mockedUnderTest.onChange(new SearchResponseData(singletonList(new IssueDto())), new IssueChange(randomAlphanumeric(12)), scanChangeContext); + mockedUnderTest.onChange(issueChangeData(newIssueDto(null)), new IssueChange(randomAlphanumeric(12)), scanChangeContext); verifyZeroInteractions(mockedDbClient, webHooks, mockedConfiguration, webhookPayloadFactory, spiedOnIssueIndex); } @@ -202,21 +214,21 @@ public class IssueChangeWebhookImplTest { public void on_transition_change_has_no_effect_if_webhooks_are_disabled() { when(webHooks.isEnabled(mockedConfiguration)).thenReturn(false); - mockedUnderTest.onChange(new SearchResponseData(singletonList(new IssueDto())), new IssueChange(randomAlphanumeric(12)), userChangeContext); + mockedUnderTest.onChange(issueChangeData(newIssueDto(null)), new IssueChange(randomAlphanumeric(12)), userChangeContext); verifyZeroInteractions(mockedDbClient, webHooks, mockedConfiguration, webhookPayloadFactory, spiedOnIssueIndex); } @Test public void on_type_and_transition_change_has_no_effect_if_SearchResponseData_has_no_issue() { - mockedUnderTest.onChange(new SearchResponseData(Collections.emptyList()), new IssueChange(randomRuleType, randomAlphanumeric(3)), userChangeContext); + mockedUnderTest.onChange(issueChangeData(), new IssueChange(randomRuleType, randomAlphanumeric(3)), userChangeContext); verifyZeroInteractions(mockedDbClient, webHooks, mockedConfiguration, webhookPayloadFactory, spiedOnIssueIndex); } @Test public void on_type_and_transition_change_has_no_effect_if_scan_IssueChangeContext() { - mockedUnderTest.onChange(new SearchResponseData(Collections.emptyList()), new IssueChange(randomRuleType, randomAlphanumeric(3)), scanChangeContext); + mockedUnderTest.onChange(issueChangeData(), new IssueChange(randomRuleType, randomAlphanumeric(3)), scanChangeContext); verifyZeroInteractions(mockedDbClient, webHooks, mockedConfiguration, webhookPayloadFactory, spiedOnIssueIndex); } @@ -225,7 +237,7 @@ public class IssueChangeWebhookImplTest { public void on_type_and_transition_change_has_no_effect_if_webhooks_are_disabled() { when(webHooks.isEnabled(mockedConfiguration)).thenReturn(false); - underTest.onChange(new SearchResponseData(singletonList(new IssueDto())), new IssueChange(randomRuleType, randomAlphanumeric(3)), userChangeContext); + underTest.onChange(issueChangeData(newIssueDto(null)), new IssueChange(randomRuleType, randomAlphanumeric(3)), userChangeContext); verifyZeroInteractions(mockedDbClient, mockedConfiguration, webhookPayloadFactory, spiedOnIssueIndex); } @@ -253,7 +265,7 @@ public class IssueChangeWebhookImplTest { reset(mockedDbClient, webHooks, mockedConfiguration, webhookPayloadFactory); when(webHooks.isEnabled(mockedConfiguration)).thenReturn(true); - mockedUnderTest.onChange(new SearchResponseData(singletonList(new IssueDto())), new IssueChange(randomRuleType, transitionKey), userChangeContext); + mockedUnderTest.onChange(issueChangeData(newIssueDto(null)), new IssueChange(randomRuleType, transitionKey), userChangeContext); verifyZeroInteractions(mockedDbClient, webHooks, mockedConfiguration, webhookPayloadFactory, spiedOnIssueIndex); } @@ -268,7 +280,7 @@ public class IssueChangeWebhookImplTest { .setKey("foo")); SnapshotDto analysis = insertAnalysisTask(branch); - underTest.onChange(new SearchResponseData(new IssueDto().setComponent(branch)), issueChange, userChangeContext); + underTest.onChange(issueChangeData(newIssueDto(branch)), issueChange, userChangeContext); ProjectAnalysis projectAnalysis = verifyWebhookCalledAndExtractPayloadFromSupplier(branch, analysis); assertThat(projectAnalysis).isEqualTo( @@ -292,10 +304,10 @@ public class IssueChangeWebhookImplTest { @Test public void compute_QG_ok_if_there_is_no_issue_in_index_ignoring_permissions() { OrganizationDto organization = dbTester.organizations().insert(); - ComponentDto branch = insertPrivateBranch(organization); + ComponentDto branch = insertPrivateBranch(organization, BranchType.SHORT); SnapshotDto analysis = insertAnalysisTask(branch); - underTest.onChange(new SearchResponseData(new IssueDto().setComponent(branch)), new IssueChange(randomRuleType), userChangeContext); + underTest.onChange(issueChangeData(newIssueDto(branch)), new IssueChange(randomRuleType), userChangeContext); ProjectAnalysis projectAnalysis = verifyWebhookCalledAndExtractPayloadFromSupplier(branch, analysis); QualityGate qualityGate = projectAnalysis.getQualityGate().get(); @@ -344,13 +356,13 @@ public class IssueChangeWebhookImplTest { private void computesQGErrorIfThereIsAtLeastOneUnresolvedIssueInIndexOfTypeIgnoringPermissions( int unresolvedIssues, RuleType ruleType, Tuple... expectedQGConditions) { OrganizationDto organization = dbTester.organizations().insert(); - ComponentDto branch = insertPrivateBranch(organization); + ComponentDto branch = insertPrivateBranch(organization, BranchType.SHORT); SnapshotDto analysis = insertAnalysisTask(branch); IntStream.range(0, unresolvedIssues).forEach(i -> insertIssue(branch, ruleType, randomOpenStatus, null)); IntStream.range(0, random.nextInt(10)).forEach(i -> insertIssue(branch, ruleType, randomNonOpenStatus, randomResolution)); indexIssues(branch); - underTest.onChange(new SearchResponseData(new IssueDto().setComponent(branch)), new IssueChange(randomRuleType), userChangeContext); + underTest.onChange(issueChangeData(newIssueDto(branch)), new IssueChange(randomRuleType), userChangeContext); ProjectAnalysis projectAnalysis = verifyWebhookCalledAndExtractPayloadFromSupplier(branch, analysis); QualityGate qualityGate = projectAnalysis.getQualityGate().get(); @@ -363,7 +375,7 @@ public class IssueChangeWebhookImplTest { @Test public void computes_QG_error_with_all_failing_conditions() { OrganizationDto organization = dbTester.organizations().insert(); - ComponentDto branch = insertPrivateBranch(organization); + ComponentDto branch = insertPrivateBranch(organization, BranchType.SHORT); SnapshotDto analysis = insertAnalysisTask(branch); int unresolvedBugs = 1 + random.nextInt(10); int unresolvedVulnerabilities = 1 + random.nextInt(10); @@ -373,7 +385,7 @@ public class IssueChangeWebhookImplTest { IntStream.range(0, unresolvedCodeSmells).forEach(i -> insertIssue(branch, RuleType.CODE_SMELL, randomOpenStatus, null)); indexIssues(branch); - underTest.onChange(new SearchResponseData(new IssueDto().setComponent(branch)), new IssueChange(randomRuleType), userChangeContext); + underTest.onChange(issueChangeData(newIssueDto(branch)), new IssueChange(randomRuleType), userChangeContext); ProjectAnalysis projectAnalysis = verifyWebhookCalledAndExtractPayloadFromSupplier(branch, analysis); QualityGate qualityGate = projectAnalysis.getQualityGate().get(); @@ -387,11 +399,11 @@ public class IssueChangeWebhookImplTest { } @Test - public void call_webhook_only_once_per_branch_with_at_least_one_issue_in_SearchResponseData() { + public void call_webhook_only_once_per_short_branch_with_at_least_one_issue_in_SearchResponseData() { OrganizationDto organization = dbTester.organizations().insert(); - ComponentDto branch1 = insertPrivateBranch(organization); - ComponentDto branch2 = insertPrivateBranch(organization); - ComponentDto branch3 = insertPrivateBranch(organization); + ComponentDto branch1 = insertPrivateBranch(organization, BranchType.SHORT); + ComponentDto branch2 = insertPrivateBranch(organization, BranchType.SHORT); + ComponentDto branch3 = insertPrivateBranch(organization, BranchType.SHORT); SnapshotDto analysis1 = insertAnalysisTask(branch1); SnapshotDto analysis2 = insertAnalysisTask(branch2); SnapshotDto analysis3 = insertAnalysisTask(branch3); @@ -399,17 +411,128 @@ public class IssueChangeWebhookImplTest { int issuesBranch2 = 2 + random.nextInt(10); int issuesBranch3 = 2 + random.nextInt(10); List issueDtos = Stream.of( - IntStream.range(0, issuesBranch1).mapToObj(i -> new IssueDto().setComponent(branch1)), - IntStream.range(0, issuesBranch2).mapToObj(i -> new IssueDto().setComponent(branch2)), - IntStream.range(0, issuesBranch3).mapToObj(i -> new IssueDto().setComponent(branch3))) + IntStream.range(0, issuesBranch1).mapToObj(i -> newIssueDto(branch1)), + IntStream.range(0, issuesBranch2).mapToObj(i -> newIssueDto(branch2)), + IntStream.range(0, issuesBranch3).mapToObj(i -> newIssueDto(branch3))) .flatMap(s -> s) .collect(MoreCollectors.toList()); - underTest.onChange(new SearchResponseData(issueDtos), new IssueChange(randomRuleType), userChangeContext); + ComponentDao componentDaoSpy = spy(dbClient.componentDao()); + BranchDao branchDaoSpy = spy(dbClient.branchDao()); + SnapshotDao snapshotDaoSpy = spy(dbClient.snapshotDao()); + when(spiedOnDbClient.componentDao()).thenReturn(componentDaoSpy); + when(spiedOnDbClient.branchDao()).thenReturn(branchDaoSpy); + when(spiedOnDbClient.snapshotDao()).thenReturn(snapshotDaoSpy); + underTest.onChange(issueChangeData(issueDtos), new IssueChange(randomRuleType), userChangeContext); verifyWebhookCalledAndExtractPayloadFromSupplier(branch1, analysis1); verifyWebhookCalledAndExtractPayloadFromSupplier(branch2, analysis2); verifyWebhookCalledAndExtractPayloadFromSupplier(branch3, analysis3); + + Set uuids = ImmutableSet.of(branch1.uuid(), branch2.uuid(), branch3.uuid()); + verify(componentDaoSpy).selectByUuids(any(DbSession.class), eq(uuids)); + verify(branchDaoSpy).selectByUuids(any(DbSession.class), eq(uuids)); + verify(snapshotDaoSpy).selectLastAnalysesByRootComponentUuids(any(DbSession.class), eq(uuids)); + verifyNoMoreInteractions(componentDaoSpy, branchDaoSpy, snapshotDaoSpy); + } + + @Test + public void call_webhood_only_for_short_branches() { + OrganizationDto organization = dbTester.organizations().insert(); + ComponentDto shortBranch = insertPrivateBranch(organization, BranchType.SHORT); + ComponentDto longBranch = insertPrivateBranch(organization, BranchType.LONG); + SnapshotDto analysis1 = insertAnalysisTask(shortBranch); + SnapshotDto analysis2 = insertAnalysisTask(longBranch); + + ComponentDao componentDaoSpy = spy(dbClient.componentDao()); + BranchDao branchDaoSpy = spy(dbClient.branchDao()); + SnapshotDao snapshotDaoSpy = spy(dbClient.snapshotDao()); + when(spiedOnDbClient.componentDao()).thenReturn(componentDaoSpy); + when(spiedOnDbClient.branchDao()).thenReturn(branchDaoSpy); + when(spiedOnDbClient.snapshotDao()).thenReturn(snapshotDaoSpy); + underTest.onChange( + issueChangeData(asList(newIssueDto(shortBranch), newIssueDto(longBranch))), + new IssueChange(randomRuleType), + userChangeContext); + + verifyWebhookCalledAndExtractPayloadFromSupplier(shortBranch, analysis1); + + Set uuids = ImmutableSet.of(shortBranch.uuid(), longBranch.uuid()); + verify(componentDaoSpy).selectByUuids(any(DbSession.class), eq(uuids)); + verify(branchDaoSpy).selectByUuids(any(DbSession.class), eq(uuids)); + verify(snapshotDaoSpy).selectLastAnalysesByRootComponentUuids(any(DbSession.class), eq(ImmutableSet.of(shortBranch.uuid()))); + verifyNoMoreInteractions(componentDaoSpy, branchDaoSpy, snapshotDaoSpy); + } + + @Test + public void call_db_only_for_componentDto_not_in_inputData() { + OrganizationDto organization = dbTester.organizations().insert(); + ComponentDto branch1 = insertPrivateBranch(organization, BranchType.SHORT); + ComponentDto branch2 = insertPrivateBranch(organization, BranchType.SHORT); + ComponentDto branch3 = insertPrivateBranch(organization, BranchType.SHORT); + SnapshotDto analysis1 = insertAnalysisTask(branch1); + SnapshotDto analysis2 = insertAnalysisTask(branch2); + SnapshotDto analysis3 = insertAnalysisTask(branch3); + List issueDtos = asList(newIssueDto(branch1), newIssueDto(branch2), newIssueDto(branch3)); + + ComponentDao componentDaoSpy = spy(dbClient.componentDao()); + BranchDao branchDaoSpy = spy(dbClient.branchDao()); + SnapshotDao snapshotDaoSpy = spy(dbClient.snapshotDao()); + when(spiedOnDbClient.componentDao()).thenReturn(componentDaoSpy); + when(spiedOnDbClient.branchDao()).thenReturn(branchDaoSpy); + when(spiedOnDbClient.snapshotDao()).thenReturn(snapshotDaoSpy); + underTest.onChange( + issueChangeData(issueDtos, branch1, branch3), + new IssueChange(randomRuleType), + userChangeContext); + + verifyWebhookCalledAndExtractPayloadFromSupplier(branch1, analysis1); + verifyWebhookCalledAndExtractPayloadFromSupplier(branch2, analysis2); + verifyWebhookCalledAndExtractPayloadFromSupplier(branch3, analysis3); + + Set uuids = ImmutableSet.of(branch1.uuid(), branch2.uuid(), branch3.uuid()); + verify(componentDaoSpy).selectByUuids(any(DbSession.class), eq(ImmutableSet.of(branch2.uuid()))); + verify(branchDaoSpy).selectByUuids(any(DbSession.class), eq(uuids)); + verify(snapshotDaoSpy).selectLastAnalysesByRootComponentUuids(any(DbSession.class), eq(uuids)); + verifyNoMoreInteractions(componentDaoSpy, branchDaoSpy, snapshotDaoSpy); + } + + @Test + public void supports_issues_on_files_and_filter_on_short_branches_asap_when_calling_db() { + OrganizationDto organization = dbTester.organizations().insert(); + ComponentDto project = dbTester.components().insertPrivateProject(organization); + ComponentDto projectFile = dbTester.components().insertComponent(ComponentTesting.newFileDto(project)); + ComponentDto shortBranch = dbTester.components().insertProjectBranch(project, branchDto -> branchDto + .setBranchType(BranchType.SHORT) + .setKey("foo")); + ComponentDto longBranch = dbTester.components().insertProjectBranch(project, branchDto -> branchDto + .setBranchType(BranchType.LONG) + .setKey("bar")); + ComponentDto shortBranchFile = dbTester.components().insertComponent(ComponentTesting.newFileDto(shortBranch)); + ComponentDto longBranchFile = dbTester.components().insertComponent(ComponentTesting.newFileDto(longBranch)); + SnapshotDto analysis1 = insertAnalysisTask(project); + SnapshotDto analysis2 = insertAnalysisTask(shortBranch); + SnapshotDto analysis3 = insertAnalysisTask(longBranch); + List issueDtos = asList( + newIssueDto(projectFile, project), + newIssueDto(shortBranchFile, shortBranch), + newIssueDto(longBranchFile, longBranch)); + + ComponentDao componentDaoSpy = spy(dbClient.componentDao()); + BranchDao branchDaoSpy = spy(dbClient.branchDao()); + SnapshotDao snapshotDaoSpy = spy(dbClient.snapshotDao()); + when(spiedOnDbClient.componentDao()).thenReturn(componentDaoSpy); + when(spiedOnDbClient.branchDao()).thenReturn(branchDaoSpy); + when(spiedOnDbClient.snapshotDao()).thenReturn(snapshotDaoSpy); + underTest.onChange(issueChangeData(issueDtos), new IssueChange(randomRuleType), userChangeContext); + + verifyWebhookCalledAndExtractPayloadFromSupplier(shortBranch, analysis2); + + Set uuids = ImmutableSet.of(project.uuid(), shortBranch.uuid(), longBranch.uuid()); + verify(componentDaoSpy).selectByUuids(any(DbSession.class), eq(uuids)); + verify(branchDaoSpy).selectByUuids(any(DbSession.class), eq(ImmutableSet.of(shortBranch.uuid(), longBranch.uuid()))); + verify(snapshotDaoSpy).selectLastAnalysesByRootComponentUuids(any(DbSession.class), eq(ImmutableSet.of(shortBranch.uuid()))); + verifyNoMoreInteractions(componentDaoSpy, branchDaoSpy, snapshotDaoSpy); } private void insertIssue(ComponentDto branch, RuleType ruleType, String status, @Nullable String resolution) { @@ -420,10 +543,10 @@ public class IssueChangeWebhookImplTest { dbTester.commit(); } - private ComponentDto insertPrivateBranch(OrganizationDto organization) { + private ComponentDto insertPrivateBranch(OrganizationDto organization, BranchType branchType) { ComponentDto project = dbTester.components().insertPrivateProject(organization); return dbTester.components().insertProjectBranch(project, branchDto -> branchDto - .setBranchType(BranchType.SHORT) + .setBranchType(branchType) .setKey("foo")); } @@ -474,4 +597,38 @@ public class IssueChangeWebhookImplTest { {new IssueChange(RuleType.CODE_SMELL, DefaultTransitions.REOPEN)} }; } + + private IssueChangeWebhook.IssueChangeData issueChangeData() { + return new IssueChangeWebhook.IssueChangeData(emptyList(), emptyList()); + } + + private IssueChangeWebhook.IssueChangeData issueChangeData(IssueDto issueDto) { + return new IssueChangeWebhook.IssueChangeData(singletonList(issueDto.toDefaultIssue()), emptyList()); + } + + private IssueChangeWebhook.IssueChangeData issueChangeData(Collection issueDtos, ComponentDto... components) { + return new IssueChangeWebhook.IssueChangeData( + issueDtos.stream().map(IssueDto::toDefaultIssue).collect(Collectors.toList()), + Arrays.stream(components).collect(Collectors.toList())); + } + + private IssueDto newIssueDto(@Nullable ComponentDto project) { + return newIssueDto(project, project); + } + + private IssueDto newIssueDto(@Nullable ComponentDto component, @Nullable ComponentDto project) { + RuleType randomRuleType = RuleType.values()[random.nextInt(RuleType.values().length)]; + String randomStatus = Issue.STATUSES.get(random.nextInt(Issue.STATUSES.size())); + IssueDto res = new IssueDto() + .setType(randomRuleType) + .setStatus(randomStatus) + .setRuleKey(randomAlphanumeric(3), randomAlphanumeric(4)); + if (component != null) { + res.setComponent(component); + } + if (project != null) { + res.setProject(project); + } + return res; + } } 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 df7bf696472..6efc13e9f2b 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 @@ -20,9 +20,12 @@ package org.sonar.server.issue.ws; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Date; import java.util.List; import java.util.stream.Collectors; import java.util.stream.IntStream; +import javax.annotation.Nullable; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -32,6 +35,8 @@ import org.sonar.api.config.internal.MapSettings; import org.sonar.api.rules.RuleType; import org.sonar.api.server.ws.WebService; 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.ComponentDto; @@ -51,6 +56,7 @@ import org.sonar.server.issue.index.IssueIndexDefinition; import org.sonar.server.issue.index.IssueIndexer; import org.sonar.server.issue.index.IssueIteratorFactory; import org.sonar.server.issue.notification.IssueChangeNotification; +import org.sonar.server.issue.webhook.IssueChangeWebhook; import org.sonar.server.issue.workflow.FunctionExecutor; import org.sonar.server.issue.workflow.IssueWorkflow; import org.sonar.server.notification.NotificationManager; @@ -67,8 +73,10 @@ import static java.util.Arrays.asList; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.tuple; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import static org.sonar.api.issue.Issue.RESOLUTION_FIXED; import static org.sonar.api.issue.Issue.STATUS_CLOSED; @@ -76,6 +84,7 @@ import static org.sonar.api.issue.Issue.STATUS_OPEN; import static org.sonar.api.rule.Severity.MAJOR; import static org.sonar.api.rule.Severity.MINOR; import static org.sonar.api.rules.RuleType.BUG; +import static org.sonar.api.rules.RuleType.CODE_SMELL; import static org.sonar.api.rules.RuleType.VULNERABILITY; import static org.sonar.api.web.UserRole.ISSUE_ADMIN; import static org.sonar.api.web.UserRole.USER; @@ -108,6 +117,7 @@ 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 IssueChangeWebhook issueChangeWebhook = mock(IssueChangeWebhook.class); private List actions = new ArrayList<>(); private RuleDto rule; @@ -116,7 +126,7 @@ public class BulkChangeActionTest { private ComponentDto file; private UserDto user; - private WsActionTester tester = new WsActionTester(new BulkChangeAction(system2, userSession, dbClient, issueStorage, notificationManager, actions)); + private WsActionTester tester = new WsActionTester(new BulkChangeAction(system2, userSession, dbClient, issueStorage, notificationManager, actions, issueChangeWebhook)); @Before public void setUp() throws Exception { @@ -144,6 +154,8 @@ public class BulkChangeActionTest { IssueDto reloaded = getIssueByKeys(issueDto.getKey()).get(0); assertThat(reloaded.getType()).isEqualTo(RuleType.CODE_SMELL.getDbConstant()); assertThat(reloaded.getUpdatedAt()).isEqualTo(NOW); + + verifyIssueChangeWebhookCalled(CODE_SMELL, null, new String[] {file.uuid()}, issueDto); } @Test @@ -160,6 +172,8 @@ public class BulkChangeActionTest { IssueDto reloaded = getIssueByKeys(issueDto.getKey()).get(0); assertThat(reloaded.getSeverity()).isEqualTo(MINOR); assertThat(reloaded.getUpdatedAt()).isEqualTo(NOW); + + verifyZeroInteractions(issueChangeWebhook); } @Test @@ -176,6 +190,8 @@ public class BulkChangeActionTest { IssueDto reloaded = getIssueByKeys(issueDto.getKey()).get(0); assertThat(reloaded.getTags()).containsOnly("tag1", "tag2", "tag3"); assertThat(reloaded.getUpdatedAt()).isEqualTo(NOW); + + verifyZeroInteractions(issueChangeWebhook); } @Test @@ -192,6 +208,8 @@ public class BulkChangeActionTest { IssueDto reloaded = getIssueByKeys(issueDto.getKey()).get(0); assertThat(reloaded.getAssignee()).isNull(); assertThat(reloaded.getUpdatedAt()).isEqualTo(NOW); + + verifyZeroInteractions(issueChangeWebhook); } @Test @@ -209,6 +227,8 @@ public class BulkChangeActionTest { IssueChangeDto issueComment = dbClient.issueChangeDao().selectByTypeAndIssueKeys(db.getSession(), singletonList(issueDto.getKey()), TYPE_COMMENT).get(0); assertThat(issueComment.getUserLogin()).isEqualTo("john"); assertThat(issueComment.getChangeData()).isEqualTo("type was badly defined"); + + verifyIssueChangeWebhookCalled(null, "confirm", new String[] {file.uuid()}, issueDto); } @Test @@ -235,6 +255,8 @@ public class BulkChangeActionTest { tuple(issue1.getKey(), userToAssign.getLogin(), VULNERABILITY.getDbConstant(), MINOR, NOW), 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); } @Test @@ -259,6 +281,8 @@ public class BulkChangeActionTest { assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("ruleName")).isEqualTo(rule.getName()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("changeAuthor")).isEqualTo(user.getLogin()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("branch")).isNull(); + + verifyIssueChangeWebhookCalled(null, "confirm", new String[] {file.uuid()}, issueDto); } @Test @@ -287,6 +311,8 @@ public class BulkChangeActionTest { assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("ruleName")).isEqualTo(rule.getName()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("changeAuthor")).isEqualTo(user.getLogin()); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("branch")).isEqualTo(branchName); + + verifyIssueChangeWebhookCalled(null, "confirm", new String[] {fileOnBranch.uuid()}, issueDto); } @Test @@ -307,6 +333,8 @@ public class BulkChangeActionTest { verify(notificationManager).scheduleForSending(issueChangeNotificationCaptor.capture()); assertThat(issueChangeNotificationCaptor.getAllValues()).hasSize(1); assertThat(issueChangeNotificationCaptor.getValue().getFieldValue("key")).isEqualTo(issue3.getKey()); + + verifyIssueChangeWebhookCalled(BUG, null, new String[] {file.uuid()}, issue3); } @Test @@ -329,6 +357,8 @@ public class BulkChangeActionTest { tuple(issue1.getKey(), VULNERABILITY.getDbConstant(), NOW), tuple(issue3.getKey(), BUG.getDbConstant(), issue2.getUpdatedAt()), tuple(issue2.getKey(), BUG.getDbConstant(), issue3.getUpdatedAt())); + + verifyIssueChangeWebhookCalled(VULNERABILITY, null, new String[] {file.uuid()}, issue1); } @Test @@ -351,6 +381,8 @@ public class BulkChangeActionTest { tuple(issue1.getKey(), VULNERABILITY.getDbConstant(), NOW), tuple(issue2.getKey(), VULNERABILITY.getDbConstant(), issue2.getUpdatedAt()), tuple(issue3.getKey(), VULNERABILITY.getDbConstant(), issue3.getUpdatedAt())); + + verifyIssueChangeWebhookCalled(VULNERABILITY, null, new String[] {file.uuid()}, issue1); } @Test @@ -371,6 +403,8 @@ public class BulkChangeActionTest { assertThat(dbClient.issueChangeDao().selectByTypeAndIssueKeys(db.getSession(), singletonList(issue1.getKey()), TYPE_COMMENT)).hasSize(1); 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); } @Test @@ -395,6 +429,8 @@ public class BulkChangeActionTest { tuple(authorizedIssue.getKey(), VULNERABILITY.getDbConstant(), NOW), tuple(notAuthorizedIssue1.getKey(), BUG.getDbConstant(), notAuthorizedIssue1.getUpdatedAt()), tuple(notAuthorizedIssue2.getKey(), BUG.getDbConstant(), notAuthorizedIssue2.getUpdatedAt())); + + verifyIssueChangeWebhookCalled(VULNERABILITY, null, new String[] {file.uuid()}, authorizedIssue); } @Test @@ -491,6 +527,23 @@ public class BulkChangeActionTest { assertThat(action.responseExample()).isNotNull(); } + private void verifyIssueChangeWebhookCalled(@Nullable RuleType expectedRuleType, @Nullable String transitionKey, + String[] componentUUids, + IssueDto... issueDtos) { + ArgumentCaptor issueChangeDataCaptor = ArgumentCaptor.forClass(IssueChangeWebhook.IssueChangeData.class); + verify(issueChangeWebhook).onChange( + issueChangeDataCaptor.capture(), + eq(new IssueChangeWebhook.IssueChange(expectedRuleType, transitionKey)), + eq(IssueChangeContext.createUser(new Date(NOW), userSession.getLogin()))); + IssueChangeWebhook.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); + } + private BulkChangeWsResponse call(BulkChangeRequest bulkChangeRequest) { TestRequest request = tester.newRequest(); setNullable(bulkChangeRequest.getIssues(), value -> request.setParam("issues", String.join(",", value))); 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 309b67b017a..3384f42a4ec 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,6 +19,7 @@ */ package org.sonar.server.issue.ws; +import java.util.Date; import javax.annotation.Nullable; import org.junit.Before; import org.junit.Rule; @@ -29,6 +30,8 @@ import org.sonar.api.config.internal.MapSettings; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; 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; @@ -68,6 +71,7 @@ import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; 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; @@ -111,7 +115,8 @@ public class DoTransitionActionTest { private ArgumentCaptor preloadedSearchResponseDataCaptor = ArgumentCaptor.forClass(SearchResponseData.class); private IssueChangeWebhook issueChangeWebhook = mock(IssueChangeWebhook.class); - private WsAction underTest = new DoTransitionAction(dbClient, userSession, new IssueFinder(dbClient, userSession), issueUpdater, transitionService, responseWriter, issueChangeWebhook); + private WsAction underTest = new DoTransitionAction(dbClient, userSession, new IssueFinder(dbClient, userSession), issueUpdater, transitionService, responseWriter, system2, + issueChangeWebhook); private WsActionTester tester = new WsActionTester(underTest); @Before @@ -123,6 +128,8 @@ public class DoTransitionActionTest { @Test public void do_transition() throws Exception { + long now = 999_776_888L; + when(system2.now()).thenReturn(now); IssueDto issueDto = issueDbTester.insertIssue(newIssue().setStatus(STATUS_OPEN).setResolution(null)); userSession.logIn("john").addProjectPermission(USER, project, file); @@ -132,6 +139,19 @@ public class DoTransitionActionTest { verifyContentOfPreloadedSearchResponseData(issueDto); IssueDto issueReloaded = dbClient.issueDao().selectByKey(dbTester.getSession(), issueDto.getKey()).get(); assertThat(issueReloaded.getStatus()).isEqualTo(STATUS_CONFIRMED); + + ArgumentCaptor issueChangeDataCaptor = ArgumentCaptor.forClass(IssueChangeWebhook.IssueChangeData.class); + verify(issueChangeWebhook).onChange( + issueChangeDataCaptor.capture(), + eq(new IssueChangeWebhook.IssueChange(null, "confirm")), + eq(IssueChangeContext.createUser(new Date(now), userSession.getLogin()))); + IssueChangeWebhook.IssueChangeData issueChangeData = issueChangeDataCaptor.getValue(); + assertThat(issueChangeData.getIssues()) + .extracting(DefaultIssue::key) + .containsOnly(issueDto.getKey()); + assertThat(issueChangeData.getComponents()) + .extracting(ComponentDto::uuid) + .containsOnly(issueDto.getComponentUuid(), issueDto.getProjectUuid()); } @Test 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 4f786af8ae3..ed9320e7884 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.Date; import java.util.List; import javax.annotation.Nullable; import org.junit.Rule; @@ -30,7 +31,9 @@ import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; import org.sonar.api.utils.System2; +import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.FieldDiffs; +import org.sonar.core.issue.IssueChangeContext; import org.sonar.db.DbClient; import org.sonar.db.DbTester; import org.sonar.db.component.ComponentDto; @@ -63,6 +66,7 @@ import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static org.sonar.api.rules.RuleType.BUG; import static org.sonar.api.rules.RuleType.CODE_SMELL; import static org.sonar.api.web.UserRole.ISSUE_ADMIN; @@ -97,10 +101,12 @@ public class SetTypeActionTest { 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, issueChangeWebhook)); + responseWriter, system2, issueChangeWebhook)); @Test public void set_type() throws Exception { + long now = 1_999_777_234L; + when(system2.now()).thenReturn(now); IssueDto issueDto = issueDbTester.insertIssue(newIssue().setType(CODE_SMELL)); setUserWithBrowseAndAdministerIssuePermission(issueDto); @@ -110,6 +116,19 @@ public class SetTypeActionTest { verifyContentOfPreloadedSearchResponseData(issueDto); IssueDto issueReloaded = dbClient.issueDao().selectByKey(dbTester.getSession(), issueDto.getKey()).get(); assertThat(issueReloaded.getType()).isEqualTo(BUG.getDbConstant()); + + ArgumentCaptor issueChangeDataCaptor = ArgumentCaptor.forClass(IssueChangeWebhook.IssueChangeData.class); + verify(issueChangeWebhook).onChange( + issueChangeDataCaptor.capture(), + eq(new IssueChangeWebhook.IssueChange(BUG, null)), + eq(IssueChangeContext.createUser(new Date(now), userSession.getLogin()))); + IssueChangeWebhook.IssueChangeData issueChangeData = issueChangeDataCaptor.getValue(); + assertThat(issueChangeData.getIssues()) + .extracting(DefaultIssue::key) + .containsOnly(issueDto.getKey()); + assertThat(issueChangeData.getComponents()) + .extracting(ComponentDto::uuid) + .containsOnly(issueDto.getComponentUuid(), issueDto.getProjectUuid()); } @Test diff --git a/sonar-core/src/main/java/org/sonar/core/issue/IssueChangeContext.java b/sonar-core/src/main/java/org/sonar/core/issue/IssueChangeContext.java index 04380338755..3d24076de19 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/IssueChangeContext.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/IssueChangeContext.java @@ -21,6 +21,7 @@ package org.sonar.core.issue; import java.io.Serializable; import java.util.Date; +import java.util.Objects; import javax.annotation.CheckForNull; import javax.annotation.Nullable; @@ -61,4 +62,32 @@ public class IssueChangeContext implements Serializable { public static IssueChangeContext createUser(Date date, @Nullable String login) { return new IssueChangeContext(login, date, false); } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + IssueChangeContext that = (IssueChangeContext) o; + return scan == that.scan && + Objects.equals(login, that.login) && + Objects.equals(date, that.date); + } + + @Override + public int hashCode() { + return Objects.hash(login, date, scan); + } + + @Override + public String toString() { + return "IssueChangeContext{" + + "login='" + login + '\'' + + ", date=" + date + + ", scan=" + scan + + '}'; + } } -- 2.39.5