Merge branch 'master' into initial-null-value-bug

jamestalmage · jamestalmage · commit bfbb0433aeed · 2015-02-03T01:42:57.000-05:00
Conflicts:
	src/FirebaseObject.js
diff --git a/bower.json b/bower.json
@@ -34,8 +34,7 @@
     "firebase": "2.1.x"
   },
   "devDependencies": {
-    "lodash": "~2.4.1",
-    "angular-mocks": "~1.2.18",
-    "mockfirebase": "~0.7.0"
+    "angular-mocks": "~1.3.11",
+    "mockfirebase": "~0.8.0"
   }
 }
diff --git a/package.json b/package.json
@@ -35,29 +35,30 @@
     "firebase": "2.1.x"
   },
   "devDependencies": {
-    "coveralls": "^2.11.1",
-    "grunt": "~0.4.1",
+    "coveralls": "^2.11.2",
+    "grunt": "~0.4.5",
     "grunt-cli": "^0.1.13",
-    "grunt-contrib-concat": "^0.4.0",
-    "grunt-contrib-connect": "^0.7.1",
-    "grunt-contrib-jshint": "~0.10.0",
-    "grunt-contrib-uglify": "~0.2.2",
-    "grunt-contrib-watch": "~0.5.1",
-    "grunt-karma": "~0.8.3",
-    "grunt-notify": "~0.2.7",
-    "grunt-protractor-runner": "^1.0.0",
-    "grunt-shell-spawn": "^0.3.0",
-    "jasmine-spec-reporter": "^0.4.0",
-    "karma": "~0.12.0",
-    "karma-chrome-launcher": "^0.1.4",
-    "karma-coverage": "^0.2.4",
-    "karma-failed-reporter": "0.0.2",
+    "grunt-contrib-concat": "^0.5.0",
+    "grunt-contrib-connect": "^0.9.0",
+    "grunt-contrib-jshint": "^0.11.0",
+    "grunt-contrib-uglify": "^0.7.0",
+    "grunt-contrib-watch": "^0.6.1",
+    "grunt-karma": "^0.10.1",
+    "grunt-notify": "^0.4.1",
+    "grunt-protractor-runner": "^1.2.1",
+    "grunt-shell-spawn": "^0.3.1",
+    "jasmine-core": "^2.1.3",
+    "jasmine-spec-reporter": "^2.1.0",
+    "karma": "~0.12.31",
+    "karma-chrome-launcher": "^0.1.7",
+    "karma-coverage": "^0.2.7",
+    "karma-failed-reporter": "0.0.3",
     "karma-html2js-preprocessor": "~0.1.0",
-    "karma-jasmine": "~0.2.0",
-    "karma-phantomjs-launcher": "~0.1.0",
-    "karma-sauce-launcher": "~0.2.9",
-    "karma-spec-reporter": "0.0.13",
-    "load-grunt-tasks": "~0.2.0",
-    "protractor": "^1.0.0"
+    "karma-jasmine": "^0.3.5",
+    "karma-phantomjs-launcher": "~0.1.4",
+    "karma-sauce-launcher": "~0.2.10",
+    "karma-spec-reporter": "0.0.16",
+    "load-grunt-tasks": "^3.1.0",
+    "protractor": "^1.6.1"
   }
 }
