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 <[email protected]> --- lib/puppet/file_serving/configuration.rb | 16 ++--- lib/puppet/indirector/file_server.rb | 2 +- .../indirector/file_content/file_server_spec.rb | 2 +- spec/shared_behaviours/file_server_terminus.rb | 2 +- spec/unit/file_serving/configuration_spec.rb | 60 ++++++++------------ spec/unit/indirector/file_server_spec.rb | 2 +- 6 files changed, 35 insertions(+), 49 deletions(-) diff --git a/lib/puppet/file_serving/configuration.rb b/lib/puppet/file_serving/configuration.rb index 78e4de6..d88d57c 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 46a08c9..d6a8ab8 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 2a8c134..86b7500 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 f591693..88ed47e 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 ed86638..a1546c9 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 6df715f..96fe101 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 -- 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.
