Use ApiClient#from_hash rather than relying on json_class in return data

Previously, knife client create was unable to create a client if the
client json provided by the user did not include json_class.  Using
ApiClient#from_hash removes the restriction.

The tests have been updated to test the contract between knife client
create and the ApiClient class.  Namely, knife client create is
expected to call ApiClient.from_hash with a properly formated hash
given the the user options and then call #save on the returned object.

2 files changed, 63 insertions, 48 deletions

diff --git a/lib/chef/knife/client_create.rb b/lib/chef/knife/client_create.rb
index b2bac36081..477a400e8a 100644
--- a/lib/chef/knife/client_create.rb
+++ b/lib/chef/knife/client_create.rb
@@ -54,14 +54,16 @@ class Chef
           exit 1
         end
 
-        client = Chef::ApiClient.new
-        client.name(@client_name)
-        client.admin(config[:admin])
-        client.validator(config[:validator])
+        client_hash = {
+          "name" => @client_name,
+          "admin" => !!config[:admin],
+          "validator" => !!config[:validator]
+        }
 
-        output = edit_data(client)
+        output = Chef::ApiClient.from_hash(edit_hash(client_hash))
 
-        # Chef::ApiClient.save will try to create a client and if it exists will update it instead silently
+        # Chef::ApiClient.save will try to create a client and if it
+        # exists will update it instead silently.
         client = output.save
 
         # We only get a private_key on client creation, not on client update.
@@ -83,4 +85,3 @@ class Chef
     end
   end
 end
-
diff --git a/spec/unit/knife/client_create_spec.rb b/spec/unit/knife/client_create_spec.rb
index 8e7cc4a5e3..10d386b5ff 100644
--- a/spec/unit/knife/client_create_spec.rb
+++ b/spec/unit/knife/client_create_spec.rb
@@ -21,82 +21,96 @@ require 'spec_helper'
 Chef::Knife::ClientCreate.load_deps
 
 describe Chef::Knife::ClientCreate do
+  let(:stderr) { StringIO.new }
+
+  let(:default_client_hash) do
+    {
+      "name" => "adam",
+      "validator" => false,
+      "admin" => false
+    }
+  end
+
+  let(:client) do
+    c = double("Chef::ApiClient")
+    allow(c).to receive(:save).and_return({"private_key" => ""})
+    allow(c).to receive(:to_s).and_return("client[adam]")
+    c
+  end
+
+  let(:knife) do
+    k = Chef::Knife::ClientCreate.new
+    k.name_args = [ "adam" ]
+    k.ui.config[:disable_editing] = true
+    allow(k.ui).to receive(:stderr).and_return(stderr)
+    allow(k.ui).to receive(:stdout).and_return(stderr)
+    k
+  end
+
   before(:each) do
     Chef::Config[:node_name]  = "webmonkey.example.com"
-    @knife = Chef::Knife::ClientCreate.new
-    @knife.config = {
-      :file => nil,
-      :admin => false,
-      :validator => false
-    }
-    @knife.name_args = [ "adam" ]
-    @client = Chef::ApiClient.new
-    allow(@client).to receive(:save).and_return({ 'private_key' => '' })
-    allow(@knife).to receive(:edit_data).and_return(@client)
-    allow(@knife).to receive(:puts)
-    allow(Chef::ApiClient).to receive(:new).and_return(@client)
-    @stderr = StringIO.new
-    allow(@knife.ui).to receive(:stderr).and_return(@stderr)
   end
 
   describe "run" do
-    it "should create a new Client" do
-      expect(Chef::ApiClient).to receive(:new).and_return(@client)
-      @knife.run
-      expect(@stderr.string).to match /created client.+adam/i
+    it "should create and save the ApiClient" do
+      expect(Chef::ApiClient).to receive(:from_hash).and_return(client)
+      expect(client).to receive(:save)
+      knife.run
+    end
+
+    it "should print a message upon creation" do
+      expect(Chef::ApiClient).to receive(:from_hash).and_return(client)
+      expect(client).to receive(:save)
+      knife.run
+      expect(stderr.string).to match /Created client.*adam/i
     end
 
     it "should set the Client name" do
-      expect(@client).to receive(:name).with("adam")
-      @knife.run
+      expect(Chef::ApiClient).to receive(:from_hash).with(hash_including("name" => "adam")).and_return(client)
+      knife.run
     end
 
     it "by default it is not an admin" do
-      expect(@client).to receive(:admin).with(false)
-      @knife.run
+      expect(Chef::ApiClient).to receive(:from_hash).with(hash_including("admin" => false)).and_return(client)
+      knife.run
     end
 
     it "by default it is not a validator" do
-      expect(@client).to receive(:validator).with(false)
-      @knife.run
+      expect(Chef::ApiClient).to receive(:from_hash).with(hash_including("validator" => false)).and_return(client)
+      knife.run
     end
 
     it "should allow you to edit the data" do
-      expect(@knife).to receive(:edit_data).with(@client)
-      @knife.run
-    end
-
-    it "should save the Client" do
-      expect(@client).to receive(:save)
-      @knife.run
+      expect(knife).to receive(:edit_hash).with(default_client_hash).and_return(default_client_hash)
+      allow(Chef::ApiClient).to receive(:from_hash).and_return(client)
+      knife.run
     end
 
     describe "with -f or --file" do
       it "should write the private key to a file" do
-        @knife.config[:file] = "/tmp/monkeypants"
-        allow(@client).to receive(:save).and_return({ 'private_key' => "woot" })
+        knife.config[:file] = "/tmp/monkeypants"
+        allow_any_instance_of(Chef::ApiClient).to receive(:save).and_return({ 'private_key' => "woot" })
         filehandle = double("Filehandle")
         expect(filehandle).to receive(:print).with('woot')
         expect(File).to receive(:open).with("/tmp/monkeypants", "w").and_yield(filehandle)
-        @knife.run
+        knife.run
       end
     end
 
     describe "with -a or --admin" do
       it "should create an admin client" do
-        @knife.config[:admin] = true
-        expect(@client).to receive(:admin).with(true)
-        @knife.run
+        knife.config[:admin] = true
+        expect(Chef::ApiClient).to receive(:from_hash).with(hash_including("admin" => true)).and_return(client)
+        knife.run
       end
     end
 
     describe "with --validator" do
       it "should create an validator client" do
-        @knife.config[:validator] = true
-        expect(@client).to receive(:validator).with(true)
-        @knife.run
+        knife.config[:validator] = true
+        expect(Chef::ApiClient).to receive(:from_hash).with(hash_including("validator" => true)).and_return(client)
+        knife.run
       end
     end
-
   end
 end