Skip to content

feat(GRAPHQL): Add language tag support in GraphQL #7663

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

Merged
merged 28 commits into from
Apr 6, 2021
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
739514d
added code for supporting language tags in graphql
JatinDev543 Mar 26, 2021
bd8026a
added schema test and modified some validations`
JatinDev543 Mar 26, 2021
010468b
added invalid schema tests for language tags
JatinDev543 Mar 27, 2021
6d9640c
added unit tests for mutation and query rewriting
JatinDev543 Mar 27, 2021
480e471
Merge branch 'master' of github.com:dgraph-io/dgraph into jatin/GRAPH…
JatinDev543 Mar 27, 2021
68a36a1
added e2e test
JatinDev543 Mar 27, 2021
b0ed0ea
clean test
JatinDev543 Mar 27, 2021
4eb535d
fix test
JatinDev543 Mar 27, 2021
058906e
added schema for directive test
JatinDev543 Mar 27, 2021
6f746db
fixed formatting
JatinDev543 Mar 28, 2021
7746323
clean code
JatinDev543 Mar 28, 2021
1e3b145
clean code
JatinDev543 Mar 28, 2021
9691175
clean code, added comments
JatinDev543 Mar 28, 2021
1195b19
addressed comments
JatinDev543 Mar 31, 2021
f86b4b7
added some more tests and modified schema
JatinDev543 Apr 1, 2021
e073f83
refactor code
JatinDev543 Apr 1, 2021
af62712
clean code
JatinDev543 Apr 1, 2021
5d943ad
fix test and indentation
JatinDev543 Apr 1, 2021
1aa1078
clean comment
JatinDev543 Apr 1, 2021
6a205d8
fixed schema test
JatinDev543 Apr 1, 2021
1905eec
added error case
JatinDev543 Apr 1, 2021
887550e
modify schema for tests
JatinDev543 Apr 1, 2021
fa159b3
addressed abhimanyu's comments
JatinDev543 Apr 1, 2021
98c69f8
fixed test formatting
JatinDev543 Apr 1, 2021
fc41d46
fixed test order issue
JatinDev543 Apr 1, 2021
f794fc5
addressed abhimanyu's comments
JatinDev543 Apr 5, 2021
02273de
addressed abhimanyu's comment
JatinDev543 Apr 6, 2021
07f080d
simplify logic
abhimanyusinghgaur Apr 6, 2021
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
2 changes: 1 addition & 1 deletion graphql/e2e/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -836,7 +836,7 @@ func RunAll(t *testing.T) {
t.Run("query id directive with int", idDirectiveWithInt)
t.Run("query id directive with int64", idDirectiveWithInt64)
t.Run("query filter ID values coercion to List", queryFilterWithIDInputCoercion)

t.Run("query multiple language Fields", queryMultipleLangFields)
// mutation tests
t.Run("add mutation", addMutation)
t.Run("update mutation by ids", updateMutationByIds)
Expand Down
58 changes: 58 additions & 0 deletions graphql/e2e/common/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -3913,3 +3913,61 @@ func idDirectiveWithInt(t *testing.T) {
}`
require.JSONEq(t, expected, string(response.Data))
}

func queryMultipleLangFields(t *testing.T) {
addHotelParams := &GraphQLParams{
Query: `
mutation addPerson($person: [AddPersonInput!]!) {
addPerson(input: $person) {
person {
name
nameHi
nameZh
}
}
}`,
Variables: map[string]interface{}{"person": []interface{}{
map[string]interface{}{
"name": "Alice",
"nameHi": "ऐलिस",
"nameZh": "爱丽丝",
},
},
},
}
gqlResponse := addHotelParams.ExecuteAsPost(t, GraphqlURL)
RequireNoGQLErrors(t, gqlResponse)

queryHotel := &GraphQLParams{
Query: `
query {
queryPerson {
name
nameZh
nameHi
nameHiZh
nameHi_Zh_Untag
name_Untag_AnyLang
}
}`,
}
gqlResponse = queryHotel.ExecuteAsPost(t, GraphqlURL)
RequireNoGQLErrors(t, gqlResponse)

queryHotelExpected := `
{
"queryPerson": [
{
"name": "Alice",
"nameZh": "爱丽丝",
"nameHi": "ऐलिस",
"nameHiZh": "ऐलिस",
"nameHi_Zh_Untag": "ऐलिस",
"name_Untag_AnyLang": "Alice"
}
]
}`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also update in this e2e test ?

testutil.CompareJSON(t, queryHotelExpected, string(gqlResponse.Data))
// Cleanup
DeleteGqlType(t, "Person", map[string]interface{}{}, 1, nil)
}
10 changes: 10 additions & 0 deletions graphql/e2e/directives/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,16 @@ type Person1 {
friends: [Person1] @hasInverse(field: friends)
}

