fix(data-objectstack): every view write path invalidates the override map (#4363)

.changeset/view-overrides-invalidation-4363.md

@@ -0,0 +1,13 @@
+---
+'@object-ui/data-objectstack': patch
+---
+
+Every view write path now invalidates the override map — a created, renamed or deleted view is no longer shadowed by a five-minute-stale batch read
+
+`ObjectStackAdapter` caches two view-shaped reads: `getView` under `view:{object}:{viewId}`, and `listViewOverrides` under `view-overrides:{object}`. Four write paths touch view rows, and until now exactly one of them — `updateViewConfig` — invalidated the second key. `createView`, `updateView` and `deleteView` invalidated only the per-view key, so the batch override map kept answering from a snapshot taken up to `MetadataCache`'s default 5-minute TTL earlier.
+
+That gap does not heal itself. `loadViewOverrides` in app-shell's `ObjectView` treats a resolved map as authoritative and deliberately does not re-probe per view — that is objectui#3774's fix, and it is correct, since re-probing reinstates the 404 flurry the batch read exists to remove. So the per-view `getView` fallback that would have masked a stale map is by design unreachable, and the stale map is served in full. Meanwhile `listViews` is uncached and answers fresh, so the view switcher could list a view whose override body came from a map written minutes earlier: the sharpest shape is the rename/pin path (`updateView`), where a user edits a view, returns to the object, and is served the pre-edit override.
+
+All four paths now emit the same ordered pair — the per-view key, then the object's override map. The rule is uniform per method rather than per branch: `updateView`'s draft half invalidates both keys as its published half does, which is deliberate over-invalidation (both readers enumerate published rows, so a draft write stales neither) chosen because an unnecessary invalidation costs one refetch while a missed one costs the full TTL. `createView` names the per-view key too, because `saveItem` is an upsert and an explicit `spec.name` that already exists overwrites a published row a prior `getView` may hold.
+
+No signature, no cache key and no read path changed; the only difference is which keys each write drops. The pin suite added by objectui#4328 now asserts the full invalidation key set for all five call sites, with the sweep's two pins kept as untouched controls: `listViews` stays uncached, and no write path names the retired `views:{object}` key.

packages/data-objectstack/src/index.ts

@@ -2917,8 +2917,16 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
    * (the cache stores on success only), so a transient failure does not
    * pin an empty answer for the TTL.
    *
-   * Result is cached identically to {@link getView}; saving a view via
-   * {@link updateViewConfig} invalidates the cache.
+   * Result is cached identically to {@link getView}, and EVERY view write path
+   * on this adapter invalidates it — {@link updateViewConfig},
+   * {@link createView}, {@link updateView} and {@link deleteView}. For a long
+   * time only the first did (objectui#4363), which left the other three stale
+   * for the cache's 5-minute TTL. That gap does not self-heal: the consumer
+   * (`loadViewOverrides`, app-shell `ObjectView`) treats a RESOLVED map as
+   * authoritative and deliberately does not re-probe per view — objectui#3774,
+   * and correct, since re-probing reinstates the 404 flurry the batch read
+   * exists to remove. So a stale map here is served in full, and the per-view
+   * {@link getView} fallback that would have masked it never runs.
    *
    * @param objectName - Object name (e.g. 'lead')
    * @returns Map keyed by view name with the persisted override config
@@ -3150,6 +3158,10 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
    *
    * Generates a snake_case name if `spec.name` is not provided by appending
    * a short timestamp suffix to the source-name hint.
+   *
+   * Invalidates both view-shaped cache keys for the object, exactly as
+   * {@link updateViewConfig} does — see {@link listViewOverrides} for why the
+   * batch map is the one that cannot heal itself (objectui#4363).
    */
   async createView(
     objectName: string,
@@ -3179,6 +3191,14 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
       data: spec?.data || { provider: 'object', object: objectName },
     };
     const result: any = await this.client.meta.saveItem('view', name, fullSpec);
+    // Same key set as `updateViewConfig` — this is the other `saveItem('view', …)`
+    // write, and `saveItem` is an UPSERT: an explicit `spec.name` that already
+    // exists overwrites the published row, which a prior `getView` may hold.
+    // (A generated name cannot collide, and a miss is a `Map.delete` on an absent
+    // key, so the uniform rule costs nothing on the create-a-new-row path.)
+    this.metadataCache.invalidate?.(`view:${objectName}:${name}`);
+    // The batch override map gains a row and cannot notice on its own (#4363).
+    this.metadataCache.invalidate?.(`view-overrides:${objectName}`);
     if (result && result.item) return result.item;
     return fullSpec;
   }
@@ -3215,6 +3235,15 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
    * *published* view (no draft pending) is unchanged — it writes the
    * published overlay, as before.
    *
+   * **Both halves invalidate the same two keys** (objectui#4363): the per-view
+   * {@link getView} key and the batch {@link listViewOverrides} map. On the
+   * draft half that is deliberate over-invalidation — both readers enumerate
+   * PUBLISHED rows, so a draft write stales neither, exactly as was already
+   * true of the per-view line this joins. The costs are not symmetric: an
+   * unnecessary invalidation costs one refetch, a missed one costs up to the
+   * cache's 5-minute TTL of stale overrides, and "which half am I in?" is not
+   * a question a future edit to this method should have to re-answer.
+   *
    * @throws when the view resolves in neither home, or when either read fails
    *   for any other reason (network, permission). Both used to be swallowed
    *   and converted into the bad partial write above; a caller that wants a
@@ -3239,6 +3268,7 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
       const mergedDraft = mergeViewPatch(draft, partial, viewName, objectName);
       await metaClient.save('view', viewName, mergedDraft, { mode: 'draft' });
       this.metadataCache.invalidate?.(`view:${objectName}:${viewName}`);
+      this.metadataCache.invalidate?.(`view-overrides:${objectName}`);
       return mergedDraft;
     }
 
@@ -3269,6 +3299,7 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
     const merged = mergeViewPatch(current, partial, viewName, objectName);
     const result: any = await this.client.meta.saveItem('view', viewName, merged);
     this.metadataCache.invalidate?.(`view:${objectName}:${viewName}`);
+    this.metadataCache.invalidate?.(`view-overrides:${objectName}`);
     if (result && result.item) return result.item;
     return merged;
   }
@@ -3277,6 +3308,10 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
    * Delete an overlay view (reset to artifact default if one exists, or
    * remove entirely if it was a user-created view). Routes to
    * `DELETE /api/v1/meta/view/:name`.
+   *
+   * Invalidates both view-shaped keys: the deleted row leaves the batch
+   * override map too, and a ghost entry there is what the object page would
+   * keep applying (objectui#4363).
    */
   async deleteView(
     objectName: string,
@@ -3285,6 +3320,7 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
     await this.connect();
     const result: any = await this.client.meta.deleteItem('view', viewName);
     this.metadataCache.invalidate?.(`view:${objectName}:${viewName}`);
+    this.metadataCache.invalidate?.(`view-overrides:${objectName}`);
     return { deleted: !!(result?.deleted ?? result?.reset ?? true) };
   }
 

packages/data-objectstack/src/viewCacheInvalidation.pin.test.ts

@@ -31,6 +31,23 @@ import { ObjectStackAdapter } from './index';
  *    request count, which is what makes it a pure dead-code removal rather
  *    than a caching change. (Whether `listViews` SHOULD be cached is a
  *    separate product question, deliberately not settled here.)
+ *
+ * ## objectui#4363 — every write path names BOTH keys
+ *
+ * Removing the dead key made the surviving asymmetry visible: only
+ * `updateViewConfig` invalidated `view-overrides:{object}`, so `createView` /
+ * `updateView` / `deleteView` left the batch map stale for the cache's
+ * 5-minute TTL. It does not self-heal — `loadViewOverrides` (app-shell
+ * `ObjectView`) treats a RESOLVED map as authoritative and deliberately does
+ * not re-probe per view (#3774), so the per-view `getView` fallback that would
+ * have masked a stale map is by design unreachable.
+ *
+ * So the rule these pins now enforce is uniform and per-METHOD, not
+ * per-branch: **a write to a view row invalidates the per-view key and the
+ * object's override map.** Four paths (five call sites — `updateView` has a
+ * draft half and a published half) emit the same ordered pair. The two sweep
+ * pins are untouched controls: `listViews` stays uncached, and no path names a
+ * `views:` key.
  */
 
 interface Harness {
@@ -152,36 +169,69 @@ describe('view metadata cache — invalidation names only keys with readers (#37
     expect(invalidated).toEqual(['view:account:v1', 'view-overrides:account']);
   });
 
-  it('updateView (published overlay) invalidates the getView key only', async () => {
+  it('listViewOverrides reads back under the key the write paths invalidate', async () => {
+    // The pairing, not the string: every assertion below names
+    // `view-overrides:account` because THIS is the key the batch reader caches
+    // under. An invalidation that named anything else would be the `views:`
+    // mistake again, one rename later.
+    const { ds, cacheReads } = makeDS({ items: [VIEW] });
+
+    await ds.listViewOverrides('account');
+
+    expect(cacheReads).toEqual(['view-overrides:account']);
+  });
+
+  it('createView invalidates the per-view key and the override map (#4363)', async () => {
+    const { ds, invalidated } = makeDS();
+
+    await ds.createView('account', { name: 'account.mine', object: 'account' });
+
+    // A created view is a new row in the batch map; nothing else notices.
+    // `saveItem` is an upsert, so the per-view key is named too — an explicit
+    // `name` that already exists overwrites a row `getView` may hold cached.
+    expect(invalidated).toEqual(['view:account:account.mine', 'view-overrides:account']);
+  });
+
+  it('updateView (published overlay) invalidates both keys (#4363)', async () => {
     const { ds, invalidated } = makeDS({ published: { name: 'v1', object: 'account' } });
 
     await ds.updateView('account', 'v1', { label: 'Renamed' });
 
-    expect(invalidated).toEqual(['view:account:v1']);
+    expect(invalidated).toEqual(['view:account:v1', 'view-overrides:account']);
   });
 
-  it('updateView (pending draft) invalidates the getView key only', async () => {
+  it('updateView (pending draft) invalidates both keys (#4363)', async () => {
     const { ds, invalidated } = makeDS({ draft: VIEW, published: notFound() });
 
     await ds.updateView('account', VIEW.name, { label: 'Renamed' });
 
-    expect(invalidated).toEqual([`view:account:${VIEW.name}`]);
+    // Deliberate over-invalidation on this half: both readers enumerate
+    // PUBLISHED rows, so a draft write stales neither — as was already true of
+    // the per-view line this pairs with. Pinned so the uniform per-method rule
+    // is a decision on the record, not an oversight the next reader "fixes".
+    expect(invalidated).toEqual([
+      `view:account:${VIEW.name}`,
+      'view-overrides:account',
+    ]);
   });
 
-  it('deleteView invalidates the getView key only', async () => {
+  it('deleteView invalidates both keys (#4363)', async () => {
     const { ds, invalidated } = makeDS();
 
     await ds.deleteView('account', 'v1');
 
-    expect(invalidated).toEqual(['view:account:v1']);
+    // The deleted row leaves the override map too — a ghost entry there is what
+    // the object page would keep applying for the rest of the TTL.
+    expect(invalidated).toEqual(['view:account:v1', 'view-overrides:account']);
   });
 
   it('no write path invalidates a `views:` key — nothing populates one', async () => {
-    // createView is the path whose ONLY invalidation was the dead key, so it
-    // now invalidates nothing. Asserted as "no `views:` key" rather than "no
-    // invalidation at all": whether it ought to invalidate the override map
-    // (`listViewOverrides` enumerates the same rows) is a separate question,
-    // filed on its own card — this pin must not freeze the answer.
+    // createView is the path whose ONLY invalidation was the dead key. #3778
+    // asserted "no `views:` key" here rather than "no invalidation at all",
+    // deliberately leaving the override-map question to its own card — and
+    // #4363 answered it: createView now names both live keys (pinned above).
+    // The ASSERTION is unchanged, which is the point of the slot: the dead key
+    // stays dead however the live key set grows.
     const created = makeDS();
     await created.ds.createView('account', { name: 'account.mine', object: 'account' });