diff --git a/app/client/src/api/ActionAPI.tsx b/app/client/src/api/ActionAPI.tsx index 7661045bc2..11c353a6ad 100644 --- a/app/client/src/api/ActionAPI.tsx +++ b/app/client/src/api/ActionAPI.tsx @@ -49,7 +49,13 @@ export interface ExecuteActionRequest extends APIRequest { params?: Property[]; paginationField?: PaginationField; viewMode: boolean; - paramProperties: Record>; + paramProperties: Record< + string, + | string + | Record> + | Record + | Record>> + >; } export type ExecuteActionResponse = ApiResponse & { diff --git a/app/client/src/constants/WidgetConstants.tsx b/app/client/src/constants/WidgetConstants.tsx index 4920c8f3f1..a13ab33c93 100644 --- a/app/client/src/constants/WidgetConstants.tsx +++ b/app/client/src/constants/WidgetConstants.tsx @@ -216,3 +216,5 @@ export const WIDGET_PROPS_TO_SKIP_FROM_EVAL = { * It is also used to calculate widget positions and highlight placements. */ export const FLEXBOX_PADDING = 4; + +export const FILE_SIZE_LIMIT_FOR_BLOBS = 5000 * 1024; // 5MB diff --git a/app/client/src/sagas/ActionExecution/PluginActionSaga.ts b/app/client/src/sagas/ActionExecution/PluginActionSaga.ts index 2c3d48cd5d..ee45612018 100644 --- a/app/client/src/sagas/ActionExecution/PluginActionSaga.ts +++ b/app/client/src/sagas/ActionExecution/PluginActionSaga.ts @@ -37,7 +37,18 @@ import { getAppMode, getCurrentApplication, } from "@appsmith/selectors/applicationSelectors"; -import { get, isArray, isString, set, find, isNil, flatten } from "lodash"; +import { + get, + isArray, + isString, + set, + find, + isNil, + flatten, + isArrayBuffer, + isEmpty, + unset, +} from "lodash"; import AppsmithConsole from "utils/AppsmithConsole"; import { ENTITY_TYPE, PLATFORM_ERROR } from "entities/AppsmithConsole"; import { @@ -122,6 +133,7 @@ import { setDefaultActionDisplayFormat } from "./PluginActionSagaUtils"; import { checkAndLogErrorsIfCyclicDependency } from "sagas/helper"; import type { TRunDescription } from "workers/Evaluation/fns/actionFns"; import { DEBUGGER_TAB_KEYS } from "components/editorComponents/Debugger/helpers"; +import { FILE_SIZE_LIMIT_FOR_BLOBS } from "constants/WidgetConstants"; enum ActionResponseDataTypes { BINARY = "BINARY", @@ -195,7 +207,16 @@ function* readBlob(blobUrl: string): any { if (fileType === FileDataTypes.Base64) { reader.readAsDataURL(file); } else if (fileType === FileDataTypes.Binary) { - reader.readAsBinaryString(file); + if (file.size < FILE_SIZE_LIMIT_FOR_BLOBS) { + //check size of the file, if less than 5mb, go with binary string method + // TODO: this method is deprecated, use readAsText instead + reader.readAsBinaryString(file); + } else { + // For files greater than 5 mb, use array buffer method + // This is to remove the bloat from the file which is added + // when using read as binary string method + reader.readAsArrayBuffer(file); + } } else { reader.readAsText(file); } @@ -228,7 +249,9 @@ function* resolvingBlobUrls( //If array elements then dont push datatypes to payload. isArray ? arrDatatype?.push(dataType) - : (executeActionRequest.paramProperties[`k${index}`] = dataType); + : (executeActionRequest.paramProperties[`k${index}`] = { + datatype: dataType, + }); if (isTrueObject(value)) { const blobUrlPaths: string[] = []; @@ -242,6 +265,17 @@ function* resolvingBlobUrls( const blobUrl = value[blobUrlPath] as string; const resolvedBlobValue: unknown = yield call(readBlob, blobUrl); set(value, blobUrlPath, resolvedBlobValue); + + // We need to store the url path map to be able to update the blob data + // and send the info to server + + // Here we fetch the blobUrlPathMap from the action payload and update it + const blobUrlPathMap = get(value, "blobUrlPaths", {}) as Record< + string, + string + >; + set(blobUrlPathMap, blobUrlPath, blobUrl); + set(value, "blobUrlPaths", blobUrlPathMap); } } else if (isBlobUrl(value)) { // @ts-expect-error: Values can take many types @@ -251,6 +285,28 @@ function* resolvingBlobUrls( return value; } +// Function that updates the blob data in the action payload for large file +// uploads +function updateBlobDataFromUrls( + blobUrlPaths: Record, + newVal: any, + blobMap: string[], + blobDataMap: Record, +) { + Object.entries(blobUrlPaths as Record).forEach( + // blobUrl: string eg: blob:1234-1234-1234?type=binary + ([path, blobUrl]) => { + if (isArrayBuffer(newVal[path])) { + // remove the ?type=binary from the blob url if present + const sanitisedBlobURL = blobUrl.split("?")[0]; + blobMap.push(sanitisedBlobURL); + set(blobDataMap, sanitisedBlobURL, new Blob([newVal[path]])); + set(newVal, path, sanitisedBlobURL); + } + }, + ); +} + /** * Api1 * URL: https://example.com/{{Text1.text}} @@ -298,6 +354,9 @@ function* evaluateActionParams( const bindingsMap: Record = {}; const bindingBlob = []; + // Maintain a blob data map to resolve blob urls of large files as array buffer + const blobDataMap: Record = {}; + let recordFilePickerInstrumentation = false; // if json bindings have filepicker reference, we need to init the instrumentation object @@ -311,12 +370,16 @@ function* evaluateActionParams( const key = bindings[i]; let value = values[i]; + let useBlobMaps = false; + // Maintain a blob map to resolve blob urls of large files + const blobMap: Array = []; + if (isArray(value)) { const tempArr = []; - const arrDatatype: string[] = []; + const arrDatatype: Array = []; // array of objects containing blob urls that is loops and individual object is checked for resolution of blob urls. for (const val of value) { - const newVal: unknown = yield call( + const newVal: Record = yield call( resolvingBlobUrls, val, executeActionRequest, @@ -324,11 +387,22 @@ function* evaluateActionParams( true, arrDatatype, ); + + if (newVal.hasOwnProperty("blobUrlPaths")) { + updateBlobDataFromUrls( + newVal.blobUrlPaths, + newVal, + blobMap, + blobDataMap, + ); + useBlobMaps = true; + unset(newVal, "blobUrlPaths"); + } + tempArr.push(newVal); if (key.includes(".files") && recordFilePickerInstrumentation) { filePickerInstrumentation["numberOfFiles"] += 1; - // @ts-expect-error: Values can take many types const { size, type } = newVal; filePickerInstrumentation["totalSize"] += size; filePickerInstrumentation["fileSizes"].push(size); @@ -337,7 +411,7 @@ function* evaluateActionParams( } //Adding array datatype along with the datatype of first element of the array executeActionRequest.paramProperties[`k${i}`] = { - array: [arrDatatype[0]], + datatype: { array: [arrDatatype[0]] }, }; value = tempArr; } else { @@ -352,17 +426,44 @@ function* evaluateActionParams( } if (typeof value === "object") { + // This is used in cases of large files, we store the bloburls with the path they were set in + // This helps in creating a unique map of blob urls to blob data when passing to the server + if (!!value && value.hasOwnProperty("blobUrlPaths")) { + updateBlobDataFromUrls(value.blobUrlPaths, value, blobMap, blobDataMap); + unset(value, "blobUrlPaths"); + } + value = JSON.stringify(value); } - value = new Blob([value], { type: "text/plain" }); + // If there are no blob urls in the value, we can directly add it to the formData + // If there are blob urls, we need to add them to the blobDataMap + if (!useBlobMaps) { + value = new Blob([value], { type: "text/plain" }); + } bindingsMap[key] = `k${i}`; bindingBlob.push({ name: `k${i}`, value: value }); + + // We need to add the blob map to the param properties + // This will allow the server to handle the scenaio of large files upload using blob data + const paramProperties = executeActionRequest.paramProperties[`k${i}`]; + if (!!paramProperties && typeof paramProperties === "object") { + paramProperties["blobIdentifiers"] = blobMap; + } } formData.append("executeActionDTO", JSON.stringify(executeActionRequest)); formData.append("parameterMap", JSON.stringify(bindingsMap)); bindingBlob?.forEach((item) => formData.append(item.name, item.value)); + + // Append blob data map to formData if not empty + if (!isEmpty(blobDataMap)) { + // blobDataMap is used to resolve blob urls of large files as array buffer + // we need to add each blob data to formData as a separate entry + Object.entries(blobDataMap).forEach(([path, blobData]) => + formData.append(path, blobData), + ); + } } export default function* executePluginActionTriggerSaga( diff --git a/app/client/src/widgets/FilePickerWidgetV2/widget/index.tsx b/app/client/src/widgets/FilePickerWidgetV2/widget/index.tsx index e73f65d49c..b7b333dec3 100644 --- a/app/client/src/widgets/FilePickerWidgetV2/widget/index.tsx +++ b/app/client/src/widgets/FilePickerWidgetV2/widget/index.tsx @@ -10,6 +10,7 @@ import UpIcon from "assets/icons/ads/up-arrow.svg"; import { EventType } from "constants/AppsmithActionConstants/ActionConstants"; import { Colors } from "constants/Colors"; import type { WidgetType } from "constants/WidgetConstants"; +import { FILE_SIZE_LIMIT_FOR_BLOBS } from "constants/WidgetConstants"; import { ValidationTypes } from "constants/WidgetValidation"; import type { Stylesheet } from "entities/AppTheming"; import { EvaluationSubstitutionType } from "entities/DataTree/dataTreeFactory"; @@ -689,7 +690,7 @@ class FilePickerWidget extends BaseWidget< const fileCount = this.props.selectedFiles?.length || 0; const fileReaderPromises = files.map((file, index) => { return new Promise((resolve) => { - if (file.size < 5000 * 1000) { + if (file.size < FILE_SIZE_LIMIT_FOR_BLOBS) { const reader = new FileReader(); if (this.props.fileDataType === FileDataTypes.Base64) { reader.readAsDataURL(file.data); diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/dtos/ExecuteActionDTO.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/dtos/ExecuteActionDTO.java index 97473e11f1..7cb3873eff 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/dtos/ExecuteActionDTO.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/dtos/ExecuteActionDTO.java @@ -26,31 +26,28 @@ public class ExecuteActionDTO { /* Sample value of paramProperties "paramProperties": { - "k1": "string", - "k2": "object", - "k3": "number", - "k4": { - "array": [ - "string", - "number", - "string", - "boolean" - ] - }, - "k5": "boolean" + "k0": { + "datatype": "string", + "blobIdentifiers": ["blobUrl1", "blobUrl2"] + } } */ - Map paramProperties; + Map paramProperties; - Map parameterMap; //e.g. {"Text1.text": "k1","Table1.data": "k2", "Api1.data": "k3"} - Map invertParameterMap; //e.g. {"k1":"Text1.text","k2":"Table1.data", "k3": "Api1.data"} + Map parameterMap; // e.g. {"Text1.text": "k1","Table1.data": "k2", "Api1.data": "k3"} + + // This map is where we store the string values of the blob parts for replacement into evaluated value params + Map blobValuesMap; // e.g. {"blobId": "stringified-blob-data"} + + Map invertParameterMap; // e.g. {"k1":"Text1.text","k2":"Table1.data", "k3": "Api1.data"} @JsonIgnore long totalReadableByteCount; public void setParameterMap(Map parameterMap) { this.parameterMap = parameterMap; - invertParameterMap = parameterMap.entrySet().stream() + invertParameterMap = parameterMap.entrySet() + .stream() .collect(Collectors.toMap( Map.Entry::getValue, Map.Entry::getKey diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/dtos/ParamProperty.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/dtos/ParamProperty.java new file mode 100644 index 0000000000..347f77f203 --- /dev/null +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/dtos/ParamProperty.java @@ -0,0 +1,19 @@ +package com.appsmith.external.dtos; + +import lombok.AllArgsConstructor; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; + +import java.util.List; + +@Getter +@Setter +@NoArgsConstructor +@AllArgsConstructor +public class ParamProperty { + + Object datatype; + + List blobIdentifiers; +} diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/Datasource.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/Datasource.java index 21521c5d90..567483410b 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/Datasource.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/Datasource.java @@ -3,7 +3,6 @@ package com.appsmith.external.models; import com.appsmith.external.views.Views; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonView; - import lombok.Getter; import lombok.NoArgsConstructor; import lombok.Setter; @@ -38,7 +37,7 @@ public class Datasource extends BranchAwareDomain { @Transient @JsonView(Views.Public.class) String pluginName; - + //Organizations migrated to workspaces, kept the field as deprecated to support the old migration @Deprecated @JsonView(Views.Public.class) @@ -122,6 +121,7 @@ public class Datasource extends BranchAwareDomain { * Intended to function like `.equals`, but only semantically significant fields, except for the ID. Semantically * significant just means that if two datasource have same values for these fields, actions against them will behave * exactly the same. + * * @return true if equal, false otherwise. */ public boolean softEquals(Datasource other) { diff --git a/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java b/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java index 90bd6b5e05..a17c8e41d1 100644 --- a/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java +++ b/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java @@ -2,6 +2,7 @@ package com.external.plugins; import com.appsmith.external.datatypes.ClientDataType; import com.appsmith.external.dtos.ExecuteActionDTO; +import com.appsmith.external.dtos.ParamProperty; import com.appsmith.external.helpers.PluginUtils; import com.appsmith.external.helpers.restApiUtils.connections.APIConnection; import com.appsmith.external.helpers.restApiUtils.helpers.HintMessageUtils; @@ -600,7 +601,9 @@ public class RestApiPluginTest { param.setPseudoBindingName("k0"); executeActionDTO.setParams(Collections.singletonList(param)); - executeActionDTO.setParamProperties(Collections.singletonMap("k0", "string")); + ParamProperty paramProperty = new ParamProperty(); + paramProperty.setDatatype("string"); + executeActionDTO.setParamProperties(Collections.singletonMap("k0", paramProperty)); executeActionDTO.setParameterMap(Collections.singletonMap("Input1.text", "k0")); executeActionDTO.setInvertParameterMap(Collections.singletonMap("k0", "Input1.text")); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCE.java index 5878d8f07a..2a9f9adf68 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCE.java @@ -1,12 +1,12 @@ package com.appsmith.server.services.ce; import com.appsmith.external.dtos.ExecuteActionDTO; +import com.appsmith.external.models.ActionDTO; import com.appsmith.external.models.ActionExecutionResult; import com.appsmith.external.models.MustacheBindingToken; import com.appsmith.server.acl.AclPermission; import com.appsmith.server.domains.NewAction; import com.appsmith.server.domains.NewPage; -import com.appsmith.external.models.ActionDTO; import com.appsmith.server.dtos.ActionViewDTO; import com.appsmith.server.dtos.LayoutActionUpdateDTO; import com.appsmith.server.services.CrudService; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCEImpl.java index 1ecc3cf179..0e6d3865c5 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCEImpl.java @@ -5,6 +5,7 @@ import com.appsmith.external.datatypes.ClientDataType; import com.appsmith.external.dtos.DatasourceDTO; import com.appsmith.external.dtos.ExecuteActionDTO; import com.appsmith.external.dtos.ExecutePluginDTO; +import com.appsmith.external.dtos.ParamProperty; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException; import com.appsmith.external.exceptions.pluginExceptions.StaleConnectionException; @@ -66,12 +67,14 @@ import com.appsmith.server.solutions.ApplicationPermission; import com.appsmith.server.solutions.DatasourcePermission; import com.appsmith.server.solutions.PagePermission; import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import io.micrometer.observation.ObservationRegistry; import jakarta.validation.Validator; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang.ArrayUtils; +import org.apache.commons.lang.StringEscapeUtils; import org.apache.commons.lang3.ObjectUtils; import org.bson.types.ObjectId; import org.springframework.core.io.buffer.DataBufferUtils; @@ -110,10 +113,20 @@ import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; import static com.appsmith.external.constants.CommonFieldName.REDACTED_DATA; -import static com.appsmith.external.constants.spans.ActionSpans.*; +import static com.appsmith.external.constants.spans.ActionSpans.ACTION_EXECUTION_CACHED_ACTION; +import static com.appsmith.external.constants.spans.ActionSpans.ACTION_EXECUTION_CACHED_DATASOURCE; +import static com.appsmith.external.constants.spans.ActionSpans.ACTION_EXECUTION_CACHED_PLUGIN; +import static com.appsmith.external.constants.spans.ActionSpans.ACTION_EXECUTION_DATASOURCE_CONTEXT; +import static com.appsmith.external.constants.spans.ActionSpans.ACTION_EXECUTION_DATASOURCE_CONTEXT_REMOTE; +import static com.appsmith.external.constants.spans.ActionSpans.ACTION_EXECUTION_EDITOR_CONFIG; +import static com.appsmith.external.constants.spans.ActionSpans.ACTION_EXECUTION_REQUEST_PARSING; +import static com.appsmith.external.constants.spans.ActionSpans.ACTION_EXECUTION_SERVER_EXECUTION; +import static com.appsmith.external.constants.spans.ActionSpans.ACTION_EXECUTION_VALIDATE_AUTHENTICATION; import static com.appsmith.external.helpers.AppsmithBeanUtils.copyNewFieldValuesIntoOldObject; import static com.appsmith.external.helpers.DataTypeStringUtils.getDisplayDataTypes; import static com.appsmith.external.helpers.PluginUtils.setValueSafelyInFormData; @@ -135,6 +148,13 @@ public class NewActionServiceCEImpl extends BaseService patternList = new ArrayList<>(); + private final NewActionRepository repository; private final DatasourceService datasourceService; private final PluginService pluginService; @@ -207,6 +227,11 @@ public class NewActionServiceCEImpl extends BaseService createExecuteActionDTO(Flux partFlux) { final AtomicLong totalReadableByteCount = new AtomicLong(0); final ExecuteActionDTO dto = new ExecuteActionDTO(); - return partFlux - .flatMap(part -> { - final String key = part.name(); - if ("executeActionDTO".equals(key)) { - return DataBufferUtils - .join(part.content()) - .flatMap(executeActionDTOBuffer -> { - byte[] byteData = new byte[executeActionDTOBuffer.readableByteCount()]; - executeActionDTOBuffer.read(byteData); - DataBufferUtils.release(executeActionDTOBuffer); - try { - return Mono.just(objectMapper.readValue(byteData, ExecuteActionDTO.class)); - } catch (IOException e) { - log.error("Error in deserializing ExecuteActionDTO", e); - return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, "executeActionDTO")); - } - }) - .flatMap(executeActionDTO -> { - dto.setActionId(executeActionDTO.getActionId()); - dto.setViewMode(executeActionDTO.getViewMode()); - dto.setParamProperties(executeActionDTO.getParamProperties()); - dto.setPaginationField(executeActionDTO.getPaginationField()); - return Mono.empty(); - }); - } else if ("parameterMap".equals(key)) { - return DataBufferUtils - .join(part.content()) - .flatMap(executeActionDTOBuffer -> { - byte[] byteData = new byte[executeActionDTOBuffer.readableByteCount()]; - executeActionDTOBuffer.read(byteData); - DataBufferUtils.release(executeActionDTOBuffer); - try { - return Mono.just(objectMapper.readValue(byteData, HashMap.class)); - } catch (IOException e) { - return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, "parameterMap")); - } - }) - .flatMap(paramMap -> { - dto.setParameterMap(paramMap); - return Mono.empty(); - }); - } - return Mono.just(part); - }) - .flatMap(part -> { - final Param param = new Param(); - param.setPseudoBindingName(part.name()); - return DataBufferUtils - .join(part.content()) - .map(dataBuffer -> { - byte[] bytes = new byte[dataBuffer.readableByteCount()]; - totalReadableByteCount.addAndGet(dataBuffer.readableByteCount()); - dataBuffer.read(bytes); - DataBufferUtils.release(dataBuffer); - param.setValue(new String(bytes, StandardCharsets.UTF_8)); - return param; - }); - }) + return this.parsePartsAndGetParamsFlux(partFlux, totalReadableByteCount, dto) .collectList() - .flatMap(params -> { - if (dto.getActionId() == null) { - return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.ACTION_ID)); - } - - dto.setTotalReadableByteCount(totalReadableByteCount.longValue()); - - final Set visitedBindings = new HashSet<>(); - /* - Parts in multipart request can appear in any order. In order to avoid NPE original name of the parameters - along with the client-side data type are set here as it's guaranteed at this point that the part having the parameterMap is already collected. - Ref: https://github.com/appsmithorg/appsmith/issues/16722 - */ - params.forEach( - param -> { - String pseudoBindingName = param.getPseudoBindingName(); - String bindingValue = dto.getInvertParameterMap().get(pseudoBindingName); - param.setKey(bindingValue); - visitedBindings.add(bindingValue); - //if the type is not an array e.g. "k1": "string" or "k1": "boolean" - if (dto.getParamProperties() - .get(pseudoBindingName) instanceof String) { - param.setClientDataType(ClientDataType.valueOf(String.valueOf(dto.getParamProperties() - .get(pseudoBindingName)) - .toUpperCase())); - } else if (dto.getParamProperties() - .get(pseudoBindingName) instanceof LinkedHashMap) { - //if the type is an array e.g. "k1": { "array": [ "string", "number", "string", "boolean"] - LinkedHashMap stringArrayListLinkedHashMap = - (LinkedHashMap) dto.getParamProperties() - .get(pseudoBindingName); - Optional firstKeyOpt = stringArrayListLinkedHashMap.keySet() - .stream() - .findFirst(); - if (firstKeyOpt.isPresent()) { - String firstKey = firstKeyOpt.get(); - param.setClientDataType(ClientDataType.valueOf(firstKey.toUpperCase())); - List individualTypes = stringArrayListLinkedHashMap.get(firstKey); - List dataTypesOfArrayElements = - individualTypes.stream() - .map(it -> ClientDataType.valueOf(String.valueOf(it) - .toUpperCase())) - .collect(Collectors.toList()); - param.setDataTypesOfArrayElements(dataTypesOfArrayElements); - } - } - - } - ); - - // In case there are parameters that did not receive a value in the multipart request, - // initialize these bindings with empty strings - if (dto.getParameterMap() != null) { - dto.getParameterMap() - .keySet() - .stream() - .forEach(parameter -> { - if (!visitedBindings.contains(parameter)) { - Param newParam = new Param(parameter, ""); - params.add(newParam); - } - }); - } - dto.setParams(params); - return Mono.just(dto); - }) + .flatMap(params -> this.enrichExecutionParam(totalReadableByteCount, dto, params)) .name(ACTION_EXECUTION_REQUEST_PARSING) .tap(Micrometer.observation(observationRegistry)); } @@ -2329,4 +2232,222 @@ public class NewActionServiceCEImpl extends BaseService parsePartsAndGetParamsFlux(Flux partFlux, AtomicLong totalReadableByteCount, ExecuteActionDTO dto) { + return partFlux + .groupBy(part -> { + // We're grouping parts by the type of processing required + // Expected types: meta, value, blob + + for (Pattern pattern : patternList) { + Matcher matcher = pattern.matcher(part.name()); + if (matcher.find()) { + return pattern.pattern(); + } + } + return part.name(); + }) + .flatMap(groupedPartsFlux -> { + String key = groupedPartsFlux.key(); + return switch (key) { + case PARAM_KEY_REGEX -> + groupedPartsFlux.flatMap(part -> this.parseExecuteParameter(part, totalReadableByteCount)); + case BLOB_KEY_REGEX -> + this.parseExecuteBlobs(groupedPartsFlux, dto, totalReadableByteCount).then(Mono.empty()); + case EXECUTE_ACTION_DTO -> + groupedPartsFlux.next().flatMap(part -> this.parseExecuteActionPart(part, dto)).then(Mono.empty()); + case PARAMETER_MAP -> + groupedPartsFlux.next().flatMap(part -> this.parseExecuteParameterMapPart(part, dto)).then(Mono.empty()); + default -> + Mono.error(new AppsmithException(AppsmithError.GENERIC_BAD_REQUEST, "Unexpected part found: " + key)); + }; + }); + } + + protected Mono enrichExecutionParam(AtomicLong totalReadableByteCount, ExecuteActionDTO dto, List params) { + if (dto.getActionId() == null) { + return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.ACTION_ID)); + } + + dto.setTotalReadableByteCount(totalReadableByteCount.longValue()); + + final Set visitedBindings = new HashSet<>(); + /* + Parts in multipart request can appear in any order. In order to avoid NPE original name of the parameters + along with the client-side data type are set here as it's guaranteed at this point that the part having the parameterMap is already collected. + Ref: https://github.com/appsmithorg/appsmith/issues/16722 + */ + params.forEach( + param -> { + String pseudoBindingName = param.getPseudoBindingName(); + String bindingValue = dto.getInvertParameterMap().get(pseudoBindingName); + param.setKey(bindingValue); + visitedBindings.add(bindingValue); + // if the type is not an array e.g. "k1": "string" or "k1": "boolean" + ParamProperty paramProperty = dto.getParamProperties().get(pseudoBindingName); + if (paramProperty != null) { + this.identifyExecutionParamDatatype(param, paramProperty); + + this.substituteBlobValuesInParam(dto, param, paramProperty); + } + + } + ); + + // In case there are parameters that did not receive a value in the multipart request, + // initialize these bindings with empty strings + if (dto.getParameterMap() != null) { + dto.getParameterMap() + .keySet() + .stream() + .forEach(parameter -> { + if (!visitedBindings.contains(parameter)) { + Param newParam = new Param(parameter, ""); + params.add(newParam); + } + }); + } + dto.setParams(params); + return Mono.just(dto); + } + + private void substituteBlobValuesInParam(ExecuteActionDTO dto, Param param, ParamProperty paramProperty) { + // Check if this param has blobUrlPaths + if (paramProperty.getBlobIdentifiers() != null && !paramProperty.getBlobIdentifiers().isEmpty()) { + // If it does, trigger the replacement logic for each of these urlPaths + String replacedValue = this.replaceBlobValuesInParam( + param.getValue(), + paramProperty.getBlobIdentifiers(), + dto.getBlobValuesMap()); + // And then update the value for this param + param.setValue(replacedValue); + } + } + + private void identifyExecutionParamDatatype(Param param, ParamProperty paramProperty) { + Object datatype = paramProperty.getDatatype(); + if (datatype instanceof String) { + param.setClientDataType(ClientDataType.valueOf(String.valueOf(datatype).toUpperCase())); + } else if (datatype instanceof LinkedHashMap) { + // if the type is an array e.g. "k1": { "array": [ "string", "number", "string", "boolean"] + LinkedHashMap stringArrayListLinkedHashMap = + (LinkedHashMap) datatype; + Optional firstKeyOpt = stringArrayListLinkedHashMap.keySet() + .stream() + .findFirst(); + if (firstKeyOpt.isPresent()) { + String firstKey = firstKeyOpt.get(); + param.setClientDataType(ClientDataType.valueOf(firstKey.toUpperCase())); + List individualTypes = stringArrayListLinkedHashMap.get(firstKey); + List dataTypesOfArrayElements = + individualTypes.stream() + .map(it -> ClientDataType.valueOf(String.valueOf(it) + .toUpperCase())) + .collect(Collectors.toList()); + param.setDataTypesOfArrayElements(dataTypesOfArrayElements); + } + } + } + + protected Mono parseExecuteActionPart(Part part, ExecuteActionDTO dto) { + return DataBufferUtils + .join(part.content()) + .flatMap(executeActionDTOBuffer -> { + byte[] byteData = new byte[executeActionDTOBuffer.readableByteCount()]; + executeActionDTOBuffer.read(byteData); + DataBufferUtils.release(executeActionDTOBuffer); + try { + return Mono.just(objectMapper.readValue(byteData, ExecuteActionDTO.class)); + } catch (IOException e) { + log.error("Error in deserializing ExecuteActionDTO", e); + return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, EXECUTE_ACTION_DTO)); + } + }) + .flatMap(executeActionDTO -> { + dto.setActionId(executeActionDTO.getActionId()); + dto.setViewMode(executeActionDTO.getViewMode()); + dto.setParamProperties(executeActionDTO.getParamProperties()); + dto.setPaginationField(executeActionDTO.getPaginationField()); + return Mono.empty(); + }); + } + + protected Mono parseExecuteParameterMapPart(Part part, ExecuteActionDTO dto) { + return DataBufferUtils + .join(part.content()) + .flatMap(parameterMapBuffer -> { + byte[] byteData = new byte[parameterMapBuffer.readableByteCount()]; + parameterMapBuffer.read(byteData); + DataBufferUtils.release(parameterMapBuffer); + try { + return Mono.just(objectMapper.readValue(byteData, new TypeReference>() { + })); + } catch (IOException e) { + return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, PARAMETER_MAP)); + } + }) + .flatMap(paramMap -> { + dto.setParameterMap(paramMap); + return Mono.empty(); + }); + } + + protected Mono parseExecuteParameter(Part part, AtomicLong totalReadableByteCount) { + final Param param = new Param(); + param.setPseudoBindingName(part.name()); + return DataBufferUtils + .join(part.content()) + .map(dataBuffer -> { + byte[] bytes = new byte[dataBuffer.readableByteCount()]; + totalReadableByteCount.addAndGet(dataBuffer.readableByteCount()); + dataBuffer.read(bytes); + DataBufferUtils.release(dataBuffer); + param.setValue(new String(bytes, StandardCharsets.UTF_8)); + return param; + }); + } + + protected Mono parseExecuteBlobs(Flux partsFlux, ExecuteActionDTO dto, AtomicLong totalReadableByteCount) { + Map blobMap = new HashMap<>(); + dto.setBlobValuesMap(blobMap); + + return partsFlux + .flatMap(part -> { + return DataBufferUtils + .join(part.content()) + .map(dataBuffer -> { + byte[] bytes = new byte[dataBuffer.readableByteCount()]; + totalReadableByteCount.addAndGet(dataBuffer.readableByteCount()); + dataBuffer.read(bytes); + DataBufferUtils.release(dataBuffer); + blobMap.put(part.name(), new String(bytes, StandardCharsets.ISO_8859_1)); + return Mono.empty(); + }); + }) + .then(); + } + + protected String replaceBlobValuesInParam(String value, List blobIdentifiers, Map blobValuesMap) { + // If there is no blobId reference against this param, return as is + if (blobIdentifiers == null || blobIdentifiers.isEmpty()) { + return value; + } + + // Otherwise, for each such blobId reference, replace the reference with the actual value from the blobMap + for (String blobId : blobIdentifiers) { + value = value.replace(blobId, StringEscapeUtils.escapeJava(blobValuesMap.get(blobId))); + } + + return value; + } + } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/NewActionServiceCEImplTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/NewActionServiceCEImplTest.java index 9d140a1545..c42a664000 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/NewActionServiceCEImplTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/NewActionServiceCEImplTest.java @@ -1,8 +1,12 @@ package com.appsmith.server.services.ce; +import com.appsmith.external.datatypes.ClientDataType; +import com.appsmith.external.dtos.ExecuteActionDTO; +import com.appsmith.external.dtos.ParamProperty; import com.appsmith.external.models.ActionDTO; import com.appsmith.external.models.ActionExecutionResult; import com.appsmith.external.models.Datasource; +import com.appsmith.external.models.Param; import com.appsmith.external.models.PluginType; import com.appsmith.server.acl.PolicyGenerator; import com.appsmith.server.constants.FieldName; @@ -67,8 +71,10 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.concurrent.atomic.AtomicLong; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; @@ -80,7 +86,7 @@ import static org.mockito.Mockito.spy; @Slf4j public class NewActionServiceCEImplTest { - NewActionServiceCE newActionService; + NewActionServiceCEImpl newActionService; @MockBean Scheduler scheduler; @@ -219,9 +225,13 @@ public class NewActionServiceCEImplTest { MockServerHttpRequest mock = MockServerHttpRequest .method(HttpMethod.POST, URI.create("https://example.com")) .contentType(new MediaType("multipart", "form-data", Map.of("boundary", "boundary"))) - .body("--boundary\r\n" + - "Content-Disposition: form-data; name=\"executeActionDTO\"\r\n" + "\r\n" + "irrelevant content\r\n" + - "--boundary--\r\n"); + .body(""" + --boundary\r + Content-Disposition: form-data; name="executeActionDTO"\r + \r + irrelevant content\r + --boundary--\r + """); final Flux partsFlux = BodyExtractors.toParts() .extract(mock, this.context); @@ -240,9 +250,13 @@ public class NewActionServiceCEImplTest { MockServerHttpRequest mock = MockServerHttpRequest .method(HttpMethod.POST, URI.create("https://example.com")) .contentType(new MediaType("multipart", "form-data", Map.of("boundary", "boundary"))) - .body("--boundary\r\n" + - "Content-Disposition: form-data; name=\"executeActionDTO\"\r\n" + "\r\n" + "{\"viewMode\":false}\r\n" + - "--boundary--\r\n"); + .body(""" + --boundary\r + Content-Disposition: form-data; name="executeActionDTO"\r + \r + {"viewMode":false}\r + --boundary--\r + """); final Flux partsFlux = BodyExtractors.toParts() .extract(mock, this.context); @@ -312,20 +326,21 @@ public class NewActionServiceCEImplTest { @Test public void testExecuteAPIWithUsualOrderingOfTheParts() { - String usualOrderOfParts = "--boundary\r\n" + - "Content-Disposition: form-data; name=\"executeActionDTO\"\r\n" + - "\r\n" + - "{\"actionId\":\"63285a3388e48972c7519b18\",\"viewMode\":false,\"paramProperties\":{\"k0\":\"string\"}}\r\n" + - "--boundary\r\n" + - "Content-Disposition: form-data; name=\"parameterMap\"\r\n" + - "\r\n" + - "{\"Input1.text\":\"k0\"}\r\n" + - "--boundary\r\n" + - "Content-Disposition: form-data; name=\"k0\"; filename=\"blob\"\r\n" + - "Content-Type: text/plain\r\n" + - "\r\n" + - "xyz\r\n" + - "--boundary--"; + String usualOrderOfParts = """ + --boundary\r + Content-Disposition: form-data; name="executeActionDTO"\r + \r + {"actionId":"63285a3388e48972c7519b18","viewMode":false,"paramProperties":{"k0":{"datatype": "string"}}}\r + --boundary\r + Content-Disposition: form-data; name="parameterMap"\r + \r + {"Input1.text":"k0"}\r + --boundary\r + Content-Disposition: form-data; name="k0"; filename="blob"\r + Content-Type: text/plain\r + \r + xyz\r + --boundary--"""; MockServerHttpRequest mock = MockServerHttpRequest .method(HttpMethod.POST, URI.create("https://example.com")) @@ -353,7 +368,7 @@ public class NewActionServiceCEImplTest { StepVerifier .create(actionExecutionResultMono) .assertNext(response -> { - assertTrue(response instanceof ActionExecutionResult); + assertNotNull(response); assertTrue(response.getIsExecutionSuccess()); assertEquals(mockResult.getBody().toString(), response.getBody().toString()); }) @@ -362,20 +377,21 @@ public class NewActionServiceCEImplTest { @Test public void testExecuteAPIWithParameterMapAsLastPart() { - String parameterMapAtLast = "--boundary\r\n" + - "Content-Disposition: form-data; name=\"executeActionDTO\"\r\n" + - "\r\n" + - "{\"actionId\":\"63285a3388e48972c7519b18\",\"viewMode\":false,\"paramProperties\":{\"k0\":\"string\"}}\r\n" + - "--boundary\r\n" + - "Content-Disposition: form-data; name=\"k0\"; filename=\"blob\"\r\n" + - "Content-Type: text/plain\r\n" + - "\r\n" + - "xyz\r\n" + - "--boundary\r\n" + - "Content-Disposition: form-data; name=\"parameterMap\"\r\n" + - "\r\n" + - "{\"Input1.text\":\"k0\"}\r\n" + - "--boundary--"; + String parameterMapAtLast = """ + --boundary\r + Content-Disposition: form-data; name="executeActionDTO"\r + \r + {"actionId":"63285a3388e48972c7519b18","viewMode":false,"paramProperties":{"k0":{"datatype": "string"}}}\r + --boundary\r + Content-Disposition: form-data; name="k0"; filename="blob"\r + Content-Type: text/plain\r + \r + xyz\r + --boundary\r + Content-Disposition: form-data; name="parameterMap"\r + \r + {"Input1.text":"k0"}\r + --boundary--"""; MockServerHttpRequest mock = MockServerHttpRequest .method(HttpMethod.POST, URI.create("https://example.com")) @@ -403,10 +419,81 @@ public class NewActionServiceCEImplTest { StepVerifier .create(actionExecutionResultMono) .assertNext(response -> { - assertTrue(response instanceof ActionExecutionResult); + assertNotNull(response); assertTrue(response.getIsExecutionSuccess()); assertEquals(mockResult.getBody().toString(), response.getBody().toString()); }) .verifyComplete(); } + + @Test + public void testParsePartsAndGetParamsFlux_withBlobIdentifiers_replacesValueInParam() { + String partsWithBlobRefs = """ + --boundary\r + Content-Disposition: form-data; name="executeActionDTO"\r + \r + {"actionId":"63285a3388e48972c7519b18","viewMode":false,"paramProperties":{"k0":{"datatype": "string", "blobIdentifiers": ["blob:12345678-1234-1234-1234-123456781234"]}}}\r + --boundary\r + Content-Disposition: form-data; name="parameterMap"\r + \r + {"Input1.text":"k0"}\r + --boundary\r + Content-Disposition: form-data; name="k0"; filename="blob"\r + Content-Type: text/plain\r + \r + {"name": "randomName", "data": "blob:12345678-1234-1234-1234-123456781234"}\r + --boundary\r + Content-Disposition: form-data; name="blob:12345678-1234-1234-1234-123456781234"; filename="blob"\r + Content-Type: text/plain\r + \r + xy\\nz\r + --boundary--"""; + + MockServerHttpRequest mock = MockServerHttpRequest + .method(HttpMethod.POST, URI.create("https://example.com")) + .contentType(new MediaType("multipart", "form-data", Map.of("boundary", "boundary"))) + .body(partsWithBlobRefs); + + final Flux partsFlux = BodyExtractors.toParts() + .extract(mock, this.context); + + AtomicLong atomicLong = new AtomicLong(); + ExecuteActionDTO executeActionDTO = new ExecuteActionDTO(); + Mono> paramsListMono = newActionService.parsePartsAndGetParamsFlux(partsFlux, atomicLong, executeActionDTO).collectList().cache(); + + StepVerifier.create(paramsListMono) + .assertNext(paramsList -> { + assertEquals(1, paramsList.size()); + Param param = paramsList.get(0); + assertEquals("{\"name\": \"randomName\", \"data\": \"blob:12345678-1234-1234-1234-123456781234\"}", param.getValue()); + }) + .verifyComplete(); + } + + @Test + public void testEnrichExecutionParams_withBlobReference_performsSubstitutionCorrectly() { + AtomicLong atomicLong = new AtomicLong(45L); + ExecuteActionDTO executeActionDTO = new ExecuteActionDTO(); + executeActionDTO.setActionId("testId"); + executeActionDTO.setViewMode(false); + executeActionDTO.setParamProperties(Map.of("k0", new ParamProperty("string", List.of("blobId")))); + executeActionDTO.setParameterMap(Map.of("Input1.text", "k0")); + executeActionDTO.setBlobValuesMap(Map.of("blobId", "xy\\nz")); + Param param1 = new Param(); + param1.setValue("{\"name\": \"randomName\", \"data\": \"blobId\"}"); + param1.setPseudoBindingName("k0"); + List params = List.of(param1); + + Mono enrichedDto = newActionService.enrichExecutionParam(atomicLong, executeActionDTO, params); + + StepVerifier.create(enrichedDto) + .assertNext(dto -> { + assertEquals(45, dto.getTotalReadableByteCount()); + + Param param = dto.getParams().get(0); + assertEquals(ClientDataType.STRING, param.getClientDataType()); + assertEquals("{\"name\": \"randomName\", \"data\": \"xy\\\\nz\"}", param.getValue()); + }) + .verifyComplete(); + } } \ No newline at end of file