Fix(GraphQL): Add filter in DQL query in case of reverse predicate (#…

…7728) (#7733) * Add code * Fix GraphQL schemagen with reverse Dgraph predicates (#7689) (cherry picked from commit f7c199e) * Add yaml test * Fix tests (cherry picked from commit 1743501)
hypermodeinc · Apr 16, 2021 · 00ada83 · 00ada83
1 parent 445f7bf
commit 00ada83
Show file tree

Hide file tree

Showing 5 changed files with 101 additions and 2 deletions.
diff --git 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
@@ -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
         }
@@ -3382,4 +3382,52 @@
           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
@@ -446,3 +446,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
@@ -3367,3 +3367,15 @@ valid_schemas:
         questionText: String
       }
 
+  - 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
@@ -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))