Changeset 1031

Timestamp:

10/11/06 11:16:07 (2 years ago)

Author:

jmoore

Message:

#326 Reworking of SessionHandler?

Location:

trunk/components/server

Files:

• resources/ome/services/hibernate.xml (modified) (1 diff)
• resources/ome/services/out-of-container.xml (modified) (1 diff)
• resources/ome/services/service-ome.api.RawPixelsStore.xml (modified) (1 diff)
• resources/ome/services/service-omeis.providers.re.RenderingEngine.xml (modified) (1 diff)
• resources/ome/services/services.xml (modified) (2 diffs)
• src/ome/tools/hibernate/SessionHandler.java (modified) (8 diffs)
• test/ome/server/utests/handlers/SessionHandlerMockHibernateTest.java (modified) (19 diffs)

trunk/components/server/resources/ome/services/hibernate.xml

@@ -65 +65 @@
     <constructor-arg ref="sessionFactory"/>    
   </bean>
-    
+
+  <bean id="hibernateHandler" class="org.springframework.orm.hibernate3.HibernateInterceptor">
+    <description>
+    Scope: private
+    </description>
+    <property name="sessionFactory" ref="sessionFactory"/>
+  </bean>
+
   <bean id="hibernateTemplate" class="org.springframework.orm.hibernate3.HibernateTemplate">
     <description>

trunk/components/server/resources/ome/services/out-of-container.xml

@@ -59 +59 @@
   
   <bean id="transactionManager" lazy-init="true"
-        class="org.springframework.orm.hibernate3.HibernateTransactionManager">
+        class="org.springframework.jdbc.datasource.DataSourceTransactionManager">
     <description>
     Scope: private
     </description>
-    <property name="dataSource" ref="dataSource"/>
-    <property name="sessionFactory" ref="sessionFactory"/>
+    <constructor-arg ref="dataSource"/>
   </bean>
 

trunk/components/server/resources/ome/services/service-ome.api.RawPixelsStore.xml

@@ -48 +48 @@
   </bean>
 
-  <bean id="managed:ome.api.RawPixelsStore" parent="managedService">
+  <bean id="managed:ome.api.RawPixelsStore" parent="managedStatefulService">
     <property name="proxyInterfaces" value="ome.api.RawPixelsStore"/>
     <property name="target" ref="internal:ome.api.RawPixelsStore"/>

trunk/components/server/resources/ome/services/service-omeis.providers.re.RenderingEngine.xml

@@ -44 +44 @@
   </bean>
 
-  <bean id="managed:omeis.providers.re.RenderingEngine" parent="managedService">
+  <bean id="managed:omeis.providers.re.RenderingEngine" parent="managedStatefulService">
     <property name="proxyInterfaces" value="omeis.providers.re.RenderingEngine"/>
     <property name="target" ref="internal:omeis.providers.re.RenderingEngine"/>

trunk/components/server/resources/ome/services/services.xml

@@ -57 +57 @@
                                 <value>proxyHandler</value>
                                 <value>transactionHandler</value>
+                                <value>hibernateHandler</value>
+                                <value>eventHandler</value>
+                        </list>
+                </property>
+        </bean>
+        <bean id="managedStatefulService" abstract="true"
+                class="org.springframework.aop.framework.ProxyFactoryBean">
+                <property name="interceptorNames">
+                        <list>
+                                <value>serviceHandler</value>
+                                <value>proxyHandler</value>
+                                <value>transactionHandler</value>
                                 <value>sessionHandler</value>
                                 <value>eventHandler</value>
                         </list>
                 </property>
-        </bean>
-  
+        </bean>  
         <bean 
                        id="serviceHandler" 
@@ -72 +83 @@
   Service Levels
   =======================================================================
+  Abstract definitions of services which can be used when configuring
+  concrete beans. Note: these Spring definitions can have a possibly 
+  separate inheritance hierarchy to that defined in Java. When defining
+  a parent bean for a concrete service, be sure that all required properties
+  are (A) filled by the abstract Spring definition and (B) available on the
+  Java class.  
 -->
   <bean id="bean" abstract="true">

