From 22b64fd33e448ffc54134b28c7226342f54b4683 Mon Sep 17 00:00:00 2001
From: Robert Godfrey <rgodfrey@apache.org>
Date: Sun, 29 Jan 2012 13:05:07 +0000
Subject: QPID-3789 : [Java] code tidyups

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk/qpid@1237273 13f79535-47bb-0310-9956-ffa450edef68
---
 .../main/java/org/apache/qpid/example/Drain.java   |   18 +-
 .../java/org/apache/qpid/example/OptionParser.java |   22 +-
 .../main/java/org/apache/qpid/example/Spout.java   |   20 +-
 .../java/org/apache/qpid/client/AMQConnection.java |   12 +-
 .../qpid/client/AMQConnectionDelegate_8_0.java     |    2 -
 .../apache/qpid/client/AMQConnectionFactory.java   |   13 +-
 .../org/apache/qpid/client/AMQConnectionURL.java   |    3 +-
 .../org/apache/qpid/client/AMQDestination.java     |    1 -
 .../main/java/org/apache/qpid/client/AMQQueue.java |    1 -
 .../java/org/apache/qpid/client/AMQSession.java    |  103 +-
 .../apache/qpid/client/BasicMessageConsumer.java   |   20 +-
 .../apache/qpid/client/BasicMessageProducer.java   |    2 +-
 .../java/org/apache/qpid/client/Closeable.java     |   13 +-
 .../apache/qpid/client/QueueReceiverAdaptor.java   |    4 +-
 .../qpid/client/failover/FailoverHandler.java      |    7 -
 .../handler/ConnectionCloseMethodHandler.java      |    3 -
 .../handler/ConnectionStartMethodHandler.java      |   21 +-
 .../client/message/AMQMessageDelegate_0_10.java    |    2 +-
 .../client/message/AMQMessageDelegate_0_8.java     |   23 +-
 .../qpid/client/message/AMQPEncodedMapMessage.java |   16 +-
 .../qpid/client/message/FieldTableSupport.java     |   46 +-
 .../qpid/client/message/JMSHeaderAdapter.java      |   12 +-
 .../apache/qpid/client/message/JMSMapMessage.java  |   11 +-
 .../qpid/client/message/MessageConverter.java      |    2 +-
 .../client/message/MessageFactoryRegistry.java     |    2 +-
 .../qpid/client/message/QpidMessageProperties.java |    3 +
 .../client/message/UnprocessedMessage_0_8.java     |    2 +-
 .../apache/qpid/client/messaging/address/Node.java |    8 +-
 .../qpid/client/protocol/AMQProtocolHandler.java   |    3 +-
 .../qpid/client/protocol/AMQProtocolSession.java   |    2 -
 .../qpid/client/protocol/HeartbeatDiagnostics.java |    4 +
 .../qpid/client/security/DynamicSaslRegistrar.java |   45 +-
 .../apache/qpid/client/security/JCAProvider.java   |    1 -
 .../apache/qpid/client/state/AMQStateManager.java  |    1 -
 .../org/apache/qpid/client/url/URLParser_0_10.java |    1 -
 .../apache/qpid/client/util/BlockingWaiter.java    |    6 +-
 .../java/org/apache/qpid/collections/KeyValue.java |   46 -
 .../org/apache/qpid/collections/ReferenceMap.java  | 1038 --------------------
 .../collections/keyvalue/AbstractKeyValue.java     |   98 --
 .../collections/keyvalue/AbstractMapEntry.java     |   81 --
 .../qpid/collections/keyvalue/DefaultMapEntry.java |   66 --
 .../apache/qpid/filter/ComparisonExpression.java   |    1 -
 .../org/apache/qpid/filter/UnaryExpression.java    |   21 +-
 .../java/org/apache/qpid/jms/FailoverPolicy.java   |    6 -
 .../qpid/jms/failover/FailoverExchangeMethod.java  |    4 +-
 .../jms/failover/FailoverRoundRobinServers.java    |    3 +-
 .../qpid/jms/failover/FailoverSingleServer.java    |   14 +-
 .../org/apache/qpid/jms/failover/NoFailover.java   |    2 +-
 .../jndi/PropertiesFileInitialContextFactory.java  |    2 +-
 .../java/org/apache/qpid/jndi/ReadOnlyContext.java |    6 +-
 .../org/apache/qpid/naming/ReadOnlyContext.java    |    6 +-
 .../qpid/nclient/util/ByteBufferMessage.java       |    2 +-
 52 files changed, 211 insertions(+), 1640 deletions(-)
 delete mode 100644 java/client/src/main/java/org/apache/qpid/collections/KeyValue.java
 delete mode 100644 java/client/src/main/java/org/apache/qpid/collections/ReferenceMap.java
 delete mode 100644 java/client/src/main/java/org/apache/qpid/collections/keyvalue/AbstractKeyValue.java
 delete mode 100644 java/client/src/main/java/org/apache/qpid/collections/keyvalue/AbstractMapEntry.java
 delete mode 100644 java/client/src/main/java/org/apache/qpid/collections/keyvalue/DefaultMapEntry.java

(limited to 'java/client')

diff --git a/java/client/example/src/main/java/org/apache/qpid/example/Drain.java b/java/client/example/src/main/java/org/apache/qpid/example/Drain.java
index b43031ad23..28e1d5a87e 100644
--- a/java/client/example/src/main/java/org/apache/qpid/example/Drain.java
+++ b/java/client/example/src/main/java/org/apache/qpid/example/Drain.java
@@ -27,8 +27,6 @@ import javax.jms.MessageConsumer;
 import javax.jms.Session;
 
 import org.apache.qpid.client.AMQAnyDestination;
-import org.apache.qpid.client.AMQConnection;
-import org.slf4j.Logger;
 
 public class Drain extends OptionParser
 {
@@ -50,13 +48,13 @@ public class Drain extends OptionParser
 
     static 
     {        
-        optDefs.add(BROKER);
-        optDefs.add(HELP);
-        optDefs.add(TIMEOUT);
-        optDefs.add(FOREVER);
-        optDefs.add(COUNT);
-        optDefs.add(CON_OPTIONS);
-        optDefs.add(BROKER_OPTIONS);
+        addOption(BROKER);
+        addOption(HELP);
+        addOption(TIMEOUT);
+        addOption(FOREVER);
+        addOption(COUNT);
+        addOption(CON_OPTIONS);
+        addOption(BROKER_OPTIONS);
     }
     
     public Drain(String[] args, String usage, String desc) throws Exception
@@ -66,7 +64,7 @@ public class Drain extends OptionParser
         Connection con = createConnection();
         con.start();
         Session ssn = con.createSession(false,Session.AUTO_ACKNOWLEDGE);     
-        Destination dest = new AMQAnyDestination(address);
+        Destination dest = new AMQAnyDestination(getAddress());
         MessageConsumer consumer = ssn.createConsumer(dest);
         Message msg;
         
diff --git a/java/client/example/src/main/java/org/apache/qpid/example/OptionParser.java b/java/client/example/src/main/java/org/apache/qpid/example/OptionParser.java
index 6cc6db1974..6aa12f07fa 100644
--- a/java/client/example/src/main/java/org/apache/qpid/example/OptionParser.java
+++ b/java/client/example/src/main/java/org/apache/qpid/example/OptionParser.java
@@ -68,12 +68,12 @@ public class OptionParser
             String.class);
     
     
-    protected Map<String,Object> optMap = new HashMap<String,Object>();
-    protected static final List<Option> optDefs = new ArrayList<Option>();
+    private Map<String,Object> optMap = new HashMap<String,Object>();
+    private static final List<Option> optDefs = new ArrayList<Option>();
     
-    protected String usage;
-    protected String desc;
-    protected String address;
+    private String usage;
+    private String desc;
+    private String address;
     
     public OptionParser(String[] args, String usage, String desc)
     {   
@@ -283,7 +283,17 @@ public class OptionParser
         Connection con = new AMQConnection(buf.toString());
         return con;
     }
