Merge pull request #458 from firebase/kato-extendArrayFactory

Simplify $extendFactory for $FirebaseArray

Author: Jacob Wenger

bower.json

@@ -36,6 +36,6 @@
   "devDependencies": {
     "lodash": "~2.4.1",
     "angular-mocks": "~1.2.18",
-    "mockfirebase": "~0.4.0"
+    "mockfirebase": "0.5.0"
   }
 }

src/FirebaseArray.js

@@ -12,24 +12,34 @@
    *    $$moved - called whenever a child_moved event occurs
    *    $$removed - called whenever a child_removed event occurs
    *    $$error - called when listeners are canceled due to a security error
+   *    $$process - called immediately after $$added/$$updated/$$moved/$$removed
+   *                to splice/manipulate the array and invokes $$notify
+   *
+   * Additionally, there is one more method of interest to devs extending this class:
+   *    $$notify - triggers notifications to any $watch listeners, called by $$process
    *
    * Instead of directly modifying this class, one should generally use the $extendFactory
-   * method to add or change how methods behave:
+   * method to add or change how methods behave. $extendFactory modifies the prototype of
+   * the array class by returning a clone of $FirebaseArray.
    *
    * <pre><code>
    * var NewFactory = $FirebaseArray.$extendFactory({
    *    // add a new method to the prototype
    *    foo: function() { return 'bar'; },
    *
    *    // change how records are created
-   *    $$added: function(snap) {
-   *       var rec = new Widget(snap);
-   *       this._process('child_added', rec);
+   *    $$added: function(snap, prevChild) {
+   *       return new Widget(snap, prevChild);
+   *    },
+   *
+   *    // change how records are updated
+   *    $$updated: function(snap) {
+   *      return this.$getRecord(snap.key()).update(snap);
    *    }
    * });
    * </code></pre>
    *
-   * And then the new factory can be used by passing it as an argument:
+   * And then the new factory can be passed as an argument:
    * <code>$firebase( firebaseRef, {arrayFactory: NewFactory}).$asArray();</code>
    */
   angular.module('firebase').factory('$FirebaseArray', ["$log", "$firebaseUtils",
@@ -108,7 +118,7 @@
           if( key !== null ) {
             return self.$inst().$set(key, $firebaseUtils.toJSON(item))
               .then(function(ref) {
-                self._notify('child_changed', key);
+                self.$$notify('child_changed', key);
                 return ref;
               });
           }
@@ -251,10 +261,11 @@
          * Called by $firebase to inform the array when a new item has been added at the server.
          * This method must exist on any array factory used by $firebase.
          *
-         * @param snap
+         * @param {object} snap a Firebase snapshot
          * @param {string} prevChild
+         * @return {object} the record to be inserted into the array
          */
-        $$added: function(snap, prevChild) {
+        $$added: function(snap/*, prevChild*/) {
           // check to make sure record does not exist
           var i = this.$indexFor($firebaseUtils.getKey(snap));
           if( i === -1 ) {
@@ -267,55 +278,59 @@
             rec.$priority = snap.getPriority();
             $firebaseUtils.applyDefaults(rec, this.$$defaults);
 
-            // add it to array and send notifications
-            this._process('child_added', rec, prevChild);
+            return rec;
           }
+          return false;
         },
 
         /**
          * Called by $firebase whenever an item is removed at the server.
-         * This method must exist on any arrayFactory passed into $firebase
+         * This method does not physically remove the objects, but instead
+         * returns a boolean indicating whether it should be removed (and
+         * taking any other desired actions before the remove completes).
          *
-         * @param snap
+         * @param {object} snap a Firebase snapshot
+         * @return {boolean} true if item should be removed
          */
         $$removed: function(snap) {
-          var rec = this.$getRecord($firebaseUtils.getKey(snap));
-          if( angular.isObject(rec) ) {
-            this._process('child_removed', rec);
-          }
+          return this.$indexFor($firebaseUtils.getKey(snap)) > -1;
         },
 
         /**
          * Called by $firebase whenever an item is changed at the server.
-         * This method must exist on any arrayFactory passed into $firebase
+         * This method should apply the changes, including changes to data
+         * and to $priority, and then return true if any changes were made.
          *
-         * @param snap
+         * @param {object} snap a Firebase snapshot
+         * @return {boolean} true if any data changed
          */
         $$updated: function(snap) {
+          var changed = false;
           var rec = this.$getRecord($firebaseUtils.getKey(snap));
           if( angular.isObject(rec) ) {
             // apply changes to the record
-            var changed = $firebaseUtils.updateRec(rec, snap);
+            changed = $firebaseUtils.updateRec(rec, snap);
             $firebaseUtils.applyDefaults(rec, this.$$defaults);
-            if( changed ) {
-              this._process('child_changed', rec);
-            }
           }
+          return changed;
         },
 
         /**
          * Called by $firebase whenever an item changes order (moves) on the server.
-         * This method must exist on any arrayFactory passed into $firebase
+         * This method should set $priority to the updated value and return true if
+         * the record should actually be moved. It should not actually apply the move
+         * operation.
          *
-         * @param snap
+         * @param {object} snap a Firebase snapshot
          * @param {string} prevChild
          */
-        $$moved: function(snap, prevChild) {
+        $$moved: function(snap/*, prevChild*/) {
           var rec = this.$getRecord($firebaseUtils.getKey(snap));
           if( angular.isObject(rec) ) {
             rec.$priority = snap.getPriority();
-            this._process('child_moved', rec, prevChild);
+            return true;
           }
+          return false;
         },
 
         /**
@@ -340,14 +355,15 @@
 
         /**
          * Handles placement of recs in the array, sending notifications,
-         * and other internals.
+         * and other internals. Called by the $firebase synchronization process
+         * after $$added, $$updated, $$moved, and $$removed.
          *
          * @param {string} event one of child_added, child_removed, child_moved, or child_changed
          * @param {object} rec
          * @param {string} [prevChild]
          * @private
          */
-        _process: function(event, rec, prevChild) {
+        $$process: function(event, rec, prevChild) {
           var key = this._getKey(rec);
           var changed = false;
           var pos;
@@ -367,27 +383,28 @@
               changed = true;
               break;
             default:
-              // nothing to do
+              throw new Error('Invalid event type: ' + event);
           }
           if( angular.isDefined(pos) ) {
             // add it to the array
             changed = this._addAfter(rec, prevChild) !== pos;
           }
           if( changed ) {
             // send notifications to anybody monitoring $watch
-            this._notify(event, key, prevChild);
+            this.$$notify(event, key, prevChild);
           }
           return changed;
         },
 
         /**
          * Used to trigger notifications for listeners registered using $watch
+         *
          * @param {string} event
          * @param {string} key
          * @param {string} [prevChild]
          * @private
          */
-        _notify: function(event, key, prevChild) {
+        $$notify: function(event, key, prevChild) {
           var eventData = {event: event, key: key};
           if( angular.isDefined(prevChild) ) {
             eventData.prevChild = prevChild;

src/firebase.js

@@ -220,10 +220,39 @@
           var def     = $firebaseUtils.defer();
           var array   = new ArrayFactory($inst, destroy, def.promise);
           var batch   = $firebaseUtils.batch();
-          var created = batch(array.$$added, array);
-          var updated = batch(array.$$updated, array);
-          var moved   = batch(array.$$moved, array);
-          var removed = batch(array.$$removed, array);
+          var created = batch(function(snap, prevChild) {
+            var rec = array.$$added(snap, prevChild);
+            if( rec ) {
+              array.$$process('child_added', rec, prevChild);
+            }
+          });
+          var updated = batch(function(snap) {
+            var rec = array.$getRecord($firebaseUtils.getKey(snap));
+            if( rec ) {
+              var changed = array.$$updated(snap);
+              if( changed ) {
+                array.$$process('child_changed', rec);
+              }
+            }
+          });
+          var moved   = batch(function(snap, prevChild) {
+            var rec = array.$getRecord($firebaseUtils.getKey(snap));
+            if( rec ) {
+              var confirmed = array.$$moved(snap, prevChild);
+              if( confirmed ) {
+                array.$$process('child_moved', rec, prevChild);
+              }
+            }
+          });
+          var removed = batch(function(snap) {
+            var rec = array.$getRecord($firebaseUtils.getKey(snap));
+            if( rec ) {
+              var confirmed = array.$$removed(snap);
+              if( confirmed ) {
+                array.$$process('child_removed', rec);
+              }
+            }
+          });
           var error   = batch(array.$$error, array);
           var resolve = batch(_resolveFn);
 

tests/lib/jasmineMatchers.js

@@ -6,16 +6,6 @@
 beforeEach(function() {
   'use strict';
 
-  // taken from Angular.js 2.0
-  var isArray = (function() {
-    if (typeof Array.isArray !== 'function') {
-      return function(value) {
-        return toString.call(value) === '[object Array]';
-      };
-    }
-    return Array.isArray;
-  })();
-
   function extendedTypeOf(x) {
     var actual;
     if( isArray(x) ) {
@@ -78,12 +68,12 @@ beforeEach(function() {
     // inspired by: https://gist.github.com/prantlf/8631877
     toBeInstanceOf: function() {
       return {
-        compare: function (actual, expected) {
+        compare: function (actual, expected, name) {
           var result = {
             pass: actual instanceof expected
           };
           var notText = result.pass? ' not' : '';
-          result.message = 'Expected ' + actual + notText + ' to be an instance of ' + expected;
+          result.message = 'Expected ' + actual + notText + ' to be an instance of ' + (name||expected.constructor.name);
           return result;
         }
       };
@@ -97,30 +87,104 @@ beforeEach(function() {
      */
     toBeA: function() {
       return {
-        compare: compare.bind(null, 'a')
+        compare: function() {
+          var args = Array.prototype.slice.apply(arguments);
+          return compare.apply(null, ['a'].concat(args));
+        }
       };
     },
 
     toBeAn: function() {
       return {
-        compare: compare.bind(null, 'an')
+        compare: function(actual) {
+          var args = Array.prototype.slice.apply(arguments);
+          return compare.apply(null, ['an'].concat(args));
+        }
       }
     },
 
     toHaveKey: function() {
       return {
         compare: function(actual, key) {
-          var pass = actual.hasOwnProperty(key);
+          var pass =
+            actual &&
+            typeof(actual) === 'object' &&
+            actual.hasOwnProperty(key);
+          var notText = pass? ' not' : '';
+          return {
+            pass: pass,
+            message: 'Expected key ' + key + notText + ' to exist in ' + extendedTypeOf(actual)
+          }
+        }
+      }
+    },
+
+    toHaveLength: function() {
+      return {
+        compare: function(actual, len) {
+          var actLen = isArray(actual)? actual.length : 'not an array';
+          var pass = actLen === len;
+          var notText = pass? ' not' : '';
+          return {
+            pass: pass,
+            message: 'Expected array ' + notText + ' to have length ' + len + ', but it was ' + actLen
+          }
+        }
+      }
+    },
+
+    toBeEmpty: function() {
+      return {
+        compare: function(actual) {
+          var pass, contents;
+          if( isObject(actual) ) {
+            actual = Object.keys(actual);
+          }
+          if( isArray(actual) ) {
+            pass = actual.length === 0;
+            contents = 'had ' + actual.length + ' items';
+          }
+          else {
+            pass = false;
+            contents = 'was not an array or object';
+          }
           var notText = pass? ' not' : '';
           return {
             pass: pass,
-            message: 'Expected ' + key + notText + ' to exist in ' + extendedTypeOf(actual)
+            message: 'Expected collection ' + notText + ' to be empty, but it ' + contents
+          }
+        }
+      }
+    },
+
+    toHaveCallCount: function() {
+      return {
+        compare: function(spy, expCount) {
+          var pass, not, count;
+          count = spy.calls.count();
+          pass = count === expCount;
+          not = pass? '" not' : '"';
+          return {
+            pass: pass,
+            message: 'Expected spy "' + spy.and.identity() + not + ' to have been called ' + expCount + ' times'
+            + (pass? '' : ', but it was called ' + count)
           }
         }
       }
     }
   });
 
+  function isObject(x) {
+    return x && typeof(x) === 'object' && !isArray(x);
+  }
+
+  function isArray(x) {
+    if (typeof Array.isArray !== 'function') {
+      return x && typeof x === 'object' && Object.prototype.toString.call(x) === '[object Array]';
+    }
+    return Array.isArray(x);
+  }
+
   function isFirebaseRef(obj) {
     return extendedTypeOf(obj) === 'object' &&
       typeof obj.ref === 'function' &&

tests/lib/module.testutils.js

@@ -35,8 +35,11 @@ angular.module('testutils', ['firebase'])
           getPriority: function () {
             return angular.isDefined(pri) ? pri : null;
           },
+          key: function() {
+            return ref.ref().key();
+          },
           name: function () {
-            return ref.ref().name();
+            return ref.ref().key();
           },
           child: function (key) {
             var childData = angular.isObject(data) && data.hasOwnProperty(key) ? data[key] : null;