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

feat(GraphQL): Disallow DQL schema changes for predicates used in GraphQL schema (DGRAPH-3245) #7742

Merged
merged 3 commits into from
Apr 22, 2021
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
144 changes: 144 additions & 0 deletions edgraph/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,8 @@ func (s *Server) Alter(ctx context.Context, op *api.Operation) (*api.Payload, er
_, err := query.ApplyMutations(ctx, m)
return empty, err
}

// it is a schema update
result, err := parseSchemaFromAlterOperation(ctx, op)
if err == errIndexingInProgress {
// Make the client wait a bit.
Expand All @@ -528,6 +530,9 @@ func (s *Server) Alter(ctx context.Context, op *api.Operation) (*api.Payload, er
} else if err != nil {
return nil, err
}
if err = validateDQLSchemaForGraphQL(ctx, result, namespace); err != nil {
return nil, err
}

glog.Infof("Got schema: %+v\n", result)
// TODO: Maybe add some checks about the schema.
Expand All @@ -546,6 +551,145 @@ func (s *Server) Alter(ctx context.Context, op *api.Operation) (*api.Payload, er
return empty, nil
}

func validateDQLSchemaForGraphQL(ctx context.Context,
dqlSch *schema.ParsedSchema, ns uint64) error {
// fetch the GraphQL schema for this namespace from disk
_, existingGQLSch, err := GetGQLSchema(ns)
if err != nil || existingGQLSch == "" {
return err
}

// convert the existing GraphQL schema to a DQL schema
handler, err := gqlSchema.NewHandler(existingGQLSch, false)
if err != nil {
return err
}
dgSchema := handler.DGSchema()
if dgSchema == "" {
return nil
}
gqlReservedDgSch, err := parseSchemaFromAlterOperation(ctx, &api.Operation{Schema: dgSchema})
if err != nil {
return err
}

// create a mapping for the GraphQL reserved predicates and types
gqlReservedPreds := make(map[string]*pb.SchemaUpdate)
gqlReservedTypes := make(map[string]*pb.TypeUpdate)
for _, pred := range gqlReservedDgSch.Preds {
gqlReservedPreds[pred.Predicate] = pred
}
for _, typ := range gqlReservedDgSch.Types {
gqlReservedTypes[typ.TypeName] = typ
}

// now validate the DQL schema to check that it doesn't break the existing GraphQL schema

// Step-1: validate predicates
for _, dqlPred := range dqlSch.Preds {
gqlPred := gqlReservedPreds[dqlPred.Predicate]
if gqlPred == nil {
continue // if the predicate isn't used by GraphQL, no need to validate it.
}

// type (including list) must match exactly
if gqlPred.ValueType != dqlPred.ValueType || gqlPred.List != dqlPred.List {
gqlType := strings.ToLower(gqlPred.ValueType.String())
dqlType := strings.ToLower(dqlPred.ValueType.String())
if gqlPred.List {
gqlType = "[" + gqlType + "]"
}
if dqlPred.List {
dqlType = "[" + dqlType + "]"
}
return errors.Errorf("can't alter predicate %s as it is used by the GraphQL API, "+
"and type definition is incompatible with what is expected by the GraphQL API. "+
"want: %s, got: %s", x.ParseAttr(gqlPred.Predicate), gqlType, dqlType)
}
// if gqlSchema had any indexes, then those must be present in the dqlSchema.
// dqlSchema may add more indexes than what gqlSchema had initially, but can't remove them.
if gqlPred.Directive == pb.SchemaUpdate_INDEX {
if dqlPred.Directive != pb.SchemaUpdate_INDEX {
return errors.Errorf("can't alter predicate %s as it is used by the GraphQL API, "+
"and is missing index definition that is expected by the GraphQL API. "+
"want: @index(%s)", x.ParseAttr(gqlPred.Predicate),
strings.Join(gqlPred.Tokenizer, ","))
}
var missingIndexes []string
for _, t := range gqlPred.Tokenizer {
if !x.HasString(dqlPred.Tokenizer, t) {
missingIndexes = append(missingIndexes, t)
}
}
if len(missingIndexes) > 0 {
return errors.Errorf("can't alter predicate %s as it is used by the GraphQL API, "+
"and is missing index definition that is expected by the GraphQL API. "+
"want: @index(%s, %s), got: @index(%s)", x.ParseAttr(gqlPred.Predicate),
strings.Join(dqlPred.Tokenizer, ","), strings.Join(missingIndexes, ","),
strings.Join(dqlPred.Tokenizer, ","))
}
}
// if gqlSchema had @reverse, then dqlSchema must have it. dqlSchema can't remove @reverse.
// if gqlSchema didn't had @reverse, it is allowed to dqlSchema to add it.
if gqlPred.Directive == pb.SchemaUpdate_REVERSE && dqlPred.Directive != pb.
SchemaUpdate_REVERSE {
return errors.Errorf("can't alter predicate %s as it is used by the GraphQL API, "+
"and is missing @reverse that is expected by the GraphQL API.",
x.ParseAttr(gqlPred.Predicate))
}
// if gqlSchema had @count, then dqlSchema must have it. dqlSchema can't remove @count.
// if gqlSchema didn't had @count, it is allowed to dqlSchema to add it.
if gqlPred.Count && !dqlPred.Count {
return errors.Errorf("can't alter predicate %s as it is used by the GraphQL API, "+
"and is missing @count that is expected by the GraphQL API.",
x.ParseAttr(gqlPred.Predicate))
}
// if gqlSchema had @upsert, then dqlSchema must have it. dqlSchema can't remove @upsert.
// if gqlSchema didn't had @upsert, it is allowed to dqlSchema to add it.
if gqlPred.Upsert && !dqlPred.Upsert {
return errors.Errorf("can't alter predicate %s as it is used by the GraphQL API, "+
"and is missing @upsert that is expected by the GraphQL API.",
x.ParseAttr(gqlPred.Predicate))
}
// if gqlSchema had @lang, then dqlSchema must have it. dqlSchema can't remove @lang.
// if gqlSchema didn't had @lang, it is allowed to dqlSchema to add it.
if gqlPred.Lang && !dqlPred.Lang {
return errors.Errorf("can't alter predicate %s as it is used by the GraphQL API, "+
"and is missing @lang that is expected by the GraphQL API.",
x.ParseAttr(gqlPred.Predicate))
}
}

// Step-2: validate types
for _, dqlType := range dqlSch.Types {
gqlType := gqlReservedTypes[dqlType.TypeName]
if gqlType == nil {
continue // if the type isn't used by GraphQL, no need to validate it.
}

// create a mapping of all the fields in the dqlType
dqlFields := make(map[string]bool)
for _, f := range dqlType.Fields {
dqlFields[f.Predicate] = true
}

// check that all the fields of the gqlType must be present in the dqlType
var missingFields []string
for _, f := range gqlType.Fields {
if !dqlFields[f.Predicate] {
missingFields = append(missingFields, x.ParseAttr(f.Predicate))
}
}
if len(missingFields) > 0 {
return errors.Errorf("can't alter type %s as it is used by the GraphQL API, "+
"and is missing fields: [%s] that are expected by the GraphQL API.",
x.ParseAttr(gqlType.TypeName), strings.Join(missingFields, ","))
}
}

return nil
}

func annotateStartTs(span *otrace.Span, ts uint64) {
span.Annotate([]otrace.Attribute{otrace.Int64Attribute("startTs", int64(ts))}, "")
}
Expand Down
Loading