From 8350d04a631a0bd47070ceae94f47b1d33fea964 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 10 Oct 2026 13:26:38 +0200 Subject: [PATCH] refactor(db): the Immich, Synology and AirTrail columns of users get repositories of their own UserImmichRepository, UserSynologyRepository and UserAirtrailRepository hold the eighteen connection statements UsersRepository carried, on the same users table and with their statements unchanged, provided through a small module as factories over the EntityManager. ImmichService, SynologyService, AirtrailService and JourneyService inject them instead of the account repository. UserRow drops the dead immich_access_token column (the table keeps it), and a shape test pins the account repository's size. --- server/scripts/size-baseline.json | 2 +- .../repositories/UserAirtrail.repository.ts | 89 ++++++ .../db/repositories/UserImmich.repository.ts | 104 +++++++ .../repositories/UserSynology.repository.ts | 111 +++++++ .../src/db/repositories/Users.repository.ts | 270 +----------------- .../user-connection-repositories.module.ts | 23 ++ .../nest/integrations/airtrail-core.module.ts | 2 + .../src/nest/integrations/airtrail.service.ts | 18 +- server/src/nest/journey/journey.module.ts | 5 +- server/src/nest/journey/journey.service.ts | 10 +- server/src/nest/memories/immich.service.ts | 16 +- server/src/nest/memories/memories.module.ts | 2 + server/src/nest/memories/synology.service.ts | 22 +- server/tests/helpers/mcp-test-controllers.ts | 11 +- server/tests/helpers/test-uow.ts | 18 ++ .../UserConnections.repository.test.ts | 216 ++++++++++++++ .../db/repositories/Users.repository.test.ts | 99 +------ .../unit/db/users-repository-shape.test.ts | 39 +++ .../tests/unit/nest/airtrail.service.test.ts | 3 +- server/tests/unit/nest/immich.service.test.ts | 6 +- .../tests/unit/nest/synology.service.test.ts | 5 +- .../services/airtrailTestConnection.test.ts | 3 +- .../unit/services/airtrailWriteback.test.ts | 15 +- 23 files changed, 673 insertions(+), 416 deletions(-) create mode 100644 server/src/db/repositories/UserAirtrail.repository.ts create mode 100644 server/src/db/repositories/UserImmich.repository.ts create mode 100644 server/src/db/repositories/UserSynology.repository.ts create mode 100644 server/src/nest/database/user-connection-repositories.module.ts create mode 100644 server/tests/unit/db/repositories/UserConnections.repository.test.ts create mode 100644 server/tests/unit/db/users-repository-shape.test.ts diff --git a/server/scripts/size-baseline.json b/server/scripts/size-baseline.json index eba5a97ea..ece3cf48a 100644 --- a/server/scripts/size-baseline.json +++ b/server/scripts/size-baseline.json @@ -4,7 +4,7 @@ "src/db/repositories/Places.repository.ts": 1440, "src/db/repositories/Reservations.repository.ts": 1675, "src/db/repositories/Trips.repository.ts": 1121, - "src/db/repositories/Users.repository.ts": 1710, + "src/db/repositories/Users.repository.ts": 1446, "src/nest/atlas/atlas-geo.ts": 1005, "src/nest/atlas/atlas.service.ts": 1182, "src/nest/auth/auth.service.ts": 1156, diff --git a/server/src/db/repositories/UserAirtrail.repository.ts b/server/src/db/repositories/UserAirtrail.repository.ts new file mode 100644 index 000000000..d878ee6f8 --- /dev/null +++ b/server/src/db/repositories/UserAirtrail.repository.ts @@ -0,0 +1,89 @@ +import type { EntityManager } from '@mikro-orm/core'; +import type { SqlEntityManager } from '@mikro-orm/sql'; +import { Users } from '../entities/Users.entity'; +import { TrekRepository } from './_shared/trek-repository'; + +/** + * The AirTrail connection a user keeps on their own `users` row + * (`airtrail_url`, `airtrail_api_key`, `airtrail_allow_insecure_tls`, + * `airtrail_write_enabled`): the statements `AirtrailService` issues, apart + * from the rest of the account in `UsersRepository`. Same table, no move of + * the columns. + * + * The encrypt-in-service discipline of `DawarichConnectionsRepository`: every + * value below is exactly what the SERVICE hands this repository — encrypted, + * plaintext or null, at the same point in the pipeline the raw SQL saw it. + * This repository never calls maybe_encrypt_api_key/decrypt_api_key itself. + */ +export class UserAirtrailRepository extends TrekRepository { + /** Built over the context-resolving EntityManager, like the repositories MikroORM hands out per entity. */ + constructor(em: EntityManager) { + super(em as SqlEntityManager, Users); + } + + /** ATC1 (`airtrail.service.ts#readRow`) — `SELECT airtrail_url, airtrail_api_key, airtrail_allow_insecure_tls, airtrail_write_enabled FROM users WHERE id = ?`. */ + async getAirtrailConnRow(id: number): Promise<{ + airtrail_url: string | null; + airtrail_api_key: string | null; + airtrail_allow_insecure_tls: number | null; + airtrail_write_enabled: number | null; + } | null> { + const row = await this.findOne( + { id }, + { fields: ['airtrail_url', 'airtrail_api_key', 'airtrail_allow_insecure_tls', 'airtrail_write_enabled'] }, + ); + return row + ? { + airtrail_url: row.airtrail_url ?? null, + airtrail_api_key: row.airtrail_api_key ?? null, + airtrail_allow_insecure_tls: row.airtrail_allow_insecure_tls ?? null, + airtrail_write_enabled: row.airtrail_write_enabled ?? null, + } + : null; + } + + /** ATC2 (`airtrail.service.ts#isAirtrailWriteEnabled`) — `SELECT airtrail_write_enabled FROM users WHERE id = ?`. */ + async getAirtrailWriteEnabled(id: number): Promise { + const row = await this.findOne({ id }, { fields: ['airtrail_write_enabled'] }); + return row?.airtrail_write_enabled ?? null; + } + + /** + * ATC3 (`airtrail.service.ts#saveSettings`, newKey branch) — `UPDATE users + * SET airtrail_url = ?, airtrail_api_key = ?, airtrail_allow_insecure_tls + * = ?, airtrail_write_enabled = ? WHERE id = ?`. **No transaction** — the + * plan's Risks section flags this as a pre-existing asymmetry with + * `DawarichConnectionsRepository`'s equivalent `saveSettings` (which DOES + * wrap its writes in `uow.transactional`): preserved exactly, not + * "fixed" to match Dawarich's shape. + */ + async setAirtrailSettingsWithKey(id: number, url: string | null, apiKey: string, allowInsecureTls: number, writeEnabled: number): Promise { + await this.nativeUpdate( + { id }, + { airtrail_url: url, airtrail_api_key: apiKey, airtrail_allow_insecure_tls: allowInsecureTls, airtrail_write_enabled: writeEnabled }, + ); + } + + /** + * ATC4 (`airtrail.service.ts#saveSettings`, no-newKey branch) — `UPDATE + * users SET airtrail_url = ?, airtrail_allow_insecure_tls = ?, + * airtrail_write_enabled = ? WHERE id = ?`. Same no-transaction asymmetry + * as {@link setAirtrailSettingsWithKey} — preserved, not fixed. + */ + async setAirtrailSettings(id: number, url: string | null, allowInsecureTls: number, writeEnabled: number): Promise { + await this.nativeUpdate({ id }, { airtrail_url: url, airtrail_allow_insecure_tls: allowInsecureTls, airtrail_write_enabled: writeEnabled }); + } + + /** + * ATC5 (`airtrail.service.ts#saveSettings`, URL-cleared branch) — `UPDATE + * users SET airtrail_api_key = NULL WHERE id = ?`. **SECURITY**: + * credential scrub — a cleared URL with no key left makes the connection + * meaningless, so the key is dropped too. Same no-transaction asymmetry + * as the two methods above — this is a THIRD, separate statement in the + * legacy `saveSettings` (its own `if (!trimmedUrl)` branch), not folded + * into {@link setAirtrailSettings}. + */ + async clearAirtrailApiKey(id: number): Promise { + await this.nativeUpdate({ id }, { airtrail_api_key: null }); + } +} diff --git a/server/src/db/repositories/UserImmich.repository.ts b/server/src/db/repositories/UserImmich.repository.ts new file mode 100644 index 000000000..94c3fde62 --- /dev/null +++ b/server/src/db/repositories/UserImmich.repository.ts @@ -0,0 +1,104 @@ +import type { EntityManager } from '@mikro-orm/core'; +import type { SqlEntityManager } from '@mikro-orm/sql'; +import { Users } from '../entities/Users.entity'; +import { TrekRepository } from './_shared/trek-repository'; +import { coalesceOverrideWhileSame } from '../dialect/sql-functions'; + +/** + * The Immich connection a user keeps on their own `users` row (`immich_url`, + * `immich_api_key`, `immich_allow_insecure_tls`, `immich_auto_upload`): the + * statements `ImmichService` and `JourneyService` issue, apart from the rest + * of the account in `UsersRepository`. Same table, no move of the columns. + * + * Every value stays exactly what the SERVICE hands this repository — + * encrypted, plaintext or null, at the same point in the pipeline the raw SQL + * saw it. This repository never calls `encrypt_api_key`/`decrypt_api_key`/ + * `maybe_encrypt_api_key` itself. + */ +export class UserImmichRepository extends TrekRepository { + /** Built over the context-resolving EntityManager, like the repositories MikroORM hands out per entity. */ + constructor(em: EntityManager) { + super(em as SqlEntityManager, Users); + } + + /** + * IM1 (`ImmichService.getImmichCredentials`) — `SELECT immich_url, + * immich_api_key, immich_allow_insecure_tls FROM users WHERE id = ?` + * (the TLS switch joined the read with #2475). + */ + async getImmichCredentials( + id: number, + ): Promise<{ immich_url: string | null; immich_api_key: string | null; immich_allow_insecure_tls: number | null } | null> { + const row = await this.findOne({ id }, { fields: ['immich_url', 'immich_api_key', 'immich_allow_insecure_tls'] }); + return row + ? { + immich_url: row.immich_url ?? null, + immich_api_key: row.immich_api_key ?? null, + immich_allow_insecure_tls: row.immich_allow_insecure_tls ?? null, + } + : null; + } + + /** JV1 (`JourneyService.immichAutoUploadEnabled`) — `SELECT immich_auto_upload FROM users WHERE id = ?`. */ + async getImmichAutoUpload(id: number): Promise { + const row = await this.findOne({ id }, { fields: ['immich_auto_upload'] }); + return row?.immich_auto_upload ?? null; + } + + /** + * IM2 (`ImmichService.getConnectionSettings`'s prefs read) — `SELECT + * immich_auto_upload, immich_allow_insecure_tls FROM users WHERE id = ?` + * (the TLS switch joined the read with #2475). + */ + async getImmichConnectionPrefs( + id: number, + ): Promise<{ immich_auto_upload: number | null; immich_allow_insecure_tls: number | null } | null> { + const row = await this.findOne({ id }, { fields: ['immich_auto_upload', 'immich_allow_insecure_tls'] }); + return row + ? { immich_auto_upload: row.immich_auto_upload ?? null, immich_allow_insecure_tls: row.immich_allow_insecure_tls ?? null } + : null; + } + + /** IM3 (`ImmichService.setImmichAutoUpload`) — `UPDATE users SET immich_auto_upload = ? WHERE id = ?`. */ + async setImmichAutoUpload(id: number, enabled: number): Promise { + await this.nativeUpdate({ id }, { immich_auto_upload: enabled }); + } + + /** + * IM4 (`ImmichService.saveImmichSettings`'s URL branch) — `UPDATE users SET + * immich_url = ?, immich_api_key = ?, immich_allow_insecure_tls = CASE WHEN + * immich_url IS ? THEN COALESCE(?, immich_allow_insecure_tls) ELSE + * COALESCE(?, 0) END WHERE id = ?` (#2475). + * + * One statement, as the legacy write was: SET expressions read the row as + * it was, so the CASE compares the STORED url with the new one and decides + * the switch inside the UPDATE itself (`coalesceOverrideWhileSame`). + * `allow_insecure_tls` null keeps the stored choice while the URL stays the + * same, and a new URL without an explicit value starts off. + */ + async setImmichSettings( + id: number, + immich_url: string, + immich_api_key: string | null, + allow_insecure_tls: number | null, + ): Promise { + const platform = this.getEntityManager().getPlatform(); + await this.nativeUpdate( + { id }, + { + immich_url, + immich_api_key, + immich_allow_insecure_tls: coalesceOverrideWhileSame(platform, allow_insecure_tls, 'immich_allow_insecure_tls', 'immich_url', immich_url), + }, + ); + } + + /** + * IM5 (`ImmichService.saveImmichSettings`'s disconnect branch) — `UPDATE + * users SET immich_url = ?, immich_api_key = ?, immich_allow_insecure_tls = 0 + * WHERE id = ?` with a `null` URL: disconnecting always turns the switch off. + */ + async clearImmichSettings(id: number, immich_api_key: string | null): Promise { + await this.nativeUpdate({ id }, { immich_url: null, immich_api_key, immich_allow_insecure_tls: 0 }); + } +} diff --git a/server/src/db/repositories/UserSynology.repository.ts b/server/src/db/repositories/UserSynology.repository.ts new file mode 100644 index 000000000..853e1e07e --- /dev/null +++ b/server/src/db/repositories/UserSynology.repository.ts @@ -0,0 +1,111 @@ +import type { EntityManager } from '@mikro-orm/core'; +import type { SqlEntityManager } from '@mikro-orm/sql'; +import { Users } from '../entities/Users.entity'; +import { TrekRepository } from './_shared/trek-repository'; + +/** + * The six Synology credential/session columns `_readSynologyUser` reads + * selectively — a closed union, not a dynamic column string, matching + * `InstanceApiKeyName`'s reasoning in `Users.repository.ts`. + */ +export type SynologyUserColumn = 'synology_url' | 'synology_username' | 'synology_password' | 'synology_sid' | 'synology_did' | 'synology_skip_ssl'; + +/** + * {@link UserSynologyRepository.getSynologyFields}'s return shape — structurally + * identical to `synology.service.ts`'s own `SynologyUserRecord` (every field + * optional, so an unrequested column is simply absent), deliberately + * defined here rather than imported from the service: a repository does not + * depend on a domain service's types, and the two stay structurally + * assignable without either side casting. + */ +export interface SynologyFieldsRow { + synology_url?: string | null; + synology_username?: string | null; + synology_password?: string | null; + synology_sid?: string | null; + synology_did?: string | null; + synology_skip_ssl?: number | null; +} + +/** + * The Synology Photos connection a user keeps on their own `users` row + * (`synology_url`, `synology_username`, `synology_password`, `synology_sid`, + * `synology_did`, `synology_skip_ssl`): the statements `SynologyService` + * issues, apart from the rest of the account in `UsersRepository`. Same + * table, no move of the columns. Values stay as the service hands them in + * (the password encrypted by the service, never here). + */ +export class UserSynologyRepository extends TrekRepository { + /** Built over the context-resolving EntityManager, like the repositories MikroORM hands out per entity. */ + constructor(em: EntityManager) { + super(em as SqlEntityManager, Users); + } + + /** + * SY1 (`SynologyService._readSynologyUser`) — `SELECT synology_url, + * synology_username, synology_password, synology_sid, synology_did, + * synology_skip_ssl FROM users WHERE id = ?`, column-filtered in JS by the + * legacy code. This method selects only the requested columns (MikroORM's + * typed `fields`) rather than all six and filtering after — the RESULT is + * identical (only the requested keys populated, matching the legacy + * `filtered` object exactly; an unrequested key is `undefined`, same as + * the legacy object never having had it set). `null` means no such user + * row (SY1's "User not found" branch). + * + * Explicit per-column `if` guards, not a generic `Pick` + + * indexed-assignment loop: MikroORM's `fields` option types against + * `AutoPath`, which does not resolve for a free type + * parameter `K` the way it does for the concrete literal union used here + * (the same reason `getApiKeyColumn` above stays non-generic); an + * indexed-assignment loop over a union key has the identical problem on + * the write side. Six `if`s reads worse but needs neither a generic nor a + * cast. + */ + async getSynologyFields(id: number, columns: SynologyUserColumn[]): Promise { + const row = await this.findOne({ id }, { fields: columns }); + if (!row) return null; + const result: SynologyFieldsRow = {}; + if (columns.includes('synology_url')) result.synology_url = row.synology_url ?? null; + if (columns.includes('synology_username')) result.synology_username = row.synology_username ?? null; + if (columns.includes('synology_password')) result.synology_password = row.synology_password ?? null; + if (columns.includes('synology_sid')) result.synology_sid = row.synology_sid ?? null; + if (columns.includes('synology_did')) result.synology_did = row.synology_did ?? null; + if (columns.includes('synology_skip_ssl')) result.synology_skip_ssl = row.synology_skip_ssl; + return result; + } + + /** SY2 (`SynologyService._clearSynologySID`) — `UPDATE users SET synology_sid = NULL WHERE id = ?`. */ + async clearSynologySID(id: number): Promise { + await this.nativeUpdate({ id }, { synology_sid: null }); + } + + /** SY3 (`SynologyService._clearSynologySession`) — `UPDATE users SET synology_sid = NULL, synology_did = NULL WHERE id = ?`. */ + async clearSynologySession(id: number): Promise { + await this.nativeUpdate({ id }, { synology_sid: null, synology_did: null }); + } + + /** + * SY4/SY7 (`SynologyService._getSynologySession`'s session-refresh write, + * `testSynologyConnection`'s sid persist) — byte-identical `UPDATE users + * SET synology_sid = ? WHERE id = ?` at both legacy sites, one method (D4). + */ + async setSynologySid(id: number, synology_sid: string): Promise { + await this.nativeUpdate({ id }, { synology_sid }); + } + + /** SY5 (`SynologyService.updateSynologySettings`) — `UPDATE users SET synology_url = ?, synology_username = ?, synology_password = ?, synology_skip_ssl = ? WHERE id = ?`. */ + async setSynologySettings(id: number, synology_url: string, synology_username: string, synology_password: string | null, synology_skip_ssl: number): Promise { + await this.nativeUpdate({ id }, { synology_url, synology_username, synology_password, synology_skip_ssl }); + } + + /** SY6 (`SynologyService.getSynologyStatus`) — `SELECT synology_username FROM users WHERE id = ?`. */ + async getSynologyUsername(id: number): Promise { + const row = await this.findOne({ id }, { fields: ['synology_username'] }); + return row?.synology_username ?? null; + } + + /** SY8 (`SynologyService.testSynologyConnection`'s did persist) — `UPDATE users SET synology_did = ? WHERE id = ?`. */ + async setSynologyDid(id: number, synology_did: string): Promise { + await this.nativeUpdate({ id }, { synology_did }); + } +} diff --git a/server/src/db/repositories/Users.repository.ts b/server/src/db/repositories/Users.repository.ts index 926ffac6c..cb65aeec6 100644 --- a/server/src/db/repositories/Users.repository.ts +++ b/server/src/db/repositories/Users.repository.ts @@ -1,6 +1,6 @@ import { Users } from '../entities/Users.entity'; import { toRow, type AssertRowKeys } from './_shared/rows'; -import { coalesce, coalesceOverrideWhileSame, columnIncrementedBy, currentTimestamp, lower, lowerParam } from '../dialect/sql-functions'; +import { coalesce, columnIncrementedBy, currentTimestamp, lower, lowerParam } from '../dialect/sql-functions'; import { TrekRepository } from './_shared/trek-repository'; import { UniqueConstraintViolationException } from '@mikro-orm/core'; @@ -41,7 +41,6 @@ export interface UserRow { mfa_secret: string | null; mfa_backup_codes: string | null; immich_url: string | null; - immich_access_token: string | null; synology_url: string | null; synology_username: string | null; synology_password: string | null; @@ -66,7 +65,15 @@ export interface UserRow { immich_allow_insecure_tls: number; } -const _userRowKeys: AssertRowKeys = true; +/** + * Columns the table still holds but no code reads or writes, left out of + * {@link UserRow}: `immich_access_token` predates `immich_api_key` and has been + * dead since. The column stays (old databases have it, and migrations are + * append-only), so a row read whole still carries it; only the type drops it. + */ +type DeadUserColumns = 'immich_access_token'; + +const _userRowKeys: AssertRowKeys> = true; /** `AuthService.registerUser` (AU10) / `OidcService.findOrCreateUser` (O14) — the two column sets `insertUser` covers as one method (Task 1 brief's Required set: "one method, nullable oidc/avatar fields"). */ export interface NewUserRow { @@ -1235,164 +1242,6 @@ export class UsersRepository extends TrekRepository { return row ? { id: row.id, username: row.username } : undefined; } - // --------------------------------------------------------------------- - // Plan 3e Task 7 (memories' provider half) — additive: the Immich/Synology - // encrypted-credential statements (IM1-5/SY1-8, inventory §7d, R6). Every - // value stays exactly what the SERVICE (`ImmichService`/`SynologyService`) - // hands this repository — encrypted, plaintext or null, at the same point - // in the pipeline the raw SQL saw it. This repository never calls - // `encrypt_api_key`/`decrypt_api_key`/`maybe_encrypt_api_key` itself. - // --------------------------------------------------------------------- - - /** - * IM1 (`ImmichService.getImmichCredentials`) — `SELECT immich_url, - * immich_api_key, immich_allow_insecure_tls FROM users WHERE id = ?` - * (the TLS switch joined the read with #2475). - */ - async getImmichCredentials( - id: number, - ): Promise<{ immich_url: string | null; immich_api_key: string | null; immich_allow_insecure_tls: number | null } | null> { - const row = await this.findOne({ id }, { fields: ['immich_url', 'immich_api_key', 'immich_allow_insecure_tls'] }); - return row - ? { - immich_url: row.immich_url ?? null, - immich_api_key: row.immich_api_key ?? null, - immich_allow_insecure_tls: row.immich_allow_insecure_tls ?? null, - } - : null; - } - - /** JV1 (`JourneyService.immichAutoUploadEnabled`) — `SELECT immich_auto_upload FROM users WHERE id = ?`. */ - async getImmichAutoUpload(id: number): Promise { - const row = await this.findOne({ id }, { fields: ['immich_auto_upload'] }); - return row?.immich_auto_upload ?? null; - } - - /** - * IM2 (`ImmichService.getConnectionSettings`'s prefs read) — `SELECT - * immich_auto_upload, immich_allow_insecure_tls FROM users WHERE id = ?` - * (the TLS switch joined the read with #2475). - */ - async getImmichConnectionPrefs( - id: number, - ): Promise<{ immich_auto_upload: number | null; immich_allow_insecure_tls: number | null } | null> { - const row = await this.findOne({ id }, { fields: ['immich_auto_upload', 'immich_allow_insecure_tls'] }); - return row - ? { immich_auto_upload: row.immich_auto_upload ?? null, immich_allow_insecure_tls: row.immich_allow_insecure_tls ?? null } - : null; - } - - /** IM3 (`ImmichService.setImmichAutoUpload`) — `UPDATE users SET immich_auto_upload = ? WHERE id = ?`. */ - async setImmichAutoUpload(id: number, enabled: number): Promise { - await this.nativeUpdate({ id }, { immich_auto_upload: enabled }); - } - - /** - * IM4 (`ImmichService.saveImmichSettings`'s URL branch) — `UPDATE users SET - * immich_url = ?, immich_api_key = ?, immich_allow_insecure_tls = CASE WHEN - * immich_url IS ? THEN COALESCE(?, immich_allow_insecure_tls) ELSE - * COALESCE(?, 0) END WHERE id = ?` (#2475). - * - * One statement, as the legacy write was: SET expressions read the row as - * it was, so the CASE compares the STORED url with the new one and decides - * the switch inside the UPDATE itself (`coalesceOverrideWhileSame`). - * `allow_insecure_tls` null keeps the stored choice while the URL stays the - * same, and a new URL without an explicit value starts off. - */ - async setImmichSettings( - id: number, - immich_url: string, - immich_api_key: string | null, - allow_insecure_tls: number | null, - ): Promise { - const platform = this.getEntityManager().getPlatform(); - await this.nativeUpdate( - { id }, - { - immich_url, - immich_api_key, - immich_allow_insecure_tls: coalesceOverrideWhileSame(platform, allow_insecure_tls, 'immich_allow_insecure_tls', 'immich_url', immich_url), - }, - ); - } - - /** - * IM5 (`ImmichService.saveImmichSettings`'s disconnect branch) — `UPDATE - * users SET immich_url = ?, immich_api_key = ?, immich_allow_insecure_tls = 0 - * WHERE id = ?` with a `null` URL: disconnecting always turns the switch off. - */ - async clearImmichSettings(id: number, immich_api_key: string | null): Promise { - await this.nativeUpdate({ id }, { immich_url: null, immich_api_key, immich_allow_insecure_tls: 0 }); - } - - /** - * SY1 (`SynologyService._readSynologyUser`) — `SELECT synology_url, - * synology_username, synology_password, synology_sid, synology_did, - * synology_skip_ssl FROM users WHERE id = ?`, column-filtered in JS by the - * legacy code. This method selects only the requested columns (MikroORM's - * typed `fields`) rather than all six and filtering after — the RESULT is - * identical (only the requested keys populated, matching the legacy - * `filtered` object exactly; an unrequested key is `undefined`, same as - * the legacy object never having had it set). `null` means no such user - * row (SY1's "User not found" branch). - * - * Explicit per-column `if` guards, not a generic `Pick` + - * indexed-assignment loop: MikroORM's `fields` option types against - * `AutoPath`, which does not resolve for a free type - * parameter `K` the way it does for the concrete literal union used here - * (the same reason `getApiKeyColumn` above stays non-generic); an - * indexed-assignment loop over a union key has the identical problem on - * the write side. Six `if`s reads worse but needs neither a generic nor a - * cast. - */ - async getSynologyFields(id: number, columns: SynologyUserColumn[]): Promise { - const row = await this.findOne({ id }, { fields: columns }); - if (!row) return null; - const result: SynologyFieldsRow = {}; - if (columns.includes('synology_url')) result.synology_url = row.synology_url ?? null; - if (columns.includes('synology_username')) result.synology_username = row.synology_username ?? null; - if (columns.includes('synology_password')) result.synology_password = row.synology_password ?? null; - if (columns.includes('synology_sid')) result.synology_sid = row.synology_sid ?? null; - if (columns.includes('synology_did')) result.synology_did = row.synology_did ?? null; - if (columns.includes('synology_skip_ssl')) result.synology_skip_ssl = row.synology_skip_ssl; - return result; - } - - /** SY2 (`SynologyService._clearSynologySID`) — `UPDATE users SET synology_sid = NULL WHERE id = ?`. */ - async clearSynologySID(id: number): Promise { - await this.nativeUpdate({ id }, { synology_sid: null }); - } - - /** SY3 (`SynologyService._clearSynologySession`) — `UPDATE users SET synology_sid = NULL, synology_did = NULL WHERE id = ?`. */ - async clearSynologySession(id: number): Promise { - await this.nativeUpdate({ id }, { synology_sid: null, synology_did: null }); - } - - /** - * SY4/SY7 (`SynologyService._getSynologySession`'s session-refresh write, - * `testSynologyConnection`'s sid persist) — byte-identical `UPDATE users - * SET synology_sid = ? WHERE id = ?` at both legacy sites, one method (D4). - */ - async setSynologySid(id: number, synology_sid: string): Promise { - await this.nativeUpdate({ id }, { synology_sid }); - } - - /** SY5 (`SynologyService.updateSynologySettings`) — `UPDATE users SET synology_url = ?, synology_username = ?, synology_password = ?, synology_skip_ssl = ? WHERE id = ?`. */ - async setSynologySettings(id: number, synology_url: string, synology_username: string, synology_password: string | null, synology_skip_ssl: number): Promise { - await this.nativeUpdate({ id }, { synology_url, synology_username, synology_password, synology_skip_ssl }); - } - - /** SY6 (`SynologyService.getSynologyStatus`) — `SELECT synology_username FROM users WHERE id = ?`. */ - async getSynologyUsername(id: number): Promise { - const row = await this.findOne({ id }, { fields: ['synology_username'] }); - return row?.synology_username ?? null; - } - - /** SY8 (`SynologyService.testSynologyConnection`'s did persist) — `UPDATE users SET synology_did = ? WHERE id = ?`. */ - async setSynologyDid(id: number, synology_did: string): Promise { - await this.nativeUpdate({ id }, { synology_did }); - } - // --------------------------------------------------------------------- // UM11 — UnifiedMemoriesService._notifySharedTripPhotos // --------------------------------------------------------------------- @@ -1429,81 +1278,6 @@ export class UsersRepository extends TrekRepository { return row ? { id: row.id, username: row.username } : null; } - // --------------------------------------------------------------------- - // Plan 3h Task 4 (`AirtrailService` — R7, the encrypt-in-service - // discipline identical to Task 3's `DawarichConnectionsRepository`: - // every value below is exactly what the SERVICE hands this repository — - // encrypted, plaintext or null, at the same point in the pipeline the - // raw SQL saw it. This repository never calls - // maybe_encrypt_api_key/decrypt_api_key itself. - // --------------------------------------------------------------------- - - /** ATC1 (`airtrail.service.ts#readRow`) — `SELECT airtrail_url, airtrail_api_key, airtrail_allow_insecure_tls, airtrail_write_enabled FROM users WHERE id = ?`. */ - async getAirtrailConnRow(id: number): Promise<{ - airtrail_url: string | null; - airtrail_api_key: string | null; - airtrail_allow_insecure_tls: number | null; - airtrail_write_enabled: number | null; - } | null> { - const row = await this.findOne( - { id }, - { fields: ['airtrail_url', 'airtrail_api_key', 'airtrail_allow_insecure_tls', 'airtrail_write_enabled'] }, - ); - return row - ? { - airtrail_url: row.airtrail_url ?? null, - airtrail_api_key: row.airtrail_api_key ?? null, - airtrail_allow_insecure_tls: row.airtrail_allow_insecure_tls ?? null, - airtrail_write_enabled: row.airtrail_write_enabled ?? null, - } - : null; - } - - /** ATC2 (`airtrail.service.ts#isAirtrailWriteEnabled`) — `SELECT airtrail_write_enabled FROM users WHERE id = ?`. */ - async getAirtrailWriteEnabled(id: number): Promise { - const row = await this.findOne({ id }, { fields: ['airtrail_write_enabled'] }); - return row?.airtrail_write_enabled ?? null; - } - - /** - * ATC3 (`airtrail.service.ts#saveSettings`, newKey branch) — `UPDATE users - * SET airtrail_url = ?, airtrail_api_key = ?, airtrail_allow_insecure_tls - * = ?, airtrail_write_enabled = ? WHERE id = ?`. **No transaction** — the - * plan's Risks section flags this as a pre-existing asymmetry with - * `DawarichConnectionsRepository`'s equivalent `saveSettings` (which DOES - * wrap its writes in `uow.transactional`): preserved exactly, not - * "fixed" to match Dawarich's shape. - */ - async setAirtrailSettingsWithKey(id: number, url: string | null, apiKey: string, allowInsecureTls: number, writeEnabled: number): Promise { - await this.nativeUpdate( - { id }, - { airtrail_url: url, airtrail_api_key: apiKey, airtrail_allow_insecure_tls: allowInsecureTls, airtrail_write_enabled: writeEnabled }, - ); - } - - /** - * ATC4 (`airtrail.service.ts#saveSettings`, no-newKey branch) — `UPDATE - * users SET airtrail_url = ?, airtrail_allow_insecure_tls = ?, - * airtrail_write_enabled = ? WHERE id = ?`. Same no-transaction asymmetry - * as {@link setAirtrailSettingsWithKey} — preserved, not fixed. - */ - async setAirtrailSettings(id: number, url: string | null, allowInsecureTls: number, writeEnabled: number): Promise { - await this.nativeUpdate({ id }, { airtrail_url: url, airtrail_allow_insecure_tls: allowInsecureTls, airtrail_write_enabled: writeEnabled }); - } - - /** - * ATC5 (`airtrail.service.ts#saveSettings`, URL-cleared branch) — `UPDATE - * users SET airtrail_api_key = NULL WHERE id = ?`. **SECURITY**: - * credential scrub — a cleared URL with no key left makes the connection - * meaningless, so the key is dropped too. Same no-transaction asymmetry - * as the two methods above — this is a THIRD, separate statement in the - * legacy `saveSettings` (its own `if (!trimmedUrl)` branch), not folded - * into {@link setAirtrailSettings}. - */ - async clearAirtrailApiKey(id: number): Promise { - await this.nativeUpdate({ id }, { airtrail_api_key: null }); - } - // --------------------------------------------------------------------- // Plan 3i Task 1 (`AdminService` — AD1-AD17, admin's user-CRUD surface). // Every statement not already covered by an existing method above @@ -1644,27 +1418,3 @@ export class UsersRepository extends TrekRepository { return row ? { id: row.id, username: row.username, display_name: row.display_name ?? null, avatar: row.avatar ?? null } : null; } } - -/** - * The six Synology credential/session columns `_readSynologyUser` reads - * selectively — a closed union, not a dynamic column string, matching - * `InstanceApiKeyName`'s reasoning above. - */ -export type SynologyUserColumn = 'synology_url' | 'synology_username' | 'synology_password' | 'synology_sid' | 'synology_did' | 'synology_skip_ssl'; - -/** - * {@link UsersRepository.getSynologyFields}'s return shape — structurally - * identical to `synology.service.ts`'s own `SynologyUserRecord` (every field - * optional, so an unrequested column is simply absent), deliberately - * defined here rather than imported from the service: a repository does not - * depend on a domain service's types, and the two stay structurally - * assignable without either side casting. - */ -export interface SynologyFieldsRow { - synology_url?: string | null; - synology_username?: string | null; - synology_password?: string | null; - synology_sid?: string | null; - synology_did?: string | null; - synology_skip_ssl?: number | null; -} diff --git a/server/src/nest/database/user-connection-repositories.module.ts b/server/src/nest/database/user-connection-repositories.module.ts new file mode 100644 index 000000000..0ea461132 --- /dev/null +++ b/server/src/nest/database/user-connection-repositories.module.ts @@ -0,0 +1,23 @@ +import { Module } from '@nestjs/common'; +import { EntityManager } from '@mikro-orm/core'; +import { UserAirtrailRepository } from '../../db/repositories/UserAirtrail.repository'; +import { UserImmichRepository } from '../../db/repositories/UserImmich.repository'; +import { UserSynologyRepository } from '../../db/repositories/UserSynology.repository'; + +/** + * Provides the repositories for the third-party connections a user keeps on + * their own `users` row (Immich, Synology Photos, AirTrail). They share the + * `users` table with `UsersRepository`, so MikroORM's one-repository-per-entity + * `@InjectRepository(Users)` cannot hand them out; each is a factory over the + * context-resolving global `EntityManager`, as `MaintenanceModule` does, so it + * resolves the request or transaction fork active when a method runs. + */ +@Module({ + providers: [ + { provide: UserImmichRepository, useFactory: (em: EntityManager) => new UserImmichRepository(em), inject: [EntityManager] }, + { provide: UserSynologyRepository, useFactory: (em: EntityManager) => new UserSynologyRepository(em), inject: [EntityManager] }, + { provide: UserAirtrailRepository, useFactory: (em: EntityManager) => new UserAirtrailRepository(em), inject: [EntityManager] }, + ], + exports: [UserImmichRepository, UserSynologyRepository, UserAirtrailRepository], +}) +export class UserConnectionRepositoriesModule {} diff --git a/server/src/nest/integrations/airtrail-core.module.ts b/server/src/nest/integrations/airtrail-core.module.ts index 2c3763b91..75c776c89 100644 --- a/server/src/nest/integrations/airtrail-core.module.ts +++ b/server/src/nest/integrations/airtrail-core.module.ts @@ -11,6 +11,7 @@ import { AppSettings } from '../../db/entities/AppSettings.entity'; import { AirtrailClient } from './airtrail.client'; import { AirtrailLinkService } from './airtrail-link.service'; import { AirtrailService } from './airtrail.service'; +import { UserConnectionRepositoriesModule } from '../database/user-connection-repositories.module'; /** * The AirTrail pieces ReservationsModule may inject: the HTTP client, the @@ -35,6 +36,7 @@ import { AirtrailService } from './airtrail.service'; AddonsModule, RealtimeModule, ReservationsReadModule, + UserConnectionRepositoriesModule, MikroOrmModule.forFeature([Users, Reservations, ReservationEndpoints, AppSettings]), ], providers: [AirtrailClient, AirtrailService, AirtrailLinkService], diff --git a/server/src/nest/integrations/airtrail.service.ts b/server/src/nest/integrations/airtrail.service.ts index d75525eeb..c5314eaa0 100644 --- a/server/src/nest/integrations/airtrail.service.ts +++ b/server/src/nest/integrations/airtrail.service.ts @@ -1,8 +1,6 @@ import { Injectable } from '@nestjs/common'; -import { InjectRepository } from '@mikro-orm/nestjs'; import type { AirtrailFlight } from '@trek/shared'; -import { Users } from '../../db/entities/Users.entity'; -import { UsersRepository } from '../../db/repositories/Users.repository'; +import { UserAirtrailRepository } from '../../db/repositories/UserAirtrail.repository'; import { AuditService } from '../audit/audit.service'; import { maybe_encrypt_api_key, decrypt_api_key } from '../common/crypto/apiKeyCrypto'; import { checkSsrf } from '../../utils/ssrfGuard'; @@ -33,7 +31,7 @@ interface UserConnRow { * and bought nothing while AirTrail is the only integration of this shape. * * Plan 3h Task 4 (ATC1-5): the raw `users` reads/writes now go through - * `UsersRepository`'s additive `getAirtrailConnRow`/`getAirtrailWriteEnabled`/ + * `UserAirtrailRepository`'s (same `users` table) `getAirtrailConnRow`/`getAirtrailWriteEnabled`/ * `setAirtrailSettingsWithKey`/`setAirtrailSettings`/`clearAirtrailApiKey` * methods (R7 — the repository sees only opaque, already-encrypted TEXT; * `maybe_encrypt_api_key`/`decrypt_api_key` stay here). `saveSettings` keeps @@ -44,19 +42,19 @@ interface UserConnRow { @Injectable() export class AirtrailService { constructor( - @InjectRepository(Users) private readonly usersRepo: UsersRepository, + private readonly airtrailRepo: UserAirtrailRepository, private readonly audit: AuditService, private readonly client: AirtrailClient, ) {} private async readRow(userId: number): Promise { - const row = await this.usersRepo.getAirtrailConnRow(userId); + const row = await this.airtrailRepo.getAirtrailConnRow(userId); return row ?? undefined; } /** Has this user opted in to TREK writing their flight edits back to AirTrail? (#1240) */ async isAirtrailWriteEnabled(userId: number): Promise { - const value = await this.usersRepo.getAirtrailWriteEnabled(userId); + const value = await this.airtrailRepo.getAirtrailWriteEnabled(userId); return !!value; } @@ -122,12 +120,12 @@ export class AirtrailService { const newKey = provided && provided !== KEY_MASK ? maybe_encrypt_api_key(provided) : undefined; if (newKey !== undefined) { - await this.usersRepo.setAirtrailSettingsWithKey(userId, trimmedUrl || null, newKey, allowInsecureTls ? 1 : 0, writeEnabled ? 1 : 0); + await this.airtrailRepo.setAirtrailSettingsWithKey(userId, trimmedUrl || null, newKey, allowInsecureTls ? 1 : 0, writeEnabled ? 1 : 0); } else { - await this.usersRepo.setAirtrailSettings(userId, trimmedUrl || null, allowInsecureTls ? 1 : 0, writeEnabled ? 1 : 0); + await this.airtrailRepo.setAirtrailSettings(userId, trimmedUrl || null, allowInsecureTls ? 1 : 0, writeEnabled ? 1 : 0); // Clearing the URL with no key left makes the connection meaningless — drop the key too. if (!trimmedUrl) { - await this.usersRepo.clearAirtrailApiKey(userId); + await this.airtrailRepo.clearAirtrailApiKey(userId); } } diff --git a/server/src/nest/journey/journey.module.ts b/server/src/nest/journey/journey.module.ts index dc17b00b6..8c15e0e5a 100644 --- a/server/src/nest/journey/journey.module.ts +++ b/server/src/nest/journey/journey.module.ts @@ -19,6 +19,7 @@ import { buildStorageUploadOptions } from '../storage/storage-upload.factory'; import { journeyImageFileFilter, journeyUploadFilename } from './journey.controller'; import { Users } from '../../db/entities/Users.entity'; import { JourneyBooks } from '../../db/entities/JourneyBooks.entity'; +import { UserConnectionRepositoriesModule } from '../database/user-connection-repositories.module'; @Module({ // MemoriesModule: the journey gallery streams provider assets and uploads to Immich. @@ -38,7 +39,9 @@ import { JourneyBooks } from '../../db/entities/JourneyBooks.entity'; }), StorageModule, AuthModule, AddonsModule, MemoriesModule, JourneyDomainModule, JourneyPhotoCaptureModule, - // Plan 3g Task 3: `JourneyService` (JV1, `UsersRepository.getImmichAutoUpload`) + // JourneyService's JV1 read (`UserImmichRepository.getImmichAutoUpload`). + UserConnectionRepositoriesModule, + // Plan 3g Task 3: `JourneyService` (JV1, now through the module above) // and `JourneyBookService` (JB1-JB7, `JourneyBooksRepository`) both // constructed HERE — `@InjectRepository` resolves from this module's own // `forFeature` graph, not from `AuthModule`'s (which registers `Users` diff --git a/server/src/nest/journey/journey.service.ts b/server/src/nest/journey/journey.service.ts index 90541e11d..c0027c349 100644 --- a/server/src/nest/journey/journey.service.ts +++ b/server/src/nest/journey/journey.service.ts @@ -1,5 +1,4 @@ import { Injectable } from '@nestjs/common'; -import { InjectRepository } from '@mikro-orm/nestjs'; import { JourneyDomainService } from './journey-domain.service'; import { JourneyShareService } from './journey-share.service'; import { ImmichService } from '../memories/immich.service'; @@ -7,8 +6,7 @@ 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'; -import { Users } from '../../db/entities/Users.entity'; -import type { UsersRepository } from '../../db/repositories/Users.repository'; +import { UserImmichRepository } from '../../db/repositories/UserImmich.repository'; import type { Response } from 'express'; /** @@ -26,7 +24,7 @@ export class JourneyService { private readonly photoResolver: PhotoResolverService, private readonly journey: JourneyDomainService, private readonly share: JourneyShareService, - @InjectRepository(Users) private readonly usersRepo: UsersRepository, + private readonly immichRepo: UserImmichRepository, ) {} async journeyAddonEnabled(): Promise { @@ -86,9 +84,9 @@ export class JourneyService { deleteJourneyShareLink(id: number, userId: number) { return this.share.deleteJourneyShareLink(id, userId); } // Immich mirror (only when the user opted in via integration settings) - // JV1 — `UsersRepository.getImmichAutoUpload`. + // JV1 — `UserImmichRepository.getImmichAutoUpload`. async immichAutoUploadEnabled(userId: number): Promise { - return !!(await this.usersRepo.getImmichAutoUpload(userId)); + return !!(await this.immichRepo.getImmichAutoUpload(userId)); } uploadToImmich(userId: number, relativePath: string, originalName: string) { return this.immich.uploadToImmich(userId, relativePath, originalName); } diff --git a/server/src/nest/memories/immich.service.ts b/server/src/nest/memories/immich.service.ts index 86ecc006b..2523895ab 100644 --- a/server/src/nest/memories/immich.service.ts +++ b/server/src/nest/memories/immich.service.ts @@ -1,7 +1,6 @@ import { Injectable } from '@nestjs/common'; import { PROVIDER_SELECT_ALL_MAX_PAGES } from '@trek/shared'; import type { Response } from 'express'; -import { InjectRepository } from '@mikro-orm/nestjs'; import { maybe_encrypt_api_key, decrypt_api_key } from '../common/crypto/apiKeyCrypto'; import { checkSsrf, safeFetch, type SafeFetchOptions } from '../../utils/ssrfGuard'; import { AuditService } from '../audit/audit.service'; @@ -10,9 +9,8 @@ import fsPromises from 'node:fs/promises'; import path from 'node:path'; import { MemoriesAccessService } from './memories-access.service'; import { describeFetchFailure, fail, handleServiceResult, isWithinLocalDayRange, pipeAsset, shiftCalendarDay, sortAssetsByTakenAtDesc, type Selection } from './memories.helpers'; -import { Users } from '../../db/entities/Users.entity'; import { UnitOfWork } from '../database/unit-of-work'; -import type { UsersRepository } from '../../db/repositories/Users.repository'; +import { UserImmichRepository } from '../../db/repositories/UserImmich.repository'; const ALBUM_PAGE_SIZE = 1000; const ALBUM_MAX_PAGES = 20; @@ -81,12 +79,12 @@ export class ImmichService { private readonly audit: AuditService, private readonly access: MemoriesAccessService, private readonly storage: StorageService, - @InjectRepository(Users) private readonly users: UsersRepository, + private readonly immichRepo: UserImmichRepository, private readonly uow: UnitOfWork, ) {} async getImmichCredentials(userId: number): Promise { - const user = await this.users.getImmichCredentials(userId); + const user = await this.immichRepo.getImmichCredentials(userId); if (!user?.immich_url || !user?.immich_api_key) return null; const apiKey = decrypt_api_key(user.immich_api_key); if (!apiKey) return null; @@ -123,7 +121,7 @@ export class ImmichService { async getConnectionSettings(userId: number) { const creds = await this.getImmichCredentials(userId); - const prefs = await this.users.getImmichConnectionPrefs(userId); + const prefs = await this.immichRepo.getImmichConnectionPrefs(userId); return { immich_url: creds?.immich_url || '', connected: !!(creds?.immich_url && creds?.immich_api_key), @@ -133,7 +131,7 @@ export class ImmichService { } async setImmichAutoUpload(userId: number, enabled: boolean): Promise { - await this.users.setImmichAutoUpload(userId, enabled ? 1 : 0); + await this.immichRepo.setImmichAutoUpload(userId, enabled ? 1 : 0); } /** @@ -165,9 +163,9 @@ export class ImmichService { if (immichUrl) { const insecure = allowInsecureTls === undefined ? null : Number(allowInsecureTls); // The stored URL decides whether an undefined switch keeps its value (IM4). - await this.users.setImmichSettings(userId, url, maybe_encrypt_api_key(immichApiKey), insecure); + await this.immichRepo.setImmichSettings(userId, url, maybe_encrypt_api_key(immichApiKey), insecure); } else { - await this.users.clearImmichSettings(userId, maybe_encrypt_api_key(immichApiKey)); + await this.immichRepo.clearImmichSettings(userId, maybe_encrypt_api_key(immichApiKey)); } if (typeof autoUpload === 'boolean') await this.setImmichAutoUpload(userId, autoUpload); }); diff --git a/server/src/nest/memories/memories.module.ts b/server/src/nest/memories/memories.module.ts index 1ef30bb3a..96b2d237c 100644 --- a/server/src/nest/memories/memories.module.ts +++ b/server/src/nest/memories/memories.module.ts @@ -3,6 +3,7 @@ import { MikroOrmModule } from '@mikro-orm/nestjs'; import { MemoriesService } from './memories.service'; import { MemoriesAccessService } from './memories-access.service'; import { ImmichService } from './immich.service'; +import { UserConnectionRepositoriesModule } from '../database/user-connection-repositories.module'; import { SynologyService } from './synology.service'; import { UnifiedMemoriesService } from './unified-memories.service'; import { PhotoResolverService } from './photo-resolver.service'; @@ -75,6 +76,7 @@ import { JourneyPhotos } from '../../db/entities/JourneyPhotos.entity'; @Module({ imports: [ NotificationsModule, AddonsModule, AuditModule, TrekPhotosModule, RealtimeModule, SchedulingModule, StorageModule, + UserConnectionRepositoriesModule, MikroOrmModule.forFeature([TripPhotos, TrekPhotos, TripAlbumLinks, Trips, TrekPhotoCacheMeta, PhotoProviders, Users, Journeys, JourneyContributors, JourneyPhotos]), ], controllers: [UnifiedMemoriesController, ImmichMemoriesController, SynologyMemoriesController], diff --git a/server/src/nest/memories/synology.service.ts b/server/src/nest/memories/synology.service.ts index e0e16af29..a9399b9a2 100644 --- a/server/src/nest/memories/synology.service.ts +++ b/server/src/nest/memories/synology.service.ts @@ -1,11 +1,9 @@ import { Injectable } from '@nestjs/common'; import { Response } from 'express'; -import { InjectRepository } from '@mikro-orm/nestjs'; import { decrypt_api_key, encrypt_api_key, maybe_encrypt_api_key } from '../common/crypto/apiKeyCrypto'; import { safeFetch, SsrfBlockedError, checkSsrf } from '../../utils/ssrfGuard'; import { MemoriesAccessService } from './memories-access.service'; -import { Users } from '../../db/entities/Users.entity'; -import type { UsersRepository, SynologyUserColumn } from '../../db/repositories/Users.repository'; +import { UserSynologyRepository, type SynologyUserColumn } from '../../db/repositories/UserSynology.repository'; import { dayStartEpochSeconds, fail, @@ -139,12 +137,12 @@ export class SynologyService { constructor( private readonly access: MemoriesAccessService, private readonly notifications: NotificationsService, - @InjectRepository(Users) private readonly users: UsersRepository, + private readonly synologyRepo: UserSynologyRepository, ) {} private async _readSynologyUser(userId: number, columns: SynologyUserColumn[]): Promise> { try { - const filtered = await this.users.getSynologyFields(userId, columns); + const filtered = await this.synologyRepo.getSynologyFields(userId, columns); if (!filtered) { return fail('User not found', 404); @@ -309,11 +307,11 @@ export class SynologyService { private async _clearSynologySID(userId: number): Promise { - await this.users.clearSynologySID(userId); + await this.synologyRepo.clearSynologySID(userId); } private async _clearSynologySession(userId: number): Promise { - await this.users.clearSynologySession(userId); + await this.synologyRepo.clearSynologySession(userId); } private _splitPackedSynologyId(rawId: string): { id: string; cacheKey: string; assetId: string } | null { @@ -352,7 +350,7 @@ export class SynologyService { return resp as ServiceResult; } - await this.users.setSynologySid(userId, encrypt_api_key(resp.data.sid)); + await this.synologyRepo.setSynologySid(userId, encrypt_api_key(resp.data.sid)); return success(resp.data.sid); } @@ -406,7 +404,7 @@ export class SynologyService { } try { - await this.users.setSynologySettings( + await this.synologyRepo.setSynologySettings( userId, synologyUrl, synologyUsername, @@ -425,7 +423,7 @@ export class SynologyService { if ('error' in sid) return success({ connected: false, error: sid.error.message }); if (!sid.data) return success({ connected: false, error: 'Not connected to Synology' }); try { - const synologyUsername = await this.users.getSynologyUsername(userId); + const synologyUsername = await this.synologyRepo.getSynologyUsername(userId); return success({ connected: true, user: { name: synologyUsername || 'unknown user' } }); } catch (err: unknown) { return success({ connected: true, user: { name: 'unknown user' } }); @@ -446,9 +444,9 @@ export class SynologyService { // Persist the session so the OTP code is not required again on save. // The did (device token) allows future re-logins without OTP. - await this.users.setSynologySid(userId, encrypt_api_key(resp.data.sid)); + await this.synologyRepo.setSynologySid(userId, encrypt_api_key(resp.data.sid)); if (resp.data.did) { - await this.users.setSynologyDid(userId, encrypt_api_key(resp.data.did)); + await this.synologyRepo.setSynologyDid(userId, encrypt_api_key(resp.data.did)); } return success({ connected: true, user: { name: synologyUsername } }); diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index 33dbb552b..c86ff38f4 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -140,6 +140,9 @@ import { createTestReservationDayPositionsRepo, createTestDayAccommodationsRepo, createTestUsersRepo, + createTestUserImmichRepo, + createTestUserSynologyRepo, + createTestUserAirtrailRepo, createTestCollectionsRepo, createTestCollectionMembersRepo, createTestCollectionLabelsRepo, createTestCollectionPlacesRepo, createTestCollectionPlaceRatingsRepo, sharedTestOrm, @@ -405,8 +408,8 @@ export async function createMcpTestRegistry(): Promise { // Plan 3g Task 4 constructor-ripple: MA1/MA2/MA6's journey half. await createTestJourneysRepo(db), await createTestJourneyContributorsRepo(db), await createTestJourneyPhotosRepo(db), ); - const immichService = new ImmichService(new AuditService(auditLogRepo, usersRepo), memoriesAccess, generalStorage, usersRepo, await createTestUnitOfWork(db)); - const synologyService = new SynologyService(memoriesAccess, notificationsStub(), usersRepo); + const immichService = new ImmichService(new AuditService(auditLogRepo, usersRepo), memoriesAccess, generalStorage, await createTestUserImmichRepo(db), await createTestUnitOfWork(db)); + const synologyService = new SynologyService(memoriesAccess, notificationsStub(), await createTestUserSynologyRepo(db)); const trekPhotos = new TrekPhotoRegistrationService(mcpOrm.repo(TrekPhotos), mcpOrm.repo(TripPhotos), await createTestJourneyPhotosRepo(db)); const captureBackfill = new PhotoCaptureBackfillService(new PhotoResolverService(trekPhotos, new ThumbnailService(addonsService, generalStorage, mcpOrm.repo(TrekPhotos)), new TrekPhotoCacheService(mcpOrm.repo(TrekPhotoCacheMeta), generalStorage), new PhotoProviderRegistry([new ImmichPhotoProvider(immichService), new SynologyPhotoProvider(synologyService)]), generalStorage), trekPhotos, generalStorage); return createTestRegistry( @@ -517,10 +520,10 @@ export async function createMcpTestRegistry(): Promise { ), addonsService, authService, new JourneyPhotoCaptureService(captureBackfill, journeyDomain, { reverseGeocode: async () => ({ name: null, address: null }) } as never, mcpOrm.orm)), new MemoriesMcp(immichService, synologyService, addonsService, mcpOrm.repo(PhotoProviders)), new NotificationsMcp(await makeNotificationsService(db, realtimeService), authService), - new AirtrailMcp(new AirtrailService(usersRepo, new AuditService(auditLogRepo, usersRepo), new AirtrailClient()), addonsService), + new AirtrailMcp(new AirtrailService(await createTestUserAirtrailRepo(db), new AuditService(auditLogRepo, usersRepo), new AirtrailClient()), addonsService), new ReservationImportMcp(new AirtrailImportService( await createTestReservationsRepo(db), await createTestReservationEndpointsRepo(db), await createTestDaysRepo(db), - realtimeService, reservationsService, new AirtrailClient(), new AirtrailService(usersRepo, new AuditService(auditLogRepo, usersRepo), new AirtrailClient()), + realtimeService, reservationsService, new AirtrailClient(), new AirtrailService(await createTestUserAirtrailRepo(db), new AuditService(auditLogRepo, usersRepo), new AirtrailClient()), await createTestUnitOfWork(db), ), await createTestTripsRepo(db), authService, guards, addonsService), new SettingsMcp(new SettingsService(await createTestUnitOfWork(db), appSettings, await createTestSettingsRepo(db)), authService), diff --git a/server/tests/helpers/test-uow.ts b/server/tests/helpers/test-uow.ts index 3ee75dcfe..bf1510627 100644 --- a/server/tests/helpers/test-uow.ts +++ b/server/tests/helpers/test-uow.ts @@ -80,6 +80,9 @@ import { CollectionPlaces } from '../../src/db/entities/CollectionPlaces.entity' import type { CollectionPlacesRepository } from '../../src/db/repositories/CollectionPlaces.repository'; import { CollectionPlaceRatings } from '../../src/db/entities/CollectionPlaceRatings.entity'; import type { CollectionPlaceRatingsRepository } from '../../src/db/repositories/CollectionPlaceRatings.repository'; +import { UserImmichRepository } from '../../src/db/repositories/UserImmich.repository'; +import { UserSynologyRepository } from '../../src/db/repositories/UserSynology.repository'; +import { UserAirtrailRepository } from '../../src/db/repositories/UserAirtrail.repository'; const perHandle = new WeakMap>(); const appSettingsPerHandle = new WeakMap>(); @@ -219,6 +222,21 @@ export function createTestUsersRepo(db: Database.Database): Promise { + return new UserImmichRepository((await sharedTestOrm(db)).orm.em); +} + +/** The Synology connection repository over the handle's shared ORM. */ +export async function createTestUserSynologyRepo(db: Database.Database): Promise { + return new UserSynologyRepository((await sharedTestOrm(db)).orm.em); +} + +/** The AirTrail connection repository over the handle's shared ORM. */ +export async function createTestUserAirtrailRepo(db: Database.Database): Promise { + return new UserAirtrailRepository((await sharedTestOrm(db)).orm.em); +} + /** * The same `UnitOfWork`, as a **global** Nest module. * diff --git a/server/tests/unit/db/repositories/UserConnections.repository.test.ts b/server/tests/unit/db/repositories/UserConnections.repository.test.ts new file mode 100644 index 000000000..045cb7975 --- /dev/null +++ b/server/tests/unit/db/repositories/UserConnections.repository.test.ts @@ -0,0 +1,216 @@ +/** + * The repositories for the third-party connections a user keeps on their own + * `users` row: Immich, Synology Photos and AirTrail. They share the table with + * UsersRepository; the Immich cases moved here with their methods, and the rest + * pin each statement's columns and its missing-user branch. + */ +import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { createSnapshotTestDb } from '../../../helpers/db-mock'; +import { resetTestDb } from '../../../helpers/test-db'; +import { createTestOrm, type TestOrm } from '../../../helpers/test-orm'; +import { createUser } from '../../../helpers/factories'; +import { Users } from '../../../../src/db/entities/Users.entity'; +import { updateRows } from '../../../helpers/factories/rows'; +import { readUser } from '../../../helpers/factories/users'; +import { UserImmichRepository } from '../../../../src/db/repositories/UserImmich.repository'; +import { UserSynologyRepository } from '../../../../src/db/repositories/UserSynology.repository'; +import { UserAirtrailRepository } from '../../../../src/db/repositories/UserAirtrail.repository'; + +const testDb = createSnapshotTestDb(); +let t: TestOrm; +let immich: UserImmichRepository; +let synology: UserSynologyRepository; +let airtrail: UserAirtrailRepository; + +beforeAll(async () => { + t = await createTestOrm(testDb); + immich = new UserImmichRepository(t.orm.em); + synology = new UserSynologyRepository(t.orm.em); + airtrail = new UserAirtrailRepository(t.orm.em); +}); +beforeEach(() => { resetTestDb(testDb); t.clear(); }); +afterAll(async () => { await t.close(); testDb.close(); }); + +/** Counts the queries MikroORM issues over the shared connection while `fn` runs. */ +async function withQueryCount(fn: () => Promise): Promise<{ value: T; queries: number }> { + const connection = t.orm.em.getConnection(); + const spy = vi.spyOn(connection, 'execute'); + try { + const value = await fn(); + return { value, queries: spy.mock.calls.length }; + } finally { + spy.mockRestore(); + } +} + +describe('UserImmichRepository — reads', () => { + it('M1: getImmichAutoUpload returns its column, and null for a missing user', async () => { + const { user } = createUser(testDb); + await updateRows(t, Users, { id: user.id }, { immich_auto_upload: 1 }); + expect(await immich.getImmichAutoUpload(user.id)).toBe(1); + expect(await immich.getImmichAutoUpload(999999)).toBeNull(); + }); + + it('M1b: getImmichCredentials / getImmichConnectionPrefs carry immich_allow_insecure_tls (#2475), and the missing-user branch', async () => { + const { user } = createUser(testDb); + await updateRows(t, Users, { id: user.id }, { immich_url: 'https://immich.test', immich_api_key: 'key-1', immich_auto_upload: 1, immich_allow_insecure_tls: 1 }); + + expect(await immich.getImmichCredentials(user.id)).toEqual({ + immich_url: 'https://immich.test', + immich_api_key: 'key-1', + immich_allow_insecure_tls: 1, + }); + expect(await immich.getImmichCredentials(999999)).toBeNull(); + + expect(await immich.getImmichConnectionPrefs(user.id)).toEqual({ immich_auto_upload: 1, immich_allow_insecure_tls: 1 }); + expect(await immich.getImmichConnectionPrefs(999999)).toBeNull(); + }); +}); + +// IM4/IM5 (`ImmichService.saveImmichSettings`, #2475): the self-signed switch +// is decided INSIDE the UPDATE by `coalesceOverrideWhileSame`, the way the +// legacy CASE did it. These pin the stored values for every branch against +// the legacy statement's own text, and that the write stays one statement — +// a read-then-write would be an interleaving window the legacy SQL never had. +describe('UserImmichRepository — Immich settings write (IM4/IM5)', () => { + const read = async (id: number) => { + const row = await readUser(t, id); + return { + immich_url: row.immich_url, immich_api_key: row.immich_api_key, immich_allow_insecure_tls: row.immich_allow_insecure_tls, + }; + }; + + it('USERSREPO-085 (IM4): setImmichSettings keeps the stored switch on a null value while the URL stays the same, and starts a new URL off', async () => { + const { user } = createUser(testDb); + await updateRows(t, Users, { id: user.id }, { immich_url: 'https://nas.local', immich_api_key: 'enc-old', immich_allow_insecure_tls: 1 }); + + // An older client that does not know the switch cannot clear it by saving. + await immich.setImmichSettings(user.id, 'https://nas.local', 'enc-new', null); + expect(await read(user.id)).toEqual({ immich_url: 'https://nas.local', immich_api_key: 'enc-new', immich_allow_insecure_tls: 1 }); + + // Sent explicitly, the value wins over the stored one. + await immich.setImmichSettings(user.id, 'https://nas.local', 'enc-new', 0); + expect((await read(user.id)).immich_allow_insecure_tls).toBe(0); + await immich.setImmichSettings(user.id, 'https://nas.local', 'enc-new', 1); + expect((await read(user.id)).immich_allow_insecure_tls).toBe(1); + + // The switch trusts one server: another URL without it starts off ... + await immich.setImmichSettings(user.id, 'https://photos.example.com', 'enc-2', null); + expect(await read(user.id)).toEqual({ immich_url: 'https://photos.example.com', immich_api_key: 'enc-2', immich_allow_insecure_tls: 0 }); + // ... and with it, holds for that server. + await immich.setImmichSettings(user.id, 'https://other.example.com', 'enc-3', 1); + expect(await read(user.id)).toEqual({ immich_url: 'https://other.example.com', immich_api_key: 'enc-3', immich_allow_insecure_tls: 1 }); + + // A first connection (no stored URL) without the switch starts off too. + await updateRows(t, Users, { id: user.id }, { immich_url: null, immich_allow_insecure_tls: 1 }); + await immich.setImmichSettings(user.id, 'https://nas.local', 'enc-4', null); + expect(await read(user.id)).toEqual({ immich_url: 'https://nas.local', immich_api_key: 'enc-4', immich_allow_insecure_tls: 0 }); + }); + + it('USERSREPO-086 (IM4): setImmichSettings matches the legacy CASE statement across every (stored url, stored switch, new url, value) combination, in ONE statement', async () => { + const { user } = createUser(testDb); + // test-sql-allow: legacy statement run raw as the parity oracle for the repository read. + const legacy = testDb.prepare( + `UPDATE users SET immich_url = ?, immich_api_key = ?, + immich_allow_insecure_tls = CASE WHEN immich_url IS ? THEN COALESCE(?, immich_allow_insecure_tls) ELSE COALESCE(?, 0) END + WHERE id = ?`, + ); + for (const storedUrl of [null, 'https://nas.local']) { + for (const storedFlag of [0, 1]) { + for (const newUrl of ['https://nas.local', 'https://photos.example.com']) { + for (const value of [null, 0, 1]) { + await updateRows(t, Users, { id: user.id }, { immich_url: storedUrl, immich_api_key: null, immich_allow_insecure_tls: storedFlag }); + legacy.run(newUrl, 'enc-key', newUrl, value, value, user.id); + const expected = await read(user.id); + + await updateRows(t, Users, { id: user.id }, { immich_url: storedUrl, immich_api_key: null, immich_allow_insecure_tls: storedFlag }); + const { queries } = await withQueryCount(() => immich.setImmichSettings(user.id, newUrl, 'enc-key', value)); + expect(queries).toBe(1); + expect(await read(user.id)).toEqual(expected); + } + } + } + } + }); + + it('USERSREPO-087 (IM5): clearImmichSettings nulls the URL, stores the key it is handed and always turns the switch off', async () => { + const { user } = createUser(testDb); + await updateRows(t, Users, { id: user.id }, { immich_url: 'https://nas.local', immich_api_key: 'enc-old', immich_allow_insecure_tls: 1 }); + await immich.clearImmichSettings(user.id, null); + expect(await read(user.id)).toEqual({ immich_url: null, immich_api_key: null, immich_allow_insecure_tls: 0 }); + }); +}); + +describe('UserSynologyRepository', () => { + it('SYNOREPO-001: getSynologyFields reads only the asked columns, and null for a missing user', async () => { + const { user } = createUser(testDb); + await updateRows(t, Users, { id: user.id }, { + synology_url: 'https://nas:5001/photo', synology_username: 'syno-login', synology_password: 'enc-pw', + synology_sid: 'sid-1', synology_did: 'did-1', synology_skip_ssl: 1, + }); + expect(await synology.getSynologyFields(user.id, ['synology_url', 'synology_sid'])).toEqual({ synology_url: 'https://nas:5001/photo', synology_sid: 'sid-1' }); + expect(await synology.getSynologyFields(user.id, ['synology_username', 'synology_password', 'synology_did', 'synology_skip_ssl'])).toEqual({ + synology_username: 'syno-login', synology_password: 'enc-pw', synology_did: 'did-1', synology_skip_ssl: 1, + }); + expect(await synology.getSynologyFields(999999, ['synology_url'])).toBeNull(); + }); + + it('SYNOREPO-002: getSynologyUsername returns its column, and null for a missing user', async () => { + const { user } = createUser(testDb); + await updateRows(t, Users, { id: user.id }, { synology_username: 'syno-login' }); + expect(await synology.getSynologyUsername(user.id)).toBe('syno-login'); + expect(await synology.getSynologyUsername(999999)).toBeNull(); + }); + + it('SYNOREPO-003: the session writes set and clear the sid and did', async () => { + const { user } = createUser(testDb); + const session = async () => { + const row = await readUser(t, user.id); + return { sid: row.synology_sid, did: row.synology_did }; + }; + await synology.setSynologySid(user.id, 'sid-2'); + await synology.setSynologyDid(user.id, 'did-2'); + expect(await session()).toEqual({ sid: 'sid-2', did: 'did-2' }); + await synology.clearSynologySID(user.id); + expect(await session()).toEqual({ sid: null, did: 'did-2' }); + await synology.setSynologySid(user.id, 'sid-3'); + await synology.clearSynologySession(user.id); + expect(await session()).toEqual({ sid: null, did: null }); + }); + + it('SYNOREPO-004: setSynologySettings writes the four connection columns as handed in', async () => { + const { user } = createUser(testDb); + await synology.setSynologySettings(user.id, 'https://nas', 'me', 'enc', 1); + const row = await readUser(t, user.id); + expect([row.synology_url, row.synology_username, row.synology_password, row.synology_skip_ssl]).toEqual(['https://nas', 'me', 'enc', 1]); + }); +}); + +describe('UserAirtrailRepository', () => { + const read = async (id: number) => { + const row = await readUser(t, id); + return { + airtrail_url: row.airtrail_url, airtrail_api_key: row.airtrail_api_key, + airtrail_allow_insecure_tls: row.airtrail_allow_insecure_tls, airtrail_write_enabled: row.airtrail_write_enabled, + }; + }; + + it('AIRTRAILREPO-001: getAirtrailConnRow and getAirtrailWriteEnabled read the connection, and null for a missing user', async () => { + const { user } = createUser(testDb); + await updateRows(t, Users, { id: user.id }, { airtrail_url: 'https://at', airtrail_api_key: 'enc', airtrail_allow_insecure_tls: 1, airtrail_write_enabled: 1 }); + expect(await airtrail.getAirtrailConnRow(user.id)).toEqual({ airtrail_url: 'https://at', airtrail_api_key: 'enc', airtrail_allow_insecure_tls: 1, airtrail_write_enabled: 1 }); + expect(await airtrail.getAirtrailConnRow(999999)).toBeNull(); + expect(await airtrail.getAirtrailWriteEnabled(user.id)).toBe(1); + expect(await airtrail.getAirtrailWriteEnabled(999999)).toBeNull(); + }); + + it('AIRTRAILREPO-002: the settings writes, with and without a new key, and the key scrub', async () => { + const { user } = createUser(testDb); + await airtrail.setAirtrailSettingsWithKey(user.id, 'https://at', 'enc-1', 0, 1); + expect(await read(user.id)).toEqual({ airtrail_url: 'https://at', airtrail_api_key: 'enc-1', airtrail_allow_insecure_tls: 0, airtrail_write_enabled: 1 }); + await airtrail.setAirtrailSettings(user.id, 'https://at2', 1, 0); + expect(await read(user.id)).toEqual({ airtrail_url: 'https://at2', airtrail_api_key: 'enc-1', airtrail_allow_insecure_tls: 1, airtrail_write_enabled: 0 }); + await airtrail.clearAirtrailApiKey(user.id); + expect((await read(user.id)).airtrail_api_key).toBeNull(); + }); +}); diff --git a/server/tests/unit/db/repositories/Users.repository.test.ts b/server/tests/unit/db/repositories/Users.repository.test.ts index 2fc8e67e6..49b0f7976 100644 --- a/server/tests/unit/db/repositories/Users.repository.test.ts +++ b/server/tests/unit/db/repositories/Users.repository.test.ts @@ -5,7 +5,6 @@ import { createTestOrm, type TestOrm } from '../../../helpers/test-orm'; import { createUser, createAdmin, type TestUser } from '../../../helpers/factories'; import { Users } from '../../../../src/db/entities/Users.entity'; import { findRow, updateRows } from '../../../helpers/factories/rows'; -import { readUser } from '../../../helpers/factories/users'; import type { UsersRepository } from '../../../../src/db/repositories/Users.repository'; import { UserIdentityTakenError } from '../../../../src/db/repositories/Users.repository'; @@ -1008,34 +1007,12 @@ describe('UsersRepository — feed tokens (Plan 3d Task 5, FD5-FD8/FD10)', () => expect(await users.findIdAndUsernameByFeedToken('')).toBeUndefined(); }); - it('M1: getImmichAutoUpload / getSynologyUsername / findUsernameEmail return their column, and the missing-user branch, honestly', async () => { + it('M1: findUsernameEmail returns its columns, and the missing-user branch, honestly', async () => { const { user } = createUser(testDb, { username: 'imm-user', email: 'imm@example.com' }); - await updateRows(t, Users, { id: user.id }, { immich_auto_upload: 1, synology_username: 'syno-login' }); - - expect(await users.getImmichAutoUpload(user.id)).toBe(1); - expect(await users.getImmichAutoUpload(999999)).toBeNull(); - - expect(await users.getSynologyUsername(user.id)).toBe('syno-login'); - expect(await users.getSynologyUsername(999999)).toBeNull(); expect(await users.findUsernameEmail(user.id)).toEqual({ username: 'imm-user', email: 'imm@example.com' }); expect(await users.findUsernameEmail(999999)).toBeUndefined(); }); - - it('M1b: getImmichCredentials / getImmichConnectionPrefs carry immich_allow_insecure_tls (#2475), and the missing-user branch', async () => { - const { user } = createUser(testDb); - await updateRows(t, Users, { id: user.id }, { immich_url: 'https://immich.test', immich_api_key: 'key-1', immich_auto_upload: 1, immich_allow_insecure_tls: 1 }); - - expect(await users.getImmichCredentials(user.id)).toEqual({ - immich_url: 'https://immich.test', - immich_api_key: 'key-1', - immich_allow_insecure_tls: 1, - }); - expect(await users.getImmichCredentials(999999)).toBeNull(); - - expect(await users.getImmichConnectionPrefs(user.id)).toEqual({ immich_auto_upload: 1, immich_allow_insecure_tls: 1 }); - expect(await users.getImmichConnectionPrefs(999999)).toBeNull(); - }); }); // Plan 3i Task 4 fix wave (should-land 5): admin's AD1-AD17 read methods @@ -1141,80 +1118,6 @@ describe('UsersRepository — admin (AD1-AD17) read methods, full-key parity', ( }); }); -// IM4/IM5 (`ImmichService.saveImmichSettings`, #2475): the self-signed switch -// is decided INSIDE the UPDATE by `coalesceOverrideWhileSame`, the way the -// legacy CASE did it. These pin the stored values for every branch against -// the legacy statement's own text, and that the write stays one statement — -// a read-then-write would be an interleaving window the legacy SQL never had. -describe('UsersRepository — Immich settings write (IM4/IM5)', () => { - const read = async (id: number) => { - const row = await readUser(t, id); - return { - immich_url: row.immich_url, immich_api_key: row.immich_api_key, immich_allow_insecure_tls: row.immich_allow_insecure_tls, - }; - }; - - it('USERSREPO-085 (IM4): setImmichSettings keeps the stored switch on a null value while the URL stays the same, and starts a new URL off', async () => { - const { user } = createUser(testDb); - await updateRows(t, Users, { id: user.id }, { immich_url: 'https://nas.local', immich_api_key: 'enc-old', immich_allow_insecure_tls: 1 }); - - // An older client that does not know the switch cannot clear it by saving. - await users.setImmichSettings(user.id, 'https://nas.local', 'enc-new', null); - expect(await read(user.id)).toEqual({ immich_url: 'https://nas.local', immich_api_key: 'enc-new', immich_allow_insecure_tls: 1 }); - - // Sent explicitly, the value wins over the stored one. - await users.setImmichSettings(user.id, 'https://nas.local', 'enc-new', 0); - expect((await read(user.id)).immich_allow_insecure_tls).toBe(0); - await users.setImmichSettings(user.id, 'https://nas.local', 'enc-new', 1); - expect((await read(user.id)).immich_allow_insecure_tls).toBe(1); - - // The switch trusts one server: another URL without it starts off ... - await users.setImmichSettings(user.id, 'https://photos.example.com', 'enc-2', null); - expect(await read(user.id)).toEqual({ immich_url: 'https://photos.example.com', immich_api_key: 'enc-2', immich_allow_insecure_tls: 0 }); - // ... and with it, holds for that server. - await users.setImmichSettings(user.id, 'https://other.example.com', 'enc-3', 1); - expect(await read(user.id)).toEqual({ immich_url: 'https://other.example.com', immich_api_key: 'enc-3', immich_allow_insecure_tls: 1 }); - - // A first connection (no stored URL) without the switch starts off too. - await updateRows(t, Users, { id: user.id }, { immich_url: null, immich_allow_insecure_tls: 1 }); - await users.setImmichSettings(user.id, 'https://nas.local', 'enc-4', null); - expect(await read(user.id)).toEqual({ immich_url: 'https://nas.local', immich_api_key: 'enc-4', immich_allow_insecure_tls: 0 }); - }); - - it('USERSREPO-086 (IM4): setImmichSettings matches the legacy CASE statement across every (stored url, stored switch, new url, value) combination, in ONE statement', async () => { - const { user } = createUser(testDb); - // test-sql-allow: legacy statement run raw as the parity oracle for the repository read. - const legacy = testDb.prepare( - `UPDATE users SET immich_url = ?, immich_api_key = ?, - immich_allow_insecure_tls = CASE WHEN immich_url IS ? THEN COALESCE(?, immich_allow_insecure_tls) ELSE COALESCE(?, 0) END - WHERE id = ?`, - ); - for (const storedUrl of [null, 'https://nas.local']) { - for (const storedFlag of [0, 1]) { - for (const newUrl of ['https://nas.local', 'https://photos.example.com']) { - for (const value of [null, 0, 1]) { - await updateRows(t, Users, { id: user.id }, { immich_url: storedUrl, immich_api_key: null, immich_allow_insecure_tls: storedFlag }); - legacy.run(newUrl, 'enc-key', newUrl, value, value, user.id); - const expected = await read(user.id); - - await updateRows(t, Users, { id: user.id }, { immich_url: storedUrl, immich_api_key: null, immich_allow_insecure_tls: storedFlag }); - const { queries } = await withQueryCount(() => users.setImmichSettings(user.id, newUrl, 'enc-key', value)); - expect(queries).toBe(1); - expect(await read(user.id)).toEqual(expected); - } - } - } - } - }); - - it('USERSREPO-087 (IM5): clearImmichSettings nulls the URL, stores the key it is handed and always turns the switch off', async () => { - const { user } = createUser(testDb); - await updateRows(t, Users, { id: user.id }, { immich_url: 'https://nas.local', immich_api_key: 'enc-old', immich_allow_insecure_tls: 1 }); - await users.clearImmichSettings(user.id, null); - expect(await read(user.id)).toEqual({ immich_url: null, immich_api_key: null, immich_allow_insecure_tls: 0 }); - }); -}); - describe('a users write the unique indexes refuse', () => { const row = (username: string, email: string) => ({ username, email, password_hash: 'hashed', role: 'user', first_seen_version: '1.0.0', diff --git a/server/tests/unit/db/users-repository-shape.test.ts b/server/tests/unit/db/users-repository-shape.test.ts new file mode 100644 index 000000000..bd111576a --- /dev/null +++ b/server/tests/unit/db/users-repository-shape.test.ts @@ -0,0 +1,39 @@ +/** + * UsersRepository had grown to 84 methods, a fifth of them for the third-party + * connections a user keeps on their row. Those now live in their own + * repositories on the same table (UserImmich/UserSynology/UserAirtrail). This + * pins the account repository's size so a new connection's statements land in + * a repository of their own, and pins which columns each connection repository + * is about, so a statement on another column does not drift into it. + */ +import { describe, expect, it } from 'vitest'; +import { UsersRepository } from '../../../src/db/repositories/Users.repository'; +import { UserImmichRepository } from '../../../src/db/repositories/UserImmich.repository'; +import { UserSynologyRepository } from '../../../src/db/repositories/UserSynology.repository'; +import { UserAirtrailRepository } from '../../../src/db/repositories/UserAirtrail.repository'; + +const ownMethods = (cls: { prototype: object }) => + Object.getOwnPropertyNames(cls.prototype).filter((name) => name !== 'constructor').sort(); + +describe('the users repositories', () => { + it('USERSHAPE-001: UsersRepository holds no more methods than it does today', () => { + // Lower this when a method leaves; a new integration gets its own repository. + expect(ownMethods(UsersRepository).length).toBeLessThanOrEqual(66); + }); + + it('USERSHAPE-002: no connection statement is left on UsersRepository', () => { + expect(ownMethods(UsersRepository).filter((name) => /immich|synology|airtrail/i.test(name))).toEqual([]); + }); + + it('USERSHAPE-003: each connection repository holds its own statements', () => { + expect(ownMethods(UserImmichRepository)).toEqual( + ['clearImmichSettings', 'getImmichAutoUpload', 'getImmichConnectionPrefs', 'getImmichCredentials', 'setImmichAutoUpload', 'setImmichSettings'], + ); + expect(ownMethods(UserSynologyRepository)).toEqual( + ['clearSynologySID', 'clearSynologySession', 'getSynologyFields', 'getSynologyUsername', 'setSynologyDid', 'setSynologySettings', 'setSynologySid'], + ); + expect(ownMethods(UserAirtrailRepository)).toEqual( + ['clearAirtrailApiKey', 'getAirtrailConnRow', 'getAirtrailWriteEnabled', 'setAirtrailSettings', 'setAirtrailSettingsWithKey'], + ); + }); +}); diff --git a/server/tests/unit/nest/airtrail.service.test.ts b/server/tests/unit/nest/airtrail.service.test.ts index 844480cb2..5738e6f7e 100644 --- a/server/tests/unit/nest/airtrail.service.test.ts +++ b/server/tests/unit/nest/airtrail.service.test.ts @@ -28,6 +28,7 @@ import { readUser } from '../../helpers/factories/users'; import { Users } from '../../../src/db/entities/Users.entity'; import { AuditLog } from '../../../src/db/entities/AuditLog.entity'; import type { UsersRepository } from '../../../src/db/repositories/Users.repository'; +import { UserAirtrailRepository } from '../../../src/db/repositories/UserAirtrail.repository'; import { AuditService } from '../../../src/nest/audit/audit.service'; import { AirtrailService } from '../../../src/nest/integrations/airtrail.service'; import type { AirtrailClient } from '../../../src/nest/integrations/airtrail.client'; @@ -41,7 +42,7 @@ const listFlights = vi.fn(); beforeAll(async () => { t = await createTestOrm(testDb); usersRepo = t.repo(Users); - svc = new AirtrailService(usersRepo, new AuditService(t.repo(AuditLog), usersRepo), { listFlights } as unknown as AirtrailClient); + svc = new AirtrailService(new UserAirtrailRepository(t.orm.em), new AuditService(t.repo(AuditLog), usersRepo), { listFlights } as unknown as AirtrailClient); }); beforeEach(() => { resetTestDb(testDb); diff --git a/server/tests/unit/nest/immich.service.test.ts b/server/tests/unit/nest/immich.service.test.ts index dc86dff07..bc045e489 100644 --- a/server/tests/unit/nest/immich.service.test.ts +++ b/server/tests/unit/nest/immich.service.test.ts @@ -47,7 +47,7 @@ import type { MemoriesAccessService } from '../../../src/nest/memories/memories- import fs from 'node:fs'; import path from 'node:path'; import { makeStorageFixture } from '../../helpers/storage-fixture'; -import { createTestUnitOfWork, createTestUsersRepo, sharedTestOrm } from '../../helpers/test-uow'; +import { createTestUnitOfWork, createTestUserImmichRepo, sharedTestOrm } from '../../helpers/test-uow'; import { deleteRows, upsertRow } from '../../helpers/factories/rows'; import { readUser } from '../../helpers/factories/users'; import { Users } from '../../../src/db/entities/Users.entity'; @@ -92,8 +92,8 @@ function upstream(opts: { ok?: boolean; status?: number; json?: unknown; url?: s } beforeAll(async () => { - const users = await createTestUsersRepo(testDb); - svc = new ImmichService(audit as unknown as AuditService, access as unknown as MemoriesAccessService, journeyFx.storage, users, await createTestUnitOfWork(testDb)); + const immichRepo = await createTestUserImmichRepo(testDb); + svc = new ImmichService(audit as unknown as AuditService, access as unknown as MemoriesAccessService, journeyFx.storage, immichRepo, await createTestUnitOfWork(testDb)); }); beforeEach(async () => { diff --git a/server/tests/unit/nest/synology.service.test.ts b/server/tests/unit/nest/synology.service.test.ts index 21c89e2c6..cf37e916c 100644 --- a/server/tests/unit/nest/synology.service.test.ts +++ b/server/tests/unit/nest/synology.service.test.ts @@ -48,7 +48,7 @@ import { db as testDb } from '../../../src/db/database'; import { SynologyService } from '../../../src/nest/memories/synology.service'; import type { MemoriesAccessService } from '../../../src/nest/memories/memories-access.service'; import { notificationsStub } from '../../helpers/notifications'; -import { createTestUsersRepo, sharedTestOrm } from '../../helpers/test-uow'; +import { createTestUserSynologyRepo, sharedTestOrm } from '../../helpers/test-uow'; import { deleteRows, upsertRow } from '../../helpers/factories/rows'; import { readUser } from '../../helpers/factories/users'; import { Users } from '../../../src/db/entities/Users.entity'; @@ -91,8 +91,7 @@ function httpError(status: number) { } beforeAll(async () => { - const users = await createTestUsersRepo(testDb); - svc = new SynologyService(access as unknown as MemoriesAccessService, notificationsStub(), users); + svc = new SynologyService(access as unknown as MemoriesAccessService, notificationsStub(), await createTestUserSynologyRepo(testDb)); }); beforeEach(async () => { diff --git a/server/tests/unit/services/airtrailTestConnection.test.ts b/server/tests/unit/services/airtrailTestConnection.test.ts index 307fd6b1e..f3f0096f7 100644 --- a/server/tests/unit/services/airtrailTestConnection.test.ts +++ b/server/tests/unit/services/airtrailTestConnection.test.ts @@ -14,6 +14,7 @@ import { AuditService } from '../../../src/nest/audit/audit.service'; import { createTestOrm, type TestOrm } from '../../helpers/test-orm'; import { AuditLog } from '../../../src/db/entities/AuditLog.entity'; import { Users } from '../../../src/db/entities/Users.entity'; +import { UserAirtrailRepository } from '../../../src/db/repositories/UserAirtrail.repository'; // The probe is the only call that would leave the process, so the client is a // stub; the credential handling around it runs against the real row. @@ -24,7 +25,7 @@ let t: TestOrm; beforeAll(async () => { t = await createTestOrm(db); svc = new AirtrailService( - t.repo(Users), + new UserAirtrailRepository(t.orm.em), new AuditService(t.repo(AuditLog), t.repo(Users)), { listFlights } as unknown as AirtrailClient, ); diff --git a/server/tests/unit/services/airtrailWriteback.test.ts b/server/tests/unit/services/airtrailWriteback.test.ts index 21b5837c0..cfe8af2aa 100644 --- a/server/tests/unit/services/airtrailWriteback.test.ts +++ b/server/tests/unit/services/airtrailWriteback.test.ts @@ -18,6 +18,7 @@ import { AuditService } from '../../../src/nest/database/../audit/audit.service' import { createTestOrm, type TestOrm } from '../../helpers/test-orm'; import { AuditLog } from '../../../src/db/entities/AuditLog.entity'; import { Users } from '../../../src/db/entities/Users.entity'; +import { UserAirtrailRepository } from '../../../src/db/repositories/UserAirtrail.repository'; // The free functions became methods with the airtrail fold; same SQL, same // behaviour, one instance over the same db handle. @@ -30,7 +31,7 @@ let t: TestOrm; beforeAll(async () => { t = await createTestOrm(db); svc = new AirtrailService( - t.repo(Users), + new UserAirtrailRepository(t.orm.em), new AuditService(t.repo(AuditLog), t.repo(Users)), new AirtrailClient(), ); @@ -70,23 +71,23 @@ describe('airtrail saveSettings preserves its pre-existing NO-transaction asymme it('ATC-TX-001: AirtrailService is constructed with no UnitOfWork at all — saveSettings has nothing to wrap its writes in', () => { // `DawarichService#saveSettings`'s equivalent DOES take a `UnitOfWork` and // wraps its writes in `uow.transactional`; AirtrailService's own - // constructor (`UsersRepository`, `AuditService`, `AirtrailClient`) never + // constructor (`UserAirtrailRepository`, `AuditService`, `AirtrailClient`) never // gained one during the Plan 3h Task 4 conversion — a structural // guarantee that a future edit re-introducing a transactional wrap here // would have to ALSO add a new constructor param to do it, not just // touch `saveSettings`'s body. const paramTypes = Reflect.getMetadata('design:paramtypes', AirtrailService) as unknown[] | undefined; expect(paramTypes).toBeDefined(); - expect(paramTypes!.map((p) => (p as { name?: string })?.name)).toEqual(['UsersRepository', 'AuditService', 'AirtrailClient']); + expect(paramTypes!.map((p) => (p as { name?: string })?.name)).toEqual(['UserAirtrailRepository', 'AuditService', 'AirtrailClient']); }); - it('ATC-TX-002: the URL-cleared branch performs its two writes as genuinely separate UsersRepository calls, not one atomic statement', async () => { + it('ATC-TX-002: the URL-cleared branch performs its two writes as genuinely separate repository calls, not one atomic statement', async () => { const { user } = createUser(db); await saveSettings(user.id, 'https://at.example.com', 'secret-key', false, false, null); - const usersRepo = t.repo(Users); - const setSpy = vi.spyOn(usersRepo, 'setAirtrailSettings'); - const clearSpy = vi.spyOn(usersRepo, 'clearAirtrailApiKey'); + const airtrailRepo = (svc as unknown as { airtrailRepo: UserAirtrailRepository }).airtrailRepo; + const setSpy = vi.spyOn(airtrailRepo, 'setAirtrailSettings'); + const clearSpy = vi.spyOn(airtrailRepo, 'clearAirtrailApiKey'); try { // Clearing the URL with no key left drops the key too (ATC4 then ATC5) — // TWO independent nativeUpdate calls, not a single transactional unit.