QPID-2575 : Add getClientID to SessionModel and standardise use accross 0-8/0-10, 0-10 does not appear to provide a client ID so maintaining use of Principal Name. This means migration between 0-8 and 0-10 will not behave as expected.

Correct erroneous usages of session.getPrincipal when comparing with queue.getOwner is requried. Queue owners are the client id not the authenticated user.

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

10 files changed, 75 insertions, 21 deletions

diff --git a/java/broker/src/main/java/org/apache/qpid/server/AMQChannel.java b/java/broker/src/main/java/org/apache/qpid/server/AMQChannel.java
index 573fa9d966..e2f6b5cfce 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/AMQChannel.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/AMQChannel.java
@@ -141,7 +141,6 @@ public class AMQChannel implements SessionConfig, AMQSessionModel
     private final AtomicLong _txnRejects = new AtomicLong(0);
     private final AtomicLong _txnCount = new AtomicLong(0);
 
-    // Why do we need this reference ? - ritchiem
     private final AMQProtocolSession _session;
     private AtomicBoolean _closing = new AtomicBoolean(false);
 
@@ -1070,6 +1069,11 @@ public class AMQChannel implements SessionConfig, AMQSessionModel
         return _session;
     }
 
+    public String getClientID()
+    {
+        return String.valueOf(_session.getContextKey());
+    }
+
     private class MessageDeliveryAction implements ServerTransaction.Action
     {
         private IncomingMessage _incommingMessage;
diff --git a/java/broker/src/main/java/org/apache/qpid/server/handler/QueueDeclareHandler.java b/java/broker/src/main/java/org/apache/qpid/server/handler/QueueDeclareHandler.java
index 961a165877..444505f5bb 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/handler/QueueDeclareHandler.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/handler/QueueDeclareHandler.java
@@ -63,6 +63,7 @@ public class QueueDeclareHandler implements StateAwareMethodListener<QueueDeclar
     public void methodReceived(AMQStateManager stateManager, QueueDeclareBody body, int channelId) throws AMQException
     {
         final AMQProtocolSession protocolConnection = stateManager.getProtocolSession();
+        final AMQSessionModel session = protocolConnection.getChannel(channelId);
         VirtualHost virtualHost = protocolConnection.getVirtualHost();
         ExchangeRegistry exchangeRegistry = virtualHost.getExchangeRegistry();
         QueueRegistry queueRegistry = virtualHost.getQueueRegistry();
@@ -100,11 +101,11 @@ public class QueueDeclareHandler implements StateAwareMethodListener<QueueDeclar
 
             queue = queueRegistry.getQueue(queueName);
 
-            AMQSessionModel session = null;
+            AMQSessionModel owningSession = null;
 
             if (queue != null)
             {
-                session = queue.getExclusiveOwningSession();
+                owningSession = queue.getExclusiveOwningSession();
             }
 
             if (queue == null)
@@ -163,7 +164,7 @@ public class QueueDeclareHandler implements StateAwareMethodListener<QueueDeclar
                     }
                 }
             }
-            else if (queue.isExclusive() && !queue.isDurable() && (session == null || session.getConnectionModel() != protocolConnection))
+            else if (queue.isExclusive() && !queue.isDurable() && (owningSession == null || owningSession.getConnectionModel() != protocolConnection))
             {
                 throw body.getConnectionException(AMQConstant.NOT_ALLOWED,
                                                   "Queue " + queue.getNameShortString() + " is exclusive, but not created on this Connection.");
@@ -175,13 +176,12 @@ public class QueueDeclareHandler implements StateAwareMethodListener<QueueDeclar
                                                   "Cannot re-declare queue '" + queue.getNameShortString() + "' with different exclusivity (was: "
                                                     + queue.isExclusive() + " requested " + body.getExclusive() + ")");
             }
