Injection overrides

[realdadfish]

Going down the rabbit hole to provide easy testing of Android Activitys and Fragments (see https://gist.github.com/realdadfish/4c65e2da781bebb1479bc4d4624c5fb7) I found myself in the pity position that I cannot override Magnet's use of an annotated factory to create an instance.

While the factory should be used when no specific instance is bound in the scope, I want to be able to explicitly bind a specific (mocked) instance into a scope for testing purposes:

val rootScope = Magnet.createRootScope()
rootScope.bind(MyViewModel::class.java, mock<MyViewModel>())

At first I thought this could not work because I annotated the implementation itself:

@Instance(
    type = MyViewModel::class,
    factory = ViewModelFactory::class,
    scoping = Scoping.UNSCOPED
)
class MyViewModel : ViewModel() { ... }

so I extracted it like so:

// Jetpack's ViewModel of course has no interface :(
abstract class MyViewModel : ViewModel() { ... }

@Instance(
    type = MyViewModel::class,
    factory = ViewModelFactory::class,
    scoping = Scoping.UNSCOPED
)
class MyViewModelImpl : MyViewModel() { ... }

but still, Magnet would again use the annotated factory to create an instance of the object, instead of just using the one that I supplied to it.

How can I make Magnet accept my mocked instance?

[realdadfish]

Hrm... looking at MagnetScope I have the feeling that it might be as easy as to remove the `if (factory == null)´ condition...? What would be the consequences of that?

    @Nullable
    @SuppressWarnings("unchecked")
    private <T> T findOrInjectOptional(
        @NotNull Class<T> objectType,
        @NotNull String classifier,
        @Nullable InstanceFactory<T> factory,
        byte cardinality
    ) {
        @NotNull InstantiationContext instantiationContext = this.instantiationContext.get();
        @NotNull String key = key(objectType, classifier);

        InstanceBucket<T> deepInstance = findDeepInstanceBucket(key, factory);
        if (factory == null) {
            if (deepInstance == null) {
                if (cardinality == CARDINALITY_SINGLE) {
                    throw new IllegalStateException(
                        String.format(
                            "Instance of type '%s' (classifier: '%s') was not found in scopes.",
                            objectType.getName(), classifier));
                }
                return null;
            }
            instantiationContext.onDependencyFound(deepInstance.getScopeDepth());
            return deepInstance.getSingleInstance();
        }
       ...
   }

[realdadfish]

Ready further there I see there is a lot more going on - somehow I think I need to suppress the call to T object = factory.create(this); when there is a deepInstance available, but right now that happens unconditionally. Don't know, you're the expert here :)

[sergejsha]

Hey Alice, you went too deep already 😉

Let me give you some context on this topic first. With Magnet I wanted to avoid the nightmare I've seen in almost each and every project where test/mocked/faked/stubbed injections were setup. Then I realized that when I decompose the code good enough, there is actually no need for any injection in unit tests.

A simple pattern like the one below applied to most classes makes the test injection useless. The classes can be easily tested using mocked objects given in constructor.

interface BookRepository {
    fun loadBook(bookId: String): Single<Book>
}

@Instance(type = BookRepository::class)
internal class DefaultBookRepository(
    private val booksApi: BooksApi,
    private val booksCache: BooksCache
    ...
) : BookRepository {
    override fun loadBook(bookId: String): Single<Book> { ... }
}

It's framework classes causing issues for testing in most the cases. Keeping those classes (Application, Activity, Fragment, ViewModel etc) empty helped to solve most those issues. The logic should always (when possible) be implemented outside of those classes and the classes should rather work as proxies with no logic in them. Then the testing will be fun and no mocked injection will be really required.

Now back to your question. I suspect you would want to avoid configuring alternative scopes with mixed (real + mocked) instances just because its maintenance is a nightmare. But it looks there is a case where you cannot avoid it. What is this case exactly? Why can't you stay with pure mocking unit testing described above? Having more insides will help me to understand the issue and maybe to come out with an easy solution for it.

[realdadfish]

Have a look at the linked Gist. I introduced a ScopeModel that holds and returns the current scope instance across restarts. I then introduced a way to replace the scope in the framework base classes, now all I need is a way for Magnet to allow me to stub this testScope with my own mocked dependencies... Am 2. März 2019 09:53:20 MEZ schrieb Sergej Shafarenka <notifications@github.com>:

…

Hey Alice, you went too deep already 😉 > > Let me give you some context on this topic first. With Magnet I wanted to avoid the nightmare I've seen in almost each and every project where test/mocked/faked/stubbed injections were setup. Then I realized that when I decompose the code good enough, there is actually no need for any injection in unit tests.> > A simple pattern like the one below applied to most classes makes the test injection useless. The classes can be easily tested using mocked objects given in constructor.> > ```kotlin> interface BookRepository {> fun loadBook(bookId: String): Single<Book>> }> > @instance(type = BookRepository::class)> internal class DefaultBookRepository(> private val booksApi: BooksApi,> private val booksCache: BooksCache> ...> ) : BookRepository {> override fun loadBook(bookId: String): Single<Book> { ... }> }> ```> > It's framework classes causing issues for testing in most the cases. Keeping those classes (`Application`, `Activity`, `Fragment`, `ViewModel` etc) empty helped to solve most those issues. The logic should always (when possible) be implemented outside of those classes and the classes should rather work as proxies with no logic in them. Then the testing will be fun and no mocked injection will be really required.> > Now back to your question. I suspect you would want to avoid configuring alternative scopes with mixed (real + mocked) instances just because its maintenance is a nightmare. But it looks there is a case where you cannot avoid it. What is this case exactly? Why can't you stay with pure mocking unit testing described above? Having more insides will help me to understand the issue and maybe to come out with an easy solution for it.> > -- > You are receiving this because you authored the thread.> Reply to this email directly or view it on GitHub:> #72 (comment)

-- Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.

[realdadfish]

And, in case it isn't obvious, I want to mock out ViewModel instances so I can test my UI in isolation :) Am 2. März 2019 09:53:20 MEZ schrieb Sergej Shafarenka <notifications@github.com>:

…

Hey Alice, you went too deep already 😉 > > Let me give you some context on this topic first. With Magnet I wanted to avoid the nightmare I've seen in almost each and every project where test/mocked/faked/stubbed injections were setup. Then I realized that when I decompose the code good enough, there is actually no need for any injection in unit tests.> > A simple pattern like the one below applied to most classes makes the test injection useless. The classes can be easily tested using mocked objects given in constructor.> > ```kotlin> interface BookRepository {> fun loadBook(bookId: String): Single<Book>> }> > @instance(type = BookRepository::class)> internal class DefaultBookRepository(> private val booksApi: BooksApi,> private val booksCache: BooksCache> ...> ) : BookRepository {> override fun loadBook(bookId: String): Single<Book> { ... }> }> ```> > It's framework classes causing issues for testing in most the cases. Keeping those classes (`Application`, `Activity`, `Fragment`, `ViewModel` etc) empty helped to solve most those issues. The logic should always (when possible) be implemented outside of those classes and the classes should rather work as proxies with no logic in them. Then the testing will be fun and no mocked injection will be really required.> > Now back to your question. I suspect you would want to avoid configuring alternative scopes with mixed (real + mocked) instances just because its maintenance is a nightmare. But it looks there is a case where you cannot avoid it. What is this case exactly? Why can't you stay with pure mocking unit testing described above? Having more insides will help me to understand the issue and maybe to come out with an easy solution for it.> > -- > You are receiving this because you authored the thread.> Reply to this email directly or view it on GitHub:> #72 (comment)

-- Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.

[sergejsha]

What it there was a @TestInstance annotation, which generates a "preferred" factory if both @Instance and @TestInstance are available in classpath?

[realdadfish]

Thinking about it, it just occurred to me that there is an even simpler solution: Instead of providing a real Scope instance, I could simply provide a mocked one that directly feeds the needed things into my UI! Sometimes it helps talking about things :) Am 2. März 2019 10:30:47 MEZ schrieb Sergej Shafarenka <notifications@github.com>:

…

What it there was a ***@***.***` annotation, which generates a "preferred" factory if both ***@***.***` and ***@***.***` are available in classpath? -- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: #72 (comment)

-- Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.

[sergejsha]

Nice, that could be a good fit for you. You might also think of creating a single mocked root Scope (e.g. in TestApp) and then return itself from createSubscope() method. This should allow having a single "flat" mocked scope at one place.

[realdadfish]

Yep! Closing this therefor.

[realdadfish]

Coming back to this, I guess the @TestInstance annotation might be a good idea actually. The "new" use case is that we want to add BDD acceptance tests to the app and these tests naturally test things not always in isolation, but have a more integrative character. In the end all you really want is to mock out / replace the storage / API layer and leave the rest wired as-is.

[sergejsha]

I have been thinking about this after our discussion and I see some disadvantages of having @TestInstance annotation like inability to exclude some instance in tests configuration, inability to have two or more different test instances of same type for two or more different tests and there can be even more.

In my opinion a solution should be dealing with a scope which can intercept instance creation, ideally at factory level, and replace instances with mocks. I have no ideal solution for now but I could think of an extension to the solution you already implemented.

Here is the MockableScope capable of providing mock instances from given MockedInstanceProvider. It's enough to wrap your original root scope usually created in the App with this mocked one and it will work out down the whole scope hierarchy. Each get-call will be passed to the provider first, and if provider doesn't return an instance, original scope will the queried.

Thus everything you want to mock should be returned from MockedInstanceProvider.

class MockableScope(
    private val origin: Scope,
    private val instanceProvider: MockedInstanceProvider
) : Scope {

    override fun <T : Any> getMany(type: Class<T>, classifier: String): MutableList<T> =
        instanceProvider.getMany(type, classifier) ?: origin.getMany(type, classifier)

    override fun <T : Any> getMany(type: Class<T>): MutableList<T> =
        instanceProvider.getMany(type, Classifier.NONE) ?: origin.getMany(type)

    override fun <T : Any> getSingle(type: Class<T>, classifier: String): T =
        instanceProvider.getSingle(type, Classifier.NONE) ?: origin.getSingle(type)

    override fun <T : Any> getOptional(type: Class<T>, classifier: String): T? =
        instanceProvider.getOptional(type, classifier) ?: origin.getOptional(type, classifier)

    override fun createSubscope(): Scope =
        MockableScope(origin.createSubscope(), instanceProvider)
     ...
}

interface MockedInstanceProvider {
    fun <T : Any> getMany(type: Class<T>, classifier: String): MutableList<T>?
    fun <T : Any> getOptional(type: Class<T>, classifier: String): T?
    fun <T : Any> getSingle(type: Class<T>, classifier: String): T?
}

If you need to exclude some instances from been instantiated, you can add has<Cardinality>() methods to the provider and only request origin if corresponding method returns true.

Would some something like this work for you? If it works, we can think of adding it to the library in a more generic way later on.

[realdadfish]

Hrm... that sounds like a solution. There could then be a MockedInstanceProvider implementation for production that always returns null to keep the overhead low. Let's see how I can wire this in. Thanks!

[sergejsha]

I suspect, you don't really want to use MockableScope in production at all 😉 Use it for tests only. If you have a TestApp, you can create it there.

[sergejsha]

Just to make it more clear. The idea is to keep original scope hierarchy in production code unchanged and to wrap it with a mockable scope hierarchy in tests only. MockableScope implements decorator pattern and adds a lookup for mocked instances in given MockedInstanceProvider. It's enough to wrap original production root scope for making the whole hierarchy mockable, because MockableScope overrides createSubscope() method and returns mockable scopes for all subscopes.

Additional notes on MockedInstanceProvider. In contrast to production scopes, provider has a flat hierarchy and it ignores (or can ignore) scoping attribute. Depending on your scope/instances configuration you might need to return the same mocked instance or a new instance from its getters.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.