Hibernate
/
hibernate-orm
mirror of https://github.com/hibernate/hibernate-orm
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

HHH-18230 disable exception when collection is unowned 

I just feel like that's a bit too heavy-handed

Signed-off-by: Gavin King <gavin@hibernate.org>
This commit is contained in:
Gavin King 2024-06-05 12:29:33 +02:00
parent aa91138b6b
commit a6ef6d1b55
3 changed files with 70 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
hibernate-core/src/main/java/org/hibernate/engine/spi/CascadingActions.java
View File

@ -24,6 +24,7 @@ import org.jboss.logging.Logger;
import java.util.Iterator; import java.util.Iterator;
import static java.util.Collections.emptyIterator;
import static org.hibernate.engine.internal.ForeignKeys.isTransient; import static org.hibernate.engine.internal.ForeignKeys.isTransient;
import static org.hibernate.engine.internal.ManagedTypeHelper.isHibernateProxy; import static org.hibernate.engine.internal.ManagedTypeHelper.isHibernateProxy;
@ -403,7 +404,17 @@ public class CascadingActions {
EventSource session, EventSource session,
CollectionType collectionType, CollectionType collectionType,
Object collection) { Object collection) {
return getLoadedElementsIterator( session, collectionType, collection ); if ( collectionType.isInverse( session.getSessionFactory() ) ) {
// For now, don't throw when an unowned collection
// contains references to transient/deleted objects.
// Strictly speaking, we should throw: but it just
// feels a bit too heavy-handed, especially in the
// case where the entity isn't transient but removed.
return emptyIterator();
}
else {
return getLoadedElementsIterator( session, collectionType, collection );
}
} }
@Override @Override

104
hibernate-core/src/main/java/org/hibernate/type/CollectionType.java
View File

@ -21,8 +21,9 @@ import java.util.TreeMap;
import org.hibernate.Hibernate; import org.hibernate.Hibernate;
import org.hibernate.HibernateException; import org.hibernate.HibernateException;
import org.hibernate.Incubating;
import org.hibernate.Internal;
import org.hibernate.MappingException; import org.hibernate.MappingException;
import org.hibernate.bytecode.enhance.spi.LazyPropertyInitializer;
import org.hibernate.collection.spi.AbstractPersistentCollection; import org.hibernate.collection.spi.AbstractPersistentCollection;
import org.hibernate.collection.spi.PersistentCollection; import org.hibernate.collection.spi.PersistentCollection;
import org.hibernate.engine.spi.CollectionEntry; import org.hibernate.engine.spi.CollectionEntry;
@ -42,12 +43,14 @@ import org.hibernate.persister.collection.QueryableCollection;
import org.hibernate.persister.entity.EntityPersister; import org.hibernate.persister.entity.EntityPersister;
import org.hibernate.persister.entity.Joinable; import org.hibernate.persister.entity.Joinable;
import org.hibernate.pretty.MessageHelper; import org.hibernate.pretty.MessageHelper;
import org.hibernate.proxy.HibernateProxy;
import org.hibernate.proxy.LazyInitializer; import org.hibernate.proxy.LazyInitializer;
import org.hibernate.sql.results.graph.collection.LoadingCollectionEntry; import org.hibernate.sql.results.graph.collection.LoadingCollectionEntry;
import org.jboss.logging.Logger; import org.jboss.logging.Logger;
import static org.hibernate.bytecode.enhance.spi.LazyPropertyInitializer.UNFETCHED_PROPERTY;
import static org.hibernate.proxy.HibernateProxy.extractLazyInitializer;
/** /**
* A type that handles Hibernate {@code PersistentCollection}s (including arrays). * A type that handles Hibernate {@code PersistentCollection}s (including arrays).
* *
@ -57,7 +60,7 @@ public abstract class CollectionType extends AbstractType implements Association
private static final CoreMessageLogger LOG = Logger.getMessageLogger(CoreMessageLogger.class, CollectionType.class.getName()); private static final CoreMessageLogger LOG = Logger.getMessageLogger(CoreMessageLogger.class, CollectionType.class.getName());
// private static final Object NOT_NULL_COLLECTION = new MarkerObject( "NOT NULL COLLECTION" ); @Internal
public static final Object UNFETCHED_COLLECTION = new MarkerObject( "UNFETCHED COLLECTION" ); public static final Object UNFETCHED_COLLECTION = new MarkerObject( "UNFETCHED COLLECTION" );
private final String role; private final String role;
@ -67,7 +70,6 @@ public abstract class CollectionType extends AbstractType implements Association
// TODO initialize it at constructor time // TODO initialize it at constructor time
private volatile CollectionPersister persister; private volatile CollectionPersister persister;
public CollectionType(String role, String foreignKeyPropertyName) { public CollectionType(String role, String foreignKeyPropertyName) {
this.role = role; this.role = role;
this.foreignKeyPropertyName = foreignKeyPropertyName; this.foreignKeyPropertyName = foreignKeyPropertyName;
@ -86,11 +88,11 @@ public abstract class CollectionType extends AbstractType implements Association
public boolean contains(Object collection, Object childObject, SharedSessionContractImplementor session) { public boolean contains(Object collection, Object childObject, SharedSessionContractImplementor session) {
// we do not have to worry about queued additions to uninitialized // we do not have to worry about queued additions to uninitialized
// collections, since they can only occur for inverse collections! // collections, since they can only occur for inverse collections!
Iterator<?> elems = getElementsIterator( collection ); final Iterator<?> elems = getElementsIterator( collection );
while ( elems.hasNext() ) { while ( elems.hasNext() ) {
Object element = elems.next(); Object element = elems.next();
// worrying about proxies is perhaps a little bit of overkill here... // worrying about proxies is perhaps a little bit of overkill here...
final LazyInitializer li = HibernateProxy.extractLazyInitializer( element ); final LazyInitializer li = extractLazyInitializer( element );
if ( li != null ) { if ( li != null ) {
if ( !li.isUninitialized() ) { if ( !li.isUninitialized() ) {
element = li.getImplementation(); element = li.getImplementation();
@ -168,8 +170,7 @@ public abstract class CollectionType extends AbstractType implements Association
} }
@Override @Override
public String toLoggableString(Object value, SessionFactoryImplementor factory) public String toLoggableString(Object value, SessionFactoryImplementor factory) {
throws HibernateException {
if ( value == null ) { if ( value == null ) {
return "null"; return "null";
} }
@ -177,32 +178,34 @@ public abstract class CollectionType extends AbstractType implements Association
if ( !getReturnedClass().isInstance( value ) && !(value instanceof PersistentCollection) ) { if ( !getReturnedClass().isInstance( value ) && !(value instanceof PersistentCollection) ) {
// its most likely the collection-key // its most likely the collection-key
final CollectionPersister persister = getPersister( factory ); final CollectionPersister persister = getPersister( factory );
if ( persister.getKeyType().getReturnedClass().isInstance( value ) ) { final Type keyType = persister.getKeyType();
return getRole() + "#" + getPersister( factory ).getKeyType().toLoggableString( value, factory ); if ( keyType.getReturnedClass().isInstance( value ) ) {
return getRole() + "#" + keyType.toLoggableString( value, factory );
} }
else { else {
// although it could also be the collection-id // although it could also be the collection-id
if ( persister.getIdentifierType() != null final Type identifierType = persister.getIdentifierType();
&& persister.getIdentifierType().getReturnedClass().isInstance( value ) ) { if ( identifierType != null
return getRole() + "#" + getPersister( factory ).getIdentifierType().toLoggableString( value, factory ); && identifierType.getReturnedClass().isInstance( value ) ) {
return getRole() + "#" + identifierType.toLoggableString( value, factory );
} }
} }
} }
return renderLoggableString( value, factory ); return renderLoggableString( value, factory );
} }
protected String renderLoggableString(Object value, SessionFactoryImplementor factory) throws HibernateException { protected String renderLoggableString(Object value, SessionFactoryImplementor factory) {
if ( !Hibernate.isInitialized( value ) ) { if ( !Hibernate.isInitialized( value ) ) {
return "<uninitialized>"; return "<uninitialized>";
} }
final List<String> list = new ArrayList<>(); final List<String> list = new ArrayList<>();
Type elemType = getElementType( factory ); final Type elemType = getElementType( factory );
Iterator<?> itr = getElementsIterator( value ); final Iterator<?> itr = getElementsIterator( value );
while ( itr.hasNext() ) { while ( itr.hasNext() ) {
Object element = itr.next(); final Object element = itr.next();
String string = final String string =
element == LazyPropertyInitializer.UNFETCHED_PROPERTY || !Hibernate.isInitialized(element) element == UNFETCHED_PROPERTY || !Hibernate.isInitialized(element)
? "<uninitialized>" ? "<uninitialized>"
: elemType.toLoggableString( element, factory ); : elemType.toLoggableString( element, factory );
list.add( string ); list.add( string );
@ -249,6 +252,11 @@ public abstract class CollectionType extends AbstractType implements Association
return false; return false;
} }
@Internal @Incubating
public boolean isInverse(SessionFactoryImplementor factory) {
return getPersister( factory ).isInverse();
}
@Override @Override
public Serializable disassemble(Object value, SharedSessionContractImplementor session, Object owner) public Serializable disassemble(Object value, SharedSessionContractImplementor session, Object owner)
throws HibernateException { throws HibernateException {
@ -259,7 +267,10 @@ public abstract class CollectionType extends AbstractType implements Association
//session.getPersistenceContext().getCollectionEntry( (PersistentCollection) value ).getKey(); //session.getPersistenceContext().getCollectionEntry( (PersistentCollection) value ).getKey();
final Object key = getKeyOfOwner( owner, session ); final Object key = getKeyOfOwner( owner, session );
return key == null ? null : getPersister( session ).getKeyType().disassemble( key, session, owner ); return key == null ? null
: getPersister( session )
.getKeyType()
.disassemble( key, session, owner );
} }
@Override @Override
@ -276,9 +287,10 @@ public abstract class CollectionType extends AbstractType implements Association
return null; return null;
} }
else { else {
final Object key = getPersister(session) final Object key =
.getKeyType() getPersister( session )
.assemble( cached, session, owner); .getKeyType()
.assemble( cached, session, owner);
return resolveKey( key, session, owner ); return resolveKey( key, session, owner );
} }
} }
@ -318,11 +330,9 @@ public abstract class CollectionType extends AbstractType implements Association
@Override @Override
public boolean isDirty(Object old, Object current, SharedSessionContractImplementor session) public boolean isDirty(Object old, Object current, SharedSessionContractImplementor session)
throws HibernateException { throws HibernateException {
// collections don't dirty an un-versioned parent entity // collections don't dirty an un-versioned parent entity
// TODO: I don't really like this implementation; it would be
// TODO: I don't really like this implementation; it would be better if // better if this was handled by searchForDirtyCollections()
// this was handled by searchForDirtyCollections()
return super.isDirty( old, current, session ); return super.isDirty( old, current, session );
// return false; // return false;
@ -331,7 +341,7 @@ public abstract class CollectionType extends AbstractType implements Association
@Override @Override
public boolean isDirty(Object old, Object current, boolean[] checkable, SharedSessionContractImplementor session) public boolean isDirty(Object old, Object current, boolean[] checkable, SharedSessionContractImplementor session)
throws HibernateException { throws HibernateException {
return isDirty(old, current, session); return isDirty( old, current, session );
} }
/** /**
@ -367,16 +377,13 @@ public abstract class CollectionType extends AbstractType implements Association
* @return The collection owner's key * @return The collection owner's key
*/ */
public Object getKeyOfOwner(Object owner, SharedSessionContractImplementor session) { public Object getKeyOfOwner(Object owner, SharedSessionContractImplementor session) {
final PersistenceContext pc = session.getPersistenceContextInternal(); final EntityEntry entityEntry = session.getPersistenceContextInternal().getEntry( owner );
final EntityEntry entityEntry = pc.getEntry( owner );
if ( entityEntry == null ) { if ( entityEntry == null ) {
// This just handles a particular case of component // This just handles a particular case of component
// projection, perhaps get rid of it and throw an exception // projection, perhaps get rid of it and throw an exception
return null; return null;
} }
else if ( foreignKeyPropertyName == null ) {
if ( foreignKeyPropertyName == null ) {
return entityEntry.getId(); return entityEntry.getId();
} }
else { else {
@ -386,14 +393,13 @@ public abstract class CollectionType extends AbstractType implements Association
// to decide to semiResolve(), trouble is that initializeEntity() reuses // to decide to semiResolve(), trouble is that initializeEntity() reuses
// the same array for resolved and hydrated values // the same array for resolved and hydrated values
final Object loadedValue = entityEntry.getLoadedValue( foreignKeyPropertyName ); final Object loadedValue = entityEntry.getLoadedValue( foreignKeyPropertyName );
final Object id = loadedValue == null ? final Object id = loadedValue == null
entityEntry.getPersister().getPropertyValue( owner, foreignKeyPropertyName ) : ? entityEntry.getPersister().getPropertyValue( owner, foreignKeyPropertyName )
loadedValue; : loadedValue;
// NOTE VERY HACKISH WORKAROUND!! // NOTE VERY HACKISH WORKAROUND!!
// TODO: Fix this so it will work for non-POJO entity mode // TODO: Fix this so it will work for non-POJO entity mode
Type keyType = getPersister( session ).getKeyType(); if ( !keyClass( session ).isInstance( id ) ) {
if ( !keyType.getReturnedClass().isInstance( id ) ) {
throw new UnsupportedOperationException( "Re-work support for semi-resolve" ); throw new UnsupportedOperationException( "Re-work support for semi-resolve" );
// id = keyType.semiResolve( // id = keyType.semiResolve(
// entityEntry.getLoadedValue( foreignKeyPropertyName ), // entityEntry.getLoadedValue( foreignKeyPropertyName ),
@ -416,26 +422,28 @@ public abstract class CollectionType extends AbstractType implements Association
* otherwise, null is returned * otherwise, null is returned
*/ */
public Object getIdOfOwnerOrNull(Object key, SharedSessionContractImplementor session) { public Object getIdOfOwnerOrNull(Object key, SharedSessionContractImplementor session) {
Object ownerId = null;
if ( foreignKeyPropertyName == null ) { if ( foreignKeyPropertyName == null ) {
ownerId = key; return key;
} }
else { else {
final CollectionPersister persister = getPersister( session ); final CollectionPersister persister = getPersister( session );
Type keyType = persister.getKeyType(); final EntityPersister ownerPersister = persister.getOwnerEntityPersister();
EntityPersister ownerPersister = persister.getOwnerEntityPersister();
// TODO: Fix this so it will work for non-POJO entity mode // TODO: Fix this so it will work for non-POJO entity mode
Class<?> ownerMappedClass = ownerPersister.getMappedClass(); final Class<?> keyClass = keyClass( session );
if ( ownerMappedClass.isAssignableFrom( keyType.getReturnedClass() ) if ( ownerPersister.getMappedClass().isAssignableFrom( keyClass )
&& keyType.getReturnedClass().isInstance( key ) ) { && keyClass.isInstance( key ) ) {
// the key is the owning entity itself, so get the ID from the key // the key is the owning entity itself, so get the ID from the key
ownerId = ownerPersister.getIdentifier( key, session ); return ownerPersister.getIdentifier( key, session );
} }
else { else {
// TODO: check if key contains the owner ID // TODO: check if key contains the owner ID
return null;
} }
} }
return ownerId; }
private Class<?> keyClass(SharedSessionContractImplementor session) {
return getPersister( session ).getKeyType().getReturnedClass();
} }
private Object resolveKey(Object key, SharedSessionContractImplementor session, Object owner) { private Object resolveKey(Object key, SharedSessionContractImplementor session, Object owner) {
@ -675,7 +683,7 @@ public abstract class CollectionType extends AbstractType implements Association
// need to put the merged elements in a new collection // need to put the merged elements in a new collection
Object result = ( target == null || Object result = ( target == null ||
target == original || target == original ||
target == LazyPropertyInitializer.UNFETCHED_PROPERTY || target == UNFETCHED_PROPERTY ||
target instanceof PersistentCollection target instanceof PersistentCollection
&& ( (PersistentCollection<?>) target ).isWrapper( original ) ) && ( (PersistentCollection<?>) target ).isWrapper( original ) )
? instantiateResult( original ) : target; ? instantiateResult( original ) : target;

4
hibernate-core/src/test/java/org/hibernate/orm/test/jpa/cascade/multicircle/MultiCircleJpaCascadeTest.java
View File

@ -206,8 +206,8 @@ public class MultiCircleJpaCascadeTest {
assertTrue( ex.getCause() instanceof IllegalStateException ); assertTrue( ex.getCause() instanceof IllegalStateException );
IllegalStateException ise = (IllegalStateException) ex.getCause(); IllegalStateException ise = (IllegalStateException) ex.getCause();
assertTyping( TransientObjectException.class, ise.getCause() ); assertTyping( TransientObjectException.class, ise.getCause() );
String message = ise.getCause().getMessage(); // String message = ise.getCause().getMessage();
assertTrue( message.contains("'org.hibernate.orm.test.jpa.cascade.multicircle.B'") ); // assertTrue( message.contains("'org.hibernate.orm.test.jpa.cascade.multicircle.B'") );
} }
finally { finally {
entityManager.getTransaction().rollback(); entityManager.getTransaction().rollback();