-
-            else if (!body.getPassive() && body.getExclusive() && !(queue.isDurable() ? queue.getOwner().equals(protocolConnection.getPrincipal().getName()) : (session == null || session.getConnectionModel() != protocolConnection)))
+            else if (!body.getPassive() && body.getExclusive() && !(queue.isDurable() ? String.valueOf(queue.getOwner()).equals(session.getClientID()) : (owningSession == null || owningSession.getConnectionModel() == protocolConnection)))
             {
                 throw body.getChannelException(AMQConstant.ALREADY_EXISTS, "Cannot declare queue('" + queueName + "'), "
                                                                            + "as exclusive queue with same name "
                                                                            + "declared on another client ID('"
-                                                                           + queue.getOwner() + "')");
+                                                                           + queue.getOwner() + "') your clientID('" + session.getClientID() + "')");
 
             }
             else if(!body.getPassive() && queue.isAutoDelete() != body.getAutoDelete())
diff --git a/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolEngine.java b/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolEngine.java
index 7d70a3cdfc..4260307d04 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolEngine.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolEngine.java
@@ -1231,6 +1231,10 @@ public class AMQProtocolEngine implements ProtocolEngine, Managable, AMQProtocol
         }
     }
 
+    public String getClientID()
+    {
+        return getContextKey().toString();
+    }
 
     public void closeSession(AMQSessionModel session, AMQConstant cause, String message) throws AMQException
     {
diff --git a/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolSessionMBean.java b/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolSessionMBean.java
index 8d832a6a79..6d3a41fbe9 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolSessionMBean.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolSessionMBean.java
@@ -125,7 +125,7 @@ public class AMQProtocolSessionMBean extends AMQManagedObject implements Managed
 
     public String getClientId()
     {
-        return (_protocolSession.getContextKey() == null) ? null : _protocolSession.getContextKey().toString();
+        return String.valueOf(_protocolSession.getContextKey());
     }
 
     public String getAuthorizedId()
diff --git a/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQSessionModel.java b/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQSessionModel.java
index 29749798be..31784fd034 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQSessionModel.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQSessionModel.java
@@ -25,4 +25,6 @@ public interface AMQSessionModel
     Object getID();
 
     AMQConnectionModel getConnectionModel();
+
+    String getClientID();
 }
diff --git a/java/broker/src/main/java/org/apache/qpid/server/queue/AMQQueueMBean.java b/java/broker/src/main/java/org/apache/qpid/server/queue/AMQQueueMBean.java
index 6102e525e3..af1f412843 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/queue/AMQQueueMBean.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/queue/AMQQueueMBean.java
@@ -158,11 +158,7 @@ public class AMQQueueMBean extends AMQManagedObject implements ManagedQueue, Que
 
     public String getOwner()
     {
-        return String.valueOf(_queue.getPrincipalHolder() == null
-                              ? null
-                              : _queue.getPrincipalHolder().getPrincipal() == null
-                                ? null
-                                : _queue.getPrincipalHolder().getPrincipal().getName());
+        return String.valueOf(_queue.getOwner());
     }
 
     public boolean isAutoDelete()
diff --git a/java/broker/src/main/java/org/apache/qpid/server/store/DerbyMessageStore.java b/java/broker/src/main/java/org/apache/qpid/server/store/DerbyMessageStore.java
index 31211d6b9e..ccef66fd13 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/store/DerbyMessageStore.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/store/DerbyMessageStore.java
@@ -813,11 +813,7 @@ public class DerbyMessageStore implements MessageStore
                 {
                     stmt = conn.prepareStatement(INSERT_INTO_QUEUE);
 
-                    String owner = queue.getPrincipalHolder() == null
-                                   ? null
-                                   : queue.getPrincipalHolder().getPrincipal() == null
-                                     ? null
-                                     : queue.getPrincipalHolder().getPrincipal().getName();
+                    String owner = queue.getOwner() == null ? null : queue.getOwner().toString();
 
                     stmt.setString(1, queue.getNameShortString().toString());
                     stmt.setString(2, owner);
diff --git a/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSession.java b/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSession.java
index 52b253c075..8a16867ad8 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSession.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSession.java
@@ -565,4 +565,11 @@ public class ServerSession extends Session implements PrincipalHolder, SessionCo
         return (ServerConnection) getConnection();
     }
 
