ugorji / go

idiomatic codec and rpc lib for msgpack, cbor, json, etc. msgpack.org[Go]
MIT License
1.86k stars 295 forks source link

codecgen: missing field with nested omitEmpty struct #344

Closed vanackere closed 3 years ago

vanackere commented 4 years ago

Hi, I hope this bug is not a user error this time ;-)

I have a test case failing with the current codecgen (the same code was working before updating the package). Minimal reproduction:

// file: user.go
package testcodec

import (
    "github.com/ugorji/go/codec"
)

type S struct {
    A string
    B string
}

type User struct {
    MaybeEmpty S `json:",omitempty"`
}

func toJSON(u *User) ([]byte, error) {
    var res []byte
    h := &codec.JsonHandle{}
    enc := codec.NewEncoderBytes(&res, h)
    err := enc.Encode(u)
    return res, err
}

The following test passes without codecgen and fails with code generation for the User type (codecgen -d 1977 -o user_codecgen.go -r User user.go):

// file: user_test.go
package testcodec

import (
    "testing"

    "github.com/stretchr/testify/assert"
)

func TestToJSON(t *testing.T) {
    r, _ := toJSON(&User{
        MaybeEmpty: S{
            A: "a",
        },
    })
    assert.Equal(t, `{"MaybeEmpty":{"A":"a","B":""}}`, string(r))
    // With codecgen:
    //--- FAIL: TestToJSON (0.00s)
    //    user_test.go:15: 
    //                Error Trace:    user_test.go:15
    //                Error:          Not equal: 
    //                                expected: "{\"MaybeEmpty\":{\"A\":\"a\",\"B\":\"\"}}"
    //                                actual  : "{}"
}
ugorji commented 4 years ago

This looks like a real bug.

Let me look into it this weekend.

ugorji commented 4 years ago

BTW, can you run it without RecursiveEmptyCheck and post what you see? Thanks.

vanackere commented 4 years ago

Mhh I see the same bug without RecursiveEmptyCheck:

--- FAIL: TestToJSON (0.00s)
    user_test.go:16: 
                Error Trace:    user_test.go:16
                Error:          Not equal: 
                                expected: "{\"ID\":12,\"MaybeEmpty\":{\"A\":\"a\",\"B\":\"\"}}"
                                actual  : "{\"ID\":12}"

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1 +1 @@
                                -{"ID":12,"MaybeEmpty":{"A":"a","B":""}}
                                +{"ID":12}
                Test:           TestToJSON
vanackere commented 4 years ago

I updated the title and made the bug reproduction code shorter (it turns out that the ID field was not necessary either)

vanackere commented 4 years ago

Here is the generated file in case it helps:

// +build go1.6

// Code generated by codecgen - DO NOT EDIT.

package testcodec

import (
    "errors"
    "runtime"
    "strconv"

    codec1978 "github.com/ugorji/go/codec"
)

const (
    // ----- content types ----
    codecSelferCcUTF81977 = 1
    codecSelferCcRAW1977  = 255
    // ----- value types used ----
    codecSelferValueTypeArray1977     = 10
    codecSelferValueTypeMap1977       = 9
    codecSelferValueTypeString1977    = 6
    codecSelferValueTypeInt1977       = 2
    codecSelferValueTypeUint1977      = 3
    codecSelferValueTypeFloat1977     = 4
    codecSelferValueTypeNil1977       = 1
    codecSelferBitsize1977            = uint8(32 << (^uint(0) >> 63))
    codecSelferDecContainerLenNil1977 = -2147483648
)

var (
    errCodecSelferOnlyMapOrArrayEncodeToStruct1977 = errors.New(`only encoded map or array can be decoded into a struct`)
)

type codecSelfer1977 struct{}

func codecSelfer1977False() bool { return false }
func codecSelfer1977True() bool  { return true }

func init() {
    if codec1978.GenVersion != 20 {
        _, file, _, _ := runtime.Caller(0)
        ver := strconv.FormatInt(int64(codec1978.GenVersion), 10)
        panic(errors.New("codecgen version mismatch: current: 20, need " + ver + ". Re-generate file: " + file))
    }
}

