Skip to content

Commit 58903ba

Browse files
akolarkunnuakolarkunnu
authored
[Enhancement] Enhance validation for create connector API (opensearch-project#3260)
This PR addresses the first part of this enhancement "Validate if connector payload has all the required fields. If not provided, throw the illegal argument exception". Validation of fields description, parameters, credential, and request_body are missing. That validations are added in this fix. Added new test cases correspong to these validations and fixed all failing test cases because of these new validations. Partially Resolves opensearch-project#1382 Signed-off-by: Abdul Muneer Kolarkunnu <muneer.kolarkunnu@netapp.com>
1 parent c2a40c1 commit 58903ba

File tree

5 files changed

+215
-169
lines changed

5 files changed

+215
-169
lines changed

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,13 @@ public ConnectorAction(
5555
String postProcessFunction
5656
) {
5757
if (actionType == null) {
58-
throw new IllegalArgumentException("action type can't null");
58+
throw new IllegalArgumentException("action type can't be null");
5959
}
6060
if (url == null) {
61-
throw new IllegalArgumentException("url can't null");
61+
throw new IllegalArgumentException("url can't be null");
6262
}
6363
if (method == null) {
64-
throw new IllegalArgumentException("method can't null");
64+
throw new IllegalArgumentException("method can't be null");
6565
}
6666
this.actionType = actionType;
6767
this.method = method;

‎common/src/main/java/org/opensearch/ml/common/transport/connector/MLCreateConnectorInput.java

+3
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,9 @@ public MLCreateConnectorInput(
9393
if (protocol == null) {
9494
throw new IllegalArgumentException("Connector protocol is null");
9595
}
96+
if (credential == null || credential.isEmpty()) {
97+
throw new IllegalArgumentException("Connector credential is null or empty list");
98+
}
9699
}
97100
this.name = name;
98101
this.description = description;

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

+64-72
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,16 @@
55

66
package org.opensearch.ml.common.connector;
77

8+
import static org.junit.Assert.assertEquals;
9+
import static org.junit.Assert.assertThrows;
810
import static org.opensearch.ml.common.connector.ConnectorAction.ActionType.isValidActionInModelPrediction;
911

1012
import java.io.IOException;
1113
import java.util.Collections;
1214
import java.util.HashMap;
1315
import java.util.Map;
1416

15-
import org.junit.Assert;
16-
import org.junit.Rule;
1717
import org.junit.Test;
18-
import org.junit.rules.ExpectedException;
1918
import org.opensearch.common.io.stream.BytesStreamOutput;
2019
import org.opensearch.common.settings.Settings;
2120
import org.opensearch.common.xcontent.XContentType;
@@ -27,130 +26,124 @@
2726
import org.opensearch.search.SearchModule;
2827

2928
public class ConnectorActionTest {
30-
@Rule
31-
public ExpectedException exceptionRule = ExpectedException.none();
29+
30+
// Shared test data for the class
31+
private static final ConnectorAction.ActionType TEST_ACTION_TYPE = ConnectorAction.ActionType.PREDICT;
32+
private static final String TEST_METHOD_POST = "post";
33+
private static final String TEST_METHOD_HTTP = "http";
34+
private static final String TEST_REQUEST_BODY = "{\"input\": \"${parameters.input}\"}";
35+
private static final String URL = "https://test.com";
3236

3337
@Test
3438
public void constructor_NullActionType() {
35-
exceptionRule.expect(IllegalArgumentException.class);
36-
exceptionRule.expectMessage("action type can't null");
37-
ConnectorAction.ActionType actionType = null;
38-
String method = "post";
39-
String url = "https://test.com";
40-
new ConnectorAction(actionType, method, url, null, null, null, null);
39+
Throwable exception = assertThrows(
40+
IllegalArgumentException.class,
41+
() -> new ConnectorAction(null, TEST_METHOD_POST, URL, null, TEST_REQUEST_BODY, null, null)
42+
);
43+
assertEquals("action type can't be null", exception.getMessage());
44+
4145
}
4246

4347
@Test
4448
public void constructor_NullUrl() {
45-
exceptionRule.expect(IllegalArgumentException.class);
46-
exceptionRule.expectMessage("url can't null");
47-
ConnectorAction.ActionType actionType = ConnectorAction.ActionType.PREDICT;
48-
String method = "post";
49-
String url = null;
50-
new ConnectorAction(actionType, method, url, null, null, null, null);
49+
Throwable exception = assertThrows(
50+
IllegalArgumentException.class,
51+
() -> new ConnectorAction(TEST_ACTION_TYPE, TEST_METHOD_POST, null, null, TEST_REQUEST_BODY, null, null)
52+
);
53+
assertEquals("url can't be null", exception.getMessage());
5154
}
5255

5356
@Test
5457
public void constructor_NullMethod() {
55-
exceptionRule.expect(IllegalArgumentException.class);
56-
exceptionRule.expectMessage("method can't null");
57-
ConnectorAction.ActionType actionType = ConnectorAction.ActionType.PREDICT;
58-
String method = null;
59-
String url = "https://test.com";
60-
new ConnectorAction(actionType, method, url, null, null, null, null);
58+
Throwable exception = assertThrows(
59+
IllegalArgumentException.class,
60+
() -> new ConnectorAction(TEST_ACTION_TYPE, null, URL, null, TEST_REQUEST_BODY, null, null)
61+
);
62+
assertEquals("method can't be null", exception.getMessage());
6163
}
6264

6365
@Test
6466
public void writeTo_NullValue() throws IOException {
65-
ConnectorAction.ActionType actionType = ConnectorAction.ActionType.PREDICT;
66-
String method = "http";
67-
String url = "https://test.com";
68-
ConnectorAction action = new ConnectorAction(actionType, method, url, null, null, null, null);
67+
ConnectorAction action = new ConnectorAction(TEST_ACTION_TYPE, TEST_METHOD_HTTP, URL, null, TEST_REQUEST_BODY, null, null);
6968
BytesStreamOutput output = new BytesStreamOutput();
7069
action.writeTo(output);
7170
ConnectorAction action2 = new ConnectorAction(output.bytes().streamInput());
72-
Assert.assertEquals(action, action2);
71+
assertEquals(action, action2);
7372
}
7473

7574
@Test
7675
public void writeTo() throws IOException {
77-
ConnectorAction.ActionType actionType = ConnectorAction.ActionType.PREDICT;
78-
String method = "http";
79-
String url = "https://test.com";
8076
Map<String, String> headers = new HashMap<>();
8177
headers.put("key1", "value1");
82-
String requestBody = "{\"input\": \"${parameters.input}\"}";
8378
String preProcessFunction = MLPreProcessFunction.TEXT_DOCS_TO_OPENAI_EMBEDDING_INPUT;
8479
String postProcessFunction = MLPostProcessFunction.OPENAI_EMBEDDING;
8580

8681
ConnectorAction action = new ConnectorAction(
87-
actionType,
88-
method,
89-
url,
82+
TEST_ACTION_TYPE,
83+
TEST_METHOD_HTTP,
84+
URL,
9085
headers,
91-
requestBody,
86+
TEST_REQUEST_BODY,
9287
preProcessFunction,
9388
postProcessFunction
9489
);
9590
BytesStreamOutput output = new BytesStreamOutput();
9691
action.writeTo(output);
9792
ConnectorAction action2 = new ConnectorAction(output.bytes().streamInput());
98-
Assert.assertEquals(action, action2);
93+
assertEquals(action, action2);
9994
}
10095

10196
@Test
10297
public void toXContent_NullValue() throws IOException {
103-
ConnectorAction.ActionType actionType = ConnectorAction.ActionType.PREDICT;
104-
String method = "http";
105-
String url = "https://test.com";
106-
ConnectorAction action = new ConnectorAction(actionType, method, url, null, null, null, null);
98+
ConnectorAction action = new ConnectorAction(TEST_ACTION_TYPE, TEST_METHOD_HTTP, URL, null, TEST_REQUEST_BODY, null, null);
10799

108100
XContentBuilder builder = XContentBuilder.builder(XContentType.JSON.xContent());
109101
action.toXContent(builder, ToXContent.EMPTY_PARAMS);
110102
String content = TestHelper.xContentBuilderToString(builder);
111-
Assert.assertEquals("{\"action_type\":\"PREDICT\",\"method\":\"http\",\"url\":\"https://test.com\"}", content);
103+
String expctedContent = """
104+
{"action_type":"PREDICT","method":"http","url":"https://test.com",\
105+
"request_body":"{\\"input\\": \\"${parameters.input}\\"}"}\
106+
""";
107+
assertEquals(expctedContent, content);
112108
}
113109

114110
@Test
115111
public void toXContent() throws IOException {
116-
ConnectorAction.ActionType actionType = ConnectorAction.ActionType.PREDICT;
117-
String method = "http";
118-
String url = "https://test.com";
119112
Map<String, String> headers = new HashMap<>();
120113
headers.put("key1", "value1");
121-
String requestBody = "{\"input\": \"${parameters.input}\"}";
122114
String preProcessFunction = MLPreProcessFunction.TEXT_DOCS_TO_OPENAI_EMBEDDING_INPUT;
123115
String postProcessFunction = MLPostProcessFunction.OPENAI_EMBEDDING;
124116

125117
ConnectorAction action = new ConnectorAction(
126-
actionType,
127-
method,
128-
url,
118+
TEST_ACTION_TYPE,
119+
TEST_METHOD_HTTP,
120+
URL,
129121
headers,
130-
requestBody,
122+
TEST_REQUEST_BODY,
131123
preProcessFunction,
132124
postProcessFunction
133125
);
134126

135127
XContentBuilder builder = XContentBuilder.builder(XContentType.JSON.xContent());
136128
action.toXContent(builder, ToXContent.EMPTY_PARAMS);
137129
String content = TestHelper.xContentBuilderToString(builder);
138-
Assert
139-
.assertEquals(
140-
"{\"action_type\":\"PREDICT\",\"method\":\"http\",\"url\":\"https://test.com\","
141-
+ "\"headers\":{\"key1\":\"value1\"},\"request_body\":\"{\\\"input\\\": \\\"${parameters.input}\\\"}\","
142-
+ "\"pre_process_function\":\"connector.pre_process.openai.embedding\","
143-
+ "\"post_process_function\":\"connector.post_process.openai.embedding\"}",
144-
content
145-
);
130+
String expctedContent = """
131+
{"action_type":"PREDICT","method":"http","url":"https://test.com","headers":{"key1":"value1"},\
132+
"request_body":"{\\"input\\": \\"${parameters.input}\\"}",\
133+
"pre_process_function":"connector.pre_process.openai.embedding",\
134+
"post_process_function":"connector.post_process.openai.embedding"}\
135+
""";
136+
assertEquals(expctedContent, content);
146137
}
147138

148139
@Test
149140
public void parse() throws IOException {
150-
String jsonStr = "{\"action_type\":\"PREDICT\",\"method\":\"http\",\"url\":\"https://test.com\","
151-
+ "\"headers\":{\"key1\":\"value1\"},\"request_body\":\"{\\\"input\\\": \\\"${parameters.input}\\\"}\","
152-
+ "\"pre_process_function\":\"connector.pre_process.openai.embedding\","
153-
+ "\"post_process_function\":\"connector.post_process.openai.embedding\"}";
141+
String jsonStr = """
142+
{"action_type":"PREDICT","method":"http","url":"https://test.com","headers":{"key1":"value1"},\
143+
"request_body":"{\\"input\\": \\"${parameters.input}\\"}",\
144+
"pre_process_function":"connector.pre_process.openai.embedding",\
145+
"post_process_function":"connector.post_process.openai.embedding"}"\
146+
""";
154147
XContentParser parser = XContentType.JSON
155148
.xContent()
156149
.createParser(
@@ -160,24 +153,23 @@ public void parse() throws IOException {
160153
);
161154
parser.nextToken();
162155
ConnectorAction action = ConnectorAction.parse(parser);
163-
Assert.assertEquals("http", action.getMethod());
164-
Assert.assertEquals(ConnectorAction.ActionType.PREDICT, action.getActionType());
165-
Assert.assertEquals("https://test.com", action.getUrl());
166-
Assert.assertEquals("{\"input\": \"${parameters.input}\"}", action.getRequestBody());
167-
Assert.assertEquals("connector.pre_process.openai.embedding", action.getPreProcessFunction());
168-
Assert.assertEquals("connector.post_process.openai.embedding", action.getPostProcessFunction());
156+
assertEquals(TEST_METHOD_HTTP, action.getMethod());
157+
assertEquals(ConnectorAction.ActionType.PREDICT, action.getActionType());
158+
assertEquals(URL, action.getUrl());
159+
assertEquals(TEST_REQUEST_BODY, action.getRequestBody());
160+
assertEquals("connector.pre_process.openai.embedding", action.getPreProcessFunction());
161+
assertEquals("connector.post_process.openai.embedding", action.getPostProcessFunction());
169162
}
170163

171164
@Test
172165
public void test_wrongActionType() {
173-
exceptionRule.expect(IllegalArgumentException.class);
174-
exceptionRule.expectMessage("Wrong Action Type");
175-
ConnectorAction.ActionType.from("badAction");
166+
Throwable exception = assertThrows(IllegalArgumentException.class, () -> { ConnectorAction.ActionType.from("badAction"); });
167+
assertEquals("Wrong Action Type of badAction", exception.getMessage());
176168
}
177169

178170
@Test
179171
public void test_invalidActionInModelPrediction() {
180172
ConnectorAction.ActionType actionType = ConnectorAction.ActionType.from("execute");
181-
Assert.assertEquals(isValidActionInModelPrediction(actionType), false);
173+
assertEquals(isValidActionInModelPrediction(actionType), false);
182174
}
183175
}

0 commit comments

Comments
 (0)