Divide command processing into steps

This way it is easier to control the error messages,
and to print out additional hints.
There is a plan to add more usage information in a new PR.

big_tests/tests/graphql_helper.erl

@@ -25,7 +25,8 @@ execute_command(Category, Command, Args, Config) ->
     execute_command(Category, Command, Args, Config, Protocol).
 
 execute_command(Category, Command, Args, Config, http) ->
-    {ok, Doc} = rpc(mim(), mongoose_graphql_commands, find_document, [Category, Command]),
+    #{Category := #{Command := #{doc := Doc}}} =
+        rpc(mim(), mongoose_graphql_commands, get_specs, []),
     execute_auth(#{query => Doc, variables => Args}, Config);
 execute_command(Category, Command, Args, Config, cli) ->
     VarsJSON = jiffy:encode(Args),

big_tests/tests/mongooseimctl_SUITE.erl

@@ -177,7 +177,15 @@ upload_enabled() ->
 graphql() ->
     [graphql_wrong_arguments_number,
      can_execute_admin_queries_with_permissions,
-     can_handle_execution_error].
+     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_command].
 
 suite() ->
     require_rpc_nodes([mim]) ++ escalus:suite().
@@ -1147,6 +1155,44 @@ graphql_wrong_arguments_number(Config) ->
     Data2 = element(1, ResTooManyArgs),
     ?assertNotEqual(nomatch, string:find(Data2, ExpectedFragment)).
 
+%% Generic GraphQL command tests
+%% 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")).
+
+graphql_error_unknown_command_without_args(Config) ->
+    {Res, 1} = mongooseimctl("account", ["makeCoffee"], Config),
+    ?assertMatch({match, _}, re:run(Res, "Unknown command")).
+
+graphql_error_no_command(Config) ->
+    {Res, 1} = mongooseimctl("account", [], Config),
+    ?assertMatch({match, _}, re:run(Res, "No command")).
+
+graphql_error_invalid_vars(Config) ->
+    {Res, 1} = mongooseimctl("account", ["countUsers", "now"], Config),
+    ?assertMatch({match, _}, re:run(Res, "Could not parse")).
+
+graphql_error_no_vars(Config) ->
+    {Res, 1} = mongooseimctl("account", ["countUsers"], Config),
+    ?assertMatch({match, _}, re:run(Res, "This command requires variables")).
+
+graphql_error_too_many_args(Config) ->
+    {Res, 1} = mongooseimctl("account", ["countUsers", "{}", "{}"], Config),
+    ?assertMatch({match, _}, re:run(Res, "Too many arguments")).
+
+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_command(Config) ->
+    VarsJSON = jiffy:encode(#{domain => domain()}),
+    {ResJSON, 0} = mongooseimctl("account", ["countUsers", VarsJSON], Config),
+    #{<<"data">> := Data} = rest_helper:decode(ResJSON, #{return_maps => true}),
+    ?assertMatch(#{<<"account">> := #{<<"countUsers">> := _}}, Data).
+
 %%-----------------------------------------------------------------
 %% Improve coverage
 %%-----------------------------------------------------------------

src/ejabberd_ctl.erl

@@ -179,7 +179,8 @@ process(["mnesia", Arg]) when is_list(Arg) ->
     ?STATUS_SUCCESS;
 process(["graphql", Arg]) when is_list(Arg) ->
     Doc = list_to_binary(Arg),
-    Result = mongoose_graphql_commands:execute(Doc, #{}), % TODO Support vars?
+    Ep = mongoose_graphql:get_endpoint(admin),
+    Result = mongoose_graphql:execute(Ep, undefined, Doc),
     handle_graphql_result(Result);
 process(["graphql" | _]) ->
     ?PRINT("This command requires one string type argument!\n", []),
@@ -213,19 +214,33 @@ process(["help" | Mode]) ->
             print_usage_commands(CmdStringU, MaxC, ShCode),
             ?STATUS_SUCCESS
     end;
-process([CategoryStr, CommandStr, VarsStr] = Args) ->
-    Category = list_to_binary(CategoryStr),
-    Command = list_to_binary(CommandStr),
-    case mongoose_graphql_commands:find_document(Category, Command) of
-        {ok, Doc} ->
-            Vars = jiffy:decode(VarsStr, [return_maps]),
-            Result = mongoose_graphql_commands:execute(Doc, Vars),
-            handle_graphql_result(Result);
-        {error, not_found} ->
-            run_command(Args)
-    end;
 process(Args) ->
-    run_command(Args).
+    case mongoose_graphql_commands:process(Args) of
+        {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;
+        {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)
+    end.
 
 handle_graphql_result({ok, Result}) ->
     JSONResult = mongoose_graphql_response:term_to_pretty_json(Result),

src/graphql/mongoose_graphql_commands.erl

@@ -2,7 +2,13 @@
 
 -module(mongoose_graphql_commands).
 
--export([start/0, stop/0, find_document/2, execute/2]).
+%% API
+-export([start/0, stop/0, process/1]).
+
+%% Only for tests
+-export([get_specs/0]).
+
+-ignore_xref([get_specs/0]).
 
 %% This level of nesting is needed for basic type introspection, e.g. see below for [String!]!
 %%                         NON_NULL          LIST              NON_NULL          SCALAR
@@ -22,6 +28,14 @@
 -type category_spec() :: #{command() => command_spec()}.
 -type specs() :: #{category() => category_spec()}.
 -type json_map() :: #{binary() => graphql:json()}.
+-type ctx() :: #{args := [string()],
+                 category => category(),
+                 cat_spec => category_spec(),
+                 command => command(),
+                 doc => doc(),
+                 vars => json_map(),
+                 reason => atom()}.
+-type result() :: {ok, #{atom() => graphql:json()}} | {error, any()}.
 
 %% API
 
@@ -39,18 +53,62 @@ stop() ->
     persistent_term:erase(?MODULE),
     ok.
 
--spec find_document(category(), command()) -> {ok, doc()} | {error, not_found}.
-find_document(Category, Command) ->
-    case persistent_term:get(?MODULE) of
-        #{Category := #{Command := CommandSpec}} ->
-            #{doc := Doc} = CommandSpec,
-            {ok, Doc};
-        _ ->
-            {error, not_found}
-    end.
+-spec process([string()]) -> result().
+process(Args) ->
+    lists:foldl(fun(_, {error, _} = Error) -> Error;
+                   (StepF, Ctx) -> StepF(Ctx)
+                end,
+                #{args => Args},
+                [fun find_category/1, fun find_command/1, fun parse_vars/1, fun execute/1]).
+
+-spec get_specs() -> specs().
+get_specs() ->
+    persistent_term:get(?MODULE).
 
--spec execute(doc(), json_map()) -> {ok, map()} | {error, term()}.
-execute(Doc, Vars) ->
+%% Internals
+
+-spec find_category(ctx()) -> ctx() | {error, ctx()}.
+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};
+        #{} ->
+            {error, Ctx#{reason => unknown_category}}
+    end;
+find_category(Ctx = #{args := []}) ->
+    {error, Ctx#{reason => no_args}}.
+
+-spec find_command(ctx()) -> ctx() | {error, ctx()}.
+find_command(CtxIn = #{args := [CommandStr | Args]}) ->
+    Command = list_to_binary(CommandStr),
+    Ctx = #{cat_spec := CatSpec} = CtxIn#{command => Command, args => Args},
+    case CatSpec of
+        #{Command := CommandSpec} ->
+            #{doc := Doc} = CommandSpec,
+            Ctx#{doc => Doc};
+        #{} ->
+            {error, Ctx#{reason => unknown_command}}
+    end;
+find_command(Ctx) ->
+    {error, Ctx#{reason => no_command}}.
+
+-spec parse_vars(ctx()) -> ctx() | {error, ctx()}.
+parse_vars(Ctx = #{args := [VarsStr]}) ->
+    try jiffy:decode(VarsStr, [return_maps]) of
+        Vars ->
+            Ctx#{vars => Vars}
+    catch _:_ ->
+            {error, Ctx#{reason => invalid_vars}}
+    end;
+parse_vars(Ctx = #{args := []}) ->
+    {error, Ctx#{reason => no_vars}};
+parse_vars(Ctx = #{}) ->
+    {error, Ctx#{reason => too_many_args}}.
+
+-spec execute(ctx()) -> result().
+execute(#{doc := Doc, vars := Vars}) ->
     execute(mongoose_graphql:get_endpoint(admin), Doc, Vars).
 
 %% Internals
@@ -186,7 +244,7 @@ return_field(#{name := Name, fields := Fields}) ->
 return_field(#{name := Name}) ->
     Name.
 
--spec execute(ep(), doc(), json_map()) -> {ok, #{atom() => graphql:json()}} | {error, term()}.
+-spec execute(ep(), doc(), json_map()) -> result().
 execute(Ep, Doc, Vars) ->
     mongoose_graphql:execute(Ep, #{document => Doc,
                                    operation_name => undefined,