Merge with defaultbug24157

4 files changed, 68 insertions, 45 deletions

diff --git a/src/rabbit_auth_backend_internal.erl b/src/rabbit_auth_backend_internal.erl
index 7cbd5dca..2a42ff88 100644
--- a/src/rabbit_auth_backend_internal.erl
+++ b/src/rabbit_auth_backend_internal.erl
@@ -28,7 +28,9 @@
          hash_password/1]).
 -export([set_permissions/5, clear_permissions/2,
          list_permissions/0, list_vhost_permissions/1, list_user_permissions/1,
-         list_user_vhost_permissions/2, vhost_perms_info_keys/0]).
+         list_user_vhost_permissions/2, perms_info_keys/0,
+         vhost_perms_info_keys/0, user_perms_info_keys/0,
+         user_vhost_perms_info_keys/0]).
 
 -include("rabbit_auth_backend_spec.hrl").
 
@@ -58,25 +60,23 @@
                            regexp(), regexp(), regexp()) -> 'ok').
 -spec(clear_permissions/2 :: (rabbit_types:username(), rabbit_types:vhost())
                              -> 'ok').
--spec(list_permissions/0 ::
-        () -> [{rabbit_types:username(), rabbit_types:vhost(),
-                regexp(), regexp(), regexp()}]).
+-spec(list_permissions/0 :: () -> rabbit_types:infos()).
 -spec(list_vhost_permissions/1 ::
-        (rabbit_types:vhost()) -> [{rabbit_types:username(),
-                                    regexp(), regexp(), regexp()}]).
+        (rabbit_types:vhost()) -> rabbit_types:infos()).
 -spec(list_user_permissions/1 ::
-        (rabbit_types:username()) -> [{rabbit_types:vhost(),
-                                       regexp(), regexp(), regexp()}]).
+        (rabbit_types:username()) -> rabbit_types:infos()).
 -spec(list_user_vhost_permissions/2 ::
         (rabbit_types:username(), rabbit_types:vhost())
-        -> [{regexp(), regexp(), regexp()}]).
+        -> rabbit_types:infos()).
+-spec(perms_info_keys/0 :: () -> rabbit_types:info_keys()).
 -spec(vhost_perms_info_keys/0 :: () -> rabbit_types:info_keys()).
-
+-spec(user_perms_info_keys/0 :: () -> rabbit_types:info_keys()).
+-spec(user_vhost_perms_info_keys/0 :: () -> rabbit_types:info_keys()).
 -endif.
 
 %%----------------------------------------------------------------------------
 
--define(PERMS_INFO_KEYS, [configure_perms, write_perms, read_perms]).
+-define(PERMS_INFO_KEYS, [configure, write, read]).
 
 %% Implementation of rabbit_auth_backend
 
@@ -286,35 +286,38 @@ clear_permissions(Username, VHostPath) ->
                                                 virtual_host = VHostPath}})
         end)).
 
-vhost_perms_info_keys() -> [username] ++ ?PERMS_INFO_KEYS.
+perms_info_keys()            -> [user, vhost | ?PERMS_INFO_KEYS].
+vhost_perms_info_keys()      -> [user | ?PERMS_INFO_KEYS].
+user_perms_info_keys()       -> [vhost | ?PERMS_INFO_KEYS].
+user_vhost_perms_info_keys() -> ?PERMS_INFO_KEYS.
 
 list_permissions() ->
-    [{Username, VHostPath, ConfigurePerm, WritePerm, ReadPerm} ||
-        {Username, VHostPath, ConfigurePerm, WritePerm, ReadPerm} <-
-            list_permissions(match_user_vhost('_', '_'))].
+    list_permissions(perms_info_keys(), match_user_vhost('_', '_')).
 
 list_vhost_permissions(VHostPath) ->
-    InfoKeys = vhost_perms_info_keys(),
-    [lists:zip(InfoKeys, [Username, ConfigurePerm, WritePerm, ReadPerm]) ||
-        {Username, _, ConfigurePerm, WritePerm, ReadPerm} <-
-            list_permissions(rabbit_vhost:with(
-                               VHostPath, match_user_vhost('_', VHostPath)))].
+    list_permissions(
+      vhost_perms_info_keys(),
+      rabbit_vhost:with(VHostPath, match_user_vhost('_', VHostPath))).
 
 list_user_permissions(Username) ->
-    [{VHostPath, ConfigurePerm, WritePerm, ReadPerm} ||
-        {_, VHostPath, ConfigurePerm, WritePerm, ReadPerm} <-
-            list_permissions(rabbit_misc:with_user(
-                               Username, match_user_vhost(Username, '_')))].
+    list_permissions(
+      user_perms_info_keys(),
+      rabbit_misc:with_user(Username, match_user_vhost(Username, '_'))).
 
 list_user_vhost_permissions(Username, VHostPath) ->
-    [{ConfigurePerm, WritePerm, ReadPerm} ||
-        {_, _, ConfigurePerm, WritePerm, ReadPerm} <-
-            list_permissions(rabbit_misc:with_user_and_vhost(
-                               Username, VHostPath,
-                               match_user_vhost(Username, VHostPath)))].
-
-list_permissions(QueryThunk) ->
-    [{Username, VHostPath, ConfigurePerm, WritePerm, ReadPerm} ||
+    list_permissions(
+      user_vhost_perms_info_keys(),
+      rabbit_misc:with_user_and_vhost(
+        Username, VHostPath, match_user_vhost(Username, VHostPath))).
+
+filter_props(Keys, Props) -> [T || T = {K, _} <- Props, lists:member(K, Keys)].
+
+list_permissions(Keys, QueryThunk) ->
+    [filter_props(Keys, [{user,      Username},
+                         {vhost,     VHostPath},
+                         {configure, ConfigurePerm},
+                         {write,     WritePerm},
+                         {read,      ReadPerm}]) ||
         #user_permission{user_vhost = #user_vhost{username     = Username,
                                                   virtual_host = VHostPath},
                          permission = #permission{ configure = ConfigurePerm,
diff --git a/src/rabbit_control.erl b/src/rabbit_control.erl
index 1fef76ee..355ac549 100644
--- a/src/rabbit_control.erl
+++ b/src/rabbit_control.erl
@@ -262,8 +262,9 @@ action(list_vhosts, Node, Args, _Opts, Inform) ->
 
 action(list_user_permissions, Node, Args = [_Username], _Opts, Inform) ->
     Inform("Listing permissions for user ~p", Args),
-    display_list(call(Node, {rabbit_auth_backend_internal,
-                             list_user_permissions, Args}));
+    display_info_list(call(Node, {rabbit_auth_backend_internal,
+                                  list_user_permissions, Args}),
+                      rabbit_auth_backend_internal:user_perms_info_keys());
 
 action(list_queues, Node, Args, Opts, Inform) ->
     Inform("Listing queues", []),
diff --git a/src/rabbit_router.erl b/src/rabbit_router.erl
index f6a1c92f..8f166672 100644
--- a/src/rabbit_router.erl
+++ b/src/rabbit_router.erl
@@ -84,21 +84,18 @@ match_bindings(SrcName, Match) ->
     mnesia:async_dirty(fun qlc:e/1, [Query]).
 
 match_routing_key(SrcName, [RoutingKey]) ->
-    MatchHead = #route{binding = #binding{source      = SrcName,
+    find_routes(#route{binding = #binding{source      = SrcName,
                                           destination = '$1',
                                           key         = RoutingKey,
                                           _           = '_'}},
-    mnesia:dirty_select(rabbit_route, [{MatchHead, [], ['$1']}]);
+                []);
 match_routing_key(SrcName, [_|_] = RoutingKeys) ->
-    Condition = list_to_tuple(['orelse' | [{'=:=', '$2', RKey} ||
-                                              RKey <- RoutingKeys]]),
-    MatchHead = #route{binding = #binding{source      = SrcName,
+    find_routes(#route{binding = #binding{source      = SrcName,
                                           destination = '$1',
                                           key         = '$2',
                                           _           = '_'}},
-    mnesia:dirty_select(rabbit_route, [{MatchHead, [Condition], ['$1']}]).
-
-
+                [list_to_tuple(['orelse' | [{'=:=', '$2', RKey} ||
+                                               RKey <- RoutingKeys]])]).
 
 %%--------------------------------------------------------------------
 
@@ -117,3 +114,25 @@ lookup_qpids(QNames) ->
                             []                      -> QPids
                         end
                 end, [], QNames).
+
+%% Normally we'd call mnesia:dirty_select/2 here, but that is quite
+%% expensive due to
+%%
+%% 1) general mnesia overheads (figuring out table types and
+%% locations, etc). We get away with bypassing these because we know
+%% that the table
+%% - is not the schema table
+%% - has a local ram copy
+%% - does not have any indices
+%%
+%% 2) 'fixing' of the table with ets:safe_fixtable/2, which is wholly
+%% unnecessary. According to the ets docs (and the code in erl_db.c),
+%% 'select' is safe anyway ("Functions that internally traverse over a
+%% table, like select and match, will give the same guarantee as
+%% safe_fixtable.") and, furthermore, even the lower level iterators
+%% ('first' and 'next') are safe on ordered_set tables ("Note that for
+%% tables of the ordered_set type, safe_fixtable/2 is not necessary as
+%% calls to first/1 and next/2 will always succeed."), which
+%% rabbit_route is.
+find_routes(MatchHead, Conditions) ->
+    ets:select(rabbit_route, [{MatchHead, Conditions, ['$1']}]).
diff --git a/src/rabbit_vhost.erl b/src/rabbit_vhost.erl
index 5270d80b..08d6c99a 100644
--- a/src/rabbit_vhost.erl
+++ b/src/rabbit_vhost.erl
@@ -91,9 +91,9 @@ delete(VHostPath) ->
 
 internal_delete(VHostPath) ->
     lists:foreach(
-      fun ({Username, _, _, _}) ->
-              ok = rabbit_auth_backend_internal:clear_permissions(Username,
-                                                                  VHostPath)
+      fun (Info) ->
+              ok = rabbit_auth_backend_internal:clear_permissions(
+                     proplists:get_value(user, Info), VHostPath)
       end,
       rabbit_auth_backend_internal:list_vhost_permissions(VHostPath)),
     ok = mnesia:delete({rabbit_vhost, VHostPath}),