Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: introduce caching and projection to optimise FPL #36118

Merged
merged 9 commits into from
Sep 9, 2024
Original file line number Diff line number Diff line change
@@ -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";
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
subrata71 marked this conversation as resolved.
Show resolved Hide resolved
Mono<Application> branchedApplicationMono = !StringUtils.hasLength(params.getFirst(FieldName.APPLICATION_ID))
? Mono.just(new Application())
: applicationService.findByBranchNameAndBaseApplicationId(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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(
subrata71 marked this conversation as resolved.
Show resolved Hide resolved
String branchName, String defaultPageId, AclPermission permission, List<String> projectedFieldNames);

Mono<NewPage> findByBranchNameAndBasePageIdAndApplicationMode(
String branchName, String basePageId, ApplicationMode mode);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {

Expand Down Expand Up @@ -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(
sneha122 marked this conversation as resolved.
Show resolved Hide resolved
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
NilanshBansal marked this conversation as resolved.
Show resolved Hide resolved
.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(
Expand All @@ -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));
}
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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
Expand Down Expand Up @@ -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) {
NilanshBansal marked this conversation as resolved.
Show resolved Hide resolved
return !StringUtils.hasText(baseApplicationId) ? Mono.empty() : Mono.just(baseApplicationId);
NilanshBansal marked this conversation as resolved.
Show resolved Hide resolved
}

@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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand All @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -60,7 +61,8 @@ public ApplicationPageServiceImpl(
DSLMigrationUtils dslMigrationUtils,
ClonePageService<NewAction> actionClonePageService,
ClonePageService<ActionCollection> actionCollectionClonePageService,
ObservationRegistry observationRegistry) {
ObservationRegistry observationRegistry,
CacheableRepositoryHelper cacheableRepositoryHelper) {
super(
workspaceService,
applicationService,
Expand Down Expand Up @@ -89,6 +91,7 @@ public ApplicationPageServiceImpl(
dslMigrationUtils,
actionClonePageService,
actionCollectionClonePageService,
observationRegistry);
observationRegistry,
cacheableRepositoryHelper);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -33,7 +34,8 @@ public ConsolidatedAPIServiceImpl(
PluginService pluginService,
DatasourceService datasourceService,
MockDataService mockDataService,
ObservationRegistry observationRegistry) {
ObservationRegistry observationRegistry,
CacheableRepositoryHelper cacheableRepositoryHelper) {
super(
sessionUserService,
userService,
Expand All @@ -50,6 +52,7 @@ public ConsolidatedAPIServiceImpl(
pluginService,
datasourceService,
mockDataService,
observationRegistry);
observationRegistry,
cacheableRepositoryHelper);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)));
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down
Loading
Loading