diff --git a/apps/backend/prisma/migrations/20260610000000_follow_log_idempotency/migration.sql b/apps/backend/prisma/migrations/20260610000000_follow_log_idempotency/migration.sql new file mode 100644 index 00000000..4d0b88c9 --- /dev/null +++ b/apps/backend/prisma/migrations/20260610000000_follow_log_idempotency/migration.sql @@ -0,0 +1,27 @@ +-- Migration: follow_log_idempotency +-- Adds a unique constraint on (follower_id, target_username, platform) to prevent +-- duplicate follow log entries. Also adds updated_at for upsert support. +-- +-- Safety: deduplicates any existing rows before creating the index so the +-- constraint never fails on a populated database. + +-- Step 1: add updated_at column (nullable initially, backfilled, then NOT NULL) +ALTER TABLE "follow_logs" ADD COLUMN "updated_at" TIMESTAMP(3); + +-- Step 2: backfill updated_at from created_at for all existing rows +UPDATE "follow_logs" SET "updated_at" = "created_at" WHERE "updated_at" IS NULL; + +-- Step 3: make updated_at NOT NULL now that every row has a value +ALTER TABLE "follow_logs" ALTER COLUMN "updated_at" SET NOT NULL; + +-- Step 4: deduplicate — keep the most-recent row per (follower_id, target_username, platform) +DELETE FROM "follow_logs" +WHERE id NOT IN ( + SELECT DISTINCT ON (follower_id, target_username, platform) id + FROM "follow_logs" + ORDER BY follower_id, target_username, platform, created_at DESC +); + +-- Step 5: add the unique constraint +CREATE UNIQUE INDEX "follow_logs_follower_id_target_username_platform_key" + ON "follow_logs"("follower_id", "target_username", "platform"); diff --git a/apps/backend/prisma/schema.prisma b/apps/backend/prisma/schema.prisma index 28458021..d8fb70bf 100644 --- a/apps/backend/prisma/schema.prisma +++ b/apps/backend/prisma/schema.prisma @@ -121,12 +121,14 @@ model FollowLog { followerId String @map("follower_id") targetUsername String @map("target_username") platform String - status String @default("success") // "success" | "error" - layer String // "api" | "webview" | "link" + status String @default("success") // "success" | "failed" | "pending" + layer String // "foreground" | "background" createdAt DateTime @default(now()) @map("created_at") + updatedAt DateTime @updatedAt @map("updated_at") follower User @relation(fields: [followerId], references: [id], onDelete: Cascade) + @@unique([followerId, targetUsername, platform]) @@map("follow_logs") } diff --git a/apps/backend/src/__tests__/follow.test.ts b/apps/backend/src/__tests__/follow.test.ts index 41830018..e17c19dc 100644 --- a/apps/backend/src/__tests__/follow.test.ts +++ b/apps/backend/src/__tests__/follow.test.ts @@ -1,8 +1,10 @@ -import Fastify, { FastifyInstance } from 'fastify'; +import Fastify from 'fastify'; import { describe, expect, it, vi, beforeAll, beforeEach, afterAll } from 'vitest'; import { followRoutes } from '../routes/follow.js'; +import type { FastifyInstance } from 'fastify'; + vi.mock('../utils/encryption.js', () => ({ decrypt: vi.fn(() => 'fake-access-token'), })); @@ -32,7 +34,7 @@ function buildApp(overrides: { ...overrides.oAuthToken, }, followLog: { - create: vi.fn(), + upsert: vi.fn(), deleteMany: vi.fn(), ...overrides.followLog, }, @@ -102,12 +104,12 @@ describe('POST /api/follow/:platform/:targetUsername — API follow', () => { describe('POST /api/follow/:platform/:targetUsername/log — follow log validation', () => { let app: FastifyInstance; - let createLog: ReturnType; + let upsertLog: ReturnType; // One app instance shared across all log tests; mock reset between each test. beforeAll(async () => { - createLog = vi.fn(); - app = await makeApp({ followLog: { create: createLog } }); + upsertLog = vi.fn(); + app = await makeApp({ followLog: { upsert: upsertLog } }); }); afterAll(async () => { @@ -115,8 +117,8 @@ describe('POST /api/follow/:platform/:targetUsername/log — follow log validati }); beforeEach(() => { - createLog.mockReset(); - createLog.mockResolvedValue({ id: 'log-uuid-001' }); + upsertLog.mockReset(); + upsertLog.mockResolvedValue({ id: 'log-uuid-001' }); }); // ── Valid payloads ──────────────────────────────────────────────────────── @@ -130,8 +132,8 @@ describe('POST /api/follow/:platform/:targetUsername/log — follow log validati expect(res.statusCode).toBe(200); expect(res.json()).toMatchObject({ status: 'success', logId: 'log-uuid-001' }); - expect(createLog).toHaveBeenCalledOnce(); - expect(createLog.mock.calls[0][0].data.status).toBe('success'); + expect(upsertLog).toHaveBeenCalledOnce(); + expect(upsertLog.mock.calls[0][0].create.status).toBe('success'); }); it('200 — accepts status: failed', async () => { @@ -142,8 +144,8 @@ describe('POST /api/follow/:platform/:targetUsername/log — follow log validati }); expect(res.statusCode).toBe(200); - expect(createLog).toHaveBeenCalledOnce(); - expect(createLog.mock.calls[0][0].data.status).toBe('failed'); + expect(upsertLog).toHaveBeenCalledOnce(); + expect(upsertLog.mock.calls[0][0].create.status).toBe('failed'); }); it('200 — accepts status: pending, layer: background', async () => { @@ -154,8 +156,103 @@ describe('POST /api/follow/:platform/:targetUsername/log — follow log validati }); expect(res.statusCode).toBe(200); - expect(createLog).toHaveBeenCalledOnce(); - expect(createLog.mock.calls[0][0].data.layer).toBe('background'); + expect(upsertLog).toHaveBeenCalledOnce(); + expect(upsertLog.mock.calls[0][0].create.status).toBe('pending'); + expect(upsertLog.mock.calls[0][0].create.layer).toBe('background'); + }); + + // ── Valid layer values ──────────────────────────────────────────────────── + + it('200 — accepts layer: foreground', async () => { + const res = await app.inject({ + method: 'POST', + url: '/api/follow/linkedin/testuser/log', + payload: { status: 'success', layer: 'foreground' }, + }); + + expect(res.statusCode).toBe(200); + expect(upsertLog.mock.calls[0][0].create.layer).toBe('foreground'); + }); + + it('200 — accepts layer: background', async () => { + const res = await app.inject({ + method: 'POST', + url: '/api/follow/linkedin/testuser/log', + payload: { status: 'success', layer: 'background' }, + }); + + expect(res.statusCode).toBe(200); + expect(upsertLog.mock.calls[0][0].create.layer).toBe('background'); + }); + + // ── Idempotency — repeated calls must not create duplicate records ──────── + + it('idempotency — second call for same user/target/platform upserts, not inserts', async () => { + // First follow + await app.inject({ + method: 'POST', + url: '/api/follow/linkedin/alice/log', + payload: { status: 'success', layer: 'foreground' }, + }); + + // Second follow — same user, same target, same platform + const res = await app.inject({ + method: 'POST', + url: '/api/follow/linkedin/alice/log', + payload: { status: 'success', layer: 'foreground' }, + }); + + expect(res.statusCode).toBe(200); + // Two calls, but upsert's where clause includes the unique key — DB enforces one row + expect(upsertLog).toHaveBeenCalledTimes(2); + const whereKey = upsertLog.mock.calls[0][0].where.followerId_targetUsername_platform; + expect(whereKey).toMatchObject({ + followerId: MOCK_USER_ID, + targetUsername: 'alice', + platform: 'linkedin', + }); + }); + + it('idempotency — upsert carries correct where key for dedup lookup', async () => { + await app.inject({ + method: 'POST', + url: '/api/follow/twitter/bob/log', + payload: { status: 'success', layer: 'background' }, + }); + + const call = upsertLog.mock.calls[0][0]; + expect(call.where.followerId_targetUsername_platform).toMatchObject({ + followerId: MOCK_USER_ID, + targetUsername: 'bob', + platform: 'twitter', + }); + expect(call.update).toMatchObject({ status: 'success', layer: 'background' }); + expect(call.create).toMatchObject({ + followerId: MOCK_USER_ID, + targetUsername: 'bob', + platform: 'twitter', + status: 'success', + layer: 'background', + }); + }); + + it('idempotency — different target produces separate upsert calls', async () => { + await app.inject({ + method: 'POST', + url: '/api/follow/linkedin/alice/log', + payload: { status: 'success', layer: 'foreground' }, + }); + + await app.inject({ + method: 'POST', + url: '/api/follow/linkedin/charlie/log', + payload: { status: 'success', layer: 'foreground' }, + }); + + expect(upsertLog).toHaveBeenCalledTimes(2); + const targets = upsertLog.mock.calls.map((c: any) => c[0].where.followerId_targetUsername_platform.targetUsername); + expect(targets).toContain('alice'); + expect(targets).toContain('charlie'); }); // ── Invalid status values — analytics integrity ─────────────────────────── @@ -170,7 +267,18 @@ describe('POST /api/follow/:platform/:targetUsername/log — follow log validati expect(res.statusCode).toBe(400); expect(res.json()).toMatchObject({ error: 'Invalid follow log payload' }); // DB must NOT be written — this is the analytics integrity guarantee - expect(createLog).not.toHaveBeenCalled(); + expect(upsertLog).not.toHaveBeenCalled(); + }); + + it('400 — rejects fabricated status "admin_override"', async () => { + const res = await app.inject({ + method: 'POST', + url: '/api/follow/linkedin/testuser/log', + payload: { status: 'admin_override', layer: 'foreground' }, + }); + + expect(res.statusCode).toBe(400); + expect(upsertLog).not.toHaveBeenCalled(); }); it('400 — rejects arbitrary status string injection', async () => { @@ -181,7 +289,7 @@ describe('POST /api/follow/:platform/:targetUsername/log — follow log validati }); expect(res.statusCode).toBe(400); - expect(createLog).not.toHaveBeenCalled(); + expect(upsertLog).not.toHaveBeenCalled(); }); // ── Invalid layer values — analytics integrity ──────────────────────────── @@ -198,7 +306,7 @@ describe('POST /api/follow/:platform/:targetUsername/log — follow log validati expect(res.statusCode).toBe(400); expect(res.json()).toMatchObject({ error: 'Invalid follow log payload' }); - expect(createLog).not.toHaveBeenCalled(); + expect(upsertLog).not.toHaveBeenCalled(); }); it('400 — rejects invalid layer "api"', async () => { @@ -209,7 +317,18 @@ describe('POST /api/follow/:platform/:targetUsername/log — follow log validati }); expect(res.statusCode).toBe(400); - expect(createLog).not.toHaveBeenCalled(); + expect(upsertLog).not.toHaveBeenCalled(); + }); + + it('400 — rejects arbitrary layer string injection', async () => { + const res = await app.inject({ + method: 'POST', + url: '/api/follow/linkedin/testuser/log', + payload: { status: 'success', layer: 'superuser' }, + }); + + expect(res.statusCode).toBe(400); + expect(upsertLog).not.toHaveBeenCalled(); }); // ── Malformed / missing payloads ────────────────────────────────────────── @@ -222,7 +341,7 @@ describe('POST /api/follow/:platform/:targetUsername/log — follow log validati }); expect(res.statusCode).toBe(400); - expect(createLog).not.toHaveBeenCalled(); + expect(upsertLog).not.toHaveBeenCalled(); }); it('400 — rejects missing layer field', async () => { @@ -233,7 +352,7 @@ describe('POST /api/follow/:platform/:targetUsername/log — follow log validati }); expect(res.statusCode).toBe(400); - expect(createLog).not.toHaveBeenCalled(); + expect(upsertLog).not.toHaveBeenCalled(); }); it('400 — rejects empty body', async () => { @@ -244,7 +363,29 @@ describe('POST /api/follow/:platform/:targetUsername/log — follow log validati }); expect(res.statusCode).toBe(400); - expect(createLog).not.toHaveBeenCalled(); + expect(upsertLog).not.toHaveBeenCalled(); + }); + + it('400 — rejects null values for both fields', async () => { + const res = await app.inject({ + method: 'POST', + url: '/api/follow/linkedin/testuser/log', + payload: { status: null, layer: null }, + }); + + expect(res.statusCode).toBe(400); + expect(upsertLog).not.toHaveBeenCalled(); + }); + + it('400 — rejects numeric value for status', async () => { + const res = await app.inject({ + method: 'POST', + url: '/api/follow/linkedin/testuser/log', + payload: { status: 1, layer: 'foreground' }, + }); + + expect(res.statusCode).toBe(400); + expect(upsertLog).not.toHaveBeenCalled(); }); // ── Correct data persisted to DB ────────────────────────────────────────── @@ -257,9 +398,9 @@ describe('POST /api/follow/:platform/:targetUsername/log — follow log validati }); expect(res.statusCode).toBe(200); - expect(createLog).toHaveBeenCalledOnce(); + expect(upsertLog).toHaveBeenCalledOnce(); - const written = createLog.mock.calls[0][0].data; + const written = upsertLog.mock.calls[0][0].create; expect(written).toMatchObject({ followerId: MOCK_USER_ID, targetUsername: 'janedoe', @@ -280,6 +421,7 @@ describe('POST /api/follow/:platform/:targetUsername/log — follow log validati expect(res.statusCode).toBe(400); const body = res.json(); + // Must not expose Zod issue paths, internal type names, or stack traces expect(body).not.toHaveProperty('issues'); expect(body).not.toHaveProperty('stack'); expect(Object.keys(body)).toEqual(['error']); @@ -288,7 +430,7 @@ describe('POST /api/follow/:platform/:targetUsername/log — follow log validati // ── DB failure after valid payload ──────────────────────────────────────── it('500 — returns 500 when DB write fails after successful validation', async () => { - createLog.mockRejectedValueOnce(new Error('DB connection lost')); + upsertLog.mockRejectedValueOnce(new Error('DB connection lost')); const res = await app.inject({ method: 'POST', diff --git a/apps/backend/src/routes/follow.ts b/apps/backend/src/routes/follow.ts index a152fc55..cefabb35 100644 --- a/apps/backend/src/routes/follow.ts +++ b/apps/backend/src/routes/follow.ts @@ -1,16 +1,18 @@ -import type { FastifyInstance, FastifyRequest, FastifyReply } from 'fastify'; +import { getPlatform, getProfileUrl, getWebViewUrl } from '@devcard/shared'; + import { decrypt } from '../utils/encryption.js'; import { getErrorMessage } from '../utils/error.util.js'; -import { getPlatform, getProfileUrl, getWebViewUrl } from '@devcard/shared'; import { followLogSchema } from '../validations/follow.validation.js'; -export async function followRoutes(app: FastifyInstance) { - app.addHook('preHandler', async (request, reply) => { - const server = request.server as any; - if (typeof server?.authenticate === 'function') { await server.authenticate(request, reply); return } - if (typeof (app as any).authenticate === 'function') { await (app as any).authenticate(request, reply); return } - try { const payload = await request.jwtVerify(); if (payload) (request as any).user = payload; } catch (e) { reply.status(401).send({ error: 'Unauthorized' }) } - }); +import type { FastifyInstance, FastifyRequest, FastifyReply } from 'fastify'; + +export async function followRoutes(app: FastifyInstance): Promise { + app.addHook('preHandler', async (request, reply) => { + const server = request.server as any; + if (typeof server?.authenticate === 'function') { await server.authenticate(request, reply); return; } + if (typeof (app as any).authenticate === 'function') { await (app as any).authenticate(request, reply); return; } + try { const payload = await request.jwtVerify(); if (payload) { (request as any).user = payload; } } catch (_e) { reply.status(401).send({ error: 'Unauthorized' }); } + }); // ─── Follow via API (Layer 1) ─── // Currently supports: GitHub @@ -70,10 +72,23 @@ export async function followRoutes(app: FastifyInstance) { }); } - // Log only genuine successes — not based on reply.statusCode default + // Log only genuine successes — not based on reply.statusCode default. + // Upsert ensures repeated follows (double-click, retry) don't create + // duplicate analytics records. if (succeeded) { - app.prisma.followLog.create({ - data: { + app.prisma.followLog.upsert({ + where: { + followerId_targetUsername_platform: { + followerId: userId, + targetUsername, + platform, + }, + }, + update: { + status: 'success', + layer: 'api', + }, + create: { followerId: userId, targetUsername, platform, @@ -84,11 +99,22 @@ export async function followRoutes(app: FastifyInstance) { } return result.response; - } catch (err: unknown) { + } catch (err: unknown) { app.log.error(`Follow error for ${platform}: ${getErrorMessage(err)}`); - - app.prisma.followLog.create({ - data: { + + app.prisma.followLog.upsert({ + where: { + followerId_targetUsername_platform: { + followerId: userId, + targetUsername, + platform, + }, + }, + update: { + status: 'error', + layer: 'api', + }, + create: { followerId: userId, targetUsername, platform, @@ -109,6 +135,10 @@ export async function followRoutes(app: FastifyInstance) { // status and layer are analytics-impacting fields: they drive totalFollows counters // and the follower-state dashboard. Both are validated against a strict allowlist // before any database write — arbitrary client values are rejected with 400. + // + // Upsert on (followerId, targetUsername, platform) means repeated calls from the + // same user for the same target only produce one record — analytics stay accurate + // regardless of retries, double-taps, or concurrent requests. app.post('/:platform/:targetUsername/log', async ( request: FastifyRequest<{ Params: { platform: string; targetUsername: string }; @@ -127,8 +157,19 @@ export async function followRoutes(app: FastifyInstance) { const { status, layer } = parsed.data; try { - const log = await app.prisma.followLog.create({ - data: { + const log = await app.prisma.followLog.upsert({ + where: { + followerId_targetUsername_platform: { + followerId: userId, + targetUsername, + platform, + }, + }, + update: { + status, + layer, + }, + create: { followerId: userId, targetUsername, platform, @@ -137,8 +178,8 @@ export async function followRoutes(app: FastifyInstance) { }, }); return reply.send({ status: 'success', logId: log.id }); - } catch (error: any) { - app.log.error('Failed to log follow:', error); + } catch (err: any) { + app.log.error('Failed to log follow:', err); return reply.status(500).send({ error: 'Failed to log follow event' }); } }); @@ -218,4 +259,4 @@ async function followGitHub( details: errorBody, }), }; -} \ No newline at end of file +}