From a1733936977b3136b551f4e04df930b26ddfa04e Mon Sep 17 00:00:00 2001 From: garrettmills Date: Tue, 13 Sep 2022 22:34:16 -0500 Subject: [PATCH] Fix property injection prototype hoisting bug --- package.json | 2 +- .../AuthenticatableRepositoryFactory.ts | 4 +-- .../repositories/ClientRepositoryFactory.ts | 4 +-- .../RedemptionCodeRepositoryFactory.ts | 4 +-- .../repositories/ScopeRepositoryFactory.ts | 4 +-- .../repositories/TokenRepositoryFactory.ts | 4 +-- .../decorator/getPropertyInjectionMetadata.ts | 9 ++++++ src/di/decorator/injection.ts | 32 +++++++++++++++---- src/di/decorator/propertyInjectionMetadata.ts | 2 ++ src/di/factory/Factory.ts | 4 +-- src/di/index.ts | 1 + src/http/session/SessionFactory.ts | 4 +-- src/orm/migrations/MigratorFactory.ts | 8 +++-- src/support/bus/queue/QueueFactory.ts | 5 ++- src/support/cache/CacheFactory.ts | 4 +-- src/views/ViewEngineFactory.ts | 4 +-- 16 files changed, 64 insertions(+), 31 deletions(-) create mode 100644 src/di/decorator/getPropertyInjectionMetadata.ts create mode 100644 src/di/decorator/propertyInjectionMetadata.ts diff --git a/package.json b/package.json index ebe2977..817c8be 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@extollo/lib", - "version": "0.14.1", + "version": "0.14.2", "description": "The framework library that lifts up your code.", "main": "lib/index.js", "types": "lib/index.d.ts", diff --git a/src/auth/repository/AuthenticatableRepositoryFactory.ts b/src/auth/repository/AuthenticatableRepositoryFactory.ts index 8d32a5a..48897bb 100644 --- a/src/auth/repository/AuthenticatableRepositoryFactory.ts +++ b/src/auth/repository/AuthenticatableRepositoryFactory.ts @@ -5,7 +5,7 @@ import { PropertyDependency, isInstantiable, DEPENDENCY_KEYS_METADATA_KEY, - DEPENDENCY_KEYS_PROPERTY_METADATA_KEY, Instantiable, FactoryProducer, + Instantiable, FactoryProducer, getPropertyInjectionMetadata, } from '../../di' import {Collection, ErrorWithContext} from '../../util' import {Config} from '../../service/Config' @@ -43,7 +43,7 @@ export class AuthenticatableRepositoryFactory extends AbstractFactory { let currentToken = this.getClientRepositoryClass() do { - const loadedMeta = Reflect.getMetadata(DEPENDENCY_KEYS_PROPERTY_METADATA_KEY, currentToken) + const loadedMeta = getPropertyInjectionMetadata(currentToken) if ( loadedMeta ) { meta.concat(loadedMeta) } diff --git a/src/auth/server/repositories/RedemptionCodeRepositoryFactory.ts b/src/auth/server/repositories/RedemptionCodeRepositoryFactory.ts index 2c2c9fa..7afe739 100644 --- a/src/auth/server/repositories/RedemptionCodeRepositoryFactory.ts +++ b/src/auth/server/repositories/RedemptionCodeRepositoryFactory.ts @@ -5,7 +5,7 @@ import { PropertyDependency, isInstantiable, DEPENDENCY_KEYS_METADATA_KEY, - DEPENDENCY_KEYS_PROPERTY_METADATA_KEY, Instantiable, FactoryProducer, + Instantiable, FactoryProducer, getPropertyInjectionMetadata, } from '../../../di' import {Collection, ErrorWithContext} from '../../../util' import {Config} from '../../../service/Config' @@ -43,7 +43,7 @@ export class RedemptionCodeRepositoryFactory extends AbstractFactory { let currentToken = this.getScopeRepositoryClass() do { - const loadedMeta = Reflect.getMetadata(DEPENDENCY_KEYS_PROPERTY_METADATA_KEY, currentToken) + const loadedMeta = getPropertyInjectionMetadata(currentToken) if ( loadedMeta ) { meta.concat(loadedMeta) } diff --git a/src/auth/server/repositories/TokenRepositoryFactory.ts b/src/auth/server/repositories/TokenRepositoryFactory.ts index 195e280..1b437a6 100644 --- a/src/auth/server/repositories/TokenRepositoryFactory.ts +++ b/src/auth/server/repositories/TokenRepositoryFactory.ts @@ -5,7 +5,7 @@ import { PropertyDependency, isInstantiable, DEPENDENCY_KEYS_METADATA_KEY, - DEPENDENCY_KEYS_PROPERTY_METADATA_KEY, Instantiable, FactoryProducer, + Instantiable, FactoryProducer, getPropertyInjectionMetadata, } from '../../../di' import {Collection, ErrorWithContext} from '../../../util' import {Config} from '../../../service/Config' @@ -43,7 +43,7 @@ export class TokenRepositoryFactory extends AbstractFactory { let currentToken = this.getTokenRepositoryClass() do { - const loadedMeta = Reflect.getMetadata(DEPENDENCY_KEYS_PROPERTY_METADATA_KEY, currentToken) + const loadedMeta = getPropertyInjectionMetadata(currentToken) if ( loadedMeta ) { meta.concat(loadedMeta) } diff --git a/src/di/decorator/getPropertyInjectionMetadata.ts b/src/di/decorator/getPropertyInjectionMetadata.ts new file mode 100644 index 0000000..df5504b --- /dev/null +++ b/src/di/decorator/getPropertyInjectionMetadata.ts @@ -0,0 +1,9 @@ +import {Instantiable, PropertyDependency} from '../types' +import {Collection, logIfDebugging} from '../../util' +import {propertyInjectionMetadata} from './propertyInjectionMetadata' + +export function getPropertyInjectionMetadata(token: Instantiable): Collection { + const loadedMeta = ((token as any)[propertyInjectionMetadata] || new Collection()) as Collection + logIfDebugging('extollo.di.injection', 'getPropertyInjectionMetadata() target:', token, 'loaded:', loadedMeta) + return loadedMeta +} diff --git a/src/di/decorator/injection.ts b/src/di/decorator/injection.ts index 94461c0..ccebbbb 100644 --- a/src/di/decorator/injection.ts +++ b/src/di/decorator/injection.ts @@ -2,16 +2,16 @@ import 'reflect-metadata' import {collect, Collection} from '../../util' import {logIfDebugging} from '../../util/support/debug' import { + DEPENDENCY_KEYS_METADATA_KEY, + DEPENDENCY_KEYS_SERVICE_TYPE_KEY, DependencyKey, DependencyRequirement, - DEPENDENCY_KEYS_METADATA_KEY, - DEPENDENCY_KEYS_PROPERTY_METADATA_KEY, - isInstantiable, InjectionType, - DEPENDENCY_KEYS_SERVICE_TYPE_KEY, + isInstantiable, PropertyDependency, } from '../types' import {ContainerBlueprint} from '../ContainerBlueprint' +import {propertyInjectionMetadata} from './propertyInjectionMetadata' /** * Get a collection of dependency requirements for the given target object. @@ -67,6 +67,7 @@ export const Injectable = (): ClassDecorator => { } } + /** * Mark the given class property to be injected by the container. * If a `key` is specified, that DependencyKey will be injected. @@ -77,10 +78,27 @@ export const Injectable = (): ClassDecorator => { */ export const Inject = (key?: DependencyKey, { debug = false } = {}): PropertyDecorator => { return (target, property) => { - let propertyMetadata = Reflect.getMetadata(DEPENDENCY_KEYS_PROPERTY_METADATA_KEY, target?.constructor || target) as Collection + if ( !target?.constructor ) { + logIfDebugging('extollo.di.decoration', '[DEBUG] @Inject(): target has no constructor', target) + throw new Error('Unable to define property injection: target has no constructor. Enable `extollo.di.decoration` logging to debug') + } + + const propertyTarget = target.constructor + + // let propertyMetadata = Reflect.getMetadata(DEPENDENCY_KEYS_PROPERTY_METADATA_KEY, propertyTarget) as Collection + // Okay, this is a little fucky. We can't use Reflect's metadata capabilities because we need to write the metadata to + // the constructor, not the `target`. Because Reflect is using the prototype to store data, defining a metadata key on the constructor + // will define it for its parent constructors as well. + // So, if you have class A, class B extends A, and class C extends A, the properties for B and C will be defined on A, causing + // BOTH B and C's properties to be injected on any class extending A. + // To get around this, we instead define a custom property on the constructor itself, then use hasOwnProperty to make sure we're not + // getting the one for the parent class via the prototype chain. + let propertyMetadata = Object.prototype.hasOwnProperty.call(propertyTarget, propertyInjectionMetadata) ? + (propertyTarget as any)[propertyInjectionMetadata] as Collection : undefined + if ( !propertyMetadata ) { propertyMetadata = new Collection() - Reflect.defineMetadata(DEPENDENCY_KEYS_PROPERTY_METADATA_KEY, propertyMetadata, target?.constructor || target) + ;(propertyTarget as any)[propertyInjectionMetadata] = propertyMetadata } const type = Reflect.getMetadata('design:type', target, property) @@ -103,7 +121,7 @@ export const Inject = (key?: DependencyKey, { debug = false } = {}): PropertyDec logIfDebugging('extollo.di.decoration', '[DEBUG] @Inject() - key:', key, 'property:', property, 'target:', target, 'target constructor:', target?.constructor, 'type:', type) - Reflect.defineMetadata(DEPENDENCY_KEYS_PROPERTY_METADATA_KEY, propertyMetadata, target?.constructor || target) + ;(propertyTarget as any)[propertyInjectionMetadata] = propertyMetadata } } diff --git a/src/di/decorator/propertyInjectionMetadata.ts b/src/di/decorator/propertyInjectionMetadata.ts new file mode 100644 index 0000000..fb6298e --- /dev/null +++ b/src/di/decorator/propertyInjectionMetadata.ts @@ -0,0 +1,2 @@ + +export const propertyInjectionMetadata = Symbol('@extollo/lib:propertyInjectionMetadata') diff --git a/src/di/factory/Factory.ts b/src/di/factory/Factory.ts index f5bd387..ca16d0b 100644 --- a/src/di/factory/Factory.ts +++ b/src/di/factory/Factory.ts @@ -1,13 +1,13 @@ import {AbstractFactory} from './AbstractFactory' import { DEPENDENCY_KEYS_METADATA_KEY, - DEPENDENCY_KEYS_PROPERTY_METADATA_KEY, DependencyRequirement, Instantiable, PropertyDependency, } from '../types' import {Collection, logIfDebugging} from '../../util' import 'reflect-metadata' +import {getPropertyInjectionMetadata} from '../decorator/getPropertyInjectionMetadata' /** * Standard static-class factory. The token of this factory is a reference to a @@ -57,7 +57,7 @@ export class Factory extends AbstractFactory { let currentToken = this.token do { - const loadedMeta = Reflect.getMetadata(DEPENDENCY_KEYS_PROPERTY_METADATA_KEY, currentToken) + const loadedMeta = getPropertyInjectionMetadata(currentToken) logIfDebugging('extollo.di.injection', 'Factory.getInjectedProperties() target:', currentToken, 'loaded:', loadedMeta) if ( loadedMeta ) { meta.concat(loadedMeta) diff --git a/src/di/index.ts b/src/di/index.ts index 5c10236..a8ece7c 100644 --- a/src/di/index.ts +++ b/src/di/index.ts @@ -13,5 +13,6 @@ export * from './ScopedContainer' export * from './types' export * from './decorator/injection' +export * from './decorator/getPropertyInjectionMetadata' export * from './InjectionAware' export * from './constructable' diff --git a/src/http/session/SessionFactory.ts b/src/http/session/SessionFactory.ts index ddea866..e43baf5 100644 --- a/src/http/session/SessionFactory.ts +++ b/src/http/session/SessionFactory.ts @@ -5,7 +5,7 @@ import { PropertyDependency, isInstantiable, DEPENDENCY_KEYS_METADATA_KEY, - DEPENDENCY_KEYS_PROPERTY_METADATA_KEY, Instantiable, + Instantiable, getPropertyInjectionMetadata, } from '../../di' import {Collection, ErrorWithContext} from '../../util' import {MemorySession} from './MemorySession' @@ -52,7 +52,7 @@ export class SessionFactory extends AbstractFactory { let currentToken = this.getSessionClass() do { - const loadedMeta = Reflect.getMetadata(DEPENDENCY_KEYS_PROPERTY_METADATA_KEY, currentToken) + const loadedMeta = getPropertyInjectionMetadata(currentToken) if ( loadedMeta ) { meta.concat(loadedMeta) } diff --git a/src/orm/migrations/MigratorFactory.ts b/src/orm/migrations/MigratorFactory.ts index 1d7b4de..fa2dc01 100644 --- a/src/orm/migrations/MigratorFactory.ts +++ b/src/orm/migrations/MigratorFactory.ts @@ -4,7 +4,11 @@ import { PropertyDependency, isInstantiable, DEPENDENCY_KEYS_METADATA_KEY, - DEPENDENCY_KEYS_PROPERTY_METADATA_KEY, Instantiable, Injectable, Inject, FactoryProducer, + Instantiable, + Injectable, + Inject, + FactoryProducer, + getPropertyInjectionMetadata, } from '../../di' import {Collection, ErrorWithContext} from '../../util' import {Logging} from '../../service/Logging' @@ -51,7 +55,7 @@ export class MigratorFactory extends AbstractFactory { let currentToken = this.getMigratorClass() do { - const loadedMeta = Reflect.getMetadata(DEPENDENCY_KEYS_PROPERTY_METADATA_KEY, currentToken) + const loadedMeta = getPropertyInjectionMetadata(currentToken) if ( loadedMeta ) { meta.concat(loadedMeta) } diff --git a/src/support/bus/queue/QueueFactory.ts b/src/support/bus/queue/QueueFactory.ts index b3a851c..f68221d 100644 --- a/src/support/bus/queue/QueueFactory.ts +++ b/src/support/bus/queue/QueueFactory.ts @@ -5,9 +5,8 @@ import { PropertyDependency, isInstantiable, DEPENDENCY_KEYS_METADATA_KEY, - DEPENDENCY_KEYS_PROPERTY_METADATA_KEY, StaticInstantiable, - FactoryProducer, + FactoryProducer, getPropertyInjectionMetadata, } from '../../../di' import {Collection, ErrorWithContext} from '../../../util' import {Logging} from '../../../service/Logging' @@ -52,7 +51,7 @@ export class QueueFactory extends AbstractFactory { let currentToken = this.getQueueClass() do { - const loadedMeta = Reflect.getMetadata(DEPENDENCY_KEYS_PROPERTY_METADATA_KEY, currentToken) + const loadedMeta = getPropertyInjectionMetadata(currentToken) if ( loadedMeta ) { meta.concat(loadedMeta) } diff --git a/src/support/cache/CacheFactory.ts b/src/support/cache/CacheFactory.ts index cc3ae77..c656885 100644 --- a/src/support/cache/CacheFactory.ts +++ b/src/support/cache/CacheFactory.ts @@ -5,7 +5,7 @@ import { PropertyDependency, isInstantiable, DEPENDENCY_KEYS_METADATA_KEY, - DEPENDENCY_KEYS_PROPERTY_METADATA_KEY, StaticClass, Instantiable, + StaticClass, Instantiable, getPropertyInjectionMetadata, } from '../../di' import {Collection, ErrorWithContext} from '../../util' import {Logging} from '../../service/Logging' @@ -52,7 +52,7 @@ export class CacheFactory extends AbstractFactory { let currentToken = this.getCacheClass() do { - const loadedMeta = Reflect.getMetadata(DEPENDENCY_KEYS_PROPERTY_METADATA_KEY, currentToken) + const loadedMeta = getPropertyInjectionMetadata(currentToken) if ( loadedMeta ) { meta.concat(loadedMeta) } diff --git a/src/views/ViewEngineFactory.ts b/src/views/ViewEngineFactory.ts index 0118922..b220137 100644 --- a/src/views/ViewEngineFactory.ts +++ b/src/views/ViewEngineFactory.ts @@ -5,7 +5,7 @@ import { PropertyDependency, isInstantiable, DEPENDENCY_KEYS_METADATA_KEY, - DEPENDENCY_KEYS_PROPERTY_METADATA_KEY, StaticClass, Instantiable, + StaticClass, Instantiable, getPropertyInjectionMetadata, } from '../di' import {Collection, ErrorWithContext} from '../util' import {Logging} from '../service/Logging' @@ -49,7 +49,7 @@ export class ViewEngineFactory extends AbstractFactory { let currentToken = this.getViewEngineClass() do { - const loadedMeta = Reflect.getMetadata(DEPENDENCY_KEYS_PROPERTY_METADATA_KEY, currentToken) + const loadedMeta = getPropertyInjectionMetadata(currentToken) if ( loadedMeta ) { meta.concat(loadedMeta) }