Repository: jensneuse/graphql-go-tools
Pull request #299 (2021-11-01 16:31:37)

fix resolving array of strings with stringified number
Fix an issue #298 .

`jsonparser.Get` [strips](https://github.com/buger/jsonparser/blob/master/parser.go#L974) quotes from string value, in case if `jsonparser.Get` is called outside of `resolveString` method, there is no way to distinguish if value is stringified number ("123") or number (123). 


Patch:
diff --git a/pkg/engine/resolve/resolve.go b/pkg/engine/resolve/resolve.go
index cfd50468..9f73cb4b 100644
--- a/pkg/engine/resolve/resolve.go
+++ b/pkg/engine/resolve/resolve.go
@@ -13,7 +13,6 @@ import (
 	"strconv"
 	"sync"
 	"time"
-	"unicode"
 
 	"github.com/buger/jsonparser"
 	"github.com/cespare/xxhash"
@@ -681,6 +680,10 @@ func (r *Resolver) resolveArray(ctx *Context, array *Array, data []byte, arrayBu
 	}()
 
 	_, err = jsonparser.ArrayEach(data, func(value []byte, dataType jsonparser.ValueType, offset int, err error) {
+		if err == nil && dataType == jsonparser.String {
+			value = data[offset-2:offset+len(value)] // add quotes to string values
+		}
+
 		*arrayItems = append(*arrayItems, value)
 	}, array.Path...)
 
@@ -856,27 +859,16 @@ func (r *Resolver) resolveString(str *String, data []byte, stringBuf *BufPair) e
 		valueType jsonparser.ValueType
 		err       error
 	)
-	if len(data) != 0 && str.Path == nil {
-		_, valueType, _, _ = jsonparser.Get(data)
-		if valueType == jsonparser.String || unicode.IsLetter(rune(data[0])) {
-			value = data
-		} else if !str.Nullable {
+
+	value, valueType, _, err = jsonparser.Get(data, str.Path...)
+	if err != nil || valueType != jsonparser.String {
+		if !str.Nullable {
 			return errNonNullableFieldValueIsNull
-		} else {
-			r.resolveNull(stringBuf.Data)
-			return nil
-		}
-	}
-	if value == nil {
-		value, valueType, _, err = jsonparser.Get(data, str.Path...)
-		if err != nil || valueType != jsonparser.String {
-			if !str.Nullable {
-				return errNonNullableFieldValueIsNull
-			}
-			r.resolveNull(stringBuf.Data)
-			return nil
 		}
+		r.resolveNull(stringBuf.Data)
+		return nil
 	}
+
 	if value == nil && !str.Nullable {
 		return errNonNullableFieldValueIsNull
 	}


Test patch:
diff --git a/pkg/engine/resolve/resolve_test.go b/pkg/engine/resolve/resolve_test.go
index dd0d00af..bb254ed2 100644
--- a/pkg/engine/resolve/resolve_test.go
+++ b/pkg/engine/resolve/resolve_test.go
@@ -86,6 +86,23 @@ func TestResolver_ResolveNode(t *testing.T) {
 			ctrl.Finish()
 		}
 	}
+
+	testErrFn := func(fn func(t *testing.T, r *Resolver, ctrl *gomock.Controller) (node Node, ctx Context, expectedErr string)) func(t *testing.T) {
+		ctrl := gomock.NewController(t)
+		c, cancel := context.WithCancel(context.Background())
+		defer cancel()
+		r := New(c)
+		node, ctx, expectedErr := fn(t, r, ctrl)
+		return func(t *testing.T) {
+			buf := &BufPair{
+				Data:   fastbuffer.New(),
+				Errors: fastbuffer.New(),
+			}
+			err := r.resolveNode(&ctx, node, nil, buf)
+			assert.EqualError(t, err, expectedErr)
+			ctrl.Finish()
+		}
+	}
 	t.Run("Nullable empty object", testFn(func(t *testing.T, r *Resolver, ctrl *gomock.Controller) (node Node, ctx Context, expectedOutput string) {
 		return &Object{
 			Nullable: true,
@@ -235,6 +252,48 @@ func TestResolver_ResolveNode(t *testing.T) {
 			},
 		}, Context{Context: context.Background(), Variables: []byte(`{"id":1}`)}, `{"name":"Jens"}`
 	}))
+	t.Run("resolve array of strings", testFn(func(t *testing.T, r *Resolver, ctrl *gomock.Controller) (node Node, ctx Context, expectedOutput string) {
+		return &Object{
+			Fetch: &SingleFetch{
+				BufferId:   0,
+				DataSource: FakeDataSource(`{"strings": ["Alex", "true", "123"]}`),
+			},
+			Fields: []*Field{
+				{
+					BufferID:  0,
+					HasBuffer: true,
+					Name:      []byte("strings"),
+					Value: &Array{
+						Path: []string{"strings"},
+						Item: &String{
+							Nullable: false,
+						},
+					},
+				},
+			},
+		}, Context{Context: context.Background()}, `{"strings":["Alex","true","123"]}`
+	}))
+	t.Run("resolve array of mixed scalar types", testErrFn(func(t *testing.T, r *Resolver, ctrl *gomock.Controller) (node Node, ctx Context, expectedErr string) {
+		return &Object{
+			Fetch: &SingleFetch{
+				BufferId:   0,
+				DataSource: FakeDataSource(`{"strings": ["Alex", "true", 123]}`),
+			},
+			Fields: []*Field{
+				{
+					BufferID:  0,
+					HasBuffer: true,
+					Name:      []byte("strings"),
+					Value: &Array{
+						Path: []string{"strings"},
+						Item: &String{
+							Nullable: false,
+						},
+					},
+				},
+			},
+		}, Context{Context: context.Background()}, `non Nullable field value is null`
+	}))
 	t.Run("resolve arrays", testFn(func(t *testing.T, r *Resolver, ctrl *gomock.Controller) (node Node, ctx Context, expectedOutput string) {
 		return &Object{
 			Fetch: &SingleFetch{