Refactor delpub-commands, Allow multiple channels

Refactor the implementation of redis commands: DELPUB, DELIEPUB
and DELNEPUB to make more clear which parts of the old implementation
are specific to each command.

Allow multiple channels as parameter in the redis commands
DELIEPUB and DELNEPUB.

New syntaxes:
DELIEPUB key oldvalue channel message [ channel message .. ]
DELNEPUB key oldvalue channel message [ channel message .. ]

Signed-off-by: Arvo Heinonen <arvo.heinonen@nokia.com>
Change-Id: I3dfb86ed4c8951bb395aff053b9c2bb898cfd0a4

Remove old delpub command implementation

After refactoring the old delpub command implementation in
'delPubStringGenericCommand' is no longer used. Remove it.

Signed-off-by: Arvo Heinonen <arvo.heinonen@nokia.com>
Change-Id: I77817744aea6c78226e8cab8aebbd401efe63c40

diff --git a/redismodule/README.md b/redismodule/README.md
index b6133ed..a49c3da 100755 (executable)
--- a/redismodule/README.md
+++ b/redismodule/README.md
@@ -159,17 +159,17 @@ Time complexity: O(N) where N is the number of keys that will be removed + O(N+M
 
 Removes the specified keys and post a message to the given channel if delete key successfully(return >0)
 
-## DELIEPUB key oldvalue channel message
+## DELIEPUB key oldvalue channel message [channel message...]
 
-ime complexity: O(1) + O(1) + O(1) + O(N+M) where N is the number of clients subscribed to the receiving channel and M is the total number of subscribed patterns (by any client)
+Time complexity: O(1) + O(1) + O(1) + O(N_1+M) [ + O(N_2+M) + ...] where N_i are the number of clients subscribed to the corrensponding receiving channel and M is the total number of subscribed patterns (by any client)
 
-Checks a String 'key' for 'oldvalue' equality and delete the key and post a message to the given channel if delete key successfully(return 1)
+If the string corresponding to 'key' is equal to 'oldvalue' then delete the key. If deletion was succesful (delete return value was 1) then post given messages to the corresponding channels.
 
-## DELNEPUB key oldvalue channel message
+## DELNEPUB key oldvalue channel message [channel message...]
 
-Time complexity: O(1) + O(1) + O(1) + O(N+M) where N is the number of clients subscribed to the receiving channel and M is the total number of subscribed patterns (by any client)
+Time complexity: O(1) + O(1) + O(1) + O(N_1+M) [ + O(N_2+M) + ...] where N_i are the number of clients subscribed to the corrensponding receiving channel and M is the total number of subscribed patterns (by any client)
 
-Checks a String 'key' for 'oldvalue' not equality and delete the key and post a message to the given channel if delete key successfully(return 1)
+If the string corresponding to 'key' is not equal to 'oldvalue' then delete the key. If deletion was succesful (delete return value was 1) then post given messages to the corresponding channels.
 
 ## NGET pattern
 

diff --git a/redismodule/src/exstrings.c b/redismodule/src/exstrings.c
index ef3cf5d..6570889 100755 (executable)
--- a/redismodule/src/exstrings.c
+++ b/redismodule/src/exstrings.c
@@ -71,6 +71,11 @@ typedef struct _PubParams {
     size_t length;
 } PubParams;
 
+typedef struct _DelParams {
+    RedisModuleString **keys;
+    size_t length;
+} DelParams;
+
 void multiPubCommand(RedisModuleCtx *ctx, PubParams* pubParams)
 {
     RedisModuleCallReply *reply = NULL;
@@ -467,106 +472,83 @@ int SetXXPub_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int arg
     return setXXNXPubStringCommon(ctx, argv, argc, OBJ_OP_XX);
 }
 
-int delPubStringGenericCommand(RedisModuleCtx *ctx, RedisModuleString **argv,
-                                       int argc, const int flag)
+int delPubStringCommon(RedisModuleCtx *ctx, DelParams *delParamsPtr, PubParams *pubParamsPtr)
 {
-    RedisModuleString *oldvalstr = NULL, *channel = NULL, *message = NULL;
-    RedisModuleCallReply *reply = NULL;
-
-    if (flag == OBJ_OP_NO) {
-        if (argc < 4)
-            return RedisModule_WrongArity(ctx);
-        else {
-            channel = argv[argc-2];
-            message = argv[argc-1];
-        }
-    } else {
-        if (argc != 5)
-            return RedisModule_WrongArity(ctx);
-        else {
-            oldvalstr = argv[2];
-            channel = argv[3];
-            message = argv[4];
-        }
-    }
-
-    if (flag != OBJ_OP_NO) {
-        /*Check if key type is string*/
-        RedisModuleKey *key = RedisModule_OpenKey(ctx,argv[1],
-            REDISMODULE_READ);
-        int type = RedisModule_KeyType(key);
-        RedisModule_CloseKey(key);
-
-        if (type == REDISMODULE_KEYTYPE_EMPTY) {
-            return RedisModule_ReplyWithLongLong(ctx, 0);
-        } else if (type != REDISMODULE_KEYTYPE_STRING) {
-            return RedisModule_ReplyWithError(ctx,REDISMODULE_ERRORMSG_WRONGTYPE);
-        }
-    }
-
-    if (flag == OBJ_OP_IE || flag == OBJ_OP_NE) {
-        /*Get the value*/
-        reply = RedisModule_Call(ctx, "GET", "s", argv[1]);
-        ASSERT_NOERROR(reply)
-        size_t curlen = 0, oldvallen = 0;
-        const char *oldval = RedisModule_StringPtrLen(oldvalstr, &oldvallen);
-        const char *curval = RedisModule_CallReplyStringPtr(reply, &curlen);
-        if (((flag == OBJ_OP_IE) &&
-            (!curval || (oldvallen != curlen) || strncmp(oldval, curval, curlen)))
-            ||
-            ((flag == OBJ_OP_NE) && curval && (oldvallen == curlen) &&
-              !strncmp(oldval, curval, curlen))) {
-            RedisModule_FreeCallReply(reply);
-            return RedisModule_ReplyWithLongLong(ctx, 0);
-        }
-        RedisModule_FreeCallReply(reply);
-    }
-
-
-    /* Prepare the arguments for the command. */
-    int i, j=0, cmdargc=argc-3;
-    RedisModuleString *cmdargv[cmdargc];
-    for (i = 1; i < argc-2; i++) {
-        if ((flag == OBJ_OP_IE || flag == OBJ_OP_NE) && (i == 2))
-            continue;
-        cmdargv[j++] = argv[i];
-    }
-
-    /* Call the command and pass back the reply. */
-    reply = RedisModule_Call(ctx, "UNLINK", "v!", cmdargv, j);
+    RedisModuleCallReply *reply = RedisModule_Call(ctx, "UNLINK", "v!", delParamsPtr->keys, delParamsPtr->length);
     ASSERT_NOERROR(reply)
     int replytype = RedisModule_CallReplyType(reply);
     if (replytype == REDISMODULE_REPLY_NULL) {
         RedisModule_ReplyWithNull(ctx);
-    }
-    else if (RedisModule_CallReplyInteger(reply) == 0) {
+    } else if (RedisModule_CallReplyInteger(reply) == 0) {
         RedisModule_ReplyWithCallReply(ctx, reply);
     } else {
-        cmdargc = 2;
-        cmdargv[0] = channel;
-        cmdargv[1] = message;
-        RedisModuleCallReply *pubreply = RedisModule_Call(ctx, "PUBLISH", "v", cmdargv, cmdargc);
-        RedisModule_FreeCallReply(pubreply);
         RedisModule_ReplyWithCallReply(ctx, reply);
+        multiPubCommand(ctx, pubParamsPtr);
     }
-
     RedisModule_FreeCallReply(reply);
     return REDISMODULE_OK;
 }
 
 int DelPub_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
 {
-   return delPubStringGenericCommand(ctx, argv, argc, OBJ_OP_NO);
+    if (argc < 4)
+        return RedisModule_WrongArity(ctx);
+
+    DelParams delParams = {
+                           .keys = argv + 1,
+                           .length = argc - 3
+                          };
+    PubParams pubParams = {
+                           .channel_msg_pairs = argv + 1 + delParams.length,
+                           .length = 2
+                          };
+
+    return delPubStringCommon(ctx, &delParams, &pubParams);
+}
+
+int delIENEPubStringCommon(RedisModuleCtx *ctx, RedisModuleString **argv, int argc, int flag)
+{
+    if (argc < 5 || (argc % 2) == 0)
+        return RedisModule_WrongArity(ctx);
+
+    DelParams delParams = {
+                           .keys = argv + 1,
+                           .length = 1
+                          };
+    PubParams pubParams = {
+                           .channel_msg_pairs = argv + 3,
+                           .length = argc - 3
+                          };
+    RedisModuleString *key = argv[1];
+    RedisModuleString *oldvalstr = argv[2];
+
+    int type = getKeyType(ctx, key);
+    if (type == REDISMODULE_KEYTYPE_EMPTY) {
+        return RedisModule_ReplyWithLongLong(ctx, 0);
+    } else if (type != REDISMODULE_KEYTYPE_STRING) {
+        return RedisModule_ReplyWithError(ctx, REDISMODULE_ERRORMSG_WRONGTYPE);
+    }
+
+    RedisModuleCallReply *reply = RedisModule_Call(ctx, "GET", "s", key);
+    ASSERT_NOERROR(reply)
+    bool is_equal = replyContentsEqualString(reply, oldvalstr);
+    RedisModule_FreeCallReply(reply);
+    if ((flag == OBJ_OP_IE && !is_equal) ||
+        (flag == OBJ_OP_NE && is_equal)) {
+        return RedisModule_ReplyWithLongLong(ctx, 0);
+    }
+
+    return delPubStringCommon(ctx, &delParams, &pubParams);
 }
 
 int DelIEPub_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
 {
-   return delPubStringGenericCommand(ctx, argv, argc, OBJ_OP_IE);
+   return delIENEPubStringCommon(ctx, argv, argc, OBJ_OP_IE);
 }
 
 int DelNEPub_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
 {
-   return delPubStringGenericCommand(ctx, argv, argc, OBJ_OP_NE);
+   return delIENEPubStringCommon(ctx, argv, argc, OBJ_OP_NE);
 }
 
 /* This function must be present on each Redis module. It is used in order to

diff --git a/redismodule/tst/mock/include/exstringsStub.h b/redismodule/tst/mock/include/exstringsStub.h
index 23842cf..9886d00 100755 (executable)
--- a/redismodule/tst/mock/include/exstringsStub.h
+++ b/redismodule/tst/mock/include/exstringsStub.h
@@ -43,7 +43,6 @@ int DelIEPub_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int arg
 int DelNEPub_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc);
 int NGet_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc);
 int NDel_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc);
-int delPubStringGenericCommand(RedisModuleCtx *ctx, RedisModuleString **argv,          int argc, const int flag);
 
 
 #endif

diff --git a/redismodule/tst/src/exstrings_test.cpp b/redismodule/tst/src/exstrings_test.cpp
index a131781..020da95 100755 (executable)
--- a/redismodule/tst/src/exstrings_test.cpp
+++ b/redismodule/tst/src/exstrings_test.cpp
@@ -1307,15 +1307,15 @@ TEST(exstring, delpub_command_parameter_number_incorrect)
 {
     RedisModuleCtx ctx;
     int ret = 0;
-    ret = delPubStringGenericCommand(&ctx, 0, 2, OBJ_OP_NO);
+    ret = DelPub_RedisCommand(&ctx, 0, 2);
     CHECK_EQUAL(ret, REDISMODULE_ERR);
 
     ret = 0;
-    ret = delPubStringGenericCommand(&ctx, 0, 4, OBJ_OP_IE);
+    ret = DelIEPub_RedisCommand(&ctx, 0, 4);
     CHECK_EQUAL(ret, REDISMODULE_ERR);
 
     ret = 0;
-    ret = delPubStringGenericCommand(&ctx, 0, 8, OBJ_OP_NE);
+    ret = DelNEPub_RedisCommand(&ctx, 0, 8);
     CHECK_EQUAL(ret, REDISMODULE_ERR);
 }
 
@@ -1333,7 +1333,7 @@ TEST(exstring, delpub_command_reply_null)
     mock().setData("RedisModule_CallReplyType_inter", 1);
     mock().setData("RedisModule_Call_Return_Null", 0);
 
-    int ret = delPubStringGenericCommand(&ctx, redisStrVec, 5, OBJ_OP_NO);
+    int ret = DelPub_RedisCommand(&ctx, redisStrVec, 5);
     CHECK_EQUAL(ret, 0);
     CHECK_EQUAL(mock().getData("GET").getIntValue(), 0);
     CHECK_EQUAL(mock().getData("UNLINK").getIntValue(), 1);
@@ -1358,7 +1358,7 @@ TEST(exstring, delpub_command_reply_error)
     mock().setData("RedisModule_CallReplyInteger", 0);
     mock().setData("RedisModule_CallReplyType_err", 1);
 
-    int ret = delPubStringGenericCommand(&ctx, redisStrVec, 5, OBJ_OP_NO);
+    int ret = DelPub_RedisCommand(&ctx, redisStrVec, 5);
     CHECK_EQUAL(ret, REDISMODULE_ERR);
     CHECK_EQUAL(mock().getData("GET").getIntValue(), 0);
     CHECK_EQUAL(mock().getData("UNLINK").getIntValue(), 1);
@@ -1383,7 +1383,7 @@ TEST(exstring, delpub_command_has_no_key)
     mock().setData("RedisModule_CallReplyInteger", 0);
     mock().setData("RedisModule_CallReplyType_inter", 1);
 
-    int ret = delPubStringGenericCommand(&ctx, redisStrVec, 5, OBJ_OP_NO);
+    int ret = DelPub_RedisCommand(&ctx, redisStrVec, 5);
     CHECK_EQUAL(ret, 0);
     CHECK_EQUAL(mock().getData("GET").getIntValue(), 0);
     CHECK_EQUAL(mock().getData("UNLINK").getIntValue(), 1);
@@ -1407,7 +1407,7 @@ TEST(exstring, deliepub_command_has_no_key)
     mock().setData("RedisModule_KeyType_empty", 1);
 
     mock().expectOneCall("RedisModule_CloseKey");
-    int ret = delPubStringGenericCommand(&ctx, redisStrVec, 5, OBJ_OP_IE);
+    int ret = DelIEPub_RedisCommand(&ctx, redisStrVec, 5);
     CHECK_EQUAL(ret, 0);
     CHECK_EQUAL(mock().getData("RedisModule_ReplyWithLongLong").getIntValue(), 0);
     CHECK_EQUAL(mock().getData("GET").getIntValue(), 0);
@@ -1433,7 +1433,7 @@ TEST(exstring, deliepub_command_has_key_set)
     mock().setData("RedisModule_OpenKey_have", 1);
     mock().setData("RedisModule_KeyType_set", 1);
     mock().expectOneCall("RedisModule_CloseKey");
-    int ret = delPubStringGenericCommand(&ctx, redisStrVec, 5, OBJ_OP_IE);
+    int ret = DelIEPub_RedisCommand(&ctx, redisStrVec, 5);
     CHECK_EQUAL(ret, REDISMODULE_OK);
     mock().checkExpectations();
     CHECK_EQUAL(mock().getData("RedisModule_ReplyWithError").getIntValue(), 1);
@@ -1461,7 +1461,7 @@ TEST(exstring, deliepub_command_key_string_nosame)
     mock().setData("RedisModule_KeyType_str", 1);
     mock().setData("RedisModule_String_nosame", 1);
     mock().expectOneCall("RedisModule_CloseKey");
-    int ret = delPubStringGenericCommand(&ctx, redisStrVec, 5, OBJ_OP_IE);
+    int ret = DelIEPub_RedisCommand(&ctx, redisStrVec, 5);
     CHECK_EQUAL(ret, REDISMODULE_OK);
     mock().checkExpectations();
     CHECK_EQUAL(mock().getData("RedisModule_ReplyWithError").getIntValue(), 0);
@@ -1491,7 +1491,7 @@ TEST(exstring, deliepub_command_same_string_replynull)
     mock().setData("RedisModule_String_same", 1);
     mock().setData("RedisModule_CallReplyType_null", 1);
     mock().expectOneCall("RedisModule_CloseKey");
-    int ret = delPubStringGenericCommand(&ctx, redisStrVec, 5, OBJ_OP_IE);
+    int ret = DelIEPub_RedisCommand(&ctx, redisStrVec, 5);
     CHECK_EQUAL(ret, REDISMODULE_OK);
     mock().checkExpectations();
     CHECK_EQUAL(mock().getData("RedisModule_ReplyWithError").getIntValue(), 0);
@@ -1522,7 +1522,7 @@ TEST(exstring, deliepub_command_same_string_reply)
     mock().setData("RedisModule_CallReplyType_str", 1);
     mock().setData("RedisModule_CallReplyInteger", 1);
     mock().expectOneCall("RedisModule_CloseKey");
-    int ret = delPubStringGenericCommand(&ctx, redisStrVec, 5, OBJ_OP_IE);
+    int ret = DelIEPub_RedisCommand(&ctx, redisStrVec, 5);
     CHECK_EQUAL(ret, REDISMODULE_OK);
     mock().checkExpectations();
     CHECK_EQUAL(mock().getData("RedisModule_ReplyWithError").getIntValue(), 0);
@@ -1552,7 +1552,7 @@ TEST(exstring, delnepub_command_same_string_reply)
     mock().setData("RedisModule_String_same", 1);
     mock().setData("RedisModule_CallReplyType_str", 1);
     mock().expectOneCall("RedisModule_CloseKey");
-    int ret = delPubStringGenericCommand(&ctx, redisStrVec, 5, OBJ_OP_NE);
+    int ret = DelNEPub_RedisCommand(&ctx, redisStrVec, 5);
     CHECK_EQUAL(ret, REDISMODULE_OK);
     mock().checkExpectations();
     CHECK_EQUAL(mock().getData("RedisModule_ReplyWithError").getIntValue(), 0);
@@ -1583,7 +1583,7 @@ TEST(exstring, delnepub_command_nosame_string_reply)
     mock().setData("RedisModule_CallReplyType_str", 1);
     mock().setData("RedisModule_CallReplyInteger", 1);
     mock().expectOneCall("RedisModule_CloseKey");
-    int ret = delPubStringGenericCommand(&ctx, redisStrVec, 5, OBJ_OP_NE);
+    int ret = DelNEPub_RedisCommand(&ctx, redisStrVec, 5);
     CHECK_EQUAL(ret, REDISMODULE_OK);
     mock().checkExpectations();
     CHECK_EQUAL(mock().getData("RedisModule_ReplyWithError").getIntValue(), 0);