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

Implement to/from json/stream using Reader_Writer #25

Closed omartijn closed 4 months ago

omartijn commented 4 months ago

This is useful when you have already written Reader_Writer classes for (de)serializing members.

eao197 commented 4 months ago

Hi! Thanks for PR. I'll handle it in a couple of days.

eao197 commented 4 months ago

Hi!

Unfortunately, this code doesn't compile:

    SECTION("from_json(reader, std_string_object)")
    {
        const std::string to_parse{ R"JSON("Hello")JSON" };
        std::string extracted_value = from_json<std::string>(
                custom_reader_writer_t{}, to_parse );

        REQUIRE( "Hello" == extracted_value );
    }

The overloading of from_json is ambigous here.

I think a solution can be in one of the following forms:

The first is addition of a special non-template marker:

struct use_reader_writer_t {};

JSON_DTO_NODISCARD
use_reader_writer_t
use_reader_writer() { return {}; }
...
template<
    typename Type,
    typename Reader_Writer,
    unsigned Rapidjson_Parseflags = rapidjson::kParseDefaultFlags >
void
from_json(
    //! Special marker to disambiguate the call.
    use_reader_writer_t marker,
    //! Custom Reader_Writer to be used.
    const Reader_Writer & reader_writer,
    //! Value to be parsed.
    const std::string & json,
    //! The receiver of the extracted value.
    Type & o )
{
    from_json< Type, Reader_Writer, Rapidjson_Parseflags >(
            marker, reader_writer, make_string_ref(json), o );
}

And then:

    SECTION("from_json(reader, std_string_object)")
    {
        const std::string to_parse{ R"JSON("Hello")JSON" };
        std::string extracted_value = from_json<std::string>(
                json_to::use_reader_writer(), custom_reader_writer_t{}, to_parse );

        REQUIRE( "Hello" == extracted_value );
    }

The second is to make use_reader_writer_t a template type:

template< typename Reader_Writer >
struct use_reader_writer_t {
    std::reference_wrapper<const Reader_Writer> m_reader_writer;
};

template< typename Reader_Writer >
JSON_DTO_NODISCARD
use_reader_writer_t<Reader_Writer>
use_reader_writer( const Reader_Writer & reader_writer ) { return { std::cref(reader_writer) }; }
...
template<
    typename Type,
    typename Reader_Writer,
    unsigned Rapidjson_Parseflags = rapidjson::kParseDefaultFlags >
void
from_json(
    //! Custom Reader_Writer to be used.
    use_reader_writer_t<Reader_Writer> reader_writer,
    //! Value to be parsed.
    const std::string & json,
    //! The receiver of the extracted value.
    Type & o )
{
    from_json< Type, Reader_Writer, Rapidjson_Parseflags >(
            marker, reader_writer, make_string_ref(json), o );
}

And then:

    SECTION("from_json(reader, std_string_object)")
    {
        const std::string to_parse{ R"JSON("Hello")JSON" };
        std::string extracted_value = from_json<std::string>(
                json_to::use_reader_writer(custom_reader_writer_t{}), to_parse );

        REQUIRE( "Hello" == extracted_value );
    }

At the moment I prefer the first approach with non-template use_reader_writer_t. But give me some time to think.

PS. I've used another overloading of from_json in my examples, but it doesn't matter.

eao197 commented 4 months ago

Hi, @omartijn !

I want to ask: do you really need this overload:

template< typename Reader_Writer, typename Dto >
std::string
to_json(
    //! Custom Reader_Writer to be used.
    reader_writer_to_use_t<Reader_Writer> reader_writer,
    //! Object to be serialized.
    const Dto & dto,
    //! Parameters for pretty_writer.
    pretty_writer_params_t writer_params )

It seems to me that writer_params won't be used by a custom reader_writer. It make such overload just useless from my point of view. But may be you have an example where it's useful?

omartijn commented 4 months ago

Why would it not be used? The custom reader/writer only takes care of filling the rapidjson::Document with the right data, the serialization after that is still done the same way as the overload without a custom reader/writer - and that's where the writer_params come in.

Unless I'm very much mistaken, that is.

eao197 commented 4 months ago

Why would it not be used?

Because reader_writer is applied only to one value. For example:

struct  hex_writer { ... };

int v = ...;
auto json_str = json_dto::to_json(json_dto::with_reader_writer(hex_writer{}), v);

There is no space to pretty-printing here.

I don't see a way how reader_writer can be used for aggregate types like structs (because structs should have own json_io).

But maybe it's useful for aggregates like arrays. Something like:

struct  hex_writer { ... };

std::vector<int> v = ...;
auto json_str = json_dto::to_json(json_dto::with_reader_writer(json_dto::apply_to_content_t<hex_writer>{}), v);

However, I still have some doubts.

Could you please share your experience of using new to_json/from_json/to_stream/from_stream with custom reader-writters?

