HHH-4959 - Concurrent HQL parsing blocks on ReflectHelper.classForName()

vladmihalcea · vladmihalcea · commit 0bd7b8eac16e · 2016-12-14T10:05:30.000+02:00
Exclude JPQL and Criteria API aliases when searching for a Java constant value
diff --git a/documentation/src/main/asciidoc/userguide/appendices/Configurations.adoc b/documentation/src/main/asciidoc/userguide/appendices/Configurations.adoc
@@ -349,6 +349,15 @@ implementations that sacrifices performance optimizations to allow legacy 4.x li
 
 Legacy 4.x behavior favored performing pagination in-memory by avoiding the use of the offset value, which is overall poor performance.
 In 5.x, the limit handler behavior favors performance, thus, if the dialect doesn't support offsets, an exception is thrown instead.
+|`hibernate.query.conventional_java_constants` | `true` (default value) or `false` |
+Setting which indicates whether or not Java constant follow the https://docs.oracle.com/javase/tutorial/java/nutsandbolts/variables.html[Java Naming conventions].
+
+Default is `true`.
+Existing applications may want to disable this (set it `false`) if non-conventional Java constants are used.
+However, there is a significant performance overhead for using non-conventional Java constants
+since Hibernate cannot determine if aliases should be treated as Java constants or not.
+
+Check out https://hibernate.atlassian.net/browse/HHH-4959[HHH-4959] for more details.
 |===================================================================================================================================================================================================================================
 
 [[configurations-batch]]
diff --git a/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryBuilderImpl.java b/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryBuilderImpl.java
@@ -528,6 +528,7 @@ public static class SessionFactoryOptionsStateStandardImpl implements SessionFac
 		private Map querySubstitutions;
 		private boolean strictJpaQueryLanguageCompliance;
 		private boolean namedQueryStartupCheckingEnabled;
+		private boolean conventionalJavaConstants;
 		private final boolean procedureParameterNullPassingEnabled;
 		private final boolean collectionJoinSubqueryRewriteEnabled;
 
