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.