From f50631dd9fd8d25e3d43f93d26c09c5f7abb2fff Mon Sep 17 00:00:00 2001 From: amarose Date: Mon, 28 Sep 2026 11:21:11 +0200 Subject: [PATCH 1/2] fix(push): prevent lost notifications and missing images --- .../sdk/push/PushNotificationDelegate.kt | 141 +++++++++++------ .../sdk/push/PushNotificationDelegateTest.kt | 148 ++++++++++++++++++ 2 files changed, 238 insertions(+), 51 deletions(-) create mode 100644 library/src/test/java/com/pushpushgo/sdk/push/PushNotificationDelegateTest.kt diff --git a/library/src/main/java/com/pushpushgo/sdk/push/PushNotificationDelegate.kt b/library/src/main/java/com/pushpushgo/sdk/push/PushNotificationDelegate.kt index 7668113a..8db6a7c5 100644 --- a/library/src/main/java/com/pushpushgo/sdk/push/PushNotificationDelegate.kt +++ b/library/src/main/java/com/pushpushgo/sdk/push/PushNotificationDelegate.kt @@ -7,6 +7,7 @@ import android.content.Context import android.content.Intent import android.graphics.Bitmap import android.net.Uri +import android.os.Looper import android.os.SystemClock import androidx.core.app.NotificationCompat import androidx.core.app.NotificationManagerCompat @@ -21,6 +22,7 @@ import com.pushpushgo.sdk.push.liveactivity.data.LiveActivityPayloadParser import com.pushpushgo.sdk.utils.* import com.pushpushgo.sdk.work.UploadDelegate import kotlinx.coroutines.* +import java.io.IOException import kotlin.random.Random internal class PushNotificationDelegate( @@ -80,32 +82,54 @@ internal class PushNotificationDelegate( return } - delegateScope.launch(errorHandler) { - val notificationId = getNotificationId(pushMessage.data["nId"] ?: "default") - logDebug("Notification ID: $notificationId") + if (Looper.myLooper() == Looper.getMainLooper()) { + // Blocking the main thread would risk an ANR, so integrations that forward + // messages from the main thread keep the asynchronous path. + delegateScope.launch(errorHandler) { showNotification(pushMessage, context, notificationManager) } + return + } - val notification = - when { - pushMessage.data.isNotEmpty() -> - getDataNotification( - context = context, - remoteMessage = pushMessage, - notificationId = notificationId, - ) + // Messaging services deliver onMessageReceived on a worker thread and stop the + // service as soon as it returns. Returning before the notification was posted + // let the service be destroyed mid image download, which dropped the + // notification (or its image), so wait for it here. + try { + runBlocking { showNotification(pushMessage, context, notificationManager) } + } catch (e: Throwable) { + logError(e) + } + } - pushMessage.notification != null -> - getSimpleNotification( - context = context, - remoteMessage = pushMessage, - notificationId = notificationId, - ) + @SuppressLint("MissingPermission") + private suspend fun showNotification( + pushMessage: PushMessage, + context: Context, + notificationManager: NotificationManagerCompat, + ) { + val notificationId = getNotificationId(pushMessage.data["nId"] ?: "default") + logDebug("Notification ID: $notificationId") - else -> throw IllegalStateException("Unknown notification type") - } + val notification = + when { + pushMessage.data.isNotEmpty() -> + getDataNotification( + context = context, + remoteMessage = pushMessage, + notificationId = notificationId, + ) - notificationManager.notify(notificationId, notification) - logDebug("Notification sent: $notificationId => $notification") - } + pushMessage.notification != null -> + getSimpleNotification( + context = context, + remoteMessage = pushMessage, + notificationId = notificationId, + ) + + else -> throw IllegalStateException("Unknown notification type") + } + + notificationManager.notify(notificationId, notification) + logDebug("Notification sent: $notificationId => $notification") } fun onNewToken(token: String) { @@ -117,7 +141,9 @@ internal class PushNotificationDelegate( } fun onDestroy() { - job.cancelChildren() + // Intentionally does not cancel in-flight work: the service is destroyed right + // after onMessageReceived returns, and cancelling here dropped notifications + // mid image download. Pending work is bounded by IMAGE_DOWNLOAD_BUDGET_MS. } private fun getUniqueNotificationId() = Random.nextInt(0, Int.MAX_VALUE) @@ -211,39 +237,48 @@ internal class PushNotificationDelegate( if (url.isNullOrBlank()) return null val startedAt = SystemClock.elapsedRealtime() - try { - val bitmap = - withTimeoutOrNull(BITMAP_TIMEOUT_MS) { - withContext(Dispatchers.IO) { - PushPushGo.getInstance().getNetwork().getBitmapFromUrl(url) - } - } - val elapsedMs = SystemClock.elapsedRealtime() - startedAt - - // Diagnostics for missing notification images: withTimeoutOrNull() swallows - // the timeout and returns null, so without this the notification is posted - // without a picture and nothing at all shows up in the logs. - return when { - bitmap != null -> { - logDebug("Bitmap downloaded in ${elapsedMs}ms (${bitmap.width}x${bitmap.height}): $url") - bitmap - } + val bitmap = withTimeoutOrNull(IMAGE_DOWNLOAD_BUDGET_MS) { downloadBitmapWithRetry(url, startedAt) } - elapsedMs >= BITMAP_TIMEOUT_MS -> { - logWarning("Bitmap download timed out after ${elapsedMs}ms (limit ${BITMAP_TIMEOUT_MS}ms): $url") - null - } + // withTimeoutOrNull() swallows the timeout and returns null, so log it here; + // otherwise the notification is posted without a picture and nothing shows up in the logs. + val elapsedMs = SystemClock.elapsedRealtime() - startedAt + if (bitmap == null && elapsedMs >= IMAGE_DOWNLOAD_BUDGET_MS) { + logWarning("Bitmap download timed out after ${elapsedMs}ms (limit ${IMAGE_DOWNLOAD_BUDGET_MS}ms): $url") + } + return bitmap + } - else -> { + private suspend fun downloadBitmapWithRetry( + url: String, + startedAt: Long, + ): Bitmap? { + var attempt = 0 + while (true) { + attempt++ + try { + val bitmap = withContext(Dispatchers.IO) { PushPushGo.getInstance().getNetwork().getBitmapFromUrl(url) } + val elapsedMs = SystemClock.elapsedRealtime() - startedAt + if (bitmap == null) { logWarning("Bitmap decoded to null after ${elapsedMs}ms: $url") - null + } else { + logDebug("Bitmap downloaded in ${elapsedMs}ms (${bitmap.width}x${bitmap.height}, attempt $attempt): $url") } + return bitmap + } catch (e: CancellationException) { + throw e + } catch (e: IOException) { + // Right after a cold start the app's network can still be blocked (DNS fails + // until the FCM network allowlist is applied), so network errors are retried + // until the download budget runs out. + val retryInMs = RETRY_DELAYS_MS.getOrElse(attempt - 1) { RETRY_DELAYS_MS.last() } + val elapsedMs = SystemClock.elapsedRealtime() - startedAt + logWarning("Bitmap download attempt $attempt failed after ${elapsedMs}ms, retrying in ${retryInMs}ms: $url ($e)") + delay(retryInMs) + } catch (e: Throwable) { + logError("Failed to download bitmap picture after ${SystemClock.elapsedRealtime() - startedAt}ms: $url", e) + return null } - } catch (e: Throwable) { - logError("Failed to download bitmap picture after ${SystemClock.elapsedRealtime() - startedAt}ms: $url", e) } - - return null } private fun createNotification( @@ -401,7 +436,11 @@ internal class PushNotificationDelegate( ) companion object { - private const val BITMAP_TIMEOUT_MS = 10_000L + // Shared by the image and icon downloads (they run concurrently). Kept well + // below the time Firebase allows for handling a message, since + // onMessageReceived now waits for the notification to be posted. + private const val IMAGE_DOWNLOAD_BUDGET_MS = 8_000L + private val RETRY_DELAYS_MS = listOf(250L, 500L, 1_000L, 2_000L) const val NOTIFICATION_ID_EXTRA = "notification_id" const val CAMPAIGN_ID_EXTRA = "campaign" diff --git a/library/src/test/java/com/pushpushgo/sdk/push/PushNotificationDelegateTest.kt b/library/src/test/java/com/pushpushgo/sdk/push/PushNotificationDelegateTest.kt new file mode 100644 index 00000000..b0b717d1 --- /dev/null +++ b/library/src/test/java/com/pushpushgo/sdk/push/PushNotificationDelegateTest.kt @@ -0,0 +1,148 @@ +package com.pushpushgo.sdk.push + +import android.app.Notification +import android.app.NotificationManager +import android.content.Context +import android.graphics.Bitmap +import android.os.Looper +import androidx.test.core.app.ApplicationProvider.getApplicationContext +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.pushpushgo.sdk.PushPushGo +import com.pushpushgo.sdk.network.ApiRepository +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkObject +import io.mockk.unmockkAll +import kotlinx.coroutines.CompletableDeferred +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.Shadows.shadowOf +import org.robolectric.annotation.Config +import java.net.UnknownHostException +import java.util.concurrent.CountDownLatch +import kotlin.concurrent.thread + +@RunWith(AndroidJUnit4::class) +@Config(sdk = [33]) +class PushNotificationDelegateTest { + private val context: Context = getApplicationContext() + private val api = mockk(relaxed = true) + private val bitmap = Bitmap.createBitmap(10, 5, Bitmap.Config.ARGB_8888) + + @Before + fun setUp() { + val pushPushGo = mockk(relaxed = true) + every { pushPushGo.isPPGoPush(any>()) } returns true + every { pushPushGo.getProjectId() } returns PROJECT_ID + every { pushPushGo.getNetwork() } returns api + coEvery { api.getBitmapFromUrl(ICON_URL) } returns null + + mockkObject(PushPushGo.Companion) + every { PushPushGo.getInstance() } returns pushPushGo + every { PushPushGo.isInitialized() } returns true + } + + @After + fun tearDown() = unmockkAll() + + @Test + fun `notification is posted with image when service is destroyed mid download`() { + val downloadStarted = CountDownLatch(1) + val finishDownload = CompletableDeferred() + coEvery { api.getBitmapFromUrl(IMAGE_URL) } coAnswers { + downloadStarted.countDown() + finishDownload.await() + bitmap + } + val delegate = PushNotificationDelegate(context) + + // Messaging services call onMessageReceived on a worker thread. + val worker = thread { delegate.onMessageReceived(pushMessage(), context) } + pumpMainLooperUntil { downloadStarted.count == 0L } + assertEquals("image download did not start", 0L, downloadStarted.count) + // The service gets destroyed while the image is still downloading. + delegate.onDestroy() + finishDownload.complete(Unit) + pumpMainLooperUntil { !worker.isAlive && postedNotifications().isNotEmpty() } + + assertEquals(BIG_PICTURE_STYLE, postedNotification().extras.getString(Notification.EXTRA_TEMPLATE)) + } + + @Test + fun `image download is retried after transient network error`() { + var attempts = 0 + coEvery { api.getBitmapFromUrl(IMAGE_URL) } coAnswers { + if (attempts++ == 0) throw UnknownHostException("Unable to resolve host") + bitmap + } + + deliver(PushNotificationDelegate(context)) + + assertEquals(BIG_PICTURE_STYLE, postedNotification().extras.getString(Notification.EXTRA_TEMPLATE)) + coVerify(exactly = 2) { api.getBitmapFromUrl(IMAGE_URL) } + } + + @Test + fun `notification is posted without image when image cannot be downloaded`() { + coEvery { api.getBitmapFromUrl(IMAGE_URL) } throws IllegalStateException("HTTP 404") + + deliver(PushNotificationDelegate(context)) + + assertEquals(BIG_TEXT_STYLE, postedNotification().extras.getString(Notification.EXTRA_TEMPLATE)) + coVerify(exactly = 1) { api.getBitmapFromUrl(IMAGE_URL) } + } + + /** Delivers the message the way a messaging service does (on a worker thread) and waits for the notification. */ + private fun deliver(delegate: PushNotificationDelegate) { + val worker = thread { delegate.onMessageReceived(pushMessage(), context) } + pumpMainLooperUntil { !worker.isAlive && postedNotifications().isNotEmpty() } + } + + /** Also runs work dispatched to the main thread, which Robolectric only executes when its looper is idled. */ + private fun pumpMainLooperUntil(condition: () -> Boolean) { + val deadline = System.currentTimeMillis() + 10_000 + while (!condition() && System.currentTimeMillis() < deadline) { + shadowOf(Looper.getMainLooper()).idle() + Thread.sleep(20) + } + } + + private fun postedNotifications() = shadowOf(context.getSystemService(NotificationManager::class.java)).allNotifications + + private fun postedNotification(): Notification { + val notifications = postedNotifications() + assertEquals(1, notifications.size) + return notifications.single() + } + + private fun pushMessage() = + PushMessage( + from = "702491788352", + data = + mapOf( + "project" to PROJECT_ID, + "subscriber" to "subscriber-id", + "campaign" to "campaign-id", + "nId" to "notification-id", + "image" to IMAGE_URL, + "icon" to ICON_URL, + "redirectLink" to "https://www.polsatnews.pl/", + "notification" to + """{"title":"Title","body":"Body","sound":"default","vibrate":"true","priority":0,"badge":1,"click_action":"APP_PUSH_CLICK"}""", + ), + notification = null, + ) + + private companion object { + const val PROJECT_ID = "648bf40e32ca635c992e0186" + const val IMAGE_URL = "https://static-f.pushpushgo.com/image_medium.webp" + const val ICON_URL = "https://static-a.pushpushgo.com/icon_original.webp" + const val BIG_PICTURE_STYLE = "android.app.Notification\$BigPictureStyle" + const val BIG_TEXT_STYLE = "android.app.Notification\$BigTextStyle" + } +} From e2af0bf63bf118f357575876b4684945bdb72ff0 Mon Sep 17 00:00:00 2001 From: amarose Date: Mon, 28 Sep 2026 11:21:16 +0200 Subject: [PATCH 2/2] chore: release 3.2.3 --- README.md | 2 +- library/src/main/java/com/pushpushgo/sdk/PushPushGo.kt | 2 +- settings.gradle | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index f71dce83..14620dfa 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ ![GitHub tag (latest)](https://img.shields.io/github/v/tag/ppgco/android-sdk?style=flat-square) > [!IMPORTANT] -> **Version 3.2.1** +> **Version 3.2.3** > > Introducing **Live Activities (Android Live Updates)** — real-time, continuously > updated notifications (football match tracking template) on Android 16+. diff --git a/library/src/main/java/com/pushpushgo/sdk/PushPushGo.kt b/library/src/main/java/com/pushpushgo/sdk/PushPushGo.kt index 686b52c7..a69fca00 100644 --- a/library/src/main/java/com/pushpushgo/sdk/PushPushGo.kt +++ b/library/src/main/java/com/pushpushgo/sdk/PushPushGo.kt @@ -48,7 +48,7 @@ class PushPushGo private constructor( private val customBaseUrl: String?, ) { companion object { - const val VERSION = "3.2.2" + const val VERSION = "3.2.3" internal const val TAG = "PPGo" diff --git a/settings.gradle b/settings.gradle index a2d23b15..0f1d6770 100644 --- a/settings.gradle +++ b/settings.gradle @@ -11,7 +11,7 @@ if (!System.env.JITPACK) include ':sample', ':samplehms', ':samplejava', ':sampl dependencyResolutionManagement { versionCatalogs { libs { - version('sdk', '3.2.2') + version('sdk', '3.2.3') version('kotlin', '2.2.0') version('ksp', '2.2.0-2.0.2') version('coroutines', '1.10.1')