fix: service factory client & clients merge (#2248)

packages/axios/src/configuration.ts

@@ -10,6 +10,19 @@ import { HttpServiceFactory } from './serviceManager';
       },
     },
   ],
+  importConfigFilter: config => {
+    if (config['axios']) {
+      if (!config['axios']['clients'] || !config['axios']['client']) {
+        config['axios'] = {
+          default: {},
+          clients: {
+            default: config['axios'],
+          },
+        };
+      }
+    }
+    return config;
+  },
 })
 export class AxiosConfiguration {
   async onReady(container) {

packages/axios/src/serviceManager.ts

@@ -7,7 +7,7 @@ import {
   Scope,
   ScopeEnum,
 } from '@midwayjs/decorator';
-import { ServiceFactory } from '@midwayjs/core';
+import { MidwayCommonError, ServiceFactory } from '@midwayjs/core';
 import { AxiosHttpService } from './interface';
 
 @Provide()
@@ -57,6 +57,9 @@ export class HttpService implements AxiosHttpService {
   @Init()
   protected async init() {
     this.instance = this.serviceFactory.get('default');
+    if (!this.instance) {
+      throw new MidwayCommonError('axios default instance not found.');
+    }
   }
 
   getUri(config?: AxiosRequestConfig): string {

packages/core/src/common/serviceFactory.ts

@@ -1,4 +1,3 @@
-import * as assert from 'assert';
 import { extend } from '../util/extend';
 
 /**
@@ -10,18 +9,16 @@ export abstract class ServiceFactory<T> {
 
   protected async initClients(options: any = {}): Promise<void> {
     this.options = options;
-    assert(
-      !(options.client && options.clients),
-      `midway:${this.getName()} can not set options.client and options.clients both`
-    );
 
-    // alias app[name] as client, but still support createInstance method
+    // merge options.client to options.clients['default']
     if (options.client) {
-      await this.createInstance(options.client, 'default');
-      return;
+      options.clients = options.clients || {};
+      options.clients['default'] = options.clients['default'] || {};
+      extend(true, options.clients['default'], options.client);
+      delete options.client;
     }
 
-    // multi client, use app[name].getInstance(id)
+    // multi client
     if (options.clients) {
       for (const id of Object.keys(options.clients)) {
         await this.createInstance(options.clients[id], id);

packages/core/src/service/configService.ts

@@ -33,7 +33,7 @@ export class MidwayConfigService implements IConfigService {
     unittest: 'test',
   };
   private configMergeOrder: Array<ConfigMergeInfo> = [];
-  protected configuration;
+  protected configuration = {};
   protected isReady = false;
   protected externalObject: Record<string, unknown>[] = [];
   protected appInfo: MidwayAppInfo;
@@ -187,6 +187,11 @@ export class MidwayConfigService implements IConfigService {
       for (let externalObject of this.externalObject) {
         if (externalObject) {
           externalObject = this.runWithFilter(externalObject);
+          if (!externalObject) {
+            debug('[config]: Filter config and got undefined will be drop it');
+            continue;
+          }
+
           debug('[config]: Loaded external object %j', externalObject);
           extend(true, target, externalObject);
           this.configMergeOrder.push({
@@ -236,7 +241,7 @@ export class MidwayConfigService implements IConfigService {
   }
 
   public clearAllConfig() {
-    this.configuration.clear();
+    this.configuration = {};
   }
 
   public clearConfigMergeOrder() {

packages/core/test/common/serviceFactory.test.ts

@@ -59,18 +59,4 @@ describe('test/common/serviceFactory.test.ts', () => {
     });
     expect(instance.get('default')).toBeDefined();
   });
-
-  it('should config must with client or clients', async () => {
-    const instance = new TestServiceFactory();
-    let err;
-    try {
-      await instance.initClients({
-        client: {},
-        clients: {}
-      });
-    } catch (e) {
-      err = e;
-    }
-    expect(err).toBeDefined();
-  });
 });

packages/core/test/error.test.ts → packages/core/test/error/error.test.ts

@@ -1,6 +1,6 @@
-import { MidwayError, registerErrorCode } from '../src';
+import { MidwayError, MidwayHttpError, registerErrorCode } from '../../src';
 
-describe('/test/error.test.ts', function () {
+describe('/test/error/error.test.ts', function () {
   it('should test base error', function () {
     expect(() => {
       throw new MidwayError('custom error');
@@ -46,4 +46,29 @@ describe('/test/error.test.ts', function () {
       } as const);
     }).toThrowError(MidwayError);
   });
+
+  it('should test http error', function () {
+    expect(() => {
+      throw new MidwayHttpError('custom error', 302);
+    }).toThrowError(MidwayHttpError);
+  });
+
+  it('should test http error with err obj', function () {
+    const err = new Error('custom message');
+    expect(() => {
+      throw new MidwayHttpError(err, 302);
+    }).toThrowError(/custom message/);
+  });
+
+  it('should test with options', function () {
+    const cause = new Error('custom');
+    const err = new MidwayError('custom error', {
+      status: 10,
+      cause,
+    });
+
+    expect(() => {
+      throw err;
+    }).toThrowError(/custom error/);
+  });
 });

packages/core/test/error/http.test.ts

@@ -0,0 +1,69 @@
+import { httpError } from '../../src';
+
+describe('test/error/http.test.ts', function () {
+  it('should test throw http error', function () {
+    expect(() => {
+      throw new httpError.BadRequestError();
+    }).toThrowError(httpError.BadRequestError);
+
+    expect(() => {
+      throw new httpError.UnauthorizedError();
+    }).toThrowError(httpError.UnauthorizedError);
+
+    expect(() => {
+      throw new httpError.NotFoundError();
+    }).toThrowError(httpError.NotFoundError);
+
+    expect(() => {
+      throw new httpError.ForbiddenError();
+    }).toThrowError(httpError.ForbiddenError);
+
+    expect(() => {
+      throw new httpError.NotAcceptableError();
+    }).toThrowError(httpError.NotAcceptableError);
+
+    expect(() => {
+      throw new httpError.RequestTimeoutError();
+    }).toThrowError(httpError.RequestTimeoutError);
+
+    expect(() => {
+      throw new httpError.ConflictError();
+    }).toThrowError(httpError.ConflictError);
+
+    expect(() => {
+      throw new httpError.GoneError();
+    }).toThrowError(httpError.GoneError);
+
+    expect(() => {
+      throw new httpError.PayloadTooLargeError();
+    }).toThrowError(httpError.PayloadTooLargeError);
+
+    expect(() => {
+      throw new httpError.UnsupportedMediaTypeError();
+    }).toThrowError(httpError.UnsupportedMediaTypeError);
+
+    expect(() => {
+      throw new httpError.UnprocessableEntityError();
+    }).toThrowError(httpError.UnprocessableEntityError);
+
+    expect(() => {
+      throw new httpError.InternalServerErrorError();
+    }).toThrowError(httpError.InternalServerErrorError);
+
+    expect(() => {
+      throw new httpError.NotImplementedError();
+    }).toThrowError(httpError.NotImplementedError);
+
+    expect(() => {
+      throw new httpError.BadGatewayError();
+    }).toThrowError(httpError.BadGatewayError);
+
+    expect(() => {
+      throw new httpError.ServiceUnavailableError();
+    }).toThrowError(httpError.ServiceUnavailableError);
+
+    expect(() => {
+      throw new httpError.GatewayTimeoutError();
+    }).toThrowError(httpError.GatewayTimeoutError);
+  })
+});

packages/core/test/service/configService.test.ts

@@ -257,10 +257,40 @@ describe('/test/service/configService.test.ts', () => {
     await cfg.load();
 
     expect(cfg.getConfiguration().bb).toEqual('111');
+  });
 
-    cfg.addObject({bb: 333, cc: 222});
+  it('should test config filter and test return undefined', async () => {
+    const cfg = await createConfigService();
 
-    expect(cfg.getConfiguration().bb).toEqual('111');
-    expect(cfg.getConfiguration().cc).toEqual(222);
+    cfg.addFilter((config) => {
+      if(config['key']) {
+        return undefined;
+      }
+      return config;
+    });
+
+    cfg.add([
+      {
+        default: {
+          key: 111,
+        }
+      }
+    ]);
+
+    await cfg.load();
+
+    cfg.addObject({bb: 222});
+
+    expect(cfg.getConfiguration().bb).toEqual(222);
+    expect(cfg.getConfiguration().key).toEqual(undefined);
+  });
+
+  it('should clear all config', async () => {
+    const cfg = new MidwayConfigService();
+    cfg['isReady'] = true;
+    cfg.addObject({bb: 222});
+    expect(Object.keys(cfg.getConfiguration()).length).toEqual(1);
+    cfg.clearAllConfig();
+    expect(Object.keys(cfg.getConfiguration()).length).toEqual(0);
   });
 });

packages/cos/src/manager.ts

@@ -7,7 +7,11 @@ import {
   Scope,
   ScopeEnum,
 } from '@midwayjs/decorator';
-import { ServiceFactory, delegateTargetPrototypeMethod } from '@midwayjs/core';
+import {
+  ServiceFactory,
+  delegateTargetPrototypeMethod,
+  MidwayCommonError,
+} from '@midwayjs/core';
 import * as assert from 'assert';
 import * as COS from 'cos-nodejs-sdk-v5';
 
@@ -46,12 +50,14 @@ export class COSService implements COS {
   @Inject()
   private serviceFactory: COSServiceFactory;
 
-  // @ts-expect-error used
   private instance: COS;
 
   @Init()
   async init() {
     this.instance = this.serviceFactory.get('default');
+    if (!this.instance) {
+      throw new MidwayCommonError('cos default instance not found.');
+    }
   }
 }
 

packages/cos/test/index.test.ts

@@ -21,4 +21,12 @@ describe('/test/index.test.ts', () => {
     expect(cosService).toBeDefined();
     await close(app);
   });
+
+  it('should throw error when instance not found', async () => {
+    await expect(async () => {
+      const service = new COSService();
+      (service as any).serviceFactory = new Map();
+      await service.init();
+    }).rejects.toThrowError(/instance not found/);
+  });
 });

packages/mongoose/src/manager.ts

@@ -2,6 +2,7 @@ import {
   DataSourceManager,
   delegateTargetMethod,
   delegateTargetProperties,
+  MidwayCommonError,
 } from '@midwayjs/core';
 import {
   Config,
@@ -128,12 +129,14 @@ export class MongooseConnectionService implements mongoose.Connection {
   @Inject()
   private mongooseDataSourceManager: MongooseDataSourceManager;
 
-  // @ts-expect-error used
   private instance: mongoose.Connection;
 
   @Init()
   async init() {
     this.instance = this.mongooseDataSourceManager.getDataSource('default');
+    if (!this.instance) {
+      throw new MidwayCommonError('mongoose default instance not found.');
+    }
   }
 }
 

packages/mongoose/test/index.test.ts

@@ -49,4 +49,14 @@ describe('/test/index.test.ts', () => {
     console.log(doc.email);
     await close(app);
   });
+
+  it('should throw error when instance not found', async () => {
+    await expect(async () => {
+      const service = new MongooseConnectionService();
+      (service as any).mongooseDataSourceManager = {
+        getDataSource() {}
+      }
+      await service.init();
+    }).rejects.toThrowError(/instance not found/);
+  });
 });

packages/oss/src/manager.ts

@@ -8,7 +8,11 @@ import {
 } from '@midwayjs/decorator';
 import * as OSS from 'ali-oss';
 import * as assert from 'assert';
-import { ServiceFactory, delegateTargetPrototypeMethod } from '@midwayjs/core';
+import {
+  ServiceFactory,
+  delegateTargetPrototypeMethod,
+  MidwayCommonError,
+} from '@midwayjs/core';
 import type {
   OSSServiceFactoryReturnType,
   MWOSSClusterOptions,
@@ -72,12 +76,14 @@ export class OSSService implements OSS {
   @Inject()
   private serviceFactory: OSSServiceFactory<OSS>;
 
-  // @ts-expect-error used
   private instance: OSS;
 
   @Init()
   async init() {
     this.instance = this.serviceFactory.get('default');
+    if (!this.instance) {
+      throw new MidwayCommonError('oss default instance not found.');
+    }
   }
 }
 
@@ -99,6 +105,9 @@ export class OSSSTSService implements OSS.STS {
   @Init()
   async init() {
     this.instance = this.serviceFactory.get('default');
+    if (!this.instance) {
+      throw new MidwayCommonError('oss sts default instance not found.');
+    }
   }
 
   async assumeRole(