From 40890ab26ffb418e7dbca6dc440fdd5fa0dfef4e Mon Sep 17 00:00:00 2001 From: bkrukowski Date: Mon, 16 Sep 2024 13:31:37 +0400 Subject: [PATCH 1/3] fix: `DefaultResolveFn` should handle with `map[K]V` for `K` that has `string` as the underlying type The current implementation returns the following errors for maps with a custom type for keys: ``` reflect.Value.MapIndex: value of type string is not assignable to type .* ``` --- executor.go | 2 +- executor_test.go | 118 ++++++++++++++++++++++++++++++----------------- 2 files changed, 77 insertions(+), 43 deletions(-) diff --git a/executor.go b/executor.go index 7440ae21e..07ce33485 100644 --- a/executor.go +++ b/executor.go @@ -995,7 +995,7 @@ func DefaultResolveFn(p ResolveParams) (interface{}, error) { // Try accessing as map via reflection if r := reflect.ValueOf(p.Source); r.Kind() == reflect.Map && r.Type().Key().Kind() == reflect.String { - val := r.MapIndex(reflect.ValueOf(p.Info.FieldName)) + val := r.MapIndex(reflect.ValueOf(p.Info.FieldName).Convert(r.Type().Key())) if val.IsValid() { property := val.Interface() if val.Type().Kind() == reflect.Func { diff --git a/executor_test.go b/executor_test.go index 856aadf3e..eeb6b75ba 100644 --- a/executor_test.go +++ b/executor_test.go @@ -297,59 +297,93 @@ func TestMergesParallelFragments(t *testing.T) { } } -type CustomMap map[string]interface{} +type ( + CustomMap map[string]interface{} + + CustomKey string + CustomMap2 map[CustomKey]interface{} +) func TestCustomMapType(t *testing.T) { query := ` query Example { data { a } } ` - data := CustomMap{ - "a": "1", - "b": "2", + + scenarios := []struct { + query string + data interface{} + expected interface{} + }{ + { + query: query, + data: CustomMap{ + "a": "1", + "b": "2", + }, + expected: map[string]interface{}{ + "data": map[string]interface{}{ + "a": "1", + }, + }, + }, + { + query: query, + data: CustomMap2{ + "a": "1", + "b": "2", + }, + expected: map[string]interface{}{ + "data": map[string]interface{}{ + "a": "1", + }, + }, + }, } - schema, err := graphql.NewSchema(graphql.SchemaConfig{ - Query: graphql.NewObject(graphql.ObjectConfig{ - Name: "RootQuery", - Fields: graphql.Fields{ - "data": &graphql.Field{ - Type: graphql.NewObject(graphql.ObjectConfig{ - Name: "Data", - Fields: graphql.Fields{ - "a": &graphql.Field{ - Type: graphql.String, - }, - "b": &graphql.Field{ - Type: graphql.String, + + for i, s := range scenarios { + s := s + + t.Run(fmt.Sprintf("Scenario #%d", i), func(t *testing.T) { + schema, err := graphql.NewSchema(graphql.SchemaConfig{ + Query: graphql.NewObject(graphql.ObjectConfig{ + Name: "RootQuery", + Fields: graphql.Fields{ + "data": &graphql.Field{ + Type: graphql.NewObject(graphql.ObjectConfig{ + Name: "Data", + Fields: graphql.Fields{ + "a": &graphql.Field{ + Type: graphql.String, + }, + "b": &graphql.Field{ + Type: graphql.String, + }, + }, + }), + Resolve: func(p graphql.ResolveParams) (interface{}, error) { + return s.data, nil }, }, - }), - Resolve: func(p graphql.ResolveParams) (interface{}, error) { - return data, nil }, - }, - }, - }), - }) - if err != nil { - t.Fatalf("Error in schema %v", err.Error()) - } + }), + }) + if err != nil { + t.Fatalf("Error in schema %v", err.Error()) + } - result := testutil.TestExecute(t, graphql.ExecuteParams{ - Schema: schema, - Root: data, - AST: testutil.TestParse(t, query), - }) - if len(result.Errors) > 0 { - t.Fatalf("wrong result, unexpected errors: %v", result.Errors) - } + result := testutil.TestExecute(t, graphql.ExecuteParams{ + Schema: schema, + Root: s.data, + AST: testutil.TestParse(t, s.query), + }) + if len(result.Errors) > 0 { + t.Fatalf("wrong result, unexpected errors: %v", result.Errors) + } - expected := map[string]interface{}{ - "data": map[string]interface{}{ - "a": "1", - }, - } - if !reflect.DeepEqual(result.Data, expected) { - t.Fatalf("Expected context.key to equal %v, got %v", expected, result.Data) + if !reflect.DeepEqual(result.Data, s.expected) { + t.Fatalf("Expected context.key to equal %v, got %v", s.expected, result.Data) + } + }) } } From aeeccbb6350064ef7018e96540c371d00a44e868 Mon Sep 17 00:00:00 2001 From: bkrukowski Date: Sun, 22 Sep 2024 22:17:44 +0400 Subject: [PATCH 2/3] fix: `DefaultResolveFn` should handle with `map[K]V` for `K` that has `string` as the underlying type The current implementation returns the following errors for maps with a custom type for keys: ``` reflect.Value.MapIndex: value of type string is not assignable to type .* ``` --- executor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executor.go b/executor.go index 07ce33485..7440ae21e 100644 --- a/executor.go +++ b/executor.go @@ -995,7 +995,7 @@ func DefaultResolveFn(p ResolveParams) (interface{}, error) { // Try accessing as map via reflection if r := reflect.ValueOf(p.Source); r.Kind() == reflect.Map && r.Type().Key().Kind() == reflect.String { - val := r.MapIndex(reflect.ValueOf(p.Info.FieldName).Convert(r.Type().Key())) + val := r.MapIndex(reflect.ValueOf(p.Info.FieldName)) if val.IsValid() { property := val.Interface() if val.Type().Kind() == reflect.Func { From 921fc06deb9ae66a99671904622403ebd3708616 Mon Sep 17 00:00:00 2001 From: bkrukowski Date: Sun, 22 Sep 2024 22:22:31 +0400 Subject: [PATCH 3/3] fix: `DefaultResolveFn` should handle with `map[K]V` for `K` that has `string` as the underlying type The current implementation returns the following errors for maps with a custom type for keys: ``` reflect.Value.MapIndex: value of type string is not assignable to type .* ``` --- executor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executor.go b/executor.go index 7440ae21e..07ce33485 100644 --- a/executor.go +++ b/executor.go @@ -995,7 +995,7 @@ func DefaultResolveFn(p ResolveParams) (interface{}, error) { // Try accessing as map via reflection if r := reflect.ValueOf(p.Source); r.Kind() == reflect.Map && r.Type().Key().Kind() == reflect.String { - val := r.MapIndex(reflect.ValueOf(p.Info.FieldName)) + val := r.MapIndex(reflect.ValueOf(p.Info.FieldName).Convert(r.Type().Key())) if val.IsValid() { property := val.Interface() if val.Type().Kind() == reflect.Func {