HHH-8533 - Add tests of JPA Metamodel handling for MappedSuperclass and mixed @Id/@IdClass declaration

hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/metamodel/AbstractIdentifiableType.java

@@ -22,6 +22,7 @@
  * Boston, MA  02110-1301  USA
  */
 package org.hibernate.jpa.internal.metamodel;
+
 import java.io.Serializable;
 import java.util.HashSet;
 import java.util.Set;
@@ -45,6 +46,7 @@ public abstract class AbstractIdentifiableType<X>
 		extends AbstractManagedType<X>
 		implements IdentifiableType<X>, Serializable {
 
+	private final boolean hasIdClass;
 	private final boolean hasIdentifierProperty;
 	private final boolean isVersioned;
 
@@ -56,169 +58,157 @@ public abstract class AbstractIdentifiableType<X>
 			Class<X> javaType,
 			String typeName,
 			AbstractIdentifiableType<? super X> superType,
+			boolean hasIdClass,
 			boolean hasIdentifierProperty,
 			boolean versioned) {
 		super( javaType, typeName, superType );
+		this.hasIdClass = hasIdClass;
 		this.hasIdentifierProperty = hasIdentifierProperty;
 		isVersioned = versioned;
 	}
 
-	/**
+	public boolean hasIdClass() {
-	 * {@inheritDoc}
+		return hasIdClass;
-	 */
-	public AbstractIdentifiableType<? super X> getSupertype() {
-		return ( AbstractIdentifiableType<? super X> ) super.getSupertype();
 	}
 
-	/**
+	@Override
-	 * Indicates if a non-null super type is required to provide the
-	 * identifier attribute(s) if this object does not have a declared
-	 * identifier.
-	 * .
-	 * @return true, if a non-null super type is required to provide
-	 * the identifier attribute(s) if this object does not have a
-	 * declared identifier; false, otherwise.
-	 */
-	protected abstract boolean requiresSupertypeForNonDeclaredIdentifier();
-
-	protected AbstractIdentifiableType<? super X> requireSupertype() {
-		if ( getSupertype() == null ) {
-			throw new IllegalStateException( "No supertype found" );
-		}
-		return getSupertype();
-	}
-
-	/**
-	 * {@inheritDoc}
-	 */
 	public boolean hasSingleIdAttribute() {
-		return hasIdentifierProperty;
+		return !hasIdClass && hasIdentifierProperty;
 	}
 
