Modified the behaviour of resource-level 'retrieve' -- it only

calls 'retrieve' on each property if the resource exists.

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

CHANGELOG

@@ -1,4 +1,7 @@
 0.24.x
+    Modified the behaviour of resource-level 'retrieve' -- it only
+    calls 'retrieve' on each property if the resource exists.
+
     Fixed #1622 - Users and their groups should again add in one transaction
 
     Fixed #1610 - Raise "Filebucketed" messages to Notice priority

lib/puppet/type.rb

@@ -906,14 +906,24 @@ class Type
     
     # get a hash of the current properties.  
     def currentpropvalues(override_value = nil)
-        # it's important to use the method here, as it follows the order
-        # in which they're defined in the object
+        # it's important to use the 'properties' method here, as it follows the order
+        # in which they're defined in the object.  It also guarantees that 'ensure'
+        # is the first property, which is important for skipping 'retrieve' on
+        # all the properties if the resource is absent.
+        ensure_state = false
         return properties().inject({}) { | prophash, property|
-                   prophash[property] = override_value.nil? ? 
-                                          property.retrieve : 
-                                             override_value
-                   prophash
-               }
+            if property.name == :ensure
+                ensure_state = property.retrieve
+                prophash[property] = ensure_state
+            else
+                if ensure_state == :absent
+                    prophash[property] = :absent
+                else
+                    prophash[property] = property.retrieve
+                end
+            end
+            prophash
+        }
     end
 
     # Are we running in noop mode?

spec/unit/type.rb

@@ -2,28 +2,62 @@
 
 require File.dirname(__FILE__) + '/../spec_helper'
 
-describe Puppet::Type, " when in a configuration" do
-    before do
-        @catalog = Puppet::Node::Catalog.new
-        @container = Puppet::Type.type(:component).create(:name => "container")
-        @one = Puppet::Type.type(:file).create(:path => "/file/one")
-        @two = Puppet::Type.type(:file).create(:path => "/file/two")
-        @catalog.add_resource @container
-        @catalog.add_resource @one
-        @catalog.add_resource @two
-        @catalog.add_edge @container, @one
-        @catalog.add_edge @container, @two
+describe Puppet::Type do
+    describe "when retrieving current properties" do
+        # Use 'mount' as an example, because it doesn't override 'retrieve'
+        before do
+            @resource = Puppet::Type.type(:mount).create(:name => "foo", :fstype => "bar", :pass => 1, :ensure => :present)
+            @properties = {}
+        end
+
+        after { Puppet::Type.type(:mount).clear }
+
+        it "should return a hash containing values for all set properties" do
+            values = @resource.retrieve
+            [@resource.property(:fstype), @resource.property(:pass)].each { |property| values.should be_include(property) }
+        end
+
+        it "should not call retrieve on non-ensure properties if the resource is absent" do
+            @resource.property(:ensure).expects(:retrieve).returns :absent
+            @resource.property(:fstype).expects(:retrieve).never
+            @resource.retrieve[@resource.property(:fstype)]
+        end
+
+        it "should set all values to :absent if the resource is absent" do
+            @resource.property(:ensure).expects(:retrieve).returns :absent
+            @resource.retrieve[@resource.property(: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[@resource.property(:fstype)].should == 15
+        end
     end
 
-    it "should have no parent if there is no in edge" do
-        @container.parent.should be_nil
-    end
+    describe "when in a catalog" do
+        before do
+            @catalog = Puppet::Node::Catalog.new
+            @container = Puppet::Type.type(:component).create(:name => "container")
+            @one = Puppet::Type.type(:file).create(:path => "/file/one")
+            @two = Puppet::Type.type(:file).create(:path => "/file/two")
+            @catalog.add_resource @container
+            @catalog.add_resource @one
+            @catalog.add_resource @two
+            @catalog.add_edge @container, @one
+            @catalog.add_edge @container, @two
+        end
 
-    it "should set its parent to its in edge" do
-        @one.parent.ref.should == @container.ref
-    end
+        it "should have no parent if there is no in edge" do
+            @container.parent.should be_nil
+        end
 
-    after do
-        @catalog.clear(true)
+        it "should set its parent to its in edge" do
+            @one.parent.ref.should == @container.ref
+        end
+
+        after do
+            @catalog.clear(true)
+        end
     end
 end