diff --git a/server/src/db/repositories/JourneyContributors.repository.ts b/server/src/db/repositories/JourneyContributors.repository.ts index 0a9b457be..f276f0c0c 100644 --- a/server/src/db/repositories/JourneyContributors.repository.ts +++ b/server/src/db/repositories/JourneyContributors.repository.ts @@ -131,4 +131,19 @@ export class JourneyContributorsRepository extends TrekRepository { return await this.nativeDelete({ journey: journeyId, user: userId, role: { $ne: 'owner' } }); } + + /** + * UC10 (Plan 3g Task 4 survivor, `UserCleanupService.cleanupUserReferences`) + * — `DELETE FROM journey_contributors WHERE user_id = ?`: contributor rows + * on OTHER users' journeys (not covered by UC8's cascade). Unscoped by + * journey and carries no `role != 'owner'` guard, unlike {@link + * deleteNonOwner}/JG119 — by the call order UC8 already ran first and + * deleted every journey this user owns (cascading away their own + * `role='owner'` row on each), so no remaining row for this user can be + * `role='owner'`; the absent guard is preserved exactly, not added + * defensively. + */ + async deleteAllForUser(userId: number): Promise { + await this.nativeDelete({ user: userId }); + } } diff --git a/server/src/db/repositories/JourneyEntries.repository.ts b/server/src/db/repositories/JourneyEntries.repository.ts index 6af4d53ac..4af644cc7 100644 --- a/server/src/db/repositories/JourneyEntries.repository.ts +++ b/server/src/db/repositories/JourneyEntries.repository.ts @@ -147,6 +147,44 @@ export class JourneyEntriesRepository extends TrekRepository { ); } + /** + * TP32 (Plan 3g Task 4 survivor, `TripsService.remove`) — `DELETE FROM + * journey_entries WHERE source_trip_id = ? AND type = 'skeleton'`. NO + * `journey_id` scope, unlike {@link deleteSkeletonsForTrip}/JG31 — a trip + * delete drops every skeleton synced from it across every journey it was + * ever linked to, not just one. A genuinely different statement text, not + * a call to {@link deleteSkeletonsForTrip}. + */ + async deleteAllSkeletonsForTrip(tripId: number): Promise { + await this.nativeDelete({ sourceTrip: tripId, type: 'skeleton' }); + } + + /** + * TP33 (Plan 3g Task 4 survivor, `TripsService.remove`) — `UPDATE + * journey_entries SET source_trip_id=NULL, source_place_id=NULL, + * source_assignment_id=NULL WHERE source_trip_id = ?`. NO `journey_id` + * scope AND no `type != 'skeleton'` filter, unlike {@link + * detachFilledForTrip}/JG32 — by the time this runs, {@link + * deleteAllSkeletonsForTrip}/TP32 has already removed every skeleton row + * with this `source_trip_id` (same transaction, called first in + * `TripsService.remove`), so every row this statement can still match is + * already non-skeleton; the missing filter is preserved exactly, not + * "fixed" into a redundant one. + */ + async detachAllFilledForTrip(tripId: number): Promise { + await this.nativeUpdate({ sourceTrip: tripId }, { sourcePlace: null, sourceTrip: null, source_assignment_id: null }); + } + + /** + * UC9 (Plan 3g Task 4 survivor, `UserCleanupService.cleanupUserReferences`) + * — `DELETE FROM journey_entries WHERE author_id = ?`: entries this user + * authored on OTHER users' journeys (not covered by UC8's cascade, since + * those journeys aren't owned by the departing user). + */ + async deleteByAuthorId(userId: number): Promise { + await this.nativeDelete({ author: userId }); + } + /** JG35 — `syncTripPlaces`'s existing-skeleton dedup-key read: `SELECT source_place_id, source_assignment_id FROM journey_entries WHERE journey_id = ? AND source_trip_id = ?`. */ async listSourceKeysForTrip(journeyId: number, tripId: number): Promise<{ source_place_id: number; source_assignment_id: number | null }[]> { return await this.qb('je') diff --git a/server/src/db/repositories/JourneyPhotos.repository.ts b/server/src/db/repositories/JourneyPhotos.repository.ts index 85b0cdbab..d9f75bd7c 100644 --- a/server/src/db/repositories/JourneyPhotos.repository.ts +++ b/server/src/db/repositories/JourneyPhotos.repository.ts @@ -117,6 +117,58 @@ export class JourneyPhotosRepository extends TrekRepository { return await this.count({ journey: journeyId }); } + /** + * PH10 (Plan 3g Task 4 survivor, `TrekPhotoRegistrationService.deleteIfOrphan`) + * — `SELECT 1 FROM journey_photos WHERE photo_id = ?`: the `journey_photos` + * half of PH10's split orphan-check (the `trip_photos` half is 3e's own, + * already converted onto `TripPhotosRepository.existsForPhoto`). + */ + async existsForPhoto(photoId: number): Promise { + const row = await this.qb('gp').select(['gp.id']).where({ photo: photoId }).execute<{ id: number } | undefined>('get', false); + return !!row; + } + + /** + * MA1 (Plan 3g Task 4 survivor, `MemoriesAccessService.canAccessUserPhoto`'s + * journey-photo half) — `SELECT gp.journey_id FROM journey_photos gp JOIN + * trek_photos tkp ON tkp.id=gp.photo_id WHERE tkp.asset_id=? AND + * tkp.provider=? AND tkp.owner_id=? LIMIT 1`. Security-critical: returns + * only the FIRST matching row's `journey_id` (the legacy statement's own + * `LIMIT 1`), not every journey the asset might appear in — preserved + * exactly, not "fixed" into a list. + */ + async findJourneyIdForAsset(assetId: string, provider: string, ownerId: number): Promise { + const row = await this.kysely() + .selectFrom('journey_photos as gp') + .innerJoin('trek_photos as tkp', 'tkp.id', 'gp.photo_id') + .select('gp.journey_id') + .where('tkp.asset_id', '=', assetId) + .where('tkp.provider', '=', provider) + .where('tkp.owner_id', '=', ownerId) + .executeTakeFirst(); + return row?.journey_id; + } + + /** + * MA6 (Plan 3g Task 4 survivor, `MemoriesAccessService.canAccessTrekPhoto`'s + * unified-photo-access read) — the outer `SELECT 1 FROM journey_photos gp + * WHERE gp.photo_id=?` half of the original correlated `EXISTS` statement; + * returns every DISTINCT `journey_id` a `journey_photos` row links this + * `trek_photos.id` to (a photo can be added to more than one journey's + * gallery), so the caller can probe owner-or-contributor access against + * each one in turn — same loop shape `MemoriesAccessService`'s own MA5 + * (`sharedTripIds`/`findAccessible`) already uses for the trip-photo half. + */ + async listJourneyIdsForPhoto(photoId: number): Promise { + const rows = await this.kysely() + .selectFrom('journey_photos') + .select('journey_id') + .distinct() + .where('photo_id', '=', photoId) + .execute(); + return rows.map((r) => r.journey_id); + } + /** JG88/JG99 — `ensureInGallery`'s and `uploadGalleryPhotos`'s next-sort-order probe: `SELECT MAX(sort_order) as m FROM journey_photos WHERE journey_id = ?`, one statement text. */ async maxSortOrder(journeyId: number): Promise { const row = await this.kysely() diff --git a/server/src/db/repositories/JourneyShareTokens.repository.ts b/server/src/db/repositories/JourneyShareTokens.repository.ts index dbe6b1791..c5e6cf01b 100644 --- a/server/src/db/repositories/JourneyShareTokens.repository.ts +++ b/server/src/db/repositories/JourneyShareTokens.repository.ts @@ -260,6 +260,15 @@ export class JourneyShareTokensRepository extends TrekRepository { + await this.nativeDelete({ createdByRef: userId }); + } + /** * JS6/JS9 — the public photo/asset validators' token lookup: `SELECT * journey_id, share_gallery FROM journey_share_tokens WHERE token = ?`. diff --git a/server/src/db/repositories/Journeys.repository.ts b/server/src/db/repositories/Journeys.repository.ts index 9f91fe50d..c55a634ba 100644 --- a/server/src/db/repositories/Journeys.repository.ts +++ b/server/src/db/repositories/Journeys.repository.ts @@ -236,4 +236,14 @@ export class JourneysRepository extends TrekRepository { async deleteById(id: number): Promise { await this.nativeDelete({ id }); } + + /** + * UC8 (Plan 3g Task 4 survivor, `UserCleanupService.cleanupUserReferences`) + * — `DELETE FROM journeys WHERE user_id = ?`: every journey this user + * owns, cascading to its entries/contributors/share_tokens/photos via the + * already-declared `deleteRule('cascade')` FKs. + */ + async deleteOwnedByUser(userId: number): Promise { + await this.nativeDelete({ user: userId }); + } } diff --git a/server/src/nest/auth/auth.module.ts b/server/src/nest/auth/auth.module.ts index bb57d3819..7300991d6 100644 --- a/server/src/nest/auth/auth.module.ts +++ b/server/src/nest/auth/auth.module.ts @@ -10,6 +10,10 @@ import { InviteTokens } from '../../db/entities/InviteTokens.entity'; import { McpTokens } from '../../db/entities/McpTokens.entity'; import { OauthTokens } from '../../db/entities/OauthTokens.entity'; import { PasswordResetTokens } from '../../db/entities/PasswordResetTokens.entity'; +import { JourneyShareTokens } from '../../db/entities/JourneyShareTokens.entity'; +import { Journeys } from '../../db/entities/Journeys.entity'; +import { JourneyEntries } from '../../db/entities/JourneyEntries.entity'; +import { JourneyContributors } from '../../db/entities/JourneyContributors.entity'; import { TokensModule } from '../tokens/tokens.module'; import { AuthPublicController } from './auth-public.controller'; import { AuthController } from './auth.controller'; @@ -84,7 +88,12 @@ import { AllowedFileTypesModule } from '../files/allowed-file-types.module'; // resolves from THIS module's own `forFeature` graph regardless of who // else also registers it (`tokens.module.ts`'s own docstring explains // why). PasswordResetTokens: AuthService's own table (Plan 3b Task 5). - MikroOrmModule.forFeature([AppSettings, Users, WebauthnCredentials, WebauthnChallenges, InviteTokens, McpTokens, OauthTokens, PasswordResetTokens, BudgetItems])], + // JourneyShareTokens/Journeys/JourneyEntries/JourneyContributors: Plan + // 3g Task 4's own addition — `UserCleanupService.cleanupUserReferences`'s + // UC7-10 GDPR-erasure deletes, reached the same cross-domain way as + // McpTokens/OauthTokens above (owned by `nest/journey`, registered here + // for THIS module's own `@InjectRepository` params). + MikroOrmModule.forFeature([AppSettings, Users, WebauthnCredentials, WebauthnChallenges, InviteTokens, McpTokens, OauthTokens, PasswordResetTokens, BudgetItems, JourneyShareTokens, Journeys, JourneyEntries, JourneyContributors])], controllers: [AuthPublicController, AuthController, PasskeyController], providers: [AuthService, UserProfileService, RegistrationInvitesService, PasskeyService, UserCleanupService, WebauthnConfigService, AuthMcp], exports: [AuthService, RegistrationInvitesService, PasskeyService, UserCleanupService], diff --git a/server/src/nest/auth/user-cleanup.service.ts b/server/src/nest/auth/user-cleanup.service.ts index 96365aad9..0dfe5c649 100644 --- a/server/src/nest/auth/user-cleanup.service.ts +++ b/server/src/nest/auth/user-cleanup.service.ts @@ -12,6 +12,14 @@ import { Users } from '../../db/entities/Users.entity'; import type { UsersRepository } from '../../db/repositories/Users.repository'; import { BudgetItems } from '../../db/entities/BudgetItems.entity'; import type { BudgetItemsRepository } from '../../db/repositories/BudgetItems.repository'; +import { JourneyShareTokens } from '../../db/entities/JourneyShareTokens.entity'; +import type { JourneyShareTokensRepository } from '../../db/repositories/JourneyShareTokens.repository'; +import { Journeys } from '../../db/entities/Journeys.entity'; +import type { JourneysRepository } from '../../db/repositories/Journeys.repository'; +import { JourneyEntries } from '../../db/entities/JourneyEntries.entity'; +import type { JourneyEntriesRepository } from '../../db/repositories/JourneyEntries.repository'; +import { JourneyContributors } from '../../db/entities/JourneyContributors.entity'; +import type { JourneyContributorsRepository } from '../../db/repositories/JourneyContributors.repository'; /** * Account erasure — everything that has to happen around `DELETE FROM users` @@ -34,6 +42,13 @@ export class UserCleanupService { @InjectRepository(Users) private readonly usersRepo: UsersRepository, // Plan 3e Task 2 (budget) — additive, UC5 only. @InjectRepository(BudgetItems) private readonly budgetItemsRepo: BudgetItemsRepository, + // Plan 3g Task 4 (UC7-10) — the genuinely-journey GDPR-erasure deletes. + // UC6 (`share_tokens`, `nest/share`, Plan 3h) is NOT one of these — see + // `cleanupUserReferences`'s own doc comment. + @InjectRepository(JourneyShareTokens) private readonly journeyShareTokensRepo: JourneyShareTokensRepository, + @InjectRepository(Journeys) private readonly journeysRepo: JourneysRepository, + @InjectRepository(JourneyEntries) private readonly journeyEntriesRepo: JourneyEntriesRepository, + @InjectRepository(JourneyContributors) private readonly journeyContributorsRepo: JourneyContributorsRepository, ) {} /** @@ -82,27 +97,35 @@ export class UserCleanupService { } /** - * UC4–UC10: every table here is owned by a domain outside this plan — - * `trip_members` (`nest/trip-membership`, Plan 3c), `budget_items` - * (`nest/budget`, Plan 3e), `share_tokens`/`journey_share_tokens`/ - * `journeys`/`journey_entries`/`journey_contributors` (`nest/share`/ - * `nest/journey-share`/`nest/journey-domain`, Plan 3g) — so they stay raw - * `DatabaseService` calls (Plan 3b Task 5 ruling; inventory §6). Verbatim - * otherwise: same statement order, same bounded 3-table - * journey/journey_entries/journey_contributors sequence, - * `this.budget.removeUserFromBudgetItems` unchanged. + * UC4–UC10: `trip_members` (`nest/trip-membership`, Plan 3c) and + * `budget_items` (`nest/budget`, Plan 3e) are owned by domains outside + * this plan, so UC4 stays a raw `DatabaseService` call (Plan 3b Task 5 + * ruling; inventory §6) — UC5 is already converted (`budgetItemsRepo + * .clearPaidByUser`). `share_tokens` (UC6) belongs to `nest/share` + * (trip-level share links, Plan 3h) — a genuinely different table from + * the four below despite sharing this method and this erasure + * transaction, so it ALSO stays a raw `DatabaseService` call, but for a + * different reason than UC4 (not this plan's table at all, not merely + * "another plan hasn't converted it yet"). `journey_share_tokens`/ + * `journeys`/`journey_entries`/`journey_contributors` (UC7-10, + * `nest/journey-share`/`nest/journey-domain`, Plan 3g) ARE this plan's + * own and are converted (Plan 3g Task 4) onto their owning repositories. + * Verbatim otherwise: same statement order, same bounded 4-table + * share_tokens/journey_share_tokens/journeys/journey_entries/ + * journey_contributors sequence, `this.budget.removeUserFromBudgetItems` + * unchanged. */ private async cleanupUserReferences(userId: number): Promise { this.db.run('UPDATE trip_members SET invited_by = NULL WHERE invited_by = ?', userId); // UC4 — Plan 3c await this.budgetItemsRepo.clearPaidByUser(userId); // UC5 — Plan 3e Task 2, converted. await this.budget.removeUserFromBudgetItems(userId); - this.db.run('DELETE FROM share_tokens WHERE created_by = ?', userId); // UC6 — Plan 3g - this.db.run('DELETE FROM journey_share_tokens WHERE created_by = ?', userId); // UC7 — Plan 3g + this.db.run('DELETE FROM share_tokens WHERE created_by = ?', userId); // UC6 — Plan 3h + await this.journeyShareTokensRepo.deleteByCreatedBy(userId); // UC7 — converted (Plan 3g Task 4) // Owned journeys cascade-delete their entries/contributors/share_tokens/photos via journey_id FKs - this.db.run('DELETE FROM journeys WHERE user_id = ?', userId); // UC8 — Plan 3g + await this.journeysRepo.deleteOwnedByUser(userId); // UC8 — converted (Plan 3g Task 4) // Entries authored on other users' journeys (not covered by the cascade above) - this.db.run('DELETE FROM journey_entries WHERE author_id = ?', userId); // UC9 — Plan 3g - this.db.run('DELETE FROM journey_contributors WHERE user_id = ?', userId); // UC10 — Plan 3g + await this.journeyEntriesRepo.deleteByAuthorId(userId); // UC9 — converted (Plan 3g Task 4) + await this.journeyContributorsRepo.deleteAllForUser(userId); // UC10 — converted (Plan 3g Task 4) } /** diff --git a/server/src/nest/memories/memories-access.service.ts b/server/src/nest/memories/memories-access.service.ts index 68faa85ae..d57b2c5b9 100644 --- a/server/src/nest/memories/memories-access.service.ts +++ b/server/src/nest/memories/memories-access.service.ts @@ -12,6 +12,12 @@ import { TripAlbumLinks } from '../../db/entities/TripAlbumLinks.entity'; import type { TripAlbumLinksRepository } from '../../db/repositories/TripAlbumLinks.repository'; import { Trips } from '../../db/entities/Trips.entity'; import type { TripsRepository } from '../../db/repositories/Trips.repository'; +import { Journeys } from '../../db/entities/Journeys.entity'; +import type { JourneysRepository } from '../../db/repositories/Journeys.repository'; +import { JourneyContributors } from '../../db/entities/JourneyContributors.entity'; +import type { JourneyContributorsRepository } from '../../db/repositories/JourneyContributors.repository'; +import { JourneyPhotos } from '../../db/entities/JourneyPhotos.entity'; +import type { JourneyPhotosRepository } from '../../db/repositories/JourneyPhotos.repository'; /** * Who may see which photo, and the album-link lookups the provider syncs need. @@ -24,9 +30,14 @@ import type { TripsRepository } from '../../db/repositories/Trips.repository'; * Stays a service, not a single-table repository (Plan 3e): it composes * access logic across `trip_photos`/`trek_photos`/`trip_album_links` * (this plan's own tables) and `journeys`/`journey_contributors`/ - * `journey_photos` (Plan 3g's, not yet converted — every read against them - * below stays raw, marked `// — Plan 3g`). `DatabaseService` stays - * injected for exactly those raw reads and for the `canAccessTrip` primitive + * `journey_photos` (Plan 3g's — MA1/MA2/MA6, converted by Plan 3g Task 4 + * onto `JourneysRepository`/`JourneyContributorsRepository`/ + * `JourneyPhotosRepository`, NOT routed through `JourneyDomainService + * .canAccessJourney` — this service re-implements the same owner-or + * -contributor logic inline for its own reasons, composing it with the + * trip-photo check in one method; a service-to-service dependency here + * would be a bigger structural change than a survivor cleanup is scoped + * for). `DatabaseService` stays injected for the `canAccessTrip` primitive * (a cross-cutting primitive, not a per-domain SQL statement this plan * converts). */ @@ -38,8 +49,19 @@ export class MemoriesAccessService { @InjectRepository(TrekPhotos) private readonly trekPhotos: TrekPhotosRepository, @InjectRepository(TripAlbumLinks) private readonly tripAlbumLinks: TripAlbumLinksRepository, @InjectRepository(Trips) private readonly trips: TripsRepository, + // Plan 3g Task 4 (MA1/MA2/MA6) — the journey half of the cross-domain + // photo-access checks below. + @InjectRepository(Journeys) private readonly journeys: JourneysRepository, + @InjectRepository(JourneyContributors) private readonly journeyContributors: JourneyContributorsRepository, + @InjectRepository(JourneyPhotos) private readonly journeyPhotos: JourneyPhotosRepository, ) {} + /** MA2's owner-OR-contributor check, shared by MA2 and MA6's per-journey loop — the same two-branch `journeys`/`journey_contributors` logic `canAccessJourney` uses, re-implemented here (see the class docstring on why it isn't a call to `JourneyDomainService`). */ + private async ownerOrContributor(journeyId: number, userId: number): Promise { + if (await this.journeys.isOwnedByUser(journeyId, userId)) return true; + return await this.journeyContributors.existsForUser(journeyId, userId); + } + async canAccessUserPhoto(requestingUserId: number, ownerUserId: number, tripId: string, assetId: string, provider: string): Promise { if (requestingUserId === ownerUserId) { return true; @@ -47,26 +69,12 @@ export class MemoriesAccessService { // Journey photos use tripId=0 — check journey_photos + journey_contributors if (tripId === '0') { - // MA1 — Plan 3g (journey_photos is not yet an owned repository table). - const journeyPhoto = this.db.get<{ journey_id: number }>(` - SELECT gp.journey_id - FROM journey_photos gp - JOIN trek_photos tkp ON tkp.id = gp.photo_id - WHERE tkp.asset_id = ? - AND tkp.provider = ? - AND tkp.owner_id = ? - LIMIT 1 - `, assetId, provider, ownerUserId); - if (!journeyPhoto) return false; + // MA1 — converted (Plan 3g Task 4). + const journeyId = await this.journeyPhotos.findJourneyIdForAsset(assetId, provider, ownerUserId); + if (journeyId === undefined) return false; - // MA2 — Plan 3g (journeys/journey_contributors are not yet owned repository tables). - const access = this.db.get(` - SELECT 1 FROM journeys WHERE id = ? AND user_id = ? - UNION ALL - SELECT 1 FROM journey_contributors WHERE journey_id = ? AND user_id = ? - LIMIT 1 - `, journeyPhoto.journey_id, requestingUserId, journeyPhoto.journey_id, requestingUserId); - return !!access; + // MA2 — converted (Plan 3g Task 4). + return await this.ownerOrContributor(journeyId, requestingUserId); } // Regular trip photos — join through trek_photos (MA3). @@ -100,22 +108,21 @@ export class MemoriesAccessService { } // Check journey_photos — is this photo in a journey the user can access? - // MA6 — Plan 3g (journey_photos/journeys/journey_contributors are not yet owned repository tables). - const journeyAccess = this.db.get(` - SELECT 1 FROM journey_photos gp - WHERE gp.photo_id = ? - AND EXISTS ( - SELECT 1 FROM journeys j WHERE j.id = gp.journey_id AND j.user_id = ? - UNION ALL - SELECT 1 FROM journey_contributors jc WHERE jc.journey_id = gp.journey_id AND jc.user_id = ? - ) - LIMIT 1 - `, trekPhotoId, requestingUserId, requestingUserId); + // MA6 — converted (Plan 3g Task 4). Every journey this photo is linked + // into (a photo can be in more than one journey's gallery), probed the + // same loop-and-check shape as the trip-photo half above (MA5). + let journeyAccess = false; + for (const journeyId of await this.journeyPhotos.listJourneyIdsForPhoto(trekPhotoId)) { + if (await this.ownerOrContributor(journeyId, requestingUserId)) { + journeyAccess = true; + break; + } + } if (journeyAccess) return true; // Local photos without owner (uploaded files) — check if user has journey access if (photo.provider === 'local' && !photo.owner_id) { - return !!journeyAccess; + return journeyAccess; } return false; diff --git a/server/src/nest/memories/memories.module.ts b/server/src/nest/memories/memories.module.ts index d71ee10c3..1ef30bb3a 100644 --- a/server/src/nest/memories/memories.module.ts +++ b/server/src/nest/memories/memories.module.ts @@ -33,6 +33,9 @@ import { Trips } from '../../db/entities/Trips.entity'; import { TrekPhotoCacheMeta } from '../../db/entities/TrekPhotoCacheMeta.entity'; import { PhotoProviders } from '../../db/entities/PhotoProviders.entity'; import { Users } from '../../db/entities/Users.entity'; +import { Journeys } from '../../db/entities/Journeys.entity'; +import { JourneyContributors } from '../../db/entities/JourneyContributors.entity'; +import { JourneyPhotos } from '../../db/entities/JourneyPhotos.entity'; /** * Memories (photo-providers) domain — mounted at /api/integrations/memories. @@ -63,12 +66,16 @@ import { Users } from '../../db/entities/Users.entity'; * this module) and `Users` (owned elsewhere, reached the same way `Trips` * is above) are added for `UnifiedMemoriesService`/`ImmichService`/ * `SynologyService`/`MemoriesMcp`'s `@InjectRepository` constructor params - * (Plan 3e Task 7). + * (Plan 3e Task 7). `Journeys`/`JourneyContributors`/`JourneyPhotos` are + * Plan 3g Task 4's own addition: `MemoriesAccessService`'s MA1/MA2/MA6 + * (`canAccessUserPhoto`/`canAccessTrekPhoto`'s journey half) now inject + * `JourneysRepository`/`JourneyContributorsRepository`/ + * `JourneyPhotosRepository` directly instead of raw `DatabaseService` reads. */ @Module({ imports: [ NotificationsModule, AddonsModule, AuditModule, TrekPhotosModule, RealtimeModule, SchedulingModule, StorageModule, - MikroOrmModule.forFeature([TripPhotos, TrekPhotos, TripAlbumLinks, Trips, TrekPhotoCacheMeta, PhotoProviders, Users]), + MikroOrmModule.forFeature([TripPhotos, TrekPhotos, TripAlbumLinks, Trips, TrekPhotoCacheMeta, PhotoProviders, Users, Journeys, JourneyContributors, JourneyPhotos]), ], controllers: [UnifiedMemoriesController, ImmichMemoriesController, SynologyMemoriesController], providers: [ diff --git a/server/src/nest/photos/trek-photos.module.ts b/server/src/nest/photos/trek-photos.module.ts index 8f31559fb..1d825af7c 100644 --- a/server/src/nest/photos/trek-photos.module.ts +++ b/server/src/nest/photos/trek-photos.module.ts @@ -3,6 +3,7 @@ import { MikroOrmModule } from '@mikro-orm/nestjs'; import { TrekPhotoRegistrationService } from './trek-photos.repository'; import { TrekPhotos } from '../../db/entities/TrekPhotos.entity'; import { TripPhotos } from '../../db/entities/TripPhotos.entity'; +import { JourneyPhotos } from '../../db/entities/JourneyPhotos.entity'; /** * The trek_photos store on its own, so both halves can have it without @@ -16,10 +17,12 @@ import { TripPhotos } from '../../db/entities/TripPhotos.entity'; * * `MikroOrmModule.forFeature` registers `TrekPhotosRepository`/ * `TripPhotosRepository` (the ORM ones) for `TrekPhotoRegistrationService`'s - * `@InjectRepository` constructor params (Plan 3e Task 6). + * `@InjectRepository` constructor params (Plan 3e Task 6). `JourneyPhotos` is + * Plan 3g Task 4's own addition — PH10's `journey_photos` orphan-check half + * (`deleteIfOrphan`) now injects `JourneyPhotosRepository` too. */ @Module({ - imports: [MikroOrmModule.forFeature([TrekPhotos, TripPhotos])], + imports: [MikroOrmModule.forFeature([TrekPhotos, TripPhotos, JourneyPhotos])], providers: [TrekPhotoRegistrationService], exports: [TrekPhotoRegistrationService], }) diff --git a/server/src/nest/photos/trek-photos.repository.ts b/server/src/nest/photos/trek-photos.repository.ts index 188ae9381..434b4dfbd 100644 --- a/server/src/nest/photos/trek-photos.repository.ts +++ b/server/src/nest/photos/trek-photos.repository.ts @@ -7,6 +7,8 @@ import { TrekPhotos } from '../../db/entities/TrekPhotos.entity'; import type { TrekPhotosRepository } from '../../db/repositories/TrekPhotos.repository'; import { TripPhotos } from '../../db/entities/TripPhotos.entity'; import type { TripPhotosRepository } from '../../db/repositories/TripPhotos.repository'; +import { JourneyPhotos } from '../../db/entities/JourneyPhotos.entity'; +import type { JourneyPhotosRepository } from '../../db/repositories/JourneyPhotos.repository'; /** * The `trek_photos` table: register a photo, look one up, retarget it, drop it @@ -35,6 +37,9 @@ export class TrekPhotoRegistrationService { constructor( @InjectRepository(TrekPhotos) private readonly trekPhotos: TrekPhotosRepository, @InjectRepository(TripPhotos) private readonly tripPhotos: TripPhotosRepository, + // Plan 3g Task 4 (PH10) — the `journey_photos` half of `deleteIfOrphan`'s + // split orphan-check. + @InjectRepository(JourneyPhotos) private readonly journeyPhotos: JourneyPhotosRepository, private readonly db: DatabaseService, ) {} @@ -137,16 +142,15 @@ export class TrekPhotoRegistrationService { * * PH10's original statement was one `UNION ALL` existence check across * `trip_photos` and `journey_photos`; split into its two halves here - * (`TripPhotosRepository.existsForPhoto` for the 3e-owned table, a still - * -raw `journey_photos` read for the not-yet-converted Plan 3g table) — - * logically identical (neither half needs to run if the other already - * found a row), never a transaction (both are reads). + * (`TripPhotosRepository.existsForPhoto` for the 3e-owned table, + * `JourneyPhotosRepository.existsForPhoto` for the Plan 3g-owned table, + * converted by Plan 3g Task 4) — logically identical (neither half needs + * to run if the other already found a row), never a transaction (both are + * reads). */ async deleteIfOrphan(photoId: number): Promise { if (await this.tripPhotos.existsForPhoto(photoId)) return; - // PH10 — Plan 3g (journey_photos is not yet an owned repository table). - const stillUsedByJourney = this.db.get('SELECT 1 FROM journey_photos WHERE photo_id = ?', photoId); - if (stillUsedByJourney) return; + if (await this.journeyPhotos.existsForPhoto(photoId)) return; // PH10 — converted (Plan 3g Task 4) await this.trekPhotos.deleteNonLocal(photoId); } } diff --git a/server/src/nest/plugins/contributions/journal-entry-rows.controller.ts b/server/src/nest/plugins/contributions/journal-entry-rows.controller.ts index d682b8840..eaf0d2580 100644 --- a/server/src/nest/plugins/contributions/journal-entry-rows.controller.ts +++ b/server/src/nest/plugins/contributions/journal-entry-rows.controller.ts @@ -1,7 +1,9 @@ import { Controller, Get, Param, Req, UseGuards } from '@nestjs/common'; import type { Request } from 'express'; -import { DatabaseService } from '../../database/database.service'; +import { InjectRepository } from '@mikro-orm/nestjs'; import { JourneyDomainService } from '../../journey/journey-domain.service'; +import { JourneyEntries } from '../../../db/entities/JourneyEntries.entity'; +import type { JourneyEntriesRepository } from '../../../db/repositories/JourneyEntries.repository'; import { AddonsService } from '../../addons/addons.service'; import { ADDON_IDS } from '../../../addons'; import { JwtAuthGuard } from '../../auth/jwt-auth.guard'; @@ -13,7 +15,10 @@ import { stripEmoji } from '../text-sanitize'; * GET /api/journal-entry-rows/:entryId — extra rows for a journal entry, * contributed by plugins implementing the `journalEntryProvider` hook. Additive + * fail-safe like place-details: the entry's journey must be one the caller can - * access (owner/contributor — the same gate as the journal detail routes), the + * access (owner/contributor via `JourneyDomainService.canAccessJourney` — the + * same gate as the journal detail routes; the `journey_id` lookup that feeds + * it is JEC1, converted onto `JourneyEntriesRepository.findById`, Plan 3g + * Task 4), the * Journey addon must be on (an off addon just yields nothing, this endpoint is * purely additive), each provider runs host->plugin on a short timeout, and one * that errors/times out contributes nothing. @@ -68,7 +73,11 @@ function normalize(raw: unknown): EntryRow[] { export class JournalEntryRowsController { constructor( private readonly hooks: PluginHooks, - private readonly dbs: DatabaseService, + // JEC1 (Plan 3g Task 4) — the entry's `journey_id` lookup below; was a + // raw `DatabaseService` read, now `JourneyEntriesRepository.findById` + // (already the widest `SELECT * FROM journey_entries WHERE id = ?` dup + // group's own resolution, so no narrower single-column method needed). + @InjectRepository(JourneyEntries) private readonly journeyEntries: JourneyEntriesRepository, private readonly addons: AddonsService, private readonly journey: JourneyDomainService, ) {} @@ -84,7 +93,7 @@ export class JournalEntryRowsController { if (!Number.isFinite(entryId) || userId == null) return { providers: [] }; // The entry's journey must be one the caller can access — same gate as a read. - const row = this.dbs.connection.prepare('SELECT journey_id FROM journey_entries WHERE id = ?').get(entryId) as { journey_id: number } | undefined; + const row = await this.journeyEntries.findById(entryId); // JEC1 — converted (Plan 3g Task 4) if (!row || !(await this.journey.canAccessJourney(row.journey_id, userId))) return { providers: [] }; const ids = this.hooks.providersOf('journalEntryProvider'); diff --git a/server/src/nest/plugins/contributions/plugin-contributions.module.ts b/server/src/nest/plugins/contributions/plugin-contributions.module.ts index c1bfc30b3..fcbc229e1 100644 --- a/server/src/nest/plugins/contributions/plugin-contributions.module.ts +++ b/server/src/nest/plugins/contributions/plugin-contributions.module.ts @@ -1,7 +1,9 @@ import { Module } from '@nestjs/common'; +import { MikroOrmModule } from '@mikro-orm/nestjs'; import { PluginsRuntimeModule } from '../plugins-runtime.module'; import { AddonsModule } from '../../addons/addons.module'; import { JourneyDomainModule } from '../../journey/journey-domain.module'; +import { JourneyEntries } from '../../../db/entities/JourneyEntries.entity'; import { PlaceDetailsController } from './place-details.controller'; import { PluginSearchController } from './plugin-search.controller'; import { TripWarningsController } from './trip-warnings.controller'; @@ -32,9 +34,16 @@ import { JournalEntryRowsController } from './journal-entry-rows.controller'; * throws — so they belong together and nowhere else. None of them can install, * activate or configure anything, which is why they are separated from the CRUD * surface in PluginsModule. + * + * `MikroOrmModule.forFeature([JourneyEntries])` registers + * `JourneyEntriesRepository` for `JournalEntryRowsController`'s own + * `@InjectRepository` constructor param (JEC1, Plan 3g Task 4) — importing + * `JourneyDomainModule` above brings in `JourneyDomainService` for the + * `canAccessJourney` call but does NOT export `MikroOrmModule`, so the + * entity needs its own registration here too. */ @Module({ - imports: [PluginsRuntimeModule, AddonsModule, JourneyDomainModule], + imports: [PluginsRuntimeModule, AddonsModule, JourneyDomainModule, MikroOrmModule.forFeature([JourneyEntries])], controllers: [ PlaceDetailsController, PluginSearchController, diff --git a/server/src/nest/trips/trips.service.ts b/server/src/nest/trips/trips.service.ts index bc3160aa4..7e4bb61ce 100644 --- a/server/src/nest/trips/trips.service.ts +++ b/server/src/nest/trips/trips.service.ts @@ -4,6 +4,7 @@ import { EntityManager } from '@mikro-orm/core'; import { DatabaseService } from '../database/database.service'; import { Trips } from '../../db/entities/Trips.entity'; import { Days } from '../../db/entities/Days.entity'; +import { JourneyEntries } from '../../db/entities/JourneyEntries.entity'; import { Users } from '../../db/entities/Users.entity'; import { Places } from '../../db/entities/Places.entity'; import { DayAssignments } from '../../db/entities/DayAssignments.entity'; @@ -178,14 +179,17 @@ export class TripsService { private readonly em: EntityManager, ) {} - // Plan 3c Task 7: the raw better-sqlite3 handle now backs only the three - // documented survivors — `remove`'s TP32/TP33 (journey_entries, Plan 3g) - // and `copy`/`getCopiedTrip` (Task 8's own file). Every other method below - // reads/writes through `tripsRepo`/`daysRepo` (or a sibling repository - // reached the same way `TripsService.canAccessTrip`/`.isOwner` already - // did since Task 0b: `this.em.getRepository(...)`, not a new constructor - // parameter — the shared per-request `EntityManager` caches repositories, - // so this is the same instance a hand-constructed test spies on). + // Plan 3c Task 7: the raw better-sqlite3 handle used to back `remove`'s + // TP32/TP33 (journey_entries) — converted by Plan 3g Task 4 onto + // `JourneyEntriesRepository` below, leaving `db`/`dbs` unused in this file + // (kept rather than removed, to avoid a `TripsService` constructor-shape + // change outside this survivor task's scope). Every method in this file + // reads/writes through `tripsRepo`/`daysRepo`/`journeyEntriesRepo` (or a + // sibling repository reached the same way `TripsService.canAccessTrip`/ + // `.isOwner` already did since Task 0b: `this.em.getRepository(...)`, not + // a new constructor parameter — the shared per-request `EntityManager` + // caches repositories, so this is the same instance a hand-constructed + // test spies on). private get db() { return this.dbs.connection; } @@ -198,6 +202,13 @@ export class TripsService { return this.em.getRepository(Days); } + // Plan 3g Task 4 (TP32/TP33) — `journey_entries`' trip-wide skeleton + // cleanup/detach, reached the same `this.em.getRepository(...)` way as + // `tripsRepo`/`daysRepo` above. + private get journeyEntriesRepo() { + return this.em.getRepository(JourneyEntries); + } + // Plan 3c Task 8 (`copy`'s own 6 owned tables): resolved the same way // `tripsRepo`/`daysRepo` above are — `this.em.getRepository(...)`, not a // new constructor parameter. @@ -632,9 +643,9 @@ export class TripsService { // ── Delete ───────────────────────────────────────────────────────────────── /** - * TP30–TP34 (inventory §11c) — TP32/TP33 (`journey_entries`) stay raw - * inside the transaction (`// Plan 3g`, that table's owning plan); every - * other statement moved through `TripsRepository`. + * TP30–TP34 (inventory §11c) — every statement, including TP32/TP33 + * (`journey_entries`), moves through a repository (TP32/TP33 converted by + * Plan 3g Task 4 onto `JourneyEntriesRepository`). */ async remove(tripId: string | number, userId: number, userRole: string): Promise { const trip = await this.tripsRepo.findIdTitleOwner(tripId); // TP30 (the `id` field this method also selects is unused here) @@ -652,15 +663,9 @@ export class TripsService { await this.uow.transactional(async () => { // Clean up journey entries synced from this trip before deleting // Delete skeleton entries (unfilled synced places) - this.db.prepare(` - DELETE FROM journey_entries - WHERE source_trip_id = ? AND type = 'skeleton' - `).run(tripId); // TP32 — Plan 3g + await this.journeyEntriesRepo.deleteAllSkeletonsForTrip(Number(tripId)); // TP32 — converted (Plan 3g Task 4) // Detach filled entries (keep user's written content, just remove trip link) - this.db.prepare(` - UPDATE journey_entries SET source_trip_id = NULL, source_place_id = NULL, source_assignment_id = NULL - WHERE source_trip_id = ? - `).run(tripId); // TP33 — Plan 3g + await this.journeyEntriesRepo.detachAllFilledForTrip(Number(tripId)); // TP33 — converted (Plan 3g Task 4) await this.tripsRepo.deleteById(Number(tripId)); // TP34 — safe: `trip` above only resolved through the raw-bind seam on a real row }); diff --git a/server/tests/helpers/accommodations-service.ts b/server/tests/helpers/accommodations-service.ts index 1ab790716..7d00e0493 100644 --- a/server/tests/helpers/accommodations-service.ts +++ b/server/tests/helpers/accommodations-service.ts @@ -53,7 +53,7 @@ export async function accommodationsOver(dbs: DatabaseService): Promise { permissionsService, new TripMembershipService(await createTestTripsRepo(dbService.connection), await createTestTripMembersRepo(dbService.connection)), new WebauthnConfigService(appSettings), - new UserCleanupService(dbService, budgetService, await createTestUnitOfWork(dbService.connection), usersRepo, await createTestBudgetItemsRepo(dbService.connection)), + new UserCleanupService(dbService, budgetService, await createTestUnitOfWork(dbService.connection), usersRepo, await createTestBudgetItemsRepo(dbService.connection), await createTestJourneyShareTokensRepo(dbService.connection), await createTestJourneysRepo(dbService.connection), await createTestJourneyEntriesRepo(dbService.connection), await createTestJourneyContributorsRepo(dbService.connection)), new MailerService(usersRepo, settingsRepo, appSettings), new EphemeralTokenService(), new AllowedFileTypesService(appSettings), await createTestUnitOfWork(dbService.connection), @@ -251,7 +252,7 @@ export async function createMcpTestRegistry(): Promise { const placePhotoCache = new PlacePhotoCacheService(dbService, makeStorageFixture('photos/google/').storage, await createTestGooglePlacePhotoMetaRepo(dbService.connection), await createTestPlacesRepo(dbService.connection)); const mapsService = new MapsService(dbService, placePhotoCache, appSettings, usersRepo); const journeyDomain = new JourneyDomainService( - dbService, realtimeService, new TrekPhotoRegistrationService(mcpOrm.repo(TrekPhotos), mcpOrm.repo(TripPhotos), dbService), await createTestUnitOfWork(dbService.connection), + dbService, realtimeService, new TrekPhotoRegistrationService(mcpOrm.repo(TrekPhotos), mcpOrm.repo(TripPhotos), await createTestJourneyPhotosRepo(dbService.connection), dbService), await createTestUnitOfWork(dbService.connection), // Plan 3g Task 1 — the constructor-ripple fix (R9): four journey-owned // repositories + the already-shared `TripsRepository` (AP1/`getTitle`). await createTestJourneysRepo(dbService.connection), await createTestJourneyContributorsRepo(dbService.connection), @@ -310,7 +311,7 @@ export async function createMcpTestRegistry(): Promise { ); // Built after it: a hotel booking writes the stay's day stop through this one. const reservationsService = new ReservationsService(dbService, permissionsService, budgetService, realtimeService, notificationsStub(), new ReservationsReadService(await createTestReservationsRepo(dbService.connection), await createTestReservationEndpointsRepo(dbService.connection), await createTestReservationTravelersRepo(dbService.connection)), accommodationsService, await createTestUnitOfWork(dbService.connection), await createTestReservationsRepo(dbService.connection), await createTestReservationEndpointsRepo(dbService.connection), await createTestReservationTravelersRepo(dbService.connection), await createTestReservationDayPositionsRepo(dbService.connection), await createTestDayAccommodationsRepo(dbService.connection), await createTestDaysRepo(dbService.connection), await createTestPlacesRepo(dbService.connection), await createTestDayAssignmentsRepo(dbService.connection), await createTestTripMembersRepo(dbService.connection), await createTestUsersRepo(dbService.connection), await createTestTripsRepo(dbService.connection), await createTestBudgetItemsRepo(dbService.connection)); - const membersService = new TripMembersService(dbService, budgetService, new UserCleanupService(dbService, budgetService, await createTestUnitOfWork(dbService.connection), usersRepo, await createTestBudgetItemsRepo(dbService.connection)), permissionsService, realtimeService, notificationsStub(), await createTestUnitOfWork(dbService.connection), await createTestTripsRepo(dbService.connection), await createTestTripMembersRepo(dbService.connection), usersRepo); + const membersService = new TripMembersService(dbService, budgetService, new UserCleanupService(dbService, budgetService, await createTestUnitOfWork(dbService.connection), usersRepo, await createTestBudgetItemsRepo(dbService.connection), await createTestJourneyShareTokensRepo(dbService.connection), await createTestJourneysRepo(dbService.connection), await createTestJourneyEntriesRepo(dbService.connection), await createTestJourneyContributorsRepo(dbService.connection)), permissionsService, realtimeService, notificationsStub(), await createTestUnitOfWork(dbService.connection), await createTestTripsRepo(dbService.connection), await createTestTripMembersRepo(dbService.connection), usersRepo); const tripsService = new TripsService( dbService, reservationsService, @@ -366,10 +367,14 @@ export async function createMcpTestRegistry(): Promise { // (which asks them when and where it was taken). Built for real rather than // stubbed: an empty provider registry would make the backfill answer "unknown // provider" for every id and hide a wiring mistake behind a caught error. - const memoriesAccess = new MemoriesAccessService(dbService, mcpOrm.repo(TripPhotos), mcpOrm.repo(TrekPhotos), mcpOrm.repo(TripAlbumLinks), mcpOrm.repo(Trips)); + const memoriesAccess = new MemoriesAccessService( + dbService, mcpOrm.repo(TripPhotos), mcpOrm.repo(TrekPhotos), mcpOrm.repo(TripAlbumLinks), mcpOrm.repo(Trips), + // Plan 3g Task 4 constructor-ripple: MA1/MA2/MA6's journey half. + await createTestJourneysRepo(dbService.connection), await createTestJourneyContributorsRepo(dbService.connection), await createTestJourneyPhotosRepo(dbService.connection), + ); const immichService = new ImmichService(new AuditService(auditLogRepo, usersRepo), memoriesAccess, generalStorage, usersRepo); const synologyService = new SynologyService(memoriesAccess, notificationsStub(), usersRepo); - const trekPhotos = new TrekPhotoRegistrationService(mcpOrm.repo(TrekPhotos), mcpOrm.repo(TripPhotos), dbService); + const trekPhotos = new TrekPhotoRegistrationService(mcpOrm.repo(TrekPhotos), mcpOrm.repo(TripPhotos), await createTestJourneyPhotosRepo(dbService.connection), dbService); 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( [ @@ -429,7 +434,20 @@ export async function createMcpTestRegistry(): Promise { new TripInviteMcp(new TripInviteService(dbService, permissionsService, new TripMembershipService(await createTestTripsRepo(dbService.connection), await createTestTripMembersRepo(dbService.connection)), await createTestUnitOfWork(dbService.connection)), dbService, new RuntimeEnvService(), guards, new AuditService(auditLogRepo, usersRepo)), new MapsMcp(mapsService), new PlacesMcp(placesService, mapsService, await createTestTripsRepo(dbService.connection), authService, journeyDomain, assignmentsService, guards, await createTestUnitOfWork(dbService.connection)), - new CollectionsMcp(new CollectionsService(dbService, permissionsService, realtimeService, notificationsStub(), generalStorage, await createTestUnitOfWork(dbService.connection)), dbService, authService, addonsService), + new CollectionsMcp( + new CollectionsService( + dbService, permissionsService, realtimeService, notificationsStub(), generalStorage, await createTestUnitOfWork(dbService.connection), + // Plan 3h Task 1 — the constructor-ripple fix: collections part A's + // own repositories, first cut, plus the already-DONE + // `CategoriesRepository` (CL21 reuse). + await createTestCollectionsRepo(dbService.connection), await createTestCollectionMembersRepo(dbService.connection), + await createTestCollectionLabelsRepo(dbService.connection), await createTestCategoriesRepo(dbService.connection), + ), + // Plan 3h Task 1 — `CollectionsMcp`'s own constructor-ripple fix: + // `DatabaseService` dropped (CL89's only use), `UsersRepository` + // (`findUsernameEmail`, UM11's precedent) added in its place. + usersRepo, authService, addonsService, + ), new TransitMcp(new TransitService(new GoogleTransitProvider(dbService, appSettings, usersRepo)), daysService, reservationsService, dbService, authService, guards), new AtlasMcp(new AtlasService( await createTestBucketListRepo(dbService.connection), await createTestHiddenCountriesRepo(dbService.connection), diff --git a/server/tests/helpers/plugin-host.ts b/server/tests/helpers/plugin-host.ts index dd5b9d148..af5e11d8e 100644 --- a/server/tests/helpers/plugin-host.ts +++ b/server/tests/helpers/plugin-host.ts @@ -76,6 +76,7 @@ import { RuntimeEnvService } from '../../src/nest/app-config/runtime-env.service import { makeStorageFixture } from './storage-fixture'; import { createTestUnitOfWork, createTestAppSettingsRepo, createTestCategoriesRepo, createTestTagsRepo, createTestSettingsRepo, sharedTestOrm, + createTestCollectionsRepo, createTestCollectionMembersRepo, createTestCollectionLabelsRepo, createTestDaysRepo, createTestDayAssignmentsRepo, createTestDayNotesRepo, createTestTripsRepo, createTestTripMembersRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo, createTestGooglePlacePhotoMetaRepo, createTestPlacesRepo, createTestRoadtripViasRepo, @@ -115,6 +116,7 @@ import { createTestJourneysRepo, createTestJourneyContributorsRepo, createTestJourneyTripsRepo, createTestJourneyEntriesRepo, createTestJourneyPhotosRepo, createTestJourneyEntryPhotosRepo, } from './journey-repos'; +import { createTestJourneyShareTokensRepo } from './journey-share-repos'; /** * Hand-wired counterpart of the PluginsModule DI graph for no-Nest tests @@ -187,7 +189,7 @@ export async function createPluginRpcHostFactory(dbs: DatabaseService): Promise< const photoCache = new PlacePhotoCacheService(dbs, makeStorageFixture('photos/google/').storage, await createTestGooglePlacePhotoMetaRepo(dbs.connection), await createTestPlacesRepo(dbs.connection)); const unsplash = new UnsplashService(appSettings, usersRepo, new RuntimeEnvService(), generalStorage); const journey = new JourneyDomainService( - dbs, realtime, new TrekPhotoRegistrationService((await sharedTestOrm(dbs.connection)).repo(TrekPhotos), (await sharedTestOrm(dbs.connection)).repo(TripPhotos), dbs), await createTestUnitOfWork(dbs.connection), + dbs, realtime, new TrekPhotoRegistrationService((await sharedTestOrm(dbs.connection)).repo(TrekPhotos), (await sharedTestOrm(dbs.connection)).repo(TripPhotos), await createTestJourneyPhotosRepo(dbs.connection), dbs), await createTestUnitOfWork(dbs.connection), // Plan 3g Task 1 — the constructor-ripple fix (R9): four journey-owned // repositories + the already-shared `TripsRepository` (AP1/`getTitle`). await createTestJourneysRepo(dbs.connection), await createTestJourneyContributorsRepo(dbs.connection), @@ -201,7 +203,14 @@ export async function createPluginRpcHostFactory(dbs: DatabaseService): Promise< await createTestJourneyPhotosRepo(dbs.connection), await createTestJourneyEntryPhotosRepo(dbs.connection), await createTestPlacesRepo(dbs.connection), ); - const collections = new CollectionsService(dbs, permissions, realtime, notificationsStub(), generalStorage, await createTestUnitOfWork(dbs.connection)); + const collections = new CollectionsService( + dbs, permissions, realtime, notificationsStub(), generalStorage, await createTestUnitOfWork(dbs.connection), + // Plan 3h Task 1 — the constructor-ripple fix: collections part A's own + // repositories, first cut (`CollectionMembersRepository` extended by + // Task 2), plus the already-DONE `CategoriesRepository` (CL21 reuse). + await createTestCollectionsRepo(dbs.connection), await createTestCollectionMembersRepo(dbs.connection), + await createTestCollectionLabelsRepo(dbs.connection), await createTestCategoriesRepo(dbs.connection), + ); const atlas = new AtlasService( await createTestBucketListRepo(dbs.connection), await createTestHiddenCountriesRepo(dbs.connection), await createTestHiddenRegionsRepo(dbs.connection), await createTestVisitedCountriesRepo(dbs.connection), @@ -248,7 +257,7 @@ export async function createPluginRpcHostFactory(dbs: DatabaseService): Promise< // After accommodations: a hotel booking writes the stay's day stop through it. const reservations = new ReservationsService(dbs, permissions, budget, realtime, notificationsStub(), new ReservationsReadService(await createTestReservationsRepo(dbs.connection), await createTestReservationEndpointsRepo(dbs.connection), await createTestReservationTravelersRepo(dbs.connection)), accommodations, await createTestUnitOfWork(dbs.connection), await createTestReservationsRepo(dbs.connection), await createTestReservationEndpointsRepo(dbs.connection), await createTestReservationTravelersRepo(dbs.connection), await createTestReservationDayPositionsRepo(dbs.connection), await createTestDayAccommodationsRepo(dbs.connection), await createTestDaysRepo(dbs.connection), await createTestPlacesRepo(dbs.connection), await createTestDayAssignmentsRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestTripsRepo(dbs.connection), await createTestBudgetItemsRepo(dbs.connection)); const trips = new TripsService(dbs, reservations, days, permissions, budget, vacay, realtime, unsplash, generalStorage, await createTestUnitOfWork(dbs.connection), (await sharedTestOrm(dbs.connection)).em); - const members = new TripMembersService(dbs, budget, new UserCleanupService(dbs, budget, await createTestUnitOfWork(dbs.connection), usersRepo, await createTestBudgetItemsRepo(dbs.connection)), permissions, realtime, notificationsStub(), await createTestUnitOfWork(dbs.connection), await createTestTripsRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection), usersRepo); + const members = new TripMembersService(dbs, budget, new UserCleanupService(dbs, budget, await createTestUnitOfWork(dbs.connection), usersRepo, await createTestBudgetItemsRepo(dbs.connection), await createTestJourneyShareTokensRepo(dbs.connection), await createTestJourneysRepo(dbs.connection), await createTestJourneyEntriesRepo(dbs.connection), await createTestJourneyContributorsRepo(dbs.connection)), permissions, realtime, notificationsStub(), await createTestUnitOfWork(dbs.connection), await createTestTripsRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection), usersRepo); const guards = new PluginGuards(dbs, permissions, addons); const registry = createTestPluginRegistry([ diff --git a/server/tests/unit/nest/admin.service.test.ts b/server/tests/unit/nest/admin.service.test.ts index 732a0f663..d833ec95c 100644 --- a/server/tests/unit/nest/admin.service.test.ts +++ b/server/tests/unit/nest/admin.service.test.ts @@ -98,6 +98,8 @@ import { } from '../../helpers/test-uow'; import { budgetRepoArgs } from '../../helpers/budget-repos'; import { createTestBudgetItemsRepo } from '../../helpers/files-repos'; +import { createTestJourneysRepo, createTestJourneyEntriesRepo, createTestJourneyContributorsRepo } from '../../helpers/journey-repos'; +import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-repos'; const dbs = new DatabaseService(testDb); const realtime = new RealtimeService(); @@ -116,7 +118,7 @@ let svc: AdminService; beforeAll(async () => { webauthn = new WebauthnConfigService(await createTestAppSettingsRepo(dbs.connection)); permissions = new PermissionsService(await createTestAppSettingsRepo(dbs.connection), await createTestUnitOfWork(dbs.connection)); - userCleanup = new UserCleanupService(dbs, new BudgetService(dbs, permissions, new ExchangeRatesService(), realtime, await createTestUnitOfWork(dbs.connection), ...(await budgetRepoArgs(dbs.connection))), await createTestUnitOfWork(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestBudgetItemsRepo(dbs.connection)); + userCleanup = new UserCleanupService(dbs, new BudgetService(dbs, permissions, new ExchangeRatesService(), realtime, await createTestUnitOfWork(dbs.connection), ...(await budgetRepoArgs(dbs.connection))), await createTestUnitOfWork(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestBudgetItemsRepo(dbs.connection), await createTestJourneyShareTokensRepo(dbs.connection), await createTestJourneysRepo(dbs.connection), await createTestJourneyEntriesRepo(dbs.connection), await createTestJourneyContributorsRepo(dbs.connection)); auth = new AuthService( permissions, new TripMembershipService(await createTestTripsRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection)), webauthn, userCleanup, new MailerService(await createTestUsersRepo(dbs.connection), await createTestSettingsRepo(dbs.connection), await createTestAppSettingsRepo(dbs.connection)), new EphemeralTokenService(), new AllowedFileTypesService(await createTestAppSettingsRepo(dbs.connection)), await createTestUnitOfWork(dbs.connection), await createTestAppSettingsRepo(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestInviteTokensRepo(dbs.connection), await createTestMcpTokensRepo(dbs.connection), diff --git a/server/tests/unit/nest/admin.version-notification.test.ts b/server/tests/unit/nest/admin.version-notification.test.ts index a4be9aac2..2b94f1eec 100644 --- a/server/tests/unit/nest/admin.version-notification.test.ts +++ b/server/tests/unit/nest/admin.version-notification.test.ts @@ -76,6 +76,8 @@ import { } from '../../helpers/test-uow'; import { budgetRepoArgs } from '../../helpers/budget-repos'; import { createTestBudgetItemsRepo } from '../../helpers/files-repos'; +import { createTestJourneysRepo, createTestJourneyEntriesRepo, createTestJourneyContributorsRepo } from '../../helpers/journey-repos'; +import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-repos'; const dbs = new DatabaseService(testDb); const realtime = new RealtimeService(); @@ -94,7 +96,7 @@ let svc: AdminService; beforeAll(async () => { webauthn = new WebauthnConfigService(await createTestAppSettingsRepo(dbs.connection)); permissions = new PermissionsService(await createTestAppSettingsRepo(dbs.connection), await createTestUnitOfWork(dbs.connection)); - userCleanup = new UserCleanupService(dbs, new BudgetService(dbs, permissions, new ExchangeRatesService(), realtime, await createTestUnitOfWork(dbs.connection), ...(await budgetRepoArgs(dbs.connection))), await createTestUnitOfWork(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestBudgetItemsRepo(dbs.connection)); + userCleanup = new UserCleanupService(dbs, new BudgetService(dbs, permissions, new ExchangeRatesService(), realtime, await createTestUnitOfWork(dbs.connection), ...(await budgetRepoArgs(dbs.connection))), await createTestUnitOfWork(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestBudgetItemsRepo(dbs.connection), await createTestJourneyShareTokensRepo(dbs.connection), await createTestJourneysRepo(dbs.connection), await createTestJourneyEntriesRepo(dbs.connection), await createTestJourneyContributorsRepo(dbs.connection)); auth = new AuthService( permissions, new TripMembershipService(await createTestTripsRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection)), webauthn, userCleanup, new MailerService(await createTestUsersRepo(dbs.connection), await createTestSettingsRepo(dbs.connection), await createTestAppSettingsRepo(dbs.connection)), new EphemeralTokenService(), new AllowedFileTypesService(await createTestAppSettingsRepo(dbs.connection)), await createTestUnitOfWork(dbs.connection), await createTestAppSettingsRepo(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestInviteTokensRepo(dbs.connection), await createTestMcpTokensRepo(dbs.connection), diff --git a/server/tests/unit/nest/assignments.service.test.ts b/server/tests/unit/nest/assignments.service.test.ts index cb74d9c54..cb9aff276 100644 --- a/server/tests/unit/nest/assignments.service.test.ts +++ b/server/tests/unit/nest/assignments.service.test.ts @@ -84,7 +84,7 @@ beforeAll(async () => { // Real collaborator rather than a stub: reconcile() runs after every mutation // and needs the same connection to see the rows these cases write. new JourneyDomainService( - dbs, realtime, new TrekPhotoRegistrationService((await sharedTestOrm(dbs.connection)).repo(TrekPhotos), (await sharedTestOrm(dbs.connection)).repo(TripPhotos), dbs), await createTestUnitOfWork(dbs.connection), + dbs, realtime, new TrekPhotoRegistrationService((await sharedTestOrm(dbs.connection)).repo(TrekPhotos), (await sharedTestOrm(dbs.connection)).repo(TripPhotos), await createTestJourneyPhotosRepo(dbs.connection), dbs), await createTestUnitOfWork(dbs.connection), await createTestJourneysRepo(dbs.connection), await createTestJourneyContributorsRepo(dbs.connection), await createTestJourneyTripsRepo(dbs.connection), await createTestJourneyEntriesRepo(dbs.connection), await createTestTripsRepo(dbs.connection), // Plan 3g Task 2 constructor-ripple: JourneyPhotosRepository/JourneyEntryPhotosRepository/PlacesRepository. diff --git a/server/tests/unit/nest/auth.service.test.ts b/server/tests/unit/nest/auth.service.test.ts index 05fb199ad..e3d1f9411 100644 --- a/server/tests/unit/nest/auth.service.test.ts +++ b/server/tests/unit/nest/auth.service.test.ts @@ -96,6 +96,8 @@ import { } from '../../helpers/test-uow'; import { budgetRepoArgs } from '../../helpers/budget-repos'; import { createTestBudgetItemsRepo } from '../../helpers/files-repos'; +import { createTestJourneysRepo, createTestJourneyEntriesRepo, createTestJourneyContributorsRepo } from '../../helpers/journey-repos'; +import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-repos'; // MailerService is injected since the notifications fold — a stub instead of a // module mock. sendPasswordResetEmail is the only thing auth reaches for. @@ -116,7 +118,7 @@ beforeAll(async () => { new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), membershipStub, new WebauthnConfigService(await createTestAppSettingsRepo(testDb)), - new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(testDb), ...(await budgetRepoArgs(testDb))), await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestBudgetItemsRepo(testDb)), + new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(testDb), ...(await budgetRepoArgs(testDb))), await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestBudgetItemsRepo(testDb), await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb)), mailerStub, new EphemeralTokenService(), new AllowedFileTypesService(await createTestAppSettingsRepo(testDb)), await createTestUnitOfWork(testDb), diff --git a/server/tests/unit/nest/budget.service.db.test.ts b/server/tests/unit/nest/budget.service.db.test.ts index 06e7f8cb6..0f2873760 100644 --- a/server/tests/unit/nest/budget.service.db.test.ts +++ b/server/tests/unit/nest/budget.service.db.test.ts @@ -75,6 +75,8 @@ import { notificationsStub } from '../../helpers/notifications'; import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm, createTestTripsRepo, createTestTripMembersRepo } from '../../helpers/test-uow'; import { budgetRepoArgs, createTestBudgetItemMembersRepo, createTestBudgetItemPayersRepo, createTestBudgetSettlementsRepo } from '../../helpers/budget-repos'; import { createTestBudgetItemsRepo } from '../../helpers/files-repos'; +import { createTestJourneysRepo, createTestJourneyEntriesRepo, createTestJourneyContributorsRepo } from '../../helpers/journey-repos'; +import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-repos'; import { BudgetItemMembers } from '../../../src/db/entities/BudgetItemMembers.entity'; @@ -118,7 +120,7 @@ beforeAll(async () => { membersSvc = new TripMembersService( dbs(), budget, - new UserCleanupService(dbs(), budget, await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestBudgetItemsRepo(testDb)), + new UserCleanupService(dbs(), budget, await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestBudgetItemsRepo(testDb), await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), diff --git a/server/tests/unit/nest/journey-book.service.test.ts b/server/tests/unit/nest/journey-book.service.test.ts index 513cda7af..a26789784 100644 --- a/server/tests/unit/nest/journey-book.service.test.ts +++ b/server/tests/unit/nest/journey-book.service.test.ts @@ -74,7 +74,7 @@ beforeAll(async () => { const uow = await createTestUnitOfWork(testDb); const t = await sharedTestOrm(testDb); domain = new JourneyDomainService( - dbs, new RealtimeService(), new TrekPhotoRegistrationService(t.repo(TrekPhotos), t.repo(TripPhotos), dbs), uow, + dbs, new RealtimeService(), new TrekPhotoRegistrationService(t.repo(TrekPhotos), t.repo(TripPhotos), await createTestJourneyPhotosRepo(testDb), dbs), uow, await createTestJourneysRepo(testDb), await createTestJourneyContributorsRepo(testDb), await createTestJourneyTripsRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestTripsRepo(testDb), // Plan 3g Task 2 constructor-ripple: JourneyPhotosRepository/JourneyEntryPhotosRepository/PlacesRepository. diff --git a/server/tests/unit/nest/journey-domain.service.test.ts b/server/tests/unit/nest/journey-domain.service.test.ts index efb5e4078..5a2e2a2c6 100644 --- a/server/tests/unit/nest/journey-domain.service.test.ts +++ b/server/tests/unit/nest/journey-domain.service.test.ts @@ -106,7 +106,7 @@ beforeAll(async () => { entryPhotosRepoDirect = await createTestJourneyEntryPhotosRepo(testDb); placesRepoDirect = await createTestPlacesRepo(testDb); svc = new JourneyDomainService( - dbs, new RealtimeService(), new TrekPhotoRegistrationService(t.repo(TrekPhotos), t.repo(TripPhotos), dbs), await createTestUnitOfWork(testDb), + dbs, new RealtimeService(), new TrekPhotoRegistrationService(t.repo(TrekPhotos), t.repo(TripPhotos), photosRepoDirect, dbs), await createTestUnitOfWork(testDb), journeysRepoDirect, contributorsRepoDirect, journeyTripsRepoDirect, entriesRepoDirect, tripsRepoDirect, // Plan 3g Task 2's own append to this SAME construction call — the two // repositories this task builds, plus `PlacesRepository` (JG44) — per diff --git a/server/tests/unit/nest/journey-share.service.test.ts b/server/tests/unit/nest/journey-share.service.test.ts index 6e8b174ce..f09f4febe 100644 --- a/server/tests/unit/nest/journey-share.service.test.ts +++ b/server/tests/unit/nest/journey-share.service.test.ts @@ -70,7 +70,7 @@ beforeAll(async () => { const journeysRepo = await createTestJourneysRepo(testDb); svc = new JourneyShareService( new JourneyDomainService( - dbs, new RealtimeService(), new TrekPhotoRegistrationService(t.repo(TrekPhotos), t.repo(TripPhotos), dbs), uow, + dbs, new RealtimeService(), new TrekPhotoRegistrationService(t.repo(TrekPhotos), t.repo(TripPhotos), await createTestJourneyPhotosRepo(testDb), dbs), uow, journeysRepo, await createTestJourneyContributorsRepo(testDb), await createTestJourneyTripsRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestTripsRepo(testDb), // Plan 3g Task 2 constructor-ripple: JourneyPhotosRepository/JourneyEntryPhotosRepository/PlacesRepository. diff --git a/server/tests/unit/nest/journey-stats.service.test.ts b/server/tests/unit/nest/journey-stats.service.test.ts index 45a5860ad..c22f2fc7a 100644 --- a/server/tests/unit/nest/journey-stats.service.test.ts +++ b/server/tests/unit/nest/journey-stats.service.test.ts @@ -97,7 +97,7 @@ beforeAll(async () => { runMigrations(testDb); const t = await sharedTestOrm(testDb); svc = new JourneyDomainService( - dbs, new RealtimeService(), new TrekPhotoRegistrationService(t.repo(TrekPhotos), t.repo(TripPhotos), dbs), await createTestUnitOfWork(testDb), + dbs, new RealtimeService(), new TrekPhotoRegistrationService(t.repo(TrekPhotos), t.repo(TripPhotos), await createTestJourneyPhotosRepo(testDb), dbs), await createTestUnitOfWork(testDb), await createTestJourneysRepo(testDb), await createTestJourneyContributorsRepo(testDb), await createTestJourneyTripsRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestTripsRepo(testDb), // Plan 3g Task 2 constructor-ripple: JourneyPhotosRepository/JourneyEntryPhotosRepository/PlacesRepository. diff --git a/server/tests/unit/nest/memories-access.service.test.ts b/server/tests/unit/nest/memories-access.service.test.ts index cc7cf34d1..11dcf49e2 100644 --- a/server/tests/unit/nest/memories-access.service.test.ts +++ b/server/tests/unit/nest/memories-access.service.test.ts @@ -64,13 +64,18 @@ import { Trips } from '../../../src/db/entities/Trips.entity'; import { TripPhotos } from '../../../src/db/entities/TripPhotos.entity'; import { TrekPhotos } from '../../../src/db/entities/TrekPhotos.entity'; import { TripAlbumLinks } from '../../../src/db/entities/TripAlbumLinks.entity'; +import { Journeys } from '../../../src/db/entities/Journeys.entity'; +import { JourneyContributors } from '../../../src/db/entities/JourneyContributors.entity'; +import { JourneyPhotos } from '../../../src/db/entities/JourneyPhotos.entity'; import { sharedTestOrm } from '../../helpers/test-uow'; -// Plan 3c Task 0b / Plan 3e Task 6: `access` used to be constructed at module -// load, before any `beforeAll` could resolve a real `EntityManager` — it now -// also needs `TripPhotosRepository`/`TrekPhotosRepository`/ -// `TripAlbumLinksRepository`/`TripsRepository` (the ORM ones), which the -// same `sharedTestOrm(testDb)` this file already used for the +// Plan 3c Task 0b / Plan 3e Task 6 / Plan 3g Task 4: `access` used to be +// constructed at module load, before any `beforeAll` could resolve a real +// `EntityManager` — it now also needs `TripPhotosRepository`/ +// `TrekPhotosRepository`/`TripAlbumLinksRepository`/`TripsRepository` (the +// ORM ones) and, for MA1/MA2/MA6's journey half, `JourneysRepository`/ +// `JourneyContributorsRepository`/`JourneyPhotosRepository`, all of which +// the same `sharedTestOrm(testDb)` this file already used for the // `canAccessTrip` patch hands out. Built inside the async `beforeAll` below. let access: MemoriesAccessService; // A typed forwarder, not a `.bind` alias: a bound alias is typed `any`, which @@ -89,7 +94,7 @@ beforeAll(async () => { vi.spyOn(DatabaseService.prototype, 'canAccessTrip').mockImplementation(async (tripId, userId) => t.em.getRepository(Trips).findAccessible(tripId, userId), ); - access = new MemoriesAccessService(dbs, t.repo(TripPhotos), t.repo(TrekPhotos), t.repo(TripAlbumLinks), t.repo(Trips)); + access = new MemoriesAccessService(dbs, t.repo(TripPhotos), t.repo(TrekPhotos), t.repo(TripAlbumLinks), t.repo(Trips), t.repo(Journeys), t.repo(JourneyContributors), t.repo(JourneyPhotos)); }); beforeEach(() => { @@ -396,6 +401,34 @@ describe('canAccessUserPhoto', () => { expect(await access.canAccessUserPhoto(contributor.id, owner.id, '0', 'j-asset', 'immich')).toBe(true); }); + it('MEM-ACCESS-006b: tripId "0" — the journey OWNER (not the photo\'s own owner_id) passes via MA2\'s owner branch, not the trivial requestingUserId===ownerUserId shortcut', async () => { + const { user: journeyOwner } = createUser(testDb, { username: 'journey-owner' }); + const { user: uploader } = createUser(testDb, { username: 'uploader-6b' }); + const journeyId = makeJourney(journeyOwner.id); + const photoId = Number(testDb.prepare( + "INSERT INTO trek_photos (provider, asset_id, owner_id) VALUES ('immich', 'j-asset-owner', ?)" + ).run(uploader.id).lastInsertRowid); + testDb.prepare('INSERT INTO journey_photos (journey_id, photo_id, created_at) VALUES (?, ?, 0)').run(journeyId, photoId); + + // requestingUserId (journeyOwner) !== ownerUserId (uploader) — MA1's read is + // scoped to tkp.owner_id=uploader, so the trivial owner shortcut at the top + // of canAccessUserPhoto never fires; access is decided by MA2's owner branch. + expect(await access.canAccessUserPhoto(journeyOwner.id, uploader.id, '0', 'j-asset-owner', 'immich')).toBe(true); + }); + + it('MEM-ACCESS-006c: tripId "0" — a VIEWER-role contributor also passes (MA2 checks any role, not just editor)', async () => { + const { user: owner } = createUser(testDb); + const { user: viewer } = createUser(testDb, { username: 'viewer-6c' }); + const journeyId = makeJourney(owner.id); + const photoId = Number(testDb.prepare( + "INSERT INTO trek_photos (provider, asset_id, owner_id) VALUES ('immich', 'j-asset-viewer', ?)" + ).run(owner.id).lastInsertRowid); + testDb.prepare('INSERT INTO journey_photos (journey_id, photo_id, created_at) VALUES (?, ?, 0)').run(journeyId, photoId); + testDb.prepare("INSERT INTO journey_contributors (journey_id, user_id, role, added_at) VALUES (?, ?, 'viewer', 0)").run(journeyId, viewer.id); + + expect(await access.canAccessUserPhoto(viewer.id, owner.id, '0', 'j-asset-viewer', 'immich')).toBe(true); + }); + it('MEM-ACCESS-007: tripId "0" refuses someone with no journey link', async () => { const { user: owner } = createUser(testDb); const { user: stranger } = createUser(testDb, { username: 'stranger' }); @@ -463,6 +496,21 @@ describe('canAccessTrekPhoto', () => { expect(await access.canAccessTrekPhoto(contributor.id, photoId)).toBe(true); }); + it('MEM-ACCESS-014b: a VIEWER-role journey contributor also passes MA6\'s unified check (any role, not just editor)', async () => { + const { user: owner } = createUser(testDb); + const { user: viewer } = createUser(testDb, { username: 'viewer-14b' }); + const { user: stranger } = createUser(testDb, { username: 'stranger-14b' }); + const journeyId = makeJourney(owner.id); + const photoId = Number(testDb.prepare( + "INSERT INTO trek_photos (provider, asset_id, owner_id) VALUES ('immich', 'j-trek-viewer', ?)" + ).run(owner.id).lastInsertRowid); + testDb.prepare('INSERT INTO journey_photos (journey_id, photo_id, created_at) VALUES (?, ?, 0)').run(journeyId, photoId); + testDb.prepare("INSERT INTO journey_contributors (journey_id, user_id, role, added_at) VALUES (?, ?, 'viewer', 0)").run(journeyId, viewer.id); + + expect(await access.canAccessTrekPhoto(viewer.id, photoId)).toBe(true); + expect(await access.canAccessTrekPhoto(stranger.id, photoId)).toBe(false); + }); + it('MEM-ACCESS-015: an ownerless local upload is reachable only through its journey', async () => { const { user: owner } = createUser(testDb); const { user: outsider } = createUser(testDb, { username: 'outsider' }); @@ -554,6 +602,47 @@ describe('canAccessTrekPhoto — MA5 parity (TripsRepository.findAccessible rewr }); }); +// --------------------------------------------------------------------------- +// MA1/MA2/MA6 — the journey half of the cross-domain photo-access checks +// (Plan 3g Task 4), onto `JourneysRepository.isOwnedByUser`/ +// `JourneyContributorsRepository.existsForUser` via the private +// `ownerOrContributor` helper. MEM-ACCESS-006/006b/006c/014/014b above +// already cover owner (006b), editor (006/014) and viewer (006c/014b) +// contributors passing, and MEM-ACCESS-007/008/016 cover a stranger/no-link +// case refused — this block is the explicit mutation-proof line item the +// plan's "security-critical" flag on these three sites calls for. +// +// Mutation proof (performed once by hand, not run in CI): editing +// `MemoriesAccessService.ownerOrContributor` from +// `if (await this.journeys.isOwnedByUser(...)) return true; return await +// this.journeyContributors.existsForUser(...);` +// to an AND (`return (await this.journeys.isOwnedByUser(...)) && +// (await this.journeyContributors.existsForUser(...));`) — collapsing the +// owner-OR-contributor union into an owner-AND-contributor intersection — +// turns MEM-ACCESS-006 (an editor who is NOT also the owner) and +// MEM-ACCESS-006c (a viewer who is NOT also the owner) red: both contributors +// incorrectly lose access, since neither is also a row in `journeys` for that +// id. MEM-ACCESS-006b (the journey owner, who has no `journey_contributors` +// row of their own reachable through this photo's read) would ALSO go red +// under the same mutation. Confirmed by hand; the fix is reverted here. +// --------------------------------------------------------------------------- + +describe('canAccessUserPhoto/canAccessTrekPhoto — MA1/MA2/MA6 mutation-proof coverage', () => { + it('MEMACCESS-MA2-001: an editor who is not the journey owner still passes (breaks under an owner-AND-contributor mutation)', async () => { + const { user: owner } = createUser(testDb); + const { user: editor } = createUser(testDb, { username: 'ma2-editor' }); + const journeyId = makeJourney(owner.id); + const photoId = Number(testDb.prepare( + "INSERT INTO trek_photos (provider, asset_id, owner_id) VALUES ('immich', 'ma2-editor-asset', ?)" + ).run(owner.id).lastInsertRowid); + testDb.prepare('INSERT INTO journey_photos (journey_id, photo_id, created_at) VALUES (?, ?, 0)').run(journeyId, photoId); + testDb.prepare("INSERT INTO journey_contributors (journey_id, user_id, role, added_at) VALUES (?, ?, 'editor', 0)").run(journeyId, editor.id); + + expect(await access.canAccessUserPhoto(editor.id, owner.id, '0', 'ma2-editor-asset', 'immich')).toBe(true); + expect(await access.canAccessTrekPhoto(editor.id, photoId)).toBe(true); + }); +}); + // --------------------------------------------------------------------------- // Album link syncing — MA7/MA8 (getAlbumIdFromLink/getAlbumLinkForSync, one // shared `findScoped` repository method) and MA9 (updateSyncTimeForAlbumLink). diff --git a/server/tests/unit/nest/oidc.service.test.ts b/server/tests/unit/nest/oidc.service.test.ts index ecc6b282c..09b6f282b 100644 --- a/server/tests/unit/nest/oidc.service.test.ts +++ b/server/tests/unit/nest/oidc.service.test.ts @@ -120,6 +120,8 @@ import { } from '../../helpers/test-uow'; import { budgetRepoArgs } from '../../helpers/budget-repos'; import { createTestBudgetItemsRepo } from '../../helpers/files-repos'; +import { createTestJourneysRepo, createTestJourneyEntriesRepo, createTestJourneyContributorsRepo } from '../../helpers/journey-repos'; +import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-repos'; // MailerService is injected since the notifications fold — a stub instead of a // module mock. sendPasswordResetEmail is the only thing auth reaches for. @@ -139,7 +141,7 @@ beforeAll(async () => { new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), membership, new WebauthnConfigService(await createTestAppSettingsRepo(testDb)), - new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(testDb), ...(await budgetRepoArgs(testDb))), await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestBudgetItemsRepo(testDb)), + new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(testDb), ...(await budgetRepoArgs(testDb))), await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestBudgetItemsRepo(testDb), await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb)), mailerStub, new EphemeralTokenService(), new AllowedFileTypesService(await createTestAppSettingsRepo(testDb)), await createTestUnitOfWork(testDb), diff --git a/server/tests/unit/nest/passkey.service.test.ts b/server/tests/unit/nest/passkey.service.test.ts index c6bbfae98..b79a55224 100644 --- a/server/tests/unit/nest/passkey.service.test.ts +++ b/server/tests/unit/nest/passkey.service.test.ts @@ -104,6 +104,8 @@ import { } from '../../helpers/test-uow'; import { budgetRepoArgs } from '../../helpers/budget-repos'; import { createTestBudgetItemsRepo } from '../../helpers/files-repos'; +import { createTestJourneysRepo, createTestJourneyEntriesRepo, createTestJourneyContributorsRepo } from '../../helpers/journey-repos'; +import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-repos'; // MailerService is injected since the notifications fold — a stub instead of a // module mock. sendPasswordResetEmail is the only thing auth reaches for. @@ -132,7 +134,7 @@ beforeAll(async () => { new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new TripMembershipService(await createTestTripsRepo(testDb), await createTestTripMembersRepo(testDb)), new WebauthnConfigService(await createTestAppSettingsRepo(testDb)), - new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(testDb), ...(await budgetRepoArgs(testDb))), await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestBudgetItemsRepo(testDb)), + new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(testDb), ...(await budgetRepoArgs(testDb))), await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestBudgetItemsRepo(testDb), await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb)), mailerStub, new EphemeralTokenService(), new AllowedFileTypesService(await createTestAppSettingsRepo(testDb)), await createTestUnitOfWork(testDb), diff --git a/server/tests/unit/nest/photo-resolver.service.test.ts b/server/tests/unit/nest/photo-resolver.service.test.ts index 3b6701f8c..3b90cb979 100644 --- a/server/tests/unit/nest/photo-resolver.service.test.ts +++ b/server/tests/unit/nest/photo-resolver.service.test.ts @@ -37,6 +37,7 @@ import { DatabaseService } from '../../../src/nest/database/database.service'; import { TrekPhotoRegistrationService } from '../../../src/nest/photos/trek-photos.repository'; import { TrekPhotos } from '../../../src/db/entities/TrekPhotos.entity'; import { TripPhotos } from '../../../src/db/entities/TripPhotos.entity'; +import { JourneyPhotos } from '../../../src/db/entities/JourneyPhotos.entity'; import { createTestOrm, type TestOrm } from '../../helpers/test-orm'; import { PhotoResolverService } from '../../../src/nest/memories/photo-resolver.service'; import type { ImmichService } from '../../../src/nest/memories/immich.service'; @@ -100,7 +101,7 @@ beforeAll(async () => { createTables(testDb); runMigrations(testDb); t = await createTestOrm(testDb); - const repo = new TrekPhotoRegistrationService(t.repo(TrekPhotos), t.repo(TripPhotos), dbs); + const repo = new TrekPhotoRegistrationService(t.repo(TrekPhotos), t.repo(TripPhotos), t.repo(JourneyPhotos), dbs); svc = new PhotoResolverService( repo, thumbnails as unknown as ThumbnailService, diff --git a/server/tests/unit/nest/places.directions.test.ts b/server/tests/unit/nest/places.directions.test.ts index 1318778eb..63665516e 100644 --- a/server/tests/unit/nest/places.directions.test.ts +++ b/server/tests/unit/nest/places.directions.test.ts @@ -97,7 +97,7 @@ async function svc(searchNominatim: MapsService['searchNominatim']): Promise { t = await createTestOrm(testDb); - repo = new TrekPhotoRegistrationService(t.repo(TrekPhotos), t.repo(TripPhotos), new DatabaseService(testDb, t.em)); + repo = new TrekPhotoRegistrationService(t.repo(TrekPhotos), t.repo(TripPhotos), t.repo(JourneyPhotos), new DatabaseService(testDb, t.em)); }); beforeEach(() => { resetTestDb(testDb); t.clear(); }); @@ -214,7 +215,7 @@ describe('TrekPhotoRegistrationService.deleteIfOrphan (PH10-11)', () => { expect(rawRow(id)).toBeDefined(); }); - it('TREKPHOTO-062: a photo still referenced by journey_photos (Plan 3g, raw) is kept', async () => { + it('TREKPHOTO-062: a photo still referenced by journey_photos (Plan 3g, converted onto JourneyPhotosRepository.existsForPhoto) is kept', async () => { const { user } = createUser(testDb); const id = await repo.getOrCreate('immich', 'referenced-2', user.id); testDb.prepare("INSERT INTO journeys (user_id, title, status, created_at, updated_at) VALUES (?, 'J', 'draft', 0, 0)").run(user.id); diff --git a/server/tests/unit/nest/trip-members.service.test.ts b/server/tests/unit/nest/trip-members.service.test.ts index 5a4bc8be9..d8ecc3dc5 100644 --- a/server/tests/unit/nest/trip-members.service.test.ts +++ b/server/tests/unit/nest/trip-members.service.test.ts @@ -70,6 +70,8 @@ import { notificationsStub } from '../../helpers/notifications'; import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, createTestTripsRepo, createTestTripMembersRepo, sharedTestOrm } from '../../helpers/test-uow'; import { budgetRepoArgs } from '../../helpers/budget-repos'; import { createTestBudgetItemsRepo } from '../../helpers/files-repos'; +import { createTestJourneysRepo, createTestJourneyEntriesRepo, createTestJourneyContributorsRepo } from '../../helpers/journey-repos'; +import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-repos'; import type { EntityManager } from '@mikro-orm/core'; import type { TripsRepository } from '../../../src/db/repositories/Trips.repository'; import type { UsersRepository } from '../../../src/db/repositories/Users.repository'; @@ -102,7 +104,7 @@ beforeAll(async () => { budgetSvc = new BudgetService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(dbs().connection), ...(await budgetRepoArgs(dbs().connection))); roster = new TripMembersService( dbs(), budgetSvc, - new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), usersRepo, await createTestBudgetItemsRepo(dbs().connection)), + new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), usersRepo, await createTestBudgetItemsRepo(dbs().connection), await createTestJourneyShareTokensRepo(dbs().connection), await createTestJourneysRepo(dbs().connection), await createTestJourneyEntriesRepo(dbs().connection), await createTestJourneyContributorsRepo(dbs().connection)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(notifySend), await createTestUnitOfWork(dbs().connection), tripsRepo, tripMembersRepo, usersRepo, diff --git a/server/tests/unit/nest/trip-read-model.service.test.ts b/server/tests/unit/nest/trip-read-model.service.test.ts index f1a14e4d2..09c4962f3 100644 --- a/server/tests/unit/nest/trip-read-model.service.test.ts +++ b/server/tests/unit/nest/trip-read-model.service.test.ts @@ -108,6 +108,7 @@ import { createTestJourneysRepo, createTestJourneyContributorsRepo, createTestJourneyTripsRepo, createTestJourneyEntriesRepo, createTestJourneyPhotosRepo, createTestJourneyEntryPhotosRepo, } from '../../helpers/journey-repos'; +import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-repos'; // Real sibling services over the same in-memory DB — the aggregation runs the // actual SQL of every domain it fans out to, so a shape change downstream shows @@ -155,7 +156,7 @@ beforeAll(async () => { new MapsService(dbs(), photoCache, await createTestAppSettingsRepo(dbs().connection), await createTestUsersRepo(dbs().connection)), new QueryHelpersService(await createTestTagsRepo(dbs().connection), await createTestPlaceRatingsRepo(dbs().connection), await createTestAssignmentParticipantsRepo(dbs().connection)), new UnsplashService(await createTestAppSettingsRepo(dbs().connection), await createTestUsersRepo(dbs().connection), new RuntimeEnvService(), makeStorageFixture('').storage), photoCache, new JourneyDomainService( - dbs(), new RealtimeService(), new TrekPhotoRegistrationService((await sharedTestOrm(testDb)).repo(TrekPhotos), (await sharedTestOrm(testDb)).repo(TripPhotos), dbs()), await createTestUnitOfWork(dbs().connection), + dbs(), new RealtimeService(), new TrekPhotoRegistrationService((await sharedTestOrm(testDb)).repo(TrekPhotos), (await sharedTestOrm(testDb)).repo(TripPhotos), await createTestJourneyPhotosRepo(dbs().connection), dbs()), await createTestUnitOfWork(dbs().connection), await createTestJourneysRepo(dbs().connection), await createTestJourneyContributorsRepo(dbs().connection), await createTestJourneyTripsRepo(dbs().connection), await createTestJourneyEntriesRepo(dbs().connection), await createTestTripsRepo(dbs().connection), // Plan 3g Task 2 constructor-ripple: JourneyPhotosRepository/JourneyEntryPhotosRepository/PlacesRepository. @@ -172,7 +173,7 @@ beforeAll(async () => { await createTestTripsRepo(dbs().connection), await createTestBudgetItemsRepo(dbs().connection), ); - membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), await createTestUsersRepo(dbs().connection), await createTestBudgetItemsRepo(dbs().connection)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(dbs().connection), await createTestTripsRepo(dbs().connection), await createTestTripMembersRepo(dbs().connection), await createTestUsersRepo(dbs().connection)); + membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), await createTestUsersRepo(dbs().connection), await createTestBudgetItemsRepo(dbs().connection), await createTestJourneyShareTokensRepo(dbs().connection), await createTestJourneysRepo(dbs().connection), await createTestJourneyEntriesRepo(dbs().connection), await createTestJourneyContributorsRepo(dbs().connection)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(dbs().connection), await createTestTripsRepo(dbs().connection), await createTestTripMembersRepo(dbs().connection), await createTestUsersRepo(dbs().connection)); }); const buildReadModel = async (tripsRepo: TripsRepository, roster: TripMembersService = membersSvc) => diff --git a/server/tests/unit/nest/trips.service.test.ts b/server/tests/unit/nest/trips.service.test.ts index 88a3334c3..7c8228679 100644 --- a/server/tests/unit/nest/trips.service.test.ts +++ b/server/tests/unit/nest/trips.service.test.ts @@ -83,6 +83,8 @@ import { JourneyDomainService } from '../../../src/nest/journey/journey-domain.s import { TrekPhotoRegistrationService } from '../../../src/nest/photos/trek-photos.repository'; import { TrekPhotos } from '../../../src/db/entities/TrekPhotos.entity'; import { TripPhotos } from '../../../src/db/entities/TripPhotos.entity'; +import { JourneyPhotos } from '../../../src/db/entities/JourneyPhotos.entity'; +import { JourneyEntries } from '../../../src/db/entities/JourneyEntries.entity'; import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; import { makeStorageFixture } from '../../helpers/storage-fixture'; import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service'; @@ -123,6 +125,7 @@ import { createTestJourneysRepo, createTestJourneyContributorsRepo, createTestJourneyTripsRepo, createTestJourneyEntriesRepo, createTestJourneyPhotosRepo, createTestJourneyEntryPhotosRepo, } from '../../helpers/journey-repos'; +import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-repos'; // Real sibling services over the same in-memory DB — updateTrip's date-shift // resyncs and the summary/bundle aggregation run their actual SQL. @@ -198,7 +201,7 @@ beforeAll(async () => { new UnsplashService(await createTestAppSettingsRepo(dbs().connection), await createTestUsersRepo(dbs().connection), new RuntimeEnvService(), coversFx.storage), photoCache, new JourneyDomainService( - dbs(), new RealtimeService(), new TrekPhotoRegistrationService(dbsEm!.getRepository(TrekPhotos), dbsEm!.getRepository(TripPhotos), dbs()), await createTestUnitOfWork(dbs().connection), + dbs(), new RealtimeService(), new TrekPhotoRegistrationService(dbsEm!.getRepository(TrekPhotos), dbsEm!.getRepository(TripPhotos), dbsEm!.getRepository(JourneyPhotos), dbs()), await createTestUnitOfWork(dbs().connection), await createTestJourneysRepo(dbs().connection), await createTestJourneyContributorsRepo(dbs().connection), await createTestJourneyTripsRepo(dbs().connection), await createTestJourneyEntriesRepo(dbs().connection), await createTestTripsRepo(dbs().connection), // Plan 3g Task 2 constructor-ripple: JourneyPhotosRepository/JourneyEntryPhotosRepository/PlacesRepository. @@ -236,7 +239,7 @@ beforeAll(async () => { await createTestUnitOfWork(dbs().connection), (await sharedTestOrm(testDb)).em, ); - membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), await createTestUsersRepo(dbs().connection), await createTestBudgetItemsRepo(dbs().connection)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(dbs().connection), await createTestTripsRepo(dbs().connection), await createTestTripMembersRepo(dbs().connection), await createTestUsersRepo(dbs().connection)); + membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), await createTestUsersRepo(dbs().connection), await createTestBudgetItemsRepo(dbs().connection), await createTestJourneyShareTokensRepo(dbs().connection), await createTestJourneysRepo(dbs().connection), await createTestJourneyEntriesRepo(dbs().connection), await createTestJourneyContributorsRepo(dbs().connection)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(dbs().connection), await createTestTripsRepo(dbs().connection), await createTestTripMembersRepo(dbs().connection), await createTestUsersRepo(dbs().connection)); readModelSvc = new TripReadModelService( await createTestTripsRepo(dbs().connection), membersSvc, daysSvc, accommodationsSvc, budgetSvc, new PackingService( diff --git a/server/tests/unit/nest/unified-memories.service.test.ts b/server/tests/unit/nest/unified-memories.service.test.ts index 8d09ac505..b4bb8445d 100644 --- a/server/tests/unit/nest/unified-memories.service.test.ts +++ b/server/tests/unit/nest/unified-memories.service.test.ts @@ -51,6 +51,9 @@ import { TripPhotos } from '../../../src/db/entities/TripPhotos.entity'; import { TripAlbumLinks } from '../../../src/db/entities/TripAlbumLinks.entity'; import { Trips } from '../../../src/db/entities/Trips.entity'; import { PhotoProviders } from '../../../src/db/entities/PhotoProviders.entity'; +import { JourneyPhotos } from '../../../src/db/entities/JourneyPhotos.entity'; +import { Journeys } from '../../../src/db/entities/Journeys.entity'; +import { JourneyContributors } from '../../../src/db/entities/JourneyContributors.entity'; import type { ServiceResult } from '../../../src/nest/memories/memories.helpers'; import { DatabaseService } from '../../../src/nest/database/database.service'; import type { ImmichService } from '../../../src/nest/memories/immich.service'; @@ -117,10 +120,10 @@ beforeAll(async () => { usersRepo = await createTestUsersRepo(testDb); svc = new UnifiedMemoriesService( dbs, - new TrekPhotoRegistrationService(t.repo(TrekPhotos), t.repo(TripPhotos), dbs), + new TrekPhotoRegistrationService(t.repo(TrekPhotos), t.repo(TripPhotos), t.repo(JourneyPhotos), dbs), {} as ImmichService, {} as SynologyService, - new MemoriesAccessService(dbs, t.repo(TripPhotos), t.repo(TrekPhotos), t.repo(TripAlbumLinks), t.repo(Trips)), + new MemoriesAccessService(dbs, t.repo(TripPhotos), t.repo(TrekPhotos), t.repo(TripAlbumLinks), t.repo(Trips), t.repo(Journeys), t.repo(JourneyContributors), t.repo(JourneyPhotos)), notificationsStub(), await createTestAddonsService(testDb, dbs), await createTestUnitOfWork(testDb), diff --git a/server/tests/unit/nest/user-cleanup.service.test.ts b/server/tests/unit/nest/user-cleanup.service.test.ts index ff50daef7..20a82bee0 100644 --- a/server/tests/unit/nest/user-cleanup.service.test.ts +++ b/server/tests/unit/nest/user-cleanup.service.test.ts @@ -49,10 +49,16 @@ import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.se import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { BudgetService } from '../../../src/nest/budget/budget.service'; import type { BudgetItemsRepository } from '../../../src/db/repositories/BudgetItems.repository'; +import type { JourneyShareTokensRepository } from '../../../src/db/repositories/JourneyShareTokens.repository'; +import type { JourneysRepository } from '../../../src/db/repositories/Journeys.repository'; +import type { JourneyEntriesRepository } from '../../../src/db/repositories/JourneyEntries.repository'; +import type { JourneyContributorsRepository } from '../../../src/db/repositories/JourneyContributors.repository'; import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service'; import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm } from '../../helpers/test-uow'; import { budgetRepoArgs } from '../../helpers/budget-repos'; import { createTestBudgetItemsRepo } from '../../helpers/files-repos'; +import { createTestJourneysRepo, createTestJourneyEntriesRepo, createTestJourneyContributorsRepo } from '../../helpers/journey-repos'; +import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-repos'; const dbs = new DatabaseService(testDb); @@ -69,7 +75,12 @@ beforeAll(async () => { vi.spyOn(dbs, 'rosterUserIds').mockImplementation((...a) => real.rosterUserIds(...a)); vi.spyOn(dbs, 'getPlaceWithTags').mockImplementation((...a) => real.getPlaceWithTags(...a)); budget = new BudgetService(dbs, new PermissionsService(await createTestAppSettingsRepo(dbs.connection), await createTestUnitOfWork(dbs.connection)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(dbs.connection), ...(await budgetRepoArgs(dbs.connection))); - svc = new UserCleanupService(dbs, budget, await createTestUnitOfWork(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestBudgetItemsRepo(dbs.connection)); + svc = new UserCleanupService( + dbs, budget, await createTestUnitOfWork(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestBudgetItemsRepo(dbs.connection), + // Plan 3g Task 4 constructor-ripple: UC7-10's repositories. + await createTestJourneyShareTokensRepo(dbs.connection), await createTestJourneysRepo(dbs.connection), + await createTestJourneyEntriesRepo(dbs.connection), await createTestJourneyContributorsRepo(dbs.connection), + ); }); const installPlugin = (id: string, permissions: string[] | null) => { @@ -164,8 +175,14 @@ describe('erasePluginUserData', () => { const slim = new (require('better-sqlite3'))(':memory:'); slim.exec('CREATE TABLE users (id INTEGER PRIMARY KEY)'); slim.prepare('INSERT INTO users (id) VALUES (1)').run(); - // `erasePluginUserData` (this test's only call) never reaches `budgetItemsRepo` (UC5's own method) — a stub is enough. - const slimSvc = new UserCleanupService(new DatabaseService(slim), budget, await createTestUnitOfWork(slim), await createTestUsersRepo(slim), {} as unknown as BudgetItemsRepository); + // `erasePluginUserData` (this test's only call) never reaches `budgetItemsRepo` + // (UC5's own method) or the Plan 3g Task 4 journey repositories (UC7-10) — + // stubs are enough, and the slim schema has no journey tables to bind against. + const slimSvc = new UserCleanupService( + new DatabaseService(slim), budget, await createTestUnitOfWork(slim), await createTestUsersRepo(slim), {} as unknown as BudgetItemsRepository, + {} as unknown as JourneyShareTokensRepository, {} as unknown as JourneysRepository, + {} as unknown as JourneyEntriesRepository, {} as unknown as JourneyContributorsRepository, + ); await expect(slimSvc.erasePluginUserData(1)).resolves.toBeUndefined(); @@ -205,6 +222,46 @@ describe('deleteUserCompletely', () => { expect(testDb.prepare('SELECT COUNT(*) AS c FROM journey_entries').get()).toEqual({ c: 0 }); }); + it('USER-CLEANUP-010: GDPR erasure — UC7-10 remove every journey-table row the departing user reaches, and only those', async () => { + const { user: victim } = createUser(testDb, { username: 'gdpr-victim' }); + const { user: second } = createUser(testDb, { username: 'gdpr-second' }); + const { user: third } = createUser(testDb, { username: 'gdpr-third' }); + + // Victim owns a journey with an entry, a gallery photo and a share token + // they created — every one of these is reached by UC8's FK cascade. + const ownJourney = createJourney(victim.id, 'Mine'); + testDb.prepare("INSERT INTO journey_entries (journey_id, author_id, type, title, entry_date, created_at, updated_at) VALUES (?, ?, 'entry', 'Own entry', '2026-01-01', 0, 0)") + .run(ownJourney, victim.id); + const photoId = Number(testDb.prepare("INSERT INTO trek_photos (provider, owner_id, created_at) VALUES ('local', ?, 0)").run(victim.id).lastInsertRowid); + testDb.prepare('INSERT INTO journey_photos (journey_id, photo_id, created_at) VALUES (?, ?, 0)').run(ownJourney, photoId); + testDb.prepare("INSERT INTO journey_share_tokens (journey_id, token, created_by) VALUES (?, 'own-tok', ?)").run(ownJourney, victim.id); + + // Victim contributes (editor) to a SECOND user's journey — not owned, so UC8's cascade never reaches it; UC10 must. + const secondJourney = createJourney(second.id, 'Theirs'); + testDb.prepare("INSERT INTO journey_contributors (journey_id, user_id, role, added_at) VALUES (?, ?, 'editor', 0)").run(secondJourney, victim.id); + + // Victim authored an entry, and separately created a share link, on a THIRD user's journey — UC9/UC7 must catch these. + const thirdJourney = createJourney(third.id, 'Elsewhere'); + testDb.prepare("INSERT INTO journey_entries (journey_id, author_id, type, title, entry_date, created_at, updated_at) VALUES (?, ?, 'entry', 'Guest entry', '2026-01-02', 0, 0)") + .run(thirdJourney, victim.id); + testDb.prepare("INSERT INTO journey_share_tokens (journey_id, token, created_by) VALUES (?, 'third-tok', ?)").run(thirdJourney, victim.id); + + await svc.deleteUserCompletely(victim.id); + + // Everything the departing user owned is gone (UC8 + cascade). + expect(testDb.prepare('SELECT id FROM journeys WHERE id = ?').get(ownJourney)).toBeUndefined(); + expect(testDb.prepare('SELECT COUNT(*) AS c FROM journey_entries WHERE journey_id = ?').get(ownJourney)).toEqual({ c: 0 }); + expect(testDb.prepare('SELECT COUNT(*) AS c FROM journey_photos WHERE journey_id = ?').get(ownJourney)).toEqual({ c: 0 }); + expect(testDb.prepare('SELECT COUNT(*) AS c FROM journey_share_tokens WHERE journey_id = ?').get(ownJourney)).toEqual({ c: 0 }); + // Everything the departing user merely touched on OTHER users' journeys is gone too (UC7/UC9/UC10). + expect(testDb.prepare('SELECT id FROM journey_entries WHERE journey_id = ? AND author_id = ?').get(thirdJourney, victim.id)).toBeUndefined(); + expect(testDb.prepare('SELECT user_id FROM journey_contributors WHERE journey_id = ? AND user_id = ?').get(secondJourney, victim.id)).toBeUndefined(); + expect(testDb.prepare('SELECT id FROM journey_share_tokens WHERE journey_id = ? AND created_by = ?').get(thirdJourney, victim.id)).toBeUndefined(); + // Nothing extra removed: the other two users' own journeys survive. + expect(testDb.prepare('SELECT id FROM journeys WHERE id = ?').get(secondJourney)).toBeDefined(); + expect(testDb.prepare('SELECT id FROM journeys WHERE id = ?').get(thirdJourney)).toBeDefined(); + }); + it('USER-CLEANUP-008: re-derives the expense divisor before the member rows cascade away', async () => { const { user: owner } = createUser(testDb); const { user: victim } = createUser(testDb, { username: 'victim' }); @@ -234,7 +291,11 @@ describe('deleteUserCompletely', () => { const usersRepo = await createTestUsersRepo(testDb); const deleteByIdSpy = vi.spyOn(usersRepo, 'deleteById').mockRejectedValue(new Error('boom')); try { - await expect(new UserCleanupService(dbs, budget, await createTestUnitOfWork(testDb), usersRepo, await createTestBudgetItemsRepo(testDb)).deleteUserCompletely(victim.id)).rejects.toThrow('boom'); + await expect(new UserCleanupService( + dbs, budget, await createTestUnitOfWork(testDb), usersRepo, await createTestBudgetItemsRepo(testDb), + await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), + await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb), + ).deleteUserCompletely(victim.id)).rejects.toThrow('boom'); expect(testDb.prepare('SELECT id FROM users WHERE id = ?').get(victim.id)).toBeDefined(); expect((testDb.prepare('SELECT invited_by FROM trip_members WHERE user_id = ?').get(owner.id) as { invited_by: number | null }).invited_by).toBe(victim.id); diff --git a/server/tests/unit/plugins/journal-entry-rows.controller.test.ts b/server/tests/unit/plugins/journal-entry-rows.controller.test.ts index b0f945ab4..2df1506cf 100644 --- a/server/tests/unit/plugins/journal-entry-rows.controller.test.ts +++ b/server/tests/unit/plugins/journal-entry-rows.controller.test.ts @@ -8,18 +8,13 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; const { entryJourney, canAccessJourney, isAddonEnabled, pluginsEnabled } = vi.hoisted(() => ({ - entryJourney: vi.fn((entryId: number) => (entryId === 7 ? { journey_id: 3 } : undefined)), + entryJourney: vi.fn((entryId: number) => (entryId === 7 ? { journey_id: 3 } : null)), canAccessJourney: vi.fn((journeyId: number, userId: number) => (journeyId === 3 && userId === 5 ? { id: 3 } : null)), isAddonEnabled: vi.fn(() => true), pluginsEnabled: vi.fn(() => true), })); -vi.mock('../../../src/db/database', () => ({ - db: { prepare: () => ({ get: (entryId: number) => entryJourney(entryId) }) }, - canAccessTrip: vi.fn(), -})); -import { db as dbConn } from '../../../src/db/database'; -import { DatabaseService } from '../../../src/nest/database/database.service'; import type { JourneyDomainService } from '../../../src/nest/journey/journey-domain.service'; +import type { JourneyEntriesRepository } from '../../../src/db/repositories/JourneyEntries.repository'; vi.mock('../../../src/nest/plugins/kill-switch', () => ({ pluginsEnabled })); import { JournalEntryRowsController } from '../../../src/nest/plugins/contributions/journal-entry-rows.controller'; @@ -27,6 +22,10 @@ import type { PluginHooks } from '../../../src/nest/plugins/plugin-hooks.service import type { AddonsService } from '../../../src/nest/addons/addons.service'; const addonsStub = { isAddonEnabled } as unknown as AddonsService; +// JEC1 (Plan 3g Task 4) — `JournalEntryRowsController` now injects +// `JourneyEntriesRepository` instead of the raw `DatabaseService`/`db.prepare` +// stub this test used to mock; `findById` stands in for the converted read. +const journeyEntriesStub = { findById: vi.fn((entryId: number) => entryJourney(entryId)) } as unknown as JourneyEntriesRepository; // eslint-disable-next-line @typescript-eslint/no-explicit-any const req = (id?: number) => ({ user: id === undefined ? undefined : { id } }) as any; @@ -35,7 +34,7 @@ function controller(invoke: (id: string) => unknown, providers = ['p1']) { providersOf: vi.fn(() => providers), journalRows: vi.fn(async (id: string) => invoke(id)), } as unknown as PluginHooks; - return { c: new JournalEntryRowsController(runtime, new DatabaseService(dbConn), addonsStub, { canAccessJourney } as unknown as JourneyDomainService), runtime }; + return { c: new JournalEntryRowsController(runtime, journeyEntriesStub, addonsStub, { canAccessJourney } as unknown as JourneyDomainService), runtime }; } const row = (over: Record = {}) => ({ label: 'Distance', value: '12 km', ...over }); diff --git a/server/tests/unit/services/conflictUpdate.test.ts b/server/tests/unit/services/conflictUpdate.test.ts index 77038603d..198fd1339 100644 --- a/server/tests/unit/services/conflictUpdate.test.ts +++ b/server/tests/unit/services/conflictUpdate.test.ts @@ -138,7 +138,7 @@ beforeAll(async () => { new UnsplashService(await createTestAppSettingsRepo(dbs.connection), await createTestUsersRepo(dbs.connection), runtimeEnv, makeStorageFixture('').storage), photoCache, new JourneyDomainService( - dbs, realtime, new TrekPhotoRegistrationService(t.repo(TrekPhotos), t.repo(TripPhotos), dbs), await createTestUnitOfWork(dbs.connection), + dbs, realtime, new TrekPhotoRegistrationService(t.repo(TrekPhotos), t.repo(TripPhotos), await createTestJourneyPhotosRepo(dbs.connection), dbs), await createTestUnitOfWork(dbs.connection), await createTestJourneysRepo(dbs.connection), await createTestJourneyContributorsRepo(dbs.connection), await createTestJourneyTripsRepo(dbs.connection), await createTestJourneyEntriesRepo(dbs.connection), await createTestTripsRepo(dbs.connection), // Plan 3g Task 2 constructor-ripple: JourneyPhotosRepository/JourneyEntryPhotosRepository/PlacesRepository. diff --git a/server/tests/unit/services/trekPhotoMedia.test.ts b/server/tests/unit/services/trekPhotoMedia.test.ts index db29de634..83a1fc233 100644 --- a/server/tests/unit/services/trekPhotoMedia.test.ts +++ b/server/tests/unit/services/trekPhotoMedia.test.ts @@ -26,6 +26,7 @@ import { createUser } from '../../helpers/factories'; import { TrekPhotoRegistrationService } from '../../../src/nest/photos/trek-photos.repository'; import { TrekPhotos } from '../../../src/db/entities/TrekPhotos.entity'; import { TripPhotos } from '../../../src/db/entities/TripPhotos.entity'; +import { JourneyPhotos } from '../../../src/db/entities/JourneyPhotos.entity'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { db as trekDb } from '../../../src/db/database'; import { createTestOrm, type TestOrm } from '../../helpers/test-orm'; @@ -42,7 +43,7 @@ beforeAll(async () => { createTables(testDb); runMigrations(testDb); t = await createTestOrm(testDb); - trekPhotos = new TrekPhotoRegistrationService(t.repo(TrekPhotos), t.repo(TripPhotos), new DatabaseService(trekDb, t.em)); + trekPhotos = new TrekPhotoRegistrationService(t.repo(TrekPhotos), t.repo(TripPhotos), t.repo(JourneyPhotos), new DatabaseService(trekDb, t.em)); }); beforeEach(() => {