Merge pull request #3873 from esl/api-modules/roster

Fixing errors in mod_roster
esl · Dec 1, 2022 · 2affe7e · 2affe7e
2 parents 3321ef4 + 6555ab7
commit 2affe7e
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 19 deletions.
diff --git a/big_tests/tests/graphql_roster_SUITE.erl b/big_tests/tests/graphql_roster_SUITE.erl
@@ -6,7 +6,7 @@
 -import(graphql_helper, [execute_user_command/5, execute_command/4, get_listener_port/1,
                          get_listener_config/1, get_ok_value/2, get_err_value/2, get_err_msg/1,
                          get_err_msg/2, get_bad_request/1, user_to_jid/1, user_to_bin/1,
-                         get_unauthorized/1]).
+                         get_unauthorized/1, get_err_code/1]).
 
 -include_lib("eunit/include/eunit.hrl").
 -include_lib("../../include/mod_roster.hrl").
@@ -65,7 +65,8 @@ admin_roster_tests() ->
      admin_list_contacts,
      admin_list_contacts_wrong_user,
      admin_get_contact,
-     admin_get_contact_wrong_user
+     admin_get_contact_wrong_user,
+     admin_subscribe_all_to_all_empty_list
     ].
 
 domain_admin_tests() ->
@@ -174,8 +175,7 @@ admin_try_add_contact_to_nonexistent_user(Config) ->
     User = ?NONEXISTENT_USER,
     Contact = ?NONEXISTENT_USER2,
     Res = admin_add_contact(User, Contact, Config),
-    ?assertNotEqual(nomatch, binary:match(get_err_msg(Res), User)),
-    check_contacts([], User).
+    ?assertEqual(<<"user_not_exist">>, get_err_code(Res)).
 
 admin_try_add_contact_with_unknown_domain(Config) ->
     User = ?NONEXISTENT_DOMAIN_USER,
@@ -402,7 +402,7 @@ admin_list_contacts_wrong_user(Config) ->
     ?assertNotEqual(nomatch, binary:match(get_err_msg(Res), <<"not found">>)),
     % Non-existent user with existent domain
     Res2 = admin_list_contacts(?NONEXISTENT_USER, Config),
-    ?assertEqual([], get_ok_value(?LIST_CONTACTS_PATH, Res2)).
+    ?assertEqual(<<"user_not_exist">>, get_err_code(Res2)).
 
 admin_get_contact(Config) ->
     escalus:fresh_story_with_config(Config, [{alice, 1}, {bob, 1}],
@@ -425,6 +425,10 @@ admin_get_contact_wrong_user(Config) ->
     Res2 = admin_get_contact(?NONEXISTENT_USER, ?NONEXISTENT_USER, Config),
     ?assertNotEqual(nomatch, binary:match(get_err_msg(Res2), <<"does not exist">>)).
 
+admin_subscribe_all_to_all_empty_list(Config) ->
+    Res = admin_subscribe_all_to_all([], Config),
+    ?assertEqual([], get_ok_value(?SUBSCRIBE_ALL_TO_ALL_PATH, Res)).
+
 %% User test cases
 
 user_add_and_delete_contact(Config) ->
@@ -661,7 +665,7 @@ domain_admin_list_contacts_wrong_user(Config) ->
     get_unauthorized(Res),
     % Non-existent user with existent domain
     Res2 = admin_list_contacts(?NONEXISTENT_USER, Config),
-    ?assertEqual([], get_ok_value(?LIST_CONTACTS_PATH, Res2)).
+    ?assertEqual(<<"user_not_exist">>, get_err_code(Res2)).
 
 domain_admin_list_contacts_no_permission(Config) ->
     escalus:fresh_story_with_config(Config, [{alice_bis, 1}],

diff --git a/priv/graphql/schemas/admin/roster.gql b/priv/graphql/schemas/admin/roster.gql
@@ -9,7 +9,7 @@ type RosterAdminMutation @protected{
   addContacts(user: JID!, contacts: [ContactInput!]!) : [String]!
     @protected(type: DOMAIN, args: ["user"])
   "Manage the user's subscription to the contact"
-  subscription(user: JID!, contact: JID!, action: SubAction): String
+  subscription(user: JID!, contact: JID!, action: SubAction!): String
     @protected(type: DOMAIN, args: ["user"])
   "Delete user's contact"
   deleteContact(user: JID!, contact: JID!): String

diff --git a/priv/graphql/schemas/user/roster.gql b/priv/graphql/schemas/user/roster.gql
@@ -19,7 +19,7 @@ Allow user to get information about user roster/contacts.
 """
 type RosterUserQuery @protected{
   "Get the user's roster/contacts"
-  listContacts: [Contact!] 
+  listContacts: [Contact!]
   "Get the user's contact"
   getContact(contact: JID!): Contact
 }
diff --git a/src/graphql/admin/mongoose_graphql_roster_admin_mutation.erl b/src/graphql/admin/mongoose_graphql_roster_admin_mutation.erl
@@ -81,6 +81,8 @@ do_subscribe_to_all(User, Contacts) ->
 
 -spec do_subscribe_all_to_all([mongoose_graphql_roster:contact_input()]) ->
     [mongoose_graphql_roster:binary_result()].
+do_subscribe_all_to_all([]) ->
+    [];
 do_subscribe_all_to_all([_]) ->
     [];
 do_subscribe_all_to_all([User | Contacts]) ->

diff --git a/src/mod_roster_api.erl b/src/mod_roster_api.erl
@@ -44,19 +44,26 @@ add_contact(#jid{lserver = LServer} = CallerJID, ContactJID, Name, Groups) ->
 list_contacts(#jid{lserver = LServer} = CallerJID) ->
     case mongoose_domain_api:get_domain_host_type(LServer) of
         {ok, HostType} ->
-            Acc0 = mongoose_acc:new(#{ location => ?LOCATION,
-                                       host_type => HostType,
-                                       lserver => LServer,
-                                       element => undefined }),
-            Acc1 = mongoose_acc:set(roster, show_full_roster, true, Acc0),
-            Acc2 = mongoose_hooks:roster_get(Acc1, CallerJID),
-            {ok, mongoose_acc:get(roster, items, Acc2)};
+            case ejabberd_auth:does_user_exist(CallerJID) of
+                true ->
+                    Acc0 = mongoose_acc:new(#{ location => ?LOCATION,
+                                               host_type => HostType,
+                                               lserver => LServer,
+                                               element => undefined }),
+                    Acc1 = mongoose_acc:set(roster, show_full_roster, true, Acc0),
+                    Acc2 = mongoose_hooks:roster_get(Acc1, CallerJID),
+                    {ok, mongoose_acc:get(roster, items, Acc2)};
+                false ->
+                    {user_not_exist, io_lib:format("The user ~s does not exist",
+                                                   [jid:to_binary(CallerJID)])}
+            end;
         {error, not_found} ->
             ?UNKNOWN_DOMAIN_RESULT
     end.
 
 -spec get_contact(jid:jid(), jid:jid()) ->
-    {ok, mod_roster:roster()} | {contact_not_found | internal | unknown_domain, iolist()}.
+    {ok, mod_roster:roster()} |
+    {contact_not_found | internal | unknown_domain | user_not_exist, iolist()}.
 get_contact(#jid{lserver = LServer} = UserJID, ContactJID) ->
     case mongoose_domain_api:get_domain_host_type(LServer) of
         {ok, HostType} ->
@@ -107,7 +114,7 @@ subscription(#jid{lserver = LServer} = CallerJID, ContactJID, Type) ->
                                        lserver => LServer,
                                        element => El }),
             Acc2 = mongoose_hooks:roster_out_subscription(Acc1, CallerJID, ContactJID, Type),
-            ejabberd_router:route(CallerJID, ContactJID, Acc2),
+            ejabberd_router:route(CallerJID, jid:to_bare(ContactJID), Acc2),
             {ok, io_lib:format("Subscription stanza with type ~s sent successfully", [StanzaType])};
         {error, not_found} ->
             ?UNKNOWN_DOMAIN_RESULT
@@ -122,7 +129,7 @@ set_mutual_subscription(UserA, UserB, disconnect) ->
            fun() -> delete_contact(UserB, UserA) end],
     case run_seq(Seq, ok) of
         ok ->
-            {ok, "Mututal subscription removed successfully"};
+            {ok, "Mutual subscription removed successfully"};
         Error ->
             Error
     end.
@@ -138,7 +145,7 @@ subscribe_both({UserA, NameA, GroupsA}, {UserB, NameB, GroupsB}) ->
            fun() -> subscription(UserB, UserA, subscribed) end],
     case run_seq(Seq, ok) of
         ok ->
-            {ok, io_lib:format("Subscription between users ~p and ~p created successfully",
+            {ok, io_lib:format("Subscription between users ~s and ~s created successfully",
                                [jid:to_binary(UserA), jid:to_binary(UserB)])};
         Error ->
             Error