Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update gqlgen init template to default to spec-compliant Int #3409

Merged
merged 20 commits into from
Dec 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion .github/workflows/check-coverage
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,29 @@ go test -covermode atomic -coverprofile=/tmp/coverage.out.tmp -coverpkg=./... $(
# ignore protobuf files
cat /tmp/coverage.out.tmp | grep -v ".pb.go" > /tmp/coverage.out

goveralls -coverprofile=/tmp/coverage.out -service=github -ignore='_examples/*/*,_examples/*/*/*,integration/*,integration/*/*,codegen/testserver/*/*,plugin/resolvergen/testdata/*/*,plugin/modelgen/*/*,plugin/federation/testdata/*/*/*,*/generated.go,*/*/generated.go,*/*/*/generated.go,graphql/executable_schema_mock.go'
join () {
phughes-scwx marked this conversation as resolved.
Show resolved Hide resolved
local IFS="$1"
shift
echo "$*"
}

ignore_list=(
'_examples/*/*'
'_examples/*/*/*'
'integration/*'
'integration/*/*'
'codegen/testserver/**/*generated*'
'codegen/testserver/**/*generated*/**'
'codegen/testserver/**/models-gen.go'
'codegen/testserver/**/resolver.go'
'plugin/resolvergen/testdata/*/*'
'plugin/modelgen/*/*'
'plugin/federation/testdata/*/*/*'
'*/generated.go'
'*/*/generated.go'
'*/*/*/generated.go'
'graphql/executable_schema_mock.go'
)
ignore=$(join , "${ignore_list[@]}")

goveralls -coverprofile=/tmp/coverage.out -service=github "-ignore=$ignore"
22 changes: 16 additions & 6 deletions codegen/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ type Config struct {
// If this is set to true, argument directives that
// decorate a field with a null value will still be called.
//
// This enables argumment directives to not just mutate
// This enables argument directives to not just mutate
// argument values but to set them even if they're null.
CallArgumentDirectivesWithNull bool `yaml:"call_argument_directives_with_null,omitempty"`
StructFieldsAlwaysPointers bool `yaml:"struct_fields_always_pointers,omitempty"`
Expand Down Expand Up @@ -807,11 +807,15 @@ func (c *Config) injectBuiltins() {
"Float": {Model: StringList{"github.com/99designs/gqlgen/graphql.FloatContext"}},
"String": {Model: StringList{"github.com/99designs/gqlgen/graphql.String"}},
"Boolean": {Model: StringList{"github.com/99designs/gqlgen/graphql.Boolean"}},
"Int": {Model: StringList{
"github.com/99designs/gqlgen/graphql.Int",
"github.com/99designs/gqlgen/graphql.Int32",
"github.com/99designs/gqlgen/graphql.Int64",
}},
"Int": {
// FIXME: using int / int64 for Int is not spec compliant and introduces
// security risks. We should default to int32.
Model: StringList{
"github.com/99designs/gqlgen/graphql.Int",
"github.com/99designs/gqlgen/graphql.Int32",
"github.com/99designs/gqlgen/graphql.Int64",
},
},
"ID": {
Model: StringList{
"github.com/99designs/gqlgen/graphql.ID",
Expand All @@ -828,6 +832,12 @@ func (c *Config) injectBuiltins() {

// These are additional types that are injected if defined in the schema as scalars.
extraBuiltins := TypeMap{
"Int64": {
Model: StringList{
"github.com/99designs/gqlgen/graphql.Int",
"github.com/99designs/gqlgen/graphql.Int64",
},
},
"Time": {Model: StringList{"github.com/99designs/gqlgen/graphql.Time"}},
"Map": {Model: StringList{"github.com/99designs/gqlgen/graphql.Map"}},
"Upload": {Model: StringList{"github.com/99designs/gqlgen/graphql.Upload"}},
Expand Down
187 changes: 187 additions & 0 deletions codegen/testserver/compliant-int/compliant_int_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
//go:generate go run ../../../testdata/gqlgen.go -config gqlgen_default.yml -stub generated-default/stub.go
//go:generate go run ../../../testdata/gqlgen.go -config gqlgen_compliant_strict.yml -stub generated-compliant-strict/stub.go

package compliant_int

import (
"bytes"
"context"
"fmt"
"go/ast"
"go/format"
"go/parser"
"go/token"
"path/filepath"
"slices"
"strings"
"testing"

"github.com/99designs/gqlgen/client"
gencompliant "github.com/99designs/gqlgen/codegen/testserver/compliant-int/generated-compliant-strict"
gendefault "github.com/99designs/gqlgen/codegen/testserver/compliant-int/generated-default"
"github.com/99designs/gqlgen/graphql"
"github.com/99designs/gqlgen/graphql/handler"
"github.com/99designs/gqlgen/graphql/handler/transport"
"github.com/stretchr/testify/require"
)

func TestCodegen(t *testing.T) {
cases := []struct {
name string
pkgPath string
signature map[string]string
models map[string][]string
}{
{
name: "no model configuration default generation",
pkgPath: "generated-default",
signature: map[string]string{
"EchoIntToInt": "func(ctx context.Context, n *int) (int, error)",
"EchoInt64ToInt64": "func(ctx context.Context, n *int) (int, error)",
},
models: map[string][]string{
"Input": {"N *int"},
"Result": {"N int"},
"Input64": {"N *int"},
"Result64": {"N int"},
},
},
{
name: "strict compliant model configuration in yaml",
pkgPath: "generated-compliant-strict",
signature: map[string]string{
"EchoIntToInt": "func(ctx context.Context, n *int32) (int32, error)",
"EchoInt64ToInt64": "func(ctx context.Context, n *int) (int, error)",
},
models: map[string][]string{
"Input": {"N *int32"},
"Result": {"N int32"},
"Input64": {"N *int"},
"Result64": {"N int"},
},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
path, err := filepath.Abs(tc.pkgPath)
require.NoError(t, err)

pkgs, err := parser.ParseDir(token.NewFileSet(), path, nil, parser.AllErrors)
require.NoError(t, err)

pkg, ok := pkgs["generated"]
require.True(t, ok, fmt.Sprintf("invalid package found at %v", tc.pkgPath))

modelsMap := make(map[string][]string)
signatureMap := make(map[string]string)
ast.Inspect(pkg, func(node ast.Node) bool {
switch node := node.(type) {
case *ast.FuncDecl:
if slices.Contains(
[]string{"EchoIntToInt", "EchoInt64ToInt64"},
node.Name.Name,
) {
signatureMap[node.Name.Name] = printNode(t, node.Type)
}
case *ast.TypeSpec:
s, ok := node.Type.(*ast.StructType)
if !ok {
return true
}
if slices.Contains(
[]string{"Input", "Input64", "Result", "Result64"},
node.Name.Name,
) {
var fields []string
for _, field := range s.Fields.List {
fields = append(fields, join(field.Names)+" "+printNode(t, field.Type))
}
modelsMap[node.Name.Name] = fields
}
return true
default:
}
return true
})

t.Run("resolver signature", func(t *testing.T) {
require.Equal(t, tc.signature, signatureMap)
})
t.Run("models", func(t *testing.T) {
require.Equal(t, tc.models, modelsMap)
})
})
}
}

func TestIntegration(t *testing.T) {
defaultStub := &gendefault.Stub{}
defaultStub.QueryResolver.EchoIntToInt = func(_ context.Context, n *int) (int, error) {
if n == nil {
return 0, nil
}
return *n, nil
}
strictCompliantStub := &gencompliant.Stub{}
strictCompliantStub.QueryResolver.EchoIntToInt = func(_ context.Context, n *int32) (int32, error) {
if n == nil {
return 0, nil
}
return *n, nil
}

cases := []struct {
name string
exec graphql.ExecutableSchema
willError bool
}{
{
name: "default generation allows int32 overflow inputs",
exec: gendefault.NewExecutableSchema(gendefault.Config{Resolvers: defaultStub}),
willError: false,
},
{
name: "strict compliant generation does not allow int32 overflow inputs",
exec: gencompliant.NewExecutableSchema(gencompliant.Config{Resolvers: strictCompliantStub}),
willError: true,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
srv := handler.New(tc.exec)
srv.AddTransport(transport.POST{})

c := client.New(srv)

var resp struct {
EchoIntToInt int
}
err := c.Post(`query { echoIntToInt(n: 2147483648) }`, &resp)
if tc.willError {
require.EqualError(t, err, `[{"message":"2147483648 overflows 32-bit integer","path":["echoIntToInt","n"]}]`)
require.Equal(t, 0, resp.EchoIntToInt)
return
}
require.NoError(t, err)
require.Equal(t, 2147483648, resp.EchoIntToInt)
})
}
}

func printNode(t *testing.T, node any) string {
t.Helper()

buf := &bytes.Buffer{}
err := format.Node(buf, token.NewFileSet(), node)
require.NoError(t, err)

return buf.String()
}

func join[T fmt.Stringer](s []T) string {
var sb strings.Builder
for _, v := range s {
sb.WriteString(v.String())
}
return sb.String()
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package generated

// THIS CODE WILL BE UPDATED WITH SCHEMA CHANGES. PREVIOUS IMPLEMENTATION FOR SCHEMA CHANGES WILL BE KEPT IN THE COMMENT SECTION. IMPLEMENTATION FOR UNCHANGED SCHEMA WILL BE KEPT.

import (
"context"
)

type Resolver struct{}

// EchoIntToInt is the resolver for the echoIntToInt field.
func (r *queryResolver) EchoIntToInt(ctx context.Context, n *int32) (int32, error) {
panic("not implemented")
}

// EchoInt64ToInt64 is the resolver for the echoInt64ToInt64 field.
func (r *queryResolver) EchoInt64ToInt64(ctx context.Context, n *int) (int, error) {
panic("not implemented")
}

// EchoIntInputToIntObject is the resolver for the echoIntInputToIntObject field.
func (r *queryResolver) EchoIntInputToIntObject(ctx context.Context, input Input) (*Result, error) {
panic("not implemented")
}

// EchoInt64InputToInt64Object is the resolver for the echoInt64InputToInt64Object field.
func (r *queryResolver) EchoInt64InputToInt64Object(ctx context.Context, input Input64) (*Result64, error) {
panic("not implemented")
}

// Query returns QueryResolver implementation.
func (r *Resolver) Query() QueryResolver { return &queryResolver{r} }

type queryResolver struct{ *Resolver }

// !!! WARNING !!!
// The code below was going to be deleted when updating resolvers. It has been copied here so you have
// one last chance to move it out of harms way if you want. There are two reasons this happens:
// - When renaming or deleting a resolver the old code will be put in here. You can safely delete
// it when you're done.
// - You have helper methods in this file. Move them out to keep these resolver files clean.
/*
type Resolver struct{}
*/
Loading
Loading