From ae48422072d29987f32e88b18bc12b561f4feced Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Thu, 21 Jan 2016 13:59:00 +0100 Subject: [PATCH] Fix IngestMetadata parsing and add unittest --- .../common/xcontent/ObjectParser.java | 8 ++- .../elasticsearch/ingest/IngestMetadata.java | 28 ++++---- .../ingest/PipelineConfiguration.java | 36 +++++------ .../ingest/IngestMetadataTests.java | 64 +++++++++++++++++++ .../suggest/CompletionSuggestSearchIT.java | 2 +- 5 files changed, 103 insertions(+), 35 deletions(-) create mode 100644 core/src/test/java/org/elasticsearch/ingest/IngestMetadataTests.java diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java b/core/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java index 979a1f2522c..395dcad8221 100644 --- a/core/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java +++ b/core/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java @@ -223,7 +223,7 @@ public final class ObjectParser implements BiFunction implements BiFunction consumer.accept(v, objectParser.apply(p, c)), field, ValueType.OBJECT); } + public void declareObjectArray(BiConsumer> consumer, BiFunction objectParser, ParseField field) { + declareField((p, v, c) -> consumer.accept(v, parseArray(p, () -> objectParser.apply(p, c))), field, ValueType.OBJECT_ARRAY); + } + + public void declareObjectOrDefault(BiConsumer consumer, BiFunction objectParser, Supplier defaultValue, ParseField field) { declareField((p, v, c) -> { if (p.currentToken() == XContentParser.Token.VALUE_BOOLEAN) { @@ -333,6 +338,7 @@ public final class ObjectParser implements BiFunction impl public final static String TYPE = "ingest"; public final static IngestMetadata PROTO = new IngestMetadata(); - private final ParseField PIPELINES_FIELD = new ParseField("pipeline"); + private static final ParseField PIPELINES_FIELD = new ParseField("pipeline"); + private static final ObjectParser, Void> INGEST_METADATA_PARSER = new ObjectParser<>("ingest_metadata", ArrayList::new); + + static { + INGEST_METADATA_PARSER.declareObjectArray(List::addAll , PipelineConfiguration.getParser(), PIPELINES_FIELD); + } + // We can't use Pipeline class directly in cluster state, because we don't have the processor factories around when // IngestMetadata is registered as custom metadata. @@ -86,20 +95,11 @@ public final class IngestMetadata extends AbstractDiffable impl @Override public MetaData.Custom fromXContent(XContentParser parser) throws IOException { - ObjectParser ingestMetaDataParser = new ObjectParser<>("ingest_metadata", null); - Map pipelines = new HashMap<>(); - ingestMetaDataParser.declareField((parser1, aVoid, aVoid2) -> { - XContentParser.Token token; - while ((token = parser1.nextToken()) != XContentParser.Token.END_ARRAY) { - if (token == XContentParser.Token.START_OBJECT) { - PipelineConfiguration pipeline = new PipelineConfiguration.Builder(parser1).build(); - pipelines.put(pipeline.getId(), pipeline); - } - } - }, PIPELINES_FIELD, ObjectParser.ValueType.OBJECT); - ingestMetaDataParser.parse(parser); - + List configs = INGEST_METADATA_PARSER.parse(parser); + for (PipelineConfiguration pipeline : configs) { + pipelines.put(pipeline.getId(), pipeline); + } return new IngestMetadata(pipelines); } diff --git a/core/src/main/java/org/elasticsearch/ingest/PipelineConfiguration.java b/core/src/main/java/org/elasticsearch/ingest/PipelineConfiguration.java index 628cf2446cb..90ab2a76c2e 100644 --- a/core/src/main/java/org/elasticsearch/ingest/PipelineConfiguration.java +++ b/core/src/main/java/org/elasticsearch/ingest/PipelineConfiguration.java @@ -35,6 +35,7 @@ import org.elasticsearch.common.xcontent.XContentParser; import java.io.IOException; import java.util.Map; import java.util.Objects; +import java.util.function.BiFunction; /** * Encapsulates a pipeline's id and configuration as a blob @@ -46,36 +47,33 @@ public final class PipelineConfiguration implements Writeable PARSER = new ObjectParser<>("pipeline_config", Builder::new); + static { + PARSER.declareString(Builder::setId, new ParseField("id")); + PARSER.declareField((parser, builder, aVoid) -> { + XContentBuilder contentBuilder = XContentBuilder.builder(parser.contentType().xContent()); + XContentHelper.copyCurrentStructure(contentBuilder.generator(), parser); + builder.setConfig(contentBuilder.bytes()); + }, new ParseField("config"), ObjectParser.ValueType.OBJECT); + } - public static class Builder { - - private final static ObjectParser PARSER = new ObjectParser<>("pipeline_config", null); - - static { - PARSER.declareString(Builder::setId, new ParseField("id")); - PARSER.declareField((parser, builder, aVoid) -> { - XContentBuilder contentBuilder = XContentBuilder.builder(parser.contentType().xContent()); - XContentHelper.copyCurrentEvent(contentBuilder.generator(), parser); - builder.setConfig(contentBuilder.bytes()); - }, new ParseField("config"), ObjectParser.ValueType.OBJECT); - } + public static BiFunction getParser() { + return (p, c) -> PARSER.apply(p ,c).build(); + } + private static class Builder { private String id; private BytesReference config; - public Builder(XContentParser parser) throws IOException { - PARSER.parse(parser, this); - } - - public void setId(String id) { + void setId(String id) { this.id = id; } - public void setConfig(BytesReference config) { + void setConfig(BytesReference config) { this.config = config; } - public PipelineConfiguration build() { + PipelineConfiguration build() { return new PipelineConfiguration(id, config); } } diff --git a/core/src/test/java/org/elasticsearch/ingest/IngestMetadataTests.java b/core/src/test/java/org/elasticsearch/ingest/IngestMetadataTests.java new file mode 100644 index 00000000000..a6cf12389a0 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/ingest/IngestMetadataTests.java @@ -0,0 +1,64 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.ingest; + +import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.xcontent.ToXContent; +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; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; + +public class IngestMetadataTests extends ESTestCase { + + public void testFromXContent() throws IOException { + PipelineConfiguration pipeline = new PipelineConfiguration( + "1",new BytesArray("{\"processors\": [{\"set\" : {\"field\": \"_field\", \"value\": \"_value\"}}]}") + ); + PipelineConfiguration pipeline2 = new PipelineConfiguration( + "2",new BytesArray("{\"processors\": [{\"set\" : {\"field\": \"_field1\", \"value\": \"_value1\"}}]}") + ); + Map map = new HashMap<>(); + map.put(pipeline.getId(), pipeline); + map.put(pipeline2.getId(), pipeline2); + IngestMetadata ingestMetadata = new IngestMetadata(map); + XContentBuilder builder = XContentFactory.jsonBuilder(); + builder.prettyPrint(); + builder.startObject(); + ingestMetadata.toXContent(builder, ToXContent.EMPTY_PARAMS); + builder.endObject(); + String string = builder.string(); + final XContentParser parser = XContentFactory.xContent(XContentType.JSON).createParser(string); + MetaData.Custom custom = ingestMetadata.fromXContent(parser); + assertTrue(custom instanceof IngestMetadata); + IngestMetadata m = (IngestMetadata) custom; + assertEquals(2, m.getPipelines().size()); + assertEquals("1", m.getPipelines().get("1").getId()); + assertEquals("2", m.getPipelines().get("2").getId()); + assertEquals(pipeline.getConfigAsMap(), m.getPipelines().get("1").getConfigAsMap()); + assertEquals(pipeline2.getConfigAsMap(), m.getPipelines().get("2").getConfigAsMap()); + } +} diff --git a/core/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestSearchIT.java b/core/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestSearchIT.java index fac7f71446a..1543433be32 100644 --- a/core/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestSearchIT.java +++ b/core/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestSearchIT.java @@ -230,7 +230,7 @@ public class CompletionSuggestSearchIT extends ESIntegTestCase { SuggestResponse suggestResponse = client().suggest(request).get(); assertThat(suggestResponse.getSuccessfulShards(), equalTo(0)); for (ShardOperationFailedException exception : suggestResponse.getShardFailures()) { - assertThat(exception.reason(), containsString("ParsingException[[completion] failed to parse field [payload]]; nested: IllegalStateException[expected value but got [START_OBJECT]]")); + assertThat(exception.reason(), containsString("ParsingException[[completion] failed to parse field [payload]]; nested: IllegalStateException[Can't get text on a START_OBJECT")); } }