Skip to content
This repository has been archived by the owner on Feb 26, 2023. It is now read-only.

Add support for bean configurations #1520

Open
AleksanderMielczarek opened this issue Aug 14, 2015 · 13 comments
Open

Add support for bean configurations #1520

AleksanderMielczarek opened this issue Aug 14, 2015 · 13 comments

Comments

@AleksanderMielczarek
Copy link

Sometimes there is a need to inject objects, which are from outside current project. In this case no clean solution is available now. I think about Configuration classes with @EConfiguration. Those classes will be singletons only. To distinguish how to inject Bean, new field configuration could be added to @bean.

@EConfiguration
public class Configuration {

    @EBean
    public Foo getFoo() {
        return new Foo();
    }

    @EBean(scope = EBean.Scope.Singleton)
    public Bar getBar(@Bean(configuration = Configuration.class) Foo foo) {
        return new Bar(foo);
    }
}

@EBean
public class Baz {

    @Bean(configuration = Configuration.class)
    protected Foo foo;

    @Bean(configuration = Configuration.class)
    protected Bar bar;

}

@Retention(RetentionPolicy.CLASS)
@Target(ElementType.TYPE)
public @interface EConfiguration {

}

@Retention(RetentionPolicy.CLASS)
@Target({ElementType.TYPE, ElementType.METHOD})
public @interface EBean {

    public enum Scope {
        Default, Singleton
    }

    Scope scope() default Scope.Default;

}

@Retention(RetentionPolicy.CLASS)
@Target({ElementType.FIELD, ElementType.PARAMETER})
public @interface Bean {

    Class<?> value() default Void.class;

    Class<?> configuration() default Void.class;

}
@WonderCsabo
Copy link
Member

Our bean injection is really simple and therefore not flexible. There are already several requests to enhance this. And even there is an implementation with a provider pattern. Please search the existing issues.

@dodgex
Copy link
Member

dodgex commented Aug 14, 2015

Sounds like an interesting idea. Maybe this could be more intelligent by looking at all available configuration classes for @Beans that do not have a classic @ebean.

This could be a powerfull Feature.

@WonderCsabo @yDelouis what do you think?

@yDelouis
Copy link
Contributor

I agree we should provide a way to configure the injection. It would be very useful for testing.
However, the proposition does not seem okay to me :

  1. @EConfiguration seems to vague to me. I would prefer @EInjector or simply @Injector.
  2. How do you match a @Bean annotated field with the right method of the @EConfiguration ?
  3. We shouldn't use @EBean for two different meaning (declaring a type is enhanced and annotating a method). It won't fit our architecture and makes thing difficult to understand. By the way, what does @EBean mean on methods of @EConfiguration annotated class ?

@dodgex
Copy link
Member

dodgex commented Aug 15, 2015

@yDelouis

  1. I think the Name is inspired by spring framework @configuration
  2. Process @EConfiguration classes first and keep the Info about the available beans. When injecting @bean Check if there is a matching @ebean and if not Check beans provided by the configuration.
  3. Means the same as @bean in spring: this method provides a bean. (and yeah. We should get a new annotation for that)

@WonderCsabo
Copy link
Member

To be honest, i do not like extending our beans too much. The current bean architecture only allows very-very limited configuration. Implementing proper dependency injection with all the fancy things is really not trivial, and i am not sure we have to do that. A jack of all trades would be nice integration with Dagger for instance.

BTW, we already have an implementation which allows specifying how to create the injected beans. Maybe we can use it.

@dodgex
Copy link
Member

dodgex commented Sep 2, 2015

@WonderCsabo i can understand your concerns, but actually i think this feature would be awesome.

i have at least one case where this would be usefull for my app that i develop in my freetime and both app in my company have a "BeanFactory" @EBean with multiple getter methods that get/create beans that are somewhat complex to instantiate. with this feature we would not need to always inject the BeanFactory and in (e.g.)@AfterInject set the class fields ourself. instead we could just let them be inject by AA.

also i think that this should be relativly simple to imlement.

  • process @EConfiguration classes first and keep the info about the available/provided beans.
  • when injecting @bean check if there is a matching @ebean
  • if no matching @ebean is found, check beans provided by the configuration.

@smaugho
Copy link
Contributor

smaugho commented Feb 15, 2019

On this issue, @dodgex sees the same benefits mentioned on #2200 , though simply "processing @EConfiguration classes first and keep the info about the available/provided beans" would mean to have to store this information somewhere else (since if kept on memory, and incremental processing takes place, there is no guarantee that the @EConfiguration classes would be processed and registered).

