Apply fixes and remove tests for missing methods

diff --git a/src/mod_admin_extra.erl b/src/mod_admin_extra.erl
index f0e5671996b02a113c4ddac2b40ad17c9099503b..feff8af8a46a57ac1448532dc47d81be6bac615a 100644 (file)
--- a/src/mod_admin_extra.erl
+++ b/src/mod_admin_extra.erl
@@ -590,36 +590,35 @@ remove_node(Node) ->
 %%%
 
 set_password(User, Host, Password) ->
-    case ejabberd_auth:set_password(User, Host, Password) of
-       ok ->
-           ok;
-       _ ->
-           error
-    end.
+    Fun = fun () -> ejabberd_auth:set_password(User, Host, Password) end,
+    user_action(User, Host, Fun, ok).
 
 %% Copied some code from ejabberd_commands.erl
 check_password_hash(User, Host, PasswordHash, HashMethod) ->
     AccountPass = ejabberd_auth:get_password_s(User, Host),
     AccountPassHash = case {AccountPass, HashMethod} of
                          {A, _} when is_tuple(A) -> scrammed;
-                         {_, "md5"} -> get_md5(AccountPass);
-                         {_, "sha"} -> get_sha(AccountPass);
-                         _ -> undefined
+                         {_, <<"md5">>} -> get_md5(AccountPass);
+                         {_, <<"sha">>} -> get_sha(AccountPass);
+                          {_, _Method} ->
+                             ?ERROR_MSG("check_password_hash called "
+                                        "with hash method", [_Method]),
+                             undefined
                      end,
     case AccountPassHash of
        scrammed ->
-           ?ERROR_MSG("Passwords are scrammed, and check_password_hash can not work.", []),
+           ?ERROR_MSG("Passwords are scrammed, and check_password_hash cannot work.", []),
            throw(passwords_scrammed_command_cannot_work);
-       undefined -> error;
+       undefined -> throw(unkown_hash_method);
        PasswordHash -> ok;
-       _ -> error
+       _ -> false
     end.
 get_md5(AccountPass) ->
-    lists:flatten([io_lib:format("~.16B", [X])
-                  || X <- binary_to_list(erlang:md5(AccountPass))]).
+    iolist_to_binary([io_lib:format("~2.16.0B", [X])
+                      || X <- binary_to_list(erlang:md5(AccountPass))]).
 get_sha(AccountPass) ->
-    lists:flatten([io_lib:format("~.16B", [X])
-                  || X <- binary_to_list(p1_sha:sha1(AccountPass))]).
+    iolist_to_binary([io_lib:format("~2.16.0B", [X])
+                     || X <- binary_to_list(p1_sha:sha1(AccountPass))]).
 
 num_active_users(Host, Days) ->
     list_last_activity(Host, true, Days).
@@ -782,7 +781,8 @@ resource_num(User, Host, Num) ->
        true ->
            lists:nth(Num, Resources);
        false ->
-           lists:flatten(io_lib:format("Error: Wrong resource number: ~p", [Num]))
+            throw({bad_argument,
+                   lists:flatten(io_lib:format("Wrong resource number: ~p", [Num]))})
     end.
 
 kick_session(User, Server, Resource, ReasonText) ->
@@ -1569,6 +1569,20 @@ decide_rip_jid({UName, UServer}, Match_list) ->
       end,
       Match_list).
 
+user_action(User, Server, Fun, OK) ->
+    case ejabberd_auth:is_user_exists(User, Server) of
+        true ->
+           case catch Fun() of
+                OK -> ok;
+               {error, Error} -> throw(Error);
+                Error ->
+                    ?ERROR_MSG("Command returned: ~p", [Error]),
+                   1
+           end;
+       false ->
+           throw({not_found, "unknown_user"})
+    end.
+
 %% Copied from ejabberd-2.0.0/src/acl.erl
 is_regexp_match(String, RegExp) ->
     case ejabberd_regexp:run(String, RegExp) of

diff --git a/test/mod_admin_extra_test.exs b/test/mod_admin_extra_test.exs
index d90aa045885d2ce9099f2f7cb96d9220851215ee..d104ed0585c183f6d6eef0db2e08d8a23ae0eb7f 100644 (file)
--- a/test/mod_admin_extra_test.exs
+++ b/test/mod_admin_extra_test.exs
@@ -111,7 +111,7 @@ defmodule EjabberdModAdminExtraTest do
 
        end
 
-       test "change_password works" do
+       test "set_password works" do
                EjabberdAuthMock.create_user @user, @domain, @password
 
                assert :ejabberd_commands.execute_command(:change_password,
@@ -127,25 +127,6 @@ defmodule EjabberdModAdminExtraTest do
                assert :meck.validate :ejabberd_auth
        end
 
-       test "check_users_registration works" do
-               EjabberdAuthMock.create_user @user<>"1", @domain, @password
-               EjabberdAuthMock.create_user @user<>"2", @domain, @password
-               EjabberdAuthMock.create_user @user<>"3", @domain, @password
-
-               assert [{@user<>"0", @domain, 0},
-                                               {@user<>"1", @domain, 1},
-                                               {@user<>"2", @domain, 1},
-                                               {@user<>"3", @domain, 1}] ==
-                       :ejabberd_commands.execute_command(:check_users_registration,
-                                                                                                                                                                [[{@user<>"0", @domain},
-                                                                                                                                                                        {@user<>"1", @domain},
-                                                                                                                                                                        {@user<>"2", @domain},
-                                                                                                                                                                        {@user<>"3", @domain}]])
-
-               assert :meck.validate :ejabberd_auth
-
-       end
-
        ###################### Sessions
 
        test "num_resources works" do
@@ -359,332 +340,6 @@ defmodule EjabberdModAdminExtraTest do
 
        end
 
-
-       test "link_contacts & unlink_contacts work" do
-               # Create user1 and keep it offline
-               EjabberdAuthMock.create_user @user<>"1", @domain, @password
-
-               # fail if one of the users doesn't exist locally
-               assert 404 ==
-                       :ejabberd_commands.execute_command(:link_contacts, [@user<>"1@"<>@domain,
-                                                                                                                                                                                                                                       "nick1",
-                                                                                                                                                                                                                                       "group1",
-                                                                                                                                                                                                                                       @user<>"2@"<>@domain,
-                                                                                                                                                                                                                                       "nick2",
-                                                                                                                                                                                                                                       "group2"])
-
-               # Create user2 and connect 2 resources
-               EjabberdAuthMock.create_user @user<>"2", @domain, @password
-
-               EjabberdSmMock.connect_resource @user<>"2", @domain, @resource<>"1"
-               EjabberdSmMock.connect_resource @user<>"2", @domain, @resource<>"2"
-
-               # Link both user1 & user2 (returns 0 if OK)
-               assert 0 ==
-                       :ejabberd_commands.execute_command(:link_contacts, [@user<>"1@"<>@domain,
-                                                                                                                                                                                                                                        "nick1",
-                                                                                                                                                                                                                                        "group2",
-                                                                                                                                                                                                                                        @user<>"2@"<>@domain,
-                                                                                                                                                                                                                                        "nick2",
-                                                                                                                                                                                                                                        "group1"])
-               assert [{@user<>"2@"<>@domain, "", 'both', 'none', "group2"}] ==
-                       :ejabberd_commands.execute_command(:get_roster, [@user<>"1", @domain], :admin)
-
-               assert [{@user<>"1@"<>@domain, "", 'both', 'none', "group1"}] ==
-                       :ejabberd_commands.execute_command(:get_roster, [@user<>"2", @domain], :admin)
-
-               # Check that the item roster user1 was pushed with subscription
-               # 'both' to the 2 user2 online ressources
-               jid = :jlib.make_jid(@user<>"2", @domain, @resource<>"1")
-               assert 1 ==
-                       :meck.num_calls(:ejabberd_sm, :route,
-                                                                                       [jid, jid,
-                                                                                        {:broadcast, {:item, {@user<>"1", @domain, ""}, :both}}])
-               jid = :jlib.make_jid(@user<>"2", @domain, @resource<>"2")
-               assert 1 ==
-                       :meck.num_calls(:ejabberd_sm, :route,
-                                                                                       [jid, jid,
-                                                                                        {:broadcast, {:item, {@user<>"1", @domain, ""}, :both}}])
-
-
-               # Ulink both user1 & user2 (returns 0 if OK)
-               assert 0 ==
-                       :ejabberd_commands.execute_command(:unlink_contacts, [@user<>"1@"<>@domain,
-                                                                                                                                                                                                                                       @user<>"2@"<>@domain])
-               assert [] ==
-                       :ejabberd_commands.execute_command(:get_roster, [@user<>"1", @domain], :admin)
-
-               assert [] ==
-                       :ejabberd_commands.execute_command(:get_roster, [@user<>"2", @domain], :admin)
-
-               # Check that the item roster user1 was pushed with subscription
-               # 'none' to the 2 user2 online ressources
-               jid = :jlib.make_jid(@user<>"2", @domain, @resource<>"1")
-               assert 1 ==
-                       :meck.num_calls(:ejabberd_sm, :route,
-                                                                                       [jid, jid,
-                                                                                        {:broadcast, {:item, {@user<>"1", @domain, ""}, :none}}])
-               jid = :jlib.make_jid(@user<>"2", @domain, @resource<>"2")
-               assert 1 ==
-                       :meck.num_calls(:ejabberd_sm, :route,
-                                                                                       [jid, jid,
-                                                                                        {:broadcast, {:item, {@user<>"1", @domain, ""}, :none}}])
-
-               # Check that nothing else was pushed to user2 resources
-               jid = jid(user: @user<>"2", server: @domain, resource: :_,
-                                                       luser: @user<>"2", lserver: @domain, lresource: :_)
-               assert 4 ==
-                       :meck.num_calls(:ejabberd_sm, :route,
-                                                                                       [jid, jid,
-                                                                                        {:broadcast, {:item, :_, :_}}])
-
-               # Check nothing was pushed to user1
-               jid = jid(user: @user<>"1", server: @domain, resource: :_,
-                                                       luser: @user<>"1", lserver: @domain, lresource: :_)
-               refute :meck.called(:ejabberd_sm, :route,
-                                                                                               [jid, jid,
-                                                                                                {:broadcast, {:item, :_, :_}}])
-
-               assert :meck.validate :ejabberd_sm
-               assert :meck.validate :ejabberd_auth
-
-       end
-
-
-
-
-       test "add_contacts and delete_contacts work" do
-               # Create user, user1 & user2
-               # Connect user & user1
-               # Add user0, user1 & user2 to user's roster
-               # Remove user0, user1 & user2 from user's roster
-
-               # user doesn't exists yet, command must fail
-               assert 404 ==
-                       :ejabberd_commands.execute_command(:add_contacts, [@user, @domain,
-                                                                                                                                                                                                                                [{@user<>"1"<>@domain,
-                                                                                                                                                                                                                                        "group1",
-                                                                                                                                                                                                                                        "nick1"},
-                                                                                                                                                                                                                                       {@user<>"2"<>@domain,
-                                                                                                                                                                                                                                        "group2",
-                                                                                                                                                                                                                                        "nick2"}]
-                                                                                                                                                                                                                                ])
-
-               EjabberdAuthMock.create_user @user, @domain, @password
-               EjabberdSmMock.connect_resource @user, @domain, @resource
-               EjabberdAuthMock.create_user @user<>"1", @domain, @password
-               EjabberdSmMock.connect_resource @user<>"1", @domain, @resource
-               EjabberdAuthMock.create_user @user<>"2", @domain, @password
-
-               # Add user1 & user2 in user's roster. Try also to add user0 that
-               # doesn't exists. Command is supposed to return number of added items.
-               assert 2 ==
-                       :ejabberd_commands.execute_command(:add_contacts, [@user, @domain,
-                                                                                                                                                                                                                                [{@user<>"0@"<>@domain,
-                                                                                                                                                                                                                                        "group0",
-                                                                                                                                                                                                                                        "nick0"},
-                                                                                                                                                                                                                                       {@user<>"1@"<>@domain,
-                                                                                                                                                                                                                                        "group1",
-                                                                                                                                                                                                                                        "nick1"},
-                                                                                                                                                                                                                                       {@user<>"2@"<>@domain,
-                                                                                                                                                                                                                                        "group2",
-                                                                                                                                                                                                                                        "nick2"}]
-                                                                                                                                                                                                                               ])
-               # Check that user1 & user2 are the only items in user's roster
-               result = ModRosterMock.get_roster(@user, @domain)
-               assert 2 == length result
-               opts1 = %{nick: "nick1", groups:  ["group1"], subs: :both,
-                                                ask: :none, askmessage: ""}
-               assert Enum.member?(result, {{@user, @domain, @user<>"1@"<>@domain}, opts1})
-               opts2 = %{nick: "nick2", groups:  ["group2"], subs: :both,
-                                                ask: :none, askmessage: ""}
-               assert Enum.member?(result, {{@user, @domain, @user<>"2@"<>@domain}, opts2})
-
-               # Check that user is the only item in user1's roster
-               assert [{{@user<>"1", @domain, @user<>"@"<>@domain}, %{opts1|:nick => ""}}] ==
-                       ModRosterMock.get_roster(@user<>"1", @domain)
-
-               # Check that user is the only item in user2's roster
-               assert [{{@user<>"2", @domain, @user<>"@"<>@domain}, %{opts2|:nick => ""}}] ==
-                       ModRosterMock.get_roster(@user<>"2", @domain)
-
-
-               # Check that the roster items user1 & user2 were pushed with subscription
-               # 'both' to the user online ressource
-               jid = :jlib.make_jid(@user, @domain, @resource)
-               assert 1 ==
-                       :meck.num_calls(:ejabberd_sm, :route,
-                                                                                       [jid, jid,
-                                                                                        {:broadcast, {:item, {@user<>"1", @domain, ""}, :both}}])
-               assert 1 ==
-                       :meck.num_calls(:ejabberd_sm, :route,
-                                                                                       [jid, jid,
-                                                                                        {:broadcast, {:item, {@user<>"2", @domain, ""}, :both}}])
-
-               # Check that the roster item user was pushed with subscription
-               # 'both' to the user1 online ressource
-               jid = :jlib.make_jid(@user<>"1", @domain, @resource)
-               assert 1 ==
-                       :meck.num_calls(:ejabberd_sm, :route,
-                                                                                       [jid, jid,
-                                                                                        {:broadcast, {:item, {@user, @domain, ""}, :both}}])
-
-               # Check that nothing else was pushed to online resources
-               assert 3 ==
-                       :meck.num_calls(:ejabberd_sm, :route,
-                                                                                       [:_, :_,
-                                                                                        {:broadcast, {:item, :_, :_}}])
-
-               # Remove user1 & user2 from user's roster. Try also to remove
-               # user0 that doesn't exists. Command is supposed to return number
-               # of removed items.
-               assert 2 ==
-                       :ejabberd_commands.execute_command(:remove_contacts, [@user, @domain,
-                                                                                                                                                                                                                                               [@user<>"0@"<>@domain,
-                                                                                                                                                                                                                                                @user<>"1@"<>@domain,
-                                                                                                                                                                                                                                                @user<>"2@"<>@domain]
-                                                                                                                                                                                                                                        ])
-               # Check that roster of user, user1 & user2 are empty
-               assert [] == ModRosterMock.get_roster(@user, @domain)
-               assert [] == ModRosterMock.get_roster(@user<>"1", @domain)
-               assert [] == ModRosterMock.get_roster(@user<>"2", @domain)
-
-               # Check that the roster items user1 & user2 were pushed with subscription
-               # 'none' to the user online ressource
-               jid = :jlib.make_jid(@user, @domain, @resource)
-               assert 1 ==
-                       :meck.num_calls(:ejabberd_sm, :route,
-                                                                                       [jid, jid,
-                                                                                        {:broadcast, {:item, {@user<>"1", @domain, ""}, :none}}])
-               assert 1 ==
-                       :meck.num_calls(:ejabberd_sm, :route,
-                                                                                       [jid, jid,
-                                                                                        {:broadcast, {:item, {@user<>"2", @domain, ""}, :none}}])
-
-               # Check that the roster item user was pushed with subscription
-               # 'none' to the user1 online ressource
-               jid = :jlib.make_jid(@user<>"1", @domain, @resource)
-               assert 1 ==
-                       :meck.num_calls(:ejabberd_sm, :route,
-                                                                                       [jid, jid,
-                                                                                        {:broadcast, {:item, {@user, @domain, ""}, :none}}])
-
-               # Check that nothing else was pushed to online resources
-               assert 6 ==
-                       :meck.num_calls(:ejabberd_sm, :route,
-                                                                                       [:_, :_,
-                                                                                        {:broadcast, {:item, :_, :_}}])
-
-               assert :meck.validate :ejabberd_sm
-               assert :meck.validate :ejabberd_auth
-       end
-
-
-       test "update_roster works" do
-               # user doesn't exists yet, command must fail
-               result =
-                       :ejabberd_commands.execute_command(:update_roster,
-                                                                                                                                                                [@user, @domain,
-                                                                                                                                                                       [{@user<>"1"<>@domain,
-                                                                                                                                                                               "group1",
-                                                                                                                                                                               "nick1"},
-                                                                                                                                                                        {@user<>"2"<>@domain,
-                                                                                                                                                                               "group2",
-                                                                                                                                                                               "nick2"}],
-                                                                                                                                                                       []
-                                                                                                                                                                ])
-               assert :invalid_user == elem(result, 0)
-
-               EjabberdAuthMock.create_user @user, @domain, @password
-               EjabberdSmMock.connect_resource @user, @domain, @resource
-               EjabberdAuthMock.create_user @user<>"1", @domain, @password
-               EjabberdSmMock.connect_resource @user<>"1", @domain, @resource
-               EjabberdAuthMock.create_user @user<>"2", @domain, @password
-               EjabberdAuthMock.create_user @user<>"3", @domain, @password
-
-               assert :ok ==
-                       :ejabberd_commands.execute_command(:update_roster,
-                                                                                                                                                                [@user, @domain,
-                                                                                                                                                                       [{[{"username", @user<>"1"},
-                                                                                                                                                                                {"nick",       "nick1"}]},
-                                                                                                                                                                        {[{"username", @user<>"2"},
-                                                                                                                                                                                {"nick",       "nick2"},
-                                                                                                                                                                                {"subscription", "from"}]}],
-                                                                                                                                                                        []])
-
-
-               # Check that user1 & user2 are the only items in user's roster
-               result = ModRosterMock.get_roster(@user, @domain)
-
-               assert 2 == length result
-               opts1 = %{nick: "nick1", groups:  [""], subs: :both,
-                                                ask: :none, askmessage: ""}
-               assert Enum.member?(result, {{@user, @domain, @user<>"1@"<>@domain}, opts1})
-               opts2 = %{nick: "nick2", groups:  [""], subs: :from,
-                                                ask: :none, askmessage: ""}
-               assert Enum.member?(result, {{@user, @domain, @user<>"2@"<>@domain}, opts2})
-
-               # Check that the roster items user1 & user2 were pushed with subscription
-               # 'both' to the user online ressource
-               jid = :jlib.make_jid(@user, @domain, @resource)
-               assert 1 ==
-                       :meck.num_calls(:ejabberd_sm, :route,
-                                                                                       [jid, jid,
-                                                                                        {:broadcast, {:item, {@user<>"1", @domain, ""}, :both}}])
-               assert 1 ==
-                       :meck.num_calls(:ejabberd_sm, :route,
-                                                                                       [jid, jid,
-                                                                                        {:broadcast, {:item, {@user<>"2", @domain, ""}, :from}}])
-
-               # Check that nothing else was pushed to online resources
-               assert 2 ==
-                       :meck.num_calls(:ejabberd_sm, :route,
-                                                                                       [:_, :_,
-                                                                                        {:broadcast, {:item, :_, :_}}])
-
-               # Add user3 & remove user1
-               assert :ok ==
-                       :ejabberd_commands.execute_command(:update_roster,
-                                                                                                                                                                [@user, @domain,
-                                                                                                                                                                       [{[{"username", @user<>"3"},
-                                                                                                                                                                                {"nick",       "nick3"},
-                                                                                                                                                                                {"subscription", "to"}]}],
-                                                                                                                                                                       [{[{"username", @user<>"1"}]}]
-                                                                                                                                                                       ])
-
-               # Check that user2 & user3 are the only items in user's roster
-               result = ModRosterMock.get_roster(@user, @domain)
-               assert 2 == length result
-               opts2 = %{nick: "nick2", groups:  [""], subs: :from,
-                                                ask: :none, askmessage: ""}
-               assert Enum.member?(result, {{@user, @domain, @user<>"2@"<>@domain}, opts2})
-               opts1 = %{nick: "nick3", groups:  [""], subs: :to,
-                                                ask: :none, askmessage: ""}
-               assert Enum.member?(result, {{@user, @domain, @user<>"3@"<>@domain}, opts1})
-
-               # Check that the roster items user1 & user3 were pushed with subscription
-               # 'none' & 'to' to the user online ressource
-               jid = :jlib.make_jid(@user, @domain, @resource)
-               assert 1 ==
-                       :meck.num_calls(:ejabberd_sm, :route,
-                                                                                       [jid, jid,
-                                                                                        {:broadcast, {:item, {@user<>"1", @domain, ""}, :none}}])
-               assert 1 ==
-                       :meck.num_calls(:ejabberd_sm, :route,
-                                                                                       [jid, jid,
-                                                                                        {:broadcast, {:item, {@user<>"3", @domain, ""}, :to}}])
-
-               # Check that nothing else was pushed to online resources
-               assert 4 ==
-                       :meck.num_calls(:ejabberd_sm, :route,
-                                                                                       [:_, :_,
-                                                                                        {:broadcast, {:item, :_, :_}}])
-
-               assert :meck.validate :ejabberd_sm
-               assert :meck.validate :ejabberd_auth
-       end
-
-
 # kick_user command is defined in ejabberd_sm, move to extra?
 #      test "kick_user works" do
 #              assert 0 == :ejabberd_commands.execute_command(:num_resources,