func (x *User) CodecEncodeSelf(e *codec1978.Encoder) {
    var h codecSelfer1977
    z, r := codec1978.GenHelper().Encoder(e)
    _, _, _ = h, z, r
    if x == nil {
        r.EncodeNil()
    } else {
        yy2arr2 := z.EncBasicHandle().StructToArray
        _ = yy2arr2
        const yyr2 bool = false // struct tag has 'toArray'
        var yyq2 = [1]bool{     // should field at this index be written?
            !(x.MaybeEmpty.IsCodecEmpty()), // MaybeEmpty
        }
        _ = yyq2
        if yyr2 || yy2arr2 {
            z.EncWriteArrayStart(1)
            z.EncWriteArrayElem()
            if yyq2[0] {
                yy4 := &x.MaybeEmpty
                if yyxt5 := z.Extension(yy4); yyxt5 != nil {
                    z.EncExtension(yy4, yyxt5)
                } else {
                    z.EncFallback(yy4)
                }
            } else {
                r.EncodeNil()
            }
            z.EncWriteArrayEnd()
        } else {
            var yynn2 int
            for _, b := range yyq2 {
                if b {
                    yynn2++
                }
            }
            z.EncWriteMapStart(yynn2)
            yynn2 = 0
            if yyq2[0] {
                z.EncWriteMapElemKey()
                if z.IsJSONHandle() {
                    z.WriteStr("\"MaybeEmpty\"")
                } else {
                    r.EncodeString(`MaybeEmpty`)
                }
                z.EncWriteMapElemValue()
                yy6 := &x.MaybeEmpty
                if yyxt7 := z.Extension(yy6); yyxt7 != nil {
                    z.EncExtension(yy6, yyxt7)
                } else {
                    z.EncFallback(yy6)
                }
            }
            z.EncWriteMapEnd()
        }
    }
}

func (x *User) CodecDecodeSelf(d *codec1978.Decoder) {
    var h codecSelfer1977
    z, r := codec1978.GenHelper().Decoder(d)
    _, _, _ = h, z, r
    yyct2 := r.ContainerType()
    if yyct2 == codecSelferValueTypeNil1977 {
        *(x) = User{}
    } else if yyct2 == codecSelferValueTypeMap1977 {
        yyl2 := z.DecReadMapStart()
        if yyl2 == 0 {
        } else {
            x.codecDecodeSelfFromMap(yyl2, d)
        }
        z.DecReadMapEnd()
    } else if yyct2 == codecSelferValueTypeArray1977 {
        yyl2 := z.DecReadArrayStart()
        if yyl2 != 0 {
            x.codecDecodeSelfFromArray(yyl2, d)
        }
        z.DecReadArrayEnd()
    } else {
        panic(errCodecSelferOnlyMapOrArrayEncodeToStruct1977)
    }
}

func (x *User) codecDecodeSelfFromMap(l int, d *codec1978.Decoder) {
    var h codecSelfer1977
    z, r := codec1978.GenHelper().Decoder(d)
    _, _, _ = h, z, r
    var yyhl3 bool = l >= 0
    for yyj3 := 0; ; yyj3++ {
        if yyhl3 {
            if yyj3 >= l {
                break
            }
        } else {
            if z.DecCheckBreak() {
                break
            }
        }
        z.DecReadMapElemKey()
        yys3 := z.StringView(r.DecodeStringAsBytes())
        z.DecReadMapElemValue()
        switch yys3 {
        case "MaybeEmpty":
            if yyxt5 := z.Extension(x.MaybeEmpty); yyxt5 != nil {
                z.DecExtension(&x.MaybeEmpty, yyxt5)
            } else {
                z.DecFallback(&x.MaybeEmpty, false)
            }
        default:
            z.DecStructFieldNotFound(-1, yys3)
        } // end switch yys3
    } // end for yyj3
}

func (x *User) codecDecodeSelfFromArray(l int, d *codec1978.Decoder) {
    var h codecSelfer1977
    z, r := codec1978.GenHelper().Decoder(d)
    _, _, _ = h, z, r
    var yyj6 int
    var yyb6 bool
    var yyhl6 bool = l >= 0
    yyj6++
    if yyhl6 {
        yyb6 = yyj6 > l
    } else {
        yyb6 = z.DecCheckBreak()
    }
    if yyb6 {
        z.DecReadArrayEnd()
        return
    }
    z.DecReadArrayElem()
    if yyxt8 := z.Extension(x.MaybeEmpty); yyxt8 != nil {
        z.DecExtension(&x.MaybeEmpty, yyxt8)
    } else {
        z.DecFallback(&x.MaybeEmpty, false)
    }
    for {
        yyj6++
        if yyhl6 {
            yyb6 = yyj6 > l
        } else {
            yyb6 = z.DecCheckBreak()
        }
        if yyb6 {
            break
        }
        z.DecReadArrayElem()
        z.DecStructFieldNotFound(yyj6-1, "")
    }
}

func (x *User) IsCodecEmpty() bool {
    return !(!(x.MaybeEmpty.IsCodecEmpty()) && true)
}

func (x *S) IsCodecEmpty() bool {
    return !(x.A != "" && x.B != "" && true)
}
vanackere commented 3 years ago

I sent a pull request that should fix this issue, please merge ASAP if correct, this is a serious issue I think

myusuf3 commented 3 years ago

Would be great to get this in.