codingwell / scala-guice

Scala extensions for Google Guice
Apache License 2.0
341 stars 44 forks source link

injector.instance[Foo[Array[Byte]] does not properly find the bound type #86

Closed cacoco closed 4 years ago

cacoco commented 4 years ago

We have a case where a Module provides a type of Foo[Array[Byte]] to the object graph via an @Provides-annotated method in a Module. When that type is manually looked up via injector.instance[Foo[Array[Byte]]] it is not found.

It appears as though the TypeLiteral built by net.codingwell.scalaguice.typeLiteral ends up using the java.lang.Byte[] boxed type as the generic arg type, so the Key is built for type Foo[java.lang.Byte[]], though the type bound is actually the primitive, byte[] type -- Foo[byte[]].

This is on Scala 2.12.10 running JDK 8 and using scalaguice 4.2.7 and google guice 4.2.3.

Example:

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> import com.google.inject.{AbstractModule, Guice, Provides}
import com.google.inject.{AbstractModule, Guice, Provides}

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

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

scala> val module = new AbstractModule with ScalaModule {
     |   @Singleton
     |   @Provides
     |   def provideSeq: Seq[Array[Byte]] =
     |     Seq.empty[Array[Byte]]
     | }
module: com.google.inject.AbstractModule with net.codingwell.scalaguice.ScalaModule{def provideSeq: Seq[Array[Byte]]} = $anon$1@2e1291a4

scala> val i = Guice.createInjector(module)
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=scala.collection.Seq<byte[]>, annotation=[none]], source=public scala.collection.Seq $anon$1.provideSeq(), scope=Scopes.SINGLETON, provider=@Provides $anon$1.provideSeq(<console>:18)}]}

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

scala> i.instance[Seq[Array[Byte]]]
com.google.inject.ConfigurationException: Guice configuration errors:

1) No implementation for scala.collection.Seq<java.lang.Byte[]> was bound.
  while locating scala.collection.Seq<java.lang.Byte[]>

1 error
  at com.google.inject.internal.InjectorImpl.getProvider(InjectorImpl.java:1120)
  at com.google.inject.internal.InjectorImpl.getInstance(InjectorImpl.java:1126)
  at net.codingwell.scalaguice.InjectorExtensions$ScalaInjector.instance(InjectorExtensions.scala:27)
  ... 34 elided

scala> 

Tracing it through, it seems that the TypeConversions.scalaTypeToJavaType does not properly handle this case for building a JavaType or at least builds one that will not match the type bound to the object graph. Any help or guidance would be appreciated. Thanks!

cacoco commented 4 years ago

I should note that this works as expected in scalaguice 4.2.0 which is pre-transition from Manifests to TypeTags and currently blocks our upgrade of Finatra to the latest version (which supports Scala 2.13 and part of why we're looking to upgrade).

tsuckow commented 4 years ago

Always something. Does look like the type conversions needs a special case for primitive arrays.

This (new) test fails

it("should handle Primative Arrays") {
      typeLiteral[Option[Array[Byte]]] shouldEqual new TypeLiteral[Option[Array[Byte]]] {}
    }

I'll look at it in my spare time, if you have any thoughts about how to do that let me know

tsuckow commented 4 years ago

I believe i fixed it. I'll see about commiting and publishing in a bit.

tsuckow commented 4 years ago

Should be fixed in 4.2.8 which should be synced with Maven Central in the next hour or two.

cacoco commented 4 years ago

@tsuckow thanks a ton -- we appreciate it.