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

Support automatic binding/injection #14

Closed
magro opened this Issue Mar 15, 2014 · 5 comments

Comments

Projects
None yet
3 participants
@magro

magro commented Mar 15, 2014

I'm starting to use scaldi with play. AFAICS all controllers need to be registered/bound explicitely in a Module. It would be nice if this would not be required, so that a class (Play controller in this case) would be bound when it's first requested (via injector.getBinding).

@OlegIlyenko

This comment has been minimized.

Show comment
Hide comment
@OlegIlyenko

OlegIlyenko Mar 16, 2014

Member

I'm very careful, when it comes to any kind of auto-discovery or automatic binding creation. So this also reflected in the feature set, as you can see :) That said, library is also designed with high degree of extensibility in mind - you can customize or extend nearly any part of the library, including the injectors themselves. To implement the feature you described, you can simply create a new injector, that will create controllers on-fly:

import play.api.mvc._
import scaldi._
import scala.reflect.runtime.universe.{runtimeMirror, typeTag}

class ControllerInjector extends MutableInjectorUser with InjectorWithLifecycle[ControllerInjector] with ShutdownHookLifecycleManager {

  private var bindings: List[BindingWithLifecycle] = Nil

  def getBindingInternal(identifiers: List[Identifier]) = identifiers match {
    case TypeTagIdentifier(tpe) :: Nil if tpe <:< typeTag[Controller].tpe =>
      bindings.find(_.isDefinedFor(identifiers)) orElse {
        this.synchronized {
          bindings.find(_.isDefinedFor(identifiers)) orElse {
            val controller =
              tpe.members
                .filter(_.isMethod)
                .map(_.asMethod)
                .find(m => m.isConstructor && (m.paramss match {
                  case List(Nil, List(paramType)) if paramType.typeSignature <:< typeTag[Injector].tpe  => true
                  case _ => false
                }))
                .map { constructor =>
                  val mirror = runtimeMirror(this.getClass.getClassLoader)
                  mirror.reflectClass(tpe.typeSymbol.asClass).reflectConstructor(constructor)(injector)
                }
                .getOrElse (throw new IllegalArgumentException(s"Type $tpe does not have constructor with single `Injector` argument."))
            val binding = LazyBinding(Some(() => controller), identifiers)
            bindings = bindings :+ binding
            Some(binding)
          }
        }
      }
    case _ => None
  }

  def getBindingsInternal(identifiers: List[Identifier]) = getBindingInternal(identifiers).toList

  protected def init(lifecycleManager: LifecycleManager) = () => ()
}

Now you can use it in the play application like this, for instance:

object Global extends GlobalSettings with ScaldiSupport {
  def applicationModule = new SomeOtherModule :: new ControllerInjector
}

Meanwhile, I will think this feature through and gather some feedback. This is definitely an interesting idea. So maybe, at some point in future it would be included in the scaldi or scaldi-play in one way or another.

Member

OlegIlyenko commented Mar 16, 2014

I'm very careful, when it comes to any kind of auto-discovery or automatic binding creation. So this also reflected in the feature set, as you can see :) That said, library is also designed with high degree of extensibility in mind - you can customize or extend nearly any part of the library, including the injectors themselves. To implement the feature you described, you can simply create a new injector, that will create controllers on-fly:

import play.api.mvc._
import scaldi._
import scala.reflect.runtime.universe.{runtimeMirror, typeTag}

class ControllerInjector extends MutableInjectorUser with InjectorWithLifecycle[ControllerInjector] with ShutdownHookLifecycleManager {

  private var bindings: List[BindingWithLifecycle] = Nil

  def getBindingInternal(identifiers: List[Identifier]) = identifiers match {
    case TypeTagIdentifier(tpe) :: Nil if tpe <:< typeTag[Controller].tpe =>
      bindings.find(_.isDefinedFor(identifiers)) orElse {
        this.synchronized {
          bindings.find(_.isDefinedFor(identifiers)) orElse {
            val controller =
              tpe.members
                .filter(_.isMethod)
                .map(_.asMethod)
                .find(m => m.isConstructor && (m.paramss match {
                  case List(Nil, List(paramType)) if paramType.typeSignature <:< typeTag[Injector].tpe  => true
                  case _ => false
                }))
                .map { constructor =>
                  val mirror = runtimeMirror(this.getClass.getClassLoader)
                  mirror.reflectClass(tpe.typeSymbol.asClass).reflectConstructor(constructor)(injector)
                }
                .getOrElse (throw new IllegalArgumentException(s"Type $tpe does not have constructor with single `Injector` argument."))
            val binding = LazyBinding(Some(() => controller), identifiers)
            bindings = bindings :+ binding
            Some(binding)
          }
        }
      }
    case _ => None
  }

