From ba81cb2ea12278dea5497be980f86d942c21196c Mon Sep 17 00:00:00 2001 From: Nayan Date: Thu, 9 Mar 2023 11:46:43 +0600 Subject: [PATCH] feat: Add option to delete a application snapshot (#21216) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description This PR adds the option to delete an application snapshot in the following cases: - A DELETE API that'll delete an application snapshot - The restore API should delete the underlying application snapshot if the restore is successful Fixes #21215 Media > A video or a GIF is preferred. when using Loom, don’t embed because it looks like it’s a GIF. instead, just link to the video ## Type of change > Please delete options that are not relevant. - Chore (housekeeping or task changes that don't impact user perception) ## How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Provide instructions, so we can reproduce. > Please also list any relevant details for your test configuration. > Delete anything that is not important - Manual - JUnit tests ### Test Plan > Add Testsmith test cases links that relate to this PR ### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) ## Checklist: ### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag ### QA activity: - [ ] Test plan has been approved by relevant developers - [ ] Test plan has been peer reviewed by QA - [ ] Cypress test cases have been added and approved by either SDET or manual QA - [ ] Organized project review call with relevant stakeholders after Round 1/2 of QA - [ ] Added Test Plan Approved label after reveiwing all Cypress test --- .../ce/ApplicationControllerCE.java | 8 +++ .../ce/ApplicationSnapshotServiceCE.java | 2 + .../ce/ApplicationSnapshotServiceCEImpl.java | 18 ++++++- .../ApplicationSnapshotServiceTest.java | 51 ++++++++++++++++++- .../ApplicationSnapshotServiceUnitTest.java | 3 ++ 5 files changed, 79 insertions(+), 3 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ApplicationControllerCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ApplicationControllerCE.java index 937cf94d65..2c182e08c9 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ApplicationControllerCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ApplicationControllerCE.java @@ -201,6 +201,14 @@ public class ApplicationControllerCE extends BaseController new ResponseDTO<>(HttpStatus.OK.value(), applicationSnapshot, null)); } + @DeleteMapping("/snapshot/{id}") + public Mono> deleteSnapshotWithoutApplicationJson(@PathVariable String id, @RequestHeader(name = FieldName.BRANCH_NAME, required = false) String branchName) { + log.debug("Going to delete snapshot with application id: {}, branch: {}", id, branchName); + + return applicationSnapshotService.deleteSnapshot(id, branchName) + .map(isDeleted -> new ResponseDTO<>(HttpStatus.OK.value(), isDeleted, null)); + } + @PostMapping("/snapshot/{id}/restore") public Mono> restoreSnapshot(@PathVariable String id, @RequestHeader(name = FieldName.BRANCH_NAME, required = false) String branchName) { log.debug("Going to restore snapshot with application id: {}, branch: {}", id, branchName); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationSnapshotServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationSnapshotServiceCE.java index 2548271337..3e22d5b593 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationSnapshotServiceCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationSnapshotServiceCE.java @@ -17,4 +17,6 @@ public interface ApplicationSnapshotServiceCE { Mono getWithoutDataByApplicationId(String applicationId, String branchName); Mono restoreSnapshot(String applicationId, String branchName); + + Mono deleteSnapshot(String applicationId, String branchName); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationSnapshotServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationSnapshotServiceCEImpl.java index eb864625ad..29ecee3db0 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationSnapshotServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationSnapshotServiceCEImpl.java @@ -91,7 +91,11 @@ public class ApplicationSnapshotServiceCEImpl implements ApplicationSnapshotServ return importExportApplicationService.importApplicationInWorkspace( application.getWorkspaceId(), applicationJson, application.getId(), branchName ); - }); + }) + .flatMap(application -> + applicationSnapshotRepository.deleteAllByApplicationId(application.getId()) + .thenReturn(application) + ); } private Mono getApplicationJsonStringFromSnapShot(String applicationId) { @@ -134,4 +138,16 @@ public class ApplicationSnapshotServiceCEImpl implements ApplicationSnapshotServ } return applicationSnapshots; } + + @Override + public Mono deleteSnapshot(String applicationId, String branchName) { + // find root application by applicationId and branchName + return applicationService.findBranchedApplicationId(branchName, applicationId, applicationPermission.getEditPermission()) + .switchIfEmpty(Mono.error( + new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.APPLICATION, applicationId)) + ) + .flatMap(branchedAppId -> + applicationSnapshotRepository.deleteAllByApplicationId(branchedAppId).thenReturn(Boolean.TRUE) + ); + } } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationSnapshotServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationSnapshotServiceTest.java index 5d74b7ce38..07815608cf 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationSnapshotServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationSnapshotServiceTest.java @@ -18,6 +18,7 @@ import reactor.test.StepVerifier; import reactor.util.function.Tuple2; import java.nio.charset.StandardCharsets; +import java.util.List; import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; @@ -210,8 +211,8 @@ public class ApplicationSnapshotServiceTest { pageDTO.setApplicationId(application.getId()); return applicationPageService.createPage(pageDTO) .then(applicationSnapshotService.restoreSnapshot(application.getId(), null)) - .then(newPageService.findApplicationPages(application.getId(), null, null, ApplicationMode.EDIT)); - }); + .then(newPageService.findApplicationPages(application.getId(), null, null, ApplicationMode.EDIT)); + }); // not using Mono.zip because we want pagesBeforeSnapshot to finish first Mono> tuple2Mono = pagesBeforeSnapshot @@ -225,4 +226,50 @@ public class ApplicationSnapshotServiceTest { }) .verifyComplete(); } + + @Test + @WithUserDetails("api_user") + public void restoreSnapshot_WhenSuccessfullyRestored_SnapshotDeleted() { + String uniqueString = UUID.randomUUID().toString(); + + // create a new workspace + Workspace workspace = new Workspace(); + workspace.setName("Test workspace " + uniqueString); + + Flux snapshotFlux = workspaceService.create(workspace) + .flatMap(createdWorkspace -> { + Application testApplication = new Application(); + testApplication.setName("App before snapshot"); + return applicationPageService.createApplication(testApplication, workspace.getId()); + }).flatMap(application -> { // create a snapshot + return applicationSnapshotService.createApplicationSnapshot(application.getId(), null) + .thenReturn(application); + }) + .flatMapMany(application -> + applicationSnapshotService.restoreSnapshot(application.getId(), null) + .thenMany(applicationSnapshotRepository.findByApplicationId(application.getId())) + ); + + StepVerifier.create(snapshotFlux) + .verifyComplete(); + } + + @Test + public void deleteSnapshot_WhenSnapshotExists_Deleted() { + String testAppId = "app-" + UUID.randomUUID().toString(); + ApplicationSnapshot snapshot1 = new ApplicationSnapshot(); + snapshot1.setChunkOrder(1); + snapshot1.setApplicationId(testAppId); + + ApplicationSnapshot snapshot2 = new ApplicationSnapshot(); + snapshot2.setApplicationId(testAppId); + snapshot2.setChunkOrder(2); + + Flux snapshotFlux = applicationSnapshotRepository.saveAll(List.of(snapshot1, snapshot2)) + .then(applicationSnapshotService.deleteSnapshot(testAppId, null)) + .thenMany(applicationSnapshotRepository.findByApplicationId(testAppId)); + + StepVerifier.create(snapshotFlux) + .verifyComplete(); + } } \ No newline at end of file diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/ApplicationSnapshotServiceUnitTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/ApplicationSnapshotServiceUnitTest.java index c3f9f10151..043b41c1bb 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/ApplicationSnapshotServiceUnitTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/ApplicationSnapshotServiceUnitTest.java @@ -134,6 +134,9 @@ public class ApplicationSnapshotServiceUnitTest { Mockito.when(importExportApplicationService.importApplicationInWorkspace(eq(application.getWorkspaceId()), argThat(matchApplicationJson), eq(branchedAppId), eq(branch))) .thenReturn(Mono.just(application)); + Mockito.when(applicationSnapshotRepository.deleteAllByApplicationId(branchedAppId)) + .thenReturn(Mono.just("application").then()); + StepVerifier.create(applicationSnapshotService.restoreSnapshot(defaultAppId, branch)) .assertNext(application1 -> { assertThat(application1.getName()).isEqualTo(application.getName());