Author: max.ross Date: 2007-05-06 20:30:03 -0400 (Sun, 06 May 2007) New Revision: 11472 Modified: trunk/HibernateExt/shards/src/java/org/hibernate/shards/session/ShardedSessionImpl.java trunk/HibernateExt/shards/src/test/org/hibernate/shards/integration/model/ModelIntegrationTest.java trunk/HibernateExt/shards/src/test/org/hibernate/shards/integration/model/ModelPermutedIntegrationTest.java Log: Fix NPE when trying to update an object that doesn't already exist. Also modified code so that the user gets the same type of exception they'd get if they weren't using shards. Modified: trunk/HibernateExt/shards/src/java/org/hibernate/shards/session/ShardedSessionImpl.java =================================================================== --- trunk/HibernateExt/shards/src/java/org/hibernate/shards/session/ShardedSessionImpl.java 2007-05-06 21:50:41 UTC (rev 11471) +++ trunk/HibernateExt/shards/src/java/org/hibernate/shards/session/ShardedSessionImpl.java 2007-05-07 00:30:03 UTC (rev 11472) @@ -32,7 +32,6 @@ import org.hibernate.ReplicationMode; import org.hibernate.SQLQuery; import org.hibernate.SessionException; -import org.hibernate.SessionFactory; import org.hibernate.Transaction; import org.hibernate.TransientObjectException; import org.hibernate.classic.Session; @@ -400,7 +399,7 @@ return someSession.getCacheMode(); } - public SessionFactory getSessionFactory() { + public ShardedSessionFactoryImplementor getSessionFactory() { return shardedSessionFactory; } @@ -800,9 +799,25 @@ * The only safe way to perform the update is to load the object and then * do a merge. */ - Object persistent = get(object.getClass(), extractId(object)); - shardId = getShardIdForObject(persistent); - op.merge(shardIdsToShards.get(shardId), object); + Serializable id = extractId(object); + if(id != null) { + Object persistent = get(object.getClass(), extractId(object)); + if(persistent != null) { + shardId = getShardIdForObject(persistent); + } + } + if(shardId == null) { + /** + * This is an error condition. In order to provide the same behavior + * as a non-sharded session we're just going to dispatch the update + * to a random shard (we know it will fail because either we don't have + * an id or the lookup returned). + */ + op.update(getShards().get(0), object); + // this call may succeed but the commit will fail + } else { + op.merge(shardIdsToShards.get(shardId), object); + } } public void update(Object object) throws HibernateException { Modified: trunk/HibernateExt/shards/src/test/org/hibernate/shards/integration/model/ModelIntegrationTest.java =================================================================== --- trunk/HibernateExt/shards/src/test/org/hibernate/shards/integration/model/ModelIntegrationTest.java 2007-05-06 21:50:41 UTC (rev 11471) +++ trunk/HibernateExt/shards/src/test/org/hibernate/shards/integration/model/ModelIntegrationTest.java 2007-05-07 00:30:03 UTC (rev 11472) @@ -17,6 +17,7 @@ */ package org.hibernate.shards.integration.model; +import org.hibernate.HibernateException; import org.hibernate.shards.integration.BaseShardingIntegrationTestCase; import org.hibernate.shards.model.IdIsBaseType; @@ -45,4 +46,22 @@ assertNotNull(hli); } + public void testUpdateIdIsBasetype() { + IdIsBaseType hli = new IdIsBaseType(); + session.beginTransaction(); + hli.setValue("yamma"); + session.update(hli); + try { + session.getTransaction().commit(); + fail("expected he"); + } catch (HibernateException he) { + // good + } + resetSession(); + session.beginTransaction(); + session.saveOrUpdate(hli); + commitAndResetSession(); + hli = reload(hli); + assertNotNull(hli); + } } Modified: trunk/HibernateExt/shards/src/test/org/hibernate/shards/integration/model/ModelPermutedIntegrationTest.java =================================================================== --- trunk/HibernateExt/shards/src/test/org/hibernate/shards/integration/model/ModelPermutedIntegrationTest.java 2007-05-06 21:50:41 UTC (rev 11471) +++ trunk/HibernateExt/shards/src/test/org/hibernate/shards/integration/model/ModelPermutedIntegrationTest.java 2007-05-07 00:30:03 UTC (rev 11472) @@ -877,6 +877,32 @@ assertEquals("a different name", b.getName()); } + // calling update on a nonexistent entity should result in an exception + public void testUpdateOfNonexistentEntity() { + session.beginTransaction(); + Building b = building("b1"); + try { + session.update(b); + fail("expected HE"); + } catch (HibernateException he) { + // good + } + resetSession(); + session.beginTransaction(); + // now we assign an id and try the same thing (calling save will assign) + session.save(b); + session.getTransaction().rollback(); + resetSession(); + session.beginTransaction(); + session.update(b); + try { + session.getTransaction().commit(); + fail("expected he"); + } catch (HibernateException he) { + // good + } + } + public void testDeleteOfAttachedEntity() { session.beginTransaction(); Building b = building("b1");