Stiffstream / json_dto

A small header-only library for converting data between json representation and c++ structs
BSD 3-Clause "New" or "Revised" License
149 stars 18 forks source link

Generic support for enums, custom (read_write)_json_value not used #7

Closed omartijn closed 4 years ago

omartijn commented 4 years ago

I'm trying to support enum values in JSON (de)serialization. For this I'm using https://github.com/Neargye/magic_enum

It works when I define the custom IO functions with the literal enum type, but it does not work when I make the function generic (so it works for all enums).

This works:

enum bla : uint16_t
{
    value_one,
    value_two
};

namespace json_dto {

    template <>
    inline void write_json_value(const bla& value, rapidjson::Value& object, rapidjson::MemoryPoolAllocator<>& allocator)
    {
        // first retrieve the enum name
        std::string name{ magic_enum::enum_name(value) };

        // now write it to the value
        write_json_value(name, object, allocator);
    }

}

This, however, does not:

enum bla : uint16_t
{
    value_one,
    value_two
};

namespace json_dto {

    template <typename T>
    std::enable_if_t<std::is_enum_v<T>>
    write_json_value(const T& value, rapidjson::Value& object, rapidjson::MemoryPoolAllocator<>& allocator)
    {
        // first retrieve the enum name
        std::string name{ magic_enum::enum_name(value) };

        // now write it to the value
        write_json_value(name, object, allocator);
    }

}

The latter case will not find the function, and then fail because it tries to call T::json_io.

eao197 commented 4 years ago

It seems that that overload:

template <typename T>
std::enable_if_t<std::is_enum_v<T>>
write_json_value(const T& value, rapidjson::Value& object, rapidjson::MemoryPoolAllocator<>& allocator);

won't work without such patch for pub.hpp:

@@ -886,7 +886,8 @@ json_io( Io & io, Dto & dto )

 template< typename Dto >
 std::enable_if_t<
