Skip to content

Commit b4e98ba

Browse files
committed
fix(fcm): Addressed review improvements for FCM v1 topic management
- Added type annotations to public topic management functions and internal methods - Updated HTTP status code mapping (401->UNAUTHENTICATED, 503->UNAVAILABLE, 408/504->DEADLINE_EXCEEDED) and prioritized status codes over free-form message strings - Updated topic regex with \Z to reject trailing newlines - Passed header copies in send, send_each, and send_each_async to prevent concurrency mutations - Added test coverage for status code mapping, async argument validation, prefixed topics, and async batch unsubscribe
1 parent 0078653 commit b4e98ba

2 files changed

Lines changed: 255 additions & 22 deletions

File tree

‎firebase_admin/messaging.py‎

Lines changed: 48 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
import json
2121
import logging
2222
import re
23-
from typing import Any, Callable, Dict, List, Optional, cast
23+
from typing import Any, Callable, Dict, List, Optional, Union, cast
2424
import urllib.parse
2525
import warnings
2626

@@ -259,7 +259,9 @@ def send_each_for_multicast(multicast_message, dry_run=False, app=None):
259259
messages = _get_messages_from_multicast(multicast_message)
260260
return _get_messaging_service(app).send_each(messages, dry_run)
261261

262-
def subscribe_to_topic(tokens, topic, app=None):
262+
def subscribe_to_topic(
263+
tokens: Union[str, List[str]], topic: str, app: Optional[App] = None
264+
) -> TopicManagementResponse:
263265
"""Subscribes a list of registration tokens to an FCM topic.
264266
265267
Args:
@@ -277,7 +279,9 @@ def subscribe_to_topic(tokens, topic, app=None):
277279
"""
278280
return _get_messaging_service(app).subscribe_to_topic(tokens, topic)
279281

280-
async def subscribe_to_topic_async(tokens, topic, app=None):
282+
async def subscribe_to_topic_async(
283+
tokens: Union[str, List[str]], topic: str, app: Optional[App] = None
284+
) -> TopicManagementResponse:
281285
"""Subscribes a list of registration tokens to an FCM topic asynchronously.
282286
283287
Args:
@@ -295,7 +299,9 @@ async def subscribe_to_topic_async(tokens, topic, app=None):
295299
"""
296300
return await _get_messaging_service(app).subscribe_to_topic_async(tokens, topic)
297301

298-
def subscribe_to_topic_legacy(tokens, topic, app=None):
302+
def subscribe_to_topic_legacy(
303+
tokens: Union[str, List[str]], topic: str, app: Optional[App] = None
304+
) -> TopicManagementResponse:
299305
"""Subscribes a list of registration tokens to an FCM topic using the legacy Instance ID API.
300306
301307
subscribe_to_topic_legacy is deprecated. Use subscribe_to_topic instead.
@@ -320,7 +326,9 @@ def subscribe_to_topic_legacy(tokens, topic, app=None):
320326
return _get_messaging_service(app).make_topic_management_request(
321327
tokens, topic, 'iid/v1:batchAdd')
322328

323-
def unsubscribe_from_topic(tokens, topic, app=None):
329+
def unsubscribe_from_topic(
330+
tokens: Union[str, List[str]], topic: str, app: Optional[App] = None
331+
) -> TopicManagementResponse:
324332
"""Unsubscribes a list of registration tokens from an FCM topic.
325333
326334
Args:
@@ -338,7 +346,9 @@ def unsubscribe_from_topic(tokens, topic, app=None):
338346
"""
339347
return _get_messaging_service(app).unsubscribe_from_topic(tokens, topic)
340348

341-
async def unsubscribe_from_topic_async(tokens, topic, app=None):
349+
async def unsubscribe_from_topic_async(
350+
tokens: Union[str, List[str]], topic: str, app: Optional[App] = None
351+
) -> TopicManagementResponse:
342352
"""Unsubscribes a list of registration tokens from an FCM topic asynchronously.
343353
344354
Args:
@@ -356,7 +366,9 @@ async def unsubscribe_from_topic_async(tokens, topic, app=None):
356366
"""
357367
return await _get_messaging_service(app).unsubscribe_from_topic_async(tokens, topic)
358368

359-
def unsubscribe_from_topic_legacy(tokens, topic, app=None):
369+
def unsubscribe_from_topic_legacy(
370+
tokens: Union[str, List[str]], topic: str, app: Optional[App] = None
371+
) -> TopicManagementResponse:
360372
"""Unsubscribes a list of registration tokens from an FCM topic using the legacy
361373
Instance ID API.
362374
@@ -534,7 +546,7 @@ def send(self, message: Message, dry_run: bool = False) -> str:
534546
resp = self._client.body(
535547
'post',
536548
url=self._fcm_url,
537-
headers=self._fcm_headers,
549+
headers=dict(self._fcm_headers),
538550
json=data
539551
)
540552
except requests.exceptions.RequestException as error:
@@ -553,7 +565,7 @@ def send_data(data):
553565
resp = self._client.body(
554566
'post',
555567
url=self._fcm_url,
556-
headers=self._fcm_headers,
568+
headers=dict(self._fcm_headers),
557569
json=data)
558570
except requests.exceptions.RequestException as exception:
559571
return SendResponse(resp=None, exception=self._handle_fcm_error(exception))
@@ -581,7 +593,7 @@ async def send_data(data):
581593
resp = await self._async_client.request(
582594
'post',
583595
url=self._fcm_url,
584-
headers=self._fcm_headers,
596+
headers=dict(self._fcm_headers),
585597
json=data)
586598
except httpx.HTTPError as exception:
587599
return SendResponse(resp=None, exception=self._handle_fcm_httpx_error(exception))
@@ -616,26 +628,34 @@ def _validate_topic_management_args(self, tokens, topic):
616628
topic_name = topic
617629
if topic_name.startswith('/topics/'):
618630
topic_name = topic_name[len('/topics/'):]
619-
if not topic_name or not re.match(r'^[a-zA-Z0-9-_\.~%]+$', topic_name):
631+
if not topic_name or not re.match(r'^[a-zA-Z0-9-_\.~%]+\Z', topic_name):
620632
raise ValueError('Malformed topic name.')
621633

