QPID-3332: Separate open/idle transaction timeout tests to remove the possibility of an unexpected log message. Remove TransactionTimeoutConfigTest as this only duplicated tests aleady performed by TransactionTimeoutTest.

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1174842 13f79535-47bb-0310-9956-ffa450edef68

5 files changed, 199 insertions, 313 deletions

diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSession.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSession.java
index 028f6d85be..67ddd6ca77 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSession.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSession.java
@@ -645,7 +645,7 @@ public class ServerSession extends Session implements AuthorizationHolder, Sessi
             // Log a warning on idle or open transactions
             if (idleWarn > 0L && idleTime > idleWarn)
             {
-                CurrentActor.get().message(getLogSubject(), ChannelMessages.IDLE_TXN(openTime));
+                CurrentActor.get().message(getLogSubject(), ChannelMessages.IDLE_TXN(idleTime));
                 _logger.warn("IDLE TRANSACTION ALERT " + getLogSubject().toString() + " " + idleTime + " ms");
             }
             else if (openWarn > 0L && openTime > openWarn)
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutConfigurationTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutConfigurationTest.java
deleted file mode 100644
index 46e5d214f5..0000000000
--- a/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutConfigurationTest.java
+++ /dev/null
@@ -1,82 +0,0 @@
-/*
- *
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- *
- */
-package org.apache.qpid.test.unit.transacted;
-
-/**
- * This verifies that changing the {@code transactionTimeout} configuration will alter
- * the behaviour of the transaction open and idle logging, and that when the connection
- * will be closed.
- */
-public class TransactionTimeoutConfigurationTest extends TransactionTimeoutTestCase
-{
-    @Override
-    protected void configure() throws Exception
-    {
-        // Setup housekeeping every second
-        setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".housekeeping.checkPeriod", "100");
-        
-        // Set transaction timout properties.
-        setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".transactionTimeout.openWarn", "200");
-        setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".transactionTimeout.openClose", "1000");
-        setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".transactionTimeout.idleWarn", "100");
-        setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".transactionTimeout.idleClose", "500");
-    }
-
-    public void testProducerIdleCommit() throws Exception
-    {
-        try
-        {
-            send(5, 0);
-            
-            sleep(2.0f);
-
-            _psession.commit();
-            fail("should fail");
-        }
-        catch (Exception e)
-        {
-            _exception = e;
-        }
-        
-        monitor(5, 0);
-        
-        check(IDLE);
-    }
-
-    public void testProducerOpenCommit() throws Exception
-    {
-        try
-        {
-            send(5, 0.3f);
-
-            _psession.commit();
-            fail("should fail");
-        }
-        catch (Exception e)
-        {
-            _exception = e;
-        }
-        
-        monitor(6, 3);
-        
-        check(OPEN);
-    }
-}
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutDisabledTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutDisabledTest.java
index db508143f9..fd8beffbe6 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutDisabledTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutDisabledTest.java
@@ -30,6 +30,8 @@ public class TransactionTimeoutDisabledTest extends TransactionTimeoutTestCase
     {
         // Setup housekeeping every second
         setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".housekeeping.checkPeriod", "100");
+
+        // No transaction timeout configuration.
     }
 
     public void testProducerIdleCommit() throws Exception
@@ -47,7 +49,7 @@ public class TransactionTimeoutDisabledTest extends TransactionTimeoutTestCase
             fail("Should have succeeded");
         }
         
-        assertTrue("Listener should not have received exception", _caught.getCount() == 1);
+        assertEquals("Listener should not have received exception", 0, getNumberOfDeliveredExceptions());
         
         monitor(0, 0);
     }
@@ -65,7 +67,7 @@ public class TransactionTimeoutDisabledTest extends TransactionTimeoutTestCase
             fail("Should have succeeded");
         }
         
-        assertTrue("Listener should not have received exception", _caught.getCount() == 1);
+        assertEquals("Listener should not have received exception", 0, getNumberOfDeliveredExceptions());
         
         monitor(0, 0);
     }
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutTest.java
index c912d6a323..f554b0089e 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutTest.java
@@ -30,152 +30,125 @@ package org.apache.qpid.test.unit.transacted;
  */
 public class TransactionTimeoutTest extends TransactionTimeoutTestCase
 {
-    public void testProducerIdle() throws Exception
+
+    protected void configure() throws Exception
     {
-        try
+        // Setup housekeeping every second
+        setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".housekeeping.checkPeriod", "100");
+
+        if (getName().contains("ProducerIdle"))
         {
-            sleep(2.0f);
-    
-            _psession.commit();
+            setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".transactionTimeout.openWarn", "0");
+            setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".transactionTimeout.openClose", "0");
+            setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".transactionTimeout.idleWarn", "500");
+            setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".transactionTimeout.idleClose", "1500");
         }
