[PATCH v1 1/2] netbox: store method_encoder args in request
Mergen Imeev
imeevma at tarantool.org
Fri Jun 14 14:50:23 MSK 2019
On Tue, Jun 11, 2019 at 11:15:06AM +0300, Vladimir Davydov wrote:
> On Mon, Jun 10, 2019 at 01:02:23PM +0300, imeevma at tarantool.org wrote:
> > This patch changes the way arguments are passed to functions in
> > the array method_encoder, and saves these arguments in REQUEST.
> > This is necessary to establish a connection between netbox space
> > objects and the tuples obtained through the netbox
> >
> > Needed for #2978
> > ---
> > src/box/lua/net_box.lua | 179 +++++++++++++++++++++++++++++++---------------
> > test/box/access.result | 15 +++-
> > test/box/access.test.lua | 9 ++-
> > test/box/net.box.result | 6 +-
> > test/box/net.box.test.lua | 6 +-
> > 5 files changed, 146 insertions(+), 69 deletions(-)
> >
> > diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
> > index 251ad40..8d42fb4 100644
> > --- a/src/box/lua/net_box.lua
> > +++ b/src/box/lua/net_box.lua
> > @@ -25,7 +25,6 @@ local check_primary_index = box.internal.check_primary_index
> >
> > local communicate = internal.communicate
> > local encode_auth = internal.encode_auth
> > -local encode_select = internal.encode_select
> > local decode_greeting = internal.decode_greeting
> >
> > local TIMEOUT_INFINITY = 500 * 365 * 86400
> > @@ -84,22 +83,70 @@ local function decode_push(raw_data)
> > return response[IPROTO_DATA_KEY][1], raw_end
> > end
> >
> > +local function encode_call(send_buf, id, method_args)
> > + return internal.encode_call(send_buf, id, method_args.func_name,
> > + method_args.args)
> > +end
> > +local function encode_call_16(send_buf, id, method_args)
> > + return internal.encode_call_16(send_buf, id, method_args.func_name,
> > + method_args.args)
> > +end
> > +local function encode_ping(send_buf, id, ...)
> > + return internal.encode_ping(send_buf, id, ...)
> > +end
> > +local function encode_eval(send_buf, id, method_args)
> > + return internal.encode_eval(send_buf, id, method_args.code,
> > + method_args.args)
> > +end
> > +local function encode_insert(send_buf, id, method_args)
> > + return internal.encode_insert(send_buf, id, method_args.space_id,
> > + method_args.tuple)
> > +end
> > +local function encode_replace(send_buf, id, method_args)
> > + return internal.encode_replace(send_buf, id, method_args.space_id,
> > + method_args.tuple)
> > +end
> > +local function encode_delete(send_buf, id, method_args)
> > + return internal.encode_delete(send_buf, id, method_args.space_id,
> > + method_args.index_id, method_args.key)
> > +end
> > +local function encode_update(send_buf, id, method_args)
> > + return internal.encode_update(send_buf, id, method_args.space_id,
> > + method_args.index_id, method_args.key,
> > + method_args.oplist)
> > +end
> > +local function encode_upsert(send_buf, id, method_args)
> > + return internal.encode_upsert(send_buf, id, method_args.space_id,
> > + method_args.key, method_args.oplist)
> > +end
> > +local function encode_select(send_buf, id, method_args)
> > + return internal.encode_select(send_buf, id, method_args.space_id,
> > + method_args.index_id, method_args.iterator,
> > + method_args.offset, method_args.limit,
> > + method_args.key)
> > +end
> > +local function encode_execute(send_buf, id, method_args)
> > + return internal.encode_execute(send_buf, id, method_args.query,
> > + method_args.parameters, method_args.sql_opts)
> > +end
>
> Why not call it simply 'args'?
>
Fixed. I used 'method_args' since 'args' was used in call
and eval.
> > @@ -1150,14 +1201,16 @@ end
> > -- @deprecated since 1.7.4
> > function remote_methods:eval_16(code, ...)
> > check_remote_arg(self, 'eval')
> > - return unpack((self:_request('eval', nil, code, {...})))
> > + local method_args = {code=code, args={...}}
> > + return unpack((self:_request('eval', nil, method_args)))
> > end
> >
> > function remote_methods:eval(code, args, opts)
> > check_remote_arg(self, 'eval')
> > check_eval_args(args)
> > args = args or {}
> > - local res = self:_request('eval', opts, code, args)
> > + local method_args = {code=code, args=args}
>
> Coding style: we add a space before and after '=':
>
> local method_args = {code = code, args = args}
>
> Please fix here and everywhere else.
>
> Other than that this patch looks okay to me.
Fixed.
New patch:
>From 8be1560eb5ed57d397c5f2da97112b448505080c Mon Sep 17 00:00:00 2001
Date: Wed, 5 Jun 2019 13:31:07 +0300
Subject: [PATCH] netbox: store method_encoder args in request
This patch changes the way arguments are passed to functions in
the array method_encoder, and saves these arguments in REQUEST.
This is necessary to establish a connection between netbox space
objects and the tuples obtained through the netbox
Needed for #2978
diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 251ad40..d9838f8 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -25,7 +25,6 @@ local check_primary_index = box.internal.check_primary_index
local communicate = internal.communicate
local encode_auth = internal.encode_auth
-local encode_select = internal.encode_select
local decode_greeting = internal.decode_greeting
local TIMEOUT_INFINITY = 500 * 365 * 86400
@@ -84,22 +83,63 @@ local function decode_push(raw_data)
return response[IPROTO_DATA_KEY][1], raw_end
end
+local function encode_call(send_buf, id, args)
+ return internal.encode_call(send_buf, id, args.func_name, args.args)
+end
+local function encode_call_16(send_buf, id, args)
+ return internal.encode_call_16(send_buf, id, args.func_name, args.args)
+end
+local function encode_ping(send_buf, id, ...)
+ return internal.encode_ping(send_buf, id, ...)
+end
+local function encode_eval(send_buf, id, args)
+ return internal.encode_eval(send_buf, id, args.code, args.args)
+end
+local function encode_insert(send_buf, id, args)
+ return internal.encode_insert(send_buf, id, args.space_id, args.tuple)
+end
+local function encode_replace(send_buf, id, args)
+ return internal.encode_replace(send_buf, id, args.space_id, args.tuple)
+end
+local function encode_delete(send_buf, id, args)
+ return internal.encode_delete(send_buf, id, args.space_id, args.index_id,
+ args.key)
+end
+local function encode_update(send_buf, id, args)
+ return internal.encode_update(send_buf, id, args.space_id, args.index_id,
+ args.key, args.oplist)
+end
+local function encode_upsert(send_buf, id, args)
+ return internal.encode_upsert(send_buf, id, args.space_id, args.key,
+ args.oplist)
+end
+local function encode_select(send_buf, id, args)
+ return internal.encode_select(send_buf, id, args.space_id, args.index_id,
+ args.iterator, args.offset, args.limit,
+ args.key)
+end
+local function encode_execute(send_buf, id, args)
+ return internal.encode_execute(send_buf, id, args.query, args.parameters,
+ args.sql_opts)
+end
+
+
local method_encoder = {
- ping = internal.encode_ping,
- call_16 = internal.encode_call_16,
- call_17 = internal.encode_call,
- eval = internal.encode_eval,
- insert = internal.encode_insert,
- replace = internal.encode_replace,
- delete = internal.encode_delete,
- update = internal.encode_update,
- upsert = internal.encode_upsert,
- select = internal.encode_select,
- execute = internal.encode_execute,
- get = internal.encode_select,
- min = internal.encode_select,
- max = internal.encode_select,
- count = internal.encode_call,
+ ping = encode_ping,
+ call_16 = encode_call_16,
+ call_17 = encode_call,
+ eval = encode_eval,
+ insert = encode_insert,
+ replace = encode_replace,
+ delete = encode_delete,
+ update = encode_update,
+ upsert = encode_upsert,
+ select = encode_select,
+ execute = encode_execute,
+ get = encode_select,
+ min = encode_select,
+ max = encode_select,
+ count = encode_call,
-- inject raw data into connection, used by console and tests
inject = function(buf, id, bytes)
local ptr = buf:reserve(#bytes)
@@ -486,7 +526,7 @@ local function create_transport(host, port, user, password, callback,
-- @retval not nil Future object.
--
local function perform_async_request(buffer, skip_header, method, on_push,
- on_push_ctx, ...)
+ on_push_ctx, args)
if state ~= 'active' and state ~= 'fetch_schema' then
return nil, box.error.new({code = last_errno or E_NO_CONNECTION,
reason = last_error})
@@ -497,12 +537,12 @@ local function create_transport(host, port, user, password, callback,
worker_fiber:wakeup()
end
local id = next_request_id
- method_encoder[method](send_buf, id, ...)
+ method_encoder[method](send_buf, id, args)
next_request_id = next_id(id)
- -- Request in most cases has maximum 9 members:
+ -- Request in most cases has maximum 10 members:
-- method, buffer, skip_header, id, cond, errno, response,
- -- on_push, on_push_ctx.
- local request = setmetatable(table_new(0, 9), request_mt)
+ -- on_push, on_push_ctx and args.
+ local request = setmetatable(table_new(0, 10), request_mt)
request.method = method
request.buffer = buffer
request.skip_header = skip_header
@@ -511,6 +551,7 @@ local function create_transport(host, port, user, password, callback,
requests[id] = request
request.on_push = on_push
request.on_push_ctx = on_push_ctx
+ request.args = args
return request
end
@@ -520,10 +561,10 @@ local function create_transport(host, port, user, password, callback,
-- @retval not nil Response object.
--
local function perform_request(timeout, buffer, skip_header, method,
- on_push, on_push_ctx, ...)
+ on_push, on_push_ctx, args)
local request, err =
perform_async_request(buffer, skip_header, method, on_push,
- on_push_ctx, ...)
+ on_push_ctx, args)
if not request then
return nil, err
end
@@ -739,13 +780,14 @@ local function create_transport(host, port, user, password, callback,
local select3_id = new_request_id()
local response = {}
-- fetch everything from space _vspace, 2 = ITER_ALL
- encode_select(send_buf, select1_id, VSPACE_ID, 0, 2, 0, 0xFFFFFFFF, nil)
+ internal.encode_select(send_buf, select1_id, VSPACE_ID, 0, 2, 0,
+ 0xFFFFFFFF, nil)
-- fetch everything from space _vindex, 2 = ITER_ALL
- encode_select(send_buf, select2_id, VINDEX_ID, 0, 2, 0, 0xFFFFFFFF, nil)
+ internal.encode_select(send_buf, select2_id, VINDEX_ID, 0, 2, 0,
+ 0xFFFFFFFF, nil)
-- fetch everything from space _vcollation, 2 = ITER_ALL
- encode_select(send_buf, select3_id, VCOLLATION_ID, 0, 2, 0, 0xFFFFFFFF,
- nil)
-
+ internal.encode_select(send_buf, select3_id, VCOLLATION_ID, 0, 2, 0,
+ 0xFFFFFFFF, nil)
schema_version = nil -- any schema_version will do provided that
-- it is consistent across responses
repeat
@@ -1064,7 +1106,7 @@ function remote_methods:wait_connected(timeout)
return self._transport.wait_state('active', timeout)
end
-function remote_methods:_request(method, opts, ...)
+function remote_methods:_request(method, opts, args)
local transport = self._transport
local on_push, on_push_ctx, buffer, skip_header, deadline
-- Extract options, set defaults, check if the request is
@@ -1078,7 +1120,7 @@ function remote_methods:_request(method, opts, ...)
end
local res, err =
transport.perform_async_request(buffer, skip_header, method,
- table.insert, {}, ...)
+ table.insert, {}, args)
if err then
box.error(err)
end
@@ -1106,7 +1148,7 @@ function remote_methods:_request(method, opts, ...)
end
local res, err = transport.perform_request(timeout, buffer, skip_header,
method, on_push, on_push_ctx,
- ...)
+ args)
if err then
box.error(err)
end
@@ -1133,14 +1175,16 @@ end
-- @deprecated since 1.7.4
function remote_methods:call_16(func_name, ...)
check_remote_arg(self, 'call')
- return (self:_request('call_16', nil, tostring(func_name), {...}))
+ local args = {func_name = tostring(func_name), args = {...}}
+ return (self:_request('call_16', nil, args))
end
-function remote_methods:call(func_name, args, opts)
+function remote_methods:call(func_name, call_args, opts)
check_remote_arg(self, 'call')
- check_call_args(args)
- args = args or {}
- local res = self:_request('call_17', opts, tostring(func_name), args)
+ check_call_args(call_args)
+ call_args = call_args or {}
+ local args = {func_name = tostring(func_name), args = call_args}
+ local res = self:_request('call_17', opts, args)
if type(res) ~= 'table' or opts and opts.is_async then
return res
end
@@ -1150,14 +1194,16 @@ end
-- @deprecated since 1.7.4
function remote_methods:eval_16(code, ...)
check_remote_arg(self, 'eval')
- return unpack((self:_request('eval', nil, code, {...})))
+ local args = {code = code, args = {...}}
+ return unpack((self:_request('eval', nil, args)))
end
-function remote_methods:eval(code, args, opts)
+function remote_methods:eval(code, eval_args, opts)
check_remote_arg(self, 'eval')
- check_eval_args(args)
- args = args or {}
- local res = self:_request('eval', opts, code, args)
+ check_eval_args(eval_args)
+ eval_args = eval_args or {}
+ local args = {code = code, args = eval_args}
+ local res = self:_request('eval', opts, args)
if type(res) ~= 'table' or opts and opts.is_async then
return res
end
@@ -1169,8 +1215,9 @@ function remote_methods:execute(query, parameters, sql_opts, netbox_opts)
if sql_opts ~= nil then
box.error(box.error.UNSUPPORTED, "execute", "options")
end
- return self:_request('execute', netbox_opts, query, parameters or {},
- sql_opts or {})
+ local args = {query =query, parameters = parameters or {},
+ sql_opts = sql_opts or {}}
+ return self:_request('execute', netbox_opts, args)
end
function remote_methods:wait_state(state, timeout)
@@ -1314,7 +1361,8 @@ function console_methods:eval(line, timeout)
end
if self.protocol == 'Binary' then
local loader = 'return require("console").eval(...)'
- res, err = pr(timeout, nil, false, 'eval', nil, nil, loader, {line})
+ local args = {code = loader, args = {line}}
+ res, err = pr(timeout, nil, false, 'eval', nil, nil, args)
else
assert(self.protocol == 'Lua console')
res, err = pr(timeout, nil, false, 'inject', nil, nil, line..'$EOF$\n')
@@ -1336,12 +1384,14 @@ space_metatable = function(remote)
function methods:insert(tuple, opts)
check_space_arg(self, 'insert')
- return remote:_request('insert', opts, self.id, tuple)
+ local args = {space_id = self.id, tuple = tuple}
+ return remote:_request('insert', opts, args)
end
function methods:replace(tuple, opts)
check_space_arg(self, 'replace')
- return remote:_request('replace', opts, self.id, tuple)
+ local args = {space_id = self.id, tuple = tuple}
+ return remote:_request('replace', opts, args)
end
function methods:select(key, opts)
@@ -1361,8 +1411,8 @@ space_metatable = function(remote)
function methods:upsert(key, oplist, opts)
check_space_arg(self, 'upsert')
- return nothing_or_data(remote:_request('upsert', opts, self.id, key,
- oplist))
+ local args = {space_id = self.id, key = key, oplist = oplist}
+ return nothing_or_data(remote:_request('upsert', opts, args))
end
function methods:get(key, opts)
@@ -1391,8 +1441,10 @@ index_metatable = function(remote)
local iterator = check_iterator_type(opts, key_is_nil)
local offset = tonumber(opts and opts.offset) or 0
local limit = tonumber(opts and opts.limit) or 0xFFFFFFFF
- return (remote:_request('select', opts, self.space.id, self.id,
- iterator, offset, limit, key))
+ local args = {space_id = self.space.id, index_id = self.id,
+ iterator = iterator, offset = offset, limit = limit,
+ key = key}
+ return (remote:_request('select', opts, args))
end
function methods:get(key, opts)
@@ -1400,8 +1452,9 @@ index_metatable = function(remote)
if opts and opts.buffer then
error("index:get() doesn't support `buffer` argument")
end
- return nothing_or_data(remote:_request('get', opts, self.space.id,
- self.id, box.index.EQ, 0, 2, key))
+ local args = {space_id = self.space.id, index_id = self.id,
+ iterator = box.index.EQ, offset = 0, limit = 2, key = key}
+ return nothing_or_data(remote:_request('get', opts, args))
end
function methods:min(key, opts)
@@ -1409,9 +1462,9 @@ index_metatable = function(remote)
if opts and opts.buffer then
error("index:min() doesn't support `buffer` argument")
end
- return nothing_or_data(remote:_request('min', opts, self.space.id,
- self.id, box.index.GE, 0, 1,
- key))
+ local args = {space_id = self.space.id, index_id = self.id,
+ iterator = box.index.GE, offset = 0, limit = 1, key = key}
+ return nothing_or_data(remote:_request('min', opts, args))
end
function methods:max(key, opts)
@@ -1419,9 +1472,9 @@ index_metatable = function(remote)
if opts and opts.buffer then
error("index:max() doesn't support `buffer` argument")
end
- return nothing_or_data(remote:_request('max', opts, self.space.id,
- self.id, box.index.LE, 0, 1,
- key))
+ local args = {space_id = self.space.id, index_id = self.id,
+ iterator = box.index.LE, offset = 0, limit = 1, key = key}
+ return nothing_or_data(remote:_request('max', opts, args))
end
function methods:count(key, opts)
@@ -1431,19 +1484,21 @@ index_metatable = function(remote)
end
local code = string.format('box.space.%s.index.%s:count',
self.space.name, self.name)
- return remote:_request('count', opts, code, { key, opts })
+ local args = {func_name = code, args = { key, opts }}
+ return remote:_request('count', opts, args)
end
function methods:delete(key, opts)
check_index_arg(self, 'delete')
- return nothing_or_data(remote:_request('delete', opts, self.space.id,
- self.id, key))
+ local args = {space_id = self.space.id, index_id = self.id, key = key}
+ return nothing_or_data(remote:_request('delete', opts, args))
end
function methods:update(key, oplist, opts)
check_index_arg(self, 'update')
- return nothing_or_data(remote:_request('update', opts, self.space.id,
- self.id, key, oplist))
+ local args = {space_id = self.space.id, index_id = self.id, key = key,
+ oplist = oplist}
+ return nothing_or_data(remote:_request('update', opts, args))
end
return { __index = methods, __metatable = false }
diff --git a/test/box/access.result b/test/box/access.result
index 0ebc6a3..e3d4036 100644
--- a/test/box/access.result
+++ b/test/box/access.result
@@ -890,15 +890,24 @@ LISTEN = require('uri').parse(box.cfg.listen)
c = (require 'net.box').connect(LISTEN.host, LISTEN.service)
---
...
-c:_request("select", nil, 1, box.index.EQ, 0, 0, 0xFFFFFFFF, {})
+args = {space_id=1, index_id=0, iterator=box.index.EQ, offset=0, limit=0xFFFFFFFF, key={}}
+---
+...
+c:_request("select", nil, args)
---
- error: Space '1' does not exist
...
-c:_request("select", nil, 65537, box.index.EQ, 0, 0, 0xFFFFFFFF, {})
+args.space_id = 65537
+---
+...
+c:_request("select", nil, args)
---
- error: Space '65537' does not exist
...
-c:_request("select", nil, 4294967295, box.index.EQ, 0, 0, 0xFFFFFFFF, {})
+args.space_id = 4294967295
+---
+...
+c:_request("select", nil, args)
---
- error: Space '4294967295' does not exist
...
diff --git a/test/box/access.test.lua b/test/box/access.test.lua
index ad63a10..c63152b 100644
--- a/test/box/access.test.lua
+++ b/test/box/access.test.lua
@@ -343,9 +343,12 @@ box.schema.func.drop(name)
-- very large space id, no crash occurs.
LISTEN = require('uri').parse(box.cfg.listen)
c = (require 'net.box').connect(LISTEN.host, LISTEN.service)
-c:_request("select", nil, 1, box.index.EQ, 0, 0, 0xFFFFFFFF, {})
-c:_request("select", nil, 65537, box.index.EQ, 0, 0, 0xFFFFFFFF, {})
-c:_request("select", nil, 4294967295, box.index.EQ, 0, 0, 0xFFFFFFFF, {})
+args = {space_id=1, index_id=0, iterator=box.index.EQ, offset=0, limit=0xFFFFFFFF, key={}}
+c:_request("select", nil, args)
+args.space_id = 65537
+c:_request("select", nil, args)
+args.space_id = 4294967295
+c:_request("select", nil, args)
c:close()
session = box.session
diff --git a/test/box/net.box.result b/test/box/net.box.result
index 474297a..e87a60b 100644
--- a/test/box/net.box.result
+++ b/test/box/net.box.result
@@ -25,9 +25,9 @@ test_run:cmd("setopt delimiter ';'")
- true
...
function x_select(cn, space_id, index_id, iterator, offset, limit, key, opts)
- local ret = cn:_request('select', opts, space_id, index_id, iterator,
- offset, limit, key)
- return ret
+ local args = {space_id = space_id, index_id = index_id, iterator = iterator,
+ offset = offset, limit = limit, key = key}
+ return cn:_request('select', opts, args)
end
function x_fatal(cn) cn._transport.perform_request(nil, nil, false, 'inject', nil, nil, '\x80') end
test_run:cmd("setopt delimiter ''");
diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
index bea4300..6e58da8 100644
--- a/test/box/net.box.test.lua
+++ b/test/box/net.box.test.lua
@@ -8,9 +8,9 @@ test_run:cmd("push filter ".."'\\.lua.*:[0-9]+: ' to '.lua...\"]:<line>: '")
test_run:cmd("setopt delimiter ';'")
function x_select(cn, space_id, index_id, iterator, offset, limit, key, opts)
- local ret = cn:_request('select', opts, space_id, index_id, iterator,
- offset, limit, key)
- return ret
+ local args = {space_id = space_id, index_id = index_id, iterator = iterator,
+ offset = offset, limit = limit, key = key}
+ return cn:_request('select', opts, args)
end
function x_fatal(cn) cn._transport.perform_request(nil, nil, false, 'inject', nil, nil, '\x80') end
test_run:cmd("setopt delimiter ''");
More information about the Tarantool-patches
mailing list