From 8f4450c43349eb8f96d613fab6ec6cdac57352b5 Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Tue, 22 Jun 2021 10:55:43 +0200 Subject: [PATCH] HHH-14597 Test and fix for NPE while trying to delete cascade to-one association within element collection --- .../hibernate/engine/internal/Cascade.java | 37 ++++++++----- .../ElementCollectionOrphanTest.java | 53 +++++++++++++++++++ .../elementcollection/EnrollableClass.java | 34 ++++++++++++ .../elementcollection/EnrolledClassSeat.java | 31 +++++++++++ .../orphan/elementcollection/Student.java | 43 +++++++++++++++ .../StudentEnrolledClass.java | 31 +++++++++++ .../orphan/elementcollection/student.hbm.xml | 45 ++++++++++++++++ 7 files changed, 261 insertions(+), 13 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/ElementCollectionOrphanTest.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/EnrollableClass.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/EnrolledClassSeat.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/Student.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/StudentEnrolledClass.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/student.hbm.xml diff --git a/hibernate-core/src/main/java/org/hibernate/engine/internal/Cascade.java b/hibernate-core/src/main/java/org/hibernate/engine/internal/Cascade.java index 107a3eff8f..762ab776f5 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/internal/Cascade.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/internal/Cascade.java @@ -232,10 +232,12 @@ public final class Cascade { } } else if ( type.isComponentType() ) { - if ( componentPath == null ) { + if ( componentPath == null && propertyName != null ) { componentPath = new ArrayList<>(); } - componentPath.add( propertyName ); + if ( componentPath != null ) { + componentPath.add( propertyName ); + } cascadeComponent( action, cascadePoint, @@ -246,7 +248,9 @@ public final class Cascade { (CompositeType) type, anything ); - componentPath.remove( componentPath.size() - 1 ); + if ( componentPath != null ) { + componentPath.remove( componentPath.size() - 1 ); + } } } @@ -293,17 +297,24 @@ public final class Cascade { // Since the loadedState in the EntityEntry is a flat domain type array // We first have to extract the component object and then ask the component type // recursively to give us the value of the sub-property of that object - loadedValue = entry.getLoadedValue( componentPath.get( 0 ) ); - ComponentType componentType = (ComponentType) entry.getPersister().getPropertyType( componentPath.get( 0 ) ); - if ( componentPath.size() != 1 ) { - for ( int i = 1; i < componentPath.size(); i++ ) { - final int subPropertyIndex = componentType.getPropertyIndex( componentPath.get( i ) ); - loadedValue = componentType.getPropertyValue( loadedValue, subPropertyIndex ); - componentType = (ComponentType) componentType.getSubtypes()[subPropertyIndex]; + final Type propertyType = entry.getPersister().getPropertyType( componentPath.get(0) ); + if ( propertyType instanceof ComponentType ) { + loadedValue = entry.getLoadedValue( componentPath.get( 0 ) ); + ComponentType componentType = (ComponentType) propertyType; + if ( componentPath.size() != 1 ) { + for ( int i = 1; i < componentPath.size(); i++ ) { + final int subPropertyIndex = componentType.getPropertyIndex( componentPath.get( i ) ); + loadedValue = componentType.getPropertyValue( loadedValue, subPropertyIndex ); + componentType = (ComponentType) componentType.getSubtypes()[subPropertyIndex]; + } } - } - loadedValue = componentType.getPropertyValue( loadedValue, componentType.getPropertyIndex( propertyName ) ); + loadedValue = componentType.getPropertyValue( loadedValue, componentType.getPropertyIndex( propertyName ) ); + } + else { + // Association is probably defined in an element collection, so we can't do orphan removals + loadedValue = null; + } } // orphaned if the association was nulled (child == null) or receives a new value while the @@ -538,7 +549,7 @@ public final class Cascade { itr.next(), elemType, style, - null, + collectionType.getRole().substring( collectionType.getRole().lastIndexOf('.') + 1 ), anything, isCascadeDeleteEnabled ); diff --git a/hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/ElementCollectionOrphanTest.java b/hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/ElementCollectionOrphanTest.java new file mode 100644 index 0000000000..83de4af020 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/ElementCollectionOrphanTest.java @@ -0,0 +1,53 @@ +package org.hibernate.test.orphan.elementcollection; + +import java.util.Collections; + +import org.hibernate.Session; +import org.hibernate.SessionFactory; +import org.hibernate.boot.Metadata; +import org.hibernate.boot.MetadataSources; +import org.hibernate.boot.registry.StandardServiceRegistryBuilder; +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; +import org.hibernate.testing.transaction.TransactionUtil; +import org.junit.Before; +import org.junit.Test; + +@TestForIssue(jiraKey = "HHH-14597") +public class ElementCollectionOrphanTest extends BaseCoreFunctionalTestCase { + + @Override + protected String[] getMappings() { + return new String[] { "orphan/elementcollection/student.hbm.xml" }; + } + + @Test + public void setCompositeElementTest() { + TransactionUtil.doInHibernate( + this::sessionFactory, + session -> { + EnrollableClass aClass = new EnrollableClass(); + aClass.setId("123"); + aClass.setName("Math"); + session.save(aClass); + + Student aStudent = new Student(); + aStudent.setId("s1"); + aStudent.setFirstName("John"); + aStudent.setLastName("Smith"); + + EnrolledClassSeat seat = new EnrolledClassSeat(); + seat.setId("seat1"); + seat.setRow(10); + seat.setColumn(5); + + StudentEnrolledClass enrClass = new StudentEnrolledClass(); + enrClass.setEnrolledClass(aClass); + enrClass.setClassStartTime(130); + enrClass.setSeat(seat); + aStudent.setEnrolledClasses(Collections.singleton(enrClass)); + session.save(aStudent); + } + ); + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/EnrollableClass.java b/hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/EnrollableClass.java new file mode 100644 index 0000000000..5ae6e94d60 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/EnrollableClass.java @@ -0,0 +1,34 @@ +package org.hibernate.test.orphan.elementcollection; + +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.Table; + +@Entity +@Table(name = "ENROLLABLECLASS") +public class EnrollableClass { + + @Id + @Column(name = "id") + private String id; + + @Column(name = "name") + private String name; + + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/EnrolledClassSeat.java b/hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/EnrolledClassSeat.java new file mode 100644 index 0000000000..3a1d2654d0 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/EnrolledClassSeat.java @@ -0,0 +1,31 @@ +package org.hibernate.test.orphan.elementcollection; + +public class EnrolledClassSeat { + private String id; + private int row; + private int column; + + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + + public int getRow() { + return row; + } + + public void setRow(int row) { + this.row = row; + } + + public int getColumn() { + return column; + } + + public void setColumn(int column) { + this.column = column; + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/Student.java b/hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/Student.java new file mode 100644 index 0000000000..541184233f --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/Student.java @@ -0,0 +1,43 @@ +package org.hibernate.test.orphan.elementcollection; + +import java.util.Set; + +public class Student { + + private String id; + private String firstName; + private String lastName; + private Set< StudentEnrolledClass > enrolledClasses; + + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + + public String getFirstName() { + return firstName; + } + + public void setFirstName(String firstName) { + this.firstName = firstName; + } + + public String getLastName() { + return lastName; + } + + public void setLastName(String lastName) { + this.lastName = lastName; + } + + public Set getEnrolledClasses() { + return enrolledClasses; + } + + public void setEnrolledClasses(Set enrolledClasses) { + this.enrolledClasses = enrolledClasses; + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/StudentEnrolledClass.java b/hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/StudentEnrolledClass.java new file mode 100644 index 0000000000..199c865000 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/StudentEnrolledClass.java @@ -0,0 +1,31 @@ +package org.hibernate.test.orphan.elementcollection; + +public class StudentEnrolledClass { + private EnrollableClass enrolledClass; + private int classStartTime; + private EnrolledClassSeat seat; + + public EnrollableClass getEnrolledClass() { + return enrolledClass; + } + + public void setEnrolledClass(EnrollableClass enrolledClass) { + this.enrolledClass = enrolledClass; + } + + public int getClassStartTime() { + return classStartTime; + } + + public void setClassStartTime(int classStartTime) { + this.classStartTime = classStartTime; + } + + public EnrolledClassSeat getSeat() { + return seat; + } + + public void setSeat(EnrolledClassSeat seat) { + this.seat = seat; + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/student.hbm.xml b/hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/student.hbm.xml new file mode 100644 index 0000000000..60af248813 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/orphan/elementcollection/student.hbm.xml @@ -0,0 +1,45 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +