Skip to content

Messaging: Support the fid message target - #1138

Open
achretien wants to merge 2 commits into
beste:8.xfrom
sagacorp:feature/messaging-fid-target
Open

achretien wants to merge 2 commits into
beste:8.xfrom
sagacorp:feature/messaging-fid-target

Conversation

@achretien

Copy link
Copy Markdown

The FCM API deprecated a message's token field in favor of fid, which targets a Firebase Installation ID. token stays fully supported until Google decommissions it and currently also accepts FIDs, so this is additive — no action required for existing code.

  • CloudMessage::withFid(), plus fid support in CloudMessage::fromArray(). Like other targets, a FID replaces a previously set one.
  • New FirebaseInstallationId and FirebaseInstallationIds value objects.
  • New MessageTarget::FID.
  • Messaging::sendMulticast() accepts FirebaseInstallationId(s). Strings and string arrays are still registration tokens.
  • MulticastSendReport::validFids() and unknownFids().

Unit tests, PHPStan, Rector and php-cs-fixer pass. Integration tests need a TEST_FIREBASE_INSTALLATION_IDS env var and weren't run locally.

@jeromegamez

Copy link
Copy Markdown
Member

Thank you for taking the time and for adding FID support, I really appreciate it!

However, I don't think it can be merged in its current form.

  • Widening the sendMulticast() method breaks implementers of the interface, renaming the argument breaks usages of named arguments.
  • Adding a new possible MessageTarget::type() and changing the public MessageTarget::TYPES constant can break consumers, e.g. when doing a match ($report->target()->type()) {} without a default case. It's not a hard BC break, but still a compatibility risk
  • sendMulticast() also accepts array|string - this makes it impossible to distinguish registration tokens from FIDs

One possible compatible approach would be a interface like I used it for App Check replay protection, for example:

interface MessagingWithFidMulticast
{
    public function sendMulticastToFids(...): MulticastSendReport;
}

Messaging could implement both contracts while Contract\Messaging remains unchanged. In the next major release we could keep but deprecate sendMulticast() and introduce another sendMulticastToRegistrationTokens(). A migration interface now could be:

interface MessagingWithMulticast
{
	public function sendMulticastToRegistrationTokens(...): MulticastSendReport;
    public function sendMulticastToFids(...): MulticastSendReport;
}

and sendMulticast() would then defer to sendMulticastToRegistrationTokens`.

While researching the changes, I noticed that FCM's deprecated token field already accepts FIDs, so we can use FIDs in the token field (and RegistrationToken class) during the transition, even though the naming becomes inaccurate. This would require no changes for sending, but we can't know if the current app instance and topic subscription handling will continue to work.

The app-instance and topic-management code still uses the legacy Instance ID API. The replacement topic-subscription API accepts registration tokens and FIDs, but does not replace all data returned by getAppInstance(). That's nothing this PR needs to solve though.

Let me know what you think!

Adds `MessageTarget::FID`, `CloudMessage::withFid()`, the
`FirebaseInstallationId(s)` value objects, `MulticastSendReport::validFids()`
and `unknownFids()`, and the optional `Contract\MessagingWithMulticast`
interface with `sendMulticastToRegistrationTokens()` and
`sendMulticastToFids()`.
The Firebase Installation ID counterpart of `validateRegistrationTokens()`,
returning the same `valid`/`unknown`/`invalid` keys. Adds
`MulticastSendReport::invalidFids()` and teaches
`SendReport::messageTargetWasInvalid()` about the `fid` field.
@achretien
achretien force-pushed the feature/messaging-fid-target branch from 7143de5 to e64b9e2 Compare September 21, 2026 12:52
@achretien

Copy link
Copy Markdown
Author

Thanks, all three points were fair. Reworked and force-pushed.

sendMulticast() is back to its exact old signature and param name, Contract\Messaging untouched. It just defers to sendMulticastToRegistrationTokens() now.

The new methods live on a MessagingWithMulticast contract like you suggested, same shape as AppCheckWithReplayProtection.

I also added a validateFids() there, since otherwise there's no FID equivalent of validateRegistrationTokens(). Makes the interface name a bit off, so rename or split it if you'd rather.

On TYPES: it's worse than a compatibility risk, I ran roave-bc-check and the version you reviewed had three breaks, including that constant. So bc-check would have gone red.

TYPES now keeps its old value and there's an ALL_TYPES next to it for the internal single-target check. Branch is at zero breaks now.

That only leaves the match on type().

I kept MessageTarget::FID since you can only ever get a fid target if you called withFid() or sendMulticastToFids() yourself, so nobody gets surprised by it.

But your token field idea works fine too (RegistrationToken doesn't validate anything, so FIDs already go through it today) and only costs the validFids() / unknownFids() helpers. As you said for now the token field accept fid during transition.

One thing I'm unsure about: messageTargetWasInvalid() greps the error message for token, which won't match anything about a fid field, so invalidFids() would always come back empty. I widened it to (token|fid).

@codecov

codecov Bot commented Sep 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.84211% with 3 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

❗ There is a different number of reports uploaded between BASE (02dd9b6) and HEAD (e64b9e2). Click for more details.

HEAD has 3 uploads less than BASE
Flag BASE (02dd9b6) HEAD (e64b9e2)
grpc 1 0
emulator 1 0
integration 1 0

Impacted file tree graph

@@              Coverage Diff              @@
##                8.x    #1138       +/-   ##
=============================================
- Coverage     87.86%   52.33%   -35.53%     
- Complexity     1507     1544       +37     
=============================================
  Files           157      159        +2     
  Lines          4234     4305       +71     
=============================================
- Hits           3720     2253     -1467     
- Misses          514     2052     +1538     

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants