Skip to content

Commit

Permalink
fix(injector): allow multiple loading of function modules
Browse files Browse the repository at this point in the history
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 angular#7255
  • Loading branch information
shahata authored and caitp committed Jun 17, 2014
1 parent ed59370 commit d71f16e
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 8 deletions.
20 changes: 13 additions & 7 deletions src/apis.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 = {
Expand All @@ -44,23 +50,23 @@ 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;
},

/**
* @param key
* @returns {Object} the value for the key
*/
get: function(key) {
return this[hashKey(key)];
return this[hashKey(key, this.nextUid)];
},

/**
* Remove the key/value pair
* @param key
*/
remove: function(key) {
var value = this[key = hashKey(key)];
var value = this[key = hashKey(key, this.nextUid)];
delete this[key];
return value;
}
Expand Down
2 changes: 1 addition & 1 deletion src/auto/injector.js
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ function createInjector(modulesToLoad) {
var INSTANTIATING = {},
providerSuffix = 'Provider',
path = [],
loadedModules = new HashMap(),
loadedModules = new HashMap([], true),
providerCache = {
$provide: {
provider: supportObject(provider),
Expand Down
6 changes: 6 additions & 0 deletions src/ngMock/angular-mocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
30 changes: 30 additions & 0 deletions test/ApiSpecs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});

23 changes: 23 additions & 0 deletions test/auto/injectorSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'; });
Expand Down
17 changes: 17 additions & 0 deletions test/ngMock/angular-mocksSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

0 comments on commit d71f16e

Please sign in to comment.