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 <jhutchi...@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 <jhutchi...@pivotal.io> Co-authored-by: Jens Deppe <jde...@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"); + } +}