From d0059cdef99916e358d37f3d122f0e2b0660b551 Mon Sep 17 00:00:00 2001 From: Vlad Mihalcea Date: Wed, 14 Dec 2016 09:54:33 +0200 Subject: [PATCH 1/3] HHH-4959 - Concurrent HQL parsing blocks on ReflectHelper.classForName() Exclude JPQL and Criteria API aliases when searching for a Java constant value (cherry picked from commit 0bd7b8eac16e6966dea81b8a5991e052192a6af2) --- .../userguide/appendices/Configurations.adoc | 11 +- .../internal/SessionFactoryBuilderImpl.java | 11 ++ .../internal/SessionFactoryOptionsImpl.java | 7 + .../internal/SessionFactoryOptionsState.java | 2 + ...stractDelegatingSessionFactoryOptions.java | 5 + .../boot/spi/SessionFactoryOptions.java | 2 + .../org/hibernate/cfg/AvailableSettings.java | 11 ++ .../hql/internal/ast/QueryTranslatorImpl.java | 4 +- .../internal/ast/tree/JavaConstantNode.java | 3 +- .../internal/ast/util/LiteralProcessor.java | 5 +- .../hql/internal/classic/WhereParser.java | 3 +- .../internal/util/ReflectHelper.java | 16 ++- .../internal/util/ReflectHelperTest.java | 126 ++++++++++++++++++ 13 files changed, 192 insertions(+), 14 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/internal/util/ReflectHelperTest.java diff --git a/documentation/src/main/asciidoc/userguide/appendices/Configurations.adoc b/documentation/src/main/asciidoc/userguide/appendices/Configurations.adoc index 77e117abdee0..0d64ae8fe99c 100644 --- a/documentation/src/main/asciidoc/userguide/appendices/Configurations.adoc +++ b/documentation/src/main/asciidoc/userguide/appendices/Configurations.adoc @@ -290,6 +290,16 @@ Should we strictly adhere to JPA Query Language (JPQL) syntax, or more broadly s Setting this to `true` may cause valid HQL to throw an exception because it violates the JPQL subset. |`hibernate.query.startup_check` | `true` (default value) or `false` |Should named queries be checked during startup? +|`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. + |`hibernate.hql.bulk_id_strategy` | A fully-qualified class name, an instance, or a `Class` object reference |Provide a custom `org.hibernate.hql.spi.id.MultiTableBulkIdStrategy` implementation for handling multi-table bulk HQL operations. |`hibernate.proc.param_null_passing` | `true` or `false` (default value) | @@ -310,7 +320,6 @@ Can reference * `StatementInspector` instance * `StatementInspector` implementation {@link Class} reference * `StatementInspector` implementation class name (fully-qualified class name) - |=================================================================================================================================================================================================================================== [[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 index d085c8862f2c..a6ede7c0bf47 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryBuilderImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryBuilderImpl.java @@ -483,6 +483,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; @@ -600,6 +601,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 ); @@ -851,6 +854,10 @@ public boolean isNamedQueryStartupCheckingEnabled() { return namedQueryStartupCheckingEnabled; } + public boolean isConventionalJavaConstants() { + return conventionalJavaConstants; + } + @Override public boolean isProcedureParameterNullPassingEnabled() { return procedureParameterNullPassingEnabled; @@ -1132,6 +1139,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 index abb5ec698adb..fd8bfe03b909 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsImpl.java @@ -88,6 +88,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; @@ -160,6 +161,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(); @@ -339,6 +341,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 index 95fb243352ac..b36737911c3b 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsState.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsState.java @@ -99,6 +99,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 index 823fee5d26b9..0fb09844c915 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/spi/AbstractDelegatingSessionFactoryOptions.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/spi/AbstractDelegatingSessionFactoryOptions.java @@ -198,6 +198,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 index 7df039ed883f..dd55bd18bbaf 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/spi/SessionFactoryOptions.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/spi/SessionFactoryOptions.java @@ -122,6 +122,8 @@ public interface SessionFactoryOptions { 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 index 3d2dff72deec..1ff65da04cda 100644 --- a/hibernate-core/src/main/java/org/hibernate/cfg/AvailableSettings.java +++ b/hibernate-core/src/main/java/org/hibernate/cfg/AvailableSettings.java @@ -562,6 +562,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. + *

+ * 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 index 6479b4ac2c41..ff85b0433f97 100644 --- 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 @@ -19,7 +19,6 @@ import org.hibernate.MappingException; import org.hibernate.QueryException; import org.hibernate.ScrollableResults; -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 index a9613b235b33..9a99cc49e847 100644 --- 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 index e5da5ee836db..147531dd2c8e 100644 --- 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 index 1b0901983840..8faa0b1713fa 100644 --- 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 index d7116f507765..e5b847e7effe 100644 --- 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 new file mode 100644 index 000000000000..cf2e1cb3e09a --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/internal/util/ReflectHelperTest.java @@ -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 . + */ +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") ); + } +} \ No newline at end of file From 9e6585b8045f24d789d8ec0d1dfda30ded2163af Mon Sep 17 00:00:00 2001 From: Gail Badner Date: Tue, 9 Jan 2018 20:53:43 -0800 Subject: [PATCH 2/3] HHH-4959 : change default for hibernate.query.conventional_java_constants to false --- .../userguide/appendices/Configurations.adoc | 12 +- .../internal/SessionFactoryBuilderImpl.java | 2 +- .../org/hibernate/cfg/AvailableSettings.java | 15 ++- ...onConventionalJavaConstantAllowedTest.java | 98 ++++++++++++++++ ...onConventionalJavaConstantDefaultTest.java | 89 +++++++++++++++ ...onventionalJavaConstantDisallowedTest.java | 105 ++++++++++++++++++ 6 files changed, 311 insertions(+), 10 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/test/queryconstant/NonConventionalJavaConstantAllowedTest.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/queryconstant/NonConventionalJavaConstantDefaultTest.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/queryconstant/NonConventionalJavaConstantDisallowedTest.java diff --git a/documentation/src/main/asciidoc/userguide/appendices/Configurations.adoc b/documentation/src/main/asciidoc/userguide/appendices/Configurations.adoc index 0d64ae8fe99c..b4c26f170608 100644 --- a/documentation/src/main/asciidoc/userguide/appendices/Configurations.adoc +++ b/documentation/src/main/asciidoc/userguide/appendices/Configurations.adoc @@ -290,13 +290,15 @@ Should we strictly adhere to JPA Query Language (JPQL) syntax, or more broadly s Setting this to `true` may cause valid HQL to throw an exception because it violates the JPQL subset. |`hibernate.query.startup_check` | `true` (default value) or `false` |Should named queries be checked during startup? -|`hibernate.query.conventional_java_constants` | `true` (default value) or `false` | +|`hibernate.query.conventional_java_constants` | `true` or `false` (default value) | 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. +Default is `false`. +Existing applications may want to enable this (set it to `true`) if only conventional Java constants are used. + +There is a significant performance improvement by setting `hibernate.query.conventional_java_constants` to `true`, because Hibernate can determine if an alias should be treated as a Java constant by simply checking if the alias follows Java constant conventions. + +With `hibernate.query.conventional_java_constants` set to `false`, Hibernate determines if an alias should be treated as a Java constant by attempting to load the alias as a class (an expensive operation); if it fails, then Hibernate treats the alias as a Java constant. Check out https://hibernate.atlassian.net/browse/HHH-4959[HHH-4959] for more details. 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 index a6ede7c0bf47..46c8bd8fc800 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryBuilderImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryBuilderImpl.java @@ -602,7 +602,7 @@ public SessionFactoryOptionsStateStandardImpl(StandardServiceRegistry serviceReg 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 ); + CONVENTIONAL_JAVA_CONSTANTS, BOOLEAN, false ); this.procedureParameterNullPassingEnabled = cfgService.getSetting( PROCEDURE_NULL_PARAM_PASSING, BOOLEAN, false ); this.collectionJoinSubqueryRewriteEnabled = cfgService.getSetting( COLLECTION_JOIN_SUBQUERY, BOOLEAN, true ); diff --git a/hibernate-core/src/main/java/org/hibernate/cfg/AvailableSettings.java b/hibernate-core/src/main/java/org/hibernate/cfg/AvailableSettings.java index 1ff65da04cda..189175f90ac6 100644 --- a/hibernate-core/src/main/java/org/hibernate/cfg/AvailableSettings.java +++ b/hibernate-core/src/main/java/org/hibernate/cfg/AvailableSettings.java @@ -565,11 +565,18 @@ public interface AvailableSettings { /** * Setting which indicates whether or not Java constant follow the Java Naming conventions. *

- * 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. + * Default is {@code false}. Existing applications may want to enable this (set it to {@code true}) + * if only conventional Java constants are used. + *

+ * There is a significant performance improvement by setting {@code hibernate.query.conventional_java_constants} + * to {@code true}, because Hibernate can determine if an alias should be treated as a Java constant by simply + * checking if the alias follows Java constant conventions. + *

+ * With {@code hibernate.query.conventional_java_constants} set to {@code false}, Hibernate determines if an alias + * should be treated as a Java constant by attempting to load the alias as a class (an expensive operation); + * if it fails, then Hibernate treats the alias as a Java constant. * - * @since 5.2 + * @since 5.1.11, 5.2 */ String CONVENTIONAL_JAVA_CONSTANTS = "hibernate.query.conventional_java_constants"; diff --git a/hibernate-core/src/test/java/org/hibernate/test/queryconstant/NonConventionalJavaConstantAllowedTest.java b/hibernate-core/src/test/java/org/hibernate/test/queryconstant/NonConventionalJavaConstantAllowedTest.java new file mode 100644 index 000000000000..104e3e878abb --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/queryconstant/NonConventionalJavaConstantAllowedTest.java @@ -0,0 +1,98 @@ +/* + * 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 . + */ +package org.hibernate.test.queryconstant; + +import java.util.Map; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; + +import org.hibernate.Session; +import org.hibernate.cfg.AvailableSettings; + +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; + +/** + * @author Gail Badner + */ +public class NonConventionalJavaConstantAllowedTest extends BaseNonConfigCoreFunctionalTestCase { + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { AnEntity.class }; + } + + @Override + @SuppressWarnings("unchecked") + protected void addSettings(Map settings) { + super.addSettings( settings ); + settings.put( AvailableSettings.CONVENTIONAL_JAVA_CONSTANTS, "false" ); + } + + @Before + public void setup() { + Session s = openSession(); + s.getTransaction().begin(); + + AnEntity anEntity = new AnEntity(); + anEntity.operationType = OPERATION_TYPE.CREATE; + + s.persist( anEntity ); + + s.getTransaction().commit(); + s.clear(); + } + + @After + public void cleanup() { + Session s = openSession(); + s.getTransaction().begin(); + + s.createQuery( "delete from AnEntity" ).executeUpdate(); + + s.getTransaction().commit(); + s.clear(); + } + + + @Test + @TestForIssue( jiraKey = "HHH-4959") + public void testDisabled() { + assertFalse( sessionFactory().getSessionFactoryOptions().isConventionalJavaConstants() ); + + Session s = openSession(); + s.getTransaction().begin(); + + AnEntity anEntity = (AnEntity) s.createQuery( + "from AnEntity e where e.operationType = org.hibernate.test.queryconstant.NonConventionalJavaConstantDefaultTest$OPERATION_TYPE.CREATE" + ).uniqueResult(); + + assertNotNull( anEntity ); + s.getTransaction().commit(); + s.close(); + } + + public enum OPERATION_TYPE { + CREATE, UPDATE, DELETE + } + + @Entity(name = "AnEntity") + public static class AnEntity { + @Id + @GeneratedValue + private long id; + + OPERATION_TYPE operationType; + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/queryconstant/NonConventionalJavaConstantDefaultTest.java b/hibernate-core/src/test/java/org/hibernate/test/queryconstant/NonConventionalJavaConstantDefaultTest.java new file mode 100644 index 000000000000..72177d0029ed --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/queryconstant/NonConventionalJavaConstantDefaultTest.java @@ -0,0 +1,89 @@ +/* + * 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 . + */ +package org.hibernate.test.queryconstant; + +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; + +import org.hibernate.Session; + +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; + +/** + * @author Gail Badner + */ +public class NonConventionalJavaConstantDefaultTest extends BaseNonConfigCoreFunctionalTestCase { + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { AnEntity.class }; + } + + @Before + public void setup() { + Session s = openSession(); + s.getTransaction().begin(); + + AnEntity anEntity = new AnEntity(); + anEntity.operationType = OPERATION_TYPE.CREATE; + + s.persist( anEntity ); + + s.getTransaction().commit(); + s.clear(); + } + + @After + public void cleanup() { + Session s = openSession(); + s.getTransaction().begin(); + + s.createQuery( "delete from AnEntity" ).executeUpdate(); + + s.getTransaction().commit(); + s.clear(); + } + + @Test + @TestForIssue( jiraKey = "HHH-4959") + public void testDefault() { + // check default + assertFalse( sessionFactory().getSessionFactoryOptions().isConventionalJavaConstants() ); + + Session s = openSession(); + s.getTransaction().begin(); + + AnEntity anEntity = (AnEntity) s.createQuery( + "from AnEntity e where e.operationType = org.hibernate.test.queryconstant.NonConventionalJavaConstantDefaultTest$OPERATION_TYPE.CREATE" + ).uniqueResult(); + + assertNotNull( anEntity ); + s.getTransaction().commit(); + s.close(); + } + + public enum OPERATION_TYPE { + CREATE, UPDATE, DELETE + } + + @Entity(name = "AnEntity") + public static class AnEntity { + @Id + @GeneratedValue + private long id; + + OPERATION_TYPE operationType; + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/queryconstant/NonConventionalJavaConstantDisallowedTest.java b/hibernate-core/src/test/java/org/hibernate/test/queryconstant/NonConventionalJavaConstantDisallowedTest.java new file mode 100644 index 000000000000..74ecc63c1633 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/queryconstant/NonConventionalJavaConstantDisallowedTest.java @@ -0,0 +1,105 @@ +/* + * 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 . + */ +package org.hibernate.test.queryconstant; + +import java.util.Map; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; + +import org.hibernate.Session; +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.hql.internal.ast.QuerySyntaxException; + +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +/** + * @author Gail Badner + */ +public class NonConventionalJavaConstantDisallowedTest extends BaseNonConfigCoreFunctionalTestCase { + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { AnEntity.class }; + } + + @Override + @SuppressWarnings("unchecked") + protected void addSettings(Map settings) { + super.addSettings( settings ); + settings.put( AvailableSettings.CONVENTIONAL_JAVA_CONSTANTS, "true" ); + } + + @Before + public void setup() { + Session s = openSession(); + s.getTransaction().begin(); + + AnEntity anEntity = new AnEntity(); + anEntity.operationType = OPERATION_TYPE.CREATE; + + s.persist( anEntity ); + + s.getTransaction().commit(); + s.clear(); + } + + @After + public void cleanup() { + Session s = openSession(); + s.getTransaction().begin(); + + s.createQuery( "delete from AnEntity" ).executeUpdate(); + + s.getTransaction().commit(); + s.clear(); + } + + + @Test + @TestForIssue( jiraKey = "HHH-4959") + public void testEnabled() { + assertTrue( sessionFactory().getSessionFactoryOptions().isConventionalJavaConstants() ); + + Session s = openSession(); + s.getTransaction().begin(); + + try { + AnEntity anEntity = (AnEntity) s.createQuery( + "from AnEntity e where e.operationType = org.hibernate.test.queryconstant.NonConventionalJavaConstantDefaultTest$OPERATION_TYPE.CREATE" + ).uniqueResult(); + fail( "should have thrown QuerySyntaxException" ); + } + catch (QuerySyntaxException expected) { + // expected + } + finally { + s.getTransaction().rollback(); + s.close(); + } + } + + public enum OPERATION_TYPE { + CREATE, UPDATE, DELETE + } + + @Entity(name = "AnEntity") + public static class AnEntity { + @Id + @GeneratedValue + private long id; + + OPERATION_TYPE operationType; + } +} From 21d60eb008df9ff54ab1d5656799518913872cd1 Mon Sep 17 00:00:00 2001 From: Vlad Mihalcea Date: Tue, 10 Jan 2017 13:51:32 +0200 Subject: [PATCH 3/3] HHH-11377 - ReflectHelper#getConstantValue should consider digits as well (cherry picked from commit 491d7341ade3cfdc2e7e34c6da7f5400d5663df1) --- build.gradle | 6 ++++++ .../hibernate/internal/util/ReflectHelper.java | 2 +- .../internal/util/ReflectHelperTest.java | 11 +++++++++++ .../util/hib3rnat3/C0nst4nts\340\245\251.java" | 15 +++++++++++++++ 4 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 "hibernate-core/src/test/java/org/hibernate/internal/util/hib3rnat3/C0nst4nts\340\245\251.java" diff --git a/build.gradle b/build.gradle index a0211ac25a45..6074b699ba16 100644 --- a/build.gradle +++ b/build.gradle @@ -170,6 +170,12 @@ subprojects { subProject -> } } // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + // compilation + compileJava.options.encoding = 'UTF-8' + + tasks.withType(JavaCompile) { + options.encoding = 'UTF-8' + } task compile compile.dependsOn compileJava, processResources, compileTestJava, processTestResources 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 index e5b847e7effe..3d16239db179 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/util/ReflectHelper.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/util/ReflectHelper.java @@ -36,7 +36,7 @@ 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_\\$]+" ); + "[a-z\\d]+\\.([A-Z]{1}[a-z\\d]+)+\\$?([A-Z]{1}[a-z\\d]+)*\\.[A-Z_\\$]+", Pattern.UNICODE_CHARACTER_CLASS); public static final Class[] NO_PARAM_SIGNATURE = new Class[0]; public static final Object[] NO_PARAMS = new Object[0]; 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 index cf2e1cb3e09a..337540e05be2 100644 --- a/hibernate-core/src/test/java/org/hibernate/internal/util/ReflectHelperTest.java +++ b/hibernate-core/src/test/java/org/hibernate/internal/util/ReflectHelperTest.java @@ -11,6 +11,7 @@ import org.hibernate.boot.registry.classloading.spi.ClassLoaderService; import org.hibernate.boot.spi.SessionFactoryOptions; import org.hibernate.engine.spi.SessionFactoryImplementor; +import org.hibernate.internal.util.hib3rnat3.C0nst4nts३; import org.hibernate.service.spi.ServiceRegistryImplementor; import org.junit.Before; @@ -123,4 +124,14 @@ public void test_getConstantValue_nestedEnum() { assertEquals( Status.ON, value ); verify(classLoaderServiceMock, times(1)).classForName( eq("org.hibernate.internal.util.ReflectHelperTest$Status") ); } + + @Test + public void test_getConstantValue_constant_digits() { + + when( sessionFactoryOptionsMock.isConventionalJavaConstants() ).thenReturn( true ); + when( classLoaderServiceMock.classForName( "org.hibernate.internal.util.hib3rnat3.C0nst4nts३" ) ).thenReturn( (Class) C0nst4nts३.class ); + Object value = ReflectHelper.getConstantValue( "org.hibernate.internal.util.hib3rnat3.C0nst4nts३.ABC_DEF", sessionFactoryImplementorMock); + assertEquals( C0nst4nts३.ABC_DEF, value ); + verify(classLoaderServiceMock, times(1)).classForName( eq("org.hibernate.internal.util.hib3rnat3.C0nst4nts३") ); + } } \ No newline at end of file diff --git "a/hibernate-core/src/test/java/org/hibernate/internal/util/hib3rnat3/C0nst4nts\340\245\251.java" "b/hibernate-core/src/test/java/org/hibernate/internal/util/hib3rnat3/C0nst4nts\340\245\251.java" new file mode 100644 index 000000000000..45ae620d9234 --- /dev/null +++ "b/hibernate-core/src/test/java/org/hibernate/internal/util/hib3rnat3/C0nst4nts\340\245\251.java" @@ -0,0 +1,15 @@ +/* + * 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 . + */ +package org.hibernate.internal.util.hib3rnat3; + +/** + * @author Vlad Mihalcea + */ +public class C0nst4nts३ { + + public static final String ABC_DEF = "xyz"; +}