From 171e2144588a9fc400b7b1a63d6ac045c59c3473 Mon Sep 17 00:00:00 2001 From: Brett Meyer Date: Tue, 4 Mar 2014 14:40:39 -0500 Subject: [PATCH] HHH-7072 force recreation of element collections of components with all nullable properties --- .../AbstractPersistentCollection.java | 15 ++++++- .../org/hibernate/type/ComponentType.java | 8 ++++ .../withcustomenumdef/Location.java | 21 +++++++++ .../withcustomenumdef/TestBasicOps.java | 45 +++++++++++++++++-- 4 files changed, 85 insertions(+), 4 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/collection/internal/AbstractPersistentCollection.java b/hibernate-core/src/main/java/org/hibernate/collection/internal/AbstractPersistentCollection.java index da873148ab..8b347d8a91 100644 --- a/hibernate-core/src/main/java/org/hibernate/collection/internal/AbstractPersistentCollection.java +++ b/hibernate-core/src/main/java/org/hibernate/collection/internal/AbstractPersistentCollection.java @@ -31,6 +31,7 @@ import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.ListIterator; + import javax.naming.NamingException; import org.hibernate.AssertionFailure; @@ -52,8 +53,8 @@ import org.hibernate.internal.util.collections.IdentitySet; import org.hibernate.persister.collection.CollectionPersister; import org.hibernate.persister.entity.EntityPersister; import org.hibernate.pretty.MessageHelper; +import org.hibernate.type.ComponentType; import org.hibernate.type.Type; - import org.jboss.logging.Logger; /** @@ -650,6 +651,18 @@ public abstract class AbstractPersistentCollection implements Serializable, Pers @Override public boolean needsRecreate(CollectionPersister persister) { + // Workaround for situations like HHH-7072. If the collection element is a component that consists entirely + // of nullable properties, we currently have to forcefully recreate the entire collection. See the use + // of hasNotNullableColumns in the AbstractCollectionPersister constructor for more info. In order to delete + // row-by-row, that would require SQL like "WHERE ( COL = ? OR ( COL is null AND ? is null ) )", rather than + // the current "WHERE COL = ?" (fails for null for most DBs). Note that + // the param would have to be bound twice. Until we eventually add "parameter bind points" concepts to the + // AST in ORM 5+, handling this type of condition is either extremely difficult or impossible. Forcing + // recreation isn't ideal, but not really any other option in ORM 4. + if (persister.getElementType() instanceof ComponentType) { + ComponentType componentType = (ComponentType) persister.getElementType(); + return !componentType.hasNotNullProperty(); + } return false; } diff --git a/hibernate-core/src/main/java/org/hibernate/type/ComponentType.java b/hibernate-core/src/main/java/org/hibernate/type/ComponentType.java index 6ed37bf5a4..b584dd98ce 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/ComponentType.java +++ b/hibernate-core/src/main/java/org/hibernate/type/ComponentType.java @@ -66,6 +66,7 @@ public class ComponentType extends AbstractType implements CompositeType, Proced private final CascadeStyle[] cascade; private final FetchMode[] joinedFetch; private final boolean isKey; + private boolean hasNotNullProperty; protected final EntityMode entityMode; protected final ComponentTuplizer componentTuplizer; @@ -88,6 +89,9 @@ public class ComponentType extends AbstractType implements CompositeType, Proced this.propertyNullability[i] = prop.isNullable(); this.cascade[i] = prop.getCascadeStyle(); this.joinedFetch[i] = prop.getFetchMode(); + if (!prop.isNullable()) { + hasNotNullProperty = true; + } } this.entityMode = metamodel.getEntityMode(); @@ -805,4 +809,8 @@ public class ComponentType extends AbstractType implements CompositeType, Proced return resolve( values, session, null ); } + + public boolean hasNotNullProperty() { + return hasNotNullProperty; + } } diff --git a/hibernate-core/src/test/java/org/hibernate/test/annotations/collectionelement/embeddables/withcustomenumdef/Location.java b/hibernate-core/src/test/java/org/hibernate/test/annotations/collectionelement/embeddables/withcustomenumdef/Location.java index 7125491a42..fe3694a4a2 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/annotations/collectionelement/embeddables/withcustomenumdef/Location.java +++ b/hibernate-core/src/test/java/org/hibernate/test/annotations/collectionelement/embeddables/withcustomenumdef/Location.java @@ -71,4 +71,25 @@ public class Location { public void setName(String name) { this.name = name; } + + @Override + public boolean equals(Object obj) { + if (! obj.getClass().equals( Location.class )) { + return false; + } + + Location loc = (Location) obj; + if (name != null ? !name.equals(loc.name) : loc.name != null) return false; + if (type != null ? !type.equals(loc.type) : loc.type != null) return false; + + return true; + } + + @Override + public int hashCode() { + int result; + result = (name != null ? name.hashCode() : 0); + result = 31 * result + (type != null ? type.hashCode() : 0); + return result; + } } diff --git a/hibernate-core/src/test/java/org/hibernate/test/annotations/collectionelement/embeddables/withcustomenumdef/TestBasicOps.java b/hibernate-core/src/test/java/org/hibernate/test/annotations/collectionelement/embeddables/withcustomenumdef/TestBasicOps.java index 611f53e699..33cfe66050 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/annotations/collectionelement/embeddables/withcustomenumdef/TestBasicOps.java +++ b/hibernate-core/src/test/java/org/hibernate/test/annotations/collectionelement/embeddables/withcustomenumdef/TestBasicOps.java @@ -23,12 +23,14 @@ */ package org.hibernate.test.annotations.collectionelement.embeddables.withcustomenumdef; -import org.junit.Test; +import static junit.framework.Assert.assertEquals; + +import java.util.Iterator; import org.hibernate.Session; +import org.hibernate.testing.TestForIssue; import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; - -import static junit.framework.Assert.assertEquals; +import org.junit.Test; /** * @author Steve Ebersole @@ -57,4 +59,41 @@ public class TestBasicOps extends BaseCoreFunctionalTestCase { s.getTransaction().commit(); s.close(); } + + @Test + @TestForIssue(jiraKey = "HHH-7072") + public void testEmbeddableWithNullables() { + Session s = openSession(); + s.beginTransaction(); + Query q = new Query( new Location( null, Location.Type.COMMUNE ) ); + s.save( q ); + s.getTransaction().commit(); + s.clear(); + + s.beginTransaction(); + q.getIncludedLocations().add( new Location( null, Location.Type.COUNTY ) ); + s.update( q ); + s.getTransaction().commit(); + s.clear(); + + s.beginTransaction(); + q = (Query) s.get( Query.class, q.getId() ); +// assertEquals( 2, q.getIncludedLocations().size() ); + s.getTransaction().commit(); + s.clear(); + + s.beginTransaction(); + Iterator itr = q.getIncludedLocations().iterator(); + itr.next(); + itr.remove(); + s.update( q ); + s.getTransaction().commit(); + s.clear(); + + s.beginTransaction(); + q = (Query) s.get( Query.class, q.getId() ); + assertEquals( 1, q.getIncludedLocations().size() ); + s.getTransaction().commit(); + s.close(); + } }