The InstanceLoader class had two methods which exposed its internal implementation: instance_hash and instance_loader. Replaced those methods with methods implementing the purposes they were used for: recording that an instance was loaded, loading all instances, and forgetting about a previously loaded instance.
Also removed the method instance_docs, which was not being used and didn't work. Signed-off-by: Paul Berry <[email protected]> --- lib/puppet/indirector/terminus.rb | 4 +- lib/puppet/reports.rb | 10 +++++-- lib/puppet/util/instance_loader.rb | 42 ++++++++++++-------------------- lib/puppet/util/queue.rb | 4 +- lib/puppet/util/reference.rb | 4 +- spec/unit/indirector/terminus_spec.rb | 2 +- spec/unit/reports_spec.rb | 12 +++++---- spec/unit/util/queue_spec.rb | 5 ++- test/util/instance_loader.rb | 12 ++++---- 9 files changed, 46 insertions(+), 49 deletions(-) diff --git a/lib/puppet/indirector/terminus.rb b/lib/puppet/indirector/terminus.rb index 4ebd0d0..c6c54c6 100644 --- a/lib/puppet/indirector/terminus.rb +++ b/lib/puppet/indirector/terminus.rb @@ -99,7 +99,7 @@ class Puppet::Indirector::Terminus # Register a class, probably autoloaded. def register_terminus_class(klass) setup_instance_loading klass.indirection_name - instance_hash(klass.indirection_name)[klass.name] = klass + instance_loaded(klass.indirection_name, klass.name, klass) end # Return a terminus by name, using the autoloader. @@ -113,7 +113,7 @@ class Puppet::Indirector::Terminus setup_instance_loading indirection_name # Load them all. - instance_loader(indirection_name).loadall + load_all_instances(indirection_name) # And return the list of names. loaded_instances(indirection_name) diff --git a/lib/puppet/reports.rb b/lib/puppet/reports.rb index 3ebd16e..30af259 100755 --- a/lib/puppet/reports.rb +++ b/lib/puppet/reports.rb @@ -16,7 +16,11 @@ class Puppet::Reports def self.register_report(name, options = {}, &block) name = symbolize(name) - mod = genmodule(name, :extend => Puppet::Util::Docs, :hash => instance_hash(:report), :block => block) + mod = genmodule(name, :extend => Puppet::Util::Docs, :block => block) + if loaded_instances(:report).include?(mod.name) + raise Puppet::SubclassAlreadyDefined, "Already a generated class named #{mod.name}" + end + instance_loaded(:report, mod.name, mod) mod.useyaml = true if options[:useyaml] @@ -30,7 +34,7 @@ class Puppet::Reports docs = "" # Use this method so they all get loaded - instance_loader(:report).loadall + load_all_instances(:report) loaded_instances(:report).sort { |a,b| a.to_s <=> b.to_s }.each do |name| mod = self.report(name) docs += "#{name}\n#{"-" * name.to_s.length}\n" @@ -43,7 +47,7 @@ class Puppet::Reports # List each of the reports. def self.reports - instance_loader(:report).loadall + load_all_instances(:report) loaded_instances(:report) end end diff --git a/lib/puppet/util/instance_loader.rb b/lib/puppet/util/instance_loader.rb index b041f56..4d74b96 100755 --- a/lib/puppet/util/instance_loader.rb +++ b/lib/puppet/util/instance_loader.rb @@ -27,45 +27,35 @@ module Puppet::Util::InstanceLoader end end - # Return a list of the names of all instances + # Return a list of the names of all instances of a given type. def loaded_instances(type) @instances[type].keys end - # Collect the docs for all of our instances. - def instance_docs(type) - docs = "" - - # Load all instances. - instance_loader(type).loadall - - # Use this method so they all get loaded - loaded_instances(type).sort { |a,b| a.to_s <=> b.to_s }.each do |name| - mod = self.loaded_instance(name) - docs += "#{name}\n#{"-" * name.to_s.length}\n" - - docs += Puppet::Util::Docs.scrub(mod.doc) + "\n\n" - end - - docs + # Record that the given instance has been loaded. + def instance_loaded(type, name, object) + @instances[symbolize(type)][name] = object end - # Return the instance hash for our type. - def instance_hash(type) - @instances[symbolize(type)] + # Load all instance of a given type. + def load_all_instances(type) + @autoloaders[symbolize(type)].loadall end - # Return the Autoload object for a given type. - def instance_loader(type) - @autoloaders[symbolize(type)] + # Forget about an instance that was previously loaded. + def forget_loaded_instance(type, name) + if instances = @instances[type] + @instances[type].delete(name) + end + nil end - # Retrieve an alread-loaded instance, or attempt to load our instance. + # Load an instance if it's not loaded already, and return it. def loaded_instance(type, name) name = symbolize(name) - return nil unless instances = instance_hash(type) + return nil unless instances = @instances[symbolize(type)] unless instances.include? name - if instance_loader(type).load(name) + if @autoloaders[symbolize(type)].load(name) unless instances.include? name Puppet.warning( "Loaded #{type} file for #{name} but #{type} was not defined" diff --git a/lib/puppet/util/queue.rb b/lib/puppet/util/queue.rb index 0235774..871c01d 100644 --- a/lib/puppet/util/queue.rb +++ b/lib/puppet/util/queue.rb @@ -52,8 +52,8 @@ module Puppet::Util::Queue # each registration. def self.register_queue_type(klass, type = nil) type ||= queue_type_from_class(klass) - raise Puppet::Error, "Queue type #{type} is already registered" if instance_hash(:queue_clients).include?(type) - instance_hash(:queue_clients)[type] = klass + raise Puppet::Error, "Queue type #{type} is already registered" if loaded_instances(:queue_clients).include?(type) + instance_loaded(:queue_clients, type, klass) end # Given a queue type symbol, returns the associated +Class+ object. If the queue type is unknown diff --git a/lib/puppet/util/reference.rb b/lib/puppet/util/reference.rb index 4f2058e..b16af1d 100644 --- a/lib/puppet/util/reference.rb +++ b/lib/puppet/util/reference.rb @@ -20,7 +20,7 @@ class Puppet::Util::Reference def self.newreference(name, options = {}, &block) ref = self.new(name, options, &block) - instance_hash(:reference)[symbolize(name)] = ref + instance_loaded(:reference, symbolize(name), ref) ref end @@ -68,7 +68,7 @@ class Puppet::Util::Reference end def self.references - instance_loader(:reference).loadall + load_all_instances(:reference) loaded_instances(:reference).sort { |a,b| a.to_s <=> b.to_s } end diff --git a/spec/unit/indirector/terminus_spec.rb b/spec/unit/indirector/terminus_spec.rb index 826b934..4630d9e 100755 --- a/spec/unit/indirector/terminus_spec.rb +++ b/spec/unit/indirector/terminus_spec.rb @@ -132,7 +132,7 @@ describe Puppet::Indirector::Terminus, " when managing terminus classes" do # Set up instance loading; it would normally happen automatically Puppet::Indirector::Terminus.instance_load :test1, "puppet/indirector/test1" - Puppet::Indirector::Terminus.instance_loader(:test1).expects(:load).with(:yay) + Puppet::Indirector::Terminus.instance_eval { @autoloaders[:test1] }.expects(:load).with(:yay) Puppet::Indirector::Terminus.terminus_class(:test1, :yay) end diff --git a/spec/unit/reports_spec.rb b/spec/unit/reports_spec.rb index 57e77a5..fe201c5 100755 --- a/spec/unit/reports_spec.rb +++ b/spec/unit/reports_spec.rb @@ -5,10 +5,6 @@ require File.dirname(__FILE__) + '/../spec_helper' require 'puppet/reports' describe Puppet::Reports do - it "should instance-load report types" do - Puppet::Reports.instance_loader(:report).should be_instance_of(Puppet::Util::Autoload) - end - it "should have a method for registering report types" do Puppet::Reports.should respond_to(:register_report) end @@ -18,7 +14,7 @@ describe Puppet::Reports do end it "should provide a method for returning documentation for all reports" do - Puppet::Reports.expects(:loaded_instances).with(:report).returns([:one, :two]) + Puppet::Reports.stubs(:loaded_instances).with(:report).returns([:one, :two]) one = mock 'one', :doc => "onedoc" two = mock 'two', :doc => "twodoc" Puppet::Reports.expects(:report).with(:one).returns(one) @@ -39,6 +35,10 @@ describe Puppet::Reports, " when loading report types" do end describe Puppet::Reports, " when registering report types" do + after do + Puppet::Reports.forget_loaded_instance(:report, :mock_report_module) + end + it "should evaluate the supplied block as code for a module" do Puppet::Reports.expects(:genmodule).returns(Module.new) Puppet::Reports.register_report(:testing) { } @@ -46,6 +46,7 @@ describe Puppet::Reports, " when registering report types" do it "should extend the report type with the Puppet::Util::Docs module" do mod = stub 'module', :define_method => true + mod.stubs(:name).returns(:mock_report_module) Puppet::Reports.expects(:genmodule).with { |name, options, block| options[:extend] == Puppet::Util::Docs }.returns(mod) Puppet::Reports.register_report(:testing) { } @@ -54,6 +55,7 @@ describe Puppet::Reports, " when registering report types" do it "should define a :report_name method in the module that returns the name of the report" do mod = mock 'module' mod.expects(:define_method).with(:report_name) + mod.stubs(:name).returns(:mock_report_module) Puppet::Reports.expects(:genmodule).returns(mod) Puppet::Reports.register_report(:testing) { } diff --git a/spec/unit/util/queue_spec.rb b/spec/unit/util/queue_spec.rb index 571bddd..497338e 100755 --- a/spec/unit/util/queue_spec.rb +++ b/spec/unit/util/queue_spec.rb @@ -33,8 +33,9 @@ describe Puppet::Util::Queue do end after :all do - instances = mod.instance_hash(:queue_clients) - [:default, :setup, :bogus, :aardvark, :conflict, :test_a, :test_b].each{ |x| instances.delete(x) } + [:default, :setup, :bogus, :aardvark, :conflict, :test_a, :test_b].each do |x| + mod.forget_loaded_instance(:queue_clients, x) + end end context 'when determining a type name from a class' do diff --git a/test/util/instance_loader.rb b/test/util/instance_loader.rb index 2e3dcb3..fa69b38 100755 --- a/test/util/instance_loader.rb +++ b/test/util/instance_loader.rb @@ -15,7 +15,7 @@ class TestInstanceloader < Test::Unit::TestCase extend Puppet::Util::InstanceLoader def self.newstuff(name, value) - instance_hash(:stuff)[name] = value + instance_loaded(:stuff, name, value) end end @@ -26,11 +26,11 @@ class TestInstanceloader < Test::Unit::TestCase # Make sure we correctly create our autoload instance. This covers the basics. def test_autoload - # Make sure we can retrieve the loader - assert_instance_of(Puppet::Util::Autoload, @loader.instance_loader(:stuff), "Could not get instance loader") - - # Make sure we can get the instance hash - assert(@loader.instance_hash(:stuff), "Could not get instance hash") + # Make sure we can report an object was loaded. + test_object = Object.new + @loader.instance_loaded(:stuff, :test_name, test_object) + assert(@loader.loaded_instance(:stuff, :test_name).equal?(test_object)) + @loader.forget_loaded_instance(:stuff, :test_name) # Make sure it defines the instance retrieval method assert(@loader.respond_to?(:stuff), "Did not define instance retrieval method") -- 1.7.2 -- 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.
