Merge branch 'jk/chef-fs-cookbook-refactor'

9 files changed, 265 insertions, 101 deletions

diff --git a/lib/chef/chef_fs/file_system/chef_server/chef_server_root_dir.rb b/lib/chef/chef_fs/file_system/chef_server/chef_server_root_dir.rb
index 1ac03166c2..4524dda608 100644
--- a/lib/chef/chef_fs/file_system/chef_server/chef_server_root_dir.rb
+++ b/lib/chef/chef_fs/file_system/chef_server/chef_server_root_dir.rb
@@ -21,6 +21,7 @@ require 'chef/chef_fs/file_system/chef_server/acls_dir'
 require 'chef/chef_fs/file_system/base_fs_dir'
 require 'chef/chef_fs/file_system/chef_server/rest_list_dir'
 require 'chef/chef_fs/file_system/chef_server/cookbooks_dir'
+require 'chef/chef_fs/file_system/chef_server/versioned_cookbooks_dir'
 require 'chef/chef_fs/file_system/chef_server/data_bags_dir'
 require 'chef/chef_fs/file_system/chef_server/nodes_dir'
 require 'chef/chef_fs/file_system/chef_server/org_entry'
@@ -141,7 +142,7 @@ class Chef
             @children ||= begin
               result = [
                 # /cookbooks
-                CookbooksDir.new("cookbooks", self),
+                versioned_cookbooks ? VersionedCookbooksDir.new("cookbooks", self) : CookbooksDir.new("cookbooks", self),
                 # /data_bags
                 DataBagsDir.new("data_bags", self, "data"),
                 # /environments
diff --git a/lib/chef/chef_fs/file_system/chef_server/cookbook_dir.rb b/lib/chef/chef_fs/file_system/chef_server/cookbook_dir.rb
index 51de0bfd86..4bc7842f8c 100644
--- a/lib/chef/chef_fs/file_system/chef_server/cookbook_dir.rb
+++ b/lib/chef/chef_fs/file_system/chef_server/cookbook_dir.rb
@@ -28,23 +28,23 @@ class Chef
   module ChefFS
     module FileSystem
       module ChefServer
+        # Unversioned cookbook.
+        #
+        #   /cookbooks/NAME
+        #
+        # Children look like:
+        #
+        # - metadata.rb
+        # - attributes/
+        # - libraries/
+        # - recipes/
+        #
         class CookbookDir < BaseFSDir
           def initialize(name, parent, options = {})
             super(name, parent)
             @exists = options[:exists]
-            # If the name is apache2-1.0.0 and versioned_cookbooks is on, we know
-            # the actual cookbook_name and version.
-            if root.versioned_cookbooks
-              if name =~ VALID_VERSIONED_COOKBOOK_NAME
-                @cookbook_name = $1
-                @version = $2
-              else
-                @exists = false
-              end
-            else
-              @cookbook_name = name
-              @version = root.cookbook_version # nil unless --cookbook-version specified in download/diff
-            end
+            @cookbook_name = name
+            @version = root.cookbook_version # nil unless --cookbook-version specified in download/diff
           end
 
           attr_reader :cookbook_name, :version
@@ -61,10 +61,6 @@ class Chef
             :root_files => { }
           }
 
-          # See Erchef code
-          # https://github.com/opscode/chef_objects/blob/968a63344d38fd507f6ace05f73d53e9cd7fb043/src/chef_regex.erl#L94
-          VALID_VERSIONED_COOKBOOK_NAME = /^([.a-zA-Z0-9_-]+)-(\d+\.\d+\.\d+)$/
-
           def add_child(child)
             @children << child
           end
diff --git a/lib/chef/chef_fs/file_system/chef_server/cookbooks_dir.rb b/lib/chef/chef_fs/file_system/chef_server/cookbooks_dir.rb
index 28d88ea330..5fd8111d7a 100644
--- a/lib/chef/chef_fs/file_system/chef_server/cookbooks_dir.rb
+++ b/lib/chef/chef_fs/file_system/chef_server/cookbooks_dir.rb
@@ -29,6 +29,15 @@ class Chef
   module ChefFS
     module FileSystem
       module ChefServer
