F/sprint24 #4
Merged
F/sprint24 #4
Conversation
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
Added .cursor/rules/cursorrules.mdc and .cursorignore for improved Cursor AI rules and ignore patterns. Removed legacy .cursorrules. Updated README.md with new architecture, build, and validation instructions. Added build.js for domain packaging. Updated and added supporting scripts and configuration files. Various updates to core schemas, tasks, workflows, and mocks for vNext domain structure and testing.
Replaces global vNext CLI commands with local npm scripts for validation and build steps in the GitHub Actions workflow. Removes global installation of vNext CLI and updates commands to use 'npm run validate' and 'npm run build' with appropriate arguments.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 23099936 | Triggered | Generic High Entropy Secret | c0b4930 | mockoon/migration-api.json | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
Reviewer's GuideThis PR turns the vnext-template into a configurable, self-contained template+CLI package with schema-aware validation and build tooling, replacing hard-coded paths and external vnext CLI usage, tightening C# workflow rules, and updating documentation and CI accordingly. Sequence diagram for JSON schema validation in validate.jssequenceDiagram
actor Developer
participant Npm as Npm_Scripts
participant Validate as validate_js
participant Template as VnextTemplateAPI
participant SchemaPkg as VnextSchemaPackage
participant Ajv as AjvValidator
participant FS as FileSystem
Developer->>Npm: run npm run validate
Npm->>Validate: node validate.js
Validate->>Template: require index.js
Validate->>Template: getDomainName()
Template-->>Validate: domainName
Validate->>Template: getPathsConfig()
Template-->>Validate: pathsConfig
Validate->>SchemaPkg: require @burgan-tech/vnext-schema
SchemaPkg-->>Validate: module_loaded
Validate->>SchemaPkg: getAvailableTypes()
SchemaPkg-->>Validate: availableTypes
loop each schema_directory
Validate->>SchemaPkg: getSchema(schemaType)
SchemaPkg-->>Validate: jsonSchema
Validate->>Ajv: compile(jsonSchema)
Ajv-->>Validate: validator
end
loop each JSON_file_in_domain
Validate->>FS: readFile(filePath)
FS-->>Validate: jsonContent
Validate->>Ajv: validator(jsonContent)
alt valid
Ajv-->>Validate: true
Validate->>Validate: record passed file
else invalid
Ajv-->>Validate: false
Validate->>Ajv: read errors
Validate->>Validate: formatErrorMessage(err, filePath)
Validate->>Validate: findErrorLineNumber(filePath, err)
Validate->>Validate: accumulate failed file and stats
end
end
alt any_schema_errors
Validate-->>Npm: exit with error
Npm-->>Developer: show failed files and statistics
else no_schema_errors
Validate-->>Npm: exit 0
Npm-->>Developer: show success and statistics
end
Class diagram for updated vnext-template runtime API and toolingclassDiagram
class VnextTemplateAPI {
+getDomainConfig()
+getPathsConfig()
+getSchemas()
+getWorkflows()
+getTasks()
+getViews()
+getFunctions()
+getExtensions()
+getAvailableTypes()
+getDomainName()
+getComponentPath(componentType)
}
class PathsConfig {
+componentsRoot : string
+schemas : string
+workflows : string
+tasks : string
+views : string
+functions : string
+extensions : string
}
class ValidateScript {
+validate(description, validationFunction)
+formatErrorMessage(err, filePath)
+findErrorLineNumber(filePath, err)
+parseErrorLocation(errorMessage)
+findLineNumberForPath(filePath, jsonPath)
+runSchemaValidation()
}
class BuildScript {
+build()
+parseArgs()
+loadConfig()
+getPathsConfig()
+findJsonFiles(dirPath, files)
+getAllFiles(dirPath, files)
+removeDir(dirPath)
+ensureDir(dirPath)
+copyFile(src, dest)
+writeJsonFile(filePath, data)
+showHelp()
}
class SetupScript {
+setup()
+loadConfig()
+getPathsConfig()
+isAlreadySetup()
+getDomainName()
+validateDomainName(domainName)
+promptDomainName()
+replaceInFile(filePath, domainName)
+replaceInDirectory(dirPath, domainName, processedFiles)
+renameDomainDirectory(domainName)
+isInstalledAsDependency()
}
class SchemaVersionSyncScript {
+syncSchemaVersion()
}
class AjvValidator {
+compile(schema)
+validate(data)
errors
}
class VnextSchemaPackage {
+getAvailableTypes()
+getSchema(type)
}
VnextTemplateAPI ..> PathsConfig : uses
ValidateScript ..> VnextTemplateAPI : loads_domain_and_paths
ValidateScript ..> AjvValidator : uses
ValidateScript ..> VnextSchemaPackage : loads_schemas
BuildScript ..> ValidateScript : invokes_validate_js
BuildScript ..> PathsConfig : uses
SetupScript ..> PathsConfig : uses
SchemaVersionSyncScript ..> VnextSchemaPackage : depends_on_version
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary of ChangesHello @yilmaztayfun, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request represents a substantial overhaul and enhancement of the vNext workflow project, transforming it from a template into a more robust and well-documented example. The core focus is on improving developer experience through automated setup, comprehensive build processes, and rigorous schema validation. It also standardizes API mocking practices and refines existing workflow definitions for clarity and correctness, setting a strong foundation for future development and understanding of the vNext ecosystem. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and found some issues that need to be addressed.
- In index.js,
getDomainConfignow returns only the paths configuration instead of the fullvnext.config.jsoncontents, which is a breaking behavioral change compared to the previous implementation; consider either restoring the original behavior or exposing a separategetConfigwhile keepinggetDomainConfigbackward compatible. - In
AccountCreationFailedRule.csxthe second failure check block usesConvert.ToString(errorCode)and compares it againstcriticalCategories, which looks like a copy‑paste bug and should likely usefailureCategoryfor the conversion and comparison instead. - In setup.js, several user-facing messages are hardcoded with the literal
$core(e.g., DOMAIN_NAME error and final summary) instead of interpolating the chosendomainName, which will confuse users and should be updated to use the actual domain value.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In index.js, `getDomainConfig` now returns only the paths configuration instead of the full `vnext.config.json` contents, which is a breaking behavioral change compared to the previous implementation; consider either restoring the original behavior or exposing a separate `getConfig` while keeping `getDomainConfig` backward compatible.
- In `AccountCreationFailedRule.csx` the second failure check block uses `Convert.ToString(errorCode)` and compares it against `criticalCategories`, which looks like a copy‑paste bug and should likely use `failureCategory` for the conversion and comparison instead.
- In setup.js, several user-facing messages are hardcoded with the literal `$core` (e.g., DOMAIN_NAME error and final summary) instead of interpolating the chosen `domainName`, which will confuse users and should be updated to use the actual domain value.
## Individual Comments
### Comment 1
<location> `core/Workflows/account-opening/src/AccountCreationFailedRule.csx:76-77` </location>
<code_context>
};
-
- failed = failed || failureCodes.Contains(errorCode.ToString());
+ string code = Convert.ToString(errorCode);
+ failed = failed || failureCodes.Contains(code, StringComparer.OrdinalIgnoreCase);
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Use the failure category variable when checking `criticalCategories` instead of `errorCode`.
In the failure-category branch, you’re still converting and checking `errorCode` instead of `failureCategory`, so the new `criticalCategories` logic never actually uses the category value and just repeats the error-code check. This block should parallel the error-code logic but operate on `failureCategory` when computing `code` and checking `criticalCategories.Contains(...)`.
</issue_to_address>
### Comment 2
<location> `setup.js:108` </location>
<code_context>
+ if (validateDomainName(domainName)) {
+ return domainName;
+ } else {
+ console.error(`❌ Invalid domain name in DOMAIN_NAME environment variable: $core`);
+ process.exit(1);
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** Hard-coded `$core` in error message should interpolate the actual invalid domain value.
In this and the other log lines using `"$core"`, the string is hard-coded instead of using the resolved `domainName`. This will produce misleading logs (and here, it hides the actual invalid value). Please interpolate `domainName` (or the correct variable) so the error messages reflect the real input.
</issue_to_address>
### Comment 3
<location> `setup.js:221-230` </location>
<code_context>
+
+ // Get domain name from command line or prompt
+ const domainName = await getDomainName();
+ console.log(`\n📝 Setting up domain: $core\n`);
+
+ // Replace in all files
+ console.log('🔄 Replacing core in files...');
+ const processedFiles = new Set();
+ replaceInDirectory('.', domainName, processedFiles);
+ console.log(` ✓ Processed ${processedFiles.size} file(s)`);
+
+ // Rename directory
+ console.log('\n📁 Renaming domain directory...');
+ renameDomainDirectory(domainName);
+
+ console.log('\n✅ Setup complete!');
+ console.log(`\nYour domain "$core" is now configured.`);
+ console.log('You can start adding your schemas, workflows, tasks, and other components.\n');
+}
</code_context>
<issue_to_address>
**issue:** User-facing logs in setup script always print `$core` instead of the chosen domain name.
These messages still print the literal string `$core` instead of the selected domain (e.g. `customer-domain`). Please interpolate `domainName` in the progress and completion logs so the output matches the user’s actual configuration.
</issue_to_address>
### Comment 4
<location> `test.js:63` </location>
<code_context>
}
- const expectedTypes = ['schemas', 'workflows', 'tasks', 'views', 'functions', 'extensions'];
+ const expectedTypes = ['Schemas', 'Workflows', 'Tasks', 'Views', 'Functions', 'Extensions'];
for (const type of expectedTypes) {
if (!types.includes(type)) {
</code_context>
<issue_to_address>
**issue (testing):** Add tests for new path-config-driven behavior and helper APIs, not just the default getAvailableTypes values.
The expectation now matches the default `pathsConfig`, but the tests still only validate the static default list. Please extend this suite to cover:
1. Custom `vnext.config.json` paths: use a temporary config with non-default `paths` and assert that `getAvailableTypes()` and `getPathsConfig()` reflect merged defaults + overrides.
2. `getComponentPath`: test known types resolve correctly from `componentsRoot` and config, plus an unknown type returning the expected fallback (e.g. `null`) without throwing.
3. `.meta` directories: verify loader helpers (e.g. `getSchemas`, `getWorkflows`) skip JSON files in `.meta` while loading normal ones.
4. Missing/malformed config: confirm behavior when `vnext.config.json` is absent or lacks `paths`, and that defaults are used without errors.
This will better exercise the new configuration-driven behavior and guard against regressions in path handling.
</issue_to_address>
### Comment 5
<location> `README.md:111` </location>
<code_context>
-**Returns:** `Object` - Key-value pairs of task names and definitions
+This will check:
+- Package.json structure and content
+- Main entry point functionality
+- vnext.config.json validation
</code_context>
<issue_to_address>
**nitpick (typo):** Use the correct lowercase filename `package.json` for consistency and accuracy.
Everywhere else this file is called `package.json`, and the filename on disk is lowercase. Updating this bullet to `package.json structure and content` will keep terminology consistent and avoid confusion.
```suggestion
- package.json structure and content
```
</issue_to_address>
### Comment 6
<location> `setup.js:38` </location>
<code_context>
const componentsRoot = pathsConfig.componentsRoot;
</code_context>
<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))
```suggestion
const {componentsRoot} = pathsConfig;
```
<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.
From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>
### Comment 7
<location> `setup.js:161-169` </location>
<code_context>
if (['.json', '.js', '.md', '.sh', '.txt', '.yml', '.yaml'].includes(ext) ||
entry.name === '.gitignore' ||
entry.name === '.gitattributes') {
if (!processedFiles.has(relativePath)) {
if (replaceInFile(fullPath, domainName)) {
processedFiles.add(relativePath);
}
}
}
</code_context>
<issue_to_address>
**suggestion (code-quality):** Merge nested if conditions ([`merge-nested-ifs`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/merge-nested-ifs))
```suggestion
if ((['.json', '.js', '.md', '.sh', '.txt', '.yml', '.yaml'].includes(ext) ||
entry.name === '.gitignore' ||
entry.name === '.gitattributes') && !processedFiles.has(relativePath)) {
if (replaceInFile(fullPath, domainName)) {
processedFiles.add(relativePath);
}
}
```
<br/><details><summary>Explanation</summary>Reading deeply nested conditional code is confusing, since you have to keep track of which
conditions relate to which levels. We therefore strive to reduce nesting where
possible, and the situation where two `if` conditions can be combined using
`and` is an easy win.
</details>
</issue_to_address>
### Comment 8
<location> `setup.js:164-168` </location>
<code_context>
if (!processedFiles.has(relativePath)) {
if (replaceInFile(fullPath, domainName)) {
processedFiles.add(relativePath);
}
}
</code_context>
<issue_to_address>
**suggestion (code-quality):** Merge nested if conditions ([`merge-nested-ifs`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/merge-nested-ifs))
```suggestion
if (!processedFiles.has(relativePath) && replaceInFile(fullPath, domainName)) {
processedFiles.add(relativePath);
}
```
<br/><details><summary>Explanation</summary>Reading deeply nested conditional code is confusing, since you have to keep track of which
conditions relate to which levels. We therefore strive to reduce nesting where
possible, and the situation where two `if` conditions can be combined using
`and` is an easy win.
</details>
</issue_to_address>
### Comment 9
<location> `sync-schema-version.js:11` </location>
<code_context>
const schemaVersion = vnextConfig.schemaVersion;
</code_context>
<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))
```suggestion
const {schemaVersion} = vnextConfig;
```
<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.
From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>
### Comment 10
<location> `validate.js:320-359` </location>
<code_context>
if (!inString) {
if (char === '{') {
braceDepth++;
} else if (char === '}') {
braceDepth--;
if (braceDepth < currentDepth) {
// Reset path tracking when exiting a level
if (pathIndex > 0 && braceDepth < pathIndex) {
pathIndex = Math.max(0, pathIndex - 1);
}
}
} else if (char === '[') {
bracketDepth++;
// Check if we're at an array index in the path
if (pathIndex < pathParts.length && /^\d+$/.test(pathParts[pathIndex])) {
const targetIndex = parseInt(pathParts[pathIndex], 10);
if (arrayIndex === targetIndex && bracketDepth === 1) {
pathIndex++;
if (pathIndex === pathParts.length) {
return lineNum + 1;
}
}
}
} else if (char === ']') {
bracketDepth--;
if (bracketDepth === 0) {
arrayIndex++;
}
} else if (char === ':' && currentKey) {
currentKey = '';
} else if (char.match(/[a-zA-Z0-9_]/) && !inString) {
if (i === 0 || line[i-1] === '"' || (i > 0 && line[i-1].match(/[^a-zA-Z0-9_]/))) {
currentKey += char;
}
}
} else {
if (char.match(/[a-zA-Z0-9_]/)) {
currentKey += char;
}
}
</code_context>
<issue_to_address>
**suggestion (code-quality):** Merge else clause's nested if statement into `else if` ([`merge-else-if`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/merge-else-if))
```suggestion
if (!inString) {
if (char === '{') {
braceDepth++;
} else if (char === '}') {
braceDepth--;
if (braceDepth < currentDepth) {
// Reset path tracking when exiting a level
if (pathIndex > 0 && braceDepth < pathIndex) {
pathIndex = Math.max(0, pathIndex - 1);
}
}
} else if (char === '[') {
bracketDepth++;
// Check if we're at an array index in the path
if (pathIndex < pathParts.length && /^\d+$/.test(pathParts[pathIndex])) {
const targetIndex = parseInt(pathParts[pathIndex], 10);
if (arrayIndex === targetIndex && bracketDepth === 1) {
pathIndex++;
if (pathIndex === pathParts.length) {
return lineNum + 1;
}
}
}
} else if (char === ']') {
bracketDepth--;
if (bracketDepth === 0) {
arrayIndex++;
}
} else if (char === ':' && currentKey) {
currentKey = '';
} else if (char.match(/[a-zA-Z0-9_]/) && !inString) {
if (i === 0 || line[i-1] === '"' || (i > 0 && line[i-1].match(/[^a-zA-Z0-9_]/))) {
currentKey += char;
}
}
}
else if (char.match(/[a-zA-Z0-9_]/)) {
currentKey += char;
}
```
<br/><details><summary>Explanation</summary>Flattening if statements nested within else clauses generates code that is
easier to read and expand upon.
</details>
</issue_to_address>
### Comment 11
<location> `validate.js:325-330` </location>
<code_context>
if (braceDepth < currentDepth) {
// Reset path tracking when exiting a level
if (pathIndex > 0 && braceDepth < pathIndex) {
pathIndex = Math.max(0, pathIndex - 1);
}
}
</code_context>
<issue_to_address>
**suggestion (code-quality):** Merge nested if conditions ([`merge-nested-ifs`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/merge-nested-ifs))
```suggestion
if (braceDepth < currentDepth && (pathIndex > 0 && braceDepth < pathIndex)) {
pathIndex = Math.max(0, pathIndex - 1);
}
```
<br/><details><summary>Explanation</summary>Reading deeply nested conditional code is confusing, since you have to keep track of which
conditions relate to which levels. We therefore strive to reduce nesting where
possible, and the situation where two `if` conditions can be combined using
`and` is an easy win.
</details>
</issue_to_address>
### Comment 12
<location> `validate.js:350-354` </location>
<code_context>
} else if (char.match(/[a-zA-Z0-9_]/) && !inString) {
if (i === 0 || line[i-1] === '"' || (i > 0 && line[i-1].match(/[^a-zA-Z0-9_]/))) {
currentKey += char;
}
}
</code_context>
<issue_to_address>
**suggestion (code-quality):** Merge nested if conditions ([`merge-nested-ifs`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/merge-nested-ifs))
```suggestion
} else if (char.match(/[a-zA-Z0-9_]/) && !inString && (i === 0 || line[i-1] === '"' || (i > 0 && line[i-1].match(/[^a-zA-Z0-9_]/)))) {
currentKey += char;
}
```
<br/><details><summary>Explanation</summary>Reading deeply nested conditional code is confusing, since you have to keep track of which
conditions relate to which levels. We therefore strive to reduce nesting where
possible, and the situation where two `if` conditions can be combined using
`and` is an easy win.
</details>
</issue_to_address>
### Comment 13
<location> `validate.js:381` </location>
<code_context>
let message = err.message;
</code_context>
<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))
```suggestion
let {message} = err;
```
<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.
From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+76
to
+77
| string code = Convert.ToString(errorCode); | ||
| failed = failed || failureCodes.Contains(code, StringComparer.OrdinalIgnoreCase); |
There was a problem hiding this comment.
issue (bug_risk): Use the failure category variable when checking criticalCategories instead of errorCode.
In the failure-category branch, you’re still converting and checking errorCode instead of failureCategory, so the new criticalCategories logic never actually uses the category value and just repeats the error-code check. This block should parallel the error-code logic but operate on failureCategory when computing code and checking criticalCategories.Contains(...).
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant set of improvements, including configurable domain paths, local build tooling, and greatly enhanced validation and documentation. The move towards a configuration-driven architecture in index.js and the addition of a comprehensive build.js script are excellent steps for maintainability and scalability. The new validation script also provides much more detailed feedback. My review includes a critical fix for a logic bug in a workflow rule, a high-severity security suggestion for the Mockoon upload script, and several medium-severity comments on maintainability and best practices in the build scripts and package configuration.
Comment on lines
+91
to
+92
| string code = Convert.ToString(errorCode); | ||
| failed = failed || criticalCategories.Contains(code, StringComparer.OrdinalIgnoreCase); |
There was a problem hiding this comment.
There appears to be a critical bug here. You are checking the errorCode (via the code variable) against criticalCategories, which is a list of failure categories, not error codes. This will cause the rule to behave incorrectly, as it will never find a match. You should be checking the failureCategory variable against this list.
string categoryCode = Convert.ToString(failureCategory);
failed = failed || criticalCategories.Contains(categoryCode, StringComparer.OrdinalIgnoreCase);
Comment on lines
+26
to
+29
| RESPONSE=$(curl -k -s -w "\n%{http_code}" -X POST \ | ||
| "$MOCKOON_URL/api/environments" \ | ||
| -H "Content-Type: application/json" \ | ||
| -d @"$JSON_FILE" 2>&1) |
There was a problem hiding this comment.
The script uses curl -k (or --insecure), which disables SSL/TLS certificate validation. This is a significant security risk, even for internal tools, as it exposes the connection to man-in-the-middle (MITM) attacks. It is strongly recommended to remove the -k flag and ensure proper certificate validation is performed. If you are using a self-signed certificate, you should use the --cacert option to provide a custom CA bundle to curl.
| - Use semantic versioning | ||
|
|
||
| 3. **Create Mockoon mocks** (for external APIs): | ||
| - Add all external API endpoints to `mockoon/banking-api-mocks.json` |
There was a problem hiding this comment.
There's an inconsistency in the mock API file name referenced in these rules. This line, along with lines 556 and 608, refers to mockoon/banking-api-mocks.json, but that file has been deleted and replaced with mockoon/migration-api.json. To avoid confusion for developers, please update all references to point to the new file name. Line 72 correctly uses the new name.
- Add all external API endpoints to `mockoon/migration-api.json`
Comment on lines
+232
to
+240
| try { | ||
| const { execSync } = require('child_process'); | ||
| execSync('node validate.js', { stdio: 'inherit' }); | ||
| console.log(colorize(' ✅ Validation completed successfully', 'green')); | ||
| } catch (error) { | ||
| console.log(colorize('\n❌ Build failed: Validation errors found', 'red')); | ||
| console.log(colorize(' Run "npm run validate" for detailed error information', 'dim')); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
The build script invokes validate.js using execSync. This creates a tight coupling between the two scripts, making them less modular and potentially brittle. A more robust approach would be to refactor the validation logic into an exported function within validate.js that can be imported and called directly from build.js. This would improve modularity, error handling, and performance by avoiding the overhead of spawning a new process.
For example:
validate.js
export function runValidation() { /* ... */ }build.js
import { runValidation } from './validate.js';
// ...
await runValidation();| "build:runtime": "node build.js -t runtime", | ||
| "sync-schema": "node sync-schema-version.js", | ||
| "setup": "node setup.js", | ||
| "postinstall": "node setup.js && npm run sync-schema", |
There was a problem hiding this comment.
The postinstall script runs setup.js, which can trigger an interactive prompt for the domain name. This can cause issues in non-interactive environments like CI/CD pipelines and may be an unexpected side-effect for developers simply running npm install.
While the script checks isInstalledAsDependency(), consider making this a manual, one-time command that the developer runs explicitly (e.g., npm run setup). The project's README.md could then instruct new users to run this command after cloning the repository.
Comment on lines
+54
to
+407
| // Helper function to find line number for a JSON path in a file | ||
| function findLineNumberForPath(filePath, jsonPath) { | ||
| try { | ||
| const fileContent = fs.readFileSync(filePath, 'utf8'); | ||
| const lines = fileContent.split('\n'); | ||
|
|
||
| // Parse the JSON path (e.g., "/attributes/states/0/transitions/1") | ||
| const pathParts = jsonPath.split('/').filter(part => part.length > 0); | ||
|
|
||
| if (pathParts.length === 0) { | ||
| return null; | ||
| } | ||
|
|
||
| // Parse JSON to get actual structure | ||
| let jsonData; | ||
| try { | ||
| jsonData = JSON.parse(fileContent); | ||
| } catch (e) { | ||
| // If JSON parsing fails, fall back to text search | ||
| return findLineNumberByTextSearch(lines, pathParts); | ||
| } | ||
|
|
||
| // Navigate through the JSON structure to find the path | ||
| let current = jsonData; | ||
| for (let i = 0; i < pathParts.length; i++) { | ||
| const part = pathParts[i]; | ||
|
|
||
| // Check if part is an array index | ||
| if (/^\d+$/.test(part)) { | ||
| const index = parseInt(part, 10); | ||
| if (Array.isArray(current) && index < current.length) { | ||
| current = current[index]; | ||
| } else { | ||
| return null; | ||
| } | ||
| } else if (current && typeof current === 'object' && part in current) { | ||
| current = current[part]; | ||
| } else { | ||
| // Path doesn't exist, try text search for the last part | ||
| return findLineNumberByTextSearch(lines, pathParts.slice(i)); | ||
| } | ||
| } | ||
|
|
||
| // Now find the line number where this value appears | ||
| return findValueLineNumber(lines, pathParts, jsonData); | ||
| } catch (error) { | ||
| // Fallback to text search | ||
| try { | ||
| const fileContent = fs.readFileSync(filePath, 'utf8'); | ||
| const lines = fileContent.split('\n'); | ||
| const pathParts = jsonPath.split('/').filter(part => part.length > 0); | ||
| return findLineNumberByTextSearch(lines, pathParts); | ||
| } catch (e) { | ||
| return null; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Helper to find line number by searching for property names in text | ||
| function findLineNumberByTextSearch(lines, pathParts) { | ||
| if (pathParts.length === 0) return null; | ||
|
|
||
| const targetProperty = pathParts[pathParts.length - 1]; | ||
|
|
||
| // For array indices, search for the parent property | ||
| if (/^\d+$/.test(targetProperty)) { | ||
| if (pathParts.length > 1) { | ||
| const parentProperty = pathParts[pathParts.length - 2]; | ||
| // Search for parent property and count array elements | ||
| let foundParent = false; | ||
| let arrayDepth = 0; | ||
| let elementIndex = 0; | ||
| const targetIndex = parseInt(targetProperty, 10); | ||
|
|
||
| for (let lineNum = 0; lineNum < lines.length; lineNum++) { | ||
| const line = lines[lineNum]; | ||
|
|
||
| if (line.includes(`"${parentProperty}"`)) { | ||
| foundParent = true; | ||
| } | ||
|
|
||
| if (foundParent) { | ||
| if (line.includes('[')) { | ||
| arrayDepth++; | ||
| } | ||
| if (line.includes(']')) { | ||
| arrayDepth--; | ||
| if (arrayDepth === 0 && elementIndex === targetIndex) { | ||
| return lineNum + 1; | ||
| } | ||
| if (arrayDepth === 0) { | ||
| elementIndex++; | ||
| } | ||
| } | ||
| if (arrayDepth > 0 && line.includes('{') && elementIndex === targetIndex) { | ||
| return lineNum + 1; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| // For regular properties, search for the property name | ||
| for (let lineNum = 0; lineNum < lines.length; lineNum++) { | ||
| const line = lines[lineNum]; | ||
| // Look for "propertyName": pattern | ||
| if (line.match(new RegExp(`"${targetProperty.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}"\\s*:`))) { | ||
| return lineNum + 1; | ||
| } | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| // Helper to find line number of a value in JSON structure | ||
| function findValueLineNumber(lines, pathParts, jsonData) { | ||
| // Navigate to the value | ||
| let current = jsonData; | ||
| for (const part of pathParts) { | ||
| if (/^\d+$/.test(part)) { | ||
| current = current[parseInt(part, 10)]; | ||
| } else { | ||
| current = current[part]; | ||
| } | ||
| } | ||
|
|
||
| // Search for the property name in the file | ||
| const targetProperty = pathParts[pathParts.length - 1]; | ||
|
|
||
| // For array indices, find the array element | ||
| if (/^\d+$/.test(targetProperty)) { | ||
| const parentProperty = pathParts[pathParts.length - 2]; | ||
| return findArrayElementLine(lines, parentProperty, parseInt(targetProperty, 10)); | ||
| } | ||
|
|
||
| // For regular properties, find the property line | ||
| for (let lineNum = 0; lineNum < lines.length; lineNum++) { | ||
| if (lines[lineNum].match(new RegExp(`"${targetProperty.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}"\\s*:`))) { | ||
| return lineNum + 1; | ||
| } | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| // Helper to find line number of an array element | ||
| function findArrayElementLine(lines, arrayProperty, elementIndex) { | ||
| let foundArray = false; | ||
| let bracketDepth = 0; | ||
| let currentIndex = 0; | ||
|
|
||
| for (let lineNum = 0; lineNum < lines.length; lineNum++) { | ||
| const line = lines[lineNum]; | ||
|
|
||
| // Find the array property | ||
| if (!foundArray && line.includes(`"${arrayProperty}"`)) { | ||
| foundArray = true; | ||
| continue; | ||
| } | ||
|
|
||
| if (foundArray) { | ||
| // Count brackets to track array depth | ||
| for (const char of line) { | ||
| if (char === '[') { | ||
| bracketDepth++; | ||
| if (bracketDepth === 1) { | ||
| // Start of array | ||
| continue; | ||
| } | ||
| } else if (char === ']') { | ||
| bracketDepth--; | ||
| if (bracketDepth === 0) { | ||
| // End of array | ||
| break; | ||
| } | ||
| } else if (bracketDepth === 1 && char === '{') { | ||
| // Found an object in the array | ||
| if (currentIndex === elementIndex) { | ||
| return lineNum + 1; | ||
| } | ||
| currentIndex++; | ||
| } | ||
| } | ||
|
|
||
| // Check if we're at the target index | ||
| if (bracketDepth === 1 && currentIndex === elementIndex && line.trim().startsWith('{')) { | ||
| return lineNum + 1; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| // Helper function to find line number for error in JSON file | ||
| function findErrorLineNumber(filePath, err) { | ||
| try { | ||
| const fileContent = fs.readFileSync(filePath, 'utf8'); | ||
| const lines = fileContent.split('\n'); | ||
|
|
||
| // Get the error path | ||
| const errPath = err.instancePath || err.dataPath || ''; | ||
|
|
||
| if (!errPath) { | ||
| // For root-level errors (like additionalProperty), search for the property | ||
| if (err.params && err.params.additionalProperty) { | ||
| const prop = err.params.additionalProperty; | ||
| for (let i = 0; i < lines.length; i++) { | ||
| if (lines[i].match(new RegExp(`"${prop.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}"\\s*:`))) { | ||
| return i + 1; | ||
| } | ||
| } | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| // Parse JSON path and find the line where the property appears | ||
| const pathParts = errPath.split('/').filter(part => part.length > 0); | ||
|
|
||
| if (pathParts.length === 0) return null; | ||
|
|
||
| // For nested paths, find the line where the target property/object appears | ||
| // Navigate through the path to find the actual line in the file | ||
| let currentDepth = 0; | ||
| let pathIndex = 0; | ||
| let inString = false; | ||
| let escapeNext = false; | ||
| let currentKey = ''; | ||
| let bracketDepth = 0; | ||
| let braceDepth = 0; | ||
| let arrayIndex = 0; | ||
| let foundPath = false; | ||
|
|
||
| for (let lineNum = 0; lineNum < lines.length; lineNum++) { | ||
| const line = lines[lineNum]; | ||
|
|
||
| for (let i = 0; i < line.length; i++) { | ||
| const char = line[i]; | ||
|
|
||
| if (escapeNext) { | ||
| escapeNext = false; | ||
| continue; | ||
| } | ||
|
|
||
| if (char === '\\') { | ||
| escapeNext = true; | ||
| continue; | ||
| } | ||
|
|
||
| if (char === '"' && !escapeNext) { | ||
| inString = !inString; | ||
| if (!inString && currentKey) { | ||
| // Check if this key matches our path | ||
| if (pathIndex < pathParts.length && currentKey === pathParts[pathIndex]) { | ||
| pathIndex++; | ||
| if (pathIndex === pathParts.length) { | ||
| // Found the target path | ||
| return lineNum + 1; | ||
| } | ||
| } | ||
| currentKey = ''; | ||
| } | ||
| continue; | ||
| } | ||
|
|
||
| if (!inString) { | ||
| if (char === '{') { | ||
| braceDepth++; | ||
| } else if (char === '}') { | ||
| braceDepth--; | ||
| if (braceDepth < currentDepth) { | ||
| // Reset path tracking when exiting a level | ||
| if (pathIndex > 0 && braceDepth < pathIndex) { | ||
| pathIndex = Math.max(0, pathIndex - 1); | ||
| } | ||
| } | ||
| } else if (char === '[') { | ||
| bracketDepth++; | ||
| // Check if we're at an array index in the path | ||
| if (pathIndex < pathParts.length && /^\d+$/.test(pathParts[pathIndex])) { | ||
| const targetIndex = parseInt(pathParts[pathIndex], 10); | ||
| if (arrayIndex === targetIndex && bracketDepth === 1) { | ||
| pathIndex++; | ||
| if (pathIndex === pathParts.length) { | ||
| return lineNum + 1; | ||
| } | ||
| } | ||
| } | ||
| } else if (char === ']') { | ||
| bracketDepth--; | ||
| if (bracketDepth === 0) { | ||
| arrayIndex++; | ||
| } | ||
| } else if (char === ':' && currentKey) { | ||
| currentKey = ''; | ||
| } else if (char.match(/[a-zA-Z0-9_]/) && !inString) { | ||
| if (i === 0 || line[i-1] === '"' || (i > 0 && line[i-1].match(/[^a-zA-Z0-9_]/))) { | ||
| currentKey += char; | ||
| } | ||
| } | ||
| } else { | ||
| if (char.match(/[a-zA-Z0-9_]/)) { | ||
| currentKey += char; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Fallback: search for the last property in the path | ||
| const lastProperty = pathParts[pathParts.length - 1]; | ||
| if (!/^\d+$/.test(lastProperty)) { | ||
| for (let i = 0; i < lines.length; i++) { | ||
| if (lines[i].includes(`"${lastProperty}"`)) { | ||
| return i + 1; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return null; | ||
| } catch (error) { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| // Helper function to format error message with better handling of additionalProperty | ||
| function formatErrorMessage(err, filePath = null) { | ||
| let message = err.message; | ||
| let lineNumber = null; | ||
|
|
||
| // Find line number in the JSON file where the error occurs | ||
| if (filePath) { | ||
| lineNumber = findErrorLineNumber(filePath, err); | ||
| } | ||
|
|
||
| // Handle "must NOT have additional properties" with better formatting | ||
| if (message.includes('must NOT have additional properties') && err.params && err.params.additionalProperty) { | ||
| const prop = err.params.additionalProperty; | ||
| message = `must NOT have additional property ${colorize(`"${prop}"`, 'yellow')}`; | ||
| } | ||
|
|
||
| // Handle "must have required property" with better formatting | ||
| if (message.includes('must have required property') && err.params && err.params.missingProperty) { | ||
| const prop = err.params.missingProperty; | ||
| message = `must have required property ${colorize(`"${prop}"`, 'yellow')}`; | ||
| } | ||
|
|
||
| // Add line number if found | ||
| if (lineNumber !== null) { | ||
| message += ` ${colorize(`(line ${lineNumber})`, 'dim')}`; | ||
| } | ||
|
|
||
| return message; | ||
| } |
There was a problem hiding this comment.
This file contains a substantial amount of complex logic for parsing JSON and finding error locations (e.g., findLineNumberForPath, findErrorLineNumber). This is effectively a partial reimplementation of a JSON parser, which can be difficult to maintain and may not cover all edge cases.
To improve robustness and simplify the code, consider using a library like jsonc-parser (from VS Code team) which can parse JSON with comments and provides detailed location information (line, column, path) for all nodes and errors. This would eliminate the need for manual parsing and make the validation logic much more reliable and easier to manage.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary by Sourcery
Introduce configurable domain paths, local build tooling, and enhanced validation and documentation for the vNext template/example package.
New Features:
Enhancements:
Build:
CI:
Documentation: