Follow-up to #11457

The queue type argument won't always be a binary,
for example, when a virtual host is created.

As such, the validation code should accept at
least atoms in addition to binaries.

While at it, improve logging and error reporting
when DQT validation fails, and while at it,
make the definition import tests focussed on
virtual host a bit more robust.

(cherry picked from commit 1e577a8)

# Conflicts:
#	deps/rabbit/src/rabbit_vhost.erl

Author: michaelklishin

deps/rabbit/src/rabbit_amqqueue.erl

@@ -1121,6 +1121,8 @@ check_queue_type(Val, _Args) when is_binary(Val) ->
         true  -> ok;
         false -> {error, rabbit_misc:format("unsupported queue type '~ts'", [Val])}
     end;
+check_queue_type(Val, Args) when is_atom(Val) ->
+    check_queue_type(rabbit_data_coercion:to_binary(Val), Args);
 check_queue_type(_Val, _Args) ->
     {error, invalid_queue_type}.
 

deps/rabbit/src/rabbit_queue_type.erl

@@ -246,6 +246,8 @@ discover(<<"classic">>) ->
     rabbit_classic_queue;
 discover(<<"stream">>) ->
     rabbit_stream_queue;
+discover(Other) when is_atom(Other) ->
+    discover(rabbit_data_coercion:to_binary(Other));
 discover(Other) when is_binary(Other) ->
     T = rabbit_registry:binary_to_type(Other),
     {ok, Mod} = rabbit_registry:lookup_module(queue, T),

deps/rabbit/src/rabbit_vhost.erl

@@ -171,6 +171,7 @@ do_add(Name, Metadata, ActingUser) ->
     case Metadata of
         #{default_queue_type := DQT} ->
             %% check that the queue type is known
+            rabbit_log:debug("Default queue type of virtual host '~ts' is ~tp", [Name, DQT]),
             try rabbit_queue_type:discover(DQT) of
                 _ ->
                     case rabbit_queue_type:feature_flag_name(DQT) of
@@ -182,7 +183,7 @@ do_add(Name, Metadata, ActingUser) ->
                             end
                     end
             catch _:_ ->
-                      throw({error, invalid_queue_type})
+                throw({error, invalid_queue_type, DQT})
             end;
         _ ->
             ok
@@ -315,20 +316,23 @@ put_vhost(Name, Description, Tags0, Trace, Username) ->
     boolean(),
     rabbit_types:username()) ->
     'ok' | {'error', any()} | {'EXIT', any()}.
-put_vhost(Name, Description, Tags0, DefaultQueueType0, Trace, Username) ->
+put_vhost(Name, Description, Tags0, DefaultQueueType, Trace, Username) ->
     Tags = case Tags0 of
       undefined   -> <<"">>;
       null        -> <<"">>;
       "undefined" -> <<"">>;
       "null"      -> <<"">>;
       Other       -> Other
     end,
+<<<<<<< HEAD
     DefaultQueueType = case DefaultQueueType0 of
         <<"undefined">> -> undefined;
         _ -> DefaultQueueType0
     end,
+=======
+>>>>>>> 1e577a82fc (Follow-up to #11457)
     ParsedTags = parse_tags(Tags),
-    rabbit_log:debug("Parsed tags ~tp to ~tp", [Tags, ParsedTags]),
+    rabbit_log:debug("Parsed virtual host tags ~tp to ~tp", [Tags, ParsedTags]),
     Result = case exists(Name) of
                  true  ->
                      update(Name, Description, ParsedTags, DefaultQueueType, Username);

deps/rabbit/test/definition_import_SUITE.erl

@@ -211,8 +211,9 @@ import_case11(Config) -> import_file_case(Config, "case11").
 import_case12(Config) -> import_invalid_file_case(Config, "failing_case12").
 
 import_case13(Config) ->
-    import_file_case(Config, "case13"),
     VHost = <<"/">>,
+    delete_vhost(Config, VHost),
+    import_file_case(Config, "case13"),
     QueueName = <<"definitions.import.case13.qq.1">>,
     QueueIsImported =
     fun () ->
@@ -231,8 +232,9 @@ import_case13(Config) ->
                  amqqueue:get_arguments(Q)).
 
 import_case13a(Config) ->
-    import_file_case(Config, "case13"),
     VHost = <<"/">>,
+    delete_vhost(Config, VHost),
+    import_file_case(Config, "case13"),
     QueueName = <<"definitions.import.case13.qq.1">>,
     QueueIsImported =
     fun () ->
@@ -254,8 +256,9 @@ import_case14(Config) -> import_file_case(Config, "case14").
 import_case15(Config) -> import_file_case(Config, "case15").
 %% contains a virtual host with tags
 import_case16(Config) ->
-    import_file_case(Config, "case16"),
     VHost = <<"tagged">>,
+    delete_vhost(Config, VHost),
+    import_file_case(Config, "case16"),
     VHostIsImported =
     fun () ->
             case vhost_lookup(Config, VHost) of
@@ -516,3 +519,6 @@ vhost_lookup(Config, VHost) ->
 
 user_lookup(Config, User) ->
     rabbit_ct_broker_helpers:rpc(Config, 0, rabbit_auth_backend_internal, lookup_user, [User]).
+
+delete_vhost(Config, VHost) ->
+    rabbit_ct_broker_helpers:rpc(Config, 0, rabbit_vhost, delete, [VHost, <<"CT tests">>]).