From d71f16e7459f1d3705ccf47a13227d4727be9670 Mon Sep 17 00:00:00 2001 From: Shahar Talmi Date: Sun, 8 Jun 2014 21:31:17 +0300 Subject: [PATCH] fix(injector): allow multiple loading of function modules Change HashMap to give $$hashKey also for functions so it will be possible to load multiple module function instances. In order to prevent problem in angular's test suite, added an option to HashMap to maintain its own id counter and added cleanup of $$hashKey from all module functions after each test. Before this CL, functions were added to the HashMap via toString(), which could potentially return the same value for different actual instances of a function. This corrects this behaviour by ensuring that functions are mapped with hashKeys, and ensuring that hashKeys are removed from functions and objects at the end of tests. In addition to these changes, the injector uses its own set of UIDs in order to prevent confusingly breaking tests which expect scopes or ng-repeated items to have specific hash keys. Closes #7255 --- src/apis.js | 20 +++++++++++++------- src/auto/injector.js | 2 +- src/ngMock/angular-mocks.js | 6 ++++++ test/ApiSpecs.js | 30 ++++++++++++++++++++++++++++++ test/auto/injectorSpec.js | 23 +++++++++++++++++++++++ test/ngMock/angular-mocksSpec.js | 17 +++++++++++++++++ 6 files changed, 90 insertions(+), 8 deletions(-) diff --git a/src/apis.js b/src/apis.js index a6df5811c87d..3b0a22ab3191 100644 --- a/src/apis.js +++ b/src/apis.js @@ -13,16 +13,16 @@ * @returns {string} hash string such that the same input will have the same hash string. * The resulting string key is in 'type:hashKey' format. */ -function hashKey(obj) { +function hashKey(obj, nextUidFn) { var objType = typeof obj, key; - if (objType == 'object' && obj !== null) { + if (objType == 'function' || (objType == 'object' && obj !== null)) { if (typeof (key = obj.$$hashKey) == 'function') { // must invoke on object to keep the right this key = obj.$$hashKey(); } else if (key === undefined) { - key = obj.$$hashKey = nextUid(); + key = obj.$$hashKey = (nextUidFn || nextUid)(); } } else { key = obj; @@ -34,7 +34,13 @@ function hashKey(obj) { /** * HashMap which can use objects as keys */ -function HashMap(array){ +function HashMap(array, isolatedUid) { + if (isolatedUid) { + var uid = 0; + this.nextUid = function() { + return ++uid; + }; + } forEach(array, this.put, this); } HashMap.prototype = { @@ -44,7 +50,7 @@ HashMap.prototype = { * @param value value to store can be any type */ put: function(key, value) { - this[hashKey(key)] = value; + this[hashKey(key, this.nextUid)] = value; }, /** @@ -52,7 +58,7 @@ HashMap.prototype = { * @returns {Object} the value for the key */ get: function(key) { - return this[hashKey(key)]; + return this[hashKey(key, this.nextUid)]; }, /** @@ -60,7 +66,7 @@ HashMap.prototype = { * @param key */ remove: function(key) { - var value = this[key = hashKey(key)]; + var value = this[key = hashKey(key, this.nextUid)]; delete this[key]; return value; } diff --git a/src/auto/injector.js b/src/auto/injector.js index c74b4d6ddaa3..cf03d23ee33c 100644 --- a/src/auto/injector.js +++ b/src/auto/injector.js @@ -591,7 +591,7 @@ function createInjector(modulesToLoad) { var INSTANTIATING = {}, providerSuffix = 'Provider', path = [], - loadedModules = new HashMap(), + loadedModules = new HashMap([], true), providerCache = { $provide: { provider: supportObject(provider), diff --git a/src/ngMock/angular-mocks.js b/src/ngMock/angular-mocks.js index 0cba5720d1b3..8846d6860476 100644 --- a/src/ngMock/angular-mocks.js +++ b/src/ngMock/angular-mocks.js @@ -1958,6 +1958,12 @@ if(window.jasmine || window.mocha) { (window.afterEach || window.teardown)(function() { var injector = currentSpec.$injector; + angular.forEach(currentSpec.$modules, function(module) { + if (module && module.$$hashKey) { + module.$$hashKey = undefined; + } + }); + currentSpec.$injector = null; currentSpec.$modules = null; currentSpec = null; diff --git a/test/ApiSpecs.js b/test/ApiSpecs.js index 12de39d03fcd..d11558c27fa5 100644 --- a/test/ApiSpecs.js +++ b/test/ApiSpecs.js @@ -22,6 +22,36 @@ describe('api', function() { expect(map.get('b')).toBe(1); expect(map.get('c')).toBe(undefined); }); + + it('should maintain hashKey for object keys', function() { + var map = new HashMap(); + var key = {}; + map.get(key); + expect(key.$$hashKey).toBeDefined(); + }); + + it('should maintain hashKey for function keys', function() { + var map = new HashMap(); + var key = function() {}; + map.get(key); + expect(key.$$hashKey).toBeDefined(); + }); + + it('should share hashKey between HashMap by default', function() { + var map1 = new HashMap(), map2 = new HashMap(); + var key1 = {}, key2 = {}; + map1.get(key1); + map2.get(key2); + expect(key1.$$hashKey).not.toEqual(key2.$$hashKey); + }); + + it('should maintain hashKey per HashMap if flag is passed', function() { + var map1 = new HashMap([], true), map2 = new HashMap([], true); + var key1 = {}, key2 = {}; + map1.get(key1); + map2.get(key2); + expect(key1.$$hashKey).toEqual(key2.$$hashKey); + }); }); }); diff --git a/test/auto/injectorSpec.js b/test/auto/injectorSpec.js index 18cca139cfd9..3b1a518090f5 100644 --- a/test/auto/injectorSpec.js +++ b/test/auto/injectorSpec.js @@ -293,6 +293,29 @@ describe('injector', function() { expect(log).toEqual('abc'); }); + it('should load different instances of dependent functions', function() { + function generateValueModule(name, value) { + return function ($provide) { + $provide.value(name, value); + }; + } + var injector = createInjector([generateValueModule('name1', 'value1'), + generateValueModule('name2', 'value2')]); + expect(injector.get('name2')).toBe('value2'); + }); + + it('should load same instance of dependent function only once', function() { + var count = 0; + function valueModule($provide) { + count++; + $provide.value('name', 'value'); + } + + var injector = createInjector([valueModule, valueModule]); + expect(injector.get('name')).toBe('value'); + expect(count).toBe(1); + }); + it('should execute runBlocks after injector creation', function() { var log = ''; angular.module('a', [], function(){ log += 'a'; }).run(function() { log += 'A'; }); diff --git a/test/ngMock/angular-mocksSpec.js b/test/ngMock/angular-mocksSpec.js index c10bbc83e0fa..856fbb38de7c 100644 --- a/test/ngMock/angular-mocksSpec.js +++ b/test/ngMock/angular-mocksSpec.js @@ -805,6 +805,23 @@ describe('ngMock', function() { expect(example).toEqual('win'); }); }); + + describe('module cleanup', function() { + function testFn() { + + } + + it('should add hashKey to module function', function() { + module(testFn); + inject(function () { + expect(testFn.$$hashKey).toBeDefined(); + }); + }); + + it('should cleanup hashKey after previous test', function() { + expect(testFn.$$hashKey).toBeUndefined(); + }); + }); }); describe('in DSL', function() {