jakartaee / cdi

CDI specification
Apache License 2.0
211 stars 78 forks source link

Add methods to BeanConfigurator for applying decorators #459

Open arjantijms opened 3 years ago

arjantijms commented 3 years ago

Currently when adding a Bean<T> using jakarta.enterprise.inject.spi.AfterBeanDiscovery.addBean() or jakarta.enterprise.inject.spi.AfterBeanDiscovery.addBean(Bean<?>) any decorators declared or programatically added for the types of the Bean<T> are not automatically applied.

Not only is this perhaps surprising, there actually is no portable way to achieve this behaviour. Someone adding a bean must resort to SPIs and implementation specific code. For instance Soteria uses the following code:

afterBeanDiscovery.addBean(
    decorator.decorateBean(authenticationMechanismBean, HttpAuthenticationMechanism.class, beanManager));

Source

Where decorator.decorateBean is an SPI, that for Weld uses code such as the following:

public class DecorableWeldBeanWrapper<T> extends RIBean<T> implements Bean<T>, PassivationCapable {

    private final Bean<T> bean;
    private final CurrentInjectionPoint currentInjectionPoint;
    private final boolean isProxyable;
    private Class<T> type;

    private List<Decorator<?>> decorators;
    private Class<T> proxyClass;
    private boolean proxyRequired;
    private boolean isPassivationCapableBean;
    private boolean isPassivationCapableDependency;

    public DecorableWeldBeanWrapper(Bean<T> bean, Class<T> type, BeanManagerImpl beanManager) {
        super(
            bean,
            new StringBeanIdentifier(BeanIdentifiers.forBuiltInBean(beanManager, type, null)),
            beanManager);

        this.bean = bean;
        this.type = type;
        this.currentInjectionPoint = beanManager.getServices().get(CurrentInjectionPoint.class);
        this.isProxyable = Proxies.isTypesProxyable(getTypes(), beanManager.getServices());
    }

    @Override
    public void initializeAfterBeanDiscovery() {
        decorators = beanManager.resolveDecorators(getTypes(), getQualifiers());

        if (!decorators.isEmpty()) {
            proxyClass = new ProxyFactory<T>(getBeanManager().getContextId(), getType(), getTypes(), this).getProxyClass();
        }
    }

    @Override
    protected void internalInitialize(BeanDeployerEnvironment environment) {
        proxyRequired = getScope() != null && isNormalScoped();
        isPassivationCapableBean = Serializable.class.isAssignableFrom(type);
        isPassivationCapableDependency = isNormalScoped() || (isDependent() && isPassivationCapableBean());
    }

    @Override
    public T create(CreationalContext<T> creationalContext) {
        T instance = bean.create(creationalContext);

        if (decorators.isEmpty()) {
            return instance;
        }

        return getOuterDelegate(this, instance, creationalContext, proxyClass, currentInjectionPoint.peek(), getBeanManager(), decorators);
    }

Source

This code is difficult to come up with, difficult to maintain and error prone. The user must also make sure the right companion SPI implementation is available on the class path.

To simplify this, I'd like to propose a method for BeanConfigurator to signal to the runtime that enabled decorators should be applied to the instance returned from the Bean.create (`Contextual.create') method, just like happens for beans which implement the bean types directly and are discovered by CDI.

For instance:

public void afterBean(final @Observes AfterBeanDiscovery afterBeanDiscovery) {
        afterBeanDiscovery
            .addBean()
            .scope(ApplicationScoped.class)
            .types(MyBean.class)
            .id("Created by " + CdiExtension.class)
            .enabledDecorators()
            .createWith(e -> new MyBeanImpl("Hi!"));
    }

Several variants could be considered, like e.g.:

manovotn commented 2 years ago

We need to take a look at whether we can lift the spec restriction of not applying decorators to producers and synth beans https://jakarta.ee/specifications/cdi/4.0/jakarta-cdi-spec-4.0.html#decorators If that doesn't work, we can at least take a look at having an API/SPI that will allow this explicitly for a given synth bean.

manovotn commented 11 months ago

As per discussion during last CDI mtg (Dec 5), I am closing this issue. It has been stale for three years now indicating that there isn't much interest nor are there any takers to submit proposal or impl drafts for this feature.

arjantijms commented 11 months ago

@manovotn can you re-open? I actually was working on a PR for this, but with the M1 deadline for security and such didn't had time to push it out yet.

This is still as important as before. Every spec (or library) that provides build-in beans via an Extension needs this, as those provided beans can not be decorated now. It seems there's no interest, but there is. People do run into this, but (almost) nobody understands that this issue is the root cause.

I think it's also somehow a testimony to us (as spec creators, including the other specs) failing to educate both our users and ourselves.

arjantijms commented 5 days ago

Thanks @starksm64