Open wilkinsona opened 7 years ago
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.
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?
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.
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).
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.
Doesn't seem to take integration tests with @SpringBootTest
into account as well… It's a good point!
+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.
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.
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).
Sounds like this might be turning into a documentation issue
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.
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
Spring Framework should do that itself then (easy to detect instances of its own interface being registered as @Configuration
).
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.
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.
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.
+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.
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)
}
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.
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.
Thanks a lot @sdeleuze I dived into this already and I like it very much.
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. :)
@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.
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?
Hi,
Would something like this make it work?
/**
* DSL for inline usage, like,
* ```
* springApplication.beans {
* bean<MyBean>()
* }
* ```
*/
fun SpringApplication.beans(init: BeanDefinitionDsl.() -> Unit) {
addInitializers(org.springframework.context.support.beans(init))
}
/**
* DSL for external usage, like
* ```
* springApplication.beans = beansMethod()
* ```
*/
inline var SpringApplication.beans: BeanDefinitionDsl
get() { throw UnsupportedOperationException("Write-only DSL") }
set(value) { addInitializers(value) }
Along with #10511 changes that should make it possible to have
runSpringApplication<MyApplication> {
beans {
bean<MyBean>()
}
}
I think you can already write:
fun main(args: Array<String>) {
runApplication<MyApplication>(*args) {
addInitializers(
beans {
bean<MyBean>()
})
}
}
The issue is that it won't be taken in account for tests.
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?
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.
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));
}
}
Fixing this means we'll also probably want to change the default context to not be an AnnotationConfigApplicationContext
. See #14590 for details.
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.
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
.
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?
@daliborfilus I think you'll have to use @SpringBootTest
to use the DSL for bean registration.
@wilkinsona I thought so. It's not a big problem, really. Thank you for corfimation.
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.
Is this still being considered? Thanks
@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.
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.
I have published a way of solving this problem. Based on Spring-Fu, I incorporated some additional functionality that I needed to implement this approach at my day job, which allows it to be used within existing Autoconfiguration-based applications. Take a look: https://github.com/wakingrufus/spring-funk A good place to start to read more is here: https://wakingrufus.github.io/spring-funk/introduction.html Feel free to reach out to me @wakingrufus
on the kotlin slack, or open a discussion on https://github.com/wakingrufus/spring-funk/discussions I welcome feedback. This is a project I am willing to work on long term if it has traction on making the functional approach to Spring Boot more accessible and mainstream
SpringApplication
makes it hard to get hold of the application context before it's refreshed. @michael-simons came up with this: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.