QPID-2857 : Address issues found by running FindBugs against the Java codebase

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

14 files changed, 122 insertions, 109 deletions

diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/client/DispatcherTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/client/DispatcherTest.java
index 98517d0687..a8a23c2c41 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/client/DispatcherTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/client/DispatcherTest.java
@@ -77,10 +77,6 @@ public class DispatcherTest extends QpidBrokerTestCase
     {
         super.setUp();
 
-        InitialContextFactory factory = new PropertiesFileInitialContextFactory();
-
-        Hashtable<String, String> env = new Hashtable<String, String>();
-
         // Create Client 1
         _clientConnection = getConnection();
 
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/server/logging/AbstractTestLogging.java b/qpid/java/systests/src/main/java/org/apache/qpid/server/logging/AbstractTestLogging.java
index f66ee38139..7b59266eb3 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/server/logging/AbstractTestLogging.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/server/logging/AbstractTestLogging.java
@@ -71,17 +71,17 @@ public class AbstractTestLogging extends QpidBrokerTestCase
                 @Override
                 protected void createBroker() throws Exception
                 {
-                    _started = true;
+                    setStarted(true);
                     CurrentActor.set(new TestLogActor(new SystemOutMessageLogger()));
 
                     // Prevent the InVM broker from logging and spoiling tests.
                     _serverConfiguration.getConfig().setProperty(ServerConfiguration.STATUS_UPDATES, "off");
 
-                    _configuration = _serverConfiguration;
-                    _registry = new TestApplicationRegistry(_configuration)
+                    setConfiguration(_serverConfiguration);
+                    setRegistry(new TestApplicationRegistry(getConfiguration())
                     {
                         /**
-                         * Create a virtualhost with a {@link SkeletonMessageStore} instead
+                         * Create a virtualhost with a {@link org.apache.qpid.server.store.SkeletonMessageStore} instead
                          * of the configured one, but remember the original configuration.
                          */
                         @Override
@@ -93,8 +93,8 @@ public class AbstractTestLogging extends QpidBrokerTestCase
                             vhostConfig.setMessageStoreClass(oldClass);
                             return host;
                         }
-                    };
-                    ApplicationRegistry.initialise(_registry);
+                    });
+                    ApplicationRegistry.initialise(getRegistry());
 
                 }
 
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/server/logging/AccessControlLoggingTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/server/logging/AccessControlLoggingTest.java
index b2e35ea8d7..2629e82831 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/server/logging/AccessControlLoggingTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/server/logging/AccessControlLoggingTest.java
@@ -55,7 +55,7 @@ public class AccessControlLoggingTest extends AbstractTestLogging
         super.setUp();
     }
 
