diff --git a/README.md b/README.md index 12c99e2..2c8b6a7 100644 --- a/README.md +++ b/README.md @@ -879,10 +879,7 @@ current account's records name. A live photo's original is cached as at its save path: its image and its video, each `originals/.` with its own extension, and -`originals/.livephoto.json` naming them; the two are evicted together. A -live photo that an earlier version cached as its ZIP is not served: the library -removes the ZIP when it opens the cache, and fetches the two files when the -photo is next read or precached. +`originals/.livephoto.json` naming them; the two are evicted together. A stored file appears only via an atomic temp-then-rename, so its presence means it is complete. Every downloaded original (by `quak get`, the cache, or diff --git a/TODO.md b/TODO.md index 29cfd56..f37e1cb 100644 --- a/TODO.md +++ b/TODO.md @@ -25,6 +25,10 @@ declares one. # Completed Steps +- 2026-10-01: The content cache no longer looks for a live photo that an earlier + version cached as one ZIP (issue 151). When the cache opens, a live photo's + file that no JSON file names is now always left alone. + - 2026-10-01: `examples/download-albums.ts` logs in, opens the library, and for every album downloads each photo to its save path, writes the photo's record and EXIF fields to a JSON file beside it, and writes the album's photos to diff --git a/src/download/index.ts b/src/download/index.ts index 7729890..e27184b 100644 --- a/src/download/index.ts +++ b/src/download/index.ts @@ -361,8 +361,8 @@ const openPart = async ( // written unpacked: each part is named `destination` with the extension // replaced by its own entry's, and the two must differ ignoring case. When the // file records a hash, `:` must match it, each over that -// part's own bytes. Only then is whatever was at `destination` removed and the -// image, then the video, renamed into place; on any failure neither is stored. +// part's own bytes. Only then are the image, then the video, renamed into +// place; on any failure neither is stored. // // The ZIP is chosen by its uploader and may expand enormously, so each part is // written as it decompresses and never held, and the ZIP is refused once the @@ -486,7 +486,6 @@ const decryptLivePhoto = async ( await part.handle.sync(); await part.handle.close(); } - await rm(destination, { force: true }); await rename(image.tmpPath, path); try { await rename(video.tmpPath, videoPath); @@ -570,8 +569,7 @@ const fetchAndDecrypt = async ( }, api.getRetryOptions()); // Write `file`'s original to `outPath`. A live photo is written as its image -// and its video beside `outPath` instead, and whatever was at `outPath` is -// removed (see `decryptLivePhoto`). +// and its video beside `outPath` instead (see `decryptLivePhoto`). export const downloadFile = async ( api: ApiClient, file: EnteFile, diff --git a/src/library/content.ts b/src/library/content.ts index 396fdb8..383319b 100644 --- a/src/library/content.ts +++ b/src/library/content.ts @@ -29,14 +29,7 @@ // the cache does not count as saved there, but is copied there rather than // fetched again. -import { - closeSync, - existsSync, - openSync, - readFileSync, - readSync, - statSync, -} from "node:fs"; +import { existsSync, readFileSync, statSync } from "node:fs"; import { chmod, copyFile, @@ -281,25 +274,6 @@ const fileSize = (path: string): number | undefined => { const hasContent = (path: string | undefined): boolean => path !== undefined && (fileSize(path) ?? 0) > 0; -// Whether the file at `path` begins as a ZIP does, with `PK\x03\x04`. False -// when it cannot be read. -const isZip = (path: string): boolean => { - try { - const fd = openSync(path, "r"); - try { - const head = Buffer.alloc(4); - return ( - readSync(fd, head, 0, 4, 0) === 4 && - head.toString("latin1") === "PK\x03\x04" - ); - } finally { - closeSync(fd); - } - } catch { - return false; - } -}; - // A live photo's image and video are named with the extensions from inside its // ZIP, so their names alone do not say which is which. Wherever the cache or a // save path stores one, a JSON file of this name beside them names both. @@ -713,8 +687,10 @@ export class ContentCache implements PhotoContent, ThumbnailsAPI { await this.touch(cached.path); return { ...cached, bytes: size, cached: true }; } - // A recorded file that has since gone, or a live photo an earlier - // version stored as one ZIP, re-fetches below. + // A recorded file that has since gone re-fetches below. So does a + // live photo recorded with no video: the cache opened before the + // library's records said it is a live photo, while its image and + // video had no JSON file beside them yet. known.delete(fileID); } @@ -983,20 +959,14 @@ export class ContentCache implements PhotoContent, ThumbnailsAPI { if (id === undefined || !existsSync(path)) continue; // A live photo's image and video are one entry, as the JSON file // beside them names them. A live photo's file with no such JSON - // file is not its original. If it is a ZIP, it is the one an - // earlier version stored under the image's name, and is removed. - // Any other is left alone: another process may have just stored - // it and not yet written the JSON file. + // file is not its original and is left alone: another process may + // have just stored it and not yet written the JSON file. const livePhoto = names.has(livePhotoJSONName(String(id))) ? readLivePhotoJSON(dir, String(id)) : undefined; if (livePhoto !== undefined) { into.set(id, livePhoto); - } else if (isLivePhoto(id)) { - if (isZip(path)) { - await rm(path, { force: true }).catch(() => undefined); - } - } else { + } else if (!isLivePhoto(id)) { into.set(id, { path }); } } diff --git a/test/download/download.test.ts b/test/download/download.test.ts index e08a948..06788fd 100644 --- a/test/download/download.test.ts +++ b/test/download/download.test.ts @@ -1883,15 +1883,6 @@ describe("downloadFile live photos", () => { expect(readdirSync(t.dir).sort()).toEqual(["f.JPG", "f.bin"]); }); - it("replaces what was at the destination, such as an earlier ZIP of the two", async () => { - const t = setup(livePhotoZip(), livePhoto); - writeFileSync(t.outPath, livePhotoZip()); - - await t.run(); - - expect(readdirSync(t.dir).sort()).toEqual(["f.heic", "f.mov"]); - }); - it("renames the image and then the video into place, each from its own temp file", async () => { const t = setup(livePhotoZip(), livePhoto); diff --git a/test/library/content-library.test.ts b/test/library/content-library.test.ts index ffa0cc7..d5976a4 100644 --- a/test/library/content-library.test.ts +++ b/test/library/content-library.test.ts @@ -176,7 +176,7 @@ describe("Library content wiring", () => { await lib.close(); }); - it("removes a live photo's ZIP an earlier version cached when it opens, and precaches its image and video", async () => { + it("does not take a live photo's image or video with no JSON file as its original when it opens, and precaches both", async () => { const { file: live, body } = await asLivePhoto(file(1, 1)); class LiveClient extends MockClient { override async filesSince(): Promise { @@ -197,7 +197,8 @@ describe("Library content wiring", () => { // A first run records the library, so the next one knows that file 1 // is a live photo when it opens the cache. await (await open({})).close(); - writeFileSync(join(originals, "1.jpg"), livePhotoZip()); + writeFileSync(join(originals, "1.heic"), "an image"); + writeFileSync(join(originals, "1.mov"), "a video"); let precached!: () => void; const done = new Promise((r) => (precached = r)); @@ -208,7 +209,9 @@ describe("Library content wiring", () => { precached(); }, }); - expect(existsSync(join(originals, "1.jpg"))).toBe(false); + expect( + lib.photos.byID({ fileID: 1 })!.record().originalPath, + ).toBeUndefined(); await done; expect(readdirSync(originals).sort()).toEqual([ @@ -216,6 +219,17 @@ describe("Library content wiring", () => { "1.livephoto.json", "1.mov", ]); + expect(readFileSync(join(originals, "1.heic"))).toEqual( + Buffer.from(IMAGE), + ); + expect(readFileSync(join(originals, "1.mov"))).toEqual( + Buffer.from(VIDEO), + ); + expect( + JSON.parse( + readFileSync(join(originals, "1.livephoto.json"), "utf-8"), + ), + ).toEqual({ image: "1.heic", video: "1.mov" }); expect(lib.photos.byID({ fileID: 1 })!.record().originalPath).toBe( join(originals, "1.heic"), ); diff --git a/test/library/content.test.ts b/test/library/content.test.ts index f209359..b461742 100644 --- a/test/library/content.test.ts +++ b/test/library/content.test.ts @@ -566,43 +566,6 @@ describe("ContentCache live photos", () => { expect(events).toEqual(["skipped"]); }); - it("replaces a live photo an earlier version stored as a ZIP under the image's name", async () => { - const { file: live, body } = await asLivePhoto(file(5, "IMG_5.HEIC")); - mkdirSync(originals(), { recursive: true }); - writeFileSync(join(originals(), "5.HEIC"), livePhotoZip()); - const cache = cacheOf([live], new Map([[5, body]])); - // Opened without being told that file 5 is a live photo, the cache - // records the ZIP, and does not serve it. - await cache.open(); - - const result = await cache.original(5); - - expect(result.videoPath).toBe(join(originals(), "5.mov")); - expect(readdirSync(originals()).sort()).toEqual([ - "5.heic", - "5.livephoto.json", - "5.mov", - ]); - }); - - it("removes a live photo's ZIP an earlier version stored when it opens, so the precache fetches the image and video", async () => { - const { file: live, body } = await asLivePhoto(file(5, "IMG_5.HEIC")); - mkdirSync(originals(), { recursive: true }); - writeFileSync(join(originals(), "5.HEIC"), livePhotoZip()); - const cache = cacheOf([live], new Map([[5, body]])); - - await cache.open((fileID) => fileID === 5); - - expect(readdirSync(originals())).toEqual([]); - expect(cache.pathsFor(5)).toEqual({}); - const [fetched] = await cache.ensureOriginals({ fileIDs: [5] }); - expect(fetched).toEqual({ - fileID: 5, - path: join(originals(), "5.heic"), - }); - expect(cache.pathsFor(5)).toEqual({ originalPath: fetched!.path }); - }); - it("leaves the image and video another process has just stored when it opens before their JSON file is written", async () => { const { file: live, body } = await asLivePhoto(file(5, "IMG_5.HEIC")); const server = cdnSource(new Map([[5, body]])); @@ -633,6 +596,36 @@ describe("ContentCache live photos", () => { expect(second!.pathsFor(5)).toEqual({}); }); + it("fetches a live photo's image and video again when the cache opened before knowing it is a live photo and no JSON file names them", async () => { + const { file: live, body } = await asLivePhoto(file(5, "IMG_5.HEIC")); + mkdirSync(originals(), { recursive: true }); + writeFileSync(join(originals(), "5.heic"), "an image"); + writeFileSync(join(originals(), "5.mov"), "a video"); + const cache = cacheOf([live], new Map([[5, body]])); + // Opened without being told that file 5 is a live photo, the cache + // records one of the two files as its original, with no video. + await cache.open(); + + const events: string[] = []; + const result = await cache.original(5, { + onProgress: (e) => events.push(e.status), + }); + + expect(events.at(-1)).toBe("done"); + expect(result).toEqual({ + path: join(originals(), "5.heic"), + videoPath: join(originals(), "5.mov"), + bytes: IMAGE.length, + }); + expect(readFileSync(result.path)).toEqual(Buffer.from(IMAGE)); + expect(readFileSync(result.videoPath!)).toEqual(Buffer.from(VIDEO)); + expect( + JSON.parse( + readFileSync(join(originals(), "5.livephoto.json"), "utf-8"), + ), + ).toEqual({ image: "5.heic", video: "5.mov" }); + }); + it.each(["missing", "empty"])( "fetches a live photo again when the video its JSON file names is %s", async (state) => {