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

Unmarshaling on unions with same parameter name #64

Open
droidpl opened this issue May 7, 2020 · 3 comments
Open

Unmarshaling on unions with same parameter name #64

droidpl opened this issue May 7, 2020 · 3 comments
Labels

Comments

@droidpl
Copy link

droidpl commented May 7, 2020

@dmitshur Thanks for the nice library on graphql and GitHub.

What is the problem?

I am trying to query for the audit log actor and that part of the API uses unions. The relevant part of the query is the actor one so I will omit the rest. Actors can be bots, organizations or users, so it uses 3 types of unions.

type EntryActor struct {
	Typename     string `graphql:"__typename"`
	Bot          `graphql:"...on Bot" json:",omitempty"`
	Organization `graphql:"... on Organization" json:",omitempty"`
	User         `graphql:"... on User" json:",omitempty"`
}

The problem comes when each of this elements have fields that are common (like login, createdAt or updatedAt), as the parser don't know where to do the parsing and if a specific field is repeated in multiple embedded structs it gets ommited (see documentation below).

The Go visibility rules for struct fields are amended for JSON when deciding which field to marshal or unmarshal. If there are multiple fields at the same level, and that level is the least nested (and would therefore be the nesting level selected by the usual Go rules), the following extra rules apply:

  1. Of those fields, if any are JSON-tagged, only tagged fields are considered, even if there are multiple untagged fields that would otherwise conflict.

  2. If there is exactly one field (tagged or not according to the first rule), that is selected.

  3. Otherwise there are multiple fields, and all are ignored; no error occurs.

I also thought of creating a custom unmarshal function, but it seems not to work due to the way jsonutil.unmarshalGraphQL behaves as it used the custom decoder. See the function below:

func (actor *EntryActor) UnmarshalJSON(data []byte) error {
	// Ignore null, like in the main JSON package.
	if string(data) == "null" {
		return nil
	}
	var top struct {
		Typename string
	}
	if err := json.Unmarshal(data, &top); err != nil {
		return err
	}
	actor.Typename = top.Typename
	var v interface{}
	switch top.Typename {
	case "User":
		v = &actor.User
	case "Org":
		v = &actor.Organization
	case "Bot":
		v = &actor.Bot
	}
	if err := json.Unmarshal(data, &v); err != nil {
		return err
	}
	return nil
}

Is there a way I could workaround this so I an tell the parser how to parse each of those by __typename? With the current implementation seems hard to do properly queries against the auditLog which makes an intensive use of unions.

Code and query

The graphql query I am testing would look like:

query($org: String!) {
  organization(login: $org) {
    auditLog(first: 20){
      nodes {
        ... on AuditEntry {
          actor {
            __typename
            ... on Actor {
	      createdAt
              login
              resourcePath
              url
            }
            ... on User {
	      createdAt
              login
              resourcePath
              url
            }
            ... on Bot {
	      createdAt
              login
              resourcePath
              url
            }
          }
        }
      }
    }
  }
}

And all the go structs:

allExistingEntriesQuery struct {
	Organization struct {
		AuditLog struct {
			PageInfo struct {
				EndCursor   graphql.String
				HasNextPage bool
			}
			Nodes LogEntries
		} `graphql:"auditLog(first: 1, after: $page)"`
	} `graphql:"organization(login: $login)"`
}


type LogEntries []LogEntry

type LogEntry struct {
	Entry `graphql:"... on AuditEntry"`
}

type Entry struct {
	Actor             *EntryActor    `json:",omitempty"`
}

type EntryActor struct {
	Typename     string       `graphql:"__typename"`
	Bot          Bot          `graphql:"...on Bot" json:",omitempty"`
	Organization Organization `graphql:"... on Organization" json:",omitempty"`
	User         User         `graphql:"... on User" json:",omitempty"`
}

type User struct {
	CreatedAt                       *time.Time                    `json:",omitempty"`
	Login                      *string          `json:",omitempty"`
	ResourcePath               *string          `json:",omitempty"`
	Url                        *string          `json:",omitempty"`
}

type Organization struct {
	CreatedAt                       *time.Time                    `json:",omitempty"`
	Login                           *string                       `json:",omitempty"`
	ResourcePath                    *string                       `json:",omitempty"`
	Url                             *string                       `json:",omitempty"`
}

type Bot struct {
	CreatedAt                       *time.Time                    `json:",omitempty"`
	Login                           *string                       `json:",omitempty"`
	ResourcePath                    *string                       `json:",omitempty"`
	Url                             *string                       `json:",omitempty"`
}

Result: using named properties

If I use Bot, User and Organization named the result duplicated the values on each of them:

[
    {
        "Actor": {
            "Typename": "User",
            "Bot": {
                "CreatedAt": "2012-08-13T10:57:11Z",
                "Login": "droidpl",
                "ResourcePath": "/droidpl",
                "Url": "https://github.com/droidpl"
            },
            "Organization": {
                "CreatedAt": "2012-08-13T10:57:11Z",
                "Login": "droidpl",
                "ResourcePath": "/droidpl",
                "Url": "https://github.com/droidpl"
            },
            "User": {
                "CreatedAt": "2012-08-13T10:57:11Z",
                "Login": "droidpl",
                "ResourcePath": "/droidpl",
                "Url": "https://github.com/droidpl"
            }
        }
    }
]

While this should be only parsing the User part as __typename references, not each of the repeated elements

Result: with unnamed properties

If I use as EntryActor the properties unnamed:

type EntryActor struct {
	Typename     string       `graphql:"__typename"`
	Bot          `graphql:"...on Bot" json:",omitempty"`
	Organization `graphql:"... on Organization" json:",omitempty"`
	User         `graphql:"... on User" json:",omitempty"`
}

It creates a collision and given the rule in the documentation, the field gets ignored and the result looks like:

[
    {
        "Actor": {
            "Typename": "User"
        }
    }
]
@droidpl
Copy link
Author

droidpl commented May 7, 2020

Seems this could be an error related with #10 as there it says this case should be supported

@dmitshur
Copy link
Member

dmitshur commented May 15, 2020

I'm not sure if I understand what exactly you're asking about, so let me try to clarify.

When there are fields with same names in multiple inline fragments, it's normal that all of them will be populated. That's the intended behavior as implemented in PR #15.

Your code can do a query that populates EntryActor and then check the Typename field to know where to look for the rest of the content:

var ea EntryActor
// make a GraphQL query that populates ea ...
switch ea.Typename {
case "Bot":
    // use ea.Bot ...
case "Organization":
    // use ea.Organization ...
case "User":
    // use ea.User ...
}

Is there something that you're not able to do this way? Or is this issue about something else?

@droidpl
Copy link
Author

droidpl commented May 17, 2020

The problem for my use case is that looking at the structure, you get an instance in all of them (Bot, Organization and User). I wanted to marshal the structure as json to send it somewhere else, however I get a lot of elements that are duplicated because of the parsing, so I have to go against the full tree to remove all the duplicities. Wouldn't be a way to tweak the parser so that only the right ones are there based on the Typename?

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

No branches or pull requests

2 participants