diff options
-rw-r--r-- | lib/chef/search/query.rb | 18 | ||||
-rw-r--r-- | spec/unit/search/query_spec.rb | 20 |
2 files changed, 35 insertions, 3 deletions
diff --git a/lib/chef/search/query.rb b/lib/chef/search/query.rb index 6469a18c49..658af8779c 100644 --- a/lib/chef/search/query.rb +++ b/lib/chef/search/query.rb @@ -88,8 +88,21 @@ WARNDEP if block response["rows"].each { |row| block.call(row) if row } - unless (response["start"] + response["rows"].length) >= response["total"] - args_h[:start] = response["start"] + response["rows"].length + # + # args_h[:rows] and args_h[:start] are the page size and + # start position requested of the search index backing the + # search API. + # + # The response may contain fewer rows than arg_h[:rows] if + # the page of index results included deleted nodes which + # have been filtered from the returned data. In this case, + # we still want to start the next page at start + + # args_h[:rows] to avoid asking the search backend for + # overlapping pages (which could result in duplicates). + # + next_start = response["start"] + (args_h[:rows] || response["rows"].length) + unless next_start >= response["total"] + args_h[:start] = next_start search(type, query, args_h, &block) end true @@ -99,6 +112,7 @@ WARNDEP end private + def validate_type(t) unless t.kind_of?(String) || t.kind_of?(Symbol) msg = "Invalid search object type #{t.inspect} (#{t.class}), must be a String or Symbol." + 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 } |