From 9c14b936f759c1c67bdb29c1b75b50b6c2733685 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 18 Aug 2026 13:31:36 +0000 Subject: [PATCH] Support GitHub-shaped resources: array pagination, PATCH updates, configurable identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three changes a GitHub Issues client needs that the generic path didn't cover: - Paginated reads now treat a top-level-array response body as the page's items (mirroring the non-paginated path), instead of only looking inside an envelope — so a bare-array list with Link-header pagination collects items across pages instead of coming back empty. (#6) - Background updates send the method the item path declares: PUT when the path offers a put (replace), otherwise PATCH. GitHub exposes no PUT for issues or comments, so updates must be PATCH. Derived from the document at discovery time; PUT-declaring resources (e.g. Google Calendar) are unchanged. (#5) - A resource's identity field is configurable via `identityField` (default `id`), used for local storage keys, create reconciliation, and the item-URL path value. GitHub issues are addressed by `number`, not the global `id` the payload also carries. (#4) Adds a GitHub-shaped end-to-end test covering all three, plus discover tests for PUT/PATCH selection. Existing behaviour (id-keyed, PUT updates, enveloped lists) is unchanged; full suite green (103 tests), lint and prettier clean. Closes #4 Closes #5 Closes #6 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01BdadF8jAvK2e8Fmis3hLJF --- __tests__/unit/acceptance/real-world.test.ts | 7 +- __tests__/unit/client/github-shape.test.ts | 183 +++++++++++++++++++ __tests__/unit/resources/discover.test.ts | 25 ++- src/client/client.ts | 43 +++-- src/resources/discover.ts | 29 +++ 5 files changed, 273 insertions(+), 14 deletions(-) create mode 100644 __tests__/unit/client/github-shape.test.ts diff --git a/__tests__/unit/acceptance/real-world.test.ts b/__tests__/unit/acceptance/real-world.test.ts index af46793..0331dd4 100644 --- a/__tests__/unit/acceptance/real-world.test.ts +++ b/__tests__/unit/acceptance/real-world.test.ts @@ -44,7 +44,12 @@ describe('acceptance: giphy.com (apis.guru)', () => { it('discovers /gifs as the only resource', () => { expect(discoverResources(document.paths)).toEqual([ - { collectionPath: '/gifs', itemPath: '/gifs/{gifId}', itemParam: 'gifId' }, + { + collectionPath: '/gifs', + itemPath: '/gifs/{gifId}', + itemParam: 'gifId', + updateMethod: 'PUT', + }, ]); }); diff --git a/__tests__/unit/client/github-shape.test.ts b/__tests__/unit/client/github-shape.test.ts new file mode 100644 index 0000000..52a5d0a --- /dev/null +++ b/__tests__/unit/client/github-shape.test.ts @@ -0,0 +1,183 @@ +import { describe, expect, it } from 'vitest'; +import { createApiClient, type ApiClient } from '../../../src/client/client.js'; +import type { OpenApiDocument } from '../../../src/openapi/types.js'; + +/** + * Exercises the three things a GitHub-shaped API needs that a generic + * `id`-keyed, PUT-updating, enveloped-list client does not provide: + * + * 1. list responses that are a **top-level array** (not an envelope) are + * collected across `Link`-header pages (issue #6); + * 2. an update is sent as **PATCH**, because the item path offers no PUT + * (issue #5); + * 3. the resource identity is a field other than `id` — here `number` — + * used for storage keys and create reconciliation (issue #4). + * + * The `{owner}/{repo}` templating a real GitHub client also needs is handled + * one layer up (in the caller's `fetch`), so the paths here are concrete. + */ +const document: OpenApiDocument = { + openapi: '3.0.0', + info: { title: 'GitHub Issues (test)', version: '1.0.0' }, + paths: { + '/issues': { + get: { + 'x-pagination': [{ scheme: 'nextLink' }], + responses: { + '200': { + description: 'issues', + content: { + 'application/json': { + schema: { type: 'array', items: { type: 'object' } }, + }, + }, + }, + }, + }, + post: { responses: { '201': { description: 'created' } } }, + }, + '/issues/{issue_number}': { + get: { responses: { '200': { description: 'issue' } } }, + patch: { responses: { '200': { description: 'updated' } } }, + }, + }, + components: { + paginationSchemes: { + nextLink: { + type: 'nextLink', + request: { queryParameters: { per_page: { role: 'pageSize' } } }, + response: { headers: { Link: { role: 'nextLink' } } }, + }, + }, + }, +} as unknown as OpenApiDocument; + +interface Call { + method: string; + url: string; +} + +function githubFetch(calls: Call[]): typeof fetch { + let nextNumber = 100; + const impl = async ( + input: RequestInfo | URL, + init?: RequestInit, + ): Promise => { + const url = typeof input === 'string' ? input : input.toString(); + const method = (init?.method ?? 'GET').toUpperCase(); + calls.push({ method, url }); + + if (method === 'GET' && url.includes('/issues')) { + if (url.includes('page=2')) { + return new Response( + JSON.stringify([{ number: 3, title: 'c', state: 'open' }]), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + ); + } + // First page: a bare array plus a Link header pointing at page 2. + return new Response( + JSON.stringify([ + { number: 1, title: 'a', state: 'open' }, + { number: 2, title: 'b', state: 'open' }, + ]), + { + status: 200, + headers: { + 'Content-Type': 'application/json', + Link: '; rel="next"', + }, + }, + ); + } + + if (method === 'POST' && url.endsWith('/issues')) { + const body = JSON.parse(String(init?.body ?? '{}')) as Record< + string, + unknown + >; + nextNumber += 1; + return new Response( + JSON.stringify({ ...body, number: nextNumber, state: 'open' }), + { status: 201, headers: { 'Content-Type': 'application/json' } }, + ); + } + + if (method === 'PATCH' && /\/issues\/\d+$/.test(url)) { + const body = JSON.parse(String(init?.body ?? '{}')) as Record< + string, + unknown + >; + return new Response(JSON.stringify(body), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + } + + return new Response('not found', { status: 404 }); + }; + return impl as typeof fetch; +} + +async function settled(client: { + pendingWrites: () => unknown[]; +}): Promise { + const start = Date.now(); + while (client.pendingWrites().length > 0) { + if (Date.now() - start > 2000) throw new Error('writes did not settle'); + await new Promise((resolve) => setTimeout(resolve, 5)); + } +} + +describe('GitHub-shaped client', () => { + const build = (calls: Call[]): ApiClient => + createApiClient(document, { + baseUrl: 'https://api.github.test', + fetch: githubFetch(calls), + identityField: 'number', + retry: { maxAttempts: 3, baseDelayMs: 1, maxDelayMs: 5 }, + }); + + it('collects a top-level-array list across Link-header pages, keyed by the identity field', async () => { + const calls: Call[] = []; + const client = build(calls); + + await client.sync(); + + const issues = await client.list('/issues'); + expect(issues.map((i) => i['number']).sort()).toEqual([1, 2, 3]); + // Stored under `number`, not `id`. + expect(await client.get('/issues', '2')).toMatchObject({ title: 'b' }); + + const pages = calls.filter((c) => c.method === 'GET'); + expect(pages.length).toBe(2); // walked both pages + }); + + it('reconciles a create onto the server-assigned number', async () => { + const calls: Call[] = []; + const client = build(calls); + + const created = await client.create('/issues', { title: 'new one' }); + // Local id is a temporary uuid until the POST settles. + await settled(client); + + // The server assigned number 101; the record now lives under it. + const stored = await client.get('/issues', '101'); + expect(stored).toMatchObject({ number: 101, title: 'new one' }); + // The temporary local key (the uuid create returned) is gone. + const tempKey = String(created['number']); + expect(tempKey).not.toBe('101'); + expect(await client.get('/issues', tempKey)).toBeUndefined(); + }); + + it('sends an update as PATCH (never PUT)', async () => { + const calls: Call[] = []; + const client = build(calls); + + await client.update('/issues', '101', { state: 'closed' }); + await settled(client); + + const patch = calls.find((c) => c.url.endsWith('/issues/101')); + expect(patch?.method).toBe('PATCH'); + expect(calls.some((c) => c.method === 'PUT')).toBe(false); + }); +}); diff --git a/__tests__/unit/resources/discover.test.ts b/__tests__/unit/resources/discover.test.ts index 61ce11b..648a4e9 100644 --- a/__tests__/unit/resources/discover.test.ts +++ b/__tests__/unit/resources/discover.test.ts @@ -9,10 +9,33 @@ describe('discoverResources', () => { }); expect(resources).toEqual([ - { collectionPath: '/pets', itemPath: '/pets/{petId}', itemParam: 'petId' }, + { + collectionPath: '/pets', + itemPath: '/pets/{petId}', + itemParam: 'petId', + updateMethod: 'PUT', + }, ]); }); + it('uses PATCH as the update method when the item path only offers patch', () => { + const resources = discoverResources({ + '/repos/{owner}/{repo}/issues': { post: {}, get: {} }, + '/repos/{owner}/{repo}/issues/{issue_number}': { get: {}, patch: {} }, + }); + + expect(resources[0]?.updateMethod).toBe('PATCH'); + }); + + it('prefers PUT when the item path declares both put and patch', () => { + const resources = discoverResources({ + '/calendars': {}, + '/calendars/{calendarId}': { put: {}, patch: {} }, + }); + + expect(resources[0]?.updateMethod).toBe('PUT'); + }); + it('omits paths that have no matching item path', () => { const resources = discoverResources({ '/health': {}, diff --git a/src/client/client.ts b/src/client/client.ts index ef9d726..7345744 100644 --- a/src/client/client.ts +++ b/src/client/client.ts @@ -32,6 +32,15 @@ export interface ApiClientOptions { storage?: StorageAdapter; fetch?: typeof fetch; retry?: RetryOptions; + /** + * Record property that holds a resource's identity — the value used as the + * local storage key, read back from a create response to reconcile the + * server-assigned id, and substituted into the item URL's path variable. + * Defaults to `id`. Set this when the API addresses a resource by a + * different field (e.g. GitHub issues are keyed by `number`, not the + * global `id` the payload also carries). + */ + identityField?: string; } export interface PaginateOptions { @@ -173,15 +182,16 @@ function itemFingerprint(item: Record): string { function hasChanges( previous: Record[] | undefined, next: Record[], + idField: string, ): boolean { if (!previous || previous.length !== next.length) { return true; } const previousById = new Map( - previous.map((item) => [String(item['id']), item]), + previous.map((item) => [String(item[idField]), item]), ); return next.some((item) => { - const match = previousById.get(String(item['id'])); + const match = previousById.get(String(item[idField])); return !match || itemFingerprint(match) !== itemFingerprint(item); }); } @@ -233,6 +243,7 @@ export function createApiClient( ): ApiClient { const storage = options.storage ?? new InMemoryStorageAdapter(); const fetchImpl = options.fetch ?? fetch; + const idField = options.identityField ?? 'id'; const routes = discoverResources(document.paths); const byResource = new Map( routes.map((route) => [route.collectionPath, route]), @@ -291,7 +302,7 @@ export function createApiClient( headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(write.data), })) as Record; - const resolvedId = String(created['id']); + const resolvedId = String(created[idField]); if (resolvedId !== write.id) { await storage.delete(write.resource, write.id); } @@ -302,7 +313,7 @@ export function createApiClient( if (write.type === 'update') { const route = resolveRoute(write.resource); const updated = (await requestJson(itemUrl(route, write.id), { - method: 'PUT', + method: route.updateMethod, headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(write.data), })) as Record; @@ -568,7 +579,13 @@ export function createApiClient( } const { body, headers } = await requestJsonWithHeaders(url); - const pageItems = itemsField ? body[itemsField] : undefined; + // A top-level array body is itself the page of items (mirrors the + // non-paginated path); otherwise the items live under `itemsField`. + const pageItems = Array.isArray(body) + ? (body as Record[]) + : itemsField + ? body[itemsField] + : undefined; if (Array.isArray(pageItems)) { items.push(...(pageItems as Record[])); } @@ -634,15 +651,15 @@ export function createApiClient( } const previous = lastSyncedItems.get(route.collectionPath); - if (hasChanges(previous, items)) { + if (hasChanges(previous, items, idField)) { changed.push(route.collectionPath); for (const item of items) { - await storage.put(route.collectionPath, String(item['id']), item); + await storage.put(route.collectionPath, String(item[idField]), item); } if (previous) { - const nextIds = new Set(items.map((item) => String(item['id']))); + const nextIds = new Set(items.map((item) => String(item[idField]))); for (const staleItem of previous) { - const staleId = String(staleItem['id']); + const staleId = String(staleItem[idField]); if (!nextIds.has(staleId)) { await storage.delete(route.collectionPath, staleId); } @@ -701,8 +718,10 @@ export function createApiClient( async create(resource, data): Promise> { const route = resolveRoute(resource); const id = - typeof data['id'] === 'string' ? data['id'] : crypto.randomUUID(); - const record = { ...data, id }; + typeof data[idField] === 'string' + ? (data[idField] as string) + : crypto.randomUUID(); + const record = { ...data, [idField]: id }; await storage.put(route.collectionPath, id, record); enqueueWrite({ resource: route.collectionPath, @@ -716,7 +735,7 @@ export function createApiClient( async update(resource, id, data): Promise> { const route = resolveRoute(resource); const existing = await storage.get(route.collectionPath, id); - const record = { ...existing, ...data, id }; + const record = { ...existing, ...data, [idField]: id }; await storage.put(route.collectionPath, id, record); enqueueWrite({ resource: route.collectionPath, diff --git a/src/resources/discover.ts b/src/resources/discover.ts index 7848176..3cb2bd5 100644 --- a/src/resources/discover.ts +++ b/src/resources/discover.ts @@ -2,6 +2,34 @@ export interface ResourceRoute { collectionPath: string; itemPath: string; itemParam: string; + /** + * HTTP method a background `update` sends. Derived from the item path's + * declared operations: `PUT` when the item path has a `put` operation + * (a full replace), otherwise `PATCH` when it only offers a partial + * update. Some APIs (e.g. GitHub) expose no `PUT` and require `PATCH`. + */ + updateMethod: 'PUT' | 'PATCH'; +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +/** + * Chooses the update method for an item path from the operations it + * declares: prefer `PUT` (replace) when present, fall back to `PATCH` + * when the path only offers a partial update, and default to `PUT` when + * neither is declared (preserving prior behaviour). + */ +function updateMethodFor( + paths: Record, + itemPath: string, +): 'PUT' | 'PATCH' { + const item = paths[itemPath]; + if (isRecord(item) && !('put' in item) && 'patch' in item) { + return 'PATCH'; + } + return 'PUT'; } /** @@ -26,6 +54,7 @@ export function discoverResources( collectionPath, itemPath, itemParam: extractParamName(itemPath), + updateMethod: updateMethodFor(paths, itemPath), }); } }