From d16b88986faef73291b5d2108e136a1bb5742b49 Mon Sep 17 00:00:00 2001 From: Charlie Carson Date: Tue, 20 Oct 2026 16:05:04 -0500 Subject: [PATCH] Plan downloads: write to a .part file, share in-flight downloads, and re-fetch slides that fail to load --- src/helpers/CachedData.ts | 36 +++++++++++++++++----- src/helpers/__tests__/CachedData.test.ts | 38 ++++++++++++++++++++++-- src/screens/PlanDownloadScreen.tsx | 7 ++++- src/screens/PlayerScreen.tsx | 8 +++++ test/mocks/react-native-fs.js | 1 + 5 files changed, 79 insertions(+), 11 deletions(-) diff --git a/src/helpers/CachedData.ts b/src/helpers/CachedData.ts index c535fe0..4143330 100644 --- a/src/helpers/CachedData.ts +++ b/src/helpers/CachedData.ts @@ -222,16 +222,38 @@ export class CachedData { return fullPath; } - static async load(file: MessageFileInterface, fileProgressCallback?: (progress: number) => void) { - if (!file.url) return; - let fullPath = this.getFilePath(file.url, file.fileType); - fullPath = decodeURIComponent(fullPath); - const exists = await RNFS.exists(fullPath); - if (!exists) { - await this.download(file, fullPath, fileProgressCallback); + private static inFlight = new Map>(); + + // A second request for a file that is still downloading waits for that download instead of treating the partial file as cached. + static load(file: MessageFileInterface, fileProgressCallback?: (progress: number) => void): Promise { + if (!file.url) return Promise.resolve(); + const fullPath = decodeURIComponent(this.getFilePath(file.url, file.fileType)); + const pending = this.inFlight.get(fullPath); + if (pending) return pending; + const job = this.loadOnce(file, fullPath, fileProgressCallback).finally(() => this.inFlight.delete(fullPath)); + this.inFlight.set(fullPath, job); + return job; + } + + // Download to a .part file and move it into place only when complete, so an interrupted download is never mistaken for a cached file. + private static async loadOnce(file: MessageFileInterface, fullPath: string, fileProgressCallback?: (progress: number) => void) { + if (await RNFS.exists(fullPath)) return; + const part = fullPath + ".part"; + try { + await this.download(file, part, fileProgressCallback); + await RNFS.moveFile(part, fullPath); + } catch (e) { + await RNFS.unlink(part).catch(() => {}); + throw e; } } + // A cached file that will not display: delete it so the next download fetches it again. + static async discard(file: MessageFileInterface) { + if (!file?.url) return; + await RNFS.unlink(decodeURIComponent(this.getFilePath(file.url, file.fileType))).catch(() => {}); + } + static async download( file: MessageFileInterface, diskPath: string, diff --git a/src/helpers/__tests__/CachedData.test.ts b/src/helpers/__tests__/CachedData.test.ts index ad9a1ce..efe95fb 100644 --- a/src/helpers/__tests__/CachedData.test.ts +++ b/src/helpers/__tests__/CachedData.test.ts @@ -48,7 +48,7 @@ describe("CachedData FreeShow images", () => { it("writes the data URL payload to disk instead of downloading the JSON", async () => { global.fetch = jest.fn(async () => ({ ok: true, status: 200, json: async () => ({ action: "get_thumbnail", data: "data:image/png;base64,QUJD" }) })) as unknown as typeof fetch; await CachedData.load({ url: url("C:\\Media\\a.png") } as any); - expect(RNFS.writeFile).toHaveBeenCalledWith(expect.stringMatching(/\.png$/), "QUJD", "base64"); + expect(RNFS.writeFile).toHaveBeenCalledWith(expect.stringMatching(/\.png\.part$/), "QUJD", "base64"); expect(RNFS.downloadFile).not.toHaveBeenCalled(); }); }); @@ -129,7 +129,7 @@ describe("CachedData.prefetch", () => { expect(RNFS.downloadFile).toHaveBeenCalledWith(expect.objectContaining({ fromUrl: "https://www.googleapis.com/drive/v3/files/vid1?alt=media&supportsAllDrives=true", headers: { Authorization: "Bearer tok" }, - toFile: "/cache/drive/vid1.mp4" + toFile: "/cache/drive/vid1.mp4.part" })); }); @@ -154,7 +154,7 @@ describe("CachedData.prefetch", () => { expect(RNFS.downloadFile).toHaveBeenCalledWith(expect.objectContaining({ fromUrl: "https://dl.example/abc", headers: undefined, - toFile: "/cache/onedrive/A!1.mp4" + toFile: "/cache/onedrive/A!1.mp4.part" })); } finally { global.fetch = realFetch; @@ -181,3 +181,35 @@ describe("CachedData.allFilesCached", () => { expect(await CachedData.allFilesCached([{ url: "https://a.com/x/y/f.mp4" }] as any)).toBe(false); }); }); + +describe("CachedData.load partial downloads (#1311)", () => { + beforeEach(() => jest.clearAllMocks()); + + it("downloads to a .part file and only moves it into place after a 200", async () => { + await CachedData.load({ url: "https://a.com/x/y/slide.jpg" } as any); + expect(RNFS.downloadFile).toHaveBeenCalledWith(expect.objectContaining({ toFile: "/cache/x/y/slide.jpg.part" })); + expect(RNFS.moveFile).toHaveBeenCalledWith("/cache/x/y/slide.jpg.part", "/cache/x/y/slide.jpg"); + }); + + it("leaves no file at the final path when the download fails", async () => { + (RNFS.downloadFile as jest.Mock).mockReturnValueOnce({ promise: Promise.resolve({ statusCode: 500 }) }); + await expect(CachedData.load({ url: "https://a.com/x/y/slide.jpg" } as any)).rejects.toThrow(); + expect(RNFS.moveFile).not.toHaveBeenCalled(); + expect(RNFS.unlink).toHaveBeenCalledWith("/cache/x/y/slide.jpg.part"); + }); + + it("a second load of a file still downloading waits for the first instead of starting another", async () => { + let finish: (v: { statusCode: number }) => void = () => {}; + (RNFS.downloadFile as jest.Mock).mockReturnValueOnce({ promise: new Promise(r => { finish = r; }) }); + const file = { url: "https://a.com/x/y/title.jpg" } as any; + const first = CachedData.load(file); + await new Promise(r => setImmediate(r)); + let secondDone = false; + const second = CachedData.load(file).then(() => { secondDone = true; }); + await new Promise(r => setImmediate(r)); + expect(secondDone).toBe(false); + finish({ statusCode: 200 }); + await Promise.all([first, second]); + expect(RNFS.downloadFile).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/screens/PlanDownloadScreen.tsx b/src/screens/PlanDownloadScreen.tsx index a5212cc..5604c9e 100644 --- a/src/screens/PlanDownloadScreen.tsx +++ b/src/screens/PlanDownloadScreen.tsx @@ -1,4 +1,4 @@ -import React, { useEffect } from "react"; +import React, { useEffect, useRef } from "react"; import { View, Text, TouchableHighlight, ActivityIndicator, BackHandler, ImageBackground } from "react-native"; import { useTranslation } from "react-i18next"; import Icon from "react-native-vector-icons/MaterialIcons"; @@ -136,8 +136,12 @@ export const PlanDownloadScreen = (props: Props) => { } }; + // The stored plan and the freshly fetched one can both start a download; only the latest one may mark the plan ready. + const downloadRun = useRef(0); + const startDownload = async () => { if (!plan) return; + const run = ++downloadRun.current; const files = plan.files || []; if (files.length === 0) { CachedData.messageFiles = []; @@ -149,6 +153,7 @@ export const PlanDownloadScreen = (props: Props) => { setReady(false); await StorageManager.ensureFreeSpace([downloadKey]); CachedData.prefetch(files, updateCounts).then(() => { + if (run !== downloadRun.current) return; setReady(true); DownloadIndex.addEntry({ downloadKey, diff --git a/src/screens/PlayerScreen.tsx b/src/screens/PlayerScreen.tsx index 659e7be..e2e3ecf 100644 --- a/src/screens/PlayerScreen.tsx +++ b/src/screens/PlayerScreen.tsx @@ -179,6 +179,13 @@ export const PlayerScreen = (props: Props) => { } }; + // An image that will not decode is likely a damaged download; delete it so the next plan download fetches it again. + const handleMediaError = () => { + const file = CachedData.messageFiles?.[messageIndex]; + if (!props.streaming && file && !PlayerHelper.isVideoFile(file)) CachedData.discard(file); + handleVideoEnd(); + }; + const handleMessageSelect = (index: number) => { if (paused) setPaused(false); feedbackAnim.setValue(0); @@ -251,6 +258,7 @@ export const PlayerScreen = (props: Props) => { paused={paused} onProgress={handleProgress} onEnd={handleVideoEnd} + onError={handleMediaError} /> diff --git a/test/mocks/react-native-fs.js b/test/mocks/react-native-fs.js index 8a674e2..5ffcbd5 100644 --- a/test/mocks/react-native-fs.js +++ b/test/mocks/react-native-fs.js @@ -4,6 +4,7 @@ module.exports = { mkdir: jest.fn(async () => {}), stat: jest.fn(async () => ({ size: 0 })), unlink: jest.fn(async () => {}), + moveFile: jest.fn(async () => {}), getFSInfo: jest.fn(async () => ({ freeSpace: Number.MAX_SAFE_INTEGER })), writeFile: jest.fn(async () => {}), downloadFile: jest.fn(() => ({ promise: Promise.resolve({ statusCode: 200 }) }))