Skip to content

Commit 9f205dc

Browse files
parfeonparfeon
committed
fix(subscribe-heartbeat): fix duplicated channels issue
Fix issue because of which it was possible to add duplicated entries of `channels` and `groups` to the `subscribe`, `heartbeat`, and `leave` requests.
1 parent 4d3afaa commit 9f205dc

File tree

11 files changed

+141
-77
lines changed

11 files changed

+141
-77
lines changed

‎pubnub/endpoints/presence/heartbeat.py‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
from typing import Dict, Optional, Union, List
1+
from typing import Dict, Optional, Union, List, Set
22
from pubnub import utils
33
from pubnub.endpoints.endpoint import Endpoint
44
from pubnub.enums import HttpMethod, PNOperationType
@@ -13,22 +13,22 @@ class Heartbeat(Endpoint):
1313
def __init__(self, pubnub, channels: Union[str, List[str]] = None, channel_groups: Union[str, List[str]] = None,
1414
state: Optional[Dict[str, any]] = None):
1515
super(Heartbeat, self).__init__(pubnub)
16-
self._channels = []
17-
self._groups = []
16+
self._channels: Set[str] = set()
17+
self._groups: Set[str] = set()
1818
if channels:
19-
utils.extend_list(self._channels, channels)
19+
utils.update_set(self._channels, channels)
2020

2121
if channel_groups:
22-
utils.extend_list(self._groups, channel_groups)
22+
utils.update_set(self._groups, channel_groups)
2323

2424
self._state = state
2525

2626
def channels(self, channels: Union[str, List[str]]) -> 'Heartbeat':
27-
utils.extend_list(self._channels, channels)
27+
utils.update_set(self._channels, channels)
2828
return self
2929

3030
def channel_groups(self, channel_groups: Union[str, List[str]]) -> 'Heartbeat':
31-
utils.extend_list(self._groups, channel_groups)
31+
utils.update_set(self._groups, channel_groups)
3232
return self
3333

3434
def state(self, state: Dict[str, any]) -> 'Heartbeat':
@@ -46,14 +46,14 @@ def validate_params(self):
4646
raise PubNubException(pn_error=PNERR_CHANNEL_OR_GROUP_MISSING)
4747

4848
def build_path(self):
49-
channels = utils.join_channels(self._channels)
49+
channels = utils.join_channels(self._channels, True)
5050
return Heartbeat.HEARTBEAT_PATH % (self.pubnub.config.subscribe_key, channels)
5151

5252
def custom_params(self):
5353
params = {'heartbeat': str(self.pubnub.config.presence_timeout)}
5454

5555
if len(self._groups) > 0:
56-
params['channel-group'] = utils.join_items(self._groups)
56+
params['channel-group'] = utils.join_items(self._groups, True)
5757

5858
if self._state is not None and len(self._state) > 0:
5959
params['state'] = utils.url_write(self._state)

‎pubnub/endpoints/presence/leave.py‎

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
from typing import Set, Union, List
2+
13
from pubnub import utils
24
from pubnub.endpoints.endpoint import Endpoint
35
from pubnub.errors import PNERR_CHANNEL_OR_GROUP_MISSING
@@ -11,38 +13,30 @@ class Leave(Endpoint):
1113

1214
def __init__(self, pubnub):
1315
Endpoint.__init__(self, pubnub)
14-
self._channels = []
15-
self._groups = []
16-
17-
def channels(self, channels):
18-
if isinstance(channels, (list, tuple)):
19-
self._channels.extend(channels)
20-
else:
21-
self._channels.extend(utils.split_items(channels))
16+
self._channels: Set[str] = set()
17+
self._groups: Set[str] = set()
2218

19+
def channels(self, channels: Union[str, List[str]]) -> 'Leave':
20+
utils.update_set(self._channels, channels)
2321
return self
2422

25-
def channel_groups(self, channel_groups):
26-
if isinstance(channel_groups, (list, tuple)):
27-
self._groups.extend(channel_groups)
28-
else:
29-
self._groups.extend(utils.split_items(channel_groups))
30-
23+
def channel_groups(self, channel_groups: Union[str, List[str]]) -> 'Leave':
24+
utils.update_set(self._groups, channel_groups)
3125
return self
3226

3327
def custom_params(self):
3428
params = {}
3529

3630
if len(self._groups) > 0:
37-
params['channel-group'] = utils.join_items(self._groups)
31+
params['channel-group'] = utils.join_items(self._groups, True)
3832

3933
if hasattr(self.pubnub, '_subscription_manager'):
4034
params.update(self.pubnub._subscription_manager.get_custom_params())
4135

4236
return params
4337

