Fixing #3668 - fixed autoloading classes from modules

This involved essentially moving all of the importing and loading
code out of the Parser and into a new 'TypeLoader' class.

The parser and the ResourceTypeCollection classes now delegate
to that class for all file handling.  Most of the code paths are
also now much cleaner, and a bit of redundancy was removed.

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

2 files changed, 202 insertions, 76 deletions

diff --git a/spec/unit/parser/parser.rb b/spec/unit/parser/parser.rb
index ccb7e8443..ed4492a87 100755
--- a/spec/unit/parser/parser.rb
+++ b/spec/unit/parser/parser.rb
@@ -32,6 +32,13 @@ describe Puppet::Parser do
         parser = Puppet::Parser::Parser.new "development"
         parser.known_resource_types.should equal(rtc)
     end
+    
+    it "should delegate importing to the known resource type loader" do
+        parser = Puppet::Parser::Parser.new "development"
+        parser.known_resource_types.loader.expects(:import).with("newfile", "current_file")
+        parser.lexer.expects(:file).returns "current_file"
+        parser.import("newfile")
+    end
 
     describe "when parsing files" do
         before do
@@ -327,91 +334,19 @@ describe Puppet::Parser do
     end
 
     describe "when looking up definitions" do
-        it "should check for them by name" do
-            @parser.stubs(:find_or_load).with("namespace","name",:definition).returns(:this_value)
+        it "should use the known resource types to check for them by name" do
+            @parser.known_resource_types.stubs(:find_or_load).with("namespace","name",:definition).returns(:this_value)
             @parser.find_definition("namespace","name").should == :this_value
         end
     end
 
     describe "when looking up hostclasses" do
-        it "should check for them by name" do
-            @parser.stubs(:find_or_load).with("namespace","name",:hostclass).returns(:this_value)
+        it "should use the known resource types to check for them by name" do
+            @parser.known_resource_types.stubs(:find_or_load).with("namespace","name",:hostclass).returns(:this_value)
             @parser.find_hostclass("namespace","name").should == :this_value
         end
     end
 
-    describe "when looking up names" do
-        before :each do
-            @known_resource_types = mock 'loaded code'
-            @known_resource_types.stubs(:find_my_type).with('loaded_namespace',  'loaded_name').returns(true)
-            @known_resource_types.stubs(:find_my_type).with('bogus_namespace',   'bogus_name' ).returns(false)
-            @parser = Puppet::Parser::Parser.new "development"
-            @parser.stubs(:known_resource_types).returns @known_resource_types
-        end
-
-        describe "that are already loaded" do
-            it "should not try to load anything" do
-                @parser.expects(:load).never
-                @parser.find_or_load("loaded_namespace","loaded_name",:my_type)
-            end
-            it "should return true" do
-                @parser.find_or_load("loaded_namespace","loaded_name",:my_type).should == true
-            end
-        end
-
-        describe "that aren't already loaded" do
-            it "should first attempt to load them with the all lowercase fully qualified name" do
-                @known_resource_types.stubs(:find_my_type).with("foo_namespace","foo_name").returns(false,true,true)
-                @parser.expects(:load).with("foo_namespace::foo_name").returns(true).then.raises(Exception)
-                @parser.find_or_load("Foo_namespace","Foo_name",:my_type).should == true
-            end
-
-            it "should next attempt to load them with the all lowercase namespace" do
-                @known_resource_types.stubs(:find_my_type).with("foo_namespace","foo_name").returns(false,false,true,true)
-                @parser.expects(:load).with("foo_namespace::foo_name").returns(false).then.raises(Exception)
-                @parser.expects(:load).with("foo_namespace"          ).returns(true ).then.raises(Exception)
-                @parser.find_or_load("Foo_namespace","Foo_name",:my_type).should == true
-            end
-
-            it "should finally attempt to load them with the all lowercase unqualified name" do
-                @known_resource_types.stubs(:find_my_type).with("foo_namespace","foo_name").returns(false,false,false,true,true)
-                @parser.expects(:load).with("foo_namespace::foo_name").returns(false).then.raises(Exception)
-                @parser.expects(:load).with("foo_namespace"          ).returns(false).then.raises(Exception)
-                @parser.expects(:load).with(               "foo_name").returns(true ).then.raises(Exception)
-                @parser.find_or_load("Foo_namespace","Foo_name",:my_type).should == true
-            end
-
-            it "should return false if the name isn't found" do
-                @parser.stubs(:load).returns(false)
-                @parser.find_or_load("Bogus_namespace","Bogus_name",:my_type).should == false
-            end
-
-            it "should directly look for fully qualified classes" do
-                @known_resource_types.stubs(:find_hostclass).with("foo_namespace","::foo_name").returns(false, true)
-                @parser.expects(:load).with("foo_name").returns true
-                @parser.find_or_load("foo_namespace","::foo_name",:hostclass)
-            end
-        end
-    end
-
-    describe "when loading classnames" do
-        before :each do
-            @known_resource_types = mock 'loaded code'
-            @parser = Puppet::Parser::Parser.new "development"
-            @parser.stubs(:known_resource_types).returns @known_resource_types
-        end
-
-        it "should just return false if the classname is empty" do
-            @parser.expects(:import).never
-            @parser.load("").should == false
-        end
-
-        it "should just return true if the item is loaded" do
-            pending "Need to access internal state (@parser's @loaded) to force this"
-            @parser.load("").should == false
-        end
-    end
-
     describe "when parsing classes" do
         before :each do
             @krt = Puppet::Resource::TypeCollection.new("development")
