replace list_{vhost_users,user_vhosts} with list_{vhost,user}_permissions

3 files changed, 71 insertions, 61 deletions

diff --git a/src/rabbit_access_control.erl b/src/rabbit_access_control.erl
index d75f1b2992..ed88263c3b 100644
--- a/src/rabbit_access_control.erl
+++ b/src/rabbit_access_control.erl
@@ -37,8 +37,9 @@
          check_vhost_access/2, check_resource_access/3]).
 -export([add_user/2, delete_user/1, change_password/2, list_users/0,
          lookup_user/1]).
--export([add_vhost/1, delete_vhost/1, list_vhosts/0, list_vhost_users/1]).
--export([list_user_vhosts/1, set_permissions/4, clear_permissions/2]).
+-export([add_vhost/1, delete_vhost/1, list_vhosts/0]).
+-export([set_permissions/4, clear_permissions/2,
+         list_vhost_permissions/1, list_user_permissions/1]).
 
 %%----------------------------------------------------------------------------
 
@@ -57,10 +58,12 @@
 -spec(add_vhost/1 :: (vhost()) -> 'ok').
 -spec(delete_vhost/1 :: (vhost()) -> 'ok').
 -spec(list_vhosts/0 :: () -> [vhost()]).
--spec(list_vhost_users/1 :: (vhost()) -> [username()]).
--spec(list_user_vhosts/1 :: (username()) -> [vhost()]).
 -spec(set_permissions/4 :: (username(), vhost(), regexp(), regexp()) -> 'ok').
 -spec(clear_permissions/2 :: (username(), vhost()) -> 'ok').
+-spec(list_vhost_permissions/1 ::
+      (vhost()) -> [{username(), regexp(), regexp()}]).
+-spec(list_user_permissions/1 ::
+      (username()) -> [{vhost(), regexp(), regexp()}]).
 
 -endif.
 
@@ -257,44 +260,16 @@ internal_delete_vhost(VHostPath) ->
                           ok = rabbit_exchange:delete(Name, false)
                   end,
                   rabbit_exchange:list(VHostPath)),
-    lists:foreach(fun (Username) ->
+    lists:foreach(fun ({Username, _, _}) ->
                           ok = clear_permissions(Username, VHostPath)
                   end,
-                  list_vhost_users(VHostPath)),
+                  list_vhost_permissions(VHostPath)),
     ok = mnesia:delete({vhost, VHostPath}),
     ok.
 
 list_vhosts() ->
     mnesia:dirty_all_keys(vhost).
 
-list_vhost_users(VHostPath) ->
-    [Username ||
-        #user_permission{user_vhost = #user_vhost{username = Username}} <-
-            %% TODO: use dirty ops instead
-            rabbit_misc:execute_mnesia_transaction(
-              rabbit_misc:with_vhost(
-                VHostPath,
-                fun () -> mnesia:match_object(
-                            #user_permission{user_vhost = #user_vhost{
-                                               username = '_',
-                                               virtual_host = VHostPath},
-                                             permission = '_'})
-                end))].
-
-list_user_vhosts(Username) ->
-    [VHostPath ||
-        #user_permission{user_vhost = #user_vhost{virtual_host = VHostPath}} <-
-            %% TODO: use dirty ops instead
-            rabbit_misc:execute_mnesia_transaction(
-              rabbit_misc:with_user(
-                Username,
-                fun () -> mnesia:match_object(
-                            #user_permission{user_vhost = #user_vhost{
-                                               username = Username,
-                                               virtual_host = '_'},
-                                             permission = '_'})
-                end))].
-
 validate_regexp(RegexpBin) ->
     Regexp = binary_to_list(RegexpBin),
     case regexp:parse(Regexp) of
@@ -317,7 +292,6 @@ set_permissions(Username, VHostPath, ConfigurationPerm, MessagingPerm) ->
                                             messaging = MessagingPerm}})
         end)).
 
-
 clear_permissions(Username, VHostPath) ->
     rabbit_misc:execute_mnesia_transaction(
       rabbit_misc:with_user_and_vhost(
@@ -328,3 +302,38 @@ clear_permissions(Username, VHostPath) ->
                                                 virtual_host = VHostPath}})
         end)).
 
+list_vhost_permissions(VHostPath) ->
+    [{Username, ConfigurationPerm, MessagingPerm} ||
+        #user_permission{user_vhost = #user_vhost{username = Username},
+                         permission = #permission{
+                          configuration = ConfigurationPerm,
+                           messaging = MessagingPerm}} <-
+            %% TODO: use dirty ops instead
+            rabbit_misc:execute_mnesia_transaction(
+              rabbit_misc:with_vhost(
+                VHostPath,
+                fun () -> mnesia:match_object(
+                            #user_permission{user_vhost = #user_vhost{
+                                               username = '_',
+                                               virtual_host = VHostPath},
+                                             permission = '_'})
+                end))].
+
+list_user_permissions(Username) ->
+    [{VHostPath, ConfigurationPerm, MessagingPerm} ||
+        #user_permission{user_vhost = #user_vhost{virtual_host = VHostPath},
+                         permission = #permission{
+                          configuration = ConfigurationPerm,
+                           messaging = MessagingPerm}} <-
+            %% TODO: use dirty ops instead
+            rabbit_misc:execute_mnesia_transaction(
+              rabbit_misc:with_user(
+                Username,
+                fun () -> mnesia:match_object(
+                            #user_permission{user_vhost = #user_vhost{
+                                               username = Username,
+                                               virtual_host = '_'},
+                                             permission = '_'})
+                end))].
+
+    
diff --git a/src/rabbit_control.erl b/src/rabbit_control.erl
index 4d771871a2..293cd79751 100644
--- a/src/rabbit_control.erl
+++ b/src/rabbit_control.erl
@@ -116,8 +116,8 @@ Available commands:
 
   set_permissions   [-p <VHostPath>] <UserName> <Regexp> <Regexp>
   clear_permissions [-p <VHostPath>] <UserName>
-  list_user_vhosts <UserName>
-  list_vhost_users <VHostPath>
+  list_permissions  [-p <VHostPath>]
+  list_user_permissions <UserName>
 
   list_queues    [-p <VHostPath>] [<QueueInfoItem> ...]
   list_exchanges [-p <VHostPath>] [<ExchangeInfoItem> ...]
@@ -223,13 +223,10 @@ action(list_vhosts, Node, [], Inform) ->
     Inform("Listing vhosts", []),
     display_list(call(Node, {rabbit_access_control, list_vhosts, []}));
 
-action(list_user_vhosts, Node, Args = [_Username], Inform) ->
-    Inform("Listing vhosts for user ~p", Args),
-    display_list(call(Node, {rabbit_access_control, list_user_vhosts, Args}));
-
-action(list_vhost_users, Node, Args = [_VHostPath], Inform) ->
-    Inform("Listing users for vhosts ~p", Args),
-    display_list(call(Node, {rabbit_access_control, list_vhost_users, Args}));
+action(list_user_permissions, Node, Args = [_Username], Inform) ->
+    Inform("Listing permissions for user ~p", Args),
+    display_list(call(Node, {rabbit_access_control, list_user_permissions,
+                             Args}));
 
 action(list_queues, Node, Args, Inform) ->
     Inform("Listing queues", []),
@@ -277,7 +274,12 @@ action(set_permissions, Node, VHost, [Username, CPerm, MPerm], Inform) ->
 
 action(clear_permissions, Node, VHost, [Username], Inform) ->
     Inform("Clearing permissions for user ~p in vhost ~p", [Username, VHost]),
-    call(Node, {rabbit_access_control, clear_permissions, [Username, VHost]}).
+    call(Node, {rabbit_access_control, clear_permissions, [Username, VHost]});
+
+action(list_permissions, Node, VHost, [], Inform) ->
+    Inform("Listing permissions in vhost ~p", [VHost]),
+    display_list(call(Node, {rabbit_access_control, list_vhost_permissions,
+                             [VHost]})).
 
 parse_vhost_flag(Args) when is_list(Args) ->
     case Args of 
@@ -299,21 +301,17 @@ default_if_empty(List, Default) when is_list(List) ->
     end.
 
 display_info_list(Results, InfoItemKeys) when is_list(Results) ->
-    lists:foreach(
-      fun (Result) ->
-              io:fwrite(
-                lists:flatten(
-                  rabbit_misc:intersperse(
-                    "\t",
-                    [format_info_item(Result, X) || X <- InfoItemKeys]))),
-              io:nl()
-      end,
-      Results),
+    lists:foreach(fun (Result) -> display_row([format_info_item(Result, X) ||
+                                                  X <- InfoItemKeys])
+                  end, Results),
     ok;
-
 display_info_list(Other, _) ->
     Other.
 
+display_row(Row) ->
+    io:fwrite(lists:flatten(rabbit_misc:intersperse("\t", Row))),
+    io:nl().
+
 format_info_item(Items, Key) ->
     {value, Info = {Key, Value}} = lists:keysearch(Key, 1, Items),
     case Info of
@@ -330,8 +328,10 @@ format_info_item(Items, Key) ->
     end.
 
 display_list(L) when is_list(L) ->
-    lists:foreach(fun (I) ->
-                          io:format("~s~n", [binary_to_list(I)])
+    lists:foreach(fun (I) when is_binary(I) ->
+                          io:format("~s~n", [url_encode(I)]);
+                      (I) when is_tuple(I) ->
+                          display_row([url_encode(V) || V <- tuple_to_list(I)])
                   end,
                   lists:sort(L)),
     ok;
diff --git a/src/rabbit_tests.erl b/src/rabbit_tests.erl
index 26398f9b46..ef390e4de6 100644
--- a/src/rabbit_tests.erl
+++ b/src/rabbit_tests.erl
@@ -448,9 +448,9 @@ test_user_management() ->
     {error, {no_such_user, _}} =
         control_action(clear_permissions, ["foo"]),
     {error, {no_such_user, _}} =
-        control_action(list_user_vhosts, ["foo"]),
+        control_action(list_user_permissions, ["foo"]),
     {error, {no_such_vhost, _}} =
-        control_action(list_vhost_users, ["/testhost"]),
+        control_action(list_permissions, ["-p", "/testhost"]),
     {error, {invalid_regexp, _, _}} =
         control_action(set_permissions, ["guest", "+foo", ".*"]),
     {error, {invalid_regexp, _, _}} =
@@ -474,7 +474,8 @@ test_user_management() ->
                                           "foo", ".*", ".*"]),
     ok = control_action(set_permissions, ["-p", "/testhost",
                                           "foo", ".*", ".*"]),
-    ok = control_action(list_user_vhosts, ["foo"]),
+    ok = control_action(list_permissions, ["-p", "/testhost"]),
+    ok = control_action(list_user_permissions, ["foo"]),
 
     %% user/vhost unmapping
     ok = control_action(clear_permissions, ["-p", "/testhost", "foo"]),