From 60b64ccb77dd7630f0a481b0e71ee36649b9b330 Mon Sep 17 00:00:00 2001 From: Abhimanyu Singh Gaur <12651351+abhimanyusinghgaur@users.noreply.github.com> Date: Mon, 31 Aug 2020 10:58:04 +0530 Subject: [PATCH] fix(GraphQL): fixes unexpected fragment behaviour (#6228) (#6274) Fixes: #5516 Fixes: [Discuss Issue](https://discuss.dgraph.io/t/graphql-fragments-generates-unexpected-behaviour/7470) (cherry picked from commit dcce8bf62a73683f7aae3306c5b3f184b9e18682) --- go.mod | 4 +- go.sum | 24 ++- graphql/e2e/common/fragment.go | 128 +++++++++++++- graphql/e2e/common/mutation.go | 91 +++++++++- graphql/e2e/common/query.go | 2 + .../e2e/directives/dgraph_directives_test.go | 40 ++++- graphql/e2e/directives/schema.graphql | 19 ++ graphql/e2e/normal/normal_test.go | 52 +++++- graphql/e2e/normal/schema.graphql | 19 ++ graphql/resolve/mutation.go | 16 +- graphql/resolve/query.go | 2 +- graphql/resolve/query_rewriter.go | 22 ++- graphql/resolve/query_test.yaml | 91 ++++++++++ graphql/resolve/resolver.go | 12 +- graphql/resolve/schema.graphql | 21 +++ graphql/schema/request.go | 86 +++++++-- graphql/schema/wrappers.go | 164 +++++++++++++++--- 17 files changed, 714 insertions(+), 79 deletions(-) diff --git a/go.mod b/go.mod index 1fbe2c794c3..2583ee62c56 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.12 require ( contrib.go.opencensus.io/exporter/jaeger v0.1.0 contrib.go.opencensus.io/exporter/prometheus v0.1.0 - github.com/99designs/gqlgen v0.11.0 + github.com/99designs/gqlgen v0.12.2 github.com/DataDog/datadog-go v0.0.0-20190425163447-40bafcb5f6c1 // indirect github.com/DataDog/opencensus-go-exporter-datadog v0.0.0-20190503082300-0f32ad59ab08 github.com/DataDog/zstd v1.4.5 // indirect @@ -35,7 +35,7 @@ require ( github.com/google/codesearch v1.0.0 github.com/google/go-cmp v0.4.0 github.com/google/uuid v1.0.0 - github.com/gorilla/websocket v1.4.1 + github.com/gorilla/websocket v1.4.2 github.com/graph-gophers/graphql-go v0.0.0-20200309224638-dae41bde9ef9 github.com/graph-gophers/graphql-transport-ws v0.0.0-20190611222414-40c048432299 // indirect github.com/hashicorp/vault/api v1.0.4 diff --git a/go.sum b/go.sum index be4345bf0f9..6553204dd19 100644 --- a/go.sum +++ b/go.sum @@ -4,8 +4,8 @@ contrib.go.opencensus.io/exporter/jaeger v0.1.0 h1:WNc9HbA38xEQmsI40Tjd/MNU/g8by contrib.go.opencensus.io/exporter/jaeger v0.1.0/go.mod h1:VYianECmuFPwU37O699Vc1GOcy+y8kOsfaxHRImmjbA= contrib.go.opencensus.io/exporter/prometheus v0.1.0 h1:SByaIoWwNgMdPSgl5sMqM2KDE5H/ukPWBRo314xiDvg= contrib.go.opencensus.io/exporter/prometheus v0.1.0/go.mod h1:cGFniUXGZlKRjzOyuZJ6mgB+PgBcCIa79kEKR8YCW+A= -github.com/99designs/gqlgen v0.11.0 h1:7MVbtFYo4IVV8ejJzqs9n+0VNP3HdJhJOaaxFV1OLnA= -github.com/99designs/gqlgen v0.11.0/go.mod h1:vjFOyBZ7NwDl+GdSD4PFn7BQn5Fy7ohJwXn7Vk8zz+c= +github.com/99designs/gqlgen v0.12.2 h1:aOdpsiCycFtCnAv8CAI1exnKrIDHMqtMzQoXeTziY4o= +github.com/99designs/gqlgen v0.12.2/go.mod h1:7zdGo6ry9u1YBp/qlb2uxSU5Mt2jQKLcBETQiKk+Bxo= github.com/AndreasBriese/bbloom v0.0.0-20190306092124-e2d15f34fcf9/go.mod h1:bOvUY6CB00SOBii9/FifXqc0awNKxLFCL/+pkDPuyl8= github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= @@ -70,7 +70,10 @@ github.com/coreos/go-etcd v2.0.0+incompatible/go.mod h1:Jez6KQU2B/sWsbdaef3ED8Nz github.com/coreos/go-semver v0.2.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk= github.com/coreos/go-systemd v0.0.0-20180511133405-39ca1b05acc7/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4= github.com/coreos/pkg v0.0.0-20160727233714-3ac0863d7acf/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= +github.com/cpuguy83/go-md2man v1.0.10 h1:BSKMNlYxDvnunlTymqtgONjNnaRV1sTpcovwwjF22jk= github.com/cpuguy83/go-md2man v1.0.10/go.mod h1:SmD6nW6nTyfqj6ABTjUi3V3JVMnlJmwcJI5acqYI6dE= +github.com/cpuguy83/go-md2man/v2 v2.0.0-20190314233015-f79a8a8ca69d h1:U+s90UTSYgptZMwQh2aRr3LuazLJIa+Pg3Kc1ylSYVY= +github.com/cpuguy83/go-md2man/v2 v2.0.0-20190314233015-f79a8a8ca69d/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= github.com/d4l3k/messagediff v1.2.1 h1:ZcAIMYsUg0EAp9X+tt8/enBE/Q8Yd5kzPynLyKptt9U= github.com/d4l3k/messagediff v1.2.1/go.mod h1:Oozbb1TVXFac9FtSIxHBMnBCq2qeH/2KkEQxENCrlLo= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -171,10 +174,9 @@ github.com/gorilla/context v1.1.1/go.mod h1:kBGZzfjB9CEq2AlWe17Uuf7NDRt0dE0s8S51 github.com/gorilla/mux v1.6.1/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs= github.com/gorilla/mux v1.6.2/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs= github.com/gorilla/websocket v0.0.0-20170926233335-4201258b820c/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ= -github.com/gorilla/websocket v1.2.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ= github.com/gorilla/websocket v1.4.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ= -github.com/gorilla/websocket v1.4.1 h1:q7AeDBpnBk8AogcD4DSag/Ukw/KV+YhzLj2bP5HvKCM= -github.com/gorilla/websocket v1.4.1/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= +github.com/gorilla/websocket v1.4.2 h1:+/TMaTYc4QFitKJxsQ7Yye35DkWvkdLcvGKqM+x0Ufc= +github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= github.com/graph-gophers/graphql-go v0.0.0-20200309224638-dae41bde9ef9 h1:kLnsdud6Fl1/7ZX/5oD23cqYAzBfuZBhNkGr2NvuEsU= github.com/graph-gophers/graphql-go v0.0.0-20200309224638-dae41bde9ef9/go.mod h1:9CQHMSxwO4MprSdzoIEobiHpoLtHm77vfxsvsIN5Vuc= github.com/graph-gophers/graphql-transport-ws v0.0.0-20190611222414-40c048432299 h1:BdXUpuP9yOGKcwD/mhhZ+6EeAmrPrAQA3yeq4YEOHL4= @@ -257,6 +259,7 @@ github.com/lib/pq v1.0.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= github.com/logrusorgru/aurora v0.0.0-20200102142835-e9ef32dff381/go.mod h1:7rIyQOR62GCctdiQpZ/zOJlFyk6y+94wXzv6RNZgaR4= github.com/magiconair/properties v1.8.0 h1:LLgXmsheXeRoUOBOjtwPQCWIYqM/LU1ayDtDePerRcY= github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= +github.com/matryer/moq v0.0.0-20200106131100-75d0ddfc0007 h1:reVOUXwnhsYv/8UqjvhrMOu5CNT9UapHFLbQ2JcXsmg= github.com/matryer/moq v0.0.0-20200106131100-75d0ddfc0007/go.mod h1:9ELz6aaclSIGnZBoaSLZ3NAl1VTufbOrXBPvtcy6WiQ= github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= github.com/mattn/go-colorable v0.1.2/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE= @@ -363,7 +366,10 @@ github.com/prometheus/procfs v0.0.0-20190517135640-51af30a78b0e h1:zK8d1aZ+gw/Ne github.com/prometheus/procfs v0.0.0-20190517135640-51af30a78b0e/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsTZCD3I8kEA= github.com/rcrowley/go-metrics v0.0.0-20181016184325-3113b8401b8a/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4= github.com/rs/cors v1.6.0/go.mod h1:gFx+x8UowdsKA9AchylcLynDq+nNFfI8FkUZdN/jGCU= +github.com/russross/blackfriday v1.5.2 h1:HyvC0ARfnZBqnXwABFeSZHpKvJHJJfPz81GNueLj0oo= github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g= +github.com/russross/blackfriday/v2 v2.0.1 h1:lPqVAte+HuHNfhJ/0LC98ESWRz8afy9tM/0RK8m9o+Q= +github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/ryanuber/columnize v2.1.0+incompatible/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts= github.com/ryanuber/go-glob v1.0.0 h1:iQh3xXAumdQ+4Ufa5b25cRpC5TYKlno6hsv6Cb3pkBk= github.com/ryanuber/go-glob v1.0.0/go.mod h1:807d1WSdnB0XRJzKNil9Om6lcp/3a0v4qIHxIXzX/Yc= @@ -371,6 +377,7 @@ github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAm github.com/sergi/go-diff v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0= github.com/sergi/go-diff v1.1.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM= github.com/shurcooL/httpfs v0.0.0-20171119174359-809beceb2371/go.mod h1:ZY1cvUeJuFPAdZ/B6v7RHavJWZn2YPVFQ1OSXhCGOkg= +github.com/shurcooL/sanitized_anchor_name v1.0.0 h1:PdmoCO6wvbs+7yrJyMORt4/BmY5IYyJwS/kOiWx8mHo= github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc= github.com/shurcooL/vfsgen v0.0.0-20180121065927-ffb13db8def0/go.mod h1:TrYk7fJVaAttu97ZZKrO9UbRa8izdowaMIZcxYMbVaw= github.com/sirupsen/logrus v1.0.5/go.mod h1:pMByvHTf9Beacp5x1UXfOR9xyW/9antXMhjMPG0dEzc= @@ -420,13 +427,19 @@ github.com/ugorji/go v1.1.7/go.mod h1:kZn38zHttfInRq0xu/PH0az30d+z6vm202qpg1oXVM github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0= github.com/ugorji/go/codec v0.0.0-20190204201341-e444a5086c43/go.mod h1:iT03XoTwV7xq/+UGwKO3UbC1nNNlopQiY61beSdrtOA= github.com/ugorji/go/codec v1.1.7/go.mod h1:Ax+UKWsSmolVDwsd+7N3ZtXu+yMGCf907BLYF3GoBXY= +github.com/urfave/cli v1.20.0 h1:fDqGv3UG/4jbVl/QkFwEdddtEDjh/5Ov6X+0B/3bPaw= github.com/urfave/cli v1.20.0/go.mod h1:70zkFmudgCuE/ngEzBv17Jvp/497gISqfk5gWijbERA= +github.com/urfave/cli/v2 v2.1.1 h1:Qt8FeAtxE/vfdrLmR3rxR6JRE0RoVmbXu8+6kZtYU4k= +github.com/urfave/cli/v2 v2.1.1/go.mod h1:SE9GqnLQmjVa0iPEY0f1w3ygNIYcIJ0OKPMoW2caLfQ= github.com/urfave/negroni v1.0.0/go.mod h1:Meg73S6kFm/4PpbYdq35yYWoCZ9mS/YSx+lKnmiohz4= github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= github.com/valyala/fasthttp v1.6.0/go.mod h1:FstJa9V+Pj9vQ7OJie2qMHdwemEDaDiSdBnvPM1Su9w= github.com/valyala/fasttemplate v1.0.1/go.mod h1:UQGH1tvbgY+Nz5t2n7tXsz52dQxojPUpymEIMZ47gx8= github.com/valyala/tcplisten v0.0.0-20161114210144-ceec8f93295a/go.mod h1:v3UYOV9WzVtRmSR+PDvWpU/qWl4Wa5LApYYX4ZtKbio= +github.com/vektah/dataloaden v0.2.1-0.20190515034641-a19b9a6e7c9e h1:+w0Zm/9gaWpEAyDlU1eKOuk5twTjAjuevXqcJJw8hrg= github.com/vektah/dataloaden v0.2.1-0.20190515034641-a19b9a6e7c9e/go.mod h1:/HUdMve7rvxZma+2ZELQeNh88+003LL7Pf/CZ089j8U= +github.com/vektah/gqlparser v1.3.1 h1:8b0IcD3qZKWJQHSzynbDlrtP3IxVydZ2DZepCGofqfU= +github.com/vektah/gqlparser v1.3.1/go.mod h1:bkVf0FX+Stjg/MHnm8mEyubuaArhNEqfQhF+OTiAL74= github.com/vektah/gqlparser/v2 v2.0.1 h1:xgl5abVnsd4hkN9rk65OJID9bfcLSMuTaTcZj777q1o= github.com/vektah/gqlparser/v2 v2.0.1/go.mod h1:SyUiHgLATUR8BiYURfTirrTcGpcE+4XkV2se04Px1Ms= github.com/willf/bitset v0.0.0-20181014161241-71fa2377963f h1:gpNz6yJT2E7nm4WlhFendQ32tHE3uGE6P6lARnQgBnQ= @@ -529,6 +542,7 @@ golang.org/x/tools v0.0.0-20190327201419-c70d86f8b7cf/go.mod h1:LCzVGOaR6xXOjkQ3 golang.org/x/tools v0.0.0-20190328211700-ab21143f2384/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs= golang.org/x/tools v0.0.0-20190515012406-7d7faa4812bd/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= +golang.org/x/tools v0.0.0-20200114235610-7ae403b6b589 h1:rjUrONFu4kLchcZTfp3/96bR8bW8dIa8uz3cR5n0cgM= golang.org/x/tools v0.0.0-20200114235610-7ae403b6b589/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= diff --git a/graphql/e2e/common/fragment.go b/graphql/e2e/common/fragment.go index b3e57d9e281..de91dc515ae 100644 --- a/graphql/e2e/common/fragment.go +++ b/graphql/e2e/common/fragment.go @@ -1,3 +1,19 @@ +/* + * Copyright 2020 Dgraph Labs, Inc. and Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package common import ( @@ -34,12 +50,12 @@ func fragmentInMutation(t *testing.T) { gqlResponse := addStarshipParams.ExecuteAsPost(t, graphqlURL) RequireNoGQLErrors(t, gqlResponse) - addStarshipExpected := fmt.Sprintf(`{"addStarship":{ + addStarshipExpected := `{"addStarship":{ "starship":[{ "name":"Millennium Falcon", "length":2 }] - }}`) + }}` var expected, result struct { AddStarship struct { @@ -112,13 +128,62 @@ func fragmentInQueryOnInterface(t *testing.T) { newStarship := addStarship(t) humanID := addHuman(t, newStarship.ID) droidID := addDroid(t) + thingOneId := addThingOne(t) + thingTwoId := addThingTwo(t) queryCharacterParams := &GraphQLParams{ Query: `query { - queryCharacter(filter: null) { + queryCharacter { __typename ...fullCharacterFrag } + qc: queryCharacter { + __typename + ... on Character { + ... on Character { + ... on Human { + ... on Human { + id + name + } + } + } + } + ... droidAppearsIn + } + qc1: queryCharacter { + ... on Human { + __typename + id + } + ... on Droid { + id + } + } + queryThing { + __typename + ... on ThingOne { + id + name + color + usedBy + } + ... on ThingTwo { + id + name + color + owner + } + } + qt: queryThing { + ... on ThingOne { + __typename + id + } + ... on ThingTwo { + __typename + } + } } fragment fullCharacterFrag on Character { __typename @@ -149,6 +214,9 @@ func fragmentInQueryOnInterface(t *testing.T) { __typename primaryFunction } + fragment droidAppearsIn on Droid { + appearsIn + } `, } @@ -179,12 +247,56 @@ func fragmentInQueryOnInterface(t *testing.T) { "appearsIn": ["EMPIRE"], "primaryFunction": "Robot" } + ], + "qc":[ + { + "__typename": "Human", + "id": "%s", + "name": "Han" + }, + { + "__typename": "Droid", + "appearsIn": ["EMPIRE"] + } + ], + "qc1":[ + { + "__typename": "Human", + "id": "%s" + }, + { + "id": "%s" + } + ], + "queryThing":[ + { + "__typename": "ThingOne", + "id": "%s", + "name": "Thing-1", + "color": "White", + "usedBy": "me" + }, + { + "__typename": "ThingTwo", + "id": "%s", + "name": "Thing-2", + "color": "Black", + "owner": "someone" + } + ], + "qt":[ + { + "__typename": "ThingOne", + "id": "%s" + }, + { + "__typename": "ThingTwo" + } ] - }`, humanID, newStarship.ID, droidID) + }`, humanID, newStarship.ID, droidID, humanID, humanID, droidID, thingOneId, thingTwoId, + thingOneId) - var expected, result struct { - QueryCharacter []map[string]interface{} - } + var expected, result map[string]interface{} err := json.Unmarshal([]byte(queryCharacterExpected), &expected) require.NoError(t, err) err = json.Unmarshal(gqlResponse.Data, &result) @@ -193,6 +305,8 @@ func fragmentInQueryOnInterface(t *testing.T) { require.Equal(t, expected, result) cleanupStarwars(t, newStarship.ID, humanID, droidID) + deleteThingOne(t, thingOneId) + deleteThingTwo(t, thingTwoId) } func fragmentInQueryOnObject(t *testing.T) { diff --git a/graphql/e2e/common/mutation.go b/graphql/e2e/common/mutation.go index 41704e8c32b..7c6f1b7e942 100644 --- a/graphql/e2e/common/mutation.go +++ b/graphql/e2e/common/mutation.go @@ -2043,6 +2043,82 @@ func addDroid(t *testing.T) string { return result.AddDroid.Droid[0].ID } +func addThingOne(t *testing.T) string { + addDroidParams := &GraphQLParams{ + Query: `mutation addThingOne($input: AddThingOneInput!) { + addThingOne(input: [$input]) { + thingOne { + id + } + } + }`, + Variables: map[string]interface{}{"input": map[string]interface{}{ + "name": "Thing-1", + "color": "White", + "usedBy": "me", + }}, + } + + gqlResponse := addDroidParams.ExecuteAsPost(t, graphqlURL) + RequireNoGQLErrors(t, gqlResponse) + + var result struct { + AddThingOne struct { + ThingOne []struct { + ID string + } + } + } + err := json.Unmarshal([]byte(gqlResponse.Data), &result) + require.NoError(t, err) + + requireUID(t, result.AddThingOne.ThingOne[0].ID) + return result.AddThingOne.ThingOne[0].ID +} + +func addThingTwo(t *testing.T) string { + addDroidParams := &GraphQLParams{ + Query: `mutation addThingTwo($input: AddThingTwoInput!) { + addThingTwo(input: [$input]) { + thingTwo { + id + } + } + }`, + Variables: map[string]interface{}{"input": map[string]interface{}{ + "name": "Thing-2", + "color": "Black", + "owner": "someone", + }}, + } + + gqlResponse := addDroidParams.ExecuteAsPost(t, graphqlURL) + RequireNoGQLErrors(t, gqlResponse) + + var result struct { + AddThingTwo struct { + ThingTwo []struct { + ID string + } + } + } + err := json.Unmarshal([]byte(gqlResponse.Data), &result) + require.NoError(t, err) + + requireUID(t, result.AddThingTwo.ThingTwo[0].ID) + return result.AddThingTwo.ThingTwo[0].ID +} + +func deleteThingOne(t *testing.T, thingOneId string) { + thingOneFilter := map[string]interface{}{"id": []string{thingOneId}} + deleteGqlType(t, "ThingOne", thingOneFilter, 1, nil) +} + +func deleteThingTwo(t *testing.T, thingTwoId string) { + thingTwoFilter := map[string]interface{}{"id": []string{thingTwoId}} + deleteGqlType(t, "ThingTwo", thingTwoFilter, 1, nil) +} + func updateCharacter(t *testing.T, id string) { updateCharacterParams := &GraphQLParams{ Query: `mutation updateCharacter($character: UpdateCharacterInput!) { @@ -3095,6 +3171,7 @@ func ensureAliasInMutationPayload(t *testing.T) { Query: `mutation { addState(input: [{xcode: "S1", name: "State1"}]) { type: __typename + numUids count: numUids op: state { xcode @@ -3106,8 +3183,15 @@ func ensureAliasInMutationPayload(t *testing.T) { gqlResponse := addStateParams.ExecuteAsPost(t, graphqlURL) RequireNoGQLErrors(t, gqlResponse) - addStateExpected := `{"addState":{"type":"AddStatePayload","count":1,"op":[{"xcode":"S1"}]}}` - require.Equal(t, addStateExpected, string(gqlResponse.Data)) + addStateExpected := `{ + "addState": { + "type": "AddStatePayload", + "numUids": 1, + "count": 1, + "op": [{"xcode":"S1"}] + } + }` + require.JSONEq(t, addStateExpected, string(gqlResponse.Data)) filter := map[string]interface{}{"xcode": map[string]interface{}{"eq": "S1"}} deleteState(t, filter, 1, nil) @@ -3152,6 +3236,7 @@ func mutationsWithAlias(t *testing.T) { set: { name: "Testland Alias" } }) { updatedCountry: country { + name theName: name } } @@ -3165,7 +3250,7 @@ func mutationsWithAlias(t *testing.T) { "filter": map[string]interface{}{"id": []string{newCountry.ID}}}, } multiMutationExpected := `{ - "upd": { "updatedCountry": [{ "theName": "Testland Alias" }] }, + "upd": { "updatedCountry": [{ "name": "Testland Alias", "theName": "Testland Alias" }] }, "del" : { "message": "Deleted", "uids": 1 } }` diff --git a/graphql/e2e/common/query.go b/graphql/e2e/common/query.go index b8844d339ec..f8ed82ff82a 100644 --- a/graphql/e2e/common/query.go +++ b/graphql/e2e/common/query.go @@ -1602,6 +1602,7 @@ func queryWithAlias(t *testing.T) { Query: `query { post : queryPost (filter: {title : { anyofterms : "Introducing" }} ) { type : __typename + title postTitle : title postAuthor : author { theName : name @@ -1616,6 +1617,7 @@ func queryWithAlias(t *testing.T) { `{ "post": [ { "type": "Post", + "title": "Introducing GraphQL in Dgraph", "postTitle": "Introducing GraphQL in Dgraph", "postAuthor": { "theName": "Ann Author" }}]}`, string(gqlResponse.Data)) diff --git a/graphql/e2e/directives/dgraph_directives_test.go b/graphql/e2e/directives/dgraph_directives_test.go index 0bf76784143..a2237913066 100644 --- a/graphql/e2e/directives/dgraph_directives_test.go +++ b/graphql/e2e/directives/dgraph_directives_test.go @@ -222,6 +222,21 @@ func TestSchema_WithDgraphDirectives(t *testing.T) { "predicate": "Student.taughtBy", "type": "uid", "list": true + }, { + "predicate": "Thing.name", + "type": "string" + }, { + "predicate": "ThingOne.color", + "type": "string" + }, { + "predicate": "ThingOne.usedBy", + "type": "string" + }, { + "predicate": "ThingTwo.color", + "type": "string" + }, { + "predicate": "ThingTwo.owner", + "type": "string" }], "types": [{ "fields": [{ @@ -383,7 +398,30 @@ func TestSchema_WithDgraphDirectives(t *testing.T) { "name": "Student.taughtBy" }], "name": "Student" - }] + }, { + "fields": [{ + "name": "Thing.name" + }], + "name": "Thing" + }, { + "fields": [{ + "name": "Thing.name" + }, { + "name": "ThingOne.color" + }, { + "name": "ThingOne.usedBy" + }], + "name": "ThingOne" + }, { + "fields": [{ + "name": "Thing.name" + }, { + "name": "ThingTwo.color" + }, { + "name": "ThingTwo.owner" + }], + "name": "ThingTwo" + }] } ` diff --git a/graphql/e2e/directives/schema.graphql b/graphql/e2e/directives/schema.graphql index 05580be0307..223576a096b 100644 --- a/graphql/e2e/directives/schema.graphql +++ b/graphql/e2e/directives/schema.graphql @@ -143,3 +143,22 @@ type Message @withSubscription { content: String! @dgraph(pred: "post") author: String @dgraph(pred: "<职业>") } + +""" +This is used for fragment related testing +""" +interface Thing { + name: String # field to act as a common inherited field for both ThingOne and ThingTwo +} + +type ThingOne implements Thing { + id: ID! # ID field with same name as the ID field in ThingTwo + color: String # field with same name as a field in ThingTwo + usedBy: String # field with different name than any field in ThingTwo +} + +type ThingTwo implements Thing { + id: ID! + color: String + owner: String +} diff --git a/graphql/e2e/normal/normal_test.go b/graphql/e2e/normal/normal_test.go index 64047206333..31b8b5b9865 100644 --- a/graphql/e2e/normal/normal_test.go +++ b/graphql/e2e/normal/normal_test.go @@ -217,12 +217,25 @@ func TestSchema_Normal(t *testing.T) { "index": true, "tokenizer": ["exact"], "list": true - },{ - "predicate": "Person.name", - "type": "string" - - }], - + }, { + "predicate": "Person.name", + "type": "string" + }, { + "predicate": "Thing.name", + "type": "string" + }, { + "predicate": "ThingOne.color", + "type": "string" + }, { + "predicate": "ThingOne.usedBy", + "type": "string" + }, { + "predicate": "ThingTwo.color", + "type": "string" + }, { + "predicate": "ThingTwo.owner", + "type": "string" + }], "types": [{ "fields": [{ "name": "Author.name" @@ -370,12 +383,35 @@ func TestSchema_Normal(t *testing.T) { }, { "name": "Student.taughtBy" }], - "name": "Student" - },{ + "name": "Student" + }, { "fields": [{ "name": "Person.name" }], "name": "Person" + }, { + "fields": [{ + "name": "Thing.name" + }], + "name": "Thing" + }, { + "fields": [{ + "name": "Thing.name" + }, { + "name": "ThingOne.color" + }, { + "name": "ThingOne.usedBy" + }], + "name": "ThingOne" + }, { + "fields": [{ + "name": "Thing.name" + }, { + "name": "ThingTwo.color" + }, { + "name": "ThingTwo.owner" + }], + "name": "ThingTwo" }, { "fields": [{ "name": "dgraph.graphql.schema" diff --git a/graphql/e2e/normal/schema.graphql b/graphql/e2e/normal/schema.graphql index 3dcbda1d7e2..d7e93e3d3ad 100644 --- a/graphql/e2e/normal/schema.graphql +++ b/graphql/e2e/normal/schema.graphql @@ -142,4 +142,23 @@ type Student implements People { type Person @withSubscription{ id: ID! name: String! +} + +""" +This is used for fragment related testing +""" +interface Thing { + name: String # field to act as a common inherited field for both ThingOne and ThingTwo +} + +type ThingOne implements Thing { + id: ID! # ID field with same name as the ID field in ThingTwo + color: String # field with same name as a field in ThingTwo + usedBy: String # field with different name than any field in ThingTwo +} + +type ThingTwo implements Thing { + id: ID! + color: String + owner: String } \ No newline at end of file diff --git a/graphql/resolve/mutation.go b/graphql/resolve/mutation.go index 2d1621c2de5..25c591b98d6 100644 --- a/graphql/resolve/mutation.go +++ b/graphql/resolve/mutation.go @@ -233,7 +233,7 @@ func (mr *dgraphResolver) rewriteAndExecute(ctx context.Context, emptyResult := func(err error) *Resolved { return &Resolved{ - Data: map[string]interface{}{mutation.Name(): nil}, + Data: map[string]interface{}{mutation.DgraphAlias(): nil}, Field: mutation, Err: err, Extensions: ext, @@ -248,9 +248,9 @@ func (mr *dgraphResolver) rewriteAndExecute(ctx context.Context, if len(upserts) == 0 { return &Resolved{ Data: map[string]interface{}{ - mutation.Name(): map[string]interface{}{ - schema.NumUid: 0, - mutation.QueryField().Name(): nil, + mutation.DgraphAlias(): map[string]interface{}{ + schema.NumUid: 0, + mutation.QueryField().DgraphAlias(): nil, }}, Field: mutation, Err: nil, @@ -333,9 +333,9 @@ func (mr *dgraphResolver) rewriteAndExecute(ctx context.Context, if resolved.Data == nil && resolved.Err != nil { return &Resolved{ Data: map[string]interface{}{ - mutation.Name(): map[string]interface{}{ - schema.NumUid: numUids, - mutation.QueryField().Name(): nil, + mutation.DgraphAlias(): map[string]interface{}{ + schema.NumUid: numUids, + mutation.QueryField().DgraphAlias(): nil, }}, Field: mutation, Err: err, @@ -349,7 +349,7 @@ func (mr *dgraphResolver) rewriteAndExecute(ctx context.Context, dgRes := resolved.Data.(map[string]interface{}) dgRes[schema.NumUid] = numUids - resolved.Data = map[string]interface{}{mutation.Name(): dgRes} + resolved.Data = map[string]interface{}{mutation.DgraphAlias(): dgRes} resolved.Field = mutation resolved.Extensions = ext diff --git a/graphql/resolve/query.go b/graphql/resolve/query.go index 39b89cc3578..2f1b5d2d47d 100644 --- a/graphql/resolve/query.go +++ b/graphql/resolve/query.go @@ -104,7 +104,7 @@ func (qr *queryResolver) rewriteAndExecute(ctx context.Context, query schema.Que emptyResult := func(err error) *Resolved { return &Resolved{ - Data: map[string]interface{}{query.Name(): nil}, + Data: map[string]interface{}{query.DgraphAlias(): nil}, Field: query, Err: err, Extensions: ext, diff --git a/graphql/resolve/query_rewriter.go b/graphql/resolve/query_rewriter.go index 30a7516f081..d659636f4f2 100644 --- a/graphql/resolve/query_rewriter.go +++ b/graphql/resolve/query_rewriter.go @@ -677,7 +677,10 @@ func addSelectionSetFrom( // are required in the body template for other fields requested within the query. We must // fetch them from Dgraph. requiredFields := make(map[string]bool) + // addedFields is a map from field name to bool addedFields := make(map[string]bool) + // fieldAdded is a map from field's dgraph alias to bool + fieldAdded := make(map[string]bool) for _, f := range field.SelectionSet() { hasCustom, rf := f.HasCustomDirective() if hasCustom { @@ -695,9 +698,17 @@ func addSelectionSetFrom( continue } - child := &gql.GraphQuery{} + // skip if we have already added a query for this field in DQL. It helps make sure that if + // a field is being asked twice or more, each time with a new alias, then we only add it + // once in DQL query. + if _, ok := fieldAdded[f.DgraphAlias()]; ok { + continue + } + fieldAdded[f.DgraphAlias()] = true - child.Alias = f.Name() + child := &gql.GraphQuery{ + Alias: f.DgraphAlias(), + } if f.Type().Name() == schema.IDType { child.Attr = "uid" @@ -819,13 +830,14 @@ func addSelectionSetFrom( for _, fname := range rfset { if _, ok := addedFields[fname]; !ok { f := field.Type().Field(fname) - child := &gql.GraphQuery{} - child.Alias = f.Name() + child := &gql.GraphQuery{ + Alias: f.DgraphAlias(), + } if f.Type().Name() == schema.IDType { child.Attr = "uid" } else { - child.Attr = field.Type().DgraphPredicate(fname) + child.Attr = f.DgraphPredicate() } q.Children = append(q.Children, child) } diff --git a/graphql/resolve/query_test.yaml b/graphql/resolve/query_test.yaml index 1912632b74b..0d3727c799a 100644 --- a/graphql/resolve/query_test.yaml +++ b/graphql/resolve/query_test.yaml @@ -1355,3 +1355,94 @@ dgraph.uid : uid } } + +- name: "querying a field multiple times with different aliases adds it only once in rewriting" + gqlquery: |- + query { + queryThingOne { + i1: id + i2: id + name + n: name + n1: name + } + } + dgquery: |- + query { + queryThingOne(func: type(ThingOne)) { + id : uid + name : Thing.name + } + } + +- name: "query with fragments inside interface" + gqlquery: |- + query { + queryThing { + __typename + ... on ThingOne { + id + name + color + prop + usedBy + } + ... thingTwoFrag + } + } + fragment thingTwoFrag on ThingTwo { + id + name + color + prop + owner + } + dgquery: |- + query { + queryThing(func: type(Thing)) { + dgraph.type + id : uid + name : Thing.name + ThingOne.color : ThingOne.color + prop : prop + usedBy : ThingOne.usedBy + ThingTwo.color : ThingTwo.color + owner : ThingTwo.owner + } + } + +- name: "query only __typename in fragments inside interface" + gqlquery: |- + query { + queryThing { + ... on ThingOne { + __typename + } + ... on ThingTwo { + __typename + } + } + } + dgquery: |- + query { + queryThing(func: type(Thing)) { + dgraph.type + dgraph.uid : uid + } + } + +- name: "query only __typename in fragment inside object" + gqlquery: |- + query { + queryThingOne { + ... on ThingOne { + __typename + } + } + } + dgquery: |- + query { + queryThingOne(func: type(ThingOne)) { + dgraph.uid : uid + } + } diff --git a/graphql/resolve/resolver.go b/graphql/resolve/resolver.go index e40b820de2d..9dd5fce8f0c 100644 --- a/graphql/resolve/resolver.go +++ b/graphql/resolve/resolver.go @@ -690,7 +690,7 @@ func completeDgraphResult( schema.GQLWrapLocationf(err, field.Location(), "couldn't unmarshal Dgraph result")) } - switch val := valToComplete[field.Name()].(type) { + switch val := valToComplete[field.DgraphAlias()].(type) { case []interface{}: if field.Type().ListType() == nil { // Turn Dgraph list result to single object @@ -724,7 +724,7 @@ func completeDgraphResult( field.Name(), field.Type().String()).WithLocations(field.Location())) } - valToComplete[field.Name()] = internalVal + valToComplete[field.DgraphAlias()] = internalVal } case interface{}: // no need to error in this case, this can be returned for custom HTTP query/mutation @@ -738,7 +738,11 @@ func completeDgraphResult( // case } - err = resolveCustomFields(field.SelectionSet(), valToComplete[field.Name()]) + // TODO: correctly handle DgraphAlias for custom field resolution, at present it uses f.Name(), + // it should be using f.DgraphAlias() to get values from valToComplete. + // It works ATM because there hasn't been a scenario where there are two fields with same + // name in implementing types of an interface with @custom on some field in those types. + err = resolveCustomFields(field.SelectionSet(), valToComplete[field.DgraphAlias()]) if err != nil { errs = append(errs, schema.AsGQLErrors(err)...) } @@ -1288,7 +1292,7 @@ func completeObject( x.Check2(buf.WriteString(f.ResponseName())) x.Check2(buf.WriteString(`": `)) - val := res[f.Name()] + val := res[f.DgraphAlias()] if f.Name() == schema.Typename { // From GraphQL spec: // https://graphql.github.io/graphql-spec/June2018/#sec-Type-Name-Introspection diff --git a/graphql/resolve/schema.graphql b/graphql/resolve/schema.graphql index b2c3074adac..eef8ed55b4e 100644 --- a/graphql/resolve/schema.graphql +++ b/graphql/resolve/schema.graphql @@ -261,3 +261,24 @@ type Tweets { id: String! @id score: Int } + +""" +This is used for fragment related testing +""" +interface Thing { + name: String # field to act as a common inherited field for both ThingOne and ThingTwo +} + +type ThingOne implements Thing { + id: ID! # ID field with same name as the ID field in ThingTwo + color: String # field with same name as a field in ThingTwo + prop: String @dgraph(pred: "prop") # field with same name and same dgraph predicate as a field in ThingTwo + usedBy: String # field with different name than any field in ThingTwo +} + +type ThingTwo implements Thing { + id: ID! + color: String + prop: String @dgraph(pred: "prop") + owner: String +} diff --git a/graphql/schema/request.go b/graphql/schema/request.go index 808dbc40241..12e302b3815 100644 --- a/graphql/schema/request.go +++ b/graphql/schema/request.go @@ -78,11 +78,12 @@ func (s *schema) Operation(req *Request) (Operation, error) { } operation := &operation{op: op, - vars: vars, - query: req.Query, - header: req.Header, - doc: doc, - inSchema: s, + vars: vars, + query: req.Query, + header: req.Header, + doc: doc, + inSchema: s, + interfaceImplFragFields: map[*ast.Field]string{}, } // recursively expand fragments in operation as selection set fields @@ -150,23 +151,28 @@ func recursivelyExpandFragmentSelections(field *ast.Field, op *operation) { // this field always has to expand any fragment on its own type // "" tackles the case for an inline fragment which doesn't specify type condition satisfies := []string{typeName, ""} - var additionalTypes []*ast.Definition + var additionalTypes map[string]bool switch typeKind { case ast.Interface: // expand fragments on types which implement this interface - additionalTypes = op.inSchema.schema.PossibleTypes[typeName] + additionalTypes = getTypeNamesAsMap(op.inSchema.schema.PossibleTypes[typeName]) + // if there is any fragment in the selection set of this field, need to store a mapping from + // fields in that fragment to the fragment's type condition, to be used later in completion. + for _, f := range field.SelectionSet { + addSelectionToInterfaceImplFragFields(typeName, f, additionalTypes, op) + } case ast.Union: // expand fragments on types of which it is a union - additionalTypes = op.inSchema.schema.PossibleTypes[typeName] + additionalTypes = getTypeNamesAsMap(op.inSchema.schema.PossibleTypes[typeName]) case ast.Object: // expand fragments on interfaces which are implemented by this object - additionalTypes = op.inSchema.schema.Implements[typeName] + additionalTypes = getTypeNamesAsMap(op.inSchema.schema.Implements[typeName]) default: // return, as fragment can't be present on a field which is not Interface, Union or Object return } - for _, typ := range additionalTypes { - satisfies = append(satisfies, typ.Name) + for typName := range additionalTypes { + satisfies = append(satisfies, typName) } // collect all fields from any satisfying fragments into selectionSet @@ -211,3 +217,61 @@ func recursivelyExpandFragmentSelections(field *ast.Field, op *operation) { recursivelyExpandFragmentSelections(f.(*ast.Field), op) } } + +// getTypeNamesAsMap returns a map containing the typeName of all the typeDefs as keys and true +// as value +func getTypeNamesAsMap(typesDefs []*ast.Definition) map[string]bool { + if typesDefs == nil { + return nil + } + + typeNameMap := make(map[string]bool) + for _, typ := range typesDefs { + typeNameMap[typ.Name] = true + } + return typeNameMap +} + +func addSelectionToInterfaceImplFragFields(interfaceTypeName string, field ast.Selection, + interfaceImplMap map[string]bool, op *operation) { + switch frag := field.(type) { + case *ast.InlineFragment: + addFragFieldsToInterfaceImplFields(interfaceTypeName, frag.TypeCondition, + frag.SelectionSet, interfaceImplMap, op) + case *ast.FragmentSpread: + addFragFieldsToInterfaceImplFields(interfaceTypeName, frag.Definition.TypeCondition, + frag.Definition.SelectionSet, interfaceImplMap, op) + } +} + +func addFragFieldsToInterfaceImplFields(interfaceTypeName, typeCond string, selSet ast.SelectionSet, + interfaceImplMap map[string]bool, op *operation) { + if interfaceImplMap[typeCond] { + // if the type condition on fragment matches one of the types implementing the interface + // then we need to store mapping of the fields inside the fragment to the type condition. + for _, fragField := range selSet { + if f, ok := fragField.(*ast.Field); ok { + // we got a field on an implementation of a interface, so save the mapping of field + // to the implementing type name. This will later be used during completion to find + // out if the field should be reported back in the response or not. + op.interfaceImplFragFields[f] = typeCond + } else { + // we got a fragment inside fragment + // the type condition for this fragment will be same as its parent fragment + addSelectionToInterfaceImplFragFields(interfaceTypeName, fragField, + interfaceImplMap, op) + } + } + } else if typeCond == "" || typeCond == interfaceTypeName { + // otherwise, if the type condition is same as the type of the interface, + // then we still need to look if there are any more fragments inside this fragment + for _, fragField := range selSet { + if _, ok := fragField.(*ast.Field); !ok { + // we got a fragment inside fragment + // the type condition for this fragment may be different that its parent fragment + addSelectionToInterfaceImplFragFields(interfaceTypeName, fragField, + interfaceImplMap, op) + } + } + } +} diff --git a/graphql/schema/wrappers.go b/graphql/schema/wrappers.go index ba5bb6174f8..078e526145d 100644 --- a/graphql/schema/wrappers.go +++ b/graphql/schema/wrappers.go @@ -113,6 +113,8 @@ type Operation interface { type Field interface { Name() string Alias() string + // DgraphAlias is used as an alias in DQL while rewriting the GraphQL field + DgraphAlias() string ResponseName() string ArgValue(name string) interface{} IsArgListType(name string) bool @@ -182,7 +184,10 @@ type Type interface { // (which in turn must have a FieldDefinition of the right type in the schema.) type FieldDefinition interface { Name() string + DgraphAlias() string + DgraphPredicate() string Type() Type + ParentType() Type IsID() bool HasIDDirective() bool Inverse() FieldDefinition @@ -208,6 +213,9 @@ type schema struct { mutatedType map[string]*astType // Map from typename to ast.Definition typeNameAst map[string][]*ast.Definition + // map from field name to bool, indicating if a field name was repeated across different types + // implementing the same interface + repeatedFieldNames map[string]bool // customDirectives stores the mapping of typeName -> fieldName -> @custom definition. // It is read-only. // The outer map will contain typeName key only if one of the fields on that type has @custom. @@ -224,6 +232,10 @@ type operation struct { op *ast.OperationDefinition vars map[string]interface{} header http.Header + // interfaceImplFragFields stores a mapping from a field collected from a fragment inside an + // interface to its typeCondition. It is used during completion to find out if a field should + // be included in GraphQL response or not. + interfaceImplFragFields map[*ast.Field]string // The fields below are used by schema introspection queries. query string @@ -242,6 +254,7 @@ type field struct { type fieldDefinition struct { fieldDef *ast.FieldDefinition + parentType Type inSchema *schema dgraphPredicate map[string]map[string]string } @@ -536,6 +549,57 @@ func typeMappings(s *ast.Schema) map[string][]*ast.Definition { return typeNameAst } +func repeatedFieldMappings(s *ast.Schema, dgPreds map[string]map[string]string) map[string]bool { + repeatedFieldNames := make(map[string]bool) + + for _, typ := range s.Types { + if typ.Kind != ast.Interface { + continue + } + + interfaceFields := make(map[string]bool) + for _, field := range typ.Fields { + interfaceFields[field.Name] = true + } + + type fieldInfo struct { + dgPred string + repeated bool + } + + repeatedFieldsInTypesWithCommonAncestor := make(map[string]*fieldInfo) + for _, typ := range s.PossibleTypes[typ.Name] { + typPreds := dgPreds[typ.Name] + for _, field := range typ.Fields { + // ignore this field if it was inherited from the common interface or is of ID type. + // We ignore ID type fields too, because they map only to uid in dgraph and can't + // map to two different predicates. + if interfaceFields[field.Name] || field.Type.Name() == IDType { + continue + } + // if we find a field with same name from types implementing a common interface + // and its DgraphPredicate is different than what was previously encountered, then + // we mark it as repeated field, so that queries will rewrite it with correct alias + dgPred := typPreds[field.Name] + if fInfo, ok := repeatedFieldsInTypesWithCommonAncestor[field.Name]; ok && fInfo. + dgPred != dgPred { + repeatedFieldsInTypesWithCommonAncestor[field.Name].repeated = true + } else { + repeatedFieldsInTypesWithCommonAncestor[field.Name] = &fieldInfo{dgPred: dgPred} + } + } + } + + for fName, info := range repeatedFieldsInTypesWithCommonAncestor { + if info.repeated { + repeatedFieldNames[fName] = true + } + } + } + + return repeatedFieldNames +} + func customMappings(s *ast.Schema) map[string]map[string]*ast.Directive { customDirectives := make(map[string]map[string]*ast.Directive) @@ -577,11 +641,12 @@ func AsSchema(s *ast.Schema) (Schema, error) { dgraphPredicate := dgraphMapping(s) sch := &schema{ - schema: s, - dgraphPredicate: dgraphPredicate, - typeNameAst: typeMappings(s), - customDirectives: customMappings(s), - authRules: authRules, + schema: s, + dgraphPredicate: dgraphPredicate, + typeNameAst: typeMappings(s), + repeatedFieldNames: repeatedFieldMappings(s, dgraphPredicate), + customDirectives: customMappings(s), + authRules: authRules, } sch.mutatedType = mutatedTypeMapping(sch, dgraphPredicate) @@ -603,6 +668,16 @@ func (f *field) Alias() string { return f.field.Alias } +func (f *field) DgraphAlias() string { + // if this field is repeated, then it should be aliased using its dgraph predicate which will be + // unique across repeated fields + if f.op.inSchema.repeatedFieldNames[f.Name()] { + return f.DgraphPredicate() + } + // if not repeated, alias it using its name + return f.Name() +} + func (f *field) ResponseName() string { return responseName(f.field) } @@ -989,11 +1064,6 @@ func (f *field) TypeName(dgraphTypes []interface{}) string { } func (f *field) IncludeInterfaceField(dgraphTypes []interface{}) bool { - // As ID maps to uid in dgraph, so it is not stored as an edge, hence does not appear in - // f.op.inSchema.dgraphPredicate map. So, always include the queried field if it is of ID type. - if f.Type().Name() == IDType { - return true - } // Given a list of dgraph types, we query the schema and find the one which is an ast.Object // and not an Interface object. for _, typ := range dgraphTypes { @@ -1003,10 +1073,22 @@ func (f *field) IncludeInterfaceField(dgraphTypes []interface{}) bool { } for _, origTyp := range f.op.inSchema.typeNameAst[styp] { if origTyp.Kind == ast.Object { - // If the field doesn't exist in the map corresponding to the object type, then we - // don't need to include it. - _, ok := f.op.inSchema.dgraphPredicate[origTyp.Name][f.Name()] - return ok || f.Name() == Typename + // If the field is from an interface implemented by this object, + // and was fetched not because of a fragment on this object, + // but because of a fragment on some other object, then we don't need to include it. + fragType, ok := f.op.interfaceImplFragFields[f.field] + if ok && fragType != origTyp.Name { + return false + } + + // We include the field in response only if any of the following conditions hold: + // * Field is __typename + // * The field is of ID type: As ID maps to uid in dgraph, so it is not stored as an + // edge, hence does not appear in f.op.inSchema.dgraphPredicate map. So, always + // include the queried field if it is of ID type. + // * If the field exists in the map corresponding to the object type + _, ok = f.op.inSchema.dgraphPredicate[origTyp.Name][f.Name()] + return ok || f.Type().Name() == IDType || f.Name() == Typename } } @@ -1043,6 +1125,10 @@ func (q *query) Alias() string { return (*field)(q).Alias() } +func (q *query) DgraphAlias() string { + return q.Name() +} + func (q *query) SetArgTo(arg string, val interface{}) { (*field)(q).SetArgTo(arg, val) } @@ -1156,6 +1242,10 @@ func (m *mutation) Alias() string { return (*field)(m).Alias() } +func (m *mutation) DgraphAlias() string { + return m.Name() +} + func (m *mutation) SetArgTo(arg string, val interface{}) { (*field)(m).SetArgTo(arg, val) } @@ -1308,6 +1398,7 @@ func (t *astType) Field(name string) FieldDefinition { fieldDef: t.inSchema.schema.Types[t.Name()].Fields.ForName(name), inSchema: t.inSchema, dgraphPredicate: t.dgraphPredicate, + parentType: t, } } @@ -1320,6 +1411,7 @@ func (t *astType) Fields() []FieldDefinition { fieldDef: fld, inSchema: t.inSchema, dgraphPredicate: t.dgraphPredicate, + parentType: t, }) } @@ -1330,6 +1422,17 @@ func (fd *fieldDefinition) Name() string { return fd.fieldDef.Name } +func (fd *fieldDefinition) DgraphAlias() string { + if fd.inSchema.repeatedFieldNames[fd.Name()] { + return fd.DgraphPredicate() + } + return fd.Name() +} + +func (fd *fieldDefinition) DgraphPredicate() string { + return fd.dgraphPredicate[fd.parentType.Name()][fd.Name()] +} + func (fd *fieldDefinition) IsID() bool { return isID(fd.fieldDef) } @@ -1358,6 +1461,10 @@ func (fd *fieldDefinition) Type() Type { } } +func (fd *fieldDefinition) ParentType() Type { + return fd.parentType +} + func (fd *fieldDefinition) Inverse() FieldDefinition { invDirective := fd.fieldDef.Directives.ForName(inverseDirective) @@ -1370,8 +1477,9 @@ func (fd *fieldDefinition) Inverse() FieldDefinition { return nil // really not possible } + typeWrapper := fd.Type() // typ must exist if the schema passed GQL validation - typ := fd.inSchema.schema.Types[fd.Type().Name()] + typ := fd.inSchema.schema.Types[typeWrapper.Name()] // fld must exist if the schema passed our validation fld := typ.Fields.ForName(invFieldArg.Value.Raw) @@ -1379,7 +1487,9 @@ func (fd *fieldDefinition) Inverse() FieldDefinition { return &fieldDefinition{ fieldDef: fld, inSchema: fd.inSchema, - dgraphPredicate: fd.dgraphPredicate} + dgraphPredicate: fd.dgraphPredicate, + parentType: typeWrapper, + } } // ForwardEdge gets the field definition for a forward edge if this field is a reverse edge @@ -1402,8 +1512,9 @@ func (fd *fieldDefinition) ForwardEdge() FieldDefinition { } fedge := strings.Trim(name, "<~>") + typeWrapper := fd.Type() // typ must exist if the schema passed GQL validation - typ := fd.inSchema.schema.Types[fd.Type().Name()] + typ := fd.inSchema.schema.Types[typeWrapper.Name()] var fld *ast.FieldDefinition // Have to range through all the fields and find the correct forward edge. This would be @@ -1426,7 +1537,9 @@ func (fd *fieldDefinition) ForwardEdge() FieldDefinition { return &fieldDefinition{ fieldDef: fld, inSchema: fd.inSchema, - dgraphPredicate: fd.dgraphPredicate} + dgraphPredicate: fd.dgraphPredicate, + parentType: typeWrapper, + } } func (t *astType) Name() string { @@ -1502,8 +1615,9 @@ func (t *astType) IDField() FieldDefinition { for _, fd := range def.Fields { if isID(fd) { return &fieldDefinition{ - fieldDef: fd, - inSchema: t.inSchema, + fieldDef: fd, + inSchema: t.inSchema, + parentType: t, } } } @@ -1523,8 +1637,9 @@ func (t *astType) PasswordField() FieldDefinition { } return &fieldDefinition{ - fieldDef: fd, - inSchema: t.inSchema, + fieldDef: fd, + inSchema: t.inSchema, + parentType: t, } } @@ -1537,8 +1652,9 @@ func (t *astType) XIDField() FieldDefinition { for _, fd := range def.Fields { if hasIDDirective(fd) { return &fieldDefinition{ - fieldDef: fd, - inSchema: t.inSchema, + fieldDef: fd, + inSchema: t.inSchema, + parentType: t, } } }