-
1. Re: Nested property ref
trustin Nov 16, 2007 5:47 AM (in response to alesj)+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
adrian.brock Nov 16, 2007 6:16 AM (in response to alesj)"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
adrian.brock Nov 16, 2007 6:18 AM (in response to alesj)"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 Nov 16, 2007 2:35 PM (in response to 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 Nov 16, 2007 4:21 PM (in response to alesj)"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 Nov 16, 2007 4:37 PM (in response to alesj)"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 Nov 23, 2007 6:22 AM (in response to 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 Dec 10, 2007 1:24 PM (in response to 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
adrian.brock Dec 10, 2007 1:39 PM (in response to alesj)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 Dec 10, 2007 2:00 PM (in response to 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 Dec 10, 2007 6:41 PM (in response to 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
adrian.brock Dec 12, 2007 5:36 AM (in response to alesj)"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 Dec 12, 2007 5:42 AM (in response to 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
adrian.brock Dec 12, 2007 6:16 AM (in response to alesj)"
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.