Skip to content

fix: [SDK-4946] register with Firebase Installation ID when the legacy FCM token API is disabled - #2734

Merged
fadi-george merged 6 commits into
firebase-installation-idsfrom
fadi/SDK-4946-fcm-fid-registration
Sep 4, 2026
Merged

fix: [SDK-4946] register with Firebase Installation ID when the legacy FCM token API is disabled#2734
fadi-george merged 6 commits into
firebase-installation-idsfrom
fadi/SDK-4946-fcm-fid-registration

Conversation

@fadi-george

Copy link
Copy Markdown
Contributor

Description

One Line Summary

Restore Firebase Installation ID registration when an app disables the legacy FCM token API.

Details

Motivation

Firebase Messaging 25.1.0 allows apps and dependencies to enable firebase_messaging_installation_id_enabled, which disables the legacy token API process-wide. OneSignal's getToken() then throws IllegalStateException: API disabled, preventing push registration.

This remakes #2700 after its temporary revert in #2719. It retains the original fallback and diagnostics, and now mirrors Firebase Messaging's sender-ID fallback for programmatically configured default Firebase apps.

Scope

Only the FCM registration path changes when Firebase reports that the legacy API is disabled. Existing integrations continue using getToken().

FID registration uses the host app's default FirebaseApp, because its sender ID, app ID, and API key belong to one Firebase project. Apps enabling FID registration must provide Firebase configuration whose sender ID matches the OneSignal dashboard.

FirebaseMessaging.register() remains reflective so the SDK can compile against Firebase Messaging 24.0.0 while consumers resolve 25.1.0 or newer. The consumer rule preserves this method through minification.

Testing

Unit testing

  • Covers legacy-token and FID paths, diagnostics, registration failures, reflection behavior, and Firebase's application-ID sender fallback.
  • Verifies successful wiring invokes registration and Firebase Installations through the host default FirebaseApp.
  • ./gradlew spotlessCheck detekt :OneSignal:notifications:testDebugUnitTest :OneSignal:notifications:assembleRelease --console=plain
  • Focused FCM tests rerun after review fixes.

Manual testing

The original implementation was manually verified on an Android 16 emulator with Firebase Messaging 25.1.1 in both debug and minified release builds, including delivery and legacy-token/FID conversion. This remake restores that implementation; the added sender fallback is covered by unit tests and was not re-tested on-device.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
  • I have reviewed this PR myself, ensuring it meets each checklist item

Made with Cursor

@fadi-george
fadi-george requested a review from a team as a code owner September 3, 2026 22:18
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

📊 Diff Coverage Report

Diff Coverage Report (Changed Lines Only)

Gate: aggregate coverage on changed executable lines must be ≥ 80% (JaCoCo line data for lines touched in the diff).

Changed Files Coverage

  • PushRegistratorFCM.kt: 78/78 touched executable lines (100.0%) (176 touched lines in diff)

Overall (aggregate gate)

78/78 touched executable lines covered (100.0% — requires ≥ 80%)

📥 View workflow run

@cursor cursor 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.

Multi-model review

Claude Opus 5, GPT 5.6 Sol, and Cursor Grok 4.6 reviewed the FID fallback (5 files, +463/−9). No blocking issues. The happy path is sound: keep using named-app getToken() until Firebase throws the V1 “API disabled” ISE, then register() + FID on the host default FirebaseApp only when its sender matches.

Act on

  • Named app is still created when V1 is the only viable path (Opus + Grok). getToken always initializes ONESIGNAL_SDK_FCM_APP_NAME (dashboard sender + OneSignal shared credentials) and calls .token before the default-app FID path. With the legacy API disabled process-wide that call is guaranteed to fail, and constructing FirebaseMessaging on the mixed app can still kick a background register against credentials this PR says Firebase rejects (#2700 saw opStatusCode=8 / topic-sync noise from that app). If the flag is on and register() exists, skip named-app init and go straight to defaultAppRegistration().

Consider

  • FID rotation (GPT; Opus adjacent). register() on the host default app can fire the host onRegistered callback and does not consult isAutoInitEnabled(). OneSignal has no FCM onNewToken/onRegistered bridge, so a later FID change is not uploaded until another SDK registration.
  • Fragile disable detection + no 25.1 fixture (3/3). Fallback is a substring match on "API disabled" + "register()", and reflection/ProGuard are only tested against hand-rolled stand-ins while the module resolves 24.0.0. A wording or signature change ships as FIREBASE_FCM_ERROR_MISC_EXCEPTION with no retry.
  • Diagnostics land in the misc bucket (Opus + Grok). Sender mismatch / missing default app (common when a dependency injects the flag and the dashboard is still on the shared sender) is a permanent config error logged as “Unknown error getting FCM Token”.
  • FidEnv vs this path (Opus). Census still uses getManifestMetaBoolean + gcm_defaultSenderId; registration uses a raw meta read + FirebaseApp.options. Programmatic default apps can report flag=0/snd=0 even when this path would succeed.

Dismissed

  • FID uploaded as a normal push token — that is the feature contract.
  • defaultSenderId("") not falling back to the applicationId parse — matches Firebase Metadata.getDefaultSenderId.
  • Gradle require '[23.0.8, 24.0.99]' — pre-existing; require is not a hard upper bound.
  • Tasks.await without timeout and the unchecked as Task<*> — same await as the existing token path; the cast is a nit.
Open in Web View Automation 

Sent by Cursor Automation: PR Reviews

@fadi-george
fadi-george changed the base branch from main to firebase-installation-ids September 3, 2026 22:52
@fadi-george
fadi-george force-pushed the fadi/SDK-4946-fcm-fid-registration branch from 73cff6b to d328c38 Compare September 3, 2026 23:49

@abdulraqeeb33 abdulraqeeb33 left a comment

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.

Looks right. Flag off stays on the named app. FID only uses the host default FirebaseApp, and only when the sender matches.

@abdulraqeeb33 abdulraqeeb33 left a comment

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.

nit and a question

fadi-george and others added 6 commits September 4, 2026 12:51
…I is disabled

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@fadi-george
fadi-george force-pushed the fadi/SDK-4946-fcm-fid-registration branch from b862679 to f21caa0 Compare September 4, 2026 19:51
@fadi-george
fadi-george merged commit 5e8bccb into firebase-installation-ids Sep 4, 2026
5 checks passed
@fadi-george
fadi-george deleted the fadi/SDK-4946-fcm-fid-registration branch September 4, 2026 20:05
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