From 7d757bb48d4cf121971b0c528ae8b6c414acf1f2 Mon Sep 17 00:00:00 2001 From: Keith Donald Date: Fri, 8 Aug 2008 14:33:44 +0000 Subject: [PATCH] fixed conversion service registration bug --- build-spring-webflow/resources/changelog.txt | 6 +++ ...owBuilderServicesBeanDefinitionParser.java | 54 +++++++++---------- ...owBuilderServicesBeanDefinitionParser.java | 17 +++++- .../FlowRegistryBeanDefinitionParser.java | 7 ++- .../DefaultExpressionParserFactory.java | 51 +++++++++++++++--- 5 files changed, 98 insertions(+), 37 deletions(-) diff --git a/build-spring-webflow/resources/changelog.txt b/build-spring-webflow/resources/changelog.txt index 1cd03d99..dacd701e 100644 --- a/build-spring-webflow/resources/changelog.txt +++ b/build-spring-webflow/resources/changelog.txt @@ -2,6 +2,12 @@ SPRING WEB FLOW CHANGELOG ========================= http://www.springframework.org/webflow +Changes in version 2.0.4 (15.08.2008) +------------------------------------- +Bug Fixes +* Fixed bug where a registered ConversionService was not auto-wired with the default ExpressionParser implementation. + This prevented user-installed type converters from being applied without explicit configuration. + Changes in version 2.0.3 (31.07.2008) ------------------------------------- New Features diff --git a/spring-faces/src/main/java/org/springframework/faces/config/FacesFlowBuilderServicesBeanDefinitionParser.java b/spring-faces/src/main/java/org/springframework/faces/config/FacesFlowBuilderServicesBeanDefinitionParser.java index 3cb2c3d9..6e0d4c08 100644 --- a/spring-faces/src/main/java/org/springframework/faces/config/FacesFlowBuilderServicesBeanDefinitionParser.java +++ b/spring-faces/src/main/java/org/springframework/faces/config/FacesFlowBuilderServicesBeanDefinitionParser.java @@ -15,9 +15,11 @@ */ package org.springframework.faces.config; +import org.springframework.beans.factory.config.RuntimeBeanReference; import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.xml.AbstractSingleBeanDefinitionParser; import org.springframework.beans.factory.xml.BeanDefinitionParser; +import org.springframework.binding.convert.ConversionService; import org.springframework.binding.expression.el.DefaultExpressionFactoryUtils; import org.springframework.faces.model.converter.FacesConversionService; import org.springframework.faces.webflow.JsfManagedBeanAwareELExpressionParser; @@ -35,20 +37,6 @@ import org.w3c.dom.Element; public class FacesFlowBuilderServicesBeanDefinitionParser extends AbstractSingleBeanDefinitionParser implements BeanDefinitionParser { - private static final String ENABLE_MANAGED_BEANS_ATTRIBUTE = "enable-managed-beans"; - - private static final String EXPRESSION_PARSER_ATTRIBUTE = "expression-parser"; - - private static final String EXPRESSION_PARSER_PROPERTY = "expressionParser"; - - private static final String VIEW_FACTORY_CREATOR_ATTRIBUTE = "view-factory-creator"; - - private static final String VIEW_FACTORY_CREATOR_PROPERTY = "viewFactoryCreator"; - - private static final String CONVERSION_SERVICE_ATTRIBUTE = "conversion-service"; - - private static final String CONVERSION_SERVICE_PROPERTY = "conversionService"; - protected Class getBeanClass(Element element) { return FlowBuilderServices.class; } @@ -56,7 +44,7 @@ public class FacesFlowBuilderServicesBeanDefinitionParser extends AbstractSingle protected void doParse(Element element, BeanDefinitionBuilder definitionBuilder) { boolean enableManagedBeans = parseEnableManagedBeans(element, definitionBuilder); if (enableManagedBeans) { - definitionBuilder.addPropertyValue(EXPRESSION_PARSER_PROPERTY, new JsfManagedBeanAwareELExpressionParser( + definitionBuilder.addPropertyValue("expressionParser", new JsfManagedBeanAwareELExpressionParser( DefaultExpressionFactoryUtils.createExpressionFactory())); } else { parseExpressionParser(element, definitionBuilder); @@ -66,7 +54,7 @@ public class FacesFlowBuilderServicesBeanDefinitionParser extends AbstractSingle } private boolean parseEnableManagedBeans(Element element, BeanDefinitionBuilder definitionBuilder) { - String enableManagedBeans = element.getAttribute(ENABLE_MANAGED_BEANS_ATTRIBUTE); + String enableManagedBeans = element.getAttribute("enable-managed-beans"); if (StringUtils.hasText(enableManagedBeans)) { return Boolean.valueOf(enableManagedBeans).booleanValue(); } else { @@ -75,31 +63,43 @@ public class FacesFlowBuilderServicesBeanDefinitionParser extends AbstractSingle } private void parseConversionService(Element element, BeanDefinitionBuilder definitionBuilder) { - String conversionService = element.getAttribute(CONVERSION_SERVICE_ATTRIBUTE); + String conversionService = element.getAttribute("conversion-service"); if (StringUtils.hasText(conversionService)) { - definitionBuilder.addPropertyReference(CONVERSION_SERVICE_PROPERTY, conversionService); + definitionBuilder.addPropertyReference("conversionService", conversionService); } else { - definitionBuilder.addPropertyValue(CONVERSION_SERVICE_PROPERTY, new FacesConversionService()); + definitionBuilder.addPropertyValue("conversionService", new FacesConversionService()); } } private void parseViewFactoryCreator(Element element, BeanDefinitionBuilder definitionBuilder) { - String viewFactoryCreator = element.getAttribute(VIEW_FACTORY_CREATOR_ATTRIBUTE); + String viewFactoryCreator = element.getAttribute("view-factory-creator"); if (StringUtils.hasText(viewFactoryCreator)) { - definitionBuilder.addPropertyReference(VIEW_FACTORY_CREATOR_PROPERTY, viewFactoryCreator); + definitionBuilder.addPropertyReference("viewFactoryCreator", viewFactoryCreator); } else { - definitionBuilder.addPropertyValue(VIEW_FACTORY_CREATOR_PROPERTY, new JsfViewFactoryCreator()); + definitionBuilder.addPropertyValue("viewFactoryCreator", new JsfViewFactoryCreator()); } } private void parseExpressionParser(Element element, BeanDefinitionBuilder definitionBuilder) { - String expressionParser = element.getAttribute(EXPRESSION_PARSER_ATTRIBUTE); + String expressionParser = element.getAttribute("expression-parser"); if (StringUtils.hasText(expressionParser)) { - definitionBuilder.addPropertyReference(EXPRESSION_PARSER_PROPERTY, expressionParser); + definitionBuilder.addPropertyReference("expressionParser", expressionParser); } else { - definitionBuilder.addPropertyValue(EXPRESSION_PARSER_PROPERTY, new WebFlowELExpressionParser( - DefaultExpressionFactoryUtils.createExpressionFactory())); + Object value = definitionBuilder.getBeanDefinition().getPropertyValues().getPropertyValue( + "conversionService"); + if (value instanceof RuntimeBeanReference) { + BeanDefinitionBuilder builder = BeanDefinitionBuilder + .genericBeanDefinition(WebFlowELExpressionParser.class); + builder.addConstructorArgValue(DefaultExpressionFactoryUtils.createExpressionFactory()); + builder.addPropertyValue("conversionService", value); + definitionBuilder.addPropertyValue("expressionParser", builder.getBeanDefinition()); + } else { + ConversionService conversionService = (ConversionService) value; + WebFlowELExpressionParser elExpressionParser = new WebFlowELExpressionParser( + DefaultExpressionFactoryUtils.createExpressionFactory()); + elExpressionParser.setConversionService(conversionService); + definitionBuilder.addPropertyValue("expressionParser", elExpressionParser); + } } } - } diff --git a/spring-webflow/src/main/java/org/springframework/webflow/config/FlowBuilderServicesBeanDefinitionParser.java b/spring-webflow/src/main/java/org/springframework/webflow/config/FlowBuilderServicesBeanDefinitionParser.java index c80ca857..e03427c3 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/config/FlowBuilderServicesBeanDefinitionParser.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/config/FlowBuilderServicesBeanDefinitionParser.java @@ -15,10 +15,12 @@ */ package org.springframework.webflow.config; +import org.springframework.beans.factory.config.RuntimeBeanReference; import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.xml.AbstractSingleBeanDefinitionParser; import org.springframework.beans.factory.xml.BeanDefinitionParser; import org.springframework.beans.factory.xml.ParserContext; +import org.springframework.binding.convert.ConversionService; import org.springframework.binding.convert.service.DefaultConversionService; import org.springframework.util.StringUtils; import org.springframework.webflow.engine.builder.support.FlowBuilderServices; @@ -57,8 +59,19 @@ class FlowBuilderServicesBeanDefinitionParser extends AbstractSingleBeanDefiniti if (StringUtils.hasText(expressionParser)) { definitionBuilder.addPropertyReference("expressionParser", expressionParser); } else { - definitionBuilder - .addPropertyValue("expressionParser", DefaultExpressionParserFactory.getExpressionParser()); + Object value = definitionBuilder.getBeanDefinition().getPropertyValues().getPropertyValue( + "converisonService"); + if (value instanceof RuntimeBeanReference) { + BeanDefinitionBuilder builder = BeanDefinitionBuilder + .genericBeanDefinition(DefaultExpressionParserFactory.class); + builder.setFactoryMethod("getExpressionParser"); + builder.addConstructorArgValue(value); + definitionBuilder.addPropertyValue("expressionParser", builder.getBeanDefinition()); + } else { + ConversionService conversionService = (ConversionService) value; + definitionBuilder.addPropertyValue("expressionParser", DefaultExpressionParserFactory + .getExpressionParser(conversionService)); + } } } diff --git a/spring-webflow/src/main/java/org/springframework/webflow/config/FlowRegistryBeanDefinitionParser.java b/spring-webflow/src/main/java/org/springframework/webflow/config/FlowRegistryBeanDefinitionParser.java index 769cae74..c532de19 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/config/FlowRegistryBeanDefinitionParser.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/config/FlowRegistryBeanDefinitionParser.java @@ -27,6 +27,7 @@ import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.xml.AbstractSingleBeanDefinitionParser; import org.springframework.beans.factory.xml.BeanDefinitionParser; import org.springframework.beans.factory.xml.ParserContext; +import org.springframework.binding.convert.ConversionService; import org.springframework.binding.convert.service.DefaultConversionService; import org.springframework.util.StringUtils; import org.springframework.util.xml.DomUtils; @@ -126,8 +127,10 @@ class FlowRegistryBeanDefinitionParser extends AbstractSingleBeanDefinitionParse private BeanDefinition createDefaultFlowBuilderServices(ParserContext context) { BeanDefinitionBuilder defaultBuilder = BeanDefinitionBuilder.genericBeanDefinition(FlowBuilderServices.class); - defaultBuilder.addPropertyValue("conversionService", new DefaultConversionService()); - defaultBuilder.addPropertyValue("expressionParser", DefaultExpressionParserFactory.getExpressionParser()); + ConversionService conversionService = new DefaultConversionService(); + defaultBuilder.addPropertyValue("conversionService", conversionService); + defaultBuilder.addPropertyValue("expressionParser", DefaultExpressionParserFactory + .getExpressionParser(conversionService)); defaultBuilder.addPropertyValue("viewFactoryCreator", BeanDefinitionBuilder.genericBeanDefinition( MvcViewFactoryCreator.class).getBeanDefinition()); return defaultBuilder.getBeanDefinition(); diff --git a/spring-webflow/src/main/java/org/springframework/webflow/expression/DefaultExpressionParserFactory.java b/spring-webflow/src/main/java/org/springframework/webflow/expression/DefaultExpressionParserFactory.java index 58be76a9..34f1a295 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/expression/DefaultExpressionParserFactory.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/expression/DefaultExpressionParserFactory.java @@ -19,6 +19,7 @@ import javax.el.ExpressionFactory; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.binding.convert.ConversionService; import org.springframework.binding.expression.Expression; import org.springframework.binding.expression.ExpressionParser; import org.springframework.binding.expression.ParserContext; @@ -56,7 +57,7 @@ public final class DefaultExpressionParserFactory { } /** - * Returns the default expression parser for Spring Web Flow. The returned instance is a thread-safe object. + * Returns the default expression parser for Spring Web Flow. The returned instance is a cached thread-safe object. * @return the expression parser */ public static synchronized ExpressionParser getExpressionParser() { @@ -70,15 +71,25 @@ public final class DefaultExpressionParserFactory { }; } + /** + * Returns the default expression parser for Spring Web Flow configured with the provided ConversionService for type + * conversion. The returned instance is a thread-safe object. + * @param conversionService the conversionService + * @return the expression parser + */ + public static synchronized ExpressionParser getExpressionParser(final ConversionService conversionService) { + return new DefaultExpressionParserProxy(conversionService); + } + /** * Returns the default expression parser, creating it if necessary. * @return the default expression parser */ private static synchronized ExpressionParser getDefaultExpressionParser() { if (INSTANCE == null) { - INSTANCE = createDefaultExpressionParser(); + INSTANCE = createDefaultExpressionParser(null); if (logger.isDebugEnabled()) { - logger.debug("Initialized default Web Flow ExpressionParser " + INSTANCE); + logger.debug("Initialized shared default Web Flow ExpressionParser " + INSTANCE); } } return INSTANCE; @@ -88,14 +99,23 @@ public final class DefaultExpressionParserFactory { * Create the default expression parser. This implementation tries EL first, then OGNL if EL is not configured. * @return the default Web Flow expression parser */ - private static ExpressionParser createDefaultExpressionParser() throws IllegalStateException { + private static ExpressionParser createDefaultExpressionParser(ConversionService conversionService) + throws IllegalStateException { try { ExpressionFactory elFactory = DefaultExpressionFactoryUtils.createExpressionFactory(); - return new WebFlowELExpressionParser(elFactory); + WebFlowELExpressionParser expressionParser = new WebFlowELExpressionParser(elFactory); + if (conversionService != null) { + expressionParser.setConversionService(conversionService); + } + return expressionParser; } catch (Exception e) { try { ClassUtils.forName("ognl.Ognl", DefaultExpressionParserFactory.class.getClassLoader()); - return new WebFlowOgnlExpressionParser(); + WebFlowOgnlExpressionParser expressionParser = new WebFlowOgnlExpressionParser(); + if (conversionService != null) { + expressionParser.setConversionService(conversionService); + } + return expressionParser; } catch (ClassNotFoundException ex) { IllegalStateException ise = new IllegalStateException( "Unable to create the default expression parser for Spring Web Flow: Neither a Unified EL implementation or OGNL could be found."); @@ -109,4 +129,23 @@ public final class DefaultExpressionParserFactory { } } } + + private static class DefaultExpressionParserProxy implements ExpressionParser { + private ConversionService conversionService; + + private ExpressionParser instance; + + public DefaultExpressionParserProxy(ConversionService conversionService) { + this.conversionService = conversionService; + } + + public Expression parseExpression(String expressionString, ParserContext context) throws ParserException { + synchronized (instance) { + if (instance == null) { + instance = createDefaultExpressionParser(conversionService); + } + } + return instance.parseExpression(expressionString, context); + } + } } \ No newline at end of file