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

fix(GraphQL): fix exclusion of filters in Query generation in case of… #6917

Merged
merged 2 commits into from
Nov 20, 2020
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
14 changes: 14 additions & 0 deletions graphql/resolve/query_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -792,6 +792,20 @@
}
}

- name: "Query Has Filter on type which has neither ID field nor any search argument"
gqlquery: |
query {
queryNode(filter: {has: name}){
name
}
}
dgquery: |-
query {
queryNode(func: type(Node)) @filter(has(Node.name)) {
name : Node.name
dgraph.uid : uid
}
}
-
name: "Filters in same input object implies AND"
gqlquery: |
Expand Down
47 changes: 34 additions & 13 deletions graphql/schema/gqlschema.go
Original file line number Diff line number Diff line change
Expand Up @@ -892,39 +892,52 @@ func cleanupInput(sch *ast.Schema, def *ast.Definition, seen map[string]bool) {
}
def.Fields = def.Fields[:i]

if len(def.Fields) == 0 {
// In case of UpdateTypeInput, if TypePatch gets cleaned up then it becomes
// input UpdateTypeInput {
// filter: TypeFilter!
// }
// In this case, UpdateTypeInput should also be deleted.
if len(def.Fields) == 0 || (strings.HasPrefix(def.Name, "Update") && len(def.Fields) == 1) {
delete(sch.Types, def.Name)
}
}

