HHH-4527 - added handling of proper default access propagation for class hierarchies and embedded classes (components)

git-svn-id: https://svn.jboss.org/repos/hibernate/core/trunk@18417 1b8cb986-b30d-0410-93ca-fae66ebed9b2

annotations/src/main/java/org/hibernate/cfg/AnnotationBinder.java

@@ -35,7 +35,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 import java.util.Set;
-import javax.persistence.Access;
 import javax.persistence.Basic;
 import javax.persistence.Column;
 import javax.persistence.DiscriminatorType;
@@ -191,8 +190,6 @@ public final class AnnotationBinder {
 	 * bindSomething usually create the mapping container and is accessed by one of the 2 first level method
 	 * makeSomething usually create the mapping container and is accessed by bindSomething[else]
 	 * fillSomething take the container into parameter and fill it.
-	 *
-	 *
 	 */
 	private AnnotationBinder() {
 	}
@@ -618,7 +615,7 @@ public final class AnnotationBinder {
 			);
 		}
 
-		//try to find class level generators
+		// try to find class level generators
 		HashMap<String, IdGenerator> classGenerators = buildLocalGenerators( clazzToProcess, mappings );
 
 		// check properties
@@ -626,9 +623,6 @@ public final class AnnotationBinder {
 				getElementsToProcess(
 						persistentClass, clazzToProcess, inheritanceStatePerClass, entityBinder, mappings
 				);
-		if ( elements == null ) {
-			throw new AnnotationException( "No identifier specified for entity: " + propertyHolder.getEntityName() );
-		}
 		final boolean subclassAndSingleTableStrategy = inheritanceState.getType() == InheritanceType.SINGLE_TABLE
 				&& inheritanceState.hasParents();
 		//process idclass if any
@@ -644,9 +638,7 @@ public final class AnnotationBinder {
 					idClass = current.getAnnotation( IdClass.class );
 					break;
 				}
-				state = InheritanceState.getSuperclassInheritanceState(
+				state = InheritanceState.getSuperclassInheritanceState( current, inheritanceStatePerClass );
-						current, inheritanceStatePerClass, mappings.getReflectionManager()
-				);
 			}
 			while ( state != null );
 		}
