Refactoring the RAL interface to logging

Previously, the Log class knew a lot about RAL objects,
but now the Logging module is the only one that does.

This greatly simplifies the Log class, which is good,
and means that whatever complexity does need to exist
is directly exposed in the Logging middleware module.

Signed-off-by: Luke Kanies <luke@madstop.com>

6 files changed, 85 insertions, 23 deletions

diff --git a/lib/puppet/parameter.rb b/lib/puppet/parameter.rb
index 0e4071cc5..285a205da 100644
--- a/lib/puppet/parameter.rb
+++ b/lib/puppet/parameter.rb
@@ -205,17 +205,8 @@ class Puppet::Parameter
         set_options(options)
     end
 
-    # Log a message using the resource's log level.
     def log(msg)
-        unless @resource[:loglevel]
-            self.devfail "Parent %s has no loglevel" %
-                @resource.name
-        end
-        Puppet::Util::Log.create(
-            :level => @resource[:loglevel],
-            :message => msg,
-            :source => self
-        )
+        send_log(resource[:loglevel], msg)
     end
 
     # Is this parameter a metaparam?
diff --git a/lib/puppet/util/log.rb b/lib/puppet/util/log.rb
index 50111ab0d..e3930fbe4 100644
--- a/lib/puppet/util/log.rb
+++ b/lib/puppet/util/log.rb
@@ -207,8 +207,7 @@ class Puppet::Util::Log
         @levels.include?(level)
     end
 
-    attr_accessor :level, :message, :time, :remote, :file, :line, :version
-    attr_reader :source
+    attr_accessor :level, :message, :time, :remote, :file, :line, :version, :source
 
     def initialize(args)
         unless args.include?(:level) && args.include?(:message)
diff --git a/lib/puppet/util/log/destinations.rb b/lib/puppet/util/log/destinations.rb
index 89f87dbec..002ca3624 100644
--- a/lib/puppet/util/log/destinations.rb
+++ b/lib/puppet/util/log/destinations.rb
@@ -113,9 +113,10 @@ Puppet::Util::Log.newdesttype :console do
 
     def colorize(level, str)
         case Puppet[:color]
-        when false; str
-        when true, :ansi, "ansi"; console_color(level, str)
+        when true, :ansi, "ansi", "yes"; console_color(level, str)
         when :html, "html"; html_color(level, str)
+        else
+            str
         end
     end
 
diff --git a/lib/puppet/util/logging.rb b/lib/puppet/util/logging.rb
index 13480743c..3af698062 100644
--- a/lib/puppet/util/logging.rb
+++ b/lib/puppet/util/logging.rb
@@ -2,18 +2,41 @@
 require 'puppet/util/log'
 
 module Puppet::Util::Logging
+
+    def send_log(level, message)
+        Puppet::Util::Log.create({:level => level, :source => log_source(), :message => message}.merge(log_metadata))
+    end
+
     # Create a method for each log level.
     Puppet::Util::Log.eachlevel do |level|
         define_method(level) do |args|
             if args.is_a?(Array)
                 args = args.join(" ")
             end
-            Puppet::Util::Log.create(
-                :level => level,
-                :source => self,
-                :message => args
-            )
+            send_log(level, args)
         end
     end
-end
 
+    private
+
+    def is_resource?
+        defined?(Puppet::Type) && is_a?(Puppet::Type)
+    end
+
+    def is_resource_parameter?
+        defined?(Puppet::Parameter) && is_a?(Puppet::Parameter)
+    end
+
+    def log_metadata
+        [:file, :line, :version, :tags].inject({}) do |result, attr|
+            result[attr] = send(attr) if respond_to?(attr)
+            result
+        end
+    end
+
+    def log_source
+        # We need to guard the existence of the constants, since this module is used by the base Puppet module.
+        (is_resource? or is_resource_parameter?) and respond_to?(:path) and return path.to_s
+        return to_s
+    end
+end
diff --git a/spec/unit/parameter.rb b/spec/unit/parameter.rb
index 817b5c6d6..f4473248a 100755
--- a/spec/unit/parameter.rb
+++ b/spec/unit/parameter.rb
@@ -157,5 +157,12 @@ describe Puppet::Parameter do
             @parameter.munge("bar").should == "bar"
         end
     end
-end
 
+    describe "when logging" do
+        it "should use its resource's log level and the provided message" do
+            @resource.expects(:[]).with(:loglevel).returns :notice
+            @parameter.expects(:send_log).with(:notice, "mymessage")
+            @parameter.log "mymessage"
+        end
+    end
+end
diff --git a/spec/unit/util/logging.rb b/spec/unit/util/logging.rb
index 39edbb47e..d6c38fd4c 100755
--- a/spec/unit/util/logging.rb
+++ b/spec/unit/util/logging.rb
@@ -19,6 +19,13 @@ describe Puppet::Util::Logging do
         end
     end
 
+    it "should have a method for sending a log with a specified log level" do
+        @logger.expects(:to_s).returns "I'm a string!"
+        Puppet::Util::Log.expects(:create).with { |args| args[:source] == "I'm a string!" and args[:level] == "loglevel" and args[:message] == "mymessage" }
+
+        @logger.send_log "loglevel", "mymessage"
+    end
+
     describe "when sending a log" do
         it "should use the Log's 'create' entrance method" do
             Puppet::Util::Log.expects(:create)
@@ -26,12 +33,35 @@ describe Puppet::Util::Logging do
             @logger.notice "foo"
         end
 
-        it "should send itself as the log source" do
-            Puppet::Util::Log.expects(:create).with { |args| args[:source].equal?(@logger) }
+        it "should send itself converted to a string as the log source" do
+            @logger.expects(:to_s).returns "I'm a string!"
+            Puppet::Util::Log.expects(:create).with { |args| args[:source] == "I'm a string!" }
 
             @logger.notice "foo"
         end
 
+        it "should use the path of any provided resource type" do
+            resource = Puppet::Type.type(:mount).new :name => "foo"
+
+            resource.expects(:path).returns "/path/to/mount".to_sym
+
+            Puppet::Util::Log.expects(:create).with { |args| args[:source] == "/path/to/mount" }
+
+            resource.notice "foo"
+        end
+
+        it "should use the path of any provided resource parameter" do
+            resource = Puppet::Type.type(:mount).new :name => "foo"
+
+            param = resource.parameter(:name)
+
+            param.expects(:path).returns "/path/to/param".to_sym
+
+            Puppet::Util::Log.expects(:create).with { |args| args[:source] == "/path/to/param" }
+
+            param.notice "foo"
+        end
+
         it "should send the provided argument as the log message" do
             Puppet::Util::Log.expects(:create).with { |args| args[:message] == "foo" }
 
@@ -43,5 +73,16 @@ describe Puppet::Util::Logging do
 
             @logger.notice ["foo", "bar", "baz"]
         end
+
+        [:file, :line, :version, :tags].each do |attr|
+            it "should include #{attr} if available" do
+                @logger.metaclass.send(:attr_accessor, attr)
+
+                @logger.send(attr.to_s + "=", "myval")
+
+                Puppet::Util::Log.expects(:create).with { |args| args[attr] == "myval" }
+                @logger.notice "foo"
+            end
+        end
     end
 end