Skip to content

Commit 169cb38

Browse files
authored
Liberalize StreamOutput#writeStringList (#37768)
In some cases we only have a string collection instead of a string list that we want to serialize out. We have a convenience method for writing a list of strings, but no such method for writing a collection of strings. Yet, a list of strings is a collection of strings, so we can simply liberalize StreamOutput#writeStringList to be more generous in the collections that it accepts and write out collections of strings too. On the other side, we do not have a convenience method for reading a list of strings. This commit addresses both of these issues.
1 parent 1c2ae91 commit 169cb38

File tree

29 files changed

+114
-81
lines changed

29 files changed

+114
-81
lines changed

server/src/main/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateRequest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ public void readFrom(StreamInput in) throws IOException {
461461
name = in.readString();
462462

463463
if (in.getVersion().onOrAfter(Version.V_6_0_0_alpha1)) {
464-
indexPatterns = in.readList(StreamInput::readString);
464+
indexPatterns = in.readStringList();
465465
} else {
466466
indexPatterns = Collections.singletonList(in.readString());
467467
}
@@ -495,7 +495,7 @@ public void writeTo(StreamOutput out) throws IOException {
495495
out.writeString(cause);
496496
out.writeString(name);
497497
if (out.getVersion().onOrAfter(Version.V_6_0_0_alpha1)) {
498-
out.writeStringList(indexPatterns);
498+
out.writeStringCollection(indexPatterns);
499499
} else {
500500
out.writeString(indexPatterns.size() > 0 ? indexPatterns.get(0) : "");
501501
}

server/src/main/java/org/elasticsearch/cluster/metadata/IndexTemplateMetaData.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ public static IndexTemplateMetaData readFrom(StreamInput in) throws IOException
189189
Builder builder = new Builder(in.readString());
190190
builder.order(in.readInt());
191191
if (in.getVersion().onOrAfter(Version.V_6_0_0_alpha1)) {
192-
builder.patterns(in.readList(StreamInput::readString));
192+
builder.patterns(in.readStringList());
193193
} else {
194194
builder.patterns(Collections.singletonList(in.readString()));
195195
}
@@ -224,7 +224,7 @@ public void writeTo(StreamOutput out) throws IOException {
224224
out.writeString(name);
225225
out.writeInt(order);
226226
if (out.getVersion().onOrAfter(Version.V_6_0_0_alpha1)) {
227-
out.writeStringList(patterns);
227+
out.writeStringCollection(patterns);
228228
} else {
229229
out.writeString(patterns.get(0));
230230
}

server/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -946,12 +946,26 @@ public <T extends Streamable> List<T> readStreamableList(Supplier<T> constructor
946946
}
947947

948948
/**
949-
* Reads a list of objects
949+
* Reads a list of objects. The list is expected to have been written using {@link StreamOutput#writeList(List)} or
950+
* {@link StreamOutput#writeStreamableList(List)}.
951+
*
952+
* @return the list of objects
953+
* @throws IOException if an I/O exception occurs reading the list
950954
*/
951-
public <T> List<T> readList(Writeable.Reader<T> reader) throws IOException {
955+
public <T> List<T> readList(final Writeable.Reader<T> reader) throws IOException {
952956
return readCollection(reader, ArrayList::new);
953957
}
954958

959+
/**
960+
* Reads a list of strings. The list is expected to have been written using {@link StreamOutput#writeStringCollection(Collection)}.
961+
*
962+
* @return the list of strings
963+
* @throws IOException if an I/O exception occurs reading the list
964+
*/
965+
public List<String> readStringList() throws IOException {
966+
return readList(StreamInput::readString);
967+
}
968+
955969
/**
956970
* Reads a set of objects
957971
*/

server/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1048,23 +1048,27 @@ public void writeList(List<? extends Writeable> list) throws IOException {
10481048
}
10491049

10501050
/**
1051-
* Writes a collection of generic objects via a {@link Writer}
1051+
* Writes a collection of objects via a {@link Writer}.
1052+
*
1053+
* @param collection the collection of objects
1054+
* @throws IOException if an I/O exception occurs writing the collection
10521055
*/
1053-
public <T> void writeCollection(Collection<T> collection, Writer<T> writer) throws IOException {
1056+
public <T> void writeCollection(final Collection<T> collection, final Writer<T> writer) throws IOException {
10541057
writeVInt(collection.size());
1055-
for (T val: collection) {
1058+
for (final T val: collection) {
10561059
writer.write(this, val);
10571060
}
10581061
}
10591062

10601063
/**
1061-
* Writes a list of strings
1064+
* Writes a collection of a strings. The corresponding collection can be read from a stream input using
1065+
* {@link StreamInput#readList(Writeable.Reader)}.
1066+
*
1067+
* @param collection the collection of strings
1068+
* @throws IOException if an I/O exception occurs writing the collection
10621069
*/
1063-
public void writeStringList(List<String> list) throws IOException {
1064-
writeVInt(list.size());
1065-
for (String string: list) {
1066-
this.writeString(string);
1067-
}
1070+
public void writeStringCollection(final Collection<String> collection) throws IOException {
1071+
writeCollection(collection, StreamOutput::writeString);
10681072
}
10691073

10701074
/**

server/src/main/java/org/elasticsearch/indices/recovery/RecoveryResponse.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,9 @@ public final class RecoveryResponse extends TransportResponse {
6060

6161
RecoveryResponse(StreamInput in) throws IOException {
6262
super(in);
63-
phase1FileNames = in.readList(StreamInput::readString);
63+
phase1FileNames = in.readStringList();
6464
phase1FileSizes = in.readList(StreamInput::readVLong);
65-
phase1ExistingFileNames = in.readList(StreamInput::readString);
65+
phase1ExistingFileNames = in.readStringList();
6666
phase1ExistingFileSizes = in.readList(StreamInput::readVLong);
6767
phase1TotalSize = in.readVLong();
6868
phase1ExistingTotalSize = in.readVLong();
@@ -76,9 +76,9 @@ public final class RecoveryResponse extends TransportResponse {
7676
@Override
7777
public void writeTo(StreamOutput out) throws IOException {
7878
super.writeTo(out);
79-
out.writeStringList(phase1FileNames);
79+
out.writeStringCollection(phase1FileNames);
8080
out.writeCollection(phase1FileSizes, StreamOutput::writeVLong);
81-
out.writeStringList(phase1ExistingFileNames);
81+
out.writeStringCollection(phase1ExistingFileNames);
8282
out.writeCollection(phase1ExistingFileSizes, StreamOutput::writeVLong);
8383
out.writeVLong(phase1TotalSize);
8484
out.writeVLong(phase1ExistingTotalSize);

server/src/main/java/org/elasticsearch/plugins/PluginInfo.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ public PluginInfo(final StreamInput in) throws IOException {
103103
}
104104
this.classname = in.readString();
105105
if (in.getVersion().onOrAfter(Version.V_6_2_0)) {
106-
extendedPlugins = in.readList(StreamInput::readString);
106+
extendedPlugins = in.readStringList();
107107
} else {
108108
extendedPlugins = Collections.emptyList();
109109
}
@@ -128,7 +128,7 @@ public void writeTo(final StreamOutput out) throws IOException {
128128
}
129129
out.writeString(classname);
130130
if (out.getVersion().onOrAfter(Version.V_6_2_0)) {
131-
out.writeStringList(extendedPlugins);
131+
out.writeStringCollection(extendedPlugins);
132132
}
133133
out.writeBoolean(hasNativeController);
134134
if (out.getVersion().onOrAfter(Version.V_6_0_0_beta2) && out.getVersion().before(Version.V_6_3_0)) {

server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/InternalComposite.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public class InternalComposite
6969
public InternalComposite(StreamInput in) throws IOException {
7070
super(in);
7171
this.size = in.readVInt();
72-
this.sourceNames = in.readList(StreamInput::readString);
72+
this.sourceNames = in.readStringList();
7373
this.formats = new ArrayList<>(sourceNames.size());
7474
for (int i = 0; i < sourceNames.size(); i++) {
7575
if (in.getVersion().onOrAfter(Version.V_6_3_0)) {
@@ -90,7 +90,7 @@ public InternalComposite(StreamInput in) throws IOException {
9090
@Override
9191
protected void doWriteTo(StreamOutput out) throws IOException {
9292
out.writeVInt(size);
93-
out.writeStringList(sourceNames);
93+
out.writeStringCollection(sourceNames);
9494
if (out.getVersion().onOrAfter(Version.V_6_3_0)) {
9595
for (DocValueFormat format : formats) {
9696
out.writeNamedWriteable(format);

server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ public SearchSourceBuilder(StreamInput in) throws IOException {
243243
}
244244
}
245245
if (in.readBoolean()) {
246-
stats = in.readList(StreamInput::readString);
246+
stats = in.readStringList();
247247
}
248248
suggestBuilder = in.readOptionalWriteable(SuggestBuilder::new);
249249
terminateAfter = in.readVInt();
@@ -311,7 +311,7 @@ public void writeTo(StreamOutput out) throws IOException {
311311
boolean hasStats = stats != null;
312312
out.writeBoolean(hasStats);
313313
if (hasStats) {
314-
out.writeStringList(stats);
314+
out.writeStringCollection(stats);
315315
}
316316
out.writeOptionalWriteable(suggestBuilder);
317317
out.writeVInt(terminateAfter);

server/src/test/java/org/elasticsearch/common/io/stream/StreamTests.java

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
package org.elasticsearch.common.io.stream;
2121

2222
import org.apache.lucene.util.BytesRef;
23+
import org.elasticsearch.common.CheckedBiConsumer;
24+
import org.elasticsearch.common.CheckedFunction;
2325
import org.elasticsearch.common.bytes.BytesArray;
2426
import org.elasticsearch.common.bytes.BytesReference;
2527
import org.elasticsearch.common.collect.Tuple;
@@ -39,6 +41,7 @@
3941
import java.util.Map;
4042
import java.util.Objects;
4143
import java.util.Set;
44+
import java.util.function.Supplier;
4245
import java.util.stream.Collectors;
4346
import java.util.stream.IntStream;
4447

@@ -293,15 +296,27 @@ public int hashCode() {
293296

294297
}
295298

296-
final int length = randomIntBetween(0, 16);
297-
final Collection<FooBar> fooBars = new ArrayList<>(length);
299+
runWriteReadCollectionTest(
300+
() -> new FooBar(randomInt(), randomInt()), StreamOutput::writeCollection, in -> in.readList(FooBar::new));
301+
}
302+
303+
public void testStringCollection() throws IOException {
304+
runWriteReadCollectionTest(() -> randomUnicodeOfLength(16), StreamOutput::writeStringCollection, StreamInput::readStringList);
305+
}
306+
307+
private <T> void runWriteReadCollectionTest(
308+
final Supplier<T> supplier,
309+
final CheckedBiConsumer<StreamOutput, Collection<T>, IOException> writer,
310+
final CheckedFunction<StreamInput, Collection<T>, IOException> reader) throws IOException {
311+
final int length = randomIntBetween(0, 10);
312+
final Collection<T> collection = new ArrayList<>(length);
298313
for (int i = 0; i < length; i++) {
299-
fooBars.add(new FooBar(randomInt(), randomInt()));
314+
collection.add(supplier.get());
300315
}
301316
try (BytesStreamOutput out = new BytesStreamOutput()) {
302-
out.writeCollection(fooBars);
317+
writer.accept(out, collection);
303318
try (StreamInput in = out.bytes().streamInput()) {
304-
assertThat(fooBars, equalTo(in.readList(FooBar::new)));
319+
assertThat(collection, equalTo(reader.apply(in)));
305320
}
306321
}
307322
}

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ccr/AutoFollowMetadata.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ public AutoFollowPattern(String remoteCluster,
273273

274274
public AutoFollowPattern(StreamInput in) throws IOException {
275275
remoteCluster = in.readString();
276-
leaderIndexPatterns = in.readList(StreamInput::readString);
276+
leaderIndexPatterns = in.readStringList();
277277
followIndexPattern = in.readOptionalString();
278278
maxReadRequestOperationCount = in.readOptionalVInt();
279279
maxReadRequestSize = in.readOptionalWriteable(ByteSizeValue::new);
@@ -350,7 +350,7 @@ public TimeValue getPollTimeout() {
350350
@Override
351351
public void writeTo(StreamOutput out) throws IOException {
352352
out.writeString(remoteCluster);
353-
out.writeStringList(leaderIndexPatterns);
353+
out.writeStringCollection(leaderIndexPatterns);
354354
out.writeOptionalString(followIndexPattern);
355355
out.writeOptionalVInt(maxReadRequestOperationCount);
356356
out.writeOptionalWriteable(maxReadRequestSize);

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ccr/action/PutAutoFollowPatternAction.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ public Request(StreamInput in) throws IOException {
275275
super(in);
276276
name = in.readString();
277277
remoteCluster = in.readString();
278-
leaderIndexPatterns = in.readList(StreamInput::readString);
278+
leaderIndexPatterns = in.readStringList();
279279
followIndexNamePattern = in.readOptionalString();
280280
maxReadRequestOperationCount = in.readOptionalVInt();
281281
maxReadRequestSize = in.readOptionalWriteable(ByteSizeValue::new);
@@ -294,7 +294,7 @@ public void writeTo(StreamOutput out) throws IOException {
294294
super.writeTo(out);
295295
out.writeString(name);
296296
out.writeString(remoteCluster);
297-
out.writeStringList(leaderIndexPatterns);
297+
out.writeStringCollection(leaderIndexPatterns);
298298
out.writeOptionalString(followIndexNamePattern);
299299
out.writeOptionalVInt(maxReadRequestOperationCount);
300300
out.writeOptionalWriteable(maxReadRequestSize);

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/action/RemoveIndexLifecyclePolicyAction.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,13 +82,13 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
8282
@Override
8383
public void readFrom(StreamInput in) throws IOException {
8484
super.readFrom(in);
85-
failedIndexes = in.readList(StreamInput::readString);
85+
failedIndexes = in.readStringList();
8686
}
8787

8888
@Override
8989
public void writeTo(StreamOutput out) throws IOException {
9090
super.writeTo(out);
91-
out.writeStringList(failedIndexes);
91+
out.writeStringCollection(failedIndexes);
9292
}
9393

9494
@Override

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/FindFileStructureAction.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ public void readFrom(StreamInput in) throws IOException {
327327
timeout = in.readOptionalTimeValue();
328328
charset = in.readOptionalString();
329329
format = in.readBoolean() ? in.readEnum(FileStructure.Format.class) : null;
330-
columnNames = in.readBoolean() ? in.readList(StreamInput::readString) : null;
330+
columnNames = in.readBoolean() ? in.readStringList() : null;
331331
hasHeaderRow = in.readOptionalBoolean();
332332
delimiter = in.readBoolean() ? (char) in.readVInt() : null;
333333
quote = in.readBoolean() ? (char) in.readVInt() : null;

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetJobsStatsAction.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public Request() {}
8484
public Request(StreamInput in) throws IOException {
8585
super(in);
8686
jobId = in.readString();
87-
expandedJobsIds = in.readList(StreamInput::readString);
87+
expandedJobsIds = in.readStringList();
8888
if (in.getVersion().onOrAfter(Version.V_6_1_0)) {
8989
allowNoJobs = in.readBoolean();
9090
}
@@ -94,7 +94,7 @@ public Request(StreamInput in) throws IOException {
9494
public void writeTo(StreamOutput out) throws IOException {
9595
super.writeTo(out);
9696
out.writeString(jobId);
97-
out.writeStringList(expandedJobsIds);
97+
out.writeStringCollection(expandedJobsIds);
9898
if (out.getVersion().onOrAfter(Version.V_6_1_0)) {
9999
out.writeBoolean(allowNoJobs);
100100
}

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/StartDatafeedAction.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ public DatafeedParams(StreamInput in) throws IOException {
197197
timeout = TimeValue.timeValueMillis(in.readVLong());
198198
if (in.getVersion().onOrAfter(Version.V_6_6_0)) {
199199
jobId = in.readOptionalString();
200-
datafeedIndices = in.readList(StreamInput::readString);
200+
datafeedIndices = in.readStringList();
201201
}
202202
}
203203

@@ -274,7 +274,7 @@ public void writeTo(StreamOutput out) throws IOException {
274274
out.writeVLong(timeout.millis());
275275
if (out.getVersion().onOrAfter(Version.V_6_6_0)) {
276276
out.writeOptionalString(jobId);
277-
out.writeStringList(datafeedIndices);
277+
out.writeStringCollection(datafeedIndices);
278278
}
279279
}
280280

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfig.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -246,14 +246,14 @@ public DatafeedConfig(StreamInput in) throws IOException {
246246
this.queryDelay = in.readOptionalTimeValue();
247247
this.frequency = in.readOptionalTimeValue();
248248
if (in.readBoolean()) {
249-
this.indices = Collections.unmodifiableList(in.readList(StreamInput::readString));
249+
this.indices = Collections.unmodifiableList(in.readStringList());
250250
} else {
251251
this.indices = null;
252252
}
253253
// This consumes the list of types if there was one.
254254
if (in.getVersion().before(Version.V_7_0_0)) {
255255
if (in.readBoolean()) {
256-
in.readList(StreamInput::readString);
256+
in.readStringList();
257257
}
258258
}
259259
if (in.getVersion().before(Version.V_6_6_0)) {
@@ -408,15 +408,15 @@ public void writeTo(StreamOutput out) throws IOException {
408408
out.writeOptionalTimeValue(frequency);
409409
if (indices != null) {
410410
out.writeBoolean(true);
411-
out.writeStringList(indices);
411+
out.writeStringCollection(indices);
412412
} else {
413413
out.writeBoolean(false);
414414
}
415415
// Write the now removed types to prior versions.
416416
// An empty list is expected
417417
if (out.getVersion().before(Version.V_7_0_0)) {
418418
out.writeBoolean(true);
419-
out.writeStringList(Collections.emptyList());
419+
out.writeStringCollection(Collections.emptyList());
420420
}
421421
if (out.getVersion().before(Version.V_6_6_0)) {
422422
out.writeNamedWriteable(getParsedQuery());

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedUpdate.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,14 +107,14 @@ public DatafeedUpdate(StreamInput in) throws IOException {
107107
this.queryDelay = in.readOptionalTimeValue();
108108
this.frequency = in.readOptionalTimeValue();
109109
if (in.readBoolean()) {
110-
this.indices = in.readList(StreamInput::readString);
110+
this.indices = in.readStringList();
111111
} else {
112112
this.indices = null;
113113
}
114114
// This consumes the list of types if there was one.
115115
if (in.getVersion().before(Version.V_7_0_0)) {
116116
if (in.readBoolean()) {
117-
in.readList(StreamInput::readString);
117+
in.readStringList();
118118
}
119119
}
120120
this.query = in.readOptionalNamedWriteable(QueryBuilder.class);
@@ -148,15 +148,15 @@ public void writeTo(StreamOutput out) throws IOException {
148148
out.writeOptionalTimeValue(frequency);
149149
if (indices != null) {
150150
out.writeBoolean(true);
151-
out.writeStringList(indices);
151+
out.writeStringCollection(indices);
152152
} else {
153153
out.writeBoolean(false);
154154
}
155155
// Write the now removed types to prior versions.
156156
// An empty list is expected
157157
if (out.getVersion().before(Version.V_7_0_0)) {
158158
out.writeBoolean(true);
159-
out.writeStringList(Collections.emptyList());
159+
out.writeStringCollection(Collections.emptyList());
160160
}
161161
out.writeOptionalNamedWriteable(query);
162162
out.writeOptionalWriteable(aggregations);

0 commit comments

Comments
 (0)