modules: Add newlen == 0 handling to RM_StringTruncate (#3717) (#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
This commit is contained in:
Evan 2021-06-22 05:26:48 -04:00 committed by GitHub
parent d0819d618e
commit 1ccf2ca2f4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 110 additions and 16 deletions

View File

@ -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 \

View File

@ -375,6 +375,8 @@ clean:
distclean: clean
-(cd ../deps && $(MAKE) distclean)
-(cd modules && $(MAKE) clean)
-(cd ../tests/modules && $(MAKE) clean)
-(rm -f .make-*)
.PHONY: distclean
@ -382,6 +384,9 @@ distclean: clean
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)

View File

@ -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;

View File

@ -18,6 +18,7 @@ endif
TEST_MODULES = \
commandfilter.so \
basics.so \
testrdb.so \
fork.so \
infotest.so \

View File

@ -31,7 +31,7 @@
*/
#define REDISMODULE_EXPERIMENTAL_API
#include "../redismodule.h"
#include "redismodule.h"
#include <string.h>
/* --------------------------------- Helpers -------------------------------- */
@ -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,
@ -324,7 +375,11 @@ end:
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;
@ -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;
@ -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);
@ -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;
@ -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;
}
@ -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;
@ -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,

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
}