Skip to content
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

Expose the functional bean registration API via SpringApplication #8115

Open
wilkinsona opened this issue Jan 26, 2017 · 39 comments
Open

Expose the functional bean registration API via SpringApplication #8115

wilkinsona opened this issue Jan 26, 2017 · 39 comments
Labels
theme: performance Issues related to general performance type: enhancement A general enhancement

Comments

@wilkinsona
Copy link
Member

SpringApplication makes it hard to get hold of the application context before it's refreshed. @michael-simons came up with this:

SpringApplication springApplication = new SpringApplication(Application.class);
springApplication.addInitializers((GenericApplicationContext ctx) -> {
    ctx.registerBean(Greeter.class, Greeter::new);
});
springApplication.run(args);

It would be nice if the initialiser wasn't needed and the API provided an easier way to access the context before it's refreshed.

@wilkinsona wilkinsona added priority: normal status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Jan 26, 2017
@michael-simons
Copy link
Contributor

Background was (again) creating an example for my book how to use Spring 5 enhancements out of the box with Spring Boot. I assume that this question will arise, especially with the functional router.

@dsyer
Copy link
Member

dsyer commented Jan 26, 2017

way to access the context before it's refreshed

Isn't that the definition of an ApplicationContextInitializer? How could the API be any simpler without duplicating?

@michael-simons
Copy link
Contributor

I - as a user - would have been looking for something like

springApplication
    .registerBean(Greeter.class, Greeter::new);
    .registerBean(Foo.class, () -> new WhateverFoo());

and having Boot delegate to an Initializer for me.

@dsyer
Copy link
Member

dsyer commented Jan 26, 2017

As a user I'd expect to have something that worked in integration tests with @SpringBootTest as well as when I run my main method (changing the API of SpringApplication doesn't help there).

@wilkinsona
Copy link
Member Author

wilkinsona commented Jan 26, 2017

How could the API be any simpler without duplicating?

I'm not sure, however the pure Spring example in the Kotlin blog post involves less ceremony:

GenericApplicationContext context = new GenericApplicationContext();
context.registerBean(Foo.class);
context.registerBean(Bar.class, () -> new 
	Bar(context.getBean(Foo.class))
);

It's a shame that SpringApplication gives you an extra hoop to jump through. Perhaps we'll have to live with that, but I think it's worth some thought.

@michael-simons
Copy link
Contributor

Doesn't seem to take integration tests with @SpringBootTest into account as well… It's a good point!

@sdeleuze
Copy link
Contributor

+1 for allowing using the functional bean registration API with Spring Boot, even if I am not sure yet exactly where should be the entry point. For the record, here is the Kotlin function bean registration I used before switching to the app to Boot. I found no proper wat to integrate with Boot so I had to remove it to use @Bean but I would be happy to restore it if you provide a way to do that easily.

If I remember well I had no way to integrate my functional bean registration at the right level in Spring Boot application context lifecycle even with @bclozel help.

@dsyer
Copy link
Member

dsyer commented Jan 26, 2017

Just to complete that loop: if you define the initializer above as a standalone class and declare it in spring.factories it will work with @SpringBootTest and in a simple one-line main() method. I see no reason why that wouldn't work with Kotlin if anyone cares.

@michael-simons
Copy link
Contributor

michael-simons commented Jan 26, 2017

I think that @dsyer's approach is ok, can also be used through context.initializer.classes, but if people see those nice examples, they forget about IT tests… (like I did).

@wilkinsona
Copy link
Member Author

Sounds like this might be turning into a documentation issue

@dsyer
Copy link
Member

dsyer commented Jan 26, 2017

What about an annotation instead of spring.factories, and a special component scan just for initializers and listeners? That's not even Spring 5 specific.

michael-simons added a commit to springbootbuch/extconfig that referenced this issue Jan 26, 2017
@philwebb
Copy link
Member

philwebb commented Jan 27, 2017

I don't like adding this directly to SpringApplication but perhaps we could promote a pattern more like this:

private class MyApp implements BeanRegistrar {

