From 6ad4ba235651c6b33518fd834261acd0572ebf75 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Wed, 14 Oct 2026 15:58:45 -0400 Subject: [PATCH 1/5] fix(browse): keep a disabled Cleanup Orphan Blobs from running Cleanup Orphan Blobs is disabled while a cleanup is running, but its handler was on onClick, which Radix runs even for a disabled menu item; only CSS kept a pointer off it. A screen reader's activation or a synthetic click therefore still asked to start a second cleanup. The handler moves to onSelect, which Radix gates on disabled, as #1953 did for Export CSV. No other disabled-capable menu item under src/features/instance/databases uses onClick. Co-Authored-By: Claude Opus 5.5 --- .../components/DatabaseTableView.test.tsx | 21 +++++++++++++++++-- .../components/DatabaseTableView.tsx | 2 +- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/features/instance/databases/components/DatabaseTableView.test.tsx b/src/features/instance/databases/components/DatabaseTableView.test.tsx index 66ca6702e..754d1c89c 100644 --- a/src/features/instance/databases/components/DatabaseTableView.test.tsx +++ b/src/features/instance/databases/components/DatabaseTableView.test.tsx @@ -45,9 +45,9 @@ vi.mock('@/hooks/useAuth', () => ({ useStaffPermission: () => staffState.isInstanceOperator, })); -const cleanupOrphanBlobs = vi.hoisted(() => ({ mutate: vi.fn() })); +const cleanupOrphanBlobs = vi.hoisted(() => ({ mutate: vi.fn(), isPending: false })); vi.mock('@/integrations/api/instance/database/cleanupOrphanBlobs', () => ({ - useCleanupOrphanBlobsMutation: () => ({ mutate: cleanupOrphanBlobs.mutate, isPending: false }), + useCleanupOrphanBlobsMutation: () => ({ mutate: cleanupOrphanBlobs.mutate, isPending: cleanupOrphanBlobs.isPending }), })); vi.mock('@/hooks/usePermissions', () => ({ @@ -174,6 +174,7 @@ afterEach(() => { watchedValues.calls = []; deleteRecords.mutate.mockReset(); cleanupOrphanBlobs.mutate.mockReset(); + cleanupOrphanBlobs.isPending = false; routerState.params = {}; staffState.isInstanceOperator = false; vi.restoreAllMocks(); @@ -637,6 +638,22 @@ describe('DatabaseTableView asking before an action', () => { expect(document.body.style.pointerEvents).not.toBe('none'); }); + it('does not ask again while a cleanup is already running', () => { + staffState.isInstanceOperator = true; + routerState.params = { instanceId: 'instance-1' }; + cleanupOrphanBlobs.isPending = true; + renderView(); + + openTableOptions(); + const cleanup = screen.getByRole('menuitem', { name: 'Cleanup Orphan Blobs' }); + expect(cleanup.getAttribute('aria-disabled')).toBe('true'); + fireEvent.click(cleanup); + fireEvent.keyDown(cleanup, { key: 'Enter' }); + + expect(question('Clean up orphan blobs?')).toBeNull(); + expect(cleanupOrphanBlobs.mutate).not.toHaveBeenCalled(); + }); + // The view stays mounted when the route moves to another table, so a question asked about one // must not be answerable once the view shows the other. it('drops a question once the view is showing another table, and does not bring it back', () => { diff --git a/src/features/instance/databases/components/DatabaseTableView.tsx b/src/features/instance/databases/components/DatabaseTableView.tsx index e2089f7f1..2cba081ab 100644 --- a/src/features/instance/databases/components/DatabaseTableView.tsx +++ b/src/features/instance/databases/components/DatabaseTableView.tsx @@ -1157,7 +1157,7 @@ export function DatabaseTableView({ && !!instanceId && ( From 02dbf857bd2306649ce2ad30836ffe0160b3bd21 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Wed, 14 Oct 2026 15:59:32 -0400 Subject: [PATCH 2/5] fix(browse): don't offer Drop Table for a table the map no longer holds The tree and overview menus offered Drop Table whenever the database had more than one table, even for a table a refetch had just dropped from the map (another tab or user removed it while its menu was open). Like Add Records, Import and Export CSV since #1953, it now waits for the map to hold the table. The menu reads the map's own keys, so a table named `constructor` no longer finds Object.prototype's and counts as present. Co-Authored-By: Claude Opus 5.5 --- src/features/instance/databases/DESIGN.md | 6 +++++- .../components/TableContextMenuItems.test.tsx | 21 ++++++++++++++----- .../components/TableContextMenuItems.tsx | 12 +++++++---- 3 files changed, 29 insertions(+), 10 deletions(-) diff --git a/src/features/instance/databases/DESIGN.md b/src/features/instance/databases/DESIGN.md index 2d870379d..f81bd4054 100644 --- a/src/features/instance/databases/DESIGN.md +++ b/src/features/instance/databases/DESIGN.md @@ -27,7 +27,11 @@ schema load. Add Records, Import and Export CSV need the key itself, so they wai that names one (`hasKnownPrimaryKey`, in the toolbar and in [`TableContextMenuItems`](components/TableContextMenuItems.tsx), the tree and overview menus). A table nothing has described yet gets the same answer as one without a key, rather than an Export -that then reports "no primary key found" (#1941). +that then reports "no primary key found" (#1941). The menus' Drop Table waits for the map to hold +the table too: one the map no longer holds may already have been dropped. + +A Radix menu item runs its `onClick` even while it is disabled, and only CSS keeps a pointer off it, +so an item that can be disabled takes its action through `onSelect`, which Radix gates. `$id` is not a stand-in key. These are live-probe observations on `harperdb:4.7.36` and `harper-pro:5.2.13` (2026-09), not covered by a test here: diff --git a/src/features/instance/databases/components/TableContextMenuItems.test.tsx b/src/features/instance/databases/components/TableContextMenuItems.test.tsx index d069bfa95..6ad7a1e98 100644 --- a/src/features/instance/databases/components/TableContextMenuItems.test.tsx +++ b/src/features/instance/databases/components/TableContextMenuItems.test.tsx @@ -32,12 +32,12 @@ function renderMenu(rolePermission: Record, dog: Record, instanceDatabaseMap: unknown) { +function renderMenuOver(rolePermission: Record, instanceDatabaseMap: unknown, tableName = 'dog') { permission.current = rolePermission as unknown as LocalRolePermission; const menu = (map: unknown) => ( - + ); @@ -109,9 +109,14 @@ describe('TableContextMenuItems on a table with no primary key', () => { }); describe('TableContextMenuItems on a table the map does not describe', () => { + const dropTable = () => screen.queryByRole('menuitem', { name: 'Drop Table' }); + // Two other tables, so the database's last table is not what withholds Drop Table. + const otherTables = { cat: keyedTable, cow: keyedTable }; + function expectNothingOffered() { expect(addRecords()).toBeNull(); expect(importData()).toBeNull(); + expect(dropTable()).toBeNull(); expect(exportCsv().hasAttribute('data-disabled')).toBe(true); fireEvent.click(exportCsv()); expect(exportCsvSpy).not.toHaveBeenCalled(); @@ -123,17 +128,23 @@ describe('TableContextMenuItems on a table the map does not describe', () => { }); it('offers nothing that needs its key when the map lacks the table', () => { - renderMenuOver({ super_user: true }, { data: { cat: keyedTable } }); + renderMenuOver({ super_user: true }, { data: otherTables }); + expectNothingOffered(); + }); + + it('reads only the map’s own tables, whatever the table is called', () => { + renderMenuOver({ super_user: true }, { data: otherTables }, 'constructor'); expectNothingOffered(); }); // Both menus list tables from the map, so an open one meets an undescribed table when a refetch // lands without it: another tab or user dropped it. it('withdraws them from an open menu once a refetched map drops the table', () => { - const { rerenderOver } = renderMenuOver({ super_user: true }, { data: { dog: keyedTable, cat: keyedTable } }); + const { rerenderOver } = renderMenuOver({ super_user: true }, { data: { dog: keyedTable, ...otherTables } }); expect(exportCsv().hasAttribute('data-disabled')).toBe(false); + expect(dropTable()).not.toBeNull(); - rerenderOver({ data: { cat: keyedTable } }); + rerenderOver({ data: otherTables }); expectNothingOffered(); }); }); diff --git a/src/features/instance/databases/components/TableContextMenuItems.tsx b/src/features/instance/databases/components/TableContextMenuItems.tsx index b2439aef4..116f0a0e5 100644 --- a/src/features/instance/databases/components/TableContextMenuItems.tsx +++ b/src/features/instance/databases/components/TableContextMenuItems.tsx @@ -28,8 +28,12 @@ export function TableContextMenuItems({ databaseName, tableName, instanceDatabas const canImport = useInstanceImportDataPermission(instanceId ?? clusterId, databaseName, tableName); const { exportCsv } = useExportTableCsv(); - const isLastTable = Object.keys(instanceDatabaseMap?.[databaseName] || {}).length <= 1; - const instanceTable = instanceDatabaseMap?.[databaseName]?.[tableName]; + const databaseTables = instanceDatabaseMap?.[databaseName]; + const isLastTable = Object.keys(databaseTables || {}).length <= 1; + // Own keys only: a table named `constructor` would otherwise find Object.prototype's. + const instanceTable = databaseTables && Object.hasOwn(databaseTables, tableName) + ? databaseTables[tableName] + : undefined; const primaryKey = browsePrimaryKey(instanceTable); // Same withdrawal as the table view's toolbar: rows added here couldn't be listed there. const hasKnownPrimaryKey = !!primaryKey; @@ -41,7 +45,7 @@ export function TableContextMenuItems({ databaseName, tableName, instanceDatabas primaryKey, conditions: null, instanceTable, - databaseTables: instanceDatabaseMap?.[databaseName], + databaseTables, }); }; @@ -63,7 +67,7 @@ export function TableContextMenuItems({ databaseName, tableName, instanceDatabas Export CSV - {canManage && !isLastTable && ( + {canManage && instanceTable && !isLastTable && ( <> Date: Wed, 14 Oct 2026 16:11:40 -0400 Subject: [PATCH 3/5] fix(browse): leave a dropped table even when it is named like an Object member resolveDatabasesRedirect sends the view away from a database or table the map no longer holds, and that redirect is what keeps the table view's toolbar from offering Drop Table for a dropped table. It looked the names up with a plain property read, so a table named `constructor` or `toString` found Object.prototype's member, counted as present, and the view stayed on a table that was gone. It now checks own keys. Co-Authored-By: Claude Opus 5.5 --- src/features/instance/databases/DESIGN.md | 4 +++- .../functions/resolveDatabasesRedirect.test.ts | 11 +++++++++++ .../databases/functions/resolveDatabasesRedirect.ts | 5 +++-- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/features/instance/databases/DESIGN.md b/src/features/instance/databases/DESIGN.md index f81bd4054..064c9172e 100644 --- a/src/features/instance/databases/DESIGN.md +++ b/src/features/instance/databases/DESIGN.md @@ -28,7 +28,9 @@ that names one (`hasKnownPrimaryKey`, in the toolbar and in [`TableContextMenuItems`](components/TableContextMenuItems.tsx), the tree and overview menus). A table nothing has described yet gets the same answer as one without a key, rather than an Export that then reports "no primary key found" (#1941). The menus' Drop Table waits for the map to hold -the table too: one the map no longer holds may already have been dropped. +the table too: one the map no longer holds may already have been dropped. The table view needs no +such gate, because `resolveDatabasesRedirect` leaves a table the map no longer holds. Both read the +map's own keys, so a table named like an `Object.prototype` member isn't found there by mistake. A Radix menu item runs its `onClick` even while it is disabled, and only CSS keeps a pointer off it, so an item that can be disabled takes its action through `onSelect`, which Radix gates. diff --git a/src/features/instance/databases/functions/resolveDatabasesRedirect.test.ts b/src/features/instance/databases/functions/resolveDatabasesRedirect.test.ts index d412e5d3a..d6f583ac4 100644 --- a/src/features/instance/databases/functions/resolveDatabasesRedirect.test.ts +++ b/src/features/instance/databases/functions/resolveDatabasesRedirect.test.ts @@ -44,6 +44,17 @@ describe('resolveDatabasesRedirect', () => { expect(resolveDatabasesRedirect(map, { databaseName: 'alpha', tableName: 'widgets' })).toBeNull(); }); + it('treats a database or table named like an Object.prototype member as missing unless the map has it', () => { + expect(resolveDatabasesRedirect(map, { databaseName: 'constructor' })).toEqual({ + databaseName: 'alpha', + tableName: undefined, + }); + expect(resolveDatabasesRedirect(map, { databaseName: 'alpha', tableName: 'toString' })).toEqual({ + databaseName: 'alpha', + tableName: undefined, + }); + }); + it('stays put on an empty database (shows its overview, no redirect loop)', () => { expect(resolveDatabasesRedirect(map, { databaseName: 'empty' })).toBeNull(); }); diff --git a/src/features/instance/databases/functions/resolveDatabasesRedirect.ts b/src/features/instance/databases/functions/resolveDatabasesRedirect.ts index 02639a13b..e2d739ded 100644 --- a/src/features/instance/databases/functions/resolveDatabasesRedirect.ts +++ b/src/features/instance/databases/functions/resolveDatabasesRedirect.ts @@ -17,7 +17,8 @@ export function resolveDatabasesRedirect( if (!instanceDatabaseMap) { return null; } - const databaseExists = !!(params.databaseName && instanceDatabaseMap[params.databaseName]); + // Own keys only: a database or table named `constructor` would otherwise find Object.prototype's. + const databaseExists = !!params.databaseName && Object.hasOwn(instanceDatabaseMap, params.databaseName); if (!databaseExists) { const firstDatabaseName = Object.keys(instanceDatabaseMap).sort()[0]; // No redirect when there are no databases at all, or the fallback would land on the same @@ -27,7 +28,7 @@ export function resolveDatabasesRedirect( } return null; } - if (params.tableName && !instanceDatabaseMap[params.databaseName!][params.tableName]) { + if (params.tableName && !Object.hasOwn(instanceDatabaseMap[params.databaseName!], params.tableName)) { return { databaseName: params.databaseName!, tableName: undefined }; } return null; From 81ee118a82a01cecfb65fdb5837ef3fba808c1c6 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Wed, 14 Oct 2026 16:19:01 -0400 Subject: [PATCH 4/5] fix(browse): read the schema maps by name through own-key helpers only Browse looked tables and databases up in the describe_all map and the schema-relationship map with plain property reads, so a name such as `constructor` or `toString` found Object.prototype's member. Import Data then took a new table called `constructor` for an existing one and said nothing about creating it, and a relationship lookup could be handed a function where it expected an array. describedTable.ts adds ownEntry, describedDatabase, describedTable and declaredRelationships, which read own keys only. The table menus, the table view, the overview, Import Data, Add Records, the CSV export, the relationship lookup and resolveDatabasesRedirect now go through them. The redirect also treats a database entry with no describe as missing instead of throwing on it. Co-Authored-By: Claude Opus 5.5 --- src/features/instance/databases/DESIGN.md | 6 ++-- .../components/DatabaseActionModals.tsx | 15 +++++++-- .../databases/components/DatabaseOverview.tsx | 3 +- .../components/DatabaseTableView.tsx | 11 +++++-- .../components/TableContextMenuItems.tsx | 8 ++--- .../functions/describedTable.test.ts | 33 +++++++++++++++++++ .../databases/functions/describedTable.ts | 29 ++++++++++++++++ .../functions/relationshipAttributes.ts | 3 +- .../resolveDatabasesRedirect.test.ts | 8 +++++ .../functions/resolveDatabasesRedirect.ts | 6 ++-- .../databases/hooks/useExportTableCsv.ts | 3 +- .../ImportDataModal.permissions.test.tsx | 9 +++++ .../databases/modals/ImportDataModal.tsx | 17 ++++++---- 13 files changed, 126 insertions(+), 25 deletions(-) create mode 100644 src/features/instance/databases/functions/describedTable.test.ts create mode 100644 src/features/instance/databases/functions/describedTable.ts diff --git a/src/features/instance/databases/DESIGN.md b/src/features/instance/databases/DESIGN.md index 064c9172e..364ab11af 100644 --- a/src/features/instance/databases/DESIGN.md +++ b/src/features/instance/databases/DESIGN.md @@ -29,8 +29,10 @@ that names one (`hasKnownPrimaryKey`, in the toolbar and in table nothing has described yet gets the same answer as one without a key, rather than an Export that then reports "no primary key found" (#1941). The menus' Drop Table waits for the map to hold the table too: one the map no longer holds may already have been dropped. The table view needs no -such gate, because `resolveDatabasesRedirect` leaves a table the map no longer holds. Both read the -map's own keys, so a table named like an `Object.prototype` member isn't found there by mistake. +such gate, because `resolveDatabasesRedirect` leaves a table the map no longer holds. Browse reads +the describe map and the schema-relationship map by database and table name only through +[`describedTable.ts`](functions/describedTable.ts), which takes their own keys, so a table named like +an `Object.prototype` member (`constructor`, `toString`) is never found there by mistake. A Radix menu item runs its `onClick` even while it is disabled, and only CSS keeps a pointer off it, so an item that can be disabled takes its action through `onSelect`, which Radix gates. diff --git a/src/features/instance/databases/components/DatabaseActionModals.tsx b/src/features/instance/databases/components/DatabaseActionModals.tsx index deb672de4..04131d47d 100644 --- a/src/features/instance/databases/components/DatabaseActionModals.tsx +++ b/src/features/instance/databases/components/DatabaseActionModals.tsx @@ -1,4 +1,9 @@ import { useInstanceClientIdParams } from '@/config/useInstanceClient'; +import { + declaredRelationships, + describedDatabase, + describedTable, +} from '@/features/instance/databases/functions/describedTable'; import { getSchemaRelationshipsQueryOptions } from '@/features/instance/databases/functions/schemaRelationships'; import { AddTableRowModal } from '@/features/instance/databases/modals/AddTableRowModal'; import { CreateNewTableModal } from '@/features/instance/databases/modals/CreateNewTableModal'; @@ -55,7 +60,7 @@ export function DatabaseActionModals({ instanceDatabaseMap }: { instanceDatabase ); const addTableFromMap = addTarget - ? instanceDatabaseMap?.[addTarget.databaseName]?.[addTarget.tableName] + ? describedTable(instanceDatabaseMap, addTarget.databaseName, addTarget.tableName) : undefined; const [describedAddTable, setDescribedAddTable] = useState<{ target: AddTarget; table: InstanceTable }>(); const addInstanceTable = addTableFromMap @@ -123,8 +128,12 @@ export function DatabaseActionModals({ instanceDatabaseMap }: { instanceDatabase { if (!open) { diff --git a/src/features/instance/databases/components/DatabaseOverview.tsx b/src/features/instance/databases/components/DatabaseOverview.tsx index dcdc4f10a..baec0d796 100644 --- a/src/features/instance/databases/components/DatabaseOverview.tsx +++ b/src/features/instance/databases/components/DatabaseOverview.tsx @@ -3,6 +3,7 @@ import { DropdownMenu, DropdownMenuContent, DropdownMenuItem, DropdownMenuTrigge import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'; import { useInstanceClientIdParams } from '@/config/useInstanceClient'; import { TableRowContextMenu } from '@/features/instance/databases/components/TableRowContextMenu'; +import { describedDatabase } from '@/features/instance/databases/functions/describedTable'; import { summarizeDatabaseCounts } from '@/features/instance/databases/functions/summarizeDatabaseCounts'; import { useInstanceBrowseManagePermission, useInstanceImportOperationsPermission } from '@/hooks/usePermissions'; import { InstanceDatabaseMap } from '@/integrations/api/api.patch'; @@ -58,7 +59,7 @@ export function DatabaseOverview({ instanceDatabaseMap, databaseName }: { const canManage = useInstanceBrowseManagePermission(); const canImport = useInstanceImportOperationsPermission(); - const tables = instanceDatabaseMap?.[databaseName] ?? {}; + const tables = describedDatabase(instanceDatabaseMap, databaseName) ?? {}; const tableNames = Object.keys(tables).sort(); // Sizes/schema are already in the fast (count-skipping) map, so they render instantly. Row counts diff --git a/src/features/instance/databases/components/DatabaseTableView.tsx b/src/features/instance/databases/components/DatabaseTableView.tsx index 2cba081ab..d5115b628 100644 --- a/src/features/instance/databases/components/DatabaseTableView.tsx +++ b/src/features/instance/databases/components/DatabaseTableView.tsx @@ -18,6 +18,11 @@ import { DropdownMenuTrigger, } from '@/components/ui/dropdownMenu'; import { useInstanceClientIdParams } from '@/config/useInstanceClient'; +import { + declaredRelationships, + describedDatabase, + describedTable, +} from '@/features/instance/databases/functions/describedTable'; import { ExtraRecordAttribute, extraRecordAttributes, @@ -169,8 +174,8 @@ export function DatabaseTableView({ // describe_table backfills this table's count asynchronously. Render schema from whichever arrives first // -- the map is usually ready before describe_table -- so columns and records are never gated on the // (slower) count scan. describe_table wins once present: it's the per-table, refresh-invalidated copy. - const databaseTables = instanceDatabaseMap?.[databaseName]; - const tableFromMap = databaseTables?.[tableName]; + const databaseTables = describedDatabase(instanceDatabaseMap, databaseName); + const tableFromMap = describedTable(instanceDatabaseMap, databaseName, tableName); const instanceTable = describeTableData ?? tableFromMap; // What the empty state may actually invite, each gated on the source it would use rather than on @@ -195,7 +200,7 @@ export function DatabaseTableView({ const hasReplaceGrants = useInstanceTablePutPermission(instanceId ?? clusterId, databaseName, tableName); const replaceBlockedReason = replaceRecordsBlockedReason(registrationInfo?.version, hasReplaceGrants); const canReplaceRecords = replaceBlockedReason === undefined; - const schemaRelationships = schemaRelationshipMap?.[databaseName]?.[tableName]; + const schemaRelationships = declaredRelationships(schemaRelationshipMap, databaseName, tableName); // Read the attributes from the same map the modal's Random Data option reads (NOT the // describe-first `instanceTable`): where the two disagree, offering Seed reopens the empty-dropdown // dead end this gate exists to close. diff --git a/src/features/instance/databases/components/TableContextMenuItems.tsx b/src/features/instance/databases/components/TableContextMenuItems.tsx index 116f0a0e5..acc5270ef 100644 --- a/src/features/instance/databases/components/TableContextMenuItems.tsx +++ b/src/features/instance/databases/components/TableContextMenuItems.tsx @@ -1,4 +1,5 @@ import { ContextMenuItem, ContextMenuSeparator } from '@/components/ui/contextMenu'; +import { describedDatabase, describedTable } from '@/features/instance/databases/functions/describedTable'; import { browsePrimaryKey } from '@/features/instance/databases/functions/formatBrowseDataTableHeader'; import { useExportTableCsv } from '@/features/instance/databases/hooks/useExportTableCsv'; import { @@ -28,12 +29,9 @@ export function TableContextMenuItems({ databaseName, tableName, instanceDatabas const canImport = useInstanceImportDataPermission(instanceId ?? clusterId, databaseName, tableName); const { exportCsv } = useExportTableCsv(); - const databaseTables = instanceDatabaseMap?.[databaseName]; + const databaseTables = describedDatabase(instanceDatabaseMap, databaseName); const isLastTable = Object.keys(databaseTables || {}).length <= 1; - // Own keys only: a table named `constructor` would otherwise find Object.prototype's. - const instanceTable = databaseTables && Object.hasOwn(databaseTables, tableName) - ? databaseTables[tableName] - : undefined; + const instanceTable = describedTable(instanceDatabaseMap, databaseName, tableName); const primaryKey = browsePrimaryKey(instanceTable); // Same withdrawal as the table view's toolbar: rows added here couldn't be listed there. const hasKnownPrimaryKey = !!primaryKey; diff --git a/src/features/instance/databases/functions/describedTable.test.ts b/src/features/instance/databases/functions/describedTable.test.ts new file mode 100644 index 000000000..525eeb065 --- /dev/null +++ b/src/features/instance/databases/functions/describedTable.test.ts @@ -0,0 +1,33 @@ +import type { SchemaRelationshipMap } from '@/features/instance/databases/functions/schemaRelationships'; +import { InstanceDatabaseMap } from '@/integrations/api/api.patch'; +import { describe, expect, it } from 'vitest'; +import { declaredRelationships, describedDatabase, describedTable, ownEntry } from './describedTable'; + +const dog = { attributes: [] }; +const map = { data: { dog } } as unknown as InstanceDatabaseMap; +const relationships = [{ attribute: 'owner' }]; +const relationshipMap = { data: { dog: relationships } } as unknown as SchemaRelationshipMap; + +describe('describedDatabase and describedTable', () => { + it('return the map’s own entries', () => { + expect(describedDatabase(map, 'data')).toEqual({ dog }); + expect(describedTable(map, 'data', 'dog')).toBe(dog); + expect(declaredRelationships(relationshipMap, 'data', 'dog')).toBe(relationships); + }); + + it('return nothing for a name the map lacks, or before the map loads', () => { + expect(describedDatabase(undefined, 'data')).toBeUndefined(); + expect(describedTable(undefined, 'data', 'dog')).toBeUndefined(); + expect(describedTable(map, 'data', 'cat')).toBeUndefined(); + expect(describedTable(map, 'other', 'dog')).toBeUndefined(); + expect(declaredRelationships(undefined, 'data', 'dog')).toBeUndefined(); + expect(ownEntry(null, 'data')).toBeUndefined(); + }); + + it('never find an Object.prototype member', () => { + expect(describedDatabase(map, 'constructor')).toBeUndefined(); + expect(describedTable(map, 'data', 'toString')).toBeUndefined(); + expect(describedTable(map, 'constructor', 'name')).toBeUndefined(); + expect(declaredRelationships(relationshipMap, 'data', 'constructor')).toBeUndefined(); + }); +}); diff --git a/src/features/instance/databases/functions/describedTable.ts b/src/features/instance/databases/functions/describedTable.ts new file mode 100644 index 000000000..e7fc67686 --- /dev/null +++ b/src/features/instance/databases/functions/describedTable.ts @@ -0,0 +1,29 @@ +import { SchemaRelationship, SchemaRelationshipMap } from '@/features/instance/databases/functions/schemaRelationships'; +import { InstanceDatabaseMap, InstanceDatabaseTableMap, InstanceTable } from '@/integrations/api/api.patch'; + +export function ownEntry(record: Readonly> | null | undefined, key: string): T | undefined { + return record && Object.hasOwn(record, key) ? record[key] : undefined; +} + +export function describedDatabase( + instanceDatabaseMap: InstanceDatabaseMap | undefined, + databaseName: string, +): InstanceDatabaseTableMap | undefined { + return ownEntry(instanceDatabaseMap, databaseName); +} + +export function describedTable( + instanceDatabaseMap: InstanceDatabaseMap | undefined, + databaseName: string, + tableName: string, +): InstanceTable | undefined { + return ownEntry(describedDatabase(instanceDatabaseMap, databaseName), tableName); +} + +export function declaredRelationships( + schemaRelationshipMap: SchemaRelationshipMap | undefined, + databaseName: string, + tableName: string, +): SchemaRelationship[] | undefined { + return ownEntry(ownEntry(schemaRelationshipMap, databaseName), tableName); +} diff --git a/src/features/instance/databases/functions/relationshipAttributes.ts b/src/features/instance/databases/functions/relationshipAttributes.ts index a2809bea9..ac63ecb91 100644 --- a/src/features/instance/databases/functions/relationshipAttributes.ts +++ b/src/features/instance/databases/functions/relationshipAttributes.ts @@ -1,3 +1,4 @@ +import { ownEntry } from '@/features/instance/databases/functions/describedTable'; import { SchemaRelationship } from '@/features/instance/databases/functions/schemaRelationships'; import { InstanceAttribute, InstanceDatabaseTableMap, InstanceTable } from '@/integrations/api/api.patch'; @@ -152,7 +153,7 @@ export function getRelationshipInfoMap( } } for (const declared of schemaRelationships ?? []) { - const relatedTable = databaseTables?.[declared.relatedTableName]; + const relatedTable = ownEntry(databaseTables, declared.relatedTableName); const relatedPrimaryKey = relatedTable?.primary_key ?? relatedTable?.hash_attribute; if (!relatedTable || !relatedPrimaryKey || !databaseTables) { continue; diff --git a/src/features/instance/databases/functions/resolveDatabasesRedirect.test.ts b/src/features/instance/databases/functions/resolveDatabasesRedirect.test.ts index d6f583ac4..7fff12a14 100644 --- a/src/features/instance/databases/functions/resolveDatabasesRedirect.test.ts +++ b/src/features/instance/databases/functions/resolveDatabasesRedirect.test.ts @@ -55,6 +55,14 @@ describe('resolveDatabasesRedirect', () => { }); }); + it('treats a database with no describe as missing rather than throwing', () => { + const withHole = { alpha: {}, broken: null } as unknown as InstanceDatabaseMap; + expect(resolveDatabasesRedirect(withHole, { databaseName: 'broken', tableName: 'orders' })).toEqual({ + databaseName: 'alpha', + tableName: undefined, + }); + }); + it('stays put on an empty database (shows its overview, no redirect loop)', () => { expect(resolveDatabasesRedirect(map, { databaseName: 'empty' })).toBeNull(); }); diff --git a/src/features/instance/databases/functions/resolveDatabasesRedirect.ts b/src/features/instance/databases/functions/resolveDatabasesRedirect.ts index e2d739ded..6af7656d9 100644 --- a/src/features/instance/databases/functions/resolveDatabasesRedirect.ts +++ b/src/features/instance/databases/functions/resolveDatabasesRedirect.ts @@ -1,3 +1,4 @@ +import { describedDatabase, describedTable } from '@/features/instance/databases/functions/describedTable'; import { InstanceDatabaseMap } from '@/integrations/api/api.patch'; /** @@ -17,8 +18,7 @@ export function resolveDatabasesRedirect( if (!instanceDatabaseMap) { return null; } - // Own keys only: a database or table named `constructor` would otherwise find Object.prototype's. - const databaseExists = !!params.databaseName && Object.hasOwn(instanceDatabaseMap, params.databaseName); + const databaseExists = !!params.databaseName && !!describedDatabase(instanceDatabaseMap, params.databaseName); if (!databaseExists) { const firstDatabaseName = Object.keys(instanceDatabaseMap).sort()[0]; // No redirect when there are no databases at all, or the fallback would land on the same @@ -28,7 +28,7 @@ export function resolveDatabasesRedirect( } return null; } - if (params.tableName && !Object.hasOwn(instanceDatabaseMap[params.databaseName!], params.tableName)) { + if (params.tableName && !describedTable(instanceDatabaseMap, params.databaseName!, params.tableName)) { return { databaseName: params.databaseName!, tableName: undefined }; } return null; diff --git a/src/features/instance/databases/hooks/useExportTableCsv.ts b/src/features/instance/databases/hooks/useExportTableCsv.ts index 96fafe91b..c371f0348 100644 --- a/src/features/instance/databases/hooks/useExportTableCsv.ts +++ b/src/features/instance/databases/hooks/useExportTableCsv.ts @@ -1,4 +1,5 @@ import { useInstanceClientIdParams } from '@/config/useInstanceClient'; +import { declaredRelationships } from '@/features/instance/databases/functions/describedTable'; import { getSchemaRelationshipsQueryOptions } from '@/features/instance/databases/functions/schemaRelationships'; import { buildExportGetAttributes, @@ -78,7 +79,7 @@ export function useExportTableCsv() { records, instanceTable, databaseTables, - schemaRelationshipMap[databaseName]?.[tableName], + declaredRelationships(schemaRelationshipMap, databaseName, tableName), ); const blob = tableCsvBlob(records, columns); const url = URL.createObjectURL(blob); diff --git a/src/features/instance/databases/modals/ImportDataModal.permissions.test.tsx b/src/features/instance/databases/modals/ImportDataModal.permissions.test.tsx index 6123f3b5d..f89be1703 100644 --- a/src/features/instance/databases/modals/ImportDataModal.permissions.test.tsx +++ b/src/features/instance/databases/modals/ImportDataModal.permissions.test.tsx @@ -143,4 +143,13 @@ describe('ImportDataModal name fields', () => { expect(screen.getByText('Maximum 75 characters reached.')).toBeTruthy(); expect(screen.getByText('Maximum 250 characters reached.')).toBeTruthy(); }); + + it('says a table named like an Object.prototype member will be created, as for any new name', () => { + renderModal({ super_user: true }); + expect(screen.queryByText('New table')).toBeNull(); + + fireEvent.change(screen.getByLabelText('Table'), { target: { value: 'constructor' } }); + + expect(screen.getByText('New table')).toBeTruthy(); + }); }); diff --git a/src/features/instance/databases/modals/ImportDataModal.tsx b/src/features/instance/databases/modals/ImportDataModal.tsx index 6a55d6699..f0462a8ca 100644 --- a/src/features/instance/databases/modals/ImportDataModal.tsx +++ b/src/features/instance/databases/modals/ImportDataModal.tsx @@ -28,6 +28,11 @@ import { } from '@/components/ui/select'; import { Separator } from '@/components/ui/separator'; import { useInstanceClientIdParams } from '@/config/useInstanceClient'; +import { + declaredRelationships, + describedDatabase, + describedTable, +} from '@/features/instance/databases/functions/describedTable'; import { generateRandomRecords, randomizableAttributes, @@ -193,15 +198,15 @@ export function ImportDataModal({ const watchedTable = form.watch('table'); const selectedDataset = sampleDatasets.find((dataset) => dataset.id === datasetId); - const targetTable = instanceDatabaseMap?.[watchedDatabase || 'data']?.[watchedTable]; + const targetTable = describedTable(instanceDatabaseMap, watchedDatabase || 'data', watchedTable); const tableExists = !!targetTable; const tableWillBeCreated = !!instanceDatabaseMap && !!watchedTable && !tableExists; // Random data needs existing columns to model values on, so the option only appears // when the target table already has some beyond the primary key and system fields. const fillableAttributes = randomizableAttributes( targetTable?.attributes, - instanceDatabaseMap?.[watchedDatabase || 'data'], - schemaRelationshipMap?.[watchedDatabase || 'data']?.[watchedTable], + describedDatabase(instanceDatabaseMap, watchedDatabase || 'data'), + declaredRelationships(schemaRelationshipMap, watchedDatabase || 'data', watchedTable), ); const canGenerateRandom = fillableAttributes.length > 0; const isRandomDataset = datasetId === RANDOM_DATASET_ID; @@ -256,9 +261,9 @@ export function ImportDataModal({ let source: ImportSource; if (values.method === 'sample' && values.datasetId === RANDOM_DATASET_ID) { const attributes = randomizableAttributes( - instanceDatabaseMap?.[database]?.[table]?.attributes, - instanceDatabaseMap?.[database], - schemaRelationshipMap?.[database]?.[table], + describedTable(instanceDatabaseMap, database, table)?.attributes, + describedDatabase(instanceDatabaseMap, database), + declaredRelationships(schemaRelationshipMap, database, table), ); if (attributes.length === 0) { form.setError('datasetId', { From c419cb9a1c648b21b9a8207019232eb7f1a14869 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Wed, 14 Oct 2026 16:42:44 -0400 Subject: [PATCH 5/5] fix(browse): build the schema-relationship map without a prototype parseSchemaRelationships filled plain objects keyed by database and table names read from the instance's component schema files. A database named `__proto__` therefore wrote its tables onto Object.prototype, and one named `constructor` wrote them onto Object itself. Both levels of the map are now null-prototype objects, where those names are ordinary keys, and the query turns off structural sharing, which would rebuild a changed level as a plain object on the next refetch. ownEntry also returns undefined, not null, for an entry that holds nothing. The databases tree also reads each database's tables through describedDatabase, so every by-name read of the describe map in Browse goes through the own-key helpers, as DESIGN.md says. Listing works the same way: the tree, the overview, the redirect's fallback and both "last table" checks use describedDatabaseNames and describedTableNames, which name only entries that have a describe, so they agree with the lookups on an entry that has none. Co-Authored-By: Claude Opus 5.5 --- .../databases/components/DatabaseOverview.tsx | 4 +- .../components/DatabaseTableView.tsx | 5 +- .../DatabasesTree/buildItems.test.ts | 6 ++ .../components/DatabasesTree/buildItems.ts | 5 +- .../components/TableContextMenuItems.tsx | 8 ++- .../functions/describedTable.test.ts | 19 +++++- .../databases/functions/describedTable.ts | 14 ++++- .../resolveDatabasesRedirect.test.ts | 2 + .../functions/resolveDatabasesRedirect.ts | 8 ++- .../functions/schemaRelationships.test.ts | 62 ++++++++++++++++++- .../functions/schemaRelationships.ts | 9 ++- 11 files changed, 127 insertions(+), 15 deletions(-) diff --git a/src/features/instance/databases/components/DatabaseOverview.tsx b/src/features/instance/databases/components/DatabaseOverview.tsx index baec0d796..0615a8298 100644 --- a/src/features/instance/databases/components/DatabaseOverview.tsx +++ b/src/features/instance/databases/components/DatabaseOverview.tsx @@ -3,7 +3,7 @@ import { DropdownMenu, DropdownMenuContent, DropdownMenuItem, DropdownMenuTrigge import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'; import { useInstanceClientIdParams } from '@/config/useInstanceClient'; import { TableRowContextMenu } from '@/features/instance/databases/components/TableRowContextMenu'; -import { describedDatabase } from '@/features/instance/databases/functions/describedTable'; +import { describedDatabase, describedTableNames } from '@/features/instance/databases/functions/describedTable'; import { summarizeDatabaseCounts } from '@/features/instance/databases/functions/summarizeDatabaseCounts'; import { useInstanceBrowseManagePermission, useInstanceImportOperationsPermission } from '@/hooks/usePermissions'; import { InstanceDatabaseMap } from '@/integrations/api/api.patch'; @@ -60,7 +60,7 @@ export function DatabaseOverview({ instanceDatabaseMap, databaseName }: { const canImport = useInstanceImportOperationsPermission(); const tables = describedDatabase(instanceDatabaseMap, databaseName) ?? {}; - const tableNames = Object.keys(tables).sort(); + const tableNames = describedTableNames(instanceDatabaseMap, databaseName); // Sizes/schema are already in the fast (count-skipping) map, so they render instantly. Row counts // aren't, so fetch them per-table via describe_table's cheap estimate path -- but only for the diff --git a/src/features/instance/databases/components/DatabaseTableView.tsx b/src/features/instance/databases/components/DatabaseTableView.tsx index d5115b628..77f86cdb7 100644 --- a/src/features/instance/databases/components/DatabaseTableView.tsx +++ b/src/features/instance/databases/components/DatabaseTableView.tsx @@ -22,6 +22,7 @@ import { declaredRelationships, describedDatabase, describedTable, + describedTableNames, } from '@/features/instance/databases/functions/describedTable'; import { ExtraRecordAttribute, @@ -252,8 +253,8 @@ export function DatabaseTableView({ // answers `false` when it simply doesn't know, which would offer an irreversible action on a // guess): dropping a table needs positive evidence that another one remains. const canDropTable = useMemo( - () => canManageBrowseInstance && !!databaseTables && Object.keys(databaseTables).length > 1, - [canManageBrowseInstance, databaseTables], + () => canManageBrowseInstance && describedTableNames(instanceDatabaseMap, databaseName).length > 1, + [canManageBrowseInstance, instanceDatabaseMap, databaseName], ); const { toggled: filtersToggled, toggleOn: showFilters, toggleOff: hideFilters } = useToggler(false); diff --git a/src/features/instance/databases/components/DatabasesTree/buildItems.test.ts b/src/features/instance/databases/components/DatabasesTree/buildItems.test.ts index bedcf4798..1add35ff7 100644 --- a/src/features/instance/databases/components/DatabasesTree/buildItems.test.ts +++ b/src/features/instance/databases/components/DatabasesTree/buildItems.test.ts @@ -37,6 +37,12 @@ describe('buildItems', () => { expect(items['empty'].children).toEqual([]); }); + it('lists only databases the map has a describe for', () => { + const { items } = buildItems({ ...map, hole: null } as unknown as InstanceDatabaseMap, { canManage: true }); + expect(items[rootId].children).toEqual([createTableId, 'alpha', 'beta', 'empty']); + expect(items['hole']).toBeUndefined(); + }); + it('gives tables composite ids so duplicate names across databases do not collide', () => { const { items } = buildItems(map, { canManage: true }); expect(items['alpha/shared'].data).toEqual({ kind: 'table', databaseName: 'alpha', tableName: 'shared' }); diff --git a/src/features/instance/databases/components/DatabasesTree/buildItems.ts b/src/features/instance/databases/components/DatabasesTree/buildItems.ts index 64576655e..8dc1e562c 100644 --- a/src/features/instance/databases/components/DatabasesTree/buildItems.ts +++ b/src/features/instance/databases/components/DatabasesTree/buildItems.ts @@ -1,3 +1,4 @@ +import { describedDatabaseNames, describedTableNames } from '@/features/instance/databases/functions/describedTable'; import { InstanceDatabaseMap } from '@/integrations/api/api.patch'; import type { TreeItem } from 'react-complex-tree'; import { createTableId, rootId } from './specialItems'; @@ -28,10 +29,10 @@ export function buildItems( { canManage }: { canManage: boolean }, ): { items: Record>; rootId: string } { const items: Record> = {}; - const databaseNames = Object.keys(instanceDatabaseMap || {}).sort(); + const databaseNames = describedDatabaseNames(instanceDatabaseMap); for (const databaseName of databaseNames) { - const tableNames = Object.keys(instanceDatabaseMap?.[databaseName] || {}).sort(); + const tableNames = describedTableNames(instanceDatabaseMap, databaseName); items[databaseName] = { index: databaseName, isFolder: true, diff --git a/src/features/instance/databases/components/TableContextMenuItems.tsx b/src/features/instance/databases/components/TableContextMenuItems.tsx index acc5270ef..d0aa53046 100644 --- a/src/features/instance/databases/components/TableContextMenuItems.tsx +++ b/src/features/instance/databases/components/TableContextMenuItems.tsx @@ -1,5 +1,9 @@ import { ContextMenuItem, ContextMenuSeparator } from '@/components/ui/contextMenu'; -import { describedDatabase, describedTable } from '@/features/instance/databases/functions/describedTable'; +import { + describedDatabase, + describedTable, + describedTableNames, +} from '@/features/instance/databases/functions/describedTable'; import { browsePrimaryKey } from '@/features/instance/databases/functions/formatBrowseDataTableHeader'; import { useExportTableCsv } from '@/features/instance/databases/hooks/useExportTableCsv'; import { @@ -30,7 +34,7 @@ export function TableContextMenuItems({ databaseName, tableName, instanceDatabas const { exportCsv } = useExportTableCsv(); const databaseTables = describedDatabase(instanceDatabaseMap, databaseName); - const isLastTable = Object.keys(databaseTables || {}).length <= 1; + const isLastTable = describedTableNames(instanceDatabaseMap, databaseName).length <= 1; const instanceTable = describedTable(instanceDatabaseMap, databaseName, tableName); const primaryKey = browsePrimaryKey(instanceTable); // Same withdrawal as the table view's toolbar: rows added here couldn't be listed there. diff --git a/src/features/instance/databases/functions/describedTable.test.ts b/src/features/instance/databases/functions/describedTable.test.ts index 525eeb065..670e1ea88 100644 --- a/src/features/instance/databases/functions/describedTable.test.ts +++ b/src/features/instance/databases/functions/describedTable.test.ts @@ -1,7 +1,14 @@ import type { SchemaRelationshipMap } from '@/features/instance/databases/functions/schemaRelationships'; import { InstanceDatabaseMap } from '@/integrations/api/api.patch'; import { describe, expect, it } from 'vitest'; -import { declaredRelationships, describedDatabase, describedTable, ownEntry } from './describedTable'; +import { + declaredRelationships, + describedDatabase, + describedDatabaseNames, + describedTable, + describedTableNames, + ownEntry, +} from './describedTable'; const dog = { attributes: [] }; const map = { data: { dog } } as unknown as InstanceDatabaseMap; @@ -22,6 +29,16 @@ describe('describedDatabase and describedTable', () => { expect(describedTable(map, 'other', 'dog')).toBeUndefined(); expect(declaredRelationships(undefined, 'data', 'dog')).toBeUndefined(); expect(ownEntry(null, 'data')).toBeUndefined(); + expect(ownEntry({ data: null }, 'data')).toBeUndefined(); + }); + + it('name the databases and tables that have a describe, sorted', () => { + const withHoles = { beta: { b: dog, a: dog, hole: null }, alpha: {}, hole: null } as unknown as InstanceDatabaseMap; + expect(describedDatabaseNames(withHoles)).toEqual(['alpha', 'beta']); + expect(describedDatabaseNames(undefined)).toEqual([]); + expect(describedTableNames(withHoles, 'beta')).toEqual(['a', 'b']); + expect(describedTableNames(withHoles, 'hole')).toEqual([]); + expect(describedTableNames(undefined, 'beta')).toEqual([]); }); it('never find an Object.prototype member', () => { diff --git a/src/features/instance/databases/functions/describedTable.ts b/src/features/instance/databases/functions/describedTable.ts index e7fc67686..ec7c4710b 100644 --- a/src/features/instance/databases/functions/describedTable.ts +++ b/src/features/instance/databases/functions/describedTable.ts @@ -2,7 +2,7 @@ import { SchemaRelationship, SchemaRelationshipMap } from '@/features/instance/d import { InstanceDatabaseMap, InstanceDatabaseTableMap, InstanceTable } from '@/integrations/api/api.patch'; export function ownEntry(record: Readonly> | null | undefined, key: string): T | undefined { - return record && Object.hasOwn(record, key) ? record[key] : undefined; + return record && Object.hasOwn(record, key) ? (record[key] ?? undefined) : undefined; } export function describedDatabase( @@ -12,6 +12,18 @@ export function describedDatabase( return ownEntry(instanceDatabaseMap, databaseName); } +export function describedDatabaseNames(instanceDatabaseMap: InstanceDatabaseMap | undefined): string[] { + return Object.keys(instanceDatabaseMap ?? {}).filter((name) => !!describedDatabase(instanceDatabaseMap, name)).sort(); +} + +export function describedTableNames( + instanceDatabaseMap: InstanceDatabaseMap | undefined, + databaseName: string, +): string[] { + const databaseTables = describedDatabase(instanceDatabaseMap, databaseName); + return Object.keys(databaseTables ?? {}).filter((name) => !!ownEntry(databaseTables, name)).sort(); +} + export function describedTable( instanceDatabaseMap: InstanceDatabaseMap | undefined, databaseName: string, diff --git a/src/features/instance/databases/functions/resolveDatabasesRedirect.test.ts b/src/features/instance/databases/functions/resolveDatabasesRedirect.test.ts index 7fff12a14..868406bc5 100644 --- a/src/features/instance/databases/functions/resolveDatabasesRedirect.test.ts +++ b/src/features/instance/databases/functions/resolveDatabasesRedirect.test.ts @@ -61,6 +61,8 @@ describe('resolveDatabasesRedirect', () => { databaseName: 'alpha', tableName: undefined, }); + const holeFirst = { aaa: null, beta: {} } as unknown as InstanceDatabaseMap; + expect(resolveDatabasesRedirect(holeFirst, {})).toEqual({ databaseName: 'beta', tableName: undefined }); }); it('stays put on an empty database (shows its overview, no redirect loop)', () => { diff --git a/src/features/instance/databases/functions/resolveDatabasesRedirect.ts b/src/features/instance/databases/functions/resolveDatabasesRedirect.ts index 6af7656d9..8e20c1069 100644 --- a/src/features/instance/databases/functions/resolveDatabasesRedirect.ts +++ b/src/features/instance/databases/functions/resolveDatabasesRedirect.ts @@ -1,4 +1,8 @@ -import { describedDatabase, describedTable } from '@/features/instance/databases/functions/describedTable'; +import { + describedDatabase, + describedDatabaseNames, + describedTable, +} from '@/features/instance/databases/functions/describedTable'; import { InstanceDatabaseMap } from '@/integrations/api/api.patch'; /** @@ -20,7 +24,7 @@ export function resolveDatabasesRedirect( } const databaseExists = !!params.databaseName && !!describedDatabase(instanceDatabaseMap, params.databaseName); if (!databaseExists) { - const firstDatabaseName = Object.keys(instanceDatabaseMap).sort()[0]; + const firstDatabaseName = describedDatabaseNames(instanceDatabaseMap)[0]; // No redirect when there are no databases at all, or the fallback would land on the same // (nonexistent) name we're already on -- both would otherwise loop. if (firstDatabaseName && firstDatabaseName !== params.databaseName) { diff --git a/src/features/instance/databases/functions/schemaRelationships.test.ts b/src/features/instance/databases/functions/schemaRelationships.test.ts index 5e27d110d..2339ccfe1 100644 --- a/src/features/instance/databases/functions/schemaRelationships.test.ts +++ b/src/features/instance/databases/functions/schemaRelationships.test.ts @@ -1,5 +1,6 @@ +import { QueryClient } from '@tanstack/react-query'; import { describe, expect, it } from 'vitest'; -import { parseSchemaRelationships } from './schemaRelationships'; +import { getSchemaRelationshipsQueryOptions, parseSchemaRelationships } from './schemaRelationships'; // The live anvils app schema this feature was built against. const anvilsSchema = `type Category @table @export @sealed { @@ -70,6 +71,65 @@ type Post @table(database: "blog") { ]); }); + it('keeps a database named __proto__ or constructor as its own entry, touching no prototype', () => { + const schemaIn = (database: string) => + `type Owner @table(database: "${database}") { + id: ID @primaryKey + pets: [Pet] @relationship(to: "ownerId") +} + +type Pet @table(database: "${database}") { + id: ID @primaryKey + ownerId: ID @indexed +} +`; + const map = parseSchemaRelationships([schemaIn('__proto__'), schemaIn('constructor')]); + + expect(Object.keys(map).sort()).toEqual(['__proto__', 'constructor']); + for (const database of ['__proto__', 'constructor']) { + expect(map[database].Owner[0]).toMatchObject({ attribute: 'pets', relatedTableName: 'Pet' }); + } + + const tableNamedProto = `type Owner @table(table: "__proto__") { + id: ID @primaryKey + pets: [Pet] @relationship(to: "ownerId") +} + +type Pet @table { + id: ID @primaryKey + ownerId: ID @indexed +} +`; + const tables = parseSchemaRelationships([tableNamedProto]).data; + expect(Object.keys(tables)).toEqual(['__proto__']); + expect(Object.getPrototypeOf(tables)).toBeNull(); + expect(({} as Record).Owner).toBeUndefined(); + expect((Object as unknown as Record).Owner).toBeUndefined(); + }); + + it('keeps a __proto__ database an entry across a refetch that changes the map', () => { + const ownerIn = (database: string, pets: string) => + `type Owner @table(database: "${database}") { + id: ID @primaryKey + ${pets}: [Pet] @relationship(to: "ownerId") +} + +type Pet @table(database: "${database}") { + id: ID @primaryKey + ownerId: ID @indexed +} +`; + const options = getSchemaRelationshipsQueryOptions({ entityId: 'instance-1' } as never); + const queryClient = new QueryClient(); + queryClient.getQueryCache().build(queryClient, options as never); + queryClient.setQueryData(options.queryKey, parseSchemaRelationships([ownerIn('__proto__', 'pets')])); + queryClient.setQueryData(options.queryKey, parseSchemaRelationships([ownerIn('__proto__', 'animals')])); + + const map = queryClient.getQueryData(options.queryKey)!; + expect(Object.hasOwn(map, '__proto__')).toBe(true); + expect(map['__proto__'].Owner[0]).toMatchObject({ attribute: 'animals' }); + }); + it('ignores unparseable sources and tables without relationships', () => { expect(parseSchemaRelationships(['type Broken @table {'])).toEqual({}); expect(parseSchemaRelationships(['type Plain @table { id: ID @primaryKey }'])).toEqual({}); diff --git a/src/features/instance/databases/functions/schemaRelationships.ts b/src/features/instance/databases/functions/schemaRelationships.ts index 5575c04be..f1dc745d0 100644 --- a/src/features/instance/databases/functions/schemaRelationships.ts +++ b/src/features/instance/databases/functions/schemaRelationships.ts @@ -41,6 +41,9 @@ export function getSchemaRelationshipsQueryOptions(params: InstanceClientIdConfi queryKey: [params.entityId, 'schema-relationships'] as const, staleTime: 60_000, retry: false, + // Structural sharing would rebuild changed levels as plain objects, where a `__proto__` key + // sets the prototype instead of staying an entry. + structuralSharing: false, enabled: params.enabled !== false, queryFn: async (): Promise => { // Best-effort: browse must keep working for users who can't read component files @@ -123,7 +126,9 @@ export function parseSchemaRelationships(sources: string[]): SchemaRelationshipM } } - const map: SchemaRelationshipMap = {}; + // Without a prototype, a database or table named `__proto__` or `constructor` is an ordinary key + // rather than a write into Object.prototype or onto Object itself. + const map: SchemaRelationshipMap = Object.create(null); for (const table of tables) { if (!table.relationships.length) { continue; @@ -140,7 +145,7 @@ export function parseSchemaRelationships(sources: string[]): SchemaRelationshipM } } if (resolved.length) { - (map[table.database] ??= {})[table.tableName] = resolved; + (map[table.database] ??= Object.create(null))[table.tableName] = resolved; } } return map;