HHH-10851 - PropertyAccessMixed not aware of Access annotation

(cherry picked from commit 5ef1da74c2)

Conflicts:
	hibernate-core/src/main/java/org/hibernate/property/access/internal/PropertyAccessEnhancedImpl.java

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 {
+public class PropertyAccessEnhancedImpl extends PropertyAccessMixedImpl {
-	private final PropertyAccessStrategyEnhancedImpl strategy;
-
-	private final Getter getter;
-	private final Setter setter;
 
 	public PropertyAccessEnhancedImpl(
-			PropertyAccessStrategyEnhancedImpl strategy,
+			PropertyAccessStrategy strategy,
 			Class containerJavaType,
 			String propertyName) {
-		this.strategy = strategy;
+		super( strategy, containerJavaType, propertyName );
-
-		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 );
 	}
 
-	private static Field fieldOrNull(Class containerJavaType, String propertyName) {
+	@Override
-		try {
+	protected Setter fieldSetter(Class<?> containerJavaType, String propertyName, Field field) {
-			return ReflectHelper.findField( containerJavaType, propertyName );
+		return resolveEnhancedSetterForField( containerJavaType, propertyName, field );
-		}
-		catch (PropertyNotFoundException e) {
-			return null;
-		}
-	}
-
-	private static Method getterMethodOrNull(Class containerJavaType, String propertyName) {
-		try {
-			return ReflectHelper.findGetterMethod( containerJavaType, propertyName );
-		}
-		catch (PropertyNotFoundException e) {
-			return null;
-		}
 	}
 
 	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;
-	}
 }

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 );
+		AccessType propertyAccessType = getAccessType( containerJavaType, propertyName );
-		final Method getterMethod = getterMethodOrNull( 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
+				this.getter = propertyGetter( containerJavaType, propertyName, getterMethod );
-		if ( field == null && getterMethod == null ) {
+				this.setter = propertySetter( containerJavaType, propertyName, setterMethod );
-			throw new PropertyAccessBuildingException(
+				break;
-					"Could not locate field nor getter method for property named [" + containerJavaType.getName() +
+			}
-							"#" + propertyName + "]"
+			default: {
-			);
+				throw new PropertyAccessBuildingException(
-		}
+					"Invalid access type " + propertyAccessType + " 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 );
 		}
 	}
 
-	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;

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

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<String, String> params = new LinkedHashMap<>();
+
+		public TestEntity(String name) {
+			this();
+			this.name = name;
+		}
+
+		protected TestEntity() {
+		}
+
+		public Map<String, String> getParams() {
+			return params;
+		}
+
+		public void setParams(Map<String, String> 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<String, String>) 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" );
+				}
+			}
+		}
+	}
+}