From e9f93cf83d46fd84c8c6b10ec2e7e3eb2bf24f9d Mon Sep 17 00:00:00 2001 From: Haoliang Gao Date: Wed, 26 Jul 2017 17:36:49 +0800 Subject: [PATCH] refactor: export app.HttpClient that can be overwritten (#1234) --- lib/core/dnscache_httpclient.js | 10 +++---- lib/core/httpclient.js | 27 ++++++++++------- lib/egg.js | 10 +++++-- .../fixtures/apps/httpclient-overwrite/app.js | 21 +++++++++++++ .../config/config.default.js | 7 +++++ .../apps/httpclient-overwrite/package.json | 3 ++ test/lib/core/httpclient.test.js | 30 +++++++++++++++++-- 7 files changed, 88 insertions(+), 20 deletions(-) create mode 100644 test/fixtures/apps/httpclient-overwrite/app.js create mode 100644 test/fixtures/apps/httpclient-overwrite/config/config.default.js create mode 100644 test/fixtures/apps/httpclient-overwrite/package.json diff --git a/lib/core/dnscache_httpclient.js b/lib/core/dnscache_httpclient.js index 14a9bbfb2d..d7c821e4a5 100644 --- a/lib/core/dnscache_httpclient.js +++ b/lib/core/dnscache_httpclient.js @@ -3,16 +3,14 @@ const dns = require('dns'); const LRU = require('ylru'); const urlparse = require('url').parse; -const urllib = require('urllib'); +const HttpClient = require('./httpclient'); const utility = require('utility'); const IP_REGEX = /^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/; -class DNSCacheHttpClient extends urllib.HttpClient { - constructor(options) { - super(options); - - this.app = options.app; +class DNSCacheHttpClient extends HttpClient { + constructor(app) { + super(app); this.dnsCacheLookupInterval = this.app.config.httpclient.dnsCacheLookupInterval; this.dnsCache = new LRU(this.app.config.httpclient.dnsCacheMaxLength); } diff --git a/lib/core/httpclient.js b/lib/core/httpclient.js index 619ad2ed3e..6896167c6a 100644 --- a/lib/core/httpclient.js +++ b/lib/core/httpclient.js @@ -5,10 +5,22 @@ const HttpsAgent = require('agentkeepalive').HttpsAgent; const urllib = require('urllib'); const ms = require('humanize-ms'); -module.exports = app => { - const HttpClient = app.config.httpclient.enableDNSCache ? - require('./dnscache_httpclient') : urllib.HttpClient; +class HttpClient extends urllib.HttpClient { + constructor(app) { + const config = app.config.httpclient; + normalizeConfig(app); + super({ + app, + defaultArgs: config.request, + agent: new Agent(config.httpAgent), + httpsAgent: new HttpsAgent(config.httpsAgent), + }); + this.app = app; + } +} + +function normalizeConfig(app) { const config = app.config.httpclient; // compatibility @@ -49,11 +61,6 @@ module.exports = app => { if (typeof config.request.timeout === 'string') { config.request.timeout = ms(config.request.timeout); } +} - return new HttpClient({ - app, - defaultArgs: config.request, - agent: new Agent(config.httpAgent), - httpsAgent: new HttpsAgent(config.httpsAgent), - }); -}; +module.exports = HttpClient; diff --git a/lib/egg.js b/lib/egg.js index 94b19bb773..1458e3d52b 100644 --- a/lib/egg.js +++ b/lib/egg.js @@ -9,7 +9,8 @@ const ContextLogger = require('egg-logger').EggContextLogger; const ContextCookies = require('egg-cookies'); const ContextHttpClient = require('./core/context_httpclient'); const Messenger = require('./core/messenger'); -const createHttpClient = require('./core/httpclient'); +const DNSCacheHttpClient = require('./core/dnscache_httpclient'); +const HttpClient = require('./core/httpclient'); const createLoggers = require('./core/logger'); const Singleton = require('./core/singleton'); const utils = require('./core/utils'); @@ -42,6 +43,7 @@ class EggApplication extends EggCore { this.ContextCookies = ContextCookies; this.ContextLogger = ContextLogger; this.ContextHttpClient = ContextHttpClient; + this.HttpClient = HttpClient; this.loader.loadConfig(); @@ -240,7 +242,11 @@ class EggApplication extends EggCore { */ get httpclient() { if (!this[HTTPCLIENT]) { - this[HTTPCLIENT] = createHttpClient(this); + if (this.config.httpclient.enableDNSCache) { + this[HTTPCLIENT] = new DNSCacheHttpClient(this); + } else { + this[HTTPCLIENT] = new this.HttpClient(this); + } } return this[HTTPCLIENT]; } diff --git a/test/fixtures/apps/httpclient-overwrite/app.js b/test/fixtures/apps/httpclient-overwrite/app.js new file mode 100644 index 0000000000..4a23a397f0 --- /dev/null +++ b/test/fixtures/apps/httpclient-overwrite/app.js @@ -0,0 +1,21 @@ +'use strict'; + +const assert = require('assert'); + +module.exports = app => { + class CustomHttpClient extends app.HttpClient { + request(url, opt) { + return new Promise(resolve => { + assert(/^http/.test(url), 'url should start with http, but got ' + url); + resolve(); + }).then(() => { + return super.request(url, opt); + }); + } + + curl(url, opt) { + return this.request(url, opt); + } + } + app.HttpClient = CustomHttpClient; +}; diff --git a/test/fixtures/apps/httpclient-overwrite/config/config.default.js b/test/fixtures/apps/httpclient-overwrite/config/config.default.js new file mode 100644 index 0000000000..46f1ab10d2 --- /dev/null +++ b/test/fixtures/apps/httpclient-overwrite/config/config.default.js @@ -0,0 +1,7 @@ +'use strict'; + +exports.httpclient = { + request: { + timeout: 100, + }, +}; diff --git a/test/fixtures/apps/httpclient-overwrite/package.json b/test/fixtures/apps/httpclient-overwrite/package.json new file mode 100644 index 0000000000..56b5d28da8 --- /dev/null +++ b/test/fixtures/apps/httpclient-overwrite/package.json @@ -0,0 +1,3 @@ +{ + "name": "httpclient-overwrite" +} diff --git a/test/lib/core/httpclient.test.js b/test/lib/core/httpclient.test.js index 70ec1ee4de..7b4a2bd6f0 100644 --- a/test/lib/core/httpclient.test.js +++ b/test/lib/core/httpclient.test.js @@ -2,7 +2,7 @@ const assert = require('assert'); const mm = require('egg-mock'); -const createHttpclient = require('../../../lib/core/httpclient'); +const Httpclient = require('../../../lib/core/httpclient'); const utils = require('../../utils'); describe('test/lib/core/httpclient.test.js', () => { @@ -10,7 +10,7 @@ describe('test/lib/core/httpclient.test.js', () => { let url; before(() => { - client = createHttpclient({ + client = new Httpclient({ config: { httpclient: { request: {}, @@ -151,4 +151,30 @@ describe('test/lib/core/httpclient.test.js', () => { }); }); }); + + describe('overwrite httpclient', () => { + let app; + before(() => { + app = utils.app('apps/httpclient-overwrite'); + return app.ready(); + }); + after(() => app.close()); + + it('should set request default global timeout to 100ms', () => { + return app.httpclient.curl(`${url}/timeout`) + .catch(err => { + assert(err); + assert(err.name === 'ResponseTimeoutError'); + assert(err.message.includes('Response timeout for 100ms')); + }); + }); + + it('should assert url', () => { + return app.httpclient.curl('unknown url') + .catch(err => { + assert(err); + assert(err.message.includes('url should start with http, but got unknown url')); + }); + }); + }); });