From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id BC02A291D4 for ; Fri, 1 Jun 2018 08:49:41 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id hbBJe9Cz8ONq for ; Fri, 1 Jun 2018 08:49:41 -0400 (EDT) Received: from smtp16.mail.ru (smtp16.mail.ru [94.100.176.153]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 75A2B291C8 for ; Fri, 1 Jun 2018 08:49:41 -0400 (EDT) From: AKhatskevich Subject: [tarantool-patches] [PATCH][vshard] Check self passed to object methods Date: Fri, 1 Jun 2018 15:49:12 +0300 Message-Id: <20180601124912.9145-1-avkhatskevich@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org, v.shpilevoy@tarantool.org Branch: https://github.com/tarantool/vshard/tree/kh/gh-81-check-self Issue: https://github.com/tarantool/vshard/issues/81 ------------ commit message ------------------ Changes: 1. `check_self_arg` function wrapper added to utils. Its mechanics: * check that the first argument is of table type * return result of a real function 2. Check, that self argument is passed to all replicaset and replica object methods. Closes: #81 --- test/router/router.result | 63 +++++++++++++++++++++++++++++++++++++++++++++ test/router/router.test.lua | 27 +++++++++++++++++++ vshard/replicaset.lua | 38 +++++++++++++++++---------- vshard/util.lua | 33 ++++++++++++++++++++++++ 4 files changed, 147 insertions(+), 14 deletions(-) diff --git a/test/router/router.result b/test/router/router.result index 5fcabc6..ea99a6a 100644 --- a/test/router/router.result +++ b/test/router/router.result @@ -977,6 +977,69 @@ util.check_error(vshard.router.sync, "xxx") vshard.router.sync(100500) --- ... +-- Check that user passed self arg +_, replicaset = next(vshard.router.internal.replicasets) +--- +... +error_messages = {} +--- +... +test_run:cmd("setopt delimiter ';'") +--- +- true +... +for _, func_name in pairs({ + "connect", "connect_master", "connect_all", "connect_replica", + "rebind_connections", "down_replica_priority", "up_replica_priority", + "call", "callrw", "callro"}) do + local ok, msg = pcall(replicaset[func_name], "arg_of_wrong_type") + table.insert(error_messages, msg:match("Use .*")) +end; +--- +... +test_run:cmd("setopt delimiter ''"); +--- +- true +... +error_messages +--- +- - Use replicaset:connect(...) instead of replicaset.connect(...) + - Use replicaset:connect_master(...) instead of replicaset.connect_master(...) + - Use replicaset:connect_all(...) instead of replicaset.connect_all(...) + - Use replicaset:connect_replica(...) instead of replicaset.connect_replica(...) + - Use replicaset:rebind_connections(...) instead of replicaset.rebind_connections(...) + - Use replicaset:down_replica_priority(...) instead of replicaset.down_replica_priority(...) + - Use replicaset:up_replica_priority(...) instead of replicaset.up_replica_priority(...) + - Use replicaset:call(...) instead of replicaset.call(...) + - Use replicaset:callrw(...) instead of replicaset.callrw(...) + - Use replicaset:callro(...) instead of replicaset.callro(...) +... +_, replica = next(replicaset.replicas) +--- +... +error_messages = {} +--- +... +test_run:cmd("setopt delimiter ';'") +--- +- true +... +for _, func_name in pairs({ + "is_connected", "safe_uri"}) do + local ok, msg = pcall(replica[func_name], "arg_of_wrong_type") + table.insert(error_messages, msg:match("Use .*")) +end; +--- +... +test_run:cmd("setopt delimiter ''"); +--- +- true +... +error_messages +--- +- - Use replica:is_connected(...) instead of replica.is_connected(...) + - Use replica:safe_uri(...) instead of replica.safe_uri(...) +... _ = test_run:cmd("switch default") --- ... diff --git a/test/router/router.test.lua b/test/router/router.test.lua index ebf0eb5..c0e66e4 100644 --- a/test/router/router.test.lua +++ b/test/router/router.test.lua @@ -361,6 +361,33 @@ vshard.router.sync() util.check_error(vshard.router.sync, "xxx") vshard.router.sync(100500) +-- Check that user passed self arg +_, replicaset = next(vshard.router.internal.replicasets) +error_messages = {} + +test_run:cmd("setopt delimiter ';'") +for _, func_name in pairs({ + "connect", "connect_master", "connect_all", "connect_replica", + "rebind_connections", "down_replica_priority", "up_replica_priority", + "call", "callrw", "callro"}) do + local ok, msg = pcall(replicaset[func_name], "arg_of_wrong_type") + table.insert(error_messages, msg:match("Use .*")) +end; +test_run:cmd("setopt delimiter ''"); +error_messages + +_, replica = next(replicaset.replicas) +error_messages = {} + +test_run:cmd("setopt delimiter ';'") +for _, func_name in pairs({ + "is_connected", "safe_uri"}) do + local ok, msg = pcall(replica[func_name], "arg_of_wrong_type") + table.insert(error_messages, msg:match("Use .*")) +end; +test_run:cmd("setopt delimiter ''"); +error_messages + _ = test_run:cmd("switch default") test_run:drop_cluster(REPLICASET_2) diff --git a/vshard/replicaset.lua b/vshard/replicaset.lua index d210973..0fc5fb1 100644 --- a/vshard/replicaset.lua +++ b/vshard/replicaset.lua @@ -48,6 +48,7 @@ local lerror = require('vshard.error') local fiber = require('fiber') local luri = require('uri') local ffi = require('ffi') +local csa = require('vshard.util').check_self_arg -- -- on_connect() trigger for net.box @@ -364,30 +365,39 @@ end -- local replicaset_mt = { __index = { - connect = replicaset_connect_master; - connect_master = replicaset_connect_master; - connect_all = replicaset_connect_all; - connect_replica = replicaset_connect_to_replica; - rebind_connections = replicaset_rebind_connections; - down_replica_priority = replicaset_down_replica_priority; - up_replica_priority = replicaset_up_replica_priority; - call = replicaset_master_call; - callrw = replicaset_master_call; - callro = replicaset_nearest_call; + connect = csa("replicaset", "connect", replicaset_connect_master); + connect_master = csa("replicaset", "connect_master", + replicaset_connect_master); + connect_all = csa("replicaset", "connect_all", replicaset_connect_all); + connect_replica = function(rs, r) + assert(type(r) == "table", + 'Use replicaset:connect_replica(...) instead of ' .. + 'replicaset.connect_replica(...)') + return replicaset_connect_to_replica(rs, r) + end; + rebind_connections = csa("replicaset", "rebind_connections", + replicaset_rebind_connections); + down_replica_priority = csa("replicaset", "down_replica_priority", + replicaset_down_replica_priority); + up_replica_priority = csa("replicaset", "up_replica_priority", + replicaset_up_replica_priority); + call = csa("replicaset", "call", replicaset_master_call); + callrw = csa("replicaset", "callrw", replicaset_master_call); + callro = csa("replicaset", "callro", replicaset_nearest_call); }; __tostring = replicaset_tostring; } local replica_mt = { __index = { - is_connected = function(replica) + is_connected = csa("replica", "is_connected", function(replica) return replica.conn and replica.conn.state == 'active' - end, - safe_uri = function(replica) + end), + safe_uri = csa("replica", "safe_uri", function(replica) local uri = luri.parse(replica.uri) uri.password = nil return luri.format(uri) - end, + end), }, __tostring = function(replica) if replica.name then diff --git a/vshard/util.lua b/vshard/util.lua index 184aad5..95d538d 100644 --- a/vshard/util.lua +++ b/vshard/util.lua @@ -48,7 +48,40 @@ local function reloadable_fiber_f(M, func_name, worker_name) end end +-- +-- This function is used to return an error in case one called a method +-- like object.func instead of object:func. +-- This wrapper takes a function and returns another function, which checks +-- anguments and calls wrapped function. +-- Returning wrapped values to the user and using raw functions inside of a +-- modele improves speed. +-- +-- This function can be used only in case the second argument is not of the +-- "table" type. +-- +-- @param obj_name Name of the called object. Used only for error message +-- construction. +-- +-- @param func_name Name of the called function. Used only for an error message +-- construction. +-- +-- @param func A function which is about to be wrapped +-- +-- @retval Wrapped function. +-- +local function check_self_arg(obj_name, func_name, func) + local function wrapped_func(self, ...) + if type(self) ~= "table" then + local fmt = 'Use %s:%s(...) instead of %s.%s(...)' + error(string.format(fmt, obj_name, func_name, obj_name, func_name)) + end + return func(self, ...) + end + return wrapped_func +end + return { tuple_extract_key = tuple_extract_key, reloadable_fiber_f = reloadable_fiber_f, + check_self_arg = check_self_arg, } -- 2.14.1