+        # cookbooks/
+        #   apache2/
+        #   mysql/
+        # cookbook_artifacts/
+        #   apache2/
+        #     1.0.0/
+        #     1.0.1/
+        #   mysql/
+        #     2.0.5/
         class CookbooksDir < RestListDir
 
           include Chef::Mixin::FileClass
@@ -40,16 +49,7 @@ class Chef
 
           def children
             @children ||= begin
-              if root.versioned_cookbooks
-                result = []
-                root.get_json("#{api_path}/?num_versions=all").each_pair do |cookbook_name, cookbooks|
-                  cookbooks['versions'].each do |cookbook_version|
-                    result << CookbookDir.new("#{cookbook_name}-#{cookbook_version['version']}", self, :exists => true)
-                  end
-                end
-              else
-                result = root.get_json(api_path).keys.map { |cookbook_name| CookbookDir.new(cookbook_name, self, :exists => true) }
-              end
+              result = root.get_json(api_path).keys.map { |cookbook_name| CookbookDir.new(cookbook_name, self, exists: true) }
               result.sort_by(&:name)
             end
           end
@@ -60,7 +60,7 @@ class Chef
           end
 
           def upload_cookbook_from(other, options = {})
-            root.versioned_cookbooks ? upload_versioned_cookbook(other, options) : upload_unversioned_cookbook(other, options)
+            upload_cookbook(other, options)
           rescue Timeout::Error => e
             raise Chef::ChefFS::FileSystem::OperationFailedError.new(:write, self, e, "Timeout writing: #{e}")
           rescue Net::HTTPServerException => e
@@ -74,53 +74,13 @@ class Chef
             raise Chef::ChefFS::FileSystem::CookbookFrozenError.new(:write, self, e, "Cookbook #{other.name} is frozen")
           end
 
-          # Knife currently does not understand versioned cookbooks
-          # Cookbook Version uploader also requires a lot of refactoring
-          # to make this work. So instead, we make a temporary cookbook
-          # symlinking back to real cookbook, and upload the proxy.
-          def upload_versioned_cookbook(other, options)
-            cookbook_name = Chef::ChefFS::FileSystem::Repository::ChefRepositoryFileSystemCookbookDir.canonical_cookbook_name(other.name)
-
-            Dir.mktmpdir do |temp_cookbooks_path|
-              proxy_cookbook_path = "#{temp_cookbooks_path}/#{cookbook_name}"
-
-              # Make a symlink
-              file_class.symlink other.file_path, proxy_cookbook_path
-
-              # Instantiate a proxy loader using the temporary symlink
-              proxy_loader = Chef::Cookbook::CookbookVersionLoader.new(proxy_cookbook_path, other.parent.chefignore)
-              proxy_loader.load_cookbooks
-
-              cookbook_to_upload = proxy_loader.cookbook_version
-              cookbook_to_upload.freeze_version if options[:freeze]
-
-              # Instantiate a new uploader based on the proxy loader
-              uploader = Chef::CookbookUploader.new(cookbook_to_upload, :force => options[:force], :rest => root.chef_rest)
-
-              with_actual_cookbooks_dir(temp_cookbooks_path) do
-                upload_cookbook!(uploader)
-              end
-
-              #
-              # When the temporary directory is being deleted on
-              # windows, the contents of the symlink under that
-              # directory is also deleted. So explicitly remove
-              # the symlink without removing the original contents if we
-              # are running on windows
-              #
-              if Chef::Platform.windows?
-                Dir.rmdir proxy_cookbook_path
-              end
-            end
-          end
-
-          def upload_unversioned_cookbook(other, options)
+          def upload_cookbook(other, options)
             cookbook_to_upload = other.chef_object
             cookbook_to_upload.freeze_version if options[:freeze]
             uploader = Chef::CookbookUploader.new(cookbook_to_upload, :force => options[:force], :rest => root.chef_rest)
 
             with_actual_cookbooks_dir(other.parent.file_path) do
-              upload_cookbook!(uploader)
+              uploader.upload_cookbooks
             end
           end
 
@@ -134,18 +94,8 @@ class Chef
             Chef::Config.cookbook_path = old_cookbook_path
           end
 
