Session.setReadOnly(Object, boolean) fails for proxies

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

core/src/main/java/org/hibernate/Session.java

@@ -839,15 +839,29 @@ public interface Session extends Serializable {
 	 * Get the statistics for this session.
 	 */
 	public SessionStatistics getStatistics();
-	
+
 	/**
-	 * Set an unmodified persistent object to read only mode, or a read only
+	 * Is the specified entity or proxy read-only?
-	 * object to modifiable mode. In read only mode, no snapshot is maintained
+	 * @param entityOrProxy, an entity or HibernateProxy
+	 * @return true, the entity or proxy is read-only;
+	 *         false, the entity or proxy is modifiable.
+	 */
+	public boolean isReadOnly(Object entityOrProxy);
+
+	/**
+	 * Set an unmodified persistent object to read-only mode, or a read-only
+	 * object to modifiable mode. In read-only mode, no snapshot is maintained
 	 * and the instance is never dirty checked.
+	 *
+	 * If the entity or proxy already has the specified read-only/modifiable
+	 * setting, then this method does nothing.
 	 * 
+	 * @param entityOrProxy, an entity or HibernateProxy
+	 * @param readOnly, if true, the entity or proxy is made read-only;
+	 *                  if false, the entity or proxy is made modifiable.
 	 * @see Query#setReadOnly(boolean)
 	 */
-	public void setReadOnly(Object entity, boolean readOnly);
+	public void setReadOnly(Object entityOrProxy, boolean readOnly);
 
 	/**
 	 * Controller for allowing users to perform JDBC related work using the Connection

core/src/main/java/org/hibernate/engine/EntityEntry.java

@@ -261,22 +261,25 @@ public final class EntityEntry implements Serializable {
 		);
 	}
 
+	public boolean isReadOnly() {
+		if (status != Status.MANAGED && status != Status.READ_ONLY) {
+			throw new HibernateException("instance was not in a valid state");
+		}
+		return status == Status.READ_ONLY;
+	}
+
 	public void setReadOnly(boolean readOnly, Object entity) {
-		if ( ( readOnly && status == Status.READ_ONLY ) ||
+		if ( readOnly == isReadOnly() ) {
-			 ( ( ! readOnly ) && status == Status.MANAGED ) ) {
 			// simply return since the status is not being changed
 			return;
 		}
-		if (status!=Status.MANAGED && status!=Status.READ_ONLY) {
+		if ( readOnly ) {
-			throw new HibernateException("instance was not in a valid state");
+			setStatus( Status.READ_ONLY );
-		}
-		if (readOnly) {
-			setStatus(Status.READ_ONLY);
 			loadedState = null;
 		}
 		else {
-			setStatus(Status.MANAGED);
+			setStatus( Status.MANAGED );
-			loadedState = getPersister().getPropertyValues(entity, entityMode);
+			loadedState = getPersister().getPropertyValues( entity, entityMode );
 		}
 	}
 	

core/src/main/java/org/hibernate/engine/PersistenceContext.java

@@ -475,11 +475,26 @@ public interface PersistenceContext {
 	 * Is the association property belonging to the keyed entity null?
 	 */
 	public boolean isPropertyNull(EntityKey ownerKey, String propertyName);
-	
+
 	/**
-	 * Set the object to read only and discard it's snapshot
+	 * Is the entity or proxy read-only?
+	 *
+	 * @param entityOrProxy
+	 * @return true, the object is read-only; false, the object is modifiable.
 	 */
-	public void setReadOnly(Object entity, boolean readOnly);
+	public boolean isReadOnly(Object entityOrProxy);
+
+	/**
+	 * Set the entity or proxy to read only and discard it's snapshot.
+	 *
+	 * If the entity or proxy already has the specified read-only/modifiable
+	 * setting, then this method does nothing.
+	 *
+	 * @param entityOrProxy, an entity or HibernateProxy
+	 * @param readOnly, if true, the entity or proxy is made read-only;
+	 *                  if false, the entity or proxy is made modifiable.
+	 */
+	public void setReadOnly(Object entityOrProxy, boolean readOnly);
 
 	void replaceDelayedEntityIdentityInsertKeys(EntityKey oldKey, Serializable generatedId);
 

core/src/main/java/org/hibernate/engine/StatefulPersistenceContext.java

@@ -1302,13 +1302,67 @@ public class StatefulPersistenceContext implements PersistenceContext {
 		nullAssociations.clear();
 	}
 
-	public void setReadOnly(Object entity, boolean readOnly) {
+	public boolean isReadOnly(Object entityOrProxy) {
-		EntityEntry entry = getEntry(entity);
+		if ( entityOrProxy == null ) {
-		if (entry==null) {
+			throw new AssertionFailure( "object must be non-null." );
-			throw new TransientObjectException("Instance was not associated with the session");
 		}
-		entry.setReadOnly(readOnly, entity);
+		boolean isReadOnly;
-		hasNonReadOnlyEntities = hasNonReadOnlyEntities || !readOnly;
+		if ( entityOrProxy instanceof HibernateProxy ) {
+			isReadOnly = ( ( HibernateProxy ) entityOrProxy ).getHibernateLazyInitializer().isReadOnly();
+		}
+		else {
+			EntityEntry ee =  getEntry( entityOrProxy );
+			if ( ee == null ) {
+				throw new TransientObjectException("Instance was not associated with this persistence context" );
+			}
+			isReadOnly = ee.isReadOnly();
+		}
+		return isReadOnly;
+	}
+
+	public void setReadOnly(Object object, boolean readOnly) {
+		if ( object == null ) {
+			throw new AssertionFailure( "object must be non-null." );
+		}
+		if ( isReadOnly( object ) == readOnly ) {
+			return;
+		}
+		if ( object instanceof HibernateProxy ) {
+			HibernateProxy proxy = ( HibernateProxy ) object;
+			setProxyReadOnly( proxy, readOnly );
+			if ( Hibernate.isInitialized( proxy ) ) {
+				setEntityReadOnly(
+						proxy.getHibernateLazyInitializer().getImplementation(),
+						readOnly
+				);
+			}
+		}
+		else {
+			setEntityReadOnly( object, readOnly );
+			// PersistenceContext.proxyFor( entity ) returns entity if there is no proxy for that entity
+			// so need to check the return value to be sure it is really a proxy
+			Object maybeProxy = getSession().getPersistenceContext().proxyFor( object );
+			if ( maybeProxy instanceof HibernateProxy ) {
+				setProxyReadOnly( ( HibernateProxy ) maybeProxy, readOnly );
+			}
+		}
+	}
+
+	private void setProxyReadOnly(HibernateProxy proxy, boolean readOnly) {
+		if ( proxy.getHibernateLazyInitializer().getSession() != getSession() ) {
+			throw new AssertionFailure(
+					"Attempt to set a proxy to read-only that is associated with a different session" );
+		}
+		proxy.getHibernateLazyInitializer().setReadOnly( readOnly );
+	}
+
+	private void setEntityReadOnly(Object entity, boolean readOnly) {
+		EntityEntry entry = getEntry(entity);
+		if (entry == null) {
+			throw new TransientObjectException("Instance was not associated with this persistence context" );
+		}
+		entry.setReadOnly(readOnly, entity );
+		hasNonReadOnlyEntities = hasNonReadOnlyEntities || ! readOnly;
 	}
 
 	public void replaceDelayedEntityIdentityInsertKeys(EntityKey oldKey, Serializable generatedId) {

core/src/main/java/org/hibernate/engine/TwoPhaseLoad.java

@@ -32,6 +32,7 @@ import org.hibernate.AssertionFailure;
 import org.hibernate.CacheMode;
 import org.hibernate.HibernateException;
 import org.hibernate.LockMode;
+import org.hibernate.proxy.HibernateProxy;
 import org.hibernate.cache.CacheKey;
 import org.hibernate.cache.entry.CacheEntry;
 import org.hibernate.event.PostLoadEvent;
@@ -188,8 +189,18 @@ public final class TwoPhaseLoad {
 				factory.getStatisticsImplementor().secondLevelCachePut( persister.getCacheAccessStrategy().getRegion().getName() );
 			}
 		}
-	
+
-		if ( readOnly || !persister.isMutable() ) {
+		boolean isReallyReadOnly = readOnly || !persister.isMutable();
+		Object proxy = persistenceContext.getProxy(
+				new EntityKey(entityEntry.getId(), entityEntry.getPersister(), session.getEntityMode()
+			)
+		);
+		if ( proxy != null ) {
+			// there is already a proxy for this impl
+			// only set the status to read-only if the proxy is read-only
+			isReallyReadOnly = ( ( HibernateProxy ) proxy ).getHibernateLazyInitializer().isReadOnly();
+		}
+		if ( isReallyReadOnly ) {
 			//no need to take a snapshot - this is a 
 			//performance optimization, but not really
 			//important, except for entities with huge 

core/src/main/java/org/hibernate/impl/SessionImpl.java

@@ -1956,6 +1956,12 @@ public final class SessionImpl extends AbstractSessionImpl
 		return true;
 	}
 
+	public boolean isReadOnly(Object entityOrProxy) {
+		errorIfClosed();
+		checkTransactionSynchStatus();
+		return persistenceContext.isReadOnly( entityOrProxy );	
+	}
+
 	public void setReadOnly(Object entity, boolean readOnly) {
 		errorIfClosed();
 		checkTransactionSynchStatus();

core/src/main/java/org/hibernate/proxy/AbstractLazyInitializer.java

@@ -28,6 +28,8 @@ import java.io.Serializable;
 
 import org.hibernate.HibernateException;
 import org.hibernate.LazyInitializationException;
+import org.hibernate.TransientObjectException;
+import org.hibernate.SessionException;
 import org.hibernate.engine.EntityKey;
 import org.hibernate.engine.SessionImplementor;
 
@@ -43,8 +45,8 @@ public abstract class AbstractLazyInitializer implements LazyInitializer {
 	private Serializable id;
 	private Object target;
 	private boolean initialized;
+	private boolean readOnly;
 	private boolean unwrap;
-
 	private transient SessionImplementor session;
 
 	/**
@@ -63,7 +65,15 @@ public abstract class AbstractLazyInitializer implements LazyInitializer {
 	protected AbstractLazyInitializer(String entityName, Serializable id, SessionImplementor session) {
 		this.entityName = entityName;
 		this.id = id;
-		this.session = session;
+		// initialize other fields depending on session state
+		if ( session == null ) {
+			// would be better to call unsetSession(), but it is not final...
+			session = null;
+			readOnly = false;
+		}
+		else {
+			setSession( session );
+		}
 	}
 
 	/**
@@ -108,7 +118,9 @@ public abstract class AbstractLazyInitializer implements LazyInitializer {
 		if ( s != session ) {
 			// check for s == null first, since it is least expensive
 			if ( s == null ){
-				unsetSession();
+				// would be better to call unsetSession(), but it is not final...
+				session = null;
+				readOnly = false;
 			}
 			else if ( isConnectedToSession() ) {
 				//TODO: perhaps this should be some other RuntimeException...
@@ -116,6 +128,8 @@ public abstract class AbstractLazyInitializer implements LazyInitializer {
 			}
 			else {
 				session = s;
+				// NOTE: the proxy may not be connected to the session yet, so set readOnly directly
+				readOnly = ! session.getFactory().getEntityPersister( entityName ).isMutable();
 			}
 		}
 	}
@@ -132,6 +146,7 @@ public abstract class AbstractLazyInitializer implements LazyInitializer {
 	 */
 	public void unsetSession() {
 		session = null;
+		readOnly = false;
 	}
 
 	/**
@@ -220,6 +235,48 @@ public abstract class AbstractLazyInitializer implements LazyInitializer {
 		return target;
 	}
 
+	/**
+	 * {@inheritDoc}
+	 */
+	public boolean isReadOnly() {
+		errorIfReadOnlySettingNotAvailable();
+		if ( !isConnectedToSession() ) {
+			throw new TransientObjectException(
+					"The read-only/modifiable setting is only accessible when the proxy is associated with a session." );
+		}
+		return readOnly;
+	}
+
+	/**
+	 * {@inheritDoc}
+	 */
+	public void setReadOnly(boolean readOnly) {
+		errorIfReadOnlySettingNotAvailable();
+		// only update if readOnly is different from current setting
+		if ( this.readOnly != readOnly ) {
+			Object proxy = getProxyOrNull();
+			if ( proxy == null ) {
+				throw new TransientObjectException(
+						"Cannot set the read-only/modifiable mode unless the proxy is associated with a session." );
+			}
+			this.readOnly = readOnly;
+			if ( initialized ) {
+				session.getPersistenceContext().setReadOnly( target, readOnly );
+			}
+		}
+	}
+
+	private void errorIfReadOnlySettingNotAvailable() {
+		if ( session == null ) {
+			throw new TransientObjectException(
+					"Proxy is detached (i.e, session is null). The read-only/modifiable setting is only accessible when the proxy is associated with an open session." );
+		}
+		if ( session.isClosed() ) {
+			throw new SessionException(
+					"Session is closed. The read-only/modifiable setting is only accessible when the proxy is associated with an open session." );
+		}
+	}
+
 	/**
 	 * {@inheritDoc}
 	 */

core/src/main/java/org/hibernate/proxy/LazyInitializer.java

@@ -104,6 +104,37 @@ public interface LazyInitializer {
 	 */
 	public void setImplementation(Object target);
 
+	/**
+	 * Is the proxy read-only?.
+	 *
+	 * @return true, if this proxy is read-only; false, otherwise
+	 * @throws org.hibernate.TransientObjectException if the proxy is not association with a session
+	 * @throws org.hibernate.SessionException if the proxy is associated with a sesssion that is closed
+	 *
+	 * @see org.hibernate.Session#isReadOnly(Object entityOrProxy)
+	 */
+	public boolean isReadOnly();
+
+	/**
+	 * Set an associated modifiable proxy to read-only mode, or a read-only
+	 * proxy to modifiable mode. If the proxy is currently initialized, its
+	 * implementation will be set to the same mode; otherwise, when the
+	 * proxy is initialized, its implementation will have the same read-only/
+	 * modifiable setting as the proxy. In read-only mode, no snapshot is
+	 * maintained and the instance is never dirty checked.
+	 *
+	 * If the associated proxy already has the specified read-only/modifiable
+	 * setting, then this method does nothing.
+	 *
+	 * @param readOnly, if true, the associated proxy is made read-only;
+	 *                  if false, the associated proxy is made modifiable.
+	 * @throws org.hibernate.TransientObjectException if the proxy is not association with a session
+	 * @throws org.hibernate.SessionException if the proxy is associated with a sesssion that is closed
+	 * 
+	 * @see org.hibernate.Session#setReadOnly(Object entityOrProxy, boolean readOnly)
+	 */
+	public void setReadOnly(boolean readOnly);
+
 	/**
 	 * Get the session to which this proxy is associated, or null if it is not attached.
 	 *

testsuite/src/test/java/org/hibernate/test/readonly/ReadOnlyTest.java

@@ -67,7 +67,7 @@ public class ReadOnlyTest extends FunctionalTestCase {
 		return new FunctionalTestClassTestSuite( ReadOnlyTest.class );
 	}
 
-	public void testReadOnlyOnProxiesFailureExpected() {
+	public void testReadOnlyOnProxies() {
 		Session s = openSession();
 		s.setCacheMode( CacheMode.IGNORE );
 		s.beginTransaction();
@@ -82,6 +82,7 @@ public class ReadOnlyTest extends FunctionalTestCase {
 
 		s = openSession();
 		s.setCacheMode(CacheMode.IGNORE);
+		s.beginTransaction();
 		dp = ( DataPoint ) s.load( DataPoint.class, new Long( dpId ) );
 		assertFalse( "was initialized", Hibernate.isInitialized( dp ) );
 		s.setReadOnly( dp, true );
@@ -178,6 +179,138 @@ public class ReadOnlyTest extends FunctionalTestCase {
 
 	}
 
+	public void testReadOnlyDelete() {
+
+		Session s = openSession();
+		s.setCacheMode(CacheMode.IGNORE);
+		Transaction t = s.beginTransaction();
+		DataPoint dp = new DataPoint();
+		dp.setX( new BigDecimal(0.1d).setScale(19, BigDecimal.ROUND_DOWN) );
+		dp.setY( new BigDecimal( Math.cos( dp.getX().doubleValue() ) ).setScale(19, BigDecimal.ROUND_DOWN) );
+		s.save(dp);
+		t.commit();
+		s.close();
+
+		s = openSession();
+		s.setCacheMode(CacheMode.IGNORE);
+		t = s.beginTransaction();
+		dp = ( DataPoint ) s.get( DataPoint.class, dp.getId() );
+		s.setReadOnly( dp, true );
+		s.delete(  dp );
+		t.commit();
+		s.close();
+
+		s = openSession();
+		t = s.beginTransaction();
+		List list = s.createQuery("from DataPoint where description='done!'").list();
+		assertTrue( list.isEmpty() );
+		t.commit();
+		s.close();
+
+	}
+
+	public void testReadOnlyModeWithExistingModifiableEntity() {
+
+		Session s = openSession();
+		s.setCacheMode(CacheMode.IGNORE);
+		Transaction t = s.beginTransaction();
+		DataPoint dp = null;
+		for ( int i=0; i<100; i++ ) {
+			dp = new DataPoint();
+			dp.setX( new BigDecimal(i * 0.1d).setScale(19, BigDecimal.ROUND_DOWN) );
+			dp.setY( new BigDecimal( Math.cos( dp.getX().doubleValue() ) ).setScale(19, BigDecimal.ROUND_DOWN) );
+			s.save(dp);
+		}
+		t.commit();
+		s.close();
+
+		s = openSession();
+		s.setCacheMode(CacheMode.IGNORE);
+		t = s.beginTransaction();
+		DataPoint dpLast = ( DataPoint ) s.get( DataPoint.class,  dp.getId() );
+		assertFalse( s.isReadOnly( dpLast ) );
+		int i = 0;
+		ScrollableResults sr = s.createQuery("from DataPoint dp order by dp.x asc")
+				.setReadOnly(true)
+				.scroll(ScrollMode.FORWARD_ONLY);
+		int nExpectedChanges = 0;
+		while ( sr.next() ) {
+			dp = (DataPoint) sr.get(0);
+			if ( dp.getId() == dpLast.getId() ) {
+				//dpLast existed in the session before executing the read-only query
+				assertFalse( s.isReadOnly( dp ) );
+			}
+			else {
+				assertTrue( s.isReadOnly( dp ) );
+			}
+			if (++i==50) {
+				s.setReadOnly(dp, false);
+				nExpectedChanges = ( dp == dpLast ? 1 : 2 );
+			}
+			dp.setDescription("done!");
+		}
+		t.commit();
+		s.clear();
+		t = s.beginTransaction();
+		List list = s.createQuery("from DataPoint where description='done!'").list();
+		assertEquals( list.size(), nExpectedChanges );
+		s.createQuery("delete from DataPoint").executeUpdate();
+		t.commit();
+		s.close();
+	}
+
+	public void testModifiableModeWithExistingReadOnlyEntity() {
+
+		Session s = openSession();
+		s.setCacheMode(CacheMode.IGNORE);
+		Transaction t = s.beginTransaction();
+		DataPoint dp = null;
+		for ( int i=0; i<100; i++ ) {
+			dp = new DataPoint();
+			dp.setX( new BigDecimal(i * 0.1d).setScale(19, BigDecimal.ROUND_DOWN) );
+			dp.setY( new BigDecimal( Math.cos( dp.getX().doubleValue() ) ).setScale(19, BigDecimal.ROUND_DOWN) );
+			s.save(dp);
+		}
+		t.commit();
+		s.close();
+
+		s = openSession();
+		s.setCacheMode(CacheMode.IGNORE);
+		t = s.beginTransaction();
+		DataPoint dpLast = ( DataPoint ) s.get( DataPoint.class,  dp.getId() );
+		assertFalse( s.isReadOnly( dpLast ) );
+		s.setReadOnly( dpLast, true );
+		assertTrue( s.isReadOnly( dpLast ) );
+		int i = 0;
+		ScrollableResults sr = s.createQuery("from DataPoint dp order by dp.x asc")
+				.setReadOnly(false)
+				.scroll(ScrollMode.FORWARD_ONLY);
+		int nExpectedChanges = 0;
+		while ( sr.next() ) {
+			dp = (DataPoint) sr.get(0);
+			if ( dp.getId() == dpLast.getId() ) {
+				//dpLast existed in the session before executing the read-only query
+				assertTrue( s.isReadOnly( dp ) );
+			}
+			else {
+				assertFalse( s.isReadOnly( dp ) );
+			}
+			if (++i==50) {
+				s.setReadOnly(dp, true);
+				nExpectedChanges = ( dp == dpLast ? 99 : 98 );
+			}
+			dp.setDescription("done!");
+		}
+		t.commit();
+		s.clear();
+		t = s.beginTransaction();
+		List list = s.createQuery("from DataPoint where description='done!'").list();
+		assertEquals( list.size(), nExpectedChanges );
+		s.createQuery("delete from DataPoint").executeUpdate();
+		t.commit();
+		s.close();
+	}
+
 	public void testReadOnlyOnTextType() {
 		final String origText = "some huge text string";
 		final String newText = "some even bigger text string";