Allow deletion of registry_key without the need for users to pass type and data key in values hash.

Signed-off-by: aliasgar16 <aliasgar.batterywala@msystechnologies.com>

7 files changed, 265 insertions, 19 deletions

diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb
index a4d5ff60e2..ea779754e2 100644
--- a/lib/chef/exceptions.rb
+++ b/lib/chef/exceptions.rb
@@ -245,6 +245,10 @@ class Chef
     class Win32RegBadValueSize < ArgumentError; end
     class Win32RegTypesMismatch < ArgumentError; end
 
+    # incorrect input for registry_key create action throws following error
+    class RegKeyValuesTypeMissing < ArgumentError; end
+    class RegKeyValuesDataMissing < ArgumentError; end
+
     class InvalidEnvironmentPath < ArgumentError; end
     class EnvironmentNotFound < RuntimeError; end
 
diff --git a/lib/chef/provider/registry_key.rb b/lib/chef/provider/registry_key.rb
index 5e8dbe9bd8..58b0a19586 100644
--- a/lib/chef/provider/registry_key.rb
+++ b/lib/chef/provider/registry_key.rb
@@ -70,27 +70,51 @@ class Chef
         end
       end
 
+      def key_missing?(values, name)
+        values.each do |v|
+          return true unless v.has_key?(name)
+        end
+        false
+      end
+
       def define_resource_requirements
         requirements.assert(:create, :create_if_missing, :delete, :delete_key) do |a|
           a.assertion { registry.hive_exists?(@new_resource.key) }
           a.failure_message(Chef::Exceptions::Win32RegHiveMissing, "Hive #{@new_resource.key.split("\\").shift} does not exist")
         end
+
         requirements.assert(:create) do |a|
           a.assertion { registry.key_exists?(@new_resource.key) }
           a.whyrun("Key #{@new_resource.key} does not exist. Unless it would have been created before, attempt to modify its values would fail.")
         end
+
         requirements.assert(:create, :create_if_missing) do |a|
-          #If keys missing in the path and recursive == false
+          # If keys missing in the path and recursive == false
           a.assertion { !registry.keys_missing?(@current_resource.key) || @new_resource.recursive }
           a.failure_message(Chef::Exceptions::Win32RegNoRecursive, "Intermediate keys missing but recursive is set to false")
           a.whyrun("Intermediate keys in #{@new_resource.key} do not exist. Unless they would have been created earlier, attempt to modify them would fail.")
         end
+
         requirements.assert(:delete_key) do |a|
-          #If key to be deleted has subkeys but recurssive == false
+          # If key to be deleted has subkeys but recurssive == false
           a.assertion { !registry.key_exists?(@new_resource.key) || !registry.has_subkeys?(@new_resource.key) || @new_resource.recursive }
           a.failure_message(Chef::Exceptions::Win32RegNoRecursive, "#{@new_resource.key} has subkeys but recursive is set to false.")
           a.whyrun("#{@current_resource.key} has subkeys, but recursive is set to false. attempt to delete would fails unless subkeys were deleted prior to this action.")
         end
+
+        requirements.assert(:create, :create_if_missing) do |a|
+          # If type key missing in the RegistryKey values hash
+          a.assertion { !key_missing?(@new_resource.values, :type) }
+          a.failure_message(Chef::Exceptions::RegKeyValuesTypeMissing, "Missing type key in RegistryKey values hash")
+          a.whyrun("Type key does not exist. Attempt would fail unless the complete values hash containing all the keys does not exist for registry_key resource's create action.")
+        end
+
+        requirements.assert(:create, :create_if_missing) do |a|
+          # If data key missing in the RegistryKey values hash
+          a.assertion { !key_missing?(@new_resource.values, :data) }
+          a.failure_message(Chef::Exceptions::RegKeyValuesDataMissing, "Missing data key in RegistryKey values hash")
+          a.whyrun("Data key does not exist. Attempt would fail unless the complete values hash containing all the keys does not exist for registry_key resource's create action.")
+        end
       end
 
       def action_create
diff --git a/lib/chef/resource/registry_key.rb b/lib/chef/resource/registry_key.rb
index d11f826c38..da09b9668b 100644
--- a/lib/chef/resource/registry_key.rb
+++ b/lib/chef/resource/registry_key.rb
@@ -87,13 +87,13 @@ class Chef
 
           @values.each do |v|
             raise ArgumentError, "Missing name key in RegistryKey values hash" unless v.has_key?(:name)
