Author: steve.ebersole(a)jboss.com Date: 2006-11-13 13:53:57 -0500 (Mon, 13 Nov 2006) New Revision: 10796 Modified: branches/Branch_3_2/Hibernate3/src/org/hibernate/event/def/DefaultLoadEventListener.java branches/Branch_3_2/Hibernate3/test/org/hibernate/test/discriminator/DiscriminatorTest.java branches/Branch_3_2/Hibernate3/test/org/hibernate/test/joinedsubclass/JoinedSubclassTest.java Log: HHH-1460 : subclass/get consistency Modified: branches/Branch_3_2/Hibernate3/src/org/hibernate/event/def/DefaultLoadEventListener.java =================================================================== --- branches/Branch_3_2/Hibernate3/src/org/hibernate/event/def/DefaultLoadEventListener.java 2006-11-13 18:52:33 UTC (rev 10795) +++ branches/Branch_3_2/Hibernate3/src/org/hibernate/event/def/DefaultLoadEventListener.java 2006-11-13 18:53:57 UTC (rev 10796) @@ -8,7 +8,6 @@ import org.hibernate.HibernateException; import org.hibernate.LockMode; import org.hibernate.NonUniqueObjectException; -import org.hibernate.ObjectDeletedException; import org.hibernate.PersistentObjectException; import org.hibernate.TypeMismatchException; import org.hibernate.EntityMode; @@ -43,9 +42,12 @@ */ public class DefaultLoadEventListener extends AbstractLockUpgradeEventListener implements LoadEventListener { + public static final Object REMOVED_ENTITY_MARKER = new Object(); + public static final Object INCONSISTENT_RTN_CLASS_MARKER = new Object(); + public static final LockMode DEFAULT_LOCK_MODE = LockMode.NONE; + private static final Log log = LogFactory.getLog(DefaultLoadEventListener.class); - public static final LockMode DEFAULT_LOCK_MODE = LockMode.NONE; /** * Handle the given load event. @@ -306,7 +308,6 @@ return proxy; } - protected static final Object REMOVED_ENTITY_MARKER = new Object(); /** * Coordinates the efforts to load a given entity. First, an attempt is @@ -314,15 +315,18 @@ * an attempt is made to locate it in second-level cache. Lastly, an * attempt is made to load it directly from the datasource. * - * @return The loaded entity. + * @param event The load event + * @param persister The persister for the entity being requested for load + * @param keyToLoad The EntityKey representing the entity to be loaded. + * @param options The load options. + * @return The loaded entity, or null. * @throws HibernateException */ protected Object doLoad( - final LoadEvent event, - final EntityPersister persister, - final EntityKey keyToLoad, - final LoadEventListener.LoadType options) - throws HibernateException { + final LoadEvent event, + final EntityPersister persister, + final EntityKey keyToLoad, + final LoadEventListener.LoadType options) throws HibernateException { if ( log.isTraceEnabled() ) { log.trace( @@ -331,11 +335,15 @@ ); } - Object entity = loadFromSessionCache(event, keyToLoad, options); + Object entity = loadFromSessionCache( event, keyToLoad, options ); if ( entity == REMOVED_ENTITY_MARKER ) { log.debug( "load request found matching entity in context, but it is scheduled for removal; returning null" ); return null; } + if ( entity == INCONSISTENT_RTN_CLASS_MARKER ) { + log.debug( "load request found matching entity in context, but the matched entity was of an inconsistent return type; returning null" ); + return null; + } if ( entity != null ) { if ( log.isTraceEnabled() ) { log.trace( @@ -346,13 +354,6 @@ return entity; } - // Entity not found in session; before going any further, see if we - // already determined that this entity does not exist - /*if ( event.getSession().getPersistenceContext().isNonExistant(keyToLoad) ) { - if ( log.isTraceEnabled() ) log.trace("entity does not exist"); - return null; - }*/ - entity = loadFromSecondLevelCache(event, persister, options); if ( entity != null ) { if ( log.isTraceEnabled() ) { @@ -378,30 +379,26 @@ * Performs the process of loading an entity from the configured * underlying datasource. * + * @param event The load event + * @param persister The persister for the entity being requested for load + * @param keyToLoad The EntityKey representing the entity to be loaded. + * @param options The load options. * @return The object loaded from the datasource, or null if not found. * @throws HibernateException */ protected Object loadFromDatasource( - final LoadEvent event, - final EntityPersister persister, - final EntityKey keyToLoad, - final LoadEventListener.LoadType options) - throws HibernateException { - + final LoadEvent event, + final EntityPersister persister, + final EntityKey keyToLoad, + final LoadEventListener.LoadType options) throws HibernateException { final SessionImplementor source = event.getSession(); - Object entity = persister.load( - event.getEntityId(), - event.getInstanceToLoad(), - event.getLockMode(), + event.getEntityId(), + event.getInstanceToLoad(), + event.getLockMode(), source - ); - - /*if ( entity == null ) { - //remember it doesn't exist, in case of next time - source.getPersistenceContext().addNonExistantEntityKey(keyToLoad); - }*/ - + ); + if ( event.isAssociationFetch() && source.getFactory().getStatistics().isStatisticsEnabled() ) { source.getFactory().getStatisticsImplementor().fetchEntity( event.getEntityClassName() ); } @@ -410,22 +407,30 @@ } /** - * Attempts to locate the entity in the session-level cache. If - * checkDeleted was set to true, then if the entity is found in the + * Attempts to locate the entity in the session-level cache. + * <p/> + * If allowed to return nulls, then if the entity happens to be found in + * the session cache, we check the entity type for proper handling + * of entity hierarchies. + * <p/> + * If checkDeleted was set to true, then if the entity is found in the * session-level cache, it's current status within the session cache * is checked to see if it has previously been scheduled for deletion. * + * @param event The load event + * @param keyToLoad The EntityKey representing the entity to be loaded. + * @param options The load options. * @return The entity from the session-level cache, or null. - * @throws HibernateException + * @throws HibernateException Generally indicates problems applying a lock-mode. */ protected Object loadFromSessionCache( - final LoadEvent event, - final EntityKey keyToLoad, - final LoadEventListener.LoadType options) - throws HibernateException { + final LoadEvent event, + final EntityKey keyToLoad, + final LoadEventListener.LoadType options) throws HibernateException { SessionImplementor session = event.getSession(); Object old = session.getEntityUsingInterceptor( keyToLoad ); + if ( old != null ) { // this object was already loaded EntityEntry oldEntry = session.getPersistenceContext().getEntry( old ); @@ -435,22 +440,31 @@ return REMOVED_ENTITY_MARKER; } } + if ( options.isAllowNulls() ) { + EntityPersister persister = event.getSession().getFactory().getEntityPersister( event.getEntityClassName() ); + if ( ! persister.isInstance( old, event.getSession().getEntityMode() ) ) { + return INCONSISTENT_RTN_CLASS_MARKER; + } + } upgradeLock( old, oldEntry, event.getLockMode(), session ); } + return old; } /** * Attempts to load the entity from the second-level cache. * + * @param event The load event + * @param persister The persister for the entity being requested for load + * @param options The load options. * @return The entity from the second-level cache, or null. * @throws HibernateException */ protected Object loadFromSecondLevelCache( - final LoadEvent event, - final EntityPersister persister, - final LoadEventListener.LoadType options) - throws HibernateException { + final LoadEvent event, + final EntityPersister persister, + final LoadEventListener.LoadType options) throws HibernateException { final SessionImplementor source = event.getSession(); @@ -504,11 +518,10 @@ } private Object assembleCacheEntry( - final CacheEntry entry, - final Serializable id, - final EntityPersister persister, - final LoadEvent event) - throws HibernateException { + final CacheEntry entry, + final Serializable id, + final EntityPersister persister, + final LoadEvent event) throws HibernateException { final Object optionalObject = event.getInstanceToLoad(); final EventSource session = event.getSession(); Modified: branches/Branch_3_2/Hibernate3/test/org/hibernate/test/discriminator/DiscriminatorTest.java =================================================================== --- branches/Branch_3_2/Hibernate3/test/org/hibernate/test/discriminator/DiscriminatorTest.java 2006-11-13 18:52:33 UTC (rev 10795) +++ branches/Branch_3_2/Hibernate3/test/org/hibernate/test/discriminator/DiscriminatorTest.java 2006-11-13 18:53:57 UTC (rev 10796) @@ -3,6 +3,7 @@ import java.util.Iterator; import java.util.List; +import java.math.BigDecimal; import junit.framework.Test; import junit.framework.TestSuite; @@ -10,6 +11,7 @@ import org.hibernate.Hibernate; import org.hibernate.Session; import org.hibernate.Transaction; +import org.hibernate.criterion.Property; import org.hibernate.test.TestCase; /** @@ -20,11 +22,11 @@ public DiscriminatorTest(String str) { super(str); } - - public void testJoinedSubclass() { + + public void testDiscriminatorSubclass() { Session s = openSession(); Transaction t = s.beginTransaction(); - + Employee mark = new Employee(); mark.setName("Mark"); mark.setTitle("internal sales"); @@ -32,7 +34,7 @@ mark.setAddress("buckhead"); mark.setZip("30305"); mark.setCountry("USA"); - + Customer joe = new Customer(); joe.setName("Joe"); joe.setAddress("San Francisco"); @@ -41,17 +43,17 @@ joe.setComments("Very demanding"); joe.setSex('M'); joe.setSalesperson(mark); - + Person yomomma = new Person(); yomomma.setName("mum"); yomomma.setSex('F'); - + s.save(yomomma); s.save(mark); s.save(joe); - + assertEquals( s.createQuery("from java.io.Serializable").list().size(), 0 ); - + assertEquals( s.createQuery("from Person").list().size(), 3 ); assertEquals( s.createQuery("from Person p where p.class = Person").list().size(), 1 ); assertEquals( s.createQuery("from Person p where p.class = Customer").list().size(), 1 ); @@ -65,7 +67,7 @@ } assertEquals( customers.size(), 1 ); s.clear(); - + customers = s.createQuery("from Customer").list(); for ( Iterator iter = customers.iterator(); iter.hasNext(); ) { Customer c = (Customer) iter.next(); @@ -74,11 +76,11 @@ } assertEquals( customers.size(), 1 ); s.clear(); - + mark = (Employee) s.get( Employee.class, new Long( mark.getId() ) ); joe = (Customer) s.get( Customer.class, new Long( joe.getId() ) ); - + mark.setZip("30306"); assertEquals( s.createQuery("from Person p where p.address.zip = '30306'").list().size(), 1 ); s.delete(mark); @@ -89,6 +91,78 @@ s.close(); } + public void testAccessAsIncorrectSubclass() { + Session s = openSession(); + s.beginTransaction(); + Employee e = new Employee(); + e.setName( "Steve" ); + e.setSex( 'M' ); + e.setTitle( "grand poobah" ); + s.save( e ); + s.getTransaction().commit(); + s.close(); + + s = openSession(); + s.beginTransaction(); + Customer c = ( Customer ) s.get( Customer.class, new Long( e.getId() ) ); + s.getTransaction().commit(); + s.close(); + assertNull( c ); + + s = openSession(); + s.beginTransaction(); + e = ( Employee ) s.get( Employee.class, new Long( e.getId() ) ); + c = ( Customer ) s.get( Customer.class, new Long( e.getId() ) ); + s.getTransaction().commit(); + s.close(); + assertNotNull( e ); + assertNull( c ); + + s = openSession(); + s.beginTransaction(); + s.delete( e ); + s.getTransaction().commit(); + s.close(); + } + + public void testQuerySubclassAttribute() { + Session s = openSession(); + Transaction t = s.beginTransaction(); + Person p = new Person(); + p.setName("Emmanuel"); + p.setSex('M'); + s.persist(p); + Employee q = new Employee(); + q.setName("Steve"); + q.setSex('M'); + q.setTitle("Mr"); + q.setSalary( new BigDecimal(1000) ); + s.persist(q); + + List result = s.createQuery("from Person where salary > 100").list(); + assertEquals( result.size(), 1 ); + assertSame( result.get(0), q ); + + result = s.createQuery("from Person where salary > 100 or name like 'E%'").list(); + assertEquals( result.size(), 2 ); + + result = s.createCriteria(Person.class) + .add( Property.forName("salary").gt( new BigDecimal(100) ) ) + .list(); + assertEquals( result.size(), 1 ); + assertSame( result.get(0), q ); + + //TODO: make this work: + /*result = s.createQuery("select salary from Person where salary > 100").list(); + assertEquals( result.size(), 1 ); + assertEquals( result.get(0), new BigDecimal(1000) );*/ + + s.delete(p); + s.delete(q); + t.commit(); + s.close(); + } + protected String[] getMappings() { return new String[] { "discriminator/Person.hbm.xml" }; Modified: branches/Branch_3_2/Hibernate3/test/org/hibernate/test/joinedsubclass/JoinedSubclassTest.java =================================================================== --- branches/Branch_3_2/Hibernate3/test/org/hibernate/test/joinedsubclass/JoinedSubclassTest.java 2006-11-13 18:52:33 UTC (rev 10795) +++ branches/Branch_3_2/Hibernate3/test/org/hibernate/test/joinedsubclass/JoinedSubclassTest.java 2006-11-13 18:53:57 UTC (rev 10796) @@ -14,10 +14,6 @@ import org.hibernate.LockMode; import org.hibernate.criterion.Expression; import org.hibernate.criterion.Property; -import org.hibernate.dialect.DB2Dialect; -import org.hibernate.dialect.HSQLDialect; -import org.hibernate.dialect.MySQLDialect; -import org.hibernate.dialect.PostgreSQLDialect; import org.hibernate.test.TestCase; /** @@ -28,11 +24,11 @@ public JoinedSubclassTest(String str) { super(str); } - + public void testJoinedSubclass() { Session s = openSession(); Transaction t = s.beginTransaction(); - + Employee mark = new Employee(); mark.setName("Mark"); mark.setTitle("internal sales"); @@ -40,7 +36,7 @@ mark.setAddress("buckhead"); mark.setZip("30305"); mark.setCountry("USA"); - + Customer joe = new Customer(); joe.setName("Joe"); joe.setAddress("San Francisco"); @@ -49,17 +45,17 @@ joe.setComments("Very demanding"); joe.setSex('M'); joe.setSalesperson(mark); - + Person yomomma = new Person(); yomomma.setName("mum"); yomomma.setSex('F'); - + s.save(yomomma); s.save(mark); s.save(joe); - + assertEquals( s.createQuery("from java.io.Serializable").list().size(), 0 ); - + assertEquals( s.createQuery("from Person").list().size(), 3 ); assertEquals( s.createQuery("from Person p where p.class = Customer").list().size(), 1 ); assertEquals( s.createQuery("from Person p where p.class = Person").list().size(), 1 ); @@ -73,7 +69,7 @@ } assertEquals( customers.size(), 1 ); s.clear(); - + customers = s.createQuery("from Customer").list(); for ( Iterator iter = customers.iterator(); iter.hasNext(); ) { Customer c = (Customer) iter.next(); @@ -82,20 +78,20 @@ } assertEquals( customers.size(), 1 ); s.clear(); - + mark = (Employee) s.get( Employee.class, new Long( mark.getId() ) ); joe = (Customer) s.get( Customer.class, new Long( joe.getId() ) ); - + mark.setZip("30306"); assertEquals( s.createQuery("from Person p where p.address.zip = '30306'").list().size(), 1 ); if ( supportsRowValueConstructorSyntaxInInList() ) { - s.createCriteria(Person.class).add( - Expression.in("address", new Address[] { mark.getAddress(), joe.getAddress() } ) + s.createCriteria(Person.class).add( + Expression.in("address", new Address[] { mark.getAddress(), joe.getAddress() } ) ).list(); } - + s.delete(mark); s.delete(joe); s.delete(yomomma); @@ -103,6 +99,40 @@ t.commit(); s.close(); } + + public void testAccessAsIncorrectSubclass() { + Session s = openSession(); + s.beginTransaction(); + Employee e = new Employee(); + e.setName( "Steve" ); + e.setSex( 'M' ); + e.setTitle( "grand poobah" ); + s.save( e ); + s.getTransaction().commit(); + s.close(); + + s = openSession(); + s.beginTransaction(); + Customer c = ( Customer ) s.get( Customer.class, new Long( e.getId() ) ); + s.getTransaction().commit(); + s.close(); + assertNull( c ); + + s = openSession(); + s.beginTransaction(); + e = ( Employee ) s.get( Employee.class, new Long( e.getId() ) ); + c = ( Customer ) s.get( Customer.class, new Long( e.getId() ) ); + s.getTransaction().commit(); + s.close(); + assertNotNull( e ); + assertNull( c ); + + s = openSession(); + s.beginTransaction(); + s.delete( e ); + s.getTransaction().commit(); + s.close(); + } public void testQuerySubclassAttribute() { Session s = openSession();