fix(news): preserve recoverable reads and verify publication against real services
This commit is contained in:
1 parent
8abfe352ef
commit
f7b9b55700
33 files changed
+1377
-220
No files matched your search
@@ -0,0 +1,198 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const state = vi.hoisted(() => ({
|
||||
fail: false,
|
||||
articleRows: [] as unknown[][],
|
||||
commentRows: [] as unknown[][],
|
||||
reactionRows: [] as unknown[][],
|
||||
total: 225,
|
||||
queries: [] as { sql: string; params: unknown[] }[],
|
||||
values: new Map<string, string>(),
|
||||
}));
|
||||
vi.mock("@/lib/services/news-cache", () => ({
|
||||
cacheNews: async (
|
||||
key: string,
|
||||
_ttl: number,
|
||||
query: () => Promise<unknown>,
|
||||
) => {
|
||||
const existing = state.values.get(key);
|
||||
if (existing !== undefined) return JSON.parse(existing);
|
||||
const result = await query();
|
||||
const serialized = JSON.stringify(result);
|
||||
state.values.set(key, serialized);
|
||||
return JSON.parse(serialized);
|
||||
},
|
||||
}));
|
||||
vi.mock("@/lib/db", async () => {
|
||||
const schema = await import("@/db/schema");
|
||||
const { drizzle } = await import("drizzle-orm/mysql-proxy");
|
||||
return {
|
||||
...schema,
|
||||
db: drizzle(async (sql, params) => {
|
||||
state.queries.push({ sql, params });
|
||||
if (state.fail) throw new Error("database unavailable");
|
||||
if (sql.includes("count(*)") && !sql.includes("group by"))
|
||||
return { rows: [[state.total]] };
|
||||
if (sql.includes("website_article_comments"))
|
||||
return { rows: state.commentRows };
|
||||
if (sql.includes("website_article_reactions"))
|
||||
return { rows: state.reactionRows };
|
||||
return { rows: state.articleRows };
|
||||
}),
|
||||
};
|
||||
});
|
||||
|
||||
import {
|
||||
getArticleComments,
|
||||
getArticleReactions,
|
||||
getPublishedArticle,
|
||||
} from "./news-detail";
|
||||
|
||||
beforeEach(() => {
|
||||
state.fail = false;
|
||||
state.values.clear();
|
||||
state.queries = [];
|
||||
state.articleRows = [];
|
||||
state.commentRows = [];
|
||||
state.reactionRows = [];
|
||||
state.total = 225;
|
||||
});
|
||||
|
||||
describe("public article loading", () => {
|
||||
it("keeps database failures retryable instead of caching a missing article", async () => {
|
||||
state.fail = true;
|
||||
await expect(getPublishedArticle("community-update")).rejects.toThrow();
|
||||
state.fail = false;
|
||||
expect(await getPublishedArticle("community-update")).toBeNull();
|
||||
expect(state.queries).toHaveLength(2);
|
||||
});
|
||||
|
||||
it("round-trips large IDs and publication dates through a JSON cache", async () => {
|
||||
state.articleRows = [
|
||||
[
|
||||
"9007199254740993",
|
||||
"community-update",
|
||||
"Title",
|
||||
"Summary",
|
||||
"<p>Body</p>",
|
||||
"",
|
||||
"2030-01-01 10:00:00",
|
||||
"2030-02-01 10:00:00",
|
||||
"2030-02-01 10:00:00",
|
||||
"2030-02-01 10:01:00",
|
||||
],
|
||||
];
|
||||
const article = await getPublishedArticle("community-update");
|
||||
expect(article?.id).toBe(9007199254740993n);
|
||||
expect(article?.publishAt).toEqual(new Date("2030-02-01T10:00:00Z"));
|
||||
expect(article?.createdAt).toEqual(new Date("2030-01-01T10:00:00Z"));
|
||||
expect(await getPublishedArticle("community-update")).toEqual(article);
|
||||
expect(state.queries).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("only selects articles that are published and already due", async () => {
|
||||
expect(await getPublishedArticle("quote'")).toBeNull();
|
||||
expect(state.queries[0].sql).toContain("`status` = ?");
|
||||
expect(state.queries[0].sql).toContain("<= NOW()");
|
||||
expect(state.queries[0].params).toEqual(["quote'", "published", 1]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("public article comments", () => {
|
||||
it("makes comments beyond the former 200 item ceiling reachable", async () => {
|
||||
state.commentRows = [
|
||||
["225", 7, "Comment beyond 200", "2030-01-01 12:00:00", "Member"],
|
||||
];
|
||||
const result = await getArticleComments(12n, 9);
|
||||
expect(result).toMatchObject({
|
||||
total: 225,
|
||||
page: 9,
|
||||
perPage: 25,
|
||||
lastPage: 9,
|
||||
rows: [{ id: 225n, comment: "Comment beyond 200", username: "Member" }],
|
||||
});
|
||||
expect(state.queries[1].params.slice(-2)).toEqual([25, 200]);
|
||||
expect(state.queries[1].sql).toMatch(
|
||||
/order by .*`created_at` asc, .*`id` asc/,
|
||||
);
|
||||
expect(state.queries.every((query) => query.params[0] === 12n)).toBe(true);
|
||||
});
|
||||
|
||||
it.each([
|
||||
[-5, 1, 0],
|
||||
[NaN, 1, 0],
|
||||
[Infinity, 1, 0],
|
||||
[1e99, 9, 200],
|
||||
["last", 9, 200],
|
||||
])("bounds requested page %s", async (requested, page, offset) => {
|
||||
expect(
|
||||
await getArticleComments(12n, requested as number | "last"),
|
||||
).toMatchObject({ page });
|
||||
expect(state.queries[1].params).toEqual(
|
||||
offset === 0 ? [12n, 25] : [12n, 25, offset],
|
||||
);
|
||||
});
|
||||
|
||||
it("preserves a successful empty page", async () => {
|
||||
state.total = 0;
|
||||
expect(await getArticleComments(12n, "last")).toMatchObject({
|
||||
rows: [],
|
||||
total: 0,
|
||||
page: 1,
|
||||
lastPage: 1,
|
||||
});
|
||||
});
|
||||
|
||||
it("propagates a failed comment read instead of claiming there are no comments", async () => {
|
||||
state.fail = true;
|
||||
await expect(getArticleComments(12n)).rejects.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe("public article reactions", () => {
|
||||
it("aggregates counts and the current user's reaction without loading every vote", async () => {
|
||||
state.reactionRows = [
|
||||
["love", 1200, 1],
|
||||
["like", 30, 0],
|
||||
];
|
||||
const result = await getArticleReactions(12n, 7);
|
||||
expect(result.counts.get("love")).toBe(1200);
|
||||
expect(result.myReaction).toBe("love");
|
||||
expect(state.queries[0].sql).toContain("group by");
|
||||
expect(state.queries[0].params).toContain(7);
|
||||
});
|
||||
|
||||
it("does not hide a failed reaction read behind zero counts", async () => {
|
||||
state.fail = true;
|
||||
await expect(getArticleReactions(12n, null)).rejects.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
it("ignores legacy cached absence and loads the current public article", async () => {
|
||||
state.values.set("article:slug:community-update", "null");
|
||||
state.articleRows = [
|
||||
[
|
||||
"12",
|
||||
"community-update",
|
||||
"Current article",
|
||||
"Summary",
|
||||
"<p>Current body</p>",
|
||||
"",
|
||||
null,
|
||||
null,
|
||||
null,
|
||||
null,
|
||||
],
|
||||
];
|
||||
expect(await getPublishedArticle("community-update")).toMatchObject({
|
||||
id: 12n,
|
||||
title: "Current article",
|
||||
fullStory: "<p>Current body</p>",
|
||||
});
|
||||
state.fail = true;
|
||||
expect(await getPublishedArticle("community-update")).toMatchObject({
|
||||
id: 12n,
|
||||
title: "Current article",
|
||||
});
|
||||
expect(state.queries).toHaveLength(1);
|
||||
});
|
||||
@@ -0,0 +1,140 @@
|
||||
import "server-only";
|
||||
import { and, asc, count, eq, or, sql } from "drizzle-orm";
|
||||
import {
|
||||
db,
|
||||
User,
|
||||
WebsiteArticleComments,
|
||||
WebsiteArticleReactions,
|
||||
WebsiteArticles,
|
||||
} from "@/lib/db";
|
||||
import { cacheNews } from "@/lib/services/news-cache";
|
||||
|
||||
/** Null means a successful lookup found no public article. Failed reads must reject. */
|
||||
export async function getPublishedArticle(slug: string) {
|
||||
const cached = await cacheNews(
|
||||
`article:v2:slug:${slug}`,
|
||||
60_000,
|
||||
async () => {
|
||||
const [article] = await db
|
||||
.select({
|
||||
id: WebsiteArticles.id,
|
||||
slug: WebsiteArticles.slug,
|
||||
title: WebsiteArticles.title,
|
||||
shortStory: WebsiteArticles.shortStory,
|
||||
fullStory: WebsiteArticles.fullStory,
|
||||
image: WebsiteArticles.image,
|
||||
createdAt: WebsiteArticles.createdAt,
|
||||
updatedAt: WebsiteArticles.updatedAt,
|
||||
publishAt: WebsiteArticles.publishAt,
|
||||
publishedAt: WebsiteArticles.publishedAt,
|
||||
})
|
||||
.from(WebsiteArticles)
|
||||
.where(
|
||||
and(
|
||||
eq(WebsiteArticles.slug, slug),
|
||||
eq(WebsiteArticles.status, "published"),
|
||||
or(
|
||||
sql`${WebsiteArticles.publishAt} IS NULL`,
|
||||
sql`${WebsiteArticles.publishAt} <= NOW()`,
|
||||
),
|
||||
),
|
||||
)
|
||||
.limit(1);
|
||||
if (!article) return null;
|
||||
// Redis uses JSON: preserve full bigint precision and restore dates on reads.
|
||||
return {
|
||||
...article,
|
||||
id: String(article.id),
|
||||
createdAt: article.createdAt?.toISOString() ?? null,
|
||||
updatedAt: article.updatedAt?.toISOString() ?? null,
|
||||
publishAt: article.publishAt?.toISOString() ?? null,
|
||||
publishedAt: article.publishedAt?.toISOString() ?? null,
|
||||
};
|
||||
},
|
||||
);
|
||||
if (!cached) return null;
|
||||
return {
|
||||
...cached,
|
||||
id: BigInt(cached.id),
|
||||
createdAt: cached.createdAt ? new Date(cached.createdAt) : null,
|
||||
updatedAt: cached.updatedAt ? new Date(cached.updatedAt) : null,
|
||||
publishAt: cached.publishAt ? new Date(cached.publishAt) : null,
|
||||
publishedAt: cached.publishedAt ? new Date(cached.publishedAt) : null,
|
||||
};
|
||||
}
|
||||
|
||||
export type PublishedArticle = NonNullable<
|
||||
Awaited<ReturnType<typeof getPublishedArticle>>
|
||||
>;
|
||||
|
||||
export async function getArticleComments(
|
||||
articleId: bigint,
|
||||
requestedPage: number | "last" = 1,
|
||||
) {
|
||||
const where = eq(WebsiteArticleComments.articleId, articleId);
|
||||
const [totals] = await db
|
||||
.select({ value: count() })
|
||||
.from(WebsiteArticleComments)
|
||||
.where(where);
|
||||
const total = Number(totals?.value ?? 0);
|
||||
const perPage = 25;
|
||||
const lastPage = Math.max(1, Math.ceil(total / perPage));
|
||||
const page =
|
||||
requestedPage === "last"
|
||||
? lastPage
|
||||
: Math.min(
|
||||
lastPage,
|
||||
Number.isFinite(requestedPage)
|
||||
? Math.max(1, Math.trunc(requestedPage))
|
||||
: 1,
|
||||
);
|
||||
const rows = await db
|
||||
.select({
|
||||
id: WebsiteArticleComments.id,
|
||||
userId: WebsiteArticleComments.userId,
|
||||
comment: WebsiteArticleComments.comment,
|
||||
createdAt: WebsiteArticleComments.createdAt,
|
||||
username: User.username,
|
||||
})
|
||||
.from(WebsiteArticleComments)
|
||||
.leftJoin(User, eq(User.id, WebsiteArticleComments.userId))
|
||||
.where(where)
|
||||
.orderBy(
|
||||
asc(WebsiteArticleComments.createdAt),
|
||||
asc(WebsiteArticleComments.id),
|
||||
)
|
||||
.limit(perPage)
|
||||
.offset((page - 1) * perPage);
|
||||
return { rows, total, page, perPage, lastPage };
|
||||
}
|
||||
|
||||
export type ArticleComments = Awaited<ReturnType<typeof getArticleComments>>;
|
||||
|
||||
export async function getArticleReactions(
|
||||
articleId: bigint,
|
||||
userId: number | null,
|
||||
) {
|
||||
const rows = await db
|
||||
.select({
|
||||
reaction: WebsiteArticleReactions.reaction,
|
||||
count: count(),
|
||||
mine:
|
||||
userId === null
|
||||
? sql<number>`0`
|
||||
: sql<number>`MAX(CASE WHEN ${WebsiteArticleReactions.userId} = ${userId} THEN 1 ELSE 0 END)`,
|
||||
})
|
||||
.from(WebsiteArticleReactions)
|
||||
.where(
|
||||
and(
|
||||
eq(WebsiteArticleReactions.articleId, articleId),
|
||||
eq(WebsiteArticleReactions.active, true),
|
||||
),
|
||||
)
|
||||
.groupBy(WebsiteArticleReactions.reaction);
|
||||
return {
|
||||
counts: new Map(rows.map((row) => [row.reaction, Number(row.count)])),
|
||||
myReaction: rows.find((row) => Number(row.mine) > 0)?.reaction ?? null,
|
||||
};
|
||||
}
|
||||
|
||||
export type ArticleReactions = Awaited<ReturnType<typeof getArticleReactions>>;
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { SQL } from "drizzle-orm";
|
||||
import { MySqlDialect } from "drizzle-orm/mysql-core";
|
||||
import { format } from "mysql2";
|
||||
import { beforeEach, expect, it, vi } from "vitest";
|
||||
|
||||
const state = vi.hoisted(() => ({
|
||||
@@ -9,6 +10,7 @@ const state = vi.hoisted(() => ({
|
||||
status: string;
|
||||
publishAt: Date;
|
||||
}>,
|
||||
queries: [] as Array<{ sql: string; params: unknown[] }>,
|
||||
operations: [] as unknown[][],
|
||||
effects: [] as unknown[][],
|
||||
failOutbox: false,
|
||||
@@ -19,15 +21,20 @@ vi.mock("@/lib/logger", () => ({ logger: { error: vi.fn() } }));
|
||||
vi.mock("./news-cache", () => ({ invalidateNewsCache: state.refresh }));
|
||||
vi.mock("@/lib/db", () => {
|
||||
const dialect = new MySqlDialect();
|
||||
const timestamp = (value: unknown) =>
|
||||
value instanceof Date
|
||||
? value
|
||||
: new Date(`${String(value).replace(" ", "T")}Z`);
|
||||
const tx = {
|
||||
execute: async (query: SQL) => {
|
||||
const { sql, params } = dialect.sqlToQuery(query);
|
||||
state.queries.push({ sql, params });
|
||||
if (sql.startsWith("SELECT"))
|
||||
return [
|
||||
state.articles
|
||||
.filter(
|
||||
(a) =>
|
||||
a.status === "scheduled" && a.publishAt <= (params[0] as Date),
|
||||
a.status === "scheduled" && a.publishAt <= timestamp(params[0]),
|
||||
)
|
||||
.slice(0, 100),
|
||||
];
|
||||
@@ -37,7 +44,7 @@ vi.mock("@/lib/db", () => {
|
||||
state.loseRace ||
|
||||
!article ||
|
||||
article.status !== "scheduled" ||
|
||||
article.publishAt > (params[3] as Date)
|
||||
article.publishAt > timestamp(params[3])
|
||||
)
|
||||
return [{ affectedRows: 0 }];
|
||||
article.status = "published";
|
||||
@@ -83,6 +90,7 @@ beforeEach(() => {
|
||||
publishAt: new Date("2030-01-01T00:00:00Z"),
|
||||
},
|
||||
];
|
||||
state.queries = [];
|
||||
state.operations = [];
|
||||
state.effects = [];
|
||||
state.failOutbox = false;
|
||||
@@ -128,3 +136,25 @@ it("reports committed publication even when immediate cache refresh fails", asyn
|
||||
expect(state.effects).toHaveLength(1);
|
||||
expect(state.refresh).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("uses the same UTC cutoff and publication timestamps regardless of the MySQL client timezone", async () => {
|
||||
const instant = new Date("2030-06-01T08:09:10.123Z");
|
||||
expect(await publishDueArticles(instant)).toBe(1);
|
||||
const statements = state.queries.filter(
|
||||
({ sql }) =>
|
||||
sql.startsWith("SELECT") || sql.startsWith("UPDATE website_articles"),
|
||||
);
|
||||
expect(statements).toHaveLength(2);
|
||||
for (const statement of statements) {
|
||||
expect(format(statement.sql, statement.params, false, "+02:00")).toBe(
|
||||
format(statement.sql, statement.params, false, "-07:00"),
|
||||
);
|
||||
}
|
||||
expect(statements[0].params).toEqual(["2030-06-01 08:09:10.123"]);
|
||||
expect(statements[1].params).toEqual([
|
||||
"2030-06-01 08:09:10.123",
|
||||
"2030-06-01 08:09:10.123",
|
||||
"1",
|
||||
"2030-06-01 08:09:10.123",
|
||||
]);
|
||||
});
|
||||
@@ -9,9 +9,11 @@ import { invalidateNewsCache } from "./news-cache";
|
||||
|
||||
/** Publish a bounded batch and persist its refresh work before committing. */
|
||||
export async function publishDueArticles(now = new Date()): Promise<number> {
|
||||
// Match Drizzle's TIMESTAMP writes; raw Date parameters use the host timezone.
|
||||
const utcNow = now.toISOString().slice(0, -1).replace("T", " ");
|
||||
const published = await db.transaction(async (tx) => {
|
||||
const [rows] = await tx.execute(
|
||||
sql`SELECT CAST(id AS CHAR) AS id,user_id AS userId,publish_at AS publishAt FROM website_articles WHERE status='scheduled' AND publish_at<=${now} ORDER BY publish_at,id LIMIT 100 FOR UPDATE`,
|
||||
sql`SELECT CAST(id AS CHAR) AS id,user_id AS userId,publish_at AS publishAt FROM website_articles WHERE status='scheduled' AND publish_at<=${utcNow} ORDER BY publish_at,id LIMIT 100 FOR UPDATE`,
|
||||
);
|
||||
let published = 0;
|
||||
for (const article of rows as unknown as Array<{
|
||||
@@ -20,7 +22,7 @@ export async function publishDueArticles(now = new Date()): Promise<number> {
|
||||
publishAt: Date | string;
|
||||
}>) {
|
||||
const [result] = await tx.execute(
|
||||
sql`UPDATE website_articles SET status='published',published_at=${now},updated_at=${now} WHERE id=${article.id} AND status='scheduled' AND publish_at<=${now}`,
|
||||
sql`UPDATE website_articles SET status='published',published_at=${utcNow},updated_at=${utcNow} WHERE id=${article.id} AND status='scheduled' AND publish_at<=${utcNow}`,
|
||||
);
|
||||
if ((result as unknown as { affectedRows: number }).affectedRows !== 1)
|
||||
continue;
|
||||
|
||||
Reference in new issue
Block a user