Skip to content
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

fix: resolve issue with ordering of middleware being applied #189

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ More information about [enableShutdownHooks](https://docs.nestjs.com/fundamental

## Multiple Database Connections

You can define multiple database connections by registering multiple `MikroOrmModule` and setting their `contextName`. If you want to use middleware request context you must disable automatic middleware and register `MikroOrmModule` with `forMiddleware()` or use NestJS `Injection Scope`
You can define multiple database connections by registering several MikroOrmModules, each with a unique contextName and setting registerRequestContext to false. If you want to use the middleware request context you must register `MultipleMikroOrmModule` with `forRoot()` or use NestJS `Injection Scope`
tsangste marked this conversation as resolved.
Show resolved Hide resolved

```typescript
@Module({
Expand All @@ -357,7 +357,7 @@ You can define multiple database connections by registering multiple `MikroOrmMo
registerRequestContext: false, // disable automatatic middleware
...
}),
MikroOrmModule.forMiddleware()
MultipleMikroOrmModule.forRoot()
tsangste marked this conversation as resolved.
Show resolved Hide resolved
],
controllers: [AppController],
providers: [AppService],
Expand Down
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export * from './mikro-orm.module';
export * from './mikro-orm.common';
export * from './mikro-orm.middleware';
export * from './multiple-mikro-orm.module';
export * from './multiple-mikro-orm.middleware';
export * from './typings';
4 changes: 2 additions & 2 deletions src/middleware.helper.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { MikroOrmMiddlewareModuleOptions, NestMiddlewareConsumer } from './typings';
import type { MultipleMikroOrmModuleOptions, NestMiddlewareConsumer } from './typings';
import type { MiddlewareConsumer } from '@nestjs/common';

