From aa4ed69677c95d94fa884fcb0ffad794368ed2cf Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 8 Aug 2026 13:53:01 +0200 Subject: [PATCH] refactor(server): give trek_photos its own repository photoResolverService was two files in one. One half registers and drops rows in trek_photos and touches no photo provider at all; the other dispatches a stream or an info lookup to immich or synology. Keeping them together meant every consumer that only wanted to store a row - the unified trip-photo service, the journey service - dragged both providers in behind it. The storage half becomes TrekPhotosRepository in nest/photos, over DatabaseService. That is also where the photos/ vs memories/ line belongs: storage here, provider dispatch with memories. photos.bridge.ts serves the two modules still in services/ that register rows, and dies with them. Every statement moved verbatim, including the two rules worth stating out loud: getOrCreate re-encrypts a passphrase onto an existing row rather than inserting a second one, and deleteIfOrphan never deletes a local photo, because its bytes are ours and the file would outlive the row. --- server/src/nest/photos/photos.bridge.ts | 37 ++++++++ server/src/nest/photos/photos.module.ts | 10 +- .../src/nest/photos/trek-photos.repository.ts | 95 +++++++++++++++++++ server/src/services/journeyService.ts | 2 +- .../services/memories/photoResolverService.ts | 81 +--------------- .../src/services/memories/unifiedService.ts | 2 +- .../integration/memories-synology.test.ts | 2 +- .../unit/services/trekPhotoMedia.test.ts | 2 +- 8 files changed, 149 insertions(+), 82 deletions(-) create mode 100644 server/src/nest/photos/photos.bridge.ts create mode 100644 server/src/nest/photos/trek-photos.repository.ts diff --git a/server/src/nest/photos/photos.bridge.ts b/server/src/nest/photos/photos.bridge.ts new file mode 100644 index 000000000..50775dd06 --- /dev/null +++ b/server/src/nest/photos/photos.bridge.ts @@ -0,0 +1,37 @@ +import { db } from '../../db/database'; +import { DatabaseService } from '../database/database.service'; +import { TrekPhotosRepository } from './trek-photos.repository'; + +/** + * Non-Nest entry point for the `trek_photos` store — for the modules still + * living in `src/services/` that only need to register or drop a row: + * journeyService (photo upload, Immich sync, entry cleanup) and the memories + * unifiedService. Exports the legacy photoResolverService names 1:1, so + * repointing a consumer is an import-path-only diff. Inside the container, + * inject TrekPhotosRepository instead. Delete this file when journeyService and + * the memories cluster fold. + * + * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto + * the shared better-sqlite3 singleton, and the repository holds no state. + */ +const photos = new TrekPhotosRepository(new DatabaseService(db)); + +export function getOrCreateTrekPhoto(...args: Parameters) { + return photos.getOrCreate(...args); +} + +export function getOrCreateLocalTrekPhoto(...args: Parameters) { + return photos.getOrCreateLocal(...args); +} + +export function resolveTrekPhoto(photoId: number) { + return photos.resolve(photoId); +} + +export function setTrekPhotoProvider(...args: Parameters) { + return photos.setProvider(...args); +} + +export function deleteTrekPhotoIfOrphan(photoId: number): void { + photos.deleteIfOrphan(photoId); +} diff --git a/server/src/nest/photos/photos.module.ts b/server/src/nest/photos/photos.module.ts index 2f42baa54..88a7f567d 100644 --- a/server/src/nest/photos/photos.module.ts +++ b/server/src/nest/photos/photos.module.ts @@ -1,9 +1,17 @@ import { Module } from '@nestjs/common'; import { PhotosController } from './photos.controller'; import { PhotosService } from './photos.service'; +import { TrekPhotosRepository } from './trek-photos.repository'; +/** + * The trek_photos store plus the /api/photos read surface. The repository is + * exported because the trip-photo and journey domains register rows in it + * without caring which provider the bytes come from — that dispatch stays in + * memories/. + */ @Module({ controllers: [PhotosController], - providers: [PhotosService], + providers: [PhotosService, TrekPhotosRepository], + exports: [TrekPhotosRepository], }) export class PhotosModule {} diff --git a/server/src/nest/photos/trek-photos.repository.ts b/server/src/nest/photos/trek-photos.repository.ts new file mode 100644 index 000000000..718bff700 --- /dev/null +++ b/server/src/nest/photos/trek-photos.repository.ts @@ -0,0 +1,95 @@ +import { Injectable } from '@nestjs/common'; +import { encrypt_api_key } from '../common/crypto/apiKeyCrypto'; +import { DatabaseService } from '../database/database.service'; +import type { TrekPhoto } from '../../types'; + +/** + * The `trek_photos` table: register a photo, look one up, retarget it, drop it + * when nothing references it any more. + * + * Split out of photoResolverService, which was two things in one file — this + * half touches no photo provider at all, while the other half (streamPhoto, + * getPhotoInfo, streamCachedThumbnail) dispatches to immich/synology. Keeping + * them together is what forced every consumer that just wanted to store a row — + * the unified trip-photo service, the journey service — to drag both providers + * in behind it. + * + * It also draws the photos/ vs memories/ line: storage lives here, provider + * dispatch stays with memories. + */ +@Injectable() +export class TrekPhotosRepository { + constructor(private readonly db: DatabaseService) {} + + getOrCreate( + provider: string, + assetId: string, + ownerId: number, + passphrase?: string, + mediaType: string = 'image', + ): number { + const existing = this.db.get<{ id: number }>( + 'SELECT id FROM trek_photos WHERE provider = ? AND asset_id = ? AND owner_id = ?', + provider, assetId, ownerId, + ); + if (existing) { + if (passphrase) { + this.db.run('UPDATE trek_photos SET passphrase = ? WHERE id = ?', encrypt_api_key(passphrase), existing.id); + } + return existing.id; + } + + const res = this.db.run( + 'INSERT INTO trek_photos (provider, asset_id, owner_id, passphrase, media_type) VALUES (?, ?, ?, ?, ?)', + provider, assetId, ownerId, passphrase ? encrypt_api_key(passphrase) : null, mediaType, + ); + return Number(res.lastInsertRowid); + } + + getOrCreateLocal( + filePath: string, + thumbnailPath?: string | null, + width?: number | null, + height?: number | null, + mediaType: string = 'image', + durationMs?: number | null, + ): number { + const existing = this.db.get<{ id: number }>( + "SELECT id FROM trek_photos WHERE provider = 'local' AND file_path = ?", filePath, + ); + if (existing) return existing.id; + + const res = this.db.run( + 'INSERT INTO trek_photos (provider, file_path, thumbnail_path, width, height, media_type, duration_ms) VALUES (?, ?, ?, ?, ?, ?, ?)', + 'local', filePath, thumbnailPath || null, width || null, height || null, mediaType, durationMs ?? null, + ); + return Number(res.lastInsertRowid); + } + + resolve(photoId: number): TrekPhoto | null { + return this.db.get('SELECT * FROM trek_photos WHERE id = ?', photoId) || null; + } + + /** Retarget an existing row — used when a local photo is uploaded to Immich. */ + setProvider(trekPhotoId: number, provider: string, assetId: string, ownerId: number): void { + this.db.run( + 'UPDATE trek_photos SET provider = ?, asset_id = ?, owner_id = ? WHERE id = ?', + provider, assetId, ownerId, trekPhotoId, + ); + } + + /** + * Drop the row once no trip and no journey references it. Local photos are + * kept: their bytes are ours, and the file would outlive the row. + */ + deleteIfOrphan(photoId: number): void { + const stillUsed = this.db.get(` + SELECT 1 FROM trip_photos WHERE photo_id = ? + UNION ALL + SELECT 1 FROM journey_photos WHERE photo_id = ? + LIMIT 1 + `, photoId, photoId); + if (stillUsed) return; + this.db.run("DELETE FROM trek_photos WHERE id = ? AND provider != 'local'", photoId); + } +} diff --git a/server/src/services/journeyService.ts b/server/src/services/journeyService.ts index c845c37fd..7b619c3ec 100644 --- a/server/src/services/journeyService.ts +++ b/server/src/services/journeyService.ts @@ -7,7 +7,7 @@ import { getOrCreateLocalTrekPhoto, setTrekPhotoProvider, deleteTrekPhotoIfOrphan, -} from './memories/photoResolverService'; +} from '../nest/photos/photos.bridge'; function ts(): number { return Date.now(); diff --git a/server/src/services/memories/photoResolverService.ts b/server/src/services/memories/photoResolverService.ts index 05750cdfa..2cbfd8d19 100644 --- a/server/src/services/memories/photoResolverService.ts +++ b/server/src/services/memories/photoResolverService.ts @@ -3,63 +3,17 @@ import path from 'path'; import fs from 'fs'; import { db } from '../../db/database'; import type { TrekPhoto } from '../../types'; +// The trek_photos rows live in nest/photos now — this module is the provider +// dispatch half of what used to be one file. +import { resolveTrekPhoto } from '../../nest/photos/photos.bridge'; import { streamImmichAsset, fetchImmichThumbnailBytes, getAssetInfo as getImmichAssetInfo } from './immichService'; import { streamSynologyAsset, fetchSynologyThumbnailBytes, getSynologyAssetInfo } from './synologyService'; import type { ServiceResult, AssetInfo } from './helpersService'; import { fail, success } from './helpersService'; -import { encrypt_api_key, decrypt_api_key } from '../../nest/common/crypto/apiKeyCrypto'; +import { decrypt_api_key } from '../../nest/common/crypto/apiKeyCrypto'; import * as photoCache from './trekPhotoCache'; import { ensureLocalThumbnail } from './thumbnailService'; -// ── Lookup / Register ──────────────────────────────────────────────────── - -export function getOrCreateTrekPhoto( - provider: string, - assetId: string, - ownerId: number, - passphrase?: string, - mediaType: string = 'image', -): number { - const existing = db.prepare( - 'SELECT id FROM trek_photos WHERE provider = ? AND asset_id = ? AND owner_id = ?' - ).get(provider, assetId, ownerId) as { id: number } | undefined; - if (existing) { - if (passphrase) { - db.prepare('UPDATE trek_photos SET passphrase = ? WHERE id = ?') - .run(encrypt_api_key(passphrase), existing.id); - } - return existing.id; - } - - const res = db.prepare( - 'INSERT INTO trek_photos (provider, asset_id, owner_id, passphrase, media_type) VALUES (?, ?, ?, ?, ?)' - ).run(provider, assetId, ownerId, passphrase ? encrypt_api_key(passphrase) : null, mediaType); - return Number(res.lastInsertRowid); -} - -export function getOrCreateLocalTrekPhoto( - filePath: string, - thumbnailPath?: string | null, - width?: number | null, - height?: number | null, - mediaType: string = 'image', - durationMs?: number | null, -): number { - const existing = db.prepare( - "SELECT id FROM trek_photos WHERE provider = 'local' AND file_path = ?" - ).get(filePath) as { id: number } | undefined; - if (existing) return existing.id; - - const res = db.prepare( - 'INSERT INTO trek_photos (provider, file_path, thumbnail_path, width, height, media_type, duration_ms) VALUES (?, ?, ?, ?, ?, ?, ?)' - ).run('local', filePath, thumbnailPath || null, width || null, height || null, mediaType, durationMs ?? null); - return Number(res.lastInsertRowid); -} - -export function resolveTrekPhoto(photoId: number): TrekPhoto | null { - return db.prepare('SELECT * FROM trek_photos WHERE id = ?').get(photoId) as TrekPhoto | undefined || null; -} - // ── Streaming ──────────────────────────────────────────────────────────── async function streamCachedThumbnail( @@ -218,30 +172,3 @@ export async function getPhotoInfo( return fail(`Unknown provider: ${photo.provider}`, 400); } } - -// ── Update provider on existing trek_photo (for Immich upload sync) ───── - -export function setTrekPhotoProvider( - trekPhotoId: number, - provider: string, - assetId: string, - ownerId: number, -): void { - db.prepare( - 'UPDATE trek_photos SET provider = ?, asset_id = ?, owner_id = ? WHERE id = ?' - ).run(provider, assetId, ownerId, trekPhotoId); -} - -// ── Orphan cleanup ─────────────────────────────────────────────────────── - -export function deleteTrekPhotoIfOrphan(photoId: number): void { - const stillUsed = db.prepare(` - SELECT 1 FROM trip_photos WHERE photo_id = ? - UNION ALL - SELECT 1 FROM journey_photos WHERE photo_id = ? - LIMIT 1 - `).get(photoId, photoId); - if (stillUsed) return; - db.prepare("DELETE FROM trek_photos WHERE id = ? AND provider != 'local'").run(photoId); -} - diff --git a/server/src/services/memories/unifiedService.ts b/server/src/services/memories/unifiedService.ts index 46f4cf63c..c92d26036 100644 --- a/server/src/services/memories/unifiedService.ts +++ b/server/src/services/memories/unifiedService.ts @@ -10,7 +10,7 @@ import { Selection, updateSyncTimeForAlbumLink, } from './helpersService'; -import { getOrCreateTrekPhoto, deleteTrekPhotoIfOrphan } from './photoResolverService'; +import { getOrCreateTrekPhoto, deleteTrekPhotoIfOrphan } from '../../nest/photos/photos.bridge'; import { collectAlbumSelection } from './immichService'; import { collectSynologyAlbumSelection } from './synologyService'; import { encrypt_api_key } from '../../nest/common/crypto/apiKeyCrypto'; diff --git a/server/tests/integration/memories-synology.test.ts b/server/tests/integration/memories-synology.test.ts index befa14368..81004e28b 100644 --- a/server/tests/integration/memories-synology.test.ts +++ b/server/tests/integration/memories-synology.test.ts @@ -1186,7 +1186,7 @@ describe('Synology SSRF blocked error handling', () => { // ── Passphrase persistence fixes ───────────────────────────────────────────── -import { getOrCreateTrekPhoto, deleteTrekPhotoIfOrphan } from '../../src/services/memories/photoResolverService'; +import { getOrCreateTrekPhoto, deleteTrekPhotoIfOrphan } from '../../src/nest/photos/photos.bridge'; import { decrypt_api_key } from '../../src/nest/common/crypto/apiKeyCrypto'; describe('trek_photos passphrase healing (SYNO-090)', () => { diff --git a/server/tests/unit/services/trekPhotoMedia.test.ts b/server/tests/unit/services/trekPhotoMedia.test.ts index d9452cb18..1ac6b187f 100644 --- a/server/tests/unit/services/trekPhotoMedia.test.ts +++ b/server/tests/unit/services/trekPhotoMedia.test.ts @@ -23,7 +23,7 @@ vi.mock('../../../src/config', () => ({ import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { createUser } from '../../helpers/factories'; -import { getOrCreateLocalTrekPhoto, getOrCreateTrekPhoto, resolveTrekPhoto } from '../../../src/services/memories/photoResolverService'; +import { getOrCreateLocalTrekPhoto, getOrCreateTrekPhoto, resolveTrekPhoto } from '../../../src/nest/photos/photos.bridge'; beforeAll(() => { createTables(testDb);