From c57dab4903a6a23fb14dc79c76efea989e694460 Mon Sep 17 00:00:00 2001 From: Chris Price Date: Fri, 17 Aug 2012 04:48:28 -0700 Subject: Add support for "global" section at beginning of file This commit does the following: * Fixes a bug in ExternalIterator * Adds support for a "global" section before the first named section at the beginning of the INI file * Improves test coverage --- lib/puppet/util/external_iterator.rb | 8 +- lib/puppet/util/ini_file.rb | 21 +-- spec/unit/puppet/provider/ini_setting/ruby_spec.rb | 154 +++++++++++++++++---- spec/unit/puppet/util/external_iterator_spec.rb | 35 +++++ spec/unit/puppet/util/ini_file_spec.rb | 77 +++++++++-- 5 files changed, 249 insertions(+), 46 deletions(-) create mode 100644 spec/unit/puppet/util/external_iterator_spec.rb diff --git a/lib/puppet/util/external_iterator.rb b/lib/puppet/util/external_iterator.rb index 67b3375..45c0fa4 100644 --- a/lib/puppet/util/external_iterator.rb +++ b/lib/puppet/util/external_iterator.rb @@ -3,7 +3,7 @@ module Util class ExternalIterator def initialize(coll) @coll = coll - @cur_index = 0 + @cur_index = -1 end def next @@ -17,7 +17,11 @@ module Util private def item_at(index) - [@coll[index], index] + if @coll.length > index + [@coll[index], index] + else + [nil, nil] + end end end end diff --git a/lib/puppet/util/ini_file.rb b/lib/puppet/util/ini_file.rb index 9fd08ad..8629db0 100644 --- a/lib/puppet/util/ini_file.rb +++ b/lib/puppet/util/ini_file.rb @@ -43,18 +43,13 @@ module Util def save File.open(@path, 'w') do |fh| - first_section = @sections_hash[@section_names[0]] - first_section.start_line == nil ? start_line = 0 : start_line = first_section.start_line - (0..start_line - 1).each do |line_num| - fh.puts(lines[line_num]) - end @section_names.each do |name| section = @sections_hash[name] - if (section.start_line.nil?) + if section.start_line.nil? fh.puts("\n[#{section.name}]") - else + elsif ! section.end_line.nil? (section.start_line..section.end_line).each do |line_num| fh.puts(lines[line_num]) end @@ -76,7 +71,13 @@ module Util def parse_file line_iter = create_line_iter + + # We always create a "global" section at the beginning of the file, for + # anything that appears before the first named section. + section = read_section('', 0, line_iter) + add_section(section) line, line_num = line_iter.next + while line if (match = SECTION_REGEX.match(line)) section = read_section(match[1], line_num, line_iter) @@ -88,13 +89,15 @@ module Util def read_section(name, start_line, line_iter) settings = {} + end_line_num = nil while true line, line_num = line_iter.peek - if (line.nil? or match = SECTION_REGEX.match(line)) - return Section.new(name, start_line, line_num - 1, settings) + if (line_num.nil? or match = SECTION_REGEX.match(line)) + return Section.new(name, start_line, end_line_num, settings) elsif (match = SETTING_REGEX.match(line)) settings[match[1]] = match[2] end + end_line_num = line_num line_iter.next 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 72a5da0..d26c78d 100644 --- a/spec/unit/puppet/provider/ini_setting/ruby_spec.rb +++ b/spec/unit/puppet/provider/ini_setting/ruby_spec.rb @@ -7,24 +7,12 @@ describe provider_class do let(:tmpfile) { tmpfilename("ini_setting_test") } let(:emptyfile) { tmpfilename("ini_setting_test_empty") } - let(:orig_content) { - <<-EOS -# This is a comment -[section1] -; This is also a comment -foo=foovalue -bar = barvalue -master = true -[section2] - -foo= foovalue2 -baz=bazvalue -url = http://192.168.1.1:8080 - #another comment - ; yet another comment - EOS -} + let(:common_params) { { + :title => 'ini_setting_ensure_present_test', + :path => tmpfile, + :section => 'section2', + } } def validate_file(expected_content,tmpfile = tmpfile) File.read(tmpfile).should == expected_content @@ -41,11 +29,24 @@ url = http://192.168.1.1:8080 end context "when ensuring that a setting is present" do - let(:common_params) { { - :title => 'ini_setting_ensure_present_test', - :path => tmpfile, - :section => 'section2', - } } + let(:orig_content) { + <<-EOS +# This is a comment +[section1] +; This is also a comment +foo=foovalue + +bar = barvalue +master = true +[section2] + +foo= foovalue2 +baz=bazvalue +url = http://192.168.1.1:8080 + #another comment + ; yet another comment + EOS + } it "should add a missing setting to the correct section" do resource = Puppet::Type::Ini_setting.new(common_params.merge( @@ -75,7 +76,7 @@ yahoo = yippee it "should modify an existing setting with a different value" do resource = Puppet::Type::Ini_setting.new(common_params.merge( - :setting => 'baz', :value => 'bazvalue2')) + :setting => 'baz', :value => 'bazvalue2')) provider = described_class.new(resource) provider.exists?.should == false provider.create @@ -100,7 +101,7 @@ url = http://192.168.1.1:8080 it "should be able to handle settings with non alphanumbering settings " do resource = Puppet::Type::Ini_setting.new(common_params.merge( - :setting => 'url', :value => 'http://192.168.0.1:8080')) + :setting => 'url', :value => 'http://192.168.0.1:8080')) provider = described_class.new(resource) provider.exists?.should == false provider.create @@ -126,7 +127,7 @@ url = http://192.168.0.1:8080 it "should recognize an existing setting with the specified value" do resource = Puppet::Type::Ini_setting.new(common_params.merge( - :setting => 'baz', :value => 'bazvalue')) + :setting => 'baz', :value => 'bazvalue')) provider = described_class.new(resource) provider.exists?.should == true end @@ -179,6 +180,109 @@ setting1 = hellowworld provider.create end + end + context "when dealing with a global section" do + let(:orig_content) { + <<-EOS +# This is a comment +foo=blah +[section2] +foo = http://192.168.1.1:8080 + ; yet another comment + EOS + } + + + it "should add a missing setting if it doesn't exist" do + resource = Puppet::Type::Ini_setting.new(common_params.merge( + :section => '', :setting => 'bar', :value => 'yippee')) + provider = described_class.new(resource) + provider.exists?.should == false + provider.create + validate_file(<<-EOS +# This is a comment +foo=blah +bar = yippee +[section2] +foo = http://192.168.1.1:8080 + ; yet another comment + EOS + ) + end + + it "should modify an existing setting with a different value" do + resource = Puppet::Type::Ini_setting.new(common_params.merge( + :section => '', :setting => 'foo', :value => 'yippee')) + provider = described_class.new(resource) + provider.exists?.should == false + provider.create + validate_file(<<-EOS +# This is a comment +foo = yippee +[section2] +foo = http://192.168.1.1:8080 + ; yet another comment + EOS + ) + end + + it "should recognize an existing setting with the specified value" do + resource = Puppet::Type::Ini_setting.new(common_params.merge( + :section => '', :setting => 'foo', :value => 'blah')) + provider = described_class.new(resource) + provider.exists?.should == true + end + end + + context "when the first line of the file is a section" do + let(:orig_content) { + <<-EOS +[section2] +foo = http://192.168.1.1:8080 + EOS + } + + it "should be able to add a global setting" do + resource = Puppet::Type::Ini_setting.new(common_params.merge( + :section => '', :setting => 'foo', :value => 'yippee')) + provider = described_class.new(resource) + provider.exists?.should == false + provider.create + validate_file(<<-EOS +foo = yippee +[section2] +foo = http://192.168.1.1:8080 + EOS + ) + end + + it "should modify an existing setting" do + resource = Puppet::Type::Ini_setting.new(common_params.merge( + :section => 'section2', :setting => 'foo', :value => 'yippee')) + provider = described_class.new(resource) + provider.exists?.should == false + provider.create + validate_file(<<-EOS +[section2] +foo = yippee + EOS + ) + end + + it "should add a new setting" do + resource = Puppet::Type::Ini_setting.new(common_params.merge( + :section => 'section2', :setting => 'bar', :value => 'baz')) + provider = described_class.new(resource) + provider.exists?.should == false + provider.create + validate_file(<<-EOS +[section2] +foo = http://192.168.1.1:8080 +bar = baz + EOS + ) + end end + end diff --git a/spec/unit/puppet/util/external_iterator_spec.rb b/spec/unit/puppet/util/external_iterator_spec.rb new file mode 100644 index 0000000..92b17af --- /dev/null +++ b/spec/unit/puppet/util/external_iterator_spec.rb @@ -0,0 +1,35 @@ +require 'spec_helper' +require 'puppet/util/external_iterator' + +describe Puppet::Util::ExternalIterator do + let(:subject) { Puppet::Util::ExternalIterator.new(["a", "b", "c"]) } + + context "#next" do + it "should iterate over the items" do + subject.next.should == ["a", 0] + subject.next.should == ["b", 1] + subject.next.should == ["c", 2] + end + end + + context "#peek" do + it "should return the 0th item repeatedly" do + subject.peek.should == ["a", 0] + subject.peek.should == ["a", 0] + end + + it "should not advance the iterator, but should reflect calls to #next" do + subject.peek.should == ["a", 0] + subject.peek.should == ["a", 0] + subject.next.should == ["a", 0] + subject.peek.should == ["b", 1] + subject.next.should == ["b", 1] + subject.peek.should == ["c", 2] + subject.next.should == ["c", 2] + subject.peek.should == [nil, nil] + subject.next.should == [nil, nil] + end + end + + +end diff --git a/spec/unit/puppet/util/ini_file_spec.rb b/spec/unit/puppet/util/ini_file_spec.rb index f2c6e20..f30c8cd 100644 --- a/spec/unit/puppet/util/ini_file_spec.rb +++ b/spec/unit/puppet/util/ini_file_spec.rb @@ -2,8 +2,16 @@ require 'spec_helper' require 'puppet/util/ini_file' describe Puppet::Util::IniFile do + let(:subject) { Puppet::Util::IniFile.new("/my/ini/file/path") } + + before :each do + File.should_receive(:file?).with("/my/ini/file/path") { true } + described_class.should_receive(:readlines).once.with("/my/ini/file/path") do + sample_content + end + end + context "when parsing a file" do - let(:subject) { Puppet::Util::IniFile.new("/my/ini/file/path") } let(:sample_content) { template = <<-EOS # This is a comment @@ -22,19 +30,14 @@ baz=bazvalue template.split("\n") } - before :each do - File.should_receive(:file?).with("/my/ini/file/path") { true } - described_class.should_receive(:readlines).once.with("/my/ini/file/path") do - sample_content - end - end - it "should parse the correct number of sections" do - subject.section_names.length.should == 2 + # there is always a "global" section, so our count should be 3. + subject.section_names.length.should == 3 end it "should parse the correct section_names" do - subject.section_names.should == ["section1", "section2"] + # there should always be a "global" section named "" at the beginning of the list + subject.section_names.should == ["", "section1", "section2"] end it "should expose settings for sections" do @@ -45,4 +48,58 @@ baz=bazvalue end end + + context "when parsing a file whose first line is a section" do + let(:sample_content) { + template = <<-EOS +[section1] +; This is a comment +foo=foovalue + EOS + template.split("\n") + } + + it "should parse the correct number of sections" do + # there is always a "global" section, so our count should be 2. + subject.section_names.length.should == 2 + end + + it "should parse the correct section_names" do + # there should always be a "global" section named "" at the beginning of the list + subject.section_names.should == ["", "section1"] + end + + it "should expose settings for sections" do + subject.get_value("section1", "foo").should == "foovalue" + end + + end + + context "when parsing a file with a 'global' section" do + let(:sample_content) { + template = <<-EOS +foo = bar +[section1] +; This is a comment +foo=foovalue + EOS + template.split("\n") + } + + it "should parse the correct number of sections" do + # there is always a "global" section, so our count should be 2. + subject.section_names.length.should == 2 + end + + it "should parse the correct section_names" do + # there should always be a "global" section named "" at the beginning of the list + subject.section_names.should == ["", "section1"] + end + + it "should expose settings for sections" do + subject.get_value("", "foo").should == "bar" + subject.get_value("section1", "foo").should == "foovalue" + end + + end end \ No newline at end of file -- cgit v1.2.3