From 1dfb25bbbbb61e0a0fb6a2a47f5503f7fe143d64 Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Mon, 30 Sep 2019 07:58:18 -0500 Subject: [PATCH 1/7] HHH-13432 - EntityManagerFactory no longer exposes "javax.persistence.nonJtaDataSource" --- .../orm/test/bootstrap/DataSourceStub.java | 88 +++++++++++++++++++ .../bootstrap/PersistenceUnitInfoTests.java | 54 ++++++++++++ .../PersistenceUnitOverridesTests.java | 85 ++---------------- 3 files changed, 149 insertions(+), 78 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/DataSourceStub.java create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/PersistenceUnitInfoTests.java diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/DataSourceStub.java b/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/DataSourceStub.java new file mode 100644 index 0000000000..708c963113 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/DataSourceStub.java @@ -0,0 +1,88 @@ +/* + * 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; + +import java.io.PrintWriter; +import java.sql.Connection; +import java.sql.SQLException; +import java.util.logging.Logger; +import javax.sql.DataSource; + +import org.hibernate.engine.jdbc.connections.internal.DriverManagerConnectionProviderImpl; + +import org.hibernate.testing.env.ConnectionProviderBuilder; + +/** + * @author Steve Ebersole + */ +public class DataSourceStub implements DataSource { + private final String id; + private final DriverManagerConnectionProviderImpl connectionProvider; + private PrintWriter printWriter; + + DataSourceStub(String id) { + this.id = id; + connectionProvider = new DriverManagerConnectionProviderImpl(); + connectionProvider.configure( ConnectionProviderBuilder.getConnectionProviderProperties() ); + + printWriter = null; + } + + public String getId() { + return id; + } + + @Override + public Connection getConnection() throws SQLException { + return connectionProvider.getConnection(); + } + + @Override + public Connection getConnection(String username, String password) { + throw new UnsupportedOperationException(); + } + + @Override + public PrintWriter getLogWriter() { + return printWriter; + } + + @Override + public void setLogWriter(PrintWriter out) { + this.printWriter = out; + } + + @Override + public void setLoginTimeout(int seconds) { + } + + @Override + public int getLoginTimeout() { + return -1; + } + + @Override + public Logger getParentLogger() { + return Logger.getGlobal(); + } + + @Override + public T unwrap(Class iface) { + //noinspection unchecked + return (T) this; + } + + @Override + public boolean isWrapperFor(Class iface) { + return iface.isAssignableFrom( getClass() ); + } + + @Override + public String toString() { + return "DataSourceImpl(" + id + ")"; + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/PersistenceUnitInfoTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/PersistenceUnitInfoTests.java new file mode 100644 index 0000000000..3fe098bdb7 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/PersistenceUnitInfoTests.java @@ -0,0 +1,54 @@ +/* + * 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; + +import java.util.Collections; +import java.util.Map; +import javax.persistence.EntityManagerFactory; +import javax.persistence.spi.PersistenceProvider; +import javax.sql.DataSource; + +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.jpa.HibernatePersistenceProvider; + +import org.hibernate.testing.FailureExpected; +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.junit4.BaseUnitTestCase; +import org.hibernate.testing.util.jpa.PersistenceUnitInfoAdapter; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +/** + * @author Steve Ebersole + */ +public class PersistenceUnitInfoTests extends BaseUnitTestCase { + @Test + @TestForIssue( jiraKey = "HHH-13432" ) + @FailureExpected( jiraKey = "HHH-13432" ) + public void testJtaDataExposedAsProperty() { + final DataSource puDataSource = new DataSourceStub( "puDataSource" ); + final PersistenceUnitInfoAdapter info = new PersistenceUnitInfoAdapter() { + + @Override + public DataSource getNonJtaDataSource() { + return puDataSource; + } + }; + + final PersistenceProvider provider = new HibernatePersistenceProvider(); + + final EntityManagerFactory emf = provider.createContainerEntityManagerFactory( + info, + Collections.emptyMap() + ); + + final Map properties = emf.getProperties(); + final Object o = properties.get( AvailableSettings.JPA_JTA_DATASOURCE ); + assertEquals( o, puDataSource ); + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/PersistenceUnitOverridesTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/PersistenceUnitOverridesTests.java index 41e45d1fa3..6cc59d058f 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/PersistenceUnitOverridesTests.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/PersistenceUnitOverridesTests.java @@ -6,13 +6,9 @@ */ package org.hibernate.orm.test.bootstrap; -import java.io.PrintWriter; -import java.sql.Connection; -import java.sql.SQLException; import java.util.HashMap; import java.util.Map; import java.util.Properties; -import java.util.logging.Logger; import javax.persistence.EntityManagerFactory; import javax.persistence.spi.PersistenceProvider; import javax.persistence.spi.PersistenceUnitInfo; @@ -100,7 +96,7 @@ public class PersistenceUnitOverridesTests extends BaseUnitTestCase { final Properties puProperties; { - puDataSource = new DataSourceImpl( "puDataSource" ); + puDataSource = new DataSourceStub( "puDataSource" ); puProperties = new Properties(); puProperties.putAll( info.getProperties() ); @@ -161,7 +157,7 @@ public class PersistenceUnitOverridesTests extends BaseUnitTestCase { final DataSource puDataSource; { - puDataSource = new DataSourceImpl( "puDataSource" ); + puDataSource = new DataSourceStub( "puDataSource" ); } @Override @@ -209,8 +205,8 @@ public class PersistenceUnitOverridesTests extends BaseUnitTestCase { "have precedence over integration settings, which is also incorrect" ) public void testPassingIntegrationJpaDataSourceOverrideForJtaDataSourceElement() { - final DataSource puDataSource = new DataSourceImpl( "puDataSource" ); - final DataSource integrationDataSource = new DataSourceImpl( "integrationDataSource" ); + final DataSource puDataSource = new DataSourceStub( "puDataSource" ); + final DataSource integrationDataSource = new DataSourceStub( "integrationDataSource" ); PersistenceProvider provider = new HibernatePersistenceProvider() { @Override @@ -263,7 +259,7 @@ public class PersistenceUnitOverridesTests extends BaseUnitTestCase { public void testIntegrationOverridesOfPersistenceXmlDataSource() { // mimics a DataSource defined in the persistence.xml - final DataSourceImpl dataSource = new DataSourceImpl( "puDataSource" ); + final DataSourceStub dataSource = new DataSourceStub( "puDataSource" ); final PersistenceUnitInfoAdapter info = new PersistenceUnitInfoAdapter() { @Override @@ -274,7 +270,7 @@ public class PersistenceUnitOverridesTests extends BaseUnitTestCase { // Now create "integration Map" that overrides the DataSource to use - final DataSource override = new DataSourceImpl( "integrationDataSource" ); + final DataSource override = new DataSourceStub( "integrationDataSource" ); final Map integrationSettings = new HashMap<>(); integrationSettings.put( AvailableSettings.JPA_NON_JTA_DATASOURCE, override ); @@ -308,7 +304,7 @@ public class PersistenceUnitOverridesTests extends BaseUnitTestCase { public void testIntegrationOverridesOfPersistenceXmlDataSourceWithDriverManagerInfo() { // mimics a DataSource defined in the persistence.xml - final DataSourceImpl dataSource = new DataSourceImpl( "puDataSource" ); + final DataSourceStub dataSource = new DataSourceStub( "puDataSource" ); final PersistenceUnitInfoAdapter info = new PersistenceUnitInfoAdapter() { @Override @@ -335,71 +331,4 @@ public class PersistenceUnitOverridesTests extends BaseUnitTestCase { assertThat( connectionProvider, instanceOf( DriverManagerConnectionProviderImpl.class ) ); } - private static class DataSourceImpl implements DataSource { - private final String id; - private final DriverManagerConnectionProviderImpl connectionProvider; - private PrintWriter printWriter; - - DataSourceImpl(String id) { - this.id = id; - connectionProvider = new DriverManagerConnectionProviderImpl(); - connectionProvider.configure( ConnectionProviderBuilder.getConnectionProviderProperties() ); - - printWriter = null; - } - - public String getId() { - return id; - } - - @Override - public Connection getConnection() throws SQLException { - return connectionProvider.getConnection(); - } - - @Override - public Connection getConnection(String username, String password) { - throw new UnsupportedOperationException(); - } - - @Override - public PrintWriter getLogWriter() { - return printWriter; - } - - @Override - public void setLogWriter(PrintWriter out) { - this.printWriter = out; - } - - @Override - public void setLoginTimeout(int seconds) { - } - - @Override - public int getLoginTimeout() { - return -1; - } - - @Override - public Logger getParentLogger() { - return Logger.getGlobal(); - } - - @Override - public T unwrap(Class iface) { - //noinspection unchecked - return (T) this; - } - - @Override - public boolean isWrapperFor(Class iface) { - return iface.isAssignableFrom( getClass() ); - } - - @Override - public String toString() { - return "DataSourceImpl(" + id + ")"; - } - } } From d46d3d66a28382f4212f68d453dc7d4d87ec1a43 Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Mon, 30 Sep 2019 07:58:18 -0500 Subject: [PATCH 2/7] HHH-13432 - EntityManagerFactory no longer exposes "javax.persistence.nonJtaDataSource" --- .../orm/test/bootstrap/DataSourceStub.java | 88 +++++++++++++++++++ .../bootstrap/PersistenceUnitInfoTests.java | 54 ++++++++++++ .../PersistenceUnitOverridesTests.java | 85 ++---------------- 3 files changed, 149 insertions(+), 78 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/DataSourceStub.java create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/PersistenceUnitInfoTests.java diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/DataSourceStub.java b/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/DataSourceStub.java new file mode 100644 index 0000000000..708c963113 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/DataSourceStub.java @@ -0,0 +1,88 @@ +/* + * 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; + +import java.io.PrintWriter; +import java.sql.Connection; +import java.sql.SQLException; +import java.util.logging.Logger; +import javax.sql.DataSource; + +import org.hibernate.engine.jdbc.connections.internal.DriverManagerConnectionProviderImpl; + +import org.hibernate.testing.env.ConnectionProviderBuilder; + +/** + * @author Steve Ebersole + */ +public class DataSourceStub implements DataSource { + private final String id; + private final DriverManagerConnectionProviderImpl connectionProvider; + private PrintWriter printWriter; + + DataSourceStub(String id) { + this.id = id; + connectionProvider = new DriverManagerConnectionProviderImpl(); + connectionProvider.configure( ConnectionProviderBuilder.getConnectionProviderProperties() ); + + printWriter = null; + } + + public String getId() { + return id; + } + + @Override + public Connection getConnection() throws SQLException { + return connectionProvider.getConnection(); + } + + @Override + public Connection getConnection(String username, String password) { + throw new UnsupportedOperationException(); + } + + @Override + public PrintWriter getLogWriter() { + return printWriter; + } + + @Override + public void setLogWriter(PrintWriter out) { + this.printWriter = out; + } + + @Override + public void setLoginTimeout(int seconds) { + } + + @Override + public int getLoginTimeout() { + return -1; + } + + @Override + public Logger getParentLogger() { + return Logger.getGlobal(); + } + + @Override + public T unwrap(Class iface) { + //noinspection unchecked + return (T) this; + } + + @Override + public boolean isWrapperFor(Class iface) { + return iface.isAssignableFrom( getClass() ); + } + + @Override + public String toString() { + return "DataSourceImpl(" + id + ")"; + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/PersistenceUnitInfoTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/PersistenceUnitInfoTests.java new file mode 100644 index 0000000000..3fe098bdb7 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/PersistenceUnitInfoTests.java @@ -0,0 +1,54 @@ +/* + * 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; + +import java.util.Collections; +import java.util.Map; +import javax.persistence.EntityManagerFactory; +import javax.persistence.spi.PersistenceProvider; +import javax.sql.DataSource; + +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.jpa.HibernatePersistenceProvider; + +import org.hibernate.testing.FailureExpected; +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.junit4.BaseUnitTestCase; +import org.hibernate.testing.util.jpa.PersistenceUnitInfoAdapter; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +/** + * @author Steve Ebersole + */ +public class PersistenceUnitInfoTests extends BaseUnitTestCase { + @Test + @TestForIssue( jiraKey = "HHH-13432" ) + @FailureExpected( jiraKey = "HHH-13432" ) + public void testJtaDataExposedAsProperty() { + final DataSource puDataSource = new DataSourceStub( "puDataSource" ); + final PersistenceUnitInfoAdapter info = new PersistenceUnitInfoAdapter() { + + @Override + public DataSource getNonJtaDataSource() { + return puDataSource; + } + }; + + final PersistenceProvider provider = new HibernatePersistenceProvider(); + + final EntityManagerFactory emf = provider.createContainerEntityManagerFactory( + info, + Collections.emptyMap() + ); + + final Map properties = emf.getProperties(); + final Object o = properties.get( AvailableSettings.JPA_JTA_DATASOURCE ); + assertEquals( o, puDataSource ); + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/PersistenceUnitOverridesTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/PersistenceUnitOverridesTests.java index 41e45d1fa3..6cc59d058f 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/PersistenceUnitOverridesTests.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/bootstrap/PersistenceUnitOverridesTests.java @@ -6,13 +6,9 @@ */ package org.hibernate.orm.test.bootstrap; -import java.io.PrintWriter; -import java.sql.Connection; -import java.sql.SQLException; import java.util.HashMap; import java.util.Map; import java.util.Properties; -import java.util.logging.Logger; import javax.persistence.EntityManagerFactory; import javax.persistence.spi.PersistenceProvider; import javax.persistence.spi.PersistenceUnitInfo; @@ -100,7 +96,7 @@ public class PersistenceUnitOverridesTests extends BaseUnitTestCase { final Properties puProperties; { - puDataSource = new DataSourceImpl( "puDataSource" ); + puDataSource = new DataSourceStub( "puDataSource" ); puProperties = new Properties(); puProperties.putAll( info.getProperties() ); @@ -161,7 +157,7 @@ public class PersistenceUnitOverridesTests extends BaseUnitTestCase { final DataSource puDataSource; { - puDataSource = new DataSourceImpl( "puDataSource" ); + puDataSource = new DataSourceStub( "puDataSource" ); } @Override @@ -209,8 +205,8 @@ public class PersistenceUnitOverridesTests extends BaseUnitTestCase { "have precedence over integration settings, which is also incorrect" ) public void testPassingIntegrationJpaDataSourceOverrideForJtaDataSourceElement() { - final DataSource puDataSource = new DataSourceImpl( "puDataSource" ); - final DataSource integrationDataSource = new DataSourceImpl( "integrationDataSource" ); + final DataSource puDataSource = new DataSourceStub( "puDataSource" ); + final DataSource integrationDataSource = new DataSourceStub( "integrationDataSource" ); PersistenceProvider provider = new HibernatePersistenceProvider() { @Override @@ -263,7 +259,7 @@ public class PersistenceUnitOverridesTests extends BaseUnitTestCase { public void testIntegrationOverridesOfPersistenceXmlDataSource() { // mimics a DataSource defined in the persistence.xml - final DataSourceImpl dataSource = new DataSourceImpl( "puDataSource" ); + final DataSourceStub dataSource = new DataSourceStub( "puDataSource" ); final PersistenceUnitInfoAdapter info = new PersistenceUnitInfoAdapter() { @Override @@ -274,7 +270,7 @@ public class PersistenceUnitOverridesTests extends BaseUnitTestCase { // Now create "integration Map" that overrides the DataSource to use - final DataSource override = new DataSourceImpl( "integrationDataSource" ); + final DataSource override = new DataSourceStub( "integrationDataSource" ); final Map integrationSettings = new HashMap<>(); integrationSettings.put( AvailableSettings.JPA_NON_JTA_DATASOURCE, override ); @@ -308,7 +304,7 @@ public class PersistenceUnitOverridesTests extends BaseUnitTestCase { public void testIntegrationOverridesOfPersistenceXmlDataSourceWithDriverManagerInfo() { // mimics a DataSource defined in the persistence.xml - final DataSourceImpl dataSource = new DataSourceImpl( "puDataSource" ); + final DataSourceStub dataSource = new DataSourceStub( "puDataSource" ); final PersistenceUnitInfoAdapter info = new PersistenceUnitInfoAdapter() { @Override @@ -335,71 +331,4 @@ public class PersistenceUnitOverridesTests extends BaseUnitTestCase { assertThat( connectionProvider, instanceOf( DriverManagerConnectionProviderImpl.class ) ); } - private static class DataSourceImpl implements DataSource { - private final String id; - private final DriverManagerConnectionProviderImpl connectionProvider; - private PrintWriter printWriter; - - DataSourceImpl(String id) { - this.id = id; - connectionProvider = new DriverManagerConnectionProviderImpl(); - connectionProvider.configure( ConnectionProviderBuilder.getConnectionProviderProperties() ); - - printWriter = null; - } - - public String getId() { - return id; - } - - @Override - public Connection getConnection() throws SQLException { - return connectionProvider.getConnection(); - } - - @Override - public Connection getConnection(String username, String password) { - throw new UnsupportedOperationException(); - } - - @Override - public PrintWriter getLogWriter() { - return printWriter; - } - - @Override - public void setLogWriter(PrintWriter out) { - this.printWriter = out; - } - - @Override - public void setLoginTimeout(int seconds) { - } - - @Override - public int getLoginTimeout() { - return -1; - } - - @Override - public Logger getParentLogger() { - return Logger.getGlobal(); - } - - @Override - public T unwrap(Class iface) { - //noinspection unchecked - return (T) this; - } - - @Override - public boolean isWrapperFor(Class iface) { - return iface.isAssignableFrom( getClass() ); - } - - @Override - public String toString() { - return "DataSourceImpl(" + id + ")"; - } - } } From ee304305e8005e479164c92edd01b55a0296df3d Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Mon, 30 Sep 2019 17:50:39 -0500 Subject: [PATCH 3/7] HHH-13640 - Uninitialized HibernateProxy mapped as NO_PROXY gets initialized when reloaded with enhancement-as-proxy enabled --- .../internal/DefaultLoadEventListener.java | 16 ++++++++-------- .../java/org/hibernate/event/spi/LoadEvent.java | 17 ----------------- .../org/hibernate/internal/SessionImpl.java | 3 --- .../LazyToOnesProxyWithSubclassesTest.java | 3 --- 4 files changed, 8 insertions(+), 31 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultLoadEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultLoadEventListener.java index 432b8650db..7b33d86f83 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultLoadEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultLoadEventListener.java @@ -287,21 +287,21 @@ public class DefaultLoadEventListener implements LoadEventListener { LazyInitializer li = ( (HibernateProxy) proxy ).getHibernateLazyInitializer(); - if ( li.isUnwrap() || event.getShouldUnwrapProxy() ) { - return li.getImplementation(); + if ( li.isUnwrap() ) { + if ( entityMetamodel.hasSubclasses() ) { + LOG.debug( "Ignoring NO_PROXY for to-one association with subclasses to honor laziness" ); + } + else { + return li.getImplementation(); + } } - return persistenceContext.narrowProxy( proxy, persister, keyToLoad, null ); } // specialized handling for entities with subclasses with a HibernateProxy factory if ( entityMetamodel.hasSubclasses() ) { - // entities with subclasses that define a ProxyFactory can create - // a HibernateProxy so long as NO_PROXY was not specified. - if ( event.getShouldUnwrapProxy() != null && event.getShouldUnwrapProxy() ) { - LOG.debug( "Ignoring NO_PROXY for to-one association with subclasses to honor laziness" ); - } + // entities with subclasses that define a ProxyFactory can create a HibernateProxy return createProxy( event, persister, keyToLoad, persistenceContext ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/event/spi/LoadEvent.java b/hibernate-core/src/main/java/org/hibernate/event/spi/LoadEvent.java index b04c0b0818..bd33e60824 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/spi/LoadEvent.java +++ b/hibernate-core/src/main/java/org/hibernate/event/spi/LoadEvent.java @@ -47,8 +47,6 @@ public class LoadEvent extends AbstractEvent { private Object result; private PostLoadEvent postLoadEvent; - private Boolean shouldUnwrapProxy; - public LoadEvent(Serializable entityId, Object instanceToLoad, EventSource source) { this( entityId, null, instanceToLoad, DEFAULT_LOCK_OPTIONS, false, source ); } @@ -192,19 +190,4 @@ public class LoadEvent extends AbstractEvent { public void setPostLoadEvent(PostLoadEvent postLoadEvent) { this.postLoadEvent = postLoadEvent; } - - public Boolean getShouldUnwrapProxy() { - if ( shouldUnwrapProxy == null ) { - final boolean enabled = getSession().getFactory() - .getSessionFactoryOptions() - .isEnhancementAsProxyEnabled(); - return enabled; - } - - return shouldUnwrapProxy; - } - - public void setShouldUnwrapProxy(Boolean shouldUnwrapProxy) { - this.shouldUnwrapProxy = shouldUnwrapProxy; - } } diff --git a/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java b/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java index 656feaa482..534ba2f9df 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java @@ -36,7 +36,6 @@ import javax.persistence.EntityNotFoundException; import javax.persistence.FlushModeType; import javax.persistence.LockModeType; import javax.persistence.PersistenceException; -import javax.persistence.PessimisticLockScope; import javax.persistence.StoredProcedureQuery; import javax.persistence.TransactionRequiredException; import javax.persistence.criteria.CriteriaBuilder; @@ -1041,7 +1040,6 @@ public final class SessionImpl loadEvent = null; event = recycleEventInstance( event, id, entityName ); - event.setShouldUnwrapProxy( unwrapProxy ); fireLoadNoChecks( event, type ); @@ -1081,7 +1079,6 @@ public final class SessionImpl event.setLockMode( LoadEvent.DEFAULT_LOCK_MODE ); event.setLockScope( LoadEvent.DEFAULT_LOCK_OPTIONS.getScope() ); event.setLockTimeout( LoadEvent.DEFAULT_LOCK_OPTIONS.getTimeOut() ); - event.setShouldUnwrapProxy( null ); return event; } } diff --git a/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/proxy/LazyToOnesProxyWithSubclassesTest.java b/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/proxy/LazyToOnesProxyWithSubclassesTest.java index 7c5d106ca1..8ff06a1cc6 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/proxy/LazyToOnesProxyWithSubclassesTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/proxy/LazyToOnesProxyWithSubclassesTest.java @@ -22,7 +22,6 @@ import org.hibernate.boot.SessionFactoryBuilder; import org.hibernate.boot.registry.StandardServiceRegistryBuilder; import org.hibernate.cfg.AvailableSettings; -import org.hibernate.testing.FailureExpected; import org.hibernate.testing.TestForIssue; import org.hibernate.testing.bytecode.enhancement.BytecodeEnhancerRunner; import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase; @@ -63,7 +62,6 @@ public class LazyToOnesProxyWithSubclassesTest extends BaseNonConfigCoreFunction } @Test - @FailureExpected( jiraKey = "HHH-13640") public void testNewProxyAssociation() { inTransaction( session -> { @@ -87,7 +85,6 @@ public class LazyToOnesProxyWithSubclassesTest extends BaseNonConfigCoreFunction } @Test - @FailureExpected( jiraKey = "HHH-13640") public void testReusedProxyAssociation() { inTransaction( session -> { From 3ecdd860a376f7bfb4250b10329bbeb25b045e2e Mon Sep 17 00:00:00 2001 From: Chris Cranford Date: Tue, 17 Sep 2019 23:31:10 -0400 Subject: [PATCH 4/7] HHH-10398 Allow MOD column naming to be driven by a strategy In the past the MOD columns were constructed based on the property name, therefore if users specified a @Column/@JoinColumn like annotation and changed the underlying schema column, the MOD column would continue to be derived based on the property name. This enhancement introduces a new ModifiedColumnNamingStrategy SPI that comes with two implementations, a default/legacy mode that maintains the prior naming model and an improved mode that will derive the MOD name based on the naming strategy ORM used to derive the column name. --- .../userguide/chapters/envers/Envers.adoc | 75 +++++++++++ .../ImprovedModifiedColumnNamingStrategy.java | 80 +++++++++++ .../LegacyModifiedColumnNamingStrategy.java | 47 +++++++ ...umnNamingStrategyRegistrationProvider.java | 45 +++++++ .../spi/ModifiedColumnNamingStrategy.java | 37 +++++ .../envers/configuration/EnversSettings.java | 9 ++ .../internal/GlobalConfiguration.java | 27 ++++ .../metadata/AuditMetadataGenerator.java | 13 +- .../internal/metadata/MetadataTools.java | 20 +++ .../reader/AuditedPropertiesReader.java | 8 +- .../ComponentAuditedPropertiesReader.java | 8 +- .../metadata/reader/PropertyAuditingData.java | 14 ++ ...stry.selector.StrategyRegistrationProvider | 1 + .../ImprovedColumnNamingStrategyTest.java | 52 ++++++++ .../LegacyColumnNamingStrategyTest.java | 42 ++++++ .../modifiedflags/naming/OtherEntity.java | 42 ++++++ .../modifiedflags/naming/OtherEntityId.java | 36 +++++ .../modifiedflags/naming/SingleIdEntity.java | 41 ++++++ .../modifiedflags/naming/TestEmbeddable.java | 34 +++++ .../modifiedflags/naming/TestEntity.java | 126 ++++++++++++++++++ 20 files changed, 739 insertions(+), 18 deletions(-) create mode 100644 hibernate-envers/src/main/java/org/hibernate/envers/boot/internal/ImprovedModifiedColumnNamingStrategy.java create mode 100644 hibernate-envers/src/main/java/org/hibernate/envers/boot/internal/LegacyModifiedColumnNamingStrategy.java create mode 100644 hibernate-envers/src/main/java/org/hibernate/envers/boot/internal/ModifiedColumnNamingStrategyRegistrationProvider.java create mode 100644 hibernate-envers/src/main/java/org/hibernate/envers/boot/spi/ModifiedColumnNamingStrategy.java create mode 100644 hibernate-envers/src/main/resources/META-INF/services/org.hibernate.boot.registry.selector.StrategyRegistrationProvider create mode 100644 hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/ImprovedColumnNamingStrategyTest.java create mode 100644 hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/LegacyColumnNamingStrategyTest.java create mode 100644 hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/OtherEntity.java create mode 100644 hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/OtherEntityId.java create mode 100644 hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/SingleIdEntity.java create mode 100644 hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/TestEmbeddable.java create mode 100644 hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/TestEntity.java diff --git a/documentation/src/main/asciidoc/userguide/chapters/envers/Envers.adoc b/documentation/src/main/asciidoc/userguide/chapters/envers/Envers.adoc index 95eefaa5a5..77a3d1f868 100644 --- a/documentation/src/main/asciidoc/userguide/chapters/envers/Envers.adoc +++ b/documentation/src/main/asciidoc/userguide/chapters/envers/Envers.adoc @@ -286,6 +286,9 @@ The suffix for columns storing "Modified Flags". + For example, a property called "age", will by default get modified flag with column name "age_MOD". +`*org.hibernate.envers.modified_column_naming_strategy*` (default: `org.hibernate.envers.boot.internal.LegacyModifiedColumnNamingStrategy` ):: +The naming strategy to be used for modified flag columns in the audit metadata. + `*org.hibernate.envers.embeddable_set_ordinal_field_name*` (default: `SETORDINAL` ):: Name of column used for storing ordinal of the change in sets of embeddable elements. @@ -314,6 +317,7 @@ The following configuration options have been added recently and should be regar . `org.hibernate.envers.track_entities_changed_in_revision` . `org.hibernate.envers.using_modified_flag` . `org.hibernate.envers.modified_flag_suffix` +. `org.hibernate.envers.modified_column_naming_strategy` . `org.hibernate.envers.original_id_prop_name` . `org.hibernate.envers.find_by_revision_exact_match` ==== @@ -721,6 +725,77 @@ include::{extrasdir}/envers-tracking-properties-changes-example.sql[] To see how "Modified Flags" can be utilized, check out the very simple query API that uses them: <>. +[[envers-tracking-properties-changes-strategy]] +=== Selecting strategy for tracking property level changes + +By default, Envers uses the `legacy` modified column naming strategy. +This strategy is designed to add columns based the following rule-set: + +. If property is annotated with `@Audited` and the _modifiedColumnName_ attribute is specified, the column will directly be based on the supplied name. +. If property is not annotated with `@Audited` or if no _modifiedColumnName_ attribute is given, the column will be named after the java class property, appended with the configured suffix, the default being `_MOD`. + +While this strategy has no performance drawbacks, it does present concerns for users who prefer consistency without verbosity. +Lets take the following entity mapping as an example. + +``` +@Audited(withModifiedFlags = true) +@Entity +public class Customer { + @Id + private Integer id; + @Column(name = "customer_name") + private String name; +} +``` + +This mapping will actually lead to some inconsistent naming between columns, see below with how the model's name will be stored in `customer_name` but the modified column that tracks whether this column changes between revisions is named `name_MOD`. + +``` +CREATE TABLE Customer_AUD ( + id bigint not null, + REV integer not null, + REVTYPE tinyint not null, + customer_name varchar(255), + name_MOD boolean, + primary key(id, REV) +) +``` + +An additional strategy called `improved`, aims to address these consistent column naming concerns. +This strategy uses the following rule-set: + +. Property is a Basic type (Single Column valued property) +.. Use the _modifiedColumnName_ directly if one is supplied on the property mapping +.. Otherwise use the resolved ORM column name appended with the modified flag suffix configured value. +. Property is an Association (to-one mapping) with a Foreign Key using a single column +.. Use the _modifiedColumnName_ directly if one is supplied on the property mapping +.. Otherwise use the resolved ORM column name appended with the modified flag suffix configured value. +. Property is an Association (to-one mapping) with a Foreign Key using multiple columns +.. Use the _modifiedColumnName_ directly if one is supplied on the property mapping +.. Otherwise use the property name appended with the modified flag suffix configured value. +. Property is an Embeddable +.. Use the _modifiedColumnName_ directly if one is supplied on the property mapping +.. Otherwise use the property name appended with the modified flag suffix configured value. + +While using this strategy, the same `Customer` mapping will generate the following table schema: + +``` +CREATE TABLE Customer_AUD ( + id bigint not null, + REV integer not null, + REVTYPE tinyint not null, + customer_name varchar(255), + customer_name_MOD boolean, + primary key(id, REV) +) +``` + +When already using Envers in conjunction with the modified columns flag feature, its advised not to enable the new strategy immediately as schema changes would be required. +You will need to either migrate your existing schema manually to adhere to the rules above or use the explicit _modifiedColumnName_ attribute on the `@Audited` annotation for existing columns that use the feature. + +To configure a custom strategy implementation or use the improved strategy, the configuration option `org.hibernate.envers.modified_column_naming_strategy` will need to be set. +This option can be the fully qualified class name of a `ModifiedColumnNameStrategy` implementation or `legacy` or `improved` for either of the two provided implementations. + [[envers-queries]] === Queries diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/boot/internal/ImprovedModifiedColumnNamingStrategy.java b/hibernate-envers/src/main/java/org/hibernate/envers/boot/internal/ImprovedModifiedColumnNamingStrategy.java new file mode 100644 index 0000000000..3fa797cbad --- /dev/null +++ b/hibernate-envers/src/main/java/org/hibernate/envers/boot/internal/ImprovedModifiedColumnNamingStrategy.java @@ -0,0 +1,80 @@ +/* + * 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.envers.boot.internal; + +import org.hibernate.envers.boot.spi.ModifiedColumnNamingStrategy; +import org.hibernate.envers.configuration.internal.GlobalConfiguration; +import org.hibernate.envers.configuration.internal.metadata.MetadataTools; +import org.hibernate.envers.configuration.internal.metadata.reader.PropertyAuditingData; +import org.hibernate.envers.internal.tools.StringTools; +import org.hibernate.mapping.Column; +import org.hibernate.mapping.Selectable; +import org.hibernate.mapping.Value; +import org.hibernate.type.BasicType; +import org.hibernate.type.ManyToOneType; +import org.hibernate.type.OneToOneType; + +import org.dom4j.Element; + +/** + * A {@link ModifiedColumnNamingStrategy} that adds modified columns with the following rules: + *
    + *
  • For basic types, prioritizes audit annotation naming followed by physical column name appended with suffix.
    • + *
  • For associations with single column foreign keys, behaves like basic types.
    • + *
  • For associations with multiple column foreign keys, prioritizes audit annotation naming followed by using property name.
    • + *
  • For embeddables, behaves like associations with multiple column foreign keys
    • + *
+ * + * @author Chris Cranford + * @since 5.4.6 + */ +public class ImprovedModifiedColumnNamingStrategy implements ModifiedColumnNamingStrategy { + @Override + public void addModifiedColumns( + GlobalConfiguration globalCfg, + Value value, + Element parent, + PropertyAuditingData propertyAuditingData) { + + boolean basicType = value.getType() instanceof BasicType; + boolean toOneType = value.getType() instanceof ManyToOneType || value.getType() instanceof OneToOneType; + + if ( basicType || toOneType ) { + if ( value.getColumnSpan() == 1 ) { + Selectable selectable = value.getColumnIterator().next(); + if ( selectable instanceof Column ) { + // This should not be applied for formulas + String columnName = propertyAuditingData.getModifiedFlagName(); + if ( !propertyAuditingData.isModifiedFlagNameExplicitlySpecified() ) { + columnName = ( (Column) selectable ).getName() + globalCfg.getModifiedFlagSuffix(); + } + else { + columnName = propertyAuditingData.getExplicitModifiedFlagName(); + } + + MetadataTools.addModifiedFlagPropertyWithColumn( + parent, + propertyAuditingData.getName(), + globalCfg.getModifiedFlagSuffix(), + propertyAuditingData.getModifiedFlagName(), + columnName + ); + + return; + } + } + } + + // Default legacy behavior + MetadataTools.addModifiedFlagProperty( + parent, + propertyAuditingData.getName(), + globalCfg.getModifiedFlagSuffix(), + propertyAuditingData.getModifiedFlagName() + ); + } +} diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/boot/internal/LegacyModifiedColumnNamingStrategy.java b/hibernate-envers/src/main/java/org/hibernate/envers/boot/internal/LegacyModifiedColumnNamingStrategy.java new file mode 100644 index 0000000000..528aea71e1 --- /dev/null +++ b/hibernate-envers/src/main/java/org/hibernate/envers/boot/internal/LegacyModifiedColumnNamingStrategy.java @@ -0,0 +1,47 @@ +/* + * 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.envers.boot.internal; + +import org.hibernate.envers.boot.spi.ModifiedColumnNamingStrategy; +import org.hibernate.envers.configuration.internal.GlobalConfiguration; +import org.hibernate.envers.configuration.internal.metadata.MetadataTools; +import org.hibernate.envers.configuration.internal.metadata.reader.PropertyAuditingData; +import org.hibernate.mapping.Value; + +import org.dom4j.Element; + +/** + * A {@link ModifiedColumnNamingStrategy} that adds modified columns with the following rules: + *
    + *
  • If an audit annotation modified column name is supplied, use it directly with no suffix.
    • + *
  • If no audit annotation modified column name is present, use the property name appended with suffix.
    • + *
+ * + * This is the default Envers modified column naming behavior. + * + * @author Chris Cranford + * @since 5.4.6 + */ +public class LegacyModifiedColumnNamingStrategy implements ModifiedColumnNamingStrategy { + @Override + public void addModifiedColumns( + GlobalConfiguration globalCfg, + Value value, + Element parent, + PropertyAuditingData propertyAuditingData) { + String columnName = propertyAuditingData.getModifiedFlagName(); + if ( propertyAuditingData.isModifiedFlagNameExplicitlySpecified() ) { + columnName = propertyAuditingData.getExplicitModifiedFlagName(); + } + MetadataTools.addModifiedFlagProperty( + parent, + propertyAuditingData.getName(), + globalCfg.getModifiedFlagSuffix(), + columnName + ); + } +} diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/boot/internal/ModifiedColumnNamingStrategyRegistrationProvider.java b/hibernate-envers/src/main/java/org/hibernate/envers/boot/internal/ModifiedColumnNamingStrategyRegistrationProvider.java new file mode 100644 index 0000000000..889eddb3e4 --- /dev/null +++ b/hibernate-envers/src/main/java/org/hibernate/envers/boot/internal/ModifiedColumnNamingStrategyRegistrationProvider.java @@ -0,0 +1,45 @@ +/* + * 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.envers.boot.internal; + +import java.util.ArrayList; +import java.util.List; + +import org.hibernate.boot.registry.selector.SimpleStrategyRegistrationImpl; +import org.hibernate.boot.registry.selector.StrategyRegistration; +import org.hibernate.boot.registry.selector.StrategyRegistrationProvider; +import org.hibernate.envers.boot.spi.ModifiedColumnNamingStrategy; + +/** + * A {@link StrategyRegistrationProvider} for {@link ModifiedColumnNamingStrategy}s. + * + * @author Chris Cranford + */ +public class ModifiedColumnNamingStrategyRegistrationProvider implements StrategyRegistrationProvider { + @Override + public Iterable getStrategyRegistrations() { + final List registrations = new ArrayList<>(); + + registrations.add( + new SimpleStrategyRegistrationImpl( + ModifiedColumnNamingStrategy.class, + LegacyModifiedColumnNamingStrategy.class, + "default", "legacy" + ) + ); + + registrations.add( + new SimpleStrategyRegistrationImpl( + ModifiedColumnNamingStrategy.class, + ImprovedModifiedColumnNamingStrategy.class, + "improved" + ) + ); + + return registrations; + } +} diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/boot/spi/ModifiedColumnNamingStrategy.java b/hibernate-envers/src/main/java/org/hibernate/envers/boot/spi/ModifiedColumnNamingStrategy.java new file mode 100644 index 0000000000..9b624affaa --- /dev/null +++ b/hibernate-envers/src/main/java/org/hibernate/envers/boot/spi/ModifiedColumnNamingStrategy.java @@ -0,0 +1,37 @@ +/* + * 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.envers.boot.spi; + +import org.hibernate.Incubating; +import org.hibernate.envers.configuration.internal.GlobalConfiguration; +import org.hibernate.envers.configuration.internal.metadata.reader.PropertyAuditingData; +import org.hibernate.mapping.Value; + +import org.dom4j.Element; + +/** + * Defines a naming strategy for applying modified columns to the audited entity metamodel. + * + * @author Chris Cranford + * @since 5.4.6 + */ +@Incubating +public interface ModifiedColumnNamingStrategy { + /** + * Adds modified columns to the audited entity metamodel. + * + * @param globalCfg the envers global configuration + * @param value the property value + * @param parent the parent audited entity metamodel + * @param propertyAuditingData the property auditing data + */ + void addModifiedColumns( + GlobalConfiguration globalCfg, + Value value, + Element parent, + PropertyAuditingData propertyAuditingData); +} diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/EnversSettings.java b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/EnversSettings.java index b789041e5a..ecda1490d7 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/EnversSettings.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/EnversSettings.java @@ -133,4 +133,13 @@ public interface EnversSettings { * @since 5.4.4 */ String FIND_BY_REVISION_EXACT_MATCH = "org.hibernate.envers.find_by_revision_exact_match"; + + /** + * Specifies the {@link org.hibernate.envers.boot.spi.ModifiedColumnNamingStrategy} to use + * + * Defaults to {@link org.hibernate.envers.boot.internal.LegacyModifiedColumnNamingStrategy}. + * + * @since 5.4.6 + */ + String MODIFIED_COLUMN_NAMING_STRATEGY = "org.hibernate.envers.modified_column_naming_strategy"; } diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/GlobalConfiguration.java b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/GlobalConfiguration.java index 359a07bfb1..c9f7eb0db5 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/GlobalConfiguration.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/GlobalConfiguration.java @@ -7,13 +7,17 @@ package org.hibernate.envers.configuration.internal; import java.util.Map; +import java.util.concurrent.Callable; import org.hibernate.MappingException; import org.hibernate.boot.registry.classloading.spi.ClassLoadingException; +import org.hibernate.boot.registry.selector.spi.StrategySelector; import org.hibernate.cfg.Environment; import org.hibernate.dialect.HSQLDialect; import org.hibernate.envers.RevisionListener; import org.hibernate.envers.boot.internal.EnversService; +import org.hibernate.envers.boot.internal.LegacyModifiedColumnNamingStrategy; +import org.hibernate.envers.boot.spi.ModifiedColumnNamingStrategy; import org.hibernate.envers.configuration.EnversSettings; import org.hibernate.envers.internal.tools.ReflectionTools; import org.hibernate.internal.util.config.ConfigurationHelper; @@ -77,11 +81,15 @@ public class GlobalConfiguration { */ private final String correlatedSubqueryOperator; + private final ModifiedColumnNamingStrategy modifiedColumnNamingStrategy; + public GlobalConfiguration( EnversService enversService, Map properties) { this.enversService = enversService; + final StrategySelector strategySelector = enversService.getServiceRegistry().getService( StrategySelector.class ); + generateRevisionsForCollections = ConfigurationHelper.getBoolean( EnversSettings.REVISION_ON_COLLECTION_CHANGE, properties, @@ -156,6 +164,21 @@ public class GlobalConfiguration { revisionListenerClass = null; } + modifiedColumnNamingStrategy = strategySelector.resolveDefaultableStrategy( + ModifiedColumnNamingStrategy.class, + properties.get( EnversSettings.MODIFIED_COLUMN_NAMING_STRATEGY ), + new Callable() { + @Override + public ModifiedColumnNamingStrategy call() throws Exception { + return strategySelector.resolveDefaultableStrategy( + ModifiedColumnNamingStrategy.class, + "default", + new LegacyModifiedColumnNamingStrategy() + ); + } + } + ); + allowIdentifierReuse = ConfigurationHelper.getBoolean( EnversSettings.ALLOW_IDENTIFIER_REUSE, properties, false ); @@ -232,4 +255,8 @@ public class GlobalConfiguration { public boolean isAuditReaderFindAtRevisionExactMatch() { return findByRevisionExactMatch; } + + public ModifiedColumnNamingStrategy getModifiedColumnNamingStrategy() { + return modifiedColumnNamingStrategy; + } } diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/AuditMetadataGenerator.java b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/AuditMetadataGenerator.java index e5b630d5df..4de1e90b8b 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/AuditMetadataGenerator.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/AuditMetadataGenerator.java @@ -213,7 +213,7 @@ public final class AuditMetadataGenerator { } return; } - addModifiedFlagIfNeeded( parent, propertyAuditingData, processModifiedFlag ); + addModifiedFlagIfNeeded( value, parent, propertyAuditingData, processModifiedFlag ); } private boolean processedInSecondPass(Type type) { @@ -290,19 +290,20 @@ public final class AuditMetadataGenerator { else { return; } - addModifiedFlagIfNeeded( parent, propertyAuditingData, processModifiedFlag ); + addModifiedFlagIfNeeded( value, parent, propertyAuditingData, processModifiedFlag ); } private void addModifiedFlagIfNeeded( + Value value, Element parent, PropertyAuditingData propertyAuditingData, boolean processModifiedFlag) { if ( processModifiedFlag && propertyAuditingData.isUsingModifiedFlag() ) { - MetadataTools.addModifiedFlagProperty( + globalCfg.getModifiedColumnNamingStrategy().addModifiedColumns( + globalCfg, + value, parent, - propertyAuditingData.getName(), - globalCfg.getModifiedFlagSuffix(), - propertyAuditingData.getModifiedFlagName() + propertyAuditingData ); } } diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/MetadataTools.java b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/MetadataTools.java index cf38c7aa46..77f86d6c84 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/MetadataTools.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/MetadataTools.java @@ -95,6 +95,26 @@ public final class MetadataTools { ); } + public static Element addModifiedFlagPropertyWithColumn( + Element parent, + String propertyName, + String suffix, + String modifiedFlagName, + String columnName) { + final Element property = addProperty( + parent, + (modifiedFlagName != null) ? modifiedFlagName : getModifiedFlagPropertyName( propertyName, suffix ), + "boolean", + true, + false, + false + ); + + addColumn( property, columnName, null, null, null, null, null, null ); + + return property; + } + public static String getModifiedFlagPropertyName(String propertyName, String suffix) { return propertyName + suffix; } diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/AuditedPropertiesReader.java b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/AuditedPropertiesReader.java index 845439ddd4..45823cb63f 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/AuditedPropertiesReader.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/AuditedPropertiesReader.java @@ -590,13 +590,9 @@ public class AuditedPropertiesReader { propertyData.setStore( aud.modStore() ); propertyData.setRelationTargetAuditMode( aud.targetAuditMode() ); propertyData.setUsingModifiedFlag( checkUsingModifiedFlag( aud ) ); + propertyData.setModifiedFlagName( MetadataTools.getModifiedFlagPropertyName( propertyName, modifiedFlagSuffix ) ); if( aud.modifiedColumnName() != null && !"".equals( aud.modifiedColumnName() ) ) { - propertyData.setModifiedFlagName( aud.modifiedColumnName() ); - } - else { - propertyData.setModifiedFlagName( - MetadataTools.getModifiedFlagPropertyName( propertyName, modifiedFlagSuffix ) - ); + propertyData.setExplicitModifiedFlagName( aud.modifiedColumnName() ); } return true; } diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/ComponentAuditedPropertiesReader.java b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/ComponentAuditedPropertiesReader.java index decae95ab0..bd5c5965bd 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/ComponentAuditedPropertiesReader.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/ComponentAuditedPropertiesReader.java @@ -44,13 +44,9 @@ public class ComponentAuditedPropertiesReader extends AuditedPropertiesReader { propertyData.setStore( aud.modStore() ); propertyData.setRelationTargetAuditMode( aud.targetAuditMode() ); propertyData.setUsingModifiedFlag( checkUsingModifiedFlag( aud ) ); + propertyData.setModifiedFlagName( MetadataTools.getModifiedFlagPropertyName( propertyName, modifiedFlagSuffix ) ); if( aud.modifiedColumnName() != null && !"".equals( aud.modifiedColumnName() ) ) { - propertyData.setModifiedFlagName( aud.modifiedColumnName() ); - } - else { - propertyData.setModifiedFlagName( - MetadataTools.getModifiedFlagPropertyName( propertyName, modifiedFlagSuffix ) - ); + propertyData.setExplicitModifiedFlagName( aud.modifiedColumnName() ); } } else { diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/PropertyAuditingData.java b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/PropertyAuditingData.java index 2e717547da..c8274fe142 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/PropertyAuditingData.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/PropertyAuditingData.java @@ -17,6 +17,7 @@ import org.hibernate.envers.AuditOverrides; import org.hibernate.envers.ModificationStore; import org.hibernate.envers.RelationTargetAuditMode; import org.hibernate.envers.internal.entities.PropertyData; +import org.hibernate.envers.internal.tools.StringTools; import org.hibernate.mapping.Value; import org.hibernate.type.Type; @@ -41,6 +42,7 @@ public class PropertyAuditingData { private boolean forceInsertable; private boolean usingModifiedFlag; private String modifiedFlagName; + private String explicitModifiedFlagName; private Value value; // Synthetic properties are ones which are not part of the actual java model. // They're properties used for bookkeeping by Hibernate @@ -237,6 +239,18 @@ public class PropertyAuditingData { this.modifiedFlagName = modifiedFlagName; } + public boolean isModifiedFlagNameExplicitlySpecified() { + return !StringTools.isEmpty( explicitModifiedFlagName ); + } + + public String getExplicitModifiedFlagName() { + return explicitModifiedFlagName; + } + + public void setExplicitModifiedFlagName(String modifiedFlagName) { + this.explicitModifiedFlagName = modifiedFlagName; + } + public void addAuditingOverride(AuditOverride annotation) { if ( annotation != null ) { final String overrideName = annotation.name(); diff --git a/hibernate-envers/src/main/resources/META-INF/services/org.hibernate.boot.registry.selector.StrategyRegistrationProvider b/hibernate-envers/src/main/resources/META-INF/services/org.hibernate.boot.registry.selector.StrategyRegistrationProvider new file mode 100644 index 0000000000..394e0d7168 --- /dev/null +++ b/hibernate-envers/src/main/resources/META-INF/services/org.hibernate.boot.registry.selector.StrategyRegistrationProvider @@ -0,0 +1 @@ +org.hibernate.envers.boot.internal.ModifiedColumnNamingStrategyRegistrationProvider \ No newline at end of file diff --git a/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/ImprovedColumnNamingStrategyTest.java b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/ImprovedColumnNamingStrategyTest.java new file mode 100644 index 0000000000..c385bde606 --- /dev/null +++ b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/ImprovedColumnNamingStrategyTest.java @@ -0,0 +1,52 @@ +/* + * 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.envers.test.integration.modifiedflags.naming; + +import java.util.Map; + +import org.hibernate.envers.configuration.EnversSettings; +import org.hibernate.envers.test.BaseEnversJPAFunctionalTestCase; +import org.hibernate.mapping.Column; +import org.hibernate.mapping.Table; +import org.junit.Test; + +import static org.junit.Assert.assertNotNull; + +/** + * @author Chris Cranford + */ +public class ImprovedColumnNamingStrategyTest extends BaseEnversJPAFunctionalTestCase { + @Override + protected void addConfigOptions(Map options) { + super.addConfigOptions( options ); + + options.put( EnversSettings.MODIFIED_COLUMN_NAMING_STRATEGY, "improved" ); + } + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { TestEntity.class, OtherEntity.class, SingleIdEntity.class }; + } + + @Test + public void testModifiedColumns() { + final Table table1 = metadata().getEntityBinding( TestEntity.class.getName() + "_AUD" ).getTable(); + assertNotNull( table1.getColumn( new Column( "data1_MOD") ) ); + assertNotNull( table1.getColumn( new Column( "mydata_MOD" ) ) ); + assertNotNull( table1.getColumn( new Column( "data_3" ) ) ); + assertNotNull( table1.getColumn( new Column( "the_data_mod" ) ) ); + + assertNotNull( table1.getColumn( new Column( "embeddable_MOD" ) ) ); + assertNotNull( table1.getColumn( new Column( "otherEntity_MOD" ) ) ); + assertNotNull( table1.getColumn( new Column( "single_id_MOD" ) ) ); + assertNotNull( table1.getColumn( new Column( "singleIdEntity2_id_MOD" ) ) ); + + final Table table2 = metadata().getEntityBinding( OtherEntity.class.getName() + "_AUD" ).getTable(); + assertNotNull( table2.getColumn( new Column( "d_MOD" ) ) ); + } + +} diff --git a/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/LegacyColumnNamingStrategyTest.java b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/LegacyColumnNamingStrategyTest.java new file mode 100644 index 0000000000..8c1bcdd171 --- /dev/null +++ b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/LegacyColumnNamingStrategyTest.java @@ -0,0 +1,42 @@ +/* + * 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.envers.test.integration.modifiedflags.naming; + +import org.hibernate.envers.test.BaseEnversJPAFunctionalTestCase; +import org.hibernate.mapping.Column; +import org.hibernate.mapping.Table; +import org.junit.Test; + +import static org.junit.Assert.assertNotNull; + +/** + * @author Chris Cranford + */ +public class LegacyColumnNamingStrategyTest extends BaseEnversJPAFunctionalTestCase { + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { TestEntity.class, OtherEntity.class, SingleIdEntity.class }; + } + + @Test + public void testModifiedColumns() { + final Table table1 = metadata().getEntityBinding( TestEntity.class.getName() + "_AUD" ).getTable(); + assertNotNull( table1.getColumn( new Column( "data1_MOD") ) ); + assertNotNull( table1.getColumn( new Column( "data2_MOD" ) ) ); + assertNotNull( table1.getColumn( new Column( "data_3" ) ) ); + assertNotNull( table1.getColumn( new Column( "the_data_mod" ) ) ); + + assertNotNull( table1.getColumn( new Column( "embeddable_MOD" ) ) ); + assertNotNull( table1.getColumn( new Column( "otherEntity_MOD" ) ) ); + assertNotNull( table1.getColumn( new Column( "singleIdEntity_MOD" ) ) ); + assertNotNull( table1.getColumn( new Column( "singleIdEntity2_MOD" ) ) ); + + final Table table2 = metadata().getEntityBinding( OtherEntity.class.getName() + "_AUD" ).getTable(); + assertNotNull( table2.getColumn( new Column( "data_MOD" ) ) ); + } + +} diff --git a/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/OtherEntity.java b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/OtherEntity.java new file mode 100644 index 0000000000..676e397ccf --- /dev/null +++ b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/OtherEntity.java @@ -0,0 +1,42 @@ +/* + * 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.envers.test.integration.modifiedflags.naming; + +import javax.persistence.Column; +import javax.persistence.EmbeddedId; +import javax.persistence.Entity; + +import org.hibernate.envers.Audited; + +/** + * @author Chris Cranford + */ +@Entity +@Audited(withModifiedFlag = true) +public class OtherEntity { + @EmbeddedId + private OtherEntityId id; + + @Column(name = "d") + private String data; + + public OtherEntityId getId() { + return id; + } + + public void setId(OtherEntityId id) { + this.id = id; + } + + public String getData() { + return data; + } + + public void setData(String data) { + this.data = data; + } +} diff --git a/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/OtherEntityId.java b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/OtherEntityId.java new file mode 100644 index 0000000000..5d3db71e52 --- /dev/null +++ b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/OtherEntityId.java @@ -0,0 +1,36 @@ +/* + * 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.envers.test.integration.modifiedflags.naming; + +import java.io.Serializable; + +import javax.persistence.Embeddable; + +/** + * @author Chris Cranford + */ +@Embeddable +public class OtherEntityId implements Serializable { + private Integer id1; + private Integer id2; + + public Integer getId1() { + return id1; + } + + public void setId1(Integer id1) { + this.id1 = id1; + } + + public Integer getId2() { + return id2; + } + + public void setId2(Integer id2) { + this.id2 = id2; + } +} diff --git a/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/SingleIdEntity.java b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/SingleIdEntity.java new file mode 100644 index 0000000000..adeef99c22 --- /dev/null +++ b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/SingleIdEntity.java @@ -0,0 +1,41 @@ +/* + * 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.envers.test.integration.modifiedflags.naming; + +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; + +import org.hibernate.envers.Audited; + +/** + * @author Chris Cranford + */ +@Entity +@Audited(withModifiedFlag = true) +public class SingleIdEntity { + @Id + @GeneratedValue + private Integer id; + private String name; + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } +} diff --git a/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/TestEmbeddable.java b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/TestEmbeddable.java new file mode 100644 index 0000000000..1ea22bd87e --- /dev/null +++ b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/TestEmbeddable.java @@ -0,0 +1,34 @@ +/* + * 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.envers.test.integration.modifiedflags.naming; + +import javax.persistence.Embeddable; + +/** + * @author Chris Cranford + */ +@Embeddable +public class TestEmbeddable { + String value1; + String value2; + + public String getValue1() { + return value1; + } + + public void setValue1(String value1) { + this.value1 = value1; + } + + public String getValue2() { + return value2; + } + + public void setValue2(String value2) { + this.value2 = value2; + } +} diff --git a/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/TestEntity.java b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/TestEntity.java new file mode 100644 index 0000000000..66afbf257f --- /dev/null +++ b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/modifiedflags/naming/TestEntity.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.envers.test.integration.modifiedflags.naming; + +import javax.persistence.Column; +import javax.persistence.Embedded; +import javax.persistence.EmbeddedId; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; +import javax.persistence.JoinColumn; +import javax.persistence.JoinColumns; +import javax.persistence.ManyToOne; +import javax.persistence.OneToOne; + +import org.hibernate.envers.Audited; + +/** + * @author Chris Cranford + */ +@Entity +@Audited(withModifiedFlag = true) +public class TestEntity { + @Id + @GeneratedValue + private Integer id; + private String data1; + @Column(name = "mydata") + private String data2; + @Audited(modifiedColumnName = "data_3", withModifiedFlag = true) + private String data3; + @Column(name = "thedata") + @Audited(modifiedColumnName = "the_data_mod", withModifiedFlag = true) + private String data4; + @Embedded + private TestEmbeddable embeddable; + @ManyToOne + @JoinColumns({ + @JoinColumn(name = "other_entity_id1", nullable = false), + @JoinColumn(name = "other_entity_id2", nullable = false) + }) + private OtherEntity otherEntity; + + @OneToOne + @JoinColumn(name = "single_id") + private SingleIdEntity singleIdEntity; + + @OneToOne + private SingleIdEntity singleIdEntity2; + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public String getData1() { + return data1; + } + + public void setData1(String data1) { + this.data1 = data1; + } + + public String getData2() { + return data2; + } + + public void setData2(String data2) { + this.data2 = data2; + } + + public String getData3() { + return data3; + } + + public void setData3(String data3) { + this.data3 = data3; + } + + public String getData4() { + return data4; + } + + public void setData4(String data4) { + this.data4 = data4; + } + + public TestEmbeddable getEmbeddable() { + return embeddable; + } + + public void setEmbeddable(TestEmbeddable embeddable) { + this.embeddable = embeddable; + } + + public OtherEntity getOtherEntity() { + return otherEntity; + } + + public void setOtherEntity(OtherEntity otherEntity) { + this.otherEntity = otherEntity; + } + + public SingleIdEntity getSingleIdEntity() { + return singleIdEntity; + } + + public void setSingleIdEntity(SingleIdEntity singleIdEntity) { + this.singleIdEntity = singleIdEntity; + } + + public SingleIdEntity getSingleIdEntity2() { + return singleIdEntity2; + } + + public void setSingleIdEntity2(SingleIdEntity singleIdEntity2) { + this.singleIdEntity2 = singleIdEntity2; + } +} From f78877a607f478841df9b9b462655501735384fa Mon Sep 17 00:00:00 2001 From: Chris Cranford Date: Wed, 18 Sep 2019 17:38:27 -0400 Subject: [PATCH 5/7] HHH-10398 Allow MOD column naming to be driven by a strategy * Fixes OSGI integration with missing service lookup registration --- ...ot.registry.selector.StrategyRegistrationProvider | 12 ++++++++++++ .../main/resources/OSGI-INF/blueprint/blueprint.xml | 5 ++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/hibernate-envers/src/main/resources/META-INF/services/org.hibernate.boot.registry.selector.StrategyRegistrationProvider b/hibernate-envers/src/main/resources/META-INF/services/org.hibernate.boot.registry.selector.StrategyRegistrationProvider index 394e0d7168..350bf85966 100644 --- a/hibernate-envers/src/main/resources/META-INF/services/org.hibernate.boot.registry.selector.StrategyRegistrationProvider +++ b/hibernate-envers/src/main/resources/META-INF/services/org.hibernate.boot.registry.selector.StrategyRegistrationProvider @@ -1 +1,13 @@ +# +# 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 . +# +# +# 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 . +# org.hibernate.envers.boot.internal.ModifiedColumnNamingStrategyRegistrationProvider \ No newline at end of file diff --git a/hibernate-envers/src/main/resources/OSGI-INF/blueprint/blueprint.xml b/hibernate-envers/src/main/resources/OSGI-INF/blueprint/blueprint.xml index fbb928cee0..5765c14a7a 100644 --- a/hibernate-envers/src/main/resources/OSGI-INF/blueprint/blueprint.xml +++ b/hibernate-envers/src/main/resources/OSGI-INF/blueprint/blueprint.xml @@ -21,5 +21,8 @@ - + + + + From b606759e61cfaf128c500d889ec743f50f10ad98 Mon Sep 17 00:00:00 2001 From: Chris Cranford Date: Mon, 30 Sep 2019 14:00:22 -0400 Subject: [PATCH 6/7] HHH-10398 Allow MOD column naming to be driven by a strategy * Fixed javadoc comments * Various code suggested code changes from review --- .../ImprovedModifiedColumnNamingStrategy.java | 14 ++++---------- .../LegacyModifiedColumnNamingStrategy.java | 7 +++++-- .../boot/spi/ModifiedColumnNamingStrategy.java | 2 +- .../envers/configuration/EnversSettings.java | 2 +- 4 files changed, 11 insertions(+), 14 deletions(-) diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/boot/internal/ImprovedModifiedColumnNamingStrategy.java b/hibernate-envers/src/main/java/org/hibernate/envers/boot/internal/ImprovedModifiedColumnNamingStrategy.java index 3fa797cbad..75741c4831 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/boot/internal/ImprovedModifiedColumnNamingStrategy.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/boot/internal/ImprovedModifiedColumnNamingStrategy.java @@ -10,7 +10,6 @@ import org.hibernate.envers.boot.spi.ModifiedColumnNamingStrategy; import org.hibernate.envers.configuration.internal.GlobalConfiguration; import org.hibernate.envers.configuration.internal.metadata.MetadataTools; import org.hibernate.envers.configuration.internal.metadata.reader.PropertyAuditingData; -import org.hibernate.envers.internal.tools.StringTools; import org.hibernate.mapping.Column; import org.hibernate.mapping.Selectable; import org.hibernate.mapping.Value; @@ -30,9 +29,9 @@ import org.dom4j.Element; * * * @author Chris Cranford - * @since 5.4.6 + * @since 5.4.7 */ -public class ImprovedModifiedColumnNamingStrategy implements ModifiedColumnNamingStrategy { +public class ImprovedModifiedColumnNamingStrategy extends LegacyModifiedColumnNamingStrategy { @Override public void addModifiedColumns( GlobalConfiguration globalCfg, @@ -48,7 +47,7 @@ public class ImprovedModifiedColumnNamingStrategy implements ModifiedColumnNamin Selectable selectable = value.getColumnIterator().next(); if ( selectable instanceof Column ) { // This should not be applied for formulas - String columnName = propertyAuditingData.getModifiedFlagName(); + final String columnName; if ( !propertyAuditingData.isModifiedFlagNameExplicitlySpecified() ) { columnName = ( (Column) selectable ).getName() + globalCfg.getModifiedFlagSuffix(); } @@ -70,11 +69,6 @@ public class ImprovedModifiedColumnNamingStrategy implements ModifiedColumnNamin } // Default legacy behavior - MetadataTools.addModifiedFlagProperty( - parent, - propertyAuditingData.getName(), - globalCfg.getModifiedFlagSuffix(), - propertyAuditingData.getModifiedFlagName() - ); + super.addModifiedColumns( globalCfg, value, parent, propertyAuditingData ); } } diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/boot/internal/LegacyModifiedColumnNamingStrategy.java b/hibernate-envers/src/main/java/org/hibernate/envers/boot/internal/LegacyModifiedColumnNamingStrategy.java index 528aea71e1..3d035330e2 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/boot/internal/LegacyModifiedColumnNamingStrategy.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/boot/internal/LegacyModifiedColumnNamingStrategy.java @@ -24,7 +24,7 @@ import org.dom4j.Element; * This is the default Envers modified column naming behavior. * * @author Chris Cranford - * @since 5.4.6 + * @since 5.4.7 */ public class LegacyModifiedColumnNamingStrategy implements ModifiedColumnNamingStrategy { @Override @@ -33,10 +33,13 @@ public class LegacyModifiedColumnNamingStrategy implements ModifiedColumnNamingS Value value, Element parent, PropertyAuditingData propertyAuditingData) { - String columnName = propertyAuditingData.getModifiedFlagName(); + final String columnName; if ( propertyAuditingData.isModifiedFlagNameExplicitlySpecified() ) { columnName = propertyAuditingData.getExplicitModifiedFlagName(); } + else { + columnName = propertyAuditingData.getModifiedFlagName(); + } MetadataTools.addModifiedFlagProperty( parent, propertyAuditingData.getName(), diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/boot/spi/ModifiedColumnNamingStrategy.java b/hibernate-envers/src/main/java/org/hibernate/envers/boot/spi/ModifiedColumnNamingStrategy.java index 9b624affaa..b38f8cc734 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/boot/spi/ModifiedColumnNamingStrategy.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/boot/spi/ModifiedColumnNamingStrategy.java @@ -17,7 +17,7 @@ import org.dom4j.Element; * Defines a naming strategy for applying modified columns to the audited entity metamodel. * * @author Chris Cranford - * @since 5.4.6 + * @since 5.4.7 */ @Incubating public interface ModifiedColumnNamingStrategy { diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/EnversSettings.java b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/EnversSettings.java index ecda1490d7..b1a079f903 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/EnversSettings.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/EnversSettings.java @@ -139,7 +139,7 @@ public interface EnversSettings { * * Defaults to {@link org.hibernate.envers.boot.internal.LegacyModifiedColumnNamingStrategy}. * - * @since 5.4.6 + * @since 5.4.7 */ String MODIFIED_COLUMN_NAMING_STRATEGY = "org.hibernate.envers.modified_column_naming_strategy"; } From 616f549f8cc50c6eb70dd2c8a5fcb2772af0f68b Mon Sep 17 00:00:00 2001 From: Gail Badner Date: Tue, 1 Oct 2019 17:32:45 -0700 Subject: [PATCH 7/7] HHH-13640 : Added failing test where a proxy is found in PersistenceContext, but it does not have a subclass --- .../LazyToOnesProxyWithSubclassesTest.java | 40 ++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/proxy/LazyToOnesProxyWithSubclassesTest.java b/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/proxy/LazyToOnesProxyWithSubclassesTest.java index 8ff06a1cc6..da3d77f093 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/proxy/LazyToOnesProxyWithSubclassesTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/proxy/LazyToOnesProxyWithSubclassesTest.java @@ -21,7 +21,9 @@ import org.hibernate.boot.MetadataSources; import org.hibernate.boot.SessionFactoryBuilder; import org.hibernate.boot.registry.StandardServiceRegistryBuilder; import org.hibernate.cfg.AvailableSettings; +import org.hibernate.proxy.HibernateProxy; +import org.hibernate.testing.FailureExpected; import org.hibernate.testing.TestForIssue; import org.hibernate.testing.bytecode.enhancement.BytecodeEnhancerRunner; import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase; @@ -78,6 +80,7 @@ public class LazyToOnesProxyWithSubclassesTest extends BaseNonConfigCoreFunction final OtherEntity otherEntity = session.get( OtherEntity.class, "test1" ); assertTrue( Hibernate.isPropertyInitialized( otherEntity, "animal" ) ); assertFalse( Hibernate.isInitialized( otherEntity.animal ) ); + assertTrue( HibernateProxy.class.isInstance( otherEntity.animal ) ); Animal animal = session.load( Animal.class, "A Human" ); assertFalse( Hibernate.isInitialized( animal ) ); } @@ -85,7 +88,7 @@ public class LazyToOnesProxyWithSubclassesTest extends BaseNonConfigCoreFunction } @Test - public void testReusedProxyAssociation() { + public void testExistingProxyAssociation() { inTransaction( session -> { Human human = new Human( "A Human" ); @@ -102,8 +105,43 @@ public class LazyToOnesProxyWithSubclassesTest extends BaseNonConfigCoreFunction final OtherEntity otherEntity = session.get( OtherEntity.class, "test1" ); assertTrue( Hibernate.isPropertyInitialized( otherEntity, "animal" ) ); assertFalse( Hibernate.isInitialized( otherEntity.animal ) ); + assertTrue( HibernateProxy.class.isInstance( otherEntity.animal ) ); assertTrue( Hibernate.isPropertyInitialized( otherEntity, "primate" ) ); assertFalse( Hibernate.isInitialized( otherEntity.primate ) ); + assertTrue( HibernateProxy.class.isInstance( otherEntity.primate ) ); + + } + ); + } + + @Test + @FailureExpected( jiraKey = "HHH-13640" ) + public void testExistingProxyAssociationLeafSubclass() { + inTransaction( + session -> { + Human human = new Human( "A Human" ); + OtherEntity otherEntity = new OtherEntity( "test1" ); + otherEntity.animal = human; + otherEntity.primate = human; + otherEntity.human = human; + session.persist( human ); + session.persist( otherEntity ); + } + ); + + inSession( + session -> { + final OtherEntity otherEntity = session.get( OtherEntity.class, "test1" ); + assertTrue( Hibernate.isPropertyInitialized( otherEntity, "animal" ) ); + assertFalse( Hibernate.isInitialized( otherEntity.animal ) ); + assertTrue( HibernateProxy.class.isInstance( otherEntity.animal ) ); + assertTrue( Hibernate.isPropertyInitialized( otherEntity, "primate" ) ); + assertFalse( Hibernate.isInitialized( otherEntity.primate ) ); + assertTrue( HibernateProxy.class.isInstance( otherEntity.primate ) ); + assertTrue( Hibernate.isPropertyInitialized( otherEntity, "human" ) ); + assertFalse( Hibernate.isInitialized( otherEntity.human ) ); + // TODO: Should otherEntity.human be a narrowed HibernateProxy or + // an uninitialized non-HibernateProxy proxy? } ); }