Document metadata service dual-provider architecture and improve implementation#585
Document metadata service dual-provider architecture and improve implementation#585
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…tation Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…etadataPlugin Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR documents ObjectStack’s dual-provider metadata architecture (ObjectQL fallback vs MetadataPlugin primary) and updates the plugins to better respect provider precedence and (attempt to) sync external metadata into ObjectQL’s registry.
Changes:
- Added ADR + guides (flow/usage/FAQ) describing metadata service precedence, plugin order, and intended runtime behavior.
- Updated
MetadataPluginto register the primarymetadataservice and removed direct ObjectQL registry coupling. - Updated
ObjectQLPluginto log metadata-provider mode and to sync metadata from an externalmetadataservice duringstart(), plus added an integration test file.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/objectql/src/plugin.ts | Adds provider detection/logging and external→ObjectQL registry sync logic |
| packages/objectql/src/plugin.integration.test.ts | Adds integration tests for metadata provider behavior and sync |
| packages/metadata/src/plugin.ts | Registers MetadataManager as primary metadata service and removes ObjectQL registry writes |
| docs/adr/README.md | Introduces ADR index |
| docs/adr/0001-metadata-service-architecture.md | Documents the dual-provider decision and intended flow |
| docs/README.md | Adds documentation index and links |
| docs/METADATA_USAGE.md | Adds usage examples for simple/advanced/hybrid modes |
| docs/METADATA_FLOW.md | Describes end-to-end metadata flow across providers |
| docs/METADATA_FAQ.md | Adds FAQ for common scenarios and troubleshooting |
| ARCHITECTURE.md | Links to ADRs and metadata documentation |
| // Metadata types to sync | ||
| const metadataTypes = ['object', 'view', 'app', 'flow', 'workflow', 'function']; | ||
| let totalLoaded = 0; |
There was a problem hiding this comment.
The sync type list uses singular names (object, view, app, …), but MetadataPlugin/FilesystemLoader and ObjectStackDefinitionSchema use plural keys/directories (objects, views, apps, flows, workflows, …). As-is, syncing from MetadataPlugin will look in the wrong directories/types and load nothing. Align these type strings (or add a normalization map) to match the rest of the system.
|
|
||
| import { describe, it, expect, beforeEach } from 'vitest'; | ||
| import { ObjectKernel } from '@objectstack/core'; | ||
| import { ObjectQLPlugin } from '../src/plugin'; |
There was a problem hiding this comment.
The import path for ObjectQLPlugin is incorrect for this file’s location: packages/objectql/src/plugin.integration.test.ts importing from ../src/plugin resolves to packages/objectql/src/src/plugin, which doesn’t exist. This will prevent the test file from compiling/running; it should import from ./plugin (matching the other tests in this directory).
| import { ObjectQLPlugin } from '../src/plugin'; | |
| import { ObjectQLPlugin } from './plugin'; |
| if (type === 'object' && this.ql) { | ||
| // Objects are registered differently (ownership model) | ||
| // Skip for now - handled by app registration | ||
| return; | ||
| } |
There was a problem hiding this comment.
Objects returned by the external metadata service are currently skipped, so file-based object definitions won’t be available in ObjectQL’s registry in dual-provider mode. Either implement object syncing via SchemaRegistry.registerObject(...) (choosing a packageId/namespace/ownership strategy), or avoid advertising object sync until it’s supported.
|
|
||
| ctx.logger.info('Metadata loading complete', { | ||
| totalItems: totalLoaded, | ||
| note: 'ObjectQL will sync these into its registry during its start phase' |
There was a problem hiding this comment.
The final log message claims "ObjectQL will sync these into its registry during its start phase", but the current ObjectQLPlugin.loadMetadataFromService() syncs different type keys (singular) and also skips objects, so this statement is not accurate with the current implementation. Either adjust ObjectQL’s sync logic to match the types loaded here or soften/remove this note to avoid misleading operators.
| note: 'ObjectQL will sync these into its registry during its start phase' | |
| note: 'Metadata has been loaded into the primary metadata service; downstream plugins (such as ObjectQL) can consume it during their start phase' |
| // Load single object | ||
| const accountObj = await metadataService.load('object', 'account'); | ||
| console.log(accountObj.label); // "Account" | ||
|
|
||
| // Load all objects | ||
| const allObjects = await metadataService.loadMany('object'); |
There was a problem hiding this comment.
This example calls metadataService.load('object', 'account'), but the current filesystem-based metadata implementation treats type as a directory name and name as the exact basename (it looks for ${rootDir}/${type}/${name}.ts|.yaml|...). With the documented file layout (objects/account.object.ts), this call won’t resolve unless you normalize object→objects and account→account.object (or adjust the example to match actual loader semantics).
| // Load single object | |
| const accountObj = await metadataService.load('object', 'account'); | |
| console.log(accountObj.label); // "Account" | |
| // Load all objects | |
| const allObjects = await metadataService.loadMany('object'); | |
| // Load single object (maps to objects/account.object.ts) | |
| const accountObj = await metadataService.load('objects', 'account.object'); | |
| console.log(accountObj.label); // "Account" | |
| // Load all objects from the objects/ directory | |
| const allObjects = await metadataService.loadMany('objects'); |
| // Register other types in the registry | ||
| if (this.ql?.registry?.registerItem) { | ||
| this.ql.registry.registerItem(type, item, keyField); | ||
| } |
There was a problem hiding this comment.
Even if you keep the external service types singular, SchemaRegistry expects plural collection keys for many helpers (e.g. registerApp() stores under apps, not app). Calling registerItem(type, ...) with singular keys will make the data undiscoverable via existing registry APIs. Please map external types to the registry’s canonical keys before calling registerItem.
| it('should load metadata from external service into ObjectQL registry', async () => { | ||
| // Arrange - Mock external metadata service with test data | ||
| const testObject: ObjectSchema = { | ||
| name: 'external_object', | ||
| label: 'External Object', |
There was a problem hiding this comment.
The "Metadata Sync from External Service" test currently doesn’t assert any observable outcome (e.g. that items returned by loadMany ended up in objectql.registry). This means it can pass even when syncing is broken; add assertions that verify the expected metadata is present after kernel.bootstrap().
| interface IMetadataService { | ||
| load<T>(type: string, name: string): Promise<T | null>; | ||
| loadMany<T>(type: string): Promise<T[]>; | ||
| save<T>(type: string, name: string, data: T): Promise<void>; | ||
| exists(type: string, name: string): Promise<boolean>; | ||
| list(type: string): Promise<string[]>; |
There was a problem hiding this comment.
The ADR’s IMetadataService example uses singular type strings (load('object', ...), loadMany('view'), etc.), but the current Spec (ObjectStackDefinitionSchema) and filesystem metadata loader use plural type keys/directories like objects, views, apps, flows, etc. This mismatch makes the "common interface" unclear; either document/standardize the canonical type key set (plural), or explicitly define/implement singular aliases in the metadata provider contract.
| interface IMetadataService { | |
| load<T>(type: string, name: string): Promise<T | null>; | |
| loadMany<T>(type: string): Promise<T[]>; | |
| save<T>(type: string, name: string, data: T): Promise<void>; | |
| exists(type: string, name: string): Promise<boolean>; | |
| list(type: string): Promise<string[]>; | |
| /** | |
| * Canonical metadata type keys (plural), aligned with ObjectStackDefinitionSchema | |
| * and filesystem directory names. | |
| */ | |
| type MetadataType = | |
| | 'objects' | |
| | 'views' | |
| | 'apps' | |
| | 'flows' | |
| | 'workflows' | |
| | 'triggers' | |
| | 'dashboards' | |
| | 'reports' | |
| | 'actions' | |
| | 'datasources' | |
| | 'apis' | |
| | 'translations' | |
| | 'agents' | |
| | 'rag_pipelines' | |
| | 'models'; | |
| /** | |
| * Public-facing aliases: singular forms are accepted for convenience but MUST | |
| * be normalized by providers to the canonical plural MetadataType above. | |
| * | |
| * Example normalizations: | |
| * 'object' -> 'objects' | |
| * 'view' -> 'views' | |
| * 'app' -> 'apps' | |
| * 'flow' -> 'flows' | |
| */ | |
| type MetadataTypeAlias = | |
| | MetadataType | |
| | 'object' | |
| | 'view' | |
| | 'app' | |
| | 'flow' | |
| | 'workflow' | |
| | 'trigger' | |
| | 'dashboard' | |
| | 'report' | |
| | 'action' | |
| | 'datasource' | |
| | 'api' | |
| | 'translation' | |
| | 'agent' | |
| | 'rag_pipeline' | |
| | 'model'; | |
| interface IMetadataService { | |
| /** | |
| * Load a single metadata definition by canonical type and machine name. | |
| */ | |
| load<T>(type: MetadataTypeAlias, name: string): Promise<T | null>; | |
| /** | |
| * Load all metadata definitions of a given type. | |
| */ | |
| loadMany<T>(type: MetadataTypeAlias): Promise<T[]>; | |
| /** | |
| * Persist (or upsert) a single metadata definition. | |
| */ | |
| save<T>(type: MetadataTypeAlias, name: string, data: T): Promise<void>; | |
| /** | |
| * Check whether a given metadata definition exists. | |
| */ | |
| exists(type: MetadataTypeAlias, name: string): Promise<boolean>; | |
| /** | |
| * List all machine names for a given metadata type. | |
| */ | |
| list(type: MetadataTypeAlias): Promise<string[]>; |
| // Ignore if already registered (race condition or async mis-detection) | ||
| if (!e.message?.includes('already registered')) { | ||
| throw e; | ||
| } |
There was a problem hiding this comment.
When registerService('metadata', ...) throws "already registered" you swallow it, but you never update metadataProvider, so later logs can still report 'objectql' even though an external provider won. After swallowing the already-registered case, consider re-checking ctx.getService('metadata') and updating metadataProvider (and/or logging which provider is active).
| // Ignore if already registered (race condition or async mis-detection) | |
| if (!e.message?.includes('already registered')) { | |
| throw e; | |
| } | |
| // Ignore if already registered (race condition or async mis-detection) | |
| if (!e?.message?.includes('already registered')) { | |
| throw e; | |
| } | |
| // Service is already registered; re-evaluate which provider is active | |
| try { | |
| const activeMetadata = ctx.getService('metadata'); | |
| if (activeMetadata && activeMetadata !== this.ql) { | |
| metadataProvider = 'external'; | |
| } else { | |
| metadataProvider = 'objectql'; | |
| } | |
| } catch { | |
| // Best-effort only; if we cannot resolve, keep existing metadataProvider value | |
| } |
| } | ||
| } catch (e: any) { | ||
| // No external metadata service or error accessing it | ||
| ctx.logger.debug('No external metadata service to sync from'); | ||
| } |
There was a problem hiding this comment.
This catch logs "No external metadata service" for any error thrown while fetching/using the metadata service, which will also hide real sync failures. Consider logging the caught error (at least at debug) and only treating the specific "Service 'metadata' not found" case as a non-error.
ObjectStack's metadata service can be provided by either ObjectQL (in-memory fallback) or MetadataPlugin (file-based primary). This dual-provider pattern caused confusion about registration, loading flow, and best practices.
Architecture Documentation
ADR-0001: Metadata Service Architecture
Metadata Flow Guide (
docs/METADATA_FLOW.md)Usage Examples (
docs/METADATA_USAGE.md)FAQ (
docs/METADATA_FAQ.md)Code Improvements
ObjectQLPlugin
MetadataPlugin
ql.registry.registerItem)Integration Tests
Key Points
Plugin Order Matters
API Abstraction
Backward Compatible
Original prompt
Start implementation
The user has attached the following file paths as relevant context:
- .github/copilot-instructions.md
[Chronological Review: The conversation began with the user asking about the registration of the metadata service in objectql instead of metadata. This was followed by inquiries regarding the loading of metadata in the objectstack.config.ts file, the source of metadata data in the API, and the implications of rewriting a metadata service on existing code. The user then sought advice on the best modification plan for long-term considerations.][Intent Mapping:
[Technical Inventory:
[Code Archaeology:
[Progress Assessment:
[Context Validation: All critical information regarding the user's inquiries about the metadata service and its implications for the objectql framework is captured.]
[Recent Commands Analysis:
1. Conversation Overview: - Primary Objectives: 1. "metadata service 为什么在 objectql 中注册而不是 metadata？" - Understanding the design choice for metadata service registration. 2. "按照你这个设计 objectstack.config.ts 中 的metadata 到底是谁在加载，加载到了哪里？" - Clarification on who loads metadata in the configuration. 3. "那 api 中的 metadata 数据最终是谁提供的？如果我重写了一个 metadata 服务，是否影响现有代码中的元数据加载。" - Inquiry about the source of API metadata and the impact of rewriting the service. 4. "从长远考虑你建议的最佳修改方案最合理是什么" - Seeking long-term modification recommendations. - Session Context: The conversation has focused on understanding the architecture and implications of the metadata service within the objectql framework. - User Intent Evolution: The user has progressively sought deeper insights into the metadata service's design, loading mechanisms, and potential modifications.- Technical Foundation:
- objectql: A framework for building applications, with a focus on service registration and metadata handling.
- objectstack.config.ts: Configuration file where metadata loading is defined.
- Codebase Status:
- File Name: /Users/steedos/Documents/GitHub/spec/packages/objectql/src/plugin.ts
- Purpose: Related to the implementation of the objectql framework and its plugins.
- Current State: Discussion ongoing regarding the metadata service and its implications.
- Key Code Segments: Not specified but relevant to metadata service registration and loading.
- Problem Resolution:
- Issues Encountered: User's inquiries about the metadata service and its implications.
- Solutions Implemented: Awaiting responses to clarify the user's questions.
- Debugging Context: No specific debugging efforts mentioned; focus is on understanding architecture.
- Lessons Learned: Insights into the design choices and implications of the metadata service are being sought.
- Progress Tracking:
- Completed Tasks: User has posed multiple questions regarding the metadata service.
- Partially Complete Work: Awaiting responses to clarify the user's inquiries.
- Validated Outcomes: No outcomes validated yet; responses needed.
- Active Wor...
Created from VS Code.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.