Optimize fragment validation (#2039)

* Optimize object field execution & validation of fragments

* Optimize Validator

build.sbt

@@ -669,22 +669,13 @@ lazy val commonSettings = Def.settings(
   })
 )
 
-lazy val enforceMimaCompatibility = true // Enable / disable failing CI on binary incompatibilities
+lazy val enforceMimaCompatibility = false // Enable / disable failing CI on binary incompatibilities
 
 lazy val enableMimaSettingsJVM =
   Def.settings(
     mimaFailOnProblem     := enforceMimaCompatibility,
     mimaPreviousArtifacts := previousStableVersion.value.map(organization.value %% moduleName.value % _).toSet,
-    mimaBinaryIssueFilters ++= Seq(
-      ProblemFilters.exclude[IncompatibleMethTypeProblem]("caliban.schema.Step#ObjectStep*"),
-      ProblemFilters.exclude[IncompatibleResultTypeProblem]("caliban.schema.Step#ObjectStep*"),
-      ProblemFilters.exclude[DirectMissingMethodProblem]("caliban.schema.Annotations*"),
-      ProblemFilters.exclude[MissingTypesProblem]("caliban.schema.Annotations*"),
-      ProblemFilters.exclude[IncompatibleResultTypeProblem]("caliban.schema.Annotations*"),
-      ProblemFilters.exclude[IncompatibleMethTypeProblem]("caliban.Quick*"),
-      ProblemFilters.exclude[DirectMissingMethodProblem]("caliban.Quick*"),
-      ProblemFilters.exclude[DirectMissingMethodProblem]("caliban.quick.package*")
-    )
+    mimaBinaryIssueFilters ++= Seq()
   )
 
 lazy val enableMimaSettingsJS =

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

@@ -64,20 +64,26 @@ object Executor {
     ): ReducedStep[R] = {
 
       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, Left(f.aliasedName) :: path)
+          (f.aliasedName, field, fieldInfo(f, path, f.directives))
+        }
+
         val filteredFields    = mergeFields(currentField, objectName)
