Ensures that our MP Config CDI extension processes only enabled injection points #708
Conversation
…tion points Signed-off-by: Laird Nelson <[email protected]>
… fix Spotbugs complaint Signed-off-by: Laird Nelson <[email protected]>
…om the GUI is a no-op) since the last run timed out spuriously Signed-off-by: Laird Nelson <[email protected]>
…rying from the GUI is a no-op) since the last run had a gRPC test failure (unrelated) Signed-off-by: Laird Nelson <[email protected]>
| private void harvestConfigPropertyInjectionPointsFromEnabledBean(@Observes ProcessBean<?> event) { | ||
| Bean<?> bean = event.getBean(); | ||
| Set<InjectionPoint> beanInjectionPoints = bean.getInjectionPoints(); | ||
| if (beanInjectionPoints != null && !beanInjectionPoints.isEmpty()) { |
There was a problem hiding this comment.
Mostly to satisfy my own curiosity... There are several null-checks in this method. The JavaDoc for the invoked methods does not say that null might be returned (although it doesn't say null will not ever be returned either although one could infer that). Are you just being really careful or have you seen these methods return null in your past experience with CDI?
There was a problem hiding this comment.
Mostly being careful. I've seen NullPointerExceptions from non-market-leader CDI implementations in odd places and it has scarred me. Also, a Bean in particular is an interface that an end user may implement, so she may return null from a variety of its methods and the specification does not forbid her from doing so. So yes, being careful.
This PR ensures that
@ConfigProperty-annotated injection points belonging only to enabled beans are processed. This is a partial fix for #650. Subsequent PRs will continue to fix the other multiple issues surfaced by that issue. The TCK continues to pass.Other minor changes:
public(i.e. are intended only for the CDI container to call and that should not be part of our public supported subclassable API) have been marked@DeprecatedSigned-off-by: Laird Nelson [email protected]