From 6ae004933867ffc8bb40fbe584ee49ed3c90d61b Mon Sep 17 00:00:00 2001 From: Chris Bianca Date: Tue, 31 Oct 2017 15:32:08 +0000 Subject: [PATCH] [firestore] Correctly support dates, GeoPoints and other types in `where` clause --- .../firestore/FirestoreSerialize.java | 2 +- ...NFirebaseFirestoreCollectionReference.java | 21 +- .../RNFirebaseFirestoreCollectionReference.m | 12 +- .../RNFirebaseFirestoreDocumentReference.h | 1 + lib/modules/firestore/Query.js | 4 +- lib/modules/firestore/utils/serialize.js | 2 +- .../firestore/collectionReferenceTests.js | 222 +++++++++++++++++- tests/src/tests/firestore/index.js | 4 +- 8 files changed, 247 insertions(+), 21 deletions(-) diff --git a/android/src/main/java/io/invertase/firebase/firestore/FirestoreSerialize.java b/android/src/main/java/io/invertase/firebase/firestore/FirestoreSerialize.java index 982189b5..019853fc 100644 --- a/android/src/main/java/io/invertase/firebase/firestore/FirestoreSerialize.java +++ b/android/src/main/java/io/invertase/firebase/firestore/FirestoreSerialize.java @@ -246,7 +246,7 @@ public class FirestoreSerialize { return list; } - private static Object parseTypeMap(FirebaseFirestore firestore, ReadableMap typeMap) { + public static Object parseTypeMap(FirebaseFirestore firestore, ReadableMap typeMap) { String type = typeMap.getString("type"); if ("boolean".equals(type)) { return typeMap.getBoolean("value"); diff --git a/android/src/main/java/io/invertase/firebase/firestore/RNFirebaseFirestoreCollectionReference.java b/android/src/main/java/io/invertase/firebase/firestore/RNFirebaseFirestoreCollectionReference.java index 263cde63..d49ac6ea 100644 --- a/android/src/main/java/io/invertase/firebase/firestore/RNFirebaseFirestoreCollectionReference.java +++ b/android/src/main/java/io/invertase/firebase/firestore/RNFirebaseFirestoreCollectionReference.java @@ -14,6 +14,7 @@ import com.google.android.gms.tasks.OnCompleteListener; import com.google.android.gms.tasks.Task; import com.google.firebase.firestore.DocumentListenOptions; import com.google.firebase.firestore.EventListener; +import com.google.firebase.firestore.FirebaseFirestore; import com.google.firebase.firestore.FirebaseFirestoreException; import com.google.firebase.firestore.ListenerRegistration; import com.google.firebase.firestore.Query; @@ -115,22 +116,22 @@ public class RNFirebaseFirestoreCollectionReference { } private Query buildQuery() { - Query query = RNFirebaseFirestore.getFirestoreForApp(appName).collection(path); - query = applyFilters(query); + FirebaseFirestore firestore = RNFirebaseFirestore.getFirestoreForApp(appName); + Query query = firestore.collection(path); + query = applyFilters(firestore, query); query = applyOrders(query); query = applyOptions(query); return query; } - private Query applyFilters(Query query) { - List filtersList = Utils.recursivelyDeconstructReadableArray(filters); - - for (Object f : filtersList) { - Map filter = (Map) f; - String fieldPath = (String) filter.get("fieldPath"); - String operator = (String) filter.get("operator"); - Object value = filter.get("value"); + private Query applyFilters(FirebaseFirestore firestore, Query query) { + for (int i = 0; i < filters.size(); i++) { + ReadableMap filter = filters.getMap(i); + String fieldPath = filter.getString("fieldPath"); + String operator = filter.getString("operator"); + ReadableMap jsValue = filter.getMap("value"); + Object value = FirestoreSerialize.parseTypeMap(firestore, jsValue); switch (operator) { case "EQUAL": diff --git a/ios/RNFirebase/firestore/RNFirebaseFirestoreCollectionReference.m b/ios/RNFirebase/firestore/RNFirebaseFirestoreCollectionReference.m index 765d2f70..72102587 100644 --- a/ios/RNFirebase/firestore/RNFirebaseFirestoreCollectionReference.m +++ b/ios/RNFirebase/firestore/RNFirebaseFirestoreCollectionReference.m @@ -81,20 +81,22 @@ queryListenOptions:(NSDictionary *) queryListenOptions { } - (FIRQuery *)buildQuery { - FIRQuery *query = (FIRQuery*)[[RNFirebaseFirestore getFirestoreForApp:_app] collectionWithPath:_path]; - query = [self applyFilters:query]; + FIRFirestore *firestore = [RNFirebaseFirestore getFirestoreForApp:_app]; + FIRQuery *query = (FIRQuery*)[firestore collectionWithPath:_path]; + query = [self applyFilters:firestore query:query]; query = [self applyOrders:query]; query = [self applyOptions:query]; return query; } -- (FIRQuery *)applyFilters:(FIRQuery *) query { +- (FIRQuery *)applyFilters:(FIRFirestore *) firestore + query:(FIRQuery *) query { for (NSDictionary *filter in _filters) { NSString *fieldPath = filter[@"fieldPath"]; NSString *operator = filter[@"operator"]; - // TODO: Validate this works - id value = filter[@"value"]; + NSDictionary *jsValue = filter[@"value"]; + id value = [RNFirebaseFirestoreDocumentReference parseJSTypeMap:firestore jsTypeMap:jsValue]; if ([operator isEqualToString:@"EQUAL"]) { query = [query queryWhereField:fieldPath isEqualTo:value]; diff --git a/ios/RNFirebase/firestore/RNFirebaseFirestoreDocumentReference.h b/ios/RNFirebase/firestore/RNFirebaseFirestoreDocumentReference.h index dd729bf8..17c42846 100644 --- a/ios/RNFirebase/firestore/RNFirebaseFirestoreDocumentReference.h +++ b/ios/RNFirebase/firestore/RNFirebaseFirestoreDocumentReference.h @@ -27,6 +27,7 @@ - (BOOL)hasListeners; + (NSDictionary *)snapshotToDictionary:(FIRDocumentSnapshot *)documentSnapshot; +(NSDictionary *)parseJSMap:(FIRFirestore *) firestore jsMap:(NSDictionary *) jsMap; ++(id)parseJSTypeMap:(FIRFirestore *) firestore jsTypeMap:(NSDictionary *) jsTypeMap; @end #else diff --git a/lib/modules/firestore/Query.js b/lib/modules/firestore/Query.js index 1d3728ac..73e646be 100644 --- a/lib/modules/firestore/Query.js +++ b/lib/modules/firestore/Query.js @@ -5,6 +5,7 @@ import DocumentSnapshot from './DocumentSnapshot'; import Path from './Path'; import QuerySnapshot from './QuerySnapshot'; +import { buildTypeMap } from './utils/serialize'; import { firestoreAutoId, isFunction, isObject } from '../../utils'; const DIRECTIONS = { @@ -261,10 +262,11 @@ export default class Query { // TODO: Validation // validate.isFieldPath('fieldPath', fieldPath); // validate.isFieldFilter('fieldFilter', opStr, value); + const nativeValue = buildTypeMap(value); const newFilter = { fieldPath, operator: OPERATORS[opStr], - value, + value: nativeValue, }; const combinedFilters = this._fieldFilters.concat(newFilter); return new Query(this.firestore, this._referencePath, combinedFilters, diff --git a/lib/modules/firestore/utils/serialize.js b/lib/modules/firestore/utils/serialize.js index 5bdebf26..78762ef6 100644 --- a/lib/modules/firestore/utils/serialize.js +++ b/lib/modules/firestore/utils/serialize.js @@ -37,7 +37,7 @@ const buildNativeArray = (array: Object[]): any[] => { return nativeArray; }; -const buildTypeMap = (value: any): any => { +export const buildTypeMap = (value: any): any => { const typeMap = {}; const type = typeOf(value); if (value === null || value === undefined) { diff --git a/tests/src/tests/firestore/collectionReferenceTests.js b/tests/src/tests/firestore/collectionReferenceTests.js index a2fe0300..888359e1 100644 --- a/tests/src/tests/firestore/collectionReferenceTests.js +++ b/tests/src/tests/firestore/collectionReferenceTests.js @@ -2,9 +2,9 @@ import sinon from 'sinon'; import 'should-sinon'; import should from 'should'; -import { COL_1 } from './index'; +import { COL_1, cleanCollection } from './index'; -function collectionReferenceTests({ describe, it, context, firebase }) { +function collectionReferenceTests({ describe, it, context, firebase, before, after }) { describe('CollectionReference', () => { context('class', () => { it('should return instance methods', () => { @@ -445,6 +445,26 @@ function collectionReferenceTests({ describe, it, context, firebase }) { }); }); + it('correctly handles == date values', () => { + return firebase.native.firestore() + .collection('collection-tests') + .where('timestamp', '==', COL_1.timestamp) + .get() + .then((querySnapshot) => { + should.equal(querySnapshot.size, 1); + }); + }); + + it('correctly handles == geopoint values', () => { + return firebase.native.firestore() + .collection('collection-tests') + .where('geopoint', '==', COL_1.geopoint) + .get() + .then((querySnapshot) => { + should.equal(querySnapshot.size, 1); + }); + }); + it('correctly handles >= number values', () => { return firebase.native.firestore() .collection('collection-tests') @@ -458,6 +478,16 @@ function collectionReferenceTests({ describe, it, context, firebase }) { }); }); + it('correctly handles >= geopoint values', () => { + return firebase.native.firestore() + .collection('collection-tests') + .where('geopoint', '>=', new firebase.native.firestore.GeoPoint(-1, -1)) + .get() + .then((querySnapshot) => { + should.equal(querySnapshot.size, 1); + }); + }); + it('correctly handles <= float values', () => { return firebase.native.firestore() .collection('collection-tests') @@ -470,6 +500,194 @@ function collectionReferenceTests({ describe, it, context, firebase }) { }); }); }); + + it('correctly handles limit', async () => { + const collectionTests = firebase.native.firestore().collection('collection-tests2'); + await Promise.all([ + collectionTests.doc('col1').set(COL_1), + collectionTests.doc('col2').set({ ...COL_1, daz: 234 }), + collectionTests.doc('col3').set({ ...COL_1, daz: 234 }), + collectionTests.doc('col4').set({ ...COL_1, daz: 234 }), + collectionTests.doc('col5').set({ ...COL_1, daz: 234 }), + ]); + + return collectionTests.limit(3) + .get() + .then((querySnapshot) => { + should.equal(querySnapshot.size, 3); + return cleanCollection(collectionTests); + }); + }); + }); + + context('cursors', () => { + let collectionTests; + before(async () => { + collectionTests = firebase.native.firestore().collection('collection-tests2'); + await Promise.all([ + collectionTests.doc('col1').set({ ...COL_1, foo: 'bar0' }), + collectionTests.doc('col2').set({ ...COL_1, foo: 'bar1', daz: 234, timestamp: new Date(2017, 2, 11, 10, 0, 0) }), + collectionTests.doc('col3').set({ ...COL_1, foo: 'bar2', daz: 345, timestamp: new Date(2017, 2, 12, 10, 0, 0) }), + collectionTests.doc('col4').set({ ...COL_1, foo: 'bar3', daz: 456, timestamp: new Date(2017, 2, 13, 10, 0, 0) }), + collectionTests.doc('col5').set({ ...COL_1, foo: 'bar4', daz: 567, timestamp: new Date(2017, 2, 14, 10, 0, 0) }), + ]); + }); + + context('endAt', () => { + it('handles dates', () => { + return collectionTests.orderBy('timestamp').endAt(new Date(2017, 2, 12, 10, 0, 0)) + .get() + .then((querySnapshot) => { + should.equal(querySnapshot.size, 3); + should.deepEqual( + querySnapshot.docs.map(doc => doc.data().daz), + [123, 234, 345], + ); + }); + }); + + it('handles numbers', () => { + return collectionTests.orderBy('daz').endAt(345) + .get() + .then((querySnapshot) => { + should.equal(querySnapshot.size, 3); + should.deepEqual( + querySnapshot.docs.map(doc => doc.data().daz), + [123, 234, 345], + ); + }); + }); + + it('handles strings', () => { + return collectionTests.orderBy('foo').endAt('bar2') + .get() + .then((querySnapshot) => { + should.equal(querySnapshot.size, 3); + should.deepEqual( + querySnapshot.docs.map(doc => doc.data().daz), + [123, 234, 345], + ); + }); + }); + }); + + context('endBefore', () => { + it('handles dates', () => { + return collectionTests.orderBy('timestamp').endBefore(new Date(2017, 2, 12, 10, 0, 0)) + .get() + .then((querySnapshot) => { + should.equal(querySnapshot.size, 2); + should.deepEqual( + querySnapshot.docs.map(doc => doc.data().daz), + [123, 234], + ); + }); + }); + + it('handles numbers', () => { + return collectionTests.orderBy('daz').endBefore(345) + .get() + .then((querySnapshot) => { + should.equal(querySnapshot.size, 2); + should.deepEqual( + querySnapshot.docs.map(doc => doc.data().daz), + [123, 234], + ); + }); + }); + + it('handles strings', () => { + return collectionTests.orderBy('foo').endBefore('bar2') + .get() + .then((querySnapshot) => { + should.equal(querySnapshot.size, 2); + should.deepEqual( + querySnapshot.docs.map(doc => doc.data().daz), + [123, 234], + ); + }); + }); + }); + + context('startAt', () => { + it('handles dates', () => { + return collectionTests.orderBy('timestamp').startAt(new Date(2017, 2, 12, 10, 0, 0)) + .get() + .then((querySnapshot) => { + should.equal(querySnapshot.size, 3); + should.deepEqual( + querySnapshot.docs.map(doc => doc.data().daz), + [345, 456, 567], + ); + }); + }); + + it('handles numbers', () => { + return collectionTests.orderBy('daz').startAt(345) + .get() + .then((querySnapshot) => { + should.equal(querySnapshot.size, 3); + should.deepEqual( + querySnapshot.docs.map(doc => doc.data().daz), + [345, 456, 567], + ); + }); + }); + + it('handles strings', () => { + return collectionTests.orderBy('foo').startAt('bar2') + .get() + .then((querySnapshot) => { + should.equal(querySnapshot.size, 3); + should.deepEqual( + querySnapshot.docs.map(doc => doc.data().daz), + [345, 456, 567], + ); + }); + }); + }); + + context('startAfter', () => { + it('handles dates', () => { + return collectionTests.orderBy('timestamp').startAfter(new Date(2017, 2, 12, 10, 0, 0)) + .get() + .then((querySnapshot) => { + should.equal(querySnapshot.size, 2); + should.deepEqual( + querySnapshot.docs.map(doc => doc.data().daz), + [456, 567], + ); + }); + }); + + it('handles numbers', () => { + return collectionTests.orderBy('daz').startAfter(345) + .get() + .then((querySnapshot) => { + should.equal(querySnapshot.size, 2); + should.deepEqual( + querySnapshot.docs.map(doc => doc.data().daz), + [456, 567], + ); + }); + }); + + it('handles strings', () => { + return collectionTests.orderBy('foo').startAfter('bar2') + .get() + .then((querySnapshot) => { + should.equal(querySnapshot.size, 2); + should.deepEqual( + querySnapshot.docs.map(doc => doc.data().daz), + [456, 567], + ); + }); + }); + }); + + after(() => { + return cleanCollection(collectionTests); + }); }); }); } diff --git a/tests/src/tests/firestore/index.js b/tests/src/tests/firestore/index.js index 2801340f..502eed95 100644 --- a/tests/src/tests/firestore/index.js +++ b/tests/src/tests/firestore/index.js @@ -14,7 +14,9 @@ export const COL_1 = { daz: 123, foo: 'bar', gaz: 12.1234567, + geopoint: new firebase.native.firestore.GeoPoint(0, 0), naz: null, + timestamp: new Date(2017, 2, 10, 10, 0, 0), }; export const DOC_1 = { name: 'doc1' }; @@ -65,7 +67,7 @@ suite.addTests(firestoreTestSuite); export default suite; /* HELPER FUNCTIONS */ -async function cleanCollection(collection) { +export async function cleanCollection(collection) { const collectionTestsDocs = await collection.get(); const tasks = []; collectionTestsDocs.forEach(doc => tasks.push(doc.ref.delete()));