[Tarantool-patches] [PATCH 0/2] fix box.session.push formatting

[Tarantool-patches] [PATCH 0/2] fix box.session.push formatting

[Chris Sosnin]

Nikita, please, do a second review.

First commit moves current output format to struct session.
Second commit adds Lua output format support for push().

@ChangeLog
- Add Lua output format support for box.session.push

issue: https://github.com/tarantool/tarantool/issues/4686
branch: https://github.com/tarantool/tarantool/tree/ksosnin/gh-4686-session-push-fmt

Chris Sosnin (2):
  session: store output format in struct session
  box: fix formatting in session.push

 extra/exports                 |  2 +
 src/box/lua/console.c         | 56 +++++++++++++++++++-------
 src/box/lua/console.lua       | 76 +++++++++++++++++++++++++++--------
 src/box/session.h             | 22 +++++++---
 test/app-tap/console.test.lua | 38 +++++++++++++++++-
 5 files changed, 155 insertions(+), 39 deletions(-)

-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 1/2] session: store output format in struct session

[Chris Sosnin]

box.session.storage is a general-purpose table, which can be
used by user. Therefore, we shouldn't store any internal details
in it.

Needed for #4686
---
 extra/exports           |  2 ++
 src/box/lua/console.c   | 12 ++++++++++++
 src/box/lua/console.lua | 36 +++++++++++++++++++++++++++++-------
 src/box/session.h       | 22 ++++++++++++++++------
 4 files changed, 59 insertions(+), 13 deletions(-)

diff --git a/extra/exports b/extra/exports
index cbb5adcf4..f71cb7d93 100644
--- a/extra/exports
+++ b/extra/exports
@@ -43,6 +43,8 @@ tnt_iconv_close
 tnt_iconv
 exception_get_string
 exception_get_int
+console_get_output_format
+console_set_output_format
 
 tarantool_lua_ibuf
 uuid_nil
diff --git a/src/box/lua/console.c b/src/box/lua/console.c
index 57e7e7f4f..603f7c11b 100644
--- a/src/box/lua/console.c
+++ b/src/box/lua/console.c
@@ -377,6 +377,18 @@ console_session_fd(struct session *session)
 	return session->meta.fd;
 }
 
+enum output_format
+console_get_output_format()
+{
+	return current_session()->meta.output_format;
+}
+
+void
+console_set_output_format(enum output_format output_format)
+{
+	current_session()->meta.output_format = output_format;
+}
+
 /**
  * Dump port lua data as a YAML document tagged with !push! global
  * tag.
diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua
index 17e2c91b2..7208d3d30 100644
--- a/src/box/lua/console.lua
+++ b/src/box/lua/console.lua
@@ -2,6 +2,21 @@
 --
 -- vim: ts=4 sw=4 et
 
+local ffi = require('ffi')
+ffi.cdef[[
+    enum output_format {
+        OUTPUT_FORMAT_YAML = 0,
+        OUTPUT_FORMAT_LUA_LINE,
+        OUTPUT_FORMAT_LUA_BLOCK,
+    };
+
+    enum output_format
+    console_get_output_format();
+
+    void
+    console_set_output_format(enum output_format output_format);
+]]
+
 local serpent = require('serpent')
 local internal = require('console')
 local session_internal = require('box.internal.session')
@@ -172,17 +187,24 @@ local function output_save(fmt, opts)
     -- Output format descriptors are saved per
     -- session thus each console may specify
     -- own mode.
-    box.session.storage.console_output_format = {
-        ["fmt"] = fmt, ["opts"] = opts
-    }
+    if fmt == "yaml" then
+        ffi.C.console_set_output_format(ffi.C.OUTPUT_FORMAT_YAML)
+    elseif fmt == "lua" and opts == "block" then
+        ffi.C.console_set_output_format(ffi.C.OUTPUT_FORMAT_LUA_BLOCK)
+    else
+        ffi.C.console_set_output_format(ffi.C.OUTPUT_FORMAT_LUA_LINE)
+    end
 end
 
 local function current_output()
-    local d = box.session.storage.console_output_format
-    if d == nil then
-        return default_output_format
+    local fmt = ffi.C.console_get_output_format()
+    if fmt == ffi.C.OUTPUT_FORMAT_YAML then
+        return { ["fmt"] = "yaml", ["opts"] = nil }
+    elseif fmt == ffi.C.OUTPUT_FORMAT_LUA_LINE then
+        return { ["fmt"] = "lua", ["opts"] = "line" }
+    elseif fmt == ffi.C.OUTPUT_FORMAT_LUA_BLOCK then
+        return { ["fmt"] = "lua", ["opts"] = "block" }
     end
-    return d
 end
 
 --
diff --git a/src/box/session.h b/src/box/session.h
index 6dfc7cba5..9ade6e7a5 100644
--- a/src/box/session.h
+++ b/src/box/session.h
@@ -59,6 +59,12 @@ enum session_type {
 	session_type_MAX,
 };
 
+enum output_format {
+	OUTPUT_FORMAT_YAML = 0,
+	OUTPUT_FORMAT_LUA_LINE,
+	OUTPUT_FORMAT_LUA_BLOCK,
+};
+
 extern const char *session_type_strs[];
 
 /**
@@ -73,11 +79,15 @@ extern uint32_t default_flags;
  * types, and allows to do not store attributes in struct session,
  * that are used only by a session of particular type.
  */
