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

Handle int64 and uint64 in properties by wrapping it #4253

Merged
merged 1 commit into from
Aug 23, 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
108 changes: 100 additions & 8 deletions internal/entities/properties/properties.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,79 @@ package properties

import (
"fmt"
"strconv"
"strings"

"github.com/puzpuzpuz/xsync/v3"
"google.golang.org/protobuf/types/known/structpb"
)

// Property is a struct that holds a value. It's just a wrapper around an interface{}
// with typed getters and handling of an empty receiver
// Property is a struct that holds a value. It's just a wrapper around structpb.Value
// with typed getters and handling of a nil receiver
type Property struct {
value *structpb.Value
}

const (
typeInt64 = "int64"
typeUint64 = "uint64"
internalPrefix = "minder.internal."
typeKey = "minder.internal.type"
valueKey = "minder.internal.value"
)

func wrapKeyValue(key, value string) map[string]any {
return map[string]any{
typeKey: key,
valueKey: value,
}
}

func wrapInt64(value int64) map[string]any {
return wrapKeyValue(typeInt64, strconv.FormatInt(value, 10))
}

func wrapUint64(value uint64) map[string]any {
return wrapKeyValue(typeUint64, strconv.FormatUint(value, 10))
}

func unwrapTypedValue(value *structpb.Value, typ string) (string, error) {
structValue := value.GetStructValue()
if structValue == nil {
return "", fmt.Errorf("value is not a map")
}

mapValue := structValue.GetFields()
typeVal, ok := mapValue[typeKey]
if !ok {
return "", fmt.Errorf("type field not found")
}

if typeVal.GetStringValue() != typ {
return "", fmt.Errorf("value is not of type %s", typ)
}

valPayload, ok := mapValue[valueKey]
if !ok {
return "", fmt.Errorf("value field not found")
}

return valPayload.GetStringValue(), nil
}

