From af3b39c11abcab506fc32fabffc3885b8de9e23c Mon Sep 17 00:00:00 2001 From: jacekwegr Date: Thu, 8 Dec 2022 13:42:44 +0100 Subject: [PATCH] Improve error handling in mnesia API --- big_tests/tests/graphql_mnesia_SUITE.erl | 31 +++++++- priv/graphql/schemas/admin/mnesia.gql | 4 +- priv/graphql/schemas/global/scalar_types.gql | 2 + ...mongoose_graphql_mnesia_admin_mutation.erl | 20 +++-- .../mongoose_graphql_mnesia_admin_query.erl | 4 +- src/graphql/mongoose_graphql_scalar.erl | 11 +++ src/mnesia_api.erl | 77 ++++++++++--------- 7 files changed, 91 insertions(+), 58 deletions(-) diff --git a/big_tests/tests/graphql_mnesia_SUITE.erl b/big_tests/tests/graphql_mnesia_SUITE.erl index 8584b48d05b..06f31445e72 100644 --- a/big_tests/tests/graphql_mnesia_SUITE.erl +++ b/big_tests/tests/graphql_mnesia_SUITE.erl @@ -1,6 +1,5 @@ -module(graphql_mnesia_SUITE). -include_lib("eunit/include/eunit.hrl"). --include_lib("common_test/include/ct.hrl"). -compile([export_all, nowarn_export_all]). @@ -8,7 +7,8 @@ -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)). diff --git a/priv/graphql/schemas/admin/mnesia.gql b/priv/graphql/schemas/admin/mnesia.gql index 1a859a23534..00ce17adc63 100644 --- a/priv/graphql/schemas/admin/mnesia.gql +++ b/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'" diff --git a/priv/graphql/schemas/global/scalar_types.gql b/priv/graphql/schemas/global/scalar_types.gql index e84ddbb7338..ae7a61ffc6b 100644 --- a/priv/graphql/schemas/global/scalar_types.gql +++ b/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" diff --git a/src/graphql/admin/mongoose_graphql_mnesia_admin_mutation.erl b/src/graphql/admin/mongoose_graphql_mnesia_admin_mutation.erl index b0ab6457cad..9fef39acec6 100644 --- a/src/graphql/admin/mongoose_graphql_mnesia_admin_mutation.erl +++ b/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 diff --git a/src/graphql/admin/mongoose_graphql_mnesia_admin_query.erl b/src/graphql/admin/mongoose_graphql_mnesia_admin_query.erl index e3d6fe9fead..8f7abcb3384 100644 --- a/src/graphql/admin/mongoose_graphql_mnesia_admin_query.erl +++ b/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; diff --git a/src/graphql/mongoose_graphql_scalar.erl b/src/graphql/mongoose_graphql_scalar.erl index 6a58bcc00a3..05af0b99bf7 100644 --- a/src/graphql/mongoose_graphql_scalar.erl +++ b/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 -> diff --git a/src/mnesia_api.erl b/src/mnesia_api.erl index 9ea10f32bbd..6cf4643faf9 100644 --- a/src/mnesia_api.erl +++ b/src/mnesia_api.erl @@ -7,18 +7,24 @@ mnesia_change_nodename/4, restore/1, mnesia_info/1]). --type info_result() :: #{binary() => binary() | [binary()] | integer()}. +-type info_result() :: {ok, #{binary() => binary() | [binary()] | integer()}}. -type info_error() :: {{internal_server_error | bad_key_error, binary()}, #{key => binary()}}. -type dump_error() :: table_does_not_exist | file_error | cannot_dump. +-type restore_error() :: cannot_restore | file_not_found | not_a_log_file_error | + table_does_not_exist. +-type backup_error() :: wrong_filename | cannot_backup. +-type load_error() :: cannot_load | bad_file_format | file_not_found. +-type change_error() :: file_not_found | bad_file_format | cannot_change. --spec mnesia_info(Keys::[binary()]) -> [info_result() | info_error()]. +-spec mnesia_info(Keys::[binary()]) -> {ok, [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,37 +39,33 @@ 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, []}. +-spec dump_mnesia(file:name()) -> {dump_error(), io_lib:chars()} | {ok, []}. dump_mnesia(Path) -> Tabs = get_local_tables(), dump_tables(Path, Tabs). --spec dump_table(file:name(), string()) -> {error, {dump_error(), io_lib:chars()}} | {ok, []}. +-spec dump_table(file:name(), string()) -> {dump_error(), io_lib:chars()} | {ok, []}. dump_table(Path, STable) -> Table = list_to_atom(STable), dump_tables(Path, [Table]). --spec backup_mnesia(file:name()) -> - {error, {wrong_filename | cannot_backup, io_lib:chars()}} | {ok, []}. +-spec backup_mnesia(file:name()) -> {backup_error(), io_lib:chars()} | {ok, []}. backup_mnesia(Path) -> case mnesia:backup(Path) of ok -> {ok, ""}; {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()}} - | {error, {file_not_found, io_lib:chars()}} - | {error, {not_a_log_file_error, io_lib:chars()}} - | {ok, []} - | {error, {table_does_not_exist, io_lib:chars()}}. +-spec restore_mnesia(file:name()) -> {restore_error(), io_lib:chars()} | {ok, []}. restore_mnesia(Path) -> ErrorString=lists:flatten( io_lib:format("Can't restore backup from ~p at node ~p: ", [filename:absname(Path), node()])), @@ -72,37 +74,37 @@ 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()) -> - {error, {cannot_load | bad_file_format | file_not_found, io_lib:chars()}} | {ok, []}. +-spec load_mnesia(file:name()) -> {load_error(), io_lib:chars()} | {ok, []}. load_mnesia(Path) -> case mnesia:load_textfile(Path) of {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, _}. +-spec mnesia_change_nodename(string(), string(), _, _) -> + {ok, _} | {change_error(), io_lib:chars()}. mnesia_change_nodename(FromString, ToString, Source, Target) -> From = list_to_atom(FromString), To = list_to_atom(ToString), @@ -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. @@ -174,7 +175,7 @@ install_fallback_mnesia(Path) -> {cannot_fallback, String} end. --spec set_master(Node :: atom() | string()) -> {error, io_lib:chars()} | {ok, []}. +-spec set_master(Node :: atom() | string()) -> {cannot_set, io_lib:chars()} | {ok, []}. set_master("self") -> set_master(node()); set_master(NodeString) when is_list(NodeString) -> @@ -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. %--------------------------------------------------------------------------------------------------- @@ -207,7 +208,7 @@ convert_value(Value) when is_list(Value) -> convert_value(Value) -> list_to_binary(io_lib:format("~p", [Value])). --spec dump_tables(file:name(), list()) -> {error, {dump_error(), io_lib:chars()}} | {ok, []}. +-spec dump_tables(file:name(), list()) -> {dump_error(), io_lib:chars()} | {ok, []}. dump_tables(File, Tabs) -> case dump_to_textfile(Tabs, file:open(File, [write])) of ok -> @@ -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.