From f2acf466aae3c6931ca00dbdc154fe26f6bdd2da Mon Sep 17 00:00:00 2001 From: javanna Date: Wed, 22 Feb 2017 14:47:46 +0100 Subject: [PATCH] Convert script/template objects to json format Elasticsearch accepts multiple content-type formats, hence scripts can be stored/provided in json, yaml, cbor or smile. Yet the format that should be used internally is json. This is a problem mainly around search templates, as they only support json out of the four content-types, so instead of maintaining the content-type of the request we should rather convert the scripts/templates to json. Binary formats were not previously supported. If you stored a template in yaml format, you'd get back an error "No encoder found for MIME type [application/yaml]" when trying to execute it. With this commit the request content-type is independent from the template, which always gets converted to json internally. That is transparent to users and doesn't affect the content type of the response obtained when executing the template. --- .../java/org/elasticsearch/script/Script.java | 7 ++++--- .../script/StoredScriptSource.java | 20 +++++++++---------- .../org/elasticsearch/script/ScriptTests.java | 18 ++++++++--------- .../mustache/RestSearchTemplateAction.java | 5 +++-- 4 files changed, 25 insertions(+), 25 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/script/Script.java b/core/src/main/java/org/elasticsearch/script/Script.java index c6a7319372b..9f8a774398c 100644 --- a/core/src/main/java/org/elasticsearch/script/Script.java +++ b/core/src/main/java/org/elasticsearch/script/Script.java @@ -169,9 +169,10 @@ public final class Script implements ToXContentObject, Writeable { type = ScriptType.INLINE; if (parser.currentToken() == Token.START_OBJECT) { - XContentBuilder builder = XContentFactory.contentBuilder(parser.contentType()); - idOrCode = builder.copyCurrentStructure(parser).bytes().utf8ToString(); - options.put(CONTENT_TYPE_OPTION, parser.contentType().mediaType()); + //this is really for search templates, that need to be converted to json format + XContentBuilder builder = XContentFactory.jsonBuilder(); + idOrCode = builder.copyCurrentStructure(parser).string(); + options.put(CONTENT_TYPE_OPTION, XContentType.JSON.mediaType()); } else { idOrCode = parser.text(); } diff --git a/core/src/main/java/org/elasticsearch/script/StoredScriptSource.java b/core/src/main/java/org/elasticsearch/script/StoredScriptSource.java index e6c5b09362c..11b78213908 100644 --- a/core/src/main/java/org/elasticsearch/script/StoredScriptSource.java +++ b/core/src/main/java/org/elasticsearch/script/StoredScriptSource.java @@ -37,7 +37,6 @@ import org.elasticsearch.common.xcontent.ObjectParser.ValueType; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentParser.Token; import org.elasticsearch.common.xcontent.XContentType; @@ -107,9 +106,10 @@ public class StoredScriptSource extends AbstractDiffable imp private void setCode(XContentParser parser) { try { if (parser.currentToken() == Token.START_OBJECT) { - XContentBuilder builder = XContentFactory.contentBuilder(parser.contentType()); - code = builder.copyCurrentStructure(parser).bytes().utf8ToString(); - options.put(Script.CONTENT_TYPE_OPTION, parser.contentType().mediaType()); + //this is really for search templates, that need to be converted to json format + XContentBuilder builder = XContentFactory.jsonBuilder(); + code = builder.copyCurrentStructure(parser).string(); + options.put(Script.CONTENT_TYPE_OPTION, XContentType.JSON.mediaType()); } else { code = parser.text(); } @@ -263,11 +263,11 @@ public class StoredScriptSource extends AbstractDiffable imp if (lang == null) { return PARSER.apply(parser, null).build(); } else { - try (XContentBuilder builder = XContentFactory.contentBuilder(parser.contentType())) { + //this is really for search templates, that need to be converted to json format + try (XContentBuilder builder = XContentFactory.jsonBuilder()) { builder.copyCurrentStructure(parser); - return new StoredScriptSource(lang, builder.string(), - Collections.singletonMap(Script.CONTENT_TYPE_OPTION, parser.contentType().mediaType())); + Collections.singletonMap(Script.CONTENT_TYPE_OPTION, XContentType.JSON.mediaType())); } } @@ -284,11 +284,11 @@ public class StoredScriptSource extends AbstractDiffable imp if (token == Token.VALUE_STRING) { return new StoredScriptSource(lang, parser.text(), - Collections.singletonMap(Script.CONTENT_TYPE_OPTION, parser.contentType().mediaType())); + Collections.singletonMap(Script.CONTENT_TYPE_OPTION, XContentType.JSON.mediaType())); } } - try (XContentBuilder builder = XContentFactory.contentBuilder(parser.contentType())) { + try (XContentBuilder builder = XContentFactory.jsonBuilder()) { if (token != Token.START_OBJECT) { builder.startObject(); builder.copyCurrentStructure(parser); @@ -298,7 +298,7 @@ public class StoredScriptSource extends AbstractDiffable imp } return new StoredScriptSource(lang, builder.string(), - Collections.singletonMap(Script.CONTENT_TYPE_OPTION, parser.contentType().mediaType())); + Collections.singletonMap(Script.CONTENT_TYPE_OPTION, XContentType.JSON.mediaType())); } } } catch (IOException ioe) { diff --git a/core/src/test/java/org/elasticsearch/script/ScriptTests.java b/core/src/test/java/org/elasticsearch/script/ScriptTests.java index fc841bd1648..70c5af00f89 100644 --- a/core/src/test/java/org/elasticsearch/script/ScriptTests.java +++ b/core/src/test/java/org/elasticsearch/script/ScriptTests.java @@ -22,8 +22,8 @@ package org.elasticsearch.script; import org.elasticsearch.common.io.stream.InputStreamStreamInput; import org.elasticsearch.common.io.stream.OutputStreamStreamOutput; import org.elasticsearch.common.xcontent.ToXContent; -import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.test.ESTestCase; @@ -39,9 +39,8 @@ import static org.hamcrest.Matchers.equalTo; public class ScriptTests extends ESTestCase { public void testScriptParsing() throws IOException { - XContent xContent = randomFrom(XContentType.JSON, XContentType.YAML).xContent(); - Script expectedScript = createScript(xContent); - try (XContentBuilder builder = XContentBuilder.builder(xContent)) { + Script expectedScript = createScript(); + try (XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values()))) { expectedScript.toXContent(builder, ToXContent.EMPTY_PARAMS); try (XContentParser parser = createParser(builder)) { Script actualScript = Script.parse(parser); @@ -51,8 +50,7 @@ public class ScriptTests extends ESTestCase { } public void testScriptSerialization() throws IOException { - XContent xContent = randomFrom(XContentType.JSON, XContentType.YAML).xContent(); - Script expectedScript = createScript(xContent); + Script expectedScript = createScript(); try (ByteArrayOutputStream out = new ByteArrayOutputStream()) { expectedScript.writeTo(new OutputStreamStreamOutput(out)); try (ByteArrayInputStream in = new ByteArrayInputStream(out.toByteArray())) { @@ -62,12 +60,12 @@ public class ScriptTests extends ESTestCase { } } - private Script createScript(XContent xContent) throws IOException { + private Script createScript() throws IOException { final Map params = randomBoolean() ? Collections.emptyMap() : Collections.singletonMap("key", "value"); ScriptType scriptType = randomFrom(ScriptType.values()); String script; if (scriptType == ScriptType.INLINE) { - try (XContentBuilder builder = XContentBuilder.builder(xContent)) { + try (XContentBuilder builder = XContentFactory.jsonBuilder()) { builder.startObject(); builder.field("field", randomAsciiOfLengthBetween(1, 5)); builder.endObject(); @@ -80,8 +78,8 @@ public class ScriptTests extends ESTestCase { scriptType, scriptType == ScriptType.STORED ? null : randomFrom("_lang1", "_lang2", "_lang3"), script, - scriptType == ScriptType.INLINE ? Collections.singletonMap(Script.CONTENT_TYPE_OPTION, xContent.type().mediaType()) : null, - params + scriptType == ScriptType.INLINE ? + Collections.singletonMap(Script.CONTENT_TYPE_OPTION, XContentType.JSON.mediaType()) : null, params ); } diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestSearchTemplateAction.java b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestSearchTemplateAction.java index a1abe5ad9a6..7d386833a6f 100644 --- a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestSearchTemplateAction.java +++ b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestSearchTemplateAction.java @@ -66,8 +66,9 @@ public class RestSearchTemplateAction extends BaseRestHandler { PARSER.declareField((parser, request, value) -> { request.setScriptType(ScriptType.INLINE); if (parser.currentToken() == XContentParser.Token.START_OBJECT) { - try (XContentBuilder builder = XContentFactory.contentBuilder(parser.contentType())) { - request.setScript(builder.copyCurrentStructure(parser).bytes().utf8ToString()); + //convert the template to json which is the only supported XContentType (see CustomMustacheFactory#createEncoder) + try (XContentBuilder builder = XContentFactory.jsonBuilder()) { + request.setScript(builder.copyCurrentStructure(parser).string()); } catch (IOException e) { throw new ParsingException(parser.getTokenLocation(), "Could not parse inline template", e); }