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

graphql: @dgraph(pred: "...") with @search #5019

Merged
merged 8 commits into from
Apr 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 16 additions & 0 deletions graphql/schema/gqlschema.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ var directiveValidators = map[string]directiveValidator{
},
}

var schemaValidations []func(schema *ast.Schema, definitions []string) gqlerror.List
var defnValidations, typeValidations []func(defn *ast.Definition) *gqlerror.Error
var fieldValidations []func(typ *ast.Definition, field *ast.FieldDefinition) *gqlerror.Error

Expand Down Expand Up @@ -353,6 +354,21 @@ func postGQLValidation(schema *ast.Schema, definitions []string) gqlerror.List {
}
}

errs = append(errs, applySchemaValidations(schema, definitions)...)

return errs
}

func applySchemaValidations(schema *ast.Schema, definitions []string) gqlerror.List {
var errs []*gqlerror.Error

for _, rule := range schemaValidations {
newErrs := rule(schema, definitions)
for _, err := range newErrs {
errs = appendIfNotNull(errs, err)
}
}

return errs
}

Expand Down
62 changes: 62 additions & 0 deletions graphql/schema/gqlschema_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,50 @@ invalid_schemas:
"locations":[{"line":1, "column":6}]},
]

-
name: "@dgraph(pred: ...) validation"
input: |
interface V {
f1: String @dgraph(pred: "ff1")
}
interface W @secret(field: "f", pred: "pwd") {
f2: String! @dgraph(pred: "name")
f3: [Float] @dgraph(pred: "val")
f4: String @dgraph(pred: "ff4")
f5: String @dgraph(pred: "ff1")
}
type X implements V & W {
f6: Y @dgraph(pred: "link")
f7: String! @dgraph(pred: "ff7") @id
f8: String
f9: String @dgraph(pred: "ff4")
}
type Y {
f2: Int @dgraph(pred: "name")
f3: Float @dgraph(pred: "val")
f6: X @dgraph(pred: "link")
f7: String @dgraph(pred: "ff7")
f8: Int @dgraph(pred: "X.f8")
f10: String @dgraph(pred: "pwd")
}
errlist: [
{"message": "Type X; implements interfaces [V W], all of which have fields with @dgraph predicate: ff1. These fields must use different Dgraph predicates.",
"locations":[{"line":10, "column":6}]},
{"message": "Type X; Field f9: has the @dgraph directive, which conflicts with interface W; field f4, that this type implements. These fields must use different Dgraph predicates.",
"locations":[{"line":14, "column":3}]},
{"message": "Type Y; Field f2: has type Int, which is different to type W; field f2, which has the same @dgraph directive but type String. These fields must have either the same GraphQL types, or use different Dgraph predicates.",
"locations":[{"line":17, "column":3}]},
{"message": "Type Y; Field f3: has type Float, which is different to type W; field f3, which has the same @dgraph directive but type [Float]. These fields must have either the same GraphQL types, or use different Dgraph predicates.",
"locations":[{"line":18, "column":3}]},
{"message": "Type Y; Field f6: has type X, which is different to type X; field f6, which has the same @dgraph directive but type Y. These fields must have either the same GraphQL types, or use different Dgraph predicates.",
"locations":[{"line":19, "column":3}]},
{"message": "Type Y; Field f7: doesn't have @id directive, which conflicts with type X; field f7, which has the same @dgraph directive along with @id directive. Both these fields must either use @id directive, or use different Dgraph predicates.",
"locations":[{"line":20, "column":3}]},
{"message": "Type Y; Field f8: has type Int, which is different to type X; field f8, which has the same @dgraph directive but type String. These fields must have either the same GraphQL types, or use different Dgraph predicates.",
"locations":[{"line":21, "column":3}]},
{"message": "Type Y; Field f10: has the @dgraph predicate, but that conflicts with type W @secret directive on the same predicate. @secret predicates are stored encrypted and so the same predicate can't be used as a String.",
"locations":[{"line":22, "column":3}]},
]


