CommandFilter API: Add unregister option.

A filter handle is returned and can be used to unregister a filter.  In
the future it can also be used to further configure or manipulate the
filter.

Filters are now automatically unregistered when a module unloads.

4 files changed, 126 insertions, 32 deletions

diff --git a/src/module.c b/src/module.c
index ee2840225..ad7bba2eb 100644
--- a/src/module.c
+++ b/src/module.c
@@ -49,6 +49,7 @@ struct RedisModule {
     list *types;    /* Module data types. */
     list *usedby;   /* List of modules using APIs from this one. */
     list *using;    /* List of modules we use some APIs of. */
+    list *filters;  /* List of filters the module has registered. */
 };
 typedef struct RedisModule RedisModule;
 
@@ -748,6 +749,7 @@ void RM_SetModuleAttribs(RedisModuleCtx *ctx, const char *name, int ver, int api
     module->types = listCreate();
     module->usedby = listCreate();
     module->using = listCreate();
+    module->filters = listCreate();
     ctx->module = module;
 }
 
@@ -4793,6 +4795,28 @@ int moduleUnregisterUsedAPI(RedisModule *module) {
     return count;
 }
 
+/* Unregister all filters registered by a module.
+ * This is called when a module is being unloaded.
+ * 
+ * Returns the number of filters unregistered. */
+int moduleUnregisterFilters(RedisModule *module) {
+    listIter li;
+    listNode *ln;
+    int count = 0;
+
+    listRewind(module->filters,&li);
+    while((ln = listNext(&li))) {
+        RedisModuleCommandFilter *filter = ln->value;
+        listNode *ln = listSearchKey(moduleCommandFilters,filter);
+        if (ln) {
+            listDelNode(moduleCommandFilters,ln);
+            count++;
+        }
+        zfree(filter);
+    }
+    return count;
+}
+
 /* --------------------------------------------------------------------------
  * Module Command Filter API
  * -------------------------------------------------------------------------- */
