diff --git a/src/features/instance/databases/DESIGN.md b/src/features/instance/databases/DESIGN.md index 2d870379d..364ab11af 100644 --- a/src/features/instance/databases/DESIGN.md +++ b/src/features/instance/databases/DESIGN.md @@ -27,7 +27,15 @@ 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. The table view needs no +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. `$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/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..0615a8298 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, 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'; @@ -58,8 +59,8 @@ export function DatabaseOverview({ instanceDatabaseMap, databaseName }: { const canManage = useInstanceBrowseManagePermission(); const canImport = useInstanceImportOperationsPermission(); - const tables = instanceDatabaseMap?.[databaseName] ?? {}; - const tableNames = Object.keys(tables).sort(); + const tables = describedDatabase(instanceDatabaseMap, databaseName) ?? {}; + 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.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..77f86cdb7 100644 --- a/src/features/instance/databases/components/DatabaseTableView.tsx +++ b/src/features/instance/databases/components/DatabaseTableView.tsx @@ -18,6 +18,12 @@ import { DropdownMenuTrigger, } from '@/components/ui/dropdownMenu'; import { useInstanceClientIdParams } from '@/config/useInstanceClient'; +import { + declaredRelationships, + describedDatabase, + describedTable, + describedTableNames, +} from '@/features/instance/databases/functions/describedTable'; import { ExtraRecordAttribute, extraRecordAttributes, @@ -169,8 +175,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 +201,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. @@ -247,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); @@ -1157,7 +1163,7 @@ export function DatabaseTableView({ && !!instanceId && ( 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.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..d0aa53046 100644 --- a/src/features/instance/databases/components/TableContextMenuItems.tsx +++ b/src/features/instance/databases/components/TableContextMenuItems.tsx @@ -1,4 +1,9 @@ import { ContextMenuItem, ContextMenuSeparator } from '@/components/ui/contextMenu'; +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 { @@ -28,8 +33,9 @@ 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 = describedDatabase(instanceDatabaseMap, databaseName); + 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. const hasKnownPrimaryKey = !!primaryKey; @@ -41,7 +47,7 @@ export function TableContextMenuItems({ databaseName, tableName, instanceDatabas primaryKey, conditions: null, instanceTable, - databaseTables: instanceDatabaseMap?.[databaseName], + databaseTables, }); }; @@ -63,7 +69,7 @@ export function TableContextMenuItems({ databaseName, tableName, instanceDatabas Export CSV - {canManage && !isLastTable && ( + {canManage && instanceTable && !isLastTable && ( <> { + 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(); + 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', () => { + 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..ec7c4710b --- /dev/null +++ b/src/features/instance/databases/functions/describedTable.ts @@ -0,0 +1,41 @@ +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) : undefined; +} + +export function describedDatabase( + instanceDatabaseMap: InstanceDatabaseMap | undefined, + databaseName: string, +): InstanceDatabaseTableMap | undefined { + 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, + 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 d412e5d3a..868406bc5 100644 --- a/src/features/instance/databases/functions/resolveDatabasesRedirect.test.ts +++ b/src/features/instance/databases/functions/resolveDatabasesRedirect.test.ts @@ -44,6 +44,27 @@ 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('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, + }); + 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)', () => { 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..8e20c1069 100644 --- a/src/features/instance/databases/functions/resolveDatabasesRedirect.ts +++ b/src/features/instance/databases/functions/resolveDatabasesRedirect.ts @@ -1,3 +1,8 @@ +import { + describedDatabase, + describedDatabaseNames, + describedTable, +} from '@/features/instance/databases/functions/describedTable'; import { InstanceDatabaseMap } from '@/integrations/api/api.patch'; /** @@ -17,9 +22,9 @@ export function resolveDatabasesRedirect( if (!instanceDatabaseMap) { return null; } - const databaseExists = !!(params.databaseName && instanceDatabaseMap[params.databaseName]); + 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) { @@ -27,7 +32,7 @@ export function resolveDatabasesRedirect( } return null; } - if (params.tableName && !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/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; 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', {