forked from Mike/freno-dev
nook: claim lifecycle sends so concurrent runs cannot double-mail
The lifecycle job marked rows with an unconditional UPDATE before sending, so two overlapping invocations (cron tick + manual hit, or retry) both passed the SELECT, both ran the UPDATE — re-writing the same value — and both sent the same email. Real duplicates went out to licensees. Rows are now claimed with a conditional UPDATE (... AND asked_at IS NULL), and the send happens only when the claim reports rowsAffected 1; a racing invocation claims nothing and sends nothing. Tests: lifecycle suite gains a lost-claim case; 36 nook API tests pass. Issue: FRE-11
This commit is contained in:
@@ -19,11 +19,15 @@ let trialRows: Record<string, unknown>[] = [];
|
|||||||
let licenseRows: Record<string, unknown>[] = [];
|
let licenseRows: Record<string, unknown>[] = [];
|
||||||
const updates: { sql: string; args?: unknown[] }[] = [];
|
const updates: { sql: string; args?: unknown[] }[] = [];
|
||||||
const sent: string[] = [];
|
const sent: string[] = [];
|
||||||
|
// When true, conditional-claim UPDATEs report rowsAffected 0 (another
|
||||||
|
// invocation won the claim first).
|
||||||
|
let racingClaim = false;
|
||||||
|
|
||||||
const fakeConn = {
|
const fakeConn = {
|
||||||
execute: async (q: { sql: string; args?: unknown[] } | string) => {
|
execute: async (q: { sql: string; args?: unknown[] } | string) => {
|
||||||
const { sql, args } =
|
const { sql: rawSql, args } =
|
||||||
typeof q === "string" ? { sql: q, args: undefined } : q;
|
typeof q === "string" ? { sql: q, args: undefined } : q;
|
||||||
|
const sql = rawSql.trim();
|
||||||
if (sql.startsWith("SELECT fingerprint")) {
|
if (sql.startsWith("SELECT fingerprint")) {
|
||||||
return { rows: trialRows, rowsAffected: 0 };
|
return { rows: trialRows, rowsAffected: 0 };
|
||||||
}
|
}
|
||||||
@@ -37,7 +41,8 @@ const fakeConn = {
|
|||||||
}
|
}
|
||||||
if (sql.startsWith("UPDATE")) {
|
if (sql.startsWith("UPDATE")) {
|
||||||
updates.push({ sql, args });
|
updates.push({ sql, args });
|
||||||
return { rows: [], rowsAffected: 1 };
|
const claimed = racingClaim ? 0 : 1;
|
||||||
|
return { rows: [], rowsAffected: claimed };
|
||||||
}
|
}
|
||||||
return { rows: [], rowsAffected: 0 };
|
return { rows: [], rowsAffected: 0 };
|
||||||
}
|
}
|
||||||
@@ -154,6 +159,21 @@ describe("GET /api/the-nook/lifecycle", () => {
|
|||||||
).toBe(true);
|
).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("claims via conditional update — a lost claim never sends", async () => {
|
||||||
|
// Simulate a racing invocation that already claimed the row: the
|
||||||
|
// conditional UPDATE reports rowsAffected 0, and no email goes out.
|
||||||
|
trialRows = [
|
||||||
|
{ fingerprint: UUID, email: "raced@example.com", started_at: daysAgo(12) }
|
||||||
|
];
|
||||||
|
licenseRows = [];
|
||||||
|
racingClaim = true;
|
||||||
|
const res = await GET(request("Bearer sekrit"));
|
||||||
|
const body = await res.json();
|
||||||
|
expect(body.reminders).toBe(0);
|
||||||
|
expect(sent).toHaveLength(0);
|
||||||
|
racingClaim = false;
|
||||||
|
});
|
||||||
|
|
||||||
it("asks for a testimonial at D+3 but not a review until D+14", async () => {
|
it("asks for a testimonial at D+3 but not a review until D+14", async () => {
|
||||||
licenseRows = [
|
licenseRows = [
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -53,10 +53,18 @@ async function runLifecycle(): Promise<{
|
|||||||
started_at: string;
|
started_at: string;
|
||||||
}[]) {
|
}[]) {
|
||||||
if (ageDays(row.started_at) < 11) continue;
|
if (ageDays(row.started_at) < 11) continue;
|
||||||
await conn.execute({
|
// Claim-then-send: the conditional UPDATE wins exactly once even when
|
||||||
sql: "UPDATE trials SET reminder_sent_at = ? WHERE fingerprint = ?",
|
// two invocations race (cron + manual hit) — the loser sees
|
||||||
|
// rowsAffected 0 and never sends. An unconditional mark-then-send let
|
||||||
|
// both invocations pass the SELECT and mail the same user twice.
|
||||||
|
const claim = await conn.execute({
|
||||||
|
sql: `
|
||||||
|
UPDATE trials SET reminder_sent_at = ?
|
||||||
|
WHERE fingerprint = ? AND reminder_sent_at IS NULL
|
||||||
|
`,
|
||||||
args: [new Date().toISOString(), row.fingerprint]
|
args: [new Date().toISOString(), row.fingerprint]
|
||||||
});
|
});
|
||||||
|
if (claim.rowsAffected === 0) continue;
|
||||||
// A trial that ended in a purchase gets no buy reminder. The address
|
// A trial that ended in a purchase gets no buy reminder. The address
|
||||||
// comparison is on the raw stored values; both writes lowercase.
|
// comparison is on the raw stored values; both writes lowercase.
|
||||||
const owned = await conn.execute({
|
const owned = await conn.execute({
|
||||||
@@ -83,18 +91,26 @@ async function runLifecycle(): Promise<{
|
|||||||
if (age < 0) continue;
|
if (age < 0) continue;
|
||||||
|
|
||||||
if (age >= 3 && !row.testimonial_asked_at) {
|
if (age >= 3 && !row.testimonial_asked_at) {
|
||||||
await conn.execute({
|
const claim = await conn.execute({
|
||||||
sql: "UPDATE licenses SET testimonial_asked_at = ? WHERE id = ?",
|
sql: `
|
||||||
|
UPDATE licenses SET testimonial_asked_at = ?
|
||||||
|
WHERE id = ? AND testimonial_asked_at IS NULL
|
||||||
|
`,
|
||||||
args: [new Date().toISOString(), row.id]
|
args: [new Date().toISOString(), row.id]
|
||||||
});
|
});
|
||||||
|
if (claim.rowsAffected === 0) continue;
|
||||||
if (await emailTestimonialAsk(row.email)) counts.testimonials++;
|
if (await emailTestimonialAsk(row.email)) counts.testimonials++;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (age >= 14 && !row.review_asked_at) {
|
if (age >= 14 && !row.review_asked_at) {
|
||||||
await conn.execute({
|
const claim = await conn.execute({
|
||||||
sql: "UPDATE licenses SET review_asked_at = ? WHERE id = ?",
|
sql: `
|
||||||
|
UPDATE licenses SET review_asked_at = ?
|
||||||
|
WHERE id = ? AND review_asked_at IS NULL
|
||||||
|
`,
|
||||||
args: [new Date().toISOString(), row.id]
|
args: [new Date().toISOString(), row.id]
|
||||||
});
|
});
|
||||||
|
if (claim.rowsAffected === 0) continue;
|
||||||
if (await emailReviewAsk(row.email)) counts.reviews++;
|
if (await emailReviewAsk(row.email)) counts.reviews++;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user