|
From: Colin S. <col...@ex...> - 2004-10-06 13:46:33
|
Guillaume, Thanks for doing this. I personally can't look at this this week, but can probably look at it next. Your mechanism of using a new <ognl> tag is abviously completely backwards compatible. The other option we were discussing before was in the regular value tag, recognizing a prefix like "ognl:" as an indication that the given value is an ognl expression. I would actually like to also investigate Janino http://www.janino.net/ as an alternative to OGNL. I've used OGNL a fair amount with Tapestry, and it's quite good. That said, the Janino approach of generating bytecode directly from the expressions is pretty intriguing, in terms of performance implications. Colin Guillaume Poirier wrote: > Concerning OGNL, I took at stab at it to see how hard it would be to > integrate. My prototype seems to work correctly, with a few > adjustments needed. Would you guys be interested in a contribution > from me? > > My implementation requires modifications in two areas. First, the a > <ognl> tag should be added in the DTD, available everywhere you can > use the <value> tag. In the XmlBeanDefinitionParser, the <ognl> tag > would be converted to an OGNL implementation of a new Expression > interface that define a class to resolve the expression given a > context/BeanFactory. > > Then, the BeanFactory implementation would need to be modified, when > it resolve ManagedList, RuntimeBeanReference, a new check would be > added for the Expression class, where the expression would be resolved > from the context. > > However, I encountered a minor issue while creating my prototype. The > resolution where the expression would be converted to a value is in > AbstractAutowireCapableBeanFactory, which doesn't implement > ListableBeanFactory. It causes a problem because OGNL ask for a Map > as context, and it calls the keySet() method on it, thus the map > passed to OGNL must be able to return all the bean names in the > context. A quick fix is to cast the > AbstractAutowireCapableBeanFactory instance to ListableBeanFactory, > since all of Spring's concrete BeanFactory implements > ListableBeanFactory anyway, but it's somewhat an hack, and if someone > extend AbstractAutowireCapableBeanFactory and doesn't implement > ListableBeanFactory, then the OGNL wouldn't work properly. I guess > it's unlikely though. > > I've attached the patch to this email, with the prototype for OGNL. > It's working, but it doesn't have any documenation, decent tests, > etc. If you guys are interested in my code, I can polish it, document > it, and add a better test coverage. > > Let me know what you think. > > Regards, > Guillaume > > ----- Original Message ----- From: "Colin Sampaleanu" <col...@ex...> > To: <spr...@li...> > Sent: Monday, October 04, 2004 1:32 PM > Subject: Re: [Springframework-developer] PropertyPathFactoryBean > > >> Good stuff. It overlaps of course with the mythical OGNL or >> expression support which keeps getting pushed back, but in the >> meantime provides some of the benefits for pretty little extra code... >> >> >> jürgen höller [werk3AT] wrote: >> >>> I've just added a PropertyPathFactoryBean, following this suggestion: >>> >>> http://opensource.atlassian.com/projects/spring/browse/SPR-343 >>> >>> >>> Usage examples: >>> >>> // target bean to be referenced by name >>> <bean id="tb" class="org.springframework.beans.TestBean" >>> singleton="false"> >>> <property name="age"><value>10</value></property> >>> <property name="spouse"> >>> <bean class="org.springframework.beans.TestBean"> >>> <property name="age"><value>11</value></property> >>> </bean> >>> </property> >>> </bean> >>> >>> // will result in 12, which is the value of property 'age' of the >>> inner bean >>> <bean id="propertyPath1" >>> class="org.springframework.beans.factory.config.PropertyPathFactoryBean"> >>> >>> <property name="targetObject"> >>> <bean class="org.springframework.beans.TestBean"> >>> <property name="age"><value>12</value></property> >>> </bean> >>> </property> >>> <property name="propertyPath"><value>age</value></property> >>> </bean> >>> >>> // will result in 11, which is the value of property 'spouse.age' of >>> bean 'tb' >>> <bean id="propertyPath2" >>> class="org.springframework.beans.factory.config.PropertyPathFactoryBean"> >>> >>> <property name="targetBeanName"><value>tb</value></property> >>> <property name="propertyPath"><value>spouse.age</value></property> >>> </bean> >>> >>> // will result in 10, which is the value of property 'age' of bean 'tb' >>> <bean id="tb.age" >>> class="org.springframework.beans.factory.config.PropertyPathFactoryBean"/> >>> >>> >>> >>> The last one, interpreting the bean name as 'beanName.property' >>> pattern, is quite concise... >>> >> >> >> >> >> ------------------------------------------------------- >> This SF.net email is sponsored by: IT Product Guide on ITManagersJournal >> Use IT products in your business? Tell us what you think of them. >> Give us >> Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find >> out more >> http://productguide.itmanagersjournal.com/guidepromo.tmpl >> _______________________________________________ >> Springframework-developer mailing list >> Spr...@li... >> https://lists.sourceforge.net/lists/listinfo/springframework-developer > > Index: org/springframework/beans/factory/OgnlTests.java > =================================================================== > RCS file: org/springframework/beans/factory/OgnlTests.java > diff -N org/springframework/beans/factory/OgnlTests.java > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ org/springframework/beans/factory/OgnlTests.java 1 Jan 1970 > 00:00:00 -0000 > @@ -0,0 +1,79 @@ > +/* > + * $Id$ > + * $Log$ > + */ > +package org.springframework.beans.factory; > + > +import java.util.Arrays; > +import java.util.List; > + > +import junit.framework.TestCase; > + > +import org.springframework.beans.factory.xml.XmlBeanFactory; > + > +/** > + * @author Guillaume Poirier > + * @version $Revision$, $Date$ > + * @since 2004-10-04 > + */ > +public class OgnlTests extends TestCase { > + public void testOgnlSupport() throws Exception { > + XmlBeanFactory factory = new XmlBeanFactory(OgnlTests.class > + .getResourceAsStream("ognl.xml")); > + Bean test = (Bean) factory.getBean("test"); > + Object list = factory.getBean("list"); > + assertSame(test.one, list); > + assertEquals(test.two, new Integer(Bean.CONSTANT)); > + assertSame(test.three, test.getProperty()); > + List expectedList = Arrays.asList(new Object[] { "Item0", null, > null, > + test.getProperty() }); > + assertEquals(expectedList, list); > + } > + > + public static class Bean { > + public static final int CONSTANT = 15; > + private Object one; > + private Object two; > + private Object three; > + > + public Bean() { > + } > + > + public Bean(Object arg) { > + throw new UnsupportedOperationException(); > + } > + > + public Object getOne() { > + return one; > + } > + > + public void setOne(Object one) { > + this.one = one; > + } > + > + public Object getTwo() { > + return two; > + } > + > + public void setTwo(Object two) { > + this.two = two; > + } > + > + public Object getThree() { > + return three; > + } > + > + public void setThree(Object three) { > + this.three = three; > + } > + > + public String getProperty() { > + return "property!!"; > + } > + > + public String toString() { > + return "OgnlTests$Bean[1='" + one + "', 2='" + two + "', 3='" + > three > + + "']"; > + } > + } > +} > Index: org/springframework/beans/factory/ognl.xml > =================================================================== > RCS file: org/springframework/beans/factory/ognl.xml > diff -N org/springframework/beans/factory/ognl.xml > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ org/springframework/beans/factory/ognl.xml 1 Jan 1970 00:00:00 > -0000 > @@ -0,0 +1,40 @@ > +<?xml version="1.0" encoding="UTF-8"?> > +<!DOCTYPE beans PUBLIC "-//SPRING//DTD BEAN//EN" > "http://www.springframework.org/dtd/spring-beans.dtd"> > + > +<beans> > + <bean id="list" class="java.util.ArrayList"> > + <constructor-arg> > + <list> > + <value>Item0</value> > + <ognl>#test.one</ognl> > + <ognl>#test.two</ognl> > + <ognl>#test.property</ognl> > + </list> > + </constructor-arg> > + </bean> > + + <bean id="test" > class="org.springframework.beans.factory.OgnlTests$Bean"> > + <property name="one"> > + <ognl>#list</ognl> > + </property> > + <property name="two"> > + > <ognl>@org.springframework.beans.factory.OgnlTests$Bean@CONSTANT</ognl> > + </property> > + <property name="three"> > + <ognl>#test.property</ognl> > + </property> > + </bean> > + + <!-- > + - This bean should not be instanciated by the BeanFactory. > + - If it was mistakenly instanciated, an > UnsupportedOperationException > + - would be thrown by the constructor. > + --> > + <bean id="dummy" > class="org.springframework.beans.factory.OgnlTests$Bean"> > + <constructor-arg> > + <list> > + <value>Item0</value> > + </list> > + </constructor-arg> > + </bean> > +</beans> > > Index: > org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java > > =================================================================== > RCS file: > /cvsroot/springframework/spring/src/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java,v > > retrieving revision 1.40 > diff -u -r1.40 AbstractAutowireCapableBeanFactory.java > --- > org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java > 4 Oct 2004 07:59:32 -0000 1.40 > +++ > org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java > 6 Oct 2004 04:05:56 -0000 > @@ -43,6 +43,7 @@ > import org.springframework.beans.factory.BeanNameAware; > import org.springframework.beans.factory.DisposableBean; > import org.springframework.beans.factory.InitializingBean; > +import org.springframework.beans.factory.ListableBeanFactory; > import org.springframework.beans.factory.UnsatisfiedDependencyException; > import > org.springframework.beans.factory.config.AutowireCapableBeanFactory; > import org.springframework.beans.factory.config.BeanDefinition; > @@ -839,10 +840,14 @@ > // May need to resolve contained runtime references. > return resolveManagedSet(beanName, mergedBeanDefinition, > argName, (Set) value); > } > - else if (value instanceof ManagedMap) { > - // May need to resolve contained runtime references. > - return resolveManagedMap(beanName, mergedBeanDefinition, > argName, (Map) value); > - } > + else if (value instanceof ManagedMap) { > + // May need to resolve contained runtime references. > + return resolveManagedMap(beanName, mergedBeanDefinition, > argName, (Map) value); > + } > + else if (value instanceof Expression) { > + // Need to resolve Expression. > + return resolveExpression(beanName, mergedBeanDefinition, > argName, (Expression) value); > + } > else { > // no need to resolve value > return value; > @@ -866,6 +871,13 @@ > return getObjectForSharedInstance(innerBeanName, innerBean); > } > > + private Object resolveExpression( > + String beanName, RootBeanDefinition mergedBeanDefinition, > String argName, Expression expr) > + throws BeansException { > + // TODO Casting to ListableBeanFactory is kind of an hack > + return expr.resolve((ListableBeanFactory) this); > + } > + /** > * Resolve a reference to another bean in the factory. > */ > Index: > org/springframework/beans/factory/xml/DefaultXmlBeanDefinitionParser.java > =================================================================== > RCS file: > /cvsroot/springframework/spring/src/org/springframework/beans/factory/xml/DefaultXmlBeanDefinitionParser.java,v > > retrieving revision 1.41 > diff -u -r1.41 DefaultXmlBeanDefinitionParser.java > --- > org/springframework/beans/factory/xml/DefaultXmlBeanDefinitionParser.java > 4 Oct 2004 08:12:00 -0000 1.41 > +++ > org/springframework/beans/factory/xml/DefaultXmlBeanDefinitionParser.java > 6 Oct 2004 04:05:56 -0000 > @@ -49,6 +49,7 @@ > import org.springframework.beans.factory.support.ManagedMap; > import org.springframework.beans.factory.support.ManagedSet; > import org.springframework.beans.factory.support.MethodOverrides; > +import org.springframework.beans.factory.support.OgnlExpression; > import org.springframework.beans.factory.support.ReplaceOverride; > import org.springframework.beans.factory.support.RootBeanDefinition; > import org.springframework.core.io.Resource; > @@ -132,6 +133,7 @@ > public static final String PROP_ELEMENT = "prop"; > public static final String VALUE_ELEMENT = "value"; > public static final String NULL_ELEMENT = "null"; > + public static final String OGNL_ELEMENT = "ognl"; > > > protected final Log logger = LogFactory.getLog(getClass()); > @@ -554,10 +556,14 @@ > // it's a literal value > return getTextValue(ele, beanName); > } > - else if (ele.getTagName().equals(NULL_ELEMENT)) { > - // it's a distinguished null value > - return null; > - } > + else if (ele.getTagName().equals(NULL_ELEMENT)) { > + // it's a distinguished null value > + return null; > + } > + else if (ele.getTagName().equals(OGNL_ELEMENT)) { > + // it's an OGNL expression > + return getOgnlExpression(ele, beanName); > + } > throw new BeanDefinitionStoreException( > this.resource, beanName, "Unknown subelement of > <property>: <" + ele.getTagName() + ">"); > } > @@ -633,24 +639,29 @@ > * Make the horrible DOM API slightly more bearable: > * get the text value we know this element contains. > */ > - protected String getTextValue(Element ele, String beanName) { > - StringBuffer value = new StringBuffer(); > - NodeList nl = ele.getChildNodes(); > - for (int i = 0; i < nl.getLength(); i++) { > - Node item = nl.item(i); > - if (item instanceof org.w3c.dom.CharacterData) { > - if (!(item instanceof Comment)) { > - value.append(item.getNodeValue()); > - } > - } > - else { > - throw new BeanDefinitionStoreException( > - this.resource, beanName, > - "<value> element is just allowed to have text > and comment nodes, not: " + item.getClass().getName()); > - } > - } > - return value.toString(); > - } > + protected String getTextValue(Element ele, String beanName) { > + StringBuffer value = new StringBuffer(); > + NodeList nl = ele.getChildNodes(); > + for (int i = 0; i < nl.getLength(); i++) { > + Node item = nl.item(i); > + if (item instanceof org.w3c.dom.CharacterData) { > + if (!(item instanceof Comment)) { > + value.append(item.getNodeValue()); > + } > + } > + else { > + throw new BeanDefinitionStoreException( > + this.resource, beanName, > + "<value> element is just allowed to have text and comment > nodes, not: " + item.getClass().getName()); > + } > + } > + return value.toString(); > + } > + + protected OgnlExpression getOgnlExpression(Element ele, String > beanName) { > + String value = getTextValue(ele, beanName); > + return new OgnlExpression(value); > + } > > protected int getDependencyCheck(String att) { > int dependencyCheckCode = > RootBeanDefinition.DEPENDENCY_CHECK_NONE; > Index: org/springframework/beans/factory/xml/spring-beans.dtd > =================================================================== > RCS file: > /cvsroot/springframework/spring/src/org/springframework/beans/factory/xml/spring-beans.dtd,v > > retrieving revision 1.41 > diff -u -r1.41 spring-beans.dtd > --- org/springframework/beans/factory/xml/spring-beans.dtd 5 Oct > 2004 08:37:53 -0000 1.41 > +++ org/springframework/beans/factory/xml/spring-beans.dtd 6 Oct > 2004 04:05:57 -0000 > @@ -280,7 +280,7 @@ > --> > <!ELEMENT constructor-arg ( > description?, > - (bean | ref | idref | list | set | map | props | value | null) > + (bean | ref | idref | list | set | map | props | value | null | > ognl) > )> > > <!-- > @@ -313,7 +313,7 @@ > --> > <!ELEMENT property ( > description?, > - (bean | ref | idref | list | set | map | props | value | null) > + (bean | ref | idref | list | set | map | props | value | null | > ognl) > )> > > <!-- > @@ -435,7 +435,7 @@ > is automatically performed by the BeanFactory. > --> > <!ELEMENT list ( > - (bean | ref | idref | list | set | map | props | value | null)* > + (bean | ref | idref | list | set | map | props | value | null | > ognl)* > )> > > <!-- > @@ -444,7 +444,7 @@ > although references will be strongly typed. > --> > <!ELEMENT set ( > - (bean | ref | idref | list | set | map | props | value | null)* > + (bean | ref | idref | list | set | map | props | value | null | > ognl)* > )> > > <!-- > @@ -460,7 +460,7 @@ > The name of the property is given by the "key" attribute. > --> > <!ELEMENT entry ( > - (bean | ref | idref | list | set | map | props | value | null) > + (bean | ref | idref | list | set | map | props | value | null | > ognl) > )> > > <!-- > @@ -508,3 +508,5 @@ > null value unless a special PropertyEditor does so. > --> > <!ELEMENT null (#PCDATA)> > + > +<!ELEMENT ognl (#PCDATA)> > Index: org/springframework/beans/factory/support/Expression.java > =================================================================== > RCS file: org/springframework/beans/factory/support/Expression.java > diff -N org/springframework/beans/factory/support/Expression.java > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ org/springframework/beans/factory/support/Expression.java 1 Jan > 1970 00:00:00 -0000 > @@ -0,0 +1,16 @@ > +/* > + * $Id$ > + * $Log$ > + */ > +package org.springframework.beans.factory.support; > + > +import org.springframework.beans.factory.ListableBeanFactory; > + > +/** > + * @author Guillaume Poirier > + * @version $Revision$, $Date$ > + * @since 2004-10-05 > + */ > +public interface Expression { > + public Object resolve(ListableBeanFactory factory); > +} > Index: org/springframework/beans/factory/support/OgnlExpression.java > =================================================================== > RCS file: org/springframework/beans/factory/support/OgnlExpression.java > diff -N org/springframework/beans/factory/support/OgnlExpression.java > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ org/springframework/beans/factory/support/OgnlExpression.java 1 > Jan 1970 00:00:00 -0000 > @@ -0,0 +1,50 @@ > +/* > + * $Id$ > + * $Log$ > + */ > +package org.springframework.beans.factory.support; > + > +import java.util.Map; > + > +import org.springframework.beans.factory.BeanCreationException; > +import org.springframework.beans.factory.ListableBeanFactory; > + > +import ognl.Ognl; > +import ognl.OgnlException; > + > +/** > + * @author Guillaume Poirier > + * @version $Revision$, $Date$ > + * @since 2004-10-05 > + */ > +public class OgnlExpression implements Expression { > + > + private Object expr; > + > + public OgnlExpression(String value) { > + try { > + expr = Ognl.parseExpression(value); > + } catch (OgnlException e) { > + throw new BeanCreationException("Failled to parse OGNL > expression", e); > + } > + } > + > + /* > + * (non-Javadoc) > + * + * @see > org.springframework.beans.factory.support.Expression#getValue(java.util.Map) > > + */ > + public Object resolve(ListableBeanFactory factory) { > + try { > + Map ctx = new OgnlExpressionContext(factory); > + return Ognl.getValue(expr, ctx, (Object) null); > + } catch (OgnlException e) { > + throw new BeanCreationException("Failled to resolve OGNL > expression", e); > + } > + } > + + public String toString() { > + return "OgnlExpression[value='" + expr.toString() + "']"; > + } > + > +} > Index: > org/springframework/beans/factory/support/OgnlExpressionContext.java > =================================================================== > RCS file: > org/springframework/beans/factory/support/OgnlExpressionContext.java > diff -N > org/springframework/beans/factory/support/OgnlExpressionContext.java > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ > org/springframework/beans/factory/support/OgnlExpressionContext.java > 1 Jan 1970 00:00:00 -0000 > @@ -0,0 +1,161 @@ > +/* > + * $Id$ > + * $Log$ > + */ > +package org.springframework.beans.factory.support; > + > +import java.util.Collection; > +import java.util.HashSet; > +import java.util.Map; > +import java.util.Set; > + > +import ognl.OgnlContext; > + > +import org.springframework.beans.factory.ListableBeanFactory; > + > +/** > + * @author Guillaume Poirier > + * @version $Revision$, $Date$ > + * @since 2004-10-05 > + */ > +public class OgnlExpressionContext extends OgnlContext { > + > + private ListableBeanFactory factory; > + > + public OgnlExpressionContext(ListableBeanFactory factory) { > + this.factory = factory; > + if (factory == null) { > + throw new NullPointerException("factory"); > + } > + } > + > + /* > + * (non-Javadoc) > + * + * @see java.util.Map#size() > + */ > + public int size() { > + throw new UnsupportedOperationException(); > + } > + > + /* > + * (non-Javadoc) > + * + * @see java.util.Map#clear() > + */ > + public void clear() { > + throw new UnsupportedOperationException(); > + } > + > + /* > + * (non-Javadoc) > + * + * @see java.util.Map#isEmpty() > + */ > + public boolean isEmpty() { > + throw new UnsupportedOperationException(); > + } > + > + /* > + * (non-Javadoc) > + * + * @see java.util.Map#containsKey(java.lang.Object) > + */ > + public boolean containsKey(Object key) { > + return super.containsKey(key) ? true : > factory.containsBean((String) key); > + } > + > + /* > + * (non-Javadoc) > + * + * @see java.util.Map#containsValue(java.lang.Object) > + */ > + public boolean containsValue(Object value) { > + throw new UnsupportedOperationException(); > + } > + > + /* > + * (non-Javadoc) > + * + * @see java.util.Map#values() > + */ > + public Collection values() { > + throw new UnsupportedOperationException(); > + } > + > + /* > + * (non-Javadoc) > + * + * @see java.util.Map#putAll(java.util.Map) > + */ > + public void putAll(Map t) { > + throw new UnsupportedOperationException(); > + } > + > + /* > + * (non-Javadoc) > + * + * @see java.util.Map#entrySet() > + */ > + public Set entrySet() { > + throw new UnsupportedOperationException(); > + } > + > + /* > + * (non-Javadoc) > + * + * @see java.util.Map#keySet() > + */ > + public Set keySet() { > + HashSet set = new HashSet(super.keySet()); > + String[] names = factory.getBeanDefinitionNames(); > + for (int i = 0, n = names.length; i < n; i++) { > + set.add(names[i]); > + } > + return set; > + } > + > + /* > + * (non-Javadoc) > + * + * @see java.util.Map#get(java.lang.Object) > + */ > + public Object get(Object key) { > + Object value = super.get(key); > + return value != null ? value : factory.getBean((String) key); > + } > + > + /* > + * (non-Javadoc) > + * + * @see java.util.Map#remove(java.lang.Object) > + */ > + public Object remove(Object key) { > + throw new UnsupportedOperationException(); > + } > + > + /* > + * (non-Javadoc) > + * + * @see java.util.Map#put(java.lang.Object, java.lang.Object) > + */ > + public Object put(Object key, Object value) { > + throw new UnsupportedOperationException(); > + } > + > + public Map getValues() { > + throw new UnsupportedOperationException(); > + } > + > + public void setValues(Map value) { > + throw new UnsupportedOperationException(); > + } > + > + public boolean equals(Object o) { > + return o == this; > + } > + > + public int hashCode() { > + return System.identityHashCode(this); > + } > +} > |