diff --git a/deps/rabbit/BUILD.bazel b/deps/rabbit/BUILD.bazel index d2dc86f326..7b783934c2 100644 --- a/deps/rabbit/BUILD.bazel +++ b/deps/rabbit/BUILD.bazel @@ -1043,6 +1043,9 @@ rabbitmq_integration_suite( name = "vhost_SUITE", size = "medium", flaky = True, + additional_srcs = [ + "test/test_rabbit_event_handler.erl", + ], ) rabbitmq_suite( diff --git a/deps/rabbit/src/rabbit_db_vhost.erl b/deps/rabbit/src/rabbit_db_vhost.erl index 1d35e22c30..d21826ed44 100644 --- a/deps/rabbit/src/rabbit_db_vhost.erl +++ b/deps/rabbit/src/rabbit_db_vhost.erl @@ -132,7 +132,7 @@ merge_metadata_in_mnesia_tx(VHostName, Metadata) -> set_tags(VHostName, Tags) when is_binary(VHostName) andalso is_list(Tags) -> - ConvertedTags = [rabbit_data_coercion:to_atom(Tag) || Tag <- Tags], + ConvertedTags = lists:usort([rabbit_data_coercion:to_atom(Tag) || Tag <- Tags]), rabbit_db:run( #{mnesia => fun() -> set_tags_in_mnesia(VHostName, ConvertedTags) end}). diff --git a/deps/rabbit/src/rabbit_vhost.erl b/deps/rabbit/src/rabbit_vhost.erl index bb9f834a05..f0f2afdd2c 100644 --- a/deps/rabbit/src/rabbit_vhost.erl +++ b/deps/rabbit/src/rabbit_vhost.erl @@ -518,15 +518,36 @@ assert(VHostName) -> false -> throw({error, {no_such_vhost, VHostName}}) end. +are_different0([], []) -> + false; +are_different0([], [_ | _]) -> + true; +are_different0([_ | _], []) -> + true; +are_different0([E], [E]) -> + false; +are_different0([E | R1], [E | R2]) -> + are_different0(R1, R2); +are_different0(_, _) -> + true. + +are_different(L1, L2) -> + are_different0(lists:usort(L1), lists:usort(L2)). + -spec update_tags(vhost:name(), [vhost_tag()], rabbit_types:username()) -> vhost:vhost(). update_tags(VHostName, Tags, ActingUser) -> try + CurrentTags = case rabbit_db_vhost:get(VHostName) of + undefined -> []; + V -> vhost:get_tags(V) + end, VHost = rabbit_db_vhost:set_tags(VHostName, Tags), ConvertedTags = vhost:get_tags(VHost), rabbit_log:info("Successfully set tags for virtual host '~ts' to ~tp", [VHostName, ConvertedTags]), - rabbit_event:notify(vhost_tags_set, [{name, VHostName}, - {tags, ConvertedTags}, - {user_who_performed_action, ActingUser}]), + rabbit_event:notify_if(are_different(CurrentTags, ConvertedTags), + vhost_tags_set, [{name, VHostName}, + {tags, ConvertedTags}, + {user_who_performed_action, ActingUser}]), VHost catch throw:{error, {no_such_vhost, _}} = Error -> diff --git a/deps/rabbit/test/test_rabbit_event_handler.erl b/deps/rabbit/test/test_rabbit_event_handler.erl new file mode 100644 index 0000000000..4e588ca9b0 --- /dev/null +++ b/deps/rabbit/test/test_rabbit_event_handler.erl @@ -0,0 +1,33 @@ +-module(test_rabbit_event_handler). + +-behaviour(gen_event). + +-export([okay/0]). +-export([init/1, handle_call/2, handle_event/2, handle_info/2, + terminate/2, code_change/3]). + +-include_lib("rabbit_common/include/rabbit.hrl"). + +% an exported callable func, used to allow rabbit_ct_broker_helpers +% to load this code when rpc'd +okay() -> ok. + +init([]) -> + {ok, #{events => []}}. + +handle_event(#event{} = Event, #{events := Events} = State) -> + {ok, State#{events => [Event | Events]}}; +handle_event(_, State) -> + {ok, State}. + +handle_call(events, #{events := Events} = State) -> + {ok, Events, State}. + +handle_info(_Info, State) -> + {ok, State}. + +terminate(_Arg, _State) -> + ok. + +code_change(_OldVsn, State, _Extra) -> + {ok, State}. diff --git a/deps/rabbit/test/vhost_SUITE.erl b/deps/rabbit/test/vhost_SUITE.erl index d0c1f7b838..46ab727f7d 100644 --- a/deps/rabbit/test/vhost_SUITE.erl +++ b/deps/rabbit/test/vhost_SUITE.erl @@ -28,6 +28,7 @@ groups() -> single_node_vhost_deletion_forces_connection_closure, vhost_failure_forces_connection_closure, vhost_creation_idempotency, + vhost_update_idempotency, parse_tags ], ClusterSize2Tests = [ @@ -321,6 +322,42 @@ vhost_creation_idempotency(Config) -> rabbit_ct_broker_helpers:delete_vhost(Config, VHost) end. +vhost_update_idempotency(Config) -> + VHost = <<"update-idempotency-test">>, + ActingUser = <<"acting-user">>, + try + % load the dummy event handler on the node + ok = rabbit_ct_broker_helpers:rpc(Config, 0, test_rabbit_event_handler, okay, []), + + ok = rabbit_ct_broker_helpers:rpc(Config, 0, gen_event, add_handler, + [rabbit_event, test_rabbit_event_handler, []]), + + ?assertEqual(ok, rabbit_ct_broker_helpers:add_vhost(Config, VHost)), + + ?assertMatch({vhost,VHost, [], #{tags := [private,replicate]}}, + rabbit_ct_broker_helpers:rpc(Config, 0, + rabbit_vhost, update_tags, + [VHost, [private, replicate], ActingUser])), + ?assertMatch({vhost,VHost, [], #{tags := [private,replicate]}}, + rabbit_ct_broker_helpers:rpc(Config, 0, + rabbit_vhost, update_tags, + [VHost, [replicate, private], ActingUser])), + + Events = rabbit_ct_broker_helpers:rpc(Config, 0, + gen_event, call, + [rabbit_event, test_rabbit_event_handler, events, 100]), + ct:pal(?LOW_IMPORTANCE, "Events: ~p", [lists:reverse(Events)]), + TagsSetEvents = lists:filter(fun + (#event{type = vhost_tags_set}) -> true; + (_) -> false + end, Events), + ?assertMatch([#event{}], TagsSetEvents) + after + rabbit_ct_broker_helpers:rpc(Config, 0, + gen_event, delete_handler, [rabbit_event, test_rabbit_event_handler, []]), + rabbit_ct_broker_helpers:delete_vhost(Config, VHost) + end. + vhost_is_created_with_default_limits(Config) -> VHost = <<"vhost1">>, Limits = [{<<"max-connections">>, 10}, {<<"max-queues">>, 1}],