You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by or...@apache.org on 2018/05/11 16:31:56 UTC

[1/6] qpid-broker-j git commit: QPID-7830: [Broker-J] [AMQP 0-8..0-91] Mechanically refactor AMQPShortString introducing factory methods and hiding constructors

Repository: qpid-broker-j
Updated Branches:
  refs/heads/7.0.x 8ea3b655f -> dc2fa863d


QPID-7830: [Broker-J] [AMQP 0-8..0-91] Mechanically refactor AMQPShortString introducing factory methods and hiding constructors

(cherry picked from commit 0a3dfc883b7af756fbdf076665eaae4ad8202bf7)


Project: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/commit/83c6dfeb
Tree: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/tree/83c6dfeb
Diff: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/diff/83c6dfeb

Branch: refs/heads/7.0.x
Commit: 83c6dfebc3c815157ed3f04403a6968cc05eabae
Parents: 8ea3b65
Author: Keith Wall <kw...@apache.org>
Authored: Thu Apr 26 10:40:38 2018 +0100
Committer: Alex Rudyy <or...@apache.org>
Committed: Fri May 11 16:05:46 2018 +0100

----------------------------------------------------------------------
 .../berkeleydb/AMQShortStringEncoding.java      |  2 +-
 .../berkeleydb/upgrade/UpgradeFrom4To5.java     |  2 +-
 .../berkeleydb/AMQShortStringEncodingTest.java  |  4 +-
 .../store/berkeleydb/BDBMessageStoreTest.java   |  4 +-
 .../server/protocol/v0_8/AMQShortString.java    | 92 +++++---------------
 .../protocol/v0_8/AMQShortStringTest.java       | 31 ++++---
 .../server/protocol/v0_8/FieldTableTest.java    |  2 +-
 .../qpid/server/protocol/v0_8/AMQChannel.java   |  9 +-
 .../transport/BasicContentHeaderProperties.java |  2 +-
 .../protocol/v0_8/AMQPConnection_0_8Test.java   |  6 +-
 .../transport/MessagePublishInfoImplTest.java   |  8 +-
 .../MessageConverter_0_10_to_0_8.java           | 10 +--
 .../v0_8_v1_0/MessageConverter_1_0_to_v0_8.java | 10 +--
 13 files changed, 67 insertions(+), 115 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/83c6dfeb/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/AMQShortStringEncoding.java
----------------------------------------------------------------------
diff --git a/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/AMQShortStringEncoding.java b/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/AMQShortStringEncoding.java
index ce004b4..8782f65 100644
--- a/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/AMQShortStringEncoding.java
+++ b/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/AMQShortStringEncoding.java
@@ -42,7 +42,7 @@ public class AMQShortStringEncoding
         {
             byte[] stringBytes = new byte[length];
             tupleInput.readFast(stringBytes);
-            return new AMQShortString(stringBytes);
+            return AMQShortString.createAMQShortString(stringBytes);
         }
 
     }

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/83c6dfeb/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/upgrade/UpgradeFrom4To5.java
----------------------------------------------------------------------
diff --git a/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/upgrade/UpgradeFrom4To5.java b/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/upgrade/UpgradeFrom4To5.java
index 9cc604d..b310081 100644
--- a/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/upgrade/UpgradeFrom4To5.java
+++ b/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/upgrade/UpgradeFrom4To5.java
@@ -307,7 +307,7 @@ public class UpgradeFrom4To5 extends AbstractStoreUpgrade
             @Override
             public void run(Database newQueueDatabase, Database newBindingsDatabase, Transaction transaction)
             {
-                AMQShortString queueNameAMQ = new AMQShortString(queueName);
+                AMQShortString queueNameAMQ = AMQShortString.createAMQShortString(queueName);
                 QueueRecord record = new QueueRecord(queueNameAMQ, null, false, null);
 
                 DatabaseEntry key = new DatabaseEntry();

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/83c6dfeb/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/AMQShortStringEncodingTest.java
----------------------------------------------------------------------
diff --git a/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/AMQShortStringEncodingTest.java b/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/AMQShortStringEncodingTest.java
index 83d307f..cfa6674 100644
--- a/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/AMQShortStringEncodingTest.java
+++ b/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/AMQShortStringEncodingTest.java
@@ -61,7 +61,7 @@ public class AMQShortStringEncodingTest extends QpidTestCase
 
     public void testWriteReadShortStringWithLengthLess127()
     {
-        AMQShortString value = new AMQShortString("test");
+        AMQShortString value = AMQShortString.createAMQShortString("test");
 
         // write into tuple output
         TupleOutput tupleOutput = new TupleOutput();
@@ -81,7 +81,7 @@ public class AMQShortStringEncodingTest extends QpidTestCase
         {
             sb.append(ch);
         }
-        return new AMQShortString(sb.toString());
+        return AMQShortString.createAMQShortString(sb.toString());
     }
 
 }

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/83c6dfeb/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreTest.java
----------------------------------------------------------------------
diff --git a/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreTest.java b/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreTest.java
index 91f698a..8b3442b 100644
--- a/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreTest.java
+++ b/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreTest.java
@@ -65,8 +65,8 @@ public class BDBMessageStoreTest extends MessageStoreTestCase
 
     private MessagePublishInfo createPublishInfoBody_0_8()
     {
-        return new MessagePublishInfo(new AMQShortString("exchange12345"), false, true,
-                                      new AMQShortString("routingKey12345"));
+        return new MessagePublishInfo(AMQShortString.createAMQShortString("exchange12345"), false, true,
+                                      AMQShortString.createAMQShortString("routingKey12345"));
 
     }
 

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/83c6dfeb/broker-core/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQShortString.java
----------------------------------------------------------------------
diff --git a/broker-core/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQShortString.java b/broker-core/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQShortString.java
index b5aa4d7..41a4322 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQShortString.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQShortString.java
@@ -42,8 +42,6 @@ public final class AMQShortString implements Comparable<AMQShortString>
      * The maximum number of octets in AMQ short string as defined in AMQP specification
      */
     public static final int MAX_LENGTH = 255;
-    private static final byte MINUS = (byte)'-';
-    private static final byte ZERO = (byte) '0';
 
     private static final Logger LOGGER = LoggerFactory.getLogger(AMQShortString.class);
 
@@ -53,11 +51,10 @@ public final class AMQShortString implements Comparable<AMQShortString>
     private String _asString = null;
 
     private final int _length;
-    private static final char[] EMPTY_CHAR_ARRAY = new char[0];
 
-    public static final AMQShortString EMPTY_STRING = new AMQShortString((String)null);
+    public static final AMQShortString EMPTY_STRING = createAMQShortString((String)null);
 
-    public AMQShortString(byte[] data)
+    private AMQShortString(byte[] data)
     {
         if (data == null)
         {
@@ -72,7 +69,7 @@ public final class AMQShortString implements Comparable<AMQShortString>
         _offset = 0;
     }
 
-    public AMQShortString(String string)
+    private AMQShortString(String string)
     {
         final byte[] data = EncodingUtils.asUTF8Bytes(string);
         final int length = data.length;
@@ -96,31 +93,20 @@ public final class AMQShortString implements Comparable<AMQShortString>
         _asString = string == null ? "" : string;
     }
 
-    public static AMQShortString readAMQShortString(ByteBuffer buffer)
+    private AMQShortString(byte[] data, final int offset, final int length)
     {
-        int length = ((int) buffer.get()) & 0xff;
-        if(length == 0)
+        if (length > MAX_LENGTH)
         {
-            return null;
+            throw new IllegalArgumentException("Cannot create AMQShortString with number of octets over 255!");
         }
-        else
+        if (data == null)
         {
-            if (length > MAX_LENGTH)
-            {
-                throw new IllegalArgumentException("Cannot create AMQShortString with number of octets over 255!");
-            }
-            if(length > buffer.remaining())
-            {
-                throw new IllegalArgumentException("Cannot create AMQShortString with length "
-                                                   + length + " from a ByteBuffer with only "
-                                                   + buffer.remaining()
-                                                   + " bytes.");
-
-            }
-            byte[] data = new byte[length];
-            buffer.get(data);
-            return new AMQShortString(data, 0, length);
+            throw new NullPointerException("Cannot create AMQShortString with null data[]");
         }
+
+        _offset = offset;
+        _length = length;
+        _data = data;
     }
 
     public static AMQShortString readAMQShortString(QpidByteBuffer buffer)
@@ -150,21 +136,14 @@ public final class AMQShortString implements Comparable<AMQShortString>
         }
     }
 
-
-    public AMQShortString(byte[] data, final int offset, final int length)
+    public static AMQShortString createAMQShortString(byte[] data)
     {
-        if (length > MAX_LENGTH)
-        {
-            throw new IllegalArgumentException("Cannot create AMQShortString with number of octets over 255!");
-        }
-        if (data == null)
-        {
-            throw new NullPointerException("Cannot create AMQShortString with null data[]");
-        }
+        return new AMQShortString(data);
+    }
 
-        _offset = offset;
-        _length = length;
-        _data = data;
+    public static AMQShortString createAMQShortString(String string)
+    {
+        return new AMQShortString(string);
     }
 
     /**
@@ -348,37 +327,6 @@ public final class AMQShortString implements Comparable<AMQShortString>
         }
     }
 
-    public int toIntValue()
-    {
-        int pos = _offset;
-        int val = 0;
-
-
-        boolean isNegative = (_data[pos] == MINUS);
-        if(isNegative)
-        {
-            pos++;
-        }
-
-        final int end = _length + _offset;
-
-        while(pos < end)
-        {
-            int digit = (int) (_data[pos++] - ZERO);
-            if((digit < 0) || (digit > 9))
-            {
-                throw new NumberFormatException("\""+toString()+"\" is not a valid number");
-            }
-            val = val * 10;
-            val += digit;
-        }
-        if(isNegative)
-        {
-            val = val * -1;
-        }
-        return val;
-    }
-
     public boolean contains(final byte b)
     {
         final int end = _length + _offset;
@@ -422,7 +370,7 @@ public final class AMQShortString implements Comparable<AMQShortString>
                 }
 
             }
-            return new AMQShortString(bytes);
+            return createAMQShortString(bytes);
         }
     }
 
@@ -439,7 +387,7 @@ public final class AMQShortString implements Comparable<AMQShortString>
         }
         else
         {
-            return new AMQShortString(obj);
+            return createAMQShortString(obj);
         }
 
     }

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/83c6dfeb/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQShortStringTest.java
----------------------------------------------------------------------
diff --git a/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQShortStringTest.java b/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQShortStringTest.java
index 26fc7dd..659d3e8 100644
--- a/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQShortStringTest.java
+++ b/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQShortStringTest.java
@@ -27,18 +27,18 @@ import java.nio.charset.StandardCharsets;
 public class AMQShortStringTest extends QpidTestCase
 {
 
-    public static final AMQShortString HELLO = new AMQShortString("Hello");
-    public static final AMQShortString HELL = new AMQShortString("Hell");
-    public static final AMQShortString GOODBYE = new AMQShortString("Goodbye");
-    public static final AMQShortString GOOD = new AMQShortString("Good");
-    public static final AMQShortString BYE = new AMQShortString("BYE");
+    public static final AMQShortString HELLO = AMQShortString.createAMQShortString("Hello");
+    public static final AMQShortString HELL = AMQShortString.createAMQShortString("Hell");
+    public static final AMQShortString GOODBYE = AMQShortString.createAMQShortString("Goodbye");
+    public static final AMQShortString GOOD = AMQShortString.createAMQShortString("Good");
+    public static final AMQShortString BYE = AMQShortString.createAMQShortString("BYE");
 
 
     public void testEquals()
     {
-        assertEquals(GOODBYE, new AMQShortString("Goodbye"));
-        assertEquals(new AMQShortString("A"), new AMQShortString("A"));
-        assertFalse(new AMQShortString("A").equals(new AMQShortString("a")));
+        assertEquals(GOODBYE, AMQShortString.createAMQShortString("Goodbye"));
+        assertEquals(AMQShortString.createAMQShortString("A"), AMQShortString.createAMQShortString("A"));
+        assertFalse(AMQShortString.createAMQShortString("A").equals(AMQShortString.createAMQShortString("a")));
     }
 
     /**
@@ -48,8 +48,10 @@ public class AMQShortStringTest extends QpidTestCase
     public void testCreateAMQShortStringByteArray()
     {
         byte[] bytes = "test".getBytes(StandardCharsets.UTF_8);
-        AMQShortString string = new AMQShortString(bytes);
-        assertEquals("constructed amq short string length differs from expected", 4, string.length());
+        AMQShortString string = AMQShortString.createAMQShortString(bytes);
+        assertEquals("constructed amq short string length differs from expected",
+                            (long) 4,
+                            (long) string.length());
 
         assertTrue("constructed amq short string differs from expected", string.toString().equals("test"));
     }
@@ -62,8 +64,9 @@ public class AMQShortStringTest extends QpidTestCase
      */
     public void testCreateAMQShortStringString()
     {
-        AMQShortString string = new AMQShortString("test");
-        assertEquals("constructed amq short string length differs from expected", 4, string.length());
+        AMQShortString string = AMQShortString.createAMQShortString("test");
+        assertEquals("constructed amq short string length differs from expected", (long) 4, (long) string.length
+                ());
 
         assertTrue("constructed amq short string differs from expected", string.toString().equals("test"));
     }
@@ -80,7 +83,7 @@ public class AMQShortStringTest extends QpidTestCase
         byte[] bytes = test.getBytes(StandardCharsets.UTF_8);
         try
         {
-            new AMQShortString(bytes);
+            AMQShortString.createAMQShortString(bytes);
             fail("It should not be possible to create AMQShortString with length over 255");
         }
         catch (IllegalArgumentException e)
