Minor tweaks to handling of removing settings

This commit makes some minor changes to how we handle removing
settings.  In particular, it updates all of the line numbers
of the various 'section' objects to correspond to the new
state of the world based on the removal of a line that appeared
before them.

Also adds one more test related to setting removal.

diff --git a/lib/puppet/util/ini_file.rb b/lib/puppet/util/ini_file.rb
index 107924d34bfc7adf80a16de78d1156719f128597..52ad32c6eea97930636035b32b2e4b2116f924cd 100644 (file)
--- a/lib/puppet/util/ini_file.rb
+++ b/lib/puppet/util/ini_file.rb
@@ -45,8 +45,19 @@ module Util
     def remove_setting(section_name, setting)
       section = @sections_hash[section_name]
       if (section.has_existing_setting?(setting))
+        # If the setting is found, we have some work to do.
+        # First, we remove the line from our array of lines:
         remove_line(section, setting)
+
+        # Then, we need to tell the setting object to remove
+        # the setting from its state:
         section.remove_existing_setting(setting)
+
+        # Finally, we need to update all of the start/end line
+        # numbers for all of the sections *after* the one that
+        # was modified.
+        section_index = @section_names.index(section_name)
+        decrement_section_line_numbers(section_index + 1)
       end
     end
 
@@ -61,9 +72,7 @@ module Util
             fh.puts("\n[#{section.name}]")
           elsif ! section.end_line.nil?
             (section.start_line..section.end_line).each do |line_num|
-              if lines[line_num]
-                fh.puts(lines[line_num])
-              end
+              fh.puts(lines[line_num])
             end
           end
 
@@ -153,6 +162,17 @@ module Util
         File.readlines(path)
     end
 
+
+    # Utility method; given a section index (index into the @section_names
+    # array), decrement the start/end line numbers for that section and all
+    # all of the other sections that appear *after* the specified section.
+    def decrement_section_line_numbers(section_index)
+      @section_names[section_index..(@section_names.length - 1)].each do |name|
+        section = @sections_hash[name]
+        section.decrement_line_nums
+      end
+    end
+
   end
 end
 end

diff --git a/lib/puppet/util/ini_file/section.rb b/lib/puppet/util/ini_file/section.rb
index e192d5bd4e142ae150ffe200e082ce118ff0773d..16f19d381f200dd90d056e8e033e1b875278f7ab 100644 (file)
--- a/lib/puppet/util/ini_file/section.rb
+++ b/lib/puppet/util/ini_file/section.rb
@@ -25,13 +25,29 @@ class IniFile
     end
 
     def remove_existing_setting(setting_name)
-      @existing_settings.delete(setting_name)
+      if (@existing_settings.delete(setting_name))
+        if @end_line
+          @end_line = @end_line - 1
+        end
+      end
     end
 
     def set_additional_setting(setting_name, value)
       @additional_settings[setting_name] = value
     end
 
+    # Decrement the start and end line numbers for the section (if they are
+    # defined); this is intended to be called when a setting is removed
+    # from a section that comes before this section in the ini file.
+    def decrement_line_nums()
+      if @start_line
+        @start_line = @start_line - 1
+      end
+      if @end_line
+        @end_line = @end_line - 1
+      end
+    end
+
   end
 end
 end

diff --git a/spec/unit/puppet/provider/ini_setting/ruby_spec.rb b/spec/unit/puppet/provider/ini_setting/ruby_spec.rb
index c8fcec41030c7df3041a15cb27936ae41e79baa4..8b2f8e5e578c3005bf50fcb886ac2fd462243524 100644 (file)
--- a/spec/unit/puppet/provider/ini_setting/ruby_spec.rb
+++ b/spec/unit/puppet/provider/ini_setting/ruby_spec.rb
@@ -505,6 +505,32 @@ subby=bar
 EOS
     )
     end
+
+    it "should do nothing for a setting that does not exist" do
+      resource = Puppet::Type::Ini_setting.new(common_params.merge(
+                                                   :section => 'section:sub', :setting => 'foo', :ensure => 'absent'))
+      provider = described_class.new(resource)
+      provider.exists?.should be_nil
+      provider.destroy
+      validate_file(<<-EOS
+[section1]
+; This is also a comment
+foo=foovalue
+
+bar = barvalue
+master = true
+[section2]
+
+foo= foovalue2
+baz=bazvalue
+url = http://192.168.1.1:8080
+[section:sub]
+subby=bar
+    #another comment
+ ; yet another comment
+      EOS
+      )
+    end
   end
 
 end