From a06b72df4145098826c085f23999bbf911d20260 Mon Sep 17 00:00:00 2001 From: Guillaume Jambet Date: Tue, 27 Feb 2018 12:05:54 +0100 Subject: SONAR-10345 Ensure that no orphaned deliveries remains when deleting webhooks. --- .../main/java/org/sonar/db/webhook/WebhookDao.java | 26 ++++++--- .../org/sonar/db/webhook/WebhookDeliveryDao.java | 4 ++ .../sonar/db/webhook/WebhookDeliveryMapper.java | 2 + .../org/sonar/db/webhook/WebhookDeliveryMapper.xml | 5 ++ .../test/java/org/sonar/db/purge/PurgeDaoTest.java | 4 +- .../java/org/sonar/db/webhook/WebhookDaoTest.java | 62 ++++++++++++++++++++-- .../org/sonar/db/webhook/WebhookDbTesting.java | 1 + .../sonar/db/webhook/WebhookDeliveryDaoTest.java | 18 ++++++- 8 files changed, 108 insertions(+), 14 deletions(-) (limited to 'server/sonar-db-dao') diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/webhook/WebhookDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/webhook/WebhookDao.java index 9860570b3e2..196290c4a7e 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/webhook/WebhookDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/webhook/WebhookDao.java @@ -19,6 +19,7 @@ */ package org.sonar.db.webhook; +import java.util.Collections; import java.util.List; import java.util.Optional; import org.sonar.api.utils.System2; @@ -28,13 +29,16 @@ import org.sonar.db.component.ComponentDto; import org.sonar.db.organization.OrganizationDto; import static com.google.common.base.Preconditions.checkState; +import static java.util.Collections.emptyList; public class WebhookDao implements Dao { private final System2 system2; + private final WebhookDeliveryDao webhookDeliveryDao; - public WebhookDao(System2 system2) { + public WebhookDao(System2 system2, WebhookDeliveryDao webhookDeliveryDao) { this.system2 = system2; + this.webhookDeliveryDao = webhookDeliveryDao; } public Optional selectByUuid(DbSession dbSession, String uuid) { @@ -66,19 +70,27 @@ public class WebhookDao implements Dao { } public void delete(DbSession dbSession, String uuid) { + Optional webhookDto = selectByUuid(dbSession, uuid); + cascadeDeletionToDeliveries(dbSession, webhookDto.map(Collections::singletonList).orElse(emptyList())); mapper(dbSession).delete(uuid); } - private static WebhookMapper mapper(DbSession dbSession) { - return dbSession.getMapper(WebhookMapper.class); - } - - public void cleanWebhooks(DbSession dbSession, OrganizationDto organization) { + public void deleteByOrganization(DbSession dbSession, OrganizationDto organization) { + cascadeDeletionToDeliveries(dbSession, selectByOrganizationUuid(dbSession, organization.getUuid())); mapper(dbSession).deleteForOrganizationUuid(organization.getUuid()); } - public void cleanWebhooks(DbSession dbSession, ComponentDto componentDto) { + public void deleteByProject(DbSession dbSession, ComponentDto componentDto) { + cascadeDeletionToDeliveries(dbSession, selectByProject(dbSession, componentDto)); mapper(dbSession).deleteForProjectUuid(componentDto.uuid()); } + private void cascadeDeletionToDeliveries(DbSession dbSession, List webhooks) { + webhooks.forEach(wh -> webhookDeliveryDao.deleteByWebhook(dbSession, wh)); + } + + private static WebhookMapper mapper(DbSession dbSession) { + return dbSession.getMapper(WebhookMapper.class); + } + } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/webhook/WebhookDeliveryDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/webhook/WebhookDeliveryDao.java index 4d42c7c8b89..bf368642971 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/webhook/WebhookDeliveryDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/webhook/WebhookDeliveryDao.java @@ -85,4 +85,8 @@ public class WebhookDeliveryDao implements Dao { private static WebhookDeliveryMapper mapper(DbSession dbSession) { return dbSession.getMapper(WebhookDeliveryMapper.class); } + + public void deleteByWebhook(DbSession dbSession, WebhookDto webhook) { + mapper(dbSession).deleteByWebhookUuid(webhook.getUuid()); + } } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/webhook/WebhookDeliveryMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/webhook/WebhookDeliveryMapper.java index 6436922de6c..bb1783b97bf 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/webhook/WebhookDeliveryMapper.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/webhook/WebhookDeliveryMapper.java @@ -44,4 +44,6 @@ public interface WebhookDeliveryMapper { void insert(WebhookDeliveryDto dto); void deleteComponentBeforeDate(@Param("componentUuid") String componentUuid, @Param("beforeDate") long beforeDate); + + void deleteByWebhookUuid(@Param("webhookUuid") String webhookUuid); } diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/webhook/WebhookDeliveryMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/webhook/WebhookDeliveryMapper.xml index 1321fe21148..94af29d28f3 100644 --- a/server/sonar-db-dao/src/main/resources/org/sonar/db/webhook/WebhookDeliveryMapper.xml +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/webhook/WebhookDeliveryMapper.xml @@ -100,6 +100,11 @@ ) + + delete from webhook_deliveries where diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/purge/PurgeDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/purge/PurgeDaoTest.java index 62bffd56a95..ab5d78febfd 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/purge/PurgeDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/purge/PurgeDaoTest.java @@ -524,8 +524,8 @@ public class PurgeDaoTest { @Test public void deleteProject_deletes_webhook_deliveries() { ComponentDto project = dbTester.components().insertPublicProject(); - dbClient.webhookDeliveryDao().insert(dbSession, newDto().setComponentUuid(project.uuid()).setUuid("D1")); - dbClient.webhookDeliveryDao().insert(dbSession, newDto().setComponentUuid("P2").setUuid("D2")); + dbClient.webhookDeliveryDao().insert(dbSession, newDto().setComponentUuid(project.uuid()).setUuid("D1").setDurationMs(1000).setWebhookUuid("webhook-uuid")); + dbClient.webhookDeliveryDao().insert(dbSession, newDto().setComponentUuid("P2").setUuid("D2").setDurationMs(1000).setWebhookUuid("webhook-uuid")); underTest.deleteProject(dbSession, project.uuid()); diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookDaoTest.java index 3f722570012..1daf026e480 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookDaoTest.java @@ -34,6 +34,7 @@ import org.sonar.db.organization.OrganizationDbTester; import org.sonar.db.organization.OrganizationDto; import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.db.webhook.WebhookDbTesting.newDto; public class WebhookDaoTest { @@ -48,11 +49,12 @@ public class WebhookDaoTest { private final DbClient dbClient = dbTester.getDbClient(); private final DbSession dbSession = dbTester.getSession(); private final WebhookDao underTest = dbClient.webhookDao(); + private final WebhookDeliveryDao deliveryDao = dbClient.webhookDeliveryDao(); private final WebhookDbTester webhookDbTester = dbTester.webhooks(); + private final WebhookDeliveryDbTester webhookDeliveryDbTester = dbTester.webhookDelivery(); private final ComponentDbTester componentDbTester = dbTester.components(); private final OrganizationDbTester organizationDbTester = dbTester.organizations(); - @Test public void selectByUuid_returns_empty_if_uuid_does_not_exist() { assertThat(underTest.selectByUuid(dbSession, "missing")).isEmpty(); @@ -130,7 +132,7 @@ public class WebhookDaoTest { webhookDbTester.insertWebhook(componentDto); webhookDbTester.insertWebhook(componentDto); - underTest.cleanWebhooks(dbSession, componentDto); + underTest.deleteByProject(dbSession, componentDto); Optional reloaded = underTest.selectByUuid(dbSession, componentDto.uuid()); assertThat(reloaded).isEmpty(); @@ -145,7 +147,7 @@ public class WebhookDaoTest { webhookDbTester.insertWebhook(organization); webhookDbTester.insertWebhook(organization); - underTest.cleanWebhooks(dbSession, organization); + underTest.deleteByOrganization(dbSession, organization); Optional reloaded = underTest.selectByUuid(dbSession, organization.getUuid()); assertThat(reloaded).isEmpty(); @@ -164,6 +166,60 @@ public class WebhookDaoTest { assertThat(reloaded).isEmpty(); } + @Test + public void ensure_deliveries_are_deleted_when_a_webhook_is_deleted_by_uuid() { + + OrganizationDto organization = organizationDbTester.insert(); + + WebhookDto dto = webhookDbTester.insertWebhook(organization); + webhookDeliveryDbTester.insert(newDto().setWebhookUuid(dto.getUuid())); + webhookDeliveryDbTester.insert(newDto().setWebhookUuid(dto.getUuid())); + + underTest.delete(dbSession, dto.getUuid()); + + Optional reloaded = underTest.selectByUuid(dbSession, dto.getUuid()); + assertThat(reloaded).isEmpty(); + + int deliveriesCount = deliveryDao.countDeliveriesByWebhookUuid(dbSession, dto.getUuid()); + assertThat(deliveriesCount).isEqualTo(0); + } + + @Test + public void ensure_deliveries_are_deleted_when_a_webhooks_are_deleted_by_organization() { + + OrganizationDto organization = organizationDbTester.insert(); + + WebhookDto dto = webhookDbTester.insertWebhook(organization); + webhookDeliveryDbTester.insert(newDto().setWebhookUuid(dto.getUuid())); + webhookDeliveryDbTester.insert(newDto().setWebhookUuid(dto.getUuid())); + + underTest.deleteByOrganization(dbSession, organization); + + Optional reloaded = underTest.selectByUuid(dbSession, dto.getUuid()); + assertThat(reloaded).isEmpty(); + + int deliveriesCount = deliveryDao.countDeliveriesByWebhookUuid(dbSession, dto.getUuid()); + assertThat(deliveriesCount).isEqualTo(0); + } + + @Test + public void ensure_deliveries_are_deleted_when_a_webhooks_are_deleted_by_project() { + + OrganizationDto organization = organizationDbTester.insert(); + ComponentDto componentDto = componentDbTester.insertPrivateProject(organization); + WebhookDto dto = webhookDbTester.insertWebhook(componentDto); + webhookDeliveryDbTester.insert(newDto().setWebhookUuid(dto.getUuid())); + webhookDeliveryDbTester.insert(newDto().setWebhookUuid(dto.getUuid())); + + underTest.deleteByProject(dbSession, componentDto); + + Optional reloaded = underTest.selectByUuid(dbSession, dto.getUuid()); + assertThat(reloaded).isEmpty(); + + int deliveriesCount = deliveryDao.countDeliveriesByWebhookUuid(dbSession, dto.getUuid()); + assertThat(deliveriesCount).isEqualTo(0); + } + @Test public void fail_if_webhook_does_not_have_an_organization_nor_a_project() { diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookDbTesting.java b/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookDbTesting.java index 56173748467..c883a8729e4 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookDbTesting.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookDbTesting.java @@ -50,6 +50,7 @@ public class WebhookDbTesting { public static WebhookDeliveryDto newDto() { return new WebhookDeliveryDto() .setUuid(randomAlphanumeric(40)) + .setWebhookUuid(randomAlphanumeric(40)) .setComponentUuid(randomAlphanumeric(40)) .setCeTaskUuid(randomAlphanumeric(40)) .setAnalysisUuid(randomAlphanumeric(40)) diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookDeliveryDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookDeliveryDaoTest.java index 4dc3c7c9d89..c3c83a7cb3a 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookDeliveryDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookDeliveryDaoTest.java @@ -164,8 +164,8 @@ public class WebhookDeliveryDaoTest { @Test public void insert_row_with_only_mandatory_columns() { WebhookDeliveryDto dto = WebhookDbTesting.newDto("DELIVERY_1", "WEBHOOK_UUID_1", "COMPONENT_1", "TASK_1") + .setDurationMs(1000) .setHttpStatus(null) - .setDurationMs(null) .setErrorStacktrace(null); underTest.insert(dbSession, dto); @@ -173,9 +173,10 @@ public class WebhookDeliveryDaoTest { WebhookDeliveryDto stored = selectByUuid(dto.getUuid()); verifyMandatoryFields(dto, stored); + assertThat(stored.getDurationMs()).isEqualTo(1000); + // optional fields are null assertThat(stored.getHttpStatus()).isNull(); - assertThat(stored.getDurationMs()).isNull(); assertThat(stored.getErrorStacktrace()).isNull(); } @@ -193,6 +194,19 @@ public class WebhookDeliveryDaoTest { assertThat(stored.getErrorStacktrace()).isEqualTo(dto.getErrorStacktrace()); } + @Test + public void deleteByWebhook() { + + WebhookDto webhookDto = dbWebhooks.insert(WebhookTesting.newProjectWebhook("COMPONENT_1")); + + underTest.insert(dbSession, WebhookDbTesting.newDto("DELIVERY_1", webhookDto.getUuid(), "COMPONENT_1", "TASK_1").setCreatedAt(1_000_000L)); + underTest.insert(dbSession, WebhookDbTesting.newDto("DELIVERY_2", webhookDto.getUuid(), "COMPONENT_1", "TASK_2").setCreatedAt(2_000_000L)); + underTest.insert(dbSession, WebhookDbTesting.newDto("DELIVERY_3", "WONT BE DELETED WEBHOOK_UUID_2", "COMPONENT_2", "TASK_3").setCreatedAt(1_000_000L)); + + underTest.deleteByWebhook(dbSession, webhookDto); + + assertThat(dbTester.countRowsOfTable(dbSession, "webhook_deliveries")).isEqualTo(1); } + @Test public void deleteComponentBeforeDate_deletes_rows_before_date() { underTest.insert(dbSession, WebhookDbTesting.newDto("DELIVERY_1", "WEBHOOK_UUID_1", "COMPONENT_1", "TASK_1").setCreatedAt(1_000_000L)); -- cgit v1.2.3