-
-
Notifications
You must be signed in to change notification settings - Fork 111
merge dev to main (v2.14.2) #2113
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
📝 WalkthroughWalkthroughThis change introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant NestJS Controller
participant ApiHandlerService
participant PrismaClient
participant RequestHandler
Client->>NestJS Controller: Sends HTTP request
NestJS Controller->>ApiHandlerService: Calls handleRequest(options?)
ApiHandlerService->>PrismaClient: Get enhanced Prisma instance
ApiHandlerService->>ApiHandlerService: Load model metadata & schemas
ApiHandlerService->>RequestHandler: Invoke with request details, Prisma, metadata, schemas, logger
RequestHandler-->>ApiHandlerService: Returns response (body, status)
alt status >= 400
ApiHandlerService->>NestJS Controller: Throw HttpException with body and status
else status < 400
ApiHandlerService->>NestJS Controller: Return response body
end
NestJS Controller-->>Client: Sends HTTP response
Possibly related PRs
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 1
🧹 Nitpick comments (4)
packages/server/src/nestjs/api-handler.service.ts (2)
22-34
: Consider improving type safety for request body extractionThe service correctly loads assets, determines the request handler, and extracts request details. However, the request body extraction could be improved for better type safety.
- const requestBody = (this.request as { body: unknown }).body; + const requestBody = this.request && typeof this.request === 'object' && 'body' in this.request + ? (this.request as { body: unknown }).body + : undefined;
46-54
: Exception handling follows NestJS patternsThe error handling approach correctly integrates with NestJS's exception handling system by throwing an
HttpException
for status codes >= 400. This allows NestJS to generate appropriate error responses while also allowing callers to handle errors manually if needed.Consider using a more type-safe approach when casting the response body.
- throw new HttpException(response.body as Record<string, any>, response.status) + throw new HttpException( + typeof response.body === 'object' ? response.body : { message: String(response.body) }, + response.status + )packages/server/src/nestjs/interfaces/zenstack-module-options.interface.ts (2)
6-11
: Refine the return type ofgetEnhancedPrisma
The
getEnhancedPrisma
callback currently returnsunknown
, which weakens type safety for consumers. Consider specifying a more precise return type or making the interface generic, e.g.:getEnhancedPrisma<TClient = EnhancedPrismaClient>(model?: string | symbol): TClient;This ensures users benefit from proper IntelliSense and compile-time checks.
13-16
: Consider supporting additional module metadata
ZenStackModuleAsyncOptions
extends only theimports
property fromModuleMetadata
. If future use cases require customizingexports
,providers
, or other metadata, you could replacePick
with aPartial<ModuleMetadata>
or selectively include more properties to enhance flexibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (14)
package.json
is excluded by!**/*.json
packages/ide/jetbrains/package.json
is excluded by!**/*.json
packages/language/package.json
is excluded by!**/*.json
packages/misc/redwood/package.json
is excluded by!**/*.json
packages/plugins/openapi/package.json
is excluded by!**/*.json
packages/plugins/swr/package.json
is excluded by!**/*.json
packages/plugins/tanstack-query/package.json
is excluded by!**/*.json
packages/plugins/trpc/package.json
is excluded by!**/*.json
packages/runtime/package.json
is excluded by!**/*.json
packages/schema/package.json
is excluded by!**/*.json
packages/sdk/package.json
is excluded by!**/*.json
packages/server/package.json
is excluded by!**/*.json
packages/server/tsconfig.json
is excluded by!**/*.json
packages/testtools/package.json
is excluded by!**/*.json
📒 Files selected for processing (9)
packages/ide/jetbrains/build.gradle.kts
(1 hunks)packages/server/src/nestjs/api-handler.service.ts
(1 hunks)packages/server/src/nestjs/index.ts
(1 hunks)packages/server/src/nestjs/interfaces/api-handler-options.interface.ts
(1 hunks)packages/server/src/nestjs/interfaces/index.ts
(1 hunks)packages/server/src/nestjs/interfaces/zenstack-module-options.interface.ts
(1 hunks)packages/server/src/nestjs/zenstack.constants.ts
(1 hunks)packages/server/src/nestjs/zenstack.module.ts
(1 hunks)packages/server/tests/adapter/nestjs.test.ts
(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/server/src/nestjs/interfaces/api-handler-options.interface.ts (1)
packages/server/src/types.ts (1)
AdapterBaseOptions
(32-56)
packages/server/src/nestjs/api-handler.service.ts (4)
packages/server/src/nestjs/zenstack.constants.ts (1)
ENHANCED_PRISMA
(4-4)packages/runtime/src/types.ts (1)
DbClientContract
(91-93)packages/server/src/nestjs/interfaces/api-handler-options.interface.ts (1)
ApiHandlerOptions
(3-18)packages/server/src/shared.ts (1)
loadAssets
(5-21)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: OSSAR-Scan
- GitHub Check: dependency-review
- GitHub Check: OSSAR-Scan
🔇 Additional comments (15)
packages/server/src/nestjs/zenstack.constants.ts (1)
1-4
: Well-structured constant definition with proper documentationThe ENHANCED_PRISMA constant is well-defined with clear JSDoc documentation explaining its purpose as the default token for exporting the enhanced Prisma service. This centralization of constants improves code organization by moving it from local usage in zenstack.module.ts to a dedicated constants file.
packages/ide/jetbrains/build.gradle.kts (1)
12-12
: Version bump looks goodThis minor version bump (2.14.1 → 2.14.2) aligns with the introduction of new NestJS API handling features in the server package. The change follows proper versioning practices for maintaining backward compatibility.
packages/server/src/nestjs/index.ts (1)
1-3
: Clean module exports following good patternsThe additional exports provide a cleaner API surface by exposing the newly introduced ApiHandlerService and ENHANCED_PRISMA constant. This follows the principle of maintaining a well-organized public API through proper module exports.
packages/server/src/nestjs/interfaces/index.ts (1)
1-2
: Good use of barrel file pattern for interface exportsThis barrel file effectively consolidates exports from multiple interface files, simplifying imports for consumers of the library. It's a clean approach to maintaining modular code organization while providing a convenient import experience.
packages/server/src/nestjs/interfaces/api-handler-options.interface.ts (1)
3-18
: Well-documented interface with clear examplesThe
ApiHandlerOptions
interface extendingAdapterBaseOptions
is well-designed with comprehensive documentation that clearly explains the purpose and usage of thebaseUrl
property. The examples for both RPC and RESTful API handlers make it easy to understand how this property affects the routing paths.packages/server/src/nestjs/zenstack.module.ts (1)
1-3
: Clean refactoring to improve modularityThe changes to import constants and interfaces from dedicated modules rather than defining them inline improves code organization and maintainability. This refactoring aligns with the module's broader enhancements without changing its functionality.
packages/server/tests/adapter/nestjs.test.ts (2)
3-5
: Updated imports for the new service and dependenciesThe import statements have been properly updated to include the new
ApiHandlerService
,HttpAdapterHost
,REQUEST
, andRESTApiHandler
that are used in the new test suite.
215-424
: Comprehensive test suite for the new ApiHandlerServiceThe test suite for
ApiHandlerService
is thorough, covering three important scenarios:
- Default options behavior
- Custom REST API handler integration
baseUrl
option functionalityEach test properly sets up the NestJS testing environment, includes necessary mocks for HTTP context, and verifies the expected responses. This ensures the service functions correctly under different configurations.
packages/server/src/nestjs/api-handler.service.ts (2)
14-20
: Appropriately scoped injectable service with proper dependency injectionThe service is correctly decorated with
@Injectable
and scoped toREQUEST
, which is appropriate for a service that handles HTTP requests. The dependencies are properly injected through the constructor.
30-30
: Well-implemented path handling with baseUrl supportThe logic to handle the
baseUrl
option is correctly implemented, slicing the path when necessary to ensure that API requests work properly when mounted under a specific URL prefix.packages/server/src/nestjs/interfaces/zenstack-module-options.interface.ts (5)
1-2
: Keep imports minimal and accurateThe imports from
@nestjs/common
correctly include only the necessary types (FactoryProvider
,ModuleMetadata
,Provider
). No issues detected.
17-20
: Global-scoped flag is correctly definedThe optional
global
boolean follows NestJS conventions for dynamic modules. No changes needed here.
27-31
: Factory signature aligns with NestJS patternsThe
useFactory
method correctly supports both synchronous and asynchronous factories, with optional injected dependencies. This aligns with NestJS dynamic module guidelines.
32-36
: Injection tokens type is correctly referencedUsing
FactoryProvider['inject']
ensures theinject
array matches NestJS standards for factory providers. No changes needed.
37-41
: Extra providers extension is appropriateThe
extraProviders
field allows additional DI providers to be merged into the module, following NestJS dynamic module patterns. Looks good.
No description provided.