From 9fff9862b9ae017e7bbd28d22401efaf04089cca Mon Sep 17 00:00:00 2001 From: Lukasz Antoniak Date: Wed, 19 Oct 2011 19:34:54 +0200 Subject: [PATCH 1/2] HHH-6636 - Fix and test --- .../reader/AuditedPropertiesReader.java | 60 +++++++++++++- .../mapper/ComponentPropertyMapper.java | 7 ++ .../envers/test/AbstractSessionTest.java | 4 +- .../components/UniquePropsEntity.java | 74 +++++++++++++++++ .../UniquePropsNotAuditedEntity.java | 74 +++++++++++++++++ .../components/PropertiesGroupTest.java | 80 +++++++++++++++++++ .../components/UniquePropsEntity.hbm.xml | 13 +++ .../UniquePropsNotAuditedEntity.hbm.xml | 13 +++ 8 files changed, 320 insertions(+), 5 deletions(-) create mode 100644 hibernate-envers/src/matrix/java/org/hibernate/envers/test/entities/components/UniquePropsEntity.java create mode 100644 hibernate-envers/src/matrix/java/org/hibernate/envers/test/entities/components/UniquePropsNotAuditedEntity.java create mode 100644 hibernate-envers/src/matrix/java/org/hibernate/envers/test/integration/components/PropertiesGroupTest.java create mode 100644 hibernate-envers/src/matrix/resources/mappings/components/UniquePropsEntity.hbm.xml create mode 100644 hibernate-envers/src/matrix/resources/mappings/components/UniquePropsNotAuditedEntity.hbm.xml diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/metadata/reader/AuditedPropertiesReader.java b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/metadata/reader/AuditedPropertiesReader.java index 32eda20a70..96b1a81db7 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/metadata/reader/AuditedPropertiesReader.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/metadata/reader/AuditedPropertiesReader.java @@ -1,9 +1,11 @@ package org.hibernate.envers.configuration.metadata.reader; +import static org.hibernate.envers.tools.Tools.newHashMap; import static org.hibernate.envers.tools.Tools.newHashSet; import java.lang.annotation.Annotation; import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Set; import javax.persistence.JoinColumn; import javax.persistence.MapKey; @@ -46,6 +48,8 @@ public class AuditedPropertiesReader { private final Set propertyAccessedPersistentProperties; private final Set fieldAccessedPersistentProperties; + // Mapping class field to corresponding element. + private final Map propertiesGroupMapping; public AuditedPropertiesReader(ModificationStore defaultStore, PersistentPropertiesSource persistentPropertiesSource, @@ -62,6 +66,7 @@ public class AuditedPropertiesReader { propertyAccessedPersistentProperties = newHashSet(); fieldAccessedPersistentProperties = newHashSet(); + propertiesGroupMapping = newHashMap(); } public void read() { @@ -116,14 +121,32 @@ public class AuditedPropertiesReader { Iterator propertyIter = persistentPropertiesSource.getPropertyIterator(); while (propertyIter.hasNext()) { Property property = (Property) propertyIter.next(); - if ("field".equals(property.getPropertyAccessorName())) { - fieldAccessedPersistentProperties.add(property.getName()); - } else { - propertyAccessedPersistentProperties.add(property.getName()); + addPersistentProperty(property); + if ("embedded".equals(property.getPropertyAccessorName()) && property.getName().equals(property.getNodeName())) { + // If property name equals node name and embedded accessor type is used, processing field + // has been defined inside tag. See HHH-6636 JIRA issue. + createPropertiesGroupMapping(property); } } } + private void addPersistentProperty(Property property) { + if ("field".equals(property.getPropertyAccessorName())) { + fieldAccessedPersistentProperties.add(property.getName()); + } else { + propertyAccessedPersistentProperties.add(property.getName()); + } + } + + private void createPropertiesGroupMapping(Property property) { + Component component = (Component) property.getValue(); + Iterator componentProperties = component.getPropertyIterator(); + while (componentProperties.hasNext()) { + Property componentProperty = componentProperties.next(); + propertiesGroupMapping.put(componentProperty.getName(), component.getNodeName()); + } + } + /** * @param clazz Class which properties are currently being added. * @param declaredAuditedSuperclasses Collection of superclasses that have been explicitly declared to be audited. @@ -177,9 +200,38 @@ public class AuditedPropertiesReader { } else { this.addFromNotComponentProperty(property, accessType, allClassAudited); } + } else if (propertiesGroupMapping.containsKey(property.getName())) { + // Retrieve embedded component name based on class field. + final String embeddedName = propertiesGroupMapping.get(property.getName()); + if (!auditedPropertiesHolder.contains(embeddedName)) { + // Manage properties mapped within tag. + Value propertyValue = persistentPropertiesSource.getProperty(embeddedName).getValue(); + this.addFromPropertiesGroup(embeddedName, property, accessType, (Component)propertyValue, allClassAudited); + } } } } + + private void addFromPropertiesGroup(String embeddedName, XProperty property, String accessType, Component propertyValue, + Audited allClassAudited) { + ComponentAuditingData componentData = new ComponentAuditingData(); + boolean isAudited = fillPropertyData(property, componentData, accessType, allClassAudited); + if (isAudited) { + // EntityPersister.getPropertyNames() returns name of embedded component instead of class field. + componentData.setName(embeddedName); + // Marking component properties as placed directly in class (not inside another component). + componentData.setBeanName(null); + + PersistentPropertiesSource componentPropertiesSource = new ComponentPropertiesSource((Component) propertyValue); + AuditedPropertiesReader audPropReader = new AuditedPropertiesReader( + ModificationStore.FULL, componentPropertiesSource, componentData, globalCfg, reflectionManager, + propertyNamePrefix + MappingTools.createComponentPrefix( embeddedName ) + ); + audPropReader.read(); + + auditedPropertiesHolder.addPropertyAuditingData(embeddedName, componentData); + } + } private void addFromComponentProperty(XProperty property, String accessType, Component propertyValue, Audited allClassAudited) { diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/ComponentPropertyMapper.java b/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/ComponentPropertyMapper.java index b3119d0552..fb10c61c0f 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/ComponentPropertyMapper.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/entities/mapper/ComponentPropertyMapper.java @@ -70,6 +70,13 @@ public class ComponentPropertyMapper implements PropertyMapper, CompositeMapperB return; } + if (propertyData.getBeanName() == null) { + // If properties are not encapsulated in a component but placed directly in a class + // (e.g. by applying tag). + delegate.mapToEntityFromMap(verCfg, obj, data, primaryKey, versionsReader, revision); + return; + } + Setter setter = ReflectionTools.getSetter(obj.getClass(), propertyData); // If all properties are null and single, then the component has to be null also. diff --git a/hibernate-envers/src/matrix/java/org/hibernate/envers/test/AbstractSessionTest.java b/hibernate-envers/src/matrix/java/org/hibernate/envers/test/AbstractSessionTest.java index c3fb459a0f..09c804bf91 100644 --- a/hibernate-envers/src/matrix/java/org/hibernate/envers/test/AbstractSessionTest.java +++ b/hibernate-envers/src/matrix/java/org/hibernate/envers/test/AbstractSessionTest.java @@ -83,7 +83,9 @@ public abstract class AbstractSessionTest extends AbstractEnversTest { return session; } - + protected Configuration getCfg() { + return config; + } protected AuditReader getAuditReader() { return auditReader; diff --git a/hibernate-envers/src/matrix/java/org/hibernate/envers/test/entities/components/UniquePropsEntity.java b/hibernate-envers/src/matrix/java/org/hibernate/envers/test/entities/components/UniquePropsEntity.java new file mode 100644 index 0000000000..84446a4de6 --- /dev/null +++ b/hibernate-envers/src/matrix/java/org/hibernate/envers/test/entities/components/UniquePropsEntity.java @@ -0,0 +1,74 @@ +package org.hibernate.envers.test.entities.components; + +import org.hibernate.envers.Audited; + +import java.io.Serializable; + +/** + * @author Lukasz Antoniak (lukasz dot antoniak at gmail dot com) + */ +@Audited +public class UniquePropsEntity implements Serializable { + private Long id; + private String data1; + private String data2; + + public UniquePropsEntity() { + } + + public UniquePropsEntity(Long id, String data1, String data2) { + this.id = id; + this.data1 = data1; + this.data2 = data2; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + UniquePropsEntity that = (UniquePropsEntity) o; + + if (data1 != null ? !data1.equals(that.data1) : that.data1 != null) return false; + if (data2 != null ? !data2.equals(that.data2) : that.data2 != null) return false; + if (id != null ? !id.equals(that.id) : that.id != null) return false; + + return true; + } + + @Override + public int hashCode() { + int result = id != null ? id.hashCode() : 0; + result = 31 * result + (data1 != null ? data1.hashCode() : 0); + result = 31 * result + (data2 != null ? data2.hashCode() : 0); + return result; + } + + public String toString() { + return "UniquePropsEntity(id = " + id + ", data1 = " + data1 + ", data2 = " + data2 + ")"; + } + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public String getData1() { + return data1; + } + + public void setData1(String data1) { + this.data1 = data1; + } + + public String getData2() { + return data2; + } + + public void setData2(String data2) { + this.data2 = data2; + } +} diff --git a/hibernate-envers/src/matrix/java/org/hibernate/envers/test/entities/components/UniquePropsNotAuditedEntity.java b/hibernate-envers/src/matrix/java/org/hibernate/envers/test/entities/components/UniquePropsNotAuditedEntity.java new file mode 100644 index 0000000000..cb034843de --- /dev/null +++ b/hibernate-envers/src/matrix/java/org/hibernate/envers/test/entities/components/UniquePropsNotAuditedEntity.java @@ -0,0 +1,74 @@ +package org.hibernate.envers.test.entities.components; + +import org.hibernate.envers.Audited; +import org.hibernate.envers.NotAudited; + +/** + * @author Lukasz Antoniak (lukasz dot antoniak at gmail dot com) + */ +@Audited +public class UniquePropsNotAuditedEntity { + private Long id; + private String data1; + private String data2; + + public UniquePropsNotAuditedEntity() { + } + + public UniquePropsNotAuditedEntity(Long id, String data1, String data2) { + this.id = id; + this.data1 = data1; + this.data2 = data2; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + UniquePropsNotAuditedEntity that = (UniquePropsNotAuditedEntity) o; + + if (data1 != null ? !data1.equals(that.data1) : that.data1 != null) return false; + if (data2 != null ? !data2.equals(that.data2) : that.data2 != null) return false; + if (id != null ? !id.equals(that.id) : that.id != null) return false; + + return true; + } + + @Override + public int hashCode() { + int result = id != null ? id.hashCode() : 0; + result = 31 * result + (data1 != null ? data1.hashCode() : 0); + result = 31 * result + (data2 != null ? data2.hashCode() : 0); + return result; + } + + public String toString() { + return "UniquePropsNotAuditedEntity(id = " + id + ", data1 = " + data1 + ", data2 = " + data2 + ")"; + } + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public String getData1() { + return data1; + } + + public void setData1(String data1) { + this.data1 = data1; + } + + @NotAudited + public String getData2() { + return data2; + } + + public void setData2(String data2) { + this.data2 = data2; + } +} diff --git a/hibernate-envers/src/matrix/java/org/hibernate/envers/test/integration/components/PropertiesGroupTest.java b/hibernate-envers/src/matrix/java/org/hibernate/envers/test/integration/components/PropertiesGroupTest.java new file mode 100644 index 0000000000..bea7b9c881 --- /dev/null +++ b/hibernate-envers/src/matrix/java/org/hibernate/envers/test/integration/components/PropertiesGroupTest.java @@ -0,0 +1,80 @@ +package org.hibernate.envers.test.integration.components; + +import org.hibernate.MappingException; +import org.hibernate.envers.test.AbstractSessionTest; +import org.hibernate.envers.test.Priority; +import org.hibernate.envers.test.entities.components.UniquePropsEntity; +import org.hibernate.envers.test.entities.components.UniquePropsNotAuditedEntity; +import org.hibernate.mapping.Column; +import org.hibernate.mapping.PersistentClass; +import org.hibernate.testing.TestForIssue; +import org.junit.Assert; +import org.junit.Test; + +import java.io.File; +import java.net.URISyntaxException; +import java.net.URL; + +/** + * @author Lukasz Antoniak (lukasz dot antoniak at gmail dot com) + */ +@TestForIssue(jiraKey = "HHH-6636") +public class PropertiesGroupTest extends AbstractSessionTest { + private PersistentClass uniquePropsAudit = null; + private PersistentClass uniquePropsNotAuditedAudit = null; + private UniquePropsEntity entityRev1 = null; + private UniquePropsNotAuditedEntity entityNotAuditedRev2 = null; + + protected void initMappings() throws MappingException, URISyntaxException { + URL url = Thread.currentThread().getContextClassLoader().getResource("mappings/components/UniquePropsEntity.hbm.xml"); + config.addFile(new File(url.toURI())); + url = Thread.currentThread().getContextClassLoader().getResource("mappings/components/UniquePropsNotAuditedEntity.hbm.xml"); + config.addFile(new File(url.toURI())); + } + + @Test + @Priority(10) + public void initData() { + uniquePropsAudit = getCfg().getClassMapping("org.hibernate.envers.test.entities.components.UniquePropsEntity_AUD"); + uniquePropsNotAuditedAudit = getCfg().getClassMapping("org.hibernate.envers.test.entities.components.UniquePropsNotAuditedEntity_AUD"); + + // Revision 1 + getSession().getTransaction().begin(); + UniquePropsEntity ent = new UniquePropsEntity(); + ent.setData1("data1"); + ent.setData2("data2"); + getSession().persist(ent); + getSession().getTransaction().commit(); + + entityRev1 = new UniquePropsEntity(ent.getId(), ent.getData1(), ent.getData2()); + + // Revision 2 + getSession().getTransaction().begin(); + UniquePropsNotAuditedEntity entNotAud = new UniquePropsNotAuditedEntity(); + entNotAud.setData1("data3"); + entNotAud.setData2("data4"); + getSession().persist(entNotAud); + getSession().getTransaction().commit(); + + entityNotAuditedRev2 = new UniquePropsNotAuditedEntity(entNotAud.getId(), entNotAud.getData1(), null); + } + + @Test + public void testAuditTableColumns() { + Assert.assertNotNull(uniquePropsAudit.getTable().getColumn(new Column("DATA1"))); + Assert.assertNotNull(uniquePropsAudit.getTable().getColumn(new Column("DATA2"))); + + Assert.assertNotNull(uniquePropsNotAuditedAudit.getTable().getColumn(new Column("DATA1"))); + Assert.assertNull(uniquePropsNotAuditedAudit.getTable().getColumn(new Column("DATA2"))); + } + + @Test + public void testHistoryOfUniquePropsEntity() { + Assert.assertEquals(entityRev1, getAuditReader().find(UniquePropsEntity.class, entityRev1.getId(), 1)); + } + + @Test + public void testHistoryOfUniquePropsNotAuditedEntity() { + Assert.assertEquals(entityNotAuditedRev2, getAuditReader().find(UniquePropsNotAuditedEntity.class, entityNotAuditedRev2.getId(), 2)); + } +} diff --git a/hibernate-envers/src/matrix/resources/mappings/components/UniquePropsEntity.hbm.xml b/hibernate-envers/src/matrix/resources/mappings/components/UniquePropsEntity.hbm.xml new file mode 100644 index 0000000000..e3c7c2ebd4 --- /dev/null +++ b/hibernate-envers/src/matrix/resources/mappings/components/UniquePropsEntity.hbm.xml @@ -0,0 +1,13 @@ + + + + + + + + + + + + + diff --git a/hibernate-envers/src/matrix/resources/mappings/components/UniquePropsNotAuditedEntity.hbm.xml b/hibernate-envers/src/matrix/resources/mappings/components/UniquePropsNotAuditedEntity.hbm.xml new file mode 100644 index 0000000000..8f5a3572e1 --- /dev/null +++ b/hibernate-envers/src/matrix/resources/mappings/components/UniquePropsNotAuditedEntity.hbm.xml @@ -0,0 +1,13 @@ + + + + + + + + + + + + + From 8dd9c79ebdbc60ae93f915a8bbfba318b3e0ae9c Mon Sep 17 00:00:00 2001 From: Lukasz Antoniak Date: Wed, 19 Oct 2011 19:43:12 +0200 Subject: [PATCH 2/2] HHH-6636 - Comment change --- .../metadata/reader/AuditedPropertiesReader.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/metadata/reader/AuditedPropertiesReader.java b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/metadata/reader/AuditedPropertiesReader.java index 96b1a81db7..34a8ff11c9 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/metadata/reader/AuditedPropertiesReader.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/metadata/reader/AuditedPropertiesReader.java @@ -123,8 +123,8 @@ public class AuditedPropertiesReader { Property property = (Property) propertyIter.next(); addPersistentProperty(property); if ("embedded".equals(property.getPropertyAccessorName()) && property.getName().equals(property.getNodeName())) { - // If property name equals node name and embedded accessor type is used, processing field - // has been defined inside tag. See HHH-6636 JIRA issue. + // If property name equals node name and embedded accessor type is used, processing component + // has been defined with tag. See HHH-6636 JIRA issue. createPropertiesGroupMapping(property); } } @@ -225,7 +225,7 @@ public class AuditedPropertiesReader { PersistentPropertiesSource componentPropertiesSource = new ComponentPropertiesSource((Component) propertyValue); AuditedPropertiesReader audPropReader = new AuditedPropertiesReader( ModificationStore.FULL, componentPropertiesSource, componentData, globalCfg, reflectionManager, - propertyNamePrefix + MappingTools.createComponentPrefix( embeddedName ) + propertyNamePrefix + MappingTools.createComponentPrefix(embeddedName) ); audPropReader.read();