You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ac...@apache.org on 2016/01/07 23:29:16 UTC

[06/15] qpid-proton git commit: PROTON-1092: c++: improve proton::message API

PROTON-1092: c++: improve proton::message API

Rename property/annotation maps to use AMQP spec terminology rather than proton C API:
    application_properties, message_annotations, delivery_annotations
    (was properties, annotations, instructions)

Use std::vector<char> for message::encode, decode instead of std::string.


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

Branch: refs/heads/go1
Commit: 06017111ed7e96b3382694a987fb105c326f9e83
Parents: 178aa46
Author: Alan Conway <ac...@redhat.com>
Authored: Wed Jan 6 14:50:16 2016 -0500
Committer: Alan Conway <ac...@redhat.com>
Committed: Wed Jan 6 16:08:00 2016 -0500

----------------------------------------------------------------------
 .../bindings/cpp/include/proton/message.hpp     | 43 ++++++------
 .../cpp/include/proton/pn_unique_ptr.hpp        |  7 +-
 proton-c/bindings/cpp/include/proton/ssl.hpp    |  1 -
 proton-c/bindings/cpp/include/proton/value.hpp  |  3 +-
 proton-c/bindings/cpp/src/message.cpp           | 70 ++++++++++----------
 proton-c/bindings/cpp/src/message_test.cpp      | 44 ++++++------
 proton-c/bindings/cpp/src/sender.cpp            |  4 +-
 proton-c/bindings/cpp/src/ssl_domain.cpp        |  8 ++-
 proton-c/bindings/cpp/src/value.cpp             |  4 +-
 9 files changed, 93 insertions(+), 91 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/06017111/proton-c/bindings/cpp/include/proton/message.hpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/include/proton/message.hpp b/proton-c/bindings/cpp/include/proton/message.hpp
index 8bd1f85..374c7e8 100644
--- a/proton-c/bindings/cpp/include/proton/message.hpp
+++ b/proton-c/bindings/cpp/include/proton/message.hpp
@@ -30,6 +30,7 @@
 #include "proton/duration.hpp"
 
 #include <string>
+#include <vector>
 #include <utility>
 
 struct pn_message_t;
@@ -60,12 +61,10 @@ class message
 
     PN_CPP_EXTERN message& operator=(const message&);
 
-    PN_CPP_EXTERN void swap(message& x);
-
     /** Clear the message content and properties. */
     PN_CPP_EXTERN void clear();
 
-    ///@name Standard AMQP properties
+    ///@name Standard AMQP message properties
     ///@{
 
     PN_CPP_EXTERN void id(const message_id& id);
@@ -116,25 +115,25 @@ class message
     PN_CPP_EXTERN value& body();
 
     /** Application properties map, can be modified in place. */
-    PN_CPP_EXTERN property_map& properties();
-    PN_CPP_EXTERN const property_map& properties() const;
+    PN_CPP_EXTERN property_map& application_properties();
+    PN_CPP_EXTERN const property_map& application_properties() const;
 
     /** Message annotations map, can be modified in place. */
-    PN_CPP_EXTERN annotation_map& annotations();
-    PN_CPP_EXTERN const annotation_map& annotations() const;
+    PN_CPP_EXTERN annotation_map& message_annotations();
+    PN_CPP_EXTERN const annotation_map& message_annotations() const;
 
-    /** Delivery instructions map, can be modified in place. */
-    PN_CPP_EXTERN annotation_map& instructions();
-    PN_CPP_EXTERN const annotation_map& instructions() const;
+    /** Delivery annotations map, can be modified in place. */
+    PN_CPP_EXTERN annotation_map& delivery_annotations();
+    PN_CPP_EXTERN const annotation_map& delivery_annotations() const;
 
-    /** Encode entire message into a string, growing the string if necessary. */
-    PN_CPP_EXTERN void encode(std::string &bytes) const;
+    /** Encode entire message into a byte vector, growing it if necessary. */
+    PN_CPP_EXTERN void encode(std::vector<char> &bytes) const;
 
-    /** Return encoded message as a string */
-    PN_CPP_EXTERN std::string encode() const;
+    /** Return encoded message as a byte vector */
+    PN_CPP_EXTERN std::vector<char> encode() const;
 
     /** Decode from string data into the message. */
