From 4b58827bebf984dc9a6c537ddb9405bcf3333268 Mon Sep 17 00:00:00 2001
From: David Turner <david.turner@elastic.co>
Date: Mon, 17 Jun 2019 10:22:11 +0100
Subject: [PATCH] Make DiscoveryNodeRole into a value object (#43257)

Adds `equals()` and `hashcode()` methods to `DiscoveryNodeRole` to compare
these objects' values for equality, and adds a field to allow us to distinguish
unknown roles from known ones with the same name and abbreviation, for clearer
test failures.

Relates #43175
---
 .../cluster/node/DiscoveryNodeRole.java       | 34 +++++++++++++++++--
 .../cluster/node/DiscoveryNodeRoleTests.java  | 22 ++++++++++++
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeRole.java b/server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeRole.java
index 711761e86e0..746acd6c1b6 100644
--- a/server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeRole.java
+++ b/server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeRole.java
@@ -56,7 +56,21 @@ public abstract class DiscoveryNodeRole {
         return roleNameAbbreviation;
     }
 
+    private final boolean isKnownRole;
+
+    /**
+     * Whether this role is known by this node, or is an {@link DiscoveryNodeRole.UnknownRole}.
+     */
+    public final boolean isKnownRole() {
+        return isKnownRole;
+    }
+
     protected DiscoveryNodeRole(final String roleName, final String roleNameAbbreviation) {
+        this(true, roleName, roleNameAbbreviation);
+    }
+
+    private DiscoveryNodeRole(final boolean isKnownRole, final String roleName, final String roleNameAbbreviation) {
+        this.isKnownRole = isKnownRole;
         this.roleName = Objects.requireNonNull(roleName);
         this.roleNameAbbreviation = Objects.requireNonNull(roleNameAbbreviation);
     }
@@ -64,10 +78,26 @@ public abstract class DiscoveryNodeRole {
     protected abstract Setting<Boolean> roleSetting();
 
     @Override
-    public String toString() {
+    public final boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        DiscoveryNodeRole that = (DiscoveryNodeRole) o;
+        return roleName.equals(that.roleName) &&
+            roleNameAbbreviation.equals(that.roleNameAbbreviation) &&
+            isKnownRole == that.isKnownRole;
+    }
+
+    @Override
+    public final int hashCode() {
+        return Objects.hash(isKnownRole, roleName(), roleNameAbbreviation());
+    }
+
+    @Override
+    public final String toString() {
         return "DiscoveryNodeRole{" +
                 "roleName='" + roleName + '\'' +
                 ", roleNameAbbreviation='" + roleNameAbbreviation + '\'' +
+                (isKnownRole ? "" : ", isKnownRole=false") +
                 '}';
     }
 
@@ -126,7 +156,7 @@ public abstract class DiscoveryNodeRole {
          * @param roleNameAbbreviation the role name abbreviation
          */
         UnknownRole(final String roleName, final String roleNameAbbreviation) {
-            super(roleName, roleNameAbbreviation);
+            super(false, roleName, roleNameAbbreviation);
         }
 
         @Override
diff --git a/server/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodeRoleTests.java b/server/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodeRoleTests.java
index d0f407958f8..35a78a87717 100644
--- a/server/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodeRoleTests.java
+++ b/server/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodeRoleTests.java
@@ -21,6 +21,7 @@ package org.elasticsearch.cluster.node;
 
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.test.EqualsHashCodeTestUtils;
 
 import java.util.Arrays;
 import java.util.HashSet;
@@ -76,4 +77,25 @@ public class DiscoveryNodeRoleTests extends ESTestCase {
         assertThat(e, hasToString(containsString("Duplicate key")));
     }
 
+    public void testDiscoveryNodeRoleEqualsHashCode() {
+        EqualsHashCodeTestUtils.checkEqualsAndHashCode(new DiscoveryNodeRole.UnknownRole(randomAlphaOfLength(10), randomAlphaOfLength(1)),
+            r -> new DiscoveryNodeRole.UnknownRole(r.roleName(), r.roleNameAbbreviation()),
+            r -> {
+                if (randomBoolean()) {
+                    return new DiscoveryNodeRole.UnknownRole(randomAlphaOfLength(21 - r.roleName().length()), r.roleNameAbbreviation());
+                } else {
+                    return new DiscoveryNodeRole.UnknownRole(r.roleName(), randomAlphaOfLength(3 - r.roleNameAbbreviation().length()));
+                }
+            });
+
+    }
+
+    public void testUnknownRoleIsDistinctFromKnownRoles() {
+        for (DiscoveryNodeRole buildInRole : DiscoveryNodeRole.BUILT_IN_ROLES) {
+            final DiscoveryNodeRole.UnknownRole unknownDataRole
+                = new DiscoveryNodeRole.UnknownRole(buildInRole.roleName(), buildInRole.roleNameAbbreviation());
+            assertNotEquals(buildInRole, unknownDataRole);
+            assertNotEquals(buildInRole.toString(), unknownDataRole.toString());
+        }
+    }
 }