edvin / tornadofx

Lightweight JavaFX Framework for Kotlin
Apache License 2.0
3.67k stars 269 forks source link

Consolidate Dependency Injection between ScopeInstances and 3rd party DI frameworks #909

Open ahinchman1 opened 5 years ago

ahinchman1 commented 5 years ago

We currently have two injection mechanisms - consolidating the use would be convenient, especially since many folks use popular 3rd party di frameworks like:

The current mechanism for 3rd party injection with Fx.dicontainer is trivial enough, and it would be even nicer if we actually supplied default implementations for the most popular containers.

Much of these implementations are similar:

Spring example provided by current documentation: https://edvin.gitbooks.io/tornadofx-guide/content/part2/Dependency%20Injection.html

SpringFu example provided by @AlexCzar:

internal val koFu = application {
    //SpringFu config
}

class MyAppUi : App(LoginScreen::class) {
    override fun init() {
        koFu
            //init SpringFu context
            .run()
            .also { applicationContext ->
                FX.dicontainer = object : DIContainer {
                    override fun <T : Any> getInstance(type: KClass<T>): T =
                        applicationContext.getBean(type.java)

                    override fun <T : Any> getInstance(type: KClass<T>, name: String): T =
                        applicationContext.getBean(name, type.java)
                }
            }
    }
}

Google Guice implementation example by @edvin found in issue #79: https://github.com/edvin/tornadofx/issues/79

TODO Try out Dagger and AspectJ as well.

Ideas:

We could add an InjectionPoint class is that it could be extended later on without breaking backwards compatibility. We could also provide dependencies per framework for builds possibly.

Spring has an example for this here: https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/beans/factory/InjectionPoint.html

ahinchman1 commented 5 years ago

Found out my grocery delivery has been canceled, so I have to leave to get groceries before Chicago gets colder than Siberia. Ha. ha ha. Good news is on Wednesday, I'll have more time with this since everything will be closed including the office.

I played around with override the application implementation this morning - there's not much yet, but I have some questions about basic design details. For example, with Guice, I overrode the App class saying I'll force the user to create their own module. This is what I have so far. You can see I commented out the bottom initialization because the way I used the module initialization, I'm unable to include id support (yet). I'm not very familiar with how Guice is used but can look in to this a bit more.

package tornadofx.di.frameworks.guice

import com.google.inject.AbstractModule
import com.google.inject.Guice
import tornadofx.*
import kotlin.reflect.KClass

/**
 * If I end up overriding the class, will the other constructors need to be overridden as well?
  */

class TornadoFXGuice(override val primaryView: KClass<out UIComponent> = NoPrimaryViewSpecified::class,
                     vararg stylesheet: KClass<out Stylesheet>,
                     private val module: AbstractModule) : App() {
    init {
        Stylesheet.importServiceLoadedStylesheets()
        stylesheet.forEach { importStylesheet(it) }

        val guice = Guice.createInjector(module)

        FX.dicontainer = object : DIContainer {
            override fun <T : Any> getInstance(type: KClass<T>): T =
                    guice.getInstance(type.java)

            // override fun <T : Any> getInstance(type: KClass<T>, name: String): T=
               //     guice.getInstance(name, type.java)
        }
    }
}

Likewise, for Spring:

                     vararg stylesheet: KClass<out Stylesheet>,
                     private val classContext: ClassXmlApplicationContext) : App() {

    init {
        Stylesheet.importServiceLoadedStylesheets()
        stylesheet.forEach { importStylesheet(it) }

        FX.dicontainer = object : DIContainer {
            override fun <T : Any> getInstance(type: KClass<T>): T =
                    classContext.getBean(type.java)

            // override fun <T : Any> getInstance(type: KClass<T>, name: String): T=
            //     guice.getInstance(name, type.java)
        }
    }
}

And on an irrelevant note, would you guys happen to know what version actually supports ClassXmlApplicationContext?

      <dependency>
            <groupId>org.springframework</groupId>
            <artifactId>spring-context-support</artifactId>
            <version>2.5.6</version>
       </dependency>

Any feedback on initial ideas for a design would be great! I'll push this up and open an pull request in case anyone wants to mess with the structure. I just went with a way, but not necessarily a great one.

Romanow88 commented 5 years ago

Do I understand correctly that you want a way so that tornado components can live in the DI container if it's provided, or use internal container per default?

Or this just about providing default implementations for popular DI frameworks (in the base jar or in companion jars)?

AlexCzar commented 5 years ago

@Romanow88 there is a discussion in slack discussing both topics, this issue as a result is about both. Regarding the second point, it'll most probably be in companion libs, as we don't want TornadoFX itself to depend on frameworks it doesn't need. @ahinchman1 I think in case of Spring we want to use BeanFactory rather than a concrete *Context, as we don't know which one is being configured.

ahinchman1 commented 5 years ago

Many apologies, I finally have time after my talk from earlier this month. Burned out for a bit too. I'm looking at this this weekend. I'll be carrying this conversation over on slack today for brainstorming and feedback.

Ah - @AlexCzar that's a really good point; you'll have to excuse me, as I work with Beans a lot less and context a lot more.

ahinchman1 commented 5 years ago

@AlexCzar so I went with application context because asides from Bean instantiation/wiring, it also provides features like:

Would these extra features end up boxing folks in as opposed to just using the BeanFactory?

AlexCzar commented 5 years ago

@ahinchman1 There are different contexts and we don't want to either implement it for all of them or be selective, so since all of them implement BeanFactory it's the common denominator we should hook onto.

ahinchman1 commented 5 years ago

Update: In Google Guice, you can actually pass in an array of modules. This is a problem for us the way we're going about it right now because in Kotlin we cannot use 2 varargs as parameters:

class TornadoFXGuice(override val primaryView: KClass<out UIComponent> =
        NoPrimaryViewSpecified::class,
                     vararg stylesheet: KClass<out Stylesheet>,
                     vararg module: <out AbstractModule>) : App() {

But this isn't necessarily bad because we can take advantage of TornadoFX dependency injection and create a controller to pass in the modules instead.

ahinchman1 commented 5 years ago

For Spring: It also turns out XmlBeanFactory class is deprecated, so I went with using ClassPathXmlApplicationContext instead.