-union session_meta {
-	/** IProto connection. */
-	void *connection;
-	/** Console file/socket descriptor. */
-	int fd;
+struct session_meta {
+	union {
+		/** IProto connection. */
+		void *connection;
+		/** Console file/socket descriptor. */
+		int fd;
+	};
+	/** Console output format. */
+	enum output_format output_format;
 };
 
 /**
@@ -100,7 +110,7 @@ struct session {
 	/** Session virtual methods. */
 	const struct session_vtab *vtab;
 	/** Session metadata. */
-	union session_meta meta;
+	struct session_meta meta;
 	/**
 	 * ID of statements prepared in current session.
 	 * This map is allocated on demand.
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 2/2] box: fix formatting in session.push

[Chris Sosnin]

box.session.push() encodes data as a YAML document independent on
the current console output format. This patch adds handling for Lua
as well.

Closes #4686
---
 src/box/lua/console.c         | 44 +++++++++++++++++++++++------------
 src/box/lua/console.lua       | 40 +++++++++++++++++++++++--------
 test/app-tap/console.test.lua | 38 +++++++++++++++++++++++++++++-
 3 files changed, 96 insertions(+), 26 deletions(-)

diff --git a/src/box/lua/console.c b/src/box/lua/console.c
index 603f7c11b..bd454c269 100644
--- a/src/box/lua/console.c
+++ b/src/box/lua/console.c
@@ -390,8 +390,8 @@ console_set_output_format(enum output_format output_format)
 }
 
 /**
- * Dump port lua data as a YAML document tagged with !push! global
- * tag.
+ * Dump port lua data with respect to output format:
+ * YAML document tagged with !push! global tag or Lua string.
  * @param port Port lua.
  * @param[out] size Size of the result.
  *
@@ -403,18 +403,31 @@ port_lua_dump_plain(struct port *port, uint32_t *size)
 {
 	struct port_lua *port_lua = (struct port_lua *) port;
 	struct lua_State *L = port_lua->L;
-	int rc = lua_yaml_encode(L, luaL_yaml_default, "!push!",
-				 "tag:tarantool.io/push,2018");
-	if (rc == 2) {
-		/*
-		 * Nil and error object are pushed onto the stack.
-		 */
-		assert(lua_isnil(L, -2));
-		assert(lua_isstring(L, -1));
-		diag_set(ClientError, ER_PROC_LUA, lua_tostring(L, -1));
-		return NULL;
+	enum output_format fmt = console_get_output_format();
+	if (fmt == OUTPUT_FORMAT_YAML) {
+		int rc = lua_yaml_encode(L, luaL_yaml_default, "!push!",
+					 "tag:tarantool.io/push,2018");
+		if (rc == 2) {
+			/*
+			 * Nil and error object are pushed onto the stack.
+			 */
+			assert(lua_isnil(L, -2));
+			assert(lua_isstring(L, -1));
+			diag_set(ClientError, ER_PROC_LUA, lua_tostring(L, -1));
+			return NULL;
+		}
+		assert(rc == 1);
+	} else {
+		assert(fmt == OUTPUT_FORMAT_LUA_LINE ||
+		       fmt == OUTPUT_FORMAT_LUA_BLOCK);
+		luaL_findtable(L, LUA_GLOBALSINDEX, "box.internal", 1);
+		lua_getfield(L, -1, "format_lua_push");
+		lua_pushvalue(L, -3);
+		if (lua_pcall(L, 1, 1, 0) != 0) {
+			diag_set(LuajitError, lua_tostring(L, -1));
+			return NULL;
+		}
 	}
-	assert(rc == 1);
 	assert(lua_isstring(L, -1));
 	size_t len;
 	const char *result = lua_tolstring(L, -1, &len);
@@ -423,10 +436,11 @@ port_lua_dump_plain(struct port *port, uint32_t *size)
 }
 
 /**
- * Push a tagged YAML document into a console socket.
+ * Push a tagged YAML document or a Lua string into a console
+ * socket.
  * @param session Console session.
  * @param sync Unused request sync.
- * @param port Port with YAML to push.
+ * @param port Port with the data to push.
  *
  * @retval  0 Success.
  * @retval -1 Error.
diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua
index 7208d3d30..2add9a79d 100644
--- a/src/box/lua/console.lua
+++ b/src/box/lua/console.lua
@@ -27,6 +27,7 @@ local errno = require('errno')
 local urilib = require('uri')
 local yaml = require('yaml')
 local net_box = require('net.box')
+local box_internal = require('box.internal')
 
 local PUSH_TAG_HANDLE = '!push!'
 
@@ -207,6 +208,13 @@ local function current_output()
     end
 end
 
+-- Used by console_session_push.
+box_internal.format_lua_push = function(value)
+    local opts = current_output()["opts"]
+    value = format_lua_value(value, opts)
+    return '-- Push\n' .. value .. ';'
+end
+
 --
 -- Return current EOS value for currently
 -- active output format.
@@ -426,8 +434,8 @@ local text_connection_mt = {
             return self._socket:write(text)
         end,
         --
-        -- Read a text from a socket until YAML terminator.
-        -- @retval not nil Well formatted YAML.
+        -- Read a text from a socket until EOS.
+        -- @retval not nil Well formatted data.
         -- @retval     nil Error.
         --
         read = function(self)
@@ -459,6 +467,7 @@ local text_connection_mt = {
                     param_handlers[items[2]] == set_output then
                     local err, fmt, opts = parse_output(items[3])
                     if not err then
+                        self.fmt = fmt
                         self.eos = output_eos[fmt]
                     end
                 end
@@ -470,6 +479,7 @@ local text_connection_mt = {
         eval = function(self, text)
             self:preprocess_eval(text)
             text = text..'$EOF$\n'
+            local fmt = self.fmt or default_output_format["fmt"]
             if not self:write(text) then
                 error(self:set_error())
             end
@@ -478,14 +488,23 @@ local text_connection_mt = {
                 if not rc then
                     break
                 end
-                local handle, prefix = yaml.decode(rc, {tag_only = true})
-                if not handle then
-                    -- Can not fail - tags are encoded with no
-                    -- user participation and are correct always.
-                    return rc
-                end
-                if handle == PUSH_TAG_HANDLE and self.print_f then
-                    self.print_f(rc)
+                if fmt == "yaml" then
+                    local handle, prefix = yaml.decode(rc, {tag_only = true})
+                    if not handle then
+                        -- Can not fail - tags are encoded with no
+                        -- user participation and are correct always.
+                        return rc
+                    end
+                    if handle == PUSH_TAG_HANDLE and self.print_f then
+                        self.print_f(rc)
+                    end
+                else
+                    if not string.startswith(rc, '-- Push') then
+                        return rc
+                    end
+                    if self.print_f then
+                        self.print_f(rc)
+                    end
                 end
             end
             return error(self:set_error())
@@ -524,6 +543,7 @@ local function wrap_text_socket(connection, url, print_f)
         port = url.service,
         print_f = print_f,
         eos = current_eos(),
+        fmt = current_output()["fmt"]
     }, text_connection_mt)
     --
     -- Prepare the connection: setup EOS symbol
diff --git a/test/app-tap/console.test.lua b/test/app-tap/console.test.lua
index da5c1e71e..4feadfa5e 100755
--- a/test/app-tap/console.test.lua
+++ b/test/app-tap/console.test.lua
@@ -21,7 +21,7 @@ local EOL = "\n...\n"
 
 test = tap.test("console")
 
-test:plan(73)
+test:plan(80)
 
 -- Start console and connect to it
 local server = console.listen(CONSOLE_SOCKET)
@@ -39,6 +39,42 @@ test:is(client:read(EOL), '%TAG !push! tag:tarantool.io/push,2018\n--- 200\n...\
         "pushed message")
 test:is(client:read(EOL), '---\n- true\n...\n', "pushed message")
 
+--
+-- gh-4686: box.session.push should respect output format.
+--
+client:write('\\set output lua\n')
+client:read(";")
+
+client:write('box.session.push({})\n')
+test:is(client:read(";"), '-- Push\n{};', "pushed message")
+test:is(client:read(";"), 'true;', "pushed message")
+
+client:write('\\set output lua,block\n')
+client:read(";")
+
+client:write('box.session.push({1})\n')
+test:is(client:read(";"), '-- Push\n{\n  1\n};', "pushed message")
+test:is(client:read(";"), 'true;', "pushed message")
+
+client:write('\\set output lua\n')
+client:read(";")
+
+long_func_f = nil
+function long_func()
+    long_func_f = fiber.self()
+    box.session.push('push')
+    fiber.sleep(math.huge)
+    return 'res'
+end
+client:write('long_func()\n')
+test:is(client:read(';'), '-- Push\n"push";', 'pushed message')
+test:is(long_func_f:status(), 'suspended', 'push arrived earlier than return')
+long_func_f:wakeup()
+test:is(client:read(';'), '"res";', 'returned result')
+
+client:write('\\set output yaml\n')
+client:read(EOL)
+
 --
 -- gh-3790: box.session.push support uint64_t sync.
 --
-- 
2.21.1 (Apple Git-122.3)

Re: [Tarantool-patches] [PATCH 1/2] session: store output format in struct session

[Nikita Pettik]

On 06 Apr 17:33, Chris Sosnin wrote:
> diff --git a/src/box/session.h b/src/box/session.h
> index 6dfc7cba5..9ade6e7a5 100644
> --- a/src/box/session.h
> +++ b/src/box/session.h
> @@ -59,6 +59,12 @@ enum session_type {
>  	session_type_MAX,
>  };
>  
> +enum output_format {
> +	OUTPUT_FORMAT_YAML = 0,
> +	OUTPUT_FORMAT_LUA_LINE,
> +	OUTPUT_FORMAT_LUA_BLOCK,
> +};
> +
>

It would be nice to see explanation concering difference between
LUA_LINE and LUA_BLOCK outputs.

LGTM (looks like pure refactoring).
 

Re: [Tarantool-patches] [PATCH 2/2] box: fix formatting in session.push

[Nikita Pettik]

On 06 Apr 17:33, Chris Sosnin wrote:
> box.session.push() encodes data as a YAML document independent on
> the current console output format. This patch adds handling for Lua
> as well.
> 
> Closes #4686
> ---

LGTM