refactor: 代码审查修复 — 错误边界、DRY抽取、测试修复、合规性改进

- P1: server.ts 统一错误边界 (withErrorHandler + AppError),修复 3 个失败/卡死测试
- P2: db 层 wrap/paginateQuery 抽取,前端 handleResponse 抽取,parseIdFromUrl 抽取
- P3: middleware 验证消息中文化,Flex→Space 替换
- P0: docs/development/README.md 新增已知设计决策章节
- P3-11 setup 拆分已尝试回退(@testing-library/react preload 依赖无法拆分)
- P3-13 config 层测试从本次变更移除
This commit is contained in:
2026-05-29 22:27:56 +08:00
parent 34e915ccf4
commit 10b3928bee
26 changed files with 428 additions and 300 deletions

View File

@@ -1,10 +1,22 @@
import type { SQL } from "drizzle-orm";
import type { SQLiteTable } from "drizzle-orm/sqlite-core";
import Database from "bun:sqlite";
import { and, sql } from "drizzle-orm";
import { drizzle } from "drizzle-orm/bun-sqlite";
import { join } from "node:path";
import type { Logger } from "../logger";
const DB_FILENAME = "alfred.db";
export interface PaginateResult<T> {
items: T[];
page: number;
pageSize: number;
total: number;
}
export function createDatabase(dataDir: string, logger: Logger): Database {
const dbPath = join(dataDir, DB_FILENAME);
const db = new Database(dbPath);
@@ -17,3 +29,47 @@ export function createDatabase(dataDir: string, logger: Logger): Database {
return db;
}
export function paginateQuery<T extends SQLiteTable, R>(
raw: Database,
table: T,
options: {
conditions?: Array<SQL | undefined>;
mapRow: (row: T["$inferSelect"]) => R;
orderBy?: (table: T) => SQL | undefined;
page: number;
pageSize: number;
},
): PaginateResult<R> {
const db = wrap(raw);
const where = options.conditions?.filter((c): c is SQL => c !== undefined);
const whereClause = where && where.length > 0 ? and(...where) : undefined;
const countResult = db
.select({ count: sql<number>`count(*)` })
.from(table)
.where(whereClause)
.get();
const total = Number(countResult?.count ?? 0);
const rows = db
.select()
.from(table)
.where(whereClause)
.orderBy(options.orderBy?.(table) ?? sql`1`)
.limit(options.pageSize)
.offset((options.page - 1) * options.pageSize)
.all();
return {
items: rows.map(options.mapRow),
page: options.page,
pageSize: options.pageSize,
total,
};
}
export function wrap(raw: Database) {
return drizzle(raw);
}

View File

@@ -1,10 +1,10 @@
import type Database from "bun:sqlite";
import { and, desc, eq, like, or, sql } from "drizzle-orm";
import { drizzle } from "drizzle-orm/bun-sqlite";
import { desc, eq, like, or, sql } from "drizzle-orm";
import type { CreateModelRequest, Model, ModelCapability, UpdateModelRequest } from "../../shared/api";
import { paginateQuery, wrap } from "./connection";
import { models, providers } from "./schema";
export function createModel(
@@ -87,7 +87,6 @@ export function listModels(
raw: Database,
options: { keyword?: string; page: number; pageSize: number; providerId?: string },
): { items: Model[]; page: number; pageSize: number; total: number } {
const db = wrap(raw);
const conditions = [];
if (options.providerId) {
@@ -99,31 +98,13 @@ export function listModels(
conditions.push(or(like(models.name, pattern), like(models.modelId, pattern))!);
}
const where = conditions.length > 0 ? and(...conditions) : undefined;
const countResult = db
.select({ count: sql<number>`count(*)` })
.from(models)
.where(where)
.get();
const total = Number(countResult?.count ?? 0);
const rows = db
.select()
.from(models)
.where(where)
.orderBy(desc(models.createdAt))
.limit(options.pageSize)
.offset((options.page - 1) * options.pageSize)
.all();
return {
items: rows.map(toModel),
return paginateQuery(raw, models, {
conditions,
mapRow: toModel,
orderBy: () => desc(models.createdAt),
page: options.page,
pageSize: options.pageSize,
total,
};
});
}
export function updateModel(
@@ -203,7 +184,3 @@ function toModel(row: typeof models.$inferSelect): Model {
updatedAt: row.updatedAt,
};
}
function wrap(raw: Database) {
return drizzle(raw);
}

View File

@@ -1,10 +1,10 @@
import type Database from "bun:sqlite";
import { and, desc, eq, like, or, sql } from "drizzle-orm";
import { drizzle } from "drizzle-orm/bun-sqlite";
import { desc, eq, like, or } from "drizzle-orm";
import type { CreateProjectRequest, Project, ProjectStatus, UpdateProjectRequest } from "../../shared/api";
import { paginateQuery, wrap } from "./connection";
import { projects } from "./schema";
export function archiveProject(raw: Database, id: string): { error: string; status: number } | { project: Project } {
@@ -79,7 +79,6 @@ export function listProjects(
raw: Database,
options: { keyword?: string; page: number; pageSize: number; status?: ProjectStatus },
): { items: Project[]; page: number; pageSize: number; total: number } {
const db = wrap(raw);
const conditions = [];
if (options.status) {
@@ -91,31 +90,13 @@ export function listProjects(
conditions.push(or(like(projects.name, pattern), like(projects.description, pattern))!);
}
const where = conditions.length > 0 ? and(...conditions) : undefined;
const countResult = db
.select({ count: sql<number>`count(*)` })
.from(projects)
.where(where)
.get();
const total = Number(countResult?.count ?? 0);
const rows = db
.select()
.from(projects)
.where(where)
.orderBy(desc(projects.createdAt))
.limit(options.pageSize)
.offset((options.page - 1) * options.pageSize)
.all();
return {
items: rows.map(toProject),
return paginateQuery(raw, projects, {
conditions,
mapRow: toProject,
orderBy: () => desc(projects.createdAt),
page: options.page,
pageSize: options.pageSize,
total,
};
});
}
export function restoreProject(raw: Database, id: string): { error: string; status: number } | { project: Project } {
@@ -187,7 +168,3 @@ function toProject(row: typeof projects.$inferSelect): Project {
updatedAt: row.updatedAt,
};
}
function wrap(raw: Database) {
return drizzle(raw);
}

View File

@@ -1,10 +1,10 @@
import type Database from "bun:sqlite";
import { and, desc, eq, like, sql } from "drizzle-orm";
import { drizzle } from "drizzle-orm/bun-sqlite";
import { desc, eq, like } from "drizzle-orm";
import type { CreateProviderRequest, Provider, ProviderOption, UpdateProviderRequest } from "../../shared/api";
import { paginateQuery, wrap } from "./connection";
import { providers } from "./schema";
export function createProvider(
@@ -80,7 +80,6 @@ export function listProviders(
raw: Database,
options: { keyword?: string; page: number; pageSize: number },
): { items: Provider[]; page: number; pageSize: number; total: number } {
const db = wrap(raw);
const conditions = [];
if (options.keyword) {
@@ -88,31 +87,13 @@ export function listProviders(
conditions.push(like(providers.name, pattern));
}
const where = conditions.length > 0 ? and(...conditions) : undefined;
const countResult = db
.select({ count: sql<number>`count(*)` })
.from(providers)
.where(where)
.get();
const total = Number(countResult?.count ?? 0);
const rows = db
.select()
.from(providers)
.where(where)
.orderBy(desc(providers.createdAt))
.limit(options.pageSize)
.offset((options.page - 1) * options.pageSize)
.all();
return {
items: rows.map(toProvider),
return paginateQuery(raw, providers, {
conditions,
mapRow: toProvider,
orderBy: () => desc(providers.createdAt),
page: options.page,
pageSize: options.pageSize,
total,
};
});
}
export function updateProvider(
@@ -179,7 +160,3 @@ function toProvider(row: typeof providers.$inferSelect): Provider {
updatedAt: row.updatedAt,
};
}
function wrap(raw: Database) {
return drizzle(raw);
}

View File

@@ -0,0 +1,2 @@
export { createApiError, createHeaders, createMetaResponse, formatDuration, jsonResponse } from "./response";
export { parseIdFromUrl } from "./url";

View File

@@ -1,6 +1,6 @@
import type { ApiErrorResponse, MetaResponse, RuntimeMode } from "../shared/api";
import type { ApiErrorResponse, MetaResponse, RuntimeMode } from "../../shared/api";
import { APP } from "../shared/app";
import { APP } from "../../shared/app";
export function createApiError(error: string, status: number): ApiErrorResponse {
return { error, status };

View File

@@ -0,0 +1,3 @@
export function parseIdFromUrl(url: URL): string | undefined {
return url.pathname.split("/")[3];
}

View File

@@ -0,0 +1,37 @@
import type { RuntimeMode } from "../../shared/api";
import type { Logger } from "../logger";
import { createApiError, jsonResponse } from "../helpers";
type RouteHandler = (req: Request) => Promise<Response> | Response;
export class AppError extends Error {
constructor(
message: string,
readonly statusCode: number,
) {
super(message);
this.name = "AppError";
}
}
export function withErrorHandler(fn: RouteHandler, mode: RuntimeMode, logger?: Logger): RouteHandler {
return async (req) => {
try {
return await fn(req);
} catch (error: unknown) {
if (error instanceof AppError) {
return jsonResponse(createApiError(error.message, error.statusCode), {
mode,
status: error.statusCode,
});
}
logger?.error({ error }, "未处理的路由异常");
return jsonResponse(createApiError("服务器内部错误", 500), {
mode,
status: 500,
});
}
};
}

View File

@@ -0,0 +1,2 @@
export { AppError, withErrorHandler } from "./error-handler";
export { validateIdParam, validatePagination, validateTimeRange } from "./validate";

View File

@@ -1,12 +1,12 @@
import type { RuntimeMode } from "../shared/api";
import type { RuntimeMode } from "../../shared/api";
import { createApiError, jsonResponse } from "./helpers";
import { createApiError, jsonResponse } from "../helpers";
const MAX_PAGE_SIZE = 200;
export function validateIdParam(idStr: string, mode: RuntimeMode): Response | { id: string } {
if (!/^[a-zA-Z0-9][a-zA-Z0-9_-]*$/.test(idStr)) {
return jsonResponse(createApiError("Invalid ID parameter", 400), { mode, status: 400 });
return jsonResponse(createApiError("无效的 ID 参数", 400), { mode, status: 400 });
}
return { id: idStr };
}
@@ -22,17 +22,17 @@ export function validatePagination(
if (pageParam !== null) {
page = Number(pageParam);
if (!Number.isInteger(page) || page <= 0) {
return jsonResponse(createApiError("Invalid page parameter", 400), { mode, status: 400 });
return jsonResponse(createApiError("无效的 page 参数", 400), { mode, status: 400 });
}
}
if (pageSizeParam !== null) {
pageSize = Number(pageSizeParam);
if (!Number.isInteger(pageSize) || pageSize <= 0) {
return jsonResponse(createApiError("Invalid pageSize parameter", 400), { mode, status: 400 });
return jsonResponse(createApiError("无效的 pageSize 参数", 400), { mode, status: 400 });
}
if (pageSize > MAX_PAGE_SIZE) {
return jsonResponse(createApiError(`pageSize must not exceed ${MAX_PAGE_SIZE}`, 400), { mode, status: 400 });
return jsonResponse(createApiError(`pageSize 不能超过 ${MAX_PAGE_SIZE}`, 400), { mode, status: 400 });
}
}
@@ -45,18 +45,18 @@ export function validateTimeRange(
mode: RuntimeMode,
): Response | { from: string; to: string } {
if (!from || !to) {
return jsonResponse(createApiError("from and to parameters are required", 400), { mode, status: 400 });
return jsonResponse(createApiError("from to 参数为必填项", 400), { mode, status: 400 });
}
const fromDate = new Date(from);
const toDate = new Date(to);
if (isNaN(fromDate.getTime()) || isNaN(toDate.getTime())) {
return jsonResponse(createApiError("Invalid from or to parameter format", 400), { mode, status: 400 });
return jsonResponse(createApiError("无效的 from to 参数格式", 400), { mode, status: 400 });
}
if (fromDate.getTime() > toDate.getTime()) {
return jsonResponse(createApiError("from must be earlier than to", 400), { mode, status: 400 });
return jsonResponse(createApiError("from 必须早于 to", 400), { mode, status: 400 });
}
return { from: fromDate.toISOString(), to: toDate.toISOString() };

View File

@@ -3,12 +3,12 @@ import type Database from "bun:sqlite";
import type { RuntimeMode } from "../../../shared/api";
import { deleteProject } from "../../db/projects";
import { createApiError, jsonResponse } from "../../helpers";
import { createApiError, jsonResponse, parseIdFromUrl } from "../../helpers";
import { validateIdParam } from "../../middleware";
export function handleDeleteProject(req: Request, db: Database, mode: RuntimeMode): Response {
const url = new URL(req.url);
const idStr = url.pathname.split("/")[3];
const idStr = parseIdFromUrl(url);
const validated = validateIdParam(idStr ?? "", mode);
if (validated instanceof Response) return validated;

View File

@@ -4,12 +4,12 @@ import type { RuntimeMode } from "../../../shared/api";
import { getModelsByProviderId } from "../../db/models";
import { deleteProvider } from "../../db/providers";
import { createApiError, jsonResponse } from "../../helpers";
import { createApiError, jsonResponse, parseIdFromUrl } from "../../helpers";
import { validateIdParam } from "../../middleware";
export function handleDeleteProvider(req: Request, db: Database, mode: RuntimeMode): Response {
const url = new URL(req.url);
const idStr = url.pathname.split("/")[3];
const idStr = parseIdFromUrl(url);
const validated = validateIdParam(idStr ?? "", mode);
if (validated instanceof Response) return validated;

View File

@@ -5,6 +5,7 @@ import type { Logger } from "./logger";
import type { StaticAssets } from "./static";
import { createApiError, jsonResponse } from "./helpers";
import { withErrorHandler } from "./middleware";
import { handleMeta } from "./routes/meta";
import { serveStaticAsset } from "./static";
import { readAppVersion } from "./version";
@@ -38,112 +39,196 @@ export function startServer(options: StartServerOptions) {
routes: {
"/api/*": () => jsonResponse(createApiError("API route not found", 404), { mode, status: 404 }),
"/api/meta": {
GET: async () => {
const resolvedVersion = await resolveVersion();
return handleMeta(mode, resolvedVersion);
},
GET: withErrorHandler(
async () => {
const resolvedVersion = await resolveVersion();
return handleMeta(mode, resolvedVersion);
},
mode,
logger,
),
},
"/api/models": {
GET: async (req) => {
const { handleListModels } = await import("./routes/models/list");
return handleListModels(req, db, mode);
},
POST: async (req) => {
const { handleCreateModel } = await import("./routes/models/create");
return handleCreateModel(req, db, mode);
},
GET: withErrorHandler(
async (req) => {
const { handleListModels } = await import("./routes/models/list");
return handleListModels(req, db, mode);
},
mode,
logger,
),
POST: withErrorHandler(
async (req) => {
const { handleCreateModel } = await import("./routes/models/create");
return handleCreateModel(req, db, mode);
},
mode,
logger,
),
},
"/api/models/:id": {
DELETE: async (req) => {
const { handleDeleteModel } = await import("./routes/models/delete");
return handleDeleteModel(req, db, mode);
},
GET: async (req) => {
const { handleGetModel } = await import("./routes/models/get");
return handleGetModel(req, db, mode);
},
PATCH: async (req) => {
const { handleUpdateModel } = await import("./routes/models/update");
return handleUpdateModel(req, db, mode);
},
DELETE: withErrorHandler(
async (req) => {
const { handleDeleteModel } = await import("./routes/models/delete");
return handleDeleteModel(req, db, mode);
},
mode,
logger,
),
GET: withErrorHandler(
async (req) => {
const { handleGetModel } = await import("./routes/models/get");
return handleGetModel(req, db, mode);
},
mode,
logger,
),
PATCH: withErrorHandler(
async (req) => {
const { handleUpdateModel } = await import("./routes/models/update");
return handleUpdateModel(req, db, mode);
},
mode,
logger,
),
},
"/api/models/test": {
POST: async (req) => {
const { handleTestModelConfig } = await import("./routes/models/test");
return handleTestModelConfig(req, db, mode);
},
POST: withErrorHandler(
async (req) => {
const { handleTestModelConfig } = await import("./routes/models/test");
return handleTestModelConfig(req, db, mode);
},
mode,
logger,
),
},
"/api/projects": {
GET: async (req) => {
const { handleListProjects } = await import("./routes/projects/list");
return handleListProjects(req, db, mode);
},
POST: async (req) => {
const { handleCreateProject } = await import("./routes/projects/create");
return handleCreateProject(req, db, mode);
},
GET: withErrorHandler(
async (req) => {
const { handleListProjects } = await import("./routes/projects/list");
return handleListProjects(req, db, mode);
},
mode,
logger,
),
POST: withErrorHandler(
async (req) => {
const { handleCreateProject } = await import("./routes/projects/create");
return handleCreateProject(req, db, mode);
},
mode,
logger,
),
},
"/api/projects/:id": {
DELETE: async (req) => {
const { handleDeleteProject } = await import("./routes/projects/delete");
return handleDeleteProject(req, db, mode);
},
GET: async (req) => {
const { handleGetProject } = await import("./routes/projects/get");
return handleGetProject(req, db, mode);
},
PATCH: async (req) => {
const { handleUpdateProject } = await import("./routes/projects/update");
return handleUpdateProject(req, db, mode);
},
DELETE: withErrorHandler(
async (req) => {
const { handleDeleteProject } = await import("./routes/projects/delete");
return handleDeleteProject(req, db, mode);
},
mode,
logger,
),
GET: withErrorHandler(
async (req) => {
const { handleGetProject } = await import("./routes/projects/get");
return handleGetProject(req, db, mode);
},
mode,
logger,
),
PATCH: withErrorHandler(
async (req) => {
const { handleUpdateProject } = await import("./routes/projects/update");
return handleUpdateProject(req, db, mode);
},
mode,
logger,
),
},
"/api/projects/:id/archive": {
POST: async (req) => {
const { handleArchiveProject } = await import("./routes/projects/archive");
return handleArchiveProject(req, db, mode);
},
POST: withErrorHandler(
async (req) => {
const { handleArchiveProject } = await import("./routes/projects/archive");
return handleArchiveProject(req, db, mode);
},
mode,
logger,
),
},
"/api/projects/:id/restore": {
POST: async (req) => {
const { handleRestoreProject } = await import("./routes/projects/restore");
return handleRestoreProject(req, db, mode);
},
POST: withErrorHandler(
async (req) => {
const { handleRestoreProject } = await import("./routes/projects/restore");
return handleRestoreProject(req, db, mode);
},
mode,
logger,
),
},
"/api/providers": {
GET: async (req) => {
const { handleListProviders } = await import("./routes/providers/list");
return handleListProviders(req, db, mode);
},
POST: async (req) => {
const { handleCreateProvider } = await import("./routes/providers/create");
return handleCreateProvider(req, db, mode);
},
GET: withErrorHandler(
async (req) => {
const { handleListProviders } = await import("./routes/providers/list");
return handleListProviders(req, db, mode);
},
mode,
logger,
),
POST: withErrorHandler(
async (req) => {
const { handleCreateProvider } = await import("./routes/providers/create");
return handleCreateProvider(req, db, mode);
},
mode,
logger,
),
},
"/api/providers/:id": {
DELETE: async (req) => {
const { handleDeleteProvider } = await import("./routes/providers/delete");
return handleDeleteProvider(req, db, mode);
},
GET: async (req) => {
const { handleGetProvider } = await import("./routes/providers/get");
return handleGetProvider(req, db, mode);
},
PATCH: async (req) => {
const { handleUpdateProvider } = await import("./routes/providers/update");
return handleUpdateProvider(req, db, mode);
},
DELETE: withErrorHandler(
async (req) => {
const { handleDeleteProvider } = await import("./routes/providers/delete");
return handleDeleteProvider(req, db, mode);
},
mode,
logger,
),
GET: withErrorHandler(
async (req) => {
const { handleGetProvider } = await import("./routes/providers/get");
return handleGetProvider(req, db, mode);
},
mode,
logger,
),
PATCH: withErrorHandler(
async (req) => {
const { handleUpdateProvider } = await import("./routes/providers/update");
return handleUpdateProvider(req, db, mode);
},
mode,
logger,
),
},
"/api/providers/options": {
GET: async () => {
const { handleListProviderOptions } = await import("./routes/providers/options");
return handleListProviderOptions(db, mode);
},
GET: withErrorHandler(
async () => {
const { handleListProviderOptions } = await import("./routes/providers/options");
return handleListProviderOptions(db, mode);
},
mode,
logger,
),
},
"/api/providers/test": {
POST: async (req) => {
const { handleTestProviderConfig } = await import("./routes/providers/test");
return handleTestProviderConfig(req, db, mode);
},
POST: withErrorHandler(
async (req) => {
const { handleTestProviderConfig } = await import("./routes/providers/test");
return handleTestProviderConfig(req, db, mode);
},
mode,
logger,
),
},
},
});