Skip to content

Commit

Permalink
modules: Add newlen == 0 handling to RM_StringTruncate (redis#3717) (r…
Browse files Browse the repository at this point in the history 
…edis#3718)

Previously, passing 0 for newlen would not truncate the string at all.
This adds handling of this case, freeing the old string and creating a new empty string.

Other changes:
- Move `src/modules/testmodule.c` to `tests/modules/basics.c`
- Introduce that basic test into the test suite
- Add tests to cover StringTruncate
- Add `test-modules` build target for the main makefile
- Extend `distclean` build target to clean modules too
  • Loading branch information
@neomantra @x77a1
neomantra authored and x77a1 committed Jul 11, 2021
1 parent d1cd475 commit f089bae
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 16 deletions.
1 change: 1 addition & 0 deletions runtest-moduleapi
View file
Expand Up @@ -16,6 +16,7 @@ fi
$MAKE -C tests/modules && \
$TCLSH tests/test_helper.tcl \
--single unit/moduleapi/commandfilter \
--single unit/moduleapi/basics \
--single unit/moduleapi/fork \
--single unit/moduleapi/testrdb \
--single unit/moduleapi/infotest \
Expand Down
5 changes: 5 additions & 0 deletions src/Makefile
View file
Expand Up @@ -375,13 +375,18 @@ clean:

distclean: clean
-(cd ../deps && $(MAKE) distclean)
-(cd modules && $(MAKE) clean)
-(cd ../tests/modules && $(MAKE) clean)
-(rm -f .make-*)

.PHONY: distclean

test: $(REDIS_SERVER_NAME) $(REDIS_CHECK_AOF_NAME) $(REDIS_CLI_NAME) $(REDIS_BENCHMARK_NAME)
@(cd ..; ./runtest)

test-modules: $(REDIS_SERVER_NAME)
@(cd ..; ./runtest-moduleapi)

test-sentinel: $(REDIS_SENTINEL_NAME) $(REDIS_CLI_NAME)
@(cd ..; ./runtest-sentinel)

Expand Down
21 changes: 13 additions & 8 deletions src/module.c
View file
Expand Up @@ -2559,14 +2559,19 @@ int RM_StringTruncate(RedisModuleKey *key, size_t newlen) {
} else {
/* Unshare and resize. */
key->value = dbUnshareStringValue(key->db, key->key, key->value);
size_t curlen = sdslen(key->value->ptr);
if (newlen > curlen) {
key->value->ptr = sdsgrowzero(key->value->ptr,newlen);
} else if (newlen < curlen) {
sdsrange(key->value->ptr,0,newlen-1);
/* If the string is too wasteful, reallocate it. */
if (sdslen(key->value->ptr) < sdsavail(key->value->ptr))
key->value->ptr = sdsRemoveFreeSpace(key->value->ptr);
if (newlen == 0) {
sdsfree(key->value->ptr);
key->value->ptr = sdsempty();
} else {
size_t curlen = sdslen(key->value->ptr);
if (newlen > curlen) {
key->value->ptr = sdsgrowzero(key->value->ptr,newlen);
} else if (newlen < curlen) {
sdsrange(key->value->ptr,0,newlen-1);
/* If the string is too wasteful, reallocate it. */
if (sdslen(key->value->ptr) < sdsavail(key->value->ptr))
key->value->ptr = sdsRemoveFreeSpace(key->value->ptr);
}
}
}
return REDISMODULE_OK;
Expand Down
1 change: 1 addition & 0 deletions tests/modules/Makefile
View file
Expand Up @@ -18,6 +18,7 @@ endif

TEST_MODULES = \
commandfilter.so \
basics.so \
testrdb.so \
fork.so \
infotest.so \
Expand Down
85 changes: 77 additions & 8 deletions src/modules/testmodule.c → tests/modules/basics.c
View file
Expand Up @@ -31,7 +31,7 @@
*/

#define REDISMODULE_EXPERIMENTAL_API
#include "../redismodule.h"
#include "redismodule.h"
#include <string.h>

/* --------------------------------- Helpers -------------------------------- */
Expand Down Expand Up @@ -152,7 +152,58 @@ int TestUnlink(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
return failTest(ctx, "Could not verify key to be unlinked");
}
return RedisModule_ReplyWithSimpleString(ctx, "OK");
}

/* TEST.STRING.TRUNCATE -- Test truncating an existing string object. */
int TestStringTruncate(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
REDISMODULE_NOT_USED(argv);
REDISMODULE_NOT_USED(argc);

RedisModule_Call(ctx, "SET", "cc", "foo", "abcde");
RedisModuleKey *k = RedisModule_OpenKey(ctx, RedisModule_CreateStringPrintf(ctx, "foo"), REDISMODULE_READ | REDISMODULE_WRITE);
if (!k) return failTest(ctx, "Could not create key");

size_t len = 0;
char* s;

/* expand from 5 to 8 and check null pad */
if (REDISMODULE_ERR == RedisModule_StringTruncate(k, 8)) {
return failTest(ctx, "Could not truncate string value (8)");
}
s = RedisModule_StringDMA(k, &len, REDISMODULE_READ);
if (!s) {
return failTest(ctx, "Failed to read truncated string (8)");
} else if (len != 8) {
return failTest(ctx, "Failed to expand string value (8)");
} else if (0 != strncmp(s, "abcde\0\0\0", 8)) {
return failTest(ctx, "Failed to null pad string value (8)");
}

/* shrink from 8 to 4 */
if (REDISMODULE_ERR == RedisModule_StringTruncate(k, 4)) {
return failTest(ctx, "Could not truncate string value (4)");
}
s = RedisModule_StringDMA(k, &len, REDISMODULE_READ);
if (!s) {
return failTest(ctx, "Failed to read truncated string (4)");
} else if (len != 4) {
return failTest(ctx, "Failed to shrink string value (4)");
} else if (0 != strncmp(s, "abcd", 4)) {
return failTest(ctx, "Failed to truncate string value (4)");
}

/* shrink to 0 */
if (REDISMODULE_ERR == RedisModule_StringTruncate(k, 0)) {
return failTest(ctx, "Could not truncate string value (0)");
}
s = RedisModule_StringDMA(k, &len, REDISMODULE_READ);
if (!s) {
return failTest(ctx, "Failed to read truncated string (0)");
} else if (len != 0) {
return failTest(ctx, "Failed to shrink string value to (0)");
}

return RedisModule_ReplyWithSimpleString(ctx, "OK");
}

int NotifyCallback(RedisModuleCtx *ctx, int type, const char *event,
Expand Down Expand Up @@ -324,7 +375,11 @@ int TestCtxFlags(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
int TestAssertStringReply(RedisModuleCtx *ctx, RedisModuleCallReply *reply, char *str, size_t len) {
RedisModuleString *mystr, *expected;

if (RedisModule_CallReplyType(reply) != REDISMODULE_REPLY_STRING) {
if (RedisModule_CallReplyType(reply) == REDISMODULE_REPLY_ERROR) {
RedisModule_Log(ctx,"warning","Test error reply: %s",
RedisModule_CallReplyStringPtr(reply, NULL));
return 0;
} else if (RedisModule_CallReplyType(reply) != REDISMODULE_REPLY_STRING) {
RedisModule_Log(ctx,"warning","Unexpected reply type %d",
RedisModule_CallReplyType(reply));
return 0;
Expand All @@ -345,7 +400,11 @@ int TestAssertStringReply(RedisModuleCtx *ctx, RedisModuleCallReply *reply, char
/* Return 1 if the reply matches the specified integer, otherwise log errors
* in the server log and return 0. */
int TestAssertIntegerReply(RedisModuleCtx *ctx, RedisModuleCallReply *reply, long long expected) {
if (RedisModule_CallReplyType(reply) != REDISMODULE_REPLY_INTEGER) {
if (RedisModule_CallReplyType(reply) == REDISMODULE_REPLY_ERROR) {
RedisModule_Log(ctx,"warning","Test error reply: %s",
RedisModule_CallReplyStringPtr(reply, NULL));
return 0;
} else if (RedisModule_CallReplyType(reply) != REDISMODULE_REPLY_INTEGER) {
RedisModule_Log(ctx,"warning","Unexpected reply type %d",
RedisModule_CallReplyType(reply));
return 0;
Expand All @@ -366,8 +425,11 @@ int TestAssertIntegerReply(RedisModuleCtx *ctx, RedisModuleCallReply *reply, lon
reply = RedisModule_Call(ctx,name,__VA_ARGS__); \
} while (0)

/* TEST.IT -- Run all the tests. */
int TestIt(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
/* TEST.BASICS -- Run all the tests.
* Note: it is useful to run these tests from the module rather than TCL
* since it's easier to check the reply types like that (make a distinction
* between 0 and "0", etc. */
int TestBasics(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
REDISMODULE_NOT_USED(argv);
REDISMODULE_NOT_USED(argc);

Expand All @@ -390,6 +452,9 @@ int TestIt(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
T("test.string.append","");
if (!TestAssertStringReply(ctx,reply,"foobar",6)) goto fail;

T("test.string.truncate","");
if (!TestAssertStringReply(ctx,reply,"OK",2)) goto fail;

T("test.unlink","");
if (!TestAssertStringReply(ctx,reply,"OK",2)) goto fail;

Expand All @@ -407,7 +472,7 @@ int TestIt(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {

fail:
RedisModule_ReplyWithSimpleString(ctx,
"SOME TEST NOT PASSED! Check server logs");
"SOME TEST DID NOT PASS! Check server logs");
return REDISMODULE_OK;
}

Expand All @@ -430,6 +495,10 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
TestStringAppendAM,"write deny-oom",1,1,1) == REDISMODULE_ERR)
return REDISMODULE_ERR;

if (RedisModule_CreateCommand(ctx,"test.string.truncate",
TestStringTruncate,"write deny-oom",1,1,1) == REDISMODULE_ERR)
return REDISMODULE_ERR;

if (RedisModule_CreateCommand(ctx,"test.string.printf",
TestStringPrintf,"write deny-oom",1,1,1) == REDISMODULE_ERR)
return REDISMODULE_ERR;
Expand All @@ -442,8 +511,8 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
TestUnlink,"write deny-oom",1,1,1) == REDISMODULE_ERR)
return REDISMODULE_ERR;

if (RedisModule_CreateCommand(ctx,"test.it",
TestIt,"readonly",1,1,1) == REDISMODULE_ERR)
if (RedisModule_CreateCommand(ctx,"test.basics",
TestBasics,"readonly",1,1,1) == REDISMODULE_ERR)
return REDISMODULE_ERR;

RedisModule_SubscribeToKeyspaceEvents(ctx,
Expand Down
13 changes: 13 additions & 0 deletions tests/unit/moduleapi/basics.tcl
View file
@@ -0,0 +1,13 @@
set testmodule [file normalize tests/modules/basics.so]


# TEST.CTXFLAGS requires RDB to be disabled, so override save file
start_server {tags {"modules"} overrides {save ""}} {
r module load $testmodule

test {test module api basics} {
r test.basics
} {ALL TESTS PASSED}

r module unload test
}

0 comments on commit f089bae

Please sign in to comment.