Merge pull request #5924 from coderanger/query-sort

Chef-13: Remove support for the sort option to searches.

5 files changed, 24 insertions, 32 deletions

diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index c7a541d284..6c35ae9354 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -110,7 +110,10 @@ will give you an experience consistent with `chef-solo --legacy-mode`.
 
 The deprecated code has been removed.  All providers and resources should now be using Chef >= 12.0 `provides` syntax.
 
+### Remove `sort` option for the Search API
+
+This option has been unimplemented on the server side for years, so any use of it has been pointless.
+
 ### Remove Chef::ShellOut
 
 This was deprecated and replaced a long time ago with mixlib-shellout and the shell_out mixin.
-
diff --git a/lib/chef/knife/search.rb b/lib/chef/knife/search.rb
index 046d1c7c52..7fc76b28c0 100644
--- a/lib/chef/knife/search.rb
+++ b/lib/chef/knife/search.rb
@@ -37,12 +37,6 @@ class Chef
 
       banner "knife search INDEX QUERY (options)"
 
-      option :sort,
-        :short => "-o SORT",
-        :long => "--sort SORT",
-        :description => "The order to sort the results in",
-        :default => nil
-
       option :start,
         :short => "-b ROW",
         :long => "--start ROW",
@@ -92,7 +86,6 @@ class Chef
         result_count = 0
 
         search_args = Hash.new
-        search_args[:sort] = config[:sort] if config[:sort]
         search_args[:start] = config[:start] if config[:start]
         search_args[:rows] = config[:rows] if config[:rows]
         if config[:filter_result]
diff --git a/lib/chef/search/query.rb b/lib/chef/search/query.rb
index fb77238ced..7357dbf6be 100644
--- a/lib/chef/search/query.rb
+++ b/lib/chef/search/query.rb
@@ -106,10 +106,12 @@ class Chef
         return args.first if args.first.is_a?(Hash)
 
         args_h = Hash.new
-        args_h[:sort] = args[0] if args[0]
-        args_h[:start] = args[1] if args[1]
-        args_h[:rows] = args[2]
-        args_h[:filter_result] = args[3]
+        # If we have 4 arguments, the first is the now-removed sort option, so
+        # just ignore it.
+        args.pop(0) if args.length == 4
+        args_h[:start] = args[0] if args[0]
+        args_h[:rows] = args[1]
+        args_h[:filter_result] = args[2]
         args_h
       end
 
@@ -119,16 +121,15 @@ class Chef
         s && Addressable::URI.encode_component(s.to_s, QUERY_PARAM_VALUE)
       end
 
-      def create_query_string(type, query, rows, start, sort)
+      def create_query_string(type, query, rows, start)
         qstr = "search/#{type}?q=#{escape_value(query)}"
-        qstr += "&sort=#{escape_value(sort)}" if sort
         qstr += "&start=#{escape_value(start)}" if start
         qstr += "&rows=#{escape_value(rows)}" if rows
         qstr
       end
 
-      def call_rest_service(type, query: "*:*", rows: nil, start: 0, sort: "X_CHEF_id_CHEF_X asc", filter_result: nil)
-        query_string = create_query_string(type, query, rows, start, sort)
+      def call_rest_service(type, query: "*:*", rows: nil, start: 0, filter_result: nil)
+        query_string = create_query_string(type, query, rows, start)
 
         if filter_result
           response = rest.post(query_string, filter_result)
diff --git a/spec/functional/knife/ssh_spec.rb b/spec/functional/knife/ssh_spec.rb
index dbd7b92c6c..fba344649f 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=*:*&sort=X_CHEF_id_CHEF_X%20asc&start=0", 200) do
+    @api.post("/search/node?q=*:*&start=0", 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 6df8587717..0837410b3c 100644
--- a/spec/unit/search/query_spec.rb
+++ b/spec/unit/search/query_spec.rb
@@ -24,7 +24,7 @@ describe Chef::Search::Query do
   let(:query) { Chef::Search::Query.new }
 
   shared_context "filtered search" do
-    let(:query_string) { "search/node?q=platform:rhel&sort=X_CHEF_id_CHEF_X%20asc&start=0" }
+    let(:query_string) { "search/node?q=platform:rhel&start=0" }
     let(:server_url) { "https://api.opscode.com/organizations/opscode/nodes" }
     let(:args) { { filter_key => filter_hash } }
     let(:filter_hash) do
@@ -81,10 +81,10 @@ describe Chef::Search::Query do
   end
 
   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(:query_string) { "search/node?q=platform:rhel&start=0" }
+    let(:query_string_continue) { "search/node?q=platform:rhel&start=4" }
+    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" }
 
     let(:response) do
       {
@@ -178,27 +178,22 @@ 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=*:*&sort=X_CHEF_id_CHEF_X%20asc&start=0").and_return(response)
+      expect(rest).to receive(:get).with("search/node?q=*:*&start=0").and_return(response)
       query.search(:node)
     end
 
     it "allows a custom query" do
-      expect(rest).to receive(:get).with("search/node?q=platform:rhel&sort=X_CHEF_id_CHEF_X%20asc&start=0").and_return(response)
+      expect(rest).to receive(:get).with("search/node?q=platform:rhel&start=0").and_return(response)
       query.search(:node, "platform:rhel")
     end
 
-    it "lets you set a sort order" do
-      expect(rest).to receive(:get).with("search/node?q=platform:rhel&sort=id%20desc&start=0").and_return(response)
-      query.search(:node, "platform:rhel", sort: "id desc")
-    end
-
     it "lets you set a starting object" do
-      expect(rest).to receive(:get).with("search/node?q=platform:rhel&sort=X_CHEF_id_CHEF_X%20asc&start=2").and_return(response)
+      expect(rest).to receive(:get).with("search/node?q=platform:rhel&start=2").and_return(response)
       query.search(:node, "platform:rhel", start: 2)
     end
 
     it "lets you set how many rows to return" do
-      expect(rest).to receive(:get).with("search/node?q=platform:rhel&sort=X_CHEF_id_CHEF_X%20asc&start=0&rows=40").and_return(response)
+      expect(rest).to receive(:get).with("search/node?q=platform:rhel&start=0&rows=40").and_return(response)
       query.search(:node, "platform:rhel", rows: 40)
     end
 
@@ -223,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(Chef::Node.from_hash(r)) }
-      query.search(:node, "*:*", sort: nil, start: 0, rows: 4) { |r| @call_me.do(r) }
+      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