4438
def build_path(self):
45-
return Leave.LEAVE_PATH % (self.pubnub.config.subscribe_key, utils.join_channels(self._channels))
39+
return Leave.LEAVE_PATH % (self.pubnub.config.subscribe_key, utils.join_channels(self._channels, True))
4640

4741
def http_method(self):
4842
return HttpMethod.GET
@@ -60,10 +54,10 @@ def is_auth_required(self):
6054
return True
6155

6256
def affected_channels(self):
63-
return self._channels
57+
return list(self._channels)
6458

6559
def affected_channels_groups(self):
66-
return self._groups
60+
return list(self._groups)
6761

6862
def request_timeout(self):
6963
return self.pubnub.config.non_subscribe_request_timeout

‎pubnub/endpoints/pubsub/subscribe.py‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
from typing import Optional, Union, List
1+
from typing import Optional, Union, List, Set
22
from pubnub import utils
33
from pubnub.endpoints.endpoint import Endpoint
44
from pubnub.enums import HttpMethod, PNOperationType
@@ -25,12 +25,12 @@ def __init__(self, pubnub, channels: Union[str, List[str]] = None,
2525
with_presence: Optional[str] = None, state: Optional[str] = None):
2626

2727
super(Subscribe, self).__init__(pubnub)
28-
self._channels = []
28+
self._channels: Set[str] = set()
29+
self._groups: Set[str] = set()
2930
if channels:
30-
utils.extend_list(self._channels, channels)
31-
self._groups = []
31+
utils.update_set(self._channels, channels)
3232
if groups:
33-
utils.extend_list(self._groups, groups)
33+
utils.update_set(self._groups, groups)
3434

3535
self._region = region
3636
self._filter_expression = filter_expression
@@ -39,11 +39,11 @@ def __init__(self, pubnub, channels: Union[str, List[str]] = None,
3939
self._state = state
4040

4141
def channels(self, channels: Union[str, List[str]]) -> 'Subscribe':
42-
utils.extend_list(self._channels, channels)
42+
utils.update_set(self._channels, channels)
4343
return self
4444

4545
def channel_groups(self, groups: Union[str, List[str]]) -> 'Subscribe':
46-
utils.extend_list(self._groups, groups)
46+
utils.update_set(self._groups, groups)
4747
return self
4848

4949
def timetoken(self, timetoken) -> 'Subscribe':
@@ -72,14 +72,14 @@ def validate_params(self):
7272
raise PubNubException(pn_error=PNERR_CHANNEL_OR_GROUP_MISSING)
7373

7474
def build_path(self):
75-
channels = utils.join_channels(self._channels)
75+
channels = utils.join_channels(self._channels, True)
7676
return Subscribe.SUBSCRIBE_PATH % (self.pubnub.config.subscribe_key, channels)
7777

7878
def custom_params(self):
7979
params = {}
8080

8181
if len(self._groups) > 0:
82-
params['channel-group'] = utils.join_items_and_encode(self._groups)
82+
params['channel-group'] = utils.join_items_and_encode(self._groups, True)
8383

8484
if self._filter_expression is not None and len(self._filter_expression) > 0:
8585
params['filter-expr'] = utils.url_encode(self._filter_expression)
@@ -108,10 +108,10 @@ def is_auth_required(self):
108108
return True
109109

110110
def affected_channels(self):
111-
return self._channels
111+
return list(self._channels)
112112

113113
def affected_channels_groups(self):
114-
return self._groups
114+
return list(self._groups)
115115

116116
def request_timeout(self):
117117
return self.pubnub.config.subscribe_request_timeout

‎pubnub/event_engine/models/states.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -568,7 +568,7 @@ def reconnect_failure(self, event: events.ReceiveReconnectFailureEvent, context:
568568
return PNTransition(
569569
state=ReceiveReconnectingState,
570570
context=self._context,
571-
invocation=invocations.EmitStatusInvocation(PNStatusCategory.UnexpectedDisconnectCategory,
571+
invocation=invocations.EmitStatusInvocation(PNStatusCategory.PNUnexpectedDisconnectCategory,
572572
operation=PNOperationType.PNSubscribeOperation,
573573
context=self._context)
574574
)

‎pubnub/utils.py‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import warnings
99

1010
from hashlib import sha256
11+
from typing import Set, List, Union
1112

1213
from pubnub.enums import PNStatusCategory, PNOperationType, PNPushType, HttpMethod, PAMPermissions
1314
from pubnub.models.consumer.common import PNStatus
@@ -54,19 +55,17 @@ def split_items(items_string):
5455
return items_string.split(",")
5556

5657

57-
def join_items(items_list):
58-
return ",".join(items_list)
58+
def join_items(items_list, sort_items=False):
59+
return ",".join(sorted(items_list) if sort_items else items_list)
5960

61+
def join_items_and_encode(items_list, sort_items=False):
62+
return ",".join(url_encode(x) for x in (sorted(items_list) if sort_items else items_list))
6063

61-
def join_items_and_encode(items_list):
62-
return ",".join(url_encode(x) for x in items_list)
63-
64-
65-
def join_channels(items_list):
64+
def join_channels(items_list, sort_items=False):
6665
if len(items_list) == 0:
6766
return ","
6867
else:
69-
return join_items_and_encode(items_list)
68+
return join_items_and_encode(items_list, sort_items)
7069

7170

7271
def extend_list(existing_items, new_items):
@@ -75,6 +74,11 @@ def extend_list(existing_items, new_items):
7574
else:
7675
existing_items.extend(new_items)
7776

77+
def update_set(existing_items: Set[str], new_items: Union[str, List[str]]):
78+
if isinstance(new_items, str):
79+
existing_items.update(split_items(new_items))
80+
else:
81+
existing_items.update(new_items)
7882

7983
def build_url(scheme, origin, path, params={}):
8084
return urllib.parse.urlunsplit((scheme, origin, path, params, ''))

‎tests/functional/test_heartbeat.py‎

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ def test_sub_single_channel(self):
3232
'heartbeat': '20'
3333
})
3434

35-
self.assertEqual(self.hb._channels, ['ch'])
35+
self.assertEqual(list(self.hb._channels), ['ch'])
3636

3737
def test_hb_multiple_channels_using_list(self):
3838
self.hb.channels(['ch1', 'ch2', 'ch3'])
@@ -46,7 +46,15 @@ def test_hb_multiple_channels_using_list(self):
4646
'heartbeat': '20'
4747
})
4848