-    PN_CPP_EXTERN void decode(const std::string &bytes);
+    PN_CPP_EXTERN void decode(const std::vector<char> &bytes);
 
     /** Decode the message corresponding to a delivery from a link. */
     PN_CPP_EXTERN void decode(proton::link, proton::delivery);
@@ -232,17 +231,19 @@ class message
     /** Get the group sequence for a message. */
     PN_CPP_EXTERN void sequence(int32_t);
 
+  friend PN_CPP_EXTERN void swap(message&, message&);
+
   private:
     pn_message_t *pn_msg() const;
-    struct impl {
+    mutable struct impl {
         PN_CPP_EXTERN impl();
         PN_CPP_EXTERN ~impl();
-        mutable pn_message_t *msg;
-        mutable value body;
+        pn_message_t *msg;
+        value body;
+        property_map application_properties;
+        annotation_map message_annotations;
+        annotation_map delivery_annotations;
     } impl_;
-    mutable property_map properties_;
-    mutable annotation_map annotations_;
-    mutable annotation_map instructions_;
 };
 
 }

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/06017111/proton-c/bindings/cpp/include/proton/pn_unique_ptr.hpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/include/proton/pn_unique_ptr.hpp b/proton-c/bindings/cpp/include/proton/pn_unique_ptr.hpp
index d63c8e5..6533b0b 100644
--- a/proton-c/bindings/cpp/include/proton/pn_unique_ptr.hpp
+++ b/proton-c/bindings/cpp/include/proton/pn_unique_ptr.hpp
@@ -38,16 +38,15 @@ template <class T> class pn_unique_ptr {
   public:
     pn_unique_ptr(T* p=0) : ptr_(p) {}
 #if PN_HAS_CPP11
-    pn_unique_ptr(pn_unique_ptr&& x) : ptr_(0)  { swap(x); }
+    pn_unique_ptr(pn_unique_ptr&& x) : ptr_(0)  { std::swap(ptr_, x.ptr_); }
 #else
-    pn_unique_ptr(const pn_unique_ptr& x) : ptr_() { swap(const_cast<pn_unique_ptr<T>&>(x)); }
+    pn_unique_ptr(const pn_unique_ptr& x) : ptr_() { std::swap(ptr_, const_cast<pn_unique_ptr&>(x).ptr_); }
 #endif
     ~pn_unique_ptr() { delete(ptr_); }
     T& operator*() const { return *ptr_; }
     T* operator->() const { return ptr_; }
     T* get() const { return ptr_; }
-    void swap(pn_unique_ptr<T>& x) { std::swap(ptr_, x.ptr_); }
-    void reset(T* p = 0) { pn_unique_ptr<T> tmp(p); swap(tmp); }
+    void reset(T* p = 0) { pn_unique_ptr<T> tmp(p); std::swap(ptr_, tmp.ptr_); }
     T* release() { T *p = ptr_; ptr_ = 0; return p; }
     operator bool() const { return get(); }
     bool operator !() const { return get(); }

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/06017111/proton-c/bindings/cpp/include/proton/ssl.hpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/include/proton/ssl.hpp b/proton-c/bindings/cpp/include/proton/ssl.hpp
index 0518dc6..6655df0 100644
--- a/proton-c/bindings/cpp/include/proton/ssl.hpp
+++ b/proton-c/bindings/cpp/include/proton/ssl.hpp
@@ -84,7 +84,6 @@ class ssl_domain {
   protected:
     ssl_domain(bool is_server);
     pn_ssl_domain_t *pn_domain();
-    void swap(ssl_domain &);
 
   private:
     ssl_domain_impl *impl_;

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/06017111/proton-c/bindings/cpp/include/proton/value.hpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/include/proton/value.hpp b/proton-c/bindings/cpp/include/proton/value.hpp
index 3c4cecd..3122dae 100644
--- a/proton-c/bindings/cpp/include/proton/value.hpp
+++ b/proton-c/bindings/cpp/include/proton/value.hpp
@@ -82,8 +82,7 @@ class value {
     PN_CPP_EXTERN std::string as_string() const; ///< Allowed if type_id_is_string_like(type())
     ///@}
 
-    PN_CPP_EXTERN void swap(value& v);
-
+  friend PN_CPP_EXTERN void swap(value&, value&);
   friend PN_CPP_EXTERN bool operator==(const value& x, const value& y);
   friend PN_CPP_EXTERN bool operator<(const value& x, const value& y);
   friend PN_CPP_EXTERN class encoder operator<<(class encoder e, const value& dv);

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/06017111/proton-c/bindings/cpp/src/message.cpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/message.cpp b/proton-c/bindings/cpp/src/message.cpp
index c7ee1f6..19741ec 100644
--- a/proton-c/bindings/cpp/src/message.cpp
+++ b/proton-c/bindings/cpp/src/message.cpp
@@ -33,6 +33,7 @@
 #include "proton_bits.hpp"
 
 #include <string>
+#include <algorithm>
 #include <assert.h>
 
 namespace proton {
@@ -59,16 +60,16 @@ message::message() {}
 message::message(const message &m) { *this = m; }
 
 #if PN_HAS_CPP11
-message::message(message &&m) { swap(m); }
+message::message(message &&m) { swap(*this, m); }
 #endif
 
 message::~message() {}
 
-void message::swap(message& m) { std::swap(impl_.msg, m.impl_.msg); }
+void swap(message& x, message& y) { std::swap(x.impl_.msg, y.impl_.msg); }
 
 message& message::operator=(const message& m) {
     // TODO aconway 2015-08-10: more efficient pn_message_copy function
-    std::string data;
+    std::vector<char> data;
     m.encode(data);
     decode(data);
     return *this;
@@ -205,10 +206,10 @@ void message::inferred(bool b) { pn_message_set_inferred(pn_msg(), b); }
 const value& message::body() const { return impl_.body.ref(pn_message_body(pn_msg())); }
 value& message::body() { return impl_.body.ref(pn_message_body(pn_msg())); }
 
-// MAP CACHING: the properties, annotations and instructions maps can either be
-// encoded in the pn_message pn_data_t structures OR decoded as C++ map members
-// of the message but not both. At least one of the pn_data_t or the map member
-// is always empty, the non-empty one is the authority.
+// MAP CACHING: the properties and annotations maps can either be encoded in the
+// pn_message pn_data_t structures OR decoded as C++ map members of the message
+// but not both. At least one of the pn_data_t or the map member is always
+// empty, the non-empty one is the authority.
 
 // Decode a map on demand
 template<class M> M& get_map(pn_message_t* msg, pn_data_t* (*get)(pn_message_t*), M& map) {
@@ -230,41 +231,40 @@ template<class M> M& put_map(pn_message_t* msg, pn_data_t* (*get)(pn_message_t*)
     return map;
 }
 
-message::property_map& message::properties() {
-    return get_map(pn_msg(), pn_message_properties, properties_);
+message::property_map& message::application_properties() {
+    return get_map(pn_msg(), pn_message_properties, impl_.application_properties);
 }
 
-const message::property_map& message::properties() const {
-    return get_map(pn_msg(), pn_message_properties, properties_);
+const message::property_map& message::application_properties() const {
+    return get_map(pn_msg(), pn_message_properties, impl_.application_properties);
 }
 
 
-message::annotation_map& message::annotations() {
-    return get_map(pn_msg(), pn_message_annotations, annotations_);
+message::annotation_map& message::message_annotations() {
+    return get_map(pn_msg(), pn_message_annotations, impl_.message_annotations);
 }
 
-const message::annotation_map& message::annotations() const {
-    return get_map(pn_msg(), pn_message_annotations, annotations_);
+const message::annotation_map& message::message_annotations() const {
+    return get_map(pn_msg(), pn_message_annotations, impl_.message_annotations);
 }
 
 
-message::annotation_map& message::instructions() {
-    return get_map(pn_msg(), pn_message_instructions, instructions_);
+message::annotation_map& message::delivery_annotations() {
+    return get_map(pn_msg(), pn_message_instructions, impl_.delivery_annotations);
 }
 
-const message::annotation_map& message::instructions() const {
-    return get_map(pn_msg(), pn_message_instructions, instructions_);
+const message::annotation_map& message::delivery_annotations() const {
+    return get_map(pn_msg(), pn_message_instructions, impl_.delivery_annotations);
 }
 
-void message::encode(std::string &s) const {
-    put_map(pn_msg(), pn_message_properties, properties_);
-    put_map(pn_msg(), pn_message_annotations, annotations_);
-    put_map(pn_msg(), pn_message_instructions, instructions_);
-    size_t sz = s.capacity();
-    if (sz < 512) sz = 512;
+void message::encode(std::vector<char> &s) const {
+    put_map(pn_msg(), pn_message_properties, impl_.application_properties);
+    put_map(pn_msg(), pn_message_annotations, impl_.message_annotations);
+    put_map(pn_msg(), pn_message_instructions, impl_.delivery_annotations);
+    size_t sz = std::max(s.capacity(), size_t(512));
     while (true) {
         s.resize(sz);
-        int err = pn_message_encode(pn_msg(), const_cast<char*>(s.data()), &sz);
+        int err = pn_message_encode(pn_msg(), const_cast<char*>(&s[0]), &sz);
         if (err) {
             if (err != PN_OVERFLOW)
                 check(err);
@@ -276,23 +276,23 @@ void message::encode(std::string &s) const {
     }
 }
 
-std::string message::encode() const {
-    std::string data;
+std::vector<char> message::encode() const {
+    std::vector<char> data;
     encode(data);
     return data;
 }
 
-void message::decode(const std::string &s) {
-    properties_.clear();
-    annotations_.clear();
-    instructions_.clear();
-    check(pn_message_decode(pn_msg(), s.data(), s.size()));
+void message::decode(const std::vector<char> &s) {
+    impl_.application_properties.clear();
+    impl_.message_annotations.clear();
+    impl_.delivery_annotations.clear();
+    check(pn_message_decode(pn_msg(), &s[0], s.size()));
 }
 
 void message::decode(proton::link link, proton::delivery delivery) {
-    std::string buf;
+    std::vector<char> buf;
     buf.resize(delivery.pending());
-    ssize_t n = link.recv(const_cast<char *>(buf.data()), buf.size());
+    ssize_t n = link.recv(const_cast<char *>(&buf[0]), buf.size());
     if (n != ssize_t(buf.size())) throw error(MSG("link read failure"));
     clear();
     decode(buf);

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/06017111/proton-c/bindings/cpp/src/message_test.cpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/message_test.cpp b/proton-c/bindings/cpp/src/message_test.cpp
index d3509aa..0e25bb5 100644
--- a/proton-c/bindings/cpp/src/message_test.cpp
+++ b/proton-c/bindings/cpp/src/message_test.cpp
@@ -104,36 +104,36 @@ void test_message_body() {
 void test_message_maps() {
     message m;
 
-    ASSERT(m.properties().empty());
-    ASSERT(m.annotations().empty());
-    ASSERT(m.instructions().empty());
+    ASSERT(m.application_properties().empty());
+    ASSERT(m.message_annotations().empty());
+    ASSERT(m.delivery_annotations().empty());
 
-    m.properties()["foo"] = 12;
-    m.instructions()["bar"] = "xyz";
-    m.annotations()[23] = "23";
+    m.application_properties()["foo"] = 12;
+    m.delivery_annotations()["bar"] = "xyz";
 
-    ASSERT_EQUAL(m.properties()["foo"], scalar(12));
-    ASSERT_EQUAL(m.instructions()["bar"], scalar("xyz"));
-    ASSERT_EQUAL(m.annotations()[23], scalar("23"));
+    m.message_annotations()[23] = "23";
+    ASSERT_EQUAL(m.application_properties()["foo"], scalar(12));
+    ASSERT_EQUAL(m.delivery_annotations()["bar"], scalar("xyz"));
+    ASSERT_EQUAL(m.message_annotations()[23], scalar("23"));
 
     message m2(m);
-    message::annotation_map& amap = m2.instructions();
+    message::annotation_map& amap = m2.delivery_annotations();
 
-    ASSERT_EQUAL(m2.properties()["foo"], scalar(12));
-    ASSERT_EQUAL(m2.instructions()["bar"], scalar("xyz"));
-    ASSERT_EQUAL(m2.annotations()[23], scalar("23"));
+    ASSERT_EQUAL(m2.application_properties()["foo"], scalar(12));
+    ASSERT_EQUAL(m2.delivery_annotations()["bar"], scalar("xyz"));
+    ASSERT_EQUAL(m2.message_annotations()[23], scalar("23"));
 
-    m.properties()["foo"] = "newfoo";
-    m.instructions()[24] = 1000;
-    m.annotations().erase(23);
+    m.application_properties()["foo"] = "newfoo";
+    m.delivery_annotations()[24] = 1000;
+    m.message_annotations().erase(23);
 
     m2 = m;
-    ASSERT_EQUAL(1, m2.properties().size());
-    ASSERT_EQUAL(m2.properties()["foo"], scalar("newfoo"));
-    ASSERT_EQUAL(2, m2.instructions().size());
-    ASSERT_EQUAL(m2.instructions()["bar"], scalar("xyz"));
-    ASSERT_EQUAL(m2.instructions()[24], scalar(1000));
-    ASSERT(m2.annotations().empty());
+    ASSERT_EQUAL(1, m2.application_properties().size());
+    ASSERT_EQUAL(m2.application_properties()["foo"], scalar("newfoo"));
+    ASSERT_EQUAL(2, m2.delivery_annotations().size());
+    ASSERT_EQUAL(m2.delivery_annotations()["bar"], scalar("xyz"));
+    ASSERT_EQUAL(m2.delivery_annotations()[24], scalar(1000));
+    ASSERT(m2.message_annotations().empty());
 }
 
 int main(int argc, char** argv) {

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/06017111/proton-c/bindings/cpp/src/sender.cpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/sender.cpp b/proton-c/bindings/cpp/src/sender.cpp
index 56f737b..64b48ec 100644
--- a/proton-c/bindings/cpp/src/sender.cpp
+++ b/proton-c/bindings/cpp/src/sender.cpp
@@ -45,9 +45,9 @@ delivery sender::send(const message &message) {
     amqp_ulong id = ++tag_counter;
     pn_delivery_t *dlv =
         pn_delivery(pn_object(), pn_dtag(reinterpret_cast<const char*>(&id), sizeof(id)));
-    std::string buf;
+    std::vector<char> buf;
     message.encode(buf);
-    pn_link_send(pn_object(), buf.data(), buf.size());
+    pn_link_send(pn_object(), &buf[0], buf.size());
     pn_link_advance(pn_object());
     if (pn_link_snd_settle_mode(pn_object()) == PN_SND_SETTLED)
         pn_delivery_settle(dlv);

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/06017111/proton-c/bindings/cpp/src/ssl_domain.cpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/ssl_domain.cpp b/proton-c/bindings/cpp/src/ssl_domain.cpp
index 95fc552..29d1be5 100644
--- a/proton-c/bindings/cpp/src/ssl_domain.cpp
+++ b/proton-c/bindings/cpp/src/ssl_domain.cpp
@@ -59,13 +59,17 @@ ssl_domain::ssl_domain(const ssl_domain &x) {
     impl_ = x.impl_;
     impl_->incref();
 }
+
 ssl_domain& ssl_domain::operator=(const ssl_domain&x) {
-    ssl_domain(x).swap(*this);
+    impl_->decref();
+    impl_ = x.impl_;
+    impl_->incref();
     return *this;
 }
+
 ssl_domain::~ssl_domain() { impl_->decref(); }
 
-void ssl_domain::swap(ssl_domain &x) { std::swap(impl_, x.impl_); }
+// FIXME void ssl_domain::swap(ssl_domain &x) { std::swap(impl_, x.impl_); }
 
 pn_ssl_domain_t *ssl_domain::pn_domain() { return impl_->pn_domain(); }
 

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/06017111/proton-c/bindings/cpp/src/value.cpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/value.cpp b/proton-c/bindings/cpp/src/value.cpp
index bdceacc..0a0e08f 100644
--- a/proton-c/bindings/cpp/src/value.cpp
+++ b/proton-c/bindings/cpp/src/value.cpp
@@ -32,7 +32,7 @@ value::value() : data_(data::create()) {}
 value::value(const value& x) : data_(data::create()) { data_.copy(x.data_); }
 
 #if PN_HAS_CPP11
-value::value(value&& x) : data_(0) { swap(x); }
+value::value(value&& x) : data_(0) { swap(*this, x); }
 #endif
 
 // Referencing an external value
@@ -43,7 +43,7 @@ value& value::ref(data d) { data_ = d; return *this; }
 
 value& value::operator=(const value& x) { data_.copy(x.data_); return *this; }
 
-void value::swap(value& x) { std::swap(data_, x.data_); }
+void swap(value& x, value& y) { std::swap(x.data_, y.data_); }
 
 void value::clear() { data_.clear(); }
 


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