omartijn commented 4 months ago

I use reader_writer for structs. I'm not allowed to change the structs, and a non-intrusive json_io won't work because the structs are templated and it prefers the json_io declared in json_dto itself (and then fails because there's no json_io member function).

eao197 commented 4 months ago

Ok, I've got the point. Thanks!

eao197 commented 4 months ago

@omartijn , can I ask you to open an issue with an example of such a case:

I'm not allowed to change the structs, and a non-intrusive json_io won't work because the structs are templated and it prefers the json_io declared in json_dto itself (and then fails because there's no json_io member function).

Maybe we'll find a solution for it with time...

omartijn commented 4 months ago

Maybe we'll find a solution for it with time...

I think I suggested this before - but we could easily do an SFINAE on that function to check whether the object has a json_io() member function. If we do that, it would completely eliminate my need for the whole custom reader/writer system.

eao197 commented 4 months ago

I have to switch between different project so I'm afraid I don't remeber details :(

omartijn commented 4 months ago

That was in an issue about generic serialization of enums: https://github.com/Stiffstream/json_dto/issues/7#issuecomment-690665914

eao197 commented 4 months ago

@omartijn , did you use a new from_stream function?

omartijn commented 4 months ago

@omartijn , did you use a new from_stream function?

No. I just added those overloads for consistency/completeness.

eao197 commented 4 months ago

No. I just added those overloads for consistency/completeness.

I suspect that :) It doesn't work as expected. I'll take a closer look tomorrow.

Anyway, a problem mentioned here was caused by another error, so we can go without any with_reader_writer helpers.

eao197 commented 4 months ago

Hi @omartijn !

I've completed the work on your PR, but before merging it into the master I want to take an attempt to find a better solution for your case:

I'm not allowed to change the structs, and a non-intrusive json_io won't work because the structs are templated and it prefers the json_io declared in json_dto itself (and then fails because there's no json_io member function).

I've added a new example into 0.3-dev branch: https://github.com/Stiffstream/json_dto/blob/0.3-dev/dev/sample/tutorial2.1/main.cpp Can you take a look at it and tell how to change it to get a case like your? When I'll have a minimal reproducible example for the problem I'll have a possibility to find a solution (try to find at least).

omartijn commented 4 months ago

I've just checked again why I use Reader/Writer for things that are structs. In the I work on, we use a lot of std::shared_ptrs to things that need to be serialized. Those structs also don't have regular members to interact with, but employ getters/setters for interaction.

The Reader/Writer pattern makes sense here, due to the separated read/write functions you implement for the type. Maybe it's possible to do another way, by not templating the json_io function for input/output, but implement that separately. I'll see if it's possible to get it working that way.

omartijn commented 4 months ago

After checking the code, I see a few reasons that a simple non-intrusive json-io function is not going to work. In certain cases, I need to invoke another serialization function and then unset some of the things it's set.

Additionally, some of the code needs to be flexible: "If this input is an object, do this, otherwise do that". This may not fit entirely within the scope of json_dto, but that's what we need.

eao197 commented 4 months ago

Hi!

In the I work on, we use a lot of std::shared_ptrs to things that need to be serialized.

It's very interesting. I tried to remember if we used shared_ptr with json_dto and can't remember any cases. Could you show a small example how it looks like?

omartijn commented 4 months ago

Well what we have are classes with getters/setters and they are distributed using shared_ptr, so we have a reader_writer class somewhat like this:

class reader_writer {
    public:
        void read(const std::shared_ptr<X>& object, const rapidjson::Value& value) const
        {
            int x, y;
            json_dto::read_json_value(x, json["x"]);
            json_dto::read_json_value(y, json["y"]);

            object->setX(x);
            object->setY(y);
        }

        void write(const std::shared_ptr<X>& object, rapidjson::Value& value, rapidjson::MemoryPoolAllocator<>& allocator) const
        {
                rapidjson::Value x, y;
                json_dto::write_json_value(object->getX(), x, allocator);
                json_dto::write_json_value(object->getY(), y, allocator);
        }
};

This is about the simplest case here. I've also written a Binder proxy, that binds to two member functions (given as template arguments), and then executes those in the read_from and write_to, but for the more complicated things that also doesn't work.

eao197 commented 4 months ago

Thanks for example!

I suppose that write method has to look like this:

        void write(const std::shared_ptr<X>& object, rapidjson::Value& value, rapidjson::MemoryPoolAllocator<>& allocator) const
        {
                rapidjson::Value x, y;
                json_dto::write_json_value(object->getX(), x, allocator);
                json_dto::write_json_value(object->getY(), y, allocator);
                value["x"] = std::move(x);
                value["y"] = std::move(y);
        }
omartijn commented 4 months ago

Ah yes, I forgot to include that, while trying to remove the non-relevant things from my example.