From 52ab4baafb0daea8bc3f5aaa83201b07ee81ef75 Mon Sep 17 00:00:00 2001 From: Adar Dembo Date: Tue, 26 Mar 2013 16:37:00 -0700 Subject: [PATCH] HHH-5845 (partial fix) Lazy Loading of audited entites with revision type 'delete' Fixed for to-one associations, but not for one-to-many and other collection types. --- .../org/hibernate/envers/AuditReader.java | 21 ++++- .../envers/entities/EntityInstantiator.java | 11 ++- .../mapper/relation/ToOneEntityLoader.java | 16 ++-- .../mapper/relation/ToOneIdMapper.java | 3 +- .../lazy/ToOneDelegateSessionImplementor.java | 6 +- .../envers/query/AuditQueryCreator.java | 19 +++++ .../query/impl/EntitiesAtRevisionQuery.java | 19 ++++- .../envers/reader/AuditReaderImpl.java | 12 ++- .../proxy/RemovedObjectQueryTest.java | 83 +++++++++++++++++++ 9 files changed, 170 insertions(+), 20 deletions(-) create mode 100644 hibernate-envers/src/test/java/org/hibernate/envers/test/integration/proxy/RemovedObjectQueryTest.java diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/AuditReader.java b/hibernate-envers/src/main/java/org/hibernate/envers/AuditReader.java index 6ed990035a..94c61bc5b1 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/AuditReader.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/AuditReader.java @@ -71,7 +71,26 @@ public interface AuditReader { T find(Class cls, String entityName, Object primaryKey, Number revision) throws IllegalArgumentException, NotAuditedException, IllegalStateException; - + + /** + * Find an entity by primary key at the given revision with the specified entityName, + * possibly including deleted entities in the search. + * @param cls Class of the entity. + * @param entityName Name of the entity (if can't be guessed basing on the {@code cls}). + * @param primaryKey Primary key of the entity. + * @param revision Revision in which to get the entity. + * @param includeDeletions Whether to include deleted entities in the search. + * @return The found entity instance at the given revision (its properties may be partially filled + * if not all properties are audited) or null, if an entity with that id didn't exist at that + * revision. + * @throws IllegalArgumentException If cls or primaryKey is null or revision is less or equal to 0. + * @throws NotAuditedException When entities of the given class are not audited. + * @throws IllegalStateException If the associated entity manager is closed. + */ + T find(Class cls, String entityName, Object primaryKey, + Number revision, boolean includeDeletions) throws IllegalArgumentException, + NotAuditedException, IllegalStateException; + /** * Get a list of revision numbers, at which an entity was modified. * @param cls Class of the entity. diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/entities/EntityInstantiator.java b/hibernate-envers/src/main/java/org/hibernate/envers/entities/EntityInstantiator.java index b8758b9b74..39f8f414ca 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/entities/EntityInstantiator.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/entities/EntityInstantiator.java @@ -23,6 +23,7 @@ */ package org.hibernate.envers.entities; +import org.hibernate.envers.RevisionType; import org.hibernate.envers.configuration.AuditConfiguration; import org.hibernate.envers.entities.mapper.id.IdMapper; import org.hibernate.envers.entities.mapper.relation.lazy.ToOneDelegateSessionImplementor; @@ -77,7 +78,7 @@ public class EntityInstantiator { // Fixes HHH-4751 issue (@IdClass with @ManyToOne relation mapping inside) // Note that identifiers are always audited // Replace identifier proxies if do not point to audit tables - replaceNonAuditIdProxies(originalId, revision); + replaceNonAuditIdProxies(versionsEntity, revision); Object primaryKey = idMapper.mapToIdFromMap(originalId); @@ -116,7 +117,8 @@ public class EntityInstantiator { } @SuppressWarnings({"unchecked"}) - private void replaceNonAuditIdProxies(Map originalId, Number revision) { + private void replaceNonAuditIdProxies(Map versionsEntity, Number revision) { + final Map originalId = (Map) versionsEntity.get( verCfg.getAuditEntCfg().getOriginalIdPropName() ); for (Object key : originalId.keySet()) { Object value = originalId.get(key); if (value instanceof HibernateProxy) { @@ -133,7 +135,10 @@ public class EntityInstantiator { catch ( ClassNotFoundException e ) { throw new AuditException( e ); } - final ToOneDelegateSessionImplementor delegate = new ToOneDelegateSessionImplementor(versionsReader, entityClass, entityId, revision, verCfg); + final ToOneDelegateSessionImplementor delegate = new ToOneDelegateSessionImplementor( + versionsReader, entityClass, entityId, revision, + RevisionType.DEL.equals( versionsEntity.get( verCfg.getAuditEntCfg().getRevisionTypePropName() ) ), + verCfg); originalId.put(key, versionsReader.getSessionImplementor().getFactory().getEntityPersister(entityName).createProxy(entityId, delegate)); } diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/ToOneEntityLoader.java b/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/ToOneEntityLoader.java index c59e3cc05d..8c4c33fd91 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/ToOneEntityLoader.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/ToOneEntityLoader.java @@ -15,10 +15,12 @@ public class ToOneEntityLoader { * Immediately loads historical entity or its current state when excluded from audit process. */ public static Object loadImmediate(AuditReaderImplementor versionsReader, Class entityClass, String entityName, - Object entityId, Number revision, AuditConfiguration verCfg) { + Object entityId, Number revision, boolean removed, AuditConfiguration verCfg) { if ( verCfg.getEntCfg().getNotVersionEntityConfiguration( entityName ) == null ) { // Audited relation, look up entity with Envers. - return versionsReader.find( entityClass, entityName, entityId, revision ); + // When user traverses removed entities graph, do not restrict revision type of referencing objects + // to ADD or MOD (DEL possible). See HHH-5845. + return versionsReader.find( entityClass, entityName, entityId, revision, removed); } else { // Not audited relation, look up entity with Hibernate. @@ -30,11 +32,11 @@ public class ToOneEntityLoader { * Creates proxy of referenced *-to-one entity. */ public static Object createProxy(AuditReaderImplementor versionsReader, Class entityClass, String entityName, - Object entityId, Number revision, AuditConfiguration verCfg) { + Object entityId, Number revision, boolean removed, AuditConfiguration verCfg) { EntityPersister persister = versionsReader.getSessionImplementor().getFactory().getEntityPersister( entityName ); return persister.createProxy( (Serializable) entityId, - new ToOneDelegateSessionImplementor( versionsReader, entityClass, entityId, revision, verCfg ) + new ToOneDelegateSessionImplementor( versionsReader, entityClass, entityId, revision, removed, verCfg ) ); } @@ -43,11 +45,11 @@ public class ToOneEntityLoader { * allowed (e.g. @Proxy(lazy=false), final class). */ public static Object createProxyOrLoadImmediate(AuditReaderImplementor versionsReader, Class entityClass, String entityName, - Object entityId, Number revision, AuditConfiguration verCfg) { + Object entityId, Number revision, boolean removed, AuditConfiguration verCfg) { EntityPersister persister = versionsReader.getSessionImplementor().getFactory().getEntityPersister( entityName ); if ( persister.hasProxy() ) { - return createProxy( versionsReader, entityClass, entityName, entityId, revision, verCfg ); + return createProxy( versionsReader, entityClass, entityName, entityId, revision, removed, verCfg ); } - return loadImmediate( versionsReader, entityClass, entityName, entityId, revision, verCfg ); + return loadImmediate( versionsReader, entityClass, entityName, entityId, revision, removed, verCfg ); } } diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/ToOneIdMapper.java b/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/ToOneIdMapper.java index d322340638..1890ed078c 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/ToOneIdMapper.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/ToOneIdMapper.java @@ -28,6 +28,7 @@ import java.util.HashMap; import java.util.Map; import org.hibernate.engine.spi.SessionImplementor; +import org.hibernate.envers.RevisionType; import org.hibernate.envers.configuration.AuditConfiguration; import org.hibernate.envers.entities.PropertyData; import org.hibernate.envers.entities.mapper.id.IdMapper; @@ -99,7 +100,7 @@ public class ToOneIdMapper extends AbstractToOneMapper { EntityInfo referencedEntity = getEntityInfo(verCfg, referencedEntityName); value = ToOneEntityLoader.createProxyOrLoadImmediate( versionsReader, referencedEntity.getEntityClass(), referencedEntityName, - entityId, revision, verCfg + entityId, revision, RevisionType.DEL.equals( data.get( verCfg.getAuditEntCfg().getRevisionTypePropName() ) ), verCfg ); } } diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/lazy/ToOneDelegateSessionImplementor.java b/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/lazy/ToOneDelegateSessionImplementor.java index adc9f0224a..2a1721debb 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/lazy/ToOneDelegateSessionImplementor.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/relation/lazy/ToOneDelegateSessionImplementor.java @@ -42,20 +42,22 @@ public class ToOneDelegateSessionImplementor extends AbstractDelegateSessionImpl private final Class entityClass; private final Object entityId; private final Number revision; + private final boolean removed; private final AuditConfiguration verCfg; public ToOneDelegateSessionImplementor(AuditReaderImplementor versionsReader, - Class entityClass, Object entityId, Number revision, + Class entityClass, Object entityId, Number revision, boolean removed, AuditConfiguration verCfg) { super(versionsReader.getSessionImplementor()); this.versionsReader = versionsReader; this.entityClass = entityClass; this.entityId = entityId; this.revision = revision; + this.removed = removed; this.verCfg = verCfg; } public Object doImmediateLoad(String entityName) throws HibernateException { - return ToOneEntityLoader.loadImmediate( versionsReader, entityClass, entityName, entityId, revision, verCfg ); + return ToOneEntityLoader.loadImmediate( versionsReader, entityClass, entityName, entityId, revision, removed, verCfg ); } } diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/query/AuditQueryCreator.java b/hibernate-envers/src/main/java/org/hibernate/envers/query/AuditQueryCreator.java index cf0cd4fba0..895abeb59f 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/query/AuditQueryCreator.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/query/AuditQueryCreator.java @@ -80,6 +80,25 @@ public class AuditQueryCreator { return new EntitiesAtRevisionQuery(auditCfg, auditReaderImplementor, c, entityName, revision); } + /** + * Creates a query, which will return entities satisfying some conditions (specified later), + * at a given revision and a given entityName. Deleted entities may be optionally + * included. + * @param c Class of the entities for which to query. + * @param entityName Name of the entity (if can't be guessed basing on the {@code c}). + * @param revision Revision number at which to execute the query. + * @param includeDeletions Whether to include deleted entities in the search. + * @return A query for entities at a given revision, to which conditions can be added and which + * can then be executed. The result of the query will be a list of entities (beans), unless a + * projection is added. + */ + public AuditQuery forEntitiesAtRevision(Class c, String entityName, Number revision, boolean includeDeletions) { + checkNotNull(revision, "Entity revision"); + checkPositive(revision, "Entity revision"); + c = getTargetClassIfProxied(c); + return new EntitiesAtRevisionQuery(auditCfg, auditReaderImplementor, c, entityName, revision, includeDeletions); + } + /** * Creates a query, which will return entities modified at the specified revision. * diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/query/impl/EntitiesAtRevisionQuery.java b/hibernate-envers/src/main/java/org/hibernate/envers/query/impl/EntitiesAtRevisionQuery.java index d7b168b486..8e98b47cb8 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/query/impl/EntitiesAtRevisionQuery.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/query/impl/EntitiesAtRevisionQuery.java @@ -45,20 +45,31 @@ import static org.hibernate.envers.entities.mapper.relation.query.QueryConstants */ public class EntitiesAtRevisionQuery extends AbstractAuditQuery { private final Number revision; + private final boolean includeDeletions; public EntitiesAtRevisionQuery(AuditConfiguration verCfg, AuditReaderImplementor versionsReader, Class cls, Number revision) { super(verCfg, versionsReader, cls); this.revision = revision; + this.includeDeletions = false; } public EntitiesAtRevisionQuery(AuditConfiguration verCfg, AuditReaderImplementor versionsReader, Class cls, String entityName, Number revision) { super(verCfg, versionsReader, cls, entityName); this.revision = revision; - } + this.includeDeletions = false; + } + public EntitiesAtRevisionQuery(AuditConfiguration verCfg, + AuditReaderImplementor versionsReader, Class cls, + String entityName, Number revision, boolean includeDeletions) { + super(verCfg, versionsReader, cls, entityName); + this.revision = revision; + this.includeDeletions = includeDeletions; + } + @SuppressWarnings({"unchecked"}) public List list() { /* @@ -91,8 +102,10 @@ public class EntitiesAtRevisionQuery extends AbstractAuditQuery { verEntCfg.getRevisionEndFieldName(), true, referencedIdData, revisionPropertyPath, originalIdPropertyName, REFERENCED_ENTITY_ALIAS, REFERENCED_ENTITY_ALIAS_DEF_AUD_STR); - // e.revision_type != DEL - qb.getRootParameters().addWhereWithParam(verEntCfg.getRevisionTypePropName(), "<>", RevisionType.DEL); + // e.revision_type != DEL + if (!includeDeletions) { + qb.getRootParameters().addWhereWithParam(verEntCfg.getRevisionTypePropName(), "<>", RevisionType.DEL); + } // all specified conditions for (AuditCriterion criterion : criterions) { diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/reader/AuditReaderImpl.java b/hibernate-envers/src/main/java/org/hibernate/envers/reader/AuditReaderImpl.java index c804594f4d..784c7b0160 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/reader/AuditReaderImpl.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/reader/AuditReaderImpl.java @@ -95,9 +95,15 @@ public class AuditReaderImpl implements AuditReaderImplementor { cls = getTargetClassIfProxied(cls); return this.find(cls, cls.getName(), primaryKey, revision); } - + + public T find(Class cls, String entityName, Object primaryKey, Number revision) + throws IllegalArgumentException, NotAuditedException, IllegalStateException { + return this.find(cls, entityName, primaryKey, revision, false); + } + @SuppressWarnings({"unchecked"}) - public T find(Class cls, String entityName, Object primaryKey, Number revision) throws + public T find(Class cls, String entityName, Object primaryKey, Number revision, + boolean includeDeletions) throws IllegalArgumentException, NotAuditedException, IllegalStateException { cls = getTargetClassIfProxied(cls); checkNotNull(cls, "Entity class"); @@ -118,7 +124,7 @@ public class AuditReaderImpl implements AuditReaderImplementor { Object result; try { // The result is put into the cache by the entity instantiator called from the query - result = createQuery().forEntitiesAtRevision(cls, entityName, revision) + result = createQuery().forEntitiesAtRevision(cls, entityName, revision, includeDeletions) .add(AuditEntity.id().eq(primaryKey)).getSingleResult(); } catch (NoResultException e) { result = null; diff --git a/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/proxy/RemovedObjectQueryTest.java b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/proxy/RemovedObjectQueryTest.java new file mode 100644 index 0000000000..9ff95b3429 --- /dev/null +++ b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/proxy/RemovedObjectQueryTest.java @@ -0,0 +1,83 @@ +package org.hibernate.envers.test.integration.proxy; + +import org.hibernate.Hibernate; +import org.hibernate.envers.RevisionType; +import org.hibernate.envers.query.AuditEntity; +import org.hibernate.envers.query.AuditQuery; +import org.hibernate.envers.test.BaseEnversJPAFunctionalTestCase; +import org.hibernate.envers.test.Priority; +import org.hibernate.envers.test.entities.onetomany.SetRefEdEntity; +import org.hibernate.envers.test.entities.onetomany.SetRefIngEntity; +import org.hibernate.envers.test.tools.TestTools; +import org.hibernate.testing.FailureExpected; +import org.hibernate.testing.TestForIssue; +import org.junit.Assert; +import org.junit.Test; + +import java.util.List; +import java.util.Map; + +import javax.persistence.EntityManager; + +@TestForIssue(jiraKey = "HHH-5845") +public class RemovedObjectQueryTest extends BaseEnversJPAFunctionalTestCase { + @Override + @SuppressWarnings("unchecked") + protected void addConfigOptions(Map options) { + options.put("org.hibernate.envers.store_data_at_delete", "true"); + } + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { SetRefEdEntity.class, SetRefIngEntity.class }; + } + + @Test + @Priority(10) + public void initData() { + EntityManager em = getEntityManager(); + + SetRefEdEntity refEdEntity = new SetRefEdEntity(1, "Demo Data"); + SetRefIngEntity refIngEntity = new SetRefIngEntity(2, "Example Data", refEdEntity); + + em.getTransaction().begin(); + em.persist(refEdEntity); + em.persist(refIngEntity); + em.getTransaction().commit(); + + em.getTransaction().begin(); + refIngEntity = em.find(SetRefIngEntity.class, 2); + em.remove(refIngEntity); + em.remove(refEdEntity); + em.getTransaction().commit(); + } + + @Test + public void testFindDeletedReference() { + AuditQuery query = getAuditReader().createQuery().forRevisionsOfEntity(SetRefIngEntity.class, false, true) + .add(AuditEntity.revisionType().eq(RevisionType.DEL)); + List queryResult = (List) query.getResultList(); + + Object[] objArray = (Object[]) queryResult.get(0); + SetRefIngEntity refIngEntity = (SetRefIngEntity) objArray[0]; + Assert.assertEquals("Example Data", refIngEntity.getData()); + + Hibernate.initialize(refIngEntity.getReference()); + Assert.assertEquals("Demo Data", refIngEntity.getReference().getData()); + } + + @FailureExpected(jiraKey = "HHH-5845") // TODO: doesn't work until collection queries are fixed + @Test + public void testFindDeletedReferring() { + AuditQuery query = getAuditReader().createQuery().forRevisionsOfEntity(SetRefEdEntity.class, false, true) + .add(AuditEntity.revisionType().eq(RevisionType.DEL)); + List queryResult = (List) query.getResultList(); + + Object[] objArray = (Object[]) queryResult.get(0); + SetRefEdEntity refEdEntity = (SetRefEdEntity) objArray[0]; + Assert.assertEquals("Demo Data", refEdEntity.getData()); + + Hibernate.initialize(refEdEntity.getReffering()); + Assert.assertEquals(TestTools.makeSet(new SetRefIngEntity(2, "Example Data")), refEdEntity.getReffering()); + } +}