-	/**
+	@Override
-	 * {@inheritDoc}
+	@SuppressWarnings("unchecked")
-	 */
+	public AbstractIdentifiableType<? super X> getSupertype() {
+		// overridden simply to perform the cast
+		return (AbstractIdentifiableType<? super X>) super.getSupertype();
+	}
+
+	@Override
 	@SuppressWarnings({ "unchecked" })
 	public <Y> SingularAttribute<? super X, Y> getId(Class<Y> javaType) {
-		final SingularAttribute<? super X, Y> id_;
+		ensureNoIdClass();
+		SingularAttributeImpl id = locateIdAttribute();
 		if ( id != null ) {
-			checkSimpleId();
+			checkType( id, javaType );
-			id_ = ( SingularAttribute<? super X, Y> ) id;
+		}
-			if ( javaType != id.getJavaType() ) {
+		return ( SingularAttribute<? super X, Y> ) id;
-				throw new IllegalArgumentException( "Id attribute was not of specified type : " + javaType.getName() );
+	}
-			}
+
+	private void ensureNoIdClass() {
+		if ( hasIdClass ) {
+			throw new IllegalArgumentException(
+					"Illegal call to IdentifiableType#getId for class [" + getTypeName() + "] defined with @IdClass"
+			);
+		}
+	}
+
+	private SingularAttributeImpl locateIdAttribute() {
+		if ( id != null ) {
+			return id;
 		}
 		else {
-			//yuk yuk bad me
+			if ( getSupertype() != null ) {
-			if ( ! requiresSupertypeForNonDeclaredIdentifier()) {
+				SingularAttributeImpl id = getSupertype().internalGetId();
-				final AbstractIdentifiableType<? super X> supertype = getSupertype();
+				if ( id != null ) {
-				if (supertype != null) {
+					return id;
-					id_ = supertype.getId( javaType );
 				}
-				else {
-					id_ = null;
-				}
-			}
-			else {
-				id_ = requireSupertype().getId( javaType );
 			}
 		}
-		return id_;
+
+		return null;
 	}
 
-	/**
+	protected SingularAttributeImpl internalGetId() {
-	 * Centralized check to ensure the id for this hierarchy is a simple one (i.e., does not use
+		if ( id != null ) {
-	 * an id-class).
+			return id;
-	 *
+		}
-	 * @see #checkIdClass()
+		else {
-	 */
+			if ( getSupertype() != null ) {
-	protected void checkSimpleId() {
+				return getSupertype().internalGetId();
-		if ( ! hasIdentifierProperty ) {
+			}
-			throw new IllegalStateException( "This class uses an @IdClass" );
+		}
+
+		return null;
+	}
+
+	@SuppressWarnings("unchecked")
+	private void checkType(SingularAttributeImpl attribute, Class javaType) {
+		if ( ! javaType.isAssignableFrom( attribute.getType().getJavaType() ) ) {
+			throw new IllegalArgumentException(
+					String.format(
+							"Attribute [%s#%s : %s] not castable to requested type [%s]",
+							getTypeName(),
+							attribute.getName(),
+							attribute.getType().getJavaType().getName(),
+							javaType.getName()
+					)
+			);
 		}
 	}
 
-
+	@Override
-	/**
-	 * {@inheritDoc}
-	 */
 	@SuppressWarnings({ "unchecked" })
 	public <Y> SingularAttribute<X, Y> getDeclaredId(Class<Y> javaType) {
-		checkDeclaredId();
+		ensureNoIdClass();
-		checkSimpleId();
+		if ( id == null ) {
-		if ( javaType != id.getJavaType() ) {
+			throw new IllegalArgumentException( "The id attribute is not declared on this type [" + getTypeName() + "]" );
-			throw new IllegalArgumentException( "Id attribute was not of specified type : " + javaType.getName() );
 		}
+		checkType( id, javaType );
 		return (SingularAttribute<X, Y>) id;
 	}
 
-	/**
+	@Override
-	 * Centralized check to ensure the id is actually declared on the class mapped here, as opposed to a
-	 * super class.
-	 */
-	protected void checkDeclaredId() {
-		if ( id == null ) {
-			throw new IllegalArgumentException( "The id attribute is not declared on this type" );
-		}
-	}
-
-	/**
-	 * {@inheritDoc}
-	 */
 	public Type<?> getIdType() {
-		if ( id != null ) {
+		SingularAttributeImpl id = locateIdAttribute();
-			checkSimpleId();
+		return id == null ? null : id.getType();
-			return id.getType();
-		}
-		else {
-			return requireSupertype().getIdType();
-		}
-	}
-
-	private boolean hasIdClassAttributesDefined() {
-		return idClassAttributes != null ||
-				( getSupertype() != null && getSupertype().hasIdClassAttributesDefined() );
 	}
 
 	/**
-	 * {@inheritDoc}
+	 * A form of {@link #getIdClassAttributes} which prefers to return {@code null} rather than throw exceptions
-	 */
-	public Set<SingularAttribute<? super X, ?>> getIdClassAttributes() {
-		if ( idClassAttributes != null ) {
-			checkIdClass();
-		}
-		else {
-			// Java does not allow casting requireSupertype().getIdClassAttributes()
-			// to Set<SingularAttribute<? super X, ?>> because the
-			// superclass X is a different Java type from this X
-			// (i.e, getSupertype().getJavaType() != getJavaType()).
-			// It will, however, allow a Set<SingularAttribute<? super X, ?>>
-			// to be initialized with requireSupertype().getIdClassAttributes(),
-			// since getSupertype().getJavaType() is a superclass of getJavaType()
-			if ( requiresSupertypeForNonDeclaredIdentifier() ) {
-				idClassAttributes = new HashSet<SingularAttribute<? super X, ?>>( requireSupertype().getIdClassAttributes() );
-			}
-			else if ( getSupertype() != null && hasIdClassAttributesDefined() ) {
-				idClassAttributes = new HashSet<SingularAttribute<? super X, ?>>( getSupertype().getIdClassAttributes() );
-			}
-		}
-		return idClassAttributes;
-	}
-
-	/**
-	 * Centralized check to ensure the id for this hierarchy uses an id-class.
 	 *
-	 * @see #checkSimpleId()
+	 * @return IdClass attributes or {@code null}
 	 */
-	private void checkIdClass() {
+	public Set<SingularAttribute<? super X, ?>> getIdClassAttributesSafely() {
-		if ( hasIdentifierProperty ) {
+		if ( !hasIdClass ) {
-			throw new IllegalArgumentException( "This class does not use @IdClass" );
+			return null;
+		}
+		final Set<SingularAttribute<? super X, ?>> attributes = new HashSet<SingularAttribute<? super X, ?>>();
+		internalCollectIdClassAttributes( attributes );
+
+		if ( attributes.isEmpty() ) {
+			return null;
+		}
+
+		return attributes;
+	}
+
+	@Override
+	public Set<SingularAttribute<? super X, ?>> getIdClassAttributes() {
+		if ( !hasIdClass ) {
+			throw new IllegalArgumentException( "This class [" + getJavaType() + "] does not define an IdClass" );
+		}
+
+		final Set<SingularAttribute<? super X, ?>> attributes = new HashSet<SingularAttribute<? super X, ?>>();
+		internalCollectIdClassAttributes( attributes );
+
+		if ( attributes.isEmpty() ) {
+			throw new IllegalArgumentException( "Unable to locate IdClass attributes [" + getJavaType() + "]" );
+		}
+
+		return attributes;
+	}
+
+	@SuppressWarnings("unchecked")
+	private void internalCollectIdClassAttributes(Set attributes) {
+		if ( idClassAttributes != null ) {
+			attributes.addAll( idClassAttributes );
+		}
+		else if ( getSupertype() != null ) {
+			getSupertype().internalCollectIdClassAttributes( attributes );
 		}
 	}
 
-	/**
+	@Override
-	 * {@inheritDoc}
-	 */
 	public boolean hasVersionAttribute() {
 		return isVersioned;
 	}
@@ -227,39 +217,67 @@ public abstract class AbstractIdentifiableType<X>
 		return isVersioned && version != null;
 	}
 
-	/**
+	@Override
-	 * {@inheritDoc}
-	 */
 	@SuppressWarnings({ "unchecked" })
 	public <Y> SingularAttribute<? super X, Y> getVersion(Class<Y> javaType) {
+		// todo : is return null allowed?
 		if ( ! hasVersionAttribute() ) {
 			return null;
 		}
-		final SingularAttribute<? super X, Y> version_;
+
+		SingularAttributeImpl version = locateVersionAttribute();
 		if ( version != null ) {
-			version_ = ( SingularAttribute<? super X, Y> ) version;
+			checkType( version, javaType );
-			if ( javaType != version.getJavaType() ) {
-				throw new IllegalArgumentException( "Version attribute was not of specified type : " + javaType.getName() );
-			}
 		}
-		else {
+		return ( SingularAttribute<? super X, Y> ) version;
-			version_ = requireSupertype().getVersion( javaType );
-		}
-		return version_;
 	}
 
-	/**
+	private SingularAttributeImpl locateVersionAttribute() {
-	 * {@inheritDoc}
+		if ( version != null ) {
-	 */
+			return version;
+		}
+		else {
+			if ( getSupertype() != null ) {
+				SingularAttributeImpl version = getSupertype().internalGetVersion();
+				if ( version != null ) {
+					return version;
+				}
+			}
+		}
+
+		return null;
+	}
+
+	protected SingularAttributeImpl internalGetVersion() {
+		if ( version != null ) {
+			return version;
+		}
+		else {
+			if ( getSupertype() != null ) {
+				return getSupertype().internalGetVersion();
+			}
+		}
+
+		return null;
+	}
+
+	@Override
 	@SuppressWarnings({ "unchecked" })
 	public <Y> SingularAttribute<X, Y> getDeclaredVersion(Class<Y> javaType) {
 		checkDeclaredVersion();
-		if ( javaType != version.getJavaType() ) {
+		checkType( version, javaType );
-			throw new IllegalArgumentException( "Version attribute was not of specified type : " + javaType.getName() );
-		}
 		return ( SingularAttribute<X, Y> ) version;
 	}
 
+	private void checkDeclaredVersion() {
+		if ( version == null || ( getSupertype() != null && getSupertype().hasVersionAttribute() )) {
+			throw new IllegalArgumentException(
+					"The version attribute is not declared by this type [" + getJavaType() + "]"
+			);
+		}
+	}
+
+
 	/**
 	 * For used to retrieve the declared version when populating the static metamodel.
 	 *
@@ -270,16 +288,6 @@ public abstract class AbstractIdentifiableType<X>
 		return version;
 	}
 
-	/**
-	 * Centralized check to ensure the version (if one) is actually declared on the class mapped here, as opposed to a
-	 * super class.
-	 */
-	protected void checkDeclaredVersion() {
-		if ( version == null || ( getSupertype() != null && getSupertype().hasVersionAttribute() )) {
-			throw new IllegalArgumentException( "The version attribute is not declared on this type" );
-		}
-	}
-
 	public Builder<X> getBuilder() {
 		final AbstractManagedType.Builder<X> managedBuilder = super.getBuilder();
 		return new Builder<X>() {

hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/metamodel/EntityTypeImpl.java

@@ -34,40 +34,39 @@ import org.hibernate.mapping.PersistentClass;
  * @author Steve Ebersole
  * @author Emmanuel Bernard
  */
-public class EntityTypeImpl<X> 
+public class EntityTypeImpl<X> extends AbstractIdentifiableType<X> implements EntityType<X>, Serializable {
-		extends AbstractIdentifiableType<X>
-		implements EntityType<X>, Serializable {
 	private final String jpaEntityName;
 
+	@SuppressWarnings("unchecked")
 	public EntityTypeImpl(Class javaType, AbstractIdentifiableType<? super X> superType, PersistentClass persistentClass) {
 		super(
 				javaType,
 				persistentClass.getEntityName(),
 				superType,
+				persistentClass.getDeclaredIdentifierMapper() != null || ( superType != null && superType.hasIdClass() ),
 				persistentClass.hasIdentifierProperty(),
 				persistentClass.isVersioned()
 		);
 		this.jpaEntityName = persistentClass.getJpaEntityName();
 	}
 
+	@Override
 	public String getName() {
 		return jpaEntityName;
 	}
 
+	@Override
 	public BindableType getBindableType() {
 		return BindableType.ENTITY_TYPE;
 	}
 
+	@Override
 	public Class<X> getBindableJavaType() {
 		return getJavaType();
 	}
 
+	@Override
 	public PersistenceType getPersistenceType() {
 		return PersistenceType.ENTITY;
 	}
-
-	@Override
-	protected boolean requiresSupertypeForNonDeclaredIdentifier() {
-		return true;
-	}
 }

hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/metamodel/MappedSuperclassTypeImpl.java

@@ -36,15 +36,18 @@ public class MappedSuperclassTypeImpl<X> extends AbstractIdentifiableType<X> imp
 			Class<X> javaType,
 			MappedSuperclass mappedSuperclass,
 			AbstractIdentifiableType<? super X> superType) {
-		super( javaType, null, superType, mappedSuperclass.hasIdentifierProperty(), mappedSuperclass.isVersioned() );
+		super(
-	}
+				javaType,
-
+				null,
-	public PersistenceType getPersistenceType() {
+				superType,
-		return PersistenceType.MAPPED_SUPERCLASS;
+				mappedSuperclass.getDeclaredIdentifierMapper() != null || ( superType != null && superType.hasIdClass() ),
+				mappedSuperclass.hasIdentifierProperty(),
+				mappedSuperclass.isVersioned()
+		);
 	}
 
 	@Override
-	protected boolean requiresSupertypeForNonDeclaredIdentifier() {
+	public PersistenceType getPersistenceType() {
-		return false;
+		return PersistenceType.MAPPED_SUPERCLASS;
 	}
 }

hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/metamodel/MetadataContext.java

@@ -393,8 +393,8 @@ class MetadataContext {
 			}
 
 			// handle id-class mappings specially
-			if ( ! entityType.hasSingleIdAttribute() ) {
+			if ( entityType.hasIdClass() ) {
-				final Set<SingularAttribute<? super X, ?>> attributes = entityType.getIdClassAttributes();
+				final Set<SingularAttribute<? super X, ?>> attributes = entityType.getIdClassAttributesSafely();
 				if ( attributes != null ) {
 					for ( SingularAttribute<? super X, ?> attribute : attributes ) {
 						registerAttribute( metamodelClass, attribute );

hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/metamodel/MappedSuperclassTypeTest.java

@@ -23,6 +23,8 @@
  */
 package org.hibernate.jpa.test.metamodel;
 
+import javax.persistence.metamodel.EntityType;
+import javax.persistence.metamodel.IdentifiableType;
 import javax.persistence.metamodel.ManagedType;
 
 import org.hibernate.jpa.test.BaseEntityManagerFunctionalTestCase;
@@ -32,6 +34,7 @@ import org.junit.Test;
 import org.hibernate.testing.TestForIssue;
 
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.fail;
 
 /**
  * @author Steve Ebersole
@@ -49,4 +52,23 @@ public class MappedSuperclassTypeTest extends BaseEntityManagerFunctionalTestCas
 		// the issue was in regards to throwing an exception, but also check for nullness
 		assertNotNull( type );
 	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-8533" )
+	@SuppressWarnings("unchecked")
+	public void testAttributeAccess() {
+		final EntityType<SomeMappedSuperclassSubclass> entityType =  entityManagerFactory().getMetamodel().entity( SomeMappedSuperclassSubclass.class );
+		final IdentifiableType<SomeMappedSuperclass> mappedSuperclassType = (IdentifiableType<SomeMappedSuperclass>) entityType.getSupertype();
+
+		assertNotNull( entityType.getId( Long.class ) );
+		try {
+			entityType.getDeclaredId( Long.class );
+			fail();
+		}
+		catch (IllegalArgumentException expected) {
+		}
+
+		assertNotNull( mappedSuperclassType.getId( Long.class ) );
+		assertNotNull( mappedSuperclassType.getDeclaredId( Long.class ) );
+	}
 }

hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/metamodel/MixedIdAndIdClassHandling.java

@@ -0,0 +1,132 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2013, Red Hat Inc. 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 Inc.
+ *
+ * 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.jpa.test.metamodel;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+import javax.persistence.Id;
+import javax.persistence.IdClass;
+import javax.persistence.MappedSuperclass;
+import javax.persistence.Table;
+import javax.persistence.metamodel.EntityType;
+
+import org.hibernate.jpa.test.BaseEntityManagerFunctionalTestCase;
+
+import org.junit.Test;
+
+import org.hibernate.testing.TestForIssue;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.fail;
+
+/**
+ * Ugh
+ *
+ * @author Steve Ebersole
+ */
+public class MixedIdAndIdClassHandling extends BaseEntityManagerFunctionalTestCase {
+
+	@Override
+	protected Class<?>[] getAnnotatedClasses() {
+		return new Class[] { FullTimeEmployee.class };
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-8533" )
+	public void testAccess() {
+		EntityType<FullTimeEmployee> entityType = entityManagerFactory().getMetamodel().entity( FullTimeEmployee.class );
+		try {
+			entityType.getId( String.class );
+			fail( "getId on entity defining @IdClass should cause IAE" );
+		}
+		catch (IllegalArgumentException expected) {
+		}
+
+		assertFalse( entityType.hasSingleIdAttribute() );
+
+		// this is questionable...
+		//assertEquals( String.class, entityType.getIdType().getJavaType() );
+	}
+
+	@MappedSuperclass
+	@IdClass( EmployeeId.class )
+	public static abstract class Employee {
+		@Id
+		private String id;
+		private String name;
+	}
+
+	@Entity( name = "FullTimeEmployee" )
+	@Table( name="EMPLOYEE" )
+	public static class FullTimeEmployee extends Employee {
+		@Column(name="SALARY")
+		private float salary;
+
+		public FullTimeEmployee() {
+		}
+	}
+
+	public static class EmployeeId implements java.io.Serializable {
+		String id;
+
+		public EmployeeId() {
+		}
+
+		public EmployeeId(String id) {
+			this.id = id;
+		}
+
+		public String getId() {
+			return id;
+		}
+
+		public void setId(String id) {
+			this.id = id;
+		}
+
+
+		@Override
+		public boolean equals(Object obj) {
+			if (obj == null) {
+				return false;
+			}
+			if (getClass() != obj.getClass()) {
+				return false;
+			}
+			final EmployeeId other = (EmployeeId) obj;
+			if ((this.id == null) ? (other.id != null) : !this.id.equals(other.id)) {
+				return false;
+			}
+			return true;
+		}
+
+		@Override
+		public int hashCode() {
+			int hash = 5;
+			hash = 29 * hash + (this.id != null ? this.id.hashCode() : 0);
+			return hash;
+		}
+	}
+}