diff --git a/.changeset/29782-missing-object-list-read.md b/.changeset/29782-missing-object-list-read.md new file mode 100644 index 00000000000..fbb0e98e91c --- /dev/null +++ b/.changeset/29782-missing-object-list-read.md @@ -0,0 +1,12 @@ +--- +'@object-ui/data-objectstack': patch +--- + +fix(data-objectstack): a plain list read of an object the server does not know rejects with its 404 code and sentence instead of reading as an empty list + +Clause-②: no +Metadata-protocol: none + +`ObjectStackAdapter.find` on its plain path (no `$expand`, no `$search`) used to turn every 404 into `{ data: [], total: 0 }` and remember the resource, so every later read of it answered empty without asking the server. That memo exists for optional platform collections such as `sys_presence` and `sys_activity`, whose absence means the deployment does not install the feature. It also swallowed the 404 `OBJECT_NOT_FOUND` of a list's own object, so an object that failed to register read as "no data yet". + +The empty answer and the memo now apply only to names in the platform registry (`isPlatformProvidedObjectName` from `@objectstack/spec`). A 404 on any other object is rethrown with its code, status and message, and is not remembered, so lists, boards, widgets and pickers over a missing object show their load-failure state. A dashboard figure over a missing object no longer reads 0. Enable-block denials (`OBJECT_API_DISABLED`, `OBJECT_API_METHOD_NOT_ALLOWED`) are rethrown as before. diff --git a/ui/packages/app-shell/src/views/objectListApiDisabled-4408.test.tsx b/ui/packages/app-shell/src/views/objectListApiDisabled-4408.test.tsx index d14c45b7914..d4fa3bbe5c3 100644 --- a/ui/packages/app-shell/src/views/objectListApiDisabled-4408.test.tsx +++ b/ui/packages/app-shell/src/views/objectListApiDisabled-4408.test.tsx @@ -66,6 +66,7 @@ function makeAdapter(status: number, body: unknown) { ); }); const ds: any = new ObjectStackAdapter({ baseUrl: 'http://test.local', fetch: fetchImpl }); + ds.__fetch = fetchImpl; return ds; } @@ -154,3 +155,53 @@ describe('object list · an OBJECT_API_DISABLED 404 is not an empty list (#4408) expect(container.querySelector('[data-testid="list-error-state"]')).toBeNull(); }); }); + +/** + * objectstack#29782 — the same masking one code over, on the list's plain + * read path. An object that failed to register answers 404 + * `OBJECT_NOT_FOUND`. The adapter's plain `find()` (no `$expand`, no + * `$search`) degraded that to zero rows and memoised it, so the list said + * "no data yet" about an object that does not exist, and #29704's panel + * (the server's sentence and code) was reachable only on the raw path. The + * empty answer now belongs to platform collections alone + * (`isPlatformProvidedObjectName`); `sys_jwks` above is one, which is why the + * bare-404 control there still reads empty. + */ +describe('object list · a 404 OBJECT_NOT_FOUND on the plain read path is not an empty list (objectstack#29782)', () => { + const missingSchema: ListViewSchema = { + type: 'list-view', + objectName: 'crm_invoice', + fields: ['name'], + } as ListViewSchema; + + it("shows the server's sentence and code, not the empty state", async () => { + const ds = makeAdapter(404, { + success: false, + error: { code: 'OBJECT_NOT_FOUND', message: "Object 'crm_invoice' is not registered" }, + }); + const { container } = render( + + + , + ); + + const panel = await waitFor(() => { + const el = container.querySelector('[data-testid="list-error-state"]'); + expect(el).not.toBeNull(); + return el as HTMLElement; + }); + + expect(panel.textContent).toContain("Object 'crm_invoice' is not registered"); + expect(panel.querySelector('[data-testid="list-error-code"]')?.textContent).toBe('OBJECT_NOT_FOUND'); + expect(panel.textContent).not.toMatch(/connection/i); + expect(container.querySelector('[data-testid="empty-state"]')).toBeNull(); + + // Every data read the list made took the plain path: the masking lived + // there, so a read that went raw would make this pass for the wrong reason. + const dataCalls = (ds.__fetch.mock.calls as unknown[][]) + .map(([url]) => String(url)) + .filter((u) => u.includes('/data/')); + expect(dataCalls.length).toBeGreaterThan(0); + expect(dataCalls.every((u) => !/populate=|[$]?search=/.test(decodeURIComponent(u)))).toBe(true); + }); +}); diff --git a/ui/packages/data-objectstack/src/apiAccessDenied.test.ts b/ui/packages/data-objectstack/src/apiAccessDenied.test.ts index 7fe69a8fcf0..6f68e83fb76 100644 --- a/ui/packages/data-objectstack/src/apiAccessDenied.test.ts +++ b/ui/packages/data-objectstack/src/apiAccessDenied.test.ts @@ -140,10 +140,13 @@ describe('ObjectStackDataSource.find — enable-block denials are surfaced, not }); it('CONTROL: a 404 RECORD_NOT_FOUND is not an enable-block denial', async () => { + // On a platform collection, so the 404 takes the optional-collection + // branch. A non-registry object's 404 is rethrown instead + // (findMissingObject-29782.test.ts). const find = vi.fn().mockRejectedValue(clientError('No such record', 404, 'RECORD_NOT_FOUND')); const ds = makeDS(find); - await expect(ds.find('account')).resolves.toMatchObject({ data: [], total: 0 }); + await expect(ds.find('sys_comment')).resolves.toMatchObject({ data: [], total: 0 }); }); it('CONTROL: a 2xx with zero rows resolves as an ordinary empty result', async () => { diff --git a/ui/packages/data-objectstack/src/findMissingObject-29782.test.ts b/ui/packages/data-objectstack/src/findMissingObject-29782.test.ts new file mode 100644 index 00000000000..6c9c5a736ac --- /dev/null +++ b/ui/packages/data-objectstack/src/findMissingObject-29782.test.ts @@ -0,0 +1,164 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * A plain list read of an object the server does not know must reach the + * surface as a failure, not as "no data yet". + * + * `find()`'s SDK path (no `$expand`, no `$search`) degraded every 404 to + * `{ data: [], total: 0 }` and memoised the resource in `missingResources`. + * That memo exists for OPTIONAL platform collections (`sys_presence`, + * `sys_activity`, …), whose absence means "this deployment does not install + * the feature". It also swallowed the 404 of a list's OWN object: an object + * that failed to register answered 404 `OBJECT_NOT_FOUND`, the list showed its + * empty state, and every later read of that object short-circuited to empty + * without asking again. + * + * The error code cannot split the two cases: an uninstalled `sys_presence` and + * an unregistered `crm_invoice` both answer 404 `OBJECT_NOT_FOUND`. The split + * is by name, against the platform's curated registry + * (`isPlatformProvidedObjectName`, @objectstack/spec): a registry name keeps + * the empty answer and the memo; any other object's 404 is rethrown and not + * memoised. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { isPlatformProvidedObjectName } from '@objectstack/spec/system'; +import { ObjectStackAdapter } from './index'; + +/** An error shaped the way `@objectstack/client`'s fetch wrapper throws one. */ +function clientError(message: string, httpStatus: number, code?: string) { + const err = new Error(message) as Error & { httpStatus: number; code?: string }; + err.httpStatus = httpStatus; + if (code) err.code = code; + return err; +} + +function makeDS(find: any) { + const ds: any = new ObjectStackAdapter({ + baseUrl: 'http://test.local', + fetch: vi.fn(async () => + new Response(JSON.stringify({ success: true, data: { capabilities: {}, routes: {} } }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + })), + }); + ds.connected = true; + ds.connectionState = 'connected'; + ds.client = { data: { find } }; + return ds; +} + +const MISSING = 'crm_invoice'; +const NOT_FOUND_SENTENCE = "Object 'crm_invoice' is not registered"; + +describe('find — a 404 on a non-platform object is rethrown, not emptied', () => { + it('the names these tests lean on sit on the right side of the registry', () => { + expect(isPlatformProvidedObjectName(MISSING)).toBe(false); + expect(isPlatformProvidedObjectName('sys_presence')).toBe(true); + expect(isPlatformProvidedObjectName('sys_activity')).toBe(true); + }); + + it('REJECTS a plain find answered 404 OBJECT_NOT_FOUND, with the code, status and sentence', async () => { + const find = vi.fn().mockRejectedValue(clientError(NOT_FOUND_SENTENCE, 404, 'OBJECT_NOT_FOUND')); + const ds = makeDS(find); + + await expect(ds.find(MISSING, { $top: 0 })).rejects.toMatchObject({ + code: 'OBJECT_NOT_FOUND', + httpStatus: 404, + message: NOT_FOUND_SENTENCE, + }); + }); + + it('does not memoise it: a second find asks the server again and rejects again', async () => { + const find = vi.fn().mockRejectedValue(clientError(NOT_FOUND_SENTENCE, 404, 'OBJECT_NOT_FOUND')); + const ds = makeDS(find); + + await expect(ds.find(MISSING)).rejects.toMatchObject({ code: 'OBJECT_NOT_FOUND' }); + await expect(ds.find(MISSING, { $top: 25 })).rejects.toMatchObject({ code: 'OBJECT_NOT_FOUND' }); + expect(find).toHaveBeenCalledTimes(2); + }); + + it('a bare 404 with no code on a non-platform object is rethrown too', async () => { + const find = vi.fn().mockRejectedValue(clientError('Not found', 404)); + const ds = makeDS(find); + + await expect(ds.find(MISSING)).rejects.toMatchObject({ httpStatus: 404 }); + await expect(ds.find(MISSING)).rejects.toMatchObject({ httpStatus: 404 }); + expect(find).toHaveBeenCalledTimes(2); + }); + + it('over the real client: the wrapped 404 envelope reaches the caller as code + status', async () => { + const dataUrls: string[] = []; + const fetchImpl = vi.fn(async (url: RequestInfo | URL) => { + const href = String(url); + if (href.includes('/data/')) { + dataUrls.push(href); + return new Response( + JSON.stringify({ success: false, error: { code: 'OBJECT_NOT_FOUND', message: NOT_FOUND_SENTENCE } }), + { status: 404, headers: { 'Content-Type': 'application/json' } }, + ); + } + return new Response(JSON.stringify({ success: true, data: { capabilities: {}, routes: {} } }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + }); + const ds: any = new ObjectStackAdapter({ baseUrl: 'http://test.local', fetch: fetchImpl }); + ds.connected = true; + ds.connectionState = 'connected'; + + await expect(ds.find(MISSING, { $top: 0 })).rejects.toMatchObject({ + code: 'OBJECT_NOT_FOUND', + httpStatus: 404, + }); + await expect(ds.find(MISSING, { $top: 0 })).rejects.toMatchObject({ code: 'OBJECT_NOT_FOUND' }); + expect(dataUrls).toHaveLength(2); + expect(dataUrls.every((u) => u.includes(`/data/${MISSING}`))).toBe(true); + }); +}); + +describe('find — CONTROLS: the platform optional-collection half is unchanged', () => { + it('CONTROL: a registry name answered 404 OBJECT_NOT_FOUND degrades to empty and is memoised', async () => { + const find = vi.fn().mockRejectedValue(clientError("Object 'sys_presence' not found", 404, 'OBJECT_NOT_FOUND')); + const ds = makeDS(find); + + await expect(ds.find('sys_presence')).resolves.toMatchObject({ data: [], total: 0 }); + // Short-circuited: the second call never reaches the server. + await expect(ds.find('sys_presence', { $top: 5 })).resolves.toMatchObject({ data: [], total: 0 }); + expect(find).toHaveBeenCalledTimes(1); + }); + + it('CONTROL: the memo is per resource: an emptied platform name does not empty a missing object', async () => { + const find = vi.fn(async (resource: string) => { + throw clientError(`Object '${resource}' not found`, 404, 'OBJECT_NOT_FOUND'); + }); + const ds = makeDS(find); + + await expect(ds.find('sys_activity')).resolves.toMatchObject({ data: [], total: 0 }); + await expect(ds.find(MISSING)).rejects.toMatchObject({ code: 'OBJECT_NOT_FOUND' }); + }); + + it('CONTROL: an enable-block denial still rethrows, on a platform name and on any other', async () => { + const find = vi.fn().mockRejectedValue(clientError('Object API is disabled', 404, 'OBJECT_API_DISABLED')); + const ds = makeDS(find); + + await expect(ds.find('sys_jwks')).rejects.toMatchObject({ code: 'OBJECT_API_DISABLED', httpStatus: 404 }); + await expect(ds.find(MISSING)).rejects.toMatchObject({ code: 'OBJECT_API_DISABLED', httpStatus: 404 }); + // Neither is memoised: the platform name is asked again. + await expect(ds.find('sys_jwks')).rejects.toMatchObject({ code: 'OBJECT_API_DISABLED' }); + expect(find).toHaveBeenCalledTimes(3); + }); + + it('CONTROL: a 2xx with zero rows on a non-platform object is still an ordinary empty result', async () => { + const find = vi.fn().mockResolvedValue({ records: [], total: 0 }); + const ds = makeDS(find); + + await expect(ds.find(MISSING)).resolves.toMatchObject({ data: [], total: 0 }); + }); +}); diff --git a/ui/packages/data-objectstack/src/index.ts b/ui/packages/data-objectstack/src/index.ts index 7ddcc01acce..f82eb0b7a2a 100644 --- a/ui/packages/data-objectstack/src/index.ts +++ b/ui/packages/data-objectstack/src/index.ts @@ -39,6 +39,7 @@ import { isRefusedTextComparand, textComparandRefusalReason } from '@objectstack import type { ApiError, BatchUpdateResponse, ValidateDataIssue } from '@objectstack/spec/api'; import { ValidateDataIssueSchema } from '@objectstack/spec/api'; import { stripReadDecorations } from '@objectstack/spec/kernel'; +import { isPlatformProvidedObjectName } from '@objectstack/spec/system'; // #4237 — the metadata save door's advisory reader, shared with `MetadataClient` // rather than forked. ONE reader, two call sites: the other client class calls it // from `MetadataClient.save` (#4133/#4236), this one from the interceptor below. @@ -3448,10 +3449,13 @@ export class ObjectStackAdapter implements DataSource { private inflightFieldAccess = new Map>(); private unansweredFieldAccess = new Set(); private fieldAccessUnsupported = false; - // Resources that have responded 404 at least once (collection not installed - // on this backend). Subsequent find() calls short-circuit to an empty result - // so optional collections like sys_presence don't hammer the server with - // failing requests on every record open / panel render. + // Platform collections that have responded 404 at least once (the package + // that registers them is not installed on this backend). Subsequent find() + // calls short-circuit to an empty result so optional collections like + // sys_presence don't hammer the server with failing requests on every record + // open / panel render. Only names in the platform registry + // (`isPlatformProvidedObjectName`, @objectstack/spec) ever enter it: a 404 + // on any other object is the surface's own object missing, and is rethrown. private missingResources = new Set(); // Set once the server has told us it can't do a cross-object transactional // batch (the client SDK's data.batchTransaction threw HTTP 404/405/501). After @@ -3812,9 +3816,10 @@ export class ObjectStackAdapter implements DataSource { * Converts OData-style params to ObjectStack query options. */ async find(resource: string, params?: QueryParams): Promise> { - // Short-circuit when this resource has previously responded 404 — the - // collection isn't installed on this backend. Callers (AppHeader, - // RecordDetailView, …) treat empty data as "feature unavailable". + // Short-circuit when this platform collection has previously responded + // 404 — the package that registers it isn't installed on this backend. + // Callers (AppHeader, RecordDetailView, …) treat empty data as "feature + // unavailable". Only registry names are ever memoised (see the catch). if (this.missingResources.has(resource)) { return { data: [], total: 0 } as QueryResult; } @@ -3849,7 +3854,21 @@ export class ObjectStackAdapter implements DataSource { // (objectui#4408). Rethrow so the surface can say what happened; the // `missingResources` memo must not absorb it either, or the very first // denial would silently pin every later call to empty. - if (!isApiAccessDeniedError(err) && is404Error(err)) { + // + // A 404 degrades to empty ONLY for a platform-provided collection + // (the curated registry in @objectstack/spec): there, absence means + // "this deployment does not install the feature", and the optional + // probes read empty as exactly that. The error code cannot make this + // split: an uninstalled sys_presence and a list's own unregistered + // object both answer 404 `OBJECT_NOT_FOUND`. Any other object is the + // surface's own, so its 404 is rethrown, unmemoised, and the surface's + // load-failure state shows the server's code and sentence instead of + // "no data yet" (an object that failed to register is not empty). + if ( + !isApiAccessDeniedError(err) + && is404Error(err) + && isPlatformProvidedObjectName(resource) + ) { // Mark the resource so subsequent calls don't repeat the 404. this.missingResources.add(resource); return { data: [], total: 0 } as QueryResult;