Skip to content

Commit bea8da1

Browse files
masseykejoegallo
masseyke
and
joegallo
authored
Fixing GetDatabaseConfigurationAction response serialization (#119233) (#119471)
Co-authored-by: Joe Gallo <[email protected]>
1 parent bfda619 commit bea8da1

File tree

4 files changed

+256
-0
lines changed

4 files changed

+256
-0
lines changed

‎docs/changelog/119233.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 119233
2+
summary: Fixing `GetDatabaseConfigurationAction` response serialization
3+
area: Ingest Node
4+
type: bug
5+
issues: []

‎modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/direct/GetDatabaseConfigurationAction.java‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.io.IOException;
2626
import java.util.Arrays;
2727
import java.util.List;
28+
import java.util.Map;
2829
import java.util.Objects;
2930

3031
import static org.elasticsearch.ingest.geoip.direct.DatabaseConfigurationMetadata.DATABASE;
@@ -91,6 +92,11 @@ protected Response(StreamInput in) throws IOException {
9192
this.databases = in.readCollectionAsList(DatabaseConfigurationMetadata::new);
9293
}
9394

95+
public void writeTo(StreamOutput out) throws IOException {
96+
super.writeTo(out);
97+
out.writeCollection(databases);
98+
}
99+
94100
@Override
95101
protected List<NodeResponse> readNodesFrom(StreamInput in) throws IOException {
96102
return in.readCollectionAsList(NodeResponse::new);
@@ -122,6 +128,63 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
122128
builder.endObject();
123129
return builder;
124130
}
131+
132+
/*
133+
* This implementation of equals exists solely for testing the serialization of this object.
134+
*/
135+
@Override
136+
public boolean equals(Object o) {
137+
if (this == o) return true;
138+
if (o == null || getClass() != o.getClass()) return false;
139+
Response response = (Response) o;
140+
return Objects.equals(databases, response.databases)
141+
&& Objects.equals(getClusterName(), response.getClusterName())
142+
&& Objects.equals(equalsHashCodeFailures(), response.equalsHashCodeFailures())
143+
&& Objects.equals(getNodes(), response.getNodes())
144+
&& Objects.equals(equalsHashCodeNodesMap(), response.equalsHashCodeNodesMap());
145+
}
146+
147+
/*
148+
* This implementation of hashCode exists solely for testing the serialization of this object.
149+
*/
150+
@Override
151+
public int hashCode() {
152+
return Objects.hash(databases, getClusterName(), equalsHashCodeFailures(), getNodes(), equalsHashCodeNodesMap());
153+
}
154+
155+
/*
156+
* FailedNodeException does not implement equals or hashCode, making it difficult to test the serialization of this class. This
157+
* helper method wraps the failures() list with a class that does implement equals and hashCode.
158+
*/
159+
private List<EqualsHashCodeFailedNodeException> equalsHashCodeFailures() {
160+
return failures().stream().map(EqualsHashCodeFailedNodeException::new).toList();
161+
}
162+
163+
private record EqualsHashCodeFailedNodeException(FailedNodeException failedNodeException) {
164+
@Override
165+
public boolean equals(Object o) {
166+
if (o == this) return true;
167+
if (o == null || getClass() != o.getClass()) return false;
168+
EqualsHashCodeFailedNodeException other = (EqualsHashCodeFailedNodeException) o;
169+
return Objects.equals(failedNodeException.nodeId(), other.failedNodeException.nodeId())
170+
&& Objects.equals(failedNodeException.getMessage(), other.failedNodeException.getMessage());
171+
}
172+
173+
@Override
174+
public int hashCode() {
175+
return Objects.hash(failedNodeException.nodeId(), failedNodeException.getMessage());
176+
}
177+
}
178+
179+
/*
180+
* The getNodesMap method changes the value of the nodesMap, causing failures when testing the concurrent serialization and
181+
* deserialization of this class. Since this is a response object, we do not actually care about concurrency since it will not
182+
* happen in practice. So this helper method synchronizes access to getNodesMap, which can be used from equals and hashCode for
183+
* tests.
184+
*/
185+
private synchronized Map<String, NodeResponse> equalsHashCodeNodesMap() {
186+
return getNodesMap();
187+
}
125188
}
126189

