Skip to content
Open
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
2 changes: 1 addition & 1 deletion packages/rpc/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"types": "./src/index.ts",
"scripts": {
"check-types": "tsc --noEmit",
"test": "bun test src/routers src/lib/analytics-utils.integration.test.ts src/procedures/*.test.ts src/services/insight-schedule.test.ts src/services/uptime-lifecycle.test.ts src/services/uptime-scheduler.test.ts src/utils/*.test.ts",
"test": "bun test src/routers src/lib/analytics-utils.integration.test.ts src/lib/funnels-cache.test.ts src/procedures/*.test.ts src/services/insight-schedule.test.ts src/services/uptime-lifecycle.test.ts src/services/uptime-scheduler.test.ts src/utils/*.test.ts",
"test:integration": "bun test src/services/uptime-scheduler.integration.test.ts"
},
"exports": {
Expand Down
111 changes: 111 additions & 0 deletions packages/rpc/src/lib/funnels-cache.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import { beforeEach, describe, expect, it, mock } from "bun:test";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Regression test is not selected

The RPC package's configured test command does not select src/lib/funnels-cache.test.ts or the complete src/lib directory, so this regression coverage is skipped by the standard package and CI test task.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

import { createDrizzleCache } from "@databuddy/redis/drizzle-cache";

const kv = new Map<string, string>();
const sets = new Map<string, Set<string>>();

const redis = {
get: mock(async (key: string) => kv.get(key) ?? null),
setex: mock(async (key: string, _ttl: number, value: string) => {
kv.set(key, value);
return "OK" as const;
}),
sadd: mock(async (key: string, ...members: string[]) => {
const set = sets.get(key) ?? new Set<string>();
for (const member of members) {
set.add(member);
}
sets.set(key, set);
return members.length;
}),
smembers: mock(async (key: string) => Array.from(sets.get(key) ?? [])),
srem: mock(async (key: string, member: string) => {
sets.get(key)?.delete(member);
return 1;
}),
unlink: mock(async (...keys: string[]) => {
for (const key of keys) {
kv.delete(key);
sets.delete(key);
}
return keys.length;
}),
sunion: mock(async (...keys: string[]) => {
const out = new Set<string>();
for (const key of keys) {
for (const member of sets.get(key) ?? []) {
out.add(member);
}
}
return Array.from(out);
}),
};

mock.module("@databuddy/redis", () => ({
createDrizzleCache,
invalidateAgentContextSnapshotsForWebsite: mock(async () => undefined),
redis,
}));

const { funnelCache, invalidateFunnelsCache } = await import("./funnels-cache");

beforeEach(() => {
kv.clear();
sets.clear();
});

describe("invalidateFunnelsCache", () => {
it("invalidates the exact key a cached getById lookup is stored under", async () => {
const funnelId = "funnel-1";
let calls = 0;

const readFunnel = () =>
funnelCache.withCache({
key: `byId:${funnelId}`,
queryFn: async () => {
calls += 1;
return { id: funnelId, name: `v${calls}` };
},
tables: ["funnelDefinitions"],
ttl: 300,
});

expect(await readFunnel()).toEqual({ id: funnelId, name: "v1" });
// Cache hit: queryFn must not run again.
expect(await readFunnel()).toEqual({ id: funnelId, name: "v1" });
expect(calls).toBe(1);

await invalidateFunnelsCache("website-1", funnelId);

// Regression guard: invalidateFunnelsCache must target the same key
// format the router actually caches under (`byId:<funnelId>`), not a
// key that was never written (e.g. `byId:<funnelId>:<websiteId>`).
expect(await readFunnel()).toEqual({ id: funnelId, name: "v2" });
expect(calls).toBe(2);
});

it("invalidates the exact key a cached list lookup is stored under", async () => {
const websiteId = "website-2";
let calls = 0;

const readList = () =>
funnelCache.withCache({
key: `list:${websiteId}`,
queryFn: async () => {
calls += 1;
return [{ id: `funnel-${calls}` }];
},
tables: ["funnelDefinitions"],
ttl: 300,
});

await readList();
await readList();
expect(calls).toBe(1);

await invalidateFunnelsCache(websiteId);

await readList();
expect(calls).toBe(2);
});
});
2 changes: 1 addition & 1 deletion packages/rpc/src/lib/funnels-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export async function invalidateFunnelsCache(
): Promise<void> {
const keys = [`list:${websiteId}`];
if (funnelId) {
keys.push(`byId:${funnelId}:${websiteId}`);
keys.push(`byId:${funnelId}`);
}

const operations: Promise<unknown>[] = keys.map((key) =>
Expand Down
40 changes: 28 additions & 12 deletions packages/rpc/src/routers/funnels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,6 @@ export const funnelsRouter = {

return cache.withCache({
key: `list:${input.websiteId}`,
disabled: true, // TODO: Remove this once we have a way to invalidate the cache
ttl: CACHE_TTL,
tables: ["funnelDefinitions"],
queryFn: () =>
Expand Down Expand Up @@ -223,10 +222,33 @@ export const funnelsRouter = {
})
.input(z.object({ id: z.string() }))
.output(funnelOutputSchema)
.handler(({ context, input }) =>
cache.withCache({
.handler(async ({ context, input }) => {
// Resolve the owning website and authorize on every request, not just
// on a cache miss: `queryFn` below is skipped entirely on a cache hit,
// so any permission check placed inside it would be bypassed for
// anyone who requests an id already cached by another caller.
const [funnelRef] = await context.db
.select({ websiteId: funnelDefinitions.websiteId })
.from(funnelDefinitions)
.where(
and(
eq(funnelDefinitions.id, input.id),
isNull(funnelDefinitions.deletedAt)
)
)
.limit(1);

if (!funnelRef) {
throw rpcError.notFound("funnel", input.id);
}

await withWorkspace(context, {
websiteId: funnelRef.websiteId,
permissions: ["read"],
});

return cache.withCache({
key: `byId:${input.id}`,
disabled: true, // TODO: Remove this once we have a way to invalidate the cache
ttl: CACHE_TTL,
tables: ["funnelDefinitions"],
queryFn: async () => {
Expand All @@ -245,15 +267,10 @@ export const funnelsRouter = {
throw rpcError.notFound("funnel", input.id);
}

await withWorkspace(context, {
websiteId: funnel.websiteId,
permissions: ["read"],
});

return funnel;
},
})
),
});
}),

create: trackedProcedure
.route({
Expand Down Expand Up @@ -650,7 +667,6 @@ export const funnelsRouter = {

return cache.withCache({
key: cacheKey,
disabled: true, // TODO: Remove this once we have a way to invalidate the cache
ttl: ANALYTICS_CACHE_TTL,
tables: ["funnelDefinitions"],
tag: `funnel:${input.funnelId}`,
Expand Down