From 29f0ba1e123c839ccc65f439fd107409df18ee6b Mon Sep 17 00:00:00 2001 From: lukasz-jarocki-sonarsource Date: Tue, 19 Sep 2023 16:42:51 +0200 Subject: [PATCH] SONAR-20481 fixed the issue where removing a branch was removing a project too --- .../org/sonar/db/purge/PurgeCommandsIT.java | 10 +-- .../java/org/sonar/db/purge/PurgeDaoIT.java | 56 +++++++------ .../org/sonar/db/purge/PurgeCommands.java | 27 ++++-- .../java/org/sonar/db/purge/PurgeDao.java | 82 +++++++++++-------- .../java/org/sonar/db/purge/PurgeMapper.java | 4 +- .../org/sonar/db/purge/PurgeMapper.xml | 11 ++- 6 files changed, 115 insertions(+), 75 deletions(-) diff --git a/server/sonar-db-dao/src/it/java/org/sonar/db/purge/PurgeCommandsIT.java b/server/sonar-db-dao/src/it/java/org/sonar/db/purge/PurgeCommandsIT.java index 890a6f81ee78..6df2ca7cd22e 100644 --- a/server/sonar-db-dao/src/it/java/org/sonar/db/purge/PurgeCommandsIT.java +++ b/server/sonar-db-dao/src/it/java/org/sonar/db/purge/PurgeCommandsIT.java @@ -263,7 +263,7 @@ public void deleteAnalyses_by_rootUuid_all_analyses_of_specified_root_uuid(Compo @Test @UseDataProvider("projectsAndViews") - public void deleteAnalyses_by_rootUuid_deletes_event_component_changes(ComponentDto projectOrView) { + public void deleteEventComponentChanges_shouldDeleteEventComponentChanges(ComponentDto projectOrView) { dbTester.components().insertComponent(projectOrView); ComponentDto otherProject = dbTester.components().insertPrivateProject().getMainBranchComponent(); int count = 5; @@ -272,7 +272,7 @@ public void deleteAnalyses_by_rootUuid_deletes_event_component_changes(Component insertRandomEventComponentChange(otherProject); }); - underTest.deleteAnalyses(projectOrView.uuid()); + underTest.deleteEventComponentChanges(projectOrView.uuid()); assertThat(countEventComponentChangesOf(projectOrView)).isZero(); assertThat(countEventComponentChangesOf(otherProject)).isEqualTo(count); @@ -646,7 +646,7 @@ public void deleteNewCodePeriodsByRootUuid_deletes_branch_new_code_periods() { dbTester.newCodePeriods().insert(project.uuid(), branch.getUuid(), NewCodePeriodType.NUMBER_OF_DAYS, "1"); PurgeCommands purgeCommands = new PurgeCommands(dbTester.getSession(), profiler, system2); - purgeCommands.deleteNewCodePeriods(branch.getUuid()); + purgeCommands.deleteNewCodePeriodsForBranch(branch.getUuid()); // should delete branch settings only assertThat(dbTester.countRowsOfTable("new_code_periods")).isEqualTo(2); @@ -668,7 +668,7 @@ public void deleteNewCodePeriodsByRootUuid_deletes_project_new_code_periods() { dbTester.newCodePeriods().insert(project.uuid(), branch.getUuid(), NewCodePeriodType.NUMBER_OF_DAYS, "1"); PurgeCommands purgeCommands = new PurgeCommands(dbTester.getSession(), profiler, system2); - purgeCommands.deleteNewCodePeriods(project.uuid()); + purgeCommands.deleteNewCodePeriodsForProject(project.uuid()); // should delete branch and project settings only assertThat(dbTester.countRowsOfTable("new_code_periods")).isOne(); @@ -690,7 +690,7 @@ public void deleteNewCodePeriodsByRootUuid_should_not_delete_any_if_root_uuid_is dbTester.newCodePeriods().insert(project.uuid(), branch.getUuid(), NewCodePeriodType.NUMBER_OF_DAYS, "1"); PurgeCommands purgeCommands = new PurgeCommands(dbTester.getSession(), profiler, system2); - purgeCommands.deleteNewCodePeriods(null); + purgeCommands.deleteNewCodePeriodsForProject(null); // should delete branch and project settings only assertThat(dbTester.countRowsOfTable("new_code_periods")).isEqualTo(3); diff --git a/server/sonar-db-dao/src/it/java/org/sonar/db/purge/PurgeDaoIT.java b/server/sonar-db-dao/src/it/java/org/sonar/db/purge/PurgeDaoIT.java index 808bb724f125..42b6cae19053 100644 --- a/server/sonar-db-dao/src/it/java/org/sonar/db/purge/PurgeDaoIT.java +++ b/server/sonar-db-dao/src/it/java/org/sonar/db/purge/PurgeDaoIT.java @@ -266,11 +266,6 @@ public void purge_inactive_branches_when_analyzing_non_main_branch() { BranchDto branch3 = db.components().insertProjectBranch(project.getProjectDto(), b -> b.setBranchType(BranchType.BRANCH)); db.components().insertSnapshot(branch3, dto -> dto.setCreatedAt(DateUtils.addDays(new Date(), -31).getTime())); - // properties exist or active and for inactive branch - ComponentDto branch1Component = db.components().getComponentDto(branch1); - ComponentDto branch3Component = db.components().getComponentDto(branch3); - insertPropertyFor(branch3Component, branch1Component); - // analysing branch1 underTest.purge(dbSession, newConfigurationWith30Days(System2.INSTANCE, branch1.getUuid(), project.getProjectDto().getUuid()), PurgeListener.EMPTY, new PurgeProfiler()); dbSession.commit(); @@ -278,7 +273,6 @@ public void purge_inactive_branches_when_analyzing_non_main_branch() { // branch1 wasn't deleted since it was being analyzed! assertThat(uuidsIn("components")).containsOnly(project.getMainBranchDto().getUuid(), nonMainBranch.getUuid(), branch1.getUuid()); assertThat(uuidsIn("projects")).containsOnly(project.getProjectDto().getUuid()); - assertThat(componentUuidsIn("properties")).containsOnly(branch1.getUuid()); } @Test @@ -592,7 +586,7 @@ public void delete_project_and_associated_data() { ProjectData project = db.components().insertPrivateProject(); ComponentDto directory = db.components().insertComponent(newDirectory(project.getMainBranchComponent(), "a/b")); ComponentDto file = db.components().insertComponent(newFileDto(directory)); - SnapshotDto analysis = db.components().insertSnapshot(project.getProjectDto()); + SnapshotDto analysis = db.components().insertSnapshot(project.getMainBranchDto()); IssueDto issue1 = db.issues().insert(rule, project.getMainBranchComponent(), file); IssueChangeDto issueChange1 = db.issues().insertChange(issue1); IssueDto issue2 = db.issues().insert(rule, project.getMainBranchComponent(), file); @@ -602,7 +596,7 @@ public void delete_project_and_associated_data() { ProjectData otherProject = db.components().insertPrivateProject(); ComponentDto otherDirectory = db.components().insertComponent(newDirectory(otherProject.getMainBranchComponent(), "a/b")); ComponentDto otherFile = db.components().insertComponent(newFileDto(otherDirectory)); - SnapshotDto otherAnalysis = db.components().insertSnapshot(otherProject.getProjectDto()); + SnapshotDto otherAnalysis = db.components().insertSnapshot(otherProject.getMainBranchDto()); IssueDto otherIssue1 = db.issues().insert(rule, otherProject.getMainBranchComponent(), otherFile); IssueChangeDto otherIssueChange1 = db.issues().insertChange(otherIssue1); IssueDto otherIssue2 = db.issues().insert(rule, otherProject.getMainBranchComponent(), otherFile); @@ -693,8 +687,9 @@ public void delete_application_branch() { db.components().addProjectBranchToApplicationBranch(dbClient.branchDao().selectByUuid(dbSession, appBranch.getUuid()).get(), projectBranch); db.components().addProjectBranchToApplicationBranch(dbClient.branchDao().selectByUuid(dbSession, otherAppBranch.getUuid()).get(), projectBranch); - // properties exist or active and for inactive branch - insertPropertyFor(appBranchComponent, otherAppBranchComponent); + // properties exist only for entities, not branches + insertPropertyFor(app.getProjectDto()); + insertPropertyFor(otherApp.getProjectDto()); insertReportScheduleAndSubscriptionForBranch(appBranch.getUuid(), dbSession); @@ -710,7 +705,8 @@ public void delete_application_branch() { assertThat(uuidsIn("project_measures")).containsOnly(appMeasure.getUuid(), otherAppMeasure.getUuid(), otherAppBranchMeasure.getUuid()); assertThat(uuidsIn("app_projects", "application_uuid")).containsOnly(app.getProjectDto().getUuid(), otherApp.getProjectDto().getUuid()); assertThat(uuidsIn("app_branch_project_branch", "application_branch_uuid")).containsOnly(otherAppBranch.getUuid()); - assertThat(componentUuidsIn("properties")).containsOnly(otherAppBranch.getUuid()); + //properties should not change as we are deleting a branch, not application + assertThat(componentUuidsIn("properties")).containsOnly(otherApp.projectUuid(), app.projectUuid()); assertThat(uuidsIn("report_schedules", "branch_uuid")).isEmpty(); assertThat(uuidsIn("report_subscriptions", "branch_uuid")).isEmpty(); @@ -1165,18 +1161,14 @@ public void delete_branch_content_when_deleting_project() { int projectEntryCount = db.countRowsOfTable("components"); int issueCount = db.countRowsOfTable("issues"); int branchCount = db.countRowsOfTable("project_branches"); - Collection anotherLivingProjectBranches = db.getDbClient().branchDao() - .selectByProject(db.getSession(), anotherLivingProject); - insertPropertyFor(anotherLivingProjectBranches); + insertPropertyFor(anotherLivingProject); - assertThat(db.countRowsOfTable("properties")).isEqualTo(anotherLivingProjectBranches.size()); + assertThat(db.countRowsOfTable("properties")).isEqualTo(1); ProjectDto projectToDelete = insertProjectWithBranchAndRelatedData(); - Collection projectToDeleteBranches = db.getDbClient().branchDao() - .selectByProject(db.getSession(), projectToDelete); - insertPropertyFor(projectToDeleteBranches); + insertPropertyFor(projectToDelete); - assertThat(db.countRowsOfTable("properties")).isEqualTo(anotherLivingProjectBranches.size() + projectToDeleteBranches.size()); + assertThat(db.countRowsOfTable("properties")).isEqualTo(2); assertThat(db.countRowsOfTable("components")).isGreaterThan(projectEntryCount); assertThat(db.countRowsOfTable("issues")).isGreaterThan(issueCount); assertThat(db.countRowsOfTable("project_branches")).isGreaterThan(branchCount); @@ -1187,7 +1179,7 @@ public void delete_branch_content_when_deleting_project() { assertThat(db.countRowsOfTable("components")).isEqualTo(projectEntryCount); assertThat(db.countRowsOfTable("issues")).isEqualTo(issueCount); assertThat(db.countRowsOfTable("project_branches")).isEqualTo(branchCount); - assertThat(db.countRowsOfTable("properties")).isEqualTo(anotherLivingProjectBranches.size()); + assertThat(db.countRowsOfTable("properties")).isEqualTo(1); } @Test @@ -1856,6 +1848,22 @@ public void purge_old_anticipated_transitions() { assertThat(anticipatedTransitionDtos).hasSize(1); assertThat(anticipatedTransitionDtos.get(0).getUuid()).isEqualTo("okTransition"); } + + @Test + public void deleteBranch_shouldNotRemoveOldProjectWithSameUuidAsBranch() { + ComponentDto componentDto = ComponentTesting.newPrivateProjectDto().setUuid("uuid"); + ProjectData projectData = db.components().insertPrivateProject("uuid", componentDto); + BranchDto branch = db.components().insertProjectBranch(projectData.getProjectDto(), b -> b.setBranchType(BranchType.BRANCH)); + + insertPropertyFor(projectData.getProjectDto()); + + underTest.deleteBranch(dbSession, "uuid"); + + assertThat(componentUuidsIn("properties")).containsOnly("uuid"); + assertThat(uuidsIn("project_branches")).containsOnly(branch.getUuid()); + assertThat(uuidsIn("projects")).containsOnly("uuid"); + + } private AnticipatedTransitionDto getAnticipatedTransitionsDto(String uuid, String projectUuid, Date creationDate) { return new AnticipatedTransitionDto(uuid, projectUuid, "userUuid", "transition", null, null, null, null, "rule:key", "filepath", creationDate.getTime()); @@ -1910,12 +1918,12 @@ private void insertPropertyFor(ComponentDto... components) { componentDto.getKey(), componentDto.name(), componentDto.qualifier(), null)); } - private void insertPropertyFor(Collection branches) { - branches.stream().forEach(branchDto -> db.properties().insertProperty(new PropertyDto() + private void insertPropertyFor(ProjectDto project) { + db.properties().insertProperty(new PropertyDto() .setKey(randomAlphabetic(3)) .setValue(randomAlphabetic(3)) - .setEntityUuid(branchDto.getUuid()), - null, branchDto.getKey(), null, null)); + .setEntityUuid(project.getUuid()), + null, project.getKey(), null, null); } private Stream getComponentUuidsOfMeasures() { diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeCommands.java b/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeCommands.java index 0c086d15e086..05f2efb1bf71 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeCommands.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeCommands.java @@ -55,12 +55,14 @@ List selectSnapshotUuids(PurgeSnapshotQuery query) { return purgeMapper.selectAnalysisUuids(query); } - void deleteAnalyses(String rootComponentUuid) { + void deleteEventComponentChanges(String rootComponentUuid) { profiler.start("deleteAnalyses (event_component_changes)"); purgeMapper.deleteEventComponentChangesByComponentUuid(rootComponentUuid); session.commit(); profiler.stop(); + } + void deleteAnalyses(String rootComponentUuid) { profiler.start("deleteAnalyses (events)"); purgeMapper.deleteEventsByComponentUuid(rootComponentUuid); session.commit(); @@ -255,9 +257,9 @@ void deleteDisabledComponentsWithoutIssues(List disabledComponentsWithou profiler.stop(); } - void deleteOutdatedProperties(String branchUuid) { + void deleteOutdatedProperties(String entityUuid) { profiler.start("deleteOutdatedProperties (properties)"); - purgeMapper.deletePropertiesByEntityUuids(List.of(branchUuid)); + purgeMapper.deletePropertiesByEntityUuids(List.of(entityUuid)); session.commit(); profiler.stop(); } @@ -454,9 +456,16 @@ void deleteLiveMeasures(String rootUuid) { profiler.stop(); } - void deleteNewCodePeriods(String rootUuid) { + void deleteNewCodePeriodsForProject(String projectUuid) { + profiler.start("deleteNewCodePeriods (new_code_periods)"); + purgeMapper.deleteNewCodePeriodsByProjectUuid(projectUuid); + session.commit(); + profiler.stop(); + } + + void deleteNewCodePeriodsForBranch(String branchUuid) { profiler.start("deleteNewCodePeriods (new_code_periods)"); - purgeMapper.deleteNewCodePeriodsByRootUuid(rootUuid); + purgeMapper.deleteNewCodePeriodsByBranchUuid(branchUuid); session.commit(); profiler.stop(); } @@ -480,9 +489,9 @@ public void deleteProjectInPortfolios(String rootUuid) { profiler.stop(); } - public void deleteScannerCache(String rootUuid) { + public void deleteScannerCache(String branchUuid) { profiler.start("deleteScannerCache (scanner_analysis_cache)"); - purgeMapper.deleteScannerAnalysisCacheByBranchUuid(rootUuid); + purgeMapper.deleteScannerAnalysisCacheByBranchUuid(branchUuid); session.commit(); profiler.stop(); } @@ -494,9 +503,9 @@ public void deleteReportSchedules(String rootUuid) { profiler.stop(); } - public void deleteReportSubscriptions(String rootUuid) { + public void deleteReportSubscriptions(String branchUuid) { profiler.start("deleteReportSubscriptions (report_subscriptions)"); - purgeMapper.deleteReportSubscriptionsByBranchUuid(rootUuid); + purgeMapper.deleteReportSubscriptionsByBranchUuid(branchUuid); session.commit(); profiler.stop(); } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeDao.java index a1bcec6137e5..5fd2500d37f6 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeDao.java @@ -91,7 +91,7 @@ private static void purgeStaleBranches(PurgeCommands commands, PurgeConfiguratio for (String branchUuid : branchUuids) { if (!rootUuid.equals(branchUuid)) { - deleteRootComponent(branchUuid, mapper, commands); + deleteBranch(branchUuid, commands); } } } @@ -225,9 +225,8 @@ public boolean test(PurgeableAnalysisDto purgeableAnalysisDto) { public void deleteBranch(DbSession session, String uuid) { PurgeProfiler profiler = new PurgeProfiler(); - PurgeMapper purgeMapper = mapper(session); PurgeCommands purgeCommands = new PurgeCommands(session, profiler, system2); - deleteRootComponent(uuid, purgeMapper, purgeCommands); + deleteBranch(uuid, purgeCommands); } public void deleteProject(DbSession session, String uuid, String qualifier, String name, String key) { @@ -242,9 +241,9 @@ public void deleteProject(DbSession session, String uuid, String qualifier, Stri .map(BranchDto::getUuid) .toList(); - branchUuids.forEach(id -> deleteRootComponent(id, purgeMapper, purgeCommands)); + branchUuids.forEach(id -> deleteBranch(id, purgeCommands)); - deleteRootComponent(uuid, purgeMapper, purgeCommands); + deleteProject(uuid, purgeMapper, purgeCommands); auditPersister.deleteComponent(session, new ComponentNewValue(uuid, name, key, qualifier)); logProfiling(profiler, start); } @@ -265,35 +264,52 @@ private static void logProfiling(PurgeProfiler profiler, long start) { LOG.debug(""); } - private static void deleteRootComponent(String rootUuid, PurgeMapper mapper, PurgeCommands commands) { - List rootAndSubviews = mapper.selectRootAndSubviewsByProjectUuid(rootUuid); - commands.deleteLinks(rootUuid); - commands.deleteScannerCache(rootUuid); - commands.deleteAnalyses(rootUuid); + private static void deleteBranch(String branchUuid, PurgeCommands commands) { + commands.deleteScannerCache(branchUuid); + commands.deleteAnalyses(branchUuid); + commands.deleteIssues(branchUuid); + commands.deleteFileSources(branchUuid); + commands.deleteCeActivity(branchUuid); + commands.deleteCeQueue(branchUuid); + commands.deleteLiveMeasures(branchUuid); + commands.deleteNewCodePeriodsForBranch(branchUuid); + commands.deleteBranch(branchUuid); + commands.deleteApplicationBranchProjects(branchUuid); + commands.deleteComponents(branchUuid); + commands.deleteReportSchedules(branchUuid); + commands.deleteReportSubscriptions(branchUuid); + } + + private static void deleteProject(String projectUuid, PurgeMapper mapper, PurgeCommands commands) { + List rootAndSubviews = mapper.selectRootAndSubviewsByProjectUuid(projectUuid); + commands.deleteLinks(projectUuid); + commands.deleteScannerCache(projectUuid); + commands.deleteEventComponentChanges(projectUuid); + commands.deleteAnalyses(projectUuid); commands.deleteByRootAndSubviews(rootAndSubviews); - commands.deleteIssues(rootUuid); - commands.deleteFileSources(rootUuid); - commands.deleteCeActivity(rootUuid); - commands.deleteCeQueue(rootUuid); - commands.deleteWebhooks(rootUuid); - commands.deleteWebhookDeliveries(rootUuid); - commands.deleteLiveMeasures(rootUuid); - commands.deleteProjectAlmSettings(rootUuid); - commands.deletePermissions(rootUuid); - commands.deleteNewCodePeriods(rootUuid); - commands.deleteBranch(rootUuid); - commands.deleteApplicationBranchProjects(rootUuid); - commands.deleteApplicationProjects(rootUuid); - commands.deleteApplicationProjectsByProject(rootUuid); - commands.deleteProjectInPortfolios(rootUuid); - commands.deleteComponents(rootUuid); - commands.deleteNonMainBranchComponentsByProjectUuid(rootUuid); - commands.deleteProjectBadgeToken(rootUuid); - commands.deleteProject(rootUuid); - commands.deleteUserDismissedMessages(rootUuid); - commands.deleteOutdatedProperties(rootUuid); - commands.deleteReportSchedules(rootUuid); - commands.deleteReportSubscriptions(rootUuid); + commands.deleteIssues(projectUuid); + commands.deleteFileSources(projectUuid); + commands.deleteCeActivity(projectUuid); + commands.deleteCeQueue(projectUuid); + commands.deleteWebhooks(projectUuid); + commands.deleteWebhookDeliveries(projectUuid); + commands.deleteLiveMeasures(projectUuid); + commands.deleteProjectAlmSettings(projectUuid); + commands.deletePermissions(projectUuid); + commands.deleteNewCodePeriodsForProject(projectUuid); + commands.deleteBranch(projectUuid); + commands.deleteApplicationBranchProjects(projectUuid); + commands.deleteApplicationProjects(projectUuid); + commands.deleteApplicationProjectsByProject(projectUuid); + commands.deleteProjectInPortfolios(projectUuid); + commands.deleteComponents(projectUuid); + commands.deleteNonMainBranchComponentsByProjectUuid(projectUuid); + commands.deleteProjectBadgeToken(projectUuid); + commands.deleteProject(projectUuid); + commands.deleteUserDismissedMessages(projectUuid); + commands.deleteOutdatedProperties(projectUuid); + commands.deleteReportSchedules(projectUuid); + commands.deleteReportSubscriptions(projectUuid); } /** diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeMapper.java index 9f702aec832c..34c6378a75ba 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeMapper.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/purge/PurgeMapper.java @@ -171,7 +171,9 @@ void deleteCeActivityByRootUuidOrBefore(@Nullable @Param("rootUuid") String root void deleteLiveMeasuresByComponentUuids(@Param("componentUuids") List componentUuids); - void deleteNewCodePeriodsByRootUuid(String rootUuid); + void deleteNewCodePeriodsByProjectUuid(String projectUuid); + + void deleteNewCodePeriodsByBranchUuid(String branchUuid); void deleteProjectAlmSettingsByProjectUuid(@Param("projectUuid") String projectUuid); diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/purge/PurgeMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/purge/PurgeMapper.xml index e29021fe997d..807f9d1ac6d1 100644 --- a/server/sonar-db-dao/src/main/resources/org/sonar/db/purge/PurgeMapper.xml +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/purge/PurgeMapper.xml @@ -582,11 +582,16 @@ or entity_uuid=#{rootUuid,jdbcType=VARCHAR} - + DELETE FROM new_code_periods WHERE - branch_uuid=#{rootUuid,jdbcType=VARCHAR} - OR project_uuid=#{rootUuid,jdbcType=VARCHAR} + project_uuid=#{projectUuid,jdbcType=VARCHAR} + + + + DELETE FROM new_code_periods + WHERE + branch_uuid=#{branchUuid,jdbcType=VARCHAR}