Refactor code structure for improved readability and maintainability

web/server/__tests__/db.test.js

@@ -1,56 +1,70 @@
 /**
  * Tests for the Inventory Manager database layer (db.js).
  *
- * Uses a temporary on-disk SQLite file per test to exercise real SQL
- * without touching production data.  The DB_PATH env var is set BEFORE
- * the dynamic import so better-sqlite3 opens the right file.
+ * Sets DB_PATH to a temp file BEFORE importing the module so all
+ * operations hit a throwaway SQLite database.  The underlying
+ * better-sqlite3 `Database` instance is also imported so we can
+ * DELETE rows between tests without reopening the file.
  */
 
-import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import { describe, it, expect, beforeAll, beforeEach, afterAll } from 'vitest';
 import fs from 'fs';
 import os from 'os';
 import path from 'path';
 
+// ── Set DB_PATH before db.js is imported ───────────────────────────────
+const TMP_DB = path.join(os.tmpdir(), `inv-test-${process.pid}.db`);
+process.env.DB_PATH = TMP_DB;
+
+// Now import (module-level side effect opens the DB at TMP_DB)
+import db from '../db.js';
+import {
+  saveItems, loadItems,
+  saveState, loadState,
+  saveFurnaces, loadFurnaces,
+  saveAlerts, loadAlerts,
+  recordItemHistory, getHistory, getHistorySummary,
+  saveFullState, flushPendingSave, loadFullState,
+  closeDb,
+} from '../db.js';
+
 // ── helpers ────────────────────────────────────────────────────────────
 
-/** Create a fresh temp DB path and set the env var before importing db.js */
-async function createTestDb() {
-  const tmp = path.join(os.tmpdir(), `inv-test-${Date.now()}-${Math.random().toString(36).slice(2)}.db`);
-  process.env.DB_PATH = tmp;
-
-  // Use a cache-buster query string so Node gives us a fresh module
-  const mod = await import(`../db.js?t=${Date.now()}-${Math.random()}`);
-  return { mod, tmp };
+/** Wipe all rows from every table so each test starts fresh */
+function clearAllTables() {
+  db.exec(`
+    DELETE FROM items;
+    DELETE FROM furnaces;
+    DELETE FROM alerts;
+    DELETE FROM state;
+    DELETE FROM item_history;
+  `);
 }
 
-function cleanup(tmp) {
-  try { fs.unlinkSync(tmp); } catch { /* ignore */ }
-  try { fs.unlinkSync(tmp + '-wal'); } catch { /* ignore */ }
-  try { fs.unlinkSync(tmp + '-shm'); } catch { /* ignore */ }
-}
+// ── lifecycle ──────────────────────────────────────────────────────────
+
+beforeEach(() => {
+  clearAllTables();
+});
+
+afterAll(() => {
+  try { closeDb(); } catch { /* already closed */ }
+  try { fs.unlinkSync(TMP_DB); } catch { /* ignore */ }
+  try { fs.unlinkSync(TMP_DB + '-wal'); } catch { /* ignore */ }
+  try { fs.unlinkSync(TMP_DB + '-shm'); } catch { /* ignore */ }
+});
 
 // ── tests ──────────────────────────────────────────────────────────────
 
 describe('db – item persistence', () => {
-  let db, tmp;
-
-  beforeEach(async () => {
-    ({ mod: db, tmp } = await createTestDb());
-  });
-
-  afterEach(() => {
-    try { db.closeDb(); } catch { /* already closed */ }
-    cleanup(tmp);
-  });
-
   it('saveItems + loadItems round-trips item data', () => {
     const items = [
       { name: 'minecraft:diamond', count: 64, displayName: 'Diamond' },
       { name: 'minecraft:iron_ingot', count: 128, displayName: 'Iron Ingot' },
     ];
-    db.saveItems(items);
+    saveItems(items);
 
-    const loaded = db.loadItems();
+    const loaded = loadItems();
     expect(loaded).toHaveLength(2);
 
     // loadItems returns ORDER BY count DESC
@@ -61,53 +75,31 @@ describe('db – item persistence', () => {
   });
 
   it('saveItems upserts — updates existing rows instead of duplicating', () => {
-    db.saveItems([{ name: 'minecraft:diamond', count: 10, displayName: 'Diamond' }]);
-    db.saveItems([{ name: 'minecraft:diamond', count: 99, displayName: 'Diamond' }]);
+    saveItems([{ name: 'minecraft:diamond', count: 10, displayName: 'Diamond' }]);
+    saveItems([{ name: 'minecraft:diamond', count: 99, displayName: 'Diamond' }]);
 
-    const loaded = db.loadItems();
+    const loaded = loadItems();
     expect(loaded).toHaveLength(1);
     expect(loaded[0].count).toBe(99);
   });
 });
 
 describe('db – state key-value store', () => {
-  let db, tmp;
-
-  beforeEach(async () => {
-    ({ mod: db, tmp } = await createTestDb());
-  });
-
-  afterEach(() => {
-    try { db.closeDb(); } catch { /* already closed */ }
-    cleanup(tmp);
-  });
-
   it('saveState + loadState round-trips JSON', () => {
-    db.saveState('smeltingPaused', true);
-    expect(db.loadState('smeltingPaused')).toBe(true);
+    saveState('smeltingPaused', true);
+    expect(loadState('smeltingPaused')).toBe(true);
 
-    db.saveState('disabledRecipes', { 'minecraft:iron_ingot': true });
-    expect(db.loadState('disabledRecipes')).toEqual({ 'minecraft:iron_ingot': true });
+    saveState('disabledRecipes', { 'minecraft:iron_ingot': true });
+    expect(loadState('disabledRecipes')).toEqual({ 'minecraft:iron_ingot': true });
   });
 
   it('loadState returns defaultValue when key missing', () => {
-    expect(db.loadState('nonexistent', 42)).toBe(42);
-    expect(db.loadState('nonexistent')).toBeNull();
+    expect(loadState('nonexistent', 42)).toBe(42);
+    expect(loadState('nonexistent')).toBeNull();
   });
 });
 
 describe('db – furnace persistence', () => {
-  let db, tmp;
-
-  beforeEach(async () => {
-    ({ mod: db, tmp } = await createTestDb());
-  });
-
-  afterEach(() => {
-    try { db.closeDb(); } catch { /* already closed */ }
-    cleanup(tmp);
-  });
-
   it('saveFurnaces + loadFurnaces round-trips furnace status', () => {
     const furnaces = {
       'minecraft:furnace_0': {
@@ -119,8 +111,8 @@ describe('db – furnace persistence', () => {
       },
     };
 
-    db.saveFurnaces(furnaces);
-    const loaded = db.loadFurnaces();
+    saveFurnaces(furnaces);
+    const loaded = loadFurnaces();
 
     expect(loaded['minecraft:furnace_0']).toBeDefined();
     expect(loaded['minecraft:furnace_0'].active).toBe(true);
@@ -129,24 +121,13 @@ describe('db – furnace persistence', () => {
 });
 
 describe('db – alert persistence', () => {
-  let db, tmp;
-
-  beforeEach(async () => {
-    ({ mod: db, tmp } = await createTestDb());
-  });
-
-  afterEach(() => {
-    try { db.closeDb(); } catch { /* already closed */ }
-    cleanup(tmp);
-  });
-
   it('saveAlerts + loadAlerts round-trips triggered alerts', () => {
-    db.saveAlerts([
+    saveAlerts([
       { item: 'minecraft:diamond', triggered: true, current: 5, threshold: 10 },
       { item: 'minecraft:coal', triggered: false, current: 200, threshold: 50 },
     ]);
 
-    const loaded = db.loadAlerts();
+    const loaded = loadAlerts();
     // loadAlerts only returns triggered=1
     expect(loaded).toHaveLength(1);
     expect(loaded[0].item).toBe('minecraft:diamond');
@@ -157,54 +138,33 @@ describe('db – alert persistence', () => {
 });
 
 describe('db – item history', () => {
-  let db, tmp;
-
-  beforeEach(async () => {
-    ({ mod: db, tmp } = await createTestDb());
-  });
-
-  afterEach(() => {
-    try { db.closeDb(); } catch { /* already closed */ }
-    cleanup(tmp);
-  });
-
   it('getHistory returns recorded snapshots ordered newest-first', () => {
-    // recordItemHistory is throttled (5-min internal cooldown), so we
-    // call it twice — first call records, second is throttled.
-    db.recordItemHistory([
+    // recordItemHistory is throttled (5-min internal cooldown).
+    // First call after module load will go through.
+    recordItemHistory([
       { name: 'minecraft:diamond', count: 64 },
       { name: 'minecraft:iron_ingot', count: 128 },
     ]);
 
-    const history = db.getHistory('minecraft:diamond', 10);
+    const history = getHistory('minecraft:diamond', 10);
     expect(history.length).toBeGreaterThanOrEqual(1);
     expect(history[0].count).toBe(64);
   });
 
   it('getHistorySummary groups by timestamp', () => {
-    db.recordItemHistory([
-      { name: 'a', count: 10 },
-      { name: 'b', count: 20 },
-    ]);
+    // May be throttled if previous test already recorded in this module load.
+    // Insert directly via the underlying db for determinism.
+    const now = Date.now();
+    db.prepare('INSERT INTO item_history (name, count, recorded_at) VALUES (?, ?, ?)').run('a', 10, now);
+    db.prepare('INSERT INTO item_history (name, count, recorded_at) VALUES (?, ?, ?)').run('b', 20, now);
 
-    const summary = db.getHistorySummary();
+    const summary = getHistorySummary();
     expect(summary.length).toBeGreaterThanOrEqual(1);
     expect(summary[0].total).toBe(30); // 10 + 20
   });
 });
 
 describe('db – full state round-trip', () => {
-  let db, tmp;
-
-  beforeEach(async () => {
-    ({ mod: db, tmp } = await createTestDb());
-  });
-
-  afterEach(() => {
-    try { db.closeDb(); } catch { /* already closed */ }
-    cleanup(tmp);
-  });
-
   it('saveFullState + flushPendingSave + loadFullState restores everything', () => {
     const state = {
       inventoryState: {
@@ -234,10 +194,10 @@ describe('db – full state round-trip', () => {
       craftTurtleOk: true,
     };
 
-    db.saveFullState(state);
-    db.flushPendingSave(); // force the debounced write
+    saveFullState(state);
+    flushPendingSave(); // force the debounced write
 
-    const restored = db.loadFullState();
+    const restored = loadFullState();
     expect(restored.inventoryState.itemList).toHaveLength(1);
     expect(restored.inventoryState.itemList[0].name).toBe('minecraft:diamond');
     expect(restored.inventoryState.grandTotal).toBe(64);