diff --git a/app/server/appsmith-plugins/firestorePlugin/src/main/java/com/external/constants/FieldName.java b/app/server/appsmith-plugins/firestorePlugin/src/main/java/com/external/constants/FieldName.java index 9a6a27e69f..2688184ed5 100644 --- a/app/server/appsmith-plugins/firestorePlugin/src/main/java/com/external/constants/FieldName.java +++ b/app/server/appsmith-plugins/firestorePlugin/src/main/java/com/external/constants/FieldName.java @@ -16,6 +16,7 @@ public class FieldName { public static final String END_BEFORE = "endBefore.data"; public static final String WHERE = "where.data"; public static final String CHILDREN = "children"; + public static final String SMART_SUBSTITUTION = "smartSubstitution"; public static final String WHERE_CHILDREN = WHERE + "." + CHILDREN; } diff --git a/app/server/appsmith-plugins/firestorePlugin/src/main/java/com/external/plugins/FirestorePlugin.java b/app/server/appsmith-plugins/firestorePlugin/src/main/java/com/external/plugins/FirestorePlugin.java index a32a92e66e..62a91d0122 100644 --- a/app/server/appsmith-plugins/firestorePlugin/src/main/java/com/external/plugins/FirestorePlugin.java +++ b/app/server/appsmith-plugins/firestorePlugin/src/main/java/com/external/plugins/FirestorePlugin.java @@ -3,6 +3,8 @@ package com.external.plugins; import com.appsmith.external.dtos.ExecuteActionDTO; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException; +import com.appsmith.external.helpers.DataTypeStringUtils; +import com.appsmith.external.helpers.MustacheHelper; import com.appsmith.external.models.ActionConfiguration; import com.appsmith.external.models.ActionExecutionRequest; import com.appsmith.external.models.ActionExecutionResult; @@ -14,6 +16,7 @@ import com.appsmith.external.models.PaginationField; import com.appsmith.external.models.RequestParamDTO; import com.appsmith.external.plugins.BasePlugin; import com.appsmith.external.plugins.PluginExecutor; +import com.appsmith.external.plugins.SmartSubstitutionInterface; import com.fasterxml.jackson.core.type.TypeReference; import com.google.api.core.ApiFuture; import com.google.auth.oauth2.GoogleCredentials; @@ -57,6 +60,7 @@ import static com.appsmith.external.constants.ActionConstants.ACTION_CONFIGURATI import static com.appsmith.external.constants.ActionConstants.ACTION_CONFIGURATION_PATH; import static com.appsmith.external.helpers.PluginUtils.getValueSafelyFromFormData; import static com.appsmith.external.helpers.PluginUtils.getValueSafelyFromFormDataOrDefault; +import static com.appsmith.external.helpers.PluginUtils.setValueSafelyInFormData; import static com.external.constants.FieldName.BODY; import static com.external.constants.FieldName.COMMAND; import static com.external.constants.FieldName.DELETE_KEY_PATH; @@ -64,6 +68,7 @@ import static com.external.constants.FieldName.END_BEFORE; import static com.external.constants.FieldName.LIMIT_DOCUMENTS; import static com.external.constants.FieldName.NEXT; import static com.external.constants.FieldName.ORDER_BY; +import static com.external.constants.FieldName.SMART_SUBSTITUTION; import static com.external.constants.FieldName.PATH; import static com.external.constants.FieldName.PREV; import static com.external.constants.FieldName.START_AFTER; @@ -71,6 +76,7 @@ import static com.external.constants.FieldName.TIMESTAMP_VALUE_PATH; import static com.external.constants.FieldName.WHERE; import static com.external.constants.FieldName.WHERE_CHILDREN; import static com.external.utils.WhereConditionUtils.applyWhereConditional; +import static java.lang.Boolean.TRUE; import static org.apache.commons.lang3.StringUtils.isBlank; /** @@ -92,7 +98,7 @@ public class FirestorePlugin extends BasePlugin { @Slf4j @Extension - public static class FirestorePluginExecutor implements PluginExecutor { + public static class FirestorePluginExecutor implements PluginExecutor, SmartSubstitutionInterface { private final Scheduler scheduler = Schedulers.elastic(); @@ -104,6 +110,18 @@ public class FirestorePlugin extends BasePlugin { return Mono.error(new AppsmithPluginException(AppsmithPluginError.PLUGIN_ERROR, "Unsupported Operation")); } + @Override + public Object substituteValueInInput(int index, + String binding, + String value, + Object input, + List> insertedParams, + Object... args) { + String jsonBody = (String) input; + return DataTypeStringUtils.jsonSmartReplacementPlaceholderWithValue(jsonBody, value, null, insertedParams, + null); + } + @Override public Mono executeParameterized( Firestore connection, @@ -111,6 +129,44 @@ public class FirestorePlugin extends BasePlugin { DatasourceConfiguration datasourceConfiguration, ActionConfiguration actionConfiguration) { + Object smartSubstitutionObject = actionConfiguration.getFormData().getOrDefault(SMART_SUBSTITUTION, TRUE); + Boolean smartBsonSubstitution = TRUE; + if (smartSubstitutionObject instanceof Boolean) { + smartBsonSubstitution = (Boolean) smartSubstitutionObject; + } else if (smartSubstitutionObject instanceof String) { + // Older UI configuration used to set this value as a string which may/may not be castable to a boolean + // directly. This is to ensure we are backward compatible + smartBsonSubstitution = Boolean.parseBoolean((String) smartSubstitutionObject); + } + + // Smartly substitute in actionConfiguration.body and replace all the bindings with values. + List> parameters = new ArrayList<>(); + if (TRUE.equals(smartBsonSubstitution)) { + String query = getValueSafelyFromFormData(actionConfiguration.getFormData(), BODY, String.class); + if (query != null) { + + // First extract all the bindings in order + List mustacheKeysInOrder = MustacheHelper.extractMustacheKeysInOrder(query); + // Replace all the bindings with a ? as expected in a prepared statement. + String updatedQuery = MustacheHelper.replaceMustacheWithPlaceholder(query, mustacheKeysInOrder); + + try { + updatedQuery = (String) smartSubstitutionOfBindings(updatedQuery, + mustacheKeysInOrder, + executeActionDTO.getParams(), + parameters); + } catch (AppsmithPluginException e) { + ActionExecutionResult errorResult = new ActionExecutionResult(); + errorResult.setIsExecutionSuccess(false); + errorResult.setErrorInfo(e); + errorResult.setStatusCode(AppsmithPluginError.PLUGIN_ERROR.getAppErrorCode().toString()); + return Mono.just(errorResult); + } + + setValueSafelyInFormData(actionConfiguration.getFormData(), BODY, updatedQuery); + } + } + // Do the template substitutions. prepareConfigurationsForExecution(executeActionDTO, actionConfiguration, datasourceConfiguration); diff --git a/app/server/appsmith-plugins/firestorePlugin/src/main/resources/setting.json b/app/server/appsmith-plugins/firestorePlugin/src/main/resources/setting.json new file mode 100644 index 0000000000..c398eaf6a8 --- /dev/null +++ b/app/server/appsmith-plugins/firestorePlugin/src/main/resources/setting.json @@ -0,0 +1,36 @@ +{ + "setting": [ + { + "sectionName": "", + "id": 1, + "children": [ + { + "label": "Run query on page load", + "configProperty": "executeOnLoad", + "controlType": "SWITCH", + "info": "Will refresh data each time the page is loaded" + }, + { + "label": "Request confirmation before running query", + "configProperty": "confirmBeforeExecute", + "controlType": "SWITCH", + "info": "Ask confirmation from the user each time before refreshing data" + }, + { + "label": "Smart JSON Substitution", + "info": "Turning on this property fixes the JSON substitution of bindings in the Body field by adding/removing quotes intelligently and reduces developer errors", + "configProperty": "actionConfiguration.formData.smartSubstitution", + "controlType": "SWITCH", + "initialValue": true + }, + { + "label": "Query timeout (in milliseconds)", + "info": "Maximum time after which the query will return", + "configProperty": "actionConfiguration.timeoutInMillisecond", + "controlType": "INPUT_TEXT", + "dataType": "NUMBER" + } + ] + } + ] +} \ No newline at end of file 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 e5116ab0bd..8c64abb460 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 @@ -166,7 +166,7 @@ public class FirestorePluginTest { actionConfiguration.setFormData(configMap); Mono resultMono = pluginExecutor - .executeParameterized(firestoreConnection, null, dsConfig, actionConfiguration); + .executeParameterized(firestoreConnection, new ExecuteActionDTO(), dsConfig, actionConfiguration); StepVerifier.create(resultMono) .assertNext(result -> { @@ -202,7 +202,7 @@ public class FirestorePluginTest { actionConfiguration.setFormData(configMap); Mono resultMono = pluginExecutor - .executeParameterized(firestoreConnection, null, dsConfig, actionConfiguration); + .executeParameterized(firestoreConnection, new ExecuteActionDTO(), dsConfig, actionConfiguration); StepVerifier.create(resultMono) .assertNext(result -> { @@ -233,7 +233,7 @@ public class FirestorePluginTest { actionConfiguration.setFormData(configMap); Mono resultMono = pluginExecutor - .executeParameterized(firestoreConnection, null, dsConfig, actionConfiguration); + .executeParameterized(firestoreConnection, new ExecuteActionDTO(), dsConfig, actionConfiguration); StepVerifier.create(resultMono) .assertNext(result -> { @@ -265,7 +265,7 @@ public class FirestorePluginTest { actionConfiguration.setFormData(configMap); Mono resultMono = pluginExecutor - .executeParameterized(firestoreConnection, null, dsConfig, actionConfiguration); + .executeParameterized(firestoreConnection, new ExecuteActionDTO(), dsConfig, actionConfiguration); StepVerifier.create(resultMono) .assertNext(result -> { @@ -329,7 +329,7 @@ public class FirestorePluginTest { actionConfiguration.setFormData(configMap); Mono resultMono = pluginExecutor - .executeParameterized(firestoreConnection, null, dsConfig, actionConfiguration); + .executeParameterized(firestoreConnection, new ExecuteActionDTO(), dsConfig, actionConfiguration); StepVerifier.create(resultMono) .assertNext(result -> { @@ -364,7 +364,7 @@ public class FirestorePluginTest { actionConfiguration.setFormData(configMap); Mono resultMono = pluginExecutor - .executeParameterized(firestoreConnection, null, dsConfig, actionConfiguration); + .executeParameterized(firestoreConnection, new ExecuteActionDTO(), dsConfig, actionConfiguration); StepVerifier.create(resultMono) .assertNext(result -> { @@ -398,7 +398,7 @@ public class FirestorePluginTest { actionConfiguration.setFormData(configMap); Mono resultMono = pluginExecutor - .executeParameterized(firestoreConnection, null, dsConfig, actionConfiguration); + .executeParameterized(firestoreConnection, new ExecuteActionDTO(), dsConfig, actionConfiguration); StepVerifier.create(resultMono) .assertNext(result -> { @@ -424,7 +424,7 @@ public class FirestorePluginTest { actionConfiguration.setFormData(configMap); Mono resultMono = pluginExecutor - .executeParameterized(firestoreConnection, null, dsConfig, actionConfiguration); + .executeParameterized(firestoreConnection, new ExecuteActionDTO(), dsConfig, actionConfiguration); StepVerifier.create(resultMono) .assertNext(result -> { @@ -464,7 +464,7 @@ public class FirestorePluginTest { actionConfiguration.setFormData(configMap); Mono resultMono = pluginExecutor - .executeParameterized(firestoreConnection, null, dsConfig, actionConfiguration); + .executeParameterized(firestoreConnection, new ExecuteActionDTO(), dsConfig, actionConfiguration); StepVerifier.create(resultMono) .assertNext(result -> { @@ -526,7 +526,7 @@ public class FirestorePluginTest { final ActionConfiguration actionConfiguration = constructActionConfiguration(null, null); // Fetch data for page 1 Mono page1Mono = pluginExecutor - .executeParameterized(firestoreConnection, null, dsConfig, actionConfiguration) + .executeParameterized(firestoreConnection, new ExecuteActionDTO(), dsConfig, actionConfiguration) .cache(); // Fetch data for page 2 by clicking on the next button @@ -625,7 +625,7 @@ public class FirestorePluginTest { actionConfiguration.setFormData(configMap); Mono resultMono = pluginExecutor - .executeParameterized(firestoreConnection, null, dsConfig, actionConfiguration); + .executeParameterized(firestoreConnection, new ExecuteActionDTO(), dsConfig, actionConfiguration); StepVerifier.create(resultMono) .assertNext(result -> { @@ -685,7 +685,7 @@ public class FirestorePluginTest { actionConfiguration.setFormData(configMap); Mono resultMono = pluginExecutor - .executeParameterized(firestoreConnection, null, dsConfig, actionConfiguration); + .executeParameterized(firestoreConnection, new ExecuteActionDTO(), dsConfig, actionConfiguration); StepVerifier.create(resultMono) .assertNext(result -> { @@ -992,7 +992,7 @@ public class FirestorePluginTest { actionConfiguration.setFormData(configMap); Mono resultMono = pluginExecutor - .executeParameterized(firestoreConnection, null, dsConfig, actionConfiguration); + .executeParameterized(firestoreConnection, new ExecuteActionDTO(), dsConfig, actionConfiguration); StepVerifier.create(resultMono) .assertNext(result -> { @@ -1036,7 +1036,7 @@ public class FirestorePluginTest { actionConfiguration.setFormData(configMap); Mono resultMono = pluginExecutor - .executeParameterized(firestoreConnection, null, dsConfig, actionConfiguration); + .executeParameterized(firestoreConnection, new ExecuteActionDTO(), dsConfig, actionConfiguration); /* * - Delete key. @@ -1065,7 +1065,7 @@ public class FirestorePluginTest { setValueSafelyInFormData(configMap, BODY, ""); setValueSafelyInFormData(configMap, COMMAND, "GET_DOCUMENT"); resultMono = pluginExecutor - .executeParameterized(firestoreConnection, null, dsConfig, actionConfiguration); + .executeParameterized(firestoreConnection, new ExecuteActionDTO(), dsConfig, actionConfiguration); /* * - Verify that the key does not exist in the list of keys returned by reading the document. @@ -1093,7 +1093,7 @@ public class FirestorePluginTest { actionConfiguration.setFormData(configMap); Mono resultMono = pluginExecutor - .executeParameterized(firestoreConnection, null, dsConfig, actionConfiguration); + .executeParameterized(firestoreConnection, new ExecuteActionDTO(), dsConfig, actionConfiguration); StepVerifier.create(resultMono) .assertNext(result -> { assertFalse(result.getIsExecutionSuccess()); @@ -1121,7 +1121,7 @@ public class FirestorePluginTest { actionConfiguration.setFormData(configMap); Mono resultMono = pluginExecutor - .executeParameterized(firestoreConnection, null, dsConfig, actionConfiguration); + .executeParameterized(firestoreConnection, new ExecuteActionDTO(), dsConfig, actionConfiguration); StepVerifier.create(resultMono) .assertNext(result -> { assertFalse(result.getIsExecutionSuccess()); @@ -1148,7 +1148,7 @@ public class FirestorePluginTest { actionConfiguration.setFormData(configMap); Mono resultMono = pluginExecutor - .executeParameterized(firestoreConnection, null, dsConfig, actionConfiguration); + .executeParameterized(firestoreConnection, new ExecuteActionDTO(), dsConfig, actionConfiguration); StepVerifier.create(resultMono) .assertNext(result -> { assertFalse(result.getIsExecutionSuccess()); @@ -1175,7 +1175,7 @@ public class FirestorePluginTest { actionConfiguration.setFormData(configMap); Mono resultMono = pluginExecutor - .executeParameterized(firestoreConnection, null, dsConfig, actionConfiguration); + .executeParameterized(firestoreConnection, new ExecuteActionDTO(), dsConfig, actionConfiguration); StepVerifier.create(resultMono) .assertNext(result -> { @@ -1243,4 +1243,101 @@ public class FirestorePluginTest { ((Map) ((List) actionConfiguration.getPluginSpecifiedTemplates().get(3).getValue()).get(0)).get( "value")); } + + @Test + public void testJsonSmartSubstitution() { + /** + * Create a new document in Firestore. This command should fail without the smart JSON substitution because + * a normal mustache replacement will create an invalid JSON. + * Please note that the smart substitution is by default set to `true` hence we haven't explicitly set it here. + */ + Map configMap1 = new HashMap<>(); + setValueSafelyInFormData(configMap1, COMMAND, "CREATE_DOCUMENT"); + setValueSafelyInFormData(configMap1, PATH, "test/json_smart_substitution_test"); + setValueSafelyInFormData(configMap1, BODY, "{\n" + + " \"firstName\":{{Input1.text}},\n" + + " \"lastName\":{{Input2.text}},\n" + + " \"locationPreferences\":{{Input3.text}},\n" + + " \"testScores\":{{Input4.text}}\n" + + "}"); + + ActionConfiguration actionConfiguration1 = new ActionConfiguration(); + actionConfiguration1.setFormData(configMap1); + + List params = new ArrayList(); + Param param = new Param(); + param.setKey("Input1.text"); + param.setValue("Jon"); + params.add(param); + param = new Param(); + param.setKey("Input2.text"); + param.setValue("Von Neumann"); + params.add(param); + param = new Param(); + param.setKey("Input3.text"); + param.setValue("[\"Zuric\", \"Gottingen\"]"); + params.add(param); + param = new Param(); + param.setKey("Input4.text"); + param.setValue("{\"computational complexity\": 100, \"math\": 100}"); + params.add(param); + + ExecuteActionDTO executeActionDTO = new ExecuteActionDTO(); + executeActionDTO.setParams(params); + + Mono resultMono = pluginExecutor + .executeParameterized(firestoreConnection, executeActionDTO, dsConfig, actionConfiguration1); + + StepVerifier.create(resultMono) + .assertNext(result -> assertTrue(result.getIsExecutionSuccess())) + .verifyComplete(); + + /* Fetch previously created document to check if correct value was saved */ + Map configMap2 = new HashMap(); + setValueSafelyInFormData(configMap2, COMMAND, "GET_DOCUMENT"); + setValueSafelyInFormData(configMap2, PATH, "test/json_smart_substitution_test"); + + ActionConfiguration actionConfiguration2 = new ActionConfiguration(); + actionConfiguration2.setFormData(configMap2); + + Mono resultMono2 = pluginExecutor + .executeParameterized(firestoreConnection, new ExecuteActionDTO(), dsConfig, actionConfiguration2); + + StepVerifier.create(resultMono2) + .assertNext(result -> { + assertTrue(result.getIsExecutionSuccess()); + final Map first = (Map) result.getBody(); + assertEquals("Jon", first.get("firstName")); + assertEquals("Von Neumann", first.get("lastName")); + assertEquals("Zuric", ((List) first.get("locationPreferences")).get(0)); + assertEquals("Gottingen", ((List) first.get("locationPreferences")).get(1)); + assertEquals("100", ((Map) first.get("testScores")).get("computational complexity").toString()); + assertEquals("100", ((Map) first.get("testScores")).get("math").toString()); + }) + .verifyComplete(); + + /* Delete the document added as part of this test */ + Map configMap3 = new HashMap<>(); + setValueSafelyInFormData(configMap3, COMMAND, "DELETE_DOCUMENT"); + setValueSafelyInFormData(configMap3, PATH, "test/json_smart_substitution_test"); + + ActionConfiguration actionConfiguration3 = new ActionConfiguration(); + actionConfiguration3.setFormData(configMap3); + + Mono resultMono3 = pluginExecutor + .executeParameterized(firestoreConnection, new ExecuteActionDTO(), dsConfig, actionConfiguration3); + + StepVerifier.create(resultMono3) + .assertNext(result -> { + assertTrue(result.getIsExecutionSuccess()); + try { + final DocumentSnapshot documentSnapshot = firestoreConnection.document("test" + + "/json_smart_substitution_test").get().get(); + assertFalse(documentSnapshot.exists()); + } catch (InterruptedException | ExecutionException e) { + e.printStackTrace(); + } + }) + .verifyComplete(); + } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java index 8c7c515380..e7269d86a9 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java @@ -166,6 +166,7 @@ public class DatabaseChangelog { public static final String KEY = "key"; public static final String START_AFTER = "startAfter"; public static final String END_BEFORE = "endBefore"; + public static final String SMART_SUBSTITUTION = "smartSubstitution"; @AllArgsConstructor @NoArgsConstructor @@ -4849,9 +4850,9 @@ public class DatabaseChangelog { * @return query */ private Query getQueryToFetchAllPluginActionsWhichAreNotDeleted(Plugin plugin) { - Criteria pluginIdIsMongoPluginId = where("pluginId").is(plugin.getId()); + Criteria pluginIdMatchesSuppliedPluginId = where("pluginId").is(plugin.getId()); Criteria isNotDeleted = where("deleted").ne(true); - return query((new Criteria()).andOperator(pluginIdIsMongoPluginId, isNotDeleted)); + return query((new Criteria()).andOperator(pluginIdMatchesSuppliedPluginId, isNotDeleted)); } /** @@ -4892,7 +4893,7 @@ public class DatabaseChangelog { updateMockdbEndpoint(mongockTemplate); } - @ChangeSet(order = "111", id = "migrate-from-RSA-SHA1-to-ECDSA-SHA2-protocol-for-key-generation", author = "") + @ChangeSet(order = "112", id = "migrate-from-RSA-SHA1-to-ECDSA-SHA2-protocol-for-key-generation", author = "") public void migrateFromRSASha1ToECDSASha2Protocol(MongockTemplate mongockTemplate) { Query query = new Query(); query.addCriteria(Criteria.where("gitApplicationMetadata.gitAuth").exists(TRUE)); @@ -5072,5 +5073,45 @@ public class DatabaseChangelog { ); } } - + + @ChangeSet(order = "118", id = "set-firestore-smart-substitution-to-false-for-old-cmds", author = "") + public void setFirestoreSmartSubstitutionToFalseForOldCommands(MongockTemplate mongockTemplate) { + Plugin firestorePlugin = mongockTemplate.findOne(query(where("packageName").is("firestore-plugin")), + Plugin.class); + + /* Query to get all Mongo actions which are not deleted */ + Query queryToGetActions = getQueryToFetchAllPluginActionsWhichAreNotDeleted(firestorePlugin); + + /* Update the previous query to only include id field */ + queryToGetActions.fields().include(fieldName(QNewAction.newAction.id)); + + /* Fetch Firestore actions using the previous query */ + List firestoreActions = mongockTemplate.find(queryToGetActions, NewAction.class); + + /* set key formData.smartSubstitution */ + setSmartSubstitutionFieldForEachAction(firestoreActions, mongockTemplate); + } + + private void setSmartSubstitutionFieldForEachAction(List firestoreActions, + MongockTemplate mongockTemplate) { + firestoreActions.stream() + .map(NewAction::getId) /* iterate over one action id at a time */ + .map(actionId -> fetchActionUsingId(actionId, mongockTemplate)) /* fetch action using id */ + .filter(this::hasUnpublishedActionConfiguration) + .forEachOrdered(firestoreAction -> { + /* set key for unpublished action */ + Map unpublishedFormData = + firestoreAction.getUnpublishedAction().getActionConfiguration().getFormData(); + setValueSafelyInFormData(unpublishedFormData, SMART_SUBSTITUTION, FALSE.toString()); + + /* set key for published action */ + if (hasPublishedActionConfiguration(firestoreAction)) { + Map publishedFormData = + firestoreAction.getPublishedAction().getActionConfiguration().getFormData(); + setValueSafelyInFormData(publishedFormData, SMART_SUBSTITUTION, FALSE.toString()); + } + + mongockTemplate.save(firestoreAction); + }); + } }