trunk/components/server/src/ome/tools/hibernate/SessionHandler.java

@@ -115 +115 @@
                                                         .synchronizedMap(new WeakHashMap<Object, SessionStatus>());
 
-    private HibernateInterceptor       delegate;
-
     private DataSource                 dataSource;
 
@@ -131 +129 @@
     public SessionHandler(DataSource dataSource, SessionFactory factory)
     {
-        if ( dataSource == null | factory == null )
+        if ( dataSource == null || factory == null )
         {
             throw new ApiUsageException(CTOR_MSG);
@@ -137 +135 @@
 
         this.dataSource = dataSource;
-        this.delegate = new HibernateInterceptor();
         this.factory = factory;
-
-        this.delegate.setSessionFactory(factory);
-    }
-    
-    /** constructor taking a {@link DataSource} and a {@link HibernateInterceptor}
-     * as arguments. The needed {@link SessionFactory} will be taken from the
-     * interceptor's {@link HibernateInterceptor#getSessionFactory() getSessionFactory}
-     * method.  
-     * @param dataSource Not null.
-     * @param interceptor Not null.
-     */
-    public SessionHandler(DataSource dataSource, HibernateInterceptor interceptor)
-    {
-        if ( dataSource == null | interceptor == null )
-        {
-            throw new ApiUsageException(CTOR_MSG);
-        }
-        
-        this.dataSource = dataSource;
-        this.delegate = interceptor;
-        this.factory = interceptor.getSessionFactory();
-    }
-
+    }
+    
     /**
      * delegates to {@link HibernateInterceptor} or manages sessions internally,
      * based on the type of service.
      */
-    public Object invoke(MethodInvocation invocation) throws Throwable
+    public Object invoke(final MethodInvocation invocation) throws Throwable
     {
         // Stateless; normal semantics.
@@ -172 +148 @@
                 invocation.getThis().getClass()))
         {
-            debug("Delegating session creation to HibernateInterceptor.");
-            return delegate.invoke(invocation);
-        }
-
+                throw new InternalException( 
+                                "Stateless service configured as stateful." );
+        }
+        
         // Stateful; let's get to work.
         debug("Performing action in stateful session.");
@@ -224 +200 @@
     }
 
