[Enhancement] Enhance validation for create connector API

This change will address the second part of validation "pre and post processing function validation".

Partially resolves opensearch-project#2993

Signed-off-by: Abdul Muneer Kolarkunnu <muneer.kolarkunnu@netapp.com>

Author: akolarkunnu

common/src/main/java/org/opensearch/ml/common/connector/ConnectorAction.java

@@ -6,10 +6,6 @@
 package org.opensearch.ml.common.connector;
 
 import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken;
-import static org.opensearch.ml.common.connector.MLPostProcessFunction.DEFAULT_EMBEDDING;
-import static org.opensearch.ml.common.connector.MLPostProcessFunction.DEFAULT_RERANK;
-import static org.opensearch.ml.common.connector.MLPreProcessFunction.TEXT_DOCS_TO_DEFAULT_EMBEDDING_INPUT;
-import static org.opensearch.ml.common.connector.MLPreProcessFunction.TEXT_SIMILARITY_TO_DEFAULT_INPUT;
 
 import java.io.IOException;
 import java.util.HashSet;
@@ -19,6 +15,8 @@
 import java.util.Set;
 
 import org.apache.commons.text.StringSubstitutor;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.opensearch.core.common.io.stream.StreamInput;
 import org.opensearch.core.common.io.stream.StreamOutput;
 import org.opensearch.core.common.io.stream.Writeable;
