Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
fix($cacheFactory): check key exists before decreasing cache size count
Browse files Browse the repository at this point in the history
Previously, there was no check for the existence of an item in the
cache when calling `$cacheFactory.remove()` before modifying the cache size
count.

Closes #12321
Closes #12329
  • Loading branch information
tealtail authored and petebacondarwin committed Oct 28, 2015
1 parent b2fc39d commit b9bed7d
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 5 deletions.
11 changes: 6 additions & 5 deletions src/ng/cacheFactory.js
Expand Up @@ -93,9 +93,9 @@ function $CacheFactoryProvider() {

var size = 0,
stats = extend({}, options, {id: cacheId}),
data = {},
data = createMap(),
capacity = (options && options.capacity) || Number.MAX_VALUE,
lruHash = {},
lruHash = createMap(),
freshEnd = null,
staleEnd = null;

Expand Down Expand Up @@ -223,6 +223,8 @@ function $CacheFactoryProvider() {
delete lruHash[key];
}

if (!(key in data)) return;

delete data[key];
size--;
},
Expand All @@ -237,9 +239,9 @@ function $CacheFactoryProvider() {
* Clears the cache object of any entries.
*/
removeAll: function() {
data = {};
data = createMap();
size = 0;
lruHash = {};
lruHash = createMap();
freshEnd = staleEnd = null;
},

Expand Down Expand Up @@ -399,4 +401,3 @@ function $TemplateCacheProvider() {
return $cacheFactory('templates');
}];
}

13 changes: 13 additions & 0 deletions test/ng/cacheFactorySpec.js
Expand Up @@ -133,6 +133,19 @@ describe('$cacheFactory', function() {
expect(cache.info().size).toBe(0);
}));

it('should only decrement size when an element is actually removed via remove', inject(function($cacheFactory) {
cache.put('foo', 'bar');
expect(cache.info().size).toBe(1);

cache.remove('undefined');
expect(cache.info().size).toBe(1);

cache.remove('hasOwnProperty');
expect(cache.info().size).toBe(1);

cache.remove('foo');
expect(cache.info().size).toBe(0);
}));

it('should return cache id', inject(function($cacheFactory) {
expect(cache.info().id).toBe('test');
Expand Down

0 comments on commit b9bed7d

Please sign in to comment.