diff --git a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/util/PessimisticNumberParser.java b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/util/PessimisticNumberParser.java new file mode 100644 index 0000000000..996eaf5fed --- /dev/null +++ b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/util/PessimisticNumberParser.java @@ -0,0 +1,75 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2014, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * This copyrighted material is made available to anyone wishing to use, modify, + * copy, or redistribute it subject to the terms and conditions of the GNU + * Lesser General Public License, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.jpa.internal.util; + +/** + * An old-style query might pass positional numbers of Query parameters as strings. This implies we always need to + * attempt parsing string parameters to see if this deprecated feature is being used, but parsing leads to catching (and + * ignoring) NumberFormatException at runtime which is a performance problem, especially as the parse would fail + * each time a non-deprecated form is processed. + * This class is meant to avoid the need to allocate these exceptions at runtime. + * + * Use this class to convert String to Integer when it's unlikely to be successful: if you expect it to be a normal number, + * for example when a non successful parsing would be an error, using this utility is just an overhead. + * + * @author Sanne Grinovero + */ +public final class PessimisticNumberParser { + + private PessimisticNumberParser() { + //not to be constructed + } + + public static Integer toNumberOrNull(final String parameterName) { + if ( isValidNumber( parameterName ) ) { + try { + return Integer.valueOf( parameterName ); + } + catch (NumberFormatException e) { + //It wasn't valid after all, so return null + } + } + return null; + } + + private static boolean isValidNumber(final String parameterName) { + if ( parameterName.length() == 0 ) { + return false; + } + final char firstDigit = parameterName.charAt( 0 ); + if ( Character.isDigit( firstDigit ) || '-' == firstDigit || '+' == firstDigit ) { + //check the remaining characters + for ( int i = 1; i < parameterName.length(); i++ ) { + if ( !Character.isDigit( parameterName.charAt( i ) ) ) { + return false; + } + } + //Some edge cases are left open: just a sign would return true. + //For those cases you'd have a NumberFormatException swallowed. + return true; + } + return false; + } + +} diff --git a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/spi/BaseQueryImpl.java b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/spi/BaseQueryImpl.java index 5643204579..94963b69cc 100644 --- a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/spi/BaseQueryImpl.java +++ b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/spi/BaseQueryImpl.java @@ -51,6 +51,7 @@ import org.hibernate.jpa.internal.EntityManagerMessageLogger; import org.hibernate.jpa.internal.util.CacheModeHelper; import org.hibernate.jpa.internal.util.ConfigurationHelper; import org.hibernate.jpa.internal.util.LockModeTypeHelper; +import org.hibernate.jpa.internal.util.PessimisticNumberParser; import org.hibernate.procedure.NoSuchParameterException; import org.hibernate.procedure.ParameterStrategyException; @@ -493,7 +494,7 @@ public abstract class BaseQueryImpl implements Query { } // legacy allowance of the application to access the parameter using the position as a String - final Integer jpaPositionalParameter = toNumberOrNull( parameterName ); + final Integer jpaPositionalParameter = PessimisticNumberParser.toNumberOrNull( parameterName ); if ( jpaPositionalParameter != null ) { for ( ParameterRegistration param : parameterRegistrations ) { if ( param.isJpaPositionalParameter() && jpaPositionalParameter.equals( param.getPosition() ) ) { @@ -506,22 +507,6 @@ public abstract class BaseQueryImpl implements Query { throw new IllegalArgumentException( "Parameter with that name [" + parameterName + "] did not exist" ); } - private Integer toNumberOrNull(String parameterName) { - // HHH-9294 Quick check if string is made of digits to avoid the overhead of throwing an exception - for(int i = 0; i < parameterName.length(); i++) { - if ( !Character.isDigit( parameterName.charAt( i ) ) ) { - return null; - } - } - - try { - return Integer.valueOf( parameterName ); - } - catch (NumberFormatException e) { - return null; - } - } - @SuppressWarnings("unchecked") protected ParameterRegistration findParameterRegistration(int parameterPosition) { if ( parameterRegistrations != null ) { diff --git a/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/util/PessimisticNumberParserUnitTest.java b/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/util/PessimisticNumberParserUnitTest.java new file mode 100644 index 0000000000..0353174247 --- /dev/null +++ b/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/util/PessimisticNumberParserUnitTest.java @@ -0,0 +1,88 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2014, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * This copyrighted material is made available to anyone wishing to use, modify, + * copy, or redistribute it subject to the terms and conditions of the GNU + * Lesser General Public License, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.jpa.test.util; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + +import org.hibernate.jpa.internal.util.PessimisticNumberParser; +import org.junit.Test; + +/** + * @author Sanne Grinovero + */ +public class PessimisticNumberParserUnitTest { + + @Test + public void testEmptyStringBehaviour() { + assertNull( PessimisticNumberParser.toNumberOrNull( "" ) ); + } + + @Test + public void testPlusStringBehaviour() { + assertNull( PessimisticNumberParser.toNumberOrNull( "+" ) ); + } + + @Test + public void testMinusStringBehaviour() { + assertNull( PessimisticNumberParser.toNumberOrNull( "-" ) ); + } + + @Test + public void testLetterStringBehaviour() { + assertNull( PessimisticNumberParser.toNumberOrNull( "h" ) ); + } + + @Test + public void testTextStringBehaviour() { + assertNull( PessimisticNumberParser.toNumberOrNull( "hello world!" ) ); + } + + @Test + public void testFoolingPrefixStringBehaviour() { + assertNull( PessimisticNumberParser.toNumberOrNull( "+60000g" ) ); + } + + @Test + public void testFiveStringBehaviour() { + assertEquals( Integer.valueOf( 5 ), PessimisticNumberParser.toNumberOrNull( "5" ) ); + } + + @Test + public void testNegativeStringBehaviour() { + //technically illegal for the case of positional parameters, but we can parse it + assertEquals( Integer.valueOf( -25 ), PessimisticNumberParser.toNumberOrNull( "-25" ) ); + } + + @Test + public void testBigintegerStringBehaviour() { + assertEquals( Integer.valueOf( 60000 ), PessimisticNumberParser.toNumberOrNull( "60000" ) ); + } + + @Test + public void testPositiveStringBehaviour() { + assertEquals( Integer.valueOf( 60000 ), PessimisticNumberParser.toNumberOrNull( "+60000" ) ); + } + +}