From 1f844f0c82aab458da6e28aa43bf9bf98677b25c Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Fri, 6 Oct 2023 13:03:03 +0300 Subject: [PATCH] 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. https://github.com/tarantool/tarantool-python/commit/a4b734aa46facc89fd967de1e74f9768341e56fc --- crud/result.go | 60 ++++++++++++++++++++++-------------------- crud/tarantool_test.go | 26 +++++++++--------- 2 files changed, 45 insertions(+), 41 deletions(-) diff --git a/crud/result.go b/crud/result.go index e65b3e55e..d08c7c41e 100644 --- a/crud/result.go +++ b/crud/result.go @@ -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) } @@ -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 + } } } @@ -175,7 +177,7 @@ 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) } @@ -183,10 +185,16 @@ func (r *NumberResult) DecodeMsgpack(d *msgpack.Decoder) error { 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++ { @@ -195,10 +203,6 @@ func (r *NumberResult) DecodeMsgpack(d *msgpack.Decoder) error { } } - if crudErr != nil { - return crudErr - } - return nil } diff --git a/crud/tarantool_test.go b/crud/tarantool_test.go index 457cb2aa0..88572b56f 100644 --- a/crud/tarantool_test.go +++ b/crud/tarantool_test.go @@ -254,7 +254,7 @@ var testGenerateDataCases = []struct { }{ { "Insert", - 2, + 1, 1, crud.MakeInsertRequest(spaceName). Tuple(tuple). @@ -262,7 +262,7 @@ var testGenerateDataCases = []struct { }, { "InsertObject", - 2, + 1, 1, crud.MakeInsertObjectRequest(spaceName). Object(object). @@ -270,7 +270,7 @@ var testGenerateDataCases = []struct { }, { "InsertMany", - 2, + 1, 10, crud.MakeInsertManyRequest(spaceName). Tuples(tuples). @@ -278,7 +278,7 @@ var testGenerateDataCases = []struct { }, { "InsertObjectMany", - 2, + 1, 10, crud.MakeInsertObjectManyRequest(spaceName). Objects(objects). @@ -286,7 +286,7 @@ var testGenerateDataCases = []struct { }, { "Replace", - 2, + 1, 1, crud.MakeReplaceRequest(spaceName). Tuple(tuple). @@ -294,7 +294,7 @@ var testGenerateDataCases = []struct { }, { "ReplaceObject", - 2, + 1, 1, crud.MakeReplaceObjectRequest(spaceName). Object(object). @@ -302,7 +302,7 @@ var testGenerateDataCases = []struct { }, { "ReplaceMany", - 2, + 1, 10, crud.MakeReplaceManyRequest(spaceName). Tuples(tuples). @@ -310,7 +310,7 @@ var testGenerateDataCases = []struct { }, { "ReplaceObjectMany", - 2, + 1, 10, crud.MakeReplaceObjectManyRequest(spaceName). Objects(objects). @@ -318,7 +318,7 @@ var testGenerateDataCases = []struct { }, { "Upsert", - 2, + 1, 1, crud.MakeUpsertRequest(spaceName). Tuple(tuple). @@ -327,7 +327,7 @@ var testGenerateDataCases = []struct { }, { "UpsertObject", - 2, + 1, 1, crud.MakeUpsertObjectRequest(spaceName). Object(object). @@ -336,7 +336,7 @@ var testGenerateDataCases = []struct { }, { "UpsertMany", - 2, + 1, 10, crud.MakeUpsertManyRequest(spaceName). TuplesOperationsData(tuplesOperationsData). @@ -344,7 +344,7 @@ var testGenerateDataCases = []struct { }, { "UpsertObjectMany", - 2, + 1, 10, crud.MakeUpsertObjectManyRequest(spaceName). ObjectsOperationsData(objectsOperationData). @@ -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 {