From 7787a0ddacc7f32047b3f0cf8cf0e846a4b7ea90 Mon Sep 17 00:00:00 2001 From: Sumit Kumar Date: Mon, 21 Jun 2021 19:36:06 +0530 Subject: [PATCH] Bug Fix: Fix dynamic binding substitution failure with Firestore `where` condition input boxes. (#5280) Modify dynamic binding substitution method to handle list and map types containing generic type values. --- .../external/dtos/ExecuteActionDTO.java | 2 + .../external/helpers/MustacheHelper.java | 87 ++++++++++--------- .../com/appsmith/external/models/Param.java | 4 + .../external/plugins/FirestorePluginTest.java | 85 ++++++++++++++++-- 4 files changed, 131 insertions(+), 47 deletions(-) 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 6565a8109c..60d3c0307a 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 @@ -4,11 +4,13 @@ import com.appsmith.external.models.PaginationField; import com.appsmith.external.models.Param; import lombok.Getter; import lombok.Setter; +import lombok.ToString; import java.util.List; @Getter @Setter +@ToString public class ExecuteActionDTO { String actionId; diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/MustacheHelper.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/MustacheHelper.java index afa5dbd5cc..3dc8147ee7 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/MustacheHelper.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/MustacheHelper.java @@ -4,12 +4,14 @@ import com.appsmith.external.models.ActionConfiguration; import lombok.extern.slf4j.Slf4j; import org.apache.commons.text.StringEscapeUtils; import org.springframework.beans.BeanWrapper; +import org.springframework.beans.BeansException; import org.springframework.beans.PropertyAccessorFactory; import org.springframework.util.StringUtils; import java.beans.PropertyDescriptor; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.LinkedList; import java.util.List; @@ -255,56 +257,57 @@ public class MustacheHelper { } } + /** + * - If object is null, then return object. + * - If object is an Appsmith domain object then iterate over all fields of the object and render field values + * for each of them. + * - If object is list type then iterate over each item in the list and render field value for them. + * - If object is map type, then iterate over each value in the map and render field value for them. + * - If the object is string type (base case), then do the binding substitution if applicable. + * - If the object falls under none of the above conditions then return the object without doing anything. + */ public static T renderFieldValues(T object, Map context) { - final BeanWrapper sourceBeanWrapper = PropertyAccessorFactory.forBeanPropertyAccess(object); + if (object == null) { + return object; + } - try { - - for (PropertyDescriptor propertyDescriptor : sourceBeanWrapper.getPropertyDescriptors()) { - // For properties like `class` that don't have a set method, just ignore them. - if (propertyDescriptor.getWriteMethod() == null) { - continue; - } - - String name = propertyDescriptor.getName(); - Object value = sourceBeanWrapper.getPropertyValue(name); - - // If value is null, don't copy it over to target and just move on to the next property. - if (value == null) { - continue; - } - - if (isDomainModel(propertyDescriptor.getPropertyType())) { - // Go deeper *only* if the property belongs to Appsmith's models, and both the source and target - // values are not null. - renderFieldValues(value, context); - - } else if (value instanceof List) { - for (Object childValue : (List) value) { - if (childValue != null && isDomainModel(childValue.getClass())) { - renderFieldValues(childValue, context); - } + if (isDomainModel(object.getClass())) { + try { + final BeanWrapper sourceBeanWrapper = PropertyAccessorFactory.forBeanPropertyAccess(object); + for (PropertyDescriptor propertyDescriptor : sourceBeanWrapper.getPropertyDescriptors()) { + // For properties like `class` that don't have a set method, just ignore them. + if (propertyDescriptor.getWriteMethod() == null) { + continue; } - } else if (value instanceof Map) { - for (Object childValue : ((Map) value).values()) { - if (childValue != null && isDomainModel(childValue.getClass())) { - renderFieldValues(childValue, context); - } - } - - } else if (value instanceof String) { - sourceBeanWrapper.setPropertyValue( - name, - render((String) value, context) - ); - + String name = propertyDescriptor.getName(); + Object value = sourceBeanWrapper.getPropertyValue(name); + sourceBeanWrapper.setPropertyValue(name, renderFieldValues(value, context)); } + } catch (BeansException e) { + log.error("Exception caught while substituting values in mustache template.", e); + } + } else if (object instanceof List) { + List renderedList = new ArrayList(); + for (Object childValue : (List) object) { + renderedList.add(renderFieldValues(childValue, context)); } - } catch (Exception e) { - log.error("Exception caught while substituting values in mustache template.", e); + return (T) renderedList; + } else if (object instanceof Map) { + Map renderedMap = new HashMap(); + for (Object entry : ((Map)object).entrySet()) { + renderedMap.put( + ((Map.Entry) entry).getKey(), // key + renderFieldValues(((Map.Entry) entry).getValue(), context) // value + ); + } + + return (T) renderedMap; + + } else if (object instanceof String) { + return (T) render((String) object, context); } return object; diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/Param.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/Param.java index 2e4d8278eb..5c4bb9234e 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/Param.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/Param.java @@ -1,10 +1,14 @@ package com.appsmith.external.models; +import lombok.AllArgsConstructor; import lombok.Getter; +import lombok.NoArgsConstructor; import lombok.Setter; +import lombok.ToString; @Getter @Setter +@ToString public class Param { String key; diff --git a/app/server/appsmith-plugins/firestorePlugin/src/test/java/com/external/plugins/FirestorePluginTest.java b/app/server/appsmith-plugins/firestorePlugin/src/test/java/com/external/plugins/FirestorePluginTest.java index 2a0eba77fa..fd884b70c8 100644 --- a/app/server/appsmith-plugins/firestorePlugin/src/test/java/com/external/plugins/FirestorePluginTest.java +++ b/app/server/appsmith-plugins/firestorePlugin/src/test/java/com/external/plugins/FirestorePluginTest.java @@ -9,6 +9,7 @@ import com.appsmith.external.models.ActionExecutionResult; import com.appsmith.external.models.DBAuth; import com.appsmith.external.models.DatasourceConfiguration; import com.appsmith.external.models.PaginationField; +import com.appsmith.external.models.Param; import com.appsmith.external.models.Property; import com.appsmith.external.models.RequestParamDTO; import com.fasterxml.jackson.core.JsonProcessingException; @@ -710,9 +711,9 @@ public class FirestorePluginTest { * - this returns 2 documents. */ ((List)whereProperty.getValue()).add(new HashMap() {{ - put("path", "category"); + put("path", "{{Input1.text}}"); put("operator", "EQ"); - put("value", "test"); + put("value", "{{Input2.text}}"); }}); /* @@ -720,16 +721,36 @@ public class FirestorePluginTest { * - Of the two documents returned by above condition, this will narrow it down to one. */ ((List)whereProperty.getValue()).add(new HashMap() {{ - put("path", "name"); + put("path", "{{Input3.text}}"); put("operator", "EQ"); - put("value", "two"); + put("value", "{{Input4.text}}"); }}); pluginSpecifiedTemplates.add(whereProperty); actionConfiguration.setPluginSpecifiedTemplates(pluginSpecifiedTemplates); + List params = new ArrayList(); + Param param = new Param(); + param.setKey("Input1.text"); + param.setValue("category"); + params.add(param); + param = new Param(); + param.setKey("Input2.text"); + param.setValue("test"); + params.add(param); + param = new Param(); + param.setKey("Input3.text"); + param.setValue("name"); + params.add(param); + param = new Param(); + param.setKey("Input4.text"); + param.setValue("two"); + params.add(param); + + ExecuteActionDTO executeActionDTO = new ExecuteActionDTO(); + executeActionDTO.setParams(params); Mono resultMono = pluginExecutor - .executeParameterized(firestoreConnection, null, dsConfig, actionConfiguration); + .executeParameterized(firestoreConnection, executeActionDTO, dsConfig, actionConfiguration); StepVerifier.create(resultMono) .assertNext(result -> { @@ -755,6 +776,7 @@ public class FirestorePluginTest { .verifyComplete(); } + @Test public void testUpdateDocumentWithFieldValueTimestamp() { List properties = new ArrayList<>(); properties.add(new Property("method", "UPDATE_DOCUMENT")); // index 0 @@ -1011,4 +1033,57 @@ public class FirestorePluginTest { }) .verifyComplete(); } + + @Test + public void testDynamicBindingSubstitutionInActionConfiguration() { + ActionConfiguration actionConfiguration = new ActionConfiguration(); + actionConfiguration.setPath("{{Input1.text}}"); + List pluginSpecifiedTemplates = new ArrayList<>(); + pluginSpecifiedTemplates.add(new Property("method", "GET_COLLECTION")); + pluginSpecifiedTemplates.add(new Property("order", null)); + pluginSpecifiedTemplates.add(new Property("limit", null)); + Property whereProperty = new Property("where", null); + whereProperty.setValue(new ArrayList<>()); + /* + * - get all documents where category == test. + * - this returns 2 documents. + */ + ((List)whereProperty.getValue()).add(new HashMap() {{ + put("path", "{{Input2.text}}"); + put("operator", "EQ"); + put("value", "{{Input3.text}}"); + }}); + + pluginSpecifiedTemplates.add(whereProperty); + actionConfiguration.setPluginSpecifiedTemplates(pluginSpecifiedTemplates); + + List params = new ArrayList(); + Param param = new Param(); + param.setKey("Input1.text"); + param.setValue("initial"); + params.add(param); + param = new Param(); + param.setKey("Input2.text"); + param.setValue("category"); + params.add(param); + param = new Param(); + param.setKey("Input3.text"); + param.setValue("test"); + params.add(param); + ExecuteActionDTO executeActionDTO = new ExecuteActionDTO(); + executeActionDTO.setParams(params); + + // Substitute dynamic binding values + pluginExecutor + .prepareConfigurationsForExecution(executeActionDTO, actionConfiguration, null); + + // check if dynamic binding values have been substituted correctly + assertEquals("initial", actionConfiguration.getPath()); + assertEquals("category", + ((Map)((List)actionConfiguration.getPluginSpecifiedTemplates().get(3).getValue()).get(0)).get( + "path")); + assertEquals("test", + ((Map)((List)actionConfiguration.getPluginSpecifiedTemplates().get(3).getValue()).get(0)).get( + "value")); + } }