-
Notifications
You must be signed in to change notification settings - Fork 136
refactor: implement optimistic locking using version columns (#2408) #2465
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,160 @@ | ||
| import "@testing-library/jest-dom/vitest"; | ||
| import { render, screen, fireEvent, waitFor } from "@testing-library/react"; | ||
| import { describe, it, expect, vi, beforeEach } from "vitest"; | ||
| import type { User } from "@supabase/supabase-js"; | ||
| import { QueryClientProvider, queryClient } from "@/hooks/useReactQueryReplacement"; | ||
| import { EditEventDialog } from "./EditEventDialog"; | ||
|
|
||
| // Mock Supabase client | ||
| const mockSingle = vi.fn(); | ||
| const mockUpdate = vi.fn(); | ||
|
|
||
| vi.mock("@/lib/supabase/client", () => ({ | ||
| createClient: () => ({ | ||
| from: vi.fn().mockImplementation((table: string) => { | ||
| if (table === "event_categories") { | ||
| return { | ||
| select: vi.fn().mockReturnValue({ | ||
| order: vi.fn().mockReturnValue({ | ||
| order: vi.fn().mockResolvedValue({ | ||
| data: [{ id: "cat-1", name: "Tech" }], | ||
| error: null, | ||
| }), | ||
| }), | ||
| }), | ||
| }; | ||
| } | ||
| if (table === "events") { | ||
| return { | ||
| select: vi.fn().mockReturnValue({ | ||
| eq: vi.fn().mockReturnValue({ single: mockSingle }), | ||
| }), | ||
| update: mockUpdate, | ||
| }; | ||
| } | ||
| return {}; | ||
| }), | ||
| }), | ||
| })); | ||
|
|
||
| const mockUser = { id: "user-1" } as User; | ||
|
|
||
| const baseEvent = { | ||
| id: "evt-1", | ||
| title: "Hackathon 2024", | ||
| description: "Original description", | ||
| category_id: "cat-1", | ||
| location: "Main Auditorium", | ||
| start_date: "2026-09-15T10:00:00.000Z", | ||
| end_date: "2026-09-15T11:00:00.000Z", | ||
| tags: [] as string[], | ||
| version: 1, | ||
| version_vector: {}, | ||
| }; | ||
|
|
||
| function renderDialog() { | ||
| return render( | ||
| <QueryClientProvider client={queryClient}> | ||
| <EditEventDialog event={baseEvent} user={mockUser} onSuccess={vi.fn()} /> | ||
| </QueryClientProvider>, | ||
| ); | ||
| } | ||
|
|
||
| describe("EditEventDialog Optimistic Concurrency Control", () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| it("rejects a stale save (0 rows updated) and shows the merge conflict modal with the new DB state", async () => { | ||
| // Pre-save merge fetch: server still on version 1 (no field conflict yet) | ||
| mockSingle.mockResolvedValueOnce({ | ||
| data: { ...baseEvent, version: 1 }, | ||
| error: null, | ||
| }); | ||
|
|
||
| // Capture the OCC predicates used on the UPDATE | ||
| const predicates: { key: string; value: unknown }[] = []; | ||
| mockUpdate.mockReturnValue({ | ||
| eq: vi.fn((key: string, value: unknown) => { | ||
| predicates.push({ key, value }); | ||
| return { | ||
| eq: vi.fn((key2: string, value2: unknown) => { | ||
| predicates.push({ key: key2, value: value2 }); | ||
| return { | ||
| select: vi.fn().mockResolvedValue({ | ||
| data: [], | ||
| error: null, | ||
| }), | ||
| }; | ||
| }), | ||
| }; | ||
| }), | ||
| }); | ||
|
|
||
| // UI recovery fetch: another admin already bumped the version to 2 | ||
| mockSingle.mockResolvedValueOnce({ | ||
| data: { ...baseEvent, description: "Server edited description", version: 2 }, | ||
| error: null, | ||
| }); | ||
|
|
||
| renderDialog(); | ||
|
|
||
| fireEvent.click(screen.getByRole("button", { name: "Edit Event" })); | ||
| fireEvent.change(await screen.findByPlaceholderText("Event description"), { | ||
| target: { value: "My local edit" }, | ||
| }); | ||
| fireEvent.click(screen.getByRole("button", { name: "Save Changes" })); | ||
|
|
||
| // The UPDATE must be guarded by id + version (optimistic locking) | ||
| await waitFor(() => { | ||
| expect(predicates).toContainEqual({ key: "version", value: 1 }); | ||
| expect(predicates).toContainEqual({ key: "id", value: "evt-1" }); | ||
| }); | ||
|
|
||
| // Conflict modal pops up showing exactly what the other admin changed | ||
| await waitFor(() => { | ||
| expect(screen.getByText("Concurrent Edit Conflict Detected")).toBeInTheDocument(); | ||
| }); | ||
| expect(screen.getByText("Server edited description")).toBeInTheDocument(); | ||
| expect(screen.getAllByText("My local edit").length).toBeGreaterThan(0); | ||
| }); | ||
|
|
||
| it("saves successfully when the submitted version still matches the database", async () => { | ||
| mockSingle.mockResolvedValueOnce({ | ||
| data: { ...baseEvent, version: 1 }, | ||
| error: null, | ||
| }); | ||
|
|
||
| mockUpdate.mockReturnValue({ | ||
| eq: vi.fn().mockReturnValue({ | ||
| eq: vi.fn().mockReturnValue({ | ||
| select: vi.fn().mockResolvedValue({ | ||
| data: [{ id: "evt-1", version: 2 }], | ||
| error: null, | ||
| }), | ||
| }), | ||
| }), | ||
| }); | ||
|
|
||
| renderDialog(); | ||
|
|
||
| fireEvent.click(screen.getByRole("button", { name: "Edit Event" })); | ||
| fireEvent.click(await screen.findByRole("button", { name: "Save Changes" })); | ||
|
|
||
| // The save must write the next version (2) atomically in the update payload | ||
| await waitFor(() => { | ||
| expect(mockUpdate).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| title: "Hackathon 2024", | ||
| description: "Original description", | ||
| version: 2, | ||
| }), | ||
| ); | ||
| }); | ||
|
|
||
| // Dialog closes on success | ||
| await waitFor(() => { | ||
| expect(screen.queryByRole("button", { name: "Save Changes" })).not.toBeInTheDocument(); | ||
| }); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| import { createClient } from "@/lib/supabase/client"; | ||
|
|
||
| interface RescheduleBody { | ||
| start_date: string; | ||
| end_date: string; | ||
| event_date: string; | ||
| version: number; | ||
| } | ||
|
|
||
| export async function PATCH(req: Request, { params }: { params: { id: string } }) { | ||
| const eventId = params.id; | ||
| const supabase = createClient(); | ||
|
|
||
| let body: RescheduleBody; | ||
| try { | ||
| body = (await req.json()) as RescheduleBody; | ||
| } catch { | ||
| return new Response("Invalid request body", { status: 400 }); | ||
| } | ||
|
|
||
| const targetVersion = Number(body.version); | ||
| if (!Number.isInteger(targetVersion)) { | ||
| return new Response("Missing expected version for optimistic locking", { status: 400 }); | ||
| } | ||
|
|
||
| // Guarded update: only succeeds when the event is still on the version the | ||
| // client fetched, so a concurrent reschedule/edit cannot be silently overwritten. | ||
| const { data, error } = await supabase | ||
| .from("events") | ||
| .update({ | ||
| start_date: body.start_date, | ||
| end_date: body.end_date, | ||
| event_date: body.event_date, | ||
| updated_at: new Date().toISOString(), | ||
| version: targetVersion + 1, | ||
| }) | ||
| .eq("id", eventId) | ||
| .eq("version", targetVersion) | ||
| .select("id, version"); | ||
|
|
||
| if (error) { | ||
| return new Response(error.message, { status: 500 }); | ||
| } | ||
|
|
||
| // 0 rows affected -> another user bumped the version first. | ||
| if (!data || data.length === 0) { | ||
| return new Response( | ||
| "Conflict: This event was modified by another user. Please refresh and try again.", | ||
| { status: 409 }, | ||
| ); | ||
|
Comment on lines
+46
to
+50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift Return the current event state with the conflict response. The After the guarded update affects zero rows, return a structured conflict payload with the current event fields and version. Update the client to handle 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| return Response.json({ | ||
| success: true, | ||
| eventId, | ||
| updatedStart: body.start_date, | ||
| updatedEnd: body.end_date, | ||
| message: "Event rescheduled successfully", | ||
| }); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.