Skip to content

Commit 6e8b566

Browse files
ansdansd
committed
Deduplicate AMQP type inference
Introduce a single place in the AMQP 1.0 Erlang client that infers the AMQP 1.0 type. Erlang integers are inferred to be AMQP type `long` to avoid overflow surprises.
1 parent 44e74ce commit 6e8b566

File tree

4 files changed

+60
-78
lines changed

4 files changed

+60
-78
lines changed

‎deps/amqp10_client/src/amqp10_client_session.erl‎

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1194,33 +1194,11 @@ make_link_ref(Role, Session, Handle) ->
11941194
translate_message_annotations(MA)
11951195
when map_size(MA) > 0 ->
11961196
{map, maps:fold(fun(K, V, Acc) ->
1197-
[{sym(K), wrap_map_value(V)} | Acc]
1197+
[{sym(K), amqp10_client_types:infer(V)} | Acc]
11981198
end, [], MA)};
11991199
translate_message_annotations(_MA) ->
12001200
undefined.
12011201

1202-
wrap_map_value(true) ->
1203-
{boolean, true};
1204-
wrap_map_value(false) ->
1205-
{boolean, false};
1206-
wrap_map_value(V) when is_integer(V) ->
1207-
case V < 0 of
1208-
true ->
1209-
{int, V};
1210-
false ->
1211-
uint(V)
1212-
end;
1213-
wrap_map_value(V) when is_binary(V) ->
1214-
utf8(V);
1215-
wrap_map_value(V) when is_list(V) ->
1216-
utf8(list_to_binary(V));
1217-
wrap_map_value(V) when is_atom(V) ->
1218-
utf8(atom_to_list(V));
1219-
wrap_map_value(TaggedValue) when is_atom(element(1, TaggedValue)) ->
1220-
TaggedValue.
1221-
1222-
utf8(V) -> amqp10_client_types:utf8(V).
1223-
12241202
sym(B) when is_binary(B) -> {symbol, B};
12251203
sym(B) when is_list(B) -> {symbol, list_to_binary(B)};
12261204
sym(B) when is_atom(B) -> {symbol, atom_to_binary(B, utf8)}.

‎deps/amqp10_client/src/amqp10_client_types.erl‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
-include_lib("amqp10_common/include/amqp10_framing.hrl").
1010

1111
-export([unpack/1,
12+
infer/1,
1213
utf8/1,
1314
uint/1,
1415
make_properties/1]).
@@ -73,13 +74,32 @@
7374
properties/0]).
7475

7576

76-
unpack({_, Value}) -> Value;
77-
unpack(Value) -> Value.
78-
79-
utf8(S) when is_list(S) -> {utf8, list_to_binary(S)};
80-
utf8(B) when is_binary(B) -> {utf8, B}.
81-
82-
uint(N) -> {uint, N}.
77+
unpack({_, Value}) ->
78+
Value;
79+
unpack(Value) ->
80+
Value.
81+
82+
infer(V) when is_integer(V) ->
83+
{long, V};
84+
infer(V) when is_number(V) ->
85+
%% AMQP double and Erlang float are both 64-bit.
86+
{double, V};
87+
infer(V) when is_boolean(V) ->
88+
{boolean, V};
89+
infer(V) when is_atom(V) ->
90+
{utf8, atom_to_binary(V, utf8)};
91+
infer(TaggedValue) when is_atom(element(1, TaggedValue)) ->
92+
TaggedValue;
93+
infer(V) ->
94+
utf8(V).
95+
96+
utf8(V) when is_binary(V) ->
97+
{utf8, V};
98+
utf8(V) when is_list(V) ->
99+
{utf8, unicode:characters_to_binary(V)}.
100+
101+
uint(N) ->
102+
{uint, N}.
83103

84104
make_properties(#{properties := Props})
85105
when map_size(Props) > 0 ->

‎deps/amqp10_client/src/amqp10_msg.erl‎

Lines changed: 21 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@
3838
set_message_annotations/2
3939
]).
4040

41+
-import(amqp10_client_types, [utf8/1]).
42+
4143
-include_lib("amqp10_common/include/amqp10_framing.hrl").
4244

