mrniko/redisson — pull request #6 by brotherdetjr (2014-02-15 19:09:05+00:00)
Title: Added promises to make redisson truly async

Hi Nikita,

Please find my pull request for issue #5 and comments.

One option was to make Command class extending CompletableFuture instead of implementing Future. But doing this I had to resolve conflicts between both implementations. Should I simply override CompletableFuture's methods? I guess not. Especially considering strange semantics of cancel(boolean) method. Formally it doesn't violate any contract, but seems implying double calls of this method. Also it seems thread unsafe hence it uses check-and-act pattern:
```
if (latch.getCount() == 1) {
latch.countDown();
...
}
```
This is suspicious. Probably we need to contact Will Glozer to clarify.
Thus I introduced FutureWithPromise interface and propose RedisAsyncConnection to return it instead of sheer Future. Thus we get less intrusive implementation and some separation of concerns. Of course it's not perfect, because getPromise() returns CompletableFuture instead of sheer promise object. But Java 8 has such a bloated class and nothing else.
I considered using other promise implementations (rx-promises, jdeferred), but Java's is the only full-fledged currently. Guava has cool ListenableFuture and so on, but they are not promises, while I wanted to get promises right off the bat.
Probably it's worth considering Guava's futures to make the library Java-version-agnostic. Then one may provide satellite library specially for promises.
Anyway dealing with simple Futures neutralize non-blocking I/O benefits. To use Future#get() we have to spawn new thread. This brings us back to threading overhead. In proposed implementation callbacks are called within nettyEventLoopGroup threads.

Tests will be provided when agreed on implementation, but here's the code snippet in Groovy to demonstrate how it works:
```
def client = new RedisClient(..., ...)
connection = client.connectAsync()

connection.flushdb().promise.thenCompose { x ->
connection.get('foo').promise
}.thenCompose { x ->
assertThat x, nullValue()
connection.set('foo', 'bar').promise
}.thenCompose {
connection.get('foo').promise
}.thenAccept { x ->
assertThat x, is('bar')
}.join() // join() is only for unit-testing purposes
```
Similar code will be in Java 8.

Could you also explain your comment on multiple operations? As far as I can see proposed solution doesn't affect the sequence of operations.

Cheers,
Denis