Closed grihabor closed 2 years ago
I wanted to merge the feature into the upstream, but it looks a bit abandoned, so I found your fork. Do you mind checking out the feature? @hgiasac
IMO this feature is possible to do with the current client. In GraphQL specs we can use alias
to execute similar queries in a single request:
type CreateUser struct {
Login graphql.String
}
type CreateUsers struct {
CreateUser1 CreateUser `graphql:"createUser1: createUser(login: $login1)"`
CreateUser2 CreateUser `graphql:"createUser2: createUser(login: $login2)"`
CreateUser3 CreateUser `graphql:"createUser3: createUser(login: $login3)"`
}
I think your need to build multiple queries in a loop such as create many users. However in API design PoV it's better if we create new mutation resolver that accept array of inputs, for example:
mutation($logins: [String!]!) {
createUsers(logins: $logins) { login }
}
Btw, your idea could be better if we define a type-safe query builder interface, instead of using a 2-dimension interface slice that is not safe to me :sweat_smile:
Thank you for your quick response!)
Unfortunately, we can't use any of these workarounds. Maintainers of the graphql server don't want to expose array methods, so we need to use multiple mutations one by one, but it's not possible using structs, because we don't know the number of mutations at compile time, so we need some kind of array.
Builder is a good idea, but I wanted to add a feature to the existing API, not redesign the library from scratch
Why exactly you think 2-dimension interface slice is not safe? If you want more checks, I could add some through panics or error propagation, what do you think?
If the builder is the only option, could you show how you think it might look like?
@grihabor sorry for the late reply
It's fine with the interface array for internal use. However instead of using it directly. It's better to abstract [][2]interface{}
into a type.
type OperationComposer [][2]interface{}
func (oc *OperationComposer) Add(q interface{}, definition string) *OperationComposer
func NewOperationComposer() *OperationComposer
type CreateUser struct {
Login graphql.String
}
m := NewOperationComposer().
Add(&CreateUser{}, "createUser(login: $login1)").
Add(&CreateUser{}, "createUser(login: $login2)").
Add(&CreateUser{}, "createUser(login: $login3)")
_ = client.Mutate(context.Background(), &m, variables)
The benefit of this approach is avoiding developer use the internal types directly, and we can change the internal implementations without breaking changes
I'm ok with this solution, but reflect package doesn't know how to distinguish between the type [][2]interface{}
and type alias type OperationComposer [][2]interface{}
, so people might misuse the API and feed [][2]interface{}
. If you want to hide internal implementation I would suggest something like this
type Fields struct {
field []field
}
type field struct {
name string
value interface{}
}
I think generic Fields
name is better, because you can also use it instead of structs in queries, so it's not an operation
anymore:
{
inV: func() interface{} {
type query struct {
Repository [][2]interface{} `graphql:"repository(owner: $repositoryOwner, name: $repositoryName)"`
}
type issue struct {
ReactionGroups []struct {
Users [][2]interface{} `graphql:"users(first:10)"`
}
}
type nodes []struct {
Login String
}
return query{Repository: [][2]interface{}{
{"issue(number: $issueNumber)", issue{
ReactionGroups: []struct {
Users [][2]interface{} `graphql:"users(first:10)"`
}{
{Users: [][2]interface{}{
{"nodes", nodes{}},
}},
},
}},
}}
}(),
inVariables: map[string]interface{}{
"repositoryOwner": String("shurcooL-test"),
"repositoryName": String("test-repo"),
"issueNumber": Int(1),
},
want: `query ($issueNumber:Int!$repositoryName:String!$repositoryOwner:String!){repository(owner: $repositoryOwner, name: $repositoryName){issue(number: $issueNumber){reactionGroups{users(first:10){nodes{login}}}}}}`,
}
Wait, I just realised, how are you supposed to fill this kind of OperationComposer structure, if the user can't access it's fields? We could provide some kind of Convert() [][2]interface{}
method, but it's a bit ugly to use it all over the place in user code
You're right. Actually I have an idea about new type-safe interface option for query builder beside struct tags reflection. It may take more functions to convert the output, but easier to testing and debugging.
In your use case, we still have to access the interface pointer and do a conversion, right?
user1, ok := m[0][1].(*CreateUser)
Btw, this is a special use case that not many users use this approach. We can merge this for internal use though. We will discuss about improvements and better error handling in another topics
Thank you! Could you make a release?
Thanks!
Suppose I can create user like this
Now I'd like to create multiple users in single request
It would be convenient to implement it in code with ordered map
Implementation is in the pr