Switch to MarshalMsgpack/UnmarshalMsgpack [TO SQUASH]

Author: ligurio

decimal/decimal.go

@@ -17,7 +17,6 @@ package decimal
 
 import (
 	"fmt"
-	"reflect"
 
 	"github.com/shopspring/decimal"
 	"gopkg.in/vmihailenco/msgpack.v2"
@@ -38,28 +37,30 @@ const (
 	DecimalPrecision = 38
 )
 
-func encodeDecimal(e *msgpack.Encoder, v reflect.Value) error {
-	number := v.Interface().(decimal.Decimal)
+type DecNumber struct {
+	decimal.Decimal
+}
+
+var _ msgpack.Marshaler = (*DecNumber)(nil)
+var _ msgpack.Unmarshaler = (*DecNumber)(nil)
+
+func (decNum *DecNumber) MarshalMsgpack() ([]byte, error) {
 	one := decimal.NewFromInt(1)
 	maxSupportedDecimal := decimal.New(1, DecimalPrecision).Sub(one) // 10^DecimalPrecision - 1
 	minSupportedDecimal := maxSupportedDecimal.Neg().Sub(one)        // -10^DecimalPrecision - 1
-	if number.GreaterThan(maxSupportedDecimal) {
-		return fmt.Errorf("msgpack: decimal number is bigger than maximum supported number (10^%d - 1)", DecimalPrecision)
+	if decNum.GreaterThan(maxSupportedDecimal) {
+		return nil, fmt.Errorf("msgpack: decimal number is bigger than maximum supported number (10^%d - 1)", DecimalPrecision)
 	}
-	if number.LessThan(minSupportedDecimal) {
-		return fmt.Errorf("msgpack: decimal number is lesser than minimum supported number (-10^%d - 1)", DecimalPrecision)
+	if decNum.LessThan(minSupportedDecimal) {
+		return nil, fmt.Errorf("msgpack: decimal number is lesser than minimum supported number (-10^%d - 1)", DecimalPrecision)
 	}
 
-	strBuf := number.String()
+	strBuf := decNum.String()
 	bcdBuf, err := EncodeStringToBCD(strBuf)
 	if err != nil {
-		return fmt.Errorf("msgpack: can't encode string (%s) to a BCD buffer: %w", strBuf, err)
+		return nil, fmt.Errorf("msgpack: can't encode string (%s) to a BCD buffer: %w", strBuf, err)
 	}
-	if _, err = e.Writer().Write(bcdBuf); err != nil {
-		return fmt.Errorf("msgpack: can't write bytes to encoder writer: %w", err)
-	}
-
-	return nil
+	return bcdBuf, nil
 }
 
 // Decimal values can be encoded to fixext MessagePack, where buffer
@@ -70,42 +71,20 @@ func encodeDecimal(e *msgpack.Encoder, v reflect.Value) error {
 // +--------+-------------------+------------+===============+
 // | MP_EXT | length (optional) | MP_DECIMAL | PackedDecimal |
 // +--------+-------------------+------------+===============+
-//
-// Before reading a buffer with encoded decimal number (PackedDecimal) we need
-// to allocate it, but before reading we don't know it's exact size.
-// msgpack.Decoder in msgpack v2 package pass a buffer with PackedDecimal bytes
-// and there is no possibility to read length in advance. For example on
-// attempt to decode MessagePack buffer c7030100088c (88) msgpack pass 00088a
-// to decodeDecimal() and length field value (03) is unavailable nor in buffer
-// nor via decoder interface.
-//
-// To solve a problem we allocate a buffer with maximum size (it is 32 bytes
-// and it is corresponds to ext32 MessagePack), then read encoded decimal to a
-// buffer and finally cut off unused part.
-func decodeDecimal(d *msgpack.Decoder, v reflect.Value) error {
-	var maxBytesCount int = 32
-	b := make([]byte, maxBytesCount)
-
-	readBytes, err := d.Buffered().Read(b)
-	if err != nil {
-		return fmt.Errorf("msgpack: can't read bytes on decimal decode: %w", err)
-	}
-	b = b[:readBytes]
+func (decNum *DecNumber) UnmarshalMsgpack(b []byte) error {
 	digits, err := DecodeStringFromBCD(b)
 	if err != nil {
 		return fmt.Errorf("msgpack: can't decode string from BCD buffer (%x): %w", b, err)
 	}
 	dec, err := decimal.NewFromString(digits)
+	*decNum = DecNumber{dec}
 	if err != nil {
 		return fmt.Errorf("msgpack: can't encode string (%s) to a decimal number: %w", digits, err)
 	}
 
-	v.Set(reflect.ValueOf(dec))
-
 	return nil
 }
 
 func init() {
-	msgpack.Register(reflect.TypeOf((*decimal.Decimal)(nil)).Elem(), encodeDecimal, decodeDecimal)
-	msgpack.RegisterExt(decimalExtID, (*decimal.Decimal)(nil))
+	msgpack.RegisterExt(decimalExtID, &DecNumber{})
 }

decimal/decimal_test.go

@@ -40,14 +40,14 @@ var space = "testDecimal"
 var index = "primary"
 
 type TupleDecimal struct {
-	number decimal.Decimal
+	number DecNumber
 }
 
 func (t *TupleDecimal) EncodeMsgpack(e *msgpack.Encoder) error {
 	if err := e.EncodeSliceLen(1); err != nil {
 		return err
 	}
-	return e.EncodeValue(reflect.ValueOf(t.number))
+	return e.EncodeValue(reflect.ValueOf(&t.number))
 }
 
 func (t *TupleDecimal) DecodeMsgpack(d *msgpack.Decoder) error {
@@ -64,7 +64,7 @@ func (t *TupleDecimal) DecodeMsgpack(d *msgpack.Decoder) error {
 	if err != nil {
 		return err
 	}
-	t.number = res.(decimal.Decimal)
+	t.number = res.(DecNumber)
 
 	return nil
 }
@@ -134,15 +134,17 @@ func TestMPEncodeDecode(t *testing.T) {
 				t.Fatal(err)
 			}
 			var buf []byte
-			tuple := TupleDecimal{number: decNum}
+			tuple := TupleDecimal{number: DecNumber{decNum}}
 			if buf, err = msgpack.Marshal(&tuple); err != nil {
 				t.Fatalf("Failed to encode decimal number '%s' to a MessagePack buffer: %s", testcase.numString, err)
 			}
 			var v TupleDecimal
 			if err = msgpack.Unmarshal(buf, &v); err != nil {
 				t.Fatalf("Failed to decode MessagePack buffer '%x' to a decimal number: %s", buf, err)
 			}
-			if !decNum.Equal(v.number) {
+			if !decNum.Equal(v.number.Decimal) {
+				fmt.Println(decNum)
+				fmt.Println(v.number)
 				t.Fatal("Decimal numbers are not equal")
 			}
 		})
@@ -235,7 +237,7 @@ func TestEncodeStringToBCDIncorrectNumber(t *testing.T) {
 func TestEncodeMaxNumber(t *testing.T) {
 	referenceErrMsg := "msgpack: decimal number is bigger than maximum supported number (10^38 - 1)"
 	decNum := decimal.New(1, DecimalPrecision) // // 10^DecimalPrecision
-	tuple := TupleDecimal{number: decNum}
+	tuple := TupleDecimal{number: DecNumber{decNum}}
 	_, err := msgpack.Marshal(&tuple)
 	if err == nil {
 		t.Fatalf("It is possible to encode a number unsupported by Tarantool")
@@ -249,12 +251,14 @@ func TestEncodeMinNumber(t *testing.T) {
 	referenceErrMsg := "msgpack: decimal number is lesser than minimum supported number (-10^38 - 1)"
 	two := decimal.NewFromInt(2)
 	decNum := decimal.New(1, DecimalPrecision).Neg().Sub(two) // -10^DecimalPrecision - 2
-	tuple := TupleDecimal{number: decNum}
+	tuple := TupleDecimal{number: DecNumber{decNum}}
 	_, err := msgpack.Marshal(&tuple)
 	if err == nil {
 		t.Fatalf("It is possible to encode a number unsupported by Tarantool")
 	}
 	if err.Error() != referenceErrMsg {
+		fmt.Println("Actual message:   ", err.Error())
+		fmt.Println("Expected message: ", referenceErrMsg)
 		t.Fatalf("Incorrect error message on attempt to encode number unsupported by Tarantool")
 	}
 }
@@ -266,7 +270,7 @@ func benchmarkMPEncodeDecode(b *testing.B, src decimal.Decimal, dst interface{})
 	var buf []byte
 	var err error
 	for i := 0; i < b.N; i++ {
-		tuple := TupleDecimal{number: src}
+		tuple := TupleDecimal{number: DecNumber{src}}
 		if buf, err = msgpack.Marshal(&tuple); err != nil {
 			b.Fatal(err)
 		}
@@ -348,7 +352,7 @@ func tupleValueIsDecimal(t *testing.T, tuples []interface{}, number decimal.Deci
 		if len(tpl) != 1 {
 			t.Fatalf("Unexpected return value body (tuple len)")
 		}
-		if val, ok := tpl[0].(decimal.Decimal); !ok || !val.Equal(number) {
+		if val, ok := tpl[0].(DecNumber); !ok || !val.Equal(number) {
 			t.Fatalf("Unexpected return value body (tuple 0 field)")
 		}
 	}
@@ -441,7 +445,7 @@ func TestSelect(t *testing.T) {
 		t.Fatalf("Failed to prepare test decimal: %s", err)
 	}
 
-	resp, err := conn.Insert(space, []interface{}{number})
+	resp, err := conn.Insert(space, []interface{}{&DecNumber{number}})
 	if err != nil {
 		t.Fatalf("Decimal insert failed: %s", err)
 	}
@@ -452,7 +456,7 @@ func TestSelect(t *testing.T) {
 
 	var offset uint32 = 0
 	var limit uint32 = 1
-	resp, err = conn.Select(space, index, offset, limit, IterEq, []interface{}{number})
+	resp, err = conn.Select(space, index, offset, limit, IterEq, []interface{}{&DecNumber{number}})
 	if err != nil {
 		t.Fatalf("Decimal select failed: %s", err.Error())
 	}
@@ -461,7 +465,7 @@ func TestSelect(t *testing.T) {
 	}
 	tupleValueIsDecimal(t, resp.Data, number)
 
-	resp, err = conn.Delete(space, index, []interface{}{number})
+	resp, err = conn.Delete(space, index, []interface{}{&DecNumber{number}})
 	if err != nil {
 		t.Fatalf("Decimal delete failed: %s", err)
 	}
@@ -474,7 +478,7 @@ func assertInsert(t *testing.T, conn *Connection, numString string) {
 		t.Fatalf("Failed to prepare test decimal: %s", err)
 	}
 
-	resp, err := conn.Insert(space, []interface{}{number})
+	resp, err := conn.Insert(space, []interface{}{&DecNumber{number}})
 	if err != nil {
 		t.Fatalf("Decimal insert failed: %s", err)
 	}
@@ -483,7 +487,7 @@ func assertInsert(t *testing.T, conn *Connection, numString string) {
 	}
 	tupleValueIsDecimal(t, resp.Data, number)
 
-	resp, err = conn.Delete(space, index, []interface{}{number})
+	resp, err = conn.Delete(space, index, []interface{}{&DecNumber{number}})
 	if err != nil {
 		t.Fatalf("Decimal delete failed: %s", err)
 	}
@@ -515,7 +519,7 @@ func TestReplace(t *testing.T) {
 		t.Fatalf("Failed to prepare test decimal: %s", err)
 	}
 
-	respRep, errRep := conn.Replace(space, []interface{}{number})
+	respRep, errRep := conn.Replace(space, []interface{}{&DecNumber{number}})
 	if errRep != nil {
 		t.Fatalf("Decimal replace failed: %s", errRep)
 	}
@@ -524,7 +528,7 @@ func TestReplace(t *testing.T) {
 	}
 	tupleValueIsDecimal(t, respRep.Data, number)
 
-	respSel, errSel := conn.Select(space, index, 0, 1, IterEq, []interface{}{number})
+	respSel, errSel := conn.Select(space, index, 0, 1, IterEq, []interface{}{&DecNumber{number}})
 	if errSel != nil {
 		t.Fatalf("Decimal select failed: %s", errSel)
 	}