Remove all the stupid json magic from Chef::ApiClientV1.

6 files changed, 44 insertions, 111 deletions

diff --git a/lib/chef/api_client_v1.rb b/lib/chef/api_client_v1.rb
index ac8918184d..80f0d2517c 100644
--- a/lib/chef/api_client_v1.rb
+++ b/lib/chef/api_client_v1.rb
@@ -57,21 +57,15 @@ class Chef
     end
 
     def chef_rest_v0
-      @chef_rest_v0 ||= Chef::ServerAPI.new(Chef::Config[:chef_server_url], {:api_version => "0"})
+      @chef_rest_v0 ||= Chef::ServerAPI.new(Chef::Config[:chef_server_url], {:api_version => "0", :inflate_json_class => false})
     end
 
     def chef_rest_v1
-      @chef_rest_v1 ||= Chef::ServerAPI.new(Chef::Config[:chef_server_url], {:api_version => "1"})
+      @chef_rest_v1 ||= Chef::ServerAPI.new(Chef::Config[:chef_server_url], {:api_version => "1", :inflate_json_class => false})
     end
 
-    # will default to the current version (Chef::Authenticator::DEFAULT_SERVER_API_VERSION)
-    def http_api
-      @http_api ||= Chef::REST.new(Chef::Config[:chef_server_url])
-    end
-
-    # will default to the current version (Chef::Authenticator::DEFAULT_SERVER_API_VERSION)
     def self.http_api
-      Chef::REST.new(Chef::Config[:chef_server_url])
+      Chef::ServerAPI.new(Chef::Config[:chef_server_url], {:api_version => "1", :inflate_json_class => false})
     end
 
     # Gets or sets the client name.
@@ -157,7 +151,6 @@ class Chef
         "name" => @name,
         "validator" => @validator,
         "admin" => @admin,
-        'json_class' => self.class.name,
         "chef_type" => "client"
       }
       result["private_key"] = @private_key unless @private_key.nil?
@@ -184,16 +177,12 @@ class Chef
       client
     end
 
-    def self.json_create(data)
-      from_hash(data)
-    end
-
     def self.from_json(j)
-      from_hash(Chef::JSONCompat.parse(j))
+      Chef::ApiClientV1.from_hash(Chef::JSONCompat.from_json(j))
     end
 
     def self.reregister(name)
-      api_client = load(name)
+      api_client = Chef::ApiClientV1.load(name)
       api_client.reregister
     end
 
@@ -201,7 +190,7 @@ class Chef
       if inflate
         response = Hash.new
         Chef::Search::Query.new.search(:client) do |n|
-          n = self.json_create(n) if n.instance_of?(Hash)
+          n = self.from_hash(n) if n.instance_of?(Hash)
           response[n.name] = n
         end
         response
@@ -213,20 +202,12 @@ class Chef
     # Load a client by name via the API
     def self.load(name)
       response = http_api.get("clients/#{name}")
-      if response.kind_of?(Chef::ApiClientV1)
-        response
-      # stupid automated object generation.
-      # just give me the json :(
-      elsif response.kind_of?(Chef::ApiClient)
-        json_create(response.to_json)
-      else
-        json_create(response)
-      end
+      Chef::ApiClientV1.from_hash(response)
     end
 
     # Remove this client via the REST API
     def destroy
-      http_api.delete("clients/#{@name}")
+      chef_rest_v1.delete("clients/#{@name}")
     end
 
     # Save this client via the REST API, returns a hash including the private key
@@ -291,7 +272,7 @@ class Chef
         new_client = chef_rest_v0.put("clients/#{name}", payload)
       end
 
-      new_client
+      Chef::ApiClientV1.from_hash(new_client)
     end
 
     # Create the client via the REST API
diff --git a/lib/chef/json_compat.rb b/lib/chef/json_compat.rb
index e8053d12f1..d0b3b4c7f8 100644
--- a/lib/chef/json_compat.rb
+++ b/lib/chef/json_compat.rb
@@ -29,7 +29,6 @@ class Chef
     JSON_CLASS = "json_class".freeze
 
     CHEF_APICLIENT          = "Chef::ApiClient".freeze
-    CHEF_APICLIENTV1        = "Chef::ApiClientV1".freeze
     CHEF_CHECKSUM           = "Chef::Checksum".freeze
     CHEF_COOKBOOKVERSION    = "Chef::CookbookVersion".freeze
     CHEF_DATABAG            = "Chef::DataBag".freeze
@@ -125,8 +124,6 @@ class Chef
         case json_class
         when CHEF_APICLIENT
           Chef::ApiClient
-        when CHEF_APICLIENTV1
-          Chef::ApiClientV1
         when CHEF_CHECKSUM
           Chef::Checksum
         when CHEF_COOKBOOKVERSION
diff --git a/lib/chef/knife/client_create.rb b/lib/chef/knife/client_create.rb
index 951457db28..fa9a1a7e32 100644
--- a/lib/chef/knife/client_create.rb
+++ b/lib/chef/knife/client_create.rb
@@ -62,7 +62,7 @@ class Chef
 
       def create_client(client)
         # should not be using save :( bad behavior
-        client.save
+        Chef::ApiClientV1.from_hash(client).save
       end
 
       def run
@@ -93,7 +93,7 @@ class Chef
 
         output = edit_data(client)
         final_client = create_client(output)
-        ui.info("Created #{output}")
+        ui.info("Created #{final_client}")
 
         # output private_key if one
         if final_client.private_key
diff --git a/lib/chef/knife/client_edit.rb b/lib/chef/knife/client_edit.rb
index a243129979..5dcd8f212b 100644
--- a/lib/chef/knife/client_edit.rb
+++ b/lib/chef/knife/client_edit.rb
@@ -38,7 +38,15 @@ class Chef
           exit 1
         end
 
-        edit_object(Chef::ApiClientV1, @client_name)
+        original_data = Chef::ApiClientV1.load(@client_name).to_hash
+        edited_client = edit_data(original_data)
+        if original_data != edited_client
+          client = Chef::ApiClientV1.from_hash(edited_client)
+          client.save
+          ui.msg("Saved #{client}.")
+        else
+          ui.msg("Client unchanged, not saving.")
+        end
       end
     end
   end
diff --git a/spec/unit/api_client_v1_spec.rb b/spec/unit/api_client_v1_spec.rb
index 4227185742..17aba8c3af 100644
--- a/spec/unit/api_client_v1_spec.rb
+++ b/spec/unit/api_client_v1_spec.rb
@@ -191,7 +191,7 @@ describe Chef::ApiClientV1 do
     end
   end
 
-  describe "when deserializing from JSON (hash) using JSONCompat#from_json" do
+  describe "when deserializing from JSON (hash) using ApiClientV1#from_json" do
     let(:client_hash) do
       {
         "name" => "black",
@@ -199,13 +199,12 @@ describe Chef::ApiClientV1 do
         "private_key" => "monkeypants",
         "admin" => true,
         "validator" => true,
-        "create_key" => true,
-        "json_class" => "Chef::ApiClientV1"
+        "create_key" => true
       }
     end
 
     let(:client) do
-      Chef::JSONCompat.from_json(Chef::JSONCompat.to_json(client_hash))
+      Chef::ApiClientV1.from_json(Chef::JSONCompat.to_json(client_hash))
     end
 
     it "should deserialize to a Chef::ApiClientV1 object" do
@@ -249,12 +248,11 @@ describe Chef::ApiClientV1 do
         "private_key" => "monkeypants",
         "admin" => true,
         "create_key" => true,
-        "validator" => true,
-        "json_class" => "Chef::ApiClientV1"
+        "validator" => true
       }
 
-      @http_client = double("Chef::REST mock")
-      allow(Chef::REST).to receive(:new).and_return(@http_client)
+      @http_client = double("Chef::ServerAPI mock")
+      allow(Chef::ServerAPI).to receive(:new).and_return(@http_client)
       expect(@http_client).to receive(:get).with("clients/black").and_return(client)
       @client = Chef::ApiClientV1.load(client['name'])
     end
@@ -304,18 +302,13 @@ describe Chef::ApiClientV1 do
       File.open(Chef::Config[:client_key], "r") {|f| f.read.chomp }
     end
 
-    it "has an HTTP client configured with default credentials" do
-      expect(@client.http_api).to be_a_kind_of(Chef::REST)
-      expect(@client.http_api.client_name).to eq("silent-bob")
-      expect(@client.http_api.signing_key.to_s).to eq(private_key_data)
-    end
   end
 
 
   describe "when requesting a new key" do
     before do
-      @http_client = double("Chef::REST mock")
-      allow(Chef::REST).to receive(:new).and_return(@http_client)
+      @http_client = double("Chef::ServerAPI mock")
+      allow(Chef::ServerAPI).to receive(:new).and_return(@http_client)
     end
 
     context "and the client does not exist on the server" do
@@ -330,65 +323,6 @@ describe Chef::ApiClientV1 do
         expect { Chef::ApiClientV1.reregister("lost-my-key") }.to raise_error(Net::HTTPServerException)
       end
     end
-
-    context "and the client exists" do
-      let(:chef_rest_v0_mock) { double('chef rest root v0 object') }
-      let(:payload) {
-        {:name => "lost-my-key", :admin => false, :validator => false, :private_key => true}
-      }
-
-      before do
-        @api_client_without_key = Chef::ApiClientV1.new
-        @api_client_without_key.name("lost-my-key")
-        allow(@api_client_without_key).to receive(:chef_rest_v0).and_return(chef_rest_v0_mock)
-        #allow(@api_client_with_key).to receive(:http_api).and_return(_api_mock)
-
-        allow(chef_rest_v0_mock).to receive(:put).with("clients/lost-my-key", payload).and_return(@api_client_with_key)
-        allow(chef_rest_v0_mock).to receive(:get).with("clients/lost-my-key").and_return(@api_client_without_key)
-        allow(@http_client).to receive(:get).with("clients/lost-my-key").and_return(@api_client_without_key)
-      end
-
-      context "and the client exists on a Chef 11-like server" do
-        before do
-          @api_client_with_key = Chef::ApiClientV1.new
-          @api_client_with_key.name("lost-my-key")
-          @api_client_with_key.private_key("the new private key")
-          allow(@api_client_with_key).to receive(:chef_rest_v0).and_return(chef_rest_v0_mock)
-        end
-
-        it "returns an ApiClient with a private key" do
-          expect(chef_rest_v0_mock).to receive(:put).with("clients/lost-my-key", payload).
-                                        and_return(@api_client_with_key)
-
-          response = Chef::ApiClientV1.reregister("lost-my-key")
-          # no sane == method for ApiClient :'(
-          expect(response).to eq(@api_client_without_key)
-          expect(response.private_key).to eq("the new private key")
-          expect(response.name).to eq("lost-my-key")
-          expect(response.admin).to be_falsey
-        end
-      end
-
-      context "and the client exists on a Chef 10-like server" do
-        before do
-          @api_client_with_key = {"name" => "lost-my-key", "private_key" => "the new private key"}
-          expect(chef_rest_v0_mock).to receive(:put).
-            with("clients/lost-my-key", :name => "lost-my-key", :admin => false, :validator => false, :private_key => true).
-            and_return(@api_client_with_key)
-        end
-
-        it "returns an ApiClient with a private key" do
-          response = Chef::ApiClientV1.reregister("lost-my-key")
-          # no sane == method for ApiClient :'(
-          expect(response).to eq(@api_client_without_key)
-          expect(response.private_key).to eq("the new private key")
-          expect(response.name).to eq("lost-my-key")
-          expect(response.admin).to be_falsey
-          expect(response.validator).to be_falsey
-        end
-      end
-
-    end
   end
 
   describe "Versioned API Interactions" do
@@ -439,7 +373,7 @@ describe Chef::ApiClientV1 do
 
         shared_examples_for "client updating" do
           it "updates the client" do
-            expect(rest). to receive(:put).with("clients/some_name", payload)
+            expect(rest). to receive(:put).with("clients/some_name", payload).and_return(payload)
             @client.update
           end
 
@@ -457,7 +391,7 @@ describe Chef::ApiClientV1 do
             end
 
             it "updates the client with only the name" do
-              expect(rest). to receive(:put).with("clients/some_name", {:name => "some_name"})
+              expect(rest). to receive(:put).with("clients/some_name", {:name => "some_name"}).and_return({:name => "some_name"})
               @client.update
             end
           end
diff --git a/spec/unit/knife/client_edit_spec.rb b/spec/unit/knife/client_edit_spec.rb
index 6a5549e28c..ad56d9212d 100644
--- a/spec/unit/knife/client_edit_spec.rb
+++ b/spec/unit/knife/client_edit_spec.rb
@@ -17,16 +17,29 @@
 #
 
 require 'spec_helper'
+require 'chef/api_client_v1'
 
 describe Chef::Knife::ClientEdit do
   before(:each) do
     @knife = Chef::Knife::ClientEdit.new
     @knife.name_args = [ 'adam' ]
+    @knife.config[:disable_editing] = true
   end
 
   describe 'run' do
+    let(:data) {
+      {
+        "name" => "adam",
+        "validator" => false,
+        "admin" => false,
+        "chef_type" => "client",
+        "create_key" => true
+      }
+    }
+
     it 'should edit the client' do
-      expect(@knife).to receive(:edit_object).with(Chef::ApiClientV1, 'adam')
+      allow(Chef::ApiClientV1).to receive(:load).with('adam').and_return(data)
+      expect(@knife).to receive(:edit_data).with(data).and_return(data)
       @knife.run
     end