Allow the best API version to be queried

Also tidy up in the face of the real world.

Signed-off-by: Thom May <thom@chef.io>

5 files changed, 30 insertions, 5 deletions

diff --git a/lib/chef/http/api_versions.rb b/lib/chef/http/api_versions.rb
index e164da262d..696c7072bf 100644
--- a/lib/chef/http/api_versions.rb
+++ b/lib/chef/http/api_versions.rb
@@ -16,6 +16,7 @@
 #
 
 require "chef/server_api_versions"
+require "chef/json_compat"
 
 class Chef
   class HTTP
@@ -32,7 +33,7 @@ class Chef
 
       def handle_response(http_response, rest_request, return_value)
         if http_response.key?("x-ops-server-api-version")
-          ServerAPIVersions.instance.set_versions(http_response["x-ops-server-api-version"])
+          ServerAPIVersions.instance.set_versions(JSONCompat.parse(http_response["x-ops-server-api-version"]))
         end
         [http_response, rest_request, return_value]
       end
diff --git a/lib/chef/mixin/versioned_api.rb b/lib/chef/mixin/versioned_api.rb
index 9c2f2f4cdb..3015345d44 100644
--- a/lib/chef/mixin/versioned_api.rb
+++ b/lib/chef/mixin/versioned_api.rb
@@ -40,11 +40,15 @@ class Chef
       end
 
       def versioned_api_class
+        get_class_for(:max_server_version)
+      end
+
+      def get_class_for(type)
         versioned_interfaces.select do |klass|
           version = klass.send(:minimum_api_version)
           # min and max versions will be nil if we've not made a request to the server yet,
           # in which case we'll just start with the highest version and see what happens
-          ServerAPIVersions.instance.min_server_version.nil? || (version >= ServerAPIVersions.instance.min_server_version && version <= ServerAPIVersions.instance.max_server_version)
+          ServerAPIVersions.instance.min_server_version.nil? || (version >= ServerAPIVersions.instance.min_server_version && version <= ServerAPIVersions.instance.send(type))
         end
           .sort { |a, b| a.send(:minimum_api_version) <=> b.send(:minimum_api_version) }
           .last
@@ -59,6 +63,13 @@ class Chef
         module_eval(str, __FILE__, line_no)
       end
 
+      # When teeing up an HTTP request, we need to be able to ask which API version we should use.
+      # Something in Net::HTTP seems to expect to strip headers, so we return this as a string.
+      def best_request_version
+        klass = get_class_for(:max_server_version)
+        klass.minimum_api_version.to_s
+      end
+
       def new(*args)
         object = versioned_api_class.allocate
         object.send(:initialize, *args)
diff --git a/lib/chef/server_api_versions.rb b/lib/chef/server_api_versions.rb
index 68dfd5ac90..2a4d0e6a5b 100644
--- a/lib/chef/server_api_versions.rb
+++ b/lib/chef/server_api_versions.rb
@@ -26,11 +26,11 @@ class Chef
     end
 
     def min_server_version
-      !@versions.nil? ? @versions["min_version"] : nil
+      !@versions.nil? ? Integer(@versions["min_version"]) : nil
     end
 
     def max_server_version
-      !@versions.nil? ? @versions["max_version"] : nil
+      !@versions.nil? ? Integer(@versions["max_version"]) : nil
     end
 
     def reset!
diff --git a/spec/unit/http/api_versions_spec.rb b/spec/unit/http/api_versions_spec.rb
index 79c97a1b69..37f1259d86 100644
--- a/spec/unit/http/api_versions_spec.rb
+++ b/spec/unit/http/api_versions_spec.rb
@@ -38,7 +38,7 @@ describe Chef::HTTP::APIVersions do
   let(:response_body) { "Thanks for checking in." }
   let(:response_headers) do
     {
-      "x-ops-server-api-version" => { "min_version" => 0, "max_version" => 2 },
+      "x-ops-server-api-version" => { "min_version" => 0, "max_version" => 2 }.to_json,
     }
   end
 
diff --git a/spec/unit/mixin/versioned_api_spec.rb b/spec/unit/mixin/versioned_api_spec.rb
index 4f2418ca24..c709871919 100644
--- a/spec/unit/mixin/versioned_api_spec.rb
+++ b/spec/unit/mixin/versioned_api_spec.rb
@@ -87,6 +87,19 @@ describe Chef::Mixin::VersionedAPIFactory do
     end
   end
 
+  describe "#best_request_version" do
+    it "returns a String" do
+      factory_class.add_versioned_api_class V2Class
+      expect(factory_class.best_request_version).to be_a(String)
+    end
+
+    it "returns the most relevant version" do
+      factory_class.add_versioned_api_class V2Class
+      factory_class.add_versioned_api_class V3Class
+      expect(factory_class.best_request_version).to eq("3")
+    end
+  end
+
   describe "#new" do
     it "creates an instance of the versioned class" do
       factory_class.add_versioned_api_class V2Class