From 29d4fd9afc740dd92e9b3223270ae5a3b48040de Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Wed, 20 Dec 2017 16:55:33 +0100 Subject: [PATCH] SONAR-10085 QGChangeEventListener#onIssueChanges replaces onChanges --- .../changeevent/QGChangeEventListener.java | 24 ++- .../QGChangeEventListenersImpl.java | 74 ++++++- .../webhook/WebhookQGChangeEventListener.java | 18 +- .../QGChangeEventListenersImplTest.java | 197 ++++++++++++++---- .../WebhookQGChangeEventListenerTest.java | 103 +++------ 5 files changed, 285 insertions(+), 131 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListener.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListener.java index 89276021c74..8a525d672ad 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListener.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListener.java @@ -19,8 +19,28 @@ */ package org.sonar.server.qualitygate.changeevent; -import java.util.Collection; +import java.util.Set; +import org.sonar.api.rules.RuleType; public interface QGChangeEventListener { - void onChanges(Trigger trigger, Collection changeEvents); + + /** + * Called consequently to a change done on one or more issue of a given project. + * + * @param qualityGateEvent can not be {@code null} + * @param changedIssues can not be {@code null} nor empty + */ + void onIssueChanges(QGChangeEvent qualityGateEvent, Set changedIssues); + + interface ChangedIssue { + String getKey(); + + Status getStatus(); + + RuleType getType(); + } + + enum Status { + OPEN, CONFIRMED, REOPENED, RESOLVED, CLOSED + } } 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 f6047038d2e..3a46c41da59 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 @@ -19,14 +19,19 @@ */ package org.sonar.server.qualitygate.changeevent; -import com.google.common.collect.ImmutableList; +import com.google.common.collect.Multimap; import java.util.Arrays; import java.util.Collection; -import java.util.List; +import java.util.Set; +import org.sonar.api.rules.RuleType; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.util.stream.MoreCollectors; +import org.sonar.server.qualitygate.changeevent.QGChangeEventListener.ChangedIssue; import static java.lang.String.format; +import static org.sonar.core.util.stream.MoreCollectors.toSet; /** * Broadcast a given collection of {@link QGChangeEvent} for a specific trigger to all the registered @@ -58,20 +63,71 @@ public class QGChangeEventListenersImpl implements QGChangeEventListeners { } try { - List immutableChangeEvents = ImmutableList.copyOf(changeEvents); - Arrays.stream(listeners) - .forEach(listener -> broadcastTo(Trigger.ISSUE_CHANGE, immutableChangeEvents, listener)); + Multimap eventsByComponentUuid = changeEvents.stream() + .collect(MoreCollectors.index(t -> t.getProject().uuid())); + Multimap issueByComponentUuid = issueChangeData.getIssues().stream() + .collect(MoreCollectors.index(DefaultIssue::projectUuid)); + + issueByComponentUuid.asMap() + .forEach((componentUuid, value) -> { + Collection qgChangeEvents = eventsByComponentUuid.get(componentUuid); + if (!qgChangeEvents.isEmpty()) { + Set changedIssues = value.stream() + .map(ChangedIssueImpl::new) + .collect(toSet()); + qgChangeEvents + .forEach(changeEvent -> Arrays.stream(listeners) + .forEach(listener -> broadcastTo(changedIssues, changeEvent, listener))); + } + }); } catch (Error e) { LOG.warn(format("Broadcasting to listeners failed for %s events", changeEvents.size()), e); } } - private static void broadcastTo(Trigger trigger, Collection changeEvents, QGChangeEventListener listener) { + private static void broadcastTo(Set changedIssues, QGChangeEvent changeEvent, QGChangeEventListener listener) { try { - LOG.trace("calling onChange() on listener {} for events {}...", listener.getClass().getName(), changeEvents); - listener.onChanges(trigger, changeEvents); + LOG.trace("calling onChange() on listener {} for events {}...", listener.getClass().getName(), changeEvent); + listener.onIssueChanges(changeEvent, changedIssues); } catch (Exception e) { - LOG.warn(format("onChange() call failed on listener %s for events %s", listener.getClass().getName(), changeEvents), e); + LOG.warn(format("onChange() call failed on listener %s for events %s", listener.getClass().getName(), changeEvent), e); + } + } + + private static class ChangedIssueImpl implements ChangedIssue { + private final String key; + private final QGChangeEventListener.Status status; + private final RuleType type; + + private ChangedIssueImpl(DefaultIssue issue) { + this.key = issue.key(); + this.status = QGChangeEventListener.Status.valueOf(issue.getStatus()); + this.type = issue.type(); + } + + @Override + public String getKey() { + return key; + } + + @Override + public QGChangeEventListener.Status getStatus() { + return status; + } + + @Override + public RuleType getType() { + return type; + } + + @Override + public String toString() { + return "ChangedIssueImpl{" + + "key='" + key + '\'' + + ", status=" + status + + ", type=" + type + + '}'; } } + } diff --git a/server/sonar-server/src/main/java/org/sonar/server/webhook/WebhookQGChangeEventListener.java b/server/sonar-server/src/main/java/org/sonar/server/webhook/WebhookQGChangeEventListener.java index 59a2204cba4..e46cf3f5bd7 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/webhook/WebhookQGChangeEventListener.java +++ b/server/sonar-server/src/main/java/org/sonar/server/webhook/WebhookQGChangeEventListener.java @@ -19,11 +19,9 @@ */ package org.sonar.server.webhook; -import java.util.Collection; -import java.util.List; import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; -import org.sonar.core.util.stream.MoreCollectors; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.component.AnalysisPropertyDto; @@ -32,7 +30,6 @@ import org.sonar.db.component.ComponentDto; import org.sonar.db.component.SnapshotDto; import org.sonar.server.qualitygate.changeevent.QGChangeEvent; import org.sonar.server.qualitygate.changeevent.QGChangeEventListener; -import org.sonar.server.qualitygate.changeevent.Trigger; public class WebhookQGChangeEventListener implements QGChangeEventListener { private final WebHooks webhooks; @@ -46,20 +43,13 @@ public class WebhookQGChangeEventListener implements QGChangeEventListener { } @Override - public void onChanges(Trigger trigger, Collection changeEvents) { - if (changeEvents.isEmpty()) { - return; - } - - List branchesWithWebhooks = changeEvents.stream() - .filter(changeEvent -> webhooks.isEnabled(changeEvent.getProjectConfiguration())) - .collect(MoreCollectors.toList()); - if (branchesWithWebhooks.isEmpty()) { + public void onIssueChanges(QGChangeEvent qualityGateEvent, Set changedIssues) { + if (!webhooks.isEnabled(qualityGateEvent.getProjectConfiguration())) { return; } try (DbSession dbSession = dbClient.openSession(false)) { - branchesWithWebhooks.forEach(event -> callWebhook(dbSession, event)); + callWebhook(dbSession, qualityGateEvent); } } 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 f96602d71b1..e37d3afe290 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 @@ -20,29 +20,42 @@ package org.sonar.server.qualitygate.changeevent; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Random; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.apache.commons.lang.RandomStringUtils; +import org.assertj.core.groups.Tuple; import org.junit.Rule; import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.InOrder; import org.mockito.Mockito; +import org.sonar.api.issue.Issue; +import org.sonar.api.rules.RuleType; 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 org.sonar.server.qualitygate.changeevent.QGChangeEventListener.ChangedIssue; 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.assertj.core.api.Assertions.tuple; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.same; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; public class QGChangeEventListenersImplTest { @Rule @@ -51,9 +64,17 @@ 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 List listeners = Arrays.asList(listener1, listener2, listener3); + + private String component1Uuid = RandomStringUtils.randomAlphabetic(6); + private ComponentDto component1 = newComponentDto(component1Uuid); + private DefaultIssue component1Issue = newDefaultIssue(component1Uuid); + private QGChangeEventFactory.IssueChangeData oneIssueOnComponent1 = new QGChangeEventFactory.IssueChangeData( + singletonList(component1Issue), + singletonList(component1)); + private QGChangeEvent component1QGChangeEvent = newQGChangeEvent(component1); + private InOrder inOrder = Mockito.inOrder(listener1, listener2, listener3); - private List threeChangeEvents = Arrays.asList(mock(QGChangeEvent.class), mock(QGChangeEvent.class)); private QGChangeEventListenersImpl underTest = new QGChangeEventListenersImpl(new QGChangeEventListener[] {listener1, listener2, listener3}); @@ -61,44 +82,47 @@ public class QGChangeEventListenersImplTest { public void broadcastOnIssueChange_has_no_effect_when_issueChangeData_is_empty() { QGChangeEventFactory.IssueChangeData issueChangeData = new QGChangeEventFactory.IssueChangeData(emptyList(), emptyList()); - underTest.broadcastOnIssueChange(issueChangeData, threeChangeEvents); + underTest.broadcastOnIssueChange(issueChangeData, singletonList(component1QGChangeEvent)); 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())); + QGChangeEventFactory.IssueChangeData issueChangeData = new QGChangeEventFactory.IssueChangeData( + emptyList(), singletonList(newComponentDto(component1Uuid))); - underTest.broadcastOnIssueChange(issueChangeData, threeChangeEvents); + underTest.broadcastOnIssueChange(issueChangeData, singletonList(component1QGChangeEvent)); 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()); + QGChangeEventFactory.IssueChangeData issueChangeData = new QGChangeEventFactory.IssueChangeData( + singletonList(newDefaultIssue(component1Uuid)), emptyList()); - underTest.broadcastOnIssueChange(issueChangeData, threeChangeEvents); + underTest.broadcastOnIssueChange(issueChangeData, singletonList(component1QGChangeEvent)); verifyZeroInteractions(listener1, listener2, listener3); } @Test public void broadcastOnIssueChange_has_no_effect_when_no_changeEvent() { - - underTest.broadcastOnIssueChange(issueChangeData, Collections.emptySet()); + underTest.broadcastOnIssueChange(oneIssueOnComponent1, Collections.emptySet()); verifyZeroInteractions(listener1, listener2, listener3); } @Test - 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); - inOrder.verify(listener3).onChanges(Trigger.ISSUE_CHANGE, threeChangeEvents); + public void broadcastOnIssueChange_passes_same_arguments_to_all_listeners_in_order_of_addition_to_constructor() { + underTest.broadcastOnIssueChange(oneIssueOnComponent1, singletonList(component1QGChangeEvent)); + + ArgumentCaptor> changedIssuesCaptor = newSetCaptor(); + inOrder.verify(listener1).onIssueChanges(same(component1QGChangeEvent), changedIssuesCaptor.capture()); + Set changedIssues = changedIssuesCaptor.getValue(); + inOrder.verify(listener2).onIssueChanges(same(component1QGChangeEvent), same(changedIssues)); + inOrder.verify(listener3).onIssueChanges(same(component1QGChangeEvent), same(changedIssues)); inOrder.verifyNoMoreInteractions(); } @@ -107,13 +131,15 @@ public class QGChangeEventListenersImplTest { 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); + .onIssueChanges(any(), any()); - underTest.broadcastOnIssueChange(issueChangeData, threeChangeEvents); + underTest.broadcastOnIssueChange(oneIssueOnComponent1, singletonList(component1QGChangeEvent)); - inOrder.verify(listener1).onChanges(Trigger.ISSUE_CHANGE, threeChangeEvents); - inOrder.verify(listener2).onChanges(Trigger.ISSUE_CHANGE, threeChangeEvents); - inOrder.verify(listener3).onChanges(Trigger.ISSUE_CHANGE, threeChangeEvents); + ArgumentCaptor> changedIssuesCaptor = newSetCaptor(); + inOrder.verify(listener1).onIssueChanges(same(component1QGChangeEvent), changedIssuesCaptor.capture()); + Set changedIssues = changedIssuesCaptor.getValue(); + inOrder.verify(listener2).onIssueChanges(same(component1QGChangeEvent), same(changedIssues)); + inOrder.verify(listener3).onIssueChanges(same(component1QGChangeEvent), same(changedIssues)); inOrder.verifyNoMoreInteractions(); assertThat(logTester.logs()).hasSize(4); assertThat(logTester.logs(LoggerLevel.WARN)).hasSize(1); @@ -123,12 +149,14 @@ public class QGChangeEventListenersImplTest { 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); + .onIssueChanges(any(), any()); - underTest.broadcastOnIssueChange(issueChangeData, threeChangeEvents); + underTest.broadcastOnIssueChange(oneIssueOnComponent1, singletonList(component1QGChangeEvent)); - inOrder.verify(listener1).onChanges(Trigger.ISSUE_CHANGE, threeChangeEvents); - inOrder.verify(listener2).onChanges(Trigger.ISSUE_CHANGE, threeChangeEvents); + ArgumentCaptor> changedIssuesCaptor = newSetCaptor(); + inOrder.verify(listener1).onIssueChanges(same(component1QGChangeEvent), changedIssuesCaptor.capture()); + Set changedIssues = changedIssuesCaptor.getValue(); + inOrder.verify(listener2).onIssueChanges(same(component1QGChangeEvent), same(changedIssues)); inOrder.verifyNoMoreInteractions(); assertThat(logTester.logs()).hasSize(3); assertThat(logTester.logs(LoggerLevel.WARN)).hasSize(1); @@ -136,35 +164,132 @@ public class QGChangeEventListenersImplTest { @Test public void broadcastOnIssueChange_logs_each_listener_call_at_TRACE_level() { - underTest.broadcastOnIssueChange(issueChangeData, threeChangeEvents); + underTest.broadcastOnIssueChange(oneIssueOnComponent1, singletonList(component1QGChangeEvent)); assertThat(logTester.logs()).hasSize(3); List traceLogs = logTester.logs(LoggerLevel.TRACE); assertThat(traceLogs).hasSize(3) .containsOnly( - "calling onChange() on listener " + listener1.getClass().getName() + " for events " + threeChangeEvents.toString() + "...", - "calling onChange() on listener " + listener2.getClass().getName() + " for events " + threeChangeEvents.toString() + "...", - "calling onChange() on listener " + listener3.getClass().getName() + " for events " + threeChangeEvents.toString() + "..."); + "calling onChange() on listener " + listener1.getClass().getName() + " for events " + component1QGChangeEvent.toString() + "...", + "calling onChange() on listener " + listener2.getClass().getName() + " for events " + component1QGChangeEvent.toString() + "...", + "calling onChange() on listener " + listener3.getClass().getName() + " for events " + component1QGChangeEvent.toString() + "..."); } @Test - public void broadcastOnIssueChange_passes_immutable_list_of_events() { + public void broadcastOnIssueChange_passes_immutable_set_of_ChangedIssues() { QGChangeEventListenersImpl underTest = new QGChangeEventListenersImpl(new QGChangeEventListener[] {listener1}); - underTest.broadcastOnIssueChange(issueChangeData, threeChangeEvents); + underTest.broadcastOnIssueChange(oneIssueOnComponent1, singletonList(component1QGChangeEvent)); - ArgumentCaptor collectionCaptor = ArgumentCaptor.forClass(Collection.class); - verify(listener1).onChanges(eq(Trigger.ISSUE_CHANGE), collectionCaptor.capture()); - assertThat(collectionCaptor.getValue()).isInstanceOf(ImmutableList.class); + ArgumentCaptor> changedIssuesCaptor = newSetCaptor(); + inOrder.verify(listener1).onIssueChanges(same(component1QGChangeEvent), changedIssuesCaptor.capture()); + assertThat(changedIssuesCaptor.getValue()).isInstanceOf(ImmutableSet.class); } @Test public void broadcastOnIssueChange_has_no_effect_when_no_listener() { QGChangeEventListenersImpl underTest = new QGChangeEventListenersImpl(); - underTest.broadcastOnIssueChange(issueChangeData, threeChangeEvents); + underTest.broadcastOnIssueChange(oneIssueOnComponent1, singletonList(component1QGChangeEvent)); verifyZeroInteractions(listener1, listener2, listener3); } + @Test + public void broadcastOnIssueChange_calls_listener_for_each_component_uuid_with_at_least_one_QGChangeEvent() { + // component2 has multiple issues + ComponentDto component2 = newComponentDto(component1Uuid + "2"); + // component 3 has multiple QGChangeEvent and only one issue + ComponentDto component3 = newComponentDto(component1Uuid + "3"); + // component 4 has multiple QGChangeEvent and multiples issues + ComponentDto component4 = newComponentDto(component1Uuid + "4"); + // component 4 has no QGChangeEvent but one issue + ComponentDto component5 = newComponentDto(component1Uuid + "5"); + DefaultIssue[] component2Issues = {newDefaultIssue(component2.uuid()), newDefaultIssue(component2.uuid())}; + DefaultIssue component3Issue = newDefaultIssue(component3.uuid()); + DefaultIssue[] component4Issues = {newDefaultIssue(component4.uuid()), newDefaultIssue(component4.uuid())}; + DefaultIssue component5Issue = newDefaultIssue(component5.uuid()); + QGChangeEvent component2QGChangeEvent = newQGChangeEvent(component2); + QGChangeEvent[] component3QGChangeEvents = {newQGChangeEvent(component3), newQGChangeEvent(component3)}; + QGChangeEvent[] component4QGChangeEvents = {newQGChangeEvent(component4), newQGChangeEvent(component4)}; + List issues = Stream.of( + Stream.of(component1Issue), + Arrays.stream(component2Issues), + Stream.of(component3Issue), + Arrays.stream(component4Issues), + Stream.of(component5Issue)) + .flatMap(s -> s) + .collect(Collectors.toList()); + QGChangeEventFactory.IssueChangeData issueChangeData = new QGChangeEventFactory.IssueChangeData( + randomizedList(issues), + randomizedList(Arrays.asList(component1, component2, component3, component4))); + List qgChangeEvents = Stream.of( + Stream.of(component1QGChangeEvent), + Stream.of(component2QGChangeEvent), + Arrays.stream(component3QGChangeEvents), + Arrays.stream(component4QGChangeEvents)) + .flatMap(s -> s) + .collect(Collectors.toList()); + + underTest.broadcastOnIssueChange(issueChangeData, randomizedList(qgChangeEvents)); + + listeners.forEach(listener -> { + verifyListenerCalled(listener, component1QGChangeEvent, component1Issue); + verifyListenerCalled(listener, component2QGChangeEvent, component2Issues); + Arrays.stream(component3QGChangeEvents) + .forEach(component3QGChangeEvent -> verifyListenerCalled(listener, component3QGChangeEvent, component3Issue)); + Arrays.stream(component4QGChangeEvents) + .forEach(component4QGChangeEvent -> verifyListenerCalled(listener, component4QGChangeEvent, component4Issues)); + }); + verifyNoMoreInteractions(listener1, listener2, listener3); + } + + private void verifyListenerCalled(QGChangeEventListener listener, QGChangeEvent changeEvent, DefaultIssue... issues) { + ArgumentCaptor> changedIssuesCaptor = newSetCaptor(); + verify(listener).onIssueChanges(same(changeEvent), changedIssuesCaptor.capture()); + Set changedIssues = changedIssuesCaptor.getValue(); + Tuple[] expected = Arrays.stream(issues) + .map(issue -> tuple(issue.key(), issue.status(), issue.type())) + .toArray(Tuple[]::new); + assertThat(changedIssues) + .hasSize(issues.length) + .extracting(ChangedIssue::getKey, t -> t.getStatus().name(), ChangedIssue::getType) + .containsOnly(expected); + } + + private static final String[] STATUSES = Issue.STATUSES.stream().toArray(String[]::new); + private static int issueIdCounter = 0; + + private static DefaultIssue newDefaultIssue(String projectUuid) { + DefaultIssue defaultIssue = new DefaultIssue(); + defaultIssue.setKey("issue_" + issueIdCounter++); + defaultIssue.setProjectUuid(projectUuid); + defaultIssue.setType(RuleType.values()[new Random().nextInt(RuleType.values().length)]); + defaultIssue.setStatus(STATUSES[new Random().nextInt(STATUSES.length)]); + return defaultIssue; + } + + private static ComponentDto newComponentDto(String uuid) { + ComponentDto componentDto = new ComponentDto(); + componentDto.setUuid(uuid); + return componentDto; + } + + private static QGChangeEvent newQGChangeEvent(ComponentDto componentDto) { + QGChangeEvent res = mock(QGChangeEvent.class); + when(res.getProject()).thenReturn(componentDto); + return res; + } + + private static ArgumentCaptor> newSetCaptor() { + Class> clazz = (Class>) (Class) Set.class; + return ArgumentCaptor.forClass(clazz); + } + + private static List randomizedList(List issues) { + ArrayList res = new ArrayList<>(issues); + Collections.shuffle(res); + return ImmutableList.copyOf(res); + } + } diff --git a/server/sonar-server/src/test/java/org/sonar/server/webhook/WebhookQGChangeEventListenerTest.java b/server/sonar-server/src/test/java/org/sonar/server/webhook/WebhookQGChangeEventListenerTest.java index 3e7c4651195..e0593390537 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/webhook/WebhookQGChangeEventListenerTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/webhook/WebhookQGChangeEventListenerTest.java @@ -19,12 +19,11 @@ */ package org.sonar.server.webhook; -import com.google.common.collect.ImmutableList; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.function.Supplier; import javax.annotation.Nullable; import org.junit.Rule; @@ -47,11 +46,10 @@ import org.sonar.server.qualitygate.EvaluatedQualityGate; import org.sonar.server.qualitygate.QualityGate; import org.sonar.server.qualitygate.ShortLivingBranchQualityGate; import org.sonar.server.qualitygate.changeevent.QGChangeEvent; -import org.sonar.server.qualitygate.changeevent.Trigger; +import org.sonar.server.qualitygate.changeevent.QGChangeEventListener; import static java.lang.String.valueOf; import static java.util.Collections.emptySet; -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; @@ -71,6 +69,7 @@ public class WebhookQGChangeEventListenerTest { .setQualityGate(new QualityGate(valueOf(ShortLivingBranchQualityGate.ID), ShortLivingBranchQualityGate.NAME, emptySet())) .setStatus(EvaluatedQualityGate.Status.OK) .build(); + private static final Set CHANGED_ISSUES_ARE_IGNORED = emptySet(); @Rule public DbTester dbTester = DbTester.create(System2.INSTANCE); @@ -85,29 +84,19 @@ public class WebhookQGChangeEventListenerTest { private WebhookQGChangeEventListener mockedUnderTest = new WebhookQGChangeEventListener(webHooks, webhookPayloadFactory, mockedDbClient); @Test - public void onChanges_has_no_effect_if_changeEvents_is_empty() { - mockedUnderTest.onChanges(Trigger.ISSUE_CHANGE, Collections.emptyList()); - - verifyZeroInteractions(webHooks, webhookPayloadFactory, mockedDbClient); - } - - @Test - public void onChanges_has_no_effect_if_no_webhook_is_configured() { + public void onIssueChanges_has_no_effect_if_no_webhook_is_configured() { Configuration configuration1 = mock(Configuration.class); - Configuration configuration2 = mock(Configuration.class); - mockWebhookDisabled(configuration1, configuration2); + mockWebhookDisabled(configuration1); + QGChangeEvent qualityGateEvent = new QGChangeEvent(new ComponentDto(), new BranchDto(), new SnapshotDto(), configuration1, Optional::empty); - mockedUnderTest.onChanges(Trigger.ISSUE_CHANGE, ImmutableList.of( - new QGChangeEvent(new ComponentDto(), new BranchDto(), new SnapshotDto(), configuration1, Optional::empty), - new QGChangeEvent(new ComponentDto(), new BranchDto(), new SnapshotDto(), configuration2, Optional::empty))); + mockedUnderTest.onIssueChanges(qualityGateEvent, CHANGED_ISSUES_ARE_IGNORED); verify(webHooks).isEnabled(configuration1); - verify(webHooks).isEnabled(configuration2); verifyZeroInteractions(webhookPayloadFactory, mockedDbClient); } @Test - public void onChanges_calls_webhook_for_changeEvent_with_webhook_enabled() { + public void onIssueChanges_calls_webhook_for_changeEvent_with_webhook_enabled() { OrganizationDto organization = dbTester.organizations().insert(); ComponentDto project = dbTester.components().insertPublicProject(organization); ComponentAndBranch branch = insertProjectBranch(project, BranchType.SHORT, "foo"); @@ -119,8 +108,9 @@ public class WebhookQGChangeEventListenerTest { properties.put("sonar.analysis.test1", randomAlphanumeric(50)); properties.put("sonar.analysis.test2", randomAlphanumeric(5000)); insertPropertiesFor(analysis.getUuid(), properties); + QGChangeEvent qualityGateEvent = newQGChangeEvent(branch, analysis, configuration, EVALUATED_QUALITY_GATE_1); - underTest.onChanges(Trigger.ISSUE_CHANGE, singletonList(newQGChangeEvent(branch, analysis, configuration, EVALUATED_QUALITY_GATE_1))); + underTest.onIssueChanges(qualityGateEvent, CHANGED_ISSUES_ARE_IGNORED); ProjectAnalysis projectAnalysis = verifyWebhookCalledAndExtractPayloadFactoryArgument(branch, configuration, analysis); assertThat(projectAnalysis).isEqualTo( @@ -135,68 +125,41 @@ public class WebhookQGChangeEventListenerTest { } @Test - public void onChanges_does_not_call_webhook_if_disabled_for_QGChangeEvent() { + public void onIssueChanges_calls_webhook_on_main_branch() { OrganizationDto organization = dbTester.organizations().insert(); - ComponentDto project = dbTester.components().insertPublicProject(organization); - ComponentAndBranch branch1 = insertProjectBranch(project, BranchType.SHORT, "foo"); - ComponentAndBranch branch2 = insertProjectBranch(project, BranchType.SHORT, "bar"); - SnapshotDto analysis1 = insertAnalysisTask(branch1); - SnapshotDto analysis2 = insertAnalysisTask(branch2); - Configuration configuration1 = mock(Configuration.class); - Configuration configuration2 = mock(Configuration.class); - mockWebhookDisabled(configuration1); - mockWebhookEnabled(configuration2); - mockPayloadSupplierConsumedByWebhooks(); + ComponentAndBranch mainBranch = insertMainBranch(organization); + SnapshotDto analysis = insertAnalysisTask(mainBranch); + Configuration configuration = mock(Configuration.class); + mockWebhookEnabled(configuration); + QGChangeEvent qualityGateEvent = newQGChangeEvent(mainBranch, analysis, configuration, EVALUATED_QUALITY_GATE_1); - underTest.onChanges( - Trigger.ISSUE_CHANGE, - ImmutableList.of( - newQGChangeEvent(branch1, analysis1, configuration1, null), - newQGChangeEvent(branch2, analysis2, configuration2, EVALUATED_QUALITY_GATE_1))); + underTest.onIssueChanges(qualityGateEvent, CHANGED_ISSUES_ARE_IGNORED); - verifyWebhookNotCalled(branch1, analysis1, configuration1); - verifyWebhookCalled(branch2, analysis2, configuration2); + verifyWebhookCalled(mainBranch, analysis, configuration); } @Test - public void onChanges_calls_webhook_for_any_type_of_branch() { - OrganizationDto organization = dbTester.organizations().insert(); - ComponentAndBranch mainBranch = insertMainBranch(organization); - ComponentAndBranch longBranch = insertProjectBranch(mainBranch.component, BranchType.LONG, "foo"); - SnapshotDto analysis1 = insertAnalysisTask(mainBranch); - SnapshotDto analysis2 = insertAnalysisTask(longBranch); - Configuration configuration1 = mock(Configuration.class); - Configuration configuration2 = mock(Configuration.class); - mockWebhookEnabled(configuration1, configuration2); - - underTest.onChanges(Trigger.ISSUE_CHANGE, ImmutableList.of( - newQGChangeEvent(mainBranch, analysis1, configuration1, EVALUATED_QUALITY_GATE_1), - newQGChangeEvent(longBranch, analysis2, configuration2, null))); - - verifyWebhookCalled(mainBranch, analysis1, configuration1); - verifyWebhookCalled(longBranch, analysis2, configuration2); + public void onIssueChanges_calls_webhook_on_long_branch() { + onIssueChangesCallsWebhookOnBranch(BranchType.LONG); } @Test - public void onChanges_calls_webhook_once_per_QGChangeEvent_even_for_same_branch_and_configuration() { + public void onIssueChanges_calls_webhook_on_short_branch() { + onIssueChangesCallsWebhookOnBranch(BranchType.SHORT); + } + + public void onIssueChangesCallsWebhookOnBranch(BranchType branchType) { OrganizationDto organization = dbTester.organizations().insert(); - ComponentAndBranch branch1 = insertPrivateBranch(organization, BranchType.SHORT); - SnapshotDto analysis1 = insertAnalysisTask(branch1); - Configuration configuration1 = mock(Configuration.class); - mockWebhookEnabled(configuration1); - mockPayloadSupplierConsumedByWebhooks(); + ComponentAndBranch mainBranch = insertMainBranch(organization); + ComponentAndBranch longBranch = insertProjectBranch(mainBranch.component, branchType, "foo"); + SnapshotDto analysis = insertAnalysisTask(longBranch); + Configuration configuration = mock(Configuration.class); + mockWebhookEnabled(configuration); + QGChangeEvent qualityGateEvent = newQGChangeEvent(longBranch, analysis, configuration, null); - underTest.onChanges(Trigger.ISSUE_CHANGE, ImmutableList.of( - newQGChangeEvent(branch1, analysis1, configuration1, null), - newQGChangeEvent(branch1, analysis1, configuration1, EVALUATED_QUALITY_GATE_1), - newQGChangeEvent(branch1, analysis1, configuration1, null))); + underTest.onIssueChanges(qualityGateEvent, CHANGED_ISSUES_ARE_IGNORED); - verify(webHooks, times(3)).isEnabled(configuration1); - verify(webHooks, times(3)).sendProjectAnalysisUpdate( - Matchers.same(configuration1), - Matchers.eq(new WebHooks.Analysis(branch1.uuid(), analysis1.getUuid(), null)), - any(Supplier.class)); - extractPayloadFactoryArguments(3); + verifyWebhookCalled(longBranch, analysis, configuration); } private void mockWebhookEnabled(Configuration... configurations) { -- 2.39.5