Merge pull request #198 from lukasz-antoniak/HHH-6636

HHH 6636 - Properties listed inside <properties> tag are not audited

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<String> propertyAccessedPersistentProperties;
 	private final Set<String> fieldAccessedPersistentProperties;
+	// Mapping class field to corresponding <properties> element.
+	private final Map<String, String> propertiesGroupMapping;
 
 	public AuditedPropertiesReader(ModificationStore defaultStore,
 								   PersistentPropertiesSource persistentPropertiesSource,
@@ -62,6 +66,7 @@ public class AuditedPropertiesReader {
 
 		propertyAccessedPersistentProperties = newHashSet();
 		fieldAccessedPersistentProperties = newHashSet();
+		propertiesGroupMapping = newHashMap();
 	}
 
 	public void read() {
@@ -116,12 +121,30 @@ public class AuditedPropertiesReader {
 		Iterator<Property> propertyIter = persistentPropertiesSource.getPropertyIterator();
 		while (propertyIter.hasNext()) {
 			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 component
+				// has been defined with <properties> 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<Property> componentProperties = component.getPropertyIterator();
+        while (componentProperties.hasNext()) {
+            Property componentProperty = componentProperties.next();
+            propertiesGroupMapping.put(componentProperty.getName(), component.getNodeName());
+        }
     }
 
     /**
@@ -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 <properties> 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) {

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 <properties> 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.

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;

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;
+    }
+}

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;
+    }
+}

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));
+    }
+}

hibernate-envers/src/matrix/resources/mappings/components/UniquePropsEntity.hbm.xml

@@ -0,0 +1,13 @@
+<?xml version="1.0"?>
+<!DOCTYPE hibernate-mapping PUBLIC "-//Hibernate/Hibernate Mapping DTD 3.0//EN" "http://www.hibernate.org/dtd/hibernate-mapping-3.0.dtd">
+<hibernate-mapping>
+    <class name="org.hibernate.envers.test.entities.components.UniquePropsEntity">
+        <id name="id" column="ID" type="long">
+            <generator class="native"/>
+        </id>
+        <properties name="uniqueProps" unique="true">
+            <property name="data1" type="string"/>
+            <property name="data2" type="string"/>
+        </properties>
+    </class>
+</hibernate-mapping>

hibernate-envers/src/matrix/resources/mappings/components/UniquePropsNotAuditedEntity.hbm.xml

@@ -0,0 +1,13 @@
+<?xml version="1.0"?>
+<!DOCTYPE hibernate-mapping PUBLIC "-//Hibernate/Hibernate Mapping DTD 3.0//EN" "http://www.hibernate.org/dtd/hibernate-mapping-3.0.dtd">
+<hibernate-mapping>
+    <class name="org.hibernate.envers.test.entities.components.UniquePropsNotAuditedEntity">
+        <id name="id" column="ID" type="long">
+            <generator class="native"/>
+        </id>
+        <properties name="uniquePropsNotAudited" unique="true">
+            <property name="data1" type="string"/>
+            <property name="data2" type="string"/>
+        </properties>
+    </class>
+</hibernate-mapping>