From 8530584fadaf4f65f22e3ac77e53a674fca179e8 Mon Sep 17 00:00:00 2001 From: Anders Wallgren Date: Wed, 1 Nov 2017 16:34:55 -0400 Subject: [PATCH] HHH-12074 - order_inserts: flush during transaction causes incorrect insert ordering and subsequent constraint violation --- .../org/hibernate/engine/spi/ActionQueue.java | 68 ++++++-- ...ithBidirectionalOneToManyFlushProblem.java | 161 ++++++++++++++++++ 2 files changed, 219 insertions(+), 10 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/test/insertordering/InsertOrderingWithBidirectionalOneToManyFlushProblem.java diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java index 41053f4c7a..ee43a5f27b 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java @@ -1024,11 +1024,21 @@ public class ActionQueue { private Set childEntityNames = new HashSet<>( ); + private BatchIdentifier parent; + BatchIdentifier(String entityName, String rootEntityName) { this.entityName = entityName; this.rootEntityName = rootEntityName; } + public BatchIdentifier getParent() { + return parent; + } + + public void setParent(BatchIdentifier parent) { + this.parent = parent; + } + @Override public boolean equals(Object o) { if ( this == o ) { @@ -1070,6 +1080,18 @@ public class ActionQueue { boolean hasAnyChildEntityNames(BatchIdentifier batchIdentifier) { return childEntityNames.contains( batchIdentifier.getEntityName() ); } + + /** + * Check if the this {@link BatchIdentifier} has a parent or grand parent + * matching the given {@link BatchIdentifier reference. + * + * @param batchIdentifier {@link BatchIdentifier} reference + * + * @return This {@link BatchIdentifier} has a parent matching the given {@link BatchIdentifier reference + */ + boolean hasParent(BatchIdentifier batchIdentifier) { + return parent == batchIdentifier || parent != null && parent.hasParent( batchIdentifier ); + } } // the mapping of entity names to their latest batch numbers. @@ -1095,7 +1117,11 @@ public class ActionQueue { for ( AbstractEntityInsertAction action : insertions ) { BatchIdentifier batchIdentifier = new BatchIdentifier( action.getEntityName(), - action.getSession().getFactory().getMetamodel().entityPersister( action.getEntityName() ).getRootEntityName() + action.getSession() + .getFactory() + .getMetamodel() + .entityPersister( action.getEntityName() ) + .getRootEntityName() ); // the entity associated with the current action. @@ -1114,7 +1140,34 @@ public class ActionQueue { } insertions.clear(); - // Examine each entry in the batch list, sorting them based on parent/child associations. + // Examine each entry in the batch list, and build the dependency graph. + for ( int i = 0; i < latestBatches.size(); i++ ) { + BatchIdentifier batchIdentifier = latestBatches.get( i ); + + for ( int j = i - 1; j >= 0; j-- ) { + BatchIdentifier prevBatchIdentifier = latestBatches.get( j ); + if ( prevBatchIdentifier.hasAnyParentEntityNames( batchIdentifier ) ) { + prevBatchIdentifier.parent = batchIdentifier; + } + if ( batchIdentifier.hasAnyChildEntityNames( prevBatchIdentifier ) ) { + prevBatchIdentifier.parent = batchIdentifier; + } + } + + for ( int j = i + 1; j < latestBatches.size(); j++ ) { + BatchIdentifier nextBatchIdentifier = latestBatches.get( j ); + + if ( nextBatchIdentifier.hasAnyParentEntityNames( batchIdentifier ) ) { + nextBatchIdentifier.parent = batchIdentifier; + } + if ( batchIdentifier.hasAnyChildEntityNames( nextBatchIdentifier ) ) { + nextBatchIdentifier.parent = batchIdentifier; + } + } + } + + // Examine each entry in the batch list, sorting them based on parent/child association + // as depicted by the dependency graph. for ( int i = 0; i < latestBatches.size(); i++ ) { BatchIdentifier batchIdentifier = latestBatches.get( i ); @@ -1124,7 +1177,7 @@ public class ActionQueue { // batch. If so, we reordered them. for ( int j = i - 1; j >= 0; j-- ) { BatchIdentifier prevBatchIdentifier = latestBatches.get( j ); - if ( prevBatchIdentifier.hasAnyParentEntityNames( batchIdentifier ) ) { + if ( prevBatchIdentifier.hasParent( batchIdentifier ) ) { latestBatches.remove( batchIdentifier ); latestBatches.add( j, batchIdentifier ); } @@ -1137,19 +1190,14 @@ public class ActionQueue { for ( int j = i + 1; j < latestBatches.size(); j++ ) { BatchIdentifier nextBatchIdentifier = latestBatches.get( j ); - final boolean nextBatchHasChild = nextBatchIdentifier.hasAnyChildEntityNames( batchIdentifier ); - final boolean batchHasChild = batchIdentifier.hasAnyChildEntityNames( nextBatchIdentifier ); - final boolean batchHasParent = batchIdentifier.hasAnyParentEntityNames( nextBatchIdentifier ); - - // Take care of unidirectional @OneToOne associations but exclude bidirectional @ManyToMany - if ( ( nextBatchHasChild && !batchHasChild ) || batchHasParent ) { + if ( batchIdentifier.hasParent( nextBatchIdentifier ) ) { latestBatches.remove( batchIdentifier ); latestBatches.add( j, batchIdentifier ); } } } - // now rebuild the insertions list. There is a batch for each entry in the name list. + // Now, rebuild the insertions list. There is a batch for each entry in the name list. for ( BatchIdentifier rootIdentifier : latestBatches ) { List batch = actionBatches.get( rootIdentifier ); insertions.addAll( batch ); diff --git a/hibernate-core/src/test/java/org/hibernate/test/insertordering/InsertOrderingWithBidirectionalOneToManyFlushProblem.java b/hibernate-core/src/test/java/org/hibernate/test/insertordering/InsertOrderingWithBidirectionalOneToManyFlushProblem.java new file mode 100644 index 0000000000..b9640d617e --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/insertordering/InsertOrderingWithBidirectionalOneToManyFlushProblem.java @@ -0,0 +1,161 @@ +/* + * 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.insertordering; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; +import javax.persistence.ManyToOne; +import javax.persistence.OneToMany; +import javax.persistence.SequenceGenerator; + +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase; +import org.junit.Test; + +import static javax.persistence.CascadeType.PERSIST; +import static javax.persistence.GenerationType.SEQUENCE; +import static org.hibernate.cfg.AvailableSettings.ORDER_INSERTS; +import static org.hibernate.cfg.AvailableSettings.STATEMENT_BATCH_SIZE; +import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate; + +@TestForIssue(jiraKey = "HHH-12074") +public class InsertOrderingWithBidirectionalOneToManyFlushProblem + extends BaseNonConfigCoreFunctionalTestCase { + + @Test + public void testBatchingWithFlush() { + doInHibernate( + this::sessionFactory, + session -> { + Top top1 = new Top(); + + session.persist( top1 ); + + // InsertActionSorter#sort is invoked during this flush. + // + // input: [top1] + // output: [top1] + session.flush(); + + Middle middle1 = new Middle(); + + middle1.addBottom( new Bottom() ); + top1.addMiddle( middle1 ); + session.persist( middle1 ); + + Top top2 = new Top(); + + session.persist( top2 ); + + Middle middle2 = new Middle(); + + middle2.addBottom( new Bottom() ); + top2.addMiddle( middle2 ); + session.persist( middle2 ); + + // InsertActionSorter#sort is invoked during this flush + // + // input: [middle1,bottom1,top2,middle2,bottom2] output: + // [middle1,middle2,bottom1,bottom2,top2] + // + // This ordering causes a constraint violation during the flush + // when the attempt to insert middle2 before top2 is made. + // + // correct ordering is: [top2,middle1,middle2,bottom1,bottom2] + } + ); + } + + @Override + protected void addSettings(Map settings) { + settings.put( ORDER_INSERTS, "true" ); + settings.put( STATEMENT_BATCH_SIZE, "10" ); + } + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { Top.class, Middle.class, Bottom.class }; + } + + @Entity(name = "Bottom") + public static class Bottom { + + @Column(nullable = false) + @GeneratedValue( + strategy = SEQUENCE, + generator = "ID" + ) + @Id + @SequenceGenerator( + name = "ID", + sequenceName = "BOTTOM_SEQ" + ) + private Long id; + + @ManyToOne(optional = false) + private Middle middle; + } + + @Entity(name = "Middle") + public static class Middle { + + @Column(nullable = false) + @GeneratedValue( + strategy = SEQUENCE, + generator = "ID" + ) + @Id + @SequenceGenerator( + name = "ID", + sequenceName = "MIDDLE_SEQ" + ) + private Long id; + + @ManyToOne(optional = false) + private Top top; + + @OneToMany( + cascade = PERSIST, + mappedBy = "middle" + ) + private List bottoms = new ArrayList<>(); + + private void addBottom(Bottom bottom) { + bottoms.add( bottom ); + bottom.middle = this; + } + } + + @Entity(name = "Top") + public static class Top { + + @Column(nullable = false) + @GeneratedValue( + strategy = SEQUENCE, + generator = "ID" + ) + @Id + @SequenceGenerator( + name = "ID", + sequenceName = "TOP_SEQ" + ) + private Long id; + + @OneToMany(mappedBy = "top") + private List middles = new ArrayList<>(); + + void addMiddle(Middle middle) { + middles.add( middle ); + middle.top = this; + } + } +}