From f4eb239b654857961b383f6a014f147b9be9d77a Mon Sep 17 00:00:00 2001 From: kyri-petrou <67301607+kyri-petrou@users.noreply.github.com> Date: Thu, 9 Nov 2023 15:44:26 +1100 Subject: [PATCH] Attempt at fixing Sum type schema derivation for good (#1994) * Attempt at fixing Scala 3 sum type derivation for good * fmt * Unpack Sum types in ArgBuilder derivation as well * inline if conditions from macros * Add test for nested interfaces --- .../caliban/schema/ArgBuilderDerivation.scala | 29 ++-- .../caliban/schema/SchemaDerivation.scala | 44 +++--- .../schema/SchemaDerivationIssuesSpec.scala | 129 ++++++++++++++++++ .../scala/caliban/schema/SchemaSpec.scala | 66 +++++++++ 4 files changed, 230 insertions(+), 38 deletions(-) diff --git a/core/src/main/scala-3/caliban/schema/ArgBuilderDerivation.scala b/core/src/main/scala-3/caliban/schema/ArgBuilderDerivation.scala index dbf01bc095..96693bfb49 100644 --- a/core/src/main/scala-3/caliban/schema/ArgBuilderDerivation.scala +++ b/core/src/main/scala-3/caliban/schema/ArgBuilderDerivation.scala @@ -18,17 +18,22 @@ trait CommonArgBuilderDerivation { inline erasedValue[(Label, A)] match { case (_: EmptyTuple, _) => values.reverse case (_: (name *: names), _: (t *: ts)) => - recurseSum[P, names, ts]( - ( - constValue[name].toString, - MagnoliaMacro.anns[t], { - if (Macros.isEnumField[P, t]) - if (!Macros.implicitExists[ArgBuilder[t]]) derived[t] - else summonInline[ArgBuilder[t]] - else summonInline[ArgBuilder[t]] - }.asInstanceOf[ArgBuilder[Any]] - ) :: values - ) + recurseSum[P, names, ts] { + inline summonInline[Mirror.Of[t]] match { + case m: Mirror.SumOf[t] => + recurseSum[t, m.MirroredElemLabels, m.MirroredElemTypes](values) + case _ => + ( + constValue[name].toString, + MagnoliaMacro.anns[t], { + inline if (Macros.isEnumField[P, t]) + inline if (!Macros.implicitExists[ArgBuilder[t]]) derived[t] + else summonInline[ArgBuilder[t]] + else summonInline[ArgBuilder[t]] + }.asInstanceOf[ArgBuilder[Any]] + ) :: values + } + } } inline def recurseProduct[P, Label, A <: Tuple]( @@ -66,7 +71,7 @@ trait CommonArgBuilderDerivation { ) = new ArgBuilder[A] { private lazy val subTypes = _subTypes private lazy val traitLabel = _traitLabel - private val emptyInput = InputValue.ObjectValue(Map()) + private val emptyInput = InputValue.ObjectValue(Map.empty) def build(input: InputValue): Either[ExecutionError, A] = input.match { diff --git a/core/src/main/scala-3/caliban/schema/SchemaDerivation.scala b/core/src/main/scala-3/caliban/schema/SchemaDerivation.scala index 47b14939a1..083618a1a1 100644 --- a/core/src/main/scala-3/caliban/schema/SchemaDerivation.scala +++ b/core/src/main/scala-3/caliban/schema/SchemaDerivation.scala @@ -44,16 +44,22 @@ trait CommonSchemaDerivation { case (_: EmptyTuple, _) => values.reverse case (_: (name *: names), _: (t *: ts)) => recurseSum[R, P, names, ts] { - ( - constValue[name].toString, - MagnoliaMacro.anns[t], { - if (Macros.isEnumField[P, t]) - if (!Macros.implicitExists[Schema[R, t]]) derived[R, t] - else summonInline[Schema[R, t]] - else summonInline[Schema[R, t]] - }.asInstanceOf[Schema[R, Any]] - ) :: values + inline summonInline[Mirror.Of[t]] match { + case m: Mirror.SumOf[t] => + recurseSum[R, t, m.MirroredElemLabels, m.MirroredElemTypes](values) + case _ => + ( + constValue[name].toString, + MagnoliaMacro.anns[t], { + inline if (Macros.isEnumField[P, t]) + inline if (!Macros.implicitExists[Schema[R, t]]) derived[R, t] + else summonInline[Schema[R, t]] + else summonInline[Schema[R, t]] + }.asInstanceOf[Schema[R, Any]] + ) :: values + } } + } inline def recurseProduct[R, P, Label, A <: Tuple]( @@ -104,7 +110,7 @@ trait CommonSchemaDerivation { }.sortBy(_._1).toList private lazy val isEnum = subTypes.forall { (_, t, _) => - unpackLeafTypes(t).foldLeft(true)((acc, t) => acc && t.allFields.isEmpty && t.allInputFields.isEmpty) + t.allFields.isEmpty && t.allInputFields.isEmpty } private lazy val isInterface = annotations.exists { @@ -123,14 +129,12 @@ trait CommonSchemaDerivation { makeUnion( Some(getName(annotations, info)), getDescription(annotations), - subTypes.flatMap((_, t, _) => unpackLeafTypes(t)).map(fixEmptyUnionObject), + subTypes.map(_._2).distinctBy(_.name).map(fixEmptyUnionObject), Some(info.full), Some(getDirectives(annotations)) ) else { - val impl = subTypes - .flatMap(v => unpackUnion(v._2)) - .map(_.copy(interfaces = () => Some(List(toType(isInput, isSubscription))))) + val impl = subTypes.map(_._2.copy(interfaces = () => Some(List(toType(isInput, isSubscription))))) mkInterface(annotations, info, impl) } @@ -193,18 +197,6 @@ trait CommonSchemaDerivation { } } - private def unpackLeafTypes(t: __Type): List[__Type] = - t.possibleTypes match { - case None | Some(Nil) => List(t) - case Some(tpes) => tpes.flatMap(unpackLeafTypes) - } - - private def unpackUnion(t: __Type): List[__Type] = - t.kind match { - case __TypeKind.UNION => t.possibleTypes.fold(List(t))(_.flatMap(unpackUnion)) - case _ => List(t) - } - // see https://github.com/graphql/graphql-spec/issues/568 private def fixEmptyUnionObject(t: __Type): __Type = t.fields(__DeprecatedArgs(Some(true))) match { diff --git a/core/src/test/scala/caliban/schema/SchemaDerivationIssuesSpec.scala b/core/src/test/scala/caliban/schema/SchemaDerivationIssuesSpec.scala index e5c7643534..369270d8b2 100644 --- a/core/src/test/scala/caliban/schema/SchemaDerivationIssuesSpec.scala +++ b/core/src/test/scala/caliban/schema/SchemaDerivationIssuesSpec.scala @@ -140,6 +140,54 @@ object SchemaDerivationIssuesSpec extends ZIOSpecDefault { | op: PrefixOperator! |}""".stripMargin ) + }, + test("i992") { + import i1992._ + + assertTrue( + schema == + """schema { + | query: Queries + |} + | + |union Parent = Child1 | Child2 | Child3 + | + |type Child1 { + | bool: Boolean! + |} + | + |type Child2 { + | i: Int! + |} + | + |type Child3 { + | str: String! + |} + | + |type Queries { + | p: Parent! + |}""".stripMargin + ) + }, + test("i1993") { + import i1993._ + + assertTrue( + schema == + """schema { + | query: Queries + |} + | + |enum Enum1 { + | Item111 + | Item112 + | Item121 + |} + | + |type Queries { + | e: Enum1! + |}""".stripMargin + ) } ) } @@ -295,3 +343,84 @@ object i1990 { caliban.graphQL(RootResolver(queries)) }.render } + +object i1992 { + sealed trait Parent + object Parent { + implicit val schema: Schema[Any, Parent] = Schema.gen + + sealed trait OtherParent extends Parent + object OtherParent { + implicit val schema: Schema[Any, OtherParent] = Schema.gen + } + + case class Child2(i: Int) extends Parent with OtherParent + object Child2 { + implicit val schema: Schema[Any, Child2] = Schema.gen + } + + case class Child1(bool: Boolean) extends Parent with OtherParent + object Child1 { + implicit val schema: Schema[Any, Child1] = Schema.gen + } + + case class Child3(str: String) extends Parent + object Child3 { + implicit val schema: Schema[Any, Child3] = Schema.gen + } + } + + case class Queries(p: Parent) + + object Queries { + implicit val schema: Schema[Any, Queries] = Schema.gen + } + + val schema = { + val queries = Queries(Parent.Child2(1)) + caliban.graphQL(RootResolver(queries)) + }.render +} + +object i1993 { + sealed trait Enum1 + + object Enum1 { + implicit val schema: Schema[Any, Enum1] = Schema.gen + + sealed trait Enum11 extends Enum1 + + object Enum11 { + implicit val schema: Schema[Any, Enum11] = Schema.gen + + case object Item111 extends Enum11 { + implicit val schema: Schema[Any, Item111.type] = Schema.gen + } + + case object Item112 extends Enum11 { + implicit val schema: Schema[Any, Item112.type] = Schema.gen + } + } + + sealed trait Enum12 extends Enum1 + + object Enum12 { + implicit val schema: Schema[Any, Enum12] = Schema.gen + + case object Item121 extends Enum12 { + implicit val schema: Schema[Any, Item121.type] = Schema.gen + } + } + } + + case class Queries(e: Enum1) + + object Queries { + implicit val schema: Schema[Any, Queries] = Schema.gen + } + + val schema = { + val queries = Queries(Enum1.Enum12.Item121) + caliban.graphQL(RootResolver(queries)) + }.render +} diff --git a/core/src/test/scala/caliban/schema/SchemaSpec.scala b/core/src/test/scala/caliban/schema/SchemaSpec.scala index abe73abd91..86e5b82119 100644 --- a/core/src/test/scala/caliban/schema/SchemaSpec.scala +++ b/core/src/test/scala/caliban/schema/SchemaSpec.scala @@ -317,6 +317,56 @@ object SchemaSpec extends ZIOSpecDefault { | myEnum: Foo! |}""".stripMargin ) + }, + test("nested interfaces") { + case class Query(top: NestedInterface, mid1: NestedInterface.Mid1, mid2: NestedInterface.Mid2) + + val v = NestedInterface.FooB("b", "c", "d") + val schema = graphQL(RootResolver(Query(v, v, v))).render + + assertTrue( + schema == """schema { + | query: Query + |} + | + |interface Mid1 { + | b: String! + | c: String! + |} + | + |interface Mid2 { + | b: String! + | d: String! + |} + | + |interface NestedInterface { + | b: String! + |} + | + |type FooA implements NestedInterface & Mid1 { + | a: String! + | b: String! + | c: String! + |} + | + |type FooB implements NestedInterface & Mid1 & Mid2 { + | b: String! + | c: String! + | d: String! + |} + | + |type FooC implements NestedInterface & Mid2 { + | b: String! + | d: String! + | e: String! + |} + | + |type Query { + | top: NestedInterface! + | mid1: Mid1! + | mid2: Mid2! + |}""".stripMargin + ) } ) @@ -364,4 +414,20 @@ object SchemaSpec extends ZIOSpecDefault { def introspect[Q](implicit schema: Schema[Any, Q]): __Type = schema.toType_() def introspectSubscription[Q](implicit schema: Schema[Any, Q]): __Type = schema.toType_(isSubscription = true) + + @GQLInterface + sealed trait NestedInterface + + object NestedInterface { + + @GQLInterface + sealed trait Mid1 extends NestedInterface + + @GQLInterface + sealed trait Mid2 extends NestedInterface + + case class FooA(a: String, b: String, c: String) extends Mid1 + case class FooB(b: String, c: String, d: String) extends Mid1 with Mid2 + case class FooC(b: String, d: String, e: String) extends Mid2 + } }