Skip to content

Commit

Permalink
fix: Determine if query is introspection query (#1255)
Browse files Browse the repository at this point in the history
Parse query content query to determine introspection
Remove hard-coded IntrospectionQuery from tests
  • Loading branch information
islamaliev committed Mar 30, 2023
1 parent 706e55d commit bce78a1
Show file tree
Hide file tree
Showing 17 changed files with 117 additions and 61 deletions.
10 changes: 7 additions & 3 deletions core/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ package core
import (
"context"

"github.com/graphql-go/graphql/language/ast"
"github.com/sourcenetwork/immutable"

"github.com/sourcenetwork/defradb/client"
Expand All @@ -34,14 +35,17 @@ type SchemaDefinition struct {
// Parser represents the object responsible for handling stuff specific to a query language.
// This includes schema and request parsing, and introspection.
type Parser interface {
// Returns true if the given string is an introspection request.
IsIntrospection(request string) bool
// BuildRequestAST builds and return AST for the given request.
BuildRequestAST(request string) (*ast.Document, error)

// Returns true if the given request ast is an introspection request.
IsIntrospection(*ast.Document) bool

// Executes the given introspection request.
ExecuteIntrospection(request string) *client.RequestResult

// Parses the given request, returning a strongly typed model of that request.
Parse(request string) (*request.Request, []error)
Parse(*ast.Document) (*request.Request, []error)

// NewFilterFromString creates a new filter from a string.
NewFilterFromString(collectionType string, body string) (immutable.Option[request.Filter], error)
Expand Down
15 changes: 9 additions & 6 deletions db/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ package db

import (
"context"
"strings"

"github.com/sourcenetwork/defradb/client"
"github.com/sourcenetwork/defradb/datastore"
Expand All @@ -22,12 +21,16 @@ import (
// execRequest executes a request against the database.
func (db *db) execRequest(ctx context.Context, request string, txn datastore.Txn) *client.RequestResult {
res := &client.RequestResult{}
// check if its Introspection request
if strings.Contains(request, "IntrospectionQuery") {
return db.ExecIntrospection(request)
ast, err := db.parser.BuildRequestAST(request)
if err != nil {
res.GQL.Errors = []any{err.Error()}
return res
}
if db.parser.IsIntrospection(ast) {
return db.parser.ExecuteIntrospection(request)
}

parsedRequest, errors := db.parser.Parse(request)
parsedRequest, errors := db.parser.Parse(ast)
if len(errors) > 0 {
errorStrings := make([]any, len(errors))
for i, err := range errors {
Expand All @@ -37,7 +40,7 @@ func (db *db) execRequest(ctx context.Context, request string, txn datastore.Txn
return res
}

pub, subRequest, err := db.checkForClientSubsciptions(parsedRequest)
pub, subRequest, err := db.checkForClientSubscriptions(parsedRequest)
if err != nil {
res.GQL.Errors = []any{err.Error()}
return res
Expand Down
4 changes: 2 additions & 2 deletions db/subscriptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
"github.com/sourcenetwork/defradb/planner"
)

func (db *db) checkForClientSubsciptions(r *request.Request) (
func (db *db) checkForClientSubscriptions(r *request.Request) (
*events.Publisher[events.Update],
*request.ObjectSubscription,
error,
Expand All @@ -39,7 +39,7 @@ func (db *db) checkForClientSubsciptions(r *request.Request) (
return pub, subRequest, nil
}

return nil, nil, client.NewErrUnexpectedType[request.ObjectSubscription]("SubcriptionSelection", s)
return nil, nil, client.NewErrUnexpectedType[request.ObjectSubscription]("SubscriptionSelection", s)
}
return nil, nil, nil
}
Expand Down
34 changes: 19 additions & 15 deletions request/graphql/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ package graphql

import (
"context"
"strings"

gql "github.com/graphql-go/graphql"
"github.com/graphql-go/graphql/language/ast"
gqlp "github.com/graphql-go/graphql/language/parser"
"github.com/graphql-go/graphql/language/source"
"github.com/sourcenetwork/immutable"
Expand Down Expand Up @@ -46,9 +46,23 @@ func NewParser() (*parser, error) {
return p, nil
}

func (p *parser) IsIntrospection(request string) bool {
// todo: This needs to be done properly https://github.com/sourcenetwork/defradb/issues/911
return strings.Contains(request, "IntrospectionQuery")
func (p *parser) BuildRequestAST(request string) (*ast.Document, error) {
source := source.NewSource(&source.Source{
Body: []byte(request),
Name: "GraphQL request",
})

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

return ast, nil
}

func (p *parser) IsIntrospection(ast *ast.Document) bool {
schema := p.schemaManager.Schema()
return defrap.IsIntrospectionQuery(*schema, ast)
}

func (p *parser) ExecuteIntrospection(request string) *client.RequestResult {
Expand All @@ -70,17 +84,7 @@ func (p *parser) ExecuteIntrospection(request string) *client.RequestResult {
return res
}

func (p *parser) Parse(request string) (*request.Request, []error) {
source := source.NewSource(&source.Source{
Body: []byte(request),
Name: "GraphQL request",
})

ast, err := gqlp.Parse(gqlp.ParseParams{Source: source})
if err != nil {
return nil, []error{err}
}

func (p *parser) Parse(ast *ast.Document) (*request.Request, []error) {
schema := p.schemaManager.Schema()
validationResult := gql.ValidateDocument(schema, ast, nil)
if !validationResult.IsValid {
Expand Down
43 changes: 43 additions & 0 deletions request/graphql/parser/introspection.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright 2022 Democratized Data Foundation
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package parser

import (
gql "github.com/graphql-go/graphql"
"github.com/graphql-go/graphql/language/ast"
)

// IsIntrospectionQuery parses a root ast.Document and determines if it is an
// introspection query. This is used to determine if the query should be
// executed against the schema or the database.
func IsIntrospectionQuery(schema gql.Schema, doc *ast.Document) bool {
for _, def := range doc.Definitions {
astOpDef, isOpDef := def.(*ast.OperationDefinition)
if !isOpDef {
continue
}

if astOpDef.Operation == ast.OperationTypeQuery {
for _, selection := range astOpDef.SelectionSet.Selections {
switch node := selection.(type) {
case *ast.Field:
if node.Name.Value == "__schema" || node.Name.Value == "__type" {
return true
}
default:
return false
}
}
}
}

return false
}
6 changes: 3 additions & 3 deletions request/graphql/parser/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func ParseRequest(schema gql.Schema, doc *ast.Document) (*request.Request, []err
}

switch astOpDef.Operation {
case "query":
case ast.OperationTypeQuery:
parsedQueryOpDef, errs := parseQueryOperationDefinition(schema, astOpDef)
if errs != nil {
return nil, errs
Expand All @@ -55,7 +55,7 @@ func ParseRequest(schema gql.Schema, doc *ast.Document) (*request.Request, []err

r.Queries = append(r.Queries, parsedQueryOpDef)

case "mutation":
case ast.OperationTypeMutation:
parsedMutationOpDef, err := parseMutationOperationDefinition(schema, astOpDef)
if err != nil {
return nil, []error{err}
Expand All @@ -69,7 +69,7 @@ func ParseRequest(schema gql.Schema, doc *ast.Document) (*request.Request, []err

r.Mutations = append(r.Mutations, parsedMutationOpDef)

case "subscription":
case ast.OperationTypeSubscription:
parsedSubscriptionOpDef, err := parseSubscriptionOperationDefinition(schema, astOpDef)
if err != nil {
return nil, []error{err}
Expand Down
6 changes: 4 additions & 2 deletions tests/bench/query/planner/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ func runQueryParserBench(

b.ResetTimer()
for i := 0; i < b.N; i++ {
_, errs := parser.Parse(query)
ast, _ := parser.BuildRequestAST(query)
_, errs := parser.Parse(ast)
if errs != nil {
return errors.Wrap("failed to parse query string", errors.New(fmt.Sprintf("%v", errs)))
}
Expand All @@ -65,7 +66,8 @@ func runMakePlanBench(
return err
}

q, errs := parser.Parse(query)
ast, _ := parser.BuildRequestAST(query)
q, errs := parser.Parse(ast)
if len(errs) > 0 {
return errors.Wrap("failed to parse query string", errors.New(fmt.Sprintf("%v", errs)))
}
Expand Down
22 changes: 11 additions & 11 deletions tests/integration/schema/aggregates/inline_array_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func TestSchemaAggregateInlineArrayCreatesUsersCount(t *testing.T) {
},
testUtils.IntrospectionRequest{
Request: `
query IntrospectionQuery {
query {
__type (name: "users") {
name
fields {
Expand Down Expand Up @@ -152,7 +152,7 @@ func TestSchemaAggregateInlineArrayCreatesUsersSum(t *testing.T) {
},
testUtils.IntrospectionRequest{
Request: `
query IntrospectionQuery {
query {
__type (name: "users") {
name
fields {
Expand Down Expand Up @@ -274,7 +274,7 @@ func TestSchemaAggregateInlineArrayCreatesUsersAverage(t *testing.T) {
},
testUtils.IntrospectionRequest{
Request: `
query IntrospectionQuery {
query {
__type (name: "users") {
name
fields {
Expand Down Expand Up @@ -474,7 +474,7 @@ func TestSchemaAggregateInlineArrayCreatesUsersNillableBooleanCountFilter(t *tes
},
testUtils.IntrospectionRequest{
Request: `
query IntrospectionQuery {
query {
__type (name: "users") {
name
fields {
Expand Down Expand Up @@ -600,7 +600,7 @@ func TestSchemaAggregateInlineArrayCreatesUsersBooleanCountFilter(t *testing.T)
},
testUtils.IntrospectionRequest{
Request: `
query IntrospectionQuery {
query {
__type (name: "users") {
name
fields {
Expand Down Expand Up @@ -726,7 +726,7 @@ func TestSchemaAggregateInlineArrayCreatesUsersNillableIntegerCountFilter(t *tes
},
testUtils.IntrospectionRequest{
Request: `
query IntrospectionQuery {
query {
__type (name: "users") {
name
fields {
Expand Down Expand Up @@ -876,7 +876,7 @@ func TestSchemaAggregateInlineArrayCreatesUsersIntegerCountFilter(t *testing.T)
},
testUtils.IntrospectionRequest{
Request: `
query IntrospectionQuery {
query {
__type (name: "users") {
name
fields {
Expand Down Expand Up @@ -1026,7 +1026,7 @@ func TestSchemaAggregateInlineArrayCreatesUsersNillableFloatCountFilter(t *testi
},
testUtils.IntrospectionRequest{
Request: `
query IntrospectionQuery {
query {
__type (name: "users") {
name
fields {
Expand Down Expand Up @@ -1176,7 +1176,7 @@ func TestSchemaAggregateInlineArrayCreatesUsersFloatCountFilter(t *testing.T) {
},
testUtils.IntrospectionRequest{
Request: `
query IntrospectionQuery {
query {
__type (name: "users") {
name
fields {
Expand Down Expand Up @@ -1326,7 +1326,7 @@ func TestSchemaAggregateInlineArrayCreatesUsersNillableStringCountFilter(t *test
},
testUtils.IntrospectionRequest{
Request: `
query IntrospectionQuery {
query {
__type (name: "users") {
name
fields {
Expand Down Expand Up @@ -1464,7 +1464,7 @@ func TestSchemaAggregateInlineArrayCreatesUsersStringCountFilter(t *testing.T) {
},
testUtils.IntrospectionRequest{
Request: `
query IntrospectionQuery {
query {
__type (name: "users") {
name
fields {
Expand Down
6 changes: 3 additions & 3 deletions tests/integration/schema/aggregates/simple_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func TestSchemaAggregateSimpleCreatesUsersCount(t *testing.T) {
},
testUtils.IntrospectionRequest{
Request: `
query IntrospectionQuery {
query {
__type (name: "users") {
name
fields {
Expand Down Expand Up @@ -122,7 +122,7 @@ func TestSchemaAggregateSimpleCreatesUsersSum(t *testing.T) {
},
testUtils.IntrospectionRequest{
Request: `
query IntrospectionQuery {
query {
__type (name: "users") {
name
fields {
Expand Down Expand Up @@ -226,7 +226,7 @@ func TestSchemaAggregateSimpleCreatesUsersAverage(t *testing.T) {
},
testUtils.IntrospectionRequest{
Request: `
query IntrospectionQuery {
query {
__type (name: "users") {
name
fields {
Expand Down
6 changes: 3 additions & 3 deletions tests/integration/schema/aggregates/top_level_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func TestSchemaAggregateTopLevelCreatesCountGivenSchema(t *testing.T) {
},
testUtils.IntrospectionRequest{
Request: `
query IntrospectionQuery {
query {
__schema {
queryType {
fields {
Expand Down Expand Up @@ -104,7 +104,7 @@ func TestSchemaAggregateTopLevelCreatesSumGivenSchema(t *testing.T) {
},
testUtils.IntrospectionRequest{
Request: `
query IntrospectionQuery {
query {
__schema {
queryType {
fields {
Expand Down Expand Up @@ -210,7 +210,7 @@ func TestSchemaAggregateTopLevelCreatesAverageGivenSchema(t *testing.T) {
},
testUtils.IntrospectionRequest{
Request: `
query IntrospectionQuery {
query {
__schema {
queryType {
fields {
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/schema/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func TestIntrospectionExplainTypeDefined(t *testing.T) {
Actions: []any{
testUtils.IntrospectionRequest{
Request: `
query IntrospectionQuery {
query {
__schema {
types {
kind
Expand Down
Loading

0 comments on commit bce78a1

Please sign in to comment.