Bookmarks in DB - API

e2e/bookmarks-api.spec.ts

@@ -0,0 +1,102 @@
+import { test, expect } from './support/fixtures.js';
+import { waitForHydration } from './support/helpers.js';
+
+/**
+ * Bookmark API end to end against the real database: the create/list/update/
+ * delete lifecycle and per-user isolation. Request validation and the 401
+ * without a user are covered by the unit tests next to the route handlers;
+ * the 401 without a session by the generic API guard tests.
+ */
+test.describe('Bookmarks API', () => {
+  // BOOKMARK_LIMIT in src/lib/dashboard/types.ts.
+  const BOOKMARK_LIMIT = 25;
+  const FIELDS = {
+    productId: 'trino',
+    name: 'API Trino',
+    environment: 'prod',
+    url: 'https://trino.example.com',
+    pinned: false
+  };
+
+  test('creates, lists, updates and deletes a bookmark', async ({ page }) => {
+    const created = await page.request.post('/api/bookmarks', { data: FIELDS });
+    expect(created.status()).toBe(201);
+    const bookmark = await created.json();
+    expect(bookmark).toEqual({
+      ...FIELDS,
+      id: expect.any(String),
+      createdAt: expect.any(String)
+    });
+
+    try {
+      const listed = await page.request.get('/api/bookmarks');
+      expect(listed.status()).toBe(200);
+      expect(await listed.json()).toContainEqual(bookmark);
+
+      const updated = await page.request.patch(`/api/bookmarks/${bookmark.id}`, {
+        data: { pinned: true, name: 'Renamed Trino' }
+      });
+      expect(updated.status()).toBe(200);
+      expect(await updated.json()).toEqual({ ...bookmark, pinned: true, name: 'Renamed Trino' });
+    } finally {
+      const deleted = await page.request.delete(`/api/bookmarks/${bookmark.id}`);
+      expect(deleted.status()).toBe(204);
+    }
+
+    const listed = await page.request.get('/api/bookmarks');
+    expect((await listed.json()).map((b: { id: string }) => b.id)).not.toContain(bookmark.id);
+    const deletedAgain = await page.request.delete(`/api/bookmarks/${bookmark.id}`);
+    expect(deletedAgain.status()).toBe(404);
+  });
+
+  test('rejects bookmarks beyond the per-user limit with 409', async ({ page }) => {
+    const existing = (await (await page.request.get('/api/bookmarks')).json()).length;
+    const created: string[] = [];
+    try {
+      for (let i = existing; i < BOOKMARK_LIMIT; i++) {
+        const response = await page.request.post('/api/bookmarks', {
+          data: { ...FIELDS, name: `Limit ${i}` }
+        });
+        expect(response.status()).toBe(201);
+        created.push((await response.json()).id);
+      }
+
+      const rejected = await page.request.post('/api/bookmarks', { data: FIELDS });
+      expect(rejected.status()).toBe(409);
+    } finally {
+      for (const id of created) await page.request.delete(`/api/bookmarks/${id}`);
+    }
+  });
+
+  test('keeps bookmarks private to their owner', async ({ page, browser, baseURL }) => {
+    const bookmark = await (await page.request.post('/api/bookmarks', { data: FIELDS })).json();
+
+    // The mock OIDC server issues a new user for every login.
+    const otherContext = await browser.newContext({
+      baseURL,
+      storageState: { cookies: [], origins: [] }
+    });
+    try {
+      const other = await otherContext.newPage();
+      await other.goto('/');
+      await waitForHydration(other);
+      await other.getByRole('button', { name: /sign in with sso/i }).click();
+      await expect(other).toHaveURL('/');
+
+      const listed = await other.request.get('/api/bookmarks');
+      expect((await listed.json()).map((b: { id: string }) => b.id)).not.toContain(bookmark.id);
+      const patched = await other.request.patch(`/api/bookmarks/${bookmark.id}`, {
+        data: { pinned: true }
+      });
+      expect(patched.status()).toBe(404);
+      const deleted = await other.request.delete(`/api/bookmarks/${bookmark.id}`);
+      expect(deleted.status()).toBe(404);
+
+      const owned = await page.request.get('/api/bookmarks');
+      expect(await owned.json()).toContainEqual(bookmark);
+    } finally {
+      await otherContext.close();
+      await page.request.delete(`/api/bookmarks/${bookmark.id}`);
+    }
+  });
+});

e2e/bookmarks.spec.ts

@@ -115,7 +115,8 @@ test.describe('Dashboard bookmarks', () => {
   test('truncates long names and hostnames instead of overlapping the card buttons', async ({
     page
   }) => {
-    const longName = 'Very long bookmark name '.repeat(6).trim();
+    // Names are capped at 40 characters; wide glyphs still overflow the card.
+    const longName = 'W'.repeat(40);
     const longHost = `${'very-long-subdomain-'.repeat(5)}example.com`;
     await page.addInitScript(
       ({ name, host }) => {

src/lib/components/dashboard/AddBookmarkModal.svelte

@@ -4,7 +4,12 @@
   import { getProduct, PRODUCTS, type Product } from '$lib/dashboard/products';
   import ProductIcon from './ProductIcon.svelte';
   import { addBookmark, removeBookmark, updateBookmark } from '$lib/dashboard/bookmarks.svelte.js';
-  import { bookmarkFormSchema, bookmarkHostname, type Bookmark } from '$lib/dashboard/types';
+  import {
+    BOOKMARK_MAX_LENGTH,
+    bookmarkFormSchema,
+    bookmarkHostname,
+    type Bookmark
+  } from '$lib/dashboard/types';
   import * as m from '$lib/paraglide/messages.js';
   import IconClose from 'virtual:icons/material-symbols/close';
 
@@ -171,6 +176,7 @@
               type="text"
               bind:value={name}
               oninput={handleNameInput}
+              maxlength={BOOKMARK_MAX_LENGTH.name}
               placeholder={selectedProduct.defaultName}
               class="input w-full"
               aria-invalid={nameError}
@@ -188,6 +194,7 @@
               id="{uid}-env"
               type="text"
               bind:value={environment}
+              maxlength={BOOKMARK_MAX_LENGTH.environment}
               placeholder={m.bookmark_env_placeholder()}
               class="input w-full"
             />
@@ -205,6 +212,7 @@
             bind:value={url}
             oninput={handleUrlInput}
             onblur={handleUrlBlur}
+            maxlength={BOOKMARK_MAX_LENGTH.url}
             placeholder={m.bookmark_url_placeholder()}
             class="input w-full"
             aria-invalid={urlError}

src/lib/dashboard/product-ids.ts

@@ -0,0 +1,19 @@
+/**
+ * Ids of the products a bookmark can point to; `custom` is a plain link. Kept
+ * apart from `products.ts` so the server can validate ids without loading the
+ * logos and messages.
+ */
+export const PRODUCT_IDS = [
+  'trino',
+  'superset',
+  'airflow',
+  'nifi',
+  'druid',
+  'opensearch',
+  'hdfs',
+  'hbase',
+  'spark',
+  'custom'
+] as const;
+
+export type ProductId = (typeof PRODUCT_IDS)[number];

src/lib/dashboard/products.test.ts

@@ -1,5 +1,6 @@
 import { describe, expect, it } from 'vitest';
 import { getProduct, PRODUCTS } from './products';
+import { PRODUCT_IDS } from './product-ids';
 
 /** WCAG 2.1 relative luminance of a `#RRGGBB` colour. */
 function luminance(hex: string): number {
@@ -23,6 +24,10 @@ describe('PRODUCTS', () => {
     }
   );
 
+  it('defines a product for every product id, in the same order', () => {
+    expect(PRODUCTS.map((p) => p.id)).toEqual(PRODUCT_IDS);
+  });
+
   it('falls back to the custom product for unknown ids', () => {
     expect(getProduct('does-not-exist').id).toBe('custom');
   });

src/lib/dashboard/products.ts

@@ -1,8 +1,9 @@
 import type { Picture } from '@sveltejs/enhanced-img';
 import * as m from '$lib/paraglide/messages.js';
+import type { ProductId } from './product-ids';
 
 export interface Product {
-  id: string;
+  id: ProductId;
   /** Proper noun, except for the custom link, which is localised. */
   readonly name: string;
   initials: string;

src/lib/dashboard/types.ts

@@ -1,10 +1,17 @@
 import { z } from 'zod';
+import { PRODUCT_IDS } from './product-ids';
+
+/** Maximum field lengths, enforced by the API and as `maxlength` in the form. */
+export const BOOKMARK_MAX_LENGTH = { name: 40, environment: 100, url: 2048 } as const;
+
+/** Maximum number of bookmarks per user, enforced by the API. */
+export const BOOKMARK_LIMIT = 25;
 
 export const bookmarkFormSchema = z.object({
-  productId: z.string(),
-  name: z.string().trim().min(1),
-  environment: z.string(),
-  url: z.url({ protocol: /^https?$/ }),
+  productId: z.string().min(1).max(64),
+  name: z.string().trim().min(1).max(BOOKMARK_MAX_LENGTH.name),
+  environment: z.string().trim().max(BOOKMARK_MAX_LENGTH.environment),
+  url: z.url({ protocol: /^https?$/ }).max(BOOKMARK_MAX_LENGTH.url),
   pinned: z.boolean()
 });
 
@@ -15,6 +22,21 @@ export const bookmarkSchema = bookmarkFormSchema.extend({
 
 export type Bookmark = z.infer<typeof bookmarkSchema>;
 
+/**
+ * Body of a bookmark create request. Stored bookmarks keep any product id, so a
+ * product removed later still shows as a custom link; new ones must be known.
+ */
+export const bookmarkCreateSchema = bookmarkFormSchema.extend({
+  productId: z.enum(PRODUCT_IDS)
+});
+
+/** Body of a bookmark update: any subset of the create fields, but at least one. */
+export const bookmarkUpdateSchema = bookmarkCreateSchema
+  .partial()
+  .refine((update) => Object.keys(update).length > 0, { message: 'No fields to update' });
+
+export type BookmarkUpdate = z.infer<typeof bookmarkUpdateSchema>;
+
 /** Hostname of a bookmark URL, or the input unchanged if it does not parse. */
 export function bookmarkHostname(url: string): string {
   try {

src/lib/server/auth-utils.ts

@@ -1,3 +1,14 @@
+import { error } from '@sveltejs/kit';
+
 export function getUserId(locals: App.Locals): string {
   return locals.user?.id ?? 'anonymous';
 }
+
+/**
+ * The signed-in user's id for per-user API routes. Without OIDC there is no
+ * user (the auth guard does not run), so such routes answer 401.
+ */
+export function requireUserId(locals: App.Locals): string {
+  if (!locals.user) throw error(401, 'Authentication required');
+  return locals.user.id;
+}

src/lib/server/dashboard/bookmarks-db.spec.ts

@@ -0,0 +1,112 @@
+import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest';
+import { and, asc, eq } from 'drizzle-orm';
+import { userBookmarks } from '$lib/server/schema.js';
+import { BOOKMARK_LIMIT } from '$lib/dashboard/types.js';
+import { listBookmarks, createBookmark, updateBookmark, deleteBookmark } from './bookmarks-db.js';
+
+vi.mock('$lib/server/logging', () => ({
+  logger: { child: () => ({ info: vi.fn(), debug: vi.fn() }) }
+}));
+
+const { mockDb } = vi.hoisted(() => ({
+  mockDb: { select: vi.fn(), insert: vi.fn(), update: vi.fn(), delete: vi.fn(), $count: vi.fn() }
+}));
+
+vi.mock('$lib/server/db.js', () => ({ db: mockDb }));
+
+type ChainFn = (...args: unknown[]) => MockChain;
+
+/** Mocked Drizzle query builder: every call returns the same chain, which is
+ *  thenable and resolves to `result`. */
+interface MockChain {
+  from: Mock<ChainFn>;
+  where: Mock<ChainFn>;
+  orderBy: Mock<ChainFn>;
+  values: Mock<ChainFn>;
+  set: Mock<ChainFn>;
+  returning: Mock<ChainFn>;
+  then: (resolve: (value: unknown) => unknown) => Promise<unknown>;
+}
+
+function buildChain(result: unknown): MockChain {
+  const chain: MockChain = {
+    from: vi.fn<ChainFn>(() => chain),
+    where: vi.fn<ChainFn>(() => chain),
+    orderBy: vi.fn<ChainFn>(() => chain),
+    values: vi.fn<ChainFn>(() => chain),
+    set: vi.fn<ChainFn>(() => chain),
+    returning: vi.fn<ChainFn>(() => chain),
+    then: (resolve) => Promise.resolve(result).then(resolve)
+  };
+  return chain;
+}
+
+const USER_ID = 'user-1';
+const BOOKMARK_ID = '3f1c2a9e-7b4d-4c8a-9e2f-5d6b7a8c9d01';
+const FIELDS = {
+  productId: 'trino',
+  name: 'Production Trino',
+  environment: 'prod',
+  url: 'https://trino.example.com',
+  pinned: false
+};
+const ROW = { id: BOOKMARK_ID, ...FIELDS, createdAt: new Date('2026-10-27T10:00:00.000Z') };
+const BOOKMARK = { ...ROW, createdAt: '2026-10-27T10:00:00.000Z' };
+const ownBookmark = and(eq(userBookmarks.userId, USER_ID), eq(userBookmarks.id, BOOKMARK_ID));
+
+describe('bookmarks-db', () => {
+  beforeEach(() => vi.clearAllMocks());
+
+  it('lists the user’s bookmarks oldest first with ISO timestamps', async () => {
+    const chain = buildChain([ROW]);
+    mockDb.select.mockReturnValue(chain);
+
+    await expect(listBookmarks(USER_ID)).resolves.toEqual([BOOKMARK]);
+    expect(chain.from).toHaveBeenCalledWith(userBookmarks);
+    expect(chain.where).toHaveBeenCalledWith(eq(userBookmarks.userId, USER_ID));
+    expect(chain.orderBy).toHaveBeenCalledWith(asc(userBookmarks.createdAt), asc(userBookmarks.id));
+  });
+
+  it('creates a bookmark owned by the user', async () => {
+    const chain = buildChain([ROW]);
+    mockDb.$count.mockResolvedValue(BOOKMARK_LIMIT - 1);
+    mockDb.insert.mockReturnValue(chain);
+    await expect(createBookmark(USER_ID, FIELDS)).resolves.toEqual(BOOKMARK);
+    expect(mockDb.$count).toHaveBeenCalledWith(userBookmarks, eq(userBookmarks.userId, USER_ID));
+    expect(mockDb.insert).toHaveBeenCalledWith(userBookmarks);
+    expect(chain.values).toHaveBeenCalledWith({ userId: USER_ID, ...FIELDS });
+  });
+
+  it('creates nothing once the user has reached the bookmark limit', async () => {
+    mockDb.$count.mockResolvedValue(BOOKMARK_LIMIT);
+
+    await expect(createBookmark(USER_ID, FIELDS)).resolves.toBeNull();
+    expect(mockDb.insert).not.toHaveBeenCalled();
+  });
+
+  it('updates only the user’s own bookmark and returns null when there is none', async () => {
+    const chain = buildChain([{ ...ROW, pinned: true }]);
+    mockDb.update.mockReturnValue(chain);
+
+    await expect(updateBookmark(USER_ID, BOOKMARK_ID, { pinned: true })).resolves.toEqual({
+      ...BOOKMARK,
+      pinned: true
+    });
+    expect(chain.set).toHaveBeenCalledWith({ pinned: true });
+    expect(chain.where).toHaveBeenCalledWith(ownBookmark);
+
+    mockDb.update.mockReturnValue(buildChain([]));
+    await expect(updateBookmark(USER_ID, BOOKMARK_ID, { pinned: true })).resolves.toBeNull();
+  });
+
+  it('deletes only the user’s own bookmark and reports whether it existed', async () => {
+    const chain = buildChain([{ id: BOOKMARK_ID }]);
+    mockDb.delete.mockReturnValue(chain);
+
+    await expect(deleteBookmark(USER_ID, BOOKMARK_ID)).resolves.toBe(true);
+    expect(chain.where).toHaveBeenCalledWith(ownBookmark);
+
+    mockDb.delete.mockReturnValue(buildChain([]));
+    await expect(deleteBookmark(USER_ID, BOOKMARK_ID)).resolves.toBe(false);
+  });
+});