From 0f023b029fea352f6b90ffce8d978fd4dfcffacf Mon Sep 17 00:00:00 2001
From: Sandy Vanderbleek <sandy.vanderbleek@gmail.com>
Date: Fri, 22 Feb 2013 16:50:01 -0800
Subject: [PATCH] PersistentStorage#get expires stale data

---
 src/js/persistent_storage.js    | 44 ++++++++++++++++++++++-----------
 test/persistent_storage_spec.js | 36 +++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 15 deletions(-)

diff --git a/src/js/persistent_storage.js b/src/js/persistent_storage.js
index 2f9cfb98..bf57b646 100644
--- a/src/js/persistent_storage.js
+++ b/src/js/persistent_storage.js
@@ -15,52 +15,66 @@ var PersistentStorage = (function() {
 
   if (window.localStorage && window.JSON) {
     methods = {
-      get: function(key) {
-        var ttl = decode(ls.getItem(this.prefix + key));
 
-        if (utils.isNumber(ttl) && now() > ttl) {
-          ls.removeItem(this.prefix + key + this.ttlKey);
+      // Private methods
+
+      _prefix: function(key) {
+        return this.prefix + key;
+      },
+
+      _ttlKey: function(key) {
+        return this._prefix(key) + this.ttlKey;
+      },
+
+      // Public methods
+
+      get: function(key) {
+        if (this.isExpired(key)) {
+          this.remove(key);
         }
 
-        return decode(ls.getItem(this.prefix + key));
+        return decode(ls.getItem(this._prefix(key)));
       },
 
       set: function(key, val, ttl) {
         if (utils.isNumber(ttl)) {
-          ls.setItem(this.prefix + key + this.ttlKey, encode(now() + ttl));
+          ls.setItem(this._ttlKey(key), encode(now() + ttl));
         }
 
         else {
-          ls.removeItem(this.prefix + key + this.ttlKey);
+          ls.removeItem(this._ttlKey(key));
         }
 
-        return ls.setItem(this.prefix + key, encode(val));
+        return ls.setItem(this._prefix(key), encode(val));
       },
 
       remove: function(key) {
-        ls.removeItem(this.prefix + key + this.ttlKey);
-        ls.removeItem(this.prefix + key);
+        ls.removeItem(this._ttlKey(key));
+        ls.removeItem(this._prefix(key));
 
         return this;
       },
 
       clear: function() {
-        var i, key, len = ls.length;
+        var i, key, keys = [], len = ls.length;
 
         for (i = 0; i < len; i += 1) {
           key = ls.key(i);
           if (key.match(this.keyMatcher)) {
-            i -= 1;
-            len -= 1;
-            this.remove(key.replace(this.keyMatcher, ""));
+            // gather keys to remove after loop exits
+            keys.push(key.replace(this.keyMatcher, ""));
           }
         }
 
+        for (i = keys.length; i--;) {
+          this.remove(keys[i]);
+        }
+
         return this;
       },
 
       isExpired: function(key) {
-        var ttl = decode(ls.getItem(this.prefix + key + this.ttlKey));
+        var ttl = decode(ls.getItem(this._ttlKey(key)));
 
         return utils.isNumber(ttl) && now() > ttl ? true : false;
       }
diff --git a/test/persistent_storage_spec.js b/test/persistent_storage_spec.js
index 2d618af3..4d1ed472 100644
--- a/test/persistent_storage_spec.js
+++ b/test/persistent_storage_spec.js
@@ -7,6 +7,8 @@ describe('PersistentStorage', function() {
     spyOn(ls, 'getItem').andCallThrough();
     spyOn(ls, 'setItem').andCallThrough();
     spyOn(ls, 'removeItem').andCallThrough();
+
+    spyOn(Date.prototype, 'getTime').andReturn(0);
   });
 
   afterEach(function() {
@@ -37,6 +39,13 @@ describe('PersistentStorage', function() {
       expect(engine.get('null')).toEqual(null);
       expect(engine.get('object')).toEqual({ obj: true });
     });
+
+    it('should expire stale keys', function() {
+      engine.set('key', 'value', -1);
+
+      expect(engine.get('key')).toEqual(undefined);
+      expect(ls.getItem('__ns__key__ttl')).toEqual(undefined);
+    });
   });
 
   describe('#set', function() {
@@ -50,6 +59,13 @@ describe('PersistentStorage', function() {
       engine.set('key', 'val');
       expect(ls.setItem.mostRecentCall.args[1]).toEqual(JSON.stringify('val'));
     });
+
+    it('should store ttl if provided', function() {
+      var ttl = 1;
+      engine.set('key', 'value', ttl);
+
+      expect(ls.setItem.argsForCall[0]).toEqual(['__ns__key__ttl__', ttl.toString()]);
+    });
   });
 
   describe('#remove', function() {
@@ -68,11 +84,13 @@ describe('PersistentStorage', function() {
       engine.set('key1', 'val1');
       engine.set('key2', 'val2');
       engine.set('key3', 'val3');
+      engine.set('key4', 'val4', 0);
       engine.clear();
 
       expect(engine.get('key1')).toEqual(undefined);
       expect(engine.get('key2')).toEqual(undefined);
       expect(engine.get('key3')).toEqual(undefined);
+      expect(engine.get('key4')).toEqual(undefined);
     });
 
     it('should not affect keys with different namespace', function() {
@@ -82,4 +100,22 @@ describe('PersistentStorage', function() {
       expect(ls.getItem('diff_namespace')).toEqual('val');
     });
   });
+
+  describe('#isExpired', function() {
+
+    it('should be false for keys without ttl', function() {
+      engine.set('key', 'value');
+      expect(engine.isExpired('key')).toBe(false);
+    });
+
+    it('should be false for fresh keys', function() {
+      engine.set('key', 'value', 1);
+      expect(engine.isExpired('key')).toBe(false);
+    });
+
+    it('should be true for stale keys', function() {
+      engine.set('key', 'value', -1);
+      expect(engine.isExpired('key')).toBe(true);
+    });
+  });
 });