Refactoring the Cacher interface to always require attribute declaration.

Previously you could dynamically use cached values, but the new interface
requires a single static declaration of the attribute:

    cached_attr(:myattr) { my_init_code() }

This is cleaner, because it makes it easy to turn the code into an init method
and generally makes the whole thing easier to think about.

Most of this commit is going through the different classes that already using the
Caching engine.

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

6 files changed, 69 insertions, 90 deletions

diff --git a/lib/puppet/file_serving/configuration.rb b/lib/puppet/file_serving/configuration.rb
index bceecc30c..907186ac3 100644
--- a/lib/puppet/file_serving/configuration.rb
+++ b/lib/puppet/file_serving/configuration.rb
@@ -10,7 +10,10 @@ require 'puppet/util/cacher'
 class Puppet::FileServing::Configuration
     require 'puppet/file_serving/configuration/parser'
 
-    extend Puppet::Util::Cacher
+    class << self
+        include Puppet::Util::Cacher
+        cached_attr(:configuration) { new() }
+    end
 
     @config_fileuration = nil
 
@@ -18,7 +21,7 @@ class Puppet::FileServing::Configuration
 
     # Create our singleton configuration.
     def self.create
-        attr_cache(:configuration) { new() }
+        configuration
     end
 
     private_class_method  :new
diff --git a/lib/puppet/file_serving/mount.rb b/lib/puppet/file_serving/mount.rb
index c52cedbfb..552cf33f2 100644
--- a/lib/puppet/file_serving/mount.rb
+++ b/lib/puppet/file_serving/mount.rb
@@ -13,16 +13,17 @@ require 'puppet/file_serving/content'
 # or content objects.
 class Puppet::FileServing::Mount < Puppet::Network::AuthStore
     include Puppet::Util::Logging
-    extend Puppet::Util::Cacher
 