func cleanSchema(sch *ast.Schema) {
// Let's go over inputs of the type TRef, TPatch and AddTInput and delete the ones which
// Let's go over inputs of the type TRef, TPatch AddTInput, UpdateTInput and delete the ones which
// don't have field inside them.
for k := range sch.Types {
if strings.HasSuffix(k, "Ref") || strings.HasSuffix(k, "Patch") ||
(strings.HasPrefix(k, "Add") && strings.HasSuffix(k, "Input")) {
((strings.HasPrefix(k, "Add") || strings.HasPrefix(k, "Update")) && strings.HasSuffix(k, "Input")) {
cleanupInput(sch, sch.Types[k], map[string]bool{})
}
}

// Let's go over mutations and cleanup those which don't have AddTInput defined in the schema
// Let's go over mutations and cleanup those which don't have AddTInput/UpdateTInput defined in the schema
// anymore.
i := 0 // helps us overwrite the array with valid entries.
for _, field := range sch.Mutation.Fields {
custom := field.Directives.ForName("custom")
// We would only modify add type queries.
if custom != nil || !strings.HasPrefix(field.Name, "add") {
// We would only modify add/update
if custom != nil || !(strings.HasPrefix(field.Name, "add") || strings.HasPrefix(field.Name, "update")) {
sch.Mutation.Fields[i] = field
i++
continue
}

// addT type mutations have an input which is AddTInput so if that doesn't exist anymore,
// we can delete the AddTPayload and also skip this mutation.
typ := field.Name[3:]
input := sch.Types["Add"+typ+"Input"]
if input == nil {
delete(sch.Types, "Add"+typ+"Payload")
// addT / updateT type mutations have an input which is AddTInput / UpdateTInput so if that doesn't exist anymore,
// we can delete the AddTPayload / UpdateTPayload and also skip this mutation.

var typeName, input string
if strings.HasPrefix(field.Name, "add") {
typeName = field.Name[3:]
input = "Add" + typeName + "Input"
} else if strings.HasPrefix(field.Name, "update") {
typeName = field.Name[6:]
input = "Update" + typeName + "Input"
}

if sch.Types[input] == nil {
delete(sch.Types, input)
continue
}
sch.Mutation.Fields[i] = field
Expand Down Expand Up @@ -1155,6 +1168,11 @@ func addFilterArgument(schema *ast.Schema, fld *ast.FieldDefinition) {
}

func addFilterArgumentForField(schema *ast.Schema, fld *ast.FieldDefinition, fldTypeName string) {
// Don't add filters for inbuilt types like String, Point, Polygon ...
if _, ok := inbuiltTypeToDgraph[fldTypeName]; ok {
return
}

fldType := schema.Types[fldTypeName]
if fldType.Kind == ast.Union || hasFilterable(fldType) {
fld.Arguments = append(fld.Arguments,
Expand Down Expand Up @@ -1342,10 +1360,13 @@ func addFilterType(schema *ast.Schema, defn *ast.Definition) {
schema.Types[filterName] = filter
}

// hasFilterable Returns whether TypeFilter for a defn will be generated or not.
// It returns true if any field have search arguments or it is an `ID` field or
// there is atleast one non-custom filter which would be the part of the has filter.
func hasFilterable(defn *ast.Definition) bool {
return fieldAny(defn.Fields,
func(fld *ast.FieldDefinition) bool {
return len(getSearchArgs(fld)) != 0 || isID(fld)
return len(getSearchArgs(fld)) != 0 || isID(fld) || !hasCustomOrLambda(fld)
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,12 @@ type AddTPayload {
numUids: Int
}

type DeleteIPayload {
i(filter: IFilter, order: IOrder, first: Int, offset: Int): [I]
msg: String
numUids: Int
}

type DeleteTPayload {
t(filter: TFilter, order: TOrder, first: Int, offset: Int): [T]
msg: String
Expand All @@ -311,6 +317,11 @@ type TAggregateResult {
count: Int
}

type UpdateIPayload {
i(filter: IFilter, order: IOrder, first: Int, offset: Int): [I]
numUids: Int
}

type UpdateTPayload {
t(filter: TFilter, order: TOrder, first: Int, offset: Int): [T]
numUids: Int
Expand Down Expand Up @@ -374,6 +385,11 @@ input IOrder {
then: IOrder
}

input IPatch {
"""Desc"""
s: String
}

input TFilter {
id: [ID!]
has: THasFilter
Expand Down Expand Up @@ -401,6 +417,12 @@ input TRef {
i: Int
}

input UpdateIInput {
filter: IFilter!
set: IPatch
remove: IPatch
}

input UpdateTInput {
filter: TFilter!
set: TPatch
Expand All @@ -412,8 +434,8 @@ input UpdateTInput {
#######################

type Query {
queryI(order: IOrder, first: Int, offset: Int): [I]
aggregateI: IAggregateResult
queryI(filter: IFilter, order: IOrder, first: Int, offset: Int): [I]
aggregateI(filter: IFilter): IAggregateResult
getT(id: ID!): T
queryT(filter: TFilter, order: TOrder, first: Int, offset: Int): [T]
aggregateT(filter: TFilter): TAggregateResult
Expand All @@ -424,6 +446,8 @@ type Query {
#######################

type Mutation {
updateI(input: UpdateIInput!): UpdateIPayload
deleteI(filter: IFilter!): DeleteIPayload
addT(input: [AddTInput!]!): AddTPayload
updateT(input: UpdateTInput!): UpdateTPayload
deleteT(filter: TFilter!): DeleteTPayload
Expand Down
30 changes: 27 additions & 3 deletions graphql/schema/testdata/schemagen/output/deprecated.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -267,14 +267,25 @@ input StringHashFilter {
#######################

type AddAtypePayload {
atype(order: AtypeOrder, first: Int, offset: Int): [Atype]
atype(filter: AtypeFilter, order: AtypeOrder, first: Int, offset: Int): [Atype]
numUids: Int
}

type AtypeAggregateResult {
count: Int
}

type DeleteAtypePayload {
atype(filter: AtypeFilter, order: AtypeOrder, first: Int, offset: Int): [Atype]
msg: String
numUids: Int
}

type UpdateAtypePayload {
atype(filter: AtypeFilter, order: AtypeOrder, first: Int, offset: Int): [Atype]
numUids: Int
}

#######################
# Generated Enums
#######################
Expand Down Expand Up @@ -311,18 +322,29 @@ input AtypeOrder {
then: AtypeOrder
}

input AtypePatch {
iamDeprecated: String
soAmI: String
}

input AtypeRef {
iamDeprecated: String
soAmI: String
}

input UpdateAtypeInput {
filter: AtypeFilter!
set: AtypePatch
remove: AtypePatch
}

#######################
# Generated Query
#######################

type Query {
queryAtype(order: AtypeOrder, first: Int, offset: Int): [Atype]
aggregateAtype: AtypeAggregateResult
queryAtype(filter: AtypeFilter, order: AtypeOrder, first: Int, offset: Int): [Atype]
aggregateAtype(filter: AtypeFilter): AtypeAggregateResult
}

#######################
Expand All @@ -331,5 +353,7 @@ type Query {

type Mutation {
addAtype(input: [AddAtypeInput!]!): AddAtypePayload
updateAtype(input: UpdateAtypeInput!): UpdateAtypePayload
deleteAtype(filter: AtypeFilter!): DeleteAtypePayload
}

Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,20 @@
#######################

type X {
name(first: Int, offset: Int): [Y]
f1(first: Int, offset: Int): [Y] @dgraph(pred: "f1")
nameAggregate: YAggregateResult
f1Aggregate: YAggregateResult
name(filter: YFilter, first: Int, offset: Int): [Y]
f1(filter: YFilter, first: Int, offset: Int): [Y] @dgraph(pred: "f1")
nameAggregate(filter: YFilter): YAggregateResult
f1Aggregate(filter: YFilter): YAggregateResult
}

type Y {
f1(first: Int, offset: Int): [X] @dgraph(pred: "~f1")
f1Aggregate: XAggregateResult
f1(filter: XFilter, first: Int, offset: Int): [X] @dgraph(pred: "~f1")
f1Aggregate(filter: XFilter): XAggregateResult
}

type Z {
add(first: Int, offset: Int): [X]
addAggregate: XAggregateResult
add(filter: XFilter, first: Int, offset: Int): [X]
addAggregate(filter: XFilter): XAggregateResult
}

#######################
Expand Down Expand Up @@ -278,6 +278,44 @@ input StringHashFilter {
# Generated Types
#######################

type AddXPayload {
x(filter: XFilter, first: Int, offset: Int): [X]
numUids: Int
}

type AddZPayload {
z(filter: ZFilter, first: Int, offset: Int): [Z]
numUids: Int
}

type DeleteXPayload {
x(filter: XFilter, first: Int, offset: Int): [X]
msg: String
numUids: Int
}

type DeleteYPayload {
y(filter: YFilter, first: Int, offset: Int): [Y]
msg: String
numUids: Int
}

type DeleteZPayload {
z(filter: ZFilter, first: Int, offset: Int): [Z]
msg: String
numUids: Int
}

type UpdateXPayload {
x(filter: XFilter, first: Int, offset: Int): [X]
numUids: Int
}

type UpdateZPayload {
z(filter: ZFilter, first: Int, offset: Int): [Z]
numUids: Int
}

type XAggregateResult {
count: Int
}
Expand Down Expand Up @@ -334,11 +372,21 @@ input ZFilter {
#######################

type Query {
queryX(first: Int, offset: Int): [X]
aggregateX: XAggregateResult
queryY(first: Int, offset: Int): [Y]
aggregateY: YAggregateResult
queryZ(first: Int, offset: Int): [Z]
aggregateZ: ZAggregateResult
queryX(filter: XFilter, first: Int, offset: Int): [X]
aggregateX(filter: XFilter): XAggregateResult
queryY(filter: YFilter, first: Int, offset: Int): [Y]
aggregateY(filter: YFilter): YAggregateResult
queryZ(filter: ZFilter, first: Int, offset: Int): [Z]
aggregateZ(filter: ZFilter): ZAggregateResult
}

#######################
# Generated Mutations
#######################

type Mutation {
deleteX(filter: XFilter!): DeleteXPayload
deleteY(filter: YFilter!): DeleteYPayload
deleteZ(filter: ZFilter!): DeleteZPayload
}

Loading