From ffd996a9982f8870f1abaafd1e8ac7e6875a2e4e Mon Sep 17 00:00:00 2001 From: Daan Date: Fri, 18 Jan 2019 09:58:14 +0100 Subject: [PATCH 1/2] Suggested fix(inequality filter) #78 --- src/GeoQuery.ts | 9 ++++++++- test/GeoQuery.test.ts | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/GeoQuery.ts b/src/GeoQuery.ts index 9abd51b..2cc627d 100644 --- a/src/GeoQuery.ts +++ b/src/GeoQuery.ts @@ -13,6 +13,7 @@ export class GeoQuery { private _center: GeoFirestoreTypes.cloud.GeoPoint | GeoFirestoreTypes.web.GeoPoint; private _radius: number; private _isWeb: boolean; + private _ineq = false; /** * @param _query The `Query` instance. @@ -114,6 +115,9 @@ export class GeoQuery { opStr: GeoFirestoreTypes.WhereFilterOp, value: any ): GeoQuery { + // True if inequality filter is used + this._ineq = (opStr !== '==' && opStr !== 'array-contains') ? true : this._ineq; + // Return GeoQuery return new GeoQuery(this._query.where((fieldPath ? ('d.' + fieldPath) : fieldPath), opStr, value), this._near); } @@ -132,7 +136,10 @@ export class GeoQuery { // decode the geohash query string const query: string[] = this._stringToQuery(toQueryStr); // Create the Firebase query - return this._query.where('g', '>=', query[0]).where('g', '<=', query[1]) as GeoFirestoreTypes.web.Query; + if(!this._ineq){ + return this._query.where('g', '>=', query[0]).where('g', '<=', query[1]) as GeoFirestoreTypes.web.Query; + } + return this._query.orderBy('g').startAt(query[0]).endAt(query[1]) as GeoFirestoreTypes.web.Query; }); } diff --git a/test/GeoQuery.test.ts b/test/GeoQuery.test.ts index a4e0a2f..e949a82 100644 --- a/test/GeoQuery.test.ts +++ b/test/GeoQuery.test.ts @@ -288,6 +288,22 @@ describe('GeoQuery Tests:', () => { }); }); + + describe('near().where():', () => { + it('near().where() does not throw an error with valid arguments', () => { + const query = new GeoQuery(collection); + expect(() => query.near({ center: new firebase.firestore.GeoPoint(0, 0), radius: 100 }) + .where('count', '==', 0)).not.to.throw(); + expect(() => query.near({ center: new firebase.firestore.GeoPoint(1, 1) }) + .where('count', '>', 0)).not.to.throw(); + expect(() => query.near({ radius: 500 }) + .where('count', '<=', 0)).not.to.throw(); + expect(() => query.near({ radius: 500 }) + .where('array', 'array-contains', 'one')).not.to.throw(); + }); + }); + + describe('_stringToQuery():', () => { it('_stringToQuery() returns an array of two string elements', () => { const query = new GeoQuery(collection); From b1261b6199c63065f245929488bbd70c8e29237a Mon Sep 17 00:00:00 2001 From: Daan Date: Fri, 18 Jan 2019 09:58:14 +0100 Subject: [PATCH 2/2] Suggested fix(inequality filter) #78 --- src/GeoQuery.ts | 11 +++++++++-- test/GeoQuery.test.ts | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/GeoQuery.ts b/src/GeoQuery.ts index d0d0469..b9877c2 100644 --- a/src/GeoQuery.ts +++ b/src/GeoQuery.ts @@ -14,6 +14,7 @@ export class GeoQuery { private _limit: number; private _radius: number; private _isWeb: boolean; + private _ineq = false; /** * @param _query The `Query` instance. @@ -92,7 +93,7 @@ export class GeoQuery { * Creates and returns a new GeoQuery that's additionally limited to only return up to the specified number of documents. * * This function returns a new (immutable) instance of the GeoQuery (rather than modify the existing instance) to impose the limit. - * + * * Note: Limits on geoqueries are applied based on the distance from the center. Geoqueries require an aggregation of queries. * When performing a geoquery the library applies the limit on the client. This may mean you are loading to the client more documents * then you intended. Use with this performance limitation in mind. @@ -139,6 +140,9 @@ export class GeoQuery { opStr: GeoFirestoreTypes.WhereFilterOp, value: any ): GeoQuery { + // True if inequality filter is used + this._ineq = (opStr !== '==' && opStr !== 'array-contains') ? true : this._ineq; + // Return GeoQuery return new GeoQuery(this._query.where((fieldPath ? ('d.' + fieldPath) : fieldPath), opStr, value), this._queryCriteria); } @@ -157,7 +161,10 @@ export class GeoQuery { // decode the geohash query string const query: string[] = this._stringToQuery(toQueryStr); // Create the Firebase query - return this._query.where('g', '>=', query[0]).where('g', '<=', query[1]) as GeoFirestoreTypes.web.Query; + if(!this._ineq){ + return this._query.where('g', '>=', query[0]).where('g', '<=', query[1]) as GeoFirestoreTypes.web.Query; + } + return this._query.orderBy('g').startAt(query[0]).endAt(query[1]) as GeoFirestoreTypes.web.Query; }); } diff --git a/test/GeoQuery.test.ts b/test/GeoQuery.test.ts index 16f04cb..d3ba474 100644 --- a/test/GeoQuery.test.ts +++ b/test/GeoQuery.test.ts @@ -443,6 +443,21 @@ describe('GeoQuery Tests:', () => { }); }); + describe('near().where():', () => { + it('near().where() does not throw an error with valid arguments', () => { + const query = new GeoQuery(collection); + expect(() => query.near({ center: new firebase.firestore.GeoPoint(0, 0), radius: 100 }) + .where('count', '==', 0)).not.to.throw(); + expect(() => query.near({ center: new firebase.firestore.GeoPoint(1, 1) }) + .where('count', '>', 0)).not.to.throw(); + expect(() => query.near({ radius: 500 }) + .where('count', '<=', 0)).not.to.throw(); + expect(() => query.near({ radius: 500 }) + .where('array', 'array-contains', 'one')).not.to.throw(); + }); + }); + + describe('_stringToQuery():', () => { it('_stringToQuery() returns an array of two string elements', () => { const query = new GeoQuery(collection);