Skip to content

Commit

Permalink
api: use msgpack/v5 instead of msgpack.v2
Browse files Browse the repository at this point in the history
We found several critical bugs in the library msgpack.v2. It was
decided to update the library to msgpack/v5.

Closes #211
Closes #236
  • Loading branch information
oleg-jukovec committed Jun 7, 2023
1 parent 3147387 commit 3391a10
Show file tree
Hide file tree
Showing 93 changed files with 555 additions and 1,087 deletions.
38 changes: 0 additions & 38 deletions .github/workflows/testing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -104,16 +104,6 @@ jobs:
make test TAGS="go_tarantool_call_17"
make testrace TAGS="go_tarantool_call_17"
- name: Run regression tests with msgpack.v5
run: |
make test TAGS="go_tarantool_msgpack_v5"
make testrace TAGS="go_tarantool_msgpack_v5"
- name: Run regression tests with msgpack.v5 and call_17
run: |
make test TAGS="go_tarantool_msgpack_v5,go_tarantool_call_17"
make testrace TAGS="go_tarantool_msgpack_v5,go_tarantool_call_17"
- name: Run fuzzing tests
if: ${{ matrix.fuzzing }}
run: make fuzzing TAGS="go_tarantool_decimal_fuzzing"
Expand Down Expand Up @@ -207,22 +197,6 @@ jobs:
env:
TEST_TNT_SSL: ${{matrix.ssl}}

- name: Run regression tests with msgpack.v5
run: |
source tarantool-enterprise/env.sh
make test TAGS="go_tarantool_msgpack_v5"
make testrace TAGS="go_tarantool_msgpack_v5"
env:
TEST_TNT_SSL: ${{matrix.ssl}}

- name: Run regression tests with msgpack.v5 and call_17
run: |
source tarantool-enterprise/env.sh
make test TAGS="go_tarantool_msgpack_v5,go_tarantool_call_17"
make testrace TAGS="go_tarantool_msgpack_v5,go_tarantool_call_17"
env:
TEST_TNT_SSL: ${{matrix.ssl}}

