Implement Production-Ready @objectstack/metadata Package#428
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Co-authored-by: xuyushun441-sys <255036401+xuyushun441-sys@users.noreply.github.com>
|
@copilot 我接受你的建议,帮我修改吧 |
Co-authored-by: xuyushun441-sys <255036401+xuyushun441-sys@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
已完成 实现内容:
可直接使用: import { MetadataManager } from '@objectstack/metadata';
const manager = new MetadataManager({
rootDir: './metadata',
formats: ['typescript', 'json', 'yaml'],
watch: true,
});
// 加载
const data = await manager.load('object', 'customer');
// 保存
await manager.save('object', 'project', projectData, {
format: 'typescript'
});📦 Commit: 774872c |
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
|
This PR is very large. Consider breaking it into smaller PRs for easier review. |
|
|
||
| try { | ||
| const stats = await fs.stat(filePath); | ||
| const content = await fs.readFile(filePath, 'utf-8'); |
Check failure
Code scanning / CodeQL
Potential file system race condition High
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…n metadata package Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
|
This PR is very large. Consider breaking it into smaller PRs for easier review. |
|
This PR is very large. Consider breaking it into smaller PRs for easier review. |
…inatedResult Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
|
This PR is very large. Consider breaking it into smaller PRs for easier review. |
|
This PR is very large. Consider breaking it into smaller PRs for easier review. |
There was a problem hiding this comment.
Pull request overview
This PR implements a production-ready @objectstack/metadata package to centralize metadata loading, saving, and persistence operations. Following a comprehensive architectural evaluation, the package is positioned at Layer 3 as a focused metadata I/O solution with multi-format support, caching, and file watching capabilities.
Changes:
- Added complete Zod protocol schemas for metadata operations in
@objectstack/spec - Implemented
@objectstack/metadatapackage with serializers (JSON/YAML/TypeScript/JavaScript) and filesystem loader - Enhanced security with SHA-256 hashing, safe YAML parsing, and robust TypeScript parsing
- Updated architecture documentation and package dependency graphs
Reviewed changes
Copilot reviewed 35 out of 36 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/spec/src/system/metadata-loader.zod.ts | Defines comprehensive Zod schemas for metadata operations protocol |
| packages/spec/src/system/metadata-loader.test.ts | Unit tests validating all protocol schemas |
| packages/metadata/src/metadata-manager.ts | Main orchestrator for metadata operations with caching and file watching |
| packages/metadata/src/serializers/*.ts | JSON, YAML, and TypeScript/JavaScript serializers |
| packages/metadata/src/loaders/filesystem-loader.ts | Filesystem-based metadata loader with glob pattern support |
| packages/objectql/src/protocol.ts | Adds OData compatibility field to query response |
| ARCHITECTURE.md, PACKAGE-DEPENDENCIES.md | Documentation updates for new package |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| const prevChar = i > 0 ? content[i - 1] : ''; | ||
|
|
||
| // Track string literals (simple handling of " and ') | ||
| if ((char === '"' || char === "'") && prevChar !== '\\') { | ||
| if (!inString) { | ||
| inString = true; | ||
| stringChar = char; | ||
| } else if (char === stringChar) { | ||
| inString = false; | ||
| stringChar = ''; | ||
| } |
There was a problem hiding this comment.
The escape character detection is incomplete. A string like 'test\\\\'quote' would incorrectly detect the quote as unescaped because prevChar would be a backslash, but that backslash itself is escaped. Consider implementing proper escape sequence handling that tracks whether the backslash itself is escaped.
| const prevChar = i > 0 ? content[i - 1] : ''; | |
| // Track string literals (simple handling of " and ') | |
| if ((char === '"' || char === "'") && prevChar !== '\\') { | |
| if (!inString) { | |
| inString = true; | |
| stringChar = char; | |
| } else if (char === stringChar) { | |
| inString = false; | |
| stringChar = ''; | |
| } | |
| // Track string literals (simple handling of " and ') | |
| if (char === '"' || char === "'") { | |
| // Determine if this quote character is escaped by counting | |
| // the number of consecutive backslashes immediately before it. | |
| let backslashCount = 0; | |
| for (let j = i - 1; j >= 0 && content[j] === '\\'; j--) { | |
| backslashCount++; | |
| } | |
| const isEscaped = backslashCount % 2 === 1; | |
| if (!isEscaped) { | |
| if (!inString) { | |
| inString = true; | |
| stringChar = char; | |
| } else if (char === stringChar) { | |
| inString = false; | |
| stringChar = ''; | |
| } | |
| } |
| * while keeping ETag headers compact (full 64-char hash is overkill for this use case) | ||
| */ | ||
| private generateETag(content: string): string { | ||
| const hash = createHash('sha256').update(content).digest('hex').substring(0, 32); |
There was a problem hiding this comment.
While the truncation rationale is documented in the comment above, the magic number 32 could be extracted as a named constant (e.g., ETAG_HASH_LENGTH) to improve code maintainability and make the truncation length easier to adjust if requirements change.
| * while keeping ETag headers compact (full 64-char hash is overkill for this use case) | ||
| */ | ||
| private generateETag(content: string): string { | ||
| const hash = createHash('sha256').update(content).digest('hex').substring(0, 32); |
There was a problem hiding this comment.
This ETag generation logic is duplicated between filesystem-loader.ts and metadata-manager.ts. Consider extracting it to a shared utility function to maintain consistency and reduce duplication.
Metadata loading/saving is scattered across
objectql(SchemaRegistry),runtime(AppPlugin), andclient-react(hooks) with no unified persistence layer. This PR implements a complete@objectstack/metadatapackage at Layer 3 following comprehensive architectural evaluation.Architecture Evaluation
Evaluated placement options against microkernel constraints and recommended creating
@objectstack/metadata(Layer 3) for centralized metadata I/O, serialization, and persistence.Rejected alternatives:
spec- violates zero-runtime-deps constraintcore- kernel must stay domain-agnosticobjectql- mixing query execution with I/O violates SRPruntime- already ambiguous, would worsen boundariescli- wrong layer, creates upward dependencyProtocol Definition
Added complete Zod schemas for metadata operations in
packages/spec/src/system/metadata-loader.zod.ts:Covers: load/save options, serialization formats (JSON/YAML/TS), caching (ETag), file watching, import/export with conflict resolution.
Implementation Complete ✅
Phase 1: Core Infrastructure - Fully implemented and production-ready
Created
packages/metadata/with complete implementation:Core Components
Security Hardening
Key Features
Usage Example
Package Design
Dependencies:
Ready for integration with:
cli- code generation commandsruntime- manifest loading at startupobjectql- SchemaRegistry persistenceDeliverables
Documentation
content/docs/developers/metadata-management.mdx)packages/spec/src/system/metadata-loader.{zod,test}.ts)ARCHITECTURE.mdandPACKAGE-DEPENDENCIES.md元数据管理架构评估报告.md)Implementation
packages/metadata/)Security Review
Completed 5 rounds of security-focused code review:
Next Steps
Status
✅ Production Ready - Core functionality complete and security-hardened
✅ Well-Tested - Unit tests for all serializers
✅ Documented - Clear API documentation and design rationale
✅ Zero Breaking Changes - Fully additive implementation
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.