Skip to content

Commit

Permalink
feat(di): injector.get resolve token when token isn't already cached
Browse files Browse the repository at this point in the history
BREAKING CHANGE: Provider with Request scope can be invoked using injector.get(). Before injector.get() returns undefined
  • Loading branch information
Romakita committed Nov 25, 2024
1 parent 6a274da commit 79ebad7
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 71 deletions.
6 changes: 4 additions & 2 deletions packages/di/src/common/integration/di.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,10 @@ describe("DI", () => {

// THEN
expect(injector().get(ServiceSingleton)).toEqual(inject(ServiceSingleton));
expect(injector().get(ServiceRequest)).toBeUndefined();
expect(injector().get(ServiceInstance)).toBeUndefined();
expect(injector().get(ServiceRequest)).toBeInstanceOf(ServiceRequest);
expect(injector().has(ServiceRequest)).toBeFalsy();
expect(injector().get(ServiceInstance)).toBeInstanceOf(ServiceInstance);
expect(injector().has(ServiceRequest)).toBeFalsy();

expect(injector().invoke(ServiceRequest) === injector().invoke(ServiceRequest)).toEqual(false);
expect(inject(ServiceInstance) === inject(ServiceInstance)).toEqual(false);
Expand Down
6 changes: 4 additions & 2 deletions packages/di/src/common/integration/singleton.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ describe("DI Singleton", () => {
expect(serviceSingletonWithReqDep.serviceRequest).toEqual(serviceSingletonWithReqDep.serviceRequest2);

// The service isn't registered in the injectorService
expect(serviceRequest).toBeUndefined();
expect(serviceRequest).toBeDefined();
expect(injector().has(ServiceRequest)).toEqual(false);
});
});
describe("when it has a INSTANCE dependency", () => {
Expand All @@ -117,7 +118,8 @@ describe("DI Singleton", () => {
expect(serviceWithInstDep.serviceInstance === serviceWithInstDep.serviceInstance2).toEqual(false);

// The service isn't registered in the injectorService
expect(serviceInstance).toBeUndefined();
expect(serviceInstance).toBeInstanceOf(ServiceInstance);
expect(injector().has(ServiceInstance)).toEqual(false);
});
});
});
8 changes: 3 additions & 5 deletions packages/di/src/common/services/InjectorService.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ describe("InjectorService", () => {
expect(injector().get(InjectorService)).toBeInstanceOf(InjectorService);
});

