Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 2.x] Removes user_inputs field from Template and Global Context Index mapping #112

Merged
merged 1 commit into from
Oct 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public void putTemplateToGlobalContext(Template template, ActionListener<IndexRe
XContentBuilder builder = XContentFactory.jsonBuilder();
ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()
) {
request.source(template.toDocumentSource(builder, ToXContent.EMPTY_PARAMS))
request.source(template.toXContent(builder, ToXContent.EMPTY_PARAMS))
.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE);
client.index(request, ActionListener.runBefore(listener, () -> context.restore()));
} catch (Exception e) {
Expand Down Expand Up @@ -113,7 +113,7 @@ public void updateTemplateInGlobalContext(String documentId, Template template,
XContentBuilder builder = XContentFactory.jsonBuilder();
ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()
) {
request.source(template.toDocumentSource(builder, ToXContent.EMPTY_PARAMS))
request.source(template.toXContent(builder, ToXContent.EMPTY_PARAMS))
.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE);
client.index(request, ActionListener.runBefore(listener, () -> context.restore()));
} catch (Exception e) {
Expand Down
265 changes: 0 additions & 265 deletions src/main/java/org/opensearch/flowframework/model/Template.java

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ protected void doExecute(Task task, WorkflowRequest request, ActionListener<Work
}

// Parse template from document source
Template template = Template.parseFromDocumentSource(response.getSourceAsString());
Template template = Template.parse(response.getSourceAsString());

// TODO : Update state index entry to PROVISIONING, given workflowId

Expand Down
17 changes: 3 additions & 14 deletions src/main/resources/mappings/global-context.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,25 +36,14 @@
}
}
},
"user_inputs": {
"type": "nested",
"properties": {
"index_name": {
"type": "keyword"
},
"index_setting": {
"type": "keyword"
}
}
},
"workflows": {
"type": "text"
"type": "object"
},
"user_outputs": {
"type": "text"
"type": "object"
},
"resources_created": {
"type": "text"
"type": "object"
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public void setUp() throws Exception {

public void testPutTemplateToGlobalContext() throws IOException {
Template template = mock(Template.class);
when(template.toDocumentSource(any(XContentBuilder.class), eq(ToXContent.EMPTY_PARAMS))).thenAnswer(invocation -> {
when(template.toXContent(any(XContentBuilder.class), eq(ToXContent.EMPTY_PARAMS))).thenAnswer(invocation -> {
XContentBuilder builder = invocation.getArgument(0);
return builder;
});
Expand Down Expand Up @@ -112,7 +112,7 @@ public void testStoreResponseToGlobalContext() {

public void testUpdateTemplateInGlobalContext() throws IOException {
Template template = mock(Template.class);
when(template.toDocumentSource(any(XContentBuilder.class), eq(ToXContent.EMPTY_PARAMS))).thenAnswer(invocation -> {
when(template.toXContent(any(XContentBuilder.class), eq(ToXContent.EMPTY_PARAMS))).thenAnswer(invocation -> {
XContentBuilder builder = invocation.getArgument(0);
return builder;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
*/
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;

Expand All @@ -17,14 +19,13 @@

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\"}]}}}";
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\"}}";

@Override
public void setUp() throws Exception {
Expand All @@ -49,7 +50,6 @@ public void testTemplate() throws IOException {
List.of("operation"),
templateVersion,
compatibilityVersion,
Map.ofEntries(Map.entry("userKey", "userValue"), Map.entry("userMapKey", Map.of("nestedKey", "nestedValue"))),
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")))
Expand All @@ -61,18 +61,11 @@ public void testTemplate() throws IOException {
assertEquals(List.of("operation"), template.operations());
assertEquals(templateVersion, template.templateVersion());
assertEquals(compatibilityVersion, template.compatibilityVersion());
Map<String, Object> 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 json = TemplateTestJsonUtil.parseToJson(template);
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());
Expand All @@ -81,38 +74,26 @@ public void testTemplate() throws IOException {
assertEquals(List.of("operation"), templateX.operations());
assertEquals(templateVersion, templateX.templateVersion());
assertEquals(compatibilityVersion, templateX.compatibilityVersion());
Map<String, Object> 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");
String badTemplateField = expectedTemplate.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");
String badVersionField = expectedTemplate.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));

Template t = Template.parse(expectedTemplate);
assertTrue(t.toJson().contains("a test template"));
assertTrue(t.toYaml().contains("a test template"));
assertTrue(t.toString().contains("a test template"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ public void setUp() throws Exception {
operations,
templateVersion,
compatibilityVersions,
Map.ofEntries(Map.entry("userKey", "userValue"), Map.entry("userMapKey", Map.of("nestedKey", "nestedValue"))),
Map.of("workflow", workflow),
Map.of("outputKey", "outputValue"),
Map.of("resourceKey", "resourceValue")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ public void setUp() throws Exception {
operations,
templateVersion,
compatibilityVersions,
Map.ofEntries(Map.entry("userKey", "userValue"), Map.entry("userMapKey", Map.of("nestedKey", "nestedValue"))),
Map.of("workflow", workflow),
Map.of("outputKey", "outputValue"),
Map.of("resourceKey", "resourceValue")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import org.opensearch.common.xcontent.XContentFactory;
import org.opensearch.core.action.ActionListener;
import org.opensearch.core.common.bytes.BytesReference;
import org.opensearch.core.xcontent.ToXContent;
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.flowframework.model.Template;
import org.opensearch.flowframework.model.Workflow;
Expand Down Expand Up @@ -84,7 +83,6 @@ public void setUp() throws Exception {
operations,
templateVersion,
compatibilityVersions,
Map.ofEntries(Map.entry("userKey", "userValue"), Map.entry("userMapKey", Map.of("nestedKey", "nestedValue"))),
Map.of("provision", workflow),
Map.of("outputKey", "outputValue"),
Map.of("resourceKey", "resourceValue")
Expand All @@ -108,7 +106,7 @@ public void testProvisionWorkflow() {
ActionListener<GetResponse> responseListener = invocation.getArgument(1);

XContentBuilder builder = XContentFactory.jsonBuilder();
this.template.toDocumentSource(builder, ToXContent.EMPTY_PARAMS);
this.template.toXContent(builder, null);
BytesReference templateBytesRef = BytesReference.bytes(builder);
GetResult getResult = new GetResult(GLOBAL_CONTEXT_INDEX, workflowId, 1, 1, 1, true, templateBytesRef, null, null);
responseListener.onResponse(new GetResponse(getResult));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ public void setUp() throws Exception {
operations,
templateVersion,
compatibilityVersions,
Map.ofEntries(Map.entry("userKey", "userValue"), Map.entry("userMapKey", Map.of("nestedKey", "nestedValue"))),
Map.of("workflow", workflow),
Map.of("outputKey", "outputValue"),
Map.of("resourceKey", "resourceValue")
Expand Down
Loading