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

ExcludeArgument transformer shouldn't exclude non-nullable arguments #2257

Merged
merged 2 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
3 changes: 3 additions & 0 deletions core/src/main/scala/caliban/introspection/adt/__Field.scala
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,7 @@ case class __Field(
args(__DeprecatedArgs.include)

private[caliban] lazy val _type: __Type = `type`()

private[caliban] lazy val allArgNames: Set[String] =
allArgs.view.map(_.name).toSet
}
16 changes: 9 additions & 7 deletions core/src/main/scala/caliban/transformers/Transformer.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package caliban.transformers

import caliban.CalibanError.ValidationError
import caliban.InputValue
import caliban.execution.Field
import caliban.introspection.adt._
Expand Down Expand Up @@ -278,6 +277,7 @@ object Transformer {
* )
* }}}
*
* @note the '''argument must be optional''', otherwise the filter will be silently ignored
* @param f tuples in the format of `(TypeName -> fieldName -> argumentToBeExcluded)`
*/
def apply(f: ((String, String), String)*): Transformer[Any] =
Expand All @@ -292,15 +292,15 @@ object Transformer {

final private class ExcludeArgument(map: Map[String, Map[String, Set[String]]]) extends Transformer[Any] {

private def shouldKeep(typeName: String, fieldName: String, argName: String): Boolean =
!getFromMap2(map, Set.empty[String])(typeName, fieldName).contains(argName)
private def shouldExclude(typeName: String, fieldName: String, arg: __InputValue): Boolean =
arg._type.isNullable && getFromMap2(map, Set.empty[String])(typeName, fieldName).contains(arg.name)

val typeVisitor: TypeVisitor =
TypeVisitor.fields.modifyWith((t, field) =>
field.copy(args =
field
.args(_)
.filter(arg => shouldKeep(t.name.getOrElse(""), field.name, arg.name))
.filterNot(arg => shouldExclude(t.name.getOrElse(""), field.name, arg))
)
)

Expand All @@ -313,9 +313,11 @@ object Transformer {
val fields = step.fields
step.copy(fields =
fieldName =>
inner.getOrElse(fieldName, null) match {
case null => fields(fieldName)
case excl => mapFunctionStep(fields(fieldName))(_.filterNot { case (argName, _) => excl(argName) })
if (inner.contains(fieldName)) {
val args = field.fieldType.allFieldsMap(fieldName).allArgNames
mapFunctionStep(fields(fieldName))(_.filterNot { case (argName, _) => !args.contains(argName) })
} else {
fields(fieldName)
}
)
}
Expand Down
76 changes: 55 additions & 21 deletions core/src/test/scala/caliban/transformers/TransformerSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -139,28 +139,62 @@ object TransformerSpec extends ZIOSpecDefault {
|}""".stripMargin
)
},
test("filter argument") {
case class Args(arg: Option[String])
case class Query(a: Args => String)
val api: GraphQL[Any] = graphQL(RootResolver(Query(_.arg.getOrElse("missing"))))
suite("ExcludeArgument")(
test("filter nullable argument") {
case class Args(arg: Option[String])
case class Query(a: Args => String)
val api: GraphQL[Any] = graphQL(RootResolver(Query(_.arg.getOrElse("missing"))))

val transformed: GraphQL[Any] = api.transform(Transformer.ExcludeArgument("Query" -> "a" -> "arg"))
val rendered = transformed.render
for {
interpreter <- transformed.interpreter
result <- interpreter.execute("""{ a }""").map(_.data.toString)
} yield assertTrue(
result == """{"a":"missing"}""",
rendered ==
"""schema {
| query: Query
|}
|
|type Query {
| a: String!
|}""".stripMargin
)
},
val transformed: GraphQL[Any] = api.transform(Transformer.ExcludeArgument("Query" -> "a" -> "arg"))
val rendered = transformed.render
for {
interpreter <- transformed.interpreter
result <- interpreter.execute("""{ a }""").map(_.data.toString)
} yield assertTrue(
result == """{"a":"missing"}""",
rendered ==
"""schema {
| query: Query
|}
|
|type Query {
| a: String!
|}""".stripMargin
)
},
test("cannot filter non-nullable arguments") {
case class Args(arg1: String, arg2: Option[String], arg3: Option[String])
case class Query(a: Args => String)
val api: GraphQL[Any] = graphQL(
RootResolver(
Query(t => s"a1:${t.arg1} a2:${t.arg2.getOrElse("missing")} a3:${t.arg3.getOrElse("missing")}")
)
)

val transformed: GraphQL[Any] = api.transform(
Transformer.ExcludeArgument(
"Query" -> "a" -> "arg1",
"Query" -> "a" -> "arg2"
)
)
val rendered = transformed.render
for {
_ <- Configurator.setSkipValidation(true)
interpreter <- transformed.interpreter
result <- interpreter.execute("""{ a(arg1:"foo", arg2:"bar", arg3:"baz") }""").map(_.data.toString)
} yield assertTrue(
result == """{"a":"a1:foo a2:missing a3:baz"}""",
rendered ==
"""schema {
| query: Query
|}
|
|type Query {
| a(arg1: String!, arg3: String): String!
|}""".stripMargin
)
}
),
test("combine transformers") {
val transformed: GraphQL[Any] = api
.transform(Transformer.RenameType("InnerObject" -> "Renamed"))
Expand Down
Loading