    @Override
    public void registerBeans(BeanRegistry registry) {
        registry.registerBean(Greeter.class, Greeter::new);
        registry.registerBean(Foo.class, () -> new WhateverFoo());
    }

    public static void main(String... args) {
        SpringApplication.run(MyApp.class);
    }

}

this assumes some new Spring Framework interface

@dsyer
Copy link
Member

dsyer commented Jan 28, 2017

Spring Framework should do that itself then (easy to detect instances of its own interface being registered as @Configuration).

@sdeleuze
Copy link
Contributor

While BeanRegistry/ BeanRegistrar like interfaces are maybe needed from Spring Framework to avoid exposing all GenericApplicationContext API, I tend to think it would be nice to avoid forcing @Configuration usage on user side for such use case.

Functional bean registration API and @Configuration + @Bean are to 2 ways to describe your beans, and while it should be possible to mix them (we will have since Spring Boot internals are based on @Configuration), I think users should keep the possibility to use only the lambda based variant in their codebase.

@philwebb
Copy link
Member

Dropping the annotation model entirely opens another can of worms. We'd need to consider component scanning, the JPA base package stuff and how to do excludes. We'd also need to think about testing.

I'm tempted to say we should park this until 2.0 is out.

@snicoll
Copy link
Member

snicoll commented Jan 31, 2017

If there isn't a massive usage of this new model in the coming months, I am convinced we should tackle this one post 2.0. We need more use cases to make it right.

@philwebb philwebb added the status: on-hold We can't start working on this issue yet label Mar 3, 2017
@philwebb philwebb removed the status: waiting-for-triage An issue we've not yet triaged label Apr 27, 2017
@sdeleuze
Copy link
Contributor

+1 for waiting post 2.0 for this, I am just adding a link to this Spring Framework 5 functional bean declaration blog post in Kotlin with a Consumer<GenericApplicationContext> style approach, since it could give food for thoughts for such support on Boot side.

SPR-13779 should also allow easier integration with Spring Boot 2.

@sdeleuze
Copy link
Contributor

sdeleuze commented Sep 4, 2017

FYI, I have updated Kotlin functional bean definition DSL to implement ApplicationContextInitializer so with Spring Framework 5 + Boot 2.0 you should be able to declare your beans like that (which is pretty nice IMO):

Beans.kt

fun beans() = beans {
    bean<Greeter>()
    bean<Foo> { WhateverFoo() }
    // ...
}

Application.kt

SpringApplication(Application::class.java).apply {
    addInitializers(beans())
    run(*args)
}

@sdeleuze
Copy link
Contributor

sdeleuze commented Sep 7, 2017

Since SPR-13779 has returned to 5.x backlog, I have given a new try to Boot + functional bean registration and have created a Boot 2.0 based branch of my spring-kotlin-functional project. It seems to work as expected.

The ApplicationContextInitializer approach seems to work for this basic use case, but unlike previous example have chosen to use an context.initializer.classes entry in the application.properties file, the big advantage compared to SpringApplication.addInitializers() being that the beans are taken in account also for tests. I have updated the related StackOverflow answer accordingly.

I guess the kind of experimental support we can provide with Spring Framework 5.0 and Spring Boot 2.0 for early adopters and people curious about experimenting on this.

@sdeleuze
Copy link
Contributor

sdeleuze commented Sep 29, 2017

I think https://github.com/tgirard12/spring-webflux-kotlin-dsl shows a very nice example of what could look like a functional WebFlux application written in Kotlin (not sure it apply to Java). It could give us some ideas for this issue.

@michael-simons
Copy link
Contributor

Thanks a lot @sdeleuze I dived into this already and I like it very much.

@benjishults
Copy link

One thing I like about java-config it the ability to make my configuration very navigable following a pattern like this. This allows me to split up my configuration and navigate the configuration easily.

I've been trying to do something like this using BeanDefinitionDsl and it's been more of a hassle than I expected. Are any of you playing with the possibility of pulling the equivalent of @import into BeanDefinitionDsl?

I have a feeling this isn't the perfect spot to be posting this question. :( I posted a kind-of-related question on stackoverflow which makes me feel better about commenting here. :)

@sdeleuze
Copy link
Contributor

