shurcooL / githubv4

Package githubv4 is a client library for accessing GitHub GraphQL API v4 (https://docs.github.com/en/graphql).
MIT License
1.1k stars 89 forks source link

Potential Bug in mapping results back into structs or usage error #79

Open marcofranssen opened 3 years ago

marcofranssen commented 3 years ago

When I run following query manually in my graphql playground all works as expected.

query($enterprise:String!$identityCursor:String$organization:String!){
  enterprise(slug: $enterprise){
    ownerInfo{
      samlIdentityProvider{
        ssoUrl,
        externalIdentities(first: 100, after: $identityCursor){
          edges{
            node{
              samlIdentity{
                nameId,
                username
              },
              user{
                id,
                login,
                name,
                organization(login: $organization){
                  name
                }
              }
           }
         },
         pageInfo{
           hasNextPage,
           endCursor
         }
       }
     }
    }
  }
}

This query is what I took from my logs, when running my code.

However if I run this from the code for a whole bunch of records my structs are note populated with the values I see from the manual query result.

So for some records e.g. my identity.User.Organization.Name is empty while for some other records it populates just fine.

See below the go structs

type Enterprise struct {
    OwnerInfo OwnerInfo `json:"ownerInfo,omitempty"`
}

type OwnerInfo struct {
    SamlIdentityProvider SamlIdentityProvider `json:"samlIdentityProvider,omitempty"`
}

type SamlIdentityProvider struct {
    SsoURL             string             `json:"ssoUrl,omitempty"`
    ExternalIdentities ExternalIdentities `graphql:"externalIdentities(first: 100, after: $identityCursor)" json:"externalIdentities,omitempty"`
}

type ExternalIdentities struct {
    Edges    []ExternalIdentityEdge `json:"edges,omitempty"`
    PageInfo PageInfo               `json:"pageInfo,omitempty"`
}

type ExternalIdentityEdge struct {
    Node ExternalIdentityNode `json:"node,omitempty"`
}

type ExternalIdentityNode struct {
    SamlIdentity SamlIdentity `json:"samlIdentity,omitempty"`
    User         Member       `json:"user,omitempty"`
}

type SamlIdentity struct {
    NameId   string `json:"nameId,omitempty"`
    Username string `json:"username,omitempty"`
}

type Member struct {
    ID           string           `json:"id,omitempty"`
    Login        string           `json:"login,omitempty"`
    Name         string           `json:"name,omitempty"`
    Organization OrganizationName `graphql:"organization(login: $organization)" json:"organization,omitempty"`
}

type OrganizationName struct {
    Name string `json:"name,omitempty"`
}

And this is the code to run the request.

var q struct {
        Enterprise graphql.Enterprise `graphql:"enterprise(slug: $enterprise)"`
    }

    variables := map[string]interface{}{
        "enterprise":     githubv4.String(enterprise),
        "organization":   githubv4.String(organization),
        "identityCursor": (*githubv4.String)(nil),
    }

    var identities []graphql.ExternalIdentityNode
    for {
        err := c.Query(ctx, &q, variables)
        if err != nil {
            return nil, err
        }
        idp := q.Enterprise.OwnerInfo.SamlIdentityProvider
        identities = append(identities, identityEdges(idp.ExternalIdentities.Edges)...)
        if !idp.ExternalIdentities.PageInfo.HasNextPage {
            break
        }

        variables["identityCursor"] = githubv4.NewString(idp.ExternalIdentities.PageInfo.EndCursor)
    }

Why are some of these organization fields empty in some of the records while others populate just fine? Yes I did check by running the query manually to see if the data combing back from the api is there.

marcofranssen commented 3 years ago

@dmitshur do you have any clue? Or any lead where to continue.

dmitshur commented 3 years ago

You're reusing the q variable between iterations. That means for the second page and onwards, it'll be non-zero as it contains data from the last page. I'm not sure how well supported that is (and it's a TODO to improve that).

As a first step, I'd suggest trying to move the variable into the for loop so its value is always zero before making the query:

for {
    var q struct { ... }
    // ...
    err := c.Query(&q, variables)

If that fixes the problem, then we'll know this is the cause.