4345
-type opt(T) :: T | undefined.
@@ -380,65 +382,44 @@ set_application_properties(
380382
Props0, #amqp10_msg{application_properties =
381383
#'v1_0.application_properties'{content = APs0}} = Msg) ->
382384
Props = maps:fold(fun (K, V, S) ->
383-
S#{utf8(K) => wrap_ap_value(V)}
385+
S#{utf8(K) => amqp10_client_types:infer(V)}
384386
end, maps:from_list(APs0), Props0),
385387
APs = #'v1_0.application_properties'{content = maps:to_list(Props)},
386388
Msg#amqp10_msg{application_properties = APs}.
387389

388390
-spec set_delivery_annotations(#{binary() => binary() | integer() | string()},
389-
amqp10_msg()) -> amqp10_msg().
391+
amqp10_msg()) -> amqp10_msg().
390392
set_delivery_annotations(Props,
391393
#amqp10_msg{delivery_annotations = undefined} =
392394
Msg) ->
393395
Anns = #'v1_0.delivery_annotations'{content = []},
394396
set_delivery_annotations(Props,
395397
Msg#amqp10_msg{delivery_annotations = Anns});
396398
set_delivery_annotations(
397-
Props0, #amqp10_msg{delivery_annotations =
398-
#'v1_0.delivery_annotations'{content = Anns0}} = Msg) ->
399-
Anns = maps:fold(fun (K, V, S) ->
400-
S#{sym(K) => wrap_ap_value(V)}
401-
end, maps:from_list(Anns0), Props0),
402-
Anns1 = #'v1_0.delivery_annotations'{content = maps:to_list(Anns)},
403-
Msg#amqp10_msg{delivery_annotations = Anns1}.
399+
Props, #amqp10_msg{delivery_annotations =
400+
#'v1_0.delivery_annotations'{content = Anns0}} = Msg) ->
401+
Anns1 = maps:fold(fun (K, V, S) ->
402+
S#{sym(K) => amqp10_client_types:infer(V)}
403+
end, maps:from_list(Anns0), Props),
404+
Anns = #'v1_0.delivery_annotations'{content = maps:to_list(Anns1)},
405+
Msg#amqp10_msg{delivery_annotations = Anns}.
404406

405407
-spec set_message_annotations(#{binary() => binary() | number() | string() | tuple()},
406408
amqp10_msg()) -> amqp10_msg().
407409
set_message_annotations(Props,
408-
#amqp10_msg{message_annotations = undefined} =
409-
Msg) ->
410+
#amqp10_msg{message_annotations = undefined} =
411+
Msg) ->
410412
Anns = #'v1_0.message_annotations'{content = []},
411413
set_message_annotations(Props,
412-
Msg#amqp10_msg{message_annotations = Anns});
414+
Msg#amqp10_msg{message_annotations = Anns});
413415
set_message_annotations(
414-
Props0, #amqp10_msg{message_annotations =
415-
#'v1_0.message_annotations'{content = Anns0}} = Msg) ->
416-
Anns = maps:fold(fun (K, V, S) ->
417-
S#{sym(K) => wrap_ap_value(V)}
418-
end, maps:from_list(Anns0), Props0),
419-
Anns1 = #'v1_0.message_annotations'{content = maps:to_list(Anns)},
420-
Msg#amqp10_msg{message_annotations = Anns1}.
421-
422-
wrap_ap_value(true) ->
423-
{boolean, true};
424-
wrap_ap_value(false) ->
425-
{boolean, false};
426-
wrap_ap_value(V) when is_binary(V) ->
427-
utf8(V);
428-
wrap_ap_value(V) when is_list(V) ->
429-
utf8(list_to_binary(V));
430-
wrap_ap_value(V) when is_atom(V) ->
431-
utf8(atom_to_binary(V));
432-
wrap_ap_value(V) when is_integer(V) ->
433-
case V < 0 of
434-
true -> {int, V};
435-
false -> {uint, V}
436-
end;
437-
wrap_ap_value(V) when is_number(V) ->
438-
%% AMQP double and Erlang float are both 64-bit.
439-
{double, V};
440-
wrap_ap_value(TaggedValue) when is_tuple(TaggedValue) ->
441-
TaggedValue.
416+
Props, #amqp10_msg{message_annotations =
417+
#'v1_0.message_annotations'{content = Anns0}} = Msg) ->
418+
Anns1 = maps:fold(fun (K, V, S) ->
419+
S#{sym(K) => amqp10_client_types:infer(V)}
420+
end, maps:from_list(Anns0), Props),
421+
Anns = #'v1_0.message_annotations'{content = maps:to_list(Anns1)},
422+
Msg#amqp10_msg{message_annotations = Anns}.
442423

443424
%% LOCAL
444425
header_value(durable, undefined) -> false;
@@ -474,7 +455,6 @@ parse_from_amqp(#'v1_0.footer'{} = Header, AmqpMsg) ->
474455
AmqpMsg#amqp10_msg{footer = Header}.
475456

476457
unpack(V) -> amqp10_client_types:unpack(V).
477-
utf8(V) -> amqp10_client_types:utf8(V).
478458
sym(B) when is_list(B) -> {symbol, list_to_binary(B)};
479459
sym(B) when is_binary(B) -> {symbol, B}.
480460
uint(B) -> {uint, B}.

‎deps/rabbit/test/amqp_client_SUITE.erl‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1301,7 +1301,7 @@ amqp_amqpl(QType, Config) ->
13011301
ok = amqp10_client:send_msg(
13021302
Sender,
13031303
amqp10_msg:set_application_properties(
1304-
#{"my int" => -2},
1304+
#{"my int" => {int, -2}},
13051305
amqp10_msg:new(<<>>, Body1, true))),
13061306
%% Send with properties
13071307
CorrelationID = <<"my correlation ID">>,
@@ -1316,7 +1316,7 @@ amqp_amqpl(QType, Config) ->
13161316
amqp10_msg:set_properties(
13171317
#{correlation_id => CorrelationID},
13181318
amqp10_msg:set_application_properties(
1319-
#{"my int" => -2},
1319+
#{"my long" => -9_000_000_000},
13201320
amqp10_msg:new(<<>>, Body1, true)))),
13211321
%% Send with footer
13221322
Footer = #'v1_0.footer'{content = [{{symbol, <<"x-my footer">>}, {ubyte, 255}}]},
@@ -1405,7 +1405,7 @@ amqp_amqpl(QType, Config) ->
14051405
correlation_id = Corr9}}} ->
14061406
?assertEqual([Body1], amqp10_framing:decode_bin(Payload9)),
14071407
?assertEqual(CorrelationID, Corr9),
1408-
?assertEqual({signedint, -2}, rabbit_misc:table_lookup(Headers9, <<"my int">>))
1408+
?assertEqual({long, -9_000_000_000}, rabbit_misc:table_lookup(Headers9, <<"my long">>))
14091409
after 30000 -> ct:fail({missing_deliver, ?LINE})
14101410
end,
14111411
receive {_, #amqp_msg{payload = Payload10}} ->
@@ -1453,12 +1453,14 @@ amqp10_to_amqp091_header_conversion(Session,Ch, QName, Address) ->
14531453
OutMsg1 = amqp10_msg:new(<<"my-tag">>, <<"my-body">>, false),
14541454
OutMsg2 = amqp10_msg:set_application_properties(
14551455
#{"string" => "string-val",
1456-
"int" => 2,
1456+
"long" => -2,
1457+
"uint" => {uint, 2},
14571458
"bool" => false},
14581459
OutMsg1),
14591460
OutMsg3 = amqp10_msg:set_message_annotations(
14601461
#{"x-string" => "string-value",
1461-
"x-int" => 3,
1462+
"x-long" => -3,
1463+
"x-uint" => {uint, 3},
14621464
"x-bool" => true},
14631465
OutMsg2),
14641466
OutMsg = amqp10_msg:set_headers(
@@ -1478,11 +1480,13 @@ amqp10_to_amqp091_header_conversion(Session,Ch, QName, Address) ->
14781480

14791481
%% assert application properties
14801482
?assertEqual({longstr, <<"string-val">>}, rabbit_misc:table_lookup(Headers, <<"string">>)),
1481-
?assertEqual({unsignedint, 2}, rabbit_misc:table_lookup(Headers, <<"int">>)),
1483+
?assertEqual({long, -2}, rabbit_misc:table_lookup(Headers, <<"long">>)),
1484+
?assertEqual({unsignedint, 2}, rabbit_misc:table_lookup(Headers, <<"uint">>)),
14821485
?assertEqual({bool, false}, rabbit_misc:table_lookup(Headers, <<"bool">>)),
14831486
%% assert message annotations
14841487
?assertEqual({longstr, <<"string-value">>}, rabbit_misc:table_lookup(Headers, <<"x-string">>)),
1485-
?assertEqual({unsignedint, 3}, rabbit_misc:table_lookup(Headers, <<"x-int">>)),
1488+
?assertEqual({long, -3}, rabbit_misc:table_lookup(Headers, <<"x-long">>)),
1489+
?assertEqual({unsignedint, 3}, rabbit_misc:table_lookup(Headers, <<"x-uint">>)),
14861490
?assertEqual({bool, true}, rabbit_misc:table_lookup(Headers, <<"x-bool">>)),
14871491
%% assert headers
14881492
?assertEqual(2, DeliveryMode),

0 commit comments

Comments
 (0)