feat(query-mikro-orm): add MikroORM adapter package #417
Conversation
Enable preview package releases on pull requests and workflow dispatch using pkg.pr.new. This allows testing packages before official release. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
feat(ci): add pkg.pr.new preview releases
Add @ptc-org/nestjs-query-mikro-orm package providing MikroORM integration for nestjs-query. This includes: - MikroOrmQueryService implementing QueryService interface - Filter conversion from nestjs-query to MikroORM operators - Support for sorting with null handling - Relation queries (findRelation, queryRelations, countRelations) - NestjsQueryMikroOrmModule with forFeature() pattern - Factory providers for automatic service registration - MikroOrmAssembler for DTO/Entity conversion - Comprehensive test suite with 46 tests Closes TriPSs#178 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Add missing .eslintrc.json for package - Move function definition before usage to fix no-use-before-define - Prefix unused parameters with underscore - Auto-fix formatting issues from prettier/eslint 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The better-sqlite3 native module needs to be compiled per Node version. Previous cache key only used yarn.lock hash, causing NODE_MODULE_VERSION mismatch errors when running tests on Node 22.x and 23.x. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…a5f9d7) throw an error if something is passed that we don't support, e.g. filtering (are there any other options that we silently ignore?)
…n (vibe-kanban c60da658)
## Problem
When using `@Authorize` decorator with a `CustomAuthorizer` that returns an empty filter `{}`, the `findRelationForEntity` method throws:
```
Error: MikroOrmQueryService does not support filtering on findRelation
```
This happens because the check `if (opts?.filter)` is truthy for empty objects `{}`.
## Root Cause
In `packages/query-mikro-orm/src/services/mikro-orm-query.service.ts`, line ~187:
```typescript
if (opts?.filter) {
throw new Error('MikroOrmQueryService does not support filtering on findRelation')
}
```
An empty filter `{}` is truthy, so it triggers the error even though there are no actual filter conditions.
## Solution
Change the check to verify the filter has actual conditions:
```typescript
if (opts?.filter && Object.keys(opts.filter).length > 0) {
throw new Error('MikroOrmQueryService does not support filtering on findRelation')
}
```
## Related
This affects any DTO using `@Authorize` with a `CustomAuthorizer` that returns `{}` when accessing `@Relation` fields.
WalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant App as Nest Application
participant Module as NestjsQueryMikroOrmModule
participant DI as Nest DI Container
participant Repo as MikroORM Repository
participant Service as MikroOrmQueryService
participant Assembler as Assembler (optional)
App->>Module: imports.forFeature(entities, dataSource)
Module->>DI: register providers via createMikroOrmQueryServiceProviders(...)
DI->>Repo: resolve getRepositoryToken(entity, dataSource)
DI->>Service: instantiate MikroOrmQueryService(repo, assembler?)
Note right of Service: Service ready to handle queries
App->>Service: query/findById/findRelation(...)
Service->>Repo: translate filter/sort -> MikroORM queries
Repo-->>Service: return entity/entities
alt assembler provided
Service->>Assembler: convertToDTO(entity)
Assembler-->>Service: DTO
Service-->>App: return DTO(s)
else no assembler
Service-->>App: return entity cast as DTO
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
|
View your CI Pipeline Execution ↗ for commit 44f8c30
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
packages/query-mikro-orm/.eslintrc.json (1)
11-35: Consider simplifying redundant override blocks.The three override blocks all have empty rules, and the first block (lines 12-20) already covers all file types that the second and third blocks target. The additional blocks add no value.
Apply this diff to simplify:
"overrides": [ { "files": [ "*.ts", "*.tsx", "*.js", "*.jsx" ], "rules": {} - }, - { - "files": [ - "*.ts", - "*.tsx" - ], - "rules": {} - }, - { - "files": [ - "*.js", - "*.jsx" - ], - "rules": {} } ]package.json (1)
49-51: Consider upgrading MikroORM packages to their latest stable versions.The specified versions (6.4.0 and 6.1.0) have no known security vulnerabilities. However, they are outdated: @mikro-orm/core and @mikro-orm/better-sqlite should be upgraded to 6.6.1, and @mikro-orm/nestjs to 6.1.1. These versions are compatible with each other (all 6.x), though upgrading from 6.1.0 to 6.4.0+ includes minor API changes (e.g., tsNode option renamed to preferTs) that may require adjustments.
packages/query-mikro-orm/src/assemblers/mikro-orm.assembler.ts (1)
3-37: Consider adding documentation for identity casting approach.The assembler performs identity casts between DTO and Entity types, assuming they have compatible structures. While this is a reasonable default for MikroORM where entities can serve directly as DTOs, consider adding a class-level comment explaining this behavior and that users should provide custom assemblers when transformation is needed.
+/** + * Default assembler for MikroORM that performs identity casts between DTO and Entity. + * Assumes DTO and Entity have compatible structures. For custom transformations, + * provide a custom assembler class via EntityServiceOptions. + */ export class MikroOrmAssembler<packages/query-mikro-orm/src/services/mikro-orm-query.service.ts (1)
293-304:countRelationsForEntityassumes relation is a Collection.Line 300 casts the relation to
Collection<RelationEntity>, which will fail at runtime if called with a single-valued relation (Reference). While counting typically applies to collections, consider adding a runtime check or type guard.+ if (!(collection instanceof Collection)) { + throw new Error(`Relation '${relationName}' is not a collection`) + } const count = await collection.loadCount({ where })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (25)
.github/actions/setup-step/action.yml(1 hunks).github/workflows/publish-preview.yml(1 hunks)package.json(1 hunks)packages/query-mikro-orm/.eslintrc.json(1 hunks)packages/query-mikro-orm/__tests__/__fixtures__/connection.fixture.ts(1 hunks)packages/query-mikro-orm/__tests__/__fixtures__/index.ts(1 hunks)packages/query-mikro-orm/__tests__/__fixtures__/seeds.ts(1 hunks)packages/query-mikro-orm/__tests__/__fixtures__/test-relation.entity.ts(1 hunks)packages/query-mikro-orm/__tests__/__fixtures__/test.entity.ts(1 hunks)packages/query-mikro-orm/__tests__/module.spec.ts(1 hunks)packages/query-mikro-orm/__tests__/providers.spec.ts(1 hunks)packages/query-mikro-orm/__tests__/services/mikro-orm-query.service.spec.ts(1 hunks)packages/query-mikro-orm/jest.config.ts(1 hunks)packages/query-mikro-orm/package.json(1 hunks)packages/query-mikro-orm/project.json(1 hunks)packages/query-mikro-orm/src/assemblers/index.ts(1 hunks)packages/query-mikro-orm/src/assemblers/mikro-orm.assembler.ts(1 hunks)packages/query-mikro-orm/src/index.ts(1 hunks)packages/query-mikro-orm/src/module.ts(1 hunks)packages/query-mikro-orm/src/providers.ts(1 hunks)packages/query-mikro-orm/src/services/index.ts(1 hunks)packages/query-mikro-orm/src/services/mikro-orm-query.service.ts(1 hunks)packages/query-mikro-orm/tsconfig.json(1 hunks)packages/query-mikro-orm/tsconfig.lib.json(1 hunks)packages/query-mikro-orm/tsconfig.spec.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
packages/query-mikro-orm/__tests__/__fixtures__/seeds.ts (1)
packages/query-typegoose/__tests__/__fixtures__/test.entity.ts (1)
TestEntity(6-39)
packages/query-mikro-orm/src/assemblers/mikro-orm.assembler.ts (5)
packages/query-mikro-orm/__tests__/__fixtures__/test.entity.ts (1)
Entity(5-33)packages/query-mikro-orm/src/services/mikro-orm-query.service.ts (1)
query(57-73)packages/core/src/interfaces/query.interface.ts (1)
Query(30-39)packages/core/src/interfaces/aggregate-query.interface.ts (1)
AggregateQuery(20-27)packages/core/src/interfaces/aggregate-response.interface.ts (1)
AggregateResponse(9-16)
packages/query-mikro-orm/__tests__/__fixtures__/test-relation.entity.ts (1)
packages/query-mikro-orm/__tests__/__fixtures__/test.entity.ts (1)
Entity(5-33)
packages/query-mikro-orm/__tests__/__fixtures__/test.entity.ts (1)
packages/query-mikro-orm/__tests__/__fixtures__/test-relation.entity.ts (1)
Entity(5-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: check (23.x, lint)
- GitHub Check: check (23.x, test)
- GitHub Check: check (22.x, build)
- GitHub Check: check (22.x, lint)
- GitHub Check: check (23.x, build)
- GitHub Check: e2e-test (21.x, mysql)
- GitHub Check: e2e-test (20.x, mysql)
- GitHub Check: check (22.x, test)
- GitHub Check: e2e-test (20.x, postgres)
- GitHub Check: e2e-test (21.x, postgres)
- GitHub Check: publish-preview
- GitHub Check: Analyze
- GitHub Check: federation-e2e-test
🔇 Additional comments (29)
packages/query-mikro-orm/src/services/index.ts (1)
1-1: LGTM!Clean barrel export for the service module.
packages/query-mikro-orm/src/assemblers/index.ts (1)
1-1: LGTM!Clean barrel export for the assembler module.
packages/query-mikro-orm/tsconfig.json (1)
1-13: LGTM!Standard TypeScript project configuration with proper project references for library and spec builds.
packages/query-mikro-orm/tsconfig.lib.json (1)
1-20: LGTM!Proper TypeScript library configuration with appropriate test exclusions and CommonJS output.
packages/query-mikro-orm/__tests__/__fixtures__/index.ts (1)
1-4: LGTM!Clean barrel export consolidating test fixtures for convenient imports.
.github/actions/setup-step/action.yml (3)
8-11: LGTM! Good cache busting strategy.Capturing the Node version for use in the cache key ensures cache invalidation when the Node version changes, preventing stale dependency issues.
20-20: LGTM! Improved cache key.Including both the Node version and yarn.lock hash in the cache key provides robust cache invalidation.
22-25: LGTM! Conditional install is correct.The conditional dependency installation only runs on cache miss, optimizing CI performance.
packages/query-mikro-orm/package.json (1)
1-42: LGTM!The package configuration is well-structured with appropriate peer dependencies for flexibility and specific dev dependencies for testing. The separation of concerns between runtime peer dependencies (^6.0.0 range) and test dependencies (^6.4.0 specific) is correct.
packages/query-mikro-orm/tsconfig.spec.json (1)
1-23: LGTM!The test TypeScript configuration is standard and comprehensive, with appropriate module format (commonjs) for Jest and comprehensive test file patterns.
.github/workflows/publish-preview.yml (1)
1-31: LGTM!The preview publishing workflow is well-structured with proper checkout depth for Nx caching and uses Node.js 22.x. The workflow appropriately builds all targets before publishing previews.
packages/query-mikro-orm/src/index.ts (1)
1-4: LGTM!Clean barrel export file that properly exposes the public API of the MikroORM integration package.
packages/query-mikro-orm/__tests__/providers.spec.ts (1)
1-49: LGTM!Comprehensive test coverage for provider creation, including entity classes, custom DTOs, repository token injection with optional data source, and factory instantiation. The test structure is clear and well-organized.
packages/query-mikro-orm/__tests__/services/mikro-orm-query.service.spec.ts (1)
1-360: LGTM!Excellent comprehensive test suite with thorough coverage of all query service operations including:
- All filter operators (eq, neq, gt, gte, lt, lte, in, notIn, like, is, isNot)
- AND/OR filter combinations
- Paging and sorting with null handling
- Entity access methods (getById, findById) with proper error handling
- Relation queries (findRelation, queryRelations, countRelations) for single and multiple entities
- Edge cases and error validation
The test structure is well-organized with proper lifecycle management and clear test descriptions.
packages/query-mikro-orm/project.json (1)
1-43: LGTM!The Nx project configuration is well-structured with proper build, test, lint, version, and publish targets. The implicit dependency on "core" correctly reflects the package's dependency on @ptc-org/nestjs-query-core.
packages/query-mikro-orm/__tests__/module.spec.ts (1)
1-69: LGTM!Well-structured integration tests that verify the NestJS module functionality:
- Query service creation for multiple entities
- Custom DTO support with proper token resolution
- MikroOrmModule export verification
- Proper cleanup in afterEach to prevent resource leaks
The tests effectively validate the module's integration with NestJS and MikroORM.
packages/query-mikro-orm/jest.config.ts (1)
1-18: LGTM!The Jest configuration follows standard practices and correctly sets up testing for the new MikroORM package.
packages/query-mikro-orm/__tests__/__fixtures__/test.entity.ts (1)
5-33: LGTM!The entity relationships are correctly configured and bidirectional. The OneToMany, ManyToOne, ManyToMany, and OneToOne decorators properly reference TestRelation with correct inverse mappings and ownership settings.
packages/query-mikro-orm/__tests__/__fixtures__/connection.fixture.ts (1)
8-32: LGTM!The test connection setup is appropriate for integration testing. The in-memory SQLite database with
allowGlobalContext: trueis acceptable for tests, and the truncate function correctly deletes TestRelation before TestEntity to respect foreign key constraints.packages/query-mikro-orm/src/module.ts (1)
7-20: LGTM!The module correctly wires MikroORM integration with NestJS Query. It properly extracts entity classes, creates query service providers, and delegates to
MikroOrmModule.forFeaturefor entity registration.packages/query-mikro-orm/__tests__/__fixtures__/seeds.ts (1)
32-72: LGTM!The seed function correctly establishes test data with a two-phase approach: first persisting entities and relations, then setting up relation collections. The
entityIndex = Math.floor(index / 3)calculation properly assigns 3 relations to each entity.packages/query-mikro-orm/__tests__/__fixtures__/test-relation.entity.ts (1)
5-21: LGTM!The TestRelation entity correctly defines bidirectional relationships with TestEntity. The ownership sides are properly configured: ManyToMany with
owner: trueon this side, and OneToOne without owner on this side (complementing TestEntity's ownership).packages/query-mikro-orm/src/providers.ts (1)
14-51: LGTM!The provider factory correctly handles three scenarios: custom assembler, DTO with registered assembler, and entity-only (no transformation). The token resolution properly uses
getQueryServiceTokenfor the provide token andgetRepositoryTokenfor dependency injection.packages/query-mikro-orm/src/services/mikro-orm-query.service.ts (6)
21-27: LGTM!Clean class structure with appropriate constructor accepting the repository and optional assembler for DTO/Entity transformations.
57-73: LGTM!Query method correctly handles filter conversion, sorting, and paging with proper assembler integration.
75-99: LGTM!Filter conversion correctly handles
and/orcombinators with proper validation preventing mixed filter structures.
161-202: LGTM!Relation finding logic correctly handles both single and batch entity scenarios with clear error messaging for unsupported
withDeletedoption.
204-229: LGTM with minor observation.The relation loading logic handles various MikroORM entity states (unloaded, Reference-wrapped, uninitialized). The fallback to
em.refreshon line 226 assumesrelationRefis a managed entity, which should hold true in this context.
402-418: LGTM!Sort direction conversion correctly handles all combinations of ASC/DESC with NULLS_FIRST/NULLS_LAST positioning.
packages/query-mikro-orm/src/services/mikro-orm-query.service.ts
Outdated
Show resolved
Hide resolved
|
@jtomaszewski can you checkout the comments of coderabbit? |
- Update actions/cache from v3 to v4 - Use dynamic primary key lookup via MikroORM metadata API - Fix notLike/notILike operator pattern for MikroORM - Replace deprecated __proto__ with Object.getPrototypeOf() 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/query-mikro-orm/src/services/mikro-orm-query.service.ts (2)
235-253: Composite primary keys may not work correctly.Line 248 uses
primaryKeys[0]with the result ofgetPrimaryKey(), which won't work for composite keys. When an entity has multiple primary keys,getPrimaryKey()may return a composite value, but only the first key field is used in the filter.Additionally, using
relation.constructor(line 245) may not be reliable with MikroORM proxy objects; consider usingwrapped.__meta.classorwrapped.__meta.root.classinstead.🔎 Proposed fix for composite key support
private async matchesFilter<Relation extends object>( relation: Relation, filter: Filter<Relation> ): Promise<Relation | undefined> { const em = this.repo.getEntityManager() const where = this.convertFilter(filter as unknown as Filter<Entity>) as unknown as FilterQuery<Relation> const wrapped = wrap(relation, true) const pk = wrapped.getPrimaryKey() + + // Build primary key condition for both single and composite keys + const pkCondition = wrapped.__meta.primaryKeys.reduce((acc, pkName) => { + if (wrapped.__meta.primaryKeys.length === 1) { + acc[pkName] = pk + } else { + // For composite keys, pk is an array or object + acc[pkName] = (pk as Record<string, unknown>)[pkName] ?? (pk as unknown[])[wrapped.__meta.primaryKeys.indexOf(pkName)] + } + return acc + }, {} as Record<string, unknown>) const found = await em.findOne( - relation.constructor as Class<Relation>, + (wrapped.__meta.class ?? wrapped.__meta.root.class) as Class<Relation>, { ...where, - [wrapped.__meta.primaryKeys[0]]: pk + ...pkCondition } as FilterQuery<Relation> ) return (found as Relation) ?? undefined }
44-59: Composite primary keys are not fully supported.The same composite key limitation exists here as in
getById. UsingprimaryKeys[0](line 47) will fail for entities with composite keys.🔎 Proposed fix for composite key support
async findById(id: string | number, opts?: FindByIdOptions<DTO>): Promise<DTO | undefined> { const where = this.convertFilter(opts?.filter) const meta = this.repo.getEntityManager().getMetadata().get(this.repo.getEntityName()) - const pkField = meta.primaryKeys[0] + + // Handle both single and composite primary keys + const pkCondition = meta.primaryKeys.length === 1 + ? { [meta.primaryKeys[0]]: id } + : typeof id === 'object' + ? (id as Record<string, unknown>) + : { [meta.primaryKeys[0]]: id } + const entity = await this.repo.findOne({ ...where, - [pkField]: id + ...pkCondition } as unknown as FilterQuery<Entity>) if (!entity) return undefined if (this.assembler) { return this.assembler.convertToDTO(entity) } return entity as unknown as DTO }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/actions/setup-step/action.ymlpackages/query-mikro-orm/src/services/mikro-orm-query.service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/actions/setup-step/action.yml
🧰 Additional context used
🧬 Code graph analysis (1)
packages/query-mikro-orm/src/services/mikro-orm-query.service.ts (7)
packages/query-mikro-orm/__tests__/__fixtures__/test-relation.entity.ts (1)
Entity(5-21)packages/query-mikro-orm/__tests__/__fixtures__/test.entity.ts (1)
Entity(5-33)packages/core/src/interfaces/get-by-id-options.interface.ts (1)
GetByIdOptions(3-3)packages/core/src/interfaces/find-by-id-options.interface.ts (1)
FindByIdOptions(6-6)packages/core/src/interfaces/query.interface.ts (1)
Query(30-39)packages/core/src/interfaces/find-relation-options.interface.ts (1)
FindRelationOptions(5-10)packages/core/src/interfaces/query-relations-options.interface.ts (1)
QueryRelationsOptions(4-4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze
- GitHub Check: federation-e2e-test
🔇 Additional comments (9)
packages/query-mikro-orm/src/services/mikro-orm-query.service.ts (9)
21-27: LGTM!Clean constructor design with proper dependency injection for the repository and optional assembler.
61-77: LGTM!The query implementation properly handles query conversion, sorting, paging, and filtering with good support for the optional assembler pattern.
79-103: LGTM!Excellent validation logic ensuring
and/orfilters cannot be mixed with other properties (lines 86-88), with proper recursive handling and clean delegation toexpandFilterfor field-level comparisons.
146-160: LGTM!The
notLikeandnotILikeoperators (lines 151, 159) now correctly use the{ $not: { $like/ilike: value } }structure as required by MikroORM, addressing the previous review feedback.
165-206: LGTM!Clean implementation handling both single and batch cases with proper conversion through the assembler. The explicit error for
withDeleted(line 195) correctly communicates the unsupported feature.
208-233: LGTM!Robust handling of MikroORM's different relation types (Reference, Collection, plain objects) with appropriate population and initialization checks.
255-308: LGTM!Well-structured overloads for single and batch operations with proper error handling for the unsupported
withDeletedoption (line 277). The use ofcollection.loadCountwith filtered where clause is appropriate.
352-389: LGTM!Excellent implementation with optimized collection loading (
loadItemsvsmatchingbased on pagination needs). Line 379 now correctly usesObject.getPrototypeOfinstead of the deprecated__proto__, addressing previous feedback. The dynamic assembler resolution throughAssemblerFactoryis well-implemented.
391-416: LGTM!Clean sorting conversion with comprehensive support for null positioning (
NULLS_FIRST/NULLS_LAST) in both ascending and descending order, properly mapping to MikroORM'sQueryOrderenum values.
|
@TriPSs resolved the comments. |
2 participants
Battle-tested in one of my apps for the last few months. It may not be perfect yet, but it is a good start I think.
Summary
@ptc-org/nestjs-query-mikroormpackage that provides MikroORM integrationMikroOrmQueryServicefollowing theQueryServiceinterface from@ptc-org/nestjs-query-coreNestjsQueryMikroOrmModule.forFeature()for easy module registrationTest plan
Summary by CodeRabbit
Release Notes
New Features
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.