fix(GraphQL): Correctly apply order for root auth query (GRAPHQL-854) (…

…#7117) This PR fixes the issue introduced by #7100. As pagination is moved to root auth query, so order also needs to be applied at root auth query, in addition to being applied at the root user query.
hypermodeinc · Dec 11, 2020 · 61ff62f · 61ff62f
1 parent e2d93de
commit 61ff62f
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 21 deletions.
diff --git a/graphql/e2e/auth/auth_test.go b/graphql/e2e/auth/auth_test.go
@@ -440,7 +440,7 @@ func TestAuthOnInterfaces(t *testing.T) {
 			result: `{"queryFbPost": [{"text": "B FbPost"}]}`,
 		},
 		{
-			name: "Query Interface should interhit auth rules from all the interfaces",
+			name: "Query Interface should inherit auth rules from all the interfaces",
 			query: `
 			query{
 				queryPost(order: {asc: text}){
@@ -603,12 +603,28 @@ func TestOrderAndOffset(t *testing.T) {
 				{Due: "2020-07-19T08:00:00", Comp: "2020-07-19T08:00:00"},
 			},
 		},
+		Task{
+			Name: "Fifth one, two occurrences",
+			Occurrences: []*TaskOccurrence{
+				{Due: "2020-07-19T08:00:00", Comp: "2020-07-19T08:00:00"},
+				{Due: "2020-07-19T08:00:00", Comp: "2020-07-19T08:00:00"},
+			},
+		},
+		Task{
+			Name: "Sixth Task four occurrences",
+			Occurrences: []*TaskOccurrence{
+				{Due: "2020-07-19T08:00:00", Comp: "2020-07-19T08:00:00"},
+				{Due: "2020-07-19T08:00:00", Comp: "2020-07-19T08:00:00"},
+				{Due: "2020-07-19T08:00:00", Comp: "2020-07-19T08:00:00"},
+				{Due: "2020-07-19T08:00:00", Comp: "2020-07-19T08:00:00"},
+			},
+		},
 	}
 	tasks.add(t)
 
 	query := `
 	query {
-	  queryTask(first: 4, order: {asc : name}) {
+	  queryTask(filter: {name: {anyofterms: "Task"}}, first: 4, offset: 1, order: {asc : name}) {
 		name
 		occurrences(first: 2) {
 		  due
@@ -624,7 +640,7 @@ func TestOrderAndOffset(t *testing.T) {
 		{
 		"queryTask": [
 		  {
-			"name": "First Task four occurrence",
+			"name": "Fourth Task two occurrences",
 			"occurrences": [
 			  {
 				"due": "2020-07-19T08:00:00Z",
@@ -637,21 +653,21 @@ func TestOrderAndOffset(t *testing.T) {
 			]
 		  },
 		  {
-			"name": "Fourth Task two occurrences",
+			"name": "Second Task single occurrence",
 			"occurrences": [
-			  {
-				"due": "2020-07-19T08:00:00Z",
-				"comp": "2020-07-19T08:00:00Z"
-			  },
 			  {
 				"due": "2020-07-19T08:00:00Z",
 				"comp": "2020-07-19T08:00:00Z"
 			  }
 			]
 		  },
 		  {
-			"name": "Second Task single occurrence",
+			"name": "Sixth Task four occurrences",
 			"occurrences": [
+			  {
+				"due": "2020-07-19T08:00:00Z",
+				"comp": "2020-07-19T08:00:00Z"
+			  },
 			  {
 				"due": "2020-07-19T08:00:00Z",
 				"comp": "2020-07-19T08:00:00Z"
@@ -677,7 +693,7 @@ func TestOrderAndOffset(t *testing.T) {
 			gqlResponse := getUserParams.ExecuteAsPost(t, common.GraphqlURL)
 			common.RequireNoGQLErrors(t, gqlResponse)
 
-			require.JSONEq(t, string(gqlResponse.Data), tcase.result)
+			require.JSONEq(t, tcase.result, string(gqlResponse.Data))
 		})
 	}
 

diff --git a/graphql/resolve/auth_delete_test.yaml b/graphql/resolve/auth_delete_test.yaml
@@ -453,7 +453,7 @@
         random : Log.random
         dgraph.uid : uid
       }
-      Log2 as var(func: uid(Log3))
+      Log2 as var(func: uid(Log3), orderasc: Log.logs)
       Log3 as var(func: uid(x))
     }
 

diff --git a/graphql/resolve/auth_query_test.yaml b/graphql/resolve/auth_query_test.yaml
@@ -695,7 +695,7 @@
         id : uid
         ownedBy : UserSecret.ownedBy
       }
-      UserSecretRoot as var(func: uid(UserSecret1), first: 1) @filter(uid(UserSecretAuth2))
+      UserSecretRoot as var(func: uid(UserSecret1), orderasc: UserSecret.aSecret, first: 1) @filter(uid(UserSecretAuth2))
       UserSecret1 as var(func: type(UserSecret)) @filter(eq(UserSecret.ownedBy, "user2"))
       UserSecretAuth2 as var(func: uid(UserSecret1)) @filter(eq(UserSecret.ownedBy, "user1")) @cascade
     }
@@ -824,7 +824,7 @@
         content : Movie.content
         dgraph.uid : uid
       }
-      MovieRoot as var(func: uid(Movie1), first: 10, offset: 10) @filter((NOT (uid(MovieAuth2)) AND (uid(MovieAuth3) OR uid(MovieAuth4))))
+      MovieRoot as var(func: uid(Movie1), orderasc: Movie.content, first: 10, offset: 10) @filter((NOT (uid(MovieAuth2)) AND (uid(MovieAuth3) OR uid(MovieAuth4))))
       Movie1 as var(func: type(Movie)) @filter(eq(Movie.content, "A. N. Author"))
       MovieAuth2 as var(func: uid(Movie1)) @filter(eq(Movie.hidden, true)) @cascade
       MovieAuth3 as var(func: uid(Movie1)) @cascade {
@@ -861,7 +861,7 @@
         }
         dgraph.uid : uid
       }
-      MovieRoot as var(func: uid(Movie3), first: 10, offset: 10) @filter((NOT (uid(MovieAuth4)) AND (uid(MovieAuth5) OR uid(MovieAuth6))))
+      MovieRoot as var(func: uid(Movie3), orderasc: Movie.content, first: 10, offset: 10) @filter((NOT (uid(MovieAuth4)) AND (uid(MovieAuth5) OR uid(MovieAuth6))))
       Movie3 as var(func: type(Movie)) @filter(eq(Movie.content, "MovieXYZ"))
       MovieAuth4 as var(func: uid(Movie3)) @filter(eq(Movie.hidden, true)) @cascade
       MovieAuth5 as var(func: uid(Movie3)) @cascade {
@@ -922,7 +922,7 @@
         }
         dgraph.uid : uid
       }
-      MovieRoot as var(func: uid(Movie8), first: 10, offset: 10) @filter((NOT (uid(MovieAuth9)) AND (uid(MovieAuth10) OR uid(MovieAuth11))))
+      MovieRoot as var(func: uid(Movie8), orderasc: Movie.content, first: 10, offset: 10) @filter((NOT (uid(MovieAuth9)) AND (uid(MovieAuth10) OR uid(MovieAuth11))))
       Movie8 as var(func: type(Movie)) @filter(eq(Movie.content, "MovieXYZ"))
       MovieAuth9 as var(func: uid(Movie8)) @filter(eq(Movie.hidden, true)) @cascade
       MovieAuth10 as var(func: uid(Movie8)) @cascade {
@@ -1466,7 +1466,7 @@
         text : Post.text
         dgraph.uid : uid
       }
-      PostRoot as var(func: uid(Post1), first: 10, offset: 5) @filter(((uid(QuestionAuth2) AND uid(QuestionAuth3)) OR uid(FbPostAuth4) OR uid(AnswerAuth5)))
+      PostRoot as var(func: uid(Post1), orderdesc: Post.text, first: 10, offset: 5) @filter(((uid(QuestionAuth2) AND uid(QuestionAuth3)) OR uid(FbPostAuth4) OR uid(AnswerAuth5)))
       Post1 as var(func: type(Post)) @filter(eq(Post.text, "A Post"))
       Question1 as var(func: type(Question))
       QuestionAuth2 as var(func: uid(Question1)) @filter(eq(Question.answered, true)) @cascade {

diff --git a/graphql/resolve/query_rewriter.go b/graphql/resolve/query_rewriter.go
@@ -716,7 +716,7 @@ func (authRw *authRewriter) addAuthQueries(
 	// build a query like
 	//   Todo1 as var(func: ... ) @filter(...)
 	// that has the filter from the user query in it.  This is then used as
-	// the starting point for both the user query and the auth query.
+	// the starting point for other auth queries.
 	//
 	// We already have the query, so just copy it and modify the original
 	varQry := &gql.GraphQuery{
@@ -726,6 +726,11 @@ func (authRw *authRewriter) addAuthQueries(
 		Filter: dgQuery[0].Filter,
 	}
 
+	// build the root auth query like
+	//   TodoRoot as var(func: uid(Todo1), orderasc: ..., first: ..., offset: ...) @filter(... type auth queries ...)
+	// that has the order and pagination params from user query in it and filter set to auth
+	// queries built for this type. This is then used as the starting point for user query and
+	// auth queries for children.
 	rootQry := &gql.GraphQuery{
 		Var:  authRw.parentVarName,
 		Attr: "var",
@@ -734,15 +739,20 @@ func (authRw *authRewriter) addAuthQueries(
 			Args: []gql.Arg{{Value: authRw.varName}},
 		},
 		Filter: filter,
-		Args:   dgQuery[0].Args,
+		Order:  dgQuery[0].Order, // we need the order here for pagination to work correctly
+		Args:   dgQuery[0].Args,  // this gets pagination from user query to root query
 	}
 
+	// The user query doesn't need the filter and pagination parameters anymore,
+	// as they have been taken care of by the var and root queries generated above.
+	// But, tt still needs the order parameter, even though it is also applied in root query.
+	// So, not setting order to nil.
 	dgQuery[0].Filter = nil
 	dgQuery[0].Args = nil
 
-	// The user query starts from the var query generated above and is filtered
-	// by the the filter generated from auth processing, so now we build
-	//   queryTodo(func: uid(Todo1)) @filter(...auth-queries...) { ... }
+	// The user query starts from the root query generated above and so gets filtered
+	// input from auth processing, so now we build
+	//   queryTodo(func: uid(TodoRoot), ...) { ... }
 	dgQuery[0].Func = &gql.Function{
 		Name: "uid",
 		Args: []gql.Arg{{Value: authRw.parentVarName}},