Skip to content

Commit

Permalink
Ensure fields are unique after merge (#2077)
Browse files Browse the repository at this point in the history
* Ensure fields are unique after merge

* Fix scala 2.12 compiling

* Check for uniqueness within the Executor
  • Loading branch information
kyri-petrou authored Jan 15, 2024
1 parent f056174 commit 0c99202
Show file tree
Hide file tree
Showing 3 changed files with 153 additions and 20 deletions.
21 changes: 5 additions & 16 deletions core/src/main/scala/caliban/execution/Executor.scala
Original file line number Diff line number Diff line change
Expand Up @@ -349,16 +349,6 @@ object Executor {
ZIO.succeed(GraphQLResponse(NullValue, List(error)))

private[caliban] def mergeFields(field: Field, typeName: String): List[Field] = {
def haveSameCondition(head: Field, tail: List[Field]): Boolean = {
val condition = head._condition
var remaining = tail
while (remaining ne Nil) {
if (remaining.head._condition != condition) return false
remaining = remaining.tail
}
true
}

def matchesTypename(f: Field): Boolean =
f._condition.isEmpty || f._condition.get.contains(typeName)

Expand All @@ -380,12 +370,11 @@ object Executor {
map.values().asScala.toList
}

field.fields match {
// Shortcut if all the fields have the same condition, which means we don't need to merge as that's been handled in Field.apply
case h :: t if haveSameCondition(h, t) => if (matchesTypename(h)) field.fields else Nil
case Nil => Nil
case fields => mergeFields(fields)
}
val fields = field.fields
if (field.allFieldsUniqueNameAndCondition) {
if (fields.isEmpty || !matchesTypename(fields.head)) Nil
else fields
} else mergeFields(fields)
}

private def fieldInfo(field: Field, path: List[PathValue], fieldDirectives: List[Directive]): FieldInfo =
Expand Down
18 changes: 18 additions & 0 deletions core/src/main/scala/caliban/execution/Field.scala
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,24 @@ case class Field(

private[caliban] val aliasedName: String = alias.getOrElse(name)

private[caliban] lazy val allFieldsUniqueNameAndCondition: Boolean = {
def inner: Boolean = {
val set = new mutable.HashSet[String]
val headCondition = fields.head._condition
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
remaining = remaining.tail
}
true
}
if (fields.isEmpty) true else inner
}

def combine(other: Field): Field =
self.copy(
fields = self.fields ::: other.fields,
Expand Down
134 changes: 130 additions & 4 deletions core/src/test/scala/caliban/schema/SchemaDerivationIssuesSpec.scala
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
package caliban.schema

import caliban.Macros._
import caliban.schema.Annotations.{ GQLDescription, GQLInterface, GQLName, GQLUnion }
import zio.Chunk
import zio.test.ZIOSpecDefault
import zio.test._
import caliban.{ graphQL, RootResolver }
import caliban.Macros._
import zio.test.{ ZIOSpecDefault, _ }
import zio.{ Chunk, ZIO }

object SchemaDerivationIssuesSpec extends ZIOSpecDefault {
def spec = suite("SchemaDerivationIssuesSpec")(
Expand Down Expand Up @@ -211,6 +210,64 @@ object SchemaDerivationIssuesSpec extends ZIOSpecDefault {
res <- i.execute(gqldoc("{ e1 { b } e2 { b } e3 { b } }"))
data = res.data.toString
} yield assertTrue(data == """{"e1":{"b":"b"},"e2":{"b":"b"},"e3":{"b":"b"}}""")
},
test("i2076") {
import i2076._
val queryFragments =
gqldoc("""
query {
widget {
__typename
...Widgets
}
}
fragment Widgets on Widget {
... on WidgetA {
name
children {
total
nodes { name }
}
}
... on WidgetB {
name
children {
total
nodes { name foo }
}
}
}
""")

val queryInlined = gqldoc("""
query {
widget {
__typename
... on WidgetA {
name
children {
total
nodes { name }
}
}
... on WidgetB {
name
children {
total
nodes { name foo }
}
}
}
}
""")
for {
i <- schema.interpreter
data1 <- i.execute(queryFragments).map(_.data.toString)
data2 <- i.execute(queryInlined).map(_.data.toString)
expected =
"""{"widget":{"__typename":"WidgetB","name":"a","children":{"total":1,"nodes":[{"name":"a","foo":"FOO"}]}}}"""
} yield assertTrue(data1 == expected, data2 == expected)
}
)
}
Expand Down Expand Up @@ -490,3 +547,72 @@ object NestedInterfaceIssue {
caliban.graphQL(RootResolver(queries))
}
}

object i2076 {
sealed trait Widget
object Widget {
implicit val schema: Schema[Any, Widget] = Schema.gen

case class Args(limit: Option[Int])
object Args {
implicit val schema: Schema[Any, Args] = Schema.gen
implicit val argBuilder: ArgBuilder[Args] = ArgBuilder.gen
}

@GQLName("WidgetA")
case class A(
name: String,
children: Args => ZIO[Any, Throwable, A.Connection]
) extends Widget
object A {
implicit val schema: Schema[Any, A] = Schema.gen

@GQLName("WidgetAConnection")
case class Connection(total: Int, nodes: Chunk[Child])
object Connection {
implicit val schema: Schema[Any, Connection] = Schema.gen
}

@GQLName("WidgetAChild")
case class Child(name: String, foo: String)
object Child {
implicit val schema: Schema[Any, Child] = Schema.gen
}
}

@GQLName("WidgetB")
case class B(
name: String,
children: Args => ZIO[Any, Throwable, B.Connection]
) extends Widget
object B {
implicit val schema: Schema[Any, B] = Schema.gen

@GQLName("WidgetBConnection")
case class Connection(total: Int, nodes: Chunk[Child])
object Connection {
implicit val schema: Schema[Any, Connection] = Schema.gen
}

@GQLName("WidgetBChild")
case class Child(name: String, foo: String)
object Child {
implicit val schema: Schema[Any, Child] = Schema.gen
}
}
}

case class Queries(
widget: Option[Widget]
)
object Queries {
implicit val schema: Schema[Any, Queries] = Schema.gen
}

val schema = {
val queries = Queries(
Some(Widget.B("a", _ => ZIO.succeed(Widget.B.Connection(1, Chunk(Widget.B.Child("a", "FOO"))))))
)
graphQL(RootResolver(queries))
}
}

0 comments on commit 0c99202

Please sign in to comment.