Skip to content

Commit 9393db1

Browse files
michaelklishinmichaelklishin
authored
Merge pull request #12049 from rabbitmq/mergify/bp/v3.13.x/pr-12048
Shovel management: tests and simplification (backport #12047) (backport #12048)
2 parents 701a733 + 12a7f34 commit 9393db1

File tree

5 files changed

+94
-30
lines changed

5 files changed

+94
-30
lines changed

‎deps/rabbitmq_management/src/rabbit_mgmt_wm_definitions.erl

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,7 @@ vhost_definitions(ReqData, VHost, Context) ->
107107
export_binding(B, QNames)],
108108
{ok, Vsn} = application:get_key(rabbit, vsn),
109109
Parameters = [strip_vhost(
110-
rabbit_mgmt_format:parameter(
111-
rabbit_mgmt_wm_parameters:fix_shovel_publish_properties(P)))
110+
rabbit_mgmt_format:parameter(P))
112111
|| P <- rabbit_runtime_parameters:list(VHost)],
113112
rabbit_mgmt_util:reply(
114113
[{rabbit_version, rabbit_data_coercion:to_binary(Vsn)}] ++

‎deps/rabbitmq_management/src/rabbit_mgmt_wm_parameter.erl

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,7 @@ resource_exists(ReqData, Context) ->
4040
end, ReqData, Context}.
4141

