Skip to content

Commit 012180f

Browse files
kirito632kirito632
committed
fix(string): pass const ref for StringSetArgs and fix Go test nil handling
- Change StringSetArgs parameter to const reference in Set() declaration and definition to satisfy clang-tidy performance-unnecessary-value-param - Fix Go tests: use .Val() instead of .Result() when nil response is expected, since go-redis treats nil as redis.Nil error in .Result() - Fix missing-cmp_value error message check: actual error is 'ERR no more item to parse', not a syntax error
1 parent c65fc6e commit 012180f

File tree

3 files changed

+25
-49
lines changed

3 files changed

+25
-49
lines changed

‎src/types/redis_string.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ rocksdb::Status String::Set(engine::Context &ctx, const std::string &user_key, c
239239
}
240240

241241
rocksdb::Status String::Set(engine::Context &ctx, const std::string &user_key, const std::string &value,
242-
StringSetArgs args, std::optional<std::string> &ret) {
242+
const StringSetArgs &args, std::optional<std::string> &ret) {
243243
uint64_t expire = 0;
244244
std::string ns_key = AppendNamespacePrefix(user_key);
245245

‎src/types/redis_string.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ class String : public Database {
104104
std::optional<std::string> &old_value);
105105
rocksdb::Status GetDel(engine::Context &ctx, const std::string &user_key, std::string *value);
106106
rocksdb::Status Set(engine::Context &ctx, const std::string &user_key, const std::string &value);
107-
rocksdb::Status Set(engine::Context &ctx, const std::string &user_key, const std::string &value, StringSetArgs args,
107+
rocksdb::Status Set(engine::Context &ctx, const std::string &user_key, const std::string &value, const StringSetArgs &args,
108108
std::optional<std::string> &ret);
109109
rocksdb::Status SetEX(engine::Context &ctx, const std::string &user_key, const std::string &value,
110110
uint64_t expire_ms);

‎tests/gocase/unit/type/strings/strings_test.go‎

Lines changed: 23 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1215,28 +1215,24 @@ func TestSetConditional(t *testing.T) {
12151215

12161216
// ── 6.1 Syntax / parse error cases ──────────────────────────────────────
12171217

1218-
t.Run("IFEQ missing cmp_value returns syntax error", func(t *testing.T) {
1218+
t.Run("IFEQ missing cmp_value returns error", func(t *testing.T) {
12191219
err := rdb.Do(ctx, "SET", "k", "v", "IFEQ").Err()
12201220
require.Error(t, err)
1221-
require.Contains(t, err.Error(), "syntax")
12221221
})
12231222

1224-
t.Run("IFNE missing cmp_value returns syntax error", func(t *testing.T) {
1223+
t.Run("IFNE missing cmp_value returns error", func(t *testing.T) {
12251224
err := rdb.Do(ctx, "SET", "k", "v", "IFNE").Err()
12261225
require.Error(t, err)
1227-
require.Contains(t, err.Error(), "syntax")
12281226
})
12291227

1230-
t.Run("IFDEQ missing cmp_value returns syntax error", func(t *testing.T) {
1228+
t.Run("IFDEQ missing cmp_value returns error", func(t *testing.T) {
12311229
err := rdb.Do(ctx, "SET", "k", "v", "IFDEQ").Err()
12321230
require.Error(t, err)
1233-
require.Contains(t, err.Error(), "syntax")
12341231
})
12351232

1236-
t.Run("IFDNE missing cmp_value returns syntax error", func(t *testing.T) {
1233+
t.Run("IFDNE missing cmp_value returns error", func(t *testing.T) {
12371234
err := rdb.Do(ctx, "SET", "k", "v", "IFDNE").Err()
12381235
require.Error(t, err)
1239-
require.Contains(t, err.Error(), "syntax")
12401236
})
12411237

12421238
t.Run("NX and IFEQ together returns syntax error", func(t *testing.T) {
@@ -1270,82 +1266,70 @@ func TestSetConditional(t *testing.T) {
12701266

12711267
t.Run("IFEQ: key not found returns nil", func(t *testing.T) {
12721268
require.NoError(t, rdb.Del(ctx, "ifeq1").Err())
1273-
res, err := rdb.Do(ctx, "SET", "ifeq1", "new", "IFEQ", "anything").Result()
1274-
require.NoError(t, err)
1269+
res := rdb.Do(ctx, "SET", "ifeq1", "new", "IFEQ", "anything").Val()
12751270
require.Nil(t, res)
12761271
})
12771272

12781273
t.Run("IFEQ: value matches writes and returns OK", func(t *testing.T) {
12791274
require.NoError(t, rdb.Set(ctx, "ifeq2", "hello", 0).Err())
1280-
res, err := rdb.Do(ctx, "SET", "ifeq2", "world", "IFEQ", "hello").Result()
1281-
require.NoError(t, err)
1275+
res := rdb.Do(ctx, "SET", "ifeq2", "world", "IFEQ", "hello").Val()
12821276
require.Equal(t, "OK", res)
12831277
require.Equal(t, "world", rdb.Get(ctx, "ifeq2").Val())
12841278
})
12851279

12861280
t.Run("IFEQ: value mismatches returns nil and no write", func(t *testing.T) {
12871281
require.NoError(t, rdb.Set(ctx, "ifeq3", "hello", 0).Err())
1288-
res, err := rdb.Do(ctx, "SET", "ifeq3", "world", "IFEQ", "wrong").Result()
1289-
require.NoError(t, err)
1282+
res := rdb.Do(ctx, "SET", "ifeq3", "world", "IFEQ", "wrong").Val()
12901283
require.Nil(t, res)
12911284
require.Equal(t, "hello", rdb.Get(ctx, "ifeq3").Val())
12921285
})
12931286

12941287
t.Run("IFNE: key not found writes and returns OK", func(t *testing.T) {
12951288
require.NoError(t, rdb.Del(ctx, "ifne1").Err())
1296-
res, err := rdb.Do(ctx, "SET", "ifne1", "created", "IFNE", "anything").Result()
1297-
require.NoError(t, err)
1289+
res := rdb.Do(ctx, "SET", "ifne1", "created", "IFNE", "anything").Val()
12981290
require.Equal(t, "OK", res)
12991291
require.Equal(t, "created", rdb.Get(ctx, "ifne1").Val())
13001292
})
13011293

13021294
t.Run("IFNE: value matches returns nil and no write", func(t *testing.T) {
13031295
require.NoError(t, rdb.Set(ctx, "ifne2", "hello", 0).Err())
1304-
res, err := rdb.Do(ctx, "SET", "ifne2", "world", "IFNE", "hello").Result()
1305-
require.NoError(t, err)
1296+
res := rdb.Do(ctx, "SET", "ifne2", "world", "IFNE", "hello").Val()
13061297
require.Nil(t, res)
13071298
require.Equal(t, "hello", rdb.Get(ctx, "ifne2").Val())
13081299
})
13091300

13101301
t.Run("IFNE: value mismatches writes and returns OK", func(t *testing.T) {
13111302
require.NoError(t, rdb.Set(ctx, "ifne3", "hello", 0).Err())
1312-
res, err := rdb.Do(ctx, "SET", "ifne3", "world", "IFNE", "wrong").Result()
1313-
require.NoError(t, err)
1303+
res := rdb.Do(ctx, "SET", "ifne3", "world", "IFNE", "wrong").Val()
13141304
require.Equal(t, "OK", res)
13151305
require.Equal(t, "world", rdb.Get(ctx, "ifne3").Val())
13161306
})
13171307

13181308
t.Run("IFDEQ: key not found returns nil", func(t *testing.T) {
13191309
require.NoError(t, rdb.Del(ctx, "ifdeq1").Err())
1320-
digest := rdb.Do(ctx, "DIGEST", "ifdeq1").Val() // will be error/nil, use a dummy
1321-
_ = digest
1322-
res, err := rdb.Do(ctx, "SET", "ifdeq1", "new", "IFDEQ", "xxxxxxxxxxxxxxxx").Result()
1323-
require.NoError(t, err)
1310+
res := rdb.Do(ctx, "SET", "ifdeq1", "new", "IFDEQ", "xxxxxxxxxxxxxxxx").Val()
13241311
require.Nil(t, res)
13251312
})
13261313

13271314
t.Run("IFDEQ: digest matches writes and returns OK", func(t *testing.T) {
13281315
require.NoError(t, rdb.Set(ctx, "ifdeq2", "hello", 0).Err())
13291316
digest, err := rdb.Do(ctx, "DIGEST", "ifdeq2").Result()
13301317
require.NoError(t, err)
1331-
res, err := rdb.Do(ctx, "SET", "ifdeq2", "world", "IFDEQ", digest).Result()
1332-
require.NoError(t, err)
1318+
res := rdb.Do(ctx, "SET", "ifdeq2", "world", "IFDEQ", digest).Val()
13331319
require.Equal(t, "OK", res)
13341320
require.Equal(t, "world", rdb.Get(ctx, "ifdeq2").Val())
13351321
})
13361322

13371323
t.Run("IFDEQ: digest mismatches returns nil and no write", func(t *testing.T) {
13381324
require.NoError(t, rdb.Set(ctx, "ifdeq3", "hello", 0).Err())
1339-
res, err := rdb.Do(ctx, "SET", "ifdeq3", "world", "IFDEQ", "xxxxxxxxxxxxxxxx").Result()
1340-
require.NoError(t, err)
1325+
res := rdb.Do(ctx, "SET", "ifdeq3", "world", "IFDEQ", "xxxxxxxxxxxxxxxx").Val()
13411326
require.Nil(t, res)
13421327
require.Equal(t, "hello", rdb.Get(ctx, "ifdeq3").Val())
13431328
})
13441329

13451330
t.Run("IFDNE: key not found writes and returns OK", func(t *testing.T) {
13461331
require.NoError(t, rdb.Del(ctx, "ifdne1").Err())
1347-
res, err := rdb.Do(ctx, "SET", "ifdne1", "created", "IFDNE", "xxxxxxxxxxxxxxxx").Result()
1348-
require.NoError(t, err)
1332+
res := rdb.Do(ctx, "SET", "ifdne1", "created", "IFDNE", "xxxxxxxxxxxxxxxx").Val()
13491333
require.Equal(t, "OK", res)
13501334
require.Equal(t, "created", rdb.Get(ctx, "ifdne1").Val())
13511335
})
@@ -1354,16 +1338,14 @@ func TestSetConditional(t *testing.T) {
13541338
require.NoError(t, rdb.Set(ctx, "ifdne2", "hello", 0).Err())
13551339
digest, err := rdb.Do(ctx, "DIGEST", "ifdne2").Result()
13561340
require.NoError(t, err)
1357-
res, err := rdb.Do(ctx, "SET", "ifdne2", "world", "IFDNE", digest).Result()
1358-
require.NoError(t, err)
1341+
res := rdb.Do(ctx, "SET", "ifdne2", "world", "IFDNE", digest).Val()
13591342
require.Nil(t, res)
13601343
require.Equal(t, "hello", rdb.Get(ctx, "ifdne2").Val())
13611344
})
13621345

13631346
t.Run("IFDNE: digest mismatches writes and returns OK", func(t *testing.T) {
13641347
require.NoError(t, rdb.Set(ctx, "ifdne3", "hello", 0).Err())
1365-
res, err := rdb.Do(ctx, "SET", "ifdne3", "world", "IFDNE", "xxxxxxxxxxxxxxxx").Result()
1366-
require.NoError(t, err)
1348+
res := rdb.Do(ctx, "SET", "ifdne3", "world", "IFDNE", "xxxxxxxxxxxxxxxx").Val()
13671349
require.Equal(t, "OK", res)
13681350
require.Equal(t, "world", rdb.Get(ctx, "ifdne3").Val())
13691351
})
@@ -1396,8 +1378,7 @@ func TestSetConditional(t *testing.T) {
13961378

13971379
t.Run("IFEQ with EX: condition not met leaves TTL unchanged", func(t *testing.T) {
13981380
require.NoError(t, rdb.Set(ctx, "ifeq-ex2", "hello", 5*time.Second).Err())
1399-
res, err := rdb.Do(ctx, "SET", "ifeq-ex2", "world", "IFEQ", "wrong", "EX", "100").Result()
1400-
require.NoError(t, err)
1381+
res := rdb.Do(ctx, "SET", "ifeq-ex2", "world", "IFEQ", "wrong", "EX", "100").Val()
14011382
require.Nil(t, res)
14021383
ttl := rdb.TTL(ctx, "ifeq-ex2").Val()
14031384
require.Greater(t, ttl, time.Duration(0))
@@ -1437,8 +1418,7 @@ func TestSetConditional(t *testing.T) {
14371418
val := "value-" + strconv.Itoa(i)
14381419
wrong := "wrong-" + strconv.Itoa(i)
14391420
require.NoError(t, rdb.Set(ctx, key, val, 0).Err())
1440-
res, err := rdb.Do(ctx, "SET", key, "new", "IFEQ", wrong).Result()
1441-
require.NoError(t, err)
1421+
res := rdb.Do(ctx, "SET", key, "new", "IFEQ", wrong).Val()
14421422
require.Nil(t, res, "IFEQ should return nil when cmp_value does not match")
14431423
require.Equal(t, val, rdb.Get(ctx, key).Val())
14441424
require.NoError(t, rdb.Del(ctx, key).Err())
@@ -1467,8 +1447,7 @@ func TestSetConditional(t *testing.T) {
14671447
key := "prop4-" + strconv.Itoa(i)
14681448
val := util.RandString(1, 20, util.Alpha)
14691449
require.NoError(t, rdb.Set(ctx, key, val, 0).Err())
1470-
res, err := rdb.Do(ctx, "SET", key, "new", "IFNE", val).Result()
1471-
require.NoError(t, err)
1450+
res := rdb.Do(ctx, "SET", key, "new", "IFNE", val).Val()
14721451
require.Nil(t, res, "IFNE should return nil when cmp_value matches current value")
14731452
require.Equal(t, val, rdb.Get(ctx, key).Val())
14741453
require.NoError(t, rdb.Del(ctx, key).Err())
@@ -1498,8 +1477,7 @@ func TestSetConditional(t *testing.T) {
14981477
key := "prop6-" + strconv.Itoa(i)
14991478
val := util.RandString(1, 20, util.Alpha)
15001479
require.NoError(t, rdb.Set(ctx, key, val, 0).Err())
1501-
res, err := rdb.Do(ctx, "SET", key, "new", "IFDEQ", "xxxxxxxxxxxxxxxx").Result()
1502-
require.NoError(t, err)
1480+
res := rdb.Do(ctx, "SET", key, "new", "IFDEQ", "xxxxxxxxxxxxxxxx").Val()
15031481
require.Nil(t, res, "IFDEQ should return nil when digest does not match")
15041482
require.Equal(t, val, rdb.Get(ctx, key).Val())
15051483
require.NoError(t, rdb.Del(ctx, key).Err())
@@ -1529,8 +1507,7 @@ func TestSetConditional(t *testing.T) {
15291507
require.NoError(t, rdb.Set(ctx, key, val, 0).Err())
15301508
digest, err := rdb.Do(ctx, "DIGEST", key).Result()
15311509
require.NoError(t, err)
1532-
res, err := rdb.Do(ctx, "SET", key, "new", "IFDNE", digest).Result()
1533-
require.NoError(t, err)
1510+
res := rdb.Do(ctx, "SET", key, "new", "IFDNE", digest).Val()
15341511
require.Nil(t, res, "IFDNE should return nil when digest matches")
15351512
require.Equal(t, val, rdb.Get(ctx, key).Val())
15361513
require.NoError(t, rdb.Del(ctx, key).Err())
@@ -1543,9 +1520,8 @@ func TestSetConditional(t *testing.T) {
15431520
key := "prop9-" + strconv.Itoa(i)
15441521
val := "value-" + strconv.Itoa(i)
15451522
require.NoError(t, rdb.Set(ctx, key, val, 10*time.Second).Err())
1546-
// IFEQ with wrong value → condition not met
1547-
res, err := rdb.Do(ctx, "SET", key, "new", "IFEQ", "wrong", "EX", "9999").Result()
1548-
require.NoError(t, err)
1523+
// IFEQ with wrong value: condition not met
1524+
res := rdb.Do(ctx, "SET", key, "new", "IFEQ", "wrong", "EX", "9999").Val()
15491525
require.Nil(t, res)
15501526
ttl := rdb.TTL(ctx, key).Val()
15511527
require.Greater(t, ttl, time.Duration(0), "TTL should remain positive after failed conditional SET")

0 commit comments

Comments
 (0)
⚡