@WonderCsabo , @dodgex having a ".androidannotations" folder holding this kind of information could be definitely a solution to this issue. I know it adds complexity, but it permits to implement advanced features, including injection of interfaces without any further configuration (which is simply a "powerful" feature, and relies just on the "ability" of AA to record annotated classes). It would permit to keep track of all the @EConfiguration classes, or all the @EBean classes to permit injections without knowing the exact class (ie. by implemented interfaces).

The process for the generation of a ".androidannoitations" folder which could contain an androidannotations.xml holding this information probably should be something like:

  1. Check if the file exists, if so, ensure that each of the declared classes exists (this should be done before starting the processing, and it is important to check it, because the user could have removed a class which was declared before), if they don't exist, remove them from the file (and keep the file in "memory")

  2. On injections like @Bean check this file (or better its memory representation) to determine the best injection for a class (I think that conflicts should be signaled, right now no needed resolution of conflicts, though could be a future feature).

  3. Inject using the best match (including the @EConfiguration classes.

  4. And the end of the processing, save the changes in memory to the configuration file.

BTW, I do know that libs like Dagger could handle this part, but I don't see that value in "forcing" AA users to use Dagger, for something that is in fact no that complex to add to AA.

@WonderCsabo
Copy link
Member

@smaugho please remember that AA is not only wants to support incremental compilation, but also is highly modularized and support plugins. Introducing some holder file which tries to retain information can only be implemented in the core processor, and somehow should be passed down to handlers. I still think implementing it would add too much complexity, and would be error prone.

@smaugho
Copy link
Contributor

smaugho commented Feb 17, 2019

@WonderCsabo , so for having this functionality on AA, then you're recommendation is instead go with Dagger? Maybe we should build a plugin for AA and Dagger, which in fact simplifies the usage of Dagger (which is complicated for many people). But even for doing that, we'll need some mechanism to know the existing @EBean annotated classes in the system (for instance, to create a Dagger module automatically for all the @ebean annotated on the system).

BTW, if plugins would be able to add behaviors to existing annotations, that would be really good for this kind of plugins, or at least be notify of other annotations.. For instance, a Dagger plugin, could automatically create a module for all the Beans on the system, and initialize a component for injections in the @eApplication (or each activity if it doesn't exist). For doing so, it should be notified at least of annotations like @ebean or @EActivity... but right now if we try to do this in a plugin, it doesn't work at all.. since only one handler for the same annotation is selected (I think).

@WonderCsabo
Copy link
Member

Iny my project, i use AAs @EBeans for basic helper where i can inject context and resources. For complex things, i use Dagger, since it have far more ability for DI, and @EBean was never intended to provide that much functionality. It is pretty easy to use AA and Dagger together, see the wiki.

Yes, only one handler is used for an annotation. Changing this would cause problems, for example which is the order for the handlers to process the same thing, etc.

@smaugho
Copy link
Contributor

smaugho commented Feb 17, 2019

For multiple handles, we could simply make a light version. It could work "aggregating" info.

Let's see, right now to make this in AA we simply "force" the user to add another annotation. So if I have a Plugin A and Plugin B, which adds annotations to enrich @EBean we should do something like:

@PluginAAnnotation
@PluginBAnnotation
@EBean
public void SomeBeanEnrichedWithPlugins {

}

So far, there isn't any control to determine in which order others plugins will "aggregate" their information.

So what I'm saying (for aggregation), it is to simplify that, plugin devs don't need to create an extra annotation (and users don't have to learn a new one). They simply add another Handler for @EBean which will do what the @PluginAAnnotation or @PluginBAnnotation annotation would do.

So, if somebody builds a Dagger plugin, which should recollect all the @EBeans, it doesn't have to force the user to place over each Bean a @DaggerDependency annotation, it simply will have to create another handler for the @EBean. The user adds the plugin to his gradle file, and everything works!

So I'm not sure if the order in which the handlers is called changes this fact, since right now it doesn't care about it neither when adding multiple annotations.

@dodgex
Copy link
Member

dodgex commented Feb 18, 2019

  1. Would it be nice to have such feature?

Hell yeah.

  1. Is this something AA has been built for or is aimed at?

Not at all.

  1. Is it easy to refactor this?

I'm afraid not. We would have to refactor way to much to make it worth. And the risk of breaking stuff is way to high.

@dodgex
Copy link
Member

dodgex commented Feb 18, 2019

In my opinion we shoud reduce complexity of the project instead of increasing it even more.

Our Processors are not built to be extensible by plugins. I think that even the plugin system itself is not the best solution. It works but i think it should have be done diffrent. But again, that would have required even more refactoring as the plugin system in its current form took.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants