Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 0 additions & 35 deletions include/rfl/cbor/Parser.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,41 +6,6 @@
#include "Reader.hpp"
#include "Writer.hpp"

namespace rfl::parsing {

/// CBOR requires us to explicitly set the number of fields in advance.
/// Because of that, we require all of the fields and then set them to nullptr,
/// if necessary.
template <class ProcessorsType, class... FieldTypes>
requires AreReaderAndWriter<cbor::Reader, cbor::Writer,
NamedTuple<FieldTypes...>>
struct Parser<cbor::Reader, cbor::Writer, NamedTuple<FieldTypes...>,
ProcessorsType>
: public NamedTupleParser<
cbor::Reader, cbor::Writer,
/*_ignore_empty_containers=*/false,
/*_all_required=*/true,
/*_no_field_names=*/ProcessorsType::no_field_names_, ProcessorsType,
FieldTypes...> {};

template <class ProcessorsType, class... Ts>
requires AreReaderAndWriter<cbor::Reader, cbor::Writer, rfl::Tuple<Ts...>>
struct Parser<cbor::Reader, cbor::Writer, rfl::Tuple<Ts...>, ProcessorsType>
: public TupleParser<cbor::Reader, cbor::Writer,
/*_ignore_empty_containers=*/false,
/*_all_required=*/true, ProcessorsType,
rfl::Tuple<Ts...>> {};

template <class ProcessorsType, class... Ts>
requires AreReaderAndWriter<cbor::Reader, cbor::Writer, std::tuple<Ts...>>
struct Parser<cbor::Reader, cbor::Writer, std::tuple<Ts...>, ProcessorsType>
: public TupleParser<cbor::Reader, cbor::Writer,
/*_ignore_empty_containers=*/false,
/*_all_required=*/true, ProcessorsType,
std::tuple<Ts...>> {};

} // namespace rfl::parsing

namespace rfl::cbor {

template <class T, class ProcessorsType>
Expand Down
8 changes: 4 additions & 4 deletions include/rfl/cbor/Writer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class Writer {

OutputArrayType array_as_root(const size_t _size) const noexcept;

OutputObjectType object_as_root(const size_t _size) const noexcept;
OutputObjectType object_as_root(const size_t) const noexcept;

OutputVarType null_as_root() const noexcept;

Expand All @@ -57,11 +57,11 @@ class Writer {
const size_t _size,
OutputObjectType* _parent) const noexcept;

OutputObjectType add_object_to_array(const size_t _size,
OutputObjectType add_object_to_array(const size_t,
OutputArrayType* _parent) const noexcept;

OutputObjectType add_object_to_object(
const std::string_view& _name, const size_t _size,
const std::string_view& _name, const size_t,
OutputObjectType* _parent) const noexcept;

template <class T>
Expand Down Expand Up @@ -90,7 +90,7 @@ class Writer {
private:
OutputArrayType new_array(const size_t _size) const noexcept;

OutputObjectType new_object(const size_t _size) const noexcept;
OutputObjectType new_object() const noexcept;

template <class T>
OutputVarType new_value(const T& _var) const noexcept {
Expand Down
16 changes: 8 additions & 8 deletions src/rfl/cbor/Writer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ Writer::OutputArrayType Writer::array_as_root(
}

Writer::OutputObjectType Writer::object_as_root(
const size_t _size) const noexcept {
return new_object(_size);
const size_t) const noexcept {
return new_object();
}

Writer::OutputVarType Writer::null_as_root() const noexcept {
Expand All @@ -34,15 +34,15 @@ Writer::OutputArrayType Writer::add_array_to_object(
}

Writer::OutputObjectType Writer::add_object_to_array(
const size_t _size, OutputArrayType* _parent) const noexcept {
return new_object(_size);
const size_t, OutputArrayType* _parent) const noexcept {
return new_object();
}

Writer::OutputObjectType Writer::add_object_to_object(
const std::string_view& _name, const size_t _size,
const std::string_view& _name, const size_t,
OutputObjectType* _parent) const noexcept {
encoder_->key(_name);
return new_object(_size);
return new_object();
}

Writer::OutputVarType Writer::add_null_to_array(
Expand Down Expand Up @@ -71,8 +71,8 @@ Writer::OutputArrayType Writer::new_array(const size_t _size) const noexcept {
return OutputArrayType{};
}

Writer::OutputObjectType Writer::new_object(const size_t _size) const noexcept {
encoder_->begin_object(_size);
Writer::OutputObjectType Writer::new_object() const noexcept {
encoder_->begin_object();
return OutputObjectType{};
}

Expand Down
6 changes: 5 additions & 1 deletion tests/cbor/test_error_messages.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,11 @@ TEST(cbor, test_decode_error_without_exception) {
});

// A proposal: A generic prefix, followed by the underlying library's error output
const std::string expected = R"(Could not parse CBOR: An unknown type was found in the stream at position 1)";
const std::string expected = R"(Found 4 errors:
1) Field named 'firstName' not found.
2) Field named 'lastName' not found.
3) Field named 'birthday' not found.
4) Field named 'children' not found.)";
EXPECT_EQ(result.error().what(), expected);

EXPECT_TRUE(!result.has_value() && true);
Expand Down
12 changes: 7 additions & 5 deletions tests/cbor/test_integers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,21 @@ TEST(cbor, test_integers_signedness) {

std::vector<char> unsigned_buffer = rfl::cbor::write(Unsigned{BIG_INT});
std::vector<unsigned char> unsigned_expected = {
0xA1, 0x63, 0x75, 0x36, 0x34,
0x1B, // Per RFC 8949, Initial byte '0x1B' indicates "unsigned integer (eight-byte uint64_t follows)"
0xBF, 0x63, 0x75, 0x36, 0x34,
0x1B, // Per RFC 8949, Initial byte '0x1B' indicates "unsigned integer (eight-byte uint64_t follows)"
// See: https://www.rfc-editor.org/rfc/rfc8949.html#section-appendix.b
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
0xFF
};

EXPECT_EQ(std::vector<char>(unsigned_expected.begin(), unsigned_expected.end()), unsigned_buffer);

std::vector<char> signed_buffer = rfl::cbor::write(Signed{static_cast<int64_t>(BIG_INT)});
std::vector<unsigned char> signed_expected = {
0xA1, 0x63, 0x69, 0x36, 0x34,
0x20 // Per RFC 8949, Initial byte '0x20' indicates "negative integer -1-0x00..-1-0x17 (-1..-24)"
0xBF, 0x63, 0x69, 0x36, 0x34,
0x20, // Per RFC 8949, Initial byte '0x20' indicates "negative integer -1-0x00..-1-0x17 (-1..-24)"
// See: https://www.rfc-editor.org/rfc/rfc8949.html#section-appendix.b
0xFF
};

EXPECT_EQ(std::vector<char>(signed_expected.begin(), signed_expected.end()), signed_buffer);
Expand Down
28 changes: 28 additions & 0 deletions tests/cbor/test_no_optionals.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
#include <rfl.hpp>
#include <rfl/cbor.hpp>
#include <string>
#include <vector>

#include "write_and_read.hpp"

namespace test_no_optionals {

struct Person {
rfl::Rename<"firstName", std::string> first_name;
rfl::Rename<"lastName", std::string> last_name = "Simpson";
rfl::Rename<"children", std::optional<std::vector<Person>>> children;
};
Comment on lines +10 to +14
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The Person struct is also defined in tests/cbor/test_optional_fields.cpp. Consider moving this common struct definition to a shared header file to avoid code duplication and improve maintainability.


struct OptionalPerson {
std::optional<Person> opt_person;
};

struct Empty{};

TEST(cbor, test_no_optionals) {
auto empty_struct_cbor = rfl::cbor::write<rfl::NoOptionals>(Empty{});
auto no_person_cbor = rfl::cbor::write<rfl::NoOptionals>(OptionalPerson{});

EXPECT_NE(empty_struct_cbor.size(), no_person_cbor.size());
}
} // namespace test_no_optionals
62 changes: 62 additions & 0 deletions tests/cbor/test_optional_fields.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
#include <iostream>
#include <rfl.hpp>
#include <rfl/cbor.hpp>
#include <string>
#include <vector>

#include "write_and_read.hpp"

namespace test_optional_fields {

struct Person {
rfl::Rename<"firstName", std::string> first_name;
rfl::Rename<"lastName", std::string> last_name = "Simpson";
rfl::Rename<"children", std::optional<std::vector<Person>>> children;

bool operator==(const Person& other) const {
return first_name.get() == other.first_name.get() &&
last_name.get() == other.last_name.get() &&
children.get() == other.children.get();
}
Comment on lines +16 to +20
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Use the C++20 default comparison operator:

Suggested change
bool operator==(const Person& other) const {
return first_name.get() == other.first_name.get() &&
last_name.get() == other.last_name.get() &&
children.get() == other.children.get();
}
bool operator==(const Person& other) const = default;

Although now that I'm thinking about it, perhaps this was done because the default comparison operator is not able to use the const Type& operator()() that is defined for the rfl::Rename<Type> member types... That would be unfortunate.

If that's the case, it would be helpful to briefly document that fact here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although now that I'm thinking about it, perhaps this was done because the default comparison operator is not able to use the const Type& operator()() that is defined for the rfl::Rename<Type> member types... That would be unfortunate.

Yep, exactly

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@liuzicheng1987 @dcorbeil I wonder if there is some comparison operator that rfl::Rename<Type> could implement to "pass through" default comparison operator support to its type.

};

struct OptionalPerson {
std::optional<Person> opt_person;
};

struct Empty{};

TEST(cbor, test_optional_no_fields) {
auto empty_struct_cbor = rfl::cbor::write(Empty{});
auto no_person_cbor = rfl::cbor::write(OptionalPerson{});

EXPECT_EQ(empty_struct_cbor.size(), no_person_cbor.size());
}

TEST(cbor, test_optional_fields) {
const auto bart = Person{.first_name = "Bart"};

const auto lisa = Person{.first_name = "Lisa"};

const auto maggie = Person{.first_name = "Maggie"};

const auto homer =
Person{.first_name = "Homer",
.children = std::vector<Person>({bart, lisa, maggie})};

const OptionalPerson homer_optional = {
.opt_person = homer
};

const auto homer_optional_cbor = rfl::cbor::write(homer_optional);
const auto res = rfl::cbor::read<OptionalPerson>(homer_optional_cbor);

EXPECT_TRUE(res && true) << "Test failed on read. Error: "
<< res.error().what();

const auto actual_homer = res.value();

EXPECT_TRUE(actual_homer.opt_person.has_value());
EXPECT_EQ(*actual_homer.opt_person, homer);
}
} // namespace test_optional_fields
Loading