Fixing GetDatabaseConfigurationAction response serialization (elastic…

…#119233)

Co-authored-by: Joe Gallo <joe.gallo@elastic.co>

docs/changelog/119233.yaml

@@ -0,0 +1,5 @@
+pr: 119233
+summary: Fixing `GetDatabaseConfigurationAction` response serialization
+area: Ingest Node
+type: bug
+issues: []

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<NodeResponse> 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<EqualsHashCodeFailedNodeException> 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<String, NodeResponse> equalsHashCodeNodesMap() {
+            return getNodesMap();
+        }
     }
 
     public static class NodeRequest extends TransportRequest {
@@ -186,6 +249,7 @@ public List<DatabaseConfigurationMetadata> getDatabases() {
 
         @Override
         public void writeTo(StreamOutput out) throws IOException {
+            super.writeTo(out);
             out.writeCollection(databases);
         }
 

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<GetDatabaseConfigurationAction.NodeResponse> 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<DatabaseConfigurationMetadata> 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
+                )
+            )
+        );
+    }
+}

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<GetDatabaseConfigurationAction.Response> {
+    @Override
+    protected Writeable.Reader<GetDatabaseConfigurationAction.Response> 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<GetDatabaseConfigurationAction.NodeResponse> getTestNodeResponses() {
+        return randomList(0, 20, GetDatabaseConfigurationActionNodeResponseTests::getRandomDatabaseConfigurationActionNodeResponse);
+    }
+
+    private List<FailedNodeException> 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
+                )
+            )
+        );
+    }
+}