Merge branch 'remove-cacher'

17 files changed, 76 insertions, 176 deletions

diff --git a/lib/puppet/file_serving/configuration.rb b/lib/puppet/file_serving/configuration.rb
index 78e4de6cb..d88d57cb0 100644
--- a/lib/puppet/file_serving/configuration.rb
+++ b/lib/puppet/file_serving/configuration.rb
@@ -2,29 +2,27 @@
 #  Created by Luke Kanies on 2007-10-16.
 #  Copyright (c) 2007. All rights reserved.
 
+require 'monitor'
 require 'puppet'
 require 'puppet/file_serving'
 require 'puppet/file_serving/mount'
 require 'puppet/file_serving/mount/file'
 require 'puppet/file_serving/mount/modules'
 require 'puppet/file_serving/mount/plugins'
-require 'puppet/util/cacher'
 
 class Puppet::FileServing::Configuration
   require 'puppet/file_serving/configuration/parser'
 
-  class << self
-    include Puppet::Util::Cacher
-    cached_attr(:configuration) { new }
+  extend MonitorMixin
+
+  def self.configuration
+    synchronize do
+      @configuration ||= new
+    end
   end
 
   Mount = Puppet::FileServing::Mount
 
-  # Create our singleton configuration.
-  def self.create
-    configuration
-  end
-
   private_class_method  :new
 
   attr_reader :mounts
diff --git a/lib/puppet/file_serving/mount.rb b/lib/puppet/file_serving/mount.rb
index 37dd89537..79290ab81 100644
--- a/lib/puppet/file_serving/mount.rb
+++ b/lib/puppet/file_serving/mount.rb
@@ -4,7 +4,6 @@
 
 require 'puppet/network/authstore'
 require 'puppet/util/logging'
-require 'puppet/util/cacher'
 require 'puppet/file_serving'
 require 'puppet/file_serving/metadata'
 require 'puppet/file_serving/content'
diff --git a/lib/puppet/file_serving/mount/file.rb b/lib/puppet/file_serving/mount/file.rb
index 7d622e4bf..7f5af7f52 100644
--- a/lib/puppet/file_serving/mount/file.rb
+++ b/lib/puppet/file_serving/mount/file.rb
@@ -1,18 +1,15 @@
-require 'puppet/util/cacher'
-
 require 'puppet/file_serving/mount'
 
 class Puppet::FileServing::Mount::File < Puppet::FileServing::Mount
-  class << self
-    include Puppet::Util::Cacher
-
-    cached_attr(:localmap) do
-      {   "h" =>  Facter.value("hostname"),
-        "H" => [Facter.value("hostname"),
-            Facter.value("domain")].join("."),
-        "d" =>  Facter.value("domain")
-      }
-    end
+  def self.localmap
+    @localmap ||= {
+      "h" => Facter.value("hostname"),
+      "H" => [
+               Facter.value("hostname"),
+               Facter.value("domain")
+             ].join("."),
+      "d" => Facter.value("domain")
+    }
   end
 
   def complete_path(relative_path, node)
diff --git a/lib/puppet/indirector/file_server.rb b/lib/puppet/indirector/file_server.rb
index 46a08c97d..d6a8ab872 100644
--- a/lib/puppet/indirector/file_server.rb
+++ b/lib/puppet/indirector/file_server.rb
@@ -64,6 +64,6 @@ class Puppet::Indirector::FileServer < Puppet::Indirector::Terminus
 
   # Our fileserver configuration, if needed.
   def configuration
-    Puppet::FileServing::Configuration.create
+    Puppet::FileServing::Configuration.configuration
   end
 end
diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb
index d958a82ac..20b260b83 100644
--- a/lib/puppet/indirector/indirection.rb
+++ b/lib/puppet/indirector/indirection.rb
@@ -1,13 +1,11 @@
 require 'puppet/util/docs'
 require 'puppet/indirector/envelope'
 require 'puppet/indirector/request'
-require 'puppet/util/cacher'
 
 # The class that connects functional classes with their different collection
 # back-ends.  Each indirection has a set of associated terminus classes,
 # each of which is a subclass of Puppet::Indirector::Terminus.
 class Puppet::Indirector::Indirection
-  include Puppet::Util::Cacher
   include Puppet::Util::Docs
 
   @@indirections = []
@@ -33,6 +31,8 @@ class Puppet::Indirector::Indirection
 
   attr_accessor :name, :model
 
+  attr_reader :termini
+
   # Create and return our cache terminus.
   def cache
     raise(Puppet::DevError, "Tried to cache when no cache class was set") unless cache_class
@@ -88,6 +88,7 @@ class Puppet::Indirector::Indirection
   def initialize(model, name, options = {})
     @model = model
     @name = name
+    @termini = {}
 
     @cache_class = nil
     @terminus_class = nil
