fix: enhance null handling in action parameter evaluation and rendering (#40910)
## Description This update improves the handling of null values in action parameter evaluation and rendering processes. Specifically, it ensures that null values are treated appropriately, preventing unnecessary stringification and Blob creation. This change enhances the robustness of the action execution flow and prevents potential errors related to null values. Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.All" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/15555494788> > Commit: b869dfa397737ef46a940ed82d0f7b92d1518bc4 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=15555494788&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Tue, 10 Jun 2025 10:21:02 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved handling of null and empty parameter values during action execution, ensuring that nulls are preserved and not incorrectly converted to strings or empty values. - Enhanced template rendering to better preserve original formatting when bindings are missing. - **New Features** - Added new logic to sanitize null parameter values before action execution for more consistent processing. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Nilesh Sarupriya <20905988+nsarupr@users.noreply.github.com>
This commit is contained in:
parent
f54592e5e5
commit
5d5f7ae665
|
|
@ -489,14 +489,25 @@ function* evaluateActionParams(
|
|||
evaluatedParams[key] = "blob";
|
||||
}
|
||||
|
||||
value = JSON.stringify(value);
|
||||
evaluatedParams[key] = value;
|
||||
// Handle null values separately to avoid stringifying them
|
||||
if (value === null) {
|
||||
value = null;
|
||||
evaluatedParams[key] = null;
|
||||
} else {
|
||||
value = JSON.stringify(value);
|
||||
evaluatedParams[key] = value;
|
||||
}
|
||||
}
|
||||
|
||||
// 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" });
|
||||
// Handle null values separately to avoid creating a Blob with "null" string
|
||||
if (value === null) {
|
||||
value = null;
|
||||
} else {
|
||||
value = new Blob([value], { type: "text/plain" });
|
||||
}
|
||||
}
|
||||
|
||||
bindingsMap[key] = `k${i}`;
|
||||
|
|
|
|||
|
|
@ -341,13 +341,18 @@ public class MustacheHelper {
|
|||
// text
|
||||
// and hence reflecting the value in the rendered string as is.
|
||||
// Example: {{Input.text}} = "This whole string is the value of Input1.text. Even this {{one}}."
|
||||
String bindingValue = keyValueMap.get(token.getValue()
|
||||
String tokenSubstring = token.getValue()
|
||||
.substring(2, token.getValue().length() - 2)
|
||||
.trim());
|
||||
if (bindingValue != null) {
|
||||
rendered.append(bindingValue);
|
||||
} else {
|
||||
.trim();
|
||||
String bindingValue = keyValueMap.get(tokenSubstring);
|
||||
if (!keyValueMap.containsKey(tokenSubstring)) {
|
||||
rendered.append(token.getValue());
|
||||
} else if (bindingValue != null) {
|
||||
// If the binding value is not null, then append the binding value to the rendered string.
|
||||
// We are using the token.getValue() here to get the original value of the binding.
|
||||
// This is because we want to preserve the original formatting of the binding.
|
||||
// For example, if the binding is {{Input.text}}, we want to preserve the {{}} around it.
|
||||
rendered.append(bindingValue);
|
||||
}
|
||||
} else {
|
||||
rendered.append(token.getValue());
|
||||
|
|
|
|||
|
|
@ -22,12 +22,13 @@ import reactor.core.publisher.Mono;
|
|||
import reactor.core.scheduler.Schedulers;
|
||||
import reactor.util.function.Tuple2;
|
||||
|
||||
import java.util.HashMap;
|
||||
import java.util.HashSet;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Objects;
|
||||
import java.util.Properties;
|
||||
import java.util.Set;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
import static com.appsmith.external.constants.spans.ActionSpan.ACTION_EXECUTION_PLUGIN_EXECUTION;
|
||||
import static com.appsmith.external.helpers.PluginUtils.getHintMessageForLocalhostUrl;
|
||||
|
|
@ -304,6 +305,7 @@ public interface PluginExecutor<C> extends ExtensionPoint, CrudTemplateService {
|
|||
ActionConfiguration actionConfiguration,
|
||||
ObservationRegistry observationRegistry,
|
||||
Map<String, Boolean> featureFlagMap) {
|
||||
this.sanitiseNullsInParams(executeActionDTO);
|
||||
return this.executeParameterizedWithFlags(
|
||||
connection, executeActionDTO, datasourceConfiguration, actionConfiguration, featureFlagMap)
|
||||
.tag("plugin", this.getClass().getName())
|
||||
|
|
@ -311,6 +313,17 @@ public interface PluginExecutor<C> extends ExtensionPoint, CrudTemplateService {
|
|||
.tap(Micrometer.observation(observationRegistry));
|
||||
}
|
||||
|
||||
default void sanitiseNullsInParams(ExecuteActionDTO executeActionDTO) {
|
||||
String stringNull = "null";
|
||||
if (executeActionDTO != null && !isEmpty(executeActionDTO.getParams())) {
|
||||
executeActionDTO.getParams().stream().forEach(param -> {
|
||||
if (Objects.isNull(param.getValue())) {
|
||||
param.setValue(stringNull);
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
default Mono<TriggerResultDTO> triggerWithFlags(
|
||||
C connection,
|
||||
DatasourceConfiguration datasourceConfiguration,
|
||||
|
|
@ -377,17 +390,21 @@ public interface PluginExecutor<C> extends ExtensionPoint, CrudTemplateService {
|
|||
// Do variable substitution
|
||||
// Do this only if params have been provided in the execute command
|
||||
if (executeActionDTO != null && !isEmpty(executeActionDTO.getParams())) {
|
||||
Map<String, String> replaceParamsMap = executeActionDTO.getParams().stream()
|
||||
.collect(Collectors.toMap(
|
||||
// Trimming here for good measure. If the keys have space on either side,
|
||||
// Mustache won't be able to find the key.
|
||||
// We also add a backslash before every double-quote or backslash character
|
||||
// because we apply the template replacing in a JSON-stringified version of
|
||||
// these properties, where these two characters are escaped.
|
||||
p -> p.getKey().trim(), // .replaceAll("[\"\n\\\\]", "\\\\$0"),
|
||||
Param::getValue,
|
||||
// In case of a conflict, we pick the older value
|
||||
(oldValue, newValue) -> oldValue));
|
||||
Map<String, String> replaceParamsMap = new HashMap<>();
|
||||
for (Param p : executeActionDTO.getParams()) {
|
||||
if (p != null && p.getKey() != null) {
|
||||
String key = p.getKey().trim();
|
||||
// We also add a backslash before every double-quote or backslash character
|
||||
// because we apply the template replacing in a JSON-stringified version of
|
||||
// these properties, where these two characters are escaped.
|
||||
// The original comment about .replaceAll("["\n\\]", "\\$0") for the key
|
||||
// is preserved here for context, though not directly applied in this simplified key trim.
|
||||
String value = p.getValue(); // This will now correctly store null if p.getValue() is null
|
||||
|
||||
// In case of a conflict, putIfAbsent picks the older value (the one already in the map)
|
||||
replaceParamsMap.putIfAbsent(key, value);
|
||||
}
|
||||
}
|
||||
|
||||
MustacheHelper.renderFieldValues(datasourceConfiguration, replaceParamsMap);
|
||||
MustacheHelper.renderFieldValues(actionConfiguration, replaceParamsMap);
|
||||
|
|
|
|||
|
|
@ -63,6 +63,7 @@ import org.apache.commons.lang.StringEscapeUtils;
|
|||
import org.apache.commons.lang3.ObjectUtils;
|
||||
import org.springframework.core.io.buffer.DataBufferUtils;
|
||||
import org.springframework.http.HttpHeaders;
|
||||
import org.springframework.http.codec.multipart.FormFieldPart;
|
||||
import org.springframework.http.codec.multipart.Part;
|
||||
import org.springframework.util.CollectionUtils;
|
||||
import org.springframework.util.StringUtils;
|
||||
|
|
@ -341,8 +342,8 @@ public class ActionExecutionSolutionCEImpl implements ActionExecutionSolutionCE
|
|||
@Override
|
||||
public Mono<ActionExecutionResult> executeAction(
|
||||
ExecuteActionDTO executeActionDTO, ExecuteActionMetaDTO executeActionMetaDTO) {
|
||||
// 1. Validate input parameters which are required for mustache replacements
|
||||
replaceNullWithQuotesForParamValues(executeActionDTO.getParams());
|
||||
|
||||
// Earlier, here we were replacing null with quotes for param values.
|
||||
|
||||
AtomicReference<String> actionName = new AtomicReference<>();
|
||||
actionName.set("");
|
||||
|
|
@ -505,6 +506,16 @@ public class ActionExecutionSolutionCEImpl implements ActionExecutionSolutionCE
|
|||
protected Mono<Param> parseExecuteParameter(Part part, AtomicLong totalReadableByteCount) {
|
||||
final Param param = new Param();
|
||||
param.setPseudoBindingName(part.name());
|
||||
|
||||
if (part instanceof FormFieldPart) {
|
||||
FormFieldPart formFieldPart = (FormFieldPart) part;
|
||||
if ("null".equals(formFieldPart.value())) {
|
||||
param.setValue(null);
|
||||
} else if (formFieldPart.value().isEmpty()) {
|
||||
param.setValue(formFieldPart.value());
|
||||
}
|
||||
return Mono.just(param);
|
||||
}
|
||||
return DataBufferUtils.join(part.content()).map(dataBuffer -> {
|
||||
byte[] bytes = new byte[dataBuffer.readableByteCount()];
|
||||
totalReadableByteCount.addAndGet(dataBuffer.readableByteCount());
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user