  def getBindingsInternal(identifiers: List[Identifier]) = getBindingInternal(identifiers).toList

  protected def init(lifecycleManager: LifecycleManager) = () => ()
}

Now you can use it in the play application like this, for instance:

object Global extends GlobalSettings with ScaldiSupport {
  def applicationModule = new SomeOtherModule :: new ControllerInjector
}

Meanwhile, I will think this feature through and gather some feedback. This is definitely an interesting idea. So maybe, at some point in future it would be included in the scaldi or scaldi-play in one way or another.

@Mironor

This comment has been minimized.

Show comment
Hide comment
@Mironor

Mironor Jul 5, 2014

Contributor

This looks so complex to me..
Well, at least I have where to start, but do you think there is a solution to automatic Controller binding like here with Guice ?

Contributor

Mironor commented Jul 5, 2014

This looks so complex to me..
Well, at least I have where to start, but do you think there is a solution to automatic Controller binding like here with Guice ?

@OlegIlyenko

This comment has been minimized.

Show comment
Hide comment
@OlegIlyenko

OlegIlyenko Jul 5, 2014

Member

@Mironor Sure, scaldi-play adds this kind of support:

https://github.com/scaldi/scaldi-play/blob/master/src/main/scala/scaldi/play/ScaldiSupport.scala#L75

You just need to extend ScaldiSupport in your Global and it should work out-of-the box:

http://scaldi.org/learn/#play-integration

ControllerInjector described above supposed to auto-discover not explicitly bound controllers and bind then on the fly.

Member

OlegIlyenko commented Jul 5, 2014

@Mironor Sure, scaldi-play adds this kind of support:

https://github.com/scaldi/scaldi-play/blob/master/src/main/scala/scaldi/play/ScaldiSupport.scala#L75

You just need to extend ScaldiSupport in your Global and it should work out-of-the box:

http://scaldi.org/learn/#play-integration

ControllerInjector described above supposed to auto-discover not explicitly bound controllers and bind then on the fly.

@Mironor

This comment has been minimized.

Show comment
Hide comment
@Mironor

Mironor Jul 5, 2014

Contributor

Yes, indeed, but I'm talking about these four lines that are mandatory. You may consider to automatically inject corresponding Controller in getControllerInstance method as there are very few, if any, use cases where someone needs to inject another Controller instead of the one defined in the "routes" file.
I may be wrong though.

Contributor

Mironor commented Jul 5, 2014

Yes, indeed, but I'm talking about these four lines that are mandatory. You may consider to automatically inject corresponding Controller in getControllerInstance method as there are very few, if any, use cases where someone needs to inject another Controller instead of the one defined in the "routes" file.
I may be wrong though.

@OlegIlyenko OlegIlyenko added this to the 0.4.1 milestone Jul 5, 2014

@OlegIlyenko OlegIlyenko added the feature label Jul 5, 2014

@OlegIlyenko OlegIlyenko self-assigned this Jul 5, 2014

@OlegIlyenko

This comment has been minimized.

Show comment
Hide comment
@OlegIlyenko

OlegIlyenko Jul 6, 2014

Member

Yeah, I think both of you have a valid point. I added a ControllerInjector which will create controller bindings on the fly. Here is some documentation how you can use it:

http://scaldi.org/learn/#play-specific-injectors

It should be available shortly in scaldi-play version 0.4.1.

Member

OlegIlyenko commented Jul 6, 2014

Yeah, I think both of you have a valid point. I added a ControllerInjector which will create controller bindings on the fly. Here is some documentation how you can use it:

http://scaldi.org/learn/#play-specific-injectors

It should be available shortly in scaldi-play version 0.4.1.

@OlegIlyenko OlegIlyenko closed this Jul 6, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment