From 19ec0292eedd94cb2e40e69af8897703fc8f55c7 Mon Sep 17 00:00:00 2001 From: Kurten Date: Tue, 16 Jun 2020 18:09:46 +0800 Subject: [PATCH] fix: 2.x extends bug (#498) --- .github/workflows/nodejs.yml | 2 +- .../test/context/midwayContainer.test.ts | 12 +- .../src/annotation/objectDef.ts | 2 +- .../src/common/decoratorManager.ts | 137 ++++++++++++------ .../test/fixtures/decorator/customClass.ts | 18 ++- .../test/web/controller.test.ts | 23 ++- packages/midway-init/test/init.test.js | 4 +- 7 files changed, 138 insertions(+), 60 deletions(-) diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index 2bab69b3f33e..970adf2068f1 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -24,7 +24,7 @@ jobs: - run: npm run cov - name: Upload coverage to Codecov uses: codecov/codecov-action@v1 - - name: Checkout Midway FaaS + - name: Run Midway FaaS case use link uses: actions/checkout@v2 with: repository: midwayjs/midway-faas diff --git a/packages/midway-core/test/context/midwayContainer.test.ts b/packages/midway-core/test/context/midwayContainer.test.ts index b8369a88705d..3c203f2d3f5a 100644 --- a/packages/midway-core/test/context/midwayContainer.test.ts +++ b/packages/midway-core/test/context/midwayContainer.test.ts @@ -19,18 +19,12 @@ describe('/test/context/midwayContainer.test.ts', () => { loadDir: path.join(__dirname, '../fixtures/ts-app-inject') }); - const origin = decs.getClassMetadata; - - mm(decs, 'getClassMetadata', (key, target) => { - if (key === decs.CLASS_KEY_CONSTRUCTOR) { - throw new Error('mock error'); - } - return origin(key, target); - }); + (decs as any).throwErrorForTest(decs.CLASS_KEY_CONSTRUCTOR, new Error('mock error')); const tt = container.get('testCons'); expect(tt.ts).gt(0); - mm.restore(); + + (decs as any).throwErrorForTest(decs.CLASS_KEY_CONSTRUCTOR); const app = container.get('app') as App; expect(app.loader).not.to.be.undefined; diff --git a/packages/midway-decorator/src/annotation/objectDef.ts b/packages/midway-decorator/src/annotation/objectDef.ts index ab35224c8468..1bbdbc900361 100644 --- a/packages/midway-decorator/src/annotation/objectDef.ts +++ b/packages/midway-decorator/src/annotation/objectDef.ts @@ -1,6 +1,6 @@ import { ScopeEnum, saveObjectDefProps } from '../common'; -const debug = require('debug')('injection:context:obj_def'); +const debug = require('debug')('decorator:context:obj_def'); export function Async() { return function (target: any): void { diff --git a/packages/midway-decorator/src/common/decoratorManager.ts b/packages/midway-decorator/src/common/decoratorManager.ts index 33175245ee09..029ec02022e1 100644 --- a/packages/midway-decorator/src/common/decoratorManager.ts +++ b/packages/midway-decorator/src/common/decoratorManager.ts @@ -3,6 +3,8 @@ import { ObjectDefinitionOptions, TagClsMetadata } from '../interface'; import { OBJ_DEF_CLS, TAGGED_CLS } from './constant'; import { classNamed } from './utils'; +const debug = require('debug')('decorator:manager'); + const STRIP_COMMENTS = /((\/\/.*$)|(\/\*[\s\S]*?\*\/))/mg; const ARGUMENT_NAMES = /([^\s,]+)/g; @@ -53,28 +55,71 @@ export class DecoratorManager extends Map { return DecoratorManager.getDecoratorClsMethodPrefix(decoratorNameKey) + ':' + methodKey.toString(); } + static getDecoratorMethod(decoratorNameKey: decoratorKey, methodKey: decoratorKey) { + return DecoratorManager.getDecoratorMethodKey(decoratorNameKey) + '_' + methodKey.toString(); + } + listModule(key) { return Array.from(this.get(key) || {}); } - static getOriginMetadata(metaKey, target, method?) { - if (method) { - // for property - if (!Reflect.hasMetadata(metaKey, target, method)) { - Reflect.defineMetadata(metaKey, new Map(), target, method); - } - return Reflect.getMetadata(metaKey, target, method); + static saveMetadata(metaKey: string, target: any, dataKey: string, data: any) { + debug('saveMetadata %s on target %o with dataKey = %s.', metaKey, target, dataKey); + // filter Object.create(null) + if (typeof target === 'object' && target.constructor) { + target = target.constructor; + } + + let m: Map; + if (Reflect.hasOwnMetadata(metaKey, target)) { + m = Reflect.getMetadata(metaKey, target); } else { - // filter Object.create(null) - if (typeof target === 'object' && target.constructor) { - target = target.constructor; - } - // for class - if (!Reflect.hasMetadata(metaKey, target)) { - Reflect.defineMetadata(metaKey, new Map(), target); - } - return Reflect.getMetadata(metaKey, target); + m = new Map(); } + + m.set(dataKey, data); + Reflect.defineMetadata(metaKey, m, target); + } + + static attachMetadata(metaKey: string, target: any, dataKey: string, data: any) { + debug('attachMetadata %s on target %o with dataKey = %s.', metaKey, target, dataKey); + // filter Object.create(null) + if (typeof target === 'object' && target.constructor) { + target = target.constructor; + } + + let m: Map; + if (Reflect.hasOwnMetadata(metaKey, target)) { + m = Reflect.getMetadata(metaKey, target); + } else { + m = new Map(); + } + + if (!m.has(dataKey)) { + m.set(dataKey, []); + } + m.get(dataKey).push(data); + Reflect.defineMetadata(metaKey, m, target); + } + + static getMetadata(metaKey: string, target: any, dataKey?: string) { + debug('getMetadata %s on target %o with dataKey = %s.', metaKey, target, dataKey); + // filter Object.create(null) + if (typeof target === 'object' && target.constructor) { + target = target.constructor; + } + + let m: Map; + if (!Reflect.hasOwnMetadata(metaKey, target)) { + m = new Map(); + Reflect.defineMetadata(metaKey, m, target); + } else { + m = Reflect.getMetadata(metaKey, target); + } + if (!dataKey) { + return m; + } + return m.get(dataKey); } /** @@ -86,11 +131,11 @@ export class DecoratorManager extends Map { */ saveMetadata(decoratorNameKey: decoratorKey, data, target, propertyName?) { if (propertyName) { - const originMap = DecoratorManager.getOriginMetadata(this.injectMethodKeyPrefix, target, propertyName); - originMap.set(DecoratorManager.getDecoratorMethodKey(decoratorNameKey), data); + const dataKey = DecoratorManager.getDecoratorMethod(decoratorNameKey, propertyName); + DecoratorManager.saveMetadata(this.injectMethodKeyPrefix, target, dataKey, data); } else { - const originMap = DecoratorManager.getOriginMetadata(this.injectClassKeyPrefix, target); - originMap.set(DecoratorManager.getDecoratorClassKey(decoratorNameKey), data); + const dataKey = DecoratorManager.getDecoratorClassKey(decoratorNameKey); + DecoratorManager.saveMetadata(this.injectClassKeyPrefix, target, dataKey, data); } } @@ -102,19 +147,13 @@ export class DecoratorManager extends Map { * @param propertyName */ attachMetadata(decoratorNameKey: decoratorKey, data, target, propertyName?) { - let originMap; - let key; if (propertyName) { - originMap = DecoratorManager.getOriginMetadata(this.injectMethodKeyPrefix, target, propertyName); - key = DecoratorManager.getDecoratorMethodKey(decoratorNameKey); + const dataKey = DecoratorManager.getDecoratorMethod(decoratorNameKey, propertyName); + DecoratorManager.attachMetadata(this.injectMethodKeyPrefix, target, dataKey, data); } else { - originMap = DecoratorManager.getOriginMetadata(this.injectClassKeyPrefix, target); - key = DecoratorManager.getDecoratorClassKey(decoratorNameKey); + const dataKey = DecoratorManager.getDecoratorClassKey(decoratorNameKey); + DecoratorManager.attachMetadata(this.injectClassKeyPrefix, target, dataKey, data); } - if (!originMap.has(key)) { - originMap.set(key, []); - } - originMap.get(key).push(data); } /** @@ -125,11 +164,11 @@ export class DecoratorManager extends Map { */ getMetadata(decoratorNameKey: decoratorKey, target, propertyName?) { if (propertyName) { - const originMap = DecoratorManager.getOriginMetadata(this.injectMethodKeyPrefix, target, propertyName); - return originMap.get(DecoratorManager.getDecoratorMethodKey(decoratorNameKey)); + const dataKey = DecoratorManager.getDecoratorMethod(decoratorNameKey, propertyName); + return DecoratorManager.getMetadata(this.injectMethodKeyPrefix, target, dataKey); } else { - const originMap = DecoratorManager.getOriginMetadata(this.injectClassKeyPrefix, target); - return originMap.get(DecoratorManager.getDecoratorClassKey(decoratorNameKey)); + const dataKey = `${DecoratorManager.getDecoratorClassKey(decoratorNameKey)}`; + return DecoratorManager.getMetadata(this.injectClassKeyPrefix, target, dataKey); } } @@ -141,8 +180,8 @@ export class DecoratorManager extends Map { * @param propertyName */ savePropertyDataToClass(decoratorNameKey: decoratorKey, data, target, propertyName) { - const originMap = DecoratorManager.getOriginMetadata(this.injectClassMethodKeyPrefix, target); - originMap.set(DecoratorManager.getDecoratorClsMethodKey(decoratorNameKey, propertyName), data); + const dataKey = DecoratorManager.getDecoratorClsMethodKey(decoratorNameKey, propertyName); + DecoratorManager.saveMetadata(this.injectClassMethodKeyPrefix, target, dataKey, data); } /** @@ -153,12 +192,8 @@ export class DecoratorManager extends Map { * @param propertyName */ attachPropertyDataToClass(decoratorNameKey: decoratorKey, data, target, propertyName) { - const originMap = DecoratorManager.getOriginMetadata(this.injectClassMethodKeyPrefix, target); - const key = DecoratorManager.getDecoratorClsMethodKey(decoratorNameKey, propertyName); - if (!originMap.has(key)) { - originMap.set(key, []); - } - originMap.get(key).push(data); + const dataKey = DecoratorManager.getDecoratorClsMethodKey(decoratorNameKey, propertyName); + DecoratorManager.attachMetadata(this.injectClassMethodKeyPrefix, target, dataKey, data); } /** @@ -168,8 +203,8 @@ export class DecoratorManager extends Map { * @param propertyName */ getPropertyDataFromClass(decoratorNameKey: decoratorKey, target, propertyName) { - const originMap = DecoratorManager.getOriginMetadata(this.injectClassMethodKeyPrefix, target); - return originMap.get(DecoratorManager.getDecoratorClsMethodKey(decoratorNameKey, propertyName)); + const dataKey = DecoratorManager.getDecoratorClsMethodKey(decoratorNameKey, propertyName); + return DecoratorManager.getMetadata(this.injectClassMethodKeyPrefix, target, dataKey); } /** @@ -178,7 +213,7 @@ export class DecoratorManager extends Map { * @param target */ listPropertyDataFromClass(decoratorNameKey: decoratorKey, target) { - const originMap = DecoratorManager.getOriginMetadata(this.injectClassMethodKeyPrefix, target); + const originMap = DecoratorManager.getMetadata(this.injectClassMethodKeyPrefix, target); const res = []; for (const [ key, value ] of originMap) { if (key.indexOf(DecoratorManager.getDecoratorClsMethodPrefix(decoratorNameKey)) !== -1) { @@ -211,14 +246,26 @@ export function attachClassMetadata(decoratorNameKey: decoratorKey, data, target return manager.attachMetadata(decoratorNameKey, data, target); } +const testKeyMap = new Map(); /** * get data from class * @param decoratorNameKey * @param target */ export function getClassMetadata(decoratorNameKey: decoratorKey, target) { + if (testKeyMap.size > 0 && testKeyMap.has(decoratorNameKey)) { + throw testKeyMap.get(decoratorNameKey); + } return manager.getMetadata(decoratorNameKey, target); } +// TODO 因 https://github.com/microsoft/TypeScript/issues/38820 等 4.0 发布移除掉 +export function throwErrorForTest(key: decoratorKey, e: Error) { + if (e) { + testKeyMap.set(key, e); + } else { + testKeyMap.delete(key); + } +} /** * save method data to class diff --git a/packages/midway-decorator/test/fixtures/decorator/customClass.ts b/packages/midway-decorator/test/fixtures/decorator/customClass.ts index 3c2ea0c3be28..99d370fd8062 100644 --- a/packages/midway-decorator/test/fixtures/decorator/customClass.ts +++ b/packages/midway-decorator/test/fixtures/decorator/customClass.ts @@ -1,5 +1,5 @@ import { attachClass, attachMethod, customCls, customMethod, preload, propertyKeyA, propertyKeyB } from './custom'; -import { Provide, Scope, ScopeEnum } from '../../../src'; +import { Provide, Scope, ScopeEnum, Logger, Controller } from '../../../src'; @Provide() @Scope(ScopeEnum.Singleton) @@ -30,3 +30,19 @@ export class ManagerTest { } } + +export class Base { + @Logger('logbase') + logger: any; +} + +@Provide() +@Controller('/api/one') +export class ControllerOne extends Base { + +} +@Provide() +@Controller('/api/two') +export class ControllerTwo extends Base { + +} diff --git a/packages/midway-decorator/test/web/controller.test.ts b/packages/midway-decorator/test/web/controller.test.ts index 0b0e42467b6f..121723fb9018 100644 --- a/packages/midway-decorator/test/web/controller.test.ts +++ b/packages/midway-decorator/test/web/controller.test.ts @@ -1,6 +1,7 @@ import { expect } from 'chai'; import { Controller, CONTROLLER_KEY, listModule, getClassMetadata, ScopeEnum, getObjectDefProps } from '../../src'; +import { ControllerOne, ControllerTwo } from '../fixtures/decorator/customClass'; @Controller('/hhh', { sensitive: true, @@ -37,6 +38,26 @@ describe('/test/web/controller.test.ts', () => { }); const m = listModule(CONTROLLER_KEY); - expect(m.length).eq(2); + expect(m.length).eq(4); + }); + + it('controller extends should be ok', () => { + const metaone = getClassMetadata(CONTROLLER_KEY, ControllerOne); + expect(metaone).deep.eq({ + prefix: '/api/one', + routerOptions: { + sensitive: true, + middleware: [] + } + }); + + const metatwo = getClassMetadata(CONTROLLER_KEY, ControllerTwo); + expect(metatwo).deep.eq({ + prefix: '/api/two', + routerOptions: { + sensitive: true, + middleware: [] + } + }); }); }); diff --git a/packages/midway-init/test/init.test.js b/packages/midway-init/test/init.test.js index 43d00d32d231..92b5dd7a6abd 100755 --- a/packages/midway-init/test/init.test.js +++ b/packages/midway-init/test/init.test.js @@ -173,7 +173,7 @@ describe('test/init.test.js', () => { 'test', ], ]); - await command.run(tmp, '--dir=.tmp/test --package=midway-boilerplate-typescript --registry=https://registry.cnpmjs.org'); + await command.run(tmp, '--dir=.tmp/test --package=midway-boilerplate-typescript --registry=https://r.cnpmjs.org'); assert(fs.existsSync(path.join(targetDir, '.gitignore'))); assert(fs.existsSync(path.join(targetDir, 'package.json'))); @@ -190,7 +190,7 @@ describe('test/init.test.js', () => { 'test', ], ]); - await command.run(tmp, '--dir=.tmp/test --package=midway-boilerplate-typescript --registry=registry.cnpmjs.org'); + await command.run(tmp, '--dir=.tmp/test --package=midway-boilerplate-typescript --registry=r.cnpmjs.org'); assert(fs.existsSync(path.join(targetDir, '.gitignore'))); assert(fs.existsSync(path.join(targetDir, 'package.json')));