127190
public static class NodeRequest extends TransportRequest {
@@ -186,6 +249,7 @@ public List<DatabaseConfigurationMetadata> getDatabases() {
186249

187250
@Override
188251
public void writeTo(StreamOutput out) throws IOException {
252+
super.writeTo(out);
189253
out.writeCollection(databases);
190254
}
191255

‎modules/ingest-geoip/src/test/java/org/elasticsearch/ingest/geoip/direct/GetDatabaseConfigurationActionNodeResponseTests.java‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.ingest.geoip.direct;
11+
12+
import org.elasticsearch.cluster.node.DiscoveryNode;
13+
import org.elasticsearch.cluster.node.DiscoveryNodeUtils;
14+
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
15+
import org.elasticsearch.common.io.stream.Writeable;
16+
import org.elasticsearch.test.AbstractWireSerializingTestCase;
17+
18+
import java.io.IOException;
19+
import java.util.List;
20+
21+
import static java.util.Collections.emptySet;
22+
23+
public class GetDatabaseConfigurationActionNodeResponseTests extends AbstractWireSerializingTestCase<
24+
GetDatabaseConfigurationAction.NodeResponse> {
25+
@Override
26+
protected Writeable.Reader<GetDatabaseConfigurationAction.NodeResponse> instanceReader() {
27+
return GetDatabaseConfigurationAction.NodeResponse::new;
28+
}
29+
30+
@Override
31+
protected GetDatabaseConfigurationAction.NodeResponse createTestInstance() {
32+
return getRandomDatabaseConfigurationActionNodeResponse();
33+
}
34+
35+
static GetDatabaseConfigurationAction.NodeResponse getRandomDatabaseConfigurationActionNodeResponse() {
36+
return new GetDatabaseConfigurationAction.NodeResponse(randomDiscoveryNode(), getRandomDatabaseConfigurationMetadata());
37+
}
38+
39+
private static DiscoveryNode randomDiscoveryNode() {
40+
return DiscoveryNodeUtils.builder(randomAlphaOfLength(6)).roles(emptySet()).build();
41+
}
42+
43+
static List<DatabaseConfigurationMetadata> getRandomDatabaseConfigurationMetadata() {
44+
return randomList(
45+
0,
46+
20,
47+
() -> new DatabaseConfigurationMetadata(
48+
new DatabaseConfiguration(
49+
randomAlphaOfLength(20),
50+
randomAlphaOfLength(20),
51+
randomFrom(
52+
List.of(
53+
new DatabaseConfiguration.Local(randomAlphaOfLength(10)),
54+
new DatabaseConfiguration.Web(),
55+
new DatabaseConfiguration.Ipinfo(),
56+
new DatabaseConfiguration.Maxmind(randomAlphaOfLength(10))
57+
)
58+
)
59+
),
60+
randomNonNegativeLong(),
61+
randomNonNegativeLong()
62+
)
63+
);
64+
}
65+
66+
@Override
67+
protected GetDatabaseConfigurationAction.NodeResponse mutateInstance(GetDatabaseConfigurationAction.NodeResponse instance)
68+
throws IOException {
69+
return null;
70+
}
71+
72+
protected NamedWriteableRegistry getNamedWriteableRegistry() {
73+
return new NamedWriteableRegistry(
74+
List.of(
75+
new NamedWriteableRegistry.Entry(
76+
DatabaseConfiguration.Provider.class,
77+
DatabaseConfiguration.Maxmind.NAME,
78+
DatabaseConfiguration.Maxmind::new
79+
),
80+
new NamedWriteableRegistry.Entry(
81+
DatabaseConfiguration.Provider.class,
82+
DatabaseConfiguration.Ipinfo.NAME,
83+
DatabaseConfiguration.Ipinfo::new
84+
),
85+
new NamedWriteableRegistry.Entry(
86+
DatabaseConfiguration.Provider.class,
87+
DatabaseConfiguration.Local.NAME,
88+
DatabaseConfiguration.Local::new
89+
),
90+
new NamedWriteableRegistry.Entry(
91+
DatabaseConfiguration.Provider.class,
92+
DatabaseConfiguration.Web.NAME,
93+
DatabaseConfiguration.Web::new
94+
)
95+
)
96+
);
97+
}
98+
}

‎modules/ingest-geoip/src/test/java/org/elasticsearch/ingest/geoip/direct/GetDatabaseConfigurationActionResponseTests.java‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.ingest.geoip.direct;
11+
12+
import org.elasticsearch.ElasticsearchException;
13+
import org.elasticsearch.action.FailedNodeException;
14+
import org.elasticsearch.cluster.ClusterName;
15+
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
16+
import org.elasticsearch.common.io.stream.Writeable;
17+
import org.elasticsearch.test.AbstractWireSerializingTestCase;
18+
19+
import java.io.IOException;
20+
import java.util.List;
21+
22+
public class GetDatabaseConfigurationActionResponseTests extends AbstractWireSerializingTestCase<GetDatabaseConfigurationAction.Response> {
23+
@Override
24+
protected Writeable.Reader<GetDatabaseConfigurationAction.Response> instanceReader() {
25+
return GetDatabaseConfigurationAction.Response::new;
26+
}
27+
28+
@Override
29+
protected GetDatabaseConfigurationAction.Response createTestInstance() {
30+
return new GetDatabaseConfigurationAction.Response(
31+
GetDatabaseConfigurationActionNodeResponseTests.getRandomDatabaseConfigurationMetadata(),
32+
getTestClusterName(),
33+
getTestNodeResponses(),
34+
getTestFailedNodeExceptions()
35+
);
36+
}
37+
38+
@Override
39+
protected GetDatabaseConfigurationAction.Response mutateInstance(GetDatabaseConfigurationAction.Response instance) throws IOException {
40+
return null;
41+
}
42+
43+
private ClusterName getTestClusterName() {
44+
return new ClusterName(randomAlphaOfLength(30));
45+
}
46+
47+
private List<GetDatabaseConfigurationAction.NodeResponse> getTestNodeResponses() {
48+
return randomList(0, 20, GetDatabaseConfigurationActionNodeResponseTests::getRandomDatabaseConfigurationActionNodeResponse);
49+
}
50+
51+
private List<FailedNodeException> getTestFailedNodeExceptions() {
52+
return randomList(
53+
0,
54+
5,
55+
() -> new FailedNodeException(
56+
randomAlphaOfLength(10),
57+
randomAlphaOfLength(20),
58+
new ElasticsearchException(randomAlphaOfLength(10))
59+
)
60+
);
61+
}
62+
63+
protected NamedWriteableRegistry getNamedWriteableRegistry() {
64+
return new NamedWriteableRegistry(
65+
List.of(
66+
new NamedWriteableRegistry.Entry(
67+
DatabaseConfiguration.Provider.class,
68+
DatabaseConfiguration.Maxmind.NAME,
69+
DatabaseConfiguration.Maxmind::new
70+
),
71+
new NamedWriteableRegistry.Entry(
72+
DatabaseConfiguration.Provider.class,
73+
DatabaseConfiguration.Ipinfo.NAME,
74+
DatabaseConfiguration.Ipinfo::new
75+
),
76+
new NamedWriteableRegistry.Entry(
77+
DatabaseConfiguration.Provider.class,
78+
DatabaseConfiguration.Local.NAME,
79+
DatabaseConfiguration.Local::new
80+
),
81+
new NamedWriteableRegistry.Entry(
82+
DatabaseConfiguration.Provider.class,
83+
DatabaseConfiguration.Web.NAME,
84+
DatabaseConfiguration.Web::new
85+
)
86+
)
87+
);
88+
}
89+
}

0 commit comments

Comments
 (0)