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

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 196290c4a7e6d3e5aea7900a3d365d86796d29c8..66de48bd195c16a5f6d759690d5fef795010821a 100644 (file)
--- 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,7 +19,6 @@
  */
 package org.sonar.db.webhook;
 
-import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import org.sonar.api.utils.System2;
@@ -29,16 +28,13 @@ 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, WebhookDeliveryDao webhookDeliveryDao) {
+  public WebhookDao(System2 system2) {
     this.system2 = system2;
-    this.webhookDeliveryDao = webhookDeliveryDao;
   }
 
   public Optional<WebhookDto> selectByUuid(DbSession dbSession, String uuid) {
@@ -70,25 +66,17 @@ 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);
   }
 
   public void deleteByOrganization(DbSession dbSession, OrganizationDto organization) {
-    cascadeDeletionToDeliveries(dbSession, selectByOrganizationUuid(dbSession, organization.getUuid()));
     mapper(dbSession).deleteForOrganizationUuid(organization.getUuid());
   }
 
   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/test/java/org/sonar/db/webhook/WebhookDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookDaoTest.java
index 1daf026e4805b684f5156e8d9755d1109ed08795..cc67bc107cd3a968d4c98e8dfefb4f6538ebd00e 100644 (file)
--- 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,7 +34,6 @@ 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 {
 
@@ -49,7 +48,6 @@ 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();
@@ -166,59 +164,6 @@ 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() {

diff --git a/server/sonar-server/src/main/java/org/sonar/server/component/ComponentCleanerService.java b/server/sonar-server/src/main/java/org/sonar/server/component/ComponentCleanerService.java
index 9be6a5576ba8f620f428ff85873d44740513b2c2..a6662676dec5c6e6064d3d32d3fc20d1c512a041 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/component/ComponentCleanerService.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/component/ComponentCleanerService.java
@@ -62,6 +62,8 @@ 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().selectByProject(dbSession, project)
+      .forEach(wh -> dbClient.webhookDeliveryDao().deleteByWebhook(dbSession, wh));
     dbClient.webhookDao().deleteByProject(dbSession, project);
     projectIndexers.commitAndIndex(dbSession, singletonList(project), ProjectIndexer.Cause.PROJECT_DELETION);
   }

diff --git a/server/sonar-server/src/main/java/org/sonar/server/organization/ws/DeleteAction.java b/server/sonar-server/src/main/java/org/sonar/server/organization/ws/DeleteAction.java
index d97583e7261c13650cb0f1b8095e4c812899bfb3..b599d89fada776f5ca1cf002d932f09f83fd42db 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/organization/ws/DeleteAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/organization/ws/DeleteAction.java
@@ -115,7 +115,9 @@ 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().deleteByProject(dbSession, project));
+    roots.forEach(project -> dbClient.webhookDao().selectByProject(dbSession, project)
+      .forEach(wh -> dbClient.webhookDeliveryDao().deleteByWebhook(dbSession, wh)));
+    roots.forEach(project -> dbClient.webhookDao().deleteByProject(dbSession, project));
     componentCleanerService.delete(dbSession, roots);
   }
 
@@ -152,6 +154,8 @@ public class DeleteAction implements OrganizationsWsAction {
     dbClient.organizationMemberDao().deleteByOrganizationUuid(dbSession, organization.getUuid());
     dbClient.organizationDao().deleteByUuid(dbSession, organization.getUuid());
     dbClient.userDao().cleanHomepage(dbSession, organization);
+    dbClient.webhookDao().selectByOrganizationUuid(dbSession, organization.getUuid())
+      .forEach(wh -> dbClient.webhookDeliveryDao().deleteByWebhook(dbSession, wh));
     dbClient.webhookDao().deleteByOrganization(dbSession, organization);
     userIndexer.commitAndIndexByLogins(dbSession, logins);
   }

