fix(database): store unwrapped snapshots

cartant · davideast · commit 9f3c47b78163 · 2017-01-30T08:54:16.000-08:00
Store either unwrapped snapshots or preserved snapshots; don't store preserved snapshots and call utils.unwrapMapFn when emitting. Closes #791
diff --git a/src/database/firebase_list_factory.spec.ts b/src/database/firebase_list_factory.spec.ts
@@ -49,14 +49,7 @@ function queryTest(observable: Observable<any>, subject: Subject<any>, done: any
 }
 
 describe('FirebaseListFactory', () => {
-  var subscription: Subscription;
-  var questions: FirebaseListObservable<any>;
-  var questionsSnapshotted: FirebaseListObservable<any>;
-  var ref: any;
-  var refSnapshotted: any;
-  var val1: any;
-  var val2: any;
-  var val3: any;
+
   var app: firebase.app.App;
 
   beforeEach(() => {
@@ -351,13 +344,22 @@ describe('FirebaseListFactory', () => {
         expect(observable.update instanceof Function).toBe(true);
         expect(observable.remove instanceof Function).toBe(true);
     });
-
-
   });
 
   describe('methods', () => {
 
+    var toKey;
+    var val1: any;
+    var val2: any;
+    var val3: any;
+    var questions: FirebaseListObservable<any>;
+    var questionsSnapshotted: FirebaseListObservable<any>;
+    var ref: any;
+    var refSnapshotted: any;
+    var subscription: Subscription;
+
     beforeEach((done: any) => {
+      toKey = (val) => val.key;
       val1 = { key: 'key1' };
       val2 = { key: 'key2' };
       val3 = { key: 'key3' };
@@ -368,6 +370,7 @@ describe('FirebaseListFactory', () => {
       refSnapshotted = questionsSnapshotted.$ref;
     });
 
+
     afterEach((done: any) => {
       if (subscription && !subscription.closed) {
         subscription.unsubscribe();
@@ -434,6 +437,46 @@ describe('FirebaseListFactory', () => {
     });
 
 
+    it('should re-emit identical instances of unchanged children', (done: any) => {
+
+      let prev;
+
+      take.call(questions, 2).subscribe(
+        (list) => {
+          if (prev) {
+            expect(list[0]).toBe(prev[0]);
+            done();
+          } else {
+            prev = list;
+            questions.push({ name: 'bob' });
+          }
+        },
+        done.fail
+      );
+      questions.push({ name: 'alice' });
+    });
+
+
+    it('should re-emit identical instances of unchanged children as snapshots', (done: any) => {
+
+      let prev;
+
+      take.call(questionsSnapshotted, 2).subscribe(
+        (list) => {
+          if (prev) {
+            expect(list[0]).toBe(prev[0]);
+            done();
+          } else {
+            prev = list;
+            questionsSnapshotted.push({ name: 'bob' });
+          }
+        },
+        done.fail
+      );
+      questionsSnapshotted.push({ name: 'alice' });
+    });
+
+
     it('should call off on all events when disposed', (done: any) => {
       const questionRef = firebase.database().ref().child('questions');
       var firebaseSpy = spyOn(questionRef, 'off').and.callThrough();
@@ -447,65 +490,57 @@ describe('FirebaseListFactory', () => {
 
 
     describe('onChildAdded', () => {
+
       it('should add the child after the prevKey', () => {
-        expect(onChildAdded([val1, val2], val3, 'key1')).toEqual([val1, val3, val2]);
+        expect(onChildAdded([val1, val2], val3, toKey, 'key1')).toEqual([val1, val3, val2]);
       });
 
 
       it('should not mutate the input array', () => {
         var inputArr = [val1];
-        expect(onChildAdded(inputArr, val2, 'key1')).not.toEqual(inputArr);
+        expect(onChildAdded(inputArr, val2, toKey, 'key1')).not.toEqual(inputArr);
       });
     });
 
 
     describe('onChildChanged', () => {
+
       it('should move the child after the specified prevKey', () => {
-        expect(onChildChanged([val1, val2], val1, 'key2')).toEqual([val2, val1]);
+        expect(onChildChanged([val1, val2], val1, toKey, 'key2')).toEqual([val2, val1]);
       });
 
 
       it('should move the child to the beginning if prevKey is null', () => {
         expect(
-          onChildChanged([val1, val2, val3], val2, null)
+          onChildChanged([val1, val2, val3], val2, toKey, null)
         ).toEqual([val2, val1, val3]);
       });
 
       it('should not duplicate the first item if it is the one that changed', () => {
         expect(
-          onChildChanged([val1, val2, val3], val1, null)
+          onChildChanged([val1, val2, val3], val1, toKey, null)
         ).not.toEqual([val1, val1, val2, val3]);
       });
 
       it('should not mutate the input array', () => {
         var inputArr = [val1, val2];
-        expect(onChildChanged(inputArr, val1, 'key2')).not.toEqual(inputArr);
+        expect(onChildChanged(inputArr, val1, toKey, 'key2')).not.toEqual(inputArr);
       });
 
 
       it('should update the child', () => {
         expect(
-          onChildUpdated([val1, val2, val3], { key: 'newkey' }, 'key1').map(v => v.key)
+          onChildUpdated([val1, val2, val3], { key: 'newkey' }, toKey, 'key1').map(v => v.key)
         ).toEqual(['key1', 'newkey', 'key3']);
       });
     });
 
 
     describe('onChildRemoved', () => {
-      var val1: any;
-      var val2: any;
-      var val3: any;
-
-      beforeEach(() => {
-        val1 = { key: () => 'key1' };
-        val2 = { key: () => 'key2' };
-        val3 = { key: () => 'key3' };
-      });
-
 
       it('should remove the child', () => {
         expect(
-          onChildRemoved([val1, val2, val3], val2)
+          onChildRemoved([val1, val2, val3], val2, toKey)
         ).toEqual([val1, val3]);
       });
     });
diff --git a/src/database/firebase_list_factory.ts b/src/database/firebase_list_factory.ts
@@ -108,14 +108,16 @@ export function FirebaseListFactory (
  * is loaded, the observable starts emitting values.
  */
 function firebaseListObservable(ref: firebase.database.Reference | firebase.database.Query, {preserveSnapshot}: FirebaseListFactoryOpts = {}): FirebaseListObservable<any> {
+  const toValue = preserveSnapshot ? (snapshot => snapshot) : utils.unwrapMapFn;
+  const toKey = preserveSnapshot ? (value => value.key) : (value => value.$key);
   // Keep track of callback handles for calling ref.off(event, handle)
   const handles = [];
   const listObs = new FirebaseListObservable(ref, (obs: Observer<any[]>) => {
     ref.once('value')
       .then((snap) => {
         let initialArray = [];
         snap.forEach(child => {
-          initialArray.push(child)
+          initialArray.push(toValue(child))
         });
         return initialArray;
       })
@@ -127,7 +129,7 @@ function firebaseListObservable(ref: firebase.database.Reference | firebase.data
         if (!isInitiallyEmpty) {
           // The last key in the initial array tells us where
           // to begin listening in realtime
-          lastKey = initialArray[initialArray.length - 1].key;
+          lastKey = toKey(initialArray[initialArray.length - 1]);
         }
 
         const addFn = ref.on('child_added', (child: any, prevKey: string) => {
@@ -137,18 +139,18 @@ function firebaseListObservable(ref: firebase.database.Reference | firebase.data
           if (!isInitiallyEmpty && !hasInitialLoad) {
             if (child.key === lastKey) {
               hasInitialLoad = true;
-              obs.next(preserveSnapshot ? initialArray : initialArray.map(utils.unwrapMapFn));
+              obs.next(initialArray);
               return;
             }
           }
 
           if (hasInitialLoad) {
-            initialArray = onChildAdded(initialArray, child, prevKey);
+            initialArray = onChildAdded(initialArray, toValue(child), toKey, prevKey);
           }
 
           // only emit the array after the initial load
           if (hasInitialLoad) {
-            obs.next(preserveSnapshot ? initialArray : initialArray.map(utils.unwrapMapFn));
+            obs.next(initialArray);
           }
         }, err => {
           if (err) { obs.error(err); obs.complete(); }
@@ -157,20 +159,20 @@ function firebaseListObservable(ref: firebase.database.Reference | firebase.data
         handles.push({ event: 'child_added', handle: addFn });
 
         let remFn = ref.on('child_removed', (child: any) => {
-          initialArray = onChildRemoved(initialArray, child)
+          initialArray = onChildRemoved(initialArray, toValue(child), toKey);
           if (hasInitialLoad) {
-            obs.next(preserveSnapshot ? initialArray : initialArray.map(utils.unwrapMapFn));
+            obs.next(initialArray);
           }
         }, err => {
           if (err) { obs.error(err); obs.complete(); }
         });
         handles.push({ event: 'child_removed', handle: remFn });
 
         let chgFn = ref.on('child_changed', (child: any, prevKey: string) => {
-          initialArray = onChildChanged(initialArray, child, prevKey)
+          initialArray = onChildChanged(initialArray, toValue(child), toKey, prevKey)
           if (hasInitialLoad) {
             // This also manages when the only change is prevKey change
-            obs.next(preserveSnapshot ? initialArray : initialArray.map(utils.unwrapMapFn));
+            obs.next(initialArray);
           }
         }, err => {
           if (err) { obs.error(err); obs.complete(); }
@@ -199,49 +201,51 @@ function firebaseListObservable(ref: firebase.database.Reference | firebase.data
   return observeOn.call(listObs, new utils.ZoneScheduler(Zone.current));
 }
 
-export function onChildAdded(arr:any[], child:any, prevKey:string): any[] {
+export function onChildAdded(arr:any[], child:any, toKey:(element:any)=>string, prevKey:string): any[] {
   if (!arr.length) {
     return [child];
   }
-
   return arr.reduce((accumulator:firebase.database.DataSnapshot[], curr:firebase.database.DataSnapshot, i:number) => {
     if (!prevKey && i===0) {
       accumulator.push(child);
     }
     accumulator.push(curr);
-    if (prevKey && prevKey === curr.key) {
+    if (prevKey && prevKey === toKey(curr)) {
       accumulator.push(child);
     }
     return accumulator;
   }, []);
 }
 
-export function onChildChanged(arr:any[], child:any, prevKey:string): any[] {
+export function onChildChanged(arr:any[], child:any, toKey:(element:any)=>string, prevKey:string): any[] {
+  const childKey = toKey(child);
   return arr.reduce((accumulator:any[], val:any, i:number) => {
+    const valKey = toKey(val);
     if (!prevKey && i==0) {
       accumulator.push(child);
-      if (val.key !== child.key) {
+      if (valKey !== childKey) {
         accumulator.push(val);
       }
-    } else if(val.key === prevKey) {
+    } else if(valKey === prevKey) {
       accumulator.push(val);
       accumulator.push(child);
-    } else if (val.key !== child.key) {
+    } else if (valKey !== childKey) {
       accumulator.push(val);
     }
     return accumulator;
   }, []);
 }
 
-export function onChildRemoved(arr:any[], child:any): any[] {
-  return arr.filter(c => c.key !== child.key);
+export function onChildRemoved(arr:any[], child:any, toKey:(element:any)=>string): any[] {
+  let childKey = toKey(child);
+  return arr.filter(c => toKey(c) !== childKey);
 }
 
-export function onChildUpdated(arr:any[], child:any, prevKey:string): any[] {
+export function onChildUpdated(arr:any[], child:any, toKey:(element:any)=>string, prevKey:string): any[] {
   return arr.map((v, i, arr) => {
     if(!prevKey && !i) {
       return child;
-    } else if (i > 0 && arr[i-1].key === prevKey) {
+    } else if (i > 0 && toKey(arr[i-1]) === prevKey) {
       return child;
     } else {
       return v;
