Skip to content

Commit 340f344

Browse files
javannajavanna
committed
Consolidate script parsing from object
The update by query action parses a script from an object (map or string). We will need to do the same for runtime fields as they are parsed as part of mappings (elastic#59391). This commit moves the existing parsing of a script from an object from RestUpdateByQueryAction to the Script class. It also adds tests and adjusts some error messages that are incorrect. Also, options were not parsed before and they are now. And unsupported fields are no longer silently ignored.
1 parent 24786ac commit 340f344

File tree

3 files changed

+187
-71
lines changed

3 files changed

+187
-71
lines changed

‎modules/reindex/src/main/java/org/elasticsearch/index/reindex/RestUpdateByQueryAction.java

Lines changed: 1 addition & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -19,23 +19,17 @@
1919

2020
package org.elasticsearch.index.reindex;
2121

22-
import org.elasticsearch.ElasticsearchParseException;
2322
import org.elasticsearch.client.node.NodeClient;
24-
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
2523
import org.elasticsearch.rest.RestRequest;
2624
import org.elasticsearch.script.Script;
27-
import org.elasticsearch.script.ScriptType;
2825

2926
import java.io.IOException;
30-
import java.util.Collections;
3127
import java.util.HashMap;
32-
import java.util.Iterator;
3328
import java.util.List;
3429
import java.util.Map;
3530
import java.util.function.Consumer;
3631

3732
import static org.elasticsearch.rest.RestRequest.Method.POST;
38-
import static org.elasticsearch.script.Script.DEFAULT_SCRIPT_LANG;
3933

4034
public class RestUpdateByQueryAction extends AbstractBulkByQueryRestHandler<UpdateByQueryRequest, UpdateByQueryAction> {
4135

@@ -69,76 +63,12 @@ protected UpdateByQueryRequest buildRequest(RestRequest request) throws IOExcept
6963

7064
Map<String, Consumer<Object>> consumers = new HashMap<>();
7165
consumers.put("conflicts", o -> internal.setConflicts((String) o));
72-
consumers.put("script", o -> internal.setScript(parseScript(o)));
66+
consumers.put("script", o -> internal.setScript(Script.parse(o)));
7367
consumers.put("max_docs", s -> setMaxDocsValidateIdentical(internal, ((Number) s).intValue()));
7468

7569
parseInternalRequest(internal, request, consumers);
7670

7771
internal.setPipeline(request.param("pipeline"));
7872
return internal;
7973
}
80-
81-
@SuppressWarnings("unchecked")
82-
private static Script parseScript(Object config) {
83-
assert config != null : "Script should not be null";
84-
85-
if (config instanceof String) {
86-
return new Script((String) config);
87-
} else if (config instanceof Map) {
88-
Map<String,Object> configMap = (Map<String, Object>) config;
89-
String script = null;
90-
ScriptType type = null;
91-
String lang = null;
92-
Map<String, Object> params = Collections.emptyMap();
93-
for (Iterator<Map.Entry<String, Object>> itr = configMap.entrySet().iterator(); itr.hasNext();) {
94-
Map.Entry<String, Object> entry = itr.next();
95-
String parameterName = entry.getKey();
96-
Object parameterValue = entry.getValue();
97-
if (Script.LANG_PARSE_FIELD.match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
98-
if (parameterValue instanceof String || parameterValue == null) {
99-
lang = (String) parameterValue;
100-
} else {
101-
throw new ElasticsearchParseException("Value must be of type String: [" + parameterName + "]");
102-
}
103-
} else if (Script.PARAMS_PARSE_FIELD.match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
104-
if (parameterValue instanceof Map || parameterValue == null) {
105-
params = (Map<String, Object>) parameterValue;
106-
} else {
107-
throw new ElasticsearchParseException("Value must be of type String: [" + parameterName + "]");
108-
}
109-
} else if (ScriptType.INLINE.getParseField().match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
110-
if (parameterValue instanceof String || parameterValue == null) {
111-
script = (String) parameterValue;
112-
type = ScriptType.INLINE;
113-
} else {
114-
throw new ElasticsearchParseException("Value must be of type String: [" + parameterName + "]");
115-
}
116-
} else if (ScriptType.STORED.getParseField().match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
117-
if (parameterValue instanceof String || parameterValue == null) {
118-
script = (String) parameterValue;
119-
type = ScriptType.STORED;
120-
} else {
121-
throw new ElasticsearchParseException("Value must be of type String: [" + parameterName + "]");
122-
}
123-
}
124-
}
125-
if (script == null) {
126-
throw new ElasticsearchParseException("expected one of [{}] or [{}] fields, but found none",
127-
ScriptType.INLINE.getParseField().getPreferredName(), ScriptType.STORED.getParseField().getPreferredName());
128-
}
129-
assert type != null : "if script is not null, type should definitely not be null";
130-
131-
if (type == ScriptType.STORED) {
132-
if (lang != null) {
133-
throw new IllegalArgumentException("lang cannot be specified for stored scripts");
134-
}
135-
136-
return new Script(type, null, script, null, params);
137-
} else {
138-
return new Script(type, lang == null ? DEFAULT_SCRIPT_LANG : lang, script, params);
139-
}
140-
} else {
141-
throw new IllegalArgumentException("Script value should be a String or a Map");
142-
}
143-
}
14474
}

