Return filtered results when :filtered_result is present.partial_search

2 files changed, 148 insertions, 291 deletions

diff --git a/lib/chef/search/query.rb b/lib/chef/search/query.rb
index c055934709..9be4b07062 100644
--- a/lib/chef/search/query.rb
+++ b/lib/chef/search/query.rb
@@ -36,7 +36,7 @@ class Chef
 
       #
       # 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.
       #
@@ -44,7 +44,7 @@ class Chef
       # {
       #   :returned_name1 => ["path", "to", "variable"],
       #   :returned_name2 => ["shorter", "path"]
-      # } 
+      # }
       # a real world example might be something like:
       # {
       #   :ip_address => ["ipaddress"],
@@ -53,7 +53,7 @@ class Chef
       #  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)
@@ -81,50 +81,45 @@ class Chef
       end
 
       private
-        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)
-          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"] ]
-          end
-        end 
+      def escape(s)
+        s && URI.escape(s.to_s)
+      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]
+      # 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))
 
-          return "search/#{type}?q=#{escape(query)}&sort=#{escape(sort)}&start=#{escape(start)}&rows=#{escape(rows)}"
+        query_string = create_query_string(type, query, args)
+        if !args.nil? && 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
-
-        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']
+        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
-          return response
+          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]
+
+        return "search/#{type}?q=#{escape(query)}&sort=#{escape(sort)}&start=#{escape(start)}&rows=#{escape(rows)}"
+      end
     end
   end
 end
diff --git a/spec/unit/search/query_spec.rb b/spec/unit/search/query_spec.rb
index e05c52a6a4..a1ec5a1dfb 100644
--- a/spec/unit/search/query_spec.rb
+++ b/spec/unit/search/query_spec.rb
@@ -20,321 +20,183 @@ 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)
+      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) }
-      @query.search(:node, "*:*", nil, 0, 1) { |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
 
-  # 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 = {
+    context "when :filter_result is provided as a result" do
+      let(:server_url) { "https://api.opscode.com/organizations/opscode/nodes"}
+      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-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)
-    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) }
-      args = Hash.new
-      args = {
-        :sort => nil,
-        :start => 0,
-        :rows => 1
-      }
-      @query.search(:node, "*:*", args) { |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 = {
-        "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