Skip to content

Commit afd28ba

Browse files
ansdansd
committed
Apply PR feedback
1 parent 53e9407 commit afd28ba

File tree

3 files changed

+66
-17
lines changed

3 files changed

+66
-17
lines changed

‎deps/amqp10_common/src/serial_number.erl

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
-export([add/2,
1212
compare/2,
1313
ranges/1,
14+
in_range/3,
1415
diff/2,
1516
foldl/4]).
1617

@@ -86,6 +87,21 @@ ranges0([H | Rest], [{First, Last} | AccRest] = Acc0) ->
8687
ranges0(Rest, Acc)
8788
end.
8889

90+
-spec in_range(serial_number(), serial_number(), serial_number()) ->
91+
boolean().
92+
in_range(S, First, Last) ->
93+
case compare(S, First) of
94+
less ->
95+
false;
96+
_ ->
97+
case compare(S, Last) of
98+
greater ->
99+
false;
100+
_ ->
101+
true
102+
end
103+
end.
104+
89105
-define(SERIAL_DIFF_BOUND, 16#80000000).
90106
-spec diff(serial_number(), serial_number()) -> integer().
91107
diff(A, B) ->

‎deps/amqp10_common/test/serial_number_SUITE.erl

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,15 @@
1515
compare/2,
1616
usort/1,
1717
ranges/1,
18+
in_range/3,
1819
diff/2,
1920
foldl/4]).
2021

2122
all() -> [test_add,
2223
test_compare,
2324
test_usort,
2425
test_ranges,
26+
test_in_range,
2527
test_diff,
2628
test_foldl].
2729

@@ -96,6 +98,31 @@ test_ranges(_Config) ->
9698
?assertEqual([{4294967294, 1}, {3, 5}, {10, 10}, {18, 19}],
9799
ranges([1, 10, 4294967294, 0, 3, 4, 5, 19, 18, 4294967295])).
98100

101+
test_in_range(_Config) ->
102+
?assert(in_range(0, 0, 0)),
103+
?assert(in_range(0, 0, 1)),
104+
?assert(in_range(4294967295, 4294967295, 4294967295)),
105+
?assert(in_range(4294967295, 4294967295, 0)),
106+
?assert(in_range(0, 4294967295, 0)),
107+
?assert(in_range(4294967230, 4294967200, 1000)),
108+
?assert(in_range(88, 4294967200, 1000)),
109+
110+
?assertNot(in_range(1, 0, 0)),
111+
?assertNot(in_range(4294967295, 0, 0)),
112+
?assertNot(in_range(0, 1, 1)),
113+
?assertNot(in_range(10, 1, 9)),
114+
?assertNot(in_range(1005, 4294967200, 1000)),
115+
?assertNot(in_range(4294967190, 4294967200, 1000)),
116+
117+
%% Pass wrong First and Last.
118+
?assertNot(in_range(1, 3, 2)),
119+
?assertNot(in_range(2, 3, 2)),
120+
?assertNot(in_range(3, 3, 2)),
121+
?assertNot(in_range(4, 3, 2)),
122+
123+
?assertExit({undefined_serial_comparison, 0, 16#80000000},
124+
in_range(0, 16#80000000, 16#80000000)).
125+
99126
test_diff(_Config) ->
100127
?assertEqual(0, diff(0, 0)),
101128
?assertEqual(0, diff(1, 1)),

‎deps/rabbit/src/rabbit_amqp_session.erl

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
-module(rabbit_amqp_session).
99

10+
-compile({inline, [maps_update_with/4]}).
11+
1012
-behaviour(gen_server).
1113

1214
-include_lib("rabbit_common/include/rabbit.hrl").
@@ -1061,20 +1063,16 @@ handle_control(#'v1_0.disposition'{role = ?RECV_ROLE,
10611063
consumer_tag = Ctag,
10621064
msg_id = MsgId} = Unsettled,
10631065
{SettledAcc, UnsettledAcc}) ->
1064-
DeliveryIdComparedToFirst = compare(DeliveryId, First),
1065-
DeliveryIdComparedToLast = compare(DeliveryId, Last),
1066-
if DeliveryIdComparedToFirst =:= less orelse
1067-
DeliveryIdComparedToLast =:= greater ->
1068-
%% Delivery ID is outside the DISPOSITION range.
1069-
{SettledAcc, UnsettledAcc#{DeliveryId => Unsettled}};
1070-
true ->
1071-
%% Delivery ID is inside the DISPOSITION range.
1072-
SettledAcc1 = maps:update_with(
1073-
{QName, Ctag},
1074-
fun(MsgIds) -> [MsgId | MsgIds] end,
1075-
[MsgId],
1076-
SettledAcc),
1077-
{SettledAcc1, UnsettledAcc}
1066+
case serial_number:in_range(DeliveryId, First, Last) of
1067+
true ->
1068+
SettledAcc1 = maps_update_with(
1069+
{QName, Ctag},
1070+
fun(MsgIds) -> [MsgId | MsgIds] end,
1071+
[MsgId],
1072+
SettledAcc),
1073+
{SettledAcc1, UnsettledAcc};
1074+
false ->
1075+
{SettledAcc, UnsettledAcc#{DeliveryId => Unsettled}}
10781076
end
10791077
end,
10801078
{#{}, #{}}, UnsettledMap)
@@ -1209,19 +1207,19 @@ session_flow_control_sent_transfers(
12091207
State#state{remote_incoming_window = RemoteIncomingWindow - NumTransfers,
12101208
next_outgoing_id = add(NextOutgoingId, NumTransfers)}.
12111209

1212-
settle_delivery_id(Current, {Settled, Unsettled}) ->
1210+
settle_delivery_id(Current, {Settled, Unsettled} = Acc) ->
12131211
case maps:take(Current, Unsettled) of
12141212
{#outgoing_unsettled{queue_name = QName,
12151213
consumer_tag = Ctag,
12161214
msg_id = MsgId}, Unsettled1} ->
1217-
Settled1 = maps:update_with(
1215+
Settled1 = maps_update_with(
12181216
{QName, Ctag},
12191217
fun(MsgIds) -> [MsgId | MsgIds] end,
12201218
[MsgId],
12211219
Settled),
12221220
{Settled1, Unsettled1};
12231221
error ->
1224-
{Settled, Unsettled}
1222+
Acc
12251223
end.
12261224

12271225
settle_op_from_outcome(#'v1_0.accepted'{}) ->
@@ -2276,6 +2274,14 @@ check_user_id(Mc, User) ->
22762274
protocol_error(?V_1_0_AMQP_ERROR_UNAUTHORIZED_ACCESS, Reason, Args)
22772275
end.
22782276

2277+
maps_update_with(Key, Fun, Init, Map) ->
2278+
case Map of
2279+
#{Key := Value} ->
2280+
Map#{Key := Fun(Value)};
2281+
_ ->
2282+
Map#{Key => Init}
2283+
end.
2284+
22792285
format_status(
22802286
#{state := #state{cfg = Cfg,
22812287
outgoing_pending = OutgoingPending,

0 commit comments

Comments
 (0)