Skip to content

Commit

Permalink
Fixed code review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Niharika Bhavaraju <[email protected]>
  • Loading branch information
niharikabhavaraju committed Jan 16, 2025
1 parent 42bebc8 commit 41d9231
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 47 deletions.
10 changes: 5 additions & 5 deletions go/api/base_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -1629,15 +1629,15 @@ func (client *baseClient) SetBit(key string, offset int64, value int64) (Result[
if err != nil {
return CreateNilInt64Result(), err
}
return handleLongResponse(result)
return handleIntOrNilResponse(result)
}

func (client *baseClient) GetBit(key string, offset int64) (Result[int64], error) {
result, err := client.executeCommand(C.GetBit, []string{key, utils.IntToString(offset)})
if err != nil {
return CreateNilInt64Result(), err
}
return handleLongResponse(result)
return handleIntOrNilResponse(result)
}

func (client *baseClient) Wait(numberOfReplicas int64, timeout int64) (Result[int64], error) {
Expand All @@ -1651,15 +1651,15 @@ func (client *baseClient) Wait(numberOfReplicas int64, timeout int64) (Result[in
if err != nil {
return CreateNilInt64Result(), err
}
return handleLongResponse(result)
return handleIntOrNilResponse(result)
}

func (client *baseClient) BitCount(key string) (Result[int64], error) {
result, err := client.executeCommand(C.BitCount, []string{key})
if err != nil {
return CreateNilInt64Result(), err
}
return handleLongResponse(result)
return handleIntOrNilResponse(result)
}

func (client *baseClient) BitCountWithOptions(key string, opts *options.BitCountOptions) (Result[int64], error) {
Expand All @@ -1672,5 +1672,5 @@ func (client *baseClient) BitCountWithOptions(key string, opts *options.BitCount
if err != nil {
return CreateNilInt64Result(), err
}
return handleLongResponse(result)
return handleIntOrNilResponse(result)
}
10 changes: 5 additions & 5 deletions go/api/bitmap_commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import "github.com/valkey-io/valkey-glide/go/glide/api/options"
// [valkey.io]: https://valkey.io/commands/#bitmap
type BitmapCommands interface {
// Sets or clears the bit at offset in the string value stored at key.
// The offset is a zero-based index, with `0`` being the first element of
// The offset is a zero-based index, with `0` being the first element of
// the list, `1` being the next element, and so on. The offset must be
// less than `2^32` and greater than or equal to `0` If a key is
// non-existent then the bit at offset is set to value and the preceding
Expand All @@ -26,7 +26,7 @@ type BitmapCommands interface {
// The bit value that was previously stored at offset.
//
// Example:
// result, err := client.SetBit("key",1 , 1)
// result, err := client.SetBit("key", 1, 1)
// result.Value(): 1
// result.IsNil(): false
//
Expand All @@ -45,7 +45,7 @@ type BitmapCommands interface {
// offset exceeds the length of the string.
//
// Example:
// result, err := client.GetBit("key1",1,1)
// result, err := client.GetBit("key1", 1, 1)
// result.Value(): 1
// result.IsNil(): false
//
Expand Down Expand Up @@ -78,8 +78,8 @@ type BitmapCommands interface {
// Parameters:
// key - The key for the string to count the set bits of.
// options - Start is the starting offset and end is the ending offset. BitmapIndexType
// is The index offset type. Could be either {@link BitmapIndexType#BIT} or
// {@link BitmapIndexType#BYTE}.
// is The index offset type. Could be either BitmapIndexType.BIT or
// BitmapIndexType.BYTE
//
// Return value:
// The number of set bits in the string interval specified by start, end, and options.
Expand Down
2 changes: 1 addition & 1 deletion go/api/generic_base_commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ type GenericBaseCommands interface {
//
// Parameters:
// numberOfReplicas - The number of replicas to reach.
// timeout - The timeout value specified in milliseconds. A value of 0 will
// timeout - The timeout value specified in milliseconds. A value of `0` will
// block indefinitely.
//
// Return value:
Expand Down
29 changes: 11 additions & 18 deletions go/api/options/bitcount_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
package options

import (
"fmt"

"github.com/valkey-io/valkey-glide/go/glide/utils"
)

Expand All @@ -22,33 +20,26 @@ type BitCountOptions struct {
BitMapIndexType BitmapIndexType
}

func NewBitCountOptionsBuilder() *BitCountOptions {
return &BitCountOptions{}
}

// SetStart defines start byte to calculate bitcount in bitcount command.
func (options *BitCountOptions) SetStart(start int64) *BitCountOptions {
options.Start = &start
return options
}

// SetEnd defines start byte to calculate bitcount in bitcount command.
func (options *BitCountOptions) SetEnd(end int64) (*BitCountOptions, error) {
if options.Start == nil {
return options, fmt.Errorf("End value cannot be set without start value, use SetStart()")
}
func (options *BitCountOptions) SetEnd(end int64) *BitCountOptions {
options.End = &end
return options, nil
return options
}

// SetBitmapIndexType to specify start and end are in BYTE or BIT
func (options *BitCountOptions) SetBitmapIndexType(bitMapIndexType BitmapIndexType) (*BitCountOptions, error) {
if options.Start == nil || options.End == nil {
return options, fmt.Errorf(
"SetBitmapIndexType value cannot be set without start and end values, use SetStart() and SetEnd()",
)
}
if bitMapIndexType != BIT && bitMapIndexType != BYTE {
return options, fmt.Errorf("Invalid argument: BIT and BYTE are the only allowed values")
}
func (options *BitCountOptions) SetBitmapIndexType(bitMapIndexType BitmapIndexType) *BitCountOptions {
options.BitMapIndexType = bitMapIndexType
return options, nil
return options
}

// ToArgs converts the options to a list of arguments.
Expand All @@ -61,7 +52,9 @@ func (opts *BitCountOptions) ToArgs() ([]string, error) {
if opts.End != nil {
args = append(args, utils.IntToString(*opts.End))
if opts.BitMapIndexType != "" {
args = append(args, string(opts.BitMapIndexType))
if opts.BitMapIndexType == BIT || opts.BitMapIndexType == BYTE {
args = append(args, string(opts.BitMapIndexType))
}
}
}
}
Expand Down
39 changes: 21 additions & 18 deletions go/integTest/shared_commands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4857,9 +4857,24 @@ func (suite *GlideTestSuite) TestWait() {
suite.runWithDefaultClients(func(client api.BaseClient) {
key := uuid.New().String()
client.Set(key, "test")
// Test 1: numberOfReplicas (2)
resultInt64, err := client.Wait(2, 2000)
assert.Nil(suite.T(), err)
assert.Equal(suite.T(), int64(2), resultInt64.Value())
assert.True(suite.T(), resultInt64.Value() >= 2)

// Test 2: Invalid numberOfReplicas (0)
resultInt64, err = client.Wait(0, 2000)

// Assert error and message for invalid number of replicas
assert.NotNil(suite.T(), err)
assert.Equal(suite.T(), "Number of Replicas should be greater than 0", err.Error())

// Test 3: Invalid timeout (negative)
resultInt64, err = client.Wait(2, -1)

// Assert error and message for invalid timeout
assert.NotNil(suite.T(), err)
assert.Equal(suite.T(), "Timeout cannot be lesser than 0", err.Error())
})
}

Expand Down Expand Up @@ -4932,14 +4947,10 @@ func (suite *GlideTestSuite) TestBitCountWithOptions_StartEnd() {
end := int64(5)
opts := &options.BitCountOptions{}
opts.SetStart(start)
opts, err := opts.SetEnd(end)
assert.Nil(suite.T(), err)
opts.SetEnd(end)

result, err := client.BitCountWithOptions(key, opts)
assert.Nil(suite.T(), err)

fmt.Println("Bit count from 1 to 5:", result.Value())

assert.Equal(suite.T(), int64(19), result.Value())
})
}
Expand All @@ -4955,15 +4966,11 @@ func (suite *GlideTestSuite) TestBitCountWithOptions_StartEndByte() {
end := int64(5)
opts := &options.BitCountOptions{}
opts.SetStart(start)
opts, err := opts.SetEnd(end)
opts, err = opts.SetBitmapIndexType(options.BYTE)
assert.Nil(suite.T(), err)
opts.SetEnd(end)
opts.SetBitmapIndexType(options.BYTE)

result, err := client.BitCountWithOptions(key, opts)
assert.Nil(suite.T(), err)

fmt.Println("Bit count from 1 to 5:", result.Value())

assert.Equal(suite.T(), int64(19), result.Value())
})
}
Expand All @@ -4979,15 +4986,11 @@ func (suite *GlideTestSuite) TestBitCountWithOptions_StartEndBit() {
end := int64(5)
opts := &options.BitCountOptions{}
opts.SetStart(start)
opts, err := opts.SetEnd(end)
opts, err = opts.SetBitmapIndexType(options.BIT)
assert.Nil(suite.T(), err)
opts.SetEnd(end)
opts.SetBitmapIndexType(options.BIT)

result, err := client.BitCountWithOptions(key, opts)
assert.Nil(suite.T(), err)

fmt.Println("Bit count from 1 to 5:", result.Value())

assert.Equal(suite.T(), int64(3), result.Value())
})
}

0 comments on commit 41d9231

Please sign in to comment.