Feature/fix fragments not working inside operation (#126)

Protect didnt correctly capture the operationName from operations which
contained fragments inside.

This MR fixes that

---------

Co-authored-by: Rick Bijkerk <[email protected]>

go.mod

@@ -4,7 +4,6 @@ go 1.23.0
 
 require (
 	cloud.google.com/go/storage v1.43.0
-	github.com/ardanlabs/conf/v3 v3.1.8
 	github.com/jedib0t/go-pretty/v6 v6.5.9
 	github.com/prometheus/client_golang v1.20.3
 	github.com/stretchr/testify v1.9.0

go.sum

@@ -20,8 +20,6 @@ github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883 h1:bvNMNQO63//z+xNg
 github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883/go.mod h1:rCTlJbsFo29Kk6CurOXKm700vrz8f0KW0JNfpkRJY/8=
 github.com/arbovm/levenshtein v0.0.0-20160628152529-48b4e1c0c4d0 h1:jfIu9sQUG6Ig+0+Ap1h4unLjW6YQJpKZVmUzxsD4E/Q=
 github.com/arbovm/levenshtein v0.0.0-20160628152529-48b4e1c0c4d0/go.mod h1:t2tdKJDJF9BV14lnkjHmOQgcvEKgtqs5a1N3LNdJhGE=
-github.com/ardanlabs/conf/v3 v3.1.8 h1:r0KUV9/Hni5XdeWR2+A1BiedIDnry5CjezoqgJ0rnFQ=
-github.com/ardanlabs/conf/v3 v3.1.8/go.mod h1:OIi6NK95fj8jKFPdZ/UmcPlY37JBg99hdP9o5XmNK9c=
 github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
 github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=
 github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8=

internal/business/persistedoperations/dir_loader.go

@@ -72,7 +72,7 @@ func (d *LocalLoader) Load(_ context.Context) (map[string]PersistedOperation, er
 
 			filesProcessed++
 
-			data, err := UnmarshallPersistedOperations(contents)
+			data, err := unmarshallPersistedOperations(contents)
 			if err != nil {
 				d.log.Warn("error unmarshalling operation file", "bytes", len(contents), "contents", string(contents), "filepath", filePath, "err", err)
 				continue

internal/business/persistedoperations/gcp_loader.go

@@ -112,5 +112,13 @@ func (g *GcpLoader) processFile(ctx context.Context, attrs *storage.ObjectAttrs)
 		return nil, fmt.Errorf("io.Copy: %w", err)
 	}
 
-	return UnmarshallPersistedOperations(data)
+	operations, err := unmarshallPersistedOperations(data)
+
+	for _, operation := range operations {
+		if operation.Name == "" {
+			g.log.Warn("Operation without operation name found!", "operation", operation)
+		}
+	}
+
+	return operations, err
 }

internal/business/persistedoperations/model.go

@@ -3,16 +3,20 @@ package persistedoperations
 import (
 	"encoding/json"
 	"fmt"
-	"strings"
+	"regexp"
 )
 
+// find the first word after the 'query' or 'mutation' keyword
+var operationNameRegexPattern = regexp.MustCompile(`\b(query|mutation)\s(\w+)`)
+
 type PersistedOperation struct {
 	Operation string
-	Name      string
+	Name      string `json:"name,omitempty"`
 }
 
-func UnmarshallPersistedOperations(payload []byte) (map[string]PersistedOperation, error) {
+func unmarshallPersistedOperations(payload []byte) (map[string]PersistedOperation, error) {
 	var manifestHashes map[string]string
+
 	err := json.Unmarshal(payload, &manifestHashes)
 	if err != nil {
 		return nil, fmt.Errorf("error unmarshalling operation file, bytes: %d, contents: %s, error: %w", len(payload), string(payload), err)
@@ -21,32 +25,22 @@ func UnmarshallPersistedOperations(payload []byte) (map[string]PersistedOperatio
 	data := make(map[string]PersistedOperation)
 
 	for hash, operation := range manifestHashes {
-		data[hash] = NewPersistedOperation(operation)
+		data[hash] = PersistedOperation{
+			Operation: operation,
+			Name:      extractOperationNameFromOperation(operation),
+		}
 	}
 	return data, nil
 }
 
-func NewPersistedOperation(operation string) PersistedOperation {
-	name := extractOperationNameFromPersistedOperation(operation)
-	return PersistedOperation{
-		Operation: operation,
-		Name:      name,
-	}
-}
-
-func extractOperationNameFromPersistedOperation(payload string) string {
-	firstSpace := strings.Index(payload, " ")
-	firstBracket := strings.Index(payload, "{")
-	firstParenthesis := strings.Index(payload, "(")
-
-	until := firstBracket
-	if firstParenthesis < firstBracket {
-		until = firstParenthesis
-	}
+func extractOperationNameFromOperation(payload string) string {
+	match := operationNameRegexPattern.FindStringSubmatch(payload)
 
-	if firstSpace > until || until == -1 {
-		return ""
+	// match[0] is the entire match
+	// match[1] is either mutation/query
+	// match[2] is the name of the operation
+	if len(match) == 3 {
+		return match[2]
 	}
-
-	return strings.TrimSpace(payload[firstSpace+1 : until])
+	return ""
 }

internal/business/persistedoperations/model_test.go

@@ -12,81 +12,77 @@ func TestNewPersistedOperation(t *testing.T) {
 	tests := []struct {
 		name string
 		args args
-		want PersistedOperation
+		want string
+		err  error
 	}{
 		{
 			name: "extracts operation from query",
 			args: args{
 				operation: "query ProductQuery{ product(id: 1) { id title as } }",
 			},
-			want: PersistedOperation{
-				Operation: "query ProductQuery{ product(id: 1) { id title as } }",
-				Name:      "ProductQuery",
-			},
+			want: "ProductQuery",
 		},
 		{
 			name: "extracts operation from mutation",
 			args: args{
 				operation: "mutation ProductQuery{ product(id: 1) { id title as } }",
 			},
-			want: PersistedOperation{
-				Operation: "mutation ProductQuery{ product(id: 1) { id title as } }",
-				Name:      "ProductQuery",
-			},
+			want: "ProductQuery",
 		},
 		{
 			name: "no operation name when not present",
 			args: args{
 				operation: "mutation { product(id: 1) { id title as } }",
 			},
-			want: PersistedOperation{
-				Operation: "mutation { product(id: 1) { id title as } }",
-				Name:      "",
-			},
+			want: "",
 		},
 		{
 			name: "no operation name when no space between type and bracket",
 			args: args{
 				operation: "mutation{ product(id: 1) { id title as } }",
 			},
-			want: PersistedOperation{
-				Operation: "mutation{ product(id: 1) { id title as } }",
-			},
+			want: "",
 		},
 		{
 			name: "excludes operation arguments",
 			args: args{
 				operation: "query Foobar($some: Int, $value: String){ product(id: 1) { id title as } }",
 			},
-			want: PersistedOperation{
-				Operation: "query Foobar($some: Int, $value: String){ product(id: 1) { id title as } }",
-				Name:      "Foobar",
-			},
+			want: "Foobar",
 		},
 		{
 			name: "no weird stuff when getting a completely malformed string",
 			args: args{
 				operation: "",
 			},
-			want: PersistedOperation{
-				Operation: "",
-				Name:      "",
-			},
+			want: "",
 		},
 		{
 			name: "handles white space around operation name",
 			args: args{
 				operation: "query Foobar ($some: Int, $value: String){ product(id: 1) { id title as } }",
 			},
-			want: PersistedOperation{
-				Operation: "query Foobar ($some: Int, $value: String){ product(id: 1) { id title as } }",
-				Name:      "Foobar",
+			want: "Foobar",
+		},
+		{
+			name: "error is thrown on non parseable queries",
+			args: args{
+				operation: "invalidQueryString",
+			},
+			want: "",
+		},
+		{
+			name: "Can deal with fragments inside a query",
+			args: args{
+				operation: "fragment BaseItem on MenuItem { action { menuItemActionType url } id imageUrl level measurement { clickedMenuItem } title } query MenuItems($country: String!, $id: String, $language: String!, $levels: String) { menuItems(country: $country, language: $language, id: $id, levels: $levels) { children { children { ...BaseItem } ...BaseItem } imageHeaderUrl level ...BaseItem } }",
 			},
+			want: "MenuItems",
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			assert.Equalf(t, tt.want, NewPersistedOperation(tt.args.operation), "NewPersistedOperation(%v)", tt.args.operation)
+			operation := extractOperationNameFromOperation(tt.args.operation)
+			assert.Equalf(t, tt.want, operation, "newPersistedOperation(%v)", tt.args.operation)
 		})
 	}
 }

internal/business/persistedoperations/persisted_operations_test.go

@@ -131,7 +131,7 @@ func TestNewPersistedOperations(t *testing.T) {
 					return bts
 				}(),
 				cache: map[string]PersistedOperation{
-					"foobar": NewPersistedOperation("query { foobar }"),
+					"foobar": newPersistedOperation("query { foobar }"),
 				},
 			},
 			want: func(t *testing.T) http.Handler {
@@ -183,8 +183,8 @@ func TestNewPersistedOperations(t *testing.T) {
 					return bts
 				}(),
 				cache: map[string]PersistedOperation{
-					"foobar": NewPersistedOperation("query { foobar }"),
-					"baz":    NewPersistedOperation("query { baz }"),
+					"foobar": newPersistedOperation("query { foobar }"),
+					"baz":    newPersistedOperation("query { baz }"),
 				},
 			},
 			want: func(t *testing.T) http.Handler {
@@ -229,8 +229,7 @@ func TestNewPersistedOperations(t *testing.T) {
 					return bts
 				}(),
 				cache: map[string]PersistedOperation{
-					"foobar": NewPersistedOperation("query { foobar }"),
-				},
+					"foobar": newPersistedOperation("query { foobar }")},
 			},
 			want: func(t *testing.T) http.Handler {
 				fn := func(_ http.ResponseWriter, _ *http.Request) {
@@ -383,6 +382,10 @@ func (e *errorLoader) Type() string {
 	return "error"
 }
 
+func newPersistedOperation(query string) PersistedOperation {
+	return PersistedOperation{query, extractOperationNameFromOperation(query)}
+}
+
 func (e *errorLoader) Load(_ context.Context) (map[string]PersistedOperation, error) {
 	if e.willReturnError {
 		return nil, e.err