From aba202c71a712a38f0ae160e7aa6b7eb05ab86b8 Mon Sep 17 00:00:00 2001 From: Chris Cranford Date: Tue, 17 Oct 2017 13:54:09 -0400 Subject: [PATCH] HHH-11988 - Avoid adding unnecessary audit rows for basic types. --- .../metadata/BasicMetadataGenerator.java | 7 ++-- .../metadata/CollectionMetadataGenerator.java | 8 ++--- .../metadata/reader/PropertyAuditingData.java | 9 ++++- .../internal/entities/PropertyData.java | 34 +++++++++++++++++++ .../entities/mapper/SinglePropertyMapper.java | 16 +++++++-- 5 files changed, 63 insertions(+), 11 deletions(-) diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/BasicMetadataGenerator.java b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/BasicMetadataGenerator.java index 6edf4cef5c..b18f3b79f4 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/BasicMetadataGenerator.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/BasicMetadataGenerator.java @@ -9,6 +9,7 @@ package org.hibernate.envers.configuration.internal.metadata; import java.util.Properties; import org.hibernate.envers.configuration.internal.metadata.reader.PropertyAuditingData; +import org.hibernate.envers.internal.entities.PropertyData; import org.hibernate.envers.internal.entities.mapper.SimpleMapperBuilder; import org.hibernate.mapping.SimpleValue; import org.hibernate.mapping.Value; @@ -52,7 +53,8 @@ public final class BasicMetadataGenerator { // A null mapper means that we only want to add xml mappings if ( mapper != null ) { - mapper.add( propertyAuditingData.getPropertyData() ); + final PropertyData propertyData = propertyAuditingData.resolvePropertyData( value.getType() ); + mapper.add( propertyData ); } return true; @@ -108,7 +110,8 @@ public final class BasicMetadataGenerator { // A null mapper means that we only want to add xml mappings if ( mapper != null ) { - mapper.add( propertyAuditingData.getPropertyData() ); + final PropertyData propertyData = propertyAuditingData.resolvePropertyData( value.getType() ); + mapper.add( propertyData ); } return true; diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/CollectionMetadataGenerator.java b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/CollectionMetadataGenerator.java index 8dc5025748..443d19315d 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/CollectionMetadataGenerator.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/CollectionMetadataGenerator.java @@ -287,13 +287,9 @@ public final class CollectionMetadataGenerator { // Checking if there's an index defined. If so, adding a mapper for it. if ( positionMappedBy != null ) { + final Type indexType = ( (IndexedCollection) propertyValue ).getIndex().getType(); fakeBidirectionalRelationIndexMapper = new SinglePropertyMapper( - new PropertyData( - positionMappedBy, - null, - null, - null - ) + PropertyData.forProperty( positionMappedBy, indexType ) ); // Also, overwriting the index component data to properly read the index. diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/PropertyAuditingData.java b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/PropertyAuditingData.java index 9459d7994e..dff0579b53 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/PropertyAuditingData.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/reader/PropertyAuditingData.java @@ -16,6 +16,7 @@ import org.hibernate.envers.ModificationStore; import org.hibernate.envers.RelationTargetAuditMode; import org.hibernate.envers.internal.entities.PropertyData; import org.hibernate.mapping.Value; +import org.hibernate.type.Type; /** * @author Adam Warski (adam at warski dot org) @@ -141,7 +142,12 @@ public class PropertyAuditingData { this.accessType = accessType; } + // todo (6.0) - remove this and use #resolvePropertyData instead public PropertyData getPropertyData() { + return resolvePropertyData( null ); + } + + public PropertyData resolvePropertyData(Type propertyType) { return new PropertyData( name, beanName, @@ -149,7 +155,8 @@ public class PropertyAuditingData { store, usingModifiedFlag, modifiedFlagName, - syntheic + syntheic, + propertyType ); } diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/PropertyData.java b/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/PropertyData.java index 018977c0ab..9d08544275 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/PropertyData.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/PropertyData.java @@ -8,6 +8,7 @@ package org.hibernate.envers.internal.entities; import org.hibernate.envers.ModificationStore; import org.hibernate.internal.util.compare.EqualsHelper; +import org.hibernate.type.Type; /** * Holds information on a property that is audited. @@ -28,6 +29,7 @@ public class PropertyData { // Synthetic properties are ones which are not part of the actual java model. // They're properties used for bookkeeping by Hibernate private boolean synthetic; + private Type propertyType; /** * Copies the given property data, except the name. @@ -55,6 +57,11 @@ public class PropertyData { this.store = store; } + private PropertyData(String name, String beanName, String accessType, ModificationStore store, Type propertyType) { + this( name, beanName, accessType, store ); + this.propertyType = propertyType; + } + /** * @param name Name of the property. * @param beanName Name of the property in the bean. @@ -76,6 +83,19 @@ public class PropertyData { this.synthetic = synthetic; } + public PropertyData( + String name, + String beanName, + String accessType, + ModificationStore store, + boolean usingModifiedFlag, + String modifiedFlagName, + boolean synthetic, + Type propertyType) { + this( name, beanName, accessType, store, usingModifiedFlag, modifiedFlagName, synthetic ); + this.propertyType = propertyType; + } + public String getName() { return name; } @@ -108,6 +128,10 @@ public class PropertyData { return synthetic; } + public Type getType() { + return propertyType; + } + @Override public boolean equals(Object o) { if ( this == o ) { @@ -136,4 +160,14 @@ public class PropertyData { result = 31 * result + (synthetic ? 1 : 0); return result; } + + public static PropertyData forProperty(String propertyName, Type propertyType) { + return new PropertyData( + propertyName, + null, + null, + null, + propertyType + ); + } } diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/SinglePropertyMapper.java b/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/SinglePropertyMapper.java index 602cd003f1..319544e9fe 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/SinglePropertyMapper.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/SinglePropertyMapper.java @@ -63,7 +63,7 @@ public class SinglePropertyMapper implements PropertyMapper, SimpleMapperBuilder // Don't generate new revision when database replaces empty string with NULL during INSERT or UPDATE statements. dbLogicallyDifferent = !(StringTools.isEmpty( newObj ) && StringTools.isEmpty( oldObj )); } - return dbLogicallyDifferent && !EqualsHelper.areEqual( newObj, oldObj ); + return dbLogicallyDifferent && !areEqual( newObj, oldObj ); } @Override @@ -74,7 +74,7 @@ public class SinglePropertyMapper implements PropertyMapper, SimpleMapperBuilder Object oldObj) { // Synthetic properties are not subject to withModifiedFlag analysis if ( propertyData.isUsingModifiedFlag() && !propertyData.isSynthetic() ) { - data.put( propertyData.getModifiedFlagPropertyName(), !EqualsHelper.areEqual( newObj, oldObj ) ); + data.put( propertyData.getModifiedFlagPropertyName(), !areEqual( newObj, oldObj ) ); } } @@ -137,4 +137,16 @@ public class SinglePropertyMapper implements PropertyMapper, SimpleMapperBuilder public boolean hasPropertiesWithModifiedFlag() { return propertyData != null && propertyData.isUsingModifiedFlag(); } + + private boolean areEqual(Object newObj, Object oldObj) { + // Should a Type have been specified on the property mapper, delegate there to make sure + // that proper equality comparison occurs based on the Type's semantics rather than the + // generalized EqualsHelper #areEqual call. + if ( propertyData.getType() != null ) { + return propertyData.getType().isEqual( newObj, oldObj ); + } + // todo (6.0) - Confirm if this is still necessary as everything should use a JavaTypeDescriptor. + // This was maintained for legacy 5.2 behavior only. + return EqualsHelper.areEqual( newObj, oldObj ); + } }