diff --git a/lib/module-entry.js b/lib/module-entry.js index 77455ac..88e7728 100644 --- a/lib/module-entry.js +++ b/lib/module-entry.js @@ -17,7 +17,7 @@ var crypto = require('crypto'); */ module.exports = ModuleEntry; -function ModuleEntry(name, version, rootDir, sourceHash) { +function ModuleEntry(name, version, rootDir, sourceHash, isValid) { // The name of the module, from its package.json file. this.name = name; @@ -43,6 +43,8 @@ function ModuleEntry(name, version, rootDir, sourceHash) { // to the dependencies listed in the package.json file. // See locate() for initialization of this value. this._dependencies = Object.create(null); + + this.isValid = isValid === false ? false : true; } ModuleEntry.prototype.addDependency = function(dependency, moduleEntry) { @@ -115,6 +117,17 @@ ModuleEntry.prototype.getHash = function(heimdallNode) { return (this._hash = hash); }; +/* + * This exists to maintain compatibbility with some unexpected old behavior. + * Specifically invalid input, would still produce a hash. + * + * This is clearly not helpful, but to fix a regression we will restore this + * behavior, but the next major version (2.x.x) we should remove support for + * this, and instead error with a helpful error message + */ +ModuleEntry.buildInvalidModule = function(name, dir) { + return new this(name, '0.0.0' , dir, crypto.createHash('sha1').update([name, dir].filter(Boolean).join('\x00')).digest('hex'), false); +}; /* * Compute and return the ModuleEntry for a given name/dir pair. This is done @@ -139,7 +152,7 @@ ModuleEntry.locate = function(caches, name, dir, hashTreeFn) { if (realPathKey !== null) { return caches.MODULE_ENTRY.get(realPathKey); } else { - return null; + return this.buildInvalidModule(name, dir); } } @@ -150,8 +163,7 @@ ModuleEntry.locate = function(caches, name, dir, hashTreeFn) { var realPath = resolvePackagePath(name, dir, caches);; if (realPath === null) { - caches.PATH.set(nameDirKey, null); - return null; + return this.buildInvalidModule(name, dir); } // We have a path to a file that supposedly is package.json. We need to be sure diff --git a/tests/hash-for-dep-test.js b/tests/hash-for-dep-test.js index caaa5f9..1db86f4 100644 --- a/tests/hash-for-dep-test.js +++ b/tests/hash-for-dep-test.js @@ -69,6 +69,11 @@ describe('hashForDep', function() { assert.equal(result, 'b2d270f1274267a5fe29a49b5d44bb86125977f9', 'Expected sha1'); }); + it('maintains compatibility for now, not erroring for garbage input', function() { + expect(hashForDep('garbage-non-existing')).to.eql('c5c7d7981c22e790055a9b6e98a2972b8d14a599'); + expect(hashForDep('garbage-non-existing', 'with-garbage-basedir')).to.eql('507cb109ec6bd0781751f7eb65faf98183c79375'); + }); + describe('cache', function() { it('caches', function() { expect(hashForDep._cache.size).to.eql(0); @@ -129,7 +134,7 @@ describe('hashForDep', function() { expect(hashForDep._cache.size).to.eql(4); expect(hashForDep._caches.MODULE_ENTRY.size).to.eql(4); - expect(hashForDep._caches.PATH.size).to.eql(10); + expect(hashForDep._caches.PATH.size).to.eql(9); }); }); }); diff --git a/tests/module-entry-test.js b/tests/module-entry-test.js index 42708c1..be4fd40 100644 --- a/tests/module-entry-test.js +++ b/tests/module-entry-test.js @@ -35,7 +35,7 @@ describe('ModuleEntry', function() { it('._gatherDependencies', function() { // gatherDependencies only occurs during getHash(). - var moduleEntry = ModuleEntry.locate(caches, 'foo', fixturesPath, hashTree); + var moduleEntry = ModuleEntry.locate(caches, 'foo', fixturesPath, hashTree); var dependencies = Object.create(null); moduleEntry._gatherDependencies(dependencies); @@ -62,8 +62,8 @@ describe('ModuleEntry', function() { assert.equal(moduleEntry.rootDir, FOO_DIR_PATH, - 'rootDir must be in node_modules/foo'); - assert.equal(Object.keys(moduleEntry._dependencies).length, 2, 'foo._dependencies must have 2 entries'); + 'rootDir must be in node_modules/foo'); + assert.equal(Object.keys(moduleEntry._dependencies).length, 3, 'foo._dependencies must have 2 entries'); // XXX we should check which dependencies they are. @@ -73,7 +73,7 @@ describe('ModuleEntry', function() { moduleEntry = ModuleEntry.locate(caches, 'foo', fixturesPath, hashTree); var hash2 = moduleEntry.getHash(); - + assert.equal(hash1, hash2, 'getHash should return the same hash after clearing and recalculating'); }); @@ -84,7 +84,7 @@ describe('ModuleEntry', function() { var moduleEntry = ModuleEntry.locate(caches, 'foo', fixturesPath, newHashFn); assert.equal(moduleEntry.rootDir, FOO_DIR_PATH, 'rootDir must be in node_modules/foo'); - assert.equal(Object.keys(moduleEntry._dependencies).length, 2, 'foo._dependencies must have 2 entries'); + assert.equal(Object.keys(moduleEntry._dependencies).length, 3, 'foo._dependencies must have 2 entries'); var hash1 = moduleEntry.getHash(); @@ -92,15 +92,19 @@ describe('ModuleEntry', function() { moduleEntry = ModuleEntry.locate(caches, 'foo', fixturesPath, newHashFn); var hash2 = moduleEntry.getHash(); - + assert.equal(hash1, hash2, 'getHash should return the same hash after clearing and recalculating'); }); it('.locate (class method) with invalid package', function() { var moduleEntry = ModuleEntry.locate(caches, 'foo2', fixturesPath, hashTree); - assert.isNull(moduleEntry); + var moduleEntry2 = ModuleEntry.locate(caches, 'foo2', fixturesPath, hashTree); + assert.isNotNull(moduleEntry); + var hash1 = moduleEntry.getHash(); + assert.isOk(typeof hash1 === 'string'); + assert.equal(hash1, moduleEntry2.getHash()); }); - + it('.getHash', function() { var moduleEntry = ModuleEntry.locate(caches, 'foo', fixturesPath, hashTree);