Skip to content

Commit a332bc4

Browse files
cartantdavideast
cartant
authored and
davideast
committed
chore(refactor): Removed factory path params
Closes angular#907
1 parent 38ba521 commit a332bc4

File tree

6 files changed

+46
-115
lines changed

6 files changed

+46
-115
lines changed

‎src/database/database.ts‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,12 @@ export class AngularFireDatabase {
1616

1717
list(pathOrRef: PathReference, opts?:FirebaseListFactoryOpts):FirebaseListObservable<any[]> {
1818
const ref = utils.getRef(this.app, pathOrRef);
19-
return FirebaseListFactory(utils.getRef(this.app, ref), opts);
19+
return FirebaseListFactory(ref, opts);
2020
}
2121

2222
object(pathOrRef: PathReference, opts?:FirebaseObjectFactoryOpts):FirebaseObjectObservable<any> {
23-
return utils.checkForUrlOrFirebaseRef(pathOrRef, {
24-
isUrl: () => FirebaseObjectFactory(this.app.database().ref(<string>pathOrRef), opts),
25-
isRef: () => FirebaseObjectFactory(pathOrRef)
26-
});
23+
const ref = utils.getRef(this.app, pathOrRef);
24+
return FirebaseObjectFactory(ref, opts);
2725
}
2826

2927
}

‎src/database/firebase_list_factory.spec.ts‎

