diff --git a/codec/reflectcodec/struct_fielder.go b/codec/reflectcodec/struct_fielder.go index d266b60a3ebf..2fa9133b7d02 100644 --- a/codec/reflectcodec/struct_fielder.go +++ b/codec/reflectcodec/struct_fielder.go @@ -18,10 +18,6 @@ const ( // TagValue is the value the tag must have to be serialized. TagValue = "true" - - // TagValue is the value the tag must have to be serialized, this variant - // includes the nullable option - TagWithNullableValue = "true,nullable" ) var _ StructFielder = (*structFielder)(nil) @@ -29,7 +25,6 @@ var _ StructFielder = (*structFielder)(nil) type FieldDesc struct { Index int MaxSliceLen uint32 - Nullable bool } // StructFielder handles discovery of serializable fields in a struct. @@ -89,17 +84,10 @@ func (s *structFielder) GetSerializedFields(t reflect.Type) ([]FieldDesc, error) // any tag with the right value var ( captureField bool - nullable bool ) for _, tag := range s.tags { - switch field.Tag.Get(tag) { - case TagValue: - captureField = true - case TagWithNullableValue: + if field.Tag.Get(tag) == TagValue { captureField = true - nullable = true - } - if captureField { break } } @@ -121,7 +109,6 @@ func (s *structFielder) GetSerializedFields(t reflect.Type) ([]FieldDesc, error) serializedFields = append(serializedFields, FieldDesc{ Index: i, MaxSliceLen: maxSliceLen, - Nullable: nullable, }) } s.serializedFieldIndices[t] = serializedFields // cache result diff --git a/codec/reflectcodec/type_codec.go b/codec/reflectcodec/type_codec.go index 6f18f8500272..06564b3016c8 100644 --- a/codec/reflectcodec/type_codec.go +++ b/codec/reflectcodec/type_codec.go @@ -90,16 +90,14 @@ func (c *genericCodec) Size(value interface{}) (int, error) { return 0, errMarshalNil // can't marshal nil } - size, _, err := c.size(reflect.ValueOf(value), false /*=nullable*/, nil /*=typeStack*/) + size, _, err := c.size(reflect.ValueOf(value), nil /*=typeStack*/) return size, err } // size returns the size of the value along with whether the value is constant -// sized. This function takes into account a `nullable` property which allows -// pointers and interfaces to serialize nil values +// sized. func (c *genericCodec) size( value reflect.Value, - nullable bool, typeStack set.Set[reflect.Type], ) (int, bool, error) { switch valueKind := value.Kind(); valueKind { @@ -125,23 +123,14 @@ func (c *genericCodec) size( return wrappers.StringLen(value.String()), false, nil case reflect.Ptr: if value.IsNil() { - if !nullable { - return 0, false, errMarshalNil - } return wrappers.BoolLen, false, nil } - size, constSize, err := c.size(value.Elem(), false /*=nullable*/, typeStack) - if nullable { - return wrappers.BoolLen + size, false, err - } + size, constSize, err := c.size(value.Elem(), typeStack) return size, constSize, err case reflect.Interface: if value.IsNil() { - if !nullable { - return 0, false, errMarshalNil - } return wrappers.BoolLen, false, nil } @@ -153,12 +142,9 @@ func (c *genericCodec) size( typeStack.Add(underlyingType) prefixSize := c.typer.PrefixSize(underlyingType) - valueSize, _, err := c.size(value.Elem(), false /*=nullable*/, typeStack) + valueSize, _, err := c.size(value.Elem(), typeStack) typeStack.Remove(underlyingType) - if nullable { - return wrappers.BoolLen + prefixSize + valueSize, false, err - } return prefixSize + valueSize, false, err case reflect.Slice: @@ -167,7 +153,7 @@ func (c *genericCodec) size( return wrappers.IntLen, false, nil } - size, constSize, err := c.size(value.Index(0), nullable, typeStack) + size, constSize, err := c.size(value.Index(0), typeStack) if err != nil { return 0, false, err } @@ -179,7 +165,7 @@ func (c *genericCodec) size( } for i := 1; i < numElts; i++ { - innerSize, _, err := c.size(value.Index(i), nullable, typeStack) + innerSize, _, err := c.size(value.Index(i), typeStack) if err != nil { return 0, false, err } @@ -193,7 +179,7 @@ func (c *genericCodec) size( return 0, true, nil } - size, constSize, err := c.size(value.Index(0), nullable, typeStack) + size, constSize, err := c.size(value.Index(0), typeStack) if err != nil { return 0, false, err } @@ -205,7 +191,7 @@ func (c *genericCodec) size( } for i := 1; i < numElts; i++ { - innerSize, _, err := c.size(value.Index(i), nullable, typeStack) + innerSize, _, err := c.size(value.Index(i), typeStack) if err != nil { return 0, false, err } @@ -224,7 +210,7 @@ func (c *genericCodec) size( constSize = true ) for _, fieldDesc := range serializedFields { - innerSize, innerConstSize, err := c.size(value.Field(fieldDesc.Index), fieldDesc.Nullable, typeStack) + innerSize, innerConstSize, err := c.size(value.Field(fieldDesc.Index), typeStack) if err != nil { return 0, false, err } @@ -239,11 +225,11 @@ func (c *genericCodec) size( return wrappers.IntLen, false, nil } - keySize, keyConstSize, err := c.size(iter.Key(), false /*=nullable*/, typeStack) + keySize, keyConstSize, err := c.size(iter.Key(), typeStack) if err != nil { return 0, false, err } - valueSize, valueConstSize, err := c.size(iter.Value(), nullable, typeStack) + valueSize, valueConstSize, err := c.size(iter.Value(), typeStack) if err != nil { return 0, false, err } @@ -258,7 +244,7 @@ func (c *genericCodec) size( totalValueSize = valueSize ) for iter.Next() { - valueSize, _, err := c.size(iter.Value(), nullable, typeStack) + valueSize, _, err := c.size(iter.Value(), typeStack) if err != nil { return 0, false, err } @@ -272,7 +258,7 @@ func (c *genericCodec) size( totalKeySize = keySize ) for iter.Next() { - keySize, _, err := c.size(iter.Key(), false /*=nullable*/, typeStack) + keySize, _, err := c.size(iter.Key(), typeStack) if err != nil { return 0, false, err } @@ -283,11 +269,11 @@ func (c *genericCodec) size( default: totalSize := wrappers.IntLen + keySize + valueSize for iter.Next() { - keySize, _, err := c.size(iter.Key(), false /*=nullable*/, typeStack) + keySize, _, err := c.size(iter.Key(), typeStack) if err != nil { return 0, false, err } - valueSize, _, err := c.size(iter.Value(), nullable, typeStack) + valueSize, _, err := c.size(iter.Value(), typeStack) if err != nil { return 0, false, err } @@ -307,7 +293,7 @@ func (c *genericCodec) MarshalInto(value interface{}, p *wrappers.Packer) error return errMarshalNil // can't marshal nil } - return c.marshal(reflect.ValueOf(value), p, c.maxSliceLen, false /*=nullable*/, nil /*=typeStack*/) + return c.marshal(reflect.ValueOf(value), p, c.maxSliceLen, nil /*=typeStack*/) } // marshal writes the byte representation of [value] to [p] @@ -317,7 +303,6 @@ func (c *genericCodec) marshal( value reflect.Value, p *wrappers.Packer, maxSliceLen uint32, - nullable bool, typeStack set.Set[reflect.Type], ) error { switch valueKind := value.Kind(); valueKind { @@ -352,25 +337,13 @@ func (c *genericCodec) marshal( p.PackBool(value.Bool()) return p.Err case reflect.Ptr: - isNil := value.IsNil() - if nullable { - p.PackBool(isNil) - if isNil || p.Err != nil { - return p.Err - } - } else if isNil { + if value.IsNil() { return errMarshalNil } - return c.marshal(value.Elem(), p, c.maxSliceLen, false /*=nullable*/, typeStack) + return c.marshal(value.Elem(), p, c.maxSliceLen, typeStack) case reflect.Interface: - isNil := value.IsNil() - if nullable { - p.PackBool(isNil) - if isNil || p.Err != nil { - return p.Err - } - } else if isNil { + if value.IsNil() { return errMarshalNil } @@ -383,7 +356,7 @@ func (c *genericCodec) marshal( if err := c.typer.PackPrefix(p, underlyingType); err != nil { return err } - if err := c.marshal(value.Elem(), p, c.maxSliceLen, false /*=nullable*/, typeStack); err != nil { + if err := c.marshal(value.Elem(), p, c.maxSliceLen, typeStack); err != nil { return err } typeStack.Remove(underlyingType) @@ -414,7 +387,7 @@ func (c *genericCodec) marshal( return p.Err } for i := 0; i < numElts; i++ { // Process each element in the slice - if err := c.marshal(value.Index(i), p, c.maxSliceLen, nullable, typeStack); err != nil { + if err := c.marshal(value.Index(i), p, c.maxSliceLen, typeStack); err != nil { return err } } @@ -434,7 +407,7 @@ func (c *genericCodec) marshal( ) } for i := 0; i < numElts; i++ { // Process each element in the array - if err := c.marshal(value.Index(i), p, c.maxSliceLen, nullable, typeStack); err != nil { + if err := c.marshal(value.Index(i), p, c.maxSliceLen, typeStack); err != nil { return err } } @@ -445,7 +418,7 @@ func (c *genericCodec) marshal( return err } for _, fieldDesc := range serializedFields { // Go through all fields of this struct that are serialized - if err := c.marshal(value.Field(fieldDesc.Index), p, fieldDesc.MaxSliceLen, fieldDesc.Nullable, typeStack); err != nil { // Serialize the field and write to byte array + if err := c.marshal(value.Field(fieldDesc.Index), p, fieldDesc.MaxSliceLen, typeStack); err != nil { // Serialize the field and write to byte array return err } } @@ -476,7 +449,7 @@ func (c *genericCodec) marshal( startOffset := p.Offset endOffset := p.Offset for i, key := range keys { - if err := c.marshal(key, p, c.maxSliceLen, false /*=nullable*/, typeStack); err != nil { + if err := c.marshal(key, p, c.maxSliceLen, typeStack); err != nil { return err } if p.Err != nil { @@ -509,7 +482,7 @@ func (c *genericCodec) marshal( } // serialize and pack value - if err := c.marshal(value.MapIndex(key.key), p, c.maxSliceLen, nullable, typeStack); err != nil { + if err := c.marshal(value.MapIndex(key.key), p, c.maxSliceLen, typeStack); err != nil { return err } } @@ -534,7 +507,7 @@ func (c *genericCodec) Unmarshal(bytes []byte, dest interface{}) error { if destPtr.Kind() != reflect.Ptr { return errNeedPointer } - if err := c.unmarshal(&p, destPtr.Elem(), c.maxSliceLen, false /*=nullable*/, nil /*=typeStack*/); err != nil { + if err := c.unmarshal(&p, destPtr.Elem(), c.maxSliceLen, nil /*=typeStack*/); err != nil { return err } if p.Offset != len(bytes) { @@ -549,16 +522,11 @@ func (c *genericCodec) Unmarshal(bytes []byte, dest interface{}) error { // Unmarshal from p.Bytes into [value]. [value] must be addressable. // -// The [nullable] property affects how pointers and interfaces are unmarshalled, -// as an extra byte would be used to unmarshal nil values for pointers and -// interaces -// // c.lock should be held for the duration of this function func (c *genericCodec) unmarshal( p *wrappers.Packer, value reflect.Value, maxSliceLen uint32, - nullable bool, typeStack set.Set[reflect.Type], ) error { switch value.Kind() { @@ -651,7 +619,7 @@ func (c *genericCodec) unmarshal( zeroValue := reflect.Zero(innerType) for i := 0; i < numElts; i++ { value.Set(reflect.Append(value, zeroValue)) - if err := c.unmarshal(p, value.Index(i), c.maxSliceLen, nullable, typeStack); err != nil { + if err := c.unmarshal(p, value.Index(i), c.maxSliceLen, typeStack); err != nil { return err } } @@ -669,7 +637,7 @@ func (c *genericCodec) unmarshal( return nil } for i := 0; i < numElts; i++ { - if err := c.unmarshal(p, value.Index(i), c.maxSliceLen, nullable, typeStack); err != nil { + if err := c.unmarshal(p, value.Index(i), c.maxSliceLen, typeStack); err != nil { return err } } @@ -681,13 +649,6 @@ func (c *genericCodec) unmarshal( } return nil case reflect.Interface: - if nullable { - isNil := p.UnpackBool() - if isNil || p.Err != nil { - return p.Err - } - } - intfImplementor, err := c.typer.UnpackPrefix(p, value.Type()) if err != nil { return err @@ -699,7 +660,7 @@ func (c *genericCodec) unmarshal( typeStack.Add(intfImplementorType) // Unmarshal into the struct - if err := c.unmarshal(p, intfImplementor, c.maxSliceLen, false /*=nullable*/, typeStack); err != nil { + if err := c.unmarshal(p, intfImplementor, c.maxSliceLen, typeStack); err != nil { return err } @@ -714,25 +675,18 @@ func (c *genericCodec) unmarshal( } // Go through the fields and umarshal into them for _, fieldDesc := range serializedFieldIndices { - if err := c.unmarshal(p, value.Field(fieldDesc.Index), fieldDesc.MaxSliceLen, fieldDesc.Nullable, typeStack); err != nil { + if err := c.unmarshal(p, value.Field(fieldDesc.Index), fieldDesc.MaxSliceLen, typeStack); err != nil { return err } } return nil case reflect.Ptr: - if nullable { - isNil := p.UnpackBool() - if isNil || p.Err != nil { - return p.Err - } - } - // Get the type this pointer points to t := value.Type().Elem() // Create a new pointer to a new value of the underlying type v := reflect.New(t) // Fill the value - if err := c.unmarshal(p, v.Elem(), c.maxSliceLen, false /*=nullable*/, typeStack); err != nil { + if err := c.unmarshal(p, v.Elem(), c.maxSliceLen, typeStack); err != nil { return err } // Assign to the top-level struct's member @@ -767,7 +721,7 @@ func (c *genericCodec) unmarshal( keyStartOffset := p.Offset - if err := c.unmarshal(p, mapKey, c.maxSliceLen, false /*=nullable*/, typeStack); err != nil { + if err := c.unmarshal(p, mapKey, c.maxSliceLen, typeStack); err != nil { return err } @@ -785,7 +739,7 @@ func (c *genericCodec) unmarshal( // Get the value mapValue := reflect.New(mapValueType).Elem() - if err := c.unmarshal(p, mapValue, c.maxSliceLen, nullable, typeStack); err != nil { + if err := c.unmarshal(p, mapValue, c.maxSliceLen, typeStack); err != nil { return err } diff --git a/codec/reflectcodec/type_codec_test.go b/codec/reflectcodec/type_codec_test.go deleted file mode 100644 index 42b256c4a6c9..000000000000 --- a/codec/reflectcodec/type_codec_test.go +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright (C) 2019-2023, Ava Labs, Inc. All rights reserved. -// See the file LICENSE for licensing terms. - -package reflectcodec - -import ( - "reflect" - "testing" - - "github.com/stretchr/testify/require" -) - -func TestSizeWithNil(t *testing.T) { - require := require.New(t) - var x *int32 - y := int32(1) - c := genericCodec{} - _, _, err := c.size(reflect.ValueOf(x), false /*=nullable*/, nil /*=typeStack*/) - require.ErrorIs(err, errMarshalNil) - len, _, err := c.size(reflect.ValueOf(x), true /*=nullable*/, nil /*=typeStack*/) - require.Empty(err) - require.Equal(1, len) - x = &y - len, _, err = c.size(reflect.ValueOf(y), true /*=nullable*/, nil /*=typeStack*/) - require.Empty(err) - require.Equal(4, len) - len, _, err = c.size(reflect.ValueOf(x), true /*=nullable*/, nil /*=typeStack*/) - require.Empty(err) - require.Equal(5, len) -} diff --git a/codec/test_codec.go b/codec/test_codec.go index 341912a823af..b24784ac7d6a 100644 --- a/codec/test_codec.go +++ b/codec/test_codec.go @@ -23,7 +23,6 @@ var ( TestBigArray, TestPointerToStruct, TestSliceOfStruct, - TestStructWithNullable, TestInterface, TestSliceOfInterface, TestArrayOfInterface, @@ -64,8 +63,7 @@ type Foo interface { } type MyInnerStruct struct { - Str string `serialize:"true"` - NumberNotProvided *int32 `serialize:"true,nullable"` + Str string `serialize:"true"` } func (*MyInnerStruct) Foo() int { @@ -88,15 +86,6 @@ type MyInnerStruct3 struct { F Foo `serialize:"true"` } -type MyStructWithNullable struct { - Interface any `serialize:"true,nullable"` - Int32 *int32 `serialize:"true,nullable"` - Int64 *int64 `serialize:"true,nullable"` - Int32Slice []*int32 `serialize:"true,nullable"` - Int32Array [2]*int32 `serialize:"true,nullable"` - Int32Map map[int32]*int32 `serialize:"true,nullable"` -} - type myStruct struct { InnerStruct MyInnerStruct `serialize:"true"` InnerStruct2 *MyInnerStruct `serialize:"true"` @@ -156,23 +145,21 @@ func TestStruct(codec GeneralCodec, t testing.TB) { myMap7["key"] = "value" myMap7[int32(1)] = int32(2) - number := int32(8) - myStructInstance := myStruct{ - InnerStruct: MyInnerStruct{"hello", nil}, - InnerStruct2: &MyInnerStruct{"yello", nil}, + InnerStruct: MyInnerStruct{"hello"}, + InnerStruct2: &MyInnerStruct{"yello"}, Member1: 1, Member2: 2, MySlice: []byte{1, 2, 3, 4}, MySlice2: []string{"one", "two", "three"}, - MySlice3: []MyInnerStruct{{"abc", nil}, {"ab", &number}, {"c", nil}}, + MySlice3: []MyInnerStruct{{"abc"}, {"ab"}, {"c"}}, MySlice4: []*MyInnerStruct2{{true}, {}}, MySlice5: []Foo{&MyInnerStruct2{true}, &MyInnerStruct2{}}, MyArray: [4]byte{5, 6, 7, 8}, MyArray2: [5]string{"four", "five", "six", "seven"}, - MyArray3: [3]MyInnerStruct{{"d", nil}, {"e", nil}, {"f", nil}}, + MyArray3: [3]MyInnerStruct{{"d"}, {"e"}, {"f"}}, MyArray4: [2]*MyInnerStruct2{{}, {true}}, - MyInterface: &MyInnerStruct{"yeet", &number}, + MyInterface: &MyInnerStruct{"yeet"}, InnerStruct3: MyInnerStruct3{ Str: "str", M1: MyInnerStruct{ @@ -427,79 +414,19 @@ func TestPointerToStruct(codec GeneralCodec, t testing.TB) { require.Equal(myPtr, myPtrUnmarshaled) } -func TestStructWithNullable(codec GeneralCodec, t testing.TB) { - require := require.New(t) - n1 := int32(5) - n2 := int64(10) - struct1 := MyStructWithNullable{ - Interface: nil, - Int32: &n1, - Int64: &n2, - Int32Slice: []*int32{ - nil, - nil, - &n1, - }, - Int32Array: [2]*int32{ - nil, - &n1, - }, - Int32Map: map[int32]*int32{ - 1: nil, - 2: &n1, - }, - } - - require.NoError(codec.RegisterType(&MyStructWithNullable{})) - manager := NewDefaultManager() - require.NoError(manager.RegisterCodec(0, codec)) - - bytes, err := manager.Marshal(0, struct1) - require.NoError(err) - - bytesLen, err := manager.Size(0, struct1) - require.NoError(err) - require.Len(bytes, bytesLen) - - var struct1Unmarshaled MyStructWithNullable - version, err := manager.Unmarshal(bytes, &struct1Unmarshaled) - require.NoError(err) - require.Zero(version) - require.Equal(struct1, struct1Unmarshaled) - - struct1 = MyStructWithNullable{ - Int32Slice: []*int32{}, - Int32Map: map[int32]*int32{}, - } - bytes, err = manager.Marshal(0, struct1) - require.NoError(err) - - bytesLen, err = manager.Size(0, struct1) - require.NoError(err) - require.Len(bytes, bytesLen) - - var struct1Unmarshaled2 MyStructWithNullable - version, err = manager.Unmarshal(bytes, &struct1Unmarshaled2) - require.NoError(err) - require.Zero(version) - require.Equal(struct1, struct1Unmarshaled2) -} - // Test marshalling a slice of structs func TestSliceOfStruct(codec GeneralCodec, t testing.TB) { require := require.New(t) - n1 := int32(-1) - n2 := int32(0xff) mySlice := []MyInnerStruct3{ { Str: "One", - M1: MyInnerStruct{"Two", &n1}, - F: &MyInnerStruct{"Three", &n2}, + M1: MyInnerStruct{"Two"}, + F: &MyInnerStruct{"Three"}, }, { Str: "Four", - M1: MyInnerStruct{"Five", nil}, - F: &MyInnerStruct{"Six", nil}, + M1: MyInnerStruct{"Five"}, + F: &MyInnerStruct{"Six"}, }, } require.NoError(codec.RegisterType(&MyInnerStruct{}))