49-
self.assertEqual(self.hb._channels, ['ch1', 'ch2', 'ch3'])
49+
self.assertEqual(sorted(self.hb._channels), ['ch1', 'ch2', 'ch3'])
50+
51+
def test_hb_unique_channels_using_list(self):
52+
self.hb.channels(['ch1', 'ch2', 'ch1'])
53+
54+
self.assertEqual(self.hb.build_path(), Heartbeat.HEARTBEAT_PATH
55+
% (pnconf.subscribe_key, "ch1,ch2"))
56+
57+
self.assertEqual(sorted(self.hb._channels), ['ch1', 'ch2'])
5058

5159
def test_hb_single_group(self):
5260
self.hb.channel_groups("gr")
@@ -61,7 +69,7 @@ def test_hb_single_group(self):
6169
'heartbeat': '20'
6270
})
6371

64-
self.assertEqual(self.hb._groups, ['gr'])
72+
self.assertEqual(list(self.hb._groups), ['gr'])
6573

6674
def test_hb_multiple_groups_using_list(self):
6775
self.hb.channel_groups(['gr1', 'gr2', 'gr3'])
@@ -76,7 +84,20 @@ def test_hb_multiple_groups_using_list(self):
7684
'heartbeat': '20'
7785
})
7886

79-
self.assertEqual(self.hb._groups, ['gr1', 'gr2', 'gr3'])
87+
self.assertEqual(sorted(self.hb._groups), ['gr1', 'gr2', 'gr3'])
88+
89+
def test_hb_unique_channel_groups_using_list(self):
90+
self.hb.channel_groups(['gr1', 'gr2', 'gr1'])
91+
92+
self.assertEqual(self.hb.build_path(), Heartbeat.HEARTBEAT_PATH
93+
% (pnconf.subscribe_key, ","))
94+
95+
self.assertEqual(self.hb.build_params_callback()({}), {
96+
'channel-group': 'gr1,gr2',
97+
'pnsdk': sdk_name,
98+
'uuid': self.pubnub.uuid,
99+
'heartbeat': '20'
100+
})
80101

81102
def test_hb_with_state(self):
82103
state = {"name": "Alex", "count": 7}
@@ -95,5 +116,5 @@ def test_hb_with_state(self):
95116
'state': state
96117
})
97118

98-
self.assertEqual(self.hb._groups, [])
99-
self.assertEqual(self.hb._channels, ['ch1', 'ch2'])
119+
self.assertEqual(list(self.hb._groups), [])
120+
self.assertEqual(sorted(self.hb._channels), ['ch1', 'ch2'])

‎tests/functional/test_leave.py‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ def test_leave_single_channel(self):
3333
'uuid': self.pubnub.uuid
3434
})
3535

