Skip to content

feat(fcm): Enable fid and deprecate token for Send API#525

Open
yvonnep165 wants to merge 6 commits into
mainfrom
yp-add-fid-arg
Open

feat(fcm): Enable fid and deprecate token for Send API#525
yvonnep165 wants to merge 6 commits into
mainfrom
yp-add-fid-arg

Conversation

@yvonnep165

@yvonnep165 yvonnep165 commented Jun 5, 2026

Copy link
Copy Markdown

This change deprecates Token (in Message) and Tokens (in MulticastMessage) as message targets, transitioning support to Fid (Firebase Installation ID) and Fids as the primary targets instead.

Legacy token tests are wrapped in Method-Level #pragma warning disable CS0618 to prevent compiler warnings and build crash and #pragma wrappers are explicitly maintained for snippet documentation code to allow compiling flawlessly.

@yvonnep165 yvonnep165 self-assigned this Jun 5, 2026
@yvonnep165 yvonnep165 added release-note release:stage Stage a release candidate labels Jun 5, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for Firebase Installation IDs (FIDs) in Firebase Cloud Messaging (FCM) by adding the Fid property to Message and the Fids property to MulticastMessage, while deprecating the older Token and Tokens properties. It updates validation, copying, and serialization logic to support these new fields, adds comprehensive unit and integration tests, and suppresses deprecation warnings where necessary. The review feedback suggests improving consistency in exception messages by capitalizing property names (Tokens and Fids) and ensuring proper punctuation.

Comment thread FirebaseAdmin/FirebaseAdmin/Messaging/MulticastMessage.cs Outdated
Comment thread FirebaseAdmin/FirebaseAdmin/Messaging/MulticastMessage.cs Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we update the comments for SendEachForMulticastAsync to mention the message response order in BatchResponse when both Tokens and FIDs are set in MulticastMessage?

@yvonnep165 yvonnep165 Jun 8, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! Updated comments for both SendEachForMulticastAsync and SendMulticastAsync

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we update this test file to include some tests using fid messages?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MessageTest.cs is already handling the payload serialization/deserialization logic for FIDs. FirebaseMessagingClientTest.cs is responsible for testing the HTTP transport layer. It typically uses a generic Topic or Token payload just as a dummy placeholder to verify that the HTTP request is built correctly. Since we already have full FID coverage in MessageTest.cs and the FirebaseMessagingTest.cs integration tests, it seems a bit redundant to add some tests using fid messages in FirebaseMessagingClientTest.cs. But please let me know if you'd still prefer I add a dummy FID payload test there for completeness!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release:stage Stage a release candidate release-note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants