From 94dc13088c3a3654ac3baa195bf82c04db59b271 Mon Sep 17 00:00:00 2001 From: fengmk2 Date: Sun, 5 Jun 2016 23:34:10 +0800 Subject: [PATCH] fix: should sync missing public scoped package on install (#946) * fix: should sync missing public scoped package on install closes #938 * refactor: sync all scoped packages when config.scopes is empty * test: test on node 6 --- .travis.yml | 2 +- LICENSE.txt | 2 +- Makefile | 2 +- controllers/registry/package/list.js | 10 +---- middleware/proxy_to_npm.js | 15 ++------ middleware/sync_by_install.js | 27 +++++++------- .../registry/module/scope_package.test.js | 21 +++-------- test/controllers/sync_module_worker.test.js | 9 +---- test/init.js | 10 ----- test/init_db.js | 12 +----- test/middleware/proxy_to_npm.test.js | 37 +++++++++---------- test/middleware/sync_by_install.test.js | 34 +++++++++++++++++ 12 files changed, 80 insertions(+), 101 deletions(-) create mode 100644 test/middleware/sync_by_install.test.js diff --git a/.travis.yml b/.travis.yml index f3b4f78..78fa0ec 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,7 +2,7 @@ sudo: false language: node_js node_js: - '4' - - '5' + - '6' addons: - postgresql: '9.3' script: 'make test-travis-all' diff --git a/LICENSE.txt b/LICENSE.txt index d0285c2..25444a7 100644 --- a/LICENSE.txt +++ b/LICENSE.txt @@ -1,6 +1,6 @@ This software is licensed under the MIT License. -Copyright(c) cnpmjs.org and other contributors. +Copyright(c) cnpm and other contributors. Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal diff --git a/Makefile b/Makefile index 85f21e5..6c0d8da 100644 --- a/Makefile +++ b/Makefile @@ -12,7 +12,7 @@ install-production production: @NODE_ENV=production $(MAKE) install jshint: install - @-node_modules/.bin/jshint ./ + @node_modules/.bin/jshint . init-database: @node test/init_db.js diff --git a/controllers/registry/package/list.js b/controllers/registry/package/list.js index b59de4b..c2eb450 100644 --- a/controllers/registry/package/list.js +++ b/controllers/registry/package/list.js @@ -1,11 +1,3 @@ -/** - * Copyright(c) cnpm and other contributors. - * MIT Licensed - * - * Authors: - * fengmk2 (http://fengmk2.com) - */ - 'use strict'; /** @@ -99,7 +91,7 @@ module.exports = function* list() { this.status = 404; this.body = { error: 'not_found', - reason: 'document not found' + reason: 'document not found', }; return; } diff --git a/middleware/proxy_to_npm.js b/middleware/proxy_to_npm.js index e29ba34..1a1fb7e 100644 --- a/middleware/proxy_to_npm.js +++ b/middleware/proxy_to_npm.js @@ -1,12 +1,3 @@ -/**! - * cnpmjs.org - middleware/proxy_to_npm.js - * - * Copyright(c) Alibaba Group Holding Limited. - * - * Authors: - * 苏千 (http://fengmk2.com) - */ - 'use strict'; /** @@ -33,11 +24,11 @@ module.exports = function (options) { } return function* proxyToNpm(next) { if (config.syncModel !== 'none') { - return yield* next; + return yield next; } // only proxy read requests if (this.method !== 'GET' && this.method !== 'HEAD') { - return yield* next; + return yield next; } var pathname = this.path; @@ -49,7 +40,7 @@ module.exports = function (options) { } } if (!match) { - return yield* next; + return yield next; } var url = redirectUrl + this.url; diff --git a/middleware/sync_by_install.js b/middleware/sync_by_install.js index 4b25797..2084f79 100644 --- a/middleware/sync_by_install.js +++ b/middleware/sync_by_install.js @@ -1,7 +1,5 @@ -/**! - * cnpmjs.org - middleware/sync_by_install.js - * - * Copyright(c) cnpmjs.org and other contributors. +/** + * Copyright(c) cnpm and other contributors. * MIT Licensed * * Authors: @@ -17,33 +15,36 @@ var config = require('../config'); /** - * this.allowSync - allow sync triggle by cnpm install + * {Boolean} this.allowSync - allow sync triggle by cnpm install */ module.exports = function* syncByInstall(next) { this.allowSync = false; if (!config.syncByInstall) { // only config.enablePrivate should enable sync on install - return yield* next; + return yield next; } - // request not by node, consider it request from web, dont sync + // request not by node, consider it request from web, don't sync var ua = this.get('user-agent'); if (!ua || ua.indexOf('node') < 0) { - return yield* next; + return yield next; } - // if request with `/xxx?write=true`, meaning the read request using for write, dont sync + // if request with `/xxx?write=true`, meaning the read request using for write, don't sync if (this.query.write) { - return yield* next; + return yield next; } var name = this.params.name || this.params[0]; - // scoped package dont sync + // private scoped package don't sync if (name && name[0] === '@') { - return yield* next; + var scope = name.split('/')[0]; + if (config.scopes.indexOf(scope) >= 0) { + return yield next; + } } this.allowSync = true; - yield* next; + yield next; }; diff --git a/test/controllers/registry/module/scope_package.test.js b/test/controllers/registry/module/scope_package.test.js index 6520b10..8876bd8 100644 --- a/test/controllers/registry/module/scope_package.test.js +++ b/test/controllers/registry/module/scope_package.test.js @@ -1,13 +1,3 @@ -/**! - * cnpmjs.org - test/controllers/registry/module/scope_package.test.js - * - * Copyright(c) fengmk2 and other contributors. - * MIT Licensed - * - * Authors: - * fengmk2 (http://fengmk2.github.com) - */ - 'use strict'; /** @@ -21,7 +11,7 @@ var config = require('../../../../config'); var app = require('../../../../servers/registry'); var utils = require('../../../utils'); -describe('controllers/registry/module/scope_package.test.js', function () { +describe('test/controllers/registry/module/scope_package.test.js', function () { var pkgname = '@cnpm/test-scope-package'; var pkgURL = '/@' + encodeURIComponent(pkgname.substring(1)); before(function (done) { @@ -52,16 +42,17 @@ describe('controllers/registry/module/scope_package.test.js', function () { afterEach(mm.restore); - it('should get 404 when do not support scope', function (done) { + it('should get 302 when do not support scope', function (done) { mm(config, 'scopes', []); request(app) .get('/@invalid/test') - .expect(404, done); + .expect('Location', 'https://registry.npmjs.com/@invalid/test') + .expect(302, done); }); - it('should get 400 when scope not match', function (done) { + it('should get 404 when scope is private', function (done) { request(app) - .get('/@invalid/test') + .get('/@cnpmtest/test') .expect(404, done); }); diff --git a/test/controllers/sync_module_worker.test.js b/test/controllers/sync_module_worker.test.js index 406958e..4c3ed2e 100644 --- a/test/controllers/sync_module_worker.test.js +++ b/test/controllers/sync_module_worker.test.js @@ -1,11 +1,3 @@ -/**! - * Copyright(c) cnpmjs.org and other contributors. - * MIT Licensed - * - * Authors: - * fengmk2 (http://fengmk2.com) - */ - 'use strict'; /** @@ -69,6 +61,7 @@ describe('test/controllers/sync_module_worker.test.js', function () { var worker = new SyncModuleWorker({ name: '@sindresorhus/df', username: 'fengmk2', + noDep: true, }); worker.start(); var end = thunkify.event(worker, 'end'); diff --git a/test/init.js b/test/init.js index ac9b2a3..6b2fe42 100644 --- a/test/init.js +++ b/test/init.js @@ -1,13 +1,3 @@ -/**! - * cnpmjs.org - test/init.js - * - * Copyright(c) fengmk2 and other contributors. - * MIT Licensed - * - * Authors: - * fengmk2 (http://fengmk2.github.com) - */ - 'use strict'; /** diff --git a/test/init_db.js b/test/init_db.js index 5cf1e04..1bdc19c 100644 --- a/test/init_db.js +++ b/test/init_db.js @@ -1,13 +1,3 @@ -/**! - * cnpmjs.org - test/init_db.js - * - * Copyright(c) fengmk2 and other contributors. - * MIT Licensed - * - * Authors: - * fengmk2 (http://fengmk2.github.com) - */ - 'use strict'; /** @@ -22,7 +12,7 @@ var config = require('../config'); // init db first var initscript = path.join(__dirname, '..', 'models', 'init_script.js'); -var cmd = ['node', '--harmony', initscript, 'true', +var cmd = ['node', initscript, 'true', config.database.dialect, config.database.port, config.database.username].join(' '); console.log('$ %s', cmd); var stdout = childProcess.execSync(cmd); diff --git a/test/middleware/proxy_to_npm.test.js b/test/middleware/proxy_to_npm.test.js index 8c245a5..7079d50 100644 --- a/test/middleware/proxy_to_npm.test.js +++ b/test/middleware/proxy_to_npm.test.js @@ -1,13 +1,3 @@ -/**! - * cnpmjs.org - test/middleware/proxy_to_npm.test.js - * - * Copyright(c) cnpmjs.org and other contributors. - * MIT Licensed - * - * Authors: - * fengmk2 (http://fengmk2.github.com) - */ - 'use strict'; /** @@ -19,44 +9,51 @@ var mm = require('mm'); var app = require('../../servers/registry'); var config = require('../../config'); -describe('middleware/proxy_to_npm.test.js', function () { - beforeEach(function () { +describe('test/middleware/proxy_to_npm.test.js', () => { + beforeEach(() => { mm(config, 'syncModel', 'none'); }); afterEach(mm.restore); - describe('package', function () { - it('should proxy to source registry when package not exists', function (done) { + describe('package', () => { + it('should proxy to source registry when package not exists', done => { request(app.listen()) .get('/ms') .expect('location', config.sourceNpmRegistry + '/ms') .expect(302, done); }); - it('should proxy to source registry when package is not local', function (done) { + it('should proxy to source registry when package is not local', done => { request(app.listen()) .get('/baidu?write=true') .expect('location', config.sourceNpmRegistry + '/baidu?write=true') .expect(302, done); }); - it('should not proxy to source registry when package is scoped', function (done) { + it('should not proxy to source registry when package is private scoped', done => { request(app.listen()) - .get('/@scoped/test-package-name') + .get('/@cnpmtest/test-package-name') .expect(404, done); }); + + it('should proxy to source registry when package is public scoped', done => { + request(app.listen()) + .get('/@jkroso/type') + .expect('Location', 'https://registry.npmjs.com/@jkroso/type') + .expect(302, done); + }); }); - describe('dist-tags', function () { - it('should proxy to source registry when package not exists', function (done) { + describe('dist-tags', () => { + it('should proxy to source registry when package not exists', done => { request(app.listen()) .get('/-/package/ms/dist-tags') .expect('location', config.sourceNpmRegistry + '/-/package/ms/dist-tags') .expect(302, done); }); - it('should dont proxy scoped package', function (done) { + it('should dont proxy scoped package', done => { request(app.listen()) .get('/-/package/@scoped/ms/dist-tags') .expect(404, done); diff --git a/test/middleware/sync_by_install.test.js b/test/middleware/sync_by_install.test.js new file mode 100644 index 0000000..53ee10a --- /dev/null +++ b/test/middleware/sync_by_install.test.js @@ -0,0 +1,34 @@ +'use strict'; + +/** + * Module dependencies. + */ + +var request = require('supertest'); +var app = require('../../servers/registry'); +var mm = require('mm'); +var config = require('../../config'); +var userService = require('../../services/user'); + +describe('test/middleware/sync_by_install.test.js', () => { + afterEach(mm.restore); + + it('should ignore sync on install private scoped package', done => { + request(app.listen()) + .get('/@cnpmtest/foo') + .set('User-Agent', 'node/v4.4.4') + .expect({ + error: 'not_found', + reason: 'document not found', + }) + .expect(404, done); + }); + + it('should sync and redirect to npmjs.com on install public scoped package', done => { + request(app.listen()) + .get('/@jkroso/type') + .set('User-Agent', 'node/v4.4.4') + .expect('Location', 'https://registry.npmjs.com/@jkroso/type') + .expect(302, done); + }); +});