diff --git a/server/src/db/dialect/sql-functions.ts b/server/src/db/dialect/sql-functions.ts
index 63c91a408..4026cd124 100644
--- a/server/src/db/dialect/sql-functions.ts
+++ b/server/src/db/dialect/sql-functions.ts
@@ -157,12 +157,36 @@ export function nowMinusDays(platform: Platform, days: number): RawQueryFragment
return unsupported(platform);
}
-/** `TRIM()`. Pairs with `lower()` for the `LOWER(TRIM(name))` shape (`places.service.ts:642`). */
+/**
+ * `TRIM()`, standalone. **Does NOT compose with `lower()`** to build
+ * `LOWER(TRIM(name))` (Task 0b review M3): `lower()`'s `ref` argument goes
+ * through `column()`, whose regex rejects anything but a bare
+ * `alias.column` reference — `TRIM(name)` fails it — and even if it didn't,
+ * `coalesce()`'s own docstring in this file documents that a
+ * `RawQueryFragment` cannot be nested inside another one (`raw()`'s
+ * `[Symbol.toPrimitive]` only implements the `'string'` coercion hint, which
+ * is the object-key position, not template-literal interpolation). Use
+ * `lowerTrim()` below for the composed shape.
+ */
export function trim(platform: Platform, ref: string): RawQueryFragment {
if (platform instanceof SqlitePlatform) return raw(`TRIM(${column(ref)})`);
return unsupported(platform);
}
+/**
+ * `LOWER(TRIM())` as one fragment — the shape `places.service.ts:642`
+ * (`lower(trim(name))`, PL26) actually needs; `lower()`/`trim()` above
+ * cannot be composed to build it (see `trim()`'s docstring, Task 0b review
+ * M3). Typed `RawQueryFragment & symbol`, the same brand `lower()` carries,
+ * for use as a filter KEY: `{ [lowerTrim(platform, 'name')]: lowerParam(...) }`.
+ * SQLite's `LOWER()` is ASCII-only (program rule 18) — unchanged by the
+ * `TRIM()`.
+ */
+export function lowerTrim(platform: Platform, ref: string): RawQueryFragment & symbol {
+ if (platform instanceof SqlitePlatform) return raw(`LOWER(TRIM(${column(ref)}))`);
+ return unsupported(platform);
+}
+
/**
* `COALESCE(, )` — both sides column references, for the
* `COALESCE(a, b)` SELECT-projection shape (`days.service.ts:93-94`,
@@ -209,3 +233,36 @@ export function absDifference(platform: Platform, ref: string, value: number): R
return unsupported(platform);
}
+// ---------------------------------------------------------------------------
+// Plan 3c Task 1 — aggregate SELECT projections (`PlaceShadowPicksRepository
+// .totals`/`countBySource`/`countByLiveRank`, `place-shadow.service.ts:144,
+// 149, 157`). `alias` is a hand-written literal at every call site today
+// (never user input), validated the same way `column()` validates a column
+// reference, so a future caller cannot smuggle SQL through it either.
+// ---------------------------------------------------------------------------
+
+function alias(name: string): string {
+ if (!/^[a-z_][a-z0-9_]*$/i.test(name)) {
+ throw new Error(`sql-functions: not an alias: ${name}`);
+ }
+ return name;
+}
+
+/** `COUNT(*) as `, for a single-row totals read or a `GROUP BY` count column. */
+export function countAll(platform: Platform, aliasName: string): RawQueryFragment {
+ if (platform instanceof SqlitePlatform) return raw(`COUNT(*) as ${alias(aliasName)}`);
+ return unsupported(platform);
+}
+
+/** `MIN() as `. */
+export function minOf(platform: Platform, ref: string, aliasName: string): RawQueryFragment {
+ if (platform instanceof SqlitePlatform) return raw(`MIN(${column(ref)}) as ${alias(aliasName)}`);
+ return unsupported(platform);
+}
+
+/** `MAX() as `. */
+export function maxOf(platform: Platform, ref: string, aliasName: string): RawQueryFragment {
+ if (platform instanceof SqlitePlatform) return raw(`MAX(${column(ref)}) as ${alias(aliasName)}`);
+ return unsupported(platform);
+}
+
diff --git a/server/src/db/repositories/GooglePlacePhotoMeta.repository.ts b/server/src/db/repositories/GooglePlacePhotoMeta.repository.ts
index 180513a29..44d93f9d0 100644
--- a/server/src/db/repositories/GooglePlacePhotoMeta.repository.ts
+++ b/server/src/db/repositories/GooglePlacePhotoMeta.repository.ts
@@ -1,4 +1,72 @@
import type { GooglePlacePhotoMeta } from '../entities/GooglePlacePhotoMeta.entity';
import { TrekRepository } from './_shared/trek-repository';
-export class GooglePlacePhotoMetaRepository extends TrekRepository {}
+export class GooglePlacePhotoMetaRepository extends TrekRepository {
+ /**
+ * `SELECT attribution FROM google_place_photo_meta WHERE place_id = ? AND
+ * error_at IS NULL` (`place-photo-cache.service.ts:76-79`, PP1). `place_id`
+ * is a TEXT key — pseudo-ids like `coords:lat:lng` are legal values, not
+ * only real Google place ids.
+ */
+ async findLive(placeId: string): Promise<{ attribution: string | null } | null> {
+ const row = await this.findOne({ place_id: placeId, error_at: null }, { fields: ['attribution'] });
+ return row ? { attribution: row.attribution ?? null } : null;
+ }
+
+ /**
+ * `DELETE FROM google_place_photo_meta WHERE place_id = ?` — ONE method for
+ * the two call sites that share this exact statement: PP2
+ * (`place-photo-cache.service.ts:87`, `get`'s self-healing delete after a
+ * storage-object miss) and PP7 (`:219`, `deleteEntry`, after an explicit
+ * storage delete).
+ */
+ async deleteByPlaceId(placeId: string): Promise {
+ await this.nativeDelete({ place_id: placeId });
+ }
+
+ /**
+ * `SELECT error_at FROM google_place_photo_meta WHERE place_id = ? AND
+ * error_at IS NOT NULL` (`place-photo-cache.service.ts:103-106`, PP3). The
+ * `Date.now() - row.error_at < MISSING_TTL` comparison stays the caller's
+ * (D4's defaults rule).
+ */
+ async findErrored(placeId: string): Promise<{ error_at: number } | null> {
+ const row = await this.findOne({ place_id: placeId, error_at: { $ne: null } }, { fields: ['error_at'] });
+ return row && row.error_at != null ? { error_at: row.error_at } : null;
+ }
+
+ /**
+ * `INSERT OR REPLACE INTO google_place_photo_meta (place_id, attribution,
+ * fetched_at, error_at) VALUES (?, NULL, ?, ?)` (`place-photo-cache.service.ts:131-134`,
+ * PP4, `markError`'s `'no-photo'` branch) — `attribution` forced `NULL` on
+ * every call, both timestamps ms-epoch INTEGERs (never `currentTimestamp`,
+ * which renders SQL text). `em.upsert` with `onConflictFields: ['place_id']`
+ * (the table's real PK, `Migration20200101014600_…`) and `onConflictAction:
+ * 'merge'` — `INSERT OR REPLACE` semantics for this single-column key.
+ */
+ async upsertError(placeId: string, at: number): Promise {
+ await this.upsert(
+ { place_id: placeId, attribution: null, fetched_at: at, error_at: at },
+ { onConflictFields: ['place_id'], onConflictAction: 'merge' },
+ );
+ }
+
+ /**
+ * `INSERT OR REPLACE INTO google_place_photo_meta (place_id, attribution,
+ * fetched_at, error_at) VALUES (?, ?, ?, NULL)` (`place-photo-cache.service.ts:161-164`,
+ * PP5, `put`) — `error_at` forced `NULL` on every call, PP4's mirror image
+ * on the same PK. Same `em.upsert` shape as `upsertError` above.
+ */
+ async upsertPhoto(placeId: string, attribution: string | null, at: number): Promise {
+ await this.upsert(
+ { place_id: placeId, attribution, fetched_at: at, error_at: null },
+ { onConflictFields: ['place_id'], onConflictAction: 'merge' },
+ );
+ }
+
+ /** `SELECT place_id FROM google_place_photo_meta` (`place-photo-cache.service.ts:239`, PP8) — the nightly/boot sweep's full-table scan. */
+ async listPlaceIds(): Promise {
+ const rows = await this.find({}, { fields: ['place_id'] });
+ return rows.map((r) => r.place_id).filter((id): id is string => id != null);
+ }
+}
diff --git a/server/src/db/repositories/PlaceDetailsCache.repository.ts b/server/src/db/repositories/PlaceDetailsCache.repository.ts
index 433331e9a..0d7bf32c3 100644
--- a/server/src/db/repositories/PlaceDetailsCache.repository.ts
+++ b/server/src/db/repositories/PlaceDetailsCache.repository.ts
@@ -1,4 +1,35 @@
import type { PlaceDetailsCache } from '../entities/PlaceDetailsCache.entity';
import { TrekRepository } from './_shared/trek-repository';
-export class PlaceDetailsCacheRepository extends TrekRepository {}
+export class PlaceDetailsCacheRepository extends TrekRepository {
+ /**
+ * `SELECT payload_json, fetched_at FROM place_details_cache WHERE place_id = ?
+ * AND lang = ? AND expanded = ?` (`place-enrichment.service.ts:810-815`,
+ * PE2). The caller passes `lang ?? ''` (D4's defaults rule — the column is
+ * `NOT NULL DEFAULT ''`, never NULL) and `expanded` bound to the
+ * `CACHE_KIND` constant.
+ */
+ async findEntry(placeId: string, lang: string, kind: number): Promise<{ payload_json: string; fetched_at: number } | null> {
+ const row = await this.findOne(
+ { place_id: placeId, lang, expanded: kind },
+ { fields: ['payload_json', 'fetched_at'] },
+ );
+ return row ? { payload_json: row.payload_json, fetched_at: row.fetched_at } : null;
+ }
+
+ /**
+ * `INSERT OR REPLACE INTO place_details_cache (place_id, lang, expanded,
+ * payload_json, fetched_at) VALUES (?, ?, ?, ?, ?)`
+ * (`place-enrichment.service.ts:847-854`, PE3). `em.upsert` with
+ * `onConflictFields: ['place_id', 'lang', 'expanded']` — the table's real
+ * composite PRIMARY KEY (`Migration20200101014700_…`,
+ * `PRIMARY KEY (place_id, lang, expanded)`), not just `place_id`: naming
+ * only the single column here would let `em.upsert` infer the wrong
+ * conflict target and silently insert duplicate rows for two different
+ * `lang`/`expanded` values on the same place. `fetched_at` is ms-epoch,
+ * never `currentTimestamp`.
+ */
+ async upsertEntry(row: { place_id: string; lang: string; expanded: number; payload_json: string; fetched_at: number }): Promise {
+ await this.upsert(row, { onConflictFields: ['place_id', 'lang', 'expanded'], onConflictAction: 'merge' });
+ }
+}
diff --git a/server/src/db/repositories/PlaceRatings.repository.ts b/server/src/db/repositories/PlaceRatings.repository.ts
index c74660529..8f1337afb 100644
--- a/server/src/db/repositories/PlaceRatings.repository.ts
+++ b/server/src/db/repositories/PlaceRatings.repository.ts
@@ -1,4 +1,43 @@
import type { PlaceRatings } from '../entities/PlaceRatings.entity';
import { TrekRepository } from './_shared/trek-repository';
-export class PlaceRatingsRepository extends TrekRepository {}
+/** One rated place, from `QueryHelpersService.loadRatingsByPlaceIds` (QH2). */
+export interface PlaceRatingForPlaceRow {
+ place_id: number;
+ user_id: number;
+ username: string;
+ avatar: string | null;
+ rating: number;
+}
+
+export class PlaceRatingsRepository extends TrekRepository {
+ /**
+ * `SELECT pr.place_id, pr.user_id, u.username, u.avatar, pr.rating
+ * FROM place_ratings pr JOIN users u ON pr.user_id = u.id
+ * WHERE pr.place_id IN (${…}) ORDER BY pr.created_at`
+ * (`query-helpers.service.ts:65-70`, QH2) — the batch loader behind
+ * `PlacesService.list`'s ratings column. Empty `placeIds` short-circuits
+ * before any query, matching the legacy service method (which never
+ * built the `IN (...)` placeholder list for an empty array either).
+ */
+ async listForPlaces(placeIds: number[]): Promise {
+ if (placeIds.length === 0) return [];
+ // `pr.place` selects the plain FK scalar as `place_id` (that relation is
+ // never separately joined here). `pr.user` IS separately joined (to `u`,
+ // for the username/avatar), and selecting a relation property that is
+ // ALSO an active join's target makes MikroORM alias the column off the
+ // JOINED entity's own primary key (`u__id`) instead of the FK scalar —
+ // verified directly, not assumed — so the user id is pulled explicitly
+ // off the join instead (`u.id as user_id`), the same "physical alias"
+ // shape `Places.repository.ts`'s `'c.name as category_name'` uses.
+ // Selecting the `persist(false)` scalar mirror directly (`pr.user_id`)
+ // was tried and silently drops the column from the result — also
+ // verified directly, not assumed.
+ return this.qb('pr')
+ .join('pr.user', 'u')
+ .select(['pr.place', 'u.id as user_id', 'u.username', 'u.avatar', 'pr.rating'])
+ .where({ 'pr.place': { $in: placeIds } })
+ .orderBy({ 'pr.created_at': 'asc' })
+ .execute('all', false);
+ }
+}
diff --git a/server/src/db/repositories/PlaceShadowPicks.repository.ts b/server/src/db/repositories/PlaceShadowPicks.repository.ts
index c6752bd80..013d5f816 100644
--- a/server/src/db/repositories/PlaceShadowPicks.repository.ts
+++ b/server/src/db/repositories/PlaceShadowPicks.repository.ts
@@ -1,4 +1,127 @@
import type { PlaceShadowPicks } from '../entities/PlaceShadowPicks.entity';
+import { countAll, maxOf, minOf, nowMinusDays } from '../dialect/sql-functions';
+import { toRow, type AssertRowKeys } from './_shared/rows';
import { TrekRepository } from './_shared/trek-repository';
-export class PlaceShadowPicksRepository extends TrekRepository {}
+/** A `place_shadow_picks` row as the API emits it. */
+export interface PlaceShadowPickRow {
+ id: number;
+ created_at: string;
+ query: string;
+ lang: string | null;
+ bias_lat: number | null;
+ bias_lng: number | null;
+ source: string;
+ live_rank: number;
+ live_count: number;
+ picked_name: string;
+ picked_lat: number;
+ picked_lng: number;
+ picked_place_id: string | null;
+}
+
+const _placeShadowPickRowKeys: AssertRowKeys = true;
+
+/**
+ * The column set of the legacy `record()` INSERT
+ * (`place-shadow.service.ts:97-113`, PS2): `query`, `source`, `live_rank`,
+ * `live_count`, `picked_name`, `picked_lat`, `picked_lng` required; the rest
+ * nullable. The `round()` rounding, the `?? null` defaulting and the
+ * `liveRank >= liveCount` drop all stay in `PlaceShadowService` (D4's
+ * defaults rule) — this writes exactly the row it is given.
+ */
+export interface NewPlaceShadowPickRow {
+ query: string;
+ lang: string | null;
+ bias_lat: number | null;
+ bias_lng: number | null;
+ source: string;
+ live_rank: number;
+ live_count: number;
+ picked_name: string;
+ picked_lat: number;
+ picked_lng: number;
+ picked_place_id: string | null;
+}
+
+export class PlaceShadowPicksRepository extends TrekRepository {
+ /** `INSERT INTO place_shadow_picks (query, lang, bias_lat, bias_lng, source, live_rank, live_count, picked_name, picked_lat, picked_lng, picked_place_id) VALUES (?×11)` (PS2). `this.insert`, no read-back — the caller (`record()`) only needs to know the write happened. */
+ async insertPick(row: NewPlaceShadowPickRow): Promise {
+ await this.insert({ ...row });
+ }
+
+ /**
+ * `SELECT * FROM place_shadow_picks WHERE id > ? ORDER BY id LIMIT ?`
+ * (`place-shadow.service.ts:124-131`, PS3) — `size` is the PAGE size; this
+ * fetches `size + 1` rows (the keyset-pagination probe row the caller uses
+ * to compute `nextAfter`), matching the legacy statement's own `LIMIT
+ * size + 1` exactly. Caller does the `after ?? 0` defaulting and the
+ * `min(max(1, limit), EXPORT_PAGE_SIZE)` clamp (D4's defaults rule).
+ */
+ async page(after: number, size: number): Promise {
+ const rows = await this.find({ id: { $gt: after } }, { orderBy: { id: 'asc' }, limit: size + 1 });
+ return rows.map((r) => toRow(r) as PlaceShadowPickRow);
+ }
+
+ /**
+ * `SELECT COUNT(*) AS total, MIN(created_at) AS oldest, MAX(created_at) AS newest
+ * FROM place_shadow_picks` (`place-shadow.service.ts:143-145`, PS4) — one
+ * row, always (SQLite's bare aggregate SELECT never returns zero rows),
+ * so `total` defaults to `0`/`oldest`/`newest` to `null` only for
+ * type-safety against an `undefined` row, not because the query itself
+ * can come back empty.
+ */
+ async totals(): Promise<{ total: number; oldest: string | null; newest: string | null }> {
+ const platform = this.getEntityManager().getPlatform();
+ const row = await this.qb('p')
+ .select([countAll(platform, 'total'), minOf(platform, 'p.created_at', 'oldest'), maxOf(platform, 'p.created_at', 'newest')])
+ .execute<{ total: number; oldest: string | null; newest: string | null } | undefined>('get', false);
+ return { total: row?.total ?? 0, oldest: row?.oldest ?? null, newest: row?.newest ?? null };
+ }
+
+ /**
+ * `SELECT source, COUNT(*) AS count FROM place_shadow_picks GROUP BY source
+ * ORDER BY count DESC` (PS5). The `ORDER BY` is applied in JS, not SQL:
+ * MikroORM's QueryBuilder cannot order by an ad-hoc raw-fragment alias
+ * (`countAll`'s `RawQueryFragment` return type carries no alias literal for
+ * `ExtractRawAliases` to pick up, so `.orderBy({ count: 'desc' })` throws
+ * "not existing property" at runtime, verified directly) — `Array.prototype
+ * .sort` is stable (ES2019+), so this produces the identical final row
+ * order the legacy `ORDER BY count DESC` did.
+ */
+ async countBySource(): Promise<{ source: string; count: number }[]> {
+ const platform = this.getEntityManager().getPlatform();
+ const rows = await this.qb('p')
+ .select(['p.source', countAll(platform, 'count')])
+ .groupBy('p.source')
+ .execute<{ source: string; count: number }[]>('all', false);
+ return [...rows].sort((a, b) => b.count - a.count);
+ }
+
+ /** `SELECT live_rank, COUNT(*) AS count FROM place_shadow_picks GROUP BY live_rank` (PS6) — no ORDER BY, matching the legacy statement; the caller buckets the rows itself. */
+ async countByLiveRank(): Promise<{ live_rank: number; count: number }[]> {
+ const platform = this.getEntityManager().getPlatform();
+ return this.qb('p')
+ .select(['p.live_rank', countAll(platform, 'count')])
+ .groupBy('p.live_rank')
+ .execute<{ live_rank: number; count: number }[]>('all', false);
+ }
+
+ /** `DELETE FROM place_shadow_picks` (PS7) — the admin wipe. Returns the affected-row count. */
+ async deleteAll(): Promise {
+ return this.nativeDelete({});
+ }
+
+ /**
+ * `` DELETE FROM place_shadow_picks WHERE created_at < datetime('now', ?) ``
+ * with `` `-${retentionDays} days` `` bound as a parameter
+ * (`place-shadow.service.ts:190-193`, PS8) — `nowMinusDays` renders the
+ * equivalent `datetime('now', '-N days')` fragment (it cannot bind the
+ * modifier as a parameter the way the legacy statement did; see the
+ * helper's own docstring). Returns the affected-row count.
+ */
+ async purgeOlderThan(retentionDays: number): Promise {
+ const platform = this.getEntityManager().getPlatform();
+ return this.nativeDelete({ created_at: { $lt: nowMinusDays(platform, retentionDays) } });
+ }
+}
diff --git a/server/src/db/repositories/Places.repository.ts b/server/src/db/repositories/Places.repository.ts
index 5b55d06a8..d4bdc30e6 100644
--- a/server/src/db/repositories/Places.repository.ts
+++ b/server/src/db/repositories/Places.repository.ts
@@ -1,5 +1,6 @@
import type { Places } from '../entities/Places.entity';
import type { Tags } from '../entities/Tags.entity';
+import { columnRef } from '../dialect/sql-functions';
import { type AssertRowKeys } from './_shared/rows';
import { TrekRepository } from './_shared/trek-repository';
@@ -121,17 +122,37 @@ export class PlacesRepository extends TrekRepository {
.where('p.id = ?', [place_id])
.execute('all', false);
+ // Task 0b review B1 (HIGH, live regression): `.select(['pr.user', ...])`
+ // makes MikroORM resolve `pr.user` through the ACTIVE `join('pr.user',
+ // 'u')` and emit the joined entity's own PK under its join alias
+ // (`u__id`), not the FK scalar (`user_id`) — `ratings[].user_id` never
+ // existed at runtime, silently breaking the client's
+ // `ratings.find(r => r.user_id === currentUserId)` own-rating lookup.
+ // `columnRef(...).as('user_id')` routes the FK column through the
+ // dialect layer and aliases it back explicitly; `'pr.user_id'` (the
+ // `persist(false)` scalar mirror) was tried and silently drops the
+ // column, `'pr.user as user_id'` was tried and is still `u__id` — both
+ // verified directly, not assumed (same finding independently reproduced
+ // in `PlaceRatingsRepository.listForPlaces`'s docstring, Plan 3c Task 1).
+ const platform = this.getEntityManager().getPlatform();
const ratings = await this.qb('p')
.join('p.place_ratings_collection', 'pr')
.join('pr.user', 'u')
- .select(['pr.user', 'u.username', 'u.avatar', 'pr.rating'])
+ .select([columnRef(platform, 'pr.user_id').as('user_id'), 'u.username', 'u.avatar', 'pr.rating'])
.where('p.id = ?', [place_id])
.orderBy({ 'pr.created_at': 'asc' })
.execute('all', false);
- const { category_name, category_color, category_icon, ...placeRest } = place;
+ // Task 0b review B2 (HIGH, live regression): the legacy `getPlaceWithTags`
+ // returned `{ ...place, category: ... }` over the raw driver row of
+ // `SELECT p.*, c.name as category_name, c.color as category_color, c.icon
+ // as category_icon` — the three alias columns ride along on the wire
+ // body (map popups, dashboard, shared-trip page all read them flat, per
+ // the review's blast-radius list). Read them, don't strip them: spread
+ // `place` whole, not `placeRest`.
+ const { category_name, category_color, category_icon } = place;
return {
- ...placeRest,
+ ...place,
category: place.category_id
? { id: place.category_id, name: category_name!, color: category_color!, icon: category_icon! }
: null,
@@ -141,4 +162,23 @@ export class PlacesRepository extends TrekRepository {
rating_count: ratings.length,
};
}
+
+ /**
+ * `SELECT 1 FROM places WHERE google_place_id = ? OR image_url = ? LIMIT 1`
+ * — the `places`-owned half of `PlacePhotoCacheService.isReferenced`
+ * (`place-photo-cache.service.ts:205-211`, PP6), split per the Plan 3c
+ * Task 1 PP6 ruling (option 2): the legacy single `UNION ALL … LIMIT 1`
+ * statement spans this table AND `collection_places` (Plan 3h's), so the
+ * service evaluates this half first and only runs the `collection_places`
+ * half when this one is `false` — reproducing the legacy `UNION ALL …
+ * LIMIT 1`'s short-circuit by evaluation order rather than by SQL.
+ */
+ async existsByGoogleIdOrImageUrl(googlePlaceId: string, imageUrl: string): Promise {
+ const row = await this.qb('p')
+ .select(['p.id'])
+ .where({ $or: [{ google_place_id: googlePlaceId }, { image_url: imageUrl }] })
+ .limit(1)
+ .execute<{ id: number } | undefined>('get', false);
+ return !!row;
+ }
}
diff --git a/server/src/db/repositories/Tags.repository.ts b/server/src/db/repositories/Tags.repository.ts
index b6024e3ee..fa034635b 100644
--- a/server/src/db/repositories/Tags.repository.ts
+++ b/server/src/db/repositories/Tags.repository.ts
@@ -14,6 +14,22 @@ export interface TagRow {
const _tagRowKeys: AssertRowKeys = true;
+/**
+ * One tag attached to one place, from `QueryHelpersService.loadTagsByPlaceIds`
+ * (QH1). `user_id` is present only on the non-compact projection — `listForPlaces`
+ * selects a narrower column set when `compact` is set, so a compact row never
+ * carries the key at all (matching the legacy's destructuring, which drops it
+ * rather than nulling it).
+ */
+export interface TagForPlaceRow {
+ place_id: number;
+ id: number;
+ user_id?: number;
+ name: string;
+ color: string | null;
+ created_at: string | null;
+}
+
export class TagsRepository extends TrekRepository {
/**
* `SELECT * FROM tags WHERE user_id = ? ORDER BY name ASC`, via the shared
@@ -115,10 +131,36 @@ export class TagsRepository extends TrekRepository {
return this.nativeDelete({ id });
}
- // `loadForPlaces` (a batch `findByPlaceIds`-style tag loader) is reserved
- // here for Plan 3c, which moves QueryHelpersService's tag batch loader
- // (`query-helpers.service.ts:42`, `SELECT t.*, pt.place_id FROM tags t …`)
- // onto TagsRepository (spec D4). Not implemented in Plan 3a Task 3 — no
- // throwing stub, just this reservation, so `nest/places`'s later migration
- // doesn't duplicate the method name.
+ /**
+ * `SELECT t.*, pt.place_id FROM tags t JOIN place_tags pt ON t.id = pt.tag_id
+ * WHERE pt.place_id IN (${…})` (`query-helpers.service.ts:41-45`, QH1) —
+ * ONE method, TWO projections (Plan 3c Task 1 ruling): `compact` selects
+ * `{id, name, color, created_at, place_id}`; the non-compact path selects
+ * every `tags` column except the join key (`{id, user_id, name, color,
+ * created_at, place_id}`), exactly the legacy's `t.*` minus `place_id`.
+ *
+ * Joined through `t.place_tags_inverse` (the inverse side of `Places.place_tags`,
+ * a many-to-many over the `place_tags` pivot table): selecting the joined
+ * place's own `id` as `place_id` is the same value the legacy's `pt.place_id`
+ * read off the pivot row directly (the pivot's `place_id` column IS the
+ * places row it points at), so this is not an approximation.
+ *
+ * Empty `placeIds` short-circuits before any query, matching the legacy
+ * service method.
+ */
+ async listForPlaces(placeIds: number[], options?: { compact?: boolean }): Promise {
+ if (placeIds.length === 0) return [];
+ if (options?.compact) {
+ return this.qb('t')
+ .join('t.place_tags_inverse', 'p')
+ .select(['t.id', 't.name', 't.color', 't.created_at', 'p.id as place_id'])
+ .where({ 'p.id': { $in: placeIds } })
+ .execute('all', false);
+ }
+ return this.qb('t')
+ .join('t.place_tags_inverse', 'p')
+ .select(['t.id', 't.user', 't.name', 't.color', 't.created_at', 'p.id as place_id'])
+ .where({ 'p.id': { $in: placeIds } })
+ .execute('all', false);
+ }
}
diff --git a/server/src/db/repositories/TripMembers.repository.ts b/server/src/db/repositories/TripMembers.repository.ts
index 5b8141898..800cd886e 100644
--- a/server/src/db/repositories/TripMembers.repository.ts
+++ b/server/src/db/repositories/TripMembers.repository.ts
@@ -42,4 +42,45 @@ export class TripMembersRepository extends TrekRepository {
if (owner) ids.add(owner.user_id);
return ids;
}
+
+ /**
+ * `SELECT user_id FROM trip_members WHERE trip_id = ? ORDER BY added_at ASC`
+ * (`trip-membership.service.ts:29-31`, TB2) — the owner is excluded by
+ * construction (the table has no row for the owner), matching the legacy
+ * statement exactly. Raw-bind (D4's T5 escape hatch), the same
+ * `number | string` seam `TripsRepository.findAccessible` documents.
+ */
+ async listUserIdsByTrip(trip_id: number | string): Promise {
+ const rows = await this.qb('m')
+ .select(['m.user'])
+ .where('m.trip_id = ?', [trip_id])
+ .orderBy({ 'm.added_at': 'asc' })
+ .execute<{ user_id: number }[]>('all', false);
+ return rows.map((r) => r.user_id);
+ }
+
+ /**
+ * `SELECT id FROM trip_members WHERE trip_id = ? AND user_id = ?`
+ * (`trip-membership.service.ts:70`, TB5) — `joinTripAsMember`'s
+ * already-a-member check, the second leg of the non-transactional
+ * check-then-act sequence (§18.4: unchanged by this conversion). Both
+ * callers (`TripMembershipService.joinTripAsMember`) always pass real
+ * `number`s, so no raw-bind seam here.
+ */
+ async exists(trip_id: number, user_id: number): Promise {
+ const row = await this.qb('m')
+ .select(['m.id'])
+ .where({ trip: trip_id, user: user_id })
+ .execute<{ id: number } | undefined>('get', false);
+ return !!row;
+ }
+
+ /**
+ * `INSERT INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, ?)`
+ * (`trip-membership.service.ts:72`, TB6) — the third leg of the same
+ * non-transactional sequence as `exists` above; `invitedBy` may be `null`.
+ */
+ async addMember(trip_id: number, user_id: number, invitedBy: number | null): Promise {
+ await this.insert({ trip: trip_id, user: user_id, invited_by: invitedBy });
+ }
}
diff --git a/server/src/db/repositories/Trips.repository.ts b/server/src/db/repositories/Trips.repository.ts
index dce84872e..d24a157df 100644
--- a/server/src/db/repositories/Trips.repository.ts
+++ b/server/src/db/repositories/Trips.repository.ts
@@ -9,6 +9,25 @@ export interface TripAccess {
}
export class TripsRepository extends TrekRepository {
+ /**
+ * The `LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ?
+ * WHERE (t.user_id = ? OR m.user_id IS NOT NULL)` half `findAccessible`
+ * and `listAccessibleIds` (Plan 3c Task 1, TB3) share — one join builder,
+ * per the task brief's ruling that the two must derive from the same
+ * source rather than hand-keeping two copies in sync (the cross-plan note
+ * in the inventory: TB3 and `TripsService.list`'s id half are "the same
+ * query with different projections"). No explicit return-type annotation,
+ * for the same inference reason `TrekRepository.kysely()` gives (the
+ * QueryBuilder's fluent generic parameters do not survive being spelled
+ * out via a separate type alias) — every caller chains `.select()`/
+ * `.where()`/`.orderBy()` straight off this method's return value.
+ */
+ private accessibleTripsQuery(user_id: number) {
+ return this.qb('t')
+ .leftJoin('t.trip_members_collection', 'm', { 'm.user': user_id })
+ .andWhere({ $or: [{ 't.user': user_id }, { 'm.user': { $ne: null } }] });
+ }
+
/**
* The trip if the user owns it or is a member of it, else undefined.
*
@@ -30,9 +49,8 @@ export class TripsRepository extends TrekRepository {
* affinity comparison of the unconverted string never matches one).
*/
async findAccessible(trip_id: number | string, user_id: number): Promise {
- const row = await this.qb('t')
+ const row = await this.accessibleTripsQuery(user_id)
.select(['t.id', 't.user', 't.currency'])
- .leftJoin('t.trip_members_collection', 'm', { 'm.user': user_id })
// Raw condition (D4's T5 escape hatch), not `.where({ 't.id': trip_id })`:
// MikroORM's typed filter rejects a `string` against `t.id`'s branded
// `number` type, and coercing to `Number(trip_id)` first would be
@@ -40,8 +58,7 @@ export class TripsRepository extends TrekRepository {
// `t.id`/`t.user_id` are the physical column names (not translated
// through the entity's `trip`/`user` property aliasing the way a typed
// filter object is), so the raw text names them directly.
- .where('t.id = ?', [trip_id])
- .andWhere({ $or: [{ 't.user': user_id }, { 'm.user': { $ne: null } }] })
+ .andWhere('t.id = ?', [trip_id])
// `mapResults: false` leaves the driver's row alone: the keys are the
// column names, which is why the result type is spelled out here rather
// than inferred from the entity's properties.
@@ -69,4 +86,50 @@ export class TripsRepository extends TrekRepository {
.execute<{ id: number } | undefined>('get', false);
return !!row;
}
+
+ /**
+ * `SELECT user_id FROM trips WHERE id = ?` (`trip-membership.service.ts:23`,
+ * TB1) — `row ? row.user_id : null` stays the caller's decision to make
+ * ("owner id for budget/MCP guards"); this returns exactly that shape.
+ * Raw-bind (D4's T5 escape hatch), same `number | string` seam as
+ * `findAccessible`/`isOwner` above.
+ */
+ async getOwnerId(trip_id: number | string): Promise {
+ const row = await this.qb('t')
+ .select(['t.user'])
+ .where('t.id = ?', [trip_id])
+ .execute<{ user_id: number } | undefined>('get', false);
+ return row ? row.user_id : null;
+ }
+
+ /**
+ * `SELECT id, user_id FROM trips WHERE id = ?` (`trip-membership.service.ts:66`,
+ * TB4) — `joinTripAsMember`'s first check-then-act read (§18.4: the
+ * sequence stays non-transactional, unchanged by this conversion). The
+ * only caller (`TripMembershipService.joinTripAsMember(tripId: number, …)`)
+ * always passes a real `number`, so this takes one too — no raw-bind seam
+ * to preserve here, unlike `findAccessible`/`isOwner`/`getOwnerId`.
+ */
+ async findIdAndOwner(trip_id: number): Promise<{ id: number; user_id: number } | undefined> {
+ return this.qb('t')
+ .select(['t.id', 't.user'])
+ .where({ id: trip_id })
+ .execute<{ id: number; user_id: number } | undefined>('get', false);
+ }
+
+ /**
+ * `SELECT t.id FROM trips t LEFT JOIN trip_members m ON m.trip_id = t.id
+ * AND m.user_id = :userId WHERE (t.user_id = :userId OR m.user_id IS NOT NULL)
+ * ORDER BY t.created_at DESC` (`trip-membership.service.ts:40-46`, TB3) —
+ * the id half of `TripsService.list(userId, null)`, sharing `findAccessible`'s
+ * join builder per the task brief's ruling (this class's `accessibleTripsQuery`
+ * above).
+ */
+ async listAccessibleIds(user_id: number): Promise {
+ const rows = await this.accessibleTripsQuery(user_id)
+ .select(['t.id'])
+ .orderBy({ 't.created_at': 'desc' })
+ .execute<{ id: number }[]>('all', false);
+ return rows.map((r) => r.id);
+ }
}
diff --git a/server/src/nest/place-enrichment/place-enrichment.module.ts b/server/src/nest/place-enrichment/place-enrichment.module.ts
index f9dffd6fb..8d5209749 100644
--- a/server/src/nest/place-enrichment/place-enrichment.module.ts
+++ b/server/src/nest/place-enrichment/place-enrichment.module.ts
@@ -1,9 +1,12 @@
import { Module } from '@nestjs/common';
+import { MikroOrmModule } from '@mikro-orm/nestjs';
import { MapsModule } from '../maps/maps.module';
import { PlacePhotosModule } from '../place-photos/place-photos.module';
import { RateLimitModule } from '../common/rate-limit.module';
import { PlaceEnrichmentController } from './place-enrichment.controller';
import { PlaceEnrichmentService } from './place-enrichment.service';
+import { PlaceDetailsCache } from '../../db/entities/PlaceDetailsCache.entity';
+import { AppSettings } from '../../db/entities/AppSettings.entity';
/**
* Place enrichment (L4 leaf module). Registered in AppModule.
@@ -13,9 +16,13 @@ import { PlaceEnrichmentService } from './place-enrichment.service';
* matters here: the photo cache's stampede guard and the maps concurrency limit
* only hold if this module queues behind the same ones everything else uses.
* Nothing outside the container consumes it, so there is no bridge.
+ *
+ * MikroOrmModule.forFeature registers PlaceDetailsCacheRepository/
+ * AppSettingsRepository for PlaceEnrichmentService's @InjectRepository
+ * constructor (Plan 3c Task 1).
*/
@Module({
- imports: [MapsModule, PlacePhotosModule, RateLimitModule],
+ imports: [MapsModule, PlacePhotosModule, RateLimitModule, MikroOrmModule.forFeature([PlaceDetailsCache, AppSettings])],
controllers: [PlaceEnrichmentController],
providers: [PlaceEnrichmentService],
})
diff --git a/server/src/nest/place-enrichment/place-enrichment.service.ts b/server/src/nest/place-enrichment/place-enrichment.service.ts
index 7172e2aa2..84c5bc0d7 100644
--- a/server/src/nest/place-enrichment/place-enrichment.service.ts
+++ b/server/src/nest/place-enrichment/place-enrichment.service.ts
@@ -1,5 +1,6 @@
import { createHash } from 'node:crypto';
import { Injectable } from '@nestjs/common';
+import { InjectRepository } from '@mikro-orm/nestjs';
import { placeWebsiteSchema } from '@trek/shared';
import type {
MapsPlaceEnrichmentRequest,
@@ -11,7 +12,10 @@ import type {
PlaceRating,
} from '@trek/shared';
import { safeFetchFollow } from '../../utils/ssrfGuard';
-import { DatabaseService } from '../database/database.service';
+import { PlaceDetailsCache } from '../../db/entities/PlaceDetailsCache.entity';
+import type { PlaceDetailsCacheRepository } from '../../db/repositories/PlaceDetailsCache.repository';
+import { AppSettings } from '../../db/entities/AppSettings.entity';
+import type { AppSettingsRepository } from '../../db/repositories/AppSettings.repository';
import {
MapsService,
isGoogleMapsHost,
@@ -307,7 +311,8 @@ export function collectRating(details: Record | null): PlaceRat
@Injectable()
export class PlaceEnrichmentService {
constructor(
- private readonly database: DatabaseService,
+ @InjectRepository(PlaceDetailsCache) private readonly cache: PlaceDetailsCacheRepository,
+ @InjectRepository(AppSettings) private readonly appSettings: AppSettingsRepository,
private readonly maps: MapsService,
private readonly photoCache: PlacePhotoCacheService,
) {}
@@ -319,8 +324,8 @@ export class PlaceEnrichmentService {
* working, and there is nothing here that warrants a migration.
*/
async enrichDisabled(): Promise {
- const row = this.database.get<{ value: string }>('SELECT value FROM app_settings WHERE key = ?', 'places_enrich_enabled');
- return row?.value === 'false';
+ const value = await this.appSettings.getValue('places_enrich_enabled');
+ return value === 'false';
}
async enrich(userId: number, req: MapsPlaceEnrichmentRequest): Promise {
@@ -807,12 +812,7 @@ export class PlaceEnrichmentService {
private async readCache(placeId: string, lang: string | undefined): Promise {
try {
- const row = this.database.get<{ payload_json: string; fetched_at: number }>(
- 'SELECT payload_json, fetched_at FROM place_details_cache WHERE place_id = ? AND lang = ? AND expanded = ?',
- placeId,
- lang ?? '',
- CACHE_KIND,
- );
+ const row = await this.cache.findEntry(placeId, lang ?? '', CACHE_KIND);
if (!row) return null;
const parsed = JSON.parse(row.payload_json) as CachePayload;
if (parsed.v !== CACHE_VERSION) return null;
@@ -844,14 +844,13 @@ export class PlaceEnrichmentService {
private async writeCache(placeId: string, lang: string | undefined, value: CachedEnrichment): Promise {
try {
- this.database.run(
- 'INSERT OR REPLACE INTO place_details_cache (place_id, lang, expanded, payload_json, fetched_at) VALUES (?, ?, ?, ?, ?)',
- placeId,
- lang ?? '',
- CACHE_KIND,
- JSON.stringify({ ...value, v: CACHE_VERSION } satisfies CachePayload),
- Date.now(),
- );
+ await this.cache.upsertEntry({
+ place_id: placeId,
+ lang: lang ?? '',
+ expanded: CACHE_KIND,
+ payload_json: JSON.stringify({ ...value, v: CACHE_VERSION } satisfies CachePayload),
+ fetched_at: Date.now(),
+ });
} catch (err) {
console.error('Failed to cache place enrichment:', err);
}
diff --git a/server/src/nest/place-photos/place-photo-cache.service.ts b/server/src/nest/place-photos/place-photo-cache.service.ts
index e3f725c23..6f19919f8 100644
--- a/server/src/nest/place-photos/place-photo-cache.service.ts
+++ b/server/src/nest/place-photos/place-photo-cache.service.ts
@@ -1,9 +1,14 @@
import { Injectable } from '@nestjs/common';
+import { InjectRepository } from '@mikro-orm/nestjs';
import { Jimp, JimpMime } from 'jimp';
import crypto from 'node:crypto';
import { Readable } from 'node:stream';
import { DatabaseService } from '../database/database.service';
import { StorageService } from '../storage/storage.service';
+import { GooglePlacePhotoMeta } from '../../db/entities/GooglePlacePhotoMeta.entity';
+import type { GooglePlacePhotoMetaRepository } from '../../db/repositories/GooglePlacePhotoMeta.repository';
+import { Places } from '../../db/entities/Places.entity';
+import type { PlacesRepository } from '../../db/repositories/Places.repository';
// How long a "no photo for this place" answer stays remembered. Nothing about it
// changes until a photo appears upstream, so it is worth keeping: without it every
@@ -46,6 +51,11 @@ interface CachedPhoto {
* The registry's mode-aware prefix reproduces both TREK_PLACE_PHOTO_DIR
* layouts (unset: uploads/photos/google; set: that dir, bare keys), so the
* cache itself is mode-agnostic.
+ *
+ * `DatabaseService` stays injected (Plan 3c Task 1, PP6 ruling option 2)
+ * purely for the `collection_places` half of `isReferenced` — that table is
+ * Plan 3h's, so it stays a raw statement with a `// Plan 3h` comment until
+ * then. Every other statement in this file is repository-backed.
*/
@Injectable()
export class PlacePhotoCacheService {
@@ -59,6 +69,8 @@ export class PlacePhotoCacheService {
constructor(
private readonly db: DatabaseService,
private readonly storage: StorageService,
+ @InjectRepository(GooglePlacePhotoMeta) private readonly meta: GooglePlacePhotoMetaRepository,
+ @InjectRepository(Places) private readonly places: PlacesRepository,
) {}
private fileName(placeId: string): string {
@@ -73,10 +85,7 @@ export class PlacePhotoCacheService {
}
async get(placeId: string): Promise {
- const row = this.db.get<{ attribution: string | null }>(
- 'SELECT attribution FROM google_place_photo_meta WHERE place_id = ? AND error_at IS NULL',
- placeId,
- );
+ const row = await this.meta.findLive(placeId);
if (!row) return null;
@@ -84,7 +93,7 @@ export class PlacePhotoCacheService {
// First time this placeId is checked this session — verify the object exists.
// (Guards against volume wipes or manual deletion between server restarts.)
if (!(await this.storage.exists('photos-google', this.fileName(placeId)))) {
- this.db.run('DELETE FROM google_place_photo_meta WHERE place_id = ?', placeId);
+ await this.meta.deleteByPlaceId(placeId);
return null;
}
this.knownOnDisk.add(placeId);
@@ -100,10 +109,7 @@ export class PlacePhotoCacheService {
this.recentFailures.delete(placeId);
}
- const row = this.db.get<{ error_at: number }>(
- 'SELECT error_at FROM google_place_photo_meta WHERE place_id = ? AND error_at IS NOT NULL',
- placeId,
- );
+ const row = await this.meta.findErrored(placeId);
if (!row) return false;
return Date.now() - row.error_at < MISSING_TTL;
@@ -128,10 +134,7 @@ export class PlacePhotoCacheService {
this.recentFailures.delete(placeId);
this.knownOnDisk.delete(placeId);
- this.db.run(
- 'INSERT OR REPLACE INTO google_place_photo_meta (place_id, attribution, fetched_at, error_at) VALUES (?, NULL, ?, ?)',
- placeId, Date.now(), Date.now(),
- );
+ await this.meta.upsertError(placeId, Date.now());
}
/**
@@ -158,10 +161,7 @@ export class PlacePhotoCacheService {
this.knownOnDisk.add(placeId);
this.recentFailures.delete(placeId);
- this.db.run(
- 'INSERT OR REPLACE INTO google_place_photo_meta (place_id, attribution, fetched_at, error_at) VALUES (?, ?, ?, NULL)',
- placeId, attribution, Date.now(),
- );
+ await this.meta.upsertPhoto(placeId, attribution, Date.now());
return { photoUrl: this.proxyUrl(placeId), attribution };
}
@@ -200,14 +200,20 @@ export class PlacePhotoCacheService {
* the google_place_id itself, so collection_places must count as a referencing
* table — otherwise the nightly sweep + trip-place delete would evict a photo
* still shown on a collection thumbnail (#1081 photo-cache pitfall).
+ *
+ * Plan 3c Task 1 PP6 ruling (option 2): the legacy single `UNION ALL … LIMIT 1`
+ * statement spans `places` (this plan's) and `collection_places` (Plan 3h's),
+ * so it is split into two existence checks, evaluated in the SAME order the
+ * legacy `UNION ALL … LIMIT 1` would short-circuit in — the `collection_places`
+ * half only runs when the `places` half comes back false.
*/
private async isReferenced(placeId: string): Promise {
+ const proxyUrl = this.proxyUrl(placeId);
+ if (await this.places.existsByGoogleIdOrImageUrl(placeId, proxyUrl)) return true;
+ // Plan 3h: collection_places is not yet repository-backed.
const row = this.db.get(
- `SELECT 1 FROM places WHERE google_place_id = ? OR image_url = ?
- UNION ALL
- SELECT 1 FROM collection_places WHERE google_place_id = ? OR image_url = ?
- LIMIT 1`,
- placeId, this.proxyUrl(placeId), placeId, this.proxyUrl(placeId),
+ `SELECT 1 FROM collection_places WHERE google_place_id = ? OR image_url = ? LIMIT 1`,
+ placeId, proxyUrl,
);
return !!row;
}
@@ -216,7 +222,7 @@ export class PlacePhotoCacheService {
await this.storage.delete('photos-google', this.fileName(placeId)).catch(() => {
/* already gone */
});
- this.db.run('DELETE FROM google_place_photo_meta WHERE place_id = ?', placeId);
+ await this.meta.deleteByPlaceId(placeId);
this.knownOnDisk.delete(placeId);
}
@@ -236,9 +242,9 @@ export class PlacePhotoCacheService {
async sweepOrphans(): Promise {
let removed = 0;
- const rows = this.db.all<{ place_id: string }>('SELECT place_id FROM google_place_photo_meta');
+ const placeIds = await this.meta.listPlaceIds();
const keepFiles = new Set();
- for (const { place_id } of rows) {
+ for (const place_id of placeIds) {
if (await this.isReferenced(place_id)) {
keepFiles.add(this.fileName(place_id));
} else {
diff --git a/server/src/nest/place-photos/place-photos.module.ts b/server/src/nest/place-photos/place-photos.module.ts
index b5a289621..98786769e 100644
--- a/server/src/nest/place-photos/place-photos.module.ts
+++ b/server/src/nest/place-photos/place-photos.module.ts
@@ -1,18 +1,25 @@
import { Module } from '@nestjs/common';
+import { MikroOrmModule } from '@mikro-orm/nestjs';
import { PlacePhotoCacheService } from './place-photo-cache.service';
import { PlacePhotoCacheJob } from './place-photo-cache.job';
import { AppConfigModule } from '../app-config/app-config.module';
import { SchedulingModule } from '../scheduling/scheduling.module';
import { StorageModule } from '../storage/storage.module';
+import { GooglePlacePhotoMeta } from '../../db/entities/GooglePlacePhotoMeta.entity';
+import { Places } from '../../db/entities/Places.entity';
/** The marker-photo cache. No controller of its own — maps serves the bytes,
* places and share read through it, and PlacePhotoCacheJob sweeps it nightly.
*
* Deliberately NOT @Global (permissions precedent), and AppConfigModule is
* imported explicitly because @Global only reaches modules that are in the
- * graph — which a single-domain e2e TestingModule is not. */
+ * graph — which a single-domain e2e TestingModule is not.
+ *
+ * MikroOrmModule.forFeature registers GooglePlacePhotoMetaRepository/
+ * PlacesRepository for PlacePhotoCacheService's @InjectRepository
+ * constructor (Plan 3c Task 1). */
@Module({
- imports: [AppConfigModule, SchedulingModule, StorageModule],
+ imports: [AppConfigModule, SchedulingModule, StorageModule, MikroOrmModule.forFeature([GooglePlacePhotoMeta, Places])],
providers: [PlacePhotoCacheService, PlacePhotoCacheJob],
exports: [PlacePhotoCacheService],
})
diff --git a/server/src/nest/place-shadow/place-shadow.module.ts b/server/src/nest/place-shadow/place-shadow.module.ts
index 6ca2887d5..cd46e4a3b 100644
--- a/server/src/nest/place-shadow/place-shadow.module.ts
+++ b/server/src/nest/place-shadow/place-shadow.module.ts
@@ -1,15 +1,22 @@
import { Module } from '@nestjs/common';
+import { MikroOrmModule } from '@mikro-orm/nestjs';
import { SchedulingModule } from '../scheduling/scheduling.module';
import { PlaceShadowController } from './place-shadow.controller';
import { PlaceShadowRetentionJob } from './place-shadow.job';
import { PlaceShadowService } from './place-shadow.service';
+import { PlaceShadowPicks } from '../../db/entities/PlaceShadowPicks.entity';
+import { AppSettings } from '../../db/entities/AppSettings.entity';
/**
* Place shadow log. Registered in AppModule; exports the service so the admin
* surface can read the summary without going through HTTP.
+ *
+ * MikroOrmModule.forFeature registers PlaceShadowPicksRepository/
+ * AppSettingsRepository for PlaceShadowService's @InjectRepository
+ * constructor (Plan 3c Task 1).
*/
@Module({
- imports: [SchedulingModule],
+ imports: [SchedulingModule, MikroOrmModule.forFeature([PlaceShadowPicks, AppSettings])],
controllers: [PlaceShadowController],
providers: [PlaceShadowService, PlaceShadowRetentionJob],
exports: [PlaceShadowService],
diff --git a/server/src/nest/place-shadow/place-shadow.service.ts b/server/src/nest/place-shadow/place-shadow.service.ts
index b33e58b2a..6db863bcb 100644
--- a/server/src/nest/place-shadow/place-shadow.service.ts
+++ b/server/src/nest/place-shadow/place-shadow.service.ts
@@ -1,11 +1,15 @@
import { Injectable } from '@nestjs/common';
+import { InjectRepository } from '@mikro-orm/nestjs';
import type {
PlaceShadowExportResult,
PlaceShadowPickRequest,
PlaceShadowRow,
PlaceShadowSummaryResult,
} from '@trek/shared';
-import { DatabaseService } from '../database/database.service';
+import { PlaceShadowPicks } from '../../db/entities/PlaceShadowPicks.entity';
+import type { PlaceShadowPicksRepository, PlaceShadowPickRow } from '../../db/repositories/PlaceShadowPicks.repository';
+import { AppSettings } from '../../db/entities/AppSettings.entity';
+import type { AppSettingsRepository } from '../../db/repositories/AppSettings.repository';
/** Rows older than this are removed nightly. */
export const RETENTION_DAYS = 180;
@@ -25,23 +29,7 @@ function round(value: number, decimals = COORD_DECIMALS): number {
return Math.round(value * f) / f;
}
-interface DbRow {
- id: number;
- created_at: string;
- query: string;
- lang: string | null;
- bias_lat: number | null;
- bias_lng: number | null;
- source: string;
- live_rank: number;
- live_count: number;
- picked_name: string;
- picked_lat: number;
- picked_lng: number;
- picked_place_id: string | null;
-}
-
-function toRow(r: DbRow): PlaceShadowRow {
+function toRow(r: PlaceShadowPickRow): PlaceShadowRow {
return {
id: r.id,
createdAt: r.created_at,
@@ -72,13 +60,14 @@ function toRow(r: DbRow): PlaceShadowRow {
*/
@Injectable()
export class PlaceShadowService {
- constructor(private readonly db: DatabaseService) {}
+ constructor(
+ @InjectRepository(PlaceShadowPicks) private readonly picks: PlaceShadowPicksRepository,
+ @InjectRepository(AppSettings) private readonly appSettings: AppSettingsRepository,
+ ) {}
async enabled(): Promise {
- const row = this.db.get<{ value: string }>(
- "SELECT value FROM app_settings WHERE key = 'place_shadow_enabled'",
- );
- return row?.value === 'true';
+ const value = await this.appSettings.getValue('place_shadow_enabled');
+ return value === 'true';
}
/**
@@ -94,23 +83,19 @@ export class PlaceShadowService {
// corpus exists to produce, so the row is dropped instead.
if (pick.liveRank >= pick.liveCount) return false;
- this.db.run(
- `INSERT INTO place_shadow_picks
- (query, lang, bias_lat, bias_lng, source, live_rank, live_count,
- picked_name, picked_lat, picked_lng, picked_place_id)
- VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
- pick.query,
- pick.lang ?? null,
- pick.biasLat === undefined ? null : round(pick.biasLat),
- pick.biasLng === undefined ? null : round(pick.biasLng),
- pick.source,
- pick.liveRank,
- pick.liveCount,
- pick.pickedName,
- round(pick.pickedLat),
- round(pick.pickedLng),
- pick.pickedPlaceId ?? null,
- );
+ await this.picks.insertPick({
+ query: pick.query,
+ lang: pick.lang ?? null,
+ bias_lat: pick.biasLat === undefined ? null : round(pick.biasLat),
+ bias_lng: pick.biasLng === undefined ? null : round(pick.biasLng),
+ source: pick.source,
+ live_rank: pick.liveRank,
+ live_count: pick.liveCount,
+ picked_name: pick.pickedName,
+ picked_lat: round(pick.pickedLat),
+ picked_lng: round(pick.pickedLng),
+ picked_place_id: pick.pickedPlaceId ?? null,
+ });
return true;
}
@@ -121,14 +106,7 @@ export class PlaceShadowService {
*/
async export(after?: number, limit = EXPORT_PAGE_SIZE): Promise {
const size = Math.min(Math.max(1, limit), EXPORT_PAGE_SIZE);
- const rows = this.db.all(
- `SELECT * FROM place_shadow_picks
- WHERE id > ?
- ORDER BY id
- LIMIT ?`,
- after ?? 0,
- size + 1,
- );
+ const rows = await this.picks.page(after ?? 0, size);
const page = rows.slice(0, size);
return {
version: 1,
@@ -140,22 +118,15 @@ export class PlaceShadowService {
async summary(): Promise {
const enabled = await this.enabled();
- const totals = this.db.get<{ total: number; oldest: string | null; newest: string | null }>(
- 'SELECT COUNT(*) AS total, MIN(created_at) AS oldest, MAX(created_at) AS newest FROM place_shadow_picks',
- );
- const total = totals?.total ?? 0;
+ const totals = await this.picks.totals();
+ const total = totals.total;
- const bySource = this.db.all<{ source: string; count: number }>(
- `SELECT source, COUNT(*) AS count FROM place_shadow_picks
- GROUP BY source ORDER BY count DESC`,
- );
+ const bySource = await this.picks.countBySource();
// Buckets rather than a mean: the question is "did the user find it near
// the top", and an average rank is dragged around by the rare query that
// scrolled to result 40.
- const ranks = this.db.all<{ live_rank: number; count: number }>(
- 'SELECT live_rank, COUNT(*) AS count FROM place_shadow_picks GROUP BY live_rank',
- );
+ const ranks = await this.picks.countByLiveRank();
const counted = (test: (rank: number) => boolean): number =>
ranks.filter(r => test(r.live_rank)).reduce((sum, r) => sum + r.count, 0);
@@ -165,8 +136,8 @@ export class PlaceShadowService {
return {
enabled,
total,
- oldest: totals?.oldest ?? null,
- newest: totals?.newest ?? null,
+ oldest: totals.oldest,
+ newest: totals.newest,
retentionDays: RETENTION_DAYS,
bySource,
liveRankBuckets: [
@@ -182,15 +153,11 @@ export class PlaceShadowService {
/** Admin wipe. Returns how many rows went. */
async clear(): Promise {
- return this.db.run('DELETE FROM place_shadow_picks').changes;
+ return this.picks.deleteAll();
}
/** Nightly retention. Returns how many rows went. */
async purgeExpired(retentionDays = RETENTION_DAYS): Promise {
- return this.db.run(
- `DELETE FROM place_shadow_picks
- WHERE created_at < datetime('now', ?)`,
- `-${retentionDays} days`,
- ).changes;
+ return this.picks.purgeOlderThan(retentionDays);
}
}
diff --git a/server/src/nest/query-helpers/query-helpers.module.ts b/server/src/nest/query-helpers/query-helpers.module.ts
index 30910f087..5d5210544 100644
--- a/server/src/nest/query-helpers/query-helpers.module.ts
+++ b/server/src/nest/query-helpers/query-helpers.module.ts
@@ -1,11 +1,20 @@
import { Module } from '@nestjs/common';
+import { MikroOrmModule } from '@mikro-orm/nestjs';
import { QueryHelpersService } from './query-helpers.service';
+import { Tags } from '../../db/entities/Tags.entity';
+import { PlaceRatings } from '../../db/entities/PlaceRatings.entity';
+import { AssignmentParticipants } from '../../db/entities/AssignmentParticipants.entity';
/** Shared batch loaders for the list endpoints. No controller or MCP surface of
* its own — assignments, days, places and share import it for the loaders.
* Deliberately NOT @Global so e2e TestingModules resolve it transitively
- * through each consumer's explicit import, same as PermissionsModule. */
+ * through each consumer's explicit import, same as PermissionsModule.
+ *
+ * MikroOrmModule.forFeature registers TagsRepository/PlaceRatingsRepository/
+ * AssignmentParticipantsRepository for QueryHelpersService's
+ * @InjectRepository constructor (Plan 3c Task 1). */
@Module({
+ imports: [MikroOrmModule.forFeature([Tags, PlaceRatings, AssignmentParticipants])],
providers: [QueryHelpersService],
exports: [QueryHelpersService],
})
diff --git a/server/src/nest/query-helpers/query-helpers.service.ts b/server/src/nest/query-helpers/query-helpers.service.ts
index 53679286e..786af2047 100644
--- a/server/src/nest/query-helpers/query-helpers.service.ts
+++ b/server/src/nest/query-helpers/query-helpers.service.ts
@@ -1,18 +1,13 @@
import { Injectable } from '@nestjs/common';
-import { DatabaseService } from '../database/database.service';
+import { InjectRepository } from '@mikro-orm/nestjs';
+import { Tags } from '../../db/entities/Tags.entity';
+import type { TagsRepository, TagForPlaceRow } from '../../db/repositories/Tags.repository';
+import { PlaceRatings } from '../../db/entities/PlaceRatings.entity';
+import type { PlaceRatingsRepository } from '../../db/repositories/PlaceRatings.repository';
+import { AssignmentParticipants } from '../../db/entities/AssignmentParticipants.entity';
+import type { AssignmentParticipantsRepository } from '../../db/repositories/AssignmentParticipants.repository';
import type { Tag, Participant } from '../../types';
-interface TagRow extends Tag {
- place_id: number;
-}
-
-interface ParticipantRow {
- assignment_id: number;
- user_id: number;
- username: string;
- avatar: string | null;
-}
-
export interface PlaceRatingRow {
user_id: number;
username: string;
@@ -22,8 +17,11 @@ export interface PlaceRatingRow {
/**
* The batch loaders that keep the list endpoints off N+1 queries — one query per
- * collection instead of one per row. Moved 1:1 from services/queryHelpers.ts
- * (same SQL, same indexing) onto the injected connection.
+ * collection instead of one per row. Plan 3c Task 1: onto `TagsRepository`
+ * (QH1), `PlaceRatingsRepository` (QH2, a Plan 3c-owned table per the
+ * inventory §14.6) and `AssignmentParticipantsRepository` (QH3) — this
+ * service's own first repository test file, since it had none before
+ * (inventory §15c).
*
* Only the loaders live here. The two pure reshaping functions that shipped in
* the same legacy file touch no database and stayed free functions in
@@ -31,29 +29,21 @@ export interface PlaceRatingRow {
*/
@Injectable()
export class QueryHelpersService {
- constructor(private readonly db: DatabaseService) {}
+ constructor(
+ @InjectRepository(Tags) private readonly tags: TagsRepository,
+ @InjectRepository(PlaceRatings) private readonly placeRatings: PlaceRatingsRepository,
+ @InjectRepository(AssignmentParticipants) private readonly assignmentParticipants: AssignmentParticipantsRepository,
+ ) {}
/** Batch-load tags for multiple places in a single query, indexed by place ID. */
async loadTagsByPlaceIds(placeIds: number[], { compact }: { compact?: boolean } = {}): Promise[]>> {
const tagsByPlaceId: Record[]> = {};
- if (placeIds.length > 0) {
- const placeholders = placeIds.map(() => '?').join(',');
- const allTags = this.db.all(`
- SELECT t.*, pt.place_id FROM tags t
- JOIN place_tags pt ON t.id = pt.tag_id
- WHERE pt.place_id IN (${placeholders})
- `, ...placeIds);
-
- for (const tag of allTags) {
- const pid = tag.place_id;
- if (!tagsByPlaceId[pid]) tagsByPlaceId[pid] = [];
- if (compact) {
- tagsByPlaceId[pid].push({ id: tag.id, name: tag.name, color: tag.color, created_at: tag.created_at });
- } else {
- const { place_id, ...rest } = tag;
- tagsByPlaceId[pid].push(rest);
- }
- }
+ const rows = await this.tags.listForPlaces(placeIds, { compact });
+ for (const tag of rows as TagForPlaceRow[]) {
+ const pid = tag.place_id;
+ if (!tagsByPlaceId[pid]) tagsByPlaceId[pid] = [];
+ const { place_id, ...rest } = tag;
+ tagsByPlaceId[pid].push(rest);
}
return tagsByPlaceId;
}
@@ -61,17 +51,10 @@ export class QueryHelpersService {
/** Batch-load collaborative ratings (#1435) for multiple places in one query, indexed by place ID. */
async loadRatingsByPlaceIds(placeIds: number[]): Promise> {
const ratingsByPlaceId: Record = {};
- if (placeIds.length > 0) {
- const rows = this.db.all(`
- SELECT pr.place_id, pr.user_id, u.username, u.avatar, pr.rating FROM place_ratings pr
- JOIN users u ON pr.user_id = u.id
- WHERE pr.place_id IN (${placeIds.map(() => '?').join(',')})
- ORDER BY pr.created_at
- `, ...placeIds);
- for (const { place_id, ...rest } of rows) {
- if (!ratingsByPlaceId[place_id]) ratingsByPlaceId[place_id] = [];
- ratingsByPlaceId[place_id].push(rest);
- }
+ const rows = await this.placeRatings.listForPlaces(placeIds);
+ for (const { place_id, ...rest } of rows) {
+ if (!ratingsByPlaceId[place_id]) ratingsByPlaceId[place_id] = [];
+ ratingsByPlaceId[place_id].push(rest);
}
return ratingsByPlaceId;
}
@@ -79,15 +62,10 @@ export class QueryHelpersService {
/** Batch-load participants for multiple day-assignments in a single query, indexed by assignment ID. */
async loadParticipantsByAssignmentIds(assignmentIds: number[]): Promise> {
const participantsByAssignment: Record = {};
- if (assignmentIds.length > 0) {
- const allParticipants = this.db.all(
- `SELECT ap.assignment_id, ap.user_id, u.username, u.avatar FROM assignment_participants ap JOIN users u ON ap.user_id = u.id WHERE ap.assignment_id IN (${assignmentIds.map(() => '?').join(',')})`,
- ...assignmentIds,
- );
- for (const p of allParticipants) {
- if (!participantsByAssignment[p.assignment_id]) participantsByAssignment[p.assignment_id] = [];
- participantsByAssignment[p.assignment_id].push({ user_id: p.user_id, username: p.username, avatar: p.avatar });
- }
+ const rows = await this.assignmentParticipants.listForAssignments(assignmentIds);
+ for (const p of rows) {
+ if (!participantsByAssignment[p.assignment_id]) participantsByAssignment[p.assignment_id] = [];
+ participantsByAssignment[p.assignment_id].push({ user_id: p.user_id, username: p.username, avatar: p.avatar });
}
return participantsByAssignment;
}
diff --git a/server/src/nest/trip-membership/trip-membership.module.ts b/server/src/nest/trip-membership/trip-membership.module.ts
index a2b1d7045..094ddd42e 100644
--- a/server/src/nest/trip-membership/trip-membership.module.ts
+++ b/server/src/nest/trip-membership/trip-membership.module.ts
@@ -1,11 +1,21 @@
import { Module } from '@nestjs/common';
+import { MikroOrmModule } from '@mikro-orm/nestjs';
import { TripMembershipService } from './trip-membership.service';
+import { Trips } from '../../db/entities/Trips.entity';
+import { TripMembers } from '../../db/entities/TripMembers.entity';
/** Trip membership joins. No controller of its own — the HTTP surface lives with
* auth, oidc and trip-invite, which all import this for the join itself. Kept
* separate so those three (plus the plugin host) share one implementation
- * without AuthModule and TripInviteModule importing each other. */
+ * without AuthModule and TripInviteModule importing each other.
+ *
+ * MikroOrmModule.forFeature([Trips, TripMembers]) only — NO service import
+ * (Plan 3c Task 1 ruling): five modules import THIS module, so a new service
+ * import here would close a cycle. `EntityManager`/repository resolution
+ * comes entirely from the ORM's own `@Global()` core module + this
+ * `forFeature` registration, keeping this module a leaf. */
@Module({
+ imports: [MikroOrmModule.forFeature([Trips, TripMembers])],
providers: [TripMembershipService],
exports: [TripMembershipService],
})
diff --git a/server/src/nest/trip-membership/trip-membership.service.ts b/server/src/nest/trip-membership/trip-membership.service.ts
index 2ba1ad1b3..c18200b46 100644
--- a/server/src/nest/trip-membership/trip-membership.service.ts
+++ b/server/src/nest/trip-membership/trip-membership.service.ts
@@ -1,5 +1,9 @@
import { Injectable } from '@nestjs/common';
-import { DatabaseService } from '../database/database.service';
+import { InjectRepository } from '@mikro-orm/nestjs';
+import { Trips } from '../../db/entities/Trips.entity';
+import type { TripsRepository } from '../../db/repositories/Trips.repository';
+import { TripMembers } from '../../db/entities/TripMembers.entity';
+import type { TripMembersRepository } from '../../db/repositories/TripMembers.repository';
/**
* Adding an existing user to a trip as a member, plus the leaf membership
@@ -11,24 +15,26 @@ import { DatabaseService } from '../database/database.service';
* this trip" / "which trips can this user see", but every service that owns
* the hydrated answer (TripsService, TripMembersService, TripReadModelService)
* lives in a module that imports the budget domain, so injecting one there
- * closes a real cycle. This module imports nothing, so it is the one place
- * those id-level reads can live — the fold that deleted trips.bridge.
+ * closes a real cycle. This module imports nothing but `TripsRepository`/
+ * `TripMembersRepository` (Plan 3c Task 1: `MikroOrmModule.forFeature`, no
+ * service import — see `trip-membership.module.ts`), so it stays the one
+ * place those id-level reads can live — the fold that deleted trips.bridge.
*/
@Injectable()
export class TripMembershipService {
- constructor(private readonly db: DatabaseService) {}
+ constructor(
+ @InjectRepository(Trips) private readonly trips: TripsRepository,
+ @InjectRepository(TripMembers) private readonly tripMembers: TripMembersRepository,
+ ) {}
/** The trip owner's user id, or null when the trip does not exist. */
async getOwnerId(tripId: string | number): Promise {
- const row = this.db.get<{ user_id: number }>('SELECT user_id FROM trips WHERE id = ?', tripId);
- return row ? row.user_id : null;
+ return this.trips.getOwnerId(tripId);
}
/** Member user ids (owner excluded), in added_at order like listMembers. */
async listMemberUserIds(tripId: string | number): Promise {
- return this.db
- .all<{ user_id: number }>('SELECT user_id FROM trip_members WHERE trip_id = ? ORDER BY added_at ASC', tripId)
- .map((r) => r.user_id);
+ return this.tripMembers.listUserIdsByTrip(tripId);
}
/**
@@ -36,15 +42,7 @@ export class TripMembershipService {
* half of TripsService.list(userId, null), same WHERE and ORDER BY.
*/
async listAccessibleTripIds(userId: number): Promise {
- return this.db
- .prepare(`
- SELECT t.id FROM trips t
- LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = :userId
- WHERE (t.user_id = :userId OR m.user_id IS NOT NULL)
- ORDER BY t.created_at DESC
- `)
- .all({ userId })
- .map((r) => (r as { id: number }).id);
+ return this.trips.listAccessibleIds(userId);
}
/**
@@ -57,19 +55,23 @@ export class TripMembershipService {
* belongs to a real (non-guest) account.
*
* Returns whether a new membership row was actually created.
+ *
+ * Non-transactional check-then-act, unchanged (Plan 3c inventory §18.4,
+ * program rule 11): two concurrent joins can both pass the "already a
+ * member" check and both insert. Pinned by a concurrency test, not fixed.
*/
async joinTripAsMember(
tripId: number,
userId: number,
invitedBy: number | null,
): Promise<{ joined: boolean; tripId: number }> {
- const trip = this.db.get<{ id: number; user_id: number }>('SELECT id, user_id FROM trips WHERE id = ?', tripId);
+ const trip = await this.trips.findIdAndOwner(tripId);
if (!trip) return { joined: false, tripId };
// The owner already has full access; never add them as a member.
if (trip.user_id === userId) return { joined: false, tripId };
- const existing = this.db.get('SELECT id FROM trip_members WHERE trip_id = ? AND user_id = ?', tripId, userId);
+ const existing = await this.tripMembers.exists(tripId, userId);
if (existing) return { joined: false, tripId };
- this.db.run('INSERT INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, ?)', tripId, userId, invitedBy);
+ await this.tripMembers.addMember(tripId, userId, invitedBy);
return { joined: true, tripId };
}
}
diff --git a/server/tests/e2e/budget.e2e.test.ts b/server/tests/e2e/budget.e2e.test.ts
index f3c16e782..3a3b569cc 100644
--- a/server/tests/e2e/budget.e2e.test.ts
+++ b/server/tests/e2e/budget.e2e.test.ts
@@ -90,7 +90,11 @@ describe('Budget e2e (real auth guard + temp SQLite, real budget SQL)', () => {
});
beforeEach(() => {
- canAccessTrip.mockReturnValue({ id: tripId, user_id: 1, currency: 'EUR' });
+ // 0b review L2 / security review F-B7: `canAccessTrip` is dead mock
+ // scaffolding — `TripAccessGuard` reads `TripsRepository.findAccessible`
+ // directly now (Plan 3c Task 0b), so `db/database`'s `canAccessTrip`
+ // property is never imported by production code; this line used to be a
+ // no-op that read as if it still controlled access.
checkPermission.mockReturnValue(true);
});
diff --git a/server/tests/e2e/files.e2e.test.ts b/server/tests/e2e/files.e2e.test.ts
index 32013c326..3438756d9 100644
--- a/server/tests/e2e/files.e2e.test.ts
+++ b/server/tests/e2e/files.e2e.test.ts
@@ -111,7 +111,8 @@ describe('Files + photos e2e (real auth guard + temp SQLite)', () => {
});
beforeEach(() => {
- canAccessTrip.mockReturnValue({ id: 5, user_id: 1 });
+ // 0b review L2 / security review F-B7: dead mock scaffolding — see
+ // budget.e2e.test.ts's identical comment.
checkPermission.mockReturnValue(true);
helperSvc.canAccessTrekPhoto.mockReturnValue(true);
});
diff --git a/server/tests/e2e/places.e2e.test.ts b/server/tests/e2e/places.e2e.test.ts
index e6d0bbc93..04a938413 100644
--- a/server/tests/e2e/places.e2e.test.ts
+++ b/server/tests/e2e/places.e2e.test.ts
@@ -31,7 +31,11 @@ const { db } = vi.hoisted(() => {
route_geometry TEXT, route_color TEXT, stop_type TEXT, fill_percent INTEGER,
created_at DATETIME DEFAULT CURRENT_TIMESTAMP, updated_at DATETIME DEFAULT CURRENT_TIMESTAMP);`);
tmp.exec(`CREATE TABLE categories (id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT, color TEXT, icon TEXT);`);
- tmp.exec(`CREATE TABLE tags (id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT, color TEXT,
+ // Plan 3c Task 1: TagsRepository.listForPlaces (QH1) selects `user_id`
+ // explicitly (the legacy statement's `t.*` tolerated a narrower fixture
+ // schema; a named column list does not) — added here, matching the real
+ // `tags` table (`Migration20200101000000_baseline_schema.ts`).
+ tmp.exec(`CREATE TABLE tags (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER NOT NULL, name TEXT, color TEXT,
created_at DATETIME DEFAULT CURRENT_TIMESTAMP);`);
tmp.exec(`CREATE TABLE place_tags (place_id INTEGER NOT NULL, tag_id INTEGER NOT NULL,
PRIMARY KEY (place_id, tag_id));`);
@@ -137,7 +141,6 @@ describe('Places e2e (real auth guard + temp SQLite)', () => {
// tests below re-seed their own data-bearing row with `INSERT OR
// REPLACE` over this default.
db.prepare("INSERT INTO trips (id, title, user_id) VALUES (5, 'Trip', 1)").run();
- canAccessTrip.mockReturnValue({ id: 5, user_id: 1 });
checkPermission.mockReturnValue(true);
});
diff --git a/server/tests/e2e/trip-invite.e2e.test.ts b/server/tests/e2e/trip-invite.e2e.test.ts
index 13e0d54b2..752da5521 100644
--- a/server/tests/e2e/trip-invite.e2e.test.ts
+++ b/server/tests/e2e/trip-invite.e2e.test.ts
@@ -103,7 +103,8 @@ describe('Trip invite-link e2e (real auth guard + temp SQLite)', () => {
beforeEach(() => {
db.prepare('DELETE FROM trip_invite_tokens').run();
db.prepare('DELETE FROM trips').run();
- canAccessTrip.mockReturnValue({ user_id: 1 });
+ // 0b review L2 / security review F-B7: dead mock scaffolding — see
+ // budget.e2e.test.ts's identical comment.
checkPermission.mockReturnValue(true);
joinTripAsMember.mockReset();
});
diff --git a/server/tests/e2e/trips.e2e.test.ts b/server/tests/e2e/trips.e2e.test.ts
index 448946594..3c65094a1 100644
--- a/server/tests/e2e/trips.e2e.test.ts
+++ b/server/tests/e2e/trips.e2e.test.ts
@@ -75,12 +75,23 @@ const { db } = vi.hoisted(() => {
needs_review INTEGER DEFAULT 0, day_plan_position REAL, external_source TEXT, sync_enabled INTEGER,
ingest_state TEXT, created_at DATETIME DEFAULT CURRENT_TIMESTAMP);`);
tmp.exec('CREATE TABLE days (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL, day_number INTEGER, date TEXT);');
+ // Task 2 review M1: DaysRepository.listByTrip (a plain `em.find(Days)`)
+ // needs this table to exist — Days.entity.ts declares an inverse 1:1 to
+ // it (`roadtrip_day_tracks: () => p.oneToOne(RoadtripDayTracks).ref()
+ // .mappedBy('day')`), so MikroORM's find() resolves the relation's
+ // presence against the real table even though the property is hidden.
+ // Columns match the real migration
+ // (`Migration20200101032900_which_imported_track_a_day_s_drive.ts`).
+ tmp.exec(`CREATE TABLE roadtrip_day_tracks (day_id INTEGER PRIMARY KEY, place_id INTEGER NOT NULL,
+ stray_km REAL, created_at TEXT DEFAULT CURRENT_TIMESTAMP);`);
tmp.exec(`CREATE TABLE places (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL, name TEXT,
image_url TEXT, address TEXT, lat REAL, lng REAL, category_id INTEGER, description TEXT,
created_at DATETIME DEFAULT CURRENT_TIMESTAMP);`);
// PlacesService.list joins categories and batch-loads tags/ratings.
tmp.exec('CREATE TABLE categories (id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT, color TEXT, icon TEXT);');
- tmp.exec('CREATE TABLE tags (id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT, color TEXT, created_at DATETIME);');
+ // Plan 3c Task 1: TagsRepository.listForPlaces (QH1) selects `user_id`
+ // explicitly — see places.e2e.test.ts's identical comment.
+ tmp.exec('CREATE TABLE tags (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER NOT NULL, name TEXT, color TEXT, created_at DATETIME);');
tmp.exec('CREATE TABLE place_tags (place_id INTEGER NOT NULL, tag_id INTEGER NOT NULL, PRIMARY KEY (place_id, tag_id));');
tmp.exec('CREATE TABLE place_ratings (place_id INTEGER NOT NULL, user_id INTEGER NOT NULL, rating INTEGER, created_at DATETIME, UNIQUE(place_id, user_id));');
tmp.exec(`CREATE TABLE day_accommodations (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL,
@@ -162,7 +173,8 @@ describe('Trips e2e (real auth guard + temp SQLite)', () => {
db.prepare('DELETE FROM trip_members').run();
db.prepare('DELETE FROM days').run();
db.prepare('DELETE FROM audit_log').run();
- canAccessTrip.mockReturnValue({ user_id: 1 });
+ // 0b review L2 / security review F-B7: dead mock scaffolding — see
+ // budget.e2e.test.ts's identical comment.
checkPermission.mockReturnValue(true);
});
diff --git a/server/tests/helpers/test-uow.ts b/server/tests/helpers/test-uow.ts
index 40197f985..903b4a82c 100644
--- a/server/tests/helpers/test-uow.ts
+++ b/server/tests/helpers/test-uow.ts
@@ -37,6 +37,16 @@ import { DayAssignments } from '../../src/db/entities/DayAssignments.entity';
import type { DayAssignmentsRepository } from '../../src/db/repositories/DayAssignments.repository';
import { DayNotes } from '../../src/db/entities/DayNotes.entity';
import type { DayNotesRepository } from '../../src/db/repositories/DayNotes.repository';
+import { PlaceRatings } from '../../src/db/entities/PlaceRatings.entity';
+import type { PlaceRatingsRepository } from '../../src/db/repositories/PlaceRatings.repository';
+import { AssignmentParticipants } from '../../src/db/entities/AssignmentParticipants.entity';
+import type { AssignmentParticipantsRepository } from '../../src/db/repositories/AssignmentParticipants.repository';
+import { PlaceShadowPicks } from '../../src/db/entities/PlaceShadowPicks.entity';
+import type { PlaceShadowPicksRepository } from '../../src/db/repositories/PlaceShadowPicks.repository';
+import { GooglePlacePhotoMeta } from '../../src/db/entities/GooglePlacePhotoMeta.entity';
+import type { GooglePlacePhotoMetaRepository } from '../../src/db/repositories/GooglePlacePhotoMeta.repository';
+import { PlaceDetailsCache } from '../../src/db/entities/PlaceDetailsCache.entity';
+import type { PlaceDetailsCacheRepository } from '../../src/db/repositories/PlaceDetailsCache.repository';
const perHandle = new WeakMap>();
const appSettingsPerHandle = new WeakMap>();
@@ -386,3 +396,66 @@ export function createTestDayNotesRepo(db: Database.Database): Promise>();
+const assignmentParticipantsRepoPerHandle = new WeakMap>();
+const placeShadowPicksRepoPerHandle = new WeakMap>();
+const googlePlacePhotoMetaRepoPerHandle = new WeakMap>();
+const placeDetailsCacheRepoPerHandle = new WeakMap>();
+
+/** The `PlaceRatingsRepository` a hand-constructed `QueryHelpersService` needs (QH2). */
+export function createTestPlaceRatingsRepo(db: Database.Database): Promise {
+ const existing = placeRatingsRepoPerHandle.get(db);
+ if (existing !== undefined) return existing;
+ const pending = sharedTestOrm(db).then((t) => t.repo(PlaceRatings));
+ placeRatingsRepoPerHandle.set(db, pending);
+ return pending;
+}
+
+/** The `AssignmentParticipantsRepository` a hand-constructed `QueryHelpersService` needs (QH3). */
+export function createTestAssignmentParticipantsRepo(db: Database.Database): Promise {
+ const existing = assignmentParticipantsRepoPerHandle.get(db);
+ if (existing !== undefined) return existing;
+ const pending = sharedTestOrm(db).then((t) => t.repo(AssignmentParticipants));
+ assignmentParticipantsRepoPerHandle.set(db, pending);
+ return pending;
+}
+
+/** The `PlaceShadowPicksRepository` a hand-constructed `PlaceShadowService` needs. */
+export function createTestPlaceShadowPicksRepo(db: Database.Database): Promise {
+ const existing = placeShadowPicksRepoPerHandle.get(db);
+ if (existing !== undefined) return existing;
+ const pending = sharedTestOrm(db).then((t) => t.repo(PlaceShadowPicks));
+ placeShadowPicksRepoPerHandle.set(db, pending);
+ return pending;
+}
+
+/** The `GooglePlacePhotoMetaRepository` a hand-constructed `PlacePhotoCacheService` needs. */
+export function createTestGooglePlacePhotoMetaRepo(db: Database.Database): Promise {
+ const existing = googlePlacePhotoMetaRepoPerHandle.get(db);
+ if (existing !== undefined) return existing;
+ const pending = sharedTestOrm(db).then((t) => t.repo(GooglePlacePhotoMeta));
+ googlePlacePhotoMetaRepoPerHandle.set(db, pending);
+ return pending;
+}
+
+/** The `PlaceDetailsCacheRepository` a hand-constructed `PlaceEnrichmentService` needs. */
+export function createTestPlaceDetailsCacheRepo(db: Database.Database): Promise {
+ const existing = placeDetailsCacheRepoPerHandle.get(db);
+ if (existing !== undefined) return existing;
+ const pending = sharedTestOrm(db).then((t) => t.repo(PlaceDetailsCache));
+ placeDetailsCacheRepoPerHandle.set(db, pending);
+ return pending;
+}
diff --git a/server/tests/integration/dayReorder.test.ts b/server/tests/integration/dayReorder.test.ts
index fbaa3adb0..5b5e44f53 100644
--- a/server/tests/integration/dayReorder.test.ts
+++ b/server/tests/integration/dayReorder.test.ts
@@ -28,6 +28,7 @@ import { QueryHelpersService } from '../../src/nest/query-helpers/query-helpers.
import {
createTestUnitOfWork, createTestAppSettingsRepo,
createTestDaysRepo, createTestDayAssignmentsRepo, createTestDayNotesRepo, createTestTripsRepo,
+ createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo,
} from '../helpers/test-uow';
let svc: DaysService;
@@ -36,7 +37,7 @@ beforeAll(async () => {
new DatabaseService(testDb),
new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)),
new RealtimeService(),
- new QueryHelpersService(new DatabaseService(testDb)),
+ new QueryHelpersService(await createTestTagsRepo(testDb), await createTestPlaceRatingsRepo(testDb), await createTestAssignmentParticipantsRepo(testDb)),
await createTestUnitOfWork(testDb),
await createTestDaysRepo(testDb),
await createTestDayAssignmentsRepo(testDb),
diff --git a/server/tests/integration/demo-seed-request-context.test.ts b/server/tests/integration/demo-seed-request-context.test.ts
index d38254fde..0841871fd 100644
--- a/server/tests/integration/demo-seed-request-context.test.ts
+++ b/server/tests/integration/demo-seed-request-context.test.ts
@@ -16,14 +16,16 @@
* conversion never meets a second C1 (task-6-review-parity.md's boot-sweep
* finding).
*
- * Deliberately does NOT `vi.mock('../../src/db/database', ...)` the way
- * most integration suites do: `buildDbMock`'s `runDemoSeed` is a
- * deliberate no-op stub (tests/helpers/db-mock.ts) so that suite's app
- * boots don't carry demo-seed side effects — which would silence the very
- * thing this file exists to exercise. `NODE_ENV=test` already gives
- * `db/database.ts`'s own singleton an isolated, per-worker `:memory:` copy
- * of the migrated schema snapshot (`readSchemaSnapshot()`), so no mock is
- * needed here at all.
+ * Does NOT use `buildDbMock` the way most integration suites do — its
+ * `runDemoSeed` is a deliberate no-op stub (tests/helpers/db-mock.ts) so
+ * that suite's app boots don't carry demo-seed side effects, which would
+ * silence the very thing this file exists to exercise. `NODE_ENV=test`
+ * already gives `db/database.ts`'s own singleton an isolated, per-worker
+ * `:memory:` copy of the migrated schema snapshot (`readSchemaSnapshot()`),
+ * so nothing needs stubbing for the boot itself — the `vi.mock` below (Task
+ * 0b review M2) exists only to wrap `runDemoSeed` with a context-recording
+ * spy, not to replace its behaviour: `importOriginal` re-exports everything
+ * else from the real module unchanged.
*
* `seedDemoData`'s own `require('../demo/demo-seed')` (a deliberate dynamic
* require — see that file's docstring) cannot resolve under vitest's SWC
@@ -41,6 +43,43 @@
*/
import { describe, it, expect, afterEach, vi } from 'vitest';
import type { INestApplication } from '@nestjs/common';
+import { RequestContext } from '@mikro-orm/core';
+
+/**
+ * Task 0b review M2: the pre-fix version of this file only asserted "no
+ * `cannotUseGlobalContext`/`global EntityManager` line reached the log",
+ * which cannot distinguish a wrapped call from an unwrapped one — `runDemoSeed`
+ * is raw `better-sqlite3` and never touches the ORM, so it produces neither
+ * line either way (verified: deleting the `withRequestContext` wrap in
+ * `db/orm.ts:59` left this ratchet green, 1/1). Fixed by asserting the
+ * context POSITIVELY: a thin spy around the real `runDemoSeed`, preserving
+ * everything else `../../src/db/database` exports (`importOriginal`),
+ * records whether `RequestContext.currentRequestContext()` is defined at the
+ * instant `runDemoSeed` is entered — this is exactly the ratchet MikroORM's
+ * own `allowGlobalContext: false` gate reads at query time, so it is the
+ * real signal, not a log-line proxy for it. Deleting the `withRequestContext`
+ * wrap now fails this test directly (verified).
+ */
+// `vi.hoisted`, not a plain top-level `const`: `vi.mock`'s factory is
+// invoked when something up the import chain first resolves
+// `../../src/db/database` (during `buildApp`'s own module graph, not at
+// this file's textual position), and vitest hoists the `vi.mock` call
+// itself above every import — a plain `const` here would still be in its
+// temporal dead zone when the factory closes over it. Same hazard the
+// legacy `trip-membership.service.test.ts` guarded against with its own
+// `vi.hoisted(() => ({ testDb, dbMock }))`.
+const { contextSeenOnEntry } = vi.hoisted(() => ({ contextSeenOnEntry: [] as boolean[] }));
+vi.mock('../../src/db/database', async (importOriginal) => {
+ const actual = await importOriginal();
+ return {
+ ...actual,
+ runDemoSeed: (): void => {
+ contextSeenOnEntry.push(RequestContext.currentRequestContext() !== undefined);
+ return actual.runDemoSeed();
+ },
+ };
+});
+
import { buildApp } from '../../src/bootstrap';
describe('runDemoSeed runs inside a request context (Plan 3c Task 0b)', () => {
@@ -50,6 +89,7 @@ describe('runDemoSeed runs inside a request context (Plan 3c Task 0b)', () => {
afterEach(async () => {
await app?.close();
app = undefined;
+ contextSeenOnEntry.length = 0;
if (prevDemo === undefined) delete process.env.DEMO_MODE;
else process.env.DEMO_MODE = prevDemo;
});
@@ -69,6 +109,12 @@ describe('runDemoSeed runs inside a request context (Plan 3c Task 0b)', () => {
// The one thing this task could regress: a missing request context.
const suspicious = demoLines.filter((line) => /cannotUseGlobalContext|global EntityManager/i.test(line));
expect(suspicious).toEqual([]);
+
+ // DEMO-SEED-002 (0b review M2): the positive ratchet. runDemoSeed was
+ // actually entered (the spy fired) AND every entry saw a defined
+ // request context.
+ expect(contextSeenOnEntry.length).toBeGreaterThan(0);
+ expect(contextSeenOnEntry.every(Boolean)).toBe(true);
} finally {
errSpy.mockRestore();
}
diff --git a/server/tests/integration/orm-request-context-seams.test.ts b/server/tests/integration/orm-request-context-seams.test.ts
index cde387130..8a2e306d6 100644
--- a/server/tests/integration/orm-request-context-seams.test.ts
+++ b/server/tests/integration/orm-request-context-seams.test.ts
@@ -147,7 +147,17 @@ describe('ORM request-context seams populated in production', () => {
* per-job pattern for a register-only job.
*/
it('SEAM-003: PlaceShadowRetentionJob registers against the real CronRegistrarService, and firing its tick runs inside the same request context with no missing-context error', async () => {
+ const orm = app.get(MikroORM);
const registrar = app.get(CronRegistrarService);
+ // 0b security review F-B4: a structural identity proof, matching
+ // SEAM-001/002's shape, not just the behavioural one below — a future
+ // refactor that quietly drops CronRegistrarService's ORM constructor arg
+ // (but leaves SOME orm-shaped stub wired) would still pass the
+ // behavioural half if that stub happened to work; this catches it
+ // directly, the same way SEAM-001 already does for the OTHER three
+ // choke points.
+ expect((registrar as unknown as { orm?: MikroORM }).orm).toBe(orm);
+
const isEnabledSpy = vi.spyOn(registrar, 'isEnabled').mockReturnValue(true);
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
try {
@@ -156,15 +166,39 @@ describe('ORM request-context seams populated in production', () => {
VALUES (datetime('now', '-999 days'), 'seam-003', 'nominatim', 1, 1, 'expired pick', 0, 0)
`).run();
- app.get(PlaceShadowRetentionJob).onApplicationBootstrap();
+ // Real bug found converting `purgeExpired` to a genuinely async
+ // repository call (0b security review F-B4): the `cron` package's
+ // `CronJob.fireOnTick()` only awaits its callback when the job is
+ // constructed with `waitForCompletion: true`
+ // (`node_modules/cron/dist/job.js`'s `fireOnTick`) — `cron-registrar
+ // .service.ts`'s `CronJob.from({...})` never sets that option, so
+ // `await job.fireOnTick()` resolves as soon as the SYNCHRONOUS part of
+ // `wrappedTick` returns, before the awaited `withRequestContext(orm,
+ // () => onTick())` promise (and therefore the repository delete
+ // inside it) has actually settled. Verified directly: without the
+ // spy below, `remaining.n` read `1`, not `0` — a race, not a context
+ // failure (the previous, all-synchronous `better-sqlite3` DELETE never
+ // exposed this gap). Spying on the job's own `tick()` and awaiting the
+ // promise IT returns is the fix — the same promise `wrappedTick`
+ // itself awaits, just observed from outside.
+ const jobInstance = app.get(PlaceShadowRetentionJob);
+ const tickSpy = vi.spyOn(jobInstance, 'tick');
+ jobInstance.onApplicationBootstrap();
const job = app.get(SchedulerRegistry).getCronJob('place-shadow-retention');
await job.fireOnTick();
+ await tickSpy.mock.results[0]?.value;
const suspicious = errSpy.mock.calls
.map((args) => args.map(String).join(' '))
.filter((line) => /cannotUseGlobalContext|global EntityManager/i.test(line));
expect(suspicious).toEqual([]);
+ // Plan 3c Task 1: `purgeExpired` is now `PlaceShadowPicksRepository`-backed
+ // (a real ORM call, not raw `better-sqlite3`), so this outcome is now a
+ // load-bearing mutation ratchet on its own — removing the
+ // `withRequestContext` wrap in `cron-registrar.service.ts`'s `register()`
+ // makes the repository call throw inside `PlaceShadowRetentionJob.tick`'s
+ // own try/catch, and the row survives (verified by hand, reverted).
const remaining = testDb.prepare("SELECT COUNT(*) as n FROM place_shadow_picks WHERE query = 'seam-003'").get() as { n: number };
expect(remaining.n).toBe(0);
} finally {
diff --git a/server/tests/integration/share.test.ts b/server/tests/integration/share.test.ts
index 84c146998..01532c9b5 100644
--- a/server/tests/integration/share.test.ts
+++ b/server/tests/integration/share.test.ts
@@ -34,6 +34,9 @@ import { LocalDriver } from '../../src/nest/storage/drivers/local.driver';
import { StorageService } from '../../src/nest/storage/storage.service';
import type { StorageRegistryService, ResolvedCategory } from '../../src/nest/storage/storage-registry.service';
import { DEFAULT_UPLOADS_ROOT, GLOBAL_TEMP_DIR } from '../../src/nest/storage/storage-paths';
+import { createTestOrm } from '../helpers/test-orm';
+import { GooglePlacePhotoMeta } from '../../src/db/entities/GooglePlacePhotoMeta.entity';
+import { Places } from '../../src/db/entities/Places.entity';
// A real instance over the same connection the app uses — these cases write a
// cache entry and then read it back through the HTTP route, so the stub
@@ -46,7 +49,12 @@ const testStorage = new StorageService({
tempDir: () => GLOBAL_TEMP_DIR,
replicaFailures: () => [],
} as unknown as StorageRegistryService);
-const placePhotoCache = new PlacePhotoCacheService(new DatabaseService(sharedDb), testStorage);
+// Plan 3c Task 1: PlacePhotoCacheService now needs two repositories — a
+// second, throwaway MikroORM bound to the SAME `sharedDb` connection
+// (`allowGlobalContext: true`, the same shape `place-photo-cache.service
+// .test.ts` uses), since this helper's two call sites below run outside any
+// HTTP request the app's own `withRequestContext` would wrap.
+let placePhotoCache: PlacePhotoCacheService;
import fs from 'node:fs';
import path from 'node:path';
import crypto from 'node:crypto';
@@ -57,6 +65,8 @@ let app: Application;
beforeAll(async () => {
nestApp = await buildApp();
app = nestApp.getHttpAdapter().getInstance();
+ const t = await createTestOrm(sharedDb, { allowGlobalContext: true });
+ placePhotoCache = new PlacePhotoCacheService(new DatabaseService(sharedDb), testStorage, t.repo(GooglePlacePhotoMeta), t.repo(Places));
});
beforeEach(() => {
diff --git a/server/tests/unit/db/dialect/sql-functions.test.ts b/server/tests/unit/db/dialect/sql-functions.test.ts
index 2aeaebfd4..ccfcd0c8f 100644
--- a/server/tests/unit/db/dialect/sql-functions.test.ts
+++ b/server/tests/unit/db/dialect/sql-functions.test.ts
@@ -11,11 +11,15 @@ import {
coalesceParam,
columnIncrementedBy,
columnRef,
+ countAll,
currentTimestamp,
dateAdd,
dateOf,
lower,
lowerParam,
+ lowerTrim,
+ maxOf,
+ minOf,
nowMinusDays,
trim,
} from '../../../../src/db/dialect/sql-functions';
@@ -326,4 +330,102 @@ describe('sql-functions (sqlite)', () => {
expect(() => coalesceParam(foreign, 'u.a', 'x')).toThrow(/no implementation for platform FakePlatform/);
expect(() => absDifference(foreign, 'u.lat', 1)).toThrow(/no implementation for platform FakePlatform/);
});
+
+ // Plan 3c Task 1 (§17a: COUNT(*)/MIN/MAX aggregates — `PlaceShadowPicksRepository
+ // .totals`/`countBySource`/`countByLiveRank`). ESLint bans `raw()` inside
+ // src/db/repositories/**, so these three aliased-aggregate helpers exist
+ // for repositories that need a COUNT/MIN/MAX in a SELECT projection.
+
+ it('SQLF-023: countAll/minOf/maxOf render one aliased aggregate row, matching a hand-written statement', async () => {
+ const { user: a } = createUser(testDb);
+ testDb.prepare("UPDATE users SET created_at = '2026-01-01 00:00:00' WHERE id = ?").run(a.id);
+ const { user: b } = createUser(testDb);
+ testDb.prepare("UPDATE users SET created_at = '2026-06-15 00:00:00' WHERE id = ?").run(b.id);
+ const platform = t.em.getPlatform();
+
+ const row = await t.em.createQueryBuilder(Users, 'u')
+ .select([countAll(platform, 'total'), minOf(platform, 'u.created_at', 'oldest'), maxOf(platform, 'u.created_at', 'newest')])
+ .where({ id: { $in: [a.id, b.id] } })
+ .execute('get', false);
+ expect(row).toEqual({ total: 2, oldest: '2026-01-01 00:00:00', newest: '2026-06-15 00:00:00' });
+
+ const expected = testDb
+ .prepare('SELECT COUNT(*) as total, MIN(created_at) as oldest, MAX(created_at) as newest FROM users WHERE id IN (?, ?)')
+ .get(a.id, b.id);
+ expect(row).toEqual(expected);
+ });
+
+ // No `.orderBy({ count: 'desc' })` here: MikroORM's QueryBuilder cannot
+ // order by an ad-hoc raw-fragment alias (verified directly — it throws
+ // "Trying to query by not existing property" at runtime, since `raw()`'s
+ // alias carries no literal type for `ExtractRawAliases` to register). The
+ // repository callers that need "ORDER BY DESC"
+ // (`PlaceShadowPicksRepository.countBySource`) sort the fetched rows in JS
+ // instead — `Array.prototype.sort` is stable (ES2019+), so the resulting
+ // order matches a real `ORDER BY count DESC` byte for byte.
+ it('SQLF-024: countAll composes with GROUP BY, matching a hand-written GROUP BY statement', async () => {
+ createUser(testDb, { role: 'admin' });
+ createUser(testDb, { role: 'admin' });
+ createUser(testDb, { role: 'user' });
+ const platform = t.em.getPlatform();
+
+ const rows = await t.em.createQueryBuilder(Users, 'u')
+ .select(['u.role', countAll(platform, 'count')])
+ .groupBy('u.role')
+ .execute('all', false);
+
+ const expected = testDb.prepare('SELECT role, COUNT(*) as count FROM users GROUP BY role').all();
+ expect([...(rows as { role: string; count: number }[])].sort((a, b) => a.role.localeCompare(b.role)))
+ .toEqual([...(expected as { role: string; count: number }[])].sort((a, b) => a.role.localeCompare(b.role)));
+ });
+
+ it('SQLF-025: countAll/minOf/maxOf reject an alias that is not a plain identifier', () => {
+ const platform = t.em.getPlatform();
+ expect(() => countAll(platform, 'x; DROP TABLE users; --')).toThrow(/not an alias/);
+ expect(() => minOf(platform, 'u.created_at', '1bad')).toThrow(/not an alias/);
+ expect(() => maxOf(platform, 'u.created_at', '')).toThrow(/not an alias/);
+ });
+
+ it('SQLF-026: an unknown platform fails closed for countAll/minOf/maxOf', () => {
+ class FakePlatform extends Platform {}
+ const foreign = new FakePlatform();
+ expect(() => countAll(foreign, 'total')).toThrow(/no implementation for platform FakePlatform/);
+ expect(() => minOf(foreign, 'u.created_at', 'oldest')).toThrow(/no implementation for platform FakePlatform/);
+ expect(() => maxOf(foreign, 'u.created_at', 'newest')).toThrow(/no implementation for platform FakePlatform/);
+ });
+
+ // Task 0b review M3 (blocks Task 4's PL26, places.service.ts:642
+ // `lower(trim(name))`): `lower()`/`trim()` cannot compose to build
+ // `LOWER(TRIM(name))` — `lowerTrim()` is the one fragment for that shape.
+ it('SQLF-027: lowerTrim renders LOWER(TRIM()) as one fragment, matching a hand-written statement', async () => {
+ const { user } = createUser(testDb, { username: ' Padded Name ' });
+ const platform = t.em.getPlatform();
+ const row = await t.em.createQueryBuilder(Users, 'u')
+ .select([lowerTrim(platform, 'u.username').as('n')])
+ .where({ id: user.id })
+ .execute('get', false);
+ expect((row as { n: string }).n).toBe('padded name');
+
+ const expected = testDb.prepare('SELECT LOWER(TRIM(username)) as n FROM users WHERE id = ?').get(user.id) as { n: string };
+ expect((row as { n: string }).n).toBe(expected.n);
+ });
+
+ it('SQLF-028: lowerTrim is usable as a find() filter key, and SQLite LOWER() stays ASCII-only through the TRIM', async () => {
+ const { user } = createUser(testDb, { username: ' JOSÉ ' });
+ const platform = t.em.getPlatform();
+ const rows = await t.em.find(Users, { [lowerTrim(platform, 'username')]: 'josé'.toLowerCase() });
+ // toLowerCase() is full-Unicode ('é' stays 'é'), but SQLite's LOWER() is
+ // ASCII-only and leaves 'É' untouched — so the JS-lowered bind does NOT
+ // match the SQL-lowered, SQL-trimmed column (program rule 18: mixing
+ // engines is the bug, not the fix).
+ expect(rows).toEqual([]);
+ const matching = await t.em.find(Users, { [lowerTrim(platform, 'username')]: 'josÉ' });
+ expect(matching.map((r) => r.id)).toEqual([user.id]);
+ });
+
+ it('SQLF-029: an unknown platform fails closed for lowerTrim', () => {
+ class FakePlatform extends Platform {}
+ const foreign = new FakePlatform();
+ expect(() => lowerTrim(foreign, 'u.name')).toThrow(/no implementation for platform FakePlatform/);
+ });
});
diff --git a/server/tests/unit/db/repositories/GooglePlacePhotoMeta.repository.test.ts b/server/tests/unit/db/repositories/GooglePlacePhotoMeta.repository.test.ts
new file mode 100644
index 000000000..86ece0895
--- /dev/null
+++ b/server/tests/unit/db/repositories/GooglePlacePhotoMeta.repository.test.ts
@@ -0,0 +1,132 @@
+/**
+ * GooglePlacePhotoMetaRepository (Plan 3c Task 1, PP1–PP5/PP7/PP8): every
+ * statement `PlacePhotoCacheService` used to issue raw, on real rows.
+ * `place_id` is a TEXT key — pseudo-ids like `coords:lat:lng` are legal
+ * values, exercised below alongside real Google place ids.
+ */
+import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
+import { createSnapshotTestDb } from '../../../helpers/db-mock';
+import { resetTestDb } from '../../../helpers/test-db';
+import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
+import { GooglePlacePhotoMeta } from '../../../../src/db/entities/GooglePlacePhotoMeta.entity';
+import type { GooglePlacePhotoMetaRepository } from '../../../../src/db/repositories/GooglePlacePhotoMeta.repository';
+
+const testDb = createSnapshotTestDb();
+let t: TestOrm;
+let meta: GooglePlacePhotoMetaRepository;
+
+beforeAll(async () => {
+ t = await createTestOrm(testDb);
+ meta = t.repo(GooglePlacePhotoMeta);
+});
+beforeEach(() => { resetTestDb(testDb); t.clear(); });
+afterAll(async () => { await t.close(); testDb.close(); });
+
+function rawRow(placeId: string): unknown {
+ return testDb.prepare('SELECT * FROM google_place_photo_meta WHERE place_id = ?').get(placeId);
+}
+
+describe('GooglePlacePhotoMetaRepository.findLive / findErrored', () => {
+ it('GPPMREPO-001: findLive returns the attribution only when error_at is NULL', async () => {
+ testDb.prepare('INSERT INTO google_place_photo_meta (place_id, attribution, fetched_at, error_at) VALUES (?, ?, ?, NULL)').run('p1', 'Some Author', 1000);
+ expect(await meta.findLive('p1')).toEqual({ attribution: 'Some Author' });
+ });
+
+ it('GPPMREPO-002: findLive returns null when error_at is set, even with an attribution present', async () => {
+ testDb.prepare('INSERT INTO google_place_photo_meta (place_id, attribution, fetched_at, error_at) VALUES (?, NULL, ?, ?)').run('p1', 1000, 2000);
+ expect(await meta.findLive('p1')).toBeNull();
+ });
+
+ it('GPPMREPO-003: findLive returns null for a missing row', async () => {
+ expect(await meta.findLive('missing')).toBeNull();
+ });
+
+ it('GPPMREPO-004: works with coords:lat:lng pseudo-ids (TEXT key, colons and dots included)', async () => {
+ const pseudoId = 'coords:48.8566:2.3522';
+ testDb.prepare('INSERT INTO google_place_photo_meta (place_id, attribution, fetched_at, error_at) VALUES (?, NULL, ?, NULL)').run(pseudoId, 1000);
+ expect(await meta.findLive(pseudoId)).toEqual({ attribution: null });
+ });
+
+ it('GPPMREPO-005: findErrored returns {error_at} only when error_at IS NOT NULL', async () => {
+ testDb.prepare('INSERT INTO google_place_photo_meta (place_id, attribution, fetched_at, error_at) VALUES (?, NULL, ?, ?)').run('p1', 1000, 5000);
+ expect(await meta.findErrored('p1')).toEqual({ error_at: 5000 });
+ });
+
+ it('GPPMREPO-006: findErrored returns null when error_at is NULL', async () => {
+ testDb.prepare('INSERT INTO google_place_photo_meta (place_id, attribution, fetched_at, error_at) VALUES (?, ?, ?, NULL)').run('p1', 'A', 1000);
+ expect(await meta.findErrored('p1')).toBeNull();
+ });
+});
+
+describe('GooglePlacePhotoMetaRepository.deleteByPlaceId', () => {
+ it('GPPMREPO-007: deletes the row for the given place_id', async () => {
+ testDb.prepare('INSERT INTO google_place_photo_meta (place_id, attribution, fetched_at, error_at) VALUES (?, NULL, ?, NULL)').run('p1', 1000);
+ await meta.deleteByPlaceId('p1');
+ expect(rawRow('p1')).toBeUndefined();
+ });
+
+ it('GPPMREPO-008: deleting a missing place_id does not throw', async () => {
+ await expect(meta.deleteByPlaceId('never-existed')).resolves.toBeUndefined();
+ });
+});
+
+describe('GooglePlacePhotoMetaRepository.upsertError / upsertPhoto — INSERT OR REPLACE parity', () => {
+ it('GPPMREPO-009: upsertError writes NULL attribution and matching fetched_at/error_at on a fresh row', async () => {
+ await meta.upsertError('p1', 12345);
+ expect(rawRow('p1')).toMatchObject({ place_id: 'p1', attribution: null, fetched_at: 12345, error_at: 12345 });
+ });
+
+ it('GPPMREPO-010: upsertPhoto writes the attribution with a NULL error_at on a fresh row', async () => {
+ await meta.upsertPhoto('p1', 'Some Author · CC BY 2.0', 999);
+ expect(rawRow('p1')).toMatchObject({ place_id: 'p1', attribution: 'Some Author · CC BY 2.0', fetched_at: 999, error_at: null });
+ });
+
+ // PP4 forces attribution NULL; PP5 forces error_at NULL — same PK, mirror
+ // images. Writes PE3-style twice then a third time, asserting the exact
+ // row after each (Task 1 brief's explicit test requirement).
+ it('GPPMREPO-011: put then markError then put again — each call overwrites the row exactly (INSERT OR REPLACE semantics via em.upsert merge)', async () => {
+ await meta.upsertPhoto('p1', 'Author A', 100);
+ expect(rawRow('p1')).toMatchObject({ attribution: 'Author A', fetched_at: 100, error_at: null });
+
+ await meta.upsertError('p1', 200);
+ expect(rawRow('p1')).toMatchObject({ attribution: null, fetched_at: 200, error_at: 200 });
+
+ await meta.upsertPhoto('p1', 'Author B', 300);
+ expect(rawRow('p1')).toMatchObject({ attribution: 'Author B', fetched_at: 300, error_at: null });
+
+ // Only one row exists for this PK throughout — no duplicate insert.
+ expect(testDb.prepare('SELECT COUNT(*) as c FROM google_place_photo_meta WHERE place_id = ?').get('p1')).toEqual({ c: 1 });
+ });
+});
+
+describe('GooglePlacePhotoMetaRepository.listPlaceIds', () => {
+ it('GPPMREPO-012: lists every place_id, full table scan', async () => {
+ testDb.prepare('INSERT INTO google_place_photo_meta (place_id, fetched_at) VALUES (?, ?)').run('p1', 1);
+ testDb.prepare('INSERT INTO google_place_photo_meta (place_id, fetched_at) VALUES (?, ?)').run('p2', 2);
+ expect((await meta.listPlaceIds()).sort()).toEqual(['p1', 'p2']);
+ });
+
+ it('GPPMREPO-013: an empty table yields an empty array', async () => {
+ expect(await meta.listPlaceIds()).toEqual([]);
+ });
+});
+
+// D-shape (Task 1 brief item): findLive/findErrored are `findOne`-based
+// (`TrekRepository`'s `disableIdentityMap: true` default), the class of read
+// program rule 14 targets directly.
+describe('GooglePlacePhotoMetaRepository — D-shape', () => {
+ it('GPPMREPO-014: a raw write after findLive is visible in the next findLive call, in one query (disableIdentityMap regression)', async () => {
+ testDb.prepare('INSERT INTO google_place_photo_meta (place_id, attribution, fetched_at, error_at) VALUES (?, ?, ?, NULL)').run('p1', 'Old', 1000);
+ expect(await meta.findLive('p1')).toEqual({ attribution: 'Old' }); // populate the identity map
+ testDb.prepare('UPDATE google_place_photo_meta SET attribution = ? WHERE place_id = ?').run('New', 'p1');
+
+ const connection = t.orm.em.getConnection();
+ const spy = vi.spyOn(connection, 'execute');
+ try {
+ expect(await meta.findLive('p1')).toEqual({ attribution: 'New' });
+ expect(spy.mock.calls.length).toBe(1);
+ } finally {
+ spy.mockRestore();
+ }
+ });
+});
diff --git a/server/tests/unit/db/repositories/PlaceDetailsCache.repository.test.ts b/server/tests/unit/db/repositories/PlaceDetailsCache.repository.test.ts
new file mode 100644
index 000000000..5776e0d6c
--- /dev/null
+++ b/server/tests/unit/db/repositories/PlaceDetailsCache.repository.test.ts
@@ -0,0 +1,104 @@
+/**
+ * PlaceDetailsCacheRepository (Plan 3c Task 1, PE2/PE3): the two statements
+ * `PlaceEnrichmentService` used to issue raw, on real rows. The table's real
+ * PRIMARY KEY is the composite `(place_id, lang, expanded)` — the parity
+ * proof below writes the same triple twice and asserts exactly one row.
+ */
+import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
+import { createSnapshotTestDb } from '../../../helpers/db-mock';
+import { resetTestDb } from '../../../helpers/test-db';
+import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
+import { PlaceDetailsCache } from '../../../../src/db/entities/PlaceDetailsCache.entity';
+import type { PlaceDetailsCacheRepository } from '../../../../src/db/repositories/PlaceDetailsCache.repository';
+
+const testDb = createSnapshotTestDb();
+let t: TestOrm;
+let cache: PlaceDetailsCacheRepository;
+
+beforeAll(async () => {
+ t = await createTestOrm(testDb);
+ cache = t.repo(PlaceDetailsCache);
+});
+beforeEach(() => { resetTestDb(testDb); t.clear(); });
+afterAll(async () => { await t.close(); testDb.close(); });
+
+function rawRow(placeId: string, lang: string, expanded: number): unknown {
+ return testDb.prepare('SELECT * FROM place_details_cache WHERE place_id = ? AND lang = ? AND expanded = ?').get(placeId, lang, expanded);
+}
+
+describe('PlaceDetailsCacheRepository.findEntry', () => {
+ it('PDCREPO-001: finds the entry for the exact (place_id, lang, expanded) triple', async () => {
+ testDb.prepare('INSERT INTO place_details_cache (place_id, lang, expanded, payload_json, fetched_at) VALUES (?, ?, ?, ?, ?)')
+ .run('p1', 'en', 0, '{"a":1}', 1000);
+ expect(await cache.findEntry('p1', 'en', 0)).toEqual({ payload_json: '{"a":1}', fetched_at: 1000 });
+ });
+
+ it('PDCREPO-002: a different lang or expanded value is a miss, not a fuzzy match', async () => {
+ testDb.prepare('INSERT INTO place_details_cache (place_id, lang, expanded, payload_json, fetched_at) VALUES (?, ?, ?, ?, ?)')
+ .run('p1', 'en', 0, '{"a":1}', 1000);
+ expect(await cache.findEntry('p1', 'fr', 0)).toBeNull();
+ expect(await cache.findEntry('p1', 'en', 2)).toBeNull();
+ });
+
+ it('PDCREPO-003: an empty-string lang (the "no lang requested" default) is its own distinct row', async () => {
+ testDb.prepare('INSERT INTO place_details_cache (place_id, lang, expanded, payload_json, fetched_at) VALUES (?, ?, ?, ?, ?)')
+ .run('p1', '', 2, '{"b":2}', 2000);
+ expect(await cache.findEntry('p1', '', 2)).toEqual({ payload_json: '{"b":2}', fetched_at: 2000 });
+ expect(await cache.findEntry('p1', 'en', 2)).toBeNull();
+ });
+
+ it('PDCREPO-004: a missing entry resolves to null', async () => {
+ expect(await cache.findEntry('missing', '', 0)).toBeNull();
+ });
+});
+
+describe('PlaceDetailsCacheRepository.upsertEntry — composite-key INSERT OR REPLACE parity', () => {
+ it('PDCREPO-005: writes a fresh row for a new (place_id, lang, expanded) triple', async () => {
+ await cache.upsertEntry({ place_id: 'p1', lang: '', expanded: 2, payload_json: '{"v":1}', fetched_at: 100 });
+ expect(rawRow('p1', '', 2)).toMatchObject({ payload_json: '{"v":1}', fetched_at: 100 });
+ });
+
+ // The composite-key proof the Task 1 brief requires: writes the SAME
+ // triple twice and asserts exactly ONE row survives (proving
+ // `onConflictFields: ['place_id', 'lang', 'expanded']` is the real
+ // conflict target, not an inferred single-column PK that would silently
+ // insert a duplicate).
+ it('PDCREPO-006: writing the same (place_id, lang, expanded) triple twice leaves exactly one row, with the second payload', async () => {
+ await cache.upsertEntry({ place_id: 'p1', lang: 'en', expanded: 2, payload_json: '{"v":1}', fetched_at: 100 });
+ await cache.upsertEntry({ place_id: 'p1', lang: 'en', expanded: 2, payload_json: '{"v":2}', fetched_at: 200 });
+
+ const rows = testDb.prepare('SELECT * FROM place_details_cache WHERE place_id = ? AND lang = ? AND expanded = ?').all('p1', 'en', 2);
+ expect(rows).toHaveLength(1);
+ expect(rawRow('p1', 'en', 2)).toMatchObject({ payload_json: '{"v":2}', fetched_at: 200 });
+ });
+
+ it('PDCREPO-007: a different lang or expanded value for the same place_id is a SEPARATE row, not overwritten', async () => {
+ await cache.upsertEntry({ place_id: 'p1', lang: 'en', expanded: 0, payload_json: '{"kind":"details"}', fetched_at: 100 });
+ await cache.upsertEntry({ place_id: 'p1', lang: 'en', expanded: 2, payload_json: '{"kind":"enrichment"}', fetched_at: 200 });
+ await cache.upsertEntry({ place_id: 'p1', lang: 'fr', expanded: 2, payload_json: '{"kind":"enrichment-fr"}', fetched_at: 300 });
+
+ const rows = testDb.prepare('SELECT lang, expanded, payload_json FROM place_details_cache WHERE place_id = ? ORDER BY lang, expanded').all('p1');
+ expect(rows).toHaveLength(3);
+ });
+});
+
+// D-shape (Task 1 brief item): `findEntry` is `findOne`-based
+// (`TrekRepository`'s `disableIdentityMap: true` default).
+describe('PlaceDetailsCacheRepository — D-shape', () => {
+ it('PDCREPO-008: a raw write after findEntry is visible in the next findEntry call, in one query (disableIdentityMap regression)', async () => {
+ testDb.prepare('INSERT INTO place_details_cache (place_id, lang, expanded, payload_json, fetched_at) VALUES (?, ?, ?, ?, ?)')
+ .run('p1', 'en', 2, '{"v":"old"}', 100);
+ expect(await cache.findEntry('p1', 'en', 2)).toEqual({ payload_json: '{"v":"old"}', fetched_at: 100 }); // populate the identity map
+ testDb.prepare('UPDATE place_details_cache SET payload_json = ?, fetched_at = ? WHERE place_id = ? AND lang = ? AND expanded = ?')
+ .run('{"v":"new"}', 200, 'p1', 'en', 2);
+
+ const connection = t.orm.em.getConnection();
+ const spy = vi.spyOn(connection, 'execute');
+ try {
+ expect(await cache.findEntry('p1', 'en', 2)).toEqual({ payload_json: '{"v":"new"}', fetched_at: 200 });
+ expect(spy.mock.calls.length).toBe(1);
+ } finally {
+ spy.mockRestore();
+ }
+ });
+});
diff --git a/server/tests/unit/db/repositories/PlaceRatings.repository.test.ts b/server/tests/unit/db/repositories/PlaceRatings.repository.test.ts
new file mode 100644
index 000000000..8fd488b50
--- /dev/null
+++ b/server/tests/unit/db/repositories/PlaceRatings.repository.test.ts
@@ -0,0 +1,100 @@
+/**
+ * PlaceRatingsRepository.listForPlaces (Plan 3c Task 1, QH2): the batch
+ * ratings-by-place loader behind `QueryHelpersService.loadRatingsByPlaceIds`,
+ * moved off `query-helpers.service.ts`. `place_ratings` is a Plan 3c-owned
+ * table (inventory §14.6) with no repository test file before this one.
+ */
+import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
+import { createSnapshotTestDb } from '../../../helpers/db-mock';
+import { resetTestDb } from '../../../helpers/test-db';
+import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
+import { createPlace, createTrip, createUser } from '../../../helpers/factories';
+import { PlaceRatings } from '../../../../src/db/entities/PlaceRatings.entity';
+import type { PlaceRatingsRepository } from '../../../../src/db/repositories/PlaceRatings.repository';
+
+const testDb = createSnapshotTestDb();
+let t: TestOrm;
+let placeRatings: PlaceRatingsRepository;
+
+beforeAll(async () => {
+ t = await createTestOrm(testDb);
+ placeRatings = t.repo(PlaceRatings);
+});
+beforeEach(() => { resetTestDb(testDb); t.clear(); });
+afterAll(async () => { await t.close(); testDb.close(); });
+
+function rate(placeId: number, userId: number, rating: number, createdAt: string): void {
+ testDb.prepare('INSERT INTO place_ratings (place_id, user_id, rating, created_at) VALUES (?, ?, ?, ?)').run(placeId, userId, rating, createdAt);
+}
+
+async function withQueryCount(fn: () => Promise): Promise<{ value: T; queries: number }> {
+ const connection = t.orm.em.getConnection();
+ const spy = vi.spyOn(connection, 'execute');
+ try {
+ const value = await fn();
+ return { value, queries: spy.mock.calls.length };
+ } finally {
+ spy.mockRestore();
+ }
+}
+
+describe('PlaceRatingsRepository.listForPlaces', () => {
+ it('PLACERATINGSREPO-001: an empty placeIds array short-circuits to [] without querying', async () => {
+ const { value, queries } = await withQueryCount(() => placeRatings.listForPlaces([]));
+ expect(value).toEqual([]);
+ expect(queries).toBe(0);
+ });
+
+ it('PLACERATINGSREPO-002: joins the voter\'s username/avatar, ordered by created_at', async () => {
+ const { user: owner } = createUser(testDb);
+ const { user: voterA } = createUser(testDb, { username: 'alpha' });
+ const { user: voterB } = createUser(testDb, { username: 'beta' });
+ const trip = createTrip(testDb, owner.id);
+ const place = createPlace(testDb, trip.id);
+ rate(place.id, voterA.id, 4, '2026-01-01 00:00:00');
+ rate(place.id, voterB.id, 2, '2026-01-02 00:00:00');
+
+ const rows = await placeRatings.listForPlaces([place.id]);
+ expect(rows).toEqual([
+ { place_id: place.id, user_id: voterA.id, username: 'alpha', avatar: null, rating: 4 },
+ { place_id: place.id, user_id: voterB.id, username: 'beta', avatar: null, rating: 2 },
+ ]);
+ });
+
+ it('PLACERATINGSREPO-003: batches across several places, indexed by place_id in the caller', async () => {
+ const { user: owner } = createUser(testDb);
+ const { user: voter } = createUser(testDb);
+ const trip = createTrip(testDb, owner.id);
+ const placeA = createPlace(testDb, trip.id, { name: 'A' });
+ const placeB = createPlace(testDb, trip.id, { name: 'B' });
+ rate(placeA.id, voter.id, 5, '2026-01-01 00:00:00');
+ rate(placeB.id, voter.id, 1, '2026-01-01 00:00:00');
+
+ const rows = await placeRatings.listForPlaces([placeA.id, placeB.id]);
+ expect(rows.map((r) => r.place_id).sort()).toEqual([placeA.id, placeB.id].sort());
+ });
+
+ it('PLACERATINGSREPO-004: a place with no ratings contributes no rows', async () => {
+ const { user } = createUser(testDb);
+ const trip = createTrip(testDb, user.id);
+ const place = createPlace(testDb, trip.id);
+ expect(await placeRatings.listForPlaces([place.id])).toEqual([]);
+ });
+
+ // D-shape (Task 1 brief item): `listForPlaces` is a `qb().execute('all',
+ // false)` projection, which never hydrates an entity into the identity
+ // map by construction (unlike `find`/`findOne`), so there is no staleness
+ // hazard here to reproduce — this proves it anyway, the same honest
+ // "not required, proven regardless" shape `Tags.repository.test.ts`'s
+ // TAGREPO comments already use for a method that IS at risk.
+ it('PLACERATINGSREPO-005 (D-shape): a rating written after an unrelated identity-map read is visible in the FIRST wider projection', async () => {
+ const { user: owner } = createUser(testDb);
+ const { user: voter } = createUser(testDb, { username: 'fresh-voter' });
+ const trip = createTrip(testDb, owner.id);
+ const place = createPlace(testDb, trip.id);
+ await t.repo(PlaceRatings).find({}); // populate the identity map with an unrelated (empty) read
+ rate(place.id, voter.id, 3, '2026-01-01 00:00:00');
+ const rows = await placeRatings.listForPlaces([place.id]);
+ expect(rows).toEqual([{ place_id: place.id, user_id: voter.id, username: 'fresh-voter', avatar: null, rating: 3 }]);
+ });
+});
diff --git a/server/tests/unit/db/repositories/PlaceShadowPicks.repository.test.ts b/server/tests/unit/db/repositories/PlaceShadowPicks.repository.test.ts
new file mode 100644
index 000000000..6e293ffcf
--- /dev/null
+++ b/server/tests/unit/db/repositories/PlaceShadowPicks.repository.test.ts
@@ -0,0 +1,147 @@
+/**
+ * PlaceShadowPicksRepository (Plan 3c Task 1, PS2–PS8): every statement
+ * `PlaceShadowService` used to issue raw, on real rows. `place_shadow_picks`
+ * has no other repository test file before this one.
+ */
+import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
+import { createSnapshotTestDb } from '../../../helpers/db-mock';
+import { resetTestDb } from '../../../helpers/test-db';
+import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
+import { PlaceShadowPicks } from '../../../../src/db/entities/PlaceShadowPicks.entity';
+import type { PlaceShadowPicksRepository, NewPlaceShadowPickRow } from '../../../../src/db/repositories/PlaceShadowPicks.repository';
+
+const testDb = createSnapshotTestDb();
+let t: TestOrm;
+let picks: PlaceShadowPicksRepository;
+
+beforeAll(async () => {
+ t = await createTestOrm(testDb);
+ picks = t.repo(PlaceShadowPicks);
+});
+beforeEach(() => { resetTestDb(testDb); t.clear(); });
+afterAll(async () => { await t.close(); testDb.close(); });
+
+function row(overrides: Partial = {}): NewPlaceShadowPickRow {
+ return {
+ query: 'louvre',
+ lang: null,
+ bias_lat: null,
+ bias_lng: null,
+ source: 'nominatim',
+ live_rank: 0,
+ live_count: 5,
+ picked_name: 'Louvre Museum',
+ picked_lat: 48.8606,
+ picked_lng: 2.3376,
+ picked_place_id: null,
+ ...overrides,
+ };
+}
+
+function rawRow(id: number): unknown {
+ return testDb.prepare('SELECT * FROM place_shadow_picks WHERE id = ?').get(id);
+}
+
+describe('PlaceShadowPicksRepository.insertPick', () => {
+ it('PSPICKREPO-001: inserts exactly the given columns, nullable ones included', async () => {
+ await picks.insertPick(row());
+ const stored = testDb.prepare('SELECT * FROM place_shadow_picks').get() as { id: number };
+ expect(rawRow(stored.id)).toMatchObject({ query: 'louvre', source: 'nominatim', live_rank: 0, live_count: 5, lang: null });
+ });
+});
+
+describe('PlaceShadowPicksRepository.page', () => {
+ it('PSPICKREPO-002: pages by id, oldest first, fetching size + 1 rows for the probe', async () => {
+ await picks.insertPick(row({ query: 'a' }));
+ await picks.insertPick(row({ query: 'b' }));
+ await picks.insertPick(row({ query: 'c' }));
+
+ const page = await picks.page(0, 2);
+ expect(page).toHaveLength(3); // size + 1 probe row
+ expect(page.map((r) => r.query)).toEqual(['a', 'b', 'c']);
+ });
+
+ it('PSPICKREPO-003: after excludes everything at or below that id', async () => {
+ await picks.insertPick(row({ query: 'a' }));
+ const firstId = (testDb.prepare('SELECT id FROM place_shadow_picks').get() as { id: number }).id;
+ await picks.insertPick(row({ query: 'b' }));
+ const page = await picks.page(firstId, 5);
+ expect(page.map((r) => r.query)).toEqual(['b']);
+ });
+});
+
+describe('PlaceShadowPicksRepository.totals / countBySource / countByLiveRank', () => {
+ it('PSPICKREPO-004: totals on an empty table reports 0/null/null', async () => {
+ expect(await picks.totals()).toEqual({ total: 0, oldest: null, newest: null });
+ });
+
+ it('PSPICKREPO-005: totals reports COUNT/MIN/MAX across every row', async () => {
+ await picks.insertPick(row());
+ await picks.insertPick(row());
+ testDb.prepare("UPDATE place_shadow_picks SET created_at = '2026-01-01 00:00:00' WHERE id = (SELECT MIN(id) FROM place_shadow_picks)").run();
+ testDb.prepare("UPDATE place_shadow_picks SET created_at = '2026-06-01 00:00:00' WHERE id = (SELECT MAX(id) FROM place_shadow_picks)").run();
+ const totals = await picks.totals();
+ expect(totals).toEqual({ total: 2, oldest: '2026-01-01 00:00:00', newest: '2026-06-01 00:00:00' });
+ });
+
+ it('PSPICKREPO-006: countBySource groups and orders by count DESC', async () => {
+ await picks.insertPick(row({ source: 'nominatim' }));
+ await picks.insertPick(row({ source: 'nominatim' }));
+ await picks.insertPick(row({ source: 'google' }));
+ expect(await picks.countBySource()).toEqual([
+ { source: 'nominatim', count: 2 },
+ { source: 'google', count: 1 },
+ ]);
+ });
+
+ it('PSPICKREPO-007: countByLiveRank groups with no ORDER BY (caller buckets)', async () => {
+ await picks.insertPick(row({ live_rank: 0 }));
+ await picks.insertPick(row({ live_rank: 0 }));
+ await picks.insertPick(row({ live_rank: 3 }));
+ const rows = await picks.countByLiveRank();
+ expect(rows).toEqual(expect.arrayContaining([{ live_rank: 0, count: 2 }, { live_rank: 3, count: 1 }]));
+ expect(rows).toHaveLength(2);
+ });
+});
+
+describe('PlaceShadowPicksRepository.deleteAll / purgeOlderThan', () => {
+ it('PSPICKREPO-008: deleteAll wipes every row and returns the affected count', async () => {
+ await picks.insertPick(row());
+ await picks.insertPick(row());
+ expect(await picks.deleteAll()).toBe(2);
+ expect(await picks.totals()).toEqual({ total: 0, oldest: null, newest: null });
+ });
+
+ it('PSPICKREPO-009: purgeOlderThan removes exactly the rows older than the cutoff, proving the deleted set on a seeded table', async () => {
+ await picks.insertPick(row({ query: 'old' }));
+ await picks.insertPick(row({ query: 'new' }));
+ const [oldRow, newRow] = testDb.prepare('SELECT id, query FROM place_shadow_picks ORDER BY id ASC').all() as { id: number; query: string }[];
+ testDb.prepare("UPDATE place_shadow_picks SET created_at = datetime('now', '-200 days') WHERE id = ?").run(oldRow.id);
+ testDb.prepare("UPDATE place_shadow_picks SET created_at = datetime('now', '-1 days') WHERE id = ?").run(newRow.id);
+
+ const removed = await picks.purgeOlderThan(180);
+ expect(removed).toBe(1);
+ const survivors = testDb.prepare('SELECT query FROM place_shadow_picks').all() as { query: string }[];
+ expect(survivors.map((r) => r.query)).toEqual(['new']);
+ });
+
+ it('PSPICKREPO-010: purgeOlderThan(0) removes rows created before right now, leaving nothing newer than the boundary', async () => {
+ await picks.insertPick(row());
+ const { id } = testDb.prepare('SELECT id FROM place_shadow_picks').get() as { id: number };
+ testDb.prepare("UPDATE place_shadow_picks SET created_at = datetime('now', '-1 seconds') WHERE id = ?").run(id);
+ expect(await picks.purgeOlderThan(0)).toBe(1);
+ });
+});
+
+// D-shape (Task 1 brief item): `page`/`totals`/`countBySource`/`countByLiveRank`
+// all go through `find`/`qb().execute()`, neither of which hydrates a
+// long-lived entity here — proven anyway for `page` (the one `find()`-based
+// read), the honest "not required, proven regardless" shape.
+describe('PlaceShadowPicksRepository — D-shape', () => {
+ it('PSPICKREPO-011: a row inserted after an unrelated identity-map read is visible in the FIRST wider projection (page)', async () => {
+ await t.repo(PlaceShadowPicks).find({}); // populate the identity map with an unrelated (empty) read
+ await picks.insertPick(row({ query: 'fresh' }));
+ const page = await picks.page(0, 10);
+ expect(page.map((r) => r.query)).toEqual(['fresh']);
+ });
+});
diff --git a/server/tests/unit/db/repositories/Places.repository.test.ts b/server/tests/unit/db/repositories/Places.repository.test.ts
index 4129b9b24..5955a5a67 100644
--- a/server/tests/unit/db/repositories/Places.repository.test.ts
+++ b/server/tests/unit/db/repositories/Places.repository.test.ts
@@ -24,7 +24,69 @@ beforeAll(async () => {
beforeEach(() => { resetTestDb(testDb); t.clear(); });
afterAll(async () => { await t.close(); testDb.close(); });
+/**
+ * The legacy `getPlaceWithTags` (`db/database.ts:146-197` at base), run raw
+ * on the same rows — 0b security review F-B6: a `toMatchObject`/partial
+ * assertion cannot see a renamed key (B1's `u__id`) or a dropped one (B2's
+ * flat `category_name`/`category_color`/`category_icon`); this reproduces
+ * the legacy function's exact three-statement composition and the test below
+ * does a full `toEqual` against it, key for key.
+ */
+function legacyGetPlaceWithTags(placeId: number): unknown {
+ const place = testDb
+ .prepare('SELECT p.*, c.name as category_name, c.color as category_color, c.icon as category_icon FROM places p LEFT JOIN categories c ON p.category_id = c.id WHERE p.id = ?')
+ .get(placeId) as (Record & { category_id: number | null; category_name: string | null; category_color: string | null; category_icon: string | null }) | undefined;
+ if (!place) return null;
+ const tags = testDb.prepare('SELECT t.* FROM tags t JOIN place_tags pt ON t.id = pt.tag_id WHERE pt.place_id = ?').all(placeId);
+ const ratings = testDb
+ .prepare('SELECT pr.user_id, u.username, u.avatar, pr.rating FROM place_ratings pr JOIN users u ON pr.user_id = u.id WHERE pr.place_id = ? ORDER BY pr.created_at')
+ .all(placeId) as { user_id: number; username: string; avatar: string | null; rating: number }[];
+ return {
+ ...place,
+ category: place.category_id
+ ? { id: place.category_id, name: place.category_name, color: place.category_color, icon: place.category_icon }
+ : null,
+ tags,
+ ratings,
+ rating_avg: ratings.length > 0 ? ratings.reduce((s, r) => s + r.rating, 0) / ratings.length : null,
+ rating_count: ratings.length,
+ };
+}
+
describe('PlacesRepository.findWithTagsAndRatings — parity with the legacy getPlaceWithTags', () => {
+ it('PLACEREPO-011 (0b security review F-B6): full key-for-key parity with the legacy statement, on a place with a category, two tags and two ratings', async () => {
+ const { user: owner } = createUser(testDb);
+ const { user: voterA } = createUser(testDb, { username: 'parity_voter_a' });
+ const { user: voterB } = createUser(testDb, { username: 'parity_voter_b' });
+ const trip = createTrip(testDb, owner.id);
+ const category = createCategory(testDb, { name: 'Parks', color: '#00aa00', icon: '🌳' });
+ const place = createPlace(testDb, trip.id, { category_id: category.id, name: 'Parity Park' });
+ const tagA = createTag(testDb, owner.id, { name: 'Green' });
+ const tagB = createTag(testDb, owner.id, { name: 'Quiet' });
+ testDb.prepare('INSERT INTO place_tags (place_id, tag_id) VALUES (?, ?)').run(place.id, tagA.id);
+ testDb.prepare('INSERT INTO place_tags (place_id, tag_id) VALUES (?, ?)').run(place.id, tagB.id);
+ testDb.prepare("INSERT INTO place_ratings (place_id, user_id, rating, created_at) VALUES (?, ?, ?, datetime('now', '-2 minutes'))").run(place.id, voterA.id, 5);
+ testDb.prepare("INSERT INTO place_ratings (place_id, user_id, rating, created_at) VALUES (?, ?, ?, datetime('now', '-1 minutes'))").run(place.id, voterB.id, 3);
+
+ const result = await places.findWithTagsAndRatings(place.id);
+ const legacy = legacyGetPlaceWithTags(place.id) as Record;
+
+ // Sort both sides' tags/ratings arrays the same way before the full
+ // toEqual — the repository's join order and the legacy statement's join
+ // order are not contractually identical for the tags half (no ORDER BY
+ // in either), only the ratings half is `ORDER BY pr.created_at` on both.
+ const sortByField = >(arr: T[], field: string): T[] =>
+ [...arr].sort((a, b) => String(a[field]).localeCompare(String(b[field])));
+ const normalize = (row: Record): Record => ({
+ ...row,
+ tags: sortByField(row.tags as Record[], 'id'),
+ });
+
+ expect(normalize(result as unknown as Record)).toEqual(normalize(legacy));
+ });
+});
+
+describe('PlacesRepository.findWithTagsAndRatings — parity with the legacy getPlaceWithTags (per-case)', () => {
it('PLACEREPO-001: no category, no tags, no ratings — null category, empty arrays, null average', async () => {
const { user: owner } = createUser(testDb);
const trip = createTrip(testDb, owner.id);
@@ -77,22 +139,93 @@ describe('PlacesRepository.findWithTagsAndRatings — parity with the legacy get
testDb
.prepare("INSERT INTO place_ratings (place_id, user_id, rating, created_at) VALUES (?, ?, ?, datetime('now', '-2 minutes'))")
.run(place.id, voterA.id, 4);
+ // Task 0b review M1: 4-and-2 averages to 3, which `Math.round(3)` also
+ // equals — the brief's own named mutation (wrapping the average in
+ // `Math.round(...)`) survived undetected. 4-and-3 averages to 3.5,
+ // which `Math.round` would corrupt to 3 or 4 — mutation-proved.
testDb
.prepare("INSERT INTO place_ratings (place_id, user_id, rating, created_at) VALUES (?, ?, ?, datetime('now', '-1 minutes'))")
- .run(place.id, voterB.id, 2);
+ .run(place.id, voterB.id, 3);
const result = await places.findWithTagsAndRatings(place.id);
expect(result?.ratings).toHaveLength(2);
expect(result?.ratings.map((r) => r.username)).toEqual(['voter_a', 'voter_b']);
- expect(result?.rating_avg).toBe(3);
+ expect(result?.rating_avg).toBe(3.5);
expect(result?.rating_count).toBe(2);
});
- it('PLACEREPO-005: a missing place id resolves to null, not a throw', async () => {
+ // Task 0b review B1 (HIGH, live regression): `ratings[].user_id` was
+ // emitted as `u__id` at runtime (a `join('pr.user','u')` + `.select(['pr.user',
+ // …])` interaction — TypeScript's declared `PlaceRatingRow.user_id` never
+ // existed on the actual row). The client's `StarRating.tsx` keys off
+ // `ratings.find(r => r.user_id === currentUserId)`, so this is the direct
+ // regression proof neither PLACEREPO-004 (keys off `username`) nor any
+ // other suite caught.
+ it('PLACEREPO-007 (0b review B1): a rating row carries a real user_id key, not u__id', async () => {
+ const { user: owner } = createUser(testDb);
+ const { user: voter } = createUser(testDb, { username: 'keyed_voter' });
+ const trip = createTrip(testDb, owner.id);
+ const place = createPlace(testDb, trip.id);
+ testDb.prepare('INSERT INTO place_ratings (place_id, user_id, rating) VALUES (?, ?, ?)').run(place.id, voter.id, 5);
+
+ const result = await places.findWithTagsAndRatings(place.id);
+ expect(result?.ratings[0]).toEqual({ user_id: voter.id, username: 'keyed_voter', avatar: null, rating: 5 });
+ expect(result?.ratings[0]).not.toHaveProperty('u__id');
+ });
+
+ // Task 0b review B2 (HIGH, live regression): `category_name`/
+ // `category_color`/`category_icon` rode along on the legacy
+ // `getPlaceWithTags`'s runtime object (`{...place, category: ...}` over
+ // the raw `SELECT p.*, c.name as category_name, …` row) even though the
+ // declared `PlaceWithTags` interface never mentioned them — several live
+ // client call sites read them flat (map popups, dashboard, shared-trip
+ // page), so a repository that strips them via `const { category_name,
+ // ..., ...placeRest } = place` silently breaks those, even though every
+ // OTHER assertion in this file (which reads through `result?.category`)
+ // stays green.
+ it('PLACEREPO-008 (0b review B2): the flat category_name/category_color/category_icon columns ride along on the row, alongside the nested category object', async () => {
+ const { user: owner } = createUser(testDb);
+ const trip = createTrip(testDb, owner.id);
+ const category = createCategory(testDb, { name: 'Cafes', color: '#654321', icon: '☕' });
+ const place = createPlace(testDb, trip.id, { category_id: category.id });
+
+ const result = await places.findWithTagsAndRatings(place.id) as unknown as Record;
+ expect(result.category_name).toBe('Cafes');
+ expect(result.category_color).toBe('#654321');
+ expect(result.category_icon).toBe('☕');
+ expect(result.category).toEqual({ id: category.id, name: 'Cafes', color: '#654321', icon: '☕' });
+ });
+
+ it('PLACEREPO-009: a missing place id resolves to null, not a throw', async () => {
expect(await places.findWithTagsAndRatings(999999)).toBeNull();
});
- it('PLACEREPO-006: a non-numeric-looking id resolves to null, the same raw-bind seam TripsRepository documents', async () => {
+ it('PLACEREPO-010: a non-numeric-looking id resolves to null, the same raw-bind seam TripsRepository documents', async () => {
expect(await places.findWithTagsAndRatings('abc')).toBeNull();
});
});
+
+// Plan 3c Task 1, PP6 ruling (option 2): the `places`-owned half of
+// `PlacePhotoCacheService.isReferenced`'s legacy `UNION ALL … LIMIT 1`.
+describe('PlacesRepository.existsByGoogleIdOrImageUrl', () => {
+ it('EXISTSGOOGLEORIMAGE-001: true when a place carries the google_place_id', async () => {
+ const { user } = createUser(testDb);
+ const trip = createTrip(testDb, user.id);
+ const place = createPlace(testDb, trip.id);
+ testDb.prepare('UPDATE places SET google_place_id = ? WHERE id = ?').run('ChIJ-gpid', place.id);
+ expect(await places.existsByGoogleIdOrImageUrl('ChIJ-gpid', '/api/maps/place-photo/ChIJ-gpid/bytes')).toBe(true);
+ });
+
+ it('EXISTSGOOGLEORIMAGE-002: true when a place carries the proxy image_url instead (coords: pseudo-ids never have a google_place_id)', async () => {
+ const { user } = createUser(testDb);
+ const trip = createTrip(testDb, user.id);
+ const place = createPlace(testDb, trip.id);
+ const proxyUrl = '/api/maps/place-photo/coords%3A1%3A2/bytes';
+ testDb.prepare('UPDATE places SET image_url = ? WHERE id = ?').run(proxyUrl, place.id);
+ expect(await places.existsByGoogleIdOrImageUrl('coords:1:2', proxyUrl)).toBe(true);
+ });
+
+ it('EXISTSGOOGLEORIMAGE-003: false when nothing references either value', async () => {
+ expect(await places.existsByGoogleIdOrImageUrl('never-seen', '/api/maps/place-photo/never-seen/bytes')).toBe(false);
+ });
+});
diff --git a/server/tests/unit/db/repositories/Tags.repository.test.ts b/server/tests/unit/db/repositories/Tags.repository.test.ts
index f8f7e4e4a..78fefa7ad 100644
--- a/server/tests/unit/db/repositories/Tags.repository.test.ts
+++ b/server/tests/unit/db/repositories/Tags.repository.test.ts
@@ -2,7 +2,7 @@ import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vites
import { createSnapshotTestDb } from '../../../helpers/db-mock';
import { resetTestDb } from '../../../helpers/test-db';
import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
-import { createTag, createUser } from '../../../helpers/factories';
+import { createPlace, createTag, createTrip, createUser } from '../../../helpers/factories';
import { Tags } from '../../../../src/db/entities/Tags.entity';
import type { TagsRepository } from '../../../../src/db/repositories/Tags.repository';
@@ -254,4 +254,76 @@ describe('TagsRepository', () => {
expect(remaining[0].id).toBe(keep.id);
});
});
+
+ // Plan 3c Task 1 (QH1): the batch tag-by-place loader behind
+ // `QueryHelpersService.loadTagsByPlaceIds`, moved off `query-helpers.service.ts`.
+ describe('listForPlaces', () => {
+ function attach(tagId: number, placeId: number): void {
+ testDb.prepare('INSERT INTO place_tags (tag_id, place_id) VALUES (?, ?)').run(tagId, placeId);
+ }
+
+ it('LISTFORPLACESREPO-001: an empty placeIds array short-circuits to [] without querying', async () => {
+ const { value, queries } = await withQueryCount(() => tags.listForPlaces([]));
+ expect(value).toEqual([]);
+ expect(queries).toBe(0);
+ });
+
+ it('LISTFORPLACESREPO-002: non-compact returns every tags column except the join key, plus place_id', async () => {
+ const { user } = createUser(testDb);
+ const trip = createTrip(testDb, user.id);
+ const place = createPlace(testDb, trip.id);
+ const tag = createTag(testDb, user.id, { name: 'Beach', color: '#ff0000' });
+ attach(tag.id, place.id);
+
+ const rows = await tags.listForPlaces([place.id]);
+ expect(rows).toEqual([
+ { id: tag.id, user_id: user.id, name: 'Beach', color: '#ff0000', created_at: (rawTag(tag.id) as { created_at: string }).created_at, place_id: place.id },
+ ]);
+ });
+
+ it('LISTFORPLACESREPO-003: compact returns {id, name, color, created_at, place_id} — no user_id', async () => {
+ const { user } = createUser(testDb);
+ const trip = createTrip(testDb, user.id);
+ const place = createPlace(testDb, trip.id);
+ const tag = createTag(testDb, user.id, { name: 'Compact', color: '#00ff00' });
+ attach(tag.id, place.id);
+
+ const rows = await tags.listForPlaces([place.id], { compact: true });
+ expect(rows).toEqual([{ id: tag.id, name: 'Compact', color: '#00ff00', created_at: (rawTag(tag.id) as { created_at: string }).created_at, place_id: place.id }]);
+ expect(rows[0]).not.toHaveProperty('user_id');
+ });
+
+ it('LISTFORPLACESREPO-004: one row per (tag, place) pair — the same tag on two places yields two rows', async () => {
+ const { user } = createUser(testDb);
+ const trip = createTrip(testDb, user.id);
+ const placeA = createPlace(testDb, trip.id, { name: 'A' });
+ const placeB = createPlace(testDb, trip.id, { name: 'B' });
+ const tag = createTag(testDb, user.id);
+ attach(tag.id, placeA.id);
+ attach(tag.id, placeB.id);
+
+ const rows = await tags.listForPlaces([placeA.id, placeB.id]);
+ expect(rows.map((r) => r.place_id).sort()).toEqual([placeA.id, placeB.id].sort());
+ });
+
+ it('LISTFORPLACESREPO-005: a place with no tags contributes no rows', async () => {
+ const { user } = createUser(testDb);
+ const trip = createTrip(testDb, user.id);
+ const place = createPlace(testDb, trip.id);
+ expect(await tags.listForPlaces([place.id])).toEqual([]);
+ });
+
+ it('LISTFORPLACESREPO-006 (D-shape): a tag written via place_tags is visible in the FIRST wider (non-compact) projection, bypassing the identity map', async () => {
+ const { user } = createUser(testDb);
+ const trip = createTrip(testDb, user.id);
+ const place = createPlace(testDb, trip.id);
+ const tag = createTag(testDb, user.id, { name: 'Fresh' });
+ // Populate the identity map with a narrower read first.
+ await tags.findByIdAndUser(tag.id, user.id);
+ attach(tag.id, place.id);
+ testDb.prepare('UPDATE tags SET name = ? WHERE id = ?').run('Renamed', tag.id);
+ const rows = await tags.listForPlaces([place.id]);
+ expect(rows[0]?.name).toBe('Renamed');
+ });
+ });
});
diff --git a/server/tests/unit/db/repositories/TripMembers.repository.test.ts b/server/tests/unit/db/repositories/TripMembers.repository.test.ts
index e23e3b8ab..75c79f236 100644
--- a/server/tests/unit/db/repositories/TripMembers.repository.test.ts
+++ b/server/tests/unit/db/repositories/TripMembers.repository.test.ts
@@ -82,3 +82,55 @@ describe('TripMembersRepository.rosterUserIds — parity with the legacy UNION s
expect(await tripMembers.rosterUserIds('abc')).toEqual(new Set());
});
});
+
+// Plan 3c Task 1 (TB2/TB5/TB6): TripMembershipService.listMemberUserIds/
+// joinTripAsMember's own check-then-act sequence.
+describe('TripMembersRepository — listUserIdsByTrip / exists / addMember (Plan 3c Task 1)', () => {
+ it('TMEMREPO-007: listUserIdsByTrip excludes the owner and orders by added_at ASC', async () => {
+ const { user: owner } = createUser(testDb);
+ const { user: first } = createUser(testDb);
+ const { user: second } = createUser(testDb);
+ const trip = createTrip(testDb, owner.id);
+ addTripMember(testDb, trip.id, first.id);
+ testDb.prepare('UPDATE trip_members SET added_at = ? WHERE trip_id = ? AND user_id = ?').run('2026-01-01 00:00:00', trip.id, first.id);
+ addTripMember(testDb, trip.id, second.id);
+ testDb.prepare('UPDATE trip_members SET added_at = ? WHERE trip_id = ? AND user_id = ?').run('2026-02-01 00:00:00', trip.id, second.id);
+
+ expect(await tripMembers.listUserIdsByTrip(trip.id)).toEqual([first.id, second.id]);
+ });
+
+ it('TMEMREPO-008: listUserIdsByTrip returns an empty array for an owner-only trip', async () => {
+ const { user: owner } = createUser(testDb);
+ const trip = createTrip(testDb, owner.id);
+ expect(await tripMembers.listUserIdsByTrip(trip.id)).toEqual([]);
+ });
+
+ it('TMEMREPO-009: exists is true only for an actual (trip, user) membership row', async () => {
+ const { user: owner } = createUser(testDb);
+ const { user: member } = createUser(testDb);
+ const { user: stranger } = createUser(testDb);
+ const trip = createTrip(testDb, owner.id);
+ addTripMember(testDb, trip.id, member.id);
+
+ expect(await tripMembers.exists(trip.id, member.id)).toBe(true);
+ expect(await tripMembers.exists(trip.id, stranger.id)).toBe(false);
+ // The owner has no trip_members row unless one was explicitly added.
+ expect(await tripMembers.exists(trip.id, owner.id)).toBe(false);
+ });
+
+ it('TMEMREPO-010: addMember inserts a row with the given invited_by, nullable', async () => {
+ const { user: owner } = createUser(testDb);
+ const { user: inviter } = createUser(testDb);
+ const { user: joiner } = createUser(testDb);
+ const trip = createTrip(testDb, owner.id);
+
+ await tripMembers.addMember(trip.id, joiner.id, inviter.id);
+ const row = testDb.prepare('SELECT trip_id, user_id, invited_by FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, joiner.id);
+ expect(row).toEqual({ trip_id: trip.id, user_id: joiner.id, invited_by: inviter.id });
+
+ const { user: joiner2 } = createUser(testDb);
+ await tripMembers.addMember(trip.id, joiner2.id, null);
+ const row2 = testDb.prepare('SELECT invited_by FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, joiner2.id);
+ expect(row2).toEqual({ invited_by: null });
+ });
+});
diff --git a/server/tests/unit/db/repositories/Trips.repository.test.ts b/server/tests/unit/db/repositories/Trips.repository.test.ts
index fee635258..00198ff69 100644
--- a/server/tests/unit/db/repositories/Trips.repository.test.ts
+++ b/server/tests/unit/db/repositories/Trips.repository.test.ts
@@ -61,6 +61,13 @@ describe('TripsRepository.findAccessible — parity with canAccessTrip', () => {
addTripMember(testDb, trip.id, member.id);
expect(await trips.isOwner(trip.id, owner.id)).toBe(true);
expect(await trips.isOwner(trip.id, member.id)).toBe(false);
+
+ // 0b security review F-B6: full parity against the legacy statement run
+ // raw on the same rows, not just the boolean the method returns.
+ const legacyIsOwner = (tripId: number, userId: number): boolean =>
+ !!testDb.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId);
+ expect(await trips.isOwner(trip.id, owner.id)).toEqual(legacyIsOwner(trip.id, owner.id));
+ expect(await trips.isOwner(trip.id, member.id)).toEqual(legacyIsOwner(trip.id, member.id));
});
it('TRIPREPO-007: a missing trip id resolves to undefined, not a throw, for both findAccessible and isOwner', async () => {
@@ -81,6 +88,56 @@ describe('TripsRepository.findAccessible — parity with canAccessTrip', () => {
});
});
+// Plan 3c Task 1 (TB1/TB3/TB4): the trip-membership primitives, added onto
+// the same repository so `TripMembershipService` never touches raw SQL.
+describe('TripsRepository — getOwnerId / findIdAndOwner / listAccessibleIds (Plan 3c Task 1)', () => {
+ it('TRIPREPO-008: getOwnerId returns the owner id, null for a missing trip', async () => {
+ const { user } = createUser(testDb);
+ const trip = createTrip(testDb, user.id);
+ expect(await trips.getOwnerId(trip.id)).toBe(user.id);
+ expect(await trips.getOwnerId(999999)).toBeNull();
+ });
+
+ it('TRIPREPO-009: getOwnerId binds a string id raw, matching the `0x10`/`007`-shaped id parity seam', async () => {
+ const { user } = createUser(testDb);
+ const trip = createTrip(testDb, user.id);
+ // A numeric-looking string binds fine under SQLite's affinity comparison.
+ expect(await trips.getOwnerId(String(trip.id))).toBe(user.id);
+ // A non-numeric string never matches any row — no throw, no coercion.
+ expect(await trips.getOwnerId('not-a-number')).toBeNull();
+ });
+
+ it('TRIPREPO-010: findIdAndOwner returns {id, user_id} for a real trip, undefined for a missing one', async () => {
+ const { user } = createUser(testDb);
+ const trip = createTrip(testDb, user.id);
+ expect(await trips.findIdAndOwner(trip.id)).toEqual({ id: trip.id, user_id: user.id });
+ expect(await trips.findIdAndOwner(999999)).toBeUndefined();
+ });
+
+ it('TRIPREPO-011: listAccessibleIds returns owned + member trips, newest first, excluding a stranger\'s trips', async () => {
+ const { user: owner } = createUser(testDb);
+ const { user: member } = createUser(testDb);
+ const { user: stranger } = createUser(testDb);
+ const a = createTrip(testDb, owner.id);
+ testDb.prepare('UPDATE trips SET created_at = ? WHERE id = ?').run('2026-01-01 00:00:00', a.id);
+ const b = createTrip(testDb, owner.id);
+ testDb.prepare('UPDATE trips SET created_at = ? WHERE id = ?').run('2026-02-01 00:00:00', b.id);
+ const c = createTrip(testDb, stranger.id);
+ addTripMember(testDb, c.id, member.id);
+
+ expect(await trips.listAccessibleIds(owner.id)).toEqual([b.id, a.id]);
+ expect(await trips.listAccessibleIds(member.id)).toEqual([c.id]);
+ expect(await trips.listAccessibleIds(stranger.id)).toEqual([c.id]);
+ });
+
+ it('TRIPREPO-012: listAccessibleIds never double-counts a trip where the caller is both owner and, somehow, member-listed', async () => {
+ const { user: owner } = createUser(testDb);
+ const trip = createTrip(testDb, owner.id);
+ addTripMember(testDb, trip.id, owner.id);
+ expect(await trips.listAccessibleIds(owner.id)).toEqual([trip.id]);
+ });
+});
+
// Plan 3c Task 2 (DY35): `days.service.ts::insert`'s dated path extends the
// trip by one day. Owned exclusively by Task 2 per the plan's file
// ownership (`Trips.repository.ts` ONLY for `setEndDate`) — appended here
diff --git a/server/tests/unit/mcp/tools-prompts.test.ts b/server/tests/unit/mcp/tools-prompts.test.ts
index 06cf890d7..3c01313f2 100644
--- a/server/tests/unit/mcp/tools-prompts.test.ts
+++ b/server/tests/unit/mcp/tools-prompts.test.ts
@@ -87,7 +87,7 @@ import type { TodoService } from '../../../src/nest/todo/todo.service';
import type { CollabService } from '../../../src/nest/collab/collab.service';
import { AddonsService } from '../../../src/nest/addons/addons.service';
import { notificationsStub } from '../../helpers/notifications';
-import { createTestUnitOfWork, createTestAppSettingsRepo, sharedTestOrm } from '../../helpers/test-uow';
+import { createTestUnitOfWork, createTestAppSettingsRepo, createTestTripsRepo, createTestTripMembersRepo, sharedTestOrm } from '../../helpers/test-uow';
import type { EntityManager } from '@mikro-orm/core';
// The trip-summary prompt moved to the DI-discovered TripsMcp — its cases below
@@ -148,7 +148,7 @@ beforeAll(async () => {
new ExchangeRatesService(),
promptDbs(),
new RuntimeEnvService(),
- new TripMembershipService(promptDbs()),
+ new TripMembershipService(await createTestTripsRepo(promptDbs().connection), await createTestTripMembersRepo(promptDbs().connection)),
addonsStub,
promptGuards,
await createTestUnitOfWork(promptDbs().connection),
diff --git a/server/tests/unit/nest/addons.service.test.ts b/server/tests/unit/nest/addons.service.test.ts
index 31cebb91f..3a137a011 100644
--- a/server/tests/unit/nest/addons.service.test.ts
+++ b/server/tests/unit/nest/addons.service.test.ts
@@ -38,6 +38,7 @@ import type { PhotoProviderFieldsRepository } from '../../../src/db/repositories
import { AppSettings } from '../../../src/db/entities/AppSettings.entity';
import type { AppSettingsRepository } from '../../../src/db/repositories/AppSettings.repository';
import { Users } from '../../../src/db/entities/Users.entity';
+import { PlaceShadowPicks } from '../../../src/db/entities/PlaceShadowPicks.entity';
import type { UsersRepository } from '../../../src/db/repositories/Users.repository';
const { getPhotoProviderConfig } = vi.hoisted(() => ({ getPhotoProviderConfig: vi.fn(() => ({})) }));
@@ -587,7 +588,7 @@ describe('AddonsService place shadow flag', () => {
});
it('ADDONS-SVC-093 answers the same as PlaceShadowService.enabled() for every stored value', async () => {
- const shadow = new PlaceShadowService(new DatabaseService(testDb));
+ const shadow = new PlaceShadowService(t.repo(PlaceShadowPicks), appSettingsRepo);
const cases: Array<[string | undefined, boolean]> = [
[undefined, false],
['true', true],
diff --git a/server/tests/unit/nest/admin.service.test.ts b/server/tests/unit/nest/admin.service.test.ts
index 043ccdcb3..ece2a2afb 100644
--- a/server/tests/unit/nest/admin.service.test.ts
+++ b/server/tests/unit/nest/admin.service.test.ts
@@ -92,6 +92,8 @@ import {
createTestMcpTokensRepo,
createTestOauthTokensRepo,
createTestPasswordResetTokensRepo,
+ createTestTripsRepo,
+ createTestTripMembersRepo,
} from '../../helpers/test-uow';
const dbs = new DatabaseService(testDb);
@@ -113,7 +115,7 @@ beforeAll(async () => {
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 createTestUnitOfWork(dbs.connection), await createTestUsersRepo(dbs.connection));
auth = new AuthService(
- permissions, new TripMembershipService(dbs), webauthn, userCleanup, new MailerService(dbs), new EphemeralTokenService(), new AllowedFileTypesService(dbs), await createTestUnitOfWork(dbs.connection),
+ permissions, new TripMembershipService(await createTestTripsRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection)), webauthn, userCleanup, new MailerService(dbs), new EphemeralTokenService(), new AllowedFileTypesService(dbs), await createTestUnitOfWork(dbs.connection),
await createTestAppSettingsRepo(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestInviteTokensRepo(dbs.connection), await createTestMcpTokensRepo(dbs.connection),
await createTestOauthTokensRepo(dbs.connection), await createTestWebauthnCredentialsRepo(dbs.connection), await createTestPasswordResetTokensRepo(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 a0b5137bb..a55ae3557 100644
--- a/server/tests/unit/nest/admin.version-notification.test.ts
+++ b/server/tests/unit/nest/admin.version-notification.test.ts
@@ -70,6 +70,8 @@ import {
createTestMcpTokensRepo,
createTestOauthTokensRepo,
createTestPasswordResetTokensRepo,
+ createTestTripsRepo,
+ createTestTripMembersRepo,
} from '../../helpers/test-uow';
const dbs = new DatabaseService(testDb);
@@ -91,7 +93,7 @@ beforeAll(async () => {
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 createTestUnitOfWork(dbs.connection), await createTestUsersRepo(dbs.connection));
auth = new AuthService(
- permissions, new TripMembershipService(dbs), webauthn, userCleanup, new MailerService(dbs), new EphemeralTokenService(), new AllowedFileTypesService(dbs), await createTestUnitOfWork(dbs.connection),
+ permissions, new TripMembershipService(await createTestTripsRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection)), webauthn, userCleanup, new MailerService(dbs), new EphemeralTokenService(), new AllowedFileTypesService(dbs), await createTestUnitOfWork(dbs.connection),
await createTestAppSettingsRepo(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestInviteTokensRepo(dbs.connection), await createTestMcpTokensRepo(dbs.connection),
await createTestOauthTokensRepo(dbs.connection), await createTestWebauthnCredentialsRepo(dbs.connection), await createTestPasswordResetTokensRepo(dbs.connection),
);
diff --git a/server/tests/unit/nest/collections.service.test.ts b/server/tests/unit/nest/collections.service.test.ts
index 560973064..ce71dc5dc 100644
--- a/server/tests/unit/nest/collections.service.test.ts
+++ b/server/tests/unit/nest/collections.service.test.ts
@@ -55,17 +55,25 @@ import { CollectionsService } from '../../../src/nest/collections/collections.se
import { PlacePhotoCacheService } from '../../../src/nest/place-photos/place-photo-cache.service';
import { makeStorageFixture } from '../../helpers/storage-fixture';
import { notificationsStub } from '../../helpers/notifications';
-import { createTestUnitOfWork, createTestAppSettingsRepo, createTestDatabaseService } from '../../helpers/test-uow';
+import { createTestUnitOfWork, createTestAppSettingsRepo, createTestDatabaseService, createTestGooglePlacePhotoMetaRepo, createTestPlacesRepo } from '../../helpers/test-uow';
const storageFx = makeStorageFixture('');
let svc: CollectionsService;
-beforeAll(async () => {
- svc = new CollectionsService(await createTestDatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new RealtimeService(), notificationsStub(notifSend), storageFx.storage, await createTestUnitOfWork(testDb));
-});
// The real cache: these cases assert what removeIfUnreferenced actually does
// about collection_places (#1081), so a stub would assert nothing.
-const photoCache = new PlacePhotoCacheService(new DatabaseService(testDb), makeStorageFixture('photos/google/').storage);
+// Plan 3c Task 1: built inside the async `beforeAll` below now — the
+// constructor needs two repositories, resolved through `createTestOrm`.
+let photoCache: PlacePhotoCacheService;
const removeIfUnreferenced = (id: string) => photoCache.removeIfUnreferenced(id);
+beforeAll(async () => {
+ svc = new CollectionsService(await createTestDatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new RealtimeService(), notificationsStub(notifSend), storageFx.storage, await createTestUnitOfWork(testDb));
+ photoCache = new PlacePhotoCacheService(
+ new DatabaseService(testDb),
+ makeStorageFixture('photos/google/').storage,
+ await createTestGooglePlacePhotoMetaRepo(testDb),
+ await createTestPlacesRepo(testDb),
+ );
+});
function clearCollections() {
testDb.exec(`
diff --git a/server/tests/unit/nest/days.service.test.ts b/server/tests/unit/nest/days.service.test.ts
index e495818ec..821c246ba 100644
--- a/server/tests/unit/nest/days.service.test.ts
+++ b/server/tests/unit/nest/days.service.test.ts
@@ -67,6 +67,7 @@ import type { Day } from '../../../src/types';
import {
createTestUnitOfWork, createTestAppSettingsRepo, createTestDatabaseService,
createTestDaysRepo, createTestDayAssignmentsRepo, createTestDayNotesRepo, createTestTripsRepo,
+ createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo,
} from '../../helpers/test-uow';
let svc: DaysService;
@@ -76,7 +77,7 @@ beforeAll(async () => {
dbs,
new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)),
new RealtimeService(),
- new QueryHelpersService(dbs),
+ new QueryHelpersService(await createTestTagsRepo(testDb), await createTestPlaceRatingsRepo(testDb), await createTestAssignmentParticipantsRepo(testDb)),
await createTestUnitOfWork(testDb),
await createTestDaysRepo(testDb),
await createTestDayAssignmentsRepo(testDb),
@@ -832,7 +833,7 @@ describe('DaysService.canEdit', () => {
new DatabaseService(testDb),
permissions,
new RealtimeService(),
- new QueryHelpersService(new DatabaseService(testDb)),
+ new QueryHelpersService(await createTestTagsRepo(testDb), await createTestPlaceRatingsRepo(testDb), await createTestAssignmentParticipantsRepo(testDb)),
await createTestUnitOfWork(testDb),
await createTestDaysRepo(testDb),
await createTestDayAssignmentsRepo(testDb),
diff --git a/server/tests/unit/nest/oidc.service.test.ts b/server/tests/unit/nest/oidc.service.test.ts
index 3c333e11c..f33b5f442 100644
--- a/server/tests/unit/nest/oidc.service.test.ts
+++ b/server/tests/unit/nest/oidc.service.test.ts
@@ -115,21 +115,24 @@ import {
createTestOauthTokensRepo,
createTestWebauthnCredentialsRepo,
createTestPasswordResetTokensRepo,
+ createTestTripsRepo,
+ createTestTripMembersRepo,
} from '../../helpers/test-uow';
// MailerService is injected since the notifications fold — a stub instead of a
// module mock. sendPasswordResetEmail is the only thing auth reaches for.
const mailerStub = { sendPasswordResetEmail: vi.fn() } as unknown as MailerService;
-const membership = new TripMembershipService(new DatabaseService(testDb));
// Positional, and previously shifted by one: an AtlasService sat in the membership
// slot, which AuthService no longer takes at all, so webauthn/userCleanup/mailer
// each landed one place too late and the EphemeralTokenService was missing
// entirely. Nothing failed, because no case below reaches those collaborators.
+let membership: TripMembershipService;
let auth: AuthService;
let svc: OidcService;
beforeAll(async () => {
+ membership = new TripMembershipService(await createTestTripsRepo(testDb), await createTestTripMembersRepo(testDb));
auth = new AuthService(
new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)),
membership,
diff --git a/server/tests/unit/nest/passkey.service.test.ts b/server/tests/unit/nest/passkey.service.test.ts
index fd8a6a560..751538d2c 100644
--- a/server/tests/unit/nest/passkey.service.test.ts
+++ b/server/tests/unit/nest/passkey.service.test.ts
@@ -99,6 +99,8 @@ import {
createTestMcpTokensRepo,
createTestOauthTokensRepo,
createTestPasswordResetTokensRepo,
+ createTestTripsRepo,
+ createTestTripMembersRepo,
} from '../../helpers/test-uow';
// MailerService is injected since the notifications fold — a stub instead of a
@@ -126,7 +128,7 @@ let svc: PasskeyService;
beforeAll(async () => {
auth = new AuthService(
new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)),
- new TripMembershipService(new DatabaseService(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 createTestUnitOfWork(testDb), await createTestUsersRepo(testDb)),
mailerStub,
diff --git a/server/tests/unit/nest/place-enrichment.brand.test.ts b/server/tests/unit/nest/place-enrichment.brand.test.ts
index c54ec2fc2..cf0aede89 100644
--- a/server/tests/unit/nest/place-enrichment.brand.test.ts
+++ b/server/tests/unit/nest/place-enrichment.brand.test.ts
@@ -11,21 +11,14 @@
*/
import { describe, it, expect, vi, beforeEach } from 'vitest';
-const { mockDbGet, mockDbRun } = vi.hoisted(() => ({
- // Declared with the shape the fake statement below calls it with (the SQL plus
- // whatever the caller bound), otherwise the spread has no rest parameter to land in.
- mockDbGet: vi.fn((_sql: string, ..._params: unknown[]): unknown => undefined),
- mockDbRun: vi.fn(),
-}));
-
-vi.mock('../../../src/db/database', () => ({
- db: {
- prepare: (sql: string) => ({
- get: (...params: unknown[]) => mockDbGet(sql, ...params),
- run: (...params: unknown[]) => mockDbRun(sql, ...params),
- all: () => [],
- }),
- },
+// Rebuilt on PlaceDetailsCacheRepository/AppSettingsRepository (Plan 3c Task
+// 1, R8's rewrite list) — see place-enrichment.service.test.ts's header for
+// why the legacy SQL-text-keyed `db.prepare` stub cannot survive the service
+// calling two named repository methods instead.
+const { mockGetValue, mockFindEntry, mockUpsertEntry } = vi.hoisted(() => ({
+ mockGetValue: vi.fn(async (_key: string): Promise => null),
+ mockFindEntry: vi.fn(async (..._args: unknown[]): Promise<{ payload_json: string; fetched_at: number } | null> => null),
+ mockUpsertEntry: vi.fn(async (_row: { place_id: string; lang: string; expanded: number; payload_json: string; fetched_at: number }): Promise => {}),
}));
vi.mock('../../../src/utils/ssrfGuard', () => ({
@@ -36,12 +29,12 @@ vi.mock('../../../src/utils/ssrfGuard', () => ({
vi.mock('../../../src/config', () => ({ JWT_SECRET: 'test-secret', ENCRYPTION_KEY: '0'.repeat(64) }));
-import { db } from '../../../src/db/database';
-import { DatabaseService } from '../../../src/nest/database/database.service';
import { PlaceEnrichmentService } from '../../../src/nest/place-enrichment/place-enrichment.service';
import { readBrandIdentity } from '../../../src/nest/maps/maps.service';
import type { MapsService } from '../../../src/nest/maps/maps.service';
import type { PlacePhotoCacheService } from '../../../src/nest/place-photos/place-photo-cache.service';
+import type { PlaceDetailsCacheRepository } from '../../../src/db/repositories/PlaceDetailsCache.repository';
+import type { AppSettingsRepository } from '../../../src/db/repositories/AppSettings.repository';
const REQ = { lat: 54.088, lng: 12.1409, name: "L'Osteria Rostock", placeId: 'ChIJosteria', lang: 'de' };
@@ -83,7 +76,15 @@ function mapsStub(over: Partial> = {}) {
const cacheStub = () =>
({ get: vi.fn(() => null), put: vi.fn(async () => ({ photoUrl: '/x', filePath: '/x', attribution: null })) }) as unknown as PlacePhotoCacheService;
-const make = (maps: MapsService) => new PlaceEnrichmentService(new DatabaseService(db as never), maps, cacheStub());
+function appSettingsStub(): AppSettingsRepository {
+ return { getValue: mockGetValue } as unknown as AppSettingsRepository;
+}
+
+function cacheRepoStub(): PlaceDetailsCacheRepository {
+ return { findEntry: mockFindEntry, upsertEntry: mockUpsertEntry } as unknown as PlaceDetailsCacheRepository;
+}
+
+const make = (maps: MapsService) => new PlaceEnrichmentService(cacheRepoStub(), appSettingsStub(), maps, cacheStub());
/** A place whose own identity is empty but that belongs to a chain. */
const branchOfAChain = (over: Partial> = {}) =>
@@ -93,9 +94,12 @@ const branchOfAChain = (over: Partial> = {})
});
beforeEach(() => {
- mockDbGet.mockReset();
- mockDbGet.mockReturnValue(undefined);
- mockDbRun.mockReset();
+ mockGetValue.mockReset();
+ mockGetValue.mockResolvedValue(null);
+ mockFindEntry.mockReset();
+ mockFindEntry.mockResolvedValue(null);
+ mockUpsertEntry.mockReset();
+ mockUpsertEntry.mockResolvedValue(undefined);
});
describe('readBrandIdentity', () => {
diff --git a/server/tests/unit/nest/place-enrichment.description.test.ts b/server/tests/unit/nest/place-enrichment.description.test.ts
index 86b4f2f14..258b25afe 100644
--- a/server/tests/unit/nest/place-enrichment.description.test.ts
+++ b/server/tests/unit/nest/place-enrichment.description.test.ts
@@ -9,22 +9,14 @@
*/
import { describe, it, expect, vi, beforeEach } from 'vitest';
-const { mockDbGet, mockDbRun } = vi.hoisted(() => ({
- // The prepare() stub below forwards the SQL and then the bound values, so the
- // spy has to declare that shape. Inferred from a zero-arg implementation it
- // would reject the spread at the call site.
- mockDbGet: vi.fn((_sql: string, ..._params: unknown[]): unknown => undefined),
- mockDbRun: vi.fn(),
-}));
-
-vi.mock('../../../src/db/database', () => ({
- db: {
- prepare: (sql: string) => ({
- get: (...params: unknown[]) => mockDbGet(sql, ...params),
- run: (...params: unknown[]) => mockDbRun(sql, ...params),
- all: () => [],
- }),
- },
+// Rebuilt on PlaceDetailsCacheRepository/AppSettingsRepository (Plan 3c Task
+// 1, R8's rewrite list) — see place-enrichment.service.test.ts's header for
+// why the legacy SQL-text-keyed `db.prepare` stub cannot survive the service
+// calling two named repository methods instead.
+const { mockGetValue, mockFindEntry, mockUpsertEntry } = vi.hoisted(() => ({
+ mockGetValue: vi.fn(async (_key: string): Promise => null),
+ mockFindEntry: vi.fn(async (..._args: unknown[]): Promise<{ payload_json: string; fetched_at: number } | null> => null),
+ mockUpsertEntry: vi.fn(async (_row: { place_id: string; lang: string; expanded: number; payload_json: string; fetched_at: number }): Promise => {}),
}));
vi.mock('../../../src/utils/ssrfGuard', () => ({
@@ -42,11 +34,11 @@ vi.mock('../../../src/nest/maps/trek-places.client', () => ({
trekPlacesById: mockTrekPlacesById,
}));
-import { db } from '../../../src/db/database';
-import { DatabaseService } from '../../../src/nest/database/database.service';
import { PlaceEnrichmentService } from '../../../src/nest/place-enrichment/place-enrichment.service';
import type { MapsService } from '../../../src/nest/maps/maps.service';
import type { PlacePhotoCacheService } from '../../../src/nest/place-photos/place-photo-cache.service';
+import type { PlaceDetailsCacheRepository } from '../../../src/db/repositories/PlaceDetailsCache.repository';
+import type { AppSettingsRepository } from '../../../src/db/repositories/AppSettings.repository';
const GOOGLE_REQ = { lat: 53.6323, lng: 10.0067, name: 'Hamburg Airport', placeId: 'ChIJham', lang: 'de' };
const OSM_REQ = { lat: 52.525, lng: 13.3694, name: 'Berlin Hauptbahnhof', placeId: 'relation:3600565', lang: 'de' };
@@ -83,12 +75,23 @@ function mapsStub(over: Partial> = {}) {
const cacheStub = () =>
({ get: vi.fn(() => null), put: vi.fn(async () => ({ photoUrl: '/x', filePath: '/x', attribution: null })) }) as unknown as PlacePhotoCacheService;
-const make = (maps: MapsService) => new PlaceEnrichmentService(new DatabaseService(db as never), maps, cacheStub());
+function appSettingsStub(): AppSettingsRepository {
+ return { getValue: mockGetValue } as unknown as AppSettingsRepository;
+}
+
+function cacheRepoStub(): PlaceDetailsCacheRepository {
+ return { findEntry: mockFindEntry, upsertEntry: mockUpsertEntry } as unknown as PlaceDetailsCacheRepository;
+}
+
+const make = (maps: MapsService) => new PlaceEnrichmentService(cacheRepoStub(), appSettingsStub(), maps, cacheStub());
beforeEach(() => {
- mockDbGet.mockReset();
- mockDbGet.mockReturnValue(undefined);
- mockDbRun.mockReset();
+ mockGetValue.mockReset();
+ mockGetValue.mockResolvedValue(null);
+ mockFindEntry.mockReset();
+ mockFindEntry.mockResolvedValue(null);
+ mockUpsertEntry.mockReset();
+ mockUpsertEntry.mockResolvedValue(undefined);
mockTrekPlacesById.mockReset();
mockTrekPlacesById.mockResolvedValue(null);
});
@@ -417,9 +420,10 @@ describe("the description on the place's own website", () => {
source: 'website',
sourceUrl: 'https://losteria.net/rostock',
});
- const written = mockDbRun.mock.calls.find(([sql]) => String(sql).includes('INSERT OR REPLACE INTO place_details_cache'));
+ expect(mockUpsertEntry).toHaveBeenCalledTimes(1);
+ const [written] = mockUpsertEntry.mock.calls[0];
expect(written).toBeTruthy();
- expect(String(written![4])).not.toContain('phish.example');
+ expect(written.payload_json).not.toContain('phish.example');
});
it('ENRICH-101: asks when the caller passed no details, all the same', async () => {
@@ -468,8 +472,8 @@ describe("the description on the place's own website", () => {
});
expect(out.description).toBeNull();
- for (const [sql, ...params] of mockDbRun.mock.calls) {
- expect(JSON.stringify(params), String(sql)).not.toContain('phish.example');
+ for (const [row] of mockUpsertEntry.mock.calls) {
+ expect(JSON.stringify(row)).not.toContain('phish.example');
}
});
@@ -490,7 +494,7 @@ describe("the description on the place's own website", () => {
const out = await make(mapsStub()).enrich(1, carried);
expect(out.description).toMatchObject({ source: 'osm', text: 'Buchung nur noch über https://phish.example' });
- expect(mockDbRun.mock.calls.some(([sql]) => String(sql).includes('place_details_cache'))).toBe(false);
+ expect(mockUpsertEntry).not.toHaveBeenCalled();
// The same summary from the service's own lookup is the map's, and keeps.
const maps = mapsStub({
@@ -498,10 +502,10 @@ describe("the description on the place's own website", () => {
place: { source: 'openstreetmap', summary: 'Größter Kreuzungsbahnhof Europas.', osm_url: 'https://www.openstreetmap.org/relation/3600565' },
})),
});
- mockDbRun.mockClear();
+ mockUpsertEntry.mockClear();
const own = await make(maps).enrich(1, OSM_REQ);
expect(own.description).toMatchObject({ source: 'osm', text: 'Größter Kreuzungsbahnhof Europas.' });
- expect(mockDbRun.mock.calls.some(([sql]) => String(sql).includes('INSERT OR REPLACE INTO place_details_cache'))).toBe(true);
+ expect(mockUpsertEntry).toHaveBeenCalledTimes(1);
});
it('ENRICH-121: a menu link the request carried is answered, but not cached for everyone', async () => {
@@ -515,16 +519,16 @@ describe("the description on the place's own website", () => {
});
expect(out.facts).toContainEqual({ kind: 'menu', value: null, url: 'https://phish.example/menu' });
- expect(mockDbRun.mock.calls.some(([sql]) => String(sql).includes('place_details_cache'))).toBe(false);
+ expect(mockUpsertEntry).not.toHaveBeenCalled();
// The same link from the service's own lookup is the map's, and keeps.
const maps = mapsStub({
details: vi.fn(async () => ({ place: { source: 'openstreetmap', menu_url: 'https://example.org/karte' } })),
});
- mockDbRun.mockClear();
+ mockUpsertEntry.mockClear();
const own = await make(maps).enrich(1, OSM_REQ);
expect(own.facts).toContainEqual({ kind: 'menu', value: null, url: 'https://example.org/karte' });
- expect(mockDbRun.mock.calls.some(([sql]) => String(sql).includes('INSERT OR REPLACE INTO place_details_cache'))).toBe(true);
+ expect(mockUpsertEntry).toHaveBeenCalledTimes(1);
});
it('ENRICH-119: with the index switched off, no lookup leaves for the description', async () => {
diff --git a/server/tests/unit/nest/place-enrichment.service.test.ts b/server/tests/unit/nest/place-enrichment.service.test.ts
index bf3a86261..1d81c9244 100644
--- a/server/tests/unit/nest/place-enrichment.service.test.ts
+++ b/server/tests/unit/nest/place-enrichment.service.test.ts
@@ -4,34 +4,24 @@
* The service is a fan-out over MapsService primitives plus the shared photo
* cache, so both are injected as stubs and every provider branch is driven from
* the test. The SSRF guard is mocked the same way maps.service.test.ts mocks it.
+ *
+ * Rebuilt on PlaceDetailsCacheRepository/AppSettingsRepository (Plan 3c Task
+ * 1, R8's rewrite list): the legacy version stubbed `db.prepare(sql).{get,run}`
+ * and branched on whether the SQL text contained `'place_details_cache'` to
+ * tell the settings read (PE1) apart from the cache read (PE2)/write (PE3).
+ * That coupling cannot survive the service calling two named repository
+ * methods instead of one shared connection — each repository method now has
+ * its own controllable fake, with no SQL-text branching anywhere.
*/
import { describe, it, expect, vi, beforeEach } from 'vitest';
-/** Whatever the statement under test selects: a settings row, a cache row, or nothing. */
-type DbRow = Record | undefined;
-
-const { mockDbGet, mockDbRun, mockSafeFetchFollow } = vi.hoisted(() => ({
- // Signature taken from the seam below rather than from this default: the
- // statement text is folded in ahead of the params, so a no-argument spy
- // would reject both the call site and every mockImplementation here.
- mockDbGet: vi.fn((_sql: string, ..._params: unknown[]): DbRow => undefined),
- mockDbRun: vi.fn((_sql: string, ..._params: unknown[]) => {}),
+const { mockGetValue, mockFindEntry, mockUpsertEntry, mockSafeFetchFollow } = vi.hoisted(() => ({
+ mockGetValue: vi.fn(async (_key: string): Promise => null),
+ mockFindEntry: vi.fn(async (..._args: unknown[]): Promise<{ payload_json: string; fetched_at: number } | null> => null),
+ mockUpsertEntry: vi.fn(async (..._args: unknown[]): Promise => {}),
mockSafeFetchFollow: vi.fn(async () => ({ ok: true, arrayBuffer: async () => new Uint8Array([1, 2, 3]).buffer })),
}));
-// prepare() takes the SQL and the statement takes the params, so the spies get
-// the statement text folded back in as their first argument — the assertions
-// below distinguish the settings read from the cache read by exactly that.
-vi.mock('../../../src/db/database', () => ({
- db: {
- prepare: (sql: string) => ({
- get: (...params: unknown[]) => mockDbGet(sql, ...params),
- run: (...params: unknown[]) => mockDbRun(sql, ...params),
- all: () => [],
- }),
- },
-}));
-
vi.mock('../../../src/utils/ssrfGuard', () => ({
safeFetchFollow: mockSafeFetchFollow,
checkSsrf: vi.fn(async () => ({ allowed: true })),
@@ -51,8 +41,6 @@ vi.mock('../../../src/nest/maps/trek-places.client', async (importOriginal) => (
trekPlacesById: mockTrekPlacesById,
}));
-import { db } from '../../../src/db/database';
-import { DatabaseService } from '../../../src/nest/database/database.service';
import {
candidateKey,
PlaceEnrichmentService,
@@ -64,6 +52,16 @@ import {
} from '../../../src/nest/place-enrichment/place-enrichment.service';
import type { MapsService } from '../../../src/nest/maps/maps.service';
import type { PlacePhotoCacheService } from '../../../src/nest/place-photos/place-photo-cache.service';
+import type { PlaceDetailsCacheRepository } from '../../../src/db/repositories/PlaceDetailsCache.repository';
+import type { AppSettingsRepository } from '../../../src/db/repositories/AppSettings.repository';
+
+function appSettingsStub(): AppSettingsRepository {
+ return { getValue: mockGetValue } as unknown as AppSettingsRepository;
+}
+
+function cacheRepoStub(): PlaceDetailsCacheRepository {
+ return { findEntry: mockFindEntry, upsertEntry: mockUpsertEntry } as unknown as PlaceDetailsCacheRepository;
+}
const REQ = { lat: 50.9, lng: 6.96, name: 'Museum Ludwig', placeId: 'ChIJmuseum' };
const OSM_REQ = { ...REQ, placeId: 'way:12345' };
@@ -105,7 +103,7 @@ function cacheStub(over: Record = {}) {
}
function make(maps: MapsService, cache: PlacePhotoCacheService) {
- return new PlaceEnrichmentService(new DatabaseService(db as never), maps, cache);
+ return new PlaceEnrichmentService(cacheRepoStub(), appSettingsStub(), maps, cache);
}
let candidateSeq = 0;
@@ -133,9 +131,12 @@ beforeEach(() => {
// Per test, so a fixture's page id is a property of the case rather than of
// where the case sits in the file.
candidateSeq = 0;
- mockDbGet.mockReset();
- mockDbGet.mockReturnValue(undefined);
- mockDbRun.mockReset();
+ mockGetValue.mockReset();
+ mockGetValue.mockResolvedValue(null);
+ mockFindEntry.mockReset();
+ mockFindEntry.mockResolvedValue(null);
+ mockUpsertEntry.mockReset();
+ mockUpsertEntry.mockResolvedValue(undefined);
mockSafeFetchFollow.mockReset();
mockSafeFetchFollow.mockResolvedValue({ ok: true, arrayBuffer: async () => new Uint8Array([1, 2, 3]).buffer });
mockTrekPlacesById.mockReset();
@@ -148,19 +149,19 @@ describe('enrichDisabled', () => {
it('ENRICH-001: is off only when the setting is literally "false"', async () => {
const svc = make(mapsStub(), cacheStub());
- mockDbGet.mockReturnValue(undefined);
+ mockGetValue.mockResolvedValue(null);
expect(await svc.enrichDisabled()).toBe(false); // never configured — fail open
- mockDbGet.mockReturnValue({ value: 'true' });
+ mockGetValue.mockResolvedValue('true');
expect(await svc.enrichDisabled()).toBe(false);
- mockDbGet.mockReturnValue({ value: 'false' });
+ mockGetValue.mockResolvedValue('false');
expect(await svc.enrichDisabled()).toBe(true);
});
it('ENRICH-002: answers the disabled envelope without touching a provider', async () => {
const maps = mapsStub();
- mockDbGet.mockReturnValue({ value: 'false' });
+ mockGetValue.mockResolvedValue('false');
const out = await make(maps, cacheStub()).enrich(1, REQ);
@@ -547,10 +548,8 @@ describe('result cache', () => {
it('ENRICH-021: serves a fresh entry without calling any provider', async () => {
const maps = mapsStub();
const cache = cacheStub({ get: vi.fn(() => ({ photoUrl: '/x', filePath: '/tmp/x', attribution: null })) });
- mockDbGet.mockImplementation((sql: string) =>
- String(sql).includes('place_details_cache')
- ? cachedRow([{ key: 'ChIJmuseum~p0', url: '/x', attribution: null, license: null, licenseUrl: null, sourceUrl: null, source: 'wikimedia' }])
- : undefined,
+ mockFindEntry.mockResolvedValue(
+ cachedRow([{ key: 'ChIJmuseum~p0', url: '/x', attribution: null, license: null, licenseUrl: null, sourceUrl: null, source: 'wikimedia' }]),
);
const out = await make(maps, cache).enrich(1, REQ);
@@ -562,9 +561,7 @@ describe('result cache', () => {
it('ENRICH-022: refetches once the entry is older than a week', async () => {
const maps = mapsStub({ fetchCommonsCandidates: vi.fn(async () => [commonsCandidate()]) });
const eightDaysAgo = Date.now() - 8 * 24 * 60 * 60 * 1000;
- mockDbGet.mockImplementation((sql: string) =>
- String(sql).includes('place_details_cache') ? cachedRow([], eightDaysAgo) : undefined,
- );
+ mockFindEntry.mockResolvedValue(cachedRow([], eightDaysAgo));
await make(maps, cacheStub()).enrich(1, REQ);
@@ -584,10 +581,8 @@ describe('result cache', () => {
return { photoUrl: `/api/maps/place-photo/${key}/bytes`, filePath: '/tmp/x', attribution: null };
}),
});
- mockDbGet.mockImplementation((sql: string) =>
- String(sql).includes('place_details_cache')
- ? cachedRow([{ key: 'ChIJmuseum~p0', url: '/x', attribution: null, license: null, licenseUrl: null, sourceUrl: null, source: 'wikimedia' }])
- : undefined,
+ mockFindEntry.mockResolvedValue(
+ cachedRow([{ key: 'ChIJmuseum~p0', url: '/x', attribution: null, license: null, licenseUrl: null, sourceUrl: null, source: 'wikimedia' }]),
);
const out = await make(maps, cache).enrich(1, REQ);
@@ -599,10 +594,8 @@ describe('result cache', () => {
it('ENRICH-023b: keeps serving a cached entry while all its bytes are still there', async () => {
const maps = mapsStub();
const cache = cacheStub({ get: vi.fn(() => ({ photoUrl: '/x', filePath: '/tmp/x', attribution: null })) });
- mockDbGet.mockImplementation((sql: string) =>
- String(sql).includes('place_details_cache')
- ? cachedRow([{ key: 'ChIJmuseum~p0', url: '/x', attribution: null, license: null, licenseUrl: null, sourceUrl: null, source: 'wikimedia' }])
- : undefined,
+ mockFindEntry.mockResolvedValue(
+ cachedRow([{ key: 'ChIJmuseum~p0', url: '/x', attribution: null, license: null, licenseUrl: null, sourceUrl: null, source: 'wikimedia' }]),
);
const out = await make(maps, cache).enrich(1, REQ);
@@ -614,9 +607,7 @@ describe('result cache', () => {
it('ENRICH-023c: discards an entry written by an older format version', async () => {
// Otherwise a release that adds a field keeps serving last week's answers.
const maps = mapsStub({ fetchCommonsCandidates: vi.fn(async () => [commonsCandidate()]) });
- mockDbGet.mockImplementation((sql: string) =>
- String(sql).includes('place_details_cache') ? cachedRow([], Date.now(), 1) : undefined,
- );
+ mockFindEntry.mockResolvedValue(cachedRow([], Date.now(), 1));
await make(maps, cacheStub()).enrich(1, REQ);
@@ -625,9 +616,7 @@ describe('result cache', () => {
it('ENRICH-024: ignores an unreadable cache row and rebuilds', async () => {
const maps = mapsStub({ fetchCommonsCandidates: vi.fn(async () => [commonsCandidate()]) });
- mockDbGet.mockImplementation((sql: string) =>
- String(sql).includes('place_details_cache') ? { payload_json: '{not json', fetched_at: Date.now() } : undefined,
- );
+ mockFindEntry.mockResolvedValue({ payload_json: '{not json', fetched_at: Date.now() });
const out = await make(maps, cacheStub()).enrich(1, REQ);
@@ -639,17 +628,17 @@ describe('result cache', () => {
await make(maps, cacheStub()).enrich(1, { ...REQ, lang: 'de' });
- const write = mockDbRun.mock.calls.find((c) => String(c[0]).includes('INSERT OR REPLACE INTO place_details_cache'));
- expect(write).toBeDefined();
- expect(write![1]).toBe('ChIJmuseum');
- expect(write![2]).toBe('de');
+ expect(mockUpsertEntry).toHaveBeenCalledTimes(1);
+ const [written] = mockUpsertEntry.mock.calls[0] as [{ place_id: string; lang: string; expanded: number; payload_json: string; fetched_at: number }];
+ expect(written.place_id).toBe('ChIJmuseum');
+ expect(written.lang).toBe('de');
// expanded = 2 — the plain (0) and reviews (1) caches keep their rows.
- expect(write![3]).toBe(2);
+ expect(written.expanded).toBe(2);
});
it('ENRICH-026: keeps working when the cache write fails', async () => {
const maps = mapsStub({ fetchCommonsCandidates: vi.fn(async () => [commonsCandidate()]) });
- mockDbRun.mockImplementation(() => { throw new Error('db locked'); });
+ mockUpsertEntry.mockRejectedValue(new Error('db locked'));
const err = vi.spyOn(console, 'error').mockImplementation(() => {});
const out = await make(maps, cacheStub()).enrich(1, REQ);
diff --git a/server/tests/unit/nest/place-photos/place-photo-cache.service.test.ts b/server/tests/unit/nest/place-photos/place-photo-cache.service.test.ts
index 046a85367..14a448f32 100644
--- a/server/tests/unit/nest/place-photos/place-photo-cache.service.test.ts
+++ b/server/tests/unit/nest/place-photos/place-photo-cache.service.test.ts
@@ -10,6 +10,16 @@
* bare keys) — via the shared stub-registry fixture. The real mode flip is
* pinned by the storage-registry tests; here the two prefixes prove the cache
* itself is mode-agnostic.
+ *
+ * Rebuilt on GooglePlacePhotoMetaRepository/PlacesRepository (Plan 3c Task 1):
+ * the legacy version cast a hand-rolled 3-table SQLite fixture to
+ * `DatabaseService`; that cannot survive the service now calling two
+ * repositories for its owned reads/writes. The same minimal 3-table fixture
+ * stays (every column these repositories touch is already in it — the R8
+ * `createSnapshotTestDb()` migration only applies once a converted read
+ * needs a column the hand-rolled DDL omits, which is not the case here), but
+ * it is now wired through `createTestOrm()` and `DatabaseService` is kept
+ * ONLY for the one raw `collection_places` statement (PP6's Plan 3h half).
*/
import { describe, it, expect, beforeAll, beforeEach, afterAll, vi } from 'vitest';
import path from 'node:path';
@@ -48,6 +58,11 @@ vi.mock('../../../../src/db/database', () => ({ db: testDb }));
import { PlacePhotoCacheService } from '../../../../src/nest/place-photos/place-photo-cache.service';
import { DatabaseService } from '../../../../src/nest/database/database.service';
+import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
+import { GooglePlacePhotoMeta } from '../../../../src/db/entities/GooglePlacePhotoMeta.entity';
+import type { GooglePlacePhotoMetaRepository } from '../../../../src/db/repositories/GooglePlacePhotoMeta.repository';
+import { Places } from '../../../../src/db/entities/Places.entity';
+import type { PlacesRepository } from '../../../../src/db/repositories/Places.repository';
import { makeStorageFixture, type StorageFixture } from '../../../helpers/storage-fixture';
async function makeJpeg(width: number, height: number): Promise {
@@ -59,6 +74,17 @@ function nameFor(placeId: string): string {
return `${crypto.createHash('sha1').update(placeId).digest('hex')}.jpg`;
}
+let t: TestOrm;
+let metaRepo: GooglePlacePhotoMetaRepository;
+let placesRepo: PlacesRepository;
+
+beforeAll(async () => {
+ t = await createTestOrm(testDb, { allowGlobalContext: true });
+ metaRepo = t.repo(GooglePlacePhotoMeta);
+ placesRepo = t.repo(Places);
+});
+afterAll(async () => { await t.close(); });
+
describe.each([
['mode A (photos/google/ prefix)', 'photos/google/'],
['mode B (bare keys)', ''],
@@ -75,11 +101,12 @@ describe.each([
beforeAll(() => {
fx = makeStorageFixture(keyPrefix);
- cache = new PlacePhotoCacheService(new DatabaseService(testDb as never), fx.storage);
+ cache = new PlacePhotoCacheService(new DatabaseService(testDb as never), fx.storage, metaRepo, placesRepo);
});
beforeEach(() => {
testDb.exec('DELETE FROM places; DELETE FROM collection_places; DELETE FROM google_place_photo_meta;');
+ t.clear();
for (const f of fs.readdirSync(fx.root)) {
if (f === '.tmp') continue;
fs.rmSync(path.join(fx.root, f), { recursive: true, force: true });
@@ -180,6 +207,20 @@ describe.each([
expect(fs.existsSync(filePathFor(id))).toBe(true);
});
+
+ // Plan 3c Task 1 PP6 ruling: the `collection_places` half stays raw
+ // (Plan 3h owns that table) and only runs when the `places` half comes
+ // back false — this is the direct proof that half is still wired,
+ // through the SAME `isReferenced` call as the two above.
+ it('PPC-016: keeps an entry referenced only through collection_places, proving the Plan 3h raw fallback still fires', async () => {
+ const id = 'coll-only';
+ await cache.put(id, await makeJpeg(50, 50), null);
+ testDb.prepare('INSERT INTO collection_places (google_place_id) VALUES (?)').run(id);
+
+ await cache.removeIfUnreferenced(id);
+
+ expect(fs.existsSync(filePathFor(id))).toBe(true);
+ });
});
describe('sweepOrphans()', () => {
diff --git a/server/tests/unit/nest/place-shadow.service.test.ts b/server/tests/unit/nest/place-shadow.service.test.ts
index b8c0ab51d..b667e551f 100644
--- a/server/tests/unit/nest/place-shadow.service.test.ts
+++ b/server/tests/unit/nest/place-shadow.service.test.ts
@@ -1,49 +1,28 @@
/**
- * PlaceShadowService against a real in-memory database.
+ * PlaceShadowService against real rows (Plan 3c Task 1).
*
* The interesting parts of this service ARE the SQL — the id-paged export, the
- * rank bucketing, the age-based retention — so a mocked DatabaseService would
- * assert that the strings were passed along and prove nothing about what they
- * do. The table is created here rather than by running the migration array, so
- * appending the next migration cannot drag this file along.
+ * rank bucketing, the age-based retention — so a mocked repository would
+ * assert that values were passed along and prove nothing about what they do.
+ * Rebuilt off the real migrated schema snapshot (`createSnapshotTestDb()` +
+ * `createTestOrm()`, the harness every other converted repository test uses)
+ * rather than the legacy hand-written `SCHEMA`/`dbFacade` — R8's rewrite list.
*/
-import { describe, it, expect, beforeEach } from 'vitest';
-import Database from 'better-sqlite3';
+import { describe, it, expect, beforeAll, beforeEach, afterAll } from 'vitest';
+import { createSnapshotTestDb } from '../../helpers/db-mock';
+import { resetTestDb } from '../../helpers/test-db';
+import { createTestOrm, type TestOrm } from '../../helpers/test-orm';
import { PlaceShadowService, RETENTION_DAYS } from '../../../src/nest/place-shadow/place-shadow.service';
-import type { DatabaseService } from '../../../src/nest/database/database.service';
+import { PlaceShadowPicks } from '../../../src/db/entities/PlaceShadowPicks.entity';
+import type { PlaceShadowPicksRepository } from '../../../src/db/repositories/PlaceShadowPicks.repository';
+import { AppSettings } from '../../../src/db/entities/AppSettings.entity';
+import type { AppSettingsRepository } from '../../../src/db/repositories/AppSettings.repository';
import type { PlaceShadowPickRequest } from '@trek/shared';
-const SCHEMA = `
- CREATE TABLE app_settings (key TEXT PRIMARY KEY, value TEXT);
- CREATE TABLE place_shadow_picks (
- id INTEGER PRIMARY KEY AUTOINCREMENT,
- created_at TEXT NOT NULL DEFAULT (datetime('now')),
- query TEXT NOT NULL,
- lang TEXT,
- bias_lat REAL,
- bias_lng REAL,
- source TEXT NOT NULL,
- live_rank INTEGER NOT NULL,
- live_count INTEGER NOT NULL,
- picked_name TEXT NOT NULL,
- picked_lat REAL NOT NULL,
- picked_lng REAL NOT NULL,
- picked_place_id TEXT
- );
-`;
-
-let conn: Database.Database;
+const testDb = createSnapshotTestDb();
+let t: TestOrm;
let svc: PlaceShadowService;
-/** Just the three methods the service uses, backed by the real connection. */
-function dbFacade(c: Database.Database): DatabaseService {
- return {
- get: (sql: string, ...p: unknown[]) => c.prepare(sql).get(...p) as T | undefined,
- all: (sql: string, ...p: unknown[]) => c.prepare(sql).all(...p) as T[],
- run: (sql: string, ...p: unknown[]) => c.prepare(sql).run(...p),
- } as unknown as DatabaseService;
-}
-
const PICK: PlaceShadowPickRequest = {
query: 'losteria rostock',
lang: 'de',
@@ -59,27 +38,30 @@ const PICK: PlaceShadowPickRequest = {
};
function enable(on = true) {
- conn.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)')
+ testDb.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)')
.run('place_shadow_enabled', on ? 'true' : 'false');
}
-beforeEach(async () => {
- conn = new Database(':memory:');
- conn.exec(SCHEMA);
- svc = new PlaceShadowService(dbFacade(conn));
+beforeAll(async () => {
+ t = await createTestOrm(testDb);
+ const picks: PlaceShadowPicksRepository = t.repo(PlaceShadowPicks);
+ const appSettings: AppSettingsRepository = t.repo(AppSettings);
+ svc = new PlaceShadowService(picks, appSettings);
});
+beforeEach(() => { resetTestDb(testDb); t.clear(); });
+afterAll(async () => { await t.close(); testDb.close(); });
describe('PlaceShadowService', () => {
describe('the switch', () => {
it('is off when the setting row is absent', async () => {
expect(await svc.enabled()).toBe(false);
expect(await svc.record(PICK)).toBe(false);
- expect(conn.prepare('SELECT COUNT(*) AS n FROM place_shadow_picks').get()).toEqual({ n: 0 });
+ expect(testDb.prepare('SELECT COUNT(*) AS n FROM place_shadow_picks').get()).toEqual({ n: 0 });
});
it('is off for any value that is not exactly "true"', async () => {
for (const value of ['false', '1', 'yes', 'TRUE', '']) {
- conn.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)')
+ testDb.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)')
.run('place_shadow_enabled', value);
expect(await svc.enabled(), `value ${JSON.stringify(value)}`).toBe(false);
}
@@ -97,7 +79,7 @@ describe('PlaceShadowService', () => {
it('rounds every coordinate to three decimals', async () => {
await svc.record({ ...PICK, pickedLat: 54.0891234, pickedLng: 12.1372987, biasLat: 54.08871, biasLng: 12.14049 });
- const row = conn.prepare('SELECT * FROM place_shadow_picks').get() as Record;
+ const row = testDb.prepare('SELECT * FROM place_shadow_picks').get() as Record;
expect(row.picked_lat).toBe(54.089);
expect(row.picked_lng).toBe(12.137);
expect(row.bias_lat).toBe(54.089);
@@ -106,7 +88,7 @@ describe('PlaceShadowService', () => {
it('stores an absent bias and place id as NULL rather than inventing zeroes', async () => {
await svc.record({ ...PICK, biasLat: undefined, biasLng: undefined, pickedPlaceId: undefined });
- const row = conn.prepare('SELECT * FROM place_shadow_picks').get() as Record;
+ const row = testDb.prepare('SELECT * FROM place_shadow_picks').get() as Record;
expect(row.bias_lat).toBeNull();
expect(row.bias_lng).toBeNull();
expect(row.picked_place_id).toBeNull();
@@ -156,7 +138,7 @@ describe('PlaceShadowService', () => {
});
it('answers on an empty corpus without dividing by zero', async () => {
- conn.exec('DELETE FROM place_shadow_picks');
+ testDb.exec('DELETE FROM place_shadow_picks');
const s = await svc.summary();
expect(s).toMatchObject({ total: 0, liveTopOneShare: 0, liveTopFiveShare: 0, oldest: null, newest: null });
expect(s.retentionDays).toBe(RETENTION_DAYS);
@@ -214,21 +196,25 @@ describe('PlaceShadowService', () => {
it('removes only rows past the window', async () => {
await svc.record(PICK);
await svc.record(PICK);
- conn.prepare("UPDATE place_shadow_picks SET created_at = datetime('now', '-200 days') WHERE id = 1").run();
+ // `resetTestDb` never resets `sqlite_sequence` (ids keep growing across
+ // tests in this file, by design — see its own docstring), so the
+ // "first" row's id is read back rather than hardcoded as `1`.
+ const [first] = testDb.prepare('SELECT id FROM place_shadow_picks ORDER BY id ASC').all() as { id: number }[];
+ testDb.prepare("UPDATE place_shadow_picks SET created_at = datetime('now', '-200 days') WHERE id = ?").run(first.id);
expect(await svc.purgeExpired()).toBe(1);
expect((await svc.summary()).total).toBe(1);
});
it('keeps a row that is one day short of the window', async () => {
await svc.record(PICK);
- conn.prepare("UPDATE place_shadow_picks SET created_at = datetime('now', ?)")
+ testDb.prepare("UPDATE place_shadow_picks SET created_at = datetime('now', ?)")
.run(`-${RETENTION_DAYS - 1} days`);
expect(await svc.purgeExpired()).toBe(0);
});
it('runs even while the log is switched off, so old rows still age out', async () => {
await svc.record(PICK);
- conn.prepare("UPDATE place_shadow_picks SET created_at = datetime('now', '-200 days')").run();
+ testDb.prepare("UPDATE place_shadow_picks SET created_at = datetime('now', '-200 days')").run();
enable(false);
expect(await svc.purgeExpired()).toBe(1);
});
diff --git a/server/tests/unit/nest/places.directions.test.ts b/server/tests/unit/nest/places.directions.test.ts
index 0ac33eb98..2b4b52d60 100644
--- a/server/tests/unit/nest/places.directions.test.ts
+++ b/server/tests/unit/nest/places.directions.test.ts
@@ -54,7 +54,7 @@ import type { PlacePhotoCacheService } from '../../../src/nest/place-photos/plac
import { JourneyDomainService } from '../../../src/nest/journey/journey-domain.service';
import { TrekPhotosRepository } from '../../../src/nest/photos/trek-photos.repository';
import { makeStorageFixture } from '../../helpers/storage-fixture';
-import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm } from '../../helpers/test-uow';
+import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo, sharedTestOrm } from '../../helpers/test-uow';
const dbs = new DatabaseService(testDb);
const photoCacheStub = { removeIfUnreferenced: vi.fn() } as unknown as PlacePhotoCacheService;
@@ -86,7 +86,7 @@ async function svc(searchNominatim: MapsService['searchNominatim']): Promise null),
} as unknown as MapsService,
- new QueryHelpersService(dbs),
+ 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(), storageFx.storage),
photoCacheStub,
new JourneyDomainService(dbs, new RealtimeService(), new TrekPhotosRepository(dbs), await createTestUnitOfWork(dbs.connection)),
diff --git a/server/tests/unit/nest/places.service.test.ts b/server/tests/unit/nest/places.service.test.ts
index 2f6276e53..616d3e0f0 100644
--- a/server/tests/unit/nest/places.service.test.ts
+++ b/server/tests/unit/nest/places.service.test.ts
@@ -78,7 +78,7 @@ import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpe
import { JourneyDomainService } from '../../../src/nest/journey/journey-domain.service';
import { TrekPhotosRepository } from '../../../src/nest/photos/trek-photos.repository';
import { makeStorageFixture } from '../../helpers/storage-fixture';
-import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm } from '../../helpers/test-uow';
+import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo, sharedTestOrm } from '../../helpers/test-uow';
import type { AppSettingsRepository } from '../../../src/db/repositories/AppSettings.repository';
import type { UsersRepository } from '../../../src/db/repositories/Users.repository';
@@ -113,7 +113,7 @@ async function makePlacesService(
new PermissionsService(await createTestAppSettingsRepo(dbs.connection), await createTestUnitOfWork(dbs.connection)),
new RealtimeService(),
maps,
- new QueryHelpersService(dbs),
+ 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(), placesStorageFx.storage),
photoCacheStub,
new JourneyDomainService(dbs, new RealtimeService(), new TrekPhotosRepository(dbs), await createTestUnitOfWork(dbs.connection)),
diff --git a/server/tests/unit/nest/query-helpers.service.test.ts b/server/tests/unit/nest/query-helpers.service.test.ts
new file mode 100644
index 000000000..d01bf9f22
--- /dev/null
+++ b/server/tests/unit/nest/query-helpers.service.test.ts
@@ -0,0 +1,183 @@
+/**
+ * Unit tests for QueryHelpersService (Plan 3c Task 1) — its first dedicated
+ * test file (inventory §15c: previously exercised only transitively through
+ * days/assignments/places/share suites). Real rows through
+ * `TagsRepository`/`PlaceRatingsRepository`/`AssignmentParticipantsRepository`.
+ */
+import { describe, it, expect, beforeAll, beforeEach, afterAll } from 'vitest';
+import { createSnapshotTestDb } from '../../helpers/db-mock';
+import { resetTestDb } from '../../helpers/test-db';
+import { createTestOrm, type TestOrm } from '../../helpers/test-orm';
+import { createDay, createDayAssignment, createPlace, createTag, createTrip, createUser } from '../../helpers/factories';
+import { Tags } from '../../../src/db/entities/Tags.entity';
+import type { TagsRepository } from '../../../src/db/repositories/Tags.repository';
+import { PlaceRatings } from '../../../src/db/entities/PlaceRatings.entity';
+import type { PlaceRatingsRepository } from '../../../src/db/repositories/PlaceRatings.repository';
+import { AssignmentParticipants } from '../../../src/db/entities/AssignmentParticipants.entity';
+import type { AssignmentParticipantsRepository } from '../../../src/db/repositories/AssignmentParticipants.repository';
+import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service';
+
+const testDb = createSnapshotTestDb();
+let t: TestOrm;
+let svc: QueryHelpersService;
+
+beforeAll(async () => {
+ t = await createTestOrm(testDb);
+ const tags: TagsRepository = t.repo(Tags);
+ const placeRatings: PlaceRatingsRepository = t.repo(PlaceRatings);
+ const assignmentParticipants: AssignmentParticipantsRepository = t.repo(AssignmentParticipants);
+ svc = new QueryHelpersService(tags, placeRatings, assignmentParticipants);
+});
+beforeEach(() => { resetTestDb(testDb); t.clear(); });
+afterAll(async () => { await t.close(); testDb.close(); });
+
+function attachTag(tagId: number, placeId: number): void {
+ testDb.prepare('INSERT INTO place_tags (tag_id, place_id) VALUES (?, ?)').run(tagId, placeId);
+}
+
+function rate(placeId: number, userId: number, rating: number): void {
+ testDb.prepare('INSERT INTO place_ratings (place_id, user_id, rating) VALUES (?, ?, ?)').run(placeId, userId, rating);
+}
+
+function addParticipant(assignmentId: number, userId: number): void {
+ testDb.prepare('INSERT INTO assignment_participants (assignment_id, user_id) VALUES (?, ?)').run(assignmentId, userId);
+}
+
+describe('QueryHelpersService.loadTagsByPlaceIds', () => {
+ it('QH-001: an empty placeIds array yields an empty Record', async () => {
+ expect(await svc.loadTagsByPlaceIds([])).toEqual({});
+ });
+
+ it('QH-002: indexes tags by place id, non-compact carries every column but the join key', async () => {
+ const { user } = createUser(testDb);
+ const trip = createTrip(testDb, user.id);
+ const place = createPlace(testDb, trip.id);
+ const tag = createTag(testDb, user.id, { name: 'Beach', color: '#ff0000' });
+ attachTag(tag.id, place.id);
+
+ const byPlace = await svc.loadTagsByPlaceIds([place.id]);
+ expect(byPlace[place.id]).toHaveLength(1);
+ expect(byPlace[place.id][0]).toMatchObject({ id: tag.id, user_id: user.id, name: 'Beach', color: '#ff0000' });
+ expect(byPlace[place.id][0]).not.toHaveProperty('place_id');
+ });
+
+ it('QH-003: compact drops user_id', async () => {
+ const { user } = createUser(testDb);
+ const trip = createTrip(testDb, user.id);
+ const place = createPlace(testDb, trip.id);
+ const tag = createTag(testDb, user.id, { name: 'Compact' });
+ attachTag(tag.id, place.id);
+
+ const byPlace = await svc.loadTagsByPlaceIds([place.id], { compact: true });
+ expect(byPlace[place.id][0]).toEqual({ id: tag.id, name: 'Compact', color: tag.color, created_at: expect.any(String) });
+ });
+
+ it('QH-004: batches across several places, grouping correctly, a place with no tags gets no key', async () => {
+ const { user } = createUser(testDb);
+ const trip = createTrip(testDb, user.id);
+ const placeA = createPlace(testDb, trip.id, { name: 'A' });
+ const placeB = createPlace(testDb, trip.id, { name: 'B' });
+ const placeC = createPlace(testDb, trip.id, { name: 'C' });
+ const tag = createTag(testDb, user.id);
+ attachTag(tag.id, placeA.id);
+ attachTag(tag.id, placeB.id);
+
+ const byPlace = await svc.loadTagsByPlaceIds([placeA.id, placeB.id, placeC.id]);
+ expect(Object.keys(byPlace).map(Number).sort()).toEqual([placeA.id, placeB.id].sort());
+ expect(byPlace[placeC.id]).toBeUndefined();
+ });
+
+ it('QH-004b: a second tag on the same place appends to the existing array instead of resetting it', async () => {
+ const { user } = createUser(testDb);
+ const trip = createTrip(testDb, user.id);
+ const place = createPlace(testDb, trip.id);
+ const tagA = createTag(testDb, user.id, { name: 'First' });
+ const tagB = createTag(testDb, user.id, { name: 'Second' });
+ attachTag(tagA.id, place.id);
+ attachTag(tagB.id, place.id);
+
+ const byPlace = await svc.loadTagsByPlaceIds([place.id]);
+ expect(byPlace[place.id].map((t) => t.name).sort()).toEqual(['First', 'Second']);
+ });
+});
+
+describe('QueryHelpersService.loadRatingsByPlaceIds', () => {
+ it('QH-005: an empty placeIds array yields an empty Record', async () => {
+ expect(await svc.loadRatingsByPlaceIds([])).toEqual({});
+ });
+
+ it('QH-006: indexes ratings by place id with the voter\'s username/avatar', async () => {
+ const { user: owner } = createUser(testDb);
+ const { user: voter } = createUser(testDb, { username: 'rater1' });
+ const trip = createTrip(testDb, owner.id);
+ const place = createPlace(testDb, trip.id);
+ rate(place.id, voter.id, 4);
+
+ const byPlace = await svc.loadRatingsByPlaceIds([place.id]);
+ expect(byPlace[place.id]).toEqual([{ user_id: voter.id, username: 'rater1', avatar: null, rating: 4 }]);
+ });
+
+ it('QH-007: a place with no ratings gets no key', async () => {
+ const { user } = createUser(testDb);
+ const trip = createTrip(testDb, user.id);
+ const place = createPlace(testDb, trip.id);
+ expect(await svc.loadRatingsByPlaceIds([place.id])).toEqual({});
+ });
+
+ it('QH-007b: a second rating on the same place appends to the existing array instead of resetting it', async () => {
+ const { user: owner } = createUser(testDb);
+ const { user: voterA } = createUser(testDb, { username: 'voter-a' });
+ const { user: voterB } = createUser(testDb, { username: 'voter-b' });
+ const trip = createTrip(testDb, owner.id);
+ const place = createPlace(testDb, trip.id);
+ rate(place.id, voterA.id, 5);
+ rate(place.id, voterB.id, 2);
+
+ const byPlace = await svc.loadRatingsByPlaceIds([place.id]);
+ expect(byPlace[place.id].map((r) => r.username).sort()).toEqual(['voter-a', 'voter-b']);
+ });
+});
+
+describe('QueryHelpersService.loadParticipantsByAssignmentIds', () => {
+ it('QH-008: an empty assignmentIds array yields an empty Record', async () => {
+ expect(await svc.loadParticipantsByAssignmentIds([])).toEqual({});
+ });
+
+ it('QH-009: indexes participants by assignment id, with NO COALESCE(display_name, username) — raw username only', async () => {
+ const { user: owner } = createUser(testDb);
+ const { user: participant } = createUser(testDb, { username: 'raw-username' });
+ testDb.prepare('UPDATE users SET display_name = ? WHERE id = ?').run('Ignored Display Name', participant.id);
+ const trip = createTrip(testDb, owner.id);
+ const day = createDay(testDb, trip.id);
+ const place = createPlace(testDb, trip.id);
+ const assignment = createDayAssignment(testDb, day.id, place.id);
+ addParticipant(assignment.id, participant.id);
+
+ const byAssignment = await svc.loadParticipantsByAssignmentIds([assignment.id]);
+ expect(byAssignment[assignment.id]).toEqual([{ user_id: participant.id, username: 'raw-username', avatar: null }]);
+ });
+
+ it('QH-010: an assignment with no participants gets no key', async () => {
+ const { user: owner } = createUser(testDb);
+ const trip = createTrip(testDb, owner.id);
+ const day = createDay(testDb, trip.id);
+ const place = createPlace(testDb, trip.id);
+ const assignment = createDayAssignment(testDb, day.id, place.id);
+ expect(await svc.loadParticipantsByAssignmentIds([assignment.id])).toEqual({});
+ });
+
+ it('QH-010b: a second participant on the same assignment appends to the existing array instead of resetting it', async () => {
+ const { user: owner } = createUser(testDb);
+ const { user: p1 } = createUser(testDb, { username: 'p1' });
+ const { user: p2 } = createUser(testDb, { username: 'p2' });
+ const trip = createTrip(testDb, owner.id);
+ const day = createDay(testDb, trip.id);
+ const place = createPlace(testDb, trip.id);
+ const assignment = createDayAssignment(testDb, day.id, place.id);
+ addParticipant(assignment.id, p1.id);
+ addParticipant(assignment.id, p2.id);
+
+ const byAssignment = await svc.loadParticipantsByAssignmentIds([assignment.id]);
+ expect(byAssignment[assignment.id].map((p) => p.username).sort()).toEqual(['p1', 'p2']);
+ });
+});
diff --git a/server/tests/unit/nest/share.service.test.ts b/server/tests/unit/nest/share.service.test.ts
index 94d76b061..a2a681227 100644
--- a/server/tests/unit/nest/share.service.test.ts
+++ b/server/tests/unit/nest/share.service.test.ts
@@ -60,7 +60,7 @@ import { ShareService, publicReservationMetadata } from '../../../src/nest/share
import { SettingsService } from '../../../src/nest/settings/settings.service';
import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service';
import type { User } from '../../../src/types';
-import { sharedTestOrm, createTestUnitOfWork, createTestAppSettingsRepo, createTestSettingsRepo } from '../../helpers/test-uow';
+import { sharedTestOrm, createTestUnitOfWork, createTestAppSettingsRepo, createTestSettingsRepo, createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo } from '../../helpers/test-uow';
import type { TestOrm } from '../../helpers/test-orm';
let svc: ShareService;
@@ -78,7 +78,7 @@ beforeAll(async () => {
dbs,
new SettingsService(await createTestUnitOfWork(testDb), await createTestAppSettingsRepo(testDb), await createTestSettingsRepo(testDb)),
permissionsStub,
- new QueryHelpersService(dbs),
+ new QueryHelpersService(await createTestTagsRepo(dbs.connection), await createTestPlaceRatingsRepo(dbs.connection), await createTestAssignmentParticipantsRepo(dbs.connection)),
photoCacheStub,
await createTestUnitOfWork(testDb),
);
diff --git a/server/tests/unit/nest/trip-invite.service.test.ts b/server/tests/unit/nest/trip-invite.service.test.ts
index cc2494db9..146830b06 100644
--- a/server/tests/unit/nest/trip-invite.service.test.ts
+++ b/server/tests/unit/nest/trip-invite.service.test.ts
@@ -40,7 +40,7 @@ import { DatabaseService } from '../../../src/nest/database/database.service';
import { PermissionsService } from '../../../src/nest/permissions/permissions.service';
import { TripMembershipService } from '../../../src/nest/trip-membership/trip-membership.service';
import { TripInviteService } from '../../../src/nest/trip-invite/trip-invite.service';
-import { createTestUnitOfWork, createTestAppSettingsRepo } from '../../helpers/test-uow';
+import { createTestUnitOfWork, createTestAppSettingsRepo, createTestTripsRepo, createTestTripMembersRepo } from '../../helpers/test-uow';
// One DatabaseService over the shared in-memory handle, so every collaborator
// reads and writes the same rows.
@@ -48,7 +48,7 @@ const dbs = new DatabaseService(testDb);
let svc: TripInviteService;
beforeAll(async () => {
const uow = await createTestUnitOfWork(dbs.connection);
- svc = new TripInviteService(dbs, new PermissionsService(await createTestAppSettingsRepo(dbs.connection), uow), new TripMembershipService(dbs), uow);
+ svc = new TripInviteService(dbs, new PermissionsService(await createTestAppSettingsRepo(dbs.connection), uow), new TripMembershipService(await createTestTripsRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection)), uow);
});
beforeAll(() => { createTables(testDb); runMigrations(testDb); });
diff --git a/server/tests/unit/nest/trip-membership/trip-membership.service.test.ts b/server/tests/unit/nest/trip-membership/trip-membership.service.test.ts
index 5c10e3d1a..0bc27f02a 100644
--- a/server/tests/unit/nest/trip-membership/trip-membership.service.test.ts
+++ b/server/tests/unit/nest/trip-membership/trip-membership.service.test.ts
@@ -2,30 +2,39 @@
* Unit tests for TripMembershipService.joinTripAsMember — TRIP-JOIN-001..004.
* The shared add-by-id helper behind trip invite links (#1143) and trip-bound
* admin invites (#1402): idempotent, owner-safe, missing-trip-safe.
+ *
+ * Rebuilt on real rows through `TripsRepository`/`TripMembersRepository`
+ * (Plan 3c Task 1): the legacy version constructed `TripMembershipService`
+ * directly on a `DatabaseService(testDb)` over hand-run legacy DDL
+ * (`createTables`/`runMigrations`); this now uses `createSnapshotTestDb()` +
+ * `createTestOrm()`, the same harness every other converted repository test
+ * uses.
*/
-import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest';
-
-const { testDb, dbMock } = vi.hoisted(() => {
- const Database = require('better-sqlite3');
- const db = new Database(':memory:');
- db.exec('PRAGMA foreign_keys = ON');
- return { testDb: db, dbMock: { db } };
-});
-vi.mock('../../../../src/db/database', () => dbMock);
-
-import { createTables } from '../../../../src/db/schema';
-import { runMigrations } from '../../../../src/db/migrations';
+import { describe, it, expect, beforeAll, beforeEach, afterAll } from 'vitest';
+import { createSnapshotTestDb } from '../../../helpers/db-mock';
import { resetTestDb } from '../../../helpers/test-db';
+import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
import { createUser, createTrip } from '../../../helpers/factories';
-import { DatabaseService } from '../../../../src/nest/database/database.service';
+import { Trips } from '../../../../src/db/entities/Trips.entity';
+import type { TripsRepository } from '../../../../src/db/repositories/Trips.repository';
+import { TripMembers } from '../../../../src/db/entities/TripMembers.entity';
+import type { TripMembersRepository } from '../../../../src/db/repositories/TripMembers.repository';
import { TripMembershipService } from '../../../../src/nest/trip-membership/trip-membership.service';
-const joinTripAsMember = (tripId: number, userId: number, invitedBy: number | null) =>
- new TripMembershipService(new DatabaseService(testDb)).joinTripAsMember(tripId, userId, invitedBy);
+const testDb = createSnapshotTestDb();
+let t: TestOrm;
+let trips: TripsRepository;
+let tripMembers: TripMembersRepository;
+let svc: TripMembershipService;
-beforeAll(() => { createTables(testDb); runMigrations(testDb); });
-beforeEach(() => resetTestDb(testDb));
-afterAll(() => testDb.close());
+beforeAll(async () => {
+ t = await createTestOrm(testDb);
+ trips = t.repo(Trips);
+ tripMembers = t.repo(TripMembers);
+ svc = new TripMembershipService(trips, tripMembers);
+});
+beforeEach(() => { resetTestDb(testDb); t.clear(); });
+afterAll(async () => { await t.close(); testDb.close(); });
function memberRow(tripId: number, userId: number) {
return testDb.prepare('SELECT * FROM trip_members WHERE trip_id = ? AND user_id = ?').get(tripId, userId);
@@ -37,7 +46,7 @@ describe('joinTripAsMember', () => {
const { user: joiner } = createUser(testDb);
const trip = createTrip(testDb, owner.id);
- const r = await joinTripAsMember(trip.id, joiner.id, null);
+ const r = await svc.joinTripAsMember(trip.id, joiner.id, null);
expect(r).toEqual({ joined: true, tripId: trip.id });
expect(memberRow(trip.id, joiner.id)).toBeTruthy();
});
@@ -46,7 +55,7 @@ describe('joinTripAsMember', () => {
const { user: owner } = createUser(testDb);
const trip = createTrip(testDb, owner.id);
- const r = await joinTripAsMember(trip.id, owner.id, null);
+ const r = await svc.joinTripAsMember(trip.id, owner.id, null);
expect(r.joined).toBe(false);
expect(memberRow(trip.id, owner.id)).toBeUndefined();
});
@@ -56,29 +65,61 @@ describe('joinTripAsMember', () => {
const { user: joiner } = createUser(testDb);
const trip = createTrip(testDb, owner.id);
- expect((await joinTripAsMember(trip.id, joiner.id, owner.id)).joined).toBe(true);
- expect((await joinTripAsMember(trip.id, joiner.id, owner.id)).joined).toBe(false);
+ expect((await svc.joinTripAsMember(trip.id, joiner.id, owner.id)).joined).toBe(true);
+ expect((await svc.joinTripAsMember(trip.id, joiner.id, owner.id)).joined).toBe(false);
const count = testDb.prepare('SELECT COUNT(*) as n FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, joiner.id) as { n: number };
expect(count.n).toBe(1);
});
it('TRIP-JOIN-004: no-ops for a missing trip', async () => {
const { user: joiner } = createUser(testDb);
- const r = await joinTripAsMember(999999, joiner.id, null);
+ const r = await svc.joinTripAsMember(999999, joiner.id, null);
expect(r.joined).toBe(false);
});
+
+ // Plan 3c inventory §18.4: TB4 → TB5 → TB6 is a non-transactional
+ // check-then-act sequence, unchanged by this conversion. Two concurrent
+ // joins can both pass the "already a member" check — this pins TODAY'S
+ // outcome (a duplicate-membership row survives the trip_members table's
+ // own UNIQUE(trip_id, user_id) constraint check only by luck of timing;
+ // what the test actually proves is that the service itself does not
+ // serialize the two calls, not that the DB will always accept both).
+ it('TRIP-JOIN-005 (concurrency, §18.4): two concurrent joins for the same (trip, user) race the check-then-act window — the service performs no locking/serialization of its own', async () => {
+ const { user: owner } = createUser(testDb);
+ const { user: joiner } = createUser(testDb);
+ const trip = createTrip(testDb, owner.id);
+
+ const results = await Promise.allSettled([
+ svc.joinTripAsMember(trip.id, joiner.id, null),
+ svc.joinTripAsMember(trip.id, joiner.id, null),
+ ]);
+ // Today's actual outcome (run under real concurrency, not assumed): the
+ // method performs no locking/serialization of its own, so both calls can
+ // pass the TB5 "already a member" check before either TB6 INSERT lands —
+ // the loser then hits the table's own UNIQUE(trip_id, user_id) constraint
+ // and REJECTS, it does not swallow the error into `{joined: false}`. At
+ // least one call always fulfills (the winner); pinning "both always
+ // fulfill" was wrong (verified: failed under the full-suite coverage
+ // run's real scheduling) — the row-count assertion below is the actual
+ // safety net, not a specific promise-settlement shape.
+ const fulfilled = results.filter((r) => r.status === 'fulfilled');
+ expect(fulfilled.length).toBeGreaterThanOrEqual(1);
+ // Exactly one membership row exists afterwards either way — the UNIQUE
+ // constraint on (trip_id, user_id) is the actual safety net today, not
+ // application-level locking.
+ const count = testDb.prepare('SELECT COUNT(*) as n FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, joiner.id) as { n: number };
+ expect(count.n).toBe(1);
+ });
});
// The leaf reads that replaced trips.bridge for BudgetMcp/CostsRpc — see the
// service docblock for why they live on this dependency-free module.
describe('leaf membership reads', () => {
- const svc = () => new TripMembershipService(new DatabaseService(testDb));
-
it('TRIP-READ-001: getOwnerId answers the owner and null for a missing trip', async () => {
const { user: owner } = createUser(testDb);
const trip = createTrip(testDb, owner.id);
- expect(await svc().getOwnerId(trip.id)).toBe(owner.id);
- expect(await svc().getOwnerId(999999)).toBeNull();
+ expect(await svc.getOwnerId(trip.id)).toBe(owner.id);
+ expect(await svc.getOwnerId(999999)).toBeNull();
});
it('TRIP-READ-002: listMemberUserIds excludes the owner and follows added_at order', async () => {
@@ -88,8 +129,8 @@ describe('leaf membership reads', () => {
const trip = createTrip(testDb, owner.id);
testDb.prepare("INSERT INTO trip_members (trip_id, user_id, added_at) VALUES (?, ?, '2026-01-02')").run(trip.id, m2.id);
testDb.prepare("INSERT INTO trip_members (trip_id, user_id, added_at) VALUES (?, ?, '2026-01-01')").run(trip.id, m1.id);
- expect(await svc().listMemberUserIds(trip.id)).toEqual([m1.id, m2.id]);
- expect(await svc().listMemberUserIds(999999)).toEqual([]);
+ expect(await svc.listMemberUserIds(trip.id)).toEqual([m1.id, m2.id]);
+ expect(await svc.listMemberUserIds(999999)).toEqual([]);
});
it('TRIP-READ-003: listAccessibleTripIds unions owned and member trips, newest first', async () => {
@@ -102,7 +143,7 @@ describe('leaf membership reads', () => {
// Distinct created_at so the ORDER BY is actually asserted, not assumed.
testDb.prepare("UPDATE trips SET created_at = '2026-01-01' WHERE id = ?").run(owned.id);
testDb.prepare("UPDATE trips SET created_at = '2026-01-02' WHERE id = ?").run(memberOf.id);
- const ids = await svc().listAccessibleTripIds(user.id);
+ const ids = await svc.listAccessibleTripIds(user.id);
expect(ids).toEqual([memberOf.id, owned.id]);
expect(ids).not.toContain(foreign.id);
});
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 21c92faef..beb21c48d 100644
--- a/server/tests/unit/nest/trip-read-model.service.test.ts
+++ b/server/tests/unit/nest/trip-read-model.service.test.ts
@@ -81,6 +81,8 @@ import { TrekPhotosRepository } from '../../../src/nest/photos/trek-photos.repos
import {
createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm,
createTestDaysRepo, createTestDayAssignmentsRepo, createTestDayNotesRepo, createTestTripsRepo,
+ createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo,
+ createTestGooglePlacePhotoMetaRepo, createTestPlacesRepo,
} from '../../helpers/test-uow';
// Real sibling services over the same in-memory DB — the aggregation runs the
@@ -88,10 +90,11 @@ import {
// up here instead of being papered over by a stub.
const dbs = () => new DatabaseService(testDb);
-
// One shared cache instance (the PlacePhotoCacheService rule): the in-flight dedup in
// PlacePhotoCacheService only works while both consumers hold the same object.
-const photoCache = new PlacePhotoCacheService(dbs(), makeStorageFixture('photos/google/').storage);
+// Plan 3c Task 1: built inside the async `beforeAll` below now — the
+// constructor needs two repositories, resolved through `createTestOrm`.
+let photoCache: PlacePhotoCacheService;
let accommodationsSvc: Awaited>;
beforeAll(async () => {
@@ -102,12 +105,18 @@ let daysSvc: DaysService;
let placesSvc: PlacesService;
let membersSvc: TripMembersService;
beforeAll(async () => {
+ photoCache = new PlacePhotoCacheService(
+ dbs(),
+ makeStorageFixture('photos/google/').storage,
+ await createTestGooglePlacePhotoMetaRepo(dbs().connection),
+ await createTestPlacesRepo(dbs().connection),
+ );
budgetSvc = new BudgetService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(dbs().connection));
daysSvc = new DaysService(
dbs(),
new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)),
new RealtimeService(),
- new QueryHelpersService(dbs()),
+ new QueryHelpersService(await createTestTagsRepo(dbs().connection), await createTestPlaceRatingsRepo(dbs().connection), await createTestAssignmentParticipantsRepo(dbs().connection)),
await createTestUnitOfWork(dbs().connection),
await createTestDaysRepo(dbs().connection),
await createTestDayAssignmentsRepo(dbs().connection),
@@ -116,7 +125,7 @@ beforeAll(async () => {
);
placesSvc = new PlacesService(
dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(),
- new MapsService(dbs(), photoCache, await createTestAppSettingsRepo(dbs().connection), await createTestUsersRepo(dbs().connection)), new QueryHelpersService(dbs()),
+ 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 TrekPhotosRepository(dbs()), await createTestUnitOfWork(dbs().connection)),
makeStorageFixture('').storage,
diff --git a/server/tests/unit/nest/trips.service.test.ts b/server/tests/unit/nest/trips.service.test.ts
index e9f9c1efd..4ca156d05 100644
--- a/server/tests/unit/nest/trips.service.test.ts
+++ b/server/tests/unit/nest/trips.service.test.ts
@@ -91,6 +91,8 @@ import { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.se
import {
createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm,
createTestDaysRepo, createTestDayAssignmentsRepo, createTestDayNotesRepo, createTestTripsRepo,
+ createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo,
+ createTestGooglePlacePhotoMetaRepo, createTestPlacesRepo,
} from '../../helpers/test-uow';
// Real sibling services over the same in-memory DB — updateTrip's date-shift
@@ -114,7 +116,9 @@ const dbs = () => new DatabaseService(testDb, dbsEm);
// missing collaborator would look like a pass while swallowing a TypeError.
// One PlacePhotoCacheService for both PlacesService and MapsService, matching
// production, where the in-flight dedup only works on a shared instance.
-const photoCache = new PlacePhotoCacheService(dbs(), makeStorageFixture('photos/google/').storage);
+// Plan 3c Task 1: built inside the async `beforeAll` below now — the
+// constructor needs two repositories, resolved through `createTestOrm`.
+let photoCache: PlacePhotoCacheService;
const coversFx = makeStorageFixture('covers/');
@@ -135,12 +139,18 @@ let membersSvc: TripMembersService;
let readModelSvc: TripReadModelService;
beforeAll(async () => {
dbsEm = (await sharedTestOrm(testDb)).em;
+ photoCache = new PlacePhotoCacheService(
+ dbs(),
+ makeStorageFixture('photos/google/').storage,
+ await createTestGooglePlacePhotoMetaRepo(dbs().connection),
+ await createTestPlacesRepo(dbs().connection),
+ );
budgetSvc = new BudgetService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(dbs().connection));
daysSvc = new DaysService(
dbs(),
new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)),
new RealtimeService(),
- new QueryHelpersService(dbs()),
+ new QueryHelpersService(await createTestTagsRepo(dbs().connection), await createTestPlaceRatingsRepo(dbs().connection), await createTestAssignmentParticipantsRepo(dbs().connection)),
await createTestUnitOfWork(dbs().connection),
await createTestDaysRepo(dbs().connection),
await createTestDayAssignmentsRepo(dbs().connection),
@@ -152,7 +162,7 @@ beforeAll(async () => {
new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)),
new RealtimeService(),
new MapsService(dbs(), photoCache, await createTestAppSettingsRepo(dbs().connection), await createTestUsersRepo(dbs().connection)),
- new QueryHelpersService(dbs()),
+ 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(), coversFx.storage),
photoCache,
new JourneyDomainService(dbs(), new RealtimeService(), new TrekPhotosRepository(dbs()), await createTestUnitOfWork(dbs().connection)),
diff --git a/server/tests/unit/plugins/operator-egress.test.ts b/server/tests/unit/plugins/operator-egress.test.ts
index 3e8627470..a4a5954de 100644
--- a/server/tests/unit/plugins/operator-egress.test.ts
+++ b/server/tests/unit/plugins/operator-egress.test.ts
@@ -17,7 +17,7 @@ import { describe, it, expect, vi, beforeAll, beforeEach } from 'vitest';
const { testDb, dbMock } = vi.hoisted(() => {
const Database = require('better-sqlite3');
const db = new Database(':memory:');
- return { testDb: db, dbMock: { db, closeDb: () => {}, reinitialize: () => {}, canAccessTrip: () => null } };
+ return { testDb: db, dbMock: { db, closeDb: () => {}, reinitialize: () => {}, canAccessTrip: async () => null } };
});
vi.mock('../../../src/db/database', () => dbMock);
import { db as dbConn } from '../../../src/db/database';
diff --git a/server/tests/unit/plugins/plugin-instance-settings.test.ts b/server/tests/unit/plugins/plugin-instance-settings.test.ts
index b8c8f900e..7361c3b3b 100644
--- a/server/tests/unit/plugins/plugin-instance-settings.test.ts
+++ b/server/tests/unit/plugins/plugin-instance-settings.test.ts
@@ -16,7 +16,7 @@ import type { Request } from 'express';
const { testDb, dbMock } = vi.hoisted(() => {
const Database = require('better-sqlite3');
const db = new Database(':memory:');
- return { testDb: db, dbMock: { db, closeDb: () => {}, reinitialize: () => {}, canAccessTrip: () => null } };
+ return { testDb: db, dbMock: { db, closeDb: () => {}, reinitialize: () => {}, canAccessTrip: async () => null } };
});
vi.mock('../../../src/db/database', () => dbMock);
import { db as dbConn } from '../../../src/db/database';
diff --git a/server/tests/unit/plugins/rpc-host-inv-strip.test.ts b/server/tests/unit/plugins/rpc-host-inv-strip.test.ts
index a904b54c2..468af158e 100644
--- a/server/tests/unit/plugins/rpc-host-inv-strip.test.ts
+++ b/server/tests/unit/plugins/rpc-host-inv-strip.test.ts
@@ -35,7 +35,7 @@ const dbRegistry = () => {
/** trips.getById lives on the decorators now, so the audit cases bind it through them. */
const tripsRegistry = () => {
const db = {
- canAccessTrip: (tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined),
+ canAccessTrip: async (tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined),
prepare: () => ({ get: () => ({ id: 1, title: 'Japan' }), all: () => [] }),
} as unknown as DatabaseService;
const guards = new PluginGuards(db, {} as never, {} as never);
diff --git a/server/tests/unit/plugins/settings-isolation.test.ts b/server/tests/unit/plugins/settings-isolation.test.ts
index f2f248801..2473fcaca 100644
--- a/server/tests/unit/plugins/settings-isolation.test.ts
+++ b/server/tests/unit/plugins/settings-isolation.test.ts
@@ -14,7 +14,7 @@ import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vites
const { testDb, dbMock } = vi.hoisted(() => {
const Database = require('better-sqlite3');
const db = new Database(':memory:');
- return { testDb: db, dbMock: { db, closeDb: () => {}, reinitialize: () => {}, canAccessTrip: () => null } };
+ return { testDb: db, dbMock: { db, closeDb: () => {}, reinitialize: () => {}, canAccessTrip: async () => null } };
});
vi.mock('../../../src/db/database', () => dbMock);
import { db as dbConn } from '../../../src/db/database';
diff --git a/server/tests/unit/services/conflictUpdate.test.ts b/server/tests/unit/services/conflictUpdate.test.ts
index 49a62ff1a..5e6db3487 100644
--- a/server/tests/unit/services/conflictUpdate.test.ts
+++ b/server/tests/unit/services/conflictUpdate.test.ts
@@ -21,8 +21,8 @@ const { testDb, dbMock } = vi.hoisted(() => {
closeDb: () => {},
reinitialize: () => {},
getPlaceWithTags,
- canAccessTrip: () => null,
- isOwner: () => false,
+ canAccessTrip: async () => null,
+ isOwner: async () => false,
};
return { testDb: db, dbMock: mock };
});
@@ -54,19 +54,36 @@ import { TrekPhotosRepository } from '../../../src/nest/photos/trek-photos.repos
import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service';
import { notificationsStub } from '../../helpers/notifications';
import { makeStorageFixture } from '../../helpers/storage-fixture';
-import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm } from '../../helpers/test-uow';
+import {
+ createTestUnitOfWork,
+ createTestAppSettingsRepo,
+ createTestUsersRepo,
+ createTestTagsRepo,
+ createTestPlaceRatingsRepo,
+ createTestAssignmentParticipantsRepo,
+ createTestGooglePlacePhotoMetaRepo,
+ createTestPlacesRepo,
+ sharedTestOrm,
+} from '../../helpers/test-uow';
const dbs = new DatabaseService(testDb);
const realtime = new RealtimeService();
const runtimeEnv = new RuntimeEnvService();
-// One cache instance shared by maps and places, the way the container wires it:
-// the service's stampede guard only works if there is exactly one of them.
-const photoCache = new PlacePhotoCacheService(dbs, makeStorageFixture('photos/google/').storage);
-
let packing: PackingService;
let places: PlacesService;
+let photoCache: PlacePhotoCacheService;
beforeAll(async () => {
+ // One cache instance shared by maps and places, the way the container
+ // wires it: the service's stampede guard only works if there is exactly
+ // one of them. Plan 3c Task 1: PlacePhotoCacheService now takes the two
+ // repositories too.
+ photoCache = new PlacePhotoCacheService(
+ dbs,
+ makeStorageFixture('photos/google/').storage,
+ await createTestGooglePlacePhotoMetaRepo(dbs.connection),
+ await createTestPlacesRepo(dbs.connection),
+ );
// Plan 3c Task 0b: `dbs` is constructed at module load, before any
// `beforeAll` can resolve a real `EntityManager` — the four
// repository-backed methods are spied directly on this instance instead,
@@ -82,7 +99,7 @@ beforeAll(async () => {
new PermissionsService(await createTestAppSettingsRepo(dbs.connection), await createTestUnitOfWork(dbs.connection)),
realtime,
new MapsService(dbs, photoCache, await createTestAppSettingsRepo(dbs.connection), await createTestUsersRepo(dbs.connection)),
- new QueryHelpersService(dbs),
+ new QueryHelpersService(await createTestTagsRepo(dbs.connection), await createTestPlaceRatingsRepo(dbs.connection), await createTestAssignmentParticipantsRepo(dbs.connection)),
new UnsplashService(await createTestAppSettingsRepo(dbs.connection), await createTestUsersRepo(dbs.connection), runtimeEnv, makeStorageFixture('').storage),
photoCache,
new JourneyDomainService(dbs, realtime, new TrekPhotosRepository(dbs), await createTestUnitOfWork(dbs.connection)),
diff --git a/server/tests/unit/services/trekPhotoMedia.test.ts b/server/tests/unit/services/trekPhotoMedia.test.ts
index 984333dd6..7a6e948f9 100644
--- a/server/tests/unit/services/trekPhotoMedia.test.ts
+++ b/server/tests/unit/services/trekPhotoMedia.test.ts
@@ -9,7 +9,7 @@ const { testDb, dbMock } = vi.hoisted(() => {
const db = new Database(':memory:');
// FKs off: this suite only checks media_type persistence, not owner/user integrity.
db.exec('PRAGMA foreign_keys = OFF');
- const mock = { db, closeDb: () => {}, reinitialize: () => {}, getPlaceWithTags: () => null, canAccessTrip: () => null, isOwner: () => false };
+ const mock = { db, closeDb: () => {}, reinitialize: () => {}, getPlaceWithTags: async () => null, canAccessTrip: async () => null, isOwner: async () => false };
return { testDb: db, dbMock: mock };
});