diff --git a/spec/unit/parser/type_loader.rb b/spec/unit/parser/type_loader.rb
new file mode 100644
index 000000000..de181eb10
--- /dev/null
+++ b/spec/unit/parser/type_loader.rb
@@ -0,0 +1,191 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../spec_helper'
+
+require 'puppet/parser/type_loader'
+require 'puppet_spec/files'
+
+describe Puppet::Parser::TypeLoader do
+    include PuppetSpec::Files
+
+    before do
+        @loader = Puppet::Parser::TypeLoader.new(:myenv)
+    end
+
+    it "should support an environment" do
+        loader = Puppet::Parser::TypeLoader.new(:myenv)
+        loader.environment.name.should == :myenv
+    end
+
+    it "should include the Environment Helper" do
+        @loader.class.ancestors.should be_include(Puppet::Node::Environment::Helper)
+    end
+
+    it "should delegate its known resource types to its environment" do
+        @loader.known_resource_types.should be_instance_of(Puppet::Resource::TypeCollection)
+    end
+
+    describe "when loading names from namespaces" do
+        it "should do nothing if the name to import is an empty string" do
+            @loader.expects(:name2files).never
+            @loader.load_until(["foo"], "") { |f| false }.should be_nil
+        end
+
+        it "should turn the provided namespaces and name into a list of files" do
+            @loader.expects(:name2files).with(["foo"], "bar").returns []
+            @loader.load_until(["foo"], "bar") { |f| false }
+        end
+
+        it "should attempt to import each generated name" do
+            @loader.expects(:name2files).returns %w{foo bar}
+            @loader.expects(:import).with("foo")
+            @loader.expects(:import).with("bar")
+            @loader.load_until(["foo"], "bar") { |f| false }
+        end
+
+        it "should yield after each import" do
+            yielded = []
+            @loader.expects(:name2files).returns %w{foo bar}
+            @loader.expects(:import).with("foo")
+            @loader.expects(:import).with("bar")
+            @loader.load_until(["foo"], "bar") { |f| yielded << f; false }
+            yielded.should == %w{foo bar}
+        end
+
+        it "should stop importing when the yielded block returns true" do
+            yielded = []
+            @loader.expects(:name2files).returns %w{foo bar baz}
+            @loader.expects(:import).with("foo")
+            @loader.expects(:import).with("bar")
+            @loader.expects(:import).with("baz").never
+            @loader.load_until(["foo"], "bar") { |f| true if f == "bar" }
+        end
+
+        it "should return the result of the block" do
+            yielded = []
+            @loader.expects(:name2files).returns %w{foo bar baz}
+            @loader.expects(:import).with("foo")
+            @loader.expects(:import).with("bar")
+            @loader.expects(:import).with("baz").never
+            @loader.load_until(["foo"], "bar") { |f| 10 if f == "bar" }.should == 10
+        end
+
+        it "should return nil if the block never returns true" do
+            @loader.expects(:name2files).returns %w{foo bar}
+            @loader.expects(:import).with("foo")
+            @loader.expects(:import).with("bar")
+            @loader.load_until(["foo"], "bar") { |f| false }.should be_nil
+        end
+
+        it "should know when a given name has been loaded" do
+            @loader.expects(:name2files).returns %w{file}
+            @loader.expects(:import).with("file")
+            @loader.load_until(["foo"], "bar") { |f| true }
+            @loader.should be_loaded("file")
+        end
+    end
+
+    describe "when mapping names to files" do
+        {
+            [["foo"], "::bar::baz"] => %w{bar/baz},
+            [[""], "foo::bar"]      => %w{foo foo/bar},
+            [%w{foo}, "bar"]        => %w{foo foo/bar bar},
+            [%w{a b}, "bar"]        => %w{a a/bar b b/bar bar},
+            [%w{a::b::c}, "bar"]    => %w{a a/b/c/bar bar},
+            [%w{a::b}, "foo::bar"]  => %w{a a/b/foo/bar foo/bar}
+        }.each do |inputs, outputs|
+            it "should produce #{outputs.inspect} from the #{inputs[0].inspect} namespace and #{inputs[1]} name" do
+                @loader.name2files(*inputs).should == outputs
+            end
+        end
+    end
+
+    describe "when importing" do
+        before do
+            Puppet::Parser::Files.stubs(:find_manifests).returns %w{file}
+            @loader.stubs(:parse_file)
+        end
+
+        it "should return immediately when imports are being ignored" do
+            Puppet::Parser::Files.expects(:find_manifests).never
+            Puppet[:ignoreimport] = true
+            @loader.import("foo").should be_nil
+        end
+
+        it "should find all manifests matching the file or pattern" do
+            Puppet::Parser::Files.expects(:find_manifests).with { |pat, opts| pat == "myfile" }.returns %w{one}
+            @loader.import("myfile")
+        end
+
+        it "should use the directory of the current file if one is set" do
+            Puppet::Parser::Files.expects(:find_manifests).with { |pat, opts| opts[:cwd] == "/current" }.returns %w{one}
+            @loader.import("myfile", "/current/file")
+        end
+
+        it "should pass the environment when looking for files" do
+            Puppet::Parser::Files.expects(:find_manifests).with { |pat, opts| opts[:environment] == @loader.environment }.returns %w{one}
+            @loader.import("myfile")
+        end
+
+        it "should fail if no files are found" do
+            Puppet::Parser::Files.expects(:find_manifests).returns []
+            lambda { @loader.import("myfile") }.should raise_error(Puppet::ImportError)
+        end
+
+        it "should parse each found file" do
+            Puppet::Parser::Files.expects(:find_manifests).returns %w{/one}
+            @loader.expects(:parse_file).with("/one")
+            @loader.import("myfile")
+        end
+
+        it "should make each file qualified before attempting to parse it" do
+            Puppet::Parser::Files.expects(:find_manifests).returns %w{one}
+            @loader.expects(:parse_file).with("/current/one")
+            @loader.import("myfile", "/current/file")
+        end
+
+        it "should know when a given file has been imported" do
+            Puppet::Parser::Files.expects(:find_manifests).returns %w{/one}
+            @loader.import("myfile")
+
+            @loader.should be_imported("/one")
+        end
+
+        it "should not attempt to import files that have already been imported" do
+            Puppet::Parser::Files.expects(:find_manifests).returns %w{/one}
+            @loader.expects(:parse_file).once
+            @loader.import("myfile")
+
+            # This will fail if it tries to reimport the file.
+            @loader.import("myfile")
+        end
+    end
+
+    describe "when parsing a file" do
+        before do
+            @parser = Puppet::Parser::Parser.new(@loader.environment)
+            @parser.stubs(:parse)
+            @parser.stubs(:file=)
+            Puppet::Parser::Parser.stubs(:new).with(@loader.environment).returns @parser
+        end
+
+        it "should create a new parser instance for each file using the current environment" do
+            Puppet::Parser::Parser.expects(:new).with(@loader.environment).returns @parser
+            @loader.parse_file("/my/file")
+        end
+
+        it "should assign the parser its file and parse" do
+            @parser.expects(:file=).with("/my/file")
+            @parser.expects(:parse)
+            @loader.parse_file("/my/file")
+        end
+    end
+
+    it "should be able to add classes to the current resource type collection" do
+        file = tmpfile("simple_file")
+        File.open(file, "w") { |f| f.puts "class foo {}" }
+        @loader.import(file)
+
+        @loader.known_resource_types.hostclass("foo").should be_instance_of(Puppet::Resource::Type)
+    end
+end