diff --git a/include/rfl/AllOf.hpp b/include/rfl/AllOf.hpp index ca25c887..5b61bb2b 100644 --- a/include/rfl/AllOf.hpp +++ b/include/rfl/AllOf.hpp @@ -1,6 +1,7 @@ #ifndef RFL_ALLOF_HPP_ #define RFL_ALLOF_HPP_ +#include #include #include "Result.hpp" @@ -16,7 +17,7 @@ struct AllOf { /// @return The value if all constraints pass, otherwise an error template static rfl::Result validate(T _value) noexcept { - return validate_impl(_value); + return validate_impl(std::move(_value)); } /// Converts the validator to a JSON schema type. @@ -33,9 +34,13 @@ struct AllOf { template static rfl::Result validate_impl(T _value) noexcept { if constexpr (sizeof...(Tail) == 0) { - return Head::validate(_value); + return Head::validate(std::move(_value)); } else { - return Head::validate(_value).and_then(validate_impl); + auto res = Head::validate(std::move(_value)); + if (!res) { + return res; + } + return validate_impl(std::move(*res)); } } }; diff --git a/include/rfl/PatternValidator.hpp b/include/rfl/PatternValidator.hpp index 6c74d709..b1dcbc04 100644 --- a/include/rfl/PatternValidator.hpp +++ b/include/rfl/PatternValidator.hpp @@ -3,6 +3,7 @@ #include #include +#include #if __has_include() #include @@ -29,10 +30,10 @@ struct PatternValidator { /// Validates that a string matches the regex pattern. /// @param _str The string to validate /// @return Result containing the string if it matches, or an error if it doesn't - static Result validate(const std::string& _str) noexcept { + static Result validate(std::string _str) noexcept { if (ctre::match{ ctll::construct_from_pointer, _regex.arr_.data()}>(_str)) { - return _str; + return std::move(_str); } else { std::stringstream stream; stream << "String '" << _str << "' did not match format '" << _name.str() diff --git a/include/rfl/Size.hpp b/include/rfl/Size.hpp index 3b6845e7..5148b0b0 100644 --- a/include/rfl/Size.hpp +++ b/include/rfl/Size.hpp @@ -2,6 +2,7 @@ #define RFL_SIZE_HPP_ #include +#include #include "Ref.hpp" #include "Result.hpp" @@ -18,13 +19,12 @@ struct Size { /// @param _t The container whose size will be validated /// @return The container if validation passes, otherwise an error template - static rfl::Result validate(const T& _t) { - const auto to_t = [&](const auto&) { return _t; }; - const auto embellish_error = [](const auto& _err) -> Error { - return Error("Size validation failed: " + _err.what()); - }; - return V::validate(_t.size()).transform(to_t).transform_error( - embellish_error); + static rfl::Result validate(T _t) { + auto res = V::validate(_t.size()); + if (!res) { + return error("Size validation failed: " + res.error().what()); + } + return std::move(_t); } /// Converts the size validator to a JSON schema type. diff --git a/include/rfl/Validator.hpp b/include/rfl/Validator.hpp index b10c543d..9be58bf6 100644 --- a/include/rfl/Validator.hpp +++ b/include/rfl/Validator.hpp @@ -33,7 +33,19 @@ struct Validator { /// if it fails static Result from_value(const T& _value) noexcept { try { - return Validator(_value); + return wrap_validated(ValidationType::validate(_value)); + } catch (std::exception& e) { + return error(e.what()); + } + } + + /// Exception-free validation from a value (move version). + /// @param _value The value to validate (will be moved) + /// @return Result containing a Validator if validation succeeds, or an error + /// if it fails + static Result from_value(T&& _value) noexcept { + try { + return wrap_validated(ValidationType::validate(std::move(_value))); } catch (std::exception& e) { return error(e.what()); } @@ -54,7 +66,8 @@ struct Validator { /// Constructs from an rvalue of T (validates the value). /// @param _value The value to validate and wrap /// @throws std::exception if validation fails - Validator(T&& _value) : value_(ValidationType::validate(_value).value()) {} + Validator(T&& _value) + : value_(ValidationType::validate(std::move(_value)).value()) {} /// Constructs from a const reference to T (validates the value). /// @param _value The value to validate and wrap @@ -177,6 +190,19 @@ struct Validator { /// Necessary for the serialization to work. const T& reflection() const { return value_; } + private: + /// Tag for constructing a Validator from a value that has already been + /// validated. + struct AlreadyValidated {}; + + Validator(AlreadyValidated, T&& _value) : value_(std::move(_value)) {} + + /// Wraps the result of a validation without throwing if it failed. + static Result wrap_validated(Result&& _res) { + return std::move(_res).transform( + [](T&& _v) { return Validator(AlreadyValidated{}, std::move(_v)); }); + } + private: /// The underlying value. T value_; diff --git a/include/rfl/parsing/ParserRflVariant.hpp b/include/rfl/parsing/ParserRflVariant.hpp index 3b90b173..857aff13 100644 --- a/include/rfl/parsing/ParserRflVariant.hpp +++ b/include/rfl/parsing/ParserRflVariant.hpp @@ -95,7 +95,6 @@ class ParserRflVariant, } else { std::optional> result; std::vector errors; - errors.reserve(sizeof...(AlternativeTypes)); read_variant( _r, _var, &result, &errors, std::make_integer_sequence()); @@ -227,7 +226,10 @@ class ParserRflVariant, if (res) { *_result = std::move(*res); } else { - _errors->emplace_back(std::move(res.error().what())); + // Reserve lazily, so that reading an alternative that matches + // does not allocate. + _errors->reserve(sizeof...(AlternativeTypes)); + _errors->emplace_back(std::move(res).error().what()); } } } diff --git a/include/rfl/parsing/ParserTaggedUnion.hpp b/include/rfl/parsing/ParserTaggedUnion.hpp index 2cd2dc97..3de19a44 100644 --- a/include/rfl/parsing/ParserTaggedUnion.hpp +++ b/include/rfl/parsing/ParserTaggedUnion.hpp @@ -239,9 +239,16 @@ struct ParserTaggedUnion, /// Writes a wrapped version of the original object, which contains the tag. template static void write_wrapped(const W& _w, const T& _val, const P& _parent) { - const auto wrapped = wrap_if_necessary(_val); - Parser, ProcessorsType>::write( - _w, wrapped, _parent); + if constexpr (named_tuple_t::Names::template contains< + _discriminator>()) { + // The object already contains the tag, so it can be written as it is, + // without the copy that wrap_if_necessary would make. + Parser::write(_w, _val, _parent); + } else { + const auto wrapped = wrap_if_necessary(_val); + Parser, + ProcessorsType>::write(_w, wrapped, _parent); + } } /// Generates a wrapped version of the original object, which contains the diff --git a/include/rfl/parsing/ParserVariant.hpp b/include/rfl/parsing/ParserVariant.hpp index 0fe1e707..6ea04ce5 100644 --- a/include/rfl/parsing/ParserVariant.hpp +++ b/include/rfl/parsing/ParserVariant.hpp @@ -116,7 +116,6 @@ class ParserVariant, ProcessorsType> { } else { std::optional> result; std::vector errors; - errors.reserve(sizeof...(AlternativeTypes)); read_variant( _r, _var, &result, &errors, std::make_integer_sequence()); @@ -269,7 +268,10 @@ class ParserVariant, ProcessorsType> { if (res) { _result->emplace(std::move(*res)); } else { - _errors->emplace_back(res.error().what()); + // Reserve lazily, so that reading an alternative that matches + // does not allocate. + _errors->reserve(sizeof...(AlternativeTypes)); + _errors->emplace_back(std::move(res).error().what()); } } } diff --git a/include/rfl/parsing/Parser_default.hpp b/include/rfl/parsing/Parser_default.hpp index 3a92a86c..b664151c 100644 --- a/include/rfl/parsing/Parser_default.hpp +++ b/include/rfl/parsing/Parser_default.hpp @@ -246,6 +246,14 @@ struct Parser { using NT = decltype(_named_tuple); if constexpr (internal::HasFromReflectionMethod) { return T::from_reflection(std::forward(_named_tuple)); + } else if constexpr (internal::is_literal_v) { + // Unlike the constructor, from_string does not throw when the + // string is not one of the literal's values. + return T::from_string(_named_tuple); + } else if constexpr (internal::is_validator_v) { + // Unlike the constructor, from_value does not throw when the + // validation fails, which makes failing much cheaper. + return T::from_value(std::forward(_named_tuple)); } else { return T{std::forward(_named_tuple)}; } diff --git a/tests/json/test_literal.cpp b/tests/json/test_literal.cpp index 372820c6..d4c28b6d 100644 --- a/tests/json/test_literal.cpp +++ b/tests/json/test_literal.cpp @@ -28,4 +28,14 @@ TEST(json, test_literal) { write_and_read(bart, R"({"firstName":"Bart","lastName":"Simpson","children":[]})"); } + +TEST(json, test_literal_reports_unsupported_string) { + const auto res = rfl::json::read( + R"({"firstName":"Ned","lastName":"Simpson","children":[]})"); + ASSERT_FALSE(res); + EXPECT_EQ(res.error().what(), + "Failed to parse field 'firstName': Literal does not support " + "string 'Ned'. The following strings are supported: 'Homer', " + "'Marge', 'Bart', 'Lisa', 'Maggie'."); +} } // namespace test_literal diff --git a/tests/json/test_tagged_union_copies.cpp b/tests/json/test_tagged_union_copies.cpp new file mode 100644 index 00000000..3e6d9030 --- /dev/null +++ b/tests/json/test_tagged_union_copies.cpp @@ -0,0 +1,45 @@ +#include + +#include +#include +#include + +namespace test_tagged_union_copies { + +struct CopyCounter { + static inline int copies = 0; + + CopyCounter() = default; + CopyCounter(const CopyCounter&) { ++copies; } + CopyCounter(CopyCounter&&) noexcept = default; + CopyCounter& operator=(const CopyCounter&) { + ++copies; + return *this; + } + CopyCounter& operator=(CopyCounter&&) noexcept = default; + ~CopyCounter() = default; +}; + +struct Circle { + rfl::Literal<"Circle"> shape; + double radius; + rfl::Skip counter; +}; + +struct Square { + rfl::Literal<"Square"> shape; + double width; + rfl::Skip counter; +}; + +using Shapes = rfl::TaggedUnion<"shape", Circle, Square>; + +TEST(json, test_tagged_union_copies_nothing_when_writing) { + const auto circle = Shapes(Circle{.radius = 2.0}); + CopyCounter::copies = 0; + const auto json_string = rfl::json::write(circle); + EXPECT_EQ(json_string, R"({"shape":"Circle","radius":2.0})"); + EXPECT_EQ(CopyCounter::copies, 0); +} + +} // namespace test_tagged_union_copies diff --git a/tests/json/test_validator_copies.cpp b/tests/json/test_validator_copies.cpp new file mode 100644 index 00000000..cbf2db0d --- /dev/null +++ b/tests/json/test_validator_copies.cpp @@ -0,0 +1,62 @@ +#include + +#include +#include +#include +#include + +namespace test_validator_copies { + +struct CopyCounter { + static inline int copies = 0; + + CopyCounter() = default; + CopyCounter(const CopyCounter&) { ++copies; } + CopyCounter(CopyCounter&&) noexcept = default; + CopyCounter& operator=(const CopyCounter&) { + ++copies; + return *this; + } + CopyCounter& operator=(CopyCounter&&) noexcept = default; + ~CopyCounter() = default; +}; + +struct Element { + int value; + rfl::Skip counter; +}; + +using Elements = + rfl::Validator, rfl::Size>, + rfl::Size>>; + +struct Container { + Elements elements; +}; + +TEST(json, test_validator_copies_nothing_when_reading) { + CopyCounter::copies = 0; + const auto res = + rfl::json::read(R"({"elements":[{"value":1},{"value":2}]})"); + ASSERT_TRUE(res) << res.error().what(); + EXPECT_EQ(res->elements->size(), 2); + EXPECT_EQ(CopyCounter::copies, 0); +} + +TEST(json, test_validator_copies_nothing_when_moved_in) { + CopyCounter::copies = 0; + auto elements = std::vector(3); + const auto res = Elements::from_value(std::move(elements)); + ASSERT_TRUE(res) << res.error().what(); + EXPECT_EQ(CopyCounter::copies, 0); +} + +TEST(json, test_validator_reports_error_when_reading) { + const auto res = rfl::json::read(R"({"elements":[]})"); + ASSERT_FALSE(res); + EXPECT_EQ(res.error().what(), + "Failed to parse field 'elements': Size validation failed: Value " + "expected to be greater than or equal to 1, but got 0."); +} + +} // namespace test_validator_copies