add support for assisted inject

[ryuichi7]

This adds support for using the guice extension assistedInject.

Without this change, we can't use Prop instances in classes using assistedInject. The current visitor implementation does not scan the factory function assistedInject methods for injectionPoints.

This PR add support to create an injectionPoint for each factory method so that they can be properly scanned and have dynamic props injected.

Why do we need this change?

The current architecture of this lib is composed of a PropMappingVisitor which is an implementation of ElementVisitor. This visitor iterates over all the Elements of a module and for each element invokes a target visitor called InjectionPointExtractor which implements BindingTargetVisitor.

The goal of the InjectionPointExtractor is to extract the InjectionPoint of a given binding, so that the arguments can be inspected by the PropMappingVisitor and any named props are then explicitly bound.

Each override of the visit function in the BindingTargetVisitor is an implementation of Binding that returns a single injection point. i.e. (LinkedKeyBinding, ProviderKeyBinding, ProvidesMethodBinding, UntargettedBinding). They return injectors that will "new" up one implementation type.

This is not the case, however, with the AssistedInjectBinding. Using AssistedInject and, specifically, the FactoryModuleBuilder employs the factory pattern. Meaning that a given factory could potentially return multiple implementations for a class and multiple injection points.

For instance, a factory might look like this:

public interface OrderFactory {
    Payment create(Date startDate, Money amount);
    Shipment create(Customer customer, Item item);
    Receipt create(Payment payment, Shipment shipment);
 }

Where each factory function possesses its own implementation type.

The AssistedInjectBinding binding has an attribute called getAssistedMethods which returns a list of AssistedMethod instances from where we can extract the InjectionPoint for each method.

This, however, results in possibly multiple InjectionPoints being returned, which breaks the current return type of the visit function implementations for InjectionPointExtractor.

My goal was to introduce the least amount of change, which is why I decided to simply change the return type of the InjectionPointExtractor to allow for Binding implementations that may return multiple InjectionPoints.

I considered possibly creating a MultipleInjectionPointExtractor, but then I would need to inspect the Element type in the visit function of the PropMappingVisitor to decide which target visitor to invoke the binding with.

Note

This is a breaking change. In order to make things work as expected, we need to return an Iterable<InjectionPoint> from InjectionPointExtractor

[jsfr]

Regarding release see inline comments :).

I don't have too much at the moment in terms of the actual solution itself, however I'd like to understand better why we need a breaking change in the return of InjectionPointExtractor? I get one is currently introduced in this solution, but I don't understand why.

[bbi22]

LGTM 👍

[harrylevick]

I'd like to understand better why we need a breaking change in the return of InjectionPointExtractor? I get one is currently introduced in this solution, but I don't understand why.

@jsfr From my understanding, we have to return the Iterable from the new visitor method because of the way that the assisted inject binding works. We can take my implementation of the assisted inject factory for example:

interface RuleFactory {
    fun createAmountRule(receiptScoreRequest: ReceiptScoreRequest, amountLegacyRule: AmountLegacyRule): AmountRule
    fun createEmailDomainRule(receiptScoreRequest: ReceiptScoreRequest): EmailDomainRule
    fun createEmailLocalPartRule(receiptScoreRequest: ReceiptScoreRequest): EmailLocalPartRule
    ...
}

The factory here is used to new up multiple different classes which are all using assisted inject, and hence each separate class has its own InjectionPoint. However the AssistedInjectBinding comes from the way that we bind this factory:

install(FactoryModuleBuilder().build(RuleFactory::class.java))

So the relationship between Binding <-> InjectionPoint here is a one-to-many, whereas in other kinds of bindings which we previously accepted, the relationship was one-to-one.

We then have to follow up by matching the return type in the original visitor, even though the logic here has not changed.

[ryuichi7]

@jsfr Thanks for your feedback! I apologize I wasn't clearer in my original description. I have updated the PR description with a more in-depth overview of how/why I made the proposed changes.

[ryuichi7]

@jsfr I have made the requested changes to follow semantic-release. I think it would make sense to add details about how to properly add a feature to the README of this lib so that we can avoid confusion in the future. I will create a follow up PR once this is merged.

[jsfr]

Thank you for the context @ryuichi7, and agreed on the addition to the README.