qchateau / packio

An asynchronous msgpack-RPC and JSON-RPC library built on top of Boost.Asio.
https://qchateau.github.io/packio/
Mozilla Public License 2.0
138 stars 20 forks source link

How to deal with optional parameters #72

Closed weili-jiang closed 1 year ago

weili-jiang commented 1 year ago

When the dispatcher encounters a missing parameter (using the nlohmann::json implementation), it throws the request away as the request fails to parse in rpc::convert_named_args, due to the use of the json::at method which throws due to the parameter key not being present.

We have a use-case where the other end omits optional parameters (instead of setting them to null), which we cannot change. Currently, I have hacked a workaround in, which is to attempt to return a default constructed object if the parameter is not found:

    template <typename T, typename NamesContainer, std::size_t... Idxs>
    static T convert_named_args(
        const nlohmann::json& args,
        const NamesContainer& names,
        std::index_sequence<Idxs...>)
    {
        return T{(args.template value<std::tuple_element_t<Idxs, T>>(names.at(Idxs), {}))...};
    }

It's not particularly elegant, and requires a suitably friendly type with a sensible default constructor to be passed into dispatcher::add (e.g. nlohmann::json, which can be explicitly tested for is_null() in the dispatcher callback).

Do you think this change is appropriate for general inclusion in a PR? I'm not sure how well it would work for the Boost JSON or MessagePack variants. Alternatively, do you have a better solution?

qchateau commented 1 year ago

Hi, I like the ability to have default arguments, but I don't want all arguments to be "default-able". I want the default arguments to be explicitely defined as such, which also opens the door for supporting any value for default arguments. Give me a few days/weeks and I think I can come up with something.

qchateau commented 1 year ago

I just released v2.4.0 with default arguments and an option to allow or reject unknown parameters. It's not available on Conan yet, but it will be as soon as possible. There's a very small example in the readme, and more usage example in the unit tests