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

server/sonar-db-core/src/main/resources/org/sonar/db/version/schema-h2.ddl

@@ -769,7 +769,7 @@ CREATE INDEX "PROJECT_WEBHOOK" ON "WEBHOOKS" ("PROJECT_UUID");
 
 CREATE TABLE "WEBHOOK_DELIVERIES" (
   "UUID" VARCHAR(40) NOT NULL PRIMARY KEY,
-  "WEBHOOK_UUID" VARCHAR(40),
+  "WEBHOOK_UUID" VARCHAR(40) NOT NULL,
   "COMPONENT_UUID" VARCHAR(40) NOT NULL,
   "ANALYSIS_UUID" VARCHAR(40),
   "CE_TASK_UUID" VARCHAR(40),
@@ -777,7 +777,7 @@ CREATE TABLE "WEBHOOK_DELIVERIES" (
   "URL" VARCHAR(2000) NOT NULL,
   "SUCCESS" BOOLEAN NOT NULL,
   "HTTP_STATUS" INT,
-  "DURATION_MS" INT,
+  "DURATION_MS" INT NOT NULL,
   "PAYLOAD" CLOB NOT NULL,
   "ERROR_STACKTRACE" CLOB,
   "CREATED_AT" BIGINT NOT NULL

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);
+  }
+
 }

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());
+  }
 }

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);
 }

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

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());
 

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();
@@ -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<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() {
 

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))

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));

server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v71/AddWebhookKeyToWebhookDeliveriesTable.java

@@ -22,8 +22,10 @@ package org.sonar.server.platform.db.migration.version.v71;
 import java.sql.SQLException;
 import org.sonar.db.Database;
 import org.sonar.server.platform.db.migration.sql.AddColumnsBuilder;
+import org.sonar.server.platform.db.migration.sql.AlterColumnsBuilder;
 import org.sonar.server.platform.db.migration.step.DdlChange;
 