-    /** FIXME This comes from {@link SimpleACLTest} and makes me suspicious. */
+    /** FIXME This comes from SimpleACLTest and makes me suspicious. */
     @Override
     public void tearDown() throws Exception
     {
@@ -135,7 +135,7 @@ public class AccessControlLoggingTest extends AbstractTestLogging
         
         assertEquals("Should only be one ACL log message", 1, matches.size());
         
-        String log = getLogMessage(matches, 0);;
+        String log = getLogMessage(matches, 0);
         String actor = fromActor(log);
         String subject = fromSubject(log);
         String message = getMessageString(fromMessage(log));
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/server/security/acl/SimpleACLTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/server/security/acl/SimpleACLTest.java
index 28125f2f19..a50817e659 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/server/security/acl/SimpleACLTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/server/security/acl/SimpleACLTest.java
@@ -46,7 +46,7 @@ import org.apache.qpid.url.URLSyntaxException;
  * 
  * These tests require an access control security plugin to be configured in the broker, and carry out various broker
  * operations that will succeed or fail depending on the user and virtual host. See the {@code config-systests-acl-setup.xml}
- * configuration file for the {@link SimpleXML} version of the ACLs used by the Java broker only, or the various {@code .txt}
+ * configuration file for the SimpleXML version of the ACLs used by the Java broker only, or the various {@code .txt}
  * files in the system tests directory for the external version 3 ACL files used by both the Java and C++ brokers.
  * <p>
  * This class can be extended and the {@link #getConfig()} method overridden to run the same tests with a different type
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/client/QueueBrowserAutoAckTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/client/QueueBrowserAutoAckTest.java
index f9cf48a2b1..f54b022c09 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/test/client/QueueBrowserAutoAckTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/test/client/QueueBrowserAutoAckTest.java
@@ -265,8 +265,8 @@ public class QueueBrowserAutoAckTest extends FailoverBaseCase
     protected void checkOverlappingMultipleGetEnum(int expectedMessages, int browserEnumerationCount, String selector) throws JMSException
     {
         QueueBrowser queueBrowser = selector == null ?
-                                    _clientSession.createBrowser(_queue, selector) :
-                                    _clientSession.createBrowser(_queue);
+                                _clientSession.createBrowser(_queue) : _clientSession.createBrowser(_queue);
+//                _clientSession.createBrowser(_queue) : _clientSession.createBrowser(_queue, selector);
 
         Enumeration[] msgs = new Enumeration[browserEnumerationCount];
         int[] msgCount = new int[browserEnumerationCount];
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/client/failover/FailoverTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/client/failover/FailoverTest.java
index 2fbac83eb4..fcbab273e5 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/test/client/failover/FailoverTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/test/client/failover/FailoverTest.java
@@ -306,7 +306,7 @@ public class FailoverTest extends FailoverBaseCase implements ConnectionListener
         //Start the connection
         connection.start();
 
-        long FAILOVER_DELAY = (RETRIES * DELAY);
+        long FAILOVER_DELAY = ((long)RETRIES * (long)DELAY);
 
         // Use Nano seconds as it is more accurate for comparision.
         long failTime = System.nanoTime() + FAILOVER_DELAY * 1000000;
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/framework/FrameworkBaseCase.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/framework/FrameworkBaseCase.java
index ddffc4cabf..f866cd572f 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/test/framework/FrameworkBaseCase.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/test/framework/FrameworkBaseCase.java
@@ -53,20 +53,15 @@ public class FrameworkBaseCase extends QpidBrokerTestCase implements FrameworkTe
     /** Used for debugging purposes. */
     private static final Logger log = Logger.getLogger(FrameworkBaseCase.class);
 
-    /** Holds the test sequencer to create and run test circuits with. */
-    protected CircuitFactory circuitFactory = new LocalAMQPCircuitFactory();
+    private CircuitFactory circuitFactory = new LocalAMQPCircuitFactory();
 
-    /** Used to read the tests configurable properties through. */
-    protected ParsedProperties testProps;
+    private ParsedProperties testProps;
 
-    /** A default setup task processor to delegate setup tasks to. */
-    protected SetupTaskHandler taskHandler = new SetupTaskHandler();
+    private SetupTaskHandler taskHandler = new SetupTaskHandler();
 
-    /** Flag used to track whether the test is in-vm or not. */
-    protected boolean isUsingInVM;
+    private boolean isUsingInVM;
 
-    /** Holds the failure mechanism. */
-    protected CauseFailure failureMechanism = new CauseFailureUserPrompt();
+    private CauseFailure failureMechanism = new CauseFailureUserPrompt();
 
     /**
      * Creates a new test case with the specified name.
@@ -78,7 +73,7 @@ public class FrameworkBaseCase extends QpidBrokerTestCase implements FrameworkTe
         super(name);
     }
 
-    /**
+    /** Holds the test sequencer to create and run test circuits with. */ /**
      * Returns the test case sequencer that provides test circuit, and test sequence implementations. The sequencer
      * that this base case returns by default is suitable for running a test circuit with both circuit ends colocated
      * on the same JVM.
@@ -283,4 +278,24 @@ public class FrameworkBaseCase extends QpidBrokerTestCase implements FrameworkTe
     {
         this.failureMechanism = failureMechanism;
     }
+
+    protected ParsedProperties getTestProps()
+    {
+        return testProps;
+    }
+
+    protected void setTestProps(ParsedProperties testProps)
+    {
+        this.testProps = testProps;
+    }
+
+    protected SetupTaskHandler getTaskHandler()
+    {
+        return taskHandler;
+    }
+
+    protected CauseFailure getFailureMechanism()
+    {
+        return failureMechanism;
+    }
 }
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/framework/LocalAMQPCircuitFactory.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/framework/LocalAMQPCircuitFactory.java
index 7fbef06265..4c8f301d1c 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/test/framework/LocalAMQPCircuitFactory.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/test/framework/LocalAMQPCircuitFactory.java
@@ -75,7 +75,7 @@ public class LocalAMQPCircuitFactory extends LocalCircuitFactory
 
         MessageProducer producer =
             props.getPublisherProducerBind()
-            ? ((props.getImmediate() | props.getMandatory())
+            ? ((props.getImmediate() || props.getMandatory())
                 ? ((AMQSession) session).createProducer(destination, props.getMandatory(), props.getImmediate())
                 : session.createProducer(destination)) : null;
 
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/testcases/FailoverTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/testcases/FailoverTest.java
index a0a8894874..a5a0d4e41f 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/test/testcases/FailoverTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/test/testcases/FailoverTest.java
@@ -76,18 +76,18 @@ public class FailoverTest extends FrameworkBaseCase
     public void testTxP2PFailover() throws Exception
     {
         // Set up the test properties to match the test cases requirements.
-        testProps.setProperty(TRANSACTED_PUBLISHER_PROPNAME, true);
-        testProps.setProperty(ACK_MODE_PROPNAME, Session.AUTO_ACKNOWLEDGE);
-        testProps.setProperty(PUBSUB_PROPNAME, false);
+        getTestProps().setProperty(TRANSACTED_PUBLISHER_PROPNAME, true);
+        getTestProps().setProperty(ACK_MODE_PROPNAME, Session.AUTO_ACKNOWLEDGE);
+        getTestProps().setProperty(PUBSUB_PROPNAME, false);
 
         // MessagingTestConfigProperties props = this.getTestParameters();
 
         // Create the test circuit from the test configuration parameters.
         CircuitFactory circuitFactory = getCircuitFactory();
-        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), testProps);
+        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), getTestProps());
 
         // Create an assertion that all messages are received.
-        Assertion allMessagesReceived = testCircuit.getReceiver().allMessagesReceivedAssertion(testProps);
+        Assertion allMessagesReceived = testCircuit.getReceiver().allMessagesReceivedAssertion(getTestProps());
 
         // This test case assumes it is using a local circuit.
         LocalCircuitImpl localCircuit = (LocalCircuitImpl) testCircuit;
@@ -105,7 +105,7 @@ public class FailoverTest extends FrameworkBaseCase
             // Cause a failover.
             if (i == 50)
             {
-                failureMechanism.causeFailure();
+                getFailureMechanism().causeFailure();
             }
 
             // Wait for the reconnection to complete.
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/testcases/ImmediateMessageTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/testcases/ImmediateMessageTest.java
index 69aa6a52ae..3001211eae 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/test/testcases/ImmediateMessageTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/test/testcases/ImmediateMessageTest.java
@@ -78,226 +78,226 @@ public class ImmediateMessageTest extends FrameworkBaseCase
     public void test_QPID_517_ImmediateOkNoTxP2P() throws Exception
     {
         // Ensure transactional sessions are off.
-        testProps.setProperty(TRANSACTED_PUBLISHER_PROPNAME, false);
-        testProps.setProperty(PUBSUB_PROPNAME, false);
+        getTestProps().setProperty(TRANSACTED_PUBLISHER_PROPNAME, false);
+        getTestProps().setProperty(PUBSUB_PROPNAME, false);
 
         // Run the default test sequence over the test circuit checking for no errors.
         CircuitFactory circuitFactory = getCircuitFactory();
-        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), testProps);
+        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), getTestProps());
 
-        assertNoFailures(testCircuit.test(1, assertionList(testCircuit.getPublisher().noExceptionsAssertion(testProps))));
+        assertNoFailures(testCircuit.test(1, assertionList(testCircuit.getPublisher().noExceptionsAssertion(getTestProps()))));
     }
 
     /** Check that an immediate message is committed succesfully in a transaction when a consumer is connected. */
     public void test_QPID_517_ImmediateOkTxP2P() throws Exception
     {
         // Ensure transactional sessions are off.
-        testProps.setProperty(TRANSACTED_PUBLISHER_PROPNAME, true);
-        testProps.setProperty(PUBSUB_PROPNAME, false);
+        getTestProps().setProperty(TRANSACTED_PUBLISHER_PROPNAME, true);
+        getTestProps().setProperty(PUBSUB_PROPNAME, false);
 
         // Send one message with no errors.
         CircuitFactory circuitFactory = getCircuitFactory();
-        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), testProps);
+        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), getTestProps());
 
