Merge branch 'tickets/master/6850-resource_type_listing_and_converting' into next

8 files changed, 241 insertions, 26 deletions

diff --git a/lib/puppet/indirector/resource_type/parser.rb b/lib/puppet/indirector/resource_type/parser.rb
index 8b1bed0a9..fd5b3938a 100644
--- a/lib/puppet/indirector/resource_type/parser.rb
+++ b/lib/puppet/indirector/resource_type/parser.rb
@@ -10,7 +10,10 @@ class Puppet::Indirector::ResourceType::Parser < Puppet::Indirector::Code
 
     # This is a bit ugly.
     [:hostclass, :definition, :node].each do |type|
-      if r = krt.send(type, request.key)
+      # We have to us 'find_<type>' here because it will
+      # load any missing types from disk, whereas the plain
+      # '<type>' method only returns from memory.
+      if r = krt.send("find_#{type}", [""], request.key)
         return r
       end
     end
@@ -20,7 +23,9 @@ class Puppet::Indirector::ResourceType::Parser < Puppet::Indirector::Code
   def search(request)
     raise ArgumentError, "Only '*' is acceptable as a search request" unless request.key == "*"
     krt = request.environment.known_resource_types
-    result = [krt.hostclasses.values, krt.definitions.values, krt.nodes.values].flatten
+    # Make sure we've got all of the types loaded.
+    krt.loader.import_all
+    result = [krt.hostclasses.values, krt.definitions.values, krt.nodes.values].flatten.reject { |t| t.name == "" }
     return nil if result.empty?
     result
   end
diff --git a/lib/puppet/module.rb b/lib/puppet/module.rb
index 43266b2b5..059591ed8 100644
--- a/lib/puppet/module.rb
+++ b/lib/puppet/module.rb
@@ -138,7 +138,7 @@ class Puppet::Module
 
   # Find this module in the modulepath.
   def path
-    environment.modulepath.collect { |path| File.join(path, name) }.find { |d| FileTest.exist?(d) }
+    environment.modulepath.collect { |path| File.join(path, name) }.find { |d| FileTest.directory?(d) }
   end
 
   # Find all plugin directories.  This is used by the Plugins fileserving mount.
diff --git a/lib/puppet/parser/type_loader.rb b/lib/puppet/parser/type_loader.rb
index 140c9f2ca..1fba73d0b 100644
--- a/lib/puppet/parser/type_loader.rb
+++ b/lib/puppet/parser/type_loader.rb
@@ -92,6 +92,34 @@ class Puppet::Parser::TypeLoader
     end
   end
 
+  def import_all
+    require 'find'
+
+    module_names = []
+    # Collect the list of all known modules
+    environment.modulepath.each do |path|
+      Dir.chdir(path) do
+        Dir.glob("*").each do |dir|
+          next unless FileTest.directory?(dir)
+          module_names << dir
+        end
+      end
+    end
+
+    module_names.uniq!
+    # And then load all files from each module, but (relying on system
+    # behavior) only load files from the first module of a given name.  E.g.,
+    # given first/foo and second/foo, only files from first/foo will be loaded.
+    module_names.each do |name|
+      mod = Puppet::Module.new(name, environment)
+      Find.find(File.join(mod.path, "manifests")) do |path|
+        if path =~ /\.pp$/ or path =~ /\.rb$/
+          import(path)
+        end
+      end
+    end
+  end
+
   def known_resource_types
     environment.known_resource_types
   end
diff --git a/lib/puppet/resource/type.rb b/lib/puppet/resource/type.rb
index b9cf6991a..48d8c1f48 100644
--- a/lib/puppet/resource/type.rb
+++ b/lib/puppet/resource/type.rb
@@ -34,13 +34,13 @@ class Puppet::Resource::Type
   end
 
   def to_pson_data_hash
-    data = [:code, :doc, :line, :file, :parent].inject({}) do |hash, param|
-      next hash unless value = self.send(param)
+    data = [:doc, :line, :file, :parent].inject({}) do |hash, param|
+      next hash unless (value = self.send(param)) and (value != "")
       hash[param.to_s] = value
       hash
     end
 