‎server/src/main/java/org/elasticsearch/script/Script.java

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
package org.elasticsearch.script;
2121

22+
import org.elasticsearch.ElasticsearchParseException;
2223
import org.elasticsearch.common.ParseField;
2324
import org.elasticsearch.common.Strings;
2425
import org.elasticsearch.common.bytes.BytesArray;
@@ -405,6 +406,84 @@ public static Script parse(XContentParser parser, String defaultLang) throws IOE
405406
return PARSER.apply(parser, null).build(defaultLang);
406407
}
407408

409+
/**
410+
* Parse a {@link Script} from an {@link Object}, that can either be a {@link String} or a {@link Map}.
411+
* @see #parse(XContentParser, String)
412+
* @param config The object to parse the script from.
413+
* @return The parsed {@link Script}.
414+
*/
415+
@SuppressWarnings("unchecked")
416+
public static Script parse(Object config) {
417+
Objects.requireNonNull(config, "script must not be null");
418+
if (config instanceof String) {
419+
return new Script((String) config);
420+
} else if (config instanceof Map) {
421+
Map<String,Object> configMap = (Map<String, Object>) config;
422+
String script = null;
423+
ScriptType type = null;
424+
String lang = null;
425+
Map<String, Object> params = Collections.emptyMap();
426+
Map<String, String> options = Collections.emptyMap();
427+
for (Map.Entry<String, Object> entry : configMap.entrySet()) {
428+
String parameterName = entry.getKey();
429+
Object parameterValue = entry.getValue();
430+
if (Script.LANG_PARSE_FIELD.match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
431+
if (parameterValue instanceof String || parameterValue == null) {
432+
lang = (String) parameterValue;
433+
} else {
434+
throw new ElasticsearchParseException("Value must be of type String: [" + parameterName + "]");
435+
}
436+
} else if (Script.PARAMS_PARSE_FIELD.match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
437+
if (parameterValue instanceof Map || parameterValue == null) {
438+
params = (Map<String, Object>) parameterValue;
439+
} else {
440+
throw new ElasticsearchParseException("Value must be of type Map: [" + parameterName + "]");
441+
}
442+
} else if (Script.OPTIONS_PARSE_FIELD.match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
443+
if (parameterValue instanceof Map || parameterValue == null) {
444+
options = (Map<String, String>) parameterValue;
445+
} else {
446+
throw new ElasticsearchParseException("Value must be of type Map: [" + parameterName + "]");
447+
}
448+
} else if (ScriptType.INLINE.getParseField().match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
449+
if (parameterValue instanceof String || parameterValue == null) {
450+
script = (String) parameterValue;
451+
type = ScriptType.INLINE;
452+
} else {
453+
throw new ElasticsearchParseException("Value must be of type String: [" + parameterName + "]");
454+
}
455+
} else if (ScriptType.STORED.getParseField().match(parameterName, LoggingDeprecationHandler.INSTANCE)) {
456+
if (parameterValue instanceof String || parameterValue == null) {
457+
script = (String) parameterValue;
458+
type = ScriptType.STORED;
459+
} else {
460+
throw new ElasticsearchParseException("Value must be of type String: [" + parameterName + "]");
461+
}
462+
} else {
463+
throw new ElasticsearchParseException("Unsupported field [" + parameterName + "]");
464+
}
465+
}
466+
if (script == null) {
467+
throw new ElasticsearchParseException("Expected one of [{}] or [{}] fields, but found none",
468+
ScriptType.INLINE.getParseField().getPreferredName(), ScriptType.STORED.getParseField().getPreferredName());
469+
}
470+
assert type != null : "if script is not null, type should definitely not be null";
471+
472+
if (type == ScriptType.STORED) {
473+
if (lang != null) {
474+
throw new IllegalArgumentException("[" + Script.LANG_PARSE_FIELD.getPreferredName() +
475+
"] cannot be specified for stored scripts");
476+
}
477+
478+
return new Script(type, null, script, null, params);
479+
} else {
480+
return new Script(type, lang == null ? DEFAULT_SCRIPT_LANG : lang, script, options, params);
481+
}
482+
} else {
483+
throw new IllegalArgumentException("Script value should be a String or a Map");
484+
}
485+
}
486+
408487
private final ScriptType type;
409488
private final String lang;
410489
private final String idOrCode;