+import static org.sonar.server.platform.db.migration.def.BigIntegerColumnDef.newBigIntegerColumnDefBuilder;
 import static org.sonar.server.platform.db.migration.def.VarcharColumnDef.newVarcharColumnDefBuilder;
 
 public class AddWebhookKeyToWebhookDeliveriesTable extends DdlChange {
@@ -34,13 +36,23 @@ public class AddWebhookKeyToWebhookDeliveriesTable extends DdlChange {
 
   @Override
   public void execute(Context context) throws SQLException {
+
+    context.execute("delete from webhook_deliveries");
+
     context.execute(new AddColumnsBuilder(getDialect(), "webhook_deliveries")
       .addColumn(newVarcharColumnDefBuilder()
         .setColumnName("webhook_uuid")
-        .setIsNullable(true)
+        .setIsNullable(false)
         .setLimit(40)
         .build())
       .build());
+
+    context.execute(new AlterColumnsBuilder(getDialect(), "webhook_deliveries")
+      .updateColumn(newBigIntegerColumnDefBuilder()
+        .setColumnName("duration_ms")
+        .setIsNullable(false)
+        .build())
+      .build());
   }
 
 }

server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v71/AddWebhookKeyToWebhookDeliveriesTableTest.java

@@ -25,7 +25,13 @@ import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.sonar.db.CoreDbTester;
 
+import static java.lang.String.valueOf;
+import static java.sql.Types.BIGINT;
 import static java.sql.Types.VARCHAR;
+import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic;
+import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric;
+import static org.apache.commons.lang.RandomStringUtils.randomNumeric;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.sonar.db.CoreDbTester.createForSchema;
 
 public class AddWebhookKeyToWebhookDeliveriesTableTest {
@@ -38,10 +44,28 @@ public class AddWebhookKeyToWebhookDeliveriesTableTest {
   private AddWebhookKeyToWebhookDeliveriesTable underTest = new AddWebhookKeyToWebhookDeliveriesTable(dbTester.database());
 
   @Test
-  public void column_is_added_to_table() throws SQLException {
+  public void table_has_been_truncated() throws SQLException {
+
+    insertDelivery();
+
+    underTest.execute();
+
+    int count = dbTester.countRowsOfTable("webhook_deliveries");
+    assertThat(count).isEqualTo(0);
+  }
+
+  @Test
+  public void column_webhook_uuid_has_been_added_to_table() throws SQLException {
     underTest.execute();
 
-    dbTester.assertColumnDefinition("webhook_deliveries", "webhook_uuid", VARCHAR, 40, true);
+    dbTester.assertColumnDefinition("webhook_deliveries", "webhook_uuid", VARCHAR, 40, false);
+  }
+
+  @Test
+  public void column_duration_ms_is_now_not_nullable() throws SQLException {
+    underTest.execute();
+
+    dbTester.assertColumnDefinition("webhook_deliveries", "duration_ms", BIGINT, null, false);
   }
 
   @Test
@@ -52,4 +76,18 @@ public class AddWebhookKeyToWebhookDeliveriesTableTest {
 
     underTest.execute();
   }
+
+  private void insertDelivery() {
+    dbTester.executeInsert("webhook_deliveries",
+      "uuid", randomAlphanumeric(40),
+//      "webhook_uuid", randomAlphanumeric(40),
+      "component_uuid", randomAlphanumeric(40),
+      "name", randomAlphabetic(60),
+      "url", randomAlphabetic(200),
+      "success", true,
+      "duration_ms", randomNumeric(7),
+      "payload", randomAlphanumeric(1000),
+      "created_at", valueOf(1_55555_555));
+  }
+
 }

server/sonar-server/src/main/java/org/sonar/server/component/ComponentCleanerService.java

@@ -62,7 +62,7 @@ public class ComponentCleanerService {
     checkArgument(!hasNotProjectScope(project) && !isNotDeletable(project) && project.getMainBranchProjectUuid() == null, "Only projects can be deleted");
     dbClient.purgeDao().deleteProject(dbSession, project.uuid());
     dbClient.userDao().cleanHomepage(dbSession, project);
-    dbClient.webhookDao().cleanWebhooks(dbSession, project);
+    dbClient.webhookDao().deleteByProject(dbSession, project);
     projectIndexers.commitAndIndex(dbSession, singletonList(project), ProjectIndexer.Cause.PROJECT_DELETION);
   }
 

server/sonar-server/src/main/java/org/sonar/server/organization/ws/DeleteAction.java

@@ -115,7 +115,7 @@ public class DeleteAction implements OrganizationsWsAction {
 
   private void deleteProjects(DbSession dbSession, OrganizationDto organization) {
     List<ComponentDto> roots = dbClient.componentDao().selectAllRootsByOrganization(dbSession, organization.getUuid());
-    roots.forEach(project ->  dbClient.webhookDao().cleanWebhooks(dbSession, project));
+    roots.forEach(project ->  dbClient.webhookDao().deleteByProject(dbSession, project));
     componentCleanerService.delete(dbSession, roots);
   }
 
@@ -152,7 +152,7 @@ public class DeleteAction implements OrganizationsWsAction {
     dbClient.organizationMemberDao().deleteByOrganizationUuid(dbSession, organization.getUuid());
     dbClient.organizationDao().deleteByUuid(dbSession, organization.getUuid());
     dbClient.userDao().cleanHomepage(dbSession, organization);
-    dbClient.webhookDao().cleanWebhooks(dbSession, organization);
+    dbClient.webhookDao().deleteByOrganization(dbSession, organization);
     userIndexer.commitAndIndexByLogins(dbSession, logins);
   }
 

server/sonar-server/src/test/java/org/sonar/server/webhook/ws/WebhookDeliveryActionTest.java

@@ -129,7 +129,6 @@ public class WebhookDeliveryActionTest {
       .setComponentUuid(project.uuid())
       .setSuccess(false)
       .setHttpStatus(null)
-      .setDurationMs(null)
       .setErrorStacktrace("IOException -> can not connect");
     dbClient.webhookDeliveryDao().insert(db.getSession(), dto);
     db.commit();
@@ -141,7 +140,6 @@ public class WebhookDeliveryActionTest {
 
     Webhooks.Delivery actual = response.getDelivery();
     assertThat(actual.hasHttpStatus()).isFalse();
-    assertThat(actual.hasDurationMs()).isFalse();
     assertThat(actual.getErrorStacktrace()).isEqualTo(dto.getErrorStacktrace());
   }
 
@@ -151,7 +149,6 @@ public class WebhookDeliveryActionTest {
       .setComponentUuid(project.uuid())
       .setCeTaskUuid(null)
       .setHttpStatus(null)
-      .setDurationMs(null)
       .setErrorStacktrace(null)
       .setAnalysisUuid(null);
     dbClient.webhookDeliveryDao().insert(db.getSession(), dto);
@@ -164,7 +161,6 @@ public class WebhookDeliveryActionTest {
 
     Webhooks.Delivery actual = response.getDelivery();
     assertThat(actual.hasHttpStatus()).isFalse();
-    assertThat(actual.hasDurationMs()).isFalse();
     assertThat(actual.hasErrorStacktrace()).isFalse();
     assertThat(actual.hasCeTaskId()).isFalse();
   }