chore: [SDK-4783] deprecate blocking OneSignal APIs in favor of suspend variants

[abdulraqeeb33]

Description

One Line Summary

Mark the blocking (non-suspend) OneSignal public APIs as @Deprecated, pointing each to its suspend equivalent.

Details

Motivation

Closes SDK-4783.

The blocking APIs (initWithContext, login, logout, updateUserJwt, the manager accessors, and the config properties) can block the calling thread until the SDK is initialized, which risks ANRs when invoked on the main thread. The SDK already exposes suspend equivalents for all of them. This PR formally deprecates the blocking variants so developers get a compile-time signal (with an IDE quick-fix via ReplaceWith) and a runtime log nudging them toward the suspend APIs.

Scope

Deprecations are applied consistently across all three layers:

• IOneSignal (interface contract)
• OneSignalImp (implementation overrides — also where the runtime Logging.warn deprecation notices live for the blocking methods)
• OneSignal (public static object app developers call)

Deprecated members and their replacements:

Deprecated (blocking)	Replacement (suspend)
initWithContext(context, appId)	initWithContextSuspend(context, appId)
login(externalId) / login(externalId, jwtBearerToken)	loginSuspend(...)
logout()	logoutSuspend()
updateUserJwt(externalId, token)	updateUserJwtSuspend(externalId, token)
User / Session / Notifications / Location / InAppMessages	getUserSuspend() / getSessionSuspend() / getNotificationsSuspend() / getLocationSuspend() / getInAppMessagesSuspend()
consentRequired / consentGiven / disableGMSMissingPrompt	getX Suspend() / setX Suspend(...)

Not changed:

• The internal-only suspend initWithContext(context) path used by the SDK itself.
• debug (no suspend equivalent; usable before init).
• Property accessors deliberately do not emit runtime logs (they can be read frequently and would flood logs); the @Deprecated annotation is the signal there.

Notes

• Deprecation level is WARNING (not ERROR) so the SDK's own unit tests that exercise the blocking APIs still compile.
• All internal SDK callers already use the suspend variants, so main source compiles with zero new deprecation warnings.

Testing

Unit testing

No new tests; this is an annotation/logging change. Existing tests that call the blocking APIs continue to compile and pass (they now surface expected deprecation warnings).

Manual testing

Verified :OneSignal:core, :OneSignal:notifications, :OneSignal:location, and :OneSignal:in-app-messages compile successfully (compileDebugKotlin) with no new deprecation warnings in SDK source.

Affected code checklist

• 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

[sherwinski]

@claude review once

[claude]

LGTM — straightforward deprecation pass; the inline nits are non-blocking message/UX polish that can be addressed in a follow-up.

Extended reasoning...

Overview

This PR adds @Deprecated annotations to the blocking public APIs (initWithContext, login, logout, updateUserJwt, the manager accessors, and the three config properties) across IOneSignal, OneSignalImp, and OneSignal, with ReplaceWith quick-fixes pointing at the existing suspend equivalents. The only behavior delta is that four pre-existing deprecation log calls in OneSignalImp are promoted from Logging.log(LogLevel.DEBUG, ...) to Logging.warn(...) and have their text rephrased. Deprecation level is WARNING, so callers (including the SDK's own tests) still compile.

Security risks

None. No auth/crypto/permission logic is touched. The promoted log lines on login/updateUserJwt still mask the JWT (no full token strings logged). No new data flows or persistence behavior.

Level of scrutiny

Low. The semantic surface is unchanged — these are pure annotation additions plus log-level/text edits. The two suspend-equivalent APIs being pointed at already exist and are exercised by internal callers, so the ReplaceWith targets are real and tested. The PR author verified compileDebugKotlin on :core, :notifications, :location, and :in-app-messages with no new deprecation warnings in SDK source.

Other factors

Two nit-level findings were posted as inline comments: (1) property-level @Deprecated on the three var properties applies to both accessors, so the setter IDE quick-fix rewrites to getX Suspend() = value (doesn't compile) and the "may block / cause ANRs" message overstates harm for the non-blocking setters; (2) the 1-arg login(externalId) path routes through the 2-arg override's runtime WARN, so Java callers of the 1-arg form see a message naming jwtBearerToken. Both are message/UX polish on deprecation guidance — no runtime impact, easily addressed in a follow-up. Given the PR's narrow scope and the nature of the findings, shadow-approving.

[claude]

🟡 The three deprecated var properties (consentRequired, consentGiven, disableGMSMissingPrompt) carry a single property-level @Deprecated annotation, which Kotlin applies to both the getter and the setter. This causes two related issues for setter callers: (1) the "may block the calling thread… cause ANRs" message is inaccurate — the setters are non-blocking direct field writes (OneSignalImp.kt lines 89-94, 110-122, 134-140), and (2) the ReplaceWith only encodes the getter form, so applying the IDE quick-fix to OneSignal.consentRequired = true rewrites to OneSignal.getConsentRequiredSuspend() = true, which doesn'''t compile. Fix by splitting into @get:Deprecated / @set:Deprecated with accessor-specific messages and ReplaceWith expressions (e.g. setConsentRequiredSuspend(value) for the setter); the same pattern needs to be applied across OneSignal.kt, IOneSignal.kt, and OneSignalImp.kt.

Extended reasoning...

What the bug is

For the three deprecated mutable properties — consentRequired, consentGiven, and disableGMSMissingPrompt — the @Deprecated annotation is placed at the property level. In Kotlin, a property-level annotation on a var applies the same message and ReplaceWith to both the getter and the setter. That is fine for vals and for the blocking funs in this PR, but it produces two distinct problems for these vars.

Problem 1 — ReplaceWith quick-fix produces invalid code on setter usage

Each property uses a getter-only ReplaceWith expression, e.g. in OneSignal.kt:

replaceWith = ReplaceWith("getConsentRequiredSuspend()"),

When a developer applies the IntelliJ/Android Studio quick-fix to a setter usage like:

OneSignal.consentRequired = true

the IDE mechanically substitutes the LHS using the ReplaceWith expression, producing:

OneSignal.getConsentRequiredSuspend() = true   // does not compile — assigning to a function call

This affects setter calls that already exist in the codebase, e.g. examples/.../MainApplication.kt:65 (OneSignal.consentRequired = SharedPreferenceUtil.getCachedConsentRequired(this)), OneSignalRepository.kt:222, and the OneSignalImpTests, so it is not hypothetical.

Problem 2 — message overstates harm for setter callers

The message claims:

Accessing this property may block the calling thread until the SDK is initialized and cause ANRs when called on the main thread.

But the setters in OneSignalImp.kt never block. Reviewing the implementation:

set(value) {
    _consentRequired = value
    if (isInitialized) { configModel.consentRequired = value }
}

This writes a local backing field and (when initialized) the in-memory ConfigModel directly. None of consentRequired, consentGiven, or disableGMSMissingPrompt setters invoke blockingGet, runBlocking, or waitForInit. The only non-trivial side effect, operationRepo.forceExecuteOperations() in the consentGiven setter, just calls wake() on two waiters — also non-blocking. Only the getters route through blockingGet { ... }, which is where the ANR risk actually lives.

A common, legitimate calling pattern looks like:

OneSignal.consentRequired = true
OneSignal.initWithContext(context, appId)

The write here cannot block on init (init has not run yet — the setter falls through to the _consentRequired = value no-init branch). Telling that caller they risk ANRs is misleading.

Step-by-step proof — Problem 1

1. A developer has OneSignal.consentRequired = true somewhere in their app.
2. After this PR ships, the compiler reports a deprecation warning on that line.
3. The developer clicks the IDE "Replace with" intention.
4. The IDE looks up the ReplaceWith text — "getConsentRequiredSuspend()" — and substitutes the property reference on the LHS.
5. The result is OneSignal.getConsentRequiredSuspend() = true, which fails to compile (error: variable expected).
6. The deprecation message text already mentions both getConsentRequiredSuspend() and setConsentRequiredSuspend(required), so a reader can manually correct it — but the quick-fix UX is broken.

How to fix

Replace the property-level annotation with accessor-targeted ones, e.g. in OneSignal.kt:

@get:Deprecated(
    message = "Reading this property may block the calling thread until the SDK is initialized " +
        "and cause ANRs when called on the main thread. Use the suspend function getConsentRequiredSuspend() instead.",
    replaceWith = ReplaceWith("getConsentRequiredSuspend()"),
)
@set:Deprecated(
    message = "Use the suspend function setConsentRequiredSuspend(required) instead.",
    replaceWith = ReplaceWith("setConsentRequiredSuspend(value)"),
)
@JvmStatic
@Suppress("DEPRECATION")
var consentRequired: Boolean
    get() = oneSignal.consentRequired
    set(value) { oneSignal.consentRequired = value }

The same shape needs to be applied in IOneSignal.kt (with the non-suspend names setConsentRequired(required)) and in OneSignalImp.kt. With accessor-targeted annotations, the IDE quick-fix on a setter usage rewrites to OneSignal.setConsentRequiredSuspend(value) (which compiles), and the messages can be honest per accessor.

Severity / impact

No runtime impact — SDK behavior is unchanged. Compilation of existing user code is unaffected (the warning level is WARNING). What is broken is the IDE-assisted migration UX, plus message accuracy. Since the readable deprecation message does name the correct setter function, a developer who reads the warning can migrate manually. Filing as a nit.

[claude]

🟡 When a caller invokes the 1-arg OneSignal.login("user"), the runtime emits a WARN that names the 2-arg signature: login(externalId, jwtBearerToken) is deprecated ... Use loginSuspend(externalId, jwtBearerToken) instead — referencing a parameter (jwtBearerToken) the caller never passed. Java callers don't see Kotlin @Deprecated messages, so this runtime log is the only signal they get, and it diverges from the IDE quick-fix on the 1-arg overload (which correctly says loginSuspend(externalId)). Easy fix: emit the warn from the 1-arg IOneSignal default impl (or add a 1-arg OneSignalImp override that logs and forwards) so the message matches the call shape.

Extended reasoning...

Call path

1. OneSignal.login("user") — the 1-arg @JvmStatic at OneSignal.kt:381 — calls oneSignal.login(externalId).
2. Kotlin overload resolution prefers the 1-arg overload (no defaulted parameter) over the 2-arg with a default. That resolves to the IOneSignal default impl at IOneSignal.kt:200:

   @Deprecated(... ReplaceWith("loginSuspend(externalId)"))
   @Suppress("DEPRECATION")
   fun login(externalId: String) = login(externalId, null)

3. That delegates to login(externalId, null), which dispatches to OneSignalImp's 2-arg override fun login(externalId, jwtBearerToken) at OneSignalImp.kt:513-522. OneSignalImp does not override the 1-arg form (grep for override fun login(externalId: String) returns no matches), so all 1-arg traffic lands here.
4. The 2-arg override emits:

   login(externalId, jwtBearerToken) is deprecated and should no longer be used.
   Use the suspend function loginSuspend(externalId, jwtBearerToken) instead.
   

Why existing code doesn't prevent it

The compile-time @Deprecated on the 1-arg OneSignal.login at OneSignal.kt:374 correctly references loginSuspend(externalId). But Kotlin's @Deprecated messages are an annotation-level signal — they're not surfaced to Java callers at compile time (Java sees only the synthetic @java.lang.Deprecated, not the message string), and they're never emitted at runtime. The runtime log is the only signal a Java caller actually observes, and it's the 2-arg message.

Step-by-step proof

Imagine a Java app:

OneSignal.login("alice");

• Compile time (Java): no message visible — Java sees only @Deprecated-the-marker, not the Kotlin message or replaceWith.
• Runtime: the Logging.warn at OneSignalImp.kt:519-522 fires with text mentioning jwtBearerToken and a replacement of loginSuspend(externalId, jwtBearerToken) — neither of which appears in the user's call. A developer reading this in logcat sees a parameter they didn't pass and is told to use a 2-arg replacement instead of the 1-arg loginSuspend(externalId) form documented on the API they actually invoked.

Impact

Cosmetic / mildly confusing. The corrective steer — "switch to loginSuspend" — is still conveyed, and loginSuspend(externalId, jwtBearerToken = null) is functionally equivalent (the suspend variant defaults jwtBearerToken to null). The inconsistency surfaces only for Java callers of the 1-arg form, and only at the message-string level; behavior is unchanged.

Suggested fix

Either:

• Add a 1-arg override in OneSignalImp that logs a 1-arg-specific message and forwards to the 2-arg form:

  override fun login(externalId: String) {
      Logging.warn(
          "login(externalId) is deprecated and should no longer be used. " +
              "Use the suspend function loginSuspend(externalId) instead.",
      )
      login(externalId, null)
  }

  (The 2-arg override's existing warn would still fire on the recursive call — to avoid double-logging, gate it via an internal helper that skips the warn, or have the 2-arg override skip the warn when called from this path.)
• Or have the IOneSignal default impl emit its own log before delegating, and pre-log in the 2-arg path conditionally.
• Or simply normalize the existing message to login(...) is deprecated; use loginSuspend(...) instead so the parameter list isn't claimed for either call shape.