Skip to content

Commit

Permalink
fix: use new sessions for lazy resolution with getter/view
Browse files Browse the repository at this point in the history
fixes #9041

Signed-off-by: Raymond Feng <enjoyjava@gmail.com>
  • Loading branch information
raymondfeng committed Dec 5, 2022
1 parent 54727ae commit 13e3e10
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -709,8 +709,7 @@ describe('Context bindings - Injecting dependencies of classes', () => {
const store = await ctx.get<Store>(STORE_KEY);
expect(store.locations).to.eql(['San Francisco', 'San Jose']);
expect(resolutionPath).to.eql(
'store --> @Store.constructor[0] --> store.locations.sj --> ' +
'@LocationProvider.prototype.location',
'store.locations.sj --> @LocationProvider.prototype.location',
);
});

Expand Down
69 changes: 63 additions & 6 deletions packages/context/src/__tests__/unit/resolver.unit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
Context,
Getter,
inject,
injectable,
Injection,
instantiateClass,
invokeMethod,
Expand Down Expand Up @@ -201,6 +202,65 @@ describe('constructor injection', () => {
expect(y).to.eql('y');
});

// https://github.com/loopbackio/loopback-next/issues/9041
it('uses a new session for getter resolution', async () => {
const context = new Context();
interface XInterface {
value: string;
xy(): Promise<string>;
}
interface YInterface {
value: string;
}

@injectable({scope: BindingScope.SINGLETON})
class XClass implements XInterface {
value = 'x';
@inject.getter('y')
public y: Getter<YInterface>;

@inject.getter('x')
public x: Getter<XInterface>;

async xy() {
const y = await this.y();
const x = await this.x();
return x.value + y.value;
}
}

@injectable({scope: BindingScope.SINGLETON})
class YClass implements YInterface {
value = 'y';
@inject.getter('x')
public x: Getter<XInterface>;
}

class ZClass {
constructor(
// Now binding x will be in the session of ZClass resolution
@inject('x') private x: XInterface,
@inject('y') private y: YInterface,
@inject.getter('y') private getY: Getter<YInterface>,
) {}

async test() {
const y = await this.getY();
expect(y.value).eql('y');
expect(this.y.value).eql('y');
expect(this.x.value).eql('x');
expect(await this.x.xy()).to.eql('xy');
}
}

context.bind('x').toClass(XClass);
context.bind('y').toClass(YClass);
context.bind('z').toClass(ZClass);

const z = context.getSync<ZClass>('z');
await z.test();
});

it('reports circular dependencies of three bindings', () => {
const context = new Context();

Expand Down Expand Up @@ -326,12 +386,9 @@ describe('constructor injection', () => {
context.bind('z').toClass(ZClass);
const x: XClass = context.getSync('x');
await x.y.z();
expect(bindingPath).to.eql('x --> y --> z');
expect(resolutionPath).to.eql(
'x --> @XClass.constructor[0] --> y --> @YClass.constructor[0]' +
' --> z --> @ZClass.prototype.myProp',
);
expect(decorators).to.eql(['@inject', '@inject.getter', '@inject']);
expect(bindingPath).to.eql('z');
expect(resolutionPath).to.eql('z --> @ZClass.prototype.myProp');
expect(decorators).to.eql(['@inject']);
});

it('tracks path of injections', () => {
Expand Down
5 changes: 4 additions & 1 deletion packages/context/src/context-view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,10 @@ export class ContextView<T = unknown>
...this.resolutionOptions,
...asResolutionOptions(session),
};
options.session = ResolutionSession.fork(options.session);
// https://github.com/loopbackio/loopback-next/issues/9041
// We should start with a new session for `view` resolution to avoid
// possible circular dependencies
options.session = undefined;
return b.getValue(this.context, options);
});
if (isPromiseLike(result)) {
Expand Down
7 changes: 4 additions & 3 deletions packages/context/src/inject-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,14 +175,15 @@ function resolveAsGetterFromConfig(
) {
assertTargetType(injection, Function, 'Getter function');
const bindingKey = getTargetBindingKey(injection, session);
// We need to clone the session for the getter as it will be resolved later
const forkedSession = ResolutionSession.fork(session);
const meta = injection.metadata;
return async function getter() {
// Return `undefined` if no current binding is present
if (!bindingKey) return undefined;
return ctx.getConfigAsValueOrPromise(bindingKey, meta.propertyPath, {
session: forkedSession,
// https://github.com/loopbackio/loopback-next/issues/9041
// We should start with a new session for `getter` resolution to avoid
// possible circular dependencies
session: undefined,
optional: meta.optional,
});
};
Expand Down
7 changes: 4 additions & 3 deletions packages/context/src/inject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -434,10 +434,11 @@ function resolveAsGetter(
) {
assertTargetType(injection, Function, 'Getter function');
const bindingSelector = injection.bindingSelector as BindingAddress;
// We need to clone the session for the getter as it will be resolved later
const forkedSession = ResolutionSession.fork(session);
const options: ResolutionOptions = {
session: forkedSession,
// https://github.com/loopbackio/loopback-next/issues/9041
// We should start with a new session for `getter` resolution to avoid
// possible circular dependencies
session: undefined,
...injection.metadata,
};
return function getter() {
Expand Down

0 comments on commit 13e3e10

Please sign in to comment.