feat(fcm): Migrate topic management to FCM v1 API - #980
lahirumaramba wants to merge 4 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates the topic management functionality in the Firebase Cloud Messaging module from the legacy Instance ID (IID) API to the FCM v1 API, introducing new synchronous and asynchronous methods (subscribe_to_topic, subscribe_to_topic_async, unsubscribe_from_topic, and unsubscribe_from_topic_async) while deprecating the legacy versions. The feedback highlights a potential concurrency race condition in the multi-threaded request execution where a shared headers dictionary is mutated, and suggests extracting duplicated error-handling logic into a single helper method to improve maintainability.
e065d10 to
be27016
Compare
…ic error parsing - Pass a copy of self._fcm_headers in topic management requests to prevent concurrent mutation race conditions in ThreadPoolExecutor. - Extract common error parsing logic from _build_topic_subscription_result_from_requests_error and _build_topic_subscription_result_from_httpx_error into _build_topic_subscription_result.
- 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.
- 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
b4e98ba to
3c654a0
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request migrates the topic subscription and unsubscription features to the FCM v1 API, introducing both synchronous and asynchronous implementations (subscribe_to_topic, subscribe_to_topic_async, unsubscribe_from_topic, and unsubscribe_from_topic_async). The previous Instance ID API implementations have been renamed to subscribe_to_topic_legacy and unsubscribe_from_topic_legacy and marked as deprecated. Comprehensive unit tests have been added to cover the new functionality, validation rules, and error handling. There are no review comments to address, and I have no additional feedback to provide.
2 participants
Migrates
subscribe_to_topicandunsubscribe_from_topicin the messaging module from the legacy Instance ID (IID) API to the FCM v1 Topic Subscriptions API.Key changes:
subscribe_to_topicandunsubscribe_from_topicto call the FCM v1 endpoints (/v1/projects/{projectId}/registrations/{token}/topicSubscriptions).subscribe_to_topic_asyncandunsubscribe_from_topic_asyncutilizingHttpxAsyncClient(HTTP/2).subscribe_to_topic_legacyandunsubscribe_from_topic_legacywith deprecation warnings.ALREADY_EXISTS(HTTP 409) is treated as success for topic subscriptions, whileNOT_FOUND(HTTP 404) is recorded as a failure for topic unsubscriptions.