chore: introduce caching and projection to optimise FPL (#36118)

## Description This PR aims to enhance the performance of the pages span in consolidated-API view mode. It does so by brining the following changes - **Add Projection:** Implement projections to enhance performance for `appsmith.consolidated-api.view.pages.getpage`. - **Implement Caching:** Introduce caching to eliminate the `getpage` query, thereby optimizing `appsmith.consolidated-api.view.application_id` and related spans. We will implement caching to store `defaultApplicationId` information in memory. This cache will reduce the need for the time-consuming `getpage` query. On a cache miss, the system will first fetch the page and then the branched application. If the information is present in the cache, it will directly retrieve the branched application, leveraging the cached data for improved efficiency. With these changes, the pages API is showing improvements. A few screenshots from local environment: ![Screenshot 2024-09-04 at 6 57 01 PM](https://github.com/user-attachments/assets/c5e91d81-7902-4f58-8d18-8c16e92af540) ![Screenshot 2024-09-04 at 6 57 46 PM](https://github.com/user-attachments/assets/3b99000e-2236-41cc-8955-b8b30780ee87) ![Screenshot 2024-09-04 at 7 03 17 PM](https://github.com/user-attachments/assets/ff60d8cb-2588-40a7-ad82-b132da6dba72) ![Screenshot 2024-09-04 at 7 12 47 PM](https://github.com/user-attachments/assets/e528ebc7-b48d-4632-a44c-4a6b3cbb2cf8) Fixes #36102 ## Automation /ok-to-test tags="@tag.Sanity" ### 🔍 Cypress test results  > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/10778438378> > Commit: 25e71b5 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10778438378&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Sanity` > Spec: > <hr>Mon, 09 Sep 2024 18:03:52 UTC  ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No  ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Enhanced page retrieval capabilities with additional parameters for improved data handling. - Introduced a caching mechanism for optimized retrieval of application IDs in view mode. - Added support for bulk cache eviction through the `@CacheEvict` annotation. - **Bug Fixes** - Adjusted method signatures in tests to align with updated service methods, ensuring proper functionality. - **Documentation** - Updated test cases to reflect changes in method parameters for clarity and accuracy. - **Chores** - Refactored method calls across various services and tests to incorporate new parameters and improve overall performance.  --------- Co-authored-by: “sneha122” <“[email protected]”>
appsmithorg · Sep 9, 2024 · 9a26066 · 9a26066
1 parent 205ba07
commit 9a26066
Show file tree

Hide file tree

Showing 23 changed files with 348 additions and 54 deletions.
diff --git a/...-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/ApplicationSpanCE.java b/...-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/ApplicationSpanCE.java
@@ -1,7 +1,10 @@
 package com.appsmith.external.constants.spans.ce;
 
+import static com.appsmith.external.constants.spans.ConsolidatedApiSpanNames.APPLICATION_ID_SPAN;
 import static com.appsmith.external.constants.spans.ConsolidatedApiSpanNames.CONSOLIDATED_API_PREFIX;
 
 public class ApplicationSpanCE {
     public static final String APPLICATION_FETCH_FROM_DB = CONSOLIDATED_API_PREFIX + "app_db";
+    public static final String APPLICATION_ID_FETCH_REDIS_SPAN = APPLICATION_ID_SPAN + ".read_redis";
+    public static final String APPLICATION_ID_UPDATE_REDIS_SPAN = APPLICATION_ID_SPAN + ".update_redis";
 }
diff --git a/...-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java b/...-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java
@@ -1037,7 +1037,9 @@ public Mono<Application> findByBaseIdBranchNameAndApplicationMode(
                 ? applicationPermission.getReadPermission()
                 : applicationPermission.getEditPermission();
 
-        return findByBranchNameAndBaseApplicationId(branchName, defaultApplicationId, permissionForApplication);
+        return findByBranchNameAndBaseApplicationId(branchName, defaultApplicationId, permissionForApplication)
+                .name(APPLICATION_FETCH_FROM_DB)
+                .tap(Micrometer.observation(observationRegistry));
     }
 
     @Override

diff --git a/...mith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java b/...mith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java
@@ -1012,7 +1012,7 @@ public Flux<ActionDTO> getUnpublishedActions(
         Mono<NewPage> branchedPageMono = !StringUtils.hasLength(params.getFirst(FieldName.PAGE_ID))
                 ? Mono.just(new NewPage())
                 : newPageService.findByBranchNameAndBasePageId(
-                        branchName, params.getFirst(FieldName.PAGE_ID), pagePermission.getReadPermission());
+                        branchName, params.getFirst(FieldName.PAGE_ID), pagePermission.getReadPermission(), null);
         Mono<Application> branchedApplicationMono = !StringUtils.hasLength(params.getFirst(FieldName.APPLICATION_ID))
                 ? Mono.just(new Application())
                 : applicationService.findByBranchNameAndBaseApplicationId(

diff --git a/...ver/appsmith-server/src/main/java/com/appsmith/server/newpages/base/NewPageServiceCE.java b/...ver/appsmith-server/src/main/java/com/appsmith/server/newpages/base/NewPageServiceCE.java
@@ -30,8 +30,6 @@ public interface NewPageServiceCE extends CrudService<NewPage, String> {
     Flux<NewPage> findNewPagesByApplicationId(
             String applicationId, AclPermission permission, List<String> includeFields);
 
-    Mono<NewPage> findByIdAndBranchName(String id, String branchName);
-
     Mono<PageDTO> saveUnpublishedPage(PageDTO page);
 
     Mono<PageDTO> createDefault(PageDTO object);
@@ -71,7 +69,8 @@ Mono<PageDTO> findByNameAndApplicationIdAndViewMode(
 
     Mono<String> getNameByPageId(String pageId, boolean isPublishedName);
 
-    Mono<NewPage> findByBranchNameAndBasePageId(String branchName, String defaultPageId, AclPermission permission);
+    Mono<NewPage> findByBranchNameAndBasePageId(
+            String branchName, String defaultPageId, AclPermission permission, List<String> projectedFieldNames);
 
     Mono<NewPage> findByBranchNameAndBasePageIdAndApplicationMode(
             String branchName, String basePageId, ApplicationMode mode);

diff --git a/...appsmith-server/src/main/java/com/appsmith/server/newpages/base/NewPageServiceCEImpl.java b/...appsmith-server/src/main/java/com/appsmith/server/newpages/base/NewPageServiceCEImpl.java
@@ -136,11 +136,6 @@ public Flux<PageDTO> findByApplicationId(String applicationId, AclPermission per
         return findNewPagesByApplicationId(applicationId, permission).flatMap(page -> getPageByViewMode(page, view));
     }
 
-    @Override
-    public Mono<NewPage> findByIdAndBranchName(String id, String branchName) {
-        return this.findByBranchNameAndBasePageId(branchName, id, pagePermission.getReadPermission());
-    }
-
     @Override
     public Mono<PageDTO> saveUnpublishedPage(PageDTO page) {
 
@@ -511,19 +506,21 @@ public Mono<String> getNameByPageId(String pageId, boolean isPublishedName) {
     }
 
     @Override
-    public Mono<NewPage> findByBranchNameAndBasePageId(String branchName, String basePageId, AclPermission permission) {
+    public Mono<NewPage> findByBranchNameAndBasePageId(
+            String branchName, String basePageId, AclPermission permission, List<String> projectedFieldNames) {
 
         if (!StringUtils.hasText(basePageId)) {
             return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.PAGE_ID));
         } else if (!StringUtils.hasText(branchName)) {
-            return this.findById(basePageId, permission)
+            return repository
+                    .findById(basePageId, permission, projectedFieldNames)
                     .name(GET_PAGE_WITHOUT_BRANCH)
                     .tap(Micrometer.observation(observationRegistry))
                     .switchIfEmpty(Mono.error(
                             new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.PAGE, basePageId)));
         }
         return repository
-                .findPageByBranchNameAndBasePageId(branchName, basePageId, permission)
+                .findPageByBranchNameAndBasePageId(branchName, basePageId, permission, projectedFieldNames)
                 .name(GET_PAGE_WITH_BRANCH)
                 .tap(Micrometer.observation(observationRegistry))
                 .switchIfEmpty(Mono.error(new AppsmithException(
@@ -541,7 +538,8 @@ public Mono<NewPage> findByBranchNameAndBasePageIdAndApplicationMode(
             permission = pagePermission.getReadPermission();
         }
 
-        return this.findByBranchNameAndBasePageId(branchName, basePageId, permission)
+        return this.findByBranchNameAndBasePageId(
+                        branchName, basePageId, permission, List.of(NewPage.Fields.id, NewPage.Fields.applicationId))
                 .name(getQualifiedSpanName(GET_PAGE, mode))
                 .tap(Micrometer.observation(observationRegistry));
     }
@@ -556,7 +554,7 @@ public Mono<String> findBranchedPageId(String branchName, String basePageId, Acl
             return Mono.just(basePageId);
         }
         return repository
-                .findPageByBranchNameAndBasePageId(branchName, basePageId, permission)
+                .findPageByBranchNameAndBasePageId(branchName, basePageId, permission, null)
                 .switchIfEmpty(Mono.error(new AppsmithException(
                         AppsmithError.NO_RESOURCE_FOUND, FieldName.PAGE_ID, basePageId + ", " + branchName)))
                 .map(NewPage::getId);

diff --git a/...server/src/main/java/com/appsmith/server/repositories/ce/CacheableRepositoryHelperCE.java b/...server/src/main/java/com/appsmith/server/repositories/ce/CacheableRepositoryHelperCE.java
@@ -4,6 +4,7 @@
 import com.appsmith.server.domains.User;
 import reactor.core.publisher.Mono;
 
+import java.util.List;
 import java.util.Set;
 
 public interface CacheableRepositoryHelperCE {
@@ -23,4 +24,25 @@ public interface CacheableRepositoryHelperCE {
     Mono<Tenant> fetchDefaultTenant(String tenantId);
 
     Mono<Void> evictCachedTenant(String tenantId);
+
+    /**
+     * Retrieves the base application ID from the cache based on the provided base page ID.
+     *
+     * <p>If the cache contains the ID for the specified {@code basePageId}, it is returned as a {@code Mono} containing the {@code baseApplicationId}.
+     * If the cache does not contain the ID (cache miss) and {@code baseApplicationId} is {@code null} or empty, an empty {@code Mono} is returned.</p>
+     *
+     * <p>If {@code baseApplicationId} is not {@code null} or empty and a cache miss occurs, the cache will be updated with the provided {@code baseApplicationId}.</p>
+     *
+     * <p>Note that calling this method with a {@code null} {@code baseApplicationId} on a cache miss will not update the cache.
+     * In this case, the method will return an empty {@code Mono}, and no cache update will occur.</p>
+     *
+     * @param basePageId the identifier for the base page used as the cache key
+     * @param baseApplicationId the base application ID to be returned or used to update the cache if not {@code null} or empty
+     * @return a {@code Mono} containing the {@code baseApplicationId} if it is present in the cache or provided; otherwise, an empty {@code Mono} on a cache miss with a {@code null} or empty {@code baseApplicationId}.
+     *
+     * <p>On a cache miss, if {@code baseApplicationId} is provided, the cache will be updated with the new value after performing additional database operations to fetch the application document.</p>
+     */
+    Mono<String> fetchBaseApplicationId(String basePageId, String baseApplicationId);
+
+    Mono<Boolean> evictCachedBasePageIds(List<String> basePageIds);
 }
diff --git a/...er/src/main/java/com/appsmith/server/repositories/ce/CacheableRepositoryHelperCEImpl.java b/...er/src/main/java/com/appsmith/server/repositories/ce/CacheableRepositoryHelperCEImpl.java
@@ -21,9 +21,11 @@
 import org.springframework.data.mongodb.core.ReactiveMongoOperations;
 import org.springframework.data.mongodb.core.query.Query;
 import org.springframework.stereotype.Component;
+import org.springframework.util.StringUtils;
 import reactor.core.observability.micrometer.Micrometer;
 import reactor.core.publisher.Mono;
 
+import java.util.List;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -41,6 +43,7 @@ public class CacheableRepositoryHelperCEImpl implements CacheableRepositoryHelpe
     private final ReactiveMongoOperations mongoOperations;
     private final InMemoryCacheableRepositoryHelper inMemoryCacheableRepositoryHelper;
     private final ObservationRegistry observationRegistry;
+    private static final String CACHE_DEFAULT_PAGE_ID_TO_DEFAULT_APPLICATION_ID = "pageIdToAppId";
 
     @Cache(cacheName = "permissionGroupsForUser", key = "{#user.email + #user.tenantId}")
     @Override
@@ -199,4 +202,16 @@ public Mono<Tenant> fetchDefaultTenant(String tenantId) {
     public Mono<Void> evictCachedTenant(String tenantId) {
         return Mono.empty().then();
     }
+
+    @Cache(cacheName = CACHE_DEFAULT_PAGE_ID_TO_DEFAULT_APPLICATION_ID, key = "{#basePageId}")
+    @Override
+    public Mono<String> fetchBaseApplicationId(String basePageId, String baseApplicationId) {
+        return !StringUtils.hasText(baseApplicationId) ? Mono.empty() : Mono.just(baseApplicationId);
+    }
+
+    @CacheEvict(cacheName = CACHE_DEFAULT_PAGE_ID_TO_DEFAULT_APPLICATION_ID, keys = "#basePageIds")
+    @Override
+    public Mono<Boolean> evictCachedBasePageIds(List<String> basePageIds) {
+        return Mono.just(Boolean.TRUE);
+    }
 }
diff --git a/...h-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCE.java b/...h-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCE.java
@@ -12,6 +12,8 @@
 
 public interface CustomNewPageRepositoryCE extends AppsmithRepository<NewPage> {
 
+    Mono<NewPage> findById(String id, AclPermission permission, List<String> projectedFields);
+
     Flux<NewPage> findByApplicationId(String applicationId, AclPermission aclPermission);
 
     Flux<NewPage> findByApplicationId(String applicationId, AclPermission aclPermission, List<String> includeFields);
@@ -30,7 +32,8 @@ Mono<NewPage> findByNameAndApplicationIdAndViewMode(
 
     Mono<String> getNameByPageId(String pageId, boolean isPublishedName);
 
-    Mono<NewPage> findPageByBranchNameAndBasePageId(String branchName, String basePageId, AclPermission permission);
+    Mono<NewPage> findPageByBranchNameAndBasePageId(
+            String branchName, String basePageId, AclPermission permission, List<String> projectedFieldNames);
 
     Flux<NewPage> findAllByApplicationIds(List<String> branchedArtifactIds, List<String> includedFields);
 

diff --git a/...rver/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCEImpl.java b/...rver/src/main/java/com/appsmith/server/repositories/ce/CustomNewPageRepositoryCEImpl.java
@@ -40,6 +40,15 @@ public class CustomNewPageRepositoryCEImpl extends BaseAppsmithRepositoryImpl<Ne
     private final MongoTemplate mongoTemplate;
     private final ObservationRegistry observationRegistry;
 
+    @Override
+    public Mono<NewPage> findById(String id, AclPermission permission, List<String> projectedFields) {
+        return queryBuilder()
+                .criteria(Bridge.equal(NewPage.Fields.id, id))
+                .permission(permission)
+                .fields(projectedFields)
+                .one();
+    }
+
     @Override
     public Flux<NewPage> findByApplicationId(String applicationId, AclPermission aclPermission) {
         return queryBuilder()
@@ -161,7 +170,7 @@ public Mono<String> getNameByPageId(String pageId, boolean isPublishedName) {
 
     @Override
     public Mono<NewPage> findPageByBranchNameAndBasePageId(
-            String branchName, String basePageId, AclPermission permission) {
+            String branchName, String basePageId, AclPermission permission, List<String> projectedFieldNames) {
 
         final BridgeQuery<NewPage> q =
                 // defaultPageIdCriteria
@@ -177,6 +186,7 @@ public Mono<NewPage> findPageByBranchNameAndBasePageId(
         return queryBuilder()
                 .criteria(q)
                 .permission(permission)
+                .fields(projectedFieldNames)
                 .one()
                 .name(FETCH_PAGE_FROM_DB)
                 .tap(Micrometer.observation(observationRegistry));

diff --git a/...ppsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java b/...ppsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java
@@ -13,6 +13,7 @@
 import com.appsmith.server.newpages.base.NewPageService;
 import com.appsmith.server.repositories.ActionCollectionRepository;
 import com.appsmith.server.repositories.ApplicationRepository;
+import com.appsmith.server.repositories.CacheableRepositoryHelper;
 import com.appsmith.server.repositories.DatasourceRepository;
 import com.appsmith.server.repositories.NewActionRepository;
 import com.appsmith.server.repositories.NewPageRepository;
@@ -60,7 +61,8 @@ public ApplicationPageServiceImpl(
             DSLMigrationUtils dslMigrationUtils,
             ClonePageService<NewAction> actionClonePageService,
             ClonePageService<ActionCollection> actionCollectionClonePageService,
-            ObservationRegistry observationRegistry) {
+            ObservationRegistry observationRegistry,
+            CacheableRepositoryHelper cacheableRepositoryHelper) {
         super(
                 workspaceService,
                 applicationService,
@@ -89,6 +91,7 @@ public ApplicationPageServiceImpl(
                 dslMigrationUtils,
                 actionClonePageService,
                 actionCollectionClonePageService,
-                observationRegistry);
+                observationRegistry,
+                cacheableRepositoryHelper);
     }
 }
diff --git a/...ppsmith-server/src/main/java/com/appsmith/server/services/ConsolidatedAPIServiceImpl.java b/...ppsmith-server/src/main/java/com/appsmith/server/services/ConsolidatedAPIServiceImpl.java
@@ -7,6 +7,7 @@
 import com.appsmith.server.newactions.base.NewActionService;
 import com.appsmith.server.newpages.base.NewPageService;
 import com.appsmith.server.plugins.base.PluginService;
+import com.appsmith.server.repositories.CacheableRepositoryHelper;
 import com.appsmith.server.services.ce_compatible.ConsolidatedAPIServiceCECompatibleImpl;
 import com.appsmith.server.themes.base.ThemeService;
 import io.micrometer.observation.ObservationRegistry;
@@ -33,7 +34,8 @@ public ConsolidatedAPIServiceImpl(
             PluginService pluginService,
             DatasourceService datasourceService,
             MockDataService mockDataService,
-            ObservationRegistry observationRegistry) {
+            ObservationRegistry observationRegistry,
+            CacheableRepositoryHelper cacheableRepositoryHelper) {
         super(
                 sessionUserService,
                 userService,
@@ -50,6 +52,7 @@ public ConsolidatedAPIServiceImpl(
                 pluginService,
                 datasourceService,
                 mockDataService,
-                observationRegistry);
+                observationRegistry,
+                cacheableRepositoryHelper);
     }
 }
diff --git a/...th-server/src/main/java/com/appsmith/server/services/ce/ApplicationPageServiceCEImpl.java b/...th-server/src/main/java/com/appsmith/server/services/ce/ApplicationPageServiceCEImpl.java
@@ -43,6 +43,7 @@
 import com.appsmith.server.newpages.base.NewPageService;
 import com.appsmith.server.repositories.ActionCollectionRepository;
 import com.appsmith.server.repositories.ApplicationRepository;
+import com.appsmith.server.repositories.CacheableRepositoryHelper;
 import com.appsmith.server.repositories.DatasourceRepository;
 import com.appsmith.server.repositories.NewActionRepository;
 import com.appsmith.server.repositories.NewPageRepository;
@@ -128,6 +129,7 @@ public class ApplicationPageServiceCEImpl implements ApplicationPageServiceCE {
     private final ClonePageService<NewAction> actionClonePageService;
     private final ClonePageService<ActionCollection> actionCollectionClonePageService;
     private final ObservationRegistry observationRegistry;
+    private final CacheableRepositoryHelper cacheableRepositoryHelper;
 
     @Override
     public Mono<PageDTO> createPage(PageDTO page) {
@@ -311,7 +313,7 @@ public Mono<PageDTO> getPageAndMigrateDslByBranchAndBasePageId(
         ApplicationMode applicationMode = viewMode ? ApplicationMode.PUBLISHED : ApplicationMode.EDIT;
         // Fetch the page with read permission in both editor and in viewer.
         return newPageService
-                .findByBranchNameAndBasePageId(branchName, defaultPageId, pagePermission.getReadPermission())
+                .findByBranchNameAndBasePageId(branchName, defaultPageId, pagePermission.getReadPermission(), null)
                 .flatMap(newPage -> getPageDTOAfterMigratingDSL(newPage, viewMode, migrateDsl)
                         .name(getQualifiedSpanName(MIGRATE_DSL, applicationMode))
                         .tap(Micrometer.observation(observationRegistry)));
@@ -1084,6 +1086,9 @@ protected Mono<Tuple2<Mono<Application>, ApplicationPublishingMetaDTO>> publishA
 
                     Mono<Boolean> archivePageMono;
 
+                    Mono<Boolean> evictDeletedDefaultPageIdsMono =
+                            cacheableRepositoryHelper.evictCachedBasePageIds(new ArrayList<>(publishedPageIds));
+
                     if (!publishedPageIds.isEmpty()) {
                         archivePageMono = newPageService.archiveByIds(publishedPageIds);
                     } else {
@@ -1102,8 +1107,12 @@ protected Mono<Tuple2<Mono<Application>, ApplicationPublishingMetaDTO>> publishA
                             newPageService.publishPages(editedPageIds, pagePermission.getEditPermission());
 
                     // Archive the deleted pages and save the application changes and then return the pages so that
-                    // the pages can also be published
-                    return Mono.when(archivePageMono, publishPagesMono, applicationService.save(application))
+                    // the pages can also be published; In addition invalidate the cache for the deleted page Ids
+                    return Mono.when(
+                                    archivePageMono,
+                                    publishPagesMono,
+                                    applicationService.save(application),
+                                    evictDeletedDefaultPageIdsMono)
                             .thenReturn(pages);
                 })
                 .cache(); // caching as we'll need this to send analytics attributes after publishing the app