diff --git a/server/sonar-server/src/main/java/org/sonar/server/webhook/ws/DeleteAction.java b/server/sonar-server/src/main/java/org/sonar/server/webhook/ws/DeleteAction.java
index 90dcc5e5de4e1b535e679e2dfd6865e9790e2b3f..988f43eb65779f2ee19bf07cf52320c33698ed3b 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/webhook/ws/DeleteAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/webhook/ws/DeleteAction.java
@@ -104,6 +104,7 @@ public class DeleteAction implements WebhooksWsAction {
   }
 
   private void deleteWebhook(DbSession dbSession, WebhookDto webhookDto) {
+    dbClient.webhookDeliveryDao().deleteByWebhook(dbSession, webhookDto);
     dbClient.webhookDao().delete(dbSession, webhookDto.getUuid());
   }
 

diff --git a/server/sonar-server/src/test/java/org/sonar/server/organization/ws/DeleteActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/organization/ws/DeleteActionTest.java
index bf99f3a9f5ea7912d7cb3b08f9875dc678a22cc8..e363905fc1b13449a595a38782a2107e87aa21e1 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/organization/ws/DeleteActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/organization/ws/DeleteActionTest.java
@@ -41,6 +41,8 @@ import org.sonar.db.qualityprofile.QProfileDto;
 import org.sonar.db.user.GroupDto;
 import org.sonar.db.user.UserDto;
 import org.sonar.db.webhook.WebhookDbTester;
+import org.sonar.db.webhook.WebhookDeliveryDao;
+import org.sonar.db.webhook.WebhookDeliveryDbTester;
 import org.sonar.db.webhook.WebhookDto;
 import org.sonar.server.component.ComponentCleanerService;
 import org.sonar.server.es.EsTester;
@@ -70,6 +72,7 @@ import static org.sonar.api.resources.Qualifiers.PROJECT;
 import static org.sonar.api.resources.Qualifiers.VIEW;
 import static org.sonar.db.permission.OrganizationPermission.ADMINISTER;
 import static org.sonar.db.user.UserTesting.newUserDto;
