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,7 +2,7 @@ sudo: false
 language: node_js
 node_js:
   - '4'
-  - '5'
+  - '6'
 addons:
   - postgresql: '9.3'
 script: 'make test-travis-all'

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

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

controllers/registry/package/list.js

@@ -1,11 +1,3 @@
-/**
- * Copyright(c) cnpm and other contributors.
- * MIT Licensed
- *
- * Authors:
- *   fengmk2 <fengmk2@gmail.com> (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;
     }

middleware/proxy_to_npm.js

@@ -1,12 +1,3 @@
-/**!
- * cnpmjs.org - middleware/proxy_to_npm.js
- *
- * Copyright(c) Alibaba Group Holding Limited.
- *
- * Authors:
- *   苏千 <suqian.yf@alipay.com> (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;

middleware/sync_by_install.js

@@ -1,7 +1,5 @@
-/**!
+/**
- * cnpmjs.org - middleware/sync_by_install.js
+ * Copyright(c) cnpm and other contributors.
- *
- * Copyright(c) cnpmjs.org 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;
 };

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 <fengmk2@gmail.com> (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);
   });
 

test/controllers/sync_module_worker.test.js

@@ -1,11 +1,3 @@
-/**!
- * Copyright(c) cnpmjs.org and other contributors.
- * MIT Licensed
- *
- * Authors:
- *  fengmk2 <fengmk2@gmail.com> (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');

test/init.js

@@ -1,13 +1,3 @@
-/**!
- * cnpmjs.org - test/init.js
- *
- * Copyright(c) fengmk2 and other contributors.
- * MIT Licensed
- *
- * Authors:
- *   fengmk2 <fengmk2@gmail.com> (http://fengmk2.github.com)
- */
-
 'use strict';
 
 /**

test/init_db.js

@@ -1,13 +1,3 @@
-/**!
- * cnpmjs.org - test/init_db.js
- *
- * Copyright(c) fengmk2 and other contributors.
- * MIT Licensed
- *
- * Authors:
- *   fengmk2 <fengmk2@gmail.com> (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);

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 <fengmk2@gmail.com> (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 () {
+describe('test/middleware/proxy_to_npm.test.js', () => {
-  beforeEach(function () {
+  beforeEach(() => {
     mm(config, 'syncModel', 'none');
   });
 
   afterEach(mm.restore);
 
-  describe('package', function () {
+  describe('package', () => {
-    it('should proxy to source registry when package not exists', function (done) {
+    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 () {
+  describe('dist-tags', () => {
-    it('should proxy to source registry when package not exists', function (done) {
+    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);

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);
+  });
+});