From 29902eee4e6dd39acca400db691b9e86e5a0f5aa Mon Sep 17 00:00:00 2001 From: Luis-ADFA Date: Mon, 3 Aug 2026 11:38:39 -0600 Subject: [PATCH 1/2] ADFA-5000 fix(update): drive OTA verify from poller so the dialog can't hang on 'verifying' --- .../update/presentation/UpdateController.java | 53 +++++++++++++++---- .../update/presentation/UpdateViewModel.java | 24 ++++++++- 2 files changed, 65 insertions(+), 12 deletions(-) diff --git a/controller/app/src/main/java/org/iiab/controller/update/presentation/UpdateController.java b/controller/app/src/main/java/org/iiab/controller/update/presentation/UpdateController.java index 31d34eec..e8141973 100644 --- a/controller/app/src/main/java/org/iiab/controller/update/presentation/UpdateController.java +++ b/controller/app/src/main/java/org/iiab/controller/update/presentation/UpdateController.java @@ -64,6 +64,10 @@ public class UpdateController { private BrandDialog.Handle updateProgressDialog; private UpdateViewModel updateViewModel; private long lastUpdateCheckTime = 0; + // ADFA-5000: guards the post-download verify/install hand-off so the two + // completion triggers (the ViewModel poller and the DownloadManager broadcast) + // run it at most once. Reset when a new download starts. + private boolean downloadCompletionHandled = false; public UpdateController(AppCompatActivity activity) { this.activity = activity; @@ -207,6 +211,7 @@ private void startDownload(String downloadUrl) { android.app.DownloadManager manager = (android.app.DownloadManager) activity.getSystemService(Context.DOWNLOAD_SERVICE); if (manager != null) { + downloadCompletionHandled = false; updateDownloadId = manager.enqueue(request); getUpdateViewModel().track(updateDownloadId); showUpdateProgressDialog(); @@ -220,23 +225,46 @@ public void onReceive(Context context, Intent intent) { if (id != updateDownloadId) { return; } + // Fast path: the broadcast arrived. Funnel through the same idempotent + // handler as the ViewModel poller so verification runs exactly once. + handleDownloadComplete(); + } + }; + + /** + * ADFA-5000: single, idempotent post-download hand-off. Invoked by BOTH the + * DownloadManager broadcast (fast path) and the ViewModel poller (reliable + * fallback that fires even when the broadcast is never delivered — the cause + * of the dialog getting stuck on "verifying"). The first caller wins; the + * rest are no-ops. Signature verification is I/O + a PackageManager APK parse, + * so it runs off the main thread and the result is posted back to the UI. + */ + private void handleDownloadComplete() { + if (downloadCompletionHandled) { + return; + } + downloadCompletionHandled = true; + + AppExecutors.get().io().execute(() -> { // F15: only install if the download actually SUCCEEDED. DownloadManager // reports completion even when the server returned an error/HTML page. - if (isDownloadSuccessful(id)) { - File apk = verifyDownloadedApk(); - if (apk != null) { + boolean success = isDownloadSuccessful(updateDownloadId); + File apk = success ? verifyDownloadedApk() : null; + + activity.runOnUiThread(() -> { + if (!success) { + Log.e(TAG, "OTA: download did not complete successfully; not installing."); + getUpdateViewModel().onError(activity.getString(R.string.ota_error_download_failed)); + Toast.makeText(activity, R.string.ota_error_download_failed, Toast.LENGTH_LONG).show(); + } else if (apk != null) { getUpdateViewModel().onReady(); } else { getUpdateViewModel().onError(activity.getString(R.string.ota_error_verify_failed)); - Toast.makeText(context, R.string.ota_error_verify_failed, Toast.LENGTH_LONG).show(); + Toast.makeText(activity, R.string.ota_error_verify_failed, Toast.LENGTH_LONG).show(); } - } else { - getUpdateViewModel().onError(activity.getString(R.string.ota_error_download_failed)); - Log.e(TAG, "OTA: download did not complete successfully; not installing."); - Toast.makeText(context, R.string.ota_error_download_failed, Toast.LENGTH_LONG).show(); - } - } - }; + }); + }); + } /** Did the DownloadManager job with this id finish with STATUS_SUCCESSFUL? */ private boolean isDownloadSuccessful(long id) { @@ -332,6 +360,9 @@ private UpdateViewModel getUpdateViewModel() { updateViewModel = new ViewModelProvider(activity, new UpdateViewModelFactory(activity)) .get(UpdateViewModel.class); updateViewModel.state().observe(activity, this::renderUpdateState); + // Reliable completion signal (fires even if the DownloadManager broadcast + // is lost) -> run the same idempotent verify/install hand-off. + updateViewModel.setOnTerminal(this::handleDownloadComplete); } return updateViewModel; } diff --git a/controller/app/src/main/java/org/iiab/controller/update/presentation/UpdateViewModel.java b/controller/app/src/main/java/org/iiab/controller/update/presentation/UpdateViewModel.java index 8de5df3d..338165c4 100644 --- a/controller/app/src/main/java/org/iiab/controller/update/presentation/UpdateViewModel.java +++ b/controller/app/src/main/java/org/iiab/controller/update/presentation/UpdateViewModel.java @@ -34,6 +34,8 @@ public class UpdateViewModel extends ViewModel { private long downloadId = -1; private Runnable poller; + private Runnable onTerminal; + private boolean terminalFired; public UpdateViewModel(OtaDownloadGateway gateway) { this.gateway = gateway; @@ -43,15 +45,34 @@ public LiveData state() { return state; } + /** + * Callback fired once when the tracked download reaches a terminal state + * (SUCCESSFUL or FAILED). This is the reliable completion signal — it comes + * from polling DownloadManager directly, so it fires even if the system's + * ACTION_DOWNLOAD_COMPLETE broadcast is never delivered (app backgrounded, + * OEM quirks). The controller uses it to run signature verification instead + * of relying solely on the broadcast (which could hang the dialog on + * "verifying" forever). Safe to call again; only the first terminal poll fires. + */ + public void setOnTerminal(Runnable r) { + this.onTerminal = r; + } + /** Start tracking a DownloadManager download id; polls until it is terminal. */ public void track(long id) { downloadId = id; + terminalFired = false; stopPolling(); poller = new Runnable() { @Override public void run() { DownloadProgress p = gateway.query(downloadId); state.setValue(UpdateUiState.fromDownload(p)); - if (!p.isTerminal()) { + if (p.isTerminal()) { + if (!terminalFired) { + terminalFired = true; + if (onTerminal != null) onTerminal.run(); + } + } else { handler.postDelayed(this, POLL_MS); } } @@ -68,6 +89,7 @@ public void cancel() { stopPolling(); if (downloadId >= 0) gateway.cancel(downloadId); downloadId = -1; + terminalFired = false; state.setValue(UpdateUiState.idle()); } From 533214a7bb47874fbf5c80b7c4767a5b287c77d6 Mon Sep 17 00:00:00 2001 From: Luis-ADFA Date: Mon, 3 Aug 2026 11:55:41 -0600 Subject: [PATCH 2/2] =?UTF-8?q?ADFA-5000=20fix(update):=20address=20review?= =?UTF-8?q?=20=E2=80=94=20guard=20poller=20from=20regressing=20OTA=20state?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../update/presentation/UpdateController.java | 3 + .../update/presentation/UpdateViewModel.java | 56 ++++++++++++--- .../presentation/UpdateViewModelPollTest.java | 71 +++++++++++++++++++ 3 files changed, 121 insertions(+), 9 deletions(-) create mode 100644 controller/app/src/test/java/org/iiab/controller/update/presentation/UpdateViewModelPollTest.java diff --git a/controller/app/src/main/java/org/iiab/controller/update/presentation/UpdateController.java b/controller/app/src/main/java/org/iiab/controller/update/presentation/UpdateController.java index e8141973..8fa081cf 100644 --- a/controller/app/src/main/java/org/iiab/controller/update/presentation/UpdateController.java +++ b/controller/app/src/main/java/org/iiab/controller/update/presentation/UpdateController.java @@ -244,6 +244,9 @@ private void handleDownloadComplete() { return; } downloadCompletionHandled = true; + // Stop the poller and mark terminal as handled so a later poll tick can't + // regress a freshly-posted READY back to VERIFYING (broadcast-first race). + getUpdateViewModel().markTerminalHandled(); AppExecutors.get().io().execute(() -> { // F15: only install if the download actually SUCCEEDED. DownloadManager diff --git a/controller/app/src/main/java/org/iiab/controller/update/presentation/UpdateViewModel.java b/controller/app/src/main/java/org/iiab/controller/update/presentation/UpdateViewModel.java index 338165c4..e70b7eaf 100644 --- a/controller/app/src/main/java/org/iiab/controller/update/presentation/UpdateViewModel.java +++ b/controller/app/src/main/java/org/iiab/controller/update/presentation/UpdateViewModel.java @@ -65,21 +65,29 @@ public void track(long id) { stopPolling(); poller = new Runnable() { @Override public void run() { - DownloadProgress p = gateway.query(downloadId); - state.setValue(UpdateUiState.fromDownload(p)); - if (p.isTerminal()) { - if (!terminalFired) { - terminalFired = true; - if (onTerminal != null) onTerminal.run(); - } - } else { - handler.postDelayed(this, POLL_MS); + PollOutcome o = decidePoll(gateway.query(downloadId), terminalFired); + if (o.state != null) state.setValue(o.state); + if (o.fireTerminal) { + terminalFired = true; + if (onTerminal != null) onTerminal.run(); } + if (o.keepPolling) handler.postDelayed(this, POLL_MS); } }; handler.post(poller); } + /** + * Called by the controller when completion has been handled via the + * DownloadManager broadcast (the fast path). Marks the terminal state as + * already handled and stops polling, so a later poll tick cannot regress the + * dialog (e.g. overwrite a freshly-posted READY back to VERIFYING). + */ + public void markTerminalHandled() { + terminalFired = true; + stopPolling(); + } + public void onReady() { state.setValue(UpdateUiState.ready()); } public void onInstalling() { state.setValue(UpdateUiState.installing()); } public void onError(String message) { stopPolling(); state.setValue(UpdateUiState.error(message)); } @@ -93,6 +101,36 @@ public void cancel() { state.setValue(UpdateUiState.idle()); } + /** + * Pure decision for a single poll tick — no Android, so it is unit-testable. + * Once a terminal state has already been handled (by an earlier poll or by the + * broadcast fast path), a later terminal observation must NOT re-emit VERIFYING; + * that is what previously regressed a completed READY back to "verifying" and + * re-hung the dialog. In that case emit no state and stop polling. + */ + static PollOutcome decidePoll(DownloadProgress p, boolean terminalAlreadyHandled) { + if (p.isTerminal()) { + if (terminalAlreadyHandled) { + return new PollOutcome(null, false, false); + } + return new PollOutcome(UpdateUiState.fromDownload(p), true, false); + } + return new PollOutcome(UpdateUiState.fromDownload(p), false, true); + } + + /** Result of one poll tick: which state to emit (null = none), whether to fire the terminal callback, whether to keep polling. */ + static final class PollOutcome { + final UpdateUiState state; + final boolean fireTerminal; + final boolean keepPolling; + + PollOutcome(UpdateUiState state, boolean fireTerminal, boolean keepPolling) { + this.state = state; + this.fireTerminal = fireTerminal; + this.keepPolling = keepPolling; + } + } + private void stopPolling() { if (poller != null) handler.removeCallbacks(poller); poller = null; diff --git a/controller/app/src/test/java/org/iiab/controller/update/presentation/UpdateViewModelPollTest.java b/controller/app/src/test/java/org/iiab/controller/update/presentation/UpdateViewModelPollTest.java new file mode 100644 index 00000000..f4284273 --- /dev/null +++ b/controller/app/src/test/java/org/iiab/controller/update/presentation/UpdateViewModelPollTest.java @@ -0,0 +1,71 @@ +/* + * ============================================================================ + * Name : UpdateViewModelPollTest.java + * Author : AppDevForAll + * Copyright : Copyright (c) 2026 AppDevForAll + * Description : ADFA-5000. Pure-JVM tests for the per-poll decision that drives + * the OTA dialog. Guards the fix for the "stuck on verifying" + * regression: once completion has been handled, a later terminal + * poll must not re-emit VERIFYING (which would overwrite READY). + * ============================================================================ + */ +package org.iiab.controller.update.presentation; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import org.iiab.controller.update.domain.DownloadProgress; +import org.iiab.controller.update.presentation.UpdateViewModel.PollOutcome; +import org.junit.Test; + +public class UpdateViewModelPollTest { + + private static DownloadProgress progress(DownloadProgress.Status s) { + return new DownloadProgress(s, 100, 100); + } + + @Test public void runningEmitsDownloadingAndKeepsPolling() { + PollOutcome o = UpdateViewModel.decidePoll(progress(DownloadProgress.Status.RUNNING), false); + assertEquals(UpdateUiState.Status.DOWNLOADING, o.state.status); + assertFalse(o.fireTerminal); + assertTrue(o.keepPolling); + } + + @Test public void firstSuccessEmitsVerifyingFiresTerminalAndStops() { + PollOutcome o = UpdateViewModel.decidePoll(progress(DownloadProgress.Status.SUCCESSFUL), false); + assertEquals(UpdateUiState.Status.VERIFYING, o.state.status); + assertTrue(o.fireTerminal); + assertFalse(o.keepPolling); + } + + @Test public void firstFailedEmitsErrorFiresTerminalAndStops() { + PollOutcome o = UpdateViewModel.decidePoll(progress(DownloadProgress.Status.FAILED), false); + assertEquals(UpdateUiState.Status.ERROR, o.state.status); + assertTrue(o.fireTerminal); + assertFalse(o.keepPolling); + } + + /** Regression guard (#1): once handled, a later SUCCESSFUL poll must NOT re-emit VERIFYING. */ + @Test public void successAfterHandledEmitsNothingAndStops() { + PollOutcome o = UpdateViewModel.decidePoll(progress(DownloadProgress.Status.SUCCESSFUL), true); + assertNull("must not overwrite a completed READY with VERIFYING", o.state); + assertFalse(o.fireTerminal); + assertFalse(o.keepPolling); + } + + /** Once handled, a later FAILED poll must also be inert (no error re-emit, no polling). */ + @Test public void failedAfterHandledEmitsNothingAndStops() { + PollOutcome o = UpdateViewModel.decidePoll(progress(DownloadProgress.Status.FAILED), true); + assertNull(o.state); + assertFalse(o.fireTerminal); + assertFalse(o.keepPolling); + } + + /** Terminal callback must fire only on the first terminal observation. */ + @Test public void terminalFiresOnlyOnFirstObservation() { + assertTrue(UpdateViewModel.decidePoll(progress(DownloadProgress.Status.SUCCESSFUL), false).fireTerminal); + assertFalse(UpdateViewModel.decidePoll(progress(DownloadProgress.Status.SUCCESSFUL), true).fireTerminal); + } +}