Complete write promise successfully on no-op writes #764

CommandHandler now completes a write promise if the write completes without actually writing a message. Previously, no-op writes did not complete and rendered an invalid state (e.g. queue size was not decremented).
redis · Apr 23, 2018 · 275f609 · 275f609
1 parent 0366ad7
commit 275f609
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 6 deletions.
diff --git a/src/main/java/com/lambdaworks/redis/protocol/CommandHandler.java b/src/main/java/com/lambdaworks/redis/protocol/CommandHandler.java
@@ -799,19 +799,18 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise)
         }
     }
 
-    private void writeSingleCommand(ChannelHandlerContext ctx, RedisCommand<K, V, ?> command, ChannelPromise promise)
-            throws Exception {
+    private void writeSingleCommand(ChannelHandlerContext ctx, RedisCommand<K, V, ?> command, ChannelPromise promise) {
 
         if (!isWriteable(command)) {
+            promise.trySuccess();
             return;
         }
 
         addToStack(command, promise);
         ctx.write(command, promise);
     }
 
-    private void writeBatch(ChannelHandlerContext ctx, Collection<RedisCommand<K, V, ?>> batch, ChannelPromise promise)
-            throws Exception {
+    private void writeBatch(ChannelHandlerContext ctx, Collection<RedisCommand<K, V, ?>> batch, ChannelPromise promise) {
 
         Collection<RedisCommand<K, V, ?>> deduplicated = new LinkedHashSet<>(batch.size(), 1);
 
@@ -842,6 +841,8 @@ private void writeBatch(ChannelHandlerContext ctx, Collection<RedisCommand<K, V,
 
         if (!deduplicated.isEmpty()) {
             ctx.write(deduplicated, promise);
+        } else {
+            promise.trySuccess();
         }
     }
 

diff --git a/src/test/java/com/lambdaworks/redis/ConnectionCommandTest.java b/src/test/java/com/lambdaworks/redis/ConnectionCommandTest.java
@@ -94,11 +94,14 @@ public void authEmpty() throws Exception {
     public void authReconnect() throws Exception {
         new WithPasswordRequired() {
             @Override
-            public void run(RedisClient client) {
+            public void run(RedisClient client) throws InterruptedException {
+
                 RedisConnection<String, String> connection = client.connect().sync();
                 assertThat(connection.auth(passwd)).isEqualTo("OK");
                 assertThat(connection.set(key, value)).isEqualTo("OK");
                 connection.quit();
+
+                Thread.sleep(100);
                 assertThat(connection.get(key)).isEqualTo(value);
             }
         };

diff --git a/src/test/java/com/lambdaworks/redis/protocol/CommandHandlerTest.java b/src/test/java/com/lambdaworks/redis/protocol/CommandHandlerTest.java
@@ -456,14 +456,27 @@ public void shouldCancelCommandsOnEncoderException() throws Exception {
         assertThat(command.exception).isInstanceOf(EncoderException.class);
     }
 
+    @Test
+    public void shouldNotWriteCancelledCommand() throws Exception {
+
+        command.cancel();
+        sut.write(context, command, promise);
+
+        verifyZeroInteractions(context);
+        assertThat(disconnectedBuffer).isEmpty();
+        verify(promise).trySuccess();
+
+    }
+
     @Test
     public void shouldNotWriteCancelledCommands() throws Exception {
 
         command.cancel();
-        sut.write(context, command, null);
+        sut.write(context, Collections.singleton(command), promise);
 
         verifyZeroInteractions(context);
         assertThat(disconnectedBuffer).isEmpty();
+        verify(promise).trySuccess();
     }
 
     @Test