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: Marshaler interface not producing "correct"/desired results #24466

Closed
pjebs opened this issue Mar 20, 2018 · 10 comments

Comments

Projects
None yet
4 participants
@pjebs
Copy link

commented Mar 20, 2018


type AWorker struct {
	ID        string  `json:"id"`
	Name      *string `json:"name"`
	AvatarURL *string `json:"avatar_url"`
}

type ReqUser struct {
	ExpireAt *time.Time `json:"expire_at"`
	Status   string     `json:"status"`
	Notes    *string    `json:"notes"`
}

func (r ReqUser) MarshalJSON() ([]byte, error) {
	out := map[string]interface{}{}
	out["status"] = r.Status
	out["notes"] = r.Notes
	if r.ExpireAt != nil {
		out["expire_at"] = (*r.ExpireAt).Format("2006-01-02")
	}
	return json.Marshal(out)
}

type AUser struct {
	AWorker
	ReqUser
}

@pjebs

This comment has been minimized.

Copy link
Author

commented Mar 20, 2018

I want to json/Marshall a []AUser.

When I remove func (r ReqUser) MarshalJSON():

"users": [
    {
      "id": "PJ_WORKER1",
      "name": "PJ 1",
      "avatar_url": null,
      "expire_at": null,
      "status": "completed",
      "notes": null
    },
    {
      "id": "PJ_WORKER2",
      "name": "PJ 2",
      "avatar_url": null,
      "expire_at": "2020-12-31T00:00:00Z",
      "status": "incomplete",
      "notes": null
    },
    {
      "id": "PJ_WORKER3",
      "name": "PJ 3",
      "avatar_url": null,
      "expire_at": "2018-12-31T00:00:00Z",
      "status": "completed",
      "notes": "id looks fake"
    }
  ]
@pjebs

This comment has been minimized.

Copy link
Author

commented Mar 20, 2018

When I keep func (r ReqUser) MarshalJSON():

"users": [
    {
      "expire_at": null,
      "notes": null,
      "status": "completed"
    },
    {
      "expire_at": "2020-12-31",
      "notes": null,
      "status": "incomplete"
    },
    {
      "expire_at": "2018-12-31",
      "notes": "id looks fake",
      "status": "completed"
    }
  ]
@pjebs

This comment has been minimized.

Copy link
Author

commented Mar 20, 2018

if this is technically correct behaviour, then perhaps for Go2, it should be changed to produce "desired" results.

@dmitshur dmitshur changed the title json unmarshaller interface not producing "correct"/desired results encoding/json: Unmarshaler interface not producing "correct"/desired results Mar 20, 2018

@dmitshur

This comment has been minimized.

Copy link
Member

commented Mar 20, 2018

Which one is the desired or expected result?

@pjebs

This comment has been minimized.

Copy link
Author

commented Mar 21, 2018

AUser has 2 structs embedded. It should display json fields from AWorker and ReqUser, but for ReqUser it should produce the results from the MarshalJSON() function (i.e. the corrected date format)

@dmitshur dmitshur changed the title encoding/json: Unmarshaler interface not producing "correct"/desired results encoding/json: Marshaler interface not producing "correct"/desired results Mar 21, 2018

@dmitshur

This comment has been minimized.

Copy link
Member

commented Mar 21, 2018

I see. Here's a runnable version on the playground: https://play.golang.org/p/FUBH0ZlswaL.

What's happening is ReqUser struct is being embedded into AUser, and as a result, AUser implements the json.Marshaler interface via ReqUser.MarshalJSON method. That way, all the AWorker fields get left out when AUser gets JSON marshaled.

It's working correctly given the rules of struct field embedding and json.Marshaler interface, but I can see how the two features are not interacting in an orthogonal and helpful way. I'm not sure how this can be improved, but perhaps other people will have ideas.

@dsnet

This comment has been minimized.

Copy link
Member

commented Mar 21, 2018

Given a struct type T with an embedded type R, it is impossible to determine whether T.Method is actually explicitly defined on T or is forwarded from R. The forwarding of methods in addition to fields is not really a gotcha of the json package but embedding in general.

If your intention to to forward only the fields, but not the methods, you can do this:

type aWorker AWorker
type reqUser ReqUser

type AUser struct {
	aWorker
	reqUser
}

Declaring aWorker and reqUser creates two new struct types that have the exact same layout as AWorker and ReqUser, but none of the methods.

I'm closing this as a duplicate of #22013.

@pjebs

This comment has been minimized.

Copy link
Author

commented Mar 21, 2018

@dsnet I require the methods to be attached, because encoding/json requires the custom MarshalJSON() function so I can modify the marshalled output.

@dsnet

This comment has been minimized.

Copy link
Member

commented Mar 21, 2018

What you want is currently not possible since you want to forward the fields of ReqUser, but don't want to forward the methods. At the same time you don't want to remove the ReqUser.MarshalJSON method.

Either way, this is still a language issue with embedding and #22013 is the place to rethink embedding for Go2.

@dsnet

This comment has been minimized.

Copy link
Member

commented Mar 21, 2018

I should also note that this is related to #6213, which may be the reason why you are embedding in the first place.

@golang golang locked and limited conversation to collaborators Mar 21, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.