Skip to content

Commit

Permalink
69 operation name is not forwarded for persisted queries (#83)
Browse files Browse the repository at this point in the history
Make sure to send out the operation name of a persisted operation to the
upstream

---------

Co-authored-by: ldebruijn <[email protected]>
  • Loading branch information
ldebruijn and ldebruijn authored May 3, 2024
1 parent ca7ac40 commit c6ce9b9
Show file tree
Hide file tree
Showing 20 changed files with 216 additions and 89 deletions.
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@ TODO.md
# ignore binary
main

store/
schema.graphql
store/
2 changes: 1 addition & 1 deletion cmd/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,7 @@ type Product {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
bts, _ := json.Marshal(tt.args.mockResponse)
w.WriteHeader(tt.args.mockStatusCode)

Expand Down
8 changes: 4 additions & 4 deletions cmd/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"fmt"
"github.com/ldebruijn/graphql-protect/internal/app/config"
"github.com/ldebruijn/graphql-protect/internal/app/otel"
"github.com/ldebruijn/graphql-protect/internal/business/persisted_operations"
"github.com/ldebruijn/graphql-protect/internal/business/persistedoperations"
"github.com/ldebruijn/graphql-protect/internal/business/protect"
"github.com/ldebruijn/graphql-protect/internal/business/rules/block_field_suggestions"
"github.com/ldebruijn/graphql-protect/internal/business/schema"
Expand Down Expand Up @@ -39,12 +39,12 @@ func httpServer(log *slog.Logger, cfg *config.Config, shutdown chan os.Signal) e
return nil
}

remoteLoader, err := persisted_operations.RemoteLoaderFromConfig(cfg.PersistedOperations, log)
if err != nil && !errors.Is(err, persisted_operations.ErrNoRemoteLoaderSpecified) {
remoteLoader, err := persistedoperations.RemoteLoaderFromConfig(cfg.PersistedOperations, log)
if err != nil && !errors.Is(err, persistedoperations.ErrNoRemoteLoaderSpecified) {
log.Warn("Error initializing remote loader", "err", err)
}

po, err := persisted_operations.NewPersistedOperations(log, cfg.PersistedOperations, persisted_operations.NewLocalDirLoader(cfg.PersistedOperations, log), remoteLoader)
po, err := persistedoperations.NewPersistedOperations(log, cfg.PersistedOperations, persistedoperations.NewLocalDirLoader(cfg.PersistedOperations, log), remoteLoader)
if err != nil {
log.Error("Error initializing Persisted Operations", "err", err)
return nil
Expand Down
8 changes: 4 additions & 4 deletions cmd/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"fmt"
"github.com/jedib0t/go-pretty/v6/table"
"github.com/ldebruijn/graphql-protect/internal/app/config"
"github.com/ldebruijn/graphql-protect/internal/business/persisted_operations"
"github.com/ldebruijn/graphql-protect/internal/business/persistedoperations"
"github.com/ldebruijn/graphql-protect/internal/business/protect"
"github.com/ldebruijn/graphql-protect/internal/business/schema"
"github.com/vektah/gqlparser/v2/gqlerror"
Expand All @@ -23,12 +23,12 @@ func validate(log *slog.Logger, cfg *config.Config, _ chan os.Signal) error {
return err
}

remoteLoader, err := persisted_operations.RemoteLoaderFromConfig(cfg.PersistedOperations, log)
if err != nil && !errors.Is(err, persisted_operations.ErrNoRemoteLoaderSpecified) {
remoteLoader, err := persistedoperations.RemoteLoaderFromConfig(cfg.PersistedOperations, log)
if err != nil && !errors.Is(err, persistedoperations.ErrNoRemoteLoaderSpecified) {
log.Warn("Error initializing remote loader", "err", err)
}

po, err := persisted_operations.NewPersistedOperations(log, cfg.PersistedOperations, persisted_operations.NewLocalDirLoader(cfg.PersistedOperations, log), remoteLoader)
po, err := persistedoperations.NewPersistedOperations(log, cfg.PersistedOperations, persistedoperations.NewLocalDirLoader(cfg.PersistedOperations, log), remoteLoader)
if err != nil {
log.Error("Error initializing Persisted Operations", "err", err)
return nil
Expand Down
4 changes: 2 additions & 2 deletions internal/app/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"fmt"
"github.com/ardanlabs/conf/v3"
"github.com/ardanlabs/conf/v3/yaml"
"github.com/ldebruijn/graphql-protect/internal/business/persisted_operations"
"github.com/ldebruijn/graphql-protect/internal/business/persistedoperations"
"github.com/ldebruijn/graphql-protect/internal/business/rules/aliases"
"github.com/ldebruijn/graphql-protect/internal/business/rules/batch"
"github.com/ldebruijn/graphql-protect/internal/business/rules/block_field_suggestions"
Expand All @@ -32,7 +32,7 @@ type Config struct {
ObfuscateValidationErrors bool `conf:"default:false" yaml:"obfuscate_validation_errors"`
Schema schema.Config `yaml:"schema"`
Target proxy.Config `yaml:"target"`
PersistedOperations persisted_operations.Config `yaml:"persisted_operations"`
PersistedOperations persistedoperations.Config `yaml:"persisted_operations"`
BlockFieldSuggestions block_field_suggestions.Config `yaml:"block_field_suggestions"`
MaxTokens tokens.Config `yaml:"max_tokens"`
MaxAliases aliases.Config `yaml:"max_aliases"`
Expand Down
8 changes: 4 additions & 4 deletions internal/app/config/config_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package config

import (
"github.com/ldebruijn/graphql-protect/internal/business/persisted_operations"
"github.com/ldebruijn/graphql-protect/internal/business/persistedoperations"
"github.com/ldebruijn/graphql-protect/internal/business/rules/aliases"
"github.com/ldebruijn/graphql-protect/internal/business/rules/batch"
"github.com/ldebruijn/graphql-protect/internal/business/rules/block_field_suggestions"
Expand All @@ -26,7 +26,7 @@ func TestNewConfig(t *testing.T) {
}{
{
name: "Assures defaults are applied correctly",
applyConfig: func(file *os.File) {
applyConfig: func(_ *os.File) {

},
want: &Config{
Expand Down Expand Up @@ -68,7 +68,7 @@ func TestNewConfig(t *testing.T) {
KeepAlive: 3 * time.Minute,
Host: "http://localhost:8081",
},
PersistedOperations: persisted_operations.Config{
PersistedOperations: persistedoperations.Config{
Enabled: false,
Store: "./store",
Reload: struct {
Expand Down Expand Up @@ -222,7 +222,7 @@ enforce_post:
KeepAlive: 1 * time.Second,
Host: "host",
},
PersistedOperations: persisted_operations.Config{
PersistedOperations: persistedoperations.Config{
Enabled: true,
Store: "store",
Reload: struct {
Expand Down
21 changes: 0 additions & 21 deletions internal/business/persisted_operations/memory_loader.go

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
package persisted_operations // nolint:revive
package persistedoperations // nolint:revive

import (
"context"
"encoding/json"
"log/slog"
"maps"
"os"
"path/filepath"
)
Expand All @@ -26,7 +25,7 @@ func NewLocalDirLoader(cfg Config, log *slog.Logger) *DirLoader {
}
}

func (d *DirLoader) Load(_ context.Context) (map[string]string, error) {
func (d *DirLoader) Load(_ context.Context) (map[string]PersistedOperation, error) {
files, err := os.ReadDir(d.path)
if err != nil {
// if we can't read the dir, try creating it
Expand All @@ -36,7 +35,7 @@ func (d *DirLoader) Load(_ context.Context) (map[string]string, error) {
}
}

result := map[string]string{}
result := map[string]PersistedOperation{}

for _, file := range files {
if file.IsDir() {
Expand All @@ -57,7 +56,9 @@ func (d *DirLoader) Load(_ context.Context) (map[string]string, error) {
continue
}

maps.Copy(result, manifestHashes)
for hash, operation := range manifestHashes {
result[hash] = NewPersistedOperation(operation)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package persisted_operations // nolint:revive
package persistedoperations // nolint:revive

import (
"cloud.google.com/go/storage"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package persisted_operations // nolint:revive
package persistedoperations // nolint:revive

import (
"context"
Expand All @@ -8,7 +8,7 @@ import (
)

type LocalLoader interface {
Load(ctx context.Context) (map[string]string, error)
Load(ctx context.Context) (map[string]PersistedOperation, error)
}

type RemoteLoader interface {
Expand Down
21 changes: 21 additions & 0 deletions internal/business/persistedoperations/memory_loader.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package persistedoperations // nolint:revive

import (
"context"
)

// MemoryLoader is a loader for testing purposes
// It allows the user to specify operations in memory
type MemoryLoader struct {
store map[string]PersistedOperation
}

func newMemoryLoader(store map[string]PersistedOperation) *MemoryLoader {
return &MemoryLoader{
store: store,
}
}

func (d *MemoryLoader) Load(_ context.Context) (map[string]PersistedOperation, error) {
return d.store, nil
}
33 changes: 33 additions & 0 deletions internal/business/persistedoperations/model.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package persistedoperations

import "strings"

type PersistedOperation struct {
Operation string
Name string
}

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
}

if firstSpace > until || until == -1 {
return ""
}

return strings.TrimSpace(payload[firstSpace+1 : until])
}
92 changes: 92 additions & 0 deletions internal/business/persistedoperations/model_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package persistedoperations

import (
"github.com/stretchr/testify/assert"
"testing"
)

func TestNewPersistedOperation(t *testing.T) {
type args struct {
operation string
}
tests := []struct {
name string
args args
want PersistedOperation
}{
{
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",
},
},
{
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",
},
},
{
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: "",
},
},
{
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 } }",
},
},
{
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",
},
},
{
name: "no weird stuff when getting a completely malformed string",
args: args{
operation: "",
},
want: PersistedOperation{
Operation: "",
Name: "",
},
},
{
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",
},
},
}
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)
})
}
}
Loading

0 comments on commit c6ce9b9

Please sign in to comment.