Add HttpTabularProxyStorage for remote table operations#534
Merged
Conversation
…dexes for consistency with other adapters
…hods (Tasks 2-5) put, putBulk, get, getBulk, delete, query, getAll, count, size, deleteAll, deleteSearch — each with in-process fake-server tests. getOffsetPage and pagination still stubbed (Task 6); subscribeToChanges still stubbed (Task 8). All 15 new tests pass. https://claude.ai/code/session_01NRd4zupMz88R5LiyTEZW9H
Implements getOffsetPage, getPage, and queryPage on the proxy so pagination requests are forwarded to the server in one shot rather than round-tripping twice through query(). Extends makeFakeServer with the three corresponding op handlers and adds pagination tests. Also adds local validatePageRequest() call before forwarding getPage and queryPage so StorageValidationError is thrown client-side for invalid orderBy columns/directions rather than being wrapped in a generic Error from the server. https://claude.ai/code/session_01NRd4zupMz88R5LiyTEZW9H
…rprint keys, deepEqual comparator) https://claude.ai/code/session_01NRd4zupMz88R5LiyTEZW9H
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new HttpTabularProxyStorage implementation in @workglow/storage that forwards ITabularStorage operations over HTTP (POST {basePath}/{table}/{op}), plus a dedicated test suite validating behavior against an in-memory backing store.
Changes:
- Introduce
HttpTabularProxyStorage(HTTP-forwarding adapter) including polling-basedsubscribeToChanges. - Add a comprehensive
HttpTabularProxyStoragetest suite and run existing generic tabular-storage contracts against the proxy. - Export the new storage from
@workglow/storageviapackages/storage/src/common.ts.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| packages/storage/src/tabular/HttpTabularProxyStorage.ts | Implements the HTTP proxy tabular storage (CRUD/query/paging/queryIndex + polling subscriptions). |
| packages/test/src/test/storage-tabular/HttpTabularProxyStorage.test.ts | Adds tests and generic contract coverage for the new HTTP proxy storage. |
| packages/storage/src/common.ts | Re-exports HttpTabularProxyStorage from the package public surface. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+99
to
+116
| protected async call<T>(op: string, body: unknown): Promise<T> { | ||
| const res = await this.fetchImpl(this.url(op), { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify(body ?? {}), | ||
| }); | ||
| if (!res.ok) { | ||
| let message: string; | ||
| try { | ||
| const data = (await res.json()) as { error?: string }; | ||
| message = data?.error ?? `HTTP ${res.status}`; | ||
| } catch { | ||
| message = `HTTP ${res.status}`; | ||
| } | ||
| throw new Error(`HttpTabularProxyStorage(${this.table}/${op}): ${message}`); | ||
| } | ||
| return (await res.json()) as T; | ||
| } |
Comment on lines
+139
to
+140
| await this.call<{ ok: true }>("delete", { key }); | ||
| this.events.emit("delete", key as keyof Entity); |
Comment on lines
+215
to
+223
| override async queryIndex<K extends keyof Entity & string>( | ||
| criteria: SearchCriteria<Entity>, | ||
| options: CoveringIndexQueryOptions<Entity, K> | ||
| ): Promise<Pick<Entity, K>[]> { | ||
| const { entities } = await this.call<{ entities: Pick<Entity, K>[] }>("queryIndex", { | ||
| criteria, | ||
| options, | ||
| }); | ||
| return entities; |
Comment on lines
+45
to
+53
| readonly indexes?: readonly ( | ||
| | keyof Schema["properties"] | ||
| | readonly (keyof Schema["properties"])[] | ||
| )[]; | ||
| /** Optional base path. Defaults to `/api/storage`. Must NOT have a trailing slash. */ | ||
| readonly basePath?: string; | ||
| /** Forwarded to BaseTabularStorage. Defaults to "if-missing". */ | ||
| readonly clientProvidedKeys?: ClientProvidedKeysOption; | ||
| } |
| // row1 -> name="x|y", type="z" -> identifier "x|y|z" (name + "|" + type) | ||
| // row2 -> name="x", type="y|z" -> identifier "x|y|z" (same string! that's the bug) | ||
| // So we assert by checking both rows' `option` values are seen: | ||
| const options = inserts.map((_, i) => i); // placeholder — we check via a second listener |
Comment on lines
+508
to
+513
| // Both distinct rows must be reported — if one shadowed the other only one INSERT fires. | ||
| expect(inserts).toContain("x|y|z"); | ||
| expect(inserts).toContain("x|y|z"); | ||
| // More specific assertion: both composite identifiers present | ||
| expect(inserts.some((s) => s === "x|y|z")).toBe(true); | ||
| // The two rows have DIFFERENT composite keys under fingerprinting: |
| } as const; | ||
| const TestPrimaryKey = ["id"] as const; | ||
|
|
||
| function makeFakeServer<Schema extends typeof TestSchema, PK extends typeof TestPrimaryKey>( |
The full `bun run build` (@workglow/test build-types via tsgo) failed where the storage-package-only type build passed: - makeFakeServer was generic over <Schema, PK>, causing excessively-deep type instantiation (TS2589) when matched against the proxy's generics. Retyped its param to AnyTabularStorage — the permissive supertype — so it isn't re-instantiated per caller. - Removed leftover placeholder code in the compound-key collision test (unused 'options' var → TS6133) and the redundant first listener; the test now asserts both rows via their unique 'option' values directly.
Coverage Report
File CoverageNo changed files found. |
…dling in HttpTabularProxyStorage Introduces a new utility function to trim trailing slashes from the base path in HttpTabularProxyStorage. Updates error handling in the fake server to provide a more user-friendly error message for internal exceptions. Adds tests to verify the new functionality, including the correct handling of trailing slashes and error messages.
- regex-free trailing-slash strip on basePath (clears CodeQL polynomial-regex alert) + doc fix - document JSON-safe schema constraint (no Uint8Array/bigint round-trip) - normalize emitted key in delete() to the primary key (parity with other backends) - add validateSelect/validateQueryParams to queryIndex() for fail-fast parity
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
HttpTabularProxyStorage, a new storage adapter that forwards allITabularStorageoperations as HTTP POST requests to a remote server. This enables tabular storage to work across process boundaries (web, Electron, etc.) without requiring direct database access.Key Changes
New
HttpTabularProxyStorageclass (packages/storage/src/tabular/HttpTabularProxyStorage.ts):BaseTabularStorageand implements the fullITabularStorageinterfacefetchimplementationPOST {basePath}/{table}/{op}URL pattern for all requests/api/storage)PollingSubscriptionManagersetupDatabase,destroy) are no-ops for a proxyComprehensive test suite (
packages/test/src/test/storage-tabular/HttpTabularProxyStorage.test.ts):runGenericTabularStorageTestswith multiple schemasExport in common.ts: Added
export * from "./tabular/HttpTabularProxyStorage"to make the new class available from@workglow/storageNotable Implementation Details
HttpTabularProxyFetchtype matches DOMfetchsignature, allowing real browser fetch, Honoapp.fetch, or Electron IPC shims to work without adaptation{ error }message propagatedmakeFingerprint) on primary keys to correctly identify rows across polling cycles, preventing collisions when naive string joins would failgetBulk([])returns immediately without calling the serverhttps://claude.ai/code/session_01NRd4zupMz88R5LiyTEZW9H