Skip to content

Conversation

@ymc9
Copy link
Member

@ymc9 ymc9 commented Apr 15, 2025

No description provided.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Apr 15, 2025

📝 Walkthrough

Walkthrough

The 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

File(s) Change Summary
packages/plugins/openapi/src/rest-generator.ts Expanded imports to include type definition support; ensured output directory creation; added schema generation for type definitions; unified handling of type definitions and JSON fields in OpenAPI schema generation; updated method signatures for field processing.
packages/plugins/openapi/src/rpc-generator.ts Ensured output directory exists before writing OpenAPI specification file.
packages/plugins/openapi/tests/openapi-restful.test.ts Added a new type definition (Meta) and extended the Foo model with JSON and type definition fields in the test schema.
packages/server/src/api/rest/index.ts Modified RequestHandler to accept full field metadata for coercion; added JSON parsing for type definition and JSON fields; updated filter and connect logic to use new coercion method; added error handling for invalid JSON.
packages/server/tests/api/rest.test.ts Added a new type (Address) and JSON fields to the User model; created test data and cases for filtering and error handling on JSON fields.

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
Loading
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)
Loading

Possibly related PRs

  • zenstackhq/zenstack#2088: Modifies the RequestHandler class to change the coerce method signature and add JSON parsing for type definition and JSON fields in filtering.
  • zenstackhq/zenstack#2090: Includes the directory creation fix in OpenAPI generators, similar to this PR, but does not add type definition support.
  • zenstackhq/zenstack#2089: Adds support for type definitions and JSON fields in OpenAPI schema generation, including method signature updates and new schema generation logic.
✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

‼️ IMPORTANT
Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 of O(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 Address only has city without 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' } under address and 'foo' under someJson ensures 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.

generateTypeDefComponent creates 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.

fieldTypeToOpenAPISchema yields 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

📥 Commits

Reviewing files that changed from the base of the PR and between bf9be5c and 82232d9.

⛔ Files ignored due to path filters (3)
  • packages/plugins/openapi/tests/baseline/rest-type-coverage-3.0.0.baseline.yaml is excluded by !**/*.yaml
  • packages/plugins/openapi/tests/baseline/rest-type-coverage-3.1.0.baseline.yaml is excluded by !**/*.yaml
  • packages/plugins/trpc/tests/projects/nuxt-trpc-v11/package.json is 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 coerce method now properly handles JSON values and type definitions by accepting the full FieldInfo object. 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 coerce method have been updated to pass the complete FieldInfo object 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 Meta type 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 @json annotation and a plain Json field 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? @json and someJson 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, and isTypeDef is necessary for supporting custom type definitions in schema generation.


560-564: Generating schemas for type definitions.

Adding schemas for TypeDef objects 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 generateField method now accepts either DataModelField or TypeDefField, enabling unified treatment of model fields and type definition fields. No issues found.

@ymc9 ymc9 merged commit 88f8c77 into main Apr 15, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants