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

Do we really need #veto() on PBA? #668

Open
jeanouii opened this issue Apr 25, 2023 · 5 comments
Open

Do we really need #veto() on PBA? #668

jeanouii opened this issue Apr 25, 2023 · 5 comments

Comments

@jeanouii
Copy link

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
Copy link
Author

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

@Ladicek
Copy link
Contributor

Ladicek commented Apr 25, 2023

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

@jeanouii
Copy link
Author

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.

@manovotn
Copy link
Contributor

manovotn commented Apr 25, 2023

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.

@JHahnHRO
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants