Improve error handling in mnesia API

big_tests/tests/graphql_mnesia_SUITE.erl

@@ -1,14 +1,14 @@
 -module(graphql_mnesia_SUITE).
 -include_lib("eunit/include/eunit.hrl").
--include_lib("common_test/include/ct.hrl").
 
 -compile([export_all, nowarn_export_all]).
 
 -import(distributed_helper, [require_rpc_nodes/1, mim/0, mim2/0, rpc/4]).
 -import(domain_helper, [host_type/1]).
 -import(mongooseimctl_helper, [rpc_call/3]).
 -import(graphql_helper, [execute_command/4, execute_user_command/5, user_to_bin/1,
-                         get_ok_value/2, get_err_code/1, get_err_value/2, get_unauthorized/1]).
+                         get_ok_value/2, get_err_code/1, get_err_value/2, get_unauthorized/1,
+                         get_coercion_err_msg/1]).
 
 -record(mnesia_table_test, {key :: integer(), name :: binary()}).
 -record(vcard, {us, vcard}).
@@ -33,17 +33,20 @@ admin_mnesia_tests() ->
      backup_wrong_filename_test,
      restore_no_file_test,
      restore_wrong_file_format_test,
+     restore_bad_file_test,
      load_mnesia_test,
      load_mnesia_no_file_test,
      load_mnesia_bad_file_test,
      load_mnesia_bad_file2_test,
      change_nodename_test,
+     change_nodename_incorrect_name,
      change_nodename_no_file_error_test,
      change_nodename_bad_file_error_test,
      get_info_test,
      get_all_info_test,
      install_fallback_error_test,
-     set_master_test].
+     set_master_test,
+     set_incorrect_master_test].
 
 domain_admin_tests() ->
     [domain_admin_dump_mnesia_table_test,
@@ -136,6 +139,10 @@ restore_no_file_test(Config) ->
     Res = restore_mnesia(<<>>, Config),
     ?assertEqual(<<"file_not_found">>, get_err_code(Res)).
 
+restore_bad_file_test(Config) ->
+    Res = restore_mnesia(<<"NON_EXISTING">>, Config),
+    ?assertEqual(<<"file_not_found">>, get_err_code(Res)).
+
 restore_wrong_file_format_test(Config) ->
     Filename = <<"restore_error">>,
     FileFullPath = create_full_filename(Filename),
@@ -185,6 +192,14 @@ change_nodename_test(Config) ->
     check_if_response_contains(Value,
         <<"Name of the node in the backup was successfully changed">>).
 
+change_nodename_incorrect_name(Config) ->
+    Filename1 = <<"change_incorrect_nodename_mnesia_test">>,
+    Filename2 = <<"change_incorrect_nodename2_mnesia_test">>,
+    ChangeFrom = <<"mongooseim@localhost">>,
+    ChangeTo = <<"incorrect_format">>,
+    Value = change_nodename(ChangeFrom, ChangeTo, Filename1, Filename2, Config),
+    get_coercion_err_msg(Value).
+
 change_nodename_no_file_error_test(Config) ->
     Filename1 = <<"non_existing">>,
     Filename2 = <<"change_nodename2_mnesia_test">>,
@@ -232,6 +247,10 @@ set_master_test(Config) ->
     ParsedRes = get_ok_value([data, mnesia, setMaster], set_master(mim(), Config)),
     ?assertEqual(<<"Master node set">>, ParsedRes).
 
+set_incorrect_master_test(Config) ->
+    Res = set_master(#{node => incorrect_name}, Config),
+    get_coercion_err_msg(Res).
+
 % Domain admin tests
 
 domain_admin_dump_mnesia_table_test(Config) ->
@@ -250,7 +269,11 @@ domain_admin_load_mnesia_test(Config) ->
     get_unauthorized(load_mnesia(<<"Path">>, Config)).
 
 domain_admin_change_nodename_test(Config) ->
-    get_unauthorized(change_nodename(<<"From">>, <<"To">>, <<"file1">>, <<"file2">>, Config)).
+    Filename1 = <<"change_nodename_mnesia_test">>,
+    Filename2 = <<"change_nodename2_mnesia_test">>,
+    ChangeFrom = <<"mongooseim@localhost">>,
+    ChangeTo = <<"change_nodename_test@localhost">>,
+    get_unauthorized(change_nodename(ChangeFrom, ChangeTo, Filename1, Filename2, Config)).
 
 domain_admin_install_fallback_test(Config) ->
     get_unauthorized(install_fallback(<<"Path">>, Config)).

priv/graphql/schemas/admin/mnesia.gql

@@ -15,10 +15,10 @@ Allow admin to backup, dump, load, restore and modify mnesia database
 """
 type MnesiaAdminMutation @protected{
   "Set mnesia's master node"
-  setMaster(node: String!): String
+  setMaster(node: NodeName!): String
     @protected(type: GLOBAL)
   "Change nodename from 'fromString' to 'toString' in 'source' backup file and create new 'target' backup file"
-  changeNodename(fromString: String!, toString: String!,
+  changeNodename(fromString: NodeName!, toString: NodeName!,
                  source: String!, target: String!): String
     @protected(type: GLOBAL)
   "Save mnesia backup to file 'path'"

priv/graphql/schemas/global/scalar_types.gql

@@ -16,6 +16,8 @@ scalar RoomName @spectaql(options: [{ key: "example", value: "my-chat-room" }])
 scalar DomainName @spectaql(options: [{ key: "example", value: "localhost" }])
 "XMPP resource name (resource part of a JID)"
 scalar ResourceName @spectaql(options: [{ key: "example", value: "res1" }])
+"Name of the Erlang node"
+scalar NodeName @spectaql(options: [{key: "example", value: "mynode@localhost"}])
 "String that contains at least one character"
 scalar NonEmptyString @spectaql(options: [{ key: "example", value: "xyz789" }])
 "Integer that has a value above zero"

src/graphql/admin/mongoose_graphql_mnesia_admin_mutation.erl

@@ -3,51 +3,49 @@
 
 -export([execute/4]).
 
--import(mongoose_graphql_helper, [make_error/2]).
+-import(mongoose_graphql_helper, [make_error/2, make_error/3]).
 
 -ignore_xref([execute/4]).
 
 -include("../mongoose_graphql_types.hrl").
--include("mongoose.hrl").
--include("jlib.hrl").
 
 execute(_Ctx, mnesia, <<"setMaster">>, #{<<"node">> := Node}) ->
     case mnesia_api:set_master(binary_to_list(Node)) of
         {ok, _} -> {ok, "Master node set"};
-        {error, Reason} -> make_error({error, Reason}, #{node => Node})
+        Error -> make_error(Error, #{node => Node})
     end;
 execute(_Ctx, mnesia, <<"backup">>, #{<<"path">> := Path}) ->
     case mnesia_api:backup_mnesia(binary_to_list(Path)) of
         {ok, _} -> {ok, "Mnesia backup was successfully created"};
-        {error, Error} -> make_error(Error, #{path => Path})
+        Error -> make_error(Error, #{path => Path})
     end;
 execute(_Ctx, mnesia, <<"changeNodename">>, #{<<"fromString">> := FromString,
     <<"toString">> := ToString, <<"source">> := Source, <<"target">> := Target}) ->
     case mnesia_api:mnesia_change_nodename(binary_to_list(FromString), binary_to_list(ToString),
                                            binary_to_list(Source), binary_to_list(Target)) of
         {ok, _} -> {ok, "Name of the node in the backup was successfully changed"};
-        {error, Error} -> make_error(Error, #{fromString => FromString, toString => ToString,
-                                              source => Source, target => Target})
+        Error -> make_error(Error, #{fromString => FromString, toString => ToString,
+                                     source => Source, target => Target})
     end;
 execute(_Ctx, mnesia, <<"restore">>, #{<<"path">> := Path}) ->
     case mnesia_api:restore_mnesia(binary_to_list(Path)) of
         {ok, _} -> {ok, "Mnesia was successfully restored"};
-        {error, Error} -> make_error(Error, #{path => Path})
+        Error -> make_error(Error, #{path => Path})
     end;
 execute(_Ctx, mnesia, <<"dump">>, #{<<"path">> := Path}) ->
     case mnesia_api:dump_mnesia(binary_to_list(Path)) of
         {ok, _} -> {ok, "Mnesia successfully dumped"};
-        {error, Error} -> make_error(Error, #{path => Path})
+        Error -> make_error(Error, #{path => Path})
     end;
 execute(_Ctx, mnesia, <<"dumpTable">>, #{<<"path">> := Path, <<"table">> := Table}) ->
     case mnesia_api:dump_table(binary_to_list(Path), binary_to_list(Table)) of
         {ok, _} -> {ok, "Mnesia table successfully dumped"};
-        {error, Error} -> make_error(Error, #{path => Path, table => Table})
+        Error -> make_error(Error, #{path => Path, table => Table})
     end;
 execute(_Ctx, mnesia, <<"load">>, #{<<"path">> := Path}) ->
     case mnesia_api:load_mnesia(binary_to_list(Path)) of
         {ok, _} -> {ok, "Mnesia was successfully loaded"};
-        {error, Error} -> make_error(Error, #{path => Path})
+        Error -> make_error(Error, #{path => Path})
     end;
 execute(_Ctx, mnesia, <<"installFallback">>, #{<<"path">> := Path}) ->
     case mnesia_api:install_fallback_mnesia(binary_to_list(Path)) of

src/graphql/admin/mongoose_graphql_mnesia_admin_query.erl

@@ -8,11 +8,9 @@
 -ignore_xref([execute/4]).
 
 -include("../mongoose_graphql_types.hrl").
--include("mongoose.hrl").
--include("jlib.hrl").
 
 execute(_Ctx, mnesia, <<"systemInfo">>, #{<<"keys">> := Keys}) ->
-    ResultList = mnesia_api:mnesia_info(Keys),
+    {ok, ResultList} = mnesia_api:mnesia_info(Keys),
     {ok, lists:map(fun process_result/1, ResultList)}.
 
 process_result({ok, _} = Result) -> Result;

src/graphql/mongoose_graphql_scalar.erl

@@ -19,6 +19,7 @@ input(<<"UserName">>, User) -> user_from_binary(User);
 input(<<"RoomName">>, Room) -> room_from_binary(Room);
 input(<<"DomainName">>, Domain) -> domain_from_binary(Domain);
 input(<<"ResourceName">>, Res) -> resource_from_binary(Res);
+input(<<"NodeName">>, Node) -> node_from_binary(Node);
 input(<<"NonEmptyString">>, Value) -> non_empty_string_to_binary(Value);
 input(<<"PosInt">>, Value) -> validate_pos_integer(Value);
 input(<<"NonNegInt">>, Value) -> validate_non_neg_integer(Value);
@@ -119,6 +120,16 @@ resource_from_binary(Value) ->
             {ok, Res}
     end.
 
+node_from_binary(<<>>) ->
+    {error, empty_node_name};
+node_from_binary(NodeName) ->
+    case string:lexemes(binary_to_list(NodeName), "@") of
+        [_Name, _Host] ->
+            {ok, NodeName};
+        _ ->
+            {error, incorrect_node_name}
+    end.
+
 binary_to_microseconds(DT) ->
     case mod_mam_utils:maybe_microseconds(DT) of
         undefined ->

src/mnesia_api.erl

@@ -14,11 +14,12 @@
 -spec mnesia_info(Keys::[binary()]) -> [info_result() | info_error()].
 mnesia_info(null) ->
     Value = mnesia:system_info(all),
-    lists:foldl(fun({Key, Result}, AllAcc) ->
+    Result = lists:foldl(fun({Key, Result}, AllAcc) ->
         AllAcc ++ [{ok, #{<<"result">> => convert_value(Result), <<"key">> => Key}}]
-    end, [], Value);
+    end, [], Value),
+    {ok, Result};
 mnesia_info(Keys) ->
-    lists:foldl(fun
+    Result = lists:foldl(fun
         (<<"all">>, Acc) ->
             Acc ++ [{{bad_key_error, <<"Key \"all\" does not exist">>},
                     #{key => <<"all">>}}];
@@ -33,7 +34,8 @@ mnesia_info(Keys) ->
                 _:_ ->
                     Acc ++ [{{internal_server_error, <<"Internal server error">>}, #{key => Key}}]
             end
-    end, [], Keys).
+    end, [], Keys),
+    {ok, Result}.
 
 -spec dump_mnesia(file:name()) -> {error, {dump_error(), io_lib:chars()}} | {ok, []}.
 dump_mnesia(Path) ->
@@ -50,13 +52,13 @@ dump_table(Path, STable) ->
 backup_mnesia(Path) ->
     case mnesia:backup(Path) of
         ok ->
-            {ok, ""};
+            {ok, filename:absname(Path)};
         {error, {'EXIT', {error, enoent}}} ->
-            {error, {wrong_filename, io_lib:format("Wrong filename: ~p", [Path])}};
+            {wrong_filename, io_lib:format("Wrong filename: ~p", [Path])};
         {error, Reason} ->
             String = io_lib:format("Can't store backup in ~p at node ~p: ~p",
                                    [filename:absname(Path), node(), Reason]),
-            {error, {cannot_backup, String}}
+            {cannot_backup, String}
     end.
 
 -spec restore_mnesia(file:name()) -> {error, {cannot_restore, io_lib:chars()}}
@@ -72,16 +74,16 @@ restore_mnesia(Path) ->
             {ok, ""};
         {aborted, {no_exists, Table}} ->
             String = io_lib:format("~sTable ~p does not exist.", [ErrorString, Table]),
-            {error, {table_does_not_exist, String}};
+            {table_does_not_exist, String};
         {aborted, enoent} ->
             String = ErrorString ++ "File not found.",
-            {error, {file_not_found, String}};
+            {file_not_found, String};
         {aborted, {not_a_log_file, Filename}} ->
             String = "Wrong file " ++ Filename ++ " structure",
-            {error, {not_a_log_file_error, String}};
+            {not_a_log_file_error, String};
         {aborted, Reason} ->
             String = io_lib:format("~s~p", [ErrorString, Reason]),
-            {error, {cannot_restore, String}}
+            {cannot_restore, String}
     end.
 
 -spec load_mnesia(file:name()) ->
@@ -91,15 +93,15 @@ load_mnesia(Path) ->
         {atomic, ok} ->
             {ok, ""};
         {error, bad_header} ->
-            {error, {bad_file_format, "File has wrong format"}};
+            {bad_file_format, "File has wrong format"};
         {error, read} ->
-            {error, {bad_file_format, "File has wrong format"}};
+            {bad_file_format, "File has wrong format"};
         {error, open} ->
-            {error, {file_not_found, "File was not found"}};
+            {file_not_found, "File was not found"};
         {error, Reason} ->
             String = io_lib:format("Can't load dump in ~p at node ~p: ~p",
                                    [filename:absname(Path), node(), Reason]),
-            {error, {cannot_load, String}}
+            {cannot_load, String}
     end.
 
 -spec mnesia_change_nodename(string(), string(), _, _) -> {ok, _} | {error, _}.
@@ -112,7 +114,6 @@ mnesia_change_nodename(FromString, ToString, Source, Target) ->
                 io:format("     - Replacing nodename: '~p' with: '~p'~n", [From, To]),
                 To;
             (Node) when Node == To ->
-                %% throw({error, already_exists});
                 io:format("     - Node: '~p' will not be modified (it is already '~p')~n", [Node, To]),
                 Node;
             (Node) ->
@@ -154,11 +155,11 @@ mnesia_change_nodename(FromString, ToString, Source, Target) ->
                                    [node(), Reason]),
             case Reason of
                 {_, enoent} ->
-                    {error, {file_not_found, String}};
+                    {file_not_found, String};
                 {_, {not_a_log_file, _}} ->
-                    {error, {bad_file_format, String}};
+                    {bad_file_format, String};
                 _ ->
-                    {error, {error, String}}
+                    {cannot_change, String}
             end
     end.
 
@@ -186,7 +187,7 @@ set_master(Node) when is_atom(Node) ->
         {error, Reason} ->
             String = io_lib:format("Can't set master node ~p at node ~p:~n~p",
                                    [Node, node(), Reason]),
-            {error, String}
+            {cannot_set, String}
     end.
 
 %---------------------------------------------------------------------------------------------------
@@ -215,15 +216,15 @@ dump_tables(File, Tabs) ->
         {file_error, Reason} ->
             String = io_lib:format("Can't store dump in ~p at node ~p: ~p",
                                    [filename:absname(File), node(), Reason]),
-            {error, {file_error, String}};
+            {file_error, String};
         {error, Reason} ->
             String = io_lib:format("Can't store dump in ~p at node ~p: ~p",
                                    [filename:absname(File), node(), Reason]),
             case Reason of
                 table_does_not_exist ->
-                    {error, {table_does_not_exist, String}};
+                    {table_does_not_exist, String};
                 _ ->
-                    {error, {cannot_dump, String}}
+                    {cannot_dump, String}
             end
     end.