From 221277e8522b42bf170fded6ea23dfc526703b07 Mon Sep 17 00:00:00 2001
From: Dan Prince <dprince@redhat.com>
Date: Thu, 29 Aug 2013 12:19:16 -0400
Subject: Update file_line resource to support 'after'.

When adding new lines to a file the 'after' option can be useful
when you need to insert file lines into the middle of a file.

This is particularly helpful when using file_line with sectioned
config files.

NOTE: the after option only works when adding new lines. If you are
updating an existing (matched) line it will simply modify it in place.
This assumes it was in the right place to begin with.
---
 lib/puppet/provider/file_line/ruby.rb            | 28 +++++++++++++++++++++---
 lib/puppet/type/file_line.rb                     |  4 ++++
 spec/unit/puppet/provider/file_line/ruby_spec.rb | 18 +++++++++++++++
 3 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/lib/puppet/provider/file_line/ruby.rb b/lib/puppet/provider/file_line/ruby.rb
index 3cb9f6e..7ddb39f 100644
--- a/lib/puppet/provider/file_line/ruby.rb
+++ b/lib/puppet/provider/file_line/ruby.rb
@@ -49,10 +49,32 @@ Puppet::Type.type(:file_line).provide(:ruby) do
   end
 
   def handle_create_without_match
-    File.open(resource[:path], 'a') do |fh|
-      fh.puts resource[:line]
+
+    regex = resource[:after] ? Regexp.new(resource[:after]) : nil
+    after_count = File.exists?(resource[:path]) ? lines.select { |l| regex.match(l) }.size : 0
+    if after_count > 1 then
+     raise Puppet::Error, "More than one line in file '#{resource[:path]}' matches after pattern '#{resource[:after]}'"
     end
-  end
 
+    if (after_count == 0)
+
+      File.open(resource[:path], 'a') do |fh|
+        fh.puts resource[:line]
+      end
+
+    else
+
+      File.open(resource[:path], 'w') do |fh|
+        lines.each do |l|
+          fh.puts(l)
+          if regex.match(l) then
+            fh.puts(resource[:line])
+          end
+        end
+      end
+
+    end
+
+  end
 
 end
diff --git a/lib/puppet/type/file_line.rb b/lib/puppet/type/file_line.rb
index 14946bb..323fc4c 100644
--- a/lib/puppet/type/file_line.rb
+++ b/lib/puppet/type/file_line.rb
@@ -42,6 +42,10 @@ Puppet::Type.newtype(:file_line) do
     newvalues(true, false)
   end
 
+  newparam(:after) do
+    desc 'An optional value used to specify the line after which we will add any new lines. (Existing lines are added in place)'
+  end
+
   newparam(:line) do
     desc 'The line to be appended to the file located by the path parameter.'
   end
diff --git a/spec/unit/puppet/provider/file_line/ruby_spec.rb b/spec/unit/puppet/provider/file_line/ruby_spec.rb
index 648c05b..c8575ab 100644
--- a/spec/unit/puppet/provider/file_line/ruby_spec.rb
+++ b/spec/unit/puppet/provider/file_line/ruby_spec.rb
@@ -80,6 +80,24 @@ describe provider_class do
       File.read(@tmpfile).chomp.should eql("foo1\nfoo = bar\nfoo2\nfoo = bar")
     end
 
+    it 'should replace all lines that matches with after' do
+      @resource = Puppet::Type::File_line.new(
+          {
+           :name => 'foo',
+           :path => @tmpfile,
+           :line => 'inserted = line',
+           :after => '^foo1',
+          }
+      )
+      @provider = provider_class.new(@resource)
+      File.open(@tmpfile, 'w') do |fh|
+        fh.write("foo1\nfoo = blah\nfoo2\nfoo = baz")
+      end
+      @provider.exists?.should be_nil
+      @provider.create
+      File.read(@tmpfile).chomp.should eql("foo1\ninserted = line\nfoo = blah\nfoo2\nfoo = baz")
+    end
+
     it 'should raise an error with invalid values' do
       expect {
         @resource = Puppet::Type::File_line.new(
-- 
cgit v1.2.3


From 948be0bb99374c699d79eb11c00366a6e2d2f977 Mon Sep 17 00:00:00 2001
From: Jeff McCune <jeff@puppetlabs.com>
Date: Thu, 29 Aug 2013 16:27:27 -0700
Subject: (maint) Improve the tests and readability of file_line

Without this patch the implementation of the file_line provider is a bit
convoluted with respect to the newly introduced "after" parameter.

This patch addresses the problem by separating out the concerns of each
case into their own methods of handling the behavior with the match
parameter, handling the behavior with the after parameter, or simply
appending the line.
---
 lib/puppet/provider/file_line/ruby.rb            |  41 ++---
 spec/unit/puppet/provider/file_line/ruby_spec.rb | 208 ++++++++++++++---------
 2 files changed, 149 insertions(+), 100 deletions(-)

diff --git a/lib/puppet/provider/file_line/ruby.rb b/lib/puppet/provider/file_line/ruby.rb
index 7ddb39f..94e7fac 100644
--- a/lib/puppet/provider/file_line/ruby.rb
+++ b/lib/puppet/provider/file_line/ruby.rb
@@ -1,5 +1,4 @@
 Puppet::Type.type(:file_line).provide(:ruby) do
-
   def exists?
     lines.find do |line|
       line.chomp == resource[:line].chomp
@@ -8,9 +7,11 @@ Puppet::Type.type(:file_line).provide(:ruby) do
 
   def create
     if resource[:match]
-      handle_create_with_match()
+      handle_create_with_match
+    elsif resource[:after]
+      handle_create_with_after
     else
-      handle_create_without_match()
+      append_line
     end
   end
 
@@ -48,22 +49,13 @@ Puppet::Type.type(:file_line).provide(:ruby) do
     end
   end
 
-  def handle_create_without_match
-
-    regex = resource[:after] ? Regexp.new(resource[:after]) : nil
-    after_count = File.exists?(resource[:path]) ? lines.select { |l| regex.match(l) }.size : 0
-    if after_count > 1 then
-     raise Puppet::Error, "More than one line in file '#{resource[:path]}' matches after pattern '#{resource[:after]}'"
-    end
-
-    if (after_count == 0)
-
-      File.open(resource[:path], 'a') do |fh|
-        fh.puts resource[:line]
-      end
+  def handle_create_with_after
+    regex = Regexp.new(resource[:after])
 
-    else
+    count = lines.count {|l| l.match(regex)}
 
+    case count
+    when 1 # find the line to put our line after
       File.open(resource[:path], 'w') do |fh|
         lines.each do |l|
           fh.puts(l)
@@ -72,9 +64,20 @@ Puppet::Type.type(:file_line).provide(:ruby) do
           end
         end
       end
-
+    when 0 # append the line to the end of the file
+      append_line
+    else
+      raise Puppet::Error, "#{count} lines match pattern '#{resource[:after]}' in file '#{resource[:path]}'.  One or no line must match the pattern."
     end
-
   end
 
+  ##
+  # append the line to the file.
+  #
+  # @api private
+  def append_line
+    File.open(resource[:path], 'a') do |fh|
+      fh.puts resource[:line]
+    end
+  end
 end
diff --git a/spec/unit/puppet/provider/file_line/ruby_spec.rb b/spec/unit/puppet/provider/file_line/ruby_spec.rb
index c8575ab..65b5d20 100644
--- a/spec/unit/puppet/provider/file_line/ruby_spec.rb
+++ b/spec/unit/puppet/provider/file_line/ruby_spec.rb
@@ -3,33 +3,36 @@ require 'tempfile'
 provider_class = Puppet::Type.type(:file_line).provider(:ruby)
 describe provider_class do
   context "when adding" do
-    before :each do
-      # TODO: these should be ported over to use the PuppetLabs spec_helper
-      #  file fixtures once the following pull request has been merged:
-      # https://github.com/puppetlabs/puppetlabs-stdlib/pull/73/files
+    let :tmpfile do
       tmp = Tempfile.new('tmp')
-      @tmpfile = tmp.path
+      path = tmp.path
       tmp.close!
-      @resource = Puppet::Type::File_line.new(
-        {:name => 'foo', :path => @tmpfile, :line => 'foo'}
+      path
+    end
+    let :resource do
+      Puppet::Type::File_line.new(
+        {:name => 'foo', :path => tmpfile, :line => 'foo'}
       )
-      @provider = provider_class.new(@resource)
     end
+    let :provider do
+      provider_class.new(resource)
+    end
+
     it 'should detect if the line exists in the file' do
-      File.open(@tmpfile, 'w') do |fh|
+      File.open(tmpfile, 'w') do |fh|
         fh.write('foo')
       end
-      @provider.exists?.should be_true
+      provider.exists?.should be_true
     end
     it 'should detect if the line does not exist in the file' do
-      File.open(@tmpfile, 'w') do |fh|
+      File.open(tmpfile, 'w') do |fh|
         fh.write('foo1')
       end
-      @provider.exists?.should be_nil
+      provider.exists?.should be_nil
     end
     it 'should append to an existing file when creating' do
-      @provider.create
-      File.read(@tmpfile).chomp.should == 'foo'
+      provider.create
+      File.read(tmpfile).chomp.should == 'foo'
     end
   end
 
@@ -52,89 +55,132 @@ describe provider_class do
       @provider = provider_class.new(@resource)
     end
 
-    it 'should raise an error if more than one line matches, and should not have modified the file' do
-      File.open(@tmpfile, 'w') do |fh|
-        fh.write("foo1\nfoo=blah\nfoo2\nfoo=baz")
+    describe 'using match' do
+      it 'should raise an error if more than one line matches, and should not have modified the file' do
+        File.open(@tmpfile, 'w') do |fh|
+          fh.write("foo1\nfoo=blah\nfoo2\nfoo=baz")
+        end
+        @provider.exists?.should be_nil
+        expect { @provider.create }.to raise_error(Puppet::Error, /More than one line.*matches/)
+        File.read(@tmpfile).should eql("foo1\nfoo=blah\nfoo2\nfoo=baz")
       end
-      @provider.exists?.should be_nil
-      expect { @provider.create }.to raise_error(Puppet::Error, /More than one line.*matches/)
-      File.read(@tmpfile).should eql("foo1\nfoo=blah\nfoo2\nfoo=baz")
-    end
 
-    it 'should replace all lines that matches' do
-      @resource = Puppet::Type::File_line.new(
+      it 'should replace all lines that matches' do
+        @resource = Puppet::Type::File_line.new(
           {
-           :name => 'foo',
-           :path => @tmpfile,
-           :line => 'foo = bar',
-           :match => '^foo\s*=.*$',
-           :multiple => true
+            :name => 'foo',
+            :path => @tmpfile,
+            :line => 'foo = bar',
+            :match => '^foo\s*=.*$',
+            :multiple => true
           }
-      )
-      @provider = provider_class.new(@resource)
-      File.open(@tmpfile, 'w') do |fh|
-        fh.write("foo1\nfoo=blah\nfoo2\nfoo=baz")
+        )
+        @provider = provider_class.new(@resource)
+        File.open(@tmpfile, 'w') do |fh|
+          fh.write("foo1\nfoo=blah\nfoo2\nfoo=baz")
+        end
+        @provider.exists?.should be_nil
+        @provider.create
+        File.read(@tmpfile).chomp.should eql("foo1\nfoo = bar\nfoo2\nfoo = bar")
       end
-      @provider.exists?.should be_nil
-      @provider.create
-      File.read(@tmpfile).chomp.should eql("foo1\nfoo = bar\nfoo2\nfoo = bar")
-    end
 
-    it 'should replace all lines that matches with after' do
-      @resource = Puppet::Type::File_line.new(
-          {
-           :name => 'foo',
-           :path => @tmpfile,
-           :line => 'inserted = line',
-           :after => '^foo1',
-          }
-      )
-      @provider = provider_class.new(@resource)
-      File.open(@tmpfile, 'w') do |fh|
-        fh.write("foo1\nfoo = blah\nfoo2\nfoo = baz")
+      it 'should raise an error with invalid values' do
+        expect {
+          @resource = Puppet::Type::File_line.new(
+            {
+              :name => 'foo',
+              :path => @tmpfile,
+              :line => 'foo = bar',
+              :match => '^foo\s*=.*$',
+              :multiple => 'asgadga'
+            }
+          )
+        }.to raise_error(Puppet::Error, /Invalid value "asgadga"\. Valid values are true, false\./)
+      end
+
+      it 'should replace a line that matches' do
+        File.open(@tmpfile, 'w') do |fh|
+          fh.write("foo1\nfoo=blah\nfoo2")
+        end
+        @provider.exists?.should be_nil
+        @provider.create
+        File.read(@tmpfile).chomp.should eql("foo1\nfoo = bar\nfoo2")
+      end
+      it 'should add a new line if no lines match' do
+        File.open(@tmpfile, 'w') do |fh|
+          fh.write("foo1\nfoo2")
+        end
+        @provider.exists?.should be_nil
+        @provider.create
+        File.read(@tmpfile).should eql("foo1\nfoo2\nfoo = bar\n")
+      end
+      it 'should do nothing if the exact line already exists' do
+        File.open(@tmpfile, 'w') do |fh|
+          fh.write("foo1\nfoo = bar\nfoo2")
+        end
+        @provider.exists?.should be_true
+        @provider.create
+        File.read(@tmpfile).chomp.should eql("foo1\nfoo = bar\nfoo2")
       end
-      @provider.exists?.should be_nil
-      @provider.create
-      File.read(@tmpfile).chomp.should eql("foo1\ninserted = line\nfoo = blah\nfoo2\nfoo = baz")
     end
 
-    it 'should raise an error with invalid values' do
-      expect {
-        @resource = Puppet::Type::File_line.new(
+    describe 'using after' do
+      let :resource do
+        Puppet::Type::File_line.new(
           {
-           :name => 'foo',
-           :path => @tmpfile,
-           :line => 'foo = bar',
-           :match => '^foo\s*=.*$',
-           :multiple => 'asgadga'
+            :name  => 'foo',
+            :path  => @tmpfile,
+            :line  => 'inserted = line',
+            :after => '^foo1',
           }
         )
-      }.to raise_error(Puppet::Error, /Invalid value "asgadga"\. Valid values are true, false\./)
-    end
+      end
 
-    it 'should replace a line that matches' do
-      File.open(@tmpfile, 'w') do |fh|
-        fh.write("foo1\nfoo=blah\nfoo2")
+      let :provider do
+        provider_class.new(resource)
       end
-      @provider.exists?.should be_nil
-      @provider.create
-      File.read(@tmpfile).chomp.should eql("foo1\nfoo = bar\nfoo2")
-    end
-    it 'should add a new line if no lines match' do
-      File.open(@tmpfile, 'w') do |fh|
-        fh.write("foo1\nfoo2")
+
+      context 'with one line matching the after expression' do
+        before :each do
+          File.open(@tmpfile, 'w') do |fh|
+            fh.write("foo1\nfoo = blah\nfoo2\nfoo = baz")
+          end
+        end
+
+        it 'inserts the specified line after the line matching the "after" expression' do
+          provider.create
+          File.read(@tmpfile).chomp.should eql("foo1\ninserted = line\nfoo = blah\nfoo2\nfoo = baz")
+        end
       end
-      @provider.exists?.should be_nil
-      @provider.create
-      File.read(@tmpfile).should eql("foo1\nfoo2\nfoo = bar\n")
-    end
-    it 'should do nothing if the exact line already exists' do
-      File.open(@tmpfile, 'w') do |fh|
-        fh.write("foo1\nfoo = bar\nfoo2")
+
+      context 'with two lines matching the after expression' do
+        before :each do
+          File.open(@tmpfile, 'w') do |fh|
+            fh.write("foo1\nfoo = blah\nfoo2\nfoo1\nfoo = baz")
+          end
+        end
+
+        it 'errors out stating "One or no line must match the pattern"' do
+          expect { provider.create }.to raise_error(Puppet::Error, /One or no line must match the pattern/)
+        end
+      end
+
+      context 'with no lines matching the after expression' do
+        let :content do
+          "foo3\nfoo = blah\nfoo2\nfoo = baz\n"
+        end
+
+        before :each do
+          File.open(@tmpfile, 'w') do |fh|
+            fh.write(content)
+          end
+        end
+
+        it 'appends the specified line to the file' do
+          provider.create
+          File.read(@tmpfile).should eq(content << resource[:line] << "\n")
+        end
       end
-      @provider.exists?.should be_true
-      @provider.create
-      File.read(@tmpfile).chomp.should eql("foo1\nfoo = bar\nfoo2")
     end
   end
 
-- 
cgit v1.2.3