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

injector.instance[Class with Trait] throws UnsupportedException #87

Closed
cacoco opened this issue Jun 23, 2020 · 5 comments
Closed

injector.instance[Class with Trait] throws UnsupportedException #87

cacoco opened this issue Jun 23, 2020 · 5 comments

Comments

@cacoco
Copy link

cacoco commented Jun 23, 2020

Hey there, back again. We're continuing our upgrade of Finatra to the latest version of scalaguice and have encountered another issue around the TypeConversions logic.

We have a fair amount of usage of the finagle-mysql library where a common pattern is to create a MySQL client, then augment it for doing transactions, e.g., Client with Transactions. An instance of this type is then typically provided to the object graph via an @Provides-annotated method in a Module. However, with the latest scalaguice library, the TypeConversions fails to translate the ScalaType to a JavaType and thus any attempt to get the bound instance via an injector lookup throws an exception like this:

java.lang.UnsupportedOperationException: Could not convert scalaType com.twitter.finagle.mysql.Client with com.twitter.finagle.mysql.Transactions to a javaType: scala.reflect.internal.Types$RefinedType0
                     	at net.codingwell.scalaguice.TypeConversions$.scalaTypeToJavaType(TypeConversions.scala:89)
                     	at net.codingwell.scalaguice.package$.typeLiteral(package.scala:37)

Here's an example repro using Scala 2.12.10, JDK 8, scalaguice 4.2.8 and guice 4.2.3.

Welcome to Scala 2.12.10 (JDK 64-Bit Server VM, Java 1.8.0_222).
Type in expressions for evaluation. Or try :help.

scala> class SomeClient {
     |   private[this] val underlying: String = "Alice"
     |
     |   def doSomething: String = s"Hello, $underlying."
     | }
defined class SomeClient

scala>

scala> trait Augmentation { self: SomeClient =>
     |   override def doSomething: String = s"${self.doSomething} Welcome to Wonderland."
     | }
defined trait Augmentation

scala>

scala> import com.google.inject.{AbstractModule, Guice, Provides}
import com.google.inject.{AbstractModule, Guice, Provides}

scala> import net.codingwell.scalaguice.ScalaModule
import net.codingwell.scalaguice.ScalaModule

scala> import net.codingwell.scalaguice.InjectorExtensions._
import net.codingwell.scalaguice.InjectorExtensions._

scala> import javax.inject.Singleton
import javax.inject.Singleton

scala>

scala> val i = Guice.createInjector(new AbstractModule with ScalaModule {
     |   @Provides
     |   @Singleton
     |   def provideSomeClient: SomeClient with Augmentation = {
     |     new SomeClient with Augmentation
     |   }
     | })
i: com.google.inject.Injector = Injector{bindings=[InstanceBinding{key=Key[type=com.google.inject.Stage, annotation=[none]], source=[unknown source], instance=DEVELOPMENT}, ProviderInstanceBinding{key=Key[type=com.google.inject.Injector, annotation=[none]], source=[unknown source], scope=Scopes.NO_SCOPE, provider=Provider<Injector>}, ProviderInstanceBinding{key=Key[type=java.util.logging.Logger, annotation=[none]], source=[unknown source], scope=Scopes.NO_SCOPE, provider=Provider<Logger>}, ProviderInstanceBinding{key=Key[type=SomeClient, annotation=[none]], source=public SomeClient $anon$1.provideSomeClient(), scope=Scopes.SINGLETON, provider=@Provides $anon$1.provideSomeClient(<console>:23)}]}

scala> i.instance[SomeClient with Augmentation]
java.lang.UnsupportedOperationException: Could not convert scalaType SomeClient with Augmentation to a javaType: scala.reflect.internal.Types$RefinedType0
  at net.codingwell.scalaguice.TypeConversions$.scalaTypeToJavaType(TypeConversions.scala:89)
  at net.codingwell.scalaguice.package$.typeLiteral(package.scala:37)
  at net.codingwell.scalaguice.InjectorExtensions$ScalaInjector.instance(InjectorExtensions.scala:27)
  ... 34 elided

scala>

Binding this type via the scalaguice bind DSL in the ScalaModule also fails similarly:

scala> val ii = Guice.createInjector(new AbstractModule with ScalaModule {
     |   override def configure(): Unit = {
     |     bind[SomeClient with Augmentation].toInstance(new SomeClient with Augmentation)
     |   }
     | })
