feat: suport mockserver file as database and add dsl-genenrator skill#1792
Open
hexqi wants to merge 13 commits into
Open
feat: suport mockserver file as database and add dsl-genenrator skill#1792hexqi wants to merge 13 commits into
hexqi wants to merge 13 commits into
Conversation
… mockserver create page error
This commit fixes 4 critical issues identified in commit 6acc71e: 1. [P0] ESM/CJS interop crash - Convert StoreAdapter.js from ES6 export to CommonJS - Convert NedbStore.js from ES6 imports/exports to CommonJS - Fixes startup crash caused by module format mismatch 2. [P0] StoreFactory constructor arguments - Pass config.fileDbPath to FileStore constructor - Pass filename in options object to NedbStore constructor - Import getDatabasePath utility 3. [P0] NedbStore double $set wrapping - Remove extra $set wrapping in update method - Pass update operators directly to NeDB 4. [P1] Unique constraint configuration - Align on 'indexes' array format across all stores - Extract unique fields from indexes in FileStore - Process indexes array directly in NedbStore All fixes verified with startup test.
…add initial mock data.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds a file-based storage adapter ( ChangesMockServer File-Based Storage Mode
TinyEngine DSL Generator AI Skill
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
mockServer/src/store/FileStore.jsParsing error: Unexpected token ... mockServer/test/store/adapter.contract.test.jsParsing error: Unexpected token ... Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
hexqi
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mockServer/src/services/pages.js (1)
106-109:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
this.dbis undefined—causes runtime crash.The
create()method at line 106 still referencesthis.db.findOneAsync(), but the constructor now initializesthis.storeinstead ofthis.db. This will throw aTypeError: Cannot read properties of undefined (reading 'findOneAsync')when attempting to create a page.Proposed fix
- const existing = await this.db.findOneAsync({ + const existing = await this.store.findOne({ app: pageData.app.toString(), route: pageData.route })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mockServer/src/services/pages.js` around lines 106 - 109, The create() method is referencing this.db.findOneAsync() but the constructor now initializes this.store instead of this.db, causing a runtime crash. Replace all instances of this.db with this.store in the create() method to ensure it uses the correct property that was initialized in the constructor.
🧹 Nitpick comments (2)
mockServer/data/pages/CreateVm.json (1)
119-119: 🧹 Nitpick | 🔵 Trivial | 💤 Low valuePre-existing data artifact: Incorrect inline style value.
Line 119 contains a serialization error:
"style": "background-color: [object Event];"where[object Event]is a literal string instead of a color value. This will render as invalid CSS but won't break file loading. This appears to be a pre-existing artifact in the mock data, not introduced by these changes.If desired, fix the style to a valid color value:
- "style": "background-color: [object Event]; color: `#8a8e99`; font-size: 12px;" + "style": "background-color: `#f5f5f5`; color: `#8a8e99`; font-size: 12px;"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mockServer/data/pages/CreateVm.json` at line 119, The style property at line 119 in the CreateVm.json mock data file contains an invalid CSS value "[object Event]" as a literal string in the background-color field. Replace this "[object Event]" string with a valid CSS color value (such as a hex code like `#cccccc` or a named color like gray) to ensure the style renders correctly as valid CSS.mockServer/scripts/export-db-to-file.js (1)
82-102: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueEnsure error handling in parseDbFile reports problematic line content.
When JSON parsing fails (line 92), the error message includes line number and parse error, but the actual malformed content is not shown. For large
.dbfiles, this can make debugging harder. Consider logging a snippet of the failed line.📝 Optional: Log malformed line content
.map((line, index) => { try { return JSON.parse(line) } catch (error) { - throw new Error(`Failed to parse ${path.basename(dbPath)} line ${index + 1}: ${error.message}`) + const snippet = line.length > 80 ? line.slice(0, 80) + '...' : line + throw new Error(`Failed to parse ${path.basename(dbPath)} line ${index + 1}: ${error.message}\nContent: ${snippet}`) } })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mockServer/scripts/export-db-to-file.js` around lines 82 - 102, In the parseDbFile function, when JSON.parse fails in the map callback (around the catch block), the error message currently includes only the line number and parse error message, but not the actual malformed content. Modify the error thrown in the catch block to include the problematic line content by appending the line variable to the error message, which will make it easier to debug malformed JSON entries in large database files.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/tinyengine-dsl-generator/scripts/check_css.py:
- Around line 200-206: In the finally block where os.unlink(temp_file) is
called, replace the bare except clause (line 204-205) with a specific exception
type such as OSError to avoid silently catching critical exceptions like
KeyboardInterrupt and SystemExit. Instead of using pass, log the error to help
with debugging. Add appropriate logging (using the same logger used elsewhere in
the file) to record when the temporary file deletion fails, providing context
about what went wrong during cleanup.
- Around line 114-141: Replace the bare Exception catch clause in the _check_css
method of the TinyCSS2Checker class with a specific exception type that tinycss2
raises when parsing fails. Research the tinycss2 module to determine the exact
exception type it raises (such as tinycss2.ParseError or similar), then update
the except clause to catch only that specific exception type instead of
Exception. This prevents masking unexpected errors that indicate bugs in the
code.
In @.claude/skills/tinyengine-dsl-generator/scripts/check_event_bindings.py:
- Around line 45-68: The _check_event_bindings method only validates event
bindings at the top level of the node dictionary, but TinyEngine events are
typically stored under a props key within the node. Modify the method to also
check for event bindings within node['props'] if it exists. Extract the event
validation logic (the loops checking event_keys and keys starting with 'on')
into a helper routine or duplicate it to validate both the node itself and the
node's props object, ensuring that _check_event_value is called for any valid
event bindings found in either location.
In @.claude/skills/tinyengine-dsl-generator/scripts/validate_page.py:
- Around line 95-100: The bare except clause on line 98 catches all exceptions
indiscriminately and returns True, which masks unexpected runtime errors as
successful validation. Replace the bare except with specific exception handling
that only catches JSON-related errors like json.JSONDecodeError or
FileNotFoundError, allowing other unexpected exceptions to propagate so they can
be properly debugged and addressed instead of being silently treated as pass
conditions.
In @.claude/skills/tinyengine-dsl-generator/SKILL.md:
- Around line 181-187: The page wrapper example contradicts the App ID Format
checklist by showing the app field as a string ("1") instead of an integer.
Locate the page wrapper example section that contains the app field definition
and change the app value from a string format to an integer format (remove
quotes around the numeric value) to align with the integer contract specified in
the App ID Format checklist and match the validation rules in validate_page.py.
In `@mockServer/data/appsSchema/16.json`:
- Around line 863-867: The Text component with the text "温馨提示：页面左上角切换区域" has an
invalid CSS value in its style property where background-color is set to [object
Event]. Replace [object Event] with a valid CSS color value (such as a hex color
code like `#ffffff` or a named color) to ensure the style renders correctly in the
UI.
- Around line 1700-1717: The occupier object in the mock data contains sensitive
account-identifying fields like email and confirmationToken that should not be
committed to the repository. Remove or redact these sensitive fields by
replacing their actual values with placeholder or dummy values that do not
contain real identifying information. Review all credential and account-related
fields within the occupier object (such as email, confirmationToken, password,
confirmed, blocked, and block) and ensure none contain actual account data
before committing.
In `@mockServer/scripts/export-db-to-file.js`:
- Around line 10-16: The collectionNamingFields object has the apps entry
configured with ['name'] but the apps service defines namingFields as ['id'],
causing a mismatch that could lead to file naming collisions. Update the apps
entry in the collectionNamingFields object to use ['id'] to align with the apps
service configuration, or if both naming fields should be supported, update it
to include both fields in the appropriate order that matches the service
definition.
In `@mockServer/src/services/apps.js`:
- Line 88: The async `insert()` operations in the create method are not being
awaited, which can cause race conditions. Add the `await` keyword before both
`this.store.insert(newApp)` at line 88 and
`this.schemaStore.insert(newAppSchema)` at line 105-106 to ensure the documents
are persisted to the database before the method continues execution. This will
guarantee that subsequent operations can reliably access the newly created app
data.
In `@mockServer/src/services/block.js`:
- Around line 78-81: The list() method accepts an appId parameter but does not
use it when calling this.store.find(), causing the method to return all blocks
regardless of the specified app. Either pass appId as a query filter to
this.store.find() to retrieve only blocks for that specific app, or remove the
unused appId parameter from the method signature if filtering by app is not
intended. Choose the approach based on whether blocks should be app-specific or
globally accessible.
In `@mockServer/src/services/blockCategory.js`:
- Around line 58-61: The find() method in the blockCategory service accepts a
params argument but ignores it when calling this.store.find(). To fix this, pass
the params argument to the this.store.find() call so that any filtering
parameters are properly applied to the query instead of returning all documents
unconditionally.
In `@mockServer/src/store/FileStore.js`:
- Around line 317-318: The FileStore.js is defaulting multi-record operations to
true by using options.multi !== false logic, which causes all matching records
to be updated/deleted by default unless explicitly disabled. This diverges from
the NeDB-backed adapter behavior which defaults to single-record operations.
Change the logic on line 317 from options.multi !== false to options.multi ===
true so that multi-record operations only occur when explicitly requested. Apply
the same fix to line 367 where this pattern is repeated, ensuring consistent
opt-in behavior for multiple record mutations across all operations.
- Around line 184-220: The query operator matching logic in FileStore.js uses an
if/else if chain to evaluate operators like $regex, $ne, $in, $nin, $gt, $gte,
$lt, and $lte, which means only the first matching operator is checked. Convert
this if/else if chain to individual if statements so that all operators on a
single field are evaluated. This ensures compound queries like { age: { $gte:
18, $lte: 30 } } properly validate all conditions instead of stopping after the
first operator match.
In `@mockServer/src/store/NedbStore.js`:
- Around line 41-46: The update and remove methods in NedbStore are returning
documents instead of counts, which violates the shared store contract that
StoreAdapter and FileStore follow. Modify the update method to return the count
of updated documents instead of the result document, and modify the remove
method to return the count of removed documents instead of the pre-delete
document. Both methods should return a number representing the affected document
count to match the behavior defined in the shared store contract.
---
Outside diff comments:
In `@mockServer/src/services/pages.js`:
- Around line 106-109: The create() method is referencing this.db.findOneAsync()
but the constructor now initializes this.store instead of this.db, causing a
runtime crash. Replace all instances of this.db with this.store in the create()
method to ensure it uses the correct property that was initialized in the
constructor.
---
Nitpick comments:
In `@mockServer/data/pages/CreateVm.json`:
- Line 119: The style property at line 119 in the CreateVm.json mock data file
contains an invalid CSS value "[object Event]" as a literal string in the
background-color field. Replace this "[object Event]" string with a valid CSS
color value (such as a hex code like `#cccccc` or a named color like gray) to
ensure the style renders correctly as valid CSS.
In `@mockServer/scripts/export-db-to-file.js`:
- Around line 82-102: In the parseDbFile function, when JSON.parse fails in the
map callback (around the catch block), the error message currently includes only
the line number and parse error message, but not the actual malformed content.
Modify the error thrown in the catch block to include the problematic line
content by appending the line variable to the error message, which will make it
easier to debug malformed JSON entries in large database files.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ce6d219d-26c7-4ccd-b4e0-3c98a22cf5fb
📒 Files selected for processing (32)
.claude/skills/tinyengine-dsl-generator/SKILL.md.claude/skills/tinyengine-dsl-generator/references/components.md.claude/skills/tinyengine-dsl-generator/references/patterns.md.claude/skills/tinyengine-dsl-generator/references/protocol.md.claude/skills/tinyengine-dsl-generator/scripts/check_css.py.claude/skills/tinyengine-dsl-generator/scripts/check_event_bindings.py.claude/skills/tinyengine-dsl-generator/scripts/validate_all.sh.claude/skills/tinyengine-dsl-generator/scripts/validate_dsl.py.claude/skills/tinyengine-dsl-generator/scripts/validate_page.pymockServer/data/apps/dashboard.jsonmockServer/data/apps/portal-app.jsonmockServer/data/appsSchema/1.jsonmockServer/data/appsSchema/16.jsonmockServer/data/blockCategories/我的分类.jsonmockServer/data/blockGroups/我的区块.jsonmockServer/data/blocks/PortalBlock.jsonmockServer/data/blocks/PortalHome.jsonmockServer/data/pages/CreateVm.jsonmockServer/gulpfile.jsmockServer/package.jsonmockServer/scripts/export-db-to-file.jsmockServer/src/config/config.jsmockServer/src/services/apps.jsmockServer/src/services/block.jsmockServer/src/services/blockCategory.jsmockServer/src/services/blockGroup.jsmockServer/src/services/pages.jsmockServer/src/store/FileStore.jsmockServer/src/store/NedbStore.jsmockServer/src/store/StoreAdapter.jsmockServer/src/store/StoreFactory.jspackage.json
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
mockServer/src/services/apps.js (1)
121-127: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider adding rollback for consistency with
create().If
this.schemaStore.remove()fails afterthis.store.remove()succeeds, the app record is deleted but the orphaned schema record remains. This is the inverse of the rollback scenario handled increate(). For a mock server this is low-risk, but for symmetry you could wrap in try/catch to restore the app on schema-removal failure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mockServer/src/services/apps.js` around lines 121 - 127, In the delete method, add try/catch error handling around the store removal operations to implement rollback consistency. Specifically, wrap the this.store.remove() and this.schemaStore.remove() calls in a try block, and if schemaStore.remove() fails, catch the error and restore the deleted app record by reinserting the result object back into this.store before re-throwing the error. This ensures that if schema removal fails, the app record is restored to maintain data consistency rather than leaving an orphaned schema record.mockServer/test/store/apps.rollback.test.js (1)
49-51: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAvoid hardcoding the generated app id in the rollback assertion.
This test is tightly coupled to the current
AppsService.create()seed (3). If id generation changes, this test can fail despite correct rollback behavior. Derive the id from the inserted record (or assert remove was called with the created id) to keep this regression test stable.Suggested test hardening
- // appList is empty in a fresh service, so the generated id falls back to 3. - const generatedId = 3 + let removedId ... - const removeSpy = jest.spyOn(service.store, 'remove') + const removeSpy = jest.spyOn(service.store, 'remove').mockImplementation(async (query, options) => { + removedId = query.id + return jest.requireActual('../../src/store/FileStore').prototype.remove.call(service.store, query, options) + }) ... - expect(removeSpy).toHaveBeenCalledWith({ id: generatedId }) + expect(removeSpy).toHaveBeenCalledWith(expect.objectContaining({ id: expect.any(Number) })) ... - const leftover = await service.store.findOne({ id: generatedId }) + const leftover = await service.store.findOne({ id: removedId })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mockServer/test/store/apps.rollback.test.js` around lines 49 - 51, Remove the hardcoded `generatedId = 3` assignment and instead capture the actual id from the inserted record returned by the AppsService.create() operation. Use this dynamically captured id in the rollback assertion instead of the hardcoded value, so the test verifies correct rollback behavior regardless of the current seed value or any future changes to id generation logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@mockServer/src/services/apps.js`:
- Around line 121-127: In the delete method, add try/catch error handling around
the store removal operations to implement rollback consistency. Specifically,
wrap the this.store.remove() and this.schemaStore.remove() calls in a try block,
and if schemaStore.remove() fails, catch the error and restore the deleted app
record by reinserting the result object back into this.store before re-throwing
the error. This ensures that if schema removal fails, the app record is restored
to maintain data consistency rather than leaving an orphaned schema record.
In `@mockServer/test/store/apps.rollback.test.js`:
- Around line 49-51: Remove the hardcoded `generatedId = 3` assignment and
instead capture the actual id from the inserted record returned by the
AppsService.create() operation. Use this dynamically captured id in the rollback
assertion instead of the hardcoded value, so the test verifies correct rollback
behavior regardless of the current seed value or any future changes to id
generation logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 158f71b8-50b5-4be4-99d6-f21f24e15bab
📒 Files selected for processing (16)
.agents/skills/tinyengine-dsl-generator/SKILL.md.agents/skills/tinyengine-dsl-generator/scripts/check_css.py.agents/skills/tinyengine-dsl-generator/scripts/check_event_bindings.py.agents/skills/tinyengine-dsl-generator/scripts/validate_page.py.gitignoremockServer/data/appsSchema/16.jsonmockServer/data/pages/CreateVm.jsonmockServer/scripts/export-db-to-file.jsmockServer/src/services/apps.jsmockServer/src/services/block.jsmockServer/src/services/blockCategory.jsmockServer/src/services/pages.jsmockServer/src/store/FileStore.jsmockServer/src/store/NedbStore.jsmockServer/test/store/adapter.contract.test.jsmockServer/test/store/apps.rollback.test.js
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- .agents/skills/tinyengine-dsl-generator/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (8)
- mockServer/data/appsSchema/16.json
- mockServer/src/services/block.js
- mockServer/scripts/export-db-to-file.js
- mockServer/src/services/pages.js
- mockServer/data/pages/CreateVm.json
- mockServer/src/services/blockCategory.js
- .agents/skills/tinyengine-dsl-generator/scripts/check_css.py
- mockServer/src/store/FileStore.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
English | 简体中文
PR
背景：当前各类软件提供CLI/MCP/Skill 让本地AI Agent能够快速接入使用已是大势所趋。 TinyEngine当前没有对外提供这类的工具或者能力，该PR主要 通过 标准Schema（JSON文件）做为桥梁，通过DSL Skill 来方便用户使用本地Agent工具来 快速生成页面/区块/应用，并同步到TinyEngine应用中。
主要特性：
使用方式：项目根目录执行 pnpm dev:file
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
dev:file) and a DB export command (export-db-to-file).data/*).