Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

codecgen: missing field with nested omitEmpty struct #344

Closed
vanackere opened this issue Nov 20, 2020 · 7 comments · Fixed by #345
Closed

codecgen: missing field with nested omitEmpty struct #344

vanackere opened this issue Nov 20, 2020 · 7 comments · Fixed by #345

Comments

@vanackere
Copy link
Contributor

vanackere commented Nov 20, 2020

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
Copy link
Owner

ugorji commented Nov 20, 2020

This looks like a real bug.

Let me look into it this weekend.

@ugorji
Copy link
Owner

ugorji commented Nov 20, 2020

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

@vanackere
Copy link
Contributor Author

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 vanackere changed the title codecgen: missing field with RecursiveEmptyCheck codecgen: missing field with nested omitEmpty struct Nov 20, 2020
@vanackere
Copy link
Contributor Author

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

@vanackere
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@myusuf3
Copy link

myusuf3 commented Nov 27, 2020

Would be great to get this in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants