Re: [Tarantool-patches] [PATCH v2] net.box: add __serialize and __tostring methods for future objects

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi! Thanks for the patch!

See 2 comments below.

> diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
> index 1783da607dcc..3ed464a93437 100644
> --- a/src/box/lua/net_box.c
> +++ b/src/box/lua/net_box.c
> @@ -1432,6 +1432,29 @@ luaT_netbox_request_gc(struct lua_State *L)
>  	return 0;
>  }
>  
> +static int
> +luaT_netbox_request_tostring(struct lua_State *L)
> +{
> +	lua_pushstring(L, netbox_request_typename);
> +	return 1;
> +}
> +
> +static int
> +luaT_netbox_request_serialize(struct lua_State *L)
> +{
> +	struct netbox_request *request = luaT_check_netbox_request(L, 1);
> +	/*
> +	 * If there are user-defined fields attached to the future object,
> +	 * return them, otherwise push the type name, like __tostring does.
> +	 */
> +	if (request->index_ref != LUA_NOREF) {
> +		lua_rawgeti(L, LUA_REGISTRYINDEX, request->index_ref);
> +	} else {
> +		lua_pushstring(L, netbox_request_typename);
> +	}
> +	return 1;
> +}

1. It does not look good that __serialize might return both a table
and a string depending on the object content. Perhaps it is worth to
return an empty table when it has no members. Otherwise, say, if I
store an optional value in the index and want to get it like this:

	serialized_req.member

then I will get sometimes nil, sometimes an error would be thrown.

> +
>  static int
>  luaT_netbox_request_index(struct lua_State *L)
>  {
> @@ -2107,6 +2130,8 @@ luaopen_net_box(struct lua_State *L)
>  
>  	static const struct luaL_Reg netbox_request_meta[] = {
>  		{ "__gc",           luaT_netbox_request_gc },
> +		{"__tostring",      luaT_netbox_request_tostring },
> +		{"__serialize",     luaT_netbox_request_serialize },

2. You have a whitespace after { in all the other lines. Could you
please add them here too?

>  		{ "__index",        luaT_netbox_request_index },
>  		{ "__newindex",     luaT_netbox_request_newindex },
>  		{ "is_ready",       luaT_netbox_request_is_ready },