Author: hardy.ferentschik Date: 2009-03-06 11:29:18 -0500 (Fri, 06 Mar 2009) New Revision: 16097 Added: validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/ValidationMessages.java Removed: validator/trunk/hibernate-validator/src/test/resources/org/hibernate/validation/ValidationMessages_de.properties Modified: validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/ResourceBundleMessageInterpolator.java validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/ResourceBundleMessageInterpolatorTest.java validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/util/performance/ValidationPerformace.java Log: HV-102 Updated interpolation according to latest algorithm. Modified: validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/ResourceBundleMessageInterpolator.java =================================================================== --- validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/ResourceBundleMessageInterpolator.java 2009-03-06 11:18:15 UTC (rev 16096) +++ validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/ResourceBundleMessageInterpolator.java 2009-03-06 16:29:18 UTC (rev 16097) @@ -17,11 +17,11 @@ */ package org.hibernate.validation.engine; -import java.util.HashMap; import java.util.Locale; import java.util.Map; import java.util.MissingResourceException; import java.util.ResourceBundle; +import java.util.concurrent.ConcurrentHashMap; import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.validation.ConstraintDescriptor; @@ -45,16 +45,22 @@ /** * Regular expression used to do message interpolation. */ - private static final Pattern messagePattern = Pattern.compile( "\\{([\\w\\.]+)\\}" ); + private static final Pattern messageParameterPattern = Pattern.compile( "(\\{[\\w\\.]+\\})" ); /** * The default locale for the current user. */ private final Locale defaultLocale; - private final Map<Locale, ResourceBundle> userBundlesMap = new HashMap<Locale, ResourceBundle>(); + /** + * User specified resource bundles hashed against their locale. + */ + private final Map<Locale, ResourceBundle> userBundlesMap = new ConcurrentHashMap<Locale, ResourceBundle>(); - private final Map<Locale, ResourceBundle> defaultBundlesMap = new HashMap<Locale, ResourceBundle>(); + /** + * Builtin resource bundles hashed against there locale. + */ + private final Map<Locale, ResourceBundle> defaultBundlesMap = new ConcurrentHashMap<Locale, ResourceBundle>(); public ResourceBundleMessageInterpolator() { this( null ); @@ -66,7 +72,9 @@ if ( resourceBundle == null ) { ResourceBundle bundle = getFileBasedResourceBundle( defaultLocale ); - userBundlesMap.put( defaultLocale, bundle ); + if ( bundle != null ) { + userBundlesMap.put( defaultLocale, bundle ); + } } else { @@ -76,17 +84,71 @@ defaultBundlesMap.put( defaultLocale, ResourceBundle.getBundle( DEFAULT_VALIDATION_MESSAGES, defaultLocale ) ); } - public String interpolate(String message, ConstraintDescriptor constraintDescriptor, Object value) { + /** + * {@inheritDoc} + */ + public String interpolate(String message, ConstraintDescriptor<?> constraintDescriptor, Object value) { // probably no need for caching, but it could be done by parameters since the map // is immutable and uniquely built per Validation definition, the comparaison has to be based on == and not equals though - return replace( message, constraintDescriptor.getParameters(), defaultLocale ); + return interpolateMessage( message, constraintDescriptor.getParameters(), defaultLocale ); } - public String interpolate(String message, ConstraintDescriptor constraintDescriptor, Object value, Locale locale) { - return replace( message, constraintDescriptor.getParameters(), locale ); + /** + * {@inheritDoc} + */ + public String interpolate(String message, ConstraintDescriptor<?> constraintDescriptor, Object value, Locale locale) { + return interpolateMessage( message, constraintDescriptor.getParameters(), locale ); } /** + * Runs the message interpolation according to alogrithm specified in JSR 303. + * <br/> + * Note: + * <br/> + * Lookups in user bundles is recursive whereas lookups in default bundle are not! + * + * @param message the message to interpolate + * @param annotationParameters the parameters of the annotation for which to interpolate this message + * @param locale the <code>Locale</code> to use for the resource bundle. + * + * @return the interpolated message. + */ + private String interpolateMessage(String message, Map<String, Object> annotationParameters, Locale locale) { + ResourceBundle userResourceBundle = findUserResourceBundle( locale ); + ResourceBundle defaultResourceBundle = findDefaultResourceBundle( locale ); + + String userBundleResolvedMessage; + String resolvedMessage = message; + boolean evaluatedDefaultBundleOnce = false; + do { + // search the user bundle recursive (step1) + userBundleResolvedMessage = replaceVariables( + resolvedMessage, userResourceBundle, locale, true + ); + + // exit condition - we have at least tried to vaidate against the default bundle and there was no + // further replacements + if ( evaluatedDefaultBundleOnce + && !hasReplacementTakenPlace( userBundleResolvedMessage, resolvedMessage ) ) { + break; + } + + // search the default bundle non recursive (step2) + resolvedMessage = replaceVariables( userBundleResolvedMessage, defaultResourceBundle, locale, false ); + + evaluatedDefaultBundleOnce = true; + } while ( true ); + + // resolve annotation attributes (step 4) + resolvedMessage = replaceAnnotationAttributes( resolvedMessage, annotationParameters ); + return resolvedMessage; + } + + private boolean hasReplacementTakenPlace(String origMessage, String newMessage) { + return !origMessage.equals( newMessage ); + } + + /** * Search current thread classloader for the resource bundle. If not found, search validator (this) classloader. * * @param locale The locale of the bundle to load. @@ -128,49 +190,65 @@ return rb; } - private String replace(String message, Map<String, Object> parameters, Locale locale) { - Matcher matcher = messagePattern.matcher( message ); + private String replaceVariables(String message, ResourceBundle bundle, Locale locale, boolean recurse) { + Matcher matcher = messageParameterPattern.matcher( message ); StringBuffer sb = new StringBuffer(); + String resolvedParameterValue; while ( matcher.find() ) { - matcher.appendReplacement( sb, resolveParameter( matcher.group( 1 ), parameters, locale ) ); + String parameter = matcher.group( 1 ); + resolvedParameterValue = resolveParameter( + parameter, bundle, locale, recurse + ); + + matcher.appendReplacement( sb, resolvedParameterValue ); } matcher.appendTail( sb ); return sb.toString(); } - private String resolveParameter(String token, Map<String, Object> parameters, Locale locale) { - Object variable = parameters.get( token ); - if ( variable != null ) { - return variable.toString(); + private String replaceAnnotationAttributes(String message, Map<String, Object> annotationParameters) { + Matcher matcher = messageParameterPattern.matcher( message ); + StringBuffer sb = new StringBuffer(); + while ( matcher.find() ) { + String resolvedParameterValue; + String parameter = matcher.group( 1 ); + Object variable = annotationParameters.get( removeCurlyBrace( parameter ) ); + if ( variable != null ) { + resolvedParameterValue = variable.toString(); + } + else { + resolvedParameterValue = message; + } + matcher.appendReplacement( sb, resolvedParameterValue ); } + matcher.appendTail( sb ); + return sb.toString(); + } - ResourceBundle userResourceBundle = findUserResourceBundle( locale ); - ResourceBundle defaultResourceBundle = findDefaultResourceBundle( locale ); - - StringBuffer buffer = new StringBuffer(); - String string = null; + private String resolveParameter(String parameterName, ResourceBundle bundle, Locale locale, boolean recurse) { + String parameterValue; try { - string = userResourceBundle != null ? userResourceBundle.getString( token ) : null; - } - catch ( MissingResourceException e ) { - //give a second chance with the default resource bundle - } - if ( string == null ) { - try { - string = defaultResourceBundle.getString( token ); + if ( bundle != null ) { + parameterValue = bundle.getString( removeCurlyBrace( parameterName ) ); + if ( recurse ) { + parameterValue = replaceVariables( parameterValue, bundle, locale, recurse ); + } } - catch ( MissingResourceException e ) { - //return the unchanged string - buffer.append( "{" ).append( token ).append( '}' ); + else { + parameterValue = parameterName; } } - if ( string != null ) { - // call resolve recusively! - buffer.append( replace( string, parameters, locale ) ); + catch ( MissingResourceException e ) { + // return parameter itself + parameterValue = parameterName; } - return buffer.toString(); + return parameterValue; } + private String removeCurlyBrace(String parameter) { + return parameter.substring( 1, parameter.length() - 1 ); + } + private ResourceBundle findDefaultResourceBundle(Locale locale) { if ( defaultBundlesMap.containsKey( locale ) ) { return defaultBundlesMap.get( locale ); @@ -187,7 +265,9 @@ } ResourceBundle bundle = getFileBasedResourceBundle( locale ); - userBundlesMap.put( locale, bundle ); + if ( bundle != null ) { + userBundlesMap.put( locale, bundle ); + } return bundle; } } Added: validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/ValidationMessages.java =================================================================== --- validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/ValidationMessages.java (rev 0) +++ validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/ValidationMessages.java 2009-03-06 16:29:18 UTC (rev 16097) @@ -0,0 +1,71 @@ +// $Id:$ +/* +* JBoss, Home of Professional Open Source +* Copyright 2008, Red Hat Middleware LLC, and individual contributors +* by the @authors tag. See the copyright.txt in the distribution for a +* full listing of individual contributors. +* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* http://www.apache.org/licenses/LICENSE-2.0 +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ +package org.hibernate.validation; + +import java.io.IOException; +import java.io.InputStream; +import java.util.Enumeration; +import java.util.HashMap; +import java.util.Map; +import java.util.PropertyResourceBundle; +import java.util.ResourceBundle; + +import org.slf4j.Logger; + +import org.hibernate.validation.util.LoggerFactory; + +/** + * @author Hardy Ferentschik + */ +public class ValidationMessages extends ResourceBundle { + + private static final Logger log = LoggerFactory.make(); + + private static final String DEFAULT_PROPERTIES_FILE_NAME = "/org/hibernate/validation/ValidationMessages.properties"; + + private Map<String, String> messages = new HashMap<String, String>(); + + public ValidationMessages() throws Exception { + + log.info( "For test purposes are we proxying the buildin messages!" ); + addTestPropertiesToBundle(); + log.info( "Adding the following properties to default properties {}", messages ); + + loadDefaultValidationProperties(); + } + + private void addTestPropertiesToBundle() { + messages.put( "replace.in.default.bundle1", "{replace.in.default.bundle2}" ); + messages.put( "replace.in.default.bundle2", "foobar" ); + } + + private void loadDefaultValidationProperties() throws IOException { + InputStream in = this.getClass() + .getResourceAsStream( DEFAULT_PROPERTIES_FILE_NAME ); + PropertyResourceBundle propertyBundle = new PropertyResourceBundle( in ); + setParent( propertyBundle ); + } + + protected Object handleGetObject(String key) { + return messages.get( key ); + } + + public Enumeration<String> getKeys() { + throw new RuntimeException( "Not needed for testing purposes." ); + } +} Modified: validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/ResourceBundleMessageInterpolatorTest.java =================================================================== --- validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/ResourceBundleMessageInterpolatorTest.java 2009-03-06 11:18:15 UTC (rev 16096) +++ validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/ResourceBundleMessageInterpolatorTest.java 2009-03-06 16:29:18 UTC (rev 16097) @@ -26,6 +26,7 @@ import java.util.ResourceBundle; import javax.validation.constraints.NotNull; import javax.validation.constraints.Size; +import javax.validation.constraints.Max; import static org.junit.Assert.assertEquals; import org.junit.Before; @@ -47,8 +48,7 @@ @Before public void setUp() { - interpolator = new ResourceBundleMessageInterpolator( new TestResources() ); - + // Create some annotations for testing using AnnotationProxies AnnotationDescriptor<NotNull> descriptor = new AnnotationDescriptor<NotNull>( NotNull.class ); notNull = AnnotationFactory.create( descriptor ); @@ -62,6 +62,8 @@ notNull, new Class<?>[] { }, new ConstraintHelper() ); + interpolator = new ResourceBundleMessageInterpolator( new TestResourceBundle() ); + String expected = "replacement worked"; String actual = interpolator.interpolate( "{foo}", descriptor, null ); assertEquals( "Wrong substitution", expected, actual ); @@ -84,6 +86,9 @@ ConstraintDescriptorImpl<NotNull> descriptor = new ConstraintDescriptorImpl<NotNull>( notNull, new Class<?>[] { }, new ConstraintHelper() ); + + interpolator = new ResourceBundleMessageInterpolator( new TestResourceBundle() ); + String expected = "foo"; // missing {} String actual = interpolator.interpolate( "foo", descriptor, null ); assertEquals( "Wrong substitution", expected, actual ); @@ -98,6 +103,9 @@ ConstraintDescriptorImpl<NotNull> descriptor = new ConstraintDescriptorImpl<NotNull>( notNull, new Class<?>[] { }, new ConstraintHelper() ); + + interpolator = new ResourceBundleMessageInterpolator( new TestResourceBundle() ); + String expected = "{bar}"; // unkown token {} String actual = interpolator.interpolate( "{bar}", descriptor, null ); assertEquals( "Wrong substitution", expected, actual ); @@ -108,6 +116,9 @@ ConstraintDescriptorImpl<NotNull> descriptor = new ConstraintDescriptorImpl<NotNull>( notNull, new Class<?>[] { }, new ConstraintHelper() ); + + interpolator = new ResourceBundleMessageInterpolator( new TestResourceBundle() ); + String expected = "may not be null"; String actual = interpolator.interpolate( notNull.message(), descriptor, null ); assertEquals( "Wrong substitution", expected, actual ); @@ -122,12 +133,12 @@ @Test public void testMessageInterpolationWithLocale() { - interpolator = new ResourceBundleMessageInterpolator(); - ConstraintDescriptorImpl<NotNull> descriptor = new ConstraintDescriptorImpl<NotNull>( notNull, new Class<?>[] { }, new ConstraintHelper() ); + interpolator = new ResourceBundleMessageInterpolator(); + String expected = "kann nicht null sein"; String actual = interpolator.interpolate( notNull.message(), descriptor, null, Locale.GERMAN ); assertEquals( "Wrong substitution", expected, actual ); @@ -135,12 +146,12 @@ @Test public void testFallbackToDefaultLocale() { - interpolator = new ResourceBundleMessageInterpolator(); - ConstraintDescriptorImpl<NotNull> descriptor = new ConstraintDescriptorImpl<NotNull>( notNull, new Class<?>[] { }, new ConstraintHelper() ); + interpolator = new ResourceBundleMessageInterpolator(); + String expected = "may not be null"; String actual = interpolator.interpolate( notNull.message(), descriptor, null, Locale.JAPAN ); assertEquals( "Wrong substitution", expected, actual ); @@ -148,25 +159,53 @@ @Test public void testUserResourceBundle() { - interpolator = new ResourceBundleMessageInterpolator(); - ConstraintDescriptorImpl<NotNull> descriptor = new ConstraintDescriptorImpl<NotNull>( notNull, new Class<?>[] { }, new ConstraintHelper() ); + interpolator = new ResourceBundleMessageInterpolator(); + String expected = "no puede ser null"; String actual = interpolator.interpolate( notNull.message(), descriptor, null, new Locale( "es", "ES" ) ); assertEquals( "Wrong substitution", expected, actual ); } - class TestResources extends ResourceBundle implements Enumeration<String> { + /** + * HV-102 + */ + @Test + public void testRecursiveMessageInterpoliation() { + AnnotationDescriptor<Max> descriptor = new AnnotationDescriptor<Max>( Max.class ); + descriptor.setValue( "message", "{replace.in.user.bundle1}" ); + descriptor.setValue( "value", 10l); + Max max = AnnotationFactory.create( descriptor ); + + + ConstraintDescriptorImpl<Max> constraintDescriptor = new ConstraintDescriptorImpl<Max>( + max, new Class<?>[] { }, new ConstraintHelper() + ); + + interpolator = new ResourceBundleMessageInterpolator( new TestResourceBundle() ); + + String expected = "{replace.in.default.bundle2}"; + String actual = interpolator.interpolate( max.message(), constraintDescriptor, null ); + assertEquals( "Within default bundle replacement parameter evauation should not be recursive!", expected, actual ); + } + + /** + * A dummy resource bundle which can be passed to the constructor of ResourceBundleMessageInterpolator to replace + * the user specified resource bundle. + */ + class TestResourceBundle extends ResourceBundle implements Enumeration<String> { private Map<String, String> testResources; Iterator<String> iter; - public TestResources() { + public TestResourceBundle() { testResources = new HashMap<String, String>(); // add some test messages testResources.put( "foo", "replacement worked" ); + testResources.put( "replace.in.user.bundle1", "{replace.in.user.bundle2}" ); + testResources.put( "replace.in.user.bundle2", "{replace.in.default.bundle1}" ); iter = testResources.keySet().iterator(); } Modified: validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/util/performance/ValidationPerformace.java =================================================================== --- validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/util/performance/ValidationPerformace.java 2009-03-06 11:18:15 UTC (rev 16096) +++ validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/util/performance/ValidationPerformace.java 2009-03-06 16:29:18 UTC (rev 16097) @@ -1,4 +1,4 @@ -// $Id:$ +// $Id$ /* * JBoss, Home of Professional Open Source * Copyright 2008, Red Hat Middleware LLC, and individual contributors @@ -24,6 +24,7 @@ import junit.framework.Test; import org.hibernate.validation.engine.ValidatorImplTest; +import org.hibernate.validation.engine.ResourceBundleMessageInterpolatorTest; /** * Work in progress. Timings have no relevance. Using this class one can verify if applied changes affect the @@ -37,8 +38,8 @@ long maxTimeInMillis = 1000; List<Class<?>> testClasses = new ArrayList<Class<?>>(); - testClasses.add( ValidatorImplTest.class ); - //testClasses.add( GroupTest.class ); + //testClasses.add( ValidatorImplTest.class ); + testClasses.add( ResourceBundleMessageInterpolatorTest.class ); //testClasses.add( ConstraintCompositionTest.class ); Test test = new JUnit4TestFactory( testClasses ).makeTestSuite(); return new TimedTest( test, maxTimeInMillis ); Deleted: validator/trunk/hibernate-validator/src/test/resources/org/hibernate/validation/ValidationMessages_de.properties =================================================================== --- validator/trunk/hibernate-validator/src/test/resources/org/hibernate/validation/ValidationMessages_de.properties 2009-03-06 11:18:15 UTC (rev 16096) +++ validator/trunk/hibernate-validator/src/test/resources/org/hibernate/validation/ValidationMessages_de.properties 2009-03-06 16:29:18 UTC (rev 16097) @@ -1,2 +0,0 @@ -# $Id: ValidationMessages.properties 15846 2009-02-02 11:56:15Z hardy.ferentschik $ -validator.notNull=kann nicht null sein