From 24c0cd47807cca680a05815909ef64aade34ef5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 11 Aug 2022 13:44:17 +0200 Subject: [PATCH 1/4] Simplify argument processing in mongooseimctl The arguments were broken into pieces (on spaces), quoted, and finally reassembled by Erlang code. All these steps are now replaced with putting the arguments into an array, whose elements are then passed as arguments. One of the main causes for this change was a bug in arguments concatenation, which was revealed by new command tests. --- rel/files/mongooseimctl | 48 ++++++++++++++-------------------- src/ejabberd_ctl.erl | 57 +---------------------------------------- 2 files changed, 20 insertions(+), 85 deletions(-) diff --git a/rel/files/mongooseimctl b/rel/files/mongooseimctl index 01e862e21d0..5e669b42368 100755 --- a/rel/files/mongooseimctl +++ b/rel/files/mongooseimctl @@ -46,8 +46,8 @@ if [ -z "$NODENAME_ARG" ]; then echo "vm.args needs to have either -name or -sname parameter." exit 1 fi -FORCE_FLAG1='"--force"' -FORCE_FLAG2='"-f"' +FORCE_FLAG1='--force' +FORCE_FLAG2='-f' NAME_TYPE="${NODENAME_ARG% *}" NODENAME="${NODENAME_ARG#* }" @@ -79,19 +79,21 @@ remove_from_cluster() manage_cluster() { - case $QUOTED_ARGS in - *$FORCE_FLAG1*|*$FORCE_FLAG2*) - QUOTED_ARGS=$(echo $QUOTED_ARGS|sed "s/$FORCE_FLAG1//") - QUOTED_ARGS=$(echo $QUOTED_ARGS|sed "s/$FORCE_FLAG2//") - ctl $QUOTED_ARGS;; - *) + case "$ARGS" in + *$FORCE_FLAG1*) + ARGS_ARRAY=( ${ARGS_ARRAY[@]/$FORCE_FLAG1} ) + ctl;; + *$FORCE_FLAG2*) + ARGS_ARRAY=( ${ARGS_ARRAY[@]/$FORCE_FLAG2} ) + ctl;; + *) GUARD="unknown" until [ "$GUARD" = "yes" ] || [ "$GUARD" = "no" ] ; do echo $1 read GUARD if [ "$GUARD" = "yes" ]; then echo $2 - ctl $QUOTED_ARGS + ctl elif [ "$GUARD" = "no" ]; then echo "Operation discarded by user" exit 1 @@ -188,8 +190,6 @@ help () # common control function ctl () { - COMMAND=$@ - # Control number of connections identifiers # using flock if available. Expects a linux-style # flock that can lock a file descriptor. @@ -202,13 +202,13 @@ ctl () if [ ! -x "$JOT" ] ; then # no flock or jot, simply invoke ctlexec() CTL_CONN="ctl-${NODENAME}" - ctlexec $CTL_CONN $COMMAND + ctlexec $CTL_CONN result=$? else # no flock, but at least there is jot RAND=`jot -r 1 0 $MAXCONNID` CTL_CONN="ctl-${RAND}-${NODENAME}" - ctlexec $CTL_CONN $COMMAND + ctlexec $CTL_CONN result=$? fi else @@ -223,7 +223,7 @@ ctl () ( exec 8>"$CTL_LOCKFILE" if flock --nb 8; then - ctlexec $CTL_CONN $COMMAND + ctlexec $CTL_CONN ssresult=$? # segregate from possible flock exit(1) ssresult=$(expr $ssresult \* 10) @@ -266,7 +266,6 @@ ctlexec () { export BINDIR=$ERTS_PATH CONN_NAME=$1; shift - COMMAND=$@ $ERTS_PATH/erlexec \ -boot "$MIM_DIR/releases/$APP_VSN/start_clean" \ $NAME_TYPE ${CONN_NAME} \ @@ -275,7 +274,7 @@ ctlexec () $COOKIE_ARG \ -args_file "$RUNNER_ETC_DIR"/vm.dist.args \ -pa "$MIM_DIR"/lib/mongooseim-*/ebin/ \ - -s ejabberd_ctl -extra $NODENAME $COMMAND + -s ejabberd_ctl -extra $NODENAME "${ARGS_ARRAY[@]}" } # display ctl usage @@ -376,18 +375,9 @@ function is_started_status_file [ "$status" = "started" ] } -# parse command line parameters -ARGS=""; QUOTED_ARGS="" -for PARAM in "$@" -do - case $PARAM in - --) - break ;; - *) - ARGS="$ARGS$PARAM"; ARGS="$ARGS "; - QUOTED_ARGS=$QUOTED_ARGS'"'$PARAM'"'; QUOTED_ARGS=$QUOTED_ARGS" " ;; - esac -done +# parse command line arguments +ARGS="$@" +ARGS_ARRAY=( "$@" ) case $1 in 'bootstrap') bootstrap;; @@ -404,5 +394,5 @@ case $1 in 'stopped') is_started_status_file && wait_for_status_file stopped 30 1 || true; wait_for_status 3 15 2; stop_epmd;; # wait 15x2s before timeout 'print_install_dir') print_install_dir;; 'escript') run_escript $@;; - *) ctl $QUOTED_ARGS;; + *) ctl;; esac diff --git a/src/ejabberd_ctl.erl b/src/ejabberd_ctl.erl index 70b0dd77615..a45d1a13c29 100644 --- a/src/ejabberd_ctl.erl +++ b/src/ejabberd_ctl.erl @@ -75,8 +75,7 @@ -spec start() -> none(). start() -> case init:get_plain_arguments() of - [SNode | Args0] -> - Args = args_join_xml(args_join_strings(Args0)), + [SNode | Args] -> SNode1 = case string:tokens(SNode, "@") of [_Node, _Server] -> SNode; @@ -375,60 +374,6 @@ call_command([CmdString | Args], Auth, AccessCommands) -> %% Format arguments %%----------------------------- -%% @private --spec args_join_xml([string()]) -> [string()]. -args_join_xml([]) -> - []; -args_join_xml([ [ $< | _ ] = Arg | RArgs ]) -> - case bal(Arg, $<, $>) of - true -> - [Arg | args_join_xml(RArgs)]; - false -> - [NextArg | RArgs1] = RArgs, - args_join_xml([Arg ++ " " ++ NextArg | RArgs1]) - end; -args_join_xml([ Arg | RArgs ]) -> - [ Arg | args_join_xml(RArgs) ]. - - -%% @private --spec args_join_strings([string()]) -> [string()]. -args_join_strings([]) -> - []; -args_join_strings([ "\"", NextArg | RArgs ]) -> - args_join_strings([ "\"" ++ NextArg | RArgs ]); -args_join_strings([ [ $" | _ ] = Arg | RArgs ]) -> - case lists:nthtail(length(Arg)-2, Arg) of - [C1, $"] when C1 /= ?ASCII_SPACE_CHARACTER -> - [ string:substr(Arg, 2, length(Arg)-2) | args_join_strings(RArgs) ]; - _ -> - [NextArg | RArgs1] = RArgs, - args_join_strings([Arg ++ " " ++ NextArg | RArgs1]) - end; -args_join_strings([ Arg | RArgs ]) -> - [ Arg | args_join_strings(RArgs) ]. - - -%% @private --spec bal(string(), char(), char()) -> boolean(). -bal(String, Left, Right) -> - bal(String, Left, Right, 0). - - -%% @private --spec bal(string(), L :: char(), R :: char(), Bal :: integer()) -> boolean(). -bal([], _Left, _Right, Bal) -> - Bal == 0; -bal([?ASCII_SPACE_CHARACTER, _NextChar | T], Left, Right, Bal) -> - bal(T, Left, Right, Bal); -bal([Left | T], Left, Right, Bal) -> - bal(T, Left, Right, Bal-1); -bal([Right | T], Left, Right, Bal) -> - bal(T, Left, Right, Bal+1); -bal([_C | T], Left, Right, Bal) -> - bal(T, Left, Right, Bal). - - %% @private -spec format_args(Args :: [any()], ArgsFormat :: [format()]) -> [any()]. From e9999b192c9397e32ea1e4a143abd0e3fa84ea2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 11 Aug 2022 15:55:22 +0200 Subject: [PATCH 2/4] Process arguments in the '--name value' format Also: - Print usage for the new format, marking the old commands as deprecated. - Gather category and command descriptions using introspection. - Rework error messages to process the context directly, making the code more concise. - Reuse the usage printing logic for category- and command-related help. It doesn't make sense to rewrite it as support for both command formats (old and new) is temporary. This can be rewritten when the odl format is dropped. --- src/ejabberd_ctl.erl | 159 ++++++++++++------- src/graphql/mongoose_graphql_commands.erl | 185 +++++++++++++++------- 2 files changed, 227 insertions(+), 117 deletions(-) diff --git a/src/ejabberd_ctl.erl b/src/ejabberd_ctl.erl index a45d1a13c29..cb003c528bb 100644 --- a/src/ejabberd_ctl.erl +++ b/src/ejabberd_ctl.erl @@ -57,7 +57,6 @@ -include("ejabberd_ctl.hrl"). -include("ejabberd_commands.hrl"). --include("mongoose.hrl"). -type format() :: integer | string | binary | {list, format()}. -type format_type() :: binary() | string() | char(). @@ -191,13 +190,13 @@ process(["help" | Mode]) -> {MaxC, ShCode} = get_shell_info(), case Mode of [] -> - print_usage(dual, MaxC, ShCode), + print_usage_old(dual, MaxC, ShCode), ?STATUS_USAGE; ["--dual"] -> - print_usage(dual, MaxC, ShCode), + print_usage_old(dual, MaxC, ShCode), ?STATUS_USAGE; ["--long"] -> - print_usage(long, MaxC, ShCode), + print_usage_old(long, MaxC, ShCode), ?STATUS_USAGE; ["--tags"] -> print_usage_tags(MaxC, ShCode), @@ -215,32 +214,40 @@ process(["help" | Mode]) -> end; process(Args) -> case mongoose_graphql_commands:process(Args) of - {error, #{reason := Reason}} when Reason =:= no_args; - Reason =:= unknown_category -> + #{status := executed, result := Result} -> + handle_graphql_result(Result); + #{status := error, reason := Reason} when Reason =:= no_args; + Reason =:= unknown_category -> run_command(Args); % Fallback to the old commands - {error, #{reason := unknown_command, category := Category, cat_spec := CatSpec, - command := Command}} -> - ?PRINT("Unknown command '~s'. Available commands in category '~s':~n", - [Command, Category]), - [?PRINT(" ~s~n", [Cmd]) || Cmd <- lists:sort(maps:keys(CatSpec))], + #{status := error} = Ctx -> + ?PRINT(error_message(Ctx) ++ "\n\n", []), + print_usage(Ctx), ?STATUS_ERROR; - {error, #{reason := no_command, category := Category, cat_spec := CatSpec}} -> - ?PRINT("No command was specified. Available commands in category '~s':~n", [Category]), - [?PRINT(" ~s~n", [Cmd]) || Cmd <- lists:sort(maps:keys(CatSpec))], - ?STATUS_ERROR; - {error, #{reason := invalid_vars}} -> - ?PRINT("Could not parse command variables from JSON~n", []), - ?STATUS_ERROR; - {error, #{reason := no_vars}} -> - ?PRINT("This command requires variables in JSON~n", []), - ?STATUS_ERROR; - {error, #{reason := too_many_args}} -> - ?PRINT("Too many arguments~n", []), - ?STATUS_ERROR; - Result -> - handle_graphql_result(Result) + #{status := usage} = Ctx -> + print_usage(Ctx), + ?STATUS_SUCCESS % not STATUS_USAGE, as that would tell the script to print general help end. +-spec error_message(mongoose_graphql_commands:context()) -> iolist(). +error_message(#{reason := unknown_command, command := Command}) -> + io_lib:format("Unknown command '~s'", [Command]); +error_message(#{reason := invalid_args}) -> + "Could not parse the command arguments"; +error_message(#{reason := {unknown_arg, ArgName}, command := Command}) -> + io_lib:format("Unknown argument '~s' for command '~s'", [ArgName, Command]); +error_message(#{reason := {invalid_arg_value, ArgName, ArgValue}, command := Command}) -> + io_lib:format("Invalid value '~s' of argument '~s' for command '~s'", + [ArgValue, ArgName, Command]); +error_message(#{reason := {missing_args, MissingArgs}, command := Command}) -> + io_lib:format("Missing mandatory arguments for command '~s': ~s", + [Command, ["'", lists:join("', '", MissingArgs), "'"]]). + +-spec print_usage(mongoose_graphql_commands:context()) -> any(). +print_usage(#{category := Category, command := Command, args_spec := ArgsSpec}) -> + print_usage_command(Category, Command, ArgsSpec); +print_usage(#{category := Category, commands := Commands}) -> + print_usage_category(Category, Commands). + handle_graphql_result({ok, Result}) -> JSONResult = mongoose_graphql_response:term_to_pretty_json(Result), ?PRINT("~s\n", [JSONResult]), @@ -494,14 +501,6 @@ tuple_command_help({Name, Args, Desc}) -> CallString = atom_to_list(Name), {CallString, Arguments, Desc}. - --spec get_list_ctls() -> [cmd()]. -get_list_ctls() -> - case catch ets:tab2list(ejabberd_ctl_cmds) of - {'EXIT', _} -> []; - Cs -> [{NameArgs, [], Desc} || {NameArgs, Desc} <- Cs] - end. - format_status([{node, Node}, {internal_status, IS}, {provided_status, PS}, {mongoose_status, MS}, {os_pid, OSPid}, {uptime, UptimeHMS}, {dist_proto, DistProto}, {logs, LogFiles}]) -> @@ -538,21 +537,22 @@ print_usage() -> print_usage(dual, MaxC, ShCode). --spec print_usage('dual' | 'long' - , MaxC :: integer() - , ShCode :: boolean()) -> 'ok'. +-spec print_usage(dual | long, MaxC :: integer(), ShCode :: boolean()) -> ok. print_usage(HelpMode, MaxC, ShCode) -> - AllCommands = - [ - {"status", [], "Get MongooseIM status"}, - {"stop", [], "Stop MongooseIM"}, - {"restart", [], "Restart MongooseIM"}, - {"help", ["[--tags [tag] | com?*]"], "Show help (try: mongooseimctl help help)"}, - {"mnesia", ["[info]"], "show information of Mnesia system"}, - {"graphql", ["query"], "Execute graphql query or mutation"}] ++ - get_list_commands() ++ - get_list_ctls(), - + ?PRINT(["Usage: ", ?B("mongooseimctl"), " [", ?U("category"), "] ", ?U("command"), + " [", ?U("arguments"), "]\n\n" + "Most MongooseIM commands are grouped into the following categories:\n"], []), + print_categories(HelpMode, MaxC, ShCode), + ?PRINT(["\nTo list the commands in a particular category:\n mongooseimctl ", ?U("category"), + "\n"], []), + ?PRINT(["\nThe following basic system management commands do not have a category:\n"], []), + print_usage_commands(HelpMode, MaxC, ShCode, basic_commands()). + +-spec print_usage_old(dual | long, MaxC :: integer(), ShCode :: boolean()) -> ok. +print_usage_old(HelpMode, MaxC, ShCode) -> + ?PRINT(["The following commands are deprecated and ", ?B("will be removed"), " soon.\n" + "To learn about the new commands, run 'mongooseimctl' without any arguments.\n\n"], []), + AllCommands = basic_commands() ++ get_list_commands(), ?PRINT( ["Usage: ", ?B("mongooseimctl"), " [--node ", ?U("nodename"), "] [--auth ", ?U("user"), " ", ?U("host"), " ", ?U("password"), "] ", @@ -567,6 +567,52 @@ print_usage(HelpMode, MaxC, ShCode) -> " mongooseimctl --node mongooseim@host restart\n"], []). +-spec basic_commands() -> [cmd()]. +basic_commands() -> + [{"status", [], "Get MongooseIM status"}, + {"stop", [], "Stop MongooseIM"}, + {"restart", [], "Restart MongooseIM"}, + {"help", ["[--tags [tag] | com?*]"], "Show help for the deprecated commands"}, + {"mnesia", ["[info]"], "show information of Mnesia system"}, + {"graphql", ["query"], "Execute graphql query or mutation"}]. + +-spec print_categories(dual | long, MaxC :: integer(), ShCode :: boolean()) -> ok. +print_categories(HelpMode, MaxC, ShCode) -> + SortedSpecs = lists:sort(maps:to_list(mongoose_graphql_commands:get_specs())), + Categories = [{binary_to_list(Category), [], binary_to_list(Desc)} + || {Category, #{desc := Desc}} <- SortedSpecs], + print_usage_commands(HelpMode, MaxC, ShCode, Categories). + +-spec print_usage_category(mongoose_graphql_commands:category(), + mongoose_graphql_commands:command_map()) -> ok. +print_usage_category(Category, Commands) -> + {MaxC, ShCode} = get_shell_info(), + ?PRINT(["Usage: ", ?B("mongooseimctl"), " ", Category, " ", ?U("command"), " ", ?U("arguments"), "\n" + "\n" + "The following commands are available in the category '", Category, "':\n"], []), + CmdSpec = [{binary_to_list(Command), [], binary_to_list(Desc)} + || {Command, #{desc := Desc}} <- maps:to_list(Commands)], + print_usage_commands(dual, MaxC, ShCode, CmdSpec), + ?PRINT(["\nTo list the arguments for a particular command:\n" + " mongooseimctl ", Category, " ", ?U("command"), " --help", "\n"], []). + +-spec print_usage_command(mongoose_graphql_commands:category(), + mongoose_graphql_commands:command(), + [mongoose_graphql_commands:arg_spec()]) -> ok. +print_usage_command(Category, Command, ArgsSpec) -> + {MaxC, ShCode} = get_shell_info(), + ?PRINT(["Usage: ", ?B("mongooseimctl"), " ", Category, " ", Command, " ", ?U("arguments"), "\n" + "\n", + "Each argument has the format: --", ?U("name"), " ", ?U("value"), "\n", + "Available arguments are listed below with the corresponding GraphQL types:\n"], []), + %% Reuse the function initially designed for printing commands for now + %% This will be replaced with new logic when old commands are dropped + Args = [{binary_to_list(Name), [], mongoose_graphql_commands:wrap_type(Wrap, Type)} + || #{name := Name, type := Type, wrap := Wrap} <- ArgsSpec], + print_usage_commands(dual, MaxC, ShCode, Args), + ?PRINT(["\nScalar values do not need quoting unless they contain special characters or spaces.\n" + "Complex input types are passed as JSON maps or lists, depending on the type.\n" + "When a type is followed by '!', the corresponding argument is required.\n"], []). -spec print_usage_commands(HelpMode :: 'dual' | 'long', MaxC :: integer(), @@ -598,10 +644,8 @@ print_usage_commands(HelpMode, MaxC, ShCode, Commands) -> %% For each command in the list of commands %% Convert its definition to a line FmtCmdDescs = format_command_lines(CmdArgsLenDescsSorted, MaxCmdLen, MaxC, ShCode, HelpMode), - ?PRINT([FmtCmdDescs], []). - %% @doc Get some info about the shell: how many columns of width and guess if %% it supports text formatting codes. -spec get_shell_info() -> {integer(), boolean()}. @@ -611,16 +655,14 @@ get_shell_info() -> {error, enotsup} -> {78, false} end. - %% @doc Split this command description in several lines of proper length -spec prepare_description(DescInit :: non_neg_integer(), MaxC :: integer(), Desc :: string()) -> [[[any()]], ...]. prepare_description(DescInit, MaxC, Desc) -> - Words = string:tokens(Desc, " "), + Words = string:tokens(Desc, " \n"), prepare_long_line(DescInit, MaxC, Words). - -spec prepare_long_line(DescInit :: non_neg_integer(), MaxC :: integer(), Words :: [nonempty_string()] @@ -632,7 +674,6 @@ prepare_long_line(DescInit, MaxC, Words) -> MoreSegmentsMixed = mix_desc_segments(MarginString, MoreSegments), [FirstSegment | MoreSegmentsMixed]. - -spec mix_desc_segments(MarginStr :: [any()], Segments :: [[[any(), ...]]]) -> [[[any()], ...]]. mix_desc_segments(MarginString, Segments) -> @@ -641,7 +682,6 @@ mix_desc_segments(MarginString, Segments) -> split_desc_segments(MaxL, Words) -> join(MaxL, Words). - %% @doc Join words in a segment, but stop adding to a segment if adding this %% word would pass L -spec join(L :: number(), Words :: [nonempty_string()]) -> [[[any(), ...]], ...]. @@ -673,7 +713,6 @@ join(L, [Word | Words], LenLastSeg, LastSeg, ResSeg) -> join(L, Words, LWord, [" ", Word], [lists:reverse(LastSeg) | ResSeg]) end. - -spec format_command_lines(CALD :: [{[any()], [any()], number(), _}, ...], MaxCmdLen :: integer(), MaxC :: integer(), @@ -698,7 +737,6 @@ format_command_lines(CALD, _MaxCmdLen, MaxC, ShCode, long) -> DescFmt, "\n"] end, CALD). - %%----------------------------- %% Print Tags %%----------------------------- @@ -795,7 +833,7 @@ print_usage_commands2(Cmds, MaxC, ShCode) -> %% Then for each one print it lists:mapfoldl( fun(Cmd, Remaining) -> - print_usage_command(Cmd, MaxC, ShCode), + print_usage_command_old(Cmd, MaxC, ShCode), case Remaining > 1 of true -> ?PRINT([" ", lists:duplicate(MaxC, 126), " \n"], []); false -> ok @@ -827,8 +865,8 @@ filter_commands_regexp(All, Glob) -> All). --spec print_usage_command(Cmd :: string(), MaxC :: integer(), ShCode :: boolean()) -> ok. -print_usage_command(Cmd, MaxC, ShCode) -> +-spec print_usage_command_old(Cmd :: string(), MaxC :: integer(), ShCode :: boolean()) -> ok. +print_usage_command_old(Cmd, MaxC, ShCode) -> Name = list_to_atom(Cmd), case ejabberd_commands:get_command_definition(Name) of command_not_found -> @@ -919,4 +957,3 @@ get_dist_proto() -> {ok, [Proto]} -> Proto; _ -> "inet_tcp" end. - diff --git a/src/graphql/mongoose_graphql_commands.erl b/src/graphql/mongoose_graphql_commands.erl index 8d79672644f..3b1dac5394e 100644 --- a/src/graphql/mongoose_graphql_commands.erl +++ b/src/graphql/mongoose_graphql_commands.erl @@ -5,6 +5,9 @@ %% API -export([start/0, stop/0, process/1]). +%% Internal API +-export([wrap_type/2]). + %% Only for tests -export([build_specs/1, get_specs/0]). @@ -18,27 +21,34 @@ -type context() :: #{args := [string()], category => category(), - cat_spec => category_spec(), + commands => command_map(), command => command(), + args_spec => [arg_spec()], doc => doc(), vars => json_map(), - reason => atom()}. + reason => atom() | tuple(), + result => result(), + status => executed | error | usage}. -type result() :: {ok, #{atom() => graphql:json()}} | {error, any()}. -type specs() :: #{category() => category_spec()}. -type category() :: binary(). --type category_spec() :: #{command() => command_spec()}. +-type category_spec() :: #{desc := binary(), commands := command_map()}. +-type command_map() :: #{command() => command_spec()}. -type command() :: binary(). --type command_spec() :: #{op_type := op_type(), +-type command_spec() :: #{desc := binary(), + op_type := op_type(), args := [arg_spec()], fields := [field_spec()], doc := doc()}. --type arg_spec() :: #{name := binary(), type := binary(), wrap := [list | required]}. +-type arg_spec() :: #{name := binary(), type := binary(), kind := binary(), wrap := [list | required]}. -type field_spec() :: #{name | on := binary(), fields => [field_spec()]}. -type op_type() :: binary(). -type doc() :: binary(). -type ep() :: graphql:endpoint_context(). -type json_map() :: #{binary() => graphql:json()}. +-export_type([category/0, command/0, command_map/0, arg_spec/0, context/0]). + %% API -spec start() -> ok. @@ -51,13 +61,15 @@ stop() -> persistent_term:erase(?MODULE), ok. --spec process([string()]) -> result(). +%% The returned context has 'status' with the following values: +%% - 'executed' means that a GraphQL command was called, and 'result' contains the returned value +%% - 'error' means that the arguments were incorrect, and 'reason' contains more information +%% - 'usage' means that help needs to be displayed +-spec process([string()]) -> context(). process(Args) -> - lists:foldl(fun(_, {error, _} = Error) -> Error; + lists:foldl(fun(_, #{status := _} = Ctx) -> Ctx; (StepF, Ctx) -> StepF(Ctx) - end, - #{args => Args}, - [fun find_category/1, fun find_command/1, fun parse_vars/1, fun execute/1]). + end, #{args => Args}, steps()). %% Internal API @@ -75,85 +87,139 @@ get_specs() -> %% Internals --spec find_category(context()) -> context() | {error, context()}. +steps() -> + [fun find_category/1, fun find_command/1, fun parse_args/1, fun check_args/1, fun execute/1]. + +-spec find_category(context()) -> context(). find_category(CtxIn = #{args := [CategoryStr | Args]}) -> Category = list_to_binary(CategoryStr), Ctx = CtxIn#{category => Category, args => Args}, case get_specs() of - #{Category := CatSpec} -> - Ctx#{cat_spec => CatSpec}; + #{Category := #{commands := Commands}} -> + Ctx#{commands => Commands}; #{} -> - {error, Ctx#{reason => unknown_category}} + Ctx#{status => error, reason => unknown_category} end; find_category(Ctx = #{args := []}) -> - {error, Ctx#{reason => no_args}}. + Ctx#{status => error, reason => no_args}. --spec find_command(context()) -> context() | {error, context()}. +-spec find_command(context()) -> context(). find_command(CtxIn = #{args := [CommandStr | Args]}) -> Command = list_to_binary(CommandStr), - Ctx = #{cat_spec := CatSpec} = CtxIn#{command => Command, args => Args}, - case CatSpec of + Ctx = #{commands := Commands} = CtxIn#{command => Command, args => Args}, + case Commands of #{Command := CommandSpec} -> - #{doc := Doc} = CommandSpec, - Ctx#{doc => Doc}; + #{doc := Doc, args := ArgSpec} = CommandSpec, + Ctx#{doc => Doc, args_spec => ArgSpec}; #{} -> - {error, Ctx#{reason => unknown_command}} + Ctx#{status => error, reason => unknown_command} end; find_command(Ctx) -> - {error, Ctx#{reason => no_command}}. - --spec parse_vars(context()) -> context() | {error, context()}. -parse_vars(Ctx = #{args := [VarsStr]}) -> - try jiffy:decode(VarsStr, [return_maps]) of - Vars -> - Ctx#{vars => Vars} - catch _:_ -> - {error, Ctx#{reason => invalid_vars}} + Ctx#{status => usage}. + +-spec parse_args(context()) -> context(). +parse_args(Ctx = #{args := ["--help"]}) -> + Ctx#{status => usage}; +parse_args(Ctx) -> + parse_args_loop(Ctx#{vars => #{}}). + +parse_args_loop(Ctx = #{vars := Vars, + args_spec := ArgsSpec, + args := ["--" ++ ArgNameStr, ArgValueStr | Rest]}) -> + ArgName = list_to_binary(ArgNameStr), + case lists:filter(fun(#{name := Name}) -> Name =:= ArgName end, ArgsSpec) of + [] -> + Ctx#{status => error, reason => {unknown_arg, ArgName}}; + [ArgSpec] -> + ArgValue = list_to_binary(ArgValueStr), + try parse_arg(ArgValue, ArgSpec) of + ParsedValue -> + NewVars = Vars#{ArgName => ParsedValue}, + parse_args_loop(Ctx#{vars := NewVars, args := Rest}) + catch _:_ -> + Ctx#{status => error, reason => {invalid_arg_value, ArgName, ArgValue}} + end end; -parse_vars(Ctx = #{args := []}) -> - {error, Ctx#{reason => no_vars}}; -parse_vars(Ctx = #{args := [_|_]}) -> - {error, Ctx#{reason => too_many_args}}. +parse_args_loop(Ctx = #{args := []}) -> + Ctx; +parse_args_loop(Ctx) -> + Ctx#{status => error, reason => invalid_args}. + +-spec parse_arg(binary(), arg_spec()) -> jiffy:json_value(). +parse_arg(Value, ArgSpec = #{type := Type}) -> + case is_json_arg(ArgSpec) of + true -> + jiffy:decode(Value, [return_maps]); + false -> + convert_input_type(Type, Value) + end. --spec execute(context()) -> result(). -execute(#{doc := Doc, vars := Vars}) -> - execute(mongoose_graphql:get_endpoint(admin), Doc, Vars). +%% Used input types that are not parsed from binaries should be handled here +convert_input_type(<<"Int">>, Value) -> binary_to_integer(Value); +convert_input_type(<<"PosInt">>, Value) -> binary_to_integer(Value); +convert_input_type(_, Value) -> Value. + +%% Complex argument values should be provided in JSON +-spec is_json_arg(arg_spec()) -> boolean(). +is_json_arg(#{kind := <<"INPUT_OBJECT">>}) -> true; +is_json_arg(#{kind := Kind, wrap := Wrap}) when Kind =:= <<"SCALAR">>; + Kind =:= <<"ENUM">> -> + lists:member(list, Wrap). + +-spec check_args(context()) -> context(). +check_args(Ctx = #{args_spec := ArgsSpec, vars := Vars}) -> + MissingArgs = [Name || #{name := Name, wrap := [required|_]} <- ArgsSpec, + not maps:is_key(Name, Vars)], + case MissingArgs of + [] -> Ctx; + _ -> Ctx#{status => error, reason => {missing_args, MissingArgs}} + end. -%% Internals +-spec execute(context()) -> context(). +execute(#{doc := Doc, vars := Vars} = Ctx) -> + Ctx#{status => executed, result => execute(mongoose_graphql:get_endpoint(admin), Doc, Vars)}. -spec get_category_specs(ep()) -> [{category(), category_spec()}]. get_category_specs(Ep) -> - {ok, #{data := #{<<"__schema">> := Schema}}} = - mongoose_graphql:execute( - Ep, undefined, - <<"{ __schema { queryType {name fields {name type {name fields {name}}}} " - " mutationType {name fields {name type {name fields {name}}}} } }">>), + CSQuery = category_spec_query(), + Doc = iolist_to_binary(["{ __schema { queryType ", CSQuery, " mutationType ", CSQuery, " } }"]), + {ok, #{data := #{<<"__schema">> := Schema}}} = mongoose_graphql:execute(Ep, undefined, Doc), #{<<"queryType">> := #{<<"fields">> := Queries}, <<"mutationType">> := #{<<"fields">> := Mutations}} = Schema, get_category_specs(Ep, <<"query">>, Queries) ++ get_category_specs(Ep, <<"mutation">>, Mutations). -spec get_category_specs(ep(), op_type(), [json_map()]) -> [{category(), category_spec()}]. get_category_specs(Ep, OpType, Categories) -> - [get_category_spec(Ep, OpType, Category) || Category <- Categories]. + [get_category_spec(Ep, OpType, Category) || Category <- Categories, is_category(Category)]. + +is_category(#{<<"name">> := <<"checkAuth">>}) -> + false; +is_category(#{}) -> + true. -spec get_category_spec(ep(), op_type(), json_map()) -> {category(), category_spec()}. -get_category_spec(Ep, OpType, #{<<"name">> := Category, +get_category_spec(Ep, OpType, #{<<"name">> := Category, <<"description">> := Desc, <<"type">> := #{<<"name">> := CategoryType}}) -> Doc = iolist_to_binary( ["query ($type: String!) { __type(name: $type) " - "{name fields {name args {name type ", arg_type_query(), "} type ", + "{name fields {name description args {name type ", arg_type_query(), "} type ", field_type_query(), "}}}"]), Vars = #{<<"type">> => CategoryType}, {ok, #{data := #{<<"__type">> := #{<<"fields">> := Commands}}}} = execute(Ep, Doc, Vars), CommandSpecs = [get_command_spec(Ep, Category, OpType, Command) || Command <- Commands], - {Category, maps:from_list(CommandSpecs)}. + {Category, #{desc => Desc, commands => maps:from_list(CommandSpecs)}}. -spec get_command_spec(ep(), category(), op_type(), json_map()) -> {command(), command_spec()}. get_command_spec(Ep, Category, OpType, - #{<<"name">> := Name, <<"args">> := Args, <<"type">> := TypeMap}) -> + #{<<"name">> := Name, <<"args">> := Args, <<"type">> := TypeMap} = Map) -> Spec = #{op_type => OpType, args => get_args(Args), fields => get_fields(Ep, TypeMap, [])}, Doc = prepare_doc(Category, Name, Spec), - {Name, Spec#{doc => Doc}}. + {Name, add_description(Spec#{doc => Doc}, Map)}. + +add_description(Spec, #{<<"description">> := Desc}) -> + Spec#{desc => Desc}; +add_description(Spec, #{}) -> + Spec. -spec get_args([json_map()]) -> [arg_spec()]. get_args(Args) -> @@ -170,7 +236,7 @@ get_arg_type(#{<<"kind">> := <<"LIST">>, <<"ofType">> := TypeMap}, Wrap) -> get_arg_type(#{<<"name">> := Type, <<"kind">> := Kind}, Wrap) when Kind =:= <<"SCALAR">>; Kind =:= <<"ENUM">>; Kind =:= <<"INPUT_OBJECT">> -> - #{type => Type, wrap => lists:reverse(Wrap)}. + #{type => Type, kind => Kind, wrap => lists:reverse(Wrap)}. -spec get_fields(ep(), json_map(), [binary()]) -> [field_spec()]. get_fields(_Ep, #{<<"kind">> := Kind}, _Path) @@ -211,14 +277,18 @@ get_object_fields(Ep, ObjectType) -> Fields. -spec insert_category(category(), category_spec(), specs()) -> specs(). -insert_category(Category, NewCatSpec, Specs) -> +insert_category(Category, NewCatSpec = #{commands := NewCommands}, Specs) -> case Specs of - #{Category := OldCatSpec} -> - case maps:with(maps:keys(OldCatSpec), NewCatSpec) of + #{Category := #{desc := OldDesc, commands := OldCommands}} -> + case maps:with(maps:keys(OldCommands), NewCommands) of Common when Common =:= #{} -> - Specs#{Category := maps:merge(OldCatSpec, NewCatSpec)}; + Specs#{Category := #{desc => OldDesc, + commands => maps:merge(OldCommands, NewCommands)}}; Common -> - error(#{what => overlapping_commands, commands => maps:keys(Common)}) + error(#{what => overlapping_graphql_commands, + text => <<"GraphQL query and mutation names are not unique">>, + category => Category, + commands => maps:keys(Common)}) end; _ -> Specs#{Category => NewCatSpec} @@ -244,7 +314,7 @@ wrap_type([required | Wrap], Type) -> wrap_type([list | Wrap], Type) -> [$[, wrap_type(Wrap, Type), $]]; wrap_type([], Type) -> - Type. + [Type]. -spec use_variables([arg_spec()]) -> iolist(). use_variables([]) -> ""; @@ -285,3 +355,6 @@ arg_type_query() -> nested_type_query(BasicQuery) -> lists:foldl(fun(_, QueryAcc) -> ["{ ", BasicQuery, " ofType ", QueryAcc, " }"] end, ["{ ", BasicQuery, " }"], lists:seq(1, ?MAX_INTROSPECTION_DEPTH)). + +category_spec_query() -> + "{name fields {name description type {name fields {name}}}}". From 1d8cec0ca5539778ff975f9a3e9246969641b3bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 11 Aug 2022 16:00:26 +0200 Subject: [PATCH 3/4] Use the new argument-passing format in test helpers The new format is '--name value' instead of JSON. JSON is used only for complex values i.e. input objects and lists. --- big_tests/tests/graphql_helper.erl | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/big_tests/tests/graphql_helper.erl b/big_tests/tests/graphql_helper.erl index 462a70ec873..71002883381 100644 --- a/big_tests/tests/graphql_helper.erl +++ b/big_tests/tests/graphql_helper.erl @@ -22,7 +22,7 @@ execute(EpName, Body, Creds) -> rest_helper:make_request(Request). execute_user_command(Category, Command, User, Args, Config) -> - #{Category := #{Command := #{doc := Doc}}} = get_specs(), + #{Category := #{commands := #{Command := #{doc := Doc}}}} = get_specs(), execute_user(#{query => Doc, variables => Args}, User, Config). execute_command(Category, Command, Args, Config) -> @@ -31,13 +31,29 @@ execute_command(Category, Command, Args, Config) -> %% Admin commands can be executed as GraphQL over HTTP or with CLI (mongooseimctl) execute_command(Category, Command, Args, Config, http) -> - #{Category := #{Command := #{doc := Doc}}} = get_specs(), + #{Category := #{commands := #{Command := #{doc := Doc}}}} = get_specs(), execute_auth(#{query => Doc, variables => Args}, Config); execute_command(Category, Command, Args, Config, cli) -> - VarsJSON = iolist_to_binary(jiffy:encode(Args)), - {Result, Code} = mongooseimctl_helper:mongooseimctl(Category, [Command, VarsJSON], Config), + CLIArgs = encode_cli_args(Args), + {Result, Code} = mongooseimctl_helper:mongooseimctl(Category, [Command | CLIArgs], Config), {{exit_status, Code}, rest_helper:decode(Result, #{return_maps => true})}. +encode_cli_args(Args) -> + lists:flatmap(fun({Name, Value}) -> encode_cli_arg(Name, Value) end, maps:to_list(Args)). + +encode_cli_arg(_Name, null) -> + []; +encode_cli_arg(Name, Value) -> + [<<"--", (arg_name_to_binary(Name))/binary>>, arg_value_to_binary(Value)]. + +arg_name_to_binary(Name) when is_atom(Name) -> atom_to_binary(Name); +arg_name_to_binary(Name) when is_binary(Name) -> Name. + +arg_value_to_binary(Value) when is_integer(Value) -> integer_to_binary(Value); +arg_value_to_binary(Value) when is_binary(Value) -> Value; +arg_value_to_binary(Value) when is_list(Value); + is_map(Value) -> iolist_to_binary(jiffy:encode(Value)). + execute_auth(Body, Config) -> Ep = ?config(schema_endpoint, Config), #{username := Username, password := Password} = get_listener_opts(Ep), From 42c2401a6a723e7a6debc860acbca8954eddbc89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 11 Aug 2022 16:01:50 +0200 Subject: [PATCH 4/4] Update tests for mongooseimctl, using the new argument format Also: messages have changed, so they are updated. --- big_tests/tests/mongooseimctl_SUITE.erl | 103 ++++++++++++++++++------ 1 file changed, 77 insertions(+), 26 deletions(-) diff --git a/big_tests/tests/mongooseimctl_SUITE.erl b/big_tests/tests/mongooseimctl_SUITE.erl index 4816e4ac4ac..970cadf0814 100644 --- a/big_tests/tests/mongooseimctl_SUITE.erl +++ b/big_tests/tests/mongooseimctl_SUITE.erl @@ -102,7 +102,8 @@ all() -> {group, stats}, {group, basic}, {group, upload}, - {group, graphql} + {group, graphql}, + {group, help} ]. groups() -> @@ -119,7 +120,8 @@ groups() -> {upload, [], upload()}, {upload_with_acl, [], upload_enabled()}, {upload_without_acl, [], upload_enabled()}, - {graphql, [], graphql()}]. + {graphql, [], graphql()}, + {help, [], help()}]. basic() -> [simple_register, @@ -180,13 +182,19 @@ graphql() -> can_handle_execution_error, graphql_error_unknown_command_with_args, graphql_error_unknown_command_without_args, - graphql_error_no_command, - graphql_error_invalid_vars, - graphql_error_no_vars, - graphql_error_too_many_args, - graphql_error_missing_variable, + graphql_no_command, + graphql_error_invalid_args, + graphql_error_invalid_arg_value, + graphql_error_no_arg_value, + graphql_error_missing_args, + graphql_error_unknown_arg, + graphql_arg_help, graphql_command]. +help() -> + [default_help, + old_help]. + suite() -> require_rpc_nodes([mim]) ++ escalus:suite(). @@ -1159,40 +1167,83 @@ graphql_wrong_arguments_number(Config) -> %% Specific commands are tested in graphql_*_SUITE graphql_error_unknown_command_with_args(Config) -> - {Res, 1} = mongooseimctl("account", ["makeCoffee", "{}"], Config), - ?assertMatch({match, _}, re:run(Res, "Unknown command")). + {Res, 1} = mongooseimctl("account", ["makeCoffee", "--strength", "medium"], Config), + ?assertMatch({match, _}, re:run(Res, "Unknown command")), + expect_existing_commands(Res). graphql_error_unknown_command_without_args(Config) -> {Res, 1} = mongooseimctl("account", ["makeCoffee"], Config), - ?assertMatch({match, _}, re:run(Res, "Unknown command")). + ?assertMatch({match, _}, re:run(Res, "Unknown command")), + expect_existing_commands(Res). -graphql_error_no_command(Config) -> - {Res, 1} = mongooseimctl("account", [], Config), - ?assertMatch({match, _}, re:run(Res, "No command")). +graphql_no_command(Config) -> + %% Not an error - lists commands in the given category + {Res, 0} = mongooseimctl("account", [], Config), + expect_existing_commands(Res). -graphql_error_invalid_vars(Config) -> +graphql_error_invalid_args(Config) -> {Res, 1} = mongooseimctl("account", ["countUsers", "now"], Config), - ?assertMatch({match, _}, re:run(Res, "Could not parse")). + ?assertMatch({match, _}, re:run(Res, "Could not parse")), + expect_command_arguments(Res). + +graphql_error_invalid_arg_value(Config) -> + {Res, 1} = mongooseimctl("vcard", ["setVcard", "--user", "user@host", "--vcard", "x"], Config), + %% vCard should be provided in JSON + ?assertMatch({match, _}, re:run(Res, "Invalid value 'x' of argument 'vcard'")), + ?assertMatch({match, _}, re:run(Res, "vcard\s+VcardInput!")). + +graphql_error_no_arg_value(Config) -> + {Res, 1} = mongooseimctl("account", ["countUsers", "--domain"], Config), + ?assertMatch({match, _}, re:run(Res, "Could not parse")), + expect_command_arguments(Res). -graphql_error_no_vars(Config) -> +graphql_error_missing_args(Config) -> {Res, 1} = mongooseimctl("account", ["countUsers"], Config), - ?assertMatch({match, _}, re:run(Res, "This command requires variables")). + ?assertMatch({match, _}, re:run(Res, "Missing mandatory arguments")), + expect_command_arguments(Res). -graphql_error_too_many_args(Config) -> - {Res, 1} = mongooseimctl("account", ["countUsers", "{}", "{}"], Config), - ?assertMatch({match, _}, re:run(Res, "Too many arguments")). +graphql_error_unknown_arg(Config) -> + {Res, 1} = mongooseimctl("account", ["countUsers", "--domain", "localhost", + "--x", "y"], Config), + ?assertMatch({match, _}, re:run(Res, "Unknown argument")), + expect_command_arguments(Res). -graphql_error_missing_variable(Config) -> - {ResJSON, 1} = mongooseimctl("account", ["countUsers", "{}"], Config), - #{<<"errors">> := Errors} = rest_helper:decode(ResJSON, #{return_maps => true}), - ?assertMatch([#{<<"extensions">> := #{<<"code">> := <<"missing_non_null_param">>}}], Errors). +graphql_arg_help(Config) -> + {Res, 0} = mongooseimctl("account", ["countUsers", "--help"], Config), + expect_command_arguments(Res). graphql_command(Config) -> - VarsJSON = jiffy:encode(#{domain => domain()}), - {ResJSON, 0} = mongooseimctl("account", ["countUsers", VarsJSON], Config), + {ResJSON, 0} = mongooseimctl("account", ["countUsers", "--domain", "localhost"], Config), #{<<"data">> := Data} = rest_helper:decode(ResJSON, #{return_maps => true}), ?assertMatch(#{<<"account">> := #{<<"countUsers">> := _}}, Data). +expect_existing_commands(Res) -> + ?assertMatch({match, _}, re:run(Res, "countUsers")). + +expect_command_arguments(Res) -> + ?assertMatch({match, _}, re:run(Res, "domain\s+String!")). + +%%----------------------------------------------------------------- +%% Help tests +%%----------------------------------------------------------------- + +default_help(Config) -> + #{node := Node} = mim(), + CtlCmd = distributed_helper:ctl_path(Node, Config), + {Res, 2} = mongooseimctl_helper:run(CtlCmd, []), + %% Expect category list and no deprecated command list + ?assertMatch({match, _}, re:run(Res, "Usage")), + ?assertMatch({match, _}, re:run(Res, "account\s+Account management")), + ?assertMatch(nomatch, re:run(Res, "add_rosteritem\s+Add an item")). + +old_help(Config) -> + {Res, 2} = mongooseimctl("help", [], Config), + %% Expect deprecated command list and no category list + ?assertMatch({match, _}, re:run(Res, "The following commands are deprecated")), + ?assertMatch({match, _}, re:run(Res, "Usage")), + ?assertMatch(nomatch, re:run(Res, "account\s+Account management")), + ?assertMatch({match, _}, re:run(Res, "add_rosteritem")). + %%----------------------------------------------------------------- %% Improve coverage %%-----------------------------------------------------------------