-        val (deferred, eager) = filteredFields.partitionMap {
-          case f @ Field("__typename", _, _, _, _, _, _, directives, _, _, _)   =>
-            Right((f.aliasedName, PureStep(StringValue(objectName)), fieldInfo(f, path, directives)))
-          case f @ Field(name, _, _, _, _, _, args, directives, _, _, fragment) =>
-            val field = reduceStep(getFieldStep(name), f, args, Left(f.aliasedName) :: path)
-            val entry = (f.aliasedName, field, fieldInfo(f, path, directives))
-
-            fragment match {
-              // The defer spec provides some latitude on how we handle responses. Since it is more performant to return
-              // pure fields rather than spin up the defer machinery we return pure fields immediately to the caller.
-              case Some(IsDeferred(label)) if isDeferredEnabled && !field.isPure => Left((label, entry))
-              case _                                                             => Right(entry)
+        val (deferred, eager) = {
+          if (isDeferredEnabled) {
+            filteredFields.partitionMap { f =>
+              val entry = reduceField(f)
+              f.fragment match {
+                // The defer spec provides some latitude on how we handle responses. Since it is more performant to return
+                // pure fields rather than spin up the defer machinery we return pure fields immediately to the caller.
+                case Some(IsDeferred(label)) if !entry._2.isPure => Left((label, entry))
+                case _                                           => Right(entry)
+              }
             }
+          } else (Nil, filteredFields.map(reduceField))
         }
 
         val eagerReduced = reduceObject(eager, wrapPureValues)
@@ -303,7 +309,7 @@ object Executor {
 
     for {
       cache    <- Cache.empty
-      reduced   = reduceStep(plan, request.field, Map(), Nil)
+      reduced   = reduceStep(plan, request.field, Map.empty, Nil)
       response <- runQuery(reduced, cache)
     } yield response
   }

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

@@ -121,7 +121,13 @@ object Field {
     val memoizedFragments      = new mutable.HashMap[String, (List[Field], Option[String])]()
     val variableDefinitionsMap = variableDefinitions.map(v => v.name -> v).toMap
 
-    def loop(selectionSet: List[Selection], fieldType: __Type, fragment: Option[Fragment]): List[Field] = {
+    def loop(
+      selectionSet: List[Selection],
+      fieldType: __Type,
+      fragment: Option[Fragment],
+      targets: Option[Set[String]],
+      condition: Option[Set[String]]
+    ): List[Field] = {
       val map = new java.util.LinkedHashMap[(String, Option[String]), Field]()
 
       def addField(f: Field, condition: Option[String]): Unit =
@@ -141,22 +147,22 @@ object Field {
             val t = selected.fold(Types.string)(_._type) // default only case where it's not found is __typename
 
             val fields =
-              if (selectionSet.nonEmpty) loop(selectionSet, t, None)
+              if (selectionSet.nonEmpty) loop(selectionSet, t, None, None, None)
               else Nil // Fragments apply on to the direct children of the fragment spread
 
             addField(
-              Field(
+              new Field(
                 name,
                 t,
                 Some(innerType),
                 alias,
                 fields,
-                None,
-                resolveVariables(arguments, variableDefinitionsMap, variableValues),
-                resolvedDirectives,
-                None,
-                () => sourceMapper.getLocation(index),
-                fragment
+                targets = targets,
+                arguments = resolveVariables(arguments, variableDefinitionsMap, variableValues),
+                directives = resolvedDirectives,
+                _condition = condition,
+                _locationInfo = () => sourceMapper.getLocation(index),
+                fragment = fragment
               ),
               None
             )
@@ -165,17 +171,16 @@ object Field {
           val (fields, condition) = memoizedFragments.getOrElseUpdate(
             name, {
               val resolvedDirectives = directives.map(resolveDirectiveVariables(variableValues, variableDefinitionsMap))
-
-              val _fields = if (checkDirectives(resolvedDirectives)) {
+              val _fields            = if (checkDirectives(resolvedDirectives)) {
                 fragments.get(name).map { f =>
-                  val t          = rootType.types.getOrElse(f.typeCondition.name, fieldType)
-                  val _targets   = Some(Set(f.typeCondition.name))
-                  val _condition = subtypeNames(f.typeCondition.name, rootType)
-
-                  loop(f.selectionSet, t, Some(Fragment(Some(name), resolvedDirectives))).map { field =>
-                    if (field._condition.isDefined) field
-                    else field.copy(targets = _targets, _condition = _condition)
-                  } -> Some(f.typeCondition.name)
+                  val t = rootType.types.getOrElse(f.typeCondition.name, fieldType)
+                  loop(
+                    f.selectionSet,
+                    t,
+                    fragment = Some(Fragment(Some(name), resolvedDirectives)),
+                    targets = Some(Set(f.typeCondition.name)),
+                    condition = subtypeNames(f.typeCondition.name, rootType)
+                  ) -> Some(f.typeCondition.name)
                 }
               } else None
               _fields.getOrElse(Nil -> None)
@@ -184,31 +189,25 @@ object Field {
           fields.foreach(addField(_, condition))
         case InlineFragment(typeCondition, directives, selectionSet)    =>
           val resolvedDirectives = directives.map(resolveDirectiveVariables(variableValues, variableDefinitionsMap))
-
           if (checkDirectives(resolvedDirectives)) {
-            val t      = innerType.possibleTypes
+            val t        = innerType.possibleTypes
               .flatMap(_.find(_.name.exists(typeCondition.map(_.name).contains)))
               .orElse(typeCondition.flatMap(typeName => rootType.types.get(typeName.name)))
               .getOrElse(fieldType)
-            val fields = loop(selectionSet, t, Some(Fragment(None, resolvedDirectives)))
-            typeCondition match {
-              case None           => fields.map(addField(_, None))
-              case Some(typeName) =>
-                val _targets   = Some(Set(typeName.name))
-                val _condition = subtypeNames(typeName.name, rootType)
-                fields.foreach { field =>
-                  val _field =
-                    if (field._condition.isDefined) field
-                    else field.copy(targets = _targets, _condition = _condition)
-                  addField(_field, Some(typeName.name))
-                }
-            }
+            val typeName = typeCondition.map(_.name)
+            loop(
+              selectionSet,
+              t,
+              fragment = Some(Fragment(None, resolvedDirectives)),
+              targets = typeName.map(Set(_)),
+              condition = typeName.flatMap(subtypeNames(_, rootType))
+            ).foreach(addField(_, typeName))
           }
       }
       map.values().asScala.toList
     }
 
-    val fields = loop(selectionSet, fieldType, None)
+    val fields = loop(selectionSet, fieldType, None, None, None)
     Field("", fieldType, None, fields = fields, directives = directives)
   }
 

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

@@ -14,6 +14,8 @@ case class __Field(
   deprecationReason: Option[String] = None,
   @GQLExcluded directives: Option[List[Directive]] = None
 ) {
+  final override lazy val hashCode: Int = super.hashCode()
+
   def toFieldDefinition: FieldDefinition = {
     val allDirectives = (if (isDeprecated)
                            List(

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

@@ -22,6 +22,8 @@ case class __Type(
   @GQLExcluded directives: Option[List[Directive]] = None,
   @GQLExcluded origin: Option[String] = None
 ) { self =>
+  final override lazy val hashCode: Int = super.hashCode()
+
   def |+|(that: __Type): __Type = __Type(
     kind,
     (name ++ that.name).reduceOption((_, b) => b),
@@ -130,8 +132,18 @@ case class __Type(
   lazy val allEnumValues: List[__EnumValue] =
     enumValues(__DeprecatedArgs(Some(true))).getOrElse(Nil)
 
-  private[caliban] lazy val allFieldsMap: Map[String, __Field] =
-    allFields.map(f => f.name -> f).toMap
+  private[caliban] lazy val allFieldsMap: collection.Map[String, __Field] = {
+    val map = collection.mutable.HashMap.empty[String, __Field]
+    allFields.foreach(f => map.update(f.name, f))
+    map
+  }
 
   lazy val innerType: __Type = Types.innerType(this)
+
+  private[caliban] lazy val possibleTypeNames: Set[String] =
+    kind match {
+      case __TypeKind.OBJECT                       => name.fold(Set.empty[String])(Set(_))
+      case __TypeKind.INTERFACE | __TypeKind.UNION => possibleTypes.fold(Set.empty[String])(_.flatMap(_.name).toSet)
+      case _                                       => Set.empty
+    }
 }

core/src/main/scala/caliban/parsing/adt/Selection.scala

@@ -3,7 +3,9 @@ package caliban.parsing.adt
 import caliban.InputValue
 import caliban.parsing.adt.Type.NamedType
 
-sealed trait Selection
+sealed trait Selection {
+  final override lazy val hashCode: Int = super.hashCode()
+}
 
 object Selection {
 

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

@@ -4,12 +4,12 @@ import caliban.CalibanError.ValidationError
 import caliban.introspection.adt._
 import caliban.parsing.adt.Selection
 import caliban.validation.Utils._
-import caliban.validation.Utils.syntax._
 import zio.Chunk
-import zio.prelude.EReader
+import zio.prelude._
 import zio.prelude.fx.ZPure
 
 import scala.collection.mutable
+import scala.util.hashing.MurmurHash3
 
 object FragmentValidator {
   def findConflictsWithinSelectionSet(
@@ -24,13 +24,13 @@ object FragmentValidator {
     val groupsCache  = mutable.Map.empty[Int, Chunk[Set[SelectedField]]]
 
     def sameResponseShapeByName(set: Iterable[Selection]): Chunk[String] = {
-      val keyHash = set.hashCode()
+      val keyHash = MurmurHash3.unorderedHash(set)
       shapeCache.get(keyHash) match {
         case Some(value) => value
         case None        =>
           val fields = FieldMap(context, parentType, set)
           val res    = Chunk.fromIterable(fields.flatMap { case (name, values) =>
-            cross(values).flatMap { case (f1, f2) =>
+            cross(values, includeIdentity = true).flatMap { case (f1, f2) =>
               if (doTypesConflict(f1.fieldDef._type, f2.fieldDef._type)) {
                 Chunk(
                   s"$name has conflicting types: ${f1.parentType.name.getOrElse("")}.${f1.fieldDef.name} and ${f2.parentType.name
@@ -46,7 +46,7 @@ object FragmentValidator {
     }
 
     def sameForCommonParentsByName(set: Iterable[Selection]): Chunk[String] = {
-      val keyHash = set.hashCode()
+      val keyHash = MurmurHash3.unorderedHash(set)
       parentsCache.get(keyHash) match {
         case Some(value) => value
         case None        =>
@@ -81,14 +81,14 @@ object FragmentValidator {
         false
 
     def requireSameNameAndArguments(fields: Set[SelectedField]) =
-      cross(fields).flatMap { case (f1, f2) =>
+      cross(fields, includeIdentity = false).flatMap { case (f1, f2) =>
         if (f1.fieldDef.name != f2.fieldDef.name) {
-          List(
+          Some(
             s"${f1.parentType.name.getOrElse("")}.${f1.fieldDef.name} and ${f2.parentType.name.getOrElse("")}.${f2.fieldDef.name} are different fields."
           )
         } else if (f1.selection.arguments != f2.selection.arguments)
-          List(s"${f1.fieldDef.name} and ${f2.fieldDef.name} have different arguments")
-        else List()
+          Some(s"${f1.fieldDef.name} and ${f2.fieldDef.name} have different arguments")
+        else None
       }
 
     def groupByCommonParents(fields: Set[SelectedField]): Chunk[Set[SelectedField]] = {
@@ -109,13 +109,7 @@ object FragmentValidator {
                   _,
                   _
                 ) if isConcrete(field.parentType) =>
-              concreteGroups.get(name) match {
-                case Some(v) => v += field
-                case None    =>
-                  val sb = Set.newBuilder ++= abstractGroup
-                  sb += field
-                  concreteGroups.update(name, sb)
-              }
+              concreteGroups.getOrElseUpdate(name, Set.newBuilder ++= abstractGroup) += field
             case _ => ()
           }
 

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

@@ -5,6 +5,8 @@ import caliban.introspection.adt.__TypeKind._
 import caliban.parsing.adt.Type.NamedType
 import zio.Chunk
 
+import scala.collection.compat._
+
 object Utils {
   def isObjectType(t: __Type): Boolean =
     t.kind match {
@@ -56,22 +58,25 @@ object Utils {
   def getType(t: NamedType, context: Context): Option[__Type] =
     context.rootType.types.get(t.name)
 
-  def cross[A](a: Iterable[A]): Chunk[(A, A)] =
-    Chunk.fromIterable(for (xs <- a; ys <- a) yield (xs, ys))
-
-  def cross[A](a: Iterable[A], b: Iterable[A]): Chunk[(A, A)] =
-    Chunk.fromIterable(for (xs <- a; ys <- b) yield (xs, ys))
-
-  object syntax {
-    implicit class OptionSyntax[+A](val self: Option[A]) extends AnyVal {
-      def zip[B](that: Option[B]): Option[(A, B)] =
-        self.flatMap(a => that.map(b => (a, b)))
+  /**
+   * For an iterable, produce a Chunk containing tuples of all possible unique combinations, optionally including the identity
+   */
+  def cross[A](a: Iterable[A], includeIdentity: Boolean): Chunk[(A, A)] = {
+    val ca       = Chunk.fromIterable(a)
+    val size     = ca.size
+    val cb       = Chunk.newBuilder[(A, A)]
+    var i1, i2   = 0
+    val modifier = if (includeIdentity) 0 else 1
+    while (i1 < size - modifier) {
+      i2 = i1 + modifier
+      val l = ca(i1)
+      while (i2 < size) {
+        cb += ((l, ca(i2)))
+        i2 += 1
+      }
+      i1 += 1
     }
-
-    implicit class Tuple2Syntax[+A, +B](val self: (Option[A], Option[B])) extends AnyVal {
-      def mapN[C](f: (A, B) => C): Option[C] =
-        self._1.flatMap(a => self._2.map(b => f(a, b)))
-    }
-
+    cb.result()
   }
+
 }