-            raise ArgumentError, "Missing type key in RegistryKey values hash" unless v.has_key?(:type)
-            raise ArgumentError, "Missing data key in RegistryKey values hash" unless v.has_key?(:data)
             v.each_key do |key|
               raise ArgumentError, "Bad key #{key} in RegistryKey values hash" unless [:name, :type, :data].include?(key)
             end
             raise ArgumentError, "Type of name => #{v[:name]} should be string" unless v[:name].is_a?(String)
-            raise ArgumentError, "Type of type => #{v[:type]} should be symbol" unless v[:type].is_a?(Symbol)
+            if v[:type]
+              raise ArgumentError, "Type of type => #{v[:type]} should be symbol" unless v[:type].is_a?(Symbol)
+            end
           end
           @unscrubbed_values = @values
         elsif self.instance_variable_defined?(:@values)
diff --git a/spec/functional/resource/registry_spec.rb b/spec/functional/resource/registry_spec.rb
index 8393e0a074..a7748fa4e7 100644
--- a/spec/functional/resource/registry_spec.rb
+++ b/spec/functional/resource/registry_spec.rb
@@ -197,13 +197,37 @@ describe Chef::Resource::RegistryKey, :windows_only, :broken => true do
       expect(@registry.value_exists?(reg_child + '\OpscodeTest', { :name => "Chef", :type => :multi_string, :data => %w{OpscodeOrange Rules} })).to eq(true)
     end
 
-    it "gives error if action create and parent does not exist and recursive is set to false" do
+    it "raises an error if action create and parent does not exist and recursive is set to false" do
       @new_resource.key(reg_child + '\Missing1\Missing2')
       @new_resource.values([{ :name => "OC", :type => :string, :data => "MissingData" }])
       @new_resource.recursive(false)
       expect { @new_resource.run_action(:create) }.to raise_error(Chef::Exceptions::Win32RegNoRecursive)
     end
 
+    it "raises an error if action create and type key missing in values hash" do
+      @new_resource.key(reg_child)
+      @new_resource.values([{ :name => "OC", :data => "my_data" }])
+      expect { @new_resource.run_action(:create) }.to raise_error(Chef::Exceptions::RegKeyValuesTypeMissing)
+    end
+
+    it "raises an error if action create and data key missing in values hash" do
+      @new_resource.key(reg_child)
+      @new_resource.values([{ :name => "OC", :type => :string }])
+      expect { @new_resource.run_action(:create) }.to raise_error(Chef::Exceptions::RegKeyValuesDataMissing)
+    end
+
+    it "raises an error if action create and only name key present in values hash" do
+      @new_resource.key(reg_child)
+      @new_resource.values([{ :name => "OC" }])
+      expect { @new_resource.run_action(:create) }.to raise_error(Chef::Exceptions::RegKeyValuesTypeMissing)
+    end
+
+    it "does not raise an error if action create and all keys are present in values hash" do
+      @new_resource.key(reg_child)
+      @new_resource.values([{ :name => "OC", :type => :string, :data => "my_data" }])
+      expect { @new_resource.run_action(:create) }.to_not raise_error
+    end
+
     it "creates missing keys if action create and parent does not exist and recursive is set to true" do
       @new_resource.key(reg_child + '\Missing1\Missing2')
       @new_resource.values([{ :name => "OC", :type => :string, :data => "MissingData" }])
@@ -270,7 +294,7 @@ describe Chef::Resource::RegistryKey, :windows_only, :broken => true do
         Chef::Config[:why_run] = true
       end
 
-      it "does not throw an exception if the keys do not exist but recursive is set to false" do
+      it "does not raise an exception if the keys do not exist but recursive is set to false" do
         @new_resource.key(reg_child + '\Slitheen\Raxicoricofallapatorius')
         @new_resource.values([{ :name => "BriskWalk", :type => :string, :data => "is good for health" }])
         @new_resource.recursive(false)
@@ -278,6 +302,7 @@ describe Chef::Resource::RegistryKey, :windows_only, :broken => true do
         expect(@registry.key_exists?(reg_child + '\Slitheen')).to eq(false)
         expect(@registry.key_exists?(reg_child + '\Slitheen\Raxicoricofallapatorius')).to eq(false)
       end
+
       it "does not create key if the action is create" do
         @new_resource.key(reg_child + '\Slitheen')
         @new_resource.values([{ :name => "BriskWalk", :type => :string, :data => "is good for health" }])
@@ -285,6 +310,34 @@ describe Chef::Resource::RegistryKey, :windows_only, :broken => true do
         @new_resource.run_action(:create)
         expect(@registry.key_exists?(reg_child + '\Slitheen')).to eq(false)
       end
