Skip to content

Commit

Permalink
Make DiscoveryNodeRole into a value object (elastic#43257)
Browse files Browse the repository at this point in the history
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 elastic#43175
  • Loading branch information
DaveCTurner authored Jun 17, 2019
1 parent 0ea892d commit f828c77
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,18 +53,48 @@ public final String roleNameAbbreviation() {
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);
}

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") +
'}';
}

Expand Down Expand Up @@ -122,7 +152,7 @@ static class UnknownRole extends DiscoveryNodeRole {
* @param roleNameAbbreviation the role name abbreviation
*/
UnknownRole(final String roleName, final String roleNameAbbreviation) {
super(roleName, roleNameAbbreviation);
super(false, roleName, roleNameAbbreviation);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.EqualsHashCodeTestUtils;

import java.util.Set;

Expand Down Expand Up @@ -75,4 +76,25 @@ protected Setting<Boolean> roleSetting() {
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());
}
}
}

0 comments on commit f828c77

Please sign in to comment.