diff --git a/compound.hh b/compound.hh index bb7f02a9b0..4d25778d4b 100644 --- a/compound.hh +++ b/compound.hh @@ -32,7 +32,7 @@ struct value_traits { } }; -enum class allow_prefixes { yes, no }; +enum class allow_prefixes { no, yes }; template class compound_type final { @@ -66,8 +66,8 @@ public: * Format: * ...(len(value_n))? * - * The value corresponding to the last component of types doesn't have its length encoded, - * its length is deduced from the input range. + * For non-prefixable compounds, the value corresponding to the last component of types doesn't + * have its length encoded, its length is deduced from the input range. * * serialize_value() and serialize_optionals() for single element rely on the fact that for a single-element * compounds their serialized form is equal to the serialized form of the component. @@ -84,7 +84,7 @@ public: for (auto&& wrapped : values) { auto&& val = value_traits::unwrap(wrapped); assert(val.size() <= std::numeric_limits::max()); - if (--n_left) { + if (--n_left || AllowPrefixes == allow_prefixes::yes) { write(out, uint16_t(val.size())); } out = std::copy(val.begin(), val.end(), out); @@ -97,7 +97,7 @@ public: for (auto&& wrapped : values) { auto&& val = value_traits::unwrap(wrapped); assert(val.size() <= std::numeric_limits::max()); - if (--n_left) { + if (--n_left || AllowPrefixes == allow_prefixes::yes) { len += sizeof(uint16_t); } len += val.size(); @@ -107,21 +107,20 @@ public: bytes serialize_single(bytes&& v) { if (AllowPrefixes == allow_prefixes::no) { assert(_types.size() == 1); + return std::move(v); } else { - if (_types.size() > 1) { - std::vector vec; - vec.reserve(1); - vec.emplace_back(std::move(v)); - return ::serialize_value(*this, vec); - } + // FIXME: Optimize + std::vector vec; + vec.reserve(1); + vec.emplace_back(std::move(v)); + return ::serialize_value(*this, vec); } - return std::move(v); } bytes serialize_value(const std::vector& values) { return ::serialize_value(*this, values); } bytes serialize_value(std::vector&& values) { - if (_types.size() == 1 && values.size() == 1) { + if (AllowPrefixes == allow_prefixes::no && _types.size() == 1 && values.size() == 1) { return std::move(values[0]); } return ::serialize_value(*this, values); @@ -130,7 +129,7 @@ public: return ::serialize_value(*this, values); } bytes serialize_optionals(std::vector&& values) { - if (_types.size() == 1 && values.size() == 1) { + if (AllowPrefixes == allow_prefixes::no && _types.size() == 1 && values.size() == 1) { assert(values[0]); return std::move(*values[0]); } @@ -164,18 +163,19 @@ public: _v = bytes_view(nullptr, 0); return; } - if (_v.empty()) { - if (AllowPrefixes == allow_prefixes::yes) { - _v = bytes_view(nullptr, 0); - return; - } else { - throw marshal_exception(); - } - } + --_types_left; uint16_t len; - if (_types_left == 1) { + if (_types_left == 0 && AllowPrefixes == allow_prefixes::no) { len = _v.size(); } else { + if (_v.empty()) { + if (AllowPrefixes == allow_prefixes::yes) { + _v = bytes_view(nullptr, 0); + return; + } else { + throw marshal_exception(); + } + } len = read_simple(_v); if (_v.size() < len) { throw marshal_exception(); @@ -191,7 +191,6 @@ public: } iterator(end_iterator_tag, const bytes_view& v) : _v(nullptr, 0) {} iterator& operator++() { - --_types_left; read_current(); return *this; } diff --git a/configure.py b/configure.py index be6a4b619e..1a7089f0c4 100755 --- a/configure.py +++ b/configure.py @@ -169,6 +169,7 @@ urchin_tests = [ 'tests/urchin/serializer_test', 'tests/urchin/message', 'tests/urchin/gossip', + 'tests/urchin/compound_test', ] tests = [ diff --git a/test.py b/test.py index 72fc9821c6..2498ae9725 100755 --- a/test.py +++ b/test.py @@ -41,6 +41,7 @@ boost_tests = [ 'test-serialization', 'cartesian_product_test', 'urchin/UUID_test', + 'urchin/compound_test', ] other_tests = [ diff --git a/tests/urchin/compound_test.cc b/tests/urchin/compound_test.cc new file mode 100644 index 0000000000..1f06cdea89 --- /dev/null +++ b/tests/urchin/compound_test.cc @@ -0,0 +1,148 @@ +/* + * Copyright 2015 Cloudius Systems + */ + +#define BOOST_TEST_DYN_LINK +#define BOOST_TEST_MODULE core + +#include +#include "compound.hh" +#include "tests/urchin/range_assert.hh" + +static std::vector to_bytes_vec(std::vector values) { + std::vector result; + for (auto&& v : values) { + result.emplace_back(to_bytes(v)); + } + return result; +} + +template +static +range_assert +assert_that_components(Compound& t, bytes packed) { + return assert_that_range(t.begin(packed), t.end(packed)); +} + +template +static void test_sequence(Compound& t, std::vector strings) { + auto packed = t.serialize_value(to_bytes_vec(strings)); + assert_that_components(t, packed).equals(to_bytes_vec(strings)); +}; + +BOOST_AUTO_TEST_CASE(test_iteration_over_non_prefixable_tuple) { + compound_type t({bytes_type, bytes_type, bytes_type}); + + test_sequence(t, {"el1", "el2", "el3"}); + test_sequence(t, {"el1", "el2", ""}); + test_sequence(t, {"", "el2", "el3"}); + test_sequence(t, {"el1", "", ""}); + test_sequence(t, {"", "", "el3"}); + test_sequence(t, {"el1", "", "el3"}); + test_sequence(t, {"", "", ""}); +} + +BOOST_AUTO_TEST_CASE(test_iteration_over_prefixable_tuple) { + compound_type t({bytes_type, bytes_type, bytes_type}); + + test_sequence(t, {"el1", "el2", "el3"}); + test_sequence(t, {"el1", "el2", ""}); + test_sequence(t, {"", "el2", "el3"}); + test_sequence(t, {"el1", "", ""}); + test_sequence(t, {"", "", "el3"}); + test_sequence(t, {"el1", "", "el3"}); + test_sequence(t, {"", "", ""}); + + test_sequence(t, {"el1", "el2", ""}); + test_sequence(t, {"el1", "el2"}); + test_sequence(t, {"el1", ""}); + test_sequence(t, {"el1"}); + test_sequence(t, {""}); + test_sequence(t, {}); +} + +BOOST_AUTO_TEST_CASE(test_iteration_over_non_prefixable_singular_tuple) { + compound_type t({bytes_type}); + + test_sequence(t, {"el1"}); + test_sequence(t, {""}); +} + +BOOST_AUTO_TEST_CASE(test_iteration_over_prefixable_singular_tuple) { + compound_type t({bytes_type}); + + test_sequence(t, {"elem1"}); + test_sequence(t, {""}); + test_sequence(t, {}); +} + +template +void do_test_conversion_methods_for_singular_compound() { + compound_type t({bytes_type}); + + { + assert_that_components(t, t.serialize_value(to_bytes_vec({"asd"}))) // r-value version + .equals(to_bytes_vec({"asd"})); + } + + { + auto vec = to_bytes_vec({"asd"}); + assert_that_components(t, t.serialize_value(vec)) // l-value version + .equals(to_bytes_vec({"asd"})); + } + + { + std::vector vec = { bytes_opt("asd") }; + assert_that_components(t, t.serialize_optionals(vec)) + .equals(to_bytes_vec({"asd"})); + } + + { + std::vector vec = { bytes_opt("asd") }; + assert_that_components(t, t.serialize_optionals(std::move(vec))) // r-value + .equals(to_bytes_vec({"asd"})); + } + + { + assert_that_components(t, t.serialize_single(bytes("asd"))) + .equals(to_bytes_vec({"asd"})); + } +} + +BOOST_AUTO_TEST_CASE(test_conversion_methods_for_singular_compound) { + do_test_conversion_methods_for_singular_compound(); + do_test_conversion_methods_for_singular_compound(); +} + +template +void do_test_conversion_methods_for_non_singular_compound() { + compound_type t({bytes_type, bytes_type, bytes_type}); + + { + assert_that_components(t, t.serialize_value(to_bytes_vec({"el1", "el2", "el2"}))) // r-value version + .equals(to_bytes_vec({"el1", "el2", "el2"})); + } + + { + auto vec = to_bytes_vec({"el1", "el2", "el3"}); + assert_that_components(t, t.serialize_value(vec)) // l-value version + .equals(to_bytes_vec({"el1", "el2", "el3"})); + } + + { + std::vector vec = { bytes_opt("el1"), bytes_opt("el2"), bytes_opt("el3") }; + assert_that_components(t, t.serialize_optionals(vec)) + .equals(to_bytes_vec({"el1", "el2", "el3"})); + } + + { + std::vector vec = { bytes_opt("el1"), bytes_opt("el2"), bytes_opt("el3") }; + assert_that_components(t, t.serialize_optionals(std::move(vec))) // r-value + .equals(to_bytes_vec({"el1", "el2", "el3"})); + } +} + +BOOST_AUTO_TEST_CASE(test_conversion_methods_for_non_singular_compound) { + do_test_conversion_methods_for_non_singular_compound(); + do_test_conversion_methods_for_non_singular_compound(); +} diff --git a/tests/urchin/range_assert.hh b/tests/urchin/range_assert.hh new file mode 100644 index 0000000000..a6dd29c53e --- /dev/null +++ b/tests/urchin/range_assert.hh @@ -0,0 +1,43 @@ +/* + * Copyright 2015 Cloudius Systems + */ + +#pragma once + +#include +#include + +template +class range_assert { + using value_type = typename Iterator::value_type; + Iterator _begin; + Iterator _end; +public: + range_assert(Iterator begin, Iterator end) + : _begin(begin) + , _end(end) + { } + + template + range_assert equals(std::vector expected) { + auto i = _begin; + auto expected_i = expected.begin(); + while (i != _end && expected_i != expected.end()) { + BOOST_REQUIRE(*i == *expected_i); + ++i; + ++expected_i; + } + if (i != _end) { + BOOST_FAIL("Expected fewer elements"); + } + if (expected_i != expected.end()) { + BOOST_FAIL("Expected more elements"); + } + return *this; + } +}; + +template +static range_assert assert_that_range(Iterator begin, Iterator end) { + return { begin, end }; +}