HHH-5908 - Avoid unnecessary updates on detached un-modified entities with SelectBeforeUpdate.

HHH-11056 - Envers audits unchanged objects for a certain use case

hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java

@@ -4172,6 +4172,7 @@ public abstract class AbstractEntityPersister
 				current,
 				old,
 				propertyColumnUpdateable,
+				getPropertyUpdateability(),
 				hasUninitializedLazyProperties( entity ),
 				session
 		);

hibernate-core/src/main/java/org/hibernate/type/TypeHelper.java

@@ -7,6 +7,7 @@
 package org.hibernate.type;
 
 import java.io.Serializable;
+import java.util.Arrays;
 import java.util.Map;
 
 import org.hibernate.bytecode.enhance.spi.LazyPropertyInitializer;
@@ -323,6 +324,7 @@ public class TypeHelper {
 	 * @param currentState The current state of the entity
 	 * @param previousState The baseline state of the entity
 	 * @param includeColumns Columns to be included in the mod checking, per property
+	 * @param includeProperties Array of property indices that identify which properties participate in check
 	 * @param anyUninitializedProperties Does the entity currently hold any uninitialized property values?
 	 * @param session The session from which the dirty check request originated.
 	 *
@@ -333,6 +335,7 @@ public class TypeHelper {
 			final Object[] currentState,
 			final Object[] previousState,
 			final boolean[][] includeColumns,
+			final boolean[] includeProperties,
 			final boolean anyUninitializedProperties,
 			final SharedSessionContractImplementor session) {
 		int[] results = null;
@@ -340,15 +343,15 @@ public class TypeHelper {
 		int span = properties.length;
 
 		for ( int i = 0; i < span; i++ ) {
-			final boolean modified = currentState[i]!=LazyPropertyInitializer.UNFETCHED_PROPERTY
-					&& properties[i].isDirtyCheckable(anyUninitializedProperties)
-					&& properties[i].getType().isModified( previousState[i], currentState[i], includeColumns[i], session );
-
+			final boolean modified = currentState[ i ] != LazyPropertyInitializer.UNFETCHED_PROPERTY
+					&& includeProperties[ i ]
+					&& properties[ i ].isDirtyCheckable( anyUninitializedProperties )
+					&& properties[ i ].getType().isModified( previousState[ i ], currentState[ i ], includeColumns[ i ], session );
 			if ( modified ) {
 				if ( results == null ) {
-					results = new int[span];
+					results = new int[ span ];
 				}
-				results[count++] = i;
+				results[ count++ ] = i;
 			}
 		}
 
@@ -356,7 +359,7 @@ public class TypeHelper {
 			return null;
 		}
 		else {
-			int[] trimmed = new int[count];
+			int[] trimmed = new int[ count ];
 			System.arraycopy( results, 0, trimmed, 0, count );
 			return trimmed;
 		}

hibernate-core/src/test/java/org/hibernate/test/annotations/selectbeforeupdate/UpdateDetachedTest.java

@@ -0,0 +1,235 @@
+/*
+ * 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.test.annotations.selectbeforeupdate;
+
+import javax.persistence.Entity;
+import javax.persistence.Id;
+import javax.persistence.JoinColumn;
+import javax.persistence.ManyToOne;
+import javax.persistence.Version;
+
+import org.hibernate.annotations.SelectBeforeUpdate;
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
+import org.hibernate.testing.transaction.TransactionUtil;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author Chris Cranford
+ */
+public class UpdateDetachedTest extends BaseCoreFunctionalTestCase{
+
+	@Override
+	protected Class<?>[] getAnnotatedClasses() {
+		return new Class<?>[] { Foo.class, Bar.class };
+	}
+
+	@Test
+	@TestForIssue(jiraKey = "HHH-5908")
+	public void testUpdateDetachedUnchanged() {
+		final Bar bar = new Bar( 1, "Bar" );
+		final Foo foo = new Foo( 1, "Foo", bar );
+
+		// this should generate versions
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			session.save( bar );
+			session.save( foo );
+		} );
+
+		// this shouldn't generate a new version.
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			session.update( foo );
+		} );
+
+		assertEquals( Integer.valueOf( 0 ), bar.getVersion() );
+		assertEquals( Integer.valueOf( 0 ), foo.getVersion() );
+
+		// this should generate a new version
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			foo.setName( "FooChanged" );
+			session.update( foo );
+		} );
+
+		assertEquals( Integer.valueOf( 0 ), bar.getVersion() );
+		assertEquals( Integer.valueOf( 1 ), foo.getVersion() );
+	}
+
+	@Test
+	@TestForIssue(jiraKey = "HHH-5908")
+	public void testUpdateDetachedChanged() {
+		final Bar bar = new Bar( 2, "Bar" );
+		final Foo foo = new Foo( 2, "Foo", bar );
+
+		// this should generate versions
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			session.save( bar );
+			session.save( foo );
+		} );
+
+		// this should generate a new version
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			foo.setName( "FooChanged" );
+			session.update( foo );
+		} );
+
+		assertEquals( Integer.valueOf( 0 ), bar.getVersion() );
+		assertEquals( Integer.valueOf( 1 ), foo.getVersion() );
+	}
+
+	@Test
+	@TestForIssue(jiraKey = "HHH-5908")
+	public void testUpdateDetachedUnchangedAndChanged() {
+		final Bar bar = new Bar( 3, "Bar" );
+		final Foo foo = new Foo( 3, "Foo", bar );
+
+		// this should generate versions
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			session.save( bar );
+			session.save( foo );
+		} );
+
+		// this shouldn't generate a new version.
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			session.update( foo );
+		} );
+
+		// this should generate a new version
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			foo.setName( "FooChanged" );
+			session.update( foo );
+		} );
+
+		assertEquals( Integer.valueOf( 0 ), bar.getVersion() );
+		assertEquals( Integer.valueOf( 1 ), foo.getVersion() );
+	}
+
+	@Test
+	@TestForIssue(jiraKey = "HHH-5908")
+	public void testUpdateDetachedChangedAndUnchanged() {
+		final Bar bar = new Bar( 4, "Bar" );
+		final Foo foo = new Foo( 4, "Foo", bar );
+
+		// this should generate versions
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			session.save( bar );
+			session.save( foo );
+		} );
+
+		// this should generate a new version
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			foo.setName( "FooChanged" );
+			session.update( foo );
+		} );
+
+		// this shouldn't generate a new version.
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			session.update( foo );
+		} );
+
+		assertEquals( Integer.valueOf( 0 ), bar.getVersion() );
+		assertEquals( Integer.valueOf( 1 ), foo.getVersion() );
+	}
+
+	@Entity(name = "Foo")
+	@SelectBeforeUpdate
+	public static class Foo {
+		@Id
+		private Integer id;
+		private String name;
+		@Version
+		private Integer version;
+		@ManyToOne
+		@JoinColumn(updatable = false)
+		private Bar bar;
+
+		Foo() {
+
+		}
+
+		Foo(Integer id, String name, Bar bar) {
+			this.id = id;
+			this.name = name;
+			this.bar = bar;
+		}
+
+		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;
+		}
+
+		public Bar getBar() {
+			return bar;
+		}
+
+		public void setBar(Bar bar) {
+			this.bar = bar;
+		}
+
+		public Integer getVersion() {
+			return version;
+		}
+
+		public void setVersion(Integer version) {
+			this.version = version;
+		}
+	}
+
+	@Entity(name = "Bar")
+	public static class Bar {
+		@Id
+		private Integer id;
+		private String name;
+		@Version
+		private Integer version;
+
+		Bar() {
+
+		}
+
+		Bar(Integer id, String name) {
+			this.id = id;
+			this.name = 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;
+		}
+
+		public Integer getVersion() {
+			return version;
+		}
+
+		public void setVersion(Integer version) {
+			this.version = version;
+		}
+	}
+}

