From 8d54befbab56cd1ba14df08456539b6fdb2561ad Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Tue, 23 Jul 2019 13:20:05 +0200 Subject: [PATCH] SONAR-11722 webhook posttask adds log statistics --- .../webhook/WebhookPostTask.java | 17 +++-- .../webhook/WebhookPostTaskTest.java | 4 +- .../org/sonar/server/webhook/WebHooks.java | 8 ++- .../sonar/server/webhook/WebHooksImpl.java | 29 ++++++-- .../webhook/AsynchronousWebHooksImplTest.java | 11 +-- .../webhook/SynchronousWebHooksImplTest.java | 68 ++++++++++++++++--- .../WebhookQGChangeEventListenerTest.java | 3 +- 7 files changed, 106 insertions(+), 34 deletions(-) diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/webhook/WebhookPostTask.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/webhook/WebhookPostTask.java index 816e874f8e5..b3e16bfb716 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/webhook/WebhookPostTask.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/webhook/WebhookPostTask.java @@ -21,6 +21,7 @@ package org.sonar.ce.task.projectanalysis.webhook; import java.util.Map; import java.util.Optional; +import java.util.function.Supplier; import org.sonar.api.ce.posttask.PostProjectAnalysisTask; import org.sonar.ce.task.projectanalysis.api.posttask.QGToEvaluatedQG; import org.sonar.server.qualitygate.EvaluatedQualityGate; @@ -29,6 +30,7 @@ import org.sonar.server.webhook.Branch; import org.sonar.server.webhook.CeTask; import org.sonar.server.webhook.Project; import org.sonar.server.webhook.WebHooks; +import org.sonar.server.webhook.WebhookPayload; import org.sonar.server.webhook.WebhookPayloadFactory; public class WebhookPostTask implements PostProjectAnalysisTask { @@ -47,13 +49,14 @@ public class WebhookPostTask implements PostProjectAnalysisTask { @Override public void finished(Context context) { - ProjectAnalysis analysis = context.getProjectAnalysis(); - webHooks.sendProjectAnalysisUpdate( - new WebHooks.Analysis( - analysis.getProject().getUuid(), - analysis.getAnalysis().map(org.sonar.api.ce.posttask.Analysis::getAnalysisUuid).orElse(null), - analysis.getCeTask().getId()), - () -> payloadFactory.create(convert(analysis))); + ProjectAnalysis projectAnalysis = context.getProjectAnalysis(); + WebHooks.Analysis analysis = new WebHooks.Analysis( + projectAnalysis.getProject().getUuid(), + projectAnalysis.getAnalysis().map(org.sonar.api.ce.posttask.Analysis::getAnalysisUuid).orElse(null), + projectAnalysis.getCeTask().getId()); + Supplier payloadSupplier = () -> payloadFactory.create(convert(projectAnalysis)); + + webHooks.sendProjectAnalysisUpdate(analysis, payloadSupplier, context.getLogStatistics()); } private static org.sonar.server.webhook.ProjectAnalysis convert(ProjectAnalysis projectAnalysis) { diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/webhook/WebhookPostTaskTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/webhook/WebhookPostTaskTest.java index 45db5743b4d..ac8c10c2f40 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/webhook/WebhookPostTaskTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/webhook/WebhookPostTaskTest.java @@ -31,6 +31,7 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; import org.sonar.api.ce.posttask.Branch; import org.sonar.api.ce.posttask.CeTask; +import org.sonar.api.ce.posttask.PostProjectAnalysisTask.LogStatistics; import org.sonar.api.ce.posttask.PostProjectAnalysisTaskTester; import org.sonar.api.ce.posttask.Project; import org.sonar.api.ce.posttask.QualityGate; @@ -141,7 +142,8 @@ public class WebhookPostTaskTest { eq(new WebHooks.Analysis(project.getUuid(), analysisUUid, ceTask.getId())), - supplierCaptor.capture()); + supplierCaptor.capture(), + any(LogStatistics.class)); assertThat(supplierCaptor.getValue().get()).isSameAs(webhookPayload); diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebHooks.java b/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebHooks.java index 0f6747f8a6c..be851cfad90 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebHooks.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebHooks.java @@ -23,6 +23,7 @@ import java.util.Objects; import java.util.function.Supplier; import javax.annotation.CheckForNull; import javax.annotation.Nullable; +import org.sonar.api.ce.posttask.PostProjectAnalysisTask; import org.sonar.api.config.Configuration; import org.sonar.db.component.ComponentDto; @@ -35,7 +36,7 @@ public interface WebHooks { * *

* This can be used to not do consuming operations before calling - * {@link #sendProjectAnalysisUpdate(Analysis, Supplier)} + * {@link #sendProjectAnalysisUpdate(Analysis, Supplier, PostProjectAnalysisTask.LogStatistics)} */ boolean isEnabled(ComponentDto projectDto); @@ -45,6 +46,11 @@ public interface WebHooks { */ void sendProjectAnalysisUpdate(Analysis analysis, Supplier payloadSupplier); + /** + * Override to be called from a {@link PostProjectAnalysisTask} implementation. + */ + void sendProjectAnalysisUpdate(Analysis analysis, Supplier payloadSupplier, PostProjectAnalysisTask.LogStatistics taskLogStatistics); + final class Analysis { private final String projectUuid; private final String ceTaskUuid; diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebHooksImpl.java b/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebHooksImpl.java index 974746a2e44..9c125cf79f6 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebHooksImpl.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebHooksImpl.java @@ -23,7 +23,10 @@ import java.util.List; import java.util.Optional; import java.util.function.Supplier; import java.util.stream.Stream; +import javax.annotation.CheckForNull; +import javax.annotation.Nullable; import org.sonar.api.ce.ComputeEngineSide; +import org.sonar.api.ce.posttask.PostProjectAnalysisTask; import org.sonar.api.server.ServerSide; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; @@ -57,12 +60,12 @@ public class WebHooksImpl implements WebHooks { @Override public boolean isEnabled(ComponentDto projectDto) { - return readWebHooksFrom(projectDto.uuid()) + return readWebHooksFrom(projectDto.uuid(), null) .findAny() .isPresent(); } - private Stream readWebHooksFrom(String projectUuid) { + private Stream readWebHooksFrom(String projectUuid, @CheckForNull PostProjectAnalysisTask.LogStatistics taskLogStatistics) { try (DbSession dbSession = dbClient.openSession(false)) { Optional optionalComponentDto = dbClient.componentDao().selectByUuid(dbSession, projectUuid); @@ -74,9 +77,13 @@ public class WebHooksImpl implements WebHooks { } WebhookDao dao = dbClient.webhookDao(); - return Stream.concat( - dao.selectByProject(dbSession, componentDto).stream(), - dao.selectByOrganizationUuid(dbSession, componentDto.getOrganizationUuid()).stream()); + List projectWebhooks = dao.selectByProject(dbSession, componentDto); + List organizationWebhooks = dao.selectByOrganizationUuid(dbSession, componentDto.getOrganizationUuid()); + if (taskLogStatistics != null) { + taskLogStatistics.add("globalWebhooks", organizationWebhooks.size()); + taskLogStatistics.add("projectWebhooks", projectWebhooks.size()); + } + return Stream.concat(projectWebhooks.stream(), organizationWebhooks.stream()); } } @@ -90,7 +97,17 @@ public class WebHooksImpl implements WebHooks { @Override public void sendProjectAnalysisUpdate(Analysis analysis, Supplier payloadSupplier) { - List webhooks = readWebHooksFrom(analysis.getProjectUuid()) + sendProjectAnalysisUpdateImpl(analysis, payloadSupplier, null); + } + + @Override + public void sendProjectAnalysisUpdate(Analysis analysis, Supplier payloadSupplier, PostProjectAnalysisTask.LogStatistics taskLogStatistics) { + sendProjectAnalysisUpdateImpl(analysis, payloadSupplier, taskLogStatistics); + } + + private void sendProjectAnalysisUpdateImpl(Analysis analysis, Supplier payloadSupplier, + @Nullable PostProjectAnalysisTask.LogStatistics taskLogStatistics) { + List webhooks = readWebHooksFrom(analysis.getProjectUuid(), taskLogStatistics) .map(dto -> new Webhook(dto.getUuid(), analysis.getProjectUuid(), analysis.getCeTaskUuid(), analysis.getAnalysisUuid(), dto.getName(), dto.getUrl(), dto.getSecret())) .collect(MoreCollectors.toList()); diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/webhook/AsynchronousWebHooksImplTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/webhook/AsynchronousWebHooksImplTest.java index 3d38d171e06..a0bc00df5c9 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/webhook/AsynchronousWebHooksImplTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/webhook/AsynchronousWebHooksImplTest.java @@ -24,15 +24,14 @@ import java.util.ArrayList; import java.util.List; import org.junit.Rule; import org.junit.Test; +import org.sonar.api.ce.posttask.PostProjectAnalysisTask.LogStatistics; import org.sonar.api.utils.System2; import org.sonar.db.DbTester; import org.sonar.db.component.ComponentDbTester; import org.sonar.db.component.ComponentDto; -import org.sonar.db.organization.OrganizationDbTester; import org.sonar.db.organization.OrganizationDto; import org.sonar.db.webhook.WebhookDbTester; import org.sonar.server.async.AsyncExecution; -import org.sonar.server.organization.DefaultOrganizationProvider; import static java.util.Objects.requireNonNull; import static org.assertj.core.api.Assertions.assertThat; @@ -43,7 +42,6 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyZeroInteractions; import static org.sonar.db.DbTester.create; import static org.sonar.db.webhook.WebhookTesting.newWebhook; -import static org.sonar.server.organization.TestDefaultOrganizationProvider.from; public class AsynchronousWebHooksImplTest { @@ -53,12 +51,9 @@ public class AsynchronousWebHooksImplTest { public DbTester db = create(system2); private WebhookDbTester webhookDbTester = db.webhooks(); private ComponentDbTester componentDbTester = db.components(); - private OrganizationDbTester organizationDbTester = db.organizations(); - private DefaultOrganizationProvider defaultOrganizationProvider = from(db); private static final long NOW = 1_500_000_000_000L; - private final TestWebhookCaller caller = new TestWebhookCaller(); private final WebhookDeliveryStorage deliveryStorage = mock(WebhookDeliveryStorage.class); private final WebhookPayload mock = mock(WebhookPayload.class); @@ -69,7 +64,7 @@ public class AsynchronousWebHooksImplTest { @Test public void send_global_webhooks() { - OrganizationDto organizationDto = db.getDefaultOrganization() ; + OrganizationDto organizationDto = db.getDefaultOrganization(); ComponentDto project = componentDbTester.insertPrivateProject(componentDto -> componentDto.setOrganizationUuid(organizationDto.getUuid())); webhookDbTester.insert(newWebhook(organizationDto).setName("First").setUrl("http://url1")); webhookDbTester.insert(newWebhook(organizationDto).setName("Second").setUrl("http://url2")); @@ -77,7 +72,7 @@ public class AsynchronousWebHooksImplTest { caller.enqueueSuccess(NOW, 200, 1_234); caller.enqueueFailure(NOW, new IOException("Fail to connect")); - underTest.sendProjectAnalysisUpdate(new WebHooks.Analysis(project.uuid(), "1", "#1"), () -> mock); + underTest.sendProjectAnalysisUpdate(new WebHooks.Analysis(project.uuid(), "1", "#1"), () -> mock, mock(LogStatistics.class)); assertThat(caller.countSent()).isZero(); verifyZeroInteractions(deliveryStorage); diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/webhook/SynchronousWebHooksImplTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/webhook/SynchronousWebHooksImplTest.java index 9a5f3b99fc0..32b925ac9c8 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/webhook/SynchronousWebHooksImplTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/webhook/SynchronousWebHooksImplTest.java @@ -20,13 +20,16 @@ package org.sonar.server.webhook; import java.io.IOException; +import java.util.List; import org.junit.Rule; import org.junit.Test; +import org.sonar.api.ce.posttask.PostProjectAnalysisTask; import org.sonar.api.utils.log.LogTester; import org.sonar.db.DbClient; import org.sonar.db.DbTester; import org.sonar.db.component.ComponentDbTester; import org.sonar.db.component.ComponentDto; +import org.sonar.db.organization.OrganizationDto; import org.sonar.db.webhook.WebhookDbTester; import org.sonar.server.async.AsyncExecution; import org.sonar.server.organization.DefaultOrganizationProvider; @@ -36,6 +39,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.verifyZeroInteractions; import static org.sonar.api.utils.log.LoggerLevel.DEBUG; import static org.sonar.db.DbTester.create; @@ -61,6 +65,7 @@ public class SynchronousWebHooksImplTest { private final WebhookDeliveryStorage deliveryStorage = mock(WebhookDeliveryStorage.class); private final WebhookPayload mock = mock(WebhookPayload.class); private final AsyncExecution synchronousAsyncExecution = Runnable::run; + private final PostProjectAnalysisTask.LogStatistics taskStatistics = mock(PostProjectAnalysisTask.LogStatistics.class); private final WebHooksImpl underTest = new WebHooksImpl(caller, deliveryStorage, synchronousAsyncExecution, dbClient); @Test @@ -87,7 +92,6 @@ public class SynchronousWebHooksImplTest { assertThat(underTest.isEnabled(componentDto)).isTrue(); } - @Test public void do_nothing_if_no_webhooks() { ComponentDto componentDto = componentDbTester.insertPrivateProject().setOrganizationUuid(defaultOrganizationProvider.get().getUuid()); @@ -100,39 +104,85 @@ public class SynchronousWebHooksImplTest { } @Test - public void send_global_webhooks() { + public void populates_log_statistics_even_if_no_webhooks() { + ComponentDto componentDto = componentDbTester.insertPrivateProject().setOrganizationUuid(defaultOrganizationProvider.get().getUuid()); - ComponentDto componentDto = componentDbTester.insertPrivateProject(); - webhookDbTester.insert(newWebhook(componentDto).setName("First").setUrl("http://url1")); - webhookDbTester.insert(newWebhook(componentDto).setName("Second").setUrl("http://url2")); + underTest.sendProjectAnalysisUpdate(new WebHooks.Analysis(componentDto.uuid(), "1", "#1"), () -> mock, taskStatistics); + + assertThat(caller.countSent()).isEqualTo(0); + assertThat(logTester.logs(DEBUG)).isEmpty(); + verifyZeroInteractions(deliveryStorage); + verifyLogStatistics(0, 0); + } + + @Test + public void send_global_webhooks() { + OrganizationDto organizationDto = db.organizations().insert(); + ComponentDto componentDto = componentDbTester.insertPrivateProject(organizationDto); + webhookDbTester.insert(newWebhook(organizationDto).setName("First").setUrl("http://url1")); + webhookDbTester.insert(newWebhook(organizationDto).setName("Second").setUrl("http://url2")); caller.enqueueSuccess(NOW, 200, 1_234); caller.enqueueFailure(NOW, new IOException("Fail to connect")); - underTest.sendProjectAnalysisUpdate(new WebHooks.Analysis(componentDto.uuid(), "1", "#1"), () -> mock); + underTest.sendProjectAnalysisUpdate(new WebHooks.Analysis(componentDto.uuid(), "1", "#1"), () -> mock, taskStatistics); assertThat(caller.countSent()).isEqualTo(2); assertThat(logTester.logs(DEBUG)).contains("Sent webhook 'First' | url=http://url1 | time=1234ms | status=200"); assertThat(logTester.logs(DEBUG)).contains("Failed to send webhook 'Second' | url=http://url2 | message=Fail to connect"); verify(deliveryStorage, times(2)).persist(any(WebhookDelivery.class)); verify(deliveryStorage).purge(componentDto.uuid()); - + verifyLogStatistics(2, 0); } @Test public void send_project_webhooks() { - String organizationUuid = defaultOrganizationProvider.get().getUuid(); ComponentDto componentDto = componentDbTester.insertPrivateProject().setOrganizationUuid(organizationUuid); webhookDbTester.insert(newWebhook(componentDto).setName("First").setUrl("http://url1")); caller.enqueueSuccess(NOW, 200, 1_234); - underTest.sendProjectAnalysisUpdate(new WebHooks.Analysis(componentDto.uuid(), "1", "#1"), () -> mock); + underTest.sendProjectAnalysisUpdate(new WebHooks.Analysis(componentDto.uuid(), "1", "#1"), () -> mock, taskStatistics); assertThat(caller.countSent()).isEqualTo(1); assertThat(logTester.logs(DEBUG)).contains("Sent webhook 'First' | url=http://url1 | time=1234ms | status=200"); verify(deliveryStorage).persist(any(WebhookDelivery.class)); verify(deliveryStorage).purge(componentDto.uuid()); + verifyLogStatistics(0, 1); + } + + @Test + public void send_global_and_project_webhooks() { + OrganizationDto organizationDto = db.organizations().insert(); + ComponentDto componentDto = componentDbTester.insertPrivateProject(organizationDto); + webhookDbTester.insert(newWebhook(componentDto).setName("1First").setUrl("http://url1")); + webhookDbTester.insert(newWebhook(componentDto).setName("2Second").setUrl("http://url2")); + webhookDbTester.insert(newWebhook(organizationDto).setName("3Third").setUrl("http://url3")); + webhookDbTester.insert(newWebhook(organizationDto).setName("4Fourth").setUrl("http://url4")); + webhookDbTester.insert(newWebhook(organizationDto).setName("5Fifth").setUrl("http://url5")); + caller.enqueueSuccess(NOW, 200, 1_234); + caller.enqueueFailure(NOW, new IOException("Fail to connect 1")); + caller.enqueueFailure(NOW, new IOException("Fail to connect 2")); + caller.enqueueSuccess(NOW, 200, 5_678); + caller.enqueueSuccess(NOW, 200, 9_256); + + underTest.sendProjectAnalysisUpdate(new WebHooks.Analysis(componentDto.uuid(), "1", "#1"), () -> mock, taskStatistics); + + assertThat(caller.countSent()).isEqualTo(5); + List debugLogs = logTester.logs(DEBUG); + assertThat(debugLogs).contains("Sent webhook '1First' | url=http://url1 | time=1234ms | status=200"); + assertThat(debugLogs).contains("Failed to send webhook '2Second' | url=http://url2 | message=Fail to connect 1"); + assertThat(debugLogs).contains("Failed to send webhook '3Third' | url=http://url3 | message=Fail to connect 2"); + assertThat(debugLogs).contains("Sent webhook '4Fourth' | url=http://url4 | time=5678ms | status=200"); + assertThat(debugLogs).contains("Sent webhook '5Fifth' | url=http://url5 | time=9256ms | status=200"); + verify(deliveryStorage, times(5)).persist(any(WebhookDelivery.class)); + verify(deliveryStorage).purge(componentDto.uuid()); + verifyLogStatistics(3, 2); + } + private void verifyLogStatistics(int global, int project) { + verify(taskStatistics).add("globalWebhooks", global); + verify(taskStatistics).add("projectWebhooks", project); + verifyNoMoreInteractions(taskStatistics); } } 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 4f43a4fc05f..4fb435e8024 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 @@ -35,7 +35,6 @@ import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; -import org.mockito.ArgumentMatchers; import org.mockito.Mockito; import org.sonar.api.config.Configuration; import org.sonar.api.measures.Metric; @@ -239,7 +238,7 @@ public class WebhookQGChangeEventListenerTest { supplier.get(); return null; }).when(webHooks) - .sendProjectAnalysisUpdate(ArgumentMatchers.any(), ArgumentMatchers.any()); + .sendProjectAnalysisUpdate(any(), any()); } private void insertPropertiesFor(String snapshotUuid, Map properties) { -- 2.39.5