One photo provider interface instead of three switches

Dispatch to a photo backend was `switch (photo.provider)` in
photo-resolver.service.ts twice and, in journey-public.controller.ts, an
if/else that handed anything not named immich to Synology — where it 404'd
from inside that provider's id parser. Adding a third backend meant finding
all three.

PhotoProviderRegistry.get(id) replaces them, over adapters registered under
the PHOTO_PROVIDERS token in memories.module.ts. Adapters rather than
`implements` on the services themselves: ImmichService and SynologyService
are also the album and browse surfaces, and folding the interface into them
would drag their argument orders and return shapes along.

PhotoAssetRef is the part worth having. The two services took the same three
values in different orders — Immich as (userId, assetId, ownerId), Synology
as (userId, targetUserId, photoId) — so every dispatch site had to remember
which was which, and a transposition would have compiled and served another
user's photo. One place builds the ref, one place decrypts the Synology
share passphrase.

`local` stays an explicit branch at both dispatch sites: the bytes are on
this disk, there is no row for it in photo_providers, and its asset info
comes from the stored row rather than from a backend.

The rename memories/ -> media/ is deliberately not here. It is import churn
across 275 files, and coverage-thresholds.mjs buckets on one path segment,
so merging photos/ in would collapse two ratchet entries into one and keep
the looser of the two.
This commit is contained in:
Maurice
2026-08-10 16:08:36 +02:00
committed by Maurice
parent 2d0eca7938
commit fc9742357b
13 changed files with 505 additions and 93 deletions
+21
View File
@@ -124,6 +124,27 @@ tools, which never pass through an HTTP guard. In the five domains piloted for
answer 404 to anonymous callers, so the addon check must beat the 401.
- `common/demo-write.ts` — `isDemoWriteBlocked`, the demo-mode upload block. A
function and not a guard on purpose; see the gotcha below.
- `memories/photo-provider.ts` + `providers/` — the PhotoProvider seam (#584).
Dispatch to a photo backend was a `switch (photo.provider)` in three places
(`photo-resolver.service.ts` twice, `journey-public.controller.ts` once, where
anything that was not Immich got handed to Synology). It is
`PhotoProviderRegistry.get(id)` now, over adapters registered under the
`PHOTO_PROVIDERS` multi-provider token in `memories.module.ts` — adding a
backend is one registration.
- Adapters, not `implements` on the services: ImmichService and
SynologyService are also the album/browse surfaces, with their own argument
orders and return shapes. The mapping lives in the adapter.
- `PhotoAssetRef` exists because the two services took the same three values
in different orders — `(userId, assetId, ownerId)` against
`(userId, targetUserId, photoId)`. A transposition compiled cleanly and
served another user's photo.
- `local` is deliberately NOT a provider. It is storage on this disk, it has
no row in `photo_providers`, and its "asset info" comes from the DB row
rather than from a backend. Both dispatch sites branch on it explicitly.
- The rename `memories/` → `media/` was considered and dropped: it is 275
files of import churn, and `scripts/coverage-thresholds.mjs` buckets on one
path segment, so merging `photos/` into it would collapse two ratchet
entries into one and drop the stricter of the two.
- `geo/` — the ONE Nominatim client (#576). There used to be two: atlas
throttled to the published 1.1s and cached its answers, maps did neither, so
five interactive routes fired straight at the service while the atlas loops
@@ -67,14 +67,22 @@ export class JourneyPublicController {
}
const effectiveOwnerId = valid.ownerId || Number(ownerId);
if (provider === 'immich') {
await this.journey.streamImmichAsset(res, effectiveOwnerId, assetId, wantThumb, effectiveOwnerId);
} else {
try {
await this.journey.streamSynologyAsset(res, effectiveOwnerId, effectiveOwnerId, assetId, wantThumb);
} catch {
res.status(404).json({ error: 'Provider not supported' });
}
const streaming = this.journey.streamProviderAsset(
res,
provider,
{ userId: effectiveOwnerId, ownerId: effectiveOwnerId, assetId },
wantThumb,
);
if (!streaming) {
res.status(404).json({ error: 'Provider not supported' });
return;
}
try {
await streaming;
} catch {
// Kept: a provider that throws mid-parse answered 404 here before, and a
// share link that half-writes a response is worse than one that 404s.
res.status(404).json({ error: 'Provider not supported' });
}
}
}
+20 -7
View File
@@ -3,7 +3,7 @@ import { DatabaseService } from '../database/database.service';
import { JourneyDomainService } from './journey-domain.service';
import { JourneyShareService } from './journey-share.service';
import { ImmichService } from '../memories/immich.service';
import { SynologyService } from '../memories/synology.service';
import { PhotoProviderRegistry } from '../memories/photo-provider.registry';
import { PhotoResolverService } from '../memories/photo-resolver.service';
import { AddonsService } from '../addons/addons.service';
import { ADDON_IDS } from '../../addons';
@@ -21,7 +21,7 @@ export class JourneyService {
private readonly dbs: DatabaseService,
private readonly addons: AddonsService,
private readonly immich: ImmichService,
private readonly synology: SynologyService,
private readonly providers: PhotoProviderRegistry,
private readonly photoResolver: PhotoResolverService,
private readonly journey: JourneyDomainService,
private readonly share: JourneyShareService,
@@ -98,10 +98,23 @@ export class JourneyService {
validateShareTokenForPhoto(token: string, photoId: number) { return this.share.validateShareTokenForPhoto(token, photoId); }
validateShareTokenForAsset(token: string, assetId: string) { return this.share.validateShareTokenForAsset(token, assetId); }
streamPhoto(res: Response, ownerId: number, photoId: number, kind: 'thumbnail' | 'original') { return this.photoResolver.streamPhoto(res, ownerId, photoId, kind); }
streamImmichAsset(res: Response, userId: number, assetId: string, kind: 'thumbnail' | 'original', ownerId: number) { return this.immich.streamImmichAsset(res, userId, assetId, kind, ownerId); }
// Was a dynamic import(): the memories cluster had a real import cycle and
// this was the workaround. The cycle is gone, so it is a plain injection now.
streamSynologyAsset(res: Response, userId: number, ownerId: number, assetId: string, kind: 'thumbnail' | 'original') {
return this.synology.streamSynologyAsset(res, userId, ownerId, assetId, kind);
/**
* Stream a shared journey asset from whichever backend holds it.
*
* Replaced a pair of per-provider passthroughs (and, before those, a dynamic
* import() around a since-removed cycle). The public controller used to pick
* between them with `provider === 'immich' ? … : trySynology()`, so a third
* provider would have been handed to Synology and 404'd from inside its
* parser. An unregistered provider is now told apart from a failing one.
*/
streamProviderAsset(
res: Response,
provider: string,
ref: { userId: number; ownerId: number; assetId: string },
kind: 'thumbnail' | 'original',
): Promise<unknown> | null {
const backend = this.providers.get(provider);
if (!backend) return null;
return backend.streamAsset(res, ref, kind);
}
}
+18 -1
View File
@@ -14,6 +14,10 @@ import { AddonsModule } from '../addons/addons.module';
import { AuditModule } from '../audit/audit.module';
import { TrekPhotosModule } from '../photos/trek-photos.module';
import { RealtimeModule } from '../realtime/realtime.module';
import { PHOTO_PROVIDERS } from './photo-provider';
import { PhotoProviderRegistry } from './photo-provider.registry';
import { ImmichPhotoProvider } from './providers/immich.provider';
import { SynologyPhotoProvider } from './providers/synology.provider';
/**
* Memories (photo-providers) domain — mounted at /api/integrations/memories.
@@ -29,6 +33,11 @@ import { RealtimeModule } from '../realtime/realtime.module';
* PhotoResolverService and MemoriesAccessService are exported because the
* /api/photos surface and the journey domain resolve provider assets through
* them.
*
* PHOTO_PROVIDERS is the multi-provider array behind PhotoProviderRegistry
* (#584): adding a photo backend means adding an adapter to this one list, not
* finding every `switch (photo.provider)`. Registered here rather than in the
* adapters themselves so the set is readable in one place.
*/
@Module({
imports: [AddonsModule, AuditModule, TrekPhotosModule, RealtimeModule],
@@ -42,7 +51,15 @@ import { RealtimeModule } from '../realtime/realtime.module';
PhotoResolverService,
ThumbnailService,
TrekPhotoCacheService,
ImmichPhotoProvider,
SynologyPhotoProvider,
PhotoProviderRegistry,
{
provide: PHOTO_PROVIDERS,
useFactory: (immich: ImmichPhotoProvider, synology: SynologyPhotoProvider) => [immich, synology],
inject: [ImmichPhotoProvider, SynologyPhotoProvider],
},
],
exports: [MemoriesAccessService, PhotoResolverService, ImmichService, SynologyService],
exports: [MemoriesAccessService, PhotoResolverService, ImmichService, SynologyService, PhotoProviderRegistry],
})
export class MemoriesModule {}
@@ -0,0 +1,30 @@
import { Inject, Injectable } from '@nestjs/common';
import { PHOTO_PROVIDERS, type PhotoProvider } from './photo-provider';
/**
* Where the `switch (photo.provider)` went (#584).
*
* Adding a backend is registering an adapter under PHOTO_PROVIDERS in
* memories.module.ts — no dispatch site changes, because there is one.
* Whether a registered provider is ENABLED is a separate question, answered by
* the photo_providers table in UnifiedMemoriesService; this only knows what the
* server can talk to at all.
*/
@Injectable()
export class PhotoProviderRegistry {
private readonly byId: Map<string, PhotoProvider>;
constructor(@Inject(PHOTO_PROVIDERS) providers: readonly PhotoProvider[]) {
this.byId = new Map(providers.map((p) => [p.id, p]));
}
/** The provider for a stored photo's `provider` column, or undefined. */
get(id: string | null | undefined): PhotoProvider | undefined {
return id ? this.byId.get(id) : undefined;
}
/** Registered ids, for error messages and diagnostics. */
ids(): string[] {
return [...this.byId.keys()];
}
}
@@ -0,0 +1,71 @@
import type { Response } from 'express';
import type { AssetInfo, ServiceResult } from './memories.helpers';
/** What a caller can ask a provider for. */
export type PhotoAssetKind = 'thumbnail' | 'original';
/**
* One asset, and everything a provider needs to reach it.
*
* The two shipped providers took the same three numbers in different orders —
* Immich as `(userId, assetId, ownerId)`, Synology as `(userId, targetUserId,
* photoId)` — so every dispatch site had to remember which was which, and a
* transposition would have compiled cleanly and served the wrong user's photo.
*/
export interface PhotoAssetRef {
/** Who is asking. */
userId: number;
/** Whose credentials the provider is reached with — not always the caller. */
ownerId: number;
assetId: string;
/** Synology share links carry their own passphrase; other providers ignore it. */
passphrase?: string;
/** Lets a provider tell a video apart from an image without a second lookup. */
mediaType?: string | null;
/** Forwarded Range header, for seeking in a video. */
range?: string;
}
export interface PhotoBytes {
bytes: Buffer;
contentType: string;
}
export interface PhotoFetchError {
error: string;
status: number;
}
/**
* A photo backend TREK can serve assets from (#584).
*
* The dispatch used to be a `switch (photo.provider)` in three places, which is
* what made "add a provider" a search-and-edit task rather than a registration.
* `local` is deliberately NOT one of these: it is storage on this disk, not an
* integration, and the `photo_providers` table it would have to appear in does
* not list it either.
*/
export interface PhotoProvider {
/** Matches `trek_photos.provider` and the `photo_providers` row id. */
readonly id: string;
/**
* Write the asset to the response, headers and status included.
*
* The error return is Immich's: it answers `{ error, status }` without
* touching the response when the owner has no credentials stored. Every
* caller has always ignored that — which leaves the request open until the
* client times out — so the shape is carried here rather than quietly
* dropped, and the pre-existing handling is left alone.
*/
streamAsset(res: Response, ref: PhotoAssetRef, kind: PhotoAssetKind): Promise<PhotoFetchError | void>;
/** Thumbnail bytes for the cache, rather than straight to a response. */
fetchThumbnailBytes(ref: PhotoAssetRef): Promise<PhotoBytes | PhotoFetchError>;
/** Capture date, dimensions, place — whatever the backend knows. */
getAssetInfo(ref: PhotoAssetRef): Promise<ServiceResult<AssetInfo>>;
}
/** Multi-provider injection token. Register an adapter here and it is reachable. */
export const PHOTO_PROVIDERS = Symbol('PHOTO_PROVIDERS');
@@ -6,25 +6,29 @@ import type { TrekPhoto } from '../../types';
import { decrypt_api_key } from '../common/crypto/apiKeyCrypto';
import { TrekPhotosRepository } from '../photos/trek-photos.repository';
import { UPLOADS_ROOT } from './uploads-root';
import { ImmichService } from './immich.service';
import { SynologyService } from './synology.service';
import { ThumbnailService } from './thumbnail.service';
import { TrekPhotoCacheService } from './trek-photo-cache.service';
import { fail, success, type AssetInfo, type ServiceResult } from './memories.helpers';
import { PhotoProviderRegistry } from './photo-provider.registry';
import type { PhotoAssetRef } from './photo-provider';
/**
* Resolves a stored trek_photo to bytes or metadata by asking whichever provider
* owns it. The storage half of the old photoResolverService lives in
* nest/photos/trek-photos.repository.ts; this is the dispatch half.
*
* It no longer knows WHICH providers exist (#584). It held ImmichService and
* SynologyService and a `switch` over their ids; it holds the registry now, so
* a third backend is a registration in memories.module.ts rather than a case
* added in two places here and a third in journey-public.controller.ts.
*/
@Injectable()
export class PhotoResolverService {
constructor(
private readonly photos: TrekPhotosRepository,
private readonly immich: ImmichService,
private readonly synology: SynologyService,
private readonly thumbnails: ThumbnailService,
private readonly cache: TrekPhotoCacheService,
private readonly providers: PhotoProviderRegistry,
) {}
// ── Streaming ────────────────────────────────────────────────────────────
@@ -114,39 +118,45 @@ export class PhotoResolverService {
}
}
switch (photo.provider) {
case 'local': {
res.status(404).json({ error: 'File not found' });
return;
}
case 'immich': {
if (kind === 'thumbnail') {
await this.streamCachedThumbnail(
res, photo,
() => this.immich.fetchImmichThumbnailBytes(userId, photo.asset_id!, photo.owner_id!),
() => this.immich.streamImmichAsset(res, userId, photo.asset_id!, kind, photo.owner_id!, { mediaType: photo.media_type }),
);
return;
}
await this.immich.streamImmichAsset(res, userId, photo.asset_id!, kind, photo.owner_id!, { mediaType: photo.media_type, range });
return;
}
case 'synologyphotos': {
const passphrase = photo.passphrase ? (decrypt_api_key(photo.passphrase) || undefined) : undefined;
if (kind === 'thumbnail') {
await this.streamCachedThumbnail(
res, photo,
() => this.synology.fetchSynologyThumbnailBytes(userId, photo.owner_id!, photo.asset_id!, passphrase),
() => this.synology.streamSynologyAsset(res, userId, photo.owner_id!, photo.asset_id!, kind, undefined, passphrase),
);
return;
}
await this.synology.streamSynologyAsset(res, userId, photo.owner_id!, photo.asset_id!, kind, undefined, passphrase);
return;
}
default:
res.status(400).json({ error: `Unknown provider: ${photo.provider}` });
// 'local' is not a provider: the bytes are on this disk, the block above
// already tried to send them, and there is no backend left to ask.
if (photo.provider === 'local') {
res.status(404).json({ error: 'File not found' });
return;
}
const provider = this.providers.get(photo.provider);
if (!provider) {
res.status(400).json({ error: `Unknown provider: ${photo.provider}` });
return;
}
const ref = this.refFor(photo, userId);
if (kind === 'thumbnail') {
await this.streamCachedThumbnail(
res, photo,
() => provider.fetchThumbnailBytes(ref),
// The fallback streams the thumbnail itself, so it carries no Range.
() => provider.streamAsset(res, ref, kind),
);
return;
}
await provider.streamAsset(res, { ...ref, range }, kind);
}
/**
* The stored row as a provider-shaped request. One place decrypts the
* Synology share passphrase, and one place decides which of the two user ids
* means "whose credentials".
*/
private refFor(photo: TrekPhoto, userId: number): PhotoAssetRef {
return {
userId,
ownerId: photo.owner_id!,
assetId: photo.asset_id!,
passphrase: photo.passphrase ? (decrypt_api_key(photo.passphrase) || undefined) : undefined,
mediaType: photo.media_type,
};
}
// ── Asset Info ────────────────────────────────────────────────────────────
@@ -158,29 +168,21 @@ export class PhotoResolverService {
const photo = this.photos.resolve(photoId);
if (!photo) return fail('Photo not found', 404);
switch (photo.provider) {
case 'local': {
return success({
id: String(photo.id),
takenAt: photo.created_at,
city: null,
country: null,
width: photo.width,
height: photo.height,
fileName: photo.file_path?.split('/').pop() || null,
} as AssetInfo);
}
case 'immich': {
const result = await this.immich.getAssetInfo(userId, photo.asset_id!, photo.owner_id!);
if (result.error) return fail(result.error, result.status || 500);
return success(result.data as AssetInfo);
}
case 'synologyphotos': {
const passphrase = photo.passphrase ? (decrypt_api_key(photo.passphrase) || undefined) : undefined;
return this.synology.getSynologyAssetInfo(userId, photo.asset_id!, photo.owner_id!, passphrase);
}
default:
return fail(`Unknown provider: ${photo.provider}`, 400);
// Local rows answer from the row itself — nothing to ask.
if (photo.provider === 'local') {
return success({
id: String(photo.id),
takenAt: photo.created_at,
city: null,
country: null,
width: photo.width,
height: photo.height,
fileName: photo.file_path?.split('/').pop() || null,
} as AssetInfo);
}
const provider = this.providers.get(photo.provider);
if (!provider) return fail(`Unknown provider: ${photo.provider}`, 400);
return provider.getAssetInfo(this.refFor(photo, userId));
}
}
@@ -0,0 +1,42 @@
import { Injectable } from '@nestjs/common';
import type { Response } from 'express';
import { ImmichService } from '../immich.service';
import { fail, success, type AssetInfo, type ServiceResult } from '../memories.helpers';
import type {
PhotoAssetKind, PhotoAssetRef, PhotoBytes, PhotoFetchError, PhotoProvider,
} from '../photo-provider';
/**
* Immich behind the PhotoProvider interface.
*
* An adapter rather than `implements` on ImmichService itself: that service is
* also the album/browse surface the controllers use, with its own argument
* order and its own `{ data?, error?, status? }` return shape. Mapping here
* keeps the interface narrow and leaves the service's callers untouched.
*/
@Injectable()
export class ImmichPhotoProvider implements PhotoProvider {
readonly id = 'immich';
constructor(private readonly immich: ImmichService) {}
async streamAsset(res: Response, ref: PhotoAssetRef, kind: PhotoAssetKind): Promise<PhotoFetchError | void> {
const result = await this.immich.streamImmichAsset(res, ref.userId, ref.assetId, kind, ref.ownerId, {
mediaType: ref.mediaType,
range: ref.range,
});
// The service types both fields optional; the one path that returns at all
// always fills them. Normalised here so the interface can require them.
if (result && result.error) return { error: result.error, status: result.status ?? 500 };
}
fetchThumbnailBytes(ref: PhotoAssetRef): Promise<PhotoBytes | PhotoFetchError> {
return this.immich.fetchImmichThumbnailBytes(ref.userId, ref.assetId, ref.ownerId);
}
async getAssetInfo(ref: PhotoAssetRef): Promise<ServiceResult<AssetInfo>> {
const result = await this.immich.getAssetInfo(ref.userId, ref.assetId, ref.ownerId);
if (result.error) return fail(result.error, result.status || 500);
return success(result.data as AssetInfo);
}
}
@@ -0,0 +1,33 @@
import { Injectable } from '@nestjs/common';
import type { Response } from 'express';
import { SynologyService } from '../synology.service';
import type { AssetInfo, ServiceResult } from '../memories.helpers';
import type {
PhotoAssetKind, PhotoAssetRef, PhotoBytes, PhotoFetchError, PhotoProvider,
} from '../photo-provider';
/**
* Synology Photos behind the PhotoProvider interface.
*
* The `size` parameter stays unset, as every caller left it. The passphrase is
* the one thing this provider needs that Immich does not: a shared album link
* carries its own, and the caller decrypts it before handing it over.
*/
@Injectable()
export class SynologyPhotoProvider implements PhotoProvider {
readonly id = 'synologyphotos';
constructor(private readonly synology: SynologyService) {}
streamAsset(res: Response, ref: PhotoAssetRef, kind: PhotoAssetKind): Promise<void> {
return this.synology.streamSynologyAsset(res, ref.userId, ref.ownerId, ref.assetId, kind, undefined, ref.passphrase);
}
fetchThumbnailBytes(ref: PhotoAssetRef): Promise<PhotoBytes | PhotoFetchError> {
return this.synology.fetchSynologyThumbnailBytes(ref.userId, ref.ownerId, ref.assetId, ref.passphrase);
}
getAssetInfo(ref: PhotoAssetRef): Promise<ServiceResult<AssetInfo>> {
return this.synology.getSynologyAssetInfo(ref.userId, ref.assetId, ref.ownerId, ref.passphrase);
}
}
@@ -297,10 +297,12 @@ describe('JourneyPublicController', () => {
it('legacy photo proxy: 404 invalid token, immich path streams', async () => {
expect(await thrownAsync(() => new JourneyPublicController(svc({ validateShareTokenForAsset: vi.fn().mockReturnValue(null) } as Partial<JourneyService>)).legacyPhoto('tok', 'immich', 'a1', '2', 'thumbnail', {} as Response))).toEqual({ status: 404, body: { error: 'Not found' } });
const streamImmichAsset = vi.fn().mockResolvedValue(undefined);
const s = svc({ validateShareTokenForAsset: vi.fn().mockReturnValue({ ownerId: 5 }), streamImmichAsset } as Partial<JourneyService>);
// One call for every provider now, with the ids in a ref instead of in a
// per-provider argument order.
const streamProviderAsset = vi.fn().mockResolvedValue(undefined);
const s = svc({ validateShareTokenForAsset: vi.fn().mockReturnValue({ ownerId: 5 }), streamProviderAsset } as Partial<JourneyService>);
await new JourneyPublicController(s).legacyPhoto('tok', 'immich', 'a1', '2', 'original', {} as Response);
expect(streamImmichAsset).toHaveBeenCalledWith({}, 5, 'a1', 'original', 5);
expect(streamProviderAsset).toHaveBeenCalledWith({}, 'immich', { userId: 5, ownerId: 5, assetId: 'a1' }, 'original');
});
it('photo proxy streams thumbnails too', async () => {
@@ -311,25 +313,39 @@ describe('JourneyPublicController', () => {
});
it('legacy photo proxy: synology streams, and a failure becomes a 404 json', async () => {
const streamSynologyAsset = vi.fn().mockResolvedValue(undefined);
const s = svc({ validateShareTokenForAsset: vi.fn().mockReturnValue({ ownerId: 5 }), streamSynologyAsset } as Partial<JourneyService>);
await new JourneyPublicController(s).legacyPhoto('tok', 'synology', 'a1', '2', 'thumbnail', {} as Response);
expect(streamSynologyAsset).toHaveBeenCalledWith({}, 5, 5, 'a1', 'thumbnail');
const streamProviderAsset = vi.fn().mockResolvedValue(undefined);
const s = svc({ validateShareTokenForAsset: vi.fn().mockReturnValue({ ownerId: 5 }), streamProviderAsset } as Partial<JourneyService>);
await new JourneyPublicController(s).legacyPhoto('tok', 'synologyphotos', 'a1', '2', 'thumbnail', {} as Response);
expect(streamProviderAsset).toHaveBeenCalledWith({}, 'synologyphotos', { userId: 5, ownerId: 5, assetId: 'a1' }, 'thumbnail');
const status = vi.fn().mockReturnThis();
const json = vi.fn();
const res = { status, json } as unknown as Response;
const failing = svc({ validateShareTokenForAsset: vi.fn().mockReturnValue({ ownerId: 0 }), streamSynologyAsset: vi.fn().mockRejectedValue(new Error('no synology')) } as Partial<JourneyService>);
await new JourneyPublicController(failing).legacyPhoto('tok', 'synology', 'a1', '6', 'original', res);
const failing = svc({ validateShareTokenForAsset: vi.fn().mockReturnValue({ ownerId: 0 }), streamProviderAsset: vi.fn().mockRejectedValue(new Error('no synology')) } as Partial<JourneyService>);
await new JourneyPublicController(failing).legacyPhoto('tok', 'synologyphotos', 'a1', '6', 'original', res);
expect(status).toHaveBeenCalledWith(404);
expect(json).toHaveBeenCalledWith({ error: 'Provider not supported' });
});
it('legacy photo proxy: an unregistered provider 404s instead of being handed to synology', async () => {
// The old if/else sent everything that was not immich to synology, which
// 404'd from inside its own id parser. The registry can tell "no such
// backend" apart from "that backend failed".
const streamProviderAsset = vi.fn().mockReturnValue(null);
const status = vi.fn().mockReturnThis();
const json = vi.fn();
const res = { status, json } as unknown as Response;
const s = svc({ validateShareTokenForAsset: vi.fn().mockReturnValue({ ownerId: 5 }), streamProviderAsset } as Partial<JourneyService>);
await new JourneyPublicController(s).legacyPhoto('tok', 'photoprism', 'a1', '2', 'original', res);
expect(status).toHaveBeenCalledWith(404);
expect(json).toHaveBeenCalledWith({ error: 'Provider not supported' });
});
it('legacy photo proxy: falls back to the path ownerId when the token has none', async () => {
const streamImmichAsset = vi.fn().mockResolvedValue(undefined);
const s = svc({ validateShareTokenForAsset: vi.fn().mockReturnValue({ ownerId: 0 }), streamImmichAsset } as Partial<JourneyService>);
const streamProviderAsset = vi.fn().mockResolvedValue(undefined);
const s = svc({ validateShareTokenForAsset: vi.fn().mockReturnValue({ ownerId: 0 }), streamProviderAsset } as Partial<JourneyService>);
await new JourneyPublicController(s).legacyPhoto('tok', 'immich', 'a1', '8', 'original', {} as Response);
expect(streamImmichAsset).toHaveBeenCalledWith({}, 8, 'a1', 'original', 8);
expect(streamProviderAsset).toHaveBeenCalledWith({}, 'immich', { userId: 8, ownerId: 8, assetId: 'a1' }, 'original');
});
it('legacy photo proxy: local provider 404s when the resolved file does not exist', async () => {
@@ -0,0 +1,148 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
/**
* The PhotoProvider seam (#584).
*
* What is worth pinning here is not that a lookup works — it is the two things
* the old `switch (photo.provider)` could not express. First, that a backend
* arrives by registration rather than by editing three dispatch sites. Second,
* that the adapters put the caller and the credential owner in the right slots:
* Immich took `(userId, assetId, ownerId)` and Synology `(userId, targetUserId,
* photoId)`, so a transposition would have compiled and served another user's
* photo.
*/
import { PhotoProviderRegistry } from '../../../src/nest/memories/photo-provider.registry';
import { ImmichPhotoProvider } from '../../../src/nest/memories/providers/immich.provider';
import { SynologyPhotoProvider } from '../../../src/nest/memories/providers/synology.provider';
import type { ImmichService } from '../../../src/nest/memories/immich.service';
import type { SynologyService } from '../../../src/nest/memories/synology.service';
import type { PhotoAssetRef, PhotoProvider } from '../../../src/nest/memories/photo-provider';
import type { Response } from 'express';
const immich = {
streamImmichAsset: vi.fn(),
fetchImmichThumbnailBytes: vi.fn(),
getAssetInfo: vi.fn(),
};
const synology = {
streamSynologyAsset: vi.fn(),
fetchSynologyThumbnailBytes: vi.fn(),
getSynologyAssetInfo: vi.fn(),
};
const immichProvider = new ImmichPhotoProvider(immich as unknown as ImmichService);
const synologyProvider = new SynologyPhotoProvider(synology as unknown as SynologyService);
// Caller and credential owner deliberately differ: a shared album is reached
// with the OWNER's credentials on behalf of the viewer.
const ref: PhotoAssetRef = {
userId: 7,
ownerId: 42,
assetId: 'asset-1',
passphrase: 'share-pw',
mediaType: 'image',
range: 'bytes=0-99',
};
const res = {} as Response;
beforeEach(() => {
vi.clearAllMocks();
immich.streamImmichAsset.mockResolvedValue(undefined);
synology.streamSynologyAsset.mockResolvedValue(undefined);
});
describe('PhotoProviderRegistry', () => {
it('PROV-001: resolves a stored photo\'s provider column', () => {
const registry = new PhotoProviderRegistry([immichProvider, synologyProvider]);
expect(registry.get('immich')).toBe(immichProvider);
expect(registry.get('synologyphotos')).toBe(synologyProvider);
expect(registry.ids().sort()).toEqual(['immich', 'synologyphotos']);
});
it('PROV-002: answers undefined for local, null and an unknown id', () => {
const registry = new PhotoProviderRegistry([immichProvider, synologyProvider]);
// 'local' is storage, not a backend, and never gets a row in photo_providers.
expect(registry.get('local')).toBeUndefined();
expect(registry.get(null)).toBeUndefined();
expect(registry.get(undefined)).toBeUndefined();
expect(registry.get('photoprism')).toBeUndefined();
});
it('PROV-003: a new backend arrives by registration, with no dispatch site touched', () => {
const photoprism: PhotoProvider = {
id: 'photoprism',
streamAsset: vi.fn(),
fetchThumbnailBytes: vi.fn(),
getAssetInfo: vi.fn(),
};
const registry = new PhotoProviderRegistry([immichProvider, synologyProvider, photoprism]);
expect(registry.get('photoprism')).toBe(photoprism);
});
});
describe('ImmichPhotoProvider', () => {
it('PROV-004: maps the ref onto the service\'s (userId, assetId, ownerId) order', async () => {
await immichProvider.streamAsset(res, ref, 'original');
expect(immich.streamImmichAsset).toHaveBeenCalledWith(res, 7, 'asset-1', 'original', 42, {
mediaType: 'image',
range: 'bytes=0-99',
});
await immichProvider.fetchThumbnailBytes(ref);
expect(immich.fetchImmichThumbnailBytes).toHaveBeenCalledWith(7, 'asset-1', 42);
});
it('PROV-005: normalises the credential-less answer, which the service types as optional', async () => {
immich.streamImmichAsset.mockResolvedValue({ error: 'Not found', status: 404 });
expect(await immichProvider.streamAsset(res, ref, 'original')).toEqual({ error: 'Not found', status: 404 });
immich.streamImmichAsset.mockResolvedValue({ error: 'Broken' });
expect(await immichProvider.streamAsset(res, ref, 'original')).toEqual({ error: 'Broken', status: 500 });
});
it('PROV-006: a written response reports nothing', async () => {
immich.streamImmichAsset.mockResolvedValue(undefined);
expect(await immichProvider.streamAsset(res, ref, 'thumbnail')).toBeUndefined();
});
it('PROV-007: turns the service\'s {data,error} into a ServiceResult', async () => {
immich.getAssetInfo.mockResolvedValue({ data: { id: 'a' } });
expect(await immichProvider.getAssetInfo(ref)).toEqual({ success: true, data: { id: 'a' } });
immich.getAssetInfo.mockResolvedValue({ error: 'gone', status: 404 });
expect(await immichProvider.getAssetInfo(ref)).toEqual({
success: false,
error: { message: 'gone', status: 404 },
});
// No status from the service is a server error, not a silent success.
immich.getAssetInfo.mockResolvedValue({ error: 'gone' });
expect(await immichProvider.getAssetInfo(ref)).toEqual({
success: false,
error: { message: 'gone', status: 500 },
});
});
});
describe('SynologyPhotoProvider', () => {
it('PROV-008: maps the ref onto the service\'s (userId, ownerId, assetId) order', async () => {
await synologyProvider.streamAsset(res, ref, 'thumbnail');
// size stays undefined, as every caller left it.
expect(synology.streamSynologyAsset).toHaveBeenCalledWith(res, 7, 42, 'asset-1', 'thumbnail', undefined, 'share-pw');
await synologyProvider.fetchThumbnailBytes(ref);
expect(synology.fetchSynologyThumbnailBytes).toHaveBeenCalledWith(7, 42, 'asset-1', 'share-pw');
});
it('PROV-009: passes the share passphrase to getAssetInfo, in its own argument order', async () => {
synology.getSynologyAssetInfo.mockResolvedValue({ data: { id: 'a' } });
await synologyProvider.getAssetInfo(ref);
expect(synology.getSynologyAssetInfo).toHaveBeenCalledWith(7, 'asset-1', 42, 'share-pw');
});
it('PROV-010: forwards an absent passphrase as undefined rather than an empty string', async () => {
await synologyProvider.streamAsset(res, { userId: 1, ownerId: 1, assetId: 'x' }, 'original');
expect(synology.streamSynologyAsset).toHaveBeenCalledWith(res, 1, 1, 'x', 'original', undefined, undefined);
});
});
@@ -40,6 +40,9 @@ import type { ImmichService } from '../../../src/nest/memories/immich.service';
import type { SynologyService } from '../../../src/nest/memories/synology.service';
import type { ThumbnailService } from '../../../src/nest/memories/thumbnail.service';
import type { TrekPhotoCacheService } from '../../../src/nest/memories/trek-photo-cache.service';
import { PhotoProviderRegistry } from '../../../src/nest/memories/photo-provider.registry';
import { ImmichPhotoProvider } from '../../../src/nest/memories/providers/immich.provider';
import { SynologyPhotoProvider } from '../../../src/nest/memories/providers/synology.provider';
const immich = {
streamImmichAsset: vi.fn(),
@@ -63,12 +66,20 @@ const cache = {
const dbs = new DatabaseService(testDb);
const repo = new TrekPhotosRepository(dbs);
// Real adapters over the stubbed services, and a real registry: the cases below
// keep asserting on immich.streamImmichAsset/synology.streamSynologyAsset, so
// they now also pin the adapters' argument mapping — which is where the two
// providers' differing parameter orders used to be a transposition waiting to
// happen.
const providers = new PhotoProviderRegistry([
new ImmichPhotoProvider(immich as unknown as ImmichService),
new SynologyPhotoProvider(synology as unknown as SynologyService),
]);
const svc = new PhotoResolverService(
repo,
immich as unknown as ImmichService,
synology as unknown as SynologyService,
thumbnails as unknown as ThumbnailService,
cache as unknown as TrekPhotoCacheService,
providers,
);
function makeRes() {
+4 -4
View File
@@ -62,7 +62,7 @@ export default defineConfig({
// only covers what sits directly under src/nest (app.module.ts and the like).
thresholds: {
'src/nest/accommodations/**/*.ts': { statements: 92, branches: 87, functions: 99, lines: 97 },
'src/nest/addons/**/*.ts': { statements: 99, branches: 87, functions: 99, lines: 99 },
'src/nest/addons/**/*.ts': { statements: 99, branches: 89, functions: 99, lines: 99 },
'src/nest/admin/**/*.ts': { statements: 86, branches: 70, functions: 89, lines: 88 },
'src/nest/airports/**/*.ts': { statements: 68, branches: 60, functions: 93, lines: 73 },
'src/nest/app-config/**/*.ts': { statements: 79, branches: 100, functions: 65, lines: 71 },
@@ -88,10 +88,10 @@ export default defineConfig({
'src/nest/health/**/*.ts': { statements: 99, branches: 65, functions: 99, lines: 99 },
'src/nest/help/**/*.ts': { statements: 81, branches: 70, functions: 99, lines: 86 },
'src/nest/integrations/**/*.ts': { statements: 72, branches: 56, functions: 76, lines: 74 },
'src/nest/journey/**/*.ts': { statements: 90, branches: 84, functions: 83, lines: 91 },
'src/nest/journey/**/*.ts': { statements: 90, branches: 84, functions: 84, lines: 91 },
'src/nest/llm-parse/**/*.ts': { statements: 91, branches: 85, functions: 85, lines: 94 },
'src/nest/maps/**/*.ts': { statements: 93, branches: 86, functions: 97, lines: 96 },
'src/nest/memories/**/*.ts': { statements: 88, branches: 80, functions: 95, lines: 90 },
'src/nest/memories/**/*.ts': { statements: 88, branches: 81, functions: 96, lines: 90 },
'src/nest/notifications/**/*.ts': { statements: 83, branches: 72, functions: 84, lines: 86 },
'src/nest/oauth/**/*.ts': { statements: 96, branches: 95, functions: 97, lines: 97 },
'src/nest/oidc/**/*.ts': { statements: 88, branches: 83, functions: 91, lines: 92 },
@@ -109,7 +109,7 @@ export default defineConfig({
'src/nest/query-helpers/**/*.ts': { statements: 90, branches: 75, functions: 99, lines: 92 },
'src/nest/realtime/**/*.ts': { statements: 99, branches: 100, functions: 99, lines: 99 },
'src/nest/reservation-import/**/*.ts': { statements: 61, branches: 55, functions: 41, lines: 61 },
'src/nest/reservations/**/*.ts': { statements: 92, branches: 83, functions: 93, lines: 95 },
'src/nest/reservations/**/*.ts': { statements: 92, branches: 83, functions: 95, lines: 96 },
'src/nest/settings/**/*.ts': { statements: 85, branches: 68, functions: 99, lines: 87 },
'src/nest/share/**/*.ts': { statements: 97, branches: 87, functions: 99, lines: 99 },
'src/nest/system-notices/**/*.ts': { statements: 99, branches: 99, functions: 99, lines: 99 },