Skip to content

Commit

Permalink
fix: fixed page data DB call getting called twice (appsmithorg#36247)
Browse files Browse the repository at this point in the history
## Description
After perf updates made in PR
https://github.com/appsmithorg/appsmith/pull/36118/files, Page data DB
fetch call was getting triggered twice, one for PAGES_SPAN and one for
ACTIONS_SPAN. With this PR, we have replaced that a single Mono which is
being cached.

More details:
https://theappsmith.slack.com/archives/C024GUDM0LT/p1725960912325389


Fixes #`Issue Number`  
_or_  
Fixes appsmithorg#36243
> [!WARNING]  
> _If no issue exists, please create an issue first, and check with the
maintainers if the issue is valid._

## Automation

/ok-to-test tags="@tag.Sanity"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/10808901838>
> Commit: d36df4c
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10808901838&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Sanity`
> Spec:
> <hr>Wed, 11 Sep 2024 09:49:39 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [x] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit


- **Performance Improvements**
- Enhanced the page loading process by implementing a caching mechanism
for retrieving branched page data, potentially reducing database calls
and improving overall application performance.
- Streamlined the retrieval logic for branched pages, ensuring
consistent execution regardless of the base page ID state.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: “sneha122” <“[email protected]”>
  • Loading branch information
sneha122 and “sneha122” authored Sep 11, 2024
1 parent cd7b66f commit 1c8712a
Showing 1 changed file with 22 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,9 @@ public Mono<ConsolidatedAPIResponseDTO> getConsolidatedInfoForPageLoad(
boolean isViewMode = ApplicationMode.PUBLISHED.equals(mode);

/* Fetch default application id if not provided */
if (isBlank(basePageId)) {
return Mono.when(fetches).thenReturn(consolidatedAPIResponseDTO);
}
Mono<Application> branchedApplicationMonoCached;
Mono<String> baseApplicationIdMono = Mono.just("");
if (isViewMode) {
Expand All @@ -215,36 +218,29 @@ public Mono<ConsolidatedAPIResponseDTO> getConsolidatedInfoForPageLoad(
.tap(Micrometer.observation(observationRegistry))
.cache();

Mono<NewPage> branchedPageMonoCached = Mono.empty();
if (!isBlank(basePageId)) {
branchedPageMonoCached = newPageService
.findByBranchNameAndBasePageIdAndApplicationMode(branchName, basePageId, mode)
.cache();
}
Mono<NewPage> branchedPageMonoCached = newPageService
.findByBranchNameAndBasePageIdAndApplicationMode(branchName, basePageId, mode)
.cache();

branchedApplicationMonoCached = baseApplicationIdMono.flatMap(cachedBaseApplicationId -> {
if (!StringUtils.hasText(cachedBaseApplicationId)) {
// Handle empty or null baseApplicationId
return newPageService
.findByBranchNameAndBasePageIdAndApplicationMode(branchName, basePageId, mode)
.flatMap(branchedPage ->
// Use the application ID to find the complete application details.
applicationService
.findByBranchedApplicationIdAndApplicationMode(
branchedPage.getApplicationId(), mode)
.flatMap(application -> {
if (isViewMode) {
// Update the cache with the new application’s base ID for future
// queries.
return cacheableRepositoryHelper
.fetchBaseApplicationId(basePageId, application.getBaseId())
.thenReturn(application)
.name(getQualifiedSpanName(
APPLICATION_ID_UPDATE_REDIS_SPAN, mode))
.tap(Micrometer.observation(observationRegistry));
}
return Mono.just(application);
}));
return branchedPageMonoCached.flatMap(branchedPage ->
// Use the application ID to find the complete application details.
applicationService
.findByBranchedApplicationIdAndApplicationMode(branchedPage.getApplicationId(), mode)
.flatMap(application -> {
if (isViewMode) {
// Update the cache with the new application’s base ID for future
// queries.
return cacheableRepositoryHelper
.fetchBaseApplicationId(basePageId, application.getBaseId())
.thenReturn(application)
.name(getQualifiedSpanName(APPLICATION_ID_UPDATE_REDIS_SPAN, mode))
.tap(Micrometer.observation(observationRegistry));
}
return Mono.just(application);
}));
} else {
// Handle non-empty baseApplicationId
return applicationService.findByBaseIdBranchNameAndApplicationMode(
Expand Down

0 comments on commit 1c8712a

Please sign in to comment.