Merge pull request chocolatey-archive#146 from michaeltlombardi/ticke…

…t/master/MODULES-8047-fix-chococonfig

(MODULES-8047) Fix puppet resource chocolateyconfig

lib/puppet/provider/chocolateyconfig/windows.rb

@@ -54,9 +54,9 @@ def self.get_config(element)
     config[:name] = element.attributes['key'] if element.attributes['key']
     config[:value] = element.attributes['value'] if element.attributes['value']
     config[:description] = element.attributes['description'] if element.attributes['description']
-
-    config[:ensure] = :present
-
+    # If the value is empty it is the default value and so is not set by Puppet.
+    # If a config item is ensured as absent it sets the value to an empty string.
+    config[:ensure] = element.attributes['value'].to_s.empty? ? :absent : :present
     Puppet.debug("Loaded config '#{config.inspect}'.")
 
     config
@@ -99,6 +99,15 @@ def destroy
   end
 
   def validate
+    # We want to ensure that specifying a config item as :present fails if no :value for the config
+    # is specified. However, during puppet resource runs the resource has an :ensure of present.
+    # We are able to overcome this by checking if the hash is empty:
+    # The hash *is* empty when the validate block is called during a puppet apply run.
+    # The hash is *not* empty when the validate block is called during a puppet resource run.
+    # If the hash is empty, fail only if :ensure is true and :value is not specified or is an empty string.
+    if @property_hash.empty? && resource[:ensure] == :present && resource[:value].to_s.empty?
+      raise ArgumentError, "Unless ensure => absent, value is required."
+    end
     choco_version = Gem::Version.new(PuppetX::Chocolatey::ChocolateyCommon.choco_version)
     if PuppetX::Chocolatey::ChocolateyCommon.file_exists?(PuppetX::Chocolatey::ChocolateyCommon.chocolatey_command) && choco_version < Gem::Version.new(CONFIG_MINIMUM_SUPPORTED_CHOCO_VERSION)
       raise Puppet::ResourceError, "Chocolatey version must be '#{CONFIG_MINIMUM_SUPPORTED_CHOCO_VERSION}' to manage configuration values. Detected '#{choco_version}' as your version. Please upgrade Chocolatey."

lib/puppet/type/chocolateyconfig.rb

@@ -70,10 +70,6 @@ def insync?(is)
   end
 
   validate do
-    if self[:ensure] != :absent
-      raise ArgumentError, "Unless ensure => absent, value is required." if self[:value].nil? || self[:value].empty?
-    end
-
     if provider.respond_to?(:validate)
       provider.validate
     end

spec/unit/puppet/provider/chocolateyconfig/windows_spec.rb

@@ -211,6 +211,30 @@
         resource.provider.validate
       }.to raise_error(Puppet::ResourceError, /Chocolatey version must be '0.9.10.0' to manage configuration values. Detected '#{last_unsupported_version}'/)
     end
+
+    it "should error when the property_hash is empty, :ensure is :present, and no :value is supplied" do
+      resource.delete(:value)
+
+      expect {
+        resource.provider.validate
+      }.to raise_error(ArgumentError, "Unless ensure => absent, value is required.")
+    end
+
+    it "should not error when the property_hash is defined, even if :ensure is :present and no :value is supplied" do
+      resource.delete(:value)
+      resource.provider.instance_variable_set("@property_hash", {:value => "something"})
+      resource.provider.validate
+    end
+
+    it "should not error when the property_hash is empty, :ensure is :present and a :value is supplied" do
+      resource.provider.validate
+    end
+
+    it "should not error when the property_hash is empty, :ensure is :absent and no :value is supplied" do
+      resource[:ensure] = :absent
+      resource.delete(:value)
+      resource.provider.validate
+    end
   end
 
   context ".flush" do

spec/unit/puppet/type/chocolateyconfig_spec.rb

@@ -77,27 +77,4 @@
     reqs[0].target.must == resource
   end
 
-  context ".validate" do
-    it "should pass when ensure => absent with no value" do
-      resource[:ensure] = :absent
-
-      resource.validate
-    end
-
-    it "should pass when ensure => present with a value" do
-      resource[:ensure] = :present
-      resource[:value] = 'yo'
-
-      resource.validate
-    end
-
-    it "should fail when ensure => present with no value" do
-      resource[:ensure] = :present
-
-      expect {
-        resource.validate
-      }.to raise_error(ArgumentError, /Unless ensure => absent, value is required/)
-    end
-  end
-
 end