- Notifications
You must be signed in to change notification settings - Fork 0
Revert #135: API contract semantics and pagination metadata#149
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -198,6 +198,6 @@ export async function POST(request: Request, { params }: { params: Promise<{ id: | ||
| return NextResponse.json({ job }, { status: 201 }); | ||
| } catch (error) { | ||
| if (error instanceof AuthenticationError) return unauthorizedResponse(); | ||
| return jsonError(error); | ||
| return jsonError(error, 400); | ||
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.
For a valid reindex request, failures while reading the document, updating its status, or inserting the ingestion job now pass ordinary Useful? React with 👍 / 👎. | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,24 +1,17 @@ | ||
| import { NextResponse } from "next/server"; | ||
| import { z } from "zod"; | ||
| import { demoSummary, getDemoDocument } from "@/lib/demo-data"; | ||
| import { isDemoMode } from "@/lib/env"; | ||
| import { summarizeDocument } from "@/lib/rag"; | ||
| import { jsonError } from "@/lib/http"; | ||
| import { consumeApiRateLimit, rateLimitJsonResponse } from "@/lib/api-rate-limit"; | ||
| import { createAdminClient } from "@/lib/supabase/admin"; | ||
| import { AuthenticationError, requireAuthenticatedUser, unauthorizedResponse } from "@/lib/supabase/auth"; | ||
| import { parseRouteParams } from "@/lib/validation/params"; | ||
| export const runtime = "nodejs"; | ||
| const summarizeRouteParamsSchema = z.object({ | ||
| id: z.string().uuid(), | ||
| }); | ||
| export async function POST(request: Request, { params }: { params: Promise<{ id: string }> }) { | ||
| try { | ||
| const { id: rawId } = await params; | ||
| const { id } = parseRouteParams({ id: rawId }, summarizeRouteParamsSchema, "Invalid document id."); | ||
| const { id } = await params; | ||
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.
When the document id in the path is malformed, this now accepts the raw value and reaches Useful? React with 👍 / 👎. | ||
| if (isDemoMode()) { | ||
| if (!getDemoDocument(id)) { | ||
| return NextResponse.json({ error: "Demo document not found." }, { status: 404 }); | ||
| @@ -39,6 +32,6 @@ export async function POST(request: Request, { params }: { params: Promise<{ id: | ||
| if (error instanceof Error && error.message === "Document not found.") { | ||
| return NextResponse.json({ error: "Document not found." }, { status: 404 }); | ||
| } | ||
| return jsonError(error); | ||
| return jsonError(error, 400); | ||
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.
When the request is valid but Useful? React with 👍 / 👎. | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,19 +1,13 @@ | ||
| import { NextResponse } from "next/server"; | ||
| import { z } from "zod"; | ||
| import { isDemoMode } from "@/lib/env"; | ||
| import { jsonError } from "@/lib/http"; | ||
| import { createAdminClient } from "@/lib/supabase/admin"; | ||
| import { AuthenticationError, requireAuthenticatedUser, unauthorizedResponse } from "@/lib/supabase/auth"; | ||
| import { parseRequestQuery, queryInteger } from "@/lib/validation/query"; | ||
| export const runtime = "nodejs"; | ||
| const ACTIVE_BATCH_STATUSES = new Set(["queued", "processing"]); | ||
| const ACTIVE_INDEXING_POLL_MS = 5_000; | ||
| const ingestionBatchesQuerySchema = z.object({ | ||
| limit: queryInteger({ fallback: 20, min: 1, max: 200 }), | ||
| offset: queryInteger({ fallback: 0, min: 0, max: 10_000 }), | ||
| }); | ||
| type BatchRow = Record<string, unknown> & { status?: string | null }; | ||
| @@ -46,34 +40,19 @@ function batchesResponse(batches: BatchRow[], extra: Record<string, unknown> = { | ||
| export async function GET(request: Request) { | ||
| try { | ||
| const { limit, offset } = parseRequestQuery(request, ingestionBatchesQuerySchema, "Invalid ingestion batches query."); | ||
| if (isDemoMode()) { | ||
| return batchesResponse([], { | ||
| demoMode: true, | ||
| pagination: { limit, offset, total: 0, nextOffset: offset, hasMore: false }, | ||
| }); | ||
| } | ||
| if (isDemoMode()) return batchesResponse([], { demoMode: true }); | ||
| const supabase = createAdminClient(); | ||
| const user = await requireAuthenticatedUser(request, supabase); | ||
| const { data, error, count } = await supabase | ||
| const { data, error } = await supabase | ||
| .from("import_batches") | ||
| .select("*", { count: "exact" }) | ||
| .select("*") | ||
| .eq("owner_id", user.id) | ||
| .order("created_at", { ascending: false }) | ||
| .range(offset, offset + limit - 1); | ||
| .limit(20); | ||
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.
For workspaces with more than 20 import batches, Useful? React with 👍 / 👎. | ||
| if (error) throw new Error(error.message); | ||
| const batches = (data ?? []) as unknown as BatchRow[]; | ||
| return batchesResponse(batches, { | ||
| pagination: { | ||
| limit, | ||
| offset, | ||
| total: count ?? batches.length, | ||
| nextOffset: offset + batches.length, | ||
| hasMore: count === null ? batches.length === limit : offset + batches.length < count, | ||
| }, | ||
| }); | ||
| return batchesResponse((data ?? []) as unknown as BatchRow[]); | ||
| } catch (error) { | ||
| if (error instanceof AuthenticationError) return unauthorizedResponse(); | ||
| return jsonError(error); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -4,7 +4,7 @@ import { isDemoMode } from "@/lib/env"; | ||
| import { jsonError } from "@/lib/http"; | ||
| import { createAdminClient } from "@/lib/supabase/admin"; | ||
| import { AuthenticationError, requireAuthenticatedUser, unauthorizedResponse } from "@/lib/supabase/auth"; | ||
| import { optionalUuidQuery, parseRequestQuery, queryInteger } from "@/lib/validation/query"; | ||
| import { optionalUuidQuery, parseRequestQuery } from "@/lib/validation/query"; | ||
| export const runtime = "nodejs"; | ||
| @@ -13,8 +13,6 @@ const ACTIVE_INDEXING_POLL_MS = 5_000; | ||
| const ingestionJobsQuerySchema = z.object({ | ||
| batchId: optionalUuidQuery(), | ||
| limit: queryInteger({ fallback: 100, min: 1, max: 200 }), | ||
| offset: queryInteger({ fallback: 0, min: 0, max: 10_000 }), | ||
| }); | ||
| type JobRow = Record<string, unknown> & { status?: string | null }; | ||
| @@ -48,38 +46,24 @@ function jobsResponse(jobs: JobRow[], extra: Record<string, unknown> = {}) { | ||
| export async function GET(request: Request) { | ||
| try { | ||
| const { batchId, limit, offset } = parseRequestQuery(request, ingestionJobsQuerySchema, "Invalid ingestion jobs query."); | ||
| if (isDemoMode()) { | ||
| return jobsResponse([], { | ||
| demoMode: true, | ||
| pagination: { limit, offset, total: 0, nextOffset: offset, hasMore: false }, | ||
| }); | ||
| } | ||
| if (isDemoMode()) return jobsResponse([], { demoMode: true }); | ||
| const supabase = createAdminClient(); | ||
| const user = await requireAuthenticatedUser(request, supabase); | ||
| const { batchId } = parseRequestQuery(request, ingestionJobsQuerySchema, "Invalid ingestion jobs query."); | ||
| let query = supabase | ||
| .from("ingestion_jobs") | ||
| .select("*, documents!inner(title,file_name,status,owner_id)", { count: "exact" }) | ||
| .select("*, documents!inner(title,file_name,status,owner_id)") | ||
| .eq("documents.owner_id", user.id) | ||
| .order("created_at", { ascending: false }) | ||
| .range(offset, offset + limit - 1); | ||
| .limit(100); | ||
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.
For users with more than 100 ingestion jobs, Useful? React with 👍 / 👎. | ||
| if (batchId) query = query.eq("batch_id", batchId); | ||
| const { data, error, count } = await query; | ||
| const { data, error } = await query; | ||
| if (error) throw new Error(error.message); | ||
| const jobs = (data ?? []) as unknown as JobRow[]; | ||
| return jobsResponse(jobs, { | ||
| pagination: { | ||
| limit, | ||
| offset, | ||
| total: count ?? jobs.length, | ||
| nextOffset: offset + jobs.length, | ||
| hasMore: count === null ? jobs.length === limit : offset + jobs.length < count, | ||
| }, | ||
| }); | ||
| return jobsResponse((data ?? []) as unknown as JobRow[]); | ||
| } catch (error) { | ||
| if (error instanceof AuthenticationError) return unauthorizedResponse(); | ||
| return jsonError(error); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -174,6 +174,6 @@ export async function POST(request: Request) { | ||
| return unauthorizedResponse(); | ||
| } | ||
| return jsonError(error); | ||
| return jsonError(error, 400); | ||
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.
When a valid upload reaches this catch because Supabase storage, document insert, or job insert fails, the thrown ordinary Useful? React with 👍 / 👎. | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the path id is not a UUID, e.g.
/api/documents/not-a-uuid/labels, this now passes the raw string intorequireOwnedDocument, which filtersdocuments.ideven thoughsupabase/schema.sqldefines that column asuuid. Postgres/PostgREST rejects that as an input-syntax error, and this route'sjsonError(error)default turns it into a 500 instead of the prior 400 validation response; PATCH and DELETE have the same raw-param path. Please validate the route id before any Supabase query.Useful? React with 👍 / 👎.