All sorts of improvements to the queue lease code - simplifications and removing dead code

3 files changed, 29 insertions, 59 deletions

diff --git a/src/rabbit_amqqueue.erl b/src/rabbit_amqqueue.erl
index c12c066b8e..3dcb34a194 100644
--- a/src/rabbit_amqqueue.erl
+++ b/src/rabbit_amqqueue.erl
@@ -184,20 +184,6 @@ recover_durable_queues(DurableQueues) ->
     Qs = [start_queue_process(Q) || Q <- DurableQueues],
     [Q || Q <- Qs, gen_server2:call(Q#amqqueue.pid, {init, true}) == Q].
 
-handle_error(QueueName, expires_not_of_type_long) ->
-    rabbit_misc:protocol_error(
-      precondition_failed,
-      "~s: Argument x-expires must be of type long.",
-      [rabbit_misc:rs(QueueName)]);
-handle_error(QueueName, expires_zero_or_less) ->
-    rabbit_misc:protocol_error(
-      precondition_failed,
-      "~s: Argument x-expires must be more than zero.",
-      [rabbit_misc:rs(QueueName)]);
-handle_error(QueueName, Error) ->
-    rabbit_misc:protocol_error(
-      internal_error, "Queue ~s: ~w", [rabbit_misc:rs(QueueName), Error]).
-
 declare(QueueName, Durable, AutoDelete, Args, Owner) ->
     Q = start_queue_process(#amqqueue{name = QueueName,
                                       durable = Durable,
@@ -206,9 +192,8 @@ declare(QueueName, Durable, AutoDelete, Args, Owner) ->
                                       exclusive_owner = Owner,
                                       pid = none}),
     case gen_server2:call(Q#amqqueue.pid, {init, false}) of
-        {error, Error} -> handle_error(QueueName, Error);
-        not_found      -> rabbit_misc:not_found(QueueName);
-        Q1             -> Q1
+        not_found -> rabbit_misc:not_found(QueueName);
+        Q1        -> Q1
     end.
 
 internal_declare(Q = #amqqueue{name = QueueName}, Recover) ->
@@ -245,7 +230,9 @@ store_queue(Q = #amqqueue{durable = false}) ->
 start_queue_process(Q) ->
     case rabbit_amqqueue_sup:start_child([Q]) of
         {ok, Pid}      -> Q#amqqueue{pid = Pid};
-        {error, Error} -> handle_error(Q#amqqueue.name, Error)
+        {error, Error} -> rabbit_misc:protocol_error(
+                            internal_error, "Queue ~s: ~w",
+                            [rabbit_misc:rs(Q#amqqueue.name), Error])
     end.
 
 add_default_binding(#amqqueue{name = QueueName}) ->
@@ -269,10 +256,10 @@ with(Name, F) ->
 with_or_die(Name, F) ->
     with(Name, F, fun () -> rabbit_misc:not_found(Name) end).
 
-assert_equivalence(#amqqueue{name = Name,
-                             durable = Durable,
+assert_equivalence(#amqqueue{name        = Name,
+                             durable     = Durable,
                              auto_delete = AutoDelete,
-                             arguments = Args1} = Q,
+                             arguments   = Args1} = Q,
                    Durable, AutoDelete, Args, Owner) ->
     check_argument_equivalent(Args1, Args, <<"x-expires">>, Name),
     check_exclusive_access(Q, Owner, strict);
@@ -299,22 +286,15 @@ with_exclusive_access_or_die(Name, ReaderPid, F) ->
                 fun (Q) -> check_exclusive_access(Q, ReaderPid), F(Q) end).
 
 check_argument_equivalent(Prev, Now, Key, QueueName) ->
-    {AType, AVal} = rabbit_misc:table_lookup(Now, Key),
-    {BType, BVal} = rabbit_misc:table_lookup(Prev, Key),
-    if AType =:= BType
-             -> ok;
-        true -> rabbit_misc:protocol_error(
-                  precondition_failed,
-                  "argument types for ~s not equivalent: ~s=~w (was ~w)",
-                  [rabbit_misc:rs(QueueName), Key, AType, BType])
-    end,
-
-    if AVal == BVal
-             -> ok;
-        true -> rabbit_misc:protocol_error(
+    case {rabbit_misc:table_lookup(Now, Key),
+          rabbit_misc:table_lookup(Prev, Key)} of
+        {Same, Same} ->
+            ok;
+        {New, Old} ->
+            rabbit_misc:protocol_error(
                   precondition_failed,
                   "arguments for ~s not equivalent: ~s=~w (was ~w)",
-                  [rabbit_misc:rs(QueueName), Key, AVal, BVal])
+                  [rabbit_misc:rs(QueueName), Key, New, Old])
     end.
 
 list(VHostPath) ->
diff --git a/src/rabbit_amqqueue_process.erl b/src/rabbit_amqqueue_process.erl
index 834176e1fb..1bd82abda9 100644
--- a/src/rabbit_amqqueue_process.erl
+++ b/src/rabbit_amqqueue_process.erl
@@ -100,20 +100,20 @@ info_keys() -> ?INFO_KEYS.
 
 -define(EXPIRES_TYPE, long).
 
-check_argument_expires(?EXPIRES_TYPE, Expires) when not is_integer(Expires) ->
+check_argument_expires({?EXPIRES_TYPE, Expires}) when not is_integer(Expires) ->
     {error, expires_not_of_type_long};
-check_argument_expires(?EXPIRES_TYPE, Expires) when Expires =< 0 ->
+check_argument_expires({?EXPIRES_TYPE, Expires}) when Expires =< 0 ->
     {error, expires_zero_or_less};
-check_argument_expires(undefined, undefined) ->
-    {ok, undefined};
-check_argument_expires(?EXPIRES_TYPE, Expires) ->
+check_argument_expires({?EXPIRES_TYPE, Expires}) ->
     {ok, Expires};
-check_argument_expires(_, _) ->
+check_argument_expires(undefined) ->
+    {ok, undefined};
+check_argument_expires(_) ->
     {error, expires_not_of_type_long}.
 
 init_expires(State = #q{q = #amqqueue{arguments = Arguments}}) ->
-    {Type, Expires} = rabbit_misc:table_lookup(Arguments, <<"x-expires">>),
-    case check_argument_expires(Type, Expires) of
+    case check_argument_expires(
+           rabbit_misc:table_lookup(Arguments, <<"x-expires">>)) of
         {error, Error} -> {error, Error};
         {ok, Expires}  -> start_expiry_timer(State, Expires)
     end.
diff --git a/src/rabbit_misc.erl b/src/rabbit_misc.erl
index a0061b1201..5d33c124e2 100644
--- a/src/rabbit_misc.erl
+++ b/src/rabbit_misc.erl
@@ -41,7 +41,7 @@
 -export([not_found/1]).
 -export([get_config/1, get_config/2, set_config/2]).
 -export([dirty_read/1]).
--export([table_lookup/2, table_lookup/3]).
+-export([table_lookup/2]).
 -export([r/3, r/2, r_arg/4, rs/1]).
 -export([enable_cover/0, report_cover/0]).
 -export([enable_cover/1, report_cover/1]).
@@ -106,11 +106,7 @@
         ({atom(), any()}) -> rabbit_types:ok_or_error2(any(), 'not_found')).
 -spec(table_lookup/2 ::
         (rabbit_framing:amqp_table(), binary())
-         -> {undefined, undefined} | {rabbit_framing:amqp_field_type(), any()}).
--spec(table_lookup/3 ::
-        (rabbit_framing:amqp_table(), binary(),
-         rabbit_framing:amqp_field_type())
-         -> undefined | any()).
+         -> 'undefined' | {rabbit_framing:amqp_field_type(), any()}).
 -spec(r/2 :: (rabbit_types:vhost(), K)
              -> rabbit_types:r3(rabbit_types:vhost(), K, '_')
                     when is_subtype(K, atom())).
@@ -236,16 +232,10 @@ dirty_read(ReadSpec) ->
         []       -> {error, not_found}
     end.
 
-table_lookup(Table, Key, Type) ->
-    case lists:keysearch(Key, 1, Table) of
-        {value, {_, Type, ValueBin}} -> ValueBin;
-        _                            -> undefined
-    end.
-
 table_lookup(Table, Key) ->
     case lists:keysearch(Key, 1, Table) of
         {value, {_, TypeBin, ValueBin}} -> {TypeBin, ValueBin};
-        false                           -> {undefined, undefined}
+        false                           -> undefined
     end.
 
 r(#resource{virtual_host = VHostPath}, Kind, Name)
@@ -260,9 +250,9 @@ r(VHostPath, Kind) when is_binary(VHostPath) ->
 r_arg(#resource{virtual_host = VHostPath}, Kind, Table, Key) ->
     r_arg(VHostPath, Kind, Table, Key);
 r_arg(VHostPath, Kind, Table, Key) ->
-    case table_lookup(Table, Key, longstr) of
-        undefined -> undefined;
-        NameBin   -> r(VHostPath, Kind, NameBin)
+    case table_lookup(Table, Key) of
+        {longstr, NameBin} -> r(VHostPath, Kind, NameBin);
+        undefined          -> undefined
     end.
 
 rs(#resource{virtual_host = VHostPath, kind = Kind, name = Name}) ->