From 81d7d35fd78917ee1a5e66cb7459ea341bc452ea Mon Sep 17 00:00:00 2001 From: tkishel Date: Thu, 13 Jul 2017 15:16:04 -0700 Subject: (MODULES-5003) file_line_does_not_change_multiple_lines_when_one_matches The exists? method is called to determine the need to call the create and destroy methods. When the ensure, match, and multiple attributes are defined, the exists? method needs to return false unless all the lines that match the match attribute equal the line attribute. The first commit is minimal, and implements this change. The second commit normalizes the code, which I needed for comprehension. --- lib/puppet/provider/file_line/ruby.rb | 27 +++++++++++++----------- spec/unit/puppet/provider/file_line/ruby_spec.rb | 4 ++-- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/lib/puppet/provider/file_line/ruby.rb b/lib/puppet/provider/file_line/ruby.rb index 1d27424..aa99986 100644 --- a/lib/puppet/provider/file_line/ruby.rb +++ b/lib/puppet/provider/file_line/ruby.rb @@ -1,18 +1,21 @@ 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(match_regex) + if resource[:replace].to_s == 'true' + found = lines_count > 0 && lines_count == match_count + else + found = match_count > 0 end end found diff --git a/spec/unit/puppet/provider/file_line/ruby_spec.rb b/spec/unit/puppet/provider/file_line/ruby_spec.rb index dcf8509..ab6edf9 100755 --- a/spec/unit/puppet/provider/file_line/ruby_spec.rb +++ b/spec/unit/puppet/provider/file_line/ruby_spec.rb @@ -190,7 +190,7 @@ describe provider_class, :unless => Puppet::Util::Platform.windows? do 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 @@ describe provider_class, :unless => Puppet::Util::Platform.windows? do 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 -- cgit v1.2.3 From 4d764068ffe456366a9c823e15581e9a9649898e Mon Sep 17 00:00:00 2001 From: tkishel Date: Thu, 13 Jul 2017 15:30:54 -0700 Subject: (MODULES-5003) file_line_does_not_change_multiple_lines_when_one_matches The exists? method is called to determine the need to call the create and destroy methods. When the ensure, match, and multiple attributes are defined, the exists? method needs to return false unless all the lines that match the match attribute equal the line attribute. The first commit is minimal, and implements this change. The second commit normalizes the code, which I needed for comprehension. --- lib/puppet/provider/file_line/ruby.rb | 79 ++++++++++++++++++----------------- 1 file changed, 41 insertions(+), 38 deletions(-) diff --git a/lib/puppet/provider/file_line/ruby.rb b/lib/puppet/provider/file_line/ruby.rb index aa99986..2f6c8ef 100644 --- a/lib/puppet/provider/file_line/ruby.rb +++ b/lib/puppet/provider/file_line/ruby.rb @@ -11,7 +11,7 @@ Puppet::Type.type(:file_line).provide(:ruby) do if resource[:match] == nil found = lines_count > 0 else - match_count = count_matches(match_regex) + match_count = count_matches(new_match_regex) if resource[:replace].to_s == 'true' found = lines_count > 0 && lines_count == match_count else @@ -22,19 +22,19 @@ Puppet::Type.type(:file_line).provide(:ruby) do 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 @@ -42,6 +42,7 @@ Puppet::Type.type(:file_line).provide(:ruby) do 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 @@ -56,25 +57,34 @@ Puppet::Type.type(:file_line).provide(:ruby) do 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 @@ -86,32 +96,29 @@ Puppet::Type.type(:file_line).provide(:ruby) do 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]}'" @@ -119,27 +126,23 @@ Puppet::Type.type(:file_line).provide(:ruby) do 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 -- cgit v1.2.3