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 TypeComparators #989

Merged
merged 2 commits into from
Mar 31, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
30 changes: 25 additions & 5 deletions modules/core/src/main/scala/sangria/schema/Schema.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1529,10 +1529,33 @@ case class Schema[Ctx, Val](

directImplementations.map { case (name, directImpls) =>
name -> directImpls.flatMap(findDescendants).groupBy(_.name).map(_._2.head).toVector
}
} ++ unionTypes.values.map(ut => ut.name -> ut.types.toVector)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I don't like this

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you have a better idea?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I have changed back and use union only in possibleTypes

}

lazy val implementations: Map[String, Vector[ObjectType[_, _]]] =
def isPossibleImplementation(baseTypeName: String, tpe: ObjectLikeType[_, _]): Boolean =
tpe.name == baseTypeName || allImplementations
.get(baseTypeName)
.exists(_.exists(_.name == tpe.name))

@deprecated("Use possibleTypes instead", "4.0.0")
lazy val implementations: Map[String, Vector[ObjectType[_, _]]] = possibleTypes

/** This contains the map of all the concrete types by supertype.
*
* The supertype can be either a union or interface.
*
* According to the spec, even if an interface can implement another interface, this must only
* contains concrete types
*
* @see
* https://spec.graphql.org/June2018/#sec-Union
* @see
* https://spec.graphql.org/June2018/#sec-Interface
*
* @return
* Map of subtype by supertype name
*/
lazy val possibleTypes: Map[String, Vector[ObjectType[_, _]]] =
allImplementations
.map { case (k, xs) =>
(
Expand All @@ -1545,9 +1568,6 @@ case class Schema[Ctx, Val](
v.nonEmpty
}

lazy val possibleTypes: Map[String, Vector[ObjectType[_, _]]] =
implementations ++ unionTypes.values.map(ut => ut.name -> ut.types.toVector)

def isPossibleType(baseTypeName: String, tpe: ObjectType[_, _]): Boolean =
possibleTypes.get(baseTypeName).exists(_.exists(_.name == tpe.name))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ object TypeComparators {
case (sub, OptionInputType(ofType2)) => isSubType(schema, sub, ofType2)
case (ListType(ofType1), ListType(ofType2)) => isSubType(schema, ofType1, ofType2)
case (ListInputType(ofType1), ListInputType(ofType2)) => isSubType(schema, ofType1, ofType2)
case (t1: ObjectType[_, _], t2: AbstractType) => schema.isPossibleType(t2.name, t1)
case (t1: ObjectLikeType[_, _], t2: AbstractType) =>
schema.isPossibleImplementation(t2.name, t1)
case (t1: Named, t2: Named) => t1.name == t2.name
case _ => false
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -971,6 +971,84 @@ class SchemaConstraintsSpec extends AnyWordSpec with Matchers {
}
""")

"rejects an Interface with a differently typed Interface field" in invalidSchema(
graphql"""
type Query {
test: AnotherObject
}

type A { foo: String }
type B { foo: String }

interface AnotherInterface {
field: A
}

type AnotherObject implements AnotherInterface {
field: B
}
""",
"AnotherInterface.field expects type 'A', but AnotherObject.field provides type 'B'." -> Seq(
Pos(14, 11),
Pos(10, 11))
)

"accepts an Interface with a subtyped Interface field (interface)" in validSchema(graphql"""
type Query {
test: Foo
}

interface Node {
id: ID!
}

interface Edge {
cursor: String!
node: Node!
}

interface Foo implements Node {
id: ID!
}

type ConcreteFoo implements Foo & Node {
id: ID!
}

type FooEdge implements Edge {
cursor: String!
node: Foo!
}

type Test {
fooEdge: FooEdge
}
""")

"accepts an Interface with a subtyped Interface field (union)" in validSchema(graphql"""
type Query {
fooBar: FooBar
}

type SomeObject {
field: String
}

union SomeUnionType = SomeObject

interface Foo {
foo: SomeUnionType
}

interface Bar implements Foo {
foo: SomeUnionType
}

type FooBar implements Foo & Bar {
test: String
}
""")

"rejects an Interface missing an Interface argument" in invalidSchema(
graphql"""
type Query {
Expand All @@ -995,6 +1073,29 @@ class SchemaConstraintsSpec extends AnyWordSpec with Matchers {
)
)

"rejects an Interface with an incorrectly typed Interface argument" in invalidSchema(
graphql"""
type Query {
fooBar: FooBar
}

interface Foo {
foo(input: String): String
}

interface Bar implements Foo {
foo(input: Int): String
}

type FooBar implements Foo & Bar {
test: Int
}
""",
"Foo.foo(input) expects type 'String', but Bar.foo(input) provides type 'Int'." -> Seq(
Pos(7, 15),
Pos(11, 15))
)

"rejects an Interface with an incorrectly typed Interface field" in invalidSchema(
graphql"""
type Query {
Expand Down