@benjishults I have only found this question that I have just answered. Please post your other question to StackOverflow and I will answer you there.

@sdeleuze
Copy link
Contributor

sdeleuze commented Nov 1, 2017

After more thoughts, I think context.initializer.classes + an ApplicationContextInitializer class is ok in Java, but in Kotlin it require too much ceremony given the fact the bean { } DSL directly returns an ApplicationContextInitializer instance, see BeansInitializer.kt and Beans.kt in this new Spring Boot + Kotlin + functional bean definition branch I have created.

What would be awesome is to support top level function reference in Kotlin (conceptually similar to static method reference in Java), in order to be able to define context.initializer.classes=io.spring.deepdive.BeansKt.beans or to support that via an alternative context.initializer.foo if you think that's not suitable for the existing one.

That would allow to just remove the need for BeansInitializer.kt and leverage natively the fact that Kotlin bean DSL is an ApplicationContextInitializer.

Any thoughts?

@sdeleuze
Copy link
Contributor

sdeleuze commented May 14, 2018

If possible, I would like to move forward on this issue for 2.1 by providing a PR implementing this improvement if we can come to an agreement about what needs to be done.

Adding support for Kotlin top level functions references like com.example.ApplicationKt.beans via Kotlin reflection for initializers and listeners could avoid the creation of an artificial class like in my functional app boot branch or in @michael-simons test.

These properties could be defined via application.properties or @TestPropertySource(properties = ["context.initializer.classes = com.example.ApplicationKt.beans"])

I am not sure if we should reuse context.initializer.classes since we are more talking about a factory. Maybe in Spring this is usual enough to be able to specify a factory instead of class to keep existing name?

Last point, it would be super useful to be able to specify the initializer via an annotation attribute. So in my dreams, the improvement would allow to write something like bellow taken in account by application and tests:

@SpringBootApplication(initializer="com.example.ApplicationKt.beans") // Could be @SpringBootApplication(initializer="com.example.Initializer") to specify a class that would make that useful in Java as well
class Application

fun main(args: Array<String>) {
	runApplication<Application>(*args)
}

fun beans() = beans {
	bean<UserHandler>()
	// ...
}

Any thoughts?

@wilkinsona wilkinsona added the for: team-attention An issue we'd like other members of the team to review label May 16, 2018
@sdeleuze
Copy link
Contributor

After more thoughts and discussions on that topic, I think I have changed my mind and agree with @snicoll about the fact that adding more functional bean definition support to Boot raises a wide range of other questions. I plan to experiment on that to move forward, but I don't think this has to be a Spring Boot 2.1 topic anymore. IMO better to focus on clearly defined features like #8762 to polish existing scope than introducing dedicated support for something different to what people use in Boot currently.

@wilkinsona wilkinsona removed the for: team-attention An issue we'd like other members of the team to review label May 30, 2018
@dsyer
Copy link
Member

dsyer commented Jun 13, 2018

This issue was hijacked a bit by the Kotlin discussion. To set it back on course, here is a proof of concept for a way for Boot to support functional bean registration (along the lines @philwebb showed above): https://github.com/dsyer/spring-boot-initializer. Simple example:

@SpringBootApplication
public class InitializerApplication
		implements ApplicationContextInitializer<GenericApplicationContext> {

	public static void main(String[] args) {
		SpringApplication.run(InitializerApplication.class, args);
	}

	@Override
	public void initialize(GenericApplicationContext context) {
		context.registerBean(RouterFunction.class, this::userEndpoints);
	}

	private RouterFunction<?> userEndpoints() {
		return route(GET("/"), request -> ok().body(Mono.just("Hello"), String.class));
	}

}

@philwebb
Copy link
Member

philwebb commented Oct 1, 2018

Fixing this means we'll also probably want to change the default context to not be an AnnotationConfigApplicationContext. See #14590 for details.

@dsyer
Copy link
Member

dsyer commented Dec 19, 2018

