cdi icon indicating copy to clipboard operation
cdi copied to clipboard

Do we really need #veto() on PBA?

Open jeanouii opened this issue 2 years ago • 5 comments

We have discussed many times the veto() impact when there is specialization involved. We could also ask ourselves if it brings much value to have veto() in PBA.

jeanouii avatar Apr 25 '23 11:04 jeanouii

If we don't think there are many usecases, we could maybe deprecate the method for removal

jeanouii avatar Apr 25 '23 11:04 jeanouii

I believe this also applies to ProcessObserverMethod.veto(), though there's no such mess as with ProcessBeanAttributes.

Ladicek avatar Apr 25 '23 11:04 Ladicek

It also messes up with the events ordering as we have discussed. Makes the logic more complex because we have to re-evaluate in the context of specialized beans.

Also a PIP might not be valid so a user might have reacted to a PIP event and now it's not valid anymore, so we have to trigger a new PIP event.

jeanouii avatar Apr 25 '23 11:04 jeanouii

Also a PIP might not be valid so a user might have reacted to a PIP event and now it's not valid anymore, so we have to trigger a new PIP event.

Where is this rooted in the spec? As far I can I tell, the spec says you fire PIP for every managed bean, regardless or its enablement. It doesn't mention refiring PIP event either. If you react to PIP that gets removed, your changes will get discarded (or rather won't be used) as well.

EDIT: During the CDI meeting (Apr 25), we clarified that re-firing the PIP event was a typo and it was meant to be PBA.

I do agree that veto() makes the logic complex but I am not really sold on removal being the solution.

manovotn avatar Apr 25 '23 14:04 manovotn

Removing veto() would kill a feature that is now used by portable extensions: Replacing an existing bean by a slightly modified version of itself. For example: The SmallRye implementation of MicroProfile Config replaces all managed beans with the qualifier @ConfigProperties with a synthetic bean.

2nd example: Quarkus provides a mechanism for "default beans", i.e. default implementations that are only used if the application does not define beans of certain types(+qualifier). If one wants to implement this feature in a portable extension (instead of implementing your own CDI container like ArC), the default bean has be vetoed whenever a non-default bean is detected.

3rd, similar example: Quarkus also has a @LookupIfProperty/@LookupUnlessProperty that makes beans (un)available for lookup based on config property values at runtime. If one wants to implement this feature in a portable extension, one needs to ability to remove beans during container start-up.

JHahnHRO avatar Aug 14 '23 15:08 JHahnHRO