@@ -654,6 +655,8 @@ public SessionFactoryOptionsStateStandardImpl(StandardServiceRegistry serviceReg
 			this.querySubstitutions = ConfigurationHelper.toMap( QUERY_SUBSTITUTIONS, " ,=;:\n\t\r\f", configurationSettings );
 			this.strictJpaQueryLanguageCompliance = cfgService.getSetting( JPAQL_STRICT_COMPLIANCE, BOOLEAN, false );
 			this.namedQueryStartupCheckingEnabled = cfgService.getSetting( QUERY_STARTUP_CHECKING, BOOLEAN, true );
+			this.conventionalJavaConstants = cfgService.getSetting(
+					CONVENTIONAL_JAVA_CONSTANTS, BOOLEAN, true );
 			this.procedureParameterNullPassingEnabled = cfgService.getSetting( PROCEDURE_NULL_PARAM_PASSING, BOOLEAN, false );
 			this.collectionJoinSubqueryRewriteEnabled = cfgService.getSetting( COLLECTION_JOIN_SUBQUERY, BOOLEAN, true );
 
@@ -1051,6 +1054,10 @@ public boolean isNamedQueryStartupCheckingEnabled() {
 			return namedQueryStartupCheckingEnabled;
 		}
 
+		public boolean isConventionalJavaConstants() {
+			return conventionalJavaConstants;
+		}
+
 		@Override
 		public boolean isProcedureParameterNullPassingEnabled() {
 			return procedureParameterNullPassingEnabled;
@@ -1365,6 +1372,10 @@ public boolean isNamedQueryStartupCheckingEnabled() {
 		return options.isNamedQueryStartupCheckingEnabled();
 	}
 
+	public boolean isConventionalJavaConstants() {
+		return options.isConventionalJavaConstants();
+	}
+
 	@Override
 	public boolean isProcedureParameterNullPassingEnabled() {
 		return options.isProcedureParameterNullPassingEnabled();
diff --git a/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsImpl.java b/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsImpl.java
@@ -96,6 +96,7 @@ public class SessionFactoryOptionsImpl implements SessionFactoryOptions {
 	private final Map querySubstitutions;
 	private final boolean strictJpaQueryLanguageCompliance;
 	private final boolean namedQueryStartupCheckingEnabled;
+	private final boolean conventionalJavaConstants;
 	private final boolean procedureParameterNullPassingEnabled;
 	private final boolean collectionJoinSubqueryRewriteEnabled;
 
@@ -175,6 +176,7 @@ public SessionFactoryOptionsImpl(SessionFactoryOptionsState state) {
 		this.querySubstitutions = state.getQuerySubstitutions();
 		this.strictJpaQueryLanguageCompliance = state.isStrictJpaQueryLanguageCompliance();
 		this.namedQueryStartupCheckingEnabled = state.isNamedQueryStartupCheckingEnabled();
+		this.conventionalJavaConstants = state.isConventionalJavaConstants();
 		this.procedureParameterNullPassingEnabled = state.isProcedureParameterNullPassingEnabled();
 		this.collectionJoinSubqueryRewriteEnabled = state.isCollectionJoinSubqueryRewriteEnabled();
 
@@ -376,6 +378,11 @@ public boolean isNamedQueryStartupCheckingEnabled() {
 		return namedQueryStartupCheckingEnabled;
 	}
 
+	@Override
+	public boolean isConventionalJavaConstants() {
+		return conventionalJavaConstants;
+	}
+
 	@Override
 	public boolean isProcedureParameterNullPassingEnabled() {
 		return procedureParameterNullPassingEnabled;
diff --git a/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsState.java b/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsState.java
@@ -117,6 +117,8 @@ public interface SessionFactoryOptionsState {
 
 	boolean isNamedQueryStartupCheckingEnabled();
 
+	boolean isConventionalJavaConstants();
+
 	boolean isProcedureParameterNullPassingEnabled();
 
 	boolean isCollectionJoinSubqueryRewriteEnabled();
diff --git a/hibernate-core/src/main/java/org/hibernate/boot/spi/AbstractDelegatingSessionFactoryOptions.java b/hibernate-core/src/main/java/org/hibernate/boot/spi/AbstractDelegatingSessionFactoryOptions.java
@@ -214,6 +214,11 @@ public boolean isNamedQueryStartupCheckingEnabled() {
 		return delegate.isNamedQueryStartupCheckingEnabled();
 	}
 
+	@Override
+	public boolean isConventionalJavaConstants() {
+		return delegate.isConventionalJavaConstants();
+	}
+
 	@Override
 	public boolean isProcedureParameterNullPassingEnabled() {
 		return delegate.isProcedureParameterNullPassingEnabled();
diff --git a/hibernate-core/src/main/java/org/hibernate/boot/spi/SessionFactoryOptions.java b/hibernate-core/src/main/java/org/hibernate/boot/spi/SessionFactoryOptions.java
@@ -147,6 +147,8 @@ default boolean isAllowRefreshDetachedEntity() {
 
 	boolean isNamedQueryStartupCheckingEnabled();
 
+	boolean isConventionalJavaConstants();
+
 	boolean isSecondLevelCacheEnabled();
 
 	boolean isQueryCacheEnabled();
diff --git a/hibernate-core/src/main/java/org/hibernate/cfg/AvailableSettings.java b/hibernate-core/src/main/java/org/hibernate/cfg/AvailableSettings.java
@@ -801,6 +801,17 @@ public interface AvailableSettings {
 	 */
 	String QUERY_STARTUP_CHECKING = "hibernate.query.startup_check";
 
+	/**
+	 * Setting which indicates whether or not Java constant follow the Java Naming conventions.
+	 * <p/>
+	 * Default is {@code true}. Existing applications may want to disable this (set it {@code false}) if non-conventional Java constants are used.
+	 * However, there is a significant performance overhead for using non-conventional Java constants since Hibernate cannot determine if aliases
+	 * should be treated as Java constants or not.
+	 *
+	 * @since 5.2
+	 */
+	String CONVENTIONAL_JAVA_CONSTANTS = "hibernate.query.conventional_java_constants";
+
 	/**
 	 * The {@link org.hibernate.exception.spi.SQLExceptionConverter} to use for converting SQLExceptions
 	 * to Hibernate's JDBCException hierarchy.  The default is to use the configured
diff --git a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/QueryTranslatorImpl.java b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/QueryTranslatorImpl.java
@@ -18,7 +18,6 @@
 import org.hibernate.HibernateException;
 import org.hibernate.MappingException;
 import org.hibernate.QueryException;
-import org.hibernate.boot.registry.classloading.spi.ClassLoaderService;
 import org.hibernate.engine.query.spi.EntityGraphQueryHint;
 import org.hibernate.engine.spi.QueryParameters;
 import org.hibernate.engine.spi.RowSelection;
@@ -590,7 +589,6 @@ public static class JavaConstantConverter implements NodeTraverser.VisitationStr
 		private AST dotRoot;
 
 		public JavaConstantConverter(SessionFactoryImplementor factory) {
-
 			this.factory = factory;
 		}
 
@@ -612,7 +610,7 @@ public void visit(AST node) {
 		}
 		private void handleDotStructure(AST dotStructureRoot) {
 			final String expression = ASTUtil.getPathText( dotStructureRoot );
-			final Object constant = ReflectHelper.getConstantValue( expression, factory.getServiceRegistry().getService( ClassLoaderService.class ) );
+			final Object constant = ReflectHelper.getConstantValue( expression, factory );
 			if ( constant != null ) {
 				dotStructureRoot.setFirstChild( null );
 				dotStructureRoot.setType( HqlTokenTypes.JAVA_CONSTANT );
diff --git a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/JavaConstantNode.java b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/JavaConstantNode.java
@@ -9,7 +9,6 @@
 import java.util.Locale;
 
 import org.hibernate.QueryException;
-import org.hibernate.boot.registry.classloading.spi.ClassLoaderService;
 import org.hibernate.dialect.Dialect;
 import org.hibernate.engine.spi.SessionFactoryImplementor;
 import org.hibernate.hql.spi.QueryTranslator;
@@ -39,7 +38,7 @@ public void setText(String s) {
 		// this method to get called twice.  The first time with an empty string
 		if ( StringHelper.isNotEmpty( s ) ) {
 			constantExpression = s;
-			constantValue = ReflectHelper.getConstantValue( s, factory.getServiceRegistry().getService( ClassLoaderService.class ) );
+			constantValue = ReflectHelper.getConstantValue( s, factory );
 			heuristicType = factory.getTypeResolver().heuristicType( constantValue.getClass().getName() );
 			super.setText( s );
 		}
diff --git a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/util/LiteralProcessor.java b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/util/LiteralProcessor.java
@@ -13,13 +13,11 @@
 import org.hibernate.HibernateException;
 import org.hibernate.MappingException;
 import org.hibernate.QueryException;
-import org.hibernate.boot.registry.classloading.spi.ClassLoaderService;
 import org.hibernate.dialect.Dialect;
 import org.hibernate.hql.internal.antlr.HqlSqlTokenTypes;
 import org.hibernate.hql.internal.antlr.SqlTokenTypes;
 import org.hibernate.hql.internal.ast.HqlSqlWalker;
 import org.hibernate.hql.internal.ast.InvalidPathException;
-import org.hibernate.hql.internal.ast.tree.BooleanLiteralNode;
 import org.hibernate.hql.internal.ast.tree.DotNode;
 import org.hibernate.hql.internal.ast.tree.FromClause;
 import org.hibernate.hql.internal.ast.tree.IdentNode;
@@ -35,7 +33,6 @@
 
 import antlr.SemanticException;
 import antlr.collections.AST;
-import java.util.Locale;
 
 /**
  * A delegate that handles literals and constants for HqlSqlWalker, performing the token replacement functions and
@@ -109,7 +106,7 @@ public void lookupConstant(DotNode node) throws SemanticException {
 			setSQLValue( node, text, discrim );
 		}
 		else {
-			Object value = ReflectHelper.getConstantValue( text, walker.getSessionFactoryHelper().getFactory().getServiceRegistry().getService( ClassLoaderService.class ) );
+			Object value = ReflectHelper.getConstantValue( text, walker.getSessionFactoryHelper().getFactory() );
 			if ( value == null ) {
 				throw new InvalidPathException( "Invalid path: '" + text + "'" );
 			}
diff --git a/hibernate-core/src/main/java/org/hibernate/hql/internal/classic/WhereParser.java b/hibernate-core/src/main/java/org/hibernate/hql/internal/classic/WhereParser.java
@@ -16,7 +16,6 @@
 
 import org.hibernate.MappingException;
 import org.hibernate.QueryException;
-import org.hibernate.boot.registry.classloading.spi.ClassLoaderService;
 import org.hibernate.engine.internal.JoinSequence;
 import org.hibernate.hql.spi.QueryTranslator;
 import org.hibernate.internal.util.ReflectHelper;
@@ -419,7 +418,7 @@ else if ( token.startsWith( ParserHelper.HQL_VARIABLE_PREFIX ) ) { //named query
 				Object constant;
 				if (
 						token.indexOf( '.' ) > -1 &&
-						( constant = ReflectHelper.getConstantValue( token, q.getFactory().getServiceRegistry().getService( ClassLoaderService.class ) ) ) != null
+						( constant = ReflectHelper.getConstantValue( token, q.getFactory() ) ) != null
 				) {
 					Type type;
 					try {
diff --git a/hibernate-core/src/main/java/org/hibernate/internal/util/ReflectHelper.java b/hibernate-core/src/main/java/org/hibernate/internal/util/ReflectHelper.java
@@ -13,12 +13,14 @@
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.util.Locale;
+import java.util.regex.Pattern;
 
 import org.hibernate.AssertionFailure;
 import org.hibernate.MappingException;
 import org.hibernate.PropertyNotFoundException;
 import org.hibernate.boot.registry.classloading.spi.ClassLoaderService;
 import org.hibernate.boot.registry.classloading.spi.ClassLoadingException;
+import org.hibernate.engine.spi.SessionFactoryImplementor;
 import org.hibernate.property.access.internal.PropertyAccessStrategyMixedImpl;
 import org.hibernate.property.access.spi.Getter;
 import org.hibernate.type.PrimitiveType;
@@ -32,6 +34,10 @@
  */
 @SuppressWarnings("unchecked")
 public final class ReflectHelper {
+
+	private static final Pattern JAVA_CONSTANT_PATTERN = Pattern.compile(
+			"[a-z]+\\.([A-Z]{1}[a-z]+)+\\$?([A-Z]{1}[a-z]+)*\\.[A-Z_\\$]+" );
+
 	public static final Class[] NO_PARAM_SIGNATURE = new Class[0];
 	public static final Object[] NO_PARAMS = new Object[0];
 
@@ -229,9 +235,15 @@ private static Getter getter(Class clazz, String name) throws MappingException {
 		return PropertyAccessStrategyMixedImpl.INSTANCE.buildPropertyAccess( clazz, name ).getGetter();
 	}
 
-	public static Object getConstantValue(String name, ClassLoaderService classLoaderService) {
+	public static Object getConstantValue(String name, SessionFactoryImplementor factory) {
+		boolean conventionalJavaConstants = factory.getSessionFactoryOptions().isConventionalJavaConstants();
 		Class clazz;
 		try {
+			if ( conventionalJavaConstants &&
+				!JAVA_CONSTANT_PATTERN.matcher( name ).find() ) {
+				return null;
+			}
+			ClassLoaderService classLoaderService = factory.getServiceRegistry().getService( ClassLoaderService.class );
 			clazz = classLoaderService.classForName( StringHelper.qualifier( name ) );
 		}
 		catch ( Throwable t ) {
@@ -331,7 +343,7 @@ public static Constructor getConstructor(Class clazz, Type[] types) throws Prope
 		throw new PropertyNotFoundException( "no appropriate constructor in class: " + clazz.getName() );
 
 	}
-	
+
 	public static Method getMethod(Class clazz, Method method) {
 		try {
 			return clazz.getMethod( method.getName(), method.getParameterTypes() );
diff --git a/hibernate-core/src/test/java/org/hibernate/internal/util/ReflectHelperTest.java b/hibernate-core/src/test/java/org/hibernate/internal/util/ReflectHelperTest.java
