Remove use of Util::Cacher in FileServing::Configuration

This class was using Util::Cacher for its singleton instance, when that was
unnecessary. The FileServing::Configuration instance already manages whether or
not to reparse its config file, based on whether it has changed. Thus, there is
no need for it to be manually expired via the cacher.

Reviewed-By: Jacob Helwig <jacob@puppetlabs.com>

6 files changed, 35 insertions, 49 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/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/spec/integration/indirector/file_content/file_server_spec.rb b/spec/integration/indirector/file_content/file_server_spec.rb
index 2a8c134a4..86b75006a 100755
--- a/spec/integration/indirector/file_content/file_server_spec.rb
+++ b/spec/integration/indirector/file_content/file_server_spec.rb
@@ -17,6 +17,7 @@ describe Puppet::Indirector::FileContent::FileServer, " when finding files", :fa
   before do
     @terminus = Puppet::Indirector::FileContent::FileServer.new
     @test_class = Puppet::FileServing::Content
+    Puppet::FileServing::Configuration.instance_variable_set(:@configuration, nil)
   end
 
   it "should find plugin file content in the environment specified in the request" do
@@ -62,7 +63,6 @@ describe Puppet::Indirector::FileContent::FileServer, " when finding files", :fa
   end
 
   it "should find file content in files when node name expansions are used" do
-    Puppet::Util::Cacher.expire
     FileTest.stubs(:exists?).returns true
     FileTest.stubs(:exists?).with(Puppet[:fileserverconfig]).returns(true)
 
diff --git a/spec/shared_behaviours/file_server_terminus.rb b/spec/shared_behaviours/file_server_terminus.rb
index f59169382..88ed47ebb 100755
--- a/spec/shared_behaviours/file_server_terminus.rb
+++ b/spec/shared_behaviours/file_server_terminus.rb
@@ -7,7 +7,7 @@ shared_examples_for "Puppet::Indirector::FileServerTerminus" do
   # This only works if the shared behaviour is included before
   # the 'before' block in the including context.
   before do
-    Puppet::Util::Cacher.expire
+    Puppet::FileServing::Configuration.instance_variable_set(:@configuration, nil)
     FileTest.stubs(:exists?).returns true
     FileTest.stubs(:exists?).with(Puppet[:fileserverconfig]).returns(true)
 
diff --git a/spec/unit/file_serving/configuration_spec.rb b/spec/unit/file_serving/configuration_spec.rb
index ed8663853..a1546c987 100755
--- a/spec/unit/file_serving/configuration_spec.rb
+++ b/spec/unit/file_serving/configuration_spec.rb
@@ -3,26 +3,6 @@ require 'spec_helper'
 
 require 'puppet/file_serving/configuration'
 
-describe Puppet::FileServing::Configuration, :fails_on_windows => true do
-  it "should make :new a private method" do
-    proc { Puppet::FileServing::Configuration.new }.should raise_error
-  end
-
-  it "should return the same configuration each time :create is called" do
-    Puppet::FileServing::Configuration.create.should equal(Puppet::FileServing::Configuration.create)
-  end
-
-  it "should have a method for removing the current configuration instance" do
-    old = Puppet::FileServing::Configuration.create
-    Puppet::Util::Cacher.expire
-    Puppet::FileServing::Configuration.create.should_not equal(old)
-  end
-
-  after do
-    Puppet::Util::Cacher.expire
-  end
-end
-
 describe Puppet::FileServing::Configuration do
   include PuppetSpec::Files
 
@@ -33,14 +13,22 @@ describe Puppet::FileServing::Configuration do
   end
 
   after :each do
-    Puppet::Util::Cacher.expire
+    Puppet::FileServing::Configuration.instance_variable_set(:@configuration, nil)
+  end
+
+  it "should make :new a private method" do
+    proc { Puppet::FileServing::Configuration.new }.should raise_error
+  end
+
+  it "should return the same configuration each time 'configuration' is called" do
+    Puppet::FileServing::Configuration.configuration.should equal(Puppet::FileServing::Configuration.configuration)
   end
 
   describe "when initializing" do
 
     it "should work without a configuration file" do
       FileTest.stubs(:exists?).with(@path).returns(false)
-      proc { Puppet::FileServing::Configuration.create }.should_not raise_error
+      proc { Puppet::FileServing::Configuration.configuration }.should_not raise_error
     end
 
     it "should parse the configuration file if present" do
@@ -48,11 +36,11 @@ describe Puppet::FileServing::Configuration do
       @parser = mock 'parser'
       @parser.expects(:parse).returns({})
       Puppet::FileServing::Configuration::Parser.stubs(:new).returns(@parser)
-      Puppet::FileServing::Configuration.create
+      Puppet::FileServing::Configuration.configuration
     end
 
     it "should determine the path to the configuration file from the Puppet settings" do
-      Puppet::FileServing::Configuration.create
+      Puppet::FileServing::Configuration.configuration
     end
   end
 
@@ -66,18 +54,18 @@ describe Puppet::FileServing::Configuration do
 
     it "should set the mount list to the results of parsing" do
       @parser.expects(:parse).returns("one" => mock("mount"))