-        catch (Exception e)
+        else if (getName().contains("ProducerOpen"))
         {
-            fail("Should have succeeded");
+            setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".transactionTimeout.openWarn", "1000");
+            setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".transactionTimeout.openClose", "2000");
+            setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".transactionTimeout.idleWarn", "0");
+            setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".transactionTimeout.idleClose", "0");
         }
-        
-        assertTrue("Listener should not have received exception", _caught.getCount() == 1);
-        
-        monitor(0, 0);
-    }
-    
-    public void testProducerIdleCommit() throws Exception
-    {
-        try
+        else
         {
-            send(5, 0);
-            
-            sleep(2.0f);
-    
-            _psession.commit();
-            fail("should fail");
+            setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".transactionTimeout.openWarn", "1000");
+            setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".transactionTimeout.openClose", "2000");
+            setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".transactionTimeout.idleWarn", "500");
+            setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".transactionTimeout.idleClose", "1000");
         }
-        catch (Exception e)
-        {
-            _exception = e;
-        }
-        
-        monitor(5, 0);
-        
-        check(IDLE);
     }
-    
-    public void testProducerOpenCommit() throws Exception
+
+    public void testProducerIdle() throws Exception
     {
-        try
-        {
-            send(6, 0.5f);
-    
-            _psession.commit();
-            fail("should fail");
-        }
-        catch (Exception e)
-        {
-            _exception = e;
-        }
-        
-        monitor(0, 10);
-        
-        check(OPEN);
+        sleep(2.0f);
+
+        _psession.commit();
+
+        assertEquals("Listener should not have received exception", 0, getNumberOfDeliveredExceptions());
+
+        monitor(0, 0);
     }
-    
-    public void testProducerIdleCommitTwice() throws Exception
+
+    public void testProducerIdleCommit() throws Exception
     {
+        send(5, 0);
+        // Idle for more than idleClose to generate idle-warns and cause a close.
+        sleep(2.0f);
+
         try
         {
-            send(5, 0);
-            
-            sleep(1.0f);
-            
             _psession.commit();
-            
-            send(5, 0);
-            
-            sleep(2.0f);
-    
-            _psession.commit();
-            fail("should fail");
+            fail("Exception not thrown");
         }
         catch (Exception e)
         {
             _exception = e;
         }
-        
+
         monitor(10, 0);
-        
+
         check(IDLE);
     }
-    
-    public void testProducerOpenCommitTwice() throws Exception
+
+    public void testProducerIdleCommitTwice() throws Exception
     {
+        send(5, 0);
+        // Idle for less than idleClose to generate idle-warns
+        sleep(1.0f);
+
+        _psession.commit();
+
+        send(5, 0);
+        // Now idle for more than idleClose to generate more idle-warns and cause a close.
+        sleep(2.0f);
+
         try
         {
-            send(5, 0);
-            
-            sleep(1.0f);
-            
             _psession.commit();
-            
-            send(6, 0.5f);
-    
-            _psession.commit();
-            fail("should fail");
+            fail("Exception not thrown");
         }
         catch (Exception e)
         {
             _exception = e;
         }
-        
-        // the presistent store generates more idle messages?
-        monitor(isBrokerStorePersistent() ? 10 : 5, 10);
-        
-        check(OPEN);
+
+        monitor(15, 0);
+
+        check(IDLE);
     }
-    
+
     public void testProducerIdleRollback() throws Exception
     {
+        send(5, 0);
+        // Now idle for more than idleClose to generate more idle-warns and cause a close.
+        sleep(2.0f);
         try
         {
-            send(5, 0);
-            
-            sleep(2.0f);
-    
             _psession.rollback();
-            fail("should fail");
+            fail("Exception not thrown");
         }
         catch (Exception e)
         {
             _exception = e;
         }
-        
-        monitor(5, 0);
-        
+
+        monitor(10, 0);
+
         check(IDLE);
     }
-    
+
     public void testProducerIdleRollbackTwice() throws Exception
     {
+        send(5, 0);
+        // Idle for less than idleClose to generate idle-warns
+        sleep(1.0f);
+        _psession.rollback();
+        send(5, 0);
+        // Now idle for more than idleClose to generate more idle-warns and cause a close.
+        sleep(2.0f);
         try
         {
-            send(5, 0);
-            
-            sleep(1.0f);
-            
-            _psession.rollback();
-            
-            send(5, 0);
-            
-            sleep(2.0f);
-    
             _psession.rollback();
             fail("should fail");
         }
@@ -183,153 +156,153 @@ public class TransactionTimeoutTest extends TransactionTimeoutTestCase
         {
             _exception = e;
         }
-        
-        monitor(10, 0);
+
+        monitor(15, 0);
         
         check(IDLE);
     }
