feat(fcm): Migrate topic management to FCM v1 API - #785
Merged
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request migrates topic subscription and unsubscription to the FCM v1 API, renaming the legacy methods and introducing new concurrent implementations using a worker pool, alongside updated test coverage. The review feedback highlights critical compilation errors in the new error-handling logic where fe.Response.Status is incorrectly referenced as StatusCode and fe.Response.Body is treated as an io.Reader instead of a []byte slice, which also leaves the io import unused. Additionally, the feedback suggests checking for context cancellation during batch operations and refactoring the test handlers to use simple request counters instead of storing *http.Request pointers, which may not retain their state after the handler returns.
OrlandriaH-G
approved these changes
Sep 21, 2026
lahirumaramba
added a commit
that referenced
this pull request
Sep 21, 2026
…rs in tests Address review feedback on PR #785: - Return ctx.Err() directly if context is cancelled during topic management requests. - Use an integer request counter in test handlers instead of storing http.Request pointers. - Add TestTopicManagementContextCancelled unit test.
yvonnep165
approved these changes
Sep 22, 2026
yvonnep165
left a comment
yvonnep165
left a comment
Contributor
There was a problem hiding this comment.
LGTM with a few comments/questions!
jonathanedey
approved these changes
Sep 22, 2026
lahirumaramba
added a commit
that referenced
this pull request
Sep 22, 2026
Address review feedback from yvonnep165 and jonathanedey on PR #785: - Extract maxTopicManagementWorkers as a named constant. - Use internal.HasSuccessStatus in topic subscription SuccessFn. - Simplify error handling in makeTopicManagementSingleRequest by leveraging parsed FirebaseError. - Remove redundant 409 conflict checks and unused io import. - Add unit test for non-200 2xx success response.
Migrated SubscribeToTopic and UnsubscribeFromTopic to use the FCM v1 API with concurrent worker pool execution. Deprecated legacy IID methods and updated unit tests.
…rs in tests Address review feedback on PR #785: - Return ctx.Err() directly if context is cancelled during topic management requests. - Use an integer request counter in test handlers instead of storing http.Request pointers. - Add TestTopicManagementContextCancelled unit test.
Address review feedback from yvonnep165 and jonathanedey on PR #785: - Extract maxTopicManagementWorkers as a named constant. - Use internal.HasSuccessStatus in topic subscription SuccessFn. - Simplify error handling in makeTopicManagementSingleRequest by leveraging parsed FirebaseError. - Remove redundant 409 conflict checks and unused io import. - Add unit test for non-200 2xx success response.
lahirumaramba
force-pushed
the
lm-fcm-topics
branch
from
September 23, 2026 15:36
137a565 to
4ce2a0f
Compare
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Migrates topic management operations (
SubscribeToTopicandUnsubscribeFromTopic) on the messaging client from the legacy Instance ID (IID) API to the FCM v1 REST API.Key changes:
SubscribeToTopicandUnsubscribeFromTopicnow target FCM v1 endpoints (/projects/{project_id}/registrations/{token}/topicSubscriptions).SubscribeToTopicLegacyandUnsubscribeFromTopicLegacyand marked as deprecated.