fix: restore persisted metadata from sys_metadata on ObjectQLPlugin cold start#1076
fix: restore persisted metadata from sys_metadata on ObjectQLPlugin cold start#1076
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…old start ObjectQLPlugin.start() now calls protocol.loadMetaFromDb() after driver init and before syncRegisteredSchemas, restoring user-created objects, views, and apps from the database on every kernel boot. Gracefully degrades when sys_metadata table doesn't exist (first-run/in-memory). Agent-Logs-Url: https://github.com/objectstack-ai/framework/sessions/8106ca9a-9f6e-4dfb-b4e6-c4e1ff984289 Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
|
@copilot 评估是否已经完成了任务中要求的所有开发,并仔细测试确认修正bug |
已完成所有任务要求的验证,确认如下: Issue 需求对照 (全部完成 ✅):
测试结果: 188/188 全部通过
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a cold-start persistence gap in ObjectQLPlugin by restoring previously saved metadata from the sys_metadata table during kernel boot, so user-created objects/views/apps survive redeployments.
Changes:
- Added a startup hydration step in
ObjectQLPlugin.start()that callsprotocol.loadMetaFromDb()after driver init and before schema sync. - Introduced integration tests covering successful restore, graceful failure, and restore-before-schema-sync ordering.
- Added a changelog entry under
[Unreleased] > Fixed.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/objectql/src/plugin.ts | Adds restoreMetadataFromDb() and invokes it during start() to hydrate the in-memory registry from DB before schema sync. |
| packages/objectql/src/plugin.integration.test.ts | Adds integration tests validating metadata restoration behavior and ordering. |
| CHANGELOG.md | Documents the cold-start metadata restoration fix. |
| private async restoreMetadataFromDb(ctx: PluginContext): Promise<void> { | ||
| try { | ||
| const protocol = ctx.getService('protocol') as any; | ||
| if (!protocol || typeof protocol.loadMetaFromDb !== 'function') { | ||
| ctx.logger.debug('Protocol service does not support loadMetaFromDb, skipping DB restore'); | ||
| return; | ||
| } | ||
|
|
||
| const { loaded, errors } = await protocol.loadMetaFromDb(); | ||
|
|
There was a problem hiding this comment.
restoreMetadataFromDb relies on ctx.getService('protocol') as any to access loadMetaFromDb, but loadMetaFromDb is not part of the typed protocol contract (and the as any defeats type-safety). Consider introducing a local ProtocolWithDbRestore type + type guard (or extending the protocol contract with an optional loadMetaFromDb?) so this call is type-checked and discoverable.
| private async restoreMetadataFromDb(ctx: PluginContext): Promise<void> { | ||
| try { | ||
| const protocol = ctx.getService('protocol') as any; | ||
| if (!protocol || typeof protocol.loadMetaFromDb !== 'function') { | ||
| ctx.logger.debug('Protocol service does not support loadMetaFromDb, skipping DB restore'); | ||
| return; | ||
| } | ||
|
|
||
| const { loaded, errors } = await protocol.loadMetaFromDb(); | ||
|
|
||
| if (loaded > 0 || errors > 0) { | ||
| ctx.logger.info('Metadata restored from database', { loaded, errors }); | ||
| } else { | ||
| ctx.logger.debug('No persisted metadata found in database'); | ||
| } | ||
| } catch (e: unknown) { | ||
| // Non-fatal: first-run or in-memory driver may not have sys_metadata yet | ||
| ctx.logger.debug('DB metadata restore skipped (non-fatal)', { | ||
| error: e instanceof Error ? e.message : String(e), | ||
| }); | ||
| } |
There was a problem hiding this comment.
The catch-all log message DB metadata restore skipped (non-fatal) can be misleading because this try also covers non-DB failures (e.g., getService('protocol') throwing, permission enforcement, async-service error). Consider narrowing the try/catch (separate service resolution from DB hydration) or adjusting the message/fields (e.g., include a structured error object/stack) so operators can distinguish missing protocol vs DB/table issues.
| type: 'app', | ||
| name: 'custom_crm', | ||
| state: 'active', | ||
| metadata: JSON.stringify({ name: 'custom_crm', label: 'Custom CRM' }), | ||
| }, |
There was a problem hiding this comment.
This test persists an app record with type: 'app', but the registry’s canonical app helpers/validation are keyed under 'apps' (e.g., SchemaRegistry.registerApp() stores type 'apps'). Using 'apps' here would better reflect how apps are typically registered/persisted and would catch type-normalization issues earlier.
|
|
||
| // Assert — items should be restored into the registry | ||
| const registry = (kernel.getService('objectql') as any).registry; | ||
| expect(registry.getItem('app', 'custom_crm')).toEqual({ |
There was a problem hiding this comment.
The assertion checks registry.getItem('app', ...), which may not match how the rest of the code consumes apps (often via getAllApps() / 'apps' type). Consider asserting against the canonical app storage ('apps' / getAllApps()) so the test validates end-to-end behavior rather than a type alias that might not be used by consumers.
| expect(registry.getItem('app', 'custom_crm')).toEqual({ | |
| expect(registry.getAllApps()).toContainEqual({ |
|
@copilot fix all |
…ndling, canonical app type in tests Agent-Logs-Url: https://github.com/objectstack-ai/framework/sessions/0c3ad9ed-acb5-48f4-a9c5-c81bde62ed87 Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
All 4 review comments addressed in commit
All 188 tests pass. |
protocol.loadMetaFromDb()exists and works, but was never called during kernel boot. Custom objects/views/apps saved tosys_metadataviaprotocol.saveMetaItem()were silently lost on every cold start or redeployment.Changes
ObjectQLPlugin.start()lifecycle — AddedrestoreMetadataFromDb()call betweendriver.init()andsyncRegisteredSchemas(), closing the persist→hydrate loop:Graceful degradation — Non-fatal when protocol service is missing,
loadMetaFromDbisn't implemented, orsys_metadatatable doesn't exist yet (first-run / in-memory driver). Logs atdebuglevel and continues.Integration tests — Three new tests: successful restoration from mock DB records, graceful handling of DB errors, and verification that restore runs before schema sync.
CHANGELOG — Entry under
[Unreleased] > Fixed.