36-
self.assertEqual(self.leave._channels, ['ch'])
36+
self.assertEqual(sorted(list(self.leave._channels)), ['ch'])
3737

3838
def test_leave_multiple_channels(self):
3939
self.leave.channels("ch1,ch2,ch3")
@@ -45,7 +45,7 @@ def test_leave_multiple_channels(self):
4545
'uuid': self.pubnub.uuid
4646
})
4747

48-
self.assertEqual(self.leave._channels, ['ch1', 'ch2', 'ch3'])
48+
self.assertEqual(sorted(list(self.leave._channels)), ['ch1', 'ch2', 'ch3'])
4949

5050
def test_leave_multiple_channels_using_list(self):
5151
self.leave.channels(['ch1', 'ch2', 'ch3'])
@@ -57,7 +57,7 @@ def test_leave_multiple_channels_using_list(self):
5757
'uuid': self.pubnub.uuid
5858
})
5959

60-
self.assertEqual(self.leave._channels, ['ch1', 'ch2', 'ch3'])
60+
self.assertEqual(sorted(list(self.leave._channels)), ['ch1', 'ch2', 'ch3'])
6161

6262
def test_leave_multiple_channels_using_tuple(self):
6363
self.leave.channels(('ch1', 'ch2', 'ch3'))
@@ -69,7 +69,14 @@ def test_leave_multiple_channels_using_tuple(self):
6969
'uuid': self.pubnub.uuid
7070
})
7171

72-
self.assertEqual(self.leave._channels, ['ch1', 'ch2', 'ch3'])
72+
self.assertEqual(sorted(list(self.leave._channels)), ['ch1', 'ch2', 'ch3'])
73+
74+
def test_leave_unique_channels_using_list(self):
75+
self.leave.channels(['ch1', 'ch2', 'ch1'])
76+
77+
self.assertEqual(self.leave.build_path(), Leave.LEAVE_PATH % (pnconf.subscribe_key, "ch1,ch2"))
78+
79+
self.assertEqual(sorted(list(self.leave._channels)), ['ch1', 'ch2'])
7380

7481
def test_leave_single_group(self):
7582
self.leave.channel_groups("gr")
@@ -83,7 +90,7 @@ def test_leave_single_group(self):
8390
'uuid': self.pubnub.uuid
8491
})
8592

86-
self.assertEqual(self.leave._groups, ['gr'])
93+
self.assertEqual(list(self.leave._groups), ['gr'])
8794

8895
def test_leave_multiple_groups_using_string(self):
8996
self.leave.channel_groups("gr1,gr2,gr3")
@@ -97,7 +104,7 @@ def test_leave_multiple_groups_using_string(self):
97104
'uuid': self.pubnub.uuid
98105
})
99106

100-
self.assertEqual(self.leave._groups, ['gr1', 'gr2', 'gr3'])
107+
self.assertEqual(sorted(list(self.leave._groups)), ['gr1', 'gr2', 'gr3'])
101108

102109
def test_leave_multiple_groups_using_list(self):
103110
self.leave.channel_groups(['gr1', 'gr2', 'gr3'])
@@ -111,7 +118,18 @@ def test_leave_multiple_groups_using_list(self):
111118
'uuid': self.pubnub.uuid
112119
})
113120

114-
self.assertEqual(self.leave._groups, ['gr1', 'gr2', 'gr3'])
121+
self.assertEqual(sorted(list(self.leave._groups)), ['gr1', 'gr2', 'gr3'])
122+
123+
def test_leave_unique_channel_groups_using_list(self):
124+
self.leave.channel_groups(['gr1', 'gr2', 'gr1'])
125+
126+
self.assertEqual(self.leave.build_params_callback()({}), {
127+
'channel-group': 'gr1,gr2',
128+
'pnsdk': sdk_name,
129+
'uuid': self.pubnub.uuid
130+
})
131+
132+
self.assertEqual(sorted(list(self.leave._groups)), ['gr1', 'gr2'])
115133

116134
def test_leave_channels_and_groups(self):
117135
self.leave.channels('ch1,ch2').channel_groups(["gr1", "gr2"])
@@ -125,5 +143,5 @@ def test_leave_channels_and_groups(self):
125143
'channel-group': 'gr1,gr2',
126144
})
127145

128-
self.assertEqual(self.leave._groups, ['gr1', 'gr2'])
129-
self.assertEqual(self.leave._channels, ['ch1', 'ch2'])
146+
self.assertEqual(sorted(list(self.leave._groups)), ['gr1', 'gr2'])
147+
self.assertEqual(sorted(list(self.leave._channels)), ['ch1', 'ch2'])

0 commit comments

Comments
 (0)