-        assertNoFailures(testCircuit.test(1, assertionList(testCircuit.getPublisher().noExceptionsAssertion(testProps))));
+        assertNoFailures(testCircuit.test(1, assertionList(testCircuit.getPublisher().noExceptionsAssertion(getTestProps()))));
     }
 
     /** Check that an immediate message results in no consumers code, not using transactions, when a consumer is disconnected. */
     public void test_QPID_517_ImmediateFailsConsumerDisconnectedNoTxP2P() throws Exception
     {
         // Ensure transactional sessions are off.
-        testProps.setProperty(TRANSACTED_PUBLISHER_PROPNAME, false);
-        testProps.setProperty(PUBSUB_PROPNAME, false);
+        getTestProps().setProperty(TRANSACTED_PUBLISHER_PROPNAME, false);
+        getTestProps().setProperty(PUBSUB_PROPNAME, false);
 
         // Disconnect the consumer.
-        testProps.setProperty(RECEIVER_CONSUMER_ACTIVE_PROPNAME, false);
+        getTestProps().setProperty(RECEIVER_CONSUMER_ACTIVE_PROPNAME, false);
 
         CircuitFactory circuitFactory = getCircuitFactory();
-        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), testProps);
+        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), getTestProps());
 
         // Send one message and get a linked no consumers exception.
         assertNoFailures(testCircuit.test(1,
-                assertionList(((AMQPPublisher) testCircuit.getPublisher()).noConsumersAssertion(testProps))));
+                assertionList(((AMQPPublisher) testCircuit.getPublisher()).noConsumersAssertion(getTestProps()))));
     }
 
     /** Check that an immediate message results in no consumers code, in a transaction, when a consumer is disconnected. */
     public void test_QPID_517_ImmediateFailsConsumerDisconnectedTxP2P() throws Exception
     {
         // Ensure transactional sessions are on.
-        testProps.setProperty(TRANSACTED_PUBLISHER_PROPNAME, true);
-        testProps.setProperty(PUBSUB_PROPNAME, false);
+        getTestProps().setProperty(TRANSACTED_PUBLISHER_PROPNAME, true);
+        getTestProps().setProperty(PUBSUB_PROPNAME, false);
 
         // Disconnect the consumer.
-        testProps.setProperty(RECEIVER_CONSUMER_ACTIVE_PROPNAME, false);
+        getTestProps().setProperty(RECEIVER_CONSUMER_ACTIVE_PROPNAME, false);
 
         CircuitFactory circuitFactory = getCircuitFactory();
-        Circuit testCircuit = circuitFactory.createCircuit(getConnection(),  testProps);
+        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), getTestProps());
 
         // Send one message and get a linked no consumers exception.
         assertNoFailures(testCircuit.test(1,
-                assertionList(((AMQPPublisher) testCircuit.getPublisher()).noConsumersAssertion(testProps))));
+                assertionList(((AMQPPublisher) testCircuit.getPublisher()).noConsumersAssertion(getTestProps()))));
     }
 
     /** Check that an immediate message results in no route code, not using transactions, when no outgoing route is connected. */
     public void test_QPID_517_ImmediateFailsNoRouteNoTxP2P() throws Exception
     {
         // Ensure transactional sessions are off.
-        testProps.setProperty(TRANSACTED_PUBLISHER_PROPNAME, false);
-        testProps.setProperty(PUBSUB_PROPNAME, false);
+        getTestProps().setProperty(TRANSACTED_PUBLISHER_PROPNAME, false);
+        getTestProps().setProperty(PUBSUB_PROPNAME, false);
 
         // Set up the messaging topology so that only the publishers producer is bound (do not set up the receivers to
         // collect its messages).
-        testProps.setProperty(RECEIVER_CONSUMER_BIND_PROPNAME, false);
+        getTestProps().setProperty(RECEIVER_CONSUMER_BIND_PROPNAME, false);
 
         // Send one message and get a linked no route exception.
         CircuitFactory circuitFactory = getCircuitFactory();
-        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), testProps);
+        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), getTestProps());
 
         assertNoFailures(testCircuit.test(1,
-                assertionList(((AMQPPublisher) testCircuit.getPublisher()).noRouteAssertion(testProps))));
+                assertionList(((AMQPPublisher) testCircuit.getPublisher()).noRouteAssertion(getTestProps()))));
     }
 
     /** Check that an immediate message results in no route code, upon transaction commit, when no outgoing route is connected. */
     public void test_QPID_517_ImmediateFailsNoRouteTxP2P() throws Exception
     {
         // Ensure transactional sessions are on.
-        testProps.setProperty(TRANSACTED_PUBLISHER_PROPNAME, true);
-        testProps.setProperty(PUBSUB_PROPNAME, false);
+        getTestProps().setProperty(TRANSACTED_PUBLISHER_PROPNAME, true);
+        getTestProps().setProperty(PUBSUB_PROPNAME, false);
 
         // Set up the messaging topology so that only the publishers producer is bound (do not set up the receivers to
         // collect its messages).
-        testProps.setProperty(RECEIVER_CONSUMER_BIND_PROPNAME, false);
+        getTestProps().setProperty(RECEIVER_CONSUMER_BIND_PROPNAME, false);
 
         // Send one message and get a linked no route exception.
         CircuitFactory circuitFactory = getCircuitFactory();
-        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), testProps);
+        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), getTestProps());
 
         assertNoFailures(testCircuit.test(1,
-                assertionList(((AMQPPublisher) testCircuit.getPublisher()).noRouteAssertion(testProps))));
+                assertionList(((AMQPPublisher) testCircuit.getPublisher()).noRouteAssertion(getTestProps()))));
     }
 
     /** Check that an immediate message is sent succesfully not using transactions when a consumer is connected. */
     public void test_QPID_517_ImmediateOkNoTxPubSub() throws Exception
     {
         // Ensure transactional sessions are off.
-        testProps.setProperty(TRANSACTED_PUBLISHER_PROPNAME, false);
-        testProps.setProperty(PUBSUB_PROPNAME, true);
+        getTestProps().setProperty(TRANSACTED_PUBLISHER_PROPNAME, false);
+        getTestProps().setProperty(PUBSUB_PROPNAME, true);
 
         // Send one message with no errors.
         CircuitFactory circuitFactory = getCircuitFactory();
-        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), testProps);
+        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), getTestProps());
 
         assertNoFailures(testCircuit.test(1,
-                assertionList(((AMQPPublisher) testCircuit.getPublisher()).noExceptionsAssertion(testProps))));
+                assertionList(((AMQPPublisher) testCircuit.getPublisher()).noExceptionsAssertion(getTestProps()))));
     }
 
     /** Check that an immediate message is committed succesfully in a transaction when a consumer is connected. */
     public void test_QPID_517_ImmediateOkTxPubSub() throws Exception
     {
         // Ensure transactional sessions are off.
-        testProps.setProperty(TRANSACTED_PUBLISHER_PROPNAME, true);
-        testProps.setProperty(PUBSUB_PROPNAME, true);
+        getTestProps().setProperty(TRANSACTED_PUBLISHER_PROPNAME, true);
+        getTestProps().setProperty(PUBSUB_PROPNAME, true);
 
         // Send one message with no errors.
         CircuitFactory circuitFactory = getCircuitFactory();
-        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), testProps);
+        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), getTestProps());
 
         assertNoFailures(testCircuit.test(1,
-                assertionList(((AMQPPublisher) testCircuit.getPublisher()).noExceptionsAssertion(testProps))));
+                assertionList(((AMQPPublisher) testCircuit.getPublisher()).noExceptionsAssertion(getTestProps()))));
     }
 
     /** Check that an immediate message results in no consumers code, not using transactions, when a consumer is disconnected. */
     public void test_QPID_517_ImmediateFailsConsumerDisconnectedNoTxPubSub() throws Exception
     {
         // Ensure transactional sessions are off.
-        testProps.setProperty(TRANSACTED_PUBLISHER_PROPNAME, false);
-        testProps.setProperty(PUBSUB_PROPNAME, true);
+        getTestProps().setProperty(TRANSACTED_PUBLISHER_PROPNAME, false);
+        getTestProps().setProperty(PUBSUB_PROPNAME, true);
 
         // Use durable subscriptions, so that the route remains open with no subscribers.
-        testProps.setProperty(DURABLE_SUBSCRIPTION_PROPNAME, true);
+        getTestProps().setProperty(DURABLE_SUBSCRIPTION_PROPNAME, true);
 
         // Disconnect the consumer.
-        testProps.setProperty(RECEIVER_CONSUMER_ACTIVE_PROPNAME, false);
+        getTestProps().setProperty(RECEIVER_CONSUMER_ACTIVE_PROPNAME, false);
 
         CircuitFactory circuitFactory = getCircuitFactory();
-        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), testProps);
+        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), getTestProps());
 
         // Send one message and get a linked no consumers exception.
         assertNoFailures(testCircuit.test(1,
-                assertionList(((AMQPPublisher) testCircuit.getPublisher()).noConsumersAssertion(testProps))));
+                assertionList(((AMQPPublisher) testCircuit.getPublisher()).noConsumersAssertion(getTestProps()))));
     }
 
     /** Check that an immediate message results in no consumers code, in a transaction, when a consumer is disconnected. */
     public void test_QPID_517_ImmediateFailsConsumerDisconnectedTxPubSub() throws Exception
     {
         // Ensure transactional sessions are on.
-        testProps.setProperty(TRANSACTED_PUBLISHER_PROPNAME, true);
-        testProps.setProperty(PUBSUB_PROPNAME, true);
+        getTestProps().setProperty(TRANSACTED_PUBLISHER_PROPNAME, true);
+        getTestProps().setProperty(PUBSUB_PROPNAME, true);
 
         // Use durable subscriptions, so that the route remains open with no subscribers.
-        testProps.setProperty(DURABLE_SUBSCRIPTION_PROPNAME, true);
+        getTestProps().setProperty(DURABLE_SUBSCRIPTION_PROPNAME, true);
 
         // Disconnect the consumer.
-        testProps.setProperty(RECEIVER_CONSUMER_ACTIVE_PROPNAME, false);
+        getTestProps().setProperty(RECEIVER_CONSUMER_ACTIVE_PROPNAME, false);
 
         CircuitFactory circuitFactory = getCircuitFactory();
-        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), testProps);
+        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), getTestProps());
 
         // Send one message and get a linked no consumers exception.
         assertNoFailures(testCircuit.test(1,
-                assertionList(((AMQPPublisher) testCircuit.getPublisher()).noConsumersAssertion(testProps))));
+                assertionList(((AMQPPublisher) testCircuit.getPublisher()).noConsumersAssertion(getTestProps()))));
     }
 
     /** Check that an immediate message results in no route code, not using transactions, when no outgoing route is connected. */
     public void test_QPID_517_ImmediateFailsNoRouteNoTxPubSub() throws Exception
     {
         // Ensure transactional sessions are off.
-        testProps.setProperty(TRANSACTED_PUBLISHER_PROPNAME, false);
-        testProps.setProperty(PUBSUB_PROPNAME, true);
+        getTestProps().setProperty(TRANSACTED_PUBLISHER_PROPNAME, false);
+        getTestProps().setProperty(PUBSUB_PROPNAME, true);
 
         // Set up the messaging topology so that only the publishers producer is bound (do not set up the receivers to
         // collect its messages).
-        testProps.setProperty(RECEIVER_CONSUMER_BIND_PROPNAME, false);
+        getTestProps().setProperty(RECEIVER_CONSUMER_BIND_PROPNAME, false);
 
         // Send one message and get a linked no route exception.
         CircuitFactory circuitFactory = getCircuitFactory();
-        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), testProps);
+        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), getTestProps());
 
         assertNoFailures(testCircuit.test(1,
-                assertionList(((AMQPPublisher) testCircuit.getPublisher()).noRouteAssertion(testProps))));
+                assertionList(((AMQPPublisher) testCircuit.getPublisher()).noRouteAssertion(getTestProps()))));
     }
 
     /** Check that an immediate message results in no route code, upon transaction commit, when no outgoing route is connected. */
     public void test_QPID_517_ImmediateFailsNoRouteTxPubSub() throws Exception
     {
         // Ensure transactional sessions are on.
-        testProps.setProperty(TRANSACTED_PUBLISHER_PROPNAME, true);
-        testProps.setProperty(PUBSUB_PROPNAME, true);
+        getTestProps().setProperty(TRANSACTED_PUBLISHER_PROPNAME, true);
+        getTestProps().setProperty(PUBSUB_PROPNAME, true);
 
         // Set up the messaging topology so that only the publishers producer is bound (do not set up the receivers to
         // collect its messages).
-        testProps.setProperty(RECEIVER_CONSUMER_BIND_PROPNAME, false);
+        getTestProps().setProperty(RECEIVER_CONSUMER_BIND_PROPNAME, false);
 
         // Send one message and get a linked no route exception.
         CircuitFactory circuitFactory = getCircuitFactory();
-        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), testProps);
+        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), getTestProps());
 
         assertNoFailures(testCircuit.test(1,
-                assertionList(((AMQPPublisher) testCircuit.getPublisher()).noRouteAssertion(testProps))));
+                assertionList(((AMQPPublisher) testCircuit.getPublisher()).noRouteAssertion(getTestProps()))));
     }
 
     protected void setUp() throws Exception
     {
         super.setUp();
 
-        testProps = TestContextProperties.getInstance(MessagingTestConfigProperties.defaults);
+        setTestProps(TestContextProperties.getInstance(MessagingTestConfigProperties.defaults));
 
         /** All these tests should have the immediate flag on. */
-        testProps.setProperty(IMMEDIATE_PROPNAME, true);
-        testProps.setProperty(MANDATORY_PROPNAME, false);
+        getTestProps().setProperty(IMMEDIATE_PROPNAME, true);
+        getTestProps().setProperty(MANDATORY_PROPNAME, false);
 
         /** Bind the receivers consumer by default. */
-        testProps.setProperty(RECEIVER_CONSUMER_BIND_PROPNAME, true);
-        testProps.setProperty(RECEIVER_CONSUMER_ACTIVE_PROPNAME, true);
+        getTestProps().setProperty(RECEIVER_CONSUMER_BIND_PROPNAME, true);
+        getTestProps().setProperty(RECEIVER_CONSUMER_ACTIVE_PROPNAME, true);
     }
 }
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/testcases/RollbackTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/testcases/RollbackTest.java
index 5c8012e700..edcde796a8 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/test/testcases/RollbackTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/test/testcases/RollbackTest.java
@@ -42,7 +42,7 @@ import org.apache.qpid.junit.extensions.util.TestContextProperties;
 public class RollbackTest extends FrameworkBaseCase
 {
     /** Used to read the tests configurable properties through. */
-    ParsedProperties testProps;
+    private ParsedProperties testProps;
 
     /**
      * Creates a new test case with the specified name.
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/testcases/TTLTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/testcases/TTLTest.java
index 384e3176f5..d4bab657d7 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/test/testcases/TTLTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/test/testcases/TTLTest.java
@@ -87,12 +87,12 @@ public class TTLTest extends FrameworkBaseCase
         List<Message> receivedMessages = new LinkedList<Message>();
 
         // Set up the test properties to match the test case requirements.
-        testProps.setProperty(ACK_MODE_PROPNAME, Session.AUTO_ACKNOWLEDGE);
-        testProps.setProperty(PUBSUB_PROPNAME, false);
+        getTestProps().setProperty(ACK_MODE_PROPNAME, Session.AUTO_ACKNOWLEDGE);
+        getTestProps().setProperty(PUBSUB_PROPNAME, false);
 
         // Create the test circuit from the test configuration parameters.
         CircuitFactory circuitFactory = getCircuitFactory();
-        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), testProps);
+        Circuit testCircuit = circuitFactory.createCircuit(getConnection(), getTestProps());
 
         // This test case assumes it is using a local circuit.
         LocalCircuitImpl localCircuit = (LocalCircuitImpl) testCircuit;
@@ -122,9 +122,12 @@ public class TTLTest extends FrameworkBaseCase
         // those received should have avoided being purged by the TTL.
         boolean timedOut = false;
 
-        while (!timedOut)
+
+        Message testMessage = null;
+
+        do
         {
-            Message testMessage = consumer.receive(1000);
+            testMessage = consumer.receive(1000);
 
             long ttl = testMessage.getLongProperty("testTTL");
             long timeStamp = testMessage.getJMSTimestamp();
@@ -140,7 +143,7 @@ public class TTLTest extends FrameworkBaseCase
             {
                 receivedMessages.add(testMessage);
             }*/
-        }
+        } while (!timedOut && testMessage != null);
 
         // Check that the queue and message store on the broker are empty.
         // assertTrue("Message store is not empty.", messageStoreEmpty.apply());
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/client/connection/ConnectionTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/client/connection/ConnectionTest.java
index 8bb588a036..51fa29b36a 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/client/connection/ConnectionTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/client/connection/ConnectionTest.java
@@ -61,7 +61,10 @@ public class ConnectionTest extends QpidBrokerTestCase
         }
         finally
         {
-            conn.close();
+            if(conn != null)
+            {
+                conn.close();
+            }
         }
     }
 
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/utils/ConversationFactory.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/utils/ConversationFactory.java
index 1e679af112..e153b2e0f5 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/test/utils/ConversationFactory.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/test/utils/ConversationFactory.java
@@ -278,10 +278,6 @@ public class ConversationFactory
             {
                 sendTo = sendDestination;
             }
-            else if (sendDestination != null)
-            {
-                sendTo = sendDestination;
-            }
             else
             {
                 throw new JMSException("The send destination was specified, and no most recent reply-to available to use.");