1 2 Previous Next 17 Replies Latest reply on Dec 12, 2007 6:49 AM by adrian.brock

    Nested property ref

    alesj

      Extending Trustin's user question to dev forum:
      - http://www.jboss.com/index.html?module=bb&op=viewtopic&p=4105341

      Supporting 2nd example is trivial, just a simple recursion on the property lookup in AbstractDependencyValueMetaData.

      I guess we could support the 1st one in a similar way as we did value-factory.
      Perhaps we can even extend existing value-factory notion to do that?

        • 1. Re: Nested property ref
          trustin

          +1.

          I'm not 100% sure how often people will face the first use case, but I already had an issue related with it. In Apache MINA, a user is not allowed to create a configuration object but only get the current configuration and set its properties (e.g. service.getConfig().setReadBufferSize(1024))

          The reason we don't allow a user to create a configuration object is because its default values differ between a server-side application and a client-side application and we don't want users to spend their time to specify it's a server side configuration or a client side configuration, which is often error-prone and leads to unnecessarily crowded mailing list. :)

          So, I think this kind of case, that an API provider wants to restrict the creation of a certain object but the provider is fine with the modification of its properties, will happen pretty often.

          • 2. Re: Nested property ref

             

            "alesj" wrote:
            Extending Trustin's user question to dev forum:
            - http://www.jboss.com/index.html?module=bb&op=viewtopic&p=4105341

            Supporting 2nd example is trivial, just a simple recursion on the property lookup in AbstractDependencyValueMetaData.

            I guess we could support the 1st one in a similar way as we did value-factory.
            Perhaps we can even extend existing value-factory notion to do that?


            You can support both very easily. The "." cannot be part of java property name,
            so would be legal to include in property names for both examples without
            breaking existing configurations.

            The first one looks a bit hacky to me. You only recurse until the second to last
            property using getters to locate the object then use the setter for the last property.

            The other part is that you have to take into null return from the getters (an error)
            and the BeanInfo for the object retrieved from the getter has to be retrieved
            at each step.

            The first example can actually be done already with a factory configuration.

            <bean name="TopLevel" .../>
            
            <bean name="Configuration" ...>
             <constructor><factory bean="TopLevel' method="getConfiguration"/></constructor>
             <property name="nested">sdfkjsdf</property>
            </bean>
            


            But that's a bit long winded compared to. ;-)
            <bean name="TopLevel" ...>
             <property name="configuration.nested">sdfkjsdf</property>
            </bean>
            


            P.S. You should also add the first example to the plain javabean xml.

            • 3. Re: Nested property ref

               

              "trustin" wrote:
              +1.


              We don't vote on code/changes at JBoss.
              Changes are made based on merit and reason arguments rather than plebiscites. ;-)

              • 4. Re: Nested property ref
                alesj

                Adrian or Trustin, could you just add a JIRA issue for this?
                So, that I don't forget. :-)

                • 5. Re: Nested property ref
                  trustin

                   

                  "adrian@jboss.org" wrote:
                  "trustin" wrote:
                  +1.


                  We don't vote on code/changes at JBoss.
                  Changes are made based on merit and reason arguments rather than plebiscites. ;-)


                  Doh! I'm probably too much used to the Apache way.

                  It was not about casting a vote but about seconding Ales's idea anyway ;)

                  • 6. Re: Nested property ref
                    trustin

                     

                    "alesj" wrote:
                    Adrian or Trustin, could you just add a JIRA issue for this?
                    So, that I don't forget. :-)

                    Here it is. :)
                    http://jira.jboss.com/jira/browse/JBMICROCONT-220

                    • 7. Re: Nested property ref
                      alesj

                       

                      "adrian@jboss.org" wrote:

                      The first one looks a bit hacky to me. You only recurse until the second to last
                      property using getters to locate the object then use the setter for the last property.

                      Hmmm, looking at the code, this doesn't seem that easy/clean to do - doing it in current KernelConfigurator/Configurator notion.

                      The way we currently do this is that we do a PropertyInfo lookup in the Configurator, but the actual target invocation is done outside the property resolution scope.
                      But in our case, we would already need to know the target, in order to execute getters.

                      Or I can refactor ConfigureAction to do this with bypassing KernelConfigurator usage.
                      And perhaps adding warning or throwing exception if name param in Configurator.resolveProperty contains '.'?

                      • 8. Re: Nested property ref
                        alesj

                         

                        "alesj" wrote:

                        Or I can refactor ConfigureAction to do this with bypassing KernelConfigurator usage.


                        I have a few question regarding this impl:
                         protected void dispatchSetProperty(PropertyMetaData property, boolean nullyfy, BeanInfo info, Object target, ClassLoader cl)
                         throws Throwable
                         {
                         String name = property.getName();
                         if (nullyfy)
                         {
                         info.setProperty(target, name, null);
                         }
                         else
                         {
                         PropertyInfo propertyInfo = info.getProperty(name);
                         ValueMetaData valueMetaData = property.getValue();
                         Object value = valueMetaData.getValue(propertyInfo.getType(), cl);
                         info.setProperty(target, name, value);
                         }
                         }
                        


                        This bypasses the KernelControllerContextAction.dispatchJoinPoint, which is probably not a good thing.

                        But on the other hand, every BeanInfo.get/set or PropertyInfo.get/set also doesn't go through JoinpointFactory.

                        What's the main purpose of JoinpointFactory then?
                        To simplyfy AOP usage or to add some other metadata notion?

                        • 9. Re: Nested property ref

                          Joinpoints are legacy. I explained the history behind them somewhere before,
                          find that for a longer discussion.

                          The original aim (abstracting different invocation models) has been dealt with
                          using other approaches.

                          Most of what they do is just delegated to Class/MethodInfo, etc.

                          They now have only two real uses:

                          1) AOP integrates by overriding the constructor joinpoint

                          2) It makes the constructor/method (e.g. for factories) handling a bit cleaner
                          since the code just has to resolve and dispatch a joinpoint.

                          Both could be implemented in different ways which would finally remove the need
                          for the Joinpoint abstraction.

                          • 10. Re: Nested property ref
                            alesj

                            OK, thanks for the short explanation. :-)

                            So, my new impl is legit? ;-)
                            Do I still need to wrap it up with MetaDataStack push and ContextClassloader change?

                            • 11. Re: Nested property ref
                              alesj

                              Commited.

                              The test to watch is NestedPropertyTestCase.
                              And the one that does most of the nested work is BeanInfoUtilTestCase.

                              • 12. Re: Nested property ref

                                 

                                "alesj" wrote:

                                Do I still need to wrap it up with MetaDataStack push and ContextClassloader change?


                                I've got no idea what you talking about?

                                • 13. Re: Nested property ref
                                  alesj

                                   

                                  "adrian@jboss.org" wrote:

                                  I've got no idea what you talking about?

                                  :-)

                                  About this code in KernelControllerContextAction:
                                   static Object dispatchJoinPoint(final KernelControllerContext context, final Joinpoint joinpoint) throws Throwable
                                   {
                                   BeanMetaData metaData = context.getBeanMetaData();
                                   ClassLoader cl = Configurator.getClassLoader(metaData);
                                   AccessControlContext access = null;
                                   if (context instanceof AbstractKernelControllerContext)
                                   {
                                   AbstractKernelControllerContext theContext = (AbstractKernelControllerContext) context;
                                   access = theContext.getAccessControlContext();
                                   }
                                  
                                   KernelController controller = (KernelController) context.getController();
                                   KernelMetaDataRepository repository = controller.getKernel().getMetaDataRepository();
                                   MetaData md = repository.getMetaData(context);
                                   if (md != null)
                                   MetaDataStack.push(md);
                                   else
                                   staticLog.warn("NO METADATA! for " + context.getName() + " with scope " + context.getScopeInfo().getScope());
                                   try
                                   {
                                  
                                   // Dispatch with the bean class loader if it exists
                                   ClassLoader tcl = Thread.currentThread().getContextClassLoader();
                                   try
                                   {
                                   if (cl != null && access == null)
                                   Thread.currentThread().setContextClassLoader(cl);
                                  
                                   if (access == null)
                                   {
                                   return joinpoint.dispatch();
                                   }
                                   else
                                   {
                                   DispatchJoinPoint action = new DispatchJoinPoint(joinpoint);
                                   try
                                   {
                                   return AccessController.doPrivileged(action, access);
                                   }
                                   catch (PrivilegedActionException e)
                                   {
                                   throw e.getCause();
                                   }
                                   }
                                   }
                                   finally
                                   {
                                   if (cl != null && access == null)
                                   Thread.currentThread().setContextClassLoader(tcl);
                                   }
                                   }
                                   finally
                                   {
                                   if (md != null)
                                   MetaDataStack.pop();
                                   }
                                   }
                                  


                                  Now that I've changed ConfigureAction to use direct BeanInfo.set, I guess there is no need for this?
                                  This == preparing MetaDataStack & ContextCL before BeanInfo.set invocation.

                                  • 14. Re: Nested property ref

                                    "
                                    Now that I've changed ConfigureAction to use direct BeanInfo.set, I guess there is no need for this?
                                    This == preparing MetaDataStack & ContextCL before BeanInfo.set invocation.
                                    "

                                    What? I don't understand the logic?

                                    You modified the code and are now claiming that you don't need to
                                    implement a feature of the old code?
                                    Or are you saying it is already done elsewhere?

                                    The classloader/metadata stack stuff needs doing more generically anyway.
                                    i.e. It should be doing it for JMX as well (although in the case of,
                                    JBossMX it already does the classloader - the RI JMX doesn't though).

                                    And it should also be doing it for all callouts to the bean (I don't know that
                                    it is done currently?).
                                    My preference is that this code should be in an AOP interceptor not in the MC
                                    anyway. You want the classloader/metadata stack for all invocations
                                    (if you want it all) not just the MC lifecycle.

                                    1 2 Previous Next