4242
to_json(ReqData, Context) ->
43-
rabbit_mgmt_util:reply(rabbit_mgmt_format:parameter(
44-
rabbit_mgmt_wm_parameters:fix_shovel_publish_properties(parameter(ReqData))),
43+
rabbit_mgmt_util:reply(rabbit_mgmt_format:parameter(parameter(ReqData)),
4544
ReqData, Context).
4645

4746
accept_content(ReqData0, Context = #context{user = User}) ->

‎deps/rabbitmq_management/src/rabbit_mgmt_wm_parameters.erl

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
-export([init/2, to_json/2, content_types_provided/2, is_authorized/2,
1111
resource_exists/2, basic/1]).
12-
-export([fix_shovel_publish_properties/1]).
1312
-export([variances/2]).
1413

1514
-include_lib("rabbitmq_management_agent/include/rabbit_mgmt_records.hrl").
@@ -42,24 +41,6 @@ is_authorized(ReqData, Context) ->
4241

4342
%%--------------------------------------------------------------------
4443

45-
%% Hackish fix to make sure we return a JSON object instead of an empty list
46-
%% when the publish-properties value is empty.
47-
fix_shovel_publish_properties(P) ->
48-
case lists:keyfind(component, 1, P) of
49-
{_, <<"shovel">>} ->
50-
case lists:keytake(value, 1, P) of
51-
{value, {_, Values}, P2} ->
52-
case lists:keytake(<<"publish-properties">>, 1, Values) of
53-
{_, {_, []}, Values2} ->
54-
P2 ++ [{value, Values2 ++ [{<<"publish-properties">>, empty_struct}]}];
55-
_ ->
56-
P
57-
end;
58-
_ -> P
59-
end;
60-
_ -> P
61-
end.
62-
6344
basic(ReqData) ->
6445
Raw = case rabbit_mgmt_util:id(component, ReqData) of
6546
none -> rabbit_runtime_parameters:list();
@@ -73,5 +54,5 @@ basic(ReqData) ->
7354
end,
7455
case Raw of
7556
not_found -> not_found;
76-
_ -> [rabbit_mgmt_format:parameter(fix_shovel_publish_properties(P)) || P <- Raw]
57+
_ -> [rabbit_mgmt_format:parameter(P) || P <- Raw]
7758
end.

‎deps/rabbitmq_shovel_management/src/rabbit_shovel_mgmt_shovel.erl

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,7 @@ resource_exists(ReqData, Context) ->
6666

6767
to_json(ReqData, Context) ->
6868
Shovel = parameter(ReqData),
69-
rabbit_mgmt_util:reply(rabbit_mgmt_format:parameter(
70-
rabbit_mgmt_wm_parameters:fix_shovel_publish_properties(Shovel)),
69+
rabbit_mgmt_util:reply(rabbit_mgmt_format:parameter(Shovel),
7170
ReqData, Context).
7271

7372
is_authorized(ReqData, Context) ->

‎deps/rabbitmq_shovel_management/test/http_SUITE.erl

Lines changed: 90 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ groups() ->
2929
{dynamic_shovels, [], [
3030
start_and_list_a_dynamic_amqp10_shovel,
3131
start_and_get_a_dynamic_amqp10_shovel,
32+
start_and_get_a_dynamic_amqp091_shovel_with_publish_properties,
33+
start_and_get_a_dynamic_amqp091_shovel_with_missing_publish_properties,
34+
start_and_get_a_dynamic_amqp091_shovel_with_empty_publish_properties,
3235
create_and_delete_a_dynamic_shovel_that_successfully_connects,
3336
create_and_delete_a_dynamic_shovel_that_fails_to_connect
3437
]},
@@ -127,7 +130,7 @@ start_inets(Config) ->
127130

128131
start_and_list_a_dynamic_amqp10_shovel(Config) ->
129132
remove_all_dynamic_shovels(Config, <<"/">>),
130-
Name = <<"dynamic-amqp10-await-startup-1">>,
133+
Name = rabbit_data_coercion:to_binary(?FUNCTION_NAME),
131134
ID = {<<"/">>, Name},
132135
await_shovel_removed(Config, ID),
133136

@@ -144,15 +147,15 @@ start_and_list_a_dynamic_amqp10_shovel(Config) ->
144147

145148
start_and_get_a_dynamic_amqp10_shovel(Config) ->
146149
remove_all_dynamic_shovels(Config, <<"/">>),
147-
Name = <<"dynamic-amqp10-get-shovel-1">>,
150+
Name = rabbit_data_coercion:to_binary(?FUNCTION_NAME),
148151
ID = {<<"/">>, Name},
149152
await_shovel_removed(Config, ID),
150153

151154
declare_shovel(Config, Name),
152155
await_shovel_startup(Config, ID),
153156
Sh = get_shovel(Config, Name),
154157
?assertEqual(Name, maps:get(name, Sh)),
155-
delete_shovel(Config, <<"dynamic-amqp10-await-startup-1">>),
158+
delete_shovel(Config, Name),
156159

157160
ok.
158161

@@ -167,6 +170,48 @@ start_and_get_a_dynamic_amqp10_shovel(Config) ->
167170
vhost := <<"v">>,
168171
type := <<"dynamic">>}).
169172

173+
start_and_get_a_dynamic_amqp091_shovel_with_publish_properties(Config) ->
174+
remove_all_dynamic_shovels(Config, <<"/">>),
175+
Name = rabbit_data_coercion:to_binary(?FUNCTION_NAME),
176+
ID = {<<"/">>, Name},
177+
await_shovel_removed(Config, ID),
178+
179+
declare_amqp091_shovel_with_publish_properties(Config, Name),
180+
await_shovel_startup(Config, ID),
181+
Sh = get_shovel(Config, Name),
182+
?assertEqual(Name, maps:get(name, Sh)),
183+
delete_shovel(Config, Name),
184+
185+
ok.
186+
187+
start_and_get_a_dynamic_amqp091_shovel_with_missing_publish_properties(Config) ->
188+
remove_all_dynamic_shovels(Config, <<"/">>),
189+
Name = rabbit_data_coercion:to_binary(?FUNCTION_NAME),
190+
ID = {<<"/">>, Name},
191+
await_shovel_removed(Config, ID),
192+
193+
declare_amqp091_shovel(Config, Name),
194+
await_shovel_startup(Config, ID),
195+
Sh = get_shovel(Config, Name),
196+
?assertEqual(Name, maps:get(name, Sh)),
197+
delete_shovel(Config, Name),
198+
199+
ok.
200+
201+
start_and_get_a_dynamic_amqp091_shovel_with_empty_publish_properties(Config) ->
202+
remove_all_dynamic_shovels(Config, <<"/">>),
203+
Name = rabbit_data_coercion:to_binary(?FUNCTION_NAME),
204+
ID = {<<"/">>, Name},
205+
await_shovel_removed(Config, ID),
206+
207+
declare_amqp091_shovel_with_publish_properties(Config, Name, #{}),
208+
await_shovel_startup(Config, ID),
209+
Sh = get_shovel(Config, Name),
210+
?assertEqual(Name, maps:get(name, Sh)),
211+
delete_shovel(Config, Name),
212+
213+
ok.
214+
170215
start_static_shovels(Config) ->
171216
http_put(Config, "/users/admin",
172217
#{password => <<"admin">>, tags => <<"administrator">>}, ?CREATED),
@@ -366,7 +411,48 @@ declare_shovel(Config, Name) ->
366411
'dest-address' => <<"test2">>,
367412
'dest-properties' => #{},
368413
'dest-application-properties' => #{},
369-
'dest-message-annotations' => #{}}
414+
'dest-message-annotations' => #{}
415+
}
416+
}, ?CREATED).
417+
418+
declare_amqp091_shovel(Config, Name) ->
419+
Port = integer_to_binary(
420+
rabbit_ct_broker_helpers:get_node_config(Config, 0, tcp_port_amqp)),
421+
http_put(Config, io_lib:format("/parameters/shovel/%2f/~ts", [Name]),
422+
#{
423+
value => #{
424+
<<"src-protocol">> => <<"amqp091">>,
425+
<<"src-uri">> => <<"amqp://localhost:", Port/binary>>,
426+
<<"src-queue">> => <<"amqp091.src.test">>,
427+
<<"src-delete-after">> => <<"never">>,
428+
<<"dest-protocol">> => <<"amqp091">>,
429+
<<"dest-uri">> => <<"amqp://localhost:", Port/binary>>,
430+
<<"dest-queue">> => <<"amqp091.dest.test">>
431+
}
432+
}, ?CREATED).
433+
434+
declare_amqp091_shovel_with_publish_properties(Config, Name) ->
435+
Props = #{
436+
<<"delivery_mode">> => 2,
437+
<<"app_id">> => <<"shovel_management:http_SUITE">>
438+
},
439+
declare_amqp091_shovel_with_publish_properties(Config, Name, Props).
440+
441+
declare_amqp091_shovel_with_publish_properties(Config, Name, Props) ->
442+
Port = integer_to_binary(
443+
rabbit_ct_broker_helpers:get_node_config(Config, 0, tcp_port_amqp)),
444+
http_put(Config, io_lib:format("/parameters/shovel/%2f/~ts", [Name]),
445+
#{
446+
value => #{
447+
<<"src-protocol">> => <<"amqp091">>,
448+
<<"src-uri">> => <<"amqp://localhost:", Port/binary>>,
449+
<<"src-queue">> => <<"amqp091.src.test">>,
450+
<<"src-delete-after">> => <<"never">>,
451+
<<"dest-protocol">> => <<"amqp091">>,
452+
<<"dest-uri">> => <<"amqp://localhost:", Port/binary>>,
453+
<<"dest-queue">> => <<"amqp091.dest.test">>,
454+
<<"dest-publish-properties">> => Props
455+
}
370456
}, ?CREATED).
371457

372458
await_shovel_startup(Config, Name) ->

0 commit comments

Comments
 (0)