From 4aa059ace44b83d40fd2ebcfa9b041b19842ac48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=8Cedomir=20Igaly?= Date: Mon, 28 Apr 2025 11:44:53 +0200 Subject: [PATCH 1/2] HHH-19393 Add test for issue --- .../query/ids/RecordIdRelatedIdQueryTest.java | 251 ++++++++++++++++++ 1 file changed, 251 insertions(+) create mode 100644 hibernate-envers/src/test/java/org/hibernate/envers/test/integration/query/ids/RecordIdRelatedIdQueryTest.java diff --git a/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/query/ids/RecordIdRelatedIdQueryTest.java b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/query/ids/RecordIdRelatedIdQueryTest.java new file mode 100644 index 000000000000..bdd76747fee3 --- /dev/null +++ b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/query/ids/RecordIdRelatedIdQueryTest.java @@ -0,0 +1,251 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.envers.test.integration.query.ids; + +import java.io.Serializable; +import java.util.Arrays; +import java.util.List; + +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.IdClass; +import jakarta.persistence.ManyToOne; + +import org.hibernate.annotations.processing.Exclude; +import org.hibernate.envers.Audited; +import org.hibernate.envers.RevisionType; +import org.hibernate.envers.query.AuditEntity; +import org.hibernate.orm.test.envers.BaseEnversJPAFunctionalTestCase; +import org.hibernate.orm.test.envers.Priority; +import org.hibernate.testing.orm.junit.Jira; +import org.junit.Test; + +import org.hibernate.testing.transaction.TransactionUtil; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + +@Exclude +@Jira("https://hibernate.atlassian.net/browse/HHH-19393") +public class RecordIdRelatedIdQueryTest extends BaseEnversJPAFunctionalTestCase { + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { Person.class, Document.class, PersonDocument.class }; + } + + @Test + @Priority(10) + public void initData() { + TransactionUtil.doInJPA( this::entityManagerFactory, entityManager -> { + final Person person = new Person( 1, "Chris" ); + final Document document = new Document( 1, "DL" ); + final PersonDocument pd = new PersonDocument( person, document ); + entityManager.persist( person ); + entityManager.persist( document ); + entityManager.persist( pd ); + } ); + + TransactionUtil.doInJPA( this::entityManagerFactory, entityManager -> { + final Person person = entityManager.find( Person.class, 1 ); + final Document document = new Document( 2, "Passport" ); + final PersonDocument pd = new PersonDocument( person, document ); + entityManager.persist( document ); + entityManager.persist( pd ); + } ); + + TransactionUtil.doInJPA( this::entityManagerFactory, entityManager -> { + final Person person = entityManager.find( Person.class, 1 ); + final Document document = entityManager.find( Document.class, 1 ); + final PersonDocument pd = entityManager + .createQuery( "FROM PersonDocument WHERE person.id = :person AND document.id = :document", PersonDocument.class ) + .setParameter( "person", person.getId() ) + .setParameter( "document", document.getId() ) + .getSingleResult(); + + entityManager.remove( pd ); + entityManager.remove( document ); + } ); + } + + @Test + public void testRevisionCounts() { + assertEquals( Arrays.asList( 1 ), getAuditReader().getRevisions( Person.class, 1 ) ); + assertEquals( Arrays.asList( 1, 3 ), getAuditReader().getRevisions( Document.class, 1 ) ); + assertEquals( Arrays.asList( 2 ), getAuditReader().getRevisions( Document.class, 2 ) ); + } + + @Test + public void testRelatedIdQueries() { + TransactionUtil.doInJPA( this::entityManagerFactory, entityManager -> { + List results = getAuditReader().createQuery().forRevisionsOfEntity( PersonDocument.class, false, true ) + .add( AuditEntity.relatedId( "person" ).eq( 1 ) ) + .add( AuditEntity.revisionNumber().eq( 1 ) ) + .getResultList(); + assertEquals( 1, results.size() ); + final Document document = ( (PersonDocument) ( (Object[]) results.get( 0 ) )[0] ).getDocument(); + assertEquals( "DL", document.getName() ); + } ); + + TransactionUtil.doInJPA( this::entityManagerFactory, entityManager -> { + List results = getAuditReader().createQuery().forRevisionsOfEntity( PersonDocument.class, false, true ) + .add( AuditEntity.relatedId( "person" ).eq( 1 ) ) + .add( AuditEntity.revisionNumber().eq( 2 ) ) + .getResultList(); + assertEquals( 1, results.size() ); + final Document document = ( (PersonDocument) ( (Object[]) results.get( 0 ) )[0] ).getDocument(); + assertEquals( "Passport", document.getName() ); + } ); + + TransactionUtil.doInJPA( this::entityManagerFactory, entityManager -> { + List results = getAuditReader().createQuery().forRevisionsOfEntity( PersonDocument.class, false, true ) + .add( AuditEntity.relatedId( "person" ).eq( 1 ) ) + .add( AuditEntity.revisionNumber().eq( 3 ) ) + .getResultList(); + assertEquals( 1, results.size() ); + final Document document = ( (PersonDocument) ( (Object[]) results.get( 0 ) )[0] ).getDocument(); + assertNull( document.getName() ); + } ); + + TransactionUtil.doInJPA( this::entityManagerFactory, entityManager -> { + List results = getAuditReader().createQuery().forRevisionsOfEntity( PersonDocument.class, false, true ) + .add( AuditEntity.relatedId( "document" ).eq( 1 ) ) + .getResultList(); + assertEquals( 2, results.size() ); + for ( Object result : results ) { + Object[] row = (Object[]) result; + final RevisionType revisionType = (RevisionType) row[2]; + final Document document = ( (PersonDocument) row[0] ).getDocument(); + if ( RevisionType.ADD.equals( revisionType ) ) { + assertEquals( "DL", document.getName() ); + } + else if ( RevisionType.DEL.equals( revisionType ) ) { + assertNull( document.getName() ); + } + } + } ); + + TransactionUtil.doInJPA( this::entityManagerFactory, entityManager -> { + List results = getAuditReader().createQuery().forRevisionsOfEntity( PersonDocument.class, false, true ) + .add( AuditEntity.relatedId( "document" ).eq( 2 ) ) + .getResultList(); + assertEquals( 1, results.size() ); + for ( Object result : results ) { + Object[] row = (Object[]) result; + final RevisionType revisionType = (RevisionType) row[2]; + final Document document = ( (PersonDocument) row[0] ).getDocument(); + assertEquals( RevisionType.ADD, revisionType ); + assertEquals( "Passport", document.getName() ); + } + } ); + } + + @Audited + @Entity(name = "PersonDocument") + @IdClass( PersonDocumentId.class ) + public static class PersonDocument implements Serializable { + @Id + @ManyToOne(optional = false) + private Document document; + + @Id + @ManyToOne(optional = false) + private Person person; + + PersonDocument() { + + } + + PersonDocument(Person person, Document document) { + this.document = document; + this.person = person; + } + + public Document getDocument() { + return document; + } + + public void setDocument(Document document) { + this.document = document; + } + + public Person getPerson() { + return person; + } + + public void setPerson(Person person) { + this.person = person; + } + } + + public record PersonDocumentId(Document document, Person person) { + } + + @Audited + @Entity(name = "Document") + public static class Document { + @Id + private Integer id; + private String name; + + Document() { + + } + + Document(Integer id, String name) { + this.id = id; + this.name = name; + } + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + } + + @Entity(name = "Person") + @Audited + public static class Person { + @Id + private Integer id; + private String name; + + Person() { + + } + + Person(Integer id, String name) { + this.id = id; + this.name = name; + } + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + } +} From 62fd95b036e4bb2f221e08a0deb576e619b3bfd0 Mon Sep 17 00:00:00 2001 From: Marco Belladelli Date: Wed, 2 Apr 2025 16:40:42 +0200 Subject: [PATCH 2/2] HHH-19393 Fix envers mappers for immutable components --- .../mapper/id/AbstractCompositeIdMapper.java | 47 ++++++++++++++----- .../entities/mapper/id/EmbeddedIdMapper.java | 23 +++++---- .../entities/mapper/id/MultipleIdMapper.java | 22 ++++----- 3 files changed, 60 insertions(+), 32 deletions(-) diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/id/AbstractCompositeIdMapper.java b/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/id/AbstractCompositeIdMapper.java index dbcd7b355613..fb6f1540d186 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/id/AbstractCompositeIdMapper.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/id/AbstractCompositeIdMapper.java @@ -9,8 +9,10 @@ import org.hibernate.envers.exception.AuditException; import org.hibernate.envers.internal.entities.PropertyData; import org.hibernate.envers.internal.tools.Tools; -import org.hibernate.internal.util.ReflectHelper; +import org.hibernate.mapping.Component; import org.hibernate.service.ServiceRegistry; +import org.hibernate.type.spi.CompositeTypeImplementor; + /** * An abstract identifier mapper implementation specific for composite identifiers. @@ -20,13 +22,16 @@ * @author Chris Cranford */ public abstract class AbstractCompositeIdMapper extends AbstractIdMapper implements SimpleIdMapperBuilder { - protected final Class compositeIdClass; - + protected final CompositeTypeImplementor compositeType; protected Map ids; - protected AbstractCompositeIdMapper(Class compositeIdClass, ServiceRegistry serviceRegistry) { + protected AbstractCompositeIdMapper(Component component) { + this( component.getServiceRegistry(), (CompositeTypeImplementor) component.getType() ); + } + + protected AbstractCompositeIdMapper(ServiceRegistry serviceRegistry, CompositeTypeImplementor compositeType) { super( serviceRegistry ); - this.compositeIdClass = compositeIdClass; + this.compositeType = compositeType; ids = Tools.newLinkedHashMap(); } @@ -46,24 +51,44 @@ public Object mapToIdFromMap(Map data) { return null; } - final Object compositeId = instantiateCompositeId(); - for ( AbstractIdMapper mapper : ids.values() ) { - if ( !mapper.mapToEntityFromMap( compositeId, data ) ) { - return null; + if ( !compositeType.isMutable() ) { + return mapToImmutableIdFromMap( data ); + } + + final Object compositeId = instantiateCompositeId( null ); + + if ( compositeType.isMutable() ) { + for ( AbstractIdMapper mapper : ids.values() ) { + if ( !mapper.mapToEntityFromMap( compositeId, data ) ) { + return null; + } } } return compositeId; } + protected Object mapToImmutableIdFromMap(Map data) { + final var propertyNames = compositeType.getPropertyNames(); + final var values = new Object[propertyNames.length]; + for ( int i = 0; i < propertyNames.length; i++ ) { + values[i] = data.get( propertyNames[i] ); + } + return instantiateCompositeId( values ); + } + @Override public void mapToEntityFromEntity(Object objectTo, Object objectFrom) { // no-op; does nothing } - protected Object instantiateCompositeId() { + protected Object instantiateCompositeId(Object[] values) { try { - return ReflectHelper.getDefaultConstructor( compositeIdClass ).newInstance(); + return compositeType.getMappingModelPart() + .getEmbeddableTypeDescriptor() + .getRepresentationStrategy() + .getInstantiator() + .instantiate( () -> values ); } catch ( Exception e ) { throw new AuditException( e ); diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/id/EmbeddedIdMapper.java b/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/id/EmbeddedIdMapper.java index f4968688bf95..4819a71732a6 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/id/EmbeddedIdMapper.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/id/EmbeddedIdMapper.java @@ -15,6 +15,7 @@ import org.hibernate.mapping.Component; import org.hibernate.property.access.spi.Setter; import org.hibernate.service.ServiceRegistry; +import org.hibernate.type.spi.CompositeTypeImplementor; /** * An identifier mapper implementation for {@link jakarta.persistence.EmbeddedId} mappings. @@ -26,12 +27,12 @@ public class EmbeddedIdMapper extends AbstractCompositeIdMapper implements Simpl private PropertyData idPropertyData; public EmbeddedIdMapper(PropertyData propertyData, Component component) { - super( component.getComponentClass(), component.getServiceRegistry() ); + super( component ); this.idPropertyData = propertyData; } - private EmbeddedIdMapper(PropertyData idPropertyData, Class compositeIdClass, ServiceRegistry serviceRegistry) { - super( compositeIdClass, serviceRegistry ); + private EmbeddedIdMapper(PropertyData idPropertyData, CompositeTypeImplementor compositeType, ServiceRegistry serviceRegistry) { + super( serviceRegistry, compositeType ); this.idPropertyData = idPropertyData; } @@ -58,11 +59,17 @@ public boolean mapToEntityFromMap(final Object obj, final Map data) { final Setter setter = ReflectionTools.getSetter( obj.getClass(), idPropertyData, getServiceRegistry() ); try { - final Object subObj = instantiateCompositeId(); - + final Object subObj; boolean ret = true; - for ( IdMapper idMapper : ids.values() ) { - ret &= idMapper.mapToEntityFromMap( subObj, data ); + if ( compositeType.isMutable() ) { + subObj = instantiateCompositeId( null ); + for ( IdMapper idMapper : ids.values() ) { + ret &= idMapper.mapToEntityFromMap( subObj, data ); + } + } + else { + subObj = mapToImmutableIdFromMap( data ); + ret = subObj != null; } if ( ret ) { @@ -78,7 +85,7 @@ public boolean mapToEntityFromMap(final Object obj, final Map data) { @Override public IdMapper prefixMappedProperties(String prefix) { - final EmbeddedIdMapper ret = new EmbeddedIdMapper( idPropertyData, compositeIdClass, getServiceRegistry() ); + final EmbeddedIdMapper ret = new EmbeddedIdMapper( idPropertyData, compositeType, getServiceRegistry() ); for ( PropertyData propertyData : ids.keySet() ) { final String propertyName = propertyData.getName(); diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/id/MultipleIdMapper.java b/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/id/MultipleIdMapper.java index 93174d888753..89a9a2f7de6a 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/id/MultipleIdMapper.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/id/MultipleIdMapper.java @@ -10,6 +10,7 @@ import org.hibernate.proxy.HibernateProxy; import org.hibernate.proxy.LazyInitializer; import org.hibernate.service.ServiceRegistry; +import org.hibernate.type.spi.CompositeTypeImplementor; import java.util.ArrayList; import java.util.LinkedHashMap; @@ -24,17 +25,12 @@ * @author Chris Cranford */ public class MultipleIdMapper extends AbstractCompositeIdMapper implements SimpleIdMapperBuilder { - - private final boolean embedded; - public MultipleIdMapper(Component component) { - super( component.getComponentClass(), component.getServiceRegistry() ); - this.embedded = component.isEmbedded(); + super( component ); } - private MultipleIdMapper(boolean embedded, Class compositeIdClass, ServiceRegistry serviceRegistry) { - super( compositeIdClass, serviceRegistry ); - this.embedded = embedded; + private MultipleIdMapper(CompositeTypeImplementor compositeType, ServiceRegistry serviceRegistry) { + super( serviceRegistry, compositeType ); } @Override @@ -44,8 +40,8 @@ public void add(PropertyData propertyData) { @Override public void mapToMapFromId(SharedSessionContractImplementor session, Map data, Object obj) { - if ( compositeIdClass.isInstance( obj ) ) { - if ( embedded ) { + if ( compositeType.getReturnedClass().isInstance( obj ) ) { + if ( compositeType.isEmbedded() ) { final LazyInitializer lazyInitializer = HibernateProxy.extractLazyInitializer( obj ); if ( lazyInitializer != null ) { obj = lazyInitializer.getInternalIdentifier(); @@ -77,7 +73,7 @@ public void mapToMapFromId(Map data, Object obj) { @Override public void mapToMapFromEntity(Map data, Object obj) { - if ( embedded ) { + if ( compositeType.isEmbedded() ) { final LazyInitializer lazyInitializer = HibernateProxy.extractLazyInitializer( obj ); if ( lazyInitializer != null ) { obj = lazyInitializer.getInternalIdentifier(); @@ -100,7 +96,7 @@ public boolean mapToEntityFromMap(Object obj, Map data) { @Override public IdMapper prefixMappedProperties(String prefix) { - final MultipleIdMapper ret = new MultipleIdMapper( embedded, compositeIdClass, getServiceRegistry() ); + final MultipleIdMapper ret = new MultipleIdMapper( compositeType, getServiceRegistry() ); for ( PropertyData propertyData : ids.keySet() ) { final String propertyName = propertyData.getName(); @@ -116,7 +112,7 @@ public Object mapToIdFromEntity(Object data) { return null; } - final Object compositeId = instantiateCompositeId(); + final Object compositeId = instantiateCompositeId( null ); for ( AbstractIdMapper mapper : ids.values() ) { mapper.mapToEntityFromEntity( compositeId, data ); }