+import static org.sonar.db.webhook.WebhookDbTesting.newDto;
 import static org.sonar.server.organization.ws.OrganizationsWsSupport.PARAM_ORGANIZATION;
 
 public class DeleteActionTest {
@@ -84,7 +87,7 @@ public class DeleteActionTest {
   public ExpectedException expectedException = ExpectedException.none();
 
   private DbClient dbClient = db.getDbClient();
-  private DbSession session = db.getSession();
+  private DbSession dbSession = db.getSession();
   private ResourceTypesRule resourceTypes = new ResourceTypesRule().setRootQualifiers(PROJECT, VIEW, APP).setAllQualifiers(PROJECT, VIEW, APP);
   private ComponentCleanerService componentCleanerService = new ComponentCleanerService(db.getDbClient(), resourceTypes, mock(ProjectIndexers.class));
   private TestOrganizationFlags organizationFlags = TestOrganizationFlags.standalone().setEnabled(true);
@@ -93,6 +96,8 @@ public class DeleteActionTest {
   private UserIndex userIndex = new UserIndex(es.client(), System2.INSTANCE);
   private UserIndexer userIndexer = new UserIndexer(dbClient, es.client());
   private final WebhookDbTester webhookDbTester = db.webhooks();
+  private final WebhookDeliveryDao deliveryDao = dbClient.webhookDeliveryDao();
+  private final WebhookDeliveryDbTester webhookDeliveryDbTester = db.webhookDelivery();
   private WsActionTester wsTester = new WsActionTester(
     new DeleteAction(userSession, dbClient, defaultOrganizationProvider, componentCleanerService, organizationFlags, userIndexer, qProfileFactory));
 
@@ -120,7 +125,9 @@ public class DeleteActionTest {
     OrganizationDto organization = db.organizations().insert();
     webhookDbTester.insertWebhook(organization);
     webhookDbTester.insertWebhook(organization);
-    webhookDbTester.insertWebhook(organization);
+    WebhookDto dto = webhookDbTester.insertWebhook(organization);
+    webhookDeliveryDbTester.insert(newDto().setWebhookUuid(dto.getUuid()));
+    webhookDeliveryDbTester.insert(newDto().setWebhookUuid(dto.getUuid()));
 
     userSession.logIn().addPermission(ADMINISTER, organization);
 
@@ -128,15 +135,19 @@ public class DeleteActionTest {
       .setParam(PARAM_ORGANIZATION, organization.getKey())
       .execute();
 
-    List<WebhookDto> webhookDtos = dbClient.webhookDao().selectByOrganization(session, organization);
+    List<WebhookDto> webhookDtos = dbClient.webhookDao().selectByOrganization(dbSession, organization);
     assertThat(webhookDtos).isEmpty();
+
+    int deliveriesCount = deliveryDao.countDeliveriesByWebhookUuid(dbSession, dto.getUuid());
+    assertThat(deliveriesCount).isEqualTo(0);
+
   }
 
   @Test
   public void organization_deletion_also_ensure_that_homepage_on_this_organization_if_it_exists_is_cleared() {
     OrganizationDto organization = db.organizations().insert();
-    UserDto user = dbClient.userDao().insert(session, newUserDto().setHomepageType("ORGANIZATION").setHomepageParameter(organization.getUuid()));
-    session.commit();
+    UserDto user = dbClient.userDao().insert(dbSession, newUserDto().setHomepageType("ORGANIZATION").setHomepageParameter(organization.getUuid()));
+    dbSession.commit();
 
     userSession.logIn().addPermission(ADMINISTER, organization);
 
@@ -144,7 +155,7 @@ public class DeleteActionTest {
       .setParam(PARAM_ORGANIZATION, organization.getKey())
       .execute();
 
-    UserDto userReloaded = dbClient.userDao().selectUserById(session, user.getId());
+    UserDto userReloaded = dbClient.userDao().selectUserById(dbSession, user.getId());
     assertThat(userReloaded.getHomepageType()).isNull();
     assertThat(userReloaded.getHomepageParameter()).isNull();
   }
@@ -153,9 +164,9 @@ public class DeleteActionTest {
   public void organization_deletion_also_ensure_that_homepage_on_project_belonging_to_this_organization_if_it_exists_is_cleared() {
     OrganizationDto organization = db.organizations().insert();
     ComponentDto project = db.components().insertPrivateProject(organization);
-    UserDto user = dbClient.userDao().insert(session,
+    UserDto user = dbClient.userDao().insert(dbSession,
       newUserDto().setHomepageType("PROJECT").setHomepageParameter(project.uuid()));
-    session.commit();
+    dbSession.commit();
 
     userSession.logIn().addPermission(ADMINISTER, organization);
 
@@ -163,7 +174,7 @@ public class DeleteActionTest {
       .setParam(PARAM_ORGANIZATION, organization.getKey())
       .execute();
 
-    UserDto userReloaded = dbClient.userDao().selectUserById(session, user.getId());
+    UserDto userReloaded = dbClient.userDao().selectUserById(dbSession, user.getId());
     assertThat(userReloaded.getHomepageType()).isNull();
     assertThat(userReloaded.getHomepageParameter()).isNull();
   }
@@ -351,12 +362,12 @@ public class DeleteActionTest {
     sendRequest(org);
 
     verifyOrganizationDoesNotExist(org);
-    assertThat(dbClient.groupDao().selectByIds(session, of(group1.getId(), otherGroup1.getId(), group2.getId(), otherGroup2.getId())))
+    assertThat(dbClient.groupDao().selectByIds(dbSession, of(group1.getId(), otherGroup1.getId(), group2.getId(), otherGroup2.getId())))
       .extracting(GroupDto::getId)
       .containsOnly(otherGroup1.getId(), otherGroup2.getId());
-    assertThat(dbClient.permissionTemplateDao().selectByUuid(session, templateDto.getUuid()))
+    assertThat(dbClient.permissionTemplateDao().selectByUuid(dbSession, templateDto.getUuid()))
       .isNull();
-    assertThat(dbClient.permissionTemplateDao().selectByUuid(session, otherTemplateDto.getUuid()))
+    assertThat(dbClient.permissionTemplateDao().selectByUuid(dbSession, otherTemplateDto.getUuid()))
       .isNotNull();
     assertThat(db.select("select role as \"role\" from USER_ROLES"))
       .extracting(row -> (String) row.get("role"))
@@ -433,7 +444,7 @@ public class DeleteActionTest {
   }
 
   private void verifyOrganizationDoesNotExist(OrganizationDto organization) {
-    assertThat(db.getDbClient().organizationDao().selectByKey(session, organization.getKey())).isEmpty();
+    assertThat(db.getDbClient().organizationDao().selectByKey(dbSession, organization.getKey())).isEmpty();
   }
 
   private void sendRequest(OrganizationDto organization) {

diff --git a/server/sonar-server/src/test/java/org/sonar/server/webhook/ws/DeleteActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/webhook/ws/DeleteActionTest.java
index f182f86511b7420349b2b096be90cf46239f49f5..429f2bcb536aaba644b8b0fa097c5d6d0e7e9eed 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/webhook/ws/DeleteActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/webhook/ws/DeleteActionTest.java
@@ -25,12 +25,15 @@ import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.db.DbClient;
+import org.sonar.db.DbSession;
 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.db.webhook.WebhookDeliveryDao;
+import org.sonar.db.webhook.WebhookDeliveryDbTester;
 import org.sonar.db.webhook.WebhookDto;
 import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.exceptions.NotFoundException;
@@ -47,6 +50,7 @@ import static org.junit.rules.ExpectedException.none;
 import static org.sonar.api.web.UserRole.ADMIN;
 import static org.sonar.db.DbTester.create;
 import static org.sonar.db.permission.OrganizationPermission.ADMINISTER;
+import static org.sonar.db.webhook.WebhookDbTesting.newDto;
 import static org.sonar.server.organization.TestDefaultOrganizationProvider.from;
 import static org.sonar.server.tester.UserSessionRule.standalone;
 import static org.sonar.server.webhook.ws.WebhooksWsParameters.KEY_PARAM;
@@ -62,7 +66,10 @@ public class DeleteActionTest {
   @Rule
   public DbTester db = create();
   private DbClient dbClient = db.getDbClient();
+  private final DbSession dbSession = db.getSession();
   private WebhookDbTester webhookDbTester = db.webhooks();
+  private WebhookDeliveryDbTester webhookDeliveryDbTester = db.webhookDelivery();
+  private final WebhookDeliveryDao deliveryDao = dbClient.webhookDeliveryDao();
   private OrganizationDbTester organizationDbTester = db.organizations();
   private ComponentDbTester componentDbTester = db.components();
 
@@ -91,6 +98,9 @@ public class DeleteActionTest {
 
     ComponentDto project = componentDbTester.insertPrivateProject();
     WebhookDto dto = webhookDbTester.insertWebhook(project);
+    webhookDeliveryDbTester.insert(newDto().setWebhookUuid(dto.getUuid()));
+    webhookDeliveryDbTester.insert(newDto().setWebhookUuid(dto.getUuid()));
+
     userSession.logIn().addProjectPermission(ADMIN, project);
 
     TestResponse response = wsActionTester.newRequest()
@@ -101,6 +111,9 @@ public class DeleteActionTest {
     Optional<WebhookDto> reloaded = webhookDbTester.selectWebhook(dto.getUuid());
     assertThat(reloaded).isEmpty();
 
+    int deliveriesCount = deliveryDao.countDeliveriesByWebhookUuid(dbSession, dto.getUuid());
+    assertThat(deliveriesCount).isEqualTo(0);
+
   }
 
   @Test
@@ -108,6 +121,9 @@ public class DeleteActionTest {
 
     OrganizationDto organization = organizationDbTester.insert();
     WebhookDto dto = webhookDbTester.insertWebhook(organization);
+    webhookDeliveryDbTester.insert(newDto().setWebhookUuid(dto.getUuid()));
+    webhookDeliveryDbTester.insert(newDto().setWebhookUuid(dto.getUuid()));
+
     userSession.logIn().addPermission(ADMINISTER, organization.getUuid());
 
     TestResponse response = wsActionTester.newRequest()
@@ -118,6 +134,8 @@ public class DeleteActionTest {
     Optional<WebhookDto> reloaded = webhookDbTester.selectWebhook(dto.getUuid());
     assertThat(reloaded).isEmpty();
 
+    int deliveriesCount = deliveryDao.countDeliveriesByWebhookUuid(dbSession, dto.getUuid());
+    assertThat(deliveriesCount).isEqualTo(0);
   }
 
   @Test