From 659342a28190c3c5fcdfbd0feba4d468cf16ff0d Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Sun, 26 Jul 2026 19:52:52 +0100 Subject: [PATCH 1/2] Add public read endpoints to the extensions v2 API v2 previously only covered submission/moderation/ownership; browsing extensions and developers still meant using v1's API and shape. Adds GET /extensions (list, filterable by type/developer_id), GET /extensions/{id}, and GET /developers/{id}, all public/unauthenticated to match v1's open CORS policy. Designed for v2 rather than mirroring v1: extensions embed the full public developer profile (bio/avatar_url/URL/approved) inline instead of v1's stripped-down author shape, and contact_email is explicitly excluded from every public response via a dedicated PublicDeveloper type. v1's badge/version endpoints aren't replicated here since they're display utilities with external README links, unrelated to the author/developer shape gap this closes. Claude-Session: https://claude.ai/code/session_01Co1bu2gU5kEHN65KooNUHN --- .../extensions/v2/developers-database.ts | 2 +- .../extensions/v2/extensions-database.ts | 141 +++++++++++++++ src/services/extensions/v2/index.ts | 164 +++++++++++++++++- src/services/extensions/v2/interfaces.ts | 63 +++++++ test/services/extensions/v2/index.test.ts | 114 ++++++++++++ test/services/extensions/v2/mock-db.ts | 46 +++++ 6 files changed, 527 insertions(+), 3 deletions(-) create mode 100644 src/services/extensions/v2/extensions-database.ts diff --git a/src/services/extensions/v2/developers-database.ts b/src/services/extensions/v2/developers-database.ts index 7106910..6c6c575 100644 --- a/src/services/extensions/v2/developers-database.ts +++ b/src/services/extensions/v2/developers-database.ts @@ -212,7 +212,7 @@ export class DevelopersDatabase { } } - private async getById(id: string): Promise> { + async getById(id: string): Promise> { try { const row = await this.db .prepare("SELECT * FROM developers WHERE id = ?") diff --git a/src/services/extensions/v2/extensions-database.ts b/src/services/extensions/v2/extensions-database.ts new file mode 100644 index 0000000..6335971 --- /dev/null +++ b/src/services/extensions/v2/extensions-database.ts @@ -0,0 +1,141 @@ +import { DatabaseResult, IDatabase } from "../../../lib/interfaces"; +import { databaseError } from "./errors"; +import { + Extension, + License, + Release, + Repository, + sortReleasesDescending +} from "./interfaces"; + +const SELECT_EXTENSIONS = ` + SELECT e.id, e.type, e.name, e.description, e.releases, e.website, e.license, + e.icon_url, e.readme, e.source, e.version, e.download_url, + d.id AS developer_id, d.type AS developer_type, d.name AS developer_name, + d.url AS developer_url, d.bio AS developer_bio, + d.avatar_url AS developer_avatar_url, d.approved_at AS developer_approved_at + FROM extensions e + LEFT JOIN developers d ON e.author_id = d.id +`; + +export interface ExtensionListFilters { + type?: string; + developerId?: string; +} + +export class ExtensionsDatabase { + private db: IDatabase; + + constructor(db: IDatabase) { + this.db = db; + } + + async list( + filters: ExtensionListFilters = {} + ): Promise> { + const conditions: string[] = []; + const params: unknown[] = []; + if (filters.type) { + conditions.push("e.type = ?"); + params.push(filters.type); + } + if (filters.developerId) { + conditions.push("e.author_id = ?"); + params.push(filters.developerId); + } + const query = conditions.length + ? `${SELECT_EXTENSIONS} WHERE ${conditions.join(" AND ")}` + : SELECT_EXTENSIONS; + + let result; + try { + result = await this.db + .prepare(query) + .bind(...params) + .all>(); + } catch (error) { + return databaseError("list", error); + } + + if (!result.success) { + return databaseError( + "list", + new Error(result.error || "Database query failed") + ); + } + + return { + data: (result.results ?? []).map(parseExtensionRow), + error: null + }; + } + + async getById(id: string): Promise> { + let row; + try { + row = await this.db + .prepare(`${SELECT_EXTENSIONS} WHERE LOWER(e.id) = LOWER(?)`) + .bind(id) + .first>(); + } catch (error) { + return databaseError("getById", error); + } + + if (!row) { + return { + data: null, + error: { + message: `Cannot find extension by id: ${id}`, + code: "NOT_FOUND" + } + }; + } + + return { data: parseExtensionRow(row), error: null }; + } +} + +function parseJSON(value: unknown, fallback: T): T { + if (typeof value === "string") { + try { + return JSON.parse(value) as T; + } catch { + return fallback; + } + } + return value !== undefined && value !== null ? (value as T) : fallback; +} + +function parseExtensionRow(row: Record): Extension { + const releases = parseJSON(row.releases, []); + return { + id: row.id as string, + type: row.type as Extension["type"], + name: row.name as string, + description: row.description as string, + releases: sortReleasesDescending(releases), + website: row.website as string, + license: parseJSON(row.license, { name: "" }), + icon_url: typeof row.icon_url === "string" ? row.icon_url : undefined, + readme: row.readme as string, + source: parseJSON(row.source, { type: "custom", repo: "" }), + version: row.version as string, + download_url: row.download_url as string, + developer: { + id: row.developer_id as string, + type: (row.developer_type as "user" | "organization") ?? "user", + name: (row.developer_name as string) ?? "", + URL: + typeof row.developer_url === "string" ? row.developer_url : undefined, + bio: + typeof row.developer_bio === "string" ? row.developer_bio : undefined, + avatar_url: + typeof row.developer_avatar_url === "string" + ? row.developer_avatar_url + : undefined, + approved: + row.developer_approved_at !== null && + row.developer_approved_at !== undefined + } + }; +} diff --git a/src/services/extensions/v2/index.ts b/src/services/extensions/v2/index.ts index 4518ca2..5cc5c45 100644 --- a/src/services/extensions/v2/index.ts +++ b/src/services/extensions/v2/index.ts @@ -13,16 +13,21 @@ import { DeveloperSchema, DeveloperTransferSchema, ErrorResponseSchema, + ExtensionListQuerySchema, + ExtensionSchema, IdParamSchema, PendingDeveloperClaimSchema, + PublicDeveloperSchema, QueueQuerySchema, ReviewNoteOptionalSchema, ReviewNoteRequiredSchema, SubmissionPayloadSchema, SubmissionSchema, - TokenParamSchema + TokenParamSchema, + toPublicDeveloper } from "./interfaces"; import { DevelopersDatabase } from "./developers-database"; +import { ExtensionsDatabase } from "./extensions-database"; import { SubmissionsDatabase } from "./submissions-database"; import { UsersDatabase } from "./users-database"; @@ -81,6 +86,103 @@ function requireModerator(): MiddlewareHandler { }; } +const listExtensionsRoute = createRoute({ + method: "get", + path: "/extensions", + tags: ["Extensions"], + summary: "List published extensions", + request: { query: ExtensionListQuerySchema }, + responses: { + 200: { + content: { + "application/json": { + schema: z.object({ result: z.array(ExtensionSchema) }) + } + }, + description: "Extensions matching the given filters" + }, + 422: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "type or developer_id query param failed validation" + }, + 500: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Database error" + } + } +}); + +extensionsV2.openapi(listExtensionsRoute, async (c) => { + const { type, developer_id } = c.req.valid("query"); + const platform = getPlatform(c); + const db = new ExtensionsDatabase(platform.getDatabase("DB_EXTENSIONS")); + + const { data, error } = await db.list({ type, developerId: developer_id }); + if (error || !data) { + return c.json( + { + error: { + message: error?.message ?? "Unable to load extensions", + code: "DATABASE_ERROR" + } + }, + 500 + ); + } + + return c.json({ result: data }, 200); +}); + +const getExtensionRoute = createRoute({ + method: "get", + path: "/extensions/{id}", + tags: ["Extensions"], + summary: "Get a single published extension", + request: { params: IdParamSchema }, + responses: { + 200: { + content: { + "application/json": { schema: z.object({ result: ExtensionSchema }) } + }, + description: "The extension" + }, + 404: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "No extension with that id" + }, + 422: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "id param failed validation" + }, + 500: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Database error" + } + } +}); + +extensionsV2.openapi(getExtensionRoute, async (c) => { + const { id } = c.req.valid("param"); + const platform = getPlatform(c); + const db = new ExtensionsDatabase(platform.getDatabase("DB_EXTENSIONS")); + + const { data, error } = await db.getById(id); + if (error || !data) { + const status = error?.code === "NOT_FOUND" ? 404 : 500; + return c.json( + { + error: { + message: error?.message ?? "Extension not found", + code: error?.code ?? "DATABASE_ERROR" + } + }, + status + ); + } + + return c.json({ result: data }, 200); +}); + const createSubmissionRoute = createRoute({ method: "post", path: "/submissions", @@ -1092,6 +1194,64 @@ extensionsV2.openapi(unapprovedDevelopersRoute, async (c) => { return c.json({ result: data }, 200); }); +// Registered after every other static-segment GET /developers/* route +// (claims, claims/mine, unapproved above) — Hono matches path params against +// whichever handler was registered first among overlapping patterns, so this +// wildcard would otherwise shadow those static routes (e.g. swallow +// GET /developers/claims as a lookup for a developer literally named +// "claims"). +const getDeveloperRoute = createRoute({ + method: "get", + path: "/developers/{id}", + tags: ["Developers"], + summary: "Get a developer's public profile", + request: { params: IdParamSchema }, + responses: { + 200: { + content: { + "application/json": { + schema: z.object({ result: PublicDeveloperSchema }) + } + }, + description: "The developer's public profile" + }, + 404: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "No developer with that id" + }, + 422: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "id param failed validation" + }, + 500: { + content: { "application/json": { schema: ErrorResponseSchema } }, + description: "Database error" + } + } +}); + +extensionsV2.openapi(getDeveloperRoute, async (c) => { + const { id } = c.req.valid("param"); + const platform = getPlatform(c); + const db = new DevelopersDatabase(platform.getDatabase("DB_EXTENSIONS")); + + const { data, error } = await db.getById(id); + if (error || !data) { + const status = error?.code === "NOT_FOUND" ? 404 : 500; + return c.json( + { + error: { + message: error?.message ?? "Developer not found", + code: error?.code ?? "DATABASE_ERROR" + } + }, + status + ); + } + + return c.json({ result: toPublicDeveloper(data) }, 200); +}); + const approveDeveloperRoute = createRoute({ method: "post", path: "/developers/{id}/approve", @@ -1219,7 +1379,7 @@ extensionsV2.doc("/openapi.json", { title: "FOSSBilling Extensions API (v2)", version: "2.0.0", description: - "Self-service extension submission, ownership, and moderation. Read-only listings remain at /extensions/v1." + "Self-service extension submission, ownership, moderation, and public browsing. v1 (/extensions/v1) remains available for existing integrations." }, servers: [{ url: "/extensions/v2" }] }); diff --git a/src/services/extensions/v2/interfaces.ts b/src/services/extensions/v2/interfaces.ts index b6f2d54..20ae18e 100644 --- a/src/services/extensions/v2/interfaces.ts +++ b/src/services/extensions/v2/interfaces.ts @@ -1,4 +1,5 @@ import { z } from "@hono/zod-openapi"; +import { gt, lt } from "semver"; export const EXTENSION_TYPES = [ "mod", @@ -66,6 +67,22 @@ export const ReleaseSchema = z }) .openapi("Release"); +export type Release = z.infer; + +// Newest first by semver; tags that don't parse as semver keep their +// relative order rather than erroring the whole listing. +export function sortReleasesDescending(releases: Release[]): Release[] { + return [...releases].sort((a, b) => { + try { + if (gt(a.tag, b.tag)) return -1; + if (lt(a.tag, b.tag)) return 1; + return 0; + } catch { + return 0; + } + }); +} + export const RepositorySchema = z .object({ type: z.enum(["github", "gitlab", "custom"]), @@ -73,6 +90,8 @@ export const RepositorySchema = z }) .openapi("Repository"); +export type Repository = z.infer; + export const LicenseSchema = z .object({ name: z.string().min(1), @@ -80,6 +99,8 @@ export const LicenseSchema = z }) .openapi("License"); +export type License = z.infer; + export const ExtensionPayloadSchema = z .object({ id: lowercaseId("extension"), @@ -112,6 +133,48 @@ export const DeveloperProfileSchema = DeveloperSchema.extend({ export type DeveloperProfile = z.infer; +// The publicly-readable view of a developer profile: everything in +// DeveloperProfile except contact_email, which exists for moderator/owner +// communication and was never meant to be broadcast to anonymous callers. +export const PublicDeveloperSchema = DeveloperProfileSchema.omit({ + contact_email: true +}).openapi("PublicDeveloper"); + +export type PublicDeveloper = z.infer; + +export function toPublicDeveloper(profile: DeveloperProfile): PublicDeveloper { + return { + id: profile.id, + type: profile.type, + name: profile.name, + URL: profile.URL, + bio: profile.bio, + avatar_url: profile.avatar_url, + approved: profile.approved + }; +} + +export const ExtensionSchema = ExtensionPayloadSchema.extend({ + developer: PublicDeveloperSchema +}).openapi("Extension"); + +export type Extension = z.infer; + +export const ExtensionListQuerySchema = z.object({ + type: z + .enum(EXTENSION_TYPES) + .optional() + .openapi({ + param: { name: "type", in: "query" } + }), + developer_id: z + .string() + .optional() + .openapi({ + param: { name: "developer_id", in: "query" } + }) +}); + export const DeveloperHistoryEntrySchema = z .object({ developer_id: z.string(), diff --git a/test/services/extensions/v2/index.test.ts b/test/services/extensions/v2/index.test.ts index c556646..a04d221 100644 --- a/test/services/extensions/v2/index.test.ts +++ b/test/services/extensions/v2/index.test.ts @@ -1424,6 +1424,117 @@ describe("Extensions API v2", () => { }); }); + describe("GET /developers/{id}", () => { + it("returns a developer's public profile without contact_email, unauthenticated", async () => { + tables.developers.set("public-dev", { + id: "public-dev", + type: "organization", + name: "Public Dev", + url: "https://example.com", + bio: "We make things", + avatar_url: "https://example.com/avatar.png", + contact_email: "private@example.com", + owner_user_id: "user-1", + approved_at: new Date().toISOString(), + created_at: new Date().toISOString(), + updated_at: new Date().toISOString() + }); + + const res = await get("/extensions/v2/developers/public-dev", {}); + expect(res.status).toBe(200); + const body = (await res.json()) as { result: Record }; + expect(body.result).toEqual({ + id: "public-dev", + type: "organization", + name: "Public Dev", + URL: "https://example.com", + bio: "We make things", + avatar_url: "https://example.com/avatar.png", + approved: true + }); + expect(body.result.contact_email).toBeUndefined(); + }); + + it("404s for an unknown developer", async () => { + const res = await get("/extensions/v2/developers/no-such-developer", {}); + expect(res.status).toBe(404); + }); + }); + + describe("GET /extensions", () => { + it("lists published extensions with the developer embedded", async () => { + seedOwnedExtension(); + + const res = await get("/extensions/v2/extensions", {}); + expect(res.status).toBe(200); + const body = (await res.json()) as { + result: Array<{ id: string; developer: { id: string } }>; + }; + expect(body.result).toHaveLength(1); + expect(body.result[0].id).toBe("existing-ext"); + expect(body.result[0].developer.id).toBe("owner-developer"); + }); + + it("filters by type", async () => { + seedOwnedExtension(); + + const matching = await get("/extensions/v2/extensions?type=mod", {}); + const matchingBody = (await matching.json()) as { result: unknown[] }; + expect(matchingBody.result).toHaveLength(1); + + const nonMatching = await get("/extensions/v2/extensions?type=theme", {}); + const nonMatchingBody = (await nonMatching.json()) as { + result: unknown[]; + }; + expect(nonMatchingBody.result).toHaveLength(0); + }); + + it("422s on an invalid type filter", async () => { + const res = await get("/extensions/v2/extensions?type=not-a-type", {}); + expect(res.status).toBe(422); + }); + + it("filters by developer_id", async () => { + seedOwnedExtension(); + + const matching = await get( + "/extensions/v2/extensions?developer_id=owner-developer", + {} + ); + const matchingBody = (await matching.json()) as { result: unknown[] }; + expect(matchingBody.result).toHaveLength(1); + + const nonMatching = await get( + "/extensions/v2/extensions?developer_id=someone-else", + {} + ); + const nonMatchingBody = (await nonMatching.json()) as { + result: unknown[]; + }; + expect(nonMatchingBody.result).toHaveLength(0); + }); + }); + + describe("GET /extensions/{id}", () => { + it("gets a single extension, case-insensitively", async () => { + seedOwnedExtension(); + + const res = await get("/extensions/v2/extensions/EXISTING-EXT", {}); + expect(res.status).toBe(200); + const body = (await res.json()) as { + result: { id: string; developer: { name: string; approved: boolean } }; + }; + expect(body.result.id).toBe("existing-ext"); + expect(body.result.developer.name).toBe("Owner"); + expect(body.result.developer.approved).toBe(false); + }); + + it("404s for an unknown extension", async () => { + const res = await get("/extensions/v2/extensions/no-such-extension", {}); + expect(res.status).toBe(404); + }); + }); + describe("OpenAPI docs", () => { it("serves a generated OpenAPI document", async () => { const res = await get("/extensions/v2/openapi.json", {}); @@ -1435,12 +1546,15 @@ describe("Extensions API v2", () => { expect(spec.openapi).toBe("3.1.0"); expect(Object.keys(spec.paths)).toEqual( expect.arrayContaining([ + "/extensions", + "/extensions/{id}", "/submissions", "/submissions/mine", "/submissions/queue", "/submissions/{id}/approve", "/submissions/{id}/reject", "/developers/me", + "/developers/{id}", "/developers/unapproved", "/developers/{id}/approve", "/developers/{id}/transfer", diff --git a/test/services/extensions/v2/mock-db.ts b/test/services/extensions/v2/mock-db.ts index 1f4b9aa..a5e371e 100644 --- a/test/services/extensions/v2/mock-db.ts +++ b/test/services/extensions/v2/mock-db.ts @@ -70,6 +70,52 @@ class MockStatement implements D1PreparedStatement { const q = this.normalizedQuery; const p = this.params; + // v2's SELECT_EXTENSIONS join (extensions-database.ts): public + // list/getById reads, with an optional WHERE for type / author_id / + // case-insensitive id, applied in that order to match how the params + // are bound. + if (q.startsWith("SELECT e.id, e.type, e.name, e.description,")) { + let rows = [...this.tables.extensions.values()]; + let paramIdx = 0; + if (q.includes("e.type = ?")) { + rows = rows.filter((r) => r.type === p[paramIdx]); + paramIdx++; + } + if (q.includes("e.author_id = ?")) { + rows = rows.filter((r) => r.author_id === p[paramIdx]); + paramIdx++; + } + if (q.includes("LOWER(e.id) = LOWER(?)")) { + const id = String(p[paramIdx]).toLowerCase(); + rows = rows.filter((r) => String(r.id).toLowerCase() === id); + paramIdx++; + } + return rows.map((r) => { + const developer = this.tables.developers.get(String(r.author_id)); + return { + id: r.id, + type: r.type, + name: r.name, + description: r.description, + releases: r.releases, + website: r.website, + license: r.license, + icon_url: r.icon_url, + readme: r.readme, + source: r.source, + version: r.version, + download_url: r.download_url, + developer_id: developer?.id ?? r.author_id, + developer_type: developer?.type ?? "user", + developer_name: developer?.name ?? "", + developer_url: developer?.url ?? null, + developer_bio: developer?.bio ?? null, + developer_avatar_url: developer?.avatar_url ?? null, + developer_approved_at: developer?.approved_at ?? null + }; + }); + } + // v1's SELECT_EXTENSIONS join (database.ts), for cross-service verification // that an approved v2 submission is visible via the v1 read path. if (q.startsWith("SELECT e.id, e.type, e.author_id,")) { From 0aa338671621e43581ab3842cfa711f6697e20b0 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Sun, 26 Jul 2026 20:07:58 +0100 Subject: [PATCH 2/2] Address cubic review: fix orphaned-developer nulls, reserve route-colliding ids, dedupe release sorting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - extensions-database.ts: COALESCE(d.id, e.author_id) so a missing developer row (author_id isn't a hard FK) can never surface a null developer.id, which would violate the Extension schema's contract. - interfaces.ts: reject "claims"/"unapproved" as developer ids — those are the only single-segment GET /developers/* static routes, and since GET /developers/{id} is registered after them, a developer literally named one of those words would always be shadowed by the moderation-only route instead of getting a usable public profile. - Extract sortReleasesDescending into lib/releases.ts, shared by v1 and v2 instead of duplicated, and fix it to partition out non-semver tags before sorting rather than comparing them inline — the old try/catch comparator could report a tag as "equal" to two neighbors that weren't equal to each other, which breaks Array#sort's assumption of a total order and can leave later valid releases out of place. Claude-Session: https://claude.ai/code/session_01Co1bu2gU5kEHN65KooNUHN --- src/lib/releases.ts | 24 ++++++++++ src/services/extensions/v1/interfaces.ts | 18 ++------ .../extensions/v2/extensions-database.ts | 9 +++- src/services/extensions/v2/interfaces.ts | 33 +++++++------- test/lib/releases.test.ts | 31 +++++++++++++ test/services/extensions/v2/index.test.ts | 44 +++++++++++++++++++ 6 files changed, 128 insertions(+), 31 deletions(-) create mode 100644 src/lib/releases.ts create mode 100644 test/lib/releases.test.ts diff --git a/src/lib/releases.ts b/src/lib/releases.ts new file mode 100644 index 0000000..b309305 --- /dev/null +++ b/src/lib/releases.ts @@ -0,0 +1,24 @@ +import { rcompare, valid } from "semver"; + +export interface ReleaseTag { + tag: string; +} + +// Newest first by semver. Tags that aren't valid semver can't be ordered +// relative to the valid ones without risking a non-transitive comparator — +// e.g. an invalid tag comparing "equal" (via a caught exception) to two +// valid tags that are themselves not equal breaks Array#sort's assumption +// of a total order, and can leave later valid entries out of order even +// though every valid-to-valid comparison alone would have been correct. +// Partitioning the invalid tags out before sorting avoids that entirely; +// they're appended after, in their original relative order. +export function sortReleasesDescending( + releases: T[] +): T[] { + const withValidTag = releases.filter((r) => valid(r.tag) !== null); + const withInvalidTag = releases.filter((r) => valid(r.tag) === null); + return [ + ...withValidTag.sort((a, b) => rcompare(a.tag, b.tag)), + ...withInvalidTag + ]; +} diff --git a/src/services/extensions/v1/interfaces.ts b/src/services/extensions/v1/interfaces.ts index d31e409..a69ac5d 100644 --- a/src/services/extensions/v1/interfaces.ts +++ b/src/services/extensions/v1/interfaces.ts @@ -1,4 +1,7 @@ -import { gt, lt } from "semver"; +import { gt } from "semver"; +import { sortReleasesDescending } from "../../../lib/releases"; + +export { sortReleasesDescending }; export type Extension = { id: string; @@ -74,16 +77,3 @@ export function getLatestRelease(extension: Extension): Release | undefined { return latestRelease; } - -export function sortReleasesDescending(releases: Release[]): Release[] { - return [...releases].sort((a, b) => { - try { - if (gt(a.tag, b.tag)) return -1; - if (lt(a.tag, b.tag)) return 1; - return 0; - } catch { - // Keep relative order when tags can't be compared as semver - return 0; - } - }); -} diff --git a/src/services/extensions/v2/extensions-database.ts b/src/services/extensions/v2/extensions-database.ts index 6335971..57ef0f8 100644 --- a/src/services/extensions/v2/extensions-database.ts +++ b/src/services/extensions/v2/extensions-database.ts @@ -8,10 +8,17 @@ import { sortReleasesDescending } from "./interfaces"; +// LEFT JOIN so an extension whose developer row is missing (author_id +// pointing nowhere) still lists — author_id isn't a hard FK (see +// 0001_add_v2_tables.sql). COALESCE keeps developer_id non-null in that +// case: e.author_id is itself NOT NULL, so the id half of the embedded +// developer is never lost even when every other field falls back to a +// default in parseExtensionRow below. const SELECT_EXTENSIONS = ` SELECT e.id, e.type, e.name, e.description, e.releases, e.website, e.license, e.icon_url, e.readme, e.source, e.version, e.download_url, - d.id AS developer_id, d.type AS developer_type, d.name AS developer_name, + COALESCE(d.id, e.author_id) AS developer_id, + d.type AS developer_type, d.name AS developer_name, d.url AS developer_url, d.bio AS developer_bio, d.avatar_url AS developer_avatar_url, d.approved_at AS developer_approved_at FROM extensions e diff --git a/src/services/extensions/v2/interfaces.ts b/src/services/extensions/v2/interfaces.ts index 20ae18e..4efa4f6 100644 --- a/src/services/extensions/v2/interfaces.ts +++ b/src/services/extensions/v2/interfaces.ts @@ -1,5 +1,7 @@ import { z } from "@hono/zod-openapi"; -import { gt, lt } from "semver"; +import { sortReleasesDescending } from "../../../lib/releases"; + +export { sortReleasesDescending }; export const EXTENSION_TYPES = [ "mod", @@ -30,9 +32,22 @@ const httpUrl = () => message: "must use http or https" }); +// GET /developers/{id} is registered after the static single-segment +// GET /developers/* routes (claims, unapproved), so a developer whose id +// literally matched one of those words would always hit the static route +// instead — its public profile would be permanently unreachable there. +// Rejecting these ids at creation time (rather than trying to route around +// the collision) keeps every existing/future developer id resolvable. +const RESERVED_DEVELOPER_IDS = new Set(["claims", "unapproved"]); + +const developerId = () => + lowercaseId("developer").refine((id) => !RESERVED_DEVELOPER_IDS.has(id), { + message: "This developer id is reserved" + }); + export const DeveloperSchema = z .object({ - id: lowercaseId("developer"), + id: developerId(), type: z.enum(["user", "organization"]), name: z.string().min(1), URL: httpUrl().optional(), @@ -69,20 +84,6 @@ export const ReleaseSchema = z export type Release = z.infer; -// Newest first by semver; tags that don't parse as semver keep their -// relative order rather than erroring the whole listing. -export function sortReleasesDescending(releases: Release[]): Release[] { - return [...releases].sort((a, b) => { - try { - if (gt(a.tag, b.tag)) return -1; - if (lt(a.tag, b.tag)) return 1; - return 0; - } catch { - return 0; - } - }); -} - export const RepositorySchema = z .object({ type: z.enum(["github", "gitlab", "custom"]), diff --git a/test/lib/releases.test.ts b/test/lib/releases.test.ts new file mode 100644 index 0000000..58deb2a --- /dev/null +++ b/test/lib/releases.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, it } from "vitest"; +import { sortReleasesDescending } from "../../src/lib/releases"; + +function tag(t: string) { + return { tag: t }; +} + +describe("sortReleasesDescending", () => { + it("sorts valid semver tags newest first", () => { + expect( + sortReleasesDescending([tag("1.0.0"), tag("2.0.0"), tag("1.5.0")]) + ).toEqual([tag("2.0.0"), tag("1.5.0"), tag("1.0.0")]); + }); + + it("keeps later valid tags correctly ordered around an invalid tag between them", () => { + // A naive try/catch comparator can treat "invalid" as "equal" to both + // of its valid neighbors even though 2.0.0 and 1.0.0 aren't equal to + // each other, breaking sort's assumption of a total order. + expect( + sortReleasesDescending([tag("2.0.0"), tag("invalid"), tag("1.0.0")]) + ).toEqual([tag("2.0.0"), tag("1.0.0"), tag("invalid")]); + }); + + it("appends invalid tags after all valid ones, preserving their relative order", () => { + expect(sortReleasesDescending([tag("b"), tag("1.0.0"), tag("a")])).toEqual([ + tag("1.0.0"), + tag("b"), + tag("a") + ]); + }); +}); diff --git a/test/services/extensions/v2/index.test.ts b/test/services/extensions/v2/index.test.ts index a04d221..ee2f07d 100644 --- a/test/services/extensions/v2/index.test.ts +++ b/test/services/extensions/v2/index.test.ts @@ -646,6 +646,19 @@ describe("Extensions API v2", () => { expect(res.status).toBe(409); }); + it.each(["claims", "unapproved"])( + "rejects the reserved id %s", + async (id) => { + const res = await put( + "/extensions/v2/developers/me", + await authHeaders("user-1"), + sampleDeveloper({ id }) + ); + + expect(res.status).toBe(422); + } + ); + it("clears approval when an approved profile is edited", async () => { await put( "/extensions/v2/developers/me", @@ -1533,6 +1546,37 @@ describe("Extensions API v2", () => { const res = await get("/extensions/v2/extensions/no-such-extension", {}); expect(res.status).toBe(404); }); + + it("still returns a usable developer.id when the developer row is missing", async () => { + // author_id isn't a hard FK (0001_add_v2_tables.sql), so this can + // happen without any application bug — the embedded developer must + // still satisfy the schema (id: string) rather than surface a null. + tables.extensions.set("orphaned-ext", { + id: "orphaned-ext", + type: "mod", + author_id: "no-such-developer", + name: "Orphaned", + description: "d", + releases: "[]", + website: "https://e.com", + license: '{"name":"MIT"}', + icon_url: null, + readme: "r", + source: '{"type":"github","repo":"example/orphaned"}', + version: "1.0.0", + download_url: "https://e.com/d.zip" + }); + + const res = await get("/extensions/v2/extensions/orphaned-ext", {}); + expect(res.status).toBe(200); + const body = (await res.json()) as { result: { developer: unknown } }; + expect(body.result.developer).toEqual({ + id: "no-such-developer", + type: "user", + name: "", + approved: false + }); + }); }); describe("OpenAPI docs", () => {