feat(openapi): add nestedRoutes support for openapi spec generation#2521
Merged
ymc9 merged 1 commit intozenstackhq:devfrom Mar 27, 2026
Merged
feat(openapi): add nestedRoutes support for openapi spec generation#2521ymc9 merged 1 commit intozenstackhq:devfrom
ymc9 merged 1 commit intozenstackhq:devfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a Changes
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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: 2
🧹 Nitpick comments (2)
packages/server/src/api/rest/openapi.ts (1)
139-149: Remove redundant variable re-declaration.Line 141 shadows
relModelDeffrom line 126, which is already validated as truthy at line 127. Simply use the outer scope variable.♻️ Suggested fix
// Nested single resource path: /{model}/{id}/{field}/{childId} if (this.nestedRoutes && fieldDef.array) { - const relModelDef = this.schema.models[fieldDef.type]!; paths[`/${modelPath}/{id}/${fieldName}/{childId}`] = this.buildNestedSinglePath( modelName, fieldName, fieldDef, relModelDef, tag, ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/api/rest/openapi.ts` around lines 139 - 149, The code redeclares relModelDef in the nested single-resource path block, shadowing the variable already validated earlier; remove the duplicate const/let declaration in the if (this.nestedRoutes && fieldDef.array) block and reuse the previously validated relModelDef (from schema.models[fieldDef.type]) when calling this.buildNestedSinglePath with modelName, fieldName, fieldDef, relModelDef, tag so you avoid shadowing and redundant lookup.packages/server/test/openapi/rest-openapi.test.ts (1)
570-686: Good test coverage for the nestedRoutes feature.The test suite comprehensively covers:
- Default behavior when nestedRoutes is disabled
- Nested single paths generation for collection vs to-one relations
- HTTP methods, request/response schemas, and parameters
- OperationId uniqueness
Consider adding a test case that verifies nested routes respect
queryOptions.slicing.excludedOperations(pending the implementation fix mentioned in the source file review).📝 Suggested additional test case
it('nested routes respect slicing excludedOperations', async () => { const client = await createTestClient(schema); const slicedHandler = new RestApiHandler({ schema: client.$schema, endpoint: 'http://localhost/api', nestedRoutes: true, queryOptions: { slicing: { models: { post: { excludedOperations: ['create', 'delete'] }, }, }, } as any, }); const slicedSpec = await slicedHandler.generateSpec(); // Nested POST should not exist when 'create' is excluded expect((slicedSpec.paths as any)['/user/{id}/posts']?.post).toBeUndefined(); // Nested DELETE should not exist when 'delete' is excluded expect((slicedSpec.paths as any)['/user/{id}/posts/{childId}']?.delete).toBeUndefined(); // GET should still exist expect((slicedSpec.paths as any)['/user/{id}/posts/{childId}']?.get).toBeDefined(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/test/openapi/rest-openapi.test.ts` around lines 570 - 686, Add a test that verifies RestApiHandler.generateSpec respects queryOptions.slicing.excludedOperations by instantiating RestApiHandler with nestedRoutes: true and queryOptions.slicing.models.post.excludedOperations containing 'create' and 'delete', call generateSpec(), and assert that the nested collection POST path '/user/{id}/posts' has no post operation and the nested single DELETE path '/user/{id}/posts/{childId}' has no delete operation while GET still exists; reference RestApiHandler, generateSpec, queryOptions.slicing.models.post.excludedOperations, and the spec.paths keys '/user/{id}/posts' and '/user/{id}/posts/{childId}' when adding the assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/server/src/api/rest/openapi.ts`:
- Around line 426-496: buildNestedSinglePath currently emits get/patch/delete
unconditionally; update it to consult isOperationIncluded(modelName,
'read'|'update'|'delete') before adding each corresponding operation (get →
'read', patch → 'update', delete → 'delete') so nested endpoints follow the same
slicing rules, reuse existing mayDenyAccess checks for conditional 403
responses, and return an empty object if none are included; also ensure
generatePaths skips registering a path when buildNestedSinglePath returns an
empty object to avoid empty path items.
- Around line 359-421: buildFetchRelatedPath and buildNestedSinglePath are
creating nested routes without honoring operation slicing; add
isOperationIncluded checks before adding each nested operation. In
buildFetchRelatedPath only add the POST (create) when
isOperationIncluded(relModelName, 'create') returns true and only add PATCH
(update) when isOperationIncluded(relModelName, 'update') returns true (keep
existing mayDenyAccess checks). In buildNestedSinglePath wrap the GET with
isOperationIncluded(relModelName, 'findUnique'), the PATCH with
isOperationIncluded(relModelName, 'update'), and the DELETE with
isOperationIncluded(relModelName, 'delete') so nested endpoints follow the same
inclusion logic used in buildCollectionPath/buildSinglePath.
---
Nitpick comments:
In `@packages/server/src/api/rest/openapi.ts`:
- Around line 139-149: The code redeclares relModelDef in the nested
single-resource path block, shadowing the variable already validated earlier;
remove the duplicate const/let declaration in the if (this.nestedRoutes &&
fieldDef.array) block and reuse the previously validated relModelDef (from
schema.models[fieldDef.type]) when calling this.buildNestedSinglePath with
modelName, fieldName, fieldDef, relModelDef, tag so you avoid shadowing and
redundant lookup.
In `@packages/server/test/openapi/rest-openapi.test.ts`:
- Around line 570-686: Add a test that verifies RestApiHandler.generateSpec
respects queryOptions.slicing.excludedOperations by instantiating RestApiHandler
with nestedRoutes: true and queryOptions.slicing.models.post.excludedOperations
containing 'create' and 'delete', call generateSpec(), and assert that the
nested collection POST path '/user/{id}/posts' has no post operation and the
nested single DELETE path '/user/{id}/posts/{childId}' has no delete operation
while GET still exists; reference RestApiHandler, generateSpec,
queryOptions.slicing.models.post.excludedOperations, and the spec.paths keys
'/user/{id}/posts' and '/user/{id}/posts/{childId}' when adding the assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ed3f29aa-69bd-404e-be86-4b0731a9b1e2
📒 Files selected for processing (2)
packages/server/src/api/rest/openapi.tspackages/server/test/openapi/rest-openapi.test.ts
lsmith77
force-pushed
the
openapi-nested-routes
branch
from
7736ccb to
6bb3452
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/server/test/openapi/rest-openapi.test.ts (3)
574-582: Validate the nested-routes document as a whole.Only the default spec later in this file goes through parser-level validation. This new variant only checks selected paths, so broken refs or parameter mismatches elsewhere in the
nestedRoutesdocument can still slip through.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/test/openapi/rest-openapi.test.ts` around lines 574 - 582, The nested-routes spec created in the beforeAll (using createTestClient and new RestApiHandler(...), then handler.generateSpec() assigned to spec) is only spot-checked; run the same full parser-level validation used for the default spec later in this file against the entire spec object (not just selected paths) to catch broken $refs or parameter mismatches—i.e., invoke the same validation routine used for the default spec on spec and fail the test if validation reports any errors.
687-709: Please coverexcludedOperations: ['update']for the new nested PATCH endpoints.This regression proves nested
POSTandDELETEare sliced away, but it never exercisesPATCH /user/{id}/posts/{childId}orPATCH /post/{id}/settingunder update exclusion. That branch can regress while this suite still stays green.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/test/openapi/rest-openapi.test.ts` around lines 687 - 709, The test nestedRoutes respects queryOptions slicing excludedOperations misses asserting that PATCH (update) endpoints are removed when 'update' is excluded; update the test in rest-openapi.test.ts (the it block creating slicedHandler via createTestClient and new RestApiHandler with queryOptions.slicing.models.post.excludedOperations) to also include expectations that PATCH /user/{id}/posts/{childId} and PATCH /post/{id}/setting are undefined (and/or that any nested PATCH endpoints are absent) so the suite verifies the update-exclusion branch for both nested and non-nested PATCH routes.
599-611: Please add one nested-route case for a compound-ID child model.All of the new path assertions here use single-column child IDs, but the shared schema also has
User.likes -> PostLikewith@@id([postId, userId]). Since this feature now synthesizes{childId}on nested resources, that edge case can regress without any signal from this suite.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/test/openapi/rest-openapi.test.ts` around lines 599 - 611, Add a test asserting that a nested route is generated for the compound-ID child model (User.likes -> PostLike with @@id([postId, userId])); inside the "generates nested single paths for collection relations" test add an expectation that spec.paths for the synthesized nested child path (e.g. spec.paths['/user/{id}/likes/{childId}']) is defined so the suite covers compound primary-key child models and will catch regressions in the childId synthesis.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/server/test/openapi/rest-openapi.test.ts`:
- Around line 574-582: The nested-routes spec created in the beforeAll (using
createTestClient and new RestApiHandler(...), then handler.generateSpec()
assigned to spec) is only spot-checked; run the same full parser-level
validation used for the default spec later in this file against the entire spec
object (not just selected paths) to catch broken $refs or parameter
mismatches—i.e., invoke the same validation routine used for the default spec on
spec and fail the test if validation reports any errors.
- Around line 687-709: The test nestedRoutes respects queryOptions slicing
excludedOperations misses asserting that PATCH (update) endpoints are removed
when 'update' is excluded; update the test in rest-openapi.test.ts (the it block
creating slicedHandler via createTestClient and new RestApiHandler with
queryOptions.slicing.models.post.excludedOperations) to also include
expectations that PATCH /user/{id}/posts/{childId} and PATCH /post/{id}/setting
are undefined (and/or that any nested PATCH endpoints are absent) so the suite
verifies the update-exclusion branch for both nested and non-nested PATCH
routes.
- Around line 599-611: Add a test asserting that a nested route is generated for
the compound-ID child model (User.likes -> PostLike with @@id([postId,
userId])); inside the "generates nested single paths for collection relations"
test add an expectation that spec.paths for the synthesized nested child path
(e.g. spec.paths['/user/{id}/likes/{childId}']) is defined so the suite covers
compound primary-key child models and will catch regressions in the childId
synthesis.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1f8eb8f6-b8af-4936-88e0-2806e7d1a9ea
📒 Files selected for processing (2)
packages/server/src/api/rest/openapi.tspackages/server/test/openapi/rest-openapi.test.ts
✅ Files skipped from review due to trivial changes (1)
- packages/server/src/api/rest/openapi.ts
lsmith77
force-pushed
the
openapi-nested-routes
branch
from
6bb3452 to
c167e9f
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/server/test/openapi/rest-openapi.test.ts (1)
574-708: Optional: extract spec-builder helper to reduce repetition.
createTestClient+new RestApiHandlersetup is repeated across tests; a small helper would keep assertions denser and easier to extend.♻️ Suggested refactor
describe('REST OpenAPI spec generation - nestedRoutes', () => { let handler: RestApiHandler; let spec: any; + + async function generateNestedSpec(options?: ConstructorParameters<typeof RestApiHandler>[0], genOptions?: any) { + const client = await createTestClient(schema); + const h = new RestApiHandler({ + schema: client.$schema, + endpoint: 'http://localhost/api', + ...options, + }); + return h.generateSpec(genOptions); + } beforeAll(async () => { - const client = await createTestClient(schema); - handler = new RestApiHandler({ - schema: client.$schema, - endpoint: 'http://localhost/api', - nestedRoutes: true, - }); - spec = await handler.generateSpec(); + spec = await generateNestedSpec({ nestedRoutes: true }); }); it('does not generate nested single paths when nestedRoutes is false', async () => { - const client = await createTestClient(schema); - const plainHandler = new RestApiHandler({ - schema: client.$schema, - endpoint: 'http://localhost/api', - }); - const plainSpec = await plainHandler.generateSpec(); + const plainSpec = await generateNestedSpec(); expect(plainSpec.paths?.['/user/{id}/posts/{childId}']).toBeUndefined(); ... });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/test/openapi/rest-openapi.test.ts` around lines 574 - 708, Many tests repeat the same setup (calling createTestClient(schema), new RestApiHandler({...}), and await handler.generateSpec()); extract a small test helper (e.g., a function like createHandlerSpec or makeSpec) that takes options (nestedRoutes, queryOptions, endpoint, schema/client) and returns {client, handler, spec} or at least the spec so tests can call it concisely; replace repeated blocks that construct a RestApiHandler and call generateSpec() with calls to this helper and update the tests that reference handler/spec accordingly (look for usages of createTestClient, RestApiHandler, and generateSpec to locate spots to replace).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/server/test/openapi/rest-openapi.test.ts`:
- Around line 574-708: Many tests repeat the same setup (calling
createTestClient(schema), new RestApiHandler({...}), and await
handler.generateSpec()); extract a small test helper (e.g., a function like
createHandlerSpec or makeSpec) that takes options (nestedRoutes, queryOptions,
endpoint, schema/client) and returns {client, handler, spec} or at least the
spec so tests can call it concisely; replace repeated blocks that construct a
RestApiHandler and call generateSpec() with calls to this helper and update the
tests that reference handler/spec accordingly (look for usages of
createTestClient, RestApiHandler, and generateSpec to locate spots to replace).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3dc89a88-d46a-46d3-abc5-5145afefd810
📒 Files selected for processing (2)
packages/server/src/api/rest/openapi.tspackages/server/test/openapi/rest-openapi.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/server/src/api/rest/openapi.ts
ymc9
changed the title
lsmith77
force-pushed
the
openapi-nested-routes
branch
from
c167e9f to
9fc98b3
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/server/src/api/rest/openapi.ts`:
- Around line 392-422: The PATCH branch for nested to-one updates incorrectly
lacks a to-one guard: update the conditional that creates the patch operation
(the `else if (isOperationIncluded(fieldDef.type, 'update', this.queryOptions))`
block) to also require `!isCollection`, so the to-one `patch` is only added for
non-collection relations; locate the logic around `isOperationIncluded`,
`isCollection`, `fieldDef`, and `pathItem` in openapi generation and add the
`!isCollection` check to prevent adding a to-one PATCH on to-many paths.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9bd45f2f-c4e7-444f-8a7c-3d73d86e8df9
📒 Files selected for processing (2)
packages/server/src/api/rest/openapi.tspackages/server/test/openapi/rest-openapi.test.ts
✅ Files skipped from review due to trivial changes (1)
- packages/server/test/openapi/rest-openapi.test.ts
lsmith77
force-pushed
the
openapi-nested-routes
branch
from
9fc98b3 to
89aa9d2
Compare
ymc9
approved these changes
Mar 27, 2026
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
2 participants
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 CodeRabbit
New Features
Bug Fixes / Refactor
Tests