diff options
| -rw-r--r-- | lib/chef/cookbook/cookbook_version_loader.rb | 3 | ||||
| -rw-r--r-- | lib/chef/cookbook_loader.rb | 69 | ||||
| -rw-r--r-- | spec/unit/cookbook_loader_spec.rb | 49 |
3 files changed, 94 insertions, 27 deletions
diff --git a/lib/chef/cookbook/cookbook_version_loader.rb b/lib/chef/cookbook/cookbook_version_loader.rb index bff17dc4a4..750d886e84 100644 --- a/lib/chef/cookbook/cookbook_version_loader.rb +++ b/lib/chef/cookbook/cookbook_version_loader.rb @@ -24,6 +24,9 @@ class Chef attr_reader :frozen attr_reader :uploaded_cookbook_version_file + # TODO: use this in preference to @cookbook_path internally + attr_reader :cookbook_path + def initialize(path, chefignore=nil) @cookbook_path = File.expand_path( path ) # cookbook_path from which this was loaded # We keep a list of all cookbook paths that have been merged in diff --git a/lib/chef/cookbook_loader.rb b/lib/chef/cookbook_loader.rb index 27cf978acb..cac5548565 100644 --- a/lib/chef/cookbook_loader.rb +++ b/lib/chef/cookbook_loader.rb @@ -50,6 +50,9 @@ class Chef repo_path = File.expand_path(repo_path) end + @preloaded_cookbooks = false + @loaders_by_name = {} + # Used to track which cookbooks appear in multiple places in the cookbook repos # and are merged in to a single cookbook by file shadowing. This behavior is # deprecated, so users of this class may issue warnings to the user by checking @@ -64,25 +67,25 @@ class Chef end def load_cookbooks - @repo_paths.each do |repo_path| - Dir[File.join(repo_path, "*")].each do |cookbook_path| - load_cookbook(File.basename(cookbook_path), [repo_path]) - end + preload_cookbooks + @loaders_by_name.each do |cookbook_name, _loaders| + load_cookbook(cookbook_name) end @cookbooks_by_name end - def load_cookbook(cookbook_name, repo_paths=nil) - repo_paths ||= @repo_paths - repo_paths.each do |repo_path| - @chefignores[repo_path] ||= Cookbook::Chefignore.new(repo_path) - cookbook_path = File.join(repo_path, cookbook_name.to_s) - next unless File.directory?(cookbook_path) and Dir[File.join(repo_path, "*")].include?(cookbook_path) - loader = Cookbook::CookbookVersionLoader.new(cookbook_path, @chefignores[repo_path]) - loader.load_cookbooks + def load_cookbook(cookbook_name) + preload_cookbooks + + return nil unless @loaders_by_name.key?(cookbook_name.to_s) + + @loaders_by_name[cookbook_name.to_s].each do |loader| + loader.load + next if loader.empty? - cookbook_name = loader.cookbook_name - @cookbooks_paths[cookbook_name] << cookbook_path # for deprecation warnings + + @cookbooks_paths[cookbook_name] << loader.cookbook_path # for deprecation warnings + if @loaded_cookbooks.key?(cookbook_name) @merged_cookbooks << cookbook_name # for deprecation warnings @loaded_cookbooks[cookbook_name].merge!(loader) @@ -130,5 +133,43 @@ class Chef end alias :cookbooks :values + private + + def preload_cookbooks + return false if @preloaded_cookbooks + + all_directories_in_repo_paths.each do |cookbook_path| + preload_cookbook(cookbook_path) + end + @preloaded_cookbooks = true + true + end + + def preload_cookbook(cookbook_path) + repo_path = File.dirname(cookbook_path) + @chefignores[repo_path] ||= Cookbook::Chefignore.new(repo_path) + loader = Cookbook::CookbookVersionLoader.new(cookbook_path, @chefignores[repo_path]) + + cookbook_name = loader.cookbook_name + + # TODO: wrap @loaders_by_name so string keys are encapsulated + @loaders_by_name[cookbook_name.to_s] ||= [] + @loaders_by_name[cookbook_name.to_s] << loader + end + + def all_directories_in_repo_paths + @all_directories_in_repo_paths ||= + all_files_in_repo_paths.select { |path| File.directory?(path) } + end + + def all_files_in_repo_paths + @all_files_in_repo_paths ||= + begin + @repo_paths.inject([]) do |all_children, repo_path| + all_children += Dir[File.join(repo_path, "*")] + end + end + end + end end diff --git a/spec/unit/cookbook_loader_spec.rb b/spec/unit/cookbook_loader_spec.rb index 713347af7b..8c1a0886c2 100644 --- a/spec/unit/cookbook_loader_spec.rb +++ b/spec/unit/cookbook_loader_spec.rb @@ -29,7 +29,25 @@ describe Chef::CookbookLoader do let(:cookbook_loader) { Chef::CookbookLoader.new(repo_paths) } - describe "loading all cookbooks" do + it "checks each directory only once when loading (CHEF-3487)" do + cookbook_paths = [] + repo_paths.each do |repo_path| + cookbook_paths |= Dir[File.join(repo_path, "*")] + end + + cookbook_paths.delete_if { |path| File.basename(path) == "chefignore" } + + cookbook_paths.each do |cookbook_path| + Chef::Cookbook::CookbookVersionLoader.should_receive(:new). + with(cookbook_path, anything). + once. + and_call_original + end + cookbook_loader.load_cookbooks + end + + + context "after loading all cookbooks" do before(:each) do cookbook_loader.load_cookbooks end @@ -77,7 +95,7 @@ describe Chef::CookbookLoader do end end - describe "load_cookbooks" do + describe "referencing cookbook files" do it "should find all the cookbooks in the cookbook path" do cookbook_loader.load_cookbooks cookbook_loader.should have_key(:openldap) @@ -149,17 +167,7 @@ describe Chef::CookbookLoader do cookbook_loader.metadata[:openldap].should be_a_kind_of(Chef::Cookbook::Metadata) end - it "should check each cookbook directory only once (CHEF-3487)" do - cookbooks = [] - repo_paths.each do |repo_path| - cookbooks |= Dir[File.join(repo_path, "*")] - end - cookbooks.each do |cookbook| - File.should_receive(:directory?).with(cookbook).once; - end - cookbook_loader.load_cookbooks - end - end # load_cookbooks + end # after loading cookbooks end # loading all cookbooks @@ -209,6 +217,7 @@ describe Chef::CookbookLoader do cookbook_loader["apache2"].should be_a_kind_of(Chef::CookbookVersion) end + describe "loading all cookbooks after loading only one cookbook" do before(:each) do cookbook_loader.load_cookbooks @@ -224,4 +233,18 @@ describe Chef::CookbookLoader do end end end # loading only one cookbook + + describe "loading a single cookbook with a different name than basename" do + + before(:each) do + cookbook_loader.load_cookbook("name-mismatch") + end + + it "loads the correct cookbook" do + cookbook_version = cookbook_loader["name-mismatch"] + cookbook_version.should be_a_kind_of(Chef::CookbookVersion) + cookbook_version.name.should == :"name-mismatch" + end + + end end |