Add indirection to allow switching policy builder implementations

3 files changed, 163 insertions, 157 deletions

diff --git a/lib/chef/client.rb b/lib/chef/client.rb
index 390dc247ab..3b50874db9 100644
--- a/lib/chef/client.rb
+++ b/lib/chef/client.rb
@@ -244,7 +244,7 @@ class Chef
     end
 
     def policy_builder
-      @policy_builder ||= Chef::PolicyBuilder.new(node_name, ohai.data, json_attribs, @override_runlist, events)
+      @policy_builder ||= Chef::PolicyBuilder.strategy.new(node_name, ohai.data, json_attribs, @override_runlist, events)
     end
 
 
diff --git a/lib/chef/policy_builder.rb b/lib/chef/policy_builder.rb
index 37e7f8f3fa..2d32b62cbb 100644
--- a/lib/chef/policy_builder.rb
+++ b/lib/chef/policy_builder.rb
@@ -41,197 +41,203 @@ class Chef
   # * a new RunStatus (probably doesn't need to be here)
   # * cookbooks sync'd to disk
   # * cookbook_hash is stored in run_context
-  class PolicyBuilder
-
-    attr_reader :events
-    attr_reader :node
-    attr_reader :node_name
-    attr_reader :ohai_data
-    attr_reader :json_attribs
-    attr_reader :override_runlist
-    attr_reader :original_runlist
-    attr_reader :run_context
-    attr_reader :run_list_expansion
-
-    def initialize(node_name, ohai_data, json_attribs, override_runlist, events)
-      @node_name = node_name
-      @ohai_data = ohai_data
-      @json_attribs = json_attribs
-      @override_runlist = override_runlist
-      @events = events
-
-      @node = nil
-      @original_runlist = nil
-      @run_list_expansion = nil
+  module PolicyBuilder
+
+    def self.strategy
+      ExpandNodeObject
     end
 
-    def setup_run_context(specific_recipes=nil)
-      if Chef::Config[:solo]
-        Chef::Cookbook::FileVendor.on_create { |manifest| Chef::Cookbook::FileSystemFileVendor.new(manifest, Chef::Config[:cookbook_path]) }
-        cl = Chef::CookbookLoader.new(Chef::Config[:cookbook_path])
-        cl.load_cookbooks
-        cookbook_collection = Chef::CookbookCollection.new(cl)
-        run_context = Chef::RunContext.new(node, cookbook_collection, @events)
-      else
-        Chef::Cookbook::FileVendor.on_create { |manifest| Chef::Cookbook::RemoteFileVendor.new(manifest, api_service) }
-        cookbook_hash = sync_cookbooks
-        cookbook_collection = Chef::CookbookCollection.new(cookbook_hash)
-        run_context = Chef::RunContext.new(node, cookbook_collection, @events)
+    class ExpandNodeObject
+      attr_reader :events
+      attr_reader :node
+      attr_reader :node_name
+      attr_reader :ohai_data
+      attr_reader :json_attribs
+      attr_reader :override_runlist
+      attr_reader :original_runlist
+      attr_reader :run_context
+      attr_reader :run_list_expansion
+
+      def initialize(node_name, ohai_data, json_attribs, override_runlist, events)
+        @node_name = node_name
+        @ohai_data = ohai_data
+        @json_attribs = json_attribs
+        @override_runlist = override_runlist
+        @events = events
+
+        @node = nil
+        @original_runlist = nil
+        @run_list_expansion = nil
       end
 
