add test, fix bug. fixed #18

cnpm · Dec 6, 2013 · 89a2688 · 89a2688
1 parent a0c7998
commit 89a2688
Show file tree

Hide file tree

Showing 7 changed files with 200 additions and 16 deletions.
diff --git a/common/qnfs.js b/common/qnfs.js
@@ -35,3 +35,7 @@ exports.upload = function (filepath, options, callback) {
     client.uploadFile(filepath, {key: options.key, size: options.size}, callback);
   });
 };
+
+exports.remove = function (key, callback) {
+  client.delete(key, callback);
+};
diff --git a/controllers/registry/module.js b/controllers/registry/module.js
@@ -21,9 +21,15 @@ var fs = require('fs');
 var crypto = require('crypto');
 var utility = require('utility');
 var eventproxy = require('eventproxy');
+var Bagpipe = require('bagpipe');
 var config = require('../../config');
 var Module = require('../../proxy/module');
 var nfs = require('../../common/nfs');
+var Url = require('url');
+
+function getCDNKey(name, filename) {
+  return '/' + name + '/-/' + filename;
+}
 
 exports.show = function (req, res, next) {
   var name = req.params.name;
@@ -149,7 +155,7 @@ exports.upload = function (req, res, next) {
         });
       }
       shasum = shasum.digest('hex');
