nestjs
nestjs copied to clipboard
Alternative NestJS Approach
In moving to a persist/flush model system like Mikro, I find it important to prevent developers on my team from making mistakes like using some global entity manager with a persisted identity map across requests. At the moment, it is very easy to inject Mikro into any service, controller, etc without realizing the repercussions. At a high level, these are my requirements:
- No one should be able access a singleton instance of EntityManager except for the Mikro module itself.
- If there is ever a need to access the application-level singleton, it should be done through some obvious abstraction like
orm.getGlobalEntityManager() - No magic should be involved with request scopes such as node.js domains (deprecated) or pulling things from global statics. Given that a service does not know if it needs to be ran in a transaction or not, guidance/best practice should be to always pass an instance of
EntityManagerto your downstream services via function arguments - The Mikro NestJS module should provide interceptors and decorators for making request scoped work easier (i'm using this for both REST and GraphQL endpoints), example:
EntityManagerInterceptor.ts
import { createParamDecorator, ExecutionContext } from '@nestjs/common';
import { EntityManager } from '@mikro-orm/postgresql';
import { CallHandler, ExecutionContext, NestInterceptor } from '@nestjs/common';
import { Observable } from 'rxjs';
import { mergeMap } from 'rxjs/operators';
export default class EntityManagerInterceptor implements NestInterceptor {
constructor(private em: EntityManager) {}
async intercept(context: ExecutionContext, next: CallHandler<any>): Promise<Observable<any>> {
context['em'] = this.em.fork();
return next.handle().pipe(
mergeMap(async (output) => {
await context['em'].flush();
return output;
})
);
}
}
RequestEntityManager.ts
export default createParamDecorator((data: unknown, context: ExecutionContext) => {
if (!context['em']) {
throw new Error('Could not find a request-scoped EntityManager');
}
return context['em'];
});
// graphql
@Mutation()
myMutation(@RequestEntityManager() em: EntityManager){}
// rest
@Get()
async myEndpoint(@RequestEntityManager() em: EntityManager){}
No one should be able access a singleton instance of EntityManager except for the Mikro module itself.
In v5 there will be a validation for working with the global EM's identity map aware API. I was originally thinking of making it opt-in, mainly because I was lazy to rewrite all the tests, but if we make it configurable via env vars, I could at least disable that globally there :D
If there is ever a need to access the application-level singleton, it should be done through some obvious abstraction like orm.getGlobalEntityManager()
Good idea.
No magic should be involved with request scopes such as node.js domains (deprecated) or pulling things from global statics. Given that a service does not know if it needs to be ran in a transaction or not, guidance/best practice should be to always pass an instance of EntityManager to your downstream services via function arguments
v5 uses ALS instead of domain API. I will definitely keep it there as it is very convenient and universal. On the other hand, this is optional, you can work without this magic if you don't like it.
The Mikro NestJS module should provide interceptors and decorators for making request scoped work easier (i'm using this for both REST and GraphQL endpoints), example:
Sure, we can add the proposed @RequestEntityManager() as an alternative approach. PR welcome.
Will move this to the nest repository to track it there.
somehow, i completely missed the ALS announcement in v14, looks neat but may impact performance at scale (probably not a concern for most projects)
that being said, would the ALS implementation account for transactions? eg: i usually put my orchestration at the controller/resolver level, and wrap all the service calls into a transaction. it isn't clear in the case of nested services (or perhaps in some peoples case, nested transactions if supported by the db) how that implementation would work.
other question w/r/t ALS, if I have a web request come in, and i run to different service calls with two different transactions like so, does it break (probably not the best example, but i'm more-so poking at if there are parallel promises using entity manager)?
@Get()
endpoint(){
await Promise.all([
this.service.a(),
this.service.b();
];
}
somehow, i completely missed the ALS announcement in v14, looks neat but may impact performance at scale (probably not a concern for most projects)
ALS is actually very fast, compared to the domain API. When I was perf testing this last time, there was quite a small penalty for ALS, few percent, maybe up to 10-20%, but I don't really remember now.
that being said, would the ALS implementation account for transactions? eg: i usually put my orchestration at the controller/resolver level, and wrap all the service calls into a transaction.
ALS is actually used internally to hold the transaction context in v5 - again opt in, you can still use the EM from parameter explicitly, only if you work with the global instance the ALS is checked (this works based on the useContext parameter of EM, which defaults to false in em.fork()).
it isn't clear in the case of nested services (or perhaps in some peoples case, nested transactions if supported by the db) how that implementation would work.
Each transaction has its own transaction context (own EM fork), including the nested ones.
other question w/r/t ALS, if I have a web request come in, and i run to different service calls with two different transactions like so, does it break (probably not the best example, but i'm more-so poking at if there are parallel promises using entity manager)?
Using Promise.all() on a single EM instance is not supported, it will throw a validation error. But if you have two separate transactions created in each of a() and b(), it should work fine.
ALS makes sense, your pretty much use it everywhere or not
Using Promise.all() on a single EM instance is not supported, it will throw a validation error
can you expand on that?
I mean you can't flush in parallel (for that you need forks with their own UoW instance), for reading it should be fine.
Just tossing this here in case anyone finds it useful.
I am using it for @nestjs/microservices since Middleware doesn't run in this context.
import type { CallHandler, ExecutionContext, NestInterceptor } from '@nestjs/common';
import { Injectable } from '@nestjs/common';
import type { EntityManager } from '@mikro-orm/core';
import { MikroORM, RequestContext } from '@mikro-orm/core';
import type { AsyncLocalStorage } from 'async_hooks';
import { Observable } from 'rxjs';
interface ExposedRequestContextInterface {
createContext: (em: EntityManager) => RequestContext;
storage: AsyncLocalStorage<RequestContext>;
}
const ExposedRequestContext = RequestContext as unknown as ExposedRequestContextInterface;
/**
* Because the MikroORM RequestContext only works as a middleware for HTTP Traffic, this is a
* monkey patch to inject context at the interceptor level
*/
@Injectable()
export class MikroOrmInterceptor implements NestInterceptor {
constructor(private readonly orm: MikroORM) {}
public intercept(_context: ExecutionContext, next: CallHandler): Observable<unknown> {
const requestContext = ExposedRequestContext.createContext(this.orm.em);
return new Observable((subscriber) => {
const subscription = ExposedRequestContext.storage.run(requestContext, () =>
next.handle().subscribe(subscriber)
);
return () => {
subscription.unsubscribe();
};
});
}
}
It of course has the limitation (being at the interceptor level) of not affecting guards, but for my use case I actually don't need that as all my guards are at the gateway level which issue request to other services which would do their business logic past any interceptors.
A note on a PR I may look into introducing, the RequestContext.createAsync() uses async so it can only handle promises, but the AsyncLocalStorage.prototype.run() can handle any type of returned value, so I am thinking of introducing a RequestContext.bind() or something like that which just calls this.storage.run() without the async and promises.
I've implemented a NestJS interceptor to flush the EM automatically upon request completion. It leverages RequestContext that itself leverage ALS.
@B4nan does that look good or may I run into problems ?
@Injectable()
class DatabaseInterceptor implements NestInterceptor {
intercept(
context: ExecutionContext,
next: CallHandler<any>,
): Observable<any> {
return next.handle().pipe(
tap(() => {
return RequestContext.currentRequestContext()?.em?.flush();
}),
);
}
}
Not entirely sure how this works, I remember tap was just a side effect without waiting, if so, I would rather wait for the flush to finish before you resolve the interceptor/send the response.
My RxJS is very rusty. This update seems to be correct.
@Injectable()
class DatabaseInterceptor implements NestInterceptor {
intercept(
context: ExecutionContext,
next: CallHandler<any>,
): Observable<any> {
return next.handle().pipe(
mergeMap(async (response) => {
await RequestContext.currentRequestContext()?.em?.flush();
return response;
}),
);
}
}