Skip to content

Commit 729a43a

Browse files
os-zhuangclaude
andauthored
fix(metadata): an FS change invalidates this node's listCache/registry (#5218) (#5229)
`NodeMetadataManager.handleFileEvent()` did two things when chokidar reported `add` / `change` / `unlink`: re-`load()` the file and `notifyWatchers()`. It touched neither `listCache` nor `registry` — and `load()` is a pure read (it delegates to `loadDiagnosed`, which only walks the loaders), so both caches kept the pre-change state. The manager's two read surfaces then contradicted each other for up to LIST_CACHE_TTL_MS (30s) after editing `rootDir/view/x.json`: `get()` returned the new definition because it falls through to the FilesystemLoader, while `list()` — REST `/api/v1/metadata/:type`, the Studio left rail, `listViews()` — kept serving the pre-change set. The HMR/SSE consumers the event woke answer it by re-reading through `list()`, so the wake-up handed back precisely the stale data it was announcing. Same defect shape as #5109 (a cluster peer's write) with a different trigger, so this reuses that fix's helper rather than re-deriving it: `invalidateForForeignWrite(type, name)`, widened `private` -> `protected`. A file event is a foreign write on the definition that matters — it did not come through this manager's write API, so nothing refreshed the caches on its behalf, and delete-not-prefill fits exactly (falling through to the loader IS the file's truth). Two constraints kept in line with every other write path in the base class: invalidate BEFORE announcing (`register` / `unregister` / `applyRepoEvent` / the cluster subscriber all do), so a watcher can never observe the event and the pre-event cache together; and drop the registry entry too, not just the list cache — FS-loaded items never enter the registry, but a same-named entry previously written by `register()` / `registerInMemory()` SHADOWS the loader in both `get()` and `list()`, and dropping the list cache alone would leave that stale copy answering forever. `type === 'api'` is unchanged in behaviour: the endpoint index was already covered on this path by #5089's `subscribe('api', ...)` seam. The fix connects the `invalidateListCache` seam too, making the two symmetric; the overlap is free because `EndpointMatcher.invalidate()` is two assignments to `undefined`. Hit surface is development-time: `MetadataPlugin` defaults to `watch: true`, forced off under `bootstrap: 'artifact-only'`, and `standalone-stack` passes `watch: false`. Tests drive `handleFileEvent` with synthetic events over real files, a real tmpdir and the real default FilesystemLoader — `startWatching` polls at `interval: 1000`, so a real watcher per case would be seconds of wait for no added coverage. One end-to-end case uses a real chokidar watcher to pin that its callbacks actually reach the handler. 9 of the 11 fail without the fix. Claude-Session: https://claude.ai/code/session_01Pbu27iNUfQCHeuS551Rqo7 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 718b229 commit 729a43a

4 files changed

Lines changed: 434 additions & 14 deletions

File tree

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
---
2+
"@objectstack/metadata": patch
3+
---
4+
5+
fix(metadata): 文件系统改动同样失效本节点的 `listCache`/`registry`,不再只叫醒 watcher (#5218)
6+
7+
`NodeMetadataManager.handleFileEvent()` 在 chokidar 报告 `add` / `change` /
8+
`unlink` 之后只做两件事:重新 `load()` 一次文件内容,然后 `notifyWatchers()`
9+
它既不碰 `listCache` 也不碰 `registry` —— 而 `load()` 是纯读路径(它委托给
10+
`loadDiagnosed`,后者只遍历 loader),两个缓存都不写。
11+
12+
后果是**同一个 manager 的两个读接口互相矛盾**。手改 `rootDir` 下的
13+
`view/<name>.json` 之后:
14+
15+
- `get(type, name)` 是新的 —— 它穿透到 `FilesystemLoader`;
16+
- `list(type)``LIST_CACHE_TTL_MS`(30 秒)窗口内继续返回改动前的清单 ——
17+
REST `/api/v1/metadata/:type`、Studio 左栏、`listViews()` 等一切走 `list()`
18+
的读都受影响。
19+
20+
更糟的是被这次事件叫醒的消费者(Studio HMR/SSE 流、ObjectQL SchemaRegistry
21+
桥)正是通过回头拉 `list()` 来响应的,于是这次唤醒**递回了它自己刚刚宣告已失效
22+
的那份数据**
23+
24+
这与 #5109(集群对端写入不失效本节点缓存)是同一形状、不同触发源,因此复用该
25+
修复落地的 `invalidateForForeignWrite(type, name)`(可见性由 `private` 放宽为
26+
`protected`):文件改动正是「不是经由本 manager 写接口发生的写入」,没有任何东西
27+
替它刷新过缓存,delete-而非-预填 的语义也正好对上 —— 穿透回 loader 读到的就是
28+
文件的真相。
29+
30+
两点与基类其余写路径一致的约束:
31+
32+
- **先失效,再通知**(`register` / `unregister` / `applyRepoEvent` / 集群订阅者
33+
都是这个次序),使 watcher 不可能同时观察到事件与事件前的缓存;
34+
- **registry 条目一并删除**,不只是列表缓存。FS 加载的条目本来就不进 registry,
35+
通常无可删;但当同名条目此前被 `register()` / `registerInMemory()` 写过时,
36+
它在 `get()``list()` 中都会**遮蔽** loader,只删列表缓存会让那份陈旧副本
37+
一直应答下去。
38+
39+
命中面主要是开发期:`MetadataPlugin` 默认 `watch: true`,在
40+
`bootstrap: 'artifact-only'` 下被强制关闭,`standalone-stack` 显式传
41+
`watch: false`。因此 artifact 模式的 `os dev` 与 standalone 不受影响,非 artifact
42+
的默认 `MetadataPlugin` 装配受影响。
43+
44+
`type === 'api'` 的行为不变:端点索引此前已由 #5089 装的 `subscribe('api', …)`
45+
那条缝覆盖,本次改动把 `invalidateListCache` 那条缝也接上,两条缝对称。
46+
`EndpointMatcher.invalidate()` 是两次赋 `undefined`,重复失效幂等。

packages/metadata/src/metadata-manager.ts

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2036,14 +2036,18 @@ export class MetadataManager implements IMetadataService {
20362036
* we did not perform ourselves has just invalidated, so the next read falls
20372037
* through to the source of truth.
20382038
*
2039-
* The two callers are the manager's two *foreign-write* seams — the
2040-
* repository watch loop ({@link applyRepoEvent}) and the cluster peer replay
2041-
* in {@link attachClusterPubSub}. Both learn about a write that landed
2042-
* somewhere else (the repo head; another node's `sys_metadata`) and hold
2043-
* caches that the write silently aged out. Local writes do not come through
2044-
* here: `register()` / `unregister()` / `registerInMemory()` update the
2045-
* registry to the value they just wrote and call `invalidateListCache()`
2046-
* themselves.
2039+
* The callers are the manager's *foreign-write* seams — the repository watch
2040+
* loop ({@link applyRepoEvent}), the cluster peer replay in
2041+
* {@link attachClusterPubSub}, and — since #5218 — `NodeMetadataManager`'s
2042+
* chokidar handler, which is why this is `protected` rather than `private`.
2043+
* All three learn about a write that landed somewhere else (the repo head;
2044+
* another node's `sys_metadata`; an editor writing `rootDir/view/x.json`) and
2045+
* hold caches that the write silently aged out. A file event qualifies on
2046+
* exactly the definition that matters here: it did not come through this
2047+
* manager's write API, so nothing has updated the caches on its behalf.
2048+
* Local writes do not come through here: `register()` / `unregister()` /
2049+
* `registerInMemory()` update the registry to the value they just wrote and
2050+
* call `invalidateListCache()` themselves.
20472051
*
20482052
* **Delete, do not pre-fill.** Even when the event carries a body we drop the
20492053
* registry entry rather than writing the body into it: the body reaching us
@@ -2052,15 +2056,17 @@ export class MetadataManager implements IMetadataService {
20522056
* a definition we did not load. Lazy invalidation is the safer default —
20532057
* `get()` then falls through to the loaders / repository, which is where the
20542058
* truth is. (This paragraph is the rationale `applyRepoEvent` carried since
2055-
* ADR-0008 PR-6; #5109 extended the same choice to the cluster path.)
2059+
* ADR-0008 PR-6; #5109 extended the same choice to the cluster path, #5218 to
2060+
* the filesystem watcher — where "the truth" is the file chokidar just
2061+
* reported, served by the `FilesystemLoader` the registry entry was shadowing.)
20562062
*
20572063
* `name` is optional because `MetadataWatchEvent.name` is: a nameless event
20582064
* cannot address a registry entry, so it invalidates the list cache only.
20592065
* Dropping the whole type store instead would evict `registerInMemory()`
20602066
* artefacts (code-owned datasources, ADR-0015 Addendum) that no loader can
20612067
* restore — an unrecoverable loss in exchange for a guess.
20622068
*/
2063-
private invalidateForForeignWrite(type: string, name?: string): void {
2069+
protected invalidateForForeignWrite(type: string, name?: string): void {
20642070
if (name) {
20652071
const typeStore = this.registry.get(type);
20662072
if (typeStore) {

0 commit comments

Comments
 (0)