From 162ad9ebfe122d736a5b4a76100d7c5c356153b6 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Wed, 25 Oct 2023 16:53:32 -0700 Subject: [PATCH] [Backport 2.x] Removes operations, resources_created, user_output fields from the Global Context and Template class (#115) Removes operations, resources_created, user_output fields from the Global Context and Template class (#114) Simplifying Template format, removing operations, resources created, user outputs (cherry picked from commit 6ee3d5323a4ca2d8bbfc87fea271d8a4cbfe479f) Signed-off-by: Joshua Palis Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] --- .../flowframework/model/Template.java | 127 +----------------- .../resources/mappings/global-context.json | 9 -- .../flowframework/model/TemplateTests.java | 18 +-- .../rest/RestCreateWorkflowActionTests.java | 6 +- .../CreateWorkflowTransportActionTests.java | 6 +- ...ProvisionWorkflowTransportActionTests.java | 6 +- .../WorkflowRequestResponseTests.java | 6 +- 7 files changed, 10 insertions(+), 168 deletions(-) diff --git a/src/main/java/org/opensearch/flowframework/model/Template.java b/src/main/java/org/opensearch/flowframework/model/Template.java index a1da67a4d..a1a526153 100644 --- a/src/main/java/org/opensearch/flowframework/model/Template.java +++ b/src/main/java/org/opensearch/flowframework/model/Template.java @@ -25,7 +25,6 @@ import java.util.Map.Entry; import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; -import static org.opensearch.flowframework.common.TemplateUtil.parseStringToStringMap; /** * The Template is the central data structure which configures workflows. This object is used to parse JSON communicated via REST API. @@ -38,8 +37,6 @@ public class Template implements ToXContentObject { 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 */ @@ -48,20 +45,13 @@ public class Template implements ToXContentObject { public static final String COMPATIBILITY_FIELD = "compatibility"; /** The template field name for template workflows */ public static final String WORKFLOWS_FIELD = "workflows"; - /** The template field name for template user outputs */ - public static final String USER_OUTPUTS_FIELD = "user_outputs"; - /** The template field name for template resources created */ - public static final String RESOURCES_CREATED_FIELD = "resources_created"; private final String name; private final String description; private final String useCase; // probably an ENUM actually - private final List operations; // probably an ENUM actually private final Version templateVersion; private final List compatibilityVersion; private final Map workflows; - private final Map userOutputs; - private final Map resourcesCreated; /** * Instantiate the object representing a use case template @@ -69,33 +59,24 @@ public class Template implements ToXContentObject { * @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 workflows Workflow graph definitions corresponding to the defined operations. - * @param userOutputs A map of essential API responses for backend to use and lookup. - * @param resourcesCreated A map of all the resources created. */ public Template( String name, String description, String useCase, - List operations, Version templateVersion, List compatibilityVersion, - Map workflows, - Map userOutputs, - Map resourcesCreated + Map workflows ) { this.name = name; this.description = description; this.useCase = useCase; - this.operations = List.copyOf(operations); this.templateVersion = templateVersion; this.compatibilityVersion = List.copyOf(compatibilityVersion); this.workflows = Map.copyOf(workflows); - this.userOutputs = Map.copyOf(userOutputs); - this.resourcesCreated = Map.copyOf(resourcesCreated); } @Override @@ -104,11 +85,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws 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(); if (this.templateVersion != null || !this.compatibilityVersion.isEmpty()) { xContentBuilder.startObject(VERSION_FIELD); @@ -131,18 +107,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } xContentBuilder.endObject(); - xContentBuilder.startObject(USER_OUTPUTS_FIELD); - for (Entry e : userOutputs.entrySet()) { - xContentBuilder.field(e.getKey(), e.getValue()); - } - xContentBuilder.endObject(); - - xContentBuilder.startObject(RESOURCES_CREATED_FIELD); - for (Entry e : resourcesCreated.entrySet()) { - xContentBuilder.field(e.getKey(), e.getValue()); - } - xContentBuilder.endObject(); - return xContentBuilder.endObject(); } @@ -157,12 +121,9 @@ public static Template parse(XContentParser parser) throws IOException { String name = null; String description = ""; String useCase = ""; - List operations = new ArrayList<>(); Version templateVersion = null; List compatibilityVersion = new ArrayList<>(); Map workflows = new HashMap<>(); - Map userOutputs = new HashMap<>(); - Map resourcesCreated = new HashMap<>(); ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); while (parser.nextToken() != XContentParser.Token.END_OBJECT) { @@ -178,12 +139,6 @@ public static Template parse(XContentParser parser) throws IOException { case USE_CASE_FIELD: useCase = parser.text(); break; - case OPERATIONS_FIELD: - ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.currentToken(), parser); - while (parser.nextToken() != XContentParser.Token.END_ARRAY) { - operations.add(parser.text()); - } - break; case VERSION_FIELD: ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); while (parser.nextToken() != XContentParser.Token.END_OBJECT) { @@ -212,42 +167,6 @@ public static Template parse(XContentParser parser) throws IOException { workflows.put(workflowFieldName, Workflow.parse(parser)); } break; - case USER_OUTPUTS_FIELD: - ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); - while (parser.nextToken() != XContentParser.Token.END_OBJECT) { - String userOutputsFieldName = parser.currentName(); - switch (parser.nextToken()) { - case VALUE_STRING: - userOutputs.put(userOutputsFieldName, parser.text()); - break; - case START_OBJECT: - userOutputs.put(userOutputsFieldName, parseStringToStringMap(parser)); - break; - default: - throw new IOException("Unable to parse field [" + userOutputsFieldName + "] in a user_outputs object."); - } - } - break; - - case RESOURCES_CREATED_FIELD: - ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); - while (parser.nextToken() != XContentParser.Token.END_OBJECT) { - String resourcesCreatedField = parser.currentName(); - switch (parser.nextToken()) { - case VALUE_STRING: - resourcesCreated.put(resourcesCreatedField, parser.text()); - break; - case START_OBJECT: - resourcesCreated.put(resourcesCreatedField, parseStringToStringMap(parser)); - break; - default: - throw new IOException( - "Unable to parse field [" + resourcesCreatedField + "] in a resources_created object." - ); - } - } - break; - default: throw new IOException("Unable to parse field [" + fieldName + "] in a template object."); } @@ -256,17 +175,7 @@ public static Template parse(XContentParser parser) throws IOException { throw new IOException("An template object requires a name."); } - return new Template( - name, - description, - useCase, - operations, - templateVersion, - compatibilityVersion, - workflows, - userOutputs, - resourcesCreated - ); + return new Template(name, description, useCase, templateVersion, compatibilityVersion, workflows); } /** @@ -338,14 +247,6 @@ 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 @@ -363,29 +264,13 @@ public List compatibilityVersion() { } /** - * Workflows encoded in this template, generally corresponding to the operations returned by {@link #operations()}. + * Workflows encoded in this template * @return the workflows */ public Map workflows() { return workflows; } - /** - * A map of essential API responses - * @return the userOutputs - */ - public Map userOutputs() { - return userOutputs; - } - - /** - * A map of all the resources created - * @return the resources created - */ - public Map resourcesCreated() { - return resourcesCreated; - } - @Override public String toString() { return "Template [name=" @@ -394,18 +279,12 @@ public String toString() { + description + ", useCase=" + useCase - + ", operations=" - + operations + ", templateVersion=" + templateVersion + ", compatibilityVersion=" + compatibilityVersion + ", workflows=" + workflows - + ", userOutputs=" - + userOutputs - + ", resourcesCreated=" - + resourcesCreated + "]"; } } diff --git a/src/main/resources/mappings/global-context.json b/src/main/resources/mappings/global-context.json index bb1256dee..5190d4c95 100644 --- a/src/main/resources/mappings/global-context.json +++ b/src/main/resources/mappings/global-context.json @@ -22,9 +22,6 @@ "use_case": { "type": "keyword" }, - "operations": { - "type": "keyword" - }, "version": { "type": "nested", "properties": { @@ -38,12 +35,6 @@ }, "workflows": { "type": "object" - }, - "user_outputs": { - "type": "object" - }, - "resources_created": { - "type": "object" } } } diff --git a/src/test/java/org/opensearch/flowframework/model/TemplateTests.java b/src/test/java/org/opensearch/flowframework/model/TemplateTests.java index 78548c46a..695a31ca4 100644 --- a/src/test/java/org/opensearch/flowframework/model/TemplateTests.java +++ b/src/test/java/org/opensearch/flowframework/model/TemplateTests.java @@ -8,8 +8,6 @@ */ package org.opensearch.flowframework.model; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import org.opensearch.Version; import org.opensearch.test.OpenSearchTestCase; @@ -19,13 +17,9 @@ public class TemplateTests extends OpenSearchTestCase { - private final Logger logger = LogManager.getLogger(TemplateTests.class); - private String expectedTemplate = - "{\"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\"]}," - + "\"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\"}]}}," - + "\"user_outputs\":{\"responsesMapKey\":{\"nestedKey\":\"nestedValue\"},\"responsesKey\":\"testValue\"}," - + "\"resources_created\":{\"resourcesMapKey\":{\"nestedKey\":\"nestedValue\"},\"resourcesKey\":\"resourceValue\"}}"; + "{\"name\":\"test\",\"description\":\"a test template\",\"use_case\":\"test use case\",\"version\":{\"template\":\"1.2.3\",\"compatibility\":[\"4.5.6\",\"7.8.9\"]}," + + "\"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 { @@ -35,7 +29,6 @@ public void setUp() throws Exception { 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"); @@ -47,18 +40,14 @@ public void testTemplate() throws IOException { "test", "a test template", "test use case", - List.of("operation"), templateVersion, compatibilityVersion, - Map.of("workflow", workflow), - Map.ofEntries(Map.entry("responsesKey", "testValue"), Map.entry("responsesMapKey", Map.of("nestedKey", "nestedValue"))), - Map.ofEntries(Map.entry("resourcesKey", "resourceValue"), Map.entry("resourcesMapKey", Map.of("nestedKey", "nestedValue"))) + 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()); Workflow wf = template.workflows().get("workflow"); @@ -71,7 +60,6 @@ public void testTemplate() throws IOException { 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()); Workflow wfX = templateX.workflows().get("workflow"); diff --git a/src/test/java/org/opensearch/flowframework/rest/RestCreateWorkflowActionTests.java b/src/test/java/org/opensearch/flowframework/rest/RestCreateWorkflowActionTests.java index 11e6c61fa..141ea61b6 100644 --- a/src/test/java/org/opensearch/flowframework/rest/RestCreateWorkflowActionTests.java +++ b/src/test/java/org/opensearch/flowframework/rest/RestCreateWorkflowActionTests.java @@ -41,7 +41,6 @@ public class RestCreateWorkflowActionTests extends OpenSearchTestCase { public void setUp() throws Exception { super.setUp(); - List operations = List.of("operation"); Version templateVersion = Version.fromString("1.0.0"); List compatibilityVersions = List.of(Version.fromString("2.0.0"), Version.fromString("3.0.0")); WorkflowNode nodeA = new WorkflowNode("A", "a-type", Map.of("foo", "bar")); @@ -55,12 +54,9 @@ public void setUp() throws Exception { "test", "description", "use case", - operations, templateVersion, compatibilityVersions, - Map.of("workflow", workflow), - Map.of("outputKey", "outputValue"), - Map.of("resourceKey", "resourceValue") + Map.of("workflow", workflow) ); // Invalid template configuration, wrong field name diff --git a/src/test/java/org/opensearch/flowframework/transport/CreateWorkflowTransportActionTests.java b/src/test/java/org/opensearch/flowframework/transport/CreateWorkflowTransportActionTests.java index 1f7df11d9..dc3840d44 100644 --- a/src/test/java/org/opensearch/flowframework/transport/CreateWorkflowTransportActionTests.java +++ b/src/test/java/org/opensearch/flowframework/transport/CreateWorkflowTransportActionTests.java @@ -50,7 +50,6 @@ public void setUp() throws Exception { globalContextHandler ); - List operations = List.of("operation"); Version templateVersion = Version.fromString("1.0.0"); List compatibilityVersions = List.of(Version.fromString("2.0.0"), Version.fromString("3.0.0")); WorkflowNode nodeA = new WorkflowNode("A", "a-type", Map.of("foo", "bar")); @@ -64,12 +63,9 @@ public void setUp() throws Exception { "test", "description", "use case", - operations, templateVersion, compatibilityVersions, - Map.of("workflow", workflow), - Map.of("outputKey", "outputValue"), - Map.of("resourceKey", "resourceValue") + Map.of("workflow", workflow) ); } diff --git a/src/test/java/org/opensearch/flowframework/transport/ProvisionWorkflowTransportActionTests.java b/src/test/java/org/opensearch/flowframework/transport/ProvisionWorkflowTransportActionTests.java index 7c0ae9ef9..d4f37261a 100644 --- a/src/test/java/org/opensearch/flowframework/transport/ProvisionWorkflowTransportActionTests.java +++ b/src/test/java/org/opensearch/flowframework/transport/ProvisionWorkflowTransportActionTests.java @@ -66,7 +66,6 @@ public void setUp() throws Exception { workflowProcessSorter ); - List operations = List.of("operation"); Version templateVersion = Version.fromString("1.0.0"); List compatibilityVersions = List.of(Version.fromString("2.0.0"), Version.fromString("3.0.0")); WorkflowNode nodeA = new WorkflowNode("A", "a-type", Map.of("foo", "bar")); @@ -80,12 +79,9 @@ public void setUp() throws Exception { "test", "description", "use case", - operations, templateVersion, compatibilityVersions, - Map.of("provision", workflow), - Map.of("outputKey", "outputValue"), - Map.of("resourceKey", "resourceValue") + Map.of("provision", workflow) ); ThreadPool clientThreadPool = mock(ThreadPool.class); diff --git a/src/test/java/org/opensearch/flowframework/transport/WorkflowRequestResponseTests.java b/src/test/java/org/opensearch/flowframework/transport/WorkflowRequestResponseTests.java index 063490c41..057088aac 100644 --- a/src/test/java/org/opensearch/flowframework/transport/WorkflowRequestResponseTests.java +++ b/src/test/java/org/opensearch/flowframework/transport/WorkflowRequestResponseTests.java @@ -33,7 +33,6 @@ public class WorkflowRequestResponseTests extends OpenSearchTestCase { @Override public void setUp() throws Exception { super.setUp(); - List operations = List.of("operation"); Version templateVersion = Version.fromString("1.0.0"); List compatibilityVersions = List.of(Version.fromString("2.0.0"), Version.fromString("3.0.0")); WorkflowNode nodeA = new WorkflowNode("A", "a-type", Map.of("foo", "bar")); @@ -47,12 +46,9 @@ public void setUp() throws Exception { "test", "description", "use case", - operations, templateVersion, compatibilityVersions, - Map.of("workflow", workflow), - Map.of("outputKey", "outputValue"), - Map.of("resourceKey", "resourceValue") + Map.of("workflow", workflow) ); }