-
Notifications
You must be signed in to change notification settings - Fork 81
add @Eager
#911
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
base: main
Are you sure you want to change the base?
add @Eager
#911
Conversation
|
I'm sure my decision to only allow One open question I've got: what's the best package for the |
Azquelt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sure my decision to only allow
@Eageron@ApplicationScopedbeans will spark a controversy, but well, such is life. I actually originally specified it for all scopes, but decided against it for reasons mentioned in the PR description.
For example, eagerly initializing
@RequestScopedbeans typically only makes sense for some beans, depending on the nature of the request (such as the URL in case of an HTTP request). My personal opinion is that such conditions are best expressed in code, which is already possible, because all beans can declare observer methods, which may include arbitrary code.
This is an interesting case. A RequestScoped bean may be instantiated in any request scope, whether it's associated with an HTTP request or not.
If you wanted to conditionally instantiate a RequestScoped bean, I guess you could do that by observing @Initialized(RequestScoped.class) on a different bean, checking whether it's an HTTP request and if so using BeanContainer.unwrapClientProxy to instantiate the bean? That seems like an unusual case which @Eager doesn't express.
I think I'd have still made it apply to all scopes, even if there are some cases where it's not appropriate, but I don't feel too strongly about it. This covers the most common use case and if there are strong arguments for other scopes, we can allow it at a later time.
One open question I've got: what's the best package for the Eager annotation? I tentatively put it into
jakarta.enterprise.context, because I couldn't find a better place, but it still doesn't feel exactly right.
I think jakarta.enterprise.context makes sense, since eagerness is associated with the lifecycle of a context and it already contains Initialized and Destroyed. jakarta.enterprise.inject might be another option, alongside Stereotype and Alternative - other annotations which affect the definition of the bean.
| /** | ||
| * Marks the annotated bean as eagerly initialized. | ||
| * | ||
| * @since 5.0 | ||
| */ | ||
| @Target({ ElementType.TYPE, ElementType.METHOD, ElementType.FIELD }) | ||
| @Retention(RetentionPolicy.RUNTIME) | ||
| @Documented | ||
| public @interface Eager { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My gut feeling is that many more people read the javadoc than read the spec so I think we should include the following information here:
Eageris equivalent to observingStartupEagermay only be used onApplicationScopedbeansEagercan be applied to a managed bean class, a producer method or a producer field
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. I had a similar text to the spec in the javadoc, but decided against it, because it's relatively dense and thick. I'll add something lighter back.
Also, this reminds me I have to add support for synthetic beans -- I had it in mind originally, but forgot. I'll fix that too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I addressed this particular comment.
Locally, I have code that allows synthetic beans to be eagerly initialized, but I think I need to prototype an implementation at least to some extent to see whether the API I've got makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My gut feeling is that many more people read the javadoc than read the spec so I think we should include the following information here:
* `Eager` is equivalent to observing `Startup` * `Eager` may only be used on `ApplicationScoped` beans * `Eager` can be applied to a managed bean class, a producer method or a producer field
And the JavaDoc here confused me.
Contextual instance of an eagerly initialized bean is created at the same time
the {@link jakarta.enterprise.event.Startup Startup} event is fired.
If the eager is inverting the lazy proxy creation. I do not think it is equivalent to @Observes StarupEvent.
It should be related to how to handle the bean instantiation. Firstly creating an empty proxy like normal, then immediately execute more steps:
- Inject the dependencies
- Call the managed bean lifecycle
@PostConstructmethod.
In theory, any scoped beans could be operated like this. Even applicationScoped bean looks more reasonable.
In contrast to Spring Boot ApplicationReady event, the bean initialization does not depend on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the eager is inverting the lazy proxy creation. I do not think it is equivalent to @observes StarupEvent.
I am not sure I understand your comment but it is equivalent.
In order for CDI container to notify a bean of such an event, it first creates its instance which includes injecting into it and calling its post construct method. This is a shorthand for the same.
In theory, any scoped beans could be operated like this. Even applicationScoped bean looks more reasonable.
See the original CDI issue, it mentioned several variants but in the end we decided to start smaller and only define it for app scoped beans which are the most common usage we know/heard of.
FTR, you can achieve the same for any scope and with the addition of BeanContainer#unwrapClientProxy it should be fairly simple.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very much not true. As mentioned above, the only thing that @Eager does is forcing the contextual instance of the bean into existence early on. If there's another bean calling into the @Eager bean before that eager bean was initialized (most likely because the other bean is also @Eager and calls into the first bean from a @PostConstruct callback or something like that), the instance will be created during that call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is the nice thing about making an @Eager bean with a @PostConstruct method, vs observing Startup and doing any setup logic in the observer method; @PostConstruct is guaranteed to run before the bean is used, whereas observer methods are called in priority order.
All that @Eager does is make sure that an instance of the bean is created when the Startup event is fired.
Would it be clearer to say this?
A contextual instance of an eagerly initialized bean is created at the same time
the {@link jakarta.enterprise.event.Startup Startup} event is fired, if one does
not exist.
I can't decide if that detail is helpful to have in the Javadoc or just makes the sentence more confusing.
Edit: this might be better:
The contextual instance of an eagerly initialized bean is created at the same time
the {@link jakarta.enterprise.event.Startup Startup} event is fired, if it has not
already been created.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'd rather say
Contextual instance of an eagerly initialized bean is created
at the same timeno later than the {@link jakarta.enterprise.event.Startup Startup} event is fired.
Or maybe reword it more:
Eagerly initialized bean is guaranteed to be created at latest when the
Startupevent is fired.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, I think the CDI implementor has a mechanism that can track all @Eager beans and make sure all are initialized successfully, then fire the StartUpEvent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still don't understand the difference between "immediately before Startup" and "during Startup at priority Integer.MIN_VALUE", but sure, we can say "immediately before firing Startup".
Aside:
fire the
StartUpEvent
Please stop calling it StartupEvent or StartUpEvent, it's called Startup.
|
I had one question: if |
This very much sums up my stance on it as well except I feel rather neutral on applying this to all scopes. I know we also discussed this for singletons, is there a reason why you chose to leave those out now?
That's where I would put it as well. |
What other observers would you want to cover? |
|
I don't have a concrete case in mind. I just always think reductively. Maybe you want to log or do something cross-cutting around these implicit observer methods. Maybe there's some kind of general way to accomplish that, where eager initialization might fall out as a convenient side effect. |
Aside: it's a shortcut for
I was thinking about that, as you can see here: #835 (comment) Let me rephrase. We could allow declaring "observer shortcuts", which would be annotations meta-annotated with
@Retention(RUNTIME)
@Target(ANNOTATION_TYPE)
@Documented
public @interface Shortcut {
Class<?> value();
}An observer shortcut could be added on a class, which would mean the class declares an empty observer method. It could also be added on a method, which would mean the method is effectively an observer method with an unused event parameter. For example: @Shortcut(Startup.class)
// no qualifiers here
@Retention(RUNTIME)
@Target(TYPE, METHOD)
public @interface OnStartup {
}Open questions:
Specifically in case of If you like the idea of observer shortcuts, feel free to file another issue. If you have any answers to the open questions, feel free to include them as well :-) (My tentative ideas would be: we don't care about parameterized types, because users can always just declare an observer method directly. For sync/async, we could just add an annotation member |
The only reason is that the rest of CDI spec ignores I did think about adding something like:
But I didn't feel too strongly about it. |
|
I just rebased and pushed a 2nd WIP commit that shows my current thoughts about:
I'm currently prototyping an implementation to validate that the API makes sense. |
|
I rewrote the spec slightly, especially in the paragraph around when are eagerly initialized beans actually initialized. I realized the previous wording only worked for managed beans and not for producers, and when I rewrote it to also include producers, I figured it's way too long for a relatively simple thing. So I went another way: I kept the specification of the precise moment when should eager init happen, but the precise mechanism is just replaced with "obtain contextual instance", which is an operation specified elsewhere. |
|
I just squashed the 2 commits, because my prototype work indicates that the API is reasonable. I believe this is ready for final review and then merge. |
99a0d27 to
ff1c1ca
Compare
This annotation may be put on `@ApplicationScoped` beans to trigger eager initialization. This occurs no later than at the time the container fires the `Startup` event. Putting the annotation on a bean of any other normal scope or on `@Dependent` bean leads to definition error. Putting the annotation on a bean of any other pseudo-scope leads to non-portable behavior (allowing implementations to do the same for `@Singleton` beans). It is possible to relax this in the future, if we find suitable meaning. For example, eagerly initializing `@RequestScoped` beans typically only makes sense for _some_ beans, depending on the nature of the request (such as the URL in case of an HTTP request). My personal opinion is that such conditions are best expressed in code, but I'm open to ideas. `@Eager` also serves as a direct replacement of EJB `@Startup`, which also only makes sense in case of application-wide beans (EJB `@Singleton`).
|
Rebased and made a tiny change: the specification (as well as the javadoc) now only contains the following text as mandatory:
This text:
is no longer prefixed with "More precisely" -- it is now prefixed with "For example", clarifying that eager init at the very beginning of |
This annotation may be put on
@ApplicationScopedbeans to trigger eager initialization. This occurs at the same time as theStartupevent.Putting the annotation on a bean of any other scope leads to definition error. This may be relaxed in the future, if we find suitable meaning.
For example, eagerly initializing
@RequestScopedbeans typically only makes sense for some beans, depending on the nature of the request (such as the URL in case of an HTTP request). My personal opinion is that such conditions are best expressed in code, which is already possible, because all beans can declare observer methods, which may include arbitrary code.@Eageris a shortcut for such observer in the most common scenario. It also serves as a direct replacement of EJB@Startup, which also only makes sense in case of application-wide beans (EJB@Singleton).Fixes #835