Fix GraphQL to use unique type names (#3194)

The names of GraphQL types needs to be globally unique. We therefore
name struct types as NAME_HASH (where hash is the first 6 chars of
the noms hash).

Also, make sure that we always map the noms type to the same GraphQL
type, even if the boxedIfScalar is true.

Fixes #3161
This commit is contained in:
Erik Arvidsson
2017-02-15 17:24:38 -08:00
committed by GitHub
parent 83b657fe62
commit cb520e76fa
3 changed files with 46 additions and 15 deletions

View File

@@ -47,7 +47,8 @@ func constructQueryType(rootValue types.Value, tm *typeMap) *graphql.Object {
}})
}
// Query takes |rootValue|, builds a GraphQL scheme from rootValue.Type() and executes |query| against it, encoding the result to |w|.
// Query takes |rootValue|, builds a GraphQL scheme from rootValue.Type() and
// executes |query| against it, encoding the result to |w|.
func Query(rootValue types.Value, query string, vr types.ValueReader, w io.Writer) error {
tm := newTypeMap()

View File

@@ -6,6 +6,7 @@ package ngql
import (
"bytes"
"fmt"
"testing"
"github.com/attic-labs/noms/go/chunks"
@@ -240,7 +241,12 @@ func (suite *QueryGraphQLSuite) TestListOfUnionOfStructs() {
}),
)
suite.assertQueryResult(list, "{root{elements{... on FooStruct{a b} ... on BarStruct{b} ... on BazStruct{c}}}}", `{"data":{"root":{"elements":[{"a":28,"b":"baz"},{"b":"bar"},{"c":true}]}}}`)
suite.assertQueryResult(list,
fmt.Sprintf("{root{elements{... on %s{a b} ... on %s{b} ... on %s{c}}}}",
getTypeName(list.Get(0).Type()),
getTypeName(list.Get(1).Type()),
getTypeName(list.Get(2).Type())),
`{"data":{"root":{"elements":[{"a":28,"b":"baz"},{"b":"bar"},{"c":true}]}}}`)
}
func (suite *QueryGraphQLSuite) TestListOfUnionOfStructsConflictingFieldTypes() {
@@ -256,7 +262,12 @@ func (suite *QueryGraphQLSuite) TestListOfUnionOfStructsConflictingFieldTypes()
}),
)
suite.assertQueryResult(list, "{root{elements{... on FooStruct{a} ... on BarStruct{b: a} ... on BazStruct{c: a}}}}", `{"data":{"root":{"elements":[{"a":28},{"b":"bar"},{"c":true}]}}}`)
suite.assertQueryResult(list,
fmt.Sprintf("{root{elements{... on %s{a} ... on %s{b: a} ... on %s{c: a}}}}",
getTypeName(list.Get(0).Type()),
getTypeName(list.Get(1).Type()),
getTypeName(list.Get(2).Type())),
`{"data":{"root":{"elements":[{"a":28},{"b":"bar"},{"c":true}]}}}`)
}
func (suite *QueryGraphQLSuite) TestListOfUnionOfScalars() {

View File

@@ -27,18 +27,21 @@ func newTypeMap() *typeMap {
return &typeMap{}
}
// In terms of resolving a graph of data, there are three types of value: scalars, lists and maps.
// During resolution, we are converting some noms value to a graphql value. A getFieldFn will
// be invoked for a matching noms type. Its job is to retrieve the sub-value from the noms type
// which is mapped to a graphql map as a fieldname.
// In terms of resolving a graph of data, there are three types of value:
// scalars, lists and maps. During resolution, we are converting some noms
// value to a graphql value. A getFieldFn will be invoked for a matching noms
// type. Its job is to retrieve the sub-value from the noms type which is
// mapped to a graphql map as a fieldname.
type getFieldFn func(v interface{}, fieldName string, ctx context.Context) types.Value
// When a field name is resolved, it may take key:value arguments. A getSubvaluesFn handles
// returning one or more *noms* values whose presence is indicated by the provided arguments.
// When a field name is resolved, it may take key:value arguments. A
// getSubvaluesFn handles returning one or more *noms* values whose presence is
// indicated by the provided arguments.
type getSubvaluesFn func(v types.Value, args map[string]interface{}) (interface{}, error)
// GraphQL requires all memberTypes in a Union to be Structs, so when a noms union contains a
// scalar, we represent it in that context as a "boxed" value. E.g.
// GraphQL requires all memberTypes in a Union to be Structs, so when a noms
// union contains a scalar, we represent it in that context as a "boxed" value.
// E.g.
// Boolean! =>
// type BooleanValue {
// scalarValue: Boolean!
@@ -56,9 +59,18 @@ func scalarToValue(nomsType *types.Type, scalarType graphql.Type, tm *typeMap) g
}})
}
func isScalar(nomsType *types.Type) bool {
switch nomsType {
case types.BoolType, types.NumberType, types.StringType:
return true
default:
return false
}
}
// Note: Always returns a graphql.NonNull() as the outer type.
func nomsTypeToGraphQLType(nomsType *types.Type, boxedIfScalar bool, tm *typeMap) graphql.Type {
key := typeMapKey{nomsType.Hash(), boxedIfScalar}
key := typeMapKey{nomsType.Hash(), boxedIfScalar && isScalar(nomsType)}
gqlType, ok := (*tm)[key]
if ok {
return gqlType
@@ -151,19 +163,23 @@ func unionToGQLUnion(nomsType *types.Type, tm *typeMap) *graphql.Union {
ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object {
tm := p.Context.Value(tmKey).(*typeMap)
var nomsType *types.Type
isScalar := false
if v, ok := p.Value.(types.Value); ok {
nomsType = v.Type()
} else {
switch p.Value.(type) {
case float64:
nomsType = types.NumberType
isScalar = true
case string:
nomsType = types.StringType
isScalar = true
case bool:
nomsType = types.BoolType
isScalar = true
}
}
key := typeMapKey{nomsType.Hash(), true}
key := typeMapKey{nomsType.Hash(), isScalar}
memberType := (*tm)[key]
// Member types cannot be non-null and must be struct (graphl.Object)
return memberType.(*graphql.NonNull).OfType.(*graphql.Object)
@@ -382,11 +398,14 @@ func getTypeName(nomsType *types.Type) string {
return fmt.Sprintf("%sSet", getTypeName(nomsValueType))
case types.StructKind:
return fmt.Sprintf("%sStruct", nomsType.Desc.(types.StructDesc).Name)
// GraphQL Name cannot start with a number.
// GraphQL type names must be globally unique.
return fmt.Sprintf("%s_%s", nomsType.Desc.(types.StructDesc).Name, nomsType.Hash().String()[:6])
case types.TypeKind:
// GraphQL Name cannot start with a number.
// TODO: https://github.com/attic-labs/noms/issues/3155
return fmt.Sprintf("%sType", nomsType.Hash().String())
return fmt.Sprintf("Type_%s", nomsType.Hash().String()[:6])
case types.UnionKind:
unionMemberTypes := nomsType.Desc.(types.CompoundDesc).ElemTypes