[Tarantool-patches] [PATCH 1.10 02/16] refactoring: adjust contract of luaT_tuple_new()

[Alexander Turenko <alexander.turenko@tarantool.org>]

Accept an index on the Lua stack. Report an error using the diagnostics
area (not raise a Lua error).

The function is used by built-in key_def and merger modules on
tarantool-2.*. Now we plan to expose necessary APIs from tarantool of
all supported versions (including 1.10) and create external key_def and
merger modules.

Part of #5273

(cherry picked from commit 64a6464d31c92ea11c4f5c3ccdaada2ae9666455)

The original commit message:

 | lua: add luaT_tuple_new()
 |
 | The function allows to create a tuple with specific tuple format in C
 | code using a Lua table, another tuple, or objects on a Lua stack.
 |
 | Needed for #3276, #3398, #4025

The original message is a bit misleading, because it was not rewritten
after introduction of the function with the same name in
9e2a905c069e58cdd95d91868025391ecd9404e6 ('box: fix format of tuple
produced with frommap()').
---
 src/box/lua/space.cc            |   2 +-
 src/box/lua/tuple.c             |  47 ++++++---
 src/box/lua/tuple.h             |  13 ++-
 test/unit/CMakeLists.txt        |   4 +
 test/unit/luaT_tuple_new.c      | 175 ++++++++++++++++++++++++++++++++
 test/unit/luaT_tuple_new.result |  22 ++++
 6 files changed, 245 insertions(+), 18 deletions(-)
 create mode 100644 test/unit/luaT_tuple_new.c
 create mode 100644 test/unit/luaT_tuple_new.result

diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc
index 94e895148..9ea0d6f7e 100644
--- a/src/box/lua/space.cc
+++ b/src/box/lua/space.cc
@@ -507,7 +507,7 @@ lbox_space_frommap(struct lua_State *L)
 
 	lua_replace(L, 1);
 	lua_settop(L, 1);
-	tuple = luaT_tuple_new(L, space->format);
+	tuple = luaT_tuple_new(L, -1, space->format);
 	if (tuple == NULL) {
 		struct error *e = diag_last_error(diag_get());
 		lua_pushnil(L);
diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c
index cff0beca5..8a8fd4d34 100644
--- a/src/box/lua/tuple.c
+++ b/src/box/lua/tuple.c
@@ -98,37 +98,38 @@ luaT_istuple(struct lua_State *L, int narg)
 }
 
 struct tuple *
-luaT_tuple_new(struct lua_State *L, struct tuple_format *format)
+luaT_tuple_new(struct lua_State *L, int idx, box_tuple_format_t *format)
 {
-	int argc = lua_gettop(L);
-	if (argc < 1) {
-		lua_newtable(L); /* create an empty tuple */
-		++argc;
+	if (idx != 0 && !lua_istable(L, idx) && !luaT_istuple(L, idx)) {
+		diag_set(IllegalParams, "A tuple or a table expected, got %s",
+			 lua_typename(L, lua_type(L, idx)));
+		return NULL;
 	}
-	struct ibuf *buf = tarantool_lua_ibuf;
 
+	struct ibuf *buf = tarantool_lua_ibuf;
 	ibuf_reset(buf);
 	struct mpstream stream;
 	mpstream_init(&stream, buf, ibuf_reserve_cb, ibuf_alloc_cb,
 		      luamp_error, L);
-
-	if (argc == 1 && (lua_istable(L, 1) || luaT_istuple(L, 1))) {
-		/* New format: box.tuple.new({1, 2, 3}) */
-		luamp_encode_tuple(L, &tuple_serializer, &stream, 1);
-	} else {
-		/* Backward-compatible format: box.tuple.new(1, 2, 3). */
+	if (idx == 0) {
+		/*
+		 * Create the tuple from lua stack
+		 * objects.
+		 */
+		int argc = lua_gettop(L);
 		luamp_encode_array(luaL_msgpack_default, &stream, argc);
 		for (int k = 1; k <= argc; ++k) {
 			luamp_encode(L, luaL_msgpack_default, &stream, k);
 		}
+	} else {
+		/* Create the tuple from a Lua table. */
+		luamp_encode_tuple(L, &tuple_serializer, &stream, idx);
 	}
 	mpstream_flush(&stream);
-
 	struct tuple *tuple = box_tuple_new(format, buf->buf,
-					   buf->buf + ibuf_used(buf));
+					    buf->buf + ibuf_used(buf));
 	if (tuple == NULL)
 		return NULL;
-	/* box_tuple_new() doesn't leak on exception, see public API doc */
 	ibuf_reinit(tarantool_lua_ibuf);
 	return tuple;
 }
@@ -136,9 +137,23 @@ luaT_tuple_new(struct lua_State *L, struct tuple_format *format)
 static int
 lbox_tuple_new(lua_State *L)
 {
-	struct tuple *tuple = luaT_tuple_new(L, box_tuple_format_default());
+	int argc = lua_gettop(L);
+	if (argc < 1) {
+		lua_newtable(L); /* create an empty tuple */
+		++argc;
+	}
+	/*
+	 * Use backward-compatible parameters format:
+	 * box.tuple.new(1, 2, 3) (idx == 0), or the new one:
+	 * box.tuple.new({1, 2, 3}) (idx == 1).
+	 */
+	int idx = argc == 1 && (lua_istable(L, 1) ||
+		luaT_istuple(L, 1));
+	box_tuple_format_t *fmt = box_tuple_format_default();
+	struct tuple *tuple = luaT_tuple_new(L, idx, fmt);
 	if (tuple == NULL)
 		return luaT_error(L);
+	/* box_tuple_new() doesn't leak on exception, see public API doc */
 	luaT_pushtuple(L, tuple);
 	return 1;
 }
diff --git a/src/box/lua/tuple.h b/src/box/lua/tuple.h
index 48b630969..dd3aa0f08 100644
--- a/src/box/lua/tuple.h
+++ b/src/box/lua/tuple.h
@@ -42,6 +42,8 @@ typedef struct tuple box_tuple_t;
 struct lua_State;
 struct mpstream;
 struct luaL_serializer;
+struct tuple_format;
+typedef struct tuple_format box_tuple_format_t;
 
 /** \cond public */
 
@@ -67,8 +69,17 @@ luaT_istuple(struct lua_State *L, int idx);
 
 /** \endcond public */
 
+/**
+ * Create a new tuple with specific format from a Lua table, a
+ * tuple, or objects on the lua stack.
+ *
+ * Set idx to zero to create the new tuple from objects on the lua
+ * stack.
+ *
+ * In case of an error set a diag and return NULL.
+ */
 struct tuple *
-luaT_tuple_new(struct lua_State *L, struct tuple_format *format);
+luaT_tuple_new(struct lua_State *L, int idx, box_tuple_format_t *format);
 
 static inline int
 luaT_pushtupleornil(struct lua_State *L, struct tuple *tuple)
diff --git a/test/unit/CMakeLists.txt b/test/unit/CMakeLists.txt
index d0138e8fd..255d27083 100644
--- a/test/unit/CMakeLists.txt
+++ b/test/unit/CMakeLists.txt
@@ -148,6 +148,10 @@ add_executable(histogram.test histogram.c)
 target_link_libraries(histogram.test stat unit)
 add_executable(ratelimit.test ratelimit.c)
 target_link_libraries(ratelimit.test unit)
+add_executable(luaT_tuple_new.test luaT_tuple_new.c)
+target_link_libraries(luaT_tuple_new.test unit box server core misc
+    ${CURL_LIBRARIES} ${LIBYAML_LIBRARIES} ${READLINE_LIBRARIES}
+    ${ICU_LIBRARIES} ${LUAJIT_LIBRARIES})
 
 add_executable(say.test say.c)
 target_link_libraries(say.test core unit)
diff --git a/test/unit/luaT_tuple_new.c b/test/unit/luaT_tuple_new.c
new file mode 100644
index 000000000..522216ce1
--- /dev/null
+++ b/test/unit/luaT_tuple_new.c
@@ -0,0 +1,175 @@
+#include <string.h>           /* strncmp() */
+#include <lua.h>              /* lua_*() */
+#include <lauxlib.h>          /* luaL_*() */
+#include <lualib.h>           /* luaL_openlibs() */
+#include "unit.h"             /* plan, header, footer, is, ok */
+#include "memory.h"           /* memory_init() */
+#include "fiber.h"            /* fiber_init() */
+#include "small/ibuf.h"       /* struct ibuf */
+#include "box/box.h"          /* box_init() */
+#include "box/tuple.h"        /* box_tuple_format_default() */
+#include "lua/msgpack.h"      /* luaopen_msgpack() */
+#include "box/lua/tuple.h"    /* luaT_tuple_new() */
+#include "diag.h"             /* struct error, diag_*() */
+#include "exception.h"        /* type_IllegalParams */
+
+/*
+ * This test checks all usage cases of luaT_tuple_new():
+ *
+ * * Use with idx == 0 and idx != 0.
+ * * Use with default and non-default formats.
+ * * Use a table and a tuple as an input.
+ * * Use with an unexpected lua type as an input.
+ *
+ * The test does not vary an input table/tuple. This is done in
+ * box/tuple.test.lua.
+ */
+
+extern struct ibuf *tarantool_lua_ibuf;
+
+uint32_t
+min_u32(uint32_t a, uint32_t b)
+{
+	return a < b ? a : b;
+}
+
+void
+check_tuple(const struct tuple *tuple, box_tuple_format_t *format,
+	    int retvals, const char *case_name)
+{
+	uint32_t size;
+	const char *data = tuple_data_range(tuple, &size);
+
+	ok(tuple != NULL, "%s: tuple != NULL", case_name);
+	is(tuple->format_id, tuple_format_id(format),
+	   "%s: check tuple format id", case_name);
+	is(size, 4, "%s: check tuple size", case_name);
+	ok(!strncmp(data, "\x93\x01\x02\x03", min_u32(size, 4)),
+	   "%s: check tuple data", case_name);
+	is(retvals, 0, "%s: check retvals count", case_name);
+}
+
+void
+check_error(struct lua_State *L, const struct tuple *tuple, int retvals,
+	    const char *case_name)
+{
+	const char *exp_err = "A tuple or a table expected, got number";
+	is(tuple, NULL, "%s: tuple == NULL", case_name);
+	is(retvals, 0, "%s: check retvals count", case_name);
+	struct error *e = diag_last_error(diag_get());
+	is(e->type, &type_IllegalParams, "%s: check error type", case_name);
+	ok(!strcmp(e->errmsg, exp_err), "%s: check error message", case_name);
+}
+
+int
+test_basic(struct lua_State *L)
+{
+	plan(19);
+	header();
+
+	int top;
+	struct tuple *tuple;
+	box_tuple_format_t *default_format = box_tuple_format_default();
+
+	/*
+	 * Case: a Lua table on idx == -2 as an input.
+	 */
+
+	/* Prepare the Lua stack. */
+	luaL_loadstring(L, "return {1, 2, 3}");
+	lua_call(L, 0, 1);
+	lua_pushnil(L);
+
+	/* Create and check a tuple. */
+	top = lua_gettop(L);
+	tuple = luaT_tuple_new(L, -2, default_format);
+	check_tuple(tuple, default_format, lua_gettop(L) - top, "table");
+
+	/* Clean up. */
+	lua_pop(L, 2);
+	assert(lua_gettop(L) == 0);
+
+	/*
+	 * Case: a tuple on idx == -1 as an input.
+	 */
+
+	/* Prepare the Lua stack. */
+	luaT_pushtuple(L, tuple);
+
+	/* Create and check a tuple. */
+	top = lua_gettop(L);
+	tuple = luaT_tuple_new(L, -1, default_format);
+	check_tuple(tuple, default_format, lua_gettop(L) - top, "tuple");
+
+	/* Clean up. */
+	lua_pop(L, 1);
+	assert(lua_gettop(L) == 0);
+
+	/*
+	 * Case: elements on the stack (idx == 0) as an input and
+	 * a non-default format.
+	 */
+
+	/* Prepare the Lua stack. */
+	lua_pushinteger(L, 1);
+	lua_pushinteger(L, 2);
+	lua_pushinteger(L, 3);
+
+	/* Create a new format. */
+	struct key_part_def part;
+	part.fieldno = 0;
+	part.type = FIELD_TYPE_INTEGER;
+	part.coll_id = COLL_NONE;
+	part.is_nullable = false;
+	struct key_def *key_def = key_def_new(&part, 1);
+	box_tuple_format_t *another_format = box_tuple_format_new(&key_def, 1);
+	key_def_delete(key_def);
+
+	/* Create and check a tuple. */
+	top = lua_gettop(L);
+	tuple = luaT_tuple_new(L, 0, another_format);
+	check_tuple(tuple, another_format, lua_gettop(L) - top, "objects");
+
+	/* Clean up. */
+	tuple_format_delete(another_format);
+	lua_pop(L, 3);
+	assert(lua_gettop(L) == 0);
+
+	/*
+	 * Case: a lua object of an unexpected type.
+	 */
+
+	/* Prepare the Lua stack. */
+	lua_pushinteger(L, 42);
+
+	/* Try to create and check for the error. */
+	top = lua_gettop(L);
+	tuple = luaT_tuple_new(L, -1, default_format);
+	check_error(L, tuple, lua_gettop(L) - top, "unexpected type");
+
+	/* Clean up. */
+	lua_pop(L, 1);
+	assert(lua_gettop(L) == 0);
+
+	footer();
+	return check_plan();
+}
+
+int
+main()
+{
+	memory_init();
+	fiber_init(fiber_c_invoke);
+
+	ibuf_create(tarantool_lua_ibuf, &cord()->slabc, 16000);
+
+	struct lua_State *L = luaL_newstate();
+	luaL_openlibs(L);
+
+	box_init();
+	luaopen_msgpack(L);
+	box_lua_tuple_init(L);
+	lua_pop(L, 1);
+
+	return test_basic(L);
+}
diff --git a/test/unit/luaT_tuple_new.result b/test/unit/luaT_tuple_new.result
new file mode 100644
index 000000000..110aa68c2
--- /dev/null
+++ b/test/unit/luaT_tuple_new.result
@@ -0,0 +1,22 @@
+1..19
+	*** test_basic ***
+ok 1 - table: tuple != NULL
+ok 2 - table: check tuple format id
+ok 3 - table: check tuple size
+ok 4 - table: check tuple data
+ok 5 - table: check retvals count
+ok 6 - tuple: tuple != NULL
+ok 7 - tuple: check tuple format id
+ok 8 - tuple: check tuple size
+ok 9 - tuple: check tuple data
+ok 10 - tuple: check retvals count
+ok 11 - objects: tuple != NULL
+ok 12 - objects: check tuple format id
+ok 13 - objects: check tuple size
+ok 14 - objects: check tuple data
+ok 15 - objects: check retvals count
+ok 16 - unexpected type: tuple == NULL
+ok 17 - unexpected type: check retvals count
+ok 18 - unexpected type: check error type
+ok 19 - unexpected type: check error message
+	*** test_basic: done ***
-- 
2.25.0