PROTON-1882: [cpp] cannot send message properties with null values

The is_scalar() function was returning false for NULL_TYPE values, so extracting a null into a proton::scalar threw a conversion_error. Fixed so NULL is now considered a scalar type. Also added additional tests for null values, and fixed some missing support for the C++11 nullptr.
alanconway · Jul 3, 2018 · 9e8edc1 · 9e8edc1
1 parent 2cf4f53
commit 9e8edc1
Show file tree

Hide file tree

Showing 10 changed files with 88 additions and 15 deletions.
diff --git a/cpp/include/proton/codec/decoder.hpp b/cpp/include/proton/codec/decoder.hpp
@@ -113,6 +113,9 @@ class decoder : public internal::data {
     PN_CPP_EXTERN decoder& operator>>(scalar&);
     PN_CPP_EXTERN decoder& operator>>(internal::value_base&);
     PN_CPP_EXTERN decoder& operator>>(null&);
+#if PN_CPP_HAS_NULLPTR
+    PN_CPP_EXTERN decoder& operator>>(decltype(nullptr)&);
+#endif
     ///@}
 
     /// Start decoding a container type, such as an ARRAY, LIST or

diff --git a/cpp/include/proton/internal/type_traits.hpp b/cpp/include/proton/internal/type_traits.hpp
@@ -93,6 +93,10 @@ template <type_id ID, class T> struct type_id_constant {
 /// @name Metafunction returning AMQP type for scalar C++ types.
 /// @{
 template <class T> struct type_id_of;
+template<> struct type_id_of<null> : public type_id_constant<NULL_TYPE, null> {};
+#if PN_CPP_HAS_NULLPTR
+template<> struct type_id_of<decltype(nullptr)> : public type_id_constant<NULL_TYPE, null> {};
+#endif
 template<> struct type_id_of<bool> : public type_id_constant<BOOLEAN, bool> {};
 template<> struct type_id_of<uint8_t> : public type_id_constant<UBYTE, uint8_t> {};
 template<> struct type_id_of<int8_t> : public type_id_constant<BYTE, int8_t> {};
@@ -113,9 +117,6 @@ template<> struct type_id_of<uuid> : public type_id_constant<UUID, uuid> {};
 template<> struct type_id_of<std::string> : public type_id_constant<STRING, std::string> {};
 template<> struct type_id_of<symbol> : public type_id_constant<SYMBOL, symbol> {};
 template<> struct type_id_of<binary> : public type_id_constant<BINARY, binary> {};
-#if PN_CPP_HAS_NULLPTR
-template<> struct type_id_of<decltype(nullptr)> : public type_id_constant<NULL_TYPE, null> {};
-#endif
 /// @}
 
 /// Metafunction to test if a class has a type_id.

diff --git a/cpp/include/proton/null.hpp b/cpp/include/proton/null.hpp
@@ -26,6 +26,7 @@
 /// @copybrief proton::null
 
 #include "./internal/config.hpp"
+#include "./internal/comparable.hpp"
 #include "./internal/export.hpp"
 
 #include <iosfwd>
@@ -35,13 +36,17 @@ namespace proton {
 /// The type of the AMQP null value
 ///
 /// @see @ref types_page
-class null {
+class null : private internal::comparable<null> {
   public:
     null() {}
 #if PN_CPP_HAS_NULLPTR
     /// Constructed from nullptr literal
     null(decltype(nullptr)) {}
 #endif
+    // null instances are always equal
+  friend bool operator==(const null&, const null&) { return true; }
+    // null instances are never unequal
+  friend bool operator<(const null&, const null&) { return false; }
 };
 
 /// Print a null value

diff --git a/cpp/include/proton/scalar_base.hpp b/cpp/include/proton/scalar_base.hpp
@@ -100,6 +100,9 @@ class scalar_base : private internal::comparable<scalar_base> {
     PN_CPP_EXTERN void put_(const binary&);
     PN_CPP_EXTERN void put_(const char* s); ///< Treated as an AMQP string
     PN_CPP_EXTERN void put_(const null&);
+#if PN_CPP_HAS_NULLPTR
+    PN_CPP_EXTERN void put_(decltype(nullptr));
+#endif
 
     template<class T> void put(const T& x) { putter<T>::put(*this, x); }
 
@@ -125,6 +128,10 @@ class scalar_base : private internal::comparable<scalar_base> {
     PN_CPP_EXTERN void get_(symbol&) const;
     PN_CPP_EXTERN void get_(binary&) const;
     PN_CPP_EXTERN void get_(null&) const;
+#if PN_CPP_HAS_NULLPTR
+    PN_CPP_EXTERN void get_(decltype(nullptr)&) const;
+#endif
+
 
     // use template structs, functions cannot be  partially specialized.
     template <class T, class Enable=void> struct putter {

diff --git a/cpp/include/proton/type_id.hpp b/cpp/include/proton/type_id.hpp
@@ -82,8 +82,19 @@ inline bool type_id_is_decimal(type_id t) { return t == DECIMAL32 || t == DECIMA
 inline bool type_id_is_signed(type_id t) { return type_id_is_signed_int(t) || type_id_is_floating_point(t) || type_id_is_decimal(t); }
 inline bool type_id_is_string_like(type_id t) { return t == BINARY || t == STRING || t == SYMBOL; }
 inline bool type_id_is_container(type_id t) { return t == LIST || t == MAP || t == ARRAY || t == DESCRIBED; }
-inline bool type_id_is_scalar(type_id t) { return type_id_is_integral(t) || type_id_is_floating_point(t) || type_id_is_decimal(t) || type_id_is_string_like(t) || t == TIMESTAMP || t == UUID; }
+
 inline bool type_id_is_null(type_id t) { return t == NULL_TYPE; }
+
+inline bool type_id_is_scalar(type_id t) {
+    return type_id_is_integral(t) ||
+        type_id_is_floating_point(t) ||
+        type_id_is_decimal(t) ||
+        type_id_is_string_like(t) ||
+        type_id_is_null(t) ||
+        t == TIMESTAMP ||
+        t == UUID;
+}
+
 /// }
 
 } // proton

diff --git a/cpp/include/proton/value.hpp b/cpp/include/proton/value.hpp
@@ -159,6 +159,11 @@ template<class T> void coerce(const value& v, T& x) {
 template<> inline void get<null>(const value& v, null&) {
     assert_type_equal(NULL_TYPE, v.type());
 }
+#if PN_CPP_HAS_NULLPTR
+template<> inline void get<decltype(nullptr)>(const value& v, decltype(nullptr)&) {
+    assert_type_equal(NULL_TYPE, v.type());
+}
+#endif
 
 /// Return a readable string representation of x for display purposes.
 PN_CPP_EXTERN std::string to_string(const value& x);

diff --git a/cpp/src/codec_test.cpp b/cpp/src/codec_test.cpp
@@ -53,6 +53,7 @@ int main(int, char**) {
     int failed = 0;
 
     // Basic AMQP types
+    RUN_TEST(failed, simple_type_test(null()));
     RUN_TEST(failed, simple_type_test(false));
     RUN_TEST(failed, simple_type_test(uint8_t(42)));
     RUN_TEST(failed, simple_type_test(int8_t(-42)));

diff --git a/cpp/src/decoder.cpp b/cpp/src/decoder.cpp
@@ -141,6 +141,14 @@ decoder& decoder::operator>>(null&) {
     return *this;
 }
 
+#if PN_CPP_HAS_NULLPTR
+decoder& decoder::operator>>(decltype(nullptr)&) {
+    internal::state_guard sg(*this);
+    assert_type_equal(NULL_TYPE, pre_get());
+    return *this;
+}
+#endif
+
 decoder& decoder::operator>>(internal::value_base& x) {
     if (*this == x.data_)
         throw conversion_error("extract into self");

diff --git a/cpp/src/scalar_base.cpp b/cpp/src/scalar_base.cpp
@@ -84,6 +84,10 @@ void scalar_base::put_(const symbol& x) { set(binary(x), PN_SYMBOL); }
 void scalar_base::put_(const binary& x) { set(x, PN_BINARY); }
 void scalar_base::put_(const char* x) { set(binary(std::string(x)), PN_STRING); }
 void scalar_base::put_(const null&) { atom_.type = PN_NULL; }
+#if PN_CPP_HAS_NULLPTR
+void scalar_base::put_(decltype(nullptr)) { atom_.type = PN_NULL; }
+#endif
+
 
 void scalar_base::ok(pn_type_t t) const {
     if (atom_.type != t) throw make_conversion_error(type_id(t), type());
@@ -110,6 +114,9 @@ void scalar_base::get_(std::string& x) const { ok(PN_STRING); x = std::string(by
 void scalar_base::get_(symbol& x) const { ok(PN_SYMBOL); x = symbol(bytes_.begin(), bytes_.end()); }
 void scalar_base::get_(binary& x) const { ok(PN_BINARY); x = bytes_; }
 void scalar_base::get_(null&) const { ok(PN_NULL); }
+#if PN_CPP_HAS_NULLPTR
+void scalar_base::get_(decltype(nullptr)&) const { ok(PN_NULL); }
+#endif
 
 namespace {
 
@@ -147,7 +154,7 @@ bool operator<(const scalar_base& x, const scalar_base& y) {
 
 std::ostream& operator<<(std::ostream& o, const scalar_base& s) {
     switch (s.type()) {
-      case NULL_TYPE: return o << "<null>";
+      case NULL_TYPE: return o << "null";
       case BYTE: return o << static_cast<int>(internal::get<int8_t>(s));
       case UBYTE: return o << static_cast<unsigned int>(internal::get<uint8_t>(s));
         // Other types printed using normal C++ operator <<

diff --git a/cpp/src/value_test.cpp b/cpp/src/value_test.cpp
@@ -76,25 +76,50 @@ template <class T, class U> void map_test(const U& values, const string& s) {
         ASSERT_EQUAL(s, to_string(v));
 }
 
-#if PN_CPP_HAS_CPP11
 void null_test() {
-    auto n = null();
-    proton::value nn = nullptr;
-    ASSERT_EQUAL(n, nn);
-    ASSERT_EQUAL("null", to_string(nn));
+    proton::null n;
     ASSERT_EQUAL("null", to_string(n));
-    std::vector<proton::value> nulls = {nullptr, null{}};
+
+    std::vector<proton::value> nulls(2, n);
     ASSERT_EQUAL("[null, null]", to_string(nulls));
 
-    std::vector<std::nullptr_t> nulls1 = {nullptr, nullptr};
+    std::vector<proton::null> nulls1(2, n);
     ASSERT_EQUAL("@PN_NULL[null, null]", to_string(nulls1));
 
-    std::vector<proton::value> vs = {nullptr, nulls, nulls1};
+    std::vector<proton::value> vs;
+    vs.push_back(n);
+    vs.push_back(nulls);
+    vs.push_back(nulls1);
     ASSERT_EQUAL("[null, [null, null], @PN_NULL[null, null]]", to_string(vs));
-}
+
+    std::map<proton::value, proton::value> vm;
+    vm[n] = 1;
+    vm[nulls1] = 2;
+    vm[nulls] = 3;
+    // FIXME aconway 2018-06-29: note different types compare by type-id,
+    // so NULL < ARRAY < LIST
+    ASSERT_EQUAL("{null=1, @PN_NULL[null, null]=2, [null, null]=3}", to_string(vm));
+
+    std::map<proton::scalar, proton::scalar> vm2;
+    vm2[n] = 1;                 // Different types compare by type-id, NULL is smallest
+    vm2[2] = n;
+    ASSERT_EQUAL("{null=1, 2=null}", to_string(vm2));
+
+#if PN_CPP_HAS_CPP11
+    proton::value nn = nullptr;
+    ASSERT(n == nn);            // Don't use ASSERT_EQUAL, it will try to print
+    ASSERT_EQUAL("null", to_string(nn));
+    std::vector<std::nullptr_t> nulls2 {nullptr, nullptr};
+    ASSERT_EQUAL("@PN_NULL[null, null]", to_string(nulls2));
+    std::map<proton::scalar, proton::scalar> m {{nullptr, nullptr}};
+    ASSERT_EQUAL("{null=null}", to_string(m));
+    std::map<proton::value, proton::value> m2 {{nullptr, nullptr}};
+    ASSERT_EQUAL("{null=null}", to_string(m2));
 #endif
 }
 
+}
+
 int main(int, char**) {
     try {
         int failed = 0;