* tests/FramingTest.cpp - added test for validation of content data type

* lib/broker/BrokerAdapter.cpp - initial unbind implementation
* lib/common/framing/FramingContent.cpp - minor code cleanup



git-svn-id: https://svn.apache.org/repos/asf/incubator/qpid/branches/qpid.0-9@497755 13f79535-47bb-0310-9956-ffa450edef68

3 files changed, 58 insertions, 40 deletions

diff --git a/cpp/lib/broker/BrokerAdapter.cpp b/cpp/lib/broker/BrokerAdapter.cpp
index 41a66b6727..0d34868710 100644
--- a/cpp/lib/broker/BrokerAdapter.cpp
+++ b/cpp/lib/broker/BrokerAdapter.cpp
@@ -115,10 +115,6 @@ class BrokerAdapter::ServerOps : public AMQP_ServerOperations
                      const qpid::framing::FieldTable& arguments); 
         void delete_(const MethodContext& context, u_int16_t ticket,
                      const std::string& exchange, bool ifUnused, bool nowait); 
-        void unbind(const MethodContext& context,
-                    u_int16_t ticket, const std::string& queue,
-                    const std::string& exchange, const std::string& routingKey,
-                    const qpid::framing::FieldTable& arguments );
     };
 
     class QueueHandlerImpl : private CoreRefs, public QueueHandler{
@@ -265,20 +261,6 @@ void BrokerAdapter::ServerOps::ExchangeHandlerImpl::declare(const MethodContext&
         connection.client->getExchange().declareOk(channel.getId());
     }
 }
