From 41093ae66ce6d2cbdd23cbda46aa90772e8efef1 Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Thu, 30 Apr 2020 09:13:37 -0500 Subject: [PATCH] * Added JpaCompliance for @OrderBy handling * Added `NonTransientException` checked during MappingModelCreationProcess "post init callback" handling to indicate non-recoverable errors * Redesigned `MappingModelCreationProcess#executePostInitCallbacks` to account for `NonTransientException` --- design/logger_id_ranges.adoc | 4 + .../hibernate/boot/SessionFactoryBuilder.java | 5 + .../internal/SessionFactoryBuilderImpl.java | 6 + .../SessionFactoryOptionsBuilder.java | 8 ++ ...stractDelegatingSessionFactoryBuilder.java | 6 + .../org/hibernate/cfg/AvailableSettings.java | 13 ++ .../hibernate/jpa/JpaComplianceViolation.java | 15 +++ .../jpa/internal/JpaComplianceImpl.java | 71 ++++++----- .../internal/MutableJpaComplianceImpl.java | 111 ++++++++++++------ .../org/hibernate/jpa/spi/JpaCompliance.java | 8 ++ .../jpa/spi/MutableJpaCompliance.java | 12 +- .../mapping/MappingModelCreationLogger.java | 30 +++++ .../mapping/NonTransientException.java | 16 +++ .../internal/MappingModelCreationHelper.java | 7 +- .../internal/MappingModelCreationProcess.java | 37 ++++-- .../internal/PluralAttributeMappingImpl.java | 5 + .../mapping/ordering/TranslationContext.java | 3 + .../ast/OrderByComplianceViolation.java | 24 ++++ .../ordering/ast/ParseTreeVisitor.java | 22 +++- .../org/hibernate/jpa/JpaComplianceStub.java | 56 +++++++++ .../jpa/JpaComplianceTestingImpl.java | 6 + .../OrderByMappingComplianceTest.java | 107 +++++++++++++++++ 22 files changed, 484 insertions(+), 88 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/jpa/JpaComplianceViolation.java create mode 100644 hibernate-core/src/main/java/org/hibernate/metamodel/mapping/MappingModelCreationLogger.java create mode 100644 hibernate-core/src/main/java/org/hibernate/metamodel/mapping/NonTransientException.java create mode 100644 hibernate-core/src/main/java/org/hibernate/metamodel/mapping/ordering/ast/OrderByComplianceViolation.java create mode 100644 hibernate-core/src/test/java/org/hibernate/jpa/JpaComplianceStub.java create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/binding/annotations/plural/orderby/compliance/OrderByMappingComplianceTest.java diff --git a/design/logger_id_ranges.adoc b/design/logger_id_ranges.adoc index 5374b694ff..d05e18564d 100644 --- a/design/logger_id_ranges.adoc +++ b/design/logger_id_ranges.adoc @@ -102,4 +102,8 @@ |90005700 |org.hibernate.envers.boot.EnversBootLogger +|90005701 +|90005800 +|org.hibernate.metamodel.mapping.MappingModelCreationLogger + |=== diff --git a/hibernate-core/src/main/java/org/hibernate/boot/SessionFactoryBuilder.java b/hibernate-core/src/main/java/org/hibernate/boot/SessionFactoryBuilder.java index 476fc30051..d53ee57eeb 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/SessionFactoryBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/SessionFactoryBuilder.java @@ -728,6 +728,11 @@ public interface SessionFactoryBuilder { */ SessionFactoryBuilder enableJpaQueryCompliance(boolean enabled); + /** + * @see JpaCompliance#isJpaQueryComplianceEnabled() + */ + SessionFactoryBuilder enableJpaOrderByMappingCompliance(boolean enabled); + /** * @see JpaCompliance#isJpaTransactionComplianceEnabled() */ 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 6944a17a2d..177bf4ddcf 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 @@ -416,6 +416,12 @@ public class SessionFactoryBuilderImpl implements SessionFactoryBuilderImplement return this; } + @Override + public SessionFactoryBuilder enableJpaOrderByMappingCompliance(boolean enabled) { + this.optionsBuilder.enableJpaOrderByMappingCompliance( enabled ); + return this; + } + @Override public SessionFactoryBuilder enableJpaTransactionCompliance(boolean enabled) { this.optionsBuilder.enableJpaTransactionCompliance( enabled ); diff --git a/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsBuilder.java b/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsBuilder.java index 3be16e0d68..2702edcb3a 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsBuilder.java @@ -1478,6 +1478,14 @@ public class SessionFactoryOptionsBuilder implements SessionFactoryOptions { mutableJpaCompliance().setCachingCompliance( enabled ); } + public void enableJpaOrderByMappingCompliance(boolean enabled) { + mutableJpaCompliance().setOrderByMappingCompliance( enabled ); + } + + public void enableGeneratorNameScopeCompliance(boolean enabled) { + mutableJpaCompliance().setGeneratorNameScopeCompliance( enabled ); + } + public void disableRefreshDetachedEntity() { this.allowRefreshDetachedEntity = false; } diff --git a/hibernate-core/src/main/java/org/hibernate/boot/spi/AbstractDelegatingSessionFactoryBuilder.java b/hibernate-core/src/main/java/org/hibernate/boot/spi/AbstractDelegatingSessionFactoryBuilder.java index 719ab27f9b..97faa4bba8 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/spi/AbstractDelegatingSessionFactoryBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/spi/AbstractDelegatingSessionFactoryBuilder.java @@ -394,6 +394,12 @@ public abstract class AbstractDelegatingSessionFactoryBuilder postInitCallbacks) { + private void executePostInitCallbacks(List postInitCallbacks) { while ( postInitCallbacks != null && !postInitCallbacks.isEmpty() ) { - // copy to avoid CCME - final ArrayList copy = new ArrayList<>( new ArrayList<>( postInitCallbacks ) ); - for ( PostInitCallback callback : copy ) { - final boolean completed = callback.process(); - if ( completed ) { - postInitCallbacks.remove( callback ); + // copy to avoid CCME + final ArrayList copy = new ArrayList<>( postInitCallbacks ); + + for ( int i = 0; i < copy.size(); i++ ) { + final PostInitCallback callback = copy.get( i ); + + try { + final boolean completed = callback.process(); + if ( completed ) { + postInitCallbacks.remove( callback ); + } + } + catch (Exception e) { + if ( e instanceof NonTransientException ) { + MappingModelCreationLogger.LOGGER.debugf( "Mapping-model creation encountered non-transient error : %s", e ); + throw e; + } + + MappingModelCreationLogger.LOGGER.debugf( "Mapping-model creation encountered (possibly) transient error : %s", e ); } } if ( copy.size() == postInitCallbacks.size() ) { - // none of the processes could complete fully, this is an error + // none of the remaining callbacks could complete fully, this is an error throw new IllegalStateException( "No post-init callbacks could complete" ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/PluralAttributeMappingImpl.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/PluralAttributeMappingImpl.java index ded4d31e5f..5cddbd9e27 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/PluralAttributeMappingImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/PluralAttributeMappingImpl.java @@ -17,6 +17,7 @@ import org.hibernate.engine.FetchStrategy; import org.hibernate.engine.FetchTiming; import org.hibernate.engine.spi.CascadeStyle; import org.hibernate.engine.spi.LoadQueryInfluencers; +import org.hibernate.jpa.spi.JpaCompliance; import org.hibernate.mapping.Collection; import org.hibernate.mapping.IndexedCollection; import org.hibernate.mapping.List; @@ -246,6 +247,10 @@ public class PluralAttributeMappingImpl extends AbstractAttributeMapping impleme if ( hasOrder || hasManyToManyOrder ) { final TranslationContext context = new TranslationContext() { + @Override + public JpaCompliance getJpaCompliance() { + return collectionDescriptor.getFactory().getSessionFactoryOptions().getJpaCompliance(); + } }; if ( hasOrder ) { diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/ordering/TranslationContext.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/ordering/TranslationContext.java index fb20bf3c22..b019eeca74 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/ordering/TranslationContext.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/ordering/TranslationContext.java @@ -6,10 +6,13 @@ */ package org.hibernate.metamodel.mapping.ordering; +import org.hibernate.jpa.spi.JpaCompliance; + /** * Access to information needed while translating a collection's order-by fragment * * @author Steve Ebersole */ public interface TranslationContext { + JpaCompliance getJpaCompliance(); } diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/ordering/ast/OrderByComplianceViolation.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/ordering/ast/OrderByComplianceViolation.java new file mode 100644 index 0000000000..a0c59ae314 --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/ordering/ast/OrderByComplianceViolation.java @@ -0,0 +1,24 @@ +/* + * 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.metamodel.mapping.ordering.ast; + +import org.hibernate.HibernateException; +import org.hibernate.jpa.JpaComplianceViolation; +import org.hibernate.metamodel.mapping.NonTransientException; + +/** + * @author Steve Ebersole + */ +public class OrderByComplianceViolation extends HibernateException implements JpaComplianceViolation, NonTransientException { + public OrderByComplianceViolation(String message) { + super( message ); + } + + public OrderByComplianceViolation(String message, Throwable cause) { + super( message, cause ); + } +} diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/ordering/ast/ParseTreeVisitor.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/ordering/ast/ParseTreeVisitor.java index b35a86b253..f7164e7ecc 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/ordering/ast/ParseTreeVisitor.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/ordering/ast/ParseTreeVisitor.java @@ -17,16 +17,12 @@ import org.hibernate.grammars.ordering.OrderingParserBaseVisitor; import org.hibernate.metamodel.mapping.PluralAttributeMapping; import org.hibernate.metamodel.mapping.ordering.TranslationContext; -import org.jboss.logging.Logger; - import org.antlr.v4.runtime.tree.TerminalNode; /** * @author Steve Ebersole */ -public class ParseTreeVisitor extends OrderingParserBaseVisitor { - private static final Logger log = Logger.getLogger( ParseTreeVisitor.class ); - +public class ParseTreeVisitor extends OrderingParserBaseVisitor { private final PathConsumer pathConsumer; private final TranslationContext translationContext; @@ -60,7 +56,21 @@ public class ParseTreeVisitor extends OrderingParserBaseVisitor { assert parsedSpec != null; assert parsedSpec.expression() != null; - final OrderingSpecification result = new OrderingSpecification( visitExpression( parsedSpec.expression() ) ); + final OrderingExpression orderingExpression = visitExpression( parsedSpec.expression() ); + if ( translationContext.getJpaCompliance().isJpaOrderByMappingComplianceEnabled() ) { + if ( orderingExpression instanceof DomainPath ) { + // nothing to do + } + else { + throw new OrderByComplianceViolation( + "`@OrderBy` expression (" + parsedSpec.expression().getText() + + ") resolved to `" + orderingExpression + + "` which is not a domain-model reference which violates the JPA specification" + ); + } + } + + final OrderingSpecification result = new OrderingSpecification( orderingExpression ); if ( parsedSpec.collationSpecification() != null ) { result.setCollation( parsedSpec.collationSpecification().identifier().getText() ); diff --git a/hibernate-core/src/test/java/org/hibernate/jpa/JpaComplianceStub.java b/hibernate-core/src/test/java/org/hibernate/jpa/JpaComplianceStub.java new file mode 100644 index 0000000000..9c1d718870 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/jpa/JpaComplianceStub.java @@ -0,0 +1,56 @@ +/* + * 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.jpa; + +import org.hibernate.jpa.spi.JpaCompliance; + +/** + * Stubbed impl of JpaCompliance + * + * @author Steve Ebersole + */ +public class JpaComplianceStub implements JpaCompliance { + @Override + public boolean isJpaQueryComplianceEnabled() { + return false; + } + + @Override + public boolean isJpaTransactionComplianceEnabled() { + return false; + } + + @Override + public boolean isJpaListComplianceEnabled() { + return false; + } + + @Override + public boolean isJpaClosedComplianceEnabled() { + return false; + } + + @Override + public boolean isJpaProxyComplianceEnabled() { + return false; + } + + @Override + public boolean isJpaCacheComplianceEnabled() { + return false; + } + + @Override + public boolean isGlobalGeneratorScopeEnabled() { + return false; + } + + @Override + public boolean isJpaOrderByMappingComplianceEnabled() { + return false; + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/jpa/JpaComplianceTestingImpl.java b/hibernate-core/src/test/java/org/hibernate/jpa/JpaComplianceTestingImpl.java index dbdb146436..4e4d16b92a 100644 --- a/hibernate-core/src/test/java/org/hibernate/jpa/JpaComplianceTestingImpl.java +++ b/hibernate-core/src/test/java/org/hibernate/jpa/JpaComplianceTestingImpl.java @@ -47,6 +47,7 @@ public class JpaComplianceTestingImpl implements JpaCompliance { private boolean proxyCompliance; private boolean cacheCompliance; private boolean idGeneratorNameScopeCompliance; + private boolean orderByCompliance; @Override public boolean isJpaQueryComplianceEnabled() { @@ -63,6 +64,11 @@ public class JpaComplianceTestingImpl implements JpaCompliance { return listCompliance; } + @Override + public boolean isJpaOrderByMappingComplianceEnabled() { + return orderByCompliance; + } + @Override public boolean isJpaClosedComplianceEnabled() { return closedCompliance; diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/binding/annotations/plural/orderby/compliance/OrderByMappingComplianceTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/binding/annotations/plural/orderby/compliance/OrderByMappingComplianceTest.java new file mode 100644 index 0000000000..b7fbc12124 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/binding/annotations/plural/orderby/compliance/OrderByMappingComplianceTest.java @@ -0,0 +1,107 @@ +/* + * 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.orm.test.bootstrap.binding.annotations.plural.orderby.compliance; + +import java.util.Set; +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.OneToMany; +import javax.persistence.OrderBy; +import javax.persistence.Table; + +import org.hibernate.SessionFactory; +import org.hibernate.boot.Metadata; +import org.hibernate.boot.MetadataSources; +import org.hibernate.boot.SessionFactoryBuilder; +import org.hibernate.boot.registry.StandardServiceRegistry; +import org.hibernate.boot.registry.StandardServiceRegistryBuilder; +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.engine.spi.SessionFactoryImplementor; +import org.hibernate.metamodel.mapping.ordering.ast.OrderByComplianceViolation; + +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.DomainModelScope; +import org.hibernate.testing.orm.junit.ServiceRegistry; +import org.hibernate.testing.transaction.TransactionUtil2; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.fail; + +/** + * @author Steve Ebersole + */ +public class OrderByMappingComplianceTest { + + @Test + public void testComplianceChecking() { + check( false ); + + try { + check( true ); + fail( "Expecting an exception" ); + } + catch (OrderByComplianceViolation expected) { + // nothing to do + } + } + + private void check(boolean complianceEnabled) { + final StandardServiceRegistry ssr = new StandardServiceRegistryBuilder() + .applySetting( AvailableSettings.HBM2DDL_AUTO, "create-drop" ) + .build(); + + try { + final Metadata bootModel = new MetadataSources( ssr ) + .addAnnotatedClass( Order.class ) + .addAnnotatedClass( LineItem.class ) + .buildMetadata(); + + final SessionFactory sf = bootModel.getSessionFactoryBuilder() + .enableJpaOrderByMappingCompliance( complianceEnabled ) + .build(); + + try { + TransactionUtil2.inTransaction( + (SessionFactoryImplementor) sf, + session -> session.createQuery( "from Order" ).list() + ); + } + finally { + try { + sf.close(); + } + catch (Exception ignore) { + } + } + } + finally { + StandardServiceRegistryBuilder.destroy( ssr ); + } + } + + @Entity( name = "Order" ) + @Table( name = "orders" ) + public static class Order { + @Id + private Integer id; + private String invoice; + @OneToMany + @OrderBy( "qty" ) + private Set lineItems; + } + + @Entity( name = "LineItem" ) + @Table( name = "line_items" ) + public static class LineItem { + @Id + private Integer id; + private String sku; + @Column( name = "qty" ) + private int quantity; + } +}