perf: Do not copy value on setItem

Uses `angular.extend` instead of `angular.copy` to make an object to remove `$promise` from. This keeps us from doubling the memory footprint on every save.
scotttrinh · Feb 22, 2017 · 6a42de3 · 6a42de3
1 parent ac1c3b9
commit 6a42de3
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 24 deletions.
diff --git a/src/angular-localForage.js b/src/angular-localForage.js
@@ -113,34 +113,34 @@
             throw new Error('If you set an array of keys, the values should be an array too');
           }
 
-          var promises = [];
-          angular.forEach(key, function(k, index) {
-            promises.push(self.setItem(k, value[index]))
-          });
-
-          return $q.all(promises);
+          return $q.all(key.map(function (k, index) {
+            return self.setItem(k, value[index]);
+          }));
         } else {
           var deferred = $q.defer(),
             args = arguments,
-            localCopy = typeof Blob !== 'undefined' && typeof ArrayBuffer !== 'undefined' && (value instanceof Blob || value instanceof ArrayBuffer) ? value : angular.copy(value);
+            localCopy = value;
 
           //avoid $promises attributes from value objects, if present.
           if(angular.isObject(localCopy) && angular.isDefined(localCopy.$promise)) {
-            delete localCopy.$promise; //delete attribut from object structure.
+            localCopy = angular.extend({}, value);
+            delete localCopy.$promise;
           }
 
-          self._localforage.setItem(self.prefix() + key, localCopy).then(function success() {
-            if(notify.setItem) {
-              $rootScope.$broadcast('LocalForageModule.setItem', {
-                key: key,
-                newvalue: localCopy,
-                driver: self.driver()
-              });
-            }
-            deferred.resolve(localCopy);
-          }, function error(data) {
-            self.onError(data, args, self.setItem, deferred);
-          });
+          self._localforage.setItem(self.prefix() + key, localCopy)
+            .then(function success() {
+              if(notify.setItem) {
+                $rootScope.$broadcast('LocalForageModule.setItem', {
+                  key: key,
+                  newvalue: localCopy,
+                  driver: self.driver()
+                });
+              }
+              deferred.resolve(localCopy);
+            })
+            .catch(function withError(error) {
+              self.onError(data, args, self.setItem, deferred);
+            });
 
           return deferred.promise;
         }

diff --git a/tests/angular-localForage.js b/tests/angular-localForage.js
@@ -183,13 +183,19 @@ describe('Module: LocalForageModule', function() {
 
   it('setItem and getItem should work', function(done) {
     var interval = triggerDigests();
+    var objectToStore = {
+      name: 'Scott Trinh'
+    };
 
-    $localForage.setItem('myName', 'Olivier Combe').then(function(data) {
-      expect(data).toEqual('Olivier Combe');
+    spyOn($localForage._localforage, 'setItem').and.callThrough();
 
-      $localForage.getItem('myName').then(function(data) {
+    $localForage.setItem('myObject', objectToStore).then(function(data) {
+      expect(data).toEqual({ name: 'Scott Trinh' });
+      expect($localForage._localforage.setItem.calls.mostRecent().args[1]).toBe(objectToStore);
+
+      $localForage.getItem('myObject').then(function(data) {
         stopDigests(interval);
-        expect(data).toEqual('Olivier Combe');
+        expect(data).toEqual({ name: 'Scott Trinh' });
         done();
       }, done);
 
@@ -448,6 +454,43 @@ describe('Module: LocalForageModule', function() {
     }).toThrowError('You must define a key to set');
   });
 
+  it('setItem should remove $promise if present', function (done) {
+    var interval = triggerDigests();
+    var objectToStore = {
+      $promise: {},
+      childObject: {}
+    };
+    var objectNoPromise = {
+      noPromise: {}
+    };
+    spyOn($localForage._localforage, 'setItem').and.callThrough();
+
+    $localForage.setItem(['myObject', 'noPromise'], [objectToStore, objectNoPromise]).then(function() {
+      var setWith = $localForage._localforage.setItem.calls.argsFor(0)[1];
+      var setWithNoPromise = $localForage._localforage.setItem.calls.argsFor(1)[1];
+      expect(setWith).not.toBe(objectToStore);
+      expect(setWith.childObject).toBe(objectToStore.childObject);
+      expect(setWith).toEqual({
+        childObject: {}
+      });
+      expect(objectToStore).toEqual({
+        $promise: {},
+        childObject: {}
+      });
+
+      expect(setWithNoPromise).toBe(objectNoPromise);
+
+      $localForage.getItem('myObject').then(function(data) {
+        stopDigests(interval);
+        expect(Object.keys(data).length).toBe(1);
+        expect(data).toEqual({
+          childObject: {}
+        });
+        done();
+      }, done);
+    }, done);
+  })
+
   describe("bind", function() {
     var $scope, $q;