graphql icon indicating copy to clipboard operation
graphql copied to clipboard

Query and Mutation methods ignore query content when it's wrapped in an interface

Open JonasDoe opened this issue 6 months ago • 3 comments

The following code won't work

var query any

// imagine a switch-case pattern here which dynamically decides what the query actually is
query = struct {
	Hero struct {
		Name  graphql.String
		Droid struct {
			PrimaryFunction graphql.String
		} `graphql:"... on Droid"`
		Human struct {
			Height graphql.Float
		} `graphql:"... on Human"`
	} `graphql:"hero(episode: \"JEDI\")"`
}{}

err := a.client.Query(context.Background(), &query, nil)

The request body will look like {"query":""}. I set a query string in the graphql tag, it would appear here - but the requested fields etc. would still be missing. The problem is probably that the type of query is now interface{} | struct {...} (or interface{} | HeroQuery with an explicit type declaration), and only type interface{} is regarded.

Edit: I had a look into the code. Seems like you only process reflect.Ptr, reflect.Type and reflect.Struct in the function writeQuery. The kind here is (pointer to) interface{}. Reflection is such a pain in Go. :/

JonasDoe avatar Dec 05 '23 15:12 JonasDoe

Thanks for reporting this. I'll take a look.

dmitshur avatar Dec 05 '23 15:12 dmitshur

The documentation for Query currently includes:

q should be a pointer to struct that corresponds to the GraphQL schema.

To make the snippet you've shown work as is, that would need to change to something like this:

q should be a pointer to struct that corresponds to the GraphQL schema, or a pointer to an interface holding a struct that corresponds to the GraphQL schema.

That seems to make the API more complex.

But maybe it's possible to make this use-case work without needing to do that, like so:

 var query any

 // imagine a switch-case pattern here which dynamically decides what the query actually is
-query = struct {
+query = &struct {
 	Hero struct {
 		Name  graphql.String
 		Droid struct {
 			PrimaryFunction graphql.String
 		} `graphql:"... on Droid"`
 		Human struct {
 			Height graphql.Float
 		} `graphql:"... on Human"`
 	} `graphql:"hero(episode: \"JEDI\")"`
 }{}
 
-err := a.client.Query(context.Background(), &query, nil)
+err := a.client.Query(context.Background(), query, nil)

The difference is that the query (type any, i.e., empty interface) is being passed in directly to Query, but the value its being assigned is a pointer to a struct. Do you think that could work for you?

TestClient_Query_Issue117

For reference, here's a test I used to check that the above should work:

func TestClient_Query_Issue117(t *testing.T) {
	mux := http.NewServeMux()
	mux.HandleFunc("/graphql", func(w http.ResponseWriter, req *http.Request) {
		body := mustRead(req.Body)
		if got, want := body, `{"query":"{user{name}}"}`+"\n"; got != want {
			t.Errorf("got body: %v, want %v", got, want)
		}
		w.Header().Set("Content-Type", "application/json")
		mustWrite(w, `{"data": {"user": {"name": "Gopher"}}}`)
	})
	client := graphql.NewClient("/graphql", &http.Client{Transport: localRoundTripper{handler: mux}})

	var q any = &struct {
		User struct {
			Name string
		}
	}{}
	err := client.Query(context.Background(), q, map[string]any{})
	if err != nil {
		t.Fatal(err)
	}
	if got, want := reflect.ValueOf(q).Elem().FieldByName("User").FieldByName("Name").Interface().(string), "Gopher"; got != want {
		t.Errorf("got q.User.Name: %q, want: %q", got, want)
	}
}

dmitshur avatar Dec 06 '23 03:12 dmitshur

Hey, thanks for the fast response! That would surely work for me. Regarding the underlying questions - whether the API would become more complex -, I'ld slightly disagree, though. The documentation might become longer b/c the list of supported cases increases, but the API itself wouldn't change a bit and just becomes more robust. It's not like it would introduce more requirements - quite the opposite, it becomes more lenient about the passed type, and produces (for my taste) less suprising behavior. But it's your call ofc., personally I'm fine with working with pointers! :)

JonasDoe avatar Dec 06 '23 09:12 JonasDoe