Fixed code review comments for multi cluster value

Signed-off-by: Niharika Bhavaraju <[email protected]>
niharikabhavaraju · Jan 27, 2025 · 2ce5dbc · 2ce5dbc
1 parent 92886ba
commit 2ce5dbc
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 31 deletions.
diff --git a/go/api/glide_cluster_client.go b/go/api/glide_cluster_client.go
@@ -5,7 +5,9 @@ package api
 // #cgo LDFLAGS: -L../target/release -lglide_rs
 // #include "../lib.h"
 import "C"
-import "github.com/valkey-io/valkey-glide/go/glide/api/options"
+import (
+	"github.com/valkey-io/valkey-glide/go/glide/api/options"
+)
 
 // GlideClusterClient interface compliance check.
 var _ GlideClusterClientCommands = (*GlideClusterClient)(nil)
@@ -89,15 +91,25 @@ func (client *GlideClusterClient) CustomCommand(args []string) (ClusterValue[int
 // Example:
 //
 //	route := api.SimpleNodeRoute(api.RandomRoute)
-//	options := options.NewTimeOptionsBuilder().SetRoute(route)
-//	result, err := client.TimeWithOptions(route)
-//	fmt.Println(result.Value()) // Output: [1737285074 67888]
+//	route := config.Route(config.AllNodes)
+//	opts  := options.ClusterTimeOptions{
+//		  Route: &route,
+//	}
+//	fmt.Println(clusterResponse.Value()) // Output: [1737994354 547816 1737994354 547856]
 //
 // [valkey.io]: https://valkey.io/commands/time/
-func (client *glideClusterClient) TimeWithOptions(opts *options.TimeOptions) (ClusterValue[[]string], error) {
-	result, err := client.executeCommandWithRoute(C.Time, []string{}, opts.Route)
-	if err != nil {
-		return CreateEmptyStringArrayClusterValue(), err
+func (client *GlideClusterClient) TimeWithOptions(opts options.ClusterTimeOptions) (ClusterValue[[]string], error) {
+	if opts.Route == nil {
+		response, err := client.executeCommand(C.Time, []string{})
+		if err != nil {
+			return CreateEmptyStringArrayClusterValue(), err
+		}
+		return handleTimeClusterResponse(response)
+	} else {
+		result, err := client.executeCommandWithRoute(C.Time, []string{}, *opts.Route)
+		if err != nil {
+			return CreateEmptyStringArrayClusterValue(), err
+		}
+		return handleTimeClusterResponse(result)
 	}
-	return handleTimeClusterResponse(result)
 }
diff --git a/go/api/options/time_options.go b/go/api/options/time_options.go
@@ -2,15 +2,6 @@ package options
 
 import "github.com/valkey-io/valkey-glide/go/glide/api/config"
 
-type TimeOptions struct {
-	Route config.Route
-}
-
-func NewTimeOptionsBuilder() *TimeOptions {
-	return &TimeOptions{}
-}
-
-func (timeOptions *TimeOptions) SetRoute(route config.Route) *TimeOptions {
-	timeOptions.Route = route
-	return timeOptions
+type ClusterTimeOptions struct {
+	Route *config.Route
 }
diff --git a/go/api/server_management_cluster_commands.go b/go/api/server_management_cluster_commands.go
@@ -10,5 +10,5 @@ import "github.com/valkey-io/valkey-glide/go/glide/api/options"
 //
 // [valkey.io]: https://valkey.io/commands/#server
 type ServerManagementClusterCommands interface {
-	TimeWithOptions(timeOptions *options.TimeOptions) (ClusterValue[[]string], error)
+	TimeWithOptions(timeOptions options.ClusterTimeOptions) (ClusterValue[[]string], error)
 }
diff --git a/go/integTest/cluster_commands_test.go b/go/integTest/cluster_commands_test.go
@@ -30,10 +30,9 @@ func (suite *GlideTestSuite) TestClusterCustomCommandEcho() {
 	assert.Equal(suite.T(), "GO GLIDE GO", result.Value().(string))
 }
 
-func (suite *GlideTestSuite) TestTime_RandomRoute() {
+func (suite *GlideTestSuite) TestTimeWithoutRoute() {
 	client := suite.defaultClusterClient()
-	route := config.SimpleNodeRoute(config.RandomRoute)
-	options := options.NewTimeOptionsBuilder().SetRoute(route)
+	options := options.ClusterTimeOptions{Route: nil}
 	result, err := client.TimeWithOptions(options)
 
 	assert.NoError(suite.T(), err)
@@ -43,27 +42,39 @@ func (suite *GlideTestSuite) TestTime_RandomRoute() {
 	assert.Equal(suite.T(), 2, len(result.Value()))
 }
 
-func (suite *GlideTestSuite) TestTime_AllNodes_MultipleValues() {
+func (suite *GlideTestSuite) TestTimeWithAllNodesRoute() {
 	client := suite.defaultClusterClient()
-	route := config.AllNodes
-	options := options.NewTimeOptionsBuilder().SetRoute(route)
+	route := config.Route(config.AllNodes)
+	options := options.ClusterTimeOptions{Route: &route}
 	result, err := client.TimeWithOptions(options)
+
 	assert.NoError(suite.T(), err)
 	assert.NotNil(suite.T(), result)
 	assert.NotEmpty(suite.T(), result.Value())
-
 	assert.Greater(suite.T(), len(result.Value()), 1)
 
 	for _, timeStr := range result.Value() {
 		assert.IsType(suite.T(), "", timeStr)
 	}
 }
 
-func (suite *GlideTestSuite) TestTime_ErrorHandling() {
+func (suite *GlideTestSuite) TestTimeWithRandomRoute() {
 	client := suite.defaultClusterClient()
-	invalidRoute := config.NewByAddressRoute("invalidHost", 9999)
+	route := config.Route(config.RandomRoute)
+	options := options.ClusterTimeOptions{Route: &route}
+	result, err := client.TimeWithOptions(options)
 
-	options := options.NewTimeOptionsBuilder().SetRoute(invalidRoute)
+	assert.NoError(suite.T(), err)
+	assert.NotNil(suite.T(), result)
+	assert.NotEmpty(suite.T(), result.Value())
+	assert.IsType(suite.T(), "", result.Value()[0])
+	assert.Equal(suite.T(), 2, len(result.Value()))
+}
+
+func (suite *GlideTestSuite) TestTimeWithInvalidRoute() {
+	client := suite.defaultClusterClient()
+	invalidRoute := config.Route(config.NewByAddressRoute("invalidHost", 9999))
+	options := options.ClusterTimeOptions{Route: &invalidRoute}
 	result, err := client.TimeWithOptions(options)
 
 	assert.NotNil(suite.T(), err)