Skip to content

Commit f4bd3f8

Browse files
committed
Raise only for non-unique provided sub labels; create unique default labels:
- If a user does not provide a subscription label, they likely don't care about tracking via a label. Create unique default labels for each subscription. - If a user provides a subscription label, but it is not unique, raise an error.
1 parent f7d1cd3 commit f4bd3f8

File tree

4 files changed

+51
-14
lines changed

4 files changed

+51
-14
lines changed

newsfragments/3594.bugfix.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Don't raise on non-unique default subscription labels (no label provided). Only raise if a non-unique custom label is explicitly set for a subscription.

tests/core/subscriptions/test_subscription_manager.py

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,19 +40,42 @@ async def subscription_manager():
4040

4141

4242
@pytest.mark.asyncio
43-
async def test_subscription_manager_raises_for_sub_with_the_same_label(
43+
async def test_subscription_default_labels_are_unique(subscription_manager):
44+
sub1 = NewHeadsSubscription()
45+
sub2 = NewHeadsSubscription()
46+
sub3 = NewHeadsSubscription()
47+
sub4 = NewHeadsSubscription()
48+
49+
await subscription_manager.subscribe([sub1, sub2, sub3, sub4])
50+
51+
assert sub1.label != sub2.label != sub3.label != sub4.label
52+
assert sub1.label == "NewHeadsSubscription('newHeads',)"
53+
assert sub2.label == "NewHeadsSubscription('newHeads',)#2"
54+
assert sub3.label == "NewHeadsSubscription('newHeads',)#3"
55+
assert sub4.label == "NewHeadsSubscription('newHeads',)#4"
56+
57+
# assert no issues unsubscribing
58+
await subscription_manager.unsubscribe_all()
59+
60+
assert subscription_manager.subscriptions == []
61+
assert subscription_manager._subscription_container.subscriptions == []
62+
assert subscription_manager._subscription_container.subscriptions_by_id == {}
63+
assert subscription_manager._subscription_container.subscriptions_by_label == {}
64+
65+
66+
@pytest.mark.asyncio
67+
async def test_subscription_manager_raises_for_new_subs_with_the_same_custom_label(
4468
subscription_manager,
4569
):
4670
sub1 = NewHeadsSubscription(label="foo")
47-
await subscription_manager.subscribe(sub1)
71+
sub2 = LogsSubscription(label="foo")
4872

4973
with pytest.raises(
5074
Web3ValueError,
5175
match="Subscription label already exists. Subscriptions must have unique "
5276
"labels.\n label: foo",
5377
):
54-
sub2 = LogsSubscription(label="foo")
55-
await subscription_manager.subscribe(sub2)
78+
await subscription_manager.subscribe([sub1, sub2])
5679

5780
# make sure the subscription was subscribed to and not added to the manager
5881
assert subscription_manager.subscriptions == [sub1]

web3/providers/persistent/subscription_manager.py

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,23 @@ def _add_subscription(self, subscription: EthSubscription[Any]) -> None:
6969
def _remove_subscription(self, subscription: EthSubscription[Any]) -> None:
7070
self._subscription_container.remove_subscription(subscription)
7171

72+
def _validate_and_normalize_label(self, subscription: EthSubscription[Any]) -> None:
73+
if subscription.label == subscription._default_label:
74+
# if no custom label was provided, generate a unique label
75+
i = 2
76+
while self.get_by_label(subscription._label) is not None:
77+
subscription._label = f"{subscription._default_label}#{i}"
78+
i += 1
79+
else:
80+
if (
81+
subscription._label
82+
in self._subscription_container.subscriptions_by_label
83+
):
84+
raise Web3ValueError(
85+
"Subscription label already exists. Subscriptions must have unique "
86+
f"labels.\n label: {subscription._label}"
87+
)
88+
7289
@property
7390
def subscriptions(self) -> List[EthSubscription[Any]]:
7491
return self._subscription_container.subscriptions
@@ -100,16 +117,8 @@ async def subscribe(
100117
:return:
101118
"""
102119
if isinstance(subscriptions, EthSubscription):
103-
if (
104-
subscriptions.label
105-
in self._subscription_container.subscriptions_by_label
106-
):
107-
raise Web3ValueError(
108-
"Subscription label already exists. Subscriptions must have "
109-
f"unique labels.\n label: {subscriptions.label}"
110-
)
111-
112120
subscriptions.manager = self
121+
self._validate_and_normalize_label(subscriptions)
113122
sub_id = await self._w3.eth._subscribe(*subscriptions.subscription_params)
114123
subscriptions._id = sub_id
115124
self._add_subscription(subscriptions)

web3/utils/subscriptions.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,10 @@ def __init__(
114114
self._label = label
115115
self.handler_call_count = 0
116116

117+
@property
118+
def _default_label(self) -> str:
119+
return f"{self.__class__.__name__}{self.subscription_params}"
120+
117121
@classmethod
118122
def _create_type_aware_subscription(
119123
cls,
@@ -170,7 +174,7 @@ def subscription_params(self) -> Sequence[Any]:
170174
@property
171175
def label(self) -> str:
172176
if not self._label:
173-
self._label = f"{self.__class__.__name__}{self.subscription_params}"
177+
self._label = self._default_label
174178
return self._label
175179

176180
@property

0 commit comments

Comments
 (0)