fix(atlas): a place that moves no longer keeps its old country (#2527)

place_regions caches the country and region Atlas resolved for a place,
and a row that was already there was never re-derived. Correcting a
place's location in the trip left the old row in charge, so Atlas, the
dashboard stats and the journey stats kept counting the country the place
had just left, with the trip's dates on it.

A trigger on places now drops the cached row whenever lat, lng or address
actually change, so every writer is covered: the place editor, the MCP
update tools, the plugin RPC and the import enrichment and address
backfill. An edit that writes the same location back keeps the row.

The background lookup that fills the cache also only writes while the
place still sits where it was resolved from. A place moved while its
lookup was running would otherwise get the old answer written back.

Places corrected before this fix would have kept their old country until
they were moved again, because the trigger only sees new edits.
PlaceRegionsRepairJob goes over place_regions once, on the first start
after the update, and remembers that in app_settings. It re-derives each
row from the bundled borders only, never Nominatim, and replaces a row
whose country or region no longer matches where its place is now. A row
the bundle cannot answer for is left alone unless both the coordinates
and the address put the place in another country; then it is dropped and
the next Atlas load looks the place up again. A place without
coordinates keeps no row. Its writes are guarded the same way as the
background lookup, so a place edited during the pass is not overwritten.

The in-memory region cache is now keyed by the country the address names
as well as the coordinates, so correcting only the address of a border
point no longer returns the old answer until a restart.
This commit is contained in:
Maurice
2026-09-25 23:52:47 +02:00
parent a0952bd871
commit cf5920ebe0
11 changed files with 721 additions and 62 deletions
+28
View File
@@ -5257,6 +5257,34 @@ function runMigrations(db: Database.Database): void {
`);
}
},
/*
* A place that moves takes its Atlas country with it (#2527).
*
* place_regions caches the country and region Atlas resolved from a
* place's coordinates and address, and nothing re-derives a row that is
* already there. Correcting a place's location left the old row in charge,
* so Atlas, the dashboard stats and the journey stats kept counting the
* country the place had just left.
*
* A trigger rather than a delete in the places code, because the row is
* derived from the place and every writer of lat, lng or address has to
* let go of it: the place editor, update_place, the plugin RPC, the import
* enrichment and its address backfill, and whatever comes next. The WHEN
* clause matters because every place edit writes lat, lng and address
* back whether they changed or not, and renaming a place must not throw
* away a good row. The next Atlas load resolves the place where it is now.
*/
() => {
db.exec(`
CREATE TRIGGER IF NOT EXISTS trg_place_regions_follow_place
AFTER UPDATE OF lat, lng, address ON places
WHEN OLD.lat IS NOT NEW.lat OR OLD.lng IS NOT NEW.lng OR OLD.address IS NOT NEW.address
BEGIN
DELETE FROM place_regions WHERE place_id = NEW.id;
END
`);
},
];
if (currentVersion < migrations.length) {
+25 -14
View File
@@ -781,7 +781,7 @@ async function resolveCountryCode(place: Place): Promise<string | null> {
return null;
}
export function resolveCountryCodeSync(place: Place): string | null {
export function resolveCountryCodeSync(place: Pick<Place, 'lat' | 'lng' | 'address'>): string | null {
const hasCoords = !!(place.lat && place.lng);
if (hasCoords) {
const fromCoords = getCountryFromCoords(place.lat!, place.lng!);
@@ -937,10 +937,13 @@ function buildRegionInfo(address: Record<string, string>, preferFinest: boolean)
};
}
export async function reverseGeocodeRegion(lat: number, lng: number, placeAddress?: string | null): Promise<RegionInfo | null> {
const key = cacheKeyFor(lat, lng, 'region');
if (regionCache.has(key)) return regionCache.get(key)!;
/**
* The offline half of reverseGeocodeRegion: the bundled admin1 polygons, then the
* box-gated address fallback. Null when the bundle has no answer for the point, which
* is where reverseGeocodeRegion goes on to ask Nominatim. Deterministic and uncached,
* so it is also what the one time place_regions repair re-derives rows with (#2527).
*/
export async function resolveRegionFromBundle(lat: number, lng: number, placeAddress?: string | null): Promise<RegionInfo | null> {
// Prefer resolving directly against the bundled polygons: offline, deterministic, and —
// unlike Nominatim's address levels — guaranteed to match a feature the client can
// actually highlight. Falls through to reverse geocoding when the country has no admin1
@@ -948,10 +951,7 @@ export async function reverseGeocodeRegion(lat: number, lng: number, placeAddres
const coordCountry = getCountryFromCoords(lat, lng);
if (coordCountry) {
const fromBundle = await getRegionFromCoords(coordCountry, lat, lng);
if (fromBundle) {
regionCache.set(key, fromBundle);
return fromBundle;
}
if (fromBundle) return fromBundle;
}
// The coordinate-only lookup found no matching region — either no country polygon contains
// the point, or a simplified admin0 border put it in the WRONG country (a place on the
@@ -966,11 +966,22 @@ export async function reverseGeocodeRegion(lat: number, lng: number, placeAddres
// trusting a region match in it.
const addressCountry = getCountryFromAddress(placeAddress ?? null);
if (addressCountry && addressCountry !== coordCountry && isPointInCountryBox(addressCountry, lat, lng)) {
const fromAddress = await getRegionFromCoords(addressCountry, lat, lng);
if (fromAddress) {
regionCache.set(key, fromAddress);
return fromAddress;
}
return getRegionFromCoords(addressCountry, lat, lng);
}
return null;
}
export async function reverseGeocodeRegion(lat: number, lng: number, placeAddress?: string | null): Promise<RegionInfo | null> {
// The address takes part in the answer only through the country it names, so that
// country is part of the key. Keyed by coordinates alone, a place whose address was
// corrected on a border point got the old answer back from memory (#2527).
const key = `${cacheKeyFor(lat, lng, 'region')}|${getCountryFromAddress(placeAddress ?? null) ?? ''}`;
if (regionCache.has(key)) return regionCache.get(key)!;
const fromBundle = await resolveRegionFromBundle(lat, lng, placeAddress);
if (fromBundle) {
regionCache.set(key, fromBundle);
return fromBundle;
}
// Only reached when the bundle's own polygons for this country don't cover the point at
+6 -2
View File
@@ -11,6 +11,8 @@ import { PublicStatsController } from './public-stats.controller';
import { ApiTokenGuard } from '../public-api/api-token.guard';
import { TokensModule } from '../tokens/tokens.module';
import { RateLimitModule } from '../common/rate-limit.module';
import { SchedulingModule } from '../scheduling/scheduling.module';
import { PlaceRegionsRepairJob } from './place-regions-repair.job';
/**
* Atlas addon domain (L7 leaf module). Registered in AppModule. Exports
@@ -29,11 +31,13 @@ import { RateLimitModule } from '../common/rate-limit.module';
* than pulled in with PublicApiModule: @UseGuards instantiates a guard in the
* declaring controller's module, so exporting it from over there would still leave
* its TokenService unresolvable here. TokensModule is a leaf, so the edge is free.
*
* PlaceRegionsRepairJob puts right, once, the place_regions rows cached before #2527.
*/
@Module({
imports: [AuthModule, PluginGuardsModule, AddonsModule, TokensModule, RateLimitModule],
imports: [AuthModule, PluginGuardsModule, AddonsModule, TokensModule, RateLimitModule, SchedulingModule],
controllers: [AtlasController, TravelStatsController, PublicStatsController],
providers: [AtlasService, AtlasMcp, AtlasRpc, ApiTokenGuard],
providers: [AtlasService, AtlasMcp, AtlasRpc, ApiTokenGuard, PlaceRegionsRepairJob],
exports: [AtlasService],
})
export class AtlasModule {}
+136 -45
View File
@@ -4,14 +4,17 @@ import type { AtlasLocateResponse } from '@trek/shared';
import { Trip, Place } from '../../types';
import { DatabaseService } from '../database/database.service';
import {
getCountryFromAddress,
getCountryFromCoords,
getCountryGeoGz,
getRegionFromCoords,
getRegionGeo,
geocodingInFlight,
resolveCountryCodeSync,
resolveRegionFromBundle,
reverseGeocodeRegion,
} from './atlas-geo';
import type { RegionInfo } from './atlas-geo';
import { KNOWN_COUNTRIES } from './known-countries';
import { cityFromAddress } from './city-from-address';
import { transferEndpointIds } from './transfer-endpoints';
@@ -19,6 +22,15 @@ import type { FlightEndpointRow } from './transfer-endpoints';
import { countryVisitDates } from './visit-dates';
import { haversineKm } from '../common/geo';
/** The part of a place that its cached region is derived from. */
type LocatedPlace = Pick<Place, 'id' | 'lat' | 'lng' | 'address'>;
/** A place_regions row next to the location of its place. */
type CachedRegionRow = LocatedPlace & { country_code: string; region_code: string };
/** How many cached rows the #2527 repair checks before it lets other work run. */
const REPAIR_YIELD_EVERY = 200;
/**
* A reservation endpoint plus the two booking columns that decide whether it may
* take part in layover pairing at all (see transfer-endpoints.ts).
@@ -175,32 +187,131 @@ export class AtlasService {
}
}
if (uncachedForGeocode.length > 0) {
const insertStmt = this.db.prepare(
'INSERT OR REPLACE INTO place_regions (place_id, country_code, region_code, region_name) VALUES (?, ?, ?, ?)',
);
for (const p of uncachedForGeocode) geocodingInFlight.add(p.id);
void (async () => {
try {
for (const place of uncachedForGeocode) {
try {
const info = await reverseGeocodeRegion(place.lat!, place.lng!, place.address);
if (info) insertStmt.run(place.id, info.country_code, info.region_code, info.region_name);
} catch {
/* continue */
} finally {
geocodingInFlight.delete(place.id);
}
}
} catch {
for (const p of uncachedForGeocode) geocodingInFlight.delete(p.id);
}
})();
}
this.cacheRegionsInBackground(uncachedForGeocode);
return out;
}
/**
* Resolve each place's region in the background and cache it in place_regions.
*/
private cacheRegionsInBackground(places: Place[]): void {
if (places.length === 0) return;
for (const p of places) geocodingInFlight.add(p.id);
void (async () => {
try {
for (const place of places) {
try {
const info = await reverseGeocodeRegion(place.lat!, place.lng!, place.address);
if (info) this.cacheRegionWhileUnmoved(place, info);
} catch {
// individual failure, continue with the remaining places
} finally {
geocodingInFlight.delete(place.id);
}
}
} catch {
for (const p of places) geocodingInFlight.delete(p.id);
}
})();
}
/**
* Cache a resolved region, but only while the place still sits where it was resolved
* from. A place moved while its lookup was running has already had its row dropped by
* the place_regions trigger (#2527), and writing the old answer back would pin it to
* the country it just left. A place deleted in the meantime is skipped the same way.
*/
private cacheRegionWhileUnmoved(place: LocatedPlace, info: RegionInfo): boolean {
const written = this.db.run(
`INSERT OR REPLACE INTO place_regions (place_id, country_code, region_code, region_name)
SELECT id, ?, ?, ? FROM places WHERE id = ? AND lat = ? AND lng = ? AND address IS ?`,
info.country_code,
info.region_code,
info.region_name,
place.id,
place.lat,
place.lng,
place.address ?? null,
);
return written.changes > 0;
}
// ── One time repair of the rows cached before #2527 ───────────────────────
/**
* Before #2527 nothing dropped a place_regions row when its place moved, so an
* install that upgrades still holds the country every corrected place left. This
* re-derives each row with the bundled resolver alone, never Nominatim, and puts
* right the ones that no longer match where their place is now.
*
* Where the bundle answers for the place's current location with another country
* or region, the row gets that answer, which is what a fresh lookup would cache. A
* good row matches it, the address fallback case included.
*
* Where the bundle has no answer (a coastal point outside the simplified polygons,
* a country the bundle has no regions for), the row came from Nominatim and only
* Nominatim could judge it, so it stays, unless the coordinates put the place in
* another country and the address does not name the cached one either. Then it is
* dropped and the next Atlas load looks the place up again.
*
* A place without coordinates keeps no row, the same as the trigger does.
*
* Every write only lands while the place still holds the location it was read with,
* so a place edited while this runs keeps what the trigger and the next lookup give it.
*/
async repairStaleRegionCache(): Promise<{ replaced: number; dropped: number }> {
const rows = this.db.all<CachedRegionRow>(`
SELECT pr.place_id AS id, pr.country_code, pr.region_code, p.lat, p.lng, p.address
FROM place_regions pr
JOIN places p ON p.id = pr.place_id
ORDER BY pr.place_id
`);
let replaced = 0;
let dropped = 0;
for (const [i, row] of rows.entries()) {
// A big install holds many rows, so requests get a turn every so often.
if (i > 0 && i % REPAIR_YIELD_EVERY === 0) await new Promise((resolve) => setImmediate(resolve));
const fix = await this.staleRegionFix(row);
if (fix === 'drop') {
if (this.dropRegionWhileUnmoved(row)) dropped++;
} else if (fix && this.cacheRegionWhileUnmoved(row, fix)) {
replaced++;
}
}
return { replaced, dropped };
}
/** What a cached row needs: a new region, 'drop', or null when it is right. */
private async staleRegionFix(row: CachedRegionRow): Promise<RegionInfo | 'drop' | null> {
if (row.lat == null || row.lng == null) return 'drop';
const cachedCountry = row.country_code.toUpperCase();
const fresh = await resolveRegionFromBundle(row.lat, row.lng, row.address);
if (fresh) {
const same = fresh.country_code.toUpperCase() === cachedCountry && fresh.region_code === row.region_code;
return same ? null : fresh;
}
const countryNow = resolveCountryCodeSync(row);
const contradicted = !!countryNow && countryNow !== cachedCountry && getCountryFromAddress(row.address) !== cachedCountry;
return contradicted ? 'drop' : null;
}
private dropRegionWhileUnmoved(row: CachedRegionRow): boolean {
const removed = this.db.run(
`DELETE FROM place_regions
WHERE place_id = ? AND country_code = ? AND region_code = ?
AND EXISTS (SELECT 1 FROM places WHERE id = ? AND lat IS ? AND lng IS ? AND address IS ?)`,
row.id,
row.country_code,
row.region_code,
row.id,
row.lat ?? null,
row.lng ?? null,
row.address ?? null,
);
return removed.changes > 0;
}
// ── Stats ─────────────────────────────────────────────────────────────────
async stats(userId: number) {
@@ -731,29 +842,9 @@ export class AtlasService {
const cachedMap = new Map(cached.map((c) => [c.place_id, c]));
// Kick off background geocoding for uncached places; return cached data immediately.
const uncached = places.filter((p) => p.lat && p.lng && !cachedMap.has(p.id) && !geocodingInFlight.has(p.id));
if (uncached.length > 0) {
const insertStmt = this.db.prepare(
'INSERT OR REPLACE INTO place_regions (place_id, country_code, region_code, region_name) VALUES (?, ?, ?, ?)',
);
for (const p of uncached) geocodingInFlight.add(p.id);
void (async () => {
try {
for (const place of uncached) {
try {
const info = await reverseGeocodeRegion(place.lat!, place.lng!, place.address);
if (info) insertStmt.run(place.id, info.country_code, info.region_code, info.region_name);
} catch {
// individual failure — continue with remaining places
} finally {
geocodingInFlight.delete(place.id);
}
}
} catch {
for (const p of uncached) geocodingInFlight.delete(p.id);
}
})();
}
this.cacheRegionsInBackground(
places.filter((p) => p.lat && p.lng && !cachedMap.has(p.id) && !geocodingInFlight.has(p.id)),
);
// Group by country → regions with place counts
const regionMap: Record<
@@ -0,0 +1,53 @@
import { logInfo, logError } from '../audit/audit-log.logger';
import { DatabaseService } from '../database/database.service';
import { CronRegistrarService } from '../scheduling/cron-registrar.service';
import { AtlasService } from './atlas.service';
import { Injectable, type OnApplicationBootstrap } from '@nestjs/common';
/** The app_settings row that says the pass below has been through every cached region. */
export const PLACE_REGIONS_REPAIR_DONE_KEY = 'place_regions_repair_2527';
/**
* One pass over place_regions on the first start after #2527 (see
* AtlasService.repairStaleRegionCache).
*
* Not a cron: from this version on the trigger on places drops a row as soon as its
* place moves, so once the rows cached before it are put right there is nothing left
* to repair. app_settings remembers that the pass ran. A pass cut short by a restart
* or an error simply runs again on the next start; a second run changes nothing the
* first one wrote.
*
* In the background, like the other boot sweeps: the server does not wait for it.
*/
@Injectable()
export class PlaceRegionsRepairJob implements OnApplicationBootstrap {
constructor(
private readonly atlas: AtlasService,
private readonly registrar: CronRegistrarService,
private readonly db: DatabaseService,
) {}
onApplicationBootstrap(): void {
if (!this.registrar.isEnabled()) return;
void this.runOnce();
}
async runOnce(): Promise<void> {
try {
if (this.db.get('SELECT 1 FROM app_settings WHERE key = ?', PLACE_REGIONS_REPAIR_DONE_KEY)) return;
const { replaced, dropped } = await this.atlas.repairStaleRegionCache();
this.db.run(
'INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)',
PLACE_REGIONS_REPAIR_DONE_KEY,
new Date().toISOString(),
);
if (replaced + dropped > 0) {
logInfo(
`Atlas region repair: ${replaced} place region(s) re-derived where the place is now, ${dropped} left for the next Atlas load`,
);
}
} catch (err: unknown) {
logError(`Atlas region repair: ${err instanceof Error ? err.message : err}`);
}
}
}
+70
View File
@@ -536,3 +536,73 @@ describe('Regions geo', () => {
expect(res.body).toHaveProperty('type', 'FeatureCollection');
});
});
describe('A place that moves takes its Atlas country with it (#2527)', () => {
// The region cache is filled by a background task that GET /regions only starts,
// so wait for the row the way the next Atlas load would find it.
async function regionRowOf(placeId: number): Promise<{ country_code: string; region_code: string } | undefined> {
for (let i = 0; i < 100; i++) {
const row = testDb.prepare('SELECT country_code, region_code FROM place_regions WHERE place_id = ?').get(placeId) as
| { country_code: string; region_code: string }
| undefined;
if (row) return row;
await new Promise((resolve) => setTimeout(resolve, 20));
}
return undefined;
}
async function atlasCountries(userId: number): Promise<string[]> {
const res = await request(app).get('/api/addons/atlas/stats').set('Cookie', authCookie(userId));
expect(res.status).toBe(200);
return (res.body.countries as { code: string }[]).map((c) => c.code);
}
it('ATLAS-015: correcting a place from France to Germany moves it on the Atlas', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id, { title: 'Wrongly imported', start_date: '2025-05-01', end_date: '2025-05-05' });
const place = testDb
.prepare('INSERT INTO places (trip_id, name, lat, lng, address) VALUES (?, ?, ?, ?, ?) RETURNING id')
.get(trip.id, 'Hotel', 48.8566, 2.3522, 'Rue de Rivoli, Paris, France') as { id: number };
// The place is seen on the Atlas once, which caches France for it.
await request(app).get('/api/addons/atlas/regions').set('Cookie', authCookie(user.id));
expect((await regionRowOf(place.id))?.country_code).toBe('FR');
expect(await atlasCountries(user.id)).toEqual(['FR']);
// The user fixes the location in the trip.
const put = await request(app)
.put(`/api/trips/${trip.id}/places/${place.id}`)
.set('Cookie', authCookie(user.id))
.send({ lat: 52.5163, lng: 13.3777, address: 'Pariser Platz, Berlin, Germany' });
expect(put.status).toBe(200);
expect(await atlasCountries(user.id)).toEqual(['DE']);
// And the regions follow on the next load, with nothing left in France.
await request(app).get('/api/addons/atlas/regions').set('Cookie', authCookie(user.id));
expect((await regionRowOf(place.id))?.country_code).toBe('DE');
const regions = await request(app).get('/api/addons/atlas/regions').set('Cookie', authCookie(user.id));
expect(Object.keys(regions.body.regions)).toEqual(['DE']);
expect(await atlasCountries(user.id)).toEqual(['DE']);
});
it('ATLAS-015: an edit that leaves the location alone keeps the cached country', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id, { title: 'Paris', start_date: '2025-05-01', end_date: '2025-05-05' });
const place = testDb
.prepare('INSERT INTO places (trip_id, name, lat, lng, address) VALUES (?, ?, ?, ?, ?) RETURNING id')
.get(trip.id, 'Louvre', 48.8606, 2.3376, 'Rue de Rivoli, Paris, France') as { id: number };
await request(app).get('/api/addons/atlas/regions').set('Cookie', authCookie(user.id));
const cached = await regionRowOf(place.id);
expect(cached?.country_code).toBe('FR');
// The place editor sends the whole form back, location included, for a notes edit.
const put = await request(app)
.put(`/api/trips/${trip.id}/places/${place.id}`)
.set('Cookie', authCookie(user.id))
.send({ notes: 'Closed on Tuesdays', lat: 48.8606, lng: 2.3376, address: 'Rue de Rivoli, Paris, France' });
expect(put.status).toBe(200);
expect(testDb.prepare('SELECT country_code, region_code FROM place_regions WHERE place_id = ?').get(place.id)).toEqual(cached);
});
});
@@ -134,7 +134,7 @@ const ALLOWED_DESTRUCTIVE: Record<string, string> = {
'DELETE FROM journey_entries':
"Migration 121: DELETE ... WHERE title IN ('Gallery','[Trip Photos]') — remove synthetic wrapper entries replaced by the gallery model.",
'DELETE FROM place_regions':
'Atlas enclave fix: DELETE ... WHERE place_id IN (places inside specific enclave boxes) — invalidate stale region cache; re-resolved on next request.',
'Atlas enclave fix: DELETE ... WHERE place_id IN (places inside specific enclave boxes) to invalidate a stale region cache, re-resolved on next request. Also the body of trg_place_regions_follow_place (#2527): DELETE ... WHERE place_id = NEW.id when that one place\'s lat, lng or address changed. place_regions is a re-derivable cache either way.',
'DELETE FROM visited_regions':
'Atlas geoBoundaries swap (#1119): DELETE ... WHERE id = ? — after UPDATE OR IGNORE re-codes a manually-marked region to its current code, drop only the single leftover row whose UNIQUE(user_id, region_code) collision caused the update to be skipped (a duplicate of a region the user already has).',
'DELETE FROM hidden_regions':
@@ -0,0 +1,112 @@
import { runMigrations } from '../../../src/db/migrations';
import { createTestDb } from '../../helpers/test-db';
import Database from 'better-sqlite3';
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
// place_regions is Atlas's cache of the country and region a place resolved to.
// It is derived from the place's lat, lng and address, so a change to any of them
// has to take the row with it, whichever code path wrote the change (#2527).
// The trigger ships at schema version 244. Later migrations are appended after it,
// so the replay case rewinds to just before it; the steps after it are idempotent.
const PLACE_REGIONS_TRIGGER_VERSION = 244;
describe('place_regions follows the place it was resolved from (#2527)', () => {
let db: Database.Database;
let placeId: number;
let otherPlaceId: number;
const cached = (id: number) =>
db.prepare('SELECT country_code, region_code FROM place_regions WHERE place_id = ?').get(id) as
| { country_code: string; region_code: string }
| undefined;
beforeEach(() => {
db = createTestDb();
db.exec("INSERT INTO users (username, email, password_hash) VALUES ('traveller', 'traveller@example.test', 'x')");
const userId = (db.prepare('SELECT id FROM users').get() as { id: number }).id;
const tripId = db.prepare("INSERT INTO trips (user_id, title) VALUES (?, 'Trip')").run(userId)
.lastInsertRowid as number;
const insertPlace = db.prepare('INSERT INTO places (trip_id, name, lat, lng, address) VALUES (?, ?, ?, ?, ?)');
placeId = insertPlace.run(tripId, 'Hotel', 48.8566, 2.3522, 'Rue de Rivoli, Paris, France')
.lastInsertRowid as number;
otherPlaceId = insertPlace.run(tripId, 'Museum', 48.8606, 2.3376, 'Rue de Rivoli, Paris, France')
.lastInsertRowid as number;
const insertRegion = db.prepare(
"INSERT INTO place_regions (place_id, country_code, region_code, region_name) VALUES (?, 'FR', 'FR-IDF', 'Ile-de-France')",
);
insertRegion.run(placeId);
insertRegion.run(otherPlaceId);
});
afterEach(() => db?.close());
it('drops the cached region when the coordinates move', () => {
db.prepare('UPDATE places SET lat = ?, lng = ? WHERE id = ?').run(52.5163, 13.3777, placeId);
expect(cached(placeId)).toBeUndefined();
expect(cached(otherPlaceId)).toEqual({ country_code: 'FR', region_code: 'FR-IDF' });
});
it('drops it when only the latitude or only the longitude changes', () => {
db.prepare('UPDATE places SET lat = ? WHERE id = ?').run(48.9, placeId);
db.prepare('UPDATE places SET lng = ? WHERE id = ?').run(2.4, otherPlaceId);
expect(cached(placeId)).toBeUndefined();
expect(cached(otherPlaceId)).toBeUndefined();
});
it('drops it when the address changes, including one filled in where there was none', () => {
db.prepare('UPDATE places SET address = ? WHERE id = ?').run('Pariser Platz, Berlin, Germany', placeId);
db.prepare('UPDATE places SET address = NULL WHERE id = ?').run(otherPlaceId);
db.prepare(
"INSERT INTO place_regions (place_id, country_code, region_code, region_name) VALUES (?, 'FR', 'FR-IDF', 'Ile-de-France')",
).run(otherPlaceId);
// The import backfill only ever fills an empty address.
db.prepare('UPDATE places SET address = COALESCE(address, ?) WHERE id = ?').run('Paris, France', otherPlaceId);
expect(cached(placeId)).toBeUndefined();
expect(cached(otherPlaceId)).toBeUndefined();
});
it('drops it when the location is cleared', () => {
db.prepare('UPDATE places SET lat = NULL, lng = NULL WHERE id = ?').run(placeId);
expect(cached(placeId)).toBeUndefined();
});
it('keeps it when an edit writes the same location back', () => {
// The place editor and update_place write every column, location included.
db.prepare(
"UPDATE places SET name = 'Hotel du Louvre', notes = 'late check-in', lat = ?, lng = ?, address = ? WHERE id = ?",
).run(48.8566, 2.3522, 'Rue de Rivoli, Paris, France', placeId);
db.prepare("UPDATE places SET place_time = '09:00', updated_at = CURRENT_TIMESTAMP WHERE id = ?").run(otherPlaceId);
expect(cached(placeId)).toEqual({ country_code: 'FR', region_code: 'FR-IDF' });
expect(cached(otherPlaceId)).toEqual({ country_code: 'FR', region_code: 'FR-IDF' });
});
it('ships at its own schema version and replays without a second trigger', () => {
const triggers = () =>
db
.prepare(
"SELECT name FROM sqlite_master WHERE type = 'trigger' AND tbl_name = 'places' AND name = 'trg_place_regions_follow_place'",
)
.all();
expect(triggers()).toHaveLength(1);
db.exec('DROP TRIGGER trg_place_regions_follow_place');
db.prepare('UPDATE schema_version SET version = ?').run(PLACE_REGIONS_TRIGGER_VERSION);
runMigrations(db);
expect(triggers()).toHaveLength(0);
db.prepare('UPDATE schema_version SET version = ?').run(PLACE_REGIONS_TRIGGER_VERSION - 1);
runMigrations(db);
runMigrations(db);
expect(triggers()).toHaveLength(1);
db.prepare('UPDATE places SET lat = ? WHERE id = ?').run(52.5163, placeId);
expect(cached(placeId)).toBeUndefined();
});
});
@@ -237,6 +237,26 @@ describe('Tool: update_place', () => {
});
});
it('moving a place drops the country Atlas cached for it, renaming it does not (#2527)', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const place = createPlace(testDb, trip.id, { name: 'Hotel', lat: 48.8566, lng: 2.3522 });
testDb.prepare("INSERT INTO place_regions (place_id, country_code, region_code, region_name) VALUES (?, 'FR', 'FR-IDF', 'Ile-de-France')").run(place.id);
const cachedRegion = () => testDb.prepare('SELECT country_code FROM place_regions WHERE place_id = ?').get(place.id);
await withHarness(user.id, async (h) => {
await h.client.callTool({ name: 'update_place', arguments: { tripId: trip.id, placeId: place.id, name: 'Hotel Adlon' } });
expect(cachedRegion()).toEqual({ country_code: 'FR' });
const moved = await h.client.callTool({
name: 'update_place',
arguments: { tripId: trip.id, placeId: place.id, lat: 52.5163, lng: 13.3777, address: 'Unter den Linden 77, Berlin, Germany' },
});
expect(moved.isError).toBeFalsy();
expect(cachedRegion()).toBeUndefined();
});
});
it('broadcasts place:updated event', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
@@ -1155,6 +1155,60 @@ describe('getVisitedRegions', () => {
});
});
// ── A place that moves (#2527) ───────────────────────────────────────────────
describe('a place that moves keeps no stale region', () => {
function cachedCountryOf(placeId: number): string | undefined {
const row = testDb.prepare('SELECT country_code FROM place_regions WHERE place_id = ?').get(placeId) as
| { country_code: string }
| undefined;
return row?.country_code;
}
// The lookup runs in the background; the first one in a worker also loads the admin-1 bundle.
async function settledCountryOf(placeId: number): Promise<string | undefined> {
for (let i = 0; i < 150; i++) {
const code = cachedCountryOf(placeId);
if (code) return code;
await new Promise((resolve) => setTimeout(resolve, 20));
}
return undefined;
}
it('ATLAS-SVC-2527a: a lookup still running when the place moves does not write the old country back', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id, { title: 'Lyon, then Munich' });
const place = insertPlaceWithCoords(testDb, trip.id, 'Hotel', 45.764, 4.8357, 'Lyon, France');
// The Atlas load starts resolving Lyon, and the place is corrected before it answers.
const pending = atlas.visitedRegions(user.id);
testDb
.prepare('UPDATE places SET lat = ?, lng = ?, address = ? WHERE id = ?')
.run(48.1374, 11.5755, 'Marienplatz, Munich, Germany', place.id);
await pending;
await new Promise((resolve) => setTimeout(resolve, 300));
expect(cachedCountryOf(place.id)).toBeUndefined();
// The next load resolves where the place is now.
expect((await atlas.stats(user.id)).countries.map((c) => c.code)).toEqual(['DE']);
await atlas.visitedRegions(user.id);
expect(await settledCountryOf(place.id)).toBe('DE');
});
it('ATLAS-SVC-2527b: a place deleted while its lookup runs leaves nothing behind', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id, { title: 'Cancelled' });
const place = insertPlaceWithCoords(testDb, trip.id, 'Hotel', 43.2965, 5.3698, 'Marseille, France');
const pending = atlas.visitedRegions(user.id);
testDb.prepare('DELETE FROM places WHERE id = ?').run(place.id);
await pending;
await new Promise((resolve) => setTimeout(resolve, 300));
expect(testDb.prepare('SELECT COUNT(*) AS n FROM place_regions').get()).toEqual({ n: 0 });
});
});
// ── unmarkRegionVisited — tombstones + country cascade ──────────────────────
// Places are region-resolved by a fire-and-forget background task (see reverseGeocodeRegion
@@ -0,0 +1,216 @@
import { reverseGeocodeRegion } from '../../../src/nest/atlas/atlas-geo';
import { AtlasService } from '../../../src/nest/atlas/atlas.service';
import { PLACE_REGIONS_REPAIR_DONE_KEY, PlaceRegionsRepairJob } from '../../../src/nest/atlas/place-regions-repair.job';
import { DatabaseService } from '../../../src/nest/database/database.service';
import type { CronRegistrarService } from '../../../src/nest/scheduling/cron-registrar.service';
import { createTrip, createUser } from '../../helpers/factories';
import { createTestDb } from '../../helpers/test-db';
import type Database from 'better-sqlite3';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
// An install that upgrades to the #2527 fix still holds the place_regions rows its
// corrected places were cached with before it. The trigger only stops new ones from
// going stale, so a one time pass re-derives the old rows from the bundled borders.
const BERLIN = { lat: 52.5163, lng: 13.3777, address: 'Unter den Linden 77, 10117 Berlin, Germany' };
// Kehl sits inside France's bounding box, so a box check alone would call its FR row fine.
const KEHL = { lat: 48.5761, lng: 7.8152, address: 'Hauptstrasse 1, 77694 Kehl, Germany' };
const PARIS = { lat: 48.8566, lng: 2.3522, address: 'Rue de Rivoli, Paris, France' };
const LYON = { lat: 45.764, lng: 4.8357, address: 'Place Bellecour, Lyon, France' };
const MUNICH = { lat: 48.1374, lng: 11.5755, address: 'Marienplatz, Munich, Germany' };
// The coordinates fall in Germany's simplified polygon, the address puts it in Luxembourg.
const BOLLENDORF_PONT = {
lat: 49.8502458,
lng: 6.3576404,
address: '4 Gruusswiss, Bollendorf-Pont, Distrikt Gréiwemaacher 6555, Luxembourg',
};
// Points the bundle has no region for: only Nominatim can answer them.
const NICE_HARBOUR = { lat: 43.695, lng: 7.285, address: 'Quai des Etats-Unis, Nice, France' };
const SYLT_BEACH = { lat: 54.9, lng: 8.29, address: 'Westerland, Sylt, Germany' };
type Location = { lat: number | null; lng: number | null; address: string | null };
describe('the one time repair of place_regions rows cached before #2527', () => {
let db: Database.Database;
let atlas: AtlasService;
let job: PlaceRegionsRepairJob;
let userId: number;
let tripId: number;
const fetchMock = vi.fn();
const addPlace = (name: string, at: Location, cached?: [string, string, string]) => {
const id = db
.prepare('INSERT INTO places (trip_id, name, lat, lng, address) VALUES (?, ?, ?, ?, ?)')
.run(tripId, name, at.lat, at.lng, at.address).lastInsertRowid as number;
if (cached) {
db.prepare(
'INSERT INTO place_regions (place_id, country_code, region_code, region_name) VALUES (?, ?, ?, ?)',
).run(id, ...cached);
}
return id;
};
const cachedRow = (id: number) =>
db.prepare('SELECT country_code, region_code FROM place_regions WHERE place_id = ?').get(id) as
| { country_code: string; region_code: string }
| undefined;
const markedDone = () => db.prepare('SELECT 1 FROM app_settings WHERE key = ?').get(PLACE_REGIONS_REPAIR_DONE_KEY);
const registrar = (enabled: boolean) => ({ isEnabled: () => enabled }) as unknown as CronRegistrarService;
beforeEach(() => {
db = createTestDb();
const dbService = new DatabaseService(db);
atlas = new AtlasService(dbService);
job = new PlaceRegionsRepairJob(atlas, registrar(true), dbService);
userId = createUser(db).user.id;
tripId = createTrip(db, userId, { title: 'Rhine', start_date: '2025-05-01', end_date: '2025-05-05' }).id;
fetchMock.mockReset();
vi.stubGlobal('fetch', fetchMock);
});
afterEach(() => {
vi.unstubAllGlobals();
vi.restoreAllMocks();
db.close();
});
it('shows an upgraded install the country its corrected places are in now, without editing them again', async () => {
// The shape from the report: moved to Germany on 4.3.2, still cached in France.
const hotel = addPlace('Hotel Adlon', BERLIN, ['FR', 'FR-IDF', 'Ile-de-France']);
const rhine = addPlace('Rhine Hotel', KEHL, ['FR', 'FR-GES', 'Grand Est']);
expect((await atlas.stats(userId)).countries.map((c) => c.code)).toEqual(['FR']);
await job.runOnce();
expect(cachedRow(hotel)).toEqual({ country_code: 'DE', region_code: 'DE-BE' });
expect(cachedRow(rhine)).toEqual({ country_code: 'DE', region_code: 'DE-BW' });
expect((await atlas.stats(userId)).countries.map((c) => c.code)).toEqual(['DE']);
expect(Object.keys((await atlas.visitedRegions(userId)).regions)).toEqual(['DE']);
expect(atlas.getTravelStats(userId).countries).toEqual(['DE']);
expect(atlas.lastTrip(userId)?.countries).toEqual(['DE']);
expect(markedDone()).toBeTruthy();
expect(fetchMock).not.toHaveBeenCalled();
});
it('re-derives a region that moved inside the same country', async () => {
const museum = addPlace('Musee des Confluences', LYON, ['FR', 'FR-IDF', 'Ile-de-France']);
expect(await atlas.repairStaleRegionCache()).toEqual({ replaced: 1, dropped: 0 });
expect(cachedRow(museum)).toEqual({ country_code: 'FR', region_code: 'FR-ARA' });
});
it('leaves good rows alone, the address fallback and rows only Nominatim could judge included', async () => {
const paris = addPlace('Louvre', PARIS, ['FR', 'FR-IDF', 'Ile-de-France']);
const airbnb = addPlace('Airbnb', BOLLENDORF_PONT, ['LU', 'LU-EC', 'Canton Echternach']);
// The bundle calls this point Italian and has no region for it; the address backs the row.
const harbour = addPlace('Port Lympia', NICE_HARBOUR, ['FR', 'FR-PAC', "Provence-Alpes-Cote d'Azur"]);
const ship = addPlace('Cruise ship', { lat: 0.5, lng: -30, address: null }, ['BR', 'BR-PE', 'Pernambuco']);
expect(await atlas.repairStaleRegionCache()).toEqual({ replaced: 0, dropped: 0 });
expect(cachedRow(paris)).toEqual({ country_code: 'FR', region_code: 'FR-IDF' });
expect(cachedRow(airbnb)).toEqual({ country_code: 'LU', region_code: 'LU-EC' });
expect(cachedRow(harbour)).toEqual({ country_code: 'FR', region_code: 'FR-PAC' });
expect(cachedRow(ship)).toEqual({ country_code: 'BR', region_code: 'BR-PE' });
});
it('drops a row the bundle cannot re-derive when coordinates and address both name another country', async () => {
const beach = addPlace('Beach hut', SYLT_BEACH, ['FR', 'FR-BRE', 'Bretagne']);
const cleared = addPlace('Somewhere', { lat: null, lng: null, address: null }, ['FR', 'FR-IDF', 'Ile-de-France']);
expect(await atlas.repairStaleRegionCache()).toEqual({ replaced: 0, dropped: 2 });
expect(cachedRow(beach)).toBeUndefined();
expect(cachedRow(cleared)).toBeUndefined();
// The dashboard, which reads only the cache, no longer counts France.
expect(atlas.getTravelStats(userId).countries).toEqual([]);
});
it('does not write over a place that is edited while the pass runs', async () => {
const hotel = addPlace('Hotel Adlon', BERLIN, ['FR', 'FR-IDF', 'Ile-de-France']);
const beach = addPlace('Beach hut', SYLT_BEACH, ['FR', 'FR-BRE', 'Bretagne']);
// The pass has read both rows before it gives way for the first time.
const pass = atlas.repairStaleRegionCache();
db.prepare('UPDATE places SET lat = ?, lng = ?, address = ? WHERE id = ?').run(
MUNICH.lat,
MUNICH.lng,
MUNICH.address,
hotel,
);
db.prepare("UPDATE places SET address = 'Westerland, Germany' WHERE id = ?").run(beach);
db.prepare(
"INSERT INTO place_regions (place_id, country_code, region_code, region_name) VALUES (?, 'DE', 'DE-SH', 'Schleswig-Holstein')",
).run(beach);
expect(await pass).toEqual({ replaced: 0, dropped: 0 });
// The trigger dropped the moved place's row and nothing wrote Berlin back.
expect(cachedRow(hotel)).toBeUndefined();
// The row a fresh lookup wrote for the edited place stays.
expect(cachedRow(beach)).toEqual({ country_code: 'DE', region_code: 'DE-SH' });
});
it('runs once, and a second pass over the same rows changes nothing', async () => {
// Enough rows for the pass to give way to other work along the way.
const moved = Array.from({ length: 250 }, (_, i) =>
addPlace(`Stop ${i}`, BERLIN, ['FR', 'FR-IDF', 'Ile-de-France']),
);
await job.runOnce();
expect(moved.every((id) => cachedRow(id)?.region_code === 'DE-BE')).toBe(true);
expect(await atlas.repairStaleRegionCache()).toEqual({ replaced: 0, dropped: 0 });
// Marked as done, the next start does not read the cache again.
const later = addPlace('Later', BERLIN, ['FR', 'FR-IDF', 'Ile-de-France']);
await job.runOnce();
expect(cachedRow(later)).toEqual({ country_code: 'FR', region_code: 'FR-IDF' });
});
it('is not marked as done when the pass fails, so the next start tries again', async () => {
const hotel = addPlace('Hotel Adlon', BERLIN, ['FR', 'FR-IDF', 'Ile-de-France']);
vi.spyOn(atlas, 'repairStaleRegionCache').mockRejectedValueOnce(new Error('disk I/O error'));
vi.spyOn(console, 'error').mockImplementation(() => {});
await job.runOnce();
expect(markedDone()).toBeUndefined();
expect(cachedRow(hotel)?.country_code).toBe('FR');
await job.runOnce();
expect(markedDone()).toBeTruthy();
expect(cachedRow(hotel)?.country_code).toBe('DE');
});
it('starts from the boot hook, but not where the scheduler is off', () => {
const dbService = new DatabaseService(db);
const offJob = new PlaceRegionsRepairJob(atlas, registrar(false), dbService);
const onJob = new PlaceRegionsRepairJob(atlas, registrar(true), dbService);
const offRun = vi.spyOn(offJob, 'runOnce').mockResolvedValue();
const onRun = vi.spyOn(onJob, 'runOnce').mockResolvedValue();
offJob.onApplicationBootstrap();
onJob.onApplicationBootstrap();
expect(offRun).not.toHaveBeenCalled();
expect(onRun).toHaveBeenCalledTimes(1);
});
});
describe('the in-memory region cache (#2527)', () => {
afterEach(() => vi.unstubAllGlobals());
it('answers again when only the address of a border point changes', async () => {
// Without an address this point falls to Nominatim, which is asked once.
const fetchMock = vi.fn().mockResolvedValue({
ok: true,
json: async () => ({ address: { country_code: 'de', state: 'Rheinland-Pfalz', 'ISO3166-2-lvl4': 'DE-RP' } }),
});
vi.stubGlobal('fetch', fetchMock);
const before = await reverseGeocodeRegion(BOLLENDORF_PONT.lat, BOLLENDORF_PONT.lng, null);
const after = await reverseGeocodeRegion(BOLLENDORF_PONT.lat, BOLLENDORF_PONT.lng, BOLLENDORF_PONT.address);
const again = await reverseGeocodeRegion(BOLLENDORF_PONT.lat, BOLLENDORF_PONT.lng, null);
expect(before?.country_code).toBe('DE');
expect(after).toEqual({ country_code: 'LU', region_code: 'LU-EC', region_name: 'Canton Echternach' });
expect(again?.country_code).toBe('DE');
expect(fetchMock).toHaveBeenCalledTimes(1);
});
});