From 1d590911c2419a1efebb260d15950e62350bd129 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Fri, 1 Dec 2023 14:18:05 +0100 Subject: [PATCH 1/3] Test access to fields of @Embeddedable records --- .../RecordFieldAccessTest.java | 196 ++++++++++++++++++ 1 file changed, 196 insertions(+) create mode 100644 extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/applicationfieldaccess/RecordFieldAccessTest.java diff --git a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/applicationfieldaccess/RecordFieldAccessTest.java b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/applicationfieldaccess/RecordFieldAccessTest.java new file mode 100644 index 0000000000000..e7df873de95a7 --- /dev/null +++ b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/applicationfieldaccess/RecordFieldAccessTest.java @@ -0,0 +1,196 @@ +package io.quarkus.hibernate.orm.applicationfieldaccess; + +import static org.assertj.core.api.Assertions.assertThat; + +import jakarta.inject.Inject; +import jakarta.persistence.AttributeOverride; +import jakarta.persistence.Column; +import jakarta.persistence.Embeddable; +import jakarta.persistence.Embedded; +import jakarta.persistence.Entity; +import jakarta.persistence.EntityManager; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; + +import org.hibernate.Hibernate; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.narayana.jta.QuarkusTransaction; +import io.quarkus.test.QuarkusUnitTest; + +/** + * Checks that access to fields or getters of embeddable records by the application works correctly. + * See https://github.com/quarkusio/quarkus/issues/36747 + */ +public class RecordFieldAccessTest { + + @RegisterExtension + static QuarkusUnitTest runner = new QuarkusUnitTest() + .withApplicationRoot((jar) -> jar + .addClasses(MyEntity.class) + .addClasses(MyRecordEmbeddableWithoutAdditionalGetters.class) + .addClasses(MyRecordEmbeddableWithAdditionalGetters.class) + .addClass(AccessDelegate.class)) + .withConfigurationResource("application.properties"); + + @Inject + EntityManager em; + + @Test + public void recordWithoutAdditionalGetters_field() { + doTestFieldAccess(new AccessDelegate() { + @Override + public void setValue(MyEntity entity, Long value) { + entity.embeddedWithoutAdditionalGetters = new MyRecordEmbeddableWithoutAdditionalGetters(value); + } + + @Override + public Long getValue(MyEntity entity) { + return entity.embeddedWithoutAdditionalGetters == null ? null : entity.embeddedWithoutAdditionalGetters.value; + } + }); + } + + @Test + public void recordWithoutAdditionalGetters_recordGetter() { + doTestFieldAccess(new AccessDelegate() { + @Override + public void setValue(MyEntity entity, Long value) { + entity.embeddedWithoutAdditionalGetters = new MyRecordEmbeddableWithoutAdditionalGetters(value); + } + + @Override + public Long getValue(MyEntity entity) { + return entity.embeddedWithoutAdditionalGetters == null ? null : entity.embeddedWithoutAdditionalGetters.value(); + } + }); + } + + @Test + public void recordWithAdditionalGetters_field() { + doTestFieldAccess(new AccessDelegate() { + @Override + public void setValue(MyEntity entity, Long value) { + entity.embeddedWithAdditionalGetters = new MyRecordEmbeddableWithAdditionalGetters(value); + } + + @Override + public Long getValue(MyEntity entity) { + return entity.embeddedWithAdditionalGetters == null ? null : entity.embeddedWithAdditionalGetters.value; + } + }); + } + + @Test + public void recordWithAdditionalGetters_recordGetter() { + doTestFieldAccess(new AccessDelegate() { + @Override + public void setValue(MyEntity entity, Long value) { + entity.embeddedWithAdditionalGetters = new MyRecordEmbeddableWithAdditionalGetters(value); + } + + @Override + public Long getValue(MyEntity entity) { + return entity.embeddedWithAdditionalGetters == null ? null : entity.embeddedWithAdditionalGetters.value(); + } + }); + } + + @Test + public void recordWithAdditionalGetters_additionalGetter() { + doTestFieldAccess(new AccessDelegate() { + @Override + public void setValue(MyEntity entity, Long value) { + entity.embeddedWithAdditionalGetters = new MyRecordEmbeddableWithAdditionalGetters(value); + } + + @Override + public Long getValue(MyEntity entity) { + return entity.embeddedWithAdditionalGetters == null ? null : entity.embeddedWithAdditionalGetters.getValue(); + } + }); + } + + // Ideally we'd make this a @ParameterizedTest and pass the access delegate as parameter, + // but we cannot do that due to JUnit using a different classloader than the test. + private void doTestFieldAccess(AccessDelegate delegate) { + Long id = QuarkusTransaction.disallowingExisting().call(() -> { + var entity = new MyEntity(); + em.persist(entity); + return entity.id; + }); + + QuarkusTransaction.disallowingExisting().run(() -> { + var entity = em.find(MyEntity.class, id); + assertThat(delegate.getValue(entity)) + .as("Loaded value before update") + .isNull(); + }); + + QuarkusTransaction.disallowingExisting().run(() -> { + var entity = em.getReference(MyEntity.class, id); + // Since field access is replaced with accessor calls, + // we expect this change to be detected by dirty tracking and persisted. + delegate.setValue(entity, 42L); + }); + + QuarkusTransaction.disallowingExisting().run(() -> { + var entity = em.find(MyEntity.class, id); + // We're working on an initialized entity. + assertThat(entity) + .as("find() should return uninitialized entity") + .returns(true, Hibernate::isInitialized); + // The above should have persisted a value that passes the assertion. + assertThat(delegate.getValue(entity)) + .as("Loaded value after update") + .isEqualTo(42L); + }); + + QuarkusTransaction.disallowingExisting().run(() -> { + var entity = em.getReference(MyEntity.class, id); + // We're working on an uninitialized entity. + assertThat(entity) + .as("getReference() should return uninitialized entity") + .returns(false, Hibernate::isInitialized); + // The above should have persisted a value that passes the assertion. + assertThat(delegate.getValue(entity)) + .as("Lazily loaded value after update") + .isEqualTo(42L); + // Accessing the value should trigger initialization of the entity. + assertThat(entity) + .as("Getting the value should initialize the entity") + .returns(true, Hibernate::isInitialized); + }); + } + + @Entity(name = "myentity") + public static class MyEntity { + @Id + @GeneratedValue + public long id; + @Embedded + @AttributeOverride(name = "value", column = @Column(name = "value1")) + public MyRecordEmbeddableWithAdditionalGetters embeddedWithAdditionalGetters; + @Embedded + @AttributeOverride(name = "value", column = @Column(name = "value2")) + public MyRecordEmbeddableWithoutAdditionalGetters embeddedWithoutAdditionalGetters; + } + + @Embeddable + public record MyRecordEmbeddableWithoutAdditionalGetters(Long value) { + } + + @Embeddable + public record MyRecordEmbeddableWithAdditionalGetters(Long value) { + Long getValue() { + return value; + } + } + + private interface AccessDelegate { + void setValue(MyEntity entity, Long value); + + Long getValue(MyEntity entity); + } +} From 79e3b19ec13360b305fa6941516559e325faa3dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Fri, 1 Dec 2023 14:32:12 +0100 Subject: [PATCH 2/3] Test access to fields of @Immutable embeddables --- .../ImmutableEmbeddableFieldAccessTest.java | 193 ++++++++++++++++++ 1 file changed, 193 insertions(+) create mode 100644 extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/applicationfieldaccess/ImmutableEmbeddableFieldAccessTest.java diff --git a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/applicationfieldaccess/ImmutableEmbeddableFieldAccessTest.java b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/applicationfieldaccess/ImmutableEmbeddableFieldAccessTest.java new file mode 100644 index 0000000000000..123a97aa63ef5 --- /dev/null +++ b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/applicationfieldaccess/ImmutableEmbeddableFieldAccessTest.java @@ -0,0 +1,193 @@ +package io.quarkus.hibernate.orm.applicationfieldaccess; + +import static org.assertj.core.api.Assertions.assertThat; + +import jakarta.inject.Inject; +import jakarta.persistence.AttributeOverride; +import jakarta.persistence.Column; +import jakarta.persistence.Embeddable; +import jakarta.persistence.Embedded; +import jakarta.persistence.Entity; +import jakarta.persistence.EntityManager; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; + +import org.hibernate.Hibernate; +import org.hibernate.annotations.Immutable; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.narayana.jta.QuarkusTransaction; +import io.quarkus.test.QuarkusUnitTest; + +/** + * Checks that access to record fields or record getters by the application works correctly. + */ +public class ImmutableEmbeddableFieldAccessTest { + + @RegisterExtension + static QuarkusUnitTest runner = new QuarkusUnitTest() + .withApplicationRoot((jar) -> jar + .addClasses(MyEntity.class) + .addClasses(MyImmutableEmbeddableWithFieldAccess.class) + .addClasses(MyImmutableEmbeddableWithAccessors.class) + .addClass(AccessDelegate.class)) + .withConfigurationResource("application.properties"); + + @Inject + EntityManager em; + + @Test + public void immutableEmbeddableWithoutAdditionalGetters_field() { + doTestFieldAccess(new AccessDelegate() { + @Override + public void setValue(MyEntity entity, Long value) { + var embedded = new MyImmutableEmbeddableWithFieldAccess(); + embedded.value = value; + entity.embeddedWithoutAccessors = embedded; + } + + @Override + public Long getValue(MyEntity entity) { + return entity.embeddedWithoutAccessors == null ? null : entity.embeddedWithoutAccessors.value; + } + }); + } + + @Test + public void immutableEmbeddableWithAdditionalGetters_field() { + doTestFieldAccess(new AccessDelegate() { + @Override + public void setValue(MyEntity entity, Long value) { + var embedded = new MyImmutableEmbeddableWithAccessors(); + // Assuming this is changed only once on initialization, + // which is the only way the @Immutable annotation would make sense. + embedded.value = value; + entity.embeddedWithFieldAccess = embedded; + } + + @Override + public Long getValue(MyEntity entity) { + return entity.embeddedWithFieldAccess == null ? null : entity.embeddedWithFieldAccess.value; + } + }); + } + + @Test + public void immutableEmbeddableWithAccessors() { + doTestFieldAccess(new AccessDelegate() { + @Override + public void setValue(MyEntity entity, Long value) { + var embedded = new MyImmutableEmbeddableWithAccessors(); + // Assuming this is changed only once on initialization, + // which is the only way the @Immutable annotation would make sense. + embedded.setValue(value); + entity.embeddedWithFieldAccess = embedded; + } + + @Override + public Long getValue(MyEntity entity) { + return entity.embeddedWithFieldAccess == null ? null : entity.embeddedWithFieldAccess.getValue(); + } + }); + } + + // Ideally we'd make this a @ParameterizedTest and pass the access delegate as parameter, + // but we cannot do that due to JUnit using a different classloader than the test. + private void doTestFieldAccess(AccessDelegate delegate) { + Long id = QuarkusTransaction.disallowingExisting().call(() -> { + var entity = new MyEntity(); + em.persist(entity); + return entity.id; + }); + + QuarkusTransaction.disallowingExisting().run(() -> { + var entity = em.find(MyEntity.class, id); + assertThat(delegate.getValue(entity)) + .as("Loaded value before update") + .isNull(); + }); + + QuarkusTransaction.disallowingExisting().run(() -> { + var entity = em.getReference(MyEntity.class, id); + // Since field access is replaced with accessor calls, + // we expect this change to be detected by dirty tracking and persisted. + delegate.setValue(entity, 42L); + }); + + QuarkusTransaction.disallowingExisting().run(() -> { + var entity = em.find(MyEntity.class, id); + // We're working on an initialized entity. + assertThat(entity) + .as("find() should return uninitialized entity") + .returns(true, Hibernate::isInitialized); + // The above should have persisted a value that passes the assertion. + assertThat(delegate.getValue(entity)) + .as("Loaded value after update") + .isEqualTo(42L); + }); + + QuarkusTransaction.disallowingExisting().run(() -> { + var entity = em.getReference(MyEntity.class, id); + // We're working on an uninitialized entity. + assertThat(entity) + .as("getReference() should return uninitialized entity") + .returns(false, Hibernate::isInitialized); + // The above should have persisted a value that passes the assertion. + assertThat(delegate.getValue(entity)) + .as("Lazily loaded value after update") + .isEqualTo(42L); + // Accessing the value should trigger initialization of the entity. + assertThat(entity) + .as("Getting the value should initialize the entity") + .returns(true, Hibernate::isInitialized); + }); + } + + @Entity(name = "myentity") + public static class MyEntity { + @Id + @GeneratedValue + public long id; + @Embedded + @AttributeOverride(name = "value", column = @Column(name = "value1")) + public MyImmutableEmbeddableWithAccessors embeddedWithFieldAccess; + @Embedded + @AttributeOverride(name = "value", column = @Column(name = "value2")) + public MyImmutableEmbeddableWithFieldAccess embeddedWithoutAccessors; + } + + @Immutable + @Embeddable + public static class MyImmutableEmbeddableWithFieldAccess { + public Long value; + + public MyImmutableEmbeddableWithFieldAccess() { + } + } + + @Immutable + @Embeddable + public static class MyImmutableEmbeddableWithAccessors { + private Long value; + + // For Hibernate ORM instantiation + protected MyImmutableEmbeddableWithAccessors() { + } + + public Long getValue() { + return value; + } + + // For Hibernate ORM instantiation + protected void setValue(Long value) { + this.value = value; + } + } + + private interface AccessDelegate { + void setValue(MyEntity entity, Long value); + + Long getValue(MyEntity entity); + } +} From 1b426fc415832a026d0f3806f685f50034bf6359 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Fri, 1 Dec 2023 14:41:10 +0100 Subject: [PATCH 3/3] Fix Panache bytecode enhancement for @Embeddable records --- .../deployment/PanacheHibernateCommonResourceProcessor.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/extensions/panache/panache-hibernate-common/deployment/src/main/java/io/quarkus/panache/common/deployment/PanacheHibernateCommonResourceProcessor.java b/extensions/panache/panache-hibernate-common/deployment/src/main/java/io/quarkus/panache/common/deployment/PanacheHibernateCommonResourceProcessor.java index 93037abea56c9..6127afcc2955b 100644 --- a/extensions/panache/panache-hibernate-common/deployment/src/main/java/io/quarkus/panache/common/deployment/PanacheHibernateCommonResourceProcessor.java +++ b/extensions/panache/panache-hibernate-common/deployment/src/main/java/io/quarkus/panache/common/deployment/PanacheHibernateCommonResourceProcessor.java @@ -157,9 +157,12 @@ private EntityModel createEntityModel(ClassInfo classInfo) { // so we need to be careful when we enhance private fields, // because the corresponding `$_hibernate_{read/write}_*()` methods // will only be generated for classes mapped through *annotations*. - boolean willBeEnhancedByHibernateOrm = classInfo.hasAnnotation(DOTNAME_ENTITY) + boolean isManaged = classInfo.hasAnnotation(DOTNAME_ENTITY) || classInfo.hasAnnotation(DOTNAME_MAPPED_SUPERCLASS) || classInfo.hasAnnotation(DOTNAME_EMBEDDABLE); + boolean willBeEnhancedByHibernateOrm = isManaged + // Records are immutable, thus never enhanced + && !classInfo.isRecord(); for (FieldInfo fieldInfo : classInfo.fields()) { String name = fieldInfo.name(); if (!Modifier.isStatic(fieldInfo.flags())