[geode] branch develop updated: GEODE-7914: create missing unit test for Redis Module Expire Command (#4852)

[je...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jensdeppe pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new fb2c273  GEODE-7914: create missing unit test for Redis Module Expire Command (#4852)
fb2c273 is described below

commit fb2c273c3e9307e8956c7127d208a50b4bbb1a43
Author: John Hutchison <jh...@gmail.com>
AuthorDate: Mon Mar 30 13:50:26 2020 -0400

    GEODE-7914: create missing unit test for Redis Module Expire Command (#4852)
    
    
    Co-authored-by: John Hutchison <jh...@pivotal.io>
    Co-authored-by: Jens Deppe <jd...@pivotal.io>
---
 .../geode/redis/general/ExpireIntegrationTest.java |   6 +-
 .../redis/internal/executor/ExpireExecutor.java    |   3 +-
 .../executor/general/ExpireExecutorJUnitTest.java  | 114 +++++++++++++++++++++
 3 files changed, 119 insertions(+), 4 deletions(-)

diff --git a/geode-redis/src/integrationTest/java/org/apache/geode/redis/general/ExpireIntegrationTest.java b/geode-redis/src/integrationTest/java/org/apache/geode/redis/general/ExpireIntegrationTest.java
index a2dc500..917670a 100644
--- a/geode-redis/src/integrationTest/java/org/apache/geode/redis/general/ExpireIntegrationTest.java
+++ b/geode-redis/src/integrationTest/java/org/apache/geode/redis/general/ExpireIntegrationTest.java
@@ -455,7 +455,7 @@ public class ExpireIntegrationTest {
 
   @Test
   @Ignore("this test needs to pass to have feature parity with native redis")
-  public void SettingExiprationToNegativeValue_ShouldDeleteKey() throws InterruptedException {
+  public void SettingExiprationToNegativeValue_ShouldDeleteKey() {
 
     String key = "key";
     String value = "value";
@@ -464,8 +464,8 @@ public class ExpireIntegrationTest {
     Long expirationWasSet = jedis.expire(key, -5);
     assertThat(expirationWasSet).isEqualTo(1);
 
-    String actualValue = jedis.get(key);
-    assertThat(actualValue).isNull();
+    Boolean keyExists = jedis.exists(key);
+    assertThat(keyExists).isTrue();
   }
 
 
diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/ExpireExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/ExpireExecutor.java
index bb458c7..d76bf53 100755
--- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/ExpireExecutor.java
+++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/ExpireExecutor.java
@@ -38,10 +38,11 @@ public class ExpireExecutor extends AbstractExecutor implements Extendable {
   public void executeCommand(Command command, ExecutionHandlerContext context) {
     List<byte[]> commandElems = command.getProcessedCommand();
 
-    if (commandElems.size() < 3) {
+    if (commandElems.size() != 3) {
       command.setResponse(Coder.getErrorResponse(context.getByteBufAllocator(), getArgsError()));
       return;
     }
+
     ByteArrayWrapper key = command.getKey();
     RegionProvider regionProvider = context.getRegionProvider();
     byte[] delayByteArray = commandElems.get(SECONDS_INDEX);
diff --git a/geode-redis/src/test/java/org/apache/geode/redis/internal/executor/general/ExpireExecutorJUnitTest.java b/geode-redis/src/test/java/org/apache/geode/redis/internal/executor/general/ExpireExecutorJUnitTest.java
new file mode 100644
index 0000000..b1603db
--- /dev/null
+++ b/geode-redis/src/test/java/org/apache/geode/redis/internal/executor/general/ExpireExecutorJUnitTest.java
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.redis.internal.executor.general;
+
+import static java.nio.charset.Charset.defaultCharset;
+import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.UnpooledByteBufAllocator;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+
+import org.apache.geode.redis.internal.Command;
+import org.apache.geode.redis.internal.ExecutionHandlerContext;
+import org.apache.geode.redis.internal.Executor;
+import org.apache.geode.redis.internal.executor.ExpireExecutor;
+
+public class ExpireExecutorJUnitTest {
+
+  private ExecutionHandlerContext context;
+  private Command command;
+  private UnpooledByteBufAllocator byteBuf;
+
+  @Before
+  public void setUp() {
+    context = mock(ExecutionHandlerContext.class);
+    command = mock(Command.class);
+    byteBuf = new UnpooledByteBufAllocator(false);
+  }
+
+  @Test
+  public void calledWithTooFewCommandArguments_returnsError() {
+    Executor executor = new ExpireExecutor();
+    List<byte[]> commandsAsBytesWithTooFewArguments = new ArrayList<>();
+    commandsAsBytesWithTooFewArguments.add("EXPIRE".getBytes());
+    commandsAsBytesWithTooFewArguments.add("key".getBytes());
+
+    ArgumentCaptor<ByteBuf> argsErrorCaptor = ArgumentCaptor.forClass(ByteBuf.class);
+
+    when(context.getByteBufAllocator()).thenReturn(byteBuf);
+    when(command.getProcessedCommand()).thenReturn(commandsAsBytesWithTooFewArguments);
+
+    executor.executeCommand(command, context);
+    verify(command, times(1)).setResponse(argsErrorCaptor.capture());
+
+    List<ByteBuf> capturedErrors = argsErrorCaptor.getAllValues();
+    assertThat(capturedErrors.get(0).toString(defaultCharset()))
+        .startsWith("-ERR The wrong number of arguments");
+  }
+
+  @Test
+  public void calledWithTooManyCommandArguments_returnsErrorMessage() {
+    Executor executor = new ExpireExecutor();
+    List<byte[]> commandsAsBytesWithTooManyArguments = new ArrayList<>();
+    commandsAsBytesWithTooManyArguments.add("EXPIRE".getBytes());
+    commandsAsBytesWithTooManyArguments.add("key".getBytes());
+    commandsAsBytesWithTooManyArguments.add("100".getBytes());
+    commandsAsBytesWithTooManyArguments.add("Bonus!".getBytes());
+
+    ArgumentCaptor<ByteBuf> argsErrorCaptor = ArgumentCaptor.forClass(ByteBuf.class);
+
+    when(context.getByteBufAllocator()).thenReturn(byteBuf);
+    when(command.getProcessedCommand()).thenReturn(commandsAsBytesWithTooManyArguments);
+
+    executor.executeCommand(command, context);
+    verify(command, times(1)).setResponse(argsErrorCaptor.capture());
+
+    List<ByteBuf> capturedErrors = argsErrorCaptor.getAllValues();
+    assertThat(capturedErrors.get(0).toString(defaultCharset()))
+        .startsWith("-ERR The wrong number of arguments");
+  }
+
+  @Test
+  public void calledWithInvalidCommandArguments_returnsErrorMessage() {
+    Executor executor = new ExpireExecutor();
+    List<byte[]> commandsAsBytesWithTooManyArguments = new ArrayList<>();
+    commandsAsBytesWithTooManyArguments.add("EXPIRE".getBytes());
+    commandsAsBytesWithTooManyArguments.add("key".getBytes());
+    commandsAsBytesWithTooManyArguments.add("not a number".getBytes());
+
+    ArgumentCaptor<ByteBuf> argsErrorCaptor = ArgumentCaptor.forClass(ByteBuf.class);
+
+    when(context.getByteBufAllocator()).thenReturn(byteBuf);
+    when(command.getProcessedCommand()).thenReturn(commandsAsBytesWithTooManyArguments);
+
+    executor.executeCommand(command, context);
+    verify(command, times(1)).setResponse(argsErrorCaptor.capture());
+
+    List<ByteBuf> capturedErrors = argsErrorCaptor.getAllValues();
+    assertThat(capturedErrors.get(0).toString(defaultCharset()))
+        .startsWith("-ERR The number of seconds specified must be numeric");
+  }
+}