From ce066a57b67a20f1615c0261b61bb8f9062321ca Mon Sep 17 00:00:00 2001 From: Favour Ohanekwu Date: Thu, 27 Jan 2022 02:01:24 -0800 Subject: [PATCH 1/8] fix: Fix crash on broken list widget (#10670) (cherry picked from commit db3e29e15dfc16cfb4d12b9f93876399c27957ac) --- app/client/src/widgets/ListWidget/widget/index.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/app/client/src/widgets/ListWidget/widget/index.tsx b/app/client/src/widgets/ListWidget/widget/index.tsx index df49c17535..0367389075 100644 --- a/app/client/src/widgets/ListWidget/widget/index.tsx +++ b/app/client/src/widgets/ListWidget/widget/index.tsx @@ -131,6 +131,7 @@ class ListWidget extends BaseWidget, WidgetState> { props, PATH_TO_ALL_WIDGETS_IN_LIST_WIDGET, ); + if (!listWidgetChildren) return; listWidgetChildren.map((child) => { privateWidgets[child.widgetName] = true; }); From 105bdfb3c7c291fc098fdca34328796990a4fae2 Mon Sep 17 00:00:00 2001 From: Anand Srinivasan <66776129+eco-monk@users.noreply.github.com> Date: Fri, 4 Feb 2022 17:58:46 +0530 Subject: [PATCH 2/8] fix: passing params from JS to API/SQL query (#10826) * Revert "fix: revert this.params solution (#10322)" This reverts commit 2bcd73e41deb1ac3f812f85b0ebaecc62a010d6b. * replace 'this.params' with 'executionParams' * replace 'this?.params' also with 'executionParams' * fix unit test lint errors * added unit tests for params handling * evaluateActionBindings unit test - add default value case * comments update * remove un-necessary `executionparams` assigment to `evalTree` --- .../ActionConstants.tsx | 3 +- .../src/workers/DataTreeEvaluator.test.ts | 109 ++++++++++++++++++ app/client/src/workers/DataTreeEvaluator.ts | 54 ++++++--- app/client/src/workers/evaluate.test.ts | 39 +++++-- app/client/src/workers/evaluate.ts | 25 +++- app/client/src/workers/validations.ts | 2 +- 6 files changed, 199 insertions(+), 33 deletions(-) create mode 100644 app/client/src/workers/DataTreeEvaluator.test.ts diff --git a/app/client/src/constants/AppsmithActionConstants/ActionConstants.tsx b/app/client/src/constants/AppsmithActionConstants/ActionConstants.tsx index f598f1ee3f..4f6d93df8c 100644 --- a/app/client/src/constants/AppsmithActionConstants/ActionConstants.tsx +++ b/app/client/src/constants/AppsmithActionConstants/ActionConstants.tsx @@ -118,7 +118,8 @@ export interface ExecuteErrorPayload extends ErrorActionPayload { export const urlGroupsRegexExp = /^(https?:\/{2}\S+?)(\/[\s\S]*?)?(\?(?![^{]*})[\s\S]*)?$/; export const EXECUTION_PARAM_KEY = "executionParams"; -export const EXECUTION_PARAM_REFERENCE_REGEX = /this.params/g; +export const EXECUTION_PARAM_REFERENCE_REGEX = /this.params|this\?.params/g; +export const THIS_DOT_PARAMS_KEY = "params"; export const RESP_HEADER_DATATYPE = "X-APPSMITH-DATATYPE"; export const API_REQUEST_HEADERS: APIHeaders = { diff --git a/app/client/src/workers/DataTreeEvaluator.test.ts b/app/client/src/workers/DataTreeEvaluator.test.ts new file mode 100644 index 0000000000..77c158e0cf --- /dev/null +++ b/app/client/src/workers/DataTreeEvaluator.test.ts @@ -0,0 +1,109 @@ +import DataTreeEvaluator from "./DataTreeEvaluator"; + +describe("DataTreeEvaluator", () => { + let dataTreeEvaluator: DataTreeEvaluator; + beforeAll(() => { + dataTreeEvaluator = new DataTreeEvaluator({}); + }); + describe("evaluateActionBindings", () => { + it("handles this.params.property", () => { + const result = dataTreeEvaluator.evaluateActionBindings( + [ + "(function() { return this.params.property })()", + "(() => { return this.params.property })()", + 'this.params.property || "default value"', + 'this.params.property1 || "default value"', + ], + { + property: "my value", + }, + ); + expect(result).toStrictEqual([ + "my value", + "my value", + "my value", + "default value", + ]); + }); + + it("handles this?.params.property", () => { + const result = dataTreeEvaluator.evaluateActionBindings( + [ + "(() => { return this?.params.property })()", + "(function() { return this?.params.property })()", + 'this?.params.property || "default value"', + 'this?.params.property1 || "default value"', + ], + { + property: "my value", + }, + ); + expect(result).toStrictEqual([ + "my value", + "my value", + "my value", + "default value", + ]); + }); + + it("handles this?.params?.property", () => { + const result = dataTreeEvaluator.evaluateActionBindings( + [ + "(() => { return this?.params?.property })()", + "(function() { return this?.params?.property })()", + 'this?.params?.property || "default value"', + 'this?.params?.property1 || "default value"', + ], + { + property: "my value", + }, + ); + expect(result).toStrictEqual([ + "my value", + "my value", + "my value", + "default value", + ]); + }); + + it("handles executionParams.property", () => { + const result = dataTreeEvaluator.evaluateActionBindings( + [ + "(function() { return executionParams.property })()", + "(() => { return executionParams.property })()", + 'executionParams.property || "default value"', + 'executionParams.property1 || "default value"', + ], + { + property: "my value", + }, + ); + expect(result).toStrictEqual([ + "my value", + "my value", + "my value", + "default value", + ]); + }); + + it("handles executionParams?.property", () => { + const result = dataTreeEvaluator.evaluateActionBindings( + [ + "(function() { return executionParams?.property })()", + "(() => { return executionParams?.property })()", + 'executionParams?.property || "default value"', + 'executionParams?.property1 || "default value"', + ], + { + property: "my value", + }, + ); + expect(result).toStrictEqual([ + "my value", + "my value", + "my value", + "default value", + ]); + }); + }); +}); diff --git a/app/client/src/workers/DataTreeEvaluator.ts b/app/client/src/workers/DataTreeEvaluator.ts index 51313b71c5..22a57ce6d8 100644 --- a/app/client/src/workers/DataTreeEvaluator.ts +++ b/app/client/src/workers/DataTreeEvaluator.ts @@ -55,11 +55,13 @@ import toposort from "toposort"; import { EXECUTION_PARAM_KEY, EXECUTION_PARAM_REFERENCE_REGEX, + THIS_DOT_PARAMS_KEY, } from "constants/AppsmithActionConstants/ActionConstants"; import { DATA_BIND_REGEX } from "constants/BindingsConstants"; import evaluateSync, { createGlobalData, EvalResult, + EvaluateContext, EvaluationScriptType, getScriptToEval, evaluateAsync, @@ -609,12 +611,19 @@ export default class DataTreeEvaluator { entity.bindingPaths[propertyPath] || EvaluationSubstitutionType.TEMPLATE; + const contextData: EvaluateContext = {}; + if (isAction(entity)) { + contextData.thisContext = { + params: {}, + }; + } try { evalPropertyValue = this.getDynamicValue( unEvalPropertyValue, currentTree, resolvedFunctions, evaluationSubstitutionType, + contextData, undefined, fullPropertyPath, ); @@ -764,6 +773,7 @@ export default class DataTreeEvaluator { data: DataTree, resolvedFunctions: Record, evaluationSubstitutionType: EvaluationSubstitutionType, + contextData?: EvaluateContext, callBackData?: Array, fullPropertyPath?: string, ) { @@ -792,6 +802,7 @@ export default class DataTreeEvaluator { toBeSentForEval, data, resolvedFunctions, + contextData, callBackData, ); if (fullPropertyPath && result.errors.length) { @@ -864,10 +875,17 @@ export default class DataTreeEvaluator { js: string, data: DataTree, resolvedFunctions: Record, + contextData?: EvaluateContext, callbackData?: Array, ): EvalResult { try { - return evaluateSync(js, data, resolvedFunctions, callbackData); + return evaluateSync( + js, + data, + resolvedFunctions, + contextData, + callbackData, + ); } catch (e) { return { result: undefined, @@ -1555,24 +1573,26 @@ export default class DataTreeEvaluator { ); } - // Replace any reference of 'this.params' to 'executionParams' (backwards compatibility) - const bindingsForExecutionParams: string[] = bindings.map( - (binding: string) => - binding.replace(EXECUTION_PARAM_REFERENCE_REGEX, EXECUTION_PARAM_KEY), - ); - - const dataTreeWithExecutionParams = Object.assign({}, this.evalTree, { - [EXECUTION_PARAM_KEY]: evaluatedExecutionParams, - }); - - return bindingsForExecutionParams.map((binding) => - this.getDynamicValue( - `{{${binding}}}`, - dataTreeWithExecutionParams, + return bindings.map((binding) => { + // Replace any reference of 'this.params' to 'executionParams' (backwards compatibility) + // also helps with dealing with IIFE which are normal functions (not arrow) + // because normal functions won't retain 'this' context (when executed elsewhere) + const replacedBinding = binding.replace( + EXECUTION_PARAM_REFERENCE_REGEX, + EXECUTION_PARAM_KEY, + ); + return this.getDynamicValue( + `{{${replacedBinding}}}`, + this.evalTree, this.resolvedFunctions, EvaluationSubstitutionType.TEMPLATE, - ), - ); + // params can be accessed via "this.params" or "executionParams" + { + thisContext: { [THIS_DOT_PARAMS_KEY]: evaluatedExecutionParams }, + globalContext: { [EXECUTION_PARAM_KEY]: evaluatedExecutionParams }, + }, + ); + }); } clearErrors() { diff --git a/app/client/src/workers/evaluate.test.ts b/app/client/src/workers/evaluate.test.ts index a34cbcf116..3e1963b772 100644 --- a/app/client/src/workers/evaluate.test.ts +++ b/app/client/src/workers/evaluate.test.ts @@ -30,6 +30,9 @@ describe("evaluateSync", () => { triggerPaths: {}, validationPaths: {}, logBlackList: {}, + overridingPropertyPaths: {}, + privateWidgets: {}, + propertyOverrideDependency: {}, }; const dataTree: DataTree = { Input1: widget, @@ -75,7 +78,7 @@ describe("evaluateSync", () => { const result = wrongJS return result; } - closedFunction() + closedFunction.call(THIS_CONTEXT) `, severity: "error", originalBinding: "wrongJS", @@ -89,7 +92,7 @@ describe("evaluateSync", () => { const result = wrongJS return result; } - closedFunction() + closedFunction.call(THIS_CONTEXT) `, severity: "error", originalBinding: "wrongJS", @@ -108,7 +111,7 @@ describe("evaluateSync", () => { const result = {}.map() return result; } - closedFunction() + closedFunction.call(THIS_CONTEXT) `, severity: "error", originalBinding: "{}.map()", @@ -135,7 +138,7 @@ describe("evaluateSync", () => { const result = setTimeout(() => {}, 100) return result; } - closedFunction() + closedFunction.call(THIS_CONTEXT) `, severity: "error", originalBinding: "setTimeout(() => {}, 100)", @@ -151,7 +154,7 @@ describe("evaluateSync", () => { it("evaluates functions with callback data", () => { const js = "(arg1, arg2) => arg1.value + arg2"; const callbackData = [{ value: "test" }, "1"]; - const response = evaluate(js, dataTree, {}, callbackData); + const response = evaluate(js, dataTree, {}, {}, callbackData); expect(response.result).toBe("test1"); }); it("handles EXPRESSIONS with new lines", () => { @@ -165,22 +168,38 @@ describe("evaluateSync", () => { }); it("handles TRIGGERS with new lines", () => { let js = "\n"; - let response = evaluate(js, dataTree, {}, undefined); + let response = evaluate(js, dataTree, {}, undefined, undefined); expect(response.errors.length).toBe(0); js = "\n\n\n"; - response = evaluate(js, dataTree, {}, undefined); + response = evaluate(js, dataTree, {}, undefined, undefined); expect(response.errors.length).toBe(0); }); it("handles ANONYMOUS_FUNCTION with new lines", () => { let js = "\n"; - let response = evaluate(js, dataTree, {}, undefined); + let response = evaluate(js, dataTree, {}, undefined, undefined); expect(response.errors.length).toBe(0); js = "\n\n\n"; - response = evaluate(js, dataTree, {}, undefined); + response = evaluate(js, dataTree, {}, undefined, undefined); expect(response.errors.length).toBe(0); }); + it("has access to this context", () => { + const js = "this.contextVariable"; + const thisContext = { contextVariable: "test" }; + const response = evaluate(js, dataTree, {}, { thisContext }); + expect(response.result).toBe("test"); + // there should not be any error when accessing "this" variables + expect(response.errors).toHaveLength(0); + }); + + it("has access to additional global context", () => { + const js = "contextVariable"; + const globalContext = { contextVariable: "test" }; + const response = evaluate(js, dataTree, {}, { globalContext }); + expect(response.result).toBe("test"); + expect(response.errors).toHaveLength(0); + }); }); describe("evaluateAsync", () => { @@ -256,7 +275,7 @@ describe("isFunctionAsync", () => { if (typeof testFunc === "string") { testFunc = eval(testFunc); } - const actual = isFunctionAsync(testFunc, {}); + const actual = isFunctionAsync(testFunc, {}, {}); expect(actual).toBe(testCase.expected); } }); diff --git a/app/client/src/workers/evaluate.ts b/app/client/src/workers/evaluate.ts index 521080e987..ed023a6b1f 100644 --- a/app/client/src/workers/evaluate.ts +++ b/app/client/src/workers/evaluate.ts @@ -34,12 +34,12 @@ export const EvaluationScripts: Record = { const result = ${ScriptTemplate} return result; } - closedFunction() + closedFunction.call(THIS_CONTEXT) `, [EvaluationScriptType.ANONYMOUS_FUNCTION]: ` function callback (script) { const userFunction = script; - const result = userFunction?.apply(self, ARGUMENTS); + const result = userFunction?.apply(THIS_CONTEXT, ARGUMENTS); return result; } callback(${ScriptTemplate}) @@ -49,7 +49,7 @@ export const EvaluationScripts: Record = { const result = await ${ScriptTemplate}; return result; } - closedFunction(); + closedFunction.call(THIS_CONTEXT); `, }; @@ -102,6 +102,18 @@ export const createGlobalData = ( const GLOBAL_DATA: Record = {}; ///// Adding callback data GLOBAL_DATA.ARGUMENTS = evalArguments; + //// Adding contextual data not part of data tree + GLOBAL_DATA.THIS_CONTEXT = {}; + if (context) { + if (context.thisContext) { + GLOBAL_DATA.THIS_CONTEXT = context.thisContext; + } + if (context.globalContext) { + Object.entries(context.globalContext).forEach(([key, value]) => { + GLOBAL_DATA[key] = value; + }); + } + } //// Add internal functions to dataTree; const dataTreeWithFunctions = enhanceDataTreeWithFunctions( dataTree, @@ -134,9 +146,13 @@ export function sanitizeScript(js: string) { } /** Define a context just for this script + * thisContext will define it on the `this` + * globalContext will define it globally * requestId is used for completing promises */ export type EvaluateContext = { + thisContext?: Record; + globalContext?: Record; requestId?: string; }; @@ -172,6 +188,7 @@ export default function evaluateSync( userScript: string, dataTree: DataTree, resolvedFunctions: Record, + context?: EvaluateContext, evalArguments?: Array, ): EvalResult { return (function() { @@ -181,7 +198,7 @@ export default function evaluateSync( const GLOBAL_DATA: Record = createGlobalData( dataTree, resolvedFunctions, - undefined, + context, evalArguments, ); GLOBAL_DATA.ALLOW_ASYNC = false; diff --git a/app/client/src/workers/validations.ts b/app/client/src/workers/validations.ts index b0fa3f7e6a..2ac004b312 100644 --- a/app/client/src/workers/validations.ts +++ b/app/client/src/workers/validations.ts @@ -803,7 +803,7 @@ export const VALIDATORS: Record = { }; if (config.params?.fnString && isString(config.params?.fnString)) { try { - const { result } = evaluate(config.params.fnString, {}, {}, [ + const { result } = evaluate(config.params.fnString, {}, {}, undefined, [ value, props, _, From 1a9775489b1b254c16fbe854ba5e1151f8a85348 Mon Sep 17 00:00:00 2001 From: Rishabh Rathod Date: Fri, 4 Feb 2022 17:52:25 +0530 Subject: [PATCH 3/8] fix: Race condition issue of meta reducer update (#10837) * Fix race condition issue of meta reducer update * refactor comment * Fix logic to update meta state --- app/client/src/actions/metaActions.ts | 19 +++---- .../reducers/entityReducers/metaReducer.ts | 54 +++++++++++++------ app/client/src/sagas/EvaluationsSaga.ts | 2 +- app/client/src/workers/evaluationUtils.ts | 8 +-- 4 files changed, 50 insertions(+), 33 deletions(-) diff --git a/app/client/src/actions/metaActions.ts b/app/client/src/actions/metaActions.ts index 2de6ff500c..fea4484f39 100644 --- a/app/client/src/actions/metaActions.ts +++ b/app/client/src/actions/metaActions.ts @@ -1,9 +1,7 @@ import { ReduxActionTypes, ReduxAction } from "constants/ReduxActionConstants"; import { BatchAction, batchAction } from "actions/batchActions"; +import { Diff } from "deep-diff"; import { DataTree } from "entities/DataTree/dataTreeFactory"; -import { isWidget } from "../workers/evaluationUtils"; -import { MetaState } from "../reducers/entityReducers/metaReducer"; -import isEmpty from "lodash/isEmpty"; export interface UpdateWidgetMetaPropertyPayload { widgetId: string; @@ -47,18 +45,15 @@ export const resetChildrenMetaProperty = ( }; }; -export const updateMetaState = (evaluatedDataTree: DataTree) => { - const updatedWidgetMetaState: MetaState = {}; - Object.values(evaluatedDataTree).forEach((entity) => { - if (isWidget(entity) && entity.widgetId && !isEmpty(entity.meta)) { - updatedWidgetMetaState[entity.widgetId] = entity.meta; - } - }); - +export const updateMetaState = ( + updates: Diff[], + updatedDataTree: DataTree, +) => { return { type: ReduxActionTypes.UPDATE_META_STATE, payload: { - updatedWidgetMetaState, + updates, + updatedDataTree, }, }; }; diff --git a/app/client/src/reducers/entityReducers/metaReducer.ts b/app/client/src/reducers/entityReducers/metaReducer.ts index 9facbe6fae..e33ba32066 100644 --- a/app/client/src/reducers/entityReducers/metaReducer.ts +++ b/app/client/src/reducers/entityReducers/metaReducer.ts @@ -1,13 +1,16 @@ -import { set, cloneDeep } from "lodash"; +import { set, cloneDeep, get } from "lodash"; import { createReducer } from "utils/AppsmithUtils"; import { UpdateWidgetMetaPropertyPayload } from "actions/metaActions"; -import isObject from "lodash/isObject"; -import { DataTreeWidget } from "entities/DataTree/dataTreeFactory"; + import { ReduxActionTypes, ReduxAction, WidgetReduxActionTypes, } from "constants/ReduxActionConstants"; +import { Diff } from "deep-diff"; +import produce from "immer"; +import { DataTree } from "entities/DataTree/dataTreeFactory"; +import { isWidget } from "../../workers/evaluationUtils"; export type MetaState = Record>; @@ -17,24 +20,41 @@ export const metaReducer = createReducer(initialState, { [ReduxActionTypes.UPDATE_META_STATE]: ( state: MetaState, action: ReduxAction<{ - updatedWidgetMetaState: Record; + updates: Diff[]; + updatedDataTree: DataTree; }>, ) => { - // if metaObject is updated in dataTree we also update meta values, to keep meta state in sync. - const newMetaState = cloneDeep(state); - const { updatedWidgetMetaState } = action.payload; + const { updatedDataTree, updates } = action.payload; - Object.entries(updatedWidgetMetaState).forEach( - ([entityWidgetId, entityMetaState]) => { - if (isObject(newMetaState[entityWidgetId])) { - Object.keys(newMetaState[entityWidgetId]).forEach((key) => { - if (key in entityMetaState) { - newMetaState[entityWidgetId][key] = entityMetaState[key]; + // if metaObject is updated in dataTree we also update meta values, to keep meta state in sync. + const newMetaState = produce(state, (draftMetaState) => { + if (updates.length) { + updates.forEach((update) => { + // if meta field is updated in the dataTree then update metaReducer values. + if ( + update.kind === "E" && + update.path?.length && + update.path?.length > 1 && + update.path[1] === "meta" + ) { + // path eg: Input1.meta.defaultText + const entity = get(updatedDataTree, update.path[0]); + const metaPropertyPath = update.path.slice(2); + if ( + isWidget(entity) && + entity.widgetId && + metaPropertyPath.length + ) { + set( + draftMetaState, + [entity.widgetId, ...metaPropertyPath], + update.rhs, + ); } - }); - } - }, - ); + } + }); + } + }); return newMetaState; }, [ReduxActionTypes.SET_META_PROP]: ( diff --git a/app/client/src/sagas/EvaluationsSaga.ts b/app/client/src/sagas/EvaluationsSaga.ts index 8ff1dbf042..0029be622d 100644 --- a/app/client/src/sagas/EvaluationsSaga.ts +++ b/app/client/src/sagas/EvaluationsSaga.ts @@ -140,7 +140,7 @@ function* evaluateTreeSaga( PerformanceTransactionName.SET_EVALUATED_TREE, ); - yield put(updateMetaState(dataTree)); + yield put(updateMetaState(updates, dataTree)); const updatedDataTree = yield select(getDataTree); log.debug({ jsUpdates: jsUpdates }); diff --git a/app/client/src/workers/evaluationUtils.ts b/app/client/src/workers/evaluationUtils.ts index 2808024ef8..f79d83eb31 100644 --- a/app/client/src/workers/evaluationUtils.ts +++ b/app/client/src/workers/evaluationUtils.ts @@ -805,10 +805,11 @@ export const overrideWidgetProperties = ( const overridingPropertyPaths = entity.overridingPropertyPaths[propertyPath]; - overridingPropertyPaths.forEach((overriddenPropertyKey) => { + overridingPropertyPaths.forEach((overriddenPropertyPath) => { + const overriddenPropertyPathArray = overriddenPropertyPath.split("."); _.set( currentTree, - `${entity.widgetName}.${overriddenPropertyKey}`, + [entity.widgetName, ...overriddenPropertyPathArray], clonedValue, ); }); @@ -824,9 +825,10 @@ export const overrideWidgetProperties = ( const defaultValue = entity[propertyOverridingKeyMap.DEFAULT]; const clonedDefaultValue = cloneDeep(defaultValue); if (defaultValue !== undefined) { + const propertyPathArray = propertyPath.split("."); _.set( currentTree, - `${entity.widgetName}.${propertyPath}`, + [entity.widgetName, ...propertyPathArray], clonedDefaultValue, ); return { From 056f2129e1ea2fec67d43902527537cd0148c91e Mon Sep 17 00:00:00 2001 From: balajisoundar Date: Fri, 4 Feb 2022 23:07:22 +0530 Subject: [PATCH 4/8] =?UTF-8?q?fix:=20skip=20bad=20table=20migration=20tha?= =?UTF-8?q?ts=20breaking=20computed=20columns=20we=20di=E2=80=A6=20(#10897?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/client/src/utils/DSLMigrations.ts | 6 ++++-- app/client/src/utils/migrations/TableWidget.ts | 3 +++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/app/client/src/utils/DSLMigrations.ts b/app/client/src/utils/DSLMigrations.ts index 7663bcc193..6c2848200d 100644 --- a/app/client/src/utils/DSLMigrations.ts +++ b/app/client/src/utils/DSLMigrations.ts @@ -25,7 +25,6 @@ import { migrateTableSanitizeColumnKeys, isSortableMigration, migrateTableWidgetIconButtonVariant, - migrateTableWidgetNumericColumnName, } from "./migrations/TableWidget"; import { migrateTextStyleFromTextWidget } from "./migrations/TextWidgetReplaceTextStyle"; import { DATA_BIND_REGEX_GLOBAL } from "constants/BindingsConstants"; @@ -1039,7 +1038,10 @@ export const transformDSL = ( } if (currentDSL.version === 50) { - currentDSL = migrateTableWidgetNumericColumnName(currentDSL); + /* + * We're skipping this to fix a bad table migration - migrateTableWidgetNumericColumnName + * it overwrites the computedValue of the table columns + */ currentDSL.version = LATEST_PAGE_VERSION; } diff --git a/app/client/src/utils/migrations/TableWidget.ts b/app/client/src/utils/migrations/TableWidget.ts index fb41c29533..6f3d7db200 100644 --- a/app/client/src/utils/migrations/TableWidget.ts +++ b/app/client/src/utils/migrations/TableWidget.ts @@ -490,6 +490,9 @@ export const migrateTableWidgetIconButtonVariant = (currentDSL: DSLWidget) => { return currentDSL; }; +/* + * DO NOT USE THIS. it overwrites conputedValues of the Table Columns + */ export const migrateTableWidgetNumericColumnName = (currentDSL: DSLWidget) => { currentDSL.children = currentDSL.children?.map((child: WidgetProps) => { if (child.type === "TABLE_WIDGET") { From e3ed1054c6bcf082a81c84a18ab5a2a6fe0a67b1 Mon Sep 17 00:00:00 2001 From: Sumit Kumar Date: Wed, 9 Feb 2022 09:38:58 +0530 Subject: [PATCH 5/8] fix: fix refresh token flow in REST API OAuth (#10875) Co-authored-by: Segun Daniel Oluwadare (cherry picked from commit aa2290405ddcddee3bc0c21d013875b953efb015) --- .../src/entities/Datasource/RestAPIForm.ts | 2 + .../Editor/DataSourceEditor/Collapsible.tsx | 2 +- .../RestAPIDatasourceForm.tsx | 77 +++++++++++++++ .../RestAPIDatasourceFormTransformer.ts | 6 ++ .../com/appsmith/external/models/OAuth2.java | 9 ++ .../connections/OAuth2AuthorizationCode.java | 95 +++++++++++++------ .../connections/OAuth2ClientCredentials.java | 1 + .../src/main/resources/form.json | 34 +++++++ .../ce/AuthenticationServiceCEImpl.java | 1 + .../ExamplesOrganizationClonerTests.java | 2 + 10 files changed, 198 insertions(+), 31 deletions(-) diff --git a/app/client/src/entities/Datasource/RestAPIForm.ts b/app/client/src/entities/Datasource/RestAPIForm.ts index 1a8829e85d..90fb5777ba 100644 --- a/app/client/src/entities/Datasource/RestAPIForm.ts +++ b/app/client/src/entities/Datasource/RestAPIForm.ts @@ -49,6 +49,8 @@ export interface Oauth2Common { isTokenHeader: boolean; audience: string; resource: string; + sendScopeWithRefreshToken: string; + refreshTokenClientCredentialsLocation: string; } export interface ClientCredentials extends Oauth2Common { diff --git a/app/client/src/pages/Editor/DataSourceEditor/Collapsible.tsx b/app/client/src/pages/Editor/DataSourceEditor/Collapsible.tsx index 39cba64939..25944a1363 100644 --- a/app/client/src/pages/Editor/DataSourceEditor/Collapsible.tsx +++ b/app/client/src/pages/Editor/DataSourceEditor/Collapsible.tsx @@ -34,7 +34,7 @@ interface ComponentState { interface ComponentProps { children: any; title: string; - defaultIsOpen: boolean; + defaultIsOpen?: boolean; } type Props = ComponentProps; diff --git a/app/client/src/pages/Editor/DataSourceEditor/RestAPIDatasourceForm.tsx b/app/client/src/pages/Editor/DataSourceEditor/RestAPIDatasourceForm.tsx index 7d50784606..e0d36ef6c2 100644 --- a/app/client/src/pages/Editor/DataSourceEditor/RestAPIDatasourceForm.tsx +++ b/app/client/src/pages/Editor/DataSourceEditor/RestAPIDatasourceForm.tsx @@ -235,6 +235,28 @@ class DatasourceRestAPIEditor extends React.Component { this.props.change("authentication.isAuthorizationHeader", true); } } + + if (_.get(authentication, "grantType") === GrantType.AuthorizationCode) { + if ( + _.get(authentication, "sendScopeWithRefreshToken") === undefined || + _.get(authentication, "sendScopeWithRefreshToken") === "" + ) { + this.props.change("authentication.sendScopeWithRefreshToken", false); + } + } + + if (_.get(authentication, "grantType") === GrantType.AuthorizationCode) { + if ( + _.get(authentication, "refreshTokenClientCredentialsLocation") === + undefined || + _.get(authentication, "refreshTokenClientCredentialsLocation") === "" + ) { + this.props.change( + "authentication.refreshTokenClientCredentialsLocation", + "BODY", + ); + } + } }; disableSave = (): boolean => { @@ -712,6 +734,57 @@ class DatasourceRestAPIEditor extends React.Component { ); }; + renderOauth2AdvancedSettings = () => { + return ( + <> + + {this.renderDropdownControlViaFormControl( + "authentication.sendScopeWithRefreshToken", + [ + { + label: "Yes", + value: true, + }, + { + label: "No", + value: false, + }, + ], + "Send scope with refresh token", + "", + false, + "", + )} + + + {this.renderDropdownControlViaFormControl( + "authentication.refreshTokenClientCredentialsLocation", + [ + { + label: "Body", + value: "BODY", + }, + { + label: "Header", + value: "HEADER", + }, + ], + "Send client credentials with", + "", + false, + "", + )} + + + ); + }; + renderOauth2CommonAdvanced = () => { return ( <> @@ -815,8 +888,12 @@ class DatasourceRestAPIEditor extends React.Component { "", )} + {!_.get(formData.authentication, "isAuthorizationHeader", true) && this.renderOauth2CommonAdvanced()} + + {this.renderOauth2AdvancedSettings()} + scope; + Boolean sendScopeWithRefreshToken; + String headerPrefix; Set customTokenParameters; diff --git a/app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/connections/OAuth2AuthorizationCode.java b/app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/connections/OAuth2AuthorizationCode.java index aeb4111175..4488bd9a60 100644 --- a/app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/connections/OAuth2AuthorizationCode.java +++ b/app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/connections/OAuth2AuthorizationCode.java @@ -10,6 +10,7 @@ import lombok.AccessLevel; import lombok.Getter; import lombok.NoArgsConstructor; import lombok.Setter; +import org.bson.internal.Base64; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; @@ -31,6 +32,10 @@ import java.time.Duration; import java.time.Instant; import java.util.Map; +import static com.appsmith.external.models.OAuth2.RefreshTokenClientCredentialsLocation.BODY; +import static com.appsmith.external.models.OAuth2.RefreshTokenClientCredentialsLocation.HEADER; +import static org.apache.commons.lang3.StringUtils.isBlank; + @Setter @Getter @NoArgsConstructor(access = AccessLevel.PRIVATE) @@ -45,6 +50,25 @@ public class OAuth2AuthorizationCode extends APIConnection implements UpdatableC private Object tokenResponse; private static final int MAX_IN_MEMORY_SIZE = 10 * 1024 * 1024; // 10 MB + private static void updateConnection(OAuth2AuthorizationCode connection, OAuth2 token) { + connection.setToken(token.getAuthenticationResponse().getToken()); + connection.setHeader(token.getIsTokenHeader()); + connection.setHeaderPrefix(token.getHeaderPrefix()); + connection.setExpiresAt(token.getAuthenticationResponse().getExpiresAt()); + connection.setRefreshToken(token.getAuthenticationResponse().getRefreshToken()); + connection.setTokenResponse(token.getAuthenticationResponse().getTokenResponse()); + } + + private static boolean isAuthenticationResponseValid(OAuth2 oAuth2) { + if (oAuth2.getAuthenticationResponse() == null + || isBlank(oAuth2.getAuthenticationResponse().getToken()) + || isExpired(oAuth2)) { + return false; + } + + return true; + } + public static Mono create(OAuth2 oAuth2) { if (oAuth2 == null) { return Mono.empty(); @@ -52,41 +76,46 @@ public class OAuth2AuthorizationCode extends APIConnection implements UpdatableC // Create OAuth2Connection OAuth2AuthorizationCode connection = new OAuth2AuthorizationCode(); - return Mono.just(oAuth2) - // Validate existing token - .filter(x -> x.getAuthenticationResponse() != null - && x.getAuthenticationResponse().getToken() != null - && !x.getAuthenticationResponse().getToken().isBlank()) - .filter(x -> x.getAuthenticationResponse().getExpiresAt() != null) - .filter(x -> { - Instant now = connection.clock.instant(); - Instant expiresAt = x.getAuthenticationResponse().getExpiresAt(); + if (!isAuthenticationResponseValid(oAuth2)) { + return connection.generateOAuth2Token(oAuth2) + .flatMap(token -> { + updateConnection(connection, token); + return Mono.just(connection); + }); + } - return now.isBefore(expiresAt.minus(Duration.ofMinutes(1))); - }) - // If invalid, regenerate token - .switchIfEmpty(connection.generateOAuth2Token(oAuth2)) - // Store valid token - .flatMap(token -> { - connection.setToken(token.getAuthenticationResponse().getToken()); - connection.setHeader(token.getIsTokenHeader()); - connection.setHeaderPrefix(token.getHeaderPrefix()); - connection.setExpiresAt(token.getAuthenticationResponse().getExpiresAt()); - connection.setRefreshToken(token.getAuthenticationResponse().getRefreshToken()); - connection.setTokenResponse(token.getAuthenticationResponse().getTokenResponse()); - return Mono.just(connection); - }); + updateConnection(connection, oAuth2); + return Mono.just(connection); + } + + private static boolean isExpired(OAuth2 oAuth2) { + if (oAuth2.getAuthenticationResponse().getExpiresAt() == null) { + return true; + } + + OAuth2AuthorizationCode connection = new OAuth2AuthorizationCode(); + Instant now = connection.clock.instant(); + Instant expiresAt = oAuth2.getAuthenticationResponse().getExpiresAt(); + + return now.isAfter(expiresAt.minus(Duration.ofMinutes(1))); } private Mono generateOAuth2Token(OAuth2 oAuth2) { - // Webclient - WebClient webClient = WebClient.builder() + WebClient.Builder webClientBuilder = WebClient.builder() .defaultHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE) .exchangeStrategies(ExchangeStrategies .builder() .codecs(configurer -> configurer.defaultCodecs().maxInMemorySize(MAX_IN_MEMORY_SIZE)) - .build()) - .build(); + .build()); + + if (HEADER.equals(oAuth2.getRefreshTokenClientCredentialsLocation())) { + byte[] clientCredentials = (oAuth2.getClientId() + ":" + oAuth2.getClientSecret()).getBytes(); + final String authorizationHeader = "Basic " + Base64.encode(clientCredentials); + webClientBuilder.defaultHeader("Authorization", authorizationHeader); + } + + // Webclient + WebClient webClient = webClientBuilder.build(); // Send oauth2 generic request return webClient @@ -168,9 +197,14 @@ public class OAuth2AuthorizationCode extends APIConnection implements UpdatableC private BodyInserters.FormInserter getTokenBody(OAuth2 oAuth2) { BodyInserters.FormInserter body = BodyInserters .fromFormData(Authentication.GRANT_TYPE, Authentication.REFRESH_TOKEN) - .with(Authentication.CLIENT_ID, oAuth2.getClientId()) - .with(Authentication.CLIENT_SECRET, oAuth2.getClientSecret()) .with(Authentication.REFRESH_TOKEN, oAuth2.getAuthenticationResponse().getRefreshToken()); + + if (BODY.equals(oAuth2.getRefreshTokenClientCredentialsLocation()) + || oAuth2.getRefreshTokenClientCredentialsLocation() == null) { + body.with(Authentication.CLIENT_ID, oAuth2.getClientId()) + .with(Authentication.CLIENT_SECRET, oAuth2.getClientSecret()); + } + // Adding optional audience parameter if (!StringUtils.isEmpty(oAuth2.getAudience())) { body.with(Authentication.AUDIENCE, oAuth2.getAudience()); @@ -180,7 +214,8 @@ public class OAuth2AuthorizationCode extends APIConnection implements UpdatableC body.with(Authentication.RESOURCE, oAuth2.getResource()); } // Optionally add scope, if applicable - if (!CollectionUtils.isEmpty(oAuth2.getScope())) { + if (!CollectionUtils.isEmpty(oAuth2.getScope()) + && (Boolean.TRUE.equals(oAuth2.getSendScopeWithRefreshToken()) || oAuth2.getSendScopeWithRefreshToken() == null)) { body.with(Authentication.SCOPE, StringUtils.collectionToDelimitedString(oAuth2.getScope(), " ")); } return body; diff --git a/app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/connections/OAuth2ClientCredentials.java b/app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/connections/OAuth2ClientCredentials.java index d859efbf5d..1954f3c5e1 100644 --- a/app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/connections/OAuth2ClientCredentials.java +++ b/app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/connections/OAuth2ClientCredentials.java @@ -165,6 +165,7 @@ public class OAuth2ClientCredentials extends APIConnection implements UpdatableC .fromFormData(Authentication.GRANT_TYPE, Authentication.CLIENT_CREDENTIALS) .with(Authentication.CLIENT_ID, oAuth2.getClientId()) .with(Authentication.CLIENT_SECRET, oAuth2.getClientSecret()); + // Adding optional audience parameter if (!StringUtils.isEmpty(oAuth2.getAudience())) { body.with(Authentication.AUDIENCE, oAuth2.getAudience()); diff --git a/app/server/appsmith-plugins/restApiPlugin/src/main/resources/form.json b/app/server/appsmith-plugins/restApiPlugin/src/main/resources/form.json index e90447c7a0..69418d1623 100644 --- a/app/server/appsmith-plugins/restApiPlugin/src/main/resources/form.json +++ b/app/server/appsmith-plugins/restApiPlugin/src/main/resources/form.json @@ -194,6 +194,40 @@ "comparison": "NOT_EQUALS", "value": "oAuth2" } + }, + { + "label": "Send scope with refresh token", + "configProperty": "datasourceConfiguration.authentication.sendScopeWithRefreshToken", + "controlType": "DROP_DOWN", + "isRequired": true, + "initialValue": false, + "options": [ + { + "label": "Yes", + "value": true + }, + { + "label": "No", + "value": false + } + ] + }, + { + "label": "Send client credentials with (on refresh token)", + "configProperty": "datasourceConfiguration.authentication.refreshTokenClientCredentialsLocation", + "controlType": "DROP_DOWN", + "isRequired": true, + "initialValue": "BODY", + "options": [ + { + "label": "Body", + "value": "BODY" + }, + { + "label": "Header", + "value": "HEADER" + } + ] } ] } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/AuthenticationServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/AuthenticationServiceCEImpl.java index 56dd3a8197..f4ccc5cacf 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/AuthenticationServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/AuthenticationServiceCEImpl.java @@ -231,6 +231,7 @@ public class AuthenticationServiceCEImpl implements AuthenticationServiceCE { expiresAt = Instant.ofEpochSecond(Long.valueOf((Integer) expiresAtResponse)); } else if (expiresInResponse != null) { expiresAt = issuedAt.plusSeconds(Long.valueOf((Integer) expiresInResponse)); + } authenticationResponse.setExpiresAt(expiresAt); // Replacing with returned scope instead diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ExamplesOrganizationClonerTests.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ExamplesOrganizationClonerTests.java index 93f0a39859..60c220f528 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ExamplesOrganizationClonerTests.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ExamplesOrganizationClonerTests.java @@ -854,6 +854,7 @@ public class ExamplesOrganizationClonerTests { DatasourceConfiguration dc2 = new DatasourceConfiguration(); ds2.setDatasourceConfiguration(dc2); dc2.setAuthentication(new OAuth2( + OAuth2.RefreshTokenClientCredentialsLocation.BODY, OAuth2.Type.CLIENT_CREDENTIALS, true, true, @@ -863,6 +864,7 @@ public class ExamplesOrganizationClonerTests { "access token url", "scope", Set.of("scope1", "scope2", "scope3"), + true, "header prefix", Set.of( new Property("custom token param 1", "custom token param value 1"), From 7ec0658cee800d6728b57bf9466c881fcc6d52b1 Mon Sep 17 00:00:00 2001 From: Shrikant Sharat Kandula Date: Wed, 9 Feb 2022 10:31:00 +0530 Subject: [PATCH 6/8] chore: Anonymous stats if telemetry is enabled (#11014) Signed-off-by: Shrikant Sharat Kandula (cherry picked from commit 6e1e2faaacd829426b044c52c831fd71d538d09d) --- .../ce/ApplicationRepositoryCE.java | 4 ++ .../ce/DatasourceRepositoryCE.java | 3 + .../ce/NewActionRepositoryCE.java | 3 + .../repositories/ce/NewPageRepositoryCE.java | 3 + .../ce/OrganizationRepositoryCE.java | 2 + .../repositories/ce/UserRepositoryCE.java | 3 + .../solutions/PingScheduledTaskImpl.java | 32 +++++++-- .../solutions/ce/PingScheduledTaskCEImpl.java | 67 ++++++++++++++++++- 8 files changed, 112 insertions(+), 5 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/ApplicationRepositoryCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/ApplicationRepositoryCE.java index f41dad4243..d49b99c096 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/ApplicationRepositoryCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/ApplicationRepositoryCE.java @@ -5,6 +5,7 @@ import com.appsmith.server.repositories.BaseRepository; import com.appsmith.server.repositories.CustomApplicationRepository; import org.springframework.stereotype.Repository; import reactor.core.publisher.Flux; +import reactor.core.publisher.Mono; import java.util.List; @@ -16,4 +17,7 @@ public interface ApplicationRepositoryCE extends BaseRepository findByOrganizationId(String organizationId); Flux findByClonedFromApplicationId(String clonedFromApplicationId); + + Mono countByDeletedAtNull(); + } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/DatasourceRepositoryCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/DatasourceRepositoryCE.java index 5362c3d7f6..57ccb57ef3 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/DatasourceRepositoryCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/DatasourceRepositoryCE.java @@ -4,6 +4,7 @@ import com.appsmith.external.models.Datasource; import com.appsmith.server.repositories.BaseRepository; import com.appsmith.server.repositories.CustomDatasourceRepository; import reactor.core.publisher.Flux; +import reactor.core.publisher.Mono; import java.util.List; @@ -13,4 +14,6 @@ public interface DatasourceRepositoryCE extends BaseRepository findAllByOrganizationId(String organizationId); + Mono countByDeletedAtNull(); + } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/NewActionRepositoryCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/NewActionRepositoryCE.java index 6d96bb4c68..a0ae98435b 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/NewActionRepositoryCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/NewActionRepositoryCE.java @@ -4,9 +4,12 @@ import com.appsmith.server.domains.NewAction; import com.appsmith.server.repositories.BaseRepository; import com.appsmith.server.repositories.CustomNewActionRepository; import reactor.core.publisher.Flux; +import reactor.core.publisher.Mono; public interface NewActionRepositoryCE extends BaseRepository, CustomNewActionRepository { Flux findByApplicationId(String applicationId); + Mono countByDeletedAtNull(); + } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/NewPageRepositoryCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/NewPageRepositoryCE.java index 8b4fb1b558..0b08dbba80 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/NewPageRepositoryCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/NewPageRepositoryCE.java @@ -4,9 +4,12 @@ import com.appsmith.server.domains.NewPage; import com.appsmith.server.repositories.BaseRepository; import com.appsmith.server.repositories.CustomNewPageRepository; import reactor.core.publisher.Flux; +import reactor.core.publisher.Mono; public interface NewPageRepositoryCE extends BaseRepository, CustomNewPageRepository { Flux findByApplicationId(String applicationId); + Mono countByDeletedAtNull(); + } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/OrganizationRepositoryCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/OrganizationRepositoryCE.java index e346e5e5ce..d350f0910e 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/OrganizationRepositoryCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/OrganizationRepositoryCE.java @@ -15,4 +15,6 @@ public interface OrganizationRepositoryCE extends BaseRepository updateUserRoleNames(String userId, String userName); + Mono countByDeletedAtNull(); + } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/UserRepositoryCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/UserRepositoryCE.java index c9ea6bbe6f..d199e25cac 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/UserRepositoryCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/UserRepositoryCE.java @@ -10,4 +10,7 @@ public interface UserRepositoryCE extends BaseRepository, CustomUs Mono findByEmail(String email); Mono findByCaseInsensitiveEmail(String email); + + Mono countByDeletedAtNull(); + } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/PingScheduledTaskImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/PingScheduledTaskImpl.java index 6b73224876..3dba94bb70 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/PingScheduledTaskImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/PingScheduledTaskImpl.java @@ -2,6 +2,12 @@ package com.appsmith.server.solutions; import com.appsmith.server.configurations.CommonConfig; import com.appsmith.server.configurations.SegmentConfig; +import com.appsmith.server.repositories.ApplicationRepository; +import com.appsmith.server.repositories.DatasourceRepository; +import com.appsmith.server.repositories.NewActionRepository; +import com.appsmith.server.repositories.NewPageRepository; +import com.appsmith.server.repositories.OrganizationRepository; +import com.appsmith.server.repositories.UserRepository; import com.appsmith.server.services.ConfigService; import com.appsmith.server.solutions.ce.PingScheduledTaskCEImpl; import lombok.extern.slf4j.Slf4j; @@ -18,10 +24,28 @@ import org.springframework.stereotype.Component; @Component public class PingScheduledTaskImpl extends PingScheduledTaskCEImpl implements PingScheduledTask { - public PingScheduledTaskImpl(ConfigService configService, - SegmentConfig segmentConfig, - CommonConfig commonConfig) { + public PingScheduledTaskImpl( + ConfigService configService, + SegmentConfig segmentConfig, + CommonConfig commonConfig, + OrganizationRepository organizationRepository, + ApplicationRepository applicationRepository, + NewPageRepository newPageRepository, + NewActionRepository newActionRepository, + DatasourceRepository datasourceRepository, + UserRepository userRepository + ) { - super(configService, segmentConfig, commonConfig); + super( + configService, + segmentConfig, + commonConfig, + organizationRepository, + applicationRepository, + newPageRepository, + newActionRepository, + datasourceRepository, + userRepository + ); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/PingScheduledTaskCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/PingScheduledTaskCEImpl.java index 7f054925c6..62126064db 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/PingScheduledTaskCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/PingScheduledTaskCEImpl.java @@ -3,6 +3,12 @@ package com.appsmith.server.solutions.ce; import com.appsmith.server.configurations.CommonConfig; import com.appsmith.server.configurations.SegmentConfig; import com.appsmith.server.helpers.NetworkUtils; +import com.appsmith.server.repositories.ApplicationRepository; +import com.appsmith.server.repositories.DatasourceRepository; +import com.appsmith.server.repositories.NewActionRepository; +import com.appsmith.server.repositories.NewPageRepository; +import com.appsmith.server.repositories.OrganizationRepository; +import com.appsmith.server.repositories.UserRepository; import com.appsmith.server.services.ConfigService; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -33,6 +39,13 @@ public class PingScheduledTaskCEImpl implements PingScheduledTaskCE { private final CommonConfig commonConfig; + private final OrganizationRepository organizationRepository; + private final ApplicationRepository applicationRepository; + private final NewPageRepository newPageRepository; + private final NewActionRepository newActionRepository; + private final DatasourceRepository datasourceRepository; + private final UserRepository userRepository; + /** * Gets the external IP address of this server and pings a data point to indicate that this server instance is live. * We use an initial delay of two minutes to roughly wait for the application along with the migrations are finished @@ -65,7 +78,8 @@ public class PingScheduledTaskCEImpl implements PingScheduledTaskCE { // are not intended to be configurable. final String ceKey = segmentConfig.getCeKey(); if (StringUtils.isEmpty(ceKey)) { - log.error("The segment key is null"); + log.error("The segment ce key is null"); + return Mono.empty(); } return WebClient @@ -84,4 +98,55 @@ public class PingScheduledTaskCEImpl implements PingScheduledTaskCE { .bodyToMono(String.class); } + // Number of milliseconds between the start of each scheduled calls to this method. + @Scheduled(initialDelay = 2 * 60 * 1000 /* two minutes */, fixedRate = 24 * 60 * 60 * 1000 /* a day */) + public void pingStats() { + if (commonConfig.isTelemetryDisabled()) { + return; + } + + final String ceKey = segmentConfig.getCeKey(); + if (StringUtils.isEmpty(ceKey)) { + log.error("The segment ce key is null"); + return; + } + + Mono.zip( + configService.getInstanceId().defaultIfEmpty("null"), + NetworkUtils.getExternalAddress(), + organizationRepository.countByDeletedAtNull().defaultIfEmpty(0L), + applicationRepository.countByDeletedAtNull().defaultIfEmpty(0L), + newPageRepository.countByDeletedAtNull().defaultIfEmpty(0L), + newActionRepository.countByDeletedAtNull().defaultIfEmpty(0L), + datasourceRepository.countByDeletedAtNull().defaultIfEmpty(0L), + userRepository.countByDeletedAtNull().defaultIfEmpty(0L) + ) + .flatMap(statsData -> { + final String ipAddress = statsData.getT2(); + return WebClient + .create("https://api.segment.io") + .post() + .uri("/v1/track") + .headers(headers -> headers.setBasicAuth(ceKey, "")) + .contentType(MediaType.APPLICATION_JSON) + .body(BodyInserters.fromValue(Map.of( + "userId", ipAddress, + "context", Map.of("ip", ipAddress), + "properties", Map.of("instanceId", statsData.getT1()), + "numOrgs", statsData.getT3(), + "numApps", statsData.getT4(), + "numPages", statsData.getT5(), + "numActions", statsData.getT6(), + "numDatasources", statsData.getT7(), + "numUsers", statsData.getT8(), + "event", "instance_stats" + ))) + .retrieve() + .bodyToMono(String.class); + }) + .doOnError(error -> log.error("Error sending anonymous counts {0}", error)) + .subscribeOn(Schedulers.boundedElastic()) + .subscribe(); + } + } From b6025769d08176f477bcf30cd51c5be816fff223 Mon Sep 17 00:00:00 2001 From: yatinappsmith <84702014+yatinappsmith@users.noreply.github.com> Date: Wed, 9 Feb 2022 18:00:56 +0530 Subject: [PATCH 7/8] test: replace full path with github workspace (#11047) --- .github/workflows/TestReuseActions.yml | 2 +- .github/workflows/integration-tests-command.yml | 2 +- .github/workflows/test-build-docker-image.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/TestReuseActions.yml b/.github/workflows/TestReuseActions.yml index fca69cd784..60d2fe6fed 100644 --- a/.github/workflows/TestReuseActions.yml +++ b/.github/workflows/TestReuseActions.yml @@ -727,7 +727,7 @@ jobs: - name: Incase of test failures copy them to a file if: failure() run: | - cd /home/runner/work/appsmith/appsmith/app/client/cypress/ + cd ${{ github.workspace }}/app/client/cypress/ find screenshots -type d|grep spec |sed 's/screenshots/cypress\/integration/g' > ~/failed_spec/failed_spec-${{ matrix.job }} # Upload failed test list using common path for all matrix job diff --git a/.github/workflows/integration-tests-command.yml b/.github/workflows/integration-tests-command.yml index c54d543386..e639da2619 100644 --- a/.github/workflows/integration-tests-command.yml +++ b/.github/workflows/integration-tests-command.yml @@ -606,7 +606,7 @@ jobs: - name: Incase of test failures copy them to a file if: failure() run: | - cd /home/runner/work/appsmith/appsmith/app/client/cypress/ + cd ${{ github.workspace }}/app/client/cypress/ find screenshots -type d|grep spec |sed 's/screenshots/cypress\/integration/g' > ~/failed_spec/failed_spec-${{ matrix.job }} # Upload failed test list using common path for all matrix job diff --git a/.github/workflows/test-build-docker-image.yml b/.github/workflows/test-build-docker-image.yml index 5b5a88436f..52bf7dcd75 100644 --- a/.github/workflows/test-build-docker-image.yml +++ b/.github/workflows/test-build-docker-image.yml @@ -722,7 +722,7 @@ jobs: - name: Incase of test failures copy them to a file if: failure() run: | - cd /home/runner/work/appsmith/appsmith/app/client/cypress/ + cd ${{ github.workspace }}/app/client/cypress/ find screenshots -type d|grep spec |sed 's/screenshots/cypress\/integration/g' > ~/failed_spec/failed_spec-${{ matrix.job }} # Upload failed test list using common path for all matrix job From 3cb50a53a4f1e26d6fbb52cc06848a310dc954c4 Mon Sep 17 00:00:00 2001 From: Shrikant Sharat Kandula Date: Mon, 21 Feb 2022 13:35:02 +0530 Subject: [PATCH 8/8] Move data counts to properties for analytics (#11236) (#11298) Minor change in data structure of analytics data points. Signed-off-by: Shrikant Sharat Kandula --- .../solutions/ce/PingScheduledTaskCEImpl.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/PingScheduledTaskCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/PingScheduledTaskCEImpl.java index 62126064db..9ce4f19833 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/PingScheduledTaskCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/PingScheduledTaskCEImpl.java @@ -132,13 +132,15 @@ public class PingScheduledTaskCEImpl implements PingScheduledTaskCE { .body(BodyInserters.fromValue(Map.of( "userId", ipAddress, "context", Map.of("ip", ipAddress), - "properties", Map.of("instanceId", statsData.getT1()), - "numOrgs", statsData.getT3(), - "numApps", statsData.getT4(), - "numPages", statsData.getT5(), - "numActions", statsData.getT6(), - "numDatasources", statsData.getT7(), - "numUsers", statsData.getT8(), + "properties", Map.of( + "instanceId", statsData.getT1(), + "numOrgs", statsData.getT3(), + "numApps", statsData.getT4(), + "numPages", statsData.getT5(), + "numActions", statsData.getT6(), + "numDatasources", statsData.getT7(), + "numUsers", statsData.getT8() + ), "event", "instance_stats" ))) .retrieve()