5 Replies Latest reply on Nov 17, 2009 6:33 AM by alesj

    Non-public field properties

    kabirkhan

      Back to the problem from earlier:

      public class SimpleFieldBean
      {
       @Inject
       public Simple field;
      
       public Simple getField()
       {
       return field;
       }
      }
      


      The issue is that if the field is public it works and PropertyInfo has both a getter and a FieldInfo. If it is private PropertyInfo.getFieldInfo() returns null and no injection happens on the field.

      Jsr 330 requires injection to be possible in non-public fields, so we probably need to fix this? I'm unsure if that will break anything else, or if we need a new BeanAccessMode.

      On a related note I haven't tried injecting into non-public methods, setters and constructors yet, which are also required to be supported.

        • 1. Re: Non-public field properties
          kabirkhan

           

          "kabir.khan@jboss.com" wrote:

          On a related note I haven't tried injecting into non-public methods, setters and constructors yet, which are also required to be supported.

          Private fields, methods, setters and constructors do not work

          • 2. Re: Non-public field properties
            alesj

             

            "kabir.khan@jboss.com" wrote:

            The issue is that if the field is public it works and PropertyInfo has both a getter and a FieldInfo. If it is private PropertyInfo.getFieldInfo() returns null and no injection happens on the field.

            This is expected with BeanAccessMode::FIELDS.
            If you want private you need to use ALL.

            "kabir.khan@jboss.com" wrote:

            Jsr 330 requires injection to be possible in non-public fields, so we probably need to fix this? I'm unsure if that will break anything else, or if we need a new BeanAccessMode.

            Wrt JSR330 I wouldn't follow all their rules,
            I would mostly use its annotations to unify the usage,
            hence increase portability.

            And injection (or any access) to non-public fields seems wrong to me,
            something I don't wanna encourage, at least not for services.

            If we really wanna go down this path, we should then provide new BAMs.

            "kabir.khan@jboss.com" wrote:

            On a related note I haven't tried injecting into non-public methods, setters and constructors yet, which are also required to be supported.

            As you found out, that doesn't work.
            Currently we only apply BAM to properties / fields.

            I would go with supporting annotations on public stuff atm,
            if there is some demand we can extend our current BAM support.
            Otoh, with proper MC+Weld support, you already get 330 and non-public injection ootb.

            • 3. Re: Non-public field properties
              kabirkhan

              Thanks it works for private fields with ALL, and I'll get rid of the tests for private methods, setters and constructors.

              "alesj" wrote:

              Wrt JSR330 I wouldn't follow all their rules,
              I would mostly use its annotations to unify the usage,
              hence increase portability.


              Do you think I should implement @Scope and @Singleton?
              public class SimpleFieldBean
              {
               @Inject
               public Simple field;
              
               @Inject
               public Simple other;
              }
              

              The way this is described is that if Simple has no annotations, a new instance is injected for each use, i.e. field != other. If Simple has a @Scope annotated annotation, such as @Singleton, the same instance is injected, so field == other, which in effect is what we have now.


              • 4. Re: Non-public field properties
                kabirkhan

                Although my tests for @Inject work fine now, I am not 100% sure I am using the right annotation plugins. Here's what I am doing for @Inject annotated constructors:

                public class Jsr330InjectConstructorAnnotationPlugin extends ConstructorAnnotationPlugin<Inject>
                {
                 public static final Jsr330InjectConstructorAnnotationPlugin INSTANCE = new Jsr330InjectConstructorAnnotationPlugin();
                
                 protected Jsr330InjectConstructorAnnotationPlugin()
                 {
                 super(Inject.class);
                 }
                
                 protected List<? extends MetaDataVisitorNode> internalApplyAnnotation(ConstructorInfo info, Inject annotation, BeanMetaData beanMetaData) throws Throwable
                 {
                 if (info.getParameters().length == 0)
                 return null;
                 if (beanMetaData instanceof AbstractBeanMetaData == false)
                 throw new IllegalArgumentException("beanMetaData is not a AbstractBeanMetaData");
                
                 AbstractConstructorMetaData constructor = new AbstractConstructorMetaData();
                
                 List<ParameterMetaData> values = new ArrayList<ParameterMetaData>(info.getParameters().length);
                 for (ParameterInfo parameter : info.getParameters())
                 values.add(new AbstractParameterMetaData(Jsr330InjectionResolver.createValueMetaData(parameter.getParameterType().getType(), parameter.getUnderlyingAnnotations())));
                
                 constructor.setParameters(values);
                 ((AbstractBeanMetaData)beanMetaData).setConstructor(constructor);
                
                 return Collections.singletonList(constructor);
                 }
                
                 @Override
                 protected void internalCleanAnnotation(ConstructorInfo info, MetaData retrieval, Inject annotation,
                 KernelControllerContext context) throws Throwable
                 {
                 if (context.getBeanMetaData() instanceof AbstractBeanMetaData == false)
                 throw new IllegalArgumentException("beanMetaData is not a AbstractBeanMetaData");
                 ((AbstractBeanMetaData)context.getBeanMetaData()).setConstructor(null);
                 }
                }
                


                I see elsewhere that the ConstructorParameterAnnotationPlugin is used, which takes a set of Annotation2ValueMetaDataAdapters used to parse the parameters. However, Annotation2ValueMetaDataAdapters need to know which annotation is being used, which in the case of Annotation2ValueMetaDataAdapters is unknown, since the trigger is looking for @Qualifier on the parameter annotations.
                Also, parameters might not have any annotations which is probably currently handled some other way for normal injection.

                • 5. Re: Non-public field properties
                  alesj

                  We don't do too much magic - which is what 330 supports.
                  e.g.

                  public class Foo
                  {
                   @Inject
                   public Foo(SomeMappedBean smb)
                   ...
                  }
                  


                  We require a bit more explicit wiring
                  public class Foo
                  {
                   @Constructor
                   public Foo(@Inject SomeMappedBean smb)
                   ...
                  }
                  

                  Which is already handled by @Constructor ctor plugin.
                  Plus the properties plugin should also be an Annotation2ValueMetaDataAdapters instance.

                  But I guess once we introduce qualifiers for MC in general,
                  we should support the example above - @Inject on ctor.
                  Which means your plugin is OK, but it could use a bit nicer code. ;-)
                  e.g. there already exist a code that checks for AbstractBeanMetaData instance of