Conversation
|
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.
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 interface MessagingWithMulticast
{
public function sendMulticastToRegistrationTokens(...): MulticastSendReport;
public function sendMulticastToFids(...): MulticastSendReport;
}and While researching the changes, I noticed that FCM's deprecated token field already accepts FIDs, so we can use FIDs in the 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 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.
7143de5 to
e64b9e2
Compare
|
Thanks, all three points were fair. Reworked and force-pushed.
The new methods live on a I also added a On
That only leaves the I kept But your One thing I'm unsure about: |
Codecov Report❌ Patch coverage is
@@ 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 |
The FCM API deprecated a message's
tokenfield in favor offid, which targets a Firebase Installation ID.tokenstays fully supported until Google decommissions it and currently also accepts FIDs, so this is additive — no action required for existing code.CloudMessage::withFid(), plusfidsupport inCloudMessage::fromArray(). Like other targets, a FID replaces a previously set one.FirebaseInstallationIdandFirebaseInstallationIdsvalue objects.MessageTarget::FID.Messaging::sendMulticast()acceptsFirebaseInstallationId(s). Strings and string arrays are still registration tokens.MulticastSendReport::validFids()andunknownFids().Unit tests, PHPStan, Rector and php-cs-fixer pass. Integration tests need a
TEST_FIREBASE_INSTALLATION_IDSenv var and weren't run locally.