Merge pull request #177 from kuzzleio/topic-176-fix-search-result

Remove previous and next cache in SearchResult

Author: xbill82

src/Collection.js

@@ -472,7 +472,8 @@ Collection.prototype.search = function (filters, options, cb) {
       result.result.total,
       documents,
       result.result.aggregations ? result.result.aggregations : [],
-      {options: options, filters: filters}
+      {options: options, filters: filters},
+      options.previous || null
     ));
   });
 };
@@ -543,7 +544,8 @@ Collection.prototype.scroll = function (scrollId, options, filters, cb) {
       result.result.total,
       documents,
       result.result.aggregations ? result.result.aggregations : [],
-      {options: options, filters: filters}
+      {options: options, filters: filters},
+      options.previous || null
     ));
   });
 

src/SearchResult.js

@@ -1,11 +1,12 @@
 /**
- *
  * @param {Collection} dataCollection
  * @param {int} total
  * @param {Document[]} documents
  * @param {object} [aggregations]
  * @param {object} [searchArgs]
- * @param {KuzzleSearchResult} [previous]
+ * @param previous
+ * @property {Collection} dataCollection
+ * @property {number} fetchedDocument
  * @constructor
  */
 function KuzzleSearchResult (dataCollection, total, documents, aggregations, searchArgs, previous) {
@@ -32,31 +33,18 @@ function KuzzleSearchResult (dataCollection, total, documents, aggregations, sea
     },
     // writable properties
     fetchedDocument: {
-      value: documents.length,
+      value: previous instanceof KuzzleSearchResult ? documents.length + previous.fetchedDocument : documents.length,
       enumerable: true,
       writable: true
-    },
-    _previous: {
-      value: previous || null,
-      writable: true
-    },
-    _next: {
-      value: null,
-      writable: true
     }
   });
 
-  if (this._previous instanceof KuzzleSearchResult) {
-    this._previous._next = this;
-    this.fetchedDocument += this._previous.fetchedDocument;
-  }
-
   // promisifying
   if (this.dataCollection.kuzzle.bluebird) {
     return this.dataCollection.kuzzle.bluebird.promisifyAll(this, {
       suffix: 'Promise',
       filter: function (name, func, target, passes) {
-        var whitelist = ['previous', 'next'];
+        var whitelist = ['next'];
 
         return passes && whitelist.indexOf(name) !== -1;
       }
@@ -66,107 +54,56 @@ function KuzzleSearchResult (dataCollection, total, documents, aggregations, sea
   return this;
 }
 
-
-/**
- * @param cb
- * @returns {*}
- */
-KuzzleSearchResult.prototype.previous = function (cb) {
-  cb(null, this._previous);
-
-  return this;
-};
-
 /**
  * @param {function} cb
  */
 KuzzleSearchResult.prototype.next = function (cb) {
   var
     filters,
-    options = Object.assign({}, this.searchArgs.options),
-    self = this;
-
-  if (!this._next) {
-    // retrieve next results with scroll if original search use it
-    if (options.scrollId) {
-      if (this.fetchedDocument >= this.total) {
-        cb(null, null);
-        return;
-      }
-
-      // from and size parameters are not valid for a scroll action
-      if (typeof options.from !== 'undefined') {
-        delete options.from;
-      }
-
-      if (options.size) {
-        delete options.size;
-      }
+    options = Object.assign({}, this.searchArgs.options);
+  
+  options.previous = this;
+
+  // retrieve next results with scroll if original search use it
+  if (options.scrollId) {
+    if (this.fetchedDocument >= this.total) {
+      cb(null, null);
+      return;
+    }
 
-      this.dataCollection.scroll(
-        options.scrollId,
-        options,
-        this.searchArgs.filters || {},
-        function(error, newSearchResults) {
-          handleNextSearchResults(error, self, newSearchResults, cb);
-        }
-      );
+    // from and size parameters are not valid for a scroll action
+    if (typeof options.from !== 'undefined') {
+      delete options.from;
+    }
 
-      return;
+    if (options.size) {
+      delete options.size;
     }
-    // retrieve next results with from/size if original search use it
-    else if (options.from !== undefined && options.size !== undefined) {
-      filters = Object.assign({}, this.searchArgs.filters);
 
-      // check if we need to do next request to fetch all matching documents
-      options.from += options.size;
+    this.dataCollection.scroll(options.scrollId, options, this.searchArgs.filters || {}, cb);
 
-      if (options.from >= this.total) {
-        cb(null, null);
+    return;
+  }
 
-        return;
-      }
+  // retrieve next results with from/size if original search use it
+  if (options.from !== undefined && options.size !== undefined) {
+    filters = Object.assign({}, this.searchArgs.filters);
+
+    // check if we need to do next request to fetch all matching documents
+    options.from += options.size;
 
-      this.dataCollection.search(
-        filters,
-        options,
-        function(error, newSearchResults) {
-          handleNextSearchResults(error, self, newSearchResults, cb);
-        }
-      );
+    if (options.from >= this.total) {
+      cb(null, null);
 
       return;
     }
-  }
 
-  if (this._next instanceof KuzzleSearchResult) {
-    cb(null, this._next);
+    this.dataCollection.search(filters, options, cb);
 
     return;
   }
 
   cb(new Error('Unable to retrieve next results from search: missing scrollId or from/size params'));
 };
 
-/**
- * @param {Error} error
- * @param {KuzzleSearchResult} currentSearchResults
- * @param {KuzzleSearchResult} newSearchResults
- * @param {Function} cb
- */
-function handleNextSearchResults (error, currentSearchResults, newSearchResults, cb) {
-  if (error) {
-    cb(error);
-    return;
-  }
-
-  newSearchResults.fetchedDocument += currentSearchResults.fetchedDocument;
-
-  newSearchResults._previous = currentSearchResults;
-  currentSearchResults._next = newSearchResults;
-
-
-  cb(null, newSearchResults);
-}
-
 module.exports = KuzzleSearchResult;

test/SearchResult/constructor.test.js

@@ -35,8 +35,6 @@ describe('Document constructor', function () {
     should(searchResult.documents[0]).be.deepEqual(document);
     should(searchResult.searchArgs).be.deepEqual(searchArgs);
     should(searchResult.fetchedDocument).be.deepEqual(1);
-    should(searchResult._previous).be.exactly(null);
-    should(searchResult._next).be.exactly(null);
   });
 
   it('should expose documented properties with the right permissions', function () {
@@ -47,14 +45,11 @@ describe('Document constructor', function () {
     should(searchResult).have.propertyWithDescriptor('documents', { enumerable: true, writable: false, configurable: false });
     should(searchResult).have.propertyWithDescriptor('searchArgs', { enumerable: true, writable: false, configurable: false });
     should(searchResult).have.propertyWithDescriptor('fetchedDocument', { enumerable: true, writable: true, configurable: false });
-    should(searchResult).have.propertyWithDescriptor('_previous', { enumerable: false, writable: true, configurable: false });
-    should(searchResult).have.propertyWithDescriptor('_next', { enumerable: false, writable: true, configurable: false });
   });
 
   it('should promisify the right functions', function () {
     var searchResult = new KuzzleSearchResult(dataCollection, 2, [document], aggregations, searchArgs);
 
     should.exist(searchResult.nextPromise);
-    should.exist(searchResult.previousPromise);
   });
 });

test/SearchResult/methods.test.js

@@ -27,19 +27,6 @@ describe('KuzzleSearchResult methods', function () {
   });
 
   describe('#next', function () {
-    it('should return the next SearchResult if it has already fetched', function (done) {
-      var
-        firstSearchResult = new KuzzleSearchResult(dataCollection, 2, [firstDocument], {}, searchArgs),
-        secondSearchResult = new KuzzleSearchResult(dataCollection, 2, [secondDocument], {}, searchArgs, firstSearchResult);
-
-      firstSearchResult.next(function(error, next) {
-        should(error).be.exactly(null);
-        should(next).be.exactly(secondSearchResult);
-        done();
-      });
-
-    });
-
     it('should be able to do a scroll request', function (done) {
       var
         mockScrollResult = new KuzzleSearchResult(
@@ -63,7 +50,6 @@ describe('KuzzleSearchResult methods', function () {
         should(result).be.an.instanceOf(KuzzleSearchResult);
         should(result.documents).be.an.Array();
         should(result.documents.length).be.exactly(1);
-        should(result._previous).be.exactly(firstSearchResult);
         done();
       });
     });
@@ -162,18 +148,4 @@ describe('KuzzleSearchResult methods', function () {
       });
     });
   });
-
-  describe('#previous', function () {
-    it('should call the callback with the previous SearchResult', function (done) {
-      var
-        firstSearchResult = new KuzzleSearchResult(dataCollection, 2, [firstDocument], {}, searchArgs),
-        secondSearchResult = new KuzzleSearchResult(dataCollection, 2, [secondDocument], {}, searchArgs, firstSearchResult);
-
-      secondSearchResult.previous(function(error, previous) {
-        should(error).be.exactly(null);
-        should(previous).be.exactly(firstSearchResult);
-        done();
-      });
-    });
-  });
 });