diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/EntityKey.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/EntityKey.java index a1ac579fad..0274269938 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/spi/EntityKey.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/spi/EntityKey.java @@ -48,7 +48,7 @@ public final class EntityKey implements Serializable { public EntityKey(Object id, EntityPersister persister) { this.persister = persister; if ( id == null ) { - throw new AssertionFailure( "null identifier" ); + throw new AssertionFailure( "null identifier (" + persister.getEntityName() + ")" ); } this.identifier = id; this.hashCode = generateHashCode(); diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/AttributeMapping.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/AttributeMapping.java index f63ad63a81..20e67b66e5 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/AttributeMapping.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/AttributeMapping.java @@ -6,6 +6,7 @@ */ package org.hibernate.metamodel.mapping; +import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.property.access.spi.PropertyAccess; import org.hibernate.sql.results.graph.Fetchable; @@ -39,11 +40,22 @@ public interface AttributeMapping extends ModelPart, ValueMapping, Fetchable, Pr * Convenient access to getting the value for this attribute from the "owner" */ default Object getValue(Object container, SharedSessionContractImplementor session) { + return getValue( container, session.getSessionFactory() ); + } + + /** + * Convenient access to getting the value for this attribute from the "owner" + */ + default Object getValue(Object container, SessionFactoryImplementor sessionFactory) { return getPropertyAccess().getGetter().get( container ); } default void setValue(Object container, Object value, SharedSessionContractImplementor session) { - getPropertyAccess().getSetter().set( container, value, session.getSessionFactory() ); + setValue( container, value, session.getSessionFactory() ); + } + + default void setValue(Object container, Object value, SessionFactoryImplementor sessionFactory) { + getPropertyAccess().getSetter().set( container, value, sessionFactory ); } /** diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/EmbeddableMappingTypeImpl.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/EmbeddableMappingTypeImpl.java index 9213be29c2..2c3b1b8e6b 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/EmbeddableMappingTypeImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/EmbeddableMappingTypeImpl.java @@ -811,12 +811,8 @@ public class EmbeddableMappingTypeImpl implements EmbeddableMappingType, Selecta } for ( int i = 0; i < attributeMappings.size(); i++ ) { - attributeMappings.get( i ) - .getAttributeMetadataAccess() - .resolveAttributeMetadata( null ) - .getPropertyAccess() - .getSetter() - .set( compositeInstance, resolvedValues[i], sessionFactory ); + final AttributeMapping attributeMapping = attributeMappings.get( i ); + attributeMapping.setValue( compositeInstance, resolvedValues[i], sessionFactory ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/AbstractEmbeddableInitializer.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/AbstractEmbeddableInitializer.java index 95d5773fc4..af5d82a660 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/AbstractEmbeddableInitializer.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/AbstractEmbeddableInitializer.java @@ -29,12 +29,14 @@ import org.hibernate.sql.results.graph.Fetch; import org.hibernate.sql.results.graph.FetchParentAccess; import org.hibernate.sql.results.graph.Initializer; import org.hibernate.sql.results.graph.collection.CollectionInitializer; -import org.hibernate.sql.results.graph.entity.AbstractEntityInitializer; import org.hibernate.sql.results.graph.entity.EntityInitializer; import org.hibernate.sql.results.internal.NullValueAssembler; import org.hibernate.sql.results.jdbc.spi.RowProcessingState; +import org.hibernate.type.descriptor.java.JavaType; import org.hibernate.type.descriptor.java.spi.EntityJavaTypeDescriptor; +import static java.lang.Boolean.FALSE; +import static java.lang.Boolean.TRUE; import static org.hibernate.internal.util.collections.CollectionHelper.arrayList; /** @@ -85,6 +87,10 @@ public abstract class AbstractEmbeddableInitializer extends AbstractFetchParentA this.resolvedValues = new Object[ numOfAttrs ]; this.assemblers = arrayList( numOfAttrs ); + // todo (6.0) - why not just use the "maps id" form right here? + // ( (CompositeIdentifierMapping) embedded ).getMappedIdEmbeddableTypeDescriptor() + // in theory this should let us avoid the explicit maps-id handling via `setPropertyValuesOnTarget` + embeddableTypeDescriptor.visitFetchables( stateArrayContributor -> { final Fetch fetch = resultDescriptor.findFetch( stateArrayContributor ); @@ -126,12 +132,21 @@ public abstract class AbstractEmbeddableInitializer extends AbstractFetchParentA } @Override - public void resolveKey(RowProcessingState rowProcessingState) { + public FetchParentAccess findFirstEntityDescriptorAccess() { + return getFetchParentAccess().findFirstEntityDescriptorAccess(); + } + + @Override + public void resolveKey(RowProcessingState processingState) { // nothing to do } @Override - public void resolveInstance(RowProcessingState rowProcessingState) { + public void resolveInstance(RowProcessingState processingState) { + reallyResolve( processingState ); + } + + private void reallyResolve(RowProcessingState processingState) { if ( compositeInstance != null ) { return; } @@ -140,14 +155,25 @@ public abstract class AbstractEmbeddableInitializer extends AbstractFetchParentA // which we access through the fetch parent access. // If this model part is an identifier, we must construct the instance as this is called during resolveKey final EmbeddableMappingType embeddableTypeDescriptor = embedded.getEmbeddableTypeDescriptor(); + final JavaType embeddableJtd = embeddableTypeDescriptor.getMappedJavaTypeDescriptor(); - if ( fetchParentAccess != null && embeddableTypeDescriptor.getMappedJavaTypeDescriptor().getJavaTypeClass() - .isAssignableFrom( fetchParentAccess.getInitializedPart().getJavaTypeDescriptor().getJavaTypeClass() ) - && embeddableTypeDescriptor.getMappedJavaTypeDescriptor() instanceof EntityJavaTypeDescriptor + if ( fetchParentAccess != null && + embeddableJtd.getJavaTypeClass().isAssignableFrom( fetchParentAccess.getInitializedPart().getJavaTypeDescriptor().getJavaTypeClass() ) + && embeddableJtd instanceof EntityJavaTypeDescriptor && !( embedded instanceof CompositeIdentifierMapping ) && !EntityIdentifierMapping.ROLE_LOCAL_NAME.equals( embedded.getFetchableName() ) ) { - fetchParentAccess.resolveInstance( rowProcessingState ); + EmbeddableLoadingLogger.INSTANCE.debugf( + "Linking composite instance to fetch-parent [%s] - %s", + navigablePath, + fetchParentAccess + ); + fetchParentAccess.resolveInstance( processingState ); compositeInstance = fetchParentAccess.getInitializedInstance(); + EmbeddableLoadingLogger.INSTANCE.debugf( + "Done linking composite instance to fetch-parent [%s] - %s", + navigablePath, + compositeInstance + ); } if ( compositeInstance == null ) { @@ -164,37 +190,24 @@ public abstract class AbstractEmbeddableInitializer extends AbstractFetchParentA @Override public void initializeInstance(RowProcessingState processingState) { - final Initializer initializer = processingState.resolveInitializer( navigablePath.getParent() ); - - handleParentInjection( processingState ); - EmbeddableLoadingLogger.INSTANCE.debugf( "Initializing composite instance [%s]", navigablePath ); - boolean areAllValuesNull = true; - for ( int i = 0; i < assemblers.size(); i++ ) { - final DomainResultAssembler assembler = assemblers.get( i ); - final Object contributorValue = assembler.assemble( - processingState, - processingState.getJdbcValuesSourceProcessingState().getProcessingOptions() - ); + handleParentInjection( processingState ); - resolvedValues[i] = contributorValue; - if ( contributorValue != null ) { - areAllValuesNull = false; - } - } + extractRowState( processingState ); - if ( !createEmptyCompositesEnabled && areAllValuesNull ) { + if ( !createEmptyCompositesEnabled && allValuesNull == TRUE ) { compositeInstance = null; } else { notifyResolutionListeners( compositeInstance ); if ( compositeInstance instanceof HibernateProxy ) { - if ( initializer != this ) { - ( (AbstractEntityInitializer) initializer ).registerResolutionListener( + final Initializer parentInitializer = processingState.resolveInitializer( navigablePath.getParent() ); + if ( parentInitializer != this ) { + ( (FetchParentAccess) parentInitializer ).registerResolutionListener( entity -> setPropertyValuesOnTarget( entity, processingState.getSession() ) ); } @@ -214,11 +227,28 @@ public abstract class AbstractEmbeddableInitializer extends AbstractFetchParentA // A possible alternative could be to initialize the resolved values for primitive fields to their default value, // but that might cause unexpected outcomes for Hibernate 5 users that use createEmptyCompositesEnabled when updating. // You can see the need for this by running EmptyCompositeEquivalentToNullTest - else if ( !areAllValuesNull ) { + else if ( allValuesNull == FALSE ) { setPropertyValuesOnTarget( compositeInstance, processingState.getSession() ); } } } + + private void extractRowState(RowProcessingState processingState) { + allValuesNull = true; + for ( int i = 0; i < assemblers.size(); i++ ) { + final DomainResultAssembler assembler = assemblers.get( i ); + final Object contributorValue = assembler.assemble( + processingState, + processingState.getJdbcValuesSourceProcessingState().getProcessingOptions() + ); + + resolvedValues[i] = contributorValue; + if ( contributorValue != null ) { + allValuesNull = false; + } + } + } + private void handleParentInjection(RowProcessingState processingState) { final PropertyAccess parentInjectionAccess = embedded.getParentInjectionAttributePropertyAccess(); if ( parentInjectionAccess == null ) { @@ -229,8 +259,7 @@ public abstract class AbstractEmbeddableInitializer extends AbstractFetchParentA // todo (6.0) : should we initialize the composite instance if we get here and it is null (not NULL_MARKER)? // we want to avoid injection for `NULL_MARKER` - final Object compositeInstance = getCompositeInstance(); - if ( compositeInstance == null ) { + if ( compositeInstance == null || compositeInstance == NULL_MARKER ) { EmbeddableLoadingLogger.INSTANCE.debugf( "Skipping parent injection for null embeddable [%s]", navigablePath @@ -342,11 +371,12 @@ public abstract class AbstractEmbeddableInitializer extends AbstractFetchParentA @Override public void finishUpRow(RowProcessingState rowProcessingState) { compositeInstance = null; + allValuesNull = null; clearResolutionListeners(); } @Override - public FetchParentAccess findFirstEntityDescriptorAccess() { - return getFetchParentAccess().findFirstEntityDescriptorAccess(); + public String toString() { + return getClass().getSimpleName() + "(" + navigablePath + ") : `" + getInitializedPart().getJavaTypeDescriptor().getJavaTypeClass() + "`"; } } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/cid/idclass/Customer.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/cid/idclass/Customer.java new file mode 100644 index 0000000000..d939113ca8 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/cid/idclass/Customer.java @@ -0,0 +1,44 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later + * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html + */ +package org.hibernate.orm.test.mapping.cid.idclass; /** + * @author Steve Ebersole + */ + +import jakarta.persistence.Entity; +import jakarta.persistence.Table; +import jakarta.persistence.Id; +import jakarta.persistence.Basic; + +@Entity(name = "Customer") +@Table(name = "customers") +public class Customer { + @Id + public Integer id; + @Basic + public String name; + + protected Customer() { + // for Hibernate use + } + + public Customer(Integer id, String name) { + this.id = id; + this.name = name; + } + + public Integer getId() { + return id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } +} \ No newline at end of file diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/cid/idclass/NestedIdClassTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/cid/idclass/NestedIdClassTests.java new file mode 100644 index 0000000000..7b01f49291 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/cid/idclass/NestedIdClassTests.java @@ -0,0 +1,103 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later + * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html + */ +package org.hibernate.orm.test.mapping.cid.idclass; + +import org.hibernate.testing.orm.junit.DomainModel; +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.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Steve Ebersole + */ +@DomainModel( annotatedClasses = { + Customer.class, + Order.class, + Payment.class +}) +@SessionFactory +public class NestedIdClassTests { + @Test + public void smokeTest(SessionFactoryScope scope) { + scope.inTransaction( (session) -> { + final String qry = "from Payment p join fetch p.id.order o join fetch o.id.customer"; + session.createQuery( qry ).list(); + }); + scope.inTransaction( (session) -> { + final String qry = "from Payment p join fetch p.order o join fetch o.customer"; + session.createQuery( qry ).list(); + }); + } + + @Test + public void smokeTest2(SessionFactoryScope scope) { + scope.inTransaction( (session) -> { + final String qry = "from Payment p"; + final Payment payment = session.createQuery( qry, Payment.class ).uniqueResult(); + assertThat( payment ).isNotNull(); + assertThat( payment.accountNumber ).isNotNull(); + assertThat( payment.order ).isNotNull(); + + assertThat( payment.order.orderNumber ).isNotNull(); + assertThat( payment.order.customer ).isNotNull(); + + assertThat( payment.order.customer.id ).isNotNull(); + assertThat( payment.order.customer.name ).isNotNull(); + }); + scope.inTransaction( (session) -> { + final String qry = "from Payment p join fetch p.order o join fetch o.customer"; + session.createQuery( qry ).list(); + }); + } + + @Test + public void smokeTest3(SessionFactoryScope scope) { + scope.inTransaction( (session) -> { + final Payment payment = session.get( Payment.class, new PaymentId( new OrderId( 1, 1 ), "123" ) ); + assertThat( payment ).isNotNull(); + assertThat( payment.accountNumber ).isNotNull(); + assertThat( payment.order ).isNotNull(); + + assertThat( payment.order.orderNumber ).isNotNull(); + assertThat( payment.order.customer ).isNotNull(); + + assertThat( payment.order.customer.id ).isNotNull(); + assertThat( payment.order.customer.name ).isNotNull(); + }); + } + + @BeforeEach + public void createTestData(SessionFactoryScope scope) { + scope.inTransaction( (session) -> { + final Customer acme = new Customer( 1, "acme" ); + final Customer spacely = new Customer( 2, "spacely" ); + session.persist( acme ); + session.persist( spacely ); + + final Order acmeOrder1 = new Order( acme, 1, 123F ); + final Order acmeOrder2 = new Order( acme, 2, 123F ); + session.persist( acmeOrder1 ); + session.persist( acmeOrder2 ); + + final Payment payment = new Payment( acmeOrder1, "123" ); + session.persist( payment ); + } ); + } + + @AfterEach + public void dropTestData(SessionFactoryScope scope) { + scope.inTransaction( (session) -> { + session.createQuery( "delete Payment" ).executeUpdate(); + session.createQuery( "delete Order" ).executeUpdate(); + session.createQuery( "delete Customer" ).executeUpdate(); + } ); + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/cid/idclass/Order.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/cid/idclass/Order.java new file mode 100644 index 0000000000..befb5c5ba7 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/cid/idclass/Order.java @@ -0,0 +1,63 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later + * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html + */ +package org.hibernate.orm.test.mapping.cid.idclass; /** + * @author Steve Ebersole + */ + +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.IdClass; +import jakarta.persistence.ManyToOne; +import jakarta.persistence.Table; + +@Entity(name = "Order") +@Table(name = "orders") +@IdClass( OrderId.class ) +public class Order { + @Id + @ManyToOne + public Customer customer; + + @Id + public Integer orderNumber; + + public Float amount; + + protected Order() { + // for Hibernate use + } + + public Order(Customer customer, Integer orderNumber, Float amount) { + this.customer = customer; + this.orderNumber = orderNumber; + this.amount = amount; + } + + public Customer getCustomer() { + return customer; + } + + public void setCustomer(Customer customer) { + this.customer = customer; + } + + public Integer getOrderNumber() { + return orderNumber; + } + + public void setOrderNumber(Integer orderNumber) { + this.orderNumber = orderNumber; + } + + public Float getAmount() { + return amount; + } + + public void setAmount(Float amount) { + this.amount = amount; + } +} \ No newline at end of file diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/cid/idclass/OrderId.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/cid/idclass/OrderId.java new file mode 100644 index 0000000000..4dab40c5f0 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/cid/idclass/OrderId.java @@ -0,0 +1,23 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later + * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html + */ +package org.hibernate.orm.test.mapping.cid.idclass; + +/** + * @author Steve Ebersole + */ +public class OrderId { + Integer customer; + Integer orderNumber; + + public OrderId() { + } + + public OrderId(Integer customer, Integer orderNumber) { + this.customer = customer; + this.orderNumber = orderNumber; + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/cid/idclass/Payment.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/cid/idclass/Payment.java new file mode 100644 index 0000000000..a1c7daf01b --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/cid/idclass/Payment.java @@ -0,0 +1,53 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later + * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html + */ +package org.hibernate.orm.test.mapping.cid.idclass; /** + * @author Steve Ebersole + */ + +import jakarta.persistence.Basic; +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.IdClass; +import jakarta.persistence.ManyToOne; +import jakarta.persistence.Table; + +@Entity(name = "Payment") +@Table(name = "payments") +@IdClass( PaymentId.class ) +public class Payment { + @Id + @ManyToOne + public Order order; + + @Basic + public String accountNumber; + + protected Payment() { + // for Hibernate use + } + + public Payment(Order order, String accountNumber) { + this.order = order; + this.accountNumber = accountNumber; + } + + public Order getOrder() { + return order; + } + + public void setOrder(Order order) { + this.order = order; + } + + public String getAccountNumber() { + return accountNumber; + } + + public void setAccountNumber(String accountNumber) { + this.accountNumber = accountNumber; + } +} \ No newline at end of file diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/cid/idclass/PaymentId.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/cid/idclass/PaymentId.java new file mode 100644 index 0000000000..28e120987a --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/cid/idclass/PaymentId.java @@ -0,0 +1,23 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later + * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html + */ +package org.hibernate.orm.test.mapping.cid.idclass; + +/** + * @author Steve Ebersole + */ +public class PaymentId { + private OrderId order; + private String accountNumber; + + public PaymentId() { + } + + public PaymentId(OrderId order, String accountNumber) { + this.order = order; + this.accountNumber = accountNumber; + } +}