Skip to content

Commit 0078653

Browse files
committed
fix(fcm): Improve topic management performance and fix test lint issues
- Hoist URL-encoded topic computation out of the per-token request loops. - Mount an HTTPAdapter with a connection pool size of 100 on the FCM client session. - Fix line length and method signature override lint warnings in test_messaging.py.
1 parent be27016 commit 0078653

2 files changed

Lines changed: 48 additions & 16 deletions

File tree

‎firebase_admin/messaging.py‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -512,6 +512,12 @@ def __init__(self, app: App) -> None:
512512
timeout = app.options.get('httpTimeout', _http_client.DEFAULT_TIMEOUT_SECONDS)
513513
self._credential = app.credential.get_credential()
514514
self._client = _http_client.JsonHttpClient(credential=self._credential, timeout=timeout)
515+
fcm_adapter = requests.adapters.HTTPAdapter(
516+
pool_connections=100,
517+
pool_maxsize=100,
518+
max_retries=_http_client.DEFAULT_RETRY_CONFIG
519+
)
520+
self._client.session.mount('https://fcm.googleapis.com', fcm_adapter)
515521
self._async_client = _http_client.HttpxAsyncClient(
516522
credential=self._credential, timeout=timeout)
517523

@@ -640,10 +646,10 @@ def _make_topic_management_request_v1(
640646
) -> TopicManagementResponse:
641647
"""Helper method that sends topic subscription requests via FCM v1 API."""
642648
tokens_list, topic_name = self._validate_topic_management_args(tokens, topic)
649+
encoded_topic = urllib.parse.quote(topic_name, safe='')
643650

644651
def send_request(token: str):
645652
encoded_token = urllib.parse.quote(token, safe='')
646-
encoded_topic = urllib.parse.quote(topic_name, safe='')
647653
base_url = f'{self._fcm_topic_url}/{encoded_token}/topicSubscriptions'
648654
if is_subscribe:
649655
url = f'{base_url}?topic_name={encoded_topic}'
@@ -682,11 +688,11 @@ async def _make_topic_management_request_v1_async(
682688
) -> TopicManagementResponse:
683689
"""Helper method that sends topic subscription requests asynchronously via FCM v1 API."""
684690
tokens_list, topic_name = self._validate_topic_management_args(tokens, topic)
691+
encoded_topic = urllib.parse.quote(topic_name, safe='')
685692
semaphore = asyncio.Semaphore(100)
686693

687694
async def send_request_async(token: str):
688695
encoded_token = urllib.parse.quote(token, safe='')
689-
encoded_topic = urllib.parse.quote(topic_name, safe='')
690696
base_url = f'{self._fcm_topic_url}/{encoded_token}/topicSubscriptions'
691697
if is_subscribe:
692698
url = f'{base_url}?topic_name={encoded_topic}'

‎tests/test_messaging.py‎

Lines changed: 40 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2688,7 +2688,9 @@ def test_invalid_topic(self, topic):
26882688
messaging.unsubscribe_from_topic('test-token', topic)
26892689
assert str(excinfo.value) == expected
26902690

2691-
@pytest.mark.parametrize('topic', ['/topics/', '/foo/bar', 'foo bar', 'f*o*o', '/topics/f+o+o', '$foo', '/topics/foo&'])
2691+
@pytest.mark.parametrize('topic', [
2692+
'/topics/', '/foo/bar', 'foo bar', 'f*o*o', '/topics/f+o+o', '$foo', '/topics/foo&'
2693+
])
26922694
def test_malformed_topic(self, topic):
26932695
with pytest.raises(ValueError) as excinfo:
26942696
messaging.subscribe_to_topic('test-token', topic)
@@ -2705,7 +2707,10 @@ def test_subscribe_to_topic_single(self):
27052707
assert resp.failure_count == 0
27062708
assert resp.errors == []
27072709
assert len(recorder) == 1
2708-
expected_url = 'https://fcm.googleapis.com/v1/projects/explicit-project-id/registrations/token1/topicSubscriptions?topic_name=test-topic'
2710+
expected_url = (
2711+
'https://fcm.googleapis.com/v1/projects/explicit-project-id/registrations/'
2712+
'token1/topicSubscriptions?topic_name=test-topic'
2713+
)
27092714
self._assert_request(recorder[0], 'POST', expected_url, {})
27102715

27112716
def test_subscribe_to_topic_prefixed(self):
@@ -2715,7 +2720,10 @@ def test_subscribe_to_topic_prefixed(self):
27152720
assert resp.failure_count == 0
27162721
assert resp.errors == []
27172722
assert len(recorder) == 1
2718-
expected_url = 'https://fcm.googleapis.com/v1/projects/explicit-project-id/registrations/token1/topicSubscriptions?topic_name=test-topic'
2723+
expected_url = (
2724+
'https://fcm.googleapis.com/v1/projects/explicit-project-id/registrations/'
2725+
'token1/topicSubscriptions?topic_name=test-topic'
2726+
)
27192727
self._assert_request(recorder[0], 'POST', expected_url, {})
27202728

27212729
def test_unsubscribe_from_topic_single(self):
@@ -2725,7 +2733,10 @@ def test_unsubscribe_from_topic_single(self):
27252733
assert resp.failure_count == 0
27262734
assert resp.errors == []
27272735
assert len(recorder) == 1
2728-
expected_url = 'https://fcm.googleapis.com/v1/projects/explicit-project-id/registrations/token1/topicSubscriptions/test-topic?allow_missing=true'
2736+
expected_url = (
2737+
'https://fcm.googleapis.com/v1/projects/explicit-project-id/registrations/'
2738+
'token1/topicSubscriptions/test-topic?allow_missing=true'
2739+
)
27292740
self._assert_request(recorder[0], 'DELETE', expected_url, None)
27302741

27312742
def test_subscribe_to_topic_already_exists_409(self):
@@ -2753,7 +2764,7 @@ def test_topic_management_multiple_tokens(self):
27532764
recorder = []
27542765

27552766
class MultiTokenMockAdapter(requests.adapters.HTTPAdapter):
2756-
def send(self, request, **kwargs):
2767+
def send(self, request, **kwargs): # pylint: disable=arguments-differ,unused-argument
27572768
recorder.append(request)
27582769
resp = requests.models.Response()
27592770
resp.url = request.url
@@ -2834,7 +2845,10 @@ def teardown_class(cls):
28342845
@pytest.mark.asyncio
28352846
@respx.mock
28362847
async def test_subscribe_to_topic_async_single(self):
2837-
url = 'https://fcm.googleapis.com/v1/projects/explicit-project-id/registrations/token1/topicSubscriptions?topic_name=test-topic'
2848+
url = (
2849+
'https://fcm.googleapis.com/v1/projects/explicit-project-id/registrations/'
2850+
'token1/topicSubscriptions?topic_name=test-topic'
2851+
)
28382852
route = respx.post(url).mock(return_value=respx.MockResponse(200, json={}))
28392853
resp = await messaging.subscribe_to_topic_async('token1', 'test-topic')
28402854
assert route.call_count == 1
@@ -2845,7 +2859,10 @@ async def test_subscribe_to_topic_async_single(self):
28452859
@pytest.mark.asyncio
28462860
@respx.mock
28472861
async def test_subscribe_to_topic_async_already_exists(self):
2848-
url = 'https://fcm.googleapis.com/v1/projects/explicit-project-id/registrations/token1/topicSubscriptions?topic_name=test-topic'
2862+
url = (
2863+
'https://fcm.googleapis.com/v1/projects/explicit-project-id/registrations/'
2864+
'token1/topicSubscriptions?topic_name=test-topic'
2865+
)
28492866
payload = {'error': {'status': 'ALREADY_EXISTS', 'message': 'Already exists'}}
28502867
route = respx.post(url).mock(return_value=respx.MockResponse(409, json=payload))
28512868
resp = await messaging.subscribe_to_topic_async('token1', 'test-topic')
@@ -2857,7 +2874,10 @@ async def test_subscribe_to_topic_async_already_exists(self):
28572874
@pytest.mark.asyncio
28582875
@respx.mock
28592876
async def test_unsubscribe_from_topic_async_single(self):
2860-
url = 'https://fcm.googleapis.com/v1/projects/explicit-project-id/registrations/token1/topicSubscriptions/test-topic?allow_missing=true'
2877+
url = (
2878+
'https://fcm.googleapis.com/v1/projects/explicit-project-id/registrations/'
2879+
'token1/topicSubscriptions/test-topic?allow_missing=true'
2880+
)
28612881
route = respx.delete(url).mock(return_value=respx.MockResponse(200, json={}))
28622882
resp = await messaging.unsubscribe_from_topic_async('token1', 'test-topic')
28632883
assert route.call_count == 1
@@ -2868,7 +2888,10 @@ async def test_unsubscribe_from_topic_async_single(self):
28682888
@pytest.mark.asyncio
28692889
@respx.mock
28702890
async def test_unsubscribe_from_topic_async_not_found(self):
2871-
url = 'https://fcm.googleapis.com/v1/projects/explicit-project-id/registrations/token1/topicSubscriptions/test-topic?allow_missing=true'
2891+
url = (
2892+
'https://fcm.googleapis.com/v1/projects/explicit-project-id/registrations/'
2893+
'token1/topicSubscriptions/test-topic?allow_missing=true'
2894+
)
28722895
payload = {'error': {'status': 'NOT_FOUND', 'message': 'Not found'}}
28732896
route = respx.delete(url).mock(return_value=respx.MockResponse(404, json=payload))
28742897
resp = await messaging.unsubscribe_from_topic_async('token1', 'test-topic')
@@ -2882,14 +2905,17 @@ async def test_unsubscribe_from_topic_async_not_found(self):
28822905
@pytest.mark.asyncio
28832906
@respx.mock
28842907
async def test_subscribe_to_topic_async_multiple(self):
2885-
url1 = 'https://fcm.googleapis.com/v1/projects/explicit-project-id/registrations/token1/topicSubscriptions?topic_name=test-topic'
2886-
url2 = 'https://fcm.googleapis.com/v1/projects/explicit-project-id/registrations/token2/topicSubscriptions?topic_name=test-topic'
2887-
url3 = 'https://fcm.googleapis.com/v1/projects/explicit-project-id/registrations/token3/topicSubscriptions?topic_name=test-topic'
2908+
base = 'https://fcm.googleapis.com/v1/projects/explicit-project-id/registrations'
2909+
url1 = f'{base}/token1/topicSubscriptions?topic_name=test-topic'
2910+
url2 = f'{base}/token2/topicSubscriptions?topic_name=test-topic'
2911+
url3 = f'{base}/token3/topicSubscriptions?topic_name=test-topic'
28882912
route1 = respx.post(url1).mock(return_value=respx.MockResponse(200, json={}))
2889-
route2 = respx.post(url2).mock(return_value=respx.MockResponse(404, json={'error': {'status': 'NOT_FOUND'}}))
2913+
route2 = respx.post(url2).mock(
2914+
return_value=respx.MockResponse(404, json={'error': {'status': 'NOT_FOUND'}}))
28902915
route3 = respx.post(url3).mock(return_value=respx.MockResponse(200, json={}))
28912916

2892-
resp = await messaging.subscribe_to_topic_async(['token1', 'token2', 'token3'], 'test-topic')
2917+
resp = await messaging.subscribe_to_topic_async(
2918+
['token1', 'token2', 'token3'], 'test-topic')
28932919
assert route1.call_count == 1
28942920
assert route2.call_count == 1
28952921
assert route3.call_count == 1

0 commit comments

Comments
 (0)