Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

client: add AutoCloseable #244

Merged
merged 1 commit into from
Sep 30, 2017
Merged

client: add AutoCloseable #244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion jetcd-core/src/main/java/com/coreos/jetcd/Client.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
* <p>The implementation may throw unchecked ConnectException or AuthFailedException on
* initialization (or when invoking *Client methods if configured to initialize lazily).
*/
public interface Client {
public interface Client extends AutoCloseable {

Auth getAuthClient();

Expand All @@ -36,6 +36,7 @@ public interface Client {

Watch getWatchClient();

@Override
void close();

static ClientBuilder builder() {
Expand Down
3 changes: 2 additions & 1 deletion jetcd-core/src/main/java/com/coreos/jetcd/Watch.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,12 @@ public interface Watch extends CloseableClient {
/**
* Interface of Watcher.
*/
interface Watcher {
interface Watcher extends AutoCloseable {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why extends not implement the AutoCloseable?

if we @Override the close() from AutoCloseable, how come our close() doesn't throw Exception?

Copy link
Contributor Author

@ScienJus ScienJus Sep 30, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Interface can only extends another interface. not implement.

  2. The overriding method can throw those checked exceptions, which have less scope than the exception(s) declared in the overridden method.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now i understand what you were doing. pretty cool java trick.


/**
* closes this watcher and all its resources.
**/
@Override
void close();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AutoCloseable::close() throws an exception, I do not think we should change the signature

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the point is whether to throw out exception on Watch#close, so far it’s prohibited.

We uses AutoCloseable just for try-with-resource, it should not be a constraint.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so lgtm :)


/**
Expand Down
4 changes: 3 additions & 1 deletion jetcd-core/src/main/java/com/coreos/jetcd/internal/impl/CloseableClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@

package com.coreos.jetcd.internal.impl;

public interface CloseableClient {
public interface CloseableClient extends AutoCloseable {

/**
* close the client and release its resources.
*/
@Override
default void close() {
// noop
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as before

}
Expand Down
40 changes: 15 additions & 25 deletions jetcd-core/src/test/java/com/coreos/jetcd/internal/impl/LoadBalancerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,15 @@ public class LoadBalancerTest {

@Test
public void testPickFirstBalancerFactory() throws Exception {
Client client = Client.builder()
.endpoints(TestConstants.endpoints)
.loadBalancerFactory(PickFirstBalancerFactory.getInstance())
.build();
try (Client client = Client.builder()
.endpoints(TestConstants.endpoints)
.loadBalancerFactory(PickFirstBalancerFactory.getInstance())
.build();

KV kv = client.getKVClient();
KV kv = client.getKVClient()) {
PutResponse response;
long lastMemberId = 0;

PutResponse response;
long lastMemberId = 0;

try {
for (int i = 0; i < TestConstants.endpoints.length * 2; i++) {
response = kv.put(TestUtil.randomByteSequence(), TestUtil.randomByteSequence()).get();

Expand All @@ -55,26 +53,21 @@ public void testPickFirstBalancerFactory() throws Exception {

assertThat(response.getHeader().getMemberId()).isEqualTo(lastMemberId);
}
} finally {
kv.close();
client.close();
}
}

@Test
public void testRoundRobinLoadBalancerFactory() throws Exception {
Client client = Client.builder()
.endpoints(TestConstants.endpoints)
.loadBalancerFactory(RoundRobinLoadBalancerFactory.getInstance())
.build();

KV kv = client.getKVClient();

PutResponse response;
long lastMemberId = 0;
long differences = 0;
try (Client client = Client.builder()
.endpoints(TestConstants.endpoints)
.loadBalancerFactory(RoundRobinLoadBalancerFactory.getInstance())
.build();
KV kv = client.getKVClient()) {
PutResponse response;
long lastMemberId = 0;
long differences = 0;

try {
for (int i = 0; i < TestConstants.endpoints.length; i++) {
response = kv.put(TestUtil.randomByteSequence(), TestUtil.randomByteSequence()).get();

Expand All @@ -86,9 +79,6 @@ public void testRoundRobinLoadBalancerFactory() throws Exception {
}

assertThat(differences).isNotEqualTo(lastMemberId);
} finally {
kv.close();
client.close();
}
}
}
10 changes: 2 additions & 8 deletions jetcd-core/src/test/java/com/coreos/jetcd/internal/impl/WatchTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,13 @@ public void tearDown() {
public void testWatchOnPut() throws ExecutionException, InterruptedException {
ByteSequence key = ByteSequence.fromString(TestUtil.randomString());
ByteSequence value = ByteSequence.fromString("value");
Watcher watcher = watchClient.watch(key);
try {
try (Watcher watcher = watchClient.watch(key)) {
kvClient.put(key, value).get();

WatchResponse response = watcher.listen();
assertThat(response.getEvents().size()).isEqualTo(1);
assertThat(response.getEvents().get(0).getEventType()).isEqualTo(EventType.PUT);
assertThat(response.getEvents().get(0).getKeyValue().getKey()).isEqualTo(key);
} finally {
watcher.close();
}
}

Expand All @@ -80,16 +77,13 @@ public void testWatchOnDelete() throws ExecutionException, InterruptedException
ByteSequence key = ByteSequence.fromString(TestUtil.randomString());
ByteSequence value = ByteSequence.fromString("value");
kvClient.put(key, value).get();
Watcher watcher = watchClient.watch(key);
try {
try (Watcher watcher = watchClient.watch(key)) {
kvClient.delete(key);
WatchResponse response = watcher.listen();
assertThat(response.getEvents().size()).isEqualTo(1);
WatchEvent event = response.getEvents().get(0);
assertThat(event.getEventType()).isEqualTo(EventType.DELETE);
assertThat(Arrays.equals(event.getKeyValue().getKey().getBytes(), key.getBytes())).isTrue();
} finally {
watcher.close();
}
}
}
Expand Down
Loading