@@ -313,7 +314,4 @@ class Puppet::Indirector::Indirection
     end
     klass.new
   end
-
-  # Cache our terminus instances indefinitely, but make it easy to clean them up.
-  cached_attr(:termini) { Hash.new }
 end
diff --git a/lib/puppet/network/http_pool.rb b/lib/puppet/network/http_pool.rb
index d4ec48d22..8baf48c77 100644
--- a/lib/puppet/network/http_pool.rb
+++ b/lib/puppet/network/http_pool.rb
@@ -1,6 +1,5 @@
 require 'puppet/ssl/host'
 require 'net/https'
-require 'puppet/util/cacher'
 
 module Puppet::Network; end
 
diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb
index 96fdc3c1e..f25bb65a9 100644
--- a/lib/puppet/node/environment.rb
+++ b/lib/puppet/node/environment.rb
@@ -95,7 +95,7 @@ class Puppet::Node::Environment
 
   # Cache the modulepath, so that we aren't searching through
   # all known directories all the time.
-  cached_attr(:modulepath, :ttl => Puppet[:filetimeout]) do
+  cached_attr(:modulepath, Puppet[:filetimeout]) do
     dirs = self[:modulepath].split(File::PATH_SEPARATOR)
     dirs = ENV["PUPPETLIB"].split(File::PATH_SEPARATOR) + dirs if ENV["PUPPETLIB"]
     validate_dirs(dirs)
@@ -103,7 +103,7 @@ class Puppet::Node::Environment
 
   # Return all modules from this environment.
   # Cache the list, because it can be expensive to create.
-  cached_attr(:modules, :ttl => Puppet[:filetimeout]) do
+  cached_attr(:modules, Puppet[:filetimeout]) do
     module_names = modulepath.collect { |path| Dir.entries(path) }.flatten.uniq
     module_names.collect do |path|
       begin
@@ -114,12 +114,6 @@ class Puppet::Node::Environment
     end.compact
   end
 
-  # Cache the manifestdir, so that we aren't searching through
-  # all known directories all the time.
-  cached_attr(:manifestdir, :ttl => Puppet[:filetimeout]) do
-    validate_dirs(self[:manifestdir].split(File::PATH_SEPARATOR))
-  end
-
   def to_s
     name.to_s
   end
diff --git a/lib/puppet/parameter.rb b/lib/puppet/parameter.rb
index 29d60fc66..c97f93b23 100644
--- a/lib/puppet/parameter.rb
+++ b/lib/puppet/parameter.rb
@@ -2,7 +2,6 @@ require 'puppet/util/methodhelper'
 require 'puppet/util/log_paths'
 require 'puppet/util/logging'
 require 'puppet/util/docs'
-require 'puppet/util/cacher'
 
 class Puppet::Parameter
   include Puppet::Util
@@ -10,7 +9,6 @@ class Puppet::Parameter
   include Puppet::Util::LogPaths
   include Puppet::Util::Logging
   include Puppet::Util::MethodHelper
-  include Puppet::Util::Cacher
 
   require 'puppet/parameter/value_collection'
 
@@ -150,10 +148,6 @@ class Puppet::Parameter
     self.fail(Puppet::DevError, msg)
   end
 
-  def expirer
-    resource.catalog
-  end
-
   def fail(*args)
     type = nil
     if args[0].is_a?(Class)
diff --git a/lib/puppet/resource/catalog.rb b/lib/puppet/resource/catalog.rb
index 8eb4266db..bb19f3ebc 100644
--- a/lib/puppet/resource/catalog.rb
+++ b/lib/puppet/resource/catalog.rb
@@ -3,7 +3,6 @@ require 'puppet/indirector'
 require 'puppet/simple_graph'
 require 'puppet/transaction'
 
-require 'puppet/util/cacher'
 require 'puppet/util/pson'
 
 require 'puppet/util/tagging'
@@ -20,7 +19,6 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
 
   include Puppet::Util::Tagging
   extend Puppet::Util::Pson
-  include Puppet::Util::Cacher::Expirer
 
   # The host name this is a catalog for.
   attr_accessor :name
@@ -123,10 +121,6 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
   def apply(options = {})
     @applying = true
 
-    # Expire all of the resource data -- this ensures that all
-    # data we're operating against is entirely current.
-    expire
-
     Puppet::Util::Storage.load if host_config?
 
     transaction = Puppet::Transaction.new(self, options[:report])
@@ -162,7 +156,6 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
     return transaction
   ensure
     @applying = false
-    cleanup
   end
 
   # Are we in the middle of applying the catalog?
@@ -197,14 +190,6 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
     resource
   end
 
-  def dependent_data_expired?(ts)
-    if applying?
-      return super
-    else
-      return true
-    end
-  end
-
   # Turn our catalog graph into an old-style tree of TransObjects and TransBuckets.
   # LAK:NOTE(20081211): This is a  pre-0.25 backward compatibility method.
   # It can be removed as soon as xmlrpc is killed.
@@ -564,11 +549,6 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
 
   private
 
-  def cleanup
-    # Expire any cached data the resources are keeping.
-    expire
-  end
-
   # Verify that the given resource isn't defined elsewhere.
   def fail_on_duplicate_type_and_title(resource)
     # Short-curcuit the common case,
diff --git a/lib/puppet/ssl/certificate_authority.rb b/lib/puppet/ssl/certificate_authority.rb
index d65067c70..a4cbaf78a 100644
--- a/lib/puppet/ssl/certificate_authority.rb
+++ b/lib/puppet/ssl/certificate_authority.rb
@@ -1,6 +1,6 @@
+require 'monitor'
 require 'puppet/ssl/host'
 require 'puppet/ssl/certificate_request'
-require 'puppet/util/cacher'
 
 # The class that knows how to sign certificates.  It creates
 # a 'special' SSL::Host whose name is 'ca', thus indicating
@@ -17,6 +17,8 @@ class Puppet::SSL::CertificateAuthority
   require 'puppet/ssl/certificate_authority/interface'
   require 'puppet/network/authstore'
 
+  extend MonitorMixin
+
   class CertificateVerificationError < RuntimeError
     attr_accessor :error_code
 
@@ -25,10 +27,10 @@ class Puppet::SSL::CertificateAuthority
     end
   end
 
-  class << self
-    include Puppet::Util::Cacher
-
-    cached_attr(:singleton_instance) { new }
+  def self.singleton_instance
+    synchronize do
+      @singleton_instance ||= new
+    end
   end
 
   def self.ca?
diff --git a/lib/puppet/ssl/host.rb b/lib/puppet/ssl/host.rb
index b9215effd..08a8ace1f 100644
--- a/lib/puppet/ssl/host.rb
+++ b/lib/puppet/ssl/host.rb
@@ -4,7 +4,6 @@ require 'puppet/ssl/key'
 require 'puppet/ssl/certificate'
 require 'puppet/ssl/certificate_request'
 require 'puppet/ssl/certificate_revocation_list'
-require 'puppet/util/cacher'
 
 # The class that manages all aspects of our SSL certificates --
 # private keys, public keys, requests, etc.
@@ -27,14 +26,10 @@ class Puppet::SSL::Host
   # This accessor is used in instances for indirector requests to hold desired state
   attr_accessor :desired_state
 
-  class << self
-    include Puppet::Util::Cacher
-
-    cached_attr(:localhost) do
-      result = new
-      result.generate unless result.certificate
-      result.key # Make sure it's read in
-      result
+  def self.localhost
+    @localhost ||= new.tap do |l|
+      l.generate unless l.certificate
+      l.key # Make sure it's read in
     end
   end
 
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 15f340f55..963b925bf 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -9,7 +9,6 @@ require 'puppet/metatype/manager'
 require 'puppet/util/errors'
 require 'puppet/util/log_paths'
 require 'puppet/util/logging'
-require 'puppet/util/cacher'
 require 'puppet/file_collection/lookup'
 require 'puppet/util/tagging'
 
@@ -21,7 +20,6 @@ class Type
   include Puppet::Util::Errors
   include Puppet::Util::LogPaths
   include Puppet::Util::Logging
-  include Puppet::Util::Cacher
   include Puppet::FileCollection::Lookup
   include Puppet::Util::Tagging
 
@@ -469,12 +467,6 @@ class Type
     Puppet::Transaction::Event.new({:resource => self, :file => file, :line => line, :tags => tags}.merge(options))
   end
 
-  # Let the catalog determine whether a given cached value is
-  # still valid or has expired.
-  def expirer
-    catalog
-  end
-
   # retrieve the 'should' value for a specified property
   def should(name)
     name = attr_alias(name)
diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index 8ab12ca2f..988416ab5 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 49e885865..222b85dbb 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/lib/puppet/util/autoload.rb b/lib/puppet/util/autoload.rb
index 6537a4a4e..2e8710ab1 100644
--- a/lib/puppet/util/autoload.rb
+++ b/lib/puppet/util/autoload.rb
@@ -1,5 +1,4 @@
 require 'puppet/util/warnings'
-require 'puppet/util/cacher'
 
 # Autoload paths, either based on names or all at once.
 class Puppet::Util::Autoload
@@ -7,7 +6,6 @@ class Puppet::Util::Autoload
 
   include Puppet::Util
   include Puppet::Util::Warnings
-  include Puppet::Util::Cacher
   include Puppet::Util::Autoload::FileCache
 
   @autoloaders = {}
diff --git a/lib/puppet/util/cacher.rb b/lib/puppet/util/cacher.rb
index 3dddec0d4..136c9973e 100644
--- a/lib/puppet/util/cacher.rb
+++ b/lib/puppet/util/cacher.rb
@@ -1,25 +1,6 @@
 require 'monitor'
 
 module Puppet::Util::Cacher
-  module Expirer
-    attr_reader :timestamp
-
-    # Cause all cached values to be considered expired.
-    def expire
-      @timestamp = Time.now
-    end
-
-    # Is the provided timestamp earlier than our expiration timestamp?
-    # If it is, then the associated value is expired.
-    def dependent_data_expired?(ts)
-      return false unless timestamp
-
-      timestamp > ts
-    end
-  end
-
-  extend Expirer
-
   # Our module has been extended in a class; we can only add the Instance methods,
   # which become *class* methods in the class.
   def self.extended(other)
@@ -40,27 +21,26 @@ module Puppet::Util::Cacher
   module ClassMethods
     # Provide a means of defining an attribute whose value will be cached.
     # Must provide a block capable of defining the value if it's flushed..
-    def cached_attr(name, options = {}, &block)
+    def cached_attr(name, ttl, &block)
       init_method = "init_#{name}"
       define_method(init_method, &block)
 
+      set_attr_ttl(name, ttl)
+
       define_method(name) do
         cached_value(name)
       end
 
       define_method(name.to_s + "=") do |value|
         # Make sure the cache timestamp is set
-        cache_timestamp
-        value_cache.synchronize { value_cache[name] = value }
-      end
-
-      if ttl = options[:ttl]
-        set_attr_ttl(name, ttl)
+        value_cache.synchronize do
+          value_cache[name] = value
+          set_expiration(name)
+        end
       end
     end
 
     def attr_ttl(name)
-      return nil unless @attr_ttls
       @attr_ttls[name]
     end
 
@@ -72,57 +52,25 @@ module Puppet::Util::Cacher
 
   # Methods that get added to instances.
   module InstanceMethods
-
-    def expire
-      # Only expire if we have an expirer.  This is
-      # mostly so that we can comfortably handle cases
-      # like Puppet::Type instances, which use their
-      # catalog as their expirer, and they often don't
-      # have a catalog.
-      if e = expirer
-        e.expire
-      end
-    end
-
-    def expirer
-      Puppet::Util::Cacher
-    end
-
     private
 
-    def cache_timestamp
-      @cache_timestamp ||= Time.now
-    end
-
     def cached_value(name)
       value_cache.synchronize do
-        # Allow a nil expirer, in which case we regenerate the value every time.
-        if expired_by_expirer?(name)
-          value_cache.clear
-          @cache_timestamp = Time.now
-        elsif expired_by_ttl?(name)
-          value_cache.delete(name)
+        if value_cache[name].nil? or expired_by_ttl?(name)
+          value_cache[name] = send("init_#{name}")
+          set_expiration(name)
         end
-        value_cache[name] = send("init_#{name}") unless value_cache.include?(name)
         value_cache[name]
       end
     end
 
-    def expired_by_expirer?(name)
-      if expirer.nil?
-        return true unless self.class.attr_ttl(name)
-      end
-      expirer.dependent_data_expired?(cache_timestamp)
-    end
-
     def expired_by_ttl?(name)
-      return false unless self.class.respond_to?(:attr_ttl)
-      return false unless ttl = self.class.attr_ttl(name)
-
-      @ttl_timestamps ||= {}
-      @ttl_timestamps[name] ||= Time.now
+      @attr_expirations[name] < Time.now
+    end
 
-      (Time.now - @ttl_timestamps[name]) > ttl
+    def set_expiration(name)
+      @attr_expirations ||= {}
+      @attr_expirations[name] = Time.now + self.class.attr_ttl(name)
     end
 
     def value_cache
diff --git a/lib/puppet/util/settings.rb b/lib/puppet/util/settings.rb
index 4559e9af3..caaf61b7b 100644
--- a/lib/puppet/util/settings.rb
+++ b/lib/puppet/util/settings.rb
@@ -2,13 +2,11 @@ require 'puppet'
 require 'sync'
 require 'getoptlong'
 require 'puppet/external/event-loop'
-require 'puppet/util/cacher'
 require 'puppet/util/loadedfile'
 
 # The class for handling configuration files.
 class Puppet::Util::Settings
   include Enumerable
-  include Puppet::Util::Cacher
 
   require 'puppet/util/settings/setting'
   require 'puppet/util/settings/file_setting'
@@ -401,11 +399,10 @@ class Puppet::Util::Settings
     }
   end
 
-  # Cache this in an easily clearable way, since we were
-  # having trouble cleaning it up after tests.
-  cached_attr(:file) do
+  def file
+    return @file if @file
     if path = self[:config] and FileTest.exist?(path)
-      Puppet::Util::LoadedFile.new(path)
+      @file = Puppet::Util::LoadedFile.new(path)
     end
   end