Skip to content

Commit 6577316

Browse files
kamilmysliwieckamilmysliwiec
authored
Merge pull request #15469 from nestjs/fix/attach-root-inquirer
fix(core): attach root inquirer for nested transient providers
2 parents 0e51937 + a9d5716 commit 6577316

File tree

3 files changed

+134
-53
lines changed

3 files changed

+134
-53
lines changed

‎integration/scopes/e2e/transient-scope.spec.ts‎

Lines changed: 108 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { INestApplication, Scope } from '@nestjs/common';
1+
import { INestApplication, Injectable, Scope } from '@nestjs/common';
22
import { Test } from '@nestjs/testing';
33
import { expect } from 'chai';
44
import * as request from 'supertest';
@@ -17,65 +17,126 @@ class Meta {
1717
}
1818

1919
describe('Transient scope', () => {
20-
let server;
21-
let app: INestApplication;
22-
23-
before(async () => {
24-
const module = await Test.createTestingModule({
25-
imports: [
26-
HelloModule.forRoot({
27-
provide: 'META',
28-
useClass: Meta,
29-
scope: Scope.TRANSIENT,
30-
}),
31-
],
32-
}).compile();
33-
34-
app = module.createNestApplication();
35-
server = app.getHttpServer();
36-
await app.init();
37-
});
20+
describe('when transient scope is used', () => {
21+
let server: any;
22+
let app: INestApplication;
3823

39-
describe('when one service is request scoped', () => {
4024
before(async () => {
41-
const performHttpCall = end =>
42-
request(server)
43-
.get('/hello')
44-
.end(err => {
45-
if (err) return end(err);
46-
end();
47-
});
48-
await new Promise<any>(resolve => performHttpCall(resolve));
49-
await new Promise<any>(resolve => performHttpCall(resolve));
50-
await new Promise<any>(resolve => performHttpCall(resolve));
51-
});
25+
const module = await Test.createTestingModule({
26+
imports: [
27+
HelloModule.forRoot({
28+
provide: 'META',
29+
useClass: Meta,
30+
scope: Scope.TRANSIENT,
31+
}),
32+
],
33+
}).compile();
5234

53-
it(`should create controller for each request`, () => {
54-
expect(HelloController.COUNTER).to.be.eql(3);
35+
app = module.createNestApplication();
36+
server = app.getHttpServer();
37+
await app.init();
5538
});
5639

57-
it(`should create service for each request`, () => {
58-
expect(UsersService.COUNTER).to.be.eql(3);
59-
});
40+
describe('and when one service is request scoped', () => {
41+
before(async () => {
42+
const performHttpCall = end =>
43+
request(server)
44+
.get('/hello')
45+
.end(err => {
46+
if (err) return end(err);
47+
end();
48+
});
49+
await new Promise<any>(resolve => performHttpCall(resolve));
50+
await new Promise<any>(resolve => performHttpCall(resolve));
51+
await new Promise<any>(resolve => performHttpCall(resolve));
52+
});
53+
54+
it(`should create controller for each request`, () => {
55+
expect(HelloController.COUNTER).to.be.eql(3);
56+
});
57+
58+
it(`should create service for each request`, () => {
59+
expect(UsersService.COUNTER).to.be.eql(3);
60+
});
6061

61-
it(`should create provider for each inquirer`, () => {
62-
expect(Meta.COUNTER).to.be.eql(7);
62+
it(`should create provider for each inquirer`, () => {
63+
expect(Meta.COUNTER).to.be.eql(7);
64+
});
65+
66+
it(`should create transient pipe for each controller (3 requests, 1 static)`, () => {
67+
expect(UserByIdPipe.COUNTER).to.be.eql(4);
68+
});
69+
70+
it(`should create transient interceptor for each controller (3 requests, 1 static)`, () => {
71+
expect(Interceptor.COUNTER).to.be.eql(4);
72+
});
73+
74+
it(`should create transient guard for each controller (3 requests, 1 static)`, () => {
75+
expect(Guard.COUNTER).to.be.eql(4);
76+
});
6377
});
6478

65-
it(`should create transient pipe for each controller (3 requests, 1 static)`, () => {
66-
expect(UserByIdPipe.COUNTER).to.be.eql(4);
79+
after(async () => {
80+
await app.close();
6781
});
82+
});
83+
84+
describe('when there is a nested structure of transient providers', () => {
85+
let app: INestApplication;
6886

69-
it(`should create transient interceptor for each controller (3 requests, 1 static)`, () => {
70-
expect(Interceptor.COUNTER).to.be.eql(4);
87+
@Injectable({ scope: Scope.TRANSIENT })
88+
class DeepTransient {
89+
public initialized = false;
90+
91+
constructor() {
92+
this.initialized = true;
93+
}
94+
}
95+
96+
@Injectable({ scope: Scope.TRANSIENT })
97+
class LoggerService {
98+
public context?: string;
99+
}
100+
101+
@Injectable({ scope: Scope.TRANSIENT })
102+
class SecondService {
103+
constructor(public readonly loggerService: LoggerService) {
104+
this.loggerService.context = 'SecondService';
105+
}
106+
}
107+
108+
@Injectable()
109+
class FirstService {
110+
constructor(
111+
public readonly secondService: SecondService,
112+
public readonly loggerService: LoggerService,
113+
public readonly deepTransient: DeepTransient,
114+
) {
115+
this.loggerService.context = 'FirstService';
116+
}
117+
}
118+
119+
before(async () => {
120+
const module = await Test.createTestingModule({
121+
providers: [FirstService, SecondService, LoggerService, DeepTransient],
122+
}).compile();
123+
124+
app = module.createNestApplication();
125+
await app.init();
71126
});
72127

73-
it(`should create transient guard for each controller (3 requests, 1 static)`, () => {
74-
expect(Guard.COUNTER).to.be.eql(4);
128+
it('should create a new instance of the transient provider for each provider', async () => {
129+
const firstService1 = app.get(FirstService);
130+
131+
expect(firstService1.secondService.loggerService.context).to.equal(
132+
'SecondService',
133+
);
134+
expect(firstService1.loggerService.context).to.equal('FirstService');
135+
expect(firstService1.deepTransient.initialized).to.be.true;
75136
});
76-
});
77137

78-
after(async () => {
79-
await app.close();
138+
after(async () => {
139+
await app.close();
140+
});
80141
});
81142
});

‎packages/core/injector/injector.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import { CircularDependencyException } from '../errors/exceptions';
3232
import { RuntimeException } from '../errors/exceptions/runtime.exception';
3333
import { UndefinedDependencyException } from '../errors/exceptions/undefined-dependency.exception';
3434
import { UnknownDependenciesException } from '../errors/exceptions/unknown-dependencies.exception';
35+
import { Barrier } from '../helpers/barrier';
3536
import { STATIC_CONTEXT } from './constants';
3637
import { INQUIRER } from './inquirer';
3738
import {
@@ -42,7 +43,6 @@ import {
4243
} from './instance-wrapper';
4344
import { Module } from './module';
4445
import { SettlementSignal } from './settlement-signal';
45-
import { Barrier } from '../helpers/barrier';
4646

4747
/**
4848
* The type of an injectable dependency
@@ -296,7 +296,7 @@ export class Injector {
296296
inquirer?: InstanceWrapper,
297297
parentInquirer?: InstanceWrapper,
298298
) {
299-
let inquirerId = this.getInquirerId(inquirer);
299+
const inquirerId = this.getInquirerId(inquirer);
300300
const metadata = wrapper.getCtorMetadata();
301301

302302
if (metadata && contextId !== STATIC_CONTEXT) {
@@ -327,8 +327,10 @@ export class Injector {
327327
return parentInquirer && parentInquirer.instance;
328328
}
329329
if (inquirer?.isTransient && parentInquirer) {
330-
inquirer = parentInquirer;
331-
inquirerId = this.getInquirerId(parentInquirer);
330+
// When `inquirer` is transient too, inherit the parent inquirer
331+
// This is required to ensure that transient providers are only resolved
332+
// when requested
333+
inquirer.attachRootInquirer(parentInquirer);
332334
}
333335
const paramWrapper = await this.resolveSingleParam<T>(
334336
wrapper,

‎packages/core/injector/instance-wrapper.ts‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,11 @@ export class InstanceWrapper<T = any> {
8282
| undefined;
8383
private isTreeStatic: boolean | undefined;
8484
private isTreeDurable: boolean | undefined;
85+
/**
86+
* The root inquirer reference. Present only if child instance wrapper
87+
* is transient and has a parent inquirer.
88+
*/
89+
private rootInquirer: InstanceWrapper | undefined;
8590

8691
constructor(
8792
metadata: Partial<InstanceWrapper<T>> & Partial<InstancePerContext<T>> = {},
@@ -405,15 +410,28 @@ export class InstanceWrapper<T = any> {
405410
const isInquirerRequestScoped =
406411
inquirer && !inquirer.isDependencyTreeStatic();
407412
const isStaticTransient = this.isTransient && !isInquirerRequestScoped;
408-
413+
const rootInquirer = inquirer?.getRootInquirer();
409414
return (
410415
this.isDependencyTreeStatic() &&
411416
contextId === STATIC_CONTEXT &&
412417
(!this.isTransient ||
413-
(isStaticTransient && !!inquirer && !inquirer.isTransient))
418+
(isStaticTransient && !!inquirer && !inquirer.isTransient) ||
419+
(isStaticTransient && !!rootInquirer && !rootInquirer.isTransient))
414420
);
415421
}
416422

423+
public attachRootInquirer(inquirer: InstanceWrapper) {
424+
if (!this.isTransient) {
425+
// Only attach root inquirer if the instance wrapper is transient
426+
return;
427+
}
428+
this.rootInquirer = inquirer.getRootInquirer() ?? inquirer;
429+
}
430+
431+
getRootInquirer(): InstanceWrapper | undefined {
432+
return this.rootInquirer;
433+
}
434+
417435
public getStaticTransientInstances() {
418436
if (!this.transientMap) {
419437
return [];

0 commit comments

Comments
 (0)