mirror of
https://github.com/scylladb/scylladb.git
synced 2026-04-24 10:30:38 +00:00
Merge branch 'tgrabiec/compound-fixes' of github.com:cloudius-systems/seastar-dev into db
Bug fixes in the compound class, from Tomasz.
This commit is contained in:
47
compound.hh
47
compound.hh
@@ -32,7 +32,7 @@ struct value_traits<bytes_opt> {
|
||||
}
|
||||
};
|
||||
|
||||
enum class allow_prefixes { yes, no };
|
||||
enum class allow_prefixes { no, yes };
|
||||
|
||||
template<allow_prefixes AllowPrefixes = allow_prefixes::no>
|
||||
class compound_type final {
|
||||
@@ -66,8 +66,8 @@ public:
|
||||
* Format:
|
||||
* <len(value1)><value1><len(value2)><value2>...<len(value_n-1)><value_n-1>(len(value_n))?<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<Wrapped>::unwrap(wrapped);
|
||||
assert(val.size() <= std::numeric_limits<uint16_t>::max());
|
||||
if (--n_left) {
|
||||
if (--n_left || AllowPrefixes == allow_prefixes::yes) {
|
||||
write<uint16_t>(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<Wrapped>::unwrap(wrapped);
|
||||
assert(val.size() <= std::numeric_limits<uint16_t>::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<bytes> vec;
|
||||
vec.reserve(1);
|
||||
vec.emplace_back(std::move(v));
|
||||
return ::serialize_value(*this, vec);
|
||||
}
|
||||
// FIXME: Optimize
|
||||
std::vector<bytes> 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<bytes>& values) {
|
||||
return ::serialize_value(*this, values);
|
||||
}
|
||||
bytes serialize_value(std::vector<bytes>&& 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<bytes_opt>&& 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<uint16_t>(_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;
|
||||
}
|
||||
|
||||
@@ -169,6 +169,7 @@ urchin_tests = [
|
||||
'tests/urchin/serializer_test',
|
||||
'tests/urchin/message',
|
||||
'tests/urchin/gossip',
|
||||
'tests/urchin/compound_test',
|
||||
]
|
||||
|
||||
tests = [
|
||||
|
||||
1
test.py
1
test.py
@@ -41,6 +41,7 @@ boost_tests = [
|
||||
'test-serialization',
|
||||
'cartesian_product_test',
|
||||
'urchin/UUID_test',
|
||||
'urchin/compound_test',
|
||||
]
|
||||
|
||||
other_tests = [
|
||||
|
||||
148
tests/urchin/compound_test.cc
Normal file
148
tests/urchin/compound_test.cc
Normal file
@@ -0,0 +1,148 @@
|
||||
/*
|
||||
* Copyright 2015 Cloudius Systems
|
||||
*/
|
||||
|
||||
#define BOOST_TEST_DYN_LINK
|
||||
#define BOOST_TEST_MODULE core
|
||||
|
||||
#include <boost/test/unit_test.hpp>
|
||||
#include "compound.hh"
|
||||
#include "tests/urchin/range_assert.hh"
|
||||
|
||||
static std::vector<bytes> to_bytes_vec(std::vector<sstring> values) {
|
||||
std::vector<bytes> result;
|
||||
for (auto&& v : values) {
|
||||
result.emplace_back(to_bytes(v));
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
template <typename Compound>
|
||||
static
|
||||
range_assert<typename Compound::iterator>
|
||||
assert_that_components(Compound& t, bytes packed) {
|
||||
return assert_that_range(t.begin(packed), t.end(packed));
|
||||
}
|
||||
|
||||
template <typename Compound>
|
||||
static void test_sequence(Compound& t, std::vector<sstring> 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<allow_prefixes::no> 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<allow_prefixes::yes> 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<allow_prefixes::no> t({bytes_type});
|
||||
|
||||
test_sequence(t, {"el1"});
|
||||
test_sequence(t, {""});
|
||||
}
|
||||
|
||||
BOOST_AUTO_TEST_CASE(test_iteration_over_prefixable_singular_tuple) {
|
||||
compound_type<allow_prefixes::yes> t({bytes_type});
|
||||
|
||||
test_sequence(t, {"elem1"});
|
||||
test_sequence(t, {""});
|
||||
test_sequence(t, {});
|
||||
}
|
||||
|
||||
template <allow_prefixes AllowPrefixes>
|
||||
void do_test_conversion_methods_for_singular_compound() {
|
||||
compound_type<AllowPrefixes> 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<bytes_opt> vec = { bytes_opt("asd") };
|
||||
assert_that_components(t, t.serialize_optionals(vec))
|
||||
.equals(to_bytes_vec({"asd"}));
|
||||
}
|
||||
|
||||
{
|
||||
std::vector<bytes_opt> 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<allow_prefixes::yes>();
|
||||
do_test_conversion_methods_for_singular_compound<allow_prefixes::no>();
|
||||
}
|
||||
|
||||
template <allow_prefixes AllowPrefixes>
|
||||
void do_test_conversion_methods_for_non_singular_compound() {
|
||||
compound_type<AllowPrefixes> 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<bytes_opt> 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<bytes_opt> 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<allow_prefixes::yes>();
|
||||
do_test_conversion_methods_for_non_singular_compound<allow_prefixes::no>();
|
||||
}
|
||||
43
tests/urchin/range_assert.hh
Normal file
43
tests/urchin/range_assert.hh
Normal file
@@ -0,0 +1,43 @@
|
||||
/*
|
||||
* Copyright 2015 Cloudius Systems
|
||||
*/
|
||||
|
||||
#pragma once
|
||||
|
||||
#include <boost/test/unit_test.hpp>
|
||||
#include <vector>
|
||||
|
||||
template <typename Iterator>
|
||||
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 <typename ValueType>
|
||||
range_assert equals(std::vector<ValueType> 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 <typename Iterator>
|
||||
static range_assert<Iterator> assert_that_range(Iterator begin, Iterator end) {
|
||||
return { begin, end };
|
||||
}
|
||||
Reference in New Issue
Block a user