-               !details::meta::is_stl_like_container<Dto>::value,
+               !details::meta::is_stl_like_container<Dto>::value
+                       && !std::is_enum<Dto>::value,
                void >
 read_json_value(
        Dto & v,
@@ -898,7 +899,8 @@ read_json_value(

 template< typename Dto >
 std::enable_if_t<
-               !details::meta::is_stl_like_container<Dto>::value,
+               !details::meta::is_stl_like_container<Dto>::value
+                       && !std::is_enum<Dto>::value,
                void >
 write_json_value(
        const Dto & v,

It also seems that the current version of json_dto doesn't support such a generic way of reading/writing enums. I'll think about some way of adding such support to json_dto.

eao197 commented 4 years ago

The best way that I could come up with at the moment is:

  1. Definition of some empty tag type: https://github.com/Stiffstream/json_dto/blob/db0fb723fc5163083d763e0b2a8262e74e05e450/dev/test/tagged_proxy/main.cpp#L12

  2. Usage of the new json_dto::tagged_proxy function during enumeration of fields to be (de)serialized: https://github.com/Stiffstream/json_dto/blob/db0fb723fc5163083d763e0b2a8262e74e05e450/dev/test/tagged_proxy/main.cpp#L82-L87

  3. Writing own partial specialization of the new json_dto::tagged_proxy_io_t type for your tag type: https://github.com/Stiffstream/json_dto/blob/db0fb723fc5163083d763e0b2a8262e74e05e450/dev/test/tagged_proxy/main.cpp#L16-L53

If such an approach looks appropriate for you then I'll finish that sketch and release an updated version of json_dto.

eao197 commented 4 years ago

I think there is another possibility based on the approach described above:

namespace test {

struct my_enum_image {}; // Tag type.

// Enums to be serialized a special way.
enum class level_t {...};
enum class category_t {...};

} /* namespace test */

namespace json_dto {

// Partial specialization for tagged_io_t.
template<typename T>
struct tagged_io_t<test::my_enum_image, T> {
   static void read_json_value(...) {...}
   static void write_json_value(...) {...}
};

// THE MAIN TRICK: specialization of tagged_t for every enums from test namespace.
template<> struct tagged_t<test::level_t> : public tagged_as<test::my_enum_image> {};
template<> struct tagged_t<test::category_t> : public tagged_as<test::my_enum_image> {};

} /* namespace json_dto */

namespace test {

struct data_t
{
    level_t m_level;
    category_t m_category;

    template<typename IO>
    void json_io( IO & io )
    {
        // NOTE: fields are described as usual.
        io & json_dto::mandatory( "level", m_level )
            & json_dto::mandatory( "cat", m_category );
    }
};

} /* namespace test */

I didn't check that approach, but it seems that it could be implemented in C++14.

omartijn commented 4 years ago

I'm not sure those ideas are the best we can come up with. The fact that you have to do extra work to specifically enable support for each enum in particular feels suboptimal.

As it is, currently it breaks because the read_json_value and write_json_value are over-eager templates which then themselves call an over-eager json_io free-standing function which then breaks because it tries to invoke a member function on the given variable.

The free-standing json_io function should be easily fixable. We can just ensure it is only enabled if the given type is a class type. So if we change this function prototype to something like this:

template< typename Io, typename Dto >
std::enable_if_t<
        std::is_class<Dto>::value,
        void >
json_io( IO & io, Dto & dto )

we ensure that this function is not called unduly. This allows us to create a type trait so we can enable the read_json_value and write_json_value functions only if we can call the free-standing json_io function.

eao197 commented 4 years ago

The free-standing json_io function should be easily fixable. We can just ensure it is only enabled if the given type is a class type.

The json_io function is one of the ways to define custom (de)serialization for a user type (see here). In most cases json_io will be used for struct/classes but there is some probability that json_io can be used for non-struct types like non-standard extension (__int128, for example).

I think that probability is very low and we can neglect it but there is another drawback:

Suppose someone writes a header-only library A using json_dto and defines its own specialization for read_json_value/write_json_value for enums. And someone else writes another header-only library B using json_dto with different specialization of read_json_value/write_json_value for working with enums.

There won't be any problems until A and B are used separately. But if someone includes both A and B in one translation unit then there will be ambiguous specializations for read_json_value/write_json_value. And that ambiguity can't be easily solved.

An idea with tagged types has no that drawbacks.

eao197 commented 4 years ago

Another possible solution is the introduction of some form of extractors/formatters like shown here.

omartijn commented 4 years ago

I understand that the free-standing json_io function may be used to define user-types for serialization. However, that should not be an issue. Simply removing the default json_io function (which does not support non-class types) from the overload set does not prevent someone from writing another version that does support it. I think doing this is useful in its own right.

In fact, I think it could be even better if we write a type trait for a class-type that has a proper implementation of a json_io member function and only enable the free-standing version for these types. Perhaps something like this (untested):

template< typename, typename = void_t<> >
struct has_json_io : public std::false_type{};

template< typename T >
struct has_json_io<
        T,
        void_t<
            decltype(T::json_io(std::declval<json_input_t>())),
            decltype(T::json_io(std::declval<json_output_t>()))
        > : public std::true_type{};

If we do this, then it becomes possibly to write a generic free-standing json_io function, because the one from json_dto does not over-eagerly try to take it

eao197 commented 4 years ago

I don't get the point.

The current scheme has several levels but is rather simple (lets use deserialization for an example):

This scheme allows a user to make own deserialization for a type T at two points:

The main drawback of that scheme is that it is oriented to a specific type T, not to a group or family of types. So, if you have a group of types that should be (de)serialized the similar way you have to make an overload of read/write_json_value or json_io functions for every type from your group.

And I don't see how your proposal of the introduction of has_json_io solves that drawback.

Suppose that we implement has_member_json_io and has_freestanding_json_io traits and make two specializations of read_json_value based on them:

template< typename Dto >
std::enable_if_t<
   !details::meta::is_stl_like_container<Dto>::value
      && details::meta::has_member_json_io<Dto>::value,
   void >
 read_json_value(Dto & v, const rapidjson::Value & object) {...}

template< typename Dto >
std::enable_if_t<
   !details::meta::is_stl_like_container<Dto>::value
      && details::meta::has_freestanding_json_io<Dto>::value,
   void >
 read_json_value(Dto & v, const rapidjson::Value & object) {...}

It allows you to make a specialization for enums like:

template< typename Dto >
std::enable_if_t<
   !details::meta::is_stl_like_container<Dto>::value
      && std::is_enum<Dto>::value,
   void >
 read_json_value(Dto & v, const rapidjson::Value & object) {...}

But such specialization can lead to conflicts in the case if someone else defines a specialization like:

template< typename Dto >
std::enable_if_t<
   !details::meta::is_stl_like_container<Dto>::value
      && (std::is_same<Dto, SomeMyEnum>::value || std::is_same<Dto, AnotherEnum>::value),
   void >
 read_json_value(Dto & v, const rapidjson::Value & object) {...}

or if someone else defines json_io for his/her enum types.

That is why I came to an idea with tagged types. And later to an idea with custom extractors/formatters. The idea with custom extractors/formatters looks even better because it also allows solving another problem (see #8).

eao197 commented 4 years ago

@omartijn Version 0.2.10 is just released. Now you can try to use magic_enum library with your types via custom Reader_Writer types as described here.

omartijn commented 4 years ago

That looks pretty nice! Thanks for implementing.