From e0288f147bb2904fd36fa6701dcccf93a8d562ee Mon Sep 17 00:00:00 2001 From: Sohaib Iftikhar Date: Tue, 29 May 2018 13:59:58 +0200 Subject: [PATCH] extended AbstractStreamableXContentTestCase -- Also fixed strings -- removed getPipelineConfigs method and switched to pipelines() method. --- .../elasticsearch/client/ClusterClient.java | 2 +- .../elasticsearch/client/ClusterClientIT.java | 6 +- .../ClusterClientDocumentationIT.java | 3 +- .../action/ingest/GetPipelineResponse.java | 59 +++++++++++--- .../ingest/GetPipelineResponseTests.java | 76 ++++++++++++++----- 5 files changed, 112 insertions(+), 34 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/ClusterClient.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/ClusterClient.java index 34a1b986e45c3..34441d4160f4c 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/ClusterClient.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/ClusterClient.java @@ -94,7 +94,7 @@ public void putPipelineAsync(PutPipelineRequest request, ActionListener * See - * Get Pipeline API on elastic.co + * Get Pipeline API on elastic.co */ public GetPipelineResponse getPipeline(GetPipelineRequest request, Header... headers) throws IOException { return restHighLevelClient.performRequestAndParseEntity( request, RequestConverters::getPipeline, diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/ClusterClientIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/ClusterClientIT.java index 0eef1cb139b87..caab4c282f4d4 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/ClusterClientIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/ClusterClientIT.java @@ -127,7 +127,7 @@ public void testPutPipeline() throws IOException { } public void testGetPipeline() throws IOException { - String id = "get_pipeline_id"; + String id = "some_pipeline_id"; XContentBuilder pipelineBuilder = buildRandomXContentPipeline(); { PutPipelineRequest request = new PutPipelineRequest( @@ -143,9 +143,9 @@ public void testGetPipeline() throws IOException { GetPipelineResponse response = execute(request, highLevelClient().cluster()::getPipeline, highLevelClient().cluster()::getPipelineAsync); assertTrue(response.isFound()); - assertEquals(response.getPipelineConfigs().get(0).getId(), id); + assertEquals(response.pipelines().get(0).getId(), id); PipelineConfiguration expectedConfig = new PipelineConfiguration(id, BytesReference.bytes(pipelineBuilder), pipelineBuilder.contentType()); - assertEquals(expectedConfig.getConfigAsMap(), response.getPipelineConfigs().get(0).getConfigAsMap()); + assertEquals(expectedConfig.getConfigAsMap(), response.pipelines().get(0).getConfigAsMap()); } } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/ClusterClientDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/ClusterClientDocumentationIT.java index bbcc314610e41..fef5c67cd461d 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/ClusterClientDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/ClusterClientDocumentationIT.java @@ -42,6 +42,7 @@ import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.Map; +import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -284,7 +285,7 @@ public void testGetPipeline() throws IOException { // tag::get-pipeline-response boolean successful = response.isFound(); // <1> - List pipelines = response.getPipelineConfigs(); // <2> + List pipelines = response.pipelines(); // <2> for(PipelineConfiguration pipeline: pipelines) { Map config = pipeline.getConfigAsMap(); // <3> } diff --git a/server/src/main/java/org/elasticsearch/action/ingest/GetPipelineResponse.java b/server/src/main/java/org/elasticsearch/action/ingest/GetPipelineResponse.java index 726e3b6f71eb7..7716084c8e346 100644 --- a/server/src/main/java/org/elasticsearch/action/ingest/GetPipelineResponse.java +++ b/server/src/main/java/org/elasticsearch/action/ingest/GetPipelineResponse.java @@ -34,6 +34,8 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Map; +import java.util.HashMap; import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken; @@ -48,8 +50,13 @@ public GetPipelineResponse(List pipelines) { this.pipelines = pipelines; } + /** + * Get the list of pipelines that were a part of this response. + * The pipeline id can be obtained using getId on the PipelineConfiguration object. + * @return A list of {@link PipelineConfiguration} objects. + */ public List pipelines() { - return pipelines; + return Collections.unmodifiableList(pipelines); } @Override @@ -80,15 +87,6 @@ public RestStatus status() { return isFound() ? RestStatus.OK : RestStatus.NOT_FOUND; } - /** - * Get the list of pipelines that were a part of this response - * The pipeline id can be obtained using - * @return A list of PipelineConfiguration objects. - */ - public List getPipelineConfigs() { - return Collections.unmodifiableList(pipelines); - } - @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); @@ -103,7 +101,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws * * @param parser the parser for the XContent that contains the serialized GetPipelineResponse. * @return an instance of GetPipelineResponse read from the parser - * @throws IOException + * @throws IOException If the parsing fails */ public static GetPipelineResponse fromXContent(XContentParser parser) throws IOException { ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser::getTokenLocation); @@ -122,4 +120,43 @@ public static GetPipelineResponse fromXContent(XContentParser parser) throws IOE ensureExpectedToken(XContentParser.Token.END_OBJECT, parser.currentToken(), parser::getTokenLocation); return new GetPipelineResponse(pipelines); } + + @Override + public boolean equals(Object other) { + if (other == null) { + return false; + } else if (other instanceof GetPipelineResponse){ + GetPipelineResponse otherResponse = (GetPipelineResponse)other; + if (pipelines == null) { + return otherResponse.pipelines == null; + } else { + // We need a map here because order does not matter for equality + Map otherPipelineMap = new HashMap<>(); + for (PipelineConfiguration pipeline: otherResponse.pipelines) { + otherPipelineMap.put(pipeline.getId(), pipeline); + } + for (PipelineConfiguration pipeline: pipelines) { + PipelineConfiguration otherPipeline = otherPipelineMap.get(pipeline.getId()); + if (otherPipeline == null || + !pipeline.getConfigAsMap().equals(otherPipeline.getConfigAsMap())) { + return false; + } + } + return true; + } + } else { + return false; + } + } + + @Override + public int hashCode() { + int result = 1; + for (PipelineConfiguration pipeline: pipelines) { + // We only take the sum here to ensure that the order does not matter. + result += (pipeline == null ? 0 : pipeline.getConfigAsMap().hashCode()); + } + return result; + } + } diff --git a/server/src/test/java/org/elasticsearch/action/ingest/GetPipelineResponseTests.java b/server/src/test/java/org/elasticsearch/action/ingest/GetPipelineResponseTests.java index 8afd627228a2c..e22318dd29ac7 100644 --- a/server/src/test/java/org/elasticsearch/action/ingest/GetPipelineResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/ingest/GetPipelineResponseTests.java @@ -26,7 +26,7 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.ingest.PipelineConfiguration; -import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.AbstractStreamableXContentTestCase; import java.io.IOException; import java.util.ArrayList; @@ -34,34 +34,37 @@ import java.util.List; import java.util.Map; -public class GetPipelineResponseTests extends ESTestCase { +public class GetPipelineResponseTests extends AbstractStreamableXContentTestCase { private XContentBuilder getRandomXContentBuilder() throws IOException { XContentType xContentType = randomFrom(XContentType.values()); return XContentBuilder.builder(xContentType.xContent()); } + private PipelineConfiguration createRandomPipeline(String pipelineId) throws IOException { + String field = "field_" + randomInt(); + String value = "value_" + randomInt(); + XContentBuilder builder = getRandomXContentBuilder(); + builder.startObject(); + // We only use a single SetProcessor here in each pipeline to test. + // Since the contents are returned as a configMap anyway this does not matter for fromXContent + builder.startObject("set"); + builder.field("field", field); + builder.field("value", value); + builder.endObject(); + builder.endObject(); + return + new PipelineConfiguration( + pipelineId, BytesReference.bytes(builder), builder.contentType() + ); + } + private Map createPipelineConfigMap() throws IOException { int numPipelines = randomInt(5); Map pipelinesMap = new HashMap<>(); for (int i=0; i(createPipelineConfigMap().values())); + } catch (IOException e) { + // If we fail to construct an instance we return `null` which would make the user of this method + // fail the test. + return null; + } + } + + @Override + protected boolean supportsUnknownFields() { + return false; + } + + @Override + protected GetPipelineResponse mutateInstance(GetPipelineResponse response) { + try { + ArrayList clonePipelines = new ArrayList<>(response.pipelines()); + clonePipelines.add(createRandomPipeline("pipeline_" + clonePipelines.size() + 1)); + return new GetPipelineResponse(clonePipelines); + } catch (IOException e) { + return null; + } + } }