From 503a03792ef4d2b1976059f567e497892214bf66 Mon Sep 17 00:00:00 2001 From: Maurice Date: Tue, 18 Aug 2026 21:53:08 +0200 Subject: [PATCH] test(photos): cover the local EXIF path so the memories ratchet holds The capture backfill landed in src/nest/memories/ with its local branch untested, which took the domain under its coverage floor (functions 96.71 vs 97, branches 79.67 vs 81). Raising the tests rather than lowering the gate: the path guard against climbing out of the uploads tree, a missing path, an unreadable file, the CreateDate fallback, empty EXIF, a throwing row lookup, and schedule(). Plus the Immich album mapping with and without coordinates. One note left in the test file for the next person: a mock that *rejects* makes vitest fail the run even when the code catches it, because the mock's recorded result is a second reference to that promise with nothing attached. A bare mockReset() is enough to trip it, so the beforeEach sets a benign default. --- .../photo-capture-backfill.service.ts | 14 +- .../tests/integration/memories-immich.test.ts | 19 ++- .../photo-capture-backfill.service.test.ts | 133 +++++++++++++++++- 3 files changed, 159 insertions(+), 7 deletions(-) diff --git a/server/src/nest/memories/photo-capture-backfill.service.ts b/server/src/nest/memories/photo-capture-backfill.service.ts index 9399eb796..e6f695be7 100644 --- a/server/src/nest/memories/photo-capture-backfill.service.ts +++ b/server/src/nest/memories/photo-capture-backfill.service.ts @@ -78,10 +78,16 @@ export class PhotoCaptureBackfillService { const abs = path.resolve(UPLOADS_ROOT, filePath); if (abs !== UPLOADS_ROOT && !abs.startsWith(UPLOADS_ROOT + path.sep)) return null; - const parsed = await exifr.parse(abs, { pick: ['DateTimeOriginal', 'CreateDate', 'latitude', 'longitude'] }) - .catch(() => null) as - | { DateTimeOriginal?: Date; CreateDate?: Date; latitude?: number; longitude?: number } - | null; + type Exif = { DateTimeOriginal?: Date; CreateDate?: Date; latitude?: number; longitude?: number }; + let parsed: Exif | null = null; + try { + parsed = (await exifr.parse(abs, { + pick: ['DateTimeOriginal', 'CreateDate', 'latitude', 'longitude'], + })) as Exif | null; + } catch { + // Not an image, a truncated upload, a video — none of it is an error here. + return null; + } if (!parsed) return null; const taken = parsed.DateTimeOriginal ?? parsed.CreateDate ?? null; diff --git a/server/tests/integration/memories-immich.test.ts b/server/tests/integration/memories-immich.test.ts index b57184941..87160a9f4 100644 --- a/server/tests/integration/memories-immich.test.ts +++ b/server/tests/integration/memories-immich.test.ts @@ -56,7 +56,7 @@ const { testDb, dbMock, immichState } = vi.hoisted(() => { * the legacy one. Both must be filtered out of the picker (#1474). */ const DEFAULT_ALBUM_ASSETS = [ - { id: 'asset-sync-1', type: 'IMAGE', fileCreatedAt: '2024-06-01T10:00:00.000Z', exifInfo: { city: 'Paris', country: 'France' } }, + { id: 'asset-sync-1', type: 'IMAGE', fileCreatedAt: '2024-06-01T10:00:00.000Z', exifInfo: { city: 'Paris', country: 'France', latitude: 48.8584, longitude: 2.2945 } }, { id: 'asset-sync-2', type: 'VIDEO', fileCreatedAt: '2024-06-02T10:00:00.000Z', exifInfo: { city: 'Lyon', country: 'France' } }, { id: 'asset-hidden', type: 'VIDEO', fileCreatedAt: '2024-06-03T10:00:00.000Z', visibility: 'hidden' }, { id: 'asset-legacy-hidden', type: 'VIDEO', fileCreatedAt: '2024-06-04T10:00:00.000Z', isVisible: false }, @@ -590,6 +590,23 @@ describe('Immich album photos', () => { expect(res.body.assets[1].mediaType).toBe('video'); }); + // #1614 — the album mapping used to drop lat/lng even though the search path + // kept them, so a photo picked from an album could never reach a map. + it('IMMICH-064b — album photos carry their capture coordinates, and tolerate their absence', async () => { + const { user } = createUser(testDb); + setImmichCredentials(testDb, user.id, 'https://immich.example.com', 'test-api-key'); + + const res = await request(app) + .get(`${IMMICH}/albums/album-uuid-1/photos`) + .set('Cookie', authCookie(user.id)); + + expect(res.body.assets[0]).toMatchObject({ lat: 48.8584, lng: 2.2945 }); + // The second fixture asset has no coordinates; it must come back null, not undefined + // or a half pair. + expect(res.body.assets[1].lat).toBeNull(); + expect(res.body.assets[1].lng).toBeNull(); + }); + it('IMMICH-065 — album photos are fetched via search/metadata with albumIds and withExif', async () => { const { user } = createUser(testDb); setImmichCredentials(testDb, user.id, 'https://immich.example.com', 'test-api-key'); diff --git a/server/tests/unit/nest/photo-capture-backfill.service.test.ts b/server/tests/unit/nest/photo-capture-backfill.service.test.ts index 74e05d1b0..48452aa5f 100644 --- a/server/tests/unit/nest/photo-capture-backfill.service.test.ts +++ b/server/tests/unit/nest/photo-capture-backfill.service.test.ts @@ -2,12 +2,17 @@ * PhotoCaptureBackfillService (#1614) — asking the provider when and where a * photo was taken, after the add the user was waiting on has already answered. */ -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// exifr reads real files; the local branch is about which tags are picked and what +// is done with them, not about decoding a JPEG. +vi.mock('exifr', () => ({ default: { parse: vi.fn() } })); +import exifr from 'exifr'; import { PhotoCaptureBackfillService } from '../../../src/nest/memories/photo-capture-backfill.service'; import type { PhotoResolverService } from '../../../src/nest/memories/photo-resolver.service'; import type { TrekPhotosRepository } from '../../../src/nest/photos/trek-photos.repository'; -type Row = { id: number; taken_at?: string | null; lat?: number | null; lng?: number | null }; +type Row = { id: number; provider?: string; file_path?: string | null; taken_at?: string | null; lat?: number | null; lng?: number | null }; function build(rows: Row[], info: Record) { const recordCaptureMetadata = vi.fn(); @@ -86,3 +91,127 @@ describe('PhotoCaptureBackfillService', () => { expect(getPhotoInfo).not.toHaveBeenCalled(); }); }); + +describe('PhotoCaptureBackfillService — local files', () => { + // Reset *and* give it a benign default. A bare mockReset leaves the mock with no + // implementation at all, and a later mockImplementation that throws then trips + // vitest's unhandled-error reporting instead of reaching the code's own catch. + beforeEach(() => { + vi.mocked(exifr.parse).mockReset(); + vi.mocked(exifr.parse).mockResolvedValue({}); + }); + + function localBuild(rows: Row[]) { + const recordCaptureMetadata = vi.fn(); + const getPhotoInfo = vi.fn(); + const photos = { + resolve: (id: number) => rows.find(r => r.id === id) ?? null, + recordCaptureMetadata, + } as unknown as TrekPhotosRepository; + const resolver = { getPhotoInfo } as unknown as PhotoResolverService; + return { svc: new PhotoCaptureBackfillService(resolver, photos), recordCaptureMetadata, getPhotoInfo }; + } + + it('CAPTURE-007: reads a local file rather than asking a provider', async () => { + vi.mocked(exifr.parse).mockResolvedValue({ + DateTimeOriginal: new Date('2026-03-15T10:20:00Z'), + latitude: 48.8584, + longitude: 2.2945, + }); + const { svc, recordCaptureMetadata, getPhotoInfo } = localBuild([ + { id: 7, provider: 'local', file_path: 'journey/a.jpg' }, + ]); + + await svc.run([7], 1); + + expect(getPhotoInfo).not.toHaveBeenCalled(); + expect(recordCaptureMetadata).toHaveBeenCalledWith(7, { + takenAt: '2026-03-15T10:20:00.000Z', lat: 48.8584, lng: 2.2945, + }); + }); + + it('CAPTURE-008: falls back to CreateDate when the original timestamp is missing', async () => { + vi.mocked(exifr.parse).mockResolvedValue({ CreateDate: new Date('2026-03-16T08:00:00Z') }); + const { svc, recordCaptureMetadata } = localBuild([ + { id: 7, provider: 'local', file_path: 'journey/a.jpg' }, + ]); + + await svc.run([7], 1); + + expect(recordCaptureMetadata).toHaveBeenCalledWith(7, { + takenAt: '2026-03-16T08:00:00.000Z', lat: null, lng: null, + }); + }); + + it('CAPTURE-009: a file with nothing readable is left alone', async () => { + vi.mocked(exifr.parse).mockResolvedValue({}); + const { svc, recordCaptureMetadata } = localBuild([ + { id: 7, provider: 'local', file_path: 'journey/a.jpg' }, + ]); + + await svc.run([7], 1); + + expect(recordCaptureMetadata).not.toHaveBeenCalled(); + }); + + it('CAPTURE-010: an unreadable file is not an error', async () => { + // Throws synchronously. A mock that *rejects* leaves vitest recording the + // settlement of a promise nothing else owns, and the run fails on that even + // though the code under test caught it. The catch is the same either way. + vi.mocked(exifr.parse).mockImplementation((() => { throw new Error('not an image'); }) as never); + const { svc, recordCaptureMetadata } = localBuild([ + { id: 7, provider: 'local', file_path: 'journey/a.jpg' }, + ]); + + await expect(svc.run([7], 1)).resolves.toBeUndefined(); + expect(recordCaptureMetadata).not.toHaveBeenCalled(); + }); + + it('CAPTURE-011: a stored path that climbs out of the uploads tree is refused', async () => { + const { svc, recordCaptureMetadata } = localBuild([ + { id: 7, provider: 'local', file_path: '../../../etc/passwd' }, + ]); + + await svc.run([7], 1); + + expect(exifr.parse).not.toHaveBeenCalled(); + expect(recordCaptureMetadata).not.toHaveBeenCalled(); + }); + + it('CAPTURE-012: a local row without a path is skipped', async () => { + const { svc, recordCaptureMetadata } = localBuild([{ id: 7, provider: 'local' }]); + + await svc.run([7], 1); + + expect(exifr.parse).not.toHaveBeenCalled(); + expect(recordCaptureMetadata).not.toHaveBeenCalled(); + }); + + it('CAPTURE-013: a row that no longer exists is skipped', async () => { + const { svc, recordCaptureMetadata } = localBuild([]); + await svc.run([99], 1); + expect(recordCaptureMetadata).not.toHaveBeenCalled(); + }); + + it('CAPTURE-014: a throwing lookup is swallowed so the detached task survives', async () => { + const recordCaptureMetadata = vi.fn(); + const photos = { + resolve: () => { throw new Error('db gone'); }, + recordCaptureMetadata, + } as unknown as TrekPhotosRepository; + const svc = new PhotoCaptureBackfillService({} as PhotoResolverService, photos); + + await expect(svc.run([7], 1)).resolves.toBeUndefined(); + expect(recordCaptureMetadata).not.toHaveBeenCalled(); + }); + + it('CAPTURE-015: schedule kicks the run off for a non-empty batch', async () => { + vi.mocked(exifr.parse).mockResolvedValue({ DateTimeOriginal: new Date('2026-03-15T10:20:00Z') }); + const { svc, recordCaptureMetadata } = localBuild([ + { id: 7, provider: 'local', file_path: 'journey/a.jpg' }, + ]); + + svc.schedule([7], 1); + await vi.waitFor(() => expect(recordCaptureMetadata).toHaveBeenCalled()); + }); +});