Skip to content

Commit

Permalink
fix(database): don't always call utils.unwrapMapFn
Browse files Browse the repository at this point in the history 
Store either unwrapped snapshots or preserved snapshots; don't store
preserved snapshots and call utils.unwrapMapFn when emitting.

Closes angular#791
  • Loading branch information
@cartant
cartant committed Jan 26, 2017
1 parent fc3774a commit d9469cb
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 48 deletions.
91 changes: 63 additions & 28 deletions src/database/firebase_list_factory.spec.ts
View file
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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' };
Expand All @@ -368,6 +370,7 @@ describe('FirebaseListFactory', () => {
refSnapshotted = questionsSnapshotted.$ref;
});


afterEach((done: any) => {
if (subscription && !subscription.closed) {
subscription.unsubscribe();
Expand Down Expand Up @@ -434,6 +437,46 @@ describe('FirebaseListFactory', () => {
});


it('should emit a unchanged references for unchanged, unwrapped elements', (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 emit a unchanged references for unchanged, snapshot elements', (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();
Expand All @@ -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]);
});
});
Expand Down
44 changes: 24 additions & 20 deletions src/database/firebase_list_factory.ts
View file
Expand Up @@ -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;
})
Expand All @@ -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) => {
Expand All @@ -137,18 +139,18 @@ function firebaseListObservable(ref: firebase.database.Reference | firebase.data
if (!isInitiallyEmpty) {
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(); }
Expand All @@ -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(); }
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit d9469cb

Please sign in to comment.