From ead27ff419602e95b65c94d0a9911596625db3ec Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 1 Oct 2026 15:23:05 +0000 Subject: [PATCH] Photo: drop fileSize, exif() throws without a content cache Remove fileSize from Photo, PhotoRecord, README and TODO.md: the server's value is the size of the encrypted file, not the original's. exif() now checks for the content cache before it returns {} for a video, so it throws like original(), thumbnail() and content() when the library has no content source. README names the methods that also serve an original from the backup, since thumbnail() does not. New tests: exif() on a JPEG whose EXIF block cannot be parsed returns {}, and exif() on a video throws without a content source. Model: opus-5-5 --- README.md | 8 +++---- TODO.md | 6 +++--- src/library/read.ts | 7 +++---- src/library/records.ts | 3 --- test/library/content-library.test.ts | 31 ++++++++++++++++++++++++++++ test/library/read.test.ts | 4 +--- test/library/records.test.ts | 1 - 7 files changed, 42 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 8d1f865..90bfe83 100644 --- a/README.md +++ b/README.md @@ -722,8 +722,9 @@ Four async methods may download: UTC. Only a JPEG's EXIF is read: any other original gives `{}`, and a video gives `{}` without being downloaded. -They serve from the on-disk content cache, or the backup, when the bytes are -present and otherwise fetch through the pools; `opts.onProgress` reports +They serve from the on-disk content cache when the bytes are present and +otherwise fetch through the pools; `original()`, `content()` and `exif()` also +serve an original the backup has already stored. `opts.onProgress` reports per-file progress. They throw when the library was opened without a content source. An original that `content()` or `exif()` downloads lands in the cache, which does not make `isLocal` true; only `lib.backup()` does. @@ -741,8 +742,7 @@ The GUI-facing records hold no key material and no binary, so they survive - `PhotoRecord`: `fileID`, `albumIDs`, `title`, `takenAt` and `modifiedAt` (milliseconds), `fileType`, optional `caption` / `width` / `height` / `latitude` / `longitude`, optional `hash` (the content hash recorded at - upload; very old files have none) and `fileSize` (the original's size in - bytes, as the server reports it), `isArchived`, `isHidden`, and + upload; very old files have none), `isArchived`, `isHidden`, and `thumbnailPath` / `originalPath` once the bytes are cached (for a live photo, `originalPath` is its image). - `AlbumRecord`: `collectionID`, `name`, `type`, `isShared`, `updationTime`, and diff --git a/TODO.md b/TODO.md index 38e8033..96f2732 100644 --- a/TODO.md +++ b/TODO.md @@ -26,13 +26,13 @@ declares one. # Completed Steps - 2026-10-01: A `Photo` has `savePath`, `isLocal`, `content()`, `exif()`, - `modifiedAt`, `hash`, `fileSize` and `year` (issue 141). `savePath` is where + `modifiedAt`, `hash` and `year` (issue 141). `savePath` is where `lib.backup()` writes the original under the library's download directory, and `isLocal` says whether the whole original is there; both look only at the disk. `content()` returns the original's bytes and `exif()` the common EXIF fields of a JPEG; both may download the original, and `exif()` downloads no - video. `PhotoRecord` gains `modifiedAt`, `hash` and `fileSize`, and the JPEG - EXIF scan moved to `src/exif.ts`. + video. `PhotoRecord` gains `modifiedAt` and `hash`, and the JPEG EXIF scan + moved to `src/exif.ts`. - 2026-09-29: The "Workflow" list at the top of this file now says to branch from `next` and open a pull request that targets `next`, that the repository diff --git a/src/library/read.ts b/src/library/read.ts index af9b354..e34abeb 100644 --- a/src/library/read.ts +++ b/src/library/read.ts @@ -82,9 +82,6 @@ export class Photo { get hash(): string | undefined { return this.rec.hash; } - get fileSize(): number | undefined { - return this.rec.fileSize; - } get isArchived(): boolean { return this.rec.isArchived; } @@ -132,8 +129,10 @@ export class Photo { // The common EXIF fields of the original, read from `content()`, so this // may download it. Only a JPEG's EXIF is read; any other file gives `{}`, - // and a video gives it without fetching anything. + // and a video gives it without fetching anything. Like the other content + // methods, it throws when there is no content cache, video or not. async exif(opts?: ContentOptions): Promise { + this.cacheOrThrow(); if (this.rec.fileType === "video") return {}; return readPhotoExif(await this.content(opts)); } diff --git a/src/library/records.ts b/src/library/records.ts index 28f0546..5136ddb 100644 --- a/src/library/records.ts +++ b/src/library/records.ts @@ -45,8 +45,6 @@ export interface PhotoRecord { // The content hash the uploader recorded (`FileMetadata.hash`); files from // very old clients have none. hash?: string; - // The original's size in bytes, as the server reports it. - fileSize?: number; isArchived: boolean; isHidden: boolean; // Local cache paths, set once a later phase caches the bytes; unset here. @@ -150,7 +148,6 @@ const toPhotoRecord = ( if (rep.metadata.longitude !== undefined) record.longitude = rep.metadata.longitude; if (rep.metadata.hash !== undefined) record.hash = rep.metadata.hash; - if (rep.file.size !== undefined) record.fileSize = rep.file.size; return record; }; diff --git a/test/library/content-library.test.ts b/test/library/content-library.test.ts index a1264a7..debd23c 100644 --- a/test/library/content-library.test.ts +++ b/test/library/content-library.test.ts @@ -296,6 +296,16 @@ const JPEG_WITH_EXIF = new Uint8Array([ ...[0xff, 0xda, 0x00, 0x02], // start of scan ]); +// A JPEG whose EXIF segment is laid out correctly but holds "XX" where the TIFF +// byte order belongs, so exif-reader cannot parse it. +const JPEG_WITH_BAD_EXIF = new Uint8Array([ + ...[0xff, 0xd8], // start of image + ...[0xff, 0xe1, ...u16(2 + 6 + 2)], // APP1 and its length + ...[...ascii("Exif"), 0], // "Exif\0\0" + ...[0x58, 0x58], // "XX" + ...[0xff, 0xda, 0x00, 0x02], // start of scan +]); + describe("Photo save path, local copy, content and EXIF", () => { // The same account, with `files` in its album instead. class FilesClient extends MockClient { @@ -407,6 +417,27 @@ describe("Photo save path, local copy, content and EXIF", () => { await lib.close(); }); + it("returns no EXIF fields for a JPEG whose EXIF cannot be parsed", async () => { + const lib = await open({ + contentSource: stubSource(JPEG_WITH_BAD_EXIF), + }); + expect(await lib.photos.byID({ fileID: 1 })!.exif()).toStrictEqual({}); + await lib.close(); + }); + + it("throws from exif() on a video without a content source, as the other content methods do", async () => { + const video = file(1, 1); + video.metadata.fileType = "video"; + const lib = await open({ + client: new FilesClient([video]), + contentSource: undefined, + }); + await expect(lib.photos.byID({ fileID: 1 })!.exif()).rejects.toThrow( + /content cache/i, + ); + await lib.close(); + }); + it("returns no EXIF fields for a video, without fetching it", async () => { const video = file(1, 1); video.metadata.fileType = "video"; diff --git a/test/library/read.test.ts b/test/library/read.test.ts index 4d3c1e7..4bb7d45 100644 --- a/test/library/read.test.ts +++ b/test/library/read.test.ts @@ -209,7 +209,7 @@ describe("lib.photos", () => { expect("key" in photo.record()).toBe(false); }); - it("byID exposes modifiedAt, hash, fileSize and the year taken", () => { + it("byID exposes modifiedAt, hash and the year taken", () => { // Mid-July, so the year is 2021 in every time zone. const takenAt = Date.UTC(2021, 6, 15, 12); const records = deriveRecords( @@ -223,14 +223,12 @@ describe("lib.photos", () => { modificationTime: micros(1_700_000_123_456), hash: "aGFzaA==", }, - file: { decryptionHeader: "aGVhZGVy", size: 2_048_000 }, }), ], ); const photo = apis(records).photos.byID({ fileID: 1001 })!; expect(photo.modifiedAt).toBe(ms(1_700_000_123_456)); expect(photo.hash).toBe("aGFzaA=="); - expect(photo.fileSize).toBe(2_048_000); expect(photo.year).toBe(2021); }); diff --git a/test/library/records.test.ts b/test/library/records.test.ts index 0d0e234..c05b09a 100644 --- a/test/library/records.test.ts +++ b/test/library/records.test.ts @@ -160,7 +160,6 @@ describe("deriveRecords: photo mapping", () => { expect("height" in rec).toBe(false); expect("latitude" in rec).toBe(false); expect("hash" in rec).toBe(false); - expect("fileSize" in rec).toBe(false); }); it("reads archived and hidden from private magicMetadata.visibility", () => {