SONAR-10345 Ensure that no orphaned deliveries remains when deleting webhooks.

8 files changed, 108 insertions, 14 deletions

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<WebhookDto> selectByUuid(DbSession dbSession, String uuid) {
@@ -66,19 +70,27 @@ public class WebhookDao implements Dao {
   }
 
   public void delete(DbSession dbSession, String uuid) {
+    Optional<WebhookDto> 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<WebhookDto> 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 @@
     )
   </insert>
 
+  <select id="deleteByWebhookUuid" parameterType="String">
+    delete from webhook_deliveries
+    where webhook_uuid = #{webhookUuid,jdbcType=VARCHAR}
+  </select>
+
   <delete id="deleteComponentBeforeDate" parameterType="map">
     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<WebhookDto> 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<WebhookDto> reloaded = underTest.selectByUuid(dbSession, organization.getUuid());
     assertThat(reloaded).isEmpty();
@@ -165,6 +167,60 @@ public class WebhookDaoTest {
   }
 
   @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<WebhookDto> 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<WebhookDto> 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<WebhookDto> 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() {
 
     expectedException.expect(IllegalStateException.class);
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();
   }
 
@@ -194,6 +195,19 @@ public class WebhookDeliveryDaoTest {
   }
 
   @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));
     underTest.insert(dbSession, WebhookDbTesting.newDto("DELIVERY_2", "WEBHOOK_UUID_1", "COMPONENT_1", "TASK_2").setCreatedAt(2_000_000L));