@@ -4840,12 +4864,33 @@ int moduleUnregisterUsedAPI(RedisModule *module) {
  * are executed in the order of registration.
  */
 
-int RM_RegisterCommandFilter(RedisModuleCtx *ctx, RedisModuleCommandFilterFunc callback) {
+RedisModuleCommandFilter *RM_RegisterCommandFilter(RedisModuleCtx *ctx, RedisModuleCommandFilterFunc callback) {
     RedisModuleCommandFilter *filter = zmalloc(sizeof(*filter));
     filter->module = ctx->module;
     filter->callback = callback;
 
     listAddNodeTail(moduleCommandFilters, filter);
+    listAddNodeTail(ctx->module->filters, filter);
+    return filter;
+}
+
+/* Unregister a command filter.
+ */
+int RM_UnregisterCommandFilter(RedisModuleCtx *ctx, RedisModuleCommandFilter *filter) {
+    listNode *ln;
+
+    /* A module can only remove its own filters */
+    if (filter->module != ctx->module) return REDISMODULE_ERR;
+
+    ln = listSearchKey(moduleCommandFilters,filter);
+    if (!ln) return REDISMODULE_ERR;
+    listDelNode(moduleCommandFilters,ln);
+    
+    ln = listSearchKey(ctx->module->filters,filter);
+    if (ln) {
+        listDelNode(moduleCommandFilters,ln);
+    }
+
     return REDISMODULE_OK;
 }
 
@@ -4874,18 +4919,18 @@ void moduleCallCommandFilters(client *c) {
 /* Return the number of arguments a filtered command has.  The number of
  * arguments include the command itself.
  */
-int RM_CommandFilterArgsCount(RedisModuleCommandFilterCtx *filter)
+int RM_CommandFilterArgsCount(RedisModuleCommandFilterCtx *fctx)
 {
-    return filter->argc;
+    return fctx->argc;
 }
 
 /* Return the specified command argument.  The first argument (position 0) is
  * the command itself, and the rest are user-provided args.
  */
-const RedisModuleString *RM_CommandFilterArgGet(RedisModuleCommandFilterCtx *filter, int pos)
+const RedisModuleString *RM_CommandFilterArgGet(RedisModuleCommandFilterCtx *fctx, int pos)
 {
-    if (pos < 0 || pos >= filter->argc) return NULL;
-    return filter->argv[pos];
+    if (pos < 0 || pos >= fctx->argc) return NULL;
+    return fctx->argv[pos];
 }
 
 /* Modify the filtered command by inserting a new argument at the specified
@@ -4894,18 +4939,18 @@ const RedisModuleString *RM_CommandFilterArgGet(RedisModuleCommandFilterCtx *fil
  * allocated, freed or used elsewhere.
  */
 
-int RM_CommandFilterArgInsert(RedisModuleCommandFilterCtx *filter, int pos, RedisModuleString *arg)
+int RM_CommandFilterArgInsert(RedisModuleCommandFilterCtx *fctx, int pos, RedisModuleString *arg)
 {
     int i;
 
-    if (pos < 0 || pos > filter->argc) return REDISMODULE_ERR;
+    if (pos < 0 || pos > fctx->argc) return REDISMODULE_ERR;
 
-    filter->argv = zrealloc(filter->argv, (filter->argc+1)*sizeof(RedisModuleString *));
-    for (i = filter->argc; i > pos; i--) {
-        filter->argv[i] = filter->argv[i-1];
+    fctx->argv = zrealloc(fctx->argv, (fctx->argc+1)*sizeof(RedisModuleString *));
+    for (i = fctx->argc; i > pos; i--) {
+        fctx->argv[i] = fctx->argv[i-1];
     }
-    filter->argv[pos] = arg;
-    filter->argc++;
+    fctx->argv[pos] = arg;
+    fctx->argc++;
 
     return REDISMODULE_OK;
 }
@@ -4916,12 +4961,12 @@ int RM_CommandFilterArgInsert(RedisModuleCommandFilterCtx *filter, int pos, Redi
  * or used elsewhere.
  */
 
-int RM_CommandFilterArgReplace(RedisModuleCommandFilterCtx *filter, int pos, RedisModuleString *arg)
+int RM_CommandFilterArgReplace(RedisModuleCommandFilterCtx *fctx, int pos, RedisModuleString *arg)
 {
-    if (pos < 0 || pos >= filter->argc) return REDISMODULE_ERR;
+    if (pos < 0 || pos >= fctx->argc) return REDISMODULE_ERR;
 
-    decrRefCount(filter->argv[pos]);
-    filter->argv[pos] = arg;
+    decrRefCount(fctx->argv[pos]);
+    fctx->argv[pos] = arg;
 
     return REDISMODULE_OK;
 }
@@ -4929,16 +4974,16 @@ int RM_CommandFilterArgReplace(RedisModuleCommandFilterCtx *filter, int pos, Red
 /* Modify the filtered command by deleting an argument at the specified
  * position.
  */
-int RM_CommandFilterArgDelete(RedisModuleCommandFilterCtx *filter, int pos)
+int RM_CommandFilterArgDelete(RedisModuleCommandFilterCtx *fctx, int pos)
 {
     int i;
-    if (pos < 0 || pos >= filter->argc) return REDISMODULE_ERR;
+    if (pos < 0 || pos >= fctx->argc) return REDISMODULE_ERR;
 
-    decrRefCount(filter->argv[pos]);
-    for (i = pos; i < filter->argc-1; i++) {
-        filter->argv[i] = filter->argv[i+1];
+    decrRefCount(fctx->argv[pos]);
+    for (i = pos; i < fctx->argc-1; i++) {
+        fctx->argv[i] = fctx->argv[i+1];
     }
-    filter->argc--;
+    fctx->argc--;
 
     return REDISMODULE_OK;
 }
@@ -5041,6 +5086,7 @@ void moduleLoadFromQueue(void) {
 
 void moduleFreeModuleStructure(struct RedisModule *module) {
     listRelease(module->types);
+    listRelease(module->filters);
     sdsfree(module->name);
     zfree(module);
 }
@@ -5132,6 +5178,7 @@ int moduleUnload(sds name) {
     moduleUnregisterCommands(module);
     moduleUnregisterSharedAPI(module);
     moduleUnregisterUsedAPI(module);
+    moduleUnregisterFilters(module);
 
     /* Remove any notification subscribers this module might have */
     moduleUnsubscribeNotifications(module);
@@ -5396,6 +5443,7 @@ void moduleRegisterCoreAPI(void) {
     REGISTER_API(ExportSharedAPI);
     REGISTER_API(GetSharedAPI);
     REGISTER_API(RegisterCommandFilter);
+    REGISTER_API(UnregisterCommandFilter);
     REGISTER_API(CommandFilterArgsCount);
     REGISTER_API(CommandFilterArgGet);
     REGISTER_API(CommandFilterArgInsert);
diff --git a/src/modules/hellofilter.c b/src/modules/hellofilter.c
index d5dd405aa..9cd440df2 100644
--- a/src/modules/hellofilter.c
+++ b/src/modules/hellofilter.c
@@ -7,10 +7,27 @@ static RedisModuleString *log_key_name;
 
 static const char log_command_name[] = "hellofilter.log";
 static const char ping_command_name[] = "hellofilter.ping";
+static const char unregister_command_name[] = "hellofilter.unregister";
 static int in_module = 0;
 
+static RedisModuleCommandFilter *filter = NULL;
+
+int HelloFilter_UnregisterCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
+{
+    (void) argc;
+    (void) argv;
+
+    RedisModule_ReplyWithLongLong(ctx,
+            RedisModule_UnregisterCommandFilter(ctx, filter));
+
+    return REDISMODULE_OK;
+}
+
 int HelloFilter_PingCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
 {
+    (void) argc;
+    (void) argv;
+
     RedisModuleCallReply *reply = RedisModule_Call(ctx, "ping", "c", "@log");
     if (reply) {
         RedisModule_ReplyWithCallReply(ctx, reply);
@@ -115,11 +132,15 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
             return REDISMODULE_ERR;
 
     if (RedisModule_CreateCommand(ctx,ping_command_name,
-                HelloFilter_PingCommand,"write deny-oom",1,1,1) == REDISMODULE_ERR)
+                HelloFilter_PingCommand,"deny-oom",1,1,1) == REDISMODULE_ERR)
             return REDISMODULE_ERR;
 
-    if (RedisModule_RegisterCommandFilter(ctx, HelloFilter_CommandFilter)
-            == REDISMODULE_ERR) return REDISMODULE_ERR;
+    if (RedisModule_CreateCommand(ctx,unregister_command_name,
+                HelloFilter_UnregisterCommand,"write deny-oom",1,1,1) == REDISMODULE_ERR)
+            return REDISMODULE_ERR;
+
+    if ((filter = RedisModule_RegisterCommandFilter(ctx, HelloFilter_CommandFilter))
+            == NULL) return REDISMODULE_ERR;
 
     return REDISMODULE_OK;
 }
diff --git a/src/redismodule.h b/src/redismodule.h
index 5df83ae6a..37b7d0d59 100644
--- a/src/redismodule.h
+++ b/src/redismodule.h
@@ -151,6 +151,7 @@ typedef struct RedisModuleClusterInfo RedisModuleClusterInfo;
 typedef struct RedisModuleDict RedisModuleDict;
 typedef struct RedisModuleDictIter RedisModuleDictIter;
 typedef struct RedisModuleCommandFilterCtx RedisModuleCommandFilterCtx;
+typedef struct RedisModuleCommandFilter RedisModuleCommandFilter;
 
 typedef int (*RedisModuleCmdFunc)(RedisModuleCtx *ctx, RedisModuleString **argv, int argc);
 typedef void (*RedisModuleDisconnectFunc)(RedisModuleCtx *ctx, RedisModuleBlockedClient *bc);
@@ -339,12 +340,13 @@ void REDISMODULE_API_FUNC(RedisModule_SetDisconnectCallback)(RedisModuleBlockedC
 void REDISMODULE_API_FUNC(RedisModule_SetClusterFlags)(RedisModuleCtx *ctx, uint64_t flags);
 int REDISMODULE_API_FUNC(RedisModule_ExportSharedAPI)(RedisModuleCtx *ctx, const char *apiname, void *func);
 void *REDISMODULE_API_FUNC(RedisModule_GetSharedAPI)(RedisModuleCtx *ctx, const char *apiname);
-int REDISMODULE_API_FUNC(RedisModule_RegisterCommandFilter)(RedisModuleCtx *ctx, RedisModuleCommandFilterFunc cb);
-int REDISMODULE_API_FUNC(RedisModule_CommandFilterArgsCount)(RedisModuleCommandFilterCtx *filter);
-const RedisModuleString *REDISMODULE_API_FUNC(RedisModule_CommandFilterArgGet)(RedisModuleCommandFilterCtx *filter, int pos);
-int REDISMODULE_API_FUNC(RedisModule_CommandFilterArgInsert)(RedisModuleCommandFilterCtx *filter, int pos, RedisModuleString *arg);
-int REDISMODULE_API_FUNC(RedisModule_CommandFilterArgReplace)(RedisModuleCommandFilterCtx *filter, int pos, RedisModuleString *arg);
-int REDISMODULE_API_FUNC(RedisModule_CommandFilterArgDelete)(RedisModuleCommandFilterCtx *filter, int pos);
+RedisModuleCommandFilter *REDISMODULE_API_FUNC(RedisModule_RegisterCommandFilter)(RedisModuleCtx *ctx, RedisModuleCommandFilterFunc cb);
+int REDISMODULE_API_FUNC(RedisModule_UnregisterCommandFilter)(RedisModuleCtx *ctx, RedisModuleCommandFilter *filter);
+int REDISMODULE_API_FUNC(RedisModule_CommandFilterArgsCount)(RedisModuleCommandFilterCtx *fctx);
+const RedisModuleString *REDISMODULE_API_FUNC(RedisModule_CommandFilterArgGet)(RedisModuleCommandFilterCtx *fctx, int pos);
+int REDISMODULE_API_FUNC(RedisModule_CommandFilterArgInsert)(RedisModuleCommandFilterCtx *fctx, int pos, RedisModuleString *arg);
+int REDISMODULE_API_FUNC(RedisModule_CommandFilterArgReplace)(RedisModuleCommandFilterCtx *fctx, int pos, RedisModuleString *arg);
+int REDISMODULE_API_FUNC(RedisModule_CommandFilterArgDelete)(RedisModuleCommandFilterCtx *fctx, int pos);
 #endif
 
 /* This is included inline inside each Redis module. */
@@ -508,6 +510,7 @@ static int RedisModule_Init(RedisModuleCtx *ctx, const char *name, int ver, int
     REDISMODULE_GET_API(ExportSharedAPI);
     REDISMODULE_GET_API(GetSharedAPI);
     REDISMODULE_GET_API(RegisterCommandFilter);
+    REDISMODULE_GET_API(UnregisterCommandFilter);
     REDISMODULE_GET_API(CommandFilterArgsCount);
     REDISMODULE_GET_API(CommandFilterArgGet);
     REDISMODULE_GET_API(CommandFilterArgInsert);
diff --git a/tests/modules/commandfilter.tcl b/tests/modules/commandfilter.tcl
index 47d9c302c..8645d8279 100644
--- a/tests/modules/commandfilter.tcl
+++ b/tests/modules/commandfilter.tcl
@@ -42,4 +42,26 @@ start_server {tags {"modules"}} {
         r eval "redis.call('hellofilter.ping')" 0
         r lrange log-key 0 -1
     } "{ping @log}"
+
+    test {Command Filter is unregistered implicitly on module unload} {
+        r del log-key
+        r module unload hellofilter
+        r set mykey @log
+        r lrange log-key 0 -1
+    } {}
+
+    r module load $testmodule log-key-2
+
+    test {Command Filter unregister works as expected} {
+        # Validate reloading succeeded
+        r set mykey @log
+        assert_equal "{set mykey @log}" [r lrange log-key-2 0 -1]
+
+        # Unregister
+        r hellofilter.unregister
+        r del log-key-2
+
+        r set mykey @log
+        r lrange log-key-2 0 -1
+    } {}
 }