fix(GraphQL): handle filters for enum properly (#6887) (#6916)

This PR is a breaking change for the `eq` filter on the List of `enum` Types. This PR fixes bugs in the filter generated for enum types. For the given schema:- ``` enum Status { ACTIVE DEACTIVATED } type VerificationFilter { status: [Status]! @search } ``` the `hash` filter generated for `status` was: ``` input Status_hash { eq: [Status]! in: [Status] } ``` whereas, it should be: ``` input Status_hash { eq: Status in: [Status] } ``` This PR also fixes incorrect filter generated for `exact` index in List of enum types. Earlier it was being generated like: ``` input Status_exact { eq: [Status] in: [Status] le: [Status] lt: [Status] ge: [Status] gt: [Status] between: Status } ``` but it should be like: ``` input Status_exact { eq: Status in: [Status] le: Status lt: Status ge: Status gt: Status between: Status } ``` Breaking change lies in the generation of `eq` filters. Earlier it was `eq: [Status]!` but now it is `eq: Status`. Both are syntactically fine but `eq: [Status]!` conflicts with the expected usage of `eq` filter. (cherry picked from commit 40b20c7)
hypermodeinc · Nov 19, 2020 · 2c2ceb4 · 2c2ceb4
1 parent cdba5dd
commit 2c2ceb4
Show file tree

Hide file tree

Showing 9 changed files with 123 additions and 40 deletions.
diff --git a/graphql/e2e/common/query.go b/graphql/e2e/common/query.go
@@ -1453,7 +1453,7 @@ func typenameForInterface(t *testing.T) {
 			Query: `query {
 				queryCharacter (filter: {
 					appearsIn: {
-						eq: [EMPIRE]
+						in: [EMPIRE]
 					}
 				}) {
 					name
@@ -1581,7 +1581,7 @@ func onlytypenameForInterface(t *testing.T) {
 			Query: `query {
 				queryCharacter (filter: {
 					appearsIn: {
-						eq: [EMPIRE]
+						in: [EMPIRE]
 					}
 				}) {
 
@@ -1626,7 +1626,7 @@ func defaultEnumFilter(t *testing.T) {
 			Query: `query {
 				queryCharacter (filter: {
 					appearsIn: {
-						eq: [EMPIRE]
+						in: [EMPIRE]
 					}
 				}) {
 					name
@@ -2230,7 +2230,7 @@ func queryWithCascade(t *testing.T) {
 		{
 			name: "parameterized cascade on interface ",
 			query: `query {
-						queryCharacter (filter: { appearsIn: { eq: [EMPIRE] } }) @cascade(fields:["appearsIn"]){	
+						queryCharacter (filter: { appearsIn: { in: [EMPIRE] } }) @cascade(fields:["appearsIn"]){	
 							name
 							appearsIn
 						} 

diff --git a/graphql/resolve/query_test.yaml b/graphql/resolve/query_test.yaml
@@ -17,7 +17,25 @@
     }
 
 -
-  name: "in filter on enum type"
+  name: "in filter on field which is of enum type"
+  gqlquery: |
+    query{
+      queryVerification(filter: {prevStatus: {in: [ACTIVE, DEACTIVATED]}}){
+        name
+        prevStatus
+      }
+    }
+  dgquery: |-
+    query {
+      queryVerification(func: type(Verification)) @filter(eq(Verification.prevStatus, "ACTIVE", "DEACTIVATED")) {
+        name : Verification.name
+        prevStatus : Verification.prevStatus
+        dgraph.uid : uid
+      }
+    }
+
+-
+  name: "in filter on field which is a List of enum type"
   gqlquery: |
     query{
       queryVerification(filter: {status: {in: [ACTIVE, DEACTIVATED]}}){
@@ -33,6 +51,40 @@
         dgraph.uid : uid
       }
     }
+
+- name: "eq filter on field which is a List of enum type"
+  gqlquery: |
+    query{
+      queryVerification(filter: {status: {eq: ACTIVE}}){
+        name
+        status
+      }
+    }
+  dgquery: |-
+    query {
+      queryVerification(func: type(Verification)) @filter(eq(Verification.status, "ACTIVE")) {
+        name : Verification.name
+        status : Verification.status
+        dgraph.uid : uid
+      }
+    }
+
+- name: "le filter on field which is a List of enum type"
+  gqlquery: |
+    query{
+      queryVerification(filter: {status: {le: INACTIVE}}){
+        name
+        status
+      }
+    }
+  dgquery: |-
+    query {
+      queryVerification(func: type(Verification)) @filter(le(Verification.status, "INACTIVE")) {
+        name : Verification.name
+        status : Verification.status
+        dgraph.uid : uid
+      }
+    }
 -
   name: "Point query near filter"
   gqlquery: |

diff --git a/graphql/resolve/schema.graphql b/graphql/resolve/schema.graphql
@@ -86,15 +86,17 @@ type Human implements Character & Employee {
         female: Boolean
 }
 
-# just for testing IN filter on enum types
+# just for testing filters on enum types
 
 type Verification {
   name: String @search(by: [exact])
-  status: Status @search
+  status: [Status!]! @search(by: [exact])
+  prevStatus: Status! @search
 }
 
 enum Status {
   ACTIVE
+  INACTIVE
   DEACTIVATED
 }
 

diff --git a/graphql/schema/gqlschema.go b/graphql/schema/gqlschema.go
@@ -1113,8 +1113,8 @@ func addFieldFilters(schema *ast.Schema, defn *ast.Definition) {
 		addFilterArgument(schema, fld)
 
 		// Ordering and pagination, however, only makes sense for fields of
-		// list types (not scalar lists).
-		if isTypeList(fld) {
+		// list types (not scalar lists or enum lists).
+		if isTypeList(fld) && !isEnumList(fld, schema) {
 			addOrderArgument(schema, fld)
 
 			// Pagination even makes sense when there's no orderables because
@@ -1220,16 +1220,25 @@ func getFilterTypes(schema *ast.Schema, fld *ast.FieldDefinition, filterName str
 	for i, search := range searchArgs {
 		filterNames[i] = builtInFilters[search]
 
+		// For enum type, if the index is "hash" or "exact", we construct filter named
+		// enumTypeName_hash/ enumTypeName_exact from StringHashFilter/StringExactFilter
+		// by replacing the Argument type.
 		if (search == "hash" || search == "exact") && schema.Types[fld.Type.Name()].Kind == ast.Enum {
 			stringFilterName := fmt.Sprintf("String%sFilter", strings.Title(search))
 			var l ast.FieldList
 
 			for _, i := range schema.Types[stringFilterName].Fields {
-				typ := fld.Type
-
-				// In case of IN filter we need to construct List of enums as Input Type.
-				if i.Type.Elem != nil && fld.Type.Elem == nil {
-					typ = &ast.Type{Elem: &ast.Type{NamedType: fld.Type.NamedType, NonNull: fld.Type.NonNull}}
+				enumTypeName := fld.Type.Name()
+				var typ *ast.Type
+
+				if i.Type.Elem == nil {
+					typ = &ast.Type{
+						NamedType: enumTypeName,
+					}
+				} else {
+					typ = &ast.Type{
+						Elem: &ast.Type{NamedType: enumTypeName},
+					}
 				}
 
 				l = append(l, &ast.FieldDefinition{
@@ -1356,6 +1365,12 @@ func isTypeList(fld *ast.FieldDefinition) bool {
 	return !scalar && fld.Type.Elem != nil
 }
 
+// Returns true if given field is a list of enum
+func isEnumList(fld *ast.FieldDefinition, sch *ast.Schema) bool {
+	typeDefn := sch.Types[fld.Type.Name()]
+	return typeDefn.Kind == "ENUM" && fld.Type.Elem != nil
+}
+
 func hasOrderables(defn *ast.Definition) bool {
 	return fieldAny(defn.Fields, isOrderable)
 }

diff --git a/graphql/schema/testdata/schemagen/input/searchables.graphql b/graphql/schema/testdata/schemagen/input/searchables.graphql
@@ -20,13 +20,15 @@ type Post {
 	isPublished: Boolean @search
 
 	postType: PostType @search
-        postTypeTrigram: PostType @search(by: [trigram])
-        postTypeRegexp: PostType @search(by: [regexp])
-        postTypeExact: PostType @search(by: [exact])
-        postTypeHash: PostType @search(by: [hash])
-        postTypeRegexpExact: PostType @search(by: [exact, regexp])
-        postTypeHashRegexp: PostType @search(by: [hash, regexp])
-        postTypeNone: PostType @search(by: [])
+	postTypeNonNull: PostType! @search
+	postTypeList: [PostType] @search
+	postTypeTrigram: PostType @search(by: [trigram])
+	postTypeRegexp: PostType @search(by: [regexp])
+	postTypeExact: [PostType] @search(by: [exact])
+	postTypeHash: PostType @search(by: [hash])
+	postTypeRegexpExact: PostType @search(by: [exact, regexp])
+	postTypeHashRegexp: PostType @search(by: [hash, regexp])
+	postTypeNone: PostType @search(by: [])
 }
 
 enum PostType {

diff --git a/graphql/schema/testdata/schemagen/output/interfaces-with-types-and-password.graphql b/graphql/schema/testdata/schemagen/output/interfaces-with-types-and-password.graphql
@@ -6,15 +6,15 @@ interface Character @secret(field: "password") {
 	id: ID!
 	name: String! @search(by: [exact])
 	friends(filter: CharacterFilter, order: CharacterOrder, first: Int, offset: Int): [Character]
-	appearsIn(first: Int, offset: Int): [Episode!]! @search
+	appearsIn: [Episode!]! @search
 	friendsAggregate(filter: CharacterFilter): CharacterAggregateResult
 }
 
 type Human implements Character @secret(field: "password") {
 	id: ID!
 	name: String! @search(by: [exact])
 	friends(filter: CharacterFilter, order: CharacterOrder, first: Int, offset: Int): [Character]
-	appearsIn(first: Int, offset: Int): [Episode!]! @search
+	appearsIn: [Episode!]! @search
 	starships(filter: StarshipFilter, order: StarshipOrder, first: Int, offset: Int): [Starship]
 	totalCredits: Int
 	friendsAggregate(filter: CharacterFilter): CharacterAggregateResult
@@ -25,7 +25,7 @@ type Droid implements Character @secret(field: "password") {
 	id: ID!
 	name: String! @search(by: [exact])
 	friends(filter: CharacterFilter, order: CharacterOrder, first: Int, offset: Int): [Character]
-	appearsIn(first: Int, offset: Int): [Episode!]! @search
+	appearsIn: [Episode!]! @search
 	primaryFunction: String
 	friendsAggregate(filter: CharacterFilter): CharacterAggregateResult
 }
@@ -512,8 +512,8 @@ input DroidRef {
 }
 
 input Episode_hash {
-	eq: [Episode!]!
-	in: [Episode!]!
+	eq: Episode
+	in: [Episode]
 }
 
 input HumanFilter {

diff --git a/graphql/schema/testdata/schemagen/output/interfaces-with-types.graphql b/graphql/schema/testdata/schemagen/output/interfaces-with-types.graphql
@@ -6,15 +6,15 @@ interface Character {
 	id: ID!
 	name: String! @search(by: [exact])
 	friends(filter: CharacterFilter, order: CharacterOrder, first: Int, offset: Int): [Character]
-	appearsIn(first: Int, offset: Int): [Episode!]! @search
+	appearsIn: [Episode!]! @search
 	friendsAggregate(filter: CharacterFilter): CharacterAggregateResult
 }
 
 type Human implements Character {
 	id: ID!
 	name: String! @search(by: [exact])
 	friends(filter: CharacterFilter, order: CharacterOrder, first: Int, offset: Int): [Character]
-	appearsIn(first: Int, offset: Int): [Episode!]! @search
+	appearsIn: [Episode!]! @search
 	starships(filter: StarshipFilter, order: StarshipOrder, first: Int, offset: Int): [Starship]
 	totalCredits: Int
 	friendsAggregate(filter: CharacterFilter): CharacterAggregateResult
@@ -25,7 +25,7 @@ type Droid implements Character {
 	id: ID!
 	name: String! @search(by: [exact])
 	friends(filter: CharacterFilter, order: CharacterOrder, first: Int, offset: Int): [Character]
-	appearsIn(first: Int, offset: Int): [Episode!]! @search
+	appearsIn: [Episode!]! @search
 	primaryFunction: String
 	friendsAggregate(filter: CharacterFilter): CharacterAggregateResult
 }
@@ -507,8 +507,8 @@ input DroidRef {
 }
 
 input Episode_hash {
-	eq: [Episode!]!
-	in: [Episode!]!
+	eq: Episode
+	in: [Episode]
 }
 
 input HumanFilter {

diff --git a/graphql/schema/testdata/schemagen/output/searchables.graphql b/graphql/schema/testdata/schemagen/output/searchables.graphql
@@ -20,9 +20,11 @@ type Post {
 	score: Float @search
 	isPublished: Boolean @search
 	postType: PostType @search
+	postTypeNonNull: PostType! @search
+	postTypeList: [PostType] @search
 	postTypeTrigram: PostType @search(by: [trigram])
 	postTypeRegexp: PostType @search(by: [regexp])
-	postTypeExact: PostType @search(by: [exact])
+	postTypeExact: [PostType] @search(by: [exact])
 	postTypeHash: PostType @search(by: [hash])
 	postTypeRegexpExact: PostType @search(by: [exact,regexp])
 	postTypeHashRegexp: PostType @search(by: [hash,regexp])
@@ -335,6 +337,8 @@ enum PostHasFilter {
 	score
 	isPublished
 	postType
+	postTypeNonNull
+	postTypeList
 	postTypeTrigram
 	postTypeRegexp
 	postTypeExact
@@ -379,9 +383,11 @@ input AddPostInput {
 	score: Float
 	isPublished: Boolean
 	postType: PostType
+	postTypeNonNull: PostType!
+	postTypeList: [PostType]
 	postTypeTrigram: PostType
 	postTypeRegexp: PostType
-	postTypeExact: PostType
+	postTypeExact: [PostType]
 	postTypeHash: PostType
 	postTypeRegexpExact: PostType
 	postTypeHashRegexp: PostType
@@ -406,6 +412,8 @@ input PostFilter {
 	score: FloatFilter
 	isPublished: Boolean
 	postType: PostType_hash
+	postTypeNonNull: PostType_hash
+	postTypeList: PostType_hash
 	postTypeTrigram: StringRegExpFilter
 	postTypeRegexp: StringRegExpFilter
 	postTypeExact: PostType_exact
@@ -442,9 +450,11 @@ input PostPatch {
 	score: Float
 	isPublished: Boolean
 	postType: PostType
+	postTypeNonNull: PostType
+	postTypeList: [PostType]
 	postTypeTrigram: PostType
 	postTypeRegexp: PostType
-	postTypeExact: PostType
+	postTypeExact: [PostType]
 	postTypeHash: PostType
 	postTypeRegexpExact: PostType
 	postTypeHashRegexp: PostType
@@ -469,9 +479,11 @@ input PostRef {
 	score: Float
 	isPublished: Boolean
 	postType: PostType
+	postTypeNonNull: PostType
+	postTypeList: [PostType]
 	postTypeTrigram: PostType
 	postTypeRegexp: PostType
-	postTypeExact: PostType
+	postTypeExact: [PostType]
 	postTypeHash: PostType
 	postTypeRegexpExact: PostType
 	postTypeHashRegexp: PostType

diff --git a/graphql/schema/testdata/schemagen/output/union.graphql b/graphql/schema/testdata/schemagen/output/union.graphql
@@ -7,7 +7,7 @@ interface Character {
 	name: String! @search(by: [exact])
 	friends(filter: CharacterFilter, order: CharacterOrder, first: Int, offset: Int): [Character]
 	enemyOf(filter: ResidentFilter): Resident
-	appearsIn(first: Int, offset: Int): [Episode!]! @search
+	appearsIn: [Episode!]! @search
 	friendsAggregate(filter: CharacterFilter): CharacterAggregateResult
 }
 
@@ -16,7 +16,7 @@ type Human implements Character {
 	name: String! @search(by: [exact])
 	friends(filter: CharacterFilter, order: CharacterOrder, first: Int, offset: Int): [Character]
 	enemyOf(filter: ResidentFilter): Resident
-	appearsIn(first: Int, offset: Int): [Episode!]! @search
+	appearsIn: [Episode!]! @search
 	starships(filter: StarshipFilter, order: StarshipOrder, first: Int, offset: Int): [Starship]
 	totalCredits: Int
 	friendsAggregate(filter: CharacterFilter): CharacterAggregateResult
@@ -28,7 +28,7 @@ type Droid implements Character {
 	name: String! @search(by: [exact])
 	friends(filter: CharacterFilter, order: CharacterOrder, first: Int, offset: Int): [Character]
 	enemyOf(filter: ResidentFilter): Resident
-	appearsIn(first: Int, offset: Int): [Episode!]! @search
+	appearsIn: [Episode!]! @search
 	primaryFunction: String
 	friendsAggregate(filter: CharacterFilter): CharacterAggregateResult
 }
@@ -569,8 +569,8 @@ input DroidRef {
 }
 
 input Episode_hash {
-	eq: [Episode!]!
-	in: [Episode!]!
+	eq: Episode
+	in: [Episode]
 }
 
 input HumanFilter {