-    data['arguments'] = arguments.dup
+    data['arguments'] = arguments.dup unless arguments.empty?
 
     data['name'] = name
     data['type'] = type
diff --git a/spec/unit/indirector/resource_type/parser_spec.rb b/spec/unit/indirector/resource_type/parser_spec.rb
index 739e58b35..27e61486c 100755
--- a/spec/unit/indirector/resource_type/parser_spec.rb
+++ b/spec/unit/indirector/resource_type/parser_spec.rb
@@ -3,13 +3,15 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../../spec_helper')
 
 require 'puppet/indirector/resource_type/parser'
+require 'puppet_spec/files'
 
 describe Puppet::Indirector::ResourceType::Parser do
+  include PuppetSpec::Files
+
   before do
     @terminus = Puppet::Indirector::ResourceType::Parser.new
     @request = Puppet::Indirector::Request.new(:resource_type, :find, "foo")
-    @krt = Puppet::Resource::TypeCollection.new(@request.environment)
-    @request.environment.stubs(:known_resource_types).returns @krt
+    @krt = @request.environment.known_resource_types
   end
 
   it "should be registered with the resource_type indirection" do
@@ -17,16 +19,29 @@ describe Puppet::Indirector::ResourceType::Parser do
   end
 
   describe "when finding" do
-    it "should use the request's environment's list of known resource types" do
-      @request.environment.known_resource_types.expects(:hostclass).returns nil
+    it "should return any found type from the request's environment" do
+      type = Puppet::Resource::Type.new(:hostclass, "foo")
+      @request.environment.known_resource_types.add(type)
 
-      @terminus.find(@request)
+      @terminus.find(@request).should == type
     end
 
-    it "should return any found type" do
-      type = @krt.add(Puppet::Resource::Type.new(:hostclass, "foo"))
+    it "should attempt to load the type if none is found in memory" do
+      dir = tmpdir("find_a_type")
+      FileUtils.mkdir_p(dir)
+      Puppet[:modulepath] = dir
 
-      @terminus.find(@request).should == type
+      # Make a new request, since we've reset the env
+      @request = Puppet::Indirector::Request.new(:resource_type, :find, "foo::bar")
+
+      manifest_path = File.join(dir, "foo", "manifests")
+      FileUtils.mkdir_p(manifest_path)
+
+      File.open(File.join(manifest_path, "bar.pp"), "w") { |f| f.puts "class foo::bar {}" }
+
+      result = @terminus.find(@request)
+      result.should be_instance_of(Puppet::Resource::Type)
+      result.name.should == "foo::bar"
     end
 
     it "should return nil if no type can be found" do
@@ -68,8 +83,41 @@ describe Puppet::Indirector::ResourceType::Parser do
       result.should be_include(define)
     end
 
+    it "should not return the 'main' class" do
+      main = @krt.add(Puppet::Resource::Type.new(:hostclass, ""))
+
+      # So there is a return value
+      foo = @krt.add(Puppet::Resource::Type.new(:hostclass, "foo"))
+
+      @terminus.search(@request).should_not be_include(main)
+    end
+
     it "should return nil if no types can be found" do
       @terminus.search(@request).should be_nil
     end
+
+    it "should load all resource types from all search paths" do
+      dir = tmpdir("searching_in_all")
+      first = File.join(dir, "first")
+      second = File.join(dir, "second")
+      FileUtils.mkdir_p(first)
+      FileUtils.mkdir_p(second)
+      Puppet[:modulepath] = "#{first}:#{second}"
+
+      # Make a new request, since we've reset the env
+      @request = Puppet::Indirector::Request.new(:resource_type, :search, "*")
+
+      onepath = File.join(first, "one", "manifests")
+      FileUtils.mkdir_p(onepath)
+      twopath = File.join(first, "two", "manifests")
+      FileUtils.mkdir_p(twopath)
+
+      File.open(File.join(onepath, "oneklass.pp"), "w") { |f| f.puts "class one::oneklass {}" }
+      File.open(File.join(twopath, "twoklass.pp"), "w") { |f| f.puts "class two::twoklass {}" }
+
+      result = @terminus.search(@request)
+      result.find { |t| t.name == "one::oneklass" }.should be_instance_of(Puppet::Resource::Type)
+      result.find { |t| t.name == "two::twoklass" }.should be_instance_of(Puppet::Resource::Type)
+    end
   end
 end
diff --git a/spec/unit/module_spec.rb b/spec/unit/module_spec.rb
index 54f5444ee..f3120e16b 100755
--- a/spec/unit/module_spec.rb
+++ b/spec/unit/module_spec.rb
@@ -1,8 +1,11 @@
 #!/usr/bin/env ruby
 
 require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
+require 'puppet_spec/files'
 
 describe Puppet::Module do
+  include PuppetSpec::Files
+
   before do
     # This is necessary because of the extra checks we have for the deprecated
     # 'plugins' directory
@@ -267,17 +270,39 @@ describe Puppet::Module do
   end
 
   it "should return the path to the first found instance in its environment's module paths as its path" do
+    dir = tmpdir("deep_path")
+    first = File.join(dir, "first")
+    second = File.join(dir, "second")
+
+    FileUtils.mkdir_p(first)
+    FileUtils.mkdir_p(second)
+    Puppet[:modulepath] = "#{first}:#{second}"
+
+    modpath = File.join(first, "foo")
+    FileUtils.mkdir_p(modpath)
+
+    # Make a second one, which we shouldn't find
+    FileUtils.mkdir_p(File.join(second, "foo"))
+
     mod = Puppet::Module.new("foo")
-    env = mock 'environment'
-    mod.stubs(:environment).returns env
+    mod.path.should == modpath
+  end
+
+  it "should be able to find itself in a directory other than the first directory in the module path" do
+    dir = tmpdir("deep_path")
+    first = File.join(dir, "first")
+    second = File.join(dir, "second")
 
-    env.expects(:modulepath).returns %w{/a /b /c}
+    FileUtils.mkdir_p(first)
+    FileUtils.mkdir_p(second)
+    Puppet[:modulepath] = "#{first}:#{second}"
 
-    FileTest.expects(:exist?).with("/a/foo").returns false
-    FileTest.expects(:exist?).with("/b/foo").returns true
-    FileTest.expects(:exist?).with("/c/foo").never
+    modpath = File.join(second, "foo")
+    FileUtils.mkdir_p(modpath)
 
-    mod.path.should == "/b/foo"
+    mod = Puppet::Module.new("foo")
+    mod.should be_exist
+    mod.path.should == modpath
   end
 
   it "should be considered existent if it exists in at least one module path" do
diff --git a/spec/unit/parser/type_loader_spec.rb b/spec/unit/parser/type_loader_spec.rb
index bd41adfb6..12bc1ccd6 100644
--- a/spec/unit/parser/type_loader_spec.rb
+++ b/spec/unit/parser/type_loader_spec.rb
@@ -93,6 +93,103 @@ describe Puppet::Parser::TypeLoader do
     end
   end
 