@@ -45,9 +43,11 @@ public class ConnectorAction implements ToXContentObject, Writeable {
     public static final String COHERE = "cohere";
     public static final String BEDROCK = "bedrock";
     public static final String SAGEMAKER = "sagemaker";
+    public static final String SAGEMAKER_PRE_POST_FUNC_TEXT = "default";
     public static final List<String> SUPPORTED_REMOTE_SERVERS_FOR_DEFAULT_ACTION_TYPES = List.of(SAGEMAKER, OPENAI, BEDROCK, COHERE);
 
     private static final String INBUILT_FUNC_PREFIX = "connector.";
+    private static final Logger logger = LogManager.getLogger(ConnectorAction.class);
 
     private ActionType actionType;
     private String method;
@@ -210,16 +210,12 @@ public void validatePrePostProcessFunctions(Map<String, String> parameters) {
         StringSubstitutor substitutor = new StringSubstitutor(parameters, "${parameters.", "}");
         String endPoint = substitutor.replace(url);
         String remoteServer = getRemoteServerFromURL(endPoint);
-        if (isInBuiltProcessFunction(preProcessFunction)) {
-            validatePreProcessFunctions(remoteServer);
-        }
-        if (isInBuiltProcessFunction(postProcessFunction)) {
-            validatePostProcessFunctions(remoteServer);
-        }
+        validateProcessFunctions(remoteServer, preProcessFunction);
+        validateProcessFunctions(remoteServer, postProcessFunction);
     }
 
     /**
-     * To get the remote server name from ULR
+     * To get the remote server name from url
      *
      * @param url - remote server url
      * @return - returns the corresponding remote server name for url, if server is not in the pre-defined list,
@@ -229,70 +225,38 @@ public static String getRemoteServerFromURL(String url) {
         return SUPPORTED_REMOTE_SERVERS_FOR_DEFAULT_ACTION_TYPES.stream().filter(url::contains).findFirst().orElse("");
     }
 
-    private boolean isInBuiltProcessFunction(String processFunction) {
-        return (processFunction != null && processFunction.startsWith(INBUILT_FUNC_PREFIX));
-    }
-
-    private void validatePreProcessFunctions(String remoteServer) {
-        switch (remoteServer) {
-            case OPENAI:
-                if (!preProcessFunction.contains(OPENAI)) {
-                    throw new IllegalArgumentException(invalidProcessFuncExcText(OPENAI, "PreProcessFunction"));
-                }
-                break;
-            case COHERE:
-                if (!preProcessFunction.contains(COHERE)) {
-                    throw new IllegalArgumentException(invalidProcessFuncExcText(COHERE, "PreProcessFunction"));
-                }
-                break;
-            case BEDROCK:
-                if (!preProcessFunction.contains(BEDROCK)) {
-                    throw new IllegalArgumentException(invalidProcessFuncExcText(BEDROCK, "PreProcessFunction"));
-                }
-                break;
-            case SAGEMAKER:
-                if (!(TEXT_DOCS_TO_DEFAULT_EMBEDDING_INPUT.equals(preProcessFunction)
-                    || TEXT_SIMILARITY_TO_DEFAULT_INPUT.equals(preProcessFunction))) {
-                    throw new IllegalArgumentException(
-                        "LLM service is "
-                            + SAGEMAKER
-                            + ", so PreProcessFunction should be "
-                            + TEXT_DOCS_TO_DEFAULT_EMBEDDING_INPUT
-                            + " or "
-                            + TEXT_SIMILARITY_TO_DEFAULT_INPUT
-                    );
-                }
+    private void validateProcessFunctions(String remoteServer, String processFunction) {
+        if (isInBuiltProcessFunction(processFunction)) {
+            switch (remoteServer) {
+                case OPENAI:
+                    if (!processFunction.contains(OPENAI)) {
+                        logger.warn(invalidProcessFuncWarnText(OPENAI));
+                    }
+                    break;
+                case COHERE:
+                    if (!processFunction.contains(COHERE)) {
+                        logger.warn(invalidProcessFuncWarnText(COHERE));
+                    }
+                    break;
+                case BEDROCK:
+                    if (!processFunction.contains(BEDROCK)) {
+                        logger.warn(invalidProcessFuncWarnText(BEDROCK));
+                    }
+                    break;
+                case SAGEMAKER:
+                    if (!processFunction.contains(SAGEMAKER_PRE_POST_FUNC_TEXT)) {
+                        logger.warn(invalidProcessFuncWarnText(SAGEMAKER));
+                    }
+            }
         }
     }
 
-    private void validatePostProcessFunctions(String remoteServer) {
-        switch (remoteServer) {
-            case OPENAI:
-                if (!postProcessFunction.contains(OPENAI)) {
-                    throw new IllegalArgumentException(invalidProcessFuncExcText(OPENAI, "PostProcessFunction"));
-                }
-                break;
-            case COHERE:
-                if (!postProcessFunction.contains(COHERE)) {
-                    throw new IllegalArgumentException(invalidProcessFuncExcText(COHERE, "PostProcessFunction"));
-                }
-                break;
-            case BEDROCK:
-                if (!postProcessFunction.contains(BEDROCK)) {
-                    throw new IllegalArgumentException(invalidProcessFuncExcText(BEDROCK, "PostProcessFunction"));
-                }
-                break;
-            case SAGEMAKER:
-                if (!(DEFAULT_EMBEDDING.equals(postProcessFunction) || DEFAULT_RERANK.equals(postProcessFunction))) {
-                    throw new IllegalArgumentException(
-                        "LLM service is " + SAGEMAKER + ", so PostProcessFunction should be " + DEFAULT_EMBEDDING + " or " + DEFAULT_RERANK
-                    );
-                }
-        }
+    private boolean isInBuiltProcessFunction(String processFunction) {
+        return (processFunction != null && processFunction.startsWith(INBUILT_FUNC_PREFIX));
     }
 
-    private String invalidProcessFuncExcText(String remoteServer, String func) {
-        return "LLM service is " + remoteServer + ", so " + func + " should be " + remoteServer + " " + func;
+    private String invalidProcessFuncWarnText(String remoteServer) {
+        return "LLM service is " + remoteServer + ", so PrePostProcessFunction should be corresponding to " + remoteServer;
     }
 
     public enum ActionType {

common/src/test/java/org/opensearch/ml/common/connector/ConnectorActionTest.java

@@ -122,7 +122,8 @@ public void testValidatePrePostProcessFunctionsWithOpenAIConnectorCorrectInBuilt
     }
 
     @Test
-    public void testValidatePrePostProcessFunctionsWithOpenAIConnectorWrongInBuiltPrePostProcessFunctionThrowsException() {
+    public void testValidatePrePostProcessFunctionsWithOpenAIConnectorWrongInBuiltPrePostProcessFunction() {
+        // Testing with wrong PreProcessFunction
         ConnectorAction action1 = new ConnectorAction(
             TEST_ACTION_TYPE,
             TEST_METHOD_HTTP,
@@ -132,8 +133,9 @@ public void testValidatePrePostProcessFunctionsWithOpenAIConnectorWrongInBuiltPr
             TEXT_DOCS_TO_BEDROCK_EMBEDDING_INPUT,
             OPENAI_EMBEDDING
         );
-        Throwable exception = assertThrows(IllegalArgumentException.class, () -> action1.validatePrePostProcessFunctions(Map.of()));
-        assertEquals("LLM service is openai, so PreProcessFunction should be openai PreProcessFunction", exception.getMessage());
+        action1.validatePrePostProcessFunctions(Map.of());
+
+        // Testing with wrong PostProcessFunction
         ConnectorAction action2 = new ConnectorAction(
             TEST_ACTION_TYPE,
             TEST_METHOD_HTTP,
@@ -143,8 +145,7 @@ public void testValidatePrePostProcessFunctionsWithOpenAIConnectorWrongInBuiltPr
             TEXT_DOCS_TO_OPENAI_EMBEDDING_INPUT,
             COHERE_EMBEDDING
         );
-        exception = assertThrows(IllegalArgumentException.class, () -> action2.validatePrePostProcessFunctions(Map.of()));
-        assertEquals("LLM service is openai, so PostProcessFunction should be openai PostProcessFunction", exception.getMessage());
+        action2.validatePrePostProcessFunctions(Map.of());
     }
 
     @Test
@@ -183,7 +184,8 @@ public void testValidatePrePostProcessFunctionsWithCohereConnectorCorrectInBuilt
     }
 
     @Test
-    public void testValidatePrePostProcessFunctionsWithCohereConnectorWrongInBuiltPrePostProcessFunctionThrowsException() {
+    public void testValidatePrePostProcessFunctionsWithCohereConnectorWrongInBuiltPrePostProcessFunction() {
+        // Testing with wrong PreProcessFunction
         ConnectorAction action1 = new ConnectorAction(
             TEST_ACTION_TYPE,
             TEST_METHOD_HTTP,
@@ -193,8 +195,9 @@ public void testValidatePrePostProcessFunctionsWithCohereConnectorWrongInBuiltPr
             TEXT_DOCS_TO_BEDROCK_EMBEDDING_INPUT,
             COHERE_EMBEDDING
         );
-        Throwable exception = assertThrows(IllegalArgumentException.class, () -> action1.validatePrePostProcessFunctions(Map.of()));
-        assertEquals("LLM service is cohere, so PreProcessFunction should be cohere PreProcessFunction", exception.getMessage());
+        action1.validatePrePostProcessFunctions(Map.of());
+
+        // Testing with wrong PostProcessFunction
         ConnectorAction action2 = new ConnectorAction(
             TEST_ACTION_TYPE,
             TEST_METHOD_HTTP,
@@ -204,8 +207,7 @@ public void testValidatePrePostProcessFunctionsWithCohereConnectorWrongInBuiltPr
             TEXT_DOCS_TO_COHERE_EMBEDDING_INPUT,
             OPENAI_EMBEDDING
         );
-        exception = assertThrows(IllegalArgumentException.class, () -> action2.validatePrePostProcessFunctions(Map.of()));
-        assertEquals("LLM service is cohere, so PostProcessFunction should be cohere PostProcessFunction", exception.getMessage());
+        action2.validatePrePostProcessFunctions(Map.of());
     }
 
     @Test
@@ -243,7 +245,8 @@ public void testValidatePrePostProcessFunctionsWithBedrockConnectorCorrectInBuil
     }
 
     @Test
-    public void testValidatePrePostProcessFunctionsWithBedrockConnectorWrongInBuiltPrePostProcessFunctionThrowsException() {
+    public void testValidatePrePostProcessFunctionsWithBedrockConnectorWrongInBuiltPrePostProcessFunction() {
+        // Testing with wrong PreProcessFunction
         ConnectorAction action1 = new ConnectorAction(
             TEST_ACTION_TYPE,
             TEST_METHOD_HTTP,
@@ -253,8 +256,9 @@ public void testValidatePrePostProcessFunctionsWithBedrockConnectorWrongInBuiltP
             TEXT_DOCS_TO_COHERE_EMBEDDING_INPUT,
             BEDROCK_EMBEDDING
         );
-        Throwable exception = assertThrows(IllegalArgumentException.class, () -> action1.validatePrePostProcessFunctions(Map.of()));
-        assertEquals("LLM service is bedrock, so PreProcessFunction should be bedrock PreProcessFunction", exception.getMessage());
+        action1.validatePrePostProcessFunctions(Map.of());
+
+        // Testing with wrong PostProcessFunction
         ConnectorAction action2 = new ConnectorAction(
             TEST_ACTION_TYPE,
             TEST_METHOD_HTTP,
@@ -264,8 +268,7 @@ public void testValidatePrePostProcessFunctionsWithBedrockConnectorWrongInBuiltP
             TEXT_IMAGE_TO_BEDROCK_EMBEDDING_INPUT,
             COHERE_EMBEDDING
         );
-        exception = assertThrows(IllegalArgumentException.class, () -> action2.validatePrePostProcessFunctions(Map.of()));
-        assertEquals("LLM service is bedrock, so PostProcessFunction should be bedrock PostProcessFunction", exception.getMessage());
+        action2.validatePrePostProcessFunctions(Map.of());
     }
 
     @Test
@@ -293,7 +296,8 @@ public void testValidatePrePostProcessFunctionsWithSagemakerConnectorWithCorrect
     }
 
     @Test
-    public void testValidatePrePostProcessFunctionsWithSagemakerConnectorWrongInBuiltPrePostProcessFunctionThrowsException() {
+    public void testValidatePrePostProcessFunctionsWithSagemakerConnectorWrongInBuiltPrePostProcessFunction() {
+        // Testing with wrong PreProcessFunction
         ConnectorAction action1 = new ConnectorAction(
             TEST_ACTION_TYPE,
             TEST_METHOD_HTTP,
@@ -303,12 +307,9 @@ public void testValidatePrePostProcessFunctionsWithSagemakerConnectorWrongInBuil
             TEXT_DOCS_TO_COHERE_EMBEDDING_INPUT,
             DEFAULT_EMBEDDING
         );
-        Throwable exception = assertThrows(IllegalArgumentException.class, () -> action1.validatePrePostProcessFunctions(Map.of()));
-        assertEquals(
-            "LLM service is sagemaker, so PreProcessFunction should be connector.pre_process.default.embedding"
-                + " or connector.pre_process.default.rerank",
-            exception.getMessage()
-        );
+        action1.validatePrePostProcessFunctions(Map.of());
+
+        // Testing with wrong PostProcessFunction
         ConnectorAction action2 = new ConnectorAction(
             TEST_ACTION_TYPE,
             TEST_METHOD_HTTP,
@@ -318,12 +319,7 @@ public void testValidatePrePostProcessFunctionsWithSagemakerConnectorWrongInBuil
             TEXT_DOCS_TO_DEFAULT_EMBEDDING_INPUT,
             BEDROCK_EMBEDDING
         );
-        exception = assertThrows(IllegalArgumentException.class, () -> action2.validatePrePostProcessFunctions(Map.of()));
-        assertEquals(
-            "LLM service is sagemaker, so PostProcessFunction should be connector.post_process.default.embedding"
-                + " or connector.post_process.default.rerank",
-            exception.getMessage()
-        );
+        action2.validatePrePostProcessFunctions(Map.of());
     }
 
     @Test