From df43e6395de2c7e4d8647643f5b152a795c61d51 Mon Sep 17 00:00:00 2001 From: gagliardetto Date: Wed, 5 Jan 2022 20:45:52 +0100 Subject: [PATCH] Fix optionality propagation; write [n]byte in one operation; more step-by-step-tests. --- encoder_bin.go | 24 ++++++-- encoder_borsh.go | 32 ++++++++--- encoder_compact-u16.go | 32 ++++++++--- encoder_test.go | 122 ++++++++++++++++++++++++++++++++++++++++- init_test.go | 4 +- 5 files changed, 188 insertions(+), 26 deletions(-) diff --git a/encoder_bin.go b/encoder_bin.go index 3d82b92..4d286a8 100644 --- a/encoder_bin.go +++ b/encoder_bin.go @@ -49,6 +49,8 @@ func (e *Encoder) encodeBin(rv reflect.Value, opt *option) (err error) { if err != nil { return err } + // The optionality has been used; stop its propagation: + opt.setIsOptional(false) } if isZero(rv) { @@ -104,9 +106,21 @@ func (e *Encoder) encodeBin(rv reflect.Value, opt *option) (err error) { zlog = zlog.Named("array") zlog.Debug("encode: array", zap.Int("length", l), zap.Stringer("type", rv.Kind())) } - for i := 0; i < l; i++ { - if err = e.encodeBin(rv.Index(i), opt); err != nil { - return + + if rv.Type().Elem().Kind() == reflect.Uint8 { + // if it's a [n]byte, accumulate and write in one command: + arr := make([]byte, l) + for i := 0; i < l; i++ { + arr[i] = byte(rv.Index(i).Uint()) + } + if err := e.WriteBytes(arr, false); err != nil { + return err + } + } else { + for i := 0; i < l; i++ { + if err = e.encodeBin(rv.Index(i), nil); err != nil { + return + } } } case reflect.Slice: @@ -131,7 +145,7 @@ func (e *Encoder) encodeBin(rv reflect.Value, opt *option) (err error) { // we would want to skip to the correct head_offset for i := 0; i < l; i++ { - if err = e.encodeBin(rv.Index(i), opt); err != nil { + if err = e.encodeBin(rv.Index(i), nil); err != nil { return } } @@ -238,7 +252,7 @@ func (e *Encoder) encodeStructBin(rt reflect.Type, rv reflect.Value) (err error) } if err := e.encodeBin(rv, option); err != nil { - return err + return fmt.Errorf("error while encoding %q field: %w", structField.Name, err) } } return nil diff --git a/encoder_borsh.go b/encoder_borsh.go index ab058eb..845893a 100644 --- a/encoder_borsh.go +++ b/encoder_borsh.go @@ -44,10 +44,14 @@ func (e *Encoder) encodeBorsh(rv reflect.Value, opt *option) (err error) { if traceEnabled { zlog.Debug("encode: skipping optional value with", zap.Stringer("type", rv.Kind())) } - e.WriteBool(false) - return nil + return e.WriteBool(false) + } + err := e.WriteBool(true) + if err != nil { + return err } - e.WriteBool(true) + // The optionality has been used; stop its propagation: + opt.setIsOptional(false) } // Reset optionality so it won't propagate to child types: opt = opt.clone().setIsOptional(false) @@ -119,9 +123,21 @@ func (e *Encoder) encodeBorsh(rv reflect.Value, opt *option) (err error) { zlog = zlog.Named("array") zlog.Debug("encode: array", zap.Int("length", l), zap.Stringer("type", rv.Kind())) } - for i := 0; i < l; i++ { - if err = e.encodeBorsh(rv.Index(i), opt); err != nil { - return + + if rv.Type().Elem().Kind() == reflect.Uint8 { + // if it's a [n]byte, accumulate and write in one command: + arr := make([]byte, l) + for i := 0; i < l; i++ { + arr[i] = byte(rv.Index(i).Uint()) + } + if err := e.WriteBytes(arr, false); err != nil { + return err + } + } else { + for i := 0; i < l; i++ { + if err = e.encodeBorsh(rv.Index(i), nil); err != nil { + return + } } } case reflect.Slice: @@ -146,7 +162,7 @@ func (e *Encoder) encodeBorsh(rv reflect.Value, opt *option) (err error) { // we would want to skip to the correct head_offset for i := 0; i < l; i++ { - if err = e.encodeBorsh(rv.Index(i), opt); err != nil { + if err = e.encodeBorsh(rv.Index(i), nil); err != nil { return } } @@ -295,7 +311,7 @@ func (e *Encoder) encodeStructBorsh(rt reflect.Type, rv reflect.Value) (err erro } if err := e.encodeBorsh(rv, option); err != nil { - return err + return fmt.Errorf("error while encoding %q field: %w", structField.Name, err) } } return nil diff --git a/encoder_compact-u16.go b/encoder_compact-u16.go index 50939c0..a9ab4fd 100644 --- a/encoder_compact-u16.go +++ b/encoder_compact-u16.go @@ -42,10 +42,14 @@ func (e *Encoder) encodeCompactU16(rv reflect.Value, opt *option) (err error) { if traceEnabled { zlog.Debug("encode: skipping optional value with", zap.Stringer("type", rv.Kind())) } - e.WriteBool(false) - return nil + return e.WriteBool(false) } - e.WriteBool(true) + err := e.WriteBool(true) + if err != nil { + return err + } + // The optionality has been used; stop its propagation: + opt.setIsOptional(false) } if isZero(rv) { @@ -101,9 +105,21 @@ func (e *Encoder) encodeCompactU16(rv reflect.Value, opt *option) (err error) { zlog = zlog.Named("array") zlog.Debug("encode: array", zap.Int("length", l), zap.Stringer("type", rv.Kind())) } - for i := 0; i < l; i++ { - if err = e.encodeCompactU16(rv.Index(i), opt); err != nil { - return + + if rv.Type().Elem().Kind() == reflect.Uint8 { + // if it's a [n]byte, accumulate and write in one command: + arr := make([]byte, l) + for i := 0; i < l; i++ { + arr[i] = byte(rv.Index(i).Uint()) + } + if err := e.WriteBytes(arr, false); err != nil { + return err + } + } else { + for i := 0; i < l; i++ { + if err = e.encodeCompactU16(rv.Index(i), nil); err != nil { + return + } } } case reflect.Slice: @@ -128,7 +144,7 @@ func (e *Encoder) encodeCompactU16(rv reflect.Value, opt *option) (err error) { // we would want to skip to the correct head_offset for i := 0; i < l; i++ { - if err = e.encodeCompactU16(rv.Index(i), opt); err != nil { + if err = e.encodeCompactU16(rv.Index(i), nil); err != nil { return } } @@ -235,7 +251,7 @@ func (e *Encoder) encodeStructCompactU16(rt reflect.Type, rv reflect.Value) (err } if err := e.encodeCompactU16(rv, option); err != nil { - return err + return fmt.Errorf("error while encoding %q field: %w", structField.Name, err) } } return nil diff --git a/encoder_test.go b/encoder_test.go index 9097800..2b43821 100644 --- a/encoder_test.go +++ b/encoder_test.go @@ -19,6 +19,7 @@ package bin import ( "bytes" + "encoding/binary" "encoding/hex" "math" "testing" @@ -492,6 +493,7 @@ func TestEncoder_BinaryStruct(t *testing.T) { } func TestEncoder_BinaryTestStructWithTags(t *testing.T) { + s := &binaryTestStructWithTags{ F1: "abc", F2: -75, @@ -503,7 +505,120 @@ func TestEncoder_BinaryTestStructWithTags(t *testing.T) { F8: -23.13, F9: 3.92, F10: true, - F12: []int64{99, 33}, + F12: (*[]int64)(&[]int64{99, 33}), + } + + expected := []byte{ + 255, 181, // F2 + 0, 99, // F3 + 255, 255, 255, 25, // F4 + 0, 0, 3, 231, // F5 + 255, 255, 255, 255, 255, 255, 204, 81, // F6 + 0, 0, 0, 0, 0, 1, 134, 159, // F7 + 193, 185, 10, 61, // F8 + 64, 15, 92, 40, 245, 194, 143, 92, // F9 + 1, // F10 + + 0, 0, 0, 0, // F11 is optional, and NOT SET (meaning uint32(0)) + + 1, 0, 0, 0, // F12 is optional, and IS SET (meaning uint32(1)) + 2, // F12 is a slice, and the len is encoded as WriteUVarInt) + 99, 0, 0, 0, 0, 0, 0, 0, + 33, 0, 0, 0, 0, 0, 0, 0, + } + { + buf := new(bytes.Buffer) + enc := NewBinEncoder(buf) + { + err := enc.WriteInt16(s.F2, binary.BigEndian) // [255, 181](len=2) + if err != nil { + panic(err) + } + } + { + err := enc.WriteUint16(s.F3, binary.BigEndian) // [0, 99](len=2) + if err != nil { + panic(err) + } + } + { + err := enc.WriteInt32(s.F4, binary.BigEndian) // [255, 255, 255, 25](len=4) + if err != nil { + panic(err) + } + } + { + err := enc.WriteUint32(s.F5, binary.BigEndian) // [0, 0, 3, 231](len=4) + if err != nil { + panic(err) + } + } + { + err := enc.WriteInt64(s.F6, binary.BigEndian) // [255, 255, 255, 255, 255, 255, 204, 81](len=8) + if err != nil { + panic(err) + } + } + { + err := enc.WriteUint64(s.F7, binary.BigEndian) // [0, 0, 0, 0, 0, 1, 134, 159](len=8) + if err != nil { + panic(err) + } + } + { + err := enc.WriteFloat32(s.F8, binary.BigEndian) // [193, 185, 10, 61](len=4) + if err != nil { + panic(err) + } + } + { + err := enc.WriteFloat64(s.F9, binary.BigEndian) // [64, 15, 92, 40, 245, 194, 143, 92](len=8) + if err != nil { + panic(err) + } + } + { + err := enc.WriteBool(s.F10) // [1](len=1) + if err != nil { + panic(err) + } + } + { + err := enc.WriteUint32(0, binary.LittleEndian) // [0, 0, 0, 0](len=4) + if err != nil { + panic(err) + } + } + { + err := enc.WriteUint32(1, binary.LittleEndian) // [1, 0, 0, 0](len=4) + if err != nil { + panic(err) + } + } + { + err := enc.WriteUVarInt(2) // [2](len=1) + if err != nil { + panic(err) + } + } + { + err := enc.WriteInt64((*s.F12)[0], binary.LittleEndian) // [99, 0, 0, 0, 0, 0, 0, 0](len=8) + if err != nil { + panic(err) + } + } + { + err := enc.WriteInt64((*s.F12)[1], binary.LittleEndian) // [33, 0, 0, 0, 0, 0, 0, 0](len=8) + if err != nil { + panic(err) + } + } + + assert.Equal(t, + expected, + buf.Bytes(), + FormatByteSlice(buf.Bytes()), + ) } buf := new(bytes.Buffer) @@ -512,8 +627,9 @@ func TestEncoder_BinaryTestStructWithTags(t *testing.T) { assert.NoError(t, err) assert.Equal(t, - "ffb50063ffffff19000003e7ffffffffffffcc51000000000001869fc1b90a3d400f5c28f5c28f5c01000000000100000002010000006300000000000000010000002100000000000000", - hex.EncodeToString(buf.Bytes()), + expected, + buf.Bytes(), + FormatByteSlice(buf.Bytes()), ) } diff --git a/init_test.go b/init_test.go index f15f217..c43197a 100644 --- a/init_test.go +++ b/init_test.go @@ -69,8 +69,8 @@ type binaryTestStructWithTags struct { F8 float32 `bin:"big"` F9 float64 `bin:"big"` F10 bool - F11 *Int64 `bin:"optional"` - F12 []int64 `bin:"optional"` + F11 *Int64 `bin:"optional"` + F12 *[]int64 `bin:"optional"` } func setupBench(b *testing.B) {