From ac2cfada5514bff7959a535f72ecda3c826ed7ab Mon Sep 17 00:00:00 2001 From: Janhavi Gupta Date: Mon, 21 Oct 2024 03:36:27 +0000 Subject: [PATCH 1/6] GO:Handling interface response Signed-off-by: Janhavi Gupta --- go/api/glide_client.go | 8 +- go/api/response_handlers.go | 96 ++++++++++++++++++++++++ go/integTest/standalone_commands_test.go | 29 ++++++- 3 files changed, 125 insertions(+), 8 deletions(-) diff --git a/go/api/glide_client.go b/go/api/glide_client.go index 27f3dbac75..2f8e8e5235 100644 --- a/go/api/glide_client.go +++ b/go/api/glide_client.go @@ -33,18 +33,12 @@ func NewGlideClient(config *GlideClientConfiguration) (*GlideClient, error) { // For example, to return a list of all pub/sub clients: // // client.CustomCommand([]string{"CLIENT", "LIST","TYPE", "PUBSUB"}) -// -// TODO: Add support for complex return types. func (client *GlideClient) CustomCommand(args []string) (interface{}, error) { res, err := client.executeCommand(C.CustomCommand, args) if err != nil { return nil, err } - resString, err := handleStringOrNullResponse(res) - if err != nil { - return nil, err - } - return resString.Value(), err + return handleInterfaceResponse(res) } // Sets configuration parameters to the specified values. diff --git a/go/api/response_handlers.go b/go/api/response_handlers.go index 08714c8b22..d069d0b524 100644 --- a/go/api/response_handlers.go +++ b/go/api/response_handlers.go @@ -59,6 +59,102 @@ func convertCharArrayToString(response *C.struct_CommandResponse, isNilable bool return CreateStringResult(string(byteSlice)), nil } +func handleInterfaceResponse(response *C.struct_CommandResponse) (interface{}, error) { + defer C.free_command_response(response) + + return parseInterface(response) +} + +func parseInterface(response *C.struct_CommandResponse) (interface{}, error) { + if response == nil { + return nil, nil + } + + switch response.response_type { + case C.Null: + return nil, nil + case C.String: + return parseString(response) + case C.Int: + return int64(response.int_value), nil + case C.Float: + return float64(response.float_value), nil + case C.Bool: + return bool(response.bool_value), nil + case C.Array: + return parseArray(response) + case C.Map: + return parseMap(response) + case C.Sets: + return parseSet(response) + } + + return nil, &RequestError{"Unexpected return type from Valkey"} +} + +func parseString(response *C.struct_CommandResponse) (interface{}, error) { + if response.string_value == nil { + return nil, nil + } + byteSlice := C.GoBytes(unsafe.Pointer(response.string_value), C.int(int64(response.string_value_len))) + + // Create Go string from byte slice (preserving null characters) + return string(byteSlice), nil +} + +func parseArray(response *C.struct_CommandResponse) (interface{}, error) { + if response.array_value == nil { + return nil, nil + } + + var slice []interface{} + for _, v := range unsafe.Slice(response.array_value, response.array_value_len) { + res, err := parseInterface(&v) + if err != nil { + return nil, err + } + slice = append(slice, res) + } + return slice, nil +} + +func parseMap(response *C.struct_CommandResponse) (interface{}, error) { + if response.array_value == nil { + return nil, nil + } + + var value_map = make(map[interface{}]interface{}, response.array_value_len) + for _, v := range unsafe.Slice(response.array_value, response.array_value_len) { + res_key, err := parseInterface(v.map_key) + if err != nil { + return nil, err + } + res_val, err := parseInterface(v.map_value) + if err != nil { + return nil, err + } + value_map[res_key] = res_val + } + return value_map, nil +} + +func parseSet(response *C.struct_CommandResponse) (interface{}, error) { + if response.array_value == nil { + return nil, nil + } + + slice := make(map[interface{}]struct{}, response.sets_value_len) + for _, v := range unsafe.Slice(response.sets_value, response.sets_value_len) { + res, err := parseInterface(&v) + if err != nil { + return nil, err + } + slice[res] = struct{}{} + } + + return slice, nil +} + func handleStringResponse(response *C.struct_CommandResponse) (Result[string], error) { defer C.free_command_response(response) diff --git a/go/integTest/standalone_commands_test.go b/go/integTest/standalone_commands_test.go index 4186c6b184..36ef3ebce4 100644 --- a/go/integTest/standalone_commands_test.go +++ b/go/integTest/standalone_commands_test.go @@ -6,6 +6,7 @@ import ( "fmt" "strings" + "github.com/google/uuid" "github.com/valkey-io/valkey-glide/go/glide/api" "github.com/stretchr/testify/assert" @@ -26,7 +27,7 @@ func (suite *GlideTestSuite) TestCustomCommandPing() { result, err := client.CustomCommand([]string{"PING"}) assert.Nil(suite.T(), err) - assert.Equal(suite.T(), "PONG", result) + assert.Equal(suite.T(), "PONG", result.(string)) } func (suite *GlideTestSuite) TestCustomCommandClientInfo() { @@ -44,6 +45,32 @@ func (suite *GlideTestSuite) TestCustomCommandClientInfo() { assert.True(suite.T(), strings.Contains(strResult, fmt.Sprintf("name=%s", clientName))) } +func (suite *GlideTestSuite) TestCustomCommandMGET() { + clientName := "TEST_CLIENT_NAME" + config := api.NewGlideClientConfiguration(). + WithAddress(&api.NodeAddress{Port: suite.standalonePorts[0]}). + WithClientName(clientName) + client := suite.client(config) + + key1 := uuid.New().String() + key2 := uuid.New().String() + key3 := uuid.New().String() + oldValue := uuid.New().String() + value := uuid.New().String() + suite.verifyOK(client.Set(key1, oldValue)) + keyValueMap := map[string]string{ + key1: value, + key2: value, + } + suite.verifyOK(client.MSet(keyValueMap)) + keys := []string{key1, key2, key3} + values := []interface{}{value, value, nil} + result, err := client.CustomCommand(append([]string{"MGET"}, keys...)) + + assert.Nil(suite.T(), err) + assert.Equal(suite.T(), values, result.([]interface{})) +} + func (suite *GlideTestSuite) TestCustomCommand_invalidCommand() { client := suite.defaultClient() result, err := client.CustomCommand([]string{"pewpew"}) From 7d2fdbc006f1c5409338b2729ee853078a0eb3a0 Mon Sep 17 00:00:00 2001 From: Janhavi Gupta Date: Mon, 4 Nov 2024 06:59:29 +0000 Subject: [PATCH 2/6] Fixing lint Signed-off-by: Janhavi Gupta --- go/api/response_handlers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/api/response_handlers.go b/go/api/response_handlers.go index e7d97fd894..23f9181c3b 100644 --- a/go/api/response_handlers.go +++ b/go/api/response_handlers.go @@ -123,7 +123,7 @@ func parseMap(response *C.struct_CommandResponse) (interface{}, error) { return nil, nil } - var value_map = make(map[interface{}]interface{}, response.array_value_len) + value_map := make(map[interface{}]interface{}, response.array_value_len) for _, v := range unsafe.Slice(response.array_value, response.array_value_len) { res_key, err := parseInterface(v.map_key) if err != nil { From cc59ece64f8d7754111f8b3ba989f54f3c541e7b Mon Sep 17 00:00:00 2001 From: Janhavi Gupta Date: Fri, 15 Nov 2024 04:13:55 +0000 Subject: [PATCH 3/6] Adding more tests for custom command Signed-off-by: Janhavi Gupta --- go/api/response_handlers.go | 2 +- go/integTest/standalone_commands_test.go | 78 +++++++++++++++++++++++- 2 files changed, 77 insertions(+), 3 deletions(-) diff --git a/go/api/response_handlers.go b/go/api/response_handlers.go index 23f9181c3b..63f0ac0007 100644 --- a/go/api/response_handlers.go +++ b/go/api/response_handlers.go @@ -139,7 +139,7 @@ func parseMap(response *C.struct_CommandResponse) (interface{}, error) { } func parseSet(response *C.struct_CommandResponse) (interface{}, error) { - if response.array_value == nil { + if response.sets_value == nil { return nil, nil } diff --git a/go/integTest/standalone_commands_test.go b/go/integTest/standalone_commands_test.go index 36ef3ebce4..80cbb63581 100644 --- a/go/integTest/standalone_commands_test.go +++ b/go/integTest/standalone_commands_test.go @@ -22,7 +22,7 @@ func (suite *GlideTestSuite) TestCustomCommandInfo() { assert.True(suite.T(), strings.Contains(strResult, "# Stats")) } -func (suite *GlideTestSuite) TestCustomCommandPing() { +func (suite *GlideTestSuite) TestCustomCommandPing_StringResponse() { client := suite.defaultClient() result, err := client.CustomCommand([]string{"PING"}) @@ -45,7 +45,51 @@ func (suite *GlideTestSuite) TestCustomCommandClientInfo() { assert.True(suite.T(), strings.Contains(strResult, fmt.Sprintf("name=%s", clientName))) } -func (suite *GlideTestSuite) TestCustomCommandMGET() { +func (suite *GlideTestSuite) TestCustomCommandGet_NullResponse() { + client := suite.defaultClient() + key := uuid.New().String() + result, err := client.CustomCommand([]string{"GET", key}) + + assert.Nil(suite.T(), err) + assert.Equal(suite.T(), nil, result) +} + +func (suite *GlideTestSuite) TestCustomCommandDel_LongResponse() { + client := suite.defaultClient() + key := uuid.New().String() + suite.verifyOK(client.Set(key, "value")) + result, err := client.CustomCommand([]string{"DEL", key}) + + assert.Nil(suite.T(), err) + assert.Equal(suite.T(), int64(1), result.(int64)) +} + +func (suite *GlideTestSuite) TestCustomCommandHExists_BoolResponse() { + client := suite.defaultClient() + fields := map[string]string{"field1": "value1"} + key := uuid.New().String() + + res1, err := client.HSet(key, fields) + assert.Nil(suite.T(), err) + assert.Equal(suite.T(), int64(1), res1.Value()) + + result, err := client.CustomCommand([]string{"HEXISTS", key, "field1"}) + + assert.Nil(suite.T(), err) + assert.Equal(suite.T(), true, result.(bool)) +} + +func (suite *GlideTestSuite) TestCustomCommandIncrByFloat_FloatResponse() { + client := suite.defaultClient() + key := uuid.New().String() + + result, err := client.CustomCommand([]string{"INCRBYFLOAT", key, fmt.Sprintf("%f", 0.1)}) + + assert.Nil(suite.T(), err) + assert.Equal(suite.T(), float64(0.1), result.(float64)) +} + +func (suite *GlideTestSuite) TestCustomCommandMGet_ArrayResponse() { clientName := "TEST_CLIENT_NAME" config := api.NewGlideClientConfiguration(). WithAddress(&api.NodeAddress{Port: suite.standalonePorts[0]}). @@ -71,6 +115,36 @@ func (suite *GlideTestSuite) TestCustomCommandMGET() { assert.Equal(suite.T(), values, result.([]interface{})) } +func (suite *GlideTestSuite) TestCustomCommandConfigGet_MapResponse() { + client := suite.defaultClient() + + if suite.serverVersion < "7.0.0" { + suite.T().Skip("This feature is added in version 7") + } + configMap := map[string]string{"timeout": "1000", "maxmemory": "1GB"} + result, err := client.ConfigSet(configMap) + assert.Nil(suite.T(), err) + assert.Equal(suite.T(), "OK", result.Value()) + + result2, err := client.CustomCommand([]string{"CONFIG", "GET", "timeout", "maxmemory"}) + assert.Nil(suite.T(), err) + assert.Equal(suite.T(), map[interface{}]interface{}{"timeout": "1000", "maxmemory": "1073741824"}, result2) +} + +func (suite *GlideTestSuite) TestCustomCommandConfigSMembers_SetResponse() { + client := suite.defaultClient() + key := uuid.NewString() + members := []string{"member1", "member2", "member3"} + + res1, err := client.SAdd(key, members) + assert.Nil(suite.T(), err) + assert.Equal(suite.T(), int64(3), res1.Value()) + + result2, err := client.CustomCommand([]string{"SMEMBERS", key}) + assert.Nil(suite.T(), err) + assert.Equal(suite.T(), map[interface{}]struct{}{"member1": {}, "member2": {}, "member3": {}}, result2) +} + func (suite *GlideTestSuite) TestCustomCommand_invalidCommand() { client := suite.defaultClient() result, err := client.CustomCommand([]string{"pewpew"}) From eb3adf8e579406cd51c54ae5f314e1220d8fde40 Mon Sep 17 00:00:00 2001 From: Janhavi Gupta Date: Mon, 18 Nov 2024 07:54:25 +0000 Subject: [PATCH 4/6] GO: Fixing errors to support version 1.18 Signed-off-by: Janhavi Gupta --- go/utils/transform_utils.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/go/utils/transform_utils.go b/go/utils/transform_utils.go index cddc6a8e0e..947c9afcb7 100644 --- a/go/utils/transform_utils.go +++ b/go/utils/transform_utils.go @@ -3,14 +3,22 @@ package utils import ( + "reflect" "strconv" "unsafe" ) // Convert `s` of type `string` into `[]byte` func StringToBytes(s string) []byte { - p := unsafe.StringData(s) - b := unsafe.Slice(p, len(s)) + if len(s) == 0 { + return nil + } + var b []byte + hdr := (*reflect.StringHeader)(unsafe.Pointer(&s)) + sliceHeader := (*reflect.SliceHeader)(unsafe.Pointer(&b)) + sliceHeader.Data = hdr.Data + sliceHeader.Len = hdr.Len + sliceHeader.Cap = hdr.Len return b } From 114dcb64f284cdce755ef3abd68aba332bfc1c82 Mon Sep 17 00:00:00 2001 From: Janhavi Gupta Date: Mon, 18 Nov 2024 08:19:46 +0000 Subject: [PATCH 5/6] GO: Fixing the rust crate errors Signed-off-by: Janhavi Gupta --- go/Cargo.toml | 1 - go/src/lib.rs | 28 ++++++++++++++++++++-------- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/go/Cargo.toml b/go/Cargo.toml index 62872578da..6d6c4ecb15 100644 --- a/go/Cargo.toml +++ b/go/Cargo.toml @@ -13,7 +13,6 @@ redis = { path = "../submodules/redis-rs/redis", features = ["aio", "tokio-comp" glide-core = { path = "../glide-core", features = ["socket-layer"] } tokio = { version = "^1", features = ["rt", "macros", "rt-multi-thread", "time"] } protobuf = { version = "3.3.0", features = [] } -derivative = "2.2.0" [profile.release] lto = true diff --git a/go/src/lib.rs b/go/src/lib.rs index d9bdf14e68..55b3c9515c 100644 --- a/go/src/lib.rs +++ b/go/src/lib.rs @@ -3,7 +3,6 @@ */ #![deny(unsafe_op_in_unsafe_fn)] -use derivative::Derivative; use glide_core::client::Client as GlideClient; use glide_core::connection_request; use glide_core::errors; @@ -28,8 +27,7 @@ use tokio::runtime::Runtime; /// The struct is freed by the external caller by using `free_command_response` to avoid memory leaks. /// TODO: Add a type enum to validate what type of response is being sent in the CommandResponse. #[repr(C)] -#[derive(Derivative)] -#[derivative(Debug, Default)] +#[derive(Debug)] pub struct CommandResponse { response_type: ResponseType, int_value: c_long, @@ -39,33 +37,47 @@ pub struct CommandResponse { /// Below two values are related to each other. /// `string_value` represents the string. /// `string_value_len` represents the length of the string. - #[derivative(Default(value = "std::ptr::null_mut()"))] string_value: *mut c_char, string_value_len: c_long, /// Below two values are related to each other. /// `array_value` represents the array of CommandResponse. /// `array_value_len` represents the length of the array. - #[derivative(Default(value = "std::ptr::null_mut()"))] array_value: *mut CommandResponse, array_value_len: c_long, /// Below two values represent the Map structure inside CommandResponse. /// The map is transformed into an array of (map_key: CommandResponse, map_value: CommandResponse) and passed to Go. /// These are represented as pointers as the map can be null (optionally present). - #[derivative(Default(value = "std::ptr::null_mut()"))] map_key: *mut CommandResponse, - #[derivative(Default(value = "std::ptr::null_mut()"))] map_value: *mut CommandResponse, /// Below two values are related to each other. /// `sets_value` represents the set of CommandResponse. /// `sets_value_len` represents the length of the set. - #[derivative(Default(value = "std::ptr::null_mut()"))] sets_value: *mut CommandResponse, sets_value_len: c_long, } +impl Default for CommandResponse { + fn default() -> Self { + CommandResponse { + response_type: ResponseType::default(), + int_value: 0, + float_value: 0.0, + bool_value: false, + string_value: std::ptr::null_mut(), + string_value_len: 0, + array_value: std::ptr::null_mut(), + array_value_len: 0, + map_key: std::ptr::null_mut(), + map_value: std::ptr::null_mut(), + sets_value: std::ptr::null_mut(), + sets_value_len: 0, + } + } +} + #[repr(C)] #[derive(Debug, Default)] pub enum ResponseType { From 3ffbf6756a1ad58c3b1322e50542b6cee73e352d Mon Sep 17 00:00:00 2001 From: Janhavi Gupta Date: Tue, 26 Nov 2024 07:57:59 +0000 Subject: [PATCH 6/6] GO: Reverting the changes to support go version 1.18 Signed-off-by: Janhavi Gupta --- go/utils/transform_utils.go | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/go/utils/transform_utils.go b/go/utils/transform_utils.go index 947c9afcb7..cddc6a8e0e 100644 --- a/go/utils/transform_utils.go +++ b/go/utils/transform_utils.go @@ -3,22 +3,14 @@ package utils import ( - "reflect" "strconv" "unsafe" ) // Convert `s` of type `string` into `[]byte` func StringToBytes(s string) []byte { - if len(s) == 0 { - return nil - } - var b []byte - hdr := (*reflect.StringHeader)(unsafe.Pointer(&s)) - sliceHeader := (*reflect.SliceHeader)(unsafe.Pointer(&b)) - sliceHeader.Data = hdr.Data - sliceHeader.Len = hdr.Len - sliceHeader.Cap = hdr.Len + p := unsafe.StringData(s) + b := unsafe.Slice(p, len(s)) return b }