Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
crud: more careful response process
In go-tarantool, crud API result decoding assumes that there are
always two response values, except for truncate. Such approach is based
on actual experience: success result is always `return res, nil`,
not `return res`. But this is not a feature guaranteed by crud
module API, just a current implementation quirk [1]
(since in Lua function result process there is no any difference).
See also similar patch in tarantool/python [2].

1. https://github.com/tarantool/crud/blob/53457477974fed42351cbd87f566d11e9f7e39bb/crud/common/schema.lua#L88
2. tarantool/tarantool-python@a4b734a
  • Loading branch information
DifferentialOrange committed Oct 9, 2023
commit be8ba74f33278bcb500e67f0b03e8e6dd08229e9
96 changes: 54 additions & 42 deletions crud/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ func (r *Result) DecodeMsgpack(d *msgpack.Decoder) error {
return err
}

if arrLen < 2 {
return fmt.Errorf("array len doesn't match: %d", arrLen)
if arrLen == 0 {
return fmt.Errorf("unexpected empty response array")
}

l, err := d.DecodeMapLen()
Expand Down Expand Up @@ -130,27 +130,32 @@ func (r *Result) DecodeMsgpack(d *msgpack.Decoder) error {
}
}

code, err := d.PeekCode()
if err != nil {
return err
}

var retErr error
if msgpackIsArray(code) {
crudErr := newErrorMany(r.rowType)
if err := d.Decode(&crudErr); err != nil {
return err
}
retErr = *crudErr
} else if code != msgpcode.Nil {
crudErr := newError(r.rowType)
if err := d.Decode(&crudErr); err != nil {
if arrLen > 1 {
code, err := d.PeekCode()
if err != nil {
return err
}
retErr = *crudErr
} else {
if err := d.DecodeNil(); err != nil {
return err

if msgpackIsArray(code) {
crudErr := newErrorMany(r.rowType)
if err := d.Decode(&crudErr); err != nil {
return err
}
if crudErr != nil {
return *crudErr
}
} else if code != msgpcode.Nil {
crudErr := newError(r.rowType)
if err := d.Decode(&crudErr); err != nil {
return err
}
if crudErr != nil {
return *crudErr
}
} else {
if err := d.DecodeNil(); err != nil {
return err
}
}
}

Expand All @@ -160,7 +165,7 @@ func (r *Result) DecodeMsgpack(d *msgpack.Decoder) error {
}
}

return retErr
return nil
}

// NumberResult describes CRUD result as an object containing number.
Expand All @@ -175,18 +180,24 @@ func (r *NumberResult) DecodeMsgpack(d *msgpack.Decoder) error {
return err
}

if arrLen < 2 {
return fmt.Errorf("array len doesn't match: %d", arrLen)
if arrLen == 0 {
return fmt.Errorf("unexpected empty response array")
}

if r.Value, err = d.DecodeUint64(); err != nil {
return err
}

var crudErr *Error = nil
if arrLen > 1 {
var crudErr *Error = nil

if err := d.Decode(&crudErr); err != nil {
return err
if err := d.Decode(&crudErr); err != nil {
return err
}

if crudErr != nil {
return crudErr
}
}

for i := 2; i < arrLen; i++ {
Expand All @@ -195,10 +206,6 @@ func (r *NumberResult) DecodeMsgpack(d *msgpack.Decoder) error {
}
}

if crudErr != nil {
return crudErr
}

return nil
}

Expand All @@ -213,26 +220,31 @@ func (r *BoolResult) DecodeMsgpack(d *msgpack.Decoder) error {
if err != nil {
return err
}
if arrLen < 2 {
if r.Value, err = d.DecodeBool(); err != nil {
return err
}

return nil
if arrLen == 0 {
return fmt.Errorf("unexpected empty response array")
}

if _, err = d.DecodeInterface(); err != nil {
if r.Value, err = d.DecodeBool(); err != nil {
return err
}

var crudErr *Error = nil
if arrLen > 1 {
var crudErr *Error = nil

if err := d.Decode(&crudErr); err != nil {
return err
if err := d.Decode(&crudErr); err != nil {
return err
}

if crudErr != nil {
return crudErr
}
}

if crudErr != nil {
return crudErr
for i := 2; i < arrLen; i++ {
if err := d.Skip(); err != nil {
return err
}
}

return nil
Expand Down
26 changes: 13 additions & 13 deletions crud/tarantool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,71 +254,71 @@ var testGenerateDataCases = []struct {
}{
{
"Insert",
2,
1,
1,
crud.MakeInsertRequest(spaceName).
Tuple(tuple).
Opts(simpleOperationOpts),
},
{
"InsertObject",
2,
1,
1,
crud.MakeInsertObjectRequest(spaceName).
Object(object).
Opts(simpleOperationObjectOpts),
},
{
"InsertMany",
2,
1,
10,
crud.MakeInsertManyRequest(spaceName).
Tuples(tuples).
Opts(opManyOpts),
},
{
"InsertObjectMany",
2,
1,
10,
crud.MakeInsertObjectManyRequest(spaceName).
Objects(objects).
Opts(opObjManyOpts),
},
{
"Replace",
2,
1,
1,
crud.MakeReplaceRequest(spaceName).
Tuple(tuple).
Opts(simpleOperationOpts),
},
{
"ReplaceObject",
2,
1,
1,
crud.MakeReplaceObjectRequest(spaceName).
Object(object).
Opts(simpleOperationObjectOpts),
},
{
"ReplaceMany",
2,
1,
10,
crud.MakeReplaceManyRequest(spaceName).
Tuples(tuples).
Opts(opManyOpts),
},
{
"ReplaceObjectMany",
2,
1,
10,
crud.MakeReplaceObjectManyRequest(spaceName).
Objects(objects).
Opts(opObjManyOpts),
},
{
"Upsert",
2,
1,
1,
crud.MakeUpsertRequest(spaceName).
Tuple(tuple).
Expand All @@ -327,7 +327,7 @@ var testGenerateDataCases = []struct {
},
{
"UpsertObject",
2,
1,
1,
crud.MakeUpsertObjectRequest(spaceName).
Object(object).
Expand All @@ -336,15 +336,15 @@ var testGenerateDataCases = []struct {
},
{
"UpsertMany",
2,
1,
10,
crud.MakeUpsertManyRequest(spaceName).
TuplesOperationsData(tuplesOperationsData).
Opts(opManyOpts),
},
{
"UpsertObjectMany",
2,
1,
10,
crud.MakeUpsertObjectManyRequest(spaceName).
ObjectsOperationsData(objectsOperationData).
Expand Down Expand Up @@ -475,7 +475,7 @@ func testCrudRequestCheck(t *testing.T, req tarantool.Request,

// resp.Data[0] - CRUD res.
// resp.Data[1] - CRUD err.
if expectedLen >= 2 {
if expectedLen >= 2 && resp.Data[1] != nil {
if crudErr, err := getCrudError(req, resp.Data[1]); err != nil {
t.Fatalf("Failed to get CRUD error: %#v", err)
} else if crudErr != nil {
Expand Down