diff --git a/CHANGELOG.md b/CHANGELOG.md index c91809d..ff0adf0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 > Entries before 1.2.0 were not tracked in this file; see the git history for details. +## [1.4.0] — 2026-07-17 +### Added +- Opt-in **keyset (Id-range-partitioned) parallel paging** for the assets batch fetch. + `getAllAssetsBatch(context, callback, { paging: 'keyset' })` fetches min/max `Id`, splits + the Id space into 16 half-open ranges, and pages each with `Id gt`/`Id le` under the same + 4-way concurrency as `getAllBatch`. For reasonably uniform Id distributions this is + ~2–2.6× faster than `$skip` OFFSET paging on large tenants (measured 2.71× on a ~215k + tenant); tenants whose Ids cluster into a few ranges see less benefit. **Default is + unchanged** (`$skip`); existing callers are unaffected. New + `FamisClient.getAllBatchKeyset` + `QueryContext.buildKeysetUrl`/`buildKeysetBoundUrl`. + ## [1.3.0] — 2026-07-13 ### Fixed - `getAllBatch` / `getAllAssetsBatch` no longer crash with `Cannot read properties of diff --git a/docs/superpowers/plans/2026-07-17-assets-keyset-parallel-paging.md b/docs/superpowers/plans/2026-07-17-assets-keyset-parallel-paging.md new file mode 100644 index 0000000..8bc0c1e --- /dev/null +++ b/docs/superpowers/plans/2026-07-17-assets-keyset-parallel-paging.md @@ -0,0 +1,432 @@ +# Assets keyset parallel paging — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add an opt-in keyset (Id-range-partitioned) parallel paging mode to the SDK's assets batch fetch, ~2–2.6× faster than the current `$skip` OFFSET paging on large tenants. + +**Architecture:** New `QueryContext.buildKeysetUrl` / `buildKeysetBoundUrl` helpers + a new `FamisClient.getAllBatchKeyset` that fetches `min(Id)`/`max(Id)`, splits the Id space into K half-open ranges, and pages each range with `Id gt and Id le ` under the **same 4-way** Bottleneck pool. `getAllAssetsBatch` gains an optional `{ paging }` arg (default `'skip'` = today's behavior). + +**Tech Stack:** TypeScript, axios 0.21, axios-retry, Bottleneck, jest. Source at repo root (`famis_client.ts`, `model/request_context.ts`); tests in `tests/`. `npm test` = jest. + +## Global Constraints + +- **Shared SDK — purely additive / opt-in.** `facility360` is consumed by the connector AND facility360 mobile + demoio integrations/assistants. Existing `getAllBatch` and `getAllAssetsBatch(context, callback)` behavior must be **byte-identical** for current callers. Keyset is reached ONLY via the new optional `paging: 'keyset'` arg; default is `'skip'`. +- **No concurrency increase.** The keyset worker pool uses `new Bottleneck({ maxConcurrent: 4 })` — identical to `getAllBatch`. K (partition count) is granularity for load-balancing, NOT concurrency; ≤4 requests in flight at any time. +- **Correctness:** every keyset page sets `$orderby=Id` (the cursor assumes each page's last row is the current max Id); ranges are half-open `Id gt lo and Id le hi`; `Id` is the entity PK (unique) so no drop/dup. +- **Endpoint requirement:** only needs `Id gt` in `$filter` — live on the FAMIS `assets` endpoint today (verified 2026-07-17). Reference endpoints are NOT keyset yet; out of scope. +- **Node >= 20.** Bump SDK version to **1.4.0** (minor: additive feature). + +--- + +## File Structure + +- **Modify** `model/request_context.ts` — add `buildKeysetUrl(entity, top, gt, le)` and `buildKeysetBoundUrl(entity, desc)`. +- **Modify** `famis_client.ts` — add `KEYSET_PARTITIONS` const, `getAllBatchKeyset(context, type, callback)`, and an optional `opts` arg on `getAllAssetsBatch`. +- **Create** `tests/keyset_paging.test.ts` — URL-builder unit tests + a synthetic-dataset adapter test proving row parity, concurrency cap, and default-skip behavior. +- **Modify** `package.json` — version 1.3.0 → 1.4.0. +- **Follow-on (connector repo `facility360_r7`, separate)** — Task 6 notes; not built here. + +--- + +### Task 1: QueryContext keyset URL builders + +**Files:** +- Modify: `model/request_context.ts` (add two methods near `buildPagedUrl`, ~line 75) +- Test: `tests/keyset_paging.test.ts` + +**Interfaces:** +- Produces: + - `buildKeysetUrl(entity: string, top: number, gt: number, le: number): string` + - `buildKeysetBoundUrl(entity: string, desc: boolean): string` +- Consumes: existing private `basePath`, `this.filter`, `this.select`, `this.expand`. + +- [ ] **Step 1: Write the failing tests** + +```typescript +// tests/keyset_paging.test.ts +import { QueryContext } from '../model/request_context'; + +function params(url: string) { + return new URL(`http://example.com/${url}`).searchParams; +} + +describe('QueryContext.buildKeysetUrl', () => { + it('forces $orderby=Id and appends the half-open Id bounds to the base filter', () => { + const ctx = new QueryContext() + .setSelect('Id,Name') + .setExpand('Space') + .setFilter('ActiveFlag eq true'); + const p = params(ctx.buildKeysetUrl('assets', 1000, 100, 200)); + expect(p.get('$top')).toEqual('1000'); + expect(p.get('$orderby')).toEqual('Id'); + expect(p.get('$select')).toEqual('Id,Name'); + expect(p.get('$expand')).toEqual('Space'); + expect(p.get('$filter')).toEqual('ActiveFlag eq true and Id gt 100 and Id le 200'); + expect(p.get('$skip')).toBeNull(); + }); + + it('uses only the Id bounds when no base filter is set', () => { + const p = params(new QueryContext().buildKeysetUrl('assets', 500, 0, 50)); + expect(p.get('$filter')).toEqual('Id gt 0 and Id le 50'); + }); +}); + +describe('QueryContext.buildKeysetBoundUrl', () => { + it('builds a 1-row Id probe (asc = min, desc = max) carrying the base filter', () => { + const ctx = new QueryContext().setFilter('ActiveFlag eq true'); + const min = params(ctx.buildKeysetBoundUrl('assets', false)); + expect(min.get('$top')).toEqual('1'); + expect(min.get('$orderby')).toEqual('Id'); + expect(min.get('$select')).toEqual('Id'); + expect(min.get('$filter')).toEqual('ActiveFlag eq true'); + const max = params(ctx.buildKeysetBoundUrl('assets', true)); + expect(max.get('$orderby')).toEqual('Id desc'); + }); +}); +``` + +- [ ] **Step 2: Run to verify they fail** + +Run: `npx jest tests/keyset_paging.test.ts -t buildKeyset` +Expected: FAIL — `buildKeysetUrl is not a function`. + +- [ ] **Step 3: Implement the two methods** + +Add to `model/request_context.ts` inside the `QueryContext` class (after `buildPagedUrl`). `basePath` is the module const already used by `buildPagedUrl`. + +```typescript + /** + * Keyset page URL: forces $orderby=Id and appends half-open Id bounds to the base + * filter (Id gt and Id le ). No $skip. See docs/.../assets-keyset-parallel-paging. + */ + buildKeysetUrl(entity: string, top: number, gt: number, le: number) { + const bounds = `Id gt ${gt} and Id le ${le}`; + const filter = this.filter ? `${this.filter} and ${bounds}` : bounds; + let urlPath = `${basePath}/${entity}?$top=${top}&$orderby=Id&$filter=${filter}`; + if (this.expand) urlPath += `&$expand=${this.expand}`; + if (this.select) urlPath += `&$select=${this.select}`; + return urlPath; + } + + /** One-row Id probe to find min (desc=false) / max (desc=true), carrying the base filter. */ + buildKeysetBoundUrl(entity: string, desc: boolean) { + let urlPath = `${basePath}/${entity}?$top=1&$orderby=Id${desc ? ' desc' : ''}&$select=Id`; + if (this.filter) urlPath += `&$filter=${this.filter}`; + return urlPath; + } +``` + +- [ ] **Step 4: Run to verify they pass** + +Run: `npx jest tests/keyset_paging.test.ts -t buildKeyset` +Expected: PASS (4 tests). + +- [ ] **Step 5: Commit** + +```bash +git add model/request_context.ts tests/keyset_paging.test.ts +git commit -m "feat(keyset): QueryContext.buildKeysetUrl + buildKeysetBoundUrl" +``` + +--- + +### Task 2: FamisClient.getAllBatchKeyset + +**Files:** +- Modify: `famis_client.ts` (add `KEYSET_PARTITIONS` const near the other module consts; add `getAllBatchKeyset` near `getAllBatch`, ~line 1883) +- Test: `tests/keyset_paging.test.ts` + +**Interfaces:** +- Consumes: `QueryContext.buildKeysetUrl`/`buildKeysetBoundUrl` (Task 1); existing `this.http`, `this.requestConfig(context)`, `this.throwResponseError`, `Bottleneck`. +- Produces: `getAllBatchKeyset(context: QueryContext, type: string, callback: ResultCallback): Promise` + +- [ ] **Step 1: Write the failing test** (synthetic dataset served by a keyset-aware stub adapter) + +```typescript +// append to tests/keyset_paging.test.ts +import { FamisClient } from '../famis_client'; + +// Adapter that answers keyset + bound probes from an in-memory Id list. +function installKeysetAdapter(client: FamisClient, ids: number[], pageSize = 1000) { + const sorted = [...ids].sort((a, b) => a - b); + let maxInFlight = 0, inFlight = 0; + (client.http.defaults as any).adapter = async (config: any) => { + inFlight++; maxInFlight = Math.max(maxInFlight, inFlight); + await new Promise((r) => setTimeout(r, 5)); // force overlap so the cap is observable + const sp = new URL(`http://h/${config.url}`).searchParams; + const top = Number(sp.get('$top')); + const filter = sp.get('$filter') ?? ''; + const desc = (sp.get('$orderby') ?? '') === 'Id desc'; + let rows: { Id: number }[]; + if (top === 1) { + rows = [{ Id: desc ? sorted[sorted.length - 1] : sorted[0] }]; + } else { + const gt = Number(/Id gt (\d+)/.exec(filter)?.[1]); + const le = Number(/Id le (\d+)/.exec(filter)?.[1]); + rows = sorted.filter((id) => id > gt && id <= le).slice(0, top).map((Id) => ({ Id })); + } + inFlight--; + return { data: { value: rows }, status: 200, statusText: 'OK', headers: {}, config, request: {} }; + }; + return { getMaxInFlight: () => maxInFlight }; +} + +describe('FamisClient.getAllBatchKeyset', () => { + const HOST = 'https://h.example.com'; + const make = () => FamisClient.withAccessToken({ token: 't', host: HOST }); + + it('delivers every row exactly once across ranges (parity), with sparse gaps', async () => { + const client = make(); + const ids = [1, 2, 3, 50, 51, 900, 901, 5000, 9999]; // gaps + clustering + installKeysetAdapter(client, ids, 2); // tiny page size → forces multi-page ranges + const got: number[] = []; + await client.getAllBatchKeyset( + new QueryContext().setSelect('Id'), + 'assets', + (resp) => got.push(...resp.value.map((r: any) => r.Id)), + ); + expect(got.sort((a, b) => a - b)).toEqual(ids); + }); + + it('never exceeds 4 concurrent requests', async () => { + const client = make(); + const ids = Array.from({ length: 2000 }, (_, i) => i + 1); + const { getMaxInFlight } = installKeysetAdapter(client, ids, 100); + await client.getAllBatchKeyset(new QueryContext().setSelect('Id'), 'assets', () => {}); + expect(getMaxInFlight()).toBeLessThanOrEqual(4); + }); + + it('returns without calling back on an empty set', async () => { + const client = make(); + installKeysetAdapter(client, [], 1000); + const cb = jest.fn(); + await client.getAllBatchKeyset(new QueryContext().setSelect('Id'), 'assets', cb); + expect(cb).not.toHaveBeenCalled(); + }); +}); +``` + +- [ ] **Step 2: Run to verify it fails** + +Run: `npx jest tests/keyset_paging.test.ts -t getAllBatchKeyset` +Expected: FAIL — `getAllBatchKeyset is not a function`. + +- [ ] **Step 3: Implement `KEYSET_PARTITIONS` + `getAllBatchKeyset`** + +Add the const near the other top-level consts in `famis_client.ts` (e.g. beside `DEFAULT_REQUEST_TIMEOUT_MS`): + +```typescript +// Keyset partition count. Granularity for load-balancing an uneven Id distribution across +// the worker pool — NOT concurrency (pool stays maxConcurrent:4). More ranges than workers +// so a worker that drains a sparse range picks up another instead of idling. +export const KEYSET_PARTITIONS = 16; +``` + +Add the method in the `FamisClient` class next to `getAllBatch`: + +```typescript + /** + * Keyset (Id-range-partitioned) parallel batch fetch. Opt-in alternative to getAllBatch's + * $count+$skip: flat page latency at depth (no OFFSET). Same 4-way concurrency. Requires + * `Id gt` support in $filter and an ordered Id PK. See the plan doc. + */ + async getAllBatchKeyset( + context: QueryContext, + type: string, + callback: ResultCallback, + ): Promise { + const minResp = await this.http.get(context.buildKeysetBoundUrl(type, false), this.requestConfig(context)); + this.throwResponseError(minResp); + const minRows = (minResp.data as FamisResponse).value; + if (minRows.length === 0) return; + const min = minRows[0].Id; + + const maxResp = await this.http.get(context.buildKeysetBoundUrl(type, true), this.requestConfig(context)); + this.throwResponseError(maxResp); + const max = (maxResp.data as FamisResponse).value[0].Id; + + const top = 1000; + const width = Math.ceil((max - min + 1) / KEYSET_PARTITIONS); + const ranges: { lo: number; hi: number }[] = []; + for (let i = 0; i < KEYSET_PARTITIONS; i++) { + const lo = min - 1 + i * width; + const hi = i === KEYSET_PARTITIONS - 1 ? max : min - 1 + (i + 1) * width; + if (lo < hi) ranges.push({ lo, hi }); + } + + const limiter = new Bottleneck({ maxConcurrent: 4 }); + const tasks = ranges.map((r) => + limiter.schedule(async () => { + let cursor = r.lo; + // sequential keyset within this range; the pool bounds concurrency across ranges + while (true) { + const url = context.buildKeysetUrl(type, top, cursor, r.hi); + const resp = await this.http.get(url, this.requestConfig(context)); + this.throwResponseError(resp); + const page = resp.data as FamisResponse; + callback(page); + const rows = page.value; + if (rows.length < top) break; + cursor = rows[rows.length - 1].Id; + } + }), + ); + await Promise.all(tasks); + } +``` + +- [ ] **Step 4: Run to verify it passes** + +Run: `npx jest tests/keyset_paging.test.ts -t getAllBatchKeyset` +Expected: PASS (3 tests) — parity, concurrency ≤4, empty-set. + +- [ ] **Step 5: Commit** + +```bash +git add famis_client.ts tests/keyset_paging.test.ts +git commit -m "feat(keyset): getAllBatchKeyset — Id-range-partitioned 4-way parallel paging" +``` + +--- + +### Task 3: Route `getAllAssetsBatch` via an opt-in `paging` arg + +**Files:** +- Modify: `famis_client.ts` (`getAllAssetsBatch`, ~line 680) +- Test: `tests/keyset_paging.test.ts` + +**Interfaces:** +- Produces: `getAllAssetsBatch(context, callback, opts?: { paging?: 'keyset' | 'skip' }): Promise` — default `'skip'` (unchanged behavior). +- Consumes: `getAllBatch` (existing), `getAllBatchKeyset` (Task 2). + +- [ ] **Step 1: Write the failing test** (default = skip path; opt-in = keyset path — distinguished by URL shape) + +```typescript +// append to tests/keyset_paging.test.ts +describe('getAllAssetsBatch paging opt-in', () => { + const HOST = 'https://h.example.com'; + function recordUrls(client: FamisClient, count = 0) { + const urls: string[] = []; + (client.http.defaults as any).adapter = async (config: any) => { + urls.push(config.url); + const sp = new URL(`http://h/${config.url}`).searchParams; + const rows = Number(sp.get('$top')) === 1 ? [{ Id: 1 }] : []; + return { data: { value: rows, '@odata.count': count }, status: 200, statusText: 'OK', headers: {}, config, request: {} }; + }; + return urls; + } + + it('defaults to the $skip path (unchanged behavior)', async () => { + const client = FamisClient.withAccessToken({ token: 't', host: HOST }); + const urls = recordUrls(client, 0); + await client.getAllAssetsBatch(new QueryContext().setSelect('Id'), () => {}); + expect(urls[0]).toContain('$skip=0'); + expect(urls.join()).not.toContain('Id gt'); + }); + + it('uses keyset when paging: "keyset"', async () => { + const client = FamisClient.withAccessToken({ token: 't', host: HOST }); + const urls = recordUrls(client, 0); + await client.getAllAssetsBatch(new QueryContext().setSelect('Id'), () => {}, { paging: 'keyset' }); + expect(urls.some((u) => u.includes('$orderby=Id') && u.includes('$top=1'))).toBe(true); // bound probe + expect(urls.join()).not.toContain('$skip='); + }); +}); +``` + +- [ ] **Step 2: Run to verify it fails** + +Run: `npx jest tests/keyset_paging.test.ts -t "paging opt-in"` +Expected: FAIL — keyset test fails (getAllAssetsBatch ignores opts / still uses `$skip`). + +- [ ] **Step 3: Implement the routing** + +Replace `getAllAssetsBatch` in `famis_client.ts`: + +```typescript + async getAllAssetsBatch( + context: QueryContext, + callback: ResultCallback, + opts?: { paging?: 'keyset' | 'skip' }, + ): Promise { + if (opts?.paging === 'keyset') { + return this.getAllBatchKeyset(context, 'assets', callback); + } + return this.getAllBatch(context, 'assets', callback); + } +``` + +- [ ] **Step 4: Run to verify it passes** + +Run: `npx jest tests/keyset_paging.test.ts -t "paging opt-in"` +Expected: PASS (2 tests). + +- [ ] **Step 5: Run the whole suite (no regressions)** + +Run: `npm test` +Expected: all suites pass (existing `getAllBatch`/timeout/reauth tests unchanged). + +- [ ] **Step 6: Commit** + +```bash +git add famis_client.ts tests/keyset_paging.test.ts +git commit -m "feat(keyset): opt-in paging on getAllAssetsBatch (default skip, unchanged)" +``` + +--- + +### Task 4: Version bump + build + +**Files:** +- Modify: `package.json` (version) + +- [ ] **Step 1: Bump version 1.3.0 → 1.4.0** + +Edit `package.json`: `"version": "1.4.0"`. + +- [ ] **Step 2: Build (typecheck + emit dist)** + +Run: `npm run build` +Expected: `tsc` exits 0, `dist/` updated (new methods present in `dist/famis_client.d.ts`). + +- [ ] **Step 3: Full test suite** + +Run: `npm test` +Expected: green. + +- [ ] **Step 4: Commit** + +```bash +git add package.json dist +git commit -m "chore: v1.4.0 — opt-in assets keyset paging" +``` + +> Publish to npm (`npm publish`) is a release step done with Matt/Alexis once reviewed — not part of this plan. + +--- + +### Task 5: Stage integration verification (manual, not in `npm test`) + +**Goal:** Confirm row-count parity + speedup against the current path on a large real tenant, on stage (no prod load). + +- [ ] **Step 1:** Against `st-udst.famis360.com` (→ `udst.stage.famis360.com`, ~209k active assets, keyset-enabled), fetch all assets both ways with the full select+expand and assert equal row counts and lower keyset wall-clock. (Reference bench: `$skip` 632.7s vs keyset 241.6s = 2.62×, 209,126 rows both.) A throwaway script using `getAllAssetsBatch(ctx, cb)` vs `getAllAssetsBatch(ctx, cb, { paging: 'keyset' })` suffices; do NOT commit it. +- [ ] **Step 2:** Spot-check parity on a small tenant (row counts equal between the two modes). + +--- + +### Task 6: Connector integration (follow-on — repo `facility360_r7`, NOT this repo) + +> Separate change, gated on this SDK being published + reviewed with Matt/Alexis. Listed for continuity; build it as its own small plan in the connector repo. + +- Bump `facility360` dependency to `^1.4.0` (package.json + yarn.lock). +- Add `ServerConfiguration.assetsKeysetEnabled` getter → `process.env.CACHE_ASSETS_KEYSET === 'true'` (default off), mirroring the `cacheAutoRetry` pattern. +- In the assets cache loader (`src/services/cache.loaders/assets.loader.ts`, the `getAllAssetsBatch` call), pass `{ paging: cfg.assetsKeysetEnabled ? 'keyset' : 'skip' }`. +- Validate on `famis360-dev` (set `CACHE_ASSETS_KEYSET=true`, run the assets loader, confirm row parity vs a `$skip` run + faster), then staging/main, then flip the default on after a clean nightly. + +--- + +## Self-Review + +- **Spec coverage:** buildKeysetUrl/bound (Task 1) ✔; range-partitioned parallel keyset with min/max + worker pool (Task 2) ✔; opt-in default-skip (Task 3, Global Constraints) ✔; no-concurrency-increase (Bottleneck maxConcurrent:4, Task 2) ✔; $orderby=Id + half-open ranges (Task 1/2) ✔; shared-SDK additive (Task 3) ✔; version bump (Task 4) ✔; stage parity/speedup verification (Task 5) ✔; connector rollout/opt-in toggle (Task 6) ✔. Non-goals (reference endpoints, nextLink-follow, live path) correctly excluded. +- **Placeholders:** none — every code/test step has concrete code and exact commands. +- **Type consistency:** `getAllBatchKeyset`, `buildKeysetUrl(entity,top,gt,le)`, `buildKeysetBoundUrl(entity,desc)`, `getAllAssetsBatch(context,callback,opts?)`, `KEYSET_PARTITIONS` used consistently across tasks. diff --git a/famis_client.ts b/famis_client.ts index 0c1b175..7404fcd 100644 --- a/famis_client.ts +++ b/famis_client.ts @@ -198,6 +198,13 @@ export const DefaultSpaceSelect = ['Id', 'Name', 'LongDescription']; */ export const DEFAULT_REQUEST_TIMEOUT_MS = 120_000; +// Keyset partition count. Splits the Id SPACE (not the row count) into more ranges than +// workers, so the pool (maxConcurrent:4, unchanged — this is NOT concurrency) stays busy +// as workers drain ranges. Note the speedup depends on Id distribution: rows spread across +// ranges parallelize well, but a tenant whose Ids cluster into one/two ranges pages those +// mostly sequentially and gains little over $skip. NOT concurrency. +export const KEYSET_PARTITIONS = 16; + /** * Default timeout (ms) for the login/token request specifically — always a small, quick * call, so a much tighter bound is safe. A hung login is never legitimate. @@ -677,7 +684,14 @@ export class FamisClient { return this.getAll(context, 'schedules'); } - async getAllAssetsBatch(context: QueryContext, callback: ResultCallback): Promise { + async getAllAssetsBatch( + context: QueryContext, + callback: ResultCallback, + opts?: { paging?: 'keyset' | 'skip' }, + ): Promise { + if (opts?.paging === 'keyset') { + return this.getAllBatchKeyset(context, 'assets', callback); + } return this.getAllBatch(context, 'assets', callback); } @@ -1933,6 +1947,60 @@ export class FamisClient { await Promise.all(promises); } + /** + * Keyset (Id-range-partitioned) parallel batch fetch. Opt-in alternative to getAllBatch's + * $count+$skip: flat page latency at depth (no OFFSET). Same 4-way concurrency. Requires + * `Id gt` support in $filter and an ordered Id PK. See the plan doc. + */ + async getAllBatchKeyset( + context: QueryContext, + type: string, + callback: ResultCallback, + ): Promise { + const minResp = await this.http.get(context.buildKeysetBoundUrl(type, false), this.requestConfig(context)); + this.throwResponseError(minResp); + const minRows = (minResp.data as FamisResponse).value; + if (minRows.length === 0) return; + const min = minRows[0].Id; + + const maxResp = await this.http.get(context.buildKeysetBoundUrl(type, true), this.requestConfig(context)); + this.throwResponseError(maxResp); + const maxRows = (maxResp.data as FamisResponse).value; + if (maxRows.length === 0) return; + const max = maxRows[0].Id; + + const top = 1000; + const width = Math.ceil((max - min + 1) / KEYSET_PARTITIONS); + const ranges: { lo: number; hi: number }[] = []; + for (let i = 0; i < KEYSET_PARTITIONS; i++) { + const lo = min - 1 + i * width; + const hi = i === KEYSET_PARTITIONS - 1 ? max : min - 1 + (i + 1) * width; + // Skip empty ranges: lo= max). + if (lo < hi && lo < max) ranges.push({ lo, hi }); + } + + const limiter = new Bottleneck({ maxConcurrent: 4 }); + const tasks = ranges.map((r) => + limiter.schedule(async () => { + let cursor = r.lo; + // sequential keyset within this range; the pool bounds concurrency across ranges + while (true) { + const url = context.buildKeysetUrl(type, top, cursor, r.hi); + const resp = await this.http.get(url, this.requestConfig(context)); + this.throwResponseError(resp); + const page = resp.data as FamisResponse; + callback(page); + const rows = page.value; + if (rows.length < top) break; + cursor = rows[rows.length - 1].Id; + } + }), + ); + await Promise.all(tasks); + } + async getAllPaged(context: QueryContext, type: string): Promise> { let fetchAll = true; let top = 1000; diff --git a/model/request_context.ts b/model/request_context.ts index 5ab1527..c775041 100644 --- a/model/request_context.ts +++ b/model/request_context.ts @@ -92,6 +92,29 @@ export class QueryContext { return urlPath; } + /** + * Keyset page URL: forces $orderby=Id and appends half-open Id bounds to the base + * filter (Id gt and Id le ). No $skip. See docs/.../assets-keyset-parallel-paging. + */ + buildKeysetUrl(entity: string, top: number, gt: number, le: number) { + const bounds = `Id gt ${gt} and Id le ${le}`; + // Parenthesize the base filter: OData `and` binds tighter than `or`, so a base + // filter like `A or B` must be grouped or the Id bounds would attach only to `B` + // (`A or (B and Id gt..)`), letting `A` rows escape the range → duplication. + const filter = this.filter ? `(${this.filter}) and ${bounds}` : bounds; + let urlPath = `${basePath}/${entity}?$top=${top}&$orderby=Id&$filter=${filter}`; + if (this.expand) urlPath += `&$expand=${this.expand}`; + if (this.select) urlPath += `&$select=${this.select}`; + return urlPath; + } + + /** One-row Id probe to find min (desc=false) / max (desc=true), carrying the base filter. */ + buildKeysetBoundUrl(entity: string, desc: boolean) { + let urlPath = `${basePath}/${entity}?$top=1&$orderby=Id${desc ? ' desc' : ''}&$select=Id`; + if (this.filter) urlPath += `&$filter=${this.filter}`; + return urlPath; + } + addFiltersToUrl(path: string) { let urlPath = path; let hasQuery = false; diff --git a/package.json b/package.json index 9747682..89ec2aa 100644 --- a/package.json +++ b/package.json @@ -1,12 +1,16 @@ { "name": "facility360", - "version": "1.3.0", + "version": "1.4.0", "description": "A Node based 360Facility client SDK", "main": "dist/index.js", + "files": [ + "dist" + ], "scripts": { "test": "jest --coverage --verbose --testPathIgnorePatterns \"integration.test.*\" --passWithNoTests", "build": "tsc", - "watch": "tsc -w" + "watch": "tsc -w", + "prepublishOnly": "npm run build" }, "types": "dist/index.d.ts", "repository": { diff --git a/tests/keyset_paging.test.ts b/tests/keyset_paging.test.ts new file mode 100644 index 0000000..006f7db --- /dev/null +++ b/tests/keyset_paging.test.ts @@ -0,0 +1,152 @@ +import { QueryContext } from '../model/request_context'; +import { FamisClient } from '../famis_client'; + +function params(url: string) { + return new URL(`http://example.com/${url}`).searchParams; +} + +describe('QueryContext.buildKeysetUrl', () => { + it('forces $orderby=Id and appends the half-open Id bounds to the base filter', () => { + const ctx = new QueryContext() + .setSelect('Id,Name') + .setExpand('Space') + .setFilter('ActiveFlag eq true'); + const p = params(ctx.buildKeysetUrl('assets', 1000, 100, 200)); + expect(p.get('$top')).toEqual('1000'); + expect(p.get('$orderby')).toEqual('Id'); + expect(p.get('$select')).toEqual('Id,Name'); + expect(p.get('$expand')).toEqual('Space'); + expect(p.get('$filter')).toEqual('(ActiveFlag eq true) and Id gt 100 and Id le 200'); + expect(p.get('$skip')).toBeNull(); + }); + + it('uses only the Id bounds when no base filter is set', () => { + const p = params(new QueryContext().buildKeysetUrl('assets', 500, 0, 50)); + expect(p.get('$filter')).toEqual('Id gt 0 and Id le 50'); + }); + + it('parenthesizes an `or` base filter so the Id bounds bind to the whole filter', () => { + // Without the parens this would be `A or B and Id gt.. and Id le..`, which OData + // parses as `A or (B and ..)` — rows matching A would ignore the Id range. + const ctx = new QueryContext().setFilter('PropertyId eq 1 or PropertyId eq 2'); + const p = params(ctx.buildKeysetUrl('assets', 1000, 100, 200)); + expect(p.get('$filter')).toEqual( + '(PropertyId eq 1 or PropertyId eq 2) and Id gt 100 and Id le 200', + ); + }); +}); + +describe('QueryContext.buildKeysetBoundUrl', () => { + it('builds a 1-row Id probe (asc = min, desc = max) carrying the base filter', () => { + const ctx = new QueryContext().setFilter('ActiveFlag eq true'); + const min = params(ctx.buildKeysetBoundUrl('assets', false)); + expect(min.get('$top')).toEqual('1'); + expect(min.get('$orderby')).toEqual('Id'); + expect(min.get('$select')).toEqual('Id'); + expect(min.get('$filter')).toEqual('ActiveFlag eq true'); + const max = params(ctx.buildKeysetBoundUrl('assets', true)); + expect(max.get('$orderby')).toEqual('Id desc'); + }); +}); + +// Adapter that answers keyset + bound probes from an in-memory Id list. Mirrors the real +// server contract: a page returns up to $top matching rows; getAllBatchKeyset's cursor loop +// (famis_client.ts) only continues within a range when a response comes back exactly $top long, +// so genuinely exercising that loop requires a partition that holds more than $top (1000) ids — +// not an artificial per-call cap unrelated to $top (which would just truncate results and drop +// data, since the client has no independent knowledge of such a cap). +function installKeysetAdapter(client: FamisClient, ids: number[]) { + const sorted = [...ids].sort((a, b) => a - b); + let maxInFlight = 0, inFlight = 0; + (client.http.defaults as any).adapter = async (config: any) => { + inFlight++; maxInFlight = Math.max(maxInFlight, inFlight); + await new Promise((r) => setTimeout(r, 5)); // force overlap so the cap is observable + const sp = new URL(`http://h/${config.url}`).searchParams; + const top = Number(sp.get('$top')); + const filter = sp.get('$filter') ?? ''; + const desc = (sp.get('$orderby') ?? '') === 'Id desc'; + let rows: { Id: number }[]; + if (top === 1) { + rows = [{ Id: desc ? sorted[sorted.length - 1] : sorted[0] }]; + } else { + const gt = Number(/Id gt (\d+)/.exec(filter)?.[1]); + const le = Number(/Id le (\d+)/.exec(filter)?.[1]); + rows = sorted.filter((id) => id > gt && id <= le).slice(0, top).map((Id) => ({ Id })); + } + inFlight--; + return { data: { value: rows }, status: 200, statusText: 'OK', headers: {}, config, request: {} }; + }; + return { getMaxInFlight: () => maxInFlight }; +} + +describe('FamisClient.getAllBatchKeyset', () => { + const HOST = 'https://h.example.com'; + const make = () => FamisClient.withAccessToken({ token: 't', host: HOST }); + + it('delivers every row exactly once across ranges (parity), with sparse gaps', async () => { + const client = make(); + const sparse = [1, 2, 3, 50, 51, 900, 901, 5000, 9999]; // gaps + clustering + // A dense cluster of 1500 consecutive ids, placed high enough (30001..31500) that it lands + // entirely inside a single keyset partition. Since $top is 1000, the first page for that + // partition fills exactly 1000 rows (a "full page"), which is the only condition under + // which getAllBatchKeyset's cursor loop continues (famis_client.ts: `if (rows.length < top) + // break`) — so this genuinely drives a 2-page fetch (1000 then 500) for that range, unlike + // the sparse ids alone which each resolve in a single under-$top page. + const dense = Array.from({ length: 1500 }, (_, i) => 30001 + i); + const ids = [...sparse, ...dense]; + installKeysetAdapter(client, ids); + const got: number[] = []; + await client.getAllBatchKeyset( + new QueryContext().setSelect('Id'), + 'assets', + (resp) => got.push(...resp.value.map((r: any) => r.Id)), + ); + expect(got.sort((a, b) => a - b)).toEqual([...ids].sort((a, b) => a - b)); + }); + + it('never exceeds 4 concurrent requests', async () => { + const client = make(); + const ids = Array.from({ length: 2000 }, (_, i) => i + 1); + const { getMaxInFlight } = installKeysetAdapter(client, ids); + await client.getAllBatchKeyset(new QueryContext().setSelect('Id'), 'assets', () => {}); + expect(getMaxInFlight()).toBeLessThanOrEqual(4); + }); + + it('returns without calling back on an empty set', async () => { + const client = make(); + installKeysetAdapter(client, []); + const cb = jest.fn(); + await client.getAllBatchKeyset(new QueryContext().setSelect('Id'), 'assets', cb); + expect(cb).not.toHaveBeenCalled(); + }); +}); + +describe('getAllAssetsBatch paging opt-in', () => { + const HOST = 'https://h.example.com'; + function recordUrls(client: FamisClient, count = 0) { + const urls: string[] = []; + (client.http.defaults as any).adapter = async (config: any) => { + urls.push(config.url); + const sp = new URL(`http://h/${config.url}`).searchParams; + const rows = Number(sp.get('$top')) === 1 ? [{ Id: 1 }] : []; + return { data: { value: rows, '@odata.count': count }, status: 200, statusText: 'OK', headers: {}, config, request: {} }; + }; + return urls; + } + + it('defaults to the $skip path (unchanged behavior)', async () => { + const client = FamisClient.withAccessToken({ token: 't', host: HOST }); + const urls = recordUrls(client, 0); + await client.getAllAssetsBatch(new QueryContext().setSelect('Id'), () => {}); + expect(urls[0]).toContain('$skip=0'); + expect(urls.join()).not.toContain('Id gt'); + }); + + it('uses keyset when paging: "keyset"', async () => { + const client = FamisClient.withAccessToken({ token: 't', host: HOST }); + const urls = recordUrls(client, 0); + await client.getAllAssetsBatch(new QueryContext().setSelect('Id'), () => {}, { paging: 'keyset' }); + expect(urls.some((u) => u.includes('$orderby=Id') && u.includes('$top=1'))).toBe(true); // bound probe + expect(urls.join()).not.toContain('$skip='); + }); +});