Fixes #2483 - Log only copies metadata from RAL objects

We were previously trying to figure out what data
was available based on what methods existed, but
that caught a different method profile from
modules.

This fixes it so we only look for this data from
Puppet::Type or Puppet::Parameter instances.

I had to add the ability to skip data that's
not available, since File's 'ensure' parameter
doesn't have 'file' data, I assume because of
the metaprogramming we do around the 'file' value
for 'ensure'.  It's a workaround for now, and there's
a test in there to verify it, anyway.

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

2 files changed, 36 insertions, 16 deletions

diff --git a/lib/puppet/util/log.rb b/lib/puppet/util/log.rb
index 6edc7f482..d6d3ba018 100644
--- a/lib/puppet/util/log.rb
+++ b/lib/puppet/util/log.rb
@@ -515,18 +515,10 @@ class Puppet::Util::Log
         # We can't just check for whether it responds to :path, because
         # plenty of providers respond to that in their normal function.
         if (source.is_a?(Puppet::Type) or source.is_a?(Puppet::Parameter)) and source.respond_to?(:path)
-            @source = source.path
+            set_source_from_ral(source)
         else
             @source = source.to_s
         end
-        if source.respond_to?(:tags)
-            source.tags.each { |t| tag(t) }
-        end
-
-        [:file, :line, :version].each do |param|
-            next unless source.respond_to?(param)
-            send(param.to_s + "=", source.send(param))
-        end
     end
 
     def to_report
@@ -536,6 +528,19 @@ class Puppet::Util::Log
     def to_s
         return @message
     end
+
+    private
+
+    def set_source_from_ral(source)
+        @source = source.path
+
+        source.tags.each { |t| tag(t) }
+
+        [:file, :line, :version].each do |param|
+            next unless source.respond_to?(param) and value = source.send(param)
+            send(param.to_s + "=", value)
+        end
+    end
 end
 
 # This is for backward compatibility from when we changed the constant to Puppet::Util::Log
diff --git a/spec/unit/util/log.rb b/spec/unit/util/log.rb
index 9d1b4108e..70309e456 100755
--- a/spec/unit/util/log.rb
+++ b/spec/unit/util/log.rb
@@ -112,7 +112,7 @@ describe Puppet::Util::Log do
             report.should be_include(log.time.to_s)
         end
 
-        describe "when setting the source" do
+        describe "when setting the source as a RAL object" do
             it "should tag itself with any tags the source has" do
                 source = Puppet::Type.type(:file).new :path => "/foo/bar"
                 log = Puppet::Util::Log.new(:level => "notice", :message => :foo, :source => source)
@@ -121,6 +121,16 @@ describe Puppet::Util::Log do
                 end
             end
 
+            it "should copy over any version information" do
+                catalog = Puppet::Resource::Catalog.new
+                catalog.version = 25
+                source = Puppet::Type.type(:file).new :path => "/foo/bar"
+                catalog.add_resource source
+
+                log = Puppet::Util::Log.new(:level => "notice", :message => :foo, :source => source)
+                log.version.should == 25
+            end
+
             it "should copy over any file and line information" do
                 source = Puppet::Type.type(:file).new :path => "/foo/bar"
                 source.file = "/my/file"
@@ -130,14 +140,19 @@ describe Puppet::Util::Log do
                 log.line.should == 50
             end
 
-            it "should copy over any version information" do
-                catalog = Puppet::Resource::Catalog.new
-                catalog.version = 25
-                source = Puppet::Type.type(:file).new :path => "/foo/bar"
-                catalog.add_resource source
+            it "should not fail when RAL objects don't actually support all of the metadata" do
+                file = Puppet::Type.type(:file).new :path => "/foo/bar", :ensure => :file
+                source = file.property(:ensure)
+                log = Puppet::Util::Log.new(:level => "notice", :message => :foo, :source => source)
+                log.file.should be_nil
+            end
+        end
 
+        describe "when setting the source as a non-RAL object" do
+            it "should not try to copy over file, version, line, or tag information" do
+                source = Puppet::Module.new("foo")
+                source.expects(:file).never
                 log = Puppet::Util::Log.new(:level => "notice", :message => :foo, :source => source)
-                log.version.should == 25
             end
         end
     end