-      # TODO: this is not the place for this. It should be in Runner or
-      # CookbookCompiler or something.
-      run_context.load(@run_list_expansion)
-      if specific_recipes
-        specific_recipes.each do |recipe_file|
-          run_context.load_recipe_file(recipe_file)
+      def setup_run_context(specific_recipes=nil)
+        if Chef::Config[:solo]
+          Chef::Cookbook::FileVendor.on_create { |manifest| Chef::Cookbook::FileSystemFileVendor.new(manifest, Chef::Config[:cookbook_path]) }
+          cl = Chef::CookbookLoader.new(Chef::Config[:cookbook_path])
+          cl.load_cookbooks
+          cookbook_collection = Chef::CookbookCollection.new(cl)
+          run_context = Chef::RunContext.new(node, cookbook_collection, @events)
+        else
+          Chef::Cookbook::FileVendor.on_create { |manifest| Chef::Cookbook::RemoteFileVendor.new(manifest, api_service) }
+          cookbook_hash = sync_cookbooks
+          cookbook_collection = Chef::CookbookCollection.new(cookbook_hash)
+          run_context = Chef::RunContext.new(node, cookbook_collection, @events)
         end
-      end
-      run_context
-    end
-
 
-    # In client-server operation, loads the node state from the server. In
-    # chef-solo operation, builds a new node object.
-    def load_node
-      events.node_load_start(node_name, Chef::Config)
-      Chef::Log.debug("Building node object for #{node_name}")
-
-      if Chef::Config[:solo]
-        @node = Chef::Node.build(node_name)
-      else
-        @node = Chef::Node.find_or_create(node_name)
+        # TODO: this is not the place for this. It should be in Runner or
+        # CookbookCompiler or something.
+        run_context.load(@run_list_expansion)
+        if specific_recipes
+          specific_recipes.each do |recipe_file|
+            run_context.load_recipe_file(recipe_file)
+          end
+        end
+        run_context
       end
-    rescue Exception => e
-      # TODO: wrap this exception so useful error info can be given to the
-      # user.
-      events.node_load_failed(node_name, e, Chef::Config)
-      raise
-    end
 
 
-    # Applies environment, external JSON attributes, and override run list to
-    # the node, Then expands the run_list.
-    #
-    # === Returns
-    # node<Chef::Node>:: The modified node object. node is modified in place.
-    def build_node
-      # Allow user to override the environment of a node by specifying
-      # a config parameter.
-      if Chef::Config[:environment] && !Chef::Config[:environment].chop.empty?
-        node.chef_environment(Chef::Config[:environment])
+      # In client-server operation, loads the node state from the server. In
+      # chef-solo operation, builds a new node object.
+      def load_node
+        events.node_load_start(node_name, Chef::Config)
+        Chef::Log.debug("Building node object for #{node_name}")
+
+        if Chef::Config[:solo]
+          @node = Chef::Node.build(node_name)
+        else
+          @node = Chef::Node.find_or_create(node_name)
+        end
+      rescue Exception => e
+        # TODO: wrap this exception so useful error info can be given to the
+        # user.
+        events.node_load_failed(node_name, e, Chef::Config)
+        raise
       end
 
-      # consume_external_attrs may add items to the run_list. Save the
-      # expanded run_list, which we will pass to the server later to
-      # determine which versions of cookbooks to use.
-      node.reset_defaults_and_overrides
-      node.consume_external_attrs(ohai_data, @json_attribs)
 
-      setup_run_list_override
+      # Applies environment, external JSON attributes, and override run list to
+      # the node, Then expands the run_list.
+      #
+      # === Returns
+      # node<Chef::Node>:: The modified node object. node is modified in place.
+      def build_node
+        # Allow user to override the environment of a node by specifying
+        # a config parameter.
+        if Chef::Config[:environment] && !Chef::Config[:environment].chop.empty?
+          node.chef_environment(Chef::Config[:environment])
+        end
 
-      @run_list_expansion = expand_run_list
+        # consume_external_attrs may add items to the run_list. Save the
+        # expanded run_list, which we will pass to the server later to
+        # determine which versions of cookbooks to use.
+        node.reset_defaults_and_overrides
+        node.consume_external_attrs(ohai_data, @json_attribs)
 
