refactor(memories): immich upload reads through storage; drop uploads-root

This commit is contained in:
jubnl
2026-08-17 06:15:01 +02:00
parent 364f376b07
commit 939c2ec3e8
5 changed files with 66 additions and 42 deletions
+11 -7
View File
@@ -3,10 +3,12 @@ import type { Response } from 'express';
import { maybe_encrypt_api_key, decrypt_api_key } from '../common/crypto/apiKeyCrypto';
import { checkSsrf, safeFetch } from '../../utils/ssrfGuard';
import { AuditService } from '../audit/audit.service';
import { StorageService } from '../storage/storage.service';
import fsPromises from 'node:fs/promises';
import path from 'node:path';
import { DatabaseService } from '../database/database.service';
import { MemoriesAccessService } from './memories-access.service';
import { fail, handleServiceResult, pipeAsset, type Selection } from './memories.helpers';
import { UPLOADS_ROOT } from './uploads-root';
const ALBUM_PAGE_SIZE = 1000;
const ALBUM_MAX_PAGES = 20;
@@ -24,6 +26,7 @@ export class ImmichService {
private readonly db: DatabaseService,
private readonly audit: AuditService,
private readonly access: MemoriesAccessService,
private readonly storage: StorageService,
) {}
getImmichCredentials(userId: number) {
@@ -570,14 +573,15 @@ export class ImmichService {
const creds = this.getImmichCredentials(userId);
if (!creds) return null;
const fs = await import('node:fs');
const path = await import('node:path');
const fullPath = path.join(UPLOADS_ROOT, filePath);
if (!fs.existsSync(fullPath)) return null;
// Journey uploads store the uploads-relative 'journey/<file>' path; only
// those are local storage objects this can push.
const name = filePath.startsWith('journey/') ? filePath.slice('journey/'.length) : null;
if (!name) return null;
try {
const fileBuffer = fs.readFileSync(fullPath);
// A missing object throws StorageNotFoundError into the catch below —
// the same null the old existsSync guard answered.
const fileBuffer = await this.storage.withLocalFile('journey', name, (p) => fsPromises.readFile(p));
const boundary = '----ImmichUpload' + Date.now();
const ext = path.extname(fileName).toLowerCase();
const mimeTypes: Record<string, string> = {
-14
View File
@@ -1,14 +0,0 @@
import path from 'node:path';
/**
* `<server>/uploads`, the one anchor for everything the app stores on disk.
*
* `__dirname` moves with the file, and three separate copies of this join used
* to live in services/memories/ — every one of them a silent breakage waiting
* for the next move (multer would have created the wrong directory without a
* word). One constant, one test (UPLOADS-ROOT-001), and the depth is stated
* rather than counted by hand: `server/{src,dist}/nest/memories` sits three
* levels under `server/`, the same depth `services/memories/` sat at — so the
* resolved path is unchanged by the move.
*/
export const UPLOADS_ROOT = path.resolve(__dirname, '..', '..', '..', 'uploads');
+51 -1
View File
@@ -46,11 +46,15 @@ import { DatabaseService } from '../../../src/nest/database/database.service';
import { ImmichService } from '../../../src/nest/memories/immich.service';
import type { AuditService } from '../../../src/nest/audit/audit.service';
import type { MemoriesAccessService } from '../../../src/nest/memories/memories-access.service';
import fs from 'node:fs';
import path from 'node:path';
import { makeStorageFixture } from '../../helpers/storage-fixture';
const audit = { writeAudit: vi.fn() };
const access = { getAlbumIdFromLink: vi.fn() };
const dbs = new DatabaseService(testDb);
const svc = new ImmichService(dbs, audit as unknown as AuditService, access as unknown as MemoriesAccessService);
const journeyFx = makeStorageFixture('journey/');
const svc = new ImmichService(dbs, audit as unknown as AuditService, access as unknown as MemoriesAccessService, journeyFx.storage);
const USER = 1;
@@ -478,3 +482,49 @@ describe('collectAlbumSelection', () => {
expect(await svc.collectAlbumSelection('1', 'l1', USER)).toEqual({ error: 'Could not reach Immich', status: 502 });
});
});
describe('uploadToImmich', () => {
function writeJourneyObject(name: string, bytes: string): void {
const dir = path.join(journeyFx.root, 'journey');
fs.mkdirSync(dir, { recursive: true });
fs.writeFileSync(path.join(dir, name), bytes);
}
it('IMMICH-UP-001: returns null without credentials and never fetches', async () => {
expect(await svc.uploadToImmich(999, 'journey/pic.jpg', 'pic.jpg')).toBeNull();
expect(safeFetch).not.toHaveBeenCalled();
});
it('IMMICH-UP-002: returns null for a path outside the journey category', async () => {
seedUser(USER, 'https://immich.test', 'key-1');
expect(await svc.uploadToImmich(USER, 'files/doc.pdf', 'doc.pdf')).toBeNull();
expect(safeFetch).not.toHaveBeenCalled();
});
it('IMMICH-UP-003: returns null when the object is missing (old existsSync guard)', async () => {
seedUser(USER, 'https://immich.test', 'key-1');
expect(await svc.uploadToImmich(USER, 'journey/gone.jpg', 'gone.jpg')).toBeNull();
expect(safeFetch).not.toHaveBeenCalled();
});
it('IMMICH-UP-004: posts the bytes as multipart and answers the created asset id', async () => {
seedUser(USER, 'https://immich.test', 'key-1');
writeJourneyObject('up.jpg', 'jpeg-bytes-here');
safeFetch.mockResolvedValueOnce({ ok: true, json: async () => ({ id: 'immich-42' }) });
expect(await svc.uploadToImmich(USER, 'journey/up.jpg', 'up.jpg')).toBe('immich-42');
const [url, init] = safeFetch.mock.calls[0] as [string, { method: string; body: Buffer; headers: Record<string, string> }];
expect(url).toBe('https://immich.test/api/assets');
expect(init.method).toBe('POST');
expect(init.headers['x-api-key']).toBe('key-1');
expect(init.body.includes(Buffer.from('jpeg-bytes-here'))).toBe(true);
});
it('IMMICH-UP-005: a rejected upstream answers null, not a throw', async () => {
seedUser(USER, 'https://immich.test', 'key-1');
writeJourneyObject('rej.jpg', 'bytes');
safeFetch.mockResolvedValueOnce({ ok: false, status: 500 });
expect(await svc.uploadToImmich(USER, 'journey/rej.jpg', 'rej.jpg')).toBeNull();
});
});
@@ -72,9 +72,10 @@ describe('storage keys', () => {
});
});
// Depth contract for the storage domain's one __dirname anchor, same guard as
// tests/unit/nest/uploads-root.test.ts: the resolved defaults must equal the
// paths the server has always used, under both src (vitest) and dist layouts.
// Depth contract for the storage domain's one __dirname anchor (the guard the
// deleted memories/uploads-root.ts pin used to carry): the resolved defaults
// must equal the paths the server has always used, under both src (vitest) and
// dist layouts.
describe('storage default paths', () => {
it('anchors the default uploads root at <server>/uploads', () => {
expect(DEFAULT_UPLOADS_ROOT).toBe(path.resolve(process.cwd(), 'uploads'));
@@ -1,17 +0,0 @@
import { describe, it, expect } from 'vitest';
import path from 'node:path';
import { UPLOADS_ROOT } from '../../../src/nest/memories/uploads-root';
/**
* The anchor three separate `__dirname` joins used to duplicate. A wrong depth
* does not throw — multer and mkdirSync would happily create the wrong tree and
* every stored photo would land somewhere nobody serves from.
*/
describe('UPLOADS_ROOT', () => {
it('UPLOADS-ROOT-001: resolves to <server>/uploads, whatever the process cwd is', () => {
// tests run from server/, so the repo-relative answer is unambiguous
expect(UPLOADS_ROOT).toBe(path.resolve(process.cwd(), 'uploads'));
expect(path.basename(UPLOADS_ROOT)).toBe('uploads');
expect(path.basename(path.dirname(UPLOADS_ROOT))).toBe('server');
});
});