Lines changed: 30 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -88,19 +88,8 @@ describe('FirebaseListFactory', () => {
8888

8989
describe('<constructor>', () => {
9090

91-
it('should accept a Firebase db path in the constructor', () => {
92-
const list = FirebaseListFactory(`questions`);
93-
expect(list instanceof FirebaseListObservable).toBe(true);
94-
});
95-
9691
it('should accept a Firebase db ref in the constructor', () => {
97-
const list = FirebaseListFactory(firebase.database().ref(`questions`));
98-
expect(list instanceof FirebaseListObservable).toBe(true);
99-
});
100-
101-
it('should take an absolute url in the constructor', () => {
102-
const absoluteUrl = COMMON_CONFIG.databaseURL + '/questions';
103-
const list = FirebaseListFactory(absoluteUrl);
92+
const list = FirebaseListFactory(app.database().ref(`questions`));
10493
expect(list instanceof FirebaseListObservable).toBe(true);
10594
});
10695

@@ -120,7 +109,7 @@ describe('FirebaseListFactory', () => {
120109
it('equalTo - should re-run a query when the observable value has emitted', (done: any) => {
121110

122111
const subject = new Subject();
123-
const observable = FirebaseListFactory(questionsPath, {
112+
const observable = FirebaseListFactory(app.database().ref(questionsPath), {
124113
query: {
125114
orderByChild: 'height',
126115
equalTo: subject
@@ -133,7 +122,7 @@ describe('FirebaseListFactory', () => {
133122
it('startAt - should re-run a query when the observable value has emitted', (done: any) => {
134123

135124
const subject = new Subject();
136-
const observable = FirebaseListFactory(questionsPath, {
125+
const observable = FirebaseListFactory(app.database().ref(questionsPath), {
137126
query: {
138127
orderByChild: 'height',
139128
startAt: subject
@@ -146,7 +135,7 @@ describe('FirebaseListFactory', () => {
146135
it('endAt - should re-run a query when the observable value has emitted', (done: any) => {
147136

148137
const subject = new Subject();
149-
const observable = FirebaseListFactory(questionsPath, {
138+
const observable = FirebaseListFactory(app.database().ref(questionsPath), {
150139
query: {
151140
orderByChild: 'height',
152141
endAt: subject
@@ -158,7 +147,7 @@ describe('FirebaseListFactory', () => {
158147

159148
it('should throw an error if limitToLast and limitToFirst are chained', () => {
160149

161-
const observable = FirebaseListFactory(questionsPath, {
150+
const observable = FirebaseListFactory(app.database().ref(questionsPath), {
162151
query: {
163152
orderByChild: 'height',
164153
limitToFirst: 10,
@@ -170,7 +159,7 @@ describe('FirebaseListFactory', () => {
170159

171160
it('should throw an error if startAt is used with equalTo', () => {
172161

173-
const observable = FirebaseListFactory(questionsPath, {
162+
const observable = FirebaseListFactory(app.database().ref(questionsPath), {
174163
query: {
175164
orderByChild: 'height',
176165
equalTo: 10,
@@ -182,7 +171,7 @@ describe('FirebaseListFactory', () => {
182171

183172
it('should throw an error if endAt is used with equalTo', () => {
184173

185-
const observable = FirebaseListFactory(questionsPath, {
174+
const observable = FirebaseListFactory(app.database().ref(questionsPath), {
186175
query: {
187176
orderByChild: 'height',
188177
equalTo: 10,
@@ -194,7 +183,7 @@ describe('FirebaseListFactory', () => {
194183

195184
it('should throw an error if startAt and endAt is used with equalTo', () => {
196185

197-
const observable = FirebaseListFactory(questionsPath, {
186+
const observable = FirebaseListFactory(app.database().ref(questionsPath), {
198187
query: {
199188
orderByChild: 'height',
200189
equalTo: 10,
@@ -219,7 +208,7 @@ describe('FirebaseListFactory', () => {
219208
it('equalTo - should re-run a query when the observable value has emitted', (done: any) => {
220209

221210
const subject = new Subject();
222-
const observable = FirebaseListFactory(questionsPath, {
211+
const observable = FirebaseListFactory(app.database().ref(questionsPath), {
223212
query: {
224213
orderByValue: true,
225214
equalTo: subject
@@ -232,7 +221,7 @@ describe('FirebaseListFactory', () => {
232221
it('startAt - should re-run a query when the observable value has emitted', (done: any) => {
233222

234223
const subject = new Subject();
235-
const observable = FirebaseListFactory(questionsPath, {
224+
const observable = FirebaseListFactory(app.database().ref(questionsPath), {
236225
query: {
237226
orderByValue: true,
238227
startAt: subject
@@ -245,7 +234,7 @@ describe('FirebaseListFactory', () => {
245234
it('endAt - should re-run a query when the observable value has emitted', (done: any) => {
246235

247236
const subject = new Subject();
248-
const observable = FirebaseListFactory(questionsPath, {
237+
const observable = FirebaseListFactory(app.database().ref(questionsPath), {
249238
query: {
250239
orderByValue: true,
251240
endAt: subject
@@ -269,7 +258,7 @@ describe('FirebaseListFactory', () => {
269258
it('equalTo - should re-run a query when the observable value has emitted', (done: any) => {
270259

271260
const subject = new Subject();
272-
const observable = FirebaseListFactory(questionsPath, {
261+
const observable = FirebaseListFactory(app.database().ref(questionsPath), {
273262
query: {
274263
orderByKey: true,
275264
equalTo: subject
@@ -282,7 +271,7 @@ describe('FirebaseListFactory', () => {
282271
it('startAt - should re-run a query when the observable value has emitted', (done: any) => {
283272

284273
const subject = new Subject();
285-
const observable = FirebaseListFactory(questionsPath, {
274+
const observable = FirebaseListFactory(app.database().ref(questionsPath), {
286275
query: {
287276
orderByKey: true,
288277
startAt: subject
@@ -295,7 +284,7 @@ describe('FirebaseListFactory', () => {
295284
it('endAt - should re-run a query when the observable value has emitted', (done: any) => {
296285

297286
const subject = new Subject();
298-
const observable = FirebaseListFactory(questionsPath, {
287+
const observable = FirebaseListFactory(app.database().ref(questionsPath), {
299288
query: {
300289
orderByKey: true,
301290
endAt: subject
@@ -318,7 +307,7 @@ describe('FirebaseListFactory', () => {
318307
it('equalTo - should re-run a query when the observable value has emitted', (done: any) => {
319308

320309
const subject = new Subject();
321-
const observable = FirebaseListFactory(questionsPath, {
310+
const observable = FirebaseListFactory(app.database().ref(questionsPath), {
322311
query: {
323312
orderByKey: true,
324313
equalTo: subject
@@ -331,7 +320,7 @@ describe('FirebaseListFactory', () => {
331320
it('startAt - should re-run a query when the observable value has emitted', (done: any) => {
332321

333322
const subject = new Subject();
334-
const observable = FirebaseListFactory(questionsPath, {
323+
const observable = FirebaseListFactory(app.database().ref(questionsPath), {
335324
query: {
336325
orderByKey: true,
337326
startAt: subject
@@ -344,7 +333,7 @@ describe('FirebaseListFactory', () => {
344333
it('endAt - should re-run a query when the observable value has emitted', (done: any) => {
345334

346335
const subject = new Subject();
347-
const observable = FirebaseListFactory(questionsPath, {
336+
const observable = FirebaseListFactory(app.database().ref(questionsPath), {
348337
query: {
349338
orderByKey: true,
350339
endAt: subject
@@ -360,7 +349,7 @@ describe('FirebaseListFactory', () => {
360349
describe('shape', () => {
361350

362351
it('should have a a FirebaseListObservable shape when queried', () => {
363-
const observable = FirebaseListFactory(questionsPath, {
352+
const observable = FirebaseListFactory(app.database().ref(questionsPath), {
364353
query: {
365354
orderByChild: 'height',
366355
equalTo: '1'
@@ -390,9 +379,9 @@ describe('FirebaseListFactory', () => {
390379
val1 = { key: 'key1' };
391380
val2 = { key: 'key2' };
392381
val3 = { key: 'key3' };
393-
firebase.database().ref().remove(done);
394-
questions = FirebaseListFactory(`questions`);
395-
questionsSnapshotted = FirebaseListFactory(`questionssnapshot`, { preserveSnapshot: true });
382+
app.database().ref().remove(done);
383+
questions = FirebaseListFactory(app.database().ref(`questions`));
384+
questionsSnapshotted = FirebaseListFactory(app.database().ref(`questionssnapshot`), { preserveSnapshot: true });
396385
ref = questions.$ref;
397386
refSnapshotted = questionsSnapshotted.$ref;
398387
});
@@ -588,7 +577,7 @@ describe('FirebaseListFactory', () => {
588577

589578

590579
it('should call off on all events when disposed', (done: any) => {
591-
const questionRef = firebase.database().ref().child('questions');
580+
const questionRef = app.database().ref().child('questions');
592581
let firebaseSpy = spyOn(questionRef, 'off').and.callThrough();
593582
subscription = FirebaseListFactory(questionRef).subscribe(_ => {
594583
expect(firebaseSpy).not.toHaveBeenCalled();
@@ -694,7 +683,7 @@ describe('FirebaseListFactory', () => {
694683
})
695684
.run(() => {
696685
// Creating a new observable so that the current zone is captured.
697-
subscription = FirebaseListFactory(`questions`)
686+
subscription = FirebaseListFactory(app.database().ref(`questions`))
698687
.filter(d => d
699688
.map(v => v.$value)
700689
.indexOf('in-the-zone') > -1)
@@ -757,15 +746,15 @@ describe('FirebaseListFactory', () => {
757746
})
758747
.then(() => {
759748

760-
let query1 = FirebaseListFactory(`questions`, {
749+
let query1 = FirebaseListFactory(app.database().ref(`questions`), {
761750
query: {
762751
orderByChild: 'data',
763752
startAt: { value: 0 }
764753
}
765754
});
766755
let promise1 = toPromise.call(take.call(query1, 1));
767756

768-
let query2 = FirebaseListFactory(`questions`, {
757+
let query2 = FirebaseListFactory(app.database().ref(`questions`), {
769758
query: {
770759
orderByChild: 'data',
771760
startAt: { value: 0, key: 'val2' }
@@ -795,15 +784,15 @@ describe('FirebaseListFactory', () => {
795784
})
796785
.then(() => {
797786

798-
let query1 = FirebaseListFactory(`questions`, {
787+
let query1 = FirebaseListFactory(app.database().ref(`questions`), {
799788
query: {
800789
orderByChild: 'data',
801790
equalTo: { value: 0 }
802791
}
803792
});
804793
let promise1 = toPromise.call(take.call(query1, 1));
805794

806-
let query2 = FirebaseListFactory(`questions`, {
795+
let query2 = FirebaseListFactory(app.database().ref(`questions`), {
807796
query: {
808797
orderByChild: 'data',
809798
equalTo: { value: 0, key: 'val2' }
@@ -833,15 +822,15 @@ describe('FirebaseListFactory', () => {
833822
})
834823
.then(() => {
835824

836-
let query1 = FirebaseListFactory(`questions`, {
825+
let query1 = FirebaseListFactory(app.database().ref(`questions`), {
837826
query: {
838827
orderByChild: 'data',
839828
endAt: { value: 0 }
840829
}
841830
});
842831
let promise1 = toPromise.call(take.call(query1, 1));
843832

844-
let query2 = FirebaseListFactory(`questions`, {
833+
let query2 = FirebaseListFactory(app.database().ref(`questions`), {
845834
query: {
846835
orderByChild: 'data',
847836
endAt: { value: 0, key: 'val2' }
@@ -871,7 +860,7 @@ describe('FirebaseListFactory', () => {
871860
.then(() => {
872861

873862
let subject = new Subject<boolean>();
874-
let query = FirebaseListFactory(`questions`, {
863+
let query = FirebaseListFactory(app.database().ref(`questions`), {
875864
query: {
876865
orderByChild: 'even',
877866
equalTo: subject

‎src/database/firebase_list_factory.ts‎

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6,33 +6,15 @@ import { FirebaseListObservable } from './firebase_list_observable';
66
import { Observer } from 'rxjs/Observer';
77
import { observeOn } from 'rxjs/operator/observeOn';
88
import { observeQuery } from './query_observable';
9-
import { Query, FirebaseListFactoryOpts, PathReference, QueryReference, DatabaseQuery, DatabaseReference } from '../interfaces';
9+
import { Query, FirebaseListFactoryOpts, DatabaseReference, DatabaseQuery } from '../interfaces';
1010
import { switchMap } from 'rxjs/operator/switchMap';
1111
import { map } from 'rxjs/operator/map';
1212

1313
export function FirebaseListFactory (
14-
pathRef: PathReference,
14+
ref: DatabaseReference,
1515
{ preserveSnapshot, query = {} } :FirebaseListFactoryOpts = {}): FirebaseListObservable<any> {
1616

17-
let ref: QueryReference;
18-
19-
utils.checkForUrlOrFirebaseRef(pathRef, {
20-
isUrl: () => {
21-
const path = pathRef as string;
22-
if(utils.isAbsoluteUrl(path)) {
23-
ref = firebase.database().refFromURL(path)
24-
} else {
25-
ref = firebase.database().ref(path);
26-
}
27-
},
28-
isRef: () => ref = <DatabaseReference>pathRef,
29-
isQuery: () => ref = <DatabaseQuery>pathRef,
30-
});
31-
32-
// if it's just a reference or string, create a regular list observable
33-
if ((utils.isFirebaseRef(pathRef) ||
34-
utils.isString(pathRef)) &&
35-
utils.isEmptyObject(query)) {
17+
if (utils.isEmptyObject(query)) {
3618
return firebaseListObservable(ref, { preserveSnapshot });
3719
}
3820

@@ -119,7 +101,7 @@ export function FirebaseListFactory (
119101
}
120102

121103
/**
122-
* Creates a FirebaseListObservable from a reference or query. Options can be provided as a second
104+
* Creates a FirebaseListObservable from a reference or query. Options can be provided as a second
123105
* parameter. This function understands the nuances of the Firebase SDK event ordering and other
124106
* quirks. This function takes into account that not all .on() callbacks are guaranteed to be
125107
* asynchonous. It creates a initial array from a promise of ref.once('value'), and then starts

‎src/database/firebase_object_factory.spec.ts‎

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -36,31 +36,20 @@ describe('FirebaseObjectFactory', () => {
3636

3737
describe('<constructor>', () => {
3838

39-
it('should accept a Firebase db url in the constructor', () => {
40-
const object = FirebaseObjectFactory(`questions`);
41-
expect(object instanceof FirebaseObjectObservable).toBe(true);
42-
});
43-
4439
it('should accept a Firebase db ref in the constructor', () => {
45-
const object = FirebaseObjectFactory(firebase.database().ref().child(`questions`));
40+
const object = FirebaseObjectFactory(app.database().ref().child(`questions`));
4641
expect(object instanceof FirebaseObjectObservable).toBe(true);
4742
});
4843

49-
it('should take an absolute url in the constructor', () => {
50-
const absoluteUrl = COMMON_CONFIG.databaseURL + '/questions/one';
51-
const list = FirebaseObjectFactory(absoluteUrl);
52-
expect(list instanceof FirebaseObjectObservable).toBe(true);
53-
});
54-
5544
});
5645

5746
describe('methods', () => {
5847

5948
beforeEach((done: any) => {
6049
i = Date.now();
61-
ref = firebase.database().ref().child(`questions/${i}`);
50+
ref = app.database().ref().child(`questions/${i}`);
6251
nextSpy = nextSpy = jasmine.createSpy('next');
63-
observable = FirebaseObjectFactory(`questions/${i}`);
52+
observable = FirebaseObjectFactory(app.database().ref(`questions/${i}`));
6453
ref.remove(done);
6554
});
6655

@@ -118,7 +107,7 @@ describe('FirebaseObjectFactory', () => {
118107
});
119108

120109
it('should emit snapshots if preserveSnapshot option is true', (done: any) => {
121-
observable = FirebaseObjectFactory(`questions/${i}`, { preserveSnapshot: true });
110+
observable = FirebaseObjectFactory(app.database().ref(`questions/${i}`), { preserveSnapshot: true });
122111
ref.remove(() => {
123112
ref.set('preserved snapshot!', () => {
124113
subscription = observable.subscribe(data => {
@@ -131,7 +120,7 @@ describe('FirebaseObjectFactory', () => {
131120

132121

133122
it('should call off on all events when disposed', () => {
134-
const dbRef = firebase.database().ref();
123+
const dbRef = app.database().ref();
135124
let firebaseSpy = spyOn(dbRef, 'off');
136125
subscription = FirebaseObjectFactory(dbRef).subscribe();
137126
expect(firebaseSpy).not.toHaveBeenCalled();
@@ -145,7 +134,7 @@ describe('FirebaseObjectFactory', () => {
145134
})
146135
.run(() => {
147136
// Creating a new observable so that the current zone is captured.
148-
subscription = FirebaseObjectFactory(`questions/${i}`)
137+
subscription = FirebaseObjectFactory(app.database().ref(`questions/${i}`))
149138
.filter(d => d.$value === 'in-the-zone')
150139
.subscribe(data => {
151140
expect(Zone.current.name).toBe('newZone');

0 commit comments

Comments
 (0)