- name: Run fuzzing tests
if: ${{ matrix.fuzzing }}
run: make fuzzing TAGS="go_tarantool_decimal_fuzzing"
Expand Down Expand Up @@ -395,18 +369,6 @@ jobs:
make test TAGS="go_tarantool_call_17"
make testrace TAGS="go_tarantool_call_17"
- name: Run regression tests with msgpack.v5
run: |
cd "${SRCDIR}"
make test TAGS="go_tarantool_msgpack_v5"
make testrace TAGS="go_tarantool_msgpack_v5"
- name: Run regression tests with msgpack.v5 and call_17
run: |
cd "${SRCDIR}"
make test TAGS="go_tarantool_msgpack_v5,go_tarantool_call_17"
make testrace TAGS="go_tarantool_msgpack_v5,go_tarantool_call_17"
- name: Run fuzzing tests
if: ${{ matrix.fuzzing }}
run: |
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@ Versioning](http://semver.org/spec/v2.0.0.html) except to the first release.
### Changed

- connection_pool renamed to pool (#239)
- Use msgpack/v5 instead of msgpack.v2 (#236)

### Removed

- multi subpackage (#240)
- msgpack.v2 support (#236)

### Fixed

Expand Down
10 changes: 1 addition & 9 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,7 @@ This allows us to introduce new features without losing backward compatibility.
go_tarantool_call_17
```
**Note:** In future releases, `Call17` may be used as default `Call` behavior.
3. To replace usage of `msgpack.v2` with `msgpack.v5`, you can use the build
tag:
```
go_tarantool_msgpack_v5
```
**Note:** In future releases, `msgpack.v5` may be used by default. We recommend
to read [msgpack.v5 migration notes](#msgpackv5-migration) and try to
use msgpack.v5 before the changes.
4. To run fuzz tests with decimals, you can use the build tag:
3. To run fuzz tests with decimals, you can use the build tag:
```
go_tarantool_decimal_fuzzing
```
Expand Down
30 changes: 18 additions & 12 deletions box_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package tarantool
import (
"bytes"
"fmt"

"github.com/vmihailenco/msgpack/v5"
)

const errorExtID = 3
Expand Down Expand Up @@ -69,7 +71,7 @@ func (e *BoxError) Depth() int {
return depth
}

func decodeBoxError(d *decoder) (*BoxError, error) {
func decodeBoxError(d *msgpack.Decoder) (*BoxError, error) {
var l, larr, l1, l2 int
var errorStack []BoxError
var err error
Expand Down Expand Up @@ -169,15 +171,15 @@ func decodeBoxError(d *decoder) (*BoxError, error) {
return &errorStack[0], nil
}

func encodeBoxError(enc *encoder, boxError *BoxError) error {
func encodeBoxError(enc *msgpack.Encoder, boxError *BoxError) error {
if boxError == nil {
return fmt.Errorf("msgpack: unexpected nil BoxError on encode")
}

if err := enc.EncodeMapLen(1); err != nil {
return err
}
if err := encodeUint(enc, keyErrorStack); err != nil {
if err := enc.EncodeUint(keyErrorStack); err != nil {
return err
}

Expand All @@ -199,50 +201,50 @@ func encodeBoxError(enc *encoder, boxError *BoxError) error {
}
}

if err := encodeUint(enc, keyErrorType); err != nil {
if err := enc.EncodeUint(keyErrorType); err != nil {
return err
}
if err := enc.EncodeString(boxError.Type); err != nil {
return err
}

if err := encodeUint(enc, keyErrorFile); err != nil {
if err := enc.EncodeUint(keyErrorFile); err != nil {
return err
}
if err := enc.EncodeString(boxError.File); err != nil {
return err
}

if err := encodeUint(enc, keyErrorLine); err != nil {
if err := enc.EncodeUint(keyErrorLine); err != nil {
return err
}
if err := enc.EncodeUint64(boxError.Line); err != nil {
return err
}

if err := encodeUint(enc, keyErrorMessage); err != nil {
if err := enc.EncodeUint(keyErrorMessage); err != nil {
return err
}
if err := enc.EncodeString(boxError.Msg); err != nil {
return err
}

if err := encodeUint(enc, keyErrorErrno); err != nil {
if err := enc.EncodeUint(keyErrorErrno); err != nil {
return err
}
if err := enc.EncodeUint64(boxError.Errno); err != nil {
return err
}

if err := encodeUint(enc, keyErrorErrcode); err != nil {
if err := enc.EncodeUint(keyErrorErrcode); err != nil {
return err
}
if err := enc.EncodeUint64(boxError.Code); err != nil {
return err
}

if fieldsLen > 0 {
if err := encodeUint(enc, keyErrorFields); err != nil {
if err := enc.EncodeUint(keyErrorFields); err != nil {
return err
}

Expand Down Expand Up @@ -276,7 +278,7 @@ func (e *BoxError) UnmarshalMsgpack(b []byte) error {
}

buf := bytes.NewBuffer(b)
dec := newDecoder(buf)
dec := msgpack.NewDecoder(buf)

if val, err := decodeBoxError(dec); err != nil {
return err
Expand All @@ -290,10 +292,14 @@ func (e *BoxError) UnmarshalMsgpack(b []byte) error {
func (e *BoxError) MarshalMsgpack() ([]byte, error) {
var buf bytes.Buffer

enc := newEncoder(&buf)
enc := msgpack.NewEncoder(&buf)
if err := encodeBoxError(enc, e); err != nil {
return nil, err
}

return buf.Bytes(), nil
}

func init() {
msgpack.RegisterExt(errorExtID, (*BoxError)(nil))
}
19 changes: 10 additions & 9 deletions box_error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"testing"

"github.com/stretchr/testify/require"
"github.com/vmihailenco/msgpack/v5"

. "github.com/tarantool/go-tarantool/v2"
"github.com/tarantool/go-tarantool/v2/test_helpers"
)
Expand Down Expand Up @@ -217,7 +219,7 @@ type TupleBoxError struct {
val BoxError
}

func (t *TupleBoxError) EncodeMsgpack(e *encoder) error {
func (t *TupleBoxError) EncodeMsgpack(e *msgpack.Encoder) error {
if err := e.EncodeArrayLen(2); err != nil {
return err
}
Expand All @@ -229,7 +231,7 @@ func (t *TupleBoxError) EncodeMsgpack(e *encoder) error {
return e.Encode(&t.val)
}

func (t *TupleBoxError) DecodeMsgpack(d *decoder) error {
func (t *TupleBoxError) DecodeMsgpack(d *msgpack.Decoder) error {
var err error
var l int
if l, err = d.DecodeArrayLen(); err != nil {
Expand Down Expand Up @@ -278,11 +280,11 @@ var tupleCases = map[string]struct {
func TestErrorTypeMPEncodeDecode(t *testing.T) {
for name, testcase := range tupleCases {
t.Run(name, func(t *testing.T) {
buf, err := marshal(&testcase.tuple)
buf, err := msgpack.Marshal(&testcase.tuple)
require.Nil(t, err)

var res TupleBoxError
err = unmarshal(buf, &res)
err = msgpack.Unmarshal(buf, &res)
require.Nil(t, err)

require.Equal(t, testcase.tuple, res)
Expand All @@ -302,9 +304,9 @@ func TestErrorTypeEval(t *testing.T) {
require.Nil(t, err)
require.NotNil(t, resp.Data)
require.Equal(t, len(resp.Data), 1)
actual, ok := toBoxError(resp.Data[0])
actual, ok := resp.Data[0].(*BoxError)
require.Truef(t, ok, "Response data has valid type")
require.Equal(t, testcase.tuple.val, actual)
require.Equal(t, testcase.tuple.val, *actual)
})
}
}
Expand Down Expand Up @@ -440,14 +442,13 @@ func TestErrorTypeSelect(t *testing.T) {
tpl, ok := resp.Data[0].([]interface{})
require.Truef(t, ok, "Tuple has valid type")
require.Equal(t, testcase.tuple.pk, tpl[0])
var actual BoxError
actual, ok = toBoxError(tpl[1])
actual, ok := tpl[1].(*BoxError)
require.Truef(t, ok, "BoxError tuple field has valid type")
// In fact, CheckEqualBoxErrors does not check than File and Line
// of connector BoxError are equal to the Tarantool ones
// since they may differ between different Tarantool versions
// and editions.
test_helpers.CheckEqualBoxErrors(t, testcase.tuple.val, actual)
test_helpers.CheckEqualBoxErrors(t, testcase.tuple.val, *actual)
})
}
}
Expand Down
32 changes: 18 additions & 14 deletions client_tools.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
package tarantool

import (
"github.com/vmihailenco/msgpack/v5"
)

// IntKey is utility type for passing integer key to Select*, Update*,
// Delete* and GetTyped. It serializes to array with single integer element.
type IntKey struct {
I int
}

func (k IntKey) EncodeMsgpack(enc *encoder) error {
func (k IntKey) EncodeMsgpack(enc *msgpack.Encoder) error {
enc.EncodeArrayLen(1)
encodeInt(enc, int64(k.I))
enc.EncodeInt(int64(k.I))
return nil
}

Expand All @@ -19,9 +23,9 @@ type UintKey struct {
I uint
}

func (k UintKey) EncodeMsgpack(enc *encoder) error {
func (k UintKey) EncodeMsgpack(enc *msgpack.Encoder) error {
enc.EncodeArrayLen(1)
encodeUint(enc, uint64(k.I))
enc.EncodeUint(uint64(k.I))
return nil
}

Expand All @@ -31,7 +35,7 @@ type StringKey struct {
S string
}

func (k StringKey) EncodeMsgpack(enc *encoder) error {
func (k StringKey) EncodeMsgpack(enc *msgpack.Encoder) error {
enc.EncodeArrayLen(1)
enc.EncodeString(k.S)
return nil
Expand All @@ -43,10 +47,10 @@ type IntIntKey struct {
I1, I2 int
}

func (k IntIntKey) EncodeMsgpack(enc *encoder) error {
func (k IntIntKey) EncodeMsgpack(enc *msgpack.Encoder) error {
enc.EncodeArrayLen(2)
encodeInt(enc, int64(k.I1))
encodeInt(enc, int64(k.I2))
enc.EncodeInt(int64(k.I1))
enc.EncodeInt(int64(k.I2))
return nil
}

Expand All @@ -57,10 +61,10 @@ type Op struct {
Arg interface{}
}

func (o Op) EncodeMsgpack(enc *encoder) error {
func (o Op) EncodeMsgpack(enc *msgpack.Encoder) error {
enc.EncodeArrayLen(3)
enc.EncodeString(o.Op)
encodeInt(enc, int64(o.Field))
enc.EncodeInt(int64(o.Field))
return enc.Encode(o.Arg)
}

Expand Down Expand Up @@ -145,12 +149,12 @@ type OpSplice struct {
Replace string
}

func (o OpSplice) EncodeMsgpack(enc *encoder) error {
func (o OpSplice) EncodeMsgpack(enc *msgpack.Encoder) error {
enc.EncodeArrayLen(5)
enc.EncodeString(o.Op)
encodeInt(enc, int64(o.Field))
encodeInt(enc, int64(o.Pos))
encodeInt(enc, int64(o.Len))
enc.EncodeInt(int64(o.Field))
enc.EncodeInt(int64(o.Pos))
enc.EncodeInt(int64(o.Len))
enc.EncodeString(o.Replace)
return nil
}
Loading

0 comments on commit 3391a10

Please sign in to comment.