-    
-    public void testConsumerCommitClose() throws Exception
+
+    public void testProducerOpenCommit() throws Exception
     {
         try
         {
-            send(1, 0);
-    
+            // Sleep between sends to cause open warns and then cause a close.
+            send(6, 0.5f);
             _psession.commit();
-    
-            expect(1, 0);
-            
-            _csession.commit();
-            
-            sleep(3.0f);
-    
-            _csession.close();
+            fail("Exception not thrown");
         }
         catch (Exception e)
         {
-            fail("should have succeeded: " + e.getMessage());
+            _exception = e;
         }
-        
-        assertTrue("Listener should not have received exception", _caught.getCount() == 1);
-        
-        monitor(0, 0);
+
+        monitor(0, 10);
+
+        check(OPEN);
     }
     
-    public void testConsumerIdleReceiveCommit() throws Exception
+    public void testProducerOpenCommitTwice() throws Exception
     {
+        send(5, 0);
+        sleep(1.0f);
+        _psession.commit();
+
         try
         {
-            send(1, 0);
-    
+            // Now sleep between sends to cause open warns and then cause a close.
+            send(6, 0.5f);
             _psession.commit();
-    
-            sleep(2.0f);
-            
-            expect(1, 0);
-            
-            sleep(2.0f);
-    
-            _csession.commit();
+            fail("Exception not thrown");
         }
         catch (Exception e)
         {
-            fail("Should have succeeded");
+            _exception = e;
         }
+
+        monitor(0, 10);
         
-        assertTrue("Listener should not have received exception", _caught.getCount() == 1);
-        
+        check(OPEN);
+    }
+
+    public void testConsumerCommitClose() throws Exception
+    {
+        send(1, 0);
+
+        _psession.commit();
+
+        expect(1, 0);
+
+        _csession.commit();
+
+        sleep(3.0f);
+
+        _csession.close();
+
+        assertEquals("Listener should not have received exception", 0, getNumberOfDeliveredExceptions());
+
         monitor(0, 0);
     }
     
+    public void testConsumerIdleReceiveCommit() throws Exception
+    {
+        send(1, 0);
+
+        _psession.commit();
+
+        sleep(2.0f);
+
+        expect(1, 0);
+
+        sleep(2.0f);
+
+        _csession.commit();
+
+        assertEquals("Listener should not have received exception", 0, getNumberOfDeliveredExceptions());
+
+        monitor(0, 0);
+    }
+
     public void testConsumerIdleCommit() throws Exception
     {
-        try
-        {
-            send(1, 0);
-    
-            _psession.commit();
-    
-            expect(1, 0);
-            
-            sleep(2.0f);
-    
-            _csession.commit();
-        }
-        catch (Exception e)
-        {
-            fail("Should have succeeded");
-        }
-        
-        assertTrue("Listener should not have received exception", _caught.getCount() == 1);
-        
+        send(1, 0);
+
+        _psession.commit();
+
+        expect(1, 0);
+
+        sleep(2.0f);
+
+        _csession.commit();
+
+        assertEquals("Listener should not have received exception", 0, getNumberOfDeliveredExceptions());
+
         monitor(0, 0);
     }
     
     public void testConsumerIdleRollback() throws Exception
     {
-        try
-        {
-            send(1, 0);
-    
-            _psession.commit();
-            
-            expect(1, 0);
-            
-            sleep(2.0f);
-    
-            _csession.rollback();
-        }
-        catch (Exception e)
-        {
-            fail("Should have succeeded");
-        }
-        
-        assertTrue("Listener should not have received exception", _caught.getCount() == 1);
-        
+        send(1, 0);
+
+        _psession.commit();
+
+        expect(1, 0);
+
+        sleep(2.0f);
+
+        _csession.rollback();
+
+        assertEquals("Listener should not have received exception", 0, getNumberOfDeliveredExceptions());
+
         monitor(0, 0);
     }