@@ -657,10 +649,10 @@ public final class AnnotationBinder {
 			String generatorType = "assigned";
 			String generator = BinderHelper.ANNOTATION_STRING_DEFAULT;
 			PropertyData inferredData = new PropertyPreloadedData(
-					entityBinder.getPropertyAccessor(), "id", compositeClass
+					entityBinder.getPropertyAccessType(), "id", compositeClass
 			);
 			PropertyData baseInferredData = new PropertyPreloadedData(
-                  entityBinder.getPropertyAccessor(), "id", current
+                  entityBinder.getPropertyAccessType(), "id", current
             );
 			HashMap<String, IdGenerator> localGenerators = new HashMap<String, IdGenerator>();
 			boolean ignoreIdAnnotations = entityBinder.isIgnoreIdAnnotations();
@@ -834,11 +826,7 @@ public final class AnnotationBinder {
 	}
 
 	private static PersistentClass getSuperEntity(XClass clazzToProcess, Map<XClass, InheritanceState> inheritanceStatePerClass, ExtendedMappings mappings, InheritanceState inheritanceState) {
-		final ReflectionManager reflectionManager = mappings.getReflectionManager();
+		InheritanceState superEntityState = InheritanceState.getInheritanceStateOfSuperEntity( clazzToProcess, inheritanceStatePerClass );
-		InheritanceState superEntityState =
-				InheritanceState.getInheritanceStateOfSuperEntity(
-						clazzToProcess, inheritanceStatePerClass, reflectionManager
-				);
 		PersistentClass superEntity = superEntityState != null ?
 				mappings.getClass(
 						superEntityState.getClazz().getName()
@@ -880,8 +868,7 @@ public final class AnnotationBinder {
 	}
 
 	/*
-	 * Get the annotated elements
+	 * Get the annotated elements, guessing the access type from @Id or @EmbeddedId presence.
-	 * Guess the annotated element from @Id or @EmbeddedId presence
 	 * Change EntityBinder by side effect
 	 */
 	private static List<PropertyData> getElementsToProcess(
@@ -892,104 +879,50 @@ public final class AnnotationBinder {
 		InheritanceState inheritanceState = inheritanceStatePerClass.get( clazzToProcess );
 		assert !inheritanceState.isEmbeddableSuperclass();
 
-		AccessType accessType = determineExplicitAccessType(
-				clazzToProcess, inheritanceStatePerClass, mappings, inheritanceState
-		);
 
 		List<XClass> classesToProcess = getMappedSuperclassesTillNextEntityOrdered(
 				persistentClass, clazzToProcess, inheritanceStatePerClass, mappings
 		);
+
+		AccessType accessType = determineDefaultAccessType( clazzToProcess, inheritanceStatePerClass );
+
 		List<PropertyData> elements = new ArrayList<PropertyData>();
 		int deep = classesToProcess.size();
 		boolean hasIdentifier = false;
 
-		/*
+		for ( int index = 0; index < deep; index++ ) {
-		 * delay the exception in case field access is used
+			PropertyContainer properyContainer = new PropertyContainer( classesToProcess.get( index ) );
-		 */
+			boolean currentHasIdentifier = addElementsOfClass( elements, accessType, properyContainer, mappings );
-		AnnotationException exceptionWhileWalkingElements = null;
+			hasIdentifier = hasIdentifier || currentHasIdentifier;
-		try {
-			for (int index = 0; index < deep; index++) {
-				PropertyContainer properyContainer = new PropertyContainer( classesToProcess.get( index ) );
-				boolean currentHasIdentifier = addElementsOfClass( elements, accessType , properyContainer, mappings );
-				hasIdentifier = hasIdentifier || currentHasIdentifier;
-			}
-		}
-		catch ( AnnotationException e ) {
-			exceptionWhileWalkingElements = e;
 		}
 
+		entityBinder.setPropertyAccessType( accessType );
+
 		if ( !hasIdentifier && !inheritanceState.hasParents() ) {
-			if (  AccessType.PROPERTY.equals( accessType ) ) {
+			throw new AnnotationException( "No identifier specified for entity: " + clazzToProcess.getName() );
-				//the original exception is legitimate
-				if ( exceptionWhileWalkingElements != null) throw exceptionWhileWalkingElements;
-				return null; //explicit but no @Id: the upper layer will raise an exception
-			}
-			accessType = AccessType.FIELD;
-			elements.clear();
-			for (int index = 0; index < deep; index++) {
-				PropertyContainer properyContainer = new PropertyContainer( classesToProcess.get( index ) );
-				boolean currentHasIdentifier = addElementsOfClass(elements, accessType, properyContainer, mappings );
-				hasIdentifier = hasIdentifier || currentHasIdentifier;
-			}
 		}
 
-		//the field show no id, fallback to he original exception
+		return elements;
-		if (!hasIdentifier && exceptionWhileWalkingElements != null) throw exceptionWhileWalkingElements;
-
-		entityBinder.setPropertyAccessor( accessType );
-		inheritanceState.setAccessType( accessType );
-		return hasIdentifier || inheritanceState.hasParents() ?
-				elements :
-				null;
 	}
 
-	/*
+	private static AccessType determineDefaultAccessType(XClass annotatedClass, Map<XClass, InheritanceState> inheritanceStatePerClass) {
-	 * Check whether either the class itself or any of its super classes explicitly defines a value access strategy.
+		XClass xclass = annotatedClass;
-	 *
+		while ( xclass != null && !Object.class.getName().equals( xclass.getName() ) ) {
-	 * @return {@code AccessType.FIELD} or {@code AccessType.PROPERTY} in case there is an explicit value,
+			if ( xclass.isAnnotationPresent( Entity.class ) || xclass.isAnnotationPresent( MappedSuperclass.class ) ) {
-	 * {@code AccessType.DEFAULT} otherwise.
+				for ( XProperty prop : xclass.getDeclaredProperties( AccessType.PROPERTY.getType() ) ) {
-	 */
+					if ( prop.isAnnotationPresent( Id.class ) || prop.isAnnotationPresent( EmbeddedId.class ) ) {
-	private static AccessType determineExplicitAccessType(XClass clazzToProcess, Map<XClass, InheritanceState> inheritanceStatePerClass, ExtendedMappings mappings, InheritanceState inheritanceState) {
+						return AccessType.PROPERTY;
-		AccessType explicitAccessType = AccessType.DEFAULT;
+					}
-
+				}
-		// check whether any of the super classes or the class itself
+				for ( XProperty prop : xclass.getDeclaredProperties( AccessType.FIELD.getType() ) ) {
-		if ( inheritanceState.hasParents() ) {
+					if ( prop.isAnnotationPresent( Id.class ) || prop.isAnnotationPresent( EmbeddedId.class ) ) {
-			InheritanceState superEntityState =
+						return AccessType.FIELD;
-					InheritanceState.getInheritanceStateOfSuperEntity(
+					}
-							clazzToProcess, inheritanceStatePerClass, mappings.getReflectionManager()
+				}
-					);
-			if ( superEntityState != null ) {
-				explicitAccessType = superEntityState.getAccessType();
 			}
+			xclass = xclass.getSuperclass();
 		}
-		else {
+		throw new AnnotationException( "No identifier specified for entity: " + annotatedClass.getName() );
-			AccessType hibernateExplicitAccessType = AccessType.DEFAULT;
-		    AccessType jpaExplicitAccessType = AccessType.DEFAULT;
-
-			//the are the root entity but we might have mapped superclasses that contain the id class
-			org.hibernate.annotations.AccessType accessType = clazzToProcess.getAnnotation( org.hibernate.annotations.AccessType.class );
-			if ( accessType != null ) {
-				hibernateExplicitAccessType = AccessType.getAccessStrategy( accessType.value() );
-			}
-
-			Access access = clazzToProcess.getAnnotation( Access.class );
-			if( access != null ) {
-				jpaExplicitAccessType = AccessType.getAccessStrategy( access.value() );
-			}
-
-			if ( hibernateExplicitAccessType != AccessType.DEFAULT
-					&& jpaExplicitAccessType != AccessType.DEFAULT
-					&& hibernateExplicitAccessType != jpaExplicitAccessType ) {
-				throw new MappingException( "@AccessType and @Access specified with contradicting values. Use of @Access only is recommended. " );
-			}
-
-			if(hibernateExplicitAccessType != AccessType.DEFAULT) {
-				explicitAccessType = hibernateExplicitAccessType;
-			}  else {
-				explicitAccessType = jpaExplicitAccessType;
-			}
-		}
-		return explicitAccessType;
 	}
 
 	private static List<XClass> getMappedSuperclassesTillNextEntityOrdered(
@@ -1021,7 +954,7 @@ public final class AnnotationBinder {
 		// classes from 0 to n-1 are @MappedSuperclass and should be linked
 		org.hibernate.mapping.MappedSuperclass mappedSuperclass = null;
 		final InheritanceState superEntityState =
-				InheritanceState.getInheritanceStateOfSuperEntity(annotatedClass, inheritanceStatePerClass, reflectionManager);
+				InheritanceState.getInheritanceStateOfSuperEntity( annotatedClass, inheritanceStatePerClass );
 		PersistentClass superEntity =
 				superEntityState != null ?
 						mappings.getClass( superEntityState.getClazz().getName() ) :
@@ -1162,7 +1095,7 @@ public final class AnnotationBinder {
 	/**
 	 *
 	 * @param elements List of {@code ProperyData} instances
-	 * @param propertyAccessor The default value access strategy which has to be used in case no explicit local access
+	 * @param defaultAccessType The default value access strategy which has to be used in case no explicit local access
 	 *        strategy is used
 	 * @param propertyContainer Metadata about a class and its properties
 	 * @param mappings Mapping meta data
@@ -1170,19 +1103,20 @@ public final class AnnotationBinder {
 	 * the determined access strategy, {@code false} otherwise.
 	 */
 	private static boolean addElementsOfClass(
-			List<PropertyData> elements, AccessType propertyAccessor, PropertyContainer propertyContainer, ExtendedMappings mappings
+			List<PropertyData> elements, AccessType defaultAccessType, PropertyContainer propertyContainer, ExtendedMappings mappings
 	) {
 		boolean hasIdentifier = false;
-		AccessType classDefinedAccessType = propertyContainer.getDefaultAccessStrategy();
+		AccessType accessType = defaultAccessType;
 
-		if ( classDefinedAccessType.equals( AccessType.DEFAULT ) ) {
+		if ( propertyContainer.hasExplicitAccessStrategy() ) {
-			classDefinedAccessType = propertyAccessor;
+			accessType = propertyContainer.getExplicitAccessStrategy();
 		}
 
-		Collection<XProperty> properties = propertyContainer.getProperties( classDefinedAccessType );
+		propertyContainer.assertTypesAreResolvable( accessType );
+		Collection<XProperty> properties = propertyContainer.getProperties( accessType );
 		for ( XProperty p : properties ) {
 			final boolean currentHasIdentifier = addProperty(
-					propertyContainer.getXClass(), p, elements, classDefinedAccessType.getType(), mappings
+					propertyContainer.getXClass(), p, elements, accessType.getType(), mappings
 			);
 			hasIdentifier = hasIdentifier || currentHasIdentifier;
 		}
@@ -1193,7 +1127,7 @@ public final class AnnotationBinder {
 			XClass declaringClass, XProperty property, List<PropertyData> annElts,
 			String propertyAccessor, ExtendedMappings mappings
 	) {
-		boolean hasIdentifier = false;
+		boolean hasIdentifier;
 		PropertyData propertyAnnotatedElement = new PropertyInferredData(
 				declaringClass, property, propertyAccessor,
 				mappings.getReflectionManager() );
@@ -2048,7 +1982,6 @@ public final class AnnotationBinder {
 
 		List<PropertyData> baseClassElements = null;
 		XClass baseReturnedClassOrElement;
-		PropertyHolder baseSubHolder;
 		if(baseInferredData != null)
 		{
 		   baseClassElements = new ArrayList<PropertyData>();
@@ -2567,29 +2500,22 @@ public final class AnnotationBinder {
 	 * inheritance status of a class.
 	 *
 	 * @param orderedClasses Order list of all annotated entities and their mapped superclasses
-	 * @param reflectionManager  Reference to the reflection manager (commons-annotations)
 	 * @return A map of {@code InheritanceState}s keyed against their {@code XClass}.
 	 */
-	public static Map<XClass, InheritanceState> buildInheritanceStates(
+	public static Map<XClass, InheritanceState> buildInheritanceStates(List<XClass> orderedClasses) {
-			List<XClass> orderedClasses, ReflectionManager reflectionManager
-	) {
 		Map<XClass, InheritanceState> inheritanceStatePerClass = new HashMap<XClass, InheritanceState>(
 				orderedClasses.size()
 		);
 		for (XClass clazz : orderedClasses) {
 			InheritanceState superclassState = InheritanceState.getSuperclassInheritanceState(
-					clazz, inheritanceStatePerClass,
+					clazz, inheritanceStatePerClass );
-					reflectionManager
-			);
 			InheritanceState state = new InheritanceState( clazz );
 			if ( superclassState != null ) {
 				//the classes are ordered thus preventing an NPE
 				//FIXME if an entity has subclasses annotated @MappedSperclass wo sub @Entity this is wrong
 				superclassState.setHasSiblings( true );
 				InheritanceState superEntityState = InheritanceState.getInheritanceStateOfSuperEntity(
-						clazz, inheritanceStatePerClass,
+						clazz, inheritanceStatePerClass );
-						reflectionManager
-				);
 				state.setHasParents( superEntityState != null );
 				final boolean nonDefault = state.getType() != null && !InheritanceType.SINGLE_TABLE.equals( state.getType() );
 				if ( superclassState.getType() != null ) {

annotations/src/main/java/org/hibernate/cfg/AnnotationConfiguration.java

@@ -611,7 +611,7 @@ public class AnnotationConfiguration extends Configuration {
 			//bind classes in the correct order calculating some inheritance state
 			List<XClass> orderedClasses = orderAndFillHierarchy( annotatedClasses );
 			Map<XClass, InheritanceState> inheritanceStatePerClass = AnnotationBinder.buildInheritanceStates(
-					orderedClasses, reflectionManager
+					orderedClasses
 			);
 			ExtendedMappings mappings = createExtendedMappings();
 

annotations/src/main/java/org/hibernate/cfg/InheritanceState.java

@@ -28,7 +28,6 @@ import javax.persistence.Inheritance;
 import javax.persistence.InheritanceType;
 import javax.persistence.MappedSuperclass;
 
-import org.hibernate.annotations.common.reflection.ReflectionManager;
 import org.hibernate.annotations.common.reflection.XAnnotatedElement;
 import org.hibernate.annotations.common.reflection.XClass;
 
@@ -57,11 +56,6 @@ public class InheritanceState {
 	private InheritanceType type;
 	private boolean isEmbeddableSuperclass = false;
 
-	/**
-	 * only defined on embedded superclasses
-	 */
-	private AccessType accessType = AccessType.DEFAULT;
-
 	private void extractInheritanceType() {
 		XAnnotatedElement element = getClazz();
 		Inheritance inhAnn = element.getAnnotation( Inheritance.class );
@@ -84,8 +78,7 @@ public class InheritanceState {
 	}
 
 	public static InheritanceState getInheritanceStateOfSuperEntity(
-			XClass clazz, Map<XClass, InheritanceState> states,
+			XClass clazz, Map<XClass, InheritanceState> states
-			ReflectionManager reflectionManager
 	) {
 		XClass superclass = clazz;
 		do {
@@ -95,14 +88,11 @@ public class InheritanceState {
 				return currentState;
 			}
 		}
-		while ( superclass != null && !reflectionManager.equals( superclass, Object.class ) );
+		while ( superclass != null && !Object.class.getName().equals( superclass.getName() ) );
 		return null;
 	}
 
-	public static InheritanceState getSuperclassInheritanceState(
+	public static InheritanceState getSuperclassInheritanceState( XClass clazz, Map<XClass, InheritanceState> states) {
-			XClass clazz, Map<XClass, InheritanceState> states,
-			ReflectionManager reflectionManager
-	) {
 		XClass superclass = clazz;
 		do {
 			superclass = superclass.getSuperclass();
@@ -111,7 +101,7 @@ public class InheritanceState {
 				return currentState;
 			}
 		}
-		while ( superclass != null && !reflectionManager.equals( superclass, Object.class ) );
+		while ( superclass != null && !Object.class.getName().equals( superclass.getName() ) );
 		return null;
 	}
 
@@ -154,12 +144,4 @@ public class InheritanceState {
 	public void setEmbeddableSuperclass(boolean embeddableSuperclass) {
 		isEmbeddableSuperclass = embeddableSuperclass;
 	}
-
-	public AccessType getAccessType() {
-		return accessType;
-	}
-
-	public void setAccessType(AccessType type) {
-		this.accessType = type;
-	}
 }

annotations/src/main/java/org/hibernate/cfg/PropertyContainer.java

@@ -45,38 +45,39 @@ import org.hibernate.annotations.Target;
 import org.hibernate.annotations.Type;
 import org.hibernate.annotations.common.reflection.XClass;
 import org.hibernate.annotations.common.reflection.XProperty;
+import org.hibernate.util.StringHelper;
 
 /**
+ * A helper class to keep the {@code XProperty}s of a class ordered by access type.
+ *
  * @author Hardy Ferentschik
  */
-
-/**
- * A temporary class where we keep the {@code XProperty}s of a class ordered by access type.
- */
 class PropertyContainer {
 
 	private static final Logger log = LoggerFactory.getLogger( AnnotationBinder.class );
 	private final TreeMap<String, XProperty> fieldAccessMap;
 	private final TreeMap<String, XProperty> propertyAccessMap;
 	private final XClass xClass;
-	private final AccessType defaultAccessType;
+	private final AccessType explicitClassDefinedAccessType;
 
 	PropertyContainer(XClass clazz) {
 		this.xClass = clazz;
 		fieldAccessMap = initProperties( AccessType.FIELD );
 		propertyAccessMap = initProperties( AccessType.PROPERTY );
-		defaultAccessType = determineClassDefinedAccessStrategy();
+		explicitClassDefinedAccessType = determineClassDefinedAccessStrategy();
 		checkForJpaAccess();
-
-
 	}
 
 	public XClass getXClass() {
 		return xClass;
 	}
 
-	public AccessType getDefaultAccessStrategy() {
+	public AccessType getExplicitAccessStrategy() {
-		return defaultAccessType;
+		return explicitClassDefinedAccessType;
+	}
+
+	public boolean hasExplicitAccessStrategy() {
+		return !explicitClassDefinedAccessType.equals( AccessType.DEFAULT );
 	}
 
 	public Collection<XProperty> getProperties(AccessType accessType) {
@@ -88,6 +89,24 @@ class PropertyContainer {
 		}
 	}
 
+	public void assertTypesAreResolvable(AccessType access) {
+		TreeMap<String, XProperty> xprops;
+		if ( AccessType.PROPERTY.equals( access ) || AccessType.DEFAULT.equals( access ) ) {
+			xprops = propertyAccessMap;
+		}
+		else {
+			xprops = fieldAccessMap;
+		}
+		for ( XProperty property : xprops.values() ) {
+			if ( !property.isTypeResolved() && !discoverTypeWithoutReflection( property ) ) {
+				String msg = "Property " + StringHelper.qualify( xClass.getName(), property.getName() ) +
+						" has an unbound type and no explicit target entity. Resolve this Generic usage issue" +
+						" or set an explicit target attribute (eg @OneToMany(target=) or use an explicit @Type";
+				throw new AnnotationException( msg );
+			}
+		}
+	}
+
 	private void checkForJpaAccess() {
 		List<XProperty> tmpList = new ArrayList<XProperty>();
 		for ( XProperty property : fieldAccessMap.values() ) {
@@ -136,16 +155,10 @@ class PropertyContainer {
 		TreeMap<String, XProperty> propertiesMap = new TreeMap<String, XProperty>();
 		List<XProperty> properties = xClass.getDeclaredProperties( access.getType() );
 		for ( XProperty property : properties ) {
-//				if ( !property.isTypeResolved() && !discoverTypeWithoutReflection( property )
+			if ( mustBeSkipped( property ) ) {
-//						&& !mustBeSkipped( property ) ) {
+				continue;
-//					String msg = "Property " + StringHelper.qualify( xClass.getName(), property.getName() ) +
-//							" has an unbound type and no explicit target entity. Resolve this Generic usage issue" +
-//							" or set an explicit target attribute (eg @OneToMany(target=) or use an explicit @Type";
-//					throw new AnnotationException( msg );
-//				}
-			if ( !mustBeSkipped( property ) ) {
-				propertiesMap.put( property.getName(), property );
 			}
+			propertiesMap.put( property.getName(), property );
 		}
 		return propertiesMap;
 	}

annotations/src/main/java/org/hibernate/cfg/annotations/EntityBinder.java

@@ -118,7 +118,7 @@ public class EntityBinder {
 	private InheritanceState inheritanceState;
 	private boolean ignoreIdAnnotations;
 	private boolean cacheLazyProperty;
-	private AccessType propertyAccessor = AccessType.DEFAULT;
+	private AccessType propertyAccessType = AccessType.DEFAULT;
 
 	/**
 	 * Use as a fake one for Collection of elements
@@ -843,16 +843,28 @@ public class EntityBinder {
 		}
 	}
 
-	public AccessType getPropertyAccessor() {
+	public AccessType getPropertyAccessType() {
-		return propertyAccessor;
+		return propertyAccessType;
 	}
 
-	public void setPropertyAccessor(AccessType propertyAccessor) {
+	public void setPropertyAccessType(AccessType propertyAccessor) {
-		this.propertyAccessor = propertyAccessor;
+		this.propertyAccessType = getExplicitAccessType( annotatedClass );
+		// only set the access type if there is no explicit access type for this class
+		if( this.propertyAccessType == null ) {
+			this.propertyAccessType = propertyAccessor;
+		}
 	}
 
 	public AccessType getPropertyAccessor(XAnnotatedElement element) {
-		AccessType accessType = propertyAccessor;
+		AccessType accessType = getExplicitAccessType( element );
+		if ( accessType == null ) {
+		   accessType = propertyAccessType;
+		}
+		return accessType;
+	}
+
+	public AccessType getExplicitAccessType(XAnnotatedElement element) {
+		AccessType accessType = null;
 
 		AccessType hibernateAccessType = null;
 		AccessType jpaAccessType = null;
@@ -868,7 +880,9 @@ public class EntityBinder {
 		}
 
 		if ( hibernateAccessType != null && jpaAccessType != null && hibernateAccessType != jpaAccessType ) {
-			throw new MappingException( " " );
+			throw new MappingException(
+					"Found @Access and @AccessType with conflicting values on a property in class " + annotatedClass.toString()
+			);
 		}
 
 		if ( hibernateAccessType != null ) {

annotations/src/test/java/org/hibernate/test/annotations/access/jpa/AccessMappingTest.java

@@ -43,7 +43,7 @@ public class AccessMappingTest extends TestCase {
 
 	public void testInconsistentAnnotationPlacement() throws Exception {
 		AnnotationConfiguration cfg = new AnnotationConfiguration();
-		cfg.addAnnotatedClass( Course.class );
+		cfg.addAnnotatedClass( Course1.class );
 		cfg.addAnnotatedClass( Student.class );
 		try {
 			cfg.buildSessionFactory();
@@ -56,7 +56,7 @@ public class AccessMappingTest extends TestCase {
 
 	public void testFieldAnnotationPlacement() throws Exception {
 		AnnotationConfiguration cfg = new AnnotationConfiguration();
-		Class<?> classUnderTest = CourseFieldAccess.class;
+		Class<?> classUnderTest = Course6.class;
 		cfg.addAnnotatedClass( classUnderTest );
 		cfg.addAnnotatedClass( Student.class );
 		SessionFactoryImplementor factory = ( SessionFactoryImplementor ) cfg.buildSessionFactory();
@@ -71,7 +71,7 @@ public class AccessMappingTest extends TestCase {
 
 	public void testPropertyAnnotationPlacement() throws Exception {
 		AnnotationConfiguration cfg = new AnnotationConfiguration();
-		Class<?> classUnderTest = CoursePropertyAccess.class;
+		Class<?> classUnderTest = Course7.class;
 		cfg.addAnnotatedClass( classUnderTest );
 		cfg.addAnnotatedClass( Student.class );
 		SessionFactoryImplementor factory = ( SessionFactoryImplementor ) cfg.buildSessionFactory();
@@ -86,7 +86,7 @@ public class AccessMappingTest extends TestCase {
 
 	public void testExplicitPropertyAccessAnnotationsOnProperty() throws Exception {
 		AnnotationConfiguration cfg = new AnnotationConfiguration();
-		Class<?> classUnderTest = CourseExplicitPropertyAccess.class;
+		Class<?> classUnderTest = Course2.class;
 		cfg.addAnnotatedClass( classUnderTest );
 		cfg.addAnnotatedClass( Student.class );
 		SessionFactoryImplementor factory = ( SessionFactoryImplementor ) cfg.buildSessionFactory();
@@ -101,7 +101,7 @@ public class AccessMappingTest extends TestCase {
 
 	public void testExplicitPropertyAccessAnnotationsOnField() throws Exception {
 		AnnotationConfiguration cfg = new AnnotationConfiguration();
-		cfg.addAnnotatedClass( CourseExplicitPropertyAccess3.class );
+		cfg.addAnnotatedClass( Course4.class );
 		cfg.addAnnotatedClass( Student.class );
 		try {
 			cfg.buildSessionFactory();
@@ -114,7 +114,7 @@ public class AccessMappingTest extends TestCase {
 
 	public void testExplicitPropertyAccessAnnotationsWithHibernateStyleOverride() throws Exception {
 		AnnotationConfiguration cfg = new AnnotationConfiguration();
-		Class<?> classUnderTest = CourseExplicitPropertyAccess2.class;
+		Class<?> classUnderTest = Course3.class;
 		cfg.addAnnotatedClass( classUnderTest );
 		cfg.addAnnotatedClass( Student.class );
 		SessionFactoryImplementor factory = ( SessionFactoryImplementor ) cfg.buildSessionFactory();
@@ -134,7 +134,7 @@ public class AccessMappingTest extends TestCase {
 
 	public void testExplicitPropertyAccessAnnotationsWithJpaStyleOverride() throws Exception {
 		AnnotationConfiguration cfg = new AnnotationConfiguration();
-		Class<?> classUnderTest = CourseExplicitPropertyAccess4.class;
+		Class<?> classUnderTest = Course5.class;
 		cfg.addAnnotatedClass( classUnderTest );
 		cfg.addAnnotatedClass( Student.class );
 		SessionFactoryImplementor factory = ( SessionFactoryImplementor ) cfg.buildSessionFactory();
@@ -151,4 +151,43 @@ public class AccessMappingTest extends TestCase {
 				tuplizer.getGetter( 0 ) instanceof BasicPropertyAccessor.BasicGetter
 		);
 	}
+
+	public void testDefaultFieldAccessIsInherited() throws Exception {
+		AnnotationConfiguration cfg = new AnnotationConfiguration();
+		Class<?> classUnderTest = User.class;
+		cfg.addAnnotatedClass( classUnderTest );
+		cfg.addAnnotatedClass( Person.class );
+		cfg.addAnnotatedClass( Being.class );
+		SessionFactoryImplementor factory = ( SessionFactoryImplementor ) cfg.buildSessionFactory();
+		EntityMetamodel metaModel = factory.getEntityPersister( classUnderTest.getName() )
+				.getEntityMetamodel();
+		PojoEntityTuplizer tuplizer = ( PojoEntityTuplizer ) metaModel.getTuplizer( EntityMode.POJO );
+		assertTrue(
+				"Field access should be used since the default access mode gets inherited",
+				tuplizer.getIdentifierGetter() instanceof DirectPropertyAccessor.DirectGetter
+		);
+	}
+
+	public void testDefaultPropertyAccessIsInherited() throws Exception {
+		AnnotationConfiguration cfg = new AnnotationConfiguration();
+		cfg.addAnnotatedClass( Horse.class );
+		cfg.addAnnotatedClass( Animal.class );
+
+		SessionFactoryImplementor factory = ( SessionFactoryImplementor ) cfg.buildSessionFactory();
+		EntityMetamodel metaModel = factory.getEntityPersister( Animal.class.getName() )
+				.getEntityMetamodel();
+		PojoEntityTuplizer tuplizer = ( PojoEntityTuplizer ) metaModel.getTuplizer( EntityMode.POJO );
+		assertTrue(
+				"Property access should be used since explicity configured via @Access",
+				tuplizer.getIdentifierGetter() instanceof BasicPropertyAccessor.BasicGetter
+		);
+
+		metaModel = factory.getEntityPersister( Horse.class.getName() )
+				.getEntityMetamodel();
+		tuplizer = ( PojoEntityTuplizer ) metaModel.getTuplizer( EntityMode.POJO );
+		assertTrue(
+				"Property access should be used since the default access mode gets inherited",
+				tuplizer.getGetter( 0 ) instanceof BasicPropertyAccessor.BasicGetter
+		);
+	}
 }

annotations/src/test/java/org/hibernate/test/annotations/access/jpa/AccessTest.java

@@ -54,8 +54,8 @@ public class AccessTest extends TestCase {
 		tx.commit();
 		s.clear();
 		tx = s.beginTransaction();
-		john = (User) s.get( User.class, john.getId() );
+		john = ( User ) s.get( User.class, john.getId() );
-		assertEquals("Wrong number of friends", 1, john.getFriends().size() );
+		assertEquals( "Wrong number of friends", 1, john.getFriends().size() );
 		assertNull( john.firstname );
 
 		s.delete( john );
@@ -74,7 +74,7 @@ public class AccessTest extends TestCase {
 		tx.commit();
 		s.clear();
 		tx = s.beginTransaction();
-		fur = (Furniture) s.get( Furniture.class, fur.getId() );
+		fur = ( Furniture ) s.get( Furniture.class, fur.getId() );
 		assertFalse( fur.isAlive );
 		assertNotNull( fur.getColor() );
 		s.delete( fur );
@@ -91,7 +91,7 @@ public class AccessTest extends TestCase {
 		tx.commit();
 		s.clear();
 		tx = s.beginTransaction();
-		fur = (Furniture) s.get( Furniture.class, fur.getId() );
+		fur = ( Furniture ) s.get( Furniture.class, fur.getId() );
 		assertNotNull( fur.getGod() );
 		s.delete( fur );
 		tx.commit();
@@ -107,7 +107,7 @@ public class AccessTest extends TestCase {
 		tx.commit();
 		s.clear();
 		tx = s.beginTransaction();
-		fur = (Furniture) s.get( Furniture.class, fur.getId() );
+		fur = ( Furniture ) s.get( Furniture.class, fur.getId() );
 		assertEquals( 5, fur.weight );
 		s.delete( fur );
 		tx.commit();
@@ -124,7 +124,7 @@ public class AccessTest extends TestCase {
 		tx.commit();
 		s.clear();
 		tx = s.beginTransaction();
-		chair = (Chair) s.get( Chair.class, chair.getId() );
+		chair = ( Chair ) s.get( Chair.class, chair.getId() );
 		assertNull( chair.getPillow() );
 		s.delete( chair );
 		tx.commit();
@@ -142,7 +142,7 @@ public class AccessTest extends TestCase {
 		tx.commit();
 		s.clear();
 		tx = s.beginTransaction();
-		bed = (BigBed) s.get( BigBed.class, bed.getId() );
+		bed = ( BigBed ) s.get( BigBed.class, bed.getId() );
 		assertEquals( 5, bed.size );
 		assertNull( bed.getQuality() );
 		s.delete( bed );
@@ -160,7 +160,7 @@ public class AccessTest extends TestCase {
 		tx.commit();
 		s.clear();
 		tx = s.beginTransaction();
-		gs = (Gardenshed) s.get( Gardenshed.class, gs.getId() );
+		gs = ( Gardenshed ) s.get( Gardenshed.class, gs.getId() );
 		assertEquals( 4, gs.floors );
 		assertEquals( 6, gs.getFloors() );
 		s.delete( gs );
@@ -169,6 +169,29 @@ public class AccessTest extends TestCase {
 
 	}
 
+	public void testEmbeddableUsesAccessStrategyOfContainingClass() throws Exception {
+		Circle circle = new Circle();
+		Color color = new Color( 5, 10, 15 );
+		circle.setColor( color );
+		Session s = openSession();
+		s.persist( circle );
+		Transaction tx = s.beginTransaction();
+		tx.commit();
+		s.clear();
+		tx = s.beginTransaction();
+		circle = ( Circle ) s.get( Circle.class, circle.getId() );
+		assertEquals( 5, circle.getColor().r );
+		try {
+			circle.getColor().getR();
+			fail();
+		} catch (RuntimeException e) {
+			// success		
+		}
+		s.delete( circle );
+		tx.commit();
+		s.close();
+	}
+
 	protected Class[] getMappings() {
 		return new Class[] {
 				Bed.class,
@@ -178,7 +201,10 @@ public class AccessTest extends TestCase {
 				Gardenshed.class,
 				Closet.class,
 				Person.class,
-				User.class
+				User.class,
+				Shape.class,
+				Circle.class,
+				Color.class
 		};
 	}
 }

annotations/src/test/java/org/hibernate/test/annotations/access/jpa/Animal.java

@@ -0,0 +1,53 @@
+//$Id$
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2008, Red Hat Middleware LLC or third-party contributors as
+ * indicated by the @author tags or express copyright attribution
+ * statements applied by the authors.  All third-party contributions are
+ * distributed under license by Red Hat Middleware LLC.
+ *
+ * This copyrighted material is made available to anyone wishing to use, modify,
+ * copy, or redistribute it subject to the terms and conditions of the GNU
+ * Lesser General Public License, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this distribution; if not, write to:
+ * Free Software Foundation, Inc.
+ * 51 Franklin Street, Fifth Floor
+ * Boston, MA  02110-1301  USA
+ */
+package org.hibernate.test.annotations.access.jpa;
+
+import javax.persistence.Access;
+import javax.persistence.AccessType;
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.Id;
+
+
+/**
+ * @author Hardy Ferentschik
+ */
+@Entity
+@Access(AccessType.FIELD)
+public class Animal {
+
+	private long id;
+
+	@Access( AccessType.PROPERTY)
+	@Id
+	@GeneratedValue
+	public long getId() {
+		return id;
+	}
+
+	public void setId(long id) {
+		this.id = id;
+	}
+}

annotations/src/test/java/org/hibernate/test/annotations/access/jpa/Circle.java

@@ -0,0 +1,49 @@
+//$Id: Being.java 18260 2009-12-17 21:14:07Z hardy.ferentschik $
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2008, Red Hat Middleware LLC or third-party contributors as
+ * indicated by the @author tags or express copyright attribution
+ * statements applied by the authors.  All third-party contributions are
+ * distributed under license by Red Hat Middleware LLC.
+ *
+ * This copyrighted material is made available to anyone wishing to use, modify,
+ * copy, or redistribute it subject to the terms and conditions of the GNU
+ * Lesser General Public License, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this distribution; if not, write to:
+ * Free Software Foundation, Inc.
+ * 51 Franklin Street, Fifth Floor
+ * Boston, MA  02110-1301  USA
+ */
+package org.hibernate.test.annotations.access.jpa;
+
+import javax.persistence.Access;
+import javax.persistence.AccessType;
+import javax.persistence.Embedded;
+import javax.persistence.Entity;
+
+
+/**
+ * @author Hardy Ferentschik
+ */
+@Entity
+@Access(AccessType.FIELD)
+public class Circle extends Shape {
+	@Embedded
+	private Color color;
+
+	public Color getColor() {
+		return color;
+	}
+
+	public void setColor(Color color) {
+		this.color = color;
+	}
+}

annotations/src/test/java/org/hibernate/test/annotations/access/jpa/Color.java

@@ -0,0 +1,71 @@
+//$Id: Being.java 18260 2009-12-17 21:14:07Z hardy.ferentschik $
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2008, Red Hat Middleware LLC or third-party contributors as
+ * indicated by the @author tags or express copyright attribution
+ * statements applied by the authors.  All third-party contributions are
+ * distributed under license by Red Hat Middleware LLC.
+ *
+ * This copyrighted material is made available to anyone wishing to use, modify,
+ * copy, or redistribute it subject to the terms and conditions of the GNU
+ * Lesser General Public License, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this distribution; if not, write to:
+ * Free Software Foundation, Inc.
+ * 51 Franklin Street, Fifth Floor
+ * Boston, MA  02110-1301  USA
+ */
+package org.hibernate.test.annotations.access.jpa;
+
+import javax.persistence.Embeddable;
+
+
+/**
+ * @author Hardy Ferentschik
+ */
+@Embeddable
+public class Color {
+	public int r;
+	public int g;
+	public int b;
+
+	public Color() {
+	}
+
+	public Color(int r, int g, int b) {
+		this.r = r;
+		this.g = g;
+		this.b = b;
+	}
+
+	public int getB() {
+		throw new RuntimeException();
+	}
+
+	public void setB(int b) {
+		this.b = b;
+	}
+
+	public int getG() {
+		throw new RuntimeException();
+	}
+
+	public void setG(int g) {
+		this.g = g;
+	}
+
+	public int getR() {
+		throw new RuntimeException();
+	}
+
+	public void setR(int r) {
+		this.r = r;
+	}
+}

annotations/src/test/java/org/hibernate/test/annotations/access/jpa/Course1.java

@@ -36,7 +36,7 @@ import javax.persistence.OneToMany;
  * @author Hardy Ferentschik
  */
 @Entity
-public class Course {
+public class Course1 {
 	@Id
 	@GeneratedValue
 	private long id;

annotations/src/test/java/org/hibernate/test/annotations/access/jpa/Course2.java

@@ -39,7 +39,7 @@ import javax.persistence.OneToMany;
  */
 @Entity
 @Access(AccessType.PROPERTY)
-public class CourseExplicitPropertyAccess {
+public class Course2 {
 	private long id;
 
 	private String title;

annotations/src/test/java/org/hibernate/test/annotations/access/jpa/Course3.java

@@ -39,7 +39,7 @@ import javax.persistence.OneToMany;
  */
 @Entity
 @Access(AccessType.PROPERTY)
-public class CourseExplicitPropertyAccess2 {
+public class Course3 {
 	private long id;
 
 	private String title;

annotations/src/test/java/org/hibernate/test/annotations/access/jpa/Course4.java

@@ -39,7 +39,7 @@ import javax.persistence.OneToMany;
  */
 @Entity
 @Access(AccessType.PROPERTY)
-public class CourseExplicitPropertyAccess3 {
+public class Course4 {
 	@Id
 	@GeneratedValue
 	private long id;

annotations/src/test/java/org/hibernate/test/annotations/access/jpa/Course5.java

@@ -38,7 +38,7 @@ import javax.persistence.ManyToMany;
  */
 @Entity
 @Access(AccessType.PROPERTY)
-public class CourseExplicitPropertyAccess4 {
+public class Course5 {
 
 	@Access(AccessType.FIELD)
 	@Id

annotations/src/test/java/org/hibernate/test/annotations/access/jpa/Course6.java

@@ -36,7 +36,7 @@ import javax.persistence.OneToMany;
  * @author Hardy Ferentschik
  */
 @Entity
-public class CourseFieldAccess {
+public class Course6 {
 	@Id
 	@GeneratedValue
 	private long id;

annotations/src/test/java/org/hibernate/test/annotations/access/jpa/Course7.java

@@ -36,7 +36,7 @@ import javax.persistence.OneToMany;
  * @author Hardy Ferentschik
  */
 @Entity
-public class CoursePropertyAccess {
+public class Course7 {
 	private long id;
 
 	private String title;

annotations/src/test/java/org/hibernate/test/annotations/access/jpa/Furniture.java

@@ -1,4 +1,4 @@
-//$Id:$
+//$Id$
 /*
  * Hibernate, Relational Persistence for Idiomatic Java
  *
@@ -59,9 +59,9 @@ public class Furniture extends Woody {
 		this.id = id;
 	}
 
-	@Access(javax.persistence.AccessType.PROPERTY)
 	public long weight;
 
+	@Access(javax.persistence.AccessType.PROPERTY)
 	public long getWeight() {
 		return weight + 1;
 	}

annotations/src/test/java/org/hibernate/test/annotations/access/jpa/Horse.java

@@ -0,0 +1,45 @@
+//$Id: Being.java 18260 2009-12-17 21:14:07Z hardy.ferentschik $
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2008, Red Hat Middleware LLC or third-party contributors as
+ * indicated by the @author tags or express copyright attribution
+ * statements applied by the authors.  All third-party contributions are
+ * distributed under license by Red Hat Middleware LLC.
+ *
+ * This copyrighted material is made available to anyone wishing to use, modify,
+ * copy, or redistribute it subject to the terms and conditions of the GNU
+ * Lesser General Public License, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this distribution; if not, write to:
+ * Free Software Foundation, Inc.
+ * 51 Franklin Street, Fifth Floor
+ * Boston, MA  02110-1301  USA
+ */
+package org.hibernate.test.annotations.access.jpa;
+
+import javax.persistence.Entity;
+
+
+/**
+ * @author Hardy Ferentschik
+ */
+@Entity
+public class Horse extends Animal {
+
+	private String name;
+
+	public String getName() {
+		return name;
+	}
+
+	public void setName(String name) {
+		this.name = name;
+	}
+}

annotations/src/test/java/org/hibernate/test/annotations/access/jpa/Shape.java

@@ -0,0 +1,50 @@
+//$Id: Being.java 18260 2009-12-17 21:14:07Z hardy.ferentschik $
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2008, Red Hat Middleware LLC or third-party contributors as
+ * indicated by the @author tags or express copyright attribution
+ * statements applied by the authors.  All third-party contributions are
+ * distributed under license by Red Hat Middleware LLC.
+ *
+ * This copyrighted material is made available to anyone wishing to use, modify,
+ * copy, or redistribute it subject to the terms and conditions of the GNU
+ * Lesser General Public License, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this distribution; if not, write to:
+ * Free Software Foundation, Inc.
+ * 51 Franklin Street, Fifth Floor
+ * Boston, MA  02110-1301  USA
+ */
+package org.hibernate.test.annotations.access.jpa;
+
+import javax.persistence.Access;
+import javax.persistence.AccessType;
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.Id;
+
+
+/**
+ * @author Hardy Ferentschik
+ */
+@Entity
+public class Shape {
+	private long id;
+
+	@Id
+	@GeneratedValue
+	public long getId() {
+		return id;
+	}
+
+	public void setId(long id) {
+		this.id = id;
+	}
+}