fix Mongo smart substitution for quotes around ObjectId (#9856)

- fix Mongo plugin's smart substitution feature to handle quotes around ObjectId in array. e.g. Initial replacement value: ["ObjectId(xyz)"] , final replacement value: [ObjectId(xyz)]
- unrelated: added version number to lombok dependency to stop build failures in IntelliJ.

app/server/appsmith-interfaces/pom.xml

@@ -58,6 +58,7 @@
         <dependency>
             <groupId>org.projectlombok</groupId>
             <artifactId>lombok</artifactId>
+            <version>1.18.8</version>
             <scope>provided</scope>
         </dependency>
         <dependency>

app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/DataTypeStringUtils.java

@@ -5,6 +5,7 @@ import com.appsmith.external.constants.DisplayDataType;
 import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError;
 import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException;
 import com.appsmith.external.models.ParsedDataType;
+import com.appsmith.external.plugins.SmartSubstitutionInterface;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.JsonNode;
@@ -188,13 +189,15 @@ public class DataTypeStringUtils {
 
     public static String jsonSmartReplacementPlaceholderWithValue(String input,
                                                                   String replacement,
-                                                                  List<Map.Entry<String, String>> insertedParams) {
+                                                                  List<Map.Entry<String, String>> insertedParams,
+                                                                  SmartSubstitutionInterface smartSubstitutionUtils) {
 
         DataType dataType = DataTypeStringUtils.stringToKnownDataTypeConverter(replacement);
 
         Map.Entry<String, String> parameter = new SimpleEntry<>(replacement, dataType.toString());
         insertedParams.add(parameter);
 
+        String updatedReplacement;
         switch (dataType) {
             case INTEGER:
             case LONG:
@@ -202,12 +205,12 @@ public class DataTypeStringUtils {
             case DOUBLE:
             case NULL:
             case BOOLEAN:
-                input = placeholderPattern.matcher(input).replaceFirst(String.valueOf(replacement));
+                updatedReplacement = String.valueOf(replacement);
                 break;
             case ARRAY:
                 try {
                     JSONArray jsonArray = (JSONArray) parser.parse(replacement);
-                    input = placeholderPattern.matcher(input).replaceFirst(String.valueOf(objectMapper.writeValueAsString(jsonArray)));
+                    updatedReplacement = String.valueOf(objectMapper.writeValueAsString(jsonArray));
                 } catch (net.minidev.json.parser.ParseException | JsonProcessingException e) {
                     throw Exceptions.propagate(
                             new AppsmithPluginException(
@@ -223,7 +226,7 @@ public class DataTypeStringUtils {
                     JSONObject jsonObject = (JSONObject) parser.parse(replacement);
                     String jsonString = String.valueOf(objectMapper.writeValueAsString(jsonObject));
                     // Adding Matcher.quoteReplacement so that "/" and "$" in the string are escaped during replacement
-                    input = placeholderPattern.matcher(input).replaceFirst(Matcher.quoteReplacement(jsonString));
+                    updatedReplacement = Matcher.quoteReplacement(jsonString);
                 } catch (net.minidev.json.parser.ParseException | JsonProcessingException e) {
                     throw Exceptions.propagate(
                             new AppsmithPluginException(
@@ -235,7 +238,7 @@ public class DataTypeStringUtils {
                 }
                 break;
             case BSON:
-                input = placeholderPattern.matcher(input).replaceFirst(Matcher.quoteReplacement(replacement));
+                updatedReplacement = Matcher.quoteReplacement(replacement);
                 break;
             case DATE:
             case TIME:
@@ -246,7 +249,7 @@ public class DataTypeStringUtils {
             default:
                 try {
                     String valueAsString = objectMapper.writeValueAsString(replacement);
-                    input = placeholderPattern.matcher(input).replaceFirst(Matcher.quoteReplacement(valueAsString));
+                    updatedReplacement = Matcher.quoteReplacement(valueAsString);
                 } catch (JsonProcessingException e) {
                     throw Exceptions.propagate(
                             new AppsmithPluginException(
@@ -258,6 +261,11 @@ public class DataTypeStringUtils {
                 }
         }
 
+        if (smartSubstitutionUtils != null) {
+            updatedReplacement = smartSubstitutionUtils.sanitizeReplacement(updatedReplacement);
+        }
+
+        input = placeholderPattern.matcher(input).replaceFirst(updatedReplacement);
         return input;
     }
 

app/server/appsmith-interfaces/src/main/java/com/appsmith/external/plugins/SmartSubstitutionInterface.java

@@ -36,6 +36,7 @@ public interface SmartSubstitutionInterface {
                 // If the evaluated value of the mustache binding is present, set it in the prepared statement
                 if (matchingParam.isPresent()) {
                     String value = matchingParam.get().getValue();
+
                     input = substituteValueInInput(i + 1, key,
                             value, input, insertedParams, args);
                 } else {
@@ -54,4 +55,18 @@ public interface SmartSubstitutionInterface {
                                           List<Map.Entry<String, String>> insertedParams, Object... args) throws AppsmithPluginException {
         return input;
     }
+
+
+    /**
+     * This method is part of the pre-processing of the replacement value before the final substitution that
+     * happens as part of smart substitution process.
+     * This is the default implementation. Each plugin that implements `SmartSubstitutionInterface` is supposed to
+     * override this method to provide plugin specific implementation.
+     *
+     * @param replacementValue - value to be substituted
+     * @return - updated replacement value
+     */
+    default String sanitizeReplacement(String replacementValue) {
+        return replacementValue;
+    }
 }

app/server/appsmith-plugins/amazons3Plugin/src/main/java/com/external/plugins/AmazonS3Plugin.java

@@ -983,7 +983,7 @@ public class AmazonS3Plugin extends BasePlugin {
                                              List<Map.Entry<String, String>> insertedParams,
                                              Object... args) {
             String jsonBody = (String) input;
-            return DataTypeStringUtils.jsonSmartReplacementPlaceholderWithValue(jsonBody, value, insertedParams);
+            return DataTypeStringUtils.jsonSmartReplacementPlaceholderWithValue(jsonBody, value, insertedParams, null);
         }
 
     }

app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/plugins/GoogleSheetsPlugin.java

@@ -299,7 +299,7 @@ public class GoogleSheetsPlugin extends BasePlugin {
                                              List<Map.Entry<String, String>> insertedParams,
                                              Object... args) {
             String jsonBody = (String) input;
-            return DataTypeStringUtils.jsonSmartReplacementPlaceholderWithValue(jsonBody, value, insertedParams);
+            return DataTypeStringUtils.jsonSmartReplacementPlaceholderWithValue(jsonBody, value, insertedParams, null);
         }
     }
 }

app/server/appsmith-plugins/mongoPlugin/src/main/java/com/external/plugins/MongoPlugin.java

@@ -117,6 +117,17 @@ public class MongoPlugin extends BasePlugin {
      */
     private static final String MONGO_URI_REGEX = "^(mongodb(\\+srv)?:\\/\\/)((.+):(.+))(@.+\\/(.+))$";
 
+    /**
+     * This regex matches the following two patterns:
+     *   - "ObjectId(someId)"  // will not match without outer double quotes
+     *     - Group 1 = "ObjectId(someId)"
+     *     - Group 2 = ObjectId(someId) // no quotes
+     *   - 'ObjectId(someId)'  // will not match without outer single quotes
+     *      - Group 3 = 'ObjectId(someId)'
+     *      - Group 4 = ObjectId(someId) // not quotes
+     */
+    private static final String OBJECT_ID_INSIDE_QUOTES_REGEX = "(\\\"(ObjectId\\(.*?\\))\\\")|('(ObjectId\\(.*?\\))')";
+
     private static final int REGEX_GROUP_HEAD = 1;
 
     private static final int REGEX_GROUP_USERNAME = 4;
@@ -406,6 +417,65 @@ public class MongoPlugin extends BasePlugin {
                     .subscribeOn(scheduler);
         }
 
+        /**
+         * This method is part of the pre-processing of the replacement value before the final substitution that
+         * happens as part of smart substitution process.
+         *
+         * @param replacementValue - value to be substituted
+         * @return - updated replacement value
+         */
+        @Override
+        public String sanitizeReplacement(String replacementValue) {
+            replacementValue = removeQuotesAroundObjectId(replacementValue);
+
+            return replacementValue;
+        }
+
+        /**
+         * This method is meant to remove extra quotes around the `ObjectId(...)` string. E.g. if the input query is
+         * "... {$in: [\"ObjectId(\"123\")\"]}" then the output query will be "... {$in: [ObjectId(\"123\")]}".
+         *
+         * @param query - input query
+         * @return - query obtained after removing quotes around ObjectId string.
+         */
+        private String removeQuotesAroundObjectId(String query) {
+            Map<String, String> objectIdMap = new HashMap();
+
+            Pattern pattern = Pattern.compile(OBJECT_ID_INSIDE_QUOTES_REGEX);
+            Matcher matcher = pattern.matcher(query);
+            while (matcher.find()) {
+                String objectIdWithQuotes;
+                String objectIdWithoutQuotes;
+
+                /**
+                 * `If` branch will match when ObjectId is wrapped within double quotes e.g. "ObjectId(someId)"
+                 *   - Group 1 = "ObjectId(someId)"
+                 *   - Group 2 = ObjectId(someId) // no quotes
+                 * `Else` branch will match when ObjectId is wrapped within single quotes e.g. 'ObjectId(someId)'
+                 *   - Group 3 = 'ObjectId(someId)'
+                 *   - Group 4 = ObjectId(someId) // no quotes
+                 */
+                if (matcher.group(1) != null) {
+                    objectIdWithQuotes = matcher.group(1);
+                    objectIdWithoutQuotes = matcher.group(2);
+                }
+                else {
+                    objectIdWithQuotes = matcher.group(3);
+                    objectIdWithoutQuotes = matcher.group(4);
+                }
+
+                objectIdMap.put(objectIdWithQuotes, objectIdWithoutQuotes);
+            }
+
+            for (Map.Entry<String, String> entry : objectIdMap.entrySet()) {
+                String objectIdWithQuotes = (entry).getKey();
+                String objectIdWithoutQuotes = (entry).getValue();
+                query = query.replace(objectIdWithQuotes, objectIdWithoutQuotes);
+            }
+
+            return query;
+        }
+
         private String smartSubstituteBSON(String rawQuery,
                                            List<Param> params,
                                            List<Map.Entry<String, String>> parameters) throws AppsmithPluginException {
@@ -878,7 +948,7 @@ public class MongoPlugin extends BasePlugin {
                                              List<Map.Entry<String, String>> insertedParams,
                                              Object... args) {
             String jsonBody = (String) input;
-            return DataTypeStringUtils.jsonSmartReplacementPlaceholderWithValue(jsonBody, value, insertedParams);
+            return DataTypeStringUtils.jsonSmartReplacementPlaceholderWithValue(jsonBody, value, insertedParams, this);
         }
 
         @Override

app/server/appsmith-plugins/mongoPlugin/src/test/java/com/external/plugins/MongoPluginTest.java

@@ -89,6 +89,7 @@ public class MongoPluginTest {
     private static String address;
     private static Integer port;
     private JsonNode value;
+    private static MongoClient mongoClient;
 
     @SuppressWarnings("rawtypes")
     @ClassRule
@@ -99,14 +100,14 @@ public class MongoPluginTest {
     public static void setUp() {
         address = mongoContainer.getContainerIpAddress();
         port = mongoContainer.getFirstMappedPort();
-        String uri = "mongodb://" + address + ":" + Integer.toString(port);
+        String uri = "mongodb://" + address + ":" + port;
-        final MongoClient mongoClient = MongoClients.create(uri);
+        mongoClient = MongoClients.create(uri);
 
         Flux.from(mongoClient.getDatabase("test").listCollectionNames()).collectList().
                 flatMap(collectionNamesList -> {
-                    final MongoCollection<Document> usersCollection = mongoClient.getDatabase("test").getCollection(
-                            "users");
                     if (collectionNamesList.size() == 0) {
+                        final MongoCollection<Document> usersCollection = mongoClient.getDatabase("test").getCollection(
+                                "users");
                         Mono.from(usersCollection.insertMany(List.of(
                                 new Document(Map.of(
                                         "name", "Cierra Vega",
@@ -122,7 +123,7 @@ public class MongoPluginTest {
                         ))).block();
                     }
 
-                    return Mono.just(usersCollection);
+                    return Mono.empty();
                 }).block();
     }
 
@@ -1816,4 +1817,116 @@ public class MongoPluginTest {
                 })
                 .verifyComplete();
     }
+
+    @Test
+    public void testSmartSubstitutionWithObjectIdInDoubleQuotes() {
+        final MongoCollection<Document> usersCollection = mongoClient.getDatabase("test").getCollection("users");
+        List<String> documentIds = new ArrayList<>();
+        Flux.from(usersCollection.find())
+                .map(doc -> documentIds.add(doc.get("_id").toString()))
+                .collectList()
+                .block();
+
+        DatasourceConfiguration dsConfig = createDatasourceConfiguration();
+        Mono<MongoClient> dsConnectionMono = pluginExecutor.datasourceCreate(dsConfig);
+
+        String findQuery = "{\n" +
+                "   \"find\": \"users\",\n" +
+                "   \"filter\": {\n" +
+                "           \"_id\": {\n" +
+                "               $in: {{Input1.text}}\n" +
+                "            }\n" +
+                "    }\n" +
+                "}";
+        ActionConfiguration actionConfiguration = new ActionConfiguration();
+        actionConfiguration.setBody(findQuery);
+
+        StringBuilder sb = new StringBuilder();
+        documentIds.stream()
+                .forEach(id -> sb.append(" \"ObjectId(\\\"" + id + "\\\")\","));
+        sb.setLength(sb.length() - 1);
+        String objectIdsAsArray = "[" + sb + "]";
+
+        Map<String, Object> configMap = new HashMap<>();
+        setValueSafelyInFormData(configMap, SMART_SUBSTITUTION, Boolean.TRUE);
+        setValueSafelyInFormData(configMap, COMMAND, "RAW");
+        actionConfiguration.setFormData(configMap);
+
+        ExecuteActionDTO executeActionDTO = new ExecuteActionDTO();
+        List<Param> params = new ArrayList<>();
+        Param param1 = new Param();
+        param1.setKey("Input1.text");
+        param1.setValue(objectIdsAsArray);
+        params.add(param1);
+        executeActionDTO.setParams(params);
+
+        Mono<Object> executeMono = dsConnectionMono.flatMap(conn -> pluginExecutor.executeParameterized(conn,
+                executeActionDTO, dsConfig, actionConfiguration));
+        StepVerifier.create(executeMono)
+                .assertNext(obj -> {
+                    ActionExecutionResult result = (ActionExecutionResult) obj;
+                    assertNotNull(result);
+                    assertTrue(result.getIsExecutionSuccess());
+                    assertNotNull(result.getBody());
+                    assertEquals(3, ((ArrayNode) result.getBody()).size());
+                })
+                .verifyComplete();
+
+    }
+
+    @Test
+    public void testSmartSubstitutionWithObjectIdInSingleQuotes() {
+        final MongoCollection<Document> usersCollection = mongoClient.getDatabase("test").getCollection("users");
+        List<String> documentIds = new ArrayList<>();
+        Flux.from(usersCollection.find())
+                .map(doc -> documentIds.add(doc.get("_id").toString()))
+                .collectList()
+                .block();
+
+        DatasourceConfiguration dsConfig = createDatasourceConfiguration();
+        Mono<MongoClient> dsConnectionMono = pluginExecutor.datasourceCreate(dsConfig);
+
+        String findQuery = "{\n" +
+                "   \"find\": \"users\",\n" +
+                "   \"filter\": {\n" +
+                "           \"_id\": {\n" +
+                "               $in: {{Input1.text}}\n" +
+                "            }\n" +
+                "    }\n" +
+                "}";
+        ActionConfiguration actionConfiguration = new ActionConfiguration();
+        actionConfiguration.setBody(findQuery);
+
+        StringBuilder sb = new StringBuilder();
+        documentIds.stream()
+                .forEach(id -> sb.append(" \'ObjectId(\\\"" + id + "\\\")\',"));
+        sb.setLength(sb.length() - 1);
+        String objectIdsAsArray = "[" + sb + "]";
+
+        Map<String, Object> configMap = new HashMap<>();
+        setValueSafelyInFormData(configMap, SMART_SUBSTITUTION, Boolean.TRUE);
+        setValueSafelyInFormData(configMap, COMMAND, "RAW");
+        actionConfiguration.setFormData(configMap);
+
+        ExecuteActionDTO executeActionDTO = new ExecuteActionDTO();
+        List<Param> params = new ArrayList<>();
+        Param param1 = new Param();
+        param1.setKey("Input1.text");
+        param1.setValue(objectIdsAsArray);
+        params.add(param1);
+        executeActionDTO.setParams(params);
+
+        Mono<Object> executeMono = dsConnectionMono.flatMap(conn -> pluginExecutor.executeParameterized(conn,
+                executeActionDTO, dsConfig, actionConfiguration));
+        StepVerifier.create(executeMono)
+                .assertNext(obj -> {
+                    ActionExecutionResult result = (ActionExecutionResult) obj;
+                    assertNotNull(result);
+                    assertTrue(result.getIsExecutionSuccess());
+                    assertNotNull(result.getBody());
+                    assertEquals(3, ((ArrayNode) result.getBody()).size());
+                })
+                .verifyComplete();
+
+    }
 }

app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/plugins/RestApiPlugin.java

@@ -671,7 +671,7 @@ public class RestApiPlugin extends BasePlugin {
                                              List<Map.Entry<String, String>> insertedParams,
                                              Object... args) {
             String jsonBody = (String) input;
-            return DataTypeStringUtils.jsonSmartReplacementPlaceholderWithValue(jsonBody, value, insertedParams);
+            return DataTypeStringUtils.jsonSmartReplacementPlaceholderWithValue(jsonBody, value, insertedParams, null);
         }
 
         @Override