Skip to content

Commit f828c77

Browse files
authored
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
1 parent 0ea892d commit f828c77

File tree

2 files changed

+54
-2
lines changed

2 files changed

+54
-2
lines changed

server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeRole.java

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,18 +53,48 @@ public final String roleNameAbbreviation() {
5353
return roleNameAbbreviation;
5454
}
5555

56+
private final boolean isKnownRole;
57+
58+
/**
59+
* Whether this role is known by this node, or is an {@link DiscoveryNodeRole.UnknownRole}.
60+
*/
61+
public final boolean isKnownRole() {
62+
return isKnownRole;
63+
}
64+
5665
protected DiscoveryNodeRole(final String roleName, final String roleNameAbbreviation) {
66+
this(true, roleName, roleNameAbbreviation);
67+
}
68+
69+
private DiscoveryNodeRole(final boolean isKnownRole, final String roleName, final String roleNameAbbreviation) {
70+
this.isKnownRole = isKnownRole;
5771
this.roleName = Objects.requireNonNull(roleName);
5872
this.roleNameAbbreviation = Objects.requireNonNull(roleNameAbbreviation);
5973
}
6074

6175
protected abstract Setting<Boolean> roleSetting();
6276

6377
@Override
64-
public String toString() {
78+
public final boolean equals(Object o) {
79+
if (this == o) return true;
80+
if (o == null || getClass() != o.getClass()) return false;
81+
DiscoveryNodeRole that = (DiscoveryNodeRole) o;
82+
return roleName.equals(that.roleName) &&
83+
roleNameAbbreviation.equals(that.roleNameAbbreviation) &&
84+
isKnownRole == that.isKnownRole;
85+
}
86+
87+
@Override
88+
public final int hashCode() {
89+
return Objects.hash(isKnownRole, roleName(), roleNameAbbreviation());
90+
}
91+
92+
@Override
93+
public final String toString() {
6594
return "DiscoveryNodeRole{" +
6695
"roleName='" + roleName + '\'' +
6796
", roleNameAbbreviation='" + roleNameAbbreviation + '\'' +
97+
(isKnownRole ? "" : ", isKnownRole=false") +
6898
'}';
6999
}
70100

@@ -122,7 +152,7 @@ static class UnknownRole extends DiscoveryNodeRole {
122152
* @param roleNameAbbreviation the role name abbreviation
123153
*/
124154
UnknownRole(final String roleName, final String roleNameAbbreviation) {
125-
super(roleName, roleNameAbbreviation);
155+
super(false, roleName, roleNameAbbreviation);
126156
}
127157

128158
@Override

server/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodeRoleTests.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import org.elasticsearch.common.settings.Setting;
2323
import org.elasticsearch.test.ESTestCase;
24+
import org.elasticsearch.test.EqualsHashCodeTestUtils;
2425

2526
import java.util.Set;
2627

@@ -75,4 +76,25 @@ protected Setting<Boolean> roleSetting() {
7576
assertThat(e, hasToString(containsString("Duplicate key")));
7677
}
7778

79+
public void testDiscoveryNodeRoleEqualsHashCode() {
80+
EqualsHashCodeTestUtils.checkEqualsAndHashCode(new DiscoveryNodeRole.UnknownRole(randomAlphaOfLength(10), randomAlphaOfLength(1)),
81+
r -> new DiscoveryNodeRole.UnknownRole(r.roleName(), r.roleNameAbbreviation()),
82+
r -> {
83+
if (randomBoolean()) {
84+
return new DiscoveryNodeRole.UnknownRole(randomAlphaOfLength(21 - r.roleName().length()), r.roleNameAbbreviation());
85+
} else {
86+
return new DiscoveryNodeRole.UnknownRole(r.roleName(), randomAlphaOfLength(3 - r.roleNameAbbreviation().length()));
87+
}
88+
});
89+
90+
}
91+
92+
public void testUnknownRoleIsDistinctFromKnownRoles() {
93+
for (DiscoveryNodeRole buildInRole : DiscoveryNodeRole.BUILT_IN_ROLES) {
94+
final DiscoveryNodeRole.UnknownRole unknownDataRole
95+
= new DiscoveryNodeRole.UnknownRole(buildInRole.roleName(), buildInRole.roleNameAbbreviation());
96+
assertNotEquals(buildInRole, unknownDataRole);
97+
assertNotEquals(buildInRole.toString(), unknownDataRole.toString());
98+
}
99+
}
78100
}

0 commit comments

Comments
 (0)