@@ -101,7 +104,7 @@ public class AMQShortStringTest extends QpidTestCase
         String test = buildString('a', 256);
         try
         {
-            new AMQShortString(test);
+            AMQShortString.createAMQShortString(test);
             fail("It should not be possible to create AMQShortString with length over 255");
         }
         catch (IllegalArgumentException e)

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/83c6dfeb/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/FieldTableTest.java
----------------------------------------------------------------------
diff --git a/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/FieldTableTest.java b/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/FieldTableTest.java
index 40241c9..410f27a 100644
--- a/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/FieldTableTest.java
+++ b/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/FieldTableTest.java
@@ -906,7 +906,7 @@ public class FieldTableTest extends QpidTestCase
      */
     public void testAddingAllFromFieldTableCreatedUsingEncodedBytes() throws Exception
     {
-        AMQShortString myBooleanTestProperty = new AMQShortString("myBooleanTestProperty");
+        AMQShortString myBooleanTestProperty = AMQShortString.createAMQShortString("myBooleanTestProperty");
 
         //Create a new FieldTable and use it to encode data into a byte array.
         FieldTable encodeTable = new FieldTable();

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/83c6dfeb/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQChannel.java
----------------------------------------------------------------------
diff --git a/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQChannel.java b/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQChannel.java
index 6418ea9..d2b340b 100644
--- a/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQChannel.java
+++ b/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQChannel.java
@@ -175,7 +175,8 @@ public class AMQChannel extends AbstractAMQPSession<AMQChannel, ConsumerTarget_0
 
     private List<MessageConsumerAssociation> _resendList = new ArrayList<>();
     private static final
-    AMQShortString IMMEDIATE_DELIVERY_REPLY_TEXT = new AMQShortString("Immediate delivery is not possible.");
+    AMQShortString IMMEDIATE_DELIVERY_REPLY_TEXT =
+            AMQShortString.createAMQShortString("Immediate delivery is not possible.");
 
     private final ClientDeliveryMethod _clientDeliveryMethod;
 
@@ -564,7 +565,7 @@ public class AMQChannel extends AbstractAMQPSession<AMQChannel, ConsumerTarget_0
     {
         if (tag == null)
         {
-            tag = new AMQShortString("sgen_" + getNextConsumerTag());
+            tag = AMQShortString.createAMQShortString("sgen_" + getNextConsumerTag());
         }
 
         if (_tag2SubscriptionTargetMap.containsKey(tag))
@@ -2538,7 +2539,7 @@ public class AMQChannel extends AbstractAMQPSession<AMQChannel, ConsumerTarget_0
 
         if (isDefaultExchange(exchangeName))
         {
-            if (!new AMQShortString(ExchangeDefaults.DIRECT_EXCHANGE_CLASS).equals(type))
+            if (!AMQShortString.createAMQShortString(ExchangeDefaults.DIRECT_EXCHANGE_CLASS).equals(type))
             {
                 _connection.sendConnectionClose(ErrorCodes.NOT_ALLOWED, "Attempt to redeclare default exchange: "
                                                                         + " of type "
@@ -2886,7 +2887,7 @@ public class AMQChannel extends AbstractAMQPSession<AMQChannel, ConsumerTarget_0
         // if we aren't given a queue name, we create one which we return to the client
         if ((queueStr == null) || (queueStr.length() == 0))
         {
-            queueName = new AMQShortString("tmp_" + UUID.randomUUID());
+            queueName = AMQShortString.createAMQShortString("tmp_" + UUID.randomUUID());
         }
         else
         {

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/83c6dfeb/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicContentHeaderProperties.java
----------------------------------------------------------------------
diff --git a/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicContentHeaderProperties.java b/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicContentHeaderProperties.java
index 9b18a1e..6d3a55e 100644
--- a/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicContentHeaderProperties.java
+++ b/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicContentHeaderProperties.java
@@ -790,7 +790,7 @@ public class BasicContentHeaderProperties
 
     public void setMessageId(String messageId)
     {
-        setMessageId(messageId == null ? null : new AMQShortString(messageId));
+        setMessageId(messageId == null ? null : AMQShortString.createAMQShortString(messageId));
     }
 
     public synchronized void setMessageId(AMQShortString messageId)

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/83c6dfeb/broker-plugins/amqp-0-8-protocol/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQPConnection_0_8Test.java
----------------------------------------------------------------------
diff --git a/broker-plugins/amqp-0-8-protocol/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQPConnection_0_8Test.java b/broker-plugins/amqp-0-8-protocol/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQPConnection_0_8Test.java
index 2631eaa..a677045 100644
--- a/broker-plugins/amqp-0-8-protocol/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQPConnection_0_8Test.java
+++ b/broker-plugins/amqp-0-8-protocol/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQPConnection_0_8Test.java
@@ -69,8 +69,8 @@ public class AMQPConnection_0_8Test extends QpidTestCase
 {
     private static final String VIRTUAL_HOST_NAME = "vhost";
     private static final byte[] SASL_RESPONSE = "response".getBytes();
-    private static final AMQShortString LOCALE = new AMQShortString("en_US");
-    private static final AMQShortString SASL_MECH = new AMQShortString("MECH");
+    private static final AMQShortString LOCALE = AMQShortString.createAMQShortString("en_US");
+    private static final AMQShortString SASL_MECH = AMQShortString.createAMQShortString("MECH");
 
     private TaskExecutorImpl _taskExecutor;
     private Broker _broker;
@@ -216,7 +216,7 @@ public class AMQPConnection_0_8Test extends QpidTestCase
         conn.receiveConnectionStartOk(new FieldTable(), SASL_MECH, SASL_RESPONSE, LOCALE);
         int maxChannels = 10;
         conn.receiveConnectionTuneOk(maxChannels, 65535, 0);
-        conn.receiveConnectionOpen(new AMQShortString(VIRTUAL_HOST_NAME), AMQShortString.EMPTY_STRING, false);
+        conn.receiveConnectionOpen(AMQShortString.createAMQShortString(VIRTUAL_HOST_NAME), AMQShortString.EMPTY_STRING, false);
 
         // check the channel count is correct
         int channelCount = conn.getSessionModels().size();

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/83c6dfeb/broker-plugins/amqp-0-8-protocol/src/test/java/org/apache/qpid/server/protocol/v0_8/transport/MessagePublishInfoImplTest.java
----------------------------------------------------------------------
diff --git a/broker-plugins/amqp-0-8-protocol/src/test/java/org/apache/qpid/server/protocol/v0_8/transport/MessagePublishInfoImplTest.java b/broker-plugins/amqp-0-8-protocol/src/test/java/org/apache/qpid/server/protocol/v0_8/transport/MessagePublishInfoImplTest.java
index fa14fc1..0012600 100644
--- a/broker-plugins/amqp-0-8-protocol/src/test/java/org/apache/qpid/server/protocol/v0_8/transport/MessagePublishInfoImplTest.java
+++ b/broker-plugins/amqp-0-8-protocol/src/test/java/org/apache/qpid/server/protocol/v0_8/transport/MessagePublishInfoImplTest.java
@@ -26,8 +26,8 @@ import org.apache.qpid.test.utils.QpidTestCase;
 public class MessagePublishInfoImplTest extends QpidTestCase
 {
     private MessagePublishInfo _mpi;
-    private final AMQShortString _exchange = new AMQShortString("exchange");
-    private final AMQShortString _routingKey = new AMQShortString("routingKey");
+    private final AMQShortString _exchange = AMQShortString.createAMQShortString("exchange");
+    private final AMQShortString _routingKey = AMQShortString.createAMQShortString("routingKey");
 
     @Override
     public void setUp() throws Exception
@@ -40,7 +40,7 @@ public class MessagePublishInfoImplTest extends QpidTestCase
     public void testExchange()
     {
         assertEquals(_exchange, _mpi.getExchange());
-        AMQShortString newExchange = new AMQShortString("newExchange");
+        AMQShortString newExchange = AMQShortString.createAMQShortString("newExchange");
         //Check we can update the exchange
         _mpi.setExchange(newExchange);
         assertEquals(newExchange, _mpi.getExchange());
@@ -88,7 +88,7 @@ public class MessagePublishInfoImplTest extends QpidTestCase
     public void testRoutingKey()
     {
         assertEquals(_routingKey, _mpi.getRoutingKey());
-        AMQShortString newRoutingKey = new AMQShortString("newRoutingKey");
+        AMQShortString newRoutingKey = AMQShortString.createAMQShortString("newRoutingKey");
 
         //Check we can update the routingKey
         _mpi.setRoutingKey(newRoutingKey);

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/83c6dfeb/broker-plugins/amqp-msg-conv-0-8-to-0-10/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v0_10/MessageConverter_0_10_to_0_8.java
----------------------------------------------------------------------
diff --git a/broker-plugins/amqp-msg-conv-0-8-to-0-10/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v0_10/MessageConverter_0_10_to_0_8.java b/broker-plugins/amqp-msg-conv-0-8-to-0-10/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v0_10/MessageConverter_0_10_to_0_8.java
index 7a2c82c..ab23155 100644
--- a/broker-plugins/amqp-msg-conv-0-8-to-0-10/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v0_10/MessageConverter_0_10_to_0_8.java
+++ b/broker-plugins/amqp-msg-conv-0-8-to-0-10/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v0_10/MessageConverter_0_10_to_0_8.java
@@ -99,7 +99,7 @@ public class MessageConverter_0_10_to_0_8 implements MessageConverter<MessageTra
             {
                 try
                 {
-                    props.setAppId(new AMQShortString(messageProps.getAppId()));
+                    props.setAppId(AMQShortString.createAMQShortString(messageProps.getAppId()));
                 }
                 catch (IllegalArgumentException e)
                 {
@@ -114,7 +114,7 @@ public class MessageConverter_0_10_to_0_8 implements MessageConverter<MessageTra
             {
                 try
                 {
-                    props.setCorrelationId(new AMQShortString(messageProps.getCorrelationId()));
+                    props.setCorrelationId(AMQShortString.createAMQShortString(messageProps.getCorrelationId()));
                 }
                 catch (IllegalArgumentException e)
                 {
@@ -165,7 +165,7 @@ public class MessageConverter_0_10_to_0_8 implements MessageConverter<MessageTra
             {
                 try
                 {
-                    props.setUserId(new AMQShortString(messageProps.getUserId()));
+                    props.setUserId(AMQShortString.createAMQShortString(messageProps.getUserId()));
                 }
                 catch (IllegalArgumentException e)
                 {
@@ -319,10 +319,10 @@ public class MessageConverter_0_10_to_0_8 implements MessageConverter<MessageTra
         DeliveryProperties delvProps = message.getHeader().getDeliveryProperties();
         final AMQShortString exchangeName = (delvProps == null || delvProps.getExchange() == null)
                                             ? null
-                                            : new AMQShortString(delvProps.getExchange());
+                                            : AMQShortString.createAMQShortString(delvProps.getExchange());
         final AMQShortString routingKey = (delvProps == null || delvProps.getRoutingKey() == null)
                                           ? null
-                                          : new AMQShortString(delvProps.getRoutingKey());
+                                          : AMQShortString.createAMQShortString(delvProps.getRoutingKey());
         final boolean immediate = delvProps != null && delvProps.getImmediate();
         final boolean mandatory = delvProps != null && !delvProps.getDiscardUnroutable();
 

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/83c6dfeb/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_1_0_to_v0_8.java
----------------------------------------------------------------------
diff --git a/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_1_0_to_v0_8.java b/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_1_0_to_v0_8.java
index e8a02fd..0576b33 100644
--- a/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_1_0_to_v0_8.java
+++ b/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_1_0_to_v0_8.java
@@ -296,7 +296,7 @@ public class MessageConverter_1_0_to_v0_8 implements MessageConverter<Message_1_
         {
             try
             {
-                return new AMQShortString(userId.getArray());
+                return AMQShortString.createAMQShortString(userId.getArray());
             }
             catch (IllegalArgumentException e)
             {
@@ -313,11 +313,11 @@ public class MessageConverter_1_0_to_v0_8 implements MessageConverter<Message_1_
         {
             if (messageId instanceof Binary)
             {
-                return new AMQShortString(((Binary) messageId).getArray());
+                return AMQShortString.createAMQShortString(((Binary) messageId).getArray());
             }
             else if (messageId instanceof byte[])
             {
-                return new AMQShortString(((byte[]) messageId));
+                return AMQShortString.createAMQShortString(((byte[]) messageId));
             }
             else
             {
@@ -388,11 +388,11 @@ public class MessageConverter_1_0_to_v0_8 implements MessageConverter<Message_1_
         {
             if (correlationIdObject instanceof Binary)
             {
-                correlationId = new AMQShortString(((Binary) correlationIdObject).getArray());
+                correlationId = AMQShortString.createAMQShortString(((Binary) correlationIdObject).getArray());
             }
             else if (correlationIdObject instanceof byte[])
             {
-                correlationId = new AMQShortString(((byte[]) correlationIdObject));
+                correlationId = AMQShortString.createAMQShortString(((byte[]) correlationIdObject));
             }
             else
             {


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org


[6/6] qpid-broker-j git commit: QPID-8182: Fix typo and remove not-addressed comment

Posted by or...@apache.org.
QPID-8182: Fix typo and remove not-addressed comment

(cherry picked from commit 8b5577b9d7b4224a03d08ba2240153bd981fb0ab)


Project: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/commit/dc2fa863
Tree: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/tree/dc2fa863
Diff: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/diff/dc2fa863

Branch: refs/heads/7.0.x
Commit: dc2fa863db9cee855d0d66abf323378ad67bb9c6
Parents: f4f9859
Author: Alex Rudyy <or...@apache.org>
Authored: Fri May 11 13:53:42 2018 +0100
Committer: Alex Rudyy <or...@apache.org>
Committed: Fri May 11 17:30:23 2018 +0100

----------------------------------------------------------------------
 .../converter/v0_10_v1_0/MessageConverter_1_0_to_v0_10.java        | 1 -
 .../converter/v0_8_v1_0/PropertyConverter_0_8_to_1_0Test.java      | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/dc2fa863/broker-plugins/amqp-msg-conv-0-10-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_10_v1_0/MessageConverter_1_0_to_v0_10.java
----------------------------------------------------------------------
diff --git a/broker-plugins/amqp-msg-conv-0-10-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_10_v1_0/MessageConverter_1_0_to_v0_10.java b/broker-plugins/amqp-msg-conv-0-10-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_10_v1_0/MessageConverter_1_0_to_v0_10.java
index 3f6837b..07aeb95 100644
--- a/broker-plugins/amqp-msg-conv-0-10-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_10_v1_0/MessageConverter_1_0_to_v0_10.java
+++ b/broker-plugins/amqp-msg-conv-0-10-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_10_v1_0/MessageConverter_1_0_to_v0_10.java
@@ -418,7 +418,6 @@ public class MessageConverter_1_0_to_v0_10 implements MessageConverter<Message_1
         {
             UUID uuid = (UUID)correlationIdObject;
             correlationId = longToBytes(uuid.getMostSignificantBits(), uuid.getLeastSignificantBits());
-            // KW: perhaps this would be more useful as the bytes of the UUID expressed as a string?
         }
         else if (correlationIdObject instanceof UnsignedLong)
         {

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/dc2fa863/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/test/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/PropertyConverter_0_8_to_1_0Test.java
----------------------------------------------------------------------
diff --git a/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/test/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/PropertyConverter_0_8_to_1_0Test.java b/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/test/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/PropertyConverter_0_8_to_1_0Test.java
index cae59f0..1bbed5a 100644
--- a/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/test/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/PropertyConverter_0_8_to_1_0Test.java
+++ b/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/test/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/PropertyConverter_0_8_to_1_0Test.java
@@ -296,7 +296,7 @@ public class PropertyConverter_0_8_to_1_0Test extends QpidTestCase
         assertEquals("Unexpected messageId", messageId, properties.getMessageId());
     }
 
-    public void testMessageUUiddConversion()
+    public void testMessageUuidConversion()
     {
         BasicContentHeaderProperties basicContentHeaderProperties = new BasicContentHeaderProperties();
         final UUID messageId = UUID.randomUUID();


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org


[2/6] qpid-broker-j git commit: QPID-7830: [Broker-J] [AMQP 0-8..0-91] Cache AMQPShortStrings that relate to exchanges/routing keys and header values (that are usually drawn from a small domain) in a time/size bound cache.

Posted by or...@apache.org.
QPID-7830: [Broker-J] [AMQP 0-8..0-91] Cache AMQPShortStrings that relate to exchanges/routing keys and header values (that are usually drawn from a small domain) in a time/size bound cache.

Intent is to reduce the amount of tenured garbage produced when messages are repeatedly sent to same destination. This should reduce frequency and length of GC pauses.

(cherry picked from commit 7d7b50824ad9f2a99b7de034ef36a529129b00ac)


Project: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/commit/995d535b
Tree: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/tree/995d535b
Diff: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/diff/995d535b

Branch: refs/heads/7.0.x
Commit: 995d535bdea6fb5efcab9c165dbd9c807f94e72e
Parents: 83c6dfe
Author: Keith Wall <kw...@apache.org>
Authored: Thu Apr 26 13:11:45 2018 +0100
Committer: Alex Rudyy <or...@apache.org>
Committed: Fri May 11 16:08:11 2018 +0100

----------------------------------------------------------------------
 .../server/protocol/v0_8/AMQShortString.java    | 162 ++++++++-----------
 .../protocol/v0_8/AMQShortStringTest.java       |  26 +--
 .../server/protocol/v0_8/MessageMetaData.java   |   8 +
 .../transport/BasicContentHeaderProperties.java | 162 +++----------------
 .../v0_8/transport/BasicPublishBody.java        |   8 +
 5 files changed, 117 insertions(+), 249 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/995d535b/broker-core/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQShortString.java
----------------------------------------------------------------------
diff --git a/broker-core/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQShortString.java b/broker-core/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQShortString.java
index 41a4322..0ee98c3 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQShortString.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQShortString.java
@@ -24,7 +24,10 @@ package org.apache.qpid.server.protocol.v0_8;
 import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
+import java.util.concurrent.TimeUnit;
 
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -45,15 +48,21 @@ public final class AMQShortString implements Comparable<AMQShortString>
 
     private static final Logger LOGGER = LoggerFactory.getLogger(AMQShortString.class);
 
+    // Unfortunately CacheBuilder does not yet support keyEquivalence, so we have to wrap the keys in ByteBuffers
+    // rather than using the byte arrays as keys.
+    private static ThreadLocal<Cache<ByteBuffer, AMQShortString>> CACHE =
+            ThreadLocal.withInitial(() -> CacheBuilder.newBuilder()
+                                                      .maximumSize(100)
+                                                      .expireAfterAccess(300, TimeUnit.SECONDS)
+                                                      .build());
+
     private final byte[] _data;
-    private final int _offset;
     private int _hashCode;
     private String _asString = null;
 
-    private final int _length;
-
     public static final AMQShortString EMPTY_STRING = createAMQShortString((String)null);
 
+
     private AMQShortString(byte[] data)
     {
         if (data == null)
@@ -64,48 +73,6 @@ public final class AMQShortString implements Comparable<AMQShortString>
         {
             throw new IllegalArgumentException("Cannot create AMQShortString with number of octets over 255!");
         }
-        _data = data.clone();
-        _length = data.length;
-        _offset = 0;
-    }
-
-    private AMQShortString(String string)
-    {
-        final byte[] data = EncodingUtils.asUTF8Bytes(string);
-        final int length = data.length;
-        if (data.length> MAX_LENGTH)
-        {
-            throw new IllegalArgumentException("Cannot create AMQShortString with number of octets over 255!");
-        }
-
-        int hash = 0;
-        for (int i = 0; i < length; i++)
-        {
-            data[i] = (byte) (0xFF & data[i]);
-            hash = (31 * hash) + data[i];
-        }
-        _hashCode = hash;
-        _data = data;
-
-        _length = length;
-        _offset = 0;
-
-        _asString = string == null ? "" : string;
-    }
-
-    private AMQShortString(byte[] data, final int offset, final int length)
-    {
-        if (length > MAX_LENGTH)
-        {
-            throw new IllegalArgumentException("Cannot create AMQShortString with number of octets over 255!");
-        }
-        if (data == null)
-        {
-            throw new NullPointerException("Cannot create AMQShortString with null data[]");
-        }
-
-        _offset = offset;
-        _length = length;
         _data = data;
     }
 
@@ -132,18 +99,46 @@ public final class AMQShortString implements Comparable<AMQShortString>
             }
             byte[] data = new byte[length];
             buffer.get(data);
-            return new AMQShortString(data, 0, length);
+
+            final AMQShortString cached = CACHE.get().getIfPresent(ByteBuffer.wrap(data));
+            return cached != null ? cached : new AMQShortString(data);
         }
     }
 
     public static AMQShortString createAMQShortString(byte[] data)
     {
-        return new AMQShortString(data);
+        if (data == null)
+        {
+            throw new NullPointerException("Cannot create AMQShortString with null data[]");
+        }
+
+        final AMQShortString cached = CACHE.get().getIfPresent(ByteBuffer.wrap(data));
+        return cached != null ? cached : new AMQShortString(data);
     }
 
     public static AMQShortString createAMQShortString(String string)
     {
-        return new AMQShortString(string);
+        final byte[] data = EncodingUtils.asUTF8Bytes(string);
+
+        final AMQShortString cached = CACHE.get().getIfPresent(ByteBuffer.wrap(data));
+        if (cached != null)
+        {
+            return cached;
+        }
+        else
+        {
+            final AMQShortString shortString = new AMQShortString(data);
+
+            int hash = 0;
+            for (int i = 0; i < data.length; i++)
+            {
+                data[i] = (byte) (0xFF & data[i]);
+                hash = (31 * hash) + data[i];
+            }
+            shortString._hashCode = hash;
+            shortString._asString = string;
+            return  shortString;
+        }
     }
 
     /**
@@ -152,35 +147,25 @@ public final class AMQShortString implements Comparable<AMQShortString>
      */
     public int length()
     {
-        return _length;
+        return _data.length;
     }
 
     public char charAt(int index)
     {
-
-        return (char) _data[_offset + index];
+        return (char) _data[index];
 
     }
 
     public byte[] getBytes()
     {
-        if(_offset == 0 && _length == _data.length)
-        {
-            return _data.clone();
-        }
-        else
-        {
-            byte[] data = new byte[_length];
-            System.arraycopy(_data,_offset,data,0,_length);
-            return data;
-        }
+        return _data.clone();
     }
 
     public void writeToBuffer(QpidByteBuffer buffer)
     {
-        final int size = length();
-        buffer.put((byte)size);
-        buffer.put(_data, _offset, size);
+        final short size = (short) length();
+        buffer.putUnsignedByte(size);
+        buffer.put(_data, 0, size);
     }
 
 
@@ -223,40 +208,14 @@ public final class AMQShortString implements Comparable<AMQShortString>
             return false;
         }
 
-        final int length = _length;
+        final int length = _data.length;
 
-        if(length != otherString._length)
+        if(length != otherString._data.length)
         {
             return false;
         }
 
-
-        final byte[] data = _data;
-
-        final byte[] otherData = otherString._data;
-
-        final int offset = _offset;
-
-        final int otherOffset = otherString._offset;
-
-        if(offset == 0 && otherOffset == 0 && length == data.length && length == otherData.length)
-        {
-            return Arrays.equals(data, otherData);
-        }
-        else
-        {
-            int thisIdx = offset;
-            int otherIdx = otherOffset;
-            for(int i = length;  i-- != 0; )
-            {
-                if(!(data[thisIdx++] == otherData[otherIdx++]))
-                {
-                    return false;
-                }
-            }
-        }
-
-        return true;
+        return Arrays.equals(_data, otherString._data);
 
     }
 
@@ -270,7 +229,7 @@ public final class AMQShortString implements Comparable<AMQShortString>
 
             for (int i = 0; i < size; i++)
             {
-                hash = (31 * hash) + _data[i+_offset];
+                hash = (31 * hash) + _data[i];
             }
 
             _hashCode = hash;
@@ -284,7 +243,7 @@ public final class AMQShortString implements Comparable<AMQShortString>
     {
         if (_asString == null)
         {
-            _asString = new String(_data, _offset, _length, StandardCharsets.UTF_8);
+            _asString = new String(_data, StandardCharsets.UTF_8);
         }
         return _asString;
     }
@@ -310,8 +269,8 @@ public final class AMQShortString implements Comparable<AMQShortString>
 
             for (int i = 0; i < length(); i++)
             {
-                final byte d = _data[i+_offset];
-                final byte n = name._data[i+name._offset];
+                final byte d = _data[i];
+                final byte n = name._data[i];
                 if (d < n)
                 {
                     return -1;
@@ -329,8 +288,8 @@ public final class AMQShortString implements Comparable<AMQShortString>
 
     public boolean contains(final byte b)
     {
-        final int end = _length + _offset;
-        for(int i = _offset; i < end; i++)
+        final int end = _data.length;
+        for(int i = 0; i < end; i++)
         {
             if(_data[i] == b)
             {
@@ -340,6 +299,11 @@ public final class AMQShortString implements Comparable<AMQShortString>
         return false;
     }
 
+    public void intern()
+    {
+        CACHE.get().put(ByteBuffer.wrap(_data), this);
+    }
+
     public static AMQShortString validValueOf(Object obj)
     {
         return valueOf(obj,true,true);

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/995d535b/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQShortStringTest.java
----------------------------------------------------------------------
diff --git a/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQShortStringTest.java b/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQShortStringTest.java
index 659d3e8..8ff7c96 100644
--- a/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQShortStringTest.java
+++ b/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQShortStringTest.java
@@ -27,12 +27,7 @@ import java.nio.charset.StandardCharsets;
 public class AMQShortStringTest extends QpidTestCase
 {
 
-    public static final AMQShortString HELLO = AMQShortString.createAMQShortString("Hello");
-    public static final AMQShortString HELL = AMQShortString.createAMQShortString("Hell");
-    public static final AMQShortString GOODBYE = AMQShortString.createAMQShortString("Goodbye");
-    public static final AMQShortString GOOD = AMQShortString.createAMQShortString("Good");
-    public static final AMQShortString BYE = AMQShortString.createAMQShortString("BYE");
-
+    private static final AMQShortString GOODBYE = AMQShortString.createAMQShortString("Goodbye");
 
     public void testEquals()
     {
@@ -57,9 +52,6 @@ public class AMQShortStringTest extends QpidTestCase
     }
 
     /**
-     * Test method for
-     * {@link AMQShortString#AMQShortString(java.lang.String)}
-     * <p>
      * Tests short string construction from string with length less than 255.
      */
     public void testCreateAMQShortStringString()
@@ -94,9 +86,6 @@ public class AMQShortStringTest extends QpidTestCase
     }
 
     /**
-     * Test method for
-     * {@link AMQShortString#AMQShortString(java.lang.String)}
-     * <p>
      * Tests an attempt to create an AMQP short string from string with length over 255
      */
     public void testCreateAMQShortStringStringOver255()
@@ -141,6 +130,19 @@ public class AMQShortStringTest extends QpidTestCase
         assertEquals("Unexpected null string from valueOf", null, shortString);
     }
 
+    public void testInterning()
+    {
+        AMQShortString str1 = AMQShortString.createAMQShortString("hello");
+        str1.intern();
+        AMQShortString str2 = AMQShortString.createAMQShortString("hello");
+        AMQShortString str3 = AMQShortString.createAMQShortString("hello".getBytes(StandardCharsets.UTF_8));
+
+        assertEquals(str1, str2);
+        assertEquals(str1, str3);
+        assertSame(str1, str2);
+        assertSame(str1, str3);
+    }
+
     /**
      * A helper method to generate a string with given length containing given
      * character

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/995d535b/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/MessageMetaData.java
----------------------------------------------------------------------
diff --git a/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/MessageMetaData.java b/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/MessageMetaData.java
index b182c8d..2ad8511 100644
--- a/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/MessageMetaData.java
+++ b/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/MessageMetaData.java
@@ -164,7 +164,15 @@ public class MessageMetaData implements StorableMessageMetaData
 
                 ContentHeaderBody chb = ContentHeaderBody.createFromBuffer(buf, size);
                 final AMQShortString exchange = AMQShortString.readAMQShortString(buf);
+                if (exchange != null)
+                {
+                    exchange.intern();
+                }
                 final AMQShortString routingKey = AMQShortString.readAMQShortString(buf);
+                if (routingKey != null)
+                {
+                    routingKey.intern();
+                }
 
                 final byte flags = buf.get();
                 long arrivalTime = buf.getLong();

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/995d535b/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicContentHeaderProperties.java
----------------------------------------------------------------------
diff --git a/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicContentHeaderProperties.java b/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicContentHeaderProperties.java
index 6d3a55e..52183fc 100644
--- a/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicContentHeaderProperties.java
+++ b/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicContentHeaderProperties.java
@@ -322,144 +322,6 @@ public class BasicContentHeaderProperties
         }
     }
 
-    public int read(QpidByteBuffer input)
-    {
-
-        _propertyFlags = input.getUnsignedShort();
-        int length = 2;
-        if ((_propertyFlags & (CONTENT_TYPE_MASK)) != 0)
-        {
-            length++;
-            _contentType = AMQShortString.readAMQShortString(input);
-            if(_contentType != null)
-            {
-                length += _contentType.length();
-            }
-        }
-
-        if ((_propertyFlags & ENCODING_MASK) != 0)
-        {
-            length++;
-            _encoding = AMQShortString.readAMQShortString(input);
-            if(_encoding != null)
-            {
-                length += _encoding.length();
-            }
-        }
-
-        if ((_propertyFlags & HEADERS_MASK) != 0)
-        {
-            int fieldTableLength = input.getInt();
-
-            _headers = new FieldTable(input, fieldTableLength);
-
-            length += 4;
-            length += fieldTableLength;
-        }
-
-        if ((_propertyFlags & DELIVERY_MODE_MASK) != 0)
-        {
-            _deliveryMode = input.get();
-            length++;
-        }
-
-        if ((_propertyFlags & PRIORITY_MASK) != 0)
-        {
-            _priority = input.get();
-            length++;
-        }
-
-        if ((_propertyFlags & CORRELATION_ID_MASK) != 0)
-        {
-            length++;
-            _correlationId = AMQShortString.readAMQShortString(input);
-            if(_correlationId != null)
-            {
-                length += _correlationId.length();
-            }
-        }
-
-        if ((_propertyFlags & REPLY_TO_MASK) != 0)
-        {
-            length++;
-            _replyTo = AMQShortString.readAMQShortString(input);
-            if(_replyTo != null)
-            {
-                length += _replyTo.length();
-            }
-        }
-
-        if ((_propertyFlags & EXPIRATION_MASK) != 0)
-        {
-            length++;
-            AMQShortString expiration = AMQShortString.readAMQShortString(input);
-            if(expiration != null)
-            {
-                length += expiration.length();
-                _expiration = Long.parseLong(expiration.toString());
-            }
-        }
-
-        if ((_propertyFlags & MESSAGE_ID_MASK) != 0)
-        {
-            length++;
-            _messageId = AMQShortString.readAMQShortString(input);
-            if(_messageId != null)
-            {
-                length += _messageId.length();
-            }
-        }
-
-        if ((_propertyFlags & TIMESTAMP_MASK) != 0)
-        {
-            _timestamp = input.getLong();
-            length += 8;
-        }
-
-        if ((_propertyFlags & TYPE_MASK) != 0)
-        {
-            length++;
-            _type = AMQShortString.readAMQShortString(input);
-            if(_type != null)
-            {
-                length += _type.length();
-            }
-        }
-
-        if ((_propertyFlags & USER_ID_MASK) != 0)
-        {
-            length++;
-            _userId = AMQShortString.readAMQShortString(input);
-            if(_userId != null)
-            {
-                length += _userId.length();
-            }
-        }
-
-        if ((_propertyFlags & APPLICATION_ID_MASK) != 0)
-        {
-            length++;
-            _appId = AMQShortString.readAMQShortString(input);
-            if(_appId != null)
-            {
-                length += _appId.length();
-            }
-        }
-
-        if ((_propertyFlags & CLUSTER_ID_MASK) != 0)
-        {
-            length++;
-            _clusterId = AMQShortString.readAMQShortString(input);
-            if(_clusterId != null)
-            {
-                length += _clusterId.length();
-            }
-        }
-
-        return length;
-    }
-
-
     public synchronized long writePropertyListPayload(final ByteBufferSender sender)
     {
         if(useEncodedForm())
@@ -512,11 +374,19 @@ public class BasicContentHeaderProperties
         if ((_propertyFlags & (CONTENT_TYPE_MASK)) != 0)
         {
             _contentType = AMQShortString.readAMQShortString(buffer);
+            if (_contentType != null)
+            {
+                _contentType.intern();
+            }
         }
 
         if ((_propertyFlags & ENCODING_MASK) != 0)
         {
             _encoding = AMQShortString.readAMQShortString(buffer);
+            if (_encoding != null)
+            {
+                _encoding.intern();
+            }
         }
 
         if ((_propertyFlags & HEADERS_MASK) != 0)
@@ -548,6 +418,10 @@ public class BasicContentHeaderProperties
         if ((_propertyFlags & REPLY_TO_MASK) != 0)
         {
             _replyTo = AMQShortString.readAMQShortString(buffer);
+            if (_replyTo != null)
+            {
+                _replyTo.intern();
+            }
         }
 
         if ((_propertyFlags & EXPIRATION_MASK) != 0)
@@ -573,16 +447,28 @@ public class BasicContentHeaderProperties
         if ((_propertyFlags & USER_ID_MASK) != 0)
         {
             _userId = AMQShortString.readAMQShortString(buffer);
+            if (_userId != null)
+            {
+                _userId.intern();
+            }
         }
 
         if ((_propertyFlags & APPLICATION_ID_MASK) != 0)
         {
             _appId = AMQShortString.readAMQShortString(buffer);
+            if (_appId != null)
+            {
+                _appId.intern();
+            }
         }
 
         if ((_propertyFlags & CLUSTER_ID_MASK) != 0)
         {
             _clusterId = AMQShortString.readAMQShortString(buffer);
+            if (_clusterId != null)
+            {
+                _clusterId.intern();
+            }
         }
 
     }

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/995d535b/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicPublishBody.java
----------------------------------------------------------------------
diff --git a/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicPublishBody.java b/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicPublishBody.java
index 9821383..a8243ea 100644
--- a/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicPublishBody.java
+++ b/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicPublishBody.java
@@ -152,7 +152,15 @@ public class BasicPublishBody extends AMQMethodBodyImpl implements EncodableAMQD
 
         int ticket = buffer.getUnsignedShort();
         AMQShortString exchange = AMQShortString.readAMQShortString(buffer);
+        if (exchange != null)
+        {
+            exchange.intern();
+        }
         AMQShortString routingKey = AMQShortString.readAMQShortString(buffer);
+        if (routingKey != null)
+        {
+            routingKey.intern();
+        }
         byte bitfield = buffer.get();
 
         boolean mandatory = (bitfield & 0x01) != 0;


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org


[5/6] qpid-broker-j git commit: QPID-8182: Ensure messageId fidelity during conversion where possible

Posted by or...@apache.org.
QPID-8182: Ensure messageId fidelity during conversion where possible

(cherry picked from commit 2696d7a542a8927d48b43ce48a1d8104d7b8dc4c)


Project: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/commit/f4f9859b
Tree: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/tree/f4f9859b
Diff: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/diff/f4f9859b

Branch: refs/heads/7.0.x
Commit: f4f9859b92d3314bf54454e5192d9a8d53b4ae86
Parents: 40df817
Author: Keith Wall <kw...@apache.org>
Authored: Wed May 9 11:54:59 2018 +0100
Committer: Alex Rudyy <or...@apache.org>
Committed: Fri May 11 17:30:23 2018 +0100

----------------------------------------------------------------------
 .../MessageConverter_1_0_to_v0_10.java          | 10 +++-
 .../PropertyConverter_1_0_to_0_10Test.java      | 28 +++++++++
 .../v0_8_v1_0/MessageConverter_0_8_to_1_0.java  | 22 ++++++-
 .../v0_8_v1_0/MessageConverter_1_0_to_v0_8.java | 61 ++++++++++----------
 .../PropertyConverter_0_8_to_1_0Test.java       | 27 +++++++++
 .../SimpleConversionTest.java                   | 26 +++------
 6 files changed, 120 insertions(+), 54 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/f4f9859b/broker-plugins/amqp-msg-conv-0-10-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_10_v1_0/MessageConverter_1_0_to_v0_10.java
----------------------------------------------------------------------
diff --git a/broker-plugins/amqp-msg-conv-0-10-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_10_v1_0/MessageConverter_1_0_to_v0_10.java b/broker-plugins/amqp-msg-conv-0-10-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_10_v1_0/MessageConverter_1_0_to_v0_10.java
index 50a9484..3f6837b 100644
--- a/broker-plugins/amqp-msg-conv-0-10-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_10_v1_0/MessageConverter_1_0_to_v0_10.java
+++ b/broker-plugins/amqp-msg-conv-0-10-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_10_v1_0/MessageConverter_1_0_to_v0_10.java
@@ -366,13 +366,18 @@ public class MessageConverter_1_0_to_v0_10 implements MessageConverter<Message_1
         }
         else if (messageId instanceof String)
         {
+            String messageIdString = (String) messageId;
             try
             {
-                return UUID.fromString(((String) messageId));
+                if (messageIdString.startsWith("ID:"))
+                {
+                    messageIdString = messageIdString.substring(3);
+                }
+                return UUID.fromString(messageIdString);
             }
             catch (IllegalArgumentException e)
             {
-                return UUID.nameUUIDFromBytes(((String) messageId).getBytes(UTF_8));
+                return UUID.nameUUIDFromBytes(messageIdString.getBytes(UTF_8));
             }
         }
         else if (messageId instanceof Binary)
@@ -413,6 +418,7 @@ public class MessageConverter_1_0_to_v0_10 implements MessageConverter<Message_1
         {
             UUID uuid = (UUID)correlationIdObject;
             correlationId = longToBytes(uuid.getMostSignificantBits(), uuid.getLeastSignificantBits());
+            // KW: perhaps this would be more useful as the bytes of the UUID expressed as a string?
         }
         else if (correlationIdObject instanceof UnsignedLong)
         {

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/f4f9859b/broker-plugins/amqp-msg-conv-0-10-to-1-0/src/test/java/org/apache/qpid/server/protocol/converter/v0_10_v1_0/PropertyConverter_1_0_to_0_10Test.java
----------------------------------------------------------------------
diff --git a/broker-plugins/amqp-msg-conv-0-10-to-1-0/src/test/java/org/apache/qpid/server/protocol/converter/v0_10_v1_0/PropertyConverter_1_0_to_0_10Test.java b/broker-plugins/amqp-msg-conv-0-10-to-1-0/src/test/java/org/apache/qpid/server/protocol/converter/v0_10_v1_0/PropertyConverter_1_0_to_0_10Test.java
index fea4a40..f024f87 100644
--- a/broker-plugins/amqp-msg-conv-0-10-to-1-0/src/test/java/org/apache/qpid/server/protocol/converter/v0_10_v1_0/PropertyConverter_1_0_to_0_10Test.java
+++ b/broker-plugins/amqp-msg-conv-0-10-to-1-0/src/test/java/org/apache/qpid/server/protocol/converter/v0_10_v1_0/PropertyConverter_1_0_to_0_10Test.java
@@ -565,6 +565,34 @@ public class PropertyConverter_1_0_to_0_10Test extends QpidTestCase
         assertEquals("Unexpected messageId", messageId, messageProperties.getMessageId());
     }
 
+    public void testMessageIdStringifiedUUIDConversion()
+    {
+        final UUID messageId = UUID.randomUUID();
+        Properties properties = new Properties();
+        properties.setMessageId(messageId.toString());
+        Message_1_0 message = createTestMessage(properties);
+
+        final MessageTransferMessage convertedMessage = _messageConverter.convert(message, _namedAddressSpace);
+
+        final MessageProperties messageProperties =
+                convertedMessage.getStoredMessage().getMetaData().getMessageProperties();
+        assertEquals("Unexpected messageId", messageId, messageProperties.getMessageId());
+    }
+
+    public void testMessageIdPrefixedStringifiedUUIDConversion()
+    {
+        final UUID messageId = UUID.randomUUID();
+        Properties properties = new Properties();
+        properties.setMessageId("ID:" + messageId.toString());
+        Message_1_0 message = createTestMessage(properties);
+
+        final MessageTransferMessage convertedMessage = _messageConverter.convert(message, _namedAddressSpace);
+
+        final MessageProperties messageProperties =
+                convertedMessage.getStoredMessage().getMetaData().getMessageProperties();
+        assertEquals("Unexpected messageId", messageId, messageProperties.getMessageId());
+    }
+
     public void testMessageIdUnsignedLongConversion()
     {
         final UnsignedLong messageId = UnsignedLong.valueOf(-1);

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/f4f9859b/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_0_8_to_1_0.java
----------------------------------------------------------------------
diff --git a/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_0_8_to_1_0.java b/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_0_8_to_1_0.java
index 6839993..27b31b8 100644
--- a/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_0_8_to_1_0.java
+++ b/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_0_8_to_1_0.java
@@ -26,6 +26,7 @@ import java.util.Arrays;
 import java.util.Date;
 import java.util.LinkedHashMap;
 import java.util.Map;
+import java.util.UUID;
 
 import org.apache.qpid.server.plugin.PluggableService;
 import org.apache.qpid.server.protocol.converter.MessageConversionException;
@@ -94,7 +95,7 @@ public class MessageConverter_0_8_to_1_0 extends MessageConverter_to_1_0<AMQMess
             final String correlationIdAsString = contentHeader.getCorrelationIdAsString();
             if (Arrays.equals(correlationIdAsBytes, correlationIdAsString.getBytes(StandardCharsets.UTF_8)))
             {
-                props.setCorrelationId(correlationIdAsString);
+                props.setCorrelationId(convertMessageId(correlationId));
             }
             else
             {
@@ -105,7 +106,7 @@ public class MessageConverter_0_8_to_1_0 extends MessageConverter_to_1_0<AMQMess
         final AMQShortString messageId = contentHeader.getMessageId();
         if(messageId != null)
         {
-            props.setMessageId(messageId.toString());
+            props.setMessageId(convertMessageId(messageId));
         }
 
         if (contentHeader.getReplyTo() != null)
@@ -196,6 +197,23 @@ public class MessageConverter_0_8_to_1_0 extends MessageConverter_to_1_0<AMQMess
                                        bodySection.getEncodedSize());
     }
 
+    private Object convertMessageId(final AMQShortString messageId)
+    {
+        try
+        {
+            String messageIdAsString = messageId.toString();
+            if(messageIdAsString.startsWith("ID:"))
+            {
+                messageIdAsString = messageIdAsString.substring(3);
+            }
+            return UUID.fromString(messageIdAsString);
+        }
+        catch (IllegalArgumentException e)
+        {
+            return messageId.toString();
+        }
+    }
+
     private String convertReplyTo(final AMQShortString replyTo)
     {
         String convertedReplyTo;

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/f4f9859b/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_1_0_to_v0_8.java
----------------------------------------------------------------------
diff --git a/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_1_0_to_v0_8.java b/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_1_0_to_v0_8.java
index 0576b33..f9a2b08 100644
--- a/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_1_0_to_v0_8.java
+++ b/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_1_0_to_v0_8.java
@@ -308,28 +308,38 @@ public class MessageConverter_1_0_to_v0_8 implements MessageConverter<Message_1_
 
     private AMQShortString getMessageIdAsShortString(final Message_1_0 serverMsg)
     {
-        Object messageId = getMessageId(serverMsg);
         try
         {
-            if (messageId instanceof Binary)
-            {
-                return AMQShortString.createAMQShortString(((Binary) messageId).getArray());
-            }
-            else if (messageId instanceof byte[])
-            {
-                return AMQShortString.createAMQShortString(((byte[]) messageId));
-            }
-            else
-            {
-                return AMQShortString.valueOf(messageId);
-            }
+            Object messageId = getMessageId(serverMsg);
+            return covertMessageIdTo08MessageId(messageId);
         }
         catch (IllegalArgumentException e)
         {
-            // pass
+            return null;
         }
-        return null;
+    }
 
+    private AMQShortString covertMessageIdTo08MessageId(final Object messageId)
+    {
+        if (messageId == null)
+        {
+            return null;
+        }
+
+        final AMQShortString result;
+        if (messageId instanceof Binary)
+        {
+            result =  AMQShortString.createAMQShortString(((Binary) messageId).getArray());
+        }
+        else if (messageId instanceof byte[])
+        {
+            result = AMQShortString.createAMQShortString((byte[]) messageId);
+        }
+        else
+        {
+            result = AMQShortString.createAMQShortString(String.valueOf(messageId));
+        }
+        return result;
     }
 
     private AMQShortString getReplyTo(final Message_1_0 serverMsg, final NamedAddressSpace addressSpace)
@@ -382,28 +392,17 @@ public class MessageConverter_1_0_to_v0_8 implements MessageConverter<Message_1_
 
     private AMQShortString getCorrelationIdAsShortString(final Message_1_0 serverMsg)
     {
-        Object correlationIdObject = getCorrelationId(serverMsg);
-        final AMQShortString correlationId;
         try
         {
-            if (correlationIdObject instanceof Binary)
-            {
-                correlationId = AMQShortString.createAMQShortString(((Binary) correlationIdObject).getArray());
-            }
-            else if (correlationIdObject instanceof byte[])
-            {
-                correlationId = AMQShortString.createAMQShortString(((byte[]) correlationIdObject));
-            }
-            else
-            {
-                correlationId = AMQShortString.valueOf(correlationIdObject);
-            }
+            Object correlationIdObject = getCorrelationId(serverMsg);
+            return covertMessageIdTo08MessageId(correlationIdObject);
         }
         catch (IllegalArgumentException e)
         {
-            throw new MessageConversionException("Could not convert message from 1.0 to 0-8 because conversion of 'correlation-id' failed.", e);
+            throw new MessageConversionException(
+                    "Could not convert message from 1.0 to 0-8 because conversion of 'correlation-id' failed.",
+                    e);
         }
-        return correlationId;
     }
 
     private AMQShortString convertToShortStringForProperty(String propertyName, String s)

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/f4f9859b/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/test/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/PropertyConverter_0_8_to_1_0Test.java
----------------------------------------------------------------------
diff --git a/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/test/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/PropertyConverter_0_8_to_1_0Test.java b/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/test/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/PropertyConverter_0_8_to_1_0Test.java
index f9443e0..cae59f0 100644
--- a/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/test/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/PropertyConverter_0_8_to_1_0Test.java
+++ b/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/test/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/PropertyConverter_0_8_to_1_0Test.java
@@ -27,6 +27,7 @@ import static org.mockito.Mockito.when;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.UUID;
 
 import org.apache.qpid.server.bytebuffer.QpidByteBuffer;
 import org.apache.qpid.server.model.NamedAddressSpace;
@@ -180,6 +181,19 @@ public class PropertyConverter_0_8_to_1_0Test extends QpidTestCase
         assertEquals("Unexpected correlationId", correlationId, properties.getCorrelationId());
     }
 
+    public void testCorrelationUuidIdConversion()
+    {
+        BasicContentHeaderProperties basicContentHeaderProperties = new BasicContentHeaderProperties();
+        UUID correlationId = UUID.randomUUID();
+        basicContentHeaderProperties.setCorrelationId(AMQShortString.valueOf("ID:" + correlationId.toString()));
+        AMQMessage message = createTestMessage(basicContentHeaderProperties);
+
+        final Message_1_0 convertedMessage = _messageConverter.convert(message, _namedAddressSpace);
+
+        Properties properties = convertedMessage.getPropertiesSection().getValue();
+        assertEquals("Unexpected correlationId", correlationId, properties.getCorrelationId());
+    }
+
     public void testReplyToConversionWhenBindingURLFormatIsUsed()
     {
         BasicContentHeaderProperties basicContentHeaderProperties = new BasicContentHeaderProperties();
@@ -282,6 +296,19 @@ public class PropertyConverter_0_8_to_1_0Test extends QpidTestCase
         assertEquals("Unexpected messageId", messageId, properties.getMessageId());
     }
 
+    public void testMessageUUiddConversion()
+    {
+        BasicContentHeaderProperties basicContentHeaderProperties = new BasicContentHeaderProperties();
+        final UUID messageId = UUID.randomUUID();
+        basicContentHeaderProperties.setMessageId("ID:" + messageId.toString());
+        AMQMessage message = createTestMessage(basicContentHeaderProperties);
+
+        final Message_1_0 convertedMessage = _messageConverter.convert(message, _namedAddressSpace);
+
+        Properties properties = convertedMessage.getPropertiesSection().getValue();
+        assertEquals("Unexpected messageId", messageId, properties.getMessageId());
+    }
+
     public void testTimestampConversion()
     {
         BasicContentHeaderProperties basicContentHeaderProperties = new BasicContentHeaderProperties();

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/f4f9859b/systests/end-to-end-conversion-tests/src/test/java/org/apache/qpid/systests/end_to_end_conversion/SimpleConversionTest.java
----------------------------------------------------------------------
diff --git a/systests/end-to-end-conversion-tests/src/test/java/org/apache/qpid/systests/end_to_end_conversion/SimpleConversionTest.java b/systests/end-to-end-conversion-tests/src/test/java/org/apache/qpid/systests/end_to_end_conversion/SimpleConversionTest.java
index 2385cd2..34c11dd 100644
--- a/systests/end-to-end-conversion-tests/src/test/java/org/apache/qpid/systests/end_to_end_conversion/SimpleConversionTest.java
+++ b/systests/end-to-end-conversion-tests/src/test/java/org/apache/qpid/systests/end_to_end_conversion/SimpleConversionTest.java
@@ -41,7 +41,6 @@ import com.google.common.collect.Lists;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TestName;
@@ -216,10 +215,11 @@ public class SimpleConversionTest extends EndToEndConversionTestBase
         ClientMessage publishedMessage = clientResults.get(0);
         ClientMessage subscriberMessage = clientResults.get(1);
 
-        // TODO: On the wire the AMQP 1.0 client receives a string containing a message with
-        // message-id-string contain a ID: prefixed UUID. Would be better if the conversion layer sent a message-id-uuid
-        // as this would offer most compatibility and miminise the exposure of the ID prefix.
-        assertThat(subscriberMessage.getJMSMessageID(), equalTo(publishedMessage.getJMSMessageID()));
+        String publishedJmsMessageID = publishedMessage.getJMSMessageID();
+        assertThat(publishedJmsMessageID, startsWith("ID:"));
+        String barePublishedJmsMessageID = publishedJmsMessageID.substring("ID:".length());
+        String expectedSubscriberJmsMessageID = String.format("ID:AMQP_UUID:%s", barePublishedJmsMessageID);
+        assertThat(subscriberMessage.getJMSMessageID(), equalTo(expectedSubscriberJmsMessageID));
     }
 
     @Test
@@ -274,7 +274,6 @@ public class SimpleConversionTest extends EndToEndConversionTestBase
     }
 
     @Test
-    @Ignore("Currently subscriber receives the correct message id but without the ID prefix")
     public void providerAssignedMessageId_UuidMode_10_09() throws Exception
     {
         assumeTrue(EnumSet.of(Protocol.AMQP_1_0).contains(getPublisherProtocolVersion())
@@ -291,17 +290,11 @@ public class SimpleConversionTest extends EndToEndConversionTestBase
         final String publishedJmsMessageID = publishedMessage.getJMSMessageID();
         assertThat(publishedJmsMessageID, startsWith("ID:AMQP_UUID:"));
         String barePublishedJmsMessageID = publishedJmsMessageID.substring("ID:AMQP_UUID:".length());
-        String expectedSubscriberJmsMessageID = String.format("ID:%s", barePublishedJmsMessageID);
+        String expectedSubscriberJmsMessageID = String.format("%s", barePublishedJmsMessageID);
         assertThat(subscriberMessage.getJMSMessageID(), equalTo(expectedSubscriberJmsMessageID));
-
-
-        // TODO: On the wire the AMQP 0-x client receives a message id containing the a stringified UUID without prefix.
-        // This is inconsistent - in all other cases the client receives message ids prefixed.  Would be
-        // better if the conversion layer sent a synthesized the ID prefix.
     }
 
     @Test
-    @Ignore("Currently subscriber receives the correct message id but without the ID prefix")
     public void providerAssignedMessageId_UuidStringMode_10_09() throws Exception
     {
         assumeTrue(EnumSet.of(Protocol.AMQP_1_0).contains(getPublisherProtocolVersion())
@@ -317,10 +310,9 @@ public class SimpleConversionTest extends EndToEndConversionTestBase
         final String publishedJmsMessageID = publishedMessage.getJMSMessageID();
         assertThat(publishedJmsMessageID, startsWith("ID:AMQP_NO_PREFIX:"));
         String barePublishedJmsMessageID = publishedJmsMessageID.substring("ID:AMQP_NO_PREFIX:".length());
-        String expectedSubscriberJmsMessageID = String.format("ID:%s", barePublishedJmsMessageID);
+        String expectedSubscriberJmsMessageID = String.format("%s", barePublishedJmsMessageID);
         assertThat(subscriberMessage.getJMSMessageID(), equalTo(expectedSubscriberJmsMessageID));
 
-        // TODO ditto above
     }
 
     @Test
@@ -401,7 +393,6 @@ public class SimpleConversionTest extends EndToEndConversionTestBase
     }
 
     @Test
-    @Ignore("Currently subscriber receives a UUID that differs from the one sent")
     public void providerAssignedMessageId_PrefixedUuidStringMode_10_010() throws Exception
     {
         assumeTrue(EnumSet.of(Protocol.AMQP_1_0).contains(getPublisherProtocolVersion())
@@ -419,9 +410,6 @@ public class SimpleConversionTest extends EndToEndConversionTestBase
         String barePublishedJmsMessageID = publishedJmsMessageID.substring("ID:".length());
         String expectedSubscriberJmsMessageID = String.format("ID:%s", barePublishedJmsMessageID);
         assertThat(subscriberMessage.getJMSMessageID(), equalTo(expectedSubscriberJmsMessageID));
-
-        // TODO correct conversion layer so that a string that contains a ID prefixed UUID is converted
-        // as a UUID.
     }
 
     @Test


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org


[4/6] qpid-broker-j git commit: QPID-8182: [End to End Conversion Tests] Extend test mechanism to allow testing of JMS provider assigned message ids. Add test cases too.

Posted by or...@apache.org.
QPID-8182: [End to End Conversion Tests] Extend test mechanism to allow testing of JMS provider assigned message ids.  Add test cases too.

(cherry picked from commit d293206d72989f1004f8fa3577f36d2da104f615)


Project: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/commit/40df8179
Tree: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/tree/40df8179
Diff: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/diff/40df8179

Branch: refs/heads/7.0.x
Commit: 40df8179df216530caa418197175fdf8bba8a0a3
Parents: d79537d
Author: Keith Wall <kw...@apache.org>
Authored: Mon May 7 07:39:27 2018 +0100
Committer: Alex Rudyy <or...@apache.org>
Committed: Fri May 11 17:30:23 2018 +0100

----------------------------------------------------------------------
 .../ClientInstructionBuilder.java               |   7 +
 .../EndToEndConversionTestBase.java             |  67 +++--
 .../client/AugumentConnectionUrl.java           |  42 +++
 .../end_to_end_conversion/client/Client.java    | 162 ++++++++++-
 .../client/ClientMessage.java                   |  37 +++
 .../client/ClientResult.java                    |  18 +-
 .../SimpleConversionTest.java                   | 289 ++++++++++++++++++-
 7 files changed, 579 insertions(+), 43 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/40df8179/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/ClientInstructionBuilder.java
----------------------------------------------------------------------
diff --git a/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/ClientInstructionBuilder.java b/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/ClientInstructionBuilder.java
index 3559693..2d208f4 100644
--- a/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/ClientInstructionBuilder.java
+++ b/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/ClientInstructionBuilder.java
@@ -25,6 +25,7 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 
+import org.apache.qpid.systests.end_to_end_conversion.client.AugumentConnectionUrl;
 import org.apache.qpid.systests.end_to_end_conversion.client.ClientInstruction;
 import org.apache.qpid.systests.end_to_end_conversion.client.ConfigureDestination;
 import org.apache.qpid.systests.end_to_end_conversion.client.MessageDescription;
@@ -35,6 +36,12 @@ public class ClientInstructionBuilder
     private List<ClientInstruction> _clientInstructions = new ArrayList<>();
     private MessageDescription _latestMessageDescription;
 
+    public ClientInstructionBuilder configureConnectionUrl(final Map<String, String> connectionUrlConfig)
+    {
+        _clientInstructions.add(new AugumentConnectionUrl(connectionUrlConfig));
+        return this;
+    }
+
     public ClientInstructionBuilder publishMessage(final String destinationJndiName)
     {
         return publishMessage(destinationJndiName, new MessageDescription());

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/40df8179/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/EndToEndConversionTestBase.java
----------------------------------------------------------------------
diff --git a/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/EndToEndConversionTestBase.java b/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/EndToEndConversionTestBase.java
index 66e601a..1e52152 100644
--- a/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/EndToEndConversionTestBase.java
+++ b/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/EndToEndConversionTestBase.java
@@ -48,6 +48,7 @@ import org.slf4j.LoggerFactory;
 import org.slf4j.event.Level;
 
 import org.apache.qpid.server.model.Protocol;
+import org.apache.qpid.systests.end_to_end_conversion.client.AugumentConnectionUrl;
 import org.apache.qpid.systests.end_to_end_conversion.client.Client;
 import org.apache.qpid.systests.end_to_end_conversion.client.ClientInstruction;
 import org.apache.qpid.systests.end_to_end_conversion.client.ClientResult;
@@ -94,7 +95,7 @@ public class EndToEndConversionTestBase extends BrokerAdminUsingTestBase
         System.out.println("LQDEBUG: " + ClasspathQuery.getCacheStats());
     }
 
-    protected ListenableFuture<?> runPublisher(final List<ClientInstruction> clientInstructions)
+    protected ListenableFuture<ClientResult> runPublisher(final List<ClientInstruction> clientInstructions)
     {
         List<String> gavs = Arrays.asList(System.getProperty("qpid.systests.end_to_end_conversion.publisherGavs",
                                                              "org.apache.qpid:qpid-jms-client:LATEST")
@@ -105,11 +106,11 @@ public class EndToEndConversionTestBase extends BrokerAdminUsingTestBase
 
         return _executorService.submit(() -> {
             Thread.currentThread().setName("Publisher");
-            runClient(gavs, additionalJavaArgs, clientInstructions);
+            return runClient(gavs, additionalJavaArgs, clientInstructions);
         });
     }
 
-    protected ListenableFuture<?> runSubscriber(final List<ClientInstruction> clientInstructions)
+    protected ListenableFuture<ClientResult> runSubscriber(final List<ClientInstruction> clientInstructions)
     {
         List<String> gavs = Arrays.asList(System.getProperty("qpid.systests.end_to_end_conversion.subscriberGavs",
                                                              "org.apache.qpid:qpid-client:LATEST,org.apache.geronimo.specs:geronimo-jms_1.1_spec:1.1.1")
@@ -121,11 +122,11 @@ public class EndToEndConversionTestBase extends BrokerAdminUsingTestBase
 
         return _executorService.submit(() -> {
             Thread.currentThread().setName("Subscriber");
-            runClient(gavs, additionalJavaArgs, clientInstructions);
+            return runClient(gavs, additionalJavaArgs, clientInstructions);
         });
     }
 
-    private List<ClientInstruction> amendClientInstructions(final List<ClientInstruction> clientInstructions,
+    private List<ClientInstruction> amendClientInstructions(List<ClientInstruction> clientInstructions,
                                                             final boolean amqp0xClient)
     {
         if (clientInstructions.isEmpty())
@@ -146,18 +147,27 @@ public class EndToEndConversionTestBase extends BrokerAdminUsingTestBase
             }
         }
 
+        List<AugumentConnectionUrl> configUrls = clientInstructions.stream()
+                                                                   .filter(AugumentConnectionUrl.class::isInstance)
+                                                                   .map(AugumentConnectionUrl.class::cast)
+                                                                   .collect(Collectors.toList());
+
         final String contextFactory;
         final String connectionUrl;
         if (amqp0xClient)
         {
             contextFactory = getAmqp0xContextFactory();
-            connectionUrl = getAmqp0xConnectionUrl();
+            connectionUrl = getAmqp0xConnectionUrl(configUrls);
         }
         else
         {
             contextFactory = getAmqp10ContextFactory();
-            connectionUrl = getAmqp10ConnectionUrl();
+            connectionUrl = getAmqp10ConnectionUrl(configUrls);
         }
+
+        clientInstructions = new ArrayList<>(clientInstructions);
+        clientInstructions.removeAll(configUrls);
+
         ConfigureJndiContext jndiContext = new ConfigureJndiContext(contextFactory, connectionUrl);
         List<ClientInstruction> instructions = new ArrayList<>();
         instructions.add(jndiContext);
@@ -209,12 +219,13 @@ public class EndToEndConversionTestBase extends BrokerAdminUsingTestBase
                 return Protocol.AMQP_0_8;
             }
         }
-        throw new RuntimeException("Unable to determine client protocol version");
+        throw new RuntimeException(String.format("Unable to determine client protocol version. Addition args are : "
+                                                 + "[%s]", additionalArgs));
     }
 
-    private void runClient(final Collection<String> clientGavs,
-                           final List<String> additionalJavaArguments,
-                           final List<ClientInstruction> jmsInstructions)
+    private ClientResult runClient(final Collection<String> clientGavs,
+                                   final List<String> additionalJavaArguments,
+                                   final List<ClientInstruction> jmsInstructions)
     {
         final List<ClientInstruction> clientInstructions = amendClientInstructions(jmsInstructions,
                                                                                    isAmqp0xClient(clientGavs));
@@ -252,11 +263,12 @@ public class EndToEndConversionTestBase extends BrokerAdminUsingTestBase
                     final Object result = inputStream.readObject();
                     if (result instanceof ClientResult)
                     {
-                        final ClientResult publisherResult = (ClientResult) result;
-                        if (publisherResult.getException() != null)
+                        final ClientResult clientResult = (ClientResult) result;
+                        if (clientResult.getException() != null)
                         {
-                            throw publisherResult.getException();
+                            throw clientResult.getException();
                         }
+                        return clientResult;
                     }
                     else
                     {
@@ -269,10 +281,9 @@ public class EndToEndConversionTestBase extends BrokerAdminUsingTestBase
                     p.waitFor();
                     loggingThread.flush();
                     loggingThread.stop();
+                    LOGGER.debug("client process finished exit value: {}", p.exitValue());
                 }
             }
-
-            LOGGER.debug("client process finished exit value: {}", p.exitValue());
         }
         catch (RuntimeException e)
         {
@@ -295,11 +306,22 @@ public class EndToEndConversionTestBase extends BrokerAdminUsingTestBase
         return "org.apache.qpid.jndi.PropertiesFileInitialContextFactory";
     }
 
-    private String getAmqp0xConnectionUrl()
+    private String getAmqp0xConnectionUrl(final List<AugumentConnectionUrl> configUrls)
     {
         InetSocketAddress brokerAddress = getBrokerAdmin().getBrokerAddress(BrokerAdmin.PortType.ANONYMOUS_AMQP);
         int port = brokerAddress.getPort();
         String hostString = "localhost";
+
+        if (!configUrls.isEmpty())
+        {
+            for (final AugumentConnectionUrl configUrl : configUrls)
+            {
+                if (!configUrl.getConnectionUrlConfig().isEmpty())
+                {
+                    throw new UnsupportedOperationException("Not implemented");
+                }
+            }
+        }
         return String.format("amqp://clientid/?brokerlist='tcp://%s:%d'", hostString, port);
     }
 
@@ -308,13 +330,20 @@ public class EndToEndConversionTestBase extends BrokerAdminUsingTestBase
         return "org.apache.qpid.jms.jndi.JmsInitialContextFactory";
     }
 
-    private String getAmqp10ConnectionUrl()
+    private String getAmqp10ConnectionUrl(final List<AugumentConnectionUrl> configUrls)
     {
         InetSocketAddress brokerAddress = getBrokerAdmin().getBrokerAddress(BrokerAdmin.PortType.ANONYMOUS_AMQP);
         int port = brokerAddress.getPort();
         String hostString = "localhost";
         int connectTimeout = 30000;
-        return String.format("amqp://%s:%d?jms.connectTimeout=%d", hostString, port, connectTimeout);
+
+        String additional = configUrls.stream()
+                                      .map(i -> i.getConnectionUrlConfig().entrySet())
+                                      .flatMap(Collection::stream)
+                                      .map(e -> String.format("%s=%s", e.getKey(), e.getValue()))
+                                      .collect(Collectors.joining("&", "&", ""));
+
+        return String.format("amqp://%s:%d?jms.connectTimeout=%d%s", hostString, port, connectTimeout, additional);
     }
 
     private boolean isAmqp0xClient(final Collection<String> gavs)

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/40df8179/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/client/AugumentConnectionUrl.java
----------------------------------------------------------------------
diff --git a/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/client/AugumentConnectionUrl.java b/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/client/AugumentConnectionUrl.java
new file mode 100644
index 0000000..df7137e
--- /dev/null
+++ b/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/client/AugumentConnectionUrl.java
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+package org.apache.qpid.systests.end_to_end_conversion.client;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+public class AugumentConnectionUrl implements ClientInstruction
+{
+    private Map<String, String> _connectionUrlConfig;
+
+    public AugumentConnectionUrl(final Map<String, String> connectionUrlConfig)
+    {
+
+        _connectionUrlConfig = new HashMap<>(connectionUrlConfig);
+    }
+
+
+    public Map<String, String> getConnectionUrlConfig()
+    {
+        return Collections.unmodifiableMap(_connectionUrlConfig);
+    }
+}

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/40df8179/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/client/Client.java
----------------------------------------------------------------------
diff --git a/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/client/Client.java b/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/client/Client.java
index b6ad152..9e53200 100644
--- a/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/client/Client.java
+++ b/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/client/Client.java
@@ -26,6 +26,7 @@ import java.io.PrintWriter;
 import java.io.Serializable;
 import java.io.StringWriter;
 import java.net.Socket;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Hashtable;
 import java.util.List;
@@ -83,6 +84,7 @@ public class Client
                 }
                 System.out.println(String.format("Received instructions : %s", instructions.toString()));
 
+                List<ClientMessage> clientMessages = new ArrayList<>();
                 if (!instructions.isEmpty())
                 {
                     String connectionUrl = null;
@@ -111,7 +113,12 @@ public class Client
                             try
                             {
                                 connection.start();
-                                handleInstructions(context, connection, instructions.subList(i, instructions.size()));
+                                List<ClientMessage> messages = handleInstructions(context,
+                                                                                  connection,
+                                                                                  instructions.subList(i,
+                                                                                                       instructions
+                                                                                                               .size()));
+                                clientMessages.addAll(messages);
                             }
                             finally
                             {
@@ -122,7 +129,7 @@ public class Client
                     }
                 }
                 System.out.println("Finished successfully");
-                objectOutputStream.writeObject(new ClientResult());
+                objectOutputStream.writeObject(new ClientResult(clientMessages));
             }
             catch (VerificationException e)
             {
@@ -153,21 +160,23 @@ public class Client
         return sw.toString();
     }
 
-    private void handleInstructions(final Context context,
-                                    final Connection connection,
-                                    final List<ClientInstruction> instructions) throws Exception
+    private List<ClientMessage> handleInstructions(final Context context,
+                                                   final Connection connection,
+                                                   final List<ClientInstruction> instructions) throws Exception
     {
+        List<ClientMessage> clientMessages = new ArrayList<>(instructions.size());
         Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
         try
         {
             for (ClientInstruction instruction : instructions)
             {
                 System.out.println(String.format("Process instruction: %s", instruction));
+                final ClientMessage clientMessage;
                 if (instruction instanceof MessagingInstruction.PublishMessage)
                 {
                     final MessagingInstruction.PublishMessage publishInstruction =
                             (MessagingInstruction.PublishMessage) instruction;
-                    publishMessage(context, session, publishInstruction);
+                    clientMessage = publishMessage(context, session, publishInstruction);
                 }
                 else if (instruction instanceof MessagingInstruction.ReceiveMessage)
                 {
@@ -176,24 +185,26 @@ public class Client
                     final Destination destination =
                             (Destination) context.lookup(receiveInstruction.getDestinationJndiName());
                     final MessageDescription messageDescription = receiveInstruction.getMessageDescription();
-                    receiveMessage(session, destination, messageDescription);
+                    clientMessage = receiveMessage(session, destination, messageDescription);
                 }
                 else
                 {
                     throw new RuntimeException(String.format("Unknown jmsInstruction class: '%s'",
                                                              instruction.getClass().getName()));
                 }
+                clientMessages.add(clientMessage);
             }
         }
         finally
         {
             session.close();
         }
+        return clientMessages;
     }
 
-    private void receiveMessage(final Session session,
-                                final Destination queue,
-                                final MessageDescription messageDescription) throws Exception
+    private ClientMessage receiveMessage(final Session session,
+                                         final Destination queue,
+                                         final MessageDescription messageDescription) throws Exception
     {
         final Message message;
         MessageConsumer consumer = session.createConsumer(queue);
@@ -215,11 +226,14 @@ public class Client
                       message.getJMSReplyTo(),
                       messageDescription.getHeader(MessageDescription.MessageHeader.CORRELATION_ID));
         }
+
+        return buildClientMessage(message);
     }
 
-    private void publishMessage(final Context context,
-                                final Session session,
-                                final MessagingInstruction.PublishMessage publishMessageInstruction) throws Exception
+    private ClientMessage publishMessage(final Context context,
+                                         final Session session,
+                                         final MessagingInstruction.PublishMessage publishMessageInstruction)
+            throws Exception
     {
         final MessageDescription messageDescription = publishMessageInstruction.getMessageDescription();
 
@@ -286,6 +300,9 @@ public class Client
                 replyToConsumer.close();
             }
         }
+
+        return buildClientMessage(message);
+
     }
 
     private void receiveReply(final MessageConsumer consumer, final Serializable expectedCorrelationId)
@@ -338,4 +355,123 @@ public class Client
             producer.close();
         }
     }
+
+    private ClientMessage buildClientMessage(final Message message) throws JMSException
+    {
+        String jmsMessageID = message.getJMSMessageID();
+        String jmsCorrelationID = message.getJMSCorrelationID();
+        byte[] jmsCorrelationIDAsBytes;
+        try
+        {
+            jmsCorrelationIDAsBytes = message.getJMSCorrelationIDAsBytes();
+        }
+        // NPE is thrown in 6.1.x JMS AMQP 0-x client on attempt to retrieve correlation ID when it is not set
+        // The issue was fixed in 6.3.0 client as part of QPID-7897
+        catch (JMSException | NullPointerException e)
+        {
+            jmsCorrelationIDAsBytes = null;
+        }
+        long jmsTimestamp = message.getJMSTimestamp();
+        int jmsDeliveryMode = message.getJMSDeliveryMode();
+        boolean jmsRedelivered = message.getJMSRedelivered();
+        String jmsType = message.getJMSType();
+        long jmsExpiration = message.getJMSExpiration();
+        int jmsPriority = message.getJMSPriority();
+
+        return new JMSMessageAdaptor(jmsMessageID,
+                                     jmsTimestamp,
+                                     jmsCorrelationID,
+                                     jmsCorrelationIDAsBytes,
+                                     jmsDeliveryMode,
+                                     jmsRedelivered,
+                                     jmsType,
+                                     jmsExpiration,
+                                     jmsPriority);
+    }
+
+    private static class JMSMessageAdaptor implements ClientMessage
+    {
+        private final String _jmsMessageID;
+        private final long _jmsTimestamp;
+        private final String _jmsCorrelationID;
+        private final byte[] _jmsCorrelationIDAsBytes;
+        private final int _jmsDeliveryMode;
+        private final boolean _jmsRedelivered;
+        private final String _jmsType;
+        private final long _jmsExpiration;
+        private final int _jmsPriority;
+
+        JMSMessageAdaptor(final String jmsMessageID,
+                          final long jmsTimestamp,
+                          final String jmsCorrelationID,
+                          final byte[] jmsCorrelationIDAsBytes,
+                          final int jmsDeliveryMode,
+                          final boolean jmsRedelivered,
+                          final String jmsType, final long jmsExpiration, final int jmsPriority)
+        {
+            _jmsMessageID = jmsMessageID;
+            _jmsTimestamp = jmsTimestamp;
+            _jmsCorrelationID = jmsCorrelationID;
+            _jmsCorrelationIDAsBytes = jmsCorrelationIDAsBytes;
+            _jmsDeliveryMode = jmsDeliveryMode;
+            _jmsRedelivered = jmsRedelivered;
+            _jmsType = jmsType;
+            _jmsExpiration = jmsExpiration;
+            _jmsPriority = jmsPriority;
+        }
+
+        @Override
+        public String getJMSMessageID()
+        {
+            return _jmsMessageID;
+        }
+
+        @Override
+        public long getJMSTimestamp()
+        {
+            return _jmsTimestamp;
+        }
+
+        @Override
+        public String getJMSCorrelationID()
+        {
+            return _jmsCorrelationID;
+        }
+
+        @Override
+        public byte[] getJMSCorrelationIDAsBytes()
+        {
+            return _jmsCorrelationIDAsBytes;
+        }
+
+        @Override
+        public int getJMSDeliveryMode()
+        {
+            return _jmsDeliveryMode;
+        }
+
+        @Override
+        public boolean getJMSRedelivered()
+        {
+            return _jmsRedelivered;
+        }
+
+        @Override
+        public String getJMSType()
+        {
+            return _jmsType;
+        }
+
+        @Override
+        public long getJMSExpiration()
+        {
+            return _jmsExpiration;
+        }
+
+        @Override
+        public int getJMSPriority()
+        {
+            return _jmsPriority;
+        }
+    }
 }

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/40df8179/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/client/ClientMessage.java
----------------------------------------------------------------------
diff --git a/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/client/ClientMessage.java b/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/client/ClientMessage.java
new file mode 100644
index 0000000..d3eb146
--- /dev/null
+++ b/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/client/ClientMessage.java
@@ -0,0 +1,37 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+package org.apache.qpid.systests.end_to_end_conversion.client;
+
+import java.io.Serializable;
+
+public interface ClientMessage extends Serializable
+{
+    String getJMSMessageID();
+    long getJMSTimestamp();
+    String getJMSCorrelationID();
+    byte[] getJMSCorrelationIDAsBytes();
+    int getJMSDeliveryMode();
+    boolean getJMSRedelivered();
+    String getJMSType();
+    long getJMSExpiration();
+    int getJMSPriority();
+}

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/40df8179/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/client/ClientResult.java
----------------------------------------------------------------------
diff --git a/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/client/ClientResult.java b/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/client/ClientResult.java
index f8eef68..ff91780 100644
--- a/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/client/ClientResult.java
+++ b/systests/end-to-end-conversion-tests/src/main/java/org/apache/qpid/systests/end_to_end_conversion/client/ClientResult.java
@@ -21,23 +21,33 @@
 package org.apache.qpid.systests.end_to_end_conversion.client;
 
 import java.io.Serializable;
+import java.util.Collections;
+import java.util.List;
 
 public class ClientResult implements Serializable
 {
     private final Exception _exception;
+    private final List<ClientMessage> _clientMessages;
 
-    public ClientResult()
+    public ClientResult(final Exception exception)
     {
-        this(null);
+        _exception = exception;
+        _clientMessages = Collections.emptyList();
     }
 
-    public ClientResult(final Exception exception)
+    public ClientResult(final List<ClientMessage> clientMessages)
     {
-        _exception = exception;
+        _exception = null;
+        _clientMessages = clientMessages;
     }
 
     public Exception getException()
     {
         return _exception;
     }
+
+    public List<ClientMessage> getClientMessages()
+    {
+        return Collections.unmodifiableList(_clientMessages);
+    }
 }

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/40df8179/systests/end-to-end-conversion-tests/src/test/java/org/apache/qpid/systests/end_to_end_conversion/SimpleConversionTest.java
----------------------------------------------------------------------
diff --git a/systests/end-to-end-conversion-tests/src/test/java/org/apache/qpid/systests/end_to_end_conversion/SimpleConversionTest.java b/systests/end-to-end-conversion-tests/src/test/java/org/apache/qpid/systests/end_to_end_conversion/SimpleConversionTest.java
index dbad8fd..2385cd2 100644
--- a/systests/end-to-end-conversion-tests/src/test/java/org/apache/qpid/systests/end_to_end_conversion/SimpleConversionTest.java
+++ b/systests/end-to-end-conversion-tests/src/test/java/org/apache/qpid/systests/end_to_end_conversion/SimpleConversionTest.java
@@ -20,13 +20,20 @@
 
 package org.apache.qpid.systests.end_to_end_conversion;
 
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.CoreMatchers.startsWith;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assume.assumeFalse;
 import static org.junit.Assume.assumeTrue;
 
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 
@@ -34,12 +41,15 @@ import com.google.common.collect.Lists;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import org.junit.Before;
+import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TestName;
 
 import org.apache.qpid.server.model.Protocol;
 import org.apache.qpid.systests.end_to_end_conversion.client.ClientInstruction;
+import org.apache.qpid.systests.end_to_end_conversion.client.ClientMessage;
+import org.apache.qpid.systests.end_to_end_conversion.client.ClientResult;
 import org.apache.qpid.systests.end_to_end_conversion.client.MessageDescription;
 import org.apache.qpid.systests.end_to_end_conversion.client.SerializableTestClass;
 import org.apache.qpid.systests.end_to_end_conversion.client.VerificationException;
@@ -48,6 +58,9 @@ public class SimpleConversionTest extends EndToEndConversionTestBase
 {
     private static final long TEST_TIMEOUT = 30000L;
     private static final String QUEUE_JNDI_NAME = "queue";
+    private static final EnumSet<Protocol> AMQP_PRE010_PROTOCOLS =
+            EnumSet.of(Protocol.AMQP_0_9, Protocol.AMQP_0_9_1, Protocol.AMQP_0_8);
+    private static final String JMS_MESSAGE_IDPOLICY_MESSAGE_IDTYPE = "jms.messageIDPolicy.messageIDType";
 
     private HashMap<String, String> _defaultDestinations;
     @Rule
@@ -178,6 +191,240 @@ public class SimpleConversionTest extends EndToEndConversionTestBase
     }
 
     @Test
+    public void providerAssignedMessageId09_010() throws Exception
+    {
+        assumeTrue(AMQP_PRE010_PROTOCOLS.contains(getPublisherProtocolVersion())
+                   && EnumSet.of(Protocol.AMQP_0_10).contains(getSubscriberProtocolVersion()));
+
+        List<ClientMessage> clientResults = performProviderAssignedMessageIdTest(Collections.emptyMap());
+
+        ClientMessage publishedMessage = clientResults.get(0);
+        ClientMessage subscriberMessage = clientResults.get(1);
+
+        // Subscriber receives 0-10 UUID message-id.  Qpid JMS 0-x library synthesizes the ID: prefix
+        assertThat(subscriberMessage.getJMSMessageID(), equalTo(publishedMessage.getJMSMessageID()));
+    }
+
+    @Test
+    public void providerAssignedMessageId09_10() throws Exception
+    {
+        assumeTrue(AMQP_PRE010_PROTOCOLS.contains(getPublisherProtocolVersion())
+                   && EnumSet.of(Protocol.AMQP_1_0).contains(getSubscriberProtocolVersion()));
+
+        List<ClientMessage> clientResults = performProviderAssignedMessageIdTest(Collections.emptyMap());
+
+        ClientMessage publishedMessage = clientResults.get(0);
+        ClientMessage subscriberMessage = clientResults.get(1);
+
+        // TODO: On the wire the AMQP 1.0 client receives a string containing a message with
+        // message-id-string contain a ID: prefixed UUID. Would be better if the conversion layer sent a message-id-uuid
+        // as this would offer most compatibility and miminise the exposure of the ID prefix.
+        assertThat(subscriberMessage.getJMSMessageID(), equalTo(publishedMessage.getJMSMessageID()));
+    }
+
+    @Test
+    public void providerAssignedMessageId010_09() throws Exception
+    {
+        assumeTrue(EnumSet.of(Protocol.AMQP_0_10).contains(getPublisherProtocolVersion())
+                   && AMQP_PRE010_PROTOCOLS.contains(getSubscriberProtocolVersion()));
+
+        List<ClientMessage> clientResults = performProviderAssignedMessageIdTest(Collections.emptyMap());
+
+        ClientMessage publishedMessage = clientResults.get(0);
+        ClientMessage subscriberMessage = clientResults.get(1);
+
+        // Subscriber receives AMQShortString message-id with a ID: prefix.  The conversion layer already synthesizes
+        // this. See MessageConverter_0_10_to_0_8.java:130
+        assertThat(subscriberMessage.getJMSMessageID(), equalTo(publishedMessage.getJMSMessageID()));
+    }
+
+    @Test
+    public void providerAssignedMessageId010_10() throws Exception
+    {
+        assumeTrue(EnumSet.of(Protocol.AMQP_0_10).contains(getPublisherProtocolVersion())
+                   && EnumSet.of(Protocol.AMQP_1_0).contains(getSubscriberProtocolVersion()));
+
+        List<ClientMessage> clientResults = performProviderAssignedMessageIdTest(Collections.emptyMap());
+
+        ClientMessage publishedMessage = clientResults.get(0);
+        ClientMessage subscriberMessage = clientResults.get(1);
+
+        // Publisher sends a 0-10 UUID message-id. This is converted into message-id-uuid.  The Qpid JMS
+        // Client returns a ID:AMQP_UUID:
+        final String publishedJmsMessageID = publishedMessage.getJMSMessageID();
+        assertThat(publishedJmsMessageID, startsWith("ID:"));
+        String barePublishedJmsMessageID = publishedJmsMessageID.substring("ID:".length());
+        String expectedSubscriberJmsMessageID = String.format("ID:AMQP_UUID:%s", barePublishedJmsMessageID);
+        assertThat(subscriberMessage.getJMSMessageID(), equalTo(expectedSubscriberJmsMessageID));
+    }
+
+    @Test
+    public void providerAssignedMessageId_DefaultMode_10_09() throws Exception
+    {
+        assumeTrue(EnumSet.of(Protocol.AMQP_1_0).contains(getPublisherProtocolVersion())
+                   && AMQP_PRE010_PROTOCOLS.contains(getSubscriberProtocolVersion()));
+
+        List<ClientMessage> clientResults = performProviderAssignedMessageIdTest(Collections.emptyMap());
+
+        // On the wire the "message-id-string comprises an identity of the publisher + a message sequence number
+        ClientMessage publishedMessage = clientResults.get(0);
+        ClientMessage subscriberMessage = clientResults.get(1);
+
+        assertThat(subscriberMessage.getJMSMessageID(), equalTo(publishedMessage.getJMSMessageID()));
+    }
+
+    @Test
+    @Ignore("Currently subscriber receives the correct message id but without the ID prefix")
+    public void providerAssignedMessageId_UuidMode_10_09() throws Exception
+    {
+        assumeTrue(EnumSet.of(Protocol.AMQP_1_0).contains(getPublisherProtocolVersion())
+                   && AMQP_PRE010_PROTOCOLS
+                           .contains(getSubscriberProtocolVersion()));
+
+        List<ClientMessage> clientResults = performProviderAssignedMessageIdTest(Collections.singletonMap(
+                JMS_MESSAGE_IDPOLICY_MESSAGE_IDTYPE, "UUID"));
+
+        ClientMessage publishedMessage = clientResults.get(0);
+        ClientMessage subscriberMessage = clientResults.get(1);
+
+        // On the wire the message id is a AMQP 1.0 UUID
+        final String publishedJmsMessageID = publishedMessage.getJMSMessageID();
+        assertThat(publishedJmsMessageID, startsWith("ID:AMQP_UUID:"));
+        String barePublishedJmsMessageID = publishedJmsMessageID.substring("ID:AMQP_UUID:".length());
+        String expectedSubscriberJmsMessageID = String.format("ID:%s", barePublishedJmsMessageID);
+        assertThat(subscriberMessage.getJMSMessageID(), equalTo(expectedSubscriberJmsMessageID));
+
+
+        // TODO: On the wire the AMQP 0-x client receives a message id containing the a stringified UUID without prefix.
+        // This is inconsistent - in all other cases the client receives message ids prefixed.  Would be
+        // better if the conversion layer sent a synthesized the ID prefix.
+    }
+
+    @Test
+    @Ignore("Currently subscriber receives the correct message id but without the ID prefix")
+    public void providerAssignedMessageId_UuidStringMode_10_09() throws Exception
+    {
+        assumeTrue(EnumSet.of(Protocol.AMQP_1_0).contains(getPublisherProtocolVersion())
+                   && AMQP_PRE010_PROTOCOLS.contains(getSubscriberProtocolVersion()));
+
+        List<ClientMessage> clientResults = performProviderAssignedMessageIdTest(Collections.singletonMap(
+                JMS_MESSAGE_IDPOLICY_MESSAGE_IDTYPE, "UUID_STRING"));
+
+        ClientMessage publishedMessage = clientResults.get(0);
+        ClientMessage subscriberMessage = clientResults.get(1);
+
+        // On the wire the message-id is a string containing a UUID with no prefix
+        final String publishedJmsMessageID = publishedMessage.getJMSMessageID();
+        assertThat(publishedJmsMessageID, startsWith("ID:AMQP_NO_PREFIX:"));
+        String barePublishedJmsMessageID = publishedJmsMessageID.substring("ID:AMQP_NO_PREFIX:".length());
+        String expectedSubscriberJmsMessageID = String.format("ID:%s", barePublishedJmsMessageID);
+        assertThat(subscriberMessage.getJMSMessageID(), equalTo(expectedSubscriberJmsMessageID));
+
+        // TODO ditto above
+    }
+
+    @Test
+    public void providerAssignedMessageId_PrefixedUuidStringMode_10_09() throws Exception
+    {
+        assumeTrue(EnumSet.of(Protocol.AMQP_1_0).contains(getPublisherProtocolVersion())
+                   && AMQP_PRE010_PROTOCOLS.contains(getSubscriberProtocolVersion()));
+
+        List<ClientMessage> clientResults = performProviderAssignedMessageIdTest(Collections.singletonMap(
+                JMS_MESSAGE_IDPOLICY_MESSAGE_IDTYPE, "PREFIXED_UUID_STRING"));
+
+        ClientMessage publishedMessage = clientResults.get(0);
+        ClientMessage subscriberMessage = clientResults.get(1);
+
+        // On the wire the message-id is a message-id-string containing a UUID with ID: prefix
+        final String publishedJmsMessageID = publishedMessage.getJMSMessageID();
+        assertThat(publishedJmsMessageID, startsWith("ID:"));
+        String barePublishedJmsMessageID = publishedJmsMessageID.substring("ID:".length());
+        String expectedSubscriberJmsMessageID = String.format("ID:%s", barePublishedJmsMessageID);
+        assertThat(subscriberMessage.getJMSMessageID(), equalTo(expectedSubscriberJmsMessageID));
+    }
+
+    @Test
+    public void providerAssignedMessageId_DefaultMode_10_010() throws Exception
+    {
+        assumeTrue(EnumSet.of(Protocol.AMQP_1_0).contains(getPublisherProtocolVersion())
+                   && EnumSet.of(Protocol.AMQP_0_10).contains(getSubscriberProtocolVersion()));
+
+        List<ClientMessage> clientResults = performProviderAssignedMessageIdTest(Collections.emptyMap());
+
+        // On the wire the message-id is a message-id-string comprising an identity of the pubisher + a message
+        // sequence number
+        ClientMessage publishedMessage = clientResults.get(0);
+        ClientMessage subscriberMessage = clientResults.get(1);
+
+        // Conversion layer manufactures an UUID.  This will be unpredictable to the client.
+        assertThat(subscriberMessage.getJMSMessageID(), is(notNullValue()));
+    }
+
+    @Test
+    public void providerAssignedMessageId_UuidMode_10_010() throws Exception
+    {
+        assumeTrue(EnumSet.of(Protocol.AMQP_1_0).contains(getPublisherProtocolVersion())
+                   && EnumSet.of(Protocol.AMQP_0_10).contains(getSubscriberProtocolVersion()));
+
+        List<ClientMessage> clientResults = performProviderAssignedMessageIdTest(Collections.singletonMap(
+                JMS_MESSAGE_IDPOLICY_MESSAGE_IDTYPE, "UUID"));
+
+        ClientMessage publishedMessage = clientResults.get(0);
+        ClientMessage subscriberMessage = clientResults.get(1);
+
+        // On the wire the message-id is a message-id-uuid
+        final String publishedJmsMessageID = publishedMessage.getJMSMessageID();
+        assertThat(publishedJmsMessageID, startsWith("ID:AMQP_UUID:"));
+        String barePublishedJmsMessageID = publishedJmsMessageID.substring("ID:AMQP_UUID:".length());
+        String expectedSubscriberJmsMessageID = String.format("ID:%s", barePublishedJmsMessageID);
+        assertThat(subscriberMessage.getJMSMessageID(), equalTo(expectedSubscriberJmsMessageID));
+    }
+
+    @Test
+    public void providerAssignedMessageId_UuidStringMode_10_010() throws Exception
+    {
+        assumeTrue(EnumSet.of(Protocol.AMQP_1_0).contains(getPublisherProtocolVersion())
+                   && EnumSet.of(Protocol.AMQP_0_10).contains(getSubscriberProtocolVersion()));
+
+        List<ClientMessage> clientResults = performProviderAssignedMessageIdTest(Collections.singletonMap(
+                JMS_MESSAGE_IDPOLICY_MESSAGE_IDTYPE, "UUID_STRING"));
+
+        ClientMessage publishedMessage = clientResults.get(0);
+        ClientMessage subscriberMessage = clientResults.get(1);
+
+        // On the wire the message-id is a message-id-string containing a UUID without prefix
+        final String publishedJmsMessageID = publishedMessage.getJMSMessageID();
+        assertThat(publishedJmsMessageID, startsWith("ID:AMQP_NO_PREFIX:"));
+        String barePublishedJmsMessageID = publishedJmsMessageID.substring("ID:AMQP_NO_PREFIX:".length());
+        String expectedSubscriberJmsMessageID = String.format("ID:%s", barePublishedJmsMessageID);
+        assertThat(subscriberMessage.getJMSMessageID(), equalTo(expectedSubscriberJmsMessageID));
+    }
+
+    @Test
+    @Ignore("Currently subscriber receives a UUID that differs from the one sent")
+    public void providerAssignedMessageId_PrefixedUuidStringMode_10_010() throws Exception
+    {
+        assumeTrue(EnumSet.of(Protocol.AMQP_1_0).contains(getPublisherProtocolVersion())
+                   && EnumSet.of(Protocol.AMQP_0_10).contains(getSubscriberProtocolVersion()));
+
+        List<ClientMessage> clientResults = performProviderAssignedMessageIdTest(Collections.singletonMap(
+                JMS_MESSAGE_IDPOLICY_MESSAGE_IDTYPE, "PREFIXED_UUID_STRING"));
+
+        ClientMessage publishedMessage = clientResults.get(0);
+        ClientMessage subscriberMessage = clientResults.get(1);
+
+        // On the wire the message-id is a message-id-string containing a UUID with ID: prefix
+        final String publishedJmsMessageID = publishedMessage.getJMSMessageID();
+        assertThat(publishedJmsMessageID, startsWith("ID:"));
+        String barePublishedJmsMessageID = publishedJmsMessageID.substring("ID:".length());
+        String expectedSubscriberJmsMessageID = String.format("ID:%s", barePublishedJmsMessageID);
+        assertThat(subscriberMessage.getJMSMessageID(), equalTo(expectedSubscriberJmsMessageID));
+
+        // TODO correct conversion layer so that a string that contains a ID prefixed UUID is converted
+        // as a UUID.
+    }
+
+    @Test
     public void property() throws Exception
     {
         final MessageDescription messageDescription = new MessageDescription();
@@ -297,7 +544,7 @@ public class SimpleConversionTest extends EndToEndConversionTestBase
         performTest(publisherInstructions, subscriberInstructions);
     }
 
-    private void performSimpleTest(final MessageDescription messageDescription) throws Exception
+    private List<ClientResult> performSimpleTest(final MessageDescription messageDescription) throws Exception
     {
         final String destinationJndiName = QUEUE_JNDI_NAME;
         final List<ClientInstruction> publisherInstructions =
@@ -308,17 +555,17 @@ public class SimpleConversionTest extends EndToEndConversionTestBase
                 new ClientInstructionBuilder().configureDestinations(_defaultDestinations)
                                               .receiveMessage(destinationJndiName, messageDescription)
                                               .build();
-        performTest(publisherInstructions,subscriberInstructions);
+        return performTest(publisherInstructions,subscriberInstructions);
     }
 
-    private void performTest(final List<ClientInstruction> publisherInstructions,
-                            final List<ClientInstruction> subscriberInstructions) throws Exception
+    private List<ClientResult> performTest(final List<ClientInstruction> publisherInstructions,
+                                           final List<ClientInstruction> subscriberInstructions) throws Exception
     {
-        final ListenableFuture<?> publisherFuture = runPublisher(publisherInstructions);
-        final ListenableFuture<?> subscriberFuture = runSubscriber(subscriberInstructions);
+        final ListenableFuture<ClientResult> publisherFuture = runPublisher(publisherInstructions);
+        final ListenableFuture<ClientResult> subscriberFuture = runSubscriber(subscriberInstructions);
         try
         {
-            Futures.allAsList(publisherFuture, subscriberFuture).get(TEST_TIMEOUT, TimeUnit.MILLISECONDS);
+            return Futures.allAsList(publisherFuture, subscriberFuture).get(TEST_TIMEOUT, TimeUnit.MILLISECONDS);
         }
         catch (ExecutionException e)
         {
@@ -337,4 +584,32 @@ public class SimpleConversionTest extends EndToEndConversionTestBase
             }
         }
     }
+
+    private List<ClientMessage> performProviderAssignedMessageIdTest(final Map<String, String> publisherConnectionUrlConfig) throws Exception
+    {
+        final MessageDescription messageDescription = new MessageDescription();
+
+        final String destinationJndiName = QUEUE_JNDI_NAME;
+        final List<ClientInstruction> publisherInstructions =
+                new ClientInstructionBuilder().configureConnectionUrl(publisherConnectionUrlConfig)
+                                              .configureDestinations(_defaultDestinations)
+                                              .publishMessage(destinationJndiName, messageDescription)
+                                              .build();
+        final List<ClientInstruction> subscriberInstructions =
+                new ClientInstructionBuilder().configureDestinations(_defaultDestinations)
+                                              .receiveMessage(destinationJndiName, messageDescription)
+                                              .build();
+        List<ClientResult> clientResults = performTest(publisherInstructions, subscriberInstructions);
+        assertThat("Unexpected number of client results", clientResults.size(), equalTo(2));
+
+        ClientResult publishedClientResult = clientResults.get(0);
+        assertThat("Unexpected number of published client messages", publishedClientResult.getClientMessages().size(), equalTo(1));
+        ClientMessage publishedMessage = publishedClientResult.getClientMessages().get(0);
+
+        ClientResult subscriberClientResults = clientResults.get(1);
+        assertThat("Unexpected number of published client messages", subscriberClientResults.getClientMessages().size(), equalTo(1));
+        ClientMessage subscriberMessage = subscriberClientResults.getClientMessages().get(0);
+
+        return Arrays.asList(publishedMessage, subscriberMessage);
+    }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org


[3/6] qpid-broker-j git commit: QPID-7830: [Broker-J] [AMQP 0-8..0-91] Move caching responsubility to virtualhost

Posted by or...@apache.org.
QPID-7830: [Broker-J] [AMQP 0-8..0-91] Move caching responsubility to virtualhost

(cherry picked from commit ddc519a551061c682877784068e755677e2c6313)


Project: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/commit/d79537d2
Tree: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/tree/d79537d2
Diff: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/diff/d79537d2

Branch: refs/heads/7.0.x
Commit: d79537d2951dc6eabc7fabe6fa0760cb0f07c11d
Parents: 995d535
Author: Keith Wall <kw...@apache.org>
Authored: Fri Apr 27 12:54:01 2018 +0100
Committer: Alex Rudyy <or...@apache.org>
Committed: Fri May 11 16:10:04 2018 +0100

----------------------------------------------------------------------
 .../server/protocol/v0_8/AMQShortString.java    |  51 +++++++--
 .../qpid/server/security/QpidPrincipal.java     |  38 +++++++
 .../security/auth/AuthenticatedPrincipal.java   |  24 +----
 .../server/virtualhost/AbstractVirtualHost.java |  19 ++++
 .../qpid/server/virtualhost/CacheProvider.java  |  28 +++++
 .../qpid/server/virtualhost/NullCache.java      | 107 +++++++++++++++++++
 .../virtualhost/QueueManagingVirtualHost.java   |  15 ++-
 .../virtualhost/VirtualHostPrincipal.java       |   5 +
 .../protocol/v0_8/AMQShortStringTest.java       |  33 ++++--
 9 files changed, 274 insertions(+), 46 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/d79537d2/broker-core/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQShortString.java
----------------------------------------------------------------------
diff --git a/broker-core/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQShortString.java b/broker-core/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQShortString.java
index 0ee98c3..6d17f8e 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQShortString.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQShortString.java
@@ -23,15 +23,20 @@ package org.apache.qpid.server.protocol.v0_8;
 
 import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
+import java.security.AccessController;
 import java.util.Arrays;
-import java.util.concurrent.TimeUnit;
+
+import javax.security.auth.Subject;
 
 import com.google.common.cache.Cache;
-import com.google.common.cache.CacheBuilder;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import org.apache.qpid.server.bytebuffer.QpidByteBuffer;
+import org.apache.qpid.server.security.QpidPrincipal;
+import org.apache.qpid.server.virtualhost.CacheProvider;
+import org.apache.qpid.server.virtualhost.NullCache;
+import org.apache.qpid.server.virtualhost.VirtualHostPrincipal;
 
 /**
  * A short string is a representation of an AMQ Short String
@@ -47,14 +52,11 @@ public final class AMQShortString implements Comparable<AMQShortString>
     public static final int MAX_LENGTH = 255;
 
     private static final Logger LOGGER = LoggerFactory.getLogger(AMQShortString.class);
+    private static final NullCache<ByteBuffer, AMQShortString> NULL_CACHE = new NullCache<>();
 
     // Unfortunately CacheBuilder does not yet support keyEquivalence, so we have to wrap the keys in ByteBuffers
     // rather than using the byte arrays as keys.
-    private static ThreadLocal<Cache<ByteBuffer, AMQShortString>> CACHE =
-            ThreadLocal.withInitial(() -> CacheBuilder.newBuilder()
-                                                      .maximumSize(100)
-                                                      .expireAfterAccess(300, TimeUnit.SECONDS)
-                                                      .build());
+    private static ThreadLocal<Cache<ByteBuffer, AMQShortString>> CACHE = new ThreadLocal<>();
 
     private final byte[] _data;
     private int _hashCode;
@@ -100,7 +102,7 @@ public final class AMQShortString implements Comparable<AMQShortString>
             byte[] data = new byte[length];
             buffer.get(data);
 
-            final AMQShortString cached = CACHE.get().getIfPresent(ByteBuffer.wrap(data));
+            final AMQShortString cached = getShortStringCache().getIfPresent(ByteBuffer.wrap(data));
             return cached != null ? cached : new AMQShortString(data);
         }
     }
@@ -112,7 +114,7 @@ public final class AMQShortString implements Comparable<AMQShortString>
             throw new NullPointerException("Cannot create AMQShortString with null data[]");
         }
 
-        final AMQShortString cached = CACHE.get().getIfPresent(ByteBuffer.wrap(data));
+        final AMQShortString cached = getShortStringCache().getIfPresent(ByteBuffer.wrap(data));
         return cached != null ? cached : new AMQShortString(data);
     }
 
@@ -120,7 +122,7 @@ public final class AMQShortString implements Comparable<AMQShortString>
     {
         final byte[] data = EncodingUtils.asUTF8Bytes(string);
 
-        final AMQShortString cached = CACHE.get().getIfPresent(ByteBuffer.wrap(data));
+        final AMQShortString cached = getShortStringCache().getIfPresent(ByteBuffer.wrap(data));
         if (cached != null)
         {
             return cached;
@@ -301,7 +303,7 @@ public final class AMQShortString implements Comparable<AMQShortString>
 
     public void intern()
     {
-        CACHE.get().put(ByteBuffer.wrap(_data), this);
+        getShortStringCache().put(ByteBuffer.wrap(_data), this);
     }
 
     public static AMQShortString validValueOf(Object obj)
@@ -361,4 +363,31 @@ public final class AMQShortString implements Comparable<AMQShortString>
         return amqShortString == null ? null : amqShortString.toString();
     }
 
+    private static Cache<ByteBuffer, AMQShortString> getShortStringCache()
+    {
+        Cache<ByteBuffer, AMQShortString> cache = CACHE.get();
+        if (cache == null)
+        {
+            cache = NULL_CACHE;
+            Subject subject = Subject.getSubject(AccessController.getContext());
+            if (subject != null)
+            {
+                VirtualHostPrincipal principal = QpidPrincipal.getSingletonPrincipal(subject, true, VirtualHostPrincipal.class);
+
+                if (principal != null && principal.getVirtualHost() instanceof CacheProvider)
+                {
+                    CacheProvider cacheProvider = (CacheProvider) principal.getVirtualHost();
+                    cache = cacheProvider.getNamedCache("amqShortStringCache");
+                }
+            }
+            CACHE.set(cache);
+        }
+        return cache;
+    }
+
+    /** Unit testing only */
+    static void setCache(final Cache<ByteBuffer, AMQShortString> cache)
+    {
+        CACHE.set(cache);
+    }
 }

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/d79537d2/broker-core/src/main/java/org/apache/qpid/server/security/QpidPrincipal.java
----------------------------------------------------------------------
diff --git a/broker-core/src/main/java/org/apache/qpid/server/security/QpidPrincipal.java b/broker-core/src/main/java/org/apache/qpid/server/security/QpidPrincipal.java
index 46c717f..382fea8 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/security/QpidPrincipal.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/security/QpidPrincipal.java
@@ -21,11 +21,49 @@ package org.apache.qpid.server.security;
 
 import java.io.Serializable;
 import java.security.Principal;
+import java.util.Set;
+
+import javax.security.auth.Subject;
 
 import org.apache.qpid.server.model.ConfiguredObject;
 
 
 public interface QpidPrincipal extends Principal, Serializable
 {
+    static <P extends Principal> P getSingletonPrincipal(final Subject authSubject,
+                                                         final boolean isPrincipalOptional,
+                                                         final Class<P> principalClazz)
+    {
+        if (authSubject == null)
+        {
+            throw new IllegalArgumentException("No authenticated subject.");
+        }
+
+        final Set<P> principals = authSubject.getPrincipals(principalClazz);
+        int numberOfAuthenticatedPrincipals = principals.size();
+
+        if(numberOfAuthenticatedPrincipals == 0 && isPrincipalOptional)
+        {
+            return null;
+        }
+        else
+        {
+            if (numberOfAuthenticatedPrincipals != 1)
+            {
+                throw new IllegalArgumentException(
+                        String.format(
+                                "Can't find single %s in the authenticated subject. There were %d "
+                                + "%s principals out of a total number of principals of: %s",
+                                principalClazz.getSimpleName(),
+                                principalClazz.getSimpleName(),
+                                numberOfAuthenticatedPrincipals,
+                                authSubject.getPrincipals()));
+            }
+            return principals.iterator().next();
+        }
+    }
+
     ConfiguredObject<?> getOrigin();
+
+
 }

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/d79537d2/broker-core/src/main/java/org/apache/qpid/server/security/auth/AuthenticatedPrincipal.java
----------------------------------------------------------------------
diff --git a/broker-core/src/main/java/org/apache/qpid/server/security/auth/AuthenticatedPrincipal.java b/broker-core/src/main/java/org/apache/qpid/server/security/auth/AuthenticatedPrincipal.java
index 3e4d688..3ead31c 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/security/auth/AuthenticatedPrincipal.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/security/auth/AuthenticatedPrincipal.java
@@ -126,29 +126,7 @@ public final class AuthenticatedPrincipal implements QpidPrincipal
 
     private static AuthenticatedPrincipal getAuthenticatedPrincipalFromSubject(final Subject authSubject, boolean isPrincipalOptional)
     {
-        if (authSubject == null)
-        {
-            throw new IllegalArgumentException("No authenticated subject.");
-        }
-
-        final Set<AuthenticatedPrincipal> principals = authSubject.getPrincipals(AuthenticatedPrincipal.class);
-        int numberOfAuthenticatedPrincipals = principals.size();
-
-        if(numberOfAuthenticatedPrincipals == 0 && isPrincipalOptional)
-        {
-            return null;
-        }
-        else
-        {
-            if (numberOfAuthenticatedPrincipals != 1)
-            {
-                throw new IllegalArgumentException(
-                        "Can't find single AuthenticatedPrincipal in authenticated subject. There were "
-                                + numberOfAuthenticatedPrincipals
-                                + " authenticated principals out of a total number of principals of: " + authSubject.getPrincipals());
-            }
-            return principals.iterator().next();
-        }
+        return QpidPrincipal.getSingletonPrincipal(authSubject, isPrincipalOptional, AuthenticatedPrincipal.class);
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/d79537d2/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java
----------------------------------------------------------------------
diff --git a/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java b/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java
index 7fab645..f097644 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java
@@ -53,6 +53,7 @@ import java.util.Set;
 import java.util.UUID;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.Future;
 import java.util.concurrent.RejectedExecutionException;
 import java.util.concurrent.ScheduledFuture;
@@ -65,6 +66,8 @@ import java.util.regex.PatternSyntaxException;
 
 import javax.security.auth.Subject;
 
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
 import com.google.common.util.concurrent.FutureCallback;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.JdkFutureAdapters;
@@ -156,6 +159,7 @@ public abstract class AbstractVirtualHost<X extends AbstractVirtualHost<X>> exte
     private final AtomicBoolean _acceptsConnections = new AtomicBoolean(false);
     private volatile TaskExecutor _preferenceTaskExecutor;
     private volatile boolean _deleteRequested;
+    private final ConcurrentMap<String, Cache> _caches = new ConcurrentHashMap<>();
 
     private enum BlockingType { STORE, FILESYSTEM };
 
@@ -3010,6 +3014,21 @@ public abstract class AbstractVirtualHost<X extends AbstractVirtualHost<X>> exte
         }));
     }
 
+    @Override
+    public <K, V> Cache<K, V> getNamedCache(final String cacheName)
+    {
+        final String maxSizeContextVarName = String.format(NAMED_CACHE_MAXIMUM_SIZE_FORMAT, cacheName);
+        final String expirationContextVarName = String.format(NAMED_CACHE_EXPIRATION_FORMAT, cacheName);
+        Set<String> contextKeys = getContextKeys(false);
+        int maxSize = contextKeys.contains(maxSizeContextVarName) ? getContextValue(Integer.class, maxSizeContextVarName) : getContextValue(Integer.class, NAMED_CACHE_MAXIMUM_SIZE);
+        long expiration = contextKeys.contains(expirationContextVarName) ? getContextValue(Long.class, expirationContextVarName) : getContextValue(Long.class, NAMED_CACHE_EXPIRATION);
+
+        return _caches.computeIfAbsent(cacheName, (k) -> CacheBuilder.<K, V>newBuilder()
+                .maximumSize(maxSize)
+                .expireAfterAccess(expiration, TimeUnit.MILLISECONDS)
+                .build());
+    }
+
     private boolean hasDifferentBindings(final Exchange<?> exchange,
                                          final Queue queue,
                                          final Map<String, Map<String,Object>> bindings)

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/d79537d2/broker-core/src/main/java/org/apache/qpid/server/virtualhost/CacheProvider.java
----------------------------------------------------------------------
diff --git a/broker-core/src/main/java/org/apache/qpid/server/virtualhost/CacheProvider.java b/broker-core/src/main/java/org/apache/qpid/server/virtualhost/CacheProvider.java
new file mode 100644
index 0000000..64a6233
--- /dev/null
+++ b/broker-core/src/main/java/org/apache/qpid/server/virtualhost/CacheProvider.java
@@ -0,0 +1,28 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+package org.apache.qpid.server.virtualhost;
+
+import com.google.common.cache.Cache;
+
+public interface CacheProvider
+{
+    <K, V> Cache<K, V> getNamedCache(String cacheName);
+}

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/d79537d2/broker-core/src/main/java/org/apache/qpid/server/virtualhost/NullCache.java
----------------------------------------------------------------------
diff --git a/broker-core/src/main/java/org/apache/qpid/server/virtualhost/NullCache.java b/broker-core/src/main/java/org/apache/qpid/server/virtualhost/NullCache.java
new file mode 100644
index 0000000..3d2ef6f
--- /dev/null
+++ b/broker-core/src/main/java/org/apache/qpid/server/virtualhost/NullCache.java
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+package org.apache.qpid.server.virtualhost;
+
+import java.util.Map;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.ExecutionException;
+
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheStats;
+import com.google.common.collect.ImmutableMap;
+
+public class NullCache<K, V> implements Cache<K, V>
+{
+    @Override
+    public V getIfPresent(final Object key)
+    {
+        return null;
+    }
+
+    @Override
+    public V get(final K key, final Callable<? extends V> loader) throws ExecutionException
+    {
+        try
+        {
+            return loader.call();
+        }
+        catch (Exception e)
+        {
+            throw new ExecutionException(e);
+        }
+    }
+
+    @Override
+    public ImmutableMap<K, V> getAllPresent(final Iterable<?> keys)
+    {
+        return ImmutableMap.of();
+    }
+
+    @Override
+    public void put(final K key, final V value)
+    {
+    }
+
+    @Override
+    public void putAll(final Map<? extends K, ? extends V> m)
+    {
+    }
+
+    @Override
+    public void invalidate(final Object key)
+    {
+    }
+
+    @Override
+    public void invalidateAll(final Iterable<?> keys)
+    {
+    }
+
+    @Override
+    public void invalidateAll()
+    {
+    }
+
+    @Override
+    public long size()
+    {
+        return 0;
+    }
+
+    @Override
+    public CacheStats stats()
+    {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public ConcurrentMap<K, V> asMap()
+    {
+        return new ConcurrentHashMap<>();
+    }
+
+    @Override
+    public void cleanUp()
+    {
+    }
+}

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/d79537d2/broker-core/src/main/java/org/apache/qpid/server/virtualhost/QueueManagingVirtualHost.java
----------------------------------------------------------------------
diff --git a/broker-core/src/main/java/org/apache/qpid/server/virtualhost/QueueManagingVirtualHost.java b/broker-core/src/main/java/org/apache/qpid/server/virtualhost/QueueManagingVirtualHost.java
index b30373a..9388304 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/virtualhost/QueueManagingVirtualHost.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/virtualhost/QueueManagingVirtualHost.java
@@ -57,7 +57,8 @@ public interface QueueManagingVirtualHost<X extends QueueManagingVirtualHost<X>>
                                                                                          EventListener,
                                                                                          StatisticsGatherer,
                                                                                          UserPreferencesCreator,
-                                                                                         EventLoggerProvider
+                                                                                         EventLoggerProvider,
+                                                                                         CacheProvider
 {
     String HOUSEKEEPING_CHECK_PERIOD            = "housekeepingCheckPeriod";
     String STORE_TRANSACTION_IDLE_TIMEOUT_CLOSE = "storeTransactionIdleTimeoutClose";
@@ -163,6 +164,18 @@ public interface QueueManagingVirtualHost<X extends QueueManagingVirtualHost<X>>
     @ManagedContextDefault( name = VIRTUALHOST_CONNECTION_THREAD_POOL_NUMBER_OF_SELECTORS)
     long DEFAULT_VIRTUALHOST_CONNECTION_THREAD_POOL_NUMBER_OF_SELECTORS = Math.max(DEFAULT_VIRTUALHOST_CONNECTION_THREAD_POOL_SIZE/8, 1);
 
+    String NAMED_CACHE_MAXIMUM_SIZE = "virtualhost.namedCache.maximumSize";
+    @SuppressWarnings("unused")
+    @ManagedContextDefault(name = NAMED_CACHE_MAXIMUM_SIZE, description = "Maximum number of entries within the named cached")
+    int DEFAULT_NAMED_CACHE_SIZE = 100;
+    String NAMED_CACHE_MAXIMUM_SIZE_FORMAT = "virtualhost.namedCache.%s.maximumSize";
+
+    String NAMED_CACHE_EXPIRATION = "virtualhost.namedCache.expiration";
+    @SuppressWarnings("unused")
+    @ManagedContextDefault(name = NAMED_CACHE_EXPIRATION, description = "Expiration time (in millis) applied to cached values within the named cache")
+    long DEFAULT_NAMED_CACHE_EXPIRATION = 300 * 1000;
+    String NAMED_CACHE_EXPIRATION_FORMAT = "virtualhost.namedCache.%s.expiration";
+
     @ManagedAttribute( defaultValue = "${" + QueueManagingVirtualHost.VIRTUALHOST_CONNECTION_THREAD_POOL_NUMBER_OF_SELECTORS + "}")
     int getNumberOfSelectors();
 

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/d79537d2/broker-core/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostPrincipal.java
----------------------------------------------------------------------
diff --git a/broker-core/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostPrincipal.java b/broker-core/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostPrincipal.java
index b3d7374..14fd8cc 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostPrincipal.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostPrincipal.java
@@ -44,6 +44,11 @@ public class VirtualHostPrincipal implements Principal, Serializable
         return _name;
     }
 
+    public VirtualHost<?> getVirtualHost()
+    {
+        return _virtualHost;
+    }
+
     @Override
     public boolean equals(Object o)
     {

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/d79537d2/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQShortStringTest.java
----------------------------------------------------------------------
diff --git a/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQShortStringTest.java b/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQShortStringTest.java
index 8ff7c96..7d29b5b 100644
--- a/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQShortStringTest.java
+++ b/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQShortStringTest.java
@@ -20,10 +20,12 @@
 
 package org.apache.qpid.server.protocol.v0_8;
 
-import org.apache.qpid.test.utils.QpidTestCase;
-
 import java.nio.charset.StandardCharsets;
 
+import com.google.common.cache.CacheBuilder;
+
+import org.apache.qpid.test.utils.QpidTestCase;
+
 public class AMQShortStringTest extends QpidTestCase
 {
 
@@ -132,15 +134,24 @@ public class AMQShortStringTest extends QpidTestCase
 
     public void testInterning()
     {
-        AMQShortString str1 = AMQShortString.createAMQShortString("hello");
-        str1.intern();
-        AMQShortString str2 = AMQShortString.createAMQShortString("hello");
-        AMQShortString str3 = AMQShortString.createAMQShortString("hello".getBytes(StandardCharsets.UTF_8));
-
-        assertEquals(str1, str2);
-        assertEquals(str1, str3);
-        assertSame(str1, str2);
-        assertSame(str1, str3);
+        AMQShortString.setCache(CacheBuilder.newBuilder().maximumSize(1).build());
+
+        try
+        {
+            AMQShortString str1 = AMQShortString.createAMQShortString("hello");
+            str1.intern();
+            AMQShortString str2 = AMQShortString.createAMQShortString("hello");
+            AMQShortString str3 = AMQShortString.createAMQShortString("hello".getBytes(StandardCharsets.UTF_8));
+
+            assertEquals(str1, str2);
+            assertEquals(str1, str3);
+            assertSame(str1, str2);
+            assertSame(str1, str3);
+        }
+        finally
+        {
+            AMQShortString.setCache(null);
+        }
     }
 
     /**


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org