Improve error handling in offline

big_tests/tests/graphql_offline_SUITE.erl

@@ -6,7 +6,7 @@
 -import(distributed_helper, [mim/0, require_rpc_nodes/1]).
 -import(domain_helper, [host_type/0, domain/0]).
 -import(graphql_helper, [execute_command/4, get_ok_value/2, get_err_code/1, user_to_bin/1,
-                         get_unauthorized/1, get_not_loaded/1]).
+                         get_unauthorized/1, get_not_loaded/1, get_coercion_err_msg/1]).
 -import(config_parser_helper, [mod_config/2]).
 -import(mongooseimctl_helper, [mongooseimctl/3, rpc_call/3]).
 
@@ -44,6 +44,7 @@ admin_offline_tests() ->
      admin_delete_old_messages_test,
      admin_delete_expired_messages2_test,
      admin_delete_old_messages2_test,
+     admin_delete_old_messages_invalid_days,
      admin_delete_expired_messages_no_domain_test,
      admin_delete_old_messages_no_domain_test].
 
@@ -56,6 +57,7 @@ domain_admin_offline_tests() ->
      admin_delete_old_messages_test,
      admin_delete_expired_messages2_test,
      admin_delete_old_messages2_test,
+     admin_delete_old_messages_invalid_days,
      domain_admin_delete_expired_messages_no_permission_test,
      domain_admin_delete_old_messages_no_permission_test].
 
@@ -148,6 +150,14 @@ admin_delete_old_messages2_test(Config, JidMike, JidKate) ->
     admin_delete_old_messages2(Config, JidMike, JidKate, domain()),
     admin_delete_old_messages2(Config, JidMike, JidKate, unprep(domain())).
 
+admin_delete_old_messages_invalid_days(Config) ->
+    Result = delete_old_messages(domain(), -1, Config),
+    ParsedResult = get_coercion_err_msg(Result),
+    ?assertMatch({_, _}, binary:match(ParsedResult, <<"Value is not a positive integer">>)),
+    Result2 = delete_old_messages(domain(), 0, Config),
+    ParsedResult2 = get_coercion_err_msg(Result2),
+    ?assertMatch({_, _}, binary:match(ParsedResult2, <<"Value is not a positive integer">>)).
+
 admin_delete_old_messages2(Config, JidMike, JidKate, Domain) ->
     generate_message(JidMike, JidKate, 2, 1), % not old enough
     generate_message(JidMike, JidKate, 5, -1),

priv/graphql/schemas/admin/offline.gql

@@ -6,6 +6,6 @@ type OfflineAdminMutation @protected @use(modules: ["mod_offline"]){
     deleteExpiredMessages(domain: DomainName!): String @use(arg: "domain")
       @protected(type: DOMAIN, args: ["domain"])
     "Delete messages at least as old as the number of days specified in the parameter"
-    deleteOldMessages(domain: DomainName!, days: Int!): String
+    deleteOldMessages(domain: DomainName!, days: PosInt!): String
       @protected(type: DOMAIN, args: ["domain"]) @use(arg: "domain")
 }

src/ejabberd_admin.erl

@@ -334,16 +334,16 @@ do_register(List) ->
 %%% Purge DB
 %%%
 
--spec delete_expired_messages(jid:lserver()) -> {ok, iolist()} | {error, iolist()}.
+-spec delete_expired_messages(binary()) -> {ok, iolist()} | {error, iolist()}.
 delete_expired_messages(Domain) ->
-    case mod_offline_api:delete_expired_messages(Domain) of
+    case mod_offline_api:delete_expired_messages(jid:nameprep(Domain)) of
         {ok, _} = Result -> Result;
         {_, Message} -> {error, Message}
     end.
 
--spec delete_old_messages(jid:lserver(), Days :: integer()) -> {ok, iolist()} | {error, iolist()}.
+-spec delete_old_messages(binary(), Days :: integer()) -> {ok, iolist()} | {error, iolist()}.
 delete_old_messages(Domain, Days) ->
-    case mod_offline_api:delete_old_messages(Domain, Days) of
+    case mod_offline_api:delete_old_messages(jid:nameprep(Domain), Days) of
         {ok, _} = Result -> Result;
         {_, Message} -> {error, Message}
     end.

src/offline/mod_offline_api.erl

@@ -2,33 +2,33 @@
 
 -export([delete_expired_messages/1, delete_old_messages/2]).
 
--spec delete_expired_messages(jid:lserver()) ->
-    {ok | domain_not_found | server_error, iolist()}.
+-type(api_result()) :: {ok | domain_not_found | server_error, iolist()}.
+
+-spec delete_expired_messages(jid:lserver()) -> api_result().
 delete_expired_messages(Domain) ->
-    call_for_loaded_module(Domain, fun remove_expired_messages/2, {Domain}).
+    call_for_host_type(Domain, fun remove_expired_messages/2, [Domain]).
 
--spec delete_old_messages(jid:lserver(), Days :: integer()) ->
-    {ok | domain_not_found | server_error, iolist()}.
+-spec delete_old_messages(jid:lserver(), Days :: pos_integer()) -> api_result().
 delete_old_messages(Domain, Days) ->
-    call_for_loaded_module(Domain, fun remove_old_messages/2, {Domain, Days}).
+    call_for_host_type(Domain, fun remove_old_messages/3, [Domain, Days]).
 
-call_for_loaded_module(Domain, Function, Args) ->
+call_for_host_type(Domain, Function, Args) ->
     case mongoose_domain_api:get_domain_host_type(Domain) of
         {ok, HostType} ->
-            Function(Args, HostType);
+            apply(Function, [HostType | Args]);
         {error, not_found} ->
             {domain_not_found, "Unknown domain"}
     end.
 
-remove_old_messages({Domain, Days}, HostType) ->
+remove_old_messages(HostType, Domain, Days) ->
     case mod_offline:remove_old_messages(HostType, Domain, Days) of
         {ok, C} ->
             {ok, io_lib:format("Removed ~p messages", [C])};
         {error, Reason} ->
             {server_error, io_lib:format("Can't remove old messages: ~n~p", [Reason])}
     end.
 
-remove_expired_messages({Domain}, HostType) ->
+remove_expired_messages(HostType, Domain) ->
     case mod_offline:remove_expired_messages(HostType, Domain) of
         {ok, C} ->
             {ok, io_lib:format("Removed ~p messages", [C])};