From e2d54450e873beab13a5ffbfcb34c33137d15721 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 1 Oct 2026 18:45:44 +0000 Subject: [PATCH] Keep a Photo's save path after its file leaves; refuse an empty download directory A Photo now keeps the file its record was made from (the record projection holds one membership of each file), so savePath and isLocal still answer after a refresh removes the file. Library.open rejects an empty downloadDirectory. A backup clears leftover temp files in every date folder under its directory, not only in those of the files in its scope. placeOriginal no longer deletes what is at the save path before copying a live photo, and the tests that depended on that are removed. The TODO.md entry for issue 143 describes only the current layout. Model: opus-5-5 --- TODO.md | 2 +- src/backup.ts | 28 +++++++---- src/library/content.ts | 7 +-- src/library/index.ts | 23 ++++----- src/library/read.ts | 26 ++++++---- src/library/records.ts | 8 +++- test/cli/backup.test.ts | 72 ++++++++-------------------- test/library/content-library.test.ts | 28 +++++++++++ 8 files changed, 103 insertions(+), 91 deletions(-) diff --git a/TODO.md b/TODO.md index cbb07c1..7195963 100644 --- a/TODO.md +++ b/TODO.md @@ -34,7 +34,7 @@ declares one. `photo.download()` puts the original there, copied from the cache when the cache holds it and fetched otherwise. `lib.backup()` does the same for each file, writes each file's JSON beside its original, and links `collections/` to - the save paths; the backup has no `originals/` folder. + the save paths. - 2026-10-01: A `Photo` has `savePath`, `isLocal`, `content()`, `exif()`, `modifiedAt`, `hash` and `year` (issue 141). `savePath` is where diff --git a/src/backup.ts b/src/backup.ts index 8ebe24f..75b4336 100644 --- a/src/backup.ts +++ b/src/backup.ts @@ -240,6 +240,23 @@ const linksFor = ( })); }; +// Every date folder (`YYYY/YYYY-MM/YYYY-MM-DD/`) under `root`, whether or not a +// file in this backup is saved there. A folder that cannot be read is skipped. +const dateFolders = (root: string): string[] => { + const subfolders = (dir: string, name: RegExp): string[] => { + try { + return readdirSync(dir, { withFileTypes: true }) + .filter((e) => e.isDirectory() && name.test(e.name)) + .map((e) => join(dir, e.name)); + } catch { + return []; + } + }; + return subfolders(root, /^\d{4}$/) + .flatMap((year) => subfolders(year, /^\d{4}-\d\d$/)) + .flatMap((month) => subfolders(month, /^\d{4}-\d\d-\d\d$/)); +}; + // Whether the entry at `path` is a symlink a backup to `root` made: one to an // original in a `YYYY/YYYY-MM/YYYY-MM-DD/` folder of `root`. const linksToOriginal = (path: string, root: string): boolean => { @@ -358,6 +375,9 @@ export const runBackup = async ( mkdirSync(collectionsDir, { recursive: true }); if (includeThumbnails) mkdirSync(thumbnailsDir, { recursive: true }); removeLeftoverTempFiles(thumbnailsDir); + for (const dir of dateFolders(downloadDirectory)) { + removeLeftoverTempFiles(dir); + } const ledgerPath = join(downloadDirectory, "failures.json"); const ledger = loadLedger(ledgerPath); @@ -417,14 +437,6 @@ export const runBackup = async ( // through the content cache/pools, as `Photo.download()` does, and fetch // the optional thumbnails; a present file is left as is. if (includeOriginals) { - // A killed run may have left temp files in the save path folders. - const saveDirs = new Set( - [...distinct.values()].map((f) => - dirname(savePath(downloadDirectory, f)), - ), - ); - for (const dir of saveDirs) removeLeftoverTempFiles(dir); - for (const [fileID, file] of distinct) { if (storedAtSavePath(downloadDirectory, file) !== undefined) { skipped++; diff --git a/src/library/content.ts b/src/library/content.ts index 530fbe2..e8436d7 100644 --- a/src/library/content.ts +++ b/src/library/content.ts @@ -405,10 +405,8 @@ export const copyAtomic = async (src: string, dest: string): Promise => { // folders. `get` is given the save path and returns where the original is: a // fetch writes it there, and a copy the cache holds is copied there. A live // photo's image and video go beside the save path, each with its own -// extension: when they came from the cache they are copied, after removing -// whatever was at the save path (an earlier version's ZIP of the two). Then -// the JSON file naming them is written, which is what makes the live photo -// count as stored. +// extension: when they came from the cache they are copied. Then the JSON file +// naming them is written, which is what makes the live photo count as stored. export const placeOriginal = async ( root: string, file: EnteFile, @@ -424,7 +422,6 @@ export const placeOriginal = async ( const path = withExtension(dest, extname(got.path)); const videoPath = withExtension(dest, extname(got.videoPath)); if (got.path !== path) { - await rm(dest, { force: true }); await copyAtomic(got.path, path); await copyAtomic(got.videoPath, videoPath); } diff --git a/src/library/index.ts b/src/library/index.ts index 2d54c23..fecfabe 100644 --- a/src/library/index.ts +++ b/src/library/index.ts @@ -328,20 +328,9 @@ export class Library { const derive = (): DerivedRecords => this.deriveNow(); const root = this.downloadDirectory; const saves: SavePathLookup = { - savePath: (fileID) => { - const file = this.store.getFileByID(fileID); - if (!file) throw new Error(`library: unknown file ${fileID}`); - return ( - storedAtSavePath(root, file)?.path ?? savePath(root, file) - ); - }, - isLocal: (fileID) => { - const file = this.store.getFileByID(fileID); - return ( - file !== undefined && - storedAtSavePath(root, file) !== undefined - ); - }, + savePath: (file) => + storedAtSavePath(root, file)?.path ?? savePath(root, file), + isLocal: (file) => storedAtSavePath(root, file) !== undefined, }; this.albums = makeAlbumsAPI(derive, saves, this.cache); this.photos = makePhotosAPI(derive, saves, this.cache); @@ -373,6 +362,12 @@ export class Library { const { userID } = opts.client.whoami(); const cacheDirectory = opts.cacheDirectory ?? defaultCacheDirectory(userID); + if (opts.downloadDirectory === "") { + throw new Error( + "library: downloadDirectory is empty (leave it out to save " + + "under photos/ in the working directory)", + ); + } const downloadDirectory = opts.downloadDirectory ?? resolve("photos"); const metadataPath = join(cacheDirectory, "metadata.json"); let store = await MetadataStore.load(metadataPath); diff --git a/src/library/read.ts b/src/library/read.ts index 8db4929..0e54468 100644 --- a/src/library/read.ts +++ b/src/library/read.ts @@ -21,15 +21,15 @@ import { readFile } from "node:fs/promises"; import { readPhotoExif, type PhotoExif } from "../exif.js"; -import type { CollectionType, FileType } from "../model/types.js"; +import type { CollectionType, EnteFile, FileType } from "../model/types.js"; import type { ContentOptions, ContentResult, PhotoContent } from "./content.js"; import type { AlbumRecord, PhotoRecord, DerivedRecords } from "./records.js"; // Where a photo's original is saved, and whether all of it is there. The // library answers both from the disk, with or without a content cache. export interface SavePathLookup { - savePath(fileID: number): string; - isLocal(fileID: number): boolean; + savePath(file: EnteFile): string; + isLocal(file: EnteFile): boolean; } // Newest first, with fileID as a stable tiebreak so equal-timed files order @@ -43,10 +43,13 @@ const byNewestAlbum = (a: AlbumRecord, b: AlbumRecord): number => b.updationTime - a.updationTime || b.collectionID - a.collectionID; // A single photo. Field access mirrors `PhotoRecord`; `record()` returns the -// underlying plain record for callers that need the IPC-safe value. +// underlying plain record for callers that need the IPC-safe value. `file` is +// the file the record was made from, so the save path stays known after a +// refresh removes the file from the library. export class Photo { constructor( private readonly rec: PhotoRecord, + private readonly file: EnteFile, private readonly saves: SavePathLookup, private readonly cache?: PhotoContent, ) {} @@ -105,13 +108,13 @@ export class Photo { // the title's extension, and the image may be stored under a different // one. get savePath(): string { - return this.saves.savePath(this.rec.fileID); + return this.saves.savePath(this.file); } // Whether the whole original is at `savePath`. A copy only in the cache // does not count. get isLocal(): boolean { - return this.saves.isLocal(this.rec.fileID); + return this.saves.isLocal(this.file); } record(): PhotoRecord { @@ -207,7 +210,10 @@ export class Album { const out: Photo[] = []; for (const id of this.rec.fileIDs) { const p = this.records.photos.get(id); - if (p) out.push(new Photo(p, this.saves, this.content)); + const file = this.records.files.get(id); + if (p && file) { + out.push(new Photo(p, file, this.saves, this.content)); + } } return out; } @@ -301,8 +307,10 @@ export const makePhotosAPI = ( content?: PhotoContent, ): PhotosAPI => ({ byID: ({ fileID }): Photo | undefined => { - const rec = derive().photos.get(fileID); - return rec ? new Photo(rec, saves, content) : undefined; + const records = derive(); + const rec = records.photos.get(fileID); + const file = records.files.get(fileID); + return rec && file ? new Photo(rec, file, saves, content) : undefined; }, records: ({ fileIDs }): PhotoRecord[] => { const { photos } = derive(); diff --git a/src/library/records.ts b/src/library/records.ts index 0fc7988..0d0be30 100644 --- a/src/library/records.ts +++ b/src/library/records.ts @@ -86,6 +86,10 @@ export interface LibraryChange { export interface DerivedRecords { albums: Map; photos: Map; + // One membership of each photo's file, for its `Photo`'s save path. It + // holds the file's key, so it stays in this process: no snapshot or change + // carries it. + files: Map; } const asString = (v: unknown): string | undefined => @@ -205,6 +209,7 @@ export const deriveRecords = ( } const photos = new Map(); + const photoFiles = new Map(); const takenAtByFile = new Map(); for (const [fileID, memberships] of byFileID) { const record = toPhotoRecord(fileID, memberships); @@ -216,6 +221,7 @@ export const deriveRecords = ( record.thumbnailPath = paths.thumbnailPath; } photos.set(fileID, record); + photoFiles.set(fileID, memberships[0]!); takenAtByFile.set(fileID, record.takenAt); } @@ -224,7 +230,7 @@ export const deriveRecords = ( albums.set(c.id, toAlbumRecord(c, files, takenAtByFile)); } - return { albums, photos }; + return { albums, photos, files: photoFiles }; }; // Sorted, GUI-ready arrays: albums newest updated first, photos newest first. diff --git a/test/cli/backup.test.ts b/test/cli/backup.test.ts index ca8a3d6..6135af2 100644 --- a/test/cli/backup.test.ts +++ b/test/cli/backup.test.ts @@ -651,6 +651,22 @@ describe("lib.backup", () => { lib.close(); }); + it("removes temp files left in a date folder no file in the backup is saved in", async () => { + const outDir = join(root, "backup"); + // As for a file since deleted, or given another date, after a run was + // killed while writing it. + const otherDay = join(outDir, "2025", "2025-01", "2025-01-02"); + mkdirSync(otherDay, { recursive: true }); + const exitedPID = spawnSync(process.execPath, ["-e", ""]).pid; + writeFileSync(join(otherDay, `.quak-${exitedPID}-abc123.tmp`), "x"); + const lib = await openLibrary(stubSource()); + + await lib.backup({ downloadDirectory: outDir }); + + expect(readdirSync(otherDay)).toEqual([]); + lib.close(); + }); + it("removes leftover temp files in thumbnails/ but not those of a backup still running", async () => { const outDir = join(root, "backup"); const thumbnails = join(outDir, "thumbnails"); @@ -1098,18 +1114,6 @@ describe("backup of live photos", () => { const open = (files: EnteFile[], bodies: Map) => openLibrary(cdnSource(bodies), new TripClient(files)); - // What an earlier version stored for live photo 500: the ZIP under the - // image's name, and its link. - const earlierZIP = (outDir: string): void => { - mkdirSync(join(outDir, DAY), { recursive: true }); - mkdirSync(join(outDir, "collections", "Trip"), { recursive: true }); - writeFileSync(saved(outDir, "500.HEIC"), livePhotoZip()); - symlinkSync( - linkTo("500.HEIC"), - join(outDir, "collections", "Trip", "IMG_0500.HEIC"), - ); - }; - const stored = [ "2026-03-01.500.heic", "2026-03-01.500.json", @@ -1173,30 +1177,13 @@ describe("backup of live photos", () => { await lib.close(); }); - it("replaces the ZIP an earlier version stored, and its link", async () => { - const { file: live, body } = await asLivePhoto( - file(500, 10, "IMG_0500.HEIC"), - ); - const outDir = join(root, "backup"); - earlierZIP(outDir); - const lib = await open([live], new Map([[500, body]])); - - const result = await lib.backup({ downloadDirectory: outDir }); - - expect(result).toMatchObject({ downloaded: 1, failed: 0 }); - expect(readdirSync(join(outDir, DAY)).sort()).toEqual(stored); - expect(tree(outDir)).toEqual(linked); - await lib.close(); - }); - - it("stores nothing for a live photo that fails its hash, and keeps what was there", async () => { + it("stores nothing for a live photo that fails its hash", async () => { const { file: live, body } = await asLivePhoto( file(500, 10, "IMG_0500.HEIC"), livePhotoZip(), "not:the recorded hash", ); const outDir = join(root, "backup"); - earlierZIP(outDir); const lib = await open([live], new Map([[500, body]])); const result = await lib.backup({ downloadDirectory: outDir }); @@ -1204,12 +1191,8 @@ describe("backup of live photos", () => { expect(result).toMatchObject({ downloaded: 0, failed: 1 }); expect(result.errors.map((e) => e.fileID)).toEqual([500]); expect(Object.keys(readLedger(outDir).files)).toEqual(["500"]); - expect(readdirSync(join(outDir, DAY))).toEqual(["2026-03-01.500.HEIC"]); - expect(tree(outDir)).toEqual([ - "Trip/", - `Trip/IMG_0500.HEIC -> ${linkTo("500.HEIC")}`, - "Trip.json", - ]); + expect(readdirSync(join(outDir, DAY))).toEqual([]); + expect(tree(outDir)).toEqual(["Trip/", "Trip.json"]); await lib.close(); }); @@ -1234,23 +1217,6 @@ describe("backup of live photos", () => { await lib.close(); }); - it("replaces an earlier ZIP and its link with the image and video the cache holds", async () => { - const { file: live, body } = await asLivePhoto( - file(500, 10, "IMG_0500.HEIC"), - ); - const lib = await open([live], new Map([[500, body]])); - await lib.photos.byID({ fileID: 500 })!.original(); - const outDir = join(root, "backup"); - earlierZIP(outDir); - - const result = await lib.backup({ downloadDirectory: outDir }); - - expect(result).toMatchObject({ downloaded: 1, failed: 0 }); - expect(readdirSync(join(outDir, DAY)).sort()).toEqual(stored); - expect(tree(outDir)).toEqual(linked); - await lib.close(); - }); - it.each(["missing", "empty"])( "fetches a live photo again when the video its JSON file names is %s", async (state) => { diff --git a/test/library/content-library.test.ts b/test/library/content-library.test.ts index 88e6371..01a3f60 100644 --- a/test/library/content-library.test.ts +++ b/test/library/content-library.test.ts @@ -379,6 +379,34 @@ describe("Photo save path, local copy, content and EXIF", () => { await lib.close(); }); + it("refuses an empty download directory", async () => { + await expect(open({ downloadDirectory: "" })).rejects.toThrow( + /downloadDirectory is empty/, + ); + }); + + it("keeps a photo's save path after a refresh removes its file", async () => { + // The same account, whose album is deleted on the second refresh. + class AlbumDeletedClient extends MockClient { + override async collectionsSince(): Promise { + if (!this.served) return super.collectionsSince(); + return { collections: [], deleted: [1], cursor: 2 }; + } + } + const lib = await open({ client: new AlbumDeletedClient() }); + const photo = lib.photos.byID({ fileID: 1 })!; + await photo.download(); + + await lib.fresh(); + + expect(lib.photos.byID({ fileID: 1 })).toBeUndefined(); + expect(photo.savePath).toBe( + join(root, "backup", DAY, "2026-03-01.1.jpg"), + ); + expect(photo.isLocal).toBe(true); + await lib.close(); + }); + it("has a save path, and is not local, without a content source", async () => { const lib = await open({ contentSource: undefined }); const photo = lib.photos.byID({ fileID: 1 })!;