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): remove support of @id directive on Float #7583

Merged
merged 3 commits into from
Mar 16, 2021
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
2 changes: 0 additions & 2 deletions graphql/e2e/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -832,7 +832,6 @@ func RunAll(t *testing.T) {
t.Run("checkUserPassword query", passwordTest)
t.Run("query id directive with int", idDirectiveWithInt)
t.Run("query id directive with int64", idDirectiveWithInt64)
t.Run("query id directive with float", idDirectiveWithFloat)
t.Run("query filter ID values coercion to List", queryFilterWithIDInputCoercion)

// mutation tests
Expand Down Expand Up @@ -883,7 +882,6 @@ func RunAll(t *testing.T) {
t.Run("filter in update mutations with array for AND/OR", filterInUpdateMutationsWithFilterAndOr)
t.Run("mutation id directive with int", idDirectiveWithIntMutation)
t.Run("mutation id directive with int64", idDirectiveWithInt64Mutation)
t.Run("mutation id directive with float", idDirectiveWithFloatMutation)
t.Run("add mutation on extended type with field of ID type as key field", addMutationOnExtendedTypeWithIDasKeyField)
t.Run("add mutation with deep extended type objects", addMutationWithDeepExtendedTypeObjects)
t.Run("three level double XID mutation", threeLevelDoubleXID)
Expand Down
34 changes: 0 additions & 34 deletions graphql/e2e/common/mutation.go
Original file line number Diff line number Diff line change
Expand Up @@ -5032,40 +5032,6 @@ func idDirectiveWithIntMutation(t *testing.T) {
DeleteGqlType(t, "Chapter", map[string]interface{}{}, 3, nil)
}

func idDirectiveWithFloatMutation(t *testing.T) {
query := &GraphQLParams{
Query: `mutation {
addSection(input:[{
chapterId: 2
name: "Graphql: Introduction"
sectionId: 2.1
},
{
chapterId: 2
name: "Graphql Available Data Types"
sectionId: 2.2
}]) {
numUids
}
}`,
}

response := query.ExecuteAsPost(t, GraphqlURL)
RequireNoGQLErrors(t, response)
var expected = `{
"addSection": {
"numUids": 2
}
}`
require.JSONEq(t, expected, string(response.Data))

// adding same mutation again should result in error because of duplicate id
response = query.ExecuteAsPost(t, GraphqlURL)
require.Contains(t, response.Errors.Error(), "already exists")

DeleteGqlType(t, "Section", map[string]interface{}{}, 4, nil)
}

func addMutationWithDeepExtendedTypeObjects(t *testing.T) {
varMap1 := map[string]interface{}{
"missionId": "Mission1",
Expand Down
23 changes: 0 additions & 23 deletions graphql/e2e/common/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -3913,26 +3913,3 @@ func idDirectiveWithInt(t *testing.T) {
}`
require.JSONEq(t, expected, string(response.Data))
}

func idDirectiveWithFloat(t *testing.T) {
query := &GraphQLParams{
Query: `query {
getSection(sectionId: 1.1) {
chapterId
sectionId
name
}
}`,
}

response := query.ExecuteAsPost(t, GraphqlURL)
RequireNoGQLErrors(t, response)
var expected = `{
"getSection": {
"chapterId": 1,
"sectionId": 1.1,
"name": "How to define dgraph schema"
}
}`
require.JSONEq(t, expected, string(response.Data))
}
6 changes: 0 additions & 6 deletions graphql/e2e/directives/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -301,12 +301,6 @@ type Chapter {
book: Book
}

type Section {
sectionId: Float! @id
name: String!
chapterId: Int! @search
}

type Mission @key(fields: "id") {
id: String! @id
crew: [Astronaut] @provides(fields: "name") @hasInverse(field: missions)
Expand Down
35 changes: 0 additions & 35 deletions graphql/e2e/directives/schema_response.json
Original file line number Diff line number Diff line change
Expand Up @@ -401,27 +401,6 @@
"predicate": "Region.name",
"type": "string"
},
{
"predicate": "Section.chapterId",
"type": "int",
"index": true,
"tokenizer": [
"int"
]
},
{
"predicate": "Section.name",
"type": "string"
},
{
"predicate": "Section.sectionId",
"type": "float",
"index": true,
"tokenizer": [
"float"
],
"upsert": true
},
{
"predicate": "SpaceShip.id",
"type": "string",
Expand Down Expand Up @@ -1136,20 +1115,6 @@
],
"name": "Region"
},
{
"fields": [
{
"name": "Section.sectionId"
},
{
"name": "Section.name"
},
{
"name": "Section.chapterId"
}
],
"name": "Section"
},
{
"fields": [
{
Expand Down
14 changes: 0 additions & 14 deletions graphql/e2e/directives/test_data.json
Original file line number Diff line number Diff line change
Expand Up @@ -143,19 +143,5 @@
"dgraph.type": "Chapter",
"Chapter.chapterId": 1,
"Chapter.name": "How Dgraph Works"
},
{
"uid": "_:section1",
"dgraph.type": "Section",
"Section.chapterId": 1,
"Section.sectionId": 1.1,
"Section.name": "How to define dgraph schema"
},
{
"uid": "_:section2",
"dgraph.type": "Section",
"Section.chapterId": 1,
"Section.sectionId": 1.2,
"Section.name": "Dgraph Data Types"
}
]
6 changes: 0 additions & 6 deletions graphql/e2e/normal/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -307,12 +307,6 @@ type Chapter {
book: Book
}

type Section {
sectionId: Float! @id
name: String!
chapterId: Int! @search
}

# multiple fields with @id directive
type Worker {
name: String!
Expand Down
35 changes: 0 additions & 35 deletions graphql/e2e/normal/schema_response.json
Original file line number Diff line number Diff line change
Expand Up @@ -562,27 +562,6 @@
"predicate": "Region.name",
"type": "string"
},
{
"predicate": "Section.chapterId",
"type": "int",
"index": true,
"tokenizer": [
"int"
]
},
{
"predicate": "Section.name",
"type": "string"
},
{
"predicate": "Section.sectionId",
"type": "float",
"index": true,
"tokenizer": [
"float"
],
"upsert": true
},
{
"predicate": "SpaceShip.id",
"type": "string",
Expand Down Expand Up @@ -1227,20 +1206,6 @@
],
"name": "Region"
},
{
"fields": [
{
"name": "Section.sectionId"
},
{
"name": "Section.name"
},
{
"name": "Section.chapterId"
}
],
"name": "Section"
},
{
"fields": [
{
Expand Down
14 changes: 0 additions & 14 deletions graphql/e2e/normal/test_data.json
Original file line number Diff line number Diff line change
Expand Up @@ -143,19 +143,5 @@
"dgraph.type": "Chapter",
"Chapter.chapterId": 1,
"Chapter.name": "How Dgraph Works"
},
{
"uid": "_:section1",
"dgraph.type": "Section",
"Section.chapterId": 1,
"Section.sectionId": 1.1,
"Section.name": "How to define dgraph schema"
},
{
"uid": "_:section2",
"dgraph.type": "Section",
"Section.chapterId": 1,
"Section.sectionId": 1.2,
"Section.name": "Dgraph Data Types"
}
]
14 changes: 0 additions & 14 deletions graphql/resolve/mutation_rewriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -2318,20 +2318,6 @@ func extractVal(xidVal interface{}, xidName, typeName string) (string, error) {
return "", fmt.Errorf("encountered an XID %s with %s that isn't "+
"a Int64 but data type in schema is Int64", xidName, typeName)
}
case "Float":
switch xVal := xidVal.(type) {
case json.Number:
val, err := xVal.Float64()
if err != nil {
return "", err
}
return strconv.FormatFloat(val, 'f', -1, 64), nil
case float64:
return strconv.FormatFloat(xVal, 'f', -1, 64), nil
default:
return "", fmt.Errorf("encountered an XID %s with %s that isn't "+
"a Float but data type in schema is Float", xidName, typeName)
}
// "ID" is given as input for the @extended type mutation.
case "String", "ID":
xidString, ok := xidVal.(string)
Expand Down
14 changes: 0 additions & 14 deletions graphql/schema/dgraph_schemagen_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -760,20 +760,6 @@ schemas:
T.id: int @index(int) @upsert .
T.value: string .

- name: "Float field with @id Directive"
input: |
type T {
id : Float! @id
value: String
}
output: |
type T {
T.id
T.value
}
T.id: float @index(float) @upsert .
T.value: string .

- name: "type extension having @external field of ID type which is @key"
input: |
extend type Product @key(fields: "id") {
Expand Down
17 changes: 14 additions & 3 deletions graphql/schema/gqlschema_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ invalid_schemas:
errlist: [
{ "message": "Type Z; Field f: Field f is of type U, but @hasInverse directive only applies to fields with object types.", "locations": [{"line":9, "column":3}]},
{ "message": "Type Z; Field f: has the @search directive but fields of type U can't have the @search directive.", "locations": [{"line":9, "column":34}]},
{ "message": "Type Z; Field f: with @id directive must be of type String!, Int!, Int64! or Float!, not U", "locations": [{"line":9, "column":42}]}
{ "message": "Type Z; Field f: with @id directive must be of type String!, Int! or Int64!, not U", "locations": [{"line":9, "column":42}]}
]

-
Expand Down Expand Up @@ -622,18 +622,29 @@ invalid_schemas:
f1: [String] @id
}
errlist: [
{"message": "Type X; Field f1: with @id directive must be of type String!, Int!, Int64! or Float!, not [String]",
{"message": "Type X; Field f1: with @id directive must be of type String!, Int! or Int64!, not [String]",
"locations":[{"line":2, "column":17}]}
]

-
name: "@id directive can't be applied on field with Float type"
input: |
type X {
f1: Float! @id
}
errlist: [
{"message": "Type X; Field f1: with @id directive must be of type String!, Int! or Int64!, not Float!",
"locations":[{"line":2, "column":15}]}
]

-
name: "Field with @id directive should be mandatory"
input: |
type X {
f1: String @id
}
errlist: [
{"message": "Type X; Field f1: with @id directive must be of type String!, Int!, Int64! or Float!, not String",
{"message": "Type X; Field f1: with @id directive must be of type String!, Int! or Int64!, not String",
"locations":[{"line":2, "column":15}]}
]

Expand Down
5 changes: 2 additions & 3 deletions graphql/schema/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -1998,13 +1998,12 @@ func idValidation(sch *ast.Schema,
secrets map[string]x.SensitiveByteSlice) gqlerror.List {
if field.Type.String() == "String!" ||
field.Type.String() == "Int!" ||
field.Type.String() == "Int64!" ||
field.Type.String() == "Float!" {
field.Type.String() == "Int64!" {
return nil
}
return []*gqlerror.Error{gqlerror.ErrorPosf(
dir.Position,
"Type %s; Field %s: with @id directive must be of type String!, Int!, Int64! or Float!, not %s",
"Type %s; Field %s: with @id directive must be of type String!, Int! or Int64!, not %s",
typ.Name, field.Name, field.Type.String())}
}

Expand Down
2 changes: 0 additions & 2 deletions graphql/schema/schemagen.go
Original file line number Diff line number Diff line change
Expand Up @@ -654,8 +654,6 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string,
switch f.Type.Name() {
case "Int", "Int64":
indexes = append(indexes, "int")
case "Float":
indexes = append(indexes, "float")
case "String", "ID":
if !x.HasString(indexes, "exact") {
indexes = append(indexes, "hash")
Expand Down
2 changes: 0 additions & 2 deletions graphql/schema/wrappers.go
Original file line number Diff line number Diff line change
Expand Up @@ -1372,8 +1372,6 @@ func (f *field) IDArgValue() (xids map[string]string, uid uint64, err error) {
switch v := f.ArgValue(xidArgName).(type) {
case int64:
xidArgVal = strconv.FormatInt(v, 10)
case float64:
xidArgVal = strconv.FormatFloat(v, 'f', -1, 64)
case string:
xidArgVal = v
default:
Expand Down