Skip to content

Commit d6c967c

Browse files
jchybjchyb
committed
Fix issue with wrong Action[Ctx, _] conversion being summoned
1 parent c659711 commit d6c967c

File tree

5 files changed

+128
-58
lines changed

5 files changed

+128
-58
lines changed

‎build.sbt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,11 @@ lazy val derivation = project
254254
name := "sangria-derivation",
255255
Test / testOptions += Tests.Argument(TestFrameworks.ScalaTest, "-oF"),
256256
mimaPreviousArtifacts := Set("org.sangria-graphql" %% "sangria-derivation" % "4.0.0"),
257+
mimaBinaryIssueFilters ++= Seq(
258+
// internal method
259+
ProblemFilters.exclude[DirectMissingMethodProblem](
260+
"sangria.macros.derive.DeriveMacroSupport.unsafeSelectByName")
261+
),
257262
// Macros
258263
libraryDependencies ++= (if (isScala3.value) Seq.empty
259264
else Seq("org.scala-lang" % "scala-reflect" % scalaVersion.value)),

‎modules/derivation/src/main/scala-3/sangria/macros/derive/DeriveMacroSupport.scala‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import sangria.schema.{InputType, OutputType}
55
import scala.quoted._
66
import scala.reflect.ClassTag
77

8-
trait DeriveMacroSupport {
8+
private[derive] trait DeriveMacroSupport {
99

1010
def reportErrors(using Quotes)(errors: Seq[(PositionPointer, String)]) = {
1111
import quotes.reflect.report
@@ -109,11 +109,11 @@ trait DeriveMacroSupport {
109109
}
110110
}
111111

112-
protected def unsafeSelectByName[S](using quotes: Quotes)(using
113-
Type[S])(memberExpr: Expr[_], name: String): Expr[S] = {
112+
protected def unsafeSelectByName[T: Type](using quotes: Quotes)(
113+
memberExpr: Expr[_], name: String
114+
): Expr[T] = {
114115
import quotes.reflect._
115-
val a = Select.unique(memberExpr.asTerm, name)
116-
a.etaExpand(Symbol.spliceOwner).asExprOf[S]
116+
Select.unique(memberExpr.asTerm, name).asExprOf[T]
117117
}
118118

119119
protected def getClassTag[T](using Type[T], Quotes): Expr[ClassTag[T]] = {

‎modules/derivation/src/main/scala-3/sangria/macros/derive/DeriveObjectTypeMacro.scala‎

Lines changed: 27 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -129,34 +129,13 @@ class DeriveObjectTypeMacro(using globalQuotes: Quotes) extends DeriveMacroSuppo
129129
ctxValType match {
130130
case Some((tpe, fn)) =>
131131
given Type[CtxVal] = tpe
132-
Expr.summon[t => Action[Ctx, t]] match
133-
case Some(conversion) =>
134-
'{
135-
$conversion(${
136-
unsafeSelectByName[t]('{ $fn(c.ctx) }, field.method.name)
137-
})
138-
}
139-
case None =>
140-
reportSummoningErrors(
141-
Seq(
142-
s"Implicit conversion not found: ${TypeRepr.of[t => Action[Ctx, t]].show}"),
143-
Seq(None)
144-
)
132+
wrappedInActionConversion[Ctx, t](
133+
unsafeSelectByName[t]('{ $fn(c.ctx) }, field.method.name)
134+
)
145135
case None =>
146-
Expr.summon[t => Action[Ctx, t]] match
147-
case Some(conversion) =>
148-
'{
149-
$conversion(${
150-
unsafeSelectByName[t]('{ c.value }, field.method.name)
151-
.asExprOf[t]
152-
})
153-
}
154-
case None =>
155-
reportSummoningErrors(
156-
Seq(
157-
s"Implicit conversion not found: ${TypeRepr.of[t => Action[Ctx, t]].show}"),
158-
Seq(None)
159-
)
136+
wrappedInActionConversion[Ctx, t](
137+
unsafeSelectByName[t]('{ c.value }, field.method.name)
138+
)
160139
}
161140
}
162141
}
@@ -181,7 +160,7 @@ class DeriveObjectTypeMacro(using globalQuotes: Quotes) extends DeriveMacroSuppo
181160
case Some(lookup) => '{ $lookup.graphqlType }
182161
case None =>
183162
reportSummoningErrors(
184-
Seq(s"GraphQlOutputType not found: ${TypeRepr.of[t => Action[Ctx, t]].show}"),
163+
Seq(s"GraphQlOutputType not found: ${TypeRepr.of[t => Action[Ctx, _]].show}"),
185164
Seq(None)
186165
)
187166
}
@@ -319,40 +298,35 @@ class DeriveObjectTypeMacro(using globalQuotes: Quotes) extends DeriveMacroSuppo
319298
case d: DefDef =>
320299
d.returnTpt.tpe.asType match
321300
case '[t] =>
322-
Expr.summon[t => Action[Ctx, t]] match
323-
case Some(conversion) =>
324-
'{
325-
$conversion(${
326-
args
327-
.foldLeft[Select | Apply](method) { (m, argList) =>
328-
Apply(m, argList)
329-
}
330-
.asExprOf[t]
331-
})
301+
wrappedInActionConversion[Ctx, t](
302+
args
303+
.foldLeft[Select | Apply](method) { (m, argList) =>
304+
Apply(m, argList)
332305
}
333-
case None =>
334-
reportSummoningErrors(
335-
Seq(
336-
s"Implicit conversion not found: ${TypeRepr.of[t => Action[Ctx, t]].show}"),
337-
Seq(None)
338-
)
306+
.asExprOf[t]
307+
)
339308
else
340309
globalQuotes.reflect.Ref(member.method).tpe.widen.asType match {
341310
case '[t] =>
342-
Expr.summon[t => Action[Ctx, t]] match
343-
case Some(conversion) =>
344-
'{ $conversion(${ method.asExprOf[t] }) }
345-
case None =>
346-
reportSummoningErrors(
347-
Seq(
348-
s"Implicit conversion not found: ${TypeRepr.of[t => Action[Ctx, t]].show}"),
349-
Seq(None)
350-
)
311+
wrappedInActionConversion[Ctx, t](method.asExprOf[t])
351312
}
352313
}
353314
}
354315
}
355316

