fix: fix execution of REST API query when dynamic binding is present in the URL (#24368)

## Description
-  fix execution of REST API query when dynamic binding is present in the URL

Fixes #24218

(cherry picked from commit 9330829e40)

app/client/cypress/e2e/Regression/ServerSide/ApiTests/API_TestExecuteWithDynamicBindingInUrl_spec.ts

@@ -0,0 +1,42 @@
+const datasourceFormData = require("../../../../fixtures/datasources.json");
+
+import {
+  apiPage,
+  agHelper,
+  jsEditor,
+} from "../../../../support/Objects/ObjectsCore";
+
+describe("Test API execution with dynamic binding in URL - Bug #24218", () => {
+  it("1. Test API execution with dynamic binding in URL", () => {
+    // Create JS Object to set Appsmith store variable to mockApiUrl
+    jsEditor.CreateJSObject(
+      `export default {
+        myVar1: [],
+        myVar2: {},
+        myFun1 () {
+          storeValue("api_url", "${datasourceFormData["mockApiUrl"]}");
+        },
+        myFun2: async function() {
+        }
+      }`,
+      {
+        paste: true,
+        completeReplace: true,
+        toRun: false,
+        shouldCreateNewJSObj: true,
+        prettify: true,
+      },
+    );
+
+    jsEditor.RunJSObj();
+
+    // Create API and set URL to {{appsmith.store.api_url}}
+    apiPage.CreateAndFillApi(
+      "{{appsmith.store.api_url}}",
+      "Api_with_dynamic_binding",
+    );
+    apiPage.RunAPI();
+    apiPage.ResponseStatusCheck("200 OK");
+    agHelper.ActionContextMenuWithInPane("Delete");
+  });
+});

app/server/appsmith-interfaces/src/main/java/com/appsmith/external/exceptions/pluginExceptions/AppsmithPluginError.java

@@ -29,6 +29,16 @@ public enum AppsmithPluginError implements BasePluginError{
             "{1}",
             "{2}"
     ),
+    PLUGIN_VALIDATE_DATASOURCE_ERROR(
+            500,
+            AppsmithPluginErrorCode.PLUGIN_VALIDATE_DATASOURCE_ERROR.getCode(),
+            "{0}",
+            AppsmithErrorAction.DEFAULT,
+            AppsmithPluginErrorCode.PLUGIN_VALIDATE_DATASOURCE_ERROR.getDescription(),
+            ErrorType.INTERNAL_ERROR,
+            "{1}",
+            "{2}"
+    ),
     PLUGIN_QUERY_TIMEOUT_ERROR(
             504,
             AppsmithPluginErrorCode.PLUGIN_QUERY_TIMEOUT_ERROR.getCode(),

app/server/appsmith-interfaces/src/main/java/com/appsmith/external/exceptions/pluginExceptions/AppsmithPluginErrorCode.java

@@ -19,7 +19,8 @@ public enum AppsmithPluginErrorCode {
     PLUGIN_AUTHENTICATION_ERROR("PE-ATH-5000", "Datasource authentication error"),
     PLUGIN_UQI_WHERE_CONDITION_UNKNOWN("PE-UQI-5000", "Where condition could not be parsed"),
     GENERIC_STALE_CONNECTION("PE-STC-5000", "Secondary stale connection error"),
-    PLUGIN_EXECUTE_ARGUMENT_ERROR("PE-ARG-5000", "Wrong arguments provided")
+    PLUGIN_EXECUTE_ARGUMENT_ERROR("PE-ARG-5000", "Wrong arguments provided"),
+    PLUGIN_VALIDATE_DATASOURCE_ERROR("PE-DSE-5005", "Failed to validate datasource")
     ;
 
 

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

@@ -15,7 +15,8 @@ public class DatasourceUtils {
 
     protected static HeaderUtils headerUtils = new HeaderUtils();
 
-    public Set<String> validateDatasource(DatasourceConfiguration datasourceConfiguration) {
+    public Set<String> validateDatasource(DatasourceConfiguration datasourceConfiguration,
+                                          boolean isEmbeddedDatasource) {
         /**
          * We don't verify whether the URL is in valid format because it can contain mustache template keys, and so
          * look invalid at this point, but become valid after mustache rendering. So we just check if URL field has
@@ -25,7 +26,19 @@ public class DatasourceUtils {
         Set<String> invalids = new HashSet<>();
 
         if (StringUtils.isEmpty(datasourceConfiguration.getUrl())) {
-            invalids.add("Missing URL.");
+            if (isEmbeddedDatasource) {
+                /**
+                 * Deliberately skipping adding any invalidity message here because based on the current parsing logic the
+                 * client can skip adding a URL to embedded datasource and instead add the entire URL to
+                 * `actionConfiguration.path`.
+                 * ref: https://theappsmith.slack.com/archives/C040LHZN03V/p1686478370473659?thread_ts=1686300736
+                 * .679729&cid=C040LHZN03V
+                 */
+            }
+            else {
+                invalids.add("Missing URL.");
+            }
+
         }
 
         final String contentTypeError = headerUtils.verifyContentType(datasourceConfiguration.getHeaders());

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

@@ -4,7 +4,6 @@ import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError;
 import com.appsmith.external.models.ActionConfiguration;
 import com.appsmith.external.models.ActionExecutionResult;
 import com.appsmith.external.models.DatasourceConfiguration;
-import lombok.AccessLevel;
 import lombok.NoArgsConstructor;
 
 @NoArgsConstructor
@@ -13,7 +12,7 @@ public class InitUtils {
     public String initializeRequestUrl(ActionConfiguration actionConfiguration,
                                             DatasourceConfiguration datasourceConfiguration ) {
         String path = (actionConfiguration.getPath() == null) ? "" : actionConfiguration.getPath();
-        return datasourceConfiguration.getUrl().trim() + path;
+        return datasourceConfiguration.getUrl().trim() + path.trim();
     }
 
     public void initializeResponseWithError(ActionExecutionResult result) {

app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/DatasourceStorage.java

@@ -164,4 +164,8 @@ public class DatasourceStorage extends BaseDomain {
         this.setPluginId(pluginMap.get(this.getPluginId()));
         this.setIsRecentlyCreated(null);
     }
+
+    public boolean isEmbedded() {
+        return this.getDatasourceId() == null;
+    }
 }

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

@@ -9,8 +9,8 @@ import com.appsmith.external.helpers.restApiUtils.helpers.DatasourceUtils;
 import com.appsmith.external.helpers.restApiUtils.helpers.HeaderUtils;
 import com.appsmith.external.helpers.restApiUtils.helpers.HintMessageUtils;
 import com.appsmith.external.helpers.restApiUtils.helpers.InitUtils;
-import com.appsmith.external.helpers.restApiUtils.helpers.SmartSubstitutionUtils;
 import com.appsmith.external.helpers.restApiUtils.helpers.RestAPIActivateUtils;
+import com.appsmith.external.helpers.restApiUtils.helpers.SmartSubstitutionUtils;
 import com.appsmith.external.helpers.restApiUtils.helpers.URIUtils;
 import com.appsmith.external.models.ActionConfiguration;
 import com.appsmith.external.models.ActionExecutionResult;
@@ -73,9 +73,22 @@ public class BaseRestApiPluginExecutor implements PluginExecutor<APIConnection>,
     }
 
     @Override
-    public Set<String> validateDatasource(DatasourceConfiguration datasourceConfiguration) {
+    public Set<String> validateDatasource(DatasourceConfiguration datasourceConfiguration,
+                                          boolean isEmbeddedDatasource) {
         /* Use the default validation routine for REST API based plugins */
-        return datasourceUtils.validateDatasource(datasourceConfiguration);
+        return datasourceUtils.validateDatasource(datasourceConfiguration, isEmbeddedDatasource);
+    }
+
+    @Override
+    public Set<String> validateDatasource(DatasourceConfiguration datasourceConfiguration) {
+        /**
+         * This method is not expected to be used as the REST API based plugins may have embedded datasource. Instead
+         * the following variant should be used:
+         * validateDatasource(DatasourceConfiguration datasourceConfiguration, boolean isEmbeddedDatasource)
+         */
+        throw new AppsmithPluginException(AppsmithPluginError.PLUGIN_VALIDATE_DATASOURCE_ERROR, "This method should " +
+                "not be used for this plugin as it may have embedded datasource. Please use validateDatasource" +
+                "(dsConfig, isEmbedded).");
     }
 
     @Override

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

@@ -10,27 +10,25 @@ import com.appsmith.external.models.DatasourceConfiguration;
 import com.appsmith.external.models.DatasourceStructure;
 import com.appsmith.external.models.DatasourceTestResult;
 import com.appsmith.external.models.Param;
-import com.appsmith.external.models.Property;
 import com.appsmith.external.models.TriggerRequestDTO;
 import com.appsmith.external.models.TriggerResultDTO;
 import io.micrometer.observation.ObservationRegistry;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Properties;
-import java.util.Set;
 import org.pf4j.ExtensionPoint;
-import org.springframework.util.CollectionUtils;
 import org.springframework.util.StringUtils;
 import reactor.core.observability.micrometer.Micrometer;
 import reactor.core.publisher.Mono;
 import reactor.core.scheduler.Schedulers;
 import reactor.util.function.Tuple2;
 
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Properties;
+import java.util.Set;
 import java.util.stream.Collectors;
 
 import static com.appsmith.external.constants.spans.ActionSpans.ACTION_EXECUTION_PLUGIN_EXECUTION;
 import static com.appsmith.external.helpers.PluginUtils.getHintMessageForLocalhostUrl;
+import static org.springframework.util.CollectionUtils.isEmpty;
 
 public interface PluginExecutor<C> extends ExtensionPoint, CrudTemplateService {
 
@@ -96,8 +94,8 @@ public interface PluginExecutor<C> extends ExtensionPoint, CrudTemplateService {
      * @param datasourceConfiguration
      * @return boolean
      */
-    default boolean isDatasourceValid(DatasourceConfiguration datasourceConfiguration) {
+    default boolean isDatasourceValid(DatasourceConfiguration datasourceConfiguration, boolean isEmbeddedDatasource) {
-        return CollectionUtils.isEmpty(validateDatasource(datasourceConfiguration));
+        return isEmpty(validateDatasource(datasourceConfiguration, isEmbeddedDatasource));
     }
 
     /**
@@ -112,6 +110,14 @@ public interface PluginExecutor<C> extends ExtensionPoint, CrudTemplateService {
      * @return Set                      : The set of invalid strings informing the user of all the invalid fields
      */
     Set<String> validateDatasource(DatasourceConfiguration datasourceConfiguration);
+    default Set<String> validateDatasource(DatasourceConfiguration datasourceConfiguration,
+                                           boolean isEmbeddedDatasource) {
+        if (!isEmbeddedDatasource) {
+            return this.validateDatasource(datasourceConfiguration);
+        }
+
+        return Set.of();
+    }
 
     /**
      * This function tests the datasource by executing a test query or hitting the endpoint to check the correctness
@@ -227,7 +233,7 @@ public interface PluginExecutor<C> extends ExtensionPoint, CrudTemplateService {
                                       ExecuteActionDTO executeActionDTO) {
         //Do variable substitution
         //Do this only if params have been provided in the execute command
-        if (executeActionDTO != null && !CollectionUtils.isEmpty(executeActionDTO.getParams())) {
+        if (executeActionDTO != null && !isEmpty(executeActionDTO.getParams())) {
             Map<String, String> replaceParamsMap = executeActionDTO
                     .getParams()
                     .stream()

app/server/appsmith-interfaces/src/test/java/com/appsmith/external/plugins/DatasourceUtilsTest.java

@@ -0,0 +1,27 @@
+package com.appsmith.external.plugins;
+
+import com.appsmith.external.helpers.restApiUtils.helpers.DatasourceUtils;
+import com.appsmith.external.models.DatasourceConfiguration;
+import org.junit.jupiter.api.Test;
+
+import java.util.Set;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class DatasourceUtilsTest {
+    @Test
+    public void testValidateDatasourceSkipsEmptyUrlCheckIfEmbedded() {
+        DatasourceConfiguration dsConfig = new DatasourceConfiguration();
+        DatasourceUtils datasourceUtils = new DatasourceUtils();
+        Set<String> invalids = datasourceUtils.validateDatasource(dsConfig, true);
+        assertTrue(invalids.isEmpty());
+    }
+
+    @Test
+    public void testValidateDatasourceAddsEmptyUrlInvalidIfNotEmbedded() {
+        DatasourceConfiguration dsConfig = new DatasourceConfiguration();
+        DatasourceUtils datasourceUtils = new DatasourceUtils();
+        Set<String> invalids = datasourceUtils.validateDatasource(dsConfig, false);
+        assertTrue(invalids.contains("Missing URL."));
+    }
+}

app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/DatasourceStorageServiceCEImpl.java

@@ -204,8 +204,10 @@ public class DatasourceStorageServiceCEImpl implements DatasourceStorageServiceC
         return pluginExecutorMono
                 .map(pluginExecutor -> {
                     DatasourceConfiguration datasourceConfiguration = datasourceStorage.getDatasourceConfiguration();
-                    if (datasourceConfiguration != null && !pluginExecutor.isDatasourceValid(datasourceConfiguration)) {
+                    if (datasourceConfiguration != null && !pluginExecutor.isDatasourceValid(datasourceConfiguration,
-                        invalids.addAll(pluginExecutor.validateDatasource(datasourceConfiguration));
+                            datasourceStorage.isEmbedded())) {
+                        invalids.addAll(pluginExecutor.validateDatasource(datasourceConfiguration,
+                                datasourceStorage.isEmbedded()));
                     }
 
                     return datasourceStorage;