Fixing #292 (A bug in tagmail that causes any tag other than 'all' to fail)

and #277 (tagmail report missing To: header).

#292 was weird because the messages just didn't have the tags at all.
The problem was that states didn't have tags, yet states were the source
of nearly all messages.  So, I added tags to the states, and included
the state name in the tag list.  Also, types were not including the type
name in the tag list, so I added that.  And, of course, a few unit tests
to check it all.



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

7 files changed, 71 insertions, 6 deletions

diff --git a/lib/puppet/log.rb b/lib/puppet/log.rb
index 7a08ba57d..ee825e406 100644
--- a/lib/puppet/log.rb
+++ b/lib/puppet/log.rb
@@ -5,6 +5,7 @@ module Puppet
     # expected that that will be the most common log destination.  Supports
     # multiple destinations, one of which is a remote server.
 	class Log
+        include Puppet::Util
         PINK=""
         GREEN=""
         YELLOW=""
@@ -501,7 +502,7 @@ module Puppet
         end
 
         def tagged?(tag)
-            @tags.include?(tag)
+            @tags.detect { |t| t.to_s == tag.to_s }
         end
 
 		def to_report
diff --git a/lib/puppet/reports/tagmail.rb b/lib/puppet/reports/tagmail.rb
index ba929b7fc..477246e0b 100644
--- a/lib/puppet/reports/tagmail.rb
+++ b/lib/puppet/reports/tagmail.rb
@@ -20,6 +20,8 @@ Puppet::Server::Report.newreport(:tagmail) do |report|
         return
     end
 
+    p report
+
     # Load the config file
     tags = {}
     File.readlines(Puppet[:tagmap]).each do |line|
@@ -83,6 +85,7 @@ Puppet::Server::Report.newreport(:tagmail) do |report|
                     # We need to open a separate process for every set of email addresses
                     IO.popen(Puppet[:sendmail] + " " + emails.join(" "), "w") do |p|
                         p.puts "From: #{Puppet[:reportfrom]}"
+                        p.puts "To: %s" % emails.join(', ')
                         p.puts "Subject: Puppet Report for %s" % report.host
 
                         p.puts messages
@@ -100,3 +103,5 @@ Puppet::Server::Report.newreport(:tagmail) do |report|
         end
     end
 end
+
+# $Id$
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 1f8af8e82..0e0628202 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -1766,6 +1766,8 @@ class Type < Puppet::Element
                 self.warning "Ignoring tag %s of type %s" % [tag.inspect, tag.class]
             end
         end
+
+        @tags << self.class.name unless @tags.include?(self.class.name)
     end
 
     # Figure out of any of the specified tags apply to this object.  This is an
diff --git a/lib/puppet/type/state.rb b/lib/puppet/type/state.rb
index e372db6db..e5aa3b1b6 100644
--- a/lib/puppet/type/state.rb
+++ b/lib/puppet/type/state.rb
@@ -363,6 +363,19 @@ class State < Puppet::Parameter
         self.set
     end
 
+    # The states need to return tags so that logs correctly collect them.
+    def tags
+        unless defined? @tags
+            @tags = []
+            # This might not be true in testing
+            if @parent.respond_to? :tags
+                @tags = @parent.tags
+            end
+            @tags << self.name
+        end
+        @tags
+    end
+
     def to_s
         return "%s(%s)" % [@parent.name,self.name]
     end
diff --git a/test/other/log.rb b/test/other/log.rb
index 750014e05..cb049b30d 100644
--- a/test/other/log.rb
+++ b/test/other/log.rb
@@ -129,23 +129,36 @@ class TestLog < Test::Unit::TestCase
 
         file = Puppet.type(:file).create(
             :path => path,
-            :check => [:owner, :group, :mode, :checksum]
+            :check => [:owner, :group, :mode, :checksum],
+            :ensure => :file
         )
         file.tags = %w{this is a test}
 
+        state = file.state(:ensure)
+        assert(state, "Did not get state")
         log = nil
         assert_nothing_raised {
             log = Puppet::Log.new(
                 :level => :info,
-                :source => file,
+                :source => state,
                 :message => "A test message"
             )
         }
 
-        assert(log.tags, "Got no tags")
+        # Now yaml and de-yaml it, and test again
+        yamllog = YAML.load(YAML.dump(log))
+
+        {:log => log, :yaml => yamllog}.each do |type, msg|
+            assert(msg.tags, "Got no tags")
+
+            msg.tags.each do |tag|
+                assert(msg.tagged?(tag), "Was not tagged with %s" % tag)
+            end
+
+            assert_equal(msg.tags, state.tags, "Tags were not equal")
+            assert_equal(msg.source, state.path, "Source was not set correctly")
+        end
 
-        assert_equal(log.tags, file.tags, "Tags were not equal")
-        assert_equal(log.source, file.path, "Source was not set correctly")
     end
 
     # Verify that we can pass strings that match printf args
diff --git a/test/types/state.rb b/test/types/state.rb
index c3ea0965f..8aad4c101 100644
--- a/test/types/state.rb
+++ b/test/types/state.rb
@@ -119,6 +119,27 @@ class TestState < Test::Unit::TestCase
         assert_equal(:fake_other, ret,
                      "Event did not get returned correctly")
     end
+
+    def test_tags
+        obj = "yay"
+        metaobj = class << obj; self; end
+
+        metaobj.send(:attr_accessor, :tags)
+
+        tags = [:some, :tags, :for, :testing]
+        obj.tags = tags
+
+        stateklass = newstate
+ 
+        inst = nil
+        assert_nothing_raised do
+            inst = stateklass.new(:parent => obj)
+        end
+
+        assert_nothing_raised do
+            assert_equal(tags + [inst.name], inst.tags)
+        end
+    end
 end
 
 # $Id$
diff --git a/test/types/type.rb b/test/types/type.rb
index f8785fd3b..929fc616e 100644
--- a/test/types/type.rb
+++ b/test/types/type.rb
@@ -745,6 +745,16 @@ end
         assert(file1, "Did not create first file")
         assert_nil(file2, "Incorrectly created second file")
     end
+
+    def test_tags
+        obj = Puppet::Type.type(:file).create(:path => tempfile())
+
+        tags = [:some, :test, :tags]
+
+        obj.tags = tags
+
+        assert_equal(tags + [:file], obj.tags)
+    end
 end
 
 # $Id$