What we need I think is a way to externalize and configure the BeanDefinitionLoader. I have found it useful in functional applications to disable it completely by overriding SpringApplication.load(), e.g.

	public static void main(String[] args) {
		new SpringApplication(InitApplication.class) {
			@Override
			protected void load(ApplicationContext context, Object[] sources) {
			};
		}.run(args);

Formalizing that as an option, or making the loader a strategy that can be manipulated, ideally through a listener so it can happen in integration tests for instance, would be ideal.

@dsyer
Copy link
Member

dsyer commented Dec 19, 2018

FWIW I have found it relatively easy to override the ApplicationContext type in a listener. E.g.

			WebApplicationType type = application.getWebApplicationType();
			Class<?> contextType = getApplicationContextType(application);
			if (type == WebApplicationType.NONE) {
				if (contextType == AnnotationConfigApplicationContext.class
						|| contextType == null) {
					application
							.setApplicationContextClass(GenericApplicationContext.class);
				}
			}
			else if (type == WebApplicationType.REACTIVE) {
				if (contextType == AnnotationConfigReactiveWebApplicationContext.class) {
					application.setApplicationContextClass(
							ReactiveWebServerApplicationContext.class);
				}
			}
			else if (type == WebApplicationType.SERVLET) {
				if (contextType == AnnotationConfigServletWebServerApplicationContext.class) {
					application.setApplicationContextClass(
							ServletWebServerApplicationContext.class);
				}
			}

This works in a listener for ApplicationEnvironmentPreparedEvent.

@daliborfilus
Copy link

daliborfilus commented Oct 23, 2020

I don't want to hijack this thread by something non-relevant, so please excuse me if it is.

I'm currently trying to migrate my application into BeanDefinitionDsl style and I stumbled upon this issue too.

Using runApplication(...) { beans() } doesn't work from tests, as some of you have already mentioned.

So I currently need to have an initializer class, which delegates into the functional DSL (not a big deal):

class BeansInitializer : ApplicationContextInitializer<GenericApplicationContext> {
    override fun initialize(context: GenericApplicationContext) {
        beans().initialize(context)
    }
}

where beans() is a your average fun beans() = beans { /* ... */ }.
Then I need to have context.initializer.classes=app.BeansInitializer in application.properties.

This works correctly in tests - without modifying them in any way.

However, tests annotated with @JsonTest doesn't work, because in that beans { /* ... */ }, I'm creating beans that use ref<NamedParameterJdbcTemplate>() and that of course isn't present in the @JsonTest context.

Is it somehow possible to know from ApplicationContextInitializer (like beans {} is) if I have full context loaded, or if I'm in the slice test? I tried to use if (!context.containsBean("namedParameterJdbcTemplate")) { beans().initialize(context) }, but this 1) doesn't work (maybe ordering issue or the bean is named differently?) and 2) is fragile - it would require updating every time I change bean definitions.

Or I have to use general @SpringBootTest if I want to use BeanDefinitionDSL?

@wilkinsona
Copy link
Member Author

@daliborfilus I think you'll have to use @SpringBootTest to use the DSL for bean registration.

@daliborfilus
Copy link

@wilkinsona I thought so. It's not a big problem, really. Thank you for corfimation.

@dsyer
Copy link
Member

dsyer commented Oct 25, 2020

If you were prepared to put some conditional logic around all the bean definitions in the DSL I think maybe you could get it to work. Probably not what you want though, and not the “house style” with Spring Fu.

@albertocavalcante
Copy link

Is this still being considered? Thanks

@wilkinsona
Copy link
Member Author

wilkinsona commented Mar 10, 2023

@albertocavalcante Yes. The issue would have been closed if that was not the case. We don't have any plans to work on it in the near future as we have other higher priority work on our plates. This is indicated by the issue being in the general backlog milestone.

@dsyer
Copy link
Member

dsyer commented Mar 10, 2023

FWIW there are already quite a few options for using functional bean definitions via SpringApplication (e.g. add an initializer) or normal @Configuration (e.g. provide a BeanDefinitionRegistryPostProcessor). Changes to Spring Boot could make that more prominent in the programming model, I suppose, but it wouldn't change the user configuration code much. Most of my comments above were directed to work towards providing a reflection-free alternative, but if you don't care about that you can still write functional bean definitions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: performance Issues related to general performance type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests