refactor: add mandatory db parameter to all service functions for testability

Make db a required first parameter on all service functions (notes-service,
api-utils, tags, collaborators, attachments, rate-limit, sync/server).
This enables unit testing with in-memory databases via createTestDb().

Also removes the NODE_ENV=test bypass from rate-limit and adds optional
now parameter for time control in tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Frederic Cabassut
2026-03-10 10:14:09 +01:00
committed by Fred Cabassut
co-authored by Claude Opus 4.6
parent 79f1e383c3
commit 8795a69b48
18 changed files with 135 additions and 134 deletions
+5 -5
View File
@@ -1,5 +1,5 @@
import { error } from '@sveltejs/kit';
import { db } from './db/index.js';
import type { Db } from './db/index.js';
import { notes, noteCollaborators } from './db/schema.js';
import { eq, and } from 'drizzle-orm';
@@ -21,7 +21,7 @@ export function requireAdmin(event: EventWithLocals) {
/**
* Check if a user can access a note (as owner or collaborator).
*/
export function canAccessNote(noteId: string, userId: number): { canAccess: boolean; isOwner: boolean } {
export function canAccessNote(db: Db, noteId: string, userId: number): { canAccess: boolean; isOwner: boolean } {
const note = db.select({ userId: notes.userId }).from(notes).where(eq(notes.id, noteId)).get();
if (!note) return { canAccess: false, isOwner: false };
if (note.userId === userId) return { canAccess: true, isOwner: true };
@@ -37,8 +37,8 @@ export function canAccessNote(noteId: string, userId: number): { canAccess: bool
/**
* Require that a user can access a note. Throws 404 if no access.
*/
export function requireNoteAccess(noteId: string, userId: number): { isOwner: boolean } {
const { canAccess, isOwner } = canAccessNote(noteId, userId);
export function requireNoteAccess(db: Db, noteId: string, userId: number): { isOwner: boolean } {
const { canAccess, isOwner } = canAccessNote(db, noteId, userId);
if (!canAccess) throw error(404, 'Not found');
return { isOwner };
}
@@ -46,7 +46,7 @@ export function requireNoteAccess(noteId: string, userId: number): { isOwner: bo
/**
* Require that a user is the owner of a note. Throws 404 if not owner.
*/
export function requireNoteOwnership(noteId: string, userId: number): void {
export function requireNoteOwnership(db: Db, noteId: string, userId: number): void {
const note = db.select({ userId: notes.userId }).from(notes).where(eq(notes.id, noteId)).get();
if (!note || note.userId !== userId) throw error(404, 'Not found');
}
+8 -7
View File
@@ -2,7 +2,7 @@ import { existsSync, mkdirSync } from 'fs';
import { writeFile, unlink } from 'fs/promises';
import { join } from 'path';
import { v4 as uuidv4 } from 'uuid';
import { db } from './db/index.js';
import type { Db } from './db/index.js';
import { attachments } from './db/schema.js';
import { eq, and, inArray } from 'drizzle-orm';
import type { Attachment } from '$lib/types/index.js';
@@ -16,6 +16,7 @@ if (!existsSync(ATTACHMENTS_DIR)) {
}
export async function saveAttachment(
db: Db,
noteId: string,
file: File,
userId: number,
@@ -59,7 +60,7 @@ export async function saveAttachment(
* Batch fetch attachments for multiple notes in a single query.
* Returns a Map of noteId → Attachment[].
*/
export function fetchAttachmentsForNotes(noteIds: string[]): Map<string, Attachment[]> {
export function fetchAttachmentsForNotes(db: Db, noteIds: string[]): Map<string, Attachment[]> {
if (noteIds.length === 0) return new Map();
const rows = db
@@ -80,7 +81,7 @@ export function fetchAttachmentsForNotes(noteIds: string[]): Map<string, Attachm
return map;
}
export async function getAttachment(id: string, noteId: string) {
export async function getAttachment(db: Db, id: string, noteId: string) {
return db
.select()
.from(attachments)
@@ -88,11 +89,11 @@ export async function getAttachment(id: string, noteId: string) {
.get();
}
export async function getAttachmentsByNote(noteId: string) {
export async function getAttachmentsByNote(db: Db, noteId: string) {
return db.select().from(attachments).where(eq(attachments.noteId, noteId));
}
export async function updateAttachment(id: string, noteId: string, data: { featured: boolean }) {
export async function updateAttachment(db: Db, id: string, noteId: string, data: { featured: boolean }) {
const [updated] = await db
.update(attachments)
.set({ featured: data.featured })
@@ -101,8 +102,8 @@ export async function updateAttachment(id: string, noteId: string, data: { featu
return updated ?? null;
}
export async function deleteAttachment(id: string, noteId: string) {
const attachment = await getAttachment(id, noteId);
export async function deleteAttachment(db: Db, id: string, noteId: string) {
const attachment = await getAttachment(db, id, noteId);
if (attachment) {
try {
await unlink(attachment.path);
+5 -5
View File
@@ -1,4 +1,4 @@
import { db } from './db/index.js';
import type { Db } from './db/index.js';
import { noteCollaborators, noteUserState, users } from './db/schema.js';
import { eq, and, inArray } from 'drizzle-orm';
import type { Collaborator } from '$lib/types/index.js';
@@ -7,7 +7,7 @@ import type { Collaborator } from '$lib/types/index.js';
* Batch fetch collaborators for multiple notes.
* Returns a Map of noteId → Collaborator[].
*/
export function fetchCollaboratorsForNotes(noteIds: string[]): Map<string, Collaborator[]> {
export function fetchCollaboratorsForNotes(db: Db, noteIds: string[]): Map<string, Collaborator[]> {
if (noteIds.length === 0) return new Map();
const rows = db
@@ -44,7 +44,7 @@ export function fetchCollaboratorsForNotes(noteIds: string[]): Map<string, Colla
/**
* Add a collaborator to a note.
*/
export function addCollaborator(noteId: string, userId: number, addedBy: number): void {
export function addCollaborator(db: Db, noteId: string, userId: number, addedBy: number): void {
db.insert(noteCollaborators)
.values({ noteId, userId, addedBy, addedAt: new Date() })
.run();
@@ -53,7 +53,7 @@ export function addCollaborator(noteId: string, userId: number, addedBy: number)
/**
* Remove a collaborator from a note. Also cleans up their per-user state.
*/
export function removeCollaborator(noteId: string, userId: number): void {
export function removeCollaborator(db: Db, noteId: string, userId: number): void {
db.delete(noteCollaborators)
.where(and(eq(noteCollaborators.noteId, noteId), eq(noteCollaborators.userId, userId)))
.run();
@@ -65,7 +65,7 @@ export function removeCollaborator(noteId: string, userId: number): void {
/**
* Get all collaborator user IDs for a note.
*/
export function getCollaboratorIds(noteId: string): number[] {
export function getCollaboratorIds(db: Db, noteId: string): number[] {
return db
.select({ userId: noteCollaborators.userId })
.from(noteCollaborators)
+1
View File
@@ -247,4 +247,5 @@ if (pwCol && pwCol.notnull) {
sqlite.pragma('foreign_keys = ON');
export const db = drizzle(sqlite, { schema });
export type Db = import('drizzle-orm/better-sqlite3').BetterSQLite3Database<typeof schema>;
export { sqlite };
+17 -16
View File
@@ -12,6 +12,7 @@ import {
reorderNotes
} from '../notes-service.js';
import { saveAttachment } from '../attachments.js';
import { db } from '../db/index.js';
const NOTE_COLORS = [
'default',
@@ -45,7 +46,7 @@ export function createMcpServer(userId: number): McpServer {
tag: z.string().optional().describe('Filter by tag name')
},
async ({ filter = 'all', tag }: { filter?: string; tag?: string }) => {
let result = listNotes(userId, filter as NoteFilter);
let result = listNotes(db, userId, filter as NoteFilter);
if (tag) {
result = result.filter((n) =>
@@ -64,7 +65,7 @@ export function createMcpServer(userId: number): McpServer {
'Get a single note by ID with its tags and attachments',
{ id: z.string().describe('Note ID') },
async ({ id }: { id: string }) => {
const note = getNote(userId, id);
const note = getNote(db, userId, id);
if (!note) {
return {
content: [{ type: 'text' as const, text: 'Note not found' }],
@@ -88,7 +89,7 @@ export function createMcpServer(userId: number): McpServer {
checklistMode: z.boolean().optional().describe('Enable checklist mode')
},
async ({ title, content, color, checklistMode }: { title?: string; content?: string; color?: string; checklistMode?: boolean }) => {
const note = createNote(userId, { title, content, color, checklistMode });
const note = createNote(db, userId, { title, content, color, checklistMode });
return {
content: [{ type: 'text' as const, text: JSON.stringify(note, null, 2) }]
};
@@ -107,7 +108,7 @@ export function createMcpServer(userId: number): McpServer {
checklistMode: z.boolean().optional().describe('Enable/disable checklist mode')
},
async ({ id, ...updates }: { id: string; title?: string; content?: string; color?: string; pinned?: boolean; checklistMode?: boolean }) => {
const result = updateNote(userId, id, updates);
const result = updateNote(db, userId, id, updates);
if (!result) {
return {
content: [{ type: 'text' as const, text: 'Note not found' }],
@@ -126,7 +127,7 @@ export function createMcpServer(userId: number): McpServer {
'Move a note to trash',
{ id: z.string().describe('Note ID') },
async ({ id }: { id: string }) => {
const result = updateNote(userId, id, { trashed: true });
const result = updateNote(db, userId, id, { trashed: true });
if (!result) {
return {
content: [{ type: 'text' as const, text: 'Note not found' }],
@@ -144,7 +145,7 @@ export function createMcpServer(userId: number): McpServer {
'Restore a note from trash',
{ id: z.string().describe('Note ID') },
async ({ id }: { id: string }) => {
const result = updateNote(userId, id, { trashed: false });
const result = updateNote(db, userId, id, { trashed: false });
if (!result) {
return {
content: [{ type: 'text' as const, text: 'Note not found' }],
@@ -162,7 +163,7 @@ export function createMcpServer(userId: number): McpServer {
'Archive a note',
{ id: z.string().describe('Note ID') },
async ({ id }: { id: string }) => {
const result = updateNote(userId, id, { archived: true });
const result = updateNote(db, userId, id, { archived: true });
if (!result) {
return {
content: [{ type: 'text' as const, text: 'Note not found' }],
@@ -180,7 +181,7 @@ export function createMcpServer(userId: number): McpServer {
'Unarchive a note',
{ id: z.string().describe('Note ID') },
async ({ id }: { id: string }) => {
const result = updateNote(userId, id, { archived: false });
const result = updateNote(db, userId, id, { archived: false });
if (!result) {
return {
content: [{ type: 'text' as const, text: 'Note not found' }],
@@ -198,7 +199,7 @@ export function createMcpServer(userId: number): McpServer {
'Permanently delete a note',
{ id: z.string().describe('Note ID') },
async ({ id }: { id: string }) => {
const existing = getNote(userId, id);
const existing = getNote(db, userId, id);
if (!existing) {
return {
content: [{ type: 'text' as const, text: 'Note not found' }],
@@ -206,7 +207,7 @@ export function createMcpServer(userId: number): McpServer {
};
}
deleteNote(userId, id);
deleteNote(db, userId, id);
return {
content: [
{
@@ -223,7 +224,7 @@ export function createMcpServer(userId: number): McpServer {
'Search notes by title, content, or tag name',
{ query: z.string().describe('Search query') },
async ({ query }: { query: string }) => {
const results = searchNotes(userId, query);
const results = searchNotes(db, userId, query);
return {
content: [{ type: 'text' as const, text: JSON.stringify(results, null, 2) }]
};
@@ -231,7 +232,7 @@ export function createMcpServer(userId: number): McpServer {
);
server.tool('list_tags', 'List all tags', {}, async () => {
const allTags = listAllTags(userId);
const allTags = listAllTags(db, userId);
return {
content: [{ type: 'text' as const, text: JSON.stringify(allTags, null, 2) }]
};
@@ -245,7 +246,7 @@ export function createMcpServer(userId: number): McpServer {
pinned: z.boolean().describe('Whether to pin (true) or unpin (false)')
},
async ({ id, pinned }: { id: string; pinned: boolean }) => {
const result = updateNote(userId, id, { pinned });
const result = updateNote(db, userId, id, { pinned });
if (!result) {
return {
content: [{ type: 'text' as const, text: 'Note not found' }],
@@ -277,7 +278,7 @@ export function createMcpServer(userId: number): McpServer {
.describe('Array of note IDs with their new sort orders')
},
async ({ orders }: { orders: { id: string; sortOrder: number }[] }) => {
reorderNotes(userId, orders);
reorderNotes(db, userId, orders);
return {
content: [{ type: 'text' as const, text: `Reordered ${orders.length} notes` }]
};
@@ -292,7 +293,7 @@ export function createMcpServer(userId: number): McpServer {
imageUrl: z.string().url().describe('URL of the image to fetch')
},
async ({ noteId, imageUrl }: { noteId: string; imageUrl: string }) => {
const existing = getNote(userId, noteId);
const existing = getNote(db, userId, noteId);
if (!existing) {
return {
content: [{ type: 'text' as const, text: 'Note not found' }],
@@ -320,7 +321,7 @@ export function createMcpServer(userId: number): McpServer {
const filename = `upload_${Date.now()}.${ext}`;
const file = new File([buffer], filename, { type: contentType });
const attachment = await saveAttachment(noteId, file, userId);
const attachment = await saveAttachment(db, noteId, file, userId);
return {
content: [{ type: 'text' as const, text: JSON.stringify(attachment, null, 2) }]
+24 -26
View File
@@ -1,4 +1,4 @@
import { db as defaultDb } from './db/index.js';
import type { Db } from './db/index.js';
import { notes, noteTags, tags, noteCollaborators, noteUserState } from './db/schema.js';
import { eq, and, desc, like, or, inArray, sql } from 'drizzle-orm';
import { v4 as uuidv4 } from 'uuid';
@@ -9,8 +9,6 @@ import { fetchCollaboratorsForNotes } from './collaborators.js';
import { canAccessNote } from './api-utils.js';
import type { NoteFilter } from '$lib/types/index.js';
const db = defaultDb;
interface NoteRow {
id: string;
userId: number;
@@ -28,11 +26,11 @@ interface NoteRow {
version: number;
}
function hydrateNotes(noteRows: NoteRow[], userId: number) {
function hydrateNotes(db: Db, noteRows: NoteRow[], userId: number) {
const noteIds = noteRows.map((n) => n.id);
const tagMap = fetchTagsForNotes(noteIds);
const attachmentMap = fetchAttachmentsForNotes(noteIds);
const collabMap = fetchCollaboratorsForNotes(noteIds);
const tagMap = fetchTagsForNotes(db, noteIds);
const attachmentMap = fetchAttachmentsForNotes(db, noteIds);
const collabMap = fetchCollaboratorsForNotes(db, noteIds);
return noteRows.map((note) => {
const collaborators = collabMap.get(note.id) ?? [];
@@ -52,7 +50,7 @@ function hydrateNotes(noteRows: NoteRow[], userId: number) {
/**
* Get shared notes for a user with per-user state overlay.
*/
function getSharedNotes(userId: number, filter: 'all' | 'archived'): NoteRow[] {
function getSharedNotes(db: Db, userId: number, filter: 'all' | 'archived'): NoteRow[] {
const rows = db
.select({
id: notes.id,
@@ -90,7 +88,7 @@ function getSharedNotes(userId: number, filter: 'all' | 'archived'): NoteRow[] {
return rows as NoteRow[];
}
export function listNotes(userId: number, filter: NoteFilter = 'all') {
export function listNotes(db: Db, userId: number, filter: NoteFilter = 'all') {
// Owned notes
let conditions;
switch (filter) {
@@ -114,7 +112,7 @@ export function listNotes(userId: number, filter: NoteFilter = 'all') {
// Shared notes (not for trashed filter — collaborators don't see trash)
let sharedNotes: NoteRow[] = [];
if (filter !== 'trashed') {
sharedNotes = getSharedNotes(userId, filter === 'archived' ? 'archived' : 'all');
sharedNotes = getSharedNotes(db, userId, filter === 'archived' ? 'archived' : 'all');
}
const combined = [...ownedNotes, ...sharedNotes];
@@ -124,11 +122,11 @@ export function listNotes(userId: number, filter: NoteFilter = 'all') {
return b.updatedAt.getTime() - a.updatedAt.getTime();
});
return hydrateNotes(combined, userId);
return hydrateNotes(db, combined, userId);
}
export function getNote(userId: number, id: string) {
const { canAccess, isOwner } = canAccessNote(id, userId);
export function getNote(db: Db, userId: number, id: string) {
const { canAccess, isOwner } = canAccessNote(db, id, userId);
if (!canAccess) return null;
const note = db.select().from(notes).where(eq(notes.id, id)).get();
@@ -151,7 +149,7 @@ export function getNote(userId: number, id: string) {
};
}
const result = hydrateNotes([effectiveNote], userId);
const result = hydrateNotes(db, [effectiveNote], userId);
return result[0] ?? null;
}
@@ -165,7 +163,7 @@ export interface CreateNoteInput {
sortOrder?: number;
}
export function createNote(userId: number, input: CreateNoteInput) {
export function createNote(db: Db, userId: number, input: CreateNoteInput) {
const now = new Date();
const id = input.id || uuidv4();
@@ -192,7 +190,7 @@ export function createNote(userId: number, input: CreateNoteInput) {
db.transaction((tx) => {
tx.insert(notes).values(newNote).run();
});
syncNoteTags(id, extractedTags, userId);
syncNoteTags(db, id, extractedTags, userId);
return { ...newNote, tags: extractedTags };
}
@@ -211,8 +209,8 @@ export interface UpdateNoteInput {
/** Per-user fields that go to noteUserState for collaborators */
const PER_USER_FIELDS = ['pinned', 'archived', 'sortOrder'] as const;
export function updateNote(userId: number, id: string, input: UpdateNoteInput) {
const { canAccess, isOwner } = canAccessNote(id, userId);
export function updateNote(db: Db, userId: number, id: string, input: UpdateNoteInput) {
const { canAccess, isOwner } = canAccessNote(db, id, userId);
if (!canAccess) return null;
const existing = db.select().from(notes).where(eq(notes.id, id)).get();
@@ -291,14 +289,14 @@ export function updateNote(userId: number, id: string, input: UpdateNoteInput) {
if (updated) {
const content = `${updated.title} ${updated.content}`;
const extractedTags = extractTags(content);
syncNoteTags(id, extractedTags, existing.userId);
syncNoteTags(db, id, extractedTags, existing.userId);
}
}
return getNote(userId, id);
return getNote(db, userId, id);
}
export function deleteNote(userId: number, id: string): boolean {
export function deleteNote(db: Db, userId: number, id: string): boolean {
const existing = db
.select()
.from(notes)
@@ -312,7 +310,7 @@ export function deleteNote(userId: number, id: string): boolean {
return true;
}
export function searchNotes(userId: number, query: string) {
export function searchNotes(db: Db, userId: number, query: string) {
if (!query) return [];
const pattern = `%${query}%`;
@@ -386,17 +384,17 @@ export function searchNotes(userId: number, query: string) {
}
const combined = [...ownedResults, ...sharedResults, ...extraNotes] as NoteRow[];
return hydrateNotes(combined, userId);
return hydrateNotes(db, combined, userId);
}
export function listAllTags(userId: number) {
export function listAllTags(db: Db, userId: number) {
return db.select().from(tags).where(eq(tags.userId, userId)).all();
}
export function reorderNotes(userId: number, orders: { id: string; sortOrder: number }[]) {
export function reorderNotes(db: Db, userId: number, orders: { id: string; sortOrder: number }[]) {
const now = new Date();
for (const { id, sortOrder } of orders) {
const { canAccess, isOwner } = canAccessNote(id, userId);
const { canAccess, isOwner } = canAccessNote(db, id, userId);
if (!canAccess) continue;
if (isOwner) {
+11 -13
View File
@@ -1,4 +1,4 @@
import { db } from './db/index.js';
import type { Db } from './db/index.js';
import { loginAttempts } from './db/schema.js';
import { and, eq, gte, sql } from 'drizzle-orm';
@@ -7,15 +7,13 @@ const WINDOW_MS = 60 * 1000; // 1 minute
const LOCKOUT_MS = 15 * 60 * 1000; // 15 minutes
export function checkRateLimit(
db: Db,
ip: string,
email: string
email: string,
now?: Date
): { allowed: boolean; retryAfter?: number } {
// Skip rate limiting in test environment
if (process.env.NODE_ENV === 'test' || process.env.PLAYWRIGHT_TEST === '1') {
return { allowed: true };
}
const windowStart = new Date(Date.now() - LOCKOUT_MS);
const currentTime = now ?? new Date();
const windowStart = new Date(currentTime.getTime() - LOCKOUT_MS);
// Count failed attempts from this IP in the lockout window
const result = db
@@ -50,7 +48,7 @@ export function checkRateLimit(
if (latest) {
const retryAfter = Math.ceil(
(latest.timestamp.getTime() + LOCKOUT_MS - Date.now()) / 1000
(latest.timestamp.getTime() + LOCKOUT_MS - currentTime.getTime()) / 1000
);
return { allowed: false, retryAfter: Math.max(retryAfter, 1) };
}
@@ -60,19 +58,19 @@ export function checkRateLimit(
return { allowed: true };
}
export function recordLoginAttempt(ip: string, email: string, success: boolean): void {
export function recordLoginAttempt(db: Db, ip: string, email: string, success: boolean, now?: Date): void {
db.insert(loginAttempts)
.values({
ip,
email,
success,
timestamp: new Date()
timestamp: now ?? new Date()
})
.run();
}
export function clearOldAttempts(): void {
const cutoff = new Date(Date.now() - LOCKOUT_MS * 2);
export function clearOldAttempts(db: Db, now?: Date): void {
const cutoff = new Date((now ?? new Date()).getTime() - LOCKOUT_MS * 2);
db.delete(loginAttempts)
.where(sql`${loginAttempts.timestamp} < ${cutoff}`)
.run();
+3 -3
View File
@@ -1,4 +1,4 @@
import { db } from './db/index.js';
import type { Db } from './db/index.js';
import { noteTags, tags } from './db/schema.js';
import { eq, and, inArray } from 'drizzle-orm';
@@ -6,7 +6,7 @@ import { eq, and, inArray } from 'drizzle-orm';
* Batch fetch tags for multiple notes in a single query.
* Returns a Map of noteId → tag names.
*/
export function fetchTagsForNotes(noteIds: string[]): Map<string, string[]> {
export function fetchTagsForNotes(db: Db, noteIds: string[]): Map<string, string[]> {
if (noteIds.length === 0) return new Map();
const rows = db
@@ -32,7 +32,7 @@ export function fetchTagsForNotes(noteIds: string[]): Map<string, string[]> {
* Sync tags for a note: removes old associations, upserts tags, creates new associations.
* Should be called within a transaction for atomicity.
*/
export function syncNoteTags(noteId: string, tagNames: string[], userId: number) {
export function syncNoteTags(db: Db, noteId: string, tagNames: string[], userId: number) {
db.delete(noteTags).where(eq(noteTags.noteId, noteId)).run();
for (const name of tagNames) {