HHH-12327 - Remove the Envers dependency on Javassist.

hibernate · Mar 6, 2018 · 4a3f7c1 · 4a3f7c1
1 parent 493c968
commit 4a3f7c1
Show file tree

Hide file tree

Showing 13 changed files with 211 additions and 395 deletions.
diff --git a/hibernate-envers/hibernate-envers.gradle b/hibernate-envers/hibernate-envers.gradle
@@ -12,16 +12,13 @@ description = 'Hibernate\'s entity version (audit/history) support'
 
 dependencies {
     compile( project( ':hibernate-core' ) )
-    //Ideally javassist should be only an optional dependency but it's currently required by Envers: see HHH-12327
-    compile( libraries.javassist )
 
     provided( [group: 'org.hibernate', name: 'hibernate-tools', version: '3.2.0.ga'] )
     provided( libraries.ant )
     provided( project( ':hibernate-jpamodelgen' ) )
 
     testCompile( project( ':hibernate-testing' ) )
     testCompile( project( path: ':hibernate-core', configuration: 'tests' ) )
-    testRuntime( libraries.byteBuddy )
 }
 
 sourceSets {

diff --git a/...s/src/main/java/org/hibernate/envers/internal/entities/mapper/AbstractPropertyMapper.java b/...s/src/main/java/org/hibernate/envers/internal/entities/mapper/AbstractPropertyMapper.java
@@ -0,0 +1,26 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.envers.internal.entities.mapper;
+
+/**
+ * Abstract implementation of a {@link PropertyMapper}.
+ *
+ * @author Chris Cranford
+ */
+public abstract class AbstractPropertyMapper implements PropertyMapper {
+	private boolean map;
+
+	@Override
+	public void markAsDynamicComponentMap() {
+		this.map = true;
+	}
+
+	@Override
+	public boolean isDynamicComponentMap() {
+		return map;
+	}
+}
diff --git a/.../src/main/java/org/hibernate/envers/internal/entities/mapper/ComponentPropertyMapper.java b/.../src/main/java/org/hibernate/envers/internal/entities/mapper/ComponentPropertyMapper.java
@@ -29,7 +29,7 @@
  * @author Lukasz Zuchowski (author at zuchos dot com)
  * @author Chris Cranford
  */
-public class ComponentPropertyMapper implements PropertyMapper, CompositeMapperBuilder {
+public class ComponentPropertyMapper extends AbstractPropertyMapper implements CompositeMapperBuilder {
 	private final PropertyData propertyData;
 	private final MultiPropertyMapper delegate;
 	private final Class componentClass;
@@ -122,28 +122,34 @@ public void mapToEntityFromMap(
 				new PrivilegedAction<Object>() {
 					@Override
 					public Object run() {
-						final Setter setter = ReflectionTools.getSetter(
-								obj.getClass(),
-								propertyData,
-								enversService.getServiceRegistry()
-						);
-
-						if ( isAllPropertiesNull( data ) ) {
-							// single property, but default value need not be null, so we'll set it to null anyway
-							setter.set( obj, null, null );
-						}
-						else {
-							// set the component
-							try {
-								final Object subObj = ReflectHelper.getDefaultConstructor( componentClass ).newInstance();
-								setter.set( obj, subObj, null );
+						try {
+							final Object subObj = ReflectHelper.getDefaultConstructor( componentClass ).newInstance();
 
+							if ( isDynamicComponentMap() ) {
+								( (Map) obj ).put( propertyData.getBeanName(), subObj );
 								delegate.mapToEntityFromMap( enversService, subObj, data, primaryKey, versionsReader, revision );
 							}
-							catch ( Exception e ) {
-								throw new AuditException( e );
+							else {
+								final Setter setter = ReflectionTools.getSetter(
+										obj.getClass(),
+										propertyData,
+										enversService.getServiceRegistry()
+								);
+
+								if ( isAllPropertiesNull( data ) ) {
+									// single property, but default value need not be null, so we'll set it to null anyway
+									setter.set( obj, null, null );
+								}
+								else {
+									// set the component
+									setter.set( obj, subObj, null );
+									delegate.mapToEntityFromMap( enversService, subObj, data, primaryKey, versionsReader, revision );
+								}
 							}
 						}
+						catch ( Exception e ) {
+							throw new AuditException( e );
+						}
 
 						return null;
 					}

diff --git a/...ain/java/org/hibernate/envers/internal/entities/mapper/DynamicComponentMapperSupport.java b/...ain/java/org/hibernate/envers/internal/entities/mapper/DynamicComponentMapperSupport.java
@@ -0,0 +1,28 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.envers.internal.entities.mapper;
+
+/**
+ * Contract for {@link PropertyMapper} implementations to expose whether they should be included
+ * as a wrapper for a {@code <dynamic-component/>} mapping.
+ *
+ * In this mapping, values are actually stored as a key-value pair in a HashMap rather than
+ * them being treated as java-bean values using a setter method.
+ *
+ * @author Chris Cranford
+ */
+public interface DynamicComponentMapperSupport {
+	/**
+	 * Mark the property mapper that it wraps a dynamic-component.
+	 */
+	void markAsDynamicComponentMap();
+
+	/**
+	 * Returns whether the property mapper wraps a dynamic-component.
+	 */
+	boolean isDynamicComponentMap();
+}
diff --git a/.../main/java/org/hibernate/envers/internal/entities/mapper/MultiDynamicComponentMapper.java b/.../main/java/org/hibernate/envers/internal/entities/mapper/MultiDynamicComponentMapper.java
@@ -1,4 +1,4 @@
-/*
+	/*
  * Hibernate, Relational Persistence for Idiomatic Java
  *
  * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
@@ -12,13 +12,12 @@
 import org.hibernate.envers.boot.internal.EnversService;
 import org.hibernate.envers.internal.entities.PropertyData;
 import org.hibernate.envers.internal.reader.AuditReaderImplementor;
-import org.hibernate.envers.internal.tools.MapProxyTool;
-import org.hibernate.envers.internal.tools.StringTools;
 
 /**
  * Multi mapper for dynamic components (it knows that component is a map, not a class)
  *
  * @author Lukasz Zuchowski (author at zuchos dot com)
+ * @author Chris Cranford
  */
 public class MultiDynamicComponentMapper extends MultiPropertyMapper {
 
@@ -28,6 +27,22 @@ public MultiDynamicComponentMapper(PropertyData dynamicComponentData) {
 		this.dynamicComponentData = dynamicComponentData;
 	}
 
+	@Override
+	public void addComposite(PropertyData propertyData, PropertyMapper propertyMapper) {
+		// delegate to the super implementation and then mark the property mapper as a dynamic-component map.
+		super.addComposite( propertyData, propertyMapper );
+		propertyMapper.markAsDynamicComponentMap();
+	}
+
+	@Override
+	public void add(PropertyData propertyData) {
+		final SinglePropertyMapper single = new SinglePropertyMapper();
+		single.add( propertyData );
+
+		// delegate to our implementation of #addComposite for specialized behavior.
+		addComposite( propertyData, single );
+	}
+
 	@Override
 	public boolean mapToMapFromEntity(
 			SessionImplementor session,
@@ -96,30 +111,15 @@ public void mapModifiedFlagsToMapFromEntity(
 	}
 
 	@Override
-	@SuppressWarnings("unchecked")
 	public void mapToEntityFromMap(
 			EnversService enversService,
 			Object obj,
 			Map data,
 			Object primaryKey,
 			AuditReaderImplementor versionsReader,
 			Number revision) {
-		Object mapProxy = MapProxyTool.newInstanceOfBeanProxyForMap(
-				generateClassName( data, dynamicComponentData.getBeanName() ),
-				(Map) obj,
-				properties.keySet(),
-				enversService.getClassLoaderService()
-		);
 		for ( PropertyMapper mapper : properties.values() ) {
-			mapper.mapToEntityFromMap( enversService, mapProxy, data, primaryKey, versionsReader, revision );
+			mapper.mapToEntityFromMap( enversService, obj, data, primaryKey, versionsReader, revision );
 		}
 	}
-
-	private String generateClassName(Map data, String dynamicComponentPropertyName) {
-		return ( data.get( "$type$" ) + StringTools.capitalizeFirst( dynamicComponentPropertyName ) ).replaceAll(
-				"_",
-				""
-		);
-	}
-
 }
diff --git a/...vers/src/main/java/org/hibernate/envers/internal/entities/mapper/MultiPropertyMapper.java b/...vers/src/main/java/org/hibernate/envers/internal/entities/mapper/MultiPropertyMapper.java
@@ -29,7 +29,7 @@
  * @author Lukasz Zuchowski (author at zuchos dot com)
  * @author Chris Cranford
  */
-public class MultiPropertyMapper implements ExtendedPropertyMapper {
+public class MultiPropertyMapper extends AbstractPropertyMapper implements ExtendedPropertyMapper {
 	protected final Map<PropertyData, PropertyMapper> properties;
 	private final Map<String, PropertyData> propertyDatas;
 

diff --git a/...te-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/PropertyMapper.java b/...te-envers/src/main/java/org/hibernate/envers/internal/entities/mapper/PropertyMapper.java
@@ -20,7 +20,7 @@
  * @author Michal Skowronek (mskowr at o2 dot pl)
  * @author Chris Cranford
  */
-public interface PropertyMapper extends ModifiedFlagMapperSupport {
+public interface PropertyMapper extends ModifiedFlagMapperSupport, DynamicComponentMapperSupport {
 	/**
 	 * Maps properties to the given map, basing on differences between properties of new and old objects.
 	 *

diff --git a/...ers/src/main/java/org/hibernate/envers/internal/entities/mapper/SinglePropertyMapper.java b/...ers/src/main/java/org/hibernate/envers/internal/entities/mapper/SinglePropertyMapper.java
@@ -33,7 +33,7 @@
  * @author Michal Skowronek (mskowr at o2 dot pl)
  * @author Chris Cranford
  */
-public class SinglePropertyMapper implements PropertyMapper, SimpleMapperBuilder {
+public class SinglePropertyMapper extends AbstractPropertyMapper implements SimpleMapperBuilder {
 	private PropertyData propertyData;
 
 	public SinglePropertyMapper(PropertyData propertyData) {
@@ -97,27 +97,34 @@ public void mapToEntityFromMap(
 			return;
 		}
 
-		AccessController.doPrivileged(
-				new PrivilegedAction<Object>() {
-					@Override
-					public Object run() {
-						final Setter setter = ReflectionTools.getSetter(
-								obj.getClass(),
-								propertyData,
-								enversService.getServiceRegistry()
-						);
-
-						final Object value = data.get( propertyData.getName() );
-
-						// We only set a null value if the field is not primitive. Otherwise, we leave it intact.
-						if ( value != null || !isPrimitive( setter, propertyData, obj.getClass() ) ) {
-							setter.set( obj, value, null );
-						}
+		final Object value = data.get( propertyData.getName() );
 
-						return null;
+		if ( isDynamicComponentMap() ) {
+			@SuppressWarnings("unchecked")
+			final Map<String, Object> map = (Map<String, Object>) obj;
+			map.put( propertyData.getBeanName(), value );
+		}
+		else {
+			AccessController.doPrivileged(
+					new PrivilegedAction<Object>() {
+						@Override
+						public Object run() {
+							final Setter setter = ReflectionTools.getSetter(
+									obj.getClass(),
+									propertyData,
+									enversService.getServiceRegistry()
+							);
+
+							// We only set a null value if the field is not primitive. Otherwise, we leave it intact.
+							if ( value != null || !isPrimitive( setter, propertyData, obj.getClass() ) ) {
+								setter.set( obj, value, null );
+							}
+
+							return null;
+						}
 					}
-				}
-		);
+			);
+		}
 	}
 
 	private boolean isPrimitive(Setter setter, PropertyData propertyData, Class<?> cls) {

diff --git a/...s/src/main/java/org/hibernate/envers/internal/entities/mapper/SubclassPropertyMapper.java b/...s/src/main/java/org/hibernate/envers/internal/entities/mapper/SubclassPropertyMapper.java
@@ -25,7 +25,7 @@
  * @author Michal Skowronek (mskowr at o2 dot pl)
  * @author Chris Cranford
  */
-public class SubclassPropertyMapper implements ExtendedPropertyMapper {
+public class SubclassPropertyMapper extends AbstractPropertyMapper implements ExtendedPropertyMapper {
 	private ExtendedPropertyMapper main;
 	private ExtendedPropertyMapper parentMapper;