diff options
| -rw-r--r-- | lib/chef/search/query.rb | 7 | ||||
| -rw-r--r-- | spec/functional/knife/ssh_spec.rb | 2 | ||||
| -rw-r--r-- | spec/unit/search/query_spec.rb | 31 |
3 files changed, 20 insertions, 20 deletions
diff --git a/lib/chef/search/query.rb b/lib/chef/search/query.rb index 6f494819ba..a2663d111d 100644 --- a/lib/chef/search/query.rb +++ b/lib/chef/search/query.rb @@ -71,6 +71,11 @@ class Chef args_h = args_h.reject { |k, v| k == :fuzz } end + # Set default rows parameter to 1000. This is the default in + # Chef Server, but we set it explicitly here so that we can + # confidently advance our start parameter. + args_h[:rows] ||= 1000 + response = call_rest_service(type, query: query, **args_h) if block @@ -87,7 +92,7 @@ class Chef # 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) + next_start = response["start"] + args_h[:rows] unless next_start >= response["total"] args_h[:start] = next_start search(type, query, args_h, &block) diff --git a/spec/functional/knife/ssh_spec.rb b/spec/functional/knife/ssh_spec.rb index fba344649f..9d6fd3ae10 100644 --- a/spec/functional/knife/ssh_spec.rb +++ b/spec/functional/knife/ssh_spec.rb @@ -304,7 +304,7 @@ describe Chef::Knife::Ssh do Chef::Config[:client_key] = nil Chef::Config[:chef_server_url] = "http://localhost:9000" - @api.post("/search/node?q=*:*&start=0", 200) do + @api.post("/search/node?q=*:*&start=0&rows=1000", 200) do %({"total":1, "start":0, "rows":[{"data": {"fqdn":"the.fqdn", "config": "the_public_hostname", "knife_config": "the_public_hostname" }}]}) end end diff --git a/spec/unit/search/query_spec.rb b/spec/unit/search/query_spec.rb index 95221870d5..151fcb51b3 100644 --- a/spec/unit/search/query_spec.rb +++ b/spec/unit/search/query_spec.rb @@ -22,9 +22,10 @@ require "chef/search/query" describe Chef::Search::Query do let(:rest) { double("Chef::ServerAPI") } let(:query) { Chef::Search::Query.new } + let(:default_rows) { 1000 } shared_context "filtered search" do - let(:query_string) { "search/node?q=platform:rhel&start=0" } + let(:query_string) { "search/node?q=platform:rhel&start=0&rows=#{default_rows}" } let(:server_url) { "https://api.opscode.com/organizations/opscode/nodes" } let(:args) { { filter_key => filter_hash } } let(:filter_hash) do @@ -81,8 +82,8 @@ describe Chef::Search::Query do end describe "search" do - let(:query_string) { "search/node?q=platform:rhel&start=0" } - let(:query_string_continue) { "search/node?q=platform:rhel&start=4" } + let(:query_string) { "search/node?q=platform:rhel&start=0&rows=#{default_rows}" } + let(:query_string_continue) { "search/node?q=platform:rhel&start=4&rows=#{default_rows}" } let(:query_string_with_rows) { "search/node?q=platform:rhel&start=0&rows=4" } let(:query_string_continue_with_rows) { "search/node?q=platform:rhel&start=4&rows=4" } @@ -150,12 +151,6 @@ describe Chef::Search::Query do "total" => 4, } end - let(:big_response) do - r = response.dup - r["total"] = 8 - r - end - let(:big_response_empty) do { "start" => 0, @@ -178,17 +173,17 @@ describe Chef::Search::Query do end it "queries for every object of a type by default" do - expect(rest).to receive(:get).with("search/node?q=*:*&start=0").and_return(response) + expect(rest).to receive(:get).with("search/node?q=*:*&start=0&rows=#{default_rows}").and_return(response) query.search(:node) end it "allows a custom query" do - expect(rest).to receive(:get).with("search/node?q=platform:rhel&start=0").and_return(response) + expect(rest).to receive(:get).with("search/node?q=platform:rhel&start=0&rows=#{default_rows}").and_return(response) query.search(:node, "platform:rhel") end it "lets you set a starting object" do - expect(rest).to receive(:get).with("search/node?q=platform:rhel&start=2").and_return(response) + expect(rest).to receive(:get).with("search/node?q=platform:rhel&start=2&rows=#{default_rows}").and_return(response) query.search(:node, "platform:rhel", start: 2) end @@ -221,9 +216,9 @@ describe Chef::Search::Query do query.search(:node, "*:*", start: 0, rows: 4) { |r| @call_me.do(r) } end - it "sends multiple API requests when the server indicates there is more data" do - expect(rest).to receive(:get).with(query_string).and_return(big_response) - expect(rest).to receive(:get).with(query_string_continue).and_return(big_response_end) + # This test would loop infinitely if pagination didn't advance + it "paginates correctly in the face of filtered nodes without explicit rows" do + allow(rest).to receive(:get).with(query_string).and_return(big_response_empty) query.search(:node, "platform:rhel") do |r| nil end @@ -239,21 +234,21 @@ describe Chef::Search::Query do it "fuzzifies node searches when fuzz is set" do expect(rest).to receive(:get).with( - "search/node?q=tags:*free.messi*%20OR%20roles:*free.messi*%20OR%20fqdn:*free.messi*%20OR%20addresses:*free.messi*%20OR%20policy_name:*free.messi*%20OR%20policy_group:*free.messi*&start=0" + "search/node?q=tags:*free.messi*%20OR%20roles:*free.messi*%20OR%20fqdn:*free.messi*%20OR%20addresses:*free.messi*%20OR%20policy_name:*free.messi*%20OR%20policy_group:*free.messi*&start=0&rows=#{default_rows}" ).and_return(response) query.search(:node, "free.messi", fuzz: true) end it "does not fuzzify node searches when fuzz is not set" do expect(rest).to receive(:get).with( - "search/node?q=free.messi&start=0" + "search/node?q=free.messi&start=0&rows=#{default_rows}" ).and_return(response) query.search(:node, "free.messi") end it "does not fuzzify client searches" do expect(rest).to receive(:get).with( - "search/client?q=messi&start=0" + "search/client?q=messi&start=0&rows=#{default_rows}" ).and_return(response) query.search(:client, "messi", fuzz: true) end |