DEL one or more keys per spec. Fix for issue 13: http://github.com/al…

…phazero/jredis/issues#issue/13
alphazero · Apr 3, 2010 · 5a3d2e8 · 5a3d2e8
1 parent c8fa0f5
commit 5a3d2e8
Show file tree

Hide file tree

Showing 9 changed files with 160 additions and 28 deletions.
diff --git a/core/api/src/main/java/org/jredis/JRedis.java b/core/api/src/main/java/org/jredis/JRedis.java
@@ -261,11 +261,12 @@ public interface JRedis {
 
 	/**
 	 * @Redis DEL
-	 * @param key
-	 * @return
+	 * @param keys one or more non-null, non-zero-length, keys to be deleted
+	 * @return number of keys actually deleted
 	 * @throws RedisException
 	 */
-	public boolean del (String key) throws RedisException;
+//	public boolean del (String key) throws RedisException;
+	public long del (String ... keys) throws RedisException;
 
 	/**
 	 * @Redis TYPE

diff --git a/core/api/src/main/java/org/jredis/JRedisFuture.java b/core/api/src/main/java/org/jredis/JRedisFuture.java
@@ -253,10 +253,10 @@ public interface JRedisFuture {
 
 	/**
 	 * @Redis DEL
-	 * @param key
-	 * @return
+	 * @param keys one or more, non-null, non-zero-length, keys to be deleted
+	 * @return Future<Long> of number keys actually deleted.
 	 */
-	public Future<Boolean> del (String key);
+	public Future<Long> del (String ... keys);
 
 	/**
 	 * @Redis TYPE

diff --git a/core/api/src/main/java/org/jredis/protocol/Command.java b/core/api/src/main/java/org/jredis/protocol/Command.java
@@ -55,7 +55,7 @@ public enum Command {
 	DECR		(RequestType.KEY, 			ResponseType.NUMBER), 
 	DECRBY		(RequestType.KEY_NUM,		ResponseType.NUMBER),  
 	EXISTS		(RequestType.KEY, 			ResponseType.BOOLEAN), 
-	DEL			(RequestType.KEY, 			ResponseType.BOOLEAN), 
+	DEL			(RequestType.MULTI_KEY, 	ResponseType.NUMBER), 
 	TYPE		(RequestType.KEY, 			ResponseType.STRING),
 	SUBSTR		(RequestType.KEY_NUM_NUM,	ResponseType.BULK),
 	APPEND		(RequestType.KEY_VALUE, 	ResponseType.NUMBER),

diff --git a/core/ri/src/main/java/org/jredis/ri/alphazero/JRedisFutureSupport.java b/core/ri/src/main/java/org/jredis/ri/alphazero/JRedisFutureSupport.java
@@ -1076,13 +1076,20 @@ public FutureStatus sdiffstore(String dest, String... sets) {
 	}
 
 //	@Override
-	public Future<Boolean> del(String key) {
-		byte[] keybytes = null;
-		if((keybytes = getKeyBytes(key)) == null) 
-			throw new IllegalArgumentException ("invalid key => ["+key+"]");
+	public Future<Long> del(String ... keys) {
+		if(null == keys || keys.length == 0) throw new IllegalArgumentException("no keys specified");
+		byte[] keydata = null;
+		byte[][] keybytes = new byte[keys.length][];
+		int i=0;
+		for(String k : keys) {
+			if((keydata = getKeyBytes(k)) == null) 
+				throw new IllegalArgumentException ("invalid key => ["+k+"] @ index: " + i);
+
+			keybytes[i++] = keydata;
+		}
 
 		Future<Response> futureResponse = this.queueRequest(Command.DEL, keybytes);
-		return new FutureBoolean(futureResponse);
+		return new FutureLong(futureResponse);
 	}
 
 

diff --git a/core/ri/src/main/java/org/jredis/ri/alphazero/JRedisSupport.java b/core/ri/src/main/java/org/jredis/ri/alphazero/JRedisSupport.java
@@ -1592,15 +1592,23 @@ public void sdiffstore(String dest, String... sets) throws RedisException {
 	}
 
 //	@Override
-	public boolean del(String key) throws RedisException {
-		byte[] keybytes = null;
-		if((keybytes = getKeyBytes(key)) == null) 
-			throw new IllegalArgumentException ("invalid key => ["+key+"]");
+	public long del(String ...keys ) throws RedisException {
+
+		if(null == keys || keys.length == 0) throw new IllegalArgumentException("no keys specified");
+		byte[] keydata = null;
+		byte[][] keybytes = new byte[keys.length][];
+		int i=0;
+		for(String k : keys) {
+			if((keydata = getKeyBytes(k)) == null) 
+				throw new IllegalArgumentException ("invalid key => ["+k+"] @ index: " + i);
+
+			keybytes[i++] = keydata;
+		}
 
-		boolean resvalue = false;
+		long resvalue = -1;
 		try {
 			ValueResponse valResponse = (ValueResponse) this.serviceRequest(Command.DEL, keybytes);
-			resvalue = valResponse.getBooleanValue();
+			resvalue = valResponse.getLongValue();
 		}
 		catch (ClassCastException e){
 			throw new ProviderException("Expecting a ValueResponse here => " + e.getLocalizedMessage(), e);

diff --git a/core/ri/src/test/java/org/jredis/ri/alphazero/ConcurrentJRedisProviderTestsBase.java b/core/ri/src/test/java/org/jredis/ri/alphazero/ConcurrentJRedisProviderTestsBase.java
@@ -73,7 +73,8 @@ public void testConcurrentBooleanCommands() {
 		Log.log("CONCURRENT TEST: %s resp type command | key: %s", cmd, key);
 		try {
 			provider.set (key, threadName);
-			assertTrue (provider.del (key), "del response should be OK");
+//			assertTrue (provider.del (key), "del response should be OK");
+			assertEquals (provider.del(key), 1, "del response should be 1 for one key deleted");
 			assertFalse (provider.exists(key), "deleted key response should not exist");
 		} 
 		catch (RedisException e) { fail(cmd + " ERROR => " + e.getLocalizedMessage(), e); }

diff --git a/core/ri/src/test/java/org/jredis/ri/alphazero/JRedisFutureProviderTestsBase.java b/core/ri/src/test/java/org/jredis/ri/alphazero/JRedisFutureProviderTestsBase.java
@@ -1315,12 +1315,69 @@ public void testDel() throws InterruptedException {
 			String key = this.keys.get(0);
 			provider.set (key, dataList.get(0));
 			Future<Boolean> existsResp1 = provider.exists(key); 
-			provider.del(key);
+			Future<Long> delResp = provider.del(key);
 			Future<Boolean> existsResp2 = provider.exists(key); 
-			
+
 			try {
+				assertEquals(delResp.get().longValue(), 1, "one key should been deleted");
 				assertTrue (existsResp1.get(), "After set key should exist");
 				assertFalse (existsResp2.get(), "After del key should not exist");
+
+				// delete many keys
+				provider.flushdb();
+				for(int i=0; i<SMALL_CNT; i++) provider.set(stringList.get(i), dataList.get(i));
+
+				String[] keysToDel = new String[SMALL_CNT];
+				for(int i=0; i<SMALL_CNT; i++) keysToDel[i] = stringList.get(i);
+
+				Future<Long> delCnt1 = provider.del(keysToDel);
+				for(int i=0; i<SMALL_CNT; i++) assertFalse (provider.exists(stringList.get(i)).get(), "key should have been deleted");
+				assertEquals(delCnt1.get().longValue(), SMALL_CNT, "SMALL_CNT keys were deleted");
+
+				// delete many keys but also spec one non existent keys - delete result should be less than key cnt
+				provider.flushdb();
+				for(int i=0; i<SMALL_CNT-1; i++) provider.set(stringList.get(i), dataList.get(i));
+
+				keysToDel = new String[SMALL_CNT];
+				for(int i=0; i<SMALL_CNT; i++) keysToDel[i] = stringList.get(i);
+
+				Future<Long> delCnt2 = provider.del(keysToDel);
+				for(int i=0; i<SMALL_CNT; i++) assertFalse (provider.exists(stringList.get(i)).get(), "key should have been deleted");
+				assertEquals(delCnt2.get().longValue(), SMALL_CNT-1, "SMALL_CNT-1 keys were actually deleted");
+
+				// edge cases
+				// all should through exceptions
+				boolean didRaiseEx;
+				didRaiseEx = false;
+				try {
+					String[] keys = null;
+					provider.del(keys).get();
+				}
+				catch (IllegalArgumentException e) {didRaiseEx = true;}
+				catch (Throwable whatsthis) { fail ("unexpected exception raised", whatsthis);}
+				if(!didRaiseEx){ fail ("Expected exception not raised."); }
+
+				didRaiseEx = false;
+				try {
+					String[] keys = new String[0];
+					provider.del(keys).get();
+				}
+				catch (IllegalArgumentException e) {didRaiseEx = true;}
+				catch (Throwable whatsthis) { fail ("unexpected exception raised", whatsthis);}
+				if(!didRaiseEx){ fail ("Expected exception not raised."); }
+
+				didRaiseEx = false;
+				try {
+					String[] keys = new String[3];
+					keys[0] = stringList.get(0);
+					keys[1] = null;
+					keys[2] = stringList.get(2);
+					provider.del(keys).get();
+				}
+				catch (IllegalArgumentException e) {didRaiseEx = true;}
+				catch (Throwable whatsthis) { fail ("unexpected exception raised", whatsthis);}
+				if(!didRaiseEx){ fail ("Expected exception not raised."); }
+
 			}
 			catch(ExecutionException e){
 				Throwable cause = e.getCause();

diff --git a/core/ri/src/test/java/org/jredis/ri/alphazero/JRedisProviderTestsBase.java b/core/ri/src/test/java/org/jredis/ri/alphazero/JRedisProviderTestsBase.java
@@ -865,8 +865,66 @@ public void testDel() {
 			provider.set (key, dataList.get(0));
 			assertTrue (provider.exists(key));
 
-			provider.del(key);
+			long delCnt;
+			delCnt = provider.del(key);
 			assertFalse (provider.exists(key));
+			assertEquals(delCnt, 1, "one key was deleted");
+
+			// delete many keys
+			provider.flushdb();
+			for(int i=0; i<SMALL_CNT; i++) provider.set(stringList.get(i), dataList.get(i));
+
+			String[] keysToDel = new String[SMALL_CNT];
+			for(int i=0; i<SMALL_CNT; i++) keysToDel[i] = stringList.get(i);
+
+			delCnt = provider.del(keysToDel);
+			for(int i=0; i<SMALL_CNT; i++) assertFalse (provider.exists(stringList.get(i)), "key should have been deleted");
+			assertEquals(delCnt, SMALL_CNT, "SMALL_CNT keys were deleted");
+
+			// delete many keys but also spec one non existent keys - delete result should be less than key cnt
+			provider.flushdb();
+			for(int i=0; i<SMALL_CNT-1; i++) provider.set(stringList.get(i), dataList.get(i));
+
+			keysToDel = new String[SMALL_CNT];
+			for(int i=0; i<SMALL_CNT; i++) keysToDel[i] = stringList.get(i);
+
+			delCnt = provider.del(keysToDel);
+			for(int i=0; i<SMALL_CNT; i++) assertFalse (provider.exists(stringList.get(i)), "key should have been deleted");
+			assertEquals(delCnt, SMALL_CNT-1, "SMALL_CNT-1 keys were actually deleted");
+
+
+			// edge cases
+			// all should through exceptions
+			boolean didRaiseEx;
+			didRaiseEx = false;
+			try {
+				String[] keys = null;
+				provider.del(keys);
+			}
+			catch (IllegalArgumentException e) {didRaiseEx = true;}
+			catch (Throwable whatsthis) { fail ("unexpected exception raised", whatsthis);}
+			if(!didRaiseEx){ fail ("Expected exception not raised."); }
+
+			didRaiseEx = false;
+			try {
+				String[] keys = new String[0];
+				provider.del(keys);
+			}
+			catch (IllegalArgumentException e) {didRaiseEx = true;}
+			catch (Throwable whatsthis) { fail ("unexpected exception raised", whatsthis);}
+			if(!didRaiseEx){ fail ("Expected exception not raised."); }
+
+			didRaiseEx = false;
+			try {
+				String[] keys = new String[3];
+				keys[0] = stringList.get(0);
+				keys[1] = null;
+				keys[2] = stringList.get(2);
+				provider.del(keys);
+			}
+			catch (IllegalArgumentException e) {didRaiseEx = true;}
+			catch (Throwable whatsthis) { fail ("unexpected exception raised", whatsthis);}
+			if(!didRaiseEx){ fail ("Expected exception not raised."); }
 		} 
 		catch (RedisException e) { fail(cmd + " ERROR => " + e.getLocalizedMessage(), e); }
 	}

diff --git a/examples/src/main/java/org/jredis/examples/PipelineInAction.java b/examples/src/main/java/org/jredis/examples/PipelineInAction.java
@@ -209,8 +209,8 @@ private static void runJRedisPipelineLPUSH (ConnectionSpec spec, int reqCnt, int
     		String key = "pipeKey";
     		byte[] data = new byte[size];
     		(new Random()).nextBytes(data);
-			Future<Boolean> futureBool = pipeline.del(key);
-			futureBool.get();
+			Future<Long> futureLong = pipeline.del(key);
+			futureLong.get();
     		do {
 	    		int cnt = 0;
 	    		Util.Timer timer = Timer.startNewTimer();
@@ -264,8 +264,8 @@ private static void runJRedisPipelineSET (ConnectionSpec spec, int reqCnt, int s
     		String key = "pipeKey";
     		byte[] data = new byte[size];
     		(new Random()).nextBytes(data);
-			Future<Boolean> futureBool = pipeline.del(key);
-			futureBool.get();
+			Future<Long> futureLong = pipeline.del(key);
+			futureLong.get();
     		do {
 	    		int cnt = 0;
 	    		Util.Timer timer = Timer.startNewTimer();
@@ -316,8 +316,8 @@ private static void runJRedisPipelineINCR (ConnectionSpec spec, int reqCnt, int
     	long iters = 0;
     	try {
     		String key = "pipeCounter";
-			Future<Boolean> futureBool = pipeline.del(key);
-			futureBool.get();
+			Future<Long> futureDelCnt = pipeline.del(key);
+			futureDelCnt.get();
     		do {
 	    		int cnt = 0;
 	    		Util.Timer timer = Timer.startNewTimer();