Merge pull request #11528 from rabbitmq/mk-fix-virtual-host-creation-…

…post-11457 Follow-up to #11457
rabbitmq · Jun 22, 2024 · 7335aaa · 7335aaa
2 parents 4b4c0fe + 1e577a8
commit 7335aaa
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 7 deletions.
diff --git a/deps/rabbit/src/rabbit_amqqueue.erl b/deps/rabbit/src/rabbit_amqqueue.erl
@@ -1100,6 +1100,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}.
 

diff --git a/deps/rabbit/src/rabbit_queue_type.erl b/deps/rabbit/src/rabbit_queue_type.erl
@@ -260,6 +260,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),

diff --git a/deps/rabbit/src/rabbit_vhost.erl b/deps/rabbit/src/rabbit_vhost.erl
@@ -167,6 +167,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
@@ -178,7 +179,7 @@ do_add(Name, Metadata, ActingUser) ->
                             end
                     end
             catch _:_ ->
-                      throw({error, invalid_queue_type})
+                throw({error, invalid_queue_type, DQT})
             end;
         _ ->
             ok
@@ -301,17 +302,16 @@ delete(VHost, ActingUser) ->
     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,
-    DefaultQueueType = rabbit_data_coercion:to_atom(DefaultQueueType0),
     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);

diff --git a/deps/rabbit/test/definition_import_SUITE.erl b/deps/rabbit/test/definition_import_SUITE.erl
@@ -210,8 +210,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 () ->
@@ -230,8 +231,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 () ->
@@ -253,8 +255,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
@@ -493,3 +496,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">>]).