// NewProperty creates a new Property with a given value
func NewProperty(value any) (*Property, error) {
val, err := structpb.NewValue(value)
var err error
var val *structpb.Value

switch v := value.(type) {
case int64:
value = wrapInt64(v)
case uint64:
value = wrapUint64(v)
}

val, err = structpb.NewValue(value)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -84,20 +143,49 @@ func (p *Property) AsInt64() (int64, error) {
if p == nil {
return 0, fmt.Errorf("property is nil")
}
fval, err := propertyValueAs[float64](p.value.AsInterface())

stringVal, err := unwrapTypedValue(p.value, typeInt64)
if err != nil {
return 0, fmt.Errorf("value is not of type int64: %w", err)
return 0, fmt.Errorf("failed to get int64 value: %w", err)
}
return int64(fval), nil
return strconv.ParseInt(stringVal, 10, 64)
}

// GetInt64 returns the int64 value, or 0 if the value is not an int64
func (p *Property) GetInt64() int64 {
if p == nil {
return 0
}
// TODO: This loses precision
return int64(p.value.GetNumberValue())
i64val, err := p.AsInt64()
if err != nil {
return 0
}
return i64val
}

// AsUint64 returns the uint64 value, or an error if the value is not an uint64
func (p *Property) AsUint64() (uint64, error) {
if p == nil {
return 0, fmt.Errorf("property is nil")
}

stringVal, err := unwrapTypedValue(p.value, typeUint64)
if err != nil {
return 0, fmt.Errorf("failed to get uint64 value: %w", err)
}
return strconv.ParseUint(stringVal, 10, 64)
}

// GetUint64 returns the uint64 value, or 0 if the value is not an uint64
func (p *Property) GetUint64() uint64 {
if p == nil {
return 0
}
u64val, err := p.AsUint64()
if err != nil {
return 0
}
return u64val
}

// RawValue returns the raw value as an any
Expand All @@ -118,6 +206,10 @@ func NewProperties(props map[string]any) (*Properties, error) {
propsMap := xsync.NewMapOf[string, Property](xsync.WithPresize(len(props)))

for key, value := range props {
if strings.HasPrefix(key, internalPrefix) {
return nil, fmt.Errorf("property key %s is reserved", key)
}

propVal, err := NewProperty(value)
if err != nil {
return nil, fmt.Errorf("failed to create property for key %s: %w", key, err)
Expand Down
169 changes: 168 additions & 1 deletion internal/entities/properties/properties_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
package properties

import (
"fmt"
"math"
"testing"

"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -179,6 +181,7 @@ func TestInt64Getters(t *testing.T) {
"id": 1,
"is_private": true,
"count": int64(5),
"large": int64(math.MaxInt64),
}

scenarios := []struct {
Expand Down Expand Up @@ -213,14 +216,25 @@ func TestInt64Getters(t *testing.T) {
{
name: "AsInt64 non-int64 property",
propName: "is_private",
errString: "value is not of type int64",
errString: "failed to get int64 value: value is not a map",
},
{
name: "GetInt64 non-int64 property",
propName: "is_private",
expValue: 0,
callGet: true,
},
{
name: "AsInt64 large property",
propName: "large",
expValue: math.MaxInt64,
},
{
name: "GetInt64 large property",
propName: "large",
expValue: math.MaxInt64,
callGet: true,
},
}

for _, s := range scenarios {
Expand Down Expand Up @@ -248,6 +262,94 @@ func TestInt64Getters(t *testing.T) {
}
}

func TestUint64Getters(t *testing.T) {
t.Parallel()

input := map[string]any{
"id": 1,
"is_private": true,
"count": uint64(5),
"large": uint64(math.MaxUint64),
}

scenarios := []struct {
name string
propName string
errString string
expValue uint64
callGet bool
}{
{
name: "AsUint64 known property",
propName: "count",
expValue: 5,
},
{
name: "GetUint64 known property",
propName: "count",
expValue: 5,
callGet: true,
},
{
name: "AsUint64 unknown property",
propName: "unknown",
errString: "property is nil",
},
{
name: "GetUint64 unknown property",
propName: "unknown",
expValue: 0,
callGet: true,
},
{
name: "AsUint64 non-uint64 property",
propName: "is_private",
errString: "failed to get uint64 value: value is not a map",
},
{
name: "GetUint64 non-uint64 property",
propName: "is_private",
expValue: 0,
callGet: true,
},
{
name: "AsUint64 large property",
propName: "large",
expValue: math.MaxUint64,
},
{
name: "GetUint64 large property",
propName: "large",
expValue: math.MaxUint64,
callGet: true,
},
}

for _, s := range scenarios {
t.Run(s.name, func(t *testing.T) {
t.Parallel()

props, err := NewProperties(input)
require.NoError(t, err)

p := props.GetProperty(s.propName)
if s.callGet {
got := p.GetUint64()
require.Equal(t, s.expValue, got)
} else {
got, err := p.AsUint64()
if s.errString == "" {
require.NoError(t, err)
require.Equal(t, s.expValue, got)
} else {
require.Error(t, err)
require.ErrorContains(t, err, s.errString)
}
}
})
}
}

func TestNewProperty(t *testing.T) {
t.Parallel()

Expand All @@ -269,6 +371,18 @@ func TestNewProperties(t *testing.T) {
p := props.GetProperty("test")
require.Nil(t, p)
})

t.Run("reserved key", func(t *testing.T) {
t.Parallel()

testKey := internalPrefix + "test"

props, err := NewProperties(map[string]any{
testKey: true,
})
require.Contains(t, err.Error(), fmt.Sprintf("property key %s is reserved", testKey))
require.Nil(t, props)
})
}

func TestNilReceiver(t *testing.T) {
Expand All @@ -283,3 +397,56 @@ func TestNilReceiver(t *testing.T) {
require.False(t, p.GetBool())
})
}

func TestUnwrapTypeErrors(t *testing.T) {
t.Parallel()

scenarios := []struct {
name string
value any
err string
}{
{
name: "no type field",
value: map[string]any{
valueKey: 1,
},
err: "type field not found",
},
{
name: "unexpected type value",
value: map[string]any{
typeKey: "unknown",
valueKey: 1,
},
err: "value is not of type",
},
{
name: "no value field",
value: map[string]any{
typeKey: typeInt64,
},
err: "value field not found",
},
{
name: "invalid value type",
value: map[string]any{
typeKey: typeInt64,
valueKey: 1,
},
err: "invalid syntax",
},
}

for _, s := range scenarios {
t.Run(s.name, func(t *testing.T) {
t.Parallel()

prop, err := NewProperty(s.value)
require.NoError(t, err)
// we test int64, but that's just a coincidence as it calls unwrapTypedValue internally
_, err = prop.AsInt64()
require.Contains(t, err.Error(), s.err)
})
}
}
Loading