Skip to main content
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [cdi-dev] Lifecycle events in Extension
Had to do a bit of archeology work to answer this one :)

The test is a copy from original Weld testsuite (back in something like 2012), even the TCK test states that.
Now, in Weld, this was originally added as part of first tests for CDI-96 (ability to programmatically alter bean metadata via extensions) and the test was slightly different.
Shortly after, an issue appeared which identified that Weld (WELD-1047) incorrectly fired the event for non-enabled beans (what we discussed earlier in this thread) and the test got changed.
However, the change only fixed the assertions in the test class (that the two events are null), but never removed the excess code from the extension - link to code changes.
And this then got copied over to TCK as it was - if you browse TCK history, you can see that there was no actual change to the test and it verified the same thing since its addition.

So the excess code can be removed and the test can just verify that no such event was observed.

Matej

On Fri, Feb 24, 2023 at 7:11 PM Jean-Louis Monteiro <jlmonteiro@xxxxxxxxxxxxx> wrote:
Thanks Matej for the useful and detailed response.

The thing that got me lost is that in the observer methods of the test you pointed it, we can see for alpha
if (!types.contains(Bravo.class) && !types.contains(Charlie.class)) {

or for bravo
if (!types.contains(Charlie.class)) {

This is misleading and that's why I was probably mistaken.
Out of curiosity, what was the intent of such a check if only one event type can be observed?


On Wed, Feb 22, 2023 at 6:34 PM Matej Novotny <manovotn@xxxxxxxxxx> wrote:
You are right that Charlie also has Bravo and Alpha types and your logic would work if you fired an event of type Charlie and had observers that expect Charlie/Bravo/Alpha.
However, with lifecycle events, we are talking parameterized types and your actual event type is PBA<Charlie>, and that won't be assignable to observed type PBA<Alpha> (or PBA<Bravo> respectively).

A simpler way to imagine it would be using something well known such as List<Integer> and List<Number>.
Imagine you have an event type List<Integer> and an observer that expects List<Number>; upon firing an event, should this observer be notified?
I don't think so because in plain Java, you cannot perform such assignment either.

If you want a parameterized observer that gets notified for subtypes, you can make use of wildcards (which the spec covers as well, see link below) - @Observes ProcessAnnotatedType<? extends Foo> pat

In specification, you correctly pointed to the resolution rules, but take a look at the parameterized type assignability section; just beware, it's not an easy read for sure - https://jakarta.ee/specifications/cdi/4.0/jakarta-cdi-spec-4.0.html#observers_assignability
The most relevant part for this should be:

> the observed event type parameter is an actual type with identical raw type to the event type parameter, and, if the type is parameterized, the event type parameter is assignable to the observed event type parameter according to these rules, or

I guess in the TCK, there have to be some other tests that verify assignability rules for events but the first one I could quickly scavenge is ParameterizedEventTest.
Not quite the same (and tests other complex cases) but might give you an idea that this isn't a made-up thing for lifecycle observers only :)

Matej

On Wed, Feb 22, 2023 at 4:54 PM Jean-Louis Monteiro <jlmonteiro@xxxxxxxxxxxxx> wrote:

I debugged Weld because my understanding was that the 3 observer methods would be notified by the PBA<Charlie> event because it matches the 3 observers. But even though Charly has the 3 types Alpha, Bravo and Charlie, only charlie() observer method is notified. In OpenWebBeans on the other hand, the 3 methods get notified and that's why there is a check to make sure we only capture events for the specific type we want.

It seems in line with https://jakarta.ee/specifications/cdi/4.0/jakarta-cdi-spec-4.0.html#observer_resolution from which I would expect the 3 methods to be notified with the single event PBA<Charlie>

I'm guessing there is something I'm missing somewhere. Can you clarify this?




On Wed, Feb 22, 2023 at 2:58 PM Matej Novotny <manovotn@xxxxxxxxxx> wrote:
It asserts exactly what you are asking about, there is an almost identical hierarchy of classes and only the last one (Charlie) should get PBA.

Regards
Matej

On Wed, Feb 22, 2023 at 2:52 PM Matej Novotny <manovotn@xxxxxxxxxx> wrote:
If you specialize the bean, it isn't enabled anymore, see https://jakarta.ee/specifications/cdi/4.0/jakarta-cdi-spec-4.0.html#enablement_full
So (using your example) you'd only fire PBA for C.

On Wed, Feb 22, 2023 at 12:01 PM Jean-Louis Monteiro <jlmonteiro@xxxxxxxxxxxxx> wrote:
I do find an issue in this situation

@Foo public class A {}
@Bar @Specializes
public class B extends A {}

@Baz @Specializes
public class C extends B {}

Let's keep it simple. No veto during PBA processing.
From what you are saying, there are 3 enabled beans right, so we should see 3 events: one for A, one for B and one for C. Am I correct?




On Tue, Feb 21, 2023 at 6:20 PM Matej Novotny <manovotn@xxxxxxxxxx> wrote:
> The "enabled beans" before the "fire PBA" is also to clarify. From what you said in your first reply. It's only for enabled beans when we are processing the PBA events. If after the event is being processed, the bean moves from enabled to not enabled state, then that's ok. This is why we trigger PBA with a veto() method. Am I correct?

Yes, IIRC the PBA is for all beans that, at the time of firing, you deem enabled.
It is also the last point in which you can decide to disable the bean by calling veto().
Note that detecting enabled beans can also be tricky because you can have alternatives, decorators and interceptors that don't use Priority annotation but instead rely on per-archive enablement via beans.xml.

Matej


On Tue, Feb 21, 2023 at 4:22 PM Jean-Louis Monteiro <jlmonteiro@xxxxxxxxxxxxx> wrote:
I created this thread because I needed more information on the use case, and what the test is meant to do and assert.

If I would be more or less sure I would have created an issue as a challenge. I don't have a strong opinion yet. I do believe we could probably improve the wording maybe in the spec for end users. As stupid as I am, if I can't have a strong opinion reading it, maybe there are more in the world in the same situation.

So I really appreciate the feedback and the fact we have all different readings of the spec shows again we could clarify the spec maybe.

I'd agree with you Matej that even though history and previous discussions are important to understand, we are at CDI 4 and I'm working on the related TCK. If there is something wrong, the past does not really matter. What's important is the current version of the TCK with the current version of the spec. Are they in agreement? Is there one more restrictive than the other? Are they some non-rewritten assertions? etc.

I also agree with you Matej that for single beans, the test and the spec are ok.
Reading the spec, there is the question I asked at the beginning when there are multiple beans, but this is not tested at the moment.

For producers, it was not clear because the type of the events in the @Observes are different and involve the 2 producer methods in the class.

The "enabled beans" before the "fire PBA" is also to clarify. From what you said in your first reply. It's only for enabled beans when we are processing the PBA events. If after the event is being processed, the bean moves from enabled to not enabled state, then that's ok. This is why we trigger PBA with a veto() method. Am I correct?

On Tue, Feb 21, 2023 at 2:54 PM Matej Novotny <manovotn@xxxxxxxxxx> wrote:
Looking more closely at the test, there isn't even anything that would assert the ordering of events for multiple beans/producers, so whether you fire 'all events for one type' or 'one event for all types' isn't being challenged - or am I missing something Jean-Louis? And I can understand that due to this not being ever tested, the impls can diverge and that's probably fine.

But the ordering of events for a single bean are IMO pretty clear in the specification (as was acknowledged in the TCK issue as well) - and this is what the test effectively asserts.

On Tue, Feb 21, 2023 at 2:08 PM Matej Novotny <manovotn@xxxxxxxxxx> wrote:
Using the 1.0 version of specification to justify behavior tested in 4.0 seems a bit off. After all, 1.0 was in 2013 (give or take a year?).
Plenty of changes, adjustments, clarifications and additional test coverage got into the spec in the meantime during those 10 years.

But even rolling with that idea, the wording in 1.0 is very similar (can be seen here) but obviously more barebones because that was the initial state of things.

> The original specification defined some strictly sequential lifecycle events (e.g BeforeBeanDiscovery clearly must be called before AfterDeploymentValidation). And other parts where the original spec indicated a type by type processing (ProcessInjectionPoint, etc).

This is still specified in the spec today, see this chapter and it isn't what the TCK test in question asserts; it targets per-bean discovery events.
ProcessInjectionPoint is an interesting example for ordering, particularly because it doesn't seem to exist in 1.0 spec and only appears in 1.1 released later (which makes using 1.0 as an argument for 4.0 behavior even weirder?)
For sure it isn't mentioned in either Chapter 11 or 12 of CDI 1.0.

> A _new_ test in the TCK suite now assumes a different order it seems. But this is not backed by the spec.

As far as CDI 1.0 spec goes it indeed isn't indeed.
The issue for the test is CDITCK-590 (which is now browseable again) and it was added some time after TCK 2.0 (and CDI 2.0).
So this test has been _new_ for the last 5+ years (Jun 2017) and last time I checked, adding test coverage wasn't a crime either.

> There is even an ancient old CDI ticket about this problem. (Which likely got  bulk closed like so many other valid tickets in both the spec and tck.)

The issue in question is linked to the TCK ticket above and I also mentioned it in the previous email.
Issues (all CDI JIRA issues) got bulk closed not because I'd just love the way it feels to slam the issue but because the project migrated under Jakarta which requires their projects to use GH for issue tracking.
That's also what those comments indicated, why are those issues still accessible and why they prompted users to re-create the ticket under GH if still applicable.
An example of such a comment/closure is here and there was also a GH issue tracking the whole process on the Jakarta side, link here.

Regards
Matej

On Mon, Feb 20, 2023 at 2:26 PM Mark Struberg <struberg@xxxxxxxxxx> wrote:
To understand how it is meant you have to go way back to the CDI-1.0 and 1.1 specifications. Because over there it was way more clearer how it was originally intended:

-------
12.2 Application initialization lifecycle [initialization]
When an application is started, the container performs the following steps:
• First, the container must search for service providers for the service javax.enterprise.inject.spi.Extension defined in Section 11.5, instantiate a single instance of each service provider, and search the service provider class for observer methods of initialization events.
• Next, the container must fire an event of type BeforeBeanDiscovery, as defined in Section 11.5.1.
 Contexts and Dependency Injection for the Java EE platform 112 / 114
 • Next, the container must perform type discovery. Additionally, for every type discovered the container must fire an event of type ProcessAnnotatedType, as defined in Section 11.5.6.
• Next, the container must fire an event of type AfterTypeDiscovery, as defined in Section 11.5.2.
• Next, the container must perform bean discovery, and abort initialization of the application if any definition errors exist, as defined in Section 2.9. Additionally, for every Java EE component class supporting injection that may be instantiated by the container at runtime, the container must create an InjectionTarget for the class, as defined in Section 11.2, and fire an event of type ProcessInjectionTarget, as defined in Section 11.5.8.
• Next, the container must fire an event of type AfterBeanDiscovery, as defined in Section 11.5.3, and abort initialization of the application if any observer registers a definition error.
• Next, the container must detect deployment problems by validating bean dependencies and specialization and abort initializa- tion of the application if any deployment problems exist, as defined in Section 2.9.
• Next, the container must fire an event of type AfterDeploymentValidation, as defined in Section 11.5.4, and abort initialization of the application if any observer registers a deployment problem.
• Finally, the container begins directing requests to the application.

-------

This wording got moved around and spread out and is now cluttered with maybe parts of the original intent gone missing.

The original specification defined some strictly sequential lifecycle events (e.g BeforeBeanDiscovery clearly must be called before AfterDeploymentValidation). And other parts where the original spec indicated a type by type processing (ProcessInjectionPoint, etc).

Think as a programmer. If you have a language (kind of python-like) which looks like

for every discovered class:
* fire ProcessBeanAttributes
* fire ProcessInjectionPoint
* fire ProcessInjectionTarget
Then it really means the later option you mentioned. And that's what we find in the spec to this day. Even if it's now cluttered into different places all over the spec paper.. A _new_ test in the TCK suite now assumes a different order it seems. But this is not backed by the spec.


> Not sure I understand, you can have a disabled bean before you get to PBA.

There is even an ancient old CDI ticket about this problem. (Which likely got  bulk closed like so many other valid tickets in both the spec and tck.)
The point is that the spec declares that PBA must ONLY be fired for enabled beans. But by changing the BeanAttributes etc it can essentially become not-enabled.

LieGrue
strub


Am 15.02.2023 um 16:35 schrieb Jean-Louis Monteiro <jlmonteiro@xxxxxxxxxxxxx>:

Hi all,

Decided to ask before creating a challenge because I'm not sure about this one, so I really need your expertise to understand.

The TCK in question is org.jboss.cdi.tck.tests.full.extensions.beanDiscovery.event.ordering.LifecycleEventOrderingTest

The goal is to test the sequence of "Bean Discovery Events". I'm referring to the following https://jakarta.ee/specifications/cdi/4.0/jakarta-cdi-spec-4.0.html#initialization_full which then says the container must perform bean discovery --> https://jakarta.ee/specifications/cdi/4.0/jakarta-cdi-spec-4.0.html#bean_discovery_steps_full

I do have some understanding issues. I apologize in advance if questions appear to be stupid.

The spec start with "For every type in the set of discovered types (as defined in Type discovery in CDI Full), the container must:"

If I simply it with a simple example, does it mean we have to fire first all event A for all types, then all events B for all types, then all events C for all types

OR

Does it mean, for bean 1, fire event A, then B, then C, then move to bean 2 and fire event A then B, then C.

The test above seems to be implemented from a bean perspective so it may not have an impact.

My other question is when the spec says "if the class is an enabled bean, interceptor or decorator, fire an event of type ProcessBeanAttributes, as defined in ProcessBeanAttributes event, and then"

I'm wondering how that is effectively possible. My understanding is that you can only decide whether a bean is enabled or not AFTER you fired ProcessBeanAttribute. Shouldn't the PBA always be fired (as soon as the bean is not vetoed of course)? If not then it gets complicated from a user perspective when you specialize a bean.


The producer side seems to be mixing events from different types or am I wrong?
For example, we are observing `ProcessProducerMethod<HighQualityAndLowCostProduct, PoorWorker>` ppm but we are on the other hand observing `ProcessInjectionPoint<PoorWorker, String> pip`

We have 2 producers in this class and I'm wondering what we are actually testing (hence the first question related to the scope, per bean, or global)

Thanks in advance

_______________________________________________
cdi-dev mailing list
cdi-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/cdi-dev

_______________________________________________
cdi-dev mailing list
cdi-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/cdi-dev
_______________________________________________
cdi-dev mailing list
cdi-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/cdi-dev
_______________________________________________
cdi-dev mailing list
cdi-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/cdi-dev
_______________________________________________
cdi-dev mailing list
cdi-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/cdi-dev
_______________________________________________
cdi-dev mailing list
cdi-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/cdi-dev
_______________________________________________
cdi-dev mailing list
cdi-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/cdi-dev
_______________________________________________
cdi-dev mailing list
cdi-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/cdi-dev
_______________________________________________
cdi-dev mailing list
cdi-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/cdi-dev
_______________________________________________
cdi-dev mailing list
cdi-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/cdi-dev

Back to the top