From 29c78641c89610ba25dda188d650e433c3c252b0 Mon Sep 17 00:00:00 2001 From: eleith Date: Thu, 5 Nov 2026 15:04:18 -0800 Subject: [PATCH] feat(analytics): record request-time visit outcomes --- .changeset/visit-outcomes.md | 6 + apps/web/e2e/visits.e2e.ts | 22 +++- apps/web/src/lib/server/dispatch.test.ts | 32 ++++- apps/web/src/lib/server/dispatch.ts | 4 +- apps/web/src/lib/server/redirect.test.ts | 71 +++++++---- apps/web/src/lib/server/redirect.ts | 112 +++++++++++++----- apps/web/src/lib/server/visits/capture.ts | 3 +- .../src/lib/server/visits/recorder.test.ts | 1 + apps/web/src/lib/server/visits/recorder.ts | 9 +- apps/worker/e2e/startup.js | 1 + apps/worker/src/services/retention.test.ts | 5 +- apps/worker/src/services/startup.test.ts | 1 + packages/db/src/index.ts | 2 +- .../db/src/migrations/0005_visit_outcome.ts | 10 ++ packages/db/src/migrations/index.ts | 4 +- packages/db/src/storage.test.ts | 16 ++- packages/db/src/types.ts | 3 + 17 files changed, 231 insertions(+), 71 deletions(-) create mode 100644 .changeset/visit-outcomes.md create mode 100644 packages/db/src/migrations/0005_visit_outcome.ts diff --git a/.changeset/visit-outcomes.md b/.changeset/visit-outcomes.md new file mode 100644 index 0000000..f7a7e63 --- /dev/null +++ b/.changeset/visit-outcomes.md @@ -0,0 +1,6 @@ +--- +'@goto/web': minor +'@goto/worker': minor +--- + +Record request-time valid and invalid visit outcomes for known redirect domains, including base URLs and inactive or unmatched paths. Preserve existing visits as valid without consulting current YAML, while keeping HEAD, malformed requests, unknown hosts, and disabled analytics untracked. diff --git a/apps/web/e2e/visits.e2e.ts b/apps/web/e2e/visits.e2e.ts index ba653f4..1961da1 100644 --- a/apps/web/e2e/visits.e2e.ts +++ b/apps/web/e2e/visits.e2e.ts @@ -18,7 +18,7 @@ function visitCount(path: string, logicalPath: string): number { } } -test('the compiled writer isolates lock waits and drains accepted visits before shutdown', async ({ +test('the compiled writer persists outcomes, isolates locks and drains visits before shutdown', async ({ request, appServer }) => { @@ -55,13 +55,29 @@ test('the compiled writer isolates lock waits and drains accepted visits before competingWriter.exec('ROLLBACK'); competingWriter.close(); } + const invalid = await request.get('/missing', options); + const base = await request.get('/', options); + const head = await request.head('/missing', options); + expect(invalid.status()).toBe(404); + expect(base.status()).toBe(404); + expect(head.status()).toBe(404); expect(await appServer.stop()).toEqual({ code: 0, signal: null }); expect(visitCount(databasePath, '/docs')).toBe(3); const db = new DatabaseSync(databasePath, { readOnly: true }); try { expect( - db.prepare('SELECT DISTINCT browser, os FROM visits WHERE path = ?').all('/docs') - ).toEqual([{ browser: 'Firefox', os: 'Linux' }]); + db.prepare('SELECT DISTINCT outcome, browser, os FROM visits WHERE path = ?').all('/docs') + ).toEqual([{ outcome: 'valid', browser: 'Firefox', os: 'Linux' }]); + expect( + db + .prepare( + 'SELECT path, outcome FROM visits WHERE domain = ? AND path IN (?, ?) ORDER BY path' + ) + .all('go.example.org', '/', '/missing') + ).toEqual([ + { path: '/', outcome: 'invalid' }, + { path: '/missing', outcome: 'invalid' } + ]); } finally { db.close(); } diff --git a/apps/web/src/lib/server/dispatch.test.ts b/apps/web/src/lib/server/dispatch.test.ts index 96f84d7..886b2b4 100644 --- a/apps/web/src/lib/server/dispatch.test.ts +++ b/apps/web/src/lib/server/dispatch.test.ts @@ -101,7 +101,7 @@ test('rejects malformed ingress without reaching either handler', () => { expect(requestApplication(incoming)).toBeUndefined(); }); -test('captures only raw visit fields from the matched logical path and trusted IP header', () => { +test('captures the request outcome, logical path and raw fields using the trusted IP header', () => { const recordVisit = vi.fn(); const tracked = { ...application, recordVisit }; const incoming = request('go.example.test', '/%73chema.json?private=not-collected'); @@ -117,6 +117,7 @@ test('captures only raw visit fields from the matched logical path and trusted I expect(recordVisit).toHaveBeenCalledExactlyOnceWith({ domain: 'go.example.test', path: '/schema.json', + outcome: 'valid', visited_at: expect.any(Number), ip: '2001:db8::1', user_agent: 'Example agent', @@ -124,7 +125,7 @@ test('captures only raw visit fields from the matched logical path and trusted I }); }); -test('does not record HEAD, fallback redirects, or visits when analytics is disabled', () => { +test('captures invalid decoded paths even when the fallback redirects', () => { const recordVisit = vi.fn(); const fallbackSnapshot = { ...snapshot, @@ -137,10 +138,28 @@ test('does not record HEAD, fallback redirects, or visits when analytics is disa } }; const tracked = { currentSnapshot: () => fallbackSnapshot, recordVisit }; + const incoming = request('go.example.test', '/%6dissing?private=not-collected'); + expect(handleIngress(tracked, incoming, {})?.status).toBe(302); + expect(recordVisit).toHaveBeenCalledExactlyOnceWith({ + domain: 'go.example.test', + path: '/missing', + outcome: 'invalid', + visited_at: expect.any(Number), + ip: '127.0.0.1', + user_agent: null, + accept_language: null + }); +}); + +test('does not record HEAD, malformed, admin, unknown-host or disabled-analytics requests', () => { + const recordVisit = vi.fn(); + const tracked = { ...application, recordVisit }; const head = request('go.example.test'); head.method = 'HEAD'; expect(handleIngress(tracked, head, {})?.status).toBe(302); - expect(handleIngress(tracked, request('go.example.test', '/missing'), {})?.status).toBe(302); + expect(handleIngress(tracked, request('go.example.test', '/%'), {})?.status).toBe(400); + expect(handleIngress(tracked, request('unknown.example.test'), {})?.status).toBe(404); + expect(handleIngress(tracked, request('admin.example.test'), {})).toBeUndefined(); const disabled = { ...snapshot, configuration: { @@ -152,6 +171,13 @@ test('does not record HEAD, fallback redirects, or visits when analytics is disa handleIngress({ currentSnapshot: () => disabled, recordVisit }, request('go.example.test'), {}) ?.status ).toBe(302); + expect( + handleIngress( + { currentSnapshot: () => disabled, recordVisit }, + request('go.example.test', '/missing'), + {} + )?.status + ).toBe(404); expect(recordVisit).not.toHaveBeenCalled(); }); diff --git a/apps/web/src/lib/server/dispatch.ts b/apps/web/src/lib/server/dispatch.ts index df72d34..e13beea 100644 --- a/apps/web/src/lib/server/dispatch.ts +++ b/apps/web/src/lib/server/dispatch.ts @@ -3,7 +3,7 @@ import type { Application } from './application.js'; import { isLocalProbe, readIngress } from './ingress.js'; import { resolveRedirect } from './redirect.js'; import { bindApplication } from './request-context.js'; -import { recordRedirectVisit } from './visits/capture.js'; +import { captureVisit } from './visits/capture.js'; function response(status: number): Response { return new Response(null, { status, headers: { 'cache-control': 'no-store' } }); @@ -26,7 +26,7 @@ export function handleIngress( request.method ?? 'GET' ); if (snapshot.configuration.analytics.retention_months !== 0) { - recordRedirectVisit(request, redirect.visit, application.recordVisit); + captureVisit(request, redirect.visit, application.recordVisit); } return redirect.response; } diff --git a/apps/web/src/lib/server/redirect.test.ts b/apps/web/src/lib/server/redirect.test.ts index c3a729a..cb3713f 100644 --- a/apps/web/src/lib/server/redirect.test.ts +++ b/apps/web/src/lib/server/redirect.test.ts @@ -55,10 +55,11 @@ describe('exact single-decode lookup', () => { ['/100%25', '/100%'], ['/docs?next=/missing?part=two', '/docs'] ])('%s resolves logical key %s', (target, path) => { - const { response } = resolveRedirect(domain, host, target, 'GET'); + const { response, visit } = resolveRedirect(domain, host, target, 'GET'); expect(response.status).toBe(302); expect(response.headers.get('location')).toBe(domain.links[path]?.url); + expect(visit).toMatchObject({ path, outcome: 'valid' }); }); test.each([ @@ -71,9 +72,10 @@ describe('exact single-decode lookup', () => { '/docs%3Fignored=one', '/docs%23fragment' ])('%s is not normalized, prefix-matched or reparsed', (target) => { - const { response } = resolveRedirect(domain, host, target, 'GET'); + const { response, visit } = resolveRedirect(domain, host, target, 'GET'); expect(response.status).toBe(404); + expect(visit?.outcome).toBe('invalid'); expect(response.headers.has('location')).toBe(false); expect(response.headers.get('cache-control')).toBe('no-store'); }); @@ -131,9 +133,10 @@ describe('HTTP responses', () => { 'rejects %s without resolving a link or fallback', (method) => { const domain = preparedDomain({ '/docs': preparedLink() }, fallback); - const { response } = resolveRedirect(domain, host, '/docs', method); + const { response, visit } = resolveRedirect(domain, host, '/docs', method); expect(response.status).toBe(405); + expect(visit).toBeUndefined(); expect(response.headers.get('allow')).toBe('GET, HEAD'); expect(response.headers.get('cache-control')).toBe('no-store'); expect(response.headers.has('location')).toBe(false); @@ -161,9 +164,11 @@ describe('HTTP responses', () => { '/%C3%28', '/%FF' ])('rejects malformed raw target %j before using fallback', (target) => { - const { response } = resolveRedirect(preparedDomain({}, fallback), host, target, 'GET'); + const domain = preparedDomain({}, fallback); + const { response, visit } = resolveRedirect(domain, host, target, 'GET'); expect(response.status).toBe(400); + expect(visit).toBeUndefined(); expect(response.headers.get('cache-control')).toBe('no-store'); expect(response.headers.has('location')).toBe(false); }); @@ -197,13 +202,13 @@ describe('validity and fallback', () => { const end = Date.parse(until); test.each([ - { enabled: true, now: start - 1, status: 302, location: fallback }, - { enabled: true, now: start, status: 307, location: destination }, - { enabled: true, now: end - 1, status: 307, location: destination }, - { enabled: true, now: end, status: 302, location: fallback }, - { enabled: true, now: end + 1, status: 302, location: fallback }, - { enabled: false, now: start, status: 302, location: fallback } - ])('enabled=$enabled at $now returns $status', ({ enabled, now, status, location }) => { + { enabled: true, now: start - 1, status: 302, location: fallback, outcome: 'invalid' }, + { enabled: true, now: start, status: 307, location: destination, outcome: 'valid' }, + { enabled: true, now: end - 1, status: 307, location: destination, outcome: 'valid' }, + { enabled: true, now: end, status: 302, location: fallback, outcome: 'invalid' }, + { enabled: true, now: end + 1, status: 302, location: fallback, outcome: 'invalid' }, + { enabled: false, now: start, status: 302, location: fallback, outcome: 'invalid' } + ])('enabled=$enabled at $now returns $status', ({ enabled, now, status, location, outcome }) => { const domain = preparedDomain( { '/docs': preparedLink({ @@ -215,13 +220,24 @@ describe('validity and fallback', () => { }, fallback ); - const { response } = resolveRedirect(domain, host, '/docs?incoming=ignored', 'GET', now); + const { response, visit } = resolveRedirect(domain, host, '/docs?incoming=ignored', 'GET', now); expect(response.status).toBe(status); + expect(visit).toEqual({ domain: host, path: '/docs', outcome, visitedAt: now }); expect(response.headers.get('location')).toBe(location); expect(response.headers.get('cache-control')).toBe('no-store'); }); + test('visit outcomes do not change with later link configuration', () => { + const link = preparedLink(); + const domain = preparedDomain({ '/docs': link }, fallback); + const first = resolveRedirect(domain, host, '/docs', 'GET'); + link.enabled = false; + const next = resolveRedirect(domain, host, '/docs', 'GET'); + expect(first.visit?.outcome).toBe('valid'); + expect(next.visit?.outcome).toBe('invalid'); + }); + test('uses Date.now when no clock is supplied', () => { const clock = vi.spyOn(Date, 'now').mockReturnValue(start); try { @@ -229,7 +245,7 @@ describe('validity and fallback', () => { const { response, visit } = resolveRedirect(domain, host, '/docs', 'GET'); expect(response.status).toBe(302); - expect(visit).toEqual({ domain: host, path: '/docs', visitedAt: start }); + expect(visit).toEqual({ domain: host, path: '/docs', outcome: 'valid', visitedAt: start }); } finally { clock.mockRestore(); } @@ -238,15 +254,11 @@ describe('validity and fallback', () => { test.each([preparedLink({ enabled: false }), preparedLink({ valid_until: until })])( 'inactive links can fall back to 404', (link) => { - const { response } = resolveRedirect( - preparedDomain({ '/docs': link }), - host, - '/docs', - 'GET', - end - ); + const domain = preparedDomain({ '/docs': link }); + const { response, visit } = resolveRedirect(domain, host, '/docs', 'GET', end); expect(response.status).toBe(404); + expect(visit?.outcome).toBe('invalid'); expect(response.headers.has('location')).toBe(false); expect(response.headers.get('cache-control')).toBe('no-store'); } @@ -255,20 +267,29 @@ describe('validity and fallback', () => { test('a root link takes precedence over URL fallback', () => { const domain = preparedDomain({ '/': preparedLink() }, fallback); - expect( - resolveRedirect(domain, host, '/?incoming=ignored', 'GET').response.headers.get('location') - ).toBe(destination); + const { response, visit } = resolveRedirect(domain, host, '/?incoming=ignored', 'GET'); + expect(response.headers.get('location')).toBe(destination); + expect(visit).toMatchObject({ path: '/', outcome: 'valid' }); + }); + + test('an unconfigured root remains a base visit with an invalid outcome', () => { + const domain = preparedDomain(); + const { response, visit } = resolveRedirect(domain, host, '/', 'GET'); + expect(response.status).toBe(404); + expect(visit).toMatchObject({ path: '/', outcome: 'invalid' }); }); test('URL fallback is always 302 and never appends the incoming path or query', () => { - const { response } = resolveRedirect( - preparedDomain({}, fallback), + const domain = preparedDomain({}, fallback); + const { response, visit } = resolveRedirect( + domain, host, '/missing/nested?owned=replaced&new=value', 'GET' ); expect(response.status).toBe(302); + expect(visit).toMatchObject({ path: '/missing/nested', outcome: 'invalid' }); expect(response.headers.get('location')).toBe(fallback); expect(response.headers.get('cache-control')).toBe('no-store'); }); diff --git a/apps/web/src/lib/server/redirect.ts b/apps/web/src/lib/server/redirect.ts index f121c0c..6303dc5 100644 --- a/apps/web/src/lib/server/redirect.ts +++ b/apps/web/src/lib/server/redirect.ts @@ -1,9 +1,11 @@ -import { isSelfRedirect, type PreparedDomain } from '@goto/config'; +import { isSelfRedirect, type PreparedDomain, type PreparedLink } from '@goto/config'; +import type { VisitOutcome } from '@goto/db'; import type { Immutable } from './snapshot.js'; export interface ResolvedVisit { domain: string; path: string; + outcome: VisitOutcome; visitedAt: number; } @@ -12,7 +14,17 @@ export interface RedirectResult { visit?: ResolvedVisit; } -function parseTarget(target: string): { path: string; query: string } | undefined { +interface ParsedTarget { + path: string; + query: string; +} + +interface ResolvedResponse { + response: Response; + outcome: VisitOutcome; +} + +function parseTarget(target: string): ParsedTarget | undefined { if (!target.startsWith('/') || target.includes('#') || target.includes('\\')) return undefined; if (!target.isWellFormed()) return undefined; for (let index = 0; index < target.length; index++) { @@ -51,6 +63,61 @@ function serializeLocation(destination: string): string { return destination; } +function emptyResponse(status: number): Response { + return new Response(null, { status, headers: { 'cache-control': 'no-store' } }); +} + +function redirectResponse(destination: string, status: number): Response { + const location = serializeLocation(destination); + const headers = { 'cache-control': 'no-store', location }; + return new Response(null, { status, headers }); +} + +function findActiveLink( + domain: Immutable, + path: string, + now: number +): Immutable | undefined { + const link = Object.hasOwn(domain.links, path) ? domain.links[path] : undefined; + if (!link || !link.enabled) return; + const started = link.valid_from === undefined || now >= Date.parse(link.valid_from); + const notExpired = link.valid_until === undefined || now < Date.parse(link.valid_until); + if (started && notExpired) return link; +} + +function fallbackResponse( + fallback: PreparedDomain['fallback'], + host: string, + path: string +): Response { + if (fallback === 404) return emptyResponse(404); + if (isSelfRedirect(fallback, host, path)) return emptyResponse(404); + return redirectResponse(fallback, 302); +} + +function resolveVisitResponse( + domain: Immutable, + host: string, + target: ParsedTarget, + now: number +): ResolvedResponse { + const link = findActiveLink(domain, target.path, now); + if (!link) { + const response = fallbackResponse(domain.fallback, host, target.path); + return { response, outcome: 'invalid' }; + } + + let destination = link.url; + if (link.forward_query) destination = forwardQuery(link.url, target.query); + if (isSelfRedirect(destination, host, target.path)) { + const response = emptyResponse(404); + return { response, outcome: 'invalid' }; + } + + const response = redirectResponse(destination, link.status_code); + return { response, outcome: 'valid' }; +} + export function resolveRedirect( domain: Immutable, host: string, @@ -58,35 +125,26 @@ export function resolveRedirect( method: string, now: number = Date.now() ): RedirectResult { - const headers = new Headers({ 'cache-control': 'no-store' }); if (method !== 'GET' && method !== 'HEAD') { - headers.set('allow', 'GET, HEAD'); - return { response: new Response(null, { status: 405, headers }) }; + const response = emptyResponse(405); + response.headers.set('allow', 'GET, HEAD'); + return { response }; } const parsed = parseTarget(target); - if (parsed === undefined) return { response: new Response(null, { status: 400, headers }) }; - - const link = Object.hasOwn(domain.links, parsed.path) ? domain.links[parsed.path] : undefined; - const active = - link !== undefined && - link.enabled && - (link.valid_from === undefined || now >= Date.parse(link.valid_from)) && - (link.valid_until === undefined || now < Date.parse(link.valid_until)); - let destination = domain.fallback; - let status = 302; - if (link !== undefined && active) { - destination = link.forward_query ? forwardQuery(link.url, parsed.query) : link.url; - status = link.status_code; + if (parsed === undefined) { + const response = emptyResponse(400); + return { response }; } - if (destination === 404 || isSelfRedirect(destination, host, parsed.path)) { - return { response: new Response(null, { status: 404, headers }) }; - } - headers.set('location', serializeLocation(destination)); - const response = new Response(null, { status, headers }); - if (active && method === 'GET') { - return { response, visit: { domain: host, path: parsed.path, visitedAt: now } }; - } - return { response }; + const resolved = resolveVisitResponse(domain, host, parsed, now); + const response = resolved.response; + if (method === 'HEAD') return { response }; + const visit: ResolvedVisit = { + domain: host, + path: parsed.path, + outcome: resolved.outcome, + visitedAt: now + }; + return { response, visit }; } diff --git a/apps/web/src/lib/server/visits/capture.ts b/apps/web/src/lib/server/visits/capture.ts index efb97b8..9034cc4 100644 --- a/apps/web/src/lib/server/visits/capture.ts +++ b/apps/web/src/lib/server/visits/capture.ts @@ -3,7 +3,7 @@ import type { IncomingMessage } from 'node:http'; import { readClientAddress } from '../ingress.js'; import type { ResolvedVisit } from '../redirect.js'; -export function recordRedirectVisit( +export function captureVisit( request: IncomingMessage, visit: ResolvedVisit | undefined, recordVisit: ((visit: NewVisit) => void) | undefined @@ -19,6 +19,7 @@ export function recordRedirectVisit( recordVisit({ domain: visit.domain, path: visit.path, + outcome: visit.outcome, visited_at: visit.visitedAt, ip, user_agent: userAgent, diff --git a/apps/web/src/lib/server/visits/recorder.test.ts b/apps/web/src/lib/server/visits/recorder.test.ts index c9f5c58..1222d78 100644 --- a/apps/web/src/lib/server/visits/recorder.test.ts +++ b/apps/web/src/lib/server/visits/recorder.test.ts @@ -13,6 +13,7 @@ let writes: ReturnType>[]; const visit: NewVisit = { domain: 'go.example.test', path: '/docs', + outcome: 'valid', visited_at: 1_700_000_000_000, ip: '2001:db8::1', user_agent: null, diff --git a/apps/web/src/lib/server/visits/recorder.ts b/apps/web/src/lib/server/visits/recorder.ts index 33a092a..f22dcb0 100644 --- a/apps/web/src/lib/server/visits/recorder.ts +++ b/apps/web/src/lib/server/visits/recorder.ts @@ -21,7 +21,14 @@ export function createVisitRecorder(databasePath: string): VisitRecorder { } function exceedsVisitSizeLimit(visit: NewVisit): boolean { - const textFields = [visit.domain, visit.path, visit.ip, visit.user_agent, visit.accept_language]; + const textFields = [ + visit.domain, + visit.path, + visit.outcome, + visit.ip, + visit.user_agent, + visit.accept_language + ]; let bytes = 8; for (const value of textFields) { bytes += Buffer.byteLength(value ?? ''); diff --git a/apps/worker/e2e/startup.js b/apps/worker/e2e/startup.js index a44f7fc..f2348c3 100644 --- a/apps/worker/e2e/startup.js +++ b/apps/worker/e2e/startup.js @@ -13,6 +13,7 @@ import { insertVisit, openDb, runMigrations } from '@goto/db'; const visit = { domain: 'go.example.test', path: '/docs', + outcome: 'valid', visited_at: Date.now(), ip: '127.0.0.1', user_agent: null, diff --git a/apps/worker/src/services/retention.test.ts b/apps/worker/src/services/retention.test.ts index 4981010..4c5d963 100644 --- a/apps/worker/src/services/retention.test.ts +++ b/apps/worker/src/services/retention.test.ts @@ -1,5 +1,5 @@ import { validateConfig } from '@goto/config'; -import { openDb, runMigrations } from '@goto/db'; +import { openDb, runMigrations, type ClassifiedVisit } from '@goto/db'; import type { OpenWorkflow } from 'openworkflow'; import { expect, test, vi } from 'vitest'; import { @@ -41,9 +41,10 @@ function client() { return { engine, result }; } -const visit = { +const visit: ClassifiedVisit = { domain: 'go.example.test', path: '/docs', + outcome: 'valid', visited_at: 1, ip: '127.0.0.1', user_agent: null, diff --git a/apps/worker/src/services/startup.test.ts b/apps/worker/src/services/startup.test.ts index c59b143..663e5d8 100644 --- a/apps/worker/src/services/startup.test.ts +++ b/apps/worker/src/services/startup.test.ts @@ -70,6 +70,7 @@ test('an existing zero-policy database is purged by a real startup workflow', as await insertVisit(db, { domain: 'go.example.test', path: '/docs', + outcome: 'valid', visited_at: Date.now(), ip: '127.0.0.1', user_agent: null, diff --git a/packages/db/src/index.ts b/packages/db/src/index.ts index ac53c74..9f9cbeb 100644 --- a/packages/db/src/index.ts +++ b/packages/db/src/index.ts @@ -7,7 +7,7 @@ import type { Database } from './types.js'; export { runMigrations } from './migrate.js'; export { insertVisit, pruneVisits, purgeVisits } from './visits.js'; -export type { ClassifiedVisit, Database, NewVisit, Visit } from './types.js'; +export type { ClassifiedVisit, Database, NewVisit, Visit, VisitOutcome } from './types.js'; export async function openDb( path: string, diff --git a/packages/db/src/migrations/0005_visit_outcome.ts b/packages/db/src/migrations/0005_visit_outcome.ts new file mode 100644 index 0000000..5a8b4bf --- /dev/null +++ b/packages/db/src/migrations/0005_visit_outcome.ts @@ -0,0 +1,10 @@ +import { sql, type Kysely, type Migration } from 'kysely'; + +export const addVisitOutcome: Migration = { + async up(db: Kysely): Promise { + await sql`ALTER TABLE visits ADD COLUMN outcome text NOT NULL DEFAULT 'valid'`.execute(db); + }, + async down(db: Kysely): Promise { + await sql`ALTER TABLE visits DROP COLUMN outcome`.execute(db); + } +}; diff --git a/packages/db/src/migrations/index.ts b/packages/db/src/migrations/index.ts index ef6c571..38062e3 100644 --- a/packages/db/src/migrations/index.ts +++ b/packages/db/src/migrations/index.ts @@ -2,10 +2,12 @@ import { initial } from './0001_visits.js'; import { addVisitClassification } from './0002_visit_classification.js'; import { visitRetention } from './0003_visit_retention.js'; import { removeVisitCollection } from './0004_remove_visit_collection.js'; +import { addVisitOutcome } from './0005_visit_outcome.js'; export const migrations = { '0001_visits': initial, '0002_visit_classification': addVisitClassification, '0003_visit_retention': visitRetention, - '0004_remove_visit_collection': removeVisitCollection + '0004_remove_visit_collection': removeVisitCollection, + '0005_visit_outcome': addVisitOutcome }; diff --git a/packages/db/src/storage.test.ts b/packages/db/src/storage.test.ts index 481afb5..bc8c155 100644 --- a/packages/db/src/storage.test.ts +++ b/packages/db/src/storage.test.ts @@ -28,6 +28,7 @@ afterEach(async () => { const visit: ClassifiedVisit = { domain: 'go.example.test', path: '/café', + outcome: 'valid', visited_at: 1_700_000_000_000, ip: '2001:db8::1', user_agent: 'Mozilla/5.0 (X11; Linux x86_64; rv:158.0) Gecko/20100101 Firefox/158.0', @@ -52,7 +53,8 @@ test('opens a real WAL database and applies versioned migrations once', async () '0001_visits', '0002_visit_classification', '0003_visit_retention', - '0004_remove_visit_collection' + '0004_remove_visit_collection', + '0005_visit_outcome' ]); const tables = await db.introspection.getTables(); const names = tables.map((table) => table.name); @@ -63,7 +65,7 @@ test('opens a real WAL database and applies versioned migrations once', async () } }); -test('persists classified visits for the same logical path on different domains', async () => { +test('persists request-time outcomes for the same logical path on different domains', async () => { const path = join(directory, 'visits.sqlite'); const writer = await openDb(path); const ids: number[] = []; @@ -74,6 +76,7 @@ test('persists classified visits for the same logical path on different domains' await insertVisit(writer, { ...visit, domain: 'tiny.example.test', + outcome: 'invalid', user_agent: null, accept_language: null, browser: 'Other', @@ -91,6 +94,7 @@ test('persists classified visits for the same logical path on different domains' { ...visit, domain: 'tiny.example.test', + outcome: 'invalid', id: ids[1], user_agent: null, accept_language: null, @@ -104,7 +108,7 @@ test('persists classified visits for the same logical path on different domains' } }); -test('adds classification columns without changing existing visit data', async () => { +test('adds request-time labels without changing existing visit data', async () => { const db = await openDb(join(directory, 'legacy.sqlite')); try { const migrator = new Migrator({ @@ -125,7 +129,8 @@ test('adds classification columns without changing existing visit data', async ( expect(await runMigrations(db)).toEqual([ '0002_visit_classification', '0003_visit_retention', - '0004_remove_visit_collection' + '0004_remove_visit_collection', + '0005_visit_outcome' ]); expect(await db.selectFrom('visits').selectAll().executeTakeFirstOrThrow()).toEqual({ ...visit, @@ -162,7 +167,8 @@ test('a failed migration rolls back schema and bookkeeping so startup can retry' '0001_visits', '0002_visit_classification', '0003_visit_retention', - '0004_remove_visit_collection' + '0004_remove_visit_collection', + '0005_visit_outcome' ]); expect(await runMigrations(db)).toEqual([]); } finally { diff --git a/packages/db/src/types.ts b/packages/db/src/types.ts index bb66cf9..8fb3033 100644 --- a/packages/db/src/types.ts +++ b/packages/db/src/types.ts @@ -1,9 +1,12 @@ import type { Generated, Insertable, Selectable } from 'kysely'; +export type VisitOutcome = 'valid' | 'invalid'; + export interface VisitTable { id: Generated; domain: string; path: string; + outcome: VisitOutcome; visited_at: number; ip: string; user_agent: string | null; -- 2.54.0