From 6e21516900f218495f9b45bd34b7f68c54c6bd5b Mon Sep 17 00:00:00 2001 From: Abhijeet <41686026+abhvsn@users.noreply.github.com> Date: Thu, 18 Apr 2024 22:15:49 +0530 Subject: [PATCH] chore: Move non-permission repository method to generic repo class (#32738) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description PR to add separate record class to implement the projection on ApplicationSnapshot repository. ## Automation /ok-to-test tags="@tag.Sanity" ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: 5baf4317e0bdb7179fe855c4fe108f9129a31f6b > Cypress dashboard url: Click here! ## Summary by CodeRabbit ## Summary by CodeRabbit - **Refactor** - Enhanced the application snapshot feature to improve performance and reduce data load by excluding unnecessary data from snapshots. - **Documentation** - Updated import statements and method return types for clarity and consistency. --- .../ce/ApplicationControllerCE.java | 4 +-- .../projections/DefaultTimestampOnly.java | 5 ++++ .../ce/ApplicationSnapshotRepositoryCE.java | 3 +++ ...CustomApplicationSnapshotRepositoryCE.java | 5 +--- ...omApplicationSnapshotRepositoryCEImpl.java | 22 +--------------- .../ce/ApplicationSnapshotServiceCE.java | 4 +-- .../ce/ApplicationSnapshotServiceCEImpl.java | 8 +++--- .../ApplicationSnapshotRepositoryTest.java | 22 +++++++--------- .../ApplicationSnapshotServiceTest.java | 26 +++++++++---------- 9 files changed, 41 insertions(+), 58 deletions(-) create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/projections/DefaultTimestampOnly.java 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 e20dee1aae..56bf80219b 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 @@ -6,7 +6,6 @@ import com.appsmith.server.applications.base.ApplicationService; import com.appsmith.server.constants.FieldName; import com.appsmith.server.constants.Url; import com.appsmith.server.domains.Application; -import com.appsmith.server.domains.ApplicationSnapshot; import com.appsmith.server.domains.GitAuth; import com.appsmith.server.domains.Theme; import com.appsmith.server.dtos.ApplicationAccessDTO; @@ -28,6 +27,7 @@ import com.appsmith.server.exports.internal.partial.PartialExportService; import com.appsmith.server.fork.internal.ApplicationForkingService; import com.appsmith.server.imports.internal.ImportService; import com.appsmith.server.imports.internal.partial.PartialImportService; +import com.appsmith.server.projections.DefaultTimestampOnly; import com.appsmith.server.services.ApplicationPageService; import com.appsmith.server.services.ApplicationSnapshotService; import com.appsmith.server.solutions.ApplicationFetcher; @@ -232,7 +232,7 @@ public class ApplicationControllerCE { @JsonView(Views.Public.class) @GetMapping("/snapshot/{id}") - public Mono> getSnapshotWithoutApplicationJson( + public Mono> getSnapshotWithoutApplicationJson( @PathVariable String id, @RequestHeader(name = FieldName.BRANCH_NAME, required = false) String branchName) { log.debug("Going to get snapshot with application id: {}, branch: {}", id, branchName); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/projections/DefaultTimestampOnly.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/projections/DefaultTimestampOnly.java new file mode 100644 index 0000000000..8abddc7d0b --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/projections/DefaultTimestampOnly.java @@ -0,0 +1,5 @@ +package com.appsmith.server.projections; + +import java.time.Instant; + +public record DefaultTimestampOnly(Instant updatedAt, Instant createdAt) {} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/ApplicationSnapshotRepositoryCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/ApplicationSnapshotRepositoryCE.java index 2be4d29f96..93deea325c 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/ApplicationSnapshotRepositoryCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/ApplicationSnapshotRepositoryCE.java @@ -1,6 +1,7 @@ package com.appsmith.server.repositories.ce; import com.appsmith.server.domains.ApplicationSnapshot; +import com.appsmith.server.projections.DefaultTimestampOnly; import com.appsmith.server.repositories.BaseRepository; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -10,4 +11,6 @@ public interface ApplicationSnapshotRepositoryCE Flux findByApplicationId(String applicationId); Mono deleteAllByApplicationId(String applicationId); + + Mono findByApplicationIdAndChunkOrder(String applicationId, Integer chunkOrder); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationSnapshotRepositoryCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationSnapshotRepositoryCE.java index 52887f17a9..f9a23f1103 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationSnapshotRepositoryCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationSnapshotRepositoryCE.java @@ -2,8 +2,5 @@ package com.appsmith.server.repositories.ce; import com.appsmith.server.domains.ApplicationSnapshot; import com.appsmith.server.repositories.AppsmithRepository; -import reactor.core.publisher.Mono; -public interface CustomApplicationSnapshotRepositoryCE extends AppsmithRepository { - Mono findWithoutData(String applicationId); -} +public interface CustomApplicationSnapshotRepositoryCE extends AppsmithRepository {} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationSnapshotRepositoryCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationSnapshotRepositoryCEImpl.java index 8d45a4f4b1..1688cd6a68 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationSnapshotRepositoryCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationSnapshotRepositoryCEImpl.java @@ -1,27 +1,7 @@ package com.appsmith.server.repositories.ce; import com.appsmith.server.domains.ApplicationSnapshot; -import com.appsmith.server.helpers.ce.bridge.Bridge; -import com.appsmith.server.helpers.ce.bridge.BridgeQuery; import com.appsmith.server.repositories.BaseAppsmithRepositoryImpl; -import reactor.core.publisher.Mono; - -import java.util.List; public class CustomApplicationSnapshotRepositoryCEImpl extends BaseAppsmithRepositoryImpl - implements CustomApplicationSnapshotRepositoryCE { - - @Override - public Mono findWithoutData(String applicationId) { - BridgeQuery query = Bridge.equal( - ApplicationSnapshot.Fields.applicationId, applicationId) - .equal(ApplicationSnapshot.Fields.chunkOrder, 1); - - List fieldNames = List.of( - ApplicationSnapshot.Fields.applicationId, - ApplicationSnapshot.Fields.chunkOrder, - ApplicationSnapshot.Fields.createdAt, - ApplicationSnapshot.Fields.updatedAt); - return queryBuilder().criteria(query).fields(fieldNames).one(); - } -} + implements CustomApplicationSnapshotRepositoryCE {} 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 33bc8329fd..298b89e750 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 @@ -1,7 +1,7 @@ package com.appsmith.server.services.ce; import com.appsmith.server.domains.Application; -import com.appsmith.server.domains.ApplicationSnapshot; +import com.appsmith.server.projections.DefaultTimestampOnly; import reactor.core.publisher.Mono; public interface ApplicationSnapshotServiceCE { @@ -15,7 +15,7 @@ public interface ApplicationSnapshotServiceCE { */ Mono createApplicationSnapshot(String applicationId, String branchName); - Mono getWithoutDataByApplicationId(String applicationId, String branchName); + Mono getWithoutDataByApplicationId(String applicationId, String branchName); Mono restoreSnapshot(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 e6c9e924a4..e75a45ede4 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 @@ -12,6 +12,7 @@ import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.exports.internal.ExportService; import com.appsmith.server.helpers.ResponseUtils; import com.appsmith.server.imports.internal.ImportService; +import com.appsmith.server.projections.DefaultTimestampOnly; import com.appsmith.server.repositories.ApplicationSnapshotRepository; import com.appsmith.server.solutions.ApplicationPermission; import com.google.gson.Gson; @@ -67,12 +68,13 @@ public class ApplicationSnapshotServiceCEImpl implements ApplicationSnapshotServ } @Override - public Mono getWithoutDataByApplicationId(String applicationId, String branchName) { + public Mono getWithoutDataByApplicationId(String applicationId, String branchName) { // get application first to check the permission and get child aka branched application ID return applicationService .findBranchedApplicationId(branchName, applicationId, applicationPermission.getEditPermission()) - .flatMap(applicationSnapshotRepository::findWithoutData) - .defaultIfEmpty(new ApplicationSnapshot()); + .flatMap(branchedApplicationId -> + applicationSnapshotRepository.findByApplicationIdAndChunkOrder(branchedApplicationId, 1)) + .defaultIfEmpty(new DefaultTimestampOnly(null, null)); } @Override diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/repositories/ApplicationSnapshotRepositoryTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/repositories/ApplicationSnapshotRepositoryTest.java index fc2ebf225e..f64d2502f4 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/repositories/ApplicationSnapshotRepositoryTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/repositories/ApplicationSnapshotRepositoryTest.java @@ -1,7 +1,7 @@ package com.appsmith.server.repositories; import com.appsmith.server.domains.ApplicationSnapshot; -import com.google.gson.Gson; +import com.appsmith.server.projections.DefaultTimestampOnly; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; @@ -20,9 +20,6 @@ public class ApplicationSnapshotRepositoryTest { @Autowired private ApplicationSnapshotRepository applicationSnapshotRepository; - @Autowired - private Gson gson; - @Test @WithUserDetails("api_user") public void findWithoutData_WhenMatched_ReturnsMatchedDocumentWithoutData() { @@ -38,15 +35,14 @@ public class ApplicationSnapshotRepositoryTest { snapshot2.setApplicationId(testAppId2); snapshot2.setChunkOrder(1); - Mono snapshotMono = applicationSnapshotRepository + Mono snapshotMono = applicationSnapshotRepository .saveAll(List.of(snapshot1, snapshot2)) - .then(applicationSnapshotRepository.findWithoutData(testAppId2)); + .then(applicationSnapshotRepository.findByApplicationIdAndChunkOrder(testAppId2, 1)); StepVerifier.create(snapshotMono) .assertNext(applicationSnapshot -> { - assertThat(applicationSnapshot.getApplicationId()).isEqualTo(testAppId2); - assertThat(applicationSnapshot.getData()).isNull(); - assertThat(applicationSnapshot.getChunkOrder()).isEqualTo(1); + assertThat(applicationSnapshot.updatedAt()).isNotNull(); + assertThat(applicationSnapshot.createdAt()).isNotNull(); }) .verifyComplete(); } @@ -65,14 +61,14 @@ public class ApplicationSnapshotRepositoryTest { snapshot2.setApplicationId(testAppId1); snapshot2.setChunkOrder(2); - Mono snapshotMono = applicationSnapshotRepository + Mono snapshotMono = applicationSnapshotRepository .saveAll(List.of(snapshot1, snapshot2)) - .then(applicationSnapshotRepository.findWithoutData(testAppId1)); + .then(applicationSnapshotRepository.findByApplicationIdAndChunkOrder(testAppId1, 1)); StepVerifier.create(snapshotMono) .assertNext(applicationSnapshot -> { - assertThat(applicationSnapshot.getApplicationId()).isEqualTo(testAppId1); - assertThat(applicationSnapshot.getChunkOrder()).isEqualTo(1); + assertThat(applicationSnapshot.createdAt()).isNotNull(); + assertThat(applicationSnapshot.updatedAt()).isNotNull(); }) .verifyComplete(); } 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 ac76c01e7e..f1687c1203 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 @@ -10,6 +10,7 @@ import com.appsmith.server.domains.Workspace; import com.appsmith.server.dtos.ApplicationPagesDTO; import com.appsmith.server.dtos.PageDTO; import com.appsmith.server.newpages.base.NewPageService; +import com.appsmith.server.projections.DefaultTimestampOnly; import com.appsmith.server.repositories.ApplicationSnapshotRepository; import com.appsmith.server.solutions.ApplicationPermission; import org.junit.jupiter.api.AfterEach; @@ -91,7 +92,7 @@ public class ApplicationSnapshotServiceTest { Application testApplication = new Application(); testApplication.setName("Test app for snapshot"); testApplication.setWorkspaceId(workspace.getId()); - Mono snapshotMono = applicationPageService + Mono snapshotMono = applicationPageService .createApplication(testApplication) .flatMap(application -> { assert application.getId() != null; @@ -104,8 +105,8 @@ public class ApplicationSnapshotServiceTest { StepVerifier.create(snapshotMono) .assertNext(snapshot -> { - assertThat(snapshot.getApplicationId()).isNotNull(); - assertThat(snapshot.getData()).isNull(); + assertThat(snapshot.updatedAt()).isNotNull(); + assertThat(snapshot.createdAt()).isNotNull(); }) .verifyComplete(); } @@ -116,7 +117,7 @@ public class ApplicationSnapshotServiceTest { Application testApplication = new Application(); testApplication.setName("Test app for snapshot"); testApplication.setWorkspaceId(workspace.getId()); - Mono snapshotMono = applicationPageService + Mono snapshotMono = applicationPageService .createApplication(testApplication) .flatMap(application -> { assert application.getId() != null; @@ -131,8 +132,8 @@ public class ApplicationSnapshotServiceTest { StepVerifier.create(snapshotMono) .assertNext(snapshot -> { - assertThat(snapshot.getApplicationId()).isNotNull(); - assertThat(snapshot.getData()).isNull(); + assertThat(snapshot.updatedAt()).isNotNull(); + assertThat(snapshot.createdAt()).isNotNull(); }) .verifyComplete(); } @@ -152,7 +153,7 @@ public class ApplicationSnapshotServiceTest { gitArtifactMetadata.setDefaultApplicationId(testDefaultAppId); gitArtifactMetadata.setBranchName(testBranchName); testApplication.setGitApplicationMetadata(gitArtifactMetadata); - Mono> tuple2Mono = applicationPageService + Mono> tuple2Mono = applicationPageService .createApplication(testApplication) .flatMap(application -> applicationSnapshotService .createApplicationSnapshot(testDefaultAppId, testBranchName) @@ -162,10 +163,9 @@ public class ApplicationSnapshotServiceTest { StepVerifier.create(tuple2Mono) .assertNext(objects -> { - ApplicationSnapshot applicationSnapshot = objects.getT1(); - Application application = objects.getT2(); - assertThat(applicationSnapshot.getData()).isNull(); - assertThat(applicationSnapshot.getApplicationId()).isEqualTo(application.getId()); + DefaultTimestampOnly applicationSnapshot = objects.getT1(); + assertThat(applicationSnapshot.updatedAt()).isNotNull(); + assertThat(applicationSnapshot.createdAt()).isNotNull(); }) .verifyComplete(); } @@ -304,7 +304,7 @@ public class ApplicationSnapshotServiceTest { Application testApplication = new Application(); testApplication.setName("Test app for snapshot"); testApplication.setWorkspaceId(workspace.getId()); - Mono applicationSnapshotMono = applicationPageService + Mono applicationSnapshotMono = applicationPageService .createApplication(testApplication) .flatMap(application1 -> { return applicationSnapshotService.getWithoutDataByApplicationId(application1.getId(), null); @@ -312,7 +312,7 @@ public class ApplicationSnapshotServiceTest { StepVerifier.create(applicationSnapshotMono) .assertNext(applicationSnapshot -> { - assertThat(applicationSnapshot.getId()).isNull(); + assertThat(applicationSnapshot.updatedAt()).isNull(); }) .verifyComplete(); }