-    private void newOrRestoredSession(MethodInvocation invocation)
+    private SessionStatus newOrRestoredSession(MethodInvocation invocation)
             throws HibernateException
     {
+
+        SessionStatus status = sessions.get(invocation.getThis());
+        Session previousSession = ! isSessionBoundToThread() ? null : 
+                sessionBoundToThread();
+
+        // a session is currently running.
+        // something has gone wrong (e.g. with cleanup) abort!
+        if ( previousSession != null )
+        {
+                        String msg = "Dirty Hibernate Session "
+                                + sessionBoundToThread() + " found in Thread "
+                                + Thread.currentThread();
+
+                        sessionBoundToThread().close();
+                        resetThreadSession();
+                        throw new InternalException(msg);
+        } 
+        
+        // we may or may not be in a session, but if we haven't yet bound 
+        // it to This, then we need to.
+        else if (status == null || !status.session.isOpen())
+        {
+            Session currentSession = acquireBindAndConfigureSession();
+            status = new SessionStatus( currentSession );
+            sessions.put(invocation.getThis(), status);
+        } 
+
+        // the session bound to This is already currently being called. abort!
+        else if (status.calls > 1)
+        {
+            throw new InternalException(
+                    "Hibernate session is not re-entrant.\n" +
+                    "Either you have two threads operating on the same " +
+                    "stateful object (don't do this)\n or you have a " +
+                    "recursive call (recurse on the unwrapped object). ");
+        }
         
-        if (isSessionBoundToThread())
-        {
-
-            String msg = "Dirty Hibernate Session "
-                    + sessionBoundToThread() + " found in Thread "
-                    + Thread.currentThread();
-
-            sessionBoundToThread().close();
-            resetThreadSession();
-            throw new InternalException(msg);
-        }
-        
-        SessionStatus status = sessions.get(invocation.getThis());
-
-        if (status == null || !status.session.isOpen())
-        {
-            
-            debug("Replacing null or closed session.");
-            status = new SessionStatus(acquireBindAndConfigureSession());
-            sessions.put(invocation.getThis(), status);
-        } else
-        {
-            if (status.calls > 1)
-                throw new InternalException(
-                        "Hibernate session is not re-entrant.\n" +
-                        "Either you have two threads operating on the same " +
-                        "stateful object (don't do this)\n or you have a " +
-                        "recursive call (recurse on the unwrapped object). ");
-
+        // all is fine.
+        else {
             debug("Binding and reconnecting session.");
             bindSession(status.session);
@@ -264 +249 @@
         // It's ready to be used. Increment.
         status.calls++;
-
-    }
-
+        return status;
+
+    }
+
+    // ~ SESSIONS
+    // =========================================================================
+
+    private boolean isCloseSession(MethodInvocation invocation)
+    {
+        return "destroy".equals(invocation.getMethod().getName());
+    }
+
+    private Session acquireBindAndConfigureSession() throws HibernateException
+    {
+        debug("Opening and binding session.");
+        Session session = factory.openSession();
+        session.setFlushMode( FlushMode.COMMIT );
+        bindSession(session);
+        return session;
+    }
+
+    private void bindSession(Session session) 
+    {
+        debug("Binding session to thread.");
+        SessionHolder sessionHolder = new SessionHolder(session);
+        sessionHolder.setTransaction(sessionHolder.getSession()
+                .beginTransaction());
+        TransactionSynchronizationManager.bindResource(factory, sessionHolder);
+        if ( ! TransactionSynchronizationManager.isSynchronizationActive())
+                throw new InternalException( "Synchronization not active for " +
+                                "TransactionSynchronizationManager");
+    }
+    
+    private Session sessionBoundToThread()
+    {
+        return SessionFactoryUtils.getSession(factory, false);
+    }
+
+    private boolean isSessionBoundToThread()
+    {
+        return TransactionSynchronizationManager.hasResource(factory)
+                && sessionBoundToThread() != null;
+    }
+
+    private void resetThreadSession()
+    {
+        if (isSessionBoundToThread())
+        {
+            debug("Session bound to thread. Reseting.");
+            TransactionSynchronizationManager.unbindResource(factory);
+        } else {
+            debug("Session not bound to thread. No need to reset.");
+        }
+    }
+
+    private void reconnectSession(Session session) throws HibernateException
+    {
+        if (!session.isConnected())
+        {
+            debug("Session not connected. Connecting.");
+            Connection connection = DataSourceUtils.getConnection(dataSource);
+            session.reconnect(connection);
+        } else {
+            debug("Session already connected. Not reconnecting.");
+        }
+    }
+
+    private void disconnectSession() throws HibernateException
+    {
+        if (isSessionBoundToThread() && sessionBoundToThread().isConnected())
+        {
+            debug("Session bound to thread. Disconnecting.");
+            sessionBoundToThread().disconnect();
+        } else {
+            debug("No session bound to thread. Can't disconnect.");
+        }
+        
+    }
+    
     private void closeSession() throws Exception
     {
@@ -278 +339 @@
                 session.connection().commit();
                 sessionBoundToThread().close();
-            } catch (Exception e)
-            {
-                throw e;
-            } finally
+            } 
+            
+            finally
             {
                 resetThreadSession();
@@ -291 +351 @@
 
     }
