fix: fix dfs circular (#457)

* fix dfs circular depth bug

* remove lodash

packages/midway-core/package.json

@@ -37,10 +37,7 @@
     "lodash.assign": "^4.2.0",
     "lodash.clonedeep": "^4.5.0",
     "lodash.defaultsdeep": "^4.6.0",
-    "lodash.forown": "^4.4.0",
     "lodash.get": "^4.4.2",
-    "lodash.has": "^4.5.2",
-    "lodash.keys": "^4.2.0",
     "lodash.set": "^4.3.2",
     "lodash.template": "^4.4.0",
     "reflect-metadata": "^0.1.13"

packages/midway-core/src/common/lodashWrap.ts

@@ -3,9 +3,6 @@ import * as assign from 'lodash.assign';
 import * as get from 'lodash.get';
 import * as set from 'lodash.set';
 import * as template from 'lodash.template';
-import * as forOwn from 'lodash.forown';
-import * as keys from 'lodash.keys';
-import * as has from 'lodash.has';
 import * as cloneDeep from 'lodash.clonedeep';
 import * as defaultsDeep from 'lodash.defaultsdeep';
 
@@ -17,9 +14,6 @@ export {
   get,
   isArray,
   template,
-  forOwn,
-  keys,
-  has,
   cloneDeep,
   defaultsDeep,
 };

packages/midway-core/src/context/managedResolverFactory.ts

@@ -601,13 +601,14 @@ export class ManagedResolverFactory {
    */
   private createProxyReference(definition: IObjectDefinition): any {
     if (this.isCreating(definition)) {
+      debug('create proxy for %s.', definition.id);
       // 非循环依赖的允许重新创建对象
       if (!this.depthFirstSearch(definition.id, definition)) {
         debug('id = %s after dfs return null', definition.id);
         return null;
       }
       // 创建代理对象
-      return new Proxy({ __is_proxy__: true }, {
+      return new Proxy({ __is_proxy__: true, __target_id__: definition.id }, {
         get: (obj, prop) => {
           let target;
           if (definition.isRequestScope()) {
@@ -636,37 +637,53 @@ export class ManagedResolverFactory {
    * @param identifier 目标id
    * @param definition 定义描述
    */
-  public depthFirstSearch(identifier: string, definition: IObjectDefinition): boolean {
+  public depthFirstSearch(identifier: string, definition: IObjectDefinition, depth?: string[]): boolean {
     if (definition) {
+      debug('dfs for %s == %s start.', identifier, definition.id);
       if (definition.constructorArgs) {
         const args = definition.constructorArgs.map(val => (val as ManagedReference).name);
         if (args.indexOf(identifier) > -1) {
+          debug('dfs exist in constructor %s == %s.', identifier, definition.id);
           return true;
         }
       }
       if (definition.properties) {
         const keys = definition.properties.keys();
         if (keys.indexOf(identifier) > -1) {
+          debug('dfs exist in properties %s == %s.', identifier, definition.id);
           return true;
         }
         for (const key of keys) {
+          if (!Array.isArray(depth)) {
+            depth = [identifier];
+          }
           let iden = key;
           const ref: ManagedReference = definition.properties.get(key);
           if (ref && ref.name) {
             iden = ref.name;
           }
           if (iden === identifier) {
+            debug('dfs exist in properties key %s == %s.', identifier, definition.id);
             return true;
           }
+          if (depth.indexOf(iden) > -1) {
+            debug('dfs depth circular %s == %s, %s, %j.', identifier, definition.id, iden, depth);
+            continue;
+          } else {
+            depth.push(iden);
+            debug('dfs depth push %s == %s, %j.', identifier, iden, depth);
+          }
           let subDefinition = this.context.registry.getDefinition(iden);
           if (!subDefinition && this.context.parent) {
             subDefinition = this.context.parent.registry.getDefinition(iden);
           }
-          if (this.depthFirstSearch(identifier, subDefinition)) {
+          if (this.depthFirstSearch(identifier, subDefinition, depth)) {
+            debug('dfs exist in sub tree %s == %s subId = %s.', identifier, definition.id, subDefinition.id);
             return true;
           }
         }
       }
+      debug('dfs for %s == %s end.', identifier, definition.id);
     }
     return false;
   }

packages/midway-core/test/context/requestContainer.test.ts

@@ -1,6 +1,25 @@
 import { expect } from 'chai';
 import { MidwayContainer as Container, REQUEST_OBJ_CTX_KEY, MidwayRequestContainer as RequestContainer, ScopeEnum } from '../../src';
 import { Inject, Provide, Scope } from '@midwayjs/decorator';
+import { CircularOne,
+  CircularTwo,
+  CircularThree,
+  TestOne,
+  TestTwo,
+  TestThree,
+  TestOne1,
+  TestTwo1,
+  TestThree1,
+  GatewayManager,
+  GatewayService,
+  GroupService,
+  FunService,
+  AppService,
+  TenService,
+  ScaleManager,
+  AutoScaleService,
+  CCController
+} from '../fixtures/circular_dependency';
 
 class Tracer {
 
@@ -130,4 +149,53 @@ describe('/test/context/requestContainer.test.ts', () => {
     expect(tracer2[ REQUEST_OBJ_CTX_KEY ]).to.equal(ctx2);
   });
 
+  it('circular should be ok in requestContainer', async () => {
+    const appCtx = new Container();
+
+    appCtx.bind(TestOne);
+    appCtx.bind(TestTwo);
+    appCtx.bind(TestThree);
+    appCtx.bind(TestOne1);
+    appCtx.bind(TestTwo1);
+    appCtx.bind(TestThree1);
+    appCtx.bind(CircularOne);
+    appCtx.bind(CircularTwo);
+    appCtx.bind(CircularThree);
+
+    const ctx1 = { a: 1 };
+    const container = new RequestContainer(ctx1, appCtx);
+    const circularTwo: CircularTwo = await container.getAsync(CircularTwo);
+    expect(circularTwo.test2).eq('this is two');
+    expect((circularTwo.circularOne as CircularOne).test1).eq('this is one');
+    expect(((circularTwo.circularOne as CircularOne).circularTwo as CircularTwo).test2).eq('this is two');
+
+    const one = await container.getAsync<TestOne1>(TestOne1);
+    expect(one).not.null;
+    expect(one).not.undefined;
+    expect(one.name).eq('one');
+    expect((one.two as TestTwo1).name).eq('two');
+  });
+
+  it('circular depth should be ok in requestContainer', async () => {
+    const appCtx = new Container();
+    appCtx.bind(GatewayManager);
+    appCtx.bind(GatewayService);
+    appCtx.bind(GroupService);
+    appCtx.bind(FunService);
+    appCtx.bind(AppService);
+    appCtx.bind(TenService);
+    appCtx.bind(ScaleManager);
+    appCtx.bind(AutoScaleService);
+    appCtx.bind(CCController);
+
+    const ctx1 = { a: 1 };
+    const container = new RequestContainer(ctx1, appCtx);
+    const one = await container.getAsync<CCController>(CCController);
+    expect(one).not.null;
+    expect(one).not.undefined;
+    expect(one.ts).eq('controller');
+
+    expect(one.autoScaleService.ts).eq('ascale');
+    expect(one.autoScaleService.scaleManager.ts).eq('scale');
+  });
 });

packages/midway-core/test/fixtures/circular_dependency.ts

@@ -105,3 +105,88 @@ export class TestThree {
   @Inject('testOne')
   one: any;
 }
+
+@Provide()
+export class GatewayManager {
+  ts = 'gtmanager';
+  @Inject()
+  groupService: GroupService;
+  @Inject()
+  funService: FunService;
+  @Inject()
+  appService: AppService;
+}
+
+@Provide()
+export class GatewayService {
+  ts = 'gateway';
+  @Inject()
+  appService: AppService;
+  @Inject()
+  funService: FunService;
+  @Inject()
+  gatewayManager: GatewayManager;
+  @Inject()
+  groupService: GroupService;
+}
+@Provide()
+export class GroupService {
+  ts = 'group';
+  @Inject()
+  gatewayService: GatewayService;
+  @Inject()
+  tenService: TenService;
+  @Inject()
+  appService: AppService;
+}
+@Provide()
+export class FunService {
+
+}
+@Provide()
+export class AppService {
+
+}
+@Provide()
+export class TenService {
+
+}
+@Provide()
+export class ScaleManager {
+  ts = 'scale';
+  @Inject()
+  tenService: TenService;
+  @Inject()
+  appService: AppService;
+  @Inject()
+  funService: FunService;
+  @Inject()
+  gatewayManager: GatewayManager;
+  @Inject()
+  gatewayService: GatewayService;
+  @Inject()
+  groupService: GroupService;
+
+  @Inject()
+  autoScaleService: AutoScaleService;
+}
+
+@Provide()
+export class AutoScaleService {
+  ts = 'ascale';
+  @Inject()
+  gatewayManager: GatewayManager;
+  @Inject()
+  gatewayService: GatewayService;
+  @Inject()
+  groupService: GroupService;
+  @Inject()
+  scaleManager: ScaleManager;
+}
+
+@Provide()
+export class CCController {
+  ts = 'controller';
+  @Inject()
+  autoScaleService: AutoScaleService;
+}

packages/midway-web/test/enhance.test.ts

@@ -292,6 +292,13 @@ describe('/test/enhance.test.ts', () => {
         .get('/hello/stage')
         .expect(200);
     });
+
+    it('circular shoule be ok', async () => {
+      await app.httpRequest()
+        .get('/circular/test')
+        .expect('success')
+        .expect(200);
+    });
   });
 
   describe('load ts file and use third party module', () => {

packages/midway-web/test/fixtures/enhance/base-app-decorator/src/app/controller/circular.ts

@@ -0,0 +1,13 @@
+import { provide, controller, get, inject } from '../../../../../../../src';
+
+@provide()
+@controller('/circular')
+export class CircularController {
+  @inject()
+  planA: any;
+
+  @get('/test')
+  async test(ctx) {
+    ctx.body = 'success';
+  }
+}

packages/midway-web/test/fixtures/enhance/base-app-decorator/src/lib/circular.ts

@@ -0,0 +1,19 @@
+import { provide, inject } from '../../../../../../src';
+import { PlanA } from './planA';
+
+// @provide()
+// export class PlanA {
+//   @inject()
+//   planB: any;
+// }
+
+@provide()
+export class PlanB {
+  @inject()
+  planA: PlanA;
+  @inject()
+  helloService: any;
+
+  @inject()
+  AppModel: any;
+}

packages/midway-web/test/fixtures/enhance/base-app-decorator/src/lib/fun.ts

@@ -0,0 +1,15 @@
+import { providerWrapper, IApplicationContext } from '@midwayjs/core';
+
+export function Test(context: IApplicationContext, args?: any) {
+  // ignore
+  return AppModel;
+}
+
+providerWrapper([
+  {
+    id: 'AppModel',
+    provider: Test
+  }
+]);
+
+export class AppModel {}

packages/midway-web/test/fixtures/enhance/base-app-decorator/src/lib/planA.ts

@@ -0,0 +1,10 @@
+import { inject, provide } from '../../../../../../src';
+import { PlanB } from './circular';
+
+@provide()
+export class PlanA {
+  @inject()
+  helloService: any;
+  @inject()
+  planB: PlanB;
+}