From 96ac7b912e47f14a952c6b02f7f04f1f2fd92a0c Mon Sep 17 00:00:00 2001
From: Vladimir Davydov <vdavydov@tarantool.org>
Date: Thu, 10 Aug 2023 16:55:12 +0300
Subject: [PATCH] lua: move check param helpers to internal.utils

The check_param and check_param_table Lua helpers are defined in
box/lua/schema.lua but used across the whole code base. The problem is
we can't use them in files that are loaded before box/lua/schema.lua,
like box/lua/session.lua. Let's move them to a separate source file
lua/utils.lua to overcome this limitation. Also, let's add some tests.

NO_DOC=refactoring
NO_CHANGELOG=refactoring

(cherry picked from commit d8d267c590504f036ac96e455b607cbd7eaf182e)
---
 src/CMakeLists.txt              |   1 +
 src/box/lua/net_box.lua         |   3 +-
 src/box/lua/schema.lua          | 119 ++------------------------------
 src/box/lua/upgrade.lua         |   3 +-
 src/lua/init.c                  |   2 +
 src/lua/utils.lua               | 114 ++++++++++++++++++++++++++++++
 test/app-luatest/utils_test.lua |  67 ++++++++++++++++++
 7 files changed, 194 insertions(+), 115 deletions(-)
 create mode 100644 src/lua/utils.lua
 create mode 100644 test/app-luatest/utils_test.lua

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 8f00d69170..8b70dcf599 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -57,6 +57,7 @@ lua_source(lua_sources lua/csv.lua csv_lua)
 lua_source(lua_sources lua/strict.lua strict_lua)
 lua_source(lua_sources lua/clock.lua clock_lua)
 lua_source(lua_sources lua/title.lua title_lua)
+lua_source(lua_sources lua/utils.lua utils_lua)
 lua_source(lua_sources lua/argparse.lua argparse_lua)
 lua_source(lua_sources lua/env.lua env_lua)
 lua_source(lua_sources lua/pwd.lua pwd_lua)
diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 6867132852..e7a5bd3cd3 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -6,6 +6,7 @@ local msgpack  = require('msgpack')
 local urilib   = require('uri')
 local internal = require('net.box.lib')
 local trigger  = require('internal.trigger')
+local utils    = require('internal.utils')
 
 local this_module
 
@@ -16,7 +17,7 @@ local check_select_opts   = box.internal.check_select_opts
 local check_index_arg     = box.internal.check_index_arg
 local check_space_arg     = box.internal.check_space_arg
 local check_primary_index = box.internal.check_primary_index
-local check_param_table   = box.internal.check_param_table
+local check_param_table   = utils.check_param_table
 
 local ibuf_t = ffi.typeof('struct ibuf')
 local is_tuple = box.tuple.is
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index abf8e5f493..c987170abd 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -9,6 +9,12 @@ local fiber = require('fiber')
 local session = box.session
 local internal = box.internal
 local utf8 = require('utf8')
+local utils = require('internal.utils')
+
+local check_param = utils.check_param
+local check_param_table = utils.check_param_table
+local update_param_table = utils.update_param_table
+
 local function setmap(table)
     return setmetatable(table, { __serialize = 'map' })
 end
@@ -234,119 +240,6 @@ local function revoke_object_privs(object_type, object_id)
     end
 end
 
--- Same as type(), but returns 'number' if 'param' is
--- of type 'cdata' and represents a 64-bit integer.
-local function param_type(param)
-    local t = type(param)
-    if t == 'cdata' and tonumber64(param) ~= nil then
-        t = 'number'
-    end
-    return t
-end
-
---[[
- @brief Common function to check table with parameters (like options)
- @param table - table with parameters
- @param template - table with expected types of expected parameters
-  type could be comma separated string with lua types (number, string etc),
-  or 'any' if any type is allowed. Instead of this string, function, which will
-  be used to check if the parameter is correct, can be used too. It should
-  accept option as an argument and return either true or false + expected_type.
- The function checks following:
- 1)that parameters table is a table (or nil)
- 2)all keys in parameters are present in template
- 3)type of every parameter fits (one of) types described in template
- The functions calls box.error(box.error.ILLEGAL_PARAMS, ..) on error
- @example
- check_param_table(options, { user = 'string',
-                              port = 'string, number',
-                              data = 'any',
-                              addr = function(opt)
-                                if not ffi.istype(addr_t, buf) then
-                                    return false, "struct addr"
-                                end
-                                return true
-                              end} )
---]]
-local function check_param_table(table, template)
-    if table == nil then
-        return
-    end
-    if type(table) ~= 'table' then
-        box.error(box.error.ILLEGAL_PARAMS,
-                  "options should be a table")
-    end
-    for k,v in pairs(table) do
-        if template[k] == nil then
-            box.error(box.error.ILLEGAL_PARAMS,
-                      "unexpected option '" .. k .. "'")
-        elseif type(template[k]) == 'function' then
-            local res, expected_type = template[k](v)
-            if not res then
-                box.error(box.error.ILLEGAL_PARAMS,
-                          "options parameter '" .. k ..
-                          "' should be of type " .. expected_type)
-            end
-        elseif template[k] == 'any' then -- luacheck: ignore
-            -- any type is ok
-        elseif (string.find(template[k], ',') == nil) then
-            -- one type
-            if param_type(v) ~= template[k] then
-                box.error(box.error.ILLEGAL_PARAMS,
-                          "options parameter '" .. k ..
-                          "' should be of type " .. template[k])
-            end
-        else
-            local good_types = string.gsub(template[k], ' ', '')
-            local haystack = ',' .. good_types .. ','
-            local needle = ',' .. param_type(v) .. ','
-            if (string.find(haystack, needle) == nil) then
-                box.error(box.error.ILLEGAL_PARAMS,
-                          "options parameter '" .. k ..
-                          "' should be one of types: " .. template[k])
-            end
-        end
-    end
-end
-box.internal.check_param_table = check_param_table
-
---[[
- @brief Common function to check type parameter (of function)
- Calls box.error(box.error.ILLEGAL_PARAMS, ) on error
- @example: check_param(user, 'user', 'string')
---]]
-local function check_param(param, name, should_be_type)
-    if param_type(param) ~= should_be_type then
-        box.error(box.error.ILLEGAL_PARAMS,
-                  name .. " should be a " .. should_be_type)
-    end
-end
-box.internal.check_param = check_param
-
---[[
- Adds to a table key-value pairs from defaults table
-  that is not present in original table.
- Returns updated table.
- If nil is passed instead of table, it's treated as empty table {}
- For example update_param_table({ type = 'hash', temporary = true },
-                                { type = 'tree', unique = true })
-  will return table { type = 'hash', temporary = true, unique = true }
---]]
-local function update_param_table(table, defaults)
-    local new_table = {}
-    if defaults ~= nil then
-        for k,v in pairs(defaults) do
-            new_table[k] = v
-        end
-    end
-    if table ~= nil then
-        for k,v in pairs(table) do
-            new_table[k] = v
-        end
-    end
-    return new_table
-end
-
 local function feedback_save_event(event)
     if internal.feedback_daemon ~= nil then
         internal.feedback_daemon.save_event(event)
diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
index 726323a6a1..f0a363c342 100644
--- a/src/box/lua/upgrade.lua
+++ b/src/box/lua/upgrade.lua
@@ -6,6 +6,7 @@ local xlog = require('xlog')
 local ffi = require('ffi')
 local fun = require('fun')
 local tarantool = require('tarantool')
