diff --git a/app/src/androidTest/kotlin/org/libremail/push/PushStatusNotificationInstrumentedTest.kt b/app/src/androidTest/kotlin/org/libremail/push/PushStatusNotificationInstrumentedTest.kt new file mode 100644 index 0000000..35ba7b4 --- /dev/null +++ b/app/src/androidTest/kotlin/org/libremail/push/PushStatusNotificationInstrumentedTest.kt @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.push + +import android.app.Notification +import android.app.NotificationManager +import android.content.Context +import androidx.core.app.NotificationManagerCompat +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.libremail.R +import org.libremail.data.sync.PushMode + +/** + * On-device coverage of [PushStatusNotification] — the foreground-notification logic [IdleService] + * delegates to (issue #257). `NotificationChannel`/`NotificationCompat` are no-op stubs in the + * unit-test `android.jar`, so this is the only place the real channel importance and the push-mode → + * text branch can be asserted. It uses the real application `Context` (a `ContextWrapper`, never a + * mocked `Context`), mirroring `BatteryOptimizationManagerIntentTest`, and touches no service + * lifecycle, Hilt graph, or network — so it is deterministic and side-effect-free beyond registering a + * low-importance notification channel. + */ +@RunWith(AndroidJUnit4::class) +class PushStatusNotificationInstrumentedTest { + + private val context = ApplicationProvider.getApplicationContext() + + @Test + fun ensureChannel_registersALowImportancePushChannel() { + PushStatusNotification.ensureChannel(context) + + val channel = requireNotNull( + NotificationManagerCompat.from(context).getNotificationChannel(PushStatusNotification.CHANNEL_ID), + ) { "push status channel must be registered" } + assertEquals(NotificationManager.IMPORTANCE_LOW, channel.importance) + } + + @Test + fun build_inIdleMode_saysConnectedForInstantDeliveryAndIsAnOngoingServiceNotification() { + val notification = PushStatusNotification.build(context, PushMode.IDLE) + + assertEquals( + context.getString(R.string.notif_push_status_title), + notification.extras.getCharSequence(Notification.EXTRA_TITLE).toString(), + ) + assertEquals( + context.getString(R.string.notif_push_status_text), + notification.extras.getCharSequence(Notification.EXTRA_TEXT).toString(), + ) + assertTrue( + "status notification must be ongoing", + (notification.flags and Notification.FLAG_ONGOING_EVENT) != 0, + ) + assertEquals(Notification.CATEGORY_SERVICE, notification.category) + } + + @Test + fun build_inPollingMode_saysLowBatteryFallback() { + val notification = PushStatusNotification.build(context, PushMode.POLLING) + + assertEquals( + context.getString(R.string.notif_push_status_text_low_battery), + notification.extras.getCharSequence(Notification.EXTRA_TEXT).toString(), + ) + } +} diff --git a/app/src/main/kotlin/org/libremail/di/ReportingModule.kt b/app/src/main/kotlin/org/libremail/di/ReportingModule.kt index cf79ebd..aa40d09 100644 --- a/app/src/main/kotlin/org/libremail/di/ReportingModule.kt +++ b/app/src/main/kotlin/org/libremail/di/ReportingModule.kt @@ -7,6 +7,8 @@ import dagger.Provides import dagger.hilt.InstallIn import dagger.hilt.android.qualifiers.ApplicationContext import dagger.hilt.components.SingletonComponent +import org.libremail.BuildConfig +import org.libremail.reporting.DebugReportEndpoint import org.libremail.reporting.ReportStore import java.io.File import javax.inject.Singleton @@ -19,4 +21,13 @@ object ReportingModule { @Singleton fun provideReportStore(@ApplicationContext context: Context): ReportStore = ReportStore(File(context.filesDir, "debug_reports")) + + /** + * The debug-report ingest endpoint (empty by default — see [DebugReportEndpoint]). Provided as an + * injectable value so [org.libremail.reporting.ReportUploadWorker] takes it via its constructor + * instead of reading the `BuildConfig` static inline, which keeps its submit path testable. + */ + @Provides + @DebugReportEndpoint + fun provideDebugReportEndpoint(): String = BuildConfig.DEBUG_REPORT_ENDPOINT } diff --git a/app/src/main/kotlin/org/libremail/push/IdleService.kt b/app/src/main/kotlin/org/libremail/push/IdleService.kt index 336dee2..c58e4f1 100644 --- a/app/src/main/kotlin/org/libremail/push/IdleService.kt +++ b/app/src/main/kotlin/org/libremail/push/IdleService.kt @@ -1,15 +1,11 @@ // SPDX-License-Identifier: GPL-3.0-or-later package org.libremail.push -import android.app.NotificationChannel -import android.app.NotificationManager import android.app.Service import android.content.Intent import android.content.pm.ServiceInfo import android.os.IBinder import android.util.Log -import androidx.core.app.NotificationCompat -import androidx.core.app.NotificationManagerCompat import androidx.core.app.ServiceCompat import dagger.Lazy import dagger.hilt.android.AndroidEntryPoint @@ -25,7 +21,6 @@ import kotlinx.coroutines.flow.combine import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import kotlinx.coroutines.withTimeoutOrNull -import org.libremail.R import org.libremail.data.local.dao.AccountDao import org.libremail.data.local.toDomain import org.libremail.data.security.EncryptedCacheGuard @@ -178,43 +173,19 @@ class IdleService : Service() { override fun onBind(intent: Intent?): IBinder? = null - /** - * Starts (or, on later calls, updates) the foreground notification. The text tells the truth per - * [mode]: "connected for instant delivery" versus the low-battery 15-minute polling fallback. - */ + /** Starts (or, on later calls, updates) the foreground notification for the current push [mode]. */ private fun startAsForeground(mode: PushMode) { - NotificationManagerCompat.from(this).createNotificationChannel( - NotificationChannel( - CHANNEL_ID, - getString(R.string.notif_channel_push_status), - NotificationManager.IMPORTANCE_LOW, - ), - ) - val text = if (mode == PushMode.POLLING) { - getString(R.string.notif_push_status_text_low_battery) - } else { - getString(R.string.notif_push_status_text) - } - val notification = NotificationCompat.Builder(this, CHANNEL_ID) - .setSmallIcon(R.drawable.ic_launcher_monochrome) - .setContentTitle(getString(R.string.notif_push_status_title)) - .setContentText(text) - .setOngoing(true) - .setShowWhen(false) - .setCategory(NotificationCompat.CATEGORY_SERVICE) - .build() + PushStatusNotification.ensureChannel(this) ServiceCompat.startForeground( this, - FOREGROUND_ID, - notification, + PushStatusNotification.FOREGROUND_ID, + PushStatusNotification.build(this, mode), ServiceInfo.FOREGROUND_SERVICE_TYPE_DATA_SYNC, ) } private companion object { const val TAG = "IdleService" - const val CHANNEL_ID = "push_status" - const val FOREGROUND_ID = 1002 const val INITIAL_BACKOFF_MS = 5_000L const val MAX_BACKOFF_MS = 5 * 60_000L diff --git a/app/src/main/kotlin/org/libremail/push/PushStatusNotification.kt b/app/src/main/kotlin/org/libremail/push/PushStatusNotification.kt new file mode 100644 index 0000000..5869a9d --- /dev/null +++ b/app/src/main/kotlin/org/libremail/push/PushStatusNotification.kt @@ -0,0 +1,55 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.push + +import android.app.Notification +import android.app.NotificationChannel +import android.app.NotificationManager +import android.content.Context +import androidx.core.app.NotificationCompat +import androidx.core.app.NotificationManagerCompat +import org.libremail.R +import org.libremail.data.sync.PushMode + +/** + * Builds the persistent foreground notification for [IdleService]. Extracted so the push-mode → text + * choice and the channel definition — the only non-lifecycle logic the service carries — can be + * verified with a real `Context` off the service (see `PushStatusNotificationInstrumentedTest`), + * without standing up the foreground service, its Hilt graph, or live IMAP connections. Behaviour is + * identical to the inline builder it replaced (issue #257). + */ +internal object PushStatusNotification { + + const val CHANNEL_ID = "push_status" + const val FOREGROUND_ID = 1002 + + /** Creates (idempotently) the low-importance channel the status notification posts on. */ + fun ensureChannel(context: Context) { + NotificationManagerCompat.from(context).createNotificationChannel( + NotificationChannel( + CHANNEL_ID, + context.getString(R.string.notif_channel_push_status), + NotificationManager.IMPORTANCE_LOW, + ), + ) + } + + /** + * The ongoing status notification. Its text tells the truth per [mode]: "connected for instant + * delivery" versus the low-battery 15-minute polling fallback (#90). + */ + fun build(context: Context, mode: PushMode): Notification { + val text = if (mode == PushMode.POLLING) { + context.getString(R.string.notif_push_status_text_low_battery) + } else { + context.getString(R.string.notif_push_status_text) + } + return NotificationCompat.Builder(context, CHANNEL_ID) + .setSmallIcon(R.drawable.ic_launcher_monochrome) + .setContentTitle(context.getString(R.string.notif_push_status_title)) + .setContentText(text) + .setOngoing(true) + .setShowWhen(false) + .setCategory(NotificationCompat.CATEGORY_SERVICE) + .build() + } +} diff --git a/app/src/main/kotlin/org/libremail/reporting/DebugReportEndpoint.kt b/app/src/main/kotlin/org/libremail/reporting/DebugReportEndpoint.kt new file mode 100644 index 0000000..d4be5e8 --- /dev/null +++ b/app/src/main/kotlin/org/libremail/reporting/DebugReportEndpoint.kt @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.reporting + +import javax.inject.Qualifier + +/** + * Qualifies the debug-report ingest endpoint URL — `BuildConfig.DEBUG_REPORT_ENDPOINT`, empty by + * default because the ingest server (issue #34) is out of scope for this repo. Injecting it, rather + * than reading the `BuildConfig` static inline in [ReportUploadWorker], keeps the submit path — which + * the empty default otherwise leaves unreachable — testable by pointing the worker at a local server + * (issue #257). + */ +@Qualifier +@Retention(AnnotationRetention.BINARY) +annotation class DebugReportEndpoint diff --git a/app/src/main/kotlin/org/libremail/reporting/ReportUploadScheduler.kt b/app/src/main/kotlin/org/libremail/reporting/ReportUploadScheduler.kt index 370354a..eb316a5 100644 --- a/app/src/main/kotlin/org/libremail/reporting/ReportUploadScheduler.kt +++ b/app/src/main/kotlin/org/libremail/reporting/ReportUploadScheduler.kt @@ -1,7 +1,6 @@ // SPDX-License-Identifier: GPL-3.0-or-later package org.libremail.reporting -import android.content.Context import androidx.work.BackoffPolicy import androidx.work.Constraints import androidx.work.ExistingWorkPolicy @@ -10,15 +9,22 @@ import androidx.work.OneTimeWorkRequestBuilder import androidx.work.WorkInfo import androidx.work.WorkManager import androidx.work.workDataOf -import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.flow.Flow import java.util.concurrent.TimeUnit import javax.inject.Inject +import javax.inject.Provider import javax.inject.Singleton /** Enqueues a user-initiated report upload as a retrying WorkManager job and observes its state. */ @Singleton -class ReportUploadScheduler @Inject constructor(@ApplicationContext private val context: Context) { +class ReportUploadScheduler @Inject constructor( + // A Provider (not the WorkManager itself) so WorkManager.getInstance() is resolved lazily at enqueue + // time — never during Hilt's Application field injection, which can run before the HiltWorkerFactory + // that on-demand WorkManager initialization needs is set. Mirrors SyncScheduler (issue #257). + private val workManagerProvider: Provider, +) { + private val workManager get() = workManagerProvider.get() + fun enqueue(reportId: String) { val request = OneTimeWorkRequestBuilder() .setConstraints( @@ -30,12 +36,11 @@ class ReportUploadScheduler @Inject constructor(@ApplicationContext private val .build() // REPLACE: a fresh Submit tap starts a clean attempt for this report, overriding any pending // retry-backoff. Reports are keyed per id, so distinct reports never collide. - WorkManager.getInstance(context) - .enqueueUniqueWork(uniqueName(reportId), ExistingWorkPolicy.REPLACE, request) + workManager.enqueueUniqueWork(uniqueName(reportId), ExistingWorkPolicy.REPLACE, request) } fun statusFlow(reportId: String): Flow> = - WorkManager.getInstance(context).getWorkInfosForUniqueWorkFlow(uniqueName(reportId)) + workManager.getWorkInfosForUniqueWorkFlow(uniqueName(reportId)) private fun uniqueName(reportId: String) = "$WORK_PREFIX$reportId" diff --git a/app/src/main/kotlin/org/libremail/reporting/ReportUploadWorker.kt b/app/src/main/kotlin/org/libremail/reporting/ReportUploadWorker.kt index 444b3c7..e3d8f16 100644 --- a/app/src/main/kotlin/org/libremail/reporting/ReportUploadWorker.kt +++ b/app/src/main/kotlin/org/libremail/reporting/ReportUploadWorker.kt @@ -9,27 +9,27 @@ import dagger.assisted.Assisted import dagger.assisted.AssistedInject import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext -import org.libremail.BuildConfig import java.net.HttpURLConnection import java.net.URL /** * Posts a single user-submitted report to the ingest endpoint. It runs ONLY when the user tapped - * Submit (enqueued by [ReportUploadScheduler]); nothing here runs automatically. The endpoint is - * [BuildConfig.DEBUG_REPORT_ENDPOINT] — empty by default, because the ingest server (issue #34) is - * out of scope for this repo, so submissions no-op with a clear failure until an endpoint is set. + * Submit (enqueued by [ReportUploadScheduler]); nothing here runs automatically. The [endpoint] is + * injected (see [DebugReportEndpoint]) from `BuildConfig.DEBUG_REPORT_ENDPOINT` — empty by default, + * because the ingest server (issue #34) is out of scope for this repo, so submissions no-op with a + * clear failure until an endpoint is set. */ @HiltWorker class ReportUploadWorker @AssistedInject constructor( @Assisted appContext: Context, @Assisted params: WorkerParameters, private val store: ReportStore, + @DebugReportEndpoint private val endpoint: String, ) : CoroutineWorker(appContext, params) { override suspend fun doWork(): Result { val id = inputData.getString(KEY_REPORT_ID) ?: return Result.success() val report = store.find(id) ?: return Result.success() // discarded before the job ran - val endpoint = BuildConfig.DEBUG_REPORT_ENDPOINT if (endpoint.isBlank()) return Result.failure() // no ingest server configured in this build return withContext(Dispatchers.IO) { runCatching { post(endpoint, report.toSubmissionPayload()) }.fold( diff --git a/app/src/test/kotlin/org/libremail/reporting/ReportUploadSchedulerTest.kt b/app/src/test/kotlin/org/libremail/reporting/ReportUploadSchedulerTest.kt new file mode 100644 index 0000000..02a3360 --- /dev/null +++ b/app/src/test/kotlin/org/libremail/reporting/ReportUploadSchedulerTest.kt @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.reporting + +import androidx.work.ExistingWorkPolicy +import androidx.work.OneTimeWorkRequest +import androidx.work.WorkInfo +import androidx.work.WorkManager +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.flow.flowOf +import org.junit.Test +import javax.inject.Provider +import kotlin.test.assertSame + +/** + * Enqueue is a thin wrapper over WorkManager, so these pin the behaviour it carries: the per-report + * unique-work name and the REPLACE policy that lets a fresh Submit tap override a pending retry. + * Injecting a `Provider` (mirroring `SyncScheduler`) is what makes this JVM-testable — the + * scheduler no longer reaches for the un-stubbable `WorkManager.getInstance` static, which MockK can't + * stub on the abstract `WorkManager` (`AbstractMethodError`) that held this class off the unit suite + * (issue #257). + */ +class ReportUploadSchedulerTest { + + private val workManager = mockk(relaxed = true) + private val scheduler = ReportUploadScheduler(Provider { workManager }) + + @Test + fun `enqueue replaces any pending upload for the same report id`() { + scheduler.enqueue("rid") + + verify { + workManager.enqueueUniqueWork( + "libremail_report_upload_rid", + ExistingWorkPolicy.REPLACE, + any(), + ) + } + } + + @Test + fun `statusFlow observes the unique work for that report id`() { + val flow = flowOf(emptyList()) + every { workManager.getWorkInfosForUniqueWorkFlow("libremail_report_upload_rid") } returns flow + + assertSame(flow, scheduler.statusFlow("rid")) + } +} diff --git a/app/src/test/kotlin/org/libremail/reporting/ReportUploadWorkerHttpTest.kt b/app/src/test/kotlin/org/libremail/reporting/ReportUploadWorkerHttpTest.kt new file mode 100644 index 0000000..9dc16c4 --- /dev/null +++ b/app/src/test/kotlin/org/libremail/reporting/ReportUploadWorkerHttpTest.kt @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.reporting + +import androidx.work.ListenableWorker.Result +import androidx.work.WorkerParameters +import androidx.work.workDataOf +import com.sun.net.httpserver.HttpServer +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.test.runTest +import org.junit.After +import org.junit.Before +import org.junit.Test +import java.net.InetAddress +import java.net.InetSocketAddress +import java.net.ServerSocket +import java.util.concurrent.atomic.AtomicInteger +import java.util.concurrent.atomic.AtomicReference +import kotlin.test.assertEquals + +/** + * Covers [ReportUploadWorker]'s transmit path — unreachable while the default build ships an empty + * `DEBUG_REPORT_ENDPOINT` — by injecting a non-empty endpoint (issue #257). The endpoint points at an + * in-process [HttpServer] on loopback (JDK built-in, so no new dependency, and the same "real + * in-process server" approach the suite already uses with GreenMail for IMAP/SMTP), so the actual + * `HttpURLConnection` POST, response-code handling, retry/backoff decision, and on-success delete run + * end to end: + * - 2xx -> success, and the delivered report is dropped from the local store; + * - 4xx -> permanent failure (retrying a client error can't help), store untouched; + * - 5xx -> retry while attempts remain, then failure once the attempt cap is hit; + * - a network error (nothing listening) -> retry while attempts remain. + * It also pins the request the server receives: a JSON POST whose body is the report's submission + * payload — the exact wire format the (out-of-scope) ingest server would get. + */ +class ReportUploadWorkerHttpTest { + + private val store = mockk(relaxed = true) + private lateinit var server: HttpServer + private val responseCode = AtomicInteger(HTTP_OK) + private val receivedBody = AtomicReference() + private val receivedContentType = AtomicReference() + private lateinit var endpoint: String + + @Before + fun startServer() { + server = HttpServer.create(InetSocketAddress(LOOPBACK, 0), 0) + server.createContext(PATH) { exchange -> + receivedContentType.set(exchange.requestHeaders.getFirst("Content-Type")) + receivedBody.set(exchange.requestBody.readBytes().toString(Charsets.UTF_8)) + exchange.sendResponseHeaders(responseCode.get(), NO_RESPONSE_BODY) + exchange.close() + } + server.start() + endpoint = "http://$LOOPBACK:${server.address.port}$PATH" + } + + @After + fun stopServer() { + server.stop(0) + } + + @Test + fun `a 2xx response succeeds and drops the delivered report`() = runTest { + val report = report() + every { store.find(REPORT_ID) } returns report + responseCode.set(HTTP_OK) + + val result = worker(endpoint).doWork() + + assertEquals(Result.success(), result) + verify { store.delete(REPORT_ID) } + assertEquals(report.toSubmissionPayload(), receivedBody.get()) + assertEquals("application/json; charset=utf-8", receivedContentType.get()) + } + + @Test + fun `a 4xx client error fails permanently without deleting the report`() = runTest { + every { store.find(REPORT_ID) } returns report() + responseCode.set(HTTP_BAD_REQUEST) + + val result = worker(endpoint).doWork() + + assertEquals(Result.failure(), result) + verify(exactly = 0) { store.delete(any()) } + } + + @Test + fun `a 5xx server error retries while attempts remain`() = runTest { + every { store.find(REPORT_ID) } returns report() + responseCode.set(HTTP_SERVER_ERROR) + + val result = worker(endpoint, attempt = 0).doWork() + + assertEquals(Result.retry(), result) + verify(exactly = 0) { store.delete(any()) } + } + + @Test + fun `a 5xx server error fails once the attempt cap is hit`() = runTest { + every { store.find(REPORT_ID) } returns report() + responseCode.set(HTTP_SERVER_ERROR) + + val result = worker(endpoint, attempt = MAX_ATTEMPTS).doWork() + + assertEquals(Result.failure(), result) + } + + @Test + fun `a network error retries while attempts remain`() = runTest { + every { store.find(REPORT_ID) } returns report() + + val result = worker(deadEndpoint(), attempt = 0).doWork() + + assertEquals(Result.retry(), result) + verify(exactly = 0) { store.delete(any()) } + } + + private fun worker(endpoint: String, attempt: Int = 0) = ReportUploadWorker( + mockk(relaxed = true), + mockk(relaxed = true) { + every { inputData } returns workDataOf(ReportUploadWorker.KEY_REPORT_ID to REPORT_ID) + every { runAttemptCount } returns attempt + }, + store, + endpoint = endpoint, + ) + + /** A loopback URL with nothing listening: claim then immediately free a port so the POST is refused. */ + private fun deadEndpoint(): String { + val port = ServerSocket(0, 0, InetAddress.getByName(LOOPBACK)).use { it.localPort } + return "http://$LOOPBACK:$port$PATH" + } + + private fun report() = DebugReport( + id = REPORT_ID, + createdAtMillis = 1L, + kind = ReportKind.CRASH, + appVersionName = "0.1.0", + appVersionCode = 1, + androidRelease = "14", + androidSdkInt = 34, + deviceManufacturer = "Google", + deviceModel = "Pixel", + stackTrace = "boom", + settings = emptyMap(), + logs = emptyList(), + ) + + private companion object { + const val LOOPBACK = "127.0.0.1" + const val PATH = "/report" + const val REPORT_ID = "rid" + const val NO_RESPONSE_BODY = -1L + const val HTTP_OK = 200 + const val HTTP_BAD_REQUEST = 400 + const val HTTP_SERVER_ERROR = 500 + + // Mirrors ReportUploadWorker.MAX_ATTEMPTS (private): at/after this count a retry becomes a failure. + const val MAX_ATTEMPTS = 5 + } +} diff --git a/app/src/test/kotlin/org/libremail/reporting/ReportUploadWorkerTest.kt b/app/src/test/kotlin/org/libremail/reporting/ReportUploadWorkerTest.kt index 52e6050..fb58185 100644 --- a/app/src/test/kotlin/org/libremail/reporting/ReportUploadWorkerTest.kt +++ b/app/src/test/kotlin/org/libremail/reporting/ReportUploadWorkerTest.kt @@ -26,6 +26,7 @@ class ReportUploadWorkerTest { if (inputId == null) workDataOf() else workDataOf(ReportUploadWorker.KEY_REPORT_ID to inputId) }, store, + endpoint = "", // default build ships no ingest endpoint; the transmit path is covered separately ) private fun report(id: String) = DebugReport(