Small performance improvement by accepting an optional package meta to avoid re-reading it.

Fix tests if previous run left dummy directories.

README.md

@@ -274,14 +274,14 @@ Returns the target.
 
 Returns the local temporary folder into which the package is being fetched. The files will remain here until the folder is moved when installing.
 
-`Resolver#hasNew(canonicalPkg)`: Promise
+`Resolver#hasNew(canonicalPkg, pkgMeta)`: Promise
 
 Checks if there is a version more recent than the provided `canonicalPkg` (folder) that complies with the resolver target.
 The hasNew process is as follows:
 
-- Reads the `package meta` from the `canonicalPkg`
-- Resolves to `true if it doesn't exist
-- Otherwise, calls _hasNew with the `package meta` and the `canonicalPkg` as arguments
+- Reads the `package meta` from the `canonical package` if not supplied
+- If there's an error while reading the `package meta`, it resolves to `true` because the package might be broken
+- Otherwise, calls `_hasNew()` with the `canonical package` and `package meta` as arguments
 
 If the resolver is already working, either resolving or checking for a newer version, the promise is immediately
 rejected.

lib/core/resolvers/GitResolver.js

@@ -17,7 +17,7 @@ mout.object.mixIn(GitResolver, Resolver);
 
 // -----------------
 
-GitResolver.prototype._hasNew = function (pkgMeta) {
+GitResolver.prototype._hasNew = function (canonicalPkg, pkgMeta) {
     var oldResolution = pkgMeta._resolution || {};
 
     return this._findResolution()

lib/core/resolvers/Resolver.js

@@ -38,15 +38,11 @@ Resolver.prototype.getTempDir = function () {
     return this._tempDir;
 };
 
-Resolver.prototype.hasNew = function (canonicalPkg) {
+Resolver.prototype.hasNew = function (canonicalPkg, pkgMeta) {
     var promise;
     var metaFile;
     var that = this;
 
-    // TODO: Change arguments to canonicalPkg, pkgMeta
-    //       where pkgMeta is optional
-    //       Change _hasNew to the same
-
     // If already working, error out
     if (this._working) {
         return Q.reject(createError('Already working', 'EWORKING'));
@@ -54,18 +50,18 @@ Resolver.prototype.hasNew = function (canonicalPkg) {
 
     this._working = true;
 
-    // Avoid reading the package meta if _hasNew was not rewritten
-    if (this._hasNew === Resolver.prototype._hasNew) {
-        promise = this._hasNew();
+    // Avoid reading the package meta if already given
+    if (pkgMeta) {
+        promise = this._hasNew(canonicalPkg, pkgMeta);
     // Otherwise call _hasNew with both the package meta and the canonical package
     } else {
         metaFile = path.join(canonicalPkg, '.bower.json');
         promise = Q.nfcall(fs.readFile, metaFile)
         .then(function (contents) {
             var pkgMeta = JSON.parse(contents.toString());
-            return that._hasNew(pkgMeta, canonicalPkg);
+            return that._hasNew(canonicalPkg, pkgMeta);
         }, function () {
-            return true;  // Simply resolve to true if there was an error reading the meta
+            return true;  // Simply resolve to true if there was an error reading the file
         });
     }
 
@@ -130,7 +126,7 @@ Resolver.prototype._resolve = function () {
 
 // -----------------
 
-Resolver.prototype._hasNew = function (pkgMeta) {
+Resolver.prototype._hasNew = function (canonicalPkg, pkgMeta) {
     return Q.resolve(true);
 };
 

lib/core/resolvers/UrlResolver.js

@@ -35,7 +35,7 @@ util.inherits(UrlResolver, Resolver);
 
 // -----------------
 
-UrlResolver.prototype._hasNew = function (pkgMeta) {
+UrlResolver.prototype._hasNew = function (canonicalPkg, pkgMeta) {
     var oldCacheHeaders = pkgMeta._cacheHeaders || {};
     var reqHeaders = {};
 

test/core/resolverFactory.js

@@ -1,6 +1,7 @@
 var expect = require('expect.js');
 var fs = require('fs');
 var path = require('path');
+var mkdirp = require('mkdirp');
 var mout = require('mout');
 var Q = require('q');
 var rimraf = require('rimraf');
@@ -166,7 +167,7 @@ describe('resolverFactory', function () {
         var temp;
 
         tempSource = path.resolve(__dirname, '../assets/tmp');
-        fs.mkdirSync(tempSource);
+        mkdirp.sync(tempSource);
         fs.writeFileSync(path.join(tempSource, '.git'), 'foo');
 
         endpoints = {};

test/core/resolvers/fsResolver.js

@@ -3,6 +3,7 @@ var path = require('path');
 var fs = require('fs');
 var path = require('path');
 var rimraf = require('rimraf');
+var mkdirp = require('mkdirp');
 var Q = require('q');
 var cmd = require('../../../lib/util/cmd');
 var copy = require('../../../lib/util/copy');
@@ -68,9 +69,15 @@ describe('FsResolver', function () {
 
     describe('.hasNew', function () {
         it('should resolve always to true (for now..)', function (next) {
-            var resolver = new FsResolver(path.relative(process.cwd(), testPackage));
+            var resolver = new FsResolver(testPackage);
 
-            resolver.hasNew()
+            tempSource = path.resolve(__dirname, '../../assets/tmp');
+            mkdirp.sync(tempSource);
+            fs.writeFileSync(path.join(tempSource, '.bower.json'), JSON.stringify({
+                name: 'test'
+            }));
+
+            resolver.hasNew(tempSource)
             .then(function (hasNew) {
                 expect(hasNew).to.be(true);
                 next();
@@ -145,7 +152,7 @@ describe('FsResolver', function () {
 
             tempSource = path.resolve(__dirname, '../../assets/tmp');
 
-            fs.mkdirSync(tempSource);
+            mkdirp.sync(tempSource);
             resolver = new FsResolver(tempSource);
 
             copy.copyFile(path.join(testPackage, 'foo'), path.join(tempSource, 'foo'))

test/core/resolvers/gitResolver.js

@@ -4,6 +4,7 @@ var path = require('path');
 var fs = require('fs');
 var chmodr = require('chmodr');
 var rimraf = require('rimraf');
+var mkdirp = require('mkdirp');
 var Q = require('q');
 var mout = require('mout');
 var copy = require('../../../lib/util/copy');
@@ -20,7 +21,7 @@ describe('GitResolver', function () {
 
     describe('.hasNew', function () {
         before(function () {
-            fs.mkdirSync(tempDir);
+            mkdirp.sync(tempDir);
         });
 
         afterEach(function (next) {
@@ -625,7 +626,7 @@ describe('GitResolver', function () {
 
     describe('._cleanup', function () {
         beforeEach(function () {
-            fs.mkdirSync(tempDir);
+            mkdirp.sync(tempDir);
         });
 
         afterEach(function (next) {
@@ -704,7 +705,7 @@ describe('GitResolver', function () {
 
     describe('._savePkgMeta', function () {
         before(function () {
-            fs.mkdirSync(tempDir);
+            mkdirp.sync(tempDir);
         });
 
         afterEach(function (next) {
@@ -734,6 +735,63 @@ describe('GitResolver', function () {
             .done();
         });
 
+        it('should save the release (under _release)', function (next) {
+            var resolver = new GitResolver('foo');
+            var metaFile = path.join(tempDir, '.bower.json');
+
+            // Test with type 'version'
+            resolver._resolution = { type: 'version', tag: '0.0.1', commit: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' };
+            resolver._tempDir = tempDir;
+
+            resolver._savePkgMeta({ name: 'foo', version: '0.0.1' })
+            .then(function () {
+                return Q.nfcall(fs.readFile, metaFile);
+            })
+            .then(function (contents) {
+                var json = JSON.parse(contents.toString());
+                expect(json._release).to.equal('0.0.1');
+            })
+            // Test with type 'tag'
+            .then(function () {
+                resolver._resolution = { type: 'tag', tag: '0.0.1', commit: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' };
+                return resolver._savePkgMeta({ name: 'foo' });
+            })
+            .then(function () {
+                return Q.nfcall(fs.readFile, metaFile);
+            })
+            .then(function (contents) {
+                var json = JSON.parse(contents.toString());
+                expect(json._release).to.equal('0.0.1');
+            })
+            // Test with type 'branch'
+            // In this case, it should be the commit
+            .then(function () {
+                resolver._resolution = { type: 'branch', commit: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' };
+                return resolver._savePkgMeta({ name: 'foo' });
+            })
+            .then(function () {
+                return Q.nfcall(fs.readFile, metaFile);
+            })
+            .then(function (contents) {
+                var json = JSON.parse(contents.toString());
+                expect(json._release).to.equal('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa');
+            })
+            // Test with type 'commit'
+            .then(function () {
+                resolver._resolution = { type: 'commit', commit: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' };
+                return resolver._savePkgMeta({ name: 'foo' });
+            })
+            .then(function () {
+                return Q.nfcall(fs.readFile, metaFile);
+            })
+            .then(function (contents) {
+                var json = JSON.parse(contents.toString());
+                expect(json._release).to.equal('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa');
+                next();
+            })
+            .done();
+        });
+
         it('should add the version to the package meta if not present and resolution is a version', function (next) {
             var resolver = new GitResolver('foo');
 
@@ -819,8 +877,6 @@ describe('GitResolver', function () {
             })
             .done();
         });
-
-        it.skip('should save the release (under _release)');
     });
 
     describe('#fetchBranches', function () {

test/core/resolvers/resolver.js

@@ -3,7 +3,9 @@ var fs = require('fs');
 var path = require('path');
 var util = require('util');
 var rimraf = require('rimraf');
+var mkdirp = require('mkdirp');
 var tmp = require('tmp');
+var Q = require('q');
 var cmd = require('../../../lib/util/cmd');
 var copy = require('../../../lib/util/copy');
 var Resolver = require('../../../lib/core/resolvers/Resolver');
@@ -55,6 +57,20 @@ describe('Resolver', function () {
     });
 
     describe('.hasNew', function () {
+        before(function () {
+            mkdirp.sync(tempDir);
+        });
+
+        beforeEach(function () {
+            fs.writeFileSync(path.join(tempDir, '.bower.json'), JSON.stringify({
+                name: 'test'
+            }));
+        });
+
+        after(function (next) {
+            rimraf(tempDir, next);
+        });
+
         it('should throw an error if already working (resolving)', function (next) {
             var resolver = new Resolver('foo');
             var succeeded;
@@ -71,7 +87,7 @@ describe('Resolver', function () {
             })
             .done();
 
-            resolver.hasNew()
+            resolver.hasNew(tempDir)
             .then(function () {
                 succeeded = true;
             }, function (err) {
@@ -85,17 +101,17 @@ describe('Resolver', function () {
             var resolver = new Resolver('foo');
             var succeeded;
 
-            resolver.hasNew()
+            resolver.hasNew(tempDir)
             .then(function () {
                 // Test if hasNew can be called again when done
-                resolver.hasNew()
+                resolver.hasNew(tempDir)
                 .then(function () {
                     next(succeeded ? new Error('Should have failed') : null);
                 });
             })
             .done();
 
-            resolver.hasNew()
+            resolver.hasNew(tempDir)
             .then(function () {
                 succeeded = true;
             }, function (err) {
@@ -108,13 +124,66 @@ describe('Resolver', function () {
         it('should resolve to true by default', function (next) {
             var resolver = new Resolver('foo');
 
-            resolver.hasNew('.')
+            resolver.hasNew(tempDir)
             .then(function (hasNew) {
                 expect(hasNew).to.equal(true);
                 next();
             })
             .done();
         });
+
+        it('should resolve to true if the there\'s an error reading the package meta', function (next) {
+            var resolver = new Resolver('foo');
+
+            rimraf.sync(path.join(tempDir, '.bower.json'));
+            resolver.hasNew(tempDir)
+            .then(function (hasNew) {
+                expect(hasNew).to.equal(true);
+                next();
+            })
+            .done();
+        });
+
+        it('should call _hasNew with the canonical package and the package meta', function (next) {
+            var resolver = new Resolver('foo');
+            var canonical;
+            var meta;
+
+            resolver._hasNew = function (canonicalPkg, pkgMeta) {
+                canonical = canonicalPkg;
+                meta = pkgMeta;
+                return Q.resolve(true);
+            };
+
+            resolver.hasNew(tempDir)
+            .then(function () {
+                expect(canonical).to.equal(tempDir);
+                expect(meta).to.be.an('object');
+                expect(meta.name).to.equal('test');
+                next();
+            })
+            .done();
+        });
+
+        it('should not read the package meta if already passed', function (next) {
+            var resolver = new Resolver('foo');
+            var meta;
+
+            resolver._hasNew = function (canonicalPkg, pkgMeta) {
+                meta = pkgMeta;
+                return Q.resolve(true);
+            };
+
+            resolver.hasNew(tempDir, {
+                name: 'foo'
+            })
+            .then(function () {
+                expect(meta).to.be.an('object');
+                expect(meta.name).to.equal('foo');
+                next();
+            })
+            .done();
+        });
     });
 
     describe('.resolve', function () {
@@ -164,10 +233,10 @@ describe('Resolver', function () {
 
             resolver._resolve = function () {};
 
-            resolver.hasNew()
+            resolver.hasNew(tempDir)
             .then(function () {
                 // Test if hasNew can be called again when done
-                resolver.hasNew()
+                resolver.hasNew(tempDir)
                 .then(function () {
                     next(succeeded ? new Error('Should have failed') : null);
                 });
@@ -355,7 +424,7 @@ describe('Resolver', function () {
         before(function () {
             var stat;
 
-            fs.mkdirSync(tempDir);
+            mkdirp.sync(tempDir);
             stat = fs.statSync(tempDir);
             dirMode0777 = stat.mode;
         });
@@ -456,7 +525,7 @@ describe('Resolver', function () {
         it('should read the bower.json file', function (next) {
             var resolver = new Resolver('foo');
 
-            fs.mkdirSync(tempDir);
+            mkdirp.sync(tempDir);
             fs.writeFileSync(path.join(tempDir, 'bower.json'), JSON.stringify({ name: 'foo', version: '0.0.0' }));
             fs.writeFileSync(path.join(tempDir, 'component.json'), JSON.stringify({ name: 'bar', version: '0.0.0' }));
 
@@ -474,7 +543,7 @@ describe('Resolver', function () {
             var resolver = new Resolver('foo');
             var notified = false;
 
-            fs.mkdirSync(tempDir);
+            mkdirp.sync(tempDir);
             fs.writeFileSync(path.join(tempDir, 'component.json'), JSON.stringify({ name: 'bar', version: '0.0.0' }));
 
             resolver._readJson(tempDir)
@@ -518,7 +587,7 @@ describe('Resolver', function () {
             var resolver = new Resolver('foo');
             var meta = { name: 'foo' };
 
-            fs.mkdirSync(tempDir);
+            mkdirp.sync(tempDir);
             resolver._tempDir = tempDir;
 
             resolver._applyPkgMeta(meta)
@@ -540,7 +609,7 @@ describe('Resolver', function () {
         it('should remove files that match the ignore patterns', function (next) {
             var resolver = new Resolver('foo', { name: 'foo' });
 
-            fs.mkdirSync(tempDir);
+            mkdirp.sync(tempDir);
 
             // Checkout test package version 0.2.1 which has a bower.json
             // with ignores
@@ -585,7 +654,7 @@ describe('Resolver', function () {
 
     describe('._savePkgMeta', function () {
         before(function () {
-            fs.mkdirSync(tempDir);
+            mkdirp.sync(tempDir);
         });
 
         afterEach(function (next) {

test/core/resolvers/urlResolver.js

@@ -5,6 +5,7 @@ var path = require('path');
 var nock = require('nock');
 var Q = require('q');
 var rimraf = require('rimraf');
+var mkdirp = require('mkdirp');
 var cmd = require('../../../lib/util/cmd');
 var UrlResolver = require('../../../lib/core/resolvers/UrlResolver');
 
@@ -64,7 +65,7 @@ describe('UrlResolver', function () {
 
     describe('.hasNew', function () {
         before(function () {
-            fs.mkdirSync(tempDir);
+            mkdirp.sync(tempDir);
         });
 
         afterEach(function (next) {