From 17435010e7edea02b99d5c2989b2c2712ed2567b Mon Sep 17 00:00:00 2001 From: vmrajas Date: Fri, 16 Apr 2021 16:48:22 +0530 Subject: [PATCH] Fix(GraphQL): Add filter in DQL query in case of reverse predicate (#7728) * Add code * Fix GraphQL schemagen with reverse Dgraph predicates (#7689) (cherry picked from commit f7c199ee06b8a8ec25bf1655cdab0c9e361893d0) * Add yaml test * Fix tests --- graphql/resolve/query_rewriter.go | 20 +++++++++++++ graphql/resolve/query_test.yaml | 50 ++++++++++++++++++++++++++++++- graphql/resolve/schema.graphql | 14 +++++++++ graphql/schema/gqlschema_test.yml | 13 ++++++++ graphql/schema/rules.go | 5 ++++ 5 files changed, 101 insertions(+), 1 deletion(-) diff --git a/graphql/resolve/query_rewriter.go b/graphql/resolve/query_rewriter.go index 0a7cb953748..72b2c9f812d 100644 --- a/graphql/resolve/query_rewriter.go +++ b/graphql/resolve/query_rewriter.go @@ -310,6 +310,7 @@ func aggregateQuery(query schema.Query, authRw *authRewriter) []*gql.GraphQuery // var(func: type(Tweets)) { // scoreVar as Tweets.score // } + mainQuery.Children = append(mainQuery.Children, child) isAggregateVarAdded[constructedForField] = true } @@ -1148,6 +1149,12 @@ func buildAggregateFields( fieldFilter, _ := f.ArgValue("filter").(map[string]interface{}) _ = addFilter(mainField, constructedForType, fieldFilter) + // Add type filter in case the Dgraph predicate for which the aggregate + // field belongs to is a reverse edge + if strings.HasPrefix(constructedForDgraphPredicate, "~") { + addTypeFilter(mainField, f.ConstructedFor()) + } + // isAggregateVarAdded is a map from field name to boolean. It is used to // ensure that a field is added to Var query at maximum once. // Eg. Even if scoreMax and scoreMin are queried, the corresponding field will @@ -1165,6 +1172,13 @@ func buildAggregateFields( } // Add filter to count aggregation field. _ = addFilter(aggregateChild, constructedForType, fieldFilter) + + // Add type filter in case the Dgraph predicate for which the aggregate + // field belongs to is a reverse edge + if strings.HasPrefix(constructedForDgraphPredicate, "~") { + addTypeFilter(aggregateChild, f.ConstructedFor()) + } + aggregateChildren = append(aggregateChildren, aggregateChild) continue } @@ -1363,6 +1377,12 @@ func addSelectionSetFrom( if includeField := addFilter(child, f.Type(), filter); !includeField { continue } + + // Add type filter in case the Dgraph predicate is a reverse edge + if strings.HasPrefix(f.DgraphPredicate(), "~") { + addTypeFilter(child, f.Type()) + } + addOrder(child, f) addPagination(child, f) addCascadeDirective(child, f) diff --git a/graphql/resolve/query_test.yaml b/graphql/resolve/query_test.yaml index 8a36386b400..0ffb235bfdd 100644 --- a/graphql/resolve/query_test.yaml +++ b/graphql/resolve/query_test.yaml @@ -2527,7 +2527,7 @@ query { queryMovie(func: type(Movie)) { Movie.name : Movie.name - Movie.director : ~directed.movies { + Movie.director : ~directed.movies @filter(type(MovieDirector)) { MovieDirector.name : MovieDirector.name dgraph.uid : uid } @@ -3410,3 +3410,51 @@ dgraph.uid : uid } } + +- + name: "Query fields linked to reverse predicates in Dgraph" + gqlquery: | + query { + queryLinkX(filter:{f9:{eq: "Alice"}}) { + f1(filter: {f6: {eq: "Eve"}}) { + f6 + } + f2(filter: {f7: {eq: "Bob"}}) { + f7 + } + f1Aggregate(filter: {f6: {eq: "Eve"}}) { + count + f6Max + } + f2Aggregate(filter: {f7: {eq: "Bob"}}) { + count + f7Min + } + } + } + dgquery: |- + query { + queryLinkX(func: type(LinkX)) @filter(eq(LinkX.f9, "Alice")) { + LinkX.f1 : ~link @filter((eq(LinkY.f6, "Eve") AND type(LinkY))) { + LinkY.f6 : LinkY.f6 + dgraph.uid : uid + } + LinkX.f2 : ~link @filter((eq(LinkZ.f7, "Bob") AND type(LinkZ))) { + LinkZ.f7 : LinkZ.f7 + dgraph.uid : uid + } + LinkX.f1Aggregate : ~link @filter((eq(LinkY.f6, "Eve") AND type(LinkY))) { + LinkX.f1Aggregate_f6Var as LinkY.f6 + dgraph.uid : uid + } + LinkYAggregateResult.count_LinkX.f1Aggregate : count(~link) @filter((eq(LinkY.f6, "Eve") AND type(LinkY))) + LinkYAggregateResult.f6Max_LinkX.f1Aggregate : max(val(LinkX.f1Aggregate_f6Var)) + LinkX.f2Aggregate : ~link @filter((eq(LinkZ.f7, "Bob") AND type(LinkZ))) { + LinkX.f2Aggregate_f7Var as LinkZ.f7 + dgraph.uid : uid + } + LinkZAggregateResult.count_LinkX.f2Aggregate : count(~link) @filter((eq(LinkZ.f7, "Bob") AND type(LinkZ))) + LinkZAggregateResult.f7Min_LinkX.f2Aggregate : min(val(LinkX.f2Aggregate_f7Var)) + dgraph.uid : uid + } + } diff --git a/graphql/resolve/schema.graphql b/graphql/resolve/schema.graphql index 0e534b52d94..f54236f4f9b 100644 --- a/graphql/resolve/schema.graphql +++ b/graphql/resolve/schema.graphql @@ -451,3 +451,17 @@ type Friend1 { type Friend { id: String! @id } + +type LinkX { + f9: String! @id + f1: [LinkY] @dgraph(pred: "~link") + f2: [LinkZ] @dgraph(pred: "~link") +} +type LinkY { + f6: String! @id + f3: [LinkX] @dgraph(pred: "link") +} +type LinkZ { + f7: String! @id + f4: [LinkX] @dgraph(pred: "link") +} diff --git a/graphql/schema/gqlschema_test.yml b/graphql/schema/gqlschema_test.yml index 975249b48bc..7882481fc5f 100644 --- a/graphql/schema/gqlschema_test.yml +++ b/graphql/schema/gqlschema_test.yml @@ -3431,3 +3431,16 @@ valid_schemas: addressHi: String @dgraph(pred: "Person.address@hi") professionEn: String @dgraph(pred: "Person.profession@en") } + + - name: "Same reverse dgraph predicate can be used by two different GraphQL fields" + input: | + type X { + f1: [Y] @dgraph(pred: "~link") + f2: [Z] @dgraph(pred: "~link") + } + type Y { + f3: [X] @dgraph(pred: "link") + } + type Z { + f4: [X] @dgraph(pred: "link") + } diff --git a/graphql/schema/rules.go b/graphql/schema/rules.go index cf635467138..c91bbe67cc1 100644 --- a/graphql/schema/rules.go +++ b/graphql/schema/rules.go @@ -195,6 +195,11 @@ func dgraphDirectivePredicateValidation(gqlSch *ast.Schema, definitions []string isSecret: false, } + // Skip the checks related to same Dgraph predicates being used twice with + // different types in case it is an inverse edge. + if strings.HasPrefix(fname, "~") || strings.HasPrefix(fname, "<~") { + continue + } if pred, ok := preds[fname]; ok { if pred.isSecret { errs = append(errs, secretError(pred, thisPred))