mirror of
https://github.com/nestjs/nest.git
synced 2026-02-21 23:11:44 +00:00
Merge pull request #15815 from mag123c/fix/transient-nested-dependency-isolation
fix(core): ensure nested transient provider isolation
This commit is contained in:
@@ -2,6 +2,7 @@ import { INestApplication, Injectable, Scope } from '@nestjs/common';
|
||||
import { Test } from '@nestjs/testing';
|
||||
import { expect } from 'chai';
|
||||
import * as request from 'supertest';
|
||||
import { NestedTransientModule } from '../src/nested-transient/nested-transient.module';
|
||||
import { Guard } from '../src/transient/guards/request-scoped.guard';
|
||||
import { HelloController } from '../src/transient/hello.controller';
|
||||
import { HelloModule } from '../src/transient/hello.module';
|
||||
@@ -139,4 +140,53 @@ describe('Transient scope', () => {
|
||||
await app.close();
|
||||
});
|
||||
});
|
||||
|
||||
describe('when nested transient providers are used in request scope', () => {
|
||||
let server: any;
|
||||
let app: INestApplication;
|
||||
|
||||
before(async () => {
|
||||
const module = await Test.createTestingModule({
|
||||
imports: [NestedTransientModule],
|
||||
}).compile();
|
||||
|
||||
app = module.createNestApplication();
|
||||
server = app.getHttpServer();
|
||||
await app.init();
|
||||
});
|
||||
|
||||
describe('when handling HTTP requests', () => {
|
||||
let response: any;
|
||||
|
||||
before(async () => {
|
||||
const performHttpCall = () =>
|
||||
new Promise<any>((resolve, reject) => {
|
||||
request(server)
|
||||
.get('/nested-transient')
|
||||
.end((err, res) => {
|
||||
if (err) return reject(err);
|
||||
resolve(res);
|
||||
});
|
||||
});
|
||||
|
||||
response = await performHttpCall();
|
||||
});
|
||||
|
||||
it('should isolate nested transient instances for each parent service', () => {
|
||||
expect(response.body.firstServiceContext).to.equal(
|
||||
'NESTED-FirstService',
|
||||
);
|
||||
expect(response.body.secondServiceContext).to.equal(
|
||||
'NESTED-SecondService',
|
||||
);
|
||||
expect(response.body.firstServiceNestedId).to.not.equal(
|
||||
response.body.secondServiceNestedId,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
after(async () => {
|
||||
await app.close();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,12 @@
|
||||
import { Injectable, Scope } from '@nestjs/common';
|
||||
import { TransientLoggerService } from './transient-logger.service';
|
||||
|
||||
@Injectable({ scope: Scope.REQUEST })
|
||||
export class FirstRequestService {
|
||||
static COUNTER = 0;
|
||||
|
||||
constructor(public readonly logger: TransientLoggerService) {
|
||||
FirstRequestService.COUNTER++;
|
||||
this.logger.setContext('FirstService');
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,25 @@
|
||||
import { Controller, Get, Scope } from '@nestjs/common';
|
||||
import { FirstRequestService } from './first-request.service';
|
||||
import { SecondRequestService } from './second-request.service';
|
||||
|
||||
@Controller({ path: 'nested-transient', scope: Scope.REQUEST })
|
||||
export class NestedTransientController {
|
||||
static COUNTER = 0;
|
||||
|
||||
constructor(
|
||||
private readonly firstService: FirstRequestService,
|
||||
private readonly secondService: SecondRequestService,
|
||||
) {
|
||||
NestedTransientController.COUNTER++;
|
||||
}
|
||||
|
||||
@Get()
|
||||
getIsolationData() {
|
||||
return {
|
||||
firstServiceContext: this.firstService.logger.getNestedContext(),
|
||||
firstServiceNestedId: this.firstService.logger.getNestedInstanceId(),
|
||||
secondServiceContext: this.secondService.logger.getNestedContext(),
|
||||
secondServiceNestedId: this.secondService.logger.getNestedInstanceId(),
|
||||
};
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,17 @@
|
||||
import { Module } from '@nestjs/common';
|
||||
import { NestedTransientController } from './nested-transient.controller';
|
||||
import { FirstRequestService } from './first-request.service';
|
||||
import { SecondRequestService } from './second-request.service';
|
||||
import { TransientLoggerService } from './transient-logger.service';
|
||||
import { NestedTransientService } from './nested-transient.service';
|
||||
|
||||
@Module({
|
||||
controllers: [NestedTransientController],
|
||||
providers: [
|
||||
FirstRequestService,
|
||||
SecondRequestService,
|
||||
TransientLoggerService,
|
||||
NestedTransientService,
|
||||
],
|
||||
})
|
||||
export class NestedTransientModule {}
|
||||
@@ -0,0 +1,21 @@
|
||||
import { Injectable, Scope } from '@nestjs/common';
|
||||
|
||||
@Injectable({ scope: Scope.TRANSIENT })
|
||||
export class NestedTransientService {
|
||||
static COUNTER = 0;
|
||||
public readonly instanceId: number;
|
||||
private context?: string;
|
||||
|
||||
constructor() {
|
||||
NestedTransientService.COUNTER++;
|
||||
this.instanceId = NestedTransientService.COUNTER;
|
||||
}
|
||||
|
||||
setContext(ctx: string) {
|
||||
this.context = ctx;
|
||||
}
|
||||
|
||||
getContext(): string | undefined {
|
||||
return this.context;
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,12 @@
|
||||
import { Injectable, Scope } from '@nestjs/common';
|
||||
import { TransientLoggerService } from './transient-logger.service';
|
||||
|
||||
@Injectable({ scope: Scope.REQUEST })
|
||||
export class SecondRequestService {
|
||||
static COUNTER = 0;
|
||||
|
||||
constructor(public readonly logger: TransientLoggerService) {
|
||||
SecondRequestService.COUNTER++;
|
||||
this.logger.setContext('SecondService');
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,25 @@
|
||||
import { Injectable, Scope } from '@nestjs/common';
|
||||
import { NestedTransientService } from './nested-transient.service';
|
||||
|
||||
@Injectable({ scope: Scope.TRANSIENT })
|
||||
export class TransientLoggerService {
|
||||
static COUNTER = 0;
|
||||
public readonly instanceId: number;
|
||||
|
||||
constructor(public readonly nested: NestedTransientService) {
|
||||
TransientLoggerService.COUNTER++;
|
||||
this.instanceId = TransientLoggerService.COUNTER;
|
||||
}
|
||||
|
||||
setContext(ctx: string) {
|
||||
this.nested.setContext(`NESTED-${ctx}`);
|
||||
}
|
||||
|
||||
getNestedContext(): string | undefined {
|
||||
return this.nested.getContext();
|
||||
}
|
||||
|
||||
getNestedInstanceId(): number {
|
||||
return this.nested.instanceId;
|
||||
}
|
||||
}
|
||||
@@ -296,7 +296,6 @@ export class Injector {
|
||||
inquirer?: InstanceWrapper,
|
||||
parentInquirer?: InstanceWrapper,
|
||||
) {
|
||||
const inquirerId = this.getInquirerId(inquirer);
|
||||
const metadata = wrapper.getCtorMetadata();
|
||||
|
||||
if (metadata && contextId !== STATIC_CONTEXT) {
|
||||
@@ -349,15 +348,21 @@ export class Injector {
|
||||
*/
|
||||
await paramBarrier.signalAndWait();
|
||||
|
||||
const effectiveInquirer = this.getEffectiveInquirer(
|
||||
paramWrapper,
|
||||
inquirer,
|
||||
parentInquirer,
|
||||
contextId,
|
||||
);
|
||||
const paramWrapperWithInstance = await this.resolveComponentHost(
|
||||
moduleRef,
|
||||
paramWrapper,
|
||||
contextId,
|
||||
inquirer,
|
||||
effectiveInquirer,
|
||||
);
|
||||
const instanceHost = paramWrapperWithInstance.getInstanceByContextId(
|
||||
this.getContextId(contextId, paramWrapperWithInstance),
|
||||
inquirerId,
|
||||
this.getInquirerId(effectiveInquirer),
|
||||
);
|
||||
if (!instanceHost.isResolved && !paramWrapperWithInstance.forwardRef) {
|
||||
isResolved = false;
|
||||
@@ -742,19 +747,24 @@ export class Injector {
|
||||
*/
|
||||
await propertyBarrier.signalAndWait();
|
||||
|
||||
const effectivePropertyInquirer = this.getEffectiveInquirer(
|
||||
paramWrapper,
|
||||
inquirer,
|
||||
parentInquirer,
|
||||
contextId,
|
||||
);
|
||||
const paramWrapperWithInstance = await this.resolveComponentHost(
|
||||
moduleRef,
|
||||
paramWrapper,
|
||||
contextId,
|
||||
inquirer,
|
||||
effectivePropertyInquirer,
|
||||
);
|
||||
if (!paramWrapperWithInstance) {
|
||||
return undefined;
|
||||
}
|
||||
const inquirerId = this.getInquirerId(inquirer);
|
||||
const instanceHost = paramWrapperWithInstance.getInstanceByContextId(
|
||||
this.getContextId(contextId, paramWrapperWithInstance),
|
||||
inquirerId,
|
||||
this.getInquirerId(effectivePropertyInquirer),
|
||||
);
|
||||
return instanceHost.instance;
|
||||
} catch (err) {
|
||||
@@ -904,14 +914,20 @@ export class Injector {
|
||||
),
|
||||
),
|
||||
);
|
||||
const inquirerId = this.getInquirerId(inquirer);
|
||||
return hosts.map(
|
||||
item =>
|
||||
item?.getInstanceByContextId(
|
||||
this.getContextId(contextId, item),
|
||||
inquirerId,
|
||||
).instance,
|
||||
return hosts.map((item, index) => {
|
||||
const dependency = metadata[index];
|
||||
const effectiveInquirer = this.getEffectiveInquirer(
|
||||
dependency,
|
||||
inquirer,
|
||||
parentInquirer,
|
||||
contextId,
|
||||
);
|
||||
|
||||
return item?.getInstanceByContextId(
|
||||
this.getContextId(contextId, item),
|
||||
this.getInquirerId(effectiveInquirer),
|
||||
).instance;
|
||||
});
|
||||
}
|
||||
|
||||
public async loadPropertiesMetadata(
|
||||
@@ -947,6 +963,27 @@ export class Injector {
|
||||
return inquirer ? inquirer.id : undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* For nested TRANSIENT dependencies (TRANSIENT -> TRANSIENT) in non-static contexts,
|
||||
* returns parentInquirer to ensure each parent TRANSIENT gets its own instance.
|
||||
* This is necessary because in REQUEST/DURABLE scopes, the same TRANSIENT wrapper
|
||||
* can be used by multiple parents, causing nested TRANSIENTs to be shared incorrectly.
|
||||
* For non-TRANSIENT -> TRANSIENT, returns inquirer (current wrapper being created).
|
||||
*/
|
||||
private getEffectiveInquirer(
|
||||
dependency: InstanceWrapper | undefined,
|
||||
inquirer: InstanceWrapper | undefined,
|
||||
parentInquirer: InstanceWrapper | undefined,
|
||||
contextId: ContextId,
|
||||
): InstanceWrapper | undefined {
|
||||
return dependency?.isTransient &&
|
||||
inquirer?.isTransient &&
|
||||
parentInquirer &&
|
||||
contextId !== STATIC_CONTEXT
|
||||
? parentInquirer
|
||||
: inquirer;
|
||||
}
|
||||
|
||||
private resolveScopedComponentHost(
|
||||
item: InstanceWrapper,
|
||||
contextId: ContextId,
|
||||
@@ -955,7 +992,12 @@ export class Injector {
|
||||
) {
|
||||
return this.isInquirerRequest(item, parentInquirer)
|
||||
? parentInquirer
|
||||
: this.resolveComponentHost(item.host!, item, contextId, inquirer);
|
||||
: this.resolveComponentHost(
|
||||
item.host!,
|
||||
item,
|
||||
contextId,
|
||||
this.getEffectiveInquirer(item, inquirer, parentInquirer, contextId),
|
||||
);
|
||||
}
|
||||
|
||||
private isInquirerRequest(
|
||||
|
||||
193
packages/core/test/injector/nested-transient-isolation.spec.ts
Normal file
193
packages/core/test/injector/nested-transient-isolation.spec.ts
Normal file
@@ -0,0 +1,193 @@
|
||||
import { Scope } from '@nestjs/common';
|
||||
import { expect } from 'chai';
|
||||
import { Injectable } from '../../../common/decorators/core/injectable.decorator';
|
||||
import { NestContainer } from '../../injector/container';
|
||||
import { Injector } from '../../injector/injector';
|
||||
import { InstanceWrapper } from '../../injector/instance-wrapper';
|
||||
import { Module } from '../../injector/module';
|
||||
|
||||
describe('Nested Transient Isolation', () => {
|
||||
let injector: Injector;
|
||||
let module: Module;
|
||||
|
||||
beforeEach(() => {
|
||||
injector = new Injector();
|
||||
module = new Module(class TestModule {}, new NestContainer());
|
||||
});
|
||||
|
||||
describe('when TRANSIENT provider depends on another TRANSIENT provider', () => {
|
||||
@Injectable({ scope: Scope.TRANSIENT })
|
||||
class NestedTransientService {
|
||||
public static instanceCount = 0;
|
||||
public readonly instanceId: number;
|
||||
|
||||
constructor() {
|
||||
NestedTransientService.instanceCount++;
|
||||
this.instanceId = NestedTransientService.instanceCount;
|
||||
}
|
||||
}
|
||||
|
||||
@Injectable({ scope: Scope.TRANSIENT })
|
||||
class TransientService {
|
||||
public static instanceCount = 0;
|
||||
public readonly instanceId: number;
|
||||
|
||||
constructor(public readonly nested: NestedTransientService) {
|
||||
TransientService.instanceCount++;
|
||||
this.instanceId = TransientService.instanceCount;
|
||||
}
|
||||
}
|
||||
|
||||
@Injectable({ scope: Scope.REQUEST })
|
||||
class RequestScopedParent1 {
|
||||
constructor(public readonly transient: TransientService) {}
|
||||
}
|
||||
|
||||
@Injectable({ scope: Scope.REQUEST })
|
||||
class RequestScopedParent2 {
|
||||
constructor(public readonly transient: TransientService) {}
|
||||
}
|
||||
|
||||
let nestedTransientWrapper: InstanceWrapper;
|
||||
let transientWrapper: InstanceWrapper;
|
||||
let parent1Wrapper: InstanceWrapper;
|
||||
let parent2Wrapper: InstanceWrapper;
|
||||
|
||||
beforeEach(() => {
|
||||
NestedTransientService.instanceCount = 0;
|
||||
TransientService.instanceCount = 0;
|
||||
|
||||
nestedTransientWrapper = new InstanceWrapper({
|
||||
name: NestedTransientService.name,
|
||||
token: NestedTransientService,
|
||||
metatype: NestedTransientService,
|
||||
scope: Scope.TRANSIENT,
|
||||
host: module,
|
||||
});
|
||||
|
||||
transientWrapper = new InstanceWrapper({
|
||||
name: TransientService.name,
|
||||
token: TransientService,
|
||||
metatype: TransientService,
|
||||
scope: Scope.TRANSIENT,
|
||||
host: module,
|
||||
});
|
||||
|
||||
parent1Wrapper = new InstanceWrapper({
|
||||
name: RequestScopedParent1.name,
|
||||
token: RequestScopedParent1,
|
||||
metatype: RequestScopedParent1,
|
||||
scope: Scope.REQUEST,
|
||||
host: module,
|
||||
});
|
||||
|
||||
parent2Wrapper = new InstanceWrapper({
|
||||
name: RequestScopedParent2.name,
|
||||
token: RequestScopedParent2,
|
||||
metatype: RequestScopedParent2,
|
||||
scope: Scope.REQUEST,
|
||||
host: module,
|
||||
});
|
||||
|
||||
module.providers.set(NestedTransientService, nestedTransientWrapper);
|
||||
module.providers.set(TransientService, transientWrapper);
|
||||
module.providers.set(RequestScopedParent1, parent1Wrapper);
|
||||
module.providers.set(RequestScopedParent2, parent2Wrapper);
|
||||
});
|
||||
|
||||
it('should create separate TRANSIENT instances for each parent', async () => {
|
||||
const contextId = { id: 1 };
|
||||
|
||||
await injector.loadInstance(
|
||||
parent1Wrapper,
|
||||
module.providers,
|
||||
module,
|
||||
contextId,
|
||||
);
|
||||
await injector.loadInstance(
|
||||
parent2Wrapper,
|
||||
module.providers,
|
||||
module,
|
||||
contextId,
|
||||
);
|
||||
|
||||
const parent1Instance =
|
||||
parent1Wrapper.getInstanceByContextId(contextId).instance;
|
||||
const parent2Instance =
|
||||
parent2Wrapper.getInstanceByContextId(contextId).instance;
|
||||
|
||||
// 각 parent는 서로 다른 TransientService instance를 가져야 함
|
||||
expect(parent1Instance.transient.instanceId).to.not.equal(
|
||||
parent2Instance.transient.instanceId,
|
||||
);
|
||||
});
|
||||
|
||||
it('should create separate nested TRANSIENT instances for each parent TRANSIENT', async () => {
|
||||
const contextId = { id: 1 };
|
||||
|
||||
await injector.loadInstance(
|
||||
parent1Wrapper,
|
||||
module.providers,
|
||||
module,
|
||||
contextId,
|
||||
);
|
||||
await injector.loadInstance(
|
||||
parent2Wrapper,
|
||||
module.providers,
|
||||
module,
|
||||
contextId,
|
||||
);
|
||||
|
||||
const parent1Instance =
|
||||
parent1Wrapper.getInstanceByContextId(contextId).instance;
|
||||
const parent2Instance =
|
||||
parent2Wrapper.getInstanceByContextId(contextId).instance;
|
||||
|
||||
// 각 TransientService는 서로 다른 NestedTransientService instance를 가져야 함
|
||||
expect(parent1Instance.transient.nested.instanceId).to.not.equal(
|
||||
parent2Instance.transient.nested.instanceId,
|
||||
);
|
||||
});
|
||||
|
||||
it('should maintain isolation across multiple request contexts', async () => {
|
||||
const contextId1 = { id: 1 };
|
||||
const contextId2 = { id: 2 };
|
||||
|
||||
await injector.loadInstance(
|
||||
parent1Wrapper,
|
||||
module.providers,
|
||||
module,
|
||||
contextId1,
|
||||
);
|
||||
await injector.loadInstance(
|
||||
parent2Wrapper,
|
||||
module.providers,
|
||||
module,
|
||||
contextId1,
|
||||
);
|
||||
await injector.loadInstance(
|
||||
parent1Wrapper,
|
||||
module.providers,
|
||||
module,
|
||||
contextId2,
|
||||
);
|
||||
|
||||
const ctx1Parent1 =
|
||||
parent1Wrapper.getInstanceByContextId(contextId1).instance;
|
||||
const ctx1Parent2 =
|
||||
parent2Wrapper.getInstanceByContextId(contextId1).instance;
|
||||
const ctx2Parent1 =
|
||||
parent1Wrapper.getInstanceByContextId(contextId2).instance;
|
||||
|
||||
// 같은 context 내에서 다른 parent
|
||||
expect(ctx1Parent1.transient.nested.instanceId).to.not.equal(
|
||||
ctx1Parent2.transient.nested.instanceId,
|
||||
);
|
||||
|
||||
// 다른 context의 같은 parent
|
||||
expect(ctx1Parent1.transient.nested.instanceId).to.not.equal(
|
||||
ctx2Parent1.transient.nested.instanceId,
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user