Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions backend/migrations/0052_item_labels_tombstones.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
-- Tombstones for item_labels_cache
--
-- DELETE /api/labels now soft-deletes (sets deleted_at + bumps updated_at)
-- instead of hard-deleting. This makes the `?since=` delta lossless: a removed
-- label still shows up in the delta (as a tombstone), so clients can replay
-- deletions made on other devices without a periodic full reconcile, and can
-- safely persist their delta cursor across sessions.
--
-- NULL deleted_at = live row. A re-add (ON CONFLICT) resets deleted_at to NULL.
-- Only archived/tagged labels flow through DELETE /api/labels; `read` positions
-- are owned by the reading route and keep hard deletes.
ALTER TABLE item_labels_cache ADD COLUMN deleted_at INTEGER;

-- Partial index to keep the hourly tombstone GC sweep cheap.
CREATE INDEX IF NOT EXISTS idx_item_labels_deleted
ON item_labels_cache(deleted_at)
WHERE deleted_at IS NOT NULL;
19 changes: 18 additions & 1 deletion backend/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,7 @@ export default {

let oauthDeleted = 0;
let sessionsDeleted = 0;
let labelTombstonesDeleted = 0;

// Clean up expired OAuth states
try {
Expand Down Expand Up @@ -561,9 +562,25 @@ export default {
console.error('[Cron] D1 WRITE ERROR deleting expired sessions:', error);
}

// Purge old label tombstones (soft-deleted archived/tagged rows). The
// retention must outlast any realistic delta-cursor staleness so a
// client offline for a while still replays the deletion; 90 days mirrors
// the read-position window. deleted_at is unix seconds.
try {
const tombstoneCutoff = Math.floor(now / 1000) - 90 * 24 * 60 * 60;
const tombstoneResult = await env.DB.prepare(
'DELETE FROM item_labels_cache WHERE deleted_at IS NOT NULL AND deleted_at < ?'
)
.bind(tombstoneCutoff)
.run();
labelTombstonesDeleted = tombstoneResult.meta?.changes || 0;
} catch (error) {
console.error('[Cron] D1 WRITE ERROR purging label tombstones:', error);
}

d1CleanupDuration = Date.now() - cleanupStart;
console.log(
`[Cron] D1 cleanup: deleted ${oauthDeleted} OAuth states, ${sessionsDeleted} sessions, ${d1CleanupDuration}ms`
`[Cron] D1 cleanup: deleted ${oauthDeleted} OAuth states, ${sessionsDeleted} sessions, ${labelTombstonesDeleted} label tombstones, ${d1CleanupDuration}ms`
);
}

Expand Down
36 changes: 31 additions & 5 deletions backend/src/routes/labels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ interface LabelRow {
rkey: string | null;
created_at: number;
updated_at: number;
deleted_at: number | null;
}

const DEFAULT_LIMIT = 100;
Expand Down Expand Up @@ -44,6 +45,13 @@ export async function handleGetLabels(request: Request, env: Env): Promise<Respo

const url = new URL(request.url);
const labelFilter = url.searchParams.get('label');
// `?labels=a,b,c` restricts to a set of label types — used by the managed
// sync so its delta isn't bloated with unrelated `read` rows that share this
// table. Combinable with `label` (single), though callers use one or neither.
const labelsFilter = (url.searchParams.get('labels') ?? '')
.split(',')
.map((s) => s.trim())
.filter(Boolean);
const itemTypeFilter = url.searchParams.get('itemType');
const cursor = url.searchParams.get('cursor');
const limitParam = url.searchParams.get('limit');
Expand All @@ -54,7 +62,7 @@ export async function handleGetLabels(request: Request, env: Env): Promise<Respo
const since = sinceParam !== null ? parseInt(sinceParam, 10) : NaN;

try {
let query = `SELECT id, item_key, item_type, label, props, rkey, created_at, updated_at
let query = `SELECT id, item_key, item_type, label, props, rkey, created_at, updated_at, deleted_at
FROM item_labels_cache
WHERE user_did = ?`;
const params: (string | number)[] = [session.did];
Expand All @@ -63,13 +71,22 @@ export async function handleGetLabels(request: Request, env: Env): Promise<Respo
query += ' AND label = ?';
params.push(labelFilter);
}
if (labelsFilter.length > 0) {
query += ` AND label IN (${labelsFilter.map(() => '?').join(', ')})`;
params.push(...labelsFilter);
}
if (itemTypeFilter) {
query += ' AND item_type = ?';
params.push(itemTypeFilter);
}
if (Number.isFinite(since)) {
// Delta: include tombstones (deleted_at set) so the client can replay
// deletions made on other devices. The row stays until GC purges it.
query += ' AND updated_at > ?';
params.push(since);
} else {
// Full snapshot: live rows only — a fresh client has nothing to remove.
query += ' AND deleted_at IS NULL';
}

if (cursor) {
Expand Down Expand Up @@ -103,6 +120,7 @@ export async function handleGetLabels(request: Request, env: Env): Promise<Respo
rkey: row.rkey,
createdAt: row.created_at,
updatedAt: row.updated_at,
deletedAt: row.deleted_at,
}));

const nextCursor = hasMore
Expand Down Expand Up @@ -166,7 +184,8 @@ export async function handleAddLabel(request: Request, env: Env): Promise<Respon
VALUES (?, ?, ?, ?, ?, ?, ?, ?)
ON CONFLICT(user_did, item_key, label) DO UPDATE SET
props = excluded.props,
updated_at = excluded.updated_at`
updated_at = excluded.updated_at,
deleted_at = NULL`
)
.bind(
session.did,
Expand Down Expand Up @@ -227,10 +246,16 @@ export async function handleDeleteLabel(request: Request, env: Env): Promise<Res
}

try {
// Soft-delete (tombstone): bump updated_at and set deleted_at so the row
// surfaces in other devices' `?since=` deltas as a removal. A later re-add
// resurrects it (ON CONFLICT clears deleted_at); the hourly cron GCs old
// tombstones. `read` positions are owned by the reading route and are still
// hard-deleted there — they never reach this handler.
const now = Math.floor(Date.now() / 1000);
await env.DB.prepare(
'DELETE FROM item_labels_cache WHERE user_did = ? AND item_key = ? AND label = ?'
'UPDATE item_labels_cache SET deleted_at = ?, updated_at = ? WHERE user_did = ? AND item_key = ? AND label = ?'
)
.bind(session.did, itemKey, label)
.bind(now, now, session.did, itemKey, label)
.run();

return new Response(JSON.stringify({ success: true }), {
Expand Down Expand Up @@ -300,7 +325,8 @@ export async function handleBulkAddLabels(request: Request, env: Env): Promise<R
VALUES (?, ?, ?, ?, ?, ?, ?, ?)
ON CONFLICT(user_did, item_key, label) DO UPDATE SET
props = excluded.props,
updated_at = excluded.updated_at`
updated_at = excluded.updated_at,
deleted_at = NULL`
).bind(
session.did,
item.itemKey,
Expand Down
89 changes: 89 additions & 0 deletions backend/test/labels.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ type LabelsResponse = {
label: string;
props: Record<string, unknown>;
updatedAt: number;
deletedAt: number | null;
}>;
cursor?: string;
};
Expand All @@ -88,6 +89,23 @@ async function getLabels(path: string): Promise<{ status: number; body: LabelsRe
return { status: response.status, body };
}

// Issue a mutating request (POST add / DELETE) against /api/labels.
async function mutateLabels(method: 'POST' | 'DELETE', body: unknown): Promise<number> {
const ctx = createExecutionContext();
const request = new IncomingRequest('http://localhost/api/labels', {
method,
headers: {
Cookie: `session_id=${TEST_SESSION_ID}`,
Origin: env.FRONTEND_URL,
'Content-Type': 'application/json',
},
body: JSON.stringify(body),
});
const response = await worker.fetch(request, env, ctx);
await waitOnExecutionContext(ctx);
return response.status;
}

describe('GET /api/labels', () => {
beforeEach(async () => {
await env.DB.prepare('DELETE FROM item_labels_cache').run();
Expand Down Expand Up @@ -132,6 +150,27 @@ describe('GET /api/labels', () => {
expect(body.labels.map((l) => l.itemKey)).toEqual(['b']);
});

it('restricts to a set of label types when ?labels= is provided', async () => {
const now = Math.floor(Date.now() / 1000);
await insertLabel('a', { updatedAt: now - 30, label: 'tagged' });
await insertLabel('b', { updatedAt: now - 20, label: 'archived' });
await insertLabel('c', { updatedAt: now - 10, label: 'read' });

const { body } = await getLabels('/api/labels?labels=tagged,archived');
expect(body.labels.map((l) => l.itemKey)).toEqual(['b', 'a']);
});

it('excludes unrelated label types (read) from a labels-filtered delta', async () => {
const now = Math.floor(Date.now() / 1000);
await insertLabel('tag', { updatedAt: now - 10, label: 'tagged' });
await insertLabel('readrow', { updatedAt: now - 5, label: 'read' });

const { body } = await getLabels(
`/api/labels?labels=tagged,archived,readProgress&since=${now - 50}`
);
expect(body.labels.map((l) => l.itemKey)).toEqual(['tag']);
});

describe('delta sync (?since=)', () => {
it('returns only rows changed strictly after the cursor', async () => {
const now = Math.floor(Date.now() / 1000);
Expand Down Expand Up @@ -160,4 +199,54 @@ describe('GET /api/labels', () => {
expect(body.labels.map((l) => l.itemKey)).toEqual(['new-tag']);
});
});

describe('tombstones (soft delete)', () => {
it('DELETE soft-deletes: the row surfaces in a delta with deletedAt set', async () => {
const now = Math.floor(Date.now() / 1000);
await insertLabel('tag-me', { updatedAt: now - 100, label: 'tagged' });

expect(await mutateLabels('DELETE', { itemKey: 'tag-me', label: 'tagged' })).toBe(200);

const { body } = await getLabels(`/api/labels?since=${now - 100}`);
expect(body.labels).toHaveLength(1);
expect(body.labels[0].itemKey).toBe('tag-me');
expect(body.labels[0].deletedAt).toBeGreaterThan(0);
});

it('a full snapshot excludes tombstoned rows', async () => {
const now = Math.floor(Date.now() / 1000);
await insertLabel('keep', { updatedAt: now - 100, label: 'tagged' });
await insertLabel('drop', { updatedAt: now - 100, label: 'tagged' });

expect(await mutateLabels('DELETE', { itemKey: 'drop', label: 'tagged' })).toBe(200);

const { body } = await getLabels('/api/labels');
expect(body.labels.map((l) => l.itemKey)).toEqual(['keep']);
});

it('re-adding a deleted label resurrects it (clears the tombstone)', async () => {
const now = Math.floor(Date.now() / 1000);
await insertLabel('revive', {
updatedAt: now - 100,
label: 'tagged',
props: { tags: ['a'] },
});

expect(await mutateLabels('DELETE', { itemKey: 'revive', label: 'tagged' })).toBe(200);
expect(
await mutateLabels('POST', {
itemKey: 'revive',
itemType: 'article',
label: 'tagged',
props: { tags: ['b'] },
})
).toBe(200);

const { body } = await getLabels('/api/labels');
expect(body.labels).toHaveLength(1);
expect(body.labels[0].itemKey).toBe('revive');
expect(body.labels[0].deletedAt).toBeNull();
expect(body.labels[0].props).toEqual({ tags: ['b'] });
});
});
});
9 changes: 8 additions & 1 deletion frontend/src/lib/services/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,7 @@ class ApiClient {
async getLabels(
options: {
label?: string;
labels?: string[];
itemType?: ItemLabelType;
cursor?: string;
limit?: number;
Expand All @@ -587,11 +588,15 @@ class ApiClient {
rkey?: string;
createdAt: number;
updatedAt: number;
// Tombstone marker: set (unix seconds) when the label was deleted; only
// appears in delta (`since`) responses. Live snapshots never include it.
deletedAt?: number | null;
}>;
cursor?: string;
}> {
const params = new URLSearchParams();
if (options.label) params.set('label', options.label);
if (options.labels?.length) params.set('labels', options.labels.join(','));
if (options.itemType) params.set('itemType', options.itemType);
if (options.cursor) params.set('cursor', options.cursor);
if (options.limit) params.set('limit', String(options.limit));
Expand All @@ -601,7 +606,7 @@ class ApiClient {
}

async getAllLabels(
options: { label?: string; itemType?: ItemLabelType; since?: number } = {}
options: { label?: string; labels?: string[]; itemType?: ItemLabelType; since?: number } = {}
): Promise<
Array<{
itemKey: string;
Expand All @@ -611,6 +616,7 @@ class ApiClient {
rkey?: string;
createdAt: number;
updatedAt: number;
deletedAt?: number | null;
}>
> {
const all: Array<{
Expand All @@ -621,6 +627,7 @@ class ApiClient {
rkey?: string;
createdAt: number;
updatedAt: number;
deletedAt?: number | null;
}> = [];
let cursor: string | undefined;
do {
Expand Down
Loading
Loading