diff --git a/.changeset/history-knows-its-collection.md b/.changeset/history-knows-its-collection.md new file mode 100644 index 0000000..d1f2cb0 --- /dev/null +++ b/.changeset/history-knows-its-collection.md @@ -0,0 +1,11 @@ +--- +'fiber': patch +--- + +Keep each collection's response history to itself. + +Response history was bucketed by request id alone. A loaded endpoint's id is `METHOD /path` and carries no section, so two collections describing the same API — staging and production — shared one list: opening either showed whichever had been sent last, and clearing one deleted both. + +The database has stored `section_id` since the column was added; it was simply never handed back. It is now, so the window can tell the two apart, and clearing is scoped to the collection you cleared. + +Entries recorded before this still show for either collection rather than disappearing, since nothing knows which one they came from. A scoped clear takes them too — they are the same request's older entries, and leaving them behind would look like the clear half-worked. diff --git a/src-tauri/src/history.rs b/src-tauri/src/history.rs index 7acbd0e..80c4bfd 100644 --- a/src-tauri/src/history.rs +++ b/src-tauri/src/history.rs @@ -55,6 +55,15 @@ pub struct HistoryRecord { pub id: String, /// The saved request this belongs to, or `scratch`. pub request_id: String, + /// The collection it was sent from, when it had one. + /// + /// Stored since the column was added but never handed back, which left the + /// window unable to tell two collections apart: a request id is unique + /// within a section, and a loaded endpoint's id — `METHOD /path` — is the + /// same in every collection describing the same API. `None` for entries + /// written before this was returned, and for a scratch send. + #[serde(default)] + pub section_id: Option, /// Epoch milliseconds. pub at: i64, pub method: String, @@ -285,7 +294,7 @@ impl HistoryStore { let mut statement = connection.prepare( "SELECT id, request_id, at, method, url, request_body, error, status, status_text, final_url, headers, is_binary, truncated, - size_bytes, ttfb_ms, total_ms + size_bytes, ttfb_ms, total_ms, section_id FROM history ORDER BY at DESC LIMIT ?1", )?; @@ -326,6 +335,7 @@ impl HistoryStore { Ok(HistoryRecord { id: row.get(0)?, request_id: row.get(1)?, + section_id: row.get(16).unwrap_or_default(), at: row.get(2)?, method: row.get(3)?, url: row.get(4)?, @@ -396,8 +406,25 @@ impl HistoryStore { self.remove_where("id = ?1", params![id]) } - pub fn clear_request(&self, request_id: &str) -> Result<(), HistoryError> { - self.remove_where("request_id = ?1", params![request_id]) + /// Clears one request's history, within one collection when given. + /// + /// Unscoped still means every collection, which is what a scratch request + /// and an entry written before `section_id` was returned both need. Scoped + /// also takes the null-section rows for that id: they are the same + /// request's older entries, and leaving them behind would look like the + /// clear half-worked. + pub fn clear_request( + &self, + request_id: &str, + section_id: Option<&str>, + ) -> Result<(), HistoryError> { + match section_id { + Some(section) => self.remove_where( + "request_id = ?1 AND (section_id = ?2 OR section_id IS NULL)", + params![request_id, section], + ), + None => self.remove_where("request_id = ?1", params![request_id]), + } } pub fn clear_all(&self) -> Result<(), HistoryError> { @@ -673,7 +700,7 @@ mod tests { .record(&spec("b1", "req-b"), 2, "u", &Ok(response("{}"))) .unwrap(); - store.clear_request("req-a").unwrap(); + store.clear_request("req-a", None).unwrap(); let left = store.list(10).unwrap(); assert_eq!(left.len(), 1); assert_eq!(left[0].request_id, "req-b"); diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 1f08b71..7e75306 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -607,8 +607,9 @@ mod gui { async fn history_clear_request( log: State<'_, HistoryStore>, request_id: String, + section_id: Option, ) -> Result<(), HistoryError> { - log.clear_request(&request_id) + log.clear_request(&request_id, section_id.as_deref()) } #[tauri::command] diff --git a/src/lib/api.ts b/src/lib/api.ts index 5b5edaa..c038f1e 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -57,6 +57,8 @@ export interface ResponseData extends ResponseMeta { export interface HistoryRecord { id: string; requestId: string; + /** The collection it was sent from. Null for scratch, and for old entries. */ + sectionId: string | null; at: number; method: string; url: string; @@ -77,8 +79,8 @@ export function historyDelete(id: string): Promise { return invoke('history_delete', { id }); } -export function historyClearRequest(requestId: string): Promise { - return invoke('history_clear_request', { requestId }); +export function historyClearRequest(requestId: string, sectionId?: string | null): Promise { + return invoke('history_clear_request', { requestId, sectionId: sectionId ?? null }); } export function historyClearAll(): Promise { diff --git a/src/lib/history.svelte.ts b/src/lib/history.svelte.ts index d9b7800..b0ccd05 100644 --- a/src/lib/history.svelte.ts +++ b/src/lib/history.svelte.ts @@ -16,6 +16,18 @@ export interface HistoryEntry { id: string; /** The saved request this belongs to, or `SCRATCH_ID`. */ requestId: string; + /** + * The collection it was sent from, when it had one. + * + * A request id is unique only *within* a section, and a loaded endpoint's id + * — `METHOD /path` — is identical in every collection describing the same + * API. Bucketing on the id alone put staging's and production's replies in + * one list, so opening either showed whichever was sent last. + * + * `null` for a scratch send, and for anything recorded before this was + * returned; those match any section rather than disappearing. + */ + sectionId: string | null; at: number; method: string; url: string; @@ -33,6 +45,7 @@ function fromRecord(record: HistoryRecord): HistoryEntry { return { id: record.id, requestId: record.requestId, + sectionId: record.sectionId ?? null, at: record.at, method: record.method, url: record.url, @@ -76,20 +89,41 @@ class History { } } - /** Newest first. */ - forRequest(requestId: string): HistoryEntry[] { - return this.entries.filter((entry) => entry.requestId === requestId); + /** + * The bucket a request's entries live in. + * + * Section first, because a request id is only unique inside one: two + * collections describing the same API give every loaded endpoint the same + * id, and keying on that alone merged their histories. + */ + static #bucket(requestId: string, sectionId: string | null | undefined): string { + return sectionId ? `${sectionId}\u0000${requestId}` : requestId; + } + + /** + * Newest first. + * + * An entry with no section belongs to whichever collection asks. It was + * written before the section came back from the database, and the request it + * names is real — dropping it would look like history had been lost. + */ + forRequest(requestId: string, sectionId?: string | null): HistoryEntry[] { + return this.entries.filter( + (entry) => + entry.requestId === requestId && + (!sectionId || !entry.sectionId || entry.sectionId === sectionId) + ); } /** The entry on screen for a request — an explicit pick, else its newest. */ - selectedFor(requestId: string): HistoryEntry | undefined { - const mine = this.forRequest(requestId); - const picked = this.#selected[requestId]; + selectedFor(requestId: string, sectionId?: string | null): HistoryEntry | undefined { + const mine = this.forRequest(requestId, sectionId); + const picked = this.#selected[History.#bucket(requestId, sectionId)]; return mine.find((entry) => entry.id === picked) ?? mine[0]; } - select(requestId: string, entryId: string): void { - this.#selected[requestId] = entryId; + select(requestId: string, entryId: string, sectionId?: string | null): void { + this.#selected[History.#bucket(requestId, sectionId)] = entryId; } /** The entry opened from the History tab, if it's still around. */ @@ -125,7 +159,7 @@ class History { // Sending shows the new response, not whatever history was open. this.viewingId = null; this.entries.unshift({ ...entry, pending: true, bodyLoaded: false }); - this.select(entry.requestId, entry.id); + this.select(entry.requestId, entry.id, entry.sectionId); } /** @@ -213,18 +247,30 @@ class History { } } - async clearFor(requestId: string): Promise { - const removed = this.entries.filter((entry) => entry.requestId === requestId); - const picked = this.#selected[requestId]; - this.entries = this.entries.filter((entry) => entry.requestId !== requestId); - delete this.#selected[requestId]; + /** + * Clears one request's history, in one collection when it has one. + * + * Scoped by the same rule `forRequest` reads by, so what disappears is + * exactly what was on screen — clearing staging used to take production's + * entries with it, since both sat under one key. + */ + async clearFor(requestId: string, sectionId?: string | null): Promise { + const mine = (entry: HistoryEntry) => + entry.requestId === requestId && + (!sectionId || !entry.sectionId || entry.sectionId === sectionId); + + const removed = this.entries.filter(mine); + const key = History.#bucket(requestId, sectionId); + const picked = this.#selected[key]; + this.entries = this.entries.filter((entry) => !mine(entry)); + delete this.#selected[key]; try { - await historyClearRequest(requestId); + await historyClearRequest(requestId, sectionId); } catch (error) { // Newest-first order survives: the survivors kept theirs, and the // removed slice kept its own, so a merge by timestamp restores both. this.entries = [...this.entries, ...removed].sort((a, b) => b.at - a.at); - if (picked !== undefined) this.#selected[requestId] = picked; + if (picked !== undefined) this.#selected[key] = picked; this.error = String(error); } } diff --git a/src/routes/+page.svelte b/src/routes/+page.svelte index cf18722..39d61cb 100644 --- a/src/routes/+page.svelte +++ b/src/routes/+page.svelte @@ -96,6 +96,14 @@ bodyKind === 'json' ? validateJsonBody(bodySchema, draft.body) : [] ); const requestKey = $derived(selection?.request.id ?? SCRATCH_ID); + /** + * Which collection the history bucket belongs to. + * + * `requestKey` alone is not a bucket: a loaded endpoint's id is the same in + * every collection describing the same API, so staging and production shared + * one list and each showed whichever was sent last. + */ + const requestSection = $derived(selection?.section.id ?? null); const baseUrl = $derived(selection?.section.baseUrl ?? ''); const bodilessMethod = $derived(draft.method === 'GET' || draft.method === 'HEAD'); /** @@ -141,7 +149,7 @@ // An entry opened from the History tab wins; otherwise a request shows its // own most recent response and never another request's. - const shown = $derived(history.viewing ?? history.selectedFor(requestKey)); + const shown = $derived(history.viewing ?? history.selectedFor(requestKey, requestSection)); /** * The section to sign back into, when the shown response reads as the API @@ -554,6 +562,7 @@ history.start({ id, requestId: requestKey, + sectionId: requestSection, at: Date.now(), method: draft.method, url, @@ -1318,7 +1327,7 @@ history.clearFor(requestKey)} + onSelect={() => history.clearFor(requestKey, requestSection)} > Clear this request's history diff --git a/tests/e2e/mock-ipc.ts b/tests/e2e/mock-ipc.ts index c861f8d..7c66184 100644 --- a/tests/e2e/mock-ipc.ts +++ b/tests/e2e/mock-ipc.ts @@ -90,6 +90,7 @@ export function historyRecord(over: Partial = {}): HistoryRecord return { id: 'h1', requestId: 'r1', + sectionId: null, at: 1_700_000_000_000, method: 'GET', url: 'https://api.acme.com/users', diff --git a/tests/e2e/twin-collections.spec.ts b/tests/e2e/twin-collections.spec.ts new file mode 100644 index 0000000..b5346b5 --- /dev/null +++ b/tests/e2e/twin-collections.spec.ts @@ -0,0 +1,172 @@ +import { expect, test } from '@playwright/test'; +import { commands, install, response, section } from './mock-ipc'; + +/** + * Two collections describing one API — staging and production — which is how + * anybody works on an API they also run. + * + * Every loaded endpoint has the same id in both, because a loaded id is + * `METHOD /path` and deliberately carries no section: it is the identity a + * saved body and a refresh must agree on. Anything else keyed on that id alone + * therefore cannot tell the two collections apart, and this file is where that + * assumption gets tested rather than assumed. + */ + +const loader = { + enabled: true, + url: '/openapi.json', + method: 'GET', + query: '.', + next: '', + ttlSeconds: 0 +}; + +const twins = [ + section({ + id: 'staging', + name: 'Staging', + baseUrl: 'https://staging.acme.com', + order: 0, + loader + }), + section({ + id: 'prod', + name: 'Production', + baseUrl: 'https://api.acme.com', + order: 1, + loader + }) +]; + +const oneEndpoint = [ + { method: 'GET', path: '/users', name: 'List users', description: '', body: '' } +]; + +/** The staging row, then the production row. */ +function rows(page: import('@playwright/test').Page) { + return page.getByText('List users', { exact: true }); +} + +test('a response in one collection does not appear under the other', async ({ page }) => { + await install(page, { + sections: twins, + loaded: oneEndpoint, + sendResponse: response({ status: 201, statusText: 'Created' }) + }); + await page.goto('/'); + + await rows(page).nth(0).click(); + await page.getByRole('button', { name: 'Send' }).click(); + await expect(page.getByText('201 Created')).toBeVisible(); + + // Production has never been sent. Its pane should be offering to send, not + // showing staging's reply. + await rows(page).nth(1).click(); + await expect(page.getByText('Send a request to see the response.')).toBeVisible(); + await expect(page.getByText('201 Created')).toBeHidden(); +}); + +test('clearing one collection history leaves the other alone', async ({ page }) => { + await install(page, { + sections: twins, + loaded: oneEndpoint, + sendResponse: response() + }); + await page.goto('/'); + + await rows(page).nth(0).click(); + await page.getByRole('button', { name: 'Send' }).click(); + await expect(page.getByText('200 OK')).toBeVisible(); + + await rows(page).nth(1).click(); + await page.getByRole('button', { name: 'Send' }).click(); + await expect(page.getByText('200 OK')).toBeVisible(); + + // Whatever the UI does, the backend must not be asked to delete a bucket + // that holds both collections' entries under one key. + const cleared = await commands(page, 'history_clear_request'); + expect(cleared).toHaveLength(0); + + // The request id is deliberately the same in both — it is the endpoint's + // identity, and a refresh has to re-attach to it. What tells the two apart, + // and what the history bucket is keyed on, is the section travelling beside + // it. + const sent = await commands(page, 'send_request'); + expect(sent).toHaveLength(2); + const specs = sent.map((call) => call.args.spec as { requestId: string; sectionId: string }); + expect(specs.map((spec) => spec.requestId)).toEqual(['GET /users', 'GET /users']); + expect(specs.map((spec) => spec.sectionId)).toEqual(['staging', 'prod']); +}); + +test('editing a body in one collection does not change the other', async ({ page }) => { + await install(page, { + sections: twins, + loaded: [ + { + method: 'POST', + path: '/users', + name: 'Create user', + description: '', + body: '{\n "name": "string"\n}' + } + ] + }); + await page.goto('/'); + + const created = page.getByText('Create user', { exact: true }); + await created.nth(0).click(); + await page.getByRole('tab', { name: 'Body' }).click(); + + const editor = page.locator('.cm-content').first(); + await editor.click(); + await page.keyboard.press('ControlOrMeta+a'); + await page.keyboard.type('{"name": "staging only"}'); + + await created.nth(1).click(); + await page.getByRole('tab', { name: 'Body' }).click(); + await expect(page.locator('.cm-content').first()).not.toContainText('staging only'); +}); + +test('forgetting an endpoint in one collection leaves the other showing it', async ({ page }) => { + const orphan = { + id: 'POST /gone', + name: 'gone', + method: 'POST', + path: '/gone', + body: '', + headers: [] + }; + await install(page, { + sections: [ + section({ ...twins[0], overlay: [orphan] }), + section({ ...twins[1], overlay: [orphan] }) + ], + loaded: oneEndpoint + }); + await page.goto('/'); + + // An endpoint the loader no longer reports, orphaned in both collections + // under the same id — so forgetting it in one must not take the other's. + const gone = page.getByText('gone', { exact: true }); + await expect(gone).toHaveCount(2); + + await gone.nth(0).click({ button: 'right' }); + await page.getByRole('menuitem', { name: 'Forget this endpoint' }).click(); + await expect(gone).toHaveCount(1); +}); + +test('opening an endpoint asks for its own collection schema', async ({ page }) => { + await install(page, { sections: twins, loaded: oneEndpoint }); + await page.goto('/'); + + await rows(page).nth(1).click(); + await expect(page.getByText('https://api.acme.com', { exact: true })).toBeVisible(); + + // The schema is per collection. Asking staging's for a row clicked in + // production would show the wrong document's fields against it. + const asked = await commands(page, 'loader_schema'); + expect(asked.length).toBeGreaterThan(0); + const last = asked[asked.length - 1].args as { sectionId: string; endpointId: string }; + expect(last.sectionId).toBe('prod'); + expect(last.endpointId).toBe('GET /users'); +});