Do not overwrite default_queue_type with undefined

Importing a definitions file with no `default_queue_type` metadata for a vhost will result in that vhosts value being set to `undefined`.

Once set to a non-`undefined` value, this PR prevents `default_queue_type` from being set back to `undefined`

Author: lukebakken

deps/rabbit/src/vhost.erl

@@ -163,10 +163,25 @@ set_metadata(VHost, Value) ->
     VHost#vhost{metadata = Value}.
 
 -spec merge_metadata(vhost(), metadata()) -> vhost().
-merge_metadata(VHost, Value) ->
-    Meta0 = get_metadata(VHost),
-    NewMeta = maps:merge(Meta0, Value),
-    VHost#vhost{metadata = NewMeta}.
+merge_metadata(VHost, NewVHostMeta) ->
+    CurrentVHostMeta = get_metadata(VHost),
+    FinalMeta =  maps:merge_with(
+                   fun metadata_merger/3, CurrentVHostMeta, NewVHostMeta),
+    VHost#vhost{metadata = FinalMeta}.
+
+%% This is the case where the existing VHost metadata has a default queue type
+%% value and the proposed value is `undefined`. We do not want the proposed
+%% value to overwrite the current value
+metadata_merger(default_queue_type, CurrentDefaultQueueType, undefined) ->
+    CurrentDefaultQueueType;
+%% This is the case where the existing VHost metadata has any default queue
+%% type value, and the proposed value is NOT `undefined`. It is OK for any
+%% proposed value to be used.
+metadata_merger(default_queue_type, _, NewVHostDefaultQueueType) ->
+    NewVHostDefaultQueueType;
+%% This is the case for all other VHost metadata keys.
+metadata_merger(_, _, NewMetadataValue) ->
+    NewMetadataValue.
 
 -spec is_tagged_with(vhost(), tag()) -> boolean().
 is_tagged_with(VHost, Tag) ->

deps/rabbit/test/vhost_SUITE.erl

@@ -33,6 +33,7 @@ groups() ->
         vhost_failure_forces_connection_closure,
         vhost_creation_idempotency,
         vhost_update_idempotency,
+        vhost_update_default_queue_type_undefined,
         vhost_deletion,
         parse_tags
     ],
@@ -42,6 +43,8 @@ groups() ->
         vhost_failure_forces_connection_closure_on_failure_node,
         node_starts_with_dead_vhosts,
         vhost_creation_idempotency,
+        vhost_update_idempotency,
+        vhost_update_default_queue_type_undefined,
         vhost_deletion
     ],
     [
@@ -300,6 +303,30 @@ vhost_update_idempotency(Config) ->
         rabbit_ct_broker_helpers:delete_vhost(Config, VHost)
     end.
 
+vhost_update_default_queue_type_undefined(Config) ->
+    VHost = <<"update-default_queue_type-with-undefined-test">>,
+    Description = <<"rmqfpas-105 test vhost">>,
+    Tags = [replicate, private],
+    DefaultQueueType = quorum,
+    Trace = false,
+    ActingUser = <<"acting-user">>,
+    try
+        ?assertMatch(ok, rabbit_ct_broker_helpers:add_vhost(Config, VHost)),
+
+        PutVhostArgs0 = [VHost, Description, Tags, DefaultQueueType, Trace, ActingUser],
+        ?assertMatch(ok,
+                     rabbit_ct_broker_helpers:rpc(Config, 0, rabbit_vhost, put_vhost, PutVhostArgs0)),
+
+        PutVhostArgs1 = [VHost, Description, Tags, undefined, Trace, ActingUser],
+        ?assertMatch(ok,
+                     rabbit_ct_broker_helpers:rpc(Config, 0, rabbit_vhost, put_vhost, PutVhostArgs1)),
+
+        V = rabbit_ct_broker_helpers:rpc(Config, 0, rabbit_vhost, lookup, [VHost]),
+        ?assertMatch(#{default_queue_type := DefaultQueueType}, vhost:get_metadata(V))
+    after
+        rabbit_ct_broker_helpers:delete_vhost(Config, VHost)
+    end.
+
 vhost_deletion(Config) ->
     VHost = <<"deletion-vhost">>,
     ActingUser = <<"acting-user">>,