Browse Source

Remove empty sections after last setting is removed

Empty sections hanging around are not useful, so if the last line in a
section is removed then remove the section. If the section still has
comments, then just leave it be.

This does not add the ability to remove sections that do not have
settings to begin with; only allows cleaning a file as puppet makes
changes to it.
Hunter Haugen 8 years ago
parent
commit
2027a0b2ab

+ 5 - 0
lib/puppet/util/ini_file.rb

@@ -134,6 +134,11 @@ module Util
           end
           end
 
 
           if ! section.is_new_section?
           if ! section.is_new_section?
+            # don't add empty sections
+            if section.empty? and ! section.is_global?
+              next
+            end
+
             # write all of the pre-existing settings
             # write all of the pre-existing settings
             (section.start_line..section.end_line).each do |line_num|
             (section.start_line..section.end_line).each do |line_num|
               line = lines[line_num]
               line = lines[line_num]

+ 5 - 5
lib/puppet/util/ini_file/section.rb

@@ -1,6 +1,4 @@
-module Puppet
-module Util
-class IniFile
+class Puppet::Util::IniFile
   class Section
   class Section
     # Some implementation details:
     # Some implementation details:
     #
     #
@@ -44,6 +42,10 @@ class IniFile
       @existing_settings.has_key?(setting_name)
       @existing_settings.has_key?(setting_name)
     end
     end
 
 
+    def empty?
+      start_line == end_line
+    end
+
     def update_existing_setting(setting_name, value)
     def update_existing_setting(setting_name, value)
       @existing_settings[setting_name] = value
       @existing_settings[setting_name] = value
     end
     end
@@ -99,5 +101,3 @@ class IniFile
 
 
   end
   end
 end
 end
-end
-end

+ 106 - 13
spec/unit/puppet/provider/ini_setting/ruby_spec.rb

@@ -89,7 +89,7 @@ subby=bar
             end
             end
           end
           end
           child_three.stubs(:file_path).returns(tmpfile)
           child_three.stubs(:file_path).returns(tmpfile)
-          child_three.instances.size == 7
+          child_three.instances.size.should eq(7)
           expected_array = [
           expected_array = [
             {:name => 'section1/foo', :value => 'foovalue' },
             {:name => 'section1/foo', :value => 'foovalue' },
             {:name => 'section1/bar', :value => 'barvalue' },
             {:name => 'section1/bar', :value => 'barvalue' },
@@ -107,7 +107,7 @@ subby=bar
             ensure_array.push(ensure_value)
             ensure_array.push(ensure_value)
             real_array.push(prop_hash)
             real_array.push(prop_hash)
           end
           end
-          ensure_array.uniq.should == [:present]
+          ensure_array.uniq.should eq([:present])
           ((real_array - expected_array) && (expected_array - real_array)).should == []
           ((real_array - expected_array) && (expected_array - real_array)).should == []
 
 
         end
         end
@@ -305,7 +305,7 @@ subby=bar
            :section => 'section:sub', :setting => 'subby', :value => 'foo'))
            :section => 'section:sub', :setting => 'subby', :value => 'foo'))
       provider = described_class.new(resource)
       provider = described_class.new(resource)
       provider.exists?.should be true
       provider.exists?.should be true
-      provider.value.should == 'bar'
+      provider.value.should eq('bar')
       provider.value=('foo')
       provider.value=('foo')
       validate_file(<<-EOS
       validate_file(<<-EOS
 # This is a comment
 # This is a comment
@@ -336,7 +336,7 @@ subby=foo
            :setting => 'url', :value => 'http://192.168.0.1:8080'))
            :setting => 'url', :value => 'http://192.168.0.1:8080'))
       provider = described_class.new(resource)
       provider = described_class.new(resource)
       provider.exists?.should be true
       provider.exists?.should be true
-      provider.value.should == 'http://192.168.1.1:8080'
+      provider.value.should eq('http://192.168.1.1:8080')
       provider.value=('http://192.168.0.1:8080')
       provider.value=('http://192.168.0.1:8080')
 
 
       validate_file( <<-EOS
       validate_file( <<-EOS
@@ -370,7 +370,7 @@ subby=bar
            :section_prefix => '-', :section_suffix => '-' ))
            :section_prefix => '-', :section_suffix => '-' ))
       provider = described_class.new(resource)
       provider = described_class.new(resource)
       provider.exists?.should be true
       provider.exists?.should be true
-      provider.value.should == 'purple'
+      provider.value.should eq('purple')
       provider.value=('http://192.168.0.1:8080')
       provider.value=('http://192.168.0.1:8080')
 
 
       validate_file( <<-EOS
       validate_file( <<-EOS
@@ -640,7 +640,7 @@ foo = http://192.168.1.1:8080
            :section => '', :setting => 'foo', :value => 'yippee'))
            :section => '', :setting => 'foo', :value => 'yippee'))
       provider = described_class.new(resource)
       provider = described_class.new(resource)
       provider.exists?.should be true
       provider.exists?.should be true
-      provider.value.should == 'blah'
+      provider.value.should eq('blah')
       provider.value=('yippee')
       provider.value=('yippee')
       validate_file(<<-EOS
       validate_file(<<-EOS
 # This is a comment
 # This is a comment
@@ -688,7 +688,7 @@ foo = http://192.168.1.1:8080
           :section => 'section2', :setting => 'foo', :value => 'yippee'))
           :section => 'section2', :setting => 'foo', :value => 'yippee'))
       provider = described_class.new(resource)
       provider = described_class.new(resource)
       provider.exists?.should be true
       provider.exists?.should be true
-      provider.value.should == 'http://192.168.1.1:8080'
+      provider.value.should eq('http://192.168.1.1:8080')
       provider.value=('yippee')
       provider.value=('yippee')
       validate_file(<<-EOS
       validate_file(<<-EOS
 [section2]
 [section2]
@@ -728,7 +728,7 @@ foo=bar
                                                    :key_val_separator => '='))
                                                    :key_val_separator => '='))
       provider = described_class.new(resource)
       provider = described_class.new(resource)
       provider.exists?.should be true
       provider.exists?.should be true
-      provider.value.should == 'bar'
+      provider.value.should eq('bar')
       provider.value=('yippee')
       provider.value=('yippee')
       validate_file(<<-EOS
       validate_file(<<-EOS
 [section2]
 [section2]
@@ -755,7 +755,7 @@ foo: bar
                                                    :key_val_separator => ': '))
                                                    :key_val_separator => ': '))
       provider = described_class.new(resource)
       provider = described_class.new(resource)
       provider.exists?.should be true
       provider.exists?.should be true
-      provider.value.should == 'bar'
+      provider.value.should eq('bar')
       provider.value=('yippee')
       provider.value=('yippee')
       validate_file(<<-EOS
       validate_file(<<-EOS
 [section2]
 [section2]
@@ -799,7 +799,7 @@ foo bar
                                                    :key_val_separator => ' '))
                                                    :key_val_separator => ' '))
       provider = described_class.new(resource)
       provider = described_class.new(resource)
       provider.exists?.should be true
       provider.exists?.should be true
-      provider.value.should == 'bar'
+      provider.value.should eq('bar')
       provider.value=('yippee')
       provider.value=('yippee')
       validate_file(<<-EOS
       validate_file(<<-EOS
 [section2]
 [section2]
@@ -840,6 +840,11 @@ master = true
 foo= foovalue2
 foo= foovalue2
 baz=bazvalue
 baz=bazvalue
 url = http://192.168.1.1:8080
 url = http://192.168.1.1:8080
+[section3]
+# com = ment
+uncom = ment
+[section4]
+uncom = ment
 [section:sub]
 [section:sub]
 subby=bar
 subby=bar
     #another comment
     #another comment
@@ -867,6 +872,11 @@ master = true
 foo= foovalue2
 foo= foovalue2
 baz=bazvalue
 baz=bazvalue
 url = http://192.168.1.1:8080
 url = http://192.168.1.1:8080
+[section3]
+# com = ment
+uncom = ment
+[section4]
+uncom = ment
 [section:sub]
 [section:sub]
 subby=bar
 subby=bar
     #another comment
     #another comment
@@ -897,12 +907,16 @@ master = true
 foo= foovalue2
 foo= foovalue2
 baz=bazvalue
 baz=bazvalue
 url = http://192.168.1.1:8080
 url = http://192.168.1.1:8080
+[section3]
+# com = ment
+uncom = ment
+[section4]
+uncom = ment
 [section:sub]
 [section:sub]
 subby=bar
 subby=bar
     #another comment
     #another comment
  ; yet another comment
  ; yet another comment
 
 
- -nonstandard-
 EOS
 EOS
     )
     )
     end
     end
@@ -925,6 +939,11 @@ master = true
 foo= foovalue2
 foo= foovalue2
 baz=bazvalue
 baz=bazvalue
 url = http://192.168.1.1:8080
 url = http://192.168.1.1:8080
+[section3]
+# com = ment
+uncom = ment
+[section4]
+uncom = ment
 [section:sub]
 [section:sub]
 subby=bar
 subby=bar
     #another comment
     #another comment
@@ -955,6 +974,47 @@ master = true
 foo= foovalue2
 foo= foovalue2
 baz=bazvalue
 baz=bazvalue
 url = http://192.168.1.1:8080
 url = http://192.168.1.1:8080
+[section3]
+# com = ment
+uncom = ment
+[section4]
+uncom = ment
+[section:sub]
+subby=bar
+    #another comment
+ ; yet another comment
+
+ -nonstandard-
+   shoes = purple
+      EOS
+      )
+    end
+
+    it "does not remove a section when the last uncommented setting is removed if there are comments" do
+      resource = Puppet::Type::Ini_setting.new(common_params.merge(
+        :section => 'section3',
+        :setting => 'uncom',
+        :ensure => 'absent',
+      ))
+      provider = described_class.new(resource)
+      provider.exists?.should be true
+      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
+[section3]
+# com = ment
+[section4]
+uncom = ment
 [section:sub]
 [section:sub]
 subby=bar
 subby=bar
     #another comment
     #another comment
@@ -965,6 +1025,41 @@ subby=bar
       EOS
       EOS
       )
       )
     end
     end
+
+    it "removes the section when removing the last line in the section" do
+      resource = Puppet::Type::Ini_setting.new(common_params.merge(
+        :section => 'section4',
+        :setting => 'uncom',
+        :ensure => 'absent',
+      ))
+      provider = described_class.new(resource)
+      provider.exists?.should be true
+      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
+[section3]
+# com = ment
+uncom = ment
+[section:sub]
+subby=bar
+    #another comment
+ ; yet another comment
+
+ -nonstandard-
+   shoes = purple
+      EOS
+                   )
+    end
   end
   end
 
 
   context "when dealing with indentation in sections" do
   context "when dealing with indentation in sections" do
@@ -1399,7 +1494,5 @@ foo = bar
                     EOS
                     EOS
                    )
                    )
     end
     end
-
   end
   end
-
 end
 end