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 4AE2429178 for ; Fri, 1 Jun 2018 14:11:32 -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 Y3MfIdYW7Vau for ; Fri, 1 Jun 2018 14:11:32 -0400 (EDT) Received: from smtp52.i.mail.ru (smtp52.i.mail.ru [94.100.177.112]) (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 0894D29177 for ; Fri, 1 Jun 2018 14:11:31 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH][vshard] Check self passed to object methods From: Vladislav Shpilevoy References: <20180601124912.9145-1-avkhatskevich@tarantool.org> Message-ID: <2404a222-e13e-6bd8-16ee-4f654ff452ea@tarantool.org> Date: Fri, 1 Jun 2018 21:11:29 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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, AKhatskevich Thanks for the fixes! Please, after fixes put the complete patch at the end of a letter. See 6 comments below. > diff --git a/test/router/router.result b/test/router/router.result > index 5fcabc6..b00cd12 100644 > --- a/test/router/router.result > +++ b/test/router/router.result > @@ -977,6 +977,75 @@ util.check_error(vshard.router.sync, "xxx") > vshard.router.sync(100500) > --- > ... > +-- Check that user passed self arg 1. Please, beforehand describe the issue: -- -- gh-####: issue description. -- > +get_object_method_names = function(object) > + local res = {} > + for name, _ in pairs(getmetatable(object).__index) do > + table.insert(res, name) > + end > + table.sort(res) > + return res > +end; 2. Why do you need table.sort here? > +for _, func_name in pairs(get_object_method_names(replicaset)) do > + local ok, msg = pcall(replicaset[func_name], "arg_of_wrong_type") > + table.insert(error_messages, msg:match("Use .*")) > +end; 3. Why does not work just iteration over getmetatable(object).__index ? > diff --git a/vshard/replicaset.lua b/vshard/replicaset.lua > index d210973..ede53c7 100644 > --- a/vshard/replicaset.lua > +++ b/vshard/replicaset.lua > @@ -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(rs) == "table" and type(r) == "table", > + 'Use replicaset:connect_replica(...) instead of ' .. > + 'replicaset.connect_replica(...)') > + return replicaset_connect_to_replica(rs, r) > + end; 4. Looks like you did not that I asked for. Please, do not use assert, and do not inline part of check_self_arg. Make two functions: check_self_arg, generate_checker or something. > +-- > +-- Wrap a given function to check that self argument is passed. > +-- New function returns an error in case one called a method > +-- like object.func instead of object:func. > +-- Returning wrapped function to a user and using raw functions inside of a > +-- module improves speed. > +-- This function can be used only in case the second argument is not of the > +-- "table" type. > +-- 5. Still extra line. > +-- @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. > +-- 6. Please, fit the comment in 66 symbols.