This fix addresses partially-generated composite id failure introduced with HHH-4848. The pull request is at https://github.com/hibernate/hibernate-orm/pull/3368 . Please review and can this be backported to 5.2? We have cherry picked it and applied it to 5.2.18.Final on JDK8 with SQLServer2012Dialect and are tracking its performance (our bug 2092). HHH-9662 will continue to need some work for SQL Server, but otherwise seems to work nicely. I have opened HHH-13970 as a sub-task for it so it can be treated as a known issue. The unit tests for this issue have some issues in the hibernate provided unit testing framework and I have opened a ticket (HHH-13971) for those issues. It is likely a very low priority item. v/r, Jason Pyeron $ git logg -5 * 4138772878 2020-04-22 (HEAD -> HHH-10956) HHH-10956 put one line if in {} jpyeron(a)pdinc.us * 683e00a399 2020-04-22 HHH-10956 added more complext tests with self referential FK jpyeron(a)pdinc.us * 9653fee5b7 2020-04-22 HHH-10956 fixed failed insertion with IdClass with partial identifier generation jpyeron(a)pdinc.us * eca5c5a884 2020-04-22 HHH-10956 created test cases IdClass with partial identifier generatiod, all marked @FailureExpected jpyeron(a)pdinc.us * 5104c4b7f3 2020-03-26 (tag: 5.4.13) 5.4.13 gbadner(a)redhat.com diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java b/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java index 8844440831..d9f8b65ec9 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java +++ b/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java @@ -264,6 +264,17 @@ public class SimpleValue implements KeyValue { private IdentifierGenerator identifierGenerator; + /** + * Returns the cached identifierGenerator. + * + * @return IdentifierGenerator null if + * {@link #createIdentifierGenerator(IdentifierGeneratorFactory, Dialect, String, String, RootClass)} was never + * completed. + */ + public IdentifierGenerator getIdentifierGenerator() { + return identifierGenerator; + } + @Override public IdentifierGenerator createIdentifierGenerator( IdentifierGeneratorFactory identifierGeneratorFactory, diff --git a/hibernate-core/src/main/java/org/hibernate/tuple/entity/AbstractEntityTuplizer.java b/hibernate-core/src/main/java/org/hibernate/tuple/entity/AbstractEntityTuplizer.java index 3d137219d0..53fa2d9d12 100644 --- a/hibernate-core/src/main/java/org/hibernate/tuple/entity/AbstractEntityTuplizer.java +++ b/hibernate-core/src/main/java/org/hibernate/tuple/entity/AbstractEntityTuplizer.java @@ -26,8 +26,10 @@ import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.id.Assigned; import org.hibernate.loader.PropertyPath; import org.hibernate.mapping.Component; +import org.hibernate.mapping.KeyValue; import org.hibernate.mapping.PersistentClass; import org.hibernate.mapping.Property; +import org.hibernate.mapping.SimpleValue; import org.hibernate.metamodel.spi.MetamodelImplementor; import org.hibernate.persister.entity.EntityPersister; import org.hibernate.property.access.spi.Getter; @@ -168,11 +170,13 @@ public abstract class AbstractEntityTuplizer implements EntityTuplizer { } else { identifierMapperType = (CompositeType) mapper.getType(); + KeyValue identifier = mappingInfo.getIdentifier(); mappedIdentifierValueMarshaller = buildMappedIdentifierValueMarshaller( getEntityName(), getFactory(), (ComponentType) entityMetamodel.getIdentifierProperty().getType(), - (ComponentType) identifierMapperType + (ComponentType) identifierMapperType, + identifier ); } } @@ -274,7 +278,8 @@ public abstract class AbstractEntityTuplizer implements EntityTuplizer { String entityName, SessionFactoryImplementor sessionFactory, ComponentType mappedIdClassComponentType, - ComponentType virtualIdComponent) { + ComponentType virtualIdComponent, + KeyValue identifier) { // so basically at this point we know we have a "mapped" composite identifier // which is an awful way to say that the identifier is represented differently // in the entity and in the identifier value. The incoming value should @@ -302,7 +307,8 @@ public abstract class AbstractEntityTuplizer implements EntityTuplizer { entityName, sessionFactory, virtualIdComponent, - mappedIdClassComponentType + mappedIdClassComponentType, + identifier ); } @@ -341,28 +347,42 @@ public abstract class AbstractEntityTuplizer implements EntityTuplizer { private final SessionFactoryImplementor sessionFactory; private final ComponentType virtualIdComponent; private final ComponentType mappedIdentifierType; + private final KeyValue identifier; private IncrediblySillyJpaMapsIdMappedIdentifierValueMarshaller( String entityName, SessionFactoryImplementor sessionFactory, ComponentType virtualIdComponent, - ComponentType mappedIdentifierType) { + ComponentType mappedIdentifierType, + KeyValue identifier) { this.sessionFactory = sessionFactory; this.entityName = entityName; this.virtualIdComponent = virtualIdComponent; this.mappedIdentifierType = mappedIdentifierType; + this.identifier = identifier; } @Override public Object getIdentifier(Object entity, EntityMode entityMode, SharedSessionContractImplementor session) { final Object id = mappedIdentifierType.instantiate( entityMode ); final Object[] propertyValues = virtualIdComponent.getPropertyValues( entity, entityMode ); + final String[] names = virtualIdComponent.getPropertyNames(); final Type[] subTypes = virtualIdComponent.getSubtypes(); final Type[] copierSubTypes = mappedIdentifierType.getSubtypes(); final int length = subTypes.length; for ( int i = 0; i < length; i++ ) { if ( propertyValues[i] == null ) { - throw new HibernateException( "No part of a composite identifier may be null" ); + try { + final String name = names[i]; + final Property p = ((Component) identifier).getProperty(name); + final SimpleValue v = (SimpleValue) p.getValue(); + if ( v.getIdentifierGenerator() == null ) { + throw new NullPointerException("No IdentifierGenerator found for property "+name); + } + } + catch (Throwable t) { + throw new HibernateException( "No part of a composite identifier may be null", t ); + } } //JPA 2 @MapsId + @IdClass points to the pk of the entity if ( subTypes[i].isAssociationType() && !copierSubTypes[i].isAssociationType() ) { diff --git a/hibernate-core/src/test/java/org/hibernate/test/annotations/cid/CompositeIdFkGeneratedValueIdentityTest.java b/hibernate-core/src/test/java/org/hibernate/test/annotations/cid/CompositeIdFkGeneratedValueIdentityTest.java new file mode 100644 index 0000000000..3ec3ffcb94 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/annotations/cid/CompositeIdFkGeneratedValueIdentityTest.java @@ -0,0 +1,676 @@ +/* + * 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.test.annotations.cid; + +import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate; + +import java.io.Serializable; +import java.util.Objects; + +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.IdClass; +import javax.persistence.ManyToOne; + +import org.hibernate.dialect.Dialect; +import org.hibernate.dialect.SQLServer2012Dialect; +import org.hibernate.testing.FailureExpected; +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; +import org.junit.Ignore; +import org.junit.Test; + +/** + * This tests the design demonstrated in the <a href= + * 'https://docs.jboss.org/hibernate/orm/5.4/userguide/html_single/Hibernate_User_Guide.html#identifiers-composite-nonaggregated'>user + * guide</a>, example "&#64;{@link IdClass} with partial identifier generation using &#64;{@link GeneratedValue}". The + * getters and setters have been omitted for clarity of the code. A separate test has been made for + * {@link GenerationType#SEQUENCE}, {@link GenerationType#IDENTITY}, {@link GenerationType#TABLE}, and + * {@link GenerationType#AUTO} since there are known complications with some {@link Dialect}s (e.g. + * {@link SQLServer2012Dialect}) and the {@link GenerationType#IDENTITY} + * + * @author Jason Pyeron <support(a)pdinc.us&gt; + * @see <a href='https://hibernate.atlassian.net/browse/HHH-10956'>HHH-10... Persisting partially-generated + * composite Ids fails + * @see <a href='https://hibernate.atlassian.net/browse/HHH-9662'>HHH-966... a related and blocking bug for + * {@link GenerationType#IDENTITY} + * @see <a href='https://hibernate.atlassian.net/browse/HHH-4848'>HHH-484... introduced the regression + */ +@TestForIssue(jiraKey = "HHH-10956") +public class CompositeIdFkGeneratedValueIdentityTest extends BaseCoreFunctionalTestCase { + + @Test + public void testCompositePkWithIdentityAndFKBySequence() throws Exception { + doInHibernate( this::sessionFactory, session -> { + HeadS head = new HeadS(); + head.name = "Head by Sequence"; + session.persist( head ); + System.out.println( "VALUE =>" + head.name + "=" + head.hid ); + + NodeS node = new NodeS(); + node.hid = head; + node.name = "Node by Sequence"; + session.persist( node ); + System.out.println( "VALUE =>" + node.name + "=" + node.nid + ":" + node.hid.hid ); + } ); + } + + @Test + @FailureExpected(jiraKey = "HHH-9662", message = "Could not set field value [POST_INSERT_INDICATOR]") + public void testCompositePkWithIdentityAndFKByIdentity() throws Exception { + doInHibernate( this::sessionFactory, session -> { + HeadI head = new HeadI(); + head.name = "Head by Identity"; + session.persist( head ); + System.out.println( "VALUE =>" + head.name + "=" + head.hid ); + + NodeI node = new NodeI(); + node.hid = head; + node.name = "Node by Identity"; + try { + session.persist( node ); + } + catch (Error | RuntimeException e) { + // expected failure... + e.printStackTrace( System.out ); + throw e; + } + System.out.println( "VALUE =>" + node.name + "=" + node.nid + ":" + node.hid.hid ); + } ); + } + + @Test + public void testCompositePkWithIdentityAndFKByTable() throws Exception { + doInHibernate( this::sessionFactory, session -> { + HeadT head = new HeadT(); + head.name = "Head by Table"; + session.persist( head ); + System.out.println( "VALUE =>" + head.name + "=" + head.hid ); + + NodeT node = new NodeT(); + node.hid = head; + node.name = "Node by Table"; + session.persist( node ); + System.out.println( "VALUE =>" + node.name + "=" + node.nid + ":" + node.hid.hid ); + } ); + } + + @Test + public void testCompositePkWithIdentityAndFKByAuto() throws Exception { + doInHibernate( this::sessionFactory, session -> { + HeadA head = new HeadA(); + head.name = "Head by Auto"; + session.persist( head ); + System.out.println( "VALUE =>" + head.name + "=" + head.hid ); + + NodeA node = new NodeA(); + node.hid = head; + node.name = "Node by Auto"; + session.persist( node ); + System.out.println( "VALUE =>" + node.name + "=" + node.nid + ":" + node.hid.hid ); + } ); + } + + @Test + public void testCompositePkWithIdentityAndFKBySequence2() throws Exception { + doInHibernate( this::sessionFactory, session -> { + HeadS head = new HeadS(); + head.name = "Head by Sequence"; + session.persist( head ); + System.out.println( "VALUE =>" + head.name + "=" + head.hid ); + + ComplexNodeS node = new ComplexNodeS(); + node.hid = head; + node.name = "Node by Sequence"; + session.persist( node ); + System.out.println( "VALUE =>" + node.name + "=" + node.nid + ":" + node.hid.hid + " with parent=" + + ( node.parent == null ? null : node.parent.nid + ":" + node.parent.hid.hid ) ); + + ComplexNodeS node2 = new ComplexNodeS(); + node2.hid = head; + node2.name = "Node 2 by Sequence"; + node2.parent = node; + session.persist( node2 ); + System.out.println( "VALUE =>" + node2.name + "=" + node2.nid + ":" + node2.hid.hid + " with parent=" + + ( node2.parent == null ? null : node2.parent.nid + ":" + node2.parent.hid.hid ) ); + } ); + } + + @Test + @Ignore("HHH-13971 - unit test DB is using call next value for hibernate_sequence when it should be Identity insert") + @FailureExpected(jiraKey = "HHH-9662", message = "Could not set field value [POST_INSERT_INDICATOR]") + public void testCompositePkWithIdentityAndFKByIdentity2() throws Exception { + doInHibernate( this::sessionFactory, session -> { + HeadI head = new HeadI(); + head.name = "Head by Identity"; + session.persist( head ); + System.out.println( "VALUE =>" + head.name + "=" + head.hid ); + + ComplexNodeI node = new ComplexNodeI(); + node.hid = head; + node.name = "Node by Identity"; + session.persist( node ); + System.out.println( "VALUE =>" + node.name + "=" + node.nid + ":" + node.hid.hid + " with parent=" + + ( node.parent == null ? null : node.parent.nid + ":" + node.parent.hid.hid ) ); + + ComplexNodeI node2 = new ComplexNodeI(); + node2.hid = head; + node2.name = "Node 2 by Identity"; + node2.parent = node; + session.persist( node2 ); + System.out.println( "VALUE =>" + node2.name + "=" + node2.nid + ":" + node2.hid.hid + " with parent=" + + ( node2.parent == null ? null : node2.parent.nid + ":" + node2.parent.hid.hid ) ); + } ); + } + + @Test + public void testCompositePkWithIdentityAndFKByTable2() throws Exception { + doInHibernate( this::sessionFactory, session -> { + HeadT head = new HeadT(); + head.name = "Head by Table"; + session.persist( head ); + System.out.println( "VALUE =>" + head.name + "=" + head.hid ); + + ComplexNodeT node = new ComplexNodeT(); + node.hid = head; + node.name = "Node by Table"; + session.persist( node ); + System.out.println( "VALUE =>" + node.name + "=" + node.nid + ":" + node.hid.hid + " with parent=" + + ( node.parent == null ? null : node.parent.nid + ":" + node.parent.hid.hid ) ); + + ComplexNodeT node2 = new ComplexNodeT(); + node2.hid = head; + node2.name = "Node 2 by Table"; + node2.parent = node; + session.persist( node2 ); + System.out.println( "VALUE =>" + node2.name + "=" + node2.nid + ":" + node2.hid.hid + " with parent=" + + ( node2.parent == null ? null : node2.parent.nid + ":" + node2.parent.hid.hid ) ); + } ); + } + + @Test + public void testCompositePkWithIdentityAndFKByAuto2() throws Exception { + doInHibernate( this::sessionFactory, session -> { + HeadA head = new HeadA(); + head.name = "Head by Auto"; + session.persist( head ); + System.out.println( "VALUE =>" + head.name + "=" + head.hid ); + + ComplexNodeA node = new ComplexNodeA(); + node.hid = head; + node.name = "Node by Auto"; + session.persist( node ); + System.out.println( "VALUE =>" + node.name + "=" + node.nid + ":" + node.hid.hid + " with parent=" + + ( node.parent == null ? null : node.parent.nid + ":" + node.parent.hid.hid ) ); + + ComplexNodeA node2 = new ComplexNodeA(); + node2.hid = head; + node2.name = "Node 2 by Auto"; + node2.parent = node; + session.persist( node2 ); + System.out.println( "VALUE =>" + node2.name + "=" + node2.nid + ":" + node2.hid.hid + " with parent=" + + ( node2.parent == null ? null : node2.parent.nid + ":" + node2.parent.hid.hid ) ); + } ); + } + + @Override + protected Class<?>[] getAnnotatedClasses() { + return new Class[]{ + HeadS.class, + NodeS.class, + HeadA.class, + NodeA.class, + HeadI.class, + NodeI.class, + HeadT.class, + NodeT.class, + ComplexNodeS.class, + ComplexNodeI.class, + ComplexNodeT.class, + ComplexNodeA.class, + }; + } + + @Entity + public static class HeadS { + + @Id + @GeneratedValue(strategy = GenerationType.SEQUENCE) + private Long hid; + + private String name; + } + + @Entity + @IdClass(CompositeIdFkGeneratedValueIdentityTest.NodeS.PK.class) + public static class NodeS { + + @Id + @GeneratedValue(strategy = GenerationType.SEQUENCE) + private Long nid; + + @Id + @ManyToOne + private HeadS hid; + + private String name; + + public static class PK implements Serializable { + + private Long nid; + + private Long hid; + + public PK(Long nid, Long hid) { + this.nid = nid; + this.hid = hid; + } + + private PK() { + } + + @Override + public boolean equals(Object o) { + if ( this == o ) { + return true; + } + if ( o == null || getClass() != o.getClass() ) { + return false; + } + PK pk = (PK) o; + return Objects.equals( nid, pk.nid ) && Objects.equals( hid, pk.hid ); + } + + @Override + public int hashCode() { + return Objects.hash( nid, hid ); + } + } + + } + + @Entity + public static class HeadI { + + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + private Long hid; + + private String name; + } + + @Entity + @IdClass(CompositeIdFkGeneratedValueIdentityTest.NodeI.PK.class) + public static class NodeI { + + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + private Long nid; + + @Id + @ManyToOne + private HeadI hid; + + private String name; + + public static class PK implements Serializable { + + private Long nid; + + private Long hid; + + public PK(Long nid, Long hid) { + this.nid = nid; + this.hid = hid; + } + + private PK() { + } + + @Override + public boolean equals(Object o) { + if ( this == o ) { + return true; + } + if ( o == null || getClass() != o.getClass() ) { + return false; + } + PK pk = (PK) o; + return Objects.equals( nid, pk.nid ) && Objects.equals( hid, pk.hid ); + } + + @Override + public int hashCode() { + return Objects.hash( nid, hid ); + } + } + + } + + @Entity + public static class HeadA { + + @Id + @GeneratedValue(strategy = GenerationType.AUTO) + private Long hid; + + private String name; + } + + @Entity + @IdClass(CompositeIdFkGeneratedValueIdentityTest.NodeA.PK.class) + public static class NodeA { + + @Id + @GeneratedValue(strategy = GenerationType.AUTO) + private Long nid; + + @Id + @ManyToOne + private HeadA hid; + + private String name; + + public static class PK implements Serializable { + + private Long nid; + + private Long hid; + + public PK(Long nid, Long hid) { + this.nid = nid; + this.hid = hid; + } + + private PK() { + } + + @Override + public boolean equals(Object o) { + if ( this == o ) { + return true; + } + if ( o == null || getClass() != o.getClass() ) { + return false; + } + PK pk = (PK) o; + return Objects.equals( nid, pk.nid ) && Objects.equals( hid, pk.hid ); + } + + @Override + public int hashCode() { + return Objects.hash( nid, hid ); + } + } + + } + + @Entity + public static class HeadT { + + @Id + @GeneratedValue(strategy = GenerationType.TABLE) + private Long hid; + + private String name; + } + + @Entity + @IdClass(CompositeIdFkGeneratedValueIdentityTest.NodeT.PK.class) + public static class NodeT { + + @Id + @GeneratedValue(strategy = GenerationType.TABLE) + private Long nid; + + @Id + @ManyToOne + private HeadT hid; + + private String name; + + public static class PK implements Serializable { + + private Long nid; + + private Long hid; + + public PK(Long nid, Long hid) { + this.nid = nid; + this.hid = hid; + } + + private PK() { + } + + @Override + public boolean equals(Object o) { + if ( this == o ) { + return true; + } + if ( o == null || getClass() != o.getClass() ) { + return false; + } + PK pk = (PK) o; + return Objects.equals( nid, pk.nid ) && Objects.equals( hid, pk.hid ); + } + + @Override + public int hashCode() { + return Objects.hash( nid, hid ); + } + } + + } + + @Entity + @IdClass(CompositeIdFkGeneratedValueIdentityTest.ComplexNodeS.PK.class) + public static class ComplexNodeS { + + @Id + @GeneratedValue(strategy = GenerationType.SEQUENCE) + private Long nid; + + @Id + @ManyToOne + private HeadS hid; + + @ManyToOne + private ComplexNodeS parent; + + private String name; + + public static class PK implements Serializable { + + private Long nid; + + private Long hid; + + public PK(Long nid, Long hid) { + this.nid = nid; + this.hid = hid; + } + + private PK() { + } + + @Override + public boolean equals(Object o) { + if ( this == o ) { + return true; + } + if ( o == null || getClass() != o.getClass() ) { + return false; + } + PK pk = (PK) o; + return Objects.equals( nid, pk.nid ) && Objects.equals( hid, pk.hid ); + } + + @Override + public int hashCode() { + return Objects.hash( nid, hid ); + } + } + + } + + @Entity + @IdClass(CompositeIdFkGeneratedValueIdentityTest.ComplexNodeI.PK.class) + public static class ComplexNodeI { + + @Id + @GeneratedValue(strategy = GenerationType.SEQUENCE) + private Long nid; + + @Id + @ManyToOne + private HeadI hid; + + @ManyToOne + private ComplexNodeI parent; + + private String name; + + public static class PK implements Serializable { + + private Long nid; + + private Long hid; + + public PK(Long nid, Long hid) { + this.nid = nid; + this.hid = hid; + } + + private PK() { + } + + @Override + public boolean equals(Object o) { + if ( this == o ) { + return true; + } + if ( o == null || getClass() != o.getClass() ) { + return false; + } + PK pk = (PK) o; + return Objects.equals( nid, pk.nid ) && Objects.equals( hid, pk.hid ); + } + + @Override + public int hashCode() { + return Objects.hash( nid, hid ); + } + } + + } + + @Entity + @IdClass(CompositeIdFkGeneratedValueIdentityTest.ComplexNodeT.PK.class) + public static class ComplexNodeT { + + @Id + @GeneratedValue(strategy = GenerationType.SEQUENCE) + private Long nid; + + @Id + @ManyToOne + private HeadT hid; + + @ManyToOne + private ComplexNodeT parent; + + private String name; + + public static class PK implements Serializable { + + private Long nid; + + private Long hid; + + public PK(Long nid, Long hid) { + this.nid = nid; + this.hid = hid; + } + + private PK() { + } + + @Override + public boolean equals(Object o) { + if ( this == o ) { + return true; + } + if ( o == null || getClass() != o.getClass() ) { + return false; + } + PK pk = (PK) o; + return Objects.equals( nid, pk.nid ) && Objects.equals( hid, pk.hid ); + } + + @Override + public int hashCode() { + return Objects.hash( nid, hid ); + } + } + + } + + @Entity + @IdClass(CompositeIdFkGeneratedValueIdentityTest.ComplexNodeA.PK.class) + public static class ComplexNodeA { + + @Id + @GeneratedValue(strategy = GenerationType.SEQUENCE) + private Long nid; + + @Id + @ManyToOne + private HeadA hid; + + @ManyToOne + private ComplexNodeA parent; + + private String name; + + public static class PK implements Serializable { + + private Long nid; + + private Long hid; + + public PK(Long nid, Long hid) { + this.nid = nid; + this.hid = hid; + } + + private PK() { + } + + @Override + public boolean equals(Object o) { + if ( this == o ) { + return true; + } + if ( o == null || getClass() != o.getClass() ) { + return false; + } + PK pk = (PK) o; + return Objects.equals( nid, pk.nid ) && Objects.equals( hid, pk.hid ); + } + + @Override + public int hashCode() { + return Objects.hash( nid, hid ); + } + } + + } + +} -- Jason Pyeron | Architect Contractor | PD Inc | 10 w 24th St | Baltimore, MD | .mil: <mailto:jason.j.pyeron.ctr@mail.mil> jason.j.pyeron.ctr(a)mail.mil .com: <mailto:jpyeron@pdinc.us> jpyeron(a)pdinc.us tel : 202-741-9397