splitting the tagmail report into multiple methods and adding test code

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@2122 980ebf18-57e1-0310-9a29-db15c13687c0

5 files changed, 179 insertions, 46 deletions

diff --git a/lib/puppet/reports/tagmail.rb b/lib/puppet/reports/tagmail.rb
index a7b3ffd3b..fef01bb39 100644
--- a/lib/puppet/reports/tagmail.rb
+++ b/lib/puppet/reports/tagmail.rb
@@ -41,30 +41,61 @@ Puppet::Server::Report.newreport(:tagmail) do
         webservers that are not also from mailservers to ``httpadmins@domain.com``.
         "
 
-    def process
-        unless FileTest.exists?(Puppet[:tagmap])
-            Puppet.notice "Cannot send tagmail report; no tagmap file %s" %
-                Puppet[:tagmap]
-            return
-        end
+    # Find all matching messages.
+    def match(taglists)
+        reports = []
+        taglists.each do |emails, pos, neg|
+            # First find all of the messages matched by our positive tags
+            messages = nil
+            if pos.include?("all")
+                messages = self.logs
+            else
+                # Find all of the messages that are tagged with any of our
+                # tags.
+                messages = self.logs.find_all do |log|
+                    pos.detect { |tag| log.tagged?(tag) }
+                end
+            end
 
-        # Load the config file
-        taglists = {}
-        File.readlines(Puppet[:tagmap]).each do |line|
+            # Now go through and remove any messages that match our negative tags
+            messages = messages.reject do |log|
+                if neg.detect do |tag| log.tagged?(tag) end
+                    true
+                end
+            end
+
+            if messages.empty?
+                Puppet.info "No messages to report to %s" % emails.join(",")
+                next
+            else
+                reports << [emails, messages.collect { |m| m.to_report }.join("\n")]
+            end
+        end
+        
+        return reports
+    end
+    
+    # Load the config file
+    def parse(text)
+        taglists = []
+        text.split("\n").each do |line|
             taglist = emails = nil
             case line.chomp
             when /^\s*#/: next
             when /^\s*$/: next
             when /^\s*(.+)\s*:\s*(.+)\s*$/:
                 taglist = $1
