Skip to content

Commit

Permalink
fix: Validate GraphQL schemas (#3152)
Browse files Browse the repository at this point in the history
## Relevant issue(s)

Resolves #3151

## Description

This PR fixes an issue where GraphQL schemas were never validated.

## Tasks

- [x] I made sure the code is well commented, particularly
hard-to-understand areas.
- [x] I made sure the repository-held documentation is changed
accordingly.
- [x] I made sure the pull request title adheres to the conventional
commit style (the subset used in the project can be found in
[tools/configs/chglog/config.yml](tools/configs/chglog/config.yml)).
- [x] I made sure to discuss its limitations such as threats to
validity, vulnerability to mistake and misuse, robustness to
invalidation of assumptions, resource requirements, ...

## How has this been tested?

Updated integration tests.

Specify the platform(s) on which this was tested:
- MacOS
  • Loading branch information
nasdf authored Oct 18, 2024
1 parent e81133e commit 8181a15
Show file tree
Hide file tree
Showing 32 changed files with 306 additions and 292 deletions.
2 changes: 1 addition & 1 deletion examples/schema/user.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ type User {
name: String
age: Int
verified: Boolean
points: Int @crdt(type: "pncounter")
points: Int @crdt(type: pncounter)
}
2 changes: 1 addition & 1 deletion internal/core/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ type Parser interface {
// The parsing should validate the syntax, but not validate what that syntax expresses
// is valid or not, i.e. we don't want the parser to make remote calls to verify the
// policy description is valid or not (that is the callers responsiblity).
ParseSDL(ctx context.Context, schemaString string) ([]client.CollectionDefinition, error)
ParseSDL(sdl string) ([]client.CollectionDefinition, error)

// Adds the given schema to this parser's model.
//
Expand Down
2 changes: 1 addition & 1 deletion internal/db/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (db *db) addSchema(
ctx context.Context,
schemaString string,
) ([]client.CollectionDescription, error) {
newDefinitions, err := db.parser.ParseSDL(ctx, schemaString)
newDefinitions, err := db.parser.ParseSDL(schemaString)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion internal/db/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (db *db) addView(
// with the all calls to the parser appart from `ParseSDL` when we implement the DQL stuff.
query := fmt.Sprintf(`query { %s }`, inputQuery)

newDefinitions, err := db.parser.ParseSDL(ctx, sdl)
newDefinitions, err := db.parser.ParseSDL(sdl)
if err != nil {
return nil, err
}
Expand Down
14 changes: 3 additions & 11 deletions internal/request/graphql/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,19 +94,11 @@ func (p *parser) Parse(ast *ast.Document, options *client.GQLOptions) (*request.
return nil, errors
}

query, parsingErrors := defrap.ParseRequest(*schema, ast, options)
if len(parsingErrors) > 0 {
return nil, parsingErrors
}

return query, nil
return defrap.ParseRequest(*schema, ast, options)
}

func (p *parser) ParseSDL(ctx context.Context, schemaString string) (
[]client.CollectionDefinition,
error,
) {
return schema.FromString(ctx, schemaString)
func (p *parser) ParseSDL(sdl string) ([]client.CollectionDefinition, error) {
return p.schemaManager.ParseSDL(sdl)
}

func (p *parser) SetSchema(ctx context.Context, txn datastore.Txn, collections []client.CollectionDefinition) error {
Expand Down
24 changes: 0 additions & 24 deletions internal/request/graphql/schema/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,12 @@
package schema

import (
"context"
"fmt"
"sort"
"strings"

gql "github.com/sourcenetwork/graphql-go"
"github.com/sourcenetwork/graphql-go/language/ast"
gqlp "github.com/sourcenetwork/graphql-go/language/parser"
"github.com/sourcenetwork/graphql-go/language/source"
"github.com/sourcenetwork/immutable"

"github.com/sourcenetwork/defradb/client"
Expand Down Expand Up @@ -50,27 +47,6 @@ var TypeToDefaultPropName = map[string]string{
typeBlob: types.DefaultDirectivePropBlob,
}

// FromString parses a GQL SDL string into a set of collection descriptions.
func FromString(ctx context.Context, schemaString string) (
[]client.CollectionDefinition,
error,
) {
source := source.NewSource(&source.Source{
Body: []byte(schemaString),
})

doc, err := gqlp.Parse(
gqlp.ParseParams{
Source: source,
},
)
if err != nil {
return nil, err
}

return fromAst(doc)
}

// fromAst parses a GQL AST into a set of collection descriptions.
func fromAst(doc *ast.Document) (
[]client.CollectionDefinition,
Expand Down
27 changes: 14 additions & 13 deletions internal/request/graphql/schema/index_parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
package schema

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -132,12 +131,12 @@ func TestParseInvalidIndexOnStruct(t *testing.T) {
{
description: "unknown argument",
sdl: `type user @index(unknown: "something", includes: [{field: "name"}]) {}`,
expectedErr: errIndexUnknownArgument,
expectedErr: `Unknown argument "unknown" on directive "@index".`,
},
{
description: "invalid index name type",
sdl: `type user @index(name: 1, includes: [{field: "name"}]) {}`,
expectedErr: errIndexInvalidArgument,
expectedErr: `Argument "name" has invalid value 1`,
},
{
description: "index name starts with a number",
Expand All @@ -162,17 +161,17 @@ func TestParseInvalidIndexOnStruct(t *testing.T) {
{
description: "invalid 'unique' value type",
sdl: `type user @index(includes: [{field: "name"}], unique: "true") {}`,
expectedErr: errIndexInvalidArgument,
expectedErr: `Argument "unique" has invalid value "true"`,
},
{
description: "invalid 'includes' value type (not a list)",
sdl: `type user @index(includes: "name") {}`,
expectedErr: errIndexInvalidArgument,
expectedErr: `Argument "includes" has invalid value "name"`,
},
{
description: "invalid 'includes' value type (not an object list)",
sdl: `type user @index(includes: [1]) {}`,
expectedErr: errIndexInvalidArgument,
expectedErr: `Argument "includes" has invalid value [1]`,
},
}

Expand Down Expand Up @@ -334,14 +333,14 @@ func TestParseInvalidIndexOnField(t *testing.T) {
sdl: `type user {
name: String @index(field: "name")
}`,
expectedErr: errIndexUnknownArgument,
expectedErr: `Unknown argument "field" on directive "@index`,
},
{
description: "invalid field index name type",
sdl: `type user {
name: String @index(name: 1)
}`,
expectedErr: errIndexInvalidArgument,
expectedErr: `Argument "name" has invalid value 1`,
},
{
description: "field index name starts with a number",
Expand Down Expand Up @@ -376,7 +375,7 @@ func TestParseInvalidIndexOnField(t *testing.T) {
sdl: `type user {
name: String @index(unique: "true")
}`,
expectedErr: errIndexInvalidArgument,
expectedErr: `Argument "unique" has invalid value "true"`,
},
}

Expand All @@ -386,9 +385,10 @@ func TestParseInvalidIndexOnField(t *testing.T) {
}

func parseIndexAndTest(t *testing.T, testCase indexTestCase) {
ctx := context.Background()
schemaManager, err := NewSchemaManager()
require.NoError(t, err)

cols, err := FromString(ctx, testCase.sdl)
cols, err := schemaManager.ParseSDL(testCase.sdl)
require.NoError(t, err, testCase.description)

require.Equal(t, len(cols), 1, testCase.description)
Expand All @@ -400,9 +400,10 @@ func parseIndexAndTest(t *testing.T, testCase indexTestCase) {
}

func parseInvalidIndexAndTest(t *testing.T, testCase invalidIndexTestCase) {
ctx := context.Background()
schemaManager, err := NewSchemaManager()
require.NoError(t, err)

_, err := FromString(ctx, testCase.sdl)
_, err = schemaManager.ParseSDL(testCase.sdl)
assert.ErrorContains(t, err, testCase.expectedErr, testCase.description)
}

Expand Down
Loading

0 comments on commit 8181a15

Please sign in to comment.