Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Go: Adding the generic map handler and config set, get commands for standalone. #2303

Merged
merged 5 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ lib.h
# benchmarking results
benchmarks/results/**
benchmarks/gobenchmarks.json
benchmarks/benchmarks
24 changes: 11 additions & 13 deletions go/api/base_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,13 @@ func (client *baseClient) executeCommand(requestType C.RequestType, args []strin
return nil, &ClosingError{"ExecuteCommand failed. The client is closed."}
}

cArgs, argLengths := toCStrings(args)
var cArgsPtr *C.uintptr_t = nil
var argLengthsPtr *C.ulong = nil
if len(args) > 0 {
cArgs, argLengths := toCStrings(args)
cArgsPtr = &cArgs[0]
argLengthsPtr = &argLengths[0]
}

resultChannel := make(chan payload)
resultChannelPtr := uintptr(unsafe.Pointer(&resultChannel))
Expand All @@ -112,8 +118,8 @@ func (client *baseClient) executeCommand(requestType C.RequestType, args []strin
C.uintptr_t(resultChannelPtr),
uint32(requestType),
C.size_t(len(args)),
&cArgs[0],
&argLengths[0],
cArgsPtr,
argLengthsPtr,
)
payload := <-resultChannel
if payload.error != nil {
Expand Down Expand Up @@ -160,23 +166,15 @@ func (client *baseClient) Get(key string) (string, error) {
}

func (client *baseClient) MSet(keyValueMap map[string]string) (string, error) {
flat := []string{}
for key, value := range keyValueMap {
flat = append(flat, key, value)
}
result, err := client.executeCommand(C.MSet, flat)
result, err := client.executeCommand(C.MSet, utils.MapToString(keyValueMap))
if err != nil {
return "", err
}
return handleStringResponse(result), nil
}

func (client *baseClient) MSetNX(keyValueMap map[string]string) (bool, error) {
flat := []string{}
for key, value := range keyValueMap {
flat = append(flat, key, value)
}
result, err := client.executeCommand(C.MSetNX, flat)
result, err := client.executeCommand(C.MSetNX, utils.MapToString(keyValueMap))
if err != nil {
return false, err
}
Expand Down
54 changes: 54 additions & 0 deletions go/api/glide_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package api
// #cgo LDFLAGS: -L../target/release -lglide_rs
// #include "../lib.h"
import "C"
import "github.com/valkey-io/valkey-glide/go/glide/utils"

// GlideClient is a client used for connection in Standalone mode.
type GlideClient struct {
Expand Down Expand Up @@ -41,3 +42,56 @@ func (client *GlideClient) CustomCommand(args []string) (interface{}, error) {
}
return handleStringOrNullResponse(res), nil
}

// Sets configuration parameters to the specified values.
//
// Note: Prior to Version 7.0.0, only one parameter can be send.
//
// Parameters:
//
// parameters - A map consisting of configuration parameters and their respective values to set.
//
// Return value:
//
// "OK" if all configurations have been successfully set. Otherwise, raises an error.
//
// For example:
//
// result, err := client.ConfigSet(map[string]string{"timeout": "1000", "maxmemory": "1GB"})
// result: "OK"
//
// [valkey.io]: https://valkey.io/commands/config-set/
func (client *GlideClient) ConfigSet(parameters map[string]string) (string, error) {
result, err := client.executeCommand(C.ConfigSet, utils.MapToString(parameters))
if err != nil {
return "", err
}
return handleStringResponse(result), nil
}

// Gets the values of configuration parameters.
//
// Note: Prior to Version 7.0.0, only one parameter can be send.
//
janhavigupta007 marked this conversation as resolved.
Show resolved Hide resolved
// Parameters:
//
// args - A slice of configuration parameter names to retrieve values for.
//
// Return value:
//
// A map of values corresponding to the configuration parameters.
//
// For example:
//
// result, err := client.ConfigGet([]string{"timeout" , "maxmemory"})
// result["timeout"] = "1000"
// result["maxmemory"] = "1GB"
//
// [valkey.io]: https://valkey.io/commands/config-get/
func (client *GlideClient) ConfigGet(args []string) (map[string]string, error) {
res, err := client.executeCommand(C.ConfigGet, args)
if err != nil {
return nil, err
}
return handleStringToStringMapResponse(res), nil
}
11 changes: 11 additions & 0 deletions go/api/response_handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,14 @@ func handleBooleanResponse(response *C.struct_CommandResponse) bool {
defer C.free_command_response(response)
return bool(response.bool_value)
}

func handleStringToStringMapResponse(response *C.struct_CommandResponse) map[string]string {
defer C.free_command_response(response)
m := make(map[string]string, response.array_value_len)
janhavigupta007 marked this conversation as resolved.
Show resolved Hide resolved
for _, v := range unsafe.Slice(response.array_value, response.array_value_len) {
key := convertCharArrayToString(v.map_key.string_value, v.map_key.string_value_len)
value := convertCharArrayToString(v.map_value.string_value, v.map_value.string_value_len)
m[key] = value
}
return m
}
6 changes: 3 additions & 3 deletions go/integTest/glide_test_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ type GlideTestSuite struct {
suite.Suite
standalonePorts []int
clusterPorts []int
redisVersion string
serverVersion string
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

clients []*api.GlideClient
clusterClients []*api.GlideClusterClient
}
Expand Down Expand Up @@ -55,8 +55,8 @@ func (suite *GlideTestSuite) SetupSuite() {
suite.T().Fatal(err.Error())
}

suite.redisVersion = extractServerVersion(string(byteOutput))
suite.T().Logf("Redis version = %s", suite.redisVersion)
suite.serverVersion = extractServerVersion(string(byteOutput))
janhavigupta007 marked this conversation as resolved.
Show resolved Hide resolved
suite.T().Logf("Detected server version = %s", suite.serverVersion)
}

func extractPorts(suite *GlideTestSuite, output string) []int {
Expand Down
48 changes: 48 additions & 0 deletions go/integTest/standalone_commands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,51 @@ func (suite *GlideTestSuite) TestCustomCommand_closedClient() {
assert.NotNil(suite.T(), err)
assert.IsType(suite.T(), &api.ClosingError{}, err)
}

func (suite *GlideTestSuite) TestConfigSetAndGet_multipleArgs() {
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"}
janhavigupta007 marked this conversation as resolved.
Show resolved Hide resolved
resultConfigMap := map[string]string{"timeout": "1000", "maxmemory": "1073741824"}
result, err := client.ConfigSet(configMap)
assert.Nil(suite.T(), err)
assert.Equal(suite.T(), result, "OK")

result2, err := client.ConfigGet([]string{"timeout", "maxmemory"})
assert.Nil(suite.T(), err)
assert.Equal(suite.T(), resultConfigMap, result2)
}

func (suite *GlideTestSuite) TestConfigSetAndGet_noArgs() {
client := suite.defaultClient()

configMap := map[string]string{}

result, err := client.ConfigSet(configMap)
assert.Equal(suite.T(), "", result)
assert.NotNil(suite.T(), err)
assert.IsType(suite.T(), &api.RequestError{}, err)

result2, err := client.ConfigGet([]string{})
assert.Nil(suite.T(), result2)
assert.NotNil(suite.T(), err)
assert.IsType(suite.T(), &api.RequestError{}, err)
}

func (suite *GlideTestSuite) TestConfigSetAndGet_invalidArgs() {
client := suite.defaultClient()

configMap := map[string]string{"time": "1000"}

result, err := client.ConfigSet(configMap)
assert.Equal(suite.T(), "", result)
assert.NotNil(suite.T(), err)
assert.IsType(suite.T(), &api.RequestError{}, err)

result2, err := client.ConfigGet([]string{"time"})
assert.Equal(suite.T(), map[string]string{}, result2)
assert.Nil(suite.T(), err)
}
46 changes: 46 additions & 0 deletions go/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,14 @@ pub struct CommandResponse {
#[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,
}

/// Success callback that is called when a command succeeds.
Expand Down Expand Up @@ -241,11 +249,17 @@ pub unsafe extern "C" fn free_command_response(command_response_ptr: *mut Comman
/// * The contained `string_value` must be valid until `free_command_response` is called and it must outlive the `CommandResponse` that contains it.
/// * The contained `array_value` must be obtained from the `CommandResponse` returned in [`SuccessCallback`] from [`command`].
/// * The contained `array_value` must be valid until `free_command_response` is called and it must outlive the `CommandResponse` that contains it.
/// * The contained `map_key` must be obtained from the `CommandResponse` returned in [`SuccessCallback`] from [`command`].
/// * The contained `map_key` must be valid until `free_command_response` is called and it must outlive the `CommandResponse` that contains it.
/// * The contained `map_value` must be obtained from the `CommandResponse` returned in [`SuccessCallback`] from [`command`].
/// * The contained `map_value` must be valid until `free_command_response` is called and it must outlive the `CommandResponse` that contains it.
fn free_command_response_elements(command_response: CommandResponse) {
let string_value = command_response.string_value;
let string_value_len = command_response.string_value_len;
let array_value = command_response.array_value;
let array_value_len = command_response.array_value_len;
let map_key = command_response.map_key;
let map_value = command_response.map_value;
if !string_value.is_null() {
let len = string_value_len as usize;
unsafe { Vec::from_raw_parts(string_value, len, len) };
Expand All @@ -257,9 +271,16 @@ fn free_command_response_elements(command_response: CommandResponse) {
free_command_response_elements(element);
}
}
if !map_key.is_null() {
unsafe { free_command_response(map_key) };
}
if !map_value.is_null() {
unsafe { free_command_response(map_value) };
}
}

/// Frees the error_message received on a command failure.
/// TODO: Add a test case to check for memory leak.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<3

///
/// # Panics
///
Expand Down Expand Up @@ -363,6 +384,31 @@ fn valkey_value_to_command_response(value: Value) -> RedisResult<Option<CommandR
command_response.array_value_len = len;
Ok(Some(command_response))
}
Value::Map(map) => {
let result: Vec<CommandResponse> = map
.into_iter()
.map(|(key, val)| {
let mut map_response = CommandResponse::default();

let map_key = valkey_value_to_command_response(key)
.expect("Value couldn't be converted to CommandResponse")
.unwrap_or_default();
map_response.map_key = Box::into_raw(Box::new(map_key));

let map_val = valkey_value_to_command_response(val)
.expect("Value couldn't be converted to CommandResponse")
.unwrap_or_default();
map_response.map_value = Box::into_raw(Box::new(map_val));

map_response
})
.collect::<Vec<_>>();

let (vec_ptr, len) = convert_vec_to_pointer(result);
command_response.array_value = vec_ptr;
command_response.array_value_len = len;
Ok(Some(command_response))
}
// TODO: Add support for other return types.
_ => todo!(),
};
Expand Down
9 changes: 9 additions & 0 deletions go/utils/transform_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,12 @@ func IntToString(value int64) string {
func FloatToString(value float64) string {
return strconv.FormatFloat(value, 'g', -1 /*precision*/, 64 /*bit*/)
}

// Flattens the Map: { (key1, value1), (key2, value2), ..} to a slice { key1, value1, key2, value2, ..}
func MapToString(parameter map[string]string) []string {
janhavigupta007 marked this conversation as resolved.
Show resolved Hide resolved
flat := make([]string, 0, len(parameter)*2)
for key, value := range parameter {
flat = append(flat, key, value)
}
return flat
}
Loading