-          def upload_cookbook!(uploader, options = {})
-            if uploader.respond_to?(:upload_cookbook)
-              uploader.upload_cookbook
-            else
-              uploader.upload_cookbooks
-            end
-          end
-
           def can_have_child?(name, is_dir)
-            return false if !is_dir
-            return false if root.versioned_cookbooks && name !~ Chef::ChefFS::FileSystem::ChefServer::CookbookDir::VALID_VERSIONED_COOKBOOK_NAME
-            return true
+            is_dir
           end
         end
       end
diff --git a/lib/chef/chef_fs/file_system/chef_server/versioned_cookbook_dir.rb b/lib/chef/chef_fs/file_system/chef_server/versioned_cookbook_dir.rb
new file mode 100644
index 0000000000..25b3fa38cc
--- /dev/null
+++ b/lib/chef/chef_fs/file_system/chef_server/versioned_cookbook_dir.rb
@@ -0,0 +1,45 @@
+#
+# Author:: John Keiser (<jkeiser@opscode.com>)
+# Copyright:: Copyright (c) 2012 Opscode, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require 'chef/chef_fs/file_system/chef_server/versioned_cookbook_dir'
+
+class Chef
+  module ChefFS
+    module FileSystem
+      module ChefServer
+        class VersionedCookbookDir < CookbookDir
+          # See Erchef code
+          # https://github.com/opscode/chef_objects/blob/968a63344d38fd507f6ace05f73d53e9cd7fb043/src/chef_regex.erl#L94
+          VALID_VERSIONED_COOKBOOK_NAME = /^([.a-zA-Z0-9_-]+)-(\d+\.\d+\.\d+)$/
+
+          def initialize(name, parent, options = {})
+            super(name, parent)
+            # If the name is apache2-1.0.0 and versioned_cookbooks is on, we know
+            # the actual cookbook_name and version.
+            if name =~ VALID_VERSIONED_COOKBOOK_NAME
+              @cookbook_name = $1
+              @version = $2
+            else
+              @exists = false
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/chef/chef_fs/file_system/chef_server/versioned_cookbooks_dir.rb b/lib/chef/chef_fs/file_system/chef_server/versioned_cookbooks_dir.rb
new file mode 100644
index 0000000000..bccb34bb37
--- /dev/null
+++ b/lib/chef/chef_fs/file_system/chef_server/versioned_cookbooks_dir.rb
@@ -0,0 +1,100 @@
+#
+# Author:: John Keiser (<jkeiser@opscode.com>)
+# Copyright:: Copyright (c) 2012 Opscode, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require 'chef/chef_fs/file_system/chef_server/cookbooks_dir'
+require 'chef/chef_fs/file_system/chef_server/versioned_cookbook_dir'
+
+class Chef
+  module ChefFS
+    module FileSystem
+      module ChefServer
+        # /cookbooks
+        #
+        # Its children look like:
+        #
+        # - apache2-1.0.0
+        # - apache2-1.0.1
+        # - mysql-2.0.5
+        #
+        class VersionedCookbooksDir < CookbooksDir
+
+          def make_child_entry(name)
+            result = @children.select { |child| child.name == name }.first if @children
+            result || VersionedCookbookDir.new(name, self)
+          end
+
+          def children
+            @children ||= begin
+              result = []
+              root.get_json("#{api_path}/?num_versions=all").each_pair do |cookbook_name, cookbooks|
+                cookbooks['versions'].each do |cookbook_version|
+                  result << VersionedCookbookDir.new("#{cookbook_name}-#{cookbook_version['version']}", self)
+                end
+              end
+              result.sort_by(&:name)
+            end
+          end
+
+          # Knife currently does not understand versioned cookbooks
+          # Cookbook Version uploader also requires a lot of refactoring
+          # to make this work. So instead, we make a temporary cookbook
+          # symlinking back to real cookbook, and upload the proxy.
+          def upload_cookbook(other, options)
+            cookbook_name = Chef::ChefFS::FileSystem::Repository::ChefRepositoryFileSystemCookbookDir.canonical_cookbook_name(other.name)
+
+            Dir.mktmpdir do |temp_cookbooks_path|
+              proxy_cookbook_path = "#{temp_cookbooks_path}/#{cookbook_name}"
+
+              # Make a symlink
+              file_class.symlink other.file_path, proxy_cookbook_path
+
+              # Instantiate a proxy loader using the temporary symlink
+              proxy_loader = Chef::Cookbook::CookbookVersionLoader.new(proxy_cookbook_path, other.parent.chefignore)
+              proxy_loader.load_cookbooks
+
+              cookbook_to_upload = proxy_loader.cookbook_version
+              cookbook_to_upload.freeze_version if options[:freeze]
+
+              # Instantiate a new uploader based on the proxy loader
+              uploader = Chef::CookbookUploader.new(cookbook_to_upload, :force => options[:force], :rest => root.chef_rest)
+
+              with_actual_cookbooks_dir(temp_cookbooks_path) do
+                uploader.upload_cookbooks
+              end
+
+              #
+              # When the temporary directory is being deleted on
+              # windows, the contents of the symlink under that
+              # directory is also deleted. So explicitly remove
+              # the symlink without removing the original contents if we
+              # are running on windows
+              #
+              if Chef::Platform.windows?
+                Dir.rmdir proxy_cookbook_path
+              end
+            end
+          end
+
+          def can_have_child?(name, is_dir)
+            is_dir && name =~ Chef::ChefFS::FileSystem::ChefServer::VersionedCookbookDir::VALID_VERSIONED_COOKBOOK_NAME
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_cookbook_dir.rb b/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_cookbook_dir.rb
index 91b8652de3..e6557bd05d 100644
--- a/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_cookbook_dir.rb
+++ b/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_cookbook_dir.rb
@@ -18,6 +18,7 @@
 
 require 'chef/chef_fs/file_system/repository/chef_repository_file_system_cookbook_entry'
 require 'chef/chef_fs/file_system/chef_server/cookbook_dir'
+require 'chef/chef_fs/file_system/chef_server/versioned_cookbook_dir'
 require 'chef/chef_fs/file_system/not_found_error'
 require 'chef/cookbook/chefignore'
 require 'chef/cookbook/cookbook_version_loader'
@@ -27,25 +28,9 @@ class Chef
     module FileSystem
       module Repository
         class ChefRepositoryFileSystemCookbookDir < ChefRepositoryFileSystemCookbookEntry
-          def initialize(name, parent, file_path = nil)
-            super(name, parent, file_path)
-          end
-
           def chef_object
             begin
-              loader = Chef::Cookbook::CookbookVersionLoader.new(file_path, parent.chefignore)
-              # We need the canonical cookbook name if we are using versioned cookbooks, but we don't
-              # want to spend a lot of time adding code to the main Chef libraries
-              if root.versioned_cookbooks
-                canonical_name = canonical_cookbook_name(File.basename(file_path))
-                raise "When versioned_cookbooks mode is on, cookbook #{file_path} must match format <cookbook_name>-x.y.z"  unless canonical_name
-
-                # KLUDGE: We shouldn't have to use instance_variable_set
-                loader.instance_variable_set(:@cookbook_name, canonical_name)
-              end
-
-              loader.load_cookbooks
-              cb = loader.cookbook_version
+              cb = cookbook_version
               if !cb
                 Chef::Log.error("Cookbook #{file_path} empty.")
                 raise "Cookbook #{file_path} empty."
@@ -74,7 +59,7 @@ class Chef
 
           # Exposed as a class method so that it can be used elsewhere
           def self.canonical_cookbook_name(entry_name)
-            name_match = Chef::ChefFS::FileSystem::ChefServer::CookbookDir::VALID_VERSIONED_COOKBOOK_NAME.match(entry_name)
+            name_match = Chef::ChefFS::FileSystem::ChefServer::VersionedCookbookDir::VALID_VERSIONED_COOKBOOK_NAME.match(entry_name)
             return nil if name_match.nil?
             return name_match[1]
           end
@@ -97,6 +82,12 @@ class Chef
             segment_info = Chef::ChefFS::FileSystem::ChefServer::CookbookDir::COOKBOOK_SEGMENT_INFO[child_name.to_sym] || {}
             ChefRepositoryFileSystemCookbookEntry.new(child_name, self, nil, segment_info[:ruby_only], segment_info[:recursive])
           end
