fix: Resolve Chapter Download Queue and Removal Issues (Closes #1937)
This commit is contained in:
+5
@@ -9,6 +9,7 @@ import androidx.work.ExistingWorkPolicy
|
||||
import androidx.work.NetworkType
|
||||
import androidx.work.OneTimeWorkRequestBuilder
|
||||
import androidx.work.WorkManager
|
||||
import androidx.work.await
|
||||
import java.util.UUID
|
||||
|
||||
object BackgroundTaskScheduler {
|
||||
@@ -36,6 +37,10 @@ object BackgroundTaskScheduler {
|
||||
return request.id
|
||||
}
|
||||
|
||||
suspend fun cancel(context: Context, taskId: String) {
|
||||
WorkManager.getInstance(context).cancelAllWorkByTag(taskId).await()
|
||||
}
|
||||
|
||||
suspend fun enqueueLibraryUpdate(
|
||||
context: Context,
|
||||
title: String,
|
||||
|
||||
+1
@@ -107,6 +107,7 @@ class NativeBackgroundTasksModule : Module() {
|
||||
if (TaskExecutionRegistry.isActive(taskId)) {
|
||||
emitInterruption(taskId, "cancel")
|
||||
}
|
||||
BackgroundTaskScheduler.cancel(appContext.reactContext!!, taskId)
|
||||
dao.updateCheckpoint(taskId, null, System.currentTimeMillis())
|
||||
TaskNotificationFactory.dismiss(appContext.reactContext!!, taskId)
|
||||
}
|
||||
|
||||
+2
-1
@@ -33,6 +33,7 @@ class TaskActionReceiver : BroadcastReceiver() {
|
||||
if (TaskExecutionRegistry.isActive(taskId)) {
|
||||
NativeBackgroundTasksModule.emitInterruption(taskId, "cancel")
|
||||
}
|
||||
BackgroundTaskScheduler.cancel(context, taskId)
|
||||
TaskNotificationFactory.dismiss(context, taskId)
|
||||
}
|
||||
}
|
||||
@@ -41,4 +42,4 @@ class TaskActionReceiver : BroadcastReceiver() {
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -245,7 +245,6 @@ export const increaseTimeSpent = async (
|
||||
});
|
||||
};
|
||||
|
||||
// TODO: Remove the need for the chapters array, as it could lead to not deleting the downloaded files but just marking them as not downloaded
|
||||
/*
|
||||
Deletes all downloaded chapters from the database
|
||||
*/
|
||||
@@ -255,19 +254,28 @@ export const deleteDownloads = async (
|
||||
if (!chapters?.length) {
|
||||
return;
|
||||
}
|
||||
chapters.forEach(chapter => {
|
||||
deleteDownloadedFiles(chapter.pluginId, chapter.novelId, chapter.id);
|
||||
});
|
||||
await Promise.all(
|
||||
chapters.map(chapter =>
|
||||
deleteDownloadedFiles(chapter.pluginId, chapter.novelId, chapter.id),
|
||||
),
|
||||
);
|
||||
const chapterIds = chapters.map(chapter => chapter.id);
|
||||
await dbManager.write(async tx => {
|
||||
await tx.update(chapterSchema).set({ isDownloaded: false }).run();
|
||||
await tx
|
||||
.update(chapterSchema)
|
||||
.set({ isDownloaded: false })
|
||||
.where(inArray(chapterSchema.id, chapterIds))
|
||||
.run();
|
||||
});
|
||||
};
|
||||
|
||||
export const deleteReadChaptersFromDb = async (): Promise<void> => {
|
||||
const chapters = await getReadDownloadedChapters();
|
||||
chapters?.forEach(chapter => {
|
||||
deleteDownloadedFiles(chapter.pluginId, chapter.novelId, chapter.id);
|
||||
});
|
||||
await Promise.all(
|
||||
chapters.map(chapter =>
|
||||
deleteDownloadedFiles(chapter.pluginId, chapter.novelId, chapter.id),
|
||||
),
|
||||
);
|
||||
const chapterIds = chapters?.map(chapter => chapter.id);
|
||||
if (chapterIds?.length) {
|
||||
await dbManager.write(async tx => {
|
||||
@@ -460,7 +468,8 @@ export const getAllUndownloadedChapters = async (
|
||||
eq(chapterSchema.novelId, novelId),
|
||||
eq(chapterSchema.isDownloaded, false),
|
||||
),
|
||||
);
|
||||
)
|
||||
.orderBy(asc(castInt(chapterSchema.page)), asc(chapterSchema.position));
|
||||
/**
|
||||
* @deprecated, use getNovelChapters with whereConditions instead
|
||||
*/
|
||||
@@ -477,6 +486,7 @@ export const getAllUndownloadedAndUnreadChapters = async (
|
||||
eq(chapterSchema.unread, true),
|
||||
),
|
||||
)
|
||||
.orderBy(asc(castInt(chapterSchema.page)), asc(chapterSchema.position))
|
||||
.all();
|
||||
|
||||
export const getChapter = async (chapterId: number) =>
|
||||
|
||||
@@ -509,9 +509,14 @@ describe('ChapterQueries', () => {
|
||||
await insertTestChapter(testDb, novelId, {
|
||||
isDownloaded: true,
|
||||
});
|
||||
const untouchedChapterId = await insertTestChapter(testDb, novelId, {
|
||||
isDownloaded: true,
|
||||
});
|
||||
|
||||
const chapters = await getNovelChapters(novelId);
|
||||
const downloadedChapters = chapters.filter(c => c.isDownloaded);
|
||||
const downloadedChapters = chapters.filter(
|
||||
c => c.isDownloaded && c.id !== untouchedChapterId,
|
||||
);
|
||||
await deleteDownloads(
|
||||
downloadedChapters.map(c => ({
|
||||
id: c.id,
|
||||
@@ -521,7 +526,14 @@ describe('ChapterQueries', () => {
|
||||
);
|
||||
|
||||
const updatedChapters = await getNovelChapters(novelId);
|
||||
expect(updatedChapters.every(c => c.isDownloaded === false)).toBe(true);
|
||||
expect(
|
||||
updatedChapters
|
||||
.filter(c => c.id !== untouchedChapterId)
|
||||
.every(c => c.isDownloaded === false),
|
||||
).toBe(true);
|
||||
expect(
|
||||
updatedChapters.find(c => c.id === untouchedChapterId)?.isDownloaded,
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -729,6 +741,18 @@ describe('ChapterQueries', () => {
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result.every(c => c.isDownloaded === false)).toBe(true);
|
||||
});
|
||||
|
||||
it('returns chapters in reading order', async () => {
|
||||
const testDb = getTestDb();
|
||||
const novelId = await insertTestNovel(testDb, { inLibrary: true });
|
||||
await insertTestChapter(testDb, novelId, { position: 3 });
|
||||
await insertTestChapter(testDb, novelId, { position: 1 });
|
||||
await insertTestChapter(testDb, novelId, { position: 2 });
|
||||
|
||||
const result = await getAllUndownloadedChapters(novelId);
|
||||
|
||||
expect(result.map(chapter => chapter.position)).toEqual([1, 2, 3]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getAllUndownloadedAndUnreadChapters', () => {
|
||||
@@ -754,6 +778,17 @@ describe('ChapterQueries', () => {
|
||||
expect(result[0].isDownloaded).toBe(false);
|
||||
expect(result[0].unread).toBe(true);
|
||||
});
|
||||
|
||||
it('returns unread chapters in reading order', async () => {
|
||||
const testDb = getTestDb();
|
||||
const novelId = await insertTestNovel(testDb, { inLibrary: true });
|
||||
await insertTestChapter(testDb, novelId, { position: 5, unread: true });
|
||||
await insertTestChapter(testDb, novelId, { position: 2, unread: true });
|
||||
|
||||
const result = await getAllUndownloadedAndUnreadChapters(novelId);
|
||||
|
||||
expect(result.map(chapter => chapter.position)).toEqual([2, 5]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getChapter', () => {
|
||||
|
||||
@@ -20,7 +20,10 @@ export default function useDownload() {
|
||||
const downloadQueue = useMemo(
|
||||
() => queue?.filter(t => t.task?.name === 'DOWNLOAD_CHAPTER') || [],
|
||||
[queue],
|
||||
) as { task: DownloadChapterTask; meta: BackgroundTaskMetadata }[];
|
||||
) as (QueuedBackgroundTask & {
|
||||
task: DownloadChapterTask;
|
||||
meta: BackgroundTaskMetadata;
|
||||
})[];
|
||||
|
||||
const downloadingChapterIds = useMemo(
|
||||
() =>
|
||||
|
||||
@@ -12,6 +12,7 @@ import { switchNovelToLibraryQuery } from '@database/queries/NovelQueries';
|
||||
import {
|
||||
BACKGROUND_TASKS_STORE_KEY,
|
||||
BackgroundTask,
|
||||
getDownloadProgressKey,
|
||||
QueuedBackgroundTask,
|
||||
} from '@services/backgroundTasks';
|
||||
import { useMMKVObject } from 'react-native-mmkv';
|
||||
@@ -120,6 +121,30 @@ export const useLibrary = (): UseLibraryReturnType => {
|
||||
const [taskQueue] = useMMKVObject<(BackgroundTask | QueuedBackgroundTask)[]>(
|
||||
BACKGROUND_TASKS_STORE_KEY,
|
||||
);
|
||||
const downloadProgressKey = useMemo(
|
||||
() => getDownloadProgressKey(taskQueue),
|
||||
[taskQueue],
|
||||
);
|
||||
const previousDownloadProgressKeyRef = useRef(downloadProgressKey);
|
||||
const hadDownloadTasksRef = useRef(downloadProgressKey.length > 0);
|
||||
|
||||
useEffect(() => {
|
||||
if (downloadProgressKey.length > 0) {
|
||||
if (
|
||||
hadDownloadTasksRef.current &&
|
||||
previousDownloadProgressKeyRef.current !== downloadProgressKey
|
||||
) {
|
||||
void getLibrary();
|
||||
}
|
||||
hadDownloadTasksRef.current = true;
|
||||
} else if (hadDownloadTasksRef.current) {
|
||||
hadDownloadTasksRef.current = false;
|
||||
void getLibrary();
|
||||
}
|
||||
|
||||
previousDownloadProgressKeyRef.current = downloadProgressKey;
|
||||
}, [downloadProgressKey, getLibrary]);
|
||||
|
||||
const restoreTasksCount = useMemo(
|
||||
() =>
|
||||
taskQueue?.filter(t => {
|
||||
|
||||
@@ -130,10 +130,20 @@ const Downloads = ({ navigation }: DownloadsScreenProps) => {
|
||||
<RemoveDownloadsDialog
|
||||
dialogVisible={visible}
|
||||
hideDialog={hideDialog}
|
||||
onSubmit={() => {
|
||||
deleteDownloads(chapters);
|
||||
setChapters([]);
|
||||
hideDialog();
|
||||
onSubmit={async () => {
|
||||
try {
|
||||
await deleteDownloads(chapters);
|
||||
setChapters([]);
|
||||
} catch (error) {
|
||||
showToast(
|
||||
error instanceof Error
|
||||
? error.message
|
||||
: getString('novelScreen.deleteChapterError'),
|
||||
);
|
||||
await getChapters();
|
||||
} finally {
|
||||
hideDialog();
|
||||
}
|
||||
}}
|
||||
/>
|
||||
</SafeAreaView>
|
||||
|
||||
@@ -4,7 +4,7 @@ import { getString } from '@i18n/translations';
|
||||
interface RemoveDownloadsDialogProps {
|
||||
dialogVisible: boolean;
|
||||
hideDialog: () => void;
|
||||
onSubmit: () => void;
|
||||
onSubmit: () => void | Promise<void>;
|
||||
}
|
||||
|
||||
const RemoveDownloadsDialog = ({
|
||||
|
||||
@@ -34,6 +34,7 @@ import { useSaveNovelCover } from '../hooks/useSaveNovelCover';
|
||||
import { NovelScreenProps } from '@navigators/types';
|
||||
import { useDownloadReconciliation } from '../hooks/useDownloadReconciliation';
|
||||
import NovelFloatingActions from './NovelFloatingActions';
|
||||
import { getDownloadProgressKey } from '@services/backgroundTasks/taskDefinitions';
|
||||
|
||||
type NovelScreenListProps = {
|
||||
headerOpacity: SharedValue<number>;
|
||||
@@ -103,14 +104,29 @@ const NovelScreenList = ({
|
||||
const theme = useTheme();
|
||||
const { top: topInset, bottom: bottomInset } = useSafeAreaInsets();
|
||||
|
||||
const { downloadingChapterIds, downloadingNovelIds, downloadChapter } =
|
||||
useDownload();
|
||||
const {
|
||||
downloadQueue,
|
||||
downloadingChapterIds,
|
||||
downloadingNovelIds,
|
||||
downloadChapter,
|
||||
} = useDownload();
|
||||
|
||||
// Queue removal can mean success, failure, or cancellation. Reconcile once
|
||||
// after this novel's queue settles instead of reloading for every task.
|
||||
// Reconcile as chapters complete and once more when the queue settles so
|
||||
// downloaded state remains accurate after success, failure, or cancellation.
|
||||
const isNovelDownloading =
|
||||
novel.id !== 'NO_ID' && downloadingNovelIds.has(novel.id);
|
||||
useDownloadReconciliation(isNovelDownloading, getChapters);
|
||||
const novelDownloadProgressKey = useMemo(
|
||||
() =>
|
||||
novel.id === 'NO_ID'
|
||||
? ''
|
||||
: getDownloadProgressKey(downloadQueue, novel.id),
|
||||
[downloadQueue, novel.id],
|
||||
);
|
||||
useDownloadReconciliation(
|
||||
isNovelDownloading,
|
||||
novelDownloadProgressKey,
|
||||
getChapters,
|
||||
);
|
||||
|
||||
const [isFabExtended, setIsFabExtended] = useState(true);
|
||||
const [showScrollToTop, setShowScrollToTop] = useState(false);
|
||||
|
||||
@@ -6,6 +6,7 @@ const mockUseNovelActions = jest.fn();
|
||||
const mockDownloadChapter = jest.fn();
|
||||
let mockDownloadingChapterIds = new Set<number>();
|
||||
let mockDownloadingNovelIds = new Set<number>();
|
||||
let mockDownloadQueue: any[] = [];
|
||||
|
||||
jest.mock('../../NovelContext', () => ({
|
||||
useNovelValue: (key: string) => mockUseNovelValue(key),
|
||||
@@ -20,6 +21,7 @@ jest.mock('@hooks/persisted', () => ({
|
||||
refreshNovelMetadata: false,
|
||||
}),
|
||||
useDownload: () => ({
|
||||
downloadQueue: mockDownloadQueue,
|
||||
downloadingChapterIds: mockDownloadingChapterIds,
|
||||
downloadingNovelIds: mockDownloadingNovelIds,
|
||||
downloadChapter: mockDownloadChapter,
|
||||
@@ -283,6 +285,7 @@ describe('NovelScreenList (task 12 context boundary cutover)', () => {
|
||||
jest.clearAllMocks();
|
||||
mockDownloadingChapterIds = new Set<number>();
|
||||
mockDownloadingNovelIds = new Set<number>();
|
||||
mockDownloadQueue = [];
|
||||
});
|
||||
|
||||
it('uses novelStore selector actions', () => {
|
||||
@@ -348,6 +351,51 @@ describe('NovelScreenList (task 12 context boundary cutover)', () => {
|
||||
expect(store.state.updateChapter).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('reconciles chapter state as a novel download makes progress', () => {
|
||||
const store = createStore();
|
||||
mockDownloadingNovelIds = new Set<number>([baseNovel.id]);
|
||||
mockDownloadQueue = [
|
||||
{
|
||||
id: 'download-1',
|
||||
task: {
|
||||
name: 'DOWNLOAD_CHAPTER',
|
||||
data: { novelId: baseNovel.id, chapters: [] },
|
||||
},
|
||||
state: 'running',
|
||||
meta: { progress: 0, isRunning: true },
|
||||
},
|
||||
];
|
||||
wireStoreSelectors(store);
|
||||
|
||||
const view = renderList();
|
||||
expect(store.state.getChapters).not.toHaveBeenCalled();
|
||||
|
||||
mockDownloadQueue = [
|
||||
{
|
||||
...mockDownloadQueue[0],
|
||||
meta: { progress: 0.5, isRunning: true },
|
||||
},
|
||||
];
|
||||
view.rerender(
|
||||
<NovelScreenList
|
||||
headerOpacity={headerOpacity as any}
|
||||
listRef={listRef as any}
|
||||
navigation={navigation}
|
||||
routeBaseNovel={{
|
||||
name: 'Route Novel',
|
||||
path: '/novel/test',
|
||||
pluginId: 'plugin.test',
|
||||
}}
|
||||
selected={[]}
|
||||
setSelected={jest.fn()}
|
||||
onRefresh={onRefresh}
|
||||
updating={false}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(store.state.getChapters).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('uses selector-backed page navigation action from novelStore', () => {
|
||||
const store = createStore();
|
||||
wireStoreSelectors(store);
|
||||
|
||||
@@ -2,19 +2,29 @@ import { useEffect, useRef } from 'react';
|
||||
|
||||
export const useDownloadReconciliation = (
|
||||
isNovelDownloading: boolean,
|
||||
downloadProgressKey: string,
|
||||
reloadChapters: () => Promise<void>,
|
||||
) => {
|
||||
const hadNovelDownloadsRef = useRef(false);
|
||||
const previousProgressKeyRef = useRef('');
|
||||
|
||||
useEffect(() => {
|
||||
if (isNovelDownloading) {
|
||||
if (
|
||||
hadNovelDownloadsRef.current &&
|
||||
previousProgressKeyRef.current !== downloadProgressKey
|
||||
) {
|
||||
void reloadChapters();
|
||||
}
|
||||
hadNovelDownloadsRef.current = true;
|
||||
previousProgressKeyRef.current = downloadProgressKey;
|
||||
return;
|
||||
}
|
||||
|
||||
if (hadNovelDownloadsRef.current) {
|
||||
hadNovelDownloadsRef.current = false;
|
||||
previousProgressKeyRef.current = '';
|
||||
void reloadChapters();
|
||||
}
|
||||
}, [isNovelDownloading, reloadChapters]);
|
||||
}, [downloadProgressKey, isNovelDownloading, reloadChapters]);
|
||||
};
|
||||
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
fromNativeTaskRecord,
|
||||
getBackgroundTaskQueueName,
|
||||
getBackgroundTaskTitle,
|
||||
getDownloadProgressKey,
|
||||
willTaskWaitInQueue,
|
||||
} from '../taskDefinitions';
|
||||
|
||||
@@ -153,4 +154,37 @@ describe('background task definitions', () => {
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it('derives progress keys only for the requested novel downloads', () => {
|
||||
const createDownload = (
|
||||
id: string,
|
||||
novelId: number,
|
||||
progress?: number,
|
||||
) => ({
|
||||
id,
|
||||
task: {
|
||||
name: 'DOWNLOAD_CHAPTER' as const,
|
||||
data: {
|
||||
novelName: `Novel ${novelId}`,
|
||||
novelId,
|
||||
chapters: [{ chapterId: novelId, chapterName: 'Chapter 1' }],
|
||||
},
|
||||
},
|
||||
state: 'running' as const,
|
||||
meta: {
|
||||
name: `Novel ${novelId}`,
|
||||
isRunning: true,
|
||||
progress,
|
||||
progressText: 'Chapter 1',
|
||||
},
|
||||
});
|
||||
const tasks: (ReturnType<typeof createDownload> | BackgroundTask)[] = [
|
||||
{ name: 'UPDATE_LIBRARY' },
|
||||
createDownload('first', 1, 0.5),
|
||||
createDownload('second', 2),
|
||||
];
|
||||
|
||||
expect(getDownloadProgressKey(tasks, 1)).toBe('first:running:0.5');
|
||||
expect(getDownloadProgressKey(tasks, 2)).toBe('second:running:pending');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -9,6 +9,7 @@ export {
|
||||
configureAutomaticBackups,
|
||||
} from './automaticBackupSchedule';
|
||||
export { runHeadlessBackgroundTask } from './headlessTask';
|
||||
export { getDownloadProgressKey } from './taskDefinitions';
|
||||
export type {
|
||||
BackgroundTask,
|
||||
BackgroundTaskEnqueuer,
|
||||
|
||||
@@ -25,6 +25,25 @@ const MAX_CONCURRENT_DOWNLOADS = 3;
|
||||
export const allowsDuplicateTask = (task: BackgroundTask) =>
|
||||
MULTIPLICABLE_TASKS.includes(task.name);
|
||||
|
||||
export const getDownloadProgressKey = (
|
||||
tasks: (QueuedBackgroundTask | BackgroundTask)[] | undefined,
|
||||
novelId?: number,
|
||||
) =>
|
||||
(tasks ?? [])
|
||||
.filter((task): task is QueuedBackgroundTask => 'task' in task)
|
||||
.filter(
|
||||
task =>
|
||||
task.task.name === 'DOWNLOAD_CHAPTER' &&
|
||||
(novelId === undefined || task.task.data.novelId === novelId),
|
||||
)
|
||||
.map(
|
||||
task =>
|
||||
`${task.id}:${task.state ?? 'queued'}:${
|
||||
task.meta.progress ?? 'pending'
|
||||
}`,
|
||||
)
|
||||
.join('|');
|
||||
|
||||
export const getBackgroundTaskQueueName = (task: BackgroundTask) =>
|
||||
task.name === 'DOWNLOAD_CHAPTER'
|
||||
? `${BACKGROUND_TASK_QUEUE_PREFIX}:download:${
|
||||
|
||||
Reference in New Issue
Block a user