Skip to content

Commit

Permalink
HHH-18936 suppress TransientObjectException when the FK is marked @on…
Browse files Browse the repository at this point in the history
…Delete(CASCADE)

note that this fix does not cover the case of an association inside an @embeddable
  • Loading branch information
gavinking committed Jan 30, 2025
1 parent 1022e9c commit f10bec8
Show file tree
Hide file tree
Showing 10 changed files with 128 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import java.util.List;

import org.hibernate.HibernateException;
import org.hibernate.annotations.OnDeleteAction;
import org.hibernate.bytecode.enhance.spi.interceptor.LazyAttributeLoadingInterceptor;
import org.hibernate.collection.spi.PersistentCollection;
import org.hibernate.engine.spi.CascadeStyle;
Expand Down Expand Up @@ -117,6 +118,8 @@ public static <T> void cascade(
hasUninitializedLazyProperties
&& !persister.getBytecodeEnhancementMetadata()
.isAttributeLoaded( parent, propertyName );
final boolean isCascadeDeleteEnabled =
persister.getEntityMetamodel().getPropertyOnDeleteActions()[i] == OnDeleteAction.CASCADE;

if ( style.doCascade( action ) ) {
final Object child;
Expand Down Expand Up @@ -178,7 +181,7 @@ else if ( action.performOnLazyProperty() && type instanceof EntityType ) {
style,
propertyName,
anything,
false
isCascadeDeleteEnabled
);
}
else {
Expand All @@ -193,7 +196,7 @@ else if ( action.performOnLazyProperty() && type instanceof EntityType ) {
type,
style,
propertyName,
false
isCascadeDeleteEnabled
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ public void cascade(
Void context,
boolean isCascadeDeleteEnabled)
throws HibernateException {
if ( child != null && isChildTransient( session, child, entityName ) ) {
if ( child != null && !isCascadeDeleteEnabled && isChildTransient( session, child, entityName ) ) {
throw new TransientObjectException( "Persistent instance references an unsaved transient instance of '"
+ entityName + "' (save the transient instance before flushing)" );
//TODO: should be TransientPropertyValueException
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.hibernate.HibernateException;
import org.hibernate.Internal;
import org.hibernate.MappingException;
import org.hibernate.annotations.OnDeleteAction;
import org.hibernate.boot.model.relational.Database;
import org.hibernate.boot.model.relational.SqlStringGenerationContext;
import org.hibernate.bytecode.enhance.spi.interceptor.EnhancementHelper;
Expand Down Expand Up @@ -134,6 +135,10 @@ public void resetOptional(boolean optional) {
}
}

public OnDeleteAction getOnDeleteAction() {
return value instanceof ToOne toOne ? toOne.getOnDeleteAction() : null;
}

public CascadeStyle getCascadeStyle() throws MappingException {
final Type type = value.getType();
if ( type instanceof AnyType ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package org.hibernate.tuple;

import org.hibernate.FetchMode;
import org.hibernate.annotations.OnDeleteAction;
import org.hibernate.engine.spi.CascadeStyle;
import org.hibernate.engine.spi.SessionFactoryImplementor;
import org.hibernate.persister.walking.spi.AttributeSource;
Expand Down Expand Up @@ -92,6 +93,11 @@ public CascadeStyle getCascadeStyle() {
return attributeInformation.getCascadeStyle();
}

@Override
public OnDeleteAction getOnDeleteAction() {
return attributeInformation.getOnDeleteAction();
}

@Override
public FetchMode getFetchMode() {
return attributeInformation.getFetchMode();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package org.hibernate.tuple;

import org.hibernate.FetchMode;
import org.hibernate.annotations.OnDeleteAction;
import org.hibernate.engine.spi.CascadeStyle;

/**
Expand All @@ -19,6 +20,7 @@ public class BaselineAttributeInformation {
private final boolean nullable;
private final boolean dirtyCheckable;
private final boolean versionable;
private final OnDeleteAction onDeleteAction;
private final CascadeStyle cascadeStyle;
private final FetchMode fetchMode;

Expand All @@ -30,6 +32,7 @@ public BaselineAttributeInformation(
boolean dirtyCheckable,
boolean versionable,
CascadeStyle cascadeStyle,
OnDeleteAction onDeleteAction,
FetchMode fetchMode) {
this.lazy = lazy;
this.insertable = insertable;
Expand All @@ -38,6 +41,7 @@ public BaselineAttributeInformation(
this.dirtyCheckable = dirtyCheckable;
this.versionable = versionable;
this.cascadeStyle = cascadeStyle;
this.onDeleteAction = onDeleteAction;
this.fetchMode = fetchMode;
}

Expand Down Expand Up @@ -73,6 +77,10 @@ public FetchMode getFetchMode() {
return fetchMode;
}

public OnDeleteAction getOnDeleteAction() {
return onDeleteAction;
}

public static class Builder {
private boolean lazy;
private boolean insertable;
Expand All @@ -81,6 +89,7 @@ public static class Builder {
private boolean dirtyCheckable;
private boolean versionable;
private CascadeStyle cascadeStyle;
private OnDeleteAction onDeleteAction;
private FetchMode fetchMode;

public Builder setLazy(boolean lazy) {
Expand Down Expand Up @@ -118,6 +127,11 @@ public Builder setCascadeStyle(CascadeStyle cascadeStyle) {
return this;
}

public Builder setOnDeleteAction(OnDeleteAction onDeleteAction) {
this.onDeleteAction = onDeleteAction;
return this;
}

public Builder setFetchMode(FetchMode fetchMode) {
this.fetchMode = fetchMode;
return this;
Expand All @@ -132,6 +146,7 @@ public BaselineAttributeInformation createInformation() {
dirtyCheckable,
versionable,
cascadeStyle,
onDeleteAction,
fetchMode
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package org.hibernate.tuple;

import org.hibernate.FetchMode;
import org.hibernate.annotations.OnDeleteAction;
import org.hibernate.engine.spi.CascadeStyle;

/**
Expand Down Expand Up @@ -32,5 +33,7 @@ public interface NonIdentifierAttribute extends Attribute {

CascadeStyle getCascadeStyle();

OnDeleteAction getOnDeleteAction();

FetchMode getFetchMode();
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ public static VersionProperty buildVersionProperty(
.setDirtyCheckable( property.isUpdateable() && !lazy )
.setVersionable( property.isOptimisticLocked() )
.setCascadeStyle( property.getCascadeStyle() )
.setOnDeleteAction( property.getOnDeleteAction() )
.createInformation()
);
}
Expand Down Expand Up @@ -169,6 +170,7 @@ public static NonIdentifierAttribute buildEntityBasedAttribute(
.setDirtyCheckable( alwaysDirtyCheck || property.isUpdateable() )
.setVersionable( property.isOptimisticLocked() )
.setCascadeStyle( property.getCascadeStyle() )
.setOnDeleteAction( property.getOnDeleteAction() )
.setFetchMode( property.getValue().getFetchMode() )
.createInformation()
);
Expand All @@ -188,6 +190,7 @@ public static NonIdentifierAttribute buildEntityBasedAttribute(
.setDirtyCheckable( alwaysDirtyCheck || property.isUpdateable() )
.setVersionable( property.isOptimisticLocked() )
.setCascadeStyle( property.getCascadeStyle() )
.setOnDeleteAction( property.getOnDeleteAction() )
.setFetchMode( property.getValue().getFetchMode() )
.createInformation()
);
Expand All @@ -209,6 +212,7 @@ public static NonIdentifierAttribute buildEntityBasedAttribute(
.setDirtyCheckable( alwaysDirtyCheck || property.isUpdateable() )
.setVersionable( property.isOptimisticLocked() )
.setCascadeStyle( property.getCascadeStyle() )
.setOnDeleteAction( property.getOnDeleteAction() )
.setFetchMode( property.getValue().getFetchMode() )
.createInformation()
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package org.hibernate.tuple;

import org.hibernate.FetchMode;
import org.hibernate.annotations.OnDeleteAction;
import org.hibernate.engine.spi.CascadeStyle;
import org.hibernate.type.Type;

Expand Down Expand Up @@ -37,6 +38,7 @@ public StandardProperty(
boolean checkable,
boolean versionable,
CascadeStyle cascadeStyle,
OnDeleteAction onDeleteAction,
FetchMode fetchMode) {
super(
null,
Expand All @@ -52,6 +54,7 @@ public StandardProperty(
.setDirtyCheckable( checkable )
.setVersionable( versionable )
.setCascadeStyle( cascadeStyle )
.setOnDeleteAction( onDeleteAction )
.setFetchMode( fetchMode )
.createInformation()
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.hibernate.HibernateException;
import org.hibernate.MappingException;
import org.hibernate.annotations.NotFoundAction;
import org.hibernate.annotations.OnDeleteAction;
import org.hibernate.boot.spi.MetadataImplementor;
import org.hibernate.bytecode.enhance.spi.interceptor.EnhancementHelper;
import org.hibernate.bytecode.internal.BytecodeEnhancementMetadataNonPojoImpl;
Expand Down Expand Up @@ -63,6 +64,7 @@
import static org.hibernate.internal.util.ReflectHelper.isFinalClass;
import static org.hibernate.internal.util.collections.ArrayHelper.toIntArray;
import static org.hibernate.internal.util.collections.CollectionHelper.toSmallSet;
import static org.hibernate.tuple.PropertyFactory.buildIdentifierAttribute;

/**
* Centralizes metamodel information about an entity.
Expand Down Expand Up @@ -102,6 +104,7 @@ public class EntityMetamodel implements Serializable {
private final boolean[] propertyInsertability;
private final boolean[] propertyNullability;
private final boolean[] propertyVersionability;
private final OnDeleteAction[] propertyOnDeleteActions;
private final CascadeStyle[] cascadeStyles;
// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down Expand Up @@ -162,7 +165,7 @@ public EntityMetamodel(
EntityPersister persister,
RuntimeModelCreationContext creationContext,
Function<String, Generator> generatorSupplier) {
this.sessionFactory = creationContext.getSessionFactory();
sessionFactory = creationContext.getSessionFactory();

// Improves performance of EntityKey#equals by avoiding content check in String#equals
name = persistentClass.getEntityName().intern();
Expand All @@ -174,18 +177,18 @@ public EntityMetamodel(
subclassId = persistentClass.getSubclassId();

final Generator idgenerator = generatorSupplier.apply( rootName );
identifierAttribute = PropertyFactory.buildIdentifierAttribute( persistentClass, idgenerator );
identifierAttribute = buildIdentifierAttribute( persistentClass, idgenerator );

versioned = persistentClass.isVersioned();

final boolean collectionsInDefaultFetchGroupEnabled =
creationContext.getSessionFactoryOptions().isCollectionsInDefaultFetchGroupEnabled();
final boolean supportsCascadeDelete = creationContext.getDialect().supportsCascadeDelete();

if ( persistentClass.hasPojoRepresentation() ) {
final Component identifierMapperComponent = persistentClass.getIdentifierMapper();
final CompositeType nonAggregatedCidMapper;
final Set<String> idAttributeNames;

if ( identifierMapperComponent != null ) {
nonAggregatedCidMapper = identifierMapperComponent.getType();
idAttributeNames = new HashSet<>( );
Expand Down Expand Up @@ -226,6 +229,7 @@ public EntityMetamodel(
propertyNullability = new boolean[propertySpan];
propertyVersionability = new boolean[propertySpan];
propertyLaziness = new boolean[propertySpan];
propertyOnDeleteActions = new OnDeleteAction[propertySpan];
cascadeStyles = new CascadeStyle[propertySpan];
// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down Expand Up @@ -318,7 +322,7 @@ public EntityMetamodel(
nonlazyPropertyUpdateability[i] = attribute.isUpdateable() && !lazy;
propertyCheckability[i] = propertyUpdateability[i]
|| propertyType.isAssociationType() && ( (AssociationType) propertyType ).isAlwaysDirtyChecked();

propertyOnDeleteActions[i] = supportsCascadeDelete ? attribute.getOnDeleteAction() : null;
cascadeStyles[i] = attribute.getCascadeStyle();
// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down Expand Up @@ -887,4 +891,8 @@ public boolean isInstrumented() {
public BytecodeEnhancementMetadata getBytecodeEnhancementMetadata() {
return bytecodeEnhancementMetadata;
}

public OnDeleteAction[] getPropertyOnDeleteActions() {
return propertyOnDeleteActions;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
* SPDX-License-Identifier: LGPL-2.1-or-later
* Copyright Red Hat Inc. and Hibernate Authors
*/
package org.hibernate.orm.test;

import jakarta.persistence.Entity;
import jakarta.persistence.Id;
import jakarta.persistence.ManyToOne;
import jakarta.persistence.OneToMany;
import org.hibernate.annotations.OnDelete;
import org.hibernate.testing.orm.junit.EntityManagerFactoryScope;
import org.hibernate.testing.orm.junit.Jpa;
import org.junit.jupiter.api.Test;

import java.util.HashSet;
import java.util.Set;

import static jakarta.persistence.FetchType.EAGER;
import static org.hibernate.annotations.OnDeleteAction.CASCADE;
import static org.junit.jupiter.api.Assertions.assertNull;

@Jpa(annotatedClasses = {OnDeleteTest.Parent.class, OnDeleteTest.Child.class})
public class OnDeleteTest {
@Test
public void testOnDelete(EntityManagerFactoryScope scope) {
Parent parent = new Parent();
Child child = new Child();
child.parent = parent;
scope.inTransaction( em -> {
em.persist( parent );
em.persist( child );
} );
scope.inTransaction( em -> {
Parent p = em.find( Parent.class, parent.id );
em.remove( p );
} );
scope.inTransaction( em -> {
assertNull( em.find( Child.class, child.id ) );
} );
}

@Test
public void testOnDeleteReference(EntityManagerFactoryScope scope) {
Parent parent = new Parent();
Child child = new Child();
child.parent = parent;
parent.children.add( child );
scope.inTransaction( em -> {
em.persist( parent );
em.persist( child );
} );
scope.inTransaction( em -> em.remove( em.getReference( parent ) ) );
scope.inTransaction( em -> assertNull( em.find( Child.class, child.id ) ) );
}

@Entity
static class Parent {
@Id
long id;
@OneToMany(mappedBy = "parent", fetch = EAGER)
@OnDelete(action = CASCADE)
Set<Child> children = new HashSet<>();
}

@Entity
static class Child {
@Id
long id;
@ManyToOne
@OnDelete(action = CASCADE)
Parent parent;
}
}

0 comments on commit f10bec8

Please sign in to comment.