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

Make it possible to mark beans as unremovable via MP config. #10007

Merged
merged 1 commit into from
Jun 16, 2020

Conversation

manovotn
Copy link
Contributor

Fixes #9941

Since we already have UnremovableBeanBuildItem concept that builds custom Predicate<BeanInfo> exclusion formulas, I thought we could just add new config and register it in a similar way. The downside is that we cannot really reuse ArcProcessor#initClassPredicates() because that needs to produce Predicate<ClassInfo>.

But maybe I am just missing a way to simplify that? I don't mind implementing it differently, this just seemed as the easiest way to me :-)

Also, do we need to mind producers and synthetic beans? Because in such case a package/class based expression in config won't do the trick. Or we would need to state that class based expression applies also to any declared producers inside that class. @loicmathieu @mkouba WDYT?

@manovotn manovotn requested review from mkouba and loicmathieu June 15, 2020 09:58
@boring-cyborg boring-cyborg bot added area/arc Issue related to ARC (dependency injection) area/documentation labels Jun 15, 2020
@mkouba
Copy link
Contributor

mkouba commented Jun 15, 2020

But maybe I am just missing a way to simplify that?

I think that you should use a similar logic we have for selected alternatives, i.e. collect the class predicates and produce one removal exclusion that iterates over those predicates:

builder.addRemovalExclusion(new Predicate<BeanInfo>() {
   public boolean test(BeanInfo bean) {
      ClassInfo beanClass = bean.getBeanClass();
      // iterate over predicates and test the bean class
   }
}); 

Also, do we need to mind producers and synthetic beans?

#9941 (comment)

"We should also clarify that all beans discovered from the matching type will be unremovable, including all producers declared on that type."

I don't think it makes sense to try to handle synthetic beans.

Copy link
Contributor

@loicmathieu loicmathieu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
It's exactly what I expected ;)

@manovotn
Copy link
Contributor Author

@mkouba how about now?
I also added another test that should cover the producer case (e.g. that you can mark it unremovable based on its return type).

Copy link
Contributor

@mkouba mkouba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very good now!

@manovotn manovotn added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jun 15, 2020
@manovotn manovotn merged commit eea5103 into quarkusio:master Jun 16, 2020
@geoand geoand added this to the 1.6.0 - master milestone Jun 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/arc Issue related to ARC (dependency injection) area/documentation triage/waiting-for-ci Ready to merge when CI successfully finishes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allowing application to control bean removal
4 participants