From 6dae9b87c0d46e67c1455f835405fd6593d52fe4 Mon Sep 17 00:00:00 2001 From: TJ Zhang Date: Mon, 9 Dec 2024 18:05:37 -0800 Subject: [PATCH] address comments Signed-off-by: TJ Zhang --- CHANGELOG.md | 2 +- go/api/base_client.go | 2 +- go/api/response_handlers.go | 5 --- go/api/set_commands.go | 41 +++++++++++++++++++++- go/integTest/shared_commands_test.go | 51 +++++++++++++++++----------- 5 files changed, 73 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 020458522b..dd8a8947e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * Java: Bump protobuf (protoc) version ([#2796](https://github.com/valkey-io/valkey-glide/pull/2796), [#2800](https://github.com/valkey-io/valkey-glide/pull/2800)) * Go: Add `SInterStore` ([#2779](https://github.com/valkey-io/valkey-glide/issues/2779)) * Node: Remove native package references for MacOs x64 architecture ([#2799](https://github.com/valkey-io/valkey-glide/issues/2799)) +* Go: Add `SScan` and `SMove` ([#2789](https://github.com/valkey-io/valkey-glide/issues/2789)) #### Breaking Changes @@ -106,7 +107,6 @@ * Core: Improve retry logic and update unmaintained dependencies for Rust lint CI ([#2673](https://github.com/valkey-io/valkey-glide/pull/2643)) * Core: Release the read lock while creating connections in `refresh_connections` ([#2630](https://github.com/valkey-io/valkey-glide/issues/2630)) * Core: SlotMap refactor - Added NodesMap, Update the slot map upon MOVED errors ([#2682](https://github.com/valkey-io/valkey-glide/issues/2682)) -* Go: Add `SScan` and `SMove` ([#2789](https://github.com/valkey-io/valkey-glide/issues/2789)) #### Breaking Changes diff --git a/go/api/base_client.go b/go/api/base_client.go index b3f13baf19..4bd8c7609b 100644 --- a/go/api/base_client.go +++ b/go/api/base_client.go @@ -652,7 +652,7 @@ func (client *baseClient) SScan(key string, cursor string) (string, []string, er return handleScanResponse(result) } -func (client *baseClient) SScanWithOption( +func (client *baseClient) SScanWithOptions( key string, cursor string, options *BaseScanOptions, diff --git a/go/api/response_handlers.go b/go/api/response_handlers.go index 6bf2105b65..7b41a589ef 100644 --- a/go/api/response_handlers.go +++ b/go/api/response_handlers.go @@ -373,11 +373,6 @@ func handleStringSetResponse(response *C.struct_CommandResponse) (map[Result[str return slice, nil } -type ScanResult struct { - cursor Result[string] - results []Result[string] -} - func handleScanResponse( response *C.struct_CommandResponse, ) (string, []string, error) { diff --git a/go/api/set_commands.go b/go/api/set_commands.go index 18f67a32c0..90110108db 100644 --- a/go/api/set_commands.go +++ b/go/api/set_commands.go @@ -327,11 +327,14 @@ type SetCommands interface { // Iterates incrementally over a set. // + // Note: When in cluster mode, all keys must map to the same hash slot. + // // See [valkey.io] for details. // // Parameters: // key - The key of the set. // cursor - The cursor that points to the next iteration of results. A value of `"0"` indicates the start of the search. + // For Valkey 8.0 and above, negative cursors are treated like the initial cursor("0") instead of throwing an error. // // Return value: // An array of the cursor and the subset of the set held by `key`. The first element is always the `cursor` and @@ -339,17 +342,34 @@ type SetCommands interface { // The second element is always an array of the subset of the set held in `key`. // // Example: + // // assume "key" contains a set + // resCursor, resCol, err := client.sscan("key", "0") + // for resCursor != "0" { + // resCursor, resCol, err = client.sscan("key", "0") + // fmt.Println("Cursor: ", resCursor) + // fmt.Println("Members: ", resCol) + // } + // // Output: + // // Cursor: 48 + // // Members: ['3', '118', '120', '86', '76', '13', '61', '111', '55', '45'] + // // Cursor: 24 + // // Members: ['38', '109', '11', '119', '34', '24', '40', '57', '20', '17'] + // // Cursor: 0 + // // Members: ['47', '122', '1', '53', '10', '14', '80'] // // [valkey.io]: https://valkey.io/commands/sscan/ SScan(key string, cursor string) (string, []string, error) // Iterates incrementally over a set. // + // Note: When in cluster mode, all keys must map to the same hash slot. + // // See [valkey.io] for details. // // Parameters: // key - The key of the set. // cursor - The cursor that points to the next iteration of results. A value of `"0"` indicates the start of the search. + // For Valkey 8.0 and above, negative cursors are treated like the initial cursor("0") instead of throwing an error. // options - [BaseScanOptions] // // Return value: @@ -358,13 +378,30 @@ type SetCommands interface { // The second element is always an array of the subset of the set held in `key`. // // Example: + // // assume "key" contains a set + // resCursor resCol, err := client.sscan("key", "0", opts) + // for resCursor != "0" { + // opts := api.NewBaseScanOptionsBuilder().SetMatch("*") + // resCursor, resCol, err = client.sscan("key", "0", opts) + // fmt.Println("Cursor: ", resCursor) + // fmt.Println("Members: ", resCol) + // } + // // Output: + // // Cursor: 48 + // // Members: ['3', '118', '120', '86', '76', '13', '61', '111', '55', '45'] + // // Cursor: 24 + // // Members: ['38', '109', '11', '119', '34', '24', '40', '57', '20', '17'] + // // Cursor: 0 + // // Members: ['47', '122', '1', '53', '10', '14', '80'] // // [valkey.io]: https://valkey.io/commands/sscan/ - SScanWithOption(key string, cursor string, options *BaseScanOptions) (string, []string, error) + SScanWithOptions(key string, cursor string, options *BaseScanOptions) (string, []string, error) // Moves `member` from the set at `source` to the set at `destination`, removing it from the source set. // Creates a new destination set if needed. The operation is atomic. // + // Note: When in cluster mode, `source` and `destination` must map to the same hash slot. + // // See [valkey.io] for details. // // Parameters: @@ -376,6 +413,8 @@ type SetCommands interface { // `true` on success, or `false` if the `source` set does not exist or the element is not a member of the source set. // // Example: + // moved := SMove("set1", "set2", "element") + // fmt.Println(moved) // Output: true // // [valkey.io]: https://valkey.io/commands/smove/ SMove(source string, destination string, member string) (Result[bool], error) diff --git a/go/integTest/shared_commands_test.go b/go/integTest/shared_commands_test.go index 30e62d2f8d..600a42cdbc 100644 --- a/go/integTest/shared_commands_test.go +++ b/go/integTest/shared_commands_test.go @@ -4,6 +4,7 @@ package integTest import ( "math" + "reflect" "strconv" "time" @@ -1849,16 +1850,20 @@ func (suite *GlideTestSuite) TestSMove() { res4, err := client.SMembers(key1) assert.NoError(t, err) - assert.Len(t, res4, 2) - assert.Contains(t, res4, api.CreateStringResult("2")) - assert.Contains(t, res4, api.CreateStringResult("3")) + expectedSet := map[api.Result[string]]struct{}{ + api.CreateStringResult("2"): {}, + api.CreateStringResult("3"): {}, + } + assert.True(t, reflect.DeepEqual(expectedSet, res4)) res5, err := client.SMembers(key2) assert.NoError(t, err) - assert.Len(t, res5, 3) - assert.Contains(t, res5, api.CreateStringResult("1")) - assert.Contains(t, res5, api.CreateStringResult("2")) - assert.Contains(t, res5, api.CreateStringResult("3")) + expectedSet = map[api.Result[string]]struct{}{ + api.CreateStringResult("1"): {}, + api.CreateStringResult("2"): {}, + api.CreateStringResult("3"): {}, + } + assert.True(t, reflect.DeepEqual(expectedSet, res5)) // moved element already exists in the destination set res6, err := client.SMove(key2, key1, "2") @@ -1867,15 +1872,19 @@ func (suite *GlideTestSuite) TestSMove() { res7, err := client.SMembers(key1) assert.NoError(t, err) - assert.Len(t, res7, 2) - assert.Contains(t, res7, api.CreateStringResult("2")) - assert.Contains(t, res7, api.CreateStringResult("3")) + expectedSet = map[api.Result[string]]struct{}{ + api.CreateStringResult("2"): {}, + api.CreateStringResult("3"): {}, + } + assert.True(t, reflect.DeepEqual(expectedSet, res7)) res8, err := client.SMembers(key2) assert.NoError(t, err) - assert.Len(t, res8, 2) - assert.Contains(t, res8, api.CreateStringResult("1")) - assert.Contains(t, res8, api.CreateStringResult("3")) + expectedSet = map[api.Result[string]]struct{}{ + api.CreateStringResult("1"): {}, + api.CreateStringResult("3"): {}, + } + assert.True(t, reflect.DeepEqual(expectedSet, res8)) // attempt to move from a non-existing key res9, err := client.SMove(nonExistingKey, key1, "4") @@ -1884,9 +1893,11 @@ func (suite *GlideTestSuite) TestSMove() { res10, err := client.SMembers(key1) assert.NoError(t, err) - assert.Len(t, res10, 2) - assert.Contains(t, res10, api.CreateStringResult("2")) - assert.Contains(t, res10, api.CreateStringResult("3")) + expectedSet = map[api.Result[string]]struct{}{ + api.CreateStringResult("2"): {}, + api.CreateStringResult("3"): {}, + } + assert.True(t, reflect.DeepEqual(expectedSet, res10)) // move to a new set res11, err := client.SMove(key1, key3, "2") @@ -1983,7 +1994,7 @@ func (suite *GlideTestSuite) TestSScan() { assert.True(t, isSubset(resCollection, charMembers)) opts := api.NewBaseScanOptionsBuilder().SetMatch("a") - resCursor, resCollection, err = client.SScanWithOption(key1, initialCursor, opts) + resCursor, resCollection, err = client.SScanWithOptions(key1, initialCursor, opts) assert.NoError(t, err) assert.Equal(t, initialCursor, resCursor) assert.True(t, isSubset(resCollection, []string{"a"})) @@ -2010,21 +2021,21 @@ func (suite *GlideTestSuite) TestSScan() { // test match pattern opts = api.NewBaseScanOptionsBuilder().SetMatch("*") - resCursor, resCollection, err = client.SScanWithOption(key1, initialCursor, opts) + resCursor, resCollection, err = client.SScanWithOptions(key1, initialCursor, opts) assert.NoError(t, err) assert.NotEqual(t, initialCursor, resCursor) assert.GreaterOrEqual(t, len(resCollection), defaultCount) // test count opts = api.NewBaseScanOptionsBuilder().SetCount(20) - resCursor, resCollection, err = client.SScanWithOption(key1, initialCursor, opts) + resCursor, resCollection, err = client.SScanWithOptions(key1, initialCursor, opts) assert.NoError(t, err) assert.NotEqual(t, initialCursor, resCursor) assert.GreaterOrEqual(t, len(resCollection), 20) // test count with match, returns a non-empty array opts = api.NewBaseScanOptionsBuilder().SetMatch("1*").SetCount(20) - resCursor, resCollection, err = client.SScanWithOption(key1, initialCursor, opts) + resCursor, resCollection, err = client.SScanWithOptions(key1, initialCursor, opts) assert.NoError(t, err) assert.NotEqual(t, initialCursor, resCursor) assert.GreaterOrEqual(t, len(resCollection), 0)