+
+      it "does not raise an exception if the action create and type key missing in values hash" do
+        @new_resource.key(reg_child + '\Slitheen')
+        @new_resource.values([{ :name => "BriskWalk", :data => "my_data" }])
+        @new_resource.run_action(:create) # should not raise_error
+        expect(@registry.key_exists?(reg_child + '\Slitheen')).to eq(false)
+      end
+
+      it "does not raise an exception if the action create and data key missing in values hash" do
+        @new_resource.key(reg_child + '\Slitheen')
+        @new_resource.values([{ :name => "BriskWalk", :type => :string }])
+        @new_resource.run_action(:create) # should not raise_error
+        expect(@registry.key_exists?(reg_child + '\Slitheen')).to eq(false)
+      end
+
+      it "does not raise an exception if the action create and only name key present in values hash" do
+        @new_resource.key(reg_child + '\Slitheen')
+        @new_resource.values([{ :name => "BriskWalk" }])
+        @new_resource.run_action(:create) # should not raise_error
+        expect(@registry.key_exists?(reg_child + '\Slitheen')).to eq(false)
+      end
+
+      it "does not raise an exception if the action create and all keys are present in values hash" do
+        @new_resource.key(reg_child + '\Slitheen')
+        @new_resource.values([{ :name => "BriskWalk", :type => :string, :data => "my_data" }])
+        @new_resource.run_action(:create) # should not raise_error
+        expect(@registry.key_exists?(reg_child + '\Slitheen')).to eq(false)
+      end
     end
   end
 
@@ -330,13 +383,37 @@ describe Chef::Resource::RegistryKey, :windows_only, :broken => true do
       expect(@registry.value_exists?(reg_child + '\Pyrovile', { :name => "Chef", :type => :multi_string, :data => %w{OpscodeOrange Rules} })).to eq(true)
     end
 
-    it "gives error if action create and parent does not exist and recursive is set to false" do
+    it "raises an error if action create and parent does not exist and recursive is set to false" do
       @new_resource.key(reg_child + '\Sontaran\Sontar')
       @new_resource.values([{ :name => "OC", :type => :string, :data => "MissingData" }])
       @new_resource.recursive(false)
       expect { @new_resource.run_action(:create_if_missing) }.to raise_error(Chef::Exceptions::Win32RegNoRecursive)
     end
 
+    it "raises an error if action create_if_missing and type key missing in values hash" do
+      @new_resource.key(reg_child)
+      @new_resource.values([{ :name => "OC", :data => "my_data" }])
+      expect { @new_resource.run_action(:create_if_missing) }.to raise_error(Chef::Exceptions::RegKeyValuesTypeMissing)
+    end
+
+    it "raises an error if action create_if_missing and data key missing in values hash" do
+      @new_resource.key(reg_child)
+      @new_resource.values([{ :name => "OC", :type => :string }])
+      expect { @new_resource.run_action(:create_if_missing) }.to raise_error(Chef::Exceptions::RegKeyValuesDataMissing)
+    end
+
+    it "raises an error if action create_if_missing and only name key present in values hash" do
+      @new_resource.key(reg_child)
+      @new_resource.values([{ :name => "OC" }])
+      expect { @new_resource.run_action(:create_if_missing) }.to raise_error(Chef::Exceptions::RegKeyValuesTypeMissing)
+    end
+
+    it "does not raise an error if action create_if_missing and all keys are present in values hash" do
+      @new_resource.key(reg_child)
+      @new_resource.values([{ :name => "OC", :type => :string, :data => "my_data" }])
+      expect { @new_resource.run_action(:create_if_missing) }.to_not raise_error
+    end
+
     it "creates missing keys if action create and parent does not exist and recursive is set to true" do
       @new_resource.key(reg_child + '\Sontaran\Sontar')
       @new_resource.values([{ :name => "OC", :type => :string, :data => "MissingData" }])
@@ -381,7 +458,7 @@ describe Chef::Resource::RegistryKey, :windows_only, :broken => true do
         Chef::Config[:why_run] = true
       end
 
-      it "does not throw an exception if the keys do not exist but recursive is set to false" do
+      it "does not raise an exception if the keys do not exist but recursive is set to false" do
         @new_resource.key(reg_child + '\Zygons\Zygor')
         @new_resource.values([{ :name => "BriskWalk", :type => :string, :data => "is good for health" }])
         @new_resource.recursive(false)
@@ -389,6 +466,7 @@ describe Chef::Resource::RegistryKey, :windows_only, :broken => true do
         expect(@registry.key_exists?(reg_child + '\Zygons')).to eq(false)
         expect(@registry.key_exists?(reg_child + '\Zygons\Zygor')).to eq(false)
       end
+
       it "does nothing if the action is create_if_missing" do
         @new_resource.key(reg_child + '\Zygons')
         @new_resource.values([{ :name => "BriskWalk", :type => :string, :data => "is good for health" }])
@@ -396,6 +474,34 @@ describe Chef::Resource::RegistryKey, :windows_only, :broken => true do
         @new_resource.run_action(:create_if_missing)
         expect(@registry.key_exists?(reg_child + '\Zygons')).to eq(false)
       end
+
+      it "does not raise an exception if the action create_if_missing and type key missing in values hash" do
+        @new_resource.key(reg_child + '\Zygons')
+        @new_resource.values([{ :name => "BriskWalk", :data => "my_data" }])
+        @new_resource.run_action(:create_if_missing) # should not raise_error
+        expect(@registry.key_exists?(reg_child + '\Zygons')).to eq(false)
+      end
+
+      it "does not raise an exception if the action create_if_missing and data key missing in values hash" do
+        @new_resource.key(reg_child + '\Zygons')
+        @new_resource.values([{ :name => "BriskWalk", :type => :string }])
+        @new_resource.run_action(:create_if_missing) # should not raise_error
+        expect(@registry.key_exists?(reg_child + '\Zygons')).to eq(false)
+      end
+
+      it "does not raise an exception if the action create_if_missing and only name key present in values hash" do
+        @new_resource.key(reg_child + '\Zygons')
+        @new_resource.values([{ :name => "BriskWalk" }])
+        @new_resource.run_action(:create_if_missing) # should not raise_error
+        expect(@registry.key_exists?(reg_child + '\Zygons')).to eq(false)
+      end
+
+      it "does not raise an exception if the action create_if_missing and all keys are present in values hash" do
+        @new_resource.key(reg_child + '\Zygons')
+        @new_resource.values([{ :name => "BriskWalk", :type => :string, :data => "my_data" }])
+        @new_resource.run_action(:create_if_missing) # should not raise_error
+        expect(@registry.key_exists?(reg_child + '\Zygons')).to eq(false)
+      end
     end
   end
 
diff --git a/spec/unit/exceptions_spec.rb b/spec/unit/exceptions_spec.rb
index 940edfec2e..e952a5448a 100644
--- a/spec/unit/exceptions_spec.rb
+++ b/spec/unit/exceptions_spec.rb
@@ -68,6 +68,8 @@ describe Chef::Exceptions do
     Chef::Exceptions::EnvironmentNotFound => RuntimeError,
     Chef::Exceptions::InvalidVersionConstraint => ArgumentError,
     Chef::Exceptions::IllegalVersionConstraint => NotImplementedError,
+    Chef::Exceptions::RegKeyValuesTypeMissing => ArgumentError,
+    Chef::Exceptions::RegKeyValuesDataMissing => ArgumentError,
   }
 
   exception_to_super_class.each do |exception, expected_super_class|
diff --git a/spec/unit/provider/registry_key_spec.rb b/spec/unit/provider/registry_key_spec.rb
index 9e19d02d5e..3fb9468f5d 100644
--- a/spec/unit/provider/registry_key_spec.rb
+++ b/spec/unit/provider/registry_key_spec.rb
@@ -311,3 +311,97 @@ describe Chef::Provider::RegistryKey do
     end
   end
 end
+
+describe Chef::Provider::RegistryKey, "key_missing?" do
+  let(:provider) { Chef::Provider::RegistryKey.new(nil, nil) }
+  let(:all_keys_present_in_all_hash) do
+    [ { :name => "input1_value1", :type => :string, :data => "my_value1" },
+      { :name => "input1_value2", :type => :string, :data => "my_value2" },
+    ]
+  end
+  let(:type_key_not_present_in_any_hash) do
+    [ { :name => "input2_value1", :data => "my_value1" },
+      { :name => "input2_value2", :data => "my_value2" },
+    ]
+  end
+  let(:type_key_not_present_in_some_hash) do
+    [ { :name => "input3_value1", :data => "my_value1" },
+      { :name => "input3_value2", :type => :string, :data => "my_value2" },
+    ]
+  end
+  let(:data_key_not_present_in_any_hash) do
+    [ { :name => "input4_value1", :type => :string },
+      { :name => "input4_value2", :type => :string },
+    ]
+  end
+  let(:data_key_not_present_in_some_hash) do
+    [ { :name => "input5_value1", :type => :string, :data => "my_value1" },
+      { :name => "input5_value2", :type => :string },
+    ]
+  end
+  let(:only_name_key_present_in_all_hash) do
+    [ { :name => "input6_value1" },
+      { :name => "input6_value2" },
+    ]
+  end
+
+  context "type key" do
+    context "when type key is present in all the values hash of registry_key resource" do
+      it "returns false" do
+        response = provider.key_missing?(all_keys_present_in_all_hash, :type)
+        expect(response).to be == false
+      end
+    end
+
+    context "when type key is not present in any of the values hash of registry_key resource" do
+      it "returns true" do
+        response = provider.key_missing?(type_key_not_present_in_any_hash, :type)
+        expect(response).to be == true
+      end
+    end
+
+    context "when type key is not present only in some of the values hash of registry_key resource" do
+      it "returns true" do
+        response = provider.key_missing?(type_key_not_present_in_some_hash, :type)
+        expect(response).to be == true
+      end
+    end
+
+    context "when only name key is present in all the values hash of registry_key resource" do
+      it "returns true" do
+        response = provider.key_missing?(only_name_key_present_in_all_hash, :type)
+        expect(response).to be == true
+      end
+    end
+  end
+
+  context "data key" do
+    context "when data key is present in all the values hash of registry_key resource" do
+      it "returns false" do
+        response = provider.key_missing?(all_keys_present_in_all_hash, :data)
+        expect(response).to be == false
+      end
+    end
+
+    context "when data key is not present in any of the values hash of registry_key resource" do
+      it "returns true" do
+        response = provider.key_missing?(data_key_not_present_in_any_hash, :data)
+        expect(response).to be == true
+      end
+    end
+
+    context "when data key is not present only in some of the values hash of registry_key resource" do
+      it "returns true" do
+        response = provider.key_missing?(data_key_not_present_in_some_hash, :data)
+        expect(response).to be == true
+      end
+    end
+
+    context "when only name key is present in all the values hash of registry_key resource" do
+      it "returns true" do
+        response = provider.key_missing?(only_name_key_present_in_all_hash, :data)
+        expect(response).to be == true
+      end
+    end
+  end
+end
diff --git a/spec/unit/resource/registry_key_spec.rb b/spec/unit/resource/registry_key_spec.rb
index 472c511857..d378da3ed0 100644
--- a/spec/unit/resource/registry_key_spec.rb
+++ b/spec/unit/resource/registry_key_spec.rb
@@ -94,19 +94,11 @@ describe Chef::Resource::RegistryKey, "values" do
     expect(@resource.values).to eql([ { :name => "poosh", :type => :binary, :data => "a8100ae6aa1940d0b663bb31cd466142ebbdbd5187131b92d93818987832eb89" } ])
   end
 
-  it "should throw an exception if the name field is missing" do
+  it "should raise an exception if the name field is missing" do
     expect { @resource.values [ { :type => :string, :data => "carmen" } ] }.to raise_error(ArgumentError)
   end
 
-  it "should throw an exception if the type field is missing" do
-    expect { @resource.values [ { :name => "poosh", :data => "carmen" } ] }.to raise_error(ArgumentError)
-  end
-
-  it "should throw an exception if the data field is missing" do
-    expect { @resource.values [ { :name => "poosh", :type => :string } ] }.to raise_error(ArgumentError)
-  end
-
-  it "should throw an exception if extra fields are present" do
+  it "should raise an exception if extra fields are present" do
     expect { @resource.values [ { :name => "poosh", :type => :string, :data => "carmen", :screwdriver => "sonic" } ] }.to raise_error(ArgumentError)
   end
 
@@ -117,6 +109,30 @@ describe Chef::Resource::RegistryKey, "values" do
   it "should not allow an integer" do
     expect { @resource.send(:values, 100) }.to raise_error(ArgumentError)
   end
+
+  it "should raise an exception if type of name is not string" do
+    expect { @resource.values([ { :name => 123, :type => :string, :data => "carmen" } ]) }.to raise_error(ArgumentError)
+  end
+
+  it "should not raise an exception if type of name is string" do
+    expect { @resource.values([ { :name => "123", :type => :string, :data => "carmen" } ]) }.to_not raise_error
+  end
+
+  context "type key not given" do
+    it "should not raise an exception" do
+      expect { @resource.values([ { :name => "123", :data => "carmen" } ]) }.to_not raise_error
+    end
+  end
+
+  context "type key given" do
+    it "should raise an exception if type of type is not symbol" do
+      expect { @resource.values([ { :name => "123", :type => "string", :data => "carmen" } ]) }.to raise_error(ArgumentError)
+    end
+
+    it "should not raise an exception if type of type is symbol" do
+      expect { @resource.values([ { :name => "123", :type => :string, :data => "carmen" } ]) }.to_not raise_error
+    end
+  end
 end
 
 describe Chef::Resource::RegistryKey, "recursive" do