diff --git a/docs/changelog/119233.yaml b/docs/changelog/119233.yaml new file mode 100644 index 0000000000000..ef89c011ce4f6 --- /dev/null +++ b/docs/changelog/119233.yaml @@ -0,0 +1,5 @@ +pr: 119233 +summary: Fixing `GetDatabaseConfigurationAction` response serialization +area: Ingest Node +type: bug +issues: [] diff --git a/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/direct/GetDatabaseConfigurationAction.java b/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/direct/GetDatabaseConfigurationAction.java index 7501c0094d647..af4ac8c93b550 100644 --- a/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/direct/GetDatabaseConfigurationAction.java +++ b/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/direct/GetDatabaseConfigurationAction.java @@ -25,6 +25,7 @@ import java.io.IOException; import java.util.Arrays; import java.util.List; +import java.util.Map; import java.util.Objects; import static org.elasticsearch.ingest.geoip.direct.DatabaseConfigurationMetadata.DATABASE; @@ -91,6 +92,11 @@ protected Response(StreamInput in) throws IOException { this.databases = in.readCollectionAsList(DatabaseConfigurationMetadata::new); } + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + out.writeCollection(databases); + } + @Override protected List readNodesFrom(StreamInput in) throws IOException { return in.readCollectionAsList(NodeResponse::new); @@ -122,6 +128,63 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.endObject(); return builder; } + + /* + * This implementation of equals exists solely for testing the serialization of this object. + */ + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Response response = (Response) o; + return Objects.equals(databases, response.databases) + && Objects.equals(getClusterName(), response.getClusterName()) + && Objects.equals(equalsHashCodeFailures(), response.equalsHashCodeFailures()) + && Objects.equals(getNodes(), response.getNodes()) + && Objects.equals(equalsHashCodeNodesMap(), response.equalsHashCodeNodesMap()); + } + + /* + * This implementation of hashCode exists solely for testing the serialization of this object. + */ + @Override + public int hashCode() { + return Objects.hash(databases, getClusterName(), equalsHashCodeFailures(), getNodes(), equalsHashCodeNodesMap()); + } + + /* + * FailedNodeException does not implement equals or hashCode, making it difficult to test the serialization of this class. This + * helper method wraps the failures() list with a class that does implement equals and hashCode. + */ + private List equalsHashCodeFailures() { + return failures().stream().map(EqualsHashCodeFailedNodeException::new).toList(); + } + + private record EqualsHashCodeFailedNodeException(FailedNodeException failedNodeException) { + @Override + public boolean equals(Object o) { + if (o == this) return true; + if (o == null || getClass() != o.getClass()) return false; + EqualsHashCodeFailedNodeException other = (EqualsHashCodeFailedNodeException) o; + return Objects.equals(failedNodeException.nodeId(), other.failedNodeException.nodeId()) + && Objects.equals(failedNodeException.getMessage(), other.failedNodeException.getMessage()); + } + + @Override + public int hashCode() { + return Objects.hash(failedNodeException.nodeId(), failedNodeException.getMessage()); + } + } + + /* + * The getNodesMap method changes the value of the nodesMap, causing failures when testing the concurrent serialization and + * deserialization of this class. Since this is a response object, we do not actually care about concurrency since it will not + * happen in practice. So this helper method synchronizes access to getNodesMap, which can be used from equals and hashCode for + * tests. + */ + private synchronized Map equalsHashCodeNodesMap() { + return getNodesMap(); + } } public static class NodeRequest extends TransportRequest { @@ -186,6 +249,7 @@ public List getDatabases() { @Override public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); out.writeCollection(databases); } diff --git a/modules/ingest-geoip/src/test/java/org/elasticsearch/ingest/geoip/direct/GetDatabaseConfigurationActionNodeResponseTests.java b/modules/ingest-geoip/src/test/java/org/elasticsearch/ingest/geoip/direct/GetDatabaseConfigurationActionNodeResponseTests.java new file mode 100644 index 0000000000000..12fb08a5a1abf --- /dev/null +++ b/modules/ingest-geoip/src/test/java/org/elasticsearch/ingest/geoip/direct/GetDatabaseConfigurationActionNodeResponseTests.java @@ -0,0 +1,98 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.ingest.geoip.direct; + +import org.elasticsearch.cluster.node.DiscoveryNode; +import org.elasticsearch.cluster.node.DiscoveryNodeUtils; +import org.elasticsearch.common.io.stream.NamedWriteableRegistry; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.test.AbstractWireSerializingTestCase; + +import java.io.IOException; +import java.util.List; + +import static java.util.Collections.emptySet; + +public class GetDatabaseConfigurationActionNodeResponseTests extends AbstractWireSerializingTestCase< + GetDatabaseConfigurationAction.NodeResponse> { + @Override + protected Writeable.Reader instanceReader() { + return GetDatabaseConfigurationAction.NodeResponse::new; + } + + @Override + protected GetDatabaseConfigurationAction.NodeResponse createTestInstance() { + return getRandomDatabaseConfigurationActionNodeResponse(); + } + + static GetDatabaseConfigurationAction.NodeResponse getRandomDatabaseConfigurationActionNodeResponse() { + return new GetDatabaseConfigurationAction.NodeResponse(randomDiscoveryNode(), getRandomDatabaseConfigurationMetadata()); + } + + private static DiscoveryNode randomDiscoveryNode() { + return DiscoveryNodeUtils.builder(randomAlphaOfLength(6)).roles(emptySet()).build(); + } + + static List getRandomDatabaseConfigurationMetadata() { + return randomList( + 0, + 20, + () -> new DatabaseConfigurationMetadata( + new DatabaseConfiguration( + randomAlphaOfLength(20), + randomAlphaOfLength(20), + randomFrom( + List.of( + new DatabaseConfiguration.Local(randomAlphaOfLength(10)), + new DatabaseConfiguration.Web(), + new DatabaseConfiguration.Ipinfo(), + new DatabaseConfiguration.Maxmind(randomAlphaOfLength(10)) + ) + ) + ), + randomNonNegativeLong(), + randomNonNegativeLong() + ) + ); + } + + @Override + protected GetDatabaseConfigurationAction.NodeResponse mutateInstance(GetDatabaseConfigurationAction.NodeResponse instance) + throws IOException { + return null; + } + + protected NamedWriteableRegistry getNamedWriteableRegistry() { + return new NamedWriteableRegistry( + List.of( + new NamedWriteableRegistry.Entry( + DatabaseConfiguration.Provider.class, + DatabaseConfiguration.Maxmind.NAME, + DatabaseConfiguration.Maxmind::new + ), + new NamedWriteableRegistry.Entry( + DatabaseConfiguration.Provider.class, + DatabaseConfiguration.Ipinfo.NAME, + DatabaseConfiguration.Ipinfo::new + ), + new NamedWriteableRegistry.Entry( + DatabaseConfiguration.Provider.class, + DatabaseConfiguration.Local.NAME, + DatabaseConfiguration.Local::new + ), + new NamedWriteableRegistry.Entry( + DatabaseConfiguration.Provider.class, + DatabaseConfiguration.Web.NAME, + DatabaseConfiguration.Web::new + ) + ) + ); + } +} diff --git a/modules/ingest-geoip/src/test/java/org/elasticsearch/ingest/geoip/direct/GetDatabaseConfigurationActionResponseTests.java b/modules/ingest-geoip/src/test/java/org/elasticsearch/ingest/geoip/direct/GetDatabaseConfigurationActionResponseTests.java new file mode 100644 index 0000000000000..1b48a409d7876 --- /dev/null +++ b/modules/ingest-geoip/src/test/java/org/elasticsearch/ingest/geoip/direct/GetDatabaseConfigurationActionResponseTests.java @@ -0,0 +1,89 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.ingest.geoip.direct; + +import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.action.FailedNodeException; +import org.elasticsearch.cluster.ClusterName; +import org.elasticsearch.common.io.stream.NamedWriteableRegistry; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.test.AbstractWireSerializingTestCase; + +import java.io.IOException; +import java.util.List; + +public class GetDatabaseConfigurationActionResponseTests extends AbstractWireSerializingTestCase { + @Override + protected Writeable.Reader instanceReader() { + return GetDatabaseConfigurationAction.Response::new; + } + + @Override + protected GetDatabaseConfigurationAction.Response createTestInstance() { + return new GetDatabaseConfigurationAction.Response( + GetDatabaseConfigurationActionNodeResponseTests.getRandomDatabaseConfigurationMetadata(), + getTestClusterName(), + getTestNodeResponses(), + getTestFailedNodeExceptions() + ); + } + + @Override + protected GetDatabaseConfigurationAction.Response mutateInstance(GetDatabaseConfigurationAction.Response instance) throws IOException { + return null; + } + + private ClusterName getTestClusterName() { + return new ClusterName(randomAlphaOfLength(30)); + } + + private List getTestNodeResponses() { + return randomList(0, 20, GetDatabaseConfigurationActionNodeResponseTests::getRandomDatabaseConfigurationActionNodeResponse); + } + + private List getTestFailedNodeExceptions() { + return randomList( + 0, + 5, + () -> new FailedNodeException( + randomAlphaOfLength(10), + randomAlphaOfLength(20), + new ElasticsearchException(randomAlphaOfLength(10)) + ) + ); + } + + protected NamedWriteableRegistry getNamedWriteableRegistry() { + return new NamedWriteableRegistry( + List.of( + new NamedWriteableRegistry.Entry( + DatabaseConfiguration.Provider.class, + DatabaseConfiguration.Maxmind.NAME, + DatabaseConfiguration.Maxmind::new + ), + new NamedWriteableRegistry.Entry( + DatabaseConfiguration.Provider.class, + DatabaseConfiguration.Ipinfo.NAME, + DatabaseConfiguration.Ipinfo::new + ), + new NamedWriteableRegistry.Entry( + DatabaseConfiguration.Provider.class, + DatabaseConfiguration.Local.NAME, + DatabaseConfiguration.Local::new + ), + new NamedWriteableRegistry.Entry( + DatabaseConfiguration.Provider.class, + DatabaseConfiguration.Web.NAME, + DatabaseConfiguration.Web::new + ) + ) + ); + } +}