622634
return tokens, topic_name
623635

624-
def subscribe_to_topic(self, tokens, topic) -> TopicManagementResponse:
636+
def subscribe_to_topic(
637+
self, tokens: Union[str, List[str]], topic: str
638+
) -> TopicManagementResponse:
625639
"""Subscribes a list of registration tokens to an FCM topic via the FCM v1 API."""
626640
return self._make_topic_management_request_v1(tokens, topic, is_subscribe=True)
627641

628-
def unsubscribe_from_topic(self, tokens, topic) -> TopicManagementResponse:
642+
def unsubscribe_from_topic(
643+
self, tokens: Union[str, List[str]], topic: str
644+
) -> TopicManagementResponse:
629645
"""Unsubscribes a list of registration tokens from an FCM topic via the FCM v1 API."""
630646
return self._make_topic_management_request_v1(tokens, topic, is_subscribe=False)
631647

632-
async def subscribe_to_topic_async(self, tokens, topic) -> TopicManagementResponse:
648+
async def subscribe_to_topic_async(
649+
self, tokens: Union[str, List[str]], topic: str
650+
) -> TopicManagementResponse:
633651
"""Subscribes a list of registration tokens to an FCM topic asynchronously
634652
via the FCM v1 API."""
635653
return await self._make_topic_management_request_v1_async(
636654
tokens, topic, is_subscribe=True)
637655

638-
async def unsubscribe_from_topic_async(self, tokens, topic) -> TopicManagementResponse:
656+
async def unsubscribe_from_topic_async(
657+
self, tokens: Union[str, List[str]], topic: str
658+
) -> TopicManagementResponse:
639659
"""Unsubscribes a list of registration tokens from an FCM topic asynchronously
640660
via the FCM v1 API."""
641661
return await self._make_topic_management_request_v1_async(
@@ -745,20 +765,27 @@ def _get_topic_error_code(cls, error_dict: dict, status_code: int) -> str:
745765
status = error_data.get('status')
746766
if status:
747767
return status
748-
message = error_data.get('message')
749-
if message:
750-
return message
751768

752769
status_map = {
753770
400: 'INVALID_ARGUMENT',
754-
401: 'PERMISSION_DENIED',
771+
401: 'UNAUTHENTICATED',
755772
403: 'PERMISSION_DENIED',
756773
404: 'NOT_FOUND',
774+
408: 'DEADLINE_EXCEEDED',
757775
429: 'RESOURCE_EXHAUSTED',
758776
500: 'INTERNAL',
759-
503: 'DEADLINE_EXCEEDED',
777+
503: 'UNAVAILABLE',
778+
504: 'DEADLINE_EXCEEDED',
760779
}
761-
return status_map.get(status_code, 'UNKNOWN_ERROR')
780+
if status_code in status_map:
781+
return status_map[status_code]
782+
783+
if isinstance(error_data, dict):
784+
message = error_data.get('message')
785+
if message:
786+
return message
787+
788+
return 'UNKNOWN_ERROR'
762789

763790
def _build_topic_subscription_result(self, response, is_subscribe):
764791
"""Constructs a result dict from a response object."""

0 commit comments

Comments
 (0)