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

feat(datastore): Return Get, GetMulti, Put and PutMulti errors with enhanced details #7061

Merged
merged 9 commits into from
Dec 2, 2022
81 changes: 70 additions & 11 deletions datastore/datastore.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,8 @@ var (
ErrInvalidKey = errors.New("datastore: invalid key")
// ErrNoSuchEntity is returned when no entity was found for a given key.
ErrNoSuchEntity = errors.New("datastore: no such entity")
// ErrDifferentKeyAndDstLength is returned when the length of dst and key are different.
ErrDifferentKeyAndDstLength = errors.New("datastore: keys and dst slices have different length")
)

type multiArgType int
Expand Down Expand Up @@ -350,7 +352,7 @@ func (c *Client) Get(ctx context.Context, key *Key, dst interface{}) (err error)
defer func() { trace.EndSpan(ctx, err) }()

if dst == nil { // get catches nil interfaces; we need to catch nil ptr here
return ErrInvalidEntityType
return fmt.Errorf("%w: dst cannot be nil", ErrInvalidEntityType)
}

var opts *pb.ReadOptions
Expand Down Expand Up @@ -401,15 +403,42 @@ func (c *Client) GetMulti(ctx context.Context, keys []*Key, dst interface{}) (er

func (c *Client) get(ctx context.Context, keys []*Key, dst interface{}, opts *pb.ReadOptions) error {
v := reflect.ValueOf(dst)
multiArgType, _ := checkMultiArg(v)

// Confidence checks
if multiArgType == multiArgTypeInvalid {
return errors.New("datastore: dst has invalid type")
var multiArgType multiArgType

// If kind is of type slice, return error
if kind := v.Kind(); kind != reflect.Slice {
return fmt.Errorf("%w: dst: expected slice got %v", ErrInvalidEntityType, kind.String())
}
if len(keys) != v.Len() {
return errors.New("datastore: keys and dst slices have different length")

// if type is a type which implements PropertyList, return error
if argType := v.Type(); argType == typeOfPropertyList {
return fmt.Errorf("%w: dst: cannot be PropertyListType", ErrInvalidEntityType)
}

elemType := v.Type().Elem()
if reflect.PtrTo(elemType).Implements(typeOfPropertyLoadSaver) {
multiArgType = multiArgTypePropertyLoadSaver
}

switch elemType.Kind() {
case reflect.Struct:
multiArgType = multiArgTypeStruct
case reflect.Interface:
multiArgType = multiArgTypeInterface
case reflect.Ptr:
elemType = elemType.Elem()
if elemType.Kind() == reflect.Struct {
multiArgType = multiArgTypeStructPtr
}
}

dstLen := v.Len()

if keysLen := len(keys); keysLen != dstLen {
return fmt.Errorf("%w: key length = %d, dst length = %d", ErrDifferentKeyAndDstLength, keysLen, v.Len())
}

if len(keys) == 0 {
return nil
}
Expand Down Expand Up @@ -563,13 +592,43 @@ func (c *Client) PutMulti(ctx context.Context, keys []*Key, src interface{}) (re

func putMutations(keys []*Key, src interface{}) ([]*pb.Mutation, error) {
v := reflect.ValueOf(src)
multiArgType, _ := checkMultiArg(v)
var multiArgType multiArgType

// If kind is of type slice, return error
if kind := v.Kind(); kind != reflect.Slice {
return nil, fmt.Errorf("%w: dst: expected slice got %v", ErrInvalidEntityType, kind.String())
}

// if type is a type which implements PropertyList, return error
if argType := v.Type(); argType == typeOfPropertyList {
return nil, fmt.Errorf("%w: dst: cannot be PropertyListType", ErrInvalidEntityType)
}

elemType := v.Type().Elem()
if reflect.PtrTo(elemType).Implements(typeOfPropertyLoadSaver) {
multiArgType = multiArgTypePropertyLoadSaver
}

switch elemType.Kind() {
case reflect.Struct:
multiArgType = multiArgTypeStruct
case reflect.Interface:
multiArgType = multiArgTypeInterface
case reflect.Ptr:
elemType = elemType.Elem()
if elemType.Kind() == reflect.Struct {
multiArgType = multiArgTypeStructPtr
}
}
if multiArgType == multiArgTypeInvalid {
return nil, errors.New("datastore: src has invalid type")
return nil, fmt.Errorf("datastore: src has invalid type")
}
if len(keys) != v.Len() {
return nil, errors.New("datastore: key and src slices have different length")
dstLen := v.Len()

if keysLen := len(keys); keysLen != dstLen {
return nil, fmt.Errorf("%w: key length = %d, dst length = %d", ErrDifferentKeyAndDstLength, keysLen, v.Len())
}

if len(keys) == 0 {
return nil, nil
}
Expand Down
14 changes: 14 additions & 0 deletions datastore/datastore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,20 @@ func TestPutMultiTypes(t *testing.T) {
src: S{1, "one"},
wantErr: true,
},
{
desc: "slice and key length is different",
src: []interface{}{
S{1, "one"},
S{2, "two"},
S{3, "three"},
},
wantErr: true,
},
{
desc: "slice length is 0, return error",
src: []interface{}{},
wantErr: true,
},
}

// Use the same keys and expected entities for all tests.
Expand Down