Refactor WorkflowResource Strings and other String reorganization (#336)

* Reorganize / Refactor String constants Signed-off-by: Daniel Widdis <[email protected]> * Replace use of resource fields with constants Signed-off-by: Daniel Widdis <[email protected]> --------- Signed-off-by: Daniel Widdis <[email protected]>
opensearch-project · Dec 28, 2023 · c3ba8f0 · c3ba8f0
1 parent 0855f8f
commit c3ba8f0
Show file tree

Hide file tree

Showing 37 changed files with 238 additions and 177 deletions.
diff --git a/src/main/java/org/opensearch/flowframework/common/CommonValue.java b/src/main/java/org/opensearch/flowframework/common/CommonValue.java
@@ -15,8 +15,11 @@ public class CommonValue {
 
     private CommonValue() {}
 
+    /*
+     * Constants associated with Global Context
+     */
     /** Default value for no schema version */
-    public static Integer NO_SCHEMA_VERSION = 0;
+    public static final Integer NO_SCHEMA_VERSION = 0;
     /** Index mapping meta field name*/
     public static final String META = "_meta";
     /** Schema Version field name */
@@ -43,18 +46,12 @@ private CommonValue() {}
     public static final String MASTER_KEY = "master_key";
     /** Create Time field  name */
     public static final String CREATE_TIME = "create_time";
-
-    /** The template field name for template use case */
-    public static final String USE_CASE_FIELD = "use_case";
-    /** The template field name for template version */
-    public static final String TEMPLATE_FIELD = "template";
-    /** The template field name for template compatibility with OpenSearch versions */
-    public static final String COMPATIBILITY_FIELD = "compatibility";
-    /** The template field name for template workflows */
-    public static final String WORKFLOWS_FIELD = "workflows";
     /** The template field name for the user who created the workflow **/
     public static final String USER_FIELD = "user";
 
+    /*
+     * Constants associated with Rest or Transport actions
+     */
     /** The transport action name prefix */
     public static final String TRANSPORT_ACTION_NAME_PREFIX = "cluster:admin/opensearch/flow_framework/";
     /** The base URI for this plugin's rest actions */
@@ -68,23 +65,25 @@ private CommonValue() {}
     /** The field name for provision workflow within a use case template*/
     public static final String PROVISION_WORKFLOW = "provision";
 
+    /*
+     * Constants associated with plugin configuration
+     */
     /** Flow Framework plugin thread pool name prefix */
     public static final String FLOW_FRAMEWORK_THREAD_POOL_PREFIX = "thread_pool.flow_framework.";
     /** The provision workflow thread pool name */
     public static final String PROVISION_THREAD_POOL = "opensearch_workflow_provision";
 
+    /*
+     * Field names common to multiple classes
+     */
     /** Success name field */
     public static final String SUCCESS = "success";
-    /** Index name field */
-    public static final String INDEX_NAME = "index_name";
     /** Type field */
     public static final String TYPE = "type";
     /** default_mapping_option filed */
     public static final String DEFAULT_MAPPING_OPTION = "default_mapping_option";
     /** ID Field */
     public static final String ID = "id";
-    /** Pipeline Id field */
-    public static final String PIPELINE_ID = "pipeline_id";
     /** Processors field */
     public static final String PROCESSORS = "processors";
     /** Field map field */
@@ -93,8 +92,6 @@ private CommonValue() {}
     public static final String INPUT_FIELD_NAME = "input_field_name";
     /** Output Field Name field */
     public static final String OUTPUT_FIELD_NAME = "output_field_name";
-    /** Model Id field */
-    public static final String MODEL_ID = "model_id";
     /** Task Id field */
     public static final String TASK_ID = "task_id";
     /** Register Model Status field */
@@ -106,13 +103,9 @@ private CommonValue() {}
     /** Model Version field */
     public static final String MODEL_VERSION = "model_version";
     /** Model Group Id field */
-    public static final String MODEL_GROUP_ID = "model_group_id";
-    /** Model Group Id field */
     public static final String MODEL_GROUP_STATUS = "model_group_status";
     /** Description field */
     public static final String DESCRIPTION_FIELD = "description";
-    /** Connector Id field */
-    public static final String CONNECTOR_ID = "connector_id";
     /** Model format field */
     public static final String MODEL_FORMAT = "model_format";
     /** Model content hash value field */
@@ -145,7 +138,24 @@ private CommonValue() {}
     public static final String MODEL_ACCESS_MODE = "access_mode";
     /** Add all backend roles */
     public static final String ADD_ALL_BACKEND_ROLES = "add_all_backend_roles";
+    /** The tools field for an agent */
+    public static final String TOOLS_FIELD = "tools";
+    /** The tools order field for an agent */
+    public static final String TOOLS_ORDER_FIELD = "tools_order";
+    /** The memory field for an agent */
+    public static final String MEMORY_FIELD = "memory";
+    /** The app type field for an agent */
+    public static final String APP_TYPE_FIELD = "app_type";
+    /** To include field for an agent response */
+    public static final String INCLUDE_OUTPUT_IN_AGENT_RESPONSE = "include_output_in_agent_response";
+    /** The created time field for an agent */
+    public static final String CREATED_TIME = "created_time";
+    /** The last updated time field for an agent */
+    public static final String LAST_UPDATED_TIME_FIELD = "last_updated_time";
 
+    /*
+     * Constants associated with resource provisioning / state
+     */
     /** The template field name for the associated workflowID **/
     public static final String WORKFLOW_ID_FIELD = "workflow_id";
     /** The template field name for the workflow error **/
@@ -172,20 +182,4 @@ private CommonValue() {}
     public static final String RESOURCE_TYPE = "resource_type";
     /** The field name for the resource id */
     public static final String RESOURCE_ID = "resource_id";
-    /** The tools field for an agent */
-    public static final String TOOLS_FIELD = "tools";
-    /** The tools order field for an agent */
-    public static final String TOOLS_ORDER_FIELD = "tools_order";
-    /** The memory field for an agent */
-    public static final String MEMORY_FIELD = "memory";
-    /** The app type field for an agent */
-    public static final String APP_TYPE_FIELD = "app_type";
-    /** The agent id of an agent */
-    public static final String AGENT_ID = "agent_id";
-    /** To include field for an agent response */
-    public static final String INCLUDE_OUTPUT_IN_AGENT_RESPONSE = "include_output_in_agent_response";
-    /** The created time field for an agent */
-    public static final String CREATED_TIME = "created_time";
-    /** The last updated time field for an agent */
-    public static final String LAST_UPDATED_TIME_FIELD = "last_updated_time";
 }
diff --git a/src/main/java/org/opensearch/flowframework/common/WorkflowResources.java b/src/main/java/org/opensearch/flowframework/common/WorkflowResources.java
@@ -12,6 +12,18 @@
 import org.apache.logging.log4j.Logger;
 import org.opensearch.core.rest.RestStatus;
 import org.opensearch.flowframework.exception.FlowFrameworkException;
+import org.opensearch.flowframework.workflow.CreateConnectorStep;
+import org.opensearch.flowframework.workflow.CreateIndexStep;
+import org.opensearch.flowframework.workflow.CreateIngestPipelineStep;
+import org.opensearch.flowframework.workflow.DeleteAgentStep;
+import org.opensearch.flowframework.workflow.DeleteConnectorStep;
+import org.opensearch.flowframework.workflow.DeleteModelStep;
+import org.opensearch.flowframework.workflow.DeployModelStep;
+import org.opensearch.flowframework.workflow.ModelGroupStep;
+import org.opensearch.flowframework.workflow.RegisterAgentStep;
+import org.opensearch.flowframework.workflow.RegisterLocalModelStep;
+import org.opensearch.flowframework.workflow.RegisterRemoteModelStep;
+import org.opensearch.flowframework.workflow.UndeployModelStep;
 
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -22,22 +34,35 @@
  */
 public enum WorkflowResources {
 
-    /** official workflow step name for creating a connector and associated created resource */
-    CREATE_CONNECTOR("create_connector", "connector_id", "delete_connector"),
-    /** official workflow step name for registering a remote model and associated created resource */
-    REGISTER_REMOTE_MODEL("register_remote_model", "model_id", "delete_model"),
-    /** official workflow step name for registering a local model and associated created resource */
-    REGISTER_LOCAL_MODEL("register_local_model", "model_id", "delete_model"),
-    /** official workflow step name for registering a model group and associated created resource */
-    REGISTER_MODEL_GROUP("register_model_group", "model_group_id", null), // TODO
-    /** official workflow step name for deploying a model and associated created resource */
-    DEPLOY_MODEL("deploy_model", "model_id", "undeploy_model"),
-    /** official workflow step name for creating an ingest-pipeline and associated created resource */
-    CREATE_INGEST_PIPELINE("create_ingest_pipeline", "pipeline_id", null), // TODO
-    /** official workflow step name for creating an index and associated created resource */
-    CREATE_INDEX("create_index", "index_name", null), // TODO
-    /** official workflow step name for register an agent and the associated created resource */
-    REGISTER_AGENT("register_agent", "agent_id", "delete_agent");
+    /** Workflow steps for creating/deleting a connector and associated created resource */
+    CREATE_CONNECTOR(CreateConnectorStep.NAME, WorkflowResources.CONNECTOR_ID, DeleteConnectorStep.NAME),
+    /** Workflow steps for registering/deleting a remote model and associated created resource */
+    REGISTER_REMOTE_MODEL(RegisterRemoteModelStep.NAME, WorkflowResources.MODEL_ID, DeleteModelStep.NAME),
+    /** Workflow steps for registering/deleting a local model and associated created resource */
+    REGISTER_LOCAL_MODEL(RegisterLocalModelStep.NAME, WorkflowResources.MODEL_ID, DeleteModelStep.NAME),
+    /** Workflow steps for registering a model group and associated created resource */
+    REGISTER_MODEL_GROUP(ModelGroupStep.NAME, WorkflowResources.MODEL_GROUP_ID, null), // TODO delete step
+    /** Workflow steps for deploying/undeploying a model and associated created resource */
+    DEPLOY_MODEL(DeployModelStep.NAME, WorkflowResources.MODEL_ID, UndeployModelStep.NAME),
+    /** Workflow steps for creating an ingest-pipeline and associated created resource */
+    CREATE_INGEST_PIPELINE(CreateIngestPipelineStep.NAME, WorkflowResources.PIPELINE_ID, null), // TODO delete step
+    /** Workflow steps for creating an index and associated created resource */
+    CREATE_INDEX(CreateIndexStep.NAME, WorkflowResources.INDEX_NAME, null), // TODO delete step
+    /** Workflow steps for registering/deleting an agent and the associated created resource */
+    REGISTER_AGENT(RegisterAgentStep.NAME, WorkflowResources.AGENT_ID, DeleteAgentStep.NAME);
+
+    /** Connector Id for a remote model connector */
+    public static final String CONNECTOR_ID = "connector_id";
+    /** Model Id for an ML model */
+    public static final String MODEL_ID = "model_id";
+    /** Model Group Id */
+    public static final String MODEL_GROUP_ID = "model_group_id";
+    /** Pipeline Id for Ingest Pipeline */
+    public static final String PIPELINE_ID = "pipeline_id";
+    /** Index name */
+    public static final String INDEX_NAME = "index_name";
+    /** Agent Id */
+    public static final String AGENT_ID = "agent_id";
 
     private final String workflowStep;
     private final String resourceCreated;

diff --git a/src/main/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandler.java b/src/main/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandler.java
@@ -32,7 +32,6 @@
 import org.opensearch.core.rest.RestStatus;
 import org.opensearch.core.xcontent.ToXContent;
 import org.opensearch.core.xcontent.XContentBuilder;
-import org.opensearch.flowframework.common.WorkflowResources;
 import org.opensearch.flowframework.exception.FlowFrameworkException;
 import org.opensearch.flowframework.model.ProvisioningProgress;
 import org.opensearch.flowframework.model.ResourceCreated;
@@ -60,6 +59,7 @@
 import static org.opensearch.flowframework.common.CommonValue.SCHEMA_VERSION_FIELD;
 import static org.opensearch.flowframework.common.CommonValue.WORKFLOW_STATE_INDEX;
 import static org.opensearch.flowframework.common.CommonValue.WORKFLOW_STATE_INDEX_MAPPING;
+import static org.opensearch.flowframework.common.WorkflowResources.getResourceByWorkflowStep;
 
 /**
  * A handler for operations on system indices in the AI Flow Framework plugin
@@ -503,7 +503,7 @@ public void updateResourceInStateIndex(
         ResourceCreated newResource = new ResourceCreated(
             workflowStepName,
             nodeId,
-            WorkflowResources.getResourceByWorkflowStep(workflowStepName),
+            getResourceByWorkflowStep(workflowStepName),
             resourceId
         );
         XContentBuilder builder = XContentBuilder.builder(XContentType.JSON.xContent());

diff --git a/src/main/java/org/opensearch/flowframework/model/ResourceCreated.java b/src/main/java/org/opensearch/flowframework/model/ResourceCreated.java
@@ -30,7 +30,6 @@
 /**
  * This represents an object in the WorkflowState {@link WorkflowState}.
  */
-// TODO: create an enum to add the resource name itself for each step example (create_connector_step -> connector)
 public class ResourceCreated implements ToXContentObject, Writeable {
 
     private static final Logger logger = LogManager.getLogger(ResourceCreated.class);

diff --git a/src/main/java/org/opensearch/flowframework/model/Template.java b/src/main/java/org/opensearch/flowframework/model/Template.java
@@ -28,21 +28,26 @@
 import java.util.Map.Entry;
 
 import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken;
-import static org.opensearch.flowframework.common.CommonValue.COMPATIBILITY_FIELD;
 import static org.opensearch.flowframework.common.CommonValue.DESCRIPTION_FIELD;
 import static org.opensearch.flowframework.common.CommonValue.NAME_FIELD;
-import static org.opensearch.flowframework.common.CommonValue.TEMPLATE_FIELD;
 import static org.opensearch.flowframework.common.CommonValue.UI_METADATA_FIELD;
 import static org.opensearch.flowframework.common.CommonValue.USER_FIELD;
-import static org.opensearch.flowframework.common.CommonValue.USE_CASE_FIELD;
 import static org.opensearch.flowframework.common.CommonValue.VERSION_FIELD;
-import static org.opensearch.flowframework.common.CommonValue.WORKFLOWS_FIELD;
 
 /**
  * The Template is the central data structure which configures workflows. This object is used to parse JSON communicated via REST API.
  */
 public class Template implements ToXContentObject, Writeable {
 
+    /** The template field name for template workflows */
+    public static final String WORKFLOWS_FIELD = "workflows";
+    /** The template field name for template compatibility with OpenSearch versions */
+    public static final String COMPATIBILITY_FIELD = "compatibility";
+    /** The template field name for template version */
+    public static final String TEMPLATE_FIELD = "template";
+    /** The template field name for template use case */
+    public static final String USE_CASE_FIELD = "use_case";
+
     private String name;
     private String description;
     private String useCase; // probably an ENUM actually
@@ -351,13 +356,16 @@ public static Template parse(XContentParser parser) throws IOException {
      * @throws IOException on failure to parse
      */
     public static Template parse(String json) throws IOException {
-        XContentParser parser = JsonXContent.jsonXContent.createParser(
-            NamedXContentRegistry.EMPTY,
-            LoggingDeprecationHandler.INSTANCE,
-            json
-        );
-        ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser);
-        return parse(parser);
+        try (
+            XContentParser parser = JsonXContent.jsonXContent.createParser(
+                NamedXContentRegistry.EMPTY,
+                LoggingDeprecationHandler.INSTANCE,
+                json
+            )
+        ) {
+            ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser);
+            return parse(parser);
+        }
     }
 
     /**

diff --git a/src/main/java/org/opensearch/flowframework/util/ParseUtils.java b/src/main/java/org/opensearch/flowframework/util/ParseUtils.java
@@ -38,8 +38,8 @@
 import java.util.regex.Pattern;
 
 import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken;
-import static org.opensearch.flowframework.common.CommonValue.MODEL_ID;
 import static org.opensearch.flowframework.common.CommonValue.PARAMETERS_FIELD;
+import static org.opensearch.flowframework.common.WorkflowResources.MODEL_ID;
 
 /**
  * Utility methods for Template parsing

diff --git a/src/main/java/org/opensearch/flowframework/workflow/AbstractRetryableWorkflowStep.java b/src/main/java/org/opensearch/flowframework/workflow/AbstractRetryableWorkflowStep.java
@@ -16,7 +16,6 @@
 import org.opensearch.common.util.concurrent.FutureUtils;
 import org.opensearch.core.action.ActionListener;
 import org.opensearch.core.rest.RestStatus;
-import org.opensearch.flowframework.common.WorkflowResources;
 import org.opensearch.flowframework.exception.FlowFrameworkException;
 import org.opensearch.flowframework.indices.FlowFrameworkIndicesHandler;
 import org.opensearch.ml.client.MachineLearningNodeClient;
@@ -28,6 +27,8 @@
 
 import static org.opensearch.flowframework.common.CommonValue.REGISTER_MODEL_STATUS;
 import static org.opensearch.flowframework.common.FlowFrameworkSettings.MAX_GET_TASK_REQUEST_RETRY;
+import static org.opensearch.flowframework.common.WorkflowResources.DEPLOY_MODEL;
+import static org.opensearch.flowframework.common.WorkflowResources.getResourceByWorkflowStep;
 
 /**
  * Abstract retryable workflow step
@@ -90,9 +91,9 @@ protected void retryableGetMlTask(
             } else {
                 try {
                     logger.info(workflowStep + " successful for {} and modelId {}", workflowId, response.getModelId());
-                    String resourceName = WorkflowResources.getResourceByWorkflowStep(getName());
+                    String resourceName = getResourceByWorkflowStep(getName());
                     String id;
-                    if (getName().equals(WorkflowResources.DEPLOY_MODEL.getWorkflowStep())) {
+                    if (getName().equals(DEPLOY_MODEL.getWorkflowStep())) {
                         id = response.getModelId();
                     } else {
                         id = response.getTaskId();

diff --git a/src/main/java/org/opensearch/flowframework/workflow/CreateConnectorStep.java b/src/main/java/org/opensearch/flowframework/workflow/CreateConnectorStep.java
@@ -13,7 +13,6 @@
 import org.opensearch.ExceptionsHelper;
 import org.opensearch.core.action.ActionListener;
 import org.opensearch.core.rest.RestStatus;
-import org.opensearch.flowframework.common.WorkflowResources;
 import org.opensearch.flowframework.exception.FlowFrameworkException;
 import org.opensearch.flowframework.indices.FlowFrameworkIndicesHandler;
 import org.opensearch.flowframework.util.ParseUtils;
@@ -43,6 +42,7 @@
 import static org.opensearch.flowframework.common.CommonValue.PARAMETERS_FIELD;
 import static org.opensearch.flowframework.common.CommonValue.PROTOCOL_FIELD;
 import static org.opensearch.flowframework.common.CommonValue.VERSION_FIELD;
+import static org.opensearch.flowframework.common.WorkflowResources.getResourceByWorkflowStep;
 import static org.opensearch.flowframework.util.ParseUtils.getStringToStringMap;
 
 /**
@@ -55,7 +55,8 @@ public class CreateConnectorStep implements WorkflowStep {
     private MachineLearningNodeClient mlClient;
     private final FlowFrameworkIndicesHandler flowFrameworkIndicesHandler;
 
-    static final String NAME = WorkflowResources.CREATE_CONNECTOR.getWorkflowStep();
+    /** The name of this step, used as a key in the template and the {@link WorkflowStepFactory} */
+    public static final String NAME = "create_connector";
 
     /**
      * Instantiate this class
@@ -83,7 +84,7 @@ public CompletableFuture<WorkflowData> execute(
             public void onResponse(MLCreateConnectorResponse mlCreateConnectorResponse) {
 
                 try {
-                    String resourceName = WorkflowResources.getResourceByWorkflowStep(getName());
+                    String resourceName = getResourceByWorkflowStep(getName());
                     logger.info("Created connector successfully");
                     flowFrameworkIndicesHandler.updateResourceInStateIndex(
                         currentNodeInputs.getWorkflowId(),