From 2ce40724d7a62a543bd56e7abc3a402baa285bba Mon Sep 17 00:00:00 2001 From: jubnl Date: Sun, 26 Jul 2026 12:36:30 +0200 Subject: [PATCH] chore: move category backend to proper DI --- server/src/mcp/resources.ts | 14 +- server/src/mcp/tools/places.ts | 17 +- server/src/nest/README.md | 13 +- .../src/nest/categories/categories.bridge.ts | 21 +++ server/src/nest/categories/categories.mcp.ts | 48 ++++++ .../src/nest/categories/categories.module.ts | 3 +- .../src/nest/categories/categories.service.ts | 50 ++++-- .../src/nest/plugins/host/create-rpc-host.ts | 2 +- server/src/services/categoryService.ts | 31 ---- server/tests/e2e/categories.e2e.test.ts | 82 +++++---- server/tests/helpers/mcp-test-controllers.ts | 3 + server/tests/unit/mcp/resources.test.ts | 22 +-- .../tests/unit/mcp/tools-categories.test.ts | 155 ++++++++++++++++++ server/tests/unit/mcp/tools-places.test.ts | 24 +-- .../categories.service.test.ts} | 107 ++++++------ .../unit/plugins/create-rpc-host.test.ts | 2 +- 16 files changed, 398 insertions(+), 196 deletions(-) create mode 100644 server/src/nest/categories/categories.bridge.ts create mode 100644 server/src/nest/categories/categories.mcp.ts delete mode 100644 server/src/services/categoryService.ts create mode 100644 server/tests/unit/mcp/tools-categories.test.ts rename server/tests/unit/{services/categoryService.test.ts => nest/categories.service.test.ts} (59%) diff --git a/server/src/mcp/resources.ts b/server/src/mcp/resources.ts index e4ccecf4c..2fa140c0a 100644 --- a/server/src/mcp/resources.ts +++ b/server/src/mcp/resources.ts @@ -9,7 +9,6 @@ import { listReservations } from '../services/reservationService'; import { listNotes as listDayNotes } from '../services/dayNoteService'; import { listNotes as listCollabNotes, listPolls, listMessages } from '../services/collabService'; import { listItems as listTodoItems } from '../services/todoService'; -import { listCategories } from '../services/categoryService'; import { listBucketList, listVisitedCountries, getStats as getAtlasStats, listManuallyVisitedRegions } from '../services/atlasService'; import { getNotifications } from '../services/inAppNotifications'; import { getActivePlanId, getActivePlan, getPlanData, getEntries as getVacayEntries, getHolidays } from '../services/vacayService'; @@ -215,16 +214,9 @@ export function registerResources(server: McpServer, userId: number, scopes: str } ); - // All place categories (global, no trip filter) — safe for any authenticated session - server.registerResource( - 'categories', - 'trek://categories', - { description: 'All available place categories (id, name, color, icon) for use when creating places', mimeType: 'application/json' }, - async (uri) => { - const categories = listCategories(); - return jsonContent(uri.href, categories); - } - ); + // The trek://categories resource moved to the DI-discovered + // src/nest/categories/categories.mcp.ts (@Resource, attached via the + // nest-mcp registry in registerTools). // User's bucket list if (isAddonEnabled(ADDON_IDS.ATLAS) && canRead(scopes, 'atlas')) server.registerResource( diff --git a/server/src/mcp/tools/places.ts b/server/src/mcp/tools/places.ts index c76ff93f2..2b2f731c9 100644 --- a/server/src/mcp/tools/places.ts +++ b/server/src/mcp/tools/places.ts @@ -5,7 +5,6 @@ import { isDemoUser } from '../../services/authService'; import { deletePlacesMany, updatePlacesMany, importGoogleList, importNaverList, listPlaces, createPlace, updatePlace, deletePlace, ratePlace } from '../../services/placeService'; import { createAssignment, dayExists } from '../../services/assignmentService'; import { onPlaceDeleted, reconcileTripSkeletons } from '../../services/journeyService'; -import { listCategories } from '../../services/categoryService'; import { searchPlaces } from '../../services/mapsService'; import { safeBroadcast, TOOL_ANNOTATIONS_READONLY, TOOL_ANNOTATIONS_WRITE, @@ -203,20 +202,8 @@ export function registerPlaceTools(server: McpServer, userId: number, scopes: st } ); - // --- CATEGORIES --- - - if (R) server.registerTool( - 'list_categories', - { - description: 'List all available place categories with their id, name, icon and color. Use category_id when creating or updating places.', - inputSchema: {}, - annotations: TOOL_ANNOTATIONS_READONLY, - }, - async () => { - const categories = listCategories(); - return ok({ categories }); - } - ); + // The list_categories tool moved to the DI-discovered + // src/nest/categories/categories.mcp.ts (@McpController). // --- SEARCH --- diff --git a/server/src/nest/README.md b/server/src/nest/README.md index 5c107f1b8..28288ef44 100644 --- a/server/src/nest/README.md +++ b/server/src/nest/README.md @@ -26,8 +26,8 @@ mount to Nest and leaves the sibling trip routes (days, places, ...) on Express. - **Phase 1 (leaf):** weather, airports, config (public), system-notices, maps, categories, tags, notifications, atlas. - **Phase 2 (trip sub-domains):** vacay (addon), packing, todo. -- **DI-native services (legacy `src/services/*` deleted):** tags — see the - migration recipe below. +- **DI-native services (legacy `src/services/*` deleted):** tags, categories — + see the migration recipe below. ## Cross-cutting Foundation pieces @@ -94,9 +94,12 @@ at the typed contract (Frontend Track). ## Migrating a legacy `src/services/*` service into its Nest module (recipe) Pilot: **tags** (`services/tagService.ts` → `nest/tags/tags.service.ts` + -`nest/tags/tags.bridge.ts`). Repeat these steps per service (next up: -categoryService, then todoService). This is a **pure relocation** — byte-identical -SQL, statuses, bodies, and error strings. +`nest/tags/tags.bridge.ts`); categories followed the same shape (and piloted the +first `@Resource` in `categories.mcp.ts`). Repeat these steps per service (next +up: todoService). This is a **pure relocation** — byte-identical +SQL, statuses, bodies, and error strings. The long-run plan for the last +non-Nest consumer (the plugin RPC host, today served via the bridges) is in +`src/nest/plugins/DI-MIGRATION.md`. 1. **Move the SQL** into `.service.ts` as methods over an injected `DatabaseService` (`this.db.all/get/run/prepare/transaction`; strict diff --git a/server/src/nest/categories/categories.bridge.ts b/server/src/nest/categories/categories.bridge.ts new file mode 100644 index 000000000..49a091b55 --- /dev/null +++ b/server/src/nest/categories/categories.bridge.ts @@ -0,0 +1,21 @@ +import type { Category } from '@trek/shared'; +import { db } from '../../db/database'; +import { DatabaseService } from '../database/database.service'; +import { CategoriesService } from './categories.service'; + +/** + * Non-Nest entry point for the categories domain — for code running OUTSIDE + * the Nest container (currently only the plugin RPC host; the MCP tool and + * resource moved to the DI-discovered categories.mcp.ts). Exports only the + * legacy services/categoryService names still consumed outside the container, + * 1:1, so repointing a consumer is an import-path-only diff. Inside the + * container, inject CategoriesService instead. + * + * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto + * the shared better-sqlite3 singleton (same pattern as tags.bridge.ts). + */ +const categories = new CategoriesService(new DatabaseService(db)); + +export function listCategories(): Category[] { + return categories.list(); +} diff --git a/server/src/nest/categories/categories.mcp.ts b/server/src/nest/categories/categories.mcp.ts new file mode 100644 index 000000000..1f13b23f7 --- /dev/null +++ b/server/src/nest/categories/categories.mcp.ts @@ -0,0 +1,48 @@ +import { + McpController, Tool, Resource, type McpContext, + TOOL_ANNOTATIONS_READONLY, ok, +} from '@trek/nest-mcp'; +import { CategoriesService } from './categories.service'; + +/** + * Categories MCP surface — ported 1:1 from the legacy registrars: the + * list_categories tool from src/mcp/tools/places.ts (identical name, + * description, annotations and places-read gating via the declarative access + * marker + trekMcpAccessPolicy) and the trek://categories resource from + * src/mcp/resources.ts (first production @Resource; intentionally ungated — + * the legacy registration was unconditional, safe for any authenticated + * session — and the payload reproduces the legacy jsonContent shape verbatim). + */ +@McpController() +export class CategoriesMcp { + constructor(private readonly categories: CategoriesService) {} + + @Tool({ + name: 'list_categories', + description: 'List all available place categories with their id, name, icon and color. Use category_id when creating or updating places.', + inputSchema: {}, + annotations: TOOL_ANNOTATIONS_READONLY, + access: { group: 'places', mode: 'read' }, + }) + async listCategories(_args: Record, _ctx: McpContext) { + const categories = this.categories.list(); + return ok({ categories }); + } + + @Resource({ + name: 'categories', + uri: 'trek://categories', + description: 'All available place categories (id, name, color, icon) for use when creating places', + mimeType: 'application/json', + }) + async categoriesResource(uri: URL, _ctx: McpContext) { + const categories = this.categories.list(); + return { + contents: [{ + uri: uri.href, + mimeType: 'application/json', + text: JSON.stringify(categories, null, 2), + }], + }; + } +} diff --git a/server/src/nest/categories/categories.module.ts b/server/src/nest/categories/categories.module.ts index a4ef57f6c..233234686 100644 --- a/server/src/nest/categories/categories.module.ts +++ b/server/src/nest/categories/categories.module.ts @@ -1,10 +1,11 @@ import { Module } from '@nestjs/common'; import { CategoriesController } from './categories.controller'; +import { CategoriesMcp } from './categories.mcp'; import { CategoriesService } from './categories.service'; /** Categories domain (L4 leaf module). Registered in AppModule. */ @Module({ controllers: [CategoriesController], - providers: [CategoriesService], + providers: [CategoriesService, CategoriesMcp], }) export class CategoriesModule {} diff --git a/server/src/nest/categories/categories.service.ts b/server/src/nest/categories/categories.service.ts index 5f652f804..0bd44c5fd 100644 --- a/server/src/nest/categories/categories.service.ts +++ b/server/src/nest/categories/categories.service.ts @@ -1,37 +1,57 @@ import { Injectable } from '@nestjs/common'; import type { Category } from '@trek/shared'; -import { - listCategories, - getCategoryById, - createCategory, - updateCategory, - deleteCategory, -} from '../../services/categoryService'; +import { DatabaseService } from '../database/database.service'; /** - * Thin Nest wrapper around the existing category service. The SQL and the - * default colour/icon fallbacks stay in categoryService, so behaviour is - * unchanged. + * Categories domain service — owns the category SQL (moved 1:1 from the legacy + * services/categoryService.ts: identical statements, the #6366f1/📍 defaults + * and the COALESCE update semantics). Non-Nest consumers (plugin RPC host) go + * through categories.bridge.ts instead of importing this class directly. */ @Injectable() export class CategoriesService { + constructor(private readonly db: DatabaseService) {} + list(): Category[] { - return listCategories() as Category[]; + return this.db.all('SELECT * FROM categories ORDER BY name ASC'); } getById(id: string | number): Category | undefined { - return getCategoryById(id) as Category | undefined; + return this.db.get('SELECT * FROM categories WHERE id = ?', id); } create(userId: number, name: string, color?: string, icon?: string): Category { - return createCategory(userId, name, color, icon) as Category; + const result = this.db.run( + 'INSERT INTO categories (name, color, icon, user_id) VALUES (?, ?, ?, ?)', + name, + color || '#6366f1', + icon || '📍', + userId, + ); + return this.db.get( + 'SELECT * FROM categories WHERE id = ?', + result.lastInsertRowid, + ) as Category; } update(id: string | number, name?: string, color?: string, icon?: string): Category { - return updateCategory(id, name, color, icon) as Category; + this.db.run( + ` + UPDATE categories SET + name = COALESCE(?, name), + color = COALESCE(?, color), + icon = COALESCE(?, icon) + WHERE id = ? + `, + name || null, + color || null, + icon || null, + id, + ); + return this.db.get('SELECT * FROM categories WHERE id = ?', id) as Category; } remove(id: string | number): void { - deleteCategory(id); + this.db.run('DELETE FROM categories WHERE id = ?', id); } } diff --git a/server/src/nest/plugins/host/create-rpc-host.ts b/server/src/nest/plugins/host/create-rpc-host.ts index 4fec02536..4eec6e72a 100644 --- a/server/src/nest/plugins/host/create-rpc-host.ts +++ b/server/src/nest/plugins/host/create-rpc-host.ts @@ -6,7 +6,7 @@ import { listBudgetItems } from '../../../services/budgetService'; import { listItems as listPackingItemsSvc, createItem as createPackingItemSvc, updateItem as updatePackingItemSvc, deleteItem as deletePackingItemSvc, listBags, createBag as createBagSvc, updateBag as updateBagSvc, deleteBag as deleteBagSvc, setBagMembers } from '../../../services/packingService'; import { isUpdateConflict } from '../../../services/conflictResult'; import { getWeather } from '../../../services/weatherService'; -import { listCategories } from '../../../services/categoryService'; +import { listCategories } from '../../categories/categories.bridge'; import { listTags, createTag, updateTag, deleteTag, getTagByIdAndUser } from '../../tags/tags.bridge'; import { listItems as listTodosSvc, createItem as createTodoSvc, updateItem as updateTodoSvc, deleteItem as deleteTodoSvc } from '../../../services/todoService'; import { listFiles, createFile, createFileLink, getFileById, updateFile, softDeleteFile, findForeignLinkTarget, resolveFilePath, BLOCKED_EXTENSIONS, filesDir } from '../../../services/fileService'; diff --git a/server/src/services/categoryService.ts b/server/src/services/categoryService.ts deleted file mode 100644 index a533d1a86..000000000 --- a/server/src/services/categoryService.ts +++ /dev/null @@ -1,31 +0,0 @@ -import { db } from '../db/database'; - -export function listCategories() { - return db.prepare('SELECT * FROM categories ORDER BY name ASC').all(); -} - -export function createCategory(userId: number, name: string, color?: string, icon?: string) { - const result = db.prepare( - 'INSERT INTO categories (name, color, icon, user_id) VALUES (?, ?, ?, ?)' - ).run(name, color || '#6366f1', icon || '\uD83D\uDCCD', userId); - return db.prepare('SELECT * FROM categories WHERE id = ?').get(result.lastInsertRowid); -} - -export function getCategoryById(categoryId: number | string) { - return db.prepare('SELECT * FROM categories WHERE id = ?').get(categoryId); -} - -export function updateCategory(categoryId: number | string, name?: string, color?: string, icon?: string) { - db.prepare(` - UPDATE categories SET - name = COALESCE(?, name), - color = COALESCE(?, color), - icon = COALESCE(?, icon) - WHERE id = ? - `).run(name || null, color || null, icon || null, categoryId); - return db.prepare('SELECT * FROM categories WHERE id = ?').get(categoryId); -} - -export function deleteCategory(categoryId: number | string) { - db.prepare('DELETE FROM categories WHERE id = ?').run(categoryId); -} diff --git a/server/tests/e2e/categories.e2e.test.ts b/server/tests/e2e/categories.e2e.test.ts index 147bd5a19..22916e14b 100644 --- a/server/tests/e2e/categories.e2e.test.ts +++ b/server/tests/e2e/categories.e2e.test.ts @@ -1,9 +1,11 @@ /** * Categories module e2e — exercises the migrated /api/categories endpoints * through the real JwtAuthGuard + AdminGuard against a temp SQLite db seeded - * with an admin and a normal user. categoryService is mocked. + * with an admin and a normal user. CategoriesService runs its real SQL via + * DatabaseModule (the DATABASE_CONNECTION factory picks up the mocked db + * singleton): listing is open to any authenticated user; writes are admin-only. */ -import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import { describe, it, expect, beforeAll, beforeEach, afterAll, vi } from 'vitest'; import request from 'supertest'; import cookieParser from 'cookie-parser'; import type { Server } from 'http'; @@ -17,27 +19,29 @@ const { db } = vi.hoisted(() => { tmp.exec('PRAGMA journal_mode = WAL'); tmp.exec(`CREATE TABLE users (id INTEGER PRIMARY KEY AUTOINCREMENT, username TEXT NOT NULL, email TEXT NOT NULL UNIQUE, role TEXT NOT NULL DEFAULT 'user', password_version INTEGER NOT NULL DEFAULT 0);`); + tmp.exec(`CREATE TABLE categories ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + name TEXT NOT NULL, + color TEXT DEFAULT '#6366f1', + icon TEXT DEFAULT '📍', + user_id INTEGER REFERENCES users(id) ON DELETE SET NULL, + created_at DATETIME DEFAULT CURRENT_TIMESTAMP + );`); return { db: tmp }; }); vi.mock('../../src/db/database', () => ({ db, closeDb: () => {}, reinitialize: () => {} })); -const { mocks } = vi.hoisted(() => ({ - mocks: { - listCategories: vi.fn(), - createCategory: vi.fn(), - getCategoryById: vi.fn(), - updateCategory: vi.fn(), - deleteCategory: vi.fn(), - }, -})); -vi.mock('../../src/services/categoryService', () => mocks); - import { CategoriesModule } from '../../src/nest/categories/categories.module'; import { DatabaseModule } from '../../src/nest/database/database.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; -const cat = { id: 1, name: 'Food', color: '#fff', icon: '🍔' }; +function insertCategory(name: string, color = '#6366f1', icon = '📍', userId = 1): number { + const res = db + .prepare('INSERT INTO categories (name, color, icon, user_id) VALUES (?, ?, ?, ?)') + .run(name, color, icon, userId); + return Number(res.lastInsertRowid); +} describe('Categories e2e (real JwtAuthGuard + AdminGuard + temp SQLite)', () => { let server: Server; @@ -57,10 +61,10 @@ describe('Categories e2e (real JwtAuthGuard + AdminGuard + temp SQLite)', () => seedUser(db as never, { id: 2, role: 'user', email: 'user@example.test' }); app = await build(); server = app.getHttpServer(); - mocks.listCategories.mockReturnValue([cat]); - mocks.createCategory.mockReturnValue(cat); - mocks.getCategoryById.mockImplementation((id: string | number) => (String(id) === '1' ? cat : undefined)); - mocks.updateCategory.mockReturnValue({ ...cat, name: 'Drinks' }); + }); + + beforeEach(() => { + db.exec('DELETE FROM categories'); }); afterAll(async () => { @@ -72,24 +76,32 @@ describe('Categories e2e (real JwtAuthGuard + AdminGuard + temp SQLite)', () => expect(res.status).toBe(401); }); - it('200 list for any authenticated user (non-admin allowed)', async () => { + it('200 list for any authenticated user (non-admin allowed), ordered by name', async () => { + insertCategory('Zoo'); + insertCategory('Aquarium'); const res = await request(server).get('/api/categories').set('Cookie', sessionCookie(2)); expect(res.status).toBe(200); - expect(res.body).toEqual({ categories: [cat] }); + expect(res.body.categories.map((c: { name: string }) => c.name)).toEqual(['Aquarium', 'Zoo']); }); it('403 when a non-admin tries to create', async () => { const res = await request(server).post('/api/categories').set('Cookie', sessionCookie(2)).send({ name: 'X' }); expect(res.status).toBe(403); expect(res.body).toEqual({ error: 'Admin access required' }); - expect(mocks.createCategory).not.toHaveBeenCalled(); + expect(db.prepare('SELECT COUNT(*) AS n FROM categories').get().n).toBe(0); }); - it('201 when an admin creates a category', async () => { + it('201 when an admin creates a category, echoing name/color/icon and user_id', async () => { const res = await request(server).post('/api/categories').set('Cookie', sessionCookie(1)).send({ name: 'Food', color: '#fff', icon: '🍔' }); expect(res.status).toBe(201); - expect(res.body).toEqual({ category: cat }); - expect(mocks.createCategory).toHaveBeenCalledWith(1, 'Food', '#fff', '🍔'); + expect(res.body.category).toMatchObject({ name: 'Food', color: '#fff', icon: '🍔', user_id: 1 }); + expect(typeof res.body.category.id).toBe('number'); + }); + + it('201 on create with the #6366f1/📍 defaults when color and icon are omitted', async () => { + const res = await request(server).post('/api/categories').set('Cookie', sessionCookie(1)).send({ name: 'Defaults' }); + expect(res.status).toBe(201); + expect(res.body.category).toMatchObject({ color: '#6366f1', icon: '📍' }); }); it('400 when an admin creates without a name', async () => { @@ -98,22 +110,30 @@ describe('Categories e2e (real JwtAuthGuard + AdminGuard + temp SQLite)', () => expect(res.body).toEqual({ error: 'Category name is required' }); }); - it('200 when an admin updates an existing category', async () => { - const res = await request(server).put('/api/categories/1').set('Cookie', sessionCookie(1)).send({ name: 'Drinks' }); + it('200 when an admin updates, COALESCE preserving omitted fields', async () => { + const id = insertCategory('Food', '#fff', '🍔'); + const res = await request(server).put(`/api/categories/${id}`).set('Cookie', sessionCookie(1)).send({ name: 'Drinks' }); expect(res.status).toBe(200); - expect(res.body).toEqual({ category: { ...cat, name: 'Drinks' } }); + expect(res.body.category).toMatchObject({ id, name: 'Drinks', color: '#fff', icon: '🍔' }); }); it('404 when an admin updates a missing category', async () => { - const res = await request(server).put('/api/categories/9').set('Cookie', sessionCookie(1)).send({ name: 'X' }); + const res = await request(server).put('/api/categories/9999').set('Cookie', sessionCookie(1)).send({ name: 'X' }); expect(res.status).toBe(404); expect(res.body).toEqual({ error: 'Category not found' }); }); - it('200 when an admin deletes an existing category', async () => { - const res = await request(server).delete('/api/categories/1').set('Cookie', sessionCookie(1)); + it('200 when an admin deletes an existing category, removing the row', async () => { + const id = insertCategory('ToDelete'); + const res = await request(server).delete(`/api/categories/${id}`).set('Cookie', sessionCookie(1)); expect(res.status).toBe(200); expect(res.body).toEqual({ success: true }); - expect(mocks.deleteCategory).toHaveBeenCalledWith('1'); + expect(db.prepare('SELECT id FROM categories WHERE id = ?').get(id)).toBeUndefined(); + }); + + it('404 when an admin deletes a missing category', async () => { + const res = await request(server).delete('/api/categories/9999').set('Cookie', sessionCookie(1)); + expect(res.status).toBe(404); + expect(res.body).toEqual({ error: 'Category not found' }); }); }); diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index f63fd2fb8..351b376f3 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -1,6 +1,8 @@ import { createTestRegistry, type McpRegistry } from '@trek/nest-mcp'; import { db } from '../../src/db/database'; import { trekMcpAccessPolicy } from '../../src/mcp/nest-mcp-policy'; +import { CategoriesMcp } from '../../src/nest/categories/categories.mcp'; +import { CategoriesService } from '../../src/nest/categories/categories.service'; import { DatabaseService } from '../../src/nest/database/database.service'; import { TagsMcp } from '../../src/nest/tags/tags.mcp'; import { TagsService } from '../../src/nest/tags/tags.service'; @@ -17,6 +19,7 @@ export function createMcpTestRegistry(): McpRegistry { return createTestRegistry( [ new TagsMcp(new TagsService(dbService)), + new CategoriesMcp(new CategoriesService(dbService)), ], { accessPolicy: trekMcpAccessPolicy }, ); diff --git a/server/tests/unit/mcp/resources.test.ts b/server/tests/unit/mcp/resources.test.ts index f96815027..4e312d0b1 100644 --- a/server/tests/unit/mcp/resources.test.ts +++ b/server/tests/unit/mcp/resources.test.ts @@ -1,6 +1,8 @@ /** * Unit tests for MCP resources (resources.ts). - * Tests all 14 resources via InMemoryTransport + Client. + * Tests all 13 legacy-registrar resources via InMemoryTransport + Client + * (trek://categories moved to the DI-discovered CategoriesMcp — see + * tools-categories.test.ts). */ import { describe, it, expect, vi, beforeAll, beforeEach, afterAll, afterEach } from 'vitest'; @@ -416,21 +418,9 @@ describe('Resource: trek://trips/{tripId}/collab-notes', () => { }); }); -describe('Resource: trek://categories', () => { - it('returns all categories', async () => { - const { user } = createUser(testDb); - - await withHarness(user.id, async (harness) => { - const result = await harness.client.readResource({ uri: 'trek://categories' }); - const categories = parseResourceResult(result) as any[]; - expect(categories.length).toBeGreaterThan(0); - expect(categories[0]).toHaveProperty('id'); - expect(categories[0]).toHaveProperty('name'); - expect(categories[0]).toHaveProperty('color'); - expect(categories[0]).toHaveProperty('icon'); - }); - }); -}); +// trek://categories moved to tools-categories.test.ts: it now registers via +// the nest-mcp registry inside registerTools (CategoriesMcp @Resource), which +// this file's withTools: false harness never attaches. describe('Resource: trek://bucket-list', () => { it('returns only the current user\'s bucket list items', async () => { diff --git a/server/tests/unit/mcp/tools-categories.test.ts b/server/tests/unit/mcp/tools-categories.test.ts new file mode 100644 index 000000000..95e52973f --- /dev/null +++ b/server/tests/unit/mcp/tools-categories.test.ts @@ -0,0 +1,155 @@ +/** + * Unit tests for the categories MCP surface (CategoriesMcp, DI-discovered): + * the list_categories tool and the trek://categories resource. + * + * Both attach via the nest-mcp registry inside registerTools, so every harness + * here keeps withTools on (the resource is NOT registered by the legacy + * registerResources fan-out anymore). + */ +import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; + +const { testDb, dbMock } = vi.hoisted(() => { + const Database = require('better-sqlite3'); + const db = new Database(':memory:'); + db.exec('PRAGMA journal_mode = WAL'); + db.exec('PRAGMA foreign_keys = ON'); + db.exec('PRAGMA busy_timeout = 5000'); + const mock = { + db, + closeDb: () => {}, + reinitialize: () => {}, + getPlaceWithTags: () => null, + canAccessTrip: () => null, + isOwner: () => false, + }; + return { testDb: db, dbMock: mock }; +}); + +vi.mock('../../../src/db/database', () => dbMock); +vi.mock('../../../src/config', () => ({ + JWT_SECRET: 'test-jwt-secret-for-trek-testing-only', + ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', + updateJwtSecret: () => {}, +})); + +import { createTables } from '../../../src/db/schema'; +import { runMigrations } from '../../../src/db/migrations'; +import { resetTestDb } from '../../helpers/test-db'; +import { createUser } from '../../helpers/factories'; +import { createMcpHarness, parseToolResult, parseResourceResult, type McpHarness } from '../../helpers/mcp-harness'; + +beforeAll(() => { + createTables(testDb); + runMigrations(testDb); +}); + +beforeEach(() => { + resetTestDb(testDb); +}); + +afterAll(() => { + testDb.close(); +}); + +async function withHarness( + userId: number, + fn: (h: McpHarness) => Promise, + scopes: string[] | null = null, +) { + const h = await createMcpHarness({ userId, withResources: false, scopes }); + try { await fn(h); } finally { await h.cleanup(); } +} + +// --------------------------------------------------------------------------- +// list_categories +// --------------------------------------------------------------------------- + +describe('Tool: list_categories', () => { + it('returns all categories with id, name, color, icon', async () => { + const { user } = createUser(testDb); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ name: 'list_categories', arguments: {} }); + const data = parseToolResult(result) as any; + expect(data.categories).toBeDefined(); + expect(data.categories.length).toBeGreaterThan(0); + const cat = data.categories[0]; + expect(cat).toHaveProperty('id'); + expect(cat).toHaveProperty('name'); + expect(cat).toHaveProperty('color'); + expect(cat).toHaveProperty('icon'); + }); + }); + + it('returns categories from all users, ordered by name', async () => { + const { user } = createUser(testDb); + const { user: other } = createUser(testDb); + testDb.prepare('INSERT INTO categories (name, color, icon, user_id) VALUES (?, ?, ?, ?)').run('Zzz Mine', '#111111', '🅰️', user.id); + testDb.prepare('INSERT INTO categories (name, color, icon, user_id) VALUES (?, ?, ?, ?)').run('Zzz Other', '#222222', '🅱️', other.id); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ name: 'list_categories', arguments: {} }); + const names = (parseToolResult(result) as any).categories.map((c: { name: string }) => c.name); + expect(names).toContain('Zzz Mine'); + expect(names).toContain('Zzz Other'); + expect(names).toEqual([...names].sort()); + }); + }); +}); + +// --------------------------------------------------------------------------- +// list_categories scope gating (places:read, registration-time) +// --------------------------------------------------------------------------- + +describe('Tool: list_categories — scope gating', () => { + async function listToolNames(userId: number, scopes: string[] | null): Promise { + const h = await createMcpHarness({ userId, withResources: false, scopes }); + try { + return (await h.client.listTools()).tools.map((t) => t.name); + } finally { + await h.cleanup(); + } + } + + it('registers with null scopes (full access)', async () => { + const { user } = createUser(testDb); + expect(await listToolNames(user.id, null)).toContain('list_categories'); + }); + + it('registers with places:read', async () => { + const { user } = createUser(testDb); + expect(await listToolNames(user.id, ['places:read'])).toContain('list_categories'); + }); + + it('does not register for an unrelated scope', async () => { + const { user } = createUser(testDb); + expect(await listToolNames(user.id, ['budget:read'])).not.toContain('list_categories'); + }); +}); + +// --------------------------------------------------------------------------- +// trek://categories resource (first production @Resource) +// --------------------------------------------------------------------------- + +describe('Resource: trek://categories', () => { + it('returns all categories', async () => { + const { user } = createUser(testDb); + await withHarness(user.id, async (h) => { + const result = await h.client.readResource({ uri: 'trek://categories' }); + const categories = parseResourceResult(result) as any[]; + expect(categories.length).toBeGreaterThan(0); + expect(categories[0]).toHaveProperty('id'); + expect(categories[0]).toHaveProperty('name'); + expect(categories[0]).toHaveProperty('color'); + expect(categories[0]).toHaveProperty('icon'); + }); + }); + + it('stays readable under restricted non-places scopes (legacy ungated behavior)', async () => { + const { user } = createUser(testDb); + await withHarness(user.id, async (h) => { + const result = await h.client.readResource({ uri: 'trek://categories' }); + const categories = parseResourceResult(result) as any[]; + expect(Array.isArray(categories)).toBe(true); + expect(categories.length).toBeGreaterThan(0); + }, ['trips:read']); + }); +}); diff --git a/server/tests/unit/mcp/tools-places.test.ts b/server/tests/unit/mcp/tools-places.test.ts index 50c81bf13..5603b7fb7 100644 --- a/server/tests/unit/mcp/tools-places.test.ts +++ b/server/tests/unit/mcp/tools-places.test.ts @@ -1,5 +1,6 @@ /** - * Unit tests for MCP place tools: create_place, update_place, delete_place, list_categories, search_place. + * Unit tests for MCP place tools: create_place, update_place, delete_place, search_place. + * (list_categories moved to tools-categories.test.ts with the CategoriesMcp migration.) */ import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; @@ -356,27 +357,6 @@ describe('Tool: create_and_assign_place', () => { }); }); -// --------------------------------------------------------------------------- -// list_categories -// --------------------------------------------------------------------------- - -describe('Tool: list_categories', () => { - it('returns all categories with id, name, color, icon', async () => { - const { user } = createUser(testDb); - await withHarness(user.id, async (h) => { - const result = await h.client.callTool({ name: 'list_categories', arguments: {} }); - const data = parseToolResult(result) as any; - expect(data.categories).toBeDefined(); - expect(data.categories.length).toBeGreaterThan(0); - const cat = data.categories[0]; - expect(cat).toHaveProperty('id'); - expect(cat).toHaveProperty('name'); - expect(cat).toHaveProperty('color'); - expect(cat).toHaveProperty('icon'); - }); - }); -}); - // --------------------------------------------------------------------------- // search_place // --------------------------------------------------------------------------- diff --git a/server/tests/unit/services/categoryService.test.ts b/server/tests/unit/nest/categories.service.test.ts similarity index 59% rename from server/tests/unit/services/categoryService.test.ts rename to server/tests/unit/nest/categories.service.test.ts index 7f401e239..9c40b339e 100644 --- a/server/tests/unit/services/categoryService.test.ts +++ b/server/tests/unit/nest/categories.service.test.ts @@ -1,6 +1,9 @@ /** - * Unit tests for categoryService — CAT-SVC-001 through CAT-SVC-015. + * Unit tests for CategoriesService — CAT-SVC-001 through CAT-SVC-016. * Uses a real in-memory SQLite DB so SQL logic is exercised faithfully. + * The service is constructed directly (new CategoriesService(new DatabaseService(db))) + * — no Nest container needed. CAT-SVC-016 covers the categories.bridge export + * used by the non-Nest plugin RPC host. */ import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; @@ -34,13 +37,11 @@ import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser } from '../../helpers/factories'; -import { - listCategories, - createCategory, - getCategoryById, - updateCategory, - deleteCategory, -} from '../../../src/services/categoryService'; +import { DatabaseService } from '../../../src/nest/database/database.service'; +import { CategoriesService } from '../../../src/nest/categories/categories.service'; +import { listCategories } from '../../../src/nest/categories/categories.bridge'; + +const svc = new CategoriesService(new DatabaseService(testDb)); beforeAll(() => { createTables(testDb); @@ -55,22 +56,22 @@ afterAll(() => { testDb.close(); }); -// ── listCategories ──────────────────────────────────────────────────────────── +// ── list ────────────────────────────────────────────────────────────────────── -describe('listCategories', () => { +describe('list', () => { it('CAT-SVC-001 — returns an array (seeded defaults are present after migrations)', () => { // Migrations seed default categories, so the list is never empty in a fully initialized DB - const cats = listCategories() as any[]; + const cats = svc.list(); expect(Array.isArray(cats)).toBe(true); expect(cats.length).toBeGreaterThan(0); }); it('CAT-SVC-002 — results are ordered by name ascending (custom categories sort correctly)', () => { const { user } = createUser(testDb); - createCategory(user.id, 'Zoo'); - createCategory(user.id, 'Aquarium'); + svc.create(user.id, 'Zoo'); + svc.create(user.id, 'Aquarium'); // Migrations seed default categories; verify ordering by checking our custom ones appear in sorted order - const names = (listCategories() as any[]).map((c: any) => c.name); + const names = svc.list().map((c) => c.name); const aquariumIdx = names.indexOf('Aquarium'); const zooIdx = names.indexOf('Zoo'); expect(aquariumIdx).toBeGreaterThanOrEqual(0); @@ -81,19 +82,19 @@ describe('listCategories', () => { it('CAT-SVC-003 — returns categories from all users (including seeded defaults)', () => { const { user: a } = createUser(testDb); const { user: b } = createUser(testDb); - const before = (listCategories() as any[]).length; - createCategory(a.id, 'Cat-A'); - createCategory(b.id, 'Cat-B'); - expect(listCategories()).toHaveLength(before + 2); + const before = svc.list().length; + svc.create(a.id, 'Cat-A'); + svc.create(b.id, 'Cat-B'); + expect(svc.list()).toHaveLength(before + 2); }); }); -// ── createCategory ──────────────────────────────────────────────────────────── +// ── create ──────────────────────────────────────────────────────────────────── -describe('createCategory', () => { +describe('create', () => { it('CAT-SVC-004 — creates a category with name, color, and icon', () => { const { user } = createUser(testDb); - const cat = createCategory(user.id, 'Restaurant', '#ff5500', '🍽️') as any; + const cat = svc.create(user.id, 'Restaurant', '#ff5500', '🍽️'); expect(cat.name).toBe('Restaurant'); expect(cat.color).toBe('#ff5500'); expect(cat.icon).toBe('🍽️'); @@ -102,55 +103,55 @@ describe('createCategory', () => { it('CAT-SVC-005 — defaults color to #6366f1 when not provided', () => { const { user } = createUser(testDb); - const cat = createCategory(user.id, 'Default Color') as any; + const cat = svc.create(user.id, 'Default Color'); expect(cat.color).toBe('#6366f1'); }); it('CAT-SVC-006 — defaults icon to 📍 when not provided', () => { const { user } = createUser(testDb); - const cat = createCategory(user.id, 'Default Icon') as any; + const cat = svc.create(user.id, 'Default Icon'); expect(cat.icon).toBe('📍'); }); it('CAT-SVC-007 — returns the inserted row with an id', () => { const { user } = createUser(testDb); - const cat = createCategory(user.id, 'WithId') as any; + const cat = svc.create(user.id, 'WithId'); expect(typeof cat.id).toBe('number'); expect(cat.id).toBeGreaterThan(0); }); }); -// ── getCategoryById ─────────────────────────────────────────────────────────── +// ── getById ─────────────────────────────────────────────────────────────────── -describe('getCategoryById', () => { +describe('getById', () => { it('CAT-SVC-008 — returns category for a valid id', () => { const { user } = createUser(testDb); - const created = createCategory(user.id, 'Find Me') as any; - const found = getCategoryById(created.id) as any; + const created = svc.create(user.id, 'Find Me'); + const found = svc.getById(created.id); expect(found).toBeDefined(); - expect(found.name).toBe('Find Me'); + expect(found?.name).toBe('Find Me'); }); it('CAT-SVC-009 — returns undefined for non-existent id', () => { - expect(getCategoryById(99999)).toBeUndefined(); + expect(svc.getById(99999)).toBeUndefined(); }); it('CAT-SVC-010 — accepts string id (coerced by SQLite)', () => { const { user } = createUser(testDb); - const created = createCategory(user.id, 'StringId') as any; - const found = getCategoryById(String(created.id)) as any; + const created = svc.create(user.id, 'StringId'); + const found = svc.getById(String(created.id)); expect(found).toBeDefined(); - expect(found.id).toBe(created.id); + expect(found?.id).toBe(created.id); }); }); -// ── updateCategory ──────────────────────────────────────────────────────────── +// ── update ──────────────────────────────────────────────────────────────────── -describe('updateCategory', () => { +describe('update', () => { it('CAT-SVC-011 — updates name, color, and icon', () => { const { user } = createUser(testDb); - const cat = createCategory(user.id, 'Old', '#aaaaaa', '❓') as any; - const updated = updateCategory(cat.id, 'New', '#bbbbbb', '✅') as any; + const cat = svc.create(user.id, 'Old', '#aaaaaa', '❓'); + const updated = svc.update(cat.id, 'New', '#bbbbbb', '✅'); expect(updated.name).toBe('New'); expect(updated.color).toBe('#bbbbbb'); expect(updated.icon).toBe('✅'); @@ -158,32 +159,44 @@ describe('updateCategory', () => { it('CAT-SVC-012 — COALESCE: omitting name preserves existing name', () => { const { user } = createUser(testDb); - const cat = createCategory(user.id, 'KeepName', '#aaaaaa', '⭐') as any; - const updated = updateCategory(cat.id, undefined, '#cccccc', '🔥') as any; + const cat = svc.create(user.id, 'KeepName', '#aaaaaa', '⭐'); + const updated = svc.update(cat.id, undefined, '#cccccc', '🔥'); expect(updated.name).toBe('KeepName'); expect(updated.color).toBe('#cccccc'); }); it('CAT-SVC-013 — COALESCE: omitting color preserves existing color', () => { const { user } = createUser(testDb); - const cat = createCategory(user.id, 'KeepColor', '#dddddd', '⭐') as any; - const updated = updateCategory(cat.id, 'NewName', undefined, '🌟') as any; + const cat = svc.create(user.id, 'KeepColor', '#dddddd', '⭐'); + const updated = svc.update(cat.id, 'NewName', undefined, '🌟'); expect(updated.name).toBe('NewName'); expect(updated.color).toBe('#dddddd'); }); }); -// ── deleteCategory ──────────────────────────────────────────────────────────── +// ── remove ──────────────────────────────────────────────────────────────────── -describe('deleteCategory', () => { +describe('remove', () => { it('CAT-SVC-014 — deletes the category from the database', () => { const { user } = createUser(testDb); - const cat = createCategory(user.id, 'ToDelete') as any; - deleteCategory(cat.id); - expect(getCategoryById(cat.id)).toBeUndefined(); + const cat = svc.create(user.id, 'ToDelete'); + svc.remove(cat.id); + expect(svc.getById(cat.id)).toBeUndefined(); }); it('CAT-SVC-015 — deleting a non-existent category does not throw', () => { - expect(() => deleteCategory(99999)).not.toThrow(); + expect(() => svc.remove(99999)).not.toThrow(); + }); +}); + +// ── categories.bridge (non-Nest entry point) ────────────────────────────────── + +describe('categories.bridge', () => { + it('CAT-SVC-016 — listCategories delegates to the service', () => { + const { user } = createUser(testDb); + svc.create(user.id, 'Bridged'); + const names = listCategories().map((c) => c.name); + expect(names).toContain('Bridged'); + expect(names).toEqual([...names].sort()); }); }); diff --git a/server/tests/unit/plugins/create-rpc-host.test.ts b/server/tests/unit/plugins/create-rpc-host.test.ts index c51dfec26..cb2dc2a18 100644 --- a/server/tests/unit/plugins/create-rpc-host.test.ts +++ b/server/tests/unit/plugins/create-rpc-host.test.ts @@ -136,7 +136,7 @@ vi.mock('../../../src/services/packingService', () => ({ })); vi.mock('../../../src/services/conflictResult', () => ({ isUpdateConflict: (r: unknown) => !!(r as { conflict?: boolean })?.conflict })); vi.mock('../../../src/services/weatherService', () => ({ getWeather: vi.fn(async (lat: string, lng: string) => ({ lat, lng, temp: 20 })) })); -vi.mock('../../../src/services/categoryService', () => ({ listCategories: vi.fn(() => [{ id: 1, name: 'Food' }]) })); +vi.mock('../../../src/nest/categories/categories.bridge', () => ({ listCategories: vi.fn(() => [{ id: 1, name: 'Food' }]) })); vi.mock('../../../src/nest/tags/tags.bridge', () => ({ listTags: vi.fn((uid: number) => [{ id: 1, user_id: uid, name: 'work' }]), createTag: vi.fn((uid: number, name: string, color?: string) => ({ id: 9, user_id: uid, name, color })),