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

add a portable event corresponding to application startup #496

Closed
Ladicek opened this issue May 24, 2021 · 8 comments · Fixed by #543
Closed

add a portable event corresponding to application startup #496

Ladicek opened this issue May 24, 2021 · 8 comments · Fixed by #543
Assignees
Labels
spec-feature An issue requesting an addition specification

Comments

@Ladicek
Copy link
Contributor

Ladicek commented May 24, 2021

CDI doesn't provide an event that would correspond to application startup. A commonly used substitute is the @Initialized(ApplicationScoped.class) Object event, but that is fired "when the application context is initialized" (quoting from the spec).

Traditionally, those two situations occur reasonably close in time, but with CDI Lite, we aim to expand the set of possible CDI implementation strategies to include those that initialize a lot of stuff (including the application context) ahead of time. In such case, the @Initialized(ApplicationScoped.class) Object event has no correlation to application startup. It may happen during application startup, or it may happen arbitrary number of hours before.

Hence, I believe CDI should provide an event type that specifically corresponds to application startup. In some ways, it would be nice to restrict it to only be fired after the application fully starts up and everything is initialized, but that has prohibitive runtime costs; even the @Initialized(ApplicationScoped.class) Object event doesn't guarantee that (and firing the new event at the same time as this "old" event should be a valid implementation strategy for "purely runtime" CDI implementations such as Weld). So I propose to add an event type that is fired synchronously during application startup.

As for the name, I originally thought it could be called AfterStartup, because that is symmetric to existing BeforeShutdown, but that reasoning is invalid. BeforeShutdown is a container lifecycle event that is designed for use with Portable Extensions (the spec directly says "observer methods of these events must belong to extensions"). Also, in previous paragraph, I specifically argue we shouldn't restrict the event to "after startup"; it should be "during startup". However, DuringStartup looks ugly.

There's also zero precedent (in CDI) for using naming conventions such as On[Something] or [Something]Event, so I'd like to avoid those. The common event naming strategy in CDI is to just use a descriptive name.

That would, in this case, be simply Startup. However, I'd like to reserve this name for a potential @Startup annotation which would, similarly to EJB's @Startup, force eager initialization of a bean.

Any suggestions? :-)

For symmetry, we may want to declare a corresponding "during shutdown" event. We already have the @BeforeDestroyed(ApplicationScoped.class) Object and @Destroyed(ApplicationScoped.class) Object events, and it's hard to imagine how these event types could be non-portable, so this is certainly not required. As I said, my main reason for adding such event type would be symmetry with the "during startup" event. Maybe also better discoverability (the application context events are, in my experience, not exactly obvious).

@Ladicek Ladicek added the spec-feature An issue requesting an addition specification label May 24, 2021
@mkouba
Copy link
Contributor

mkouba commented May 24, 2021

I think that it would also make a lot of sense to specify some SPI (could be even CDI-based) to integrate with other technologies so that it's possible to fire this event when the app is ready, e.g. when the app is ready to accept http requests.

@manovotn
Copy link
Contributor

Just a remark - we can do this two ways. It can either be an event of type Object and with qualifiers (just like the @Initialized is used) or we can specify the payload type and have no qualifiers. Personally, I like the latter more and I think that is what you propose here, correct?

As for name, I actually like Startup as event name or maybe StartupEvent.
Other than that, since it is a CDI container status, we could prefix the name with CDI/Cdi to make sure this is understood as CDI status.

That would, in this case, be simply Startup. However, I'd like to reserve this name for a potential @startup annotation which would, similarly to EJB's @startup, force eager initialization of a bean.

And as a side note - I wouldn't use Startup as eager init annotation because that would then introduce an issue with two equally named annotations (with same purpose even) which have a reasonable chance to be present on classpath simultaneously.

I think that it would also make a lot of sense to specify some SPI (could be even CDI-based) to integrate with other technologies so that it's possible to fire this event when the app is ready, e.g. when the app is ready to accept http requests.

