From e827042c696af07aad333746766ebd49f793a350 Mon Sep 17 00:00:00 2001 From: Rudraprasad Das Date: Wed, 29 Nov 2023 14:18:50 +0530 Subject: [PATCH] fix: fixing non blocking calls (#29170) ## Description Git-connected apps are taking substantially more time to load compared to normal apps The primary reason was due to long-running blocking API calls for `git status` and `git fetch`. This PR makes the blocking API calls run in the background instead #### PR fixes following issue(s) Fixes #29169 #### Media Previously ![image](https://github.com/appsmithorg/appsmith/assets/8724051/b5bd08eb-ea95-4002-9ff3-4c6815e1c0ef) Now ![image](https://github.com/appsmithorg/appsmith/assets/8724051/53e70d61-2773-40ab-962c-bb145eb113a1) #### Type of change - Bug fix (non-breaking change which fixes an issue) > > > ## Testing > #### How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Also list any relevant details for your test configuration. > Delete anything that is not relevant - [x] Manual - [ ] JUnit - [ ] Jest - [x] Cypress > > #### 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 - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed --- .../src/entities/Engine/AppEditorEngine.ts | 31 +++++++------ app/client/src/sagas/GitSyncSagas.ts | 43 +++++++++++++------ 2 files changed, 46 insertions(+), 28 deletions(-) diff --git a/app/client/src/entities/Engine/AppEditorEngine.ts b/app/client/src/entities/Engine/AppEditorEngine.ts index 803af3e2bc..bbd2d65653 100644 --- a/app/client/src/entities/Engine/AppEditorEngine.ts +++ b/app/client/src/entities/Engine/AppEditorEngine.ts @@ -1,11 +1,11 @@ import { fetchMockDatasources } from "actions/datasourceActions"; import { fetchGitRemoteStatusInit, - fetchBranchesInit, fetchGitProtectedBranchesInit, fetchGitStatusInit, remoteUrlInputValue, resetPullMergeStatus, + fetchBranchesInit, } from "actions/gitSyncActions"; import { restoreRecentEntitiesRequest } from "actions/globalSearchActions"; import { resetEditorSuccess } from "actions/initActions"; @@ -26,7 +26,7 @@ import { } from "@appsmith/constants/ReduxActionConstants"; import { addBranchParam } from "constants/routes"; import type { APP_MODE } from "entities/App"; -import { call, put, select, spawn, take } from "redux-saga/effects"; +import { call, fork, put, select, spawn } from "redux-saga/effects"; import { failFastApiCalls, reportSWStatus, @@ -263,9 +263,6 @@ export default class AppEditorEngine extends AppEngine { public *loadGit(applicationId: string) { const branchInStore: string = yield select(getCurrentGitBranch); - const isGitStatusLiteEnabled: boolean = yield select( - getIsGitStatusLiteEnabled, - ); yield put( restoreRecentEntitiesRequest({ applicationId, @@ -277,17 +274,23 @@ export default class AppEditorEngine extends AppEngine { // add branch query to path and fetch status if (branchInStore) { history.replace(addBranchParam(branchInStore)); + yield fork(this.loadGitInBackground); + } + } - if (isGitStatusLiteEnabled) { - yield put(fetchGitRemoteStatusInit()); - yield put(fetchGitStatusInit({ compareRemote: false })); - } else { - yield put(fetchGitStatusInit({ compareRemote: true })); - } + private *loadGitInBackground() { + const isGitStatusLiteEnabled: boolean = yield select( + getIsGitStatusLiteEnabled, + ); - yield put(fetchBranchesInit()); - yield take(ReduxActionTypes.FETCH_BRANCHES_SUCCESS); - yield put(fetchGitProtectedBranchesInit()); + yield put(fetchBranchesInit()); + yield put(fetchGitProtectedBranchesInit()); + + if (isGitStatusLiteEnabled) { + yield put(fetchGitRemoteStatusInit()); + yield put(fetchGitStatusInit({ compareRemote: false })); + } else { + yield put(fetchGitStatusInit({ compareRemote: true })); } yield put(resetPullMergeStatus()); } diff --git a/app/client/src/sagas/GitSyncSagas.ts b/app/client/src/sagas/GitSyncSagas.ts index df2e59308d..f68b073130 100644 --- a/app/client/src/sagas/GitSyncSagas.ts +++ b/app/client/src/sagas/GitSyncSagas.ts @@ -15,6 +15,7 @@ import { put, select, take, + takeLatest, } from "redux-saga/effects"; import type { TakeableChannel } from "@redux-saga/core"; import type { MergeBranchPayload, MergeStatusPayload } from "api/GitSyncAPI"; @@ -1144,38 +1145,44 @@ function* updateGitProtectedBranchesSaga({ } } -const gitRequestActions: Record< +const gitRequestBlockingActions: Record< (typeof ReduxActionTypes)[keyof typeof ReduxActionTypes], (...args: any[]) => any > = { [ReduxActionTypes.COMMIT_TO_GIT_REPO_INIT]: commitToGitRepoSaga, [ReduxActionTypes.CONNECT_TO_GIT_INIT]: connectToGitSaga, - [ReduxActionTypes.FETCH_GLOBAL_GIT_CONFIG_INIT]: fetchGlobalGitConfig, [ReduxActionTypes.UPDATE_GLOBAL_GIT_CONFIG_INIT]: updateGlobalGitConfig, - [ReduxActionTypes.SWITCH_GIT_BRANCH_INIT]: switchBranch, [ReduxActionTypes.FETCH_BRANCHES_INIT]: fetchBranches, + [ReduxActionTypes.SWITCH_GIT_BRANCH_INIT]: switchBranch, [ReduxActionTypes.CREATE_NEW_BRANCH_INIT]: createNewBranch, - [ReduxActionTypes.FETCH_LOCAL_GIT_CONFIG_INIT]: fetchLocalGitConfig, [ReduxActionTypes.UPDATE_LOCAL_GIT_CONFIG_INIT]: updateLocalGitConfig, - [ReduxActionTypes.FETCH_GIT_STATUS_INIT]: fetchGitStatusSaga, - [ReduxActionTypes.FETCH_GIT_REMOTE_STATUS_INIT]: fetchGitRemoteStatusSaga, [ReduxActionTypes.MERGE_BRANCH_INIT]: mergeBranchSaga, [ReduxActionTypes.FETCH_MERGE_STATUS_INIT]: fetchMergeStatusSaga, [ReduxActionTypes.GIT_PULL_INIT]: gitPullSaga, - [ReduxActionTypes.SHOW_CONNECT_GIT_MODAL]: showConnectGitModal, [ReduxActionTypes.REVOKE_GIT]: disconnectGitSaga, [ReduxActionTypes.IMPORT_APPLICATION_FROM_GIT_INIT]: importAppFromGitSaga, [ReduxActionTypes.GENERATE_SSH_KEY_PAIR_INIT]: generateSSHKeyPairSaga, - [ReduxActionTypes.FETCH_SSH_KEY_PAIR_INIT]: getSSHKeyPairSaga, [ReduxActionTypes.DELETE_BRANCH_INIT]: deleteBranch, [ReduxActionTypes.GIT_DISCARD_CHANGES]: discardChanges, [ReduxActionTypes.GIT_UPDATE_DEFAULT_BRANCH_INIT]: updateGitDefaultBranchSaga, - [ReduxActionTypes.GIT_FETCH_PROTECTED_BRANCHES_INIT]: - fetchGitProtectedBranchesSaga, [ReduxActionTypes.GIT_UPDATE_PROTECTED_BRANCHES_INIT]: updateGitProtectedBranchesSaga, }; +const gitRequestNonBlockingActions: Record< + (typeof ReduxActionTypes)[keyof typeof ReduxActionTypes], + (...args: any[]) => any +> = { + [ReduxActionTypes.FETCH_GLOBAL_GIT_CONFIG_INIT]: fetchGlobalGitConfig, + [ReduxActionTypes.FETCH_LOCAL_GIT_CONFIG_INIT]: fetchLocalGitConfig, + [ReduxActionTypes.FETCH_GIT_STATUS_INIT]: fetchGitStatusSaga, + [ReduxActionTypes.FETCH_GIT_REMOTE_STATUS_INIT]: fetchGitRemoteStatusSaga, + [ReduxActionTypes.SHOW_CONNECT_GIT_MODAL]: showConnectGitModal, + [ReduxActionTypes.FETCH_SSH_KEY_PAIR_INIT]: getSSHKeyPairSaga, + [ReduxActionTypes.GIT_FETCH_PROTECTED_BRANCHES_INIT]: + fetchGitProtectedBranchesSaga, +}; + /** * All git actions on the server are behind a lock, * that means that only one action can be performed at once. @@ -1185,18 +1192,26 @@ const gitRequestActions: Record< * * This will ensure that client is not running parallel requests to the server for git * */ -function* watchGitRequests() { +function* watchGitBlockingRequests() { const gitActionChannel: TakeableChannel = yield actionChannel( - Object.keys(gitRequestActions), + Object.keys(gitRequestBlockingActions), ); while (true) { const { type, ...args }: ReduxAction = yield take(gitActionChannel); - yield call(gitRequestActions[type], { type, ...args }); + yield call(gitRequestBlockingActions[type], { type, ...args }); + } +} + +function* watchGitNonBlockingRequests() { + const keys = Object.keys(gitRequestNonBlockingActions); + for (const actionType of keys) { + yield takeLatest(actionType, gitRequestNonBlockingActions[actionType]); } } export default function* gitSyncSagas() { - yield fork(watchGitRequests); + yield fork(watchGitNonBlockingRequests); + yield fork(watchGitBlockingRequests); }