From 309e4a11b5060e290e6b2ab0bce6e61a421b5cd5 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 9 May 2019 13:08:33 +0100 Subject: [PATCH] Cut AnalyzeResponse over to Writeable (#41915) This commit makes AnalyzeResponse and its various helper classes implement Writeable. The classes are also now immutable. Relates to #34389 --- .../admin/indices/analyze/AnalyzeAction.java | 8 +- .../indices/analyze/AnalyzeResponse.java | 101 +++++------ .../analyze/DetailAnalyzeResponse.java | 165 +++++++----------- .../analyze/TransportAnalyzeAction.java | 8 +- .../shard/TransportSingleShardAction.java | 21 ++- .../indices/analyze/AnalyzeResponseTests.java | 26 +-- 6 files changed, 156 insertions(+), 173 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/analyze/AnalyzeAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/analyze/AnalyzeAction.java index e2bbd655992..3677cd6cb4e 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/analyze/AnalyzeAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/analyze/AnalyzeAction.java @@ -20,6 +20,7 @@ package org.elasticsearch.action.admin.indices.analyze; import org.elasticsearch.action.Action; +import org.elasticsearch.common.io.stream.Writeable; public class AnalyzeAction extends Action { @@ -30,8 +31,13 @@ public class AnalyzeAction extends Action { super(NAME); } + @Override + public Writeable.Reader getResponseReader() { + return AnalyzeResponse::new; + } + @Override public AnalyzeResponse newResponse() { - return new AnalyzeResponse(); + throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable"); } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/analyze/AnalyzeResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/analyze/AnalyzeResponse.java index e571db951cb..945c2128bab 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/analyze/AnalyzeResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/analyze/AnalyzeResponse.java @@ -23,7 +23,7 @@ import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.io.stream.Streamable; +import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -43,17 +43,14 @@ import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpect public class AnalyzeResponse extends ActionResponse implements Iterable, ToXContentObject { - public static class AnalyzeToken implements Streamable, ToXContentObject { - private String term; - private int startOffset; - private int endOffset; - private int position; - private int positionLength = 1; - private Map attributes; - private String type; - - AnalyzeToken() { - } + public static class AnalyzeToken implements Writeable, ToXContentObject { + private final String term; + private final int startOffset; + private final int endOffset; + private final int position; + private final int positionLength; + private final Map attributes; + private final String type; @Override public boolean equals(Object o) { @@ -85,6 +82,21 @@ public class AnalyzeResponse extends ActionResponse implements Iterable tokens; - - AnalyzeResponse() { - } + private final List tokens; public AnalyzeResponse(List tokens, DetailAnalyzeResponse detail) { this.tokens = tokens; this.detail = detail; } + public AnalyzeResponse(StreamInput in) throws IOException { + super.readFrom(in); + int size = in.readVInt(); + if (size > 0) { + tokens = new ArrayList<>(size); + for (int i = 0; i < size; i++) { + tokens.add(new AnalyzeToken(in)); + } + } + else { + tokens = null; + } + detail = in.readOptionalWriteable(DetailAnalyzeResponse::new); + } + + @Override + public void readFrom(StreamInput in) throws IOException { + throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable"); + } + public List getTokens() { return this.tokens; } @@ -268,20 +275,6 @@ public class AnalyzeResponse extends ActionResponse implements Iterable(size); - for (int i = 0; i < size; i++) { - tokens.add(AnalyzeToken.readAnalyzeToken(in)); - } - if (tokens.size() == 0) { - tokens = null; - } - detail = in.readOptionalStreamable(DetailAnalyzeResponse::new); - } - @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); @@ -293,7 +286,7 @@ public class AnalyzeResponse extends ActionResponse implements Iterable 0) { + charfilters = new CharFilteredText[size]; + for (int i = 0; i < size; i++) { + charfilters[i] = new CharFilteredText(in); + } + } + else { + charfilters = null; + } + size = in.readVInt(); + if (size > 0) { + tokenfilters = new AnalyzeTokenList[size]; + for (int i = 0; i < size; i++) { + tokenfilters[i] = new AnalyzeTokenList(in); + } + } + else { + tokenfilters = null; + } + analyzer = null; + } else { + analyzer = new AnalyzeTokenList(in); + tokenfilters = null; + tokenizer = null; + charfilters = null; + } } - public DetailAnalyzeResponse analyzer(AnalyzeTokenList analyzer) { - this.customAnalyzer = false; - this.analyzer = analyzer; - return this; + public AnalyzeTokenList analyzer() { + return this.analyzer; } public CharFilteredText[] charfilters() { return this.charfilters; } - public DetailAnalyzeResponse charfilters(CharFilteredText[] charfilters) { - this.customAnalyzer = true; - this.charfilters = charfilters; - return this; - } - public AnalyzeTokenList tokenizer() { return tokenizer; } - public DetailAnalyzeResponse tokenizer(AnalyzeTokenList tokenizer) { - this.customAnalyzer = true; - this.tokenizer = tokenizer; - return this; - } - public AnalyzeTokenList[] tokenfilters() { return tokenfilters; } - public DetailAnalyzeResponse tokenfilters(AnalyzeTokenList[] tokenfilters) { - this.customAnalyzer = true; - this.tokenfilters = tokenfilters; - return this; - } - @Override public boolean equals(Object o) { if (this == o) return true; @@ -201,30 +207,6 @@ public class DetailAnalyzeResponse implements Streamable, ToXContentFragment { static final String TOKENFILTERS = "tokenfilters"; } - @Override - public void readFrom(StreamInput in) throws IOException { - this.customAnalyzer = in.readBoolean(); - if (customAnalyzer) { - tokenizer = AnalyzeTokenList.readAnalyzeTokenList(in); - int size = in.readVInt(); - if (size > 0) { - charfilters = new CharFilteredText[size]; - for (int i = 0; i < size; i++) { - charfilters[i] = CharFilteredText.readCharFilteredText(in); - } - } - size = in.readVInt(); - if (size > 0) { - tokenfilters = new AnalyzeTokenList[size]; - for (int i = 0; i < size; i++) { - tokenfilters[i] = AnalyzeTokenList.readAnalyzeTokenList(in); - } - } - } else { - analyzer = AnalyzeTokenList.readAnalyzeTokenList(in); - } - } - @Override public void writeTo(StreamOutput out) throws IOException { out.writeBoolean(customAnalyzer); @@ -251,9 +233,9 @@ public class DetailAnalyzeResponse implements Streamable, ToXContentFragment { } } - public static class AnalyzeTokenList implements Streamable, ToXContentObject { - private String name; - private AnalyzeResponse.AnalyzeToken[] tokens; + public static class AnalyzeTokenList implements Writeable, ToXContentObject { + private final String name; + private final AnalyzeResponse.AnalyzeToken[] tokens; @Override public boolean equals(Object o) { @@ -271,14 +253,25 @@ public class DetailAnalyzeResponse implements Streamable, ToXContentFragment { return result; } - AnalyzeTokenList() { - } - public AnalyzeTokenList(String name, AnalyzeResponse.AnalyzeToken[] tokens) { this.name = name; this.tokens = tokens; } + public AnalyzeTokenList(StreamInput in) throws IOException { + name = in.readString(); + int size = in.readVInt(); + if (size > 0) { + tokens = new AnalyzeResponse.AnalyzeToken[size]; + for (int i = 0; i < size; i++) { + tokens[i] = new AnalyzeResponse.AnalyzeToken(in); + } + } + else { + tokens = null; + } + } + public String getName() { return name; } @@ -287,12 +280,6 @@ public class DetailAnalyzeResponse implements Streamable, ToXContentFragment { return tokens; } - public static AnalyzeTokenList readAnalyzeTokenList(StreamInput in) throws IOException { - AnalyzeTokenList list = new AnalyzeTokenList(); - list.readFrom(in); - return list; - } - XContentBuilder toXContentWithoutObject(XContentBuilder builder, Params params) throws IOException { builder.field(Fields.NAME, this.name); builder.startArray(AnalyzeResponse.Fields.TOKENS); @@ -327,18 +314,6 @@ public class DetailAnalyzeResponse implements Streamable, ToXContentFragment { return PARSER.parse(parser, null); } - @Override - public void readFrom(StreamInput in) throws IOException { - name = in.readString(); - int size = in.readVInt(); - if (size > 0) { - tokens = new AnalyzeResponse.AnalyzeToken[size]; - for (int i = 0; i < size; i++) { - tokens[i] = AnalyzeResponse.AnalyzeToken.readAnalyzeToken(in); - } - } - } - @Override public void writeTo(StreamOutput out) throws IOException { out.writeString(name); @@ -353,12 +328,9 @@ public class DetailAnalyzeResponse implements Streamable, ToXContentFragment { } } - public static class CharFilteredText implements Streamable, ToXContentObject { - private String name; - private String[] texts; - - CharFilteredText() { - } + public static class CharFilteredText implements Writeable, ToXContentObject { + private final String name; + private final String[] texts; public CharFilteredText(String name, String[] texts) { this.name = name; @@ -369,6 +341,11 @@ public class DetailAnalyzeResponse implements Streamable, ToXContentFragment { } } + public CharFilteredText(StreamInput in) throws IOException { + name = in.readString(); + texts = in.readStringArray(); + } + public String getName() { return name; } @@ -398,18 +375,6 @@ public class DetailAnalyzeResponse implements Streamable, ToXContentFragment { return PARSER.parse(parser, null); } - public static CharFilteredText readCharFilteredText(StreamInput in) throws IOException { - CharFilteredText text = new CharFilteredText(); - text.readFrom(in); - return text; - } - - @Override - public void readFrom(StreamInput in) throws IOException { - name = in.readString(); - texts = in.readStringArray(); - } - @Override public void writeTo(StreamOutput out) throws IOException { out.writeString(name); diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/analyze/TransportAnalyzeAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/analyze/TransportAnalyzeAction.java index 9538bd4b4d2..07f445b6fc7 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/analyze/TransportAnalyzeAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/analyze/TransportAnalyzeAction.java @@ -40,6 +40,7 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.env.Environment; @@ -97,7 +98,12 @@ public class TransportAnalyzeAction extends TransportSingleShardAction getResponseReader() { + return AnalyzeResponse::new; } @Override diff --git a/server/src/main/java/org/elasticsearch/action/support/single/shard/TransportSingleShardAction.java b/server/src/main/java/org/elasticsearch/action/support/single/shard/TransportSingleShardAction.java index 6d7ad085dcd..8b0e69bd457 100644 --- a/server/src/main/java/org/elasticsearch/action/support/single/shard/TransportSingleShardAction.java +++ b/server/src/main/java/org/elasticsearch/action/support/single/shard/TransportSingleShardAction.java @@ -38,6 +38,7 @@ import org.elasticsearch.cluster.routing.ShardsIterator; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.logging.LoggerMessageFormat; import org.elasticsearch.common.util.concurrent.AbstractRunnable; import org.elasticsearch.index.shard.ShardId; @@ -118,8 +119,18 @@ public abstract class TransportSingleShardAction getResponseReader() { + return in -> { + Response response = newResponse(); + response.readFrom(in); + return response; + }; + } + protected abstract boolean resolveIndex(Request request); protected ClusterBlockException checkGlobalBlock(ClusterState state) { @@ -182,13 +193,12 @@ public abstract class TransportSingleShardAction reader = getResponseReader(); transportService.sendRequest(clusterService.localNode(), transportShardAction, internalRequest.request(), new TransportResponseHandler() { @Override public Response read(StreamInput in) throws IOException { - Response response = newResponse(); - response.readFrom(in); - return response; + return reader.read(in); } @Override @@ -251,14 +261,13 @@ public abstract class TransportSingleShardAction reader = getResponseReader(); transportService.sendRequest(node, transportShardAction, internalRequest.request(), new TransportResponseHandler() { @Override public Response read(StreamInput in) throws IOException { - Response response = newResponse(); - response.readFrom(in); - return response; + return reader.read(in); } @Override diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/analyze/AnalyzeResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/analyze/AnalyzeResponseTests.java index 7f1b7fb41ba..a4cee7a4cde 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/analyze/AnalyzeResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/analyze/AnalyzeResponseTests.java @@ -20,12 +20,13 @@ package org.elasticsearch.action.admin.indices.analyze; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; -import org.elasticsearch.test.AbstractStreamableXContentTestCase; +import org.elasticsearch.test.AbstractSerializingTestCase; import java.io.IOException; import java.util.ArrayList; @@ -37,7 +38,7 @@ import java.util.function.Predicate; import static org.hamcrest.Matchers.equalTo; -public class AnalyzeResponseTests extends AbstractStreamableXContentTestCase { +public class AnalyzeResponseTests extends AbstractSerializingTestCase { @Override protected Predicate getRandomFieldsExcludeFilter() { @@ -50,8 +51,8 @@ public class AnalyzeResponseTests extends AbstractStreamableXContentTestCase instanceReader() { + return AnalyzeResponse::new; } @Override @@ -61,21 +62,24 @@ public class AnalyzeResponseTests extends AbstractStreamableXContentTestCase