+    public String getClientID()
+    {
+        //fixme this will only work for 0-10 connections
+        // In 0-8 there is an explicit ClientID property that is != Principal.
+        return getPrincipal().getName();
+    }
+
 }
diff --git a/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSessionDelegate.java b/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSessionDelegate.java
index 7dcb268290..7479d801be 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSessionDelegate.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSessionDelegate.java
@@ -960,7 +960,7 @@ public class ServerSessionDelegate extends SessionDelegate
     {
         final QueueRegistry registry = virtualHost.getQueueRegistry();
 
-        String owner = body.getExclusive() ? session.getPrincipal().getName() : null;
+        String owner = body.getExclusive() ? session.getClientID() : null;
 
         final AMQQueue queue = AMQQueueFactory.createAMQQueueImpl(queueName, body.getDurable(), owner, body.getAutoDelete(), virtualHost,
                                                                   body.getArguments());
diff --git a/java/systests/src/main/java/org/apache/qpid/server/queue/ModelTest.java b/java/systests/src/main/java/org/apache/qpid/server/queue/ModelTest.java
index a1f8295c9b..48c30d6409 100644
--- a/java/systests/src/main/java/org/apache/qpid/server/queue/ModelTest.java
+++ b/java/systests/src/main/java/org/apache/qpid/server/queue/ModelTest.java
@@ -85,6 +85,26 @@ public class ModelTest extends QpidTestCase
     }
 
     /**
+     * Test that an exclusive transient queue can be created via AMQP.
+     *
+     * @throws Exception On unexpected error
+     */
+    public void testExclusiveQueueCreationTransientViaAMQP() throws Exception
+    {
+        Connection connection = getConnection();
+
+        String queueName = getTestQueueName();
+        boolean durable = false;
+        boolean autoDelete = false;
+        boolean exclusive = true;
+
+        createViaAMQPandValidateViaJMX(connection, queueName, durable,
+                                       autoDelete, exclusive);
+    }
+
+
+
+    /**
      * Test that a transient queue can be created via AMQP.
      *
      * @throws Exception On unexpected error
@@ -96,10 +116,34 @@ public class ModelTest extends QpidTestCase
         String queueName = getTestQueueName();
         boolean durable = false;
         boolean autoDelete = false;
-        boolean exclusive = false;
+        boolean exclusive = true;
+
+        createViaAMQPandValidateViaJMX(connection, queueName, durable,
+                                       autoDelete, exclusive);
+    }
+
+    /**
+     * Test that a durable exclusive queue can be created via AMQP.
+     *
+     * @throws Exception On unexpected error
+     */
+
+    public void testExclusiveQueueCreationDurableViaAMQP() throws Exception
+    {
+        Connection connection = getConnection();
+
+        String queueName = getTestQueueName();
+        boolean durable = true;
+        boolean autoDelete = false;
+        boolean exclusive = true;
 
         createViaAMQPandValidateViaJMX(connection, queueName, durable,
                                        autoDelete, exclusive);
+
+        // Clean up
+        ManagedBroker managedBroker =
+                _jmxUtils.getManagedBroker(VIRTUALHOST_NAME);
+        managedBroker.deleteQueue(queueName);
     }
 
     /**
@@ -126,6 +170,7 @@ public class ModelTest extends QpidTestCase
         managedBroker.deleteQueue(queueName);
     }
 
+
     /**
      * Test that a transient queue can be created via JMX.
      *
@@ -246,7 +291,7 @@ public class ModelTest extends QpidTestCase
         session.createQueue(new AMQShortString(queueName),
                             autoDelete, durable, exclusive);
 
-        validateQueueViaJMX(queueName, ((AMQConnection) connection).getUsername(), durable, autoDelete);
+        validateQueueViaJMX(queueName, exclusive ? connection.getClientID() : null, durable, autoDelete);
     }
 
     /**