From 20ae4920b7cd30a3a781b9eabf1da5ceeae20c48 Mon Sep 17 00:00:00 2001 From: Chris Cranford Date: Fri, 19 Aug 2016 21:10:54 -0500 Subject: [PATCH] HHH-5908 - Avoid unnecessary updates on detached un-modified entities with SelectBeforeUpdate. HHH-11056 - Envers audits unchanged objects for a certain use case --- .../entity/AbstractEntityPersister.java | 1 + .../java/org/hibernate/type/TypeHelper.java | 17 +- .../UpdateDetachedTest.java | 235 ++++++++++++++++++ .../update/SelectBeforeUpdateTest.java | 222 +++++++++++++++++ 4 files changed, 468 insertions(+), 7 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/test/annotations/selectbeforeupdate/UpdateDetachedTest.java create mode 100644 hibernate-envers/src/test/java/org/hibernate/envers/test/integration/update/SelectBeforeUpdateTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java index a6375731adaa..b4c20b0de990 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java @@ -4172,6 +4172,7 @@ public int[] findModified(Object[] old, Object[] current, Object entity, SharedS current, old, propertyColumnUpdateable, + getPropertyUpdateability(), hasUninitializedLazyProperties( entity ), session ); diff --git a/hibernate-core/src/main/java/org/hibernate/type/TypeHelper.java b/hibernate-core/src/main/java/org/hibernate/type/TypeHelper.java index 78d0c2ace25e..25bb503edc78 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/TypeHelper.java +++ b/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 static int[] findDirty( * @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 static int[] findModified( 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 static int[] findModified( 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 static int[] findModified( return null; } else { - int[] trimmed = new int[count]; + int[] trimmed = new int[ count ]; System.arraycopy( results, 0, trimmed, 0, count ); return trimmed; } diff --git a/hibernate-core/src/test/java/org/hibernate/test/annotations/selectbeforeupdate/UpdateDetachedTest.java b/hibernate-core/src/test/java/org/hibernate/test/annotations/selectbeforeupdate/UpdateDetachedTest.java new file mode 100644 index 000000000000..94087954d863 --- /dev/null +++ b/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 . + */ +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; + } + } +} diff --git a/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/update/SelectBeforeUpdateTest.java b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/update/SelectBeforeUpdateTest.java new file mode 100644 index 000000000000..071de015ca50 --- /dev/null +++ b/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 . + */ +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; + } + } +}