Not sure what you meant here. If it is just a plain CDI event, then you can fire it from just about anywhere and anytime you know all your libraries/fkws are ready (as an integrator). But maybe I am missing something?

@mkouba
Copy link
Contributor

mkouba commented May 31, 2021

I think that it would also make a lot of sense to specify some SPI (could be even CDI-based) to integrate with other technologies so that it's possible to fire this event when the app is ready, e.g. when the app is ready to accept http requests.

Not sure what you meant here. If it is just a plain CDI event, then you can fire it from just about anywhere and anytime you know all your libraries/fkws are ready (as an integrator). But maybe I am missing something?

My point is that you might need some portable SPI to allow extension authors to participate in the "startup sequence", otherwise they would have to implement some runtime-specific API, e.g. io.quarkus.deployment.builditem.ServiceStartBuildItem in Quarkus.

@Ladicek
Copy link
Contributor Author

Ladicek commented May 31, 2021

Just a remark - we can do this two ways. It can either be an event of type Object and with qualifiers (just like the @Initialized is used) or we can specify the payload type and have no qualifiers. Personally, I like the latter more and I think that is what you propose here, correct?

Correct. I wouldn't mind [too much] doing a qualifier with Object payload, but I think a meaningful payload type without qualifiers is a lot more intuitive.

As for name, I actually like Startup as event name or maybe StartupEvent.
Other than that, since it is a CDI container status, we could prefix the name with CDI/Cdi to make sure this is understood as CDI status.

I think I'd prefer if it was defined in terms of application status, not CDI container status (see what happened with @Initialized(ApplicationScoped.class) Object :-) ).

That would, in this case, be simply Startup. However, I'd like to reserve this name for a potential @startup annotation which would, similarly to EJB's @startup, force eager initialization of a bean.

And as a side note - I wouldn't use Startup as eager init annotation because that would then introduce an issue with two equally named annotations (with same purpose even) which have a reasonable chance to be present on classpath simultaneously.

That is a good point. If we called the event Startup, that would also lead to a conflict, but possibly less troublesome, as the event type wouldn't be an annotation.

I'd be fine with that.

@Emily-Jiang
Copy link
Contributor

Emily-Jiang commented Jun 1, 2021

Since this event is after startup and before shutdown, I think @afterstartup and @BeforeShutdown make sense. By the way, these events are only fired at runtime, right? I think the payload should contain the app name or CDI container name.

@manovotn
Copy link
Contributor

manovotn commented Jun 1, 2021

By the way, these events are only fired at runtime, right?

That's the point of this issue - to have an event that really only happens when the app is actually running.

I think the payload should contain the app name or CDI container name.

CDI container might work but I am not sure it has any value.
App name, not really. I don't think you'd know the app name in many cases.
Either way, the payload can be just an "empty" class, it doesn't really matter. The point of the event is that it gives you a hook into a certain container state.

@Emily-Jiang
Copy link
Contributor

CDI container might work but I am not sure it has any value.
App name, not really. I don't think you'd know the app name in many cases.
Either way, the payload can be just an "empty" class, it doesn't really matter. The point of the event is that it gives you a hook into a certain container state.

I think it would be nice to find out which CDI container is up especially in Jakarta EE environment (if there is starting up order issue etc)

@Ladicek
Copy link
Contributor Author

Ladicek commented Jun 1, 2021

Since this event is after startup and before shutdown, I think @afterstartup and @BeforeShutdown make sense.

We already have @BeforeShutdown, but it's a lifecycle event that can only be observed by extensions. I'm not sure how much work it would be to generalize that -- it might be simple, or not.

Also, even though I originally wanted to call it @AfterStartup, I'm now against that, because the name implies much stronger lifecycle guarantees that we can currently make. (I believe that aliasing this event to @Initialized(ApplicationScoped.class) Object should be a valid implementation strategy for "purely runtime" implementations. That, however, isn't after startup, that's during startup.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec-feature An issue requesting an addition specification
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants