refactor(places): import paths through repositories

This commit is contained in:
jubnl
2026-09-23 08:53:54 +02:00
parent 955809f15f
commit f19face405
13 changed files with 644 additions and 77 deletions
@@ -21,6 +21,18 @@ export class CategoriesRepository extends TrekRepository<Categories> {
return categories.map((category) => toRow(category) as CategoryRow);
}
/**
* PL33 (`PlacesService.importKmlPlaces`) — `SELECT id, name FROM
* categories`, the id/name-only projection the KML importer folds into
* `buildCategoryNameLookup` to resolve a folder name to a category id. No
* `ORDER BY` in the legacy statement — none added here.
*/
async listIdName(): Promise<{ id: number; name: string }[]> {
return this.qb('c')
.select(['c.id', 'c.name'])
.execute<{ id: number; name: string }[]>('all', false);
}
/**
* `SELECT * FROM categories WHERE id = ?` — no owner filter. `user_id`
* being nullable makes a row *look* ownable, but this legacy statement
+122 -1
View File
@@ -1,6 +1,6 @@
import type { Places } from '../entities/Places.entity';
import type { Tags } from '../entities/Tags.entity';
import { absDifference, columnRef, currentTimestamp, lowerTrim } from '../dialect/sql-functions';
import { absDifference, coalesceParam, columnRef, currentTimestamp, lowerTrim } from '../dialect/sql-functions';
import { type AssertRowKeys } from './_shared/rows';
import { TrekRepository } from './_shared/trek-repository';
@@ -594,4 +594,125 @@ export class PlacesRepository extends TrekRepository<Places> {
return qb.execute<PlaceWithCategoryRow[]>('all', false);
}
// ---------------------------------------------------------------------------
// Plan 3c Task 5 (`PlacesService` import paths) — every method below is
// additive: `insertPlace`/`updatePlace`/`findWithTagsAndRatings`/the
// `findDuplicateBy*` trio above are Task 4's, unchanged and reused verbatim
// by the four importers (GPX/KML/Google/Naver) this task converts.
// ---------------------------------------------------------------------------
/**
* PL36 (`colorizeImportedTracks`'s read half) — `SELECT DISTINCT
* route_color AS c FROM places WHERE trip_id = ? AND route_color IS NOT
* NULL`. `DISTINCT` via `.select([...], true)` (the same shape
* `listForTrip`/PL3 uses for its own `SELECT DISTINCT`), not `$ne: null`
* folded into a single-object filter, so the emitted `WHERE` clause reads
* in the legacy's own `trip_id = ? AND route_color IS NOT NULL` order.
*
* Called and its result acted on inside the SAME `uow.transactional(...)`
* block `setRouteColor` below writes in (the service's own PL36+PL37
* pairing) — read and write share one transaction so two concurrent
* imports can never both observe the same set of free colours.
*/
async distinctRouteColors(trip_id: string | number): Promise<string[]> {
const rows = await this.qb('p')
.select(['p.route_color as c'], true)
.where('p.trip_id = ? AND p.route_color IS NOT NULL', [trip_id])
.execute<{ c: string }[]>('all', false);
return rows.map((r) => r.c);
}
/**
* PL37 (`colorizeImportedTracks`'s write half) — `UPDATE places SET
* route_color = ? WHERE id = ?`. No `updated_at` stamp — the legacy
* statement never wrote one for this column (#776's own deliberate
* omission, restated in `PlacesService`'s class docstring), and this
* method reproduces exactly that, not the "every write stamps
* `updated_at`" shape `updatePlace`/`backfillFtid`/`fillIfEmpty` below
* follow.
*/
async setRouteColor(id: number, route_color: string): Promise<void> {
await this.nativeUpdate({ id }, { route_color });
}
/**
* PL39 (`storeGooglePlaces`'s dedup-backfill half) — `UPDATE places SET
* google_ftid = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?`, unscoped
* by `trip_id` (matching the legacy statement exactly — the candidate id
* was already resolved against this trip by the `findDuplicatePlace` call
* immediately before it).
*/
async backfillFtid(id: number, google_ftid: string): Promise<void> {
const platform = this.getEntityManager().getPlatform();
await this.nativeUpdate({ id }, { google_ftid, updated_at: currentTimestamp(platform) });
}
/**
* PL43/PL44/PL46 (`enrichOne`'s two `UPDATE`s and
* `backfillMissingAddresses`'s one) — three legacy statements, one shape:
* `UPDATE places SET <col> = COALESCE(<col>, ?), ..., updated_at =
* CURRENT_TIMESTAMP WHERE id = ? AND trip_id = ?`. One `nativeUpdate` per
* CALL, built from whichever of the six columns the caller passes
* (`fields`'s keys are all optional — a key left OFF `fields` entirely is
* not touched at all, not even COALESCE'd against itself); every key
* that IS present renders `COALESCE(<col>, ?)` through `coalesceParam`
* (never an inline `raw()`), so a column already holding a value is never
* overwritten and an empty one is filled from `fields`'s value.
* `updated_at` is stamped unconditionally — all three legacy statements
* stamp it, none of them conditionally.
*
* The three call shapes this serves (Task 5's "×3 shapes"):
* - PL43 (`enrichOne`'s main update) — `google_place_id`, `google_ftid`,
* `address`, `website`, `phone` all at once.
* - PL44 (`enrichOne`'s photo update) — `image_url` alone.
* - PL46 (`backfillMissingAddresses`) — `address` alone.
*/
async fillIfEmpty(id: number, trip_id: number, fields: {
google_place_id?: string | null;
google_ftid?: string | null;
address?: string | null;
website?: string | null;
phone?: string | null;
image_url?: string | null;
}): Promise<void> {
const platform = this.getEntityManager().getPlatform();
const data: {
google_place_id?: ReturnType<typeof coalesceParam>;
google_ftid?: ReturnType<typeof coalesceParam>;
address?: ReturnType<typeof coalesceParam>;
website?: ReturnType<typeof coalesceParam>;
phone?: ReturnType<typeof coalesceParam>;
image_url?: ReturnType<typeof coalesceParam>;
updated_at: ReturnType<typeof currentTimestamp>;
} = { updated_at: currentTimestamp(platform) };
if (fields.google_place_id !== undefined) data.google_place_id = coalesceParam(platform, 'google_place_id', fields.google_place_id);
if (fields.google_ftid !== undefined) data.google_ftid = coalesceParam(platform, 'google_ftid', fields.google_ftid);
if (fields.address !== undefined) data.address = coalesceParam(platform, 'address', fields.address);
if (fields.website !== undefined) data.website = coalesceParam(platform, 'website', fields.website);
if (fields.phone !== undefined) data.phone = coalesceParam(platform, 'phone', fields.phone);
if (fields.image_url !== undefined) data.image_url = coalesceParam(platform, 'image_url', fields.image_url);
await this.nativeUpdate({ id, trip: trip_id }, data);
}
// ---------------------------------------------------------------------------
// Plan 3c Task 7 (`trips.rpc.ts`) — additive (per `task-4-report.md`'s own
// note that this method "may not exist"): every column above is Task 4/5's,
// unchanged.
// ---------------------------------------------------------------------------
/**
* RP2 (`trips.rpc.ts::getPlaces`) — `SELECT * FROM places WHERE trip_id = ?
* ORDER BY created_at DESC`. The trip's place POOL has no itinerary
* position of its own (`day_id`/`order_index` live on `day_assignments`),
* so this orders by `created_at` like the REST list does — the RPC
* method's own comment (`trips.rpc.ts`).
*/
async listForTripOrdered(trip_id: number): Promise<PlaceRow[]> {
return await this.qb('p')
.select(['p.*'])
.where({ trip: trip_id })
.orderBy({ 'p.created_at': 'desc' })
.execute<PlaceRow[]>('all', false);
}
}
+4 -2
View File
@@ -6,6 +6,7 @@ import { PlaceRatings } from '../../db/entities/PlaceRatings.entity';
import { TripMembers } from '../../db/entities/TripMembers.entity';
import { DayAssignments } from '../../db/entities/DayAssignments.entity';
import { Trips } from '../../db/entities/Trips.entity';
import { Categories } from '../../db/entities/Categories.entity';
import { JourneyDomainModule } from '../journey/journey-domain.module';
import { PlacesController } from './places.controller';
import { PlacesService } from './places.service';
@@ -60,8 +61,9 @@ import { MAX_PLACE_IMAGE_SIZE } from '../common/place-image-upload';
// THIS module (not a sibling domain module's `exports`), matching
// `AssignmentsDomainModule`'s own `MikroOrmModule.forFeature(...)`
// precedent: a `forFeature` registration only reaches providers declared
// in the SAME module.
MikroOrmModule.forFeature([Places, Tags, PlaceRatings, TripMembers, DayAssignments, Trips]),
// in the SAME module. Plan 3c Task 5 adds `CategoriesRepository` for
// PL33's `importKmlPlaces` folder → category lookup.
MikroOrmModule.forFeature([Places, Tags, PlaceRatings, TripMembers, DayAssignments, Trips, Categories]),
],
controllers: [PlacesController],
providers: [PlacesService, PlacesMcp, PlacesRpc],
+170 -70
View File
@@ -38,11 +38,13 @@ import { Tags } from '../../db/entities/Tags.entity';
import { PlaceRatings } from '../../db/entities/PlaceRatings.entity';
import { TripMembers } from '../../db/entities/TripMembers.entity';
import { DayAssignments } from '../../db/entities/DayAssignments.entity';
import { Categories } from '../../db/entities/Categories.entity';
import type { PlacesRepository, PlaceWithCategoryRow } from '../../db/repositories/Places.repository';
import type { TagsRepository } from '../../db/repositories/Tags.repository';
import type { PlaceRatingsRepository } from '../../db/repositories/PlaceRatings.repository';
import type { TripMembersRepository } from '../../db/repositories/TripMembers.repository';
import type { DayAssignmentsRepository } from '../../db/repositories/DayAssignments.repository';
import type { CategoriesRepository } from '../../db/repositories/Categories.repository';
/** Rows a place delete took down with the nights booked there, for the caller to announce. */
export interface CancelledStays {
@@ -152,6 +154,7 @@ export class PlacesService {
@InjectRepository(PlaceRatings) private readonly placeRatingsRepo: PlaceRatingsRepository,
@InjectRepository(TripMembers) private readonly tripMembersRepo: TripMembersRepository,
@InjectRepository(DayAssignments) private readonly dayAssignmentsRepo: DayAssignmentsRepository,
@InjectRepository(Categories) private readonly categoriesRepo: CategoriesRepository,
) {}
async verifyTripAccess(tripId: string, userId: number) {
@@ -812,20 +815,51 @@ export class PlacesService {
if (waypoints.length === 0) return null;
const dedup = await this.buildDedupSet(tripId);
const insertStmt = this.dbs.prepare(`
INSERT INTO places (trip_id, name, description, lat, lng, transport_mode, route_geometry)
VALUES (?, ?, ?, ?, ?, 'walking', ?)
`);
const created: PlaceWithTags[] = [];
let skipped = 0;
// PL31/PL32 — one `insertPlace` call per waypoint. The legacy statement's
// narrower 7-column INSERT (trip_id, name, description, lat, lng,
// transport_mode='walking' literal, route_geometry) is reproduced as a
// typed `insertPlace` partial: the literal becomes a value, and every
// column the legacy INSERT omitted takes the same value the DB column
// default would have produced (`duration_minutes: 60`, everything else
// `null`) — parity by stored row, not by SQL text. Re-selected via
// `findWithTagsAndRatings` INSIDE the same transaction the insert ran
// in, so the read sees the still-uncommitted row.
await this.uow.transactional(async () => {
for (const wp of waypoints) {
if (isPlaceDuplicate({ name: wp.name, lat: wp.lat, lng: wp.lng }, dedup)) {
skipped++;
continue;
}
const result = insertStmt.run(tripId, wp.name, wp.description, wp.lat, wp.lng, wp.routeGeometry || null);
const place = (await this.dbs.getPlaceWithTags(Number(result.lastInsertRowid)))!;
const placeId = await this.placesRepo.insertPlace({
trip_id: Number(tripId),
name: wp.name,
description: wp.description,
lat: wp.lat,
lng: wp.lng,
address: null,
category_id: null,
price: null,
currency: null,
place_time: null,
end_time: null,
duration_minutes: 60,
notes: null,
image_url: null,
google_place_id: null,
google_ftid: null,
osm_id: null,
amap_poi_id: null,
website: null,
phone: null,
transport_mode: 'walking',
route_geometry: wp.routeGeometry || null,
route_color: null,
stop_type: null,
fill_percent: null,
});
const place = (await this.placesRepo.findWithTagsAndRatings(placeId))!;
created.push(place);
trackInsertedInDedupSet({ name: wp.name, lat: wp.lat, lng: wp.lng }, dedup);
}
@@ -879,17 +913,13 @@ export class PlacesService {
summary.warnings.push(decoded.warning);
}
const categories = this.dbs.all<{ id: number; name: string }>('SELECT id, name FROM categories');
// PL33 — `CategoriesRepository.listIdName` (Plan 3a's repository).
const categories = await this.categoriesRepo.listIdName();
const categoryLookup = buildCategoryNameLookup(categories);
const dedup = await this.buildDedupSet(tripId);
const created: PlaceWithTags[] = [];
let dupCount = 0;
const insertStmt = this.dbs.prepare(`
INSERT INTO places (trip_id, name, description, lat, lng, category_id, transport_mode, route_geometry)
VALUES (?, ?, ?, ?, ?, ?, 'walking', ?)
`);
await this.uow.transactional(async () => {
let fallbackIndex = 1;
for (const node of placemarkNodes) {
@@ -928,17 +958,40 @@ export class PlacesService {
const categoryId = resolveCategoryIdForFolder(parsedPlacemark.folderName, categoryLookup);
const result = insertStmt.run(
tripId,
// PL34/PL35 — `insertPlace` with the legacy 8-column set (trip_id,
// name, description, lat, lng, category_id, transport_mode='walking'
// literal, route_geometry); every omitted column takes its DB-default
// value. Re-selected via `findWithTagsAndRatings` INSIDE the same
// transaction.
const placeId = await this.placesRepo.insertPlace({
trip_id: Number(tripId),
name,
parsedPlacemark.description,
parsedPlacemark.lat,
parsedPlacemark.lng,
categoryId,
parsedPlacemark.routeGeometry,
);
description: parsedPlacemark.description,
lat: parsedPlacemark.lat,
lng: parsedPlacemark.lng,
address: null,
category_id: categoryId,
price: null,
currency: null,
place_time: null,
end_time: null,
duration_minutes: 60,
notes: null,
image_url: null,
google_place_id: null,
google_ftid: null,
osm_id: null,
amap_poi_id: null,
website: null,
phone: null,
transport_mode: 'walking',
route_geometry: parsedPlacemark.routeGeometry,
route_color: null,
stop_type: null,
fill_percent: null,
});
const place = (await this.dbs.getPlaceWithTags(Number(result.lastInsertRowid)))!;
const place = (await this.placesRepo.findWithTagsAndRatings(placeId))!;
created.push(place);
trackInsertedInDedupSet({ name, lat: parsedPlacemark.lat, lng: parsedPlacemark.lng }, dedup);
summary.createdCount += 1;
@@ -981,22 +1034,19 @@ export class PlacesService {
const tracks = result?.places?.filter((p) => p.route_geometry && !p.route_color) ?? [];
if (tracks.length === 0) return;
// Read and write in one transaction so two concurrent imports cannot both
// read the same set of free colours.
// PL36+PL37 — read (`distinctRouteColors`) and write (`setRouteColor`) in
// ONE transaction so two concurrent imports cannot both read the same set
// of free colours.
await this.uow.transactional(async () => {
const taken = new Set(
this.dbs.all<{ c: string }>(
'SELECT DISTINCT route_color AS c FROM places WHERE trip_id = ? AND route_color IS NOT NULL', tripId,
).map((r) => r.c),
);
const taken = new Set(await this.placesRepo.distinctRouteColors(tripId));
const free = TRACK_COLORS.filter((c) => !taken.has(c));
tracks.forEach((track, i) => {
for (const [i, track] of tracks.entries()) {
// Free ones first, then wrap through the whole palette — never reuse a
// free colour twice within the same import.
const color = i < free.length ? free[i] : TRACK_COLORS[(i - free.length) % TRACK_COLORS.length];
this.dbs.run('UPDATE places SET route_color = ? WHERE id = ?', color, track.id);
await this.placesRepo.setRouteColor(track.id, color);
track.route_color = color;
});
}
});
}
@@ -1145,11 +1195,6 @@ export class PlacesService {
places: { name: string; lat: number; lng: number; notes: string | null; googleFtid: string | null }[],
): Promise<{ created: PlaceWithTags[]; skipped: number }> {
const dedup = await this.buildDedupSet(tripId);
const insertStmt = this.dbs.prepare(`
INSERT INTO places (trip_id, name, lat, lng, notes, google_ftid, transport_mode)
VALUES (?, ?, ?, ?, ?, ?, 'walking')
`);
const updateGoogleFtidStmt = this.dbs.prepare('UPDATE places SET google_ftid = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?');
const created: PlaceWithTags[] = [];
let skipped = 0;
await this.uow.transactional(async () => {
@@ -1162,14 +1207,47 @@ export class PlacesService {
const candidate = { name: p.name, lat: p.lat, lng: p.lng, google_ftid: p.googleFtid };
if (isPlaceDuplicate(candidate, dedup)) {
const duplicate = await this.findDuplicatePlace(tripId, candidate);
// PL39 — `backfillFtid`, unscoped by trip (matching the legacy
// statement — `duplicate` was already resolved against this trip).
if (duplicate && !duplicate.google_ftid && p.googleFtid) {
updateGoogleFtidStmt.run(p.googleFtid, duplicate.id);
await this.placesRepo.backfillFtid(duplicate.id, p.googleFtid);
}
skipped++;
continue;
}
const result = insertStmt.run(tripId, p.name, p.lat, p.lng, p.notes, p.googleFtid);
const place = (await this.dbs.getPlaceWithTags(Number(result.lastInsertRowid)))!;
// PL38/PL40 — `insertPlace` with the legacy 7-column set (trip_id,
// name, lat, lng, notes, google_ftid, transport_mode='walking'
// literal); every omitted column takes its DB-default value.
// Re-selected via `findWithTagsAndRatings` INSIDE the same
// transaction.
const placeId = await this.placesRepo.insertPlace({
trip_id: Number(tripId),
name: p.name,
description: null,
lat: p.lat,
lng: p.lng,
address: null,
category_id: null,
price: null,
currency: null,
place_time: null,
end_time: null,
duration_minutes: 60,
notes: p.notes,
image_url: null,
google_place_id: null,
google_ftid: p.googleFtid,
osm_id: null,
amap_poi_id: null,
website: null,
phone: null,
transport_mode: 'walking',
route_geometry: null,
route_color: null,
stop_type: null,
fill_percent: null,
});
const place = (await this.placesRepo.findWithTagsAndRatings(placeId))!;
created.push(place);
trackInsertedInDedupSet(candidate, dedup);
}
@@ -1396,20 +1474,46 @@ export class PlacesService {
}
const dedup = await this.buildDedupSet(tripId);
const insertStmt = this.dbs.prepare(`
INSERT INTO places (trip_id, name, lat, lng, address, notes, transport_mode)
VALUES (?, ?, ?, ?, ?, ?, 'walking')
`);
const created: PlaceWithTags[] = [];
let skipped = 0;
// PL41/PL42 — `insertPlace` with the legacy 7-column set (trip_id, name,
// lat, lng, address, notes, transport_mode='walking' literal); every
// omitted column takes its DB-default value. Re-selected via
// `findWithTagsAndRatings` INSIDE the same transaction.
await this.uow.transactional(async () => {
for (const p of places) {
if (isPlaceDuplicate({ name: p.name, lat: p.lat, lng: p.lng }, dedup)) {
skipped++;
continue;
}
const result = insertStmt.run(tripId, p.name, p.lat, p.lng, p.address, p.notes);
const place = (await this.dbs.getPlaceWithTags(Number(result.lastInsertRowid)))!;
const placeId = await this.placesRepo.insertPlace({
trip_id: Number(tripId),
name: p.name,
description: null,
lat: p.lat,
lng: p.lng,
address: p.address,
category_id: null,
price: null,
currency: null,
place_time: null,
end_time: null,
duration_minutes: 60,
notes: p.notes,
image_url: null,
google_place_id: null,
google_ftid: null,
osm_id: null,
amap_poi_id: null,
website: null,
phone: null,
transport_mode: 'walking',
route_geometry: null,
route_color: null,
stop_type: null,
fill_percent: null,
});
const place = (await this.placesRepo.findWithTagsAndRatings(placeId))!;
created.push(place);
trackInsertedInDedupSet({ name: p.name, lat: p.lat, lng: p.lng }, dedup);
}
@@ -1466,19 +1570,16 @@ export class PlacesService {
if (!gpid) return;
const gftid = trimOrNull(match.google_ftid);
// COALESCE so enrichment only fills empty columns — never overwrites data the
// import already captured (e.g. Naver's address) or anything the user edited.
this.dbs.run(
`UPDATE places
SET google_place_id = COALESCE(google_place_id, ?),
google_ftid = COALESCE(google_ftid, ?),
address = COALESCE(address, ?),
website = COALESCE(website, ?),
phone = COALESCE(phone, ?),
updated_at = CURRENT_TIMESTAMP
WHERE id = ? AND trip_id = ?`,
gpid, gftid, trimOrNull(match.address), trimOrNull(match.website), trimOrNull(match.phone), place.id, tripId,
);
// PL43 — `fillIfEmpty`, COALESCE-per-column so enrichment only fills
// empty columns — never overwrites data the import already captured
// (e.g. Naver's address) or anything the user edited.
await this.placesRepo.fillIfEmpty(place.id, Number(tripId), {
google_place_id: gpid,
google_ftid: gftid,
address: trimOrNull(match.address),
website: trimOrNull(match.website),
phone: trimOrNull(match.phone),
});
// Photo is best-effort: Google often has none, in which case getPlacePhoto
// resolves with photoUrl: null. A missing photo (or a provider outage, which
@@ -1486,18 +1587,17 @@ export class PlacesService {
try {
const photo = await this.maps.getPlacePhoto(userId, gpid, place.lat, place.lng, place.name);
if (photo?.photoUrl) {
this.dbs.run(
'UPDATE places SET image_url = COALESCE(image_url, ?), updated_at = CURRENT_TIMESTAMP WHERE id = ? AND trip_id = ?',
photo.photoUrl, place.id, tripId,
);
// PL44 — `fillIfEmpty`, the `image_url`-only shape.
await this.placesRepo.fillIfEmpty(place.id, Number(tripId), { image_url: photo.photoUrl });
}
} catch {
/* no photo — leave image_url as-is */
}
// Push the enriched row to every connected client (no socket exclusion: the
// importer's own client should also receive the late update).
const updated = await this.dbs.getPlaceWithTags(place.id);
// PL45 — push the enriched row to every connected client (no socket
// exclusion: the importer's own client should also receive the late
// update — `socketId: undefined`, no `X-Socket-Id` to exclude).
const updated = await this.placesRepo.findWithTagsAndRatings(place.id);
if (updated) this.realtime.broadcast(tripId, 'place:updated', { place: updated }, undefined);
}
@@ -1564,11 +1664,11 @@ export class PlacesService {
timeoutMs: 10000,
});
if (!address) continue;
this.dbs.run(
'UPDATE places SET address = COALESCE(address, ?), updated_at = CURRENT_TIMESTAMP WHERE id = ? AND trip_id = ?',
address, place.id, tripId,
);
const updated = await this.dbs.getPlaceWithTags(place.id);
// PL46 — `fillIfEmpty`, the `address`-only shape.
await this.placesRepo.fillIfEmpty(place.id, Number(tripId), { address });
// PL47 — same broadcast shape as PL45: `socketId: undefined`, no
// exclusion.
const updated = await this.placesRepo.findWithTagsAndRatings(place.id);
if (updated) this.realtime.broadcast(tripId, 'place:updated', { place: updated }, undefined);
} catch (err) {
console.error(`[Places] address backfill failed for place ${place.id}:`, err instanceof Error ? err.message : err);