Fix search result paginationssd/search-pagination-fix

The start and rows parameter that are passed as part of the search
request are passed directly to Solr on the backend. Results from Solr
may contain deleted nodes no longer in the erchef database.  Erchef will
filter such nodes from the results. Thus, a user may receive fewer rows
than they asked for. Incrementing 'start' only by the number of rows
received will then result in the next Solr response overlapping the
first, which can lead to duplicate results. In the case of a Solr
results page that was completely filtered, it would lead to an infinite
loop.

This commit changes the code to always increment by the requested page
size (args[:rows]) when it is non-nil. Incrementing by the length of the
response set is still wrong in the case when the args[:rows] is nil, but
the server doesn't give us anything else to increment by.

Fixes #4027

1 files changed, 19 insertions, 1 deletions

diff --git a/spec/unit/search/query_spec.rb b/spec/unit/search/query_spec.rb
index 59ac80f228..f85b1760d4 100644
--- a/spec/unit/search/query_spec.rb
+++ b/spec/unit/search/query_spec.rb
@@ -83,6 +83,8 @@ describe Chef::Search::Query do
   describe "search" do
     let(:query_string) { "search/node?q=platform:rhel&sort=X_CHEF_id_CHEF_X%20asc&start=0" }
     let(:query_string_continue) { "search/node?q=platform:rhel&sort=X_CHEF_id_CHEF_X%20asc&start=4" }
+    let(:query_string_with_rows) { "search/node?q=platform:rhel&sort=X_CHEF_id_CHEF_X%20asc&start=0&rows=4" }
+    let(:query_string_continue_with_rows) { "search/node?q=platform:rhel&sort=X_CHEF_id_CHEF_X%20asc&start=4&rows=4" }
 
     let(:response) { {
       "rows" => [
@@ -149,6 +151,14 @@ describe Chef::Search::Query do
       r
     }
 
+    let(:big_response_empty) {
+      {
+        "start" => 0,
+        "total" => 8,
+        "rows" => []
+      }
+    }
+
     let(:big_response_end) {
       r = response.dup
       r["start"] = 4
@@ -208,7 +218,7 @@ describe Chef::Search::Query do
     it "pages through the responses" do
       @call_me = double("blocky")
       response["rows"].each { |r| expect(@call_me).to receive(:do).with(r) }
-      query.search(:node, "*:*", sort: nil, start: 0, rows: 1) { |r| @call_me.do(r) }
+      query.search(:node, "*:*", sort: nil, start: 0, rows: 4) { |r| @call_me.do(r) }
     end
 
     it "sends multiple API requests when the server indicates there is more data" do
@@ -219,6 +229,14 @@ describe Chef::Search::Query do
       end
     end
 
+    it "paginates correctly in the face of filtered nodes" do
+      expect(rest).to receive(:get_rest).with(query_string_with_rows).and_return(big_response_empty)
+      expect(rest).to receive(:get_rest).with(query_string_continue_with_rows).and_return(big_response_end)
+      query.search(:node, "platform:rhel", rows: 4) do |r|
+        nil
+      end
+    end
+
     context "when :filter_result is provided as a result" do
       include_context "filtered search" do
         let(:filter_key) { :filter_result }