From 5846eb62eddd1ca5ac54a1473f3d12805b2c7e97 Mon Sep 17 00:00:00 2001 From: Scott Hain Date: Thu, 26 Jun 2014 10:24:14 -0700 Subject: Updated based on awesome feedback --- lib/chef/knife/search.rb | 21 +++++--- lib/chef/search/query.rb | 113 ++++++++++++++++++++++++----------------- spec/unit/search/query_spec.rb | 30 +++++++---- 3 files changed, 97 insertions(+), 67 deletions(-) diff --git a/lib/chef/knife/search.rb b/lib/chef/knife/search.rb index 1bb182a61b..cf2ca7b683 100644 --- a/lib/chef/knife/search.rb +++ b/lib/chef/knife/search.rb @@ -92,8 +92,6 @@ class Chef result_items = [] result_count = 0 - # rows = config[:rows] - # start = config[:start] search_args = Hash.new search_args[:sort] = config[:sort] search_args[:start] = config[:start] @@ -163,17 +161,24 @@ class Chef end end + # This method turns a set of key value pairs in a string into the appropriate data structure that the + # chef-server search api is expecting. + # expected input is in the form of: + # -f "return_var1=path.to.attribute, return_var2=shorter.path" + # + # a more concrete example might be: + # -f "env=chef_environment, ruby_platform=languages.ruby.platform" + # + # The end result is a hash where the key is a symbol in the hash (the return variable) + # and the path is an array with the path elements as strings (in order) + # See lib/chef/search/query.rb for more examples of this. def create_result_filter(filter_string) final_filter = Hash.new - - # this probably needs review. I betcha there's a better way!!! filter_string.gsub!(" ", "") filters = filter_string.split(",") filters.each do |f| - fsplit = f.split("=") - return_id = fsplit[0] - attr_path = fsplit[1].split(".") - final_filter[return_id.to_sym] = attr_path + return_id, attr_path = f.split("=") + final_filter[return_id.to_sym] = attr_path.split(".") end return final_filter end diff --git a/lib/chef/search/query.rb b/lib/chef/search/query.rb index bf3518e695..de64d64ed0 100644 --- a/lib/chef/search/query.rb +++ b/lib/chef/search/query.rb @@ -34,40 +34,35 @@ class Chef @rest = Chef::REST.new(url ||Chef::Config[:chef_server_url]) end - # new search api that allows for a cleaner implementation of things like return filters - # (formerly known as 'partial search'). A passthrough to either the old style ("full search") - # or the new 'filtered' search - def search_new(type, query="*:*", args=nil, &block) - raise ArgumentError, "Type must be a string or a symbol!" unless (type.kind_of?(String) || type.kind_of?(Symbol)) - - # if args is nil, we need to set some defaults, for backwards compatibility - if args.nil? - args = Hash.new - args[:sort] = 'X_CHEF_id_CHEF_X asc' - args[:start] = 0 - args[:rows] = 1000 - end - - query_string = create_query_string(type, query, args) - response = call_rest_service(query_string, args) - if block - response["rows"].each { |o| block.call(o) unless o.nil?} - unless (response["start"] + response["rows"].length) >= response["total"] - args[:start] = response["start"] + args[:rows] - search_new(type, query, args, &block) - end - true - else - [ response["rows"], response["start"], response["total"] ] - end - - 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 + # search arguments with symbols as the keys (ie :sort, :start, :rows) and a :filter_result + # option. + # + # :filter_result should be in the format of another Hash with the structure of: + # { + # :returned_name1 => ["path", "to", "variable"], + # :returned_name2 => ["shorter", "path"] + # } + # a real world example might be something like: + # { + # :ip_address => ["ipaddress"], + # :ruby_version => ["languages", "ruby", "version"] + # } + # this will bring back 2 variables 'ip_address' and 'ruby_version' with whatever value was found + # an example of the returned json may be: + # {"ip_address":"127.0.0.1", "ruby_version": "1.9.3"} + # def search(type, query='*:*', *args, &block) raise ArgumentError, "Type must be a string or a symbol!" unless (type.kind_of?(String) || type.kind_of?(Symbol)) raise ArgumentError, "Invalid number of arguments!" if (args.size > 3) if args.size == 1 && args[0].is_a?(Hash) - args_hash = args[0] + args_hash = args[0] + # just in case the hash doesn't have the correct defaults, we'll set them + args_hash[:sort] = args_hash.key?(:sort) ? args_hash[:sort] : 'X_CHEF_id_CHEF_X asc' + args_hash[:start] = args_hash.key?(:start) ? args_hash[:start] : 0 + args_hash[:rows] = args_hash.key?(:rows) ? args_hash[:rows] : 1000 search_new(type, query, args_hash, &block) else sort = args.length >= 1 ? args[0] : 'X_CHEF_id_CHEF_X asc' @@ -77,21 +72,6 @@ class Chef end end - - # Search Solr for objects of a given type, for a given query. If you give - # it a block, it will handle the paging for you dynamically. - def search_old(type, query="*:*", sort='X_CHEF_id_CHEF_X asc', start=0, rows=1000, &block) - raise ArgumentError, "Type must be a string or a symbol!" unless (type.kind_of?(String) || type.kind_of?(Symbol)) - - # argify things - args = Hash.new - args[:sort] = sort - args[:start] = start - args[:rows] = rows - - search_new(type, query, args, &block) - end - def list_indexes @rest.get_rest("search") end @@ -100,13 +80,50 @@ class Chef 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'). A passthrough to either the old style ("full search") + # or the new 'filtered' search + def search_new(type, query="*:*", args=nil, &block) + raise ArgumentError, "Type must be a string or a symbol!" unless (type.kind_of?(String) || type.kind_of?(Symbol)) + + # if args is nil, we need to set some defaults, for backwards compatibility + if args.nil? + args = Hash.new + args = args || { :sort => 'X_CHEF_id_CHEF_X asc', :start => 0, :rows => 1000 } + end + + query_string = create_query_string(type, query, args) + response = call_rest_service(query_string, args) + if block + response["rows"].each { |o| block.call(o) unless o.nil?} + unless (response["start"] + response["rows"].length) >= response["total"] + args[:start] = response["start"] + args[:rows] + search_new(type, query, args, &block) + end + true + else + [ response["rows"], response["start"], response["total"] ] + end + end + # Search Solr for objects of a given type, for a given query. If you give + # it a block, it will handle the paging for you dynamically. + def search_old(type, query="*:*", sort='X_CHEF_id_CHEF_X asc', start=0, rows=1000, &block) + raise ArgumentError, "Type must be a string or a symbol!" unless (type.kind_of?(String) || type.kind_of?(Symbol)) + + # argify things + args = Hash.new + args = { :sort => sort, :start => start, :rows => rows } + search_new(type, query, args, &block) + 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.key?(:sort) ? args[:sort] : 'X_CHEF_id_CHEF_X asc' - start = args.key?(:start) ? args[:start] : 0 - rows = args.key?(:rows) ? args[:rows] : 1000 + 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 diff --git a/spec/unit/search/query_spec.rb b/spec/unit/search/query_spec.rb index 714806ce29..8ab653b6f7 100644 --- a/spec/unit/search/query_spec.rb +++ b/spec/unit/search/query_spec.rb @@ -145,7 +145,7 @@ describe Chef::Search::Query do end end - # copypasta existing functionality for new searhc, because new search should at the very least do the same stuff! + # 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 = { @@ -239,8 +239,10 @@ describe Chef::Search::Query 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" - args[:start] = 2 + args = { + :sort => "id desc", + :start => 2 + } @query.search(:node, "platform:rhel", args) end @@ -248,9 +250,11 @@ describe Chef::Search::Query 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" - args[:start] = 2 - args[:rows] = 40 + args = { + :sort => "id desc", + :start => 2, + :rows => 40 + } @query.search(:node, "platform:rhel", args) end @@ -271,9 +275,11 @@ describe Chef::Search::Query do @call_me = double("blocky") @response["rows"].each { |r| @call_me.should_receive(:do).with(r) } args = Hash.new - args[:sort] = nil - args[:start] = 0 - args[:rows] = 1 + args = { + :sort => nil, + :start => 0, + :rows => 1 + } @query.search(:node, "*:*", args) { |r| @call_me.do(r) } end end @@ -316,8 +322,10 @@ describe Chef::Search::Query do it "should allow you to filter search results" do filter_args = Hash.new - filter_args[:env] = ["chef_environment"] - filter_args[:ruby_plat] = ["languages", "ruby", "platform"] + 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 -- cgit v1.2.1