diff --git a/README.md b/README.md index 1674064..a419eb5 100644 --- a/README.md +++ b/README.md @@ -159,10 +159,11 @@ After a `1`, the next run fetches each missing original again, fetches the ML data that is not cached, and rebuilds the album links. An album quak could not decrypt, or a file in `failures.json` because quak could not decrypt it, keeps the exit code at `1` until the server sends a copy that decrypts, or deletes it -(see "Backup layout"). An original that a `--verify` run could not read, or put -back with bytes that still do not match, stays at its save path. Only a later -`--verify` run checks it again: a run without `--verify` leaves it as it is and -takes the file out of `failures.json`, so that run can exit 0. +(see "Backup layout"). An original that a `--verify` run could not read, or +could not replace with a download that matches, stays at its save path as it +was. Only a later `--verify` run checks it again: a run without `--verify` +leaves it as it is and takes the file out of `failures.json`, so that run can +exit 0. ## Examples @@ -692,8 +693,8 @@ reads no tag from is recorded, base64, as `exifRaw`, with the reason in and `helper fix-missing-thumbnails` take `--json` for machine-readable output. `backup --verify` also hashes the originals already in the backup and replaces -any that do not match the content hash Ente records; one it cannot replace goes -into `failures.json` (see "Backup layout"). +any that do not match the content hash Ente records; one it cannot replace stays +as it was and goes into `failures.json` (see "Backup layout"). `backup-metadata` fetches ML data in requests of up to 200 files. When a request fails, the error is logged, each of its files is written with the reason in an @@ -878,33 +879,41 @@ original already at its save path the way a download is checked (see "On-disk cache layout" below): its bytes, read in chunks, or a live photo's image and video, joined as `:`. An original that matches the content hash its metadata records is left as it is. One that does not is logged on one -line naming the file, deleted (a live photo's image and video both), and put -back in the same run like a missing one: downloaded, or copied from the cache if -the cache holds it. What is put back is hashed too, because a copy from the -cache is not checked as a download is. If it still does not match, it stays at -its save path and the file goes into `failures.json`, as it does when the -download fails. A file whose metadata records no hash is left as it is and -counted as unchecked. A stored original that cannot be read is left as it is and -counts as failed. The summary and `--json` add the counts `verified`, -`mismatched` and `unchecked`, all of originals that were already stored; one -first downloaded in this run is in none of them. A mismatch that was put back -with matching bytes does not make the exit code non-zero. Without `--verify` -nothing is hashed, the summary is unchanged, and the three counts are 0 in -`--json`. +line naming the file, and replaced once a matching copy is downloaded, in the +same run. The copy is downloaded even when the cache holds the original, because +a copy from the cache is not checked as a download is. It is written to a +temporary file beside the original, a live photo's image and video each to its +own, hashed the same way, and renamed over the original only when it matches; a +live photo's image and video are both hashed before either is renamed. Until +then the original stays as it was: when the download fails, or what it brings +does not match either, the temporary files are deleted, the original keeps its +bytes and its album links, and the file goes into `failures.json`. A run that +stops between renaming a live photo's new image and its new video leaves the new +image beside the old video, which a later `--verify` run replaces if the video +was the part that did not match. A file whose metadata records no hash is left +as it is and counted as unchecked. A stored original that cannot be read is left +as it is and counts as failed. The summary and `--json` add the counts +`verified`, `mismatched` and `unchecked`, all of originals that were already +stored; one first downloaded in this run is in none of them. A mismatch that was +replaced with matching bytes does not make the exit code non-zero. Without +`--verify` nothing is hashed, the summary is unchanged, and the three counts are +0 in `--json`. Each original is written to a temporary file in the same directory, synced to disk, and renamed into place, so an original is either complete or absent, even after a power cut. A downloaded original's temporary file is named `.quak--.tmp`, one copied from the cache -`.quak-backup-YYYY-MM-DD..--.tmp`. A run that is -killed can leave one of these temporary files behind; the next backup deletes -those whose process is no longer running. The content cache uses the same -scheme, and opening a library deletes the temporary files in the cache whose -process is no longer running, so a download another process has in progress in -the same cache is left alone. The rename replaces whatever was at the -destination rather than writing through it: a symlink there is replaced, not -followed, and the new file has the temporary file's permissions, not those of -the file it replaced. +`.quak-backup-YYYY-MM-DD..--.tmp`. A copy that +`--verify` downloads to replace an original is written to such a +`.quak--.tmp` too, a live photo's image and video to +`.quak--.`, each with its own extension. A run that is killed +can leave one of these temporary files behind; the next backup deletes those +whose process is no longer running. The content cache uses the same scheme, and +opening a library deletes the temporary files in the cache whose process is no +longer running, so a download another process has in progress in the same cache +is left alone. The rename replaces whatever was at the destination rather than +writing through it: a symlink there is replaced, not followed, and the new file +has the temporary file's permissions, not those of the file it replaced. ## TODO diff --git a/TODO.md b/TODO.md index c2dbf45..3850249 100644 --- a/TODO.md +++ b/TODO.md @@ -17,8 +17,7 @@ pre-1.0 # Next Step -Three bugs are open: https://git.eeqj.de/sneak/quak/issues/198, -https://git.eeqj.de/sneak/quak/issues/199 and +Two bugs are open: https://git.eeqj.de/sneak/quak/issues/198 and https://git.eeqj.de/sneak/quak/issues/205. The check of quak against a live Ente account, https://git.eeqj.de/sneak/quak/issues/187, needs an account only sneak has, so it waits on him. @@ -28,6 +27,16 @@ declares one. # Completed Steps +- 2026-10-16: With `--verify`, an original whose hash does not match is no + longer removed before its replacement is downloaded (issue 199). + `replaceOriginal` in `src/library/content.ts` downloads the replacement to a + temporary file beside it, a live photo's image and video each to its own, + hashes it and renames it over the original only when it matches. When the + download fails or does not match either, the original keeps its bytes and + links, and the file goes into `failures.json`. The replacement is always + downloaded, through the new `fetchOriginal` of the content cache, never copied + from the cache. + - 2026-10-16: The `~/bin/quak` that `make install` builds retries a refused connection and a name that does not resolve, and the backup records either as transient (issue 206). Bun's `fetch` reports them with codes of its own, diff --git a/src/backup.ts b/src/backup.ts index 1c44dd4..8dc9e4a 100644 --- a/src/backup.ts +++ b/src/backup.ts @@ -41,8 +41,9 @@ // // With `verify`, each original already at its save path is hashed as the // download check hashes it, and one that does not match the content hash its -// metadata records is removed and fetched again in the same run. What is put -// back is hashed too, and recorded as failed if it still does not match. +// metadata records is replaced once a matching copy is downloaded, in the same +// run. Until then it stays as it is: a download that fails, or that does not +// match either, leaves it in place and is recorded as failed. // // Resilience (issue #8): no per-file condition aborts the run. A failed // download, a failed symlink, or ML data missing because the ML data fetch @@ -61,7 +62,6 @@ // code. import { - createReadStream, lstatSync, mkdirSync, readdirSync, @@ -77,12 +77,6 @@ import { readFile } from "node:fs/promises"; import { dirname, extname, join, relative, resolve } from "node:path"; import lockfile from "proper-lockfile"; -import { - chunkHashFinal, - chunkHashInit, - chunkHashUpdate, - init, -} from "./crypto/index.js"; import { removeLeftoverTempFiles } from "./download/index.js"; import { describeError } from "./errors.js"; import { @@ -94,8 +88,10 @@ import { import { copyAtomic, placeOriginal, + replaceOriginal, savePath, storedAtSavePath, + storedHash, } from "./library/content.js"; import { representative } from "./library/records.js"; import type { Undecryptable } from "./library/store.js"; @@ -152,9 +148,9 @@ export interface BackupResult { // Originals already at their save path and left untouched. skipped: number; // With `verify`, the originals already at their save path whose hash - // matched, those whose hash did not (each removed and fetched again), and - // those whose metadata records no hash (left as they are). All three are - // zero without `verify`. + // matched, those whose hash did not (each replaced once a matching copy is + // downloaded), and those whose metadata records no hash (left as they + // are). All three are zero without `verify`. verified: number; mismatched: number; unchecked: number; @@ -192,6 +188,15 @@ export interface BackupLibrary { fileID: number, destination: string, ): Promise<{ path: string; videoPath?: string }>; + // Fetch an original's bytes to `destination` through the content pool, + // a live photo as its image and its video beside it, even when the cache + // or the library's save path already holds it, and return where they + // landed. With `verify`, an original whose hash does not match is replaced + // with what this fetches. + fetchOriginal( + fileID: number, + destination: string, + ): Promise<{ path: string; videoPath?: string }>; thumbnail(fileID: number): Promise<{ path: string }>; // How many originals the backup works on at once: the limit of the // library's content pool. @@ -444,26 +449,6 @@ const saveLedger = (path: string, ledger: Map): void => { ); }; -// The content hash of the original stored at `stored`, computed as the download -// check computes it: over each file's bytes, read in chunks, and for a live -// photo `:`. -const storedHash = async (stored: { - path: string; - videoPath?: string; -}): Promise => { - await init(); - const hashFile = async (path: string): Promise => { - const state = chunkHashInit(); - for await (const chunk of createReadStream(path)) { - chunkHashUpdate(state, chunk as Buffer); - } - return chunkHashFinal(state); - }; - const hash = await hashFile(stored.path); - if (stored.videoPath === undefined) return hash; - return `${hash}:${await hashFile(stored.videoPath)}`; -}; - // A file's EXIF, XMP and dimensions as its JSON holds them: what // `extractImageMetadata` found in its original, or why the original could not // be read. @@ -685,11 +670,13 @@ const runLockedBackup = async ( // through the content cache/pools, as `Photo.download()` does, and fetch // the optional thumbnails; a present file is left as is. With `verify`, a // present original is hashed first, and one that does not match the hash - // its metadata records is removed and fetched like a missing one. One - // that cannot be read is recorded as failed and left where it is. - // `storeOriginal` does this for one original, and records its failure. + // its metadata records is downloaded again and replaced by + // `replaceOriginal`, which leaves it as it was when that download fails + // or does not match either. One that cannot be read is recorded as failed + // and left where it is. `storeOriginal` does this for one original, and + // records its failure. const storeOriginal = async (file: EnteFile): Promise => { - let stored = storedAtSavePath(downloadDirectory, file); + const stored = storedAtSavePath(downloadDirectory, file); let mismatch = false; if (stored !== undefined && verify) { try { @@ -703,11 +690,6 @@ const runLockedBackup = async ( ); mismatched++; mismatch = true; - rmSync(stored.path); - if (stored.videoPath !== undefined) { - rmSync(stored.videoPath); - } - stored = undefined; } } catch (err) { log( @@ -722,29 +704,25 @@ const runLockedBackup = async ( return; } } - if (stored !== undefined) { + if (stored !== undefined && !mismatch) { skipped++; return; } try { log(`Fetching original ${file.metadata.title} (${file.id})...`); - // A fetched original is written straight to its save path (a - // live photo beside it); only one that was already cached - // elsewhere is copied. - const placed = await placeOriginal( - downloadDirectory, - file, - (dest) => lib.original(file.id, dest), - ); - storedThisRun.add(file.id); - // A copy from the cache is not checked as a download is and can - // hold the same bad bytes, so what is put back after a mismatch - // is hashed too. A bad copy stays where it is and the file fails. - if (mismatch && (await storedHash(placed)) !== file.metadata.hash) { - throw new Error( - "the original put back does not match its content hash either", + if (mismatch) { + await replaceOriginal(downloadDirectory, file, (dest) => + lib.fetchOriginal(file.id, dest), + ); + } else { + // A fetched original is written straight to its save path (a + // live photo beside it); only one that was already cached + // elsewhere is copied. + await placeOriginal(downloadDirectory, file, (dest) => + lib.original(file.id, dest), ); } + storedThisRun.add(file.id); downloaded++; } catch (err) { log( diff --git a/src/download/index.ts b/src/download/index.ts index 1e8b30a..59b510a 100644 --- a/src/download/index.ts +++ b/src/download/index.ts @@ -199,11 +199,13 @@ const isRunning = (pid: number): boolean => { }; // Delete the temp files a killed process left in `dir`: the writer's -// `.quak--.tmp` and the backup copy's -// `.quak-backup---.tmp`. Only files whose process is no -// longer running are removed, so another process writing into the same -// directory keeps its own. A reused process ID can only keep a leftover a while -// longer, never remove a live one. +// `.quak--.tmp`, the backup copy's +// `.quak-backup---.tmp`, and the image and video of a live +// photo downloaded to replace a stored one, `.quak--.` (see +// `replaceOriginal`). Only files whose process is no longer running are +// removed, so another process writing into the same directory keeps its own. A +// reused process ID can only keep a leftover a while longer, never remove a +// live one. export const removeLeftoverTempFiles = (dir: string): void => { let names: string[]; try { @@ -212,7 +214,9 @@ export const removeLeftoverTempFiles = (dir: string): void => { return; } for (const name of names) { - const match = /^\.quak-(?:.*-)?(\d+)-[0-9a-z]*\.tmp$/.exec(name); + const match = /^\.quak-(?:.*-)?(\d+)-[0-9a-z]*\.[0-9A-Za-z]+$/.exec( + name, + ); if (match && !isRunning(Number(match[1]))) { rmSync(join(dir, name), { force: true }); } diff --git a/src/library/content.ts b/src/library/content.ts index 383319b..9f2d6e6 100644 --- a/src/library/content.ts +++ b/src/library/content.ts @@ -13,6 +13,8 @@ // originals, the thumbnail pool for thumbnails. The pool limits concurrency, // orders on-demand work ahead of background, and dedups by key so a fileID // requested twice while the first is still in flight downloads once. +// `fetchOriginal` passes no key, so it is not shared with a fetch of the same +// file already in flight. // // Integrity. The reused streaming decrypt is the enforced guarantee: every // chunk is authenticated and the writer renames the file into place only once @@ -27,9 +29,10 @@ // It also names each original's save path under the download directory // (`savePath`), where `Photo.download()` and `lib.backup()` put it. A copy in // the cache does not count as saved there, but is copied there rather than -// fetched again. +// fetched again. An original stored there whose bytes do not match its content +// hash is replaced only by a download that does (`replaceOriginal`). -import { existsSync, readFileSync, statSync } from "node:fs"; +import { createReadStream, existsSync, readFileSync, statSync } from "node:fs"; import { chmod, copyFile, @@ -44,6 +47,12 @@ import { import { basename, dirname, extname, join } from "node:path"; import type { ApiClient } from "../api/client.js"; +import { + chunkHashFinal, + chunkHashInit, + chunkHashUpdate, + init, +} from "../crypto/index.js"; import { downloadFile, downloadThumbnail, @@ -349,6 +358,26 @@ export const storedAtSavePath = ( return storedOriginal(dirname(path), basename(path, extname(path)), file); }; +// The content hash of the original stored at `stored`, computed as the download +// check computes it: over each file's bytes, read in chunks, and for a live +// photo `:`. +export const storedHash = async (stored: { + path: string; + videoPath?: string; +}): Promise => { + await init(); + const hashFile = async (path: string): Promise => { + const state = chunkHashInit(); + for await (const chunk of createReadStream(path)) { + chunkHashUpdate(state, chunk as Buffer); + } + return chunkHashFinal(state); + }; + const hash = await hashFile(stored.path); + if (stored.videoPath === undefined) return hash; + return `${hash}:${await hashFile(stored.videoPath)}`; +}; + // Copy bytes into `dest` via a temp file in the same directory plus rename, so // `dest` appears only once it is whole ("present means complete"). As in the // download writer, the temp file is fsynced before the rename and the directory @@ -405,6 +434,62 @@ export const placeOriginal = async ( return { path, videoPath }; }; +// Replace the original of `file` stored at its save path under `root` with a +// new copy, keeping the stored one until the copy is complete and its hash, +// computed as `storedHash` computes it, is the content hash the file's metadata +// records. `get` is given a temporary file beside the save path and writes the +// copy there, a live photo as its image and its video beside that file, each +// with its own extension. Only a copy that matches is renamed over the stored +// one, and a live photo's image and video are both hashed before either is +// renamed. The image is renamed first: should the run stop, or the video's +// rename fail, before the video is renamed, the new image is left beside the +// old video, and a later backup with `verify` replaces the video if the video +// was the part that did not match. When `get` fails or the copy does not match, +// the temporary files are removed and the stored original stays as it was. +export const replaceOriginal = async ( + root: string, + file: EnteFile, + get: (dest: string) => Promise<{ path: string; videoPath?: string }>, +): Promise<{ path: string; videoPath?: string }> => { + const dest = savePath(root, file); + const dir = dirname(dest); + // Named as the download writer names its temporary files; a live photo's + // image and video get this name with their own extensions. The next + // backup removes any that a killed run leaves (`removeLeftoverTempFiles`). + const tmp = join( + dir, + `.quak-${process.pid}-${Math.random().toString(36).slice(2)}.tmp`, + ); + let got: { path: string; videoPath?: string } | undefined; + try { + got = await get(tmp); + if ((await storedHash(got)) !== file.metadata.hash) { + throw new Error( + "the downloaded original does not match its content hash either", + ); + } + if (got.videoPath === undefined) { + await rename(got.path, dest); + await fsyncPath(dir); + return { path: dest }; + } + const path = withExtension(dest, extname(got.path)); + const videoPath = withExtension(dest, extname(got.videoPath)); + await rename(got.path, path); + await rename(got.videoPath, videoPath); + const name = basename(dest, extname(dest)); + await writeLivePhotoJSON(dir, name, { path, videoPath }); + return { path, videoPath }; + } finally { + // Whatever was not renamed into place. A failure to remove it must + // not replace the error that explains what went wrong. + for (const path of [tmp, got?.path, got?.videoPath]) { + if (path === undefined) continue; + await rm(path, { force: true }).catch(() => undefined); + } + } +}; + export class ContentCache implements PhotoContent, ThumbnailsAPI { private readonly pools: RequestPools; private readonly source: ContentSource; @@ -543,6 +628,35 @@ export class ContentCache implements PhotoContent, ThumbnailsAPI { }; } + // Fetch an original through the content pool to `destination`, a live + // photo as its image and its video beside it, and return where it landed. + // Unlike `backupOriginal`, it fetches even when the cache or the save path + // already holds the original, and it records nothing: what it writes is + // the caller's. It is not shared with a fetch of the same file already + // under way, which writes somewhere else. + async fetchOriginal( + fileID: number, + destination: string, + ): Promise<{ path: string; videoPath?: string }> { + const file = this.getFile(fileID); + if (!file) throw new Error(`content cache: unknown file ${fileID}`); + return this.pools.content.run( + async () => { + const fetched = await this.fetchInto( + file, + destination, + "original", + undefined, + ); + for (const path of [fetched.path, fetched.videoPath]) { + if (path !== undefined) await chmod(path, FILE_MODE); + } + return fetched; + }, + { priority: "on-demand" }, + ); + } + async ensure(args: EnsureOptions): Promise { return this.ensureThumbnails(args); } diff --git a/src/library/index.ts b/src/library/index.ts index 95d8e1d..61fa734 100644 --- a/src/library/index.ts +++ b/src/library/index.ts @@ -606,6 +606,8 @@ export class Library { undecryptable: () => this.store.listUndecryptable(), original: (fileID, destination) => cache!.backupOriginal(fileID, destination), + fetchOriginal: (fileID, destination) => + cache!.fetchOriginal(fileID, destination), thumbnail: (fileID) => cache!.thumbnail(fileID), contentConcurrency: this.pools.content.concurrency, fetchMLData: () => this.fetchMLDataNow(), diff --git a/test/cli/backup.test.ts b/test/cli/backup.test.ts index 09bedc6..7cbb402 100644 --- a/test/cli/backup.test.ts +++ b/test/cli/backup.test.ts @@ -191,9 +191,12 @@ class HostileClient extends MockClient { } // A content source that writes byte buffers of the expected length and can be -// told to fail one fileID's original, to exercise per-file resilience. +// told to fail one fileID's original, as a server error or as a 404, or to +// write other bytes for it, to exercise per-file resilience. interface StubSource extends ContentSource { failID?: number; + notFoundID?: number; + otherBytesID?: number; failThumbID?: number; originalCalls: number; } @@ -204,8 +207,10 @@ const stubSource = (): StubSource => { original: async ({ file: f, destination }) => { s.originalCalls++; if (s.failID === f.id) throw new Error("HTTP 500 from server"); + if (s.notFoundID === f.id) throw new Error("HTTP 404 from server"); const size = SIZE_BY_ID[f.id] ?? 10; - writeFileSync(destination, Buffer.alloc(size)); + const fill = s.otherBytesID === f.id ? 2 : 0; + writeFileSync(destination, Buffer.alloc(size, fill)); return { bytesWritten: size }; }, thumbnail: async ({ file: f, destination }) => { @@ -696,17 +701,26 @@ describe("lib.backup", () => { // A child that has already exited: its process ID is not running. const exitedPID = spawnSync(process.execPath, ["-e", ""]).pid; const leftover = `.quak-backup-2026-03-01.100.jpg-${exitedPID}-abc123.tmp`; + // A live photo's image and video downloaded to replace a stored one, + // before they were renamed into place. + const replacement = [ + `.quak-${exitedPID}-abc789.heic`, + `.quak-${exitedPID}-abc789.mov`, + ]; // This test's own process stands in for a backup running at the same // time. const inProgress = `.quak-backup-2026-03-01.101.jpg-${process.pid}-def456.tmp`; - writeFileSync(join(day, leftover), "partial"); - writeFileSync(join(day, inProgress), "partial"); + for (const name of [leftover, ...replacement, inProgress]) { + writeFileSync(join(day, name), "partial"); + } const lib = await openLibrary(stubSource()); await lib.backup({ downloadDirectory: outDir }); const names = readdirSync(day); expect(names).not.toContain(leftover); + expect(names).not.toContain(replacement[0]); + expect(names).not.toContain(replacement[1]); expect(names).toContain(inProgress); lib.close(); }); @@ -1294,7 +1308,8 @@ describe("image metadata in each file's JSON", () => { }); // With `verify`, each original already stored is hashed, and one that does not -// match the content hash its metadata records is downloaded again. +// match the content hash its metadata records is downloaded again, and replaced +// only by a download that matches. describe("backup with verify", () => { // MockClient's files, each recording the hash of what `stubSource` writes // for it, except diagram.png (200), which records none. @@ -1337,6 +1352,12 @@ describe("backup with verify", () => { const good = Buffer.alloc(SIZE_BY_ID[100]!); const corrupt = Buffer.alloc(SIZE_BY_ID[100]!, 1); + // The temporary files left in the date folder of the backup in `outDir`. + const tempFiles = (outDir: string): string[] => + readdirSync(join(outDir, DAY)).filter((name) => + name.startsWith(".quak-"), + ); + it("leaves an original that matches its hash, and one with no hash, as they are", async () => { const { lib, source, outDir } = await backedUp(); const calls = source.originalCalls; @@ -1381,13 +1402,16 @@ describe("backup with verify", () => { expect(log.filter((msg) => msg.startsWith("MISMATCH"))).toEqual([ "MISMATCH original beach.jpg (100): its bytes do not match its content hash", ]); + expect(tempFiles(outDir)).toEqual([]); await lib.close(); }); - it("records a failed download in failures.json, with the original removed", async () => { + // A copy whose hash does not match is still mostly the photo, so the + // backup keeps it until it holds a better one. + it("keeps an original that does not match its hash when its download answers 404", async () => { const { lib, source, outDir } = await backedUp(); writeFileSync(saved(outDir, "100.jpg"), corrupt); - source.failID = 100; + source.notFoundID = 100; const result = await lib.backup({ downloadDirectory: outDir, @@ -1400,18 +1424,18 @@ describe("backup with verify", () => { failed: 1, }); expect(Object.keys(readLedger(outDir).files)).toEqual(["100"]); - expect(existsSync(saved(outDir, "100.jpg"))).toBe(false); + expect(readFileSync(saved(outDir, "100.jpg"))).toEqual(corrupt); + expect( + readlinkSync(join(outDir, "collections", "Vacation", "beach.jpg")), + ).toBe(linkTo("100.jpg")); + expect(tempFiles(outDir)).toEqual([]); await lib.close(); }); - it("records in failures.json an original the cache puts back with the same bad bytes", async () => { - const lib = await openLibrary(stubSource(), new HashedClient()); - const outDir = join(root, "backup"); - // The cache holds a bad copy, and a backup copies an original the - // cache holds to its save path. - const cached = await lib.photos.byID({ fileID: 100 })!.original(); - writeFileSync(cached.path, corrupt); - await lib.backup({ downloadDirectory: outDir }); + it("keeps an original that does not match its hash when its download does not match either", async () => { + const { lib, source, outDir } = await backedUp(); + writeFileSync(saved(outDir, "100.jpg"), corrupt); + source.otherBytesID = 100; const result = await lib.backup({ downloadDirectory: outDir, @@ -1424,10 +1448,39 @@ describe("backup with verify", () => { failed: 1, }); expect(result.errors.map((e) => e.error)).toEqual([ - "the original put back does not match its content hash either", + "the downloaded original does not match its content hash either", ]); expect(Object.keys(readLedger(outDir).files)).toEqual(["100"]); expect(readFileSync(saved(outDir, "100.jpg"))).toEqual(corrupt); + expect(tempFiles(outDir)).toEqual([]); + await lib.close(); + }); + + // The cache can hold the same bad bytes, as a copy from it is not checked + // as a download is, so the replacement is always downloaded. + it("downloads the replacement even when the cache holds the original", async () => { + const source = stubSource(); + const lib = await openLibrary(source, new HashedClient()); + const outDir = join(root, "backup"); + // The cache holds a bad copy, and a backup copies an original the + // cache holds to its save path. + const cached = await lib.photos.byID({ fileID: 100 })!.original(); + writeFileSync(cached.path, corrupt); + await lib.backup({ downloadDirectory: outDir }); + const calls = source.originalCalls; + + const result = await lib.backup({ + downloadDirectory: outDir, + verify: true, + }); + + expect(result).toMatchObject({ + downloaded: 1, + mismatched: 1, + failed: 0, + }); + expect(source.originalCalls).toBe(calls + 1); + expect(readFileSync(saved(outDir, "100.jpg"))).toEqual(good); await lib.close(); }); @@ -1511,6 +1564,9 @@ describe("backup album folders", () => { writeFileSync(path, `original ${fileID}`); return { path }; }, + fetchOriginal: async () => { + throw new Error("no verify in this stand-in"); + }, thumbnail: async () => { throw new Error("no thumbnails in this stand-in"); }, @@ -2244,11 +2300,12 @@ describe("backup of live photos", () => { Buffer.from(IMAGE), ); expect(readFileSync(video)).toEqual(Buffer.from(VIDEO)); + expect(readdirSync(join(outDir, DAY)).sort()).toEqual(stored); expect(tree(outDir)).toEqual(linked); await lib.close(); }); - it("removes both files of a live photo that does not match its hash when downloading it again fails", async () => { + it("keeps both files of a live photo that does not match its hash when downloading it again fails", async () => { const { file: live, body } = await asLivePhoto( file(500, 10, "IMG_0500.HEIC"), ); @@ -2266,8 +2323,14 @@ describe("backup of live photos", () => { expect(result).toMatchObject({ mismatched: 1, failed: 1 }); expect(Object.keys(readLedger(outDir).files)).toEqual(["500"]); - expect(existsSync(saved(outDir, "500.heic"))).toBe(false); - expect(existsSync(saved(outDir, "500.mov"))).toBe(false); + expect(readFileSync(saved(outDir, "500.heic"))).toEqual( + Buffer.from(IMAGE), + ); + expect(readFileSync(saved(outDir, "500.mov"), "utf-8")).toBe( + "another few seconds of video", + ); + expect(readdirSync(join(outDir, DAY)).sort()).toEqual(stored); + expect(tree(outDir)).toEqual(linked); await lib.close(); });