Skip to content

Commit 19e1d12

Browse files
liketicliketic
committed
Add support for parsing inline script (elastic#23824)
1 parent 1715fd7 commit 19e1d12

File tree

6 files changed

+65
-68
lines changed

6 files changed

+65
-68
lines changed

‎core/src/main/java/org/elasticsearch/ingest/ConfigurationUtils.java

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import java.util.ArrayList;
3131
import java.util.Arrays;
3232
import java.util.Collections;
33+
import java.util.HashMap;
3334
import java.util.List;
3435
import java.util.Map;
3536

@@ -294,13 +295,13 @@ public static ElasticsearchException newConfigurationException(String processorT
294295
return exception;
295296
}
296297

297-
public static List<Processor> readProcessorConfigs(List<Map<String, Map<String, Object>>> processorConfigs,
298+
public static List<Processor> readProcessorConfigs(List<Map<String, Object>> processorConfigs,
298299
Map<String, Processor.Factory> processorFactories) throws Exception {
299300
Exception exception = null;
300301
List<Processor> processors = new ArrayList<>();
301302
if (processorConfigs != null) {
302-
for (Map<String, Map<String, Object>> processorConfigWithKey : processorConfigs) {
303-
for (Map.Entry<String, Map<String, Object>> entry : processorConfigWithKey.entrySet()) {
303+
for (Map<String, Object> processorConfigWithKey : processorConfigs) {
304+
for (Map.Entry<String, Object> entry : processorConfigWithKey.entrySet()) {
304305
try {
305306
processors.add(readProcessor(processorFactories, entry.getKey(), entry.getValue()));
306307
} catch (Exception e) {
@@ -353,13 +354,28 @@ private static void addHeadersToException(ElasticsearchException exception, Stri
353354
}
354355
}
355356

357+
@SuppressWarnings("unchecked")
358+
public static Processor readProcessor(Map<String, Processor.Factory> processorFactories,
359+
String type, Object config) throws Exception {
360+
if (config instanceof Map) {
361+
return readProcessor(processorFactories, type, (Map<String, Object>) config);
362+
} else if (config instanceof String && "script".equals(type)) {
363+
Map<String, Object> normalizedScript = new HashMap<>(1);
364+
normalizedScript.put(ScriptType.INLINE.getName(), config);
365+
return readProcessor(processorFactories, type, normalizedScript);
366+
} else {
367+
throw newConfigurationException(type, null, null,
368+
"property isn't a map, but of type [" + config.getClass().getName() + "]");
369+
}
370+
}
371+
356372
public static Processor readProcessor(Map<String, Processor.Factory> processorFactories,
357373
String type, Map<String, Object> config) throws Exception {
358374
String tag = ConfigurationUtils.readOptionalStringProperty(null, null, config, TAG_KEY);
359375
Processor.Factory factory = processorFactories.get(type);
360376
if (factory != null) {
361377
boolean ignoreFailure = ConfigurationUtils.readBooleanProperty(null, null, config, "ignore_failure", false);
362-
List<Map<String, Map<String, Object>>> onFailureProcessorConfigs =
378+
List<Map<String, Object>> onFailureProcessorConfigs =
363379
ConfigurationUtils.readOptionalList(null, null, config, Pipeline.ON_FAILURE_KEY);
364380

365381
List<Processor> onFailureProcessors = readProcessorConfigs(onFailureProcessorConfigs, processorFactories);

‎core/src/main/java/org/elasticsearch/ingest/Pipeline.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,9 @@ public static final class Factory {
118118
public Pipeline create(String id, Map<String, Object> config, Map<String, Processor.Factory> processorFactories) throws Exception {
119119
String description = ConfigurationUtils.readOptionalStringProperty(null, null, config, DESCRIPTION_KEY);
120120
Integer version = ConfigurationUtils.readIntProperty(null, null, config, VERSION_KEY, null);
121-
List<Map<String, Map<String, Object>>> processorConfigs = ConfigurationUtils.readList(null, null, config, PROCESSORS_KEY);
121+
List<Map<String, Object>> processorConfigs = ConfigurationUtils.readList(null, null, config, PROCESSORS_KEY);
122122
List<Processor> processors = ConfigurationUtils.readProcessorConfigs(processorConfigs, processorFactories);
123-
List<Map<String, Map<String, Object>>> onFailureProcessorConfigs =
123+
List<Map<String, Object>> onFailureProcessorConfigs =
124124
ConfigurationUtils.readOptionalList(null, null, config, ON_FAILURE_KEY);
125125
List<Processor> onFailureProcessors = ConfigurationUtils.readProcessorConfigs(onFailureProcessorConfigs, processorFactories);
126126
if (config.isEmpty() == false) {

‎core/src/test/java/org/elasticsearch/ingest/ConfigurationUtilsTests.java

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ public void testReadProcessors() throws Exception {
115115
Map<String, Processor.Factory> registry =
116116
Collections.singletonMap("test_processor", (factories, tag, config) -> processor);
117117

118-
List<Map<String, Map<String, Object>>> config = new ArrayList<>();
118+
List<Map<String, Object>> config = new ArrayList<>();
119119
Map<String, Object> emptyConfig = Collections.emptyMap();
120120
config.add(Collections.singletonMap("test_processor", emptyConfig));
121121
config.add(Collections.singletonMap("test_processor", emptyConfig));
@@ -135,7 +135,7 @@ public void testReadProcessors() throws Exception {
135135
assertThat(e.getHeader("processor_type"), equalTo(Collections.singletonList("unknown_processor")));
136136
assertThat(e.getHeader("property_name"), is(nullValue()));
137137

138-
List<Map<String, Map<String, Object>>> config2 = new ArrayList<>();
138+
List<Map<String, Object>> config2 = new ArrayList<>();
139139
unknownTaggedConfig = new HashMap<>();
140140
unknownTaggedConfig.put("tag", "my_unknown");
141141
config2.add(Collections.singletonMap("unknown_processor", unknownTaggedConfig));
@@ -157,4 +157,27 @@ public void testReadProcessors() throws Exception {
157157
assertThat(e2.getHeader("property_name"), is(nullValue()));
158158
}
159159

160+
public void testReadProcessorFromObjectOrMap() throws Exception {
161+
Processor processor = mock(Processor.class);
162+
Map<String, Processor.Factory> registry =
163+
Collections.singletonMap("script", (processorFactories, tag, config) -> {
164+
config.clear();
165+
return processor;
166+
});
167+
168+
Object emptyConfig = Collections.emptyMap();
169+
Processor processor1 = ConfigurationUtils.readProcessor(registry, "script", emptyConfig);
170+
assertThat(processor1, sameInstance(processor));
171+
172+
Object inlineScript = "test_script";
173+
Processor processor2 = ConfigurationUtils.readProcessor(registry, "script", inlineScript);
174+
assertThat(processor2, sameInstance(processor));
175+
176+
Object invalidConfig = 12L;
177+
178+
ElasticsearchParseException ex = expectThrows(ElasticsearchParseException.class,
179+
() -> ConfigurationUtils.readProcessor(registry, "unknown_processor", invalidConfig));
180+
assertThat(ex.getMessage(), equalTo("property isn't a map, but of type [" + invalidConfig.getClass().getName() + "]"));
181+
}
182+
160183
}

‎modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/ScriptProcessor.java

Lines changed: 14 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,12 @@
1919

2020
package org.elasticsearch.ingest.common;
2121

22-
import org.apache.logging.log4j.Logger;
23-
import org.elasticsearch.common.Strings;
24-
import org.elasticsearch.common.logging.DeprecationLogger;
25-
import org.elasticsearch.common.logging.ESLoggerFactory;
22+
import com.fasterxml.jackson.core.JsonFactory;
23+
24+
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
25+
import org.elasticsearch.common.xcontent.XContentBuilder;
26+
import org.elasticsearch.common.xcontent.json.JsonXContent;
27+
import org.elasticsearch.common.xcontent.json.JsonXContentParser;
2628
import org.elasticsearch.ingest.AbstractProcessor;
2729
import org.elasticsearch.ingest.IngestDocument;
2830
import org.elasticsearch.ingest.Processor;
@@ -31,22 +33,18 @@
3133
import org.elasticsearch.script.ScriptException;
3234
import org.elasticsearch.script.ScriptService;
3335

36+
import java.util.Arrays;
3437
import java.util.Map;
3538

36-
import static java.util.Collections.emptyMap;
37-
import static org.elasticsearch.common.Strings.hasLength;
3839
import static org.elasticsearch.ingest.ConfigurationUtils.newConfigurationException;
39-
import static org.elasticsearch.ingest.ConfigurationUtils.readOptionalMap;
40-
import static org.elasticsearch.ingest.ConfigurationUtils.readOptionalStringProperty;
41-
import static org.elasticsearch.script.ScriptType.INLINE;
42-
import static org.elasticsearch.script.ScriptType.STORED;
4340

4441
/**
4542
* Processor that evaluates a script with an ingest document in its context.
4643
*/
4744
public final class ScriptProcessor extends AbstractProcessor {
4845

4946
public static final String TYPE = "script";
47+
private static final JsonFactory JSON_FACTORY = new JsonFactory();
5048

5149
private final Script script;
5250
private final ScriptService scriptService;
@@ -87,66 +85,27 @@ Script getScript() {
8785
}
8886

8987
public static final class Factory implements Processor.Factory {
90-
private final Logger logger = ESLoggerFactory.getLogger(Factory.class);
91-
private final DeprecationLogger deprecationLogger = new DeprecationLogger(logger);
92-
9388
private final ScriptService scriptService;
9489

9590
public Factory(ScriptService scriptService) {
9691
this.scriptService = scriptService;
9792
}
9893

9994
@Override
100-
@SuppressWarnings("unchecked")
10195
public ScriptProcessor create(Map<String, Processor.Factory> registry, String processorTag,
10296
Map<String, Object> config) throws Exception {
103-
String lang = readOptionalStringProperty(TYPE, processorTag, config, "lang");
104-
String source = readOptionalStringProperty(TYPE, processorTag, config, "source");
105-
String id = readOptionalStringProperty(TYPE, processorTag, config, "id");
106-
Map<String, ?> params = readOptionalMap(TYPE, processorTag, config, "params");
107-
108-
if (source == null) {
109-
source = readOptionalStringProperty(TYPE, processorTag, config, "inline");
110-
if (source != null) {
111-
deprecationLogger.deprecated("Specifying script source with [inline] is deprecated, use [source] instead.");
112-
}
113-
}
114-
115-
boolean containsNoScript = !hasLength(id) && !hasLength(source);
116-
if (containsNoScript) {
117-
throw newConfigurationException(TYPE, processorTag, null, "Need [id] or [source] parameter to refer to scripts");
118-
}
119-
120-
boolean moreThanOneConfigured = Strings.hasLength(id) && Strings.hasLength(source);
121-
if (moreThanOneConfigured) {
122-
throw newConfigurationException(TYPE, processorTag, null, "Only one of [id] or [source] may be configured");
123-
}
124-
125-
if (lang == null) {
126-
lang = Script.DEFAULT_SCRIPT_LANG;
127-
}
97+
XContentBuilder builder = XContentBuilder.builder(JsonXContent.jsonXContent).map(config);
98+
JsonXContentParser parser = new JsonXContentParser(NamedXContentRegistry.EMPTY,
99+
JSON_FACTORY.createParser(builder.bytes().streamInput()));
100+
Script script = Script.parse(parser);
128101

129-
if (params == null) {
130-
params = emptyMap();
131-
}
132-
133-
final Script script;
134-
String scriptPropertyUsed;
135-
if (Strings.hasLength(source)) {
136-
script = new Script(INLINE, lang, source, (Map<String, Object>)params);
137-
scriptPropertyUsed = "source";
138-
} else if (Strings.hasLength(id)) {
139-
script = new Script(STORED, null, id, (Map<String, Object>)params);
140-
scriptPropertyUsed = "id";
141-
} else {
142-
throw newConfigurationException(TYPE, processorTag, null, "Could not initialize script");
143-
}
102+
Arrays.asList("id", "source", "inline", "lang", "params", "options").forEach(config::remove);
144103

145104
// verify script is able to be compiled before successfully creating processor.
146105
try {
147106
scriptService.compile(script, ExecutableScript.INGEST_CONTEXT);
148107
} catch (ScriptException e) {
149-
throw newConfigurationException(TYPE, processorTag, scriptPropertyUsed, e);
108+
throw newConfigurationException(TYPE, processorTag, null, e);
150109
}
151110

152111
return new ScriptProcessor(processorTag, script, scriptService);

‎modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,25 +82,25 @@ public void testFactoryValidationForMultipleScriptingTypes() throws Exception {
8282

8383
ElasticsearchException exception = expectThrows(ElasticsearchException.class,
8484
() -> factory.create(null, randomAlphaOfLength(10), configMap));
85-
assertThat(exception.getMessage(), is("Only one of [id] or [source] may be configured"));
85+
assertThat(exception.getMessage(), is("[script] failed to parse field [source]"));
8686
}
8787

8888
public void testFactoryValidationAtLeastOneScriptingType() throws Exception {
8989
Map<String, Object> configMap = new HashMap<>();
9090
configMap.put("lang", "mockscript");
9191

92-
ElasticsearchException exception = expectThrows(ElasticsearchException.class,
92+
IllegalArgumentException exception = expectThrows(IllegalArgumentException.class,
9393
() -> factory.create(null, randomAlphaOfLength(10), configMap));
9494

95-
assertThat(exception.getMessage(), is("Need [id] or [source] parameter to refer to scripts"));
95+
assertThat(exception.getMessage(), is("must specify either [source] for an inline script or [id] for a stored script"));
9696
}
9797

9898
public void testInlineBackcompat() throws Exception {
9999
Map<String, Object> configMap = new HashMap<>();
100100
configMap.put("inline", "code");
101101

102102
factory.create(null, randomAlphaOfLength(10), configMap);
103-
assertWarnings("Specifying script source with [inline] is deprecated, use [source] instead.");
103+
assertWarnings("Deprecated field [inline] used, expected [source] instead");
104104
}
105105

106106
public void testFactoryInvalidateWithInvalidCompiledScript() throws Exception {

‎qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/50_script_processor_using_painless.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,6 @@
104104
]
105105
}
106106
- match: { error.header.processor_type: "script" }
107-
- match: { error.header.property_name: "source" }
108107
- match: { error.type: "script_exception" }
109108
- match: { error.reason: "compile error" }
110109

0 commit comments

Comments
 (0)