diff --git a/server/src/app.ts b/server/src/app.ts index 39ffadd..a8a41fa 100644 --- a/server/src/app.ts +++ b/server/src/app.ts @@ -8,6 +8,7 @@ import { fileURLToPath } from 'node:url' import type { Config } from './config.js' import { SerialQueue } from './queue.js' import { SyncManager } from './sync.js' +import { ReleaseCache } from './releaseCache.js' import { registerAuthRoutes } from './routes/authRoutes.js' import { registerSettingsRoutes } from './routes/settingsRoutes.js' import { registerLookupRoutes } from './routes/lookupRoutes.js' @@ -21,6 +22,7 @@ declare module 'fastify' { fetchImpl: typeof fetch discogsQueue: SerialQueue sync: SyncManager + releaseCache: ReleaseCache } interface FastifyRequest { user?: import('./auth.js').UserRow @@ -42,6 +44,7 @@ export async function buildApp(opts: AppOptions): Promise { app.decorate('fetchImpl', opts.fetchImpl ?? fetch) app.decorate('discogsQueue', new SerialQueue({ minIntervalMs: 1100 })) app.decorate('sync', new SyncManager(opts.db, opts.fetchImpl ?? fetch)) + app.decorate('releaseCache', new ReleaseCache(path.join(opts.config.dataDir, 'release-cache'))) await app.register(cookie) await registerAuthRoutes(app) @@ -66,7 +69,7 @@ export async function buildApp(opts: AppOptions): Promise { app.setNotFoundHandler((request, reply) => { const url = request.raw.url ?? '' - if (url.startsWith('/api')) { + if (url.startsWith('/api') || url.startsWith('/artwork')) { return reply.code(404).send({ error: 'not_found' }) } if (hasWeb) { diff --git a/server/src/discogs.ts b/server/src/discogs.ts index 16722dc..f6a7fc4 100644 --- a/server/src/discogs.ts +++ b/server/src/discogs.ts @@ -112,7 +112,6 @@ export class DiscogsClient { const doFetch = async (): Promise => { const url = new URL(`${this.baseUrl}${path}`) for (const [k, v] of Object.entries(params)) url.searchParams.set(k, v) - url.searchParams.set('token', this.token) let res: Response try { res = await this.fetchImpl(url.toString(), { diff --git a/server/src/releaseCache.ts b/server/src/releaseCache.ts new file mode 100644 index 0000000..b5feb23 --- /dev/null +++ b/server/src/releaseCache.ts @@ -0,0 +1,31 @@ +import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs' +import path from 'node:path' +import type { DiscogsReleaseFull } from './discogs.js' + +/** + * Disk cache for full Discogs release payloads, keyed by release id. + * Cache read/write failures are best-effort; fetcher errors propagate so + * route-level Discogs error mapping still applies. + */ +export class ReleaseCache { + constructor(private dir: string) {} + + async get(id: number, fetcher: () => Promise): Promise { + const file = path.join(this.dir, `${id}.json`) + if (existsSync(file)) { + try { + return JSON.parse(readFileSync(file, 'utf8')) as DiscogsReleaseFull + } catch { + // unreadable/corrupt — refetch below + } + } + const release = await fetcher() + try { + mkdirSync(this.dir, { recursive: true }) + writeFileSync(file, JSON.stringify(release)) + } catch { + // best-effort persistence + } + return release + } +} diff --git a/server/src/ripstatus.ts b/server/src/ripstatus.ts index d30b7e7..ba3fffd 100644 --- a/server/src/ripstatus.ts +++ b/server/src/ripstatus.ts @@ -1,5 +1,5 @@ import type { DB } from './db.js' -import { isConfidentMatch } from './matcher.js' +import { isConfidentMatch, normalize } from './matcher.js' export type RipStatus = 'ripped' | 'not_ripped' @@ -29,3 +29,36 @@ export function resolveRipStatus(db: DB, userId: number, itemId: number): RipSta .all(userId) as { title: string; artist: string }[] return albums.some((a) => isConfidentMatch(item, a)) ? 'ripped' : 'not_ripped' } + +/** + * Batch resolution for list views: loads albums and match links once per + * user instead of per item (GET /api/collection is otherwise O(items x albums)). + * Resolution order matches resolveRipStatus. + */ +export function resolveRipStatusBatch( + db: DB, + userId: number, + items: { id: number; title: string; artist: string; rip_override: number | null }[] +): RipStatus[] { + const albums = db + .prepare('SELECT title, artist FROM digital_albums WHERE user_id = ?') + .all(userId) as { title: string; artist: string }[] + const albumKeys = new Set( + albums.map((a) => `${normalize(a.title)}|${normalize(a.artist)}`) + ) + const linked = new Set( + ( + db.prepare('SELECT item_id FROM match_links WHERE user_id = ?').all(userId) as { + item_id: number + }[] + ).map((r) => r.item_id) + ) + return items.map((item) => { + if (item.rip_override !== null && item.rip_override !== undefined) { + return item.rip_override === 1 ? 'ripped' : 'not_ripped' + } + if (linked.has(item.id)) return 'ripped' + const key = `${normalize(item.title)}|${normalize(item.artist)}` + return albumKeys.has(key) ? 'ripped' : 'not_ripped' + }) +} diff --git a/server/src/routes/collectionRoutes.ts b/server/src/routes/collectionRoutes.ts index 429ea8c..f3cb5cc 100644 --- a/server/src/routes/collectionRoutes.ts +++ b/server/src/routes/collectionRoutes.ts @@ -1,7 +1,7 @@ import { FastifyInstance } from 'fastify' import type { DB } from '../db.js' import { cacheArtwork } from '../artwork.js' -import { resolveRipStatus } from '../ripstatus.js' +import { resolveRipStatus, resolveRipStatusBatch, type RipStatus } from '../ripstatus.js' import { requireAuth } from './authRoutes.js' import { discogsClientFor, discogsErrorStatus } from './lookupRoutes.js' @@ -25,7 +25,7 @@ interface ItemRow { date_added: string } -export function rowToItem(db: DB, row: ItemRow) { +export function rowToItem(db: DB, row: ItemRow, ripStatus?: RipStatus) { return { id: row.id, discogsReleaseId: row.discogs_release_id, @@ -42,7 +42,7 @@ export function rowToItem(db: DB, row: ItemRow) { barcodes: JSON.parse(row.barcodes), dateAdded: row.date_added, ripOverride: row.rip_override === null ? null : row.rip_override === 1, - ripStatus: resolveRipStatus(db, row.user_id, row.id), + ripStatus: ripStatus ?? resolveRipStatus(db, row.user_id, row.id), } } @@ -75,7 +75,7 @@ export async function registerCollectionRoutes(app: FastifyInstance): Promise client.getRelease(releaseId)) } catch (err) { const { code, body } = discogsErrorStatus(err) return reply.code(code).send(body) @@ -137,7 +137,8 @@ export async function registerCollectionRoutes(app: FastifyInstance): Promise rowToItem(db, row)) + const statuses = resolveRipStatusBatch(db, userId, rows) + const items = rows.map((row, i) => rowToItem(db, row, statuses[i])) const counts = { total: items.length, diff --git a/server/src/routes/lookupRoutes.ts b/server/src/routes/lookupRoutes.ts index 8654ed5..8634927 100644 --- a/server/src/routes/lookupRoutes.ts +++ b/server/src/routes/lookupRoutes.ts @@ -60,8 +60,9 @@ export async function registerLookupRoutes(app: FastifyInstance): Promise const client = discogsClientFor(request) if (!client) return reply.code(409).send({ error: 'no_discogs_token' }) const id = Number((request.params as { id: string }).id) + if (!Number.isInteger(id)) return reply.code(400).send({ error: 'invalid_input' }) try { - const release = await client.getRelease(id) + const release = await request.server.releaseCache.get(id, () => client.getRelease(id)) const db = request.server.db const userId = request.user!.id const duplicate = !!db diff --git a/server/test/discogs.test.ts b/server/test/discogs.test.ts index d31bbd2..cc1ca0c 100644 --- a/server/test/discogs.test.ts +++ b/server/test/discogs.test.ts @@ -35,7 +35,6 @@ describe('DiscogsClient', () => { expect(seen[0]).toContain('/database/search') expect(seen[0]).toContain('barcode=5021592210629') expect(seen[0]).toContain('type=release') - expect(seen[0]).toContain('token=testtoken') expect(seenAuth[0]).toBe('Discogs token=testtoken') expect(results).toHaveLength(2) expect(results[0]).toEqual({ diff --git a/server/test/helpers.ts b/server/test/helpers.ts index 9e640eb..f4d0fcf 100644 --- a/server/test/helpers.ts +++ b/server/test/helpers.ts @@ -9,8 +9,10 @@ import type { FastifyInstance } from 'fastify' export function testConfig(): Config { // artworkDir must be a real writable directory (cached images land there) const artworkDir = mkdtempSync(path.join(tmpdir(), 'rs-art-')) + // dataDir must be a real writable directory (release-cache payloads land there) + const dataDir = mkdtempSync(path.join(tmpdir(), 'rs-data-')) return { - dataDir: ':memory:', + dataDir, artworkDir, dbPath: ':memory:', port: 0, diff --git a/server/test/lookup.test.ts b/server/test/lookup.test.ts index 8bb9115..6608f74 100644 --- a/server/test/lookup.test.ts +++ b/server/test/lookup.test.ts @@ -126,6 +126,34 @@ describe('GET /api/lookup/release/:id', () => { await app.close() }) + it('caches release payloads on disk (second lookup costs no discogs call)', async () => { + let releaseCalls = 0 + const { app, cookie } = await appWithToken( + stubFetch((url) => { + if (url.includes('/releases/1001')) { + releaseCalls++ + return new Response(JSON.stringify(discogsReleaseFixture), { + status: 200, + headers: { 'content-type': 'application/json' }, + }) + } + return new Response('nope', { status: 404 }) + }) + ) + await app.inject({ method: 'GET', url: '/api/lookup/release/1001', ...auth(cookie) }) + await app.inject({ method: 'GET', url: '/api/lookup/release/1001', ...auth(cookie) }) + expect(releaseCalls).toBe(1) + await app.close() + }) + + it('rejects non-integer release ids with 400', async () => { + const { app, cookie } = await appWithToken(discogsStub()) + const res = await app.inject({ method: 'GET', url: '/api/lookup/release/abc', ...auth(cookie) }) + expect(res.statusCode).toBe(400) + expect(res.json()).toEqual({ error: 'invalid_input' }) + await app.close() + }) + it('reports duplicate when release already in collection', async () => { const { app, cookie } = await appWithToken(discogsStub()) await app.inject({ method: 'POST', url: '/api/collection', ...auth(cookie), payload: { releaseId: 1001 } }) diff --git a/server/test/static.test.ts b/server/test/static.test.ts index 1594322..7448de4 100644 --- a/server/test/static.test.ts +++ b/server/test/static.test.ts @@ -50,6 +50,14 @@ describe('static serving', () => { await app.close() }) + it('unknown artwork paths return json 404, not the SPA', async () => { + const app = await build() + const res = await app.inject({ method: 'GET', url: '/artwork/missing.jpg' }) + expect(res.statusCode).toBe(404) + expect(res.json()).toEqual({ error: 'not_found' }) + await app.close() + }) + it('unknown api routes return json 404, not the SPA', async () => { const app = await build() const res = await app.inject({ method: 'GET', url: '/api/nope' })