Introduce observable CDI events representing container state. #543
Conversation
|
Maybe move the event types to |
Hmm, I thought about that location as well. Didn't have a strong preference to be fair, so yea, we can do that. |
manovotn
force-pushed
the
issue496
branch
2 times, most recently
from
a0abfd0
to
1b66775
Compare
|
As usual, thanks for your review @Ladicek. PR is now updated; all comments should be addressed and I have also moved the classes from |
|
Otherwise LGTM. |
|
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 Now I'm not sure what's the relationship between |
|
From specification on
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:
This is a very early event where you lack contexts, so even before @Ladicek does the above make sense? |
|
Ah ah very nice, so the existing specification already implies the ordering that I described. Great! |
Meaning we can merge this? :-) |
| @@ -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 | |||
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
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?
|
I thought we wanted @mkouba to also take a look? :-) |
|
I am going to merge this. @mkouba if you have any comments, please feel free to drop them here even after merge. |
Fixes #496