+
+          def cookbook_version
+            loader = Chef::Cookbook::CookbookVersionLoader.new(file_path, parent.chefignore)
+            loader.load_cookbooks
+            cb = loader.cookbook_version
+          end
         end
       end
     end
diff --git a/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_root_dir.rb b/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_root_dir.rb
index c189323c9a..2e9eeea0ee 100644
--- a/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_root_dir.rb
+++ b/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_root_dir.rb
@@ -20,6 +20,7 @@ require 'chef/chef_fs/file_system/base_fs_dir'
 require 'chef/chef_fs/file_system/repository/chef_repository_file_system_entry'
 require 'chef/chef_fs/file_system/repository/chef_repository_file_system_acls_dir'
 require 'chef/chef_fs/file_system/repository/chef_repository_file_system_cookbooks_dir'
+require 'chef/chef_fs/file_system/repository/chef_repository_file_system_versioned_cookbooks_dir'
 require 'chef/chef_fs/file_system/repository/chef_repository_file_system_data_bags_dir'
 require 'chef/chef_fs/file_system/multiplexed_dir'
 require 'chef/chef_fs/data_handler/client_data_handler'
@@ -162,7 +163,11 @@ class Chef
             end
             case name
             when 'cookbooks'
-              dirs = paths.map { |path| ChefRepositoryFileSystemCookbooksDir.new(name, self, path) }
+              if versioned_cookbooks
+                dirs = paths.map { |path| ChefRepositoryFileSystemVersionedCookbooksDir.new(name, self, path) }
+              else
+                dirs = paths.map { |path| ChefRepositoryFileSystemCookbooksDir.new(name, self, path) }
+              end
             when 'data_bags'
               dirs = paths.map { |path| ChefRepositoryFileSystemDataBagsDir.new(name, self, path) }
             when 'acls'
diff --git a/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_versioned_cookbook_dir.rb b/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_versioned_cookbook_dir.rb
new file mode 100644
index 0000000000..cf9378dbf9
--- /dev/null
+++ b/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_versioned_cookbook_dir.rb
@@ -0,0 +1,42 @@
+#
+# Author:: John Keiser (<jkeiser@opscode.com>)
+# Copyright:: Copyright (c) 2013 Opscode, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require 'chef/chef_fs/file_system/repository/chef_repository_file_system_cookbook_dir'
+
+class Chef
+  module ChefFS
+    module FileSystem
+      module Repository
+        class ChefRepositoryFileSystemVersionedCookbookDir < ChefRepositoryFileSystemCookbookDir
+          # Override from parent
+          def cookbook_version
+            loader = Chef::Cookbook::CookbookVersionLoader.new(file_path, parent.chefignore)
+            # We need the canonical cookbook name if we are using versioned cookbooks, but we don't
+            # want to spend a lot of time adding code to the main Chef libraries
+            canonical_name = canonical_cookbook_name(File.basename(file_path))
+            raise "When versioned_cookbooks mode is on, cookbook #{file_path} must match format <cookbook_name>-x.y.z"  unless canonical_name
+            # KLUDGE: We shouldn't have to use instance_variable_set
+            loader.instance_variable_set(:@cookbook_name, canonical_name)
+            loader.load_cookbooks
+            loader.cookbook_version
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_versioned_cookbooks_dir.rb b/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_versioned_cookbooks_dir.rb
new file mode 100644
index 0000000000..322944fc82
--- /dev/null
+++ b/lib/chef/chef_fs/file_system/repository/chef_repository_file_system_versioned_cookbooks_dir.rb
@@ -0,0 +1,34 @@
+#
+# Author:: John Keiser (<jkeiser@opscode.com>)
+# Copyright:: Copyright (c) 2013 Opscode, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require 'chef/chef_fs/file_system/repository/chef_repository_file_system_cookbooks_dir'
+require 'chef/chef_fs/file_system/repository/chef_repository_file_system_versioned_cookbook_dir'
+
+class Chef
+  module ChefFS
+    module FileSystem
+      module Repository
+        class ChefRepositoryFileSystemVersionedCookbooksDir < ChefRepositoryFileSystemCookbooksDir
+          def make_child_entry(child_name)
+            ChefRepositoryFileSystemVersionedCookbookDir.new(child_name, self)
+          end
+        end
+      end
+    end
+  end
+end