-
-    // ~ SESSIONS
-    // =========================================================================
-
-    private boolean isCloseSession(MethodInvocation invocation)
-    {
-        return "destroy".equals(invocation.getMethod().getName());
-    }
-
-    private Session acquireBindAndConfigureSession() throws HibernateException
-    {
-        debug("Opening and binding session.");
-        Session session = factory.openSession();
-        session.setFlushMode( FlushMode.COMMIT );
-        bindSession(session);
-        return session;
-    }
-
-    private void bindSession(Session session) 
-    {
-        debug("Binding session to thread.");
-        SessionHolder sessionHolder = new SessionHolder(session);
-        sessionHolder.setTransaction(sessionHolder.getSession()
-                .beginTransaction());
-        TransactionSynchronizationManager.bindResource(factory, sessionHolder);
-        TransactionSynchronizationManager.initSynchronization();
-    }
-    
-    private Session sessionBoundToThread()
-    {
-        return SessionFactoryUtils.getSession(factory, false);
-    }
-
-    private boolean isSessionBoundToThread()
-    {
-        return TransactionSynchronizationManager.hasResource(factory)
-                && sessionBoundToThread() != null;
-    }
-
-    private void resetThreadSession()
-    {
-        if (isSessionBoundToThread())
-        {
-            debug("Session bound to thread. Reseting.");
-            TransactionSynchronizationManager.unbindResource(factory);
-            TransactionSynchronizationManager.clearSynchronization();
-        } else {
-            debug("Session not bound to thread. No need to reset.");
-        }
-    }
-
-    private void reconnectSession(Session session) throws HibernateException
-    {
-        if (!session.isConnected())
-        {
-            debug("Session not connected. Connecting.");
-            Connection connection = DataSourceUtils.getConnection(dataSource);
-            session.reconnect(connection);
-        } else {
-            debug("Session already connected. Not reconnecting.");
-        }
-    }
-
-    private void disconnectSession() throws HibernateException
-    {
-        if (isSessionBoundToThread()
-                && SessionFactoryUtils.getSession(factory, false).isConnected())
-        {
-            debug("Session bound to thread. Disconnecting.");
-            sessionBoundToThread().disconnect();
-        } else {
-            debug("No session bound to thread. Can't disconnect.");
-        }
-        
-    }
     
     private void debug(String message)

trunk/components/server/test/ome/server/utests/handlers/SessionHandlerMockHibernateTest.java

@@ -47 +47 @@
 import org.jmock.Mock;
 import org.jmock.MockObjectTestCase;
-import org.jmock.builder.MatchBuilder;
+import org.jmock.builder.ArgumentsMatchBuilder;
 import org.jmock.core.Invocation;
 import org.jmock.core.InvocationMatcher;
 import org.jmock.core.Stub;
 import org.jmock.core.stub.DefaultResultStub;
-import org.springframework.orm.hibernate3.HibernateInterceptor;
 import org.springframework.orm.hibernate3.SessionHolder;
 import org.springframework.transaction.support.TransactionSynchronizationManager;
@@ -70 +69 @@
  * @since Omero 2.0
  */
-@Test( groups = {"ignore","sessions","hibernate","priority"} )
+@Test( groups = {"hibernate","stateful","ticket:326"} )
 public class SessionHandlerMockHibernateTest extends MockObjectTestCase
 {
@@ -108 +107 @@
         // Things should always be cleaned up by handler/interceptor
         assertFalse( TransactionSynchronizationManager.hasResource(factory) );
+        if (!TransactionSynchronizationManager.isSynchronizationActive())
+                TransactionSynchronizationManager.initSynchronization();
     }
 
@@ -113 +114 @@
     protected void tearDown() throws Exception
     {
+        session = null;
+        mockStateful.reset();
+        mockStateless.reset();
+        mockSession.reset();
+        mockFactory.reset();
+        mockTransaction.reset();
+        mockDataSource.reset();
+        mockConnection.reset();
+        mockInvocation.reset();
         super.tearDown();
+        if (TransactionSynchronizationManager.isSynchronizationActive())
+                TransactionSynchronizationManager.clearSynchronization();
     }
 
@@ -120 +132 @@
 
     @Test