-    
+
     public void testConsumerOpenCommit() throws Exception
     {
-        try
-        {
-            send(1, 0);
-    
-            _psession.commit();
-            
-            sleep(3.0f);
-    
-            _csession.commit();
-        }
-        catch (Exception e)
-        {
-            fail("Should have succeeded");
-        }
-        
-        assertTrue("Listener should not have received exception", _caught.getCount() == 1);
-        
+        send(1, 0);
+
+        _psession.commit();
+
+        sleep(3.0f);
+
+        _csession.commit();
+
+        assertEquals("Listener should not have received exception", 0, getNumberOfDeliveredExceptions());
+
         monitor(0, 0);
     }
     
     public void testConsumerOpenRollback() throws Exception
     {
-        try
-        {
-            send(1, 0);
-    
-            _psession.commit();
-    
-            sleep(3.0f);
-    
-            _csession.rollback();
-        }
-        catch (Exception e)
-        {
-            fail("Should have succeeded");
-        }
-        
-        assertTrue("Listener should not have received exception", _caught.getCount() == 1);
+        send(1, 0);
         
+        _psession.commit();
+
+        sleep(3.0f);
+
+        _csession.rollback();
+
+        assertEquals("Listener should not have received exception", 0, getNumberOfDeliveredExceptions());
+
         monitor(0, 0);
     }
 }
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutTestCase.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutTestCase.java
index ef2de5c592..2b90d38049 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutTestCase.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/transacted/TransactionTimeoutTestCase.java
@@ -23,6 +23,7 @@ package org.apache.qpid.test.unit.transacted;
 import java.util.List;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import javax.jms.DeliveryMode;
 import javax.jms.ExceptionListener;
@@ -49,7 +50,7 @@ import org.apache.qpid.util.LogMonitor;
 /**
  * The {@link TestCase} for transaction timeout testing.
  */
-public class TransactionTimeoutTestCase extends QpidBrokerTestCase implements ExceptionListener
+public abstract class TransactionTimeoutTestCase extends QpidBrokerTestCase implements ExceptionListener
 {
     public static final String VIRTUALHOST = "test";
     public static final String TEXT = "0123456789abcdefghiforgettherest";
@@ -64,31 +65,16 @@ public class TransactionTimeoutTestCase extends QpidBrokerTestCase implements Ex
     protected Queue _queue;
     protected MessageConsumer _consumer;
     protected MessageProducer _producer;
-    protected CountDownLatch _caught = new CountDownLatch(1);
+    private CountDownLatch _exceptionLatch = new CountDownLatch(1);
+    protected AtomicInteger _exceptionCount = new AtomicInteger(0);
     protected String _message;
     protected Exception _exception;
     protected AMQConstant _code;
-    
-    protected void configure() throws Exception
-    {
-        // Setup housekeeping every second
-        setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".housekeeping.checkPeriod", "100");
-        
-        /*
-         * Set transaction timout properties. The XML in the virtualhosts configuration is as follows:
-         * 
-         *  <transactionTimeout>
-         *      <openWarn>1000</openWarn>
-         *      <openClose>2000</openClose>
-         *      <idleWarn>500</idleWarn>
-         *      <idleClose>1500</idleClose>
-         *  </transactionTimeout>
-         */
-        setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".transactionTimeout.openWarn", "1000");
-        setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".transactionTimeout.openClose", "2000");
-        setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".transactionTimeout.idleWarn", "500");
-        setConfigurationProperty("virtualhosts.virtualhost." + VIRTUALHOST + ".transactionTimeout.idleClose", "1000");
-    }
+
+    /**
+     * Subclasses must implement this to configure transaction timeout parameters.
+     */
+    protected abstract void configure() throws Exception;
         
     protected void setUp() throws Exception
     {
@@ -233,7 +219,7 @@ public class TransactionTimeoutTestCase extends QpidBrokerTestCase implements Ex
      */
     protected void check(String reason)throws InterruptedException
     {
-        assertTrue("Should have caught exception in listener", _caught.await(1, TimeUnit.SECONDS));
+        assertTrue("Should have caught exception in listener", _exceptionLatch.await(1, TimeUnit.SECONDS));
         assertNotNull("Should have thrown exception to client", _exception);
         assertTrue("Exception message should contain '" + reason + "': " + _message, _message.contains(reason + " transaction timed out"));
         assertNotNull("Exception should have an error code", _code);
@@ -243,11 +229,18 @@ public class TransactionTimeoutTestCase extends QpidBrokerTestCase implements Ex
     /** @see javax.jms.ExceptionListener#onException(javax.jms.JMSException) */
     public void onException(JMSException jmse)
     {
-        _caught.countDown();
+        _exceptionLatch.countDown();
+        _exceptionCount.incrementAndGet();
+
         _message = jmse.getLinkedException().getMessage();
         if (jmse.getLinkedException() instanceof AMQException)
         {
             _code = ((AMQException) jmse.getLinkedException()).getErrorCode();
         }
     }
+
+    protected int getNumberOfDeliveredExceptions()
+    {
+        return _exceptionCount.get();
+    }
 }