Author: hardy.ferentschik Date: 2010-05-27 14:44:57 -0400 (Thu, 27 May 2010) New Revision: 19626 Added: core/trunk/annotations/src/test/java/org/hibernate/test/annotations/access/jpa/Course8.java Modified: core/trunk/annotations/src/main/java/org/hibernate/cfg/AnnotationBinder.java core/trunk/annotations/src/main/java/org/hibernate/cfg/InheritanceState.java core/trunk/annotations/src/main/java/org/hibernate/cfg/PropertyContainer.java core/trunk/annotations/src/test/java/org/hibernate/test/annotations/access/jpa/AccessMappingTest.java core/trunk/annotations/src/test/java/org/hibernate/test/annotations/access/jpa/AccessTest.java Log: HHH-5004 Fixed problems in PropertyContainer and added some more comments. Cleaned up some code in InheritanceState Modified: core/trunk/annotations/src/main/java/org/hibernate/cfg/AnnotationBinder.java =================================================================== --- core/trunk/annotations/src/main/java/org/hibernate/cfg/AnnotationBinder.java 2010-05-26 22:27:27 UTC (rev 19625) +++ core/trunk/annotations/src/main/java/org/hibernate/cfg/AnnotationBinder.java 2010-05-27 18:44:57 UTC (rev 19626) @@ -1341,7 +1341,6 @@ accessType = propertyContainer.getExplicitAccessStrategy(); } - propertyContainer.assertTypesAreResolvable( accessType ); Collection<XProperty> properties = propertyContainer.getProperties( accessType ); for ( XProperty p : properties ) { final int currentIdPropertyCounter = addProperty( Modified: core/trunk/annotations/src/main/java/org/hibernate/cfg/InheritanceState.java =================================================================== --- core/trunk/annotations/src/main/java/org/hibernate/cfg/InheritanceState.java 2010-05-26 22:27:27 UTC (rev 19625) +++ core/trunk/annotations/src/main/java/org/hibernate/cfg/InheritanceState.java 2010-05-27 18:44:57 UTC (rev 19626) @@ -35,7 +35,6 @@ import javax.persistence.MappedSuperclass; import org.hibernate.AnnotationException; -import org.hibernate.annotations.common.reflection.ReflectionManager; import org.hibernate.annotations.common.reflection.XAnnotatedElement; import org.hibernate.annotations.common.reflection.XClass; import org.hibernate.annotations.common.reflection.XProperty; @@ -49,7 +48,6 @@ */ public class InheritanceState { private XClass clazz; - private XClass identifierType; /** * Has sibling (either mappedsuperclass entity) @@ -75,7 +73,6 @@ this.setClazz( clazz ); this.mappings = mappings; this.inheritanceStatePerClass = inheritanceStatePerClass; - this.identifierType = mappings.getReflectionManager().toXClass( void.class ); //not initialized extractInheritanceType(); } @@ -115,7 +112,7 @@ return null; } - public static InheritanceState getSuperclassInheritanceState( XClass clazz, Map<XClass, InheritanceState> states) { + public static InheritanceState getSuperclassInheritanceState(XClass clazz, Map<XClass, InheritanceState> states) { XClass superclass = clazz; do { superclass = superclass.getSuperclass(); @@ -171,7 +168,7 @@ void postProcess(PersistentClass persistenceClass, EntityBinder entityBinder) { //make sure we run elements to process getElementsToProcess(); - addMappedSuperClassInMetadata(persistenceClass); + addMappedSuperClassInMetadata( persistenceClass ); entityBinder.setPropertyAccessType( accessType ); } @@ -184,8 +181,8 @@ } else { InheritanceState state = InheritanceState.getSuperclassInheritanceState( clazz, inheritanceStatePerClass ); - if (state != null){ - return state.getClassWithIdClass(true); + if ( state != null ) { + return state.getClassWithIdClass( true ); } else { return null; @@ -194,14 +191,14 @@ } public Boolean hasIdClassOrEmbeddedId() { - if (hasIdClassOrEmbeddedId == null) { + if ( hasIdClassOrEmbeddedId == null ) { hasIdClassOrEmbeddedId = false; if ( getClassWithIdClass( true ) != null ) { hasIdClassOrEmbeddedId = true; } else { final ElementsToProcess process = getElementsToProcess(); - for(PropertyData property : process.getElements() ) { + for ( PropertyData property : process.getElements() ) { if ( property.getProperty().isAnnotationPresent( EmbeddedId.class ) ) { hasIdClassOrEmbeddedId = true; break; @@ -213,33 +210,40 @@ } /* - * Get the annotated elements, guessing the access type from @Id or @EmbeddedId presence. - * Change EntityBinder by side effect - */ + * Get the annotated elements, guessing the access type from @Id or @EmbeddedId presence. + * Change EntityBinder by side effect + */ + public ElementsToProcess getElementsToProcess() { - if (elementsToProcess == null) { + if ( elementsToProcess == null ) { InheritanceState inheritanceState = inheritanceStatePerClass.get( clazz ); assert !inheritanceState.isEmbeddableSuperclass(); getMappedSuperclassesTillNextEntityOrdered(); - accessType = determineDefaultAccessType( ); + accessType = determineDefaultAccessType(); List<PropertyData> elements = new ArrayList<PropertyData>(); int deep = classesToProcessForMappedSuperclass.size(); int idPropertyCount = 0; for ( int index = 0; index < deep; index++ ) { - PropertyContainer propertyContainer = new PropertyContainer( classesToProcessForMappedSuperclass.get( index ), clazz ); - int currentIdPropertyCount = AnnotationBinder.addElementsOfClass( elements, accessType, propertyContainer, mappings ); - idPropertyCount += currentIdPropertyCount; + PropertyContainer propertyContainer = new PropertyContainer( + classesToProcessForMappedSuperclass.get( + index + ), clazz + ); + int currentIdPropertyCount = AnnotationBinder.addElementsOfClass( + elements, accessType, propertyContainer, mappings + ); + idPropertyCount += currentIdPropertyCount; } if ( idPropertyCount == 0 && !inheritanceState.hasParents() ) { throw new AnnotationException( "No identifier specified for entity: " + clazz.getName() ); } - elementsToProcess = new ElementsToProcess( elements, idPropertyCount); + elementsToProcess = new ElementsToProcess( elements, idPropertyCount ); } return elementsToProcess; } @@ -251,25 +255,19 @@ for ( XProperty prop : xclass.getDeclaredProperties( AccessType.PROPERTY.getType() ) ) { final boolean isEmbeddedId = prop.isAnnotationPresent( EmbeddedId.class ); if ( prop.isAnnotationPresent( Id.class ) || isEmbeddedId ) { - if (isEmbeddedId) { - identifierType = prop.getClassOrElementClass(); - } return AccessType.PROPERTY; } } for ( XProperty prop : xclass.getDeclaredProperties( AccessType.FIELD.getType() ) ) { final boolean isEmbeddedId = prop.isAnnotationPresent( EmbeddedId.class ); if ( prop.isAnnotationPresent( Id.class ) || isEmbeddedId ) { - if ( isEmbeddedId ) { - identifierType = prop.getClassOrElementClass(); - } return AccessType.FIELD; } } } xclass = xclass.getSuperclass(); } - throw new AnnotationException( "No identifier specified for entity: " + clazz.getName() ); + throw new AnnotationException( "No identifier specified for entity: " + clazz ); } private void getMappedSuperclassesTillNextEntityOrdered() { @@ -303,19 +301,20 @@ mappings.getClass( superEntityState.getClazz().getName() ) : null; final int lastMappedSuperclass = classesToProcessForMappedSuperclass.size() - 1; - for ( int index = 0 ; index < lastMappedSuperclass ; index++ ) { + for ( int index = 0; index < lastMappedSuperclass; index++ ) { org.hibernate.mapping.MappedSuperclass parentSuperclass = mappedSuperclass; - final Class<?> type = mappings.getReflectionManager().toClass( classesToProcessForMappedSuperclass.get( index ) ); + final Class<?> type = mappings.getReflectionManager() + .toClass( classesToProcessForMappedSuperclass.get( index ) ); //add MAppedSuperclass if not already there mappedSuperclass = mappings.getMappedSuperclass( type ); - if (mappedSuperclass == null) { - mappedSuperclass = new org.hibernate.mapping.MappedSuperclass(parentSuperclass, superEntity ); + if ( mappedSuperclass == null ) { + mappedSuperclass = new org.hibernate.mapping.MappedSuperclass( parentSuperclass, superEntity ); mappedSuperclass.setMappedClass( type ); mappings.addMappedSuperclass( type, mappedSuperclass ); } } - if (mappedSuperclass != null) { - persistentClass.setSuperMappedSuperclass(mappedSuperclass); + if ( mappedSuperclass != null ) { + persistentClass.setSuperMappedSuperclass( mappedSuperclass ); } } Modified: core/trunk/annotations/src/main/java/org/hibernate/cfg/PropertyContainer.java =================================================================== --- core/trunk/annotations/src/main/java/org/hibernate/cfg/PropertyContainer.java 2010-05-26 22:27:27 UTC (rev 19625) +++ core/trunk/annotations/src/main/java/org/hibernate/cfg/PropertyContainer.java 2010-05-27 18:44:57 UTC (rev 19626) @@ -24,9 +24,10 @@ */ package org.hibernate.cfg; -import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.TreeMap; import javax.persistence.Access; import javax.persistence.ManyToMany; @@ -55,19 +56,40 @@ class PropertyContainer { private static final Logger log = LoggerFactory.getLogger( AnnotationBinder.class ); - private final XClass entityAtStake; + + private final AccessType explicitClassDefinedAccessType; + + /** + * Constains the properties which must be returned in case the class is accessed via {@code AccessType.FIELD}. Note, + * this does not mean that all {@code XProperty}s in this map are fields. Due to JPA access rules single properties + * can have different access type than the overall class access type. + */ private final TreeMap<String, XProperty> fieldAccessMap; + + /** + * Constains the properties which must be returned in case the class is accessed via {@code AccessType.Property}. Note, + * this does not mean that all {@code XProperty}s in this map are properties/methods. Due to JPA access rules single properties + * can have different access type than the overall class access type. + */ private final TreeMap<String, XProperty> propertyAccessMap; + + /** + * The class for which this container is created. + */ private final XClass xClass; - private final AccessType explicitClassDefinedAccessType; + private final XClass entityAtStake; PropertyContainer(XClass clazz, XClass entityAtStake) { this.xClass = clazz; this.entityAtStake = entityAtStake; + + explicitClassDefinedAccessType = determineClassDefinedAccessStrategy(); + + // first add all properties to field and property map fieldAccessMap = initProperties( AccessType.FIELD ); propertyAccessMap = initProperties( AccessType.PROPERTY ); - explicitClassDefinedAccessType = determineClassDefinedAccessStrategy(); - checkForJpaAccess(); + + considerExplicitFieldAndPropertyAccess(); } public XClass getEntityAtStake() { @@ -87,16 +109,17 @@ } public Collection<XProperty> getProperties(AccessType accessType) { + assertTypesAreResolvable( accessType ); if ( AccessType.DEFAULT == accessType || AccessType.PROPERTY == accessType ) { - return propertyAccessMap.values(); + return Collections.unmodifiableCollection( propertyAccessMap.values() ); } else { - return fieldAccessMap.values(); + return Collections.unmodifiableCollection( fieldAccessMap.values() ); } } - public void assertTypesAreResolvable(AccessType access) { - TreeMap<String, XProperty> xprops; + private void assertTypesAreResolvable(AccessType access) { + Map<String, XProperty> xprops; if ( AccessType.PROPERTY.equals( access ) || AccessType.DEFAULT.equals( access ) ) { xprops = propertyAccessMap; } @@ -113,29 +136,25 @@ } } - private void checkForJpaAccess() { - List<XProperty> tmpList = new ArrayList<XProperty>(); + private void considerExplicitFieldAndPropertyAccess() { for ( XProperty property : fieldAccessMap.values() ) { Access access = property.getAnnotation( Access.class ); if ( access == null ) { continue; } + // see "2.3.2 Explicit Access Type" of JPA 2 spec + // the access type for this property is explicitly set to AccessType.FIELD, hence we have to + // use field access for this property even if the default access type for the class is AccessType.PROPERTY AccessType accessType = AccessType.getAccessStrategy( access.value() ); - if ( accessType == AccessType.PROPERTY ) { + if ( accessType == AccessType.FIELD ) { + propertyAccessMap.put( property.getName(), property ); + } + else { // AccessType.PROPERTY log.warn( "Placing @Access(AccessType.PROPERTY) on a field does not have any effect." ); - continue; } - - tmpList.add( property ); } - for ( XProperty property : tmpList ) { - fieldAccessMap.remove( property.getName() ); - propertyAccessMap.put( property.getName(), property ); - } - - tmpList.clear(); for ( XProperty property : propertyAccessMap.values() ) { Access access = property.getAnnotation( Access.class ); if ( access == null ) { @@ -143,20 +162,32 @@ } AccessType accessType = AccessType.getAccessStrategy( access.value() ); - if ( accessType == AccessType.FIELD ) { - log.warn( "Placing @Access(AccessType.FIELD) on a field does not have any effect." ); - continue; + + // see "2.3.2 Explicit Access Type" of JPA 2 spec + // the access type for this property is explicitly set to AccessType.PROPERTY, hence we have to + // return use method access even if the default class access type is AccessType.FIELD + if ( accessType == AccessType.PROPERTY ) { + fieldAccessMap.put( property.getName(), property ); } - - tmpList.add( property ); + else { // AccessType.FIELD + log.warn( "Placing @Access(AccessType.FIELD) on a property does not have any effect." ); + } } - for ( XProperty property : tmpList ) { - propertyAccessMap.remove( property.getName() ); - fieldAccessMap.put( property.getName(), property ); - } } + /** + * Retrieves all properties from the {@code xClass} with the specified access type. This method does not take + * any jpa access rules/annotations into account yet. + * + * @param access The access type - {@code AccessType.FIELD} or {@code AccessType.Property} + * + * @return A maps of the properties with the given access type keyed against their property name + */ private TreeMap<String, XProperty> initProperties(AccessType access) { + if ( !( AccessType.PROPERTY.equals( access ) || AccessType.FIELD.equals( access ) ) ) { + throw new IllegalArgumentException( "Acces type has to be AccessType.FIELD or AccessType.Property" ); + } + //order so that property are used in the same order when binding native query TreeMap<String, XProperty> propertiesMap = new TreeMap<String, XProperty>(); List<XProperty> properties = xClass.getDeclaredProperties( access.getType() ); Modified: core/trunk/annotations/src/test/java/org/hibernate/test/annotations/access/jpa/AccessMappingTest.java =================================================================== --- core/trunk/annotations/src/test/java/org/hibernate/test/annotations/access/jpa/AccessMappingTest.java 2010-05-26 22:27:27 UTC (rev 19625) +++ core/trunk/annotations/src/test/java/org/hibernate/test/annotations/access/jpa/AccessMappingTest.java 2010-05-27 18:44:57 UTC (rev 19626) @@ -192,4 +192,14 @@ tuplizer.getGetter( 0 ) instanceof BasicPropertyAccessor.BasicGetter ); } + + /** + * HHH-5004 + */ + public void testAccessOnClassAndId() throws Exception { + AnnotationConfiguration cfg = new AnnotationConfiguration(); + cfg.addAnnotatedClass( Course8.class ); + cfg.addAnnotatedClass( Student.class ); + cfg.buildSessionFactory(); + } } \ No newline at end of file Modified: core/trunk/annotations/src/test/java/org/hibernate/test/annotations/access/jpa/AccessTest.java =================================================================== --- core/trunk/annotations/src/test/java/org/hibernate/test/annotations/access/jpa/AccessTest.java 2010-05-26 22:27:27 UTC (rev 19625) +++ core/trunk/annotations/src/test/java/org/hibernate/test/annotations/access/jpa/AccessTest.java 2010-05-27 18:44:57 UTC (rev 19626) @@ -34,6 +34,7 @@ /** * @author Emmanuel Bernard + * @author Hardy Ferentschik */ public class AccessTest extends TestCase { Copied: core/trunk/annotations/src/test/java/org/hibernate/test/annotations/access/jpa/Course8.java (from rev 19610, core/trunk/annotations/src/test/java/org/hibernate/test/annotations/access/jpa/Course7.java) =================================================================== --- core/trunk/annotations/src/test/java/org/hibernate/test/annotations/access/jpa/Course8.java (rev 0) +++ core/trunk/annotations/src/test/java/org/hibernate/test/annotations/access/jpa/Course8.java 2010-05-27 18:44:57 UTC (rev 19626) @@ -0,0 +1,78 @@ +//$Id: AccessTest.java 15025 2008-08-11 09:14:39Z 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 java.util.List; +import javax.persistence.Access; +import javax.persistence.AccessType; +import javax.persistence.CascadeType; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; +import javax.persistence.OneToMany; + + +/** + * Test class for HHH-5004 + * + * @author Hardy Ferentschik + */ +@Entity +(a)Access(AccessType.PROPERTY) +public class Course8 { + private long id; + + private String title; + + private List<Student> students; + + @Id + @GeneratedValue + @Access(AccessType.PROPERTY) + public long getId() { + return id; + } + + public void setId(long id) { + this.id = id; + } + + @OneToMany(cascade = CascadeType.ALL) + public List<Student> getStudents() { + return students; + } + + public void setStudents(List<Student> students) { + this.students = students; + } + + public String getTitle() { + return title; + } + + public void setTitle(String title) { + this.title = title; + } +} \ No newline at end of file