-      # @run_list_expansion is a RunListExpansion.
-      #
-      # Convert @expanded_run_list, which is an
-      # Array of Hashes of the form
-      #   {:name => NAME, :version_constraint => Chef::VersionConstraint },
-      # into @expanded_run_list_with_versions, an
-      # Array of Strings of the form
-      #   "#{NAME}@#{VERSION}"
-      @expanded_run_list_with_versions = @run_list_expansion.recipes.with_version_constraints_strings
+        setup_run_list_override
 
-      Chef::Log.info("Run List is [#{node.run_list}]")
-      Chef::Log.info("Run List expands to [#{@expanded_run_list_with_versions.join(', ')}]")
+        @run_list_expansion = expand_run_list
 
+        # @run_list_expansion is a RunListExpansion.
+        #
+        # Convert @expanded_run_list, which is an
+        # Array of Hashes of the form
+        #   {:name => NAME, :version_constraint => Chef::VersionConstraint },
+        # into @expanded_run_list_with_versions, an
+        # Array of Strings of the form
+        #   "#{NAME}@#{VERSION}"
+        @expanded_run_list_with_versions = @run_list_expansion.recipes.with_version_constraints_strings
 
-      events.node_load_completed(node, @expanded_run_list_with_versions, Chef::Config)
+        Chef::Log.info("Run List is [#{node.run_list}]")
+        Chef::Log.info("Run List expands to [#{@expanded_run_list_with_versions.join(', ')}]")
 
-      node
-    end
 
-    ########################################
-    # Internal public API
-    ########################################
+        events.node_load_completed(node, @expanded_run_list_with_versions, Chef::Config)
 
-    def expand_run_list
-      if Chef::Config[:solo]
-        node.expand!('disk')
-      else
-        node.expand!('server')
+        node
       end
-    rescue Exception => e
-      # TODO: wrap/munge exception with useful error output.
-      events.run_list_expand_failed(node, e)
-      raise
-    end
 
-    # Sync_cookbooks eagerly loads all files except files and
-    # templates.  It returns the cookbook_hash -- the return result
-    # from /environments/#{node.chef_environment}/cookbook_versions,
-    # which we will use for our run_context.
-    #
-    # === Returns
-    # Hash:: The hash of cookbooks with download URLs as given by the server
-    def sync_cookbooks
-      Chef::Log.debug("Synchronizing cookbooks")
-
-      begin
-        events.cookbook_resolution_start(@expanded_run_list_with_versions)
-        cookbook_hash = api_service.post("environments/#{node.chef_environment}/cookbook_versions",
-                                       {:run_list => @expanded_run_list_with_versions})
+      ########################################
+      # Internal public API
+      ########################################
+
+      def expand_run_list
+        if Chef::Config[:solo]
+          node.expand!('disk')
+        else
+          node.expand!('server')
+        end
       rescue Exception => e
-        # TODO: wrap/munge exception to provide helpful error output
-        events.cookbook_resolution_failed(@expanded_run_list_with_versions, e)
+        # TODO: wrap/munge exception with useful error output.
+        events.run_list_expand_failed(node, e)
         raise
-      else
-        events.cookbook_resolution_complete(cookbook_hash)
       end
 
-      synchronizer = Chef::CookbookSynchronizer.new(cookbook_hash, events)
-      synchronizer.sync_cookbooks
+      # Sync_cookbooks eagerly loads all files except files and
+      # templates.  It returns the cookbook_hash -- the return result
+      # from /environments/#{node.chef_environment}/cookbook_versions,
+      # which we will use for our run_context.
+      #
+      # === Returns
+      # Hash:: The hash of cookbooks with download URLs as given by the server
+      def sync_cookbooks
+        Chef::Log.debug("Synchronizing cookbooks")
+
+        begin
+          events.cookbook_resolution_start(@expanded_run_list_with_versions)
+          cookbook_hash = api_service.post("environments/#{node.chef_environment}/cookbook_versions",
+                                         {:run_list => @expanded_run_list_with_versions})
+        rescue Exception => e
+          # TODO: wrap/munge exception to provide helpful error output
+          events.cookbook_resolution_failed(@expanded_run_list_with_versions, e)
+          raise
+        else
+          events.cookbook_resolution_complete(cookbook_hash)
+        end
 