Jun 23, 2020 7:54:25 AM com.google.inject.internal.MessageProcessor visit
INFO: An exception was caught and reported. Message: java.lang.UnsupportedOperationException: Could not convert scalaType SomeClient with Augmentation to a javaType: scala.reflect.internal.Types$RefinedType0
java.lang.UnsupportedOperationException: Could not convert scalaType SomeClient with Augmentation to a javaType: scala.reflect.internal.Types$RefinedType0
        at net.codingwell.scalaguice.TypeConversions$.scalaTypeToJavaType(TypeConversions.scala:89)
        at net.codingwell.scalaguice.package$.typeLiteral(package.scala:37)
        at net.codingwell.scalaguice.InternalModule$BindingBuilder.<init>(ScalaModule.scala:70)
        at net.codingwell.scalaguice.InternalModule.bind(ScalaModule.scala:75)
        at net.codingwell.scalaguice.InternalModule.bind$(ScalaModule.scala:75)
        at $line13.$read$$iw$$iw$$iw$$iw$$anon$1.bind(<console>:19)
        at $line13.$read$$iw$$iw$$iw$$iw$$anon$1.configure(<console>:21)
        at com.google.inject.AbstractModule.configure(AbstractModule.java:61)
        at com.google.inject.spi.Elements$RecordingBinder.install(Elements.java:347)
        at com.google.inject.spi.Elements.getElements(Elements.java:104)
        at com.google.inject.internal.InjectorShell$Builder.build(InjectorShell.java:137)
        at com.google.inject.internal.InternalInjectorCreator.build(InternalInjectorCreator.java:105)
        at com.google.inject.Guice.createInjector(Guice.java:87)
        at com.google.inject.Guice.createInjector(Guice.java:69)
        at com.google.inject.Guice.createInjector(Guice.java:59)
        at $line13.$read$$iw$$iw$$iw$$iw$.<init>(<console>:19)
        at $line13.$read$$iw$$iw$$iw$$iw$.<clinit>(<console>)
        at $line13.$eval$.$print$lzycompute(<console>:7)
        at $line13.$eval$.$print(<console>:6)
        at $line13.$eval.$print(<console>)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at scala.tools.nsc.interpreter.IMain$ReadEvalPrint.call(IMain.scala:745)
        at scala.tools.nsc.interpreter.IMain$Request.loadAndRun(IMain.scala:1021)
        at scala.tools.nsc.interpreter.IMain.$anonfun$interpret$1(IMain.scala:574)
        at scala.reflect.internal.util.ScalaClassLoader.asContext(ScalaClassLoader.scala:41)
        at scala.reflect.internal.util.ScalaClassLoader.asContext$(ScalaClassLoader.scala:37)
        at scala.reflect.internal.util.AbstractFileClassLoader.asContext(AbstractFileClassLoader.scala:41)
        at scala.tools.nsc.interpreter.IMain.loadAndRunReq$1(IMain.scala:573)
        at scala.tools.nsc.interpreter.IMain.interpret(IMain.scala:600)
        at scala.tools.nsc.interpreter.IMain.interpret(IMain.scala:570)
        at scala.tools.nsc.interpreter.ILoop.interpretStartingWith(ILoop.scala:894)
        at scala.tools.nsc.interpreter.ILoop.interpretStartingWith(ILoop.scala:912)
        at scala.tools.nsc.interpreter.ILoop.interpretStartingWith(ILoop.scala:912)
        at scala.tools.nsc.interpreter.ILoop.interpretStartingWith(ILoop.scala:912)
        at scala.tools.nsc.interpreter.ILoop.interpretStartingWith(ILoop.scala:912)
        at scala.tools.nsc.interpreter.ILoop.command(ILoop.scala:762)
        at scala.tools.nsc.interpreter.ILoop.processLine(ILoop.scala:464)
        at scala.tools.nsc.interpreter.ILoop.loop(ILoop.scala:485)
        at scala.tools.nsc.interpreter.ILoop.process(ILoop.scala:1077)
        at scala.tools.nsc.MainGenericRunner.runTarget$1(MainGenericRunner.scala:89)
        at scala.tools.nsc.MainGenericRunner.run$1(MainGenericRunner.scala:92)
        at scala.tools.nsc.MainGenericRunner.process(MainGenericRunner.scala:103)
        at scala.tools.nsc.MainGenericRunner$.main(MainGenericRunner.scala:108)
        at scala.tools.nsc.MainGenericRunner.main(MainGenericRunner.scala)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.pantsbuild.tools.runner.PantsRunner.runMainMethod(PantsRunner.java:98)
        at org.pantsbuild.tools.runner.PantsRunner.main(PantsRunner.java:42)

