From bc1facd3f65b2fcbf5fa58efeb877de632aec3f6 Mon Sep 17 00:00:00 2001 From: YANG QIA <2013xile@gmail.com> Date: Fri, 16 Oct 2026 06:22:55 +0800 Subject: [PATCH] perf(projects): read the issue board in fixed queries and keep it live in studio (#10932) * perf(projects): read the issue board in a fixed number of queries Read each column's ids in its order, then every column's rows at once; let the issue list take issueIds; and keep the board and the trash on Studio's pages current through the app-wide refresh, reading only the rows a change names. * docs(studio): regenerate the CLI reference for issueIds * chore(ci): rerun checks --- .changeset/issue-pages-fixed-reads.md | 6 + .../apps/studio/client/issues/groups/api.ts | 23 ++- .../apps/studio/client/issues/groups/live.ts | 163 ++++++++++++++++- .../apps/studio/docs/cli/reference/issue.md | 29 +-- .../cli/__snapshots__/manifest.test.ts.snap | 4 +- .../tests/components/issue-board-live.test.ts | 172 ++++++++++++++++++ .../app-plugin-projects/client/api/client.ts | 3 +- .../server/domains/issues/issue.queries.ts | 131 ++++++++++--- .../server/domains/issues/issue.store.ts | 25 +++ .../server/routes/schemas.ts | 21 +++ .../app-plugin-projects/shared/issues.ts | 8 + .../tests/domains/list-reads.test.ts | 125 +++++++++++++ .../app-plugin-projects/tests/routes.test.ts | 22 +++ 13 files changed, 678 insertions(+), 54 deletions(-) create mode 100644 .changeset/issue-pages-fixed-reads.md create mode 100644 packages/apps/studio/tests/components/issue-board-live.test.ts create mode 100644 packages/plugins/app-plugin-projects/tests/domains/list-reads.test.ts diff --git a/.changeset/issue-pages-fixed-reads.md b/.changeset/issue-pages-fixed-reads.md new file mode 100644 index 00000000000..66c6e5ba4f8 --- /dev/null +++ b/.changeset/issue-pages-fixed-reads.md @@ -0,0 +1,6 @@ +--- +'@nocobase/app-plugin-projects': patch +'@nocobase/studio': patch +--- + +The issue board reads a fixed number of queries whatever the number of issues: each column's ids are read in its order, then every column's rows and what they show beside them at once (16 queries instead of about 54). `GET /api/projects/issues` takes `issueIds` (comma-separated, at most 100) to read only some issues within the other filters. On Studio's issue pages the board and the trash now stay current through the app-wide refresh: a change naming issues reads only the rows shown, and the list is read again when one of them left it or changed column, or for a change naming no issues. diff --git a/packages/apps/studio/client/issues/groups/api.ts b/packages/apps/studio/client/issues/groups/api.ts index b053950de1e..1ebda19cb77 100644 --- a/packages/apps/studio/client/issues/groups/api.ts +++ b/packages/apps/studio/client/issues/groups/api.ts @@ -12,6 +12,8 @@ import { type ApiClient, } from '@nocobase/app-client'; import type { IssueFilters } from '@nocobase/app-plugin-projects/client/issues'; +import { usePmApi } from '@nocobase/app-plugin-projects/client/kit'; +import { ISSUE_IDS_MAX } from '@nocobase/app-plugin-projects/shared/issues'; import { useQueries, useQuery, @@ -67,14 +69,29 @@ export function isChanged(error: unknown): boolean { ); } -/** Keeps the grouped task reads current from announcements; mounted once, in the app's layout. */ +/** + * Keeps the grouped task reads, and the projects plugin's board and issue pages on Studio's pages, current from + * announcements; mounted once, in the app's layout. + */ export function useTaskGroupsLive(): void { const client = useQueryClient(); const realtime = useService(realtimeClientToken); const api = useApiClient(); + const pm = usePmApi(); useEffect( - () => subscribeTaskGroupsLive(realtime, client, readTaskGroupRows(api)), - [client, realtime, api], + () => + subscribeTaskGroupsLive(realtime, client, readTaskGroupRows(api), { + issueRows: async (query, ids) => + ( + await pm.issuePage({ + ...query, + cursor: undefined, + issueIds: ids, + limit: ISSUE_IDS_MAX, + }) + ).data, + }), + [client, realtime, api, pm], ); } diff --git a/packages/apps/studio/client/issues/groups/live.ts b/packages/apps/studio/client/issues/groups/live.ts index fae09c47097..a9296b5038d 100644 --- a/packages/apps/studio/client/issues/groups/live.ts +++ b/packages/apps/studio/client/issues/groups/live.ts @@ -6,6 +6,11 @@ * (`GET /api/taskGroups/rows`) and merges them in; a list whose membership moved meanwhile is read whole instead; * - one that names nothing, the inbox's, the plans', and a reconnect read every shown list whole, and the summary too. * + * The projects plugin's flat lists on Studio's pages, the board (`pmKeys.board`) and the issue pages (`pmKeys.issuePages`, + * the trash), are kept current the same way: a named change reads those rows alone through `GET /api/projects/issues` + * with `issueIds` and the list's own filters, and the list is read whole when a shown row left it or changed column, + * or for an announcement naming no issues. + * * One refresh runs at a time and never cancels a read under way: a burst during a refresh is handled after it. Nothing * is read while the page is hidden; what was missed is read when it is shown again. Lists not shown are only marked * stale. @@ -16,7 +21,18 @@ import { PM_REALTIME_TOPIC, type PmChangeEvent, } from '@nocobase/app-plugin-projects/shared/realtime'; -import type { QueryClient, QueryKey } from '@tanstack/react-query'; +import type { Page } from '@nocobase/app-plugin-projects/shared/common'; +import { + ISSUE_IDS_MAX, + type IssueBoard, + type IssueListItem, + type IssueListQuery, +} from '@nocobase/app-plugin-projects/shared/issues'; +import type { + InfiniteData, + QueryClient, + QueryKey, +} from '@tanstack/react-query'; import { STUDIO_INBOX_TOPIC } from '../../../shared/inbox.js'; import { @@ -66,7 +82,22 @@ export function readTaskGroupRows( ).data; } +/** Reads the rows of `ids` a flat issue list holds now (`GET /api/projects/issues?issueIds=`), within its filters. */ +export type IssueRowsReader = ( + query: IssueListQuery, + ids: readonly string[], +) => Promise; + +/** The projects plugin's flat lists this keeps current: `['pm', 'issues', 'board' | 'pages', query]`. */ +const PM_LISTS = ['board', 'pages'] as const; +const isPmList = (key: QueryKey): boolean => + key[0] === 'pm' && + key[1] === 'issues' && + (PM_LISTS as readonly unknown[]).includes(key[2]); + export interface TaskGroupsLiveOptions { + /** Reads rows of the projects plugin's flat lists; without it they are read whole for every named change. */ + readonly issueRows?: IssueRowsReader; readonly delay?: number; /** Whether the page is shown; read nothing while it is not. */ readonly visible?: () => boolean; @@ -110,6 +141,8 @@ export function subscribeTaskGroupsLive( const visible = options.visible ?? documentVisible; const named = new Set(); let whole = false; + // The flat issue lists are read whole only for announcements about issues or labels, never for the inbox's. + let wholeIssues = false; let timer: ReturnType | undefined; let running = false; let stopped = false; @@ -128,6 +161,109 @@ export function subscribeTaskGroupsLive( ]); } + const readPmListsAgain = () => + client.invalidateQueries( + { + queryKey: ['pm', 'issues'], + predicate: (query) => isPmList(query.queryKey), + refetchType: 'active', + }, + { cancelRefetch: false }, + ); + + /** The issues a flat list shows, each with the column it is in (the board) or null (pages). */ + function shownIn(key: QueryKey, data: unknown): Map { + const shown = new Map(); + if (key[2] === 'board') + for (const column of (data as IssueBoard).columns) + for (const issue of column.issues) + shown.set(issue.id, column.status.key); + else + for (const page of (data as InfiniteData>).pages) + for (const issue of page.data) shown.set(issue.id, null); + return shown; + } + + function mergeInto( + key: QueryKey, + data: unknown, + rows: ReadonlyMap, + ): unknown { + if (key[2] === 'board') { + const board = data as IssueBoard; + return { + ...board, + columns: board.columns.map((column) => ({ + ...column, + issues: column.issues.map((issue) => rows.get(issue.id) ?? issue), + })), + }; + } + const pages = data as InfiniteData>; + return { + ...pages, + pages: pages.pages.map((page) => ({ + ...page, + data: page.data.map((issue) => rows.get(issue.id) ?? issue), + })), + }; + } + + /** The rows of `ids` in one flat list: merged, or the list read whole when one left it or changed column. */ + async function refreshPmList( + queryKey: QueryKey, + ids: ReadonlySet, + ): Promise { + const query = client.getQueryCache().find({ queryKey, exact: true }); + const data = query?.state.data; + if (!query || !data) return; + const shown = new Map( + [...shownIn(queryKey, data)].filter(([id]) => ids.has(id)), + ); + if (shown.size === 0) return; + if (query.getObserversCount() === 0) { + await client.invalidateQueries({ + queryKey, + exact: true, + refetchType: 'none', + }); + return; + } + const readRows = options.issueRows; + if (!readRows || shown.size > ISSUE_IDS_MAX) { + await readAgain(queryKey); + return; + } + let answer: readonly IssueListItem[]; + try { + answer = await readRows(queryKey[3] ?? {}, [...shown.keys()]); + } catch { + await readAgain(queryKey); + return; + } + const current = client + .getQueryCache() + .find({ queryKey, exact: true })?.state; + if ( + stopped || + !current?.data || + current.data !== data || + current.fetchStatus === 'fetching' + ) + return; + const rows = new Map(answer.map((issue) => [issue.id, issue])); + // Left the list's filters, or moved to another column: the list itself changed. + const moved = [...shown].some(([id, column]) => { + const row = rows.get(id); + return !row || (column !== null && row.statusKey !== column); + }); + if (moved) { + await readAgain(queryKey); + return; + } + client.setQueryData(queryKey, mergeInto(queryKey, data, rows)); + } + /** The rows of `ids` in one list: merged, or the list read whole when its membership moved. */ async function refreshRows( queryKey: QueryKey, @@ -211,7 +347,11 @@ export function subscribeTaskGroupsLive( if (stopped) return; if (!visible()) return; const listsFetching = - client.isFetching({ queryKey: taskGroupKeys.all }) > 0; + client.isFetching({ queryKey: taskGroupKeys.all }) > 0 || + client.isFetching({ + queryKey: ['pm', 'issues'], + predicate: (query) => isPmList(query.queryKey), + }) > 0; // A read under way may have started before the change: follow it rather than cancel it. if (listsFetching) { schedule(Math.max(50, Math.round(delay / 4))); @@ -221,6 +361,8 @@ export function subscribeTaskGroupsLive( named.clear(); const all = whole; whole = false; + const allIssues = wholeIssues; + wholeIssues = false; running = true; try { if (all) await refreshWhole(); @@ -229,10 +371,17 @@ export function subscribeTaskGroupsLive( .getQueryCache() .findAll({ queryKey: taskGroupKeys.lists })) await refreshRows(query.queryKey, ids); + if (allIssues) await readPmListsAgain(); + else if (ids.size > 0) + for (const query of client.getQueryCache().findAll({ + queryKey: ['pm', 'issues'], + predicate: (each) => isPmList(each.queryKey), + })) + await refreshPmList(query.queryKey, ids); } finally { running = false; } - if (named.size > 0 || whole) schedule(); + if (named.size > 0 || whole || wholeIssues) schedule(); } function schedule(wait: number = delay): void { @@ -247,6 +396,7 @@ export function subscribeTaskGroupsLive( const stops = [ realtime.onOpen(() => { whole = true; + wholeIssues = true; schedule(); }), realtime.subscribe(PM_REALTIME_TOPIC, ({ payload }) => { @@ -254,7 +404,10 @@ export function subscribeTaskGroupsLive( if (change?.kind !== 'pm.changed') return; const ids = change.domain === 'issues' ? announcedIssueIds(change) : null; if (ids) for (const id of ids) named.add(id); - else whole = true; + else { + whole = true; + if (change.domain !== 'plans') wholeIssues = true; + } schedule(); }), realtime.subscribe(STUDIO_INBOX_TOPIC, () => { @@ -262,7 +415,7 @@ export function subscribeTaskGroupsLive( schedule(); }), (options.onVisible ?? onDocumentVisible)(() => { - if (named.size > 0 || whole) schedule(0); + if (named.size > 0 || whole || wholeIssues) schedule(0); }), ]; return () => { diff --git a/packages/apps/studio/docs/cli/reference/issue.md b/packages/apps/studio/docs/cli/reference/issue.md index bd784bd86e0..c08efa7fd5b 100644 --- a/packages/apps/studio/docs/cli/reference/issue.md +++ b/packages/apps/studio/docs/cli/reference/issue.md @@ -468,20 +468,21 @@ The issues the caller sees, cursor-paged. `deleted=true` lists deleted issues in nb-studio issue search ``` -| Flag | Type | Description | -| -------------------- | ------- | ----------------------------------------------------------------------- | -| `--involves-user-id` | string | Owned by this user or executed by them as a person, each issue once. | -| `--status` | string | | -| `--project` | string | | -| `--owner` | string | | -| `--executor` | string | | -| `--label` | string | | -| `--parent` | string | A parent issue, by identifier (PM-12) or id; none for top-level issues. | -| `--q` | string | | -| `--deleted` | boolean | | -| `--order-by` | string | | -| `--limit` | integer | (default 20) | -| `--page-token` | string | | +| Flag | Type | Description | +| -------------------- | ------- | ----------------------------------------------------------------------------------- | +| `--issue-ids` | string | Only these issues, comma-separated ids, at most 100; the other filters still apply. | +| `--involves-user-id` | string | Owned by this user or executed by them as a person, each issue once. | +| `--status` | string | | +| `--project` | string | | +| `--owner` | string | | +| `--executor` | string | | +| `--label` | string | | +| `--parent` | string | A parent issue, by identifier (PM-12) or id; none for top-level issues. | +| `--q` | string | | +| `--deleted` | boolean | | +| `--order-by` | string | | +| `--limit` | integer | (default 20) | +| `--page-token` | string | | **Output:** A list (table columns: `identifier`, `title`, `statusKey`, `owner.name`, `executorName`); with `--json`, `result.data` is the array and `result.meta` the paging. diff --git a/packages/apps/studio/tests/cli/__snapshots__/manifest.test.ts.snap b/packages/apps/studio/tests/cli/__snapshots__/manifest.test.ts.snap index bc6eb2a1de1..7665d90e570 100644 --- a/packages/apps/studio/tests/cli/__snapshots__/manifest.test.ts.snap +++ b/packages/apps/studio/tests/cli/__snapshots__/manifest.test.ts.snap @@ -182,7 +182,7 @@ issue handoff reject -> POST /api/issueOwnerHandoffs/{issueId}/reject (p issue progress -> GET /api/issueProgress/{issueId} (person) issue restore -> POST /api/projects/issues/{issueId}/restore (person,run) pm.issues/delete issue runs [issue] [--limit --page-token] -> GET /api/issueRuns (person,run) pm.issues/view -issue search [--involves-user-id --status --project --owner --executor --label --parent --q --deleted --order-by --limit --page-token] -> GET /api/projects/issues (person,run) pm.issues/view +issue search [--issue-ids --involves-user-id --status --project --owner --executor --label --parent --q --deleted --order-by --limit --page-token] -> GET /api/projects/issues (person,run) pm.issues/view issue status list -> GET /api/projects/issues/statuses (person) issue subscribe -> POST /api/projects/issues/{issueId}/subscribe (person) pm.issues/view issue unsubscribe -> POST /api/projects/issues/{issueId}/unsubscribe (person) pm.issues/view @@ -405,7 +405,7 @@ issue dependency remove [--type] -> POST /api/projects/issue issue get -> GET /api/projects/issues/{issueId} (person,run) pm.issues/view issue restore -> POST /api/projects/issues/{issueId}/restore (person,run) pm.issues/delete issue runs [issue] [--limit --page-token] -> GET /api/issueRuns (person,run) pm.issues/view -issue search [--involves-user-id --status --project --owner --executor --label --parent --q --deleted --order-by --limit --page-token] -> GET /api/projects/issues (person,run) pm.issues/view +issue search [--issue-ids --involves-user-id --status --project --owner --executor --label --parent --q --deleted --order-by --limit --page-token] -> GET /api/projects/issues (person,run) pm.issues/view issue update [--needs-you --next --title --description --status --priority --owner --executor --parent --stage --project --start-date --due-date --label --start --file] -> PATCH /api/projects/issues/{issueId} (person,run) pm.issues/edit kb download [--version --project] -> GET /api/kb/docs/{doc}/file (person,run) kb.knowledge/read kb list [--q --parent --project] -> GET /api/kb/docs (person,run) kb.knowledge/read diff --git a/packages/apps/studio/tests/components/issue-board-live.test.ts b/packages/apps/studio/tests/components/issue-board-live.test.ts new file mode 100644 index 00000000000..018549bbcb7 --- /dev/null +++ b/packages/apps/studio/tests/components/issue-board-live.test.ts @@ -0,0 +1,172 @@ +/** + * The projects plugin's board and issue pages on Studio's pages, kept current by the app-wide subscription + * (`client/issues/groups/live.ts`): a named change reads only the shown rows (`issueIds`) and merges them in; a row that + * left the filters or changed column, or an announcement naming no issues, reads the list whole; the inbox reads + * neither; a burst is one request. + */ +import type { RealtimeListener } from '@nocobase/app-client'; +import type { + IssueBoard, + IssueListItem, +} from '@nocobase/app-plugin-projects/shared/issues'; +import { PM_REALTIME_TOPIC } from '@nocobase/app-plugin-projects/shared/realtime'; +import { + InfiniteQueryObserver, + QueryClient, + QueryObserver, +} from '@tanstack/react-query'; +import { waitFor } from '@testing-library/react'; +import { afterEach, expect, it, vi } from 'vitest'; + +import { + subscribeTaskGroupsLive, + type IssueRowsReader, +} from '../../client/issues/groups/live.js'; +import { STUDIO_INBOX_TOPIC } from '../../shared/inbox.js'; + +const FILTERS = { projectId: 'p1' }; +const BOARD = ['pm', 'issues', 'board', FILTERS]; +const PAGES = ['pm', 'issues', 'pages', { deleted: true }]; + +const issue = ( + id: string, + statusKey: string, + title = `Task ${id}`, +): IssueListItem => ({ id, statusKey, title }) as unknown as IssueListItem; + +const boardOf = (issues: readonly IssueListItem[]): IssueBoard => + ({ + columns: ['backlog', 'in_progress'].map((key) => ({ + status: { key, name: key, category: 'unstarted', color: 'gray' }, + issues: issues.filter((each) => each.statusKey === key), + nextCursor: null, + })), + }) as IssueBoard; + +const clients: QueryClient[] = []; +const stops: (() => void)[] = []; +afterEach(() => { + for (const stop of stops.splice(0)) stop(); + for (const client of clients.splice(0)) client.clear(); +}); + +function tab(rows: IssueRowsReader) { + const client = new QueryClient({ + defaultOptions: { queries: { retry: false, staleTime: Infinity } }, + }); + clients.push(client); + const board = vi.fn(async () => + boardOf([issue('a', 'backlog'), issue('b', 'in_progress')]), + ); + const pages = vi.fn(async () => ({ + data: [issue('d', 'backlog')], + nextCursor: null, + })); + stops.push( + new QueryObserver(client, { queryKey: BOARD, queryFn: board }).subscribe( + () => {}, + ), + new InfiniteQueryObserver(client, { + queryKey: PAGES, + queryFn: pages, + initialPageParam: null, + getNextPageParam: () => undefined, + }).subscribe(() => {}), + ); + const listeners = new Map>(); + const readRows = vi.fn(rows); + stops.push( + subscribeTaskGroupsLive( + { + onOpen: () => () => {}, + subscribe: (topic, listener) => { + listeners.set(topic, listener as RealtimeListener); + return () => listeners.delete(topic); + }, + }, + client, + async () => ({ membership: 'm', data: [] }), + { delay: 20, issueRows: readRows, onVisible: () => () => {} }, + ), + ); + const emit = (topic: string, payload: unknown) => + listeners.get(topic)?.({ + type: 'event', + topic, + payload, + publishedAt: '2026-10-15T08:00:00Z', + }); + return { + client, + board, + pages, + readRows, + data: () => client.getQueryData(BOARD), + named: (...ids: string[]) => + emit(PM_REALTIME_TOPIC, { + kind: 'pm.changed', + domain: 'issues', + issueIds: ids, + }), + anything: () => + emit(PM_REALTIME_TOPIC, { kind: 'pm.changed', domain: 'issues' }), + inbox: () => emit(STUDIO_INBOX_TOPIC, { kind: 'studio.inbox.changed' }), + }; +} + +const settle = () => new Promise((resolve) => setTimeout(resolve, 80)); + +it('reads only the named rows a board shows, once for a burst, and merges them', async () => { + const page = tab(async (_query, ids) => + ids.map((id) => + issue(id, id === 'a' ? 'backlog' : 'in_progress', `New ${id}`), + ), + ); + await waitFor(() => expect(page.data()).toBeDefined()); + page.named('a'); + page.named('b', 'elsewhere'); + await waitFor(() => + expect( + page + .data()! + .columns.flatMap((column) => column.issues.map((each) => each.title)), + ).toEqual(['New a', 'New b']), + ); + await settle(); + expect(page.readRows).toHaveBeenCalledOnce(); + expect(page.readRows.mock.calls[0]![0]).toEqual(FILTERS); + expect([...page.readRows.mock.calls[0]![1]].sort()).toEqual(['a', 'b']); + expect(page.board).toHaveBeenCalledOnce(); + // The trash showed none of them. + expect(page.pages).toHaveBeenCalledOnce(); +}); + +it('reads the board whole when a shown row changed column or left its filters', async () => { + const moved = tab(async () => [issue('a', 'in_progress')]); + await waitFor(() => expect(moved.data()).toBeDefined()); + moved.named('a'); + await waitFor(() => expect(moved.board).toHaveBeenCalledTimes(2)); + + const left = tab(async () => []); + await waitFor(() => expect(left.data()).toBeDefined()); + left.named('b'); + await waitFor(() => expect(left.board).toHaveBeenCalledTimes(2)); +}); + +it('reads every list whole for an announcement naming nothing, and nothing for the inbox', async () => { + const page = tab(async () => []); + await waitFor(() => expect(page.data()).toBeDefined()); + page.inbox(); + await settle(); + expect(page.board).toHaveBeenCalledOnce(); + expect(page.pages).toHaveBeenCalledOnce(); + page.anything(); + page.anything(); + await waitFor(() => { + expect(page.board).toHaveBeenCalledTimes(2); + expect(page.pages).toHaveBeenCalledTimes(2); + }); + await settle(); + expect(page.board).toHaveBeenCalledTimes(2); + expect(page.readRows).not.toHaveBeenCalled(); +}); diff --git a/packages/plugins/app-plugin-projects/client/api/client.ts b/packages/plugins/app-plugin-projects/client/api/client.ts index 6f912ebaa96..bb790f4f13c 100644 --- a/packages/plugins/app-plugin-projects/client/api/client.ts +++ b/packages/plugins/app-plugin-projects/client/api/client.ts @@ -89,9 +89,10 @@ const ORDER_FIELDS: Readonly> = { /** The list query as URL parameters: the filters, `orderBy`, and a page of `ISSUE_PAGE_LIMIT.default` issues. */ function issueQuery(query: IssueListQuery): Query { - const { sort, direction, cursor, limit, ...filters } = query; + const { sort, direction, cursor, limit, issueIds, ...filters } = query; return present({ ...filters, + issueIds: issueIds?.join(','), orderBy: `${ORDER_FIELDS[sort ?? 'updated']} ${direction ?? 'desc'}`, pageSize: limit ?? ISSUE_PAGE_LIMIT.default, pageToken: cursor, diff --git a/packages/plugins/app-plugin-projects/server/domains/issues/issue.queries.ts b/packages/plugins/app-plugin-projects/server/domains/issues/issue.queries.ts index af6bb560145..9afa81b94bb 100644 --- a/packages/plugins/app-plugin-projects/server/domains/issues/issue.queries.ts +++ b/packages/plugins/app-plugin-projects/server/domains/issues/issue.queries.ts @@ -13,7 +13,6 @@ import { ACTIVITY_PAGE_LIMIT, ISSUE_PAGE_LIMIT, type ActivityPage, - type BoardColumn, type Issue, type IssueBoard, type IssueDetail, @@ -39,6 +38,7 @@ import { } from './issue.access.js'; import { findIssue, + findIssueRefs, findIssueRow, findIssueRows, hierarchyRecords, @@ -50,6 +50,7 @@ import { import type { KindRegistry } from '../../kernel/kinds.js'; import { isTerminal, type IssueExtras, type StatusService } from './ports.js'; import { FIXED_STATUSES } from '../../../shared/statuses.js'; +import { oneOf } from '../../kernel/db.js'; export interface IssueQueries { /** The fixed statuses, in order. */ @@ -142,6 +143,7 @@ function listFilter(viewer: Viewer, query: IssueListQuery) { const nodes: FilterNode[] = [ query.deleted ? deletedIssues(f, viewer) : visibleIssues(f, viewer), ]; + if (query.issueIds) nodes.push(oneOf(f, 'id', query.issueIds)); if (query.statusKey) nodes.push(f.string('statusKey').eq(query.statusKey)); if (query.projectId) nodes.push(f.string('projectId').eq(query.projectId)); if (query.involvesUserId) @@ -187,7 +189,8 @@ function listFilter(viewer: Viewer, query: IssueListQuery) { function exactMatcher( query: IssueListQuery, ): ((issue: Issue) => boolean) | null { - if (query.q?.trim() || query.labelId || query.parentIssueId) return null; + if (query.q?.trim() || query.labelId || query.parentIssueId || query.issueIds) + return null; return (issue) => (!query.statusKey || issue.statusKey === query.statusKey) && (!query.projectId || issue.projectId === query.projectId) && @@ -233,13 +236,11 @@ async function listItems( })); } -async function page( - deps: QueryDeps, - conn: DatabaseConnection, - viewer: Viewer, - query: IssueListQuery, - limit: number, -): Promise { +/** The sort field and direction a list query orders by. */ +function orderOf(query: IssueListQuery): { + readonly order: Order; + readonly direction: 'asc' | 'desc'; +} { const order: Order = query.sort === 'created' ? 'createdAt' @@ -253,23 +254,96 @@ async function page( order === 'priorityRank' ? query.direction !== 'asc' : query.direction === 'asc'; + return { order, direction: ascending ? 'asc' : 'desc' }; +} + +const cursorFieldsOf = + (order: Order) => + ( + row: Pick, + ) => ({ + [order]: order === 'priorityRank' ? priorityRank(row.priority) : row[order], + id: row.id, + }); + +async function page( + deps: QueryDeps, + conn: DatabaseConnection, + viewer: Viewer, + query: IssueListQuery, + limit: number, +): Promise { + const { order, direction } = orderOf(query); const rows = await findIssueRows(conn, { filter: listFilter(viewer, query), order, - direction: ascending ? 'asc' : 'desc', + direction, limit: limit + 1, ...(query.cursor ? { cursor: cursorOf(query.cursor, order) } : {}), }); - const result = pageOf(rows, limit, (row) => ({ - [order]: order === 'priorityRank' ? priorityRank(row.priority) : row[order], - id: row.id, - })); + const result = pageOf(rows, limit, cursorFieldsOf(order)); return { data: await listItems(deps, conn, result.rows), nextCursor: result.nextCursor, }; } +/** + * The first page of each of `statusKeys`, in a fixed number of reads whatever the number of issues: the ids of each + * column in its order, then the rows of every column at once, and what the rows show beside them at once. + */ +async function columnPages( + deps: QueryDeps, + conn: DatabaseConnection, + viewer: Viewer, + query: IssueListQuery, + statusKeys: readonly string[], + limit: number, +): Promise> { + const { order, direction } = orderOf(query); + const refs = new Map< + string, + { readonly ids: string[]; readonly nextCursor: string | null } + >(); + for (const statusKey of statusKeys) { + const found = await findIssueRefs(conn, { + filter: listFilter(viewer, { ...query, statusKey, cursor: undefined }), + order, + direction, + limit: limit + 1, + }); + const result = pageOf(found, limit, cursorFieldsOf(order)); + refs.set(statusKey, { + ids: result.rows.map((row) => row.id), + nextCursor: result.nextCursor, + }); + } + const ids = [...refs.values()].flatMap((column) => column.ids); + const rows = + ids.length > 0 + ? await findIssueRows(conn, { + filter: (f) => f.or(ids.map((id) => f.string('id').eq(id))), + order: 'number', + limit: ids.length, + }) + : []; + const items = new Map( + (await listItems(deps, conn, rows)).map((item) => [item.id, item]), + ); + return new Map( + [...refs].map(([statusKey, column]) => [ + statusKey, + { + data: column.ids.flatMap((id) => { + const item = items.get(id); + return item ? [item] : []; + }), + nextCursor: column.nextCursor, + }, + ]), + ); +} + /** An unknown status some issue is still in gets a plain column of its own. */ function unknownStatus(key: string): StatusDefinition { return { key, name: key, category: 'unstarted', color: 'gray' }; @@ -372,22 +446,21 @@ export function createIssueQueries(deps: QueryDeps): IssueQueries { for (const key of await statusesOf(conn, listFilter(viewer, query))) if (!statuses.some((status) => status.key === key)) statuses.push(unknownStatus(key)); - const columns: BoardColumn[] = []; - for (const status of statuses) { - const column = await page( - deps, - conn, - viewer, - { ...query, statusKey: status.key, cursor: undefined }, - limit, - ); - columns.push({ + const pages = await columnPages( + deps, + conn, + viewer, + query, + statuses.map((status) => status.key), + limit, + ); + return { + columns: statuses.map((status) => ({ status, - issues: column.data, - nextCursor: column.nextCursor, - }); - } - return { columns }; + issues: pages.get(status.key)?.data ?? [], + nextCursor: pages.get(status.key)?.nextCursor ?? null, + })), + }; }, visible(viewer, idOrKey) { diff --git a/packages/plugins/app-plugin-projects/server/domains/issues/issue.store.ts b/packages/plugins/app-plugin-projects/server/domains/issues/issue.store.ts index 834e719b5b4..7be534d2ad1 100644 --- a/packages/plugins/app-plugin-projects/server/domains/issues/issue.store.ts +++ b/packages/plugins/app-plugin-projects/server/domains/issues/issue.store.ts @@ -194,6 +194,31 @@ export async function findIssueRows( return rows.map((row) => toRow(row as Parameters[0])); } +/** The sort fields of an issue, for placing it in a list without reading what the list shows of it. */ +export type IssueRef = Pick< + Issue, + 'id' | 'number' | 'priority' | 'createdAt' | 'updatedAt' +>; + +/** The ids and sort fields of the issues `query` lists, in its order: a cheap first step before `findIssueRows`. */ +export async function findIssueRefs( + conn: DatabaseConnection, + query: IssueQuery, +): Promise { + const rows = await issues(conn).findMany({ + ...(query.filter ? { filter: query.filter } : {}), + ...(query.cursor ? { cursor: query.cursor } : {}), + select: (select) => + select.fields('id', 'number', 'priority', 'createdAt', 'updatedAt'), + sort: (sort) => + query.direction === 'asc' + ? [sort.field(query.order).asc(), sort.field('id').asc()] + : [sort.field(query.order).desc(), sort.field('id').desc()], + limit: query.limit, + }); + return rows; +} + const IDENTIFIER = /^[A-Za-z][A-Za-z0-9]*-\d+$/u; /** An issue by id or identifier (`PM-12`, any case), deleted or not. */ diff --git a/packages/plugins/app-plugin-projects/server/routes/schemas.ts b/packages/plugins/app-plugin-projects/server/routes/schemas.ts index bfe411fe597..37214281425 100644 --- a/packages/plugins/app-plugin-projects/server/routes/schemas.ts +++ b/packages/plugins/app-plugin-projects/server/routes/schemas.ts @@ -47,6 +47,7 @@ import type { InvitationResult, } from '../../shared/invitations.js'; import { + ISSUE_IDS_MAX, type Activity, type CreateIssueRequest, type Executor, @@ -310,6 +311,25 @@ function sortOf(value: string | undefined): { } const issueFilters = { + issueIds: z + .string() + .optional() + .transform((value) => + value === undefined + ? undefined + : [ + ...new Set( + value + .split(',') + .map((id) => id.trim()) + .filter(Boolean), + ), + ], + ) + .pipe(z.array(z.string().max(64)).max(ISSUE_IDS_MAX).optional()) + .meta({ + description: `Only these issues, comma-separated ids, at most ${ISSUE_IDS_MAX}; the other filters still apply.`, + }), involvesUserId: text.meta({ description: 'Owned by this user or executed by them as a person, each issue once.', @@ -327,6 +347,7 @@ const issueFilters = { }; type IssueFilterInput = { + issueIds?: string[] | undefined; involvesUserId?: string | undefined; statusKey?: string | undefined; projectId?: string | undefined; diff --git a/packages/plugins/app-plugin-projects/shared/issues.ts b/packages/plugins/app-plugin-projects/shared/issues.ts index 218f3341992..ef2cebc5bad 100644 --- a/packages/plugins/app-plugin-projects/shared/issues.ts +++ b/packages/plugins/app-plugin-projects/shared/issues.ts @@ -154,12 +154,20 @@ export const ISSUE_SORTS = [ ] as const; export type IssueSort = (typeof ISSUE_SORTS)[number]; +/** The most issues one list request names by id (`IssueListQuery.issueIds`). */ +export const ISSUE_IDS_MAX = 100; + /** * The issues a service lists. `GET /api/projects/issues` takes the same filters, `deleted=true`, `orderBy` (such as * `updatedAt desc`; see `server/routes/schemas.ts`) for `sort` and `direction`, and `pageSize` and `pageToken` for * `limit` and `cursor`. `GET /api/projects/issues/board` takes the filters and `pageSize` per column. */ export interface IssueListQuery { + /** + * Only these issues (at most `ISSUE_IDS_MAX`), still within the other filters: a page refreshes the rows an + * announcement named without reading the whole list. `issueIds` on the API, comma-separated. + */ + readonly issueIds?: readonly string[]; /** Owned by this person or executed by them as a user, each issue once. */ readonly involvesUserId?: string; readonly statusKey?: string; diff --git a/packages/plugins/app-plugin-projects/tests/domains/list-reads.test.ts b/packages/plugins/app-plugin-projects/tests/domains/list-reads.test.ts new file mode 100644 index 00000000000..a691e9b0861 --- /dev/null +++ b/packages/plugins/app-plugin-projects/tests/domains/list-reads.test.ts @@ -0,0 +1,125 @@ +// @vitest-environment node +/** + * The flat issue lists under load (`issueQueries.board`, `issueQueries.page`): the board reads a fixed number of + * queries whatever the number of issues, answers each column as its own page would, and a page narrowed to some issues + * (`issueIds`) keeps the other filters. + */ +import { AsyncLocalStorage } from 'node:async_hooks'; + +import type { Knex } from 'knex'; +import { afterEach, expect, it } from 'vitest'; + +import type { Issue } from '../../shared/issues.js'; +import { createHarness, type Harness } from '../harness.js'; + +const harnesses: Harness[] = []; +afterEach(async () => { + for (const h of harnesses.splice(0)) await h.close(); +}); + +const STATUSES = ['backlog', 'in_progress', 'in_review', 'blocked', 'done']; + +async function workload(n: number): Promise<{ h: Harness; issues: Issue[] }> { + const h = await createHarness(); + harnesses.push(h); + await h.addUser('admin', 'admin'); + await h.addUser('alice', 'alice'); + const admin = h.viewer('admin', 'admin'); + const project = await h.services.projects.create(admin, { name: 'P' }); + const label = await h.services.labels.create(admin, { + name: 'L', + color: 'blue', + }); + const issues: Issue[] = []; + for (let at = 0; at < n; at += 1) { + const created = await h.services.issues.create(admin, { + title: `Issue ${at}`, + ...(at % 2 === 0 ? { projectId: project.id } : {}), + ...(at % 3 === 0 ? { labelIds: [label.id] } : {}), + ...(at % 4 === 1 && issues[0] ? { parentIssueId: issues[0].id } : {}), + ...(at % 5 === 4 && issues.at(-2) + ? { blockedBy: [issues.at(-2)!.id] } + : {}), + }); + const statusKey = STATUSES[at % STATUSES.length]!; + issues.push( + statusKey === 'backlog' + ? created + : await h.services.issues.update(admin, created.id, { + revision: created.revision, + statusKey, + }), + ); + } + return { h, issues }; +} + +const scope = new AsyncLocalStorage(); + +async function queries(h: Harness, read: () => Promise) { + const client = await h.database.connection().client(); + await read(); + const mark = Symbol('read'); + let count = 0; + const listener = () => { + if (scope.getStore() === mark) count += 1; + }; + client.on('query', listener); + try { + await scope.run(mark, read); + } finally { + client.removeListener('query', listener); + } + return count; +} + +it('reads the board in a fixed number of queries whatever the number of issues', async () => { + const counts: number[][] = []; + for (const n of [8, 32]) { + const { h, issues } = await workload(n); + const admin = h.viewer('admin', 'admin'); + const member = h.viewer('alice'); + counts.push([ + await queries(h, () => h.services.issueQueries.board(admin, {})), + await queries(h, () => h.services.issueQueries.board(member, {})), + await queries(h, () => + h.services.issueQueries.board(admin, { q: 'Issue', limit: 3 }), + ), + await queries(h, () => + h.services.issueQueries.page(admin, { + issueIds: issues.slice(0, 5).map((issue) => issue.id), + }), + ), + ]); + } + expect(counts[1]).toEqual(counts[0]); +}, 120_000); + +it('answers each board column as its own page would', async () => { + const { h } = await workload(14); + const admin = h.viewer('admin', 'admin'); + for (const query of [{ limit: 2 }, { limit: 2, sort: 'priority' as const }]) { + const board = await h.services.issueQueries.board(admin, query); + for (const column of board.columns) { + const page = await h.services.issueQueries.page(admin, { + ...query, + statusKey: column.status.key, + }); + expect(column.issues).toEqual(page.data); + expect(column.nextCursor).toEqual(page.nextCursor); + } + } +}, 60_000); + +it('narrows a page to the issues named, within the other filters', async () => { + const { h, issues } = await workload(10); + const admin = h.viewer('admin', 'admin'); + const ids = issues.slice(0, 4).map((issue) => issue.id); + const named = await h.services.issueQueries.page(admin, { issueIds: ids }); + expect(named.data.map((issue) => issue.id).sort()).toEqual([...ids].sort()); + const backlog = await h.services.issueQueries.page(admin, { + issueIds: ids, + statusKey: 'backlog', + }); + expect(backlog.data.map((issue) => issue.id)).toEqual([issues[0]!.id]); +}, 60_000); diff --git a/packages/plugins/app-plugin-projects/tests/routes.test.ts b/packages/plugins/app-plugin-projects/tests/routes.test.ts index c8938ee2f5a..1312a7a5c38 100644 --- a/packages/plugins/app-plugin-projects/tests/routes.test.ts +++ b/packages/plugins/app-plugin-projects/tests/routes.test.ts @@ -549,6 +549,28 @@ describe('issues over HTTP', () => { expect( (await call('/issues?orderBy=title', { role: 'member' })).status, ).toBe(400); + // Some issues by id, within the other filters; too many ids are refused. + const ids = (await ( + await call('/issues?orderBy=number%20asc', { role: 'member' }) + ).json()) as { data: { id: string; identifier: string }[] }; + const named = (await ( + await call( + `/issues?issueIds=${ids.data[0]!.id},${ids.data[2]!.id},unknown`, + { role: 'member' }, + ) + ).json()) as { data: { identifier: string }[] }; + expect(named.data.map((issue) => issue.identifier).sort()).toEqual([ + 'PM-1', + 'PM-3', + ]); + expect( + ( + await call( + `/issues?issueIds=${Array.from({ length: 101 }, (_, n) => `i${n}`).join(',')}`, + { role: 'member' }, + ) + ).status, + ).toBe(400); const board = (await ( await call('/issues/board', { role: 'member' }) ).json()) as {