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

Exclude JPQL and Criteria API aliases when searching for a Java constant value
This commit is contained in:
Vlad Mihalcea 2016-12-14 09:54:33 +02:00
parent 159bc99a36
commit 0bd7b8eac1
13 changed files with 191 additions and 13 deletions

View File

@ -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]]

View File

@ -528,6 +528,7 @@ public class SessionFactoryBuilderImpl implements SessionFactoryBuilderImplement
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 class SessionFactoryBuilderImpl implements SessionFactoryBuilderImplement
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 class SessionFactoryBuilderImpl implements SessionFactoryBuilderImplement
return namedQueryStartupCheckingEnabled;
}
public boolean isConventionalJavaConstants() {
return conventionalJavaConstants;
}
@Override
public boolean isProcedureParameterNullPassingEnabled() {
return procedureParameterNullPassingEnabled;
@ -1365,6 +1372,10 @@ public class SessionFactoryBuilderImpl implements SessionFactoryBuilderImplement
return options.isNamedQueryStartupCheckingEnabled();
}
public boolean isConventionalJavaConstants() {
return options.isConventionalJavaConstants();
}
@Override
public boolean isProcedureParameterNullPassingEnabled() {
return options.isProcedureParameterNullPassingEnabled();

View File

@ -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 class SessionFactoryOptionsImpl implements SessionFactoryOptions {
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 class SessionFactoryOptionsImpl implements SessionFactoryOptions {
return namedQueryStartupCheckingEnabled;
}
@Override
public boolean isConventionalJavaConstants() {
return conventionalJavaConstants;
}
@Override
public boolean isProcedureParameterNullPassingEnabled() {
return procedureParameterNullPassingEnabled;

View File

@ -117,6 +117,8 @@ public interface SessionFactoryOptionsState {
boolean isNamedQueryStartupCheckingEnabled();
boolean isConventionalJavaConstants();
boolean isProcedureParameterNullPassingEnabled();
boolean isCollectionJoinSubqueryRewriteEnabled();

View File

@ -214,6 +214,11 @@ public class AbstractDelegatingSessionFactoryOptions implements SessionFactoryOp
return delegate.isNamedQueryStartupCheckingEnabled();
}
@Override
public boolean isConventionalJavaConstants() {
return delegate.isConventionalJavaConstants();
}
@Override
public boolean isProcedureParameterNullPassingEnabled() {
return delegate.isProcedureParameterNullPassingEnabled();

View File

@ -147,6 +147,8 @@ public interface SessionFactoryOptions {
boolean isNamedQueryStartupCheckingEnabled();
boolean isConventionalJavaConstants();
boolean isSecondLevelCacheEnabled();
boolean isQueryCacheEnabled();

View File

@ -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

View File

@ -18,7 +18,6 @@ import java.util.Set;
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 class QueryTranslatorImpl implements FilterTranslator {
private AST dotRoot;
public JavaConstantConverter(SessionFactoryImplementor factory) {
this.factory = factory;
}
@ -612,7 +610,7 @@ public class QueryTranslatorImpl implements FilterTranslator {
}
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 );

View File

@ -9,7 +9,6 @@ package org.hibernate.hql.internal.ast.tree;
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 class JavaConstantNode extends Node implements ExpectedTypeAwareNode, Ses
// 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 );
}

View File

@ -13,13 +13,11 @@ import java.text.DecimalFormat;
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 org.jboss.logging.Logger;
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 class LiteralProcessor implements HqlSqlTokenTypes {
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 + "'" );
}

View File

@ -16,7 +16,6 @@ import java.util.StringTokenizer;
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 @@ public class WhereParser implements Parser {
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 {

View File

@ -13,12 +13,14 @@ import java.lang.reflect.Member;
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 @@ import org.hibernate.type.Type;
*/
@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 @@ public final class ReflectHelper {
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 final class ReflectHelper {
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() );

View File

@ -0,0 +1,126 @@
/*
* Hibernate, Relational Persistence for Idiomatic Java
*
* License: GNU Lesser General Public License (LGPL), version 2.1 or later.
* See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
*/
package org.hibernate.internal.util;
import javax.persistence.FetchType;
import org.hibernate.boot.registry.classloading.spi.ClassLoaderService;
import org.hibernate.boot.spi.SessionFactoryOptions;
import org.hibernate.engine.spi.SessionFactoryImplementor;
import org.hibernate.service.spi.ServiceRegistryImplementor;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mockito;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
/**
* @author Vlad Mihalcea
*/
public class ReflectHelperTest {
public enum Status {
ON,
OFF
}
private SessionFactoryImplementor sessionFactoryImplementorMock;
private SessionFactoryOptions sessionFactoryOptionsMock;
private ServiceRegistryImplementor serviceRegistryMock;
private ClassLoaderService classLoaderServiceMock;
@Before
public void init() {
sessionFactoryImplementorMock = Mockito.mock(SessionFactoryImplementor.class);
sessionFactoryOptionsMock = Mockito.mock(SessionFactoryOptions.class);
when(sessionFactoryImplementorMock.getSessionFactoryOptions()).thenReturn( sessionFactoryOptionsMock );
serviceRegistryMock = Mockito.mock(ServiceRegistryImplementor.class);
when( sessionFactoryImplementorMock.getServiceRegistry() ).thenReturn( serviceRegistryMock );
classLoaderServiceMock = Mockito.mock(ClassLoaderService.class);
when( serviceRegistryMock.getService( eq( ClassLoaderService.class ) ) ).thenReturn( classLoaderServiceMock );
}
@Test
public void test_getConstantValue_simpleAlias() {
when( sessionFactoryOptionsMock.isConventionalJavaConstants() ).thenReturn( true );
Object value = ReflectHelper.getConstantValue( "alias.b", sessionFactoryImplementorMock);
assertNull(value);
verify(classLoaderServiceMock, never()).classForName( anyString() );
}
@Test
public void test_getConstantValue_simpleAlias_non_conventional() {
when( sessionFactoryOptionsMock.isConventionalJavaConstants() ).thenReturn( false );
Object value = ReflectHelper.getConstantValue( "alias.b", sessionFactoryImplementorMock);
assertNull(value);
verify(classLoaderServiceMock, times(1)).classForName( eq( "alias" ) );
}
@Test
public void test_getConstantValue_nestedAlias() {
when( sessionFactoryOptionsMock.isConventionalJavaConstants() ).thenReturn( true );
Object value = ReflectHelper.getConstantValue( "alias.b.c", sessionFactoryImplementorMock);
assertNull(value);
verify(classLoaderServiceMock, never()).classForName( anyString() );
}
@Test
public void test_getConstantValue_nestedAlias_non_conventional() {
when( sessionFactoryOptionsMock.isConventionalJavaConstants() ).thenReturn( false );
Object value = ReflectHelper.getConstantValue( "alias.b.c", sessionFactoryImplementorMock);
assertNull(value);
verify(classLoaderServiceMock, times(1)).classForName( eq( "alias.b" ) );
}
@Test
public void test_getConstantValue_outerEnum() {
when( sessionFactoryOptionsMock.isConventionalJavaConstants() ).thenReturn( true );
when( classLoaderServiceMock.classForName( "javax.persistence.FetchType" ) ).thenReturn( (Class) FetchType.class );
Object value = ReflectHelper.getConstantValue( "javax.persistence.FetchType.LAZY", sessionFactoryImplementorMock);
assertEquals( FetchType.LAZY, value );
verify(classLoaderServiceMock, times(1)).classForName( eq("javax.persistence.FetchType") );
}
@Test
public void test_getConstantValue_enumClass() {
when( sessionFactoryOptionsMock.isConventionalJavaConstants() ).thenReturn( true );
when( classLoaderServiceMock.classForName( "org.hibernate.internal.util.ReflectHelperTest$Status" ) ).thenReturn( (Class) Status.class );
Object value = ReflectHelper.getConstantValue( "org.hibernate.internal.util.ReflectHelperTest$Status", sessionFactoryImplementorMock);
assertNull(value);
verify(classLoaderServiceMock, never()).classForName( eq("org.hibernate.internal.util") );
}
@Test
public void test_getConstantValue_nestedEnum() {
when( sessionFactoryOptionsMock.isConventionalJavaConstants() ).thenReturn( true );
when( classLoaderServiceMock.classForName( "org.hibernate.internal.util.ReflectHelperTest$Status" ) ).thenReturn( (Class) Status.class );
Object value = ReflectHelper.getConstantValue( "org.hibernate.internal.util.ReflectHelperTest$Status.ON", sessionFactoryImplementorMock);
assertEquals( Status.ON, value );
verify(classLoaderServiceMock, times(1)).classForName( eq("org.hibernate.internal.util.ReflectHelperTest$Status") );
}
}