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

Issue with embedded Struct response #170

Open
SDkie opened this issue Nov 26, 2016 · 23 comments · May be fixed by #371
Open

Issue with embedded Struct response #170

SDkie opened this issue Nov 26, 2016 · 23 comments · May be fixed by #371

Comments

@SDkie
Copy link

SDkie commented Nov 26, 2016

type Address struct {
City string json:"city"
State string json:"state"
}

type User struct {
Name string json:"name"
Email string json:"email"
Address
}

My resolver is returning User struct, And I am getting following response :
{
"name":"Jon",
"email":"jon@email.com",
"city":null,
"state":null
}

Address is common struct shared by many sql tables.
How can I fix city and state response?

@SDkie SDkie changed the title Fix response of embedded Struct Issue with embedded Struct response Nov 26, 2016
@bbuck
Copy link
Collaborator

bbuck commented Nov 26, 2016

How is your GraphQL Object defined? What are the resolve functions specified?

@SDkie
Copy link
Author

SDkie commented Nov 26, 2016

@bbuck : My sample code is here - https://play.golang.org/p/wAJA_MPGPU
Response:

{
	"data": {
		"getUser": {
			"city": null,
			"email": "jon@gmail.com",
			"name": "Jon",
			"state": null
		}
	}
}

@bbuck
Copy link
Collaborator

bbuck commented Nov 26, 2016

So that's kind of what I was thinking. You're not implementing a resolver for the "city" and "state" fields. I don't think it's really in the scope of the default resolver to search through nested struct fields, especially those that don't have a JSON struct tag specifying the name. Two solutions would be to either nest and "address" property, making the query:

{
  getUser {
    address {
      city
      state
    }
  }
}

The other is to implement the resolvers for those fields so they properly hunt the nested attribute correctly, you could make them generic functions by implementing an Addresser (or other named) interface and define a City() string and State() string so that your numerous data types can benefit from one Resolve function and just pass that as the Resolve in the object definition.

@SDkie
Copy link
Author

SDkie commented Nov 27, 2016

@bbuck : Can I keep city, state, name and email at same level instead of nesting?
Btw when I json.Marshal User object all fields are in the same level.

@bbuck
Copy link
Collaborator

bbuck commented Nov 28, 2016

If you want to, that's fine. Your resolver just needs to know where to get that data from.

Btw when I json.Marshal User object all fields are in the same level.

This is kind of fundamentally different. In this regard you're converting the entire struct into a JSON representation using struct tags to determine how to look at things. Here in GraphQL the default resolver is just piggybacking off the json struct tags to assume a field to resolve. However, if you define a field "city" on the User GraphQL object, then logically speaking the resolver should expect to find a field with json:"city" or named "City" on the struct User. I think it would be a significant amount of extra work if it hunted for the json:"city" on the structs fields, then any and all nested struct fields then looked for fields with the name --- etc.

At least that's how I see it. Resolving the object based on the config you can either match the the object definition to the struct or the struct to the object definition. But to deviate either layout from the other (like here with nested fields) you should be expected to provide a resolution function. Don't take my opinion as final, I just contribute here. Perhaps @chris-ramon or @sogko have their own ideas on how this should behave.

@dominicbarnes
Copy link

I ran into this problem today, and I think that embedded structs should be handled in a more transparent way. I agree that this behavior should mimic json.Marshal and json.Unmarshal, and embedded properties should be part of the object rather than nested.

@dominicbarnes
Copy link

dominicbarnes commented Aug 28, 2017

You can work around this by implementing field-level resolve functions, but it is pretty inconvenient and not particularly intuitive/obvious.

"city": &graphql.Field{
	Type: graphql.String,
	Resolve: func(p graphql.ResolveParams) (interface{}, error) {
		m := p.Source.(User)
		return m.City, nil
	},
},

@januszm
Copy link

januszm commented Jan 23, 2018

It'd be great to have an easy way of converting structs to simple (optionally nested) key : value pairs. It's probably a very popular use case to just dump a struct into an JSON API response.

@alexflint
Copy link
Contributor

I agree that the default resolver should automatically look for fields on nested structs.

@niondir
Copy link

niondir commented May 31, 2018

I'm also facing this with interfaces. I'm using embedded structs that contain all fields of the interface. When implementing a concrete type I need to implement the Resolve function as states above.

@lynic
Copy link

lynic commented Jul 31, 2018

Met the same issue, the default resolver didn't search for nested struct although json.Marshal works.

@niondir
Copy link

niondir commented Oct 15, 2018

Any news on this? Is it planned to search nested structs like json.Marshal does for default resolvers?

Or do I miss any way to somehow override the default resolver behaviour?

@imjma
Copy link

imjma commented Oct 16, 2018

@niondir I think this pr will fix it, but it stays there for a while #371

@abraithwaite
Copy link

I don't think it's really in the scope of the default resolver to search through nested struct fields

Is this still the official position? I know how annoying nested structs are to deal with as library authors, but as consumers you just kind of expect nested structs to work everywhere.

@niondir
Copy link

niondir commented Sep 24, 2019

could we actually merge #371 ?

@zhulinwei
Copy link

Has this problem been solved?

@emhohensee
Copy link

Running into this using jinzhu/gorm. GORM uses an embedded struct to DRY out common fields like ID, CreatedAt, UpdatedAt, and DeletedAt in database models. I'm having to create custom field revolvers for all of these fields on all types, which is just boilerplate noise. It would save many, many keystrokes to have graphql-go/graphql dive the embedded structs.

var UserType = gql.NewObject(gql.ObjectConfig{
	Name: "User",
	Fields: gql.Fields{
		"id": &gql.Field{
			Type: gql.Int,
			Resolve: func(p gql.ResolveParams) (i interface{}, err error) {
				user, ok := p.Source.(models.User)
				if !ok {
					return nil, errors.New("casting failed for user type")
				}
				return user.ID, nil
			},
		},
		"email":        &gql.Field{Type: gql.String},
		"dw_campaigns": &gql.Field{Type: gql.NewList(DwCampaignType)},
		"created_at": &gql.Field{
			Type: gql.DateTime,
			Resolve: func(p gql.ResolveParams) (i interface{}, err error) {
				user, ok := p.Source.(models.User)
				if !ok {
					return nil, errors.New("casting failed for user type")
				}
				return user.CreatedAt, nil
			},
		},
	},
})

vs

var UserType = gql.NewObject(gql.ObjectConfig{
	Name: "User",
	Fields: gql.Fields{
		"id":           &gql.Field{Type: gql.Int},
		"email":        &gql.Field{Type: gql.String},
		"dw_campaigns": &gql.Field{Type: gql.NewList(DwCampaignType)},
		"created_at":   &gql.Field{Type: gql.DateTime},
	},
})

@limoli
Copy link

limoli commented Apr 28, 2020

Since the JSON returned from a composite structure gives the correct values for a schema, there should be no issue. Instead, the problem persists and limit us very much.
We must fix this.

@mrdulin
Copy link

mrdulin commented Aug 11, 2020

same issue. I populated the ValidateAccessTokenResponse struct with RESTful API response data. But the fields of embed structs are NOT resolved. Only the id field is resolved.

Go struct

type ResponseStatus struct {
	Success bool `json:"success"`
}
type User struct {
	Loginname string `json:"loginname"`
	AvatarURL string `json:"avatar_url"`
}
type ValidateAccessTokenResponse struct {
        ResponseStatus
	User
	ID string `json:"id"`
}
type UserDetail struct {
	User
	GithubUsername string        `json:"githubUsername"`
	CreateAt       string        `json:"create_at"`
	Score          int           `json:"score"`
	RecentTopics   []RecentTopic `json:"recent_topics"`
}

Got data:

{
  "data": {
    "validateAccessToken": {
      "avatar_url": null,
      "id": "58e104cdc669764920c00964",
      "loginname": null
    }
  }
}

Solution:

var UserBaseFields = graphql.Fields{
	"loginname": &graphql.Field{
		Type: graphql.String,
		Resolve: func(p graphql.ResolveParams) (interface{}, error) {
			switch t := p.Source.(type) {
                         // Both t.User.Loginname and t.Loginname are ok.
			case *models.ValidateAccessTokenResponse:
				return t.User.Loginname, nil
			case *models.UserDetail:
				return t.Loginname, nil
			default:
				return graphql.DefaultResolveFn(p)
			}
		}},
	"avatar_url": &graphql.Field{
		Type: graphql.String,
	},
}

var UserType = graphql.NewObject(graphql.ObjectConfig{
	Name:        "User",
	Description: "This respresents an user",
	Fields:      UserBaseFields,
})

var UserDetailType = graphql.NewObject(graphql.ObjectConfig{
	Name:        "UserDetail",
	Description: "This respresents an user detail",
	Fields: utils.MergeGraphqlFields(UserBaseFields, graphql.Fields{
		"githubUsername": &graphql.Field{Type: graphql.String},
		"create_at":      &graphql.Field{Type: graphql.String},
		"score":          &graphql.Field{Type: graphql.Int},
		"recent_topics":  &graphql.Field{Type: graphql.NewList(RecentTopicType)},
	}),
})

var AccessTokenValidationType = graphql.NewObject(graphql.ObjectConfig{
	Name:        "AccessTokenValidation",
	Description: "The response type for validating accessToken",
	Fields: utils.MergeGraphqlFields(UserBaseFields, graphql.Fields{
		"id": &graphql.Field{Type: graphql.NewNonNull(graphql.ID)},
	}),
})

Since UserBaseFields is used by UserDetailType and AccessTokenValidationType. The return value(and type) of these resolvers are different. So I need to use type cast to get the different field from p.Source.

@rifqimf12
Copy link

not resolved yet?

@sheepwall
Copy link

Noticed this issue when trying to resolve a field using a struct tag in an embedded field, like previous commenters have described in further detail. Thanks to the commenters here for providing some workarounds!

@dumim
Copy link

dumim commented Mar 14, 2024

This is still happening

@taleeus
Copy link

taleeus commented Mar 20, 2024

Yes, very annoying 🥲

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.