-      # register the file cache path in the cookbook path so that CookbookLoader actually picks up the synced cookbooks
-      Chef::Config[:cookbook_path] = File.join(Chef::Config[:file_cache_path], "cookbooks")
+        synchronizer = Chef::CookbookSynchronizer.new(cookbook_hash, events)
+        synchronizer.sync_cookbooks
 
-      cookbook_hash
-    end
+        # register the file cache path in the cookbook path so that CookbookLoader actually picks up the synced cookbooks
+        Chef::Config[:cookbook_path] = File.join(Chef::Config[:file_cache_path], "cookbooks")
 
-    def setup_run_list_override
-      runlist_override_sanity_check!
-      unless(override_runlist.empty?)
-        @original_runlist = node.run_list.run_list_items.dup
-        node.run_list(*override_runlist)
-        Chef::Log.warn "Run List override has been provided."
-        Chef::Log.warn "Original Run List: [#{original_runlist.join(', ')}]"
-        Chef::Log.warn "Overridden Run List: [#{node.run_list}]"
+        cookbook_hash
       end
-    end
 
-    # Ensures runlist override contains RunListItem instances
-    def runlist_override_sanity_check!
-      # Convert to array and remove whitespace
-      if override_runlist.is_a?(String)
-        @override_runlist = override_runlist.split(',').map { |e| e.strip }
+      def setup_run_list_override
+        runlist_override_sanity_check!
+        unless(override_runlist.empty?)
+          @original_runlist = node.run_list.run_list_items.dup
+          node.run_list(*override_runlist)
+          Chef::Log.warn "Run List override has been provided."
+          Chef::Log.warn "Original Run List: [#{original_runlist.join(', ')}]"
+          Chef::Log.warn "Overridden Run List: [#{node.run_list}]"
+        end
       end
-      @override_runlist = [override_runlist].flatten.compact
-      override_runlist.map! do |item|
-        if(item.is_a?(Chef::RunList::RunListItem))
-          item
-        else
-          Chef::RunList::RunListItem.new(item)
+
+      # Ensures runlist override contains RunListItem instances
+      def runlist_override_sanity_check!
+        # Convert to array and remove whitespace
+        if override_runlist.is_a?(String)
+          @override_runlist = override_runlist.split(',').map { |e| e.strip }
+        end
+        @override_runlist = [override_runlist].flatten.compact
+        override_runlist.map! do |item|
+          if(item.is_a?(Chef::RunList::RunListItem))
+            item
+          else
+            Chef::RunList::RunListItem.new(item)
+          end
         end
       end
-    end
 
-    def api_service
-      @api_service ||= Chef::REST.new(config[:chef_server_url])
-    end
+      def api_service
+        @api_service ||= Chef::REST.new(config[:chef_server_url])
+      end
 
-    def config
-      Chef::Config
-    end
+      def config
+        Chef::Config
+      end
 
+    end
   end
 end
diff --git a/spec/unit/policy_builder_spec.rb b/spec/unit/policy_builder_spec.rb
index 4b95c079b7..8b1b55163d 100644
--- a/spec/unit/policy_builder_spec.rb
+++ b/spec/unit/policy_builder_spec.rb
@@ -26,7 +26,7 @@ describe Chef::PolicyBuilder do
   let(:json_attribs) { {"run_list" => []} }
   let(:override_runlist) { "recipe[foo::default]" }
   let(:events) { Chef::EventDispatch::Dispatcher.new }
-  let(:policy_builder) { Chef::PolicyBuilder.new(node_name, ohai_data, json_attribs, override_runlist, events) }
+  let(:policy_builder) { Chef::PolicyBuilder::ExpandNodeObject.new(node_name, ohai_data, json_attribs, override_runlist, events) }
 
   # All methods that Chef::Client calls on this class.
   describe "Public API" do