+local utils = require('internal.utils')
 
 ffi.cdef([[
     uint32_t box_dd_version_id(void);
@@ -2034,7 +2035,7 @@ local downgrade_versions = {
 -- In case of downgrade check for issues is done before making any changes.
 -- If any issue is found then downgrade is failed and no any changes are done.
 local function downgrade_impl(version_str, dry_run)
-    box.internal.check_param(version_str, 'version_str', 'string')
+    utils.check_param(version_str, 'version_str', 'string')
     local version = mkversion.parse(version_str)
     if fun.index(version_str, downgrade_versions) == nil then
         error("Downgrade is only possible to version listed in" ..
diff --git a/src/lua/init.c b/src/lua/init.c
index 389abddf88..94eed570da 100644
--- a/src/lua/init.c
+++ b/src/lua/init.c
@@ -135,6 +135,7 @@ extern char minifio_lua[],
 	tap_lua[],
 	fio_lua[],
 	error_lua[],
+	utils_lua[],
 	argparse_lua[],
 	iconv_lua[],
 	/* jit.* library */
@@ -302,6 +303,7 @@ static const char *lua_modules[] = {
 	"tap", tap_lua,
 	"help.en_US", help_en_US_lua,
 	"help", help_lua,
+	"internal.utils", utils_lua,
 	"internal.argparse", argparse_lua,
 	"internal.trigger", trigger_lua,
 	"pwd", pwd_lua,
diff --git a/src/lua/utils.lua b/src/lua/utils.lua
new file mode 100644
index 0000000000..53797e2834
--- /dev/null
+++ b/src/lua/utils.lua
@@ -0,0 +1,114 @@
+local utils = {}
+
+-- Same as type(), but returns 'number' if 'param' is
+-- of type 'cdata' and represents a 64-bit integer.
+local function param_type(param)
+    local t = type(param)
+    if t == 'cdata' and tonumber64(param) ~= nil then
+        t = 'number'
+    end
+    return t
+end
+
+--[[
+ @brief Common function to check type parameter (of function)
+ Calls box.error(box.error.ILLEGAL_PARAMS, ) on error
+ @example: check_param(user, 'user', 'string')
+--]]
+function utils.check_param(param, name, should_be_type)
+    if param_type(param) ~= should_be_type then
+        box.error(box.error.ILLEGAL_PARAMS,
+                  name .. " should be a " .. should_be_type)
+    end
+end
+
+--[[
+ @brief Common function to check table with parameters (like options)
+ @param table - table with parameters
+ @param template - table with expected types of expected parameters
+  type could be comma separated string with lua types (number, string etc),
+  or 'any' if any type is allowed. Instead of this string, function, which will
+  be used to check if the parameter is correct, can be used too. It should
+  accept option as an argument and return either true or false + expected_type.
+ The function checks following:
+ 1)that parameters table is a table (or nil)
+ 2)all keys in parameters are present in template
+ 3)type of every parameter fits (one of) types described in template
+ The functions calls box.error(box.error.ILLEGAL_PARAMS, ..) on error
+ @example
+ check_param_table(options, { user = 'string',
+                              port = 'string, number',
+                              data = 'any',
+                              addr = function(opt)
+                                if not ffi.istype(addr_t, buf) then
+                                    return false, "struct addr"
+                                end
+                                return true
+                              end} )
+--]]
+function utils.check_param_table(table, template)
+    if table == nil then
+        return
+    end
+    if type(table) ~= 'table' then
+        box.error(box.error.ILLEGAL_PARAMS,
+                  "options should be a table")
+    end
+    for k,v in pairs(table) do
+        if template[k] == nil then
+            box.error(box.error.ILLEGAL_PARAMS,
+                      "unexpected option '" .. k .. "'")
+        elseif type(template[k]) == 'function' then
+            local res, expected_type = template[k](v)
+            if not res then
+                box.error(box.error.ILLEGAL_PARAMS,
+                          "options parameter '" .. k ..
+                          "' should be of type " .. expected_type)
+            end
+        elseif template[k] == 'any' then -- luacheck: ignore
+            -- any type is ok
+        elseif (string.find(template[k], ',') == nil) then
+            -- one type
+            if param_type(v) ~= template[k] then
+                box.error(box.error.ILLEGAL_PARAMS,
+                          "options parameter '" .. k ..
+                          "' should be of type " .. template[k])
+            end
+        else
+            local good_types = string.gsub(template[k], ' ', '')
+            local haystack = ',' .. good_types .. ','
+            local needle = ',' .. param_type(v) .. ','
+            if (string.find(haystack, needle) == nil) then
+                box.error(box.error.ILLEGAL_PARAMS,
+                          "options parameter '" .. k ..
+                          "' should be one of types: " .. template[k])
+            end
+        end
+    end
+end
+
+--[[
+ Adds to a table key-value pairs from defaults table
+  that is not present in original table.
+ Returns updated table.
+ If nil is passed instead of table, it's treated as empty table {}
+ For example update_param_table({ type = 'hash', temporary = true },
+                                { type = 'tree', unique = true })
+  will return table { type = 'hash', temporary = true, unique = true }
+--]]
+function utils.update_param_table(table, defaults)
+    local new_table = {}
+    if defaults ~= nil then
+        for k,v in pairs(defaults) do
+            new_table[k] = v
+        end
+    end
+    if table ~= nil then
+        for k,v in pairs(table) do
+            new_table[k] = v
+        end
+    end
+    return new_table
+end
+
+return utils
diff --git a/test/app-luatest/utils_test.lua b/test/app-luatest/utils_test.lua
new file mode 100644
index 0000000000..1b210d1ba3
--- /dev/null
+++ b/test/app-luatest/utils_test.lua
@@ -0,0 +1,67 @@
+local utils = require('internal.utils')
+local t = require('luatest')
+
+local g = t.group()
+
+g.test_check_param = function()
+    t.assert_error_msg_equals("Illegal parameters, foo should be a string",
+                              utils.check_param, 42, "foo", 'string')
+    t.assert_error_msg_equals("Illegal parameters, foo should be a number",
+                              utils.check_param, '42', "foo", 'number')
+    t.assert(pcall(utils.check_param, '42', 'foo', 'string'))
+    t.assert(pcall(utils.check_param, 42, 'foo', 'number'))
+    t.assert(pcall(utils.check_param, 123456789123456789ULL, 'foo', 'number'))
+end
+
+g.test_check_param_table = function()
+    local opts = {
+        any_opt = 'any',
+        str_opt = 'string',
+        str_num_opt = 'string,number',
+        tuple_opt = function(opt)
+            return box.tuple.is(opt), 'tuple'
+        end
+    }
+    t.assert_error_msg_equals("Illegal parameters, options should be a table",
+                              utils.check_param_table, 'foo', opts)
+    t.assert_error_msg_equals("Illegal parameters, unexpected option 'foo'",
+                              utils.check_param_table, {foo = 'bar'}, opts)
+    t.assert_error_msg_equals("Illegal parameters, options parameter " ..
+                              "'tuple_opt' should be of type tuple",
+                              utils.check_param_table, {tuple_opt = {}}, opts)
+    t.assert_error_msg_equals("Illegal parameters, options parameter " ..
+                              "'str_opt' should be of type string",
+                              utils.check_param_table, {str_opt = 42}, opts)
+    t.assert_error_msg_equals("Illegal parameters, options parameter " ..
+                              "'str_num_opt' should be one of types: " ..
+                              "string,number",
+                              utils.check_param_table, {str_num_opt = {}}, opts)
+    t.assert(pcall(utils.check_param_table, nil, opts))
+    t.assert(pcall(utils.check_param_table, {}, opts))
+    t.assert(pcall(utils.check_param_table, {
+        any_opt = 'foo',
+        str_opt = 'bar',
+        str_num_opt = 42,
+        tuple_opt = box.tuple.new(),
+    }, opts))
+    t.assert(pcall(utils.check_param_table, {
+        any_opt = {},
+        str_num_opt = 'foo',
+    }, opts))
+end
+
+g.test_update_param_table = function()
+    local opts = {
+        a = 'foo',
+        b = 'bar',
+    }
+    local defaults = {
+        b = 'x',
+        c = 'y',
+    }
+    t.assert_equals(utils.update_param_table(opts, defaults), {
+        a = 'foo',
+        b = 'bar',
+        c = 'y',
+    })
+end
-- 
GitLab