mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-06-19 13:21:46 +00:00
fix(maps): bound place-photo cache growth (Wikimedia + Google) (#1174)
The place-photo cache (uploads/photos/google) grew unbounded: a Wikimedia geosearch path cached full-res originals despite requesting a 400px thumb, the writer applied no size guard, nothing reclaimed orphaned files, and backups archived the whole re-derivable cache verbatim. - Prefer the scaled `thumburl` over the full-res `info.url` in the Commons geosearch fallback. - Downscale any cached image to <=800px JPEG via the existing jimp dep, with a safe fallback to the original bytes on decode failure. - Add sweepOrphans() (orphaned meta rows + stray files) wired into the scheduler (startup + nightly), and removeIfUnreferenced() called on place delete for prompt reclamation. - Exclude the re-derivable photo/trek caches from backups; restores self-heal as the cache dirs are recreated at startup.
This commit is contained in:
@@ -25,6 +25,7 @@ const archiverInstanceMock = vi.hoisted(() => ({
|
||||
pipe: vi.fn(),
|
||||
file: vi.fn(),
|
||||
directory: vi.fn(),
|
||||
glob: vi.fn(),
|
||||
finalize: vi.fn(),
|
||||
on: vi.fn(),
|
||||
}));
|
||||
@@ -441,7 +442,7 @@ describe('BACKUP-036 createBackup', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('BACKUP-036e — includes uploads directory when it exists', async () => {
|
||||
it('BACKUP-036e — includes uploads but excludes the re-derivable photo caches', async () => {
|
||||
fsMock.existsSync.mockImplementation((p: string) => {
|
||||
if (String(p).endsWith('uploads')) return true;
|
||||
return false;
|
||||
@@ -467,10 +468,16 @@ describe('BACKUP-036 createBackup', () => {
|
||||
|
||||
await createBackup();
|
||||
|
||||
expect(archiverInstanceMock.directory).toHaveBeenCalledWith(
|
||||
expect.stringContaining('uploads'),
|
||||
'uploads'
|
||||
expect(archiverInstanceMock.glob).toHaveBeenCalledWith(
|
||||
'**/*',
|
||||
expect.objectContaining({
|
||||
cwd: expect.stringContaining('uploads'),
|
||||
ignore: ['photos/google/**', 'photos/trek/**'],
|
||||
}),
|
||||
{ prefix: 'uploads' },
|
||||
);
|
||||
// The re-derivable caches must not be archived verbatim.
|
||||
expect(archiverInstanceMock.directory).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -538,6 +538,31 @@ describe('fetchWikimediaPhoto (fetch stubbed)', () => {
|
||||
expect(result!.attribution).toBe('Alice');
|
||||
});
|
||||
|
||||
it('MAPS-036b: geosearch prefers the scaled thumburl over the full-res original', async () => {
|
||||
const wikiResponse = { ok: true, json: async () => ({ query: { pages: { '-1': {} } } }) };
|
||||
const commonsResponse = {
|
||||
ok: true,
|
||||
json: async () => ({
|
||||
query: { pages: { '1': {
|
||||
imageinfo: [{
|
||||
url: 'https://commons.org/original-16mb.jpg',
|
||||
thumburl: 'https://commons.org/thumb-400.jpg',
|
||||
mime: 'image/jpeg',
|
||||
extmetadata: { Artist: { value: 'Alice' } },
|
||||
}],
|
||||
} } },
|
||||
}),
|
||||
};
|
||||
vi.stubGlobal('fetch', vi.fn()
|
||||
.mockResolvedValueOnce(wikiResponse)
|
||||
.mockResolvedValueOnce(commonsResponse));
|
||||
const { fetchWikimediaPhoto } = await import('../../../src/services/mapsService');
|
||||
const result = await fetchWikimediaPhoto(48.8, 2.3, 'Some Place');
|
||||
expect(result).toBeDefined();
|
||||
expect(result!.photoUrl).toBe('https://commons.org/thumb-400.jpg');
|
||||
expect(result!.attribution).toBe('Alice');
|
||||
});
|
||||
|
||||
it('MAPS-037: returns null when both strategies find nothing', async () => {
|
||||
vi.stubGlobal('fetch', vi.fn().mockResolvedValue({
|
||||
ok: true,
|
||||
|
||||
@@ -0,0 +1,151 @@
|
||||
/**
|
||||
* Unit tests for placePhotoCache — PPC-001 through PPC-010.
|
||||
* Covers the downscale guard in put(), removeIfUnreferenced(), and sweepOrphans().
|
||||
* Uses a real in-memory SQLite DB and a throwaway temp upload dir
|
||||
* (TREK_PLACE_PHOTO_DIR) so the real uploads tree is never touched.
|
||||
*/
|
||||
import { describe, it, expect, beforeAll, beforeEach, afterAll, vi } from 'vitest';
|
||||
import path from 'node:path';
|
||||
import fs from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import crypto from 'node:crypto';
|
||||
import { Jimp, JimpMime } from 'jimp';
|
||||
import Database from 'better-sqlite3';
|
||||
|
||||
// Throwaway upload dir — set before importing the module under test (it reads the
|
||||
// env at load time and mkdirs the dir).
|
||||
const TMP_DIR = fs.mkdtempSync(path.join(os.tmpdir(), 'ppc-'));
|
||||
process.env.TREK_PLACE_PHOTO_DIR = TMP_DIR;
|
||||
|
||||
// Minimal real DB with just the two tables placePhotoCache touches.
|
||||
const testDb = new Database(':memory:');
|
||||
testDb.exec(`
|
||||
CREATE TABLE places (
|
||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
google_place_id TEXT,
|
||||
image_url TEXT
|
||||
);
|
||||
CREATE TABLE google_place_photo_meta (
|
||||
place_id TEXT PRIMARY KEY,
|
||||
attribution TEXT,
|
||||
fetched_at INTEGER NOT NULL,
|
||||
error_at INTEGER
|
||||
);
|
||||
`);
|
||||
|
||||
vi.mock('../../../src/db/database', () => ({ db: testDb }));
|
||||
|
||||
function filePathFor(placeId: string): string {
|
||||
const hash = crypto.createHash('sha1').update(placeId).digest('hex');
|
||||
return path.join(TMP_DIR, `${hash}.jpg`);
|
||||
}
|
||||
|
||||
async function makeJpeg(width: number, height: number): Promise<Buffer> {
|
||||
const img = new Jimp({ width, height, color: 0xff0000ff });
|
||||
return img.getBuffer(JimpMime.jpeg, { quality: 80 });
|
||||
}
|
||||
|
||||
let cache: typeof import('../../../src/services/placePhotoCache');
|
||||
|
||||
beforeAll(async () => {
|
||||
cache = await import('../../../src/services/placePhotoCache');
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
testDb.exec('DELETE FROM places; DELETE FROM google_place_photo_meta;');
|
||||
for (const f of fs.readdirSync(TMP_DIR)) fs.rmSync(path.join(TMP_DIR, f), { force: true });
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
testDb.close();
|
||||
fs.rmSync(TMP_DIR, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
describe('placePhotoCache.put() downscale guard', () => {
|
||||
it('PPC-001: downscales an oversized image to <= 800px', async () => {
|
||||
const big = await makeJpeg(1600, 1200);
|
||||
await cache.put('big-place', big, 'Alice');
|
||||
|
||||
const written = fs.readFileSync(filePathFor('big-place'));
|
||||
const decoded = await Jimp.read(written);
|
||||
expect(Math.max(decoded.bitmap.width, decoded.bitmap.height)).toBeLessThanOrEqual(800);
|
||||
expect(written.length).toBeLessThan(big.length);
|
||||
});
|
||||
|
||||
it('PPC-002: passes a small image through unchanged', async () => {
|
||||
const small = await makeJpeg(100, 100);
|
||||
await cache.put('small-place', small, null);
|
||||
|
||||
const written = fs.readFileSync(filePathFor('small-place'));
|
||||
expect(written.equals(small)).toBe(true);
|
||||
});
|
||||
|
||||
it('PPC-003: falls back to original bytes when the input is not a decodable image', async () => {
|
||||
const garbage = Buffer.from('definitely not an image');
|
||||
await cache.put('garbage-place', garbage, null);
|
||||
|
||||
const written = fs.readFileSync(filePathFor('garbage-place'));
|
||||
expect(written.equals(garbage)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('placePhotoCache.removeIfUnreferenced()', () => {
|
||||
it('PPC-004: removes a cache entry that no place references', async () => {
|
||||
await cache.put('orphan', await makeJpeg(50, 50), null);
|
||||
expect(fs.existsSync(filePathFor('orphan'))).toBe(true);
|
||||
|
||||
cache.removeIfUnreferenced('orphan');
|
||||
|
||||
expect(fs.existsSync(filePathFor('orphan'))).toBe(false);
|
||||
expect(testDb.prepare('SELECT 1 FROM google_place_photo_meta WHERE place_id = ?').get('orphan')).toBeUndefined();
|
||||
});
|
||||
|
||||
it('PPC-005: keeps an entry still referenced by google_place_id', async () => {
|
||||
await cache.put('gid-1', await makeJpeg(50, 50), null);
|
||||
testDb.prepare('INSERT INTO places (google_place_id) VALUES (?)').run('gid-1');
|
||||
|
||||
cache.removeIfUnreferenced('gid-1');
|
||||
|
||||
expect(fs.existsSync(filePathFor('gid-1'))).toBe(true);
|
||||
});
|
||||
|
||||
it('PPC-006: keeps an entry referenced by a coords proxy URL in image_url', async () => {
|
||||
const id = 'coords:48.8:2.3';
|
||||
await cache.put(id, await makeJpeg(50, 50), null);
|
||||
const proxy = `/api/maps/place-photo/${encodeURIComponent(id)}/bytes`;
|
||||
testDb.prepare('INSERT INTO places (image_url) VALUES (?)').run(proxy);
|
||||
|
||||
cache.removeIfUnreferenced(id);
|
||||
|
||||
expect(fs.existsSync(filePathFor(id))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('placePhotoCache.sweepOrphans()', () => {
|
||||
it('PPC-007: removes orphaned meta rows + files, keeps referenced ones, deletes stray files', async () => {
|
||||
await cache.put('keep-gid', await makeJpeg(50, 50), null);
|
||||
await cache.put('drop-me', await makeJpeg(50, 50), null);
|
||||
testDb.prepare('INSERT INTO places (google_place_id) VALUES (?)').run('keep-gid');
|
||||
|
||||
// A stray .jpg on disk with no meta row (e.g. a crash between write and upsert).
|
||||
const strayPath = path.join(TMP_DIR, 'deadbeef'.padEnd(40, '0') + '.jpg');
|
||||
fs.writeFileSync(strayPath, 'stray');
|
||||
|
||||
const removed = cache.sweepOrphans();
|
||||
|
||||
expect(fs.existsSync(filePathFor('keep-gid'))).toBe(true);
|
||||
expect(fs.existsSync(filePathFor('drop-me'))).toBe(false);
|
||||
expect(fs.existsSync(strayPath)).toBe(false);
|
||||
expect(testDb.prepare('SELECT 1 FROM google_place_photo_meta WHERE place_id = ?').get('drop-me')).toBeUndefined();
|
||||
expect(testDb.prepare('SELECT 1 FROM google_place_photo_meta WHERE place_id = ?').get('keep-gid')).toBeDefined();
|
||||
expect(removed).toBe(2); // drop-me (orphan meta+file) + stray file
|
||||
});
|
||||
|
||||
it('PPC-008: returns 0 when every entry is referenced', async () => {
|
||||
await cache.put('ref-a', await makeJpeg(50, 50), null);
|
||||
testDb.prepare('INSERT INTO places (google_place_id) VALUES (?)').run('ref-a');
|
||||
|
||||
expect(cache.sweepOrphans()).toBe(0);
|
||||
expect(fs.existsSync(filePathFor('ref-a'))).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -41,6 +41,14 @@ vi.mock('../../../src/config', () => ({
|
||||
updateJwtSecret: () => {},
|
||||
}));
|
||||
|
||||
// Spy on the photo-cache reclaim hook so delete tests assert the wiring without
|
||||
// touching disk. The removal logic itself is covered in placePhotoCache.test.ts.
|
||||
const { removeIfUnreferencedSpy } = vi.hoisted(() => ({ removeIfUnreferencedSpy: vi.fn() }));
|
||||
vi.mock('../../../src/services/placePhotoCache', async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import('../../../src/services/placePhotoCache')>()),
|
||||
removeIfUnreferenced: removeIfUnreferencedSpy,
|
||||
}));
|
||||
|
||||
import { createTables } from '../../../src/db/schema';
|
||||
import { runMigrations } from '../../../src/db/migrations';
|
||||
import { resetTestDb } from '../../helpers/test-db';
|
||||
@@ -252,6 +260,18 @@ describe('deletePlace', () => {
|
||||
expect(remaining).toHaveLength(1);
|
||||
expect(remaining[0].id).toBe(p1.id);
|
||||
});
|
||||
|
||||
it('PLACE-SVC-019b — reclaims the photo cache for the deleted place', () => {
|
||||
removeIfUnreferencedSpy.mockClear();
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id, { name: 'With Photo' }) as any;
|
||||
testDb.prepare('UPDATE places SET google_place_id = ? WHERE id = ?').run('ChIJgid', place.id);
|
||||
|
||||
deletePlace(String(trip.id), String(place.id));
|
||||
|
||||
expect(removeIfUnreferencedSpy).toHaveBeenCalledWith('ChIJgid');
|
||||
});
|
||||
});
|
||||
|
||||
// ── importGpx ─────────────────────────────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user