address comments

Signed-off-by: TJ Zhang <tj.zhang@improving.com>

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
 

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,

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) {

go/api/set_commands.go

@@ -327,29 +327,49 @@ 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
 	//  for the next iteration of results. The `cursor` will be `"0"` on the last iteration of the set.
 	//  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)

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)