type Person {
id: ID!
name: String!
nameHi: String @dgraph(pred:"Person.name@hi")
nameZh: String @dgraph(pred:"Person.name@zh")
nameHiZh: String @dgraph(pred:"Person.name@hi:zh")
nameHi_Zh_Untag: String @dgraph(pred:"Person.name@hi:zh:.")
name_Untag_AnyLang: String @dgraph(pred:"Person.name@.")
}

# union testing - start
enum AnimalCategory {
Fish
Expand Down
13 changes: 13 additions & 0 deletions graphql/e2e/directives/schema_response.json
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@
"predicate": "Employer.worker",
"type": "uid"
},
{
"lang": true,
"predicate": "Person.name",
"type": "string"
},
{
"predicate": "Book.chapters",
"type": "uid",
Expand Down Expand Up @@ -914,6 +919,14 @@
],
"name": "Home"
},
{
"fields": [
{
"name": "Person.name"
}
],
"name": "Person"
},
{
"fields": [
{
Expand Down
9 changes: 7 additions & 2 deletions graphql/e2e/normal/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,13 @@ type Student implements People {
}

type Person @withSubscription{
id: ID!
name: String!
id: ID!
name: String!
nameHi: String @dgraph(pred:"Person.name@hi")
nameZh: String @dgraph(pred:"Person.name@zh")
nameHiZh: String @dgraph(pred:"Person.name@hi:zh")
nameHi_Zh_Untag: String @dgraph(pred:"Person.name@hi:zh:.")
name_Untag_AnyLang: String @dgraph(pred:"Person.name@.")
}

"""
Expand Down
1 change: 1 addition & 0 deletions graphql/e2e/normal/schema_response.json
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,7 @@
"upsert": true
},
{
"lang": true,
"predicate": "Person.name",
"type": "string"
},
Expand Down
23 changes: 22 additions & 1 deletion graphql/resolve/add_mutation_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4520,4 +4520,25 @@
"Friend1.id":"Main Friend",
"dgraph.type":["Friend1"],
"uid":"_:Friend1_1"
}
}

-
name: "Add mutation with language tag fields"
gqlmutation: |
mutation {
addPerson(input: { name: "Alice", nameHi: "ऐलिस",nameZh: "爱丽丝"}) {
person {
name
nameZh
nameHi
}
}
}
dgmutations:
- setjson: |
{ "Person.name":"Alice",
"Person.name@hi":"ऐलिस",
"Person.name@zh":"爱丽丝",
"dgraph.type": ["Person"],
"uid": "_:Person_1"
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Add a line at end of file.

28 changes: 27 additions & 1 deletion graphql/resolve/query_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3382,4 +3382,30 @@
dgraph.uid : uid
}
}
}
}

-
name: "query with language tag fields"
gqlquery: |
query {
queryPerson {
name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Indentation

nameZh
nameHi
nameHiZh
nameHi_Zh_Untag
name_Untag_AnyLang
}
}
dgquery: |-
query {
queryPerson(func: type(Person)) {
Person.name : Person.name
Person.nameZh : Person.name@zh
Person.nameHi : Person.name@hi
Person.nameHiZh : Person.name@hi:zh
Person.nameHi_Zh_Untag : Person.name@hi:zh:.
Person.name_Untag_AnyLang : Person.name@.
dgraph.uid : uid
}
}
5 changes: 5 additions & 0 deletions graphql/resolve/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,11 @@ type ThingTwo implements Thing {
type Person {
id: ID!
name: String @search(by: [hash])
nameHi: String @dgraph(pred:"Person.name@hi")
nameZh: String @dgraph(pred:"Person.name@zh")
nameHiZh: String @dgraph(pred:"Person.name@hi:zh")
nameHi_Zh_Untag: String @dgraph(pred:"Person.name@hi:zh:.")
name_Untag_AnyLang: String @dgraph(pred:"Person.name@.")
friends: [Person] @hasInverse(field: friends)
}

Expand Down
41 changes: 34 additions & 7 deletions graphql/schema/gqlschema.go
Original file line number Diff line number Diff line change
Expand Up @@ -1200,7 +1200,7 @@ func addUnionMemberTypeEnum(schema *ast.Schema, defn *ast.Definition) {
// it should be present in the addTypeInput as it should not be generated automatically by dgraph
// but determined by the value of field in the GraphQL service where the type is defined.
func addInputType(schema *ast.Schema, defn *ast.Definition, providesTypeMap map[string]bool) {
field := getFieldsWithoutIDType(schema, defn, providesTypeMap)
field := getFieldsWithoutIDType(schema, defn, providesTypeMap, true)
if hasExtends(defn) {
idField := getIDField(defn, providesTypeMap)
field = append(idField, field...)
Expand All @@ -1223,7 +1223,8 @@ func addReferenceType(schema *ast.Schema, defn *ast.Definition, providesTypeMap
}
flds = append(getIDField(defn, providesTypeMap), getXIDField(defn, providesTypeMap)...)
} else {
flds = append(getIDField(defn, providesTypeMap), getFieldsWithoutIDType(schema, defn, providesTypeMap)...)
flds = append(getIDField(defn, providesTypeMap),
getFieldsWithoutIDType(schema, defn, providesTypeMap, true)...)
}

if len(flds) == 1 && (hasID(defn) || hasXID(defn)) {
Expand Down Expand Up @@ -1571,7 +1572,7 @@ func addFilterType(schema *ast.Schema, defn *ast.Definition, providesTypeMap map
}

// Has filter makes sense only if there is atleast one non ID field in the defn
if len(getFieldsWithoutIDType(schema, defn, providesTypeMap)) > 0 {
if len(getFieldsWithoutIDType(schema, defn, providesTypeMap, false)) > 0 {
filter.Fields = append(filter.Fields,
&ast.FieldDefinition{Name: "has", Type: &ast.Type{Elem: &ast.Type{NamedType: defn.Name + "HasFilter"}}},
)
Expand Down Expand Up @@ -2208,7 +2209,7 @@ func getNonIDFields(schema *ast.Schema, defn *ast.Definition, providesTypeMap ma

// Ignore Fields with @external directives also as they shouldn't be present
// in the Patch Type also. If the field is an argument to `@provides` directive
// then it should be presnt.
// then it should be present.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

if externalAndNonKeyField(fld, defn, providesTypeMap) {
continue
}
Expand All @@ -2217,7 +2218,12 @@ func getNonIDFields(schema *ast.Schema, defn *ast.Definition, providesTypeMap ma
if hasCustomOrLambda(fld) {
continue
}

// We don't include fields in update patch, which corresponds to multiple language tags in dgraph
// Example, nameHi_En: String @dgraph(pred:"Person.name@hi:en")
// We don't add above field in update patch because it corresponds to multiple languages
if !isMutableLanguageField(fld) {
continue
}
// Remove edges which have a reverse predicate as they should only be updated through their
// forward edge.
fname := fieldName(fld, defn.Name)
Expand Down Expand Up @@ -2246,7 +2252,8 @@ func getNonIDFields(schema *ast.Schema, defn *ast.Definition, providesTypeMap ma
return append(fldList, pd)
}

func getFieldsWithoutIDType(schema *ast.Schema, defn *ast.Definition, providesTypeMap map[string]bool) ast.FieldList {
func getFieldsWithoutIDType(schema *ast.Schema, defn *ast.Definition,
providesTypeMap map[string]bool, addInput bool) ast.FieldList {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Rename addInput to something more meaningful like isAddingInput or something.

fldList := make([]*ast.FieldDefinition, 0)
for _, fld := range defn.Fields {
if isIDField(defn, fld) {
Expand All @@ -2264,7 +2271,10 @@ func getFieldsWithoutIDType(schema *ast.Schema, defn *ast.Definition, providesTy
if hasCustomOrLambda(fld) {
continue
}

// see also comment in getNonIDFields
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: see the comment in getNonIDFields as well.

if !isMutableLanguageField(fld) && addInput {
continue
}
// Remove edges which have a reverse predicate as they should only be updated through their
// forward edge.
fname := fieldName(fld, defn.Name)
Expand All @@ -2287,6 +2297,23 @@ func getFieldsWithoutIDType(schema *ast.Schema, defn *ast.Definition, providesTy
return append(fldList, pd)
}

// This function check if given gql field have multiple language tags
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: have --> has

func isMutableLanguageField(fld *ast.FieldDefinition) bool {
dgDirective := fld.Directives.ForName(dgraphDirective)
if dgDirective != nil {
pred := dgDirective.Arguments.ForName("pred")
if pred != nil {
if strings.Contains(pred.Value.Raw, "@") {
langs := strings.Split(pred.Value.Raw, "@")[1]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a chance of this panicking if there is a dgraph predicate with name like "somePredicate@" . Can you confirm that @ cannot be at the end of Dgraph predicate or handle this case as well.

if strings.Contains(langs, ":") || langs == "." {
return false
}
}
}
}
return true
}

func getIDField(defn *ast.Definition, providesTypeMap map[string]bool) ast.FieldList {
fldList := make([]*ast.FieldDefinition, 0)
for _, fld := range defn.Fields {
Expand Down
62 changes: 62 additions & 0 deletions graphql/schema/gqlschema_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2889,6 +2889,68 @@ invalid_schemas:
{ "message": "Type TwitterUser; @lambdaOnMutate directive not allowed along with @remote directive.", "locations": [{"line": 1, "column": 27}]}
]

- name: "language tag field should be of String type"
input: |
type Person {
name: String!
nameHi: Int @dgraph(pred:"Person.name@hi")
}
errlist: [
{ "message": "Expected `String` type for language tag field: `nameHi` but got :`Int` inside Type `Person`", "locations": [ { "line": 3, "column": 3 } ] },
]

- name: "@id directive not supported on language tag field"
input: |
type Person {
name: String!
nameHi: String! @dgraph(pred:"Person.name@hi") @id
}
errlist: [
{ "message": "@id directive not supported on language tag fields, field: `nameHi`,type: `Person`", "locations": [ { "line": 3, "column": 3 } ] },
]

- name: "unsupported `*` language tag in graphql"
input: |
type Person {
name: String!
nameHi: String @dgraph(pred:"Person.name@*")
}
errlist: [
{ "message": "`*` language tag not supported in GraphQL, field: `nameHi`,type: `Person`", "locations": [ { "line": 3, "column": 3 } ] },
]

- name: "untagged language field should be of type String"
input: |
type Person {
name: Int!
nameHi: String @dgraph(pred:"Person.name@en:hi")
}
errlist: [
{ "message": "Expected type: String, but got: `Int` for untagged language field `name` inside type: `Person`", "locations": [ { "line": 2, "column": 3 } ] },
]

- name: "Incompatible exact and hash indexes are not allowed at same time on untagged and tagged languages field "
input: |
type Person {
name: String! @search(by: [exact])
nameHi: String @dgraph(pred:"Person.name@en:hi") @search(by: [hash])
}
errlist: [
{ "message": "Incompatible indexes hash and exact are not allowed on language tagged and untagged fields, language untagged field: `name` have exact index
and language tagged field `nameHi` have hash index inside type: `Person`", "locations": [ { "line": 2, "column": 3 } ] },
]

- name: "Incompatible exact and hash indexes are not allowed at same time on tagged and untagged languages field, when hash index is given explicitly on untagged language field "
input: |
type Person {
name: String! @search(by: [hash])
nameHi: String @dgraph(pred:"Person.name@en:hi") @search(by: [exact])
}
errlist: [
{ "message": "Incompatible indexes hash and exact are not allowed on language tagged and untagged fields, language untagged field: `name` have hash index
and language tagged field `nameHi` have exact index inside type: `Person`", "locations": [ { "line": 2, "column": 3 } ] },
]

valid_schemas:
- name: "Multiple fields with @id directive should be allowed"
input: |
Expand Down
Loading