-
-                
-void BrokerAdapter::ServerOps::ExchangeHandlerImpl::unbind(
-    const MethodContext&,
-    u_int16_t /*ticket*/,
-    const string& /*queue*/,
-    const string& /*exchange*/,
-    const string& /*routingKey*/,
-    const qpid::framing::FieldTable& /*arguments*/ )
-{
-    assert(0);            // FIXME aconway 2007-01-04: 0-9 feature
-}
-
-
                 
 void BrokerAdapter::ServerOps::ExchangeHandlerImpl::delete_(const MethodContext&, u_int16_t /*ticket*/, 
                                                             const string& exchange, bool /*ifUnused*/, bool nowait){
@@ -330,9 +312,6 @@ void BrokerAdapter::ServerOps::QueueHandlerImpl::bind(const MethodContext&, u_in
     Queue::shared_ptr queue = connection.getQueue(queueName, channel.getId());
     Exchange::shared_ptr exchange = broker.getExchanges().get(exchangeName);
     if(exchange){
-        // kpvdr - cannot use this any longer as routingKey is now const
-        //        if(routingKey.empty() && queueName.empty()) routingKey = queue->getName();
-        //        exchange->bind(queue, routingKey, &arguments);
         string exchangeRoutingKey = routingKey.empty() && queueName.empty() ? queue->getName() : routingKey;
         exchange->bind(queue, exchangeRoutingKey, &arguments);
         if(!nowait) connection.client->getQueue().bindOk(channel.getId());    
@@ -340,7 +319,27 @@ void BrokerAdapter::ServerOps::QueueHandlerImpl::bind(const MethodContext&, u_in
         throw ChannelException(
             404, "Bind failed. No such exchange: " + exchangeName);
     }
-} 
+}
+ 
+void 
+BrokerAdapter::ServerOps::QueueHandlerImpl::unbind(
+    const MethodContext&,
+    u_int16_t /*ticket*/,
+    const string& queueName,
+    const string& exchangeName,
+    const string& routingKey,
+    const qpid::framing::FieldTable& arguments )
+{
+    Queue::shared_ptr queue = connection.getQueue(queueName, channel.getId());
+    if (!queue.get()) throw ChannelException(404, "Unbind failed. No such exchange: " + exchangeName);
+
+    Exchange::shared_ptr exchange = broker.getExchanges().get(exchangeName);
+    if (!exchange.get()) throw ChannelException(404, "Unbind failed. No such exchange: " + exchangeName);
+
+    exchange->unbind(queue, routingKey, &arguments);
+
+    connection.client->getQueue().unbindOk(channel.getId());    
+}
         
 void BrokerAdapter::ServerOps::QueueHandlerImpl::purge(const MethodContext&, u_int16_t /*ticket*/, const string& queueName, bool nowait){
 
@@ -471,21 +470,9 @@ void BrokerAdapter::ServerOps::TxHandlerImpl::rollback(const MethodContext&){
 }
               
 void
-BrokerAdapter::ServerOps::QueueHandlerImpl::unbind(
-    const MethodContext&,
-    u_int16_t /*ticket*/,
-    const string& /*queue*/,
-    const string& /*exchange*/,
-    const string& /*routingKey*/,
-    const qpid::framing::FieldTable& /*arguments*/ )
-{
-    assert(0);                // FIXME aconway 2007-01-04: 0-9 feature
-}
-
-void
 BrokerAdapter::ServerOps::ChannelHandlerImpl::ok( const MethodContext& )
 {
-    assert(0);                // FIXME aconway 2007-01-04: 0-9 feature
+    //no specific action required, generic response handling should be sufficient
 }
 
 void
diff --git a/cpp/lib/common/framing/FramingContent.cpp b/cpp/lib/common/framing/FramingContent.cpp
index bf3399525e..45335ba9c9 100644
--- a/cpp/lib/common/framing/FramingContent.cpp
+++ b/cpp/lib/common/framing/FramingContent.cpp
@@ -35,16 +35,13 @@ Content::Content(u_int8_t _discriminator, const string& _value): discriminator(_
 }
 
 void Content::validate() {
-    //validation:
     if (discriminator == REFERENCE) {
         if(value.empty()) {
-            //cannot have empty reference
             THROW_QPID_ERROR(FRAMING_ERROR, "Reference cannot be empty");
         }
     }else if (discriminator != INLINE) {
-        //invalid discriminator
         std::stringstream out;
-        out << "Invalid discriminator: " << discriminator;
+        out << "Invalid discriminator: " << (int) discriminator;
 	THROW_QPID_ERROR(FRAMING_ERROR, out.str());
     }
 }
@@ -63,7 +60,7 @@ void Content::decode(Buffer& buffer) {
 }
 
 size_t Content::size() const {
-    return 1 + 4 + value.size();
+    return 1/*discriminator*/ + 4/*for recording size of long string*/ + value.size();
 }
 
 std::ostream& operator<<(std::ostream& out, const Content& content) {
diff --git a/cpp/tests/FramingTest.cpp b/cpp/tests/FramingTest.cpp
index b081b5822b..445d13e384 100644
--- a/cpp/tests/FramingTest.cpp
+++ b/cpp/tests/FramingTest.cpp
@@ -30,8 +30,10 @@
 #include "AMQResponseBody.h"
 #include "Requester.h"
 #include "Responder.h"
+#include <QpidError.h>
 
 using namespace qpid::framing;
+using qpid::QpidError;
 
 template <class T>
 std::string tostring(const T& x) 
@@ -57,6 +59,7 @@ class FramingTest : public CppUnit::TestCase
     CPPUNIT_TEST(testResponder);
     CPPUNIT_TEST(testInlineContent);
     CPPUNIT_TEST(testContentReference);
+    CPPUNIT_TEST(testContentValidation);
     CPPUNIT_TEST_SUITE_END();
 
   private:
@@ -199,6 +202,37 @@ class FramingTest : public CppUnit::TestCase
         CPPUNIT_ASSERT_EQUAL(content.getValue(), recovered.getValue());
     }
 
+    void testContentValidation() {
+        try {
+            Content content(REFERENCE, "");
+            CPPUNIT_ASSERT(false);//fail, expected exception
+        } catch (QpidError& e) {
+            CPPUNIT_ASSERT_EQUAL(FRAMING_ERROR, e.code);
+            CPPUNIT_ASSERT_EQUAL(string("Reference cannot be empty"), e.msg);
+        }
+        
+        try {
+            Content content(2, "Blah");
+            CPPUNIT_ASSERT(false);//fail, expected exception
+        } catch (QpidError& e) {
+            CPPUNIT_ASSERT_EQUAL(FRAMING_ERROR, e.code);
+            CPPUNIT_ASSERT_EQUAL(string("Invalid discriminator: 2"), e.msg);
+        }
+        
+        try {
+            buffer.putOctet(2);
+            buffer.putLongString("blah, blah");
+            buffer.flip();
+            Content content;
+            content.decode(buffer);
+            CPPUNIT_ASSERT(false);//fail, expected exception
+        } catch (QpidError& e) {
+            CPPUNIT_ASSERT_EQUAL(FRAMING_ERROR, e.code);
+            CPPUNIT_ASSERT_EQUAL(string("Invalid discriminator: 2"), e.msg);
+        }
+        
+    }
+
     void testRequester() {
         Requester r;
         AMQRequestBody::Data q;