From 14d8b6a0df6cd3385d1618ecaa53e923fd665fb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 28 Sep 2017 13:55:45 +0300 Subject: [PATCH] Remove MODULECMD_ARG_OUTPUT argument type Since the module command interface was expanded to include a JSON output parameter, there is no longer a need for an output DCB. As the JSON can be printed by both maxadmin and the REST API, this allows the removal of explicit output formatting in module commands. --- include/maxscale/modulecmd.h | 13 ------- server/core/load_utils.cc | 6 --- server/core/modulecmd.cc | 24 ------------ server/core/resource.cc | 2 +- server/modules/filter/cache/cachefilter.cc | 9 +---- .../modules/filter/dbfwfilter/dbfwfilter.cc | 21 +++------- .../modules/filter/masking/maskingfilter.cc | 32 ++++++++-------- .../modules/filter/masking/maskingfilter.hh | 2 +- server/modules/routing/debugcli/debugcmd.c | 38 +++---------------- 9 files changed, 32 insertions(+), 115 deletions(-) diff --git a/include/maxscale/modulecmd.h b/include/maxscale/modulecmd.h index 427208484..0ed784014 100644 --- a/include/maxscale/modulecmd.h +++ b/include/maxscale/modulecmd.h @@ -62,9 +62,6 @@ typedef struct #define MODULECMD_ARG_DCB 8 /**< DCB */ #define MODULECMD_ARG_MONITOR 9 /**< Monitor */ #define MODULECMD_ARG_FILTER 10 /**< Filter */ -#define MODULECMD_ARG_OUTPUT 11 /**< DCB suitable for writing results to. - This should always be the first argument - if the function requires an output DCB. */ /** What type of an action does the command perform? */ enum modulecmd_type @@ -196,7 +193,6 @@ const MODULECMD* modulecmd_find_command(const char *domain, const char *identifi * | MODULECMD_ARG_STRING | String | * | MODULECMD_ARG_BOOLEAN | Boolean value | * | MODULECMD_ARG_DCB | Raw DCB pointer | - * | MODULECMD_ARG_OUTPUT | DCB for output | * * @param cmd Command for which the parameters are parsed * @param argc Number of arguments @@ -223,15 +219,6 @@ void modulecmd_arg_free(MODULECMD_ARG *arg); */ bool modulecmd_arg_is_present(const MODULECMD_ARG *arg, int idx); -/** - * @brief Check if module command requires an output DCB - * - * @param cmd Command to check - * - * @return True if module requires a DCB for printing output - */ -bool modulecmd_requires_output_dcb(const MODULECMD* cmd); - /** * @brief Call a registered command * diff --git a/server/core/load_utils.cc b/server/core/load_utils.cc index a27906c19..2884ff143 100644 --- a/server/core/load_utils.cc +++ b/server/core/load_utils.cc @@ -360,12 +360,6 @@ struct cb_param bool modulecmd_cb(const MODULECMD *cmd, void *data) { - if (modulecmd_requires_output_dcb(cmd)) - { - /** Module requires an output DCB, don't print it */ - return true; - } - cb_param* d = static_cast(data); json_t* obj = json_object(); diff --git a/server/core/modulecmd.cc b/server/core/modulecmd.cc index 6d2b4f615..d8c96f55f 100644 --- a/server/core/modulecmd.cc +++ b/server/core/modulecmd.cc @@ -358,12 +358,6 @@ static bool process_argument(const MODULECMD *cmd, modulecmd_arg_type_t *type, c } break; - case MODULECMD_ARG_OUTPUT: - arg->type.type = MODULECMD_ARG_OUTPUT; - arg->value.dcb = (DCB*)value; - rval = true; - break; - default: ss_dassert(false); MXS_ERROR("Undefined argument type: %0lx", type->type); @@ -713,10 +707,6 @@ const char* modulecmd_argtype_to_str(modulecmd_arg_type_t *type) rval = format_type(type, "FILTER"); break; - case MODULECMD_ARG_OUTPUT: - rval = format_type(type, "OUTPUT"); - break; - default: ss_dassert(false); MXS_ERROR("Unknown type"); @@ -731,17 +721,3 @@ bool modulecmd_arg_is_present(const MODULECMD_ARG *arg, int idx) return arg->argc > idx && MODULECMD_GET_TYPE(&arg->argv[idx].type) != MODULECMD_ARG_NONE; } - -bool modulecmd_requires_output_dcb(const MODULECMD* cmd) -{ - for (int i = 0; i < cmd->arg_count_max; i++) - { - if (cmd->arg_types[i].type == MODULECMD_ARG_OUTPUT) - { - /** We can't call this as it requries a DCB for output so don't show it */ - return true; - } - } - - return false; -} diff --git a/server/core/resource.cc b/server/core/resource.cc index 618d03c9c..25fc3d09d 100644 --- a/server/core/resource.cc +++ b/server/core/resource.cc @@ -638,7 +638,7 @@ HttpResponse cb_modulecmd(const HttpRequest& request) const MODULECMD* cmd = modulecmd_find_command(module.c_str(), identifier.c_str()); - if (cmd && !modulecmd_requires_output_dcb(cmd)) + if (cmd) { if ((!MODULECMD_MODIFIES_DATA(cmd) && verb == MHD_HTTP_METHOD_GET) || (MODULECMD_MODIFIES_DATA(cmd) && verb == MHD_HTTP_METHOD_POST)) diff --git a/server/modules/filter/cache/cachefilter.cc b/server/modules/filter/cache/cachefilter.cc index 16fd1b0cc..3fb358ee0 100644 --- a/server/modules/filter/cache/cachefilter.cc +++ b/server/modules/filter/cache/cachefilter.cc @@ -90,18 +90,14 @@ void cache_config_reset(CACHE_CONFIG& config) */ bool cache_command_show(const MODULECMD_ARG* pArgs, json_t** output) { - ss_dassert(pArgs->argc == 2); - ss_dassert(MODULECMD_GET_TYPE(&pArgs->argv[0].type) == MODULECMD_ARG_OUTPUT); + ss_dassert(pArgs->argc == 1); ss_dassert(MODULECMD_GET_TYPE(&pArgs->argv[1].type) == MODULECMD_ARG_FILTER); - DCB* pDcb = pArgs->argv[0].value.dcb; - ss_dassert(pDcb); - const MXS_FILTER_DEF* pFilterDef = pArgs->argv[1].value.filter; ss_dassert(pFilterDef); CacheFilter* pFilter = reinterpret_cast(filter_def_get_instance(pFilterDef)); - MXS_EXCEPTION_GUARD(pFilter->cache().show(pDcb)); + MXS_EXCEPTION_GUARD(*output = pFilter->cache().show_json()); return true; } @@ -146,7 +142,6 @@ extern "C" MXS_MODULE* MXS_CREATE_MODULE() { static modulecmd_arg_type_t show_argv[] = { - { MODULECMD_ARG_OUTPUT, "The output dcb" }, { MODULECMD_ARG_FILTER | MODULECMD_ARG_NAME_MATCHES_DOMAIN, "Cache name" } }; diff --git a/server/modules/filter/dbfwfilter/dbfwfilter.cc b/server/modules/filter/dbfwfilter/dbfwfilter.cc index 7e0b4f41b..d664d0838 100644 --- a/server/modules/filter/dbfwfilter/dbfwfilter.cc +++ b/server/modules/filter/dbfwfilter/dbfwfilter.cc @@ -78,6 +78,7 @@ #include #include #include +#include #include "rules.hh" #include "user.hh" @@ -499,27 +500,17 @@ MXS_MODULE* MXS_CREATE_MODULE() }; modulecmd_register_command(MXS_MODULE_NAME, "rules/reload", MODULECMD_TYPE_ACTIVE, - dbfw_reload_rules, 2, args_rules_reload, - "Reload dbfwfilter rules"); - - modulecmd_arg_type_t args_rules_show[] = - { - {MODULECMD_ARG_OUTPUT, "DCB where result is written"}, - {MODULECMD_ARG_FILTER | MODULECMD_ARG_NAME_MATCHES_DOMAIN, "Filter to inspect"} - }; - - modulecmd_register_command(MXS_MODULE_NAME, "rules", MODULECMD_TYPE_PASSIVE, - dbfw_show_rules, 2, args_rules_show, - "(deprecated) Show dbfwfilter rule statistics"); + dbfw_reload_rules, MXS_ARRAY_NELEMS(args_rules_reload), + args_rules_reload, "Reload dbfwfilter rules"); modulecmd_arg_type_t args_rules_show_json[] = { {MODULECMD_ARG_FILTER | MODULECMD_ARG_NAME_MATCHES_DOMAIN, "Filter to inspect"} }; - modulecmd_register_command(MXS_MODULE_NAME, "rules/json", MODULECMD_TYPE_PASSIVE, - dbfw_show_rules_json, 1, args_rules_show_json, - "Show dbfwfilter rule statistics as JSON"); + modulecmd_register_command(MXS_MODULE_NAME, "rules", MODULECMD_TYPE_PASSIVE, + dbfw_show_rules_json, MXS_ARRAY_NELEMS(args_rules_show_json), + args_rules_show_json, "Show dbfwfilter rule statistics"); static MXS_MODULE info = { diff --git a/server/modules/filter/masking/maskingfilter.cc b/server/modules/filter/masking/maskingfilter.cc index 557c43452..da7eec3d8 100644 --- a/server/modules/filter/masking/maskingfilter.cc +++ b/server/modules/filter/masking/maskingfilter.cc @@ -14,6 +14,7 @@ #define MXS_MODULE_NAME "masking" #include "maskingfilter.hh" +#include #include #include #include @@ -37,20 +38,23 @@ char VERSION_STRING[] = "V1.0.0"; */ bool masking_command_reload(const MODULECMD_ARG* pArgs, json_t** output) { - ss_dassert(pArgs->argc == 2); - ss_dassert(MODULECMD_GET_TYPE(&pArgs->argv[0].type) == MODULECMD_ARG_OUTPUT); + ss_dassert(pArgs->argc == 1); ss_dassert(MODULECMD_GET_TYPE(&pArgs->argv[1].type) == MODULECMD_ARG_FILTER); - DCB* pDcb = pArgs->argv[0].value.dcb; - ss_dassert(pDcb); - const MXS_FILTER_DEF* pFilterDef = pArgs->argv[1].value.filter; ss_dassert(pFilterDef); MaskingFilter* pFilter = reinterpret_cast(filter_def_get_instance(pFilterDef)); - MXS_EXCEPTION_GUARD(pFilter->reload(pDcb)); + bool rv = false; + MXS_EXCEPTION_GUARD(rv = pFilter->reload()); - return true; + if (!rv) + { + modulecmd_set_error("Could not reload the rules. Check the log file " + "for more detailed information."); + } + + return rv; } } @@ -63,7 +67,6 @@ extern "C" MXS_MODULE* MXS_CREATE_MODULE() { static modulecmd_arg_type_t reload_argv[] = { - { MODULECMD_ARG_OUTPUT, "The output dcb" }, { MODULECMD_ARG_FILTER | MODULECMD_ARG_NAME_MATCHES_DOMAIN, "Masking name" } }; @@ -168,19 +171,16 @@ std::tr1::shared_ptr MaskingFilter::rules() const return m_sRules; } -void MaskingFilter::reload(DCB* pOut) +bool MaskingFilter::reload() { + bool rval = false; auto_ptr sRules = MaskingRules::load(m_config.rules().c_str()); if (sRules.get()) { m_sRules = sRules; + rval = true; + } - dcb_printf(pOut, "Rules reloaded.\n"); - } - else - { - dcb_printf(pOut, "Could not reload the rules. Check the log file for more " - "detailed information.\n"); - } + return rval; } diff --git a/server/modules/filter/masking/maskingfilter.hh b/server/modules/filter/masking/maskingfilter.hh index 816258556..0b8ef1dff 100644 --- a/server/modules/filter/masking/maskingfilter.hh +++ b/server/modules/filter/masking/maskingfilter.hh @@ -38,7 +38,7 @@ public: uint64_t getCapabilities(); - void reload(DCB* pOut); + bool reload(); const Config& config() const { diff --git a/server/modules/routing/debugcli/debugcmd.c b/server/modules/routing/debugcli/debugcmd.c index 36ce9d4da..afd2eb821 100644 --- a/server/modules/routing/debugcli/debugcmd.c +++ b/server/modules/routing/debugcli/debugcmd.c @@ -303,13 +303,9 @@ bool listfuncs_cb(const MODULECMD *cmd, void *data) for (int i = 0; i < cmd->arg_count_max; i++) { modulecmd_arg_type_t *type = &cmd->arg_types[i]; - - if (MODULECMD_GET_TYPE(type) != MODULECMD_ARG_OUTPUT) - { - dcb_printf(dcb, "%s%s", - modulecmd_argtype_to_str(&cmd->arg_types[i]), - i < cmd->arg_count_max - 1 ? " " : ""); - } + dcb_printf(dcb, "%s%s", + modulecmd_argtype_to_str(&cmd->arg_types[i]), + i < cmd->arg_count_max - 1 ? " " : ""); } dcb_printf(dcb, "\n\n"); @@ -317,13 +313,9 @@ bool listfuncs_cb(const MODULECMD *cmd, void *data) for (int i = 0; i < cmd->arg_count_max; i++) { modulecmd_arg_type_t *type = &cmd->arg_types[i]; - - if (MODULECMD_GET_TYPE(type) != MODULECMD_ARG_OUTPUT) - { - dcb_printf(dcb, " %s - %s\n", - modulecmd_argtype_to_str(&cmd->arg_types[i]), - cmd->arg_types[i].description); - } + dcb_printf(dcb, " %s - %s\n", + modulecmd_argtype_to_str(&cmd->arg_types[i]), + cmd->arg_types[i].description); } dcb_printf(dcb, "\n"); @@ -1662,12 +1654,6 @@ struct subcommand alteroptions[] = } }; -static inline bool requires_output_dcb(const MODULECMD *cmd) -{ - modulecmd_arg_type_t *type = &cmd->arg_types[0]; - return cmd->arg_count_max > 0 && MODULECMD_GET_TYPE(type) == MODULECMD_ARG_OUTPUT; -} - static void callModuleCommand(DCB *dcb, char *domain, char *id, char *v3, char *v4, char *v5, char *v6, char *v7, char *v8, char *v9, char *v10, char *v11, char *v12) @@ -1685,18 +1671,6 @@ static void callModuleCommand(DCB *dcb, char *domain, char *id, char *v3, if (cmd) { - if (requires_output_dcb(cmd)) - { - /** The command requires a DCB for output, add the client DCB - * as the first argument */ - for (int i = valuelen - 1; i > 0; i--) - { - values[i] = values[i - 1]; - } - values[0] = dcb; - numargs += numargs + 1 < valuelen - 1 ? 1 : 0; - } - MODULECMD_ARG *arg = modulecmd_arg_parse(cmd, numargs, values); if (arg)