317+
private def wrappedInActionConversion[Ctx: Type, T: Type](value: Expr[T]) = {
318+
import globalQuotes.reflect._
319+
Expr.summon[T => Action[Ctx, _]] match
320+
case Some(conversion) =>
321+
'{ $conversion(${ value }) }
322+
case None =>
323+
reportSummoningErrors(
324+
Seq(
325+
s"Implicit conversion not found: ${TypeRepr.of[T => Action[Ctx, _]].show}"),
326+
Seq(None)
327+
)
328+
}
329+
356330
private def createArg(config: Seq[MacroDeriveObjectSetting], member: KnownMember)(
357331
arg: globalQuotes.reflect.Symbol) =
358332
import globalQuotes.reflect._

‎modules/derivation/src/test/scala/sangria/macros/derive/DeriveObjectTypeMacroSpec.scala‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -836,5 +836,32 @@ class DeriveObjectTypeMacroSpec extends AnyWordSpec with Matchers with FutureRes
836836

837837
"deriveObjectType[Unit, TestContainer[TestSubject]]()" should compile
838838
}
839+
840+
"handle Future result values correctly (issue #906)" in {
841+
import Issue906Context._
842+
import sangria.parser.QueryParser
843+
844+
val query = QueryParser
845+
.parse(
846+
s"""
847+
query SomeTest {
848+
mySample {
849+
myFutureString
850+
}
851+
}
852+
"""
853+
)
854+
.fold(throw _, identity)
855+
856+
Executor.execute(MySample.schema, query, MyRepository.sample).await should be(
857+
Map(
858+
"data" -> Map(
859+
"mySample" -> Map(
860+
"myFutureString" -> "Hello World"
861+
)
862+
)
863+
)
864+
)
865+
}
839866
}
840867
}

‎modules/derivation/src/test/scala/sangria/macros/derive/Issue906Context.scala‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
package sangria.macros.derive
2+
3+
import sangria.schema._
4+
5+
import scala.concurrent.Future
6+
7+
object Issue906Context {
8+
9+
@GraphQLName("SampleCaseClass")
10+
@GraphQLDescription("A sample case class")
11+
final case class SampleCaseClass(
12+
name: String
13+
) {
14+
@GraphQLField
15+
@GraphQLName("myString")
16+
def graphqlMyString: String = "Hello World"
17+
18+
@GraphQLField
19+
@GraphQLName("myFutureString")
20+
def graphqlMyFutureString: Future[String] = Future.successful("Hello World")
21+
22+
}
23+
24+
object SampleCaseClass {
25+
val sample: SampleCaseClass = SampleCaseClass("My test")
26+
}
27+
28+
trait MyRepository {
29+
def getSampleCaseClass: SampleCaseClass
30+
}
31+
32+
object MyRepository {
33+
def sample: MyRepository = new MyRepository {
34+
override def getSampleCaseClass: SampleCaseClass = SampleCaseClass.sample
35+
}
36+
}
37+
38+
object MySample {
39+
val schema: Schema[MyRepository, Unit] = {
40+
41+
implicit val graphqlSampleCaseClass: ObjectType[MyRepository, SampleCaseClass] =
42+
deriveObjectType[MyRepository, SampleCaseClass]()
43+
44+
val queries: ObjectType[MyRepository, Unit] = ObjectType(
45+
"Query",
46+
fields(
47+
Field(
48+
"mySample",
49+
graphqlSampleCaseClass,
50+
Some("test"),
51+
arguments = Nil,
52+
resolve = (ctx: Context[MyRepository, Unit]) => SampleCaseClass.sample
53+
)
54+
)
55+
)
56+
57+
Schema(
58+
query = queries,
59+
mutation = None,
60+
additionalTypes = Nil
61+
)
62+
}
63+
}
64+
}

0 commit comments

Comments
 (0)