+    @ExpectedExceptions( InternalException.class )
     public void testStatelessInvocation() throws Throwable
     {
         newStatelessInvocation();
-        HibernateInterceptor interceptor = new HibernateInterceptor() {
-            @Override
-            public Object invoke(MethodInvocation methodInvocation) throws Throwable
-            {
-                return null;
-            }
-        };
-        interceptor.setSessionFactory( factory );
-        // for testing stateless, we need control over the interceptor.
-        handler = new SessionHandler( dataSource, interceptor );
+        handler = new SessionHandler( dataSource, factory );
         handler.invoke( invocation );
         super.verify();
@@ -138 +142 @@
 
     @Test
-    public void testStatefulInvocation() throws Throwable
+    public void testStatefulInvocationGetsNewSession() throws Throwable
     {
         newStatefulReadInvocation();
         opensSession();
+        setsFlushMode();
         beginsTransaction(1);
         checksSessionIsOpen();
         getsFactoryFromSession();
         getsAutoFlushMode();
+        // invocation here
         checksSessionIsConnected();
         disconnectsSession();
@@ -153 +159 @@
 
     @Test
-    public void testTwoStatefulInvocations() throws Throwable
+    public void testSecondStatefulInvocationsReusesSession() throws Throwable
     {
         newStatefulReadInvocation();
         opensSession();
+        setsFlushMode();
         beginsTransaction(2);
         checksSessionIsOpen();
         getsFactoryFromSession();
         getsAutoFlushMode();
-        checksSessionIsConnected();
-        disconnectsSession();
-        handler.invoke( invocation );
+        // invocation here
+        checksSessionIsConnected();
+        disconnectsSession();
+        handler.invoke( invocation );
+        
         // And a second call should just work.
         newStatefulReadInvocation();
+        // invocation here
+        checksSessionIsConnected();
+        disconnectsSession();
         handler.invoke( invocation );
         super.verify();
@@ -171 +183 @@
     
     @Test
-    @ExpectedExceptions(InternalException.class)
+    @ExpectedExceptions( InternalException.class )
     public void testStatefulInvocationWithExistingSession() throws Throwable
     {
-        // setup Session
-        prepareThread();
+        prepareThreadWithSession();
         
         newStatefulReadInvocation();
         checksSessionIsOpen();
-        //checksSessionIsConnected();
-        getsFactoryFromSession();
+        checksSessionIsConnected();
+        getsFactoryFromSession();
+        getsAutoFlushMode();
         disconnectsSession();
         closesSession();
@@ -188 +200 @@
     
     @Test
+    public void testClosedOnException() throws Throwable
+    {
+        prepareThreadWithSession();
+
+        try {
+        newStatefulReadInvocationThrows();
+        checksSessionIsOpen();
+        checksSessionIsConnected();
+        getsFactoryFromSession();
+        getsAutoFlushMode();
+        // here it throws
+        disconnectsSession();
+        closesSession();
+        handler.invoke( invocation );
+        fail("Should have thrown.");
+        } catch (Exception e)
+        {}
+    }
+    
+    @Test
     public void testStatefulInvocationWithSessionThenClosed() throws Throwable
     {
@@ -194 +226 @@
         getsFactoryFromSession();
         getsAutoFlushMode();
+        setsFlushMode();
         opensSession();
+//        setsFlushMode(); TODO huh?
         beginsTransaction(1);
         getsSessionsConnection();
@@ -204 +238 @@
     
     @Test
-    public void testSyncResetEvenOnException() throws Throwable
-    {
-        // setup Session
-        prepareThread();
-
-        try {
-        newStatefulReadInvocation();
-        checksSessionIsOpen();
-        //checksSessionIsConnected();
-        getsFactoryFromSession();
+    @ExpectedExceptions( InternalException.class )
+    public void testStatefulReentrantCallThrows() throws Throwable
+    {
+        Method method = RenderingEngine.class.getMethod("getDefaultZ");
+        newStatefulInvocation( method, new Stub() {
+                public Object invoke(Invocation dummy) throws Throwable {
+                        handler.invoke( invocation );
+                        return null;
+                }
+                public StringBuffer describeTo(StringBuffer buffer) {
+                        return buffer.append(" reentrant call ");
+                }
+        });
+        opensSession();
+        setsFlushMode();
+        beginsTransaction(2);
+        checksSessionIsOpen();
+        getsFactoryFromSession();
+        getsAutoFlushMode();
+        // invocation here
+        checksSessionIsConnected();