From ac47825db226b6fea8437709fa626a2f59e7c34e Mon Sep 17 00:00:00 2001 From: barreiro Date: Mon, 13 Jun 2016 20:35:17 +0100 Subject: [PATCH] HHH-10851 - PropertyAccessMixed not aware of Access annotation (cherry picked from commit 5ef1da74c2d0c2566938758f772df2dfcf9ccad3) Conflicts: hibernate-core/src/main/java/org/hibernate/property/access/internal/PropertyAccessEnhancedImpl.java --- .../internal/PropertyAccessEnhancedImpl.java | 73 +--------- .../internal/PropertyAccessMixedImpl.java | 119 +++++++++++----- .../bytecode/enhancement/EnhancerTest.java | 7 + .../access/MixedAccessTestTask.java | 131 ++++++++++++++++++ 4 files changed, 225 insertions(+), 105 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/access/MixedAccessTestTask.java diff --git a/hibernate-core/src/main/java/org/hibernate/property/access/internal/PropertyAccessEnhancedImpl.java b/hibernate-core/src/main/java/org/hibernate/property/access/internal/PropertyAccessEnhancedImpl.java index adf92be39c..96d2b1abe8 100644 --- a/hibernate-core/src/main/java/org/hibernate/property/access/internal/PropertyAccessEnhancedImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/property/access/internal/PropertyAccessEnhancedImpl.java @@ -8,16 +8,8 @@ package org.hibernate.property.access.internal; import java.lang.reflect.Field; import java.lang.reflect.Method; - -import org.hibernate.PropertyNotFoundException; import org.hibernate.bytecode.enhance.spi.EnhancerConstants; -import org.hibernate.internal.util.ReflectHelper; import org.hibernate.property.access.spi.EnhancedSetterImpl; -import org.hibernate.property.access.spi.Getter; -import org.hibernate.property.access.spi.GetterFieldImpl; -import org.hibernate.property.access.spi.GetterMethodImpl; -import org.hibernate.property.access.spi.PropertyAccess; -import org.hibernate.property.access.spi.PropertyAccessBuildingException; import org.hibernate.property.access.spi.PropertyAccessStrategy; import org.hibernate.property.access.spi.Setter; import org.hibernate.property.access.spi.SetterFieldImpl; @@ -29,57 +21,18 @@ import org.hibernate.property.access.spi.SetterFieldImpl; * @author Steve Ebersole * @author Luis Barreiro */ -public class PropertyAccessEnhancedImpl implements PropertyAccess { - private final PropertyAccessStrategyEnhancedImpl strategy; - - private final Getter getter; - private final Setter setter; +public class PropertyAccessEnhancedImpl extends PropertyAccessMixedImpl { public PropertyAccessEnhancedImpl( - PropertyAccessStrategyEnhancedImpl strategy, + PropertyAccessStrategy strategy, Class containerJavaType, String propertyName) { - this.strategy = strategy; - - final Field field = fieldOrNull( containerJavaType, propertyName ); - final Method getterMethod = getterMethodOrNull( containerJavaType, propertyName ); - - // need one of field or getterMethod to be non-null - if ( field == null && getterMethod == null ) { - throw new PropertyAccessBuildingException( - String.format( - "Could not locate field for property [%s] on bytecode-enhanced Class [%s]", - propertyName, - containerJavaType.getName() - ) - ); - } - else if ( field != null ) { - this.getter = new GetterFieldImpl( containerJavaType, propertyName, field ); - } - else { - this.getter = new GetterMethodImpl( containerJavaType, propertyName, getterMethod ); - } - - this.setter = resolveEnhancedSetterForField( containerJavaType, propertyName, field ); + super( strategy, containerJavaType, propertyName ); } - private static Field fieldOrNull(Class containerJavaType, String propertyName) { - try { - return ReflectHelper.findField( containerJavaType, propertyName ); - } - catch (PropertyNotFoundException e) { - return null; - } - } - - private static Method getterMethodOrNull(Class containerJavaType, String propertyName) { - try { - return ReflectHelper.findGetterMethod( containerJavaType, propertyName ); - } - catch (PropertyNotFoundException e) { - return null; - } + @Override + protected Setter fieldSetter(Class containerJavaType, String propertyName, Field field) { + return resolveEnhancedSetterForField( containerJavaType, propertyName, field ); } private static Setter resolveEnhancedSetterForField(Class containerClass, String propertyName, Field field) { @@ -95,18 +48,4 @@ public class PropertyAccessEnhancedImpl implements PropertyAccess { } } - @Override - public PropertyAccessStrategy getPropertyAccessStrategy() { - return strategy; - } - - @Override - public Getter getGetter() { - return getter; - } - - @Override - public Setter getSetter() { - return setter; - } } diff --git a/hibernate-core/src/main/java/org/hibernate/property/access/internal/PropertyAccessMixedImpl.java b/hibernate-core/src/main/java/org/hibernate/property/access/internal/PropertyAccessMixedImpl.java index ac3f07c995..c38a61660c 100644 --- a/hibernate-core/src/main/java/org/hibernate/property/access/internal/PropertyAccessMixedImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/property/access/internal/PropertyAccessMixedImpl.java @@ -6,9 +6,12 @@ */ package org.hibernate.property.access.internal; +import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Field; import java.lang.reflect.Method; +import javax.persistence.Access; +import javax.persistence.AccessType; import org.hibernate.PropertyNotFoundException; import org.hibernate.internal.util.ReflectHelper; import org.hibernate.property.access.spi.Getter; @@ -27,56 +30,53 @@ import org.hibernate.property.access.spi.SetterMethodImpl; * @author Steve Ebersole */ public class PropertyAccessMixedImpl implements PropertyAccess { - private final PropertyAccessStrategyMixedImpl strategy; + private final PropertyAccessStrategy strategy; private final Getter getter; private final Setter setter; public PropertyAccessMixedImpl( - PropertyAccessStrategyMixedImpl strategy, + PropertyAccessStrategy strategy, Class containerJavaType, String propertyName) { this.strategy = strategy; - final Field field = fieldOrNull( containerJavaType, propertyName ); - final Method getterMethod = getterMethodOrNull( containerJavaType, propertyName ); + AccessType propertyAccessType = getAccessType( containerJavaType, propertyName ); - final Class propertyJavaType; + switch ( propertyAccessType ) { + case FIELD: { + Field field = fieldOrNull( containerJavaType, propertyName ); + if ( field == null ) { + throw new PropertyAccessBuildingException( + "Could not locate field for property named [" + containerJavaType.getName() + "#" + propertyName + "]" + ); + } + this.getter = fieldGetter( containerJavaType, propertyName, field ); + this.setter = fieldSetter( containerJavaType, propertyName, field ); + break; + } + case PROPERTY: { + Method getterMethod = getterMethodOrNull( containerJavaType, propertyName ); + if ( getterMethod == null ) { + throw new PropertyAccessBuildingException( + "Could not locate getter for property named [" + containerJavaType.getName() + "#" + propertyName + "]" + ); + } + Method setterMethod = setterMethodOrNull( containerJavaType, propertyName, getterMethod.getReturnType() ); - // need one of field or getterMethod to be non-null - if ( field == null && getterMethod == null ) { - throw new PropertyAccessBuildingException( - "Could not locate field nor getter method for property named [" + containerJavaType.getName() + - "#" + propertyName + "]" - ); - } - else if ( field != null ) { - propertyJavaType = field.getType(); - this.getter = new GetterFieldImpl( containerJavaType, propertyName, field ); - } - else { - propertyJavaType = getterMethod.getReturnType(); - this.getter = new GetterMethodImpl( containerJavaType, propertyName, getterMethod ); - } - - final Method setterMethod = setterMethodOrNull( containerJavaType, propertyName, propertyJavaType ); - - // need one of field or setterMethod to be non-null - if ( field == null && setterMethod == null ) { - throw new PropertyAccessBuildingException( - "Could not locate field nor setter method for property named [" + containerJavaType.getName() + - "#" + propertyName + "]" - ); - } - else if ( field != null ) { - this.setter = new SetterFieldImpl( containerJavaType, propertyName, field ); - } - else { - this.setter = new SetterMethodImpl( containerJavaType, propertyName, setterMethod ); + this.getter = propertyGetter( containerJavaType, propertyName, getterMethod ); + this.setter = propertySetter( containerJavaType, propertyName, setterMethod ); + break; + } + default: { + throw new PropertyAccessBuildingException( + "Invalid access type " + propertyAccessType + " for property named [" + containerJavaType.getName() + "#" + propertyName + "]" + ); + } } } - private static Field fieldOrNull(Class containerJavaType, String propertyName) { + protected static Field fieldOrNull(Class containerJavaType, String propertyName) { try { return ReflectHelper.findField( containerJavaType, propertyName ); } @@ -85,7 +85,7 @@ public class PropertyAccessMixedImpl implements PropertyAccess { } } - private static Method getterMethodOrNull(Class containerJavaType, String propertyName) { + protected static Method getterMethodOrNull(Class containerJavaType, String propertyName) { try { return ReflectHelper.findGetterMethod( containerJavaType, propertyName ); } @@ -94,7 +94,7 @@ public class PropertyAccessMixedImpl implements PropertyAccess { } } - private static Method setterMethodOrNull(Class containerJavaType, String propertyName, Class propertyJavaType) { + protected static Method setterMethodOrNull(Class containerJavaType, String propertyName, Class propertyJavaType) { try { return ReflectHelper.findSetterMethod( containerJavaType, propertyName, propertyJavaType ); } @@ -103,6 +103,49 @@ public class PropertyAccessMixedImpl implements PropertyAccess { } } + protected static AccessType getAccessType(Class containerJavaType, String propertyName) { + AccessType classAccessType = getAccessTypeOrNull( containerJavaType ); + if ( classAccessType != null ) { + return classAccessType; + } + Field field = fieldOrNull( containerJavaType, propertyName ); + AccessType fieldAccessType = getAccessTypeOrNull( field ); + if ( fieldAccessType != null ) { + return fieldAccessType; + } + AccessType methodAccessType = getAccessTypeOrNull( getterMethodOrNull( containerJavaType, propertyName ) ); + if ( methodAccessType != null ) { + return methodAccessType; + } + return field != null ? AccessType.FIELD : AccessType.PROPERTY; + } + + private static AccessType getAccessTypeOrNull(AnnotatedElement element) { + if ( element == null ) { + return null; + } + Access elementAccess = element.getAnnotation( Access.class ); + return elementAccess == null ? null : elementAccess.value(); + } + + // --- // + + protected Getter fieldGetter(Class containerJavaType, String propertyName, Field field) { + return new GetterFieldImpl( containerJavaType, propertyName, field ); + } + + protected Setter fieldSetter(Class containerJavaType, String propertyName, Field field) { + return new SetterFieldImpl( containerJavaType, propertyName, field ); + } + + protected Getter propertyGetter(Class containerJavaType, String propertyName, Method method) { + return new GetterMethodImpl( containerJavaType, propertyName, method ); + } + + protected Setter propertySetter(Class containerJavaType, String propertyName, Method method) { + return method == null ? null : new SetterMethodImpl( containerJavaType, propertyName, method ); + } + @Override public PropertyAccessStrategy getPropertyAccessStrategy() { return strategy; diff --git a/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/EnhancerTest.java b/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/EnhancerTest.java index f338e55ec4..7f0a90426c 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/EnhancerTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/EnhancerTest.java @@ -7,6 +7,7 @@ package org.hibernate.test.bytecode.enhancement; import org.hibernate.test.bytecode.enhancement.eviction.EvictionTestTask; +import org.hibernate.test.bytecode.enhancement.access.MixedAccessTestTask; import org.hibernate.testing.FailureExpected; import org.hibernate.testing.TestForIssue; import org.hibernate.testing.junit4.BaseUnitTestCase; @@ -55,6 +56,12 @@ public class EnhancerTest extends BaseUnitTestCase { EnhancerTestUtils.runEnhancerTestTask( HHH9529TestTask.class ); } + @Test + @TestForIssue( jiraKey = "HHH-10851" ) + public void testAccess() { + EnhancerTestUtils.runEnhancerTestTask( MixedAccessTestTask.class ); + } + @Test public void testDirty() { EnhancerTestUtils.runEnhancerTestTask( DirtyTrackingTestTask.class ); diff --git a/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/access/MixedAccessTestTask.java b/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/access/MixedAccessTestTask.java new file mode 100644 index 0000000000..56581540c2 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/access/MixedAccessTestTask.java @@ -0,0 +1,131 @@ +package org.hibernate.test.bytecode.enhancement.access; + +import org.hibernate.Session; +import org.hibernate.cfg.Configuration; +import org.hibernate.cfg.Environment; +import org.hibernate.test.bytecode.enhancement.AbstractEnhancerTestTask; +import org.junit.Assert; + +import javax.persistence.Access; +import javax.persistence.AccessType; +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.Transient; +import javax.script.ScriptEngine; +import javax.script.ScriptEngineManager; +import javax.script.ScriptException; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.stream.Collectors; + +/** + * @author Luis Barreiro + */ +public class MixedAccessTestTask extends AbstractEnhancerTestTask { + + private static ScriptEngine engine = new ScriptEngineManager().getEngineByName( "javascript" ); + private static boolean cleanup = false; + + public Class[] getAnnotatedClasses() { + return new Class[]{TestEntity.class}; + } + + public void prepare() { + Configuration cfg = new Configuration(); + cfg.setProperty( Environment.ENABLE_LAZY_LOAD_NO_TRANS, "true" ); + cfg.setProperty( Environment.USE_SECOND_LEVEL_CACHE, "false" ); + super.prepare( cfg ); + + Session s = getFactory().openSession(); + s.beginTransaction(); + + TestEntity testEntity = new TestEntity( "foo" ); + testEntity.setParamsAsString( "{\"paramName\":\"paramValue\"}" ); + s.persist( testEntity ); + + s.getTransaction().commit(); + s.clear(); + s.close(); + } + + public void execute() { + Session s = getFactory().openSession(); + s.beginTransaction(); + + TestEntity testEntity = s.get( TestEntity.class, "foo" ); + Assert.assertEquals( "{\"paramName\":\"paramValue\"}", testEntity.getParamsAsString() ); + + // Clean parameters + cleanup = true; + testEntity.setParamsAsString( "{}" ); + s.persist( testEntity ); + + s.getTransaction().commit(); + s.clear(); + s.close(); + } + + protected void cleanup() { + Session s = getFactory().openSession(); + s.beginTransaction(); + + TestEntity testEntity = s.get( TestEntity.class, "foo" ); + Assert.assertTrue( testEntity.getParams().isEmpty() ); + + s.getTransaction().commit(); + s.clear(); + s.close(); + } + + @Entity + private static class TestEntity { + + @Id + String name; + + @Transient + Map params = new LinkedHashMap<>(); + + public TestEntity(String name) { + this(); + this.name = name; + } + + protected TestEntity() { + } + + public Map getParams() { + return params; + } + + public void setParams(Map params) { + this.params = params; + } + + @Column( name = "params", length = 4000 ) + @Access( AccessType.PROPERTY ) + public String getParamsAsString() { + if ( params.size() > 0 ) { + // Convert to JSON + return "{" + params.entrySet().stream().map( + e -> "\"" + e.getKey() + "\":\"" + e.getValue() + "\"" + ).collect( Collectors.joining( "," ) ) + "}"; + } + return null; + } + + public void setParamsAsString(String string) { + params.clear(); + + try { + params.putAll( (Map) engine.eval( "Java.asJSONCompatible(" + string + ")" ) ); + } catch ( ScriptException ignore ) { + // JDK 8u60 required --- use hard coded values to pass the test + if ( !cleanup ) { + params.put( "paramName", "paramValue" ); + } + } + } + } +}