Better handling of quotes for subsettings

Prior to this commit, the `ini_subsetting` type assumed that
all of the settings strings were quoted, and always wrote
out the modified value with double-quotes around it.

This commit adds tests for the case where the original setting
is not quoted, and intelligently writes the modified setting
with the same quote character (or lack thereof) that the
original setting used.

lib/puppet/util/setting_value.rb

@@ -8,7 +8,7 @@ module Util
       @subsetting_separator = subsetting_separator
       
       if @setting_value
-        unquoted = setting_value[1, setting_value.length - 2]
+        unquoted, @quote_char = unquote_setting_value(setting_value)
         @subsetting_items = unquoted.scan(Regexp.new("(?:(?:[^\\#{@subsetting_separator}]|\\.)+)"))  # an item can contain escaped separator
         @subsetting_items.map! { |item| item.strip }
       else
@@ -16,6 +16,23 @@ module Util
       end     
     end
 
+    def unquote_setting_value(setting_value)
+      quote_char = ""
+      if (setting_value.start_with?('"') and setting_value.end_with?('"'))
+        quote_char = '"'
+      elsif (setting_value.start_with?("'") and setting_value.end_with?("'"))
+        quote_char = "'"
+      end
+
+      unquoted = setting_value
+
+      if (quote_char != "")
+        unquoted = setting_value[1, setting_value.length - 2]
+      end
+
+      [unquoted, quote_char]
+    end
+
     def get_value
     
       result = ""
@@ -27,7 +44,7 @@ module Util
         first = false
       }
       
-      "\"" + result + "\""
+      @quote_char + result + @quote_char
     end
 
     def get_subsetting_value(subsetting)
@@ -71,4 +88,4 @@ module Util
     
   end
 end
-end
+end

spec/unit/puppet/provider/ini_subsetting/ruby_spec.rb

@@ -7,14 +7,6 @@ describe provider_class do
 
   let(:tmpfile) { tmpfilename("ini_setting_test") }
 
-  let(:common_params) { {
-      :title    => 'ini_setting_ensure_present_test',
-      :path     => tmpfile,
-      :section  => '',
-      :key_val_separator => '=',
-      :setting => 'JAVA_ARGS',
-  } }
-
   def validate_file(expected_content,tmpfile = tmpfile)
     File.read(tmpfile).should == expected_content
   end
@@ -27,6 +19,14 @@ describe provider_class do
   end
 
   context "when ensuring that a subsetting is present" do
+    let(:common_params) { {
+        :title    => 'ini_setting_ensure_present_test',
+        :path     => tmpfile,
+        :section  => '',
+        :key_val_separator => '=',
+        :setting => 'JAVA_ARGS',
+    } }
+
     let(:orig_content) {
       <<-EOS
 JAVA_ARGS="-Xmx192m -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=/var/log/pe-puppetdb/puppetdb-oom.hprof"
@@ -68,6 +68,52 @@ JAVA_ARGS="-Xmx256m -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=/var/log/pe
       EOS
 )
     end
-    
+  end
+
+  context "when working with subsettings in files with unquoted settings values" do
+    let(:common_params) { {
+        :title    => 'ini_setting_ensure_present_test',
+        :path     => tmpfile,
+        :section  => 'master',
+        :key_val_separator => '=',
+        :setting => 'reports',
+    } }
+
+    let(:orig_content) {
+      <<-EOS
+[master]
+
+reports = http,foo
+      EOS
+    }
+
+    it "should remove an existing subsetting" do
+      resource = Puppet::Type::Ini_subsetting.new(common_params.merge(
+          :subsetting => 'http', :subsetting_separator => ','))
+      provider = described_class.new(resource)
+      provider.exists?.should == ""
+      provider.destroy
+      validate_file(<<-EOS
+[master]
+
+reports = foo
+      EOS
+      )
+    end
+
+    it "should add a new subsetting" do
+      resource = Puppet::Type::Ini_subsetting.new(common_params.merge(
+          :subsetting => 'puppetdb', :subsetting_separator => ','))
+      provider = described_class.new(resource)
+      provider.exists?.should be_nil
+      provider.value=('')
+      validate_file(<<-EOS
+[master]
+
+reports = http,foo,puppetdb
+      EOS
+      )
+    end
+
   end
 end