Uploaded image for project: 'CDI Specification Issues'
  1. CDI Specification Issues
  2. CDI-584

Clarification needed on Set mutability returned by SPI

    • Icon: Clarification Clarification
    • Resolution: Obsolete
    • Icon: Major Major
    • None
    • None
    • Portable Extensions
    • None

      ProcessBean lifecycle event could be the occasion to add new Injection point to an existing bean, but when you call ProcessBean#getBean()#getInjectionPoints() the returned set is mutable in OWB and immutable in Weld.
      As the spec doesn't precise if the Set should be mutable or not, each impl made its own choice.
      The result is a lack of consistency between both impl and an interesting feature harder to do with Weld (work with AnnotatedType or create a custom bean to add injection point)

      More generally we should clarify mutability of each Set returned in the SPI. And check if it wouldn't be better to add method to life cycle event to provide a "temporary" mutable set (i.e. ProcessBean#getInjectionPoints()

            [CDI-584] Clarification needed on Set mutability returned by SPI

            The CDI project is part ofJakarta and uses GitHub issues as it's issue tracking system.
            Therefore, all issues in CDI JIRA project are being bulk-closed as described in this GitHub issue.

            If you feel like this particular issue deserves ongoing discussion, investigation or fixes in CDI/CDI TCK, please create a new issue under GitHub repository and include a link to this JIRA.

            For specification related question/issues, please use - https://github.com/eclipse-ee4j/cdi/issues
            For CDI TCK related questions/issues, please use - https://github.com/eclipse-ee4j/cdi-tck/issues

            Matěj Novotný added a comment - The CDI project is part ofJakarta and uses GitHub issues as it's issue tracking system. Therefore, all issues in CDI JIRA project are being bulk-closed as described in this GitHub issue . If you feel like this particular issue deserves ongoing discussion, investigation or fixes in CDI/CDI TCK, please create a new issue under GitHub repository and include a link to this JIRA. For specification related question/issues, please use - https://github.com/eclipse-ee4j/cdi/issues For CDI TCK related questions/issues, please use - https://github.com/eclipse-ee4j/cdi-tck/issues

            Good catch. Error was on purpose to check your attention ... Ok, bad version pasted.
            We should have

            Set<InjectionPoint> ips = bean.getInjectionPoints();
            

            I create the ticket for addition to ProcessBean

            Antoine Sabot-Durand (Inactive) added a comment - Good catch. Error was on purpose to check your attention ... Ok, bad version pasted. We should have Set<InjectionPoint> ips = bean.getInjectionPoints(); I create the ticket for addition to ProcessBean

            Martin Kouba added a comment - - edited

            I see, that's a good example. So something like ProcessBean#setBeanInjectionPoints() might be useful. By the way, the example wouldn't work because ips is not a reference to the original set of injection points (it only contains all the elements from the original set .

            Martin Kouba added a comment - - edited I see, that's a good example. So something like ProcessBean#setBeanInjectionPoints() might be useful. By the way, the example wouldn't work because ips is not a reference to the original set of injection points (it only contains all the elements from the original set .

            To that I could answer, "what's the use of ProcessBean if you can't do anything about the bean tied to the event" ?
            But I have an example. I was asked to build a prototype extension to create an injection point when a given annotation (@CacheContext) is found on a field. Here's the portable code I had to wrote to create the feature :

            Extension code is small but AnnotatedType and AnnotatedField impls are verbose and the whole is cumbersome to read and maintain (3 classes and 178 lines of code)

            If we only target OWB, the extension code is:

            package org.cdisandbox.autoinject.Extension;
            
            import org.cdisandbox.autoinject.CacheContext;
            import javax.enterprise.event.Observes;
            import javax.enterprise.inject.spi.*;
            import java.util.HashSet;
            import java.util.Set;
            
            /**
             * Extension looking for the @CacheContext annotation to transform it in an InjectionPoint
             */
            public class AutoInjectExtension implements Extension {
            
                private Set<AnnotatedType<?>> atWithCacheContext = new HashSet<>();
            
                /**
                 * This Observer captures all AnnotatedType containing @CacheContext
                 */
                public void CreateInjectionPoint(@Observes @WithAnnotations(CacheContext.class) ProcessAnnotatedType<?> pat) {
                    atWithCacheContext.add(pat.getAnnotatedType());
                }
            
                /**
                 * This observer add an injection point in all bean having @Cachecontext annotaion on some field
                 */
                public <T> void CheckBeansForCacheContext(@Observes ProcessManagedBean<T> pmb, BeanManager bm) {
                    AnnotatedType<T> beanAT = pmb.getAnnotatedBeanClass();
                    if(atWithCacheContext.contains(beanAT)) {
                        Bean<T> bean = pmb.getBean();
                        Set<InjectionPoint> ips = new HashSet(bean.getInjectionPoints());
                        for (AnnotatedField<? super T> f : beanAT.getFields()) {
                            if(f.isAnnotationPresent(CacheContext.class)) {
                                ips.add(bm.createInjectionPoint(f));
                            }
                        }
                    }
                }
            }
            

            One class, 42 lines.

            Antoine Sabot-Durand (Inactive) added a comment - - edited To that I could answer, "what's the use of ProcessBean if you can't do anything about the bean tied to the event" ? But I have an example. I was asked to build a prototype extension to create an injection point when a given annotation ( @CacheContext ) is found on a field. Here's the portable code I had to wrote to create the feature : An AnnotatedType implementation An AnnotatedField implementation and the Extension Extension code is small but AnnotatedType and AnnotatedField impls are verbose and the whole is cumbersome to read and maintain (3 classes and 178 lines of code) If we only target OWB, the extension code is: package org.cdisandbox.autoinject.Extension; import org.cdisandbox.autoinject.CacheContext; import javax.enterprise.event.Observes; import javax.enterprise.inject.spi.*; import java.util.HashSet; import java.util.Set; /** * Extension looking for the @CacheContext annotation to transform it in an InjectionPoint */ public class AutoInjectExtension implements Extension { private Set<AnnotatedType<?>> atWithCacheContext = new HashSet<>(); /** * This Observer captures all AnnotatedType containing @CacheContext */ public void CreateInjectionPoint(@Observes @WithAnnotations(CacheContext.class) ProcessAnnotatedType<?> pat) { atWithCacheContext.add(pat.getAnnotatedType()); } /** * This observer add an injection point in all bean having @Cachecontext annotaion on some field */ public <T> void CheckBeansForCacheContext(@Observes ProcessManagedBean<T> pmb, BeanManager bm) { AnnotatedType<T> beanAT = pmb.getAnnotatedBeanClass(); if (atWithCacheContext.contains(beanAT)) { Bean<T> bean = pmb.getBean(); Set<InjectionPoint> ips = new HashSet(bean.getInjectionPoints()); for (AnnotatedField<? super T> f : beanAT.getFields()) { if (f.isAnnotationPresent(CacheContext.class)) { ips.add(bm.createInjectionPoint(f)); } } } } } One class, 42 lines.

            I'm sorry to be so sceptical but why do I need to add an injection point to a bean at this phase? A simple example would be enough.

            Martin Kouba added a comment - I'm sorry to be so sceptical but why do I need to add an injection point to a bean at this phase? A simple example would be enough.

            mkouba@redhat.com agree with you, but, that means we should check if some Lifecycle events (like ProcessBean) could have new method to change the underlying meta-data (i.e ProcessBean#addInjectionPoint()).
            rmannibucau@gmail.com also agree. The idea is not to force OWB to change its behavior but to state that it's not portable. And we should provide standard way to perform these meta data mutation at the lifecycle event level (see my remark above).

            Antoine Sabot-Durand (Inactive) added a comment - mkouba@redhat.com agree with you, but, that means we should check if some Lifecycle events (like ProcessBean) could have new method to change the underlying meta-data (i.e ProcessBean#addInjectionPoint() ). rmannibucau@gmail.com also agree. The idea is not to force OWB to change its behavior but to state that it's not portable. And we should provide standard way to perform these meta data mutation at the lifecycle event level (see my remark above).

            Not sure in term of implementation but stating the user shouldnt modify it and it is not portable what happens if so sounds good.

            Romain Manni-Bucau (Inactive) added a comment - Not sure in term of implementation but stating the user shouldnt modify it and it is not portable what happens if so sounds good.

            I'm a big fan of defensive programming and my assumption is all the metadata exposed to the user should be immutable. This "temporary mutable thing" is a bad idea and might only bring confusion. Also I think it's not a clear API if an interface behaves differently in different situations.

            Martin Kouba added a comment - I'm a big fan of defensive programming and my assumption is all the metadata exposed to the user should be immutable. This "temporary mutable thing" is a bad idea and might only bring confusion. Also I think it's not a clear API if an interface behaves differently in different situations.

              Unassigned Unassigned
              asabotdu@redhat.com Antoine Sabot-Durand (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Created:
                Updated:
                Resolved: