From defdfc54f4bb12c8b8e9be04ac5bf1640590f832 Mon Sep 17 00:00:00 2001 From: Marco Belladelli Date: Thu, 23 Nov 2023 21:40:47 +0100 Subject: [PATCH] HHH-17334 Simplify indexed element collection update and assert element column mutability --- .../boot/model/internal/CollectionBinder.java | 29 ++++++++++ .../collection/BasicCollectionPersister.java | 58 ++++--------------- ...ementCollectionWithUpdatableFalseTest.java | 2 + ...ementCollectionWithUpdatableFalseTest.java | 2 + 4 files changed, 44 insertions(+), 47 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/CollectionBinder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/CollectionBinder.java index dff2ca943c..f7527bab5b 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/CollectionBinder.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/CollectionBinder.java @@ -2179,6 +2179,7 @@ public abstract class CollectionBinder { } checkFilterConditions( collection ); + checkConsistentColumnMutability( collection ); } private void handleElementCollection(XClass elementType, String hqlOrderBy) { @@ -2666,6 +2667,34 @@ public abstract class CollectionBinder { } } + private static void checkConsistentColumnMutability(Collection collection) { + checkConsistentColumnMutability( collection.getRole(), collection.getKey() ); + checkConsistentColumnMutability( collection.getRole(), collection.getElement() ); + } + + private static void checkConsistentColumnMutability(String collectionRole, Value value) { + Boolean readOnly = null; + for ( int i = 0; i < value.getColumnSpan(); i++ ) { + final boolean insertable = value.isColumnInsertable( i ); + if ( insertable != value.isColumnUpdateable( i ) ) { + throw new AnnotationException( + "Join column '" + value.getColumns().get( i ).getName() + "' on collection property '" + + collectionRole + "' must be defined with the same insertable and updatable attributes" + ); + } + if ( readOnly == null ) { + readOnly = insertable; + } + else if ( readOnly != insertable && !value.getColumns().get( i ).isFormula() ) { + // We also assert that all join columns have the same mutability (except formulas) + throw new AnnotationException( + "All join columns on collection '" + collectionRole + "' should have" + + " the same insertable and updatable setting" + ); + } + } + } + private void bindCollectionSecondPass(PersistentClass targetEntity, AnnotatedJoinColumns joinColumns) { if ( !isUnownedCollection() ) { diff --git a/hibernate-core/src/main/java/org/hibernate/persister/collection/BasicCollectionPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/collection/BasicCollectionPersister.java index 8b0f0cc3f0..1f51160f56 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/collection/BasicCollectionPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/collection/BasicCollectionPersister.java @@ -491,32 +491,8 @@ public class BasicCollectionPersister extends AbstractCollectionPersister { // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // SET - if ( hasIndex() ) { - /* - Given : + attribute.getElementDescriptor().forEachUpdatable( updateBuilder ); - class MyEntity { - @ElementCollection(fetch = FetchType.LAZY) - @OrderColumn - private List myEmbeddables; - } - - @Embeddable - public static class MyEmbeddable { - @Column(updatable = false) - private String embeddedProperty; - } - - we cannot understand if the update is due to a change in the value embeddedProperty or because a - new element has been added to the list in an existing position (update) shifting the old value (insert). - - For this reason we cannot take into consideration the `@Column(updatable = false)` - */ - attribute.getElementDescriptor().forEachNonFormula( updateBuilder ); - } - else { - attribute.getElementDescriptor().forEachUpdatable( updateBuilder ); - } // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // WHERE @@ -552,28 +528,16 @@ public class BasicCollectionPersister extends AbstractCollectionPersister { 0, jdbcValueBindings, null, - hasIndex() ? - (valueIndex, bindings, y, jdbcValue, jdbcValueMapping) -> { - if ( jdbcValueMapping.isFormula() ) { - return; - } - bindings.bindValue( - jdbcValue, - jdbcValueMapping, - ParameterUsage.SET - ); - } - : - (valueIndex, bindings, y, jdbcValue, jdbcValueMapping) -> { - if ( !jdbcValueMapping.isUpdateable() || jdbcValueMapping.isFormula() ) { - return; - } - bindings.bindValue( - jdbcValue, - jdbcValueMapping, - ParameterUsage.SET - ); - }, + (valueIndex, bindings, y, jdbcValue, jdbcValueMapping) -> { + if ( !jdbcValueMapping.isUpdateable() || jdbcValueMapping.isFormula() ) { + return; + } + bindings.bindValue( + jdbcValue, + jdbcValueMapping, + ParameterUsage.SET + ); + }, session ); } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/embeddable/EmbeddableAsElementCollectionWithUpdatableFalseTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/embeddable/EmbeddableAsElementCollectionWithUpdatableFalseTest.java index f3ed5fd3aa..1e3bcd24e3 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/embeddable/EmbeddableAsElementCollectionWithUpdatableFalseTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/embeddable/EmbeddableAsElementCollectionWithUpdatableFalseTest.java @@ -8,6 +8,7 @@ import org.hibernate.testing.orm.junit.JiraKey; import org.hibernate.testing.orm.junit.SessionFactory; import org.hibernate.testing.orm.junit.SessionFactoryScope; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import jakarta.persistence.Column; @@ -26,6 +27,7 @@ import static org.assertj.core.api.Assertions.assertThat; }) @SessionFactory @JiraKey("HHH-16573") +@Disabled("We now assert that collection columns have the same insertable / updatable attributes, see HHH-17334") public class EmbeddableAsElementCollectionWithUpdatableFalseTest { @AfterEach diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/embeddable/EmbeddableAsOrderedElementCollectionWithUpdatableFalseTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/embeddable/EmbeddableAsOrderedElementCollectionWithUpdatableFalseTest.java index 5edcb7513f..b9bdf6ceec 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/embeddable/EmbeddableAsOrderedElementCollectionWithUpdatableFalseTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/embeddable/EmbeddableAsOrderedElementCollectionWithUpdatableFalseTest.java @@ -8,6 +8,7 @@ import org.hibernate.testing.orm.junit.JiraKey; import org.hibernate.testing.orm.junit.SessionFactory; import org.hibernate.testing.orm.junit.SessionFactoryScope; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import jakarta.persistence.Column; @@ -27,6 +28,7 @@ import static org.assertj.core.api.Assertions.assertThat; }) @SessionFactory @JiraKey("HHH-16573") +@Disabled("We now assert that collection columns have the same insertable / updatable attributes, see HHH-17334") public class EmbeddableAsOrderedElementCollectionWithUpdatableFalseTest { @AfterEach