Skip to content

Commit

Permalink
crud: more careful response process
Browse files Browse the repository at this point in the history
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 6, 2023
1 parent c33f351 commit 1f844f0
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 41 deletions.
60 changes: 32 additions & 28 deletions crud/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,14 @@ func msgpackIsArray(code byte) bool {

// DecodeMsgpack provides custom msgpack decoder.
func (r *Result) DecodeMsgpack(d *msgpack.Decoder) error {
var retErr error

arrLen, err := d.DecodeArrayLen()
if err != nil {
return err
}

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

Expand Down Expand Up @@ -130,27 +132,27 @@ 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
}
retErr = *crudErr
} else if code != msgpcode.Nil {
crudErr := newError(r.rowType)
if err := d.Decode(&crudErr); err != nil {
return err
}
retErr = *crudErr
} else {
if err := d.DecodeNil(); err != nil {
return err
}
}
}

Expand All @@ -175,18 +177,24 @@ func (r *NumberResult) DecodeMsgpack(d *msgpack.Decoder) error {
return err
}

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

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 +203,6 @@ func (r *NumberResult) DecodeMsgpack(d *msgpack.Decoder) error {
}
}

if crudErr != nil {
return crudErr
}

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

0 comments on commit 1f844f0

Please sign in to comment.