These values needn't be cached_attrs, because they can be managed manually. 'stat' does need to be cached, so that we avoid statting the file for each property we want to check from disk. The 'content' attribute of 'source' also needs to be cached, because it's retrieved from the server, which we certainly don't want to do multiple times.
We need a mechanism for invalidating the 'stat' after we've written the file, so we use a special value :needs_stat, which essentially represented "undefined". We use this rather than nil so that we can store a failed stat if it occurs. Because the content and metadata of our source file will never change, there is no need to be able to similarly expire the values of those attributes. Reviewed-By: Jacob Helwig <[email protected]> --- lib/puppet/type/file.rb | 23 ++++++++++++++--------- lib/puppet/type/file/source.rb | 18 +++++++++++------- spec/unit/type/file/source_spec.rb | 22 ---------------------- 3 files changed, 25 insertions(+), 38 deletions(-) diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index 8ab12ca..988416a 100644 --- a/lib/puppet/type/file.rb +++ b/lib/puppet/type/file.rb @@ -394,7 +394,7 @@ Puppet::Type.newtype(:file) do @parameters.each do |name, param| param.flush if param.respond_to?(:flush) end - @stat = nil + @stat = :needs_stat end def initialize(hash) @@ -413,7 +413,7 @@ Puppet::Type.newtype(:file) do end end - @stat = nil + @stat = :needs_stat end # Configure discovered resources to be purged. @@ -623,7 +623,7 @@ Puppet::Type.newtype(:file) do else self.fail "Could not back up files of type #{s.ftype}" end - expire + @stat = :needs_stat end def retrieve @@ -674,22 +674,27 @@ Puppet::Type.newtype(:file) do # use either 'stat' or 'lstat', and we expect the properties to use the # resulting stat object accordingly (mostly by testing the 'ftype' # value). - cached_attr(:stat) do + # + # We use the initial value :needs_stat to ensure we only stat the file once, + # but can also keep track of a failed stat (@stat == nil). This also allows + # us to re-stat on demand by setting @stat = :needs_stat. + def stat + return @stat unless @stat == :needs_stat + method = :stat # Files are the only types that support links if (self.class.name == :file and self[:links] != :follow) or self.class.name == :tidy method = :lstat end - path = self[:path] - begin + @stat = begin ::File.send(method, self[:path]) rescue Errno::ENOENT => error - return nil + nil rescue Errno::EACCES => error warning "Could not stat; permission denied" - return nil + nil end end @@ -776,7 +781,7 @@ Puppet::Type.newtype(:file) do next unless [:mode, :owner, :group, :seluser, :selrole, :seltype, :selrange].include?(thing.name) # Make sure we get a new stat objct - expire + @stat = :needs_stat currentvalue = thing.retrieve thing.sync unless thing.safe_insync?(currentvalue) end diff --git a/lib/puppet/type/file/source.rb b/lib/puppet/type/file/source.rb index 49e8858..222b85d 100755 --- a/lib/puppet/type/file/source.rb +++ b/lib/puppet/type/file/source.rb @@ -95,13 +95,14 @@ module Puppet end # Look up (if necessary) and return remote content. - cached_attr(:content) do + def content + return @content if @content raise Puppet::DevError, "No source for content was stored with the metadata" unless metadata.source unless tmp = Puppet::FileServing::Content.indirection.find(metadata.source) fail "Could not find any content at %s" % metadata.source end - tmp.content + @content = tmp.content end # Copy the values from the source to the resource. Yay. @@ -137,25 +138,28 @@ module Puppet ! (metadata.nil? or metadata.ftype.nil?) end + attr_writer :metadata + # Provide, and retrieve if necessary, the metadata for this file. Fail # if we can't find data about this host, and fail if there are any # problems in our query. - cached_attr(:metadata) do + def metadata + return @metadata if @metadata return nil unless value result = nil value.each do |source| begin if data = Puppet::FileServing::Metadata.indirection.find(source) - result = data - result.source = source + @metadata = data + @metadata.source = source break end rescue => detail fail detail, "Could not retrieve file metadata for #{source}: #{detail}" end end - fail "Could not retrieve information from source(s) #{value.join(", ")}" unless result - result + fail "Could not retrieve information from source(s) #{value.join(", ")}" unless @metadata + @metadata end def local? diff --git a/spec/unit/type/file/source_spec.rb b/spec/unit/type/file/source_spec.rb index 5129d1e..c696fea 100755 --- a/spec/unit/type/file/source_spec.rb +++ b/spec/unit/type/file/source_spec.rb @@ -31,14 +31,6 @@ describe Puppet::Type.type(:file).attrclass(:source) do end end - it "should have a method for retrieving its metadata" do - source.new(:resource => @resource).must respond_to(:metadata) - end - - it "should have a method for setting its metadata" do - source.new(:resource => @resource).must respond_to(:metadata=) - end - describe "when returning the metadata", :fails_on_windows => true do before do @metadata = stub 'metadata', :source= => nil @@ -94,20 +86,6 @@ describe Puppet::Type.type(:file).attrclass(:source) do lambda { @source.metadata }.should raise_error(RuntimeError) end - - it "should expire the metadata appropriately" do - expirer = stub 'expired', :dependent_data_expired? => true - - metadata = stub 'metadata', :source= => nil - Puppet::FileServing::Metadata.indirection.expects(:find).with(@feebooz).returns metadata - - @source = source.new(:resource => @resource, :value => [@feebooz]) - @source.metadata = "foo" - - @source.stubs(:expirer).returns expirer - - @source.metadata.should_not == "foo" - end end it "should have a method for setting the desired values on the resource" do -- 1.7.5.4 -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
