-
-
Couldn't load subscription status.
- Fork 126
merge dev to main (v2.14.0) #2091
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: Yiming <yiming@whimslab.io>
📝 WalkthroughWalkthroughThe changes introduce enhanced support for custom type definitions and JSON fields in both OpenAPI schema generation and the REST API server. The OpenAPI generators now ensure output directories exist before writing files and can generate schemas for user-defined type definitions, handling their fields appropriately. The REST API server's type coercion logic is unified and extended to parse JSON for fields representing type definitions or JSON types, with error handling for invalid input. Corresponding tests are updated and expanded to cover these new capabilities, including filtering on JSON fields and handling invalid JSON input. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant REST_API_Server
participant Prisma
Client->>REST_API_Server: Send request with filter (JSON/type def field)
REST_API_Server->>REST_API_Server: Coerce filter value (parse JSON if needed)
alt Valid JSON
REST_API_Server->>Prisma: Query with parsed filter value
Prisma-->>REST_API_Server: Return result
REST_API_Server-->>Client: Respond with data
else Invalid JSON
REST_API_Server-->>Client: Respond with 400 error (invalid-value)
end
sequenceDiagram
participant OpenAPI_Generator
participant FileSystem
OpenAPI_Generator->>FileSystem: Ensure output directory exists
OpenAPI_Generator->>OpenAPI_Generator: Generate schema for models, enums, type definitions
OpenAPI_Generator->>FileSystem: Write OpenAPI spec file (YAML/JSON)
Possibly related PRs
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (7)
packages/server/src/api/rest/index.ts (2)
1750-1750: TODO for inequality filters noted.The TODO comment accurately identifies an area for potential future enhancement. Consider creating a separate issue to track the implementation of inequality filters for JSON fields.
1358-1358: Consider refactoring to avoid spread on accumulators.The static analysis tool has correctly identified potential performance issues with using spread syntax on accumulators in reduce operations, which can lead to O(n²) time complexity.
-return idFields.reduce( - (acc, curr, idx) => ({ - ...acc, - [curr.name]: this.coerce(curr, decodedId.split(this.idDivider)[idx]), - }), - {} -); +const result = {}; +idFields.forEach((curr, idx) => { + result[curr.name] = this.coerce(curr, decodedId.split(this.idDivider)[idx]); +}); +return result;Also applies to: 1389-1389
🧰 Tools
🪛 Biome (1.9.4)
[error] 1358-1358: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
packages/server/tests/api/rest.test.ts (2)
25-28: Define additional field constraints if needed.Currently,
type Addressonly hascitywithout additional validations (e.g., max length). Consider whether you need to enforce constraints or mark it as optional/required within the test schema.
437-438: Good addition for JSON data testing.Storing
{ city: 'Seattle' }underaddressand'foo'undersomeJsonensures coverage of JSON-typed fields. You may want to test more complex or nested structures in future tests.packages/plugins/openapi/src/rest-generator.ts (3)
94-96: Ensure directory creation error handling.Using
fs.mkdirSync(path.dirname(output), { recursive: true })is good for guaranteeing the output folder existence. Consider wrapping it in a try/catch or adding logs for potential I/O issues (e.g., insufficient permissions).try { fs.mkdirSync(path.dirname(output), { recursive: true }); } catch (err) { + console.error('Failed to create directory:', err); }
805-815: Consider specifying required fields in type definitions.
generateTypeDefComponentcreates an object schema but does not mark any fields as required. If your type definitions enforce required fields, consider adding logic to identify and mark them as required.
989-991: Potential consistency check for JSON fields.
fieldTypeToOpenAPISchemayields an empty{}schema for'Json', whereas the filter parameters use{ type: 'string', format: 'json' }. You might align these for clarity and consistent documentation..with('Json', () => ({ - /* possibly empty */ + type: 'string', + format: 'json' }))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
packages/plugins/openapi/tests/baseline/rest-type-coverage-3.0.0.baseline.yamlis excluded by!**/*.yamlpackages/plugins/openapi/tests/baseline/rest-type-coverage-3.1.0.baseline.yamlis excluded by!**/*.yamlpackages/plugins/trpc/tests/projects/nuxt-trpc-v11/package.jsonis excluded by!**/*.json
📒 Files selected for processing (5)
packages/plugins/openapi/src/rest-generator.ts(7 hunks)packages/plugins/openapi/src/rpc-generator.ts(1 hunks)packages/plugins/openapi/tests/openapi-restful.test.ts(2 hunks)packages/server/src/api/rest/index.ts(8 hunks)packages/server/tests/api/rest.test.ts(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
packages/plugins/openapi/src/rpc-generator.ts (1)
packages/schema/build/bundle.js (1)
fs(4-4)
packages/server/src/api/rest/index.ts (1)
packages/runtime/src/cross/model-meta.ts (1)
FieldInfo(26-107)
packages/plugins/openapi/src/rest-generator.ts (3)
packages/schema/build/bundle.js (1)
fs(4-4)packages/server/tests/utils.ts (1)
schema(3-29)packages/schema/src/plugins/prisma/prisma-builder.ts (1)
Enum(319-355)
🪛 Biome (1.9.4)
packages/server/src/api/rest/index.ts
[error] 1358-1358: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 1389-1389: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: OSSAR-Scan
- GitHub Check: build-test (20.x)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (11)
packages/plugins/openapi/src/rpc-generator.ts (1)
97-99: Good addition - ensures output directory exists.Adding this directory creation logic is a solid improvement that aligns with best practices. It creates all necessary parent directories before writing the specification file, preventing potential failures when output paths include non-existent directories.
packages/server/src/api/rest/index.ts (3)
1439-1447: Improved JSON handling in the coerce method.The updated
coercemethod now properly handles JSON values and type definitions by accepting the fullFieldInfoobject. This change enables proper parsing of JSON strings and appropriate error handling for invalid JSON input.
1352-1352: Correctly modified coerce method calls to pass FieldInfo.All calls to the
coercemethod have been updated to pass the completeFieldInfoobject instead of just the type string, which is consistent with the method signature change. This ensures proper type handling throughout the codebase.Also applies to: 1359-1359, 1368-1368, 1384-1384, 1390-1390, 1771-1771, 1780-1780, 1799-1799
1790-1793: Added JSON field equality filter support.This addition enables equality filtering on JSON and type definition fields by parsing and comparing the JSON values. This completes the integration of JSON field support in the filtering mechanism.
packages/plugins/openapi/tests/openapi-restful.test.ts (2)
309-311: Good addition of type definition for testing.The new
Metatype definition provides a good test case for custom type definition support in the OpenAPI schema generator.
324-325: Added JSON field test coverage.The addition of both a custom type JSON field with
@jsonannotation and a plainJsonfield provides comprehensive test coverage for the JSON handling capabilities added to the OpenAPI generator.packages/server/tests/api/rest.test.ts (2)
37-38: No issues with the new JSON fields.Defining
address Address? @jsonandsomeJson Json?is a valid way to test custom and raw JSON fields. The optional modifier is consistent with your test scenarios.
734-782: Comprehensive JSON field filtering tests.These tests thoroughly validate equality filters on typedef fields (
address) and raw JSON fields (someJson), including handling of invalid JSON. This suits the new JSON parsing logic well.packages/plugins/openapi/src/rest-generator.ts (3)
15-27: New imports for type definitions look appropriate.Bringing in
TypeDef,TypeDefField,TypeDefFieldType, andisTypeDefis necessary for supporting custom type definitions in schema generation.
560-564: Generating schemas for type definitions.Adding schemas for
TypeDefobjects ensures custom types are included in the OpenAPI spec. Verify that nested type definitions (if any) are also resolved.
982-988: Expanded parameter type for fields.The
generateFieldmethod now accepts eitherDataModelFieldorTypeDefField, enabling unified treatment of model fields and type definition fields. No issues found.
No description provided.