diff --git a/.changeset/visit-classification.md b/.changeset/visit-classification.md new file mode 100644 index 0000000..e2ded5e --- /dev/null +++ b/.changeset/visit-classification.md @@ -0,0 +1,5 @@ +--- +'@goto/web': patch +--- + +Classify broad browser and operating-system families when recording visits in the background worker. diff --git a/apps/web/e2e/visits.e2e.ts b/apps/web/e2e/visits.e2e.ts index 0429755..ba653f4 100644 --- a/apps/web/e2e/visits.e2e.ts +++ b/apps/web/e2e/visits.e2e.ts @@ -23,7 +23,14 @@ test('the compiled writer isolates lock waits and drains accepted visits before appServer }) => { const databasePath = resolve(dirname(appServer.configurationPath), '../data/goto.sqlite'); - const options = { headers: { host: 'go.example.org' }, maxRedirects: 0, timeout: 2000 }; + const options = { + headers: { + host: 'go.example.org', + 'user-agent': 'Mozilla/5.0 (X11; Linux x86_64; rv:158.0) Gecko/20100101 Firefox/158.0' + }, + maxRedirects: 0, + timeout: 2000 + }; await expect .poll( async () => { @@ -50,5 +57,13 @@ test('the compiled writer isolates lock waits and drains accepted visits before } 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' }]); + } finally { + db.close(); + } expect(appServer.logs()).not.toContain('Visit tracking unavailable'); }); diff --git a/packages/db/src/classify-user-agent.test.ts b/packages/db/src/classify-user-agent.test.ts new file mode 100644 index 0000000..ed5704e --- /dev/null +++ b/packages/db/src/classify-user-agent.test.ts @@ -0,0 +1,46 @@ +import { expect, test } from 'vitest'; +import { classifyUserAgent } from './classify-user-agent.js'; + +test('recognizes Edge as one family before the generic Chrome and Safari tokens', () => { + for (const token of ['Edg', 'EdgA', 'EdgiOS', 'Edge']) { + const result = classifyUserAgent(`Mozilla/5.0 Chrome/125.0 Safari/537.36 ${token}/125.0`); + expect(result.browser).toBe('Edge'); + } +}); + +test.each([ + { + ua: 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) Chrome/125.0 Safari/537.36', + browser: 'Chrome', + os: 'Windows' + }, + { + ua: 'Mozilla/5.0 (Linux; Android 14) Chrome/125.0 Safari/537.36 OPR/110.0', + browser: 'Opera', + os: 'Android' + }, + { + ua: 'Mozilla/5.0 (iPhone; CPU iPhone OS 17_5 like Mac OS X) CriOS/125.0 Mobile Safari/604.1', + browser: 'Chrome', + os: 'iOS' + }, + { + ua: 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) Version/17.5 Safari/605.1', + browser: 'Safari', + os: 'macOS' + }, + { + ua: 'Mozilla/5.0 (X11; Linux x86_64; rv:158.0) Gecko/20100101 Firefox/158.0', + browser: 'Firefox', + os: 'Linux' + }, + { + ua: 'Mozilla/5.0 (X11; CrOS x86_64 15886.44.0) Chrome/125.0 Safari/537.36', + browser: 'Chrome', + os: 'ChromeOS' + }, + { ua: null, browser: 'Other', os: 'Other' }, + { ua: 'curl/8.12.0', browser: 'Other', os: 'Other' } +])('classifies $browser on $os without parsing versions', ({ ua, browser, os }) => { + expect(classifyUserAgent(ua)).toEqual({ browser, os }); +}); diff --git a/packages/db/src/classify-user-agent.ts b/packages/db/src/classify-user-agent.ts new file mode 100644 index 0000000..6c1b8a2 --- /dev/null +++ b/packages/db/src/classify-user-agent.ts @@ -0,0 +1,27 @@ +function browserFamily(ua: string): string { + if (ua.includes('edg/') || ua.includes('edga/') || ua.includes('edgios/') || ua.includes('edge/')) + return 'Edge'; + if (ua.includes('opr/') || ua.includes('opera/')) return 'Opera'; + if (ua.includes('firefox/') || ua.includes('fxios/')) return 'Firefox'; + if (ua.includes('chrome/') || ua.includes('crios/') || ua.includes('chromium/')) return 'Chrome'; + if (ua.includes('safari/')) return 'Safari'; + return 'Other'; +} + +function operatingSystem(ua: string): string { + if (ua.includes('windows')) return 'Windows'; + if (ua.includes('android')) return 'Android'; + if (ua.includes('iphone') || ua.includes('ipad') || ua.includes('ipod')) return 'iOS'; + if (ua.includes('macintosh') || ua.includes('mac os x')) return 'macOS'; + if (ua.includes('cros ')) return 'ChromeOS'; + if (ua.includes('linux')) return 'Linux'; + return 'Other'; +} + +export function classifyUserAgent(userAgent: string | null | undefined): { + browser: string; + os: string; +} { + const ua = userAgent?.toLowerCase() ?? ''; + return { browser: browserFamily(ua), os: operatingSystem(ua) }; +} diff --git a/packages/db/src/index.ts b/packages/db/src/index.ts index 3e8456b..8e54bb8 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 } from './visits.js'; -export type { Database, NewVisit, Visit } from './types.js'; +export type { ClassifiedVisit, Database, NewVisit, Visit } from './types.js'; export async function openDb( path: string, diff --git a/packages/db/src/migrations/0002_visit_classification.ts b/packages/db/src/migrations/0002_visit_classification.ts new file mode 100644 index 0000000..5e8c287 --- /dev/null +++ b/packages/db/src/migrations/0002_visit_classification.ts @@ -0,0 +1,12 @@ +import { sql, type Kysely, type Migration } from 'kysely'; + +export const addVisitClassification: Migration = { + async up(db: Kysely): Promise { + await sql`ALTER TABLE visits ADD COLUMN browser text NOT NULL DEFAULT 'Other'`.execute(db); + await sql`ALTER TABLE visits ADD COLUMN os text NOT NULL DEFAULT 'Other'`.execute(db); + }, + async down(db: Kysely): Promise { + await sql`ALTER TABLE visits DROP COLUMN os`.execute(db); + await sql`ALTER TABLE visits DROP COLUMN browser`.execute(db); + } +}; diff --git a/packages/db/src/migrations/index.ts b/packages/db/src/migrations/index.ts index ba18487..a3ad836 100644 --- a/packages/db/src/migrations/index.ts +++ b/packages/db/src/migrations/index.ts @@ -1,3 +1,7 @@ import { initial } from './0001_visits.js'; +import { addVisitClassification } from './0002_visit_classification.js'; -export const migrations = { '0001_visits': initial }; +export const migrations = { + '0001_visits': initial, + '0002_visit_classification': addVisitClassification +}; diff --git a/packages/db/src/node-sqlite-dialect.ts b/packages/db/src/node-sqlite-dialect.ts index 3ca2978..ff74391 100644 --- a/packages/db/src/node-sqlite-dialect.ts +++ b/packages/db/src/node-sqlite-dialect.ts @@ -1,4 +1,4 @@ -import { SqliteDialect, type SqliteDatabase, type SqliteStatement } from 'kysely'; +import { SqliteAdapter, SqliteDialect, type SqliteDatabase, type SqliteStatement } from 'kysely'; import type { DatabaseSync, SQLInputValue } from 'node:sqlite'; export interface NodeSqliteDialectConfig { @@ -20,9 +20,19 @@ function adaptDatabase(database: DatabaseSync): SqliteDatabase { }; } +class NodeSqliteAdapter extends SqliteAdapter { + override get supportsTransactionalDdl(): boolean { + return true; + } +} + export class NodeSqliteDialect extends SqliteDialect { constructor(config: NodeSqliteDialectConfig) { // Native connection ownership keeps other requests out of an active transaction. super({ database: adaptDatabase(config.database) }); } + + override createAdapter(): SqliteAdapter { + return new NodeSqliteAdapter(); + } } diff --git a/packages/db/src/storage.test.ts b/packages/db/src/storage.test.ts index 48812ba..8826a0b 100644 --- a/packages/db/src/storage.test.ts +++ b/packages/db/src/storage.test.ts @@ -3,9 +3,10 @@ import { mkdtemp, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { DatabaseSync } from 'node:sqlite'; import { dirname, join } from 'node:path'; -import { sql } from 'kysely'; +import { Migrator, sql, type Migration } from 'kysely'; import { afterEach, beforeEach, expect, test, vi } from 'vitest'; -import { insertVisit, openDb, runMigrations, type NewVisit } from './index.js'; +import { insertVisit, openDb, runMigrations, type ClassifiedVisit } from './index.js'; +import { migrations } from './migrations/index.js'; let directory: string; @@ -17,13 +18,15 @@ afterEach(async () => { await rm(directory, { recursive: true, force: true }); }); -const visit: NewVisit = { +const visit: ClassifiedVisit = { domain: 'go.example.test', path: '/café', visited_at: 1_700_000_000_000, ip: '2001:db8::1', - user_agent: 'Storage test agent', - accept_language: 'en-US,en;q=0.9' + user_agent: 'Mozilla/5.0 (X11; Linux x86_64; rv:158.0) Gecko/20100101 Firefox/158.0', + accept_language: 'en-US,en;q=0.9', + browser: 'Firefox', + os: 'Linux' }; test('opens a real WAL database and applies versioned migrations once', async () => { @@ -38,14 +41,14 @@ test('opens a real WAL database and applies versioned migrations once', async () expect((await sql<{ timeout: number }>`PRAGMA busy_timeout`.execute(db)).rows[0]?.timeout).toBe( 5000 ); - expect(await runMigrations(db)).toEqual(['0001_visits']); + expect(await runMigrations(db)).toEqual(['0001_visits', '0002_visit_classification']); expect(await runMigrations(db)).toEqual([]); } finally { await db.destroy(); } }); -test('persists distinct raw visits for the same logical path on different domains', async () => { +test('persists classified visits for the same logical path on different domains', async () => { const path = join(directory, 'visits.sqlite'); const writer = await openDb(path); const ids: number[] = []; @@ -57,7 +60,9 @@ test('persists distinct raw visits for the same logical path on different domain ...visit, domain: 'tiny.example.test', user_agent: null, - accept_language: null + accept_language: null, + browser: 'Other', + os: 'Other' }) ); } finally { @@ -73,7 +78,9 @@ test('persists distinct raw visits for the same logical path on different domain domain: 'tiny.example.test', id: ids[1], user_agent: null, - accept_language: null + accept_language: null, + browser: 'Other', + os: 'Other' } ]); await expect(insertVisit(reader, visit)).rejects.toThrow(); @@ -82,6 +89,63 @@ test('persists distinct raw visits for the same logical path on different domain } }); +test('adds classification columns without changing existing visit data', async () => { + const db = await openDb(join(directory, 'legacy.sqlite')); + try { + const migrator = new Migrator({ + db, + provider: { + async getMigrations() { + return migrations; + } + } + }); + const { error } = await migrator.migrateTo('0001_visits'); + if (error) throw error; + await sql` + INSERT INTO visits (domain, path, visited_at, ip, user_agent, accept_language) + VALUES (${visit.domain}, ${visit.path}, ${visit.visited_at}, ${visit.ip}, ${visit.user_agent}, ${visit.accept_language}) + `.execute(db); + + expect(await runMigrations(db)).toEqual(['0002_visit_classification']); + expect(await db.selectFrom('visits').selectAll().executeTakeFirstOrThrow()).toEqual({ + ...visit, + id: 1, + browser: 'Other', + os: 'Other' + }); + } finally { + await db.destroy(); + } +}); + +test('a failed migration rolls back schema and bookkeeping so startup can retry', async () => { + const db = await openDb(join(directory, 'failed-migration.sqlite')); + try { + const failure = new Error('Simulated failure after classification DDL'); + const failedClassification: Migration = { + async up(transaction) { + await migrations['0002_visit_classification'].up(transaction); + throw failure; + } + }; + const migrator = new Migrator({ + db, + provider: { + async getMigrations() { + return { ...migrations, '0002_visit_classification': failedClassification }; + } + } + }); + + expect((await migrator.migrateToLatest()).error).toBe(failure); + expect(await runMigrations(db)).toEqual(['0001_visits', '0002_visit_classification']); + expect(await runMigrations(db)).toEqual([]); + } finally { + await db.destroy(); + } +}); + test('destroy closes an opened database without an intervening query', async () => { const close = vi.spyOn(DatabaseSync.prototype, 'close'); try { diff --git a/packages/db/src/types.ts b/packages/db/src/types.ts index 31f81ca..bb66cf9 100644 --- a/packages/db/src/types.ts +++ b/packages/db/src/types.ts @@ -8,11 +8,14 @@ export interface VisitTable { ip: string; user_agent: string | null; accept_language: string | null; + browser: string; + os: string; } export interface Database { visits: VisitTable; } -export type NewVisit = Omit, 'id'>; +export type ClassifiedVisit = Omit, 'id'>; +export type NewVisit = Omit; export type Visit = Selectable; diff --git a/packages/db/src/visit-worker.ts b/packages/db/src/visit-worker.ts index 76f7dee..33b69e0 100644 --- a/packages/db/src/visit-worker.ts +++ b/packages/db/src/visit-worker.ts @@ -1,4 +1,5 @@ import { workerData } from 'node:worker_threads'; +import { classifyUserAgent } from './classify-user-agent.js'; import { openDb, runMigrations, insertVisit, type NewVisit } from './index.js'; const { databasePath } = workerData as { databasePath: string }; @@ -13,7 +14,8 @@ try { export function ready(): void {} export async function recordVisit(visit: NewVisit): Promise { - await insertVisit(db, visit); + const classification = classifyUserAgent(visit.user_agent); + await insertVisit(db, { ...visit, ...classification }); } export async function close(): Promise { diff --git a/packages/db/src/visits.ts b/packages/db/src/visits.ts index f602473..5002173 100644 --- a/packages/db/src/visits.ts +++ b/packages/db/src/visits.ts @@ -1,7 +1,7 @@ import type { Kysely } from 'kysely'; -import type { Database, NewVisit } from './types.js'; +import type { ClassifiedVisit, Database } from './types.js'; -export async function insertVisit(db: Kysely, visit: NewVisit): Promise { +export async function insertVisit(db: Kysely, visit: ClassifiedVisit): Promise { const row = await db.insertInto('visits').values(visit).returning('id').executeTakeFirstOrThrow(); return row.id; }