From 342c5cc673699a173ba4323dde6dbcb1abd44b69 Mon Sep 17 00:00:00 2001 From: Ayangade Adeoluwa <37867493+Irongade@users.noreply.github.com> Date: Tue, 13 Sep 2022 06:02:59 +0100 Subject: [PATCH] fix: Fix Api url path issue (#16628) * Fix Api url path issue * Modify test cases --- .../BugTests/AbortAction_Spec.ts | 16 +++++-- .../ClientSideTests/BugTests/Bug16377_spec.ts | 24 ++++++++++ .../support/Objects/CommonErrorMessages.ts | 12 +++++ app/client/cypress/support/Pages/ApiPage.ts | 7 ++- .../editorComponents/ApiResponseView.tsx | 1 + .../fields/EmbeddedDatasourcePathField.tsx | 3 +- .../src/transformers/RestActionTransformer.ts | 47 ++++++++++++++++++- .../RestActionTransformers.test.ts | 41 +++++++++++++++- 8 files changed, 141 insertions(+), 10 deletions(-) create mode 100644 app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/BugTests/Bug16377_spec.ts create mode 100644 app/client/cypress/support/Objects/CommonErrorMessages.ts diff --git a/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/BugTests/AbortAction_Spec.ts b/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/BugTests/AbortAction_Spec.ts index 6bd0e42c81..23b3d8e593 100644 --- a/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/BugTests/AbortAction_Spec.ts +++ b/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/BugTests/AbortAction_Spec.ts @@ -1,5 +1,10 @@ import { ObjectsRegistry } from "../../../../support/Objects/Registry"; +import { + ACTION_EXECUTION_CANCELLED, + createMessage, +} from "../../../../support/Objects/CommonErrorMessages"; + const agHelper = ObjectsRegistry.AggregateHelper, locator = ObjectsRegistry.CommonLocators, apiPage = ObjectsRegistry.ApiPage, @@ -10,15 +15,14 @@ let dsName: any; const largeResponseApiUrl = "https://api.publicapis.org/entries"; //"https://jsonplaceholder.typicode.com/photos";//Commenting since this is faster sometimes & case is failing -export const ACTION_EXECUTION_CANCELLED = (actionName: string) => - `${actionName} was cancelled`; - describe("Abort Action Execution", function() { it("1. Bug #14006, #16093 - Cancel Request button should abort API action execution", function() { apiPage.CreateAndFillApi(largeResponseApiUrl, "AbortApi", 0); apiPage.RunAPI(false, 0); agHelper.GetNClick(locator._cancelActionExecution, 0, true); - agHelper.AssertContains(ACTION_EXECUTION_CANCELLED("AbortApi")); + agHelper.AssertContains( + createMessage(ACTION_EXECUTION_CANCELLED, "AbortApi"), + ); agHelper.AssertElementAbsence(locator._specificToast("{}")); //Assert that empty toast does not appear - Bug #16093 agHelper.ActionContextMenuWithInPane("Delete", "Are you sure?"); }); @@ -38,7 +42,9 @@ describe("Abort Action Execution", function() { dataSources.SetQueryTimeout(0); dataSources.RunQuery(false, false, 0); agHelper.GetNClick(locator._cancelActionExecution, 0, true); - agHelper.AssertContains(ACTION_EXECUTION_CANCELLED("AbortQuery")); + agHelper.AssertContains( + createMessage(ACTION_EXECUTION_CANCELLED, "AbortQuery"), + ); agHelper.AssertElementAbsence(locator._specificToast("{}")); //Assert that empty toast does not appear - Bug #16093 agHelper.ActionContextMenuWithInPane("Delete", "Are you sure?"); dataSources.DeleteDatasouceFromWinthinDS(dsName); diff --git a/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/BugTests/Bug16377_spec.ts b/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/BugTests/Bug16377_spec.ts new file mode 100644 index 0000000000..1a45f229d7 --- /dev/null +++ b/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/BugTests/Bug16377_spec.ts @@ -0,0 +1,24 @@ +import { ObjectsRegistry } from "../../../../support/Objects/Registry"; +import { + ERROR_ACTION_EXECUTE_FAIL, + createMessage, +} from "../../../../support/Objects/CommonErrorMessages"; + +const locator = ObjectsRegistry.CommonLocators, + apiPage = ObjectsRegistry.ApiPage, + agHelper = ObjectsRegistry.AggregateHelper; + +describe("Binding Expressions should not be truncated in Url and path extraction", function() { + it("Bug 16377, When Api url has dynamic binding expressions, ensure the url and path derived is not corrupting Api execution", function() { + const apiUrl = `https://mock-api.appsmith.com/{{true ? 'users' : 'user'}}`; + + apiPage.CreateAndFillApi(apiUrl, "BindingExpressions"); + apiPage.RunAPI(); + agHelper.AssertElementAbsence( + locator._specificToast( + createMessage(ERROR_ACTION_EXECUTE_FAIL, "BindingExpressions"), + ), + ); //Assert that an error is not returned. + apiPage.ResponseStatusCheck("200 OK"); + }); +}); diff --git a/app/client/cypress/support/Objects/CommonErrorMessages.ts b/app/client/cypress/support/Objects/CommonErrorMessages.ts new file mode 100644 index 0000000000..2c18309988 --- /dev/null +++ b/app/client/cypress/support/Objects/CommonErrorMessages.ts @@ -0,0 +1,12 @@ +export function createMessage( + format: (...strArgs: any[]) => string, + ...args: any[] +) { + return format(...args); +} + +export const ERROR_ACTION_EXECUTE_FAIL = (actionName: string) => + `${actionName} action returned an error response`; + +export const ACTION_EXECUTION_CANCELLED = (actionName: string) => + `${actionName} was cancelled`; diff --git a/app/client/cypress/support/Pages/ApiPage.ts b/app/client/cypress/support/Pages/ApiPage.ts index 3318a64740..fa9410c591 100644 --- a/app/client/cypress/support/Pages/ApiPage.ts +++ b/app/client/cypress/support/Pages/ApiPage.ts @@ -41,6 +41,7 @@ export class ApiPage { "input[name='confirmBeforeExecute'][type='checkbox']"; private _paginationTypeLabels = ".t--apiFormPaginationType label"; _saveAsDS = ".t--store-as-datasource"; + _responseStatus = ".t--response-status-code"; CreateApi( apiName = "", @@ -251,7 +252,7 @@ export class ApiPage { } ReadApiResponsebyKey(key: string) { - let apiResp: string = ""; + let apiResp = ""; cy.get(this._responseBody) .contains(key) .siblings("span") @@ -273,6 +274,10 @@ export class ApiPage { .click(); } + ResponseStatusCheck(statusCode: string) { + this.agHelper.AssertElementVisible(this._responseStatus); + cy.get(this._responseStatus).contains(statusCode); + } public SelectPaginationTypeViaIndex(index: number) { cy.get(this._paginationTypeLabels) .eq(index) diff --git a/app/client/src/components/editorComponents/ApiResponseView.tsx b/app/client/src/components/editorComponents/ApiResponseView.tsx index 2be0c88647..2bd1930b58 100644 --- a/app/client/src/components/editorComponents/ApiResponseView.tsx +++ b/app/client/src/components/editorComponents/ApiResponseView.tsx @@ -569,6 +569,7 @@ function ApiResponseView(props: Props) { Status: {response.statusCode} diff --git a/app/client/src/components/editorComponents/form/fields/EmbeddedDatasourcePathField.tsx b/app/client/src/components/editorComponents/form/fields/EmbeddedDatasourcePathField.tsx index dcdf54d2d4..450cafe315 100644 --- a/app/client/src/components/editorComponents/form/fields/EmbeddedDatasourcePathField.tsx +++ b/app/client/src/components/editorComponents/form/fields/EmbeddedDatasourcePathField.tsx @@ -58,6 +58,7 @@ import { getDatasourcesByPluginId, } from "selectors/entitiesSelector"; import { datasourcesEditorIdURL } from "RouteBuilder"; +import { extractApiUrlPath } from "transformers/RestActionTransformer"; type ReduxStateProps = { workspaceId: string; @@ -381,7 +382,7 @@ class EmbeddedDatasourcePathComponent extends React.Component< let evaluatedPath = "path" in entity.config ? entity.config.path : ""; if (evaluatedPath && evaluatedPath.indexOf("?") > -1) { - evaluatedPath = evaluatedPath.slice(0, evaluatedPath.indexOf("?")); + evaluatedPath = extractApiUrlPath(evaluatedPath); } const evaluatedQueryParameters = entity.config.queryParameters ?.filter((p: KeyValuePair) => p.key) diff --git a/app/client/src/transformers/RestActionTransformer.ts b/app/client/src/transformers/RestActionTransformer.ts index a850aaab50..a055803cf6 100644 --- a/app/client/src/transformers/RestActionTransformer.ts +++ b/app/client/src/transformers/RestActionTransformer.ts @@ -6,6 +6,10 @@ import { ApiAction } from "entities/Action"; import isEmpty from "lodash/isEmpty"; import isString from "lodash/isString"; import cloneDeep from "lodash/cloneDeep"; +import { + getDynamicStringSegments, + isDynamicValue, +} from "utils/DynamicBindingUtils"; export const transformRestAction = (data: ApiAction): ApiAction => { let action = cloneDeep(data); @@ -32,12 +36,16 @@ export const transformRestAction = (data: ApiAction): ApiAction => { action.actionConfiguration.queryParameters.length ) { const path = action.actionConfiguration.path; - if (path && path.indexOf("?") > -1) { + + // This can help extract paths from the following examples + const templatePaths = extractApiUrlPath(path); + + if (path && templatePaths !== path) { action = { ...action, actionConfiguration: { ...action.actionConfiguration, - path: path.slice(0, path.indexOf("?")), + path: templatePaths, }, }; } @@ -81,3 +89,38 @@ function removeEmptyPairs(keyValueArray: any) { (!isEmpty(data.key) || !isEmpty(data.value) || !isEmpty(data.type)), ); } + +// This function extracts the appropriate paths regardless of whatever expressions exist within the dynamic bindings. + +// Example 1: `/{{Text1.text ? 'users' : 'user'}}` +// Example 2: `/{{Text1.text ? 'users' : 'user'}}/{{"test"}}?` +// Example 3: `/{{Text1.text ? 'users' : 'user'}}/{{"test"}}?a=hello&b=world` + +// Output 1: /{{Text1.text ? 'users' : 'user'}}` +// Output 2: /{{Text1.text ? 'users' : 'user'}}/{{"test"}}` +// Output 3: /{{Text1.text ? 'users' : 'user'}}/{{"test"}}` + +export const extractApiUrlPath = (path: string | undefined) => { + const dynamicStringSegments = getDynamicStringSegments(path || ""); + const dynamicValuesDetected: string[] = []; + + const templateStringSegments = dynamicStringSegments.map((segment) => { + if (isDynamicValue(segment)) { + dynamicValuesDetected.push(segment); + return "~"; + } + return segment; + }); + + const indexOfQueryParams = templateStringSegments.join("").indexOf("?"); + + let templatePaths = templateStringSegments + .join("") + .slice(0, indexOfQueryParams === -1 ? undefined : indexOfQueryParams); + + dynamicValuesDetected.forEach((val) => { + templatePaths = templatePaths.replace(/~/, val); + }); + + return templatePaths; +}; diff --git a/app/client/src/transformers/RestActionTransformers.test.ts b/app/client/src/transformers/RestActionTransformers.test.ts index 39912ed62d..b384122b7b 100644 --- a/app/client/src/transformers/RestActionTransformers.test.ts +++ b/app/client/src/transformers/RestActionTransformers.test.ts @@ -1,4 +1,7 @@ -import { transformRestAction } from "transformers/RestActionTransformer"; +import { + extractApiUrlPath, + transformRestAction, +} from "transformers/RestActionTransformer"; import { PluginType, ApiAction } from "entities/Action"; import { MultiPartOptionTypes, @@ -327,4 +330,40 @@ describe("Api action transformer", () => { const result = transformRestAction(input); expect(result).toEqual(output); }); + + it("Ensures that Api url path is being correctly extracted regardless of expressions witin dynamic bindings", () => { + // testing for simple dynamic bindings in path + const path1 = `/{{Text1.text ? 'users' : 'user'}}`; + const output1 = `/{{Text1.text ? 'users' : 'user'}}`; + + const result1 = extractApiUrlPath(path1); + expect(result1).toEqual(output1); + + // testing multiple dynamic bindings in path with empty query params + const path2 = `/{{Text1.text ? 'users' : 'user'}}/{{"test"}}?`; + const output2 = `/{{Text1.text ? 'users' : 'user'}}/{{"test"}}`; + + const result2 = extractApiUrlPath(path2); + expect(result2).toEqual(output2); + + // testing multiple dynamic bindings in path with non-empty query params + const path3 = `/{{Text1.text ? 'users' : 'user'}}/{{"test"}}?a=hello&b=world`; + const output3 = `/{{Text1.text ? 'users' : 'user'}}/{{"test"}}`; + + const result3 = extractApiUrlPath(path3); + expect(result3).toEqual(output3); + + // testing normal strings and dynamic bindings in path with non-empty query params + const path4 = `/key/{{Text1.text}}?a=hello&b=world`; + const output4 = `/key/{{Text1.text}}`; + + const result4 = extractApiUrlPath(path4); + expect(result4).toEqual(output4); + + const path5 = "/{{Text1.text ?? 'user1'}}"; + const output5 = "/{{Text1.text ?? 'user1'}}"; + + const result5 = extractApiUrlPath(path5); + expect(result5).toEqual(output5); + }); });