valid_schemas:
Expand Down Expand Up @@ -810,3 +854,21 @@ valid_schemas:
type Y {
f1: [X] @dgraph(pred: "f1")
}

-
name: "@dgraph predicate type validation gives no errors with non-null variations"
input: |
type X {
f1: String @dgraph(pred: "f1")
f2: [String] @dgraph(pred: "f2")
f3: [String!] @dgraph(pred: "f3")
f4: [String]! @dgraph(pred: "f4")
f5: String
}
type Y {
f1: String! @dgraph(pred: "f1")
f2: [String!] @dgraph(pred: "f2")
f3: [String]! @dgraph(pred: "f3")
f4: [String!]! @dgraph(pred: "f4")
f5: String @dgraph(pred: "X.f5")
}
200 changes: 199 additions & 1 deletion graphql/schema/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
)

func init() {
schemaValidations = append(schemaValidations, dgraphDirectivePredicateValidation)
defnValidations = append(defnValidations, dataTypeCheck, nameCheck)

typeValidations = append(typeValidations, idCountCheck, dgraphDirectiveTypeValidation,
Expand All @@ -39,6 +40,203 @@ func init() {

}

func dgraphDirectivePredicateValidation(gqlSch *ast.Schema, definitions []string) gqlerror.List {
var errs []*gqlerror.Error

type pred struct {
name string
parentName string
typ string
position *ast.Position
isId bool
isSecret bool
}

preds := make(map[string]pred)
interfacePreds := make(map[string]map[string]bool)

secretError := func(secretPred, newPred pred) *gqlerror.Error {
return gqlerror.ErrorPosf(newPred.position,
"Type %s; Field %s: has the @dgraph predicate, but that conflicts with type %s "+
"@secret directive on the same predicate. @secret predicates are stored encrypted"+
" and so the same predicate can't be used as a %s.", newPred.parentName,
newPred.name, secretPred.parentName, newPred.typ)
}

typeError := func(existingPred, newPred pred) *gqlerror.Error {
return gqlerror.ErrorPosf(newPred.position,
"Type %s; Field %s: has type %s, which is different to type %s; field %s, which has "+
"the same @dgraph directive but type %s. These fields must have either the same "+
"GraphQL types, or use different Dgraph predicates.", newPred.parentName,
newPred.name, newPred.typ, existingPred.parentName, existingPred.name,
existingPred.typ)
}

idError := func(idPred, newPred pred) *gqlerror.Error {
return gqlerror.ErrorPosf(newPred.position,
"Type %s; Field %s: doesn't have @id directive, which conflicts with type %s; field "+
"%s, which has the same @dgraph directive along with @id directive. Both these "+
"fields must either use @id directive, or use different Dgraph predicates.",
newPred.parentName, newPred.name, idPred.parentName, idPred.name)
}

existingInterfaceFieldError := func(interfacePred, newPred pred) *gqlerror.Error {
return gqlerror.ErrorPosf(newPred.position,
"Type %s; Field %s: has the @dgraph directive, which conflicts with interface %s; "+
"field %s, that this type implements. These fields must use different Dgraph "+
"predicates.", newPred.parentName, newPred.name, interfacePred.parentName,
interfacePred.name)
}

conflictingFieldsInImplementedInterfacesError := func(def *ast.Definition,
interfaces []string, pred string) *gqlerror.Error {
return gqlerror.ErrorPosf(def.Position,
"Type %s; implements interfaces %v, all of which have fields with @dgraph predicate:"+
" %s. These fields must use different Dgraph predicates.", def.Name, interfaces,
pred)
}

checkExistingInterfaceFieldError := func(def *ast.Definition, existingPred, newPred pred) {
for _, defName := range def.Interfaces {
if existingPred.parentName == defName {
errs = append(errs, existingInterfaceFieldError(existingPred, newPred))
}
}
}

checkConflictingFieldsInImplementedInterfacesError := func(typ *ast.Definition) {
fieldsToReport := make(map[string][]string)
interfaces := typ.Interfaces

for i := 0; i < len(interfaces); i++ {
intr1 := interfaces[i]
interfacePreds1 := interfacePreds[intr1]
for j := i + 1; j < len(interfaces); j++ {
intr2 := interfaces[j]
for fname := range interfacePreds[intr2] {
if interfacePreds1[fname] {
if len(fieldsToReport[fname]) == 0 {
fieldsToReport[fname] = append(fieldsToReport[fname], intr1)
}
fieldsToReport[fname] = append(fieldsToReport[fname], intr2)
}
}
}
}

for fname, interfaces := range fieldsToReport {
errs = append(errs, conflictingFieldsInImplementedInterfacesError(typ, interfaces,
fname))
}
}

// make sure all the interfaces are validated before validating any concrete types
// this is required when validating that a type if implements two interfaces, then none of the
// fields in those interfaces has the same dgraph predicate
var interfaces, concreteTypes []string
for _, def := range definitions {
if gqlSch.Types[def].Kind == ast.Interface {
interfaces = append(interfaces, def)
} else {
concreteTypes = append(concreteTypes, def)
}
}
definitions = append(interfaces, concreteTypes...)

for _, key := range definitions {
def := gqlSch.Types[key]
switch def.Kind {
case ast.Object, ast.Interface:
typName := typeName(def)
if def.Kind == ast.Interface {
interfacePreds[def.Name] = make(map[string]bool)
} else {
checkConflictingFieldsInImplementedInterfacesError(def)
}

for _, f := range def.Fields {
if f.Type.Name() == "ID" {
continue
}

fname := fieldName(f, typName)
// this field could have originally been defined in an interface that this type
// implements. If we get a parent interface, that means this field gets validated
// during the validation of that interface. So, no need to validate this field here.
if parentInterface(gqlSch, def, f.Name) == nil {
if def.Kind == ast.Interface {
interfacePreds[def.Name][fname] = true
}

var prefix, suffix string
if f.Type.Elem != nil {
prefix = "["
suffix = "]"
}

thisPred := pred{
name: f.Name,
parentName: def.Name,
typ: fmt.Sprintf("%s%s%s", prefix, f.Type.Name(), suffix),
position: f.Position,
isId: f.Directives.ForName(idDirective) != nil,
isSecret: false,
}

if pred, ok := preds[fname]; ok {
if pred.isSecret {
errs = append(errs, secretError(pred, thisPred))
} else if thisPred.typ != pred.typ {
errs = append(errs, typeError(pred, thisPred))
}
if pred.isId != thisPred.isId {
if pred.isId {
errs = append(errs, idError(pred, thisPred))
} else {
errs = append(errs, idError(thisPred, pred))
}
}
if def.Kind == ast.Object {
checkExistingInterfaceFieldError(def, pred, thisPred)
}
} else {
preds[fname] = thisPred
}
}
}

pwdField := getPasswordField(def)
if pwdField != nil {
fname := fieldName(pwdField, typName)
if getDgraphDirPredArg(pwdField) != nil && parentInterfaceForPwdField(gqlSch, def,
pwdField.Name) == nil {
thisPred := pred{
name: pwdField.Name,
parentName: def.Name,
typ: pwdField.Type.Name(),
position: pwdField.Position,
isId: false,
isSecret: true,
}

if pred, ok := preds[fname]; ok {
if thisPred.typ != pred.typ || !pred.isSecret {
errs = append(errs, secretError(thisPred, pred))
}
if def.Kind == ast.Object {
checkExistingInterfaceFieldError(def, pred, thisPred)
}
} else {
preds[fname] = thisPred
}
}
}
}
}

return errs
}

func dataTypeCheck(defn *ast.Definition) *gqlerror.Error {
if defn.Kind == ast.Object || defn.Kind == ast.Enum || defn.Kind == ast.Interface {
return nil
Expand Down Expand Up @@ -95,7 +293,7 @@ func passwordDirectiveValidation(typ *ast.Definition) *gqlerror.Error {
dirs := make([]string, 0)

for _, dir := range typ.Directives {
if dir.Name != "secret" {
if dir.Name != secretDirective {
continue
}
val := dir.Arguments.ForName("field").Value.Raw
Expand Down
Loading