Introduce observable CDI events representing container state. #543

manovotn · 2021-10-14T13:22:49Z

Fixes #496

api/src/main/java/jakarta/enterprise/util/Shutdown.java

api/src/main/java/jakarta/enterprise/util/Startup.java

Ladicek · 2021-10-14T13:36:07Z

Maybe move the event types to jakarta.enterprise.event?

spec/src/main/asciidoc/core/events.asciidoc

manovotn · 2021-10-15T10:56:41Z

Maybe move the event types to jakarta.enterprise.event?

Hmm, I thought about that location as well. Didn't have a strong preference to be fair, so yea, we can do that.

manovotn · 2021-10-15T11:21:54Z

As usual, thanks for your review @Ladicek. PR is now updated; all comments should be addressed and I have also moved the classes from util to event package.

manovotn · 2021-10-15T11:23:01Z

Also CC @mkouba as an FYI because you had some comments in the original issue (#496) and we discussed this recently.

spec/src/main/asciidoc/core/events.asciidoc

Ladicek · 2021-10-15T13:43:31Z

Otherwise LGTM.

Ladicek · 2021-10-16T15:59:15Z

Actually I just realized one thing: we should add a section about this to CDI Full where we specify the relationship of these events to container lifecycle events (that may only be declared on Portable Extensions), notably AfterDeploymentValidation vs Startup and BeforeShutdown vs. Shutdown.

Now I'm not sure what's the relationship between @Initialized(ApplicationScoped.class) and AfterDeploymentValidation, but I think Startup should probably be after AfterDeploymentValidation. Similarly, I'm not sure what's the relationship between @BeforeDestroyed(ApplicationScoped.class) and BeforeShutdown, but I think that Shutdown should be before BeforeShutdown. But that's a naive interpretation and probably needs more thinking.

manovotn · 2021-10-18T10:08:03Z

From specification on BeforeShutdown event:

The container must fire a final event after it has finished processing requests and destroyed all contexts.

In other words, this is a very late event where you cannot do anything with contexts and beans anymore. This itself IMO implies the ordering.

Similarly, there is following text for AfterDeploymentValidation:

The container must fire an event after it has validated that there are no deployment problems and before creating contexts or processing requests.

This is a very early event where you lack contexts, so even before @Initialized event for app context. Again, this IMO defined ordering sufficiently.

@Ladicek does the above make sense?

Ladicek · 2021-10-18T10:09:50Z

Ah ah very nice, so the existing specification already implies the ordering that I described. Great!

manovotn · 2021-10-18T10:20:09Z

Ah ah very nice, so the existing specification already implies the ordering that I described. Great!

Meaning we can merge this? :-)

Ladicek · 2021-10-18T10:23:24Z

spec/src/main/asciidoc/core/events.asciidoc

@@ -596,4 +596,33 @@ As specified in <<builtin_contexts>>, contexts associated with built-in normal s
 * Otherwise, if the observer method is any other kind of transactional observer method, it is called in an unspecified transaction context, but with the same lifecycle contexts as the transaction that just completed.
 * Otherwise, the observer method is called in the same transaction context and lifecycle contexts as the invocation of `Event.fire()` or `BeanManager.getEvent()`.

+[[observable_container_lifecycle_events]]
+
+=== Observable container lifecycle events


Since "Container lifecycle events" belong to Portable Extensions, should we call these differently? Maybe "Application lifecycle events"? The Packaging and Deployment chapter has sections "Application initialization lifecycle" and "Application shutdown lifecycle", which is a nice match. (These sections don't have to refer to these new event types, as you described.)

Well, I deliberately chose this name because it is under Events section and it actually describes container lifecycle because ATM we cannot guarantee the whole app lifecycle which IMO makes "Application initialization lifecycle" a bad choice.

If you really dislike current naming then how about Observable container state events?

Fair enough.

Ladicek · 2021-10-18T10:23:58Z

I thought we wanted @mkouba to also take a look? :-)

manovotn · 2021-10-19T14:11:14Z

I am going to merge this. @mkouba if you have any comments, please feel free to drop them here even after merge.

manovotn requested a review from Ladicek October 14, 2021 13:22