‎server/src/test/java/org/elasticsearch/script/ScriptTests.java

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,15 @@
1919

2020
package org.elasticsearch.script;
2121

22+
import org.elasticsearch.ElasticsearchParseException;
2223
import org.elasticsearch.common.Strings;
2324
import org.elasticsearch.common.io.stream.InputStreamStreamInput;
2425
import org.elasticsearch.common.io.stream.OutputStreamStreamOutput;
2526
import org.elasticsearch.common.settings.Settings;
2627
import org.elasticsearch.common.xcontent.ToXContent;
2728
import org.elasticsearch.common.xcontent.XContentBuilder;
2829
import org.elasticsearch.common.xcontent.XContentFactory;
30+
import org.elasticsearch.common.xcontent.XContentHelper;
2931
import org.elasticsearch.common.xcontent.XContentParser;
3032
import org.elasticsearch.common.xcontent.XContentType;
3133
import org.elasticsearch.test.ESTestCase;
@@ -34,6 +36,7 @@
3436
import java.io.ByteArrayOutputStream;
3537
import java.io.IOException;
3638
import java.util.Collections;
39+
import java.util.HashMap;
3740
import java.util.Map;
3841

3942
import static org.hamcrest.Matchers.equalTo;
@@ -96,4 +99,108 @@ public void testParse() throws IOException {
9699
}
97100
}
98101
}
102+
103+
public void testParseFromObjectShortSyntax() {
104+
Script script = Script.parse("doc['my_field']");
105+
assertEquals(Script.DEFAULT_SCRIPT_LANG, script.getLang());
106+
assertEquals("doc['my_field']", script.getIdOrCode());
107+
assertEquals(0, script.getParams().size());
108+
assertEquals(0, script.getOptions().size());
109+
assertEquals(ScriptType.INLINE, script.getType());
110+
}
111+
112+
public void testParseFromObject() {
113+
Map<String, Object> map = new HashMap<>();
114+
map.put("source", "doc['my_field']");
115+
Map<String, Object> params = new HashMap<>();
116+
int numParams = randomIntBetween(0, 3);
117+
for (int i = 0; i < numParams; i++) {
118+
params.put("param" + i, i);
119+
}
120+
map.put("params", params);
121+
Map<String, String> options = new HashMap<>();
122+
int numOptions = randomIntBetween(0, 3);
123+
for (int i = 0; i < numOptions; i++) {
124+
options.put("option" + i, Integer.toString(i));
125+
}
126+
map.put("options", options);
127+
String lang = Script.DEFAULT_SCRIPT_LANG;;
128+
if (randomBoolean()) {
129+
map.put("lang", lang);
130+
} else if(randomBoolean()) {
131+
lang = "expression";
132+
map.put("lang", lang);
133+
}
134+
135+
Script script = Script.parse(map);
136+
assertEquals(lang, script.getLang());
137+
assertEquals("doc['my_field']", script.getIdOrCode());
138+
assertEquals(ScriptType.INLINE, script.getType());
139+
assertEquals(params, script.getParams());
140+
assertEquals(options, script.getOptions());
141+
}
142+
143+
public void testParseFromObjectFromScript() {
144+
Map<String, Object> params = new HashMap<>();
145+
int numParams = randomIntBetween(0, 3);
146+
for (int i = 0; i < numParams; i++) {
147+
params.put("param" + i, i);
148+
}
149+
Map<String, String> options = new HashMap<>();
150+
int numOptions = randomIntBetween(0, 3);
151+
for (int i = 0; i < numOptions; i++) {
152+
options.put("option" + i, Integer.toString(i));
153+
}
154+
Script script = new Script(ScriptType.INLINE, Script.DEFAULT_SCRIPT_LANG, "doc['field']", options, params);
155+
Map<String, Object> scriptObject = XContentHelper.convertToMap(XContentType.JSON.xContent(), Strings.toString(script), false);
156+
Script parsedScript = Script.parse(scriptObject);
157+
assertEquals(script, parsedScript);
158+
}
159+
160+
public void testParseFromObjectWrongFormat() {
161+
{
162+
IllegalArgumentException exc = expectThrows(
163+
IllegalArgumentException.class,
164+
() -> Script.parse(3)
165+
);
166+
assertEquals("Script value should be a String or a Map", exc.getMessage());
167+
}
168+
{
169+
ElasticsearchParseException exc = expectThrows(
170+
ElasticsearchParseException.class,
171+
() -> Script.parse(Collections.emptyMap())
172+
);
173+
assertEquals("Expected one of [source] or [id] fields, but found none", exc.getMessage());
174+
}
175+
}
176+
177+
public void testParseFromObjectUnsupportedFields() {
178+
ElasticsearchParseException exc = expectThrows(
179+
ElasticsearchParseException.class,
180+
() -> Script.parse(Map.of("source", "script", "unsupported", "value"))
181+
);
182+
assertEquals("Unsupported field [unsupported]", exc.getMessage());
183+
}
184+
185+
public void testParseFromObjectWrongOptionsFormat() {
186+
Map<String, Object> map = new HashMap<>();
187+
map.put("source", "doc['my_field']");
188+
map.put("options", 3);
189+
ElasticsearchParseException exc = expectThrows(
190+
ElasticsearchParseException.class,
191+
() -> Script.parse(map)
192+
);
193+
assertEquals("Value must be of type Map: [options]", exc.getMessage());
194+
}
195+
196+
public void testParseFromObjectWrongParamsFormat() {
197+
Map<String, Object> map = new HashMap<>();
198+
map.put("source", "doc['my_field']");
199+
map.put("params", 3);
200+
ElasticsearchParseException exc = expectThrows(
201+
ElasticsearchParseException.class,
202+
() -> Script.parse(map)
203+
);
204+
assertEquals("Value must be of type Map: [params]", exc.getMessage());
205+
}
99206
}

0 commit comments

Comments
 (0)