export function forRoutesPath(options: MikroOrmMiddlewareModuleOptions, consumer: MiddlewareConsumer) {
export function forRoutesPath(options: MultipleMikroOrmModuleOptions, consumer: MiddlewareConsumer) {
const isNestMiddleware = (consumer: MiddlewareConsumer): consumer is NestMiddlewareConsumer => {
return typeof (consumer as any).httpAdapter === 'object';
};
Expand Down
9 changes: 5 additions & 4 deletions src/mikro-orm.module.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { Utils, type AnyEntity } from '@mikro-orm/core';
import { Module, type DynamicModule } from '@nestjs/common';
import { MikroOrmCoreModule } from './mikro-orm-core.module';
import { MikroOrmMiddlewareModule } from './mikro-orm-middleware.module';
import { MultipleMikroOrmModule } from './multiple-mikro-orm.module';
import { MikroOrmEntitiesStorage } from './mikro-orm.entities.storage';
import { createMikroOrmRepositoryProviders } from './mikro-orm.providers';
import type {
EntityName,
MikroOrmMiddlewareModuleOptions,
MultipleMikroOrmModuleOptions,
MikroOrmModuleAsyncOptions,
MikroOrmModuleFeatureOptions,
MikroOrmModuleSyncOptions,
Expand Down Expand Up @@ -55,10 +55,11 @@ export class MikroOrmModule {
};
}

static forMiddleware(options?: MikroOrmMiddlewareModuleOptions): DynamicModule {
// Remove this in the next major version, kept for backwards compatibility
tsangste marked this conversation as resolved.
Show resolved Hide resolved
static forMiddleware(options?: MultipleMikroOrmModuleOptions): DynamicModule {
return {
module: MikroOrmModule,
imports: [MikroOrmMiddlewareModule.forMiddleware(options)],
imports: [MultipleMikroOrmModule.forRoot(options)],
};
tsangste marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,19 @@ import type { MikroORM } from '@mikro-orm/core';
import { forRoutesPath } from './middleware.helper';
import { CONTEXT_NAMES, getMikroORMToken, MIKRO_ORM_MODULE_OPTIONS } from './mikro-orm.common';
import { MultipleMikroOrmMiddleware } from './multiple-mikro-orm.middleware';
import { MikroOrmMiddlewareModuleOptions } from './typings';
import { MultipleMikroOrmModuleOptions } from './typings';

@Global()
@Module({})
export class MikroOrmMiddlewareModule {
export class MultipleMikroOrmModule {

constructor(@Inject(MIKRO_ORM_MODULE_OPTIONS)
private readonly options: MikroOrmMiddlewareModuleOptions) { }
private readonly options: MultipleMikroOrmModuleOptions) { }

static forMiddleware(options?: MikroOrmMiddlewareModuleOptions) {
// Work around due to nestjs not supporting the ability to register multiple types
// https://github.com/nestjs/nest/issues/770
// https://github.com/nestjs/nest/issues/4786#issuecomment-755032258 - workaround suggestion
static forRoot(options?: MultipleMikroOrmModuleOptions) {
const inject = CONTEXT_NAMES.map(name => getMikroORMToken(name));
return {
module: MikroOrmMiddlewareModule,
module: MultipleMikroOrmModule,
providers: [
{ provide: MIKRO_ORM_MODULE_OPTIONS, useValue: options || {} },
{
Expand Down
4 changes: 2 additions & 2 deletions src/typings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ type MikroOrmNestScopeOptions = {
scope?: Scope;
};

export type MikroOrmMiddlewareModuleOptions = {
export type MultipleMikroOrmModuleOptions = {
tsangste marked this conversation as resolved.
Show resolved Hide resolved
/**
* Routes to apply the middleware.
*
Expand All @@ -24,7 +24,7 @@ export type MikroOrmMiddlewareModuleOptions = {
export type MikroOrmModuleOptions<D extends IDatabaseDriver = IDatabaseDriver> = {
registerRequestContext?: boolean;
autoLoadEntities?: boolean;
} & Options<D> & MikroOrmMiddlewareModuleOptions;
} & Options<D> & MultipleMikroOrmModuleOptions;

export interface MikroOrmModuleFeatureOptions {
entities?: EntityName<AnyEntity>[];
Expand Down
3 changes: 2 additions & 1 deletion tests/entities/foo.entity.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { PrimaryKey, Entity } from '@mikro-orm/core';
import { PrimaryKey, Entity, Filter } from '@mikro-orm/core';

@Entity()
@Filter({ name: 'id', cond: args => ({ id: args.id }) })
export class Foo {

@PrimaryKey()
Expand Down
86 changes: 60 additions & 26 deletions tests/mikro-orm.middleware.test.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
import type { Options } from '@mikro-orm/core';
import { MikroORM } from '@mikro-orm/core';
import { EntityManager, MikroORM, type Options } from '@mikro-orm/core';
import { SqliteDriver } from '@mikro-orm/sqlite';
import type { INestApplication } from '@nestjs/common';
import {
Controller,
Get,
Module,
type INestApplication,
Injectable,
MiddlewareConsumer,
Module, type NestMiddleware,
NestModule,
} from '@nestjs/common';
import type { TestingModule } from '@nestjs/testing';
import { Test } from '@nestjs/testing';
import { Test, type TestingModule } from '@nestjs/testing';
import request from 'supertest';
import { InjectMikroORM, MikroOrmModule } from '../src';
import { InjectEntityManager, InjectMikroORM, MikroOrmModule } from '../src';
import { Bar } from './entities/bar.entity';
import { Foo } from './entities/foo.entity';

Expand All @@ -21,45 +22,82 @@ const testOptions: Options = {
entities: ['entities'],
};

@Controller()
class TestController {
@Controller('/foo')
class FooController {

constructor(
@InjectMikroORM('database1') private database1: MikroORM,
@InjectMikroORM('database2') private database2: MikroORM,
) {}
constructor(@InjectMikroORM('database-foo') private database1: MikroORM) {}

@Get('foo')
@Get()
foo() {
return this.database1.em !== this.database1.em.getContext();
}

@Get('bar')
}

@Controller('/bar')
class BarController {

constructor(@InjectMikroORM('database-bar') private database2: MikroORM) {}

@Get()
bar() {
return this.database2.em !== this.database2.em.getContext();
}

}

@Injectable()
class TestMiddleware implements NestMiddleware {

constructor(@InjectEntityManager('database-foo') private readonly em: EntityManager) {}

use(req: unknown, res: unknown, next: (...args: any[]) => void) {
// Throws error "Using global EntityManager instance methods for context specific actions is disallowed"
this.em.setFilterParams('id', { id: '1' });

return next();
}

}

@Module({
imports: [MikroOrmModule.forFeature([Foo], 'database-foo')],
controllers: [FooController],
})
class FooModule implements NestModule {

configure(consumer: MiddlewareConsumer): void {
consumer
.apply(TestMiddleware)
.forRoutes('/');
}

}

@Module({
imports: [MikroOrmModule.forFeature([Bar], 'database-bar')],
controllers: [BarController],
})
class BarModule {}

@Module({
imports: [
MikroOrmModule.forRootAsync({
contextName: 'database1',
contextName: 'database-foo',
useFactory: () => ({
registerRequestContext: false,
...testOptions,
}),
}),
MikroOrmModule.forRoot({
contextName: 'database2',
contextName: 'database-bar',
registerRequestContext: false,
...testOptions,
}),
MikroOrmModule.forMiddleware(),
MikroOrmModule.forFeature([Foo], 'database1'),
MikroOrmModule.forFeature([Bar], 'database2'),
FooModule,
BarModule,
],
controllers: [TestController],
})
class TestModule {}

Expand All @@ -76,12 +114,8 @@ describe('Middleware executes request context for all MikroORM registered', () =
await app.init();
});

it(`forRoutes(/foo) should return 'true'`, () => {
return request(app.getHttpServer()).get('/foo').expect(200, 'true');
});

it(`forRoutes(/bar) should return 'true'`, () => {
return request(app.getHttpServer()).get('/foo').expect(200, 'true');
it(`forRoutes(/foo) should return error`, () => {
return request(app.getHttpServer()).get('/foo').expect(500);
});
tsangste marked this conversation as resolved.
Show resolved Hide resolved

afterAll(async () => {
Expand Down
128 changes: 128 additions & 0 deletions tests/mikro-orm.multiple-middleware.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
import { EntityManager, MikroORM, type Options } from '@mikro-orm/core';
import { SqliteDriver } from '@mikro-orm/sqlite';
import {
Controller,
Get,
Module,
type INestApplication,
Injectable,
type NestMiddleware,
MiddlewareConsumer,
NestModule,
} from '@nestjs/common';
import { Test, type TestingModule } from '@nestjs/testing';
import request from 'supertest';
import { InjectEntityManager, InjectMikroORM, MikroOrmModule, MultipleMikroOrmModule } from '../src';
import { Bar } from './entities/bar.entity';
import { Foo } from './entities/foo.entity';

const testOptions: Options = {
dbName: ':memory:',
driver: SqliteDriver,
baseDir: __dirname,
entities: ['entities'],
};

@Controller('/foo')
class FooController {

constructor(@InjectMikroORM('database-multi-foo') private database1: MikroORM) {}

@Get()
foo() {
return this.database1.em !== this.database1.em.getContext();
}

}

@Controller('/bar')
class BarController {

constructor(@InjectMikroORM('database-multi-bar') private database2: MikroORM) {}

@Get()
bar() {
return this.database2.em !== this.database2.em.getContext();
}

}

@Injectable()
export class TestMiddleware implements NestMiddleware {

constructor(@InjectEntityManager('database-multi-foo') private readonly em: EntityManager) {}

use(req: unknown, res: unknown, next: (...args: any[]) => void) {
this.em.setFilterParams('id', { id: '1' });

return next();
}

}

@Module({
imports: [MikroOrmModule.forFeature([Foo], 'database-multi-foo')],
controllers: [FooController],
})
class FooModule implements NestModule {

configure(consumer: MiddlewareConsumer): void {
consumer
.apply(TestMiddleware)
.forRoutes('/');
}

}

@Module({
imports: [MikroOrmModule.forFeature([Bar], 'database-multi-bar')],
controllers: [BarController],
})
class BarModule {}

@Module({
imports: [
MikroOrmModule.forRootAsync({
contextName: 'database-multi-foo',
useFactory: () => ({
registerRequestContext: false,
...testOptions,
}),
}),
MikroOrmModule.forRoot({
contextName: 'database-multi-bar',
registerRequestContext: false,
...testOptions,
}),
MultipleMikroOrmModule.forRoot(),
FooModule,
BarModule,
],
})
class TestModule {}

describe('Multiple Middleware executes request context for all MikroORM registered', () => {
let app: INestApplication;

beforeAll(async () => {
const moduleFixture: TestingModule = await Test.createTestingModule({
imports: [TestModule],
}).compile();

app = moduleFixture.createNestApplication();

await app.init();
});

it(`forRoutes(/foo) should return 'true'`, () => {
return request(app.getHttpServer()).get('/foo').expect(200, 'true');
});

it(`forRoutes(/bar) should return 'true'`, () => {
return request(app.getHttpServer()).get('/bar').expect(200, 'true');
});

afterAll(async () => {
await app.close();
});
});