[#4110] Wrap Type#retrieve calls for backwards compatibility

This patch introduces Type#retrieve_resource as a wrapper for
Type#resource, to coerce the return value from legacy types from Hash to
Resource.

Signed-off-by: Jesse Wolfe <jes5199@gmail.com>

6 files changed, 19 insertions, 13 deletions

diff --git a/lib/puppet/transaction/resource_harness.rb b/lib/puppet/transaction/resource_harness.rb
index ae38bcb66..848ba7b9a 100644
--- a/lib/puppet/transaction/resource_harness.rb
+++ b/lib/puppet/transaction/resource_harness.rb
@@ -38,7 +38,7 @@ class Puppet::Transaction::ResourceHarness
     end
 
     def changes_to_perform(status, resource)
-        current = resource.retrieve
+        current = resource.retrieve_resource
 
         cache resource, :checked, Time.now
 
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 6e553d463..57caf1d4f 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -743,6 +743,14 @@ class Type
         result
     end
 
+    def retrieve_resource
+        resource = retrieve
+        if resource.is_a? Hash
+            resource = Resource.new(type, title, :parameters => resource)
+        end
+        resource
+    end
+
     # Get a hash of the current properties.  Returns a hash with
     # the actual property instance as the key and the current value
     # as the, um, value.
@@ -1924,10 +1932,8 @@ class Type
     def to_trans(ret = true)
         trans = TransObject.new(self.title, self.class.name)
 
-        values = retrieve()
+        values = retrieve_resource
         values.each do |name, value|
-            # sometimes we get symbols and sometimes we get Properties
-            # I think it's a bug, but I can't find it. ~JW
             name = name.name if name.respond_to? :name
             trans[name] = value
         end
diff --git a/lib/puppet/type/mount.rb b/lib/puppet/type/mount.rb
index e79bc0a4a..b2185b608 100755
--- a/lib/puppet/type/mount.rb
+++ b/lib/puppet/type/mount.rb
@@ -67,7 +67,7 @@ module Puppet
 
             def syncothers
                 # We have to flush any changes to disk.
-                currentvalues = @resource.retrieve
+                currentvalues = @resource.retrieve_resource
 
                 # Determine if there are any out-of-sync properties.
                 oos = @resource.send(:properties).find_all do |prop|
diff --git a/lib/puppet/type/resources.rb b/lib/puppet/type/resources.rb
index 136b691c3..00808461b 100644
--- a/lib/puppet/type/resources.rb
+++ b/lib/puppet/type/resources.rb
@@ -131,7 +131,7 @@ Puppet::Type.newtype(:resources) do
         return true unless self[:unless_system_user]
 
         resource[:audit] = :uid
-        current_values = resource.retrieve
+        current_values = resource.retrieve_resource
 
         if system_users().include?(resource[:name])
             return false
diff --git a/spec/unit/type_spec.rb b/spec/unit/type_spec.rb
index 54fb2978c..70597f748 100755
--- a/spec/unit/type_spec.rb
+++ b/spec/unit/type_spec.rb
@@ -368,11 +368,11 @@ describe Puppet::Type do
         it "should fail if its provider is unsuitable" do
             @resource = Puppet::Type.type(:mount).new(:name => "foo", :fstype => "bar", :pass => 1, :ensure => :present)
             @resource.provider.class.expects(:suitable?).returns false
-            lambda { @resource.retrieve }.should raise_error(Puppet::Error)
+            lambda { @resource.retrieve_resource }.should raise_error(Puppet::Error)
         end
 
         it "should return a Puppet::Resource instance with its type and title set appropriately" do
-            result = @resource.retrieve
+            result = @resource.retrieve_resource
             result.should be_instance_of(Puppet::Resource)
             result.type.should == "Mount"
             result.title.should == "foo"
@@ -381,11 +381,11 @@ describe Puppet::Type do
         it "should set the name of the returned resource if its own name and title differ" do
             @resource[:name] = "my name"
             @resource.title = "other name"
-            @resource.retrieve[:name].should == "my name"
+            @resource.retrieve_resource[:name].should == "my name"
         end
 
         it "should provide a value for all set properties" do
-            values = @resource.retrieve
+            values = @resource.retrieve_resource
             [:ensure, :fstype, :pass].each { |property| values[property].should_not be_nil }
         end
 
@@ -396,13 +396,13 @@ describe Puppet::Type do
         it "should not call retrieve on non-ensure properties if the resource is absent and should consider the property absent" do
             @resource.property(:ensure).expects(:retrieve).returns :absent
             @resource.property(:fstype).expects(:retrieve).never
-            @resource.retrieve[:fstype].should == :absent
+            @resource.retrieve_resource[:fstype].should == :absent
         end
 
         it "should include the result of retrieving each property's current value if the resource is present" do
             @resource.property(:ensure).expects(:retrieve).returns :present
             @resource.property(:fstype).expects(:retrieve).returns 15
-            @resource.retrieve[:fstype] == 15
+            @resource.retrieve_resource[:fstype] == 15
         end
     end
 
diff --git a/test/ral/type/mailalias.rb b/test/ral/type/mailalias.rb
index 5d5023a8c..ff0e62e09 100755
--- a/test/ral/type/mailalias.rb
+++ b/test/ral/type/mailalias.rb
@@ -34,7 +34,7 @@ class TestMailAlias < Test::Unit::TestCase
     # This isn't much of a test, but then, it's not much of a type.
     def test_recipient_arrays
         resource = @type.new(:name => "luke", :recipient => "yay", :target => tempfile)
-        values = resource.retrieve
+        values = resource.retrieve_resource
         assert_equal(:absent, values[:recipient])
         resource.property(:recipient).expects(:set).with(%w{yay})
         assert_nothing_raised("Could not sync mailalias") do