com.google.inject.CreationException: Unable to create injector, see the following errors:

1) An exception was caught and reported. Message: Could not convert scalaType SomeClient with Augmentation to a javaType: scala.reflect.internal.Types$RefinedType0
  at com.google.inject.internal.InjectorShell$Builder.build(InjectorShell.java:137)

1 error
  at com.google.inject.internal.Errors.throwCreationExceptionIfErrorsExist(Errors.java:554)
  at com.google.inject.internal.InternalInjectorCreator.initializeStatically(InternalInjectorCreator.java:161)
  at com.google.inject.internal.InternalInjectorCreator.build(InternalInjectorCreator.java:108)
  at com.google.inject.Guice.createInjector(Guice.java:87)
  at com.google.inject.Guice.createInjector(Guice.java:69)
  at com.google.inject.Guice.createInjector(Guice.java:59)
  ... 38 elided
Caused by: java.lang.UnsupportedOperationException: Could not convert scalaType SomeClient with Augmentation to a javaType: scala.reflect.internal.Types$RefinedType0
  at net.codingwell.scalaguice.TypeConversions$.scalaTypeToJavaType(TypeConversions.scala:89)
  at net.codingwell.scalaguice.package$.typeLiteral(package.scala:37)
  at net.codingwell.scalaguice.InternalModule$BindingBuilder.<init>(ScalaModule.scala:70)
  at net.codingwell.scalaguice.InternalModule.bind(ScalaModule.scala:75)
  at net.codingwell.scalaguice.InternalModule.bind$(ScalaModule.scala:75)
  at $anon$1.bind(<console>:19)
  at $anon$1.configure(<console>:21)
  at com.google.inject.AbstractModule.configure(AbstractModule.java:61)
  at com.google.inject.spi.Elements$RecordingBinder.install(Elements.java:347)
  at com.google.inject.spi.Elements.getElements(Elements.java:104)
  at com.google.inject.internal.InjectorShell$Builder.build(InjectorShell.java:137)
  at com.google.inject.internal.InternalInjectorCreator.build(InternalInjectorCreator.java:105)
  ... 41 more

scala>

And like before this worked in scalaguice 4.2.0 pre the transition from Manifests to TypeTags.

I wanted to also point out that translation from Scala types to Java types is something we have to do a lot in JSON serialization/deserialization and after years of having our own hand-rolled stuff we switched to make use of the reflection portion of the json4s library which handles a fair amount of these cases -- not sure if it's worth taking a look but thought I'd mention.

Any help here would be greatly appreciated as this blocks our Finatra library upgrade to the latest version of scalaguice which enables us to get to Scala 2.13 support. Thanks!

@cacoco
Copy link
Author

cacoco commented Jun 23, 2020

I wanted to add that we've discovered another common case where this fails for us as well. When attempting to bind or read the bound instance of a Jackson ObjectMapper augmented with the ScalaObjectMapper:

java.lang.UnsupportedOperationException: Could not convert scalaType com.fasterxml.jackson.databind.ObjectMapper with com.fasterxml.jackson.module.scala.ScalaObjectMapper to a javaType: scala.reflect.internal.Types$RefinedType0
                     	at net.codingwell.scalaguice.TypeConversions$.scalaTypeToJavaType(TypeConversions.scala:89)
                     	at net.codingwell.scalaguice.package$.typeLiteral(package.scala:37)

Thanks.

@tsuckow
Copy link
Member

tsuckow commented Jun 23, 2020

I can probably look at that in a few days. If you wanted to fiddle with substituting the contents of the scalatypetojavatype method with the json4s stuff that would greatly speed it up. The only time I have to work on this is after dinner when the spouse is busy.

If the json4s thing worked it would fix a lot of issues and probably get rid of all the ignored tests. We should sure to make regression tests for all the cases you are finding.

@cacoco
Copy link
Author

cacoco commented Jun 23, 2020

@tsuckow understood -- thanks for your help here. Let me see if I can get json4s stuff working without too much surgery. Thanks again.

@cacoco
Copy link
Author

cacoco commented Jun 24, 2020

@tsuckow took a stab at something minimally invasive: #88.

Additionally looking at scala.reflect.internal.Types, there's a comment which list a standard pattern match against types, see here which is a bit illuminating on the cases to potentially be accounted for.

@tsuckow
Copy link
Member

tsuckow commented Jun 25, 2020

Should be fixed in 4.2.9. Thank you for your PR's

@tsuckow tsuckow closed this as completed Jun 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants