Merge pull request #794 from tkishel/MODULES-5003-file_line_does_not_change_multiple_lines_when_one_matches

(MODULES-5003) file_line does not change multiple lines when one matches

Author: tphoney

lib/puppet/provider/file_line/ruby.rb

@@ -1,44 +1,48 @@
 Puppet::Type.type(:file_line).provide(:ruby) do
   def exists?
-    found = true
-    if resource[:replace].to_s != 'true' and count_matches(match_regex) > 0
-      found = true
+    found = false
+    lines_count = 0
+    lines.each do |line|
+      found = line.chomp == resource[:line]
+      if found
+        lines_count += 1
+      end
+    end
+    if resource[:match] == nil
+      found = lines_count > 0
     else
-      lines.find do |line|
-        if resource[:ensure].to_s == 'absent' and resource[:match_for_absence].to_s == 'true'
-          found = line.chomp =~ Regexp.new(resource[:match])
-        else
-          found = line.chomp == resource[:line].chomp
-        end
-        if found == false then
-          break
-        end
+      match_count = count_matches(new_match_regex)
+      if resource[:replace].to_s == 'true'
+        found = lines_count > 0 && lines_count == match_count
+      else
+        found = match_count > 0
       end
     end
     found
   end
 
   def create
-    unless resource[:replace].to_s != 'true' and count_matches(match_regex) > 0
+    unless resource[:replace].to_s != 'true' && count_matches(new_match_regex) > 0
       if resource[:match]
         handle_create_with_match
       elsif resource[:after]
         handle_create_with_after
       else
-        append_line
+        handle_append_line
       end
     end
   end
 
   def destroy
-    if resource[:match_for_absence].to_s == 'true' and resource[:match]
+    if resource[:match_for_absence].to_s == 'true' && resource[:match]
       handle_destroy_with_match
     else
       handle_destroy_line
     end
   end
 
   private
+
   def lines
     # If this type is ever used with very large files, we should
     #  write this in a different way, using a temp
@@ -53,25 +57,34 @@ def lines
     end
   end
 
-  def match_regex
+  def new_after_regex
+    resource[:after] ? Regexp.new(resource[:after]) : nil
+  end
+
+  def new_match_regex
     resource[:match] ? Regexp.new(resource[:match]) : nil
   end
 
+  def count_matches(regex)
+    lines.select{ |line| line.match(regex) }.size
+  end
+
   def handle_create_with_match()
-    regex_after = resource[:after] ? Regexp.new(resource[:after]) : nil
-    match_count = count_matches(match_regex)
+    after_regex = new_after_regex
+    match_regex = new_match_regex
+    match_count = count_matches(new_match_regex)
 
     if match_count > 1 && resource[:multiple].to_s != 'true'
      raise Puppet::Error, "More than one line in file '#{resource[:path]}' matches pattern '#{resource[:match]}'"
     end
 
     File.open(resource[:path], 'w') do |fh|
-      lines.each do |l|
-        fh.puts(match_regex.match(l) ? resource[:line] : l)
-        if (match_count == 0 and regex_after)
-          if regex_after.match(l)
+      lines.each do |line|
+        fh.puts(match_regex.match(line) ? resource[:line] : line)
+        if match_count == 0 && after_regex
+          if after_regex.match(line)
             fh.puts(resource[:line])
-            match_count += 1 #Increment match_count to indicate that the new line has been inserted.
+            match_count += 1 # Increment match_count to indicate that the new line has been inserted.
           end
         end
       end
@@ -83,60 +96,53 @@ def handle_create_with_match()
   end
 
   def handle_create_with_after
-    regex = Regexp.new(resource[:after])
-    count = count_matches(regex)
+    after_regex = new_after_regex
+    after_count = count_matches(after_regex)
 
-    if count > 1 && resource[:multiple].to_s != 'true'
-      raise Puppet::Error, "#{count} lines match pattern '#{resource[:after]}' in file '#{resource[:path]}'.  One or no line must match the pattern."
+    if after_count > 1 && resource[:multiple].to_s != 'true'
+      raise Puppet::Error, "#{after_count} lines match pattern '#{resource[:after]}' in file '#{resource[:path]}'. One or no line must match the pattern."
     end
 
-    File.open(resource[:path], 'w') do |fh|
-      lines.each do |l|
-        fh.puts(l)
-        if regex.match(l) then
+    File.open(resource[:path],'w') do |fh|
+      lines.each do |line|
+        fh.puts(line)
+        if after_regex.match(line)
           fh.puts(resource[:line])
         end
       end
-    end
 
-    if (count == 0) # append the line to the end of the file
-      append_line
+      if (after_count == 0)
+        fh.puts(resource[:line])
+      end
     end
   end
 
-  def count_matches(regex)
-    lines.select{|l| l.match(regex)}.size
-  end
-
   def handle_destroy_with_match
+    match_regex = new_match_regex
     match_count = count_matches(match_regex)
     if match_count > 1 && resource[:multiple].to_s != 'true'
      raise Puppet::Error, "More than one line in file '#{resource[:path]}' matches pattern '#{resource[:match]}'"
     end
 
     local_lines = lines
     File.open(resource[:path],'w') do |fh|
-      fh.write(local_lines.reject{|l| match_regex.match(l) }.join(''))
+      fh.write(local_lines.reject{ |line| match_regex.match(line) }.join(''))
     end
   end
 
   def handle_destroy_line
     local_lines = lines
     File.open(resource[:path],'w') do |fh|
-      fh.write(local_lines.reject{|l| l.chomp == resource[:line] }.join(''))
+      fh.write(local_lines.reject{ |line| line.chomp == resource[:line] }.join(''))
     end
   end
 
-  ##
-  # append the line to the file.
-  #
-  # @api private
-  def append_line
-    File.open(resource[:path], 'w') do |fh|
-      lines.each do |l|
-        fh.puts(l)
+  def handle_append_line
+    File.open(resource[:path],'w') do |fh|
+      lines.each do |line|
+        fh.puts(line)
       end
-      fh.puts resource[:line]
+      fh.puts(resource[:line])
     end
   end
 end

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

@@ -190,7 +190,7 @@
         File.open(@tmpfile, 'w') do |fh|
           fh.write("foo1\nfoo = bar\nfoo2")
         end
-        expect(@provider.exists?).to eql(false)
+        expect(@provider.exists?).to eql(true)
         @provider.create
         expect(File.read(@tmpfile).chomp).to eql("foo1\nfoo = bar\nfoo2")
       end
@@ -387,7 +387,7 @@
       File.open(@tmpfile, 'w') do |fh|
         fh.write("foo1\nfoo\nfoo2")
       end
-      expect(@provider.exists?).to be_nil
+      expect(@provider.exists?).to eql (true)
     end
 
     it 'should remove one line if it matches' do