-                emails = $2
+                emails = $2.sub(/#.*$/,'')
             else
                 raise ArgumentError, "Invalid tagmail config file"
             end
 
             pos = []
             neg = []
-            taglist.split(/\s*,\s*/).each do |tag|
+            taglist.sub(/\s+$/,'').split(/\s*,\s*/).each do |tag|
+                unless tag =~ /^!?[-\w]+$/
+                    raise ArgumentError, "Invalid tag %s" % tag.inspect
+                end
                 case tag
                 when /^\w+/: pos << tag
                 when /^!\w+/: neg << tag.sub("!", '')
@@ -74,44 +105,30 @@ Puppet::Server::Report.newreport(:tagmail) do
             end
 
             # Now split the emails
-            emails = emails.split(/\s*,\s*/)
-            taglists[emails] = [pos, neg]
+            emails = emails.sub(/\s+$/,'').split(/\s*,\s*/)
+            taglists << [emails, pos, neg]
         end
+        return taglists
+    end
 
-        # Now find any appropriately tagged messages.
-        reports = {}
-        taglists.each do |emails, tags|
-            pos, neg = tags
-
-            # First find all of the messages matched by our positive tags
-            messages = nil
-            if pos.include?("all")
-                messages = self.logs
-            else
-                # Find all of the messages that are tagged with any of our
-                # tags.
-                messages = self.logs.find_all do |log|
-                    pos.detect { |tag| log.tagged?(tag) }
-                end
-            end
-
-            # Now go through and remove any messages that match our negative tags
-            messages.reject! do |log|
-                if neg.detect do |tag| log.tagged?(tag) end
-                    true
-                end
-            end
-
-            if messages.empty?
-                Puppet.info "No messages to report to %s" % emails.join(",")
-                next
-            else
-                reports[emails] = messages.collect { |m| m.to_report }.join("\n")
-            end
+    # Process the report.  This just calls the other associated messages.
+    def process
+        unless FileTest.exists?(Puppet[:tagmap])
+            Puppet.notice "Cannot send tagmail report; no tagmap file %s" %
+                Puppet[:tagmap]
+            return
         end
 
-        # Let's fork for the sending of the email, since you never know what might
-        # happen.
+        taglists = parse(File.read(Puppet[:tagmap]))
+        
+        # Now find any appropriately tagged messages.
+        reports = match(taglists)
+
+        send(reports)
+    end
+    
+    # Send the email reports.
+    def send(reports)
         pid = fork do
             if Puppet[:smtpserver] != "none"
                 begin
@@ -137,7 +154,7 @@ Puppet::Server::Report.newreport(:tagmail) do
                             p.puts "From: #{Puppet[:reportfrom]}"
                             p.puts "To: %s" % emails.join(', ')
                             p.puts "Subject: Puppet Report for %s" % self.host
-                           p.puts "To: " + emails.join(", ")
+                            p.puts "To: " + emails.join(", ")
 
                             p.puts messages
                         end
@@ -154,6 +171,7 @@ Puppet::Server::Report.newreport(:tagmail) do
             end
         end
 
+        # Don't bother waiting for the pid to return.
         Process.detach(pid)
     end
 end
diff --git a/lib/puppet/transaction/report.rb b/lib/puppet/transaction/report.rb
index 2a81251a3..769866ef0 100644
--- a/lib/puppet/transaction/report.rb
+++ b/lib/puppet/transaction/report.rb
@@ -6,6 +6,11 @@ require 'puppet'
 # in the transaction.
 class Puppet::Transaction::Report
     attr_accessor :logs, :metrics, :time, :host
+    
+    def <<(msg)
+        @logs << msg
+        return self
+    end
 
     def initialize
         @metrics = {}
diff --git a/test/data/reports/tagmail_failers.conf b/test/data/reports/tagmail_failers.conf
new file mode 100644
index 000000000..d116b5fc7
--- /dev/null
+++ b/test/data/reports/tagmail_failers.conf
@@ -0,0 +1,3 @@
+tag:
+: abuse@domain.com
+invalid!tag: abuse@domain.com
diff --git a/test/data/reports/tagmail_passers.conf b/test/data/reports/tagmail_passers.conf
new file mode 100644
index 000000000..6cadf03f8
--- /dev/null
+++ b/test/data/reports/tagmail_passers.conf
@@ -0,0 +1,30 @@
+# A comment
+# or maybe two
+# plus some blank lines
+    # with some blanks plus a comment
+
+# a simple tag report
+one: abuse@domain.com
+
+# with weird spacing
+ one : abuse@domain.com 
+
+# with multiple tags
+one, two: abuse@domain.com
+
+# again with the weird syntax
+ one , two : abuse@domain.com
+
+# Some negations
+one, !two: abuse@domain.com
+
+# some oddly-formatted tags
+one, two-three, !four-five, !six: abuse@domain.com
+
+# multiple addresses
+one, two: abuse@domain.com, testing@domain.com
+
+# and with weird spacing
+one, two: abuse@domain.com , testing@domain.com    
+
+# and a trailing comment
diff --git a/test/other/report.rb b/test/other/report.rb
index e53cb0050..1ecebf307 100755
--- a/test/other/report.rb
+++ b/test/other/report.rb
@@ -151,6 +151,83 @@ class TestReports < Test::Unit::TestCase
     else
     $stderr.puts "Install RRD for metric reporting tests"
     end
+    
+    def test_tagmail_parsing
+        report = Object.new
+        report.extend(Puppet::Server::Report.report(:tagmail))
+        
+        passers = File.join(datadir, "reports", "tagmail_passers.conf")
+        assert(FileTest.exists?(passers), "no passers file %s" % passers)
+        
+        File.readlines(passers).each do |line|
+            assert_nothing_raised("Could not parse %s" % line.inspect) do
+                report.parse(line)
+            end
+        end
+        
+        # Now make sure the failers fail
+        failers = File.join(datadir, "reports", "tagmail_failers.conf")
+        assert(FileTest.exists?(failers), "no failers file %s" % failers)
+        
+        File.readlines(failers).each do |line|
+            assert_raise(ArgumentError, "Parsed %s" % line.inspect) do
+                report.parse(line)
+            end
+        end
+    end
+    
+    def test_tagmail_parsing_results
+        report = Object.new
+        report.extend(Puppet::Server::Report.report(:tagmail))
+        # Now test a few specific lines to make sure we get the results we want
+        {
+            "tag: abuse@domain.com" => [%w{abuse@domain.com}, %w{tag}, []],
+            "tag, other: abuse@domain.com" => [%w{abuse@domain.com}, %w{tag other}, []],
+            "tag-other: abuse@domain.com" => [%w{abuse@domain.com}, %w{tag-other}, []],
+            "tag, !other: abuse@domain.com" => [%w{abuse@domain.com}, %w{tag}, %w{other}],
+            "tag, !other, one, !two: abuse@domain.com" => [%w{abuse@domain.com}, %w{tag one}, %w{other two}],
+            "tag: abuse@domain.com, other@domain.com" => [%w{abuse@domain.com other@domain.com}, %w{tag}, []]
+            
+        }.each do |line, results|
+            assert_nothing_raised("Failed to parse %s" % line.inspect) do
+                assert_equal(results, report.parse(line).shift, "line %s returned incorrect results %s" % [line.inspect, results.inspect])
+            end
+        end
+    end
+    
+    def test_tagmail_matching
+        report = Puppet::Transaction::Report.new
+        Puppet::Log.close
+        [%w{one}, %w{one two}, %w{one two three}, %w{one two three four}].each do |tags|
+            log = Puppet::Log.new(:level => :notice, :message => tags.join(" "), :tags => tags)
+            
+            report << log
+        end
+        
+        list = report.logs.collect { |l| l.to_report }
+        
+        report.extend(Puppet::Server::Report.report(:tagmail))
+        
+        {
+            [%w{abuse@domain.com}, %w{all}, []] => list,
+            [%w{abuse@domain.com}, %w{all}, %w{three}] => list[0..1],
+            [%w{abuse@domain.com}, %w{one}, []] => list,
+            [%w{abuse@domain.com}, %w{two}, []] => list[1..3],
+            [%w{abuse@domain.com}, %w{two}, %w{three}] => list[1..1],
+            [%w{abuse@domain.com}, %w{}, %w{one}] => nil
+        }.each do |args, expected|
+            results = nil
+            assert_nothing_raised("Could not match with %s" % args.inspect) do
+                results = report.match([args])
+            end
+            
+            if expected
+                assert_equal([args[0], expected.join("\n")], results[0], "did get correct results for %s" % args.inspect)
+            else
+                assert_nil(results[0], "got a report for %s" % args.inspect)
+            end
+        end
+    end
 end
 
 # $Id$