[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