feat: release payload disk cache, batched rip status for list, artwork 404, stricter input handling
This commit is contained in:
@@ -8,6 +8,7 @@ import { fileURLToPath } from 'node:url'
|
|||||||
import type { Config } from './config.js'
|
import type { Config } from './config.js'
|
||||||
import { SerialQueue } from './queue.js'
|
import { SerialQueue } from './queue.js'
|
||||||
import { SyncManager } from './sync.js'
|
import { SyncManager } from './sync.js'
|
||||||
|
import { ReleaseCache } from './releaseCache.js'
|
||||||
import { registerAuthRoutes } from './routes/authRoutes.js'
|
import { registerAuthRoutes } from './routes/authRoutes.js'
|
||||||
import { registerSettingsRoutes } from './routes/settingsRoutes.js'
|
import { registerSettingsRoutes } from './routes/settingsRoutes.js'
|
||||||
import { registerLookupRoutes } from './routes/lookupRoutes.js'
|
import { registerLookupRoutes } from './routes/lookupRoutes.js'
|
||||||
@@ -21,6 +22,7 @@ declare module 'fastify' {
|
|||||||
fetchImpl: typeof fetch
|
fetchImpl: typeof fetch
|
||||||
discogsQueue: SerialQueue
|
discogsQueue: SerialQueue
|
||||||
sync: SyncManager
|
sync: SyncManager
|
||||||
|
releaseCache: ReleaseCache
|
||||||
}
|
}
|
||||||
interface FastifyRequest {
|
interface FastifyRequest {
|
||||||
user?: import('./auth.js').UserRow
|
user?: import('./auth.js').UserRow
|
||||||
@@ -42,6 +44,7 @@ export async function buildApp(opts: AppOptions): Promise<FastifyInstance> {
|
|||||||
app.decorate('fetchImpl', opts.fetchImpl ?? fetch)
|
app.decorate('fetchImpl', opts.fetchImpl ?? fetch)
|
||||||
app.decorate('discogsQueue', new SerialQueue({ minIntervalMs: 1100 }))
|
app.decorate('discogsQueue', new SerialQueue({ minIntervalMs: 1100 }))
|
||||||
app.decorate('sync', new SyncManager(opts.db, opts.fetchImpl ?? fetch))
|
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 app.register(cookie)
|
||||||
await registerAuthRoutes(app)
|
await registerAuthRoutes(app)
|
||||||
@@ -66,7 +69,7 @@ export async function buildApp(opts: AppOptions): Promise<FastifyInstance> {
|
|||||||
|
|
||||||
app.setNotFoundHandler((request, reply) => {
|
app.setNotFoundHandler((request, reply) => {
|
||||||
const url = request.raw.url ?? ''
|
const url = request.raw.url ?? ''
|
||||||
if (url.startsWith('/api')) {
|
if (url.startsWith('/api') || url.startsWith('/artwork')) {
|
||||||
return reply.code(404).send({ error: 'not_found' })
|
return reply.code(404).send({ error: 'not_found' })
|
||||||
}
|
}
|
||||||
if (hasWeb) {
|
if (hasWeb) {
|
||||||
|
|||||||
@@ -112,7 +112,6 @@ export class DiscogsClient {
|
|||||||
const doFetch = async (): Promise<any> => {
|
const doFetch = async (): Promise<any> => {
|
||||||
const url = new URL(`${this.baseUrl}${path}`)
|
const url = new URL(`${this.baseUrl}${path}`)
|
||||||
for (const [k, v] of Object.entries(params)) url.searchParams.set(k, v)
|
for (const [k, v] of Object.entries(params)) url.searchParams.set(k, v)
|
||||||
url.searchParams.set('token', this.token)
|
|
||||||
let res: Response
|
let res: Response
|
||||||
try {
|
try {
|
||||||
res = await this.fetchImpl(url.toString(), {
|
res = await this.fetchImpl(url.toString(), {
|
||||||
|
|||||||
31
server/src/releaseCache.ts
Normal file
31
server/src/releaseCache.ts
Normal file
@@ -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<DiscogsReleaseFull>): Promise<DiscogsReleaseFull> {
|
||||||
|
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
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -1,5 +1,5 @@
|
|||||||
import type { DB } from './db.js'
|
import type { DB } from './db.js'
|
||||||
import { isConfidentMatch } from './matcher.js'
|
import { isConfidentMatch, normalize } from './matcher.js'
|
||||||
|
|
||||||
export type RipStatus = 'ripped' | 'not_ripped'
|
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 }[]
|
.all(userId) as { title: string; artist: string }[]
|
||||||
return albums.some((a) => isConfidentMatch(item, a)) ? 'ripped' : 'not_ripped'
|
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'
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
import { FastifyInstance } from 'fastify'
|
import { FastifyInstance } from 'fastify'
|
||||||
import type { DB } from '../db.js'
|
import type { DB } from '../db.js'
|
||||||
import { cacheArtwork } from '../artwork.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 { requireAuth } from './authRoutes.js'
|
||||||
import { discogsClientFor, discogsErrorStatus } from './lookupRoutes.js'
|
import { discogsClientFor, discogsErrorStatus } from './lookupRoutes.js'
|
||||||
|
|
||||||
@@ -25,7 +25,7 @@ interface ItemRow {
|
|||||||
date_added: string
|
date_added: string
|
||||||
}
|
}
|
||||||
|
|
||||||
export function rowToItem(db: DB, row: ItemRow) {
|
export function rowToItem(db: DB, row: ItemRow, ripStatus?: RipStatus) {
|
||||||
return {
|
return {
|
||||||
id: row.id,
|
id: row.id,
|
||||||
discogsReleaseId: row.discogs_release_id,
|
discogsReleaseId: row.discogs_release_id,
|
||||||
@@ -42,7 +42,7 @@ export function rowToItem(db: DB, row: ItemRow) {
|
|||||||
barcodes: JSON.parse(row.barcodes),
|
barcodes: JSON.parse(row.barcodes),
|
||||||
dateAdded: row.date_added,
|
dateAdded: row.date_added,
|
||||||
ripOverride: row.rip_override === null ? null : row.rip_override === 1,
|
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<vo
|
|||||||
|
|
||||||
let release
|
let release
|
||||||
try {
|
try {
|
||||||
release = await client.getRelease(releaseId)
|
release = await request.server.releaseCache.get(releaseId, () => client.getRelease(releaseId))
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
const { code, body } = discogsErrorStatus(err)
|
const { code, body } = discogsErrorStatus(err)
|
||||||
return reply.code(code).send(body)
|
return reply.code(code).send(body)
|
||||||
@@ -137,7 +137,8 @@ export async function registerCollectionRoutes(app: FastifyInstance): Promise<vo
|
|||||||
const rows = db
|
const rows = db
|
||||||
.prepare('SELECT * FROM collection_items WHERE user_id = ? ORDER BY date_added DESC, id DESC')
|
.prepare('SELECT * FROM collection_items WHERE user_id = ? ORDER BY date_added DESC, id DESC')
|
||||||
.all(userId) as ItemRow[]
|
.all(userId) as ItemRow[]
|
||||||
const items = rows.map((row) => rowToItem(db, row))
|
const statuses = resolveRipStatusBatch(db, userId, rows)
|
||||||
|
const items = rows.map((row, i) => rowToItem(db, row, statuses[i]))
|
||||||
|
|
||||||
const counts = {
|
const counts = {
|
||||||
total: items.length,
|
total: items.length,
|
||||||
|
|||||||
@@ -60,8 +60,9 @@ export async function registerLookupRoutes(app: FastifyInstance): Promise<void>
|
|||||||
const client = discogsClientFor(request)
|
const client = discogsClientFor(request)
|
||||||
if (!client) return reply.code(409).send({ error: 'no_discogs_token' })
|
if (!client) return reply.code(409).send({ error: 'no_discogs_token' })
|
||||||
const id = Number((request.params as { id: string }).id)
|
const id = Number((request.params as { id: string }).id)
|
||||||
|
if (!Number.isInteger(id)) return reply.code(400).send({ error: 'invalid_input' })
|
||||||
try {
|
try {
|
||||||
const release = await client.getRelease(id)
|
const release = await request.server.releaseCache.get(id, () => client.getRelease(id))
|
||||||
const db = request.server.db
|
const db = request.server.db
|
||||||
const userId = request.user!.id
|
const userId = request.user!.id
|
||||||
const duplicate = !!db
|
const duplicate = !!db
|
||||||
|
|||||||
@@ -35,7 +35,6 @@ describe('DiscogsClient', () => {
|
|||||||
expect(seen[0]).toContain('/database/search')
|
expect(seen[0]).toContain('/database/search')
|
||||||
expect(seen[0]).toContain('barcode=5021592210629')
|
expect(seen[0]).toContain('barcode=5021592210629')
|
||||||
expect(seen[0]).toContain('type=release')
|
expect(seen[0]).toContain('type=release')
|
||||||
expect(seen[0]).toContain('token=testtoken')
|
|
||||||
expect(seenAuth[0]).toBe('Discogs token=testtoken')
|
expect(seenAuth[0]).toBe('Discogs token=testtoken')
|
||||||
expect(results).toHaveLength(2)
|
expect(results).toHaveLength(2)
|
||||||
expect(results[0]).toEqual({
|
expect(results[0]).toEqual({
|
||||||
|
|||||||
@@ -9,8 +9,10 @@ import type { FastifyInstance } from 'fastify'
|
|||||||
export function testConfig(): Config {
|
export function testConfig(): Config {
|
||||||
// artworkDir must be a real writable directory (cached images land there)
|
// artworkDir must be a real writable directory (cached images land there)
|
||||||
const artworkDir = mkdtempSync(path.join(tmpdir(), 'rs-art-'))
|
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 {
|
return {
|
||||||
dataDir: ':memory:',
|
dataDir,
|
||||||
artworkDir,
|
artworkDir,
|
||||||
dbPath: ':memory:',
|
dbPath: ':memory:',
|
||||||
port: 0,
|
port: 0,
|
||||||
|
|||||||
@@ -126,6 +126,34 @@ describe('GET /api/lookup/release/:id', () => {
|
|||||||
await app.close()
|
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 () => {
|
it('reports duplicate when release already in collection', async () => {
|
||||||
const { app, cookie } = await appWithToken(discogsStub())
|
const { app, cookie } = await appWithToken(discogsStub())
|
||||||
await app.inject({ method: 'POST', url: '/api/collection', ...auth(cookie), payload: { releaseId: 1001 } })
|
await app.inject({ method: 'POST', url: '/api/collection', ...auth(cookie), payload: { releaseId: 1001 } })
|
||||||
|
|||||||
@@ -50,6 +50,14 @@ describe('static serving', () => {
|
|||||||
await app.close()
|
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 () => {
|
it('unknown api routes return json 404, not the SPA', async () => {
|
||||||
const app = await build()
|
const app = await build()
|
||||||
const res = await app.inject({ method: 'GET', url: '/api/nope' })
|
const res = await app.inject({ method: 'GET', url: '/api/nope' })
|
||||||
|
|||||||
Reference in New Issue
Block a user