Getting interceptor bindings in standard way

[arjantijms]

As per the discussion here:

https://www.eclipse.org/lists/cdi-dev/msg00133.html

Essentially boils down to standardising the implementation specific key for this.

[Ladicek]

If only we could change the InvocationContext interface. Adding a method or two to obtain interceptor binding annotations would be quite a bit better.

[manovotn]

If only we could change the InvocationContext interface.

True, we could try that - it's too late for EE 10 anyway. But I think even the original solution is fine. This isn't a feature you need very commonly anyway.

[arjantijms]

The original solution is the key right?

I can change the invocation interface, but the Interceptors spec is technically still CDI independent.

We have at least 14 days for EE 10 btw.

[Ladicek]

Technically speaking, if we standardized the key, then we could add a few methods to InvocationContext for free, because they could be default. What's more important is, if we standardize the key, where do we do that? In CDI, or in Interceptors? If in Interceptors, then EJB also needs to comply.

[arjantijms]

You're right, so essentially what we have today is:

Set<Annotation> bindings = (Set<Annotation>) 
    invocationContext.getContextData().get("org.jboss.weld.interceptor.bindings");

We indeed should define it in the Interceptor spec. But nobody really wants to touch Enterprise Beans I guess. It technically doesn't have to comply, although it may. For instance, in Faces we defined some functionality that Facelets complied to but JSP just didn't. Nobody wanted to touch JSP anymore.

The method could be:

public interface InvocationContext {
    
   default Set<Annotation> getInterceptorBindings() {
       return emptySet(); // or null?
   }

[mkouba]

+1 for default Set<Annotation> getInterceptorBindings() { return emptySet(); } and the interceptors spec should state that this method returns an empty set if no interceptor bindings were used to associate the interceptors, i.e. for EJB-style association via @javax.interceptor.Interceptors.

[manovotn]

default Set<Annotation> getInterceptorBindings()

That sounds good. I am also +1 for empty set (rather than null) if no bindings are used as that will naturally fit into EJB binding style where an empty set is a correct return value.

[Ladicek]

My understanding is that EJB also has to implement the full Interceptors specification, just like CDI, so even though the idiomatic way of associating interceptor with components in CDI is through interceptor binding annotations and in EJB through @Interceptors, they both have to support both...

That said, I'd be fine with adding something like the following to InvocationContext:

/**
 * Returns the set of interceptor binding annotations used to associate interceptors with the target component.
 * Returns an empty set if all interceptors are associated with the target component using the {@link Interceptors @Interceptors} annotation.
 * <p>
 * No requirements exist on mutability or shareability of the returned set.
 * <p>
 * This method is not required to be implemented by EJB containers.
 * It must be implemented by CDI containers.
 *
 * @return the set of interceptor bindings, never {@code null}
 */
default Set<Annotation> getInterceptorBindings() {
    return Collections.emptySet();
}

/**
 * Returns the interceptor binding annotation of given type used to associate interceptors with the target component.
 * Returns {@code null} if the {@link #getInterceptorBindings() full set} of interceptor bindings does not contain an annotation of given type,
 * or if all interceptors are associated with the target component using the {@link Interceptors @Interceptors} annotation.
 * <p>
 * In case of repeatable interceptor bindings, {@link #getInterceptorBindings(Class)} should be used.
 * <p>
 * This method is not required to be implemented by EJB containers.
 * It must be implemented by CDI containers.
 *
 * @param annotationType type of the interceptor binding annotation, must not be {@code null}
 * @return the interceptor binding annotation of given type, may be {@code null}
 */
default <T extends Annotation> T getInterceptorBinding(Class<T> annotationType) {
    for (Annotation interceptorBinding : getInterceptorBindings()) {
        if (interceptorBinding.annotationType().equals(annotationType)) {
            return (T) annotation;
        }
    }
    return null;
}

/**
 * Returns the set of interceptor binding annotations of given type used to associate interceptors with the target component.
 * The result is typically a singleton set, unless repeatable interceptor bindings are used.
 * Returns an empty set if all interceptors are associated with the target component using the {@link Interceptors @Interceptors} annotation.
 * <p>
 * No requirements exist on mutability or shareability of the returned set.
 * <p>
 * This method is not required to be implemented by EJB containers.
 * It must be implemented by CDI containers.
 *
 * @param annotationType type of the interceptor binding annotation, must not be {@code null}
 * @return the set of interceptor bindings of given type, never {@code null}
 */
default <T extends Annotation> Set<T> getInterceptorBindings(Class<T> annotationType) {
    Set<T> result = new HashSet<>();
    for (Annotation interceptorBinding : getInterceptorBindings()) {
        if (interceptorBinding.annotationType().equals(annotationType)) {
            result.add((T) interceptorBinding);
        }
    }
    return result;
}

I understand that the Interceptors spec ignores the existence of repeatable annotations (because they didn't exist at that time), so I'd be fine with dropping the last method (and the paragraph mentioning repeatable interceptor bindings in the javadoc of the 2nd). The 2nd seems pretty useful to me.

[mkouba]

My understanding is that EJB also has to implement the full Interceptors specification, just like CDI, so even though the idiomatic way of associating interceptor with components in CDI is through interceptor binding annotations and in EJB through @Interceptors, they both have to support both...

Well, the thing is that EJB session beans are considered CDI beans anyway. I don't think the EJB impls implement CDI-style interceptors by themselves, instead they delegate to the CDI container.

This method is not required to be implemented by EJB containers. It must be implemented by CDI containers.

I would not include these sentences at all.

No requirements exist on mutability or shareability of the returned set

Hm, I'd prefer to say that the returned set is immutable. This wording does not help anyone. A client should not modify the set and the CDI implementor should either return a new set or make it immutable (to protect users from shooting in their own leg ;-).

[Ladicek]

Hmm, assuming that

The use of interceptors defined by means of the Interceptors annotation is required to be supported for Jakarta Enterprise Beans and Managed Bean components, including in the absence of CDI. When CDI is enabled, the use of interceptors defined both by means of interceptor binding annotations and by means of the Interceptors annotation is required to be supported for component classes that support injection

-- Jakarta Interceptors, chapter 1.3

actually means that interceptor bindings are always handled by CDI, then indeed this provision

This method is not required to be implemented by EJB containers. It must be implemented by CDI containers.

is unnecessary.

When it comes to mutability etc., I'd be fine with requiring the resulting sets to be immutable, for sure.

[arjantijms]

When it comes to mutability etc., I'd be fine with requiring the resulting sets to be immutable, for sure.

Okay, so I think that for the next release cycle (post EE 10), we have our work cut out then ;)

[njr-11]

It would be nice to see this added in Jakarta EE 11. We ran into some difficulty where we are needing to rely on the WELD-specific approach in our implementation of the Transactional interceptor to properly detect the information configured on the interceptor binding at class level.

[manovotn]

Yes, I think this is something we want to get in, but ideally via interceptors spec (change to InvocationContext). FTR, today's call should be focused on going through issues listed for future version of CDI and curating them in terms of which issues are still relevant and doable for next version.

[Ladicek]

Note to self: if we go forward with this, need to add tests to the CDI TCK for this.

[Ladicek]

Submitted https://github.com/jakartaee/interceptors/pull/99 for this.

[manovotn]

The linked interceptors PR was merged, I think we can close this and open a tracking issue for TCK coverage. Agreed @Ladicek?

EDIT: Mea culpa, we already have the PR! See https://github.com/jakartaee/cdi-tck/pull/479

[manovotn]

This is already resolved in the interceptors spec and TCK coverage has been merged as well; closing this issue.