+  describe "when importing all" do
+    before do
+      @base = tmpdir("base")
+
+      # Create two module path directories
+      @modulebase1 = File.join(@base, "first")
+      FileUtils.mkdir_p(@modulebase1)
+      @modulebase2 = File.join(@base, "second")
+      FileUtils.mkdir_p(@modulebase2)
+
+      Puppet[:modulepath] = "#{@modulebase1}:#{@modulebase2}"
+    end
+
+    def mk_module(basedir, name)
+      module_dir = File.join(basedir, name)
+
+      # Go ahead and make our manifest directory
+      FileUtils.mkdir_p(File.join(module_dir, "manifests"))
+
+      return Puppet::Module.new(name)
+    end
+
+    # We have to pass the base path so that we can
+    # write to modules that are in the second search path
+    def mk_manifests(base, mod, type, files)
+      exts = {"ruby" => ".rb", "puppet" => ".pp"}
+      files.collect do |file|
+        name = mod.name + "::" + file.gsub("/", "::")
+        path = File.join(base, mod.name, "manifests", file + exts[type])
+        FileUtils.mkdir_p(File.split(path)[0])
+
+        # write out the class
+        if type == "ruby"
+          File.open(path, "w") { |f| f.print "hostclass '#{name}' do\nend" }
+        else
+          File.open(path, "w") { |f| f.print "class #{name} {}" }
+        end
+        name
+      end
+    end
+
+    it "should load all puppet manifests from all modules in the specified environment" do
+      @module1 = mk_module(@modulebase1, "one")
+      @module2 = mk_module(@modulebase2, "two")
+
+      mk_manifests(@modulebase1, @module1, "puppet", %w{a b})
+      mk_manifests(@modulebase2, @module2, "puppet", %w{c d})
+
+      @loader.import_all
+
+      @loader.environment.known_resource_types.hostclass("one::a").should be_instance_of(Puppet::Resource::Type)
+      @loader.environment.known_resource_types.hostclass("one::b").should be_instance_of(Puppet::Resource::Type)
+      @loader.environment.known_resource_types.hostclass("two::c").should be_instance_of(Puppet::Resource::Type)
+      @loader.environment.known_resource_types.hostclass("two::d").should be_instance_of(Puppet::Resource::Type)
+    end
+
+    it "should load all ruby manifests from all modules in the specified environment" do
+      @module1 = mk_module(@modulebase1, "one")
+      @module2 = mk_module(@modulebase2, "two")
+
+      mk_manifests(@modulebase1, @module1, "ruby", %w{a b})
+      mk_manifests(@modulebase2, @module2, "ruby", %w{c d})
+
+      @loader.import_all
+
+      @loader.environment.known_resource_types.hostclass("one::a").should be_instance_of(Puppet::Resource::Type)
+      @loader.environment.known_resource_types.hostclass("one::b").should be_instance_of(Puppet::Resource::Type)
+      @loader.environment.known_resource_types.hostclass("two::c").should be_instance_of(Puppet::Resource::Type)
+      @loader.environment.known_resource_types.hostclass("two::d").should be_instance_of(Puppet::Resource::Type)
+    end
+
+    it "should not load manifests from duplicate modules later in the module path" do
+      @module1 = mk_module(@modulebase1, "one")
+
+      # duplicate
+      @module2 = mk_module(@modulebase2, "one")
+
+      mk_manifests(@modulebase1, @module1, "puppet", %w{a})
+      mk_manifests(@modulebase2, @module2, "puppet", %w{c})
+
+      @loader.import_all
+
+      @loader.environment.known_resource_types.hostclass("one::c").should be_nil
+    end
+
+    it "should load manifests from subdirectories" do
+      @module1 = mk_module(@modulebase1, "one")
+
+      mk_manifests(@modulebase1, @module1, "puppet", %w{a a/b a/b/c})
+
+      @loader.import_all
+
+      @loader.environment.known_resource_types.hostclass("one::a::b").should be_instance_of(Puppet::Resource::Type)
+      @loader.environment.known_resource_types.hostclass("one::a::b::c").should be_instance_of(Puppet::Resource::Type)
+    end
+  end
+
   describe "when parsing a file" do
     before do
       @parser = Puppet::Parser::Parser.new(@loader.environment)
diff --git a/spec/unit/resource/type_spec.rb b/spec/unit/resource/type_spec.rb
index e9c203526..41b5554d9 100755
--- a/spec/unit/resource/type_spec.rb
+++ b/spec/unit/resource/type_spec.rb
@@ -55,12 +55,24 @@ describe Puppet::Resource::Type do
       double_convert.arguments.should == {"one" => nil, "two" => "foo"}
     end
 
-    it "should include any extra attributes" do
-      @type.file = "/my/file"
-      @type.line = 50
+    it "should not include arguments if none are present" do
+      @type.to_pson["arguments"].should be_nil
+    end
+
+    [:line, :doc, :file, :parent].each do |attr|
+      it "should include #{attr} when set" do
+        @type.send(attr.to_s + "=", "value")
+        double_convert.send(attr).should == "value"
+      end
+
+      it "should not include #{attr} when not set" do
+        @type.to_pson[attr.to_s].should be_nil
+      end
+    end
 
-      double_convert.file.should == "/my/file"
-      double_convert.line.should == 50
+    it "should not include docs if they are empty" do
+      @type.doc = ""
+      @type.to_pson["doc"].should be_nil
     end
   end