-      config = Puppet::FileServing::Configuration.create
+      config = Puppet::FileServing::Configuration.configuration
       config.mounted?("one").should be_true
     end
 
     it "should not raise exceptions" do
       @parser.expects(:parse).raises(ArgumentError)
-      proc { Puppet::FileServing::Configuration.create }.should_not raise_error
+      proc { Puppet::FileServing::Configuration.configuration }.should_not raise_error
     end
 
     it "should replace the existing mount list with the results of reparsing" do
       @parser.expects(:parse).returns("one" => mock("mount"))
-      config = Puppet::FileServing::Configuration.create
+      config = Puppet::FileServing::Configuration.configuration
       config.mounted?("one").should be_true
       # Now parse again
       @parser.expects(:parse).returns("two" => mock('other'))
@@ -89,7 +77,7 @@ describe Puppet::FileServing::Configuration do
     it "should not replace the mount list until the file is entirely parsed successfully" do
       @parser.expects(:parse).returns("one" => mock("mount"))
       @parser.expects(:parse).raises(ArgumentError)
-      config = Puppet::FileServing::Configuration.create
+      config = Puppet::FileServing::Configuration.configuration
       # Now parse again, so the exception gets thrown
       config.send(:readconfig, false)
       config.mounted?("one").should be_true
@@ -97,7 +85,7 @@ describe Puppet::FileServing::Configuration do
 
     it "should add modules and plugins mounts even if the file does not exist" do
       FileTest.expects(:exists?).returns false # the file doesn't exist
-      config = Puppet::FileServing::Configuration.create
+      config = Puppet::FileServing::Configuration.configuration
       config.mounted?("modules").should be_true
       config.mounted?("plugins").should be_true
     end
@@ -112,7 +100,7 @@ describe Puppet::FileServing::Configuration do
       Puppet::FileServing::Mount::Plugins.stubs(:new).returns(plugins)
       plugins.expects(:allow).with('*')
 
-      Puppet::FileServing::Configuration.create
+      Puppet::FileServing::Configuration.configuration
     end
 
     it "should not allow access from all to modules and plugins if the fileserver.conf provided some rules" do
@@ -126,13 +114,13 @@ describe Puppet::FileServing::Configuration do
       Puppet::FileServing::Mount::Plugins.stubs(:new).returns(plugins)
       plugins.expects(:allow).with('*').never
 
-      Puppet::FileServing::Configuration.create
+      Puppet::FileServing::Configuration.configuration
     end
 
     it "should add modules and plugins mounts even if they are not returned by the parser" do
       @parser.expects(:parse).returns("one" => mock("mount"))
       FileTest.expects(:exists?).returns true # the file doesn't exist
-      config = Puppet::FileServing::Configuration.create
+      config = Puppet::FileServing::Configuration.configuration
       config.mounted?("modules").should be_true
       config.mounted?("plugins").should be_true
     end
@@ -140,13 +128,13 @@ describe Puppet::FileServing::Configuration do
 
   describe "when finding the specified mount" do
     it "should choose the named mount if one exists" do
-      config = Puppet::FileServing::Configuration.create
+      config = Puppet::FileServing::Configuration.configuration
       config.expects(:mounts).returns("one" => "foo")
       config.find_mount("one", mock('env')).should == "foo"
     end
 
     it "should use the provided environment to find a matching module if the named module cannot be found" do
-      config = Puppet::FileServing::Configuration.create
+      config = Puppet::FileServing::Configuration.configuration
 
       mod = mock 'module'
       env = mock 'environment'
@@ -159,7 +147,7 @@ describe Puppet::FileServing::Configuration do
     end
 
     it "should return nil if there is no such named mount and no module with the same name exists" do
-      config = Puppet::FileServing::Configuration.create
+      config = Puppet::FileServing::Configuration.configuration
 
       env = mock 'environment'
       env.expects(:module).with("foo").returns nil
@@ -172,7 +160,7 @@ describe Puppet::FileServing::Configuration do
 
   describe "when finding the mount name and relative path in a request key" do
     before do
-      @config = Puppet::FileServing::Configuration.create
+      @config = Puppet::FileServing::Configuration.configuration
       @config.stubs(:find_mount)
 
       @request = stub 'request', :key => "foo/bar/baz", :options => {}, :node => nil, :environment => mock("env")
diff --git a/spec/unit/indirector/file_server_spec.rb b/spec/unit/indirector/file_server_spec.rb
index 6df715fb1..96fe101d5 100755
--- a/spec/unit/indirector/file_server_spec.rb
+++ b/spec/unit/indirector/file_server_spec.rb
@@ -27,7 +27,7 @@ describe Puppet::Indirector::FileServer do
 
     @uri = "puppet://host/my/local/file"
     @configuration = mock 'configuration'
-    Puppet::FileServing::Configuration.stubs(:create).returns(@configuration)
+    Puppet::FileServing::Configuration.stubs(:configuration).returns(@configuration)
 
     @request = Puppet::Indirector::Request.new(:myind, :mymethod, @uri, :environment => "myenv")
   end