-    def self.localmap
-        attr_cache(:localmap) { 
+    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
     end
 
     attr_reader :name
diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb
index db2772f25..d4f8af7f7 100644
--- a/lib/puppet/indirector/indirection.rb
+++ b/lib/puppet/indirector/indirection.rb
@@ -296,8 +296,6 @@ class Puppet::Indirector::Indirection
         return klass.new
     end
 
-    # Use cached termini.
-    def termini
-        attr_cache(:termini) { Hash.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 4b371abe7..ee28a3116 100644
--- a/lib/puppet/network/http_pool.rb
+++ b/lib/puppet/network/http_pool.rb
@@ -6,7 +6,14 @@ module Puppet::Network; end
 
 # Manage Net::HTTP instances for keep-alive.
 module Puppet::Network::HttpPool
-    extend Puppet::Util::Cacher
+    class << self
+        include Puppet::Util::Cacher
+        cached_attr(:ssl_host) { Puppet::SSL::Host.new }
+
+        private
+        
+        cached_attr(:http_cache) { Hash.new }
+    end
 
     # 2008/03/23
     # LAK:WARNING: Enabling this has a high propability of
@@ -17,12 +24,6 @@ module Puppet::Network::HttpPool
         HTTP_KEEP_ALIVE
     end
 
-    # Create an ssl host instance for getting certificate
-    # information.
-    def self.ssl_host
-        attr_cache(:ssl_host) { Puppet::SSL::Host.new }
-    end
-
     # Clear our http cache, closing all connections.
     def self.clear_http_instances
         http_cache.each do |name, connection|
@@ -102,11 +103,4 @@ module Puppet::Network::HttpPool
 
         return http
     end
-
-    private
-    
-    def self.http_cache
-        # Default to an empty hash.
-        attr_cache(:http) { Hash.new }
-    end
 end
diff --git a/lib/puppet/ssl/certificate_authority.rb b/lib/puppet/ssl/certificate_authority.rb
index 6947af11c..08feff0ac 100644
--- a/lib/puppet/ssl/certificate_authority.rb
+++ b/lib/puppet/ssl/certificate_authority.rb
@@ -17,7 +17,11 @@ class Puppet::SSL::CertificateAuthority
 
     require 'puppet/ssl/certificate_authority/interface'
 
-    extend Puppet::Util::Cacher
+    class << self
+        include Puppet::Util::Cacher
+
+        cached_attr(:singleton_instance) { new }
+    end
 
     def self.ca?
         return false unless Puppet[:ca]
@@ -30,7 +34,7 @@ class Puppet::SSL::CertificateAuthority
     def self.instance
         return nil unless ca?
 
-        attr_cache(:instance) { new }
+        singleton_instance
     end
 
     attr_reader :name, :host
diff --git a/lib/puppet/util/cacher.rb b/lib/puppet/util/cacher.rb
index 139aa3659..0406b12f1 100644
--- a/lib/puppet/util/cacher.rb
+++ b/lib/puppet/util/cacher.rb
@@ -1,25 +1,30 @@
 module Puppet::Util::Cacher
-    # It's basically not possible to test that this is set,
-    # but we need to start with a value so that all initial values
-    # start out valid -- that is, everything's valid until the
-    # first call to 'expire'.
-    @timestamp = Time.now
+    module Expirer
+        attr_reader :timestamp
 
-    # Cause all cached values to be considered expired.
-    def self.expire
-        @timestamp = Time.now
-    end
+        # Cause all cached values to be considered expired.
+        def expire
+            @timestamp = Time.now
+        end
 
-    # Is the provided timestamp later than or equal to our global timestamp?
-    # If it is, then the associated value is valid, otherwise it should be flushed.
-    def self.valid?(timestamp)
-        return timestamp >= @timestamp
+        # Is the provided timestamp earlier than our expiration timestamp?
+        # If it is, then the associated value is expired.
+        def expired?(ts)
+            return false unless timestamp
+
+            return 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)
-        other.extend(InstanceMethods)
+        class << other
+            extend ClassMethods
+            include InstanceMethods
+        end
     end
 
     # Our module has been included in a class, which means the class gets the class methods
@@ -40,72 +45,46 @@ module Puppet::Util::Cacher
             define_method(init_method, &block)
 
             define_method(name) do
-                cacher_caches.value(name) { send(init_method) }
+                cached_value(name)
             end
         end
     end
 
     # Methods that get added to instances.
     module InstanceMethods
-        private
-
-        # Use/define a cached value.  We just use the Cache class to do all
-        # of the thinking.  Note that we're using a single Cache instance
-        # for all of this instance's cached values.
-        def attr_cache(name, &block)
-            cacher_caches.value(name, &block)
-        end
-
-        def cacher_caches
-            unless defined?(@cacher_caches) and @cacher_caches
-                @cacher_caches = Cache.new
-            end
-            @cacher_caches
+        def expire
+            expirer.expire
         end
-    end
-
-    # An internal class that does all of our comparisons and calculations.
-    # This both caches a given value, and determines whether a given cache is
-    # still valid.
-    class Cache
-        attr_accessor :caches, :timestamp
 
-        def clear
-            caches.clear
-            self.timestamp = Time.now
+        def expirer
+            Puppet::Util::Cacher
         end
 
-        def initialize
-            @caches = {}
-            @timestamp = Time.now
-        end
+        private
 
-        # If our timestamp is out of date, our cached data is expired.
-        def expired?
-            ! Puppet::Util::Cacher.valid?(timestamp)
+        def cache_timestamp
+            unless defined?(@cache_timestamp)
+                @cache_timestamp = Time.now
+            end
+            @cache_timestamp
         end
 
-        # Return a value; use the cached version if the associated timestamp is recent enough,
-        # else calculate and store a new a value using the provided block.
-        def value(name)
-            raise ArgumentError, "You must provide a block when using the cache" unless block_given?
-
-            # If the cached data is expired, clear the cache and get a new
-            # value.  Note that if we clear the cache here, we potentially
-            # clear other cached values, too (if this instance is caching more
-            # than one value).
-            if expired?
-                clear
+        def cached_value(name)
+            if expirer.expired?(cache_timestamp)
+                value_cache.clear
+                @cache_timestamp = Time.now
             end
-
-            # Generate a new value if we don't have one.  Use 'include?' here
-            # rather than testing for truth, so we can cache false values.
-            unless caches.include?(name)
-                caches[name] = yield
+            unless value_cache.include?(name)
+                value_cache[name] = send("init_%s" % name)
             end
+            value_cache[name]
+        end
 
-            # Finally, return our cached value.
-            caches[name]
+        def value_cache
+            unless defined?(@value_cache) and @value_cache
+                @value_cache = {}
+            end
+            @value_cache
         end
     end
 end