From cc4b808264013326716a0ce7ef67eb1bd4452a6f Mon Sep 17 00:00:00 2001 From: Guillaume Jambet Date: Fri, 9 Feb 2018 16:45:10 +0100 Subject: [PATCH] SONAR-10345 Webhooks consumers source them from WEBHOOKS table. --- .../webhook/WebhookPostTask.java | 9 +--- .../org/sonar/server/webhook/WebHooks.java | 16 +++++-- .../webhook/WebhookQGChangeEventListener.java | 5 +- .../webhook/WebhookPostTaskTest.java | 4 +- .../WebhookQGChangeEventListenerTest.java | 48 +++++++++---------- 5 files changed, 41 insertions(+), 41 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/webhook/WebhookPostTask.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/webhook/WebhookPostTask.java index 576df71153e..e8be7daead6 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/webhook/WebhookPostTask.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/webhook/WebhookPostTask.java @@ -23,10 +23,8 @@ import java.util.Map; import java.util.Optional; import java.util.Set; import org.sonar.api.ce.posttask.PostProjectAnalysisTask; -import org.sonar.api.config.Configuration; import org.sonar.api.measures.Metric; import org.sonar.core.util.stream.MoreCollectors; -import org.sonar.server.computation.task.projectanalysis.component.ConfigurationRepository; import org.sonar.server.qualitygate.Condition; import org.sonar.server.qualitygate.EvaluatedCondition; import org.sonar.server.qualitygate.EvaluatedQualityGate; @@ -39,22 +37,17 @@ import org.sonar.server.webhook.WebhookPayloadFactory; public class WebhookPostTask implements PostProjectAnalysisTask { - private final ConfigurationRepository configRepository; private final WebhookPayloadFactory payloadFactory; private final WebHooks webHooks; - public WebhookPostTask(ConfigurationRepository configRepository, WebhookPayloadFactory payloadFactory, WebHooks webHooks) { - this.configRepository = configRepository; + public WebhookPostTask(WebhookPayloadFactory payloadFactory, WebHooks webHooks) { this.payloadFactory = payloadFactory; this.webHooks = webHooks; } @Override public void finished(ProjectAnalysis analysis) { - Configuration config = configRepository.getConfiguration(); - webHooks.sendProjectAnalysisUpdate( - config, new WebHooks.Analysis( analysis.getProject().getUuid(), analysis.getAnalysis().map(org.sonar.api.ce.posttask.Analysis::getAnalysisUuid).orElse(null), diff --git a/server/sonar-server/src/main/java/org/sonar/server/webhook/WebHooks.java b/server/sonar-server/src/main/java/org/sonar/server/webhook/WebHooks.java index 4480661e88b..70cb9c858f8 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/webhook/WebHooks.java +++ b/server/sonar-server/src/main/java/org/sonar/server/webhook/WebHooks.java @@ -24,24 +24,32 @@ import java.util.function.Supplier; import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.sonar.api.config.Configuration; +import org.sonar.db.component.ComponentDto; import static java.util.Objects.requireNonNull; public interface WebHooks { + /** - * Tells whether any webHook is configured at all for the specified {@link Configuration}. + * Tells whether any webHook is configured for the specified {@link Configuration}. * *

* This can be used to not do consuming operations before calling - * {@link #sendProjectAnalysisUpdate(Configuration, Analysis, Supplier)} + * {@link #sendProjectAnalysisUpdate(ComponentDto, Analysis, Supplier)} + */ + boolean isEnabled(ComponentDto projectDto); + + /** + * Calls all WebHooks configured in the specified {@link Configuration} for the specified analysis with the + * {@link WebhookPayload} provided by the specified Supplier. */ - boolean isEnabled(Configuration configuration); + void sendProjectAnalysisUpdate(ComponentDto projectDto, Analysis analysis, Supplier payloadSupplier); /** * Calls all WebHooks configured in the specified {@link Configuration} for the specified analysis with the * {@link WebhookPayload} provided by the specified Supplier. */ - void sendProjectAnalysisUpdate(Configuration configuration, Analysis analysis, Supplier payloadSupplier); + void sendProjectAnalysisUpdate(Analysis analysis, Supplier payloadSupplier); final class Analysis { private final String projectUuid; 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 f73d05506ff..a6fb71e4a6e 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 @@ -50,7 +50,8 @@ public class WebhookQGChangeEventListener implements QGChangeEventListener { @Override public void onIssueChanges(QGChangeEvent qualityGateEvent, Set changedIssues) { - if (!webhooks.isEnabled(qualityGateEvent.getProjectConfiguration())) { + + if (!webhooks.isEnabled(qualityGateEvent.getProject())) { return; } Optional evaluatedQualityGate = qualityGateEvent.getQualityGateSupplier().get(); @@ -78,7 +79,7 @@ public class WebhookQGChangeEventListener implements QGChangeEventListener { private void callWebhook(DbSession dbSession, QGChangeEvent event, @Nullable EvaluatedQualityGate evaluatedQualityGate) { webhooks.sendProjectAnalysisUpdate( - event.getProjectConfiguration(), + event.getProject(), new WebHooks.Analysis(event.getBranch().getUuid(), event.getAnalysis().getUuid(), null), () -> buildWebHookPayload(dbSession, event, evaluatedQualityGate)); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/webhook/WebhookPostTaskTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/webhook/WebhookPostTaskTest.java index a1d76d8df90..8b2ef5f6858 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/webhook/WebhookPostTaskTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/webhook/WebhookPostTaskTest.java @@ -50,7 +50,6 @@ 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.verify; import static org.mockito.Mockito.when; @@ -69,7 +68,7 @@ public class WebhookPostTaskTest { private final WebhookPayloadFactory payloadFactory = mock(WebhookPayloadFactory.class); private final WebHooks webHooks = mock(WebHooks.class); private final ConfigurationRepository configurationRepository = mock(ConfigurationRepository.class); - private WebhookPostTask underTest = new WebhookPostTask(configurationRepository, payloadFactory, webHooks); + private WebhookPostTask underTest = new WebhookPostTask(payloadFactory, webHooks); @Before public void wireMocks() { @@ -135,7 +134,6 @@ public class WebhookPostTaskTest { ArgumentCaptor supplierCaptor = ArgumentCaptor.forClass(Supplier.class); verify(webHooks) .sendProjectAnalysisUpdate( - same(configuration), eq(new WebHooks.Analysis(project.getUuid(), analysisUUid, ceTask.getId())), 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 e93bc8321c5..64619526680 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 @@ -66,6 +66,7 @@ import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import static org.sonar.core.util.stream.MoreCollectors.toArrayList; import static org.sonar.db.component.BranchType.LONG; +import static org.sonar.db.component.BranchType.SHORT; import static org.sonar.db.component.ComponentTesting.newBranchDto; import static org.sonar.db.component.ComponentTesting.newPrivateProjectDto; @@ -91,13 +92,13 @@ public class WebhookQGChangeEventListenerTest { @UseDataProvider("allCombinationsOfStatuses") public void onIssueChanges_has_no_effect_if_no_webhook_is_configured(Metric.Level previousStatus, Metric.Level newStatus) { Configuration configuration1 = mock(Configuration.class); - mockWebhookDisabled(configuration1); when(newQualityGate.getStatus()).thenReturn(newStatus); QGChangeEvent qualityGateEvent = newQGChangeEvent(configuration1, previousStatus, newQualityGate); + mockWebhookDisabled(qualityGateEvent.getProject()); mockedUnderTest.onIssueChanges(qualityGateEvent, CHANGED_ISSUES_ARE_IGNORED); - verify(webHooks).isEnabled(configuration1); + verify(webHooks).isEnabled(qualityGateEvent.getProject()); verifyZeroInteractions(webhookPayloadFactory, mockedDbClient); } @@ -120,8 +121,8 @@ public class WebhookQGChangeEventListenerTest { @Test public void onIssueChanges_has_no_effect_if_event_has_neither_previousQGStatus_nor_qualityGate() { Configuration configuration = mock(Configuration.class); - mockWebhookEnabled(configuration); QGChangeEvent qualityGateEvent = newQGChangeEvent(configuration, null, null); + mockWebhookEnabled(qualityGateEvent.getProject()); underTest.onIssueChanges(qualityGateEvent, CHANGED_ISSUES_ARE_IGNORED); @@ -131,10 +132,10 @@ public class WebhookQGChangeEventListenerTest { @Test public void onIssueChanges_has_no_effect_if_event_has_same_status_in_previous_and_new_QG() { Configuration configuration = mock(Configuration.class); - mockWebhookEnabled(configuration); Metric.Level previousStatus = randomLevel(); when(newQualityGate.getStatus()).thenReturn(previousStatus); QGChangeEvent qualityGateEvent = newQGChangeEvent(configuration, previousStatus, newQualityGate); + mockWebhookEnabled(qualityGateEvent.getProject()); underTest.onIssueChanges(qualityGateEvent, CHANGED_ISSUES_ARE_IGNORED); @@ -149,17 +150,17 @@ public class WebhookQGChangeEventListenerTest { ComponentAndBranch branch = insertProjectBranch(project, BranchType.SHORT, "foo"); SnapshotDto analysis = insertAnalysisTask(branch); Configuration configuration = mock(Configuration.class); - mockWebhookEnabled(configuration); mockPayloadSupplierConsumedByWebhooks(); Map properties = new HashMap<>(); properties.put("sonar.analysis.test1", randomAlphanumeric(50)); properties.put("sonar.analysis.test2", randomAlphanumeric(5000)); insertPropertiesFor(analysis.getUuid(), properties); QGChangeEvent qualityGateEvent = newQGChangeEvent(branch, analysis, configuration, newQualityGate); + mockWebhookEnabled(qualityGateEvent.getProject()); underTest.onIssueChanges(qualityGateEvent, CHANGED_ISSUES_ARE_IGNORED); - ProjectAnalysis projectAnalysis = verifyWebhookCalledAndExtractPayloadFactoryArgument(branch, configuration, analysis); + ProjectAnalysis projectAnalysis = verifyWebhookCalledAndExtractPayloadFactoryArgument(branch, analysis, qualityGateEvent.getProject()); assertThat(projectAnalysis).isEqualTo( new ProjectAnalysis( new Project(project.uuid(), project.getKey(), project.name()), @@ -178,12 +179,12 @@ public class WebhookQGChangeEventListenerTest { ComponentAndBranch mainBranch = insertMainBranch(organization); SnapshotDto analysis = insertAnalysisTask(mainBranch); Configuration configuration = mock(Configuration.class); - mockWebhookEnabled(configuration); QGChangeEvent qualityGateEvent = newQGChangeEvent(mainBranch, analysis, configuration, newQualityGate); + mockWebhookEnabled(qualityGateEvent.getProject()); underTest.onIssueChanges(qualityGateEvent, CHANGED_ISSUES_ARE_IGNORED); - verifyWebhookCalled(mainBranch, analysis, configuration); + verifyWebhookCalled(mainBranch, analysis, qualityGateEvent.getProject()); } @Test @@ -193,7 +194,7 @@ public class WebhookQGChangeEventListenerTest { @Test public void onIssueChanges_calls_webhook_on_short_branch() { - onIssueChangesCallsWebhookOnBranch(BranchType.SHORT); + onIssueChangesCallsWebhookOnBranch(SHORT); } public void onIssueChangesCallsWebhookOnBranch(BranchType branchType) { @@ -202,12 +203,12 @@ public class WebhookQGChangeEventListenerTest { 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); + mockWebhookEnabled(qualityGateEvent.getProject()); underTest.onIssueChanges(qualityGateEvent, CHANGED_ISSUES_ARE_IGNORED); - verifyWebhookCalled(longBranch, analysis, configuration); + verifyWebhookCalled(longBranch, analysis, qualityGateEvent.getProject()); } @DataProvider @@ -219,25 +220,25 @@ public class WebhookQGChangeEventListenerTest { }; } - private void mockWebhookEnabled(Configuration... configurations) { - for (Configuration configuration : configurations) { - when(webHooks.isEnabled(configuration)).thenReturn(true); + private void mockWebhookEnabled(ComponentDto... projects) { + for (ComponentDto dto : projects) { + when(webHooks.isEnabled(dto)).thenReturn(true); } } - private void mockWebhookDisabled(Configuration... configurations) { - for (Configuration configuration : configurations) { - when(webHooks.isEnabled(configuration)).thenReturn(false); + private void mockWebhookDisabled(ComponentDto... projects) { + for (ComponentDto dto : projects) { + when(webHooks.isEnabled(dto)).thenReturn(false); } } private void mockPayloadSupplierConsumedByWebhooks() { Mockito.doAnswer(invocationOnMock -> { - Supplier supplier = (Supplier) invocationOnMock.getArguments()[2]; + Supplier supplier = (Supplier) invocationOnMock.getArguments()[1]; supplier.get(); return null; }).when(webHooks) - .sendProjectAnalysisUpdate(Matchers.any(Configuration.class), Matchers.any(), Matchers.any()); + .sendProjectAnalysisUpdate(Matchers.any(), Matchers.any()); } private void insertPropertiesFor(String snapshotUuid, Map properties) { @@ -256,16 +257,15 @@ public class WebhookQGChangeEventListenerTest { return dbTester.components().insertSnapshot(componentAndBranch.component); } - private ProjectAnalysis verifyWebhookCalledAndExtractPayloadFactoryArgument(ComponentAndBranch componentAndBranch, Configuration configuration, SnapshotDto analysis) { - verifyWebhookCalled(componentAndBranch, analysis, configuration); + private ProjectAnalysis verifyWebhookCalledAndExtractPayloadFactoryArgument(ComponentAndBranch componentAndBranch, SnapshotDto analysis, ComponentDto project) { + verifyWebhookCalled(componentAndBranch, analysis, project); return extractPayloadFactoryArguments(1).iterator().next(); } - private void verifyWebhookCalled(ComponentAndBranch componentAndBranch, SnapshotDto analysis, Configuration branchConfiguration) { - verify(webHooks).isEnabled(branchConfiguration); + private void verifyWebhookCalled(ComponentAndBranch componentAndBranch, SnapshotDto analysis, ComponentDto project) { + verify(webHooks).isEnabled(project); verify(webHooks).sendProjectAnalysisUpdate( - Matchers.same(branchConfiguration), Matchers.eq(new WebHooks.Analysis(componentAndBranch.uuid(), analysis.getUuid(), null)), any(Supplier.class)); } -- 2.39.5