From ebb296bce83bba6621f16a1ea66b01e95e5618dc Mon Sep 17 00:00:00 2001 From: Jesse Carter Date: Tue, 5 Mar 2019 17:06:49 -0500 Subject: [PATCH] Fix for Providers that don't provide classType - Updated the API to differentiate between the constructor of the injectable vs the constructor of the dependency - Add better checking and reflection to make sure we don't reflect on undefined objects --- LICENSE | 22 +++ packages/discovery/README.md | 11 +- .../discovery/src/discovery.interfaces.ts | 3 +- packages/discovery/src/discovery.service.ts | 47 ++++- .../advanced-controller-discovery.spec.ts | 7 +- .../src/tests/async-providers.spec.ts | 75 -------- .../discovery/src/tests/discovery.spec.ts | 10 +- .../src/tests/provider-types.spec.ts | 181 ++++++++++++++++++ 8 files changed, 261 insertions(+), 95 deletions(-) create mode 100644 LICENSE delete mode 100644 packages/discovery/src/tests/async-providers.spec.ts create mode 100644 packages/discovery/src/tests/provider-types.spec.ts diff --git a/LICENSE b/LICENSE new file mode 100644 index 000000000..1676227d6 --- /dev/null +++ b/LICENSE @@ -0,0 +1,22 @@ +(The MIT License) + +Copyright (c) 2019 Jesse Carter + +Permission is hereby granted, free of charge, to any person obtaining +a copy of this software and associated documentation files (the +'Software'), to deal in the Software without restriction, including +without limitation the rights to use, copy, modify, merge, publish, +distribute, sublicense, and/or sell copies of the Software, and to +permit persons to whom the Software is furnished to do so, subject to +the following conditions: + +The above copyright notice and this permission notice shall be +included in all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED 'AS IS', WITHOUT WARRANTY OF ANY KIND, +EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. +IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY +CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, +TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE +SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. diff --git a/packages/discovery/README.md b/packages/discovery/README.md index 045973a29..4a88ed143 100644 --- a/packages/discovery/README.md +++ b/packages/discovery/README.md @@ -56,7 +56,8 @@ In the case of querying for `providers` or `controllers`, the service returns th export interface DiscoveredModule { name: string; instance: {}; - classType: Type<{}>; + injectType?: Type<{}>; + dependencyType: Type<{}>; } export interface DiscoveredClass extends DiscoveredModule { @@ -64,7 +65,13 @@ export interface DiscoveredClass extends DiscoveredModule { } ``` -This gives access to the (singleton) `instance` of the matching provider or controller created by the NestJS Dependency Injection container as well as the `classType` which is the class constructor function. It also provides the string based name for convenience. A `DiscoveredClass` contains a `parentModule` which provides the same set of information for the `@Module` class that the dependency was discovered in. +This gives access to the (singleton) `instance` of the matching provider or controller created by the NestJS Dependency Injection container. + +The `injectType` can contain the constructor function of the provider token if it is provided as an @Injectable class. In the case of custom providers, this value will either contain the type of the factory function that created the dependency, or undefined if a value was directly provided with `useValue`. + +The `dependencyType` is a shortcut to retrieve the constructor function of the actual provided dependency itself. For @Injectable providers/controllers this will simply be the decorated class but for dyanmic providers it will return the constructor function of whatever dependency was actually returned from `useValue` or `useFactory`. + +It also provides the string based name for convenience. A `DiscoveredClass` contains a `parentModule` which provides the same set of information for the `@Module` class that the dependency was discovered in. When querying for methods on `providers` or `controllers` the following interface is returned: diff --git a/packages/discovery/src/discovery.interfaces.ts b/packages/discovery/src/discovery.interfaces.ts index 8373c2909..51096b587 100644 --- a/packages/discovery/src/discovery.interfaces.ts +++ b/packages/discovery/src/discovery.interfaces.ts @@ -3,7 +3,8 @@ import { Type } from '@nestjs/common'; export interface DiscoveredModule { name: string; instance: {}; - classType: Type<{}>; + injectType?: Type<{}>; + dependencyType: Type<{}>; } export interface DiscoveredClass extends DiscoveredModule { diff --git a/packages/discovery/src/discovery.service.ts b/packages/discovery/src/discovery.service.ts index 20f30a01c..1d3688e2f 100644 --- a/packages/discovery/src/discovery.service.ts +++ b/packages/discovery/src/discovery.service.ts @@ -4,7 +4,7 @@ import { InstanceWrapper } from '@nestjs/core/injector/container'; import { Module } from '@nestjs/core/injector/module'; import { ModulesContainer } from '@nestjs/core/injector/modules-container'; import { MetadataScanner } from '@nestjs/core/metadata-scanner'; -import { flatMap, uniqBy } from 'lodash'; +import { flatMap, some, uniqBy } from 'lodash'; import { DiscoveredClass, DiscoveredClassWithMeta, @@ -13,6 +13,28 @@ import { MetaKey } from './discovery.interfaces'; +/** + * Attempts to retrieve meta information from a Nest DiscoveredClass component + * @param key The meta key to retrieve data from + * @param component The discovered component to retrieve meta from + */ +export function getComponentMetaAtKey( + key: MetaKey, + component: DiscoveredClass +): T | undefined { + const dependencyMeta = Reflect.getMetadata( + key, + component.dependencyType + ) as T; + if (dependencyMeta) { + return dependencyMeta; + } + + if (component.injectType != null) { + return Reflect.getMetadata(key, component.injectType) as T; + } +} + /** * A filter that can be used to search for DiscoveredClasses in an App that contain meta attached to a * certain key @@ -20,9 +42,14 @@ import { */ export const withMetaAtKey: ( key: MetaKey -) => Filter = key => component => - Reflect.getMetadata(key, component.classType) || - Reflect.getMetadata(key, component.instance.constructor); +) => Filter = key => component => { + const metaTargets: Function[] = [ + component.instance.constructor, + component.injectType as Function + ].filter(x => x != null); + + return some(metaTargets, x => Reflect.getMetadata(key, x)); +}; @Injectable() export class DiscoveryService { @@ -106,9 +133,7 @@ export class DiscoveryService { const providers = await this.providers(withMetaAtKey(metaKey)); return providers.map(x => ({ - meta: - (Reflect.getMetadata(metaKey, x.classType) as T) || - Reflect.getMetadata(metaKey, x.instance.constructor), + meta: getComponentMetaAtKey(metaKey, x) as T, discoveredClass: x })); } @@ -133,7 +158,7 @@ export class DiscoveryService { const controllers = await this.controllers(withMetaAtKey(metaKey)); return controllers.map(x => ({ - meta: Reflect.getMetadata(metaKey, x.classType) as T, + meta: getComponentMetaAtKey(metaKey, x) as T, discoveredClass: x })); } @@ -203,11 +228,13 @@ export class DiscoveryService { return { name: component.name as string, instance: component.instance, - classType: component.metatype, + injectType: component.metatype, + dependencyType: component.instance.constructor, parentModule: { name: nestModule.metatype.name, instance: nestModule.instance, - classType: nestModule.metatype + injectType: nestModule.metatype, + dependencyType: component.instance.constructor } }; } diff --git a/packages/discovery/src/tests/advanced-controller-discovery.spec.ts b/packages/discovery/src/tests/advanced-controller-discovery.spec.ts index fea579a46..fbab69073 100644 --- a/packages/discovery/src/tests/advanced-controller-discovery.spec.ts +++ b/packages/discovery/src/tests/advanced-controller-discovery.spec.ts @@ -10,6 +10,7 @@ import { import { METHOD_METADATA, PATH_METADATA } from '@nestjs/common/constants'; import { Test, TestingModule } from '@nestjs/testing'; import { DiscoveryModule, DiscoveryService } from '..'; +import { getComponentMetaAtKey } from '../discovery.service'; const rolesKey = 'roles'; const Roles = (roles: string[]) => ReflectMetadata(rolesKey, roles); @@ -86,7 +87,7 @@ describe('Advanced Controller Discovery', () => { expect(guestControllers).toHaveLength(1); const [guestController] = guestControllers; - expect(guestController.discoveredClass.classType).toBe(GuestController); + expect(guestController.discoveredClass.injectType).toBe(GuestController); expect(guestController.discoveredClass.instance).toBeInstanceOf( GuestController ); @@ -111,9 +112,9 @@ describe('Advanced Controller Discovery', () => { expect(allMethods).toHaveLength(4); const fullPaths = allMethods.map(x => { - const controllerPath = Reflect.getMetadata( + const controllerPath = getComponentMetaAtKey( PATH_METADATA, - x.discoveredMethod.parentClass.classType + x.discoveredMethod.parentClass ); const methodPath = Reflect.getMetadata( diff --git a/packages/discovery/src/tests/async-providers.spec.ts b/packages/discovery/src/tests/async-providers.spec.ts deleted file mode 100644 index 00a999d0f..000000000 --- a/packages/discovery/src/tests/async-providers.spec.ts +++ /dev/null @@ -1,75 +0,0 @@ -import { Injectable, Module, ReflectMetadata } from '@nestjs/common'; -import { Test, TestingModule } from '@nestjs/testing'; -import { DiscoveryModule, DiscoveryService, withMetaAtKey } from '..'; - -const TestDecorator = (config: any) => ReflectMetadata('test', config); - -@Injectable() -@TestDecorator('dynamicProvider') -class DynamicProvider { - @TestDecorator('dynamicMethod') - doSomething() { - return 42; - } -} - -@Module({ - providers: [ - { - provide: DynamicProvider, - useFactory: async (): Promise => { - const dynamic = new DynamicProvider(); - return new Promise((resolve, reject) => { - setTimeout(() => resolve(dynamic), 100); - }); - } - } - ] -}) -class ExampleModule {} - -describe('DiscoveryWithDynamicProviders', () => { - let app: TestingModule; - let discoveryService: DiscoveryService; - - beforeEach(async () => { - app = await Test.createTestingModule({ - imports: [DiscoveryModule, ExampleModule] - }).compile(); - - await app.init(); - - discoveryService = app.get(DiscoveryService); - }); - - describe('Discovering Dynamic Providers', () => { - it('should discover providers based on a metadata key on dynamic async providers', async () => { - const providers = await discoveryService.providers(withMetaAtKey('test')); - - expect(providers).toHaveLength(1); - const [provider] = providers; - expect(provider.instance).toBeInstanceOf(DynamicProvider); - }); - - it('should discover provider method handler meta based on a metadata key', async () => { - const providerMethodMeta = await discoveryService.providerMethodsWithMetaAtKey( - 'test' - ); - - expect(providerMethodMeta.length).toBe(1); - - const meta = providerMethodMeta[0]; - - expect(meta).toMatchObject({ - meta: 'dynamicMethod', - discoveredMethod: { - methodName: 'doSomething' - } - }); - - expect(meta.discoveredMethod.parentClass.instance).toBeInstanceOf( - DynamicProvider - ); - }); - }); -}); diff --git a/packages/discovery/src/tests/discovery.spec.ts b/packages/discovery/src/tests/discovery.spec.ts index 9375bd038..9b74b8a03 100644 --- a/packages/discovery/src/tests/discovery.spec.ts +++ b/packages/discovery/src/tests/discovery.spec.ts @@ -67,7 +67,7 @@ describe('Discovery', () => { expect(providers).toHaveLength(1); const [provider] = providers; - expect(provider.classType).toBe(ExampleService); + expect(provider.injectType).toBe(ExampleService); expect(provider.instance).toBeInstanceOf(ExampleService); }); @@ -85,7 +85,8 @@ describe('Discovery', () => { discoveredMethod: { methodName: 'specialMethod', parentClass: { - classType: ExampleService + injectType: ExampleService, + dependencyType: ExampleService } } }); @@ -104,7 +105,7 @@ describe('Discovery', () => { expect(controllers).toHaveLength(1); const [controller] = controllers; - expect(controller.classType).toBe(ExampleController); + expect(controller.injectType).toBe(ExampleController); expect(controller.instance).toBeInstanceOf(ExampleController); }); @@ -123,7 +124,8 @@ describe('Discovery', () => { discoveredMethod: { methodName: 'get', parentClass: { - classType: ExampleController + injectType: ExampleController, + dependencyType: ExampleController } } }); diff --git a/packages/discovery/src/tests/provider-types.spec.ts b/packages/discovery/src/tests/provider-types.spec.ts new file mode 100644 index 000000000..841c0567b --- /dev/null +++ b/packages/discovery/src/tests/provider-types.spec.ts @@ -0,0 +1,181 @@ +import { Injectable, Module, ReflectMetadata } from '@nestjs/common'; +import { Test, TestingModule } from '@nestjs/testing'; +import { DiscoveryModule, DiscoveryService, withMetaAtKey } from '..'; + +const TestDecorator = (config: any) => ReflectMetadata('test', config); + +@Injectable() +@TestDecorator('dynamicProvider') +class DynamicProvider { + @TestDecorator('dynamicMethod') + doSomething() { + return 42; + } +} + +@Module({ + providers: [ + { + provide: DynamicProvider, + useFactory: async (): Promise => { + const dynamic = new DynamicProvider(); + return new Promise((resolve, reject) => { + setTimeout(() => resolve(dynamic), 50); + }); + } + } + ] +}) +class ExampleModule {} + +describe('Provider Types', () => { + describe('Value Providers', async () => { + let app: TestingModule; + let discover: DiscoveryService; + + beforeEach(async () => { + app = await Test.createTestingModule({ + imports: [DiscoveryModule], + providers: [ + { + provide: 'ValueProviderKey', + useValue: new DynamicProvider() + } + ] + }).compile(); + + await app.init(); + + discover = app.get(DiscoveryService); + }); + + it('should discover providers based on a metadata key', async () => { + const providers = await discover.providers(withMetaAtKey('test')); + + expect(providers).toHaveLength(1); + const [provider] = providers; + expect(provider.instance).toBeInstanceOf(DynamicProvider); + }); + + it('should discover provider method handler meta based on a metadata key', async () => { + const providerMethodMeta = await discover.providerMethodsWithMetaAtKey( + 'test' + ); + + expect(providerMethodMeta.length).toBe(1); + + const meta = providerMethodMeta[0]; + + expect(meta).toMatchObject({ + meta: 'dynamicMethod', + discoveredMethod: { + methodName: 'doSomething', + parentClass: { + name: 'ValueProviderKey' + } + } + }); + + expect(meta.discoveredMethod.parentClass.instance).toBeInstanceOf( + DynamicProvider + ); + }); + }); + + describe('Factory Providers', () => { + let app: TestingModule; + let discover: DiscoveryService; + + beforeEach(async () => { + app = await Test.createTestingModule({ + imports: [DiscoveryModule], + providers: [ + { + provide: 'FactoryProviderKey', + useFactory: () => new DynamicProvider() + } + ] + }).compile(); + + await app.init(); + + discover = app.get(DiscoveryService); + }); + + it('should discover providers based on a metadata key', async () => { + const providers = await discover.providers(withMetaAtKey('test')); + + expect(providers).toHaveLength(1); + const [provider] = providers; + expect(provider.instance).toBeInstanceOf(DynamicProvider); + }); + + it('should discover provider method handler meta based on a metadata key', async () => { + const providerMethodMeta = await discover.providerMethodsWithMetaAtKey( + 'test' + ); + + expect(providerMethodMeta.length).toBe(1); + + const meta = providerMethodMeta[0]; + + expect(meta).toMatchObject({ + meta: 'dynamicMethod', + discoveredMethod: { + methodName: 'doSomething', + parentClass: { + name: 'FactoryProviderKey' + } + } + }); + + expect(meta.discoveredMethod.parentClass.instance).toBeInstanceOf( + DynamicProvider + ); + }); + }); + + describe('Async Factory Providers', () => { + let app: TestingModule; + let discover: DiscoveryService; + + beforeEach(async () => { + app = await Test.createTestingModule({ + imports: [DiscoveryModule, ExampleModule] + }).compile(); + + await app.init(); + + discover = app.get(DiscoveryService); + }); + + it('should discover providers based on a metadata key', async () => { + const providers = await discover.providers(withMetaAtKey('test')); + + expect(providers).toHaveLength(1); + const [provider] = providers; + expect(provider.instance).toBeInstanceOf(DynamicProvider); + }); + + it('should discover provider method handler meta based on a metadata key', async () => { + const providerMethodMeta = await discover.providerMethodsWithMetaAtKey( + 'test' + ); + + expect(providerMethodMeta.length).toBe(1); + + const meta = providerMethodMeta[0]; + + expect(meta).toMatchObject({ + meta: 'dynamicMethod', + discoveredMethod: { + methodName: 'doSomething' + } + }); + + expect(meta.discoveredMethod.parentClass.instance).toBeInstanceOf( + DynamicProvider + ); + }); + }); +});