From af9a625de92e272b0e86f4b8a34a37969b831e2f Mon Sep 17 00:00:00 2001 From: Dipyaman Biswas Date: Thu, 12 Oct 2023 23:02:38 +0530 Subject: [PATCH] feat: add feature flags to workers. (#27258) ## Description Adding feature flags to Lint and Eval workers which will be used to compute the feature in EE #### PR fixes following issue(s) Fixes #https://github.com/appsmithorg/appsmith-ee/issues/1905 #### Type of change > Please delete options that are not relevant. - New feature (non-breaking change which adds functionality) - Breaking change (fix or feature that would cause existing functionality to not work as expected) > > > ## 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/ce/workers/Evaluation/Actions.ts | 4 ++-- app/client/src/plugins/Linting/Linter.ts | 5 +++++ .../src/plugins/Linting/handlers/index.ts | 2 ++ .../Linting/handlers/setupLinkingWorkerEnv.ts | 6 +++++ .../src/plugins/Linting/linters/index.ts | 6 +++++ app/client/src/plugins/Linting/types.ts | 1 + .../Linting/utils/getEvaluationContext.ts | 2 +- .../src/sagas/ActionExecution/errorUtils.ts | 7 +----- app/client/src/sagas/EvaluationsSaga.ts | 13 +++++++++++ app/client/src/sagas/LintingSagas.ts | 7 ++++++ .../src/workers/Evaluation/fns/index.ts | 10 ++------- .../Evaluation/handlers/setupEvalEnv.ts | 8 +++---- .../workers/Evaluation/handlers/workerEnv.ts | 22 +++++++++++++++++++ 13 files changed, 72 insertions(+), 21 deletions(-) create mode 100644 app/client/src/plugins/Linting/handlers/setupLinkingWorkerEnv.ts create mode 100644 app/client/src/workers/Evaluation/handlers/workerEnv.ts diff --git a/app/client/src/ce/workers/Evaluation/Actions.ts b/app/client/src/ce/workers/Evaluation/Actions.ts index fcfe8a4a87..3dc9b0d46b 100644 --- a/app/client/src/ce/workers/Evaluation/Actions.ts +++ b/app/client/src/ce/workers/Evaluation/Actions.ts @@ -113,7 +113,7 @@ export const addDataTreeToContext = (args: { }; export const addPlatformFunctionsToEvalContext = (context: any) => { - for (const fnDef of getPlatformFunctions(self.$cloudHosting)) { + for (const fnDef of getPlatformFunctions()) { addFn(context, fnDef.name, fnDef.fn.bind(context)); } }; @@ -174,7 +174,7 @@ export const getAllAsyncFunctions = ( } const setterMethods = getAllSetterFunctions(dataTree, configTree); allAsyncFunctions = { ...allAsyncFunctions, ...setterMethods }; - for (const platformFn of getPlatformFunctions(self.$cloudHosting)) { + for (const platformFn of getPlatformFunctions()) { allAsyncFunctions[platformFn.name] = true; } return allAsyncFunctions; diff --git a/app/client/src/plugins/Linting/Linter.ts b/app/client/src/plugins/Linting/Linter.ts index ed343339a3..87eba9714e 100644 --- a/app/client/src/plugins/Linting/Linter.ts +++ b/app/client/src/plugins/Linting/Linter.ts @@ -1,3 +1,4 @@ +import type { FeatureFlags } from "@appsmith/entities/FeatureFlag"; import type { ILinter } from "./linters"; import { WorkerLinter } from "./linters"; import type { LintTreeRequestPayload, updateJSLibraryProps } from "./types"; @@ -10,6 +11,7 @@ export class Linter { this.updateJSLibraryGlobals = this.updateJSLibraryGlobals.bind(this); this.start = this.start.bind(this); this.shutdown = this.shutdown.bind(this); + this.setup = this.setup.bind(this); } *lintTree(data: LintTreeRequestPayload) { return yield* this.linter.lintTree(data); @@ -23,4 +25,7 @@ export class Linter { *shutdown() { yield this.linter.shutdown(); } + *setup(featureFlags: FeatureFlags) { + yield this.linter.setup(featureFlags); + } } diff --git a/app/client/src/plugins/Linting/handlers/index.ts b/app/client/src/plugins/Linting/handlers/index.ts index 3db9823e8b..286a264c39 100644 --- a/app/client/src/plugins/Linting/handlers/index.ts +++ b/app/client/src/plugins/Linting/handlers/index.ts @@ -1,8 +1,10 @@ import { LINT_WORKER_ACTIONS } from "plugins/Linting/types"; import { updateJSLibraryGlobals } from "./updateJSLibraryGlobals"; import { lintService } from "./lintService"; +import { setupLintingWorkerEnv } from "./setupLinkingWorkerEnv"; export const handlerMap = { [LINT_WORKER_ACTIONS.LINT_TREE]: lintService.lintTree, [LINT_WORKER_ACTIONS.UPDATE_LINT_GLOBALS]: updateJSLibraryGlobals, + [LINT_WORKER_ACTIONS.SETUP]: setupLintingWorkerEnv, } as const; diff --git a/app/client/src/plugins/Linting/handlers/setupLinkingWorkerEnv.ts b/app/client/src/plugins/Linting/handlers/setupLinkingWorkerEnv.ts new file mode 100644 index 0000000000..89afebc362 --- /dev/null +++ b/app/client/src/plugins/Linting/handlers/setupLinkingWorkerEnv.ts @@ -0,0 +1,6 @@ +import type { FeatureFlags } from "@appsmith/entities/FeatureFlag"; +import { WorkerEnv } from "workers/Evaluation/handlers/workerEnv"; + +export const setupLintingWorkerEnv = (featureFlags: FeatureFlags) => { + WorkerEnv.setFeatureFlags(featureFlags); +}; diff --git a/app/client/src/plugins/Linting/linters/index.ts b/app/client/src/plugins/Linting/linters/index.ts index 06f32b1dd1..aca0f73499 100644 --- a/app/client/src/plugins/Linting/linters/index.ts +++ b/app/client/src/plugins/Linting/linters/index.ts @@ -4,12 +4,14 @@ import type { updateJSLibraryProps, } from "plugins/Linting/types"; import { LINT_WORKER_ACTIONS as LINT_ACTIONS } from "plugins/Linting/types"; +import type { FeatureFlags } from "@appsmith/entities/FeatureFlag"; export interface ILinter { lintTree(args: LintTreeRequestPayload): any; updateJSLibraryGlobals(args: updateJSLibraryProps): any; start(): void; shutdown(): void; + setup(args: FeatureFlags): any; } export class WorkerLinter implements ILinter { @@ -23,6 +25,7 @@ export class WorkerLinter implements ILinter { name: "lintWorker", }), ); + this.setup = this.setup.bind(this); this.start = this.start.bind(this); this.shutdown = this.shutdown.bind(this); this.lintTree = this.lintTree.bind(this); @@ -40,4 +43,7 @@ export class WorkerLinter implements ILinter { *updateJSLibraryGlobals(args: updateJSLibraryProps) { return yield* this.server.request(LINT_ACTIONS.UPDATE_LINT_GLOBALS, args); } + *setup(args: FeatureFlags) { + return yield* this.server.request(LINT_ACTIONS.SETUP, args); + } } diff --git a/app/client/src/plugins/Linting/types.ts b/app/client/src/plugins/Linting/types.ts index 897bd48104..b3cfa00d0b 100644 --- a/app/client/src/plugins/Linting/types.ts +++ b/app/client/src/plugins/Linting/types.ts @@ -15,6 +15,7 @@ import type { TJSLibrary } from "workers/common/JSLibrary"; export enum LINT_WORKER_ACTIONS { LINT_TREE = "LINT_TREE", UPDATE_LINT_GLOBALS = "UPDATE_LINT_GLOBALS", + SETUP = "SETUP", } export interface LintTreeResponse { errors: LintErrorsStore; diff --git a/app/client/src/plugins/Linting/utils/getEvaluationContext.ts b/app/client/src/plugins/Linting/utils/getEvaluationContext.ts index 84f04b6e6d..b90bd55c51 100644 --- a/app/client/src/plugins/Linting/utils/getEvaluationContext.ts +++ b/app/client/src/plugins/Linting/utils/getEvaluationContext.ts @@ -24,7 +24,7 @@ export function getEvaluationContext( }); const platformFnNamesMap = Object.values( - getActionTriggerFunctionNames(cloudHosting), + getActionTriggerFunctionNames(), ).reduce( (acc, name) => ({ ...acc, [name]: true }), {} as { [x: string]: boolean }, diff --git a/app/client/src/sagas/ActionExecution/errorUtils.ts b/app/client/src/sagas/ActionExecution/errorUtils.ts index 0b242bff8a..20315cd414 100644 --- a/app/client/src/sagas/ActionExecution/errorUtils.ts +++ b/app/client/src/sagas/ActionExecution/errorUtils.ts @@ -7,7 +7,6 @@ import { isString } from "lodash"; import type { Types } from "utils/TypeHelpers"; import type { ActionTriggerKeys } from "@appsmith/workers/Evaluation/fns/index"; import { getActionTriggerFunctionNames } from "@appsmith/workers/Evaluation/fns/index"; -import { getAppsmithConfigs } from "@appsmith/configs"; import { getAppMode } from "@appsmith/selectors/applicationSelectors"; import AnalyticsUtil from "../../utils/AnalyticsUtil"; import { @@ -19,8 +18,6 @@ import store from "store"; import showToast from "sagas/ToastSagas"; import { call } from "redux-saga/effects"; -const APPSMITH_CONFIGS = getAppsmithConfigs(); - /* * The base trigger error that also logs the errors in the debugger. * Extend this error to make custom handling of errors @@ -52,9 +49,7 @@ export class ActionValidationError extends TriggerFailureError { ) { const errorMessage = createMessage( TRIGGER_ACTION_VALIDATION_ERROR, - getActionTriggerFunctionNames(!!APPSMITH_CONFIGS.cloudHosting)[ - functionName - ], + getActionTriggerFunctionNames()[functionName], argumentName, expectedType, received, diff --git a/app/client/src/sagas/EvaluationsSaga.ts b/app/client/src/sagas/EvaluationsSaga.ts index 93de0767b8..a5299d80c3 100644 --- a/app/client/src/sagas/EvaluationsSaga.ts +++ b/app/client/src/sagas/EvaluationsSaga.ts @@ -96,7 +96,10 @@ import { executeJSUpdates } from "actions/pluginActionActions"; import { setEvaluatedActionSelectorField } from "actions/actionSelectorActions"; import { waitForWidgetConfigBuild } from "./InitSagas"; import { logDynamicTriggerExecution } from "@appsmith/sagas/analyticsSaga"; +import { selectFeatureFlags } from "@appsmith/selectors/featureFlagsSelectors"; +import { fetchFeatureFlagsInit } from "actions/userActions"; import { parseUpdatesAndDeleteUndefinedUpdates } from "./EvaluationSaga.utils"; +import { getFeatureFlagsFetched } from "selectors/usersSelectors"; const APPSMITH_CONFIGS = getAppsmithConfigs(); export const evalWorker = new GracefulWorkerService( new Worker( @@ -579,8 +582,18 @@ function* evaluationChangeListenerSaga(): any { call(lintWorker.start), ]); + const isFFFetched = yield select(getFeatureFlagsFetched); + if (!isFFFetched) { + yield call(fetchFeatureFlagsInit); + yield take(ReduxActionTypes.FETCH_FEATURE_FLAGS_SUCCESS); + } + + const featureFlags: Record = + yield select(selectFeatureFlags); + yield call(evalWorker.request, EVAL_WORKER_ACTIONS.SETUP, { cloudHosting: !!APPSMITH_CONFIGS.cloudHosting, + featureFlags: featureFlags, }); yield spawn(handleEvalWorkerRequestSaga, evalWorkerListenerChannel); diff --git a/app/client/src/sagas/LintingSagas.ts b/app/client/src/sagas/LintingSagas.ts index 56f31135ca..ea0c96edbd 100644 --- a/app/client/src/sagas/LintingSagas.ts +++ b/app/client/src/sagas/LintingSagas.ts @@ -22,6 +22,7 @@ import { getEntityNameAndPropertyPath } from "@appsmith/workers/Evaluation/evalu import { Linter } from "plugins/Linting/Linter"; import log from "loglevel"; import { getFixedTimeDifference } from "workers/common/DataTreeEvaluator/utils"; +import { selectFeatureFlags } from "@appsmith/selectors/featureFlagsSelectors"; const APPSMITH_CONFIGS = getAppsmithConfigs(); @@ -124,4 +125,10 @@ export function* initiateLinting( export default function* lintTreeSagaWatcher() { yield takeEvery(ReduxActionTypes.UPDATE_LINT_GLOBALS, updateLintGlobals); + yield takeEvery(ReduxActionTypes.START_EVALUATION, setupSaga); +} + +export function* setupSaga(): any { + const featureFlags = yield select(selectFeatureFlags); + yield call(lintWorker.setup, featureFlags); } diff --git a/app/client/src/workers/Evaluation/fns/index.ts b/app/client/src/workers/Evaluation/fns/index.ts index b8d9e37fc0..c0d56d0ff7 100644 --- a/app/client/src/workers/Evaluation/fns/index.ts +++ b/app/client/src/workers/Evaluation/fns/index.ts @@ -63,9 +63,7 @@ import { } from "./geolocationFns"; import { getFnWithGuards, isAsyncGuard } from "./utils/fnGuard"; -// cloudHosting -> to use in EE -// eslint-disable-next-line @typescript-eslint/no-unused-vars -export const getPlatformFunctions = (cloudHosting: boolean) => { +export const getPlatformFunctions = () => { return platformFns; }; @@ -187,11 +185,7 @@ export type ActionTriggerKeys = | TWatchGeoLocationActionType | TStopWatchGeoLocationActionType; -export const getActionTriggerFunctionNames = ( - // cloudHosting -> to use in ee - // eslint-disable-next-line @typescript-eslint/no-unused-vars - cloudHosting: boolean, -): Record => { +export const getActionTriggerFunctionNames = (): Record => { return ActionTriggerFunctionNames; }; diff --git a/app/client/src/workers/Evaluation/handlers/setupEvalEnv.ts b/app/client/src/workers/Evaluation/handlers/setupEvalEnv.ts index 0a9e7b5b65..fed0addbbe 100644 --- a/app/client/src/workers/Evaluation/handlers/setupEvalEnv.ts +++ b/app/client/src/workers/Evaluation/handlers/setupEvalEnv.ts @@ -3,6 +3,7 @@ import setupDOM from "../SetupDOM"; import type { EvalWorkerSyncRequest } from "../types"; import { addPlatformFunctionsToEvalContext } from "@appsmith/workers/Evaluation/Actions"; import { overrideWebAPIs } from "../fns/overrides"; +import { WorkerEnv } from "./workerEnv"; export default function (request: EvalWorkerSyncRequest) { self.$isDataField = false; @@ -13,10 +14,9 @@ export default function (request: EvalWorkerSyncRequest) { }); setupDOM(); overrideWebAPIs(self); - Object.defineProperty(self, "$cloudHosting", { - value: request.data.cloudHosting, - enumerable: false, - }); + + WorkerEnv.setFeatureFlags(request.data.featureFlags); + WorkerEnv.setCloudHosting(request.data.cloudHosting); addPlatformFunctionsToEvalContext(self); return true; } diff --git a/app/client/src/workers/Evaluation/handlers/workerEnv.ts b/app/client/src/workers/Evaluation/handlers/workerEnv.ts new file mode 100644 index 0000000000..3f0be4ba9f --- /dev/null +++ b/app/client/src/workers/Evaluation/handlers/workerEnv.ts @@ -0,0 +1,22 @@ +import type { FeatureFlags } from "@appsmith/entities/FeatureFlag"; + +export class WorkerEnv { + static flags: any; + static cloudHosting: boolean; + + static setFeatureFlags(featureFlags: FeatureFlags) { + WorkerEnv.flags = featureFlags; + } + + static setCloudHosting(cloudHosting: boolean) { + WorkerEnv.cloudHosting = cloudHosting; + } + + static getFeatureFlags() { + return WorkerEnv.flags; + } + + static getCloudHosting() { + return WorkerEnv.cloudHosting; + } +}