Make default project creation race-safe (#206)
This commit is contained in:
parent
2484f93ce6
commit
01ecf11415
@ -54,6 +54,21 @@ async function createProject(
|
|||||||
return id;
|
return id;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async function tryCreateDefaultProject(organizationId: string) {
|
||||||
|
const id = crypto.randomUUID();
|
||||||
|
const inserted = await db
|
||||||
|
.insert(projects)
|
||||||
|
.values({
|
||||||
|
id,
|
||||||
|
organizationId,
|
||||||
|
name: "Default",
|
||||||
|
domain: null,
|
||||||
|
})
|
||||||
|
.onConflictDoNothing()
|
||||||
|
.returning({ id: projects.id });
|
||||||
|
return inserted.length > 0 ? id : null;
|
||||||
|
}
|
||||||
|
|
||||||
async function deleteProject(projectId: string, organizationId: string) {
|
async function deleteProject(projectId: string, organizationId: string) {
|
||||||
const project = await getProjectForOrganization(projectId, organizationId);
|
const project = await getProjectForOrganization(projectId, organizationId);
|
||||||
if (!project) {
|
if (!project) {
|
||||||
@ -76,5 +91,6 @@ export const ProjectRepository = {
|
|||||||
getProjectForOrganization,
|
getProjectForOrganization,
|
||||||
getProjectById,
|
getProjectById,
|
||||||
createProject,
|
createProject,
|
||||||
|
tryCreateDefaultProject,
|
||||||
deleteProject,
|
deleteProject,
|
||||||
} as const;
|
} as const;
|
||||||
|
|||||||
@ -7,6 +7,7 @@ const mocks = vi.hoisted(() => ({
|
|||||||
getProjectById: vi.fn(),
|
getProjectById: vi.fn(),
|
||||||
getProjectForOrganization: vi.fn(),
|
getProjectForOrganization: vi.fn(),
|
||||||
listProjects: vi.fn(),
|
listProjects: vi.fn(),
|
||||||
|
tryCreateDefaultProject: vi.fn(),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
vi.mock("@/server/features/projects/repositories/ProjectRepository", () => ({
|
vi.mock("@/server/features/projects/repositories/ProjectRepository", () => ({
|
||||||
@ -26,30 +27,24 @@ describe("project service", () => {
|
|||||||
for (const mock of Object.values(mocks)) mock.mockReset();
|
for (const mock of Object.values(mocks)) mock.mockReset();
|
||||||
});
|
});
|
||||||
|
|
||||||
it("recovers from the default project unique constraint race", async () => {
|
it("recovers from the default project creation race", async () => {
|
||||||
mocks.getDefaultProjectForOrganization
|
mocks.getDefaultProjectForOrganization
|
||||||
.mockResolvedValueOnce(null)
|
.mockResolvedValueOnce(null)
|
||||||
.mockResolvedValueOnce(defaultProject);
|
.mockResolvedValueOnce(defaultProject);
|
||||||
mocks.createProject.mockRejectedValue(
|
mocks.tryCreateDefaultProject.mockResolvedValue(null);
|
||||||
new Error("UNIQUE constraint failed: projects.organization_id"),
|
|
||||||
);
|
|
||||||
const { getOrCreateDefaultProject } = await import("./projects");
|
const { getOrCreateDefaultProject } = await import("./projects");
|
||||||
|
|
||||||
await expect(getOrCreateDefaultProject("org_1")).resolves.toEqual(
|
await expect(getOrCreateDefaultProject("org_1")).resolves.toEqual(
|
||||||
defaultProject,
|
defaultProject,
|
||||||
);
|
);
|
||||||
expect(mocks.createProject).toHaveBeenCalledWith(
|
expect(mocks.tryCreateDefaultProject).toHaveBeenCalledWith("org_1");
|
||||||
"org_1",
|
|
||||||
"Default",
|
|
||||||
undefined,
|
|
||||||
);
|
|
||||||
expect(mocks.getDefaultProjectForOrganization).toHaveBeenCalledTimes(2);
|
expect(mocks.getDefaultProjectForOrganization).toHaveBeenCalledTimes(2);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("does not swallow unrelated default project create failures", async () => {
|
it("does not swallow unrelated default project create failures", async () => {
|
||||||
const error = new Error("D1 unavailable");
|
const error = new Error("D1 unavailable");
|
||||||
mocks.getDefaultProjectForOrganization.mockResolvedValue(null);
|
mocks.getDefaultProjectForOrganization.mockResolvedValue(null);
|
||||||
mocks.createProject.mockRejectedValue(error);
|
mocks.tryCreateDefaultProject.mockRejectedValue(error);
|
||||||
const { getOrCreateDefaultProject } = await import("./projects");
|
const { getOrCreateDefaultProject } = await import("./projects");
|
||||||
|
|
||||||
await expect(getOrCreateDefaultProject("org_1")).rejects.toBe(error);
|
await expect(getOrCreateDefaultProject("org_1")).rejects.toBe(error);
|
||||||
|
|||||||
@ -51,22 +51,14 @@ export async function getOrCreateDefaultProject(organizationId: string) {
|
|||||||
return mapProject(existing);
|
return mapProject(existing);
|
||||||
}
|
}
|
||||||
|
|
||||||
try {
|
const id = await ProjectRepository.tryCreateDefaultProject(organizationId);
|
||||||
const id = await ProjectRepository.createProject(
|
if (id) {
|
||||||
organizationId,
|
|
||||||
"Default",
|
|
||||||
undefined,
|
|
||||||
);
|
|
||||||
|
|
||||||
return {
|
return {
|
||||||
id,
|
id,
|
||||||
name: "Default",
|
name: "Default",
|
||||||
domain: null,
|
domain: null,
|
||||||
createdAt: new Date().toISOString(),
|
createdAt: new Date().toISOString(),
|
||||||
};
|
};
|
||||||
} catch (error) {
|
|
||||||
if (!isDefaultProjectUniqueConstraintError(error)) {
|
|
||||||
throw error;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
const createdProject =
|
const createdProject =
|
||||||
@ -75,17 +67,7 @@ export async function getOrCreateDefaultProject(organizationId: string) {
|
|||||||
return mapProject(createdProject);
|
return mapProject(createdProject);
|
||||||
}
|
}
|
||||||
|
|
||||||
throw error;
|
throw new AppError("INTERNAL_ERROR");
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
function isDefaultProjectUniqueConstraintError(error: unknown) {
|
|
||||||
if (!(error instanceof Error)) return false;
|
|
||||||
const message = error.message.toLowerCase();
|
|
||||||
return (
|
|
||||||
message.includes("unique constraint failed") &&
|
|
||||||
message.includes("projects.organization_id")
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
export async function getProject(projectId: string) {
|
export async function getProject(projectId: string) {
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user