hibernate-envers/src/test/java/org/hibernate/envers/test/integration/update/SelectBeforeUpdateTest.java

@@ -0,0 +1,222 @@
+/*
+ * 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.envers.test.integration.update;
+
+import javax.persistence.Entity;
+import javax.persistence.Id;
+import javax.persistence.JoinColumn;
+import javax.persistence.ManyToOne;
+
+import org.hibernate.annotations.SelectBeforeUpdate;
+import org.hibernate.envers.Audited;
+import org.hibernate.envers.test.BaseEnversFunctionalTestCase;
+import org.hibernate.envers.test.Priority;
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.transaction.TransactionUtil;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author Chris Cranford
+ */
+@TestForIssue(jiraKey = "HHH-11056")
+public class SelectBeforeUpdateTest extends BaseEnversFunctionalTestCase {
+
+	@Override
+	protected Class[] getAnnotatedClasses() {
+		return new Class<?>[] { Book.class, Author.class };
+	}
+
+	@Test
+	@Priority(10)
+	public void initDataUpdateDetachedUnchanged() {
+		final Author author = new Author( 1, "Author1" );
+		final Book book = new Book( 1, "Book1", author );
+
+		// Revision 1 - insert new entities.
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			session.save( author );
+			session.save( book );
+		} );
+
+		// Revision 2 - update detached with no changes.
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			session.update( book );
+		} );
+	}
+
+	@Test
+	@Priority(9)
+	public void initDataUpdateDetachedChanged() {
+		final Author author = new Author( 2, "Author2" );
+		final Book book = new Book( 2, "Book2", author );
+
+		// Revision 1 - insert new entities.
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			session.save( author );
+			session.save( book );
+		} );
+
+		// Revision 2 - update detached with changes.
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			book.setName( "Book2Updated" );
+			session.update( book );
+		} );
+	}
+
+	@Test
+	@Priority(8)
+	public void initDataUpdateDetachedUnchangedAndChanged() {
+		final Author author = new Author( 3, "Author3" );
+		final Book book = new Book( 3, "Book3", author );
+
+		// Revision 1 - insert new entities.
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			session.save( author );
+			session.save( book );
+		} );
+
+		// Revision 2 - update detached with no changes.
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			session.update( book );
+		} );
+
+		// Revision 3 - update detached with changes.
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			book.setName( "Book3Updated" );
+			session.update( book );
+		} );
+	}
+
+	@Test
+	@Priority(7)
+	public void initDataUpdateDetachedChangedAndUnchanged() {
+		final Author author = new Author( 4, "Author4" );
+		final Book book = new Book( 4, "Book4", author );
+
+		// Revision 1 - insert new entities.
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			session.save( author );
+			session.save( book );
+		} );
+
+		// Revision 2 - update detached with changes.
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			book.setName( "Book4Updated" );
+			session.update( book );
+		} );
+
+		// Revision 3 - update detached with no changes.
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			session.update( book );
+		} );
+	}
+
+	@Test
+	public void testRevisionCountsUpdateDetachedUnchanged() {
+		assertEquals( 1, getAuditReader().getRevisions( Author.class, 1 ).size() );
+		assertEquals( 1, getAuditReader().getRevisions( Book.class, 1 ).size() );
+	}
+
+	@Test
+	public void testRevisionCountsUpdateDetachedChanged() {
+		assertEquals( 1, getAuditReader().getRevisions( Author.class, 2 ).size() );
+		assertEquals( 2, getAuditReader().getRevisions( Book.class, 2 ).size() );
+	}
+
+	@Test
+	public void testRevisionCountsUpdateDetachedUnchangedAndChanged() {
+		assertEquals( 1, getAuditReader().getRevisions( Author.class, 3 ).size() );
+		assertEquals( 2, getAuditReader().getRevisions( Book.class, 3 ).size() );
+	}
+
+	@Test
+	public void testRevisionCountsUpdateDetachedChangedAndUnchanged() {
+		assertEquals( 1, getAuditReader().getRevisions( Author.class, 4 ).size() );
+		assertEquals( 2, getAuditReader().getRevisions( Book.class, 4 ).size() );
+	}
+
+	@Entity(name = "Book")
+	@SelectBeforeUpdate
+	@Audited
+	public static class Book {
+		@Id
+		private Integer id;
+		private String name;
+		@ManyToOne
+		@JoinColumn(updatable = false)
+		private Author author;
+
+		Book() {
+
+		}
+
+		Book(Integer id, String name, Author author) {
+			this.id = id;
+			this.name = name;
+			this.author = author;
+		}
+
+		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;
+		}
+
+		public Author getAuthor() {
+			return author;
+		}
+
+		public void setAuthor(Author author) {
+			this.author = author;
+		}
+	}
+
+	@Entity
+	@Audited
+	public static class Author {
+		@Id
+		private Integer id;
+		private String name;
+
+		Author() {
+
+		}
+
+		Author(Integer id, String name) {
+			this.id = id;
+			this.name = 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;
+		}
+	}
+}