test(GeoQuery): add tests for new limit method

@todo Remove docs that have already been added? Need to test. Maybe reduce...

README.md

@@ -80,7 +80,7 @@ Internally GeoFirestore creates multiple geohases around a requested area. It qu
 
 ### `limit()`
 
-The `limit` filtering method is exposed through GeoFirestore, however as geoqueries require an aggregation of queries, the library applies a paritial limit on the server, but primarily runs its 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.
+The `limit` filtering method is exposed through GeoFirestore, however there are some unique considerations when using it. 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.
 
 ## Contributing
 

src/GeoJoinerGet.ts

@@ -26,7 +26,7 @@ export class GeoJoinerGet {
 
     if (this._queryCriteria.limit && this._docs.size > this._queryCriteria.limit) {
       const arrayToLimit = Array.from(this._docs.values()).map((doc) => {
-        return {...doc, distance: calculateDistance(this._queryCriteria.center, doc.data().l)};
+        return { distance: calculateDistance(this._queryCriteria.center, doc.data().l), id: doc.id };
       }).sort((a, b) => a.distance - b.distance);
 
       for (let i = this._queryCriteria.limit; i < arrayToLimit.length; i++) {
@@ -41,8 +41,11 @@ export class GeoJoinerGet {
    * @return A new `GeoQuerySnapshot` of the filtered documents from the `get`.
    */
   public getGeoQuerySnapshot(): GeoQuerySnapshot {
+    const docs = Array.from(this._docs.values());
     return new GeoQuerySnapshot(
-      { docs: Array.from(this._docs.values()), docChanges: () => [] } as GeoFirestoreTypes.web.QuerySnapshot,
+      { docs, docChanges: () => docs.map((doc, index) => {
+        return { doc, newIndex: index, oldIndex: -1, type: 'added' };
+      }) } as GeoFirestoreTypes.web.QuerySnapshot,
       this._queryCriteria.center
     );
   }

src/GeoJoinerOnSnapshot.ts

@@ -56,8 +56,7 @@ export class GeoJoinerOnSnapshot {
   private _next(): void {
     // Sort docs based on distance if there is a limit so we can then limit it
     if (this._queryCriteria.limit && this._docs.size > this._queryCriteria.limit) {
-      const arrayToLimit = Array.from(this._docs.values());
-      arrayToLimit.sort((a, b) => a.distance - b.distance);
+      const arrayToLimit = Array.from(this._docs.values()).sort((a, b) => a.distance - b.distance);
       // Iterate over documents outside of limit
       for (let i = this._queryCriteria.limit; i < arrayToLimit.length; i++) {
         if (arrayToLimit[i].emitted) { // Mark as removed if outside of query and previously emitted
@@ -99,6 +98,7 @@ export class GeoJoinerOnSnapshot {
     this._firstEmitted = true;
     this._onNext(new GeoQuerySnapshot({
           docs,
+          // @TODO Remove docs that have already been added? Need to test. Maybe reduce...
           docChanges: () => docChanges
     } as GeoFirestoreTypes.web.QuerySnapshot, this._queryCriteria.center));
   }

src/GeoQuery.ts

@@ -3,7 +3,7 @@ import { GeoFirestore } from './GeoFirestore';
 import { GeoJoinerGet } from './GeoJoinerGet';
 import { GeoJoinerOnSnapshot } from './GeoJoinerOnSnapshot';
 import { GeoQuerySnapshot } from './GeoQuerySnapshot';
-import { validateQueryCriteria, geohashQueries } from './utils';
+import { validateQueryCriteria, geohashQueries, validateLimit } from './utils';
 
 /**
  * A `GeoQuery` refers to a Query which you can read or listen to. You can also construct refined `GeoQuery` objects by adding filters and
@@ -61,7 +61,8 @@ export class GeoQuery {
       if (this._center && this._radius) {
         return new GeoJoinerOnSnapshot(this._generateQuery(), this._queryCriteria, onNext, onError).unsubscribe();
       } else {
-        return (this._query as GeoFirestoreTypes.web.Query).onSnapshot((snapshot) => onNext(new GeoQuerySnapshot(snapshot)), onError);
+        const query = this._limit ? this._query.limit(this._limit) : this._query;
+        return (query as GeoFirestoreTypes.web.Query).onSnapshot((snapshot) => onNext(new GeoQuerySnapshot(snapshot)), onError);
       }
     };
   }
@@ -81,8 +82,8 @@ export class GeoQuery {
       const queries = this._generateQuery().map((query) => this._isWeb ? query.get(options) : query.get());
       return Promise.all(queries).then(value => new GeoJoinerGet(value, this._queryCriteria).getGeoQuerySnapshot());
     } else {
-      const promise = this._isWeb ?
-        (this._query as GeoFirestoreTypes.web.Query).get(options) : (this._query as GeoFirestoreTypes.web.Query).get();
+      const query = this._limit ? this._query.limit(this._limit) : this._query;
+      const promise = this._isWeb ? (query as GeoFirestoreTypes.web.Query).get(options) : (query as GeoFirestoreTypes.web.Query).get();
       return promise.then((snapshot) => new GeoQuerySnapshot(snapshot));
     }
   }
@@ -92,16 +93,17 @@ export class GeoQuery {
    *
    * This function returns a new (immutable) instance of the GeoQuery (rather than modify the existing instance) to impose the limit.
    * 
-   * Note: Geoqueries require an aggregation of queries, the library applies a paritial limit on the server,
-   * but primarily runs its 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.
+   * 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.
    *
    * @param limit The maximum number of items to return.
    * @return The created GeoQuery.
    */
   public limit(limit: number): GeoQuery {
-    this._limit = limit || this._limit;
-    return new GeoQuery(this._query.limit(limit), this._queryCriteria);
+    validateLimit(limit);
+    this._limit = limit;
+    return new GeoQuery(this._query, this._queryCriteria);
   }
 
   /**

src/utils.ts

@@ -503,6 +503,27 @@ export function validateGeohash(geohash: string, flag = false): boolean {
   }
 }
 
+/**
+ * Validates the inputted limit and throws an error, or returns boolean, if it is invalid.
+ *
+ * @param limit The limit to be applied by `GeoQuery.limit()`
+ * @param flag Tells function to send up boolean if valid instead of throwing an error.
+ */
+export function validateLimit(limit: number, flag = false): boolean {
+  let error: string;
+  if (typeof limit !== 'number' || isNaN(limit)) {
+    error = 'limit must be a number';
+  } else if (limit < 0) {
+    error = 'limit must be greater than or equal to 0';
+  }
+
+  if (typeof error !== 'undefined' && !flag) {
+    throw new Error(error);
+  } else {
+    return !error;
+  }
+}
+
 /**
  * Validates the inputted location and throws an error, or returns boolean, if it is invalid.
  *
@@ -579,11 +600,7 @@ export function validateQueryCriteria(newQueryCriteria: GeoFirestoreTypes.QueryC
 
   // Validate the 'limit' attribute
   if (typeof newQueryCriteria.limit !== 'undefined') {
-    if (typeof newQueryCriteria.limit !== 'number' || isNaN(newQueryCriteria.limit)) {
-      throw new Error('limit must be a number');
-    } else if (newQueryCriteria.limit < 0) {
-      throw new Error('limit must be greater than or equal to 0');
-    }
+    validateLimit(newQueryCriteria.limit);
   }
 }
 

test/GeoQuery.test.ts

@@ -5,7 +5,7 @@ import { GeoFirestore } from '../src/GeoFirestore';
 import { GeoQuery } from '../src/GeoQuery';
 import {
   afterEachHelper, beforeEachHelper, collection, dummyData,
-  firestore, invalidFirestores, stubDatabase, invalidLocations, geocollection
+  firestore, invalidFirestores, stubDatabase, invalidLocations, geocollection, generateDocs
 } from './common';
 
 const expect = chai.expect;
@@ -143,6 +143,77 @@ describe('GeoQuery Tests:', () => {
         });
       });
     });
+
+    it('onSnapshot docChanges() returns an array of the \'added\' docs as well as their index and `type`', (done) => {
+      const query = new GeoQuery(collection);
+      stubDatabase().then(() => {
+        const subscription = query.onSnapshot((snapshot) => {
+          subscription();
+          snapshot.docChanges().forEach((doc, index) => {
+            expect(doc.newIndex).to.be.equal(index);
+            expect(doc.oldIndex).to.be.equal(-1);
+            expect(doc.type).to.be.equal('added');
+          });
+          done();
+        });
+      });
+    });
+
+    it('onSnapshot docChanges() returns an geofiltered array of the \'added\' docs as well as their index and `type`', (done) => {
+      const query = new GeoQuery(collection);
+      const generatedData = generateDocs();
+      const radius = 100;
+      const dummyLimitedData = generatedData.reduce((limited, doc) => {
+        if (doc.distance <= radius) {
+          limited.push(doc);
+        }
+        return limited;
+      }, []) as any[];
+      stubDatabase(generatedData).then(() => {
+        const subscription = query.near({ center: new firebase.firestore.GeoPoint(0, 0), radius }).onSnapshot((snapshot) => {
+          subscription();
+          const docChanges = snapshot.docChanges();
+          const docs = docChanges.map(doc => doc.doc.data());
+          expect(docChanges.length).to.be.equal(dummyLimitedData.length);
+          expect(docs).to.have.deep.members(dummyLimitedData);
+          docChanges.forEach((doc, index) => {
+            expect(doc.newIndex).to.be.equal(index);
+            expect(doc.oldIndex).to.be.equal(-1);
+            expect(doc.type).to.be.equal('added');
+          });
+          done();
+        });
+      });
+    });
+
+    it('onSnapshot returns n amount of documents when a `limit(n)` is applied', (done) => {
+      const query = new GeoQuery(collection);
+      const n = Math.floor(Math.random() * 99) + 1;
+      const generatedData = generateDocs();
+      stubDatabase(generatedData).then(() => {
+        const subscription = query.limit(n).onSnapshot((snapshot) => {
+          subscription();
+          expect(snapshot.size).to.be.equal(n);
+          done();
+        });
+      });
+    });
+
+    it('onSnapshot returns n amount of geofiltered documents when a `limit(n)` is applied', (done) => {
+      const query = new GeoQuery(collection);
+      const n = Math.floor(Math.random() * 99) + 1;
+      const generatedData = generateDocs();
+      const dummyLimitedData = [...generatedData].sort((a, b) => a.distance - b.distance).slice(0, n).sort((a, b) => a.key - b.key);
+      stubDatabase(generatedData).then(() => {
+        const subscription = query.limit(n).near({ center: new firebase.firestore.GeoPoint(0, 0), radius: 1000 }).onSnapshot((snapshot) => {
+          subscription();
+          const result = snapshot.docs.map(d => d.data()).sort((a, b) => a.key - b.key);
+          expect(snapshot.size).to.be.equal(n);
+          expect(result).to.have.deep.members(dummyLimitedData);
+          done();
+        });
+      });
+    });
   });
 
   describe('get():', () => {
@@ -167,8 +238,7 @@ describe('GeoQuery Tests:', () => {
             { key: 'loc5', coordinates: new firebase.firestore.GeoPoint(67, 55), count: 4 },
             { key: 'loc6', coordinates: new firebase.firestore.GeoPoint(8, 8), count: 5 },
           ]);
-          done();
-        });
+        }).then(done);
       });
     });
 
@@ -181,20 +251,60 @@ describe('GeoQuery Tests:', () => {
             { key: 'loc1', coordinates: new firebase.firestore.GeoPoint(2, 3), count: 0 },
             { key: 'loc4', coordinates: new firebase.firestore.GeoPoint(5, 5), count: 3 },
           ]);
-          done();
-        });
+        }).then(done);
+      });
+    });
+
+    it('get() docChanges() returns an array of the \'added\' docs as well as their index and `type`', (done) => {
+      const query = new GeoQuery(collection);
+      stubDatabase().then(() => {
+        query.get().then((snapshot) => {
+          const docChanges = snapshot.docChanges();
+          const docs = docChanges.map(doc => doc.doc.data());
+          expect(docChanges.length).to.be.equal(dummyData.length);
+          expect(docs).to.have.deep.members(dummyData);
+          docChanges.forEach((doc, index) => {
+            expect(doc.newIndex).to.be.equal(index);
+            expect(doc.oldIndex).to.be.equal(-1);
+            expect(doc.type).to.be.equal('added');
+          });
+        }).then(done);
+      });
+    });
+
+    it('get() docChanges() returns an geofiltered array of the \'added\' docs as well as their index and `type`', (done) => {
+      const query = new GeoQuery(collection);
+      const generatedData = generateDocs();
+      const radius = 100;
+      const dummyLimitedData = generatedData.reduce((limited, doc) => {
+        if ((doc.distance <= radius)) {
+          limited.push(doc);
+        }
+        return limited;
+      }, []) as any[];
+      stubDatabase(generatedData).then(() => {
+        query.near({ center: new firebase.firestore.GeoPoint(0, 0), radius }).get().then((snapshot) => {
+          const docChanges = snapshot.docChanges();
+          const docs = docChanges.map(doc => doc.doc.data());
+          expect(docChanges.length).to.be.equal(dummyLimitedData.length);
+          expect(docs).to.have.deep.members(dummyLimitedData);
+          docChanges.forEach((doc, index) => {
+            expect(doc.newIndex).to.be.equal(index);
+            expect(doc.oldIndex).to.be.equal(-1);
+            expect(doc.type).to.be.equal('added');
+          });
+        }).then(done);
       });
     });
 
     it('get() returns dummy data, when not on web', (done) => {
       const query = new GeoQuery(collection);
       stubDatabase().then(() => {
         query['_isWeb'] = false;
-        query.get().then(data => {
-          const result = data.docs.map(d => d.data());
+        query.get().then((snapshot) => {
+          const result = snapshot.docs.map(d => d.data());
           expect(result).to.have.deep.members(dummyData);
-          done();
-        });
+        }).then(done);
       });
     });
 
@@ -209,19 +319,17 @@ describe('GeoQuery Tests:', () => {
             { key: 'loc1', coordinates: new firebase.firestore.GeoPoint(2, 3), count: 0 },
             { key: 'loc4', coordinates: new firebase.firestore.GeoPoint(5, 5), count: 3 },
           ]);
-          done();
-        });
+        }).then(done);
       });
     });
 
     it('get() returns dummy data from server (web only)', (done) => {
       const query = new GeoQuery(collection);
       stubDatabase().then(() => {
-        query.get({ source: 'server' }).then(data => {
-          const result = data.docs.map(d => d.data());
+        query.get({ source: 'server' }).then((snapshot) => {
+          const result = snapshot.docs.map(d => d.data());
           expect(result).to.have.deep.members(dummyData);
-          done();
-        });
+        }).then(done);
       });
     });
 
@@ -234,8 +342,32 @@ describe('GeoQuery Tests:', () => {
             { key: 'loc1', coordinates: new firebase.firestore.GeoPoint(2, 3), count: 0 },
             { key: 'loc4', coordinates: new firebase.firestore.GeoPoint(5, 5), count: 3 },
           ]);
-          done();
-        });
+        }).then(done);
+      });
+    });
+
+    it('get() returns n amount of documents when a `limit(n)` is applied', (done) => {
+      const query = new GeoQuery(collection);
+      const n = Math.floor(Math.random() * 99) + 1;
+      const generatedData = generateDocs();
+      stubDatabase(generatedData).then(() => {
+        query.limit(n).get().then((snapshot) => {
+          expect(snapshot.size).to.be.equal(n);
+        }).then(done);
+      });
+    });
+
+    it('get() returns n amount of geofiltered documents when a `limit(n)` is applied', (done) => {
+      const query = new GeoQuery(collection);
+      const n = Math.floor(Math.random() * 99) + 1;
+      const generatedData = generateDocs();
+      const dummyLimitedData = [...generatedData].sort((a, b) => a.distance - b.distance).slice(0, n);
+      stubDatabase(generatedData).then(() => {
+        query.limit(n).near({ center: new firebase.firestore.GeoPoint(0, 0), radius: 1000 }).get().then((snapshot) => {
+          const result = snapshot.docs.map(d => d.data());
+          expect(snapshot.size).to.be.equal(n);
+          expect(result).to.have.deep.members(dummyLimitedData);
+        }).then(done);
       });
     });
   });
@@ -288,6 +420,29 @@ describe('GeoQuery Tests:', () => {
     });
   });
 
+  describe('limit():', () => {
+    it('limit() does not throw an error with valid arguments', () => {
+      const query = new GeoQuery(collection);
+      [1, 50, 233, 0.9].forEach((limit) => {
+        expect(() => query.limit(limit)).not.to.throw();
+      });
+    });
+
+    it('limit() throws error with no arguments', () => {
+      const query = new GeoQuery(collection);
+      // @ts-ignore
+      expect(() => query.limit()).to.throw();
+    });
+
+    it('limit() throws error with invalid arguments', () => {
+      const query = new GeoQuery(collection);
+      [query, '50', null, () => {}, {}].forEach((limit) => {
+        // @ts-ignore
+        expect(() => query.limit(limit)).to.throw();
+      });
+    });
+  });
+
   describe('_stringToQuery():', () => {
     it('_stringToQuery() returns an array of two string elements', () => {
       const query = new GeoQuery(collection);