-      var key = '/' + name + '/-/' + filename;
+      var key = getCDNKey(name, filename);
       nfs.upload(filepath, {key: key, size: length}, function (err, result) {
         // remove tmp file whatever
         fs.unlink(filepath, utility.noop);
@@ -313,19 +319,20 @@ exports.update = function (req, res, next) {
   debug('update module %s, with info %j', req.body);
   var name = req.params.name;
   var username = req.session.name;
-  var versions = req.body.versions;
+  var versions = req.body.versions || {};
   var ep = eventproxy.create();
   ep.fail(next);
 
   Module.listByName(name, ep.doneLater('list'));
   ep.once('list', function (mods) {
-    if (!mods) {
+    if (!mods || !mods.length) {
       return next();
     }
     //TODO replace this maintainer check
     var match = mods[0].package.maintainers.filter(function (item) {
       return item.name === username;
     });
+
     if (!match.length || mods[0].name !== name) {
       return res.json(403, {
         error: 'no_perms',
@@ -340,10 +347,11 @@ exports.update = function (req, res, next) {
         removeVersions.push(v);
       }
     }
-
+    if (!removeVersions.length) {
+      return res.json(201, {ok: true});
+    }
     Module.removeByNameAndVersions(name, removeVersions, ep.done(function () {
-      res.statusCode = 201;
-      res.end();
+      res.json(201, {ok: true});
     }));
   });
 };
@@ -353,6 +361,7 @@ exports.removeTar = function (req, res, next) {
   debug('remove tarball with filename: %s, id: %s', req.params.filename, req.params.rev);
   var id = Number(req.params.rev);
   var filename = req.params.filename;
+  var name = req.params.name;
   var username = req.session.name;
   var ep = eventproxy.create();
   ep.fail(next);
@@ -366,17 +375,16 @@ exports.removeTar = function (req, res, next) {
     var match = mod.package.maintainers.filter(function (item) {
       return item.name === username;
     });
-    if (!match.length || filename.indexOf(mod.name + '-') !== 0) {
+    if (!match.length || mod.name !== name) {
       return res.json(403, {
         error: 'no_perms',
         reason: 'Current user can not delete this tarball'
       });
     }
-    //TODO change local file to remote CDN file
-    var filePath = path.join(config.uploadDir, filename);
-    fs.unlink(filePath, ep.done(function () {
-      res.statusCode = 201;
-      res.send();
+
+    var key = getCDNKey(mod.name, filename);
+    nfs.remove(key, ep.done(function () {
+      res.json(200, {ok: true});
     }));
   });
 };
@@ -386,6 +394,41 @@ exports.removeAll = function (req, res, next) {
   var id = Number(req.params.rev);
   var name = req.params.name;
   var username = req.session.name;
+
   var ep = eventproxy.create();
   ep.fail(next);
+
+  Module.listByName(name, ep.doneLater('list'));
+  ep.once('list', function (mods) {
+    //TODO replace this maintainer check
+    var mod = mods[0];
+    var match = mod.package.maintainers.filter(function (item) {
+      return item.name === username;
+    });
+    if (!match.length || mod.name !== name) {
+      return res.json(403, {
+        error: 'no_perms',
+        reason: 'Current user can not delete this tarball'
+      });
+    }    
+    Module.removeByName(name, ep.done('remove'));
+  });
+
+  ep.all('list', 'remove', function (mods) {
+    var keys = [];
+    for (var i = 0; i < mods.length; i++) {
+      var key = Url.parse(mods[i].dist_tarball).path;
+      key && keys.push(key);
+    }
+    var queue = new Bagpipe(5);
+    keys.forEach(function (key) {
+      queue.push(nfs.remove, key, function () {
+        //ignore err here
+        ep.emit('removeTar');
+      });
+    });
+    ep.after('removeTar', keys.length, function () {
+      res.json(200, {});
+    });
+  });
 };
diff --git a/docs/install.md b/docs/install.md
@@ -1,3 +1,5 @@
+# npm install flow
+
 ```
 npm install pkg
 npm install pkg@<version range>

diff --git a/docs/unpublish.md b/docs/unpublish.md
@@ -1,3 +1,14 @@
+# npm unpublish flow  
+
+```
+npm unpublish pkg@version
+npm unpublish pkg -f
+```
+
+`npm unpublish pkg@version` will try to PUT /:name/-rev/:rev to update the module info. then try to DELETE /:name/-/:filename/-rev/:rev to delete the tgz file.  If this version is the only version of this package, this command turns into `npm unpublish pkg`
+
+`npm unpulbish pkg` will try to DELETE /:name/-rev, so clear everything of this package.  
+
 
 ```
 $ npm unpublish cnpm_test_module@0.0.2 --verbose

diff --git a/package.json b/package.json
@@ -34,6 +34,7 @@
     "mysql": "2.0.0-rc1",
     "response-patch": "0.1.1",
     "response-cookie": "0.0.2",
+    "bagpipe": "0.3.5",
     "eventproxy": "0.2.6"
   },
   "devDependencies": {

diff --git a/routes/registry.js b/routes/registry.js
@@ -34,15 +34,15 @@ function routes(app) {
   // put tarball
   // https://registry.npmjs.org/cnpmjs.org/-/cnpmjs.org-0.0.0.tgz/-rev/1-c85bc65e8d2470cc4d82b8f40da65b8e
   app.put('/:name/-/:filename/-rev/:rev', login, mod.upload);
-
+  //delete tarball
+  app.delete('/:name/-/:filename/-rev/:rev', login, mod.removeTar);
+
   // put package.json to module
   app.put('/:name/:version/-tag/latest', login, mod.updateLatest);
 
   //update module, unpublish will PUT this
   app.put('/:name/-rev/:rev', login, mod.update);
   app.delete('/:name/-rev/:rev', login, mod.removeAll);
-  //delete tarball
-  app.delete('/dist/:filename/-rev/:rev', login, mod.removeTar);
 
   // try to create a new user
   // https://registry.npmjs.org/-/user/org.couchdb.user:fengmk2

diff --git a/test/controllers/registry/module.test.js b/test/controllers/registry/module.test.js
@@ -21,6 +21,7 @@ var should = require('should');
 var request = require('supertest');
 var app = require('../../../servers/registry');
 var Module = require('../../../proxy/module');
+var mm = require('mm');
 
 var fixtures = path.join(path.dirname(path.dirname(__dirname)), 'fixtures');
 
@@ -31,7 +32,7 @@ describe('controllers/registry/module.test.js', function () {
   after(function (done) {
     app.close(done);
   });
-
+  afterEach(mm.restore);
   describe('GET /:name', function () {
     it('should return module info', function (done) {
       request(app)
@@ -269,4 +270,126 @@ describe('controllers/registry/module.test.js', function () {
     });
 
   });
+
+  describe('PUT /:name/-rev/:rev', function () { 
+    var baseauth = 'Basic ' + new Buffer('cnpmjstest10:cnpmjstest10').toString('base64');
+    var baseauthOther = 'Basic ' + new Buffer('cnpmjstest101:cnpmjstest101').toString('base64');
+    var lastRev;
+    before(function (done) {
+      request(app)
+      .get('/testputmodule')
+      .end(function (err, res) {
+        lastRev = res.body._rev;
+        done(err);
+      });
+    });
+
+    it('should update 401 when no auth', function (done) {
+      request(app)
+      .put('/testputmodule/-rev/123')
+      .expect(401, done);
+    });
+
+    it('should update 403 when auth error', function (done) {
+      request(app)
+      .put('/testputmodule/-rev/123')
+      .set('authorization', baseauthOther)
+      .expect(403, done);
+    });
+
+    it('should remove nothing removed ok', function (done) {
+      request(app)
+      .put('/testputmodule/-rev/' + lastRev)
+      .set('authorization', baseauth)
+      .send({
+        versions: {
+          '0.1.9': {}
+        }
+      })
+      .expect(201, done);
+    });
+
+    it('should remove version ok', function (done) {
+      //do not really remove it here
+      mm.empty(Module, 'removeByNameAndVersions');
+      request(app)
+      .put('/testputmodule/-rev/' + lastRev)
+      .set('authorization', baseauth)
+      .send({
+        versions: {
+        }
+      })
+      .expect(201, done);
+    });
+  });
+
+  describe('DELETE /:name/-/:filename/-rev/:rev', function () {
+    var baseauth = 'Basic ' + new Buffer('cnpmjstest10:cnpmjstest10').toString('base64');
+    var baseauthOther = 'Basic ' + new Buffer('cnpmjstest101:cnpmjstest101').toString('base64');
+    var lastRev;
+    before(function (done) {
+      request(app)
+      .get('/testputmodule')
+      .end(function (err, res) {
+        lastRev = res.body._rev;
+        done(err);
+      });
+    });
+
+    it('should delete 401 when no auth', function (done) {
+      request(app)
+      .del('/testputmodule/-/testputmodule-0.1.9.tgz/-rev/' + lastRev)
+      .expect(401, done);
+    });
+
+    it('should delete 403 when auth error', function (done) {
+      request(app)
+      .del('/testputmodule/-/testputmodule-0.1.9.tgz/-rev/' + lastRev)
+      .set('authorization', baseauthOther)
+      .expect(403, done);
+    });
+
+    it('should delete file ok', function (done) {
+      request(app)
+      .del('/testputmodule/-/testputmodule-0.1.9.tgz/-rev/' + lastRev)
+      .set('authorization', baseauth)
+      .expect(200, done);
+    });
+  });
+
+  describe('DELETE /:name/-rev/:rev', function (done) {
+    var baseauth = 'Basic ' + new Buffer('cnpmjstest10:cnpmjstest10').toString('base64');
+    var baseauthOther = 'Basic ' + new Buffer('cnpmjstest101:cnpmjstest101').toString('base64');
+    var lastRev;
+    before(function (done) {
+      request(app)
+      .get('/testputmodule')
+      .end(function (err, res) {
+        lastRev = res.body._rev;
+        done(err);
+      });
+    });
+
+    it('should delete 401 when no auth', function (done) {
+      request(app)
+      .del('/testputmodule/-rev/' + lastRev)
+      .expect(401, done);
+    });
+
+    it('should delete 403 when auth error', function (done) {
+      request(app)
+      .del('/testputmodule/-rev/' + lastRev)
+      .set('authorization', baseauthOther)
+      .expect(403, done);
+    });
+
+    it('shold remove all the module ok', function (done) {
+      //do not really remove
+      mm.empty(Module, 'removeByName');
+      request(app)
+      .del('/testputmodule/-rev/' + lastRev)
+      .set('authorization', baseauth)
+      .expect(200, done);
+    });
+  });
 });