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

encoding/json: promoted Unmarshal method on embedded field caused confusion #39470

Closed
talbspx opened this issue Jun 9, 2020 · 7 comments
Closed

Comments

@talbspx
Copy link

@talbspx talbspx commented Jun 9, 2020

What version of Go are you using (go version)?

$ go version 
go1.13.4 darwin/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/talbenshabtay/Library/Caches/go-build"
GOENV="/Users/talbenshabtay/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/talbenshabtay/Desktop/workspace/dev/go"
GOPRIVATE=""
GOPROXY="direct"
GOROOT="/usr/local/Cellar/go/1.13.4/libexec"
GOSUMDB="off"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.13.4/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/mr/vdtvd9f91m91tzm8k38r14ww0000gn/T/go-build469092631=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I wanted to add custom logic for json unmarshaling of a nested struct.
https://play.golang.org/p/RruGGGiWmN-

What did you expect to see?

i expected to have a complete object deserialized from the json string.
in the playground example, the field Num should have been valued as 5
along with the nested Dur field.

What did you see instead?

only the nested struct deserialized.
in the playground example, only the nested Dur field was deserialized correctly and
the composite object fields , Num , are zero valued.

@hantmac
Copy link

@hantmac hantmac commented Jun 9, 2020

In func (o *Object) UnmarshalJSON(data []byte) you will get: 'cannot unmarshal string into Go struct field .duration of type time.Duration' here, so it won't exec the next code, so the tmp.Num will be zero. Some suggestion:
https://play.golang.org/p/uOvbPBbnYDF

@talbspx
Copy link
Author

@talbspx talbspx commented Jun 9, 2020

@hantmac did u run the program ?
it executes just fine and prints out

parsing nested json {"num":5,"duration":"5s"} 
result: {Nested:{Dur:5s} Num:0} 

Program exited.

and uncommenting the Object unmarshalling results with :

parsing object json {"num":5,"duration":"5s"} 
parsing nested json {"num":5,"duration":"5s"} 
tmp object: {Nested:{Dur:5s} Num:0} 
result: {Nested:{Dur:5s} Num:0} 

Program exited.
@hantmac
Copy link

@hantmac hantmac commented Jun 9, 2020

@hantmac did u run the program ?
it executes just fine and prints out

parsing nested json {"num":5,"duration":"5s"} 
result: {Nested:{Dur:5s} Num:0} 

Program exited.

Of course, executes just fine because you did not handle the error in _ = json.Unmarshal([]byte(testJSON), &obj), just print out the error of if err := json.Unmarshal(data, &tmp); err != nil { // return err // }

@talbspx
Copy link
Author

@talbspx talbspx commented Jun 9, 2020

@hantmac
fair enough , but still i dont see an error.
https://play.golang.org/p/GsgQ28FBIBn

parsing object json {"num":5,"duration":"5s"} 
parsing nested json {"num":5,"duration":"5s"} 
tmp object: {Nested:{Dur:5s} Num:0} 
result: {Nested:{Dur:5s} Num:0} 

Program exited.
@hantmac
Copy link

@hantmac hantmac commented Jun 9, 2020

@talbspx Sorry,I misunderstood what you meant. But, the reason that you can't get what you expect is the order of execution of the code is obj.nested.Unmarshal, so it only unmarshal nested struct and the Num field won't be unmarshaled. If you wanna get what you expect, try this code: https://play.golang.org/p/QAwxENRiRAI

@odeke-em
Copy link
Member

@odeke-em odeke-em commented Jun 10, 2020

@talbspx thank you for this question and welcome to the Go project!
@hantmac thank you for the responses!

@talbspx, the technicality that's subtle here is that your embedded struct Nested's UnmarshalJSON method is promoted when you used embedded structs and is instead being used which means that without a top-level (*Object).UnmarshalJSON method, unmarshalling to Num will never work. You can verify this behavior by adding this single line anywhere in your code

var _ json.Unmarshaler = (*Object)(nil)

To fix your problem there are a couple of ways:
a) Edited your code to add (*Object).UnmarshalJSON https://play.golang.org/p/PElmUONvyDB

package main

import (
	"encoding/json"
	"fmt"
	"time"
)

var testJSON = `{"num":5,"duration":"5s"}`

type Nested struct {
	Dur time.Duration `json:"duration"`
}

func (obj *Object) UnmarshalJSON(data []byte) error {
	tmp := struct {
		Dur string `json:"duration"`
		Num int    `json:"num"`
	}{}
	if err := json.Unmarshal(data, &tmp); err != nil {
		return err
	}

	dur, err := time.ParseDuration(tmp.Dur)
	if err != nil {
		return err
	}
	obj.Dur = dur
	obj.Num = tmp.Num
	return nil
}

type Object struct {
	Nested
	Num int `json:"num"`
}

var _ json.Unmarshaler = (*Object)(nil)

func main() {
	obj := Object{}
	_ = json.Unmarshal([]byte(testJSON), &obj)
	fmt.Printf("result: %+v \n", obj)
}

b) Add a custom time unmarshaller https://play.golang.org/p/ALzsum_E-hw

package main

import (
	"encoding/json"
	"fmt"
	"time"
)

var testJSON = `{"num":5,"duration":"5s"}`

type customTimeDuration time.Duration

type Nested struct {
	Dur customTimeDuration `json:"duration"`
}

func (ctd *customTimeDuration) UnmarshalJSON(b []byte) error {
	var durStr string
	if err := json.Unmarshal(b, &durStr); err != nil {
		return err
	}
	dur, err := time.ParseDuration(durStr)
	if err == nil {
		*ctd = customTimeDuration(dur)
	}
	return err
}

type Object struct {
	Nested
	Num int `json:"num"`
}

func main() {
	obj := Object{}
	_ = json.Unmarshal([]byte(testJSON), &obj)
	fmt.Printf("result: %+v \n", obj)
}

Hope that resolves your questions, but the tip you can use to verify whether a promoted method will be invoked is with type assertions, and also remembering the relationship between promoted methods and embedded fields. The type assertion will be a nice guide

var _ json.Unmarshaler = (*Object)(nil)
var _ json.Unmarshaler = (*Nested)(nil)

that method will also later on help with catching bugs that manifest at runtime and might not easily be caught because they are embedded into lots of logic. Some 3 years ago when someone on my team encountered similar problems and after I examined Go bugs I wrote this guide that could be helpful https://medium.com/@odeke_et/compile-type-assertions-to-the-rescue-6ddab4b8398b

Asking questions

We use the Go issue tracker for actual bugs in Go. In the future, please ask questions on any of these resources https://github.com/golang/go/wiki/Questions
Stack Overflow with questions tagged "go"

  • The Go Forum, a web-based forum
  • Gophers Slack, use the invite app for access. The #general channel is a good starting point.
    -Go Community on Hashnode with questions and posts tagged with "go"
  • The golang-nuts mailing list
  • IRC channel #go-nuts on Freenode

Thank you again, and cheers!

@odeke-em odeke-em changed the title issue - json unmarshaling of nested struct zero values the composite struct encoding/json: promoted Unmarshal method on embedded field caused confusion Jun 10, 2020
@odeke-em odeke-em closed this Jun 10, 2020
@talbspx
Copy link
Author

@talbspx talbspx commented Jun 10, 2020

@odeke-em
thank you so much for your very detailed response !
i wanted to avoid option 1 because i actually use the embedded struct in many different structs so it will mean i will have to make an UnmarshalJSON and its representative "json" struct.
however your workaround on option 2 is clever :) 👍

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

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.