Skip to content

Commit

Permalink
Drop equals from Node
Browse files Browse the repository at this point in the history
  • Loading branch information
nik9000 committed Jun 1, 2018
1 parent 2ae7c27 commit d551dcb
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 70 deletions.
18 changes: 0 additions & 18 deletions client/rest/src/main/java/org/elasticsearch/client/Node.java
Original file line number Diff line number Diff line change
Expand Up @@ -156,24 +156,6 @@ public String toString() {
return b.append(']').toString();
}

@Override
public boolean equals(Object obj) {
if (obj == null || obj.getClass() != getClass()) {
return false;
}
Node other = (Node) obj;
return host.equals(other.host)
&& Objects.equals(boundHosts, other.boundHosts)
&& Objects.equals(version, other.version)
&& Objects.equals(name, other.name)
&& Objects.equals(roles, other.roles);
}

@Override
public int hashCode() {
return Objects.hash(host, boundHosts, name, version, roles);
}

/**
* Role information about an Elasticsearch process.
*/
Expand Down
25 changes: 0 additions & 25 deletions client/rest/src/test/java/org/elasticsearch/client/NodeTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,7 @@
import java.util.Arrays;
import java.util.HashSet;

import static java.util.Collections.singleton;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

public class NodeTests extends RestClientTestCase {
public void testWithHost() {
Expand Down Expand Up @@ -64,26 +61,4 @@ public void testToString() {
"nam", "ver", new Roles(true, false, false)).toString());

}

public void testEqualsAndHashCode() {
HttpHost host = new HttpHost(randomAsciiAlphanumOfLength(5));
Node node = new Node(host,
randomBoolean() ? null : singleton(host),
randomBoolean() ? null : randomAsciiAlphanumOfLength(5),
randomBoolean() ? null : randomAsciiAlphanumOfLength(5),
randomBoolean() ? null : new Roles(true, true, true));
assertFalse(node.equals(null));
assertTrue(node.equals(node));
assertEquals(node.hashCode(), node.hashCode());
Node copy = new Node(host, node.getBoundHosts(), node.getName(), node.getVersion(), node.getRoles());
assertTrue(node.equals(copy));
assertEquals(node.hashCode(), copy.hashCode());
assertFalse(node.equals(new Node(new HttpHost(host.toHostString() + "changed"), node.getBoundHosts(),
node.getName(), node.getVersion(), node.getRoles())));
assertFalse(node.equals(new Node(host, new HashSet<>(Arrays.asList(host, new HttpHost(host.toHostString() + "changed"))),
node.getName(), node.getVersion(), node.getRoles())));
assertFalse(node.equals(new Node(host, node.getBoundHosts(), node.getName() + "changed", node.getVersion(), node.getRoles())));
assertFalse(node.equals(new Node(host, node.getBoundHosts(), node.getName(), node.getVersion() + "changed", node.getRoles())));
assertFalse(node.equals(new Node(host, node.getBoundHosts(), node.getName(), node.getVersion(), new Roles(false, false, false))));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,16 @@

import java.io.IOException;
import java.io.InputStream;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

import com.fasterxml.jackson.core.JsonFactory;

import static org.elasticsearch.client.sniff.SnifferTests.assertNodesEquals;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.hasSize;
import static org.junit.Assert.assertThat;
Expand All @@ -53,50 +57,53 @@ private void checkFile(String file, Node... expected) throws IOException {
try {
HttpEntity entity = new InputStreamEntity(in, ContentType.APPLICATION_JSON);
List<Node> nodes = ElasticsearchNodesSniffer.readHosts(entity, Scheme.HTTP, new JsonFactory());
// Use these assertions because the error messages are nicer than hasItems.
assertThat(nodes, hasSize(expected.length));
for (Node expectedNode : expected) {
assertThat(nodes, hasItem(expectedNode));
}
// Sort he list so the error messages are easier to read.
Collections.sort(nodes, new Comparator<Node>() {
@Override
public int compare(Node lhs, Node rhs) {
return lhs.getName().compareTo(rhs.getName());
}
});
assertNodesEquals(Arrays.asList(expected), nodes);
} finally {
in.close();
}
}

public void test2x() throws IOException {
checkFile("2.0.0_nodes_http.json",
node(9200, "m1", "2.0.0", true, false, false),
node(9202, "m2", "2.0.0", true, true, false),
node(9201, "m3", "2.0.0", true, false, false),
node(9207, "c1", "2.0.0", false, false, false),
node(9206, "c2", "2.0.0", false, false, false),
node(9205, "d1", "2.0.0", false, true, false),
node(9204, "d2", "2.0.0", false, true, false),
node(9203, "d3", "2.0.0", false, true, false),
node(9207, "c1", "2.0.0", false, false, false),
node(9206, "c2", "2.0.0", false, false, false));
node(9200, "m1", "2.0.0", true, false, false),
node(9202, "m2", "2.0.0", true, true, false),
node(9201, "m3", "2.0.0", true, false, false));
}

public void test5x() throws IOException {
checkFile("5.0.0_nodes_http.json",
node(9200, "m1", "5.0.0", true, false, true),
node(9201, "m2", "5.0.0", true, true, true),
node(9202, "m3", "5.0.0", true, false, true),
node(9206, "c1", "5.0.0", false, false, true),
node(9207, "c2", "5.0.0", false, false, true),
node(9203, "d1", "5.0.0", false, true, true),
node(9204, "d2", "5.0.0", false, true, true),
node(9205, "d3", "5.0.0", false, true, true),
node(9206, "c1", "5.0.0", false, false, true),
node(9207, "c2", "5.0.0", false, false, true));
node(9200, "m1", "5.0.0", true, false, true),
node(9201, "m2", "5.0.0", true, true, true),
node(9202, "m3", "5.0.0", true, false, true));
}

public void test6x() throws IOException {
checkFile("6.0.0_nodes_http.json",
node(9200, "m1", "6.0.0", true, false, true),
node(9201, "m2", "6.0.0", true, true, true),
node(9202, "m3", "6.0.0", true, false, true),
node(9206, "c1", "6.0.0", false, false, true),
node(9207, "c2", "6.0.0", false, false, true),
node(9203, "d1", "6.0.0", false, true, true),
node(9204, "d2", "6.0.0", false, true, true),
node(9205, "d3", "6.0.0", false, true, true),
node(9206, "c1", "6.0.0", false, false, true),
node(9207, "c2", "6.0.0", false, false, true));
node(9200, "m1", "6.0.0", true, false, true),
node(9201, "m2", "6.0.0", true, true, true),
node(9202, "m3", "6.0.0", true, false, true));
}

private Node node(int port, String name, String version, boolean master, boolean data, boolean ingest) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import java.util.Map;
import java.util.Set;

import static org.elasticsearch.client.sniff.SnifferTests.assertNodesEquals;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.startsWith;
Expand Down Expand Up @@ -120,7 +121,7 @@ public void testSniffNodes() throws IOException {
if (sniffResponse.isFailure) {
fail("sniffNodes should have failed");
}
assertEquals(sniffResponse.result, sniffedNodes);
assertNodesEquals(sniffResponse.result, sniffedNodes);
} catch(ResponseException e) {
Response response = e.getResponse();
if (sniffResponse.isFailure) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Set;
import java.util.concurrent.CancellationException;
Expand All @@ -56,7 +57,7 @@
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotSame;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertThat;
Expand Down Expand Up @@ -111,11 +112,11 @@ public void shutdown() {
fail("should have failed given that nodesSniffer says it threw an exception");
} else if (nodesSniffer.emptyList.get() > emptyList) {
emptyList++;
assertEquals(lastNodes, restClient.getNodes());
assertNodesEquals(lastNodes, restClient.getNodes());
} else {
assertNotEquals(lastNodes, restClient.getNodes());
assertNotSame(lastNodes, restClient.getNodes());
List<Node> expectedNodes = CountingNodesSniffer.buildNodes(runs);
assertEquals(expectedNodes, restClient.getNodes());
assertNodesEquals(expectedNodes, restClient.getNodes());
lastNodes = restClient.getNodes();
}
} catch(IOException e) {
Expand Down Expand Up @@ -653,4 +654,23 @@ public void testDefaultSchedulerShutdown() throws Exception {
verify(executor, times(2)).awaitTermination(1000, TimeUnit.MILLISECONDS);
verifyNoMoreInteractions(executor);
}

static final void assertNodesEquals(List<Node> expected, List<Node> actual) {
try {
assertEquals(expected.size(), actual.size());
Iterator<Node> expectedItr = expected.iterator();
Iterator<Node> actualItr = actual.iterator();
while (expectedItr.hasNext()) {
Node expectedNode = expectedItr.next();
Node actualNode = actualItr.next();
assertEquals(expectedNode.getHost(), actualNode.getHost());
assertEquals(expectedNode.getBoundHosts(), actualNode.getBoundHosts());
assertEquals(expectedNode.getName(), actualNode.getName());
assertEquals(expectedNode.getVersion(), actualNode.getVersion());
assertEquals(expectedNode.getRoles(), actualNode.getRoles());
}
} catch (AssertionError e) {
throw new AssertionError("nodes differ expected: " + expected + " but was: " + actual, e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -406,8 +406,8 @@ private void sniffHostMetadata(RestClient client) throws IOException {
ElasticsearchNodesSniffer.Scheme.valueOf(getProtocol().toUpperCase(Locale.ROOT));
/*
* We don't want to change the list of nodes that the client communicates with
* because that'd just be rude. So instead we replace the nodes find the nodes
* returned by the sniffer that correspond with the nodes already the client
* because that'd just be rude. So instead we find the nodes
* returned by the sniffer that correspond with the nodes already in the client
* and set the nodes to them. That *shouldn't* change the nodes that the client
* communicates with.
*/
Expand Down

0 comments on commit d551dcb

Please sign in to comment.