-    
+
+    public static void addOption(Option opt)
+    {
+        optDefs.add(opt);
+    }
+
+    protected String getAddress()
+    {
+        return address;
+    }
+
     static class Option
     {
         private final String shortForm;
diff --git a/java/client/example/src/main/java/org/apache/qpid/example/Spout.java b/java/client/example/src/main/java/org/apache/qpid/example/Spout.java
index 5da319a658..61ff2dfc19 100644
--- a/java/client/example/src/main/java/org/apache/qpid/example/Spout.java
+++ b/java/client/example/src/main/java/org/apache/qpid/example/Spout.java
@@ -69,15 +69,15 @@ public class Spout extends OptionParser
 
     static 
     {        
-        optDefs.add(BROKER);
-        optDefs.add(HELP);
-        optDefs.add(TIMEOUT);
-        optDefs.add(COUNT);
-        optDefs.add(MSG_PROPERTY);
-        optDefs.add(MAP_ENTRY);
-        optDefs.add(CONTENT);
-        optDefs.add(CON_OPTIONS);
-        optDefs.add(BROKER_OPTIONS);
+        addOption(BROKER);
+        addOption(HELP);
+        addOption(TIMEOUT);
+        addOption(COUNT);
+        addOption(MSG_PROPERTY);
+        addOption(MAP_ENTRY);
+        addOption(CONTENT);
+        addOption(CON_OPTIONS);
+        addOption(BROKER_OPTIONS);
     }
     
     public Spout(String[] args, String usage, String desc) throws Exception
@@ -87,7 +87,7 @@ public class Spout extends OptionParser
         Connection con = createConnection();
         con.start();
         Session ssn = con.createSession(false,Session.AUTO_ACKNOWLEDGE);     
-        Destination dest = new AMQAnyDestination(address);
+        Destination dest = new AMQAnyDestination(getAddress());
         MessageProducer producer = ssn.createProducer(dest);
         
         int count = Integer.parseInt(getOp(COUNT));
diff --git a/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java b/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
index 6c684e593d..39ad282422 100644
--- a/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
+++ b/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
@@ -809,13 +809,13 @@ public class AMQConnection extends Closeable implements Connection, QueueConnect
 
     public void close(List<AMQSession> sessions, long timeout) throws JMSException
     {
-        if (!_closed.getAndSet(true))
+        if (!setClosed())
         {
-            _closing.set(true);
+            setClosing(true);
             try{
                 doClose(sessions, timeout);
             }finally{
-                _closing.set(false);
+                setClosing(false);
             }
         }
     }
@@ -1241,8 +1241,8 @@ public class AMQConnection extends Closeable implements Connection, QueueConnect
         if (cause instanceof IOException || cause instanceof AMQDisconnectedException)
         {
             // If we have an IOE/AMQDisconnect there is no connection to close on.
-            _closing.set(false);
-            closer = !_closed.getAndSet(true);
+            setClosing(false);
+            closer = !setClosed();
 
             _protocolHandler.getProtocolSession().notifyError(je);
         }
@@ -1253,7 +1253,7 @@ public class AMQConnection extends Closeable implements Connection, QueueConnect
             // decide if we are going to close the session
             if (hardError(cause))
             {
-                closer = (!_closed.getAndSet(true)) || closer;
+                closer = (!setClosed()) || closer;
                 {
                     _logger.info("Closing AMQConnection due to :" + cause);
                 }
diff --git a/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java b/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java
index 5068b1bc50..40323844f6 100644
--- a/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java
+++ b/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java
@@ -213,7 +213,6 @@ public class AMQConnectionDelegate_8_0 implements AMQConnectionDelegate
                         AMQSession session =
                                 new AMQSession_0_8(_conn, channelId, transacted, acknowledgeMode, prefetchHigh,
                                                prefetchLow);
-                        // _protocolHandler.addSessionByChannel(channelId, session);
                         _conn.registerSession(channelId, session);
 
                         boolean success = false;
@@ -307,7 +306,6 @@ public class AMQConnectionDelegate_8_0 implements AMQConnectionDelegate
         for (Iterator it = sessions.iterator(); it.hasNext();)
         {
             AMQSession s = (AMQSession) it.next();
-            // _protocolHandler.addSessionByChannel(s.getChannelId(), s);
             reopenChannel(s.getChannelId(), s.getDefaultPrefetchHigh(), s.getDefaultPrefetchLow(), s.isTransacted());
             s.resubscribe();
         }
diff --git a/java/client/src/main/java/org/apache/qpid/client/AMQConnectionFactory.java b/java/client/src/main/java/org/apache/qpid/client/AMQConnectionFactory.java
index da57fefd27..cc91746d98 100644
--- a/java/client/src/main/java/org/apache/qpid/client/AMQConnectionFactory.java
+++ b/java/client/src/main/java/org/apache/qpid/client/AMQConnectionFactory.java
@@ -27,7 +27,6 @@ import org.apache.qpid.url.URLSyntaxException;
 import javax.jms.Connection;
 import javax.jms.ConnectionFactory;
 import javax.jms.JMSException;
-import javax.jms.JMSSecurityException;
 import javax.jms.QueueConnection;
 import javax.jms.QueueConnectionFactory;
 import javax.jms.TopicConnection;
@@ -278,7 +277,7 @@ public class AMQConnectionFactory implements ConnectionFactory, QueueConnectionF
      *
      * @return A newly created XAConnection
      * @throws JMSException         If creating the XAConnection fails due to some internal error.
-     * @throws JMSSecurityException If client authentication fails due to an invalid user name or password.
+     * @throws javax.jms.JMSSecurityException If client authentication fails due to an invalid user name or password.
      */
     public XAConnection createXAConnection() throws JMSException
     {
@@ -305,7 +304,7 @@ public class AMQConnectionFactory implements ConnectionFactory, QueueConnectionF
      * @param password the caller's password
      * @return A newly created XAConnection.
      * @throws JMSException         If creating the XAConnection fails due to some internal error.
-     * @throws JMSSecurityException If client authentication fails due to an invalid user name or password.
+     * @throws javax.jms.JMSSecurityException If client authentication fails due to an invalid user name or password.
      */
     public XAConnection createXAConnection(String username, String password) throws JMSException
     {
@@ -346,7 +345,7 @@ public class AMQConnectionFactory implements ConnectionFactory, QueueConnectionF
      *
      * @return A newly created XATopicConnection
      * @throws JMSException         If creating the XATopicConnection fails due to some internal error.
-     * @throws JMSSecurityException If client authentication fails due to an invalid user name or password.
+     * @throws javax.jms.JMSSecurityException If client authentication fails due to an invalid user name or password.
      */
     public XATopicConnection createXATopicConnection() throws JMSException
     {
@@ -363,7 +362,7 @@ public class AMQConnectionFactory implements ConnectionFactory, QueueConnectionF
      * @param password the caller's password
      * @return A newly created XATopicConnection.
      * @throws JMSException         If creating the XATopicConnection fails due to some internal error.
-     * @throws JMSSecurityException If client authentication fails due to an invalid user name or password.
+     * @throws javax.jms.JMSSecurityException If client authentication fails due to an invalid user name or password.
      */
     public XATopicConnection createXATopicConnection(String username, String password) throws JMSException
     {
@@ -378,7 +377,7 @@ public class AMQConnectionFactory implements ConnectionFactory, QueueConnectionF
      *
      * @return A newly created XAQueueConnection
      * @throws JMSException         If creating the XAQueueConnection fails due to some internal error.
-     * @throws JMSSecurityException If client authentication fails due to an invalid user name or password.
+     * @throws javax.jms.JMSSecurityException If client authentication fails due to an invalid user name or password.
      */
     public XAQueueConnection createXAQueueConnection() throws JMSException
     {
@@ -395,7 +394,7 @@ public class AMQConnectionFactory implements ConnectionFactory, QueueConnectionF
      * @param password the caller's password
      * @return A newly created XAQueueConnection.
      * @throws JMSException         If creating the XAQueueConnection fails due to some internal error.
-     * @throws JMSSecurityException If client authentication fails due to an invalid user name or password.
+     * @throws javax.jms.JMSSecurityException If client authentication fails due to an invalid user name or password.
      */
     public XAQueueConnection createXAQueueConnection(String username, String password) throws JMSException
     {
diff --git a/java/client/src/main/java/org/apache/qpid/client/AMQConnectionURL.java b/java/client/src/main/java/org/apache/qpid/client/AMQConnectionURL.java
index e96689b7b1..d59f48542f 100644
--- a/java/client/src/main/java/org/apache/qpid/client/AMQConnectionURL.java
+++ b/java/client/src/main/java/org/apache/qpid/client/AMQConnectionURL.java
@@ -276,7 +276,8 @@ public class AMQConnectionURL implements ConnectionURL
         if (_failoverMethod != null)
         {
             sb.append(URLHelper.DEFAULT_OPTION_SEPERATOR);
-            sb.append(OPTIONS_FAILOVER + "='");
+            sb.append(OPTIONS_FAILOVER);
+            sb.append("='");
             sb.append(_failoverMethod);
             sb.append(URLHelper.printOptions(_failoverOptions));
             sb.append("'");
diff --git a/java/client/src/main/java/org/apache/qpid/client/AMQDestination.java b/java/client/src/main/java/org/apache/qpid/client/AMQDestination.java
index 9e19cc8969..2479f89b52 100644
--- a/java/client/src/main/java/org/apache/qpid/client/AMQDestination.java
+++ b/java/client/src/main/java/org/apache/qpid/client/AMQDestination.java
@@ -622,7 +622,6 @@ public abstract class AMQDestination implements Destination, Referenceable
         int result;
         result = _exchangeName == null ? "".hashCode() : _exchangeName.hashCode();
         result = 29 * result + (_exchangeClass == null ? "".hashCode() :_exchangeClass.hashCode());
-        //result = 29 * result + _destinationName.hashCode();
         if (_queueName != null)
         {
             result = 29 * result + _queueName.hashCode();
diff --git a/java/client/src/main/java/org/apache/qpid/client/AMQQueue.java b/java/client/src/main/java/org/apache/qpid/client/AMQQueue.java
index 2ea9c4b468..c413f71906 100644
--- a/java/client/src/main/java/org/apache/qpid/client/AMQQueue.java
+++ b/java/client/src/main/java/org/apache/qpid/client/AMQQueue.java
@@ -155,7 +155,6 @@ public class AMQQueue extends AMQDestination implements Queue
 
     public AMQShortString getRoutingKey()
     {
-        //return getAMQQueueName();
         if (getAMQQueueName() != null && getAMQQueueName().equals(super.getRoutingKey()))
         {
             return getAMQQueueName();
diff --git a/java/client/src/main/java/org/apache/qpid/client/AMQSession.java b/java/client/src/main/java/org/apache/qpid/client/AMQSession.java
index 92579c31f0..25a2875b3f 100644
--- a/java/client/src/main/java/org/apache/qpid/client/AMQSession.java
+++ b/java/client/src/main/java/org/apache/qpid/client/AMQSession.java
@@ -826,15 +826,13 @@ public abstract class AMQSession<C extends BasicMessageConsumer, P extends Basic
     {
         if (_logger.isInfoEnabled())
         {
-            // StackTraceElement[] stackTrace = Thread.currentThread().getStackTrace();
-            _logger.info("Closing session: " + this); // + ":"
-            // Arrays.asList(stackTrace).subList(3, stackTrace.length - 1));
+            _logger.info("Closing session: " + this);
         }
 
         // Ensure we only try and close an open session.
-        if (!_closed.getAndSet(true))
+        if (!setClosed())
         {
-            _closing.set(true);
+            setClosing(true);
             synchronized (getFailoverMutex())
             {
                 // We must close down all producers and consumers in an orderly fashion. This is the only method
@@ -911,9 +909,9 @@ public abstract class AMQSession<C extends BasicMessageConsumer, P extends Basic
        }
 
         //if we don't have an exception then we can perform closing operations
-        _closing.set(e == null);
+        setClosing(e == null);
 
-        if (!_closed.getAndSet(true))
+        if (!setClosed())
         {
             synchronized (_messageDeliveryLock)
             {
@@ -1012,9 +1010,7 @@ public abstract class AMQSession<C extends BasicMessageConsumer, P extends Basic
                 // Just close the consumer
                 // fixme  the CancelOK is being processed before the arriving messages..
                 // The dispatcher is still to process them so the server sent in order but the client
-                // has yet to receive before the close comes in.
-
-                // consumer.markClosed();
+                // has yet to receive before the close comes in
 
                 if (consumer.isAutoClose())
                 {
@@ -1694,7 +1690,6 @@ public abstract class AMQSession<C extends BasicMessageConsumer, P extends Basic
 
     public MessageListener getMessageListener() throws JMSException
     {
-        // checkNotClosed();
         return _messageListener;
     }
 
@@ -1981,31 +1976,6 @@ public abstract class AMQSession<C extends BasicMessageConsumer, P extends Basic
 
     public void setMessageListener(MessageListener listener) throws JMSException
     {
-        // checkNotClosed();
-        //
-        // if (_dispatcher != null && !_dispatcher.connectionStopped())
-        // {
-        // throw new javax.njms.IllegalStateException("Attempt to set listener while session is started.");
-        // }
-        //
-        // // We are stopped
-        // for (Iterator<BasicMessageConsumer> i = _consumers.values().iterator(); i.hasNext();)
-        // {
-        // BasicMessageConsumer consumer = i.next();
-        //
-        // if (consumer.isReceiving())
-        // {
-        // throw new javax.njms.IllegalStateException("Another thread is already receiving synchronously.");
-        // }
-        // }
-        //
-        // _messageListener = listener;
-        //
-        // for (Iterator<BasicMessageConsumer> i = _consumers.values().iterator(); i.hasNext();)
-        // {
-        // i.next().setMessageListener(_messageListener);
-        // }
-
     }
     
     /**
@@ -2279,7 +2249,7 @@ public abstract class AMQSession<C extends BasicMessageConsumer, P extends Basic
      */
     void markClosed()
     {
-        _closed.set(true);
+        setClosed();
         _connection.deregisterSession(_channelId);
         markClosedProducersAndConsumers();
 
@@ -2294,7 +2264,7 @@ public abstract class AMQSession<C extends BasicMessageConsumer, P extends Basic
     {
         if (Thread.currentThread() == _dispatcherThread)
         {
-            while (!_closed.get() && !_queue.isEmpty())
+            while (!super.isClosed() && !_queue.isEmpty())
             {
                 Dispatchable disp;
                 try
@@ -2346,7 +2316,7 @@ public abstract class AMQSession<C extends BasicMessageConsumer, P extends Basic
     {
         if (Thread.currentThread() == _dispatcherThread)
         {
-            while (!_closed.get() && !_queue.isEmpty())
+            while (!super.isClosed() && !_queue.isEmpty())
             {
                 Dispatchable disp;
                 try
@@ -2518,32 +2488,6 @@ public abstract class AMQSession<C extends BasicMessageConsumer, P extends Basic
         }
     }
 
-    /*
-     * Binds the named queue, with the specified routing key, to the named exchange.
-     *
-     * <p/>Note that this operation automatically retries in the event of fail-over.
-     *
-     * @param queueName    The name of the queue to bind.
-     * @param routingKey   The routing key to bind the queue with.
-     * @param arguments    Additional arguments.
-     * @param exchangeName The exchange to bind the queue on.
-     *
-     * @throws AMQException If the queue cannot be bound for any reason.
-     */
-    /*private void bindQueue(AMQDestination amqd, AMQShortString queueName, AMQProtocolHandler protocolHandler, FieldTable ft)
-        throws AMQException, FailoverException
-    {
-        AMQFrame queueBind =
-            QueueBindBody.createAMQFrame(_channelId, getProtocolMajorVersion(), getProtocolMinorVersion(), ft, // arguments
-                amqd.getExchangeName(), // exchange
-                false, // nowait
-                queueName, // queue
-                amqd.getRoutingKey(), // routingKey
-                getTicket()); // ticket
-
-        protocolHandler.syncWrite(queueBind, QueueBindOkBody.class);
-    }*/
-
     private void checkNotTransacted() throws JMSException
     {
         if (getTransacted())
@@ -3372,7 +3316,6 @@ public abstract class AMQSession<C extends BasicMessageConsumer, P extends Basic
                     else
                     {
                         // should perhaps clear the _SQ here.
-                        // consumer._synchronousQueue.clear();
                         consumer.clearReceiveQueue();
                     }
 
@@ -3595,30 +3538,6 @@ public abstract class AMQSession<C extends BasicMessageConsumer, P extends Basic
 
     public abstract AMQMessageDelegateFactory getMessageDelegateFactory();
 
-    /*public void requestAccess(AMQShortString realm, boolean exclusive, boolean passive, boolean active, boolean write,
-        boolean read) throws AMQException
-    {
-        getProtocolHandler().writeCommandFrameAndWaitForReply(AccessRequestBody.createAMQFrame(getChannelId(),
-                getProtocolMajorVersion(), getProtocolMinorVersion(), active, exclusive, passive, read, realm, write),
-            new BlockingMethodFrameListener(_channelId)
-            {
-
-                public boolean processMethod(int channelId, AMQMethodBody frame) // throws AMQException
-                {
-                    if (frame instanceof AccessRequestOkBody)
-                    {
-                        setTicket(((AccessRequestOkBody) frame).getTicket());
-
-                        return true;
-                    }
-                    else
-                    {
-                        return false;
-                    }
-                }
-            });
-    }*/
-
     private class SuspenderRunner implements Runnable
     {
         private AtomicBoolean _suspend;
@@ -3662,7 +3581,7 @@ public abstract class AMQSession<C extends BasicMessageConsumer, P extends Basic
     @Override
     public boolean isClosed()
     {
-        return _closed.get() || _connection.isClosed();
+        return super.isClosed() || _connection.isClosed();
     }
 
     /**
@@ -3674,7 +3593,7 @@ public abstract class AMQSession<C extends BasicMessageConsumer, P extends Basic
     @Override
     public boolean isClosing()
     {
-        return _closing.get()|| _connection.isClosing();
+        return super.isClosing() || _connection.isClosing();
     }
     
     public boolean isDeclareExchanges()
diff --git a/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer.java b/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer.java
index f0f2c85c2f..1def0c4237 100644
--- a/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer.java
+++ b/java/client/src/main/java/org/apache/qpid/client/BasicMessageConsumer.java
@@ -536,7 +536,7 @@ public abstract class BasicMessageConsumer<U> extends Closeable implements Messa
         }
         else if (o instanceof CloseConsumerMessage)
         {
-            _closed.set(true);
+            setClosed();
             deregisterConsumer();
             return null;
         }
@@ -558,9 +558,9 @@ public abstract class BasicMessageConsumer<U> extends Closeable implements Messa
             _logger.info("Closing consumer:" + debugIdentity());
         }
 
-        if (!_closed.getAndSet(true))
+        if (!setClosed())
         {
-            _closing.set(true);
+            setClosing(true);
             if (_logger.isDebugEnabled())
             {
                 StackTraceElement[] stackTrace = Thread.currentThread().getStackTrace();
@@ -606,12 +606,8 @@ public abstract class BasicMessageConsumer<U> extends Closeable implements Messa
             }
             else
             {
-            	// FIXME: wow this is ugly
-                // //fixme this probably is not right
-                // if (!isNoConsume())
-                { // done in BasicCancelOK Handler but not sending one so just deregister.
-                    deregisterConsumer();
-                }
+            	// FIXME?
+                deregisterConsumer();
             }
 
             // This will occur if session.close is called closing all consumers we may be blocked waiting for a receive
@@ -640,7 +636,7 @@ public abstract class BasicMessageConsumer<U> extends Closeable implements Messa
     {
         // synchronized (_closed)
         {
-            _closed.set(true);
+            setClosed();
 
             if (_logger.isDebugEnabled())
             {
@@ -817,7 +813,7 @@ public abstract class BasicMessageConsumer<U> extends Closeable implements Messa
     {
         // synchronized (_closed)
         {
-            _closed.set(true);
+            setClosed();
             if (_logger.isDebugEnabled())
             {
                 StackTraceElement[] stackTrace = Thread.currentThread().getStackTrace();
@@ -1002,8 +998,6 @@ public abstract class BasicMessageConsumer<U> extends Closeable implements Messa
     public void failedOverPre()
     {
         clearReceiveQueue();
-        // TGM FIXME: think this should just be removed
-        // clearUnackedMessages();
     }
 
     public void failedOverPost() {}
diff --git a/java/client/src/main/java/org/apache/qpid/client/BasicMessageProducer.java b/java/client/src/main/java/org/apache/qpid/client/BasicMessageProducer.java
index 84747d6f09..75f198e1fa 100644
--- a/java/client/src/main/java/org/apache/qpid/client/BasicMessageProducer.java
+++ b/java/client/src/main/java/org/apache/qpid/client/BasicMessageProducer.java
@@ -327,7 +327,7 @@ public abstract class BasicMessageProducer extends Closeable implements org.apac
 
     public void close() throws JMSException
     {
-        _closed.set(true);
+        setClosed();
         _session.deregisterProducer(_producerId);
     }
 
diff --git a/java/client/src/main/java/org/apache/qpid/client/Closeable.java b/java/client/src/main/java/org/apache/qpid/client/Closeable.java
index 770e3e73fa..ba26bfc485 100644
--- a/java/client/src/main/java/org/apache/qpid/client/Closeable.java
+++ b/java/client/src/main/java/org/apache/qpid/client/Closeable.java
@@ -48,14 +48,14 @@ public abstract class Closeable
      * We use an atomic boolean so that we do not have to synchronized access to this flag. Synchronizing access to this
      * flag would mean have a synchronized block in every method.
      */
-    protected final AtomicBoolean _closed = new AtomicBoolean(false);
+    private final AtomicBoolean _closed = new AtomicBoolean(false);
 
     /**
      * Are we in the process of closing. We have this distinction so we can
      * still signal we are in the process of closing so other objects can tell
      * the difference and tidy up.
      */
-    protected final AtomicBoolean _closing = new AtomicBoolean(false);
+    private final AtomicBoolean _closing = new AtomicBoolean(false);
 
     /**
      * Checks if this is closed, and raises a JMSException if it is.
@@ -90,6 +90,15 @@ public abstract class Closeable
         return _closing.get();
     }
 
+    protected boolean setClosed()
+    {
+        return _closed.getAndSet(true);
+    }
+
+    protected void setClosing(boolean closing)
+    {
+        _closing.set(closing);
+    }
 
     /**
      * Closes this object.
diff --git a/java/client/src/main/java/org/apache/qpid/client/QueueReceiverAdaptor.java b/java/client/src/main/java/org/apache/qpid/client/QueueReceiverAdaptor.java
index 7059588367..b778ee22d6 100644
--- a/java/client/src/main/java/org/apache/qpid/client/QueueReceiverAdaptor.java
+++ b/java/client/src/main/java/org/apache/qpid/client/QueueReceiverAdaptor.java
@@ -33,8 +33,8 @@ import javax.jms.QueueReceiver;
  */
 public class QueueReceiverAdaptor implements QueueReceiver {
 
-    protected MessageConsumer _consumer;
-    protected Queue _queue;
+    private MessageConsumer _consumer;
+    private Queue _queue;
 
     protected QueueReceiverAdaptor(Queue queue, MessageConsumer consumer)
     {
diff --git a/java/client/src/main/java/org/apache/qpid/client/failover/FailoverHandler.java b/java/client/src/main/java/org/apache/qpid/client/failover/FailoverHandler.java
index 61ed8ba864..4099da18d2 100644
--- a/java/client/src/main/java/org/apache/qpid/client/failover/FailoverHandler.java
+++ b/java/client/src/main/java/org/apache/qpid/client/failover/FailoverHandler.java
@@ -231,14 +231,7 @@ public class FailoverHandler implements Runnable
                 {
                     _logger.info("Failover process failed - exception being propagated by protocol handler");
                     _amqProtocolHandler.setFailoverState(FailoverState.FAILED);
-                    /*try
-                    {*/
                     _amqProtocolHandler.exception(e);
-                    /*}
-                    catch (Exception ex)
-                    {
-                        _logger.error("Error notifying protocol session of error: " + ex, ex);
-                    }*/
                 }
             }
         }
diff --git a/java/client/src/main/java/org/apache/qpid/client/handler/ConnectionCloseMethodHandler.java b/java/client/src/main/java/org/apache/qpid/client/handler/ConnectionCloseMethodHandler.java
index 59cf805709..2cf7b089eb 100644
--- a/java/client/src/main/java/org/apache/qpid/client/handler/ConnectionCloseMethodHandler.java
+++ b/java/client/src/main/java/org/apache/qpid/client/handler/ConnectionCloseMethodHandler.java
@@ -54,9 +54,6 @@ public class ConnectionCloseMethodHandler implements StateAwareMethodListener<Co
     {
         _logger.info("ConnectionClose frame received");
 
-        // does it matter
-        // stateManager.changeState(AMQState.CONNECTION_CLOSING);
-
         AMQConstant errorCode = AMQConstant.getConstant(method.getReplyCode());
         AMQShortString reason = method.getReplyText();
 
diff --git a/java/client/src/main/java/org/apache/qpid/client/handler/ConnectionStartMethodHandler.java b/java/client/src/main/java/org/apache/qpid/client/handler/ConnectionStartMethodHandler.java
index 12dab8134f..8fc51f7799 100644
--- a/java/client/src/main/java/org/apache/qpid/client/handler/ConnectionStartMethodHandler.java
+++ b/java/client/src/main/java/org/apache/qpid/client/handler/ConnectionStartMethodHandler.java
@@ -180,14 +180,19 @@ public class ConnectionStartMethodHandler implements StateAwareMethodListener<Co
 
     private String getFullSystemInfo()
     {
-        StringBuffer fullSystemInfo = new StringBuffer();
-        fullSystemInfo.append(System.getProperty("java.runtime.name"));
-        fullSystemInfo.append(", " + System.getProperty("java.runtime.version"));
-        fullSystemInfo.append(", " + System.getProperty("java.vendor"));
-        fullSystemInfo.append(", " + System.getProperty("os.arch"));
-        fullSystemInfo.append(", " + System.getProperty("os.name"));
-        fullSystemInfo.append(", " + System.getProperty("os.version"));
-        fullSystemInfo.append(", " + System.getProperty("sun.os.patch.level"));
+        StringBuilder fullSystemInfo = new StringBuilder(System.getProperty("java.runtime.name"));
+        fullSystemInfo.append(", ");
+        fullSystemInfo.append(System.getProperty("java.runtime.version"));
+        fullSystemInfo.append(", ");
+        fullSystemInfo.append(System.getProperty("java.vendor"));
+        fullSystemInfo.append(", ");
+        fullSystemInfo.append(System.getProperty("os.arch"));
+        fullSystemInfo.append(", ");
+        fullSystemInfo.append(System.getProperty("os.name"));
+        fullSystemInfo.append(", ");
+        fullSystemInfo.append(System.getProperty("os.version"));
+        fullSystemInfo.append(", ");
+        fullSystemInfo.append(System.getProperty("sun.os.patch.level"));
 
         return fullSystemInfo.toString();
     }
diff --git a/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java b/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java
index 8f2cf80974..0973815231 100644
--- a/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java
+++ b/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java
@@ -947,7 +947,7 @@ public class AMQMessageDelegate_0_10 extends AbstractAMQMessageDelegate
 //          apply when a property is used in a message selector expression. For
 //          example, suppose you set a property as a string value, as in the
 //          following:
-//              myMessage.setStringProperty("NumberOfOrders", "2");
+//              myMessage.setStringProperty("NumberOfOrders", "2")
 //          The following expression in a message selector would evaluate to false,
 //          because a string cannot be used in an arithmetic expression:
 //          "NumberOfOrders > 1"
diff --git a/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_8.java b/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_8.java
index 5d5fbad88b..fe9f9f4d00 100644
--- a/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_8.java
+++ b/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_8.java
@@ -28,7 +28,6 @@ import org.apache.qpid.client.AMQSession_0_8;
 import org.apache.qpid.client.AMQTopic;
 import org.apache.qpid.client.CustomJMSXProperty;
 import org.apache.qpid.client.JMSAMQException;
-import org.apache.qpid.collections.ReferenceMap;
 import org.apache.qpid.framing.AMQShortString;
 import org.apache.qpid.framing.BasicContentHeaderProperties;
 import org.apache.qpid.framing.ContentHeaderProperties;
@@ -41,12 +40,28 @@ import javax.jms.MessageNotWriteableException;
 import java.net.URISyntaxException;
 import java.util.Collections;
 import java.util.Enumeration;
+import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.UUID;
 
+
 public class AMQMessageDelegate_0_8 extends AbstractAMQMessageDelegate
 {
-    private static final Map _destinationCache = Collections.synchronizedMap(new ReferenceMap());
+    private static final float DESTINATION_CACHE_LOAD_FACTOR = 0.75f;
+    private static final int DESTINATION_CACHE_SIZE = 500;
+    private static final int DESTINATION_CACHE_CAPACITY = (int) (DESTINATION_CACHE_SIZE / DESTINATION_CACHE_LOAD_FACTOR);
+
+    private static final Map<String, Destination> _destinationCache =
+            Collections.synchronizedMap(new LinkedHashMap<String,Destination>(DESTINATION_CACHE_CAPACITY,
+                                                                              DESTINATION_CACHE_LOAD_FACTOR,
+                                                                              true)
+    {
+        @Override
+        protected boolean removeEldestEntry(Map.Entry<String, Destination> eldest)
+        {
+            return size() >= DESTINATION_CACHE_SIZE;
+        }
+    });
 
     public static final String JMS_TYPE = "x-jms-type";
 
@@ -234,7 +249,7 @@ public class AMQMessageDelegate_0_8 extends AbstractAMQMessageDelegate
         }
         else
         {
-            Destination dest = (Destination) _destinationCache.get(replyToEncoding);
+            Destination dest = _destinationCache.get(replyToEncoding);
             if (dest == null)
             {
                 try
@@ -271,7 +286,7 @@ public class AMQMessageDelegate_0_8 extends AbstractAMQMessageDelegate
         final AMQDestination amqd = (AMQDestination) destination;
 
         final AMQShortString encodedDestination = amqd.getEncodedName();
-        _destinationCache.put(encodedDestination, destination);
+        _destinationCache.put(encodedDestination.asString(), destination);
         getContentHeaderProperties().setReplyTo(encodedDestination);
     }
 
diff --git a/java/client/src/main/java/org/apache/qpid/client/message/AMQPEncodedMapMessage.java b/java/client/src/main/java/org/apache/qpid/client/message/AMQPEncodedMapMessage.java
index 62822fd528..11d99f5446 100644
--- a/java/client/src/main/java/org/apache/qpid/client/message/AMQPEncodedMapMessage.java
+++ b/java/client/src/main/java/org/apache/qpid/client/message/AMQPEncodedMapMessage.java
@@ -67,7 +67,7 @@ public class AMQPEncodedMapMessage extends JMSMapMessage
                 || (value instanceof Double) || (value instanceof String) || (value instanceof byte[])
                 || (value instanceof List) || (value instanceof Map) || (value instanceof UUID) || (value == null))
         {
-            _map.put(propName, value);
+            getMap().put(propName, value);
         }
         else
         {
@@ -81,7 +81,7 @@ public class AMQPEncodedMapMessage extends JMSMapMessage
     public ByteBuffer getData()
     {
         BBEncoder encoder = new BBEncoder(1024);
-        encoder.writeMap(_map);
+        encoder.writeMap(getMap());
         return encoder.segment();
     }
     
@@ -93,22 +93,18 @@ public class AMQPEncodedMapMessage extends JMSMapMessage
             data.rewind();
             BBDecoder decoder = new BBDecoder();
             decoder.init(data);
-            _map = decoder.readMap();
+            setMap(decoder.readMap());
         }
         else
         {
-            _map.clear();
+            getMap().clear();
         }
     }
 
     // for testing
     public Map<String,Object> getMap()
     {
-        return _map;
-    }
-    
-    void setMap(Map<String,Object> map)
-    {
-        _map = map;
+        return super.getMap();
     }
+
 }
diff --git a/java/client/src/main/java/org/apache/qpid/client/message/FieldTableSupport.java b/java/client/src/main/java/org/apache/qpid/client/message/FieldTableSupport.java
index 79f8924e33..31a0440b04 100644
--- a/java/client/src/main/java/org/apache/qpid/client/message/FieldTableSupport.java
+++ b/java/client/src/main/java/org/apache/qpid/client/message/FieldTableSupport.java
@@ -28,27 +28,31 @@ import java.util.Map;
 
 public class FieldTableSupport
 {
-  public static FieldTable convertToFieldTable(Map<String,?> props)
-  {
-      FieldTable ft = new FieldTable();
-      if (props != null)
-      {
-          for (String key : props.keySet())
-          {
-              ft.setObject(key, props.get(key));
-          }
-      }
-      return ft;
-  }
+    private FieldTableSupport()
+    {
+    }
 
-  public static Map<String,Object> convertToMap(FieldTable ft)
-  {
-     Map<String,Object> map = new HashMap<String,Object>();
-     for (AMQShortString key: ft.keySet() )
-     {
-         map.put(key.asString(), ft.getObject(key));
-     }
+    public static FieldTable convertToFieldTable(Map<String,?> props)
+    {
+        FieldTable ft = new FieldTable();
+        if (props != null)
+        {
+            for (String key : props.keySet())
+            {
+                ft.setObject(key, props.get(key));
+            }
+        }
+        return ft;
+    }
 
-     return map;
-  }
+    public static Map<String,Object> convertToMap(FieldTable ft)
+    {
+        Map<String,Object> map = new HashMap<String,Object>();
+        for (AMQShortString key: ft.keySet() )
+        {
+            map.put(key.asString(), ft.getObject(key));
+        }
+
+        return map;
+    }
 }
diff --git a/java/client/src/main/java/org/apache/qpid/client/message/JMSHeaderAdapter.java b/java/client/src/main/java/org/apache/qpid/client/message/JMSHeaderAdapter.java
index 7b4bfd2e2a..122a5c4ef2 100644
--- a/java/client/src/main/java/org/apache/qpid/client/message/JMSHeaderAdapter.java
+++ b/java/client/src/main/java/org/apache/qpid/client/message/JMSHeaderAdapter.java
@@ -59,7 +59,7 @@ public final class JMSHeaderAdapter
             {
                 Object str = getHeaders().getObject(string);
 
-                if (str == null || !(str instanceof String))
+                if (!(str instanceof String))
                 {
                     throw new MessageFormatException("getBoolean can't use " + string + " item.");
                 }
@@ -88,7 +88,7 @@ public final class JMSHeaderAdapter
             {
                 Object str = getHeaders().getObject(string);
 
-                if (str == null || !(str instanceof String))
+                if (!(str instanceof String))
                 {
                     throw new MessageFormatException("getBoolean can't use " + string + " item.");
                 }
@@ -159,7 +159,7 @@ public final class JMSHeaderAdapter
             {
                 Object str = getHeaders().getObject(string);
 
-                if (str == null || !(str instanceof String))
+                if (!(str instanceof String))
                 {
                     throw new MessageFormatException("getByte can't use " + string + " item.");
                 }
@@ -227,7 +227,7 @@ public final class JMSHeaderAdapter
             {
                 Object str = getHeaders().getObject(string);
 
-                if (str == null || !(str instanceof String))
+                if (!(str instanceof String))
                 {
                     throw new MessageFormatException("getFloat can't use " + string + " item.");
                 }
@@ -284,7 +284,7 @@ public final class JMSHeaderAdapter
                         s = String.valueOf(o);
                     }
                 }
-            }//else return s // null;
+            }
         }
 
         return s;
@@ -526,7 +526,7 @@ public final class JMSHeaderAdapter
 //          apply when a property is used in a message selector expression. For
 //          example, suppose you set a property as a string value, as in the
 //          following:
-//              myMessage.setStringProperty("NumberOfOrders", "2");
+//              myMessage.setStringProperty("NumberOfOrders", "2")
 //          The following expression in a message selector would evaluate to false,
 //          because a string cannot be used in an arithmetic expression:
 //          "NumberOfOrders > 1"
diff --git a/java/client/src/main/java/org/apache/qpid/client/message/JMSMapMessage.java b/java/client/src/main/java/org/apache/qpid/client/message/JMSMapMessage.java
index 580bc5647d..e18ed80f6d 100644
--- a/java/client/src/main/java/org/apache/qpid/client/message/JMSMapMessage.java
+++ b/java/client/src/main/java/org/apache/qpid/client/message/JMSMapMessage.java
@@ -39,7 +39,7 @@ public class JMSMapMessage extends AbstractJMSMessage implements javax.jms.MapMe
 
     public static final String MIME_TYPE = "jms/map-message";
 
-    protected Map<String, Object> _map = new HashMap<String, Object>();
+    private Map<String, Object> _map = new HashMap<String, Object>();
 
     public JMSMapMessage(AMQMessageDelegateFactory delegateFactory) throws JMSException
     {
@@ -483,4 +483,13 @@ public class JMSMapMessage extends AbstractJMSMessage implements javax.jms.MapMe
         return writer.getData();
     }
 
+    protected Map<String, Object> getMap()
+    {
+        return _map;
+    }
+
+    protected void setMap(Map<String, Object> map)
+    {
+        _map = map;
+    }
 }
diff --git a/java/client/src/main/java/org/apache/qpid/client/message/MessageConverter.java b/java/client/src/main/java/org/apache/qpid/client/message/MessageConverter.java
index 78766ea036..5abd02f150 100644
--- a/java/client/src/main/java/org/apache/qpid/client/message/MessageConverter.java
+++ b/java/client/src/main/java/org/apache/qpid/client/message/MessageConverter.java
@@ -41,7 +41,7 @@ public class MessageConverter
     /**
      * Log4J logger
      */
-    protected final Logger _logger = LoggerFactory.getLogger(getClass());
+    private final Logger _logger = LoggerFactory.getLogger(getClass());
 
     /**
      * AbstractJMSMessage which will hold the converted message
diff --git a/java/client/src/main/java/org/apache/qpid/client/message/MessageFactoryRegistry.java b/java/client/src/main/java/org/apache/qpid/client/message/MessageFactoryRegistry.java
index 4540692190..fa39b4c93c 100644
--- a/java/client/src/main/java/org/apache/qpid/client/message/MessageFactoryRegistry.java
+++ b/java/client/src/main/java/org/apache/qpid/client/message/MessageFactoryRegistry.java
@@ -44,7 +44,7 @@ public class MessageFactoryRegistry
     /**
      * This class logger
      */
-    protected final Logger _logger = LoggerFactory.getLogger(getClass());
+    private final Logger _logger = LoggerFactory.getLogger(getClass());
 
     private final Map<String, MessageFactory> _mimeStringToFactoryMap = new HashMap<String, MessageFactory>();
     private final Map<AMQShortString, MessageFactory> _mimeShortStringToFactoryMap =
diff --git a/java/client/src/main/java/org/apache/qpid/client/message/QpidMessageProperties.java b/java/client/src/main/java/org/apache/qpid/client/message/QpidMessageProperties.java
index b30afafa35..663dfd39b1 100644
--- a/java/client/src/main/java/org/apache/qpid/client/message/QpidMessageProperties.java
+++ b/java/client/src/main/java/org/apache/qpid/client/message/QpidMessageProperties.java
@@ -25,6 +25,9 @@ package org.apache.qpid.client.message;
  */
 public class QpidMessageProperties 
 {
+    private QpidMessageProperties()
+    {
+    }
 
     public static final String QPID_SUBJECT = "qpid.subject";
     
diff --git a/java/client/src/main/java/org/apache/qpid/client/message/UnprocessedMessage_0_8.java b/java/client/src/main/java/org/apache/qpid/client/message/UnprocessedMessage_0_8.java
index c78b6ced93..5ed98dc8ea 100644
--- a/java/client/src/main/java/org/apache/qpid/client/message/UnprocessedMessage_0_8.java
+++ b/java/client/src/main/java/org/apache/qpid/client/message/UnprocessedMessage_0_8.java
@@ -44,7 +44,7 @@ public class UnprocessedMessage_0_8 extends UnprocessedMessage
     private AMQShortString _exchange;
     private AMQShortString _routingKey;
     private final long _deliveryId;
-    protected boolean _redelivered;
+    private boolean _redelivered;
 
     private BasicDeliverBody _deliverBody;
     private ContentHeaderBody _contentHeader;
diff --git a/java/client/src/main/java/org/apache/qpid/client/messaging/address/Node.java b/java/client/src/main/java/org/apache/qpid/client/messaging/address/Node.java
index bb5bba9068..0da0327885 100644
--- a/java/client/src/main/java/org/apache/qpid/client/messaging/address/Node.java
+++ b/java/client/src/main/java/org/apache/qpid/client/messaging/address/Node.java
@@ -104,8 +104,8 @@ public abstract class Node
     
     public static class QueueNode extends Node 
     {
-       protected boolean _isExclusive;      
-       protected QpidQueueOptions _queueOptions = new QpidQueueOptions();
+       private boolean _isExclusive;
+       private QpidQueueOptions _queueOptions = new QpidQueueOptions();
        
        public QueueNode()
        {
@@ -125,8 +125,8 @@ public abstract class Node
     
     public static class ExchangeNode extends Node 
     {
-       protected QpidExchangeOptions _exchangeOptions = new QpidExchangeOptions();
-       protected String _exchangeType;
+       private QpidExchangeOptions _exchangeOptions = new QpidExchangeOptions();
+       private String _exchangeType;
        
        public ExchangeNode()
        {
diff --git a/java/client/src/main/java/org/apache/qpid/client/protocol/AMQProtocolHandler.java b/java/client/src/main/java/org/apache/qpid/client/protocol/AMQProtocolHandler.java
index d4da0ede32..2bc2d2ef03 100644
--- a/java/client/src/main/java/org/apache/qpid/client/protocol/AMQProtocolHandler.java
+++ b/java/client/src/main/java/org/apache/qpid/client/protocol/AMQProtocolHandler.java
@@ -299,7 +299,6 @@ public class AMQProtocolHandler implements ProtocolEngine
     {
         if (_failoverState == FailoverState.NOT_STARTED)
         {
-            // if (!(cause instanceof AMQUndeliveredException) && (!(cause instanceof AMQAuthenticationException)))
             if ((cause instanceof AMQConnectionClosedException) || cause instanceof IOException)
             {
                 _logger.info("Exception caught therefore going to attempt failover: " + cause, cause);
@@ -313,7 +312,7 @@ public class AMQProtocolHandler implements ProtocolEngine
             }
 
             // FIXME Need to correctly handle other exceptions. Things like ...
-            // if (cause instanceof AMQChannelClosedException)
+            // AMQChannelClosedException
             // which will cause the JMSSession to end due to a channel close and so that Session needs
             // to be removed from the map so we can correctly still call close without an exception when trying to close
             // the server closed session.  See also CloseChannelMethodHandler as the sessionClose is never called on exception
diff --git a/java/client/src/main/java/org/apache/qpid/client/protocol/AMQProtocolSession.java b/java/client/src/main/java/org/apache/qpid/client/protocol/AMQProtocolSession.java
index c9b2e9cdc4..ced734f70f 100644
--- a/java/client/src/main/java/org/apache/qpid/client/protocol/AMQProtocolSession.java
+++ b/java/client/src/main/java/org/apache/qpid/client/protocol/AMQProtocolSession.java
@@ -90,8 +90,6 @@ public class AMQProtocolSession implements AMQVersionAwareProtocolSession
     private final Object _queueIdLock = new Object();
 
     private ProtocolVersion _protocolVersion;
-//    private VersionSpecificRegistry _registry =
-//        MainRegistry.getVersionSpecificRegistry(ProtocolVersion.getLatestSupportedVersion());
 
     private MethodRegistry _methodRegistry =
             MethodRegistry.getMethodRegistry(ProtocolVersion.getLatestSupportedVersion());
diff --git a/java/client/src/main/java/org/apache/qpid/client/protocol/HeartbeatDiagnostics.java b/java/client/src/main/java/org/apache/qpid/client/protocol/HeartbeatDiagnostics.java
index d44faeab04..d387a8ba93 100644
--- a/java/client/src/main/java/org/apache/qpid/client/protocol/HeartbeatDiagnostics.java
+++ b/java/client/src/main/java/org/apache/qpid/client/protocol/HeartbeatDiagnostics.java
@@ -24,6 +24,10 @@ class HeartbeatDiagnostics
 {
     private static final Diagnostics _impl = init();
 
+    private HeartbeatDiagnostics()
+    {
+    }
+
     private static Diagnostics init()
     {
         return Boolean.getBoolean("amqj.heartbeat.diagnostics") ? new On() : new Off();
diff --git a/java/client/src/main/java/org/apache/qpid/client/security/DynamicSaslRegistrar.java b/java/client/src/main/java/org/apache/qpid/client/security/DynamicSaslRegistrar.java
index cb3d1dea28..9198903408 100644
--- a/java/client/src/main/java/org/apache/qpid/client/security/DynamicSaslRegistrar.java
+++ b/java/client/src/main/java/org/apache/qpid/client/security/DynamicSaslRegistrar.java
@@ -62,6 +62,10 @@ public class DynamicSaslRegistrar
     /** The default name of the SASL properties file resource. */
     public static final String DEFAULT_RESOURCE_NAME = "org/apache/qpid/client/security/DynamicSaslRegistrar.properties";
 
+    private DynamicSaslRegistrar()
+    {
+    }
+
     /** Reads the properties file, and creates a dynamic security provider to register the SASL implementations with. */
     public static void registerSaslProviders()
     {
@@ -120,47 +124,6 @@ public class DynamicSaslRegistrar
         }
     }
 
-    /**
-     * Either attempts to open the specified filename as an input stream, or uses the default SASL configuration
-     * resource.
-     *
-     * @param filename The name of the file to get the SASL properties from, null to use the default.
-     *
-     * @return An input stream to read the dynamic SASL configuration from, or null if one could not be opened.
-     */
-    /*private static InputStream openPropertiesInputStream(String filename)
-    {
-        InputStream is = null;
-
-        // Flag to indicate whether the default resource should be used. By default this is true, so that the default
-        // is used when opening the file fails.
-        boolean useDefault = true;
-
-        // Try to open the file if one was specified.
-        if (filename != null)
-        {
-            try
-            {
-                is = new BufferedInputStream(new FileInputStream(new File(filename)));
-
-                // Clear the default flag because the file was succesfully opened.
-                useDefault = false;
-            }
-            catch (FileNotFoundException e)
-            {
-                _logger.error("Unable to read from file " + filename + ": " + e, e);
-            }
-        }
-
-        // Load the default resource if a file was not specified, or if opening the file failed.
-        if (useDefault)
-        {
-            is = CallbackHandlerRegistry.class.getResourceAsStream(DEFAULT_RESOURCE_NAME);
-        }
-
-        return is;
-    }*/
-
     /**
      * Parses the specified properties as a mapping from IANA registered SASL mechanism names to implementing client
      * factories. If the client factories cannot be instantiated or do not implement SaslClientFactory then the
diff --git a/java/client/src/main/java/org/apache/qpid/client/security/JCAProvider.java b/java/client/src/main/java/org/apache/qpid/client/security/JCAProvider.java
index bbf90d725c..4a91f805f6 100644
--- a/java/client/src/main/java/org/apache/qpid/client/security/JCAProvider.java
+++ b/java/client/src/main/java/org/apache/qpid/client/security/JCAProvider.java
@@ -51,7 +51,6 @@ public class JCAProvider extends Provider
         super("AMQSASLProvider-Client", 1.0, "A JCA provider that registers all "
             + "AMQ SASL providers that want to be registered");
         register(providerMap);
-//        Security.addProvider(this);
     }
 
     /**
diff --git a/java/client/src/main/java/org/apache/qpid/client/state/AMQStateManager.java b/java/client/src/main/java/org/apache/qpid/client/state/AMQStateManager.java
index 616c02f3aa..7d028e022a 100644
--- a/java/client/src/main/java/org/apache/qpid/client/state/AMQStateManager.java
+++ b/java/client/src/main/java/org/apache/qpid/client/state/AMQStateManager.java
@@ -111,7 +111,6 @@ public class AMQStateManager implements AMQMethodListener
     {
         B method = evt.getMethod();
 
-        //    StateAwareMethodListener handler = findStateTransitionHandler(_currentState, evt.getMethod());
         method.execute(_protocolSession.getMethodDispatcher(), evt.getChannelId());
         return true;
     }
diff --git a/java/client/src/main/java/org/apache/qpid/client/url/URLParser_0_10.java b/java/client/src/main/java/org/apache/qpid/client/url/URLParser_0_10.java
index 9b85ee3a60..d81868f924 100644
--- a/java/client/src/main/java/org/apache/qpid/client/url/URLParser_0_10.java
+++ b/java/client/src/main/java/org/apache/qpid/client/url/URLParser_0_10.java
@@ -57,7 +57,6 @@ public class URLParser_0_10
     private static final char PROPERTY_SEPARATOR_CHAR = ';';
     private static final char ADDRESS_SEPERATOR_CHAR = ',';
 
-    //private static final char CLIENT_ID_TRANSPORT_SEPARATOR_CHAR = ':';
     private static final char TRANSPORT_HOST_SEPARATOR_CHAR = ':';
     private static final char HOST_PORT_SEPARATOR_CHAR = ':';
     private static final char AT_CHAR = '@';
diff --git a/java/client/src/main/java/org/apache/qpid/client/util/BlockingWaiter.java b/java/client/src/main/java/org/apache/qpid/client/util/BlockingWaiter.java
index c371341265..80d171592f 100644
--- a/java/client/src/main/java/org/apache/qpid/client/util/BlockingWaiter.java
+++ b/java/client/src/main/java/org/apache/qpid/client/util/BlockingWaiter.java
@@ -184,11 +184,7 @@ public abstract class BlockingWaiter<T>
                     {
                         _logger.error(e.getMessage(), e);
                         // IGNORE    -- //fixme this isn't ideal as being interrupted isn't equivellant to sucess
-                        // if (!_ready && timeout != -1)
-                        // {
-                        // _error = new AMQException("Server did not respond timely");
-                        // _ready = true;
-                        // }
+
                     }
                 }
             }
diff --git a/java/client/src/main/java/org/apache/qpid/collections/KeyValue.java b/java/client/src/main/java/org/apache/qpid/collections/KeyValue.java
deleted file mode 100644
index e890aba968..0000000000
--- a/java/client/src/main/java/org/apache/qpid/collections/KeyValue.java
+++ /dev/null
@@ -1,46 +0,0 @@
-/*
- *  Copyright 2003-2004 The Apache Software Foundation
- *
- *  Licensed 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.collections;
-
-/**
- * Defines a simple key value pair.
- * <p>
- * A Map Entry has considerable additional semantics over and above a simple
- * key-value pair. This interface defines the minimum key value, with just the
- * two get methods.
- *
- * @since Commons Collections 3.0
- * @version $Revision$ $Date$
- * 
- * @author Stephen Colebourne
- */
-public interface KeyValue {
-
-    /**
-     * Gets the key from the pair.
-     *
-     * @return the key 
-     */
-    Object getKey();
-
-    /**
-     * Gets the value from the pair.
-     *
-     * @return the value
-     */
-    Object getValue();
-
-}
\ No newline at end of file
diff --git a/java/client/src/main/java/org/apache/qpid/collections/ReferenceMap.java b/java/client/src/main/java/org/apache/qpid/collections/ReferenceMap.java
deleted file mode 100644
index 84e4704867..0000000000
--- a/java/client/src/main/java/org/apache/qpid/collections/ReferenceMap.java
+++ /dev/null
@@ -1,1038 +0,0 @@
-/*
- *  Copyright 2001-2004 The Apache Software Foundation
- *
- *  Licensed 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.collections;
-
-import org.apache.qpid.collections.keyvalue.DefaultMapEntry;
-
-import java.io.IOException;
-import java.io.ObjectInputStream;
-import java.io.ObjectOutputStream;
-import java.lang.ref.Reference;
-import java.lang.ref.ReferenceQueue;
-import java.lang.ref.SoftReference;
-import java.lang.ref.WeakReference;
-import java.util.AbstractCollection;
-import java.util.AbstractMap;
-import java.util.AbstractSet;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.ConcurrentModificationException;
-import java.util.Iterator;
-import java.util.Map;
-import java.util.NoSuchElementException;
-import java.util.Set;
-
-/**
- *  Hash-based {@link Map} implementation that allows
- *  mappings to be removed by the garbage collector.<p>
- *
- *  When you construct a <code>ReferenceMap</code>, you can 
- *  specify what kind of references are used to store the
- *  map's keys and values.  If non-hard references are 
- *  used, then the garbage collector can remove mappings
- *  if a key or value becomes unreachable, or if the 
- *  JVM's memory is running low.  For information on how
- *  the different reference types behave, see
- *  {@link Reference}.<p>
- *
- *  Different types of references can be specified for keys
- *  and values.  The keys can be configured to be weak but
- *  the values hard, in which case this class will behave
- *  like a <a href="http://java.sun.com/j2se/1.4/docs/api/java/util/WeakHashMap.html">
- *  <code>WeakHashMap</code></a>.  However, you
- *  can also specify hard keys and weak values, or any other
- *  combination.  The default constructor uses hard keys
- *  and soft values, providing a memory-sensitive cache.<p>
- *
- *  The algorithms used are basically the same as those
- *  in {@link java.util.HashMap}.  In particular, you 
- *  can specify a load factor and capacity to suit your
- *  needs.  All optional {@link Map} operations are 
- *  supported.<p>
- *
- *  However, this {@link Map} implementation does <I>not</I>
- *  allow null elements.  Attempting to add a null key or
- *  or a null value to the map will raise a 
- *  <code>NullPointerException</code>.<p>
- *
- *  As usual, this implementation is not synchronized.  You
- *  can use {@link java.util.Collections#synchronizedMap} to 
- *  provide synchronized access to a <code>ReferenceMap</code>.
- *
- * @see java.lang.ref.Reference
- * 
- * @deprecated Moved to map subpackage. Due to be removed in v4.0.
- * @since Commons Collections 2.1
- * @version $Revision$ $Date$
- * 
- * @author Paul Jack
- */
-public class ReferenceMap extends AbstractMap
-{
-
-    /**
-     *  For serialization.
-     */
-    private static final long serialVersionUID = -3370601314380922368L;
-
-
-    /**
-     *  Constant indicating that hard references should be used.
-     */
-    final public static int HARD = 0;
-
-
-    /**
-     *  Constant indicating that soft references should be used.
-     */
-    final public static int SOFT = 1;
-
-
-    /**
-     *  Constant indicating that weak references should be used.
-     */
-    final public static int WEAK = 2;
-
-
-    // --- serialized instance variables:
-
-
-    /**
-     *  The reference type for keys.  Must be HARD, SOFT, WEAK.
-     *  Note: I originally marked this field as final, but then this class
-     *   didn't compile under JDK1.2.2.
-     *  @serial
-     */
-    private int keyType;
-
-
-    /**
-     *  The reference type for values.  Must be HARD, SOFT, WEAK.
-     *  Note: I originally marked this field as final, but then this class
-     *   didn't compile under JDK1.2.2.
-     *  @serial
-     */
-    private int valueType;
-
-
-    /**
-     *  The threshold variable is calculated by multiplying
-     *  table.length and loadFactor.  
-     *  Note: I originally marked this field as final, but then this class
-     *   didn't compile under JDK1.2.2.
-     *  @serial
-     */
-    private float loadFactor;
-    
-    /**
-     * Should the value be automatically purged when the associated key has been collected?
-     */
-    private boolean purgeValues = false;
-
-
-    // -- Non-serialized instance variables
-
-    /**
-     *  ReferenceQueue used to eliminate stale mappings.
-     *  See purge.
-     */
-    private transient ReferenceQueue queue = new ReferenceQueue();
-
-
-    /**
-     *  The hash table.  Its length is always a power of two.  
-     */
-    private transient Entry[] table;
-
-
-    /**
-     *  Number of mappings in this map.
-     */
-    private transient int size;
-
-
-    /**
-     *  When size reaches threshold, the map is resized.  
-     *  See resize().
-     */
-    private transient int threshold;
-
-
-    /**
-     *  Number of times this map has been modified.
-     */
-    private transient volatile int modCount;
-
-
-    /**
-     *  Cached key set.  May be null if key set is never accessed.
-     */
-    private transient Set keySet;
-
-
-    /**
-     *  Cached entry set.  May be null if entry set is never accessed.
-     */
-    private transient Set entrySet;
-
-
-    /**
-     *  Cached values.  May be null if values() is never accessed.
-     */
-    private transient Collection values;
-
-
-    /**
-     *  Constructs a new <code>ReferenceMap</code> that will
-     *  use hard references to keys and soft references to values.
-     */
-    public ReferenceMap() {
-        this(HARD, SOFT);
-    }
-
-    /**
-     *  Constructs a new <code>ReferenceMap</code> that will
-     *  use the specified types of references.
-     *
-     *  @param keyType  the type of reference to use for keys;
-     *   must be {@link #HARD}, {@link #SOFT}, {@link #WEAK}
-     *  @param valueType  the type of reference to use for values;
-     *   must be {@link #HARD}, {@link #SOFT}, {@link #WEAK}
-     *  @param purgeValues should the value be automatically purged when the 
-     *   key is garbage collected 
-     */
-    public ReferenceMap(int keyType, int valueType, boolean purgeValues) {
-        this(keyType, valueType);
-        this.purgeValues = purgeValues;
-    }
-
-    /**
-     *  Constructs a new <code>ReferenceMap</code> that will
-     *  use the specified types of references.
-     *
-     *  @param keyType  the type of reference to use for keys;
-     *   must be {@link #HARD}, {@link #SOFT}, {@link #WEAK}
-     *  @param valueType  the type of reference to use for values;
-     *   must be {@link #HARD}, {@link #SOFT}, {@link #WEAK}
-     */
-    public ReferenceMap(int keyType, int valueType) {
-        this(keyType, valueType, 16, 0.75f);
-    }
-
-    /**
-     *  Constructs a new <code>ReferenceMap</code> with the
-     *  specified reference types, load factor and initial
-     *  capacity.
-     *
-     *  @param keyType  the type of reference to use for keys;
-     *   must be {@link #HARD}, {@link #SOFT}, {@link #WEAK}
-     *  @param valueType  the type of reference to use for values;
-     *   must be {@link #HARD}, {@link #SOFT}, {@link #WEAK}
-     *  @param capacity  the initial capacity for the map
-     *  @param loadFactor  the load factor for the map
-     *  @param purgeValues should the value be automatically purged when the 
-     *   key is garbage collected 
-     */
-    public ReferenceMap(
-                        int keyType, 
-                        int valueType, 
-                        int capacity, 
-                        float loadFactor, 
-                        boolean purgeValues) {
-        this(keyType, valueType, capacity, loadFactor);
-        this.purgeValues = purgeValues;
-    }
-
-    /**
-     *  Constructs a new <code>ReferenceMap</code> with the
-     *  specified reference types, load factor and initial
-     *  capacity.
-     *
-     *  @param keyType  the type of reference to use for keys;
-     *   must be {@link #HARD}, {@link #SOFT}, {@link #WEAK}
-     *  @param valueType  the type of reference to use for values;
-     *   must be {@link #HARD}, {@link #SOFT}, {@link #WEAK}
-     *  @param capacity  the initial capacity for the map
-     *  @param loadFactor  the load factor for the map
-     */
-    public ReferenceMap(int keyType, int valueType, int capacity, float loadFactor) {
-        super();
-
-        verify("keyType", keyType);
-        verify("valueType", valueType);
-
-        if (capacity <= 0) {
-            throw new IllegalArgumentException("capacity must be positive");
-        }
-        if ((loadFactor <= 0.0f) || (loadFactor >= 1.0f)) {
-            throw new IllegalArgumentException("Load factor must be greater than 0 and less than 1.");
-        }
-
-        this.keyType = keyType;
-        this.valueType = valueType;
-
-        int v = 1;
-        while (v < capacity)
-        {
-            v *= 2;
-        }
-
-        this.table = new Entry[v];
-        this.loadFactor = loadFactor;
-        this.threshold = (int)(v * loadFactor);
-    }
-
-
-    // used by constructor
-    private static void verify(String name, int type) {
-        if ((type < HARD) || (type > WEAK)) {
-            throw new IllegalArgumentException(name + 
-               " must be HARD, SOFT, WEAK.");
-        }
-    }
-
-
-    /**
-     *  Writes this object to the given output stream.
-     *
-     *  @param out  the output stream to write to
-     *  @throws IOException  if the stream raises it
-     */
-    private void writeObject(ObjectOutputStream out) throws IOException {
-        out.defaultWriteObject();
-        out.writeInt(table.length);
-
-        // Have to use null-terminated list because size might shrink
-        // during iteration
-
-        for (Iterator iter = entrySet().iterator(); iter.hasNext();) {
-            Map.Entry entry = (Map.Entry)iter.next();
-            out.writeObject(entry.getKey());
-            out.writeObject(entry.getValue());
-        }
-        out.writeObject(null);
-    }
-
-
-    /**
-     *  Reads the contents of this object from the given input stream.
-     *
-     *  @param inp  the input stream to read from
-     *  @throws IOException  if the stream raises it
-     *  @throws ClassNotFoundException  if the stream raises it
-     */
-    private void readObject(ObjectInputStream inp) throws IOException, ClassNotFoundException {
-        inp.defaultReadObject();
-        table = new Entry[inp.readInt()];
-        threshold = (int)(table.length * loadFactor);
-        queue = new ReferenceQueue();
-        Object key = inp.readObject();
-        while (key != null) {
-            Object value = inp.readObject();
-            put(key, value);
-            key = inp.readObject();
-        }
-    }
-
-
-    /**
-     *  Constructs a reference of the given type to the given 
-     *  referent.  The reference is registered with the queue
-     *  for later purging.
-     *
-     *  @param type  HARD, SOFT or WEAK
-     *  @param referent  the object to refer to
-     *  @param hash  the hash code of the <I>key</I> of the mapping;
-     *    this number might be different from referent.hashCode() if
-     *    the referent represents a value and not a key
-     */
-    private Object toReference(int type, Object referent, int hash) {
-        switch (type) {
-            case HARD: return referent;
-            case SOFT: return new SoftRef(hash, referent, queue);
-            case WEAK: return new WeakRef(hash, referent, queue);
-            default: throw new Error();
-        }
-    }
-
-
-    /**
-     *  Returns the entry associated with the given key.
-     *
-     *  @param key  the key of the entry to look up
-     *  @return  the entry associated with that key, or null
-     *    if the key is not in this map
-     */
-    private Entry getEntry(Object key)
-    {
-        if (key == null)
-        {
-            return null;
-        }
-        int hash = key.hashCode();
-        int index = indexFor(hash);
-        for (Entry entry = table[index]; entry != null; entry = entry.next)
-        {
-            if ((entry.hash == hash) && key.equals(entry.getKey()))
-            {
-                return entry;
-            }
-        }
-        return null;
-    }
-
-
-    /**
-     *  Converts the given hash code into an index into the
-     *  hash table.
-     */
-    private int indexFor(int hash)
-    {
-        // mix the bits to avoid bucket collisions...
-        hash += ~(hash << 15);
-        hash ^= (hash >>> 10);
-        hash += (hash << 3);
-        hash ^= (hash >>> 6);
-        hash += ~(hash << 11);
-        hash ^= (hash >>> 16);
-        return hash & (table.length - 1);
-    }
-
-
-
-    /**
-     *  Resizes this hash table by doubling its capacity.
-     *  This is an expensive operation, as entries must
-     *  be copied from the old smaller table to the new 
-     *  bigger table.
-     */
-    private void resize() {
-        Entry[] old = table;
-        table = new Entry[old.length * 2];
-
-        for (int i = 0; i < old.length; i++) {
-            Entry next = old[i];
-            while (next != null) {
-                Entry entry = next;
-                next = next.next;
-                int index = indexFor(entry.hash);
-                entry.next = table[index];
-                table[index] = entry;
-            }
-            old[i] = null;
-        }
-        threshold = (int)(table.length * loadFactor);
-    }
-
-
-
-    /**
-     * Purges stale mappings from this map.
-     * <p>
-     * Ordinarily, stale mappings are only removed during
-     * a write operation, although this method is called for both
-     * read and write operations to maintain a consistent state.
-     * <p>
-     * Note that this method is not synchronized!  Special
-     * care must be taken if, for instance, you want stale
-     * mappings to be removed on a periodic basis by some
-     * background thread.
-     */
-    private void purge() {
-        Reference ref = queue.poll();
-        while (ref != null) {
-            purge(ref);
-            ref = queue.poll();
-        }
-    }
-
-
-    private void purge(Reference ref) {
-        // The hashCode of the reference is the hashCode of the
-        // mapping key, even if the reference refers to the 
-        // mapping value...
-        int hash = ref.hashCode();
-        int index = indexFor(hash);
-        Entry previous = null;
-        Entry entry = table[index];
-        while (entry != null)
-        {
-            if (entry.purge(ref))
-            {
-                if (previous == null)
-                {
-                    table[index] = entry.next;
-                }
-                else
-                {
-                    previous.next = entry.next;
-                }
-                this.size--;
-                return;
-            }
-            previous = entry;
-            entry = entry.next;
-        }
-
-    }
-
-
-    /**
-     *  Returns the size of this map.
-     *
-     *  @return  the size of this map
-     */
-    public int size() {
-        purge();
-        return size;
-    }
-
-
-    /**
-     *  Returns <code>true</code> if this map is empty.
-     *
-     *  @return <code>true</code> if this map is empty
-     */
-    public boolean isEmpty() {
-        purge();
-        return size == 0;
-    }
-
-
-    /**
-     *  Returns <code>true</code> if this map contains the given key.
-     *
-     *  @return true if the given key is in this map
-     */
-    public boolean containsKey(Object key) {
-        purge();
-        Entry entry = getEntry(key);
-        if (entry == null)
-        {
-            return false;
-        }
-        return entry.getValue() != null;
-    }
-
-
-    /**
-     *  Returns the value associated with the given key, if any.
-     *
-     *  @return the value associated with the given key, or <code>null</code>
-     *   if the key maps to no value
-     */
-    public Object get(Object key) {
-        purge();
-        Entry entry = getEntry(key);
-        if (entry == null)
-        {
-            return null;
-        }
-        return entry.getValue();
-    }
-
-
-    /**
-     *  Associates the given key with the given value.<p>
-     *  Neither the key nor the value may be null.
-     *
-     *  @param key  the key of the mapping
-     *  @param value  the value of the mapping
-     *  @return  the last value associated with that key, or 
-     *   null if no value was associated with the key
-     *  @throws NullPointerException if either the key or value
-     *   is null
-     */
-    public Object put(Object key, Object value) {
-        if (key == null)
-        {
-            throw new NullPointerException("null keys not allowed");
-        }
-        if (value == null)
-        {
-            throw new NullPointerException("null values not allowed");
-        }
-
-        purge();
-        if (size + 1 > threshold)
-        {
-            resize();
-        }
-
-        int hash = key.hashCode();
-        int index = indexFor(hash);
-        Entry entry = table[index];
-        while (entry != null) {
-            if ((hash == entry.hash) && key.equals(entry.getKey())) {
-                Object result = entry.getValue();
-                entry.setValue(value);
-                return result;
-            }
-            entry = entry.next;
-        }
-        this.size++; 
-        modCount++;
-        key = toReference(keyType, key, hash);
-        value = toReference(valueType, value, hash);
-        table[index] = new Entry(key, hash, value, table[index]);
-        return null;
-    }
-
-
-    /**
-     *  Removes the key and its associated value from this map.
-     *
-     *  @param key  the key to remove
-     *  @return the value associated with that key, or null if
-     *   the key was not in the map
-     */
-    public Object remove(Object key) {
-        if (key == null)
-        {
-            return null;
-        }
-        purge();
-        int hash = key.hashCode();
-        int index = indexFor(hash);
-        Entry previous = null;
-        Entry entry = table[index];
-        while (entry != null) {
-            if ((hash == entry.hash) && key.equals(entry.getKey())) {
-                if (previous == null)
-                {
-                    table[index] = entry.next;
-                }
-                else{
-                    previous.next = entry.next;
-                }
-                this.size--;
-                modCount++;
-                return entry.getValue();
-            }
-            previous = entry;
-            entry = entry.next;
-        }
-        return null;
-    }
-
-
-    /**
-     *  Clears this map.
-     */
-    public void clear() {
-        Arrays.fill(table, null);
-        size = 0;
-        while (queue.poll() != null) {}; // drain the queue
-    }
-
-
-    /**
-     *  Returns a set view of this map's entries.
-     *
-     *  @return a set view of this map's entries
-     */
-    public Set entrySet() {
-        if (entrySet != null) {
-            return entrySet;
-        } 
-        entrySet = new AbstractSet() {
-            public int size() {
-                return ReferenceMap.this.size();
-            }
-
-            public void clear() {
-                ReferenceMap.this.clear();
-            }
-
-            public boolean contains(Object o) {
-                if (o == null)
-                {
-                    return false;
-                }
-                if (!(o instanceof Map.Entry))
-                {
-                    return false;
-                }
-                Map.Entry e = (Map.Entry)o;
-                Entry e2 = getEntry(e.getKey());
-                return (e2 != null) && e.equals(e2);
-            }
-
-            public boolean remove(Object o) {
-                boolean r = contains(o);
-                if (r) {
-                    Map.Entry e = (Map.Entry)o;
-                    ReferenceMap.this.remove(e.getKey());
-                }
-                return r;
-            }
-
-            public Iterator iterator() {
-                return new EntryIterator();
-            }
-
-            public Object[] toArray() {
-                return toArray(new Object[0]);
-            }
-
-            public Object[] toArray(Object[] arr) {
-                ArrayList list = new ArrayList();
-                Iterator iterator = iterator();
-                while (iterator.hasNext()) {
-                    Entry e = (Entry)iterator.next();
-                    list.add(new DefaultMapEntry(e.getKey(), e.getValue()));
-                }
-                return list.toArray(arr);
-            }
-        };
-        return entrySet;
-    }
-
-
-    /**
-     *  Returns a set view of this map's keys.
-     *
-     *  @return a set view of this map's keys
-     */
-    public Set keySet() {
-        if (keySet != null)
-        {
-            return keySet;
-        }
-        keySet = new AbstractSet() {
-            public int size() {
-                return ReferenceMap.this.size();
-            }
-
-            public Iterator iterator() {
-                return new KeyIterator();
-            }
-
-            public boolean contains(Object o) {
-                return containsKey(o);
-            }
-
-
-            public boolean remove(Object o) {
-                Object r = ReferenceMap.this.remove(o);
-                return r != null;
-            }
-
-            public void clear() {
-                ReferenceMap.this.clear();
-            }
-
-            public Object[] toArray() {
-                return toArray(new Object[0]);
-            }
-
-            public Object[] toArray(Object[] array) {
-                Collection c = new ArrayList(size());
-                for (Iterator it = iterator(); it.hasNext(); ) {
-                    c.add(it.next());
-                }
-                return c.toArray(array);
-            }
-        };
-        return keySet;
-    }
-
-
-    /**
-     *  Returns a collection view of this map's values.
-     *
-     *  @return a collection view of this map's values.
-     */
-    public Collection values() {
-        if (values != null)
-        {
-            return values;
-        }
-        values = new AbstractCollection()  {
-            public int size() {
-                return ReferenceMap.this.size();
-            }
-
-            public void clear() {
-                ReferenceMap.this.clear();
-            }
-
-            public Iterator iterator() {
-                return new ValueIterator();
-            }
-
-            public Object[] toArray() {
-                return toArray(new Object[0]);
-            }
-
-            public Object[] toArray(Object[] array) {
-                Collection c = new ArrayList(size());
-                for (Iterator it = iterator(); it.hasNext(); ) {
-                    c.add(it.next());
-                }
-                return c.toArray(array);
-            }
-        };
-        return values;
-    }
-
-
-    // If getKey() or getValue() returns null, it means
-    // the mapping is stale and should be removed.
-    private class Entry implements Map.Entry, KeyValue {
-
-        private Object key;
-        private Object value;
-        private int hash;
-        private Entry next;
-
-
-        public Entry(Object key, int hash, Object value, Entry next) {
-            this.key = key;
-            this.hash = hash;
-            this.value = value;
-            this.next = next;
-        }
-
-
-        public Object getKey() {
-            return (keyType > HARD) ? ((Reference)key).get() : key;
-        }
-
-
-        public Object getValue() {
-            return (valueType > HARD) ? ((Reference)value).get() : value;
-        }
-
-
-        public Object setValue(Object object) {
-            Object old = getValue();
-            if (valueType > HARD)
-            {
-                ((Reference)value).clear();
-            }
-            value = toReference(valueType, object, hash);
-            return old;
-        }
-
-
-        public boolean equals(Object o) {
-            if (o == null)
-            {
-                return false;
-            }
-            if (o == this)
-            {
-                return true;
-            }
-            if (!(o instanceof Map.Entry))
-            {
-                return false;
-            }
-            
-            Map.Entry entry = (Map.Entry)o;
-            Object key = entry.getKey();
-            Object value = entry.getValue();
-            if ((key == null) || (value == null))
-            {
-                return false;
-            }
-            return key.equals(getKey()) && value.equals(getValue());
-        }
-
-
-        public int hashCode() {
-            Object v = getValue();
-            return hash ^ ((v == null) ? 0 : v.hashCode());
-        }
-
-
-        public String toString() {
-            return getKey() + "=" + getValue();
-        }
-
-
-        boolean purge(Reference ref) {
-            boolean r = (keyType > HARD) && (key == ref);
-            r = r || ((valueType > HARD) && (value == ref));
-            if (r) {
-                if (keyType > HARD)
-                {
-                    ((Reference)key).clear();
-                }
-                if (valueType > HARD) {
-                    ((Reference)value).clear();
-                } else if (purgeValues) {
-                    value = null;
-                }
-            }
-            return r;
-        }
-    }
-
-
-    private class EntryIterator implements Iterator {
-        // These fields keep track of where we are in the table.
-        private int index;
-        private Entry entry;
-        private Entry previous;
-
-        // These Object fields provide hard references to the
-        // current and next entry; this assures that if hasNext()
-        // returns true, next() will actually return a valid element.
-        private Object nextKey, nextValue;
-        private Object currentKey, currentValue;
-
-        private int expectedModCount;
-
-
-        public EntryIterator() {
-            index = (size() != 0 ? table.length : 0);
-            // have to do this here!  size() invocation above
-            // may have altered the modCount.
-            expectedModCount = modCount;
-        }
-
-
-        public boolean hasNext() {
-            checkMod();
-            while (nextNull()) {
-                Entry e = entry;
-                int i = index;
-                while ((e == null) && (i > 0)) {
-                    i--;
-                    e = table[i];
-                }
-                entry = e;
-                index = i;
-                if (e == null) {
-                    currentKey = null;
-                    currentValue = null;
-                    return false;
-                }
-                nextKey = e.getKey();
-                nextValue = e.getValue();
-                if (nextNull())
-                {
-                    entry = entry.next;
-                }
-            }
-            return true;
-        }
-
-
-        private void checkMod() {
-            if (modCount != expectedModCount) {
-                throw new ConcurrentModificationException();
-            }
-        }
-
-
-        private boolean nextNull() {
-            return (nextKey == null) || (nextValue == null);
-        }
-
-        protected Entry nextEntry() {    
-            checkMod();
-            if (nextNull() && !hasNext())
-            {
-                throw new NoSuchElementException();
-            }
-            previous = entry;
-            entry = entry.next;
-            currentKey = nextKey;
-            currentValue = nextValue;
-            nextKey = null;
-            nextValue = null;
-            return previous;
-        }
-
-
-        public Object next() {
-            return nextEntry();
-        }
-
-
-        public void remove() {
-            checkMod();
-            if (previous == null)
-            {
-                throw new IllegalStateException();
-            }
-            ReferenceMap.this.remove(currentKey);
-            previous = null;
-            currentKey = null;
-            currentValue = null;
-            expectedModCount = modCount;
-        }
-
-    }
-
-
-    private class ValueIterator extends EntryIterator {
-        public Object next() {
-            return nextEntry().getValue();
-        }
-    }
-
-
-    private class KeyIterator extends EntryIterator {
-        public Object next() {
-            return nextEntry().getKey();
-        }
-    }
-
-
-
-    // These two classes store the hashCode of the key of
-    // of the mapping, so that after they're dequeued a quick
-    // lookup of the bucket in the table can occur.
-
-
-    private static class SoftRef extends SoftReference {
-        private int hash;
-
-
-        public SoftRef(int hash, Object r, ReferenceQueue q) {
-            super(r, q);
-            this.hash = hash;
-        }
-
-
-        public int hashCode() {
-            return hash;
-        }
-    }
-
-
-    private static class WeakRef extends WeakReference {
-        private int hash;
-
-
-        public WeakRef(int hash, Object r, ReferenceQueue q) {
-            super(r, q);
-            this.hash = hash;
-        }
-
-
-        public int hashCode() {
-            return hash;
-        }
-    }
-
-
-}
diff --git a/java/client/src/main/java/org/apache/qpid/collections/keyvalue/AbstractKeyValue.java b/java/client/src/main/java/org/apache/qpid/collections/keyvalue/AbstractKeyValue.java
deleted file mode 100644
index f1b6d11bee..0000000000
--- a/java/client/src/main/java/org/apache/qpid/collections/keyvalue/AbstractKeyValue.java
+++ /dev/null
@@ -1,98 +0,0 @@
-/*
- *  Copyright 2003-2006 The Apache Software Foundation
- *
- *  Licensed 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.collections.keyvalue;
-
-import org.apache.qpid.collections.KeyValue;
-
-
-/**
- * Abstract pair class to assist with creating <code>KeyValue</code>
- * and {@link java.util.Map.Entry Map.Entry} implementations.
- *
- * @since Commons Collections 3.0
- * @version $Revision$ $Date$
- * 
- * @author James Strachan
- * @author Michael A. Smith
- * @author Neil O'Toole
- * @author Stephen Colebourne
- */
-public abstract class AbstractKeyValue implements KeyValue {
-
-    /** The key */
-    private Object key;
-    /** The value */
-    private Object value;
-
-    /**
-     * Constructs a new pair with the specified key and given value.
-     *
-     * @param key  the key for the entry, may be null
-     * @param value  the value for the entry, may be null
-     */
-    protected AbstractKeyValue(Object key, Object value) {
-        super();
-        this.key = key;
-        this.value = value;
-    }
-
-    /**
-     * Gets the key from the pair.
-     *
-     * @return the key 
-     */
-    public Object getKey() {
-        return key;
-    }
-
-    /**
-     * Gets the value from the pair.
-     *
-     * @return the value
-     */
-    public Object getValue() {
-        return value;
-    }
-
-    /**
-     * Sets the value stored in this <code>Map.Entry</code>.
-     * <p>
-     * This <code>Map.Entry</code> is not connected to a Map, so only the
-     * local data is changed.
-     *
-     * @param value  the new value
-     * @return the previous value
-     */
-    public Object setValue(Object value) {
-        Object answer = this.value;
-        this.value = value;
-        return answer;
-    }
-
-    /**
-     * Gets a debugging String view of the pair.
-     * 
-     * @return a String view of the entry
-     */
-    public String toString() {
-        return new StringBuffer()
-            .append(getKey())
-            .append('=')
-            .append(getValue())
-            .toString();
-    }
-
-}
diff --git a/java/client/src/main/java/org/apache/qpid/collections/keyvalue/AbstractMapEntry.java b/java/client/src/main/java/org/apache/qpid/collections/keyvalue/AbstractMapEntry.java
deleted file mode 100644
index 7135c31fd7..0000000000
--- a/java/client/src/main/java/org/apache/qpid/collections/keyvalue/AbstractMapEntry.java
+++ /dev/null
@@ -1,81 +0,0 @@
-/*
- *  Copyright 2003-2006 The Apache Software Foundation
- *
- *  Licensed 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.collections.keyvalue;
-
-import java.util.Map;
-
-/**
- * Abstract Pair class to assist with creating correct
- * {@link java.util.Map.Entry Map.Entry} implementations.
- *
- * @since Commons Collections 3.0
- * @version $Revision$ $Date$
- * 
- * @author James Strachan
- * @author Michael A. Smith
- * @author Neil O'Toole
- * @author Stephen Colebourne
- */
-public abstract class AbstractMapEntry extends AbstractKeyValue implements Map.Entry {
-
-    /**
-     * Constructs a new entry with the given key and given value.
-     *
-     * @param key  the key for the entry, may be null
-     * @param value  the value for the entry, may be null
-     */
-    protected AbstractMapEntry(Object key, Object value) {
-        super(key, value);
-    }
-
-    // Map.Entry interface
-    //-------------------------------------------------------------------------
-
-
-    /**
-     * Compares this <code>Map.Entry</code> with another <code>Map.Entry</code>.
-     * <p>
-     * Implemented per API documentation of {@link java.util.Map.Entry#equals(Object)}
-     * 
-     * @param obj  the object to compare to
-     * @return true if equal key and value
-     */
-    public boolean equals(Object obj) {
-        if (obj == this) {
-            return true;
-        }
-        if (obj instanceof Map.Entry == false) {
-            return false;
-        }
-        Map.Entry other = (Map.Entry) obj;
-        return
-            (getKey() == null ? other.getKey() == null : getKey().equals(other.getKey())) &&
-            (getValue() == null ? other.getValue() == null : getValue().equals(other.getValue()));
-    }
-
-    /**
-     * Gets a hashCode compatible with the equals method.
-     * <p>
-     * Implemented per API documentation of {@link java.util.Map.Entry#hashCode()}
-     * 
-     * @return a suitable hash code
-     */
-    public int hashCode() {
-        return (getKey() == null ? 0 : getKey().hashCode()) ^
-               (getValue() == null ? 0 : getValue().hashCode()); 
-    }
-
-}
\ No newline at end of file
diff --git a/java/client/src/main/java/org/apache/qpid/collections/keyvalue/DefaultMapEntry.java b/java/client/src/main/java/org/apache/qpid/collections/keyvalue/DefaultMapEntry.java
deleted file mode 100644
index 27cc4236d3..0000000000
--- a/java/client/src/main/java/org/apache/qpid/collections/keyvalue/DefaultMapEntry.java
+++ /dev/null
@@ -1,66 +0,0 @@
-/*
- *  Copyright 2001-2006 The Apache Software Foundation
- *
- *  Licensed 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.collections.keyvalue;
-
-import org.apache.qpid.collections.KeyValue;
-
-import java.util.Map;
-
-/**
- * A restricted implementation of {@link java.util.Map.Entry} that prevents
- * the <code>Map.Entry</code> contract from being broken.
- *
- * @since Commons Collections 3.0
- * @version $Revision$ $Date$
- * 
- * @author James Strachan
- * @author Michael A. Smith
- * @author Neil O'Toole
- * @author Stephen Colebourne
- */
-public final class DefaultMapEntry extends AbstractMapEntry {
-
-    /**
-     * Constructs a new entry with the specified key and given value.
-     *
-     * @param key  the key for the entry, may be null
-     * @param value  the value for the entry, may be null
-     */
-    public DefaultMapEntry(final Object key, final Object value) {
-        super(key, value);
-    }
-
-    /**
-     * Constructs a new entry from the specified <code>KeyValue</code>.
-     *
-     * @param pair  the pair to copy, must not be null
-     * @throws NullPointerException if the entry is null
-     */
-    public DefaultMapEntry(final KeyValue pair) {
-        super(pair.getKey(), pair.getValue());
-    }
-
-    /**
-     * Constructs a new entry from the specified <code>Map.Entry</code>.
-     *
-     * @param entry  the entry to copy, must not be null
-     * @throws NullPointerException if the entry is null
-     */
-    public DefaultMapEntry(final Map.Entry entry) {
-        super(entry.getKey(), entry.getValue());
-    }
-
-}
\ No newline at end of file
diff --git a/java/client/src/main/java/org/apache/qpid/filter/ComparisonExpression.java b/java/client/src/main/java/org/apache/qpid/filter/ComparisonExpression.java
index 87d43ec343..bf5220f695 100644
--- a/java/client/src/main/java/org/apache/qpid/filter/ComparisonExpression.java
+++ b/java/client/src/main/java/org/apache/qpid/filter/ComparisonExpression.java
@@ -145,7 +145,6 @@ public abstract class ComparisonExpression extends BinaryExpression implements B
             {
                 return
                     Boolean.FALSE;
-                    // throw new RuntimeException("LIKE can only operate on String identifiers.  LIKE attemped on: '" + rv.getClass());
             }
 
             return likePattern.matcher((String) rv).matches() ? Boolean.TRUE : Boolean.FALSE;
diff --git a/java/client/src/main/java/org/apache/qpid/filter/UnaryExpression.java b/java/client/src/main/java/org/apache/qpid/filter/UnaryExpression.java
index 857fbd3ba4..55fb044839 100644
--- a/java/client/src/main/java/org/apache/qpid/filter/UnaryExpression.java
+++ b/java/client/src/main/java/org/apache/qpid/filter/UnaryExpression.java
@@ -33,7 +33,7 @@ public abstract class UnaryExpression implements Expression
 {
 
     private static final BigDecimal BD_LONG_MIN_VALUE = BigDecimal.valueOf(Long.MIN_VALUE);
-    protected Expression right;
+    private final Expression right;
 
     public static Expression createNegate(Expression left)
     {
@@ -41,7 +41,7 @@ public abstract class UnaryExpression implements Expression
         {
             public Object evaluate(AbstractJMSMessage message) throws AMQInternalException
             {
-                Object rvalue = right.evaluate(message);
+                Object rvalue = getRight().evaluate(message);
                 if (rvalue == null)
                 {
                     return null;
@@ -87,7 +87,7 @@ public abstract class UnaryExpression implements Expression
             public Object evaluate(AbstractJMSMessage message) throws AMQInternalException
             {
 
-                Object rvalue = right.evaluate(message);
+                Object rvalue = getRight().evaluate(message);
                 if (rvalue == null)
                 {
                     return null;
@@ -112,7 +112,7 @@ public abstract class UnaryExpression implements Expression
             public String toString()
             {
                 StringBuffer answer = new StringBuffer();
-                answer.append(right);
+                answer.append(getRight());
                 answer.append(" ");
                 answer.append(getExpressionSymbol());
                 answer.append(" ( ");
@@ -172,7 +172,7 @@ public abstract class UnaryExpression implements Expression
         {
             public Object evaluate(AbstractJMSMessage message) throws AMQInternalException
             {
-                Boolean lvalue = (Boolean) right.evaluate(message);
+                Boolean lvalue = (Boolean) getRight().evaluate(message);
                 if (lvalue == null)
                 {
                     return null;
@@ -193,7 +193,7 @@ public abstract class UnaryExpression implements Expression
         {
             public Object evaluate(AbstractJMSMessage message) throws AMQInternalException
             {
-                Object rvalue = right.evaluate(message);
+                Object rvalue = getRight().evaluate(message);
                 if (rvalue == null)
                 {
                     return null;
@@ -209,7 +209,7 @@ public abstract class UnaryExpression implements Expression
 
             public String toString()
             {
-                return right.toString();
+                return getRight().toString();
             }
 
             public String getExpressionSymbol()
@@ -270,17 +270,12 @@ public abstract class UnaryExpression implements Expression
         return right;
     }
 
-    public void setRight(Expression expression)
-    {
-        right = expression;
-    }
-
     /**
      * @see Object#toString()
      */
     public String toString()
     {
-        return "(" + getExpressionSymbol() + " " + right.toString() + ")";
+        return "(" + getExpressionSymbol() + " " + getRight().toString() + ")";
     }
 
     /**
diff --git a/java/client/src/main/java/org/apache/qpid/jms/FailoverPolicy.java b/java/client/src/main/java/org/apache/qpid/jms/FailoverPolicy.java
index 1a4b8937ae..f4d2ecc36d 100644
--- a/java/client/src/main/java/org/apache/qpid/jms/FailoverPolicy.java
+++ b/java/client/src/main/java/org/apache/qpid/jms/FailoverPolicy.java
@@ -74,12 +74,6 @@ public class FailoverPolicy
         {
             String failoverMethod = connectionDetails.getFailoverMethod();
 
-            /*
-                        if (failoverMethod.equals(FailoverMethod.RANDOM))
-                        {
-                            //todo write a random connection Failover
-                        }
-             */
             if (failoverMethod.equals(FailoverMethod.SINGLE_BROKER))
             {
                 method = new FailoverSingleServer(connectionDetails);
diff --git a/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverExchangeMethod.java b/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverExchangeMethod.java
index fcadc7bbc7..a5eda29274 100644
--- a/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverExchangeMethod.java
+++ b/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverExchangeMethod.java
@@ -127,9 +127,7 @@ public class FailoverExchangeMethod implements FailoverMethod, MessageListener
             for (String brokerEntry:list)
             {                
                 String[] urls = brokerEntry.substring(5) .split(",");
-                // Iterate until you find the correct transport
-                // Need to reconsider the logic when the C++ broker supports
-                // SSL URLs.
+
                 for (String url:urls)
                 {
                     String[] tokens = url.split(":");
diff --git a/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverRoundRobinServers.java b/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverRoundRobinServers.java
index 17eb8fc613..84c1794723 100644
--- a/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverRoundRobinServers.java
+++ b/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverRoundRobinServers.java
@@ -50,8 +50,7 @@ public class FailoverRoundRobinServers implements FailoverMethod
     /** The current number of cycles performed. */
     private int _currentCycleRetries = 0;
 
-    /** Array of BrokerDetail used to make connections. */
-    protected ConnectionURL _connectionDetails;
+    private ConnectionURL _connectionDetails;
 
     public FailoverRoundRobinServers(ConnectionURL connectionDetails)
     {
diff --git a/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverSingleServer.java b/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverSingleServer.java
index 4da6d86ebc..1ef71eccba 100644
--- a/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverSingleServer.java
+++ b/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverSingleServer.java
@@ -37,10 +37,10 @@ public class FailoverSingleServer implements FailoverMethod
     private BrokerDetails _brokerDetail;
 
     /** The number of times to retry connecting to the sever */
-    protected int _retries;
+    private int _retries;
 
     /** The current number of attempts made to the server */
-    protected int _currentRetries = 0;
+    private int _currentRetries = 0;
 
 
     public FailoverSingleServer(ConnectionURL connectionDetails)
@@ -65,6 +65,11 @@ public class FailoverSingleServer implements FailoverMethod
         _currentRetries = 0;
     }
 
+    protected void setCurrentRetries(int currentRetries)
+    {
+        _currentRetries = currentRetries;
+    }
+
     public boolean failoverAllowed()
     {
         return _currentRetries < _retries;
@@ -151,6 +156,11 @@ public class FailoverSingleServer implements FailoverMethod
         _retries = retries;
     }
 
+    public int getRetries()
+    {
+        return _retries;
+    }
+
     public String methodName()
     {
         return "Single Server";
diff --git a/java/client/src/main/java/org/apache/qpid/jms/failover/NoFailover.java b/java/client/src/main/java/org/apache/qpid/jms/failover/NoFailover.java
index 1231324397..82cb0f9153 100644
--- a/java/client/src/main/java/org/apache/qpid/jms/failover/NoFailover.java
+++ b/java/client/src/main/java/org/apache/qpid/jms/failover/NoFailover.java
@@ -44,7 +44,7 @@ public class NoFailover extends FailoverSingleServer
     public void attainedConnection()
     {
         _connected=true;
-        _currentRetries = _retries;
+        setCurrentRetries(getRetries());
     }
 
     @Override
diff --git a/java/client/src/main/java/org/apache/qpid/jndi/PropertiesFileInitialContextFactory.java b/java/client/src/main/java/org/apache/qpid/jndi/PropertiesFileInitialContextFactory.java
index 4a02f9b341..bc3f89849e 100644
--- a/java/client/src/main/java/org/apache/qpid/jndi/PropertiesFileInitialContextFactory.java
+++ b/java/client/src/main/java/org/apache/qpid/jndi/PropertiesFileInitialContextFactory.java
@@ -53,7 +53,7 @@ import java.util.concurrent.ConcurrentHashMap;
 
 public class PropertiesFileInitialContextFactory implements InitialContextFactory
 {
-    protected final Logger _logger = LoggerFactory.getLogger(PropertiesFileInitialContextFactory.class);
+    private final Logger _logger = LoggerFactory.getLogger(PropertiesFileInitialContextFactory.class);
 
     private String CONNECTION_FACTORY_PREFIX = "connectionfactory.";
     private String DESTINATION_PREFIX = "destination.";
diff --git a/java/client/src/main/java/org/apache/qpid/jndi/ReadOnlyContext.java b/java/client/src/main/java/org/apache/qpid/jndi/ReadOnlyContext.java
index e9190a278f..76ec5f9498 100644
--- a/java/client/src/main/java/org/apache/qpid/jndi/ReadOnlyContext.java
+++ b/java/client/src/main/java/org/apache/qpid/jndi/ReadOnlyContext.java
@@ -65,9 +65,9 @@ public class ReadOnlyContext implements Context, Serializable
     private static final long serialVersionUID = -5754338187296859149L;
     protected static final NameParser nameParser = new NameParserImpl();
 
-    protected final Hashtable environment; // environment for this context
-    protected final Map bindings; // bindings at my level
-    protected final Map treeBindings; // all bindings under me
+    private final Hashtable environment; // environment for this context
+    private final Map bindings; // bindings at my level
+    private final Map treeBindings; // all bindings under me
 
     private boolean frozen = false;
     private String nameInNamespace = "";
diff --git a/java/client/src/main/java/org/apache/qpid/naming/ReadOnlyContext.java b/java/client/src/main/java/org/apache/qpid/naming/ReadOnlyContext.java
index 2216d8796a..62e6672289 100644
--- a/java/client/src/main/java/org/apache/qpid/naming/ReadOnlyContext.java
+++ b/java/client/src/main/java/org/apache/qpid/naming/ReadOnlyContext.java
@@ -64,9 +64,9 @@ public class ReadOnlyContext implements Context, Serializable
     private static final long serialVersionUID = -5754338187296859149L;
     protected static final NameParser nameParser = new NameParserImpl();
 
-    protected final Hashtable environment; // environment for this context
-    protected final Map bindings; // bindings at my level
-    protected final Map treeBindings; // all bindings under me
+    private final Hashtable environment; // environment for this context
+    private final Map bindings; // bindings at my level
+    private final Map treeBindings; // all bindings under me
 
     private boolean frozen = false;
     private String nameInNamespace = "";
diff --git a/java/client/src/main/java/org/apache/qpid/nclient/util/ByteBufferMessage.java b/java/client/src/main/java/org/apache/qpid/nclient/util/ByteBufferMessage.java
index 802cbe7270..7134f0a960 100644
--- a/java/client/src/main/java/org/apache/qpid/nclient/util/ByteBufferMessage.java
+++ b/java/client/src/main/java/org/apache/qpid/nclient/util/ByteBufferMessage.java
@@ -45,7 +45,7 @@ import java.util.List;
  */
 public class ByteBufferMessage implements Message
 {
-    private List<ByteBuffer> _data;// = new ArrayList<ByteBuffer>();
+    private List<ByteBuffer> _data;
     private ByteBuffer _readBuffer;
     private int _dataSize;
     private DeliveryProperties _currentDeliveryProps;
-- 
cgit v1.2.1