diff options
author | Scott Hain <shain@getchef.com> | 2014-07-14 14:51:53 -0700 |
---|---|---|
committer | Claire McQuin <claire@getchef.com> | 2014-09-04 15:52:23 -0700 |
commit | 47f3b3d93cfd21831debcb4db22ddb7005f28763 (patch) | |
tree | 686982e8d50ddbbb67623cc555f3b84ebfde5ee9 | |
parent | 38e7ef5946ba911f8be407cb2d44f6d607ec131f (diff) | |
download | chef-47f3b3d93cfd21831debcb4db22ddb7005f28763.tar.gz |
Updated based on mcq and others feedback
-rw-r--r-- | lib/chef/search/query.rb | 91 | ||||
-rw-r--r-- | spec/unit/search/query_spec.rb | 361 |
2 files changed, 164 insertions, 288 deletions
diff --git a/lib/chef/search/query.rb b/lib/chef/search/query.rb index c055934709..c93c12082c 100644 --- a/lib/chef/search/query.rb +++ b/lib/chef/search/query.rb @@ -34,9 +34,18 @@ class Chef @rest = Chef::REST.new(url ||Chef::Config[:chef_server_url]) end + + # This search is only kept for backwards compatibility, since the results of the + # new filtered search method will be in a slightly different format + def partial_search(type, query='*:*', *args, &block) + results = search(type,query,args,&block) + + end + + # # New search input, designed to be backwards compatible with the old method signature - # 'type' and 'query' are the same as before, args now will accept either a Hash of + # 'type' and 'query' are the same as before, args now will accept either a Hash of # search arguments with symbols as the keys (ie :sort, :start, :rows) and a :filter_result # option. # @@ -81,50 +90,56 @@ class Chef end private - def escape(s) - s && URI.escape(s.to_s) + def escape(s) + s && URI.escape(s.to_s) + end + + # new search api that allows for a cleaner implementation of things like return filters + # (formerly known as 'partial search'). + # Also args should never be nil, but that is required for Ruby 1.8 compatibility + def do_search(type, query="*:*", args=nil, &block) + raise ArgumentError, "Type must be a string or a symbol!" unless (type.kind_of?(String) || type.kind_of?(Symbol)) + + query_string = create_query_string(type, query, args) + response = call_rest_service(query_string, args) + if !args.nil? && args.key?(:filter_result) + response_rows = response['rows'].map { |row| row['data'] } + else + response_rows = response['rows'] end - # new search api that allows for a cleaner implementation of things like return filters - # (formerly known as 'partial search'). - # Also args should never be nil, but that is required for Ruby 1.8 compatibility - def do_search(type, query="*:*", args=nil, &block) - raise ArgumentError, "Type must be a string or a symbol!" unless (type.kind_of?(String) || type.kind_of?(Symbol)) - - query_string = create_query_string(type, query, args) - response = call_rest_service(query_string, args) - unless block.nil? - response["rows"].each { |rowset| block.call(rowset) unless rowset.nil?} - unless (response["start"] + response["rows"].length) >= response["total"] - args[:start] = response["start"] + args[:rows] - do_search(type, query, args, &block) - end - true - else - [ response["rows"], response["start"], response["total"] ] + unless block.nil? + response_rows.each { |rowset| block.call(rowset) unless rowset.nil?} + unless (response["start"] + response_rows.length) >= response["total"] + args[:start] = response["start"] + args[:rows] + do_search(type, query, args, &block) end - end + true + else + [ response_rows, response["start"], response["total"] ] + end + end - # create the full rest url string - def create_query_string(type, query, args) - # create some default variables just so we don't break backwards compatibility - sort = args[:sort] - start = args[:start] - rows = args[:rows] + # create the full rest url string + def create_query_string(type, query, args) + # create some default variables just so we don't break backwards compatibility + sort = args[:sort] + start = args[:start] + rows = args[:rows] - return "search/#{type}?q=#{escape(query)}&sort=#{escape(sort)}&start=#{escape(start)}&rows=#{escape(rows)}" - end + return "search/#{type}?q=#{escape(query)}&sort=#{escape(sort)}&start=#{escape(start)}&rows=#{escape(rows)}" + end - def call_rest_service(query_string, args) - if args.key?(:filter_result) - response = @rest.post_rest(query_string, args[:filter_result]) - response_rows = response['rows'].map { |row| row['data'] } - else - response = @rest.get_rest(query_string) - response_rows = response['rows'] - end - return response + def call_rest_service(query_string, args) + if args.key?(:filter_result) + response = @rest.post_rest(query_string, args[:filter_result]) + response_rows = response['rows'].map { |row| row['data'] } + else + response = @rest.get_rest(query_string) + response_rows = response['rows'] end + return response + end end end end diff --git a/spec/unit/search/query_spec.rb b/spec/unit/search/query_spec.rb index e05c52a6a4..e7068960ab 100644 --- a/spec/unit/search/query_spec.rb +++ b/spec/unit/search/query_spec.rb @@ -20,321 +20,182 @@ require 'spec_helper' require 'chef/search/query' describe Chef::Search::Query do + let(:rest) { double("Chef::REST") } + let(:query) { Chef::Search::Query.new } + before(:each) do - @rest = double("Chef::REST") - Chef::REST.stub(:new).and_return(@rest) - @query = Chef::Search::Query.new + Chef::REST.stub(:new).and_return(rest) + rest.stub(:get_rest).and_return(response) end - describe "legacy search" do - before(:each) do - @response = { - "rows" => [ - { "name" => "my-name-is-node", - "chef_environment" => "elysium", - "platform" => "rhel", - "automatic" => { - "languages" => { - "ruby" => { - "platform" => "nudibranch", - "version" => "1.9.3", - "target" => "ming-the-merciless" - } + describe "search" do + let(:response) { { + "rows" => [ + { "name" => "my-name-is-node", + "chef_environment" => "elysium", + "platform" => "rhel", + "automatic" => { + "languages" => { + "ruby" => { + "platform" => "nudibranch", + "version" => "1.9.3", + "target" => "ming-the-merciless" } } - }, - { "name" => "my-name-is-jonas", - "chef_environment" => "hades", - "platform" => "rhel", - "automatic" => { - "languages" => { - "ruby" => { - "platform" => "i386-mingw32", - "version" => "1.9.3", - "target" => "bilbo" - } + } + }, + { "name" => "my-name-is-jonas", + "chef_environment" => "hades", + "platform" => "rhel", + "automatic" => { + "languages" => { + "ruby" => { + "platform" => "i386-mingw32", + "version" => "1.9.3", + "target" => "bilbo" } } - }, - { "name" => "my-name-is-flipper", - "chef_environment" => "elysium", - "platform" => "rhel", - "automatic" => { - "languages" => { - "ruby" => { - "platform" => "centos", - "version" => "2.0.0", - "target" => "uno" - } + } + }, + { "name" => "my-name-is-flipper", + "chef_environment" => "elysium", + "platform" => "rhel", + "automatic" => { + "languages" => { + "ruby" => { + "platform" => "centos", + "version" => "2.0.0", + "target" => "uno" } } - }, - { "name" => "my-name-is-butters", - "chef_environment" => "moon", - "platform" => "rhel", - "automatic" => { - "languages" => { - "ruby" => { - "platform" => "solaris2", - "version" => "2.1.2", - "target" => "random" - } + } + }, + { "name" => "my-name-is-butters", + "chef_environment" => "moon", + "platform" => "rhel", + "automatic" => { + "languages" => { + "ruby" => { + "platform" => "solaris2", + "version" => "2.1.2", + "target" => "random" } } - }, - ], - "start" => 0, - "total" => 4 - } - @rest.stub(:get_rest).and_return(@response) - end + } + }, + ], + "start" => 0, + "total" => 4 + } } it "should accept a type as the first argument" do - lambda { @query.search("node") }.should_not raise_error - lambda { @query.search(:node) }.should_not raise_error - lambda { @query.search(Hash.new) }.should raise_error(ArgumentError) + lambda { query.search("node") }.should_not raise_error + lambda { query.search(:node) }.should_not raise_error + lambda { query.search(Hash.new) }.should raise_error(ArgumentError) end it "should query for every object of a type by default" do - @rest.should_receive(:get_rest).with("search/node?q=*:*&sort=X_CHEF_id_CHEF_X%20asc&start=0&rows=1000").and_return(@response) - @query = Chef::Search::Query.new - @query.search(:node) + rest.should_receive(:get_rest).with("search/node?q=*:*&sort=X_CHEF_id_CHEF_X%20asc&start=0&rows=1000").and_return(response) + query.search(:node) end it "should allow a custom query" do - @rest.should_receive(:get_rest).with("search/node?q=platform:rhel&sort=X_CHEF_id_CHEF_X%20asc&start=0&rows=1000").and_return(@response) - @query = Chef::Search::Query.new - @query.search(:node, "platform:rhel") + rest.should_receive(:get_rest).with("search/node?q=platform:rhel&sort=X_CHEF_id_CHEF_X%20asc&start=0&rows=1000").and_return(response) + query.search(:node, "platform:rhel") end it "should let you set a sort order" do - @rest.should_receive(:get_rest).with("search/node?q=platform:rhel&sort=id%20desc&start=0&rows=1000").and_return(@response) - @query = Chef::Search::Query.new - @query.search(:node, "platform:rhel", "id desc") + rest.should_receive(:get_rest).with("search/node?q=platform:rhel&sort=id%20desc&start=0&rows=1000").and_return(response) + query.search(:node, "platform:rhel", "id desc") end it "should let you set a starting object" do - @rest.should_receive(:get_rest).with("search/node?q=platform:rhel&sort=id%20desc&start=2&rows=1000").and_return(@response) - @query = Chef::Search::Query.new - @query.search(:node, "platform:rhel", "id desc", 2) + rest.should_receive(:get_rest).with("search/node?q=platform:rhel&sort=id%20desc&start=2&rows=1000").and_return(response) + query.search(:node, "platform:rhel", "id desc", 2) end it "should let you set how many rows to return" do - @rest.should_receive(:get_rest).with("search/node?q=platform:rhel&sort=id%20desc&start=2&rows=40").and_return(@response) - @query = Chef::Search::Query.new - @query.search(:node, "platform:rhel", "id desc", 2, 40) + rest.should_receive(:get_rest).with("search/node?q=platform:rhel&sort=id%20desc&start=2&rows=40").and_return(response) + query.search(:node, "platform:rhel", "id desc", 2, 40) end it "should throw an exception if you pass to many options" do - lambda { @query.search(:node, "platform:rhel", "id desc", 2, 40, "wrong") }.should raise_error(ArgumentError) - end - - it "should return the raw rows, start, and total if no block is passed" do - rows, start, total = @query.search(:node) - rows.should equal(@response["rows"]) - start.should equal(@response["start"]) - total.should equal(@response["total"]) - end - - it "should call a block for each object in the response" do - @call_me = double("blocky") - @response["rows"].each { |r| @call_me.should_receive(:do).with(r) } - @query.search(:node) { |r| @call_me.do(r) } - end - - it "should page through the responses" do - @call_me = double("blocky") - @response["rows"].each { |r| @call_me.should_receive(:do).with(r) } - @query.search(:node, "*:*", nil, 0, 1) { |r| @call_me.do(r) } - end - end - - # copypasta existing functionality for new search, because new search should at the very least do the same stuff! - describe "new search" do - before(:each) do - @response = { - "rows" => [ - { "name" => "my-name-is-node", - "chef_environment" => "elysium", - "platform" => "rhel", - "automatic" => { - "languages" => { - "ruby" => { - "platform" => "nudibranch", - "version" => "1.9.3", - "target" => "ming-the-merciless" - } - } - } - }, - { "name" => "my-name-is-jonas", - "chef_environment" => "hades", - "platform" => "rhel", - "automatic" => { - "languages" => { - "ruby" => { - "platform" => "i386-mingw32", - "version" => "1.9.3", - "target" => "bilbo" - } - } - } - }, - { "name" => "my-name-is-flipper", - "chef_environment" => "elysium", - "platform" => "rhel", - "automatic" => { - "languages" => { - "ruby" => { - "platform" => "centos", - "version" => "2.0.0", - "target" => "uno" - } - } - } - }, - { "name" => "my-name-is-butters", - "chef_environment" => "moon", - "platform" => "rhel", - "automatic" => { - "languages" => { - "ruby" => { - "platform" => "solaris2", - "version" => "2.1.2", - "target" => "random" - } - } - } - }, - ], - "start" => 0, - "total" => 4 - } - @rest.stub(:get_rest).and_return(@response) - end - - it "should accept a type as the first argument" do - lambda { @query.search("node") }.should_not raise_error - lambda { @query.search(:node) }.should_not raise_error - lambda { @query.search(Hash.new) }.should raise_error(ArgumentError) - end - - it "should query for every object of a type by default" do - @rest.should_receive(:get_rest).with("search/node?q=*:*&sort=X_CHEF_id_CHEF_X%20asc&start=0&rows=1000").and_return(@response) - @query = Chef::Search::Query.new - @query.search(:node) - end - - it "should allow a custom query" do - @rest.should_receive(:get_rest).with("search/node?q=platform:rhel&sort=X_CHEF_id_CHEF_X%20asc&start=0&rows=1000").and_return(@response) - @query = Chef::Search::Query.new - @query.search(:node, "platform:rhel") - end - - it "should let you set a sort order" do - @rest.should_receive(:get_rest).with("search/node?q=platform:rhel&sort=id%20desc&start=0&rows=1000").and_return(@response) - @query = Chef::Search::Query.new - args = Hash.new - args[:sort] = "id desc" - @query.search(:node, "platform:rhel", args) - end - - it "should let you set a starting object" do - @rest.should_receive(:get_rest).with("search/node?q=platform:rhel&sort=id%20desc&start=2&rows=1000").and_return(@response) - @query = Chef::Search::Query.new - args = Hash.new - args = { - :sort => "id desc", - :start => 2 - } - @query.search(:node, "platform:rhel", args) - end - - it "should let you set how many rows to return" do - @rest.should_receive(:get_rest).with("search/node?q=platform:rhel&sort=id%20desc&start=2&rows=40").and_return(@response) - @query = Chef::Search::Query.new - args = Hash.new - args = { - :sort => "id desc", - :start => 2, - :rows => 40 - } - @query.search(:node, "platform:rhel", args) + lambda { query.search(:node, "platform:rhel", "id desc", 2, 40, "wrong") }.should raise_error(ArgumentError) end it "should return the raw rows, start, and total if no block is passed" do - rows, start, total = @query.search(:node) - rows.should equal(@response["rows"]) - start.should equal(@response["start"]) - total.should equal(@response["total"]) + rows, start, total = query.search(:node) + rows.should equal(response["rows"]) + start.should equal(response["start"]) + total.should equal(response["total"]) end it "should call a block for each object in the response" do @call_me = double("blocky") - @response["rows"].each { |r| @call_me.should_receive(:do).with(r) } - @query.search(:node) { |r| @call_me.do(r) } + response["rows"].each { |r| @call_me.should_receive(:do).with(r) } + query.search(:node) { |r| @call_me.do(r) } end it "should page through the responses" do @call_me = double("blocky") - @response["rows"].each { |r| @call_me.should_receive(:do).with(r) } - args = Hash.new - args = { - :sort => nil, - :start => 0, - :rows => 1 - } - @query.search(:node, "*:*", args) { |r| @call_me.do(r) } + response["rows"].each { |r| @call_me.should_receive(:do).with(r) } + query.search(:node, "*:*", nil, 0, 1) { |r| @call_me.do(r) } end - end - # filtered search results should only return the things asked for - describe "new search" do - before(:each) do - @response = { + context "when :filter_result is provided as a result" do + let(:server_url) { "https://api.opscode.com/organizations/opscode/nodes"} + let(:response) { { "rows" => [ - { "url" => "my-url-is-node", + { "url" => "#{server_url}/my-name-is-node", "data" => { "env" => "elysium", - "ruby_plat" => "i386-mingw32" + "ruby_plat" => "nudibranch" } }, - { "url" => "my-url-is-jonas", + { "url" => "#{server_url}/my-name-is-jonas", "data" => { "env" => "hades", "ruby_plat" => "i386-mingw32" } }, - { "url" => "my-url-is-flipper", + { "url" => "#{server_url}/my-name-is-flipper", "data" => { "env" => "elysium", "ruby_plat" => "centos" } }, - { "url" => "my-url-is-butters", + { "url" => "#{server_url}/my-name-is-butters", "data" => { "env" => "moon", - "ruby_plat" => "solaris2" + "ruby_plat" => "solaris2", } - }, + } ], "start" => 0, "total" => 4 - } - @rest.stub(:post_rest).and_return(@response) - end + } } + + it "should return only the filtered data" do + args = { + :filter_result => { + 'env' => ['chef_environment'], + 'ruby_plat' => ['languages', 'ruby', 'platform'] + } + } + + rest.should_receive(:post_rest).with("search/node?q=platform:rhel&sort=X_CHEF_id_CHEF_X%20asc&start=0&rows=1000", args[:filter_result]).and_return(response) + results, start, total = query.search(:node, "platform:rhel", args) + + results.each_with_index do |result, idx| + expected = response["rows"][idx] + + result.should have_key('env') + result['env'].should == expected['data']['env'] - it "should allow you to filter search results" do - filter_args = Hash.new - filter_args = { - :env => [ "chef_environment" ], - :ruby_plat => [ "languages", "ruby", "platform" ] - } - @rest.should_receive(:post_rest).with("search/node?q=platform:rhel&sort=X_CHEF_id_CHEF_X%20asc&start=0&rows=1000", filter_args).and_return(@response) - @query = Chef::Search::Query.new - args = Hash.new - args[:filter_result] = filter_args - @query.search(:node, "platform:rhel", args) + result.should have_key('ruby_plat') + result['ruby_plat'].should == expected['data']['ruby_plat'] + end + end end end -end +end
\ No newline at end of file |