From 24b354252212db0d58f2dcad978eb3f3dee7db3f Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Tue, 19 Sep 2023 10:47:19 -0700 Subject: [PATCH 01/27] Add WorkflowStepFactory class Signed-off-by: Daniel Widdis --- src/main/java/demo/DataDemo.java | 13 +-- src/main/java/demo/Demo.java | 16 +--- .../template/TemplateParser.java | 25 ++---- .../workflow/WorkflowStepFactory.java | 80 +++++++++++++++++++ .../template/TemplateParserTests.java | 2 +- src/test/resources/template/datademo.json | 2 + src/test/resources/template/demo.json | 12 ++- 7 files changed, 101 insertions(+), 49 deletions(-) create mode 100644 src/main/java/org/opensearch/flowframework/workflow/WorkflowStepFactory.java diff --git a/src/main/java/demo/DataDemo.java b/src/main/java/demo/DataDemo.java index f2d606f07..4f304d2e5 100644 --- a/src/main/java/demo/DataDemo.java +++ b/src/main/java/demo/DataDemo.java @@ -14,16 +14,13 @@ import org.opensearch.common.io.PathUtils; import org.opensearch.flowframework.template.ProcessNode; import org.opensearch.flowframework.template.TemplateParser; -import org.opensearch.flowframework.workflow.WorkflowStep; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Locale; -import java.util.Map; import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.stream.Collectors; @@ -35,14 +32,6 @@ public class DataDemo { private static final Logger logger = LogManager.getLogger(DataDemo.class); - // This is temporary. We need a factory class to generate these workflow steps - // based on a field in the JSON. - private static Map workflowMap = new HashMap<>(); - static { - workflowMap.put("create_index", new CreateIndexWorkflowStep()); - workflowMap.put("create_another_index", new CreateIndexWorkflowStep()); - } - /** * Demonstrate parsing a JSON graph. * @@ -60,7 +49,7 @@ public static void main(String[] args) { } logger.info("Parsing graph to sequence..."); - List processSequence = TemplateParser.parseJsonGraphToSequence(json, workflowMap); + List processSequence = TemplateParser.parseJsonGraphToSequence(json); List> futureList = new ArrayList<>(); for (ProcessNode n : processSequence) { diff --git a/src/main/java/demo/Demo.java b/src/main/java/demo/Demo.java index 58d977827..15dde22a9 100644 --- a/src/main/java/demo/Demo.java +++ b/src/main/java/demo/Demo.java @@ -14,16 +14,13 @@ import org.opensearch.common.io.PathUtils; import org.opensearch.flowframework.template.ProcessNode; import org.opensearch.flowframework.template.TemplateParser; -import org.opensearch.flowframework.workflow.WorkflowStep; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Locale; -import java.util.Map; import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.stream.Collectors; @@ -35,16 +32,6 @@ public class Demo { private static final Logger logger = LogManager.getLogger(Demo.class); - // This is temporary. We need a factory class to generate these workflow steps - // based on a field in the JSON. - private static Map workflowMap = new HashMap<>(); - static { - workflowMap.put("fetch_model", new DemoWorkflowStep(3000)); - workflowMap.put("create_ingest_pipeline", new DemoWorkflowStep(3000)); - workflowMap.put("create_search_pipeline", new DemoWorkflowStep(5000)); - workflowMap.put("create_neural_search_index", new DemoWorkflowStep(2000)); - } - /** * Demonstrate parsing a JSON graph. * @@ -62,7 +49,7 @@ public static void main(String[] args) { } logger.info("Parsing graph to sequence..."); - List processSequence = TemplateParser.parseJsonGraphToSequence(json, workflowMap); + List processSequence = TemplateParser.parseJsonGraphToSequence(json); List> futureList = new ArrayList<>(); for (ProcessNode n : processSequence) { @@ -78,7 +65,6 @@ public static void main(String[] args) { predecessors.stream().map(p -> p.id()).collect(Collectors.joining(", ")) ) ); - // TODO need to handle this better, passing an argument when we start them all at the beginning is silly futureList.add(n.execute()); } futureList.forEach(CompletableFuture::join); diff --git a/src/main/java/org/opensearch/flowframework/template/TemplateParser.java b/src/main/java/org/opensearch/flowframework/template/TemplateParser.java index 56635f1b4..6b0e3a841 100644 --- a/src/main/java/org/opensearch/flowframework/template/TemplateParser.java +++ b/src/main/java/org/opensearch/flowframework/template/TemplateParser.java @@ -13,9 +13,9 @@ import com.google.gson.JsonObject; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.opensearch.action.admin.indices.create.CreateIndexRequest; import org.opensearch.flowframework.workflow.WorkflowData; import org.opensearch.flowframework.workflow.WorkflowStep; +import org.opensearch.flowframework.workflow.WorkflowStepFactory; import java.util.ArrayDeque; import java.util.ArrayList; @@ -36,13 +36,16 @@ public class TemplateParser { private static final Logger logger = LogManager.getLogger(TemplateParser.class); - // Field names in the JSON. Package private for tests. + // Field names in the JSON. + // Currently package private for tests. + // These may eventually become part of the template definition in which case they might be better declared public static final String WORKFLOW = "sequence"; static final String NODES = "nodes"; static final String NODE_ID = "id"; static final String EDGES = "edges"; static final String SOURCE = "source"; static final String DESTINATION = "dest"; + static final String STEP_TYPE = "step_type"; /** * Prevent instantiating this class. @@ -52,10 +55,9 @@ private TemplateParser() {} /** * Parse a JSON representation of nodes and edges into a topologically sorted list of process nodes. * @param json A string containing a JSON representation of nodes and edges - * @param workflowSteps A map linking JSON node names to Java objects implementing {@link WorkflowStep} * @return A list of Process Nodes sorted topologically. All predecessors of any node will occur prior to it in the list. */ - public static List parseJsonGraphToSequence(String json, Map workflowSteps) { + public static List parseJsonGraphToSequence(String json) { Gson gson = new Gson(); JsonObject jsonObject = gson.fromJson(json, JsonObject.class); @@ -67,20 +69,9 @@ public static List parseJsonGraphToSequence(String json, Map stepMap = new HashMap<>(); + + private WorkflowStepFactory() { + populateMap(); + } + + /** + * Gets the singleton instance of this class + * @return The instance of this class + */ + public static WorkflowStepFactory get() { + return INSTANCE; + } + + private void populateMap() { + // TODO: These are from the demo class as placeholders + // Replace with actual implementations such as + // https://github.com/opensearch-project/opensearch-ai-flow-framework/pull/38 + // https://github.com/opensearch-project/opensearch-ai-flow-framework/pull/44 + stepMap.put("create_index", new CreateIndexWorkflowStep()); + stepMap.put("fetch_model", new DemoWorkflowStep(3000)); + stepMap.put("create_ingest_pipeline", new DemoWorkflowStep(3000)); + stepMap.put("create_search_pipeline", new DemoWorkflowStep(5000)); + stepMap.put("create_neural_search_index", new DemoWorkflowStep(2000)); + + // Use until all the actual implementations are ready + stepMap.put("placeholder", new WorkflowStep() { + @Override + public CompletableFuture execute(List data) { + CompletableFuture future = new CompletableFuture<>(); + future.complete(WorkflowData.EMPTY); + return future; + } + + @Override + public String getName() { + return "placeholder"; + } + }); + } + + /** + * Create a new instance of a {@link WorkflowStep}. + * @param type The type of instance to create + * @return an instance of the specified type + */ + public WorkflowStep createStep(String type) { + if (stepMap.containsKey(type)) { + return stepMap.get(type); + } + // TODO: replace this with a FlowFrameworkException + // https://github.com/opensearch-project/opensearch-ai-flow-framework/pull/43 + throw new UnsupportedOperationException("No workflow steps of type [" + type + "] are implemented."); + } +} diff --git a/src/test/java/org/opensearch/flowframework/template/TemplateParserTests.java b/src/test/java/org/opensearch/flowframework/template/TemplateParserTests.java index 24dcf0640..78e8ff28a 100644 --- a/src/test/java/org/opensearch/flowframework/template/TemplateParserTests.java +++ b/src/test/java/org/opensearch/flowframework/template/TemplateParserTests.java @@ -50,7 +50,7 @@ private static ProcessNode expectedNode(String id) { // Less verbose parser private static List parse(String json) { - return TemplateParser.parseJsonGraphToSequence(json, Collections.emptyMap()); + return TemplateParser.parseJsonGraphToSequence(json); } @Override diff --git a/src/test/resources/template/datademo.json b/src/test/resources/template/datademo.json index a1323ed2c..10a2bfdc6 100644 --- a/src/test/resources/template/datademo.json +++ b/src/test/resources/template/datademo.json @@ -3,10 +3,12 @@ "nodes": [ { "id": "create_index", + "step_type": "create_index", "index_name": "demo" }, { "id": "create_another_index", + "step_type": "create_index", "index_name": "second_demo" } ], diff --git a/src/test/resources/template/demo.json b/src/test/resources/template/demo.json index 38f1d0644..c068301e8 100644 --- a/src/test/resources/template/demo.json +++ b/src/test/resources/template/demo.json @@ -2,16 +2,20 @@ "sequence": { "nodes": [ { - "id": "fetch_model" + "id": "fetch_model", + "step_type": "fetch_model" }, { - "id": "create_ingest_pipeline" + "id": "create_ingest_pipeline", + "step_type": "create_ingest_pipeline" }, { - "id": "create_search_pipeline" + "id": "create_search_pipeline", + "step_type": "create_search_pipeline" }, { - "id": "create_neural_search_index" + "id": "create_neural_search_index", + "step_type": "create_neural_search_index" } ], "edges": [ From e70b9e8552cb2a17f210843856baf2e9b6ebe3ff Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Wed, 20 Sep 2023 16:22:24 -0700 Subject: [PATCH 02/27] Add XContent classes representing Template JSON Signed-off-by: Daniel Widdis --- .../flowframework/template/Template.java | 69 ++++++++++++++ .../template/TemplateParser.java | 16 ++-- ...essSequenceEdge.java => WorkflowEdge.java} | 23 ++++- .../flowframework/template/WorkflowNode.java | 49 ++++++++++ .../flowframework/workflow/Workflow.java | 59 ++++++++++++ ...eEdgeTests.java => WorkflowEdgeTests.java} | 8 +- .../resources/template/finaltemplate.json | 90 +++++++++++++++++++ 7 files changed, 298 insertions(+), 16 deletions(-) create mode 100644 src/main/java/org/opensearch/flowframework/template/Template.java rename src/main/java/org/opensearch/flowframework/template/{ProcessSequenceEdge.java => WorkflowEdge.java} (63%) create mode 100644 src/main/java/org/opensearch/flowframework/template/WorkflowNode.java create mode 100644 src/main/java/org/opensearch/flowframework/workflow/Workflow.java rename src/test/java/org/opensearch/flowframework/template/{ProcessSequenceEdgeTests.java => WorkflowEdgeTests.java} (70%) create mode 100644 src/test/resources/template/finaltemplate.json diff --git a/src/main/java/org/opensearch/flowframework/template/Template.java b/src/main/java/org/opensearch/flowframework/template/Template.java new file mode 100644 index 000000000..25e25b691 --- /dev/null +++ b/src/main/java/org/opensearch/flowframework/template/Template.java @@ -0,0 +1,69 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ +package org.opensearch.flowframework.template; + +import org.opensearch.Version; +import org.opensearch.core.xcontent.ToXContentObject; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.flowframework.workflow.Workflow; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; +import java.util.Map.Entry; + +/** + * 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 { + + // TODO: Some of thse are placeholders based on the template design + // Current code is only using user inputs and workflows + private String name = null; + private String description = null; + private String useCase = null; + private String[] operations = null; // probably an ENUM actually + private Version templateVersion = null; + private Version[] compatibilityVersion = null; + private Map userInputs = new HashMap<>(); + private Map workflows = new HashMap<>(); + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + XContentBuilder xContentBuilder = builder.startObject(); + xContentBuilder.field("name", this.name); + xContentBuilder.field("description", this.description); + xContentBuilder.field("use_case", this.useCase); + xContentBuilder.field("operations", this.operations); + + xContentBuilder.startObject("version"); + xContentBuilder.field("template", this.templateVersion); + xContentBuilder.startArray("compatibility"); + for (Version v : this.compatibilityVersion) { + xContentBuilder.value(v); + } + xContentBuilder.endArray(); + xContentBuilder.endObject(); + + xContentBuilder.startObject("user_inputs"); + for (Entry e : userInputs.entrySet()) { + xContentBuilder.field(e.getKey(), e.getValue()); + } + xContentBuilder.endObject(); + + xContentBuilder.startObject("workflows"); + for (Entry e : workflows.entrySet()) { + xContentBuilder.field(e.getKey(), e.getValue(), params); + } + xContentBuilder.endObject(); + + return xContentBuilder.endObject(); + } + +} diff --git a/src/main/java/org/opensearch/flowframework/template/TemplateParser.java b/src/main/java/org/opensearch/flowframework/template/TemplateParser.java index 6b0e3a841..884f18174 100644 --- a/src/main/java/org/opensearch/flowframework/template/TemplateParser.java +++ b/src/main/java/org/opensearch/flowframework/template/TemplateParser.java @@ -64,7 +64,7 @@ public static List parseJsonGraphToSequence(String json) { JsonObject graph = jsonObject.getAsJsonObject(WORKFLOW); List nodes = new ArrayList<>(); - List edges = new ArrayList<>(); + List edges = new ArrayList<>(); for (JsonElement nodeJson : graph.getAsJsonArray(NODES)) { JsonObject nodeObject = nodeJson.getAsJsonObject(); @@ -82,21 +82,21 @@ public static List parseJsonGraphToSequence(String json) { if (sourceNodeId.equals(destNodeId)) { throw new IllegalArgumentException("Edge connects node " + sourceNodeId + " to itself."); } - edges.add(new ProcessSequenceEdge(sourceNodeId, destNodeId)); + edges.add(new WorkflowEdge(sourceNodeId, destNodeId)); } return topologicalSort(nodes, edges); } - private static List topologicalSort(List nodes, List edges) { + private static List topologicalSort(List nodes, List edges) { // Define the graph - Set graph = new HashSet<>(edges); + Set graph = new HashSet<>(edges); // Map node id string to object Map nodeMap = nodes.stream().collect(Collectors.toMap(ProcessNode::id, Function.identity())); // Build predecessor and successor maps - Map> predecessorEdges = new HashMap<>(); - Map> successorEdges = new HashMap<>(); - for (ProcessSequenceEdge edge : edges) { + Map> predecessorEdges = new HashMap<>(); + Map> successorEdges = new HashMap<>(); + for (WorkflowEdge edge : edges) { ProcessNode source = nodeMap.get(edge.getSource()); ProcessNode dest = nodeMap.get(edge.getDestination()); predecessorEdges.computeIfAbsent(dest, k -> new HashSet<>()).add(edge); @@ -125,7 +125,7 @@ private static List topologicalSort(List nodes, List

inputs = new HashMap<>(); // maps to WorkflowData + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + XContentBuilder xContentBuilder = builder.startObject(); + xContentBuilder.field(ID_FIELD, this.id); + xContentBuilder.field(TYPE_FIELD, this.type); + + xContentBuilder.startObject(INPUTS_FIELD); + for (Entry e : inputs.entrySet()) { + xContentBuilder.field(e.getKey(), e.getValue()); + } + xContentBuilder.endObject(); + + return xContentBuilder.endObject(); + } +} diff --git a/src/main/java/org/opensearch/flowframework/workflow/Workflow.java b/src/main/java/org/opensearch/flowframework/workflow/Workflow.java new file mode 100644 index 000000000..6666ff10a --- /dev/null +++ b/src/main/java/org/opensearch/flowframework/workflow/Workflow.java @@ -0,0 +1,59 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ +package org.opensearch.flowframework.workflow; + +import org.opensearch.core.xcontent.ToXContentFragment; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.flowframework.template.WorkflowEdge; +import org.opensearch.flowframework.template.WorkflowNode; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; +import java.util.Map.Entry; + +/** + * This represents an object in the workflows section of a {@link Template}. + */ +public class Workflow implements ToXContentFragment { + + private static final String USER_PARAMS_FIELD = "user_params"; + private static final String NODES_FIELD = "nodes"; + private static final String EDGES_FIELD = "edges"; + + private Map userParams = new HashMap<>(); + private WorkflowNode[] nodes = null; + private WorkflowEdge[] edges = null; + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + XContentBuilder xContentBuilder = builder.startObject(); + + xContentBuilder.startObject(USER_PARAMS_FIELD); + for (Entry e : userParams.entrySet()) { + xContentBuilder.field(e.getKey(), e.getValue()); + } + xContentBuilder.endObject(); + + xContentBuilder.startArray(NODES_FIELD); + for (WorkflowNode n : nodes) { + xContentBuilder.value(n); + } + xContentBuilder.endArray(); + + xContentBuilder.startArray(EDGES_FIELD); + for (WorkflowEdge e : edges) { + xContentBuilder.value(e); + } + xContentBuilder.endArray(); + + return xContentBuilder.endObject(); + } + +} diff --git a/src/test/java/org/opensearch/flowframework/template/ProcessSequenceEdgeTests.java b/src/test/java/org/opensearch/flowframework/template/WorkflowEdgeTests.java similarity index 70% rename from src/test/java/org/opensearch/flowframework/template/ProcessSequenceEdgeTests.java rename to src/test/java/org/opensearch/flowframework/template/WorkflowEdgeTests.java index 80cecd96e..f44b54527 100644 --- a/src/test/java/org/opensearch/flowframework/template/ProcessSequenceEdgeTests.java +++ b/src/test/java/org/opensearch/flowframework/template/WorkflowEdgeTests.java @@ -10,7 +10,7 @@ import org.opensearch.test.OpenSearchTestCase; -public class ProcessSequenceEdgeTests extends OpenSearchTestCase { +public class WorkflowEdgeTests extends OpenSearchTestCase { @Override public void setUp() throws Exception { @@ -18,15 +18,15 @@ public void setUp() throws Exception { } public void testEdge() { - ProcessSequenceEdge edgeAB = new ProcessSequenceEdge("A", "B"); + WorkflowEdge edgeAB = new WorkflowEdge("A", "B"); assertEquals("A", edgeAB.getSource()); assertEquals("B", edgeAB.getDestination()); assertEquals("A->B", edgeAB.toString()); - ProcessSequenceEdge edgeAB2 = new ProcessSequenceEdge("A", "B"); + WorkflowEdge edgeAB2 = new WorkflowEdge("A", "B"); assertEquals(edgeAB, edgeAB2); - ProcessSequenceEdge edgeAC = new ProcessSequenceEdge("A", "C"); + WorkflowEdge edgeAC = new WorkflowEdge("A", "C"); assertNotEquals(edgeAB, edgeAC); } } diff --git a/src/test/resources/template/finaltemplate.json b/src/test/resources/template/finaltemplate.json new file mode 100644 index 000000000..be0e3da2c --- /dev/null +++ b/src/test/resources/template/finaltemplate.json @@ -0,0 +1,90 @@ +{ + "name": "semantic-search", + "description": "My semantic search use case", + "use_case": "SEMANTIC_SEARCH", + "operations": [ + "PROVISION", + "INGEST", + "QUERY" + ], + "version": { + "template": "1.0", + "compatibility": [ + "2.9", + "3.0" + ] + }, + "user_inputs": { + "index_name": "my-knn-index", + "index_settings": { + } + }, + "workflows": { + "provision": { + "steps": [{ + "id": "create_index", + "inputs": { + "name": "user_inputs.index_name", + "settings": "user_inputs.index_settings" + } + }, + { + "id": "create_ingest_pipeline", + "inputs": { + "name": "my-ingest-pipeline", + "description": "some description", + "processors": [{ + "type": "text_embedding", + "model_id": "my-existing-model-id", + "input_field": "text_passage", + "output_field": "text_embedding" + }] + } + } + ], + "edges": [{ + "source": "create_index", + "dest": "create_ingest_pipeline" + }] + }, + "ingest": { + "user_params": { + "document": {} + }, + "steps": [{ + "id": "ingest_index", + "inputs": { + "index": "user_inputs.index_name", + "ingest_pipeline": "my-ingest-pipeline", + "document": "user_params.document" + } + }] + }, + "query": { + "user_params": { + "plaintext": "string" + }, + "nodes": [{ + "id": "transform_query", + "inputs": { + "template": "neural-search-template-1", + "plaintext": "user_params.plaintext" + } + }, + { + "id": "query_index", + "inputs": { + "index": "user_inputs.index_name", + "query": "{{output-from-prev-step}}.query", + "search_request_processors": [], + "search_response_processors": [] + } + } + ], + "edges": [{ + "source": "transform_query", + "dest": "query_index" + }] + } + } +} From 677cc245f30edf3ccad8e59ea3bbc34e6e81a2c9 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Thu, 21 Sep 2023 10:25:56 -0700 Subject: [PATCH 03/27] Add parse methods for the Template XContent Signed-off-by: Daniel Widdis --- .../flowframework/template/Template.java | 197 +++++++++++++++--- .../flowframework/template/WorkflowEdge.java | 35 ++++ .../flowframework/template/WorkflowNode.java | 63 +++++- .../flowframework/workflow/Workflow.java | 67 +++++- .../flowframework/template/GraphJsonUtil.java | 16 ++ 5 files changed, 347 insertions(+), 31 deletions(-) create mode 100644 src/test/java/org/opensearch/flowframework/template/GraphJsonUtil.java diff --git a/src/main/java/org/opensearch/flowframework/template/Template.java b/src/main/java/org/opensearch/flowframework/template/Template.java index 25e25b691..205fa61b7 100644 --- a/src/main/java/org/opensearch/flowframework/template/Template.java +++ b/src/main/java/org/opensearch/flowframework/template/Template.java @@ -11,53 +11,111 @@ import org.opensearch.Version; import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.core.xcontent.XContentParser; import org.opensearch.flowframework.workflow.Workflow; import java.io.IOException; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Map.Entry; +import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; + /** * 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 { + private static final String WORKFLOWS_FIELD = "workflows"; + private static final String USER_INPUTS_FIELD = "user_inputs"; + private static final String COMPATIBILITY_FIELD = "compatibility"; + private static final String TEMPLATE_FIELD = "template"; + private static final String VERSION_FIELD = "version"; + private static final String OPERATIONS_FIELD = "operations"; + private static final String USE_CASE_FIELD = "use_case"; + private static final String DESCRIPTION_FIELD = "description"; + private static final String NAME_FIELD = "name"; // TODO: Some of thse are placeholders based on the template design - // Current code is only using user inputs and workflows - private String name = null; - private String description = null; - private String useCase = null; - private String[] operations = null; // probably an ENUM actually - private Version templateVersion = null; - private Version[] compatibilityVersion = null; - private Map userInputs = new HashMap<>(); - private Map workflows = new HashMap<>(); + // Change as needed to match defined template + private final String name; + private final String description; + private final String useCase; // probably an ENUM actually + private final String[] operations; // probably an ENUM actually + private final Version templateVersion; + private final Version[] compatibilityVersion; + private final Map userInputs; + private final Map workflows; + + /** + * Instantiate the object representing a use case template + * + * @param name The template's name + * @param description A description of the template's use case + * @param useCase A string defining the internal use case type + * @param operations Expected operations of this template. Should match defined workflows. + * @param templateVersion The version of this template + * @param compatibilityVersion OpenSearch version compatibility of this template + * @param userInputs Optional user inputs to apply globally + * @param workflows Workflow graph definitions corresponding to the defined operations. + */ + public Template( + String name, + String description, + String useCase, + String[] operations, + Version templateVersion, + Version[] compatibilityVersion, + Map userInputs, + Map workflows + ) { + this.name = name; + this.description = description; + this.useCase = useCase; + this.operations = operations; + this.templateVersion = templateVersion; + this.compatibilityVersion = compatibilityVersion; + this.userInputs = userInputs; + this.workflows = workflows; + } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { XContentBuilder xContentBuilder = builder.startObject(); - xContentBuilder.field("name", this.name); - xContentBuilder.field("description", this.description); - xContentBuilder.field("use_case", this.useCase); - xContentBuilder.field("operations", this.operations); - - xContentBuilder.startObject("version"); - xContentBuilder.field("template", this.templateVersion); - xContentBuilder.startArray("compatibility"); - for (Version v : this.compatibilityVersion) { - xContentBuilder.value(v); + xContentBuilder.field(NAME_FIELD, this.name); + xContentBuilder.field(DESCRIPTION_FIELD, this.description); + xContentBuilder.field(USE_CASE_FIELD, this.useCase); + xContentBuilder.startArray(OPERATIONS_FIELD); + for (String op : this.operations) { + xContentBuilder.value(op); } xContentBuilder.endArray(); - xContentBuilder.endObject(); - xContentBuilder.startObject("user_inputs"); - for (Entry e : userInputs.entrySet()) { - xContentBuilder.field(e.getKey(), e.getValue()); + if (this.templateVersion != null || this.compatibilityVersion.length > 0) { + xContentBuilder.startObject(VERSION_FIELD); + if (this.templateVersion != null) { + xContentBuilder.field(TEMPLATE_FIELD, this.templateVersion); + } + if (this.compatibilityVersion.length > 0) { + xContentBuilder.startArray(COMPATIBILITY_FIELD); + for (Version v : this.compatibilityVersion) { + xContentBuilder.value(v); + } + xContentBuilder.endArray(); + } + xContentBuilder.endObject(); + } + + if (!this.userInputs.isEmpty()) { + xContentBuilder.startObject(USER_INPUTS_FIELD); + for (Entry e : userInputs.entrySet()) { + xContentBuilder.field(e.getKey(), e.getValue()); + } + xContentBuilder.endObject(); } - xContentBuilder.endObject(); - xContentBuilder.startObject("workflows"); + xContentBuilder.startObject(WORKFLOWS_FIELD); for (Entry e : workflows.entrySet()) { xContentBuilder.field(e.getKey(), e.getValue(), params); } @@ -66,4 +124,93 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return xContentBuilder.endObject(); } + /** + * Parse raw json content into a workflow node instance. + * + * @param parser json based content parser + * @throws IOException if content can't be parsed correctly + */ + public static Template parse(XContentParser parser) throws IOException { + String name = null; + String description = ""; + String useCase = ""; + String[] operations = new String[0]; + Version templateVersion = null; + Version[] compatibilityVersion = new Version[0]; + Map userInputs = new HashMap<>(); + Map workflows = new HashMap<>(); + + ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); + while (parser.nextToken() != XContentParser.Token.END_OBJECT) { + String fieldName = parser.currentName(); + parser.nextToken(); + switch (fieldName) { + case NAME_FIELD: + name = parser.text(); + break; + case DESCRIPTION_FIELD: + description = parser.text(); + break; + case USE_CASE_FIELD: + useCase = parser.text(); + break; + case OPERATIONS_FIELD: + ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.currentToken(), parser); + List operationsList = new ArrayList<>(); + while (parser.nextToken() != XContentParser.Token.END_ARRAY) { + operationsList.add(parser.text()); + } + operations = operationsList.toArray(new String[0]); + break; + case VERSION_FIELD: + ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); + while (parser.nextToken() != XContentParser.Token.END_OBJECT) { + String versionFieldName = parser.currentName(); + parser.nextToken(); + switch (versionFieldName) { + case TEMPLATE_FIELD: + templateVersion = Version.fromString(parser.text()); + break; + case COMPATIBILITY_FIELD: + ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.currentToken(), parser); + List compatibilityList = new ArrayList<>(); + while (parser.nextToken() != XContentParser.Token.END_ARRAY) { + compatibilityList.add(Version.fromString(parser.text())); + } + compatibilityVersion = compatibilityList.toArray(new Version[0]); + break; + default: + throw new IOException("Unable to parse field [" + fieldName + "] in a version object."); + } + + } + break; + case USER_INPUTS_FIELD: + ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); + while (parser.nextToken() != XContentParser.Token.END_OBJECT) { + String inputFieldName = parser.currentName(); + parser.nextToken(); + userInputs.put(inputFieldName, parser.text()); + } + break; + case WORKFLOWS_FIELD: + ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); + while (parser.nextToken() != XContentParser.Token.END_OBJECT) { + String workflowFieldName = parser.currentName(); + parser.nextToken(); + workflows.put(workflowFieldName, Workflow.parse(parser)); + } + break; + + default: + throw new IOException("Unable to parse field [" + fieldName + "] in a template object."); + } + } + if (name == null) { + throw new IOException("An template object requires a name."); + } + + return new Template(name, description, useCase, operations, templateVersion, compatibilityVersion, userInputs, workflows); + } + } diff --git a/src/main/java/org/opensearch/flowframework/template/WorkflowEdge.java b/src/main/java/org/opensearch/flowframework/template/WorkflowEdge.java index 1a7b202b7..247000c12 100644 --- a/src/main/java/org/opensearch/flowframework/template/WorkflowEdge.java +++ b/src/main/java/org/opensearch/flowframework/template/WorkflowEdge.java @@ -10,10 +10,13 @@ import org.opensearch.core.xcontent.ToXContentFragment; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.core.xcontent.XContentParser; import java.io.IOException; import java.util.Objects; +import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; + /** * This represents an edge between process nodes (steps) in a workflow graph in the {@link Template}. */ @@ -43,6 +46,38 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return xContentBuilder.endObject(); } + /** + * Parse raw json content into a workflow edge instance. + * + * @param parser json based content parser + * @throws IOException if content can't be parsed correctly + */ + public static WorkflowEdge parse(XContentParser parser) throws IOException { + String source = null; + String destination = null; + + ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); + while (parser.nextToken() != XContentParser.Token.END_OBJECT) { + String fieldName = parser.currentName(); + parser.nextToken(); + switch (fieldName) { + case SOURCE_FIELD: + source = parser.text(); + break; + case DEST_FIELD: + destination = parser.text(); + break; + default: + throw new IOException("Unable to parse field [" + fieldName + "] in an edge object."); + } + } + if (source == null || destination == null) { + throw new IOException("An edge object requires both a source and dest field."); + } + + return new WorkflowEdge(source, destination); + } + /** * Gets the source node id. * diff --git a/src/main/java/org/opensearch/flowframework/template/WorkflowNode.java b/src/main/java/org/opensearch/flowframework/template/WorkflowNode.java index 128cf1164..28d368cfb 100644 --- a/src/main/java/org/opensearch/flowframework/template/WorkflowNode.java +++ b/src/main/java/org/opensearch/flowframework/template/WorkflowNode.java @@ -10,12 +10,15 @@ import org.opensearch.core.xcontent.ToXContentFragment; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.core.xcontent.XContentParser; import java.io.IOException; import java.util.HashMap; import java.util.Map; import java.util.Map.Entry; +import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; + /** * This represents a process node (step) in a workflow graph in the {@link Template}. * It will have a one-to-one correspondence with a {@link ProcessNode}, @@ -28,9 +31,22 @@ public class WorkflowNode implements ToXContentFragment { private static final String TYPE_FIELD = "type"; private static final String ID_FIELD = "id"; - private String id = null; // unique id - private String type = null; // maps to a WorkflowStep - private Map inputs = new HashMap<>(); // maps to WorkflowData + private final String id; // unique id + private final String type; // maps to a WorkflowStep + private final Map inputs; // maps to WorkflowData + + /** + * Create this node with the id and type, and any user input. + * + * @param id A unique string identifying this node + * @param type The type of {@link WorkflowStep} to create for the corresponding {@link ProcessNode} + * @param inputs Optional input to populate params in {@link WorkflowData} + */ + public WorkflowNode(String id, String type, Map inputs) { + this.id = id; + this.type = type; + this.inputs = inputs; + } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { @@ -46,4 +62,45 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return xContentBuilder.endObject(); } + + /** + * Parse raw json content into a workflow node instance. + * + * @param parser json based content parser + * @throws IOException if content can't be parsed correctly + */ + public static WorkflowNode parse(XContentParser parser) throws IOException { + String id = null; + String type = null; + Map inputs = new HashMap<>(); + + ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); + while (parser.nextToken() != XContentParser.Token.END_OBJECT) { + String fieldName = parser.currentName(); + parser.nextToken(); + switch (fieldName) { + case ID_FIELD: + id = parser.text(); + break; + case TYPE_FIELD: + type = parser.text(); + break; + case INPUTS_FIELD: + ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); + while (parser.nextToken() != XContentParser.Token.END_OBJECT) { + String inputFieldName = parser.currentName(); + parser.nextToken(); + inputs.put(inputFieldName, parser.text()); + } + break; + default: + throw new IOException("Unable to parse field [" + fieldName + "] in a node object."); + } + } + if (id == null || type == null) { + throw new IOException("An node object requires both an id and type field."); + } + + return new WorkflowNode(id, type, inputs); + } } diff --git a/src/main/java/org/opensearch/flowframework/workflow/Workflow.java b/src/main/java/org/opensearch/flowframework/workflow/Workflow.java index 6666ff10a..b3cfcca40 100644 --- a/src/main/java/org/opensearch/flowframework/workflow/Workflow.java +++ b/src/main/java/org/opensearch/flowframework/workflow/Workflow.java @@ -10,14 +10,19 @@ import org.opensearch.core.xcontent.ToXContentFragment; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.core.xcontent.XContentParser; import org.opensearch.flowframework.template.WorkflowEdge; import org.opensearch.flowframework.template.WorkflowNode; import java.io.IOException; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Map.Entry; +import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; + /** * This represents an object in the workflows section of a {@link Template}. */ @@ -25,11 +30,18 @@ public class Workflow implements ToXContentFragment { private static final String USER_PARAMS_FIELD = "user_params"; private static final String NODES_FIELD = "nodes"; + // TODO: private static final String STEPS_FIELD = "steps"; private static final String EDGES_FIELD = "edges"; - private Map userParams = new HashMap<>(); - private WorkflowNode[] nodes = null; - private WorkflowEdge[] edges = null; + private final Map userParams; + private final WorkflowNode[] nodes; + private final WorkflowEdge[] edges; + + public Workflow(Map userParams, WorkflowNode[] nodes, WorkflowEdge[] edges) { + this.userParams = userParams; + this.nodes = nodes; + this.edges = edges; + } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { @@ -56,4 +68,53 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return xContentBuilder.endObject(); } + /** + * Parse raw json content into a workflow instance. + * + * @param parser json based content parser + * @throws IOException if content can't be parsed correctly + */ + public static Workflow parse(XContentParser parser) throws IOException { + Map userParams = new HashMap<>(); + WorkflowNode[] nodes = null; + WorkflowEdge[] edges = null; + + ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); + while (parser.nextToken() != XContentParser.Token.END_OBJECT) { + String fieldName = parser.currentName(); + parser.nextToken(); + switch (fieldName) { + case USER_PARAMS_FIELD: + ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); + while (parser.nextToken() != XContentParser.Token.END_OBJECT) { + String userParamFieldName = parser.currentName(); + parser.nextToken(); + userParams.put(userParamFieldName, parser.text()); + } + break; + case NODES_FIELD: + ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.currentToken(), parser); + List nodesList = new ArrayList<>(); + while (parser.nextToken() != XContentParser.Token.END_ARRAY) { + nodesList.add(WorkflowNode.parse(parser)); + } + nodes = nodesList.toArray(new WorkflowNode[0]); + break; + case EDGES_FIELD: + ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.currentToken(), parser); + List edgesList = new ArrayList<>(); + while (parser.nextToken() != XContentParser.Token.END_ARRAY) { + edgesList.add(WorkflowEdge.parse(parser)); + } + edges = edgesList.toArray(new WorkflowEdge[0]); + break; + } + + } + if (nodes == null || nodes.length == 0) { + throw new IOException("A workflow must have at least one node."); + } + // TODO: if edges are empty, create edges by iterating over nodes and adding one between each pair + return new Workflow(userParams, nodes, edges); + } } diff --git a/src/test/java/org/opensearch/flowframework/template/GraphJsonUtil.java b/src/test/java/org/opensearch/flowframework/template/GraphJsonUtil.java new file mode 100644 index 000000000..6945691a0 --- /dev/null +++ b/src/test/java/org/opensearch/flowframework/template/GraphJsonUtil.java @@ -0,0 +1,16 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ +package org.opensearch.flowframework.template; + +/** + * Utility methods to create a JSON string useful for testing nodes and edges + */ +public class GraphJsonUtil { + +} From 57fa740af0dc92f2cb3e00e4bf39020357fc2147 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Thu, 21 Sep 2023 15:53:29 -0700 Subject: [PATCH 04/27] Cleanup parsing, javadocs, and demo output Signed-off-by: Daniel Widdis --- src/main/java/demo/TemplateParseDemo.java | 63 ++++++++ .../flowframework/template/Template.java | 138 +++++++++++++++--- .../flowframework/template/WorkflowEdge.java | 12 +- .../flowframework/template/WorkflowNode.java | 99 +++++++++++-- .../flowframework/workflow/Workflow.java | 42 +++++- .../resources/template/finaltemplate.json | 16 +- 6 files changed, 323 insertions(+), 47 deletions(-) create mode 100644 src/main/java/demo/TemplateParseDemo.java diff --git a/src/main/java/demo/TemplateParseDemo.java b/src/main/java/demo/TemplateParseDemo.java new file mode 100644 index 000000000..0e92ba8ba --- /dev/null +++ b/src/main/java/demo/TemplateParseDemo.java @@ -0,0 +1,63 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ +package demo; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.common.SuppressForbidden; +import org.opensearch.common.io.PathUtils; +import org.opensearch.common.xcontent.LoggingDeprecationHandler; +import org.opensearch.common.xcontent.json.JsonXContent; +import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.flowframework.template.Template; +import org.opensearch.flowframework.template.TemplateParser; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; + +import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; + +/** + * Demo class exercising {@link TemplateParser}. This will be moved to a unit test. + */ +public class TemplateParseDemo { + + private static final Logger logger = LogManager.getLogger(TemplateParseDemo.class); + + /** + * Demonstrate parsing a JSON graph. + * + * @param args unused + * @throws IOException on error. + */ + @SuppressForbidden(reason = "just a demo class that will be deleted") + public static void main(String[] args) throws IOException { + String path = "src/test/resources/template/finaltemplate.json"; + String json; + try { + json = new String(Files.readAllBytes(PathUtils.get(path)), StandardCharsets.UTF_8); + } catch (IOException e) { + logger.error("Failed to read JSON at path {}", path); + return; + } + + logger.info("Parsing template..."); + XContentParser parser = JsonXContent.jsonXContent.createParser( + NamedXContentRegistry.EMPTY, + LoggingDeprecationHandler.INSTANCE, + json + ); + ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser); + Template t = Template.parse(parser); + System.out.println(t.toJson()); + System.out.println(t.toYaml()); + } +} diff --git a/src/main/java/org/opensearch/flowframework/template/Template.java b/src/main/java/org/opensearch/flowframework/template/Template.java index 205fa61b7..2a629dffb 100644 --- a/src/main/java/org/opensearch/flowframework/template/Template.java +++ b/src/main/java/org/opensearch/flowframework/template/Template.java @@ -9,6 +9,8 @@ package org.opensearch.flowframework.template; import org.opensearch.Version; +import org.opensearch.common.xcontent.json.JsonXContent; +import org.opensearch.common.xcontent.yaml.YamlXContent; import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; @@ -16,6 +18,7 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -28,24 +31,32 @@ */ public class Template implements ToXContentObject { - private static final String WORKFLOWS_FIELD = "workflows"; - private static final String USER_INPUTS_FIELD = "user_inputs"; - private static final String COMPATIBILITY_FIELD = "compatibility"; - private static final String TEMPLATE_FIELD = "template"; - private static final String VERSION_FIELD = "version"; - private static final String OPERATIONS_FIELD = "operations"; - private static final String USE_CASE_FIELD = "use_case"; - private static final String DESCRIPTION_FIELD = "description"; - private static final String NAME_FIELD = "name"; - // TODO: Some of thse are placeholders based on the template design - // Change as needed to match defined template + /** The template field name for template name */ + public static final String NAME_FIELD = "name"; + /** The template field name for template description */ + public static final String DESCRIPTION_FIELD = "description"; + /** The template field name for template use case */ + public static final String USE_CASE_FIELD = "use_case"; + /** The template field name for template operations */ + public static final String OPERATIONS_FIELD = "operations"; + /** The template field name for template version information */ + public static final String VERSION_FIELD = "version"; + /** 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 user inputs */ + public static final String USER_INPUTS_FIELD = "user_inputs"; + /** The template field name for template workflows */ + public static final String WORKFLOWS_FIELD = "workflows"; + private final String name; private final String description; private final String useCase; // probably an ENUM actually private final String[] operations; // probably an ENUM actually private final Version templateVersion; private final Version[] compatibilityVersion; - private final Map userInputs; + private final Map userInputs; private final Map workflows; /** @@ -67,7 +78,7 @@ public Template( String[] operations, Version templateVersion, Version[] compatibilityVersion, - Map userInputs, + Map userInputs, Map workflows ) { this.name = name; @@ -109,7 +120,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws if (!this.userInputs.isEmpty()) { xContentBuilder.startObject(USER_INPUTS_FIELD); - for (Entry e : userInputs.entrySet()) { + for (Entry e : userInputs.entrySet()) { xContentBuilder.field(e.getKey(), e.getValue()); } xContentBuilder.endObject(); @@ -128,6 +139,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws * Parse raw json content into a workflow node instance. * * @param parser json based content parser + * @return an instance of the template * @throws IOException if content can't be parsed correctly */ public static Template parse(XContentParser parser) throws IOException { @@ -137,7 +149,7 @@ public static Template parse(XContentParser parser) throws IOException { String[] operations = new String[0]; Version templateVersion = null; Version[] compatibilityVersion = new Version[0]; - Map userInputs = new HashMap<>(); + Map userInputs = new HashMap<>(); Map workflows = new HashMap<>(); ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); @@ -182,15 +194,22 @@ public static Template parse(XContentParser parser) throws IOException { default: throw new IOException("Unable to parse field [" + fieldName + "] in a version object."); } - } break; case USER_INPUTS_FIELD: ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); while (parser.nextToken() != XContentParser.Token.END_OBJECT) { String inputFieldName = parser.currentName(); - parser.nextToken(); - userInputs.put(inputFieldName, parser.text()); + switch (parser.nextToken()) { + case VALUE_STRING: + userInputs.put(inputFieldName, parser.text()); + break; + case START_OBJECT: + userInputs.put(inputFieldName, parseStringToStringMap(parser)); + break; + default: + throw new IOException("Unable to parse field [" + inputFieldName + "] in a user inputs object."); + } } break; case WORKFLOWS_FIELD: @@ -213,4 +232,87 @@ public static Template parse(XContentParser parser) throws IOException { return new Template(name, description, useCase, operations, templateVersion, compatibilityVersion, userInputs, workflows); } + /** + * Builds an XContent object representing a map of String keys to String values. + * + * @param xContentBuilder An XContent builder whose position is at the start of the map object to build + * @param map A map as key-value String pairs. + * @throws IOException on a build failure + */ + public static void buildStringToStringMap(XContentBuilder xContentBuilder, Map map) throws IOException { + xContentBuilder.startObject(); + for (Entry e : map.entrySet()) { + xContentBuilder.field((String) e.getKey(), (String) e.getValue()); + } + xContentBuilder.endObject(); + } + + /** + * Parses an XContent object representing a map of String keys to String values. + * + * @param parser An XContent parser whose position is at the start of the map object to parse + * @return A map as identified by the key-value pairs in the XContent + * @throws IOException on a parse failure + */ + public static Map parseStringToStringMap(XContentParser parser) throws IOException { + ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); + Map map = new HashMap<>(); + while (parser.nextToken() != XContentParser.Token.END_OBJECT) { + String fieldName = parser.currentName(); + parser.nextToken(); + map.put(fieldName, parser.text()); + } + return map; + } + + /** + * Output this object in a compact JSON string. + * + * @return a JSON representation of the template. + */ + public String toJson() { + try { + XContentBuilder builder = JsonXContent.contentBuilder(); + return this.toXContent(builder, EMPTY_PARAMS).toString(); + } catch (IOException e) { + e.printStackTrace(); + return "{\"error\": \"couldn't create JSON\"}"; + } + } + + /** + * Output this object in YAML. + * + * @return a YAML representation of the template. + */ + public String toYaml() { + try { + XContentBuilder builder = YamlXContent.contentBuilder(); + return this.toXContent(builder, EMPTY_PARAMS).toString(); + } catch (IOException e) { + e.printStackTrace(); + return "error: couldn't create YAML"; + } + } + + @Override + public String toString() { + return "Template [name=" + + name + + ", description=" + + description + + ", useCase=" + + useCase + + ", operations=" + + Arrays.toString(operations) + + ", templateVersion=" + + templateVersion + + ", compatibilityVersion=" + + Arrays.toString(compatibilityVersion) + + ", userInputs=" + + userInputs + + ", workflows=" + + workflows + + "]"; + } } diff --git a/src/main/java/org/opensearch/flowframework/template/WorkflowEdge.java b/src/main/java/org/opensearch/flowframework/template/WorkflowEdge.java index 247000c12..2d0525b84 100644 --- a/src/main/java/org/opensearch/flowframework/template/WorkflowEdge.java +++ b/src/main/java/org/opensearch/flowframework/template/WorkflowEdge.java @@ -8,7 +8,7 @@ */ package org.opensearch.flowframework.template; -import org.opensearch.core.xcontent.ToXContentFragment; +import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; @@ -20,9 +20,12 @@ /** * This represents an edge between process nodes (steps) in a workflow graph in the {@link Template}. */ -public class WorkflowEdge implements ToXContentFragment { - public static final String DEST_FIELD = "dest"; +public class WorkflowEdge implements ToXContentObject { + + /** The template field name for source node */ public static final String SOURCE_FIELD = "source"; + /** The template field name for destination node */ + public static final String DEST_FIELD = "dest"; private final String source; private final String destination; @@ -33,7 +36,7 @@ public class WorkflowEdge implements ToXContentFragment { * @param source The source node id. * @param destination The destination node id. */ - WorkflowEdge(String source, String destination) { + public WorkflowEdge(String source, String destination) { this.source = source; this.destination = destination; } @@ -50,6 +53,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws * Parse raw json content into a workflow edge instance. * * @param parser json based content parser + * @return the parsed WorkflowEdge instance * @throws IOException if content can't be parsed correctly */ public static WorkflowEdge parse(XContentParser parser) throws IOException { diff --git a/src/main/java/org/opensearch/flowframework/template/WorkflowNode.java b/src/main/java/org/opensearch/flowframework/template/WorkflowNode.java index 28d368cfb..0d7253a5d 100644 --- a/src/main/java/org/opensearch/flowframework/template/WorkflowNode.java +++ b/src/main/java/org/opensearch/flowframework/template/WorkflowNode.java @@ -8,14 +8,19 @@ */ package org.opensearch.flowframework.template; -import org.opensearch.core.xcontent.ToXContentFragment; +import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.flowframework.workflow.WorkflowData; +import org.opensearch.flowframework.workflow.WorkflowStep; import java.io.IOException; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.Objects; import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; @@ -25,15 +30,18 @@ * where its type is used to determine the correct {@link WorkflowStep} object, * and its inputs are used to populate the {@link WorkflowData} input. */ -public class WorkflowNode implements ToXContentFragment { +public class WorkflowNode implements ToXContentObject { - private static final String INPUTS_FIELD = "inputs"; - private static final String TYPE_FIELD = "type"; - private static final String ID_FIELD = "id"; + /** The template field name for node id */ + public static final String ID_FIELD = "id"; + /** The template field name for node type */ + public static final String TYPE_FIELD = "type"; + /** The template field name for node inputs */ + public static final String INPUTS_FIELD = "inputs"; private final String id; // unique id private final String type; // maps to a WorkflowStep - private final Map inputs; // maps to WorkflowData + private final Map inputs; // maps to WorkflowData /** * Create this node with the id and type, and any user input. @@ -42,7 +50,7 @@ public class WorkflowNode implements ToXContentFragment { * @param type The type of {@link WorkflowStep} to create for the corresponding {@link ProcessNode} * @param inputs Optional input to populate params in {@link WorkflowData} */ - public WorkflowNode(String id, String type, Map inputs) { + public WorkflowNode(String id, String type, Map inputs) { this.id = id; this.type = type; this.inputs = inputs; @@ -55,8 +63,19 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws xContentBuilder.field(TYPE_FIELD, this.type); xContentBuilder.startObject(INPUTS_FIELD); - for (Entry e : inputs.entrySet()) { - xContentBuilder.field(e.getKey(), e.getValue()); + for (Entry e : inputs.entrySet()) { + xContentBuilder.field(e.getKey()); + if (e.getValue() instanceof String) { + xContentBuilder.value(e.getValue()); + } else if (e.getValue() instanceof Map) { + Template.buildStringToStringMap(xContentBuilder, (Map) e.getValue()); + } else if (e.getValue() instanceof Object[]) { + xContentBuilder.startArray(); + for (Map map : (Map[]) e.getValue()) { + Template.buildStringToStringMap(xContentBuilder, map); + } + xContentBuilder.endArray(); + } } xContentBuilder.endObject(); @@ -67,12 +86,13 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws * Parse raw json content into a workflow node instance. * * @param parser json based content parser + * @return the parsed WorkflowNode instance * @throws IOException if content can't be parsed correctly */ public static WorkflowNode parse(XContentParser parser) throws IOException { String id = null; String type = null; - Map inputs = new HashMap<>(); + Map inputs = new HashMap<>(); ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); while (parser.nextToken() != XContentParser.Token.END_OBJECT) { @@ -89,8 +109,20 @@ public static WorkflowNode parse(XContentParser parser) throws IOException { ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); while (parser.nextToken() != XContentParser.Token.END_OBJECT) { String inputFieldName = parser.currentName(); - parser.nextToken(); - inputs.put(inputFieldName, parser.text()); + switch (parser.nextToken()) { + case VALUE_STRING: + inputs.put(inputFieldName, parser.text()); + break; + case START_ARRAY: + List> mapList = new ArrayList<>(); + while (parser.nextToken() != XContentParser.Token.END_ARRAY) { + mapList.add(Template.parseStringToStringMap(parser)); + } + inputs.put(inputFieldName, mapList.toArray(new Map[0])); + break; + default: + throw new IOException("Unable to parse field [" + inputFieldName + "] in a node object."); + } } break; default: @@ -103,4 +135,47 @@ public static WorkflowNode parse(XContentParser parser) throws IOException { return new WorkflowNode(id, type, inputs); } + + /** + * Return this node's id + * @return the id + */ + public String getId() { + return id; + } + + /** + * Return this node's type + * @return the type + */ + public String getType() { + return type; + } + + /** + * Return this node's input data + * @return the inputs + */ + public Map getInputs() { + return inputs; + } + + @Override + public int hashCode() { + return Objects.hash(id); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null) return false; + if (getClass() != obj.getClass()) return false; + WorkflowNode other = (WorkflowNode) obj; + return Objects.equals(id, other.id); + } + + @Override + public String toString() { + return this.id; + } } diff --git a/src/main/java/org/opensearch/flowframework/workflow/Workflow.java b/src/main/java/org/opensearch/flowframework/workflow/Workflow.java index b3cfcca40..97d38b16d 100644 --- a/src/main/java/org/opensearch/flowframework/workflow/Workflow.java +++ b/src/main/java/org/opensearch/flowframework/workflow/Workflow.java @@ -8,14 +8,16 @@ */ package org.opensearch.flowframework.workflow; -import org.opensearch.core.xcontent.ToXContentFragment; +import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.flowframework.template.Template; import org.opensearch.flowframework.template.WorkflowEdge; import org.opensearch.flowframework.template.WorkflowNode; import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -26,17 +28,28 @@ /** * This represents an object in the workflows section of a {@link Template}. */ -public class Workflow implements ToXContentFragment { +public class Workflow implements ToXContentObject { - private static final String USER_PARAMS_FIELD = "user_params"; - private static final String NODES_FIELD = "nodes"; - // TODO: private static final String STEPS_FIELD = "steps"; - private static final String EDGES_FIELD = "edges"; + /** The template field name for workflow user params */ + public static final String USER_PARAMS_FIELD = "user_params"; + /** The template field name for workflow nodes */ + public static final String NODES_FIELD = "nodes"; + /** The template field name for workflow steps, an alternative name for nodes */ + public static final String STEPS_FIELD = "steps"; + /** The template field name for workflow edges */ + public static final String EDGES_FIELD = "edges"; private final Map userParams; private final WorkflowNode[] nodes; private final WorkflowEdge[] edges; + /** + * Create this workflow with any user params and the graph of nodes and edges. + * + * @param userParams A map of user params. + * @param nodes An array of {@link WorkflowNode} objects + * @param edges An array of {@link WorkflowEdge} objects. + */ public Workflow(Map userParams, WorkflowNode[] nodes, WorkflowEdge[] edges) { this.userParams = userParams; this.nodes = nodes; @@ -72,6 +85,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws * Parse raw json content into a workflow instance. * * @param parser json based content parser + * @return the parsed Workflow instance * @throws IOException if content can't be parsed correctly */ public static Workflow parse(XContentParser parser) throws IOException { @@ -93,6 +107,7 @@ public static Workflow parse(XContentParser parser) throws IOException { } break; case NODES_FIELD: + case STEPS_FIELD: ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.currentToken(), parser); List nodesList = new ArrayList<>(); while (parser.nextToken() != XContentParser.Token.END_ARRAY) { @@ -114,7 +129,20 @@ public static Workflow parse(XContentParser parser) throws IOException { if (nodes == null || nodes.length == 0) { throw new IOException("A workflow must have at least one node."); } - // TODO: if edges are empty, create edges by iterating over nodes and adding one between each pair + if (edges == null || edges.length == 0) { + // infer edges from sequence of nodes + List edgesList = new ArrayList<>(); + // Start iteration at 1, will skip for a one-node array + for (int i = 1; i < nodes.length; i++) { + edgesList.add(new WorkflowEdge(nodes[i - 1].getId(), nodes[i].getId())); + } + edges = edgesList.toArray(new WorkflowEdge[0]); + } return new Workflow(userParams, nodes, edges); } + + @Override + public String toString() { + return "Workflow [userParams=" + userParams + ", nodes=" + Arrays.toString(nodes) + ", edges=" + Arrays.toString(edges) + "]"; + } } diff --git a/src/test/resources/template/finaltemplate.json b/src/test/resources/template/finaltemplate.json index be0e3da2c..82affd97f 100644 --- a/src/test/resources/template/finaltemplate.json +++ b/src/test/resources/template/finaltemplate.json @@ -8,21 +8,21 @@ "QUERY" ], "version": { - "template": "1.0", + "template": "1.0.0", "compatibility": [ - "2.9", - "3.0" + "2.9.0", + "3.0.0" ] }, "user_inputs": { "index_name": "my-knn-index", - "index_settings": { - } + "index_settings": {} }, "workflows": { "provision": { "steps": [{ "id": "create_index", + "type": "create_index", "inputs": { "name": "user_inputs.index_name", "settings": "user_inputs.index_settings" @@ -30,6 +30,7 @@ }, { "id": "create_ingest_pipeline", + "type": "create_ingest_pipeline", "inputs": { "name": "my-ingest-pipeline", "description": "some description", @@ -49,10 +50,11 @@ }, "ingest": { "user_params": { - "document": {} + "document": "doc" }, "steps": [{ "id": "ingest_index", + "type": "ingest_index", "inputs": { "index": "user_inputs.index_name", "ingest_pipeline": "my-ingest-pipeline", @@ -66,6 +68,7 @@ }, "nodes": [{ "id": "transform_query", + "type": "transform_query", "inputs": { "template": "neural-search-template-1", "plaintext": "user_params.plaintext" @@ -73,6 +76,7 @@ }, { "id": "query_index", + "type": "query_index", "inputs": { "index": "user_inputs.index_name", "query": "{{output-from-prev-step}}.query", From 1214ebcfb86f5dfd71e20e25ef64755bcb1e8c43 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Thu, 21 Sep 2023 16:22:23 -0700 Subject: [PATCH 05/27] Refactor to use field name constants, get tests working again Signed-off-by: Daniel Widdis --- .../flowframework/template/Template.java | 6 ++--- .../template/TemplateParser.java | 26 ++++++------------ .../flowframework/template/GraphJsonUtil.java | 21 +++++++++++++++ .../template/TemplateParserTests.java | 27 +++---------------- 4 files changed, 34 insertions(+), 46 deletions(-) diff --git a/src/main/java/org/opensearch/flowframework/template/Template.java b/src/main/java/org/opensearch/flowframework/template/Template.java index 2a629dffb..48a1630a2 100644 --- a/src/main/java/org/opensearch/flowframework/template/Template.java +++ b/src/main/java/org/opensearch/flowframework/template/Template.java @@ -275,8 +275,7 @@ public String toJson() { XContentBuilder builder = JsonXContent.contentBuilder(); return this.toXContent(builder, EMPTY_PARAMS).toString(); } catch (IOException e) { - e.printStackTrace(); - return "{\"error\": \"couldn't create JSON\"}"; + return "{\"error\": \"couldn't create JSON: " + e.getMessage() + "\"}"; } } @@ -290,8 +289,7 @@ public String toYaml() { XContentBuilder builder = YamlXContent.contentBuilder(); return this.toXContent(builder, EMPTY_PARAMS).toString(); } catch (IOException e) { - e.printStackTrace(); - return "error: couldn't create YAML"; + return "error: couldn't create YAML: " + e.getMessage(); } } diff --git a/src/main/java/org/opensearch/flowframework/template/TemplateParser.java b/src/main/java/org/opensearch/flowframework/template/TemplateParser.java index 884f18174..315726acf 100644 --- a/src/main/java/org/opensearch/flowframework/template/TemplateParser.java +++ b/src/main/java/org/opensearch/flowframework/template/TemplateParser.java @@ -13,6 +13,7 @@ import com.google.gson.JsonObject; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.opensearch.flowframework.workflow.Workflow; import org.opensearch.flowframework.workflow.WorkflowData; import org.opensearch.flowframework.workflow.WorkflowStep; import org.opensearch.flowframework.workflow.WorkflowStepFactory; @@ -36,17 +37,6 @@ public class TemplateParser { private static final Logger logger = LogManager.getLogger(TemplateParser.class); - // Field names in the JSON. - // Currently package private for tests. - // These may eventually become part of the template definition in which case they might be better declared public - static final String WORKFLOW = "sequence"; - static final String NODES = "nodes"; - static final String NODE_ID = "id"; - static final String EDGES = "edges"; - static final String SOURCE = "source"; - static final String DESTINATION = "dest"; - static final String STEP_TYPE = "step_type"; - /** * Prevent instantiating this class. */ @@ -61,24 +51,24 @@ public static List parseJsonGraphToSequence(String json) { Gson gson = new Gson(); JsonObject jsonObject = gson.fromJson(json, JsonObject.class); - JsonObject graph = jsonObject.getAsJsonObject(WORKFLOW); + JsonObject graph = jsonObject.getAsJsonObject("workflow"); List nodes = new ArrayList<>(); List edges = new ArrayList<>(); - for (JsonElement nodeJson : graph.getAsJsonArray(NODES)) { + for (JsonElement nodeJson : graph.getAsJsonArray(Workflow.NODES_FIELD)) { JsonObject nodeObject = nodeJson.getAsJsonObject(); - String nodeId = nodeObject.get(NODE_ID).getAsString(); - String stepType = nodeObject.get(STEP_TYPE).getAsString(); + String nodeId = nodeObject.get(WorkflowNode.ID_FIELD).getAsString(); + String stepType = nodeObject.get(WorkflowNode.TYPE_FIELD).getAsString(); WorkflowStep workflowStep = WorkflowStepFactory.get().createStep(stepType); WorkflowData inputData = WorkflowData.EMPTY; nodes.add(new ProcessNode(nodeId, workflowStep, inputData)); } - for (JsonElement edgeJson : graph.getAsJsonArray(EDGES)) { + for (JsonElement edgeJson : graph.getAsJsonArray(Workflow.EDGES_FIELD)) { JsonObject edgeObject = edgeJson.getAsJsonObject(); - String sourceNodeId = edgeObject.get(SOURCE).getAsString(); - String destNodeId = edgeObject.get(DESTINATION).getAsString(); + String sourceNodeId = edgeObject.get(WorkflowEdge.SOURCE_FIELD).getAsString(); + String destNodeId = edgeObject.get(WorkflowEdge.DEST_FIELD).getAsString(); if (sourceNodeId.equals(destNodeId)) { throw new IllegalArgumentException("Edge connects node " + sourceNodeId + " to itself."); } diff --git a/src/test/java/org/opensearch/flowframework/template/GraphJsonUtil.java b/src/test/java/org/opensearch/flowframework/template/GraphJsonUtil.java index 6945691a0..026d4f6e6 100644 --- a/src/test/java/org/opensearch/flowframework/template/GraphJsonUtil.java +++ b/src/test/java/org/opensearch/flowframework/template/GraphJsonUtil.java @@ -8,9 +8,30 @@ */ package org.opensearch.flowframework.template; +import org.opensearch.flowframework.workflow.Workflow; + +import java.util.List; +import java.util.stream.Collectors; + /** * Utility methods to create a JSON string useful for testing nodes and edges */ public class GraphJsonUtil { + public static String node(String id) { + return "{\"" + WorkflowNode.ID_FIELD + "\": \"" + id + "\", \"" + WorkflowNode.TYPE_FIELD + "\": \"" + "placeholder" + "\"}"; + } + + public static String edge(String sourceId, String destId) { + return "{\"" + WorkflowEdge.SOURCE_FIELD + "\": \"" + sourceId + "\", \"" + WorkflowEdge.DEST_FIELD + "\": \"" + destId + "\"}"; + } + + public static String workflow(List nodes, List edges) { + return "{\"workflow\": {" + arrayField(Workflow.NODES_FIELD, nodes) + ", " + arrayField(Workflow.EDGES_FIELD, edges) + "}}"; + } + + private static String arrayField(String fieldName, List objects) { + return "\"" + fieldName + "\": [" + objects.stream().collect(Collectors.joining(", ")) + "]"; + } + } diff --git a/src/test/java/org/opensearch/flowframework/template/TemplateParserTests.java b/src/test/java/org/opensearch/flowframework/template/TemplateParserTests.java index 78e8ff28a..1a50da906 100644 --- a/src/test/java/org/opensearch/flowframework/template/TemplateParserTests.java +++ b/src/test/java/org/opensearch/flowframework/template/TemplateParserTests.java @@ -12,37 +12,16 @@ import java.util.Collections; import java.util.List; -import java.util.stream.Collectors; -import static org.opensearch.flowframework.template.TemplateParser.DESTINATION; -import static org.opensearch.flowframework.template.TemplateParser.EDGES; -import static org.opensearch.flowframework.template.TemplateParser.NODES; -import static org.opensearch.flowframework.template.TemplateParser.NODE_ID; -import static org.opensearch.flowframework.template.TemplateParser.SOURCE; -import static org.opensearch.flowframework.template.TemplateParser.WORKFLOW; +import static org.opensearch.flowframework.template.GraphJsonUtil.edge; +import static org.opensearch.flowframework.template.GraphJsonUtil.node; +import static org.opensearch.flowframework.template.GraphJsonUtil.workflow; public class TemplateParserTests extends OpenSearchTestCase { private static final String NO_START_NODE_DETECTED = "No start node detected: all nodes have a predecessor."; private static final String CYCLE_DETECTED = "Cycle detected:"; - // Input JSON generators - private static String node(String id) { - return "{\"" + NODE_ID + "\": \"" + id + "\"}"; - } - - private static String edge(String sourceId, String destId) { - return "{\"" + SOURCE + "\": \"" + sourceId + "\", \"" + DESTINATION + "\": \"" + destId + "\"}"; - } - - private static String workflow(List nodes, List edges) { - return "{\"" + WORKFLOW + "\": {" + arrayField(NODES, nodes) + ", " + arrayField(EDGES, edges) + "}}"; - } - - private static String arrayField(String fieldName, List objects) { - return "\"" + fieldName + "\": [" + objects.stream().collect(Collectors.joining(", ")) + "]"; - } - // Output list elements private static ProcessNode expectedNode(String id) { return new ProcessNode(id, null, null); From fd72b2dae3d6cca06b09f84a023d3ec8b493c119 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Fri, 22 Sep 2023 11:44:07 -0700 Subject: [PATCH 06/27] Separate WorkflowNode and ProcessNode functionality Signed-off-by: Daniel Widdis --- build.gradle | 2 +- src/main/java/demo/DataDemo.java | 12 +- src/main/java/demo/Demo.java | 10 +- src/main/java/demo/TemplateParseDemo.java | 23 ++-- .../flowframework/template/ProcessNode.java | 59 ++------- .../flowframework/template/Template.java | 101 +++++++++++---- .../template/TemplateParser.java | 116 +++++++++++------- .../flowframework/template/WorkflowEdge.java | 4 +- .../flowframework/template/WorkflowNode.java | 8 +- .../flowframework/workflow/Workflow.java | 65 ++++++---- .../template/ProcessNodeTests.java | 10 +- .../template/TemplateParserTests.java | 75 ++++++----- .../template/WorkflowEdgeTests.java | 4 +- src/test/resources/template/datademo.json | 22 ---- src/test/resources/template/demo.json | 104 ++++++++++------ 15 files changed, 345 insertions(+), 270 deletions(-) delete mode 100644 src/test/resources/template/datademo.json diff --git a/build.gradle b/build.gradle index c3b193b15..4fd9bd00f 100644 --- a/build.gradle +++ b/build.gradle @@ -106,8 +106,8 @@ repositories { dependencies { implementation "org.opensearch:opensearch:${opensearch_version}" implementation 'org.junit.jupiter:junit-jupiter:5.10.0' - implementation "com.google.code.gson:gson:2.10.1" implementation "com.google.guava:guava:32.1.2-jre" + compileOnly "com.google.guava:guava:32.1.2-jre" api group: 'org.opensearch', name:'opensearch-ml-client', version: "${opensearch_build}" configurations.all { diff --git a/src/main/java/demo/DataDemo.java b/src/main/java/demo/DataDemo.java index 4f304d2e5..d3e7eab91 100644 --- a/src/main/java/demo/DataDemo.java +++ b/src/main/java/demo/DataDemo.java @@ -13,6 +13,7 @@ import org.opensearch.common.SuppressForbidden; import org.opensearch.common.io.PathUtils; import org.opensearch.flowframework.template.ProcessNode; +import org.opensearch.flowframework.template.Template; import org.opensearch.flowframework.template.TemplateParser; import java.io.IOException; @@ -21,7 +22,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Locale; -import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.stream.Collectors; @@ -36,10 +36,11 @@ public class DataDemo { * Demonstrate parsing a JSON graph. * * @param args unused + * @throws IOException on a failure */ @SuppressForbidden(reason = "just a demo class that will be deleted") - public static void main(String[] args) { - String path = "src/test/resources/template/datademo.json"; + public static void main(String[] args) throws IOException { + String path = "src/test/resources/template/demo.json"; String json; try { json = new String(Files.readAllBytes(PathUtils.get(path)), StandardCharsets.UTF_8); @@ -49,11 +50,12 @@ public static void main(String[] args) { } logger.info("Parsing graph to sequence..."); - List processSequence = TemplateParser.parseJsonGraphToSequence(json); + Template t = TemplateParser.parseJsonToTemplate(json); + List processSequence = TemplateParser.parseWorkflowToSequence(t.workflows().get("datademo")); List> futureList = new ArrayList<>(); for (ProcessNode n : processSequence) { - Set predecessors = n.getPredecessors(); + List predecessors = n.predecessors(); logger.info( "Queueing process [{}].{}", n.id(), diff --git a/src/main/java/demo/Demo.java b/src/main/java/demo/Demo.java index 15dde22a9..19396dc2e 100644 --- a/src/main/java/demo/Demo.java +++ b/src/main/java/demo/Demo.java @@ -13,6 +13,7 @@ import org.opensearch.common.SuppressForbidden; import org.opensearch.common.io.PathUtils; import org.opensearch.flowframework.template.ProcessNode; +import org.opensearch.flowframework.template.Template; import org.opensearch.flowframework.template.TemplateParser; import java.io.IOException; @@ -21,7 +22,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Locale; -import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.stream.Collectors; @@ -36,9 +36,10 @@ public class Demo { * Demonstrate parsing a JSON graph. * * @param args unused + * @throws IOException on a failure */ @SuppressForbidden(reason = "just a demo class that will be deleted") - public static void main(String[] args) { + public static void main(String[] args) throws IOException { String path = "src/test/resources/template/demo.json"; String json; try { @@ -49,11 +50,12 @@ public static void main(String[] args) { } logger.info("Parsing graph to sequence..."); - List processSequence = TemplateParser.parseJsonGraphToSequence(json); + Template t = TemplateParser.parseJsonToTemplate(json); + List processSequence = TemplateParser.parseWorkflowToSequence(t.workflows().get("demo")); List> futureList = new ArrayList<>(); for (ProcessNode n : processSequence) { - Set predecessors = n.getPredecessors(); + List predecessors = n.predecessors(); logger.info( "Queueing process [{}].{}", n.id(), diff --git a/src/main/java/demo/TemplateParseDemo.java b/src/main/java/demo/TemplateParseDemo.java index 0e92ba8ba..6e9800861 100644 --- a/src/main/java/demo/TemplateParseDemo.java +++ b/src/main/java/demo/TemplateParseDemo.java @@ -12,18 +12,14 @@ import org.apache.logging.log4j.Logger; import org.opensearch.common.SuppressForbidden; import org.opensearch.common.io.PathUtils; -import org.opensearch.common.xcontent.LoggingDeprecationHandler; -import org.opensearch.common.xcontent.json.JsonXContent; -import org.opensearch.core.xcontent.NamedXContentRegistry; -import org.opensearch.core.xcontent.XContentParser; import org.opensearch.flowframework.template.Template; import org.opensearch.flowframework.template.TemplateParser; +import org.opensearch.flowframework.workflow.Workflow; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; - -import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; +import java.util.Map.Entry; /** * Demo class exercising {@link TemplateParser}. This will be moved to a unit test. @@ -49,15 +45,14 @@ public static void main(String[] args) throws IOException { return; } - logger.info("Parsing template..."); - XContentParser parser = JsonXContent.jsonXContent.createParser( - NamedXContentRegistry.EMPTY, - LoggingDeprecationHandler.INSTANCE, - json - ); - ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser); - Template t = Template.parse(parser); + Template t = TemplateParser.parseJsonToTemplate(json); + System.out.println(t.toJson()); System.out.println(t.toYaml()); + + for (Entry e : t.workflows().entrySet()) { + logger.info("Parsing {} workflow.", e.getKey()); + TemplateParser.parseWorkflowToSequence(e.getValue()); + } } } diff --git a/src/main/java/org/opensearch/flowframework/template/ProcessNode.java b/src/main/java/org/opensearch/flowframework/template/ProcessNode.java index 08a7ec841..b774384a4 100644 --- a/src/main/java/org/opensearch/flowframework/template/ProcessNode.java +++ b/src/main/java/org/opensearch/flowframework/template/ProcessNode.java @@ -14,10 +14,7 @@ import org.opensearch.flowframework.workflow.WorkflowStep; import java.util.ArrayList; -import java.util.Collections; import java.util.List; -import java.util.Objects; -import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; @@ -33,32 +30,22 @@ public class ProcessNode { private final String id; private final WorkflowStep workflowStep; private final WorkflowData input; - private CompletableFuture future = null; - - // will be populated during graph parsing - private Set predecessors = Collections.emptySet(); - - /** - * Create this node linked to its executing process. - * - * @param id A string identifying the workflow step - * @param workflowStep A java class implementing {@link WorkflowStep} to be executed when it's this node's turn. - */ - ProcessNode(String id, WorkflowStep workflowStep) { - this(id, workflowStep, WorkflowData.EMPTY); - } + private final List predecessors; + private final CompletableFuture future = new CompletableFuture<>(); /** - * Create this node linked to its executing process. + * Create this node linked to its executing process, including input data and any predecessor nodes. * * @param id A string identifying the workflow step * @param workflowStep A java class implementing {@link WorkflowStep} to be executed when it's this node's turn. - * @param input Input required by the node + * @param input Input required by the node encoded in a {@link WorkflowData} instance. + * @param predecessors Nodes preceding this one in the workflow */ - public ProcessNode(String id, WorkflowStep workflowStep, WorkflowData input) { + public ProcessNode(String id, WorkflowStep workflowStep, WorkflowData input, List predecessors) { this.id = id; this.workflowStep = workflowStep; this.input = input; + this.predecessors = predecessors; } /** @@ -92,41 +79,31 @@ public WorkflowData input() { * @return A future indicating the processing state of this node. * Returns {@code null} if it has not begun executing, should not happen if a workflow is sorted and executed topologically. */ - public CompletableFuture getFuture() { + public CompletableFuture future() { return future; } /** * Returns the predecessors of this node in the workflow. - * The predecessor's {@link #getFuture()} must complete before execution begins on this node. + * The predecessor's {@link #future()} must complete before execution begins on this node. * * @return a set of predecessor nodes, if any. At least one node in the graph must have no predecessors and serve as a start node. */ - public Set getPredecessors() { + public List predecessors() { return predecessors; } - /** - * Sets the predecessor node. Called by {@link TemplateParser}. - * - * @param predecessors The predecessors of this node. - */ - void setPredecessors(Set predecessors) { - this.predecessors = Set.copyOf(predecessors); - } - /** * Execute this node in the sequence. Initializes the node's {@link CompletableFuture} and completes it when the process completes. * * @return this node's future. This is returned immediately, while process execution continues asynchronously. */ public CompletableFuture execute() { - this.future = new CompletableFuture<>(); // TODO this class will be instantiated with the OpenSearch thread pool (or one for tests!) // the generic executor from that pool should be passed to this runAsync call // https://github.com/opensearch-project/opensearch-ai-flow-framework/issues/42 CompletableFuture.runAsync(() -> { - List> predFutures = predecessors.stream().map(p -> p.getFuture()).collect(Collectors.toList()); + List> predFutures = predecessors.stream().map(p -> p.future()).collect(Collectors.toList()); if (!predecessors.isEmpty()) { CompletableFuture waitForPredecessors = CompletableFuture.allOf(predFutures.toArray(new CompletableFuture[0])); try { @@ -168,20 +145,6 @@ private void handleException(Exception e) { logger.debug("<<< Completed Exceptionally {}", this.id); } - @Override - public int hashCode() { - return Objects.hash(id); - } - - @Override - public boolean equals(Object obj) { - if (this == obj) return true; - if (obj == null) return false; - if (getClass() != obj.getClass()) return false; - ProcessNode other = (ProcessNode) obj; - return Objects.equals(id, other.id); - } - @Override public String toString() { return this.id; diff --git a/src/main/java/org/opensearch/flowframework/template/Template.java b/src/main/java/org/opensearch/flowframework/template/Template.java index 48a1630a2..2e353605b 100644 --- a/src/main/java/org/opensearch/flowframework/template/Template.java +++ b/src/main/java/org/opensearch/flowframework/template/Template.java @@ -18,7 +18,6 @@ import java.io.IOException; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -53,9 +52,9 @@ public class Template implements ToXContentObject { private final String name; private final String description; private final String useCase; // probably an ENUM actually - private final String[] operations; // probably an ENUM actually + private final List operations; // probably an ENUM actually private final Version templateVersion; - private final Version[] compatibilityVersion; + private final List compatibilityVersion; private final Map userInputs; private final Map workflows; @@ -75,20 +74,20 @@ public Template( String name, String description, String useCase, - String[] operations, + List operations, Version templateVersion, - Version[] compatibilityVersion, + List compatibilityVersion, Map userInputs, Map workflows ) { this.name = name; this.description = description; this.useCase = useCase; - this.operations = operations; + this.operations = List.copyOf(operations); this.templateVersion = templateVersion; - this.compatibilityVersion = compatibilityVersion; - this.userInputs = userInputs; - this.workflows = workflows; + this.compatibilityVersion = List.copyOf(compatibilityVersion); + this.userInputs = Map.copyOf(userInputs); + this.workflows = Map.copyOf(workflows); } @Override @@ -103,12 +102,12 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } xContentBuilder.endArray(); - if (this.templateVersion != null || this.compatibilityVersion.length > 0) { + if (this.templateVersion != null || !this.compatibilityVersion.isEmpty()) { xContentBuilder.startObject(VERSION_FIELD); if (this.templateVersion != null) { xContentBuilder.field(TEMPLATE_FIELD, this.templateVersion); } - if (this.compatibilityVersion.length > 0) { + if (!this.compatibilityVersion.isEmpty()) { xContentBuilder.startArray(COMPATIBILITY_FIELD); for (Version v : this.compatibilityVersion) { xContentBuilder.value(v); @@ -146,9 +145,9 @@ public static Template parse(XContentParser parser) throws IOException { String name = null; String description = ""; String useCase = ""; - String[] operations = new String[0]; + List operations = new ArrayList<>(); Version templateVersion = null; - Version[] compatibilityVersion = new Version[0]; + List compatibilityVersion = new ArrayList<>(); Map userInputs = new HashMap<>(); Map workflows = new HashMap<>(); @@ -168,11 +167,9 @@ public static Template parse(XContentParser parser) throws IOException { break; case OPERATIONS_FIELD: ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.currentToken(), parser); - List operationsList = new ArrayList<>(); while (parser.nextToken() != XContentParser.Token.END_ARRAY) { - operationsList.add(parser.text()); + operations.add(parser.text()); } - operations = operationsList.toArray(new String[0]); break; case VERSION_FIELD: ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); @@ -185,11 +182,9 @@ public static Template parse(XContentParser parser) throws IOException { break; case COMPATIBILITY_FIELD: ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.currentToken(), parser); - List compatibilityList = new ArrayList<>(); while (parser.nextToken() != XContentParser.Token.END_ARRAY) { - compatibilityList.add(Version.fromString(parser.text())); + compatibilityVersion.add(Version.fromString(parser.text())); } - compatibilityVersion = compatibilityList.toArray(new Version[0]); break; default: throw new IOException("Unable to parse field [" + fieldName + "] in a version object."); @@ -293,6 +288,70 @@ public String toYaml() { } } + /** + * The name of this template + * @return the name + */ + public String name() { + return name; + } + + /** + * A description of what this template does + * @return the description + */ + public String description() { + return description; + } + + /** + * A canonical use case name for this template + * @return the useCase + */ + public String useCase() { + return useCase; + } + + /** + * Operations this use case supports + * @return the operations + */ + public List operations() { + return operations; + } + + /** + * The version of this template + * @return the templateVersion + */ + public Version templateVersion() { + return templateVersion; + } + + /** + * OpenSearch version compatibility of this template + * @return the compatibilityVersion + */ + public List compatibilityVersion() { + return compatibilityVersion; + } + + /** + * A map of user inputs + * @return the userInputs + */ + public Map userInputs() { + return userInputs; + } + + /** + * Workflows encoded in this template, generally corresponding to the operations returned by {@link #operations()}. + * @return the workflows + */ + public Map workflows() { + return workflows; + } + @Override public String toString() { return "Template [name=" @@ -302,11 +361,11 @@ public String toString() { + ", useCase=" + useCase + ", operations=" - + Arrays.toString(operations) + + operations + ", templateVersion=" + templateVersion + ", compatibilityVersion=" - + Arrays.toString(compatibilityVersion) + + compatibilityVersion + ", userInputs=" + userInputs + ", workflows=" diff --git a/src/main/java/org/opensearch/flowframework/template/TemplateParser.java b/src/main/java/org/opensearch/flowframework/template/TemplateParser.java index 315726acf..c225aeb85 100644 --- a/src/main/java/org/opensearch/flowframework/template/TemplateParser.java +++ b/src/main/java/org/opensearch/flowframework/template/TemplateParser.java @@ -8,16 +8,18 @@ */ package org.opensearch.flowframework.template; -import com.google.gson.Gson; -import com.google.gson.JsonElement; -import com.google.gson.JsonObject; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.opensearch.common.xcontent.LoggingDeprecationHandler; +import org.opensearch.common.xcontent.json.JsonXContent; +import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.core.xcontent.XContentParser; import org.opensearch.flowframework.workflow.Workflow; import org.opensearch.flowframework.workflow.WorkflowData; import org.opensearch.flowframework.workflow.WorkflowStep; import org.opensearch.flowframework.workflow.WorkflowStepFactory; +import java.io.IOException; import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Collections; @@ -30,6 +32,8 @@ import java.util.function.Function; import java.util.stream.Collectors; +import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; + /** * Utility class for parsing templates. */ @@ -42,67 +46,87 @@ public class TemplateParser { */ private TemplateParser() {} + /** + * Parse a JSON use case template + * + * @param json A string containing a JSON representation of a use case template + * @return A {@link Template} represented by the JSON. + * @throws IOException on failure to parse + */ + public static Template parseJsonToTemplate(String json) throws IOException { + logger.info("Parsing template..."); + XContentParser parser = JsonXContent.jsonXContent.createParser( + NamedXContentRegistry.EMPTY, + LoggingDeprecationHandler.INSTANCE, + json + ); + ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser); + return Template.parse(parser); + } + /** * Parse a JSON representation of nodes and edges into a topologically sorted list of process nodes. - * @param json A string containing a JSON representation of nodes and edges + * @param workflow A string containing a JSON representation of nodes and edges * @return A list of Process Nodes sorted topologically. All predecessors of any node will occur prior to it in the list. */ - public static List parseJsonGraphToSequence(String json) { - Gson gson = new Gson(); - JsonObject jsonObject = gson.fromJson(json, JsonObject.class); - - JsonObject graph = jsonObject.getAsJsonObject("workflow"); + public static List parseWorkflowToSequence(Workflow workflow) { + List sortedNodes = topologicalSort(workflow.nodes(), workflow.edges()); List nodes = new ArrayList<>(); - List edges = new ArrayList<>(); + Map idToNodeMap = new HashMap<>(); + for (WorkflowNode node : sortedNodes) { + WorkflowStep step = WorkflowStepFactory.get().createStep(node.type()); + WorkflowData data = new WorkflowData(node.inputs()); + List predecessorNodes = workflow.edges() + .stream() + .filter(e -> e.destination().equals(node.id())) + // since we are iterating in topological order we know all predecessors will be in the map + .map(e -> idToNodeMap.get(e.source())) + .collect(Collectors.toList()); - for (JsonElement nodeJson : graph.getAsJsonArray(Workflow.NODES_FIELD)) { - JsonObject nodeObject = nodeJson.getAsJsonObject(); - String nodeId = nodeObject.get(WorkflowNode.ID_FIELD).getAsString(); - String stepType = nodeObject.get(WorkflowNode.TYPE_FIELD).getAsString(); - WorkflowStep workflowStep = WorkflowStepFactory.get().createStep(stepType); - WorkflowData inputData = WorkflowData.EMPTY; - nodes.add(new ProcessNode(nodeId, workflowStep, inputData)); + ProcessNode processNode = new ProcessNode(node.id(), step, data, predecessorNodes); + idToNodeMap.put(processNode.id(), processNode); + nodes.add(processNode); } - for (JsonElement edgeJson : graph.getAsJsonArray(Workflow.EDGES_FIELD)) { - JsonObject edgeObject = edgeJson.getAsJsonObject(); - String sourceNodeId = edgeObject.get(WorkflowEdge.SOURCE_FIELD).getAsString(); - String destNodeId = edgeObject.get(WorkflowEdge.DEST_FIELD).getAsString(); - if (sourceNodeId.equals(destNodeId)) { - throw new IllegalArgumentException("Edge connects node " + sourceNodeId + " to itself."); + return nodes; + } + + private static List topologicalSort(List workflowNodes, List workflowEdges) { + // Basic validation + Set nodeIds = workflowNodes.stream().map(n -> n.id()).collect(Collectors.toSet()); + for (WorkflowEdge edge : workflowEdges) { + String source = edge.source(); + if (!nodeIds.contains(source)) { + throw new IllegalArgumentException("Edge source " + source + " does not correspond to a node."); + } + String dest = edge.destination(); + if (!nodeIds.contains(dest)) { + throw new IllegalArgumentException("Edge destination " + dest + " does not correspond to a node."); + } + if (source.equals(dest)) { + throw new IllegalArgumentException("Edge connects node " + source + " to itself."); } - edges.add(new WorkflowEdge(sourceNodeId, destNodeId)); } - return topologicalSort(nodes, edges); - } - - private static List topologicalSort(List nodes, List edges) { - // Define the graph - Set graph = new HashSet<>(edges); - // Map node id string to object - Map nodeMap = nodes.stream().collect(Collectors.toMap(ProcessNode::id, Function.identity())); // Build predecessor and successor maps - Map> predecessorEdges = new HashMap<>(); - Map> successorEdges = new HashMap<>(); - for (WorkflowEdge edge : edges) { - ProcessNode source = nodeMap.get(edge.getSource()); - ProcessNode dest = nodeMap.get(edge.getDestination()); + Map> predecessorEdges = new HashMap<>(); + Map> successorEdges = new HashMap<>(); + Map nodeMap = workflowNodes.stream().collect(Collectors.toMap(WorkflowNode::id, Function.identity())); + for (WorkflowEdge edge : workflowEdges) { + WorkflowNode source = nodeMap.get(edge.source()); + WorkflowNode dest = nodeMap.get(edge.destination()); predecessorEdges.computeIfAbsent(dest, k -> new HashSet<>()).add(edge); successorEdges.computeIfAbsent(source, k -> new HashSet<>()).add(edge); } - // update predecessors on the node object - nodes.stream().filter(n -> predecessorEdges.containsKey(n)).forEach(n -> { - n.setPredecessors(predecessorEdges.get(n).stream().map(e -> nodeMap.get(e.getSource())).collect(Collectors.toSet())); - }); // See https://en.wikipedia.org/wiki/Topological_sorting#Kahn's_algorithm + Set graph = new HashSet<>(workflowEdges); // L <- Empty list that will contain the sorted elements - List sortedNodes = new ArrayList<>(); + List sortedNodes = new ArrayList<>(); // S <- Set of all nodes with no incoming edge - Queue sourceNodes = new ArrayDeque<>(); - nodes.stream().filter(n -> !predecessorEdges.containsKey(n)).forEach(n -> sourceNodes.add(n)); + Queue sourceNodes = new ArrayDeque<>(); + workflowNodes.stream().filter(n -> !predecessorEdges.containsKey(n)).forEach(n -> sourceNodes.add(n)); if (sourceNodes.isEmpty()) { throw new IllegalArgumentException("No start node detected: all nodes have a predecessor."); } @@ -111,12 +135,12 @@ private static List topologicalSort(List nodes, List inputs) { this.id = id; this.type = type; - this.inputs = inputs; + this.inputs = Map.copyOf(inputs); } @Override @@ -140,7 +140,7 @@ public static WorkflowNode parse(XContentParser parser) throws IOException { * Return this node's id * @return the id */ - public String getId() { + public String id() { return id; } @@ -148,7 +148,7 @@ public String getId() { * Return this node's type * @return the type */ - public String getType() { + public String type() { return type; } @@ -156,7 +156,7 @@ public String getType() { * Return this node's input data * @return the inputs */ - public Map getInputs() { + public Map inputs() { return inputs; } diff --git a/src/main/java/org/opensearch/flowframework/workflow/Workflow.java b/src/main/java/org/opensearch/flowframework/workflow/Workflow.java index 97d38b16d..f0bddc6de 100644 --- a/src/main/java/org/opensearch/flowframework/workflow/Workflow.java +++ b/src/main/java/org/opensearch/flowframework/workflow/Workflow.java @@ -17,7 +17,6 @@ import java.io.IOException; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -40,8 +39,8 @@ public class Workflow implements ToXContentObject { public static final String EDGES_FIELD = "edges"; private final Map userParams; - private final WorkflowNode[] nodes; - private final WorkflowEdge[] edges; + private final List nodes; + private final List edges; /** * Create this workflow with any user params and the graph of nodes and edges. @@ -50,10 +49,10 @@ public class Workflow implements ToXContentObject { * @param nodes An array of {@link WorkflowNode} objects * @param edges An array of {@link WorkflowEdge} objects. */ - public Workflow(Map userParams, WorkflowNode[] nodes, WorkflowEdge[] edges) { - this.userParams = userParams; - this.nodes = nodes; - this.edges = edges; + public Workflow(Map userParams, List nodes, List edges) { + this.userParams = Map.copyOf(userParams); + this.nodes = List.copyOf(nodes); + this.edges = List.copyOf(edges); } @Override @@ -82,16 +81,16 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } /** - * Parse raw json content into a workflow instance. + * Parse raw JSON content into a workflow instance. * - * @param parser json based content parser + * @param parser JSON based content parser * @return the parsed Workflow instance * @throws IOException if content can't be parsed correctly */ public static Workflow parse(XContentParser parser) throws IOException { Map userParams = new HashMap<>(); - WorkflowNode[] nodes = null; - WorkflowEdge[] edges = null; + List nodes = new ArrayList<>(); + List edges = new ArrayList<>(); ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); while (parser.nextToken() != XContentParser.Token.END_OBJECT) { @@ -109,40 +108,58 @@ public static Workflow parse(XContentParser parser) throws IOException { case NODES_FIELD: case STEPS_FIELD: ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.currentToken(), parser); - List nodesList = new ArrayList<>(); while (parser.nextToken() != XContentParser.Token.END_ARRAY) { - nodesList.add(WorkflowNode.parse(parser)); + nodes.add(WorkflowNode.parse(parser)); } - nodes = nodesList.toArray(new WorkflowNode[0]); break; case EDGES_FIELD: ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.currentToken(), parser); - List edgesList = new ArrayList<>(); while (parser.nextToken() != XContentParser.Token.END_ARRAY) { - edgesList.add(WorkflowEdge.parse(parser)); + edges.add(WorkflowEdge.parse(parser)); } - edges = edgesList.toArray(new WorkflowEdge[0]); break; } } - if (nodes == null || nodes.length == 0) { + if (nodes.isEmpty()) { throw new IOException("A workflow must have at least one node."); } - if (edges == null || edges.length == 0) { + if (edges.isEmpty()) { // infer edges from sequence of nodes - List edgesList = new ArrayList<>(); // Start iteration at 1, will skip for a one-node array - for (int i = 1; i < nodes.length; i++) { - edgesList.add(new WorkflowEdge(nodes[i - 1].getId(), nodes[i].getId())); + for (int i = 1; i < nodes.size(); i++) { + edges.add(new WorkflowEdge(nodes.get(i - 1).id(), nodes.get(i).id())); } - edges = edgesList.toArray(new WorkflowEdge[0]); } return new Workflow(userParams, nodes, edges); } + /** + * Get user parameters. These will be passed to all workflow nodes and available as {@link WorkflowData#getParams()} + * @return the userParams + */ + public Map userParams() { + return userParams; + } + + /** + * Get the nodes in the workflow. Ordering matches the user template which may or may not match execution order. + * @return the nodes + */ + public List nodes() { + return nodes; + } + + /** + * Get the edges in the workflow. These specify connections of nodes which form a graph. + * @return the edges + */ + public List edges() { + return edges; + } + @Override public String toString() { - return "Workflow [userParams=" + userParams + ", nodes=" + Arrays.toString(nodes) + ", edges=" + Arrays.toString(edges) + "]"; + return "Workflow [userParams=" + userParams + ", nodes=" + nodes + ", edges=" + edges + "]"; } } diff --git a/src/test/java/org/opensearch/flowframework/template/ProcessNodeTests.java b/src/test/java/org/opensearch/flowframework/template/ProcessNodeTests.java index d9f365708..ff9c24e4c 100644 --- a/src/test/java/org/opensearch/flowframework/template/ProcessNodeTests.java +++ b/src/test/java/org/opensearch/flowframework/template/ProcessNodeTests.java @@ -41,11 +41,11 @@ public CompletableFuture execute(List data) { public String getName() { return "test"; } - }); + }, WorkflowData.EMPTY, Collections.emptyList()); assertEquals("A", nodeA.id()); assertEquals("test", nodeA.workflowStep().getName()); assertEquals(WorkflowData.EMPTY, nodeA.input()); - assertEquals(Collections.emptySet(), nodeA.getPredecessors()); + assertEquals(Collections.emptyList(), nodeA.predecessors()); assertEquals("A", nodeA.toString()); // TODO: This test is flaky on Windows. Disabling until thread pool is integrated @@ -55,11 +55,5 @@ public String getName() { // f.orTimeout(5, TimeUnit.SECONDS); // assertTrue(f.isDone()); // assertEquals(WorkflowData.EMPTY, f.get()); - - ProcessNode nodeB = new ProcessNode("B", null); - assertNotEquals(nodeA, nodeB); - - ProcessNode nodeA2 = new ProcessNode("A", null); - assertEquals(nodeA, nodeA2); } } diff --git a/src/test/java/org/opensearch/flowframework/template/TemplateParserTests.java b/src/test/java/org/opensearch/flowframework/template/TemplateParserTests.java index 1a50da906..1d85e059c 100644 --- a/src/test/java/org/opensearch/flowframework/template/TemplateParserTests.java +++ b/src/test/java/org/opensearch/flowframework/template/TemplateParserTests.java @@ -8,28 +8,39 @@ */ package org.opensearch.flowframework.template; +import org.opensearch.common.xcontent.LoggingDeprecationHandler; +import org.opensearch.common.xcontent.json.JsonXContent; +import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.flowframework.workflow.Workflow; import org.opensearch.test.OpenSearchTestCase; +import java.io.IOException; import java.util.Collections; import java.util.List; +import java.util.stream.Collectors; +import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; import static org.opensearch.flowframework.template.GraphJsonUtil.edge; import static org.opensearch.flowframework.template.GraphJsonUtil.node; import static org.opensearch.flowframework.template.GraphJsonUtil.workflow; public class TemplateParserTests extends OpenSearchTestCase { + private static final String MUST_HAVE_AT_LEAST_ONE_NODE = "A workflow must have at least one node."; private static final String NO_START_NODE_DETECTED = "No start node detected: all nodes have a predecessor."; private static final String CYCLE_DETECTED = "Cycle detected:"; - // Output list elements - private static ProcessNode expectedNode(String id) { - return new ProcessNode(id, null, null); - } - - // Less verbose parser - private static List parse(String json) { - return TemplateParser.parseJsonGraphToSequence(json); + // Wrap parser into string list + private static List parse(String json) throws IOException { + XContentParser parser = JsonXContent.jsonXContent.createParser( + NamedXContentRegistry.EMPTY, + LoggingDeprecationHandler.INSTANCE, + json + ); + ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser); + Workflow w = Workflow.parse(parser); + return TemplateParser.parseWorkflowToSequence(w).stream().map(ProcessNode::id).collect(Collectors.toList()); } @Override @@ -37,13 +48,13 @@ public void setUp() throws Exception { super.setUp(); } - public void testOrdering() { - List workflow; + public void testOrdering() throws IOException { + List workflow; workflow = parse(workflow(List.of(node("A"), node("B"), node("C")), List.of(edge("C", "B"), edge("B", "A")))); - assertEquals(0, workflow.indexOf(expectedNode("C"))); - assertEquals(1, workflow.indexOf(expectedNode("B"))); - assertEquals(2, workflow.indexOf(expectedNode("A"))); + assertEquals(0, workflow.indexOf("C")); + assertEquals(1, workflow.indexOf("B")); + assertEquals(2, workflow.indexOf("A")); workflow = parse( workflow( @@ -51,12 +62,12 @@ public void testOrdering() { List.of(edge("A", "B"), edge("A", "C"), edge("B", "D"), edge("C", "D")) ) ); - assertEquals(0, workflow.indexOf(expectedNode("A"))); - int b = workflow.indexOf(expectedNode("B")); - int c = workflow.indexOf(expectedNode("C")); + assertEquals(0, workflow.indexOf("A")); + int b = workflow.indexOf("B"); + int c = workflow.indexOf("C"); assertTrue(b == 1 || b == 2); assertTrue(c == 1 || c == 2); - assertEquals(3, workflow.indexOf(expectedNode("D"))); + assertEquals(3, workflow.indexOf("D")); workflow = parse( workflow( @@ -64,14 +75,14 @@ public void testOrdering() { List.of(edge("A", "B"), edge("A", "C"), edge("B", "D"), edge("D", "E"), edge("C", "E")) ) ); - assertEquals(0, workflow.indexOf(expectedNode("A"))); - b = workflow.indexOf(expectedNode("B")); - c = workflow.indexOf(expectedNode("C")); - int d = workflow.indexOf(expectedNode("D")); + assertEquals(0, workflow.indexOf("A")); + b = workflow.indexOf("B"); + c = workflow.indexOf("C"); + int d = workflow.indexOf("D"); assertTrue(b == 1 || b == 2); assertTrue(c == 1 || c == 2); assertTrue(d == 2 || d == 3); - assertEquals(4, workflow.indexOf(expectedNode("E"))); + assertEquals(4, workflow.indexOf("E")); } public void testCycles() { @@ -115,18 +126,18 @@ public void testCycles() { assertTrue(ex.getMessage().contains("D->B")); } - public void testNoEdges() { - Exception ex = assertThrows( - IllegalArgumentException.class, - () -> parse(workflow(Collections.emptyList(), Collections.emptyList())) - ); - assertEquals(NO_START_NODE_DETECTED, ex.getMessage()); + public void testNoEdges() throws IOException { + List workflow; + Exception ex = assertThrows(IOException.class, () -> parse(workflow(Collections.emptyList(), Collections.emptyList()))); + assertEquals(MUST_HAVE_AT_LEAST_ONE_NODE, ex.getMessage()); - assertEquals(List.of(expectedNode("A")), parse(workflow(List.of(node("A")), Collections.emptyList()))); + workflow = parse(workflow(List.of(node("A")), Collections.emptyList())); + assertEquals(1, workflow.size()); + assertEquals("A", workflow.get(0)); - List workflow = parse(workflow(List.of(node("A"), node("B")), Collections.emptyList())); + workflow = parse(workflow(List.of(node("A"), node("B")), Collections.emptyList())); assertEquals(2, workflow.size()); - assertTrue(workflow.contains(expectedNode("A"))); - assertTrue(workflow.contains(expectedNode("B"))); + assertTrue(workflow.contains("A")); + assertTrue(workflow.contains("B")); } } diff --git a/src/test/java/org/opensearch/flowframework/template/WorkflowEdgeTests.java b/src/test/java/org/opensearch/flowframework/template/WorkflowEdgeTests.java index f44b54527..6e863445a 100644 --- a/src/test/java/org/opensearch/flowframework/template/WorkflowEdgeTests.java +++ b/src/test/java/org/opensearch/flowframework/template/WorkflowEdgeTests.java @@ -19,8 +19,8 @@ public void setUp() throws Exception { public void testEdge() { WorkflowEdge edgeAB = new WorkflowEdge("A", "B"); - assertEquals("A", edgeAB.getSource()); - assertEquals("B", edgeAB.getDestination()); + assertEquals("A", edgeAB.source()); + assertEquals("B", edgeAB.destination()); assertEquals("A->B", edgeAB.toString()); WorkflowEdge edgeAB2 = new WorkflowEdge("A", "B"); diff --git a/src/test/resources/template/datademo.json b/src/test/resources/template/datademo.json deleted file mode 100644 index 10a2bfdc6..000000000 --- a/src/test/resources/template/datademo.json +++ /dev/null @@ -1,22 +0,0 @@ -{ - "sequence": { - "nodes": [ - { - "id": "create_index", - "step_type": "create_index", - "index_name": "demo" - }, - { - "id": "create_another_index", - "step_type": "create_index", - "index_name": "second_demo" - } - ], - "edges": [ - { - "source": "create_index", - "dest": "create_another_index" - } - ] - } -} diff --git a/src/test/resources/template/demo.json b/src/test/resources/template/demo.json index c068301e8..86ba1005c 100644 --- a/src/test/resources/template/demo.json +++ b/src/test/resources/template/demo.json @@ -1,40 +1,70 @@ { - "sequence": { - "nodes": [ - { - "id": "fetch_model", - "step_type": "fetch_model" - }, - { - "id": "create_ingest_pipeline", - "step_type": "create_ingest_pipeline" - }, - { - "id": "create_search_pipeline", - "step_type": "create_search_pipeline" - }, - { - "id": "create_neural_search_index", - "step_type": "create_neural_search_index" - } - ], - "edges": [ - { - "source": "fetch_model", - "dest": "create_ingest_pipeline" - }, - { - "source": "fetch_model", - "dest": "create_search_pipeline" - }, - { - "source": "create_ingest_pipeline", - "dest": "create_neural_search_index" - }, - { - "source": "create_search_pipeline", - "dest": "create_neural_search_index" - } - ] + "name": "demo-template", + "description": "Demonstrates workflow steps and passing around of input/output", + "user_inputs": { + "index_name": "my-knn-index", + "index_settings": { + + } + }, + "workflows": { + "demo": { + "nodes": [ + { + "id": "fetch_model", + "type": "fetch_model" + }, + { + "id": "create_ingest_pipeline", + "type": "create_ingest_pipeline" + }, + { + "id": "create_search_pipeline", + "type": "create_search_pipeline" + }, + { + "id": "create_neural_search_index", + "type": "create_neural_search_index" + } + ], + "edges": [ + { + "source": "fetch_model", + "dest": "create_ingest_pipeline" + }, + { + "source": "fetch_model", + "dest": "create_search_pipeline" + }, + { + "source": "create_ingest_pipeline", + "dest": "create_neural_search_index" + }, + { + "source": "create_search_pipeline", + "dest": "create_neural_search_index" + } + ] + }, + "datademo": { + "nodes": [ + { + "id": "create_index", + "type": "create_index", + "index_name": "demo" + }, + { + "id": "create_another_index", + "type": "create_index", + "index_name": "second_demo" + } + ], + "edges": [ + { + "source": "create_index", + "dest": "create_another_index" + } + ] + } } } From f2441b3e5a2d326198a77ef147baa7341aa0549d Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Fri, 22 Sep 2023 11:57:54 -0700 Subject: [PATCH 07/27] Fix demos to align with template field names Signed-off-by: Daniel Widdis --- .../workflow/WorkflowStepFactory.java | 2 +- src/test/resources/template/demo.json | 16 ++++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/opensearch/flowframework/workflow/WorkflowStepFactory.java b/src/main/java/org/opensearch/flowframework/workflow/WorkflowStepFactory.java index 20fc2be23..da08949df 100644 --- a/src/main/java/org/opensearch/flowframework/workflow/WorkflowStepFactory.java +++ b/src/main/java/org/opensearch/flowframework/workflow/WorkflowStepFactory.java @@ -75,6 +75,6 @@ public WorkflowStep createStep(String type) { } // TODO: replace this with a FlowFrameworkException // https://github.com/opensearch-project/opensearch-ai-flow-framework/pull/43 - throw new UnsupportedOperationException("No workflow steps of type [" + type + "] are implemented."); + return stepMap.get("placeholder"); } } diff --git a/src/test/resources/template/demo.json b/src/test/resources/template/demo.json index 86ba1005c..b3428f0ff 100644 --- a/src/test/resources/template/demo.json +++ b/src/test/resources/template/demo.json @@ -2,10 +2,7 @@ "name": "demo-template", "description": "Demonstrates workflow steps and passing around of input/output", "user_inputs": { - "index_name": "my-knn-index", - "index_settings": { - - } + "knn_index_name": "my-knn-index" }, "workflows": { "demo": { @@ -47,16 +44,23 @@ ] }, "datademo": { + "user_params": { + "workflow_id": "workflow_demo" + }, "nodes": [ { "id": "create_index", "type": "create_index", - "index_name": "demo" + "inputs": { + "index_name": "demo" + } }, { "id": "create_another_index", "type": "create_index", - "index_name": "second_demo" + "inputs": { + "index_name": "second_demo" + } } ], "edges": [ From c7a2371c7e96812a4549effdc5349641d707c11e Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Fri, 22 Sep 2023 15:40:54 -0700 Subject: [PATCH 08/27] Add workflow, node, and edge tests Signed-off-by: Daniel Widdis --- .../template/TemplateParserTests.java | 17 ++----- ...sonUtil.java => TemplateTestJsonUtil.java} | 26 +++++++++- .../template/WorkflowEdgeTests.java | 15 +++++- .../template/WorkflowNodeTests.java | 47 +++++++++++++++++ .../flowframework/template/WorkflowTests.java | 51 +++++++++++++++++++ 5 files changed, 140 insertions(+), 16 deletions(-) rename src/test/java/org/opensearch/flowframework/template/{GraphJsonUtil.java => TemplateTestJsonUtil.java} (52%) create mode 100644 src/test/java/org/opensearch/flowframework/template/WorkflowNodeTests.java create mode 100644 src/test/java/org/opensearch/flowframework/template/WorkflowTests.java diff --git a/src/test/java/org/opensearch/flowframework/template/TemplateParserTests.java b/src/test/java/org/opensearch/flowframework/template/TemplateParserTests.java index 1d85e059c..85a4a15c2 100644 --- a/src/test/java/org/opensearch/flowframework/template/TemplateParserTests.java +++ b/src/test/java/org/opensearch/flowframework/template/TemplateParserTests.java @@ -8,9 +8,6 @@ */ package org.opensearch.flowframework.template; -import org.opensearch.common.xcontent.LoggingDeprecationHandler; -import org.opensearch.common.xcontent.json.JsonXContent; -import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.core.xcontent.XContentParser; import org.opensearch.flowframework.workflow.Workflow; import org.opensearch.test.OpenSearchTestCase; @@ -20,10 +17,9 @@ import java.util.List; import java.util.stream.Collectors; -import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; -import static org.opensearch.flowframework.template.GraphJsonUtil.edge; -import static org.opensearch.flowframework.template.GraphJsonUtil.node; -import static org.opensearch.flowframework.template.GraphJsonUtil.workflow; +import static org.opensearch.flowframework.template.TemplateTestJsonUtil.edge; +import static org.opensearch.flowframework.template.TemplateTestJsonUtil.node; +import static org.opensearch.flowframework.template.TemplateTestJsonUtil.workflow; public class TemplateParserTests extends OpenSearchTestCase { @@ -33,12 +29,7 @@ public class TemplateParserTests extends OpenSearchTestCase { // Wrap parser into string list private static List parse(String json) throws IOException { - XContentParser parser = JsonXContent.jsonXContent.createParser( - NamedXContentRegistry.EMPTY, - LoggingDeprecationHandler.INSTANCE, - json - ); - ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser); + XContentParser parser = TemplateTestJsonUtil.jsonToParser(json); Workflow w = Workflow.parse(parser); return TemplateParser.parseWorkflowToSequence(w).stream().map(ProcessNode::id).collect(Collectors.toList()); } diff --git a/src/test/java/org/opensearch/flowframework/template/GraphJsonUtil.java b/src/test/java/org/opensearch/flowframework/template/TemplateTestJsonUtil.java similarity index 52% rename from src/test/java/org/opensearch/flowframework/template/GraphJsonUtil.java rename to src/test/java/org/opensearch/flowframework/template/TemplateTestJsonUtil.java index 026d4f6e6..bd59fd61b 100644 --- a/src/test/java/org/opensearch/flowframework/template/GraphJsonUtil.java +++ b/src/test/java/org/opensearch/flowframework/template/TemplateTestJsonUtil.java @@ -8,15 +8,24 @@ */ package org.opensearch.flowframework.template; +import org.opensearch.common.xcontent.LoggingDeprecationHandler; +import org.opensearch.common.xcontent.json.JsonXContent; +import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.ToXContentObject; +import org.opensearch.core.xcontent.XContentParser; import org.opensearch.flowframework.workflow.Workflow; +import java.io.IOException; import java.util.List; import java.util.stream.Collectors; +import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; + /** - * Utility methods to create a JSON string useful for testing nodes and edges + * Utility methods for tests of template JSON */ -public class GraphJsonUtil { +public class TemplateTestJsonUtil { public static String node(String id) { return "{\"" + WorkflowNode.ID_FIELD + "\": \"" + id + "\", \"" + WorkflowNode.TYPE_FIELD + "\": \"" + "placeholder" + "\"}"; @@ -34,4 +43,17 @@ private static String arrayField(String fieldName, List objects) { return "\"" + fieldName + "\": [" + objects.stream().collect(Collectors.joining(", ")) + "]"; } + public static String parseToJson(ToXContentObject object) throws IOException { + return object.toXContent(JsonXContent.contentBuilder(), ToXContent.EMPTY_PARAMS).toString(); + } + + public static XContentParser jsonToParser(String json) throws IOException { + XContentParser parser = JsonXContent.jsonXContent.createParser( + NamedXContentRegistry.EMPTY, + LoggingDeprecationHandler.INSTANCE, + json + ); + ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser); + return parser; + } } diff --git a/src/test/java/org/opensearch/flowframework/template/WorkflowEdgeTests.java b/src/test/java/org/opensearch/flowframework/template/WorkflowEdgeTests.java index 6e863445a..358081ef8 100644 --- a/src/test/java/org/opensearch/flowframework/template/WorkflowEdgeTests.java +++ b/src/test/java/org/opensearch/flowframework/template/WorkflowEdgeTests.java @@ -8,8 +8,11 @@ */ package org.opensearch.flowframework.template; +import org.opensearch.core.xcontent.XContentParser; import org.opensearch.test.OpenSearchTestCase; +import java.io.IOException; + public class WorkflowEdgeTests extends OpenSearchTestCase { @Override @@ -17,7 +20,7 @@ public void setUp() throws Exception { super.setUp(); } - public void testEdge() { + public void testEdge() throws IOException { WorkflowEdge edgeAB = new WorkflowEdge("A", "B"); assertEquals("A", edgeAB.source()); assertEquals("B", edgeAB.destination()); @@ -28,5 +31,15 @@ public void testEdge() { WorkflowEdge edgeAC = new WorkflowEdge("A", "C"); assertNotEquals(edgeAB, edgeAC); + + String expectedJson = "{\"source\":\"A\",\"dest\":\"B\"}"; + String json = TemplateTestJsonUtil.parseToJson(edgeAB); + assertEquals(expectedJson, json); + + XContentParser parser = TemplateTestJsonUtil.jsonToParser(json); + WorkflowEdge edgeX = WorkflowEdge.parse(parser); + assertEquals("A", edgeX.source()); + assertEquals("B", edgeX.destination()); + assertEquals("A->B", edgeX.toString()); } } diff --git a/src/test/java/org/opensearch/flowframework/template/WorkflowNodeTests.java b/src/test/java/org/opensearch/flowframework/template/WorkflowNodeTests.java new file mode 100644 index 000000000..83d0dcd5a --- /dev/null +++ b/src/test/java/org/opensearch/flowframework/template/WorkflowNodeTests.java @@ -0,0 +1,47 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ +package org.opensearch.flowframework.template; + +import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.test.OpenSearchTestCase; + +import java.io.IOException; +import java.util.Map; + +public class WorkflowNodeTests extends OpenSearchTestCase { + + @Override + public void setUp() throws Exception { + super.setUp(); + } + + public void testNode() throws IOException { + WorkflowNode nodeA = new WorkflowNode("A", "a-type", Map.of("foo", "bar")); + assertEquals("A", nodeA.id()); + assertEquals("a-type", nodeA.type()); + assertEquals(Map.of("foo", "bar"), nodeA.inputs()); + + // node equality is based only on ID + WorkflowNode nodeA2 = new WorkflowNode("A", "a2-type", Map.of("bar", "baz")); + assertEquals(nodeA, nodeA2); + + WorkflowNode nodeB = new WorkflowNode("B", "b-type", Map.of("baz", "qux")); + assertNotEquals(nodeA, nodeB); + + String expectedJson = "{\"id\":\"A\",\"type\":\"a-type\",\"inputs\":{\"foo\":\"bar\"}}"; + String json = TemplateTestJsonUtil.parseToJson(nodeA); + assertEquals(expectedJson, json); + + XContentParser parser = TemplateTestJsonUtil.jsonToParser(json); + WorkflowNode nodeX = WorkflowNode.parse(parser); + assertEquals("A", nodeX.id()); + assertEquals("a-type", nodeX.type()); + assertEquals(Map.of("foo", "bar"), nodeX.inputs()); + } +} diff --git a/src/test/java/org/opensearch/flowframework/template/WorkflowTests.java b/src/test/java/org/opensearch/flowframework/template/WorkflowTests.java new file mode 100644 index 000000000..ee149ecae --- /dev/null +++ b/src/test/java/org/opensearch/flowframework/template/WorkflowTests.java @@ -0,0 +1,51 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ +package org.opensearch.flowframework.template; + +import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.flowframework.workflow.Workflow; +import org.opensearch.test.OpenSearchTestCase; + +import java.io.IOException; +import java.util.List; +import java.util.Map; + +public class WorkflowTests extends OpenSearchTestCase { + + @Override + public void setUp() throws Exception { + super.setUp(); + } + + public void testWorkflow() throws IOException { + WorkflowNode nodeA = new WorkflowNode("A", "a-type", Map.of("foo", "bar")); + WorkflowNode nodeB = new WorkflowNode("B", "b-type", Map.of("baz", "qux")); + WorkflowEdge edgeAB = new WorkflowEdge("A", "B"); + List nodes = List.of(nodeA, nodeB); + List edges = List.of(edgeAB); + + Workflow workflow = new Workflow(Map.of("key", "value"), nodes, edges); + assertEquals(Map.of("key", "value"), workflow.userParams()); + assertEquals(List.of(nodeA, nodeB), workflow.nodes()); + assertEquals(List.of(edgeAB), workflow.edges()); + + String expectedJson = "{\"user_params\":{\"key\":\"value\"}," + + "\"nodes\":[{\"id\":\"A\",\"type\":\"a-type\",\"inputs\":{\"foo\":\"bar\"}}," + + "{\"id\":\"B\",\"type\":\"b-type\",\"inputs\":{\"baz\":\"qux\"}}]," + + "\"edges\":[{\"source\":\"A\",\"dest\":\"B\"}]}"; + String json = TemplateTestJsonUtil.parseToJson(workflow); + assertEquals(expectedJson, json); + + XContentParser parser = TemplateTestJsonUtil.jsonToParser(json); + Workflow workflowX = Workflow.parse(parser); + assertEquals(Map.of("key", "value"), workflowX.userParams()); + assertEquals(List.of(nodeA, nodeB), workflowX.nodes()); + assertEquals(List.of(edgeAB), workflowX.edges()); + } +} From 388741c354b96fba4ac66b970e012a62ca575967 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Fri, 22 Sep 2023 16:07:14 -0700 Subject: [PATCH 09/27] Add Template tests Signed-off-by: Daniel Widdis --- .../flowframework/template/TemplateTests.java | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 src/test/java/org/opensearch/flowframework/template/TemplateTests.java diff --git a/src/test/java/org/opensearch/flowframework/template/TemplateTests.java b/src/test/java/org/opensearch/flowframework/template/TemplateTests.java new file mode 100644 index 000000000..c079d7cf3 --- /dev/null +++ b/src/test/java/org/opensearch/flowframework/template/TemplateTests.java @@ -0,0 +1,80 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ +package org.opensearch.flowframework.template; + +import org.opensearch.Version; +import org.opensearch.flowframework.workflow.Workflow; +import org.opensearch.test.OpenSearchTestCase; + +import java.io.IOException; +import java.util.List; +import java.util.Map; + +public class TemplateTests extends OpenSearchTestCase { + + @Override + public void setUp() throws Exception { + super.setUp(); + } + + public void testTemplate() throws IOException { + Version templateVersion = Version.fromString("1.2.3"); + List compatibilityVersion = List.of(Version.fromString("4.5.6"), Version.fromString("7.8.9")); + + WorkflowNode nodeA = new WorkflowNode("A", "a-type", Map.of("foo", "bar")); + WorkflowNode nodeB = new WorkflowNode("B", "b-type", Map.of("baz", "qux")); + WorkflowEdge edgeAB = new WorkflowEdge("A", "B"); + List nodes = List.of(nodeA, nodeB); + List edges = List.of(edgeAB); + Workflow workflow = new Workflow(Map.of("key", "value"), nodes, edges); + + Template template = new Template( + "test", + "a test template", + "test use case", + List.of("operation"), + templateVersion, + compatibilityVersion, + Map.of("userKey", "userValue"), + Map.of("workflow", workflow) + ); + + assertEquals("test", template.name()); + assertEquals("a test template", template.description()); + assertEquals("test use case", template.useCase()); + assertEquals(List.of("operation"), template.operations()); + assertEquals(templateVersion, template.templateVersion()); + assertEquals(compatibilityVersion, template.compatibilityVersion()); + assertEquals(Map.of("userKey", "userValue"), template.userInputs()); + Workflow wf = template.workflows().get("workflow"); + assertNotNull(wf); + assertEquals("Workflow [userParams={key=value}, nodes=[A, B], edges=[A->B]]", wf.toString()); + + String expectedJson = "{\"name\":\"test\",\"description\":\"a test template\",\"use_case\":\"test use case\"," + + "\"operations\":[\"operation\"],\"version\":{\"template\":\"1.2.3\",\"compatibility\":[\"4.5.6\",\"7.8.9\"]}," + + "\"user_inputs\":{\"userKey\":\"userValue\"},\"workflows\":{\"workflow\":{\"user_params\":{\"key\":\"value\"}," + + "\"nodes\":[{\"id\":\"A\",\"type\":\"a-type\",\"inputs\":{\"foo\":\"bar\"}}," + + "{\"id\":\"B\",\"type\":\"b-type\",\"inputs\":{\"baz\":\"qux\"}}]," + + "\"edges\":[{\"source\":\"A\",\"dest\":\"B\"}]}}}"; + String json = TemplateTestJsonUtil.parseToJson(template); + assertEquals(expectedJson, json); + + Template templateX = TemplateParser.parseJsonToTemplate(json); + assertEquals("test", templateX.name()); + assertEquals("a test template", templateX.description()); + assertEquals("test use case", templateX.useCase()); + assertEquals(List.of("operation"), templateX.operations()); + assertEquals(templateVersion, templateX.templateVersion()); + assertEquals(compatibilityVersion, templateX.compatibilityVersion()); + assertEquals(Map.of("userKey", "userValue"), templateX.userInputs()); + Workflow wfX = templateX.workflows().get("workflow"); + assertNotNull(wfX); + assertEquals("Workflow [userParams={key=value}, nodes=[A, B], edges=[A->B]]", wfX.toString()); + } +} From df4a0d88f17dfe4df99797e1f85209cf75b6967c Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Fri, 22 Sep 2023 16:20:10 -0700 Subject: [PATCH 10/27] Refactor TemplateParser to WorkflowProcessSorter Signed-off-by: Daniel Widdis --- src/main/java/demo/DataDemo.java | 8 +-- src/main/java/demo/Demo.java | 8 +-- src/main/java/demo/TemplateParseDemo.java | 8 +-- .../flowframework/template/Template.java | 19 +++++++ ...Parser.java => WorkflowProcessSorter.java} | 49 ++++++------------- .../template/TemplateParserTests.java | 2 +- .../flowframework/template/TemplateTests.java | 2 +- 7 files changed, 49 insertions(+), 47 deletions(-) rename src/main/java/org/opensearch/flowframework/template/{TemplateParser.java => WorkflowProcessSorter.java} (77%) diff --git a/src/main/java/demo/DataDemo.java b/src/main/java/demo/DataDemo.java index d3e7eab91..5e5623aa5 100644 --- a/src/main/java/demo/DataDemo.java +++ b/src/main/java/demo/DataDemo.java @@ -14,7 +14,7 @@ import org.opensearch.common.io.PathUtils; import org.opensearch.flowframework.template.ProcessNode; import org.opensearch.flowframework.template.Template; -import org.opensearch.flowframework.template.TemplateParser; +import org.opensearch.flowframework.template.WorkflowProcessSorter; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -26,7 +26,7 @@ import java.util.stream.Collectors; /** - * Demo class exercising {@link TemplateParser}. This will be moved to a unit test. + * Demo class exercising {@link WorkflowProcessSorter}. This will be moved to a unit test. */ public class DataDemo { @@ -50,8 +50,8 @@ public static void main(String[] args) throws IOException { } logger.info("Parsing graph to sequence..."); - Template t = TemplateParser.parseJsonToTemplate(json); - List processSequence = TemplateParser.parseWorkflowToSequence(t.workflows().get("datademo")); + Template t = Template.parse(json); + List processSequence = WorkflowProcessSorter.sortProcessNodes(t.workflows().get("datademo")); List> futureList = new ArrayList<>(); for (ProcessNode n : processSequence) { diff --git a/src/main/java/demo/Demo.java b/src/main/java/demo/Demo.java index 19396dc2e..28f8aaf7f 100644 --- a/src/main/java/demo/Demo.java +++ b/src/main/java/demo/Demo.java @@ -14,7 +14,7 @@ import org.opensearch.common.io.PathUtils; import org.opensearch.flowframework.template.ProcessNode; import org.opensearch.flowframework.template.Template; -import org.opensearch.flowframework.template.TemplateParser; +import org.opensearch.flowframework.template.WorkflowProcessSorter; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -26,7 +26,7 @@ import java.util.stream.Collectors; /** - * Demo class exercising {@link TemplateParser}. This will be moved to a unit test. + * Demo class exercising {@link WorkflowProcessSorter}. This will be moved to a unit test. */ public class Demo { @@ -50,8 +50,8 @@ public static void main(String[] args) throws IOException { } logger.info("Parsing graph to sequence..."); - Template t = TemplateParser.parseJsonToTemplate(json); - List processSequence = TemplateParser.parseWorkflowToSequence(t.workflows().get("demo")); + Template t = Template.parse(json); + List processSequence = WorkflowProcessSorter.sortProcessNodes(t.workflows().get("demo")); List> futureList = new ArrayList<>(); for (ProcessNode n : processSequence) { diff --git a/src/main/java/demo/TemplateParseDemo.java b/src/main/java/demo/TemplateParseDemo.java index 6e9800861..49e712ee2 100644 --- a/src/main/java/demo/TemplateParseDemo.java +++ b/src/main/java/demo/TemplateParseDemo.java @@ -13,7 +13,7 @@ import org.opensearch.common.SuppressForbidden; import org.opensearch.common.io.PathUtils; import org.opensearch.flowframework.template.Template; -import org.opensearch.flowframework.template.TemplateParser; +import org.opensearch.flowframework.template.WorkflowProcessSorter; import org.opensearch.flowframework.workflow.Workflow; import java.io.IOException; @@ -22,7 +22,7 @@ import java.util.Map.Entry; /** - * Demo class exercising {@link TemplateParser}. This will be moved to a unit test. + * Demo class exercising {@link WorkflowProcessSorter}. This will be moved to a unit test. */ public class TemplateParseDemo { @@ -45,14 +45,14 @@ public static void main(String[] args) throws IOException { return; } - Template t = TemplateParser.parseJsonToTemplate(json); + Template t = Template.parse(json); System.out.println(t.toJson()); System.out.println(t.toYaml()); for (Entry e : t.workflows().entrySet()) { logger.info("Parsing {} workflow.", e.getKey()); - TemplateParser.parseWorkflowToSequence(e.getValue()); + WorkflowProcessSorter.sortProcessNodes(e.getValue()); } } } diff --git a/src/main/java/org/opensearch/flowframework/template/Template.java b/src/main/java/org/opensearch/flowframework/template/Template.java index 2e353605b..327a508b5 100644 --- a/src/main/java/org/opensearch/flowframework/template/Template.java +++ b/src/main/java/org/opensearch/flowframework/template/Template.java @@ -9,8 +9,10 @@ package org.opensearch.flowframework.template; import org.opensearch.Version; +import org.opensearch.common.xcontent.LoggingDeprecationHandler; import org.opensearch.common.xcontent.json.JsonXContent; import org.opensearch.common.xcontent.yaml.YamlXContent; +import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; @@ -227,6 +229,23 @@ public static Template parse(XContentParser parser) throws IOException { return new Template(name, description, useCase, operations, templateVersion, compatibilityVersion, userInputs, workflows); } + /** + * Parse a JSON use case template + * + * @param json A string containing a JSON representation of a use case template + * @return A {@link Template} represented by the JSON. + * @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); + } + /** * Builds an XContent object representing a map of String keys to String values. * diff --git a/src/main/java/org/opensearch/flowframework/template/TemplateParser.java b/src/main/java/org/opensearch/flowframework/template/WorkflowProcessSorter.java similarity index 77% rename from src/main/java/org/opensearch/flowframework/template/TemplateParser.java rename to src/main/java/org/opensearch/flowframework/template/WorkflowProcessSorter.java index c225aeb85..d1d56187d 100644 --- a/src/main/java/org/opensearch/flowframework/template/TemplateParser.java +++ b/src/main/java/org/opensearch/flowframework/template/WorkflowProcessSorter.java @@ -10,16 +10,11 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.opensearch.common.xcontent.LoggingDeprecationHandler; -import org.opensearch.common.xcontent.json.JsonXContent; -import org.opensearch.core.xcontent.NamedXContentRegistry; -import org.opensearch.core.xcontent.XContentParser; import org.opensearch.flowframework.workflow.Workflow; import org.opensearch.flowframework.workflow.WorkflowData; import org.opensearch.flowframework.workflow.WorkflowStep; import org.opensearch.flowframework.workflow.WorkflowStepFactory; -import java.io.IOException; import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Collections; @@ -32,51 +27,39 @@ import java.util.function.Function; import java.util.stream.Collectors; -import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; - /** - * Utility class for parsing templates. + * Utility class converting a workflow of nodes and edges into a topologically sorted list of Process Nodes. */ -public class TemplateParser { +public class WorkflowProcessSorter { - private static final Logger logger = LogManager.getLogger(TemplateParser.class); + private static final Logger logger = LogManager.getLogger(WorkflowProcessSorter.class); /** * Prevent instantiating this class. */ - private TemplateParser() {} - - /** - * Parse a JSON use case template - * - * @param json A string containing a JSON representation of a use case template - * @return A {@link Template} represented by the JSON. - * @throws IOException on failure to parse - */ - public static Template parseJsonToTemplate(String json) throws IOException { - logger.info("Parsing template..."); - XContentParser parser = JsonXContent.jsonXContent.createParser( - NamedXContentRegistry.EMPTY, - LoggingDeprecationHandler.INSTANCE, - json - ); - ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser); - return Template.parse(parser); - } + private WorkflowProcessSorter() {} /** - * Parse a JSON representation of nodes and edges into a topologically sorted list of process nodes. - * @param workflow A string containing a JSON representation of nodes and edges + * Sort a workflow into a topologically sorted list of process nodes. + * @param workflow A workflow with (unsorted) nodes and edges which define predecessors and successors * @return A list of Process Nodes sorted topologically. All predecessors of any node will occur prior to it in the list. */ - public static List parseWorkflowToSequence(Workflow workflow) { + public static List sortProcessNodes(Workflow workflow) { List sortedNodes = topologicalSort(workflow.nodes(), workflow.edges()); List nodes = new ArrayList<>(); Map idToNodeMap = new HashMap<>(); for (WorkflowNode node : sortedNodes) { WorkflowStep step = WorkflowStepFactory.get().createStep(node.type()); - WorkflowData data = new WorkflowData(node.inputs()); + WorkflowData data = new WorkflowData() { + public Map getContent() { + return node.inputs(); + }; + + public Map getParams() { + return workflow.userParams(); + }; + }; List predecessorNodes = workflow.edges() .stream() .filter(e -> e.destination().equals(node.id())) diff --git a/src/test/java/org/opensearch/flowframework/template/TemplateParserTests.java b/src/test/java/org/opensearch/flowframework/template/TemplateParserTests.java index 85a4a15c2..f7bdea986 100644 --- a/src/test/java/org/opensearch/flowframework/template/TemplateParserTests.java +++ b/src/test/java/org/opensearch/flowframework/template/TemplateParserTests.java @@ -31,7 +31,7 @@ public class TemplateParserTests extends OpenSearchTestCase { private static List parse(String json) throws IOException { XContentParser parser = TemplateTestJsonUtil.jsonToParser(json); Workflow w = Workflow.parse(parser); - return TemplateParser.parseWorkflowToSequence(w).stream().map(ProcessNode::id).collect(Collectors.toList()); + return WorkflowProcessSorter.sortProcessNodes(w).stream().map(ProcessNode::id).collect(Collectors.toList()); } @Override diff --git a/src/test/java/org/opensearch/flowframework/template/TemplateTests.java b/src/test/java/org/opensearch/flowframework/template/TemplateTests.java index c079d7cf3..ed3fc459b 100644 --- a/src/test/java/org/opensearch/flowframework/template/TemplateTests.java +++ b/src/test/java/org/opensearch/flowframework/template/TemplateTests.java @@ -65,7 +65,7 @@ public void testTemplate() throws IOException { String json = TemplateTestJsonUtil.parseToJson(template); assertEquals(expectedJson, json); - Template templateX = TemplateParser.parseJsonToTemplate(json); + Template templateX = Template.parse(json); assertEquals("test", templateX.name()); assertEquals("a test template", templateX.description()); assertEquals("test use case", templateX.useCase()); From 8589cfad565d3307170c891801138ac31777985a Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Fri, 22 Sep 2023 17:10:11 -0700 Subject: [PATCH 11/27] Test exceptional cases Signed-off-by: Daniel Widdis --- .../flowframework/template/WorkflowNode.java | 4 ++ .../template/WorkflowEdgeTests.java | 15 +++++-- .../template/WorkflowNodeTests.java | 40 +++++++++++++++---- 3 files changed, 48 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/opensearch/flowframework/template/WorkflowNode.java b/src/main/java/org/opensearch/flowframework/template/WorkflowNode.java index 00c09b263..d1f493ca9 100644 --- a/src/main/java/org/opensearch/flowframework/template/WorkflowNode.java +++ b/src/main/java/org/opensearch/flowframework/template/WorkflowNode.java @@ -70,6 +70,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } else if (e.getValue() instanceof Map) { Template.buildStringToStringMap(xContentBuilder, (Map) e.getValue()); } else if (e.getValue() instanceof Object[]) { + // This assumes an array of maps for "processor" key xContentBuilder.startArray(); for (Map map : (Map[]) e.getValue()) { Template.buildStringToStringMap(xContentBuilder, map); @@ -113,6 +114,9 @@ public static WorkflowNode parse(XContentParser parser) throws IOException { case VALUE_STRING: inputs.put(inputFieldName, parser.text()); break; + case START_OBJECT: + inputs.put(inputFieldName, Template.parseStringToStringMap(parser)); + break; case START_ARRAY: List> mapList = new ArrayList<>(); while (parser.nextToken() != XContentParser.Token.END_ARRAY) { diff --git a/src/test/java/org/opensearch/flowframework/template/WorkflowEdgeTests.java b/src/test/java/org/opensearch/flowframework/template/WorkflowEdgeTests.java index 358081ef8..f6b61fb52 100644 --- a/src/test/java/org/opensearch/flowframework/template/WorkflowEdgeTests.java +++ b/src/test/java/org/opensearch/flowframework/template/WorkflowEdgeTests.java @@ -8,7 +8,6 @@ */ package org.opensearch.flowframework.template; -import org.opensearch.core.xcontent.XContentParser; import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; @@ -36,10 +35,20 @@ public void testEdge() throws IOException { String json = TemplateTestJsonUtil.parseToJson(edgeAB); assertEquals(expectedJson, json); - XContentParser parser = TemplateTestJsonUtil.jsonToParser(json); - WorkflowEdge edgeX = WorkflowEdge.parse(parser); + WorkflowEdge edgeX = WorkflowEdge.parse(TemplateTestJsonUtil.jsonToParser(json)); assertEquals("A", edgeX.source()); assertEquals("B", edgeX.destination()); assertEquals("A->B", edgeX.toString()); } + + public void testExceptions() throws IOException { + String badJson = "{\"badField\":\"A\",\"dest\":\"B\"}"; + IOException e = assertThrows(IOException.class, () -> WorkflowEdge.parse(TemplateTestJsonUtil.jsonToParser(badJson))); + assertEquals("Unable to parse field [badField] in an edge object.", e.getMessage()); + + String missingJson = "{\"dest\":\"B\"}"; + e = assertThrows(IOException.class, () -> WorkflowEdge.parse(TemplateTestJsonUtil.jsonToParser(missingJson))); + assertEquals("An edge object requires both a source and dest field.", e.getMessage()); + } + } diff --git a/src/test/java/org/opensearch/flowframework/template/WorkflowNodeTests.java b/src/test/java/org/opensearch/flowframework/template/WorkflowNodeTests.java index 83d0dcd5a..157edfc01 100644 --- a/src/test/java/org/opensearch/flowframework/template/WorkflowNodeTests.java +++ b/src/test/java/org/opensearch/flowframework/template/WorkflowNodeTests.java @@ -8,7 +8,6 @@ */ package org.opensearch.flowframework.template; -import org.opensearch.core.xcontent.XContentParser; import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; @@ -22,10 +21,21 @@ public void setUp() throws Exception { } public void testNode() throws IOException { - WorkflowNode nodeA = new WorkflowNode("A", "a-type", Map.of("foo", "bar")); + WorkflowNode nodeA = new WorkflowNode( + "A", + "a-type", + Map.ofEntries( + Map.entry("foo", "a string"), + Map.entry("bar", Map.of("key", "value")), + Map.entry("baz", new Map[] { Map.of("A", "a"), Map.of("B", "b") }) + ) + ); assertEquals("A", nodeA.id()); assertEquals("a-type", nodeA.type()); - assertEquals(Map.of("foo", "bar"), nodeA.inputs()); + Map map = nodeA.inputs(); + assertEquals("a string", (String) map.get("foo")); + assertEquals(Map.of("key", "value"), (Map) map.get("bar")); + assertArrayEquals(new Map[] { Map.of("A", "a"), Map.of("B", "b") }, (Map[]) map.get("baz")); // node equality is based only on ID WorkflowNode nodeA2 = new WorkflowNode("A", "a2-type", Map.of("bar", "baz")); @@ -34,14 +44,28 @@ public void testNode() throws IOException { WorkflowNode nodeB = new WorkflowNode("B", "b-type", Map.of("baz", "qux")); assertNotEquals(nodeA, nodeB); - String expectedJson = "{\"id\":\"A\",\"type\":\"a-type\",\"inputs\":{\"foo\":\"bar\"}}"; String json = TemplateTestJsonUtil.parseToJson(nodeA); - assertEquals(expectedJson, json); + assertTrue(json.startsWith("{\"id\":\"A\",\"type\":\"a-type\",\"inputs\":")); + assertTrue(json.contains("\"foo\":\"a string\"")); + assertTrue(json.contains("\"baz\":[{\"A\":\"a\"},{\"B\":\"b\"}]")); + assertTrue(json.contains("\"bar\":{\"key\":\"value\"}")); - XContentParser parser = TemplateTestJsonUtil.jsonToParser(json); - WorkflowNode nodeX = WorkflowNode.parse(parser); + WorkflowNode nodeX = WorkflowNode.parse(TemplateTestJsonUtil.jsonToParser(json)); assertEquals("A", nodeX.id()); assertEquals("a-type", nodeX.type()); - assertEquals(Map.of("foo", "bar"), nodeX.inputs()); + Map mapX = nodeX.inputs(); + assertEquals("a string", mapX.get("foo")); + assertEquals(Map.of("key", "value"), mapX.get("bar")); + assertArrayEquals(new Map[] { Map.of("A", "a"), Map.of("B", "b") }, (Map[]) map.get("baz")); + } + + public void testExceptions() throws IOException { + String badJson = "{\"badField\":\"A\",\"type\":\"a-type\",\"inputs\":{\"foo\":\"bar\"}}"; + IOException e = assertThrows(IOException.class, () -> WorkflowNode.parse(TemplateTestJsonUtil.jsonToParser(badJson))); + assertEquals("Unable to parse field [badField] in a node object.", e.getMessage()); + + String missingJson = "{\"id\":\"A\",\"inputs\":{\"foo\":\"bar\"}}"; + e = assertThrows(IOException.class, () -> WorkflowNode.parse(TemplateTestJsonUtil.jsonToParser(missingJson))); + assertEquals("An node object requires both an id and type field.", e.getMessage()); } } From 309d059fe8f5760dec154bdcf76e6ae01b4f2aa0 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Fri, 22 Sep 2023 22:48:05 -0700 Subject: [PATCH 12/27] Finish up exceptional cases Signed-off-by: Daniel Widdis --- .../flowframework/template/TemplateTests.java | 58 +++++++++++++++---- ...s.java => WorkflowProcessSorterTests.java} | 13 ++++- 2 files changed, 60 insertions(+), 11 deletions(-) rename src/test/java/org/opensearch/flowframework/template/{TemplateParserTests.java => WorkflowProcessSorterTests.java} (89%) diff --git a/src/test/java/org/opensearch/flowframework/template/TemplateTests.java b/src/test/java/org/opensearch/flowframework/template/TemplateTests.java index ed3fc459b..70d341152 100644 --- a/src/test/java/org/opensearch/flowframework/template/TemplateTests.java +++ b/src/test/java/org/opensearch/flowframework/template/TemplateTests.java @@ -18,6 +18,15 @@ public class TemplateTests extends OpenSearchTestCase { + private String expectedPrefix = + "{\"name\":\"test\",\"description\":\"a test template\",\"use_case\":\"test use case\",\"operations\":[\"operation\"]," + + "\"version\":{\"template\":\"1.2.3\",\"compatibility\":[\"4.5.6\",\"7.8.9\"]},\"user_inputs\":{"; + private String expectedKV1 = "\"userKey\":\"userValue\""; + private String expectedKV2 = "\"userMapKey\":{\"nestedKey\":\"nestedValue\"}"; + private String expectedSuffix = "},\"workflows\":{\"workflow\":{\"user_params\":{\"key\":\"value\"}," + + "\"nodes\":[{\"id\":\"A\",\"type\":\"a-type\",\"inputs\":{\"foo\":\"bar\"}}," + + "{\"id\":\"B\",\"type\":\"b-type\",\"inputs\":{\"baz\":\"qux\"}}],\"edges\":[{\"source\":\"A\",\"dest\":\"B\"}]}}}"; + @Override public void setUp() throws Exception { super.setUp(); @@ -41,7 +50,7 @@ public void testTemplate() throws IOException { List.of("operation"), templateVersion, compatibilityVersion, - Map.of("userKey", "userValue"), + Map.ofEntries(Map.entry("userKey", "userValue"), Map.entry("userMapKey", Map.of("nestedKey", "nestedValue"))), Map.of("workflow", workflow) ); @@ -51,19 +60,18 @@ public void testTemplate() throws IOException { assertEquals(List.of("operation"), template.operations()); assertEquals(templateVersion, template.templateVersion()); assertEquals(compatibilityVersion, template.compatibilityVersion()); - assertEquals(Map.of("userKey", "userValue"), template.userInputs()); + Map inputsMap = template.userInputs(); + assertEquals("userValue", inputsMap.get("userKey")); + assertEquals(Map.of("nestedKey", "nestedValue"), inputsMap.get("userMapKey")); Workflow wf = template.workflows().get("workflow"); assertNotNull(wf); assertEquals("Workflow [userParams={key=value}, nodes=[A, B], edges=[A->B]]", wf.toString()); - String expectedJson = "{\"name\":\"test\",\"description\":\"a test template\",\"use_case\":\"test use case\"," - + "\"operations\":[\"operation\"],\"version\":{\"template\":\"1.2.3\",\"compatibility\":[\"4.5.6\",\"7.8.9\"]}," - + "\"user_inputs\":{\"userKey\":\"userValue\"},\"workflows\":{\"workflow\":{\"user_params\":{\"key\":\"value\"}," - + "\"nodes\":[{\"id\":\"A\",\"type\":\"a-type\",\"inputs\":{\"foo\":\"bar\"}}," - + "{\"id\":\"B\",\"type\":\"b-type\",\"inputs\":{\"baz\":\"qux\"}}]," - + "\"edges\":[{\"source\":\"A\",\"dest\":\"B\"}]}}}"; String json = TemplateTestJsonUtil.parseToJson(template); - assertEquals(expectedJson, json); + assertTrue(json.startsWith(expectedPrefix)); + assertTrue(json.contains(expectedKV1)); + assertTrue(json.contains(expectedKV2)); + assertTrue(json.endsWith(expectedSuffix)); Template templateX = Template.parse(json); assertEquals("test", templateX.name()); @@ -72,9 +80,39 @@ public void testTemplate() throws IOException { assertEquals(List.of("operation"), templateX.operations()); assertEquals(templateVersion, templateX.templateVersion()); assertEquals(compatibilityVersion, templateX.compatibilityVersion()); - assertEquals(Map.of("userKey", "userValue"), templateX.userInputs()); + Map inputsMapX = template.userInputs(); + assertEquals("userValue", inputsMapX.get("userKey")); + assertEquals(Map.of("nestedKey", "nestedValue"), inputsMapX.get("userMapKey")); Workflow wfX = templateX.workflows().get("workflow"); assertNotNull(wfX); assertEquals("Workflow [userParams={key=value}, nodes=[A, B], edges=[A->B]]", wfX.toString()); } + + public void testExceptions() throws IOException { + String json = expectedPrefix + expectedKV1 + "," + expectedKV2 + expectedSuffix; + IOException e; + + String badTemplateField = json.replace("use_case", "badField"); + e = assertThrows(IOException.class, () -> Template.parse(badTemplateField)); + assertEquals("Unable to parse field [badField] in a template object.", e.getMessage()); + + String badVersionField = json.replace("compatibility", "badField"); + e = assertThrows(IOException.class, () -> Template.parse(badVersionField)); + assertEquals("Unable to parse field [version] in a version object.", e.getMessage()); + + String badUserInputType = json.replace("{\"nestedKey\":\"nestedValue\"}},", "[]"); + e = assertThrows(IOException.class, () -> Template.parse(badUserInputType)); + assertEquals("Unable to parse field [userMapKey] in a user inputs object.", e.getMessage()); + } + + public void testStrings() throws IOException { + Template t = Template.parse(expectedPrefix + expectedKV1 + "," + expectedKV2 + expectedSuffix); + assertTrue(t.toJson().contains(expectedPrefix)); + assertTrue(t.toJson().contains(expectedKV1)); + assertTrue(t.toJson().contains(expectedKV2)); + assertTrue(t.toJson().contains(expectedSuffix)); + + assertTrue(t.toYaml().contains("a test template")); + assertTrue(t.toString().contains("a test template")); + } } diff --git a/src/test/java/org/opensearch/flowframework/template/TemplateParserTests.java b/src/test/java/org/opensearch/flowframework/template/WorkflowProcessSorterTests.java similarity index 89% rename from src/test/java/org/opensearch/flowframework/template/TemplateParserTests.java rename to src/test/java/org/opensearch/flowframework/template/WorkflowProcessSorterTests.java index f7bdea986..537fedbc7 100644 --- a/src/test/java/org/opensearch/flowframework/template/TemplateParserTests.java +++ b/src/test/java/org/opensearch/flowframework/template/WorkflowProcessSorterTests.java @@ -21,7 +21,7 @@ import static org.opensearch.flowframework.template.TemplateTestJsonUtil.node; import static org.opensearch.flowframework.template.TemplateTestJsonUtil.workflow; -public class TemplateParserTests extends OpenSearchTestCase { +public class WorkflowProcessSorterTests extends OpenSearchTestCase { private static final String MUST_HAVE_AT_LEAST_ONE_NODE = "A workflow must have at least one node."; private static final String NO_START_NODE_DETECTED = "No start node detected: all nodes have a predecessor."; @@ -131,4 +131,15 @@ public void testNoEdges() throws IOException { assertTrue(workflow.contains("A")); assertTrue(workflow.contains("B")); } + + public void testExceptions() throws IOException { + Exception ex = assertThrows( + IllegalArgumentException.class, + () -> parse(workflow(List.of(node("A"), node("B")), List.of(edge("C", "B")))) + ); + assertEquals("Edge source C does not correspond to a node.", ex.getMessage()); + + ex = assertThrows(IllegalArgumentException.class, () -> parse(workflow(List.of(node("A"), node("B")), List.of(edge("A", "C"))))); + assertEquals("Edge destination C does not correspond to a node.", ex.getMessage()); + } } From 2804a52e72cbbac91861ac7937c74a8613a75225 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Sun, 24 Sep 2023 22:39:03 -0700 Subject: [PATCH 13/27] Fix a template field name bug in demo Signed-off-by: Daniel Widdis --- src/main/java/demo/CreateIndexWorkflowStep.java | 12 ++++-------- .../flowframework/template/ProcessNode.java | 6 +++--- .../flowframework/workflow/WorkflowStepFactory.java | 10 +++++++--- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/main/java/demo/CreateIndexWorkflowStep.java b/src/main/java/demo/CreateIndexWorkflowStep.java index 6b2ab0a7b..21f1e27c2 100644 --- a/src/main/java/demo/CreateIndexWorkflowStep.java +++ b/src/main/java/demo/CreateIndexWorkflowStep.java @@ -43,20 +43,16 @@ public CompletableFuture execute(List data) { // https://github.com/opensearch-project/opensearch-ai-flow-framework/issues/42 CompletableFuture.runAsync(() -> { String inputIndex = null; - boolean first = true; for (WorkflowData wfData : data) { logger.debug( "{} sent params: {}, content: {}", - first ? "Initialization" : "Previous step", + inputIndex == null ? "Initialization" : "Previous step", wfData.getParams(), wfData.getContent() ); - if (first) { - Map params = data.get(0).getParams(); - if (params.containsKey("index")) { - inputIndex = params.get("index"); - } - first = false; + if (inputIndex == null) { + inputIndex = wfData.getParams() + .getOrDefault("index_name", (String) wfData.getContent().getOrDefault("index_name", "NOT FOUND")); } } // do some work, simulating a REST API call diff --git a/src/main/java/org/opensearch/flowframework/template/ProcessNode.java b/src/main/java/org/opensearch/flowframework/template/ProcessNode.java index b774384a4..81df31fa3 100644 --- a/src/main/java/org/opensearch/flowframework/template/ProcessNode.java +++ b/src/main/java/org/opensearch/flowframework/template/ProcessNode.java @@ -129,9 +129,9 @@ public CompletableFuture execute() { } CompletableFuture stepFuture = this.workflowStep.execute(input); try { - stepFuture.join(); + stepFuture.orTimeout(15, TimeUnit.SECONDS).join(); + logger.info(">>> Finished {}.", this.id); future.complete(stepFuture.get()); - logger.debug("<<< Completed {}", this.id); } catch (InterruptedException | ExecutionException e) { handleException(e); } @@ -142,7 +142,7 @@ public CompletableFuture execute() { private void handleException(Exception e) { // TODO: better handling of getCause this.future.completeExceptionally(e); - logger.debug("<<< Completed Exceptionally {}", this.id); + logger.debug("<<< Completed Exceptionally {}", this.id, e.getCause()); } @Override diff --git a/src/main/java/org/opensearch/flowframework/workflow/WorkflowStepFactory.java b/src/main/java/org/opensearch/flowframework/workflow/WorkflowStepFactory.java index da08949df..e375910df 100644 --- a/src/main/java/org/opensearch/flowframework/workflow/WorkflowStepFactory.java +++ b/src/main/java/org/opensearch/flowframework/workflow/WorkflowStepFactory.java @@ -42,7 +42,6 @@ private void populateMap() { // Replace with actual implementations such as // https://github.com/opensearch-project/opensearch-ai-flow-framework/pull/38 // https://github.com/opensearch-project/opensearch-ai-flow-framework/pull/44 - stepMap.put("create_index", new CreateIndexWorkflowStep()); stepMap.put("fetch_model", new DemoWorkflowStep(3000)); stepMap.put("create_ingest_pipeline", new DemoWorkflowStep(3000)); stepMap.put("create_search_pipeline", new DemoWorkflowStep(5000)); @@ -70,8 +69,13 @@ public String getName() { * @return an instance of the specified type */ public WorkflowStep createStep(String type) { - if (stepMap.containsKey(type)) { - return stepMap.get(type); + switch (type) { + case "create_index": + return new CreateIndexWorkflowStep(); + default: + if (stepMap.containsKey(type)) { + return stepMap.get(type); + } } // TODO: replace this with a FlowFrameworkException // https://github.com/opensearch-project/opensearch-ai-flow-framework/pull/43 From d846cb9e01d83cbb0833d90b208e728da10efe16 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Mon, 25 Sep 2023 12:54:22 -0700 Subject: [PATCH 14/27] Rebase with #34 Signed-off-by: Daniel Widdis --- build.gradle | 1 - 1 file changed, 1 deletion(-) diff --git a/build.gradle b/build.gradle index 4fd9bd00f..2f6d80aa9 100644 --- a/build.gradle +++ b/build.gradle @@ -107,7 +107,6 @@ dependencies { implementation "org.opensearch:opensearch:${opensearch_version}" implementation 'org.junit.jupiter:junit-jupiter:5.10.0' implementation "com.google.guava:guava:32.1.2-jre" - compileOnly "com.google.guava:guava:32.1.2-jre" api group: 'org.opensearch', name:'opensearch-ml-client', version: "${opensearch_build}" configurations.all { From 078059b86cce6607e9fd5a4b41014aaac5600b80 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Mon, 25 Sep 2023 13:50:07 -0700 Subject: [PATCH 15/27] Rebase changes from #54 Signed-off-by: Daniel Widdis --- .../flowframework/template/WorkflowProcessSorter.java | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/main/java/org/opensearch/flowframework/template/WorkflowProcessSorter.java b/src/main/java/org/opensearch/flowframework/template/WorkflowProcessSorter.java index d1d56187d..59ca044c9 100644 --- a/src/main/java/org/opensearch/flowframework/template/WorkflowProcessSorter.java +++ b/src/main/java/org/opensearch/flowframework/template/WorkflowProcessSorter.java @@ -51,15 +51,7 @@ public static List sortProcessNodes(Workflow workflow) { Map idToNodeMap = new HashMap<>(); for (WorkflowNode node : sortedNodes) { WorkflowStep step = WorkflowStepFactory.get().createStep(node.type()); - WorkflowData data = new WorkflowData() { - public Map getContent() { - return node.inputs(); - }; - - public Map getParams() { - return workflow.userParams(); - }; - }; + WorkflowData data = new WorkflowData(node.inputs(), workflow.userParams()); List predecessorNodes = workflow.edges() .stream() .filter(e -> e.destination().equals(node.id())) From 90e95e3aad575ab084500d89825d1f4bea59edf5 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Mon, 25 Sep 2023 17:53:50 -0700 Subject: [PATCH 16/27] Integrate thread pool executor service Signed-off-by: Daniel Widdis --- src/main/java/demo/DataDemo.java | 10 +++- src/main/java/demo/Demo.java | 10 +++- src/main/java/demo/TemplateParseDemo.java | 6 ++- .../flowframework/FlowFrameworkPlugin.java | 15 +++--- .../flowframework/template/ProcessNode.java | 9 +++- .../template/WorkflowProcessSorter.java | 43 ++++++++++++++-- .../workflow/CreateIndex/CreateIndexStep.java | 6 ++- .../workflow/WorkflowStepFactory.java | 49 +++++++++++++------ .../template/ProcessNodeTests.java | 30 ++++++++---- .../template/WorkflowProcessSorterTests.java | 23 +++++++-- 10 files changed, 154 insertions(+), 47 deletions(-) diff --git a/src/main/java/demo/DataDemo.java b/src/main/java/demo/DataDemo.java index 5e5623aa5..36c27efb6 100644 --- a/src/main/java/demo/DataDemo.java +++ b/src/main/java/demo/DataDemo.java @@ -15,6 +15,7 @@ import org.opensearch.flowframework.template.ProcessNode; import org.opensearch.flowframework.template.Template; import org.opensearch.flowframework.template.WorkflowProcessSorter; +import org.opensearch.flowframework.workflow.WorkflowStepFactory; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -23,6 +24,8 @@ import java.util.List; import java.util.Locale; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.stream.Collectors; /** @@ -48,10 +51,13 @@ public static void main(String[] args) throws IOException { logger.error("Failed to read JSON at path {}", path); return; } + WorkflowStepFactory factory = WorkflowStepFactory.create(null); + ExecutorService executor = Executors.newFixedThreadPool(10); + WorkflowProcessSorter.create(factory, executor); logger.info("Parsing graph to sequence..."); Template t = Template.parse(json); - List processSequence = WorkflowProcessSorter.sortProcessNodes(t.workflows().get("datademo")); + List processSequence = WorkflowProcessSorter.get().sortProcessNodes(t.workflows().get("datademo")); List> futureList = new ArrayList<>(); for (ProcessNode n : processSequence) { @@ -71,6 +77,6 @@ public static void main(String[] args) throws IOException { } futureList.forEach(CompletableFuture::join); logger.info("All done!"); + executor.shutdown(); } - } diff --git a/src/main/java/demo/Demo.java b/src/main/java/demo/Demo.java index 28f8aaf7f..ab709089d 100644 --- a/src/main/java/demo/Demo.java +++ b/src/main/java/demo/Demo.java @@ -15,6 +15,7 @@ import org.opensearch.flowframework.template.ProcessNode; import org.opensearch.flowframework.template.Template; import org.opensearch.flowframework.template.WorkflowProcessSorter; +import org.opensearch.flowframework.workflow.WorkflowStepFactory; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -23,6 +24,8 @@ import java.util.List; import java.util.Locale; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.stream.Collectors; /** @@ -48,10 +51,13 @@ public static void main(String[] args) throws IOException { logger.error("Failed to read JSON at path {}", path); return; } + WorkflowStepFactory factory = WorkflowStepFactory.create(null); + ExecutorService executor = Executors.newFixedThreadPool(10); + WorkflowProcessSorter.create(factory, executor); logger.info("Parsing graph to sequence..."); Template t = Template.parse(json); - List processSequence = WorkflowProcessSorter.sortProcessNodes(t.workflows().get("demo")); + List processSequence = WorkflowProcessSorter.get().sortProcessNodes(t.workflows().get("demo")); List> futureList = new ArrayList<>(); for (ProcessNode n : processSequence) { @@ -71,6 +77,6 @@ public static void main(String[] args) throws IOException { } futureList.forEach(CompletableFuture::join); logger.info("All done!"); + executor.shutdown(); } - } diff --git a/src/main/java/demo/TemplateParseDemo.java b/src/main/java/demo/TemplateParseDemo.java index 49e712ee2..719e5bf89 100644 --- a/src/main/java/demo/TemplateParseDemo.java +++ b/src/main/java/demo/TemplateParseDemo.java @@ -15,11 +15,13 @@ import org.opensearch.flowframework.template.Template; import org.opensearch.flowframework.template.WorkflowProcessSorter; import org.opensearch.flowframework.workflow.Workflow; +import org.opensearch.flowframework.workflow.WorkflowStepFactory; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.util.Map.Entry; +import java.util.concurrent.Executors; /** * Demo class exercising {@link WorkflowProcessSorter}. This will be moved to a unit test. @@ -44,6 +46,8 @@ public static void main(String[] args) throws IOException { logger.error("Failed to read JSON at path {}", path); return; } + WorkflowStepFactory factory = WorkflowStepFactory.create(null); + WorkflowProcessSorter.create(factory, Executors.newFixedThreadPool(10)); Template t = Template.parse(json); @@ -52,7 +56,7 @@ public static void main(String[] args) throws IOException { for (Entry e : t.workflows().entrySet()) { logger.info("Parsing {} workflow.", e.getKey()); - WorkflowProcessSorter.sortProcessNodes(e.getValue()); + WorkflowProcessSorter.get().sortProcessNodes(e.getValue()); } } } diff --git a/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java b/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java index e5df0bf46..5f0429422 100644 --- a/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java +++ b/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java @@ -16,8 +16,13 @@ import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.env.Environment; import org.opensearch.env.NodeEnvironment; +<<<<<<< HEAD import org.opensearch.flowframework.workflow.CreateIndex.CreateIndexStep; import org.opensearch.flowframework.workflow.CreateIngestPipelineStep; +======= +import org.opensearch.flowframework.template.WorkflowProcessSorter; +import org.opensearch.flowframework.workflow.WorkflowStepFactory; +>>>>>>> abffd2d (Integrate thread pool executor service) import org.opensearch.plugins.Plugin; import org.opensearch.repositories.RepositoriesService; import org.opensearch.script.ScriptService; @@ -32,8 +37,6 @@ */ public class FlowFrameworkPlugin extends Plugin { - private Client client; - @Override public Collection createComponents( Client client, @@ -48,9 +51,9 @@ public Collection createComponents( IndexNameExpressionResolver indexNameExpressionResolver, Supplier repositoriesServiceSupplier ) { - this.client = client; - CreateIngestPipelineStep createIngestPipelineStep = new CreateIngestPipelineStep(client); - CreateIndexStep createIndexStep = new CreateIndexStep(client); - return ImmutableList.of(createIngestPipelineStep, createIndexStep); + WorkflowStepFactory workflowStepFactory = WorkflowStepFactory.create(client); + WorkflowProcessSorter workflowProcessSorter = WorkflowProcessSorter.create(workflowStepFactory, threadPool.generic()); + + return ImmutableList.of(workflowStepFactory, workflowProcessSorter); } } diff --git a/src/main/java/org/opensearch/flowframework/template/ProcessNode.java b/src/main/java/org/opensearch/flowframework/template/ProcessNode.java index 81df31fa3..071bda159 100644 --- a/src/main/java/org/opensearch/flowframework/template/ProcessNode.java +++ b/src/main/java/org/opensearch/flowframework/template/ProcessNode.java @@ -17,6 +17,7 @@ import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; @@ -31,6 +32,8 @@ public class ProcessNode { private final WorkflowStep workflowStep; private final WorkflowData input; private final List predecessors; + private Executor executor; + private final CompletableFuture future = new CompletableFuture<>(); /** @@ -40,12 +43,14 @@ public class ProcessNode { * @param workflowStep A java class implementing {@link WorkflowStep} to be executed when it's this node's turn. * @param input Input required by the node encoded in a {@link WorkflowData} instance. * @param predecessors Nodes preceding this one in the workflow + * @param executor The OpenSearch thread pool */ - public ProcessNode(String id, WorkflowStep workflowStep, WorkflowData input, List predecessors) { + public ProcessNode(String id, WorkflowStep workflowStep, WorkflowData input, List predecessors, Executor executor) { this.id = id; this.workflowStep = workflowStep; this.input = input; this.predecessors = predecessors; + this.executor = executor; } /** @@ -135,7 +140,7 @@ public CompletableFuture execute() { } catch (InterruptedException | ExecutionException e) { handleException(e); } - }); + }, executor); return this.future; } diff --git a/src/main/java/org/opensearch/flowframework/template/WorkflowProcessSorter.java b/src/main/java/org/opensearch/flowframework/template/WorkflowProcessSorter.java index 59ca044c9..aaf200fff 100644 --- a/src/main/java/org/opensearch/flowframework/template/WorkflowProcessSorter.java +++ b/src/main/java/org/opensearch/flowframework/template/WorkflowProcessSorter.java @@ -24,6 +24,7 @@ import java.util.Map; import java.util.Queue; import java.util.Set; +import java.util.concurrent.Executor; import java.util.function.Function; import java.util.stream.Collectors; @@ -34,23 +35,55 @@ public class WorkflowProcessSorter { private static final Logger logger = LogManager.getLogger(WorkflowProcessSorter.class); + private static WorkflowProcessSorter instance = null; + + private WorkflowStepFactory workflowStepFactory; + private Executor executor; + /** - * Prevent instantiating this class. + * Create the singleton instance of this class. Throws an {@link IllegalStateException} if already created. + * + * @param workflowStepFactory The singleton instance of {@link WorkflowStepFactory} + * @param executor A thread executor + * @return The created instance */ - private WorkflowProcessSorter() {} + public static synchronized WorkflowProcessSorter create(WorkflowStepFactory workflowStepFactory, Executor executor) { + if (instance != null) { + throw new IllegalStateException("This class was already created."); + } + instance = new WorkflowProcessSorter(workflowStepFactory, executor); + return instance; + } + + /** + * Gets the singleton instance of this class. Throws an {@link IllegalStateException} if not yet created. + * + * @return The created instance + */ + public static synchronized WorkflowProcessSorter get() { + if (instance == null) { + throw new IllegalStateException("This factory has not yet been created."); + } + return instance; + } + + private WorkflowProcessSorter(WorkflowStepFactory workflowStepFactory, Executor executor) { + this.workflowStepFactory = workflowStepFactory; + this.executor = executor; + } /** * Sort a workflow into a topologically sorted list of process nodes. * @param workflow A workflow with (unsorted) nodes and edges which define predecessors and successors * @return A list of Process Nodes sorted topologically. All predecessors of any node will occur prior to it in the list. */ - public static List sortProcessNodes(Workflow workflow) { + public List sortProcessNodes(Workflow workflow) { List sortedNodes = topologicalSort(workflow.nodes(), workflow.edges()); List nodes = new ArrayList<>(); Map idToNodeMap = new HashMap<>(); for (WorkflowNode node : sortedNodes) { - WorkflowStep step = WorkflowStepFactory.get().createStep(node.type()); + WorkflowStep step = workflowStepFactory.createStep(node.type()); WorkflowData data = new WorkflowData(node.inputs(), workflow.userParams()); List predecessorNodes = workflow.edges() .stream() @@ -59,7 +92,7 @@ public static List sortProcessNodes(Workflow workflow) { .map(e -> idToNodeMap.get(e.source())) .collect(Collectors.toList()); - ProcessNode processNode = new ProcessNode(node.id(), step, data, predecessorNodes); + ProcessNode processNode = new ProcessNode(node.id(), step, data, predecessorNodes, executor); idToNodeMap.put(processNode.id(), processNode); nodes.add(processNode); } diff --git a/src/main/java/org/opensearch/flowframework/workflow/CreateIndex/CreateIndexStep.java b/src/main/java/org/opensearch/flowframework/workflow/CreateIndex/CreateIndexStep.java index 7f92b8057..12675b445 100644 --- a/src/main/java/org/opensearch/flowframework/workflow/CreateIndex/CreateIndexStep.java +++ b/src/main/java/org/opensearch/flowframework/workflow/CreateIndex/CreateIndexStep.java @@ -20,6 +20,7 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.flowframework.workflow.WorkflowData; import org.opensearch.flowframework.workflow.WorkflowStep; +import org.opensearch.flowframework.workflow.WorkflowStepFactory; import java.io.IOException; import java.net.URL; @@ -34,7 +35,9 @@ public class CreateIndexStep implements WorkflowStep { private static final Logger logger = LogManager.getLogger(CreateIndexStep.class); private Client client; - private final String NAME = "create_index_step"; + + /** The name of this step, used as a key in the template and the {@link WorkflowStepFactory} */ + public static final String NAME = "create_index_step"; /** * Instantiate this class @@ -79,6 +82,7 @@ public void onFailure(Exception e) { // 1. Create settings based on the index settings received from content try { + // TODO: mapping() is deprecated CreateIndexRequest request = new CreateIndexRequest(index).mapping( getIndexMappings("mappings/" + type + ".json"), XContentType.JSON diff --git a/src/main/java/org/opensearch/flowframework/workflow/WorkflowStepFactory.java b/src/main/java/org/opensearch/flowframework/workflow/WorkflowStepFactory.java index e375910df..cd203f8de 100644 --- a/src/main/java/org/opensearch/flowframework/workflow/WorkflowStepFactory.java +++ b/src/main/java/org/opensearch/flowframework/workflow/WorkflowStepFactory.java @@ -8,12 +8,14 @@ */ package org.opensearch.flowframework.workflow; +import org.opensearch.client.Client; +import org.opensearch.flowframework.workflow.CreateIndex.CreateIndexStep; + import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.CompletableFuture; -import demo.CreateIndexWorkflowStep; import demo.DemoWorkflowStep; /** @@ -21,23 +23,45 @@ */ public class WorkflowStepFactory { - private static final WorkflowStepFactory INSTANCE = new WorkflowStepFactory(); + private static WorkflowStepFactory instance = null; + private Client client; private final Map stepMap = new HashMap<>(); - private WorkflowStepFactory() { - populateMap(); + /** + * Create the singleton instance of this class. Throws an {@link IllegalStateException} if already created. + * + * @param client The OpenSearch client steps can use + * @return The created instance + */ + public static synchronized WorkflowStepFactory create(Client client) { + if (instance != null) { + throw new IllegalStateException("This factory was already created."); + } + instance = new WorkflowStepFactory(client); + return instance; } /** - * Gets the singleton instance of this class - * @return The instance of this class + * Gets the singleton instance of this class. Throws an {@link IllegalStateException} if not yet created. + * + * @return The created instance */ - public static WorkflowStepFactory get() { - return INSTANCE; + public static synchronized WorkflowStepFactory get() { + if (instance == null) { + throw new IllegalStateException("This factory has not yet been created."); + } + return instance; + } + + private WorkflowStepFactory(Client client) { + this.client = client; + populateMap(); } private void populateMap() { + stepMap.put(CreateIndexStep.NAME, new CreateIndexStep(client)); + // TODO: These are from the demo class as placeholders // Replace with actual implementations such as // https://github.com/opensearch-project/opensearch-ai-flow-framework/pull/38 @@ -69,13 +93,8 @@ public String getName() { * @return an instance of the specified type */ public WorkflowStep createStep(String type) { - switch (type) { - case "create_index": - return new CreateIndexWorkflowStep(); - default: - if (stepMap.containsKey(type)) { - return stepMap.get(type); - } + if (stepMap.containsKey(type)) { + return stepMap.get(type); } // TODO: replace this with a FlowFrameworkException // https://github.com/opensearch-project/opensearch-ai-flow-framework/pull/43 diff --git a/src/test/java/org/opensearch/flowframework/template/ProcessNodeTests.java b/src/test/java/org/opensearch/flowframework/template/ProcessNodeTests.java index ff9c24e4c..2e7c6168c 100644 --- a/src/test/java/org/opensearch/flowframework/template/ProcessNodeTests.java +++ b/src/test/java/org/opensearch/flowframework/template/ProcessNodeTests.java @@ -14,18 +14,30 @@ import org.opensearch.flowframework.workflow.WorkflowData; import org.opensearch.flowframework.workflow.WorkflowStep; import org.opensearch.test.OpenSearchTestCase; +import org.junit.AfterClass; +import org.junit.BeforeClass; import java.util.Collections; import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; @ThreadLeakScope(Scope.NONE) public class ProcessNodeTests extends OpenSearchTestCase { - @Override - public void setUp() throws Exception { - super.setUp(); + private static ExecutorService executor; + + @BeforeClass + public static void setup() { + executor = Executors.newFixedThreadPool(10); + } + + @AfterClass + public static void cleanup() { + executor.shutdown(); } public void testNode() throws InterruptedException, ExecutionException { @@ -41,7 +53,7 @@ public CompletableFuture execute(List data) { public String getName() { return "test"; } - }, WorkflowData.EMPTY, Collections.emptyList()); + }, WorkflowData.EMPTY, Collections.emptyList(), executor); assertEquals("A", nodeA.id()); assertEquals("test", nodeA.workflowStep().getName()); assertEquals(WorkflowData.EMPTY, nodeA.input()); @@ -50,10 +62,10 @@ public String getName() { // TODO: This test is flaky on Windows. Disabling until thread pool is integrated // https://github.com/opensearch-project/opensearch-ai-flow-framework/issues/42 - // CompletableFuture f = nodeA.execute(); - // assertEquals(f, nodeA.future()); - // f.orTimeout(5, TimeUnit.SECONDS); - // assertTrue(f.isDone()); - // assertEquals(WorkflowData.EMPTY, f.get()); + CompletableFuture f = nodeA.execute(); + assertEquals(f, nodeA.future()); + f.orTimeout(15, TimeUnit.SECONDS); + assertTrue(f.isDone()); + assertEquals(WorkflowData.EMPTY, f.get()); } } diff --git a/src/test/java/org/opensearch/flowframework/template/WorkflowProcessSorterTests.java b/src/test/java/org/opensearch/flowframework/template/WorkflowProcessSorterTests.java index 537fedbc7..ac96fa1cc 100644 --- a/src/test/java/org/opensearch/flowframework/template/WorkflowProcessSorterTests.java +++ b/src/test/java/org/opensearch/flowframework/template/WorkflowProcessSorterTests.java @@ -10,11 +10,16 @@ import org.opensearch.core.xcontent.XContentParser; import org.opensearch.flowframework.workflow.Workflow; +import org.opensearch.flowframework.workflow.WorkflowStepFactory; import org.opensearch.test.OpenSearchTestCase; +import org.junit.AfterClass; +import org.junit.BeforeClass; import java.io.IOException; import java.util.Collections; import java.util.List; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.stream.Collectors; import static org.opensearch.flowframework.template.TemplateTestJsonUtil.edge; @@ -31,12 +36,22 @@ public class WorkflowProcessSorterTests extends OpenSearchTestCase { private static List parse(String json) throws IOException { XContentParser parser = TemplateTestJsonUtil.jsonToParser(json); Workflow w = Workflow.parse(parser); - return WorkflowProcessSorter.sortProcessNodes(w).stream().map(ProcessNode::id).collect(Collectors.toList()); + return workflowProcessSorter.sortProcessNodes(w).stream().map(ProcessNode::id).collect(Collectors.toList()); } - @Override - public void setUp() throws Exception { - super.setUp(); + private static ExecutorService executor; + private static WorkflowProcessSorter workflowProcessSorter; + + @BeforeClass + public static void setup() { + executor = Executors.newFixedThreadPool(10); + WorkflowStepFactory factory = WorkflowStepFactory.create(null); + workflowProcessSorter = WorkflowProcessSorter.create(factory, executor); + } + + @AfterClass + public static void cleanup() { + executor.shutdown(); } public void testOrdering() throws IOException { From 3fa4d0ab05a1b82f1c34e765fbe9206a92ec229c Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Mon, 25 Sep 2023 18:13:41 -0700 Subject: [PATCH 17/27] Fix flaky ProcessNodeTests by removing orTimeout Signed-off-by: Daniel Widdis --- .../template/ProcessNodeTests.java | 23 ++++++------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/src/test/java/org/opensearch/flowframework/template/ProcessNodeTests.java b/src/test/java/org/opensearch/flowframework/template/ProcessNodeTests.java index 2e7c6168c..b3e69253c 100644 --- a/src/test/java/org/opensearch/flowframework/template/ProcessNodeTests.java +++ b/src/test/java/org/opensearch/flowframework/template/ProcessNodeTests.java @@ -8,14 +8,11 @@ */ package org.opensearch.flowframework.template; -import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; -import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope.Scope; - import org.opensearch.flowframework.workflow.WorkflowData; import org.opensearch.flowframework.workflow.WorkflowStep; import org.opensearch.test.OpenSearchTestCase; -import org.junit.AfterClass; -import org.junit.BeforeClass; +import org.junit.After; +import org.junit.Before; import java.util.Collections; import java.util.List; @@ -23,20 +20,18 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; -import java.util.concurrent.TimeUnit; -@ThreadLeakScope(Scope.NONE) public class ProcessNodeTests extends OpenSearchTestCase { - private static ExecutorService executor; + private ExecutorService executor; - @BeforeClass - public static void setup() { + @Before + public void setup() { executor = Executors.newFixedThreadPool(10); } - @AfterClass - public static void cleanup() { + @After + public void cleanup() { executor.shutdown(); } @@ -60,12 +55,8 @@ public String getName() { assertEquals(Collections.emptyList(), nodeA.predecessors()); assertEquals("A", nodeA.toString()); - // TODO: This test is flaky on Windows. Disabling until thread pool is integrated - // https://github.com/opensearch-project/opensearch-ai-flow-framework/issues/42 CompletableFuture f = nodeA.execute(); assertEquals(f, nodeA.future()); - f.orTimeout(15, TimeUnit.SECONDS); - assertTrue(f.isDone()); assertEquals(WorkflowData.EMPTY, f.get()); } } From 62fa53c12e35483f7bb5eff23d6b14f7d10a67e5 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Mon, 25 Sep 2023 18:42:47 -0700 Subject: [PATCH 18/27] Rebase and refactor with #44 Signed-off-by: Daniel Widdis --- .../flowframework/FlowFrameworkPlugin.java | 5 ----- .../{CreateIndex => }/CreateIndexStep.java | 7 ++----- .../workflow/CreateIngestPipelineStep.java | 4 +++- .../workflow/WorkflowStepFactory.java | 21 +++++++------------ .../template/WorkflowProcessSorterTests.java | 10 ++++++++- .../CreateIndexStepTests.java | 3 +-- .../CreateIngestPipelineStepTests.java | 4 +--- src/test/resources/template/demo.json | 8 +++---- 8 files changed, 27 insertions(+), 35 deletions(-) rename src/main/java/org/opensearch/flowframework/workflow/{CreateIndex => }/CreateIndexStep.java (92%) rename src/test/java/org/opensearch/flowframework/workflow/{CreateIndex => }/CreateIndexStepTests.java (96%) rename src/test/java/org/opensearch/flowframework/workflow/{CreateIngestPipeline => }/CreateIngestPipelineStepTests.java (96%) diff --git a/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java b/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java index 5f0429422..e04cfd799 100644 --- a/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java +++ b/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java @@ -16,13 +16,8 @@ import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.env.Environment; import org.opensearch.env.NodeEnvironment; -<<<<<<< HEAD -import org.opensearch.flowframework.workflow.CreateIndex.CreateIndexStep; -import org.opensearch.flowframework.workflow.CreateIngestPipelineStep; -======= import org.opensearch.flowframework.template.WorkflowProcessSorter; import org.opensearch.flowframework.workflow.WorkflowStepFactory; ->>>>>>> abffd2d (Integrate thread pool executor service) import org.opensearch.plugins.Plugin; import org.opensearch.repositories.RepositoriesService; import org.opensearch.script.ScriptService; diff --git a/src/main/java/org/opensearch/flowframework/workflow/CreateIndex/CreateIndexStep.java b/src/main/java/org/opensearch/flowframework/workflow/CreateIndexStep.java similarity index 92% rename from src/main/java/org/opensearch/flowframework/workflow/CreateIndex/CreateIndexStep.java rename to src/main/java/org/opensearch/flowframework/workflow/CreateIndexStep.java index 12675b445..ac32d0338 100644 --- a/src/main/java/org/opensearch/flowframework/workflow/CreateIndex/CreateIndexStep.java +++ b/src/main/java/org/opensearch/flowframework/workflow/CreateIndexStep.java @@ -6,7 +6,7 @@ * this file be licensed under the Apache-2.0 license or a * compatible open source license. */ -package org.opensearch.flowframework.workflow.CreateIndex; +package org.opensearch.flowframework.workflow; import com.google.common.base.Charsets; import com.google.common.io.Resources; @@ -18,9 +18,6 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.action.ActionListener; -import org.opensearch.flowframework.workflow.WorkflowData; -import org.opensearch.flowframework.workflow.WorkflowStep; -import org.opensearch.flowframework.workflow.WorkflowStepFactory; import java.io.IOException; import java.net.URL; @@ -37,7 +34,7 @@ public class CreateIndexStep implements WorkflowStep { private Client client; /** The name of this step, used as a key in the template and the {@link WorkflowStepFactory} */ - public static final String NAME = "create_index_step"; + static final String NAME = "create_index"; /** * Instantiate this class diff --git a/src/main/java/org/opensearch/flowframework/workflow/CreateIngestPipelineStep.java b/src/main/java/org/opensearch/flowframework/workflow/CreateIngestPipelineStep.java index 8382925b2..4770b94a9 100644 --- a/src/main/java/org/opensearch/flowframework/workflow/CreateIngestPipelineStep.java +++ b/src/main/java/org/opensearch/flowframework/workflow/CreateIngestPipelineStep.java @@ -32,7 +32,9 @@ public class CreateIngestPipelineStep implements WorkflowStep { private static final Logger logger = LogManager.getLogger(CreateIngestPipelineStep.class); - private static final String NAME = "create_ingest_pipeline_step"; + + /** The name of this step, used as a key in the template and the {@link WorkflowStepFactory} */ + static final String NAME = "create_ingest_pipeline"; // Common pipeline configuration fields private static final String PIPELINE_ID_FIELD = "id"; diff --git a/src/main/java/org/opensearch/flowframework/workflow/WorkflowStepFactory.java b/src/main/java/org/opensearch/flowframework/workflow/WorkflowStepFactory.java index cd203f8de..dc0dc29a2 100644 --- a/src/main/java/org/opensearch/flowframework/workflow/WorkflowStepFactory.java +++ b/src/main/java/org/opensearch/flowframework/workflow/WorkflowStepFactory.java @@ -9,7 +9,6 @@ package org.opensearch.flowframework.workflow; import org.opensearch.client.Client; -import org.opensearch.flowframework.workflow.CreateIndex.CreateIndexStep; import java.util.HashMap; import java.util.List; @@ -25,7 +24,6 @@ public class WorkflowStepFactory { private static WorkflowStepFactory instance = null; - private Client client; private final Map stepMap = new HashMap<>(); /** @@ -55,23 +53,18 @@ public static synchronized WorkflowStepFactory get() { } private WorkflowStepFactory(Client client) { - this.client = client; - populateMap(); + populateMap(client); } - private void populateMap() { + private void populateMap(Client client) { stepMap.put(CreateIndexStep.NAME, new CreateIndexStep(client)); + stepMap.put(CreateIngestPipelineStep.NAME, new CreateIngestPipelineStep(client)); - // TODO: These are from the demo class as placeholders - // Replace with actual implementations such as - // https://github.com/opensearch-project/opensearch-ai-flow-framework/pull/38 - // https://github.com/opensearch-project/opensearch-ai-flow-framework/pull/44 - stepMap.put("fetch_model", new DemoWorkflowStep(3000)); - stepMap.put("create_ingest_pipeline", new DemoWorkflowStep(3000)); - stepMap.put("create_search_pipeline", new DemoWorkflowStep(5000)); - stepMap.put("create_neural_search_index", new DemoWorkflowStep(2000)); + // TODO: These are from the demo class as placeholders, remove when demos are deleted + stepMap.put("demo_delay_3", new DemoWorkflowStep(3000)); + stepMap.put("demo_delay_5", new DemoWorkflowStep(3000)); - // Use until all the actual implementations are ready + // Use as a default until all the actual implementations are ready stepMap.put("placeholder", new WorkflowStep() { @Override public CompletableFuture execute(List data) { diff --git a/src/test/java/org/opensearch/flowframework/template/WorkflowProcessSorterTests.java b/src/test/java/org/opensearch/flowframework/template/WorkflowProcessSorterTests.java index ac96fa1cc..bad7f77de 100644 --- a/src/test/java/org/opensearch/flowframework/template/WorkflowProcessSorterTests.java +++ b/src/test/java/org/opensearch/flowframework/template/WorkflowProcessSorterTests.java @@ -8,6 +8,8 @@ */ package org.opensearch.flowframework.template; +import org.opensearch.client.AdminClient; +import org.opensearch.client.Client; import org.opensearch.core.xcontent.XContentParser; import org.opensearch.flowframework.workflow.Workflow; import org.opensearch.flowframework.workflow.WorkflowStepFactory; @@ -25,6 +27,8 @@ import static org.opensearch.flowframework.template.TemplateTestJsonUtil.edge; import static org.opensearch.flowframework.template.TemplateTestJsonUtil.node; import static org.opensearch.flowframework.template.TemplateTestJsonUtil.workflow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class WorkflowProcessSorterTests extends OpenSearchTestCase { @@ -44,8 +48,12 @@ private static List parse(String json) throws IOException { @BeforeClass public static void setup() { + AdminClient adminClient = mock(AdminClient.class); + Client client = mock(Client.class); + when(client.admin()).thenReturn(adminClient); + executor = Executors.newFixedThreadPool(10); - WorkflowStepFactory factory = WorkflowStepFactory.create(null); + WorkflowStepFactory factory = WorkflowStepFactory.create(client); workflowProcessSorter = WorkflowProcessSorter.create(factory, executor); } diff --git a/src/test/java/org/opensearch/flowframework/workflow/CreateIndex/CreateIndexStepTests.java b/src/test/java/org/opensearch/flowframework/workflow/CreateIndexStepTests.java similarity index 96% rename from src/test/java/org/opensearch/flowframework/workflow/CreateIndex/CreateIndexStepTests.java rename to src/test/java/org/opensearch/flowframework/workflow/CreateIndexStepTests.java index c5d680a94..cf77c54b2 100644 --- a/src/test/java/org/opensearch/flowframework/workflow/CreateIndex/CreateIndexStepTests.java +++ b/src/test/java/org/opensearch/flowframework/workflow/CreateIndexStepTests.java @@ -6,7 +6,7 @@ * this file be licensed under the Apache-2.0 license or a * compatible open source license. */ -package org.opensearch.flowframework.workflow.CreateIndex; +package org.opensearch.flowframework.workflow; import org.opensearch.action.admin.indices.create.CreateIndexRequest; import org.opensearch.action.admin.indices.create.CreateIndexResponse; @@ -14,7 +14,6 @@ import org.opensearch.client.Client; import org.opensearch.client.IndicesAdminClient; import org.opensearch.core.action.ActionListener; -import org.opensearch.flowframework.workflow.WorkflowData; import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; diff --git a/src/test/java/org/opensearch/flowframework/workflow/CreateIngestPipeline/CreateIngestPipelineStepTests.java b/src/test/java/org/opensearch/flowframework/workflow/CreateIngestPipelineStepTests.java similarity index 96% rename from src/test/java/org/opensearch/flowframework/workflow/CreateIngestPipeline/CreateIngestPipelineStepTests.java rename to src/test/java/org/opensearch/flowframework/workflow/CreateIngestPipelineStepTests.java index 286bc2de9..15b8b5786 100644 --- a/src/test/java/org/opensearch/flowframework/workflow/CreateIngestPipeline/CreateIngestPipelineStepTests.java +++ b/src/test/java/org/opensearch/flowframework/workflow/CreateIngestPipelineStepTests.java @@ -6,7 +6,7 @@ * this file be licensed under the Apache-2.0 license or a * compatible open source license. */ -package org.opensearch.flowframework.workflow.CreateIngestPipeline; +package org.opensearch.flowframework.workflow; import org.opensearch.action.ingest.PutPipelineRequest; import org.opensearch.action.support.master.AcknowledgedResponse; @@ -14,8 +14,6 @@ import org.opensearch.client.Client; import org.opensearch.client.ClusterAdminClient; import org.opensearch.core.action.ActionListener; -import org.opensearch.flowframework.workflow.CreateIngestPipelineStep; -import org.opensearch.flowframework.workflow.WorkflowData; import org.opensearch.test.OpenSearchTestCase; import java.util.List; diff --git a/src/test/resources/template/demo.json b/src/test/resources/template/demo.json index b3428f0ff..d30f8a7f8 100644 --- a/src/test/resources/template/demo.json +++ b/src/test/resources/template/demo.json @@ -9,19 +9,19 @@ "nodes": [ { "id": "fetch_model", - "type": "fetch_model" + "type": "demo_delay_3" }, { "id": "create_ingest_pipeline", - "type": "create_ingest_pipeline" + "type": "demo_delay_3" }, { "id": "create_search_pipeline", - "type": "create_search_pipeline" + "type": "demo_delay_5" }, { "id": "create_neural_search_index", - "type": "create_neural_search_index" + "type": "demo_delay_3" } ], "edges": [ From 6c86dd1cd0c6b95b3f22a168eb001ded8378088e Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Mon, 25 Sep 2023 19:10:23 -0700 Subject: [PATCH 19/27] Fix demos and remove DataDemo Signed-off-by: Daniel Widdis --- src/main/java/demo/DataDemo.java | 82 ----------------------- src/main/java/demo/Demo.java | 5 +- src/main/java/demo/TemplateParseDemo.java | 5 +- src/test/resources/template/demo.json | 27 -------- 4 files changed, 8 insertions(+), 111 deletions(-) delete mode 100644 src/main/java/demo/DataDemo.java diff --git a/src/main/java/demo/DataDemo.java b/src/main/java/demo/DataDemo.java deleted file mode 100644 index 36c27efb6..000000000 --- a/src/main/java/demo/DataDemo.java +++ /dev/null @@ -1,82 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ -package demo; - -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.opensearch.common.SuppressForbidden; -import org.opensearch.common.io.PathUtils; -import org.opensearch.flowframework.template.ProcessNode; -import org.opensearch.flowframework.template.Template; -import org.opensearch.flowframework.template.WorkflowProcessSorter; -import org.opensearch.flowframework.workflow.WorkflowStepFactory; - -import java.io.IOException; -import java.nio.charset.StandardCharsets; -import java.nio.file.Files; -import java.util.ArrayList; -import java.util.List; -import java.util.Locale; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.stream.Collectors; - -/** - * Demo class exercising {@link WorkflowProcessSorter}. This will be moved to a unit test. - */ -public class DataDemo { - - private static final Logger logger = LogManager.getLogger(DataDemo.class); - - /** - * Demonstrate parsing a JSON graph. - * - * @param args unused - * @throws IOException on a failure - */ - @SuppressForbidden(reason = "just a demo class that will be deleted") - public static void main(String[] args) throws IOException { - String path = "src/test/resources/template/demo.json"; - String json; - try { - json = new String(Files.readAllBytes(PathUtils.get(path)), StandardCharsets.UTF_8); - } catch (IOException e) { - logger.error("Failed to read JSON at path {}", path); - return; - } - WorkflowStepFactory factory = WorkflowStepFactory.create(null); - ExecutorService executor = Executors.newFixedThreadPool(10); - WorkflowProcessSorter.create(factory, executor); - - logger.info("Parsing graph to sequence..."); - Template t = Template.parse(json); - List processSequence = WorkflowProcessSorter.get().sortProcessNodes(t.workflows().get("datademo")); - List> futureList = new ArrayList<>(); - - for (ProcessNode n : processSequence) { - List predecessors = n.predecessors(); - logger.info( - "Queueing process [{}].{}", - n.id(), - predecessors.isEmpty() - ? " Can start immediately!" - : String.format( - Locale.getDefault(), - " Must wait for [%s] to complete first.", - predecessors.stream().map(p -> p.id()).collect(Collectors.joining(", ")) - ) - ); - futureList.add(n.execute()); - } - futureList.forEach(CompletableFuture::join); - logger.info("All done!"); - executor.shutdown(); - } -} diff --git a/src/main/java/demo/Demo.java b/src/main/java/demo/Demo.java index ab709089d..d75cf3fba 100644 --- a/src/main/java/demo/Demo.java +++ b/src/main/java/demo/Demo.java @@ -10,6 +10,8 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.opensearch.client.Client; +import org.opensearch.client.node.NodeClient; import org.opensearch.common.SuppressForbidden; import org.opensearch.common.io.PathUtils; import org.opensearch.flowframework.template.ProcessNode; @@ -51,7 +53,8 @@ public static void main(String[] args) throws IOException { logger.error("Failed to read JSON at path {}", path); return; } - WorkflowStepFactory factory = WorkflowStepFactory.create(null); + Client client = new NodeClient(null, null); + WorkflowStepFactory factory = WorkflowStepFactory.create(client); ExecutorService executor = Executors.newFixedThreadPool(10); WorkflowProcessSorter.create(factory, executor); diff --git a/src/main/java/demo/TemplateParseDemo.java b/src/main/java/demo/TemplateParseDemo.java index 719e5bf89..4b9a67e3b 100644 --- a/src/main/java/demo/TemplateParseDemo.java +++ b/src/main/java/demo/TemplateParseDemo.java @@ -10,6 +10,8 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.opensearch.client.Client; +import org.opensearch.client.node.NodeClient; import org.opensearch.common.SuppressForbidden; import org.opensearch.common.io.PathUtils; import org.opensearch.flowframework.template.Template; @@ -46,7 +48,8 @@ public static void main(String[] args) throws IOException { logger.error("Failed to read JSON at path {}", path); return; } - WorkflowStepFactory factory = WorkflowStepFactory.create(null); + Client client = new NodeClient(null, null); + WorkflowStepFactory factory = WorkflowStepFactory.create(client); WorkflowProcessSorter.create(factory, Executors.newFixedThreadPool(10)); Template t = Template.parse(json); diff --git a/src/test/resources/template/demo.json b/src/test/resources/template/demo.json index d30f8a7f8..e27158bff 100644 --- a/src/test/resources/template/demo.json +++ b/src/test/resources/template/demo.json @@ -42,33 +42,6 @@ "dest": "create_neural_search_index" } ] - }, - "datademo": { - "user_params": { - "workflow_id": "workflow_demo" - }, - "nodes": [ - { - "id": "create_index", - "type": "create_index", - "inputs": { - "index_name": "demo" - } - }, - { - "id": "create_another_index", - "type": "create_index", - "inputs": { - "index_name": "second_demo" - } - } - ], - "edges": [ - { - "source": "create_index", - "dest": "create_another_index" - } - ] } } } From c11a5bf8e1be30e720d165502670ff1ec588468a Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Mon, 25 Sep 2023 19:18:28 -0700 Subject: [PATCH 20/27] Use non-deprecated mapping method for CreateIndexStep Signed-off-by: Daniel Widdis --- .../opensearch/flowframework/workflow/CreateIndexStep.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/opensearch/flowframework/workflow/CreateIndexStep.java b/src/main/java/org/opensearch/flowframework/workflow/CreateIndexStep.java index ac32d0338..1f0d074c2 100644 --- a/src/main/java/org/opensearch/flowframework/workflow/CreateIndexStep.java +++ b/src/main/java/org/opensearch/flowframework/workflow/CreateIndexStep.java @@ -16,7 +16,7 @@ import org.opensearch.action.admin.indices.create.CreateIndexResponse; import org.opensearch.client.Client; import org.opensearch.common.settings.Settings; -import org.opensearch.common.xcontent.XContentType; +import org.opensearch.common.xcontent.json.JsonXContent; import org.opensearch.core.action.ActionListener; import java.io.IOException; @@ -79,10 +79,9 @@ public void onFailure(Exception e) { // 1. Create settings based on the index settings received from content try { - // TODO: mapping() is deprecated CreateIndexRequest request = new CreateIndexRequest(index).mapping( getIndexMappings("mappings/" + type + ".json"), - XContentType.JSON + JsonXContent.jsonXContent.mediaType() ); client.admin().indices().create(request, actionListener); } catch (Exception e) { From adc349a83ea8eee51901aef386b17e42a1df1b8c Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Mon, 25 Sep 2023 19:32:23 -0700 Subject: [PATCH 21/27] Eliminate casting and deprecation warnings on test classes Signed-off-by: Daniel Widdis --- .../flowframework/workflow/CreateIndexStepTests.java | 6 ++++-- .../workflow/CreateIngestPipelineStepTests.java | 7 +++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/test/java/org/opensearch/flowframework/workflow/CreateIndexStepTests.java b/src/test/java/org/opensearch/flowframework/workflow/CreateIndexStepTests.java index cf77c54b2..0fdc05cbd 100644 --- a/src/test/java/org/opensearch/flowframework/workflow/CreateIndexStepTests.java +++ b/src/test/java/org/opensearch/flowframework/workflow/CreateIndexStepTests.java @@ -58,7 +58,8 @@ public void testCreateIndexStep() throws ExecutionException, InterruptedExceptio CreateIndexStep createIndexStep = new CreateIndexStep(client); - ArgumentCaptor actionListenerCaptor = ArgumentCaptor.forClass(ActionListener.class); + @SuppressWarnings("unchecked") + ArgumentCaptor> actionListenerCaptor = ArgumentCaptor.forClass(ActionListener.class); CompletableFuture future = createIndexStep.execute(List.of(inputData)); assertFalse(future.isDone()); verify(indicesAdminClient, times(1)).create(any(CreateIndexRequest.class), actionListenerCaptor.capture()); @@ -75,7 +76,8 @@ public void testCreateIndexStepFailure() throws ExecutionException, InterruptedE CreateIndexStep createIndexStep = new CreateIndexStep(client); - ArgumentCaptor actionListenerCaptor = ArgumentCaptor.forClass(ActionListener.class); + @SuppressWarnings("unchecked") + ArgumentCaptor> actionListenerCaptor = ArgumentCaptor.forClass(ActionListener.class); CompletableFuture future = createIndexStep.execute(List.of(inputData)); assertFalse(future.isDone()); verify(indicesAdminClient, times(1)).create(any(CreateIndexRequest.class), actionListenerCaptor.capture()); diff --git a/src/test/java/org/opensearch/flowframework/workflow/CreateIngestPipelineStepTests.java b/src/test/java/org/opensearch/flowframework/workflow/CreateIngestPipelineStepTests.java index 15b8b5786..9dab2a8d7 100644 --- a/src/test/java/org/opensearch/flowframework/workflow/CreateIngestPipelineStepTests.java +++ b/src/test/java/org/opensearch/flowframework/workflow/CreateIngestPipelineStepTests.java @@ -29,6 +29,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +@SuppressWarnings("deprecation") public class CreateIngestPipelineStepTests extends OpenSearchTestCase { private WorkflowData inputData; @@ -67,7 +68,8 @@ public void testCreateIngestPipelineStep() throws InterruptedException, Executio CreateIngestPipelineStep createIngestPipelineStep = new CreateIngestPipelineStep(client); - ArgumentCaptor actionListenerCaptor = ArgumentCaptor.forClass(ActionListener.class); + @SuppressWarnings("unchecked") + ArgumentCaptor> actionListenerCaptor = ArgumentCaptor.forClass(ActionListener.class); CompletableFuture future = createIngestPipelineStep.execute(List.of(inputData)); assertFalse(future.isDone()); @@ -84,7 +86,8 @@ public void testCreateIngestPipelineStepFailure() throws InterruptedException { CreateIngestPipelineStep createIngestPipelineStep = new CreateIngestPipelineStep(client); - ArgumentCaptor actionListenerCaptor = ArgumentCaptor.forClass(ActionListener.class); + @SuppressWarnings("unchecked") + ArgumentCaptor> actionListenerCaptor = ArgumentCaptor.forClass(ActionListener.class); CompletableFuture future = createIngestPipelineStep.execute(List.of(inputData)); assertFalse(future.isDone()); From 21b530b390f9fe7a9a142c55332dc8c3c03239f9 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Mon, 25 Sep 2023 19:38:00 -0700 Subject: [PATCH 22/27] Remove unused/leftover demo class Signed-off-by: Daniel Widdis --- .../java/demo/CreateIndexWorkflowStep.java | 74 ------------------- 1 file changed, 74 deletions(-) delete mode 100644 src/main/java/demo/CreateIndexWorkflowStep.java diff --git a/src/main/java/demo/CreateIndexWorkflowStep.java b/src/main/java/demo/CreateIndexWorkflowStep.java deleted file mode 100644 index 21f1e27c2..000000000 --- a/src/main/java/demo/CreateIndexWorkflowStep.java +++ /dev/null @@ -1,74 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ -package demo; - -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.opensearch.action.admin.indices.create.CreateIndexResponse; -import org.opensearch.flowframework.workflow.WorkflowData; -import org.opensearch.flowframework.workflow.WorkflowStep; - -import java.util.List; -import java.util.Map; -import java.util.concurrent.CompletableFuture; - -/** - * Sample to show other devs how to pass data around. Will be deleted once other PRs are merged. - */ -public class CreateIndexWorkflowStep implements WorkflowStep { - - private static final Logger logger = LogManager.getLogger(CreateIndexWorkflowStep.class); - - private final String name; - - /** - * Instantiate this class. - */ - public CreateIndexWorkflowStep() { - this.name = "CREATE_INDEX"; - } - - @Override - public CompletableFuture execute(List data) { - CompletableFuture future = new CompletableFuture<>(); - // TODO we will be passing a thread pool to this object when it's instantiated - // we should either add the generic executor from that pool to this call - // or use executorservice.submit or any of various threading options - // https://github.com/opensearch-project/opensearch-ai-flow-framework/issues/42 - CompletableFuture.runAsync(() -> { - String inputIndex = null; - for (WorkflowData wfData : data) { - logger.debug( - "{} sent params: {}, content: {}", - inputIndex == null ? "Initialization" : "Previous step", - wfData.getParams(), - wfData.getContent() - ); - if (inputIndex == null) { - inputIndex = wfData.getParams() - .getOrDefault("index_name", (String) wfData.getContent().getOrDefault("index_name", "NOT FOUND")); - } - } - // do some work, simulating a REST API call - try { - Thread.sleep(2000); - } catch (InterruptedException e) {} - // Simulate response of created index - CreateIndexResponse response = new CreateIndexResponse(true, true, inputIndex); - future.complete(new WorkflowData(Map.of("index", response.index()))); - }); - - return future; - } - - @Override - public String getName() { - return name; - } -} From 8d28308a8f0f52465d26f8ae502d22a84d780696 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Tue, 26 Sep 2023 10:33:57 -0700 Subject: [PATCH 23/27] Typo Signed-off-by: Daniel Widdis --- .../java/org/opensearch/flowframework/template/Template.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/opensearch/flowframework/template/Template.java b/src/main/java/org/opensearch/flowframework/template/Template.java index 327a508b5..0c8e296fa 100644 --- a/src/main/java/org/opensearch/flowframework/template/Template.java +++ b/src/main/java/org/opensearch/flowframework/template/Template.java @@ -137,7 +137,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } /** - * Parse raw json content into a workflow node instance. + * Parse raw json content into a Template instance. * * @param parser json based content parser * @return an instance of the template From 555d88b67b702d6d2f364e798418660d4a8b2c4c Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Tue, 26 Sep 2023 10:56:07 -0700 Subject: [PATCH 24/27] Don't offer steps as an alternative to nodes Signed-off-by: Daniel Widdis --- .../java/org/opensearch/flowframework/workflow/Workflow.java | 3 --- src/test/resources/template/finaltemplate.json | 4 ++-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/opensearch/flowframework/workflow/Workflow.java b/src/main/java/org/opensearch/flowframework/workflow/Workflow.java index f0bddc6de..82a365231 100644 --- a/src/main/java/org/opensearch/flowframework/workflow/Workflow.java +++ b/src/main/java/org/opensearch/flowframework/workflow/Workflow.java @@ -33,8 +33,6 @@ public class Workflow implements ToXContentObject { public static final String USER_PARAMS_FIELD = "user_params"; /** The template field name for workflow nodes */ public static final String NODES_FIELD = "nodes"; - /** The template field name for workflow steps, an alternative name for nodes */ - public static final String STEPS_FIELD = "steps"; /** The template field name for workflow edges */ public static final String EDGES_FIELD = "edges"; @@ -106,7 +104,6 @@ public static Workflow parse(XContentParser parser) throws IOException { } break; case NODES_FIELD: - case STEPS_FIELD: ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.currentToken(), parser); while (parser.nextToken() != XContentParser.Token.END_ARRAY) { nodes.add(WorkflowNode.parse(parser)); diff --git a/src/test/resources/template/finaltemplate.json b/src/test/resources/template/finaltemplate.json index 82affd97f..65be77758 100644 --- a/src/test/resources/template/finaltemplate.json +++ b/src/test/resources/template/finaltemplate.json @@ -20,7 +20,7 @@ }, "workflows": { "provision": { - "steps": [{ + "nodes": [{ "id": "create_index", "type": "create_index", "inputs": { @@ -52,7 +52,7 @@ "user_params": { "document": "doc" }, - "steps": [{ + "nodes": [{ "id": "ingest_index", "type": "ingest_index", "inputs": { From 5c7804689989e0907c03125922fb82267ad00b34 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Tue, 26 Sep 2023 12:33:31 -0700 Subject: [PATCH 25/27] Move Workflow into package with all the other parsing classes Signed-off-by: Daniel Widdis --- src/main/java/demo/TemplateParseDemo.java | 2 +- .../org/opensearch/flowframework/template/Template.java | 1 - .../flowframework/{workflow => template}/Workflow.java | 6 ++---- .../flowframework/template/WorkflowProcessSorter.java | 1 - .../flowframework/template/TemplateTestJsonUtil.java | 1 - .../opensearch/flowframework/template/TemplateTests.java | 1 - .../flowframework/template/WorkflowProcessSorterTests.java | 1 - .../opensearch/flowframework/template/WorkflowTests.java | 1 - 8 files changed, 3 insertions(+), 11 deletions(-) rename src/main/java/org/opensearch/flowframework/{workflow => template}/Workflow.java (96%) diff --git a/src/main/java/demo/TemplateParseDemo.java b/src/main/java/demo/TemplateParseDemo.java index 4b9a67e3b..191e98630 100644 --- a/src/main/java/demo/TemplateParseDemo.java +++ b/src/main/java/demo/TemplateParseDemo.java @@ -15,8 +15,8 @@ import org.opensearch.common.SuppressForbidden; import org.opensearch.common.io.PathUtils; import org.opensearch.flowframework.template.Template; +import org.opensearch.flowframework.template.Workflow; import org.opensearch.flowframework.template.WorkflowProcessSorter; -import org.opensearch.flowframework.workflow.Workflow; import org.opensearch.flowframework.workflow.WorkflowStepFactory; import java.io.IOException; diff --git a/src/main/java/org/opensearch/flowframework/template/Template.java b/src/main/java/org/opensearch/flowframework/template/Template.java index 0c8e296fa..6b7035465 100644 --- a/src/main/java/org/opensearch/flowframework/template/Template.java +++ b/src/main/java/org/opensearch/flowframework/template/Template.java @@ -16,7 +16,6 @@ import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; -import org.opensearch.flowframework.workflow.Workflow; import java.io.IOException; import java.util.ArrayList; diff --git a/src/main/java/org/opensearch/flowframework/workflow/Workflow.java b/src/main/java/org/opensearch/flowframework/template/Workflow.java similarity index 96% rename from src/main/java/org/opensearch/flowframework/workflow/Workflow.java rename to src/main/java/org/opensearch/flowframework/template/Workflow.java index 82a365231..f1a7071b6 100644 --- a/src/main/java/org/opensearch/flowframework/workflow/Workflow.java +++ b/src/main/java/org/opensearch/flowframework/template/Workflow.java @@ -6,14 +6,12 @@ * this file be licensed under the Apache-2.0 license or a * compatible open source license. */ -package org.opensearch.flowframework.workflow; +package org.opensearch.flowframework.template; import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; -import org.opensearch.flowframework.template.Template; -import org.opensearch.flowframework.template.WorkflowEdge; -import org.opensearch.flowframework.template.WorkflowNode; +import org.opensearch.flowframework.workflow.WorkflowData; import java.io.IOException; import java.util.ArrayList; diff --git a/src/main/java/org/opensearch/flowframework/template/WorkflowProcessSorter.java b/src/main/java/org/opensearch/flowframework/template/WorkflowProcessSorter.java index aaf200fff..c05800e45 100644 --- a/src/main/java/org/opensearch/flowframework/template/WorkflowProcessSorter.java +++ b/src/main/java/org/opensearch/flowframework/template/WorkflowProcessSorter.java @@ -10,7 +10,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.opensearch.flowframework.workflow.Workflow; import org.opensearch.flowframework.workflow.WorkflowData; import org.opensearch.flowframework.workflow.WorkflowStep; import org.opensearch.flowframework.workflow.WorkflowStepFactory; diff --git a/src/test/java/org/opensearch/flowframework/template/TemplateTestJsonUtil.java b/src/test/java/org/opensearch/flowframework/template/TemplateTestJsonUtil.java index bd59fd61b..e0e619f9e 100644 --- a/src/test/java/org/opensearch/flowframework/template/TemplateTestJsonUtil.java +++ b/src/test/java/org/opensearch/flowframework/template/TemplateTestJsonUtil.java @@ -14,7 +14,6 @@ import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentParser; -import org.opensearch.flowframework.workflow.Workflow; import java.io.IOException; import java.util.List; diff --git a/src/test/java/org/opensearch/flowframework/template/TemplateTests.java b/src/test/java/org/opensearch/flowframework/template/TemplateTests.java index 70d341152..a00a6d4c9 100644 --- a/src/test/java/org/opensearch/flowframework/template/TemplateTests.java +++ b/src/test/java/org/opensearch/flowframework/template/TemplateTests.java @@ -9,7 +9,6 @@ package org.opensearch.flowframework.template; import org.opensearch.Version; -import org.opensearch.flowframework.workflow.Workflow; import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; diff --git a/src/test/java/org/opensearch/flowframework/template/WorkflowProcessSorterTests.java b/src/test/java/org/opensearch/flowframework/template/WorkflowProcessSorterTests.java index bad7f77de..8413b9f94 100644 --- a/src/test/java/org/opensearch/flowframework/template/WorkflowProcessSorterTests.java +++ b/src/test/java/org/opensearch/flowframework/template/WorkflowProcessSorterTests.java @@ -11,7 +11,6 @@ import org.opensearch.client.AdminClient; import org.opensearch.client.Client; import org.opensearch.core.xcontent.XContentParser; -import org.opensearch.flowframework.workflow.Workflow; import org.opensearch.flowframework.workflow.WorkflowStepFactory; import org.opensearch.test.OpenSearchTestCase; import org.junit.AfterClass; diff --git a/src/test/java/org/opensearch/flowframework/template/WorkflowTests.java b/src/test/java/org/opensearch/flowframework/template/WorkflowTests.java index ee149ecae..94a68455e 100644 --- a/src/test/java/org/opensearch/flowframework/template/WorkflowTests.java +++ b/src/test/java/org/opensearch/flowframework/template/WorkflowTests.java @@ -9,7 +9,6 @@ package org.opensearch.flowframework.template; import org.opensearch.core.xcontent.XContentParser; -import org.opensearch.flowframework.workflow.Workflow; import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; From 87685ffb6a56d5dc68286c1423b5086d8d658b8a Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Tue, 26 Sep 2023 12:35:46 -0700 Subject: [PATCH 26/27] Move process sequencing classes into workflow package Signed-off-by: Daniel Widdis --- src/main/java/demo/Demo.java | 4 ++-- src/main/java/demo/TemplateParseDemo.java | 2 +- .../org/opensearch/flowframework/FlowFrameworkPlugin.java | 2 +- .../opensearch/flowframework/template/WorkflowNode.java | 1 + .../flowframework/{template => workflow}/ProcessNode.java | 4 +--- .../{template => workflow}/WorkflowProcessSorter.java | 8 ++++---- .../{template => workflow}/ProcessNodeTests.java | 4 +--- .../WorkflowProcessSorterTests.java | 5 +++-- 8 files changed, 14 insertions(+), 16 deletions(-) rename src/main/java/org/opensearch/flowframework/{template => workflow}/ProcessNode.java (97%) rename src/main/java/org/opensearch/flowframework/{template => workflow}/WorkflowProcessSorter.java (96%) rename src/test/java/org/opensearch/flowframework/{template => workflow}/ProcessNodeTests.java (91%) rename src/test/java/org/opensearch/flowframework/{template => workflow}/WorkflowProcessSorterTests.java (97%) diff --git a/src/main/java/demo/Demo.java b/src/main/java/demo/Demo.java index d75cf3fba..5a714ecb0 100644 --- a/src/main/java/demo/Demo.java +++ b/src/main/java/demo/Demo.java @@ -14,9 +14,9 @@ import org.opensearch.client.node.NodeClient; import org.opensearch.common.SuppressForbidden; import org.opensearch.common.io.PathUtils; -import org.opensearch.flowframework.template.ProcessNode; import org.opensearch.flowframework.template.Template; -import org.opensearch.flowframework.template.WorkflowProcessSorter; +import org.opensearch.flowframework.workflow.ProcessNode; +import org.opensearch.flowframework.workflow.WorkflowProcessSorter; import org.opensearch.flowframework.workflow.WorkflowStepFactory; import java.io.IOException; diff --git a/src/main/java/demo/TemplateParseDemo.java b/src/main/java/demo/TemplateParseDemo.java index 191e98630..7f53a3adc 100644 --- a/src/main/java/demo/TemplateParseDemo.java +++ b/src/main/java/demo/TemplateParseDemo.java @@ -16,7 +16,7 @@ import org.opensearch.common.io.PathUtils; import org.opensearch.flowframework.template.Template; import org.opensearch.flowframework.template.Workflow; -import org.opensearch.flowframework.template.WorkflowProcessSorter; +import org.opensearch.flowframework.workflow.WorkflowProcessSorter; import org.opensearch.flowframework.workflow.WorkflowStepFactory; import java.io.IOException; diff --git a/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java b/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java index e04cfd799..d701c832e 100644 --- a/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java +++ b/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java @@ -16,7 +16,7 @@ import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.env.Environment; import org.opensearch.env.NodeEnvironment; -import org.opensearch.flowframework.template.WorkflowProcessSorter; +import org.opensearch.flowframework.workflow.WorkflowProcessSorter; import org.opensearch.flowframework.workflow.WorkflowStepFactory; import org.opensearch.plugins.Plugin; import org.opensearch.repositories.RepositoriesService; diff --git a/src/main/java/org/opensearch/flowframework/template/WorkflowNode.java b/src/main/java/org/opensearch/flowframework/template/WorkflowNode.java index d1f493ca9..e01e8ba0c 100644 --- a/src/main/java/org/opensearch/flowframework/template/WorkflowNode.java +++ b/src/main/java/org/opensearch/flowframework/template/WorkflowNode.java @@ -11,6 +11,7 @@ import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.flowframework.workflow.ProcessNode; import org.opensearch.flowframework.workflow.WorkflowData; import org.opensearch.flowframework.workflow.WorkflowStep; diff --git a/src/main/java/org/opensearch/flowframework/template/ProcessNode.java b/src/main/java/org/opensearch/flowframework/workflow/ProcessNode.java similarity index 97% rename from src/main/java/org/opensearch/flowframework/template/ProcessNode.java rename to src/main/java/org/opensearch/flowframework/workflow/ProcessNode.java index 071bda159..a2d7628c3 100644 --- a/src/main/java/org/opensearch/flowframework/template/ProcessNode.java +++ b/src/main/java/org/opensearch/flowframework/workflow/ProcessNode.java @@ -6,12 +6,10 @@ * this file be licensed under the Apache-2.0 license or a * compatible open source license. */ -package org.opensearch.flowframework.template; +package org.opensearch.flowframework.workflow; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.opensearch.flowframework.workflow.WorkflowData; -import org.opensearch.flowframework.workflow.WorkflowStep; import java.util.ArrayList; import java.util.List; diff --git a/src/main/java/org/opensearch/flowframework/template/WorkflowProcessSorter.java b/src/main/java/org/opensearch/flowframework/workflow/WorkflowProcessSorter.java similarity index 96% rename from src/main/java/org/opensearch/flowframework/template/WorkflowProcessSorter.java rename to src/main/java/org/opensearch/flowframework/workflow/WorkflowProcessSorter.java index c05800e45..1cde5c078 100644 --- a/src/main/java/org/opensearch/flowframework/template/WorkflowProcessSorter.java +++ b/src/main/java/org/opensearch/flowframework/workflow/WorkflowProcessSorter.java @@ -6,13 +6,13 @@ * this file be licensed under the Apache-2.0 license or a * compatible open source license. */ -package org.opensearch.flowframework.template; +package org.opensearch.flowframework.workflow; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.opensearch.flowframework.workflow.WorkflowData; -import org.opensearch.flowframework.workflow.WorkflowStep; -import org.opensearch.flowframework.workflow.WorkflowStepFactory; +import org.opensearch.flowframework.template.Workflow; +import org.opensearch.flowframework.template.WorkflowEdge; +import org.opensearch.flowframework.template.WorkflowNode; import java.util.ArrayDeque; import java.util.ArrayList; diff --git a/src/test/java/org/opensearch/flowframework/template/ProcessNodeTests.java b/src/test/java/org/opensearch/flowframework/workflow/ProcessNodeTests.java similarity index 91% rename from src/test/java/org/opensearch/flowframework/template/ProcessNodeTests.java rename to src/test/java/org/opensearch/flowframework/workflow/ProcessNodeTests.java index b3e69253c..1972d20eb 100644 --- a/src/test/java/org/opensearch/flowframework/template/ProcessNodeTests.java +++ b/src/test/java/org/opensearch/flowframework/workflow/ProcessNodeTests.java @@ -6,10 +6,8 @@ * this file be licensed under the Apache-2.0 license or a * compatible open source license. */ -package org.opensearch.flowframework.template; +package org.opensearch.flowframework.workflow; -import org.opensearch.flowframework.workflow.WorkflowData; -import org.opensearch.flowframework.workflow.WorkflowStep; import org.opensearch.test.OpenSearchTestCase; import org.junit.After; import org.junit.Before; diff --git a/src/test/java/org/opensearch/flowframework/template/WorkflowProcessSorterTests.java b/src/test/java/org/opensearch/flowframework/workflow/WorkflowProcessSorterTests.java similarity index 97% rename from src/test/java/org/opensearch/flowframework/template/WorkflowProcessSorterTests.java rename to src/test/java/org/opensearch/flowframework/workflow/WorkflowProcessSorterTests.java index 8413b9f94..5e199272a 100644 --- a/src/test/java/org/opensearch/flowframework/template/WorkflowProcessSorterTests.java +++ b/src/test/java/org/opensearch/flowframework/workflow/WorkflowProcessSorterTests.java @@ -6,12 +6,13 @@ * this file be licensed under the Apache-2.0 license or a * compatible open source license. */ -package org.opensearch.flowframework.template; +package org.opensearch.flowframework.workflow; import org.opensearch.client.AdminClient; import org.opensearch.client.Client; import org.opensearch.core.xcontent.XContentParser; -import org.opensearch.flowframework.workflow.WorkflowStepFactory; +import org.opensearch.flowframework.template.TemplateTestJsonUtil; +import org.opensearch.flowframework.template.Workflow; import org.opensearch.test.OpenSearchTestCase; import org.junit.AfterClass; import org.junit.BeforeClass; From 2fca71b3adce51217512f313b69adb2ee00d0dee Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Tue, 26 Sep 2023 22:03:02 -0700 Subject: [PATCH 27/27] Add PipelineProcessor class and XContent parsing, rename package Signed-off-by: Daniel Widdis --- src/main/java/demo/Demo.java | 2 +- src/main/java/demo/TemplateParseDemo.java | 4 +- .../model/PipelineProcessor.java | 101 ++++++++++++++++++ .../{template => model}/Template.java | 2 +- .../{template => model}/Workflow.java | 2 +- .../{template => model}/WorkflowEdge.java | 2 +- .../{template => model}/WorkflowNode.java | 31 ++++-- .../workflow/WorkflowProcessSorter.java | 6 +- .../model/PipelineProcessorTests.java | 43 ++++++++ .../TemplateTestJsonUtil.java | 2 +- .../{template => model}/TemplateTests.java | 2 +- .../WorkflowEdgeTests.java | 2 +- .../WorkflowNodeTests.java | 14 ++- .../{template => model}/WorkflowTests.java | 2 +- .../workflow/WorkflowProcessSorterTests.java | 10 +- .../resources/template/finaltemplate.json | 8 +- 16 files changed, 202 insertions(+), 31 deletions(-) create mode 100644 src/main/java/org/opensearch/flowframework/model/PipelineProcessor.java rename src/main/java/org/opensearch/flowframework/{template => model}/Template.java (99%) rename src/main/java/org/opensearch/flowframework/{template => model}/Workflow.java (99%) rename src/main/java/org/opensearch/flowframework/{template => model}/WorkflowEdge.java (98%) rename src/main/java/org/opensearch/flowframework/{template => model}/WorkflowNode.java (80%) create mode 100644 src/test/java/org/opensearch/flowframework/model/PipelineProcessorTests.java rename src/test/java/org/opensearch/flowframework/{template => model}/TemplateTestJsonUtil.java (97%) rename src/test/java/org/opensearch/flowframework/{template => model}/TemplateTests.java (99%) rename src/test/java/org/opensearch/flowframework/{template => model}/WorkflowEdgeTests.java (97%) rename src/test/java/org/opensearch/flowframework/{template => model}/WorkflowNodeTests.java (79%) rename src/test/java/org/opensearch/flowframework/{template => model}/WorkflowTests.java (97%) diff --git a/src/main/java/demo/Demo.java b/src/main/java/demo/Demo.java index 5a714ecb0..53cf3499c 100644 --- a/src/main/java/demo/Demo.java +++ b/src/main/java/demo/Demo.java @@ -14,7 +14,7 @@ import org.opensearch.client.node.NodeClient; import org.opensearch.common.SuppressForbidden; import org.opensearch.common.io.PathUtils; -import org.opensearch.flowframework.template.Template; +import org.opensearch.flowframework.model.Template; import org.opensearch.flowframework.workflow.ProcessNode; import org.opensearch.flowframework.workflow.WorkflowProcessSorter; import org.opensearch.flowframework.workflow.WorkflowStepFactory; diff --git a/src/main/java/demo/TemplateParseDemo.java b/src/main/java/demo/TemplateParseDemo.java index 7f53a3adc..307d707c0 100644 --- a/src/main/java/demo/TemplateParseDemo.java +++ b/src/main/java/demo/TemplateParseDemo.java @@ -14,8 +14,8 @@ import org.opensearch.client.node.NodeClient; import org.opensearch.common.SuppressForbidden; import org.opensearch.common.io.PathUtils; -import org.opensearch.flowframework.template.Template; -import org.opensearch.flowframework.template.Workflow; +import org.opensearch.flowframework.model.Template; +import org.opensearch.flowframework.model.Workflow; import org.opensearch.flowframework.workflow.WorkflowProcessSorter; import org.opensearch.flowframework.workflow.WorkflowStepFactory; diff --git a/src/main/java/org/opensearch/flowframework/model/PipelineProcessor.java b/src/main/java/org/opensearch/flowframework/model/PipelineProcessor.java new file mode 100644 index 000000000..1407036b3 --- /dev/null +++ b/src/main/java/org/opensearch/flowframework/model/PipelineProcessor.java @@ -0,0 +1,101 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ +package org.opensearch.flowframework.model; + +import org.opensearch.core.xcontent.ToXContentObject; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.core.xcontent.XContentParser; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; + +import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; + +/** + * This represents a processor associated with search and ingest pipelines in the {@link Template}. + */ +public class PipelineProcessor implements ToXContentObject { + + /** The type field name for pipeline processors */ + public static final String TYPE_FIELD = "type"; + /** The params field name for pipeline processors */ + public static final String PARAMS_FIELD = "params"; + + private final String type; + private final Map params; + + /** + * Create this processor with a type and map of parameters + * @param type the processor type + * @param params a map of params + */ + public PipelineProcessor(String type, Map params) { + this.type = type; + this.params = params; + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + XContentBuilder xContentBuilder = builder.startObject(); + xContentBuilder.field(TYPE_FIELD, this.type); + xContentBuilder.field(PARAMS_FIELD); + Template.buildStringToStringMap(xContentBuilder, this.params); + return xContentBuilder.endObject(); + } + + /** + * Parse raw json content into a processor instance. + * + * @param parser json based content parser + * @return the parsed PipelineProcessor instance + * @throws IOException if content can't be parsed correctly + */ + public static PipelineProcessor parse(XContentParser parser) throws IOException { + String type = null; + Map params = new HashMap<>(); + + ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); + while (parser.nextToken() != XContentParser.Token.END_OBJECT) { + String fieldName = parser.currentName(); + parser.nextToken(); + switch (fieldName) { + case TYPE_FIELD: + type = parser.text(); + break; + case PARAMS_FIELD: + params = Template.parseStringToStringMap(parser); + break; + default: + throw new IOException("Unable to parse field [" + fieldName + "] in a pipeline processor object."); + } + } + if (type == null) { + throw new IOException("A processor object requires a type field."); + } + + return new PipelineProcessor(type, params); + } + + /** + * Get the processor type + * @return the type + */ + public String type() { + return type; + } + + /** + * Get the processor params + * @return the params + */ + public Map params() { + return params; + } +} diff --git a/src/main/java/org/opensearch/flowframework/template/Template.java b/src/main/java/org/opensearch/flowframework/model/Template.java similarity index 99% rename from src/main/java/org/opensearch/flowframework/template/Template.java rename to src/main/java/org/opensearch/flowframework/model/Template.java index 6b7035465..dd998aefa 100644 --- a/src/main/java/org/opensearch/flowframework/template/Template.java +++ b/src/main/java/org/opensearch/flowframework/model/Template.java @@ -6,7 +6,7 @@ * this file be licensed under the Apache-2.0 license or a * compatible open source license. */ -package org.opensearch.flowframework.template; +package org.opensearch.flowframework.model; import org.opensearch.Version; import org.opensearch.common.xcontent.LoggingDeprecationHandler; diff --git a/src/main/java/org/opensearch/flowframework/template/Workflow.java b/src/main/java/org/opensearch/flowframework/model/Workflow.java similarity index 99% rename from src/main/java/org/opensearch/flowframework/template/Workflow.java rename to src/main/java/org/opensearch/flowframework/model/Workflow.java index f1a7071b6..81f2677a7 100644 --- a/src/main/java/org/opensearch/flowframework/template/Workflow.java +++ b/src/main/java/org/opensearch/flowframework/model/Workflow.java @@ -6,7 +6,7 @@ * this file be licensed under the Apache-2.0 license or a * compatible open source license. */ -package org.opensearch.flowframework.template; +package org.opensearch.flowframework.model; import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; diff --git a/src/main/java/org/opensearch/flowframework/template/WorkflowEdge.java b/src/main/java/org/opensearch/flowframework/model/WorkflowEdge.java similarity index 98% rename from src/main/java/org/opensearch/flowframework/template/WorkflowEdge.java rename to src/main/java/org/opensearch/flowframework/model/WorkflowEdge.java index 3507f6c26..7fbdaf568 100644 --- a/src/main/java/org/opensearch/flowframework/template/WorkflowEdge.java +++ b/src/main/java/org/opensearch/flowframework/model/WorkflowEdge.java @@ -6,7 +6,7 @@ * this file be licensed under the Apache-2.0 license or a * compatible open source license. */ -package org.opensearch.flowframework.template; +package org.opensearch.flowframework.model; import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; diff --git a/src/main/java/org/opensearch/flowframework/template/WorkflowNode.java b/src/main/java/org/opensearch/flowframework/model/WorkflowNode.java similarity index 80% rename from src/main/java/org/opensearch/flowframework/template/WorkflowNode.java rename to src/main/java/org/opensearch/flowframework/model/WorkflowNode.java index e01e8ba0c..b48b6e0d2 100644 --- a/src/main/java/org/opensearch/flowframework/template/WorkflowNode.java +++ b/src/main/java/org/opensearch/flowframework/model/WorkflowNode.java @@ -6,7 +6,7 @@ * this file be licensed under the Apache-2.0 license or a * compatible open source license. */ -package org.opensearch.flowframework.template; +package org.opensearch.flowframework.model; import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; @@ -39,6 +39,8 @@ public class WorkflowNode implements ToXContentObject { public static final String TYPE_FIELD = "type"; /** The template field name for node inputs */ public static final String INPUTS_FIELD = "inputs"; + /** The field defining processors in the inputs for search and ingest pipelines */ + public static final String PROCESSORS_FIELD = "processors"; private final String id; // unique id private final String type; // maps to a WorkflowStep @@ -71,10 +73,15 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } else if (e.getValue() instanceof Map) { Template.buildStringToStringMap(xContentBuilder, (Map) e.getValue()); } else if (e.getValue() instanceof Object[]) { - // This assumes an array of maps for "processor" key xContentBuilder.startArray(); - for (Map map : (Map[]) e.getValue()) { - Template.buildStringToStringMap(xContentBuilder, map); + if (PROCESSORS_FIELD.equals(e.getKey())) { + for (PipelineProcessor p : (PipelineProcessor[]) e.getValue()) { + xContentBuilder.value(p); + } + } else { + for (Map map : (Map[]) e.getValue()) { + Template.buildStringToStringMap(xContentBuilder, map); + } } xContentBuilder.endArray(); } @@ -119,11 +126,19 @@ public static WorkflowNode parse(XContentParser parser) throws IOException { inputs.put(inputFieldName, Template.parseStringToStringMap(parser)); break; case START_ARRAY: - List> mapList = new ArrayList<>(); - while (parser.nextToken() != XContentParser.Token.END_ARRAY) { - mapList.add(Template.parseStringToStringMap(parser)); + if (PROCESSORS_FIELD.equals(inputFieldName)) { + List processorList = new ArrayList<>(); + while (parser.nextToken() != XContentParser.Token.END_ARRAY) { + processorList.add(PipelineProcessor.parse(parser)); + } + inputs.put(inputFieldName, processorList.toArray(new PipelineProcessor[0])); + } else { + List> mapList = new ArrayList<>(); + while (parser.nextToken() != XContentParser.Token.END_ARRAY) { + mapList.add(Template.parseStringToStringMap(parser)); + } + inputs.put(inputFieldName, mapList.toArray(new Map[0])); } - inputs.put(inputFieldName, mapList.toArray(new Map[0])); break; default: throw new IOException("Unable to parse field [" + inputFieldName + "] in a node object."); diff --git a/src/main/java/org/opensearch/flowframework/workflow/WorkflowProcessSorter.java b/src/main/java/org/opensearch/flowframework/workflow/WorkflowProcessSorter.java index 1cde5c078..3370f6384 100644 --- a/src/main/java/org/opensearch/flowframework/workflow/WorkflowProcessSorter.java +++ b/src/main/java/org/opensearch/flowframework/workflow/WorkflowProcessSorter.java @@ -10,9 +10,9 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.opensearch.flowframework.template.Workflow; -import org.opensearch.flowframework.template.WorkflowEdge; -import org.opensearch.flowframework.template.WorkflowNode; +import org.opensearch.flowframework.model.Workflow; +import org.opensearch.flowframework.model.WorkflowEdge; +import org.opensearch.flowframework.model.WorkflowNode; import java.util.ArrayDeque; import java.util.ArrayList; diff --git a/src/test/java/org/opensearch/flowframework/model/PipelineProcessorTests.java b/src/test/java/org/opensearch/flowframework/model/PipelineProcessorTests.java new file mode 100644 index 000000000..5e9a81d0d --- /dev/null +++ b/src/test/java/org/opensearch/flowframework/model/PipelineProcessorTests.java @@ -0,0 +1,43 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ +package org.opensearch.flowframework.model; + +import org.opensearch.test.OpenSearchTestCase; + +import java.io.IOException; +import java.util.Map; + +public class PipelineProcessorTests extends OpenSearchTestCase { + + public void testProcessor() throws IOException { + PipelineProcessor processor = new PipelineProcessor("foo", Map.of("bar", "baz")); + + assertEquals("foo", processor.type()); + assertEquals(Map.of("bar", "baz"), processor.params()); + + String expectedJson = "{\"type\":\"foo\",\"params\":{\"bar\":\"baz\"}}"; + String json = TemplateTestJsonUtil.parseToJson(processor); + assertEquals(expectedJson, json); + + PipelineProcessor processorX = PipelineProcessor.parse(TemplateTestJsonUtil.jsonToParser(json)); + assertEquals("foo", processorX.type()); + assertEquals(Map.of("bar", "baz"), processorX.params()); + } + + public void testExceptions() throws IOException { + String badJson = "{\"badField\":\"foo\",\"params\":{\"bar\":\"baz\"}}"; + IOException e = assertThrows(IOException.class, () -> PipelineProcessor.parse(TemplateTestJsonUtil.jsonToParser(badJson))); + assertEquals("Unable to parse field [badField] in a pipeline processor object.", e.getMessage()); + + String noTypeJson = "{\"params\":{\"bar\":\"baz\"}}"; + e = assertThrows(IOException.class, () -> PipelineProcessor.parse(TemplateTestJsonUtil.jsonToParser(noTypeJson))); + assertEquals("A processor object requires a type field.", e.getMessage()); + } + +} diff --git a/src/test/java/org/opensearch/flowframework/template/TemplateTestJsonUtil.java b/src/test/java/org/opensearch/flowframework/model/TemplateTestJsonUtil.java similarity index 97% rename from src/test/java/org/opensearch/flowframework/template/TemplateTestJsonUtil.java rename to src/test/java/org/opensearch/flowframework/model/TemplateTestJsonUtil.java index e0e619f9e..247521084 100644 --- a/src/test/java/org/opensearch/flowframework/template/TemplateTestJsonUtil.java +++ b/src/test/java/org/opensearch/flowframework/model/TemplateTestJsonUtil.java @@ -6,7 +6,7 @@ * this file be licensed under the Apache-2.0 license or a * compatible open source license. */ -package org.opensearch.flowframework.template; +package org.opensearch.flowframework.model; import org.opensearch.common.xcontent.LoggingDeprecationHandler; import org.opensearch.common.xcontent.json.JsonXContent; diff --git a/src/test/java/org/opensearch/flowframework/template/TemplateTests.java b/src/test/java/org/opensearch/flowframework/model/TemplateTests.java similarity index 99% rename from src/test/java/org/opensearch/flowframework/template/TemplateTests.java rename to src/test/java/org/opensearch/flowframework/model/TemplateTests.java index a00a6d4c9..69f14dfaf 100644 --- a/src/test/java/org/opensearch/flowframework/template/TemplateTests.java +++ b/src/test/java/org/opensearch/flowframework/model/TemplateTests.java @@ -6,7 +6,7 @@ * this file be licensed under the Apache-2.0 license or a * compatible open source license. */ -package org.opensearch.flowframework.template; +package org.opensearch.flowframework.model; import org.opensearch.Version; import org.opensearch.test.OpenSearchTestCase; diff --git a/src/test/java/org/opensearch/flowframework/template/WorkflowEdgeTests.java b/src/test/java/org/opensearch/flowframework/model/WorkflowEdgeTests.java similarity index 97% rename from src/test/java/org/opensearch/flowframework/template/WorkflowEdgeTests.java rename to src/test/java/org/opensearch/flowframework/model/WorkflowEdgeTests.java index f6b61fb52..ffbd07bd1 100644 --- a/src/test/java/org/opensearch/flowframework/template/WorkflowEdgeTests.java +++ b/src/test/java/org/opensearch/flowframework/model/WorkflowEdgeTests.java @@ -6,7 +6,7 @@ * this file be licensed under the Apache-2.0 license or a * compatible open source license. */ -package org.opensearch.flowframework.template; +package org.opensearch.flowframework.model; import org.opensearch.test.OpenSearchTestCase; diff --git a/src/test/java/org/opensearch/flowframework/template/WorkflowNodeTests.java b/src/test/java/org/opensearch/flowframework/model/WorkflowNodeTests.java similarity index 79% rename from src/test/java/org/opensearch/flowframework/template/WorkflowNodeTests.java rename to src/test/java/org/opensearch/flowframework/model/WorkflowNodeTests.java index 157edfc01..46d897b42 100644 --- a/src/test/java/org/opensearch/flowframework/template/WorkflowNodeTests.java +++ b/src/test/java/org/opensearch/flowframework/model/WorkflowNodeTests.java @@ -6,7 +6,7 @@ * this file be licensed under the Apache-2.0 license or a * compatible open source license. */ -package org.opensearch.flowframework.template; +package org.opensearch.flowframework.model; import org.opensearch.test.OpenSearchTestCase; @@ -27,7 +27,8 @@ public void testNode() throws IOException { Map.ofEntries( Map.entry("foo", "a string"), Map.entry("bar", Map.of("key", "value")), - Map.entry("baz", new Map[] { Map.of("A", "a"), Map.of("B", "b") }) + Map.entry("baz", new Map[] { Map.of("A", "a"), Map.of("B", "b") }), + Map.entry("processors", new PipelineProcessor[] { new PipelineProcessor("test-type", Map.of("key2", "value2")) }) ) ); assertEquals("A", nodeA.id()); @@ -36,6 +37,10 @@ public void testNode() throws IOException { assertEquals("a string", (String) map.get("foo")); assertEquals(Map.of("key", "value"), (Map) map.get("bar")); assertArrayEquals(new Map[] { Map.of("A", "a"), Map.of("B", "b") }, (Map[]) map.get("baz")); + PipelineProcessor[] pp = (PipelineProcessor[]) map.get("processors"); + assertEquals(1, pp.length); + assertEquals("test-type", pp[0].type()); + assertEquals(Map.of("key2", "value2"), pp[0].params()); // node equality is based only on ID WorkflowNode nodeA2 = new WorkflowNode("A", "a2-type", Map.of("bar", "baz")); @@ -49,6 +54,7 @@ public void testNode() throws IOException { assertTrue(json.contains("\"foo\":\"a string\"")); assertTrue(json.contains("\"baz\":[{\"A\":\"a\"},{\"B\":\"b\"}]")); assertTrue(json.contains("\"bar\":{\"key\":\"value\"}")); + assertTrue(json.contains("\"processors\":[{\"type\":\"test-type\",\"params\":{\"key2\":\"value2\"}}]")); WorkflowNode nodeX = WorkflowNode.parse(TemplateTestJsonUtil.jsonToParser(json)); assertEquals("A", nodeX.id()); @@ -57,6 +63,10 @@ public void testNode() throws IOException { assertEquals("a string", mapX.get("foo")); assertEquals(Map.of("key", "value"), mapX.get("bar")); assertArrayEquals(new Map[] { Map.of("A", "a"), Map.of("B", "b") }, (Map[]) map.get("baz")); + PipelineProcessor[] ppX = (PipelineProcessor[]) map.get("processors"); + assertEquals(1, ppX.length); + assertEquals("test-type", ppX[0].type()); + assertEquals(Map.of("key2", "value2"), ppX[0].params()); } public void testExceptions() throws IOException { diff --git a/src/test/java/org/opensearch/flowframework/template/WorkflowTests.java b/src/test/java/org/opensearch/flowframework/model/WorkflowTests.java similarity index 97% rename from src/test/java/org/opensearch/flowframework/template/WorkflowTests.java rename to src/test/java/org/opensearch/flowframework/model/WorkflowTests.java index 94a68455e..db070da4b 100644 --- a/src/test/java/org/opensearch/flowframework/template/WorkflowTests.java +++ b/src/test/java/org/opensearch/flowframework/model/WorkflowTests.java @@ -6,7 +6,7 @@ * this file be licensed under the Apache-2.0 license or a * compatible open source license. */ -package org.opensearch.flowframework.template; +package org.opensearch.flowframework.model; import org.opensearch.core.xcontent.XContentParser; import org.opensearch.test.OpenSearchTestCase; diff --git a/src/test/java/org/opensearch/flowframework/workflow/WorkflowProcessSorterTests.java b/src/test/java/org/opensearch/flowframework/workflow/WorkflowProcessSorterTests.java index 5e199272a..1e9c8e808 100644 --- a/src/test/java/org/opensearch/flowframework/workflow/WorkflowProcessSorterTests.java +++ b/src/test/java/org/opensearch/flowframework/workflow/WorkflowProcessSorterTests.java @@ -11,8 +11,8 @@ import org.opensearch.client.AdminClient; import org.opensearch.client.Client; import org.opensearch.core.xcontent.XContentParser; -import org.opensearch.flowframework.template.TemplateTestJsonUtil; -import org.opensearch.flowframework.template.Workflow; +import org.opensearch.flowframework.model.TemplateTestJsonUtil; +import org.opensearch.flowframework.model.Workflow; import org.opensearch.test.OpenSearchTestCase; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -24,9 +24,9 @@ import java.util.concurrent.Executors; import java.util.stream.Collectors; -import static org.opensearch.flowframework.template.TemplateTestJsonUtil.edge; -import static org.opensearch.flowframework.template.TemplateTestJsonUtil.node; -import static org.opensearch.flowframework.template.TemplateTestJsonUtil.workflow; +import static org.opensearch.flowframework.model.TemplateTestJsonUtil.edge; +import static org.opensearch.flowframework.model.TemplateTestJsonUtil.node; +import static org.opensearch.flowframework.model.TemplateTestJsonUtil.workflow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; diff --git a/src/test/resources/template/finaltemplate.json b/src/test/resources/template/finaltemplate.json index 65be77758..d8443c4c6 100644 --- a/src/test/resources/template/finaltemplate.json +++ b/src/test/resources/template/finaltemplate.json @@ -36,9 +36,11 @@ "description": "some description", "processors": [{ "type": "text_embedding", - "model_id": "my-existing-model-id", - "input_field": "text_passage", - "output_field": "text_embedding" + "params": { + "model_id": "my-existing-model-id", + "input_field": "text_passage", + "output_field": "text_embedding" + } }] } }