Merge pull request #274 from cjepeway/MODULES-1821-empty-sections

MODULES-1821 support empty sections

.gitignore

@@ -22,3 +22,4 @@
 /convert_report.txt
 /update_report.txt
 .DS_Store
+.byebug_history

lib/puppet/provider/ini_setting/ruby.rb

@@ -13,7 +13,7 @@ def self.instances
     # the catalog.
     raise(Puppet::Error, 'Ini_settings only support collecting instances when a file path is hard coded') unless respond_to?(:file_path)
 
-    # figure out what to do about the seperator
+    # figure out what to do about the separator
     ini_file  = Puppet::Util::IniFile.new(file_path, '=')
     resources = []
     ini_file.section_names.each do |section_name|
@@ -31,10 +31,11 @@ def self.instances
   end
 
   def self.namevar(section_name, setting)
-    "#{section_name}/#{setting}"
+    setting.nil? ? section_name : "#{section_name}/#{setting}"
   end
 
   def exists?
+    setting.nil? && ini_file.section_names.include?(section) || !ini_file.get_value(section, setting).nil?
     if ini_file.section?(section)
       !ini_file.get_value(section, setting).nil?
     elsif resource.parameters.keys.include?(:force_new_section_creation) && !resource[:force_new_section_creation]
@@ -54,7 +55,11 @@ def exists?
   end
 
   def create
-    ini_file.set_value(section, setting, separator, resource[:value])
+    if setting.nil? && resource[:value].nil?
+      ini_file.set_value(section)
+    else
+      ini_file.set_value(section, setting, separator, resource[:value])
+    end
     ini_file.save
     @ini_file = nil
   end
@@ -70,7 +75,11 @@ def value
   end
 
   def value=(_value)
-    ini_file.set_value(section, setting, separator, resource[:value])
+    if setting.nil? && resource[:value].nil?
+      ini_file.set_value(section)
+    else
+      ini_file.set_value(section, setting, separator, resource[:value])
+    end
     ini_file.save
   end
 

lib/puppet/util/ini_file.rb

@@ -24,7 +24,7 @@ def initialize(path, key_val_separator = ' = ', section_prefix = '[', section_su
       @key_val_separator = key_val_separator
       @section_names = []
       @sections_hash = {}
-      parse_file if File.file?(@path)
+      parse_file
     end
 
     def section_regex
@@ -66,6 +66,8 @@ def get_value(section_name, setting)
 
     def set_value(*args) # rubocop:disable Style/AccessorMethodName : Recomended alternative is a common value name
       case args.size
+      when 1
+        section_name = args[0]
       when 3
         # Backwards compatible set_value function, See MODULES-5172
         (section_name, setting, value) = args
@@ -108,7 +110,7 @@ def set_value(*args) # rubocop:disable Style/AccessorMethodName : Recomended alt
         # was modified.
         section_index = @section_names.index(section_name)
         increment_section_line_numbers(section_index + 1)
-      else
+      elsif !setting.nil? || !value.nil?
         section.set_additional_setting(setting, value)
       end
     end
@@ -129,9 +131,17 @@ def remove_setting(section_name, setting)
       # was modified.
       section_index = @section_names.index(section_name)
       decrement_section_line_numbers(section_index + 1)
+
+      return unless section.empty?
+      # By convention, it's time to remove this newly emptied out section
+      lines.delete_at(section.start_line)
+      decrement_section_line_numbers(section_index + 1)
+      @section_names.delete_at(section_index)
+      @sections_hash.delete(section.name)
     end
 
     def save
+      global_empty = @sections_hash[''].empty? && @sections_hash[''].additional_settings.empty?
       File.open(@path, 'w') do |fh|
         @section_names.each_index do |index|
           name = @section_names[index]
@@ -142,16 +152,15 @@ def save
           whitespace_buffer = []
 
           if section.new_section? && !section.global?
-            fh.puts("\n#{@section_prefix}#{section.name}#{@section_suffix}")
+            if index == 1 && !global_empty || index > 1
+              fh.puts('')
+            end
+
+            fh.puts("#{@section_prefix}#{section.name}#{@section_suffix}")
           end
 
           unless section.new_section?
-            # don't add empty sections
-            if section.empty? && !section.global?
-              next
-            end
-
-            # write all of the pre-existing settings
+            # write all of the pre-existing lines
             (section.start_line..section.end_line).each do |line_num|
               line = lines[line_num]
 
@@ -220,17 +229,25 @@ def parse_file
 
     def read_section(name, start_line, line_iter)
       settings = {}
-      end_line_num = nil
+      end_line_num = start_line
       min_indentation = nil
+      empty = true
       loop do
         line, line_num = line_iter.peek
-        return Section.new(name, start_line, end_line_num, settings, min_indentation) if line_num.nil? || @section_regex.match(line)
+        if line_num.nil? || @section_regex.match(line)
+          # the global section always exists, even when it's empty;
+          # when it's empty, we must be sure it's thought of as new,
+          # which is signalled with a nil ending line
+          end_line_num = nil if name == '' && empty
+          return Section.new(name, start_line, end_line_num, settings, min_indentation)
+        end
         if (match = @setting_regex.match(line))
           settings[match[2]] = match[4]
           indentation = match[1].length
           min_indentation = [indentation, min_indentation || indentation].min
         end
         end_line_num = line_num
+        empty = false
         line_iter.next
       end
     end
@@ -269,7 +286,7 @@ def self.readlines(path) # rubocop:disable Lint/IneffectiveAccessModifier : Atte
       #  file; for now assuming that this type is only used on
       #  small-ish config files that can fit into memory without
       #  too much trouble.
-      File.readlines(path)
+      File.file?(path) ? File.readlines(path) : []
     end
 
     # This utility method scans through the lines for a section looking for

lib/puppet/util/ini_file/section.rb

@@ -45,8 +45,10 @@ def existing_setting?(setting_name)
       @existing_settings.key?(setting_name)
     end
 
+    # the global section is empty whenever it's new;
+    # other sections are empty when they have no lines
     def empty?
-      start_line == end_line
+      global? ? new_section? : start_line == end_line
     end
 
     def update_existing_setting(setting_name, value)

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

@@ -641,58 +641,99 @@ def self.file_path
       validate_file(expected_content_sixteen, tmpfile)
     end
 
-    validate_one = '
-[section1]
+    it 'adds a new empty section' do
+      resource = Puppet::Type::Ini_setting.new(common_params.merge(section: 'section'))
+      provider = described_class.new(resource)
+      expect(provider.exists?).to be false
+      provider.create
+      validate_file(orig_content + "\n[section]\n", tmpfile)
+    end
+
+    it 'is able to handle variables of any type' do
+      resource = Puppet::Type::Ini_setting.new(common_params.merge(section: 'section1', setting: 'master', value: true))
+      provider = described_class.new(resource)
+      expect(provider.value).to eq('true')
+    end
+  end
+
+  context 'when no sections exist' do
+    let(:orig_content) do
+      ''
+    end
+
+    validate_zero = "[section]\n"
+
+    it 'adds an empty section' do
+      resource = Puppet::Type::Ini_setting.new(common_params.merge(section: 'section', path: emptyfile))
+      provider = described_class.new(resource)
+      expect(provider.exists?).to be false
+      provider.create
+      validate_file(validate_zero, emptyfile)
+    end
+
+    validate_one = '[section1]
 setting1 = hellowworld
 '
-    it 'adds a new section if no sections exists' do
+    it 'adds a new section' do
       resource = Puppet::Type::Ini_setting.new(common_params.merge(section: 'section1', setting: 'setting1', value: 'hellowworld', path: emptyfile))
       provider = described_class.new(resource)
       expect(provider.exists?).to be false
       provider.create
       validate_file(validate_one, emptyfile)
     end
 
-    validate_two = '
--section1-
+    validate_two = '-section1-
 setting1 = hellowworld
 '
-    it 'adds a new section with pre/suffix if no sections exists' do
+    it 'adds a new section with pre/suffix' do
       resource = Puppet::Type::Ini_setting.new(common_params.merge(section: 'section1', setting: 'setting1', value: 'hellowworld', path: emptyfile, section_prefix: '-', section_suffix: '-'))
       provider = described_class.new(resource)
       expect(provider.exists?).to be false
       provider.create
       validate_file(validate_two, emptyfile)
     end
 
-    validate_three = '
-[section:subsection]
+    validate_three = '[section:subsection]
 setting1 = hellowworld
 '
-    it 'adds a new section with colon if no sections exists' do
+    it 'adds a new section with colon' do
       resource = Puppet::Type::Ini_setting.new(common_params.merge(section: 'section:subsection', setting: 'setting1', value: 'hellowworld', path: emptyfile))
       provider = described_class.new(resource)
       expect(provider.exists?).to be false
       provider.create
       validate_file(validate_three, emptyfile)
     end
 
-    validate_four = '
--section:subsection-
+    validate_four = '-section:subsection-
 setting1 = hellowworld
 '
-    it 'adds a new section with pre/suffix with colon if no sections exists' do
+    it 'adds a new section with pre/suffix with colon' do
       resource = Puppet::Type::Ini_setting.new(common_params.merge(section: 'section:subsection', setting: 'setting1', value: 'hellowworld', path: emptyfile, section_prefix: '-', section_suffix: '-'))
       provider = described_class.new(resource)
       expect(provider.exists?).to be false
       provider.create
       validate_file(validate_four, emptyfile)
     end
+  end
 
-    it 'is able to handle variables of any type' do
-      resource = Puppet::Type::Ini_setting.new(common_params.merge(section: 'section1', setting: 'master', value: true))
-      provider = described_class.new(resource)
-      expect(provider.value).to eq('true')
+  context 'when only an empty section exists' do
+    let(:orig_content) do
+      "[section]\n"
+    end
+
+    it 'adds a new setting' do
+      expected = orig_content
+      { 'section' => { 'first' => 1 } }.each_pair do |section, settings|
+        settings.each_pair do |setting, value|
+          resource = Puppet::Type::Ini_setting.new(common_params.merge(section: section, setting: setting, value: value))
+          provider = described_class.new(resource)
+          expect(provider.exists?).to be false
+          # byebug
+          provider.create
+          expected += "#{setting} = #{value}\n"
+        end
+      end
+      validate_file(expected, tmpfile)
     end
   end