it("should return undefined", () => {
expect(injector().get(Test)).toBeUndefined();
it("should return Test", () => {
expect(injector().get(Test)).toBeInstanceOf(Test);
});
});
describe("getMany()", () => {
Expand Down Expand Up @@ -222,14 +222,12 @@ describe("InjectorService", () => {
const locals = new LocalsContainer();

// WHEN

const result1: any = inject(token, {locals});
const result2: any = inject(token, {locals});

// THEN
expect(result1).toEqual(result2);

return expect((injector() as any).invokeToken).not.toHaveBeenCalled();
expect((injector() as any).invokeToken).not.toHaveBeenCalled();
});
});
describe("when provider is a Value (useValue)", () => {
Expand Down
80 changes: 47 additions & 33 deletions packages/di/src/common/services/InjectorService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import {$alter, $asyncAlter, $asyncEmit, $emit, $off, $on} from "@tsed/hooks";

import {DI_INVOKE_OPTIONS, DI_USE_PARAM_OPTIONS} from "../constants/constants.js";
import {Configuration} from "../decorators/configuration.js";
import {Injectable} from "../decorators/injectable.js";
import {Container} from "../domain/Container.js";
import {LocalsContainer} from "../domain/LocalsContainer.js";
import {Provider} from "../domain/Provider.js";
Expand Down Expand Up @@ -41,19 +40,23 @@ const EXCLUDED_CONFIGURATION_KEYS = ["mount", "imports"];
* const myService1 = injector.get<MyService1>(MyService1);
* ```
*/
@Injectable({
scope: ProviderScope.SINGLETON
})
export class InjectorService extends Container {
public settings: DIConfiguration = new DIConfiguration();
public logger: DILogger = console;
private resolvedConfiguration: boolean = false;
#cache = new LocalsContainer();

constructor() {
super();
this.#cache.set(InjectorService, this);
this.#cache.set(Configuration, this.settings);
this.#cache.set(Configuration, new DIConfiguration());
}

get settings(): DIConfiguration {
return this.#cache.get(Configuration);
}

set settings(settings: DIConfiguration) {
this.#cache.set(Configuration, settings);
}

/**
Expand All @@ -80,8 +83,13 @@ export class InjectorService extends Container {
* ```
*
* @param token The class or symbol registered in InjectorService.
* @param options
*/
get<T = any>(token: TokenProvider<T>): T | undefined {
get<T = any>(token: TokenProvider<T>, options?: Partial<InvokeOptions>): T {
if (!this.has(token)) {
return this.resolve(token, options);
}

return this.#cache.get(token);
}

Expand Down Expand Up @@ -151,16 +159,12 @@ export class InjectorService extends Container {

const provider = this.ensureProvider(token);

const set = (instance: any) => {
this.#cache.set(token, instance);
provider?.alias && this.alias(token, provider.alias);
};

// maybe not necessary
if (!provider || options.rebuild) {
instance = this.invokeToken(token, options);

if (this.hasProvider(token)) {
set(instance);
if (provider) {
return this.setToCache(provider!, instance);
}

return instance;
Expand All @@ -174,21 +178,7 @@ export class InjectorService extends Container {
this.registerHooks(provider, options);
}

if (!provider.isAsync() || !isPromise(instance)) {
set(instance);
return instance;
}

// store promise to lock token in cache
set(instance);

instance = instance.then((instance: any) => {
set(instance);

return instance;
});
return instance;

return this.setToCache(provider, instance);
case ProviderScope.REQUEST:
if (options.locals) {
options.locals.set(provider.token, instance);
Expand Down Expand Up @@ -245,6 +235,7 @@ export class InjectorService extends Container {
*/
loadSync() {
for (const [, provider] of this) {
// TODO try to lazy provider instead initiate all providers (&& provider.hasRegisteredHooks())
if (!this.has(provider.token) && provider.scope === ProviderScope.SINGLETON) {
this.resolve(provider.token);
}
Expand All @@ -271,7 +262,8 @@ export class InjectorService extends Container {
}

/**
* Load all configurations registered on providers
* Load all configurations registered on providers via @Configuration decorator.
* It inspects for each provider some fields like imports, mount, etc. to resolve the configuration.
*/
resolveConfiguration() {
if (this.resolvedConfiguration) {
Expand Down Expand Up @@ -414,14 +406,12 @@ export class InjectorService extends Container {
return this.getMany(token[0], options);
}

const useOpts = provider?.getArgOpts(index) || options.useOpts;

return isInheritedFrom(token, Provider, 1)
? provider
: this.resolve(token, {
parent,
locals: options.locals,
useOpts
useOpts: provider?.getArgOpts(index) || options.useOpts
});
};

Expand Down Expand Up @@ -576,4 +566,28 @@ export class InjectorService extends Container {
});
}
}

private setToCache(provider: Provider, instance: any) {
const set = (instance: any) => {
this.#cache.set(provider.token, instance);
provider?.alias && this.alias(provider.token, provider.alias);
};

if ("isAsync" in provider && !provider.isAsync() && !isPromise(instance)) {
set(instance);

return instance;
}

// store promise to lock token in cache
set(instance);

instance = instance.then((instance: any) => {
set(instance);

return instance;
});

return instance;
}
}
36 changes: 14 additions & 22 deletions packages/di/src/node/services/DITest.spec.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,16 @@
import {Logger} from "@tsed/logger";

import {Inject, Injectable, InjectorService, registerProvider, Service} from "../../index.js";
import {Inject, Injectable, injectable, InjectorService} from "../../index.js";
import {DITest} from "../services/DITest.js";

class Model {}

const SQLITE_DATA_SOURCE = Symbol.for("SQLITE_DATA_SOURCE");

registerProvider({
provide: SQLITE_DATA_SOURCE,
type: "typeorm:datasource",
deps: [Logger],
useAsyncFactory(logger: Logger) {
const SQLITE_DATA_SOURCE = injectable(Symbol.for("SQLITE_DATA_SOURCE"))
.type("typeorm:datasource")
.asyncFactory(() => {
return Promise.resolve({
id: "sqlite"
});
}
});
})
.token();

export abstract class AbstractDao {
private readonly dao: any;
Expand Down Expand Up @@ -91,24 +85,22 @@ describe("DITest", () => {
});

it("should return a service with pre mocked dependencies (invoke + mock)", async () => {
const dao = {
initialize: vi.fn(),
getRepository: vi.fn().mockReturnValue({
repository: false
})
};
const service = await DITest.invoke<FileDao>(FileDao, [
{
token: SQLITE_DATA_SOURCE,
use: {
initialize: vi.fn(),
getRepository: vi.fn().mockReturnValue({
repository: false
})
}
use: dao
}
]);

const repository = DITest.get(SQLITE_DATA_SOURCE);

expect(repository.getRepository).toHaveBeenCalledWith(Model);

const result = service.getRepository();

expect(dao.getRepository).toHaveBeenCalledWith(Model);
expect(result).toEqual({
repository: false
});
Expand Down
5 changes: 2 additions & 3 deletions packages/di/src/node/services/DITest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,9 @@ export class DITest {
/**
* Return the instance from injector registry
* @param target
* @param options
*/
static get<T = any>(target: TokenProvider, options: any = {}): T {
return injector().get<T>(target, options)!;
static get<T = any>(target: TokenProvider): T {
return injector().get<T>(target)!;
}

static createDIContext() {
Expand Down
8 changes: 4 additions & 4 deletions packages/di/vitest.config.mts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ export default defineConfig(
coverage: {
...presets.test.coverage,
thresholds: {
statements: 98.91,
branches: 97.04,
functions: 97.15,
lines: 98.91
statements: 98.69,
branches: 97.2,
functions: 97.02,
lines: 98.69
}
}
}
Expand Down

0 comments on commit 79ebad7

Please sign in to comment.