Resolve empty union objects within Sum schema derivation & cleanups (#…

…2121)

* Resolve empty union objects within Sum schema derivation

* Cleanup

* Make Scala 2.12 and MiMa happy

* Remove unused imports

* Use `.contains(true)` instead of `.exists(identity)`

core/src/main/scala-2/caliban/schema/SchemaDerivation.scala

@@ -4,7 +4,6 @@ import caliban.Value._
 import caliban.introspection.adt._
 import caliban.parsing.adt.Directive
 import caliban.schema.Annotations._
-import caliban.schema.Step.{ PureStep => _ }
 import caliban.schema.Types._
 import magnolia1._
 
@@ -117,14 +116,22 @@ trait CommonSchemaDerivation[R] {
   }
 
   def split[T](ctx: SealedTrait[Typeclass, T]): Typeclass[T] = new Typeclass[T] {
+
+    private lazy val subtypes =
+      ctx.subtypes
+        .map(s => s.typeclass.toType_() -> s.annotations)
+        .toList
+        .sortBy { case (tpe, _) =>
+          tpe.name.getOrElse("")
+        }
+
+    private lazy val emptyUnionObjectIdxs =
+      subtypes.map { case (t, _) => SchemaUtils.isEmptyUnionObject(t) }.toArray[Boolean]
+
+    private var containsEmptyUnionObjects = false
+
     override def toType(isInput: Boolean, isSubscription: Boolean): __Type = {
-      val subtypes    =
-        ctx.subtypes
-          .map(s => s.typeclass.toType_() -> s.annotations)
-          .toList
-          .sortBy { case (tpe, _) =>
-            tpe.name.getOrElse("")
-          }
+
       val isEnum      = subtypes.forall {
         case (t, _) if t.allFields.isEmpty && t.allInputFields.isEmpty => true
         case _                                                         => false
@@ -154,15 +161,16 @@ trait CommonSchemaDerivation[R] {
           Some(ctx.typeName.full),
           Some(getDirectives(ctx.annotations))
         )
-      else if (!isInterface)
+      else if (!isInterface) {
+        containsEmptyUnionObjects = emptyUnionObjectIdxs.contains(true)
         makeUnion(
           Some(getName(ctx)),
           getDescription(ctx),
-          subtypes.map { case (t, _) => fixEmptyUnionObject(t) },
+          subtypes.map { case (t, _) => SchemaUtils.fixEmptyUnionObject(t) },
           Some(ctx.typeName.full),
           Some(getDirectives(ctx.annotations))
         )
-      else {
+      } else {
         val excl         = ctx.annotations.collectFirst { case i: GQLInterface => i.excludedFields.toSet }.getOrElse(Set.empty)
         val impl         = subtypes.map(_._1.copy(interfaces = () => Some(List(toType(isInput, isSubscription)))))
         val commonFields = () =>
@@ -192,30 +200,13 @@ trait CommonSchemaDerivation[R] {
       }
     }
 
-    // see https://github.com/graphql/graphql-spec/issues/568
-    private def fixEmptyUnionObject(t: __Type): __Type =
-      t.fields(__DeprecatedArgs(Some(true))) match {
-        case Some(Nil) =>
-          t.copy(
-            fields = (_: __DeprecatedArgs) =>
-              Some(
-                List(
-                  __Field(
-                    "_",
-                    Some(
-                      "Fake field because GraphQL does not support empty objects. Do not query, use __typename instead."
-                    ),
-                    _ => Nil,
-                    () => makeScalar("Boolean")
-                  )
-                )
-              )
-          )
-        case _         => t
-      }
-
     override def resolve(value: T): Step[R] =
-      ctx.split(value)(subType => subType.typeclass.resolve(subType.cast(value)))
+      ctx.split(value) { subType =>
+        val step = subType.typeclass.resolve(subType.cast(value))
+        if (containsEmptyUnionObjects && emptyUnionObjectIdxs(subType.index))
+          SchemaUtils.resolveEmptyUnionStep(step)
+        else step
+      }
   }
 
   private def getDirectives(annotations: Seq[Any]): List[Directive] =

core/src/main/scala-3/caliban/schema/DerivationUtils.scala

@@ -18,28 +18,6 @@ private object DerivationUtils {
     if (name.endsWith("Input")) name
     else s"${name}Input"
 
-  // see https://github.com/graphql/graphql-spec/issues/568
-  def fixEmptyUnionObject(t: __Type): __Type =
-    t.fields(__DeprecatedArgs(Some(true))) match {
-      case Some(Nil) =>
-        t.copy(
-          fields = (_: __DeprecatedArgs) =>
-            Some(
-              List(
-                __Field(
-                  "_",
-                  Some(
-                    "Fake field because GraphQL does not support empty objects. Do not query, use __typename instead."
-                  ),
-                  _ => Nil,
-                  () => makeScalar("Boolean")
-                )
-              )
-            )
-        )
-      case _         => t
-    }
-
   def getName(annotations: Seq[Any], info: TypeInfo): String =
     annotations.collectFirst { case GQLName(name) => name }.getOrElse {
       info.typeParams match {

core/src/main/scala-3/caliban/schema/SumSchema.scala

@@ -16,33 +16,44 @@ final private class SumSchema[R, A](
     extends Schema[R, A] {
 
   @threadUnsafe
-  private lazy val (subTypes, schemas) = {
+  private lazy val (subTypes, schemas, emptyUnionObjectIdxs) = {
     val (m, s) = _members
-    (m.sortBy(_._1), s.toVector)
+    (
+      m.sortBy(_._1),
+      s.toVector,
+      s.map(s0 => SchemaUtils.isEmptyUnionObject(s0.toType_())).toArray[Boolean]
+    )
   }
 
-  @threadUnsafe
-  private lazy val isEnum = subTypes.forall((_, t, _) => t.allFields.isEmpty && t.allInputFields.isEmpty)
-
-  private val isInterface = annotations.exists(_.isInstanceOf[GQLInterface])
-  private val isUnion     = annotations.contains(GQLUnion())
+  private var containsEmptyUnionObjects = false
 
   def toType(isInput: Boolean, isSubscription: Boolean): __Type = {
-    val _ = schemas
+    val _                         = schemas
+    val isInterface               = annotations.exists(_.isInstanceOf[GQLInterface])
+    val isUnion                   = annotations.contains(GQLUnion())
+    @threadUnsafe lazy val isEnum = subTypes.forall((_, t, _) => t.allFields.isEmpty && t.allInputFields.isEmpty)
+
     if (!isInterface && !isUnion && subTypes.nonEmpty && isEnum) mkEnum(annotations, info, subTypes)
-    else if (!isInterface)
+    else if (!isInterface) {
+      containsEmptyUnionObjects = emptyUnionObjectIdxs.contains(true)
       makeUnion(
         Some(getName(annotations, info)),
         getDescription(annotations),
-        subTypes.map(_._2).distinctBy(_.name).map(fixEmptyUnionObject),
+        subTypes.map(_._2).distinctBy(_.name).map(SchemaUtils.fixEmptyUnionObject),
         Some(info.full),
         Some(getDirectives(annotations))
       )
-    else {
+    } else {
       val impl = subTypes.map(_._2.copy(interfaces = () => Some(List(toType(isInput, isSubscription)))))
       mkInterface(annotations, info, impl)
     }
   }
 
-  def resolve(value: A): Step[R] = schemas(ordinal(value)).resolve(value)
+  def resolve(value: A): Step[R] = {
+    val idx  = ordinal(value)
+    val step = schemas(idx).resolve(value)
+    if (containsEmptyUnionObjects && emptyUnionObjectIdxs(idx))
+      SchemaUtils.resolveEmptyUnionStep(step)
+    else step
+  }
 }

core/src/main/scala/caliban/execution/Executor.scala

@@ -70,9 +70,11 @@ object Executor {
 
       def reduceObjectStep(objectName: String, getFieldStep: String => Step[R]): ReducedStep[R] = {
         def reduceField(f: Field): (String, ReducedStep[R], FieldInfo) = {
-          val field =
-            if (f.name == "__typename") PureStep(StringValue(objectName))
-            else reduceStep(getFieldStep(f.name), f, f.arguments, PathValue.Key(f.aliasedName) :: path)
+          val field = {
+            val fname = f.name
+            if (fname == "__typename") PureStep(StringValue(objectName))
+            else reduceStep(getFieldStep(fname), f, f.arguments, PathValue.Key(f.aliasedName) :: path)
+          }
           (f.aliasedName, field, fieldInfo(f, path, f.directives))
         }
 
@@ -133,43 +135,34 @@ object Executor {
           )
         )
 
+      def reduceStream(stream: ZStream[R, Throwable, Step[R]]) =
+        if (isSubscription) {
+          ReducedStep.StreamStep(
+            stream
+              .mapErrorCause(effectfulExecutionError(path, Some(currentField.locationInfo), _))
+              .map(reduceStep(_, currentField, arguments, path))
+          )
+        } else {
+          reduceStep(
+            QueryStep(ZQuery.fromZIO(stream.runCollect.map(chunk => ListStep(chunk.toList)))),
+            currentField,
+            arguments,
+            path
+          )
+        }
+
       def handleError(step: => Step[R]): Step[R] =
         try step
         catch { case NonFatal(e) => Step.fail(e) }
 
       step match {
-        case s @ PureStep(EnumValue(v))     =>
-          // special case of an hybrid union containing case objects, those should return an object instead of a string
-          currentField.fields.view.filter(_._condition.forall(_.contains(v))).collectFirst {
-            case f if f.name == "__typename" =>
-              ObjectValue(List(f.aliasedName -> StringValue(v)))
-            case f if f.name == "_"          =>
-              NullValue
-          } match {
-            case Some(v) => PureStep(v)
-            case None    => s
-          }
         case s: PureStep                    => s
         case QueryStep(inner)               => reduceQuery(inner)
         case ObjectStep(objectName, fields) => reduceObjectStep(objectName, fields)
         case FunctionStep(step)             => reduceStep(handleError(step(arguments)), currentField, Map.empty, path)
         case MetadataFunctionStep(step)     => reduceStep(handleError(step(currentField)), currentField, arguments, path)
         case ListStep(steps)                => reduceListStep(steps)
-        case StreamStep(stream)             =>
-          if (isSubscription) {
-            ReducedStep.StreamStep(
-              stream
-                .mapErrorCause(effectfulExecutionError(path, Some(currentField.locationInfo), _))
-                .map(reduceStep(_, currentField, arguments, path))
-            )
-          } else {
-            reduceStep(
-              QueryStep(ZQuery.fromZIO(stream.runCollect.map(chunk => ListStep(chunk.toList)))),
-              currentField,
-              arguments,
-              path
-            )
-          }
+        case StreamStep(stream)             => reduceStream(stream)
       }
     }
 

core/src/main/scala/caliban/execution/Field.scala

@@ -51,15 +51,15 @@ case class Field(
       val _             = set.add(fields.head.aliasedName)
 
       var remaining = fields.tail
-      while (remaining ne Nil) {
-        val f      = remaining.head
-        val result = set.add(f.aliasedName) && f._condition == headCondition
-        if (!result) return false
+      var result    = true
+      while ((remaining ne Nil) && result) {
+        val f = remaining.head
+        result = set.add(f.aliasedName) && f._condition == headCondition
         remaining = remaining.tail
       }
-      true
+      result
     }
-    if (fields.isEmpty) true else inner
+    fields.isEmpty || fields.tail.isEmpty || inner
   }
 
   def combine(other: Field): Field =

core/src/main/scala/caliban/introspection/adt/__DeprecatedArgs.scala

@@ -1,3 +1,13 @@
 package caliban.introspection.adt
 
+import scala.runtime.AbstractFunction1
+
 case class __DeprecatedArgs(includeDeprecated: Option[Boolean] = None)
+
+// NOTE: This object extends AbstractFunction1 to maintain binary compatibility for Scala 2.13.
+// TODO: Remove inheritance in the next major version
+object __DeprecatedArgs extends AbstractFunction1[Option[Boolean], __DeprecatedArgs] {
+  val include: __DeprecatedArgs = __DeprecatedArgs(Some(true))
+
+  def apply(v: Option[Boolean] = None): __DeprecatedArgs = new __DeprecatedArgs(v)
+}

core/src/main/scala/caliban/introspection/adt/__Directive.scala

@@ -19,5 +19,5 @@ case class __Directive(
     )
 
   lazy val allArgs: List[__InputValue] =
-    args(__DeprecatedArgs(Some(true)))
+    args(__DeprecatedArgs.include)
 }

core/src/main/scala/caliban/introspection/adt/__Field.scala

@@ -32,7 +32,7 @@ case class __Field(
     InputValueDefinition(description, name, _type.toType(), None, directives.getOrElse(Nil))
 
   lazy val allArgs: List[__InputValue] =
-    args(__DeprecatedArgs(Some(true)))
+    args(__DeprecatedArgs.include)
 
   private[caliban] lazy val _type: __Type = `type`()
 }

core/src/main/scala/caliban/introspection/adt/__Type.scala

@@ -102,7 +102,7 @@ case class __Type(
             description,
             name.getOrElse(""),
             directives.getOrElse(Nil),
-            enumValues(__DeprecatedArgs(Some(true))).getOrElse(Nil).map(_.toEnumValueDefinition)
+            enumValues(__DeprecatedArgs.include).getOrElse(Nil).map(_.toEnumValueDefinition)
           )
         )
       case __TypeKind.INPUT_OBJECT =>
@@ -127,13 +127,13 @@ case class __Type(
   lazy val nonNull: __Type = __Type(__TypeKind.NON_NULL, ofType = Some(self))
 
   lazy val allFields: List[__Field] =
-    fields(__DeprecatedArgs(Some(true))).getOrElse(Nil)
+    fields(__DeprecatedArgs.include).getOrElse(Nil)
 
   lazy val allInputFields: List[__InputValue] =
-    inputFields(__DeprecatedArgs(Some(true))).getOrElse(Nil)
+    inputFields(__DeprecatedArgs.include).getOrElse(Nil)
 
   lazy val allEnumValues: List[__EnumValue] =
-    enumValues(__DeprecatedArgs(Some(true))).getOrElse(Nil)
+    enumValues(__DeprecatedArgs.include).getOrElse(Nil)
 
   private[caliban] lazy val allFieldsMap: collection.Map[String, __Field] = {
     val map = collection.mutable.HashMap.empty[String, __Field]

core/src/main/scala/caliban/schema/SchemaUtils.scala

@@ -0,0 +1,44 @@
+package caliban.schema
+
+import caliban.ResponseValue.ObjectValue
+import caliban.Value.{ EnumValue, NullValue, StringValue }
+import caliban.introspection.adt.{ __DeprecatedArgs, __Field, __Type }
+import caliban.schema.Step.MetadataFunctionStep
+
+private[schema] object SchemaUtils {
+  private val fakeField =
+    Some(
+      List(
+        __Field(
+          "_",
+          Some(
+            "Fake field because GraphQL does not support empty objects. Do not query, use __typename instead."
+          ),
+          _ => Nil,
+          () => Types.makeScalar("Boolean")
+        )
+      )
+    )
+
+  def isEmptyUnionObject(t: __Type): Boolean =
+    t.fields(__DeprecatedArgs.include).contains(Nil)
+
+  // see https://github.com/graphql/graphql-spec/issues/568
+  def fixEmptyUnionObject(t: __Type): __Type =
+    if (isEmptyUnionObject(t)) t.copy(fields = (_: __DeprecatedArgs) => fakeField)
+    else t
+
+  def resolveEmptyUnionStep[R](step: Step[R]): Step[R] = step match {
+    case s @ PureStep(EnumValue(v)) =>
+      MetadataFunctionStep[R] { field =>
+        field.fields.view
+          .filter(_._condition.forall(_.contains(v)))
+          .collectFirst {
+            case f if f.name == "__typename" => ObjectValue(List(f.aliasedName -> StringValue(v)))
+            case f if f.name == "_"          => NullValue
+          }
+          .fold(s)(PureStep(_))
+      }
+    case _                          => step
+  }
+}

core/src/main/scala/caliban/validation/ValueValidator.scala

@@ -121,7 +121,7 @@ object ValueValidator {
 
   def validateEnum(value: String, inputType: __Type, errorContext: => String): EReader[Any, ValidationError, Unit] = {
     val possible = inputType
-      .enumValues(__DeprecatedArgs(Some(true)))
+      .enumValues(__DeprecatedArgs.include)
       .getOrElse(List.empty)
       .map(_.name)
     val exists   = possible.contains(value)

core/src/test/scala/caliban/execution/ExecutionSpec.scala

@@ -692,7 +692,7 @@ object ExecutionSpec extends ZIOSpecDefault {
           assertTrue(response.data.toString == """{"test":{"union":{"__typename":"UnionChild","field":"f"}}}""")
         }
       },
-      test("rename on a union child and parent") {
+      test("rename on a union child and parent (typename)") {
         sealed trait Union
         object Union {
           case class Child(field: String) extends Union