diff --git a/src/FirebaseArray.js b/src/FirebaseArray.js
@@ -210,6 +210,8 @@
         $loaded: function(resolve, reject) {
           var promise = this._promise;
           if( arguments.length ) {
+            // allow this method to be called just like .then
+            // by passing any arguments on to .then
             promise = promise.then.call(promise, resolve, reject);
           }
           return promise;
diff --git a/src/FirebaseObject.js b/src/FirebaseObject.js
@@ -318,64 +318,54 @@
             self.scope = scope;
             self.varName = varName;
 
-            function equals(rec) {
-              var parsed = getScope();
-              var newData = $firebaseUtils.scopeData(rec);
-              return angular.equals(parsed, newData) &&
-                parsed.$priority === rec.$priority &&
-                parsed.$value === rec.$value;
-            }
-
-            function getScope() {
-              return $firebaseUtils.scopeData(parsed(scope));
+            function equals(scopeValue) {
+              return angular.equals(scopeValue, rec) &&
+                scopeValue.$priority === rec.$priority &&
+                scopeValue.$value === rec.$value;
             }
 
             function setScope(rec) {
               parsed.assign(scope, $firebaseUtils.scopeData(rec));
             }
 
-            var send = $firebaseUtils.debounce(function() {
-              var scopeData = getScope();
-              rec.$$scopeUpdated(scopeData)
-                  ['finally'](function() {
-                    sending = false;
-                    if(!scopeData.hasOwnProperty('$value')){
-                      delete rec.$value;
-                      delete parsed(scope).$value;
-                    }
+            var send = $firebaseUtils.debounce(function(val) {
+              rec.$$scopeUpdated($firebaseUtils.scopeData(val))
+                ['finally'](function() {
+                  sending = false;
+                  if(!scopeData.hasOwnProperty('$value')){
+                    delete rec.$value;
+                    delete parsed(scope).$value;
                   }
+                }
               );
             }, 50, 500);
 
-            var scopeUpdated = function() {
-              if( !equals(rec) ) {
+            var scopeUpdated = function(newVal) {
+              newVal = newVal[0];
+              if( !equals(newVal) ) {
                 sending = true;
-                send();
+                send(newVal);
               }
             };
 
             var recUpdated = function() {
-              if( !sending && !equals(rec) ) {
+              if( !sending && !equals(parsed(scope)) ) {
                 setScope(rec);
               }
             };
 
             // $watch will not check any vars prefixed with $, so we
             // manually check $priority and $value using this method
-            function checkMetaVars() {
-              var dat = parsed(scope);
-              if( dat.$value !== rec.$value || dat.$priority !== rec.$priority ) {
-                scopeUpdated();
-              }
+            function watchExp(){
+              var obj = parsed(scope);
+              return [obj, obj.$priority, obj.$value];
             }
 
-            self.subs.push(scope.$watch(checkMetaVars));
-
             setScope(rec);
             self.subs.push(scope.$on('$destroy', self.unbind.bind(self)));
 
             // monitor scope for any changes
-            self.subs.push(scope.$watch(varName, scopeUpdated, true));
+            self.subs.push(scope.$watch(watchExp, scopeUpdated, true));
 
             // monitor the object for changes
             self.subs.push(rec.$watch(recUpdated));
diff --git a/src/firebase.js b/src/firebase.js
@@ -9,8 +9,8 @@
     //
     //   * `ref`: A Firebase reference. Queries or limits may be applied.
     //   * `config`: An object containing any of the advanced config options explained in API docs
-    .factory("$firebase", [ "$firebaseUtils", "$firebaseConfig",
-      function ($firebaseUtils, $firebaseConfig) {
+    .factory("$firebase", [ "$log", "$firebaseUtils", "$firebaseConfig",
+      function ($log, $firebaseUtils, $firebaseConfig) {
         function AngularFire(ref, config) {
           // make the new keyword optional
           if (!(this instanceof AngularFire)) {
@@ -52,7 +52,7 @@
             }
             if( angular.isFunction(ref.set) || !angular.isObject(data) ) {
               // this is not a query, just do a flat set
-              ref.ref().set(data, this._handle(def, ref));
+              ref.set(data, this._handle(def, ref));
             }
             else {
               var dataCopy = angular.extend({}, data);
@@ -202,7 +202,13 @@
             ref.on('child_removed', removed, error);
 
             // determine when initial load is completed
-            ref.once('value', function() { resolve(null); }, resolve);
+            ref.once('value', function(snap) {
+              if (angular.isArray(snap.val())) {
+                $log.warn('Storing data using array indices in Firebase can result in unexpected behavior. See https://www.firebase.com/docs/web/guide/understanding-data.html#section-arrays-in-firebase for more information.');
+              }
+
+              resolve(null);
+            }, resolve);
           }
 
           // call resolve(), do not call this directly
@@ -281,7 +287,13 @@
 
           function init() {
             ref.on('value', applyUpdate, error);
-            ref.once('value', function() { resolve(null); }, resolve);
+            ref.once('value', function(snap) {
+              if (angular.isArray(snap.val())) {
+                $log.warn('Storing data using array indices in Firebase can result in unexpected behavior. See https://www.firebase.com/docs/web/guide/understanding-data.html#section-arrays-in-firebase for more information.');
+              }
+
+              resolve(null);
+            }, resolve);
           }
 
           // call resolve(); do not call this directly
diff --git a/src/utils.js b/src/utils.js
@@ -430,6 +430,12 @@
             });
             return dat;
           },
+
+          /**
+           * AngularFire version number.
+           */
+          VERSION: '0.0.0',
+
           batchDelay: firebaseBatchDelay,
           allPromises: $q.all.bind($q)
         };
diff --git a/tests/protractor/chat/chat.spec.js b/tests/protractor/chat/chat.spec.js
@@ -2,9 +2,6 @@ var protractor = require('protractor');
 var Firebase = require('firebase');
 
 describe('Chat App', function () {
-  // Protractor instance
-  var ptor = protractor.getInstance();
-
   // Reference to the Firebase which stores the data for this demo
   var firebaseRef = new Firebase('https://angularFireTests.firebaseio-demo.com/chat');
 
@@ -148,4 +145,4 @@ describe('Chat App', function () {
     expect(messages.count()).toBe(0);
     expect(messagesCount.getText()).toEqual('0');
   });
-});
+});
diff --git a/tests/travis.sh b/tests/travis.sh
@@ -1,3 +1,5 @@
+#!/bin/bash
+set -e
 grunt build
 grunt test:unit
 if [ $TRAVIS_TAG ]; then
diff --git a/tests/unit/FirebaseObject.spec.js b/tests/unit/FirebaseObject.spec.js
@@ -346,8 +346,7 @@ describe('$FirebaseObject', function() {
         $scope.test.$priority = 999;
       });
       $interval.flush(500);
-      $timeout.flush(); // for $interval
-      $timeout.flush(); // for $watch
+      $timeout.flush();
       expect(spy).toHaveBeenCalledWith(jasmine.objectContaining({'.priority': 999}));
     });
 
@@ -363,11 +362,39 @@ describe('$FirebaseObject', function() {
         $scope.test.$value = 'bar';
       });
       $interval.flush(500);
-      $timeout.flush(); // for $interval
-      $timeout.flush(); // for $watch
+      $timeout.flush();
       expect(spy).toHaveBeenCalledWith(jasmine.objectContaining({'.value': 'bar'}));
     });
 
+    it('should only call $$scopeUpdated once if both metaVars and properties change in the same $digest',function(){
+      var $scope = $rootScope.$new();
+      var fb = new MockFirebase();
+      fb.autoFlush(true);
+      fb.setWithPriority({text:'hello'},3);
+      var $fb = new $firebase(fb);
+      var obj = $fb.$asObject();
+      flushAll();
+      flushAll();
+      obj.$bindTo($scope, 'test');
+      $scope.$apply();
+      expect($scope.test).toEqual({text:'hello', $id: obj.$id, $priority: 3});
+      var callCount = 0;
+      var old$scopeUpdated = obj.$$scopeUpdated;
+      obj.$$scopeUpdated = function(){
+        callCount++;
+        return old$scopeUpdated.apply(this,arguments);
+      };
+      $scope.$apply(function(){
+        $scope.test.text='goodbye';
+        $scope.test.$priority=4;
+      });
+      flushAll();
+      flushAll();
+      flushAll();
+      flushAll();
+      expect(callCount).toEqual(1);
+    });
+
     it('should throw error if double bound', function() {
       var $scope = $rootScope.$new();
       var aSpy = jasmine.createSpy('firstBind');
diff --git a/tests/unit/firebase.spec.js b/tests/unit/firebase.spec.js
@@ -136,11 +136,11 @@ describe('$firebase', function () {
     it('should reject if fails', function() {
       var whiteSpy = jasmine.createSpy('resolve');
       var blackSpy = jasmine.createSpy('reject');
-      $fb.$ref().failNext('push', 'failpush');
+      $fb.$ref().failNext('push', new Error('failpush'));
       $fb.$push({foo: 'bar'}).then(whiteSpy, blackSpy);
       flushAll();
       expect(whiteSpy).not.toHaveBeenCalled();
-      expect(blackSpy).toHaveBeenCalledWith('failpush');
+      expect(blackSpy).toHaveBeenCalledWith(new Error('failpush'));
     });
 
     it('should save correct data into Firebase', function() {
@@ -206,13 +206,13 @@ describe('$firebase', function () {
     });
 
     it('should reject if fails', function() {
-      $fb.$ref().failNext('set', 'setfail');
+      $fb.$ref().failNext('set', new Error('setfail'));
       var whiteSpy = jasmine.createSpy('resolve');
       var blackSpy = jasmine.createSpy('reject');
       $fb.$set({foo: 'bar'}).then(whiteSpy, blackSpy);
       flushAll();
       expect(whiteSpy).not.toHaveBeenCalled();
-      expect(blackSpy).toHaveBeenCalledWith('setfail');
+      expect(blackSpy).toHaveBeenCalledWith(new Error('setfail'));
     });
 
     it('should affect query keys only if query used', function() {
@@ -285,11 +285,11 @@ describe('$firebase', function () {
     it('should reject if fails', function() {
       var whiteSpy = jasmine.createSpy('resolve');
       var blackSpy = jasmine.createSpy('reject');
-      $fb.$ref().failNext('remove', 'test_fail_remove');
+      $fb.$ref().failNext('remove', new Error('test_fail_remove'));
       $fb.$remove().then(whiteSpy, blackSpy);
       flushAll();
       expect(whiteSpy).not.toHaveBeenCalled();
-      expect(blackSpy).toHaveBeenCalledWith('test_fail_remove');
+      expect(blackSpy).toHaveBeenCalledWith(new Error('test_fail_remove'));
     });
 
     it('should remove data in Firebase', function() {
@@ -371,11 +371,11 @@ describe('$firebase', function () {
     it('should reject if failed', function() {
       var whiteSpy = jasmine.createSpy('resolve');
       var blackSpy = jasmine.createSpy('reject');
-      $fb.$ref().failNext('update', 'oops');
+      $fb.$ref().failNext('update', new Error('oops'));
       $fb.$update({index: {foo: 'bar'}}).then(whiteSpy, blackSpy);
       flushAll();
       expect(whiteSpy).not.toHaveBeenCalled();
-      expect(blackSpy).toHaveBeenCalled();
+      expect(blackSpy).toHaveBeenCalledWith(new Error('oops'));
     });
 
     it('should not destroy untouched keys', function() {
@@ -435,11 +435,11 @@ describe('$firebase', function () {
     it('should reject if failed', function() {
       var whiteSpy = jasmine.createSpy('success');
       var blackSpy = jasmine.createSpy('failed');
-      $fb.$ref().child('a').failNext('transaction', 'test_fail');
+      $fb.$ref().child('a').failNext('transaction', new Error('test_fail'));
       $fb.$transaction('a', function() { return true; }).then(whiteSpy, blackSpy);
       flushAll();
       expect(whiteSpy).not.toHaveBeenCalled();
-      expect(blackSpy).toHaveBeenCalledWith('test_fail');
+      expect(blackSpy).toHaveBeenCalledWith(new Error('test_fail'));
     });
 
     it('should modify data in firebase', function() {
diff --git a/tests/unit/utils.spec.js b/tests/unit/utils.spec.js
@@ -260,7 +260,7 @@ describe('$firebaseUtils', function () {
       callback(false);
       expect(deferred.reject).toHaveBeenCalledWith(false);
     });
-    
+
     it('should resolve the promise if the first argument is null', function(){
       var result = {data:'hello world'};
       callback(null,result);
@@ -275,6 +275,11 @@ describe('$firebaseUtils', function () {
     });
   });
 
+  describe('#VERSION', function() {
+    it('should return the version number', function() {
+      expect($utils.VERSION).toEqual('0.0.0');
+    });
+  });
 });
 
 describe('#promise (ES6 Polyfill)', function(){

Original file line number	Diff line number	Diff line change
`@@ -34,8 +34,7 @@`
`34`	`34`	`"firebase": "2.1.x"`
`35`	`35`	`},`
`36`	`36`	`"devDependencies": {`
`37`		`- "lodash": "~2.4.1",`
`38`		`- "angular-mocks": "~1.2.18",`
`39`		`- "mockfirebase": "~0.7.0"`
	`37`	`+ "angular-mocks": "~1.3.11",`
	`38`	`+ "mockfirebase": "~0.8.0"`
`40`	`39`	`}`
`41`	`40`	`}`
Original file line number	Diff line number	Diff line change
`@@ -210,6 +210,8 @@`
`210`	`210`	`$loaded: function(resolve, reject) {`
`211`	`211`	`var promise = this._promise;`
`212`	`212`	`if( arguments.length ) {`
	`213`	`+ // allow this method to be called just like .then`
	`214`	`+ // by passing any arguments on to .then`
`213`	`215`	`promise = promise.then.call(promise, resolve, reject);`
`214`	`216`	`}`
`215`	`217`	`return promise;`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+#!/bin/bash`
	`2`	`+set -e`
`1`	`3`	`grunt build`
`2`	`4`	`grunt test:unit`
`3`	`5`	`if [ $TRAVIS_TAG ]; then`