From 93bcefd25919aca3906b0abb9dfd10f7701807a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=94=B5=E5=AD=90=E8=85=8B=E6=AF=9B=E7=A8=8B=E5=BA=8F/Ele?= =?UTF-8?q?ctronic=20hirci=20program?= Date: Sat, 3 Oct 2026 19:42:42 +1300 Subject: [PATCH 01/10] fix(download): stop archive service only when idle #166 --- .../ehviewer/download/ArchiverDownloader.kt | 28 ++- .../download/ArchiverDownloaderTest.java | 171 ++++++++++++++++++ 2 files changed, 190 insertions(+), 9 deletions(-) create mode 100644 app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloaderTest.java diff --git a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloader.kt b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloader.kt index 41777fd60d..4f5f9213ce 100644 --- a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloader.kt +++ b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloader.kt @@ -130,7 +130,17 @@ class ArchiverDownloader private constructor(appContext: Context) { deleteZipFile(task.zipFile) clearTaskSettings(gid, task.taskId) notifyCancel(gid, task.taskId) - ArchiverDownloadService.stop(appContext) + stopServiceIfIdle() + } + + /** + * Every task shares one service and one notification (#166): a task that ends stops it only + * when no other task is still downloading. Paused tasks need no foreground service. + */ + private fun stopServiceIfIdle() { + if (activeTasks.values.none { !it.paused }) { + ArchiverDownloadService.stop(appContext) + } } private fun startInternal( @@ -188,13 +198,13 @@ class ArchiverDownloader private constructor(appContext: Context) { activeTasks.remove(info.gid) if (call.isCanceled) { notifyCancel(info.gid, taskId) - ArchiverDownloadService.stop(appContext) + stopServiceIfIdle() return } deleteZipFile(zipFile) clearTaskSettings(info.gid, taskId) notifyFailure(info.gid, taskId, e) - ArchiverDownloadService.stop(appContext) + stopServiceIfIdle() } override fun onResponse(call: Call, response: Response) { @@ -219,7 +229,7 @@ class ArchiverDownloader private constructor(appContext: Context) { clearTaskSettings(info.gid, taskId) notifyFailure(info.gid, taskId, IOException("HTTP $code")) response.close() - ArchiverDownloadService.stop(appContext) + stopServiceIfIdle() return } val body = response.body() @@ -229,7 +239,7 @@ class ArchiverDownloader private constructor(appContext: Context) { clearTaskSettings(info.gid, taskId) notifyFailure(info.gid, taskId, IOException("Empty response body")) response.close() - ArchiverDownloadService.stop(appContext) + stopServiceIfIdle() return } var writeOffset = offset @@ -283,7 +293,7 @@ class ArchiverDownloader private constructor(appContext: Context) { notifySuccess(info.gid, taskId, zipFile) ArchiverDownloadCompleter.getInstance(appContext) ?.importDownloadedZip(zipFile, info, taskId) - ArchiverDownloadService.stop(appContext) + stopServiceIfIdle() } catch (e: PauseSignal) { if (task.call === call) { publishProgress(task, true) @@ -307,7 +317,7 @@ class ArchiverDownloader private constructor(appContext: Context) { } else { notifyFailure(info.gid, taskId, e) } - ArchiverDownloadService.stop(appContext) + stopServiceIfIdle() } } } @@ -329,7 +339,7 @@ class ArchiverDownloader private constructor(appContext: Context) { notifySuccess(info.gid, taskId, zipFile) ArchiverDownloadCompleter.getInstance(appContext) ?.importDownloadedZip(zipFile, info, taskId) - ArchiverDownloadService.stop(appContext) + stopServiceIfIdle() return } if (task.paused) { @@ -340,7 +350,7 @@ class ArchiverDownloader private constructor(appContext: Context) { deleteZipFile(zipFile) clearTaskSettings(info.gid, taskId) notifyFailure(info.gid, taskId, IOException("HTTP 416")) - ArchiverDownloadService.stop(appContext) + stopServiceIfIdle() } private fun publishProgress(task: ActiveTask, force: Boolean) { diff --git a/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloaderTest.java b/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloaderTest.java new file mode 100644 index 0000000000..28beaee321 --- /dev/null +++ b/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloaderTest.java @@ -0,0 +1,171 @@ +package com.hippo.ehviewer.download; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.robolectric.Shadows.shadowOf; + +import android.app.Application; +import android.content.Context; +import android.content.Intent; + +import com.hippo.ehviewer.AppConfig; +import com.hippo.ehviewer.EhApplication; +import com.hippo.ehviewer.Settings; +import com.hippo.ehviewer.client.data.GalleryInfo; + +import java.lang.reflect.Field; +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Deque; +import java.util.List; +import java.util.concurrent.AbstractExecutorService; +import java.util.concurrent.TimeUnit; + +import okhttp3.Dispatcher; +import okhttp3.OkHttpClient; +import okhttp3.Protocol; +import okhttp3.Response; +import okhttp3.ResponseBody; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.RuntimeEnvironment; +import org.robolectric.annotation.Config; +import org.robolectric.annotation.Implementation; +import org.robolectric.annotation.Implements; + +/** Pins when the archive downloader lets its shared foreground service go (#166). */ +@RunWith(RobolectricTestRunner.class) +@Config(application = android.app.Application.class, + shadows = ArchiverDownloaderTest.ShadowEhApplication.class, + instrumentedPackages = {"com.hippo.ehviewer.EhApplication"}) +public class ArchiverDownloaderTest { + + private static final long GID_A = 3054001L; + private static final long GID_B = 3054002L; + + static OkHttpClient client; + + private final HeldCalls calls = new HeldCalls(); + private Application app; + private ArchiverDownloader downloader; + + @Implements(EhApplication.class) + public static class ShadowEhApplication { + @Implementation + protected static OkHttpClient getOkHttpClient(Context context) { + return client; + } + } + + /** Holds each enqueued call until the test runs it, so no OkHttp thread races an assertion. */ + private static final class HeldCalls extends AbstractExecutorService { + final Deque held = new ArrayDeque<>(); + + void runNext() { + held.removeFirst().run(); + } + + @Override + public void execute(Runnable command) { + held.addLast(command); + } + + @Override + public void shutdown() {} + + @Override + public List shutdownNow() { + return new ArrayList<>(); + } + + @Override + public boolean isShutdown() { + return false; + } + + @Override + public boolean isTerminated() { + return false; + } + + @Override + public boolean awaitTermination(long timeout, TimeUnit unit) { + return true; + } + } + + private static GalleryInfo gallery(long gid) { + GalleryInfo info = new GalleryInfo(); + info.gid = gid; + info.title = "archive fixture " + gid; + info.token = "token" + gid; + return info; + } + + @Before + public void setUp() throws Exception { + app = RuntimeEnvironment.getApplication(); + Settings.initialize(app); + AppConfig.initialize(app); + // Every call fails at once, which ends its task the way a dead link would. + client = new OkHttpClient.Builder() + .dispatcher(new Dispatcher(calls)) + .addInterceptor(chain -> new Response.Builder() + .request(chain.request()) + .protocol(Protocol.HTTP_1_1) + .code(500) + .message("Server Error") + .body(ResponseBody.create(null, "")) + .build()) + .build(); + // A process-wide singleton: without a fresh one it keeps the first test's application. + Field instance = ArchiverDownloader.class.getDeclaredField("sInstance"); + instance.setAccessible(true); + instance.set(null, null); + downloader = ArchiverDownloader.getInstance(app); + } + + private void start(long gid) { + downloader.start(app, gallery(gid), "https://example.org/archive/" + gid, "archive" + gid); + } + + /** The actions of every service start since the last call. */ + private List serviceActions() { + List actions = new ArrayList<>(); + Intent intent; + while ((intent = shadowOf(app).getNextStartedService()) != null) { + actions.add(intent.getAction()); + } + return actions; + } + + @Test + public void oneTaskEnding_leavesTheServiceToTheTaskStillDownloading() { + start(GID_A); + start(GID_B); + serviceActions(); + + calls.runNext(); + assertFalse("B lost its foreground service when A ended", + serviceActions().contains(ArchiverDownloadService.ACTION_STOP)); + + calls.runNext(); + assertTrue("the last task ended and the service stayed up", + serviceActions().contains(ArchiverDownloadService.ACTION_STOP)); + } + + @Test + public void aPausedTask_doesNotKeepTheServiceUp() { + start(GID_A); + start(GID_B); + downloader.pause(GID_B); + serviceActions(); + + calls.runNext(); + + assertTrue(serviceActions().contains(ArchiverDownloadService.ACTION_STOP)); + } +} From d53915c13912ef929efb7dccc5d19d5c35746d55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=94=B5=E5=AD=90=E8=85=8B=E6=AF=9B=E7=A8=8B=E5=BA=8F/Ele?= =?UTF-8?q?ctronic=20hirci=20program?= Date: Sat, 3 Oct 2026 19:42:49 +1300 Subject: [PATCH 02/10] fix(download): stop on Android 15 dataSync timeout #166 --- .../download/ArchiverDownloadService.kt | 38 ++++++++-- .../ehviewer/download/ArchiverDownloader.kt | 7 ++ .../ehviewer/download/DownloadService.kt | 8 ++ .../download/ArchiverDownloaderTest.java | 62 +++++++++++++++- .../download/DownloadServiceTest.java | 73 +++++++++++++++++++ 5 files changed, 178 insertions(+), 10 deletions(-) create mode 100644 app/src/test/java/com/hippo/ehviewer/download/DownloadServiceTest.java diff --git a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadService.kt b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadService.kt index 8f9a4085bf..d1ccaf5dec 100644 --- a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadService.kt +++ b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadService.kt @@ -11,6 +11,7 @@ import android.content.pm.ServiceInfo import android.os.Build import android.os.Bundle import android.os.IBinder +import androidx.annotation.RequiresApi import androidx.core.app.NotificationCompat import com.hippo.ehviewer.R import com.hippo.ehviewer.ui.MainActivity @@ -95,14 +96,29 @@ class ArchiverDownloadService : Service() { return START_NOT_STICKY } if (!foregroundStarted) { - startForegroundCompat(NOTIFICATION_ID, notification) - foregroundStarted = true + // Stays false when refused: later updates would post the ongoing notification through + // notify(), and the stop's stopForeground would leave that one stuck on screen. + foregroundStarted = startForegroundCompat(NOTIFICATION_ID, notification) } else { notificationManager?.notify(NOTIFICATION_ID, notification) } return START_STICKY } + /** + * Android 15 gives dataSync foreground services six hours a day; when they are spent the + * service has seconds to stop or the app crashes (#166). Tasks are paused, not lost. + */ + @RequiresApi(Build.VERSION_CODES.VANILLA_ICE_CREAM) + override fun onTimeout(startId: Int, fgsType: Int) { + // Stop first: each pause posts its notification through startForegroundService, and + // stopping under such a pending start crashes the app. + stopForeground(STOP_FOREGROUND_REMOVE) + foregroundStarted = false + stopSelf() + ArchiverDownloader.getInstance(this)?.pauseAll() + } + override fun onBind(intent: Intent?): IBinder? = null private fun showPausedNotification(notification: Notification, startId: Int) { @@ -209,11 +225,19 @@ class ArchiverDownloadService : Service() { return PendingIntent.getService(this, requestCode, intent, PENDING_INTENT_FLAGS) } - private fun startForegroundCompat(id: Int, notification: Notification) { - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) { - startForeground(id, notification, ServiceInfo.FOREGROUND_SERVICE_TYPE_DATA_SYNC) - } else { - startForeground(id, notification) + // Refused once the dataSync time is spent (#166), as DownloadService's is from the background. + // The download is an OkHttp call and carries on without the service's protection. + private fun startForegroundCompat(id: Int, notification: Notification): Boolean { + try { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) { + startForeground(id, notification, ServiceInfo.FOREGROUND_SERVICE_TYPE_DATA_SYNC) + } else { + startForeground(id, notification) + } + return true + } catch (e: IllegalStateException) { + android.util.Log.w("ArchiverDownloadService", "startForeground refused", e) + return false } } diff --git a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloader.kt b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloader.kt index 4f5f9213ce..055fa56cb3 100644 --- a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloader.kt +++ b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloader.kt @@ -115,6 +115,13 @@ class ArchiverDownloader private constructor(appContext: Context) { publishProgress(task, true) } + /** Pauses every running task; each one resumes later from its partial file, by Range. */ + fun pauseAll() { + for (gid in activeTasks.keys) { + pause(gid) + } + } + fun resume(gid: Long) { val task = activeTasks[gid] ?: return if (!task.paused) { diff --git a/app/src/main/java/com/hippo/ehviewer/download/DownloadService.kt b/app/src/main/java/com/hippo/ehviewer/download/DownloadService.kt index b54e305c02..925de6f244 100644 --- a/app/src/main/java/com/hippo/ehviewer/download/DownloadService.kt +++ b/app/src/main/java/com/hippo/ehviewer/download/DownloadService.kt @@ -156,6 +156,14 @@ class DownloadService : Service(), DownloadManager.DownloadListener { return START_STICKY } + // Android 15 gives dataSync foreground services six hours a day; when they are spent the + // service has seconds to stop or the app crashes (#166). This is the notification's "stop + // all": the downloads stay listed and start again from where they were. + override fun onTimeout(startId: Int, fgsType: Int) { + mDownloadManager?.stopAllDownload() + checkStopSelf() + } + private fun handleIntent(intent: Intent?) { var action: String? = null if (intent != null) { diff --git a/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloaderTest.java b/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloaderTest.java index 28beaee321..876b1fdebb 100644 --- a/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloaderTest.java +++ b/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloaderTest.java @@ -1,12 +1,18 @@ package com.hippo.ehviewer.download; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.robolectric.Shadows.shadowOf; import android.app.Application; +import android.app.ForegroundServiceStartNotAllowedException; +import android.app.Notification; +import android.app.NotificationManager; +import android.app.Service; import android.content.Context; import android.content.Intent; +import android.content.pm.ServiceInfo; import com.hippo.ehviewer.AppConfig; import com.hippo.ehviewer.EhApplication; @@ -30,13 +36,15 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.robolectric.Robolectric; import org.robolectric.RobolectricTestRunner; import org.robolectric.RuntimeEnvironment; import org.robolectric.annotation.Config; import org.robolectric.annotation.Implementation; import org.robolectric.annotation.Implements; +import org.robolectric.shadows.ShadowService; -/** Pins when the archive downloader lets its shared foreground service go (#166). */ +/** Pins when the archive downloader and its shared foreground service stop (#166). */ @RunWith(RobolectricTestRunner.class) @Config(application = android.app.Application.class, shadows = ArchiverDownloaderTest.ShadowEhApplication.class, @@ -128,8 +136,9 @@ public void setUp() throws Exception { downloader = ArchiverDownloader.getInstance(app); } - private void start(long gid) { - downloader.start(app, gallery(gid), "https://example.org/archive/" + gid, "archive" + gid); + private long start(long gid) { + return downloader.start(app, gallery(gid), "https://example.org/archive/" + gid, + "archive" + gid); } /** The actions of every service start since the last call. */ @@ -168,4 +177,51 @@ public void aPausedTask_doesNotKeepTheServiceUp() { assertTrue(serviceActions().contains(ArchiverDownloadService.ACTION_STOP)); } + + // --- Android 15's daily dataSync time running out ----------------------------------------- + + /** What every promotion gets once the time is spent. */ + @Implements(Service.class) + public static class ShadowSpentService extends ShadowService { + @Override + @Implementation + protected void startForeground(int id, Notification notification, int foregroundServiceType) { + throw new ForegroundServiceStartNotAllowedException( + "Time limit already exhausted for foreground service type dataSync"); + } + } + + @Test + public void timeout_pausesEveryTaskAndStopsTheService() { + long taskA = start(GID_A); + start(GID_B); + ArchiverDownloadService service = + Robolectric.buildService(ArchiverDownloadService.class).create().get(); + + service.onTimeout(1, ServiceInfo.FOREGROUND_SERVICE_TYPE_DATA_SYNC); + + assertTrue("the service outlived its timeout", shadowOf(service).isStoppedBySelf()); + assertTrue(downloader.getProgress(GID_A).paused); + assertTrue(downloader.getProgress(GID_B).paused); + assertTrue("the pause would not survive the process", + Settings.getArchiverDownloadPaused(taskA)); + } + + @Test + @Config(shadows = ShadowSpentService.class) + public void aRefusedPromotion_leavesNoNotificationThatNoStopCanRemove() { + ArchiverDownloadService service = + Robolectric.buildService(ArchiverDownloadService.class).create().get(); + Intent update = new Intent(app, ArchiverDownloadService.class) + .setAction(ArchiverDownloadService.ACTION_UPDATE) + .putExtra(ArchiverDownloadService.EXTRA_GID, GID_A) + .putExtra(ArchiverDownloadService.EXTRA_DOWNLOADED, 10L) + .putExtra(ArchiverDownloadService.EXTRA_TOTAL, 100L); + + service.onStartCommand(update, 0, 1); + service.onStartCommand(update, 0, 2); + + NotificationManager nm = (NotificationManager) app.getSystemService(Context.NOTIFICATION_SERVICE); + assertEquals(0, shadowOf(nm).size()); + } } diff --git a/app/src/test/java/com/hippo/ehviewer/download/DownloadServiceTest.java b/app/src/test/java/com/hippo/ehviewer/download/DownloadServiceTest.java new file mode 100644 index 0000000000..c542a3eef9 --- /dev/null +++ b/app/src/test/java/com/hippo/ehviewer/download/DownloadServiceTest.java @@ -0,0 +1,73 @@ +package com.hippo.ehviewer.download; + +import static org.junit.Assert.assertTrue; +import static org.robolectric.Shadows.shadowOf; + +import android.content.Context; +import android.content.pm.ServiceInfo; + +import com.hippo.ehviewer.EhApplication; +import com.hippo.ehviewer.Settings; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.Robolectric; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.RuntimeEnvironment; +import org.robolectric.annotation.Config; +import org.robolectric.annotation.Implementation; +import org.robolectric.annotation.Implements; +import org.robolectric.shadow.api.Shadow; + +/** Pins what the phone download service does when Android 15 ends its dataSync time (#166). */ +@RunWith(RobolectricTestRunner.class) +@Config(application = android.app.Application.class, + shadows = { + DownloadServiceTest.ShadowEhApplication.class, + DownloadServiceTest.ShadowDownloadManager.class, + }, + // Robolectric instruments by name prefix, so one class can be listed on its own. + instrumentedPackages = {"com.hippo.ehviewer.EhApplication", "com.hippo.ehviewer.download"}) +public class DownloadServiceTest { + + static boolean stoppedAll; + + @Implements(EhApplication.class) + public static class ShadowEhApplication { + @Implementation + protected static DownloadManager getDownloadManager(Context context) { + return Shadow.newInstanceOf(DownloadManager.class); + } + } + + /** A download is running until every download is stopped. */ + @Implements(DownloadManager.class) + public static class ShadowDownloadManager { + @Implementation + protected void stopAllDownload() { + stoppedAll = true; + } + + @Implementation + protected boolean isIdle() { + return stoppedAll; + } + } + + @Before + public void setUp() { + Settings.initialize(RuntimeEnvironment.getApplication()); + stoppedAll = false; + } + + @Test + public void timeout_stopsEveryDownloadAndThenTheService() { + DownloadService service = Robolectric.buildService(DownloadService.class).create().get(); + + service.onTimeout(1, ServiceInfo.FOREGROUND_SERVICE_TYPE_DATA_SYNC); + + assertTrue(stoppedAll); + assertTrue("the service outlived its timeout", shadowOf(service).isStoppedBySelf()); + } +} From a760c4c32905e35b73e50df45a5749740fde91e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=94=B5=E5=AD=90=E8=85=8B=E6=AF=9B=E7=A8=8B=E5=BA=8F/Ele?= =?UTF-8?q?ctronic=20hirci=20program?= Date: Sat, 3 Oct 2026 19:43:00 +1300 Subject: [PATCH 03/10] fix(smb): pause downloads on dataSync timeout #166 --- .../ehviewer/smb/SmbDirectDownloader.java | 10 +++ .../ehviewer/smb/SmbDownloadService.java | 33 ++++++++-- .../ehviewer/smb/SmbDirectDownloaderTest.java | 65 ++++++++++++++++++- 3 files changed, 101 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/hippo/ehviewer/smb/SmbDirectDownloader.java b/app/src/main/java/com/hippo/ehviewer/smb/SmbDirectDownloader.java index e1faa92d28..9399bffcb0 100644 --- a/app/src/main/java/com/hippo/ehviewer/smb/SmbDirectDownloader.java +++ b/app/src/main/java/com/hippo/ehviewer/smb/SmbDirectDownloader.java @@ -142,6 +142,16 @@ public void pause(long gid) { }); } + /** {@link #pause} for every task at once, queued ones included (#166). */ + public void pauseAll() { + SimpleHandler.getInstance().post(() -> { + for (SmbTaskLedger.ActiveJob job : ledger.suspendAll()) { + releaseQueen(job, "pause", job.info.gid); + } + afterQueueChange(); + }); + } + /** Resume a paused task by re-enqueueing it. No-op if the task isn't paused. */ public void resume(long gid) { SimpleHandler.getInstance().post(() -> { diff --git a/app/src/main/java/com/hippo/ehviewer/smb/SmbDownloadService.java b/app/src/main/java/com/hippo/ehviewer/smb/SmbDownloadService.java index 5532f46eae..df9c632e0c 100644 --- a/app/src/main/java/com/hippo/ehviewer/smb/SmbDownloadService.java +++ b/app/src/main/java/com/hippo/ehviewer/smb/SmbDownloadService.java @@ -80,9 +80,26 @@ public int onStartCommand(@Nullable Intent intent, int flags, int startId) { return START_STICKY; } + /** + * Android 15 gives dataSync foreground services six hours a day; when they are spent the + * service has seconds to stop or the app crashes (#166). Every task is held as if the user had + * paused it, and picks up from what is already on the share when resumed. + */ + @Override + public void onTimeout(int startId, int fgsType) { + SmbDirectDownloader.getInstance().pauseAll(); + stopForegroundCompat(); + stopSelf(); + } + @Override public void onDestroy() { SmbDirectDownloader.getInstance().detachService(); + // Normally gone with stopForeground already; this catches one posted while unpromoted. + NotificationManager nm = (NotificationManager) getSystemService(NOTIFICATION_SERVICE); + if (nm != null) { + nm.cancel(NOTIFICATION_ID); + } super.onDestroy(); } @@ -103,12 +120,18 @@ public void updateNotification(String title, String text, int max, int progress, } } + // Refused from the background on API 31+, and on Android 15 once the dataSync time is spent + // (#166). The service then runs unpromoted, as DownloadService does, until the queue is idle. private void startInForeground(Notification notification) { - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) { - startForeground(NOTIFICATION_ID, notification, - ServiceInfo.FOREGROUND_SERVICE_TYPE_DATA_SYNC); - } else { - startForeground(NOTIFICATION_ID, notification); + try { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) { + startForeground(NOTIFICATION_ID, notification, + ServiceInfo.FOREGROUND_SERVICE_TYPE_DATA_SYNC); + } else { + startForeground(NOTIFICATION_ID, notification); + } + } catch (IllegalStateException e) { + Log.w(TAG, "startForeground refused", e); } } diff --git a/app/src/test/java/com/hippo/ehviewer/smb/SmbDirectDownloaderTest.java b/app/src/test/java/com/hippo/ehviewer/smb/SmbDirectDownloaderTest.java index 0bd3abdc95..281b9e5991 100644 --- a/app/src/test/java/com/hippo/ehviewer/smb/SmbDirectDownloaderTest.java +++ b/app/src/test/java/com/hippo/ehviewer/smb/SmbDirectDownloaderTest.java @@ -6,7 +6,11 @@ import static org.junit.Assert.assertTrue; import static org.robolectric.Shadows.shadowOf; +import android.app.ForegroundServiceStartNotAllowedException; +import android.app.Notification; +import android.app.NotificationManager; import android.content.Context; +import android.content.pm.ServiceInfo; import android.os.Looper; import com.hippo.ehviewer.client.data.GalleryInfo; @@ -22,13 +26,16 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.robolectric.Robolectric; import org.robolectric.RobolectricTestRunner; import org.robolectric.RuntimeEnvironment; +import org.robolectric.android.controller.ServiceController; import org.robolectric.annotation.Config; import org.robolectric.annotation.Implementation; import org.robolectric.annotation.Implements; import org.robolectric.annotation.Resetter; import org.robolectric.shadow.api.Shadow; +import org.robolectric.shadows.ShadowService; /** Pins the SMB download task state machine (issue #43). */ @RunWith(RobolectricTestRunner.class) @@ -51,6 +58,11 @@ public class SmbDirectDownloaderTest { static CountDownLatch deleteLatch = new CountDownLatch(1); /** The spider listeners the downloader registered; a test fires finish through these. */ static final List listeners = new ArrayList<>(); + /** Android 15 once the daily dataSync time is spent: every promotion is refused. */ + static boolean spent = false; + + /** A real service a test built; attached to the downloader until destroyed. */ + private ServiceController service; /** Replaces the real fetch engine: no worker thread, no network. */ @Implements(SpiderQueen.class) @@ -86,9 +98,9 @@ protected void addOnSpiderListener(SpiderQueen.OnSpiderListener listener) { protected void removeOnSpiderListener(SpiderQueen.OnSpiderListener listener) {} } - /** Keeps the foreground service out of the test. */ + /** Keeps the foreground service out of the test; spent = Android 15 refusing promotion. */ @Implements(SmbDownloadService.class) - public static class ShadowSmbDownloadService { + public static class ShadowSmbDownloadService extends ShadowService { @Implementation protected static void start(Context context) { @@ -99,6 +111,15 @@ protected static void start(Context context) { protected static void stop(Context context) { calls.add("stopService"); } + + @Override + protected void startForeground(int id, Notification notification, int foregroundServiceType) { + if (spent) { + throw new ForegroundServiceStartNotAllowedException( + "Time limit already exhausted for foreground service type dataSync"); + } + super.startForeground(id, notification, foregroundServiceType); + } } /** Only the folder delete is stood in for. */ @@ -156,11 +177,17 @@ public void setUp() { calls.clear(); listeners.clear(); obtainThrows = false; + spent = false; deleteLatch = new CountDownLatch(1); } @After public void tearDown() { + // An attached service would stop the next test's first task from starting one. + if (service != null) { + service.destroy(); + service = null; + } // The downloader is a process-wide singleton, so leave no task behind for the next test. for (SmbDirectDownloader.TaskSnapshot t : new ArrayList<>(tasks())) { SmbDirectDownloader.getInstance().cancel(t.gid); @@ -344,4 +371,38 @@ public void service_startsWithTheFirstTask() { assertTrue(calls.contains("startService")); } + + // --- Android 15's daily dataSync time running out (#166) ------------------------------------ + + @Test + public void timeout_holdsEveryTaskAndStopsTheService() { + SmbDirectDownloader.getInstance().start(context, gallery(1)); + SmbDirectDownloader.getInstance().start(context, gallery(2)); + drain(); + service = Robolectric.buildService(SmbDownloadService.class).create(); + + service.get().onTimeout(1, ServiceInfo.FOREGROUND_SERVICE_TYPE_DATA_SYNC); + drain(); + + assertEquals(SmbDirectDownloader.TaskSnapshot.State.PAUSED, stateOf(1)); + assertEquals("the queued gallery must not start in its place", + SmbDirectDownloader.TaskSnapshot.State.PAUSED, stateOf(2)); + assertTrue("the queen must be released: " + calls, calls.contains("stop")); + assertTrue("the service outlived its timeout", shadowOf(service.get()).isStoppedBySelf()); + } + + @Test + public void aRefusedPromotion_isSurvivedAndLeavesNoNotificationBehind() { + spent = true; + service = Robolectric.buildService(SmbDownloadService.class).create(); + // What the downloader posts while the service runs unpromoted. + service.get().updateNotification("title", "text", 10, 3, false); + + service.destroy(); + service = null; + + NotificationManager nm = + (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE); + assertEquals(0, shadowOf(nm).size()); + } } From 7d8639f1b84dc64c57a1886acdd20c9272933ac3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=94=B5=E5=AD=90=E8=85=8B=E6=AF=9B=E7=A8=8B=E5=BA=8F/Ele?= =?UTF-8?q?ctronic=20hirci=20program?= Date: Sat, 3 Oct 2026 19:43:09 +1300 Subject: [PATCH 04/10] fix(download): unpack archives in the app cache #166 --- .../download/ArchiverDownloadCompleter.kt | 18 +- .../ArchiverDownloadCompleterTest.java | 178 ++++++++++++++++++ 2 files changed, 195 insertions(+), 1 deletion(-) create mode 100644 app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java diff --git a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadCompleter.kt b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadCompleter.kt index 157c3c45e0..814e554e12 100644 --- a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadCompleter.kt +++ b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadCompleter.kt @@ -44,7 +44,9 @@ class ArchiverDownloadCompleter private constructor(appContext: Context) { if (!tryBeginHandling(taskId)) { return } - val tempDir = AppConfig.getExternalTempDir() + // Without all-files access there is no external temp dir and the zip itself sits in the + // app's cache (#166): unpack there too, rather than fail and strand the zip out of reach. + val tempDir = AppConfig.getExternalTempDir() ?: AppConfig.getTempDir() if (tempDir == null) { endHandling(taskId) handleFailedTask(galleryInfo, taskId) @@ -66,6 +68,10 @@ class ArchiverDownloadCompleter private constructor(appContext: Context) { tempFile.deleteOnExit() } postImportFailed(galleryInfo, taskId) + } finally { + // Behind the success post, which forgets the task: a zip gone before that would be + // downloaded again by resumePending if the process died in between. + mainHandler.post { deleteIfPrivate(zipFile) } } }.start() } @@ -294,6 +300,16 @@ class ArchiverDownloadCompleter private constructor(appContext: Context) { return true } + /** + * Once an import is over, either way, a zip in the app's own cache goes: the user cannot + * reach it there, and the task that could use it again is gone (#166). + */ + private fun deleteIfPrivate(zipFile: File) { + if (zipFile.parentFile == AppConfig.getArchiverDir() && !zipFile.delete()) { + Log.w(TAG, "Failed to delete zip: ${zipFile.path}") + } + } + private fun deleteTempDir(destDir: File) { if (destDir.exists() && !YorozuyaFileUtils.delete(destDir)) { Log.w(TAG, "Failed to delete temp dir: ${destDir.path}") diff --git a/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java b/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java new file mode 100644 index 0000000000..1df9039312 --- /dev/null +++ b/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java @@ -0,0 +1,178 @@ +package com.hippo.ehviewer.download; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.robolectric.Shadows.shadowOf; + +import android.app.Application; +import android.content.Context; +import android.os.Environment; +import android.os.Looper; + +import com.hippo.ehviewer.AppConfig; +import com.hippo.ehviewer.EhApplication; +import com.hippo.ehviewer.EhDB; +import com.hippo.ehviewer.Settings; +import com.hippo.ehviewer.client.data.GalleryInfo; +import com.hippo.ehviewer.dao.DownloadInfo; +import com.hippo.unifile.UniFile; + +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.lang.reflect.Field; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.TimeUnit; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.RuntimeEnvironment; +import org.robolectric.annotation.Config; +import org.robolectric.annotation.Implementation; +import org.robolectric.annotation.Implements; +import org.robolectric.shadow.api.Shadow; +import org.robolectric.shadows.ShadowEnvironment; + +/** Pins where a downloaded archive is imported, and what is left of its zip (#166). */ +@RunWith(RobolectricTestRunner.class) +@Config(application = android.app.Application.class, + shadows = { + ArchiverDownloadCompleterTest.ShadowEhApplication.class, + ArchiverDownloadCompleterTest.ShadowDownloadManager.class, + }, + // Robolectric instruments by name prefix, so one class can be listed on its own. + instrumentedPackages = {"com.hippo.ehviewer.EhApplication", "com.hippo.ehviewer.download"}) +public class ArchiverDownloadCompleterTest { + + private static final long GID = 3054010L; + private static final long TASK_ID = 77L; + + /** What the import put on the download list, as gid:state. */ + static final List added = new ArrayList<>(); + + private Application app; + private File downloads; + + @Implements(EhApplication.class) + public static class ShadowEhApplication { + @Implementation + protected static DownloadManager getDownloadManager(Context context) { + return Shadow.newInstanceOf(DownloadManager.class); + } + } + + @Implements(DownloadManager.class) + public static class ShadowDownloadManager { + @Implementation + protected void addLabel(String label) {} + + @Implementation + protected void addDownload(GalleryInfo info, String label, int state) { + added.add(info.gid + ":" + state); + } + } + + @Before + public void setUp() throws Exception { + app = RuntimeEnvironment.getApplication(); + Settings.initialize(app); + AppConfig.initialize(app); + // The import names the gallery's folder through the download database. + EhDB.initialize(app); + // No all-files access: the app has no folder of its own on shared storage. + ShadowEnvironment.setExternalStorageState(Environment.MEDIA_UNMOUNTED); + downloads = new File(app.getCacheDir(), "phone-downloads"); + // The cache dir can outlive one test; start from an empty download location. + deleteTree(downloads); + assertTrue(downloads.mkdirs()); + Settings.putDownloadLocation(UniFile.fromFile(downloads)); + added.clear(); + // A process-wide singleton: without a fresh one it keeps the first test's application. + Field instance = ArchiverDownloadCompleter.class.getDeclaredField("sInstance"); + instance.setAccessible(true); + instance.set(null, null); + } + + private static GalleryInfo gallery() { + GalleryInfo info = new GalleryInfo(); + info.gid = GID; + info.token = "token"; + info.title = "archive fixture"; + return info; + } + + /** A two-page archive, laid out as the archiver serves one. */ + private static File archive(File dir) throws IOException { + File zip = new File(dir, "fixture.zip"); + try (ZipOutputStream out = new ZipOutputStream(new FileOutputStream(zip))) { + for (String page : new String[]{"01.jpg", "02.jpg"}) { + out.putNextEntry(new ZipEntry("archive fixture/" + page)); + out.write(page.getBytes(StandardCharsets.UTF_8)); + out.closeEntry(); + } + } + return zip; + } + + private void importArchive(File zip) { + ArchiverDownloadCompleter.getInstance(app).importDownloadedZip(zip, gallery(), TASK_ID); + } + + /** Runs the main thread until the import lets go of the zip, which is its last step. */ + private static void awaitZipGone(File zip) throws InterruptedException { + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5); + while (zip.exists()) { + assertTrue("the import never let go of " + zip.getName(), System.nanoTime() < deadline); + Thread.sleep(10); + shadowOf(Looper.getMainLooper()).idle(); + } + } + + private String[] galleryFolder() { + File[] made = downloads.listFiles((dir, name) -> name.startsWith(GID + "-")); + assertNotNull(made); + assertEquals("one folder for the gallery", 1, made.length); + return made[0].list(); + } + + @Test + public void withoutAllFilesAccess_theArchiveIsImportedAndItsZipDropped() throws Exception { + File zip = archive(AppConfig.getArchiverDir()); + + importArchive(zip); + awaitZipGone(zip); + + assertEquals(Collections.singletonList(GID + ":" + DownloadInfo.STATE_FINISH), added); + assertEquals(2, galleryFolder().length); + } + + @Test + public void aFailedImport_dropsTheZipToo() throws Exception { + File zip = new File(AppConfig.getArchiverDir(), "broken.zip"); + Files.write(zip.toPath(), "not a zip".getBytes(StandardCharsets.UTF_8)); + + importArchive(zip); + awaitZipGone(zip); + + assertTrue(added.isEmpty()); + } + + private static void deleteTree(File file) { + File[] children = file.listFiles(); + if (children != null) { + for (File child : children) { + deleteTree(child); + } + } + file.delete(); + } +} From 340121db78a72b8021a6d14122b8f91fd2b7da6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=94=B5=E5=AD=90=E8=85=8B=E6=AF=9B=E7=A8=8B=E5=BA=8F/Ele?= =?UTF-8?q?ctronic=20hirci=20program?= Date: Sat, 3 Oct 2026 19:43:16 +1300 Subject: [PATCH 05/10] fix(download): always import archives to the phone #166 --- .../download/ArchiverDownloadCompleter.kt | 8 ++--- .../ArchiverDownloadCompleterTest.java | 31 ++++++++++++++++++- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadCompleter.kt b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadCompleter.kt index 814e554e12..710e902759 100644 --- a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadCompleter.kt +++ b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadCompleter.kt @@ -13,7 +13,6 @@ import com.hippo.ehviewer.client.data.GalleryInfo import com.hippo.ehviewer.dao.DownloadInfo import com.hippo.ehviewer.gallery.GalleryProvider2 import com.hippo.ehviewer.spider.SpiderDen -import com.hippo.ehviewer.spider.SpiderQueen import com.hippo.lib.yorozuya.FileUtils as YorozuyaFileUtils import com.hippo.lib.yorozuya.StringUtils import com.hippo.lib.yorozuya.Utilities @@ -134,10 +133,9 @@ class ArchiverDownloadCompleter private constructor(appContext: Context) { } Collections.sort(tempPictures) { file1, file2 -> file1.name.compareTo(file2.name) } - val spiderDen = SpiderDen(galleryInfo) - spiderDen.setMode(SpiderQueen.MODE_DOWNLOAD) - spiderDen.prepareDownloadStorage() - val downloadDir = spiderDen.getDownloadDir() + // An archive is always imported to the phone (#166). A SpiderDen routes a gallery marked + // for network storage to the share, and has no phone folder to give for it. + val downloadDir = SpiderDen.getGalleryDownloadDir(galleryInfo)?.takeIf { it.ensureDir() } if (downloadDir == null) { postImportFailed(galleryInfo, taskId) return diff --git a/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java b/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java index 1df9039312..c0b747e010 100644 --- a/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java +++ b/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java @@ -16,6 +16,8 @@ import com.hippo.ehviewer.Settings; import com.hippo.ehviewer.client.data.GalleryInfo; import com.hippo.ehviewer.dao.DownloadInfo; +import com.hippo.ehviewer.smb.SmbSpiderStorage; +import com.hippo.ehviewer.storage.GalleryTargets; import com.hippo.unifile.UniFile; import java.io.File; @@ -48,9 +50,11 @@ shadows = { ArchiverDownloadCompleterTest.ShadowEhApplication.class, ArchiverDownloadCompleterTest.ShadowDownloadManager.class, + ArchiverDownloadCompleterTest.ShadowSmbSpiderStorage.class, }, // Robolectric instruments by name prefix, so one class can be listed on its own. - instrumentedPackages = {"com.hippo.ehviewer.EhApplication", "com.hippo.ehviewer.download"}) + instrumentedPackages = {"com.hippo.ehviewer.EhApplication", "com.hippo.ehviewer.download", + "com.hippo.ehviewer.smb.SmbSpiderStorage"}) public class ArchiverDownloadCompleterTest { private static final long GID = 3054010L; @@ -81,6 +85,15 @@ protected void addDownload(GalleryInfo info, String label, int state) { } } + /** The share, ready for any gallery routed to it, without a network round trip. */ + @Implements(SmbSpiderStorage.class) + public static class ShadowSmbSpiderStorage { + @Implementation + protected boolean prepareDir() { + return true; + } + } + @Before public void setUp() throws Exception { app = RuntimeEnvironment.getApplication(); @@ -166,6 +179,22 @@ public void aFailedImport_dropsTheZipToo() throws Exception { assertTrue(added.isEmpty()); } + /** Marked by an earlier download to the share, or by the share's list showing it. */ + @Test + public void aGalleryMarkedForTheShare_isStillImportedToThePhone() throws Exception { + File zip = archive(AppConfig.getArchiverDir()); + GalleryTargets.mark(GID); + try { + importArchive(zip); + awaitZipGone(zip); + } finally { + GalleryTargets.unmark(GID); + } + + assertEquals(Collections.singletonList(GID + ":" + DownloadInfo.STATE_FINISH), added); + assertEquals(2, galleryFolder().length); + } + private static void deleteTree(File file) { File[] children = file.listFiles(); if (children != null) { From e01948b057e9e8a4ce0b7ded3673ca9f56e26d5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=94=B5=E5=AD=90=E8=85=8B=E6=AF=9B=E7=A8=8B=E5=BA=8F/Ele?= =?UTF-8?q?ctronic=20hirci=20program?= Date: Sat, 3 Oct 2026 19:43:23 +1300 Subject: [PATCH 06/10] fix(download): add the gid to archive zip names #166 --- .../download/ArchiverDownloadCompleter.kt | 21 ++++++---- .../ArchiverDownloadCompleterTest.java | 42 +++++++++++++++++++ 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadCompleter.kt b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadCompleter.kt index 710e902759..d0238735cd 100644 --- a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadCompleter.kt +++ b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadCompleter.kt @@ -9,6 +9,7 @@ import com.hippo.ehviewer.AppConfig import com.hippo.ehviewer.EhApplication import com.hippo.ehviewer.R import com.hippo.ehviewer.Settings +import com.hippo.ehviewer.client.EhUtils import com.hippo.ehviewer.client.data.GalleryInfo import com.hippo.ehviewer.dao.DownloadInfo import com.hippo.ehviewer.gallery.GalleryProvider2 @@ -51,7 +52,9 @@ class ArchiverDownloadCompleter private constructor(appContext: Context) { handleFailedTask(galleryInfo, taskId) return } - val displayName = createFileName(galleryInfo.title, galleryInfo.gid) + // The gallery's name as the download list shows it, not its zip's. + val displayName = EhUtils.getSuitableTitle(galleryInfo) + ?: createFileName(galleryInfo.title, galleryInfo.gid) val tempFile = File(tempDir, "archiver_${galleryInfo.gid}") Thread { @@ -223,14 +226,18 @@ class ArchiverDownloadCompleter private constructor(appContext: Context) { return sInstance } + /** + * The zip's name: title, then gid (#166). Re-uploads and updated versions often keep the + * exact title, and two galleries sharing one zip delete or resume into each other's file. + */ @JvmStatic fun createFileName(name: String?, gid: Long): String { - var result = name?.let { YorozuyaFileUtils.sanitizeFilename(it) } ?: "" - result = truncateUtf8ToMaxBytes(result, MAX_ARCHIVER_BASENAME_UTF8_BYTES) - if (result.isEmpty()) { - result = if (gid > 0) "archiver_$gid" else "archiver" - } - return result + val suffix = if (gid > 0) "_$gid" else "" + val title = truncateUtf8ToMaxBytes( + name?.let { YorozuyaFileUtils.sanitizeFilename(it) } ?: "", + MAX_ARCHIVER_BASENAME_UTF8_BYTES - suffix.length + ) + return title.ifEmpty { "archiver" } + suffix } /** diff --git a/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java b/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java index c0b747e010..ed50f2df0b 100644 --- a/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java +++ b/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java @@ -1,6 +1,7 @@ package com.hippo.ehviewer.download; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.robolectric.Shadows.shadowOf; @@ -13,6 +14,7 @@ import com.hippo.ehviewer.AppConfig; import com.hippo.ehviewer.EhApplication; import com.hippo.ehviewer.EhDB; +import com.hippo.ehviewer.R; import com.hippo.ehviewer.Settings; import com.hippo.ehviewer.client.data.GalleryInfo; import com.hippo.ehviewer.dao.DownloadInfo; @@ -43,6 +45,7 @@ import org.robolectric.annotation.Implements; import org.robolectric.shadow.api.Shadow; import org.robolectric.shadows.ShadowEnvironment; +import org.robolectric.shadows.ShadowToast; /** Pins where a downloaded archive is imported, and what is left of its zip (#166). */ @RunWith(RobolectricTestRunner.class) @@ -195,6 +198,45 @@ public void aGalleryMarkedForTheShare_isStillImportedToThePhone() throws Excepti assertEquals(2, galleryFolder().length); } + @Test + public void theImportToast_namesTheGalleryNotItsZip() throws Exception { + File zip = archive(AppConfig.getArchiverDir()); + + importArchive(zip); + awaitZipGone(zip); + + assertEquals(app.getString(R.string.stat_download_done_line_succeeded, "archive fixture"), + ShadowToast.getTextOfLatestToast()); + } + + // --- the zip's name ------------------------------------------------------------------------- + + /** Re-uploads and updated versions often keep the exact title. */ + @Test + public void twoGalleriesWithOneTitle_getTwoZips() { + assertNotEquals(ArchiverDownloadCompleter.createFileName("one title", 1L), + ArchiverDownloadCompleter.createFileName("one title", 2L)); + } + + @Test + public void aLongTitle_makesRoomForTheGid() { + StringBuilder title = new StringBuilder(); + for (int i = 0; i < 200; i++) { + title.append('画'); // three bytes in UTF-8 + } + + String name = ArchiverDownloadCompleter.createFileName(title.toString(), GID); + + assertTrue(name, name.endsWith("_" + GID)); + assertTrue("one path segment holds 255 bytes", + (name + ".zip").getBytes(StandardCharsets.UTF_8).length <= 255); + } + + @Test + public void noTitle_fallsBackToTheGidAlone() { + assertEquals("archiver_" + GID, ArchiverDownloadCompleter.createFileName(null, GID)); + } + private static void deleteTree(File file) { File[] children = file.listFiles(); if (children != null) { From a8b222a827ecce8b0b5aca32f46fb056172de791 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=94=B5=E5=AD=90=E8=85=8B=E6=AF=9B=E7=A8=8B=E5=BA=8F/Ele?= =?UTF-8?q?ctronic=20hirci=20program?= Date: Sat, 3 Oct 2026 20:18:57 +1300 Subject: [PATCH 07/10] chore(smb): note the timeout use of suspendAll #166 --- app/src/main/java/com/hippo/ehviewer/smb/SmbTaskLedger.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/hippo/ehviewer/smb/SmbTaskLedger.java b/app/src/main/java/com/hippo/ehviewer/smb/SmbTaskLedger.java index 3525dafb67..5ea0fb3090 100644 --- a/app/src/main/java/com/hippo/ehviewer/smb/SmbTaskLedger.java +++ b/app/src/main/java/com/hippo/ehviewer/smb/SmbTaskLedger.java @@ -234,7 +234,10 @@ ActiveJob yield(long gid) { } } - /** Holds everything (SMB went away); returns the jobs the caller must release. */ + /** + * Holds everything (SMB went away, or Android ended the service's dataSync time); returns + * the jobs the caller must release. + */ @NonNull List suspendAll() { synchronized (lock) { From 342e8303923a5fc9b0ca16b0176163c8e733021a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=94=B5=E5=AD=90=E8=85=8B=E6=AF=9B=E7=A8=8B=E5=BA=8F/Ele?= =?UTF-8?q?ctronic=20hirci=20program?= Date: Sat, 3 Oct 2026 20:18:58 +1300 Subject: [PATCH 08/10] test(download): pin which zips an import keeps #166 --- .../ArchiverDownloadCompleterTest.java | 53 ++++++++++++------- 1 file changed, 35 insertions(+), 18 deletions(-) diff --git a/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java b/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java index ed50f2df0b..554aff162f 100644 --- a/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java +++ b/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java @@ -1,6 +1,7 @@ package com.hippo.ehviewer.download; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -31,6 +32,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.zip.ZipEntry; import java.util.zip.ZipOutputStream; @@ -139,18 +141,21 @@ private static File archive(File dir) throws IOException { return zip; } - private void importArchive(File zip) { + /** + * Runs one import to its end: joins the worker thread the completer starts, then runs what + * that thread posted to the main thread (the outcome first, then the clean-up of the zip). + */ + private void importAndWait(File zip) throws InterruptedException { + Set before = Thread.getAllStackTraces().keySet(); ArchiverDownloadCompleter.getInstance(app).importDownloadedZip(zip, gallery(), TASK_ID); - } - - /** Runs the main thread until the import lets go of the zip, which is its last step. */ - private static void awaitZipGone(File zip) throws InterruptedException { - long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5); - while (zip.exists()) { - assertTrue("the import never let go of " + zip.getName(), System.nanoTime() < deadline); - Thread.sleep(10); - shadowOf(Looper.getMainLooper()).idle(); + for (Thread thread : Thread.getAllStackTraces().keySet()) { + // The completer's worker is a plain thread, so it keeps the default name. + if (!before.contains(thread) && thread.getName().startsWith("Thread-")) { + thread.join(TimeUnit.SECONDS.toMillis(5)); + assertFalse("the import never finished", thread.isAlive()); + } } + shadowOf(Looper.getMainLooper()).idle(); } private String[] galleryFolder() { @@ -164,11 +169,11 @@ private String[] galleryFolder() { public void withoutAllFilesAccess_theArchiveIsImportedAndItsZipDropped() throws Exception { File zip = archive(AppConfig.getArchiverDir()); - importArchive(zip); - awaitZipGone(zip); + importAndWait(zip); assertEquals(Collections.singletonList(GID + ":" + DownloadInfo.STATE_FINISH), added); assertEquals(2, galleryFolder().length); + assertFalse("nobody can reach a zip left in the app's cache", zip.exists()); } @Test @@ -176,10 +181,24 @@ public void aFailedImport_dropsTheZipToo() throws Exception { File zip = new File(AppConfig.getArchiverDir(), "broken.zip"); Files.write(zip.toPath(), "not a zip".getBytes(StandardCharsets.UTF_8)); - importArchive(zip); - awaitZipGone(zip); + importAndWait(zip); assertTrue(added.isEmpty()); + assertFalse(zip.exists()); + } + + /** With all-files access the zip lands on shared storage, where the user can still open it. */ + @Test + public void aZipOutsideTheAppCache_isKeptAfterItsImport() throws Exception { + File shared = new File(app.getCacheDir(), "shared-archiver"); + deleteTree(shared); + assertTrue(shared.mkdirs()); + File zip = archive(shared); + + importAndWait(zip); + + assertEquals(Collections.singletonList(GID + ":" + DownloadInfo.STATE_FINISH), added); + assertTrue(zip.exists()); } /** Marked by an earlier download to the share, or by the share's list showing it. */ @@ -188,8 +207,7 @@ public void aGalleryMarkedForTheShare_isStillImportedToThePhone() throws Excepti File zip = archive(AppConfig.getArchiverDir()); GalleryTargets.mark(GID); try { - importArchive(zip); - awaitZipGone(zip); + importAndWait(zip); } finally { GalleryTargets.unmark(GID); } @@ -202,8 +220,7 @@ public void aGalleryMarkedForTheShare_isStillImportedToThePhone() throws Excepti public void theImportToast_namesTheGalleryNotItsZip() throws Exception { File zip = archive(AppConfig.getArchiverDir()); - importArchive(zip); - awaitZipGone(zip); + importAndWait(zip); assertEquals(app.getString(R.string.stat_download_done_line_succeeded, "archive fixture"), ShadowToast.getTextOfLatestToast()); From b361a47e2bc6f5f4abdfd2166082cf0c28441d5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=94=B5=E5=AD=90=E8=85=8B=E6=AF=9B=E7=A8=8B=E5=BA=8F/Ele?= =?UTF-8?q?ctronic=20hirci=20program?= Date: Sat, 3 Oct 2026 21:32:26 +1300 Subject: [PATCH 09/10] chore(download): shorten archive patch comments #166 --- .../download/ArchiverDownloadCompleter.kt | 19 +++++-------------- .../download/ArchiverDownloadService.kt | 13 +++---------- .../ehviewer/download/ArchiverDownloader.kt | 6 +----- .../ehviewer/download/DownloadService.kt | 3 --- .../ehviewer/smb/SmbDownloadService.java | 10 ++-------- .../com/hippo/ehviewer/smb/SmbTaskLedger.java | 5 +---- .../ArchiverDownloadCompleterTest.java | 18 ++++++------------ .../download/ArchiverDownloaderTest.java | 5 ++--- .../download/DownloadServiceTest.java | 1 - .../ehviewer/smb/SmbDirectDownloaderTest.java | 6 +++--- 10 files changed, 23 insertions(+), 63 deletions(-) diff --git a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadCompleter.kt b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadCompleter.kt index d0238735cd..0ae66a4eb8 100644 --- a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadCompleter.kt +++ b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadCompleter.kt @@ -44,8 +44,7 @@ class ArchiverDownloadCompleter private constructor(appContext: Context) { if (!tryBeginHandling(taskId)) { return } - // Without all-files access there is no external temp dir and the zip itself sits in the - // app's cache (#166): unpack there too, rather than fail and strand the zip out of reach. + // No external temp dir without all-files access (#166). val tempDir = AppConfig.getExternalTempDir() ?: AppConfig.getTempDir() if (tempDir == null) { endHandling(taskId) @@ -71,8 +70,7 @@ class ArchiverDownloadCompleter private constructor(appContext: Context) { } postImportFailed(galleryInfo, taskId) } finally { - // Behind the success post, which forgets the task: a zip gone before that would be - // downloaded again by resumePending if the process died in between. + // After the success post: a task still pending must keep its zip. mainHandler.post { deleteIfPrivate(zipFile) } } }.start() @@ -136,8 +134,7 @@ class ArchiverDownloadCompleter private constructor(appContext: Context) { } Collections.sort(tempPictures) { file1, file2 -> file1.name.compareTo(file2.name) } - // An archive is always imported to the phone (#166). A SpiderDen routes a gallery marked - // for network storage to the share, and has no phone folder to give for it. + // Always the phone: a SpiderDen routes marked galleries to the share (#166). val downloadDir = SpiderDen.getGalleryDownloadDir(galleryInfo)?.takeIf { it.ensureDir() } if (downloadDir == null) { postImportFailed(galleryInfo, taskId) @@ -226,10 +223,7 @@ class ArchiverDownloadCompleter private constructor(appContext: Context) { return sInstance } - /** - * The zip's name: title, then gid (#166). Re-uploads and updated versions often keep the - * exact title, and two galleries sharing one zip delete or resume into each other's file. - */ + /** Title plus gid: re-uploads often keep the exact title (#166). */ @JvmStatic fun createFileName(name: String?, gid: Long): String { val suffix = if (gid > 0) "_$gid" else "" @@ -305,10 +299,7 @@ class ArchiverDownloadCompleter private constructor(appContext: Context) { return true } - /** - * Once an import is over, either way, a zip in the app's own cache goes: the user cannot - * reach it there, and the task that could use it again is gone (#166). - */ + /** The user cannot reach a zip in the app's cache. */ private fun deleteIfPrivate(zipFile: File) { if (zipFile.parentFile == AppConfig.getArchiverDir() && !zipFile.delete()) { Log.w(TAG, "Failed to delete zip: ${zipFile.path}") diff --git a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadService.kt b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadService.kt index d1ccaf5dec..748ca69b30 100644 --- a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadService.kt +++ b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadService.kt @@ -96,8 +96,7 @@ class ArchiverDownloadService : Service() { return START_NOT_STICKY } if (!foregroundStarted) { - // Stays false when refused: later updates would post the ongoing notification through - // notify(), and the stop's stopForeground would leave that one stuck on screen. + // False when refused: an ongoing notify() would outlive the stop. foregroundStarted = startForegroundCompat(NOTIFICATION_ID, notification) } else { notificationManager?.notify(NOTIFICATION_ID, notification) @@ -105,14 +104,9 @@ class ArchiverDownloadService : Service() { return START_STICKY } - /** - * Android 15 gives dataSync foreground services six hours a day; when they are spent the - * service has seconds to stop or the app crashes (#166). Tasks are paused, not lost. - */ @RequiresApi(Build.VERSION_CODES.VANILLA_ICE_CREAM) override fun onTimeout(startId: Int, fgsType: Int) { - // Stop first: each pause posts its notification through startForegroundService, and - // stopping under such a pending start crashes the app. + // Stop first: stopping under a pause's pending startForegroundService crashes. stopForeground(STOP_FOREGROUND_REMOVE) foregroundStarted = false stopSelf() @@ -225,8 +219,7 @@ class ArchiverDownloadService : Service() { return PendingIntent.getService(this, requestCode, intent, PENDING_INTENT_FLAGS) } - // Refused once the dataSync time is spent (#166), as DownloadService's is from the background. - // The download is an OkHttp call and carries on without the service's protection. + // Refused once the dataSync time is spent (#166). private fun startForegroundCompat(id: Int, notification: Notification): Boolean { try { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) { diff --git a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloader.kt b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloader.kt index 055fa56cb3..f338a44d24 100644 --- a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloader.kt +++ b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloader.kt @@ -115,7 +115,6 @@ class ArchiverDownloader private constructor(appContext: Context) { publishProgress(task, true) } - /** Pauses every running task; each one resumes later from its partial file, by Range. */ fun pauseAll() { for (gid in activeTasks.keys) { pause(gid) @@ -140,10 +139,7 @@ class ArchiverDownloader private constructor(appContext: Context) { stopServiceIfIdle() } - /** - * Every task shares one service and one notification (#166): a task that ends stops it only - * when no other task is still downloading. Paused tasks need no foreground service. - */ + /** One service serves every task (#166). */ private fun stopServiceIfIdle() { if (activeTasks.values.none { !it.paused }) { ArchiverDownloadService.stop(appContext) diff --git a/app/src/main/java/com/hippo/ehviewer/download/DownloadService.kt b/app/src/main/java/com/hippo/ehviewer/download/DownloadService.kt index 925de6f244..b416ab5b59 100644 --- a/app/src/main/java/com/hippo/ehviewer/download/DownloadService.kt +++ b/app/src/main/java/com/hippo/ehviewer/download/DownloadService.kt @@ -156,9 +156,6 @@ class DownloadService : Service(), DownloadManager.DownloadListener { return START_STICKY } - // Android 15 gives dataSync foreground services six hours a day; when they are spent the - // service has seconds to stop or the app crashes (#166). This is the notification's "stop - // all": the downloads stay listed and start again from where they were. override fun onTimeout(startId: Int, fgsType: Int) { mDownloadManager?.stopAllDownload() checkStopSelf() diff --git a/app/src/main/java/com/hippo/ehviewer/smb/SmbDownloadService.java b/app/src/main/java/com/hippo/ehviewer/smb/SmbDownloadService.java index df9c632e0c..9987d8e4b0 100644 --- a/app/src/main/java/com/hippo/ehviewer/smb/SmbDownloadService.java +++ b/app/src/main/java/com/hippo/ehviewer/smb/SmbDownloadService.java @@ -80,11 +80,6 @@ public int onStartCommand(@Nullable Intent intent, int flags, int startId) { return START_STICKY; } - /** - * Android 15 gives dataSync foreground services six hours a day; when they are spent the - * service has seconds to stop or the app crashes (#166). Every task is held as if the user had - * paused it, and picks up from what is already on the share when resumed. - */ @Override public void onTimeout(int startId, int fgsType) { SmbDirectDownloader.getInstance().pauseAll(); @@ -95,7 +90,7 @@ public void onTimeout(int startId, int fgsType) { @Override public void onDestroy() { SmbDirectDownloader.getInstance().detachService(); - // Normally gone with stopForeground already; this catches one posted while unpromoted. + // stopForeground leaves one that was posted while unpromoted. NotificationManager nm = (NotificationManager) getSystemService(NOTIFICATION_SERVICE); if (nm != null) { nm.cancel(NOTIFICATION_ID); @@ -120,8 +115,7 @@ public void updateNotification(String title, String text, int max, int progress, } } - // Refused from the background on API 31+, and on Android 15 once the dataSync time is spent - // (#166). The service then runs unpromoted, as DownloadService does, until the queue is idle. + // Refused as DownloadService.startForegroundCompat can be, plus the dataSync time (#166). private void startInForeground(Notification notification) { try { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) { diff --git a/app/src/main/java/com/hippo/ehviewer/smb/SmbTaskLedger.java b/app/src/main/java/com/hippo/ehviewer/smb/SmbTaskLedger.java index 5ea0fb3090..7f680dce01 100644 --- a/app/src/main/java/com/hippo/ehviewer/smb/SmbTaskLedger.java +++ b/app/src/main/java/com/hippo/ehviewer/smb/SmbTaskLedger.java @@ -234,10 +234,7 @@ ActiveJob yield(long gid) { } } - /** - * Holds everything (SMB went away, or Android ended the service's dataSync time); returns - * the jobs the caller must release. - */ + /** Holds everything (SMB gone, or the dataSync time spent); returns the jobs to release. */ @NonNull List suspendAll() { synchronized (lock) { diff --git a/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java b/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java index 554aff162f..e7790f8597 100644 --- a/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java +++ b/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java @@ -57,7 +57,6 @@ ArchiverDownloadCompleterTest.ShadowDownloadManager.class, ArchiverDownloadCompleterTest.ShadowSmbSpiderStorage.class, }, - // Robolectric instruments by name prefix, so one class can be listed on its own. instrumentedPackages = {"com.hippo.ehviewer.EhApplication", "com.hippo.ehviewer.download", "com.hippo.ehviewer.smb.SmbSpiderStorage"}) public class ArchiverDownloadCompleterTest { @@ -90,7 +89,7 @@ protected void addDownload(GalleryInfo info, String label, int state) { } } - /** The share, ready for any gallery routed to it, without a network round trip. */ + /** Spares a routed import the network round trip. */ @Implements(SmbSpiderStorage.class) public static class ShadowSmbSpiderStorage { @Implementation @@ -106,15 +105,14 @@ public void setUp() throws Exception { AppConfig.initialize(app); // The import names the gallery's folder through the download database. EhDB.initialize(app); - // No all-files access: the app has no folder of its own on shared storage. + // No all-files access. ShadowEnvironment.setExternalStorageState(Environment.MEDIA_UNMOUNTED); downloads = new File(app.getCacheDir(), "phone-downloads"); - // The cache dir can outlive one test; start from an empty download location. + // The cache dir outlives a test. deleteTree(downloads); assertTrue(downloads.mkdirs()); Settings.putDownloadLocation(UniFile.fromFile(downloads)); added.clear(); - // A process-wide singleton: without a fresh one it keeps the first test's application. Field instance = ArchiverDownloadCompleter.class.getDeclaredField("sInstance"); instance.setAccessible(true); instance.set(null, null); @@ -141,15 +139,12 @@ private static File archive(File dir) throws IOException { return zip; } - /** - * Runs one import to its end: joins the worker thread the completer starts, then runs what - * that thread posted to the main thread (the outcome first, then the clean-up of the zip). - */ + /** Runs an import to its end: its worker thread, then what it posted to the main thread. */ private void importAndWait(File zip) throws InterruptedException { Set before = Thread.getAllStackTraces().keySet(); ArchiverDownloadCompleter.getInstance(app).importDownloadedZip(zip, gallery(), TASK_ID); for (Thread thread : Thread.getAllStackTraces().keySet()) { - // The completer's worker is a plain thread, so it keeps the default name. + // The worker keeps the default thread name. if (!before.contains(thread) && thread.getName().startsWith("Thread-")) { thread.join(TimeUnit.SECONDS.toMillis(5)); assertFalse("the import never finished", thread.isAlive()); @@ -187,7 +182,7 @@ public void aFailedImport_dropsTheZipToo() throws Exception { assertFalse(zip.exists()); } - /** With all-files access the zip lands on shared storage, where the user can still open it. */ + /** With all-files access the zip lands on shared storage. */ @Test public void aZipOutsideTheAppCache_isKeptAfterItsImport() throws Exception { File shared = new File(app.getCacheDir(), "shared-archiver"); @@ -228,7 +223,6 @@ public void theImportToast_namesTheGalleryNotItsZip() throws Exception { // --- the zip's name ------------------------------------------------------------------------- - /** Re-uploads and updated versions often keep the exact title. */ @Test public void twoGalleriesWithOneTitle_getTwoZips() { assertNotEquals(ArchiverDownloadCompleter.createFileName("one title", 1L), diff --git a/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloaderTest.java b/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloaderTest.java index 876b1fdebb..149c799ebc 100644 --- a/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloaderTest.java +++ b/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloaderTest.java @@ -118,7 +118,7 @@ public void setUp() throws Exception { app = RuntimeEnvironment.getApplication(); Settings.initialize(app); AppConfig.initialize(app); - // Every call fails at once, which ends its task the way a dead link would. + // Every call fails, which ends its task. client = new OkHttpClient.Builder() .dispatcher(new Dispatcher(calls)) .addInterceptor(chain -> new Response.Builder() @@ -129,7 +129,6 @@ public void setUp() throws Exception { .body(ResponseBody.create(null, "")) .build()) .build(); - // A process-wide singleton: without a fresh one it keeps the first test's application. Field instance = ArchiverDownloader.class.getDeclaredField("sInstance"); instance.setAccessible(true); instance.set(null, null); @@ -178,7 +177,7 @@ public void aPausedTask_doesNotKeepTheServiceUp() { assertTrue(serviceActions().contains(ArchiverDownloadService.ACTION_STOP)); } - // --- Android 15's daily dataSync time running out ----------------------------------------- + // --- dataSync timeout --------------------------------------------------------------------- /** What every promotion gets once the time is spent. */ @Implements(Service.class) diff --git a/app/src/test/java/com/hippo/ehviewer/download/DownloadServiceTest.java b/app/src/test/java/com/hippo/ehviewer/download/DownloadServiceTest.java index c542a3eef9..a24a65a424 100644 --- a/app/src/test/java/com/hippo/ehviewer/download/DownloadServiceTest.java +++ b/app/src/test/java/com/hippo/ehviewer/download/DownloadServiceTest.java @@ -27,7 +27,6 @@ DownloadServiceTest.ShadowEhApplication.class, DownloadServiceTest.ShadowDownloadManager.class, }, - // Robolectric instruments by name prefix, so one class can be listed on its own. instrumentedPackages = {"com.hippo.ehviewer.EhApplication", "com.hippo.ehviewer.download"}) public class DownloadServiceTest { diff --git a/app/src/test/java/com/hippo/ehviewer/smb/SmbDirectDownloaderTest.java b/app/src/test/java/com/hippo/ehviewer/smb/SmbDirectDownloaderTest.java index 281b9e5991..aa0f367092 100644 --- a/app/src/test/java/com/hippo/ehviewer/smb/SmbDirectDownloaderTest.java +++ b/app/src/test/java/com/hippo/ehviewer/smb/SmbDirectDownloaderTest.java @@ -58,7 +58,7 @@ public class SmbDirectDownloaderTest { static CountDownLatch deleteLatch = new CountDownLatch(1); /** The spider listeners the downloader registered; a test fires finish through these. */ static final List listeners = new ArrayList<>(); - /** Android 15 once the daily dataSync time is spent: every promotion is refused. */ + /** Refuses every promotion, as once the dataSync time is spent. */ static boolean spent = false; /** A real service a test built; attached to the downloader until destroyed. */ @@ -98,7 +98,7 @@ protected void addOnSpiderListener(SpiderQueen.OnSpiderListener listener) { protected void removeOnSpiderListener(SpiderQueen.OnSpiderListener listener) {} } - /** Keeps the foreground service out of the test; spent = Android 15 refusing promotion. */ + /** Keeps the foreground service out of the test. */ @Implements(SmbDownloadService.class) public static class ShadowSmbDownloadService extends ShadowService { @@ -372,7 +372,7 @@ public void service_startsWithTheFirstTask() { assertTrue(calls.contains("startService")); } - // --- Android 15's daily dataSync time running out (#166) ------------------------------------ + // --- dataSync timeout --------------------------------------------------------------------- @Test public void timeout_holdsEveryTaskAndStopsTheService() { From 282fac8cd47177800b6728269cfa50af6b7e7829 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=94=B5=E5=AD=90=E8=85=8B=E6=AF=9B=E7=A8=8B=E5=BA=8F/Ele?= =?UTF-8?q?ctronic=20hirci=20program?= Date: Sat, 3 Oct 2026 21:49:16 +1300 Subject: [PATCH 10/10] chore(download): drop comments the code explains #166 --- .../com/hippo/ehviewer/download/ArchiverDownloadCompleter.kt | 5 ----- .../com/hippo/ehviewer/download/ArchiverDownloadService.kt | 2 -- .../java/com/hippo/ehviewer/download/ArchiverDownloader.kt | 1 - .../main/java/com/hippo/ehviewer/smb/SmbDownloadService.java | 2 -- app/src/main/java/com/hippo/ehviewer/smb/SmbTaskLedger.java | 2 +- .../ehviewer/download/ArchiverDownloadCompleterTest.java | 4 ---- .../com/hippo/ehviewer/download/ArchiverDownloaderTest.java | 3 --- .../com/hippo/ehviewer/download/DownloadServiceTest.java | 1 - .../java/com/hippo/ehviewer/smb/SmbDirectDownloaderTest.java | 1 - 9 files changed, 1 insertion(+), 20 deletions(-) diff --git a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadCompleter.kt b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadCompleter.kt index 0ae66a4eb8..12b39780de 100644 --- a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadCompleter.kt +++ b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadCompleter.kt @@ -44,14 +44,12 @@ class ArchiverDownloadCompleter private constructor(appContext: Context) { if (!tryBeginHandling(taskId)) { return } - // No external temp dir without all-files access (#166). val tempDir = AppConfig.getExternalTempDir() ?: AppConfig.getTempDir() if (tempDir == null) { endHandling(taskId) handleFailedTask(galleryInfo, taskId) return } - // The gallery's name as the download list shows it, not its zip's. val displayName = EhUtils.getSuitableTitle(galleryInfo) ?: createFileName(galleryInfo.title, galleryInfo.gid) val tempFile = File(tempDir, "archiver_${galleryInfo.gid}") @@ -134,7 +132,6 @@ class ArchiverDownloadCompleter private constructor(appContext: Context) { } Collections.sort(tempPictures) { file1, file2 -> file1.name.compareTo(file2.name) } - // Always the phone: a SpiderDen routes marked galleries to the share (#166). val downloadDir = SpiderDen.getGalleryDownloadDir(galleryInfo)?.takeIf { it.ensureDir() } if (downloadDir == null) { postImportFailed(galleryInfo, taskId) @@ -223,7 +220,6 @@ class ArchiverDownloadCompleter private constructor(appContext: Context) { return sInstance } - /** Title plus gid: re-uploads often keep the exact title (#166). */ @JvmStatic fun createFileName(name: String?, gid: Long): String { val suffix = if (gid > 0) "_$gid" else "" @@ -299,7 +295,6 @@ class ArchiverDownloadCompleter private constructor(appContext: Context) { return true } - /** The user cannot reach a zip in the app's cache. */ private fun deleteIfPrivate(zipFile: File) { if (zipFile.parentFile == AppConfig.getArchiverDir() && !zipFile.delete()) { Log.w(TAG, "Failed to delete zip: ${zipFile.path}") diff --git a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadService.kt b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadService.kt index 748ca69b30..e3f9c6df19 100644 --- a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadService.kt +++ b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloadService.kt @@ -96,7 +96,6 @@ class ArchiverDownloadService : Service() { return START_NOT_STICKY } if (!foregroundStarted) { - // False when refused: an ongoing notify() would outlive the stop. foregroundStarted = startForegroundCompat(NOTIFICATION_ID, notification) } else { notificationManager?.notify(NOTIFICATION_ID, notification) @@ -219,7 +218,6 @@ class ArchiverDownloadService : Service() { return PendingIntent.getService(this, requestCode, intent, PENDING_INTENT_FLAGS) } - // Refused once the dataSync time is spent (#166). private fun startForegroundCompat(id: Int, notification: Notification): Boolean { try { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) { diff --git a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloader.kt b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloader.kt index f338a44d24..0ce4289d9b 100644 --- a/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloader.kt +++ b/app/src/main/java/com/hippo/ehviewer/download/ArchiverDownloader.kt @@ -139,7 +139,6 @@ class ArchiverDownloader private constructor(appContext: Context) { stopServiceIfIdle() } - /** One service serves every task (#166). */ private fun stopServiceIfIdle() { if (activeTasks.values.none { !it.paused }) { ArchiverDownloadService.stop(appContext) diff --git a/app/src/main/java/com/hippo/ehviewer/smb/SmbDownloadService.java b/app/src/main/java/com/hippo/ehviewer/smb/SmbDownloadService.java index 9987d8e4b0..b61f76f666 100644 --- a/app/src/main/java/com/hippo/ehviewer/smb/SmbDownloadService.java +++ b/app/src/main/java/com/hippo/ehviewer/smb/SmbDownloadService.java @@ -90,7 +90,6 @@ public void onTimeout(int startId, int fgsType) { @Override public void onDestroy() { SmbDirectDownloader.getInstance().detachService(); - // stopForeground leaves one that was posted while unpromoted. NotificationManager nm = (NotificationManager) getSystemService(NOTIFICATION_SERVICE); if (nm != null) { nm.cancel(NOTIFICATION_ID); @@ -115,7 +114,6 @@ public void updateNotification(String title, String text, int max, int progress, } } - // Refused as DownloadService.startForegroundCompat can be, plus the dataSync time (#166). private void startInForeground(Notification notification) { try { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) { diff --git a/app/src/main/java/com/hippo/ehviewer/smb/SmbTaskLedger.java b/app/src/main/java/com/hippo/ehviewer/smb/SmbTaskLedger.java index 7f680dce01..32c9605838 100644 --- a/app/src/main/java/com/hippo/ehviewer/smb/SmbTaskLedger.java +++ b/app/src/main/java/com/hippo/ehviewer/smb/SmbTaskLedger.java @@ -234,7 +234,7 @@ ActiveJob yield(long gid) { } } - /** Holds everything (SMB gone, or the dataSync time spent); returns the jobs to release. */ + /** Holds everything; returns the jobs the caller must release. */ @NonNull List suspendAll() { synchronized (lock) { diff --git a/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java b/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java index e7790f8597..4f5c10b1bb 100644 --- a/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java +++ b/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloadCompleterTest.java @@ -126,7 +126,6 @@ private static GalleryInfo gallery() { return info; } - /** A two-page archive, laid out as the archiver serves one. */ private static File archive(File dir) throws IOException { File zip = new File(dir, "fixture.zip"); try (ZipOutputStream out = new ZipOutputStream(new FileOutputStream(zip))) { @@ -139,7 +138,6 @@ private static File archive(File dir) throws IOException { return zip; } - /** Runs an import to its end: its worker thread, then what it posted to the main thread. */ private void importAndWait(File zip) throws InterruptedException { Set before = Thread.getAllStackTraces().keySet(); ArchiverDownloadCompleter.getInstance(app).importDownloadedZip(zip, gallery(), TASK_ID); @@ -182,7 +180,6 @@ public void aFailedImport_dropsTheZipToo() throws Exception { assertFalse(zip.exists()); } - /** With all-files access the zip lands on shared storage. */ @Test public void aZipOutsideTheAppCache_isKeptAfterItsImport() throws Exception { File shared = new File(app.getCacheDir(), "shared-archiver"); @@ -196,7 +193,6 @@ public void aZipOutsideTheAppCache_isKeptAfterItsImport() throws Exception { assertTrue(zip.exists()); } - /** Marked by an earlier download to the share, or by the share's list showing it. */ @Test public void aGalleryMarkedForTheShare_isStillImportedToThePhone() throws Exception { File zip = archive(AppConfig.getArchiverDir()); diff --git a/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloaderTest.java b/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloaderTest.java index 149c799ebc..300390b62b 100644 --- a/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloaderTest.java +++ b/app/src/test/java/com/hippo/ehviewer/download/ArchiverDownloaderTest.java @@ -118,7 +118,6 @@ public void setUp() throws Exception { app = RuntimeEnvironment.getApplication(); Settings.initialize(app); AppConfig.initialize(app); - // Every call fails, which ends its task. client = new OkHttpClient.Builder() .dispatcher(new Dispatcher(calls)) .addInterceptor(chain -> new Response.Builder() @@ -140,7 +139,6 @@ private long start(long gid) { "archive" + gid); } - /** The actions of every service start since the last call. */ private List serviceActions() { List actions = new ArrayList<>(); Intent intent; @@ -179,7 +177,6 @@ public void aPausedTask_doesNotKeepTheServiceUp() { // --- dataSync timeout --------------------------------------------------------------------- - /** What every promotion gets once the time is spent. */ @Implements(Service.class) public static class ShadowSpentService extends ShadowService { @Override diff --git a/app/src/test/java/com/hippo/ehviewer/download/DownloadServiceTest.java b/app/src/test/java/com/hippo/ehviewer/download/DownloadServiceTest.java index a24a65a424..98a4d73acf 100644 --- a/app/src/test/java/com/hippo/ehviewer/download/DownloadServiceTest.java +++ b/app/src/test/java/com/hippo/ehviewer/download/DownloadServiceTest.java @@ -40,7 +40,6 @@ protected static DownloadManager getDownloadManager(Context context) { } } - /** A download is running until every download is stopped. */ @Implements(DownloadManager.class) public static class ShadowDownloadManager { @Implementation diff --git a/app/src/test/java/com/hippo/ehviewer/smb/SmbDirectDownloaderTest.java b/app/src/test/java/com/hippo/ehviewer/smb/SmbDirectDownloaderTest.java index aa0f367092..700741a44d 100644 --- a/app/src/test/java/com/hippo/ehviewer/smb/SmbDirectDownloaderTest.java +++ b/app/src/test/java/com/hippo/ehviewer/smb/SmbDirectDownloaderTest.java @@ -395,7 +395,6 @@ public void timeout_holdsEveryTaskAndStopsTheService() { public void aRefusedPromotion_isSurvivedAndLeavesNoNotificationBehind() { spent = true; service = Robolectric.buildService(